AW: [sdo] ISSUE 44: ChangeSummary semantics

["Barack, Ron" <ron.barack@sap.com>]

Title: ISSUE 44: ChangeSummary semantics

Hi Everyone,

 

After some discussion on recent calls, I decided to look at the specification for what would actually have to be changed here.  I actually think my reworking of the text on change summary, specifically 4.2.3, unintentially decided this issue.  If we accept the text as is, then I believe we can close ISSUE 44 as a duplicate.

 

To save everyone opening the document, here is the relevant text:

 

The scope of a ChangeSummary is defined as the set of DataObjects reachable from the ChangeSummary root either through containment or over any orphanHolder properties.  In other words the scope of the change summary is the same set of DataObjects that would be in the sub-tree whose root node is ChangeSummary root, if the graph were serialized to XML.

DataObjects that were in the ChangeSummary scope when logging was activated but are not in the scope when logging was deactivated (or when ChangeSummary.getChangedObjects() was called, if logging is still active) and which are not themselves contained by a deleted DataObject MUST appear in the ChangeSummary.changedObjects list, and passing the DataObject to ChangeSummary.isDeleted MUST return true [COR04020301] DataObjects that were not in the ChangeSummary scope when logging was activated , but are in scope when logging was deactivated (or when ChangeSummary.getChangedObjects() was called, if logging is still active) and which are not themselves contained by a created DataObject MUST be appear in the ChangeSummary.changedObjects list, and passing the DataObject to ChangeSummary.isCreated MUST return true.[COR04020302] DataObjects that remained in the ChangeSummary scope and whose property values changed during the time that logging was activated MUST appear in the ChangeSummary.changedObjects list, and passing the DataObject to ChangeSummary.isModified MUST return true. [COR04020303].  The changeObject MUST NOT contain any objects other than those meeting the defined criteria. [COR04020304]. 

So, in other words, to be deleted, an object has to have been removed from the scope, a created object must be new in the scope.  We are talking here about instances of DataObjects, not about objects that have identical XML serializations.

 

One of the things I've at least tried to clean up is for chapter 10, Change Summary XML Format, to focus on...ummm...the XML format only, not on defining, or redefining the contents of the change summary.  After all, the goal is for the ChangeSummary to have the same values before serialization and after deserialization, for the change summary to survive unaltered the roundtrip, therefore the XML serialization shouldn't go about changing the definition of these values.

 

The only place I expect that the described behavior will be surprising to users is when DataObject.delete() is called.  A "deleted" object that is returned to the graph is also shown as a move...which is surprising, to say the least.  Also, although a deleted object is implicitly detached, it is may still be a referenced orphan.  Then the resulting graph still contains the deleted object, just with all its properties unset.  Ugly.

 

The most direct solution is to simply deprecate the delete method.  Do we really expect application to implement the kind of DataObject pooling that delete seems to imply? 

 

An alternative approach is for objects on which "delete" was called to always be renedered as deleted.  If we ignore the pooling of DataObjects use-case, we could also add a restriction that deleted DataObjects not be re-used.

 

Comments?

 

Ron

---

Von: Barack, Ron [mailto:ron.barack@sap.com]
Gesendet: Montag, 18. Mai 2009 15:41
An: sdo@lists.oasis-open.org
Betreff: [sdo] ISSUE 44: ChangeSummary semantics

Hi Everyone,

I realize that I may getting too far ahead of the group here, but I want to write down my thoughts on this issue:  sooner or later we must address it in the TC.

When an object is moved from one container to another, so long as both are within the scope of the same ChangeSummary, the 2.1 spec allows for 2 possible interpretations, as far as the ChangeSummary is concerned.  The object may simply be moved (that is, the object itself is not changed, but the old container loses a reference to it), or the object may be deleted from the old container, and created in the new container.

I realize this will be a very sensitive issue: as soon as we decide one way or the other, we are breaking someone's backwards compatibility.  The trouble is, this is a pretty large whole to have been left open.  It makes it very hard to write a DAS that is independent of the SDO client, the DAS has to be programmed for different interpretations of the CS, and users that access the CS themselves never know what to expect.

