Home » Modeling » EMF » [CDO] Merging and nested branches
| [CDO] Merging and nested branches [message #1731613] |
Sun, 08 May 2016 01:37  |
 András Péteri
Messages: 22
Registered: January 2010 |
Junior Member |
|
|
Greetings,
We encountered an interesting edge case when trying to merge changes from one branch to another, similar to git's rebase operation (red dots represent commits, white dots are branch creation points):
@SuppressWarnings("unused")
public void testRebaseWithRemoval() throws Exception
{
CDOSession session = openSession();
CDOBranch mainBranch = session.getBranchManager().getMainBranch();
CDOTransaction mainTransaction = session.openTransaction(mainBranch);
// Create company0 on MAIN
CDOResource mainResource = mainTransaction.createResource(getResourcePath("/res"));
EList<EObject> mainContents = mainResource.getContents();
Company company0 = addCompany(mainContents);
long time1 = mainTransaction.commit().getTimeStamp();
CDOBranch source1 = mainBranch.createBranch("source1", time1);
// Create company1 and company2 on MAIN/source1
CDOTransaction source1Transaction = session.openTransaction(source1);
CDOResource source1Resource = source1Transaction.getResource(getResourcePath("/res"));
EList<EObject> source1Contents = source1Resource.getContents();
Company company1 = addCompany(source1Contents);
Company company2 = addCompany(source1Contents);
long time2 = commitAndSync(source1Transaction, mainTransaction).getTimeStamp();
CDOBranch source2 = source1.createBranch("source2", time2);
assertEquals(3, source1Resource.getContents().size());
// Update company0 and company1 on MAIN/source1/source2
CDOTransaction source2Transaction = session.openTransaction(source2);
CDOResource source2Resource = source2Transaction.getResource(getResourcePath("/res"));
EList<EObject> source2Contents = source2Resource.getContents();
((Company)source2Contents.get(0)).setName("C0");
((Company)source2Contents.get(1)).setName("C1");
long time3 = commitAndSync(source2Transaction, source1Transaction, mainTransaction).getTimeStamp();
source2Transaction.close();
// Delete company1 on MAIN/source1
source1Contents.remove(source1Transaction.getObject(company1));
long time4 = commitAndSync(source1Transaction, mainTransaction).getTimeStamp();
assertEquals(2, source1Contents.size());
source1Transaction.close();
// Create company3 on MAIN
Company company3 = addCompany(mainContents);
long time5 = mainTransaction.commit().getTimeStamp();
CDOBranch rebasedSource1 = mainBranch.createBranch("rebasedSource1", time5);
mainTransaction.close();
// Rebase source1
CDOTransaction rebasedSource1Transaction = session.openTransaction(rebasedSource1);
CDOResource rebasedSource1Resource = rebasedSource1Transaction.getResource(getResourcePath("/res"));
rebasedSource1Transaction.merge(source1.getPoint(time4) //
, source1.getBase() //
, new DefaultCDOMerger.PerFeature.ManyValued());
long time6 = rebasedSource1Transaction.commit().getTimeStamp();
CDOBranch rebasedSource2 = rebasedSource1.createBranch("rebasedSource2", time6);
assertEquals(3, rebasedSource1Resource.getContents().size());
rebasedSource1Transaction.close();
// Rebase source2 on already rebased source1
CDOTransaction rebasedSource2Transaction = session.openTransaction(rebasedSource2);
CDOResource rebasedSource2Resource = rebasedSource2Transaction.getResource(getResourcePath("/res"));
rebasedSource2Transaction.merge(source2.getPoint(time3) //
, source2.getBase() //
, new DefaultCDOMerger.PerFeature.ManyValued());
rebasedSource2Transaction.commit();
assertEquals(3, rebasedSource2Resource.getContents().size());
assertEquals("C0", ((Company)rebasedSource2Resource.getContents().get(0)).getName());
rebasedSource2Transaction.close();
}
This test throws ObjectNotFoundException when trying to commit merged changes on rebasedSource2 at the end. CDOTransactionImpl.applyChangedObjects(...) assumes that any CDORevisionDelta that appears in the change set can be acted upon, and tries to load company1, but it does not exist on rebasedSource1.
Another problem occurs when company1 is updated instead of getting removed:
@SuppressWarnings("unused")
public void testRebaseWithUpdate() throws Exception
{
CDOSession session = openSession();
CDOBranch mainBranch = session.getBranchManager().getMainBranch();
CDOTransaction mainTransaction = session.openTransaction(mainBranch);
// Create company0 on MAIN
CDOResource mainResource = mainTransaction.createResource(getResourcePath("/res"));
EList<EObject> mainContents = mainResource.getContents();
Company company0 = addCompany(mainContents);
long time1 = mainTransaction.commit().getTimeStamp();
CDOBranch source1 = mainBranch.createBranch("source1", time1);
// Create company1 and company2 on MAIN/source1
CDOTransaction source1Transaction = session.openTransaction(source1);
CDOResource source1Resource = source1Transaction.getResource(getResourcePath("/res"));
EList<EObject> source1Contents = source1Resource.getContents();
Company company1 = addCompany(source1Contents);
Company company2 = addCompany(source1Contents);
long time2 = commitAndSync(source1Transaction, mainTransaction).getTimeStamp();
CDOBranch source2 = source1.createBranch("source2", time2);
// Update company0 and company1 on MAIN/source1/source2
CDOTransaction source2Transaction = session.openTransaction(source2);
CDOResource source2Resource = source2Transaction.getResource(getResourcePath("/res"));
EList<EObject> source2Contents = source2Resource.getContents();
source2Transaction.getObject(company0).setName("C0");
source2Transaction.getObject(company1).setName("C1");
long time3 = commitAndSync(source2Transaction, source1Transaction, mainTransaction).getTimeStamp();
assertEquals("C0", source2Transaction.getObject(company0).getName());
assertEquals("C1", source2Transaction.getObject(company1).getName());
source2Transaction.close();
// Update company1 on MAIN/source1
source1Transaction.getObject(company1).setName("X");
long time4 = commitAndSync(source1Transaction, mainTransaction).getTimeStamp();
assertEquals("X", source1Transaction.getObject(company1).getName());
source1Transaction.close();
// Create company3 on MAIN
Company company3 = addCompany(mainContents);
long time5 = mainTransaction.commit().getTimeStamp();
CDOBranch rebasedSource1 = mainBranch.createBranch("rebasedSource1", time5);
mainTransaction.close();
// Rebase source1
CDOTransaction rebasedSource1Transaction = session.openTransaction(rebasedSource1);
CDOResource rebasedSource1Resource = rebasedSource1Transaction.getResource(getResourcePath("/res"));
rebasedSource1Transaction.merge(source1.getPoint(time4) //
, source1.getBase() //
, new DefaultCDOMerger.PerFeature.ManyValued());
long time6 = rebasedSource1Transaction.commit().getTimeStamp();
CDOBranch rebasedSource2 = rebasedSource1.createBranch("rebasedSource2", time6);
assertEquals("X", rebasedSource1Transaction.getObject(company1).getName());
rebasedSource1Transaction.close();
// Rebase source2 on already rebased source1
CDOTransaction rebasedSource2Transaction = session.openTransaction(rebasedSource2);
CDOResource rebasedSource2Resource = rebasedSource2Transaction.getResource(getResourcePath("/res"));
rebasedSource2Transaction.merge(source2.getPoint(time3) //
, source2.getBase() //
, new DefaultCDOMerger.PerFeature.ManyValued());
rebasedSource2Transaction.commit();
assertEquals("C0", rebasedSource2Transaction.getObject(company0).getName());
assertEquals("C1", rebasedSource2Transaction.getObject(company1).getName());
rebasedSource2Transaction.close();
}
In this test, the last assertion fails; company1.getName() returns "X" instead of "C1".
If the branch base point of source2 is specified, the comparison for company1 ends up being a "CDORevision vs. CDORevisionDelta" case in DefaultCDOMerger, which the class does not handle currently, and the delta does not appear in the resulting change set.
If I leave out the base point parameter, the merge will arrive at a "CDORevision vs. CDORevision" comparison. I also have to override the default implementation of DefaultCDOMerger.addedInSourceAndTarget(...) to return the source revision in this case, but since rebasedSource1 already contains company1 (getObjectIfExists(id) returns a non-null value), CDOTransaction.applyNewChanges(...) will skip the new revision altogether, and the change set from source2 ends up getting partially applied this way as well.
For the first scenario, we thought that the following change to CDOTransactionImpl.applyChangedObjects(...) would do the trick:
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
index c80de5c..1056844 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
@@ -753,9 +753,14 @@
InternalCDORevisionDelta ancestorGoalDelta = (InternalCDORevisionDelta)key;
ancestorGoalDelta.setTarget(null);
CDOID id = ancestorGoalDelta.getID();
- InternalCDORevision ancestorRevision = (InternalCDORevision)ancestorProvider.getRevision(id);
- InternalCDOObject object = getObject(id);
+ InternalCDOObject object = getObjectIfExists(id);
+ if (object == null)
+ {
+ continue;
+ }
+
+ InternalCDORevision ancestorRevision = (InternalCDORevision)ancestorProvider.getRevision(id);
boolean revisionChanged = false;
InternalCDORevision targetRevision = object.cdoRevision();
...to keep the theme with the other two methods for applying changes (don't apply an addition if the object already exists and don't attempt to delete it if it doesn't).
For the second scenario, my colleague's suggestion was to introduce another "else if" branch for comparing CDORevisions against CDORevisionDeltas, and use the revision on the target if it did not exist at the ancestor point:
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
index c80de5c..a09c824 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
@@ -768,7 +773,7 @@
oldRevisions.put(id, targetRevision);
- InternalCDORevision goalRevision = ancestorRevision.copy();
+ InternalCDORevision goalRevision = ancestorRevision != null ? ancestorRevision.copy() : targetRevision.copy();
goalRevision.setBranchPoint(this);
if (!keepVersions)
{
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/DefaultCDOMerger.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/DefaultCDOMerger.java
index 651c91d..5036bda 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/DefaultCDOMerger.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/DefaultCDOMerger.java
@@ -195,6 +195,10 @@
{
data = changedInTargetAndDetachedInSource((CDORevisionDelta)targetData);
}
+ else if (targetData instanceof CDORevision && sourceData instanceof CDORevisionDelta)
+ {
+ data = addedInTargetAndChangedInSource((CDORevision)targetData, (CDORevisionDelta)sourceData);
+ }
return take(data);
}
@@ -214,6 +218,11 @@
return targetRevision;
}
+ protected Object addedInTargetAndChangedInSource(CDORevision targetRevision, CDORevisionDelta sourceDelta)
+ {
+ return sourceDelta;
+ }
+
protected Object changedInTarget(CDORevisionDelta delta)
{
return delta;
Alternatively, CDOTransactionImpl.applyNewObjects(...) could be modified to apply the difference between the target revision and the new revision in the change set as a delta, if the object already exists on the branch. In this case, DefaultCDOMerger would not need to be changed.
As this post is probably getting way too long, I'll put my questions below:
- Does CDO support the scenarios depicted above?
- Are we on the right track with suggested fixes? (They do not break the H2 branching test suite, including the two test cases above.)
Thank you in advance!
--
András
|
|
| |
Goto Forum:
Current Time: Thu Apr 25 08:33:38 GMT 2024
Powered by FUDForum. Page generated in 0.03050 seconds |
] 
Search
Help
Register
Login
Home
