On Mon, Nov 5, 2012 at 8:25 AM, Andreas Jonsson <aj(a)member.fsf.org> 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.
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.
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
> devs(a)xwiki.org
>
http://lists.xwiki.org/mailman/listinfo/devs