To make things concrete, let's start with a code sample:

        DataObject root = DataFactory.INSTANCE.create("commonj.sdo","DataGraph");
            DataObject company = DataFactory.INSTANCE.create("example","Company");
        DataObject dept1 = company.createDataObject("department");
            DataObject dept2 = company.createDataObject("department");
            DataObject child = dept1.createDataObject("employee");

        root.getChangeSummary().beginLogging();
        dept2.getList("employee").add(child);
        root.getChangeSummary().endLoggin();

If we want to depict this as a "move", the following unit test would look like this:

        assertFalse(root.getChangeSummary().isModified(child));
        assertFalse(root.getChangeSummary().isCreated(child)); 
        assertFalse(root.getChangeSummary().isDeleted(child)); 
        assertTrue(root.getChangeSummary().isModified(dept1));
        assertTrue(root.getChangeSummary().isModified(dept2));

The XML looks like this

<DataGraph>
        <ChangeSummary>
                <Department ref="Company/Department[1]">
                        <Employee ref="Company/Department[2]/Employee[1]/>
                </Department>
                <Department ref="Company/Department[2]" sdox:unset="Employee"/>
        </ChangeSummary>
        <Company>
                        <Department>
                </Department>
                <Department>
                        <Employee/>
                </Department>
        </Company>
</DataGraph>

By contrast, if we we wanted to use create/delete, then the test would be something like this:

        assertFalse(root.getChangeSummary().isModified(child));
        assertTrue(root.getChangeSummary().isCreated(child));   // Is this correct?
        assertTrue(root.getChangeSummary().isDeleted(child));   // Is this correct?
        assertTrue(root.getChangeSummary().isModified(dept1));
        assertTrue(root.getChangeSummary().isModified(dept2));

And the XML would look like:

<DataGraph>
        <ChangeSummary deleted="ChangeSummary/Department[1]/Employee[1]" created="Company/Department[2]/Employee[1]">
                <Department ref="Company/Department[1]">
                        <Employee/>
                </Department>
                <Department ref="Company/Department[2]" sdox:unset="Employee"/>
        </ChangeSummary>
        <Company>
                        <Department>
                </Department>
                <Department>
                        <Employee/>
                </Department>
        </Company>
</DataGraph>

The result coming from deserializing the XML gives a different ChangeSummary than the one we had before serialization.  Namely, the following:

        DataObject root = XMLHelper.INSTANCE.load(…).getRootObject();
        DataObject company = root.getDataObject(0);
        DataObject dept1 = company.getList("department").get(0);
        DataObject dept2 = company.getList("department").get(1);
        DataObject child = dept2.getList("employee").get(0);

        assertFalse(root.getChangeSummary().isModified(child));
        assertTrue(root.getChangeSummary().isCreated(child));  
        assertFalse(root.getChangeSummary().isDeleted(child)); 
        assertTrue(root.getChangeSummary().isModified(dept1));
        assertTrue(root.getChangeSummary().isModified(dept2));

        DataObject deleted = root.getChangeSummary().getChangedObjects()…
        assertFalse(root.getChangeSummary().isCreated(child)); 
        assertTrue(root.getChangeSummary().isDeleted(child));  

I think these two behaviours are too diverent, we should really try to reconcile them.

Our implementation takes the "move" approach.  I would argue that this is the approach that obeys the "priciple of least surprise".  Being able to roundtrip to XML is also a major advantage.

I've heard two arguments for the create/delete approach:

1)  That containment "means" aggregation, and therefore that "moving" things between containers is nonsence.  What looks like a move is actually a delete/create.  The problem with this argument is that there is absolutely nothing else in the spec or the API that supports it.  For instance, if moving an object between two containers is impossible, then we should not allow it.  That is, the statement "dept2.getList("employee").add(child)" in the above code sample should throw an exception.  Actually, I'd be really happy to discuss this approach, and it certainly would clear up problems like "isCreated" and "isDeleted" both returning true.  On the other hand, this is a breaking change.

2)  That we should be able to calculate the change summary based on diff'ing the XML generated "before" and "after" the changes.  If we follow this argument to ist logical consequences, don't we also run into a bunch of other nasty problems.  Isn't it true that, from looking at the XML, you cannot distinguish any modification from a delete followed by a create?   And I never understood the role of XML is the calculation.  That's not to say that the CS cannot be calculated by diff'ing two graphs, but I am saying that the CS cannot necessarily be calculated by diff'ing the XML representation of the graphs.

Looking forward to an intersting discussion on this one!

Best Regards,
Ron