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.

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

Reply via email to