[354963] handleRevisions() does not work correctly for sub branches

https://bugs.eclipse.org/bugs/show_bug.cgi?id=354963

11 files changed, 229 insertions, 111 deletions

diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/revision/CDORevisionHandler.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/revision/CDORevisionHandler.java
index 19efdb499c..e51a317f83 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/revision/CDORevisionHandler.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/revision/CDORevisionHandler.java
@@ -10,6 +10,8 @@
  */
 package org.eclipse.emf.cdo.common.revision;
 
+import org.eclipse.emf.cdo.spi.common.revision.DetachedCDORevision;
+
 /**
  * A call-back interface that indicates the ability to <i>handle</i> {@link CDORevision revisions} that are passed from
  * other entities.
@@ -26,4 +28,50 @@ public interface CDORevisionHandler
    * @since 4.0
    */
   public boolean handleRevision(CDORevision revision);
+
+  /**
+   * @author Eike Stepper
+   * @since 4.1
+   */
+  public static class Filtered implements CDORevisionHandler
+  {
+    private final CDORevisionHandler delegate;
+
+    public Filtered(CDORevisionHandler delegate)
+    {
+      this.delegate = delegate;
+    }
+
+    public final boolean handleRevision(CDORevision revision)
+    {
+      if (filter(revision))
+      {
+        return true;
+      }
+
+      return delegate.handleRevision(revision);
+    }
+
+    protected boolean filter(CDORevision revision)
+    {
+      return false;
+    }
+
+    /**
+     * @author Eike Stepper
+     */
+    public static final class Undetached extends Filtered
+    {
+      public Undetached(CDORevisionHandler delegate)
+      {
+        super(delegate);
+      }
+
+      @Override
+      protected boolean filter(CDORevision revision)
+      {
+        return revision instanceof DetachedCDORevision;
+      }
+    }
+  }
 }
diff --git a/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/DBStore.java b/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/DBStore.java
index fa4e098f26..849e5ce8b8 100644
--- a/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/DBStore.java
+++ b/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/DBStore.java
@@ -430,22 +430,23 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
 
     try
     {
-      accessor.handleRevisions(null, null, CDOBranchPoint.UNSPECIFIED_DATE, true, new CDORevisionHandler()
-      {
-        public boolean handleRevision(CDORevision revision)
-        {
-          CDOBranch branch = revision.getBranch();
-          List<CDORevision> list = result.get(branch);
-          if (list == null)
+      accessor.handleRevisions(null, null, CDOBranchPoint.UNSPECIFIED_DATE, true,
+          new CDORevisionHandler.Filtered.Undetached(new CDORevisionHandler()
           {
-            list = new ArrayList<CDORevision>();
-            result.put(branch, list);
-          }
-
-          list.add(revision);
-          return true;
-        }
-      });
+            public boolean handleRevision(CDORevision revision)
+            {
+              CDOBranch branch = revision.getBranch();
+              List<CDORevision> list = result.get(branch);
+              if (list == null)
+              {
+                list = new ArrayList<CDORevision>();
+                result.put(branch, list);
+              }
+
+              list.add(revision);
+              return true;
+            }
+          }));
     }
     finally
     {
diff --git a/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/mapping/horizontal/HorizontalBranchingClassMapping.java b/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/mapping/horizontal/HorizontalBranchingClassMapping.java
index 7f8a9b0ae8..04225840f7 100644
--- a/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/mapping/horizontal/HorizontalBranchingClassMapping.java
+++ b/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/mapping/horizontal/HorizontalBranchingClassMapping.java
@@ -48,6 +48,7 @@ import org.eclipse.emf.cdo.server.db.mapping.ITypeMapping;
 import org.eclipse.emf.cdo.server.internal.db.CDODBSchema;
 import org.eclipse.emf.cdo.server.internal.db.bundle.OM;
 import org.eclipse.emf.cdo.spi.common.commit.CDOChangeSetSegment;
+import org.eclipse.emf.cdo.spi.common.revision.DetachedCDORevision;
 import org.eclipse.emf.cdo.spi.common.revision.InternalCDORevision;
 import org.eclipse.emf.cdo.spi.common.revision.InternalCDORevisionDelta;
 import org.eclipse.emf.cdo.spi.server.InternalRepository;
@@ -870,14 +871,11 @@ public class HorizontalBranchingClassMapping extends AbstractHorizontalClassMapp
         if (timeStamp != CDOBranchPoint.UNSPECIFIED_DATE)
         {
           builder.append(CDODBSchema.ATTRIBUTES_CREATED);
-          builder.append(">=?"); //$NON-NLS-1$
-          builder.append(" AND ("); //$NON-NLS-1$
+          builder.append("<=? AND ("); //$NON-NLS-1$
           builder.append(CDODBSchema.ATTRIBUTES_REVISED);
-          builder.append("<=? OR "); //$NON-NLS-1$
+          builder.append("=0 OR "); //$NON-NLS-1$
           builder.append(CDODBSchema.ATTRIBUTES_REVISED);
-          builder.append("="); //$NON-NLS-1$
-          builder.append(CDOBranchPoint.UNSPECIFIED_DATE);
-          builder.append(")"); //$NON-NLS-1$
+          builder.append(">=?)"); //$NON-NLS-1$
           timeParameters = 2;
         }
         else
@@ -918,10 +916,10 @@ public class HorizontalBranchingClassMapping extends AbstractHorizontalClassMapp
       {
         CDOID id = idHandler.getCDOID(resultSet, 1);
         int version = resultSet.getInt(2);
-        int branchID = resultSet.getInt(3);
 
         if (version >= CDOBranchVersion.FIRST_VERSION)
         {
+          int branchID = resultSet.getInt(3);
           CDOBranchVersion branchVersion = branchManager.getBranch(branchID).getVersion(Math.abs(version));
           InternalCDORevision revision = (InternalCDORevision)revisionManager.getRevisionByVersion(id, branchVersion,
               CDORevision.UNCHUNKED, true);
@@ -931,6 +929,12 @@ public class HorizontalBranchingClassMapping extends AbstractHorizontalClassMapp
             break;
           }
         }
+        else
+        {
+          // Tell handler about detached IDs
+          InternalCDORevision revision = new DetachedCDORevision(null, id, null, version, 0);
+          handler.handleRevision(revision);
+        }
       }
     }
     catch (SQLException e)
