On Mon, Oct 22, 2012 at 6:58 PM, Jerome Velociter <[email protected]> wrote: > On 10/22/2012 12:26 PM, Marius Dumitru Florea wrote: >> >> On Mon, Oct 22, 2012 at 1:13 PM, Thomas Mortagne >> <[email protected]> wrote: >>> >>> On Mon, Oct 22, 2012 at 11:33 AM, Marius Dumitru Florea >>> <[email protected]> wrote: >>>> >>>> On Mon, Oct 22, 2012 at 12:15 PM, Thomas Mortagne >>>> <[email protected]> wrote: >>>>> >>>>> On Mon, Oct 22, 2012 at 10:36 AM, Marius Dumitru Florea >>>>> <[email protected]> wrote: >>>>>> >>>>>> Hi devs, >>>>>> >>>>>> Currently, the available XClass property types are hard-coded in >>>>>> MetaClass [1]. So in order to add a new property type you need to >>>>>> recompile the XWiki old core. I'd like to be able to create new >>>>>> property types using components so I created this branch [2] that >>>>>> includes the following important changes: >>>>>> >>>>>> (A) Use the value of the "classType" XML element from the XAR as a >>>>>> property type hint >>>>>> >>>>>> If you look at the XML export of an XClass you'll see that each >>>>>> property has a "classType" element whose content is the full name of >>>>>> the Java class used to implement that property type: >>>>>> >>>>>> <classType>com.xpn.xwiki.objects.classes.DBTreeListClass</classType> >>>>>> >>>>>> I think this is bad because: >>>>>> * it exposes an implementation detail >>>>>> * you cannot change the property type implementation without breaking >>>>>> backwards compatibility >>>>>> >>>>>> So I'm proposing to use the "classType" as a hint for the property >>>>>> type. Well, technically it will be a hint, but semantically it will >>>>>> specify the data type of the property value (e.g. String, Date, >>>>>> Number). For backwards compatibility, the existing property types will >>>>>> have hints that can be extracted from the value of the 'classType' >>>>>> (i.e. the full Java class name): >>>>>> >>>>>> String hint = >>>>>> StringUtils.removeEnd(StringUtils.substringAfterLast(classType, >>>>>> "."), "Class"); >>>>>> >>>>>> So both: >>>>>> >>>>>> <classType>com.xpn.xwiki.objects.classes.DBTreeListClass</classType> >>>>>> <classType>DBTreeList</classType> >>>>>> >>>>>> will point to the property type with hint "DBTreeList". Of course, >>>>>> when exporting an XClass with the new version of the core you'll only >>>>>> see the property type hint. >>>>>> >>>>>> The only issue with this approach is that XClasses exported with the >>>>>> new version will not work on an older version but this is acceptable >>>>>> IMO. >>>>> >>>>> I don't like breaking stuff too much especially when it's far from >>>> >>>> To be clear. This is not backwards compatibility. Are you sure that a >>>> XAR from 4.2 can be imported in XWiki 1.0 for instance? In other >>>> words, whenever we extended the XAR/XML format did we take care to >>>> keep the new XARs working on older versions of XWiki? If the answer is >>>> yes then I agree with your suggestions below. But for me it's enough >>>> to ensure that older XARs work with newer versions of XWiki, not the >>>> other way around. >>> >>> I don't have any breaking change in mind except that obviously a 2.0 >>> document is not going to be parsed properly with XWiki version that >>> only knows 1.0. >>> >>> Anyway the point is not if it's going to fully work with XWiki 1.0. >>> I don't see any reason to break anything here and it's not even like >>> either of my suggestions was introducing any important complexity in >>> your proposal. >> >> Sure. That's what I did initially. My latest code is a bit more >> complex exactly because I didn't wanted to use the full Java class >> name as hint. So it's a choice between: 'don't break anything' and >> 'hide technical details from the XML'. I'm more for the second option >> but let's see what others think. > > > If I read correctly, "breaking the format" here means an extension exported > as XAR from 4.3 can only be installed in 4.3+ installations ? If that's > correct, IMO it's not acceptable.
Yes that's the main example I have in mlnd too. > > Jerome > > >> >> Thanks, >> Marius >> >>>> Thanks, >>>> Marius >>>> >>>>> mandatory technically. We could either: >>>>> * keep theses old classType and have cleaner ones for new types. It's >>>>> not a big deal IMO, we are talking about oldcore API here and we will >>>>> have to introduce a new component API for this anyway with the new >>>>> model. >>>>> * or simply introduce a new field in the class which would fallback on >>>>> String hint = >>>>> StringUtils.removeEnd(StringUtils.substringAfterLast(classType, >>>>> "."), "Class"); when it's not defined >>>>> >>>>>> (B) Add a PropertyClassProvider [3] role to retrieve an instance of a >>>>>> property class and to access its meta class >>>>>> >>>>>> Currently, in order to define a new property type you need to create >>>>>> two Java classes: >>>>>> * one extending PropertyMetaClass, to define the list of meta >>>>>> properties of the new property type (e.g. displayType, dateFormat, >>>>>> multiSelect, etc.) >>>>>> * one extending ProperyClass, to define setters and getters for the >>>>>> meta properties and maybe some custom display. This is the property >>>>>> type itself. >>>>>> >>>>>> So I added a PropertyClassProvider role that has two methods: one to >>>>>> get the meta class and one to get a new instance of the property type >>>>>> (e.g. when adding a property to an XClass). As a quick implementation >>>>>> I transformed all meta classes into implementations of this role. >>>>>> Finally, I modified MetaClass to lookup property class providers >>>>>> instead of hard-coding the property types. >>>>>> >>>>>> WDYT? >>>>>> >>>>>> Note that I've not added any CLIRR excludes and I tested my changes >>>>>> with the default, unchanged, class editor and it works fine. >>>>>> >>>>>> Thanks, >>>>>> Marius >>>>>> >>>>>> [1] >>>>>> https://github.com/xwiki/xwiki-platform/blob/xwiki-platform-4.2/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/meta/MetaClass.java#L33 >>>>>> [2] >>>>>> https://github.com/xwiki/xwiki-platform/compare/feature-xclass-property-component >>>>>> [3] >>>>>> https://github.com/xwiki/xwiki-platform/commit/fb9fcc7313ccb16f38c2d17dd7edf3a8e299a69b#diff-2 >>>>>> _______________________________________________ >>>>>> devs mailing list >>>>>> [email protected] >>>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>>> >>>>> >>>>> >>>>> -- >>>>> Thomas Mortagne >>>>> _______________________________________________ >>>>> devs mailing list >>>>> [email protected] >>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>> >>>> _______________________________________________ >>>> devs mailing list >>>> [email protected] >>>> http://lists.xwiki.org/mailman/listinfo/devs >>> >>> >>> >>> -- >>> Thomas Mortagne >>> _______________________________________________ >>> devs mailing list >>> [email protected] >>> http://lists.xwiki.org/mailman/listinfo/devs >> >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs > > > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs -- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
