Hi, Marius!

2012-11-07 09:39, Marius Dumitru Florea skrev:
> On Mon, Nov 5, 2012 at 8:25 AM, Andreas Jonsson <[email protected]> wrote:
>> Hi,
>>
>> I am requesting a vote for adding the following clirr difference:
>>
>>             <difference>
>>               <className>com/xpn/xwiki/objects/ListProperty</className>
>>               <field>list</field>
>>               <differenceType>6006</differenceType>
>>               <justification>Elemenents must be added to the wrapping
>> NotifyOnUpdateList to ensure that the property is marked 'dirty' when
>> updated. To avoid that this mechanism is circumvented, the field is made
>> final.</justification>
>>             </difference>
> +1
>
>> The flag isContentDirty in XWikiDocument is currently always true when a
>> document is loaded from the database.  This means that a script that
>> just load and save a document without modifying it will generate a new
>> version, and the point of the flag is completely lost.  It is also a
>> blocker for my work in the feature-authorization-context branch.
>>
>> Fixing this was non-trivial.  It is assumed everywhere that the content
>> dirty will be set, not only when the document content changes, but also
>> when a property, the xclass or any attachment changes.  But there were
>> no code that actually set the flag on updates (because it was always
>> true anyway).  What's worse is that we have API methods that returns
>> lists that are live updateable.  I have added a wrapper list class to
>> detect the updates in these lists.
>>
>> Hibernate refuse to store elements from my wrapper list for some reason
>> which forced me to add a workaround for hibernate.
>>
>> Surprisingly, Clirr doesn't require any exception for the below methods
>> which I have added, but I will list them here for your information:
> This is normal, because adding new methods to a class (not interface)
> doesn't break (binary) compatibility.

Thanks for clarifying!

>
>> In BaseClass:
>>
>>     public void setOwnerDocument(XWikiDocument ownerDocument)
> BaseClass is extended by PropertyMetaClass which defines the meta
> properties of an XClass property, like dateFormat, multiSelect,
> disabled, etc. Owner document doesn't make sense for a meta class, but
> I guess this is not a problem.
>
>>     public void setDirty(boolean isDirty)
> You need to fix the Javadoc for both methods. @param doesn't match the
> actual parameter name.. Best is to activate Checkstyle from your IDE
> to see these errors right away.
>
>> In BaseProperty:
>>
>>     public boolean isValueDirty()
>>     protected void setValueDirty(Object newValue)
>>     public void setValueDirty(boolean valueDirty)
>>     public void setOwnerDocument(XWikiDocument ownerDocument)
> You need to fix the Javadoc here too. Also, BaseProperty is the base
> class for all raw types (StringProperty, IntegerProperty, etc.) which
> are not bound to any document so owner document doesn't make sense for
> them. It makes sense only for XClass properties (which extend
> PropertyClass, which is derived from BaseClass).
>
> Sergiu or Ludovic, who know this part of the code better, should tell
> us if adding this method here is fine.

The "owner document" is optional and may be set to null wherever it
doesn't make sense to have it.  This is also true for BaseClass above. 
The important thing is that it is set correctly when a property is
associated with an xobject that is associated with a document.


Best Regards,

/Andreas

>> In ListProperty:
>>
>>     public void setUseHibernateWorkaround(boolean useHibernateWorkaround)
> Could you improve the Javadoc? It doesn't explain anything, it doesn't
> say which workaround. The old core is already very cryptic..
>
> Thanks,
> Marius
>
>> In XWikiAttachmentContent:
>>
>>     public void setOwnerDocument(XWikiDocument ownerDocument)
>>
>> New class: AbstractNotifyOnUpdateList<E>
>>
>>
>> Best Regards,
>>
>> /Andreas
>>
>> _______________________________________________
>> 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

Reply via email to