On Mon, Oct 22, 2012 at 12:26 PM, Marius Dumitru Florea < [email protected]> 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. > Well, I am more like Marius, and imagine Thomas will agree, that it would be better to remove that technical details from the XML as soon as we can. I also agree with Thomas that it is not nice to break a format we have kept stable since very long. However, we will need to break it soon or later, if we want real improvement. This is a usual situation with file formats, at some point you need to introduce some versioning. So, why not starting now ? What I mean by introducing versioning is: 1) Add a version to the file, stating its format version 2) Support loading any earlier formats 3) Support saving to an older format 2) and 3) could be implemented using XSLT or a similar kind of converter, so it will avoid garbaging our latest format version. WDYT ? > > 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 > -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
