On Mon, Oct 22, 2012 at 7: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. If the extension was developed on 4.3 then it may use APIs that are not available in previous versions so it might not work in older versions even if the XAR format is the same. Now, I agree that it would be nice if the extension would work in previous versions provided the API compatibility is satisfied, but since we cannot guarantee this all the time I don't think 'breaking the XAR format' is that bad :) Thanks, Marius > > 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 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
