Bug 100484

I propose we punt on this one for RC1.  Here are the reasons why.

At the time this bug was filed, I was not sure where the source of the
problem was.  It could have been that versioning was not recording the
versioning log correctly. Or it could've been that some properties of
the ContentItem object type needed to be marked "unversioned".

Having looked into this, I am 99% certain that the versioning log is
recorded correctly.  The problem is that the rollback functionality is
implemented in a less than robust fashion.  Currently, this
incorrectness is only manifested, if I remove the call to
Versions.suspendVersioning() from the ContentItem.publish method.  If
I leave things the way they currently are on the tip, then rollback
works.

So,

1. The discovered lack of robustness in the current implementation of
   rollback does not affect anyone at the moment.

2. Once the bug is fixed, the change will not require data model
   upgrades.

3. Fixing the current rollback implementation is going to take 2 to 3
   days.


In light of the above, I say we should defer this ticket until after
RC1.  Sorry it took me so long to get to the bottom of this.

For the curious, here's an outline of what needs to happen for this
ticket to be fixed.

The current lack of rollback robustness is due to a combination of two
factors:

1. The initial implementation was coded up with the assumption that
   the versioning log can be trusted to be "correct," i.e. all the
   relevant events are expected to be there.

2. The current implementation assumes that all objects we are dealing
   with are versioned ACSObjects.


Assumption 1 was useful at one point. It allowed me to add a lot of
assertions about the correctness of the versioning log.  For example,
for any given data object, the sequence of CREATE and DELETE events
must alternate.  You can't have two juxtaposed DELETE events - the
object cannot be deleted twice, without having been resurrected
sometime in between the two deletions.  Having these assertions in
place helped me catch some implementation errors early on.

After the introduction of suspendVersioning/resumeVersioning, most of
these assertion are no longer valid.  The versioning log *can* have
two consecutive DELETE events for the same data object, if the
versioning service was suspended at some point.

So, these assertions need to be either removed, or conditionalized on
whether or not we are running in the "strict" mode. (Having the
"strict" mode where all these assertions are enabled is useful for
unit tests.)

The second factor is the current implementation assumes that it only
has to deal with "fully versioned" data objects.  As of
//core-platform/dev/src/com/arsdigita/versioning/ProxyDataObject.java#11,
the state transition diagram for the ProxyDataObject makes this
assumption.  This assumption mostly holds true in the current system,
as a carry-over from the old days, when you had to derive everything
from VersionedACSObject in order to use the versioning service in a
semi-correct manner.  Addressing factor #1 listed above would lead to
this faulty assumption being broken much more easily.

Fixing #2 requires supporting a more complicated state transition
diagram.  Instead of the ASCII diagram in
//core-platform/dev/src/com/arsdigita/versioning/ProxyDataObject.java#11,
we now have to support something along the lines of
//core-platform/dev/src/com/arsdigita/versioning/doc-files/ProxyDataObject-1.png#3,
which is a good deal more complicated.

Oh, and once I'm done with adding new states and state transitions,
the diffing code implementing the CMS history UI will need to be
revamped, too.  Rafi wrote  it (Justin might've had a hand in it, too),
and I haven't really looked at it yet.

Put on Troika final blockers list for tracking. 

Moved to Rickshaw September

cleaning up stale bugs