@@ -965,14 +969,12 @@ public class HorizontalBranchingClassMapping extends AbstractHorizontalClassMapp
       builder.append("=? AND "); //$NON-NLS-1$
 
       builder.append(CDODBSchema.ATTRIBUTES_CREATED);
-      builder.append(">=?"); //$NON-NLS-1$
+      builder.append("<=?"); //$NON-NLS-1$
       builder.append(" AND ("); //$NON-NLS-1$
       builder.append(CDODBSchema.ATTRIBUTES_REVISED);
-      builder.append("<=? OR "); //$NON-NLS-1$
+      builder.append("=0 OR "); //$NON-NLS-1$
       builder.append(CDODBSchema.ATTRIBUTES_REVISED);
-      builder.append("="); //$NON-NLS-1$
-      builder.append(CDOBranchPoint.UNSPECIFIED_DATE);
-      builder.append(")"); //$NON-NLS-1$
+      builder.append(">=?)"); //$NON-NLS-1$
     }
 
     IIDHandler idHandler = getMappingStrategy().getStore().getIDHandler();
diff --git a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/HandleRevisionsIndication.java b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/HandleRevisionsIndication.java
index 506317d098..e736ea5a2b 100644
--- a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/HandleRevisionsIndication.java
+++ b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/HandleRevisionsIndication.java
@@ -93,28 +93,29 @@ public class HandleRevisionsIndication extends CDOServerReadIndication
     final IOException[] ioException = { null };
     final RuntimeException[] runtimeException = { null };
 
