[Product-Developers] Fwd: Re: Review of a branch that should make CMFEditions and collective.indexing work together peacefully
az at zitc.de
Tue Mar 2 11:44:33 UTC 2010
forwarding on behalf of alec as the cc to the list got lost...
-------- Original Message --------
Subject: Re: Review of a branch that should make CMFEditions and
collective.indexing work together peacefully
Date: Wed, 24 Feb 2010 10:31:02 -0800
From: Alec Mitchell <apm13 at columbia.edu>
To: Andreas Zeidler <az at zitc.de>
On Wed, Feb 24, 2010 at 5:09 AM, Andreas Zeidler <az at zitc.de> wrote:
> On 22.02.10 16:54, Patrick Gerken wrote:
> hi patrick,
>> I have created a test case and a change in behavior that fixes the issue.
>> Instead of comparing the modified date of new and old object, I check
>> _p_changed attribute. That is a rather drastic change but adding an
>> assertion in
>> the change to compare old modification check vs new modification check
>> showed that both yield the same results in all test cases when used
> great. could you please elaborate a bit more on what the problem was
> here, though?
>> I would be rather grateful if somebody with a bit of
>> collective.indexing knowledge could have
>> a look at it, maybe there is a better way than the one I implemented.
>> My changes are in the do3cc_collective.indexing branch.
> i can have a look on friday, but i'm not sure i'm on top of the
> CMFEditions part. i'm cc'ing alec in the hope he might be able to join
> the tuneup? :)
My understanding is that the combination of collective.indexing's
delayed indexing and the fact that AT sets the modification date on
reindex caused CMFEditions to think that newly modified content was
not actually modified and prevented saving new versions. Essentially,
the CMFEditions script that saves new versions on edit uses a method
portal_modifier.isUpToDate(obj) to check if the object has actually
changed since the last save, if not it doesn't bother to save a new
version (one can always manually save a version without this check
though). Because CMFEditions is intended for use with CMF content it
uses the standard ModificationDate() method to determine if the object
was modified since the last saved version. That interaction between
collective.indexing and AT means that the date is not changed until
the transaction is over and so the indexing does not happen (or
happens only every other save).
I would not call this a bug in CMFEditions though, since the change to
the behavior of ModificationDate really is breaking expectations in a
problematic way. I wouldn't be surprised if other products and
applications rely on checks to ModificationDate within a transaction,
so I suggest we bind that change to some more reliable event than the
reindexing action itself. Using _p_changed (or more appropriately
bobobase_modification_time) may work in some cases, but it is not
equivalent and will certainly fail in a number of fairly common cases
(e.g. when all the changed fields are stored in an annotation
storage). It's not surprising that the tests don't pick up this
failure since the tests don't test anything quite so pathological.
More information about the Product-Developers