-    getRepository().handleRevisions(eClass, branch, exactBranch, timeStamp, exactTime, new CDORevisionHandler()
-    {
-      public boolean handleRevision(CDORevision revision)
-      {
-        try
-        {
-          out.writeBoolean(true);
-          out.writeCDORevision(revision, CDORevision.UNCHUNKED);
-          return true;
-        }
-        catch (IOException ex)
+    getRepository().handleRevisions(eClass, branch, exactBranch, timeStamp, exactTime,
+        new CDORevisionHandler.Filtered.Undetached(new CDORevisionHandler()
         {
-          ioException[0] = ex;
-        }
-        catch (RuntimeException ex)
-        {
-          runtimeException[0] = ex;
-        }
-
-        return false;
-      }
-    });
+          public boolean handleRevision(CDORevision revision)
+          {
+            try
+            {
+              out.writeBoolean(true);
+              out.writeCDORevision(revision, CDORevision.UNCHUNKED);
+              return true;
+            }
+            catch (IOException ex)
+            {
+              ioException[0] = ex;
+            }
+            catch (RuntimeException ex)
+            {
+              runtimeException[0] = ex;
+            }
+
+            return false;
+          }
+        }));
 
     if (ioException[0] != null)
     {
diff --git a/plugins/org.eclipse.emf.cdo.server.ocl/src/org/eclipse/emf/cdo/server/ocl/CDOExtentCreator.java b/plugins/org.eclipse.emf.cdo.server.ocl/src/org/eclipse/emf/cdo/server/ocl/CDOExtentCreator.java
index ea47b4db90..19600441e4 100644
--- a/plugins/org.eclipse.emf.cdo.server.ocl/src/org/eclipse/emf/cdo/server/ocl/CDOExtentCreator.java
+++ b/plugins/org.eclipse.emf.cdo.server.ocl/src/org/eclipse/emf/cdo/server/ocl/CDOExtentCreator.java
@@ -106,28 +106,29 @@ public class CDOExtentCreator implements OCLExtentCreator
       }
     }
 
-    accessor.handleRevisions(eClass, branch, timeStamp, false, new CDORevisionHandler()
-    {
-      public boolean handleRevision(CDORevision revision)
-      {
-        if (revisionCacheAdder != null)
-        {
-          revisionCacheAdder.addRevision(revision);
-        }
-
-        CDOID id = revision.getID();
-        if (!isDetached(id))
+    accessor.handleRevisions(eClass, branch, timeStamp, false, new CDORevisionHandler.Filtered.Undetached(
+        new CDORevisionHandler()
         {
-          EObject object = getEObject(id);
-          if (object != null)
+          public boolean handleRevision(CDORevision revision)
           {
-            extent.add(object);
-          }
-        }
+            if (revisionCacheAdder != null)
+            {
+              revisionCacheAdder.addRevision(revision);
+            }
 
-        return !canceled.get();
-      }
-    });
+            CDOID id = revision.getID();
+            if (!isDetached(id))
+            {
+              EObject object = getEObject(id);
+              if (object != null)
+              {
+                extent.add(object);
+              }
+            }
+
+            return !canceled.get();
+          }
+        }));
 
     return extent;
   }
@@ -248,28 +249,29 @@ public class CDOExtentCreator implements OCLExtentCreator
 
             private void handlePersistentState()
             {
-              accessor.handleRevisions(eClass, branch, timeStamp, false, new CDORevisionHandler()
-              {
-                public boolean handleRevision(CDORevision revision)
-                {
-                  empty = false;
-                  emptyKnown.countDown();
-
-                  CDORevisionCacheAdder revisionCacheAdder = getRevisionCacheAdder();
-                  if (revisionCacheAdder != null)
-                  {
-                    revisionCacheAdder.addRevision(revision);
-                  }
-
-                  CDOID id = revision.getID();
-                  if (!isDetached(id))
+              accessor.handleRevisions(eClass, branch, timeStamp, false, new CDORevisionHandler.Filtered.Undetached(
+                  new CDORevisionHandler()
                   {
-                    enqueue(id);
-                  }
-
-                  return !canceled.get();
-                }
-              });
+                    public boolean handleRevision(CDORevision revision)
+                    {
+                      empty = false;
+                      emptyKnown.countDown();
+
+                      CDORevisionCacheAdder revisionCacheAdder = getRevisionCacheAdder();
+                      if (revisionCacheAdder != null)
+                      {
+                        revisionCacheAdder.addRevision(revision);
+                      }
+
+                      CDOID id = revision.getID();
+                      if (!isDetached(id))
+                      {
+                        enqueue(id);
+                      }
+
+                      return !canceled.get();
+                    }
+                  }));
             }
 
             private void enqueue(CDOID id)
diff --git a/plugins/org.eclipse.emf.cdo.server.product/config/cdo-server.xml b/plugins/org.eclipse.emf.cdo.server.product/config/cdo-server.xml
index ad333c69ed..626445cdc5 100644
--- a/plugins/org.eclipse.emf.cdo.server.product/config/cdo-server.xml
+++ b/plugins/org.eclipse.emf.cdo.server.product/config/cdo-server.xml
@@ -20,7 +20,7 @@
     <property name="supportingEcore" value="false"/>
     <property name="ensureReferentialIntegrity" value="false"/>
     <property name="allowInterruptRunningQueries" value="true"/>
-    <property name="idGenerationLocation" value="STORE"/> <!-- Possible values: STORE | CLIENT -->
+    <property name="idGenerationLocation" value="CLIENT"/> <!-- Possible values: STORE | CLIENT -->
 
     <!-- Example (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=302775):
     <userManager type="file" description="_database/repo1.users"/>
diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/Repository.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/Repository.java
index f207568e10..79d2f213a0 100644
--- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/Repository.java
+++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/Repository.java
@@ -1286,7 +1286,7 @@ public class Repository extends Container<Object> implements InternalRepository
       final CDORevisionHandler handler)
   {
     CDORevisionHandler wrapper = handler;
-    if (!exactBranch)
+    if (!exactBranch && !branch.isMainBranch())
     {
       if (exactTime && timeStamp == CDOBranchPoint.UNSPECIFIED_DATE)
       {
@@ -1299,7 +1299,8 @@ public class Repository extends Container<Object> implements InternalRepository
 
         public boolean handleRevision(CDORevision revision)
         {
-          if (handled.add(revision.getID()))
+          CDOID id = revision.getID();
+          if (handled.add(id))
           {
             return handler.handleRevision(revision);
           }
diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/mem/MEMStore.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/mem/MEMStore.java
index c5fb283593..3793ac14aa 100644
--- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/mem/MEMStore.java
+++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/mem/MEMStore.java
@@ -345,11 +345,6 @@ public class MEMStore extends LongIDStore implements IMEMStore, BranchLoader, Du
   private boolean handleRevision(InternalCDORevision revision, EClass eClass, CDOBranch branch, long timeStamp,
       boolean exactTime, CDORevisionHandler handler)
   {
-    if (revision instanceof DetachedCDORevision)
-    {
-      return true;
-    }
-
     if (eClass != null && revision.getEClass() != eClass)
     {
       return true;
diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/CDOServerExporter.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/CDOServerExporter.java
index 54992562f9..cc9a2adedb 100644
--- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/CDOServerExporter.java
+++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/CDOServerExporter.java
@@ -177,21 +177,22 @@ public abstract class CDOServerExporter<OUT>
 
   protected void exportRevisions(final OUT out, CDOBranch branch) throws Exception
   {
-    repository.handleRevisions(null, branch, true, CDOBranchPoint.INVALID_DATE, false, new CDORevisionHandler()
-    {
-      public boolean handleRevision(CDORevision revision)
-      {
-        try
-        {
-          exportRevision(out, revision);
-          return true;
-        }
-        catch (Exception ex)
+    repository.handleRevisions(null, branch, true, CDOBranchPoint.INVALID_DATE, false,
+        new CDORevisionHandler.Filtered.Undetached(new CDORevisionHandler()
         {
-          throw WrappedException.wrap(ex);
-        }
-      }
-    });
+          public boolean handleRevision(CDORevision revision)
+          {
+            try
+            {
+              exportRevision(out, revision);
+              return true;
+            }
+            catch (Exception ex)
+            {
+              throw WrappedException.wrap(ex);
+            }
+          }
+        }));
   }
 
   protected abstract void exportRevision(OUT out, CDORevision revision) throws Exception;
diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/spi/server/StoreAccessorBase.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/spi/server/StoreAccessorBase.java
index 644f6c03de..7be7a2a3a8 100644
--- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/spi/server/StoreAccessorBase.java
+++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/spi/server/StoreAccessorBase.java
@@ -311,7 +311,7 @@ public abstract class StoreAccessorBase extends Lifecycle implements IStoreAcces
 
     public CDOCommitData getCommitData()
     {
-      storeAccessor.handleRevisions(null, null, timeStamp, true, this);
+      storeAccessor.handleRevisions(null, null, timeStamp, true, new CDORevisionHandler.Filtered.Undetached(this));
 
       List<CDOIDAndVersion> detachedObjects = detachCounter.getDetachedObjects();
       return new CDOCommitDataImpl(newPackageUnits, newObjects, changedObjects, detachedObjects);
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/BranchingTest.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/BranchingTest.java
index abcdb180f2..04f549a076 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/BranchingTest.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/BranchingTest.java
@@ -18,6 +18,7 @@ import org.eclipse.emf.cdo.common.branch.CDOBranchPoint;
 import org.eclipse.emf.cdo.common.commit.CDOCommitInfo;
 import org.eclipse.emf.cdo.common.id.CDOID;
 import org.eclipse.emf.cdo.common.revision.CDORevision;
+import org.eclipse.emf.cdo.common.revision.CDORevisionHandler;
 import org.eclipse.emf.cdo.common.revision.CDORevisionUtil;
 import org.eclipse.emf.cdo.eresource.CDOResource;
 import org.eclipse.emf.cdo.internal.common.revision.AbstractCDORevisionCache;
@@ -41,6 +42,7 @@ import org.eclipse.net4j.util.event.IListener;
 import org.eclipse.emf.spi.cdo.InternalCDOSession;
 
 import java.lang.reflect.Field;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
@@ -857,6 +859,71 @@ public class BranchingTest extends AbstractCDOTest
     session.close();
   }
 
+  @CleanRepositoriesBefore
+  public void testhandleRevisionsAfterDetachInSubBranch() throws Exception
+  {
+    CDOSession session = openSession1();
+    CDOBranchManager branchManager = session.getBranchManager();
+
+    // Commit to main branch
+    CDOBranch mainBranch = branchManager.getMainBranch();
+    CDOTransaction transaction = session.openTransaction(mainBranch);
+    assertEquals(mainBranch, transaction.getBranch());
+    assertEquals(CDOBranchPoint.UNSPECIFIED_DATE, transaction.getTimeStamp());
+
+    Product1 product = getModel1Factory().createProduct1();
+    product.setName("CDO");
+
+    CDOResource resource = transaction.createResource(getResourcePath("/res"));
+    resource.getContents().add(product);
+
+    CDOCommitInfo commitInfo = transaction.commit();
+    assertEquals(mainBranch, commitInfo.getBranch());
+    long commitTime1 = commitInfo.getTimeStamp();
+    transaction.close();
+
+    // Commit to sub branch
+    CDOBranch subBranch = mainBranch.createBranch("subBranch", commitTime1);
+    transaction = session.openTransaction(subBranch);
+    assertEquals(subBranch, transaction.getBranch());
+    assertEquals(CDOBranchPoint.UNSPECIFIED_DATE, transaction.getTimeStamp());
+
+    resource = transaction.getResource(getResourcePath("/res"));
+    product = (Product1)resource.getContents().get(0);
+    assertEquals("CDO", product.getName());
+
+    product.setName("handleRevisions");
+    commitInfo = transaction.commit();
+    assertEquals(subBranch, commitInfo.getBranch());
+
+    resource.getContents().remove(0);
+    commitInfo = transaction.commit();
+    assertEquals(subBranch, commitInfo.getBranch());
+
+    transaction.close();
+    closeSession1();
+
+    session = openSession2();
+    branchManager = session.getBranchManager();
+    mainBranch = branchManager.getMainBranch();
+    subBranch = mainBranch.getBranch("subBranch");
+
+    final List<CDORevision> revisions = new ArrayList<CDORevision>();
+
+    ((InternalCDOSession)session).getSessionProtocol().handleRevisions(null, subBranch, false,
+        CDOBranchPoint.UNSPECIFIED_DATE, false, new CDORevisionHandler()
+        {
+          public boolean handleRevision(CDORevision revision)
+          {
+            assertNotSame("Product1", revision.getEClass().getName());
+            revisions.add(revision);
+            return true;
+          }
+        });
+
+    assertEquals(3, revisions.size());
+  }
+
   public void testSwitchViewTarget() throws CommitException
   {
     CDOSession session = openSession1();