[517168] CDOView.queryXRef returns invalid values when an object is removed on a newly created CDO branch

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

2 files changed, 261 insertions, 44 deletions

diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/XRefsQueryHandler.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/XRefsQueryHandler.java
index fff86f0e83..459ef34391 100644
--- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/XRefsQueryHandler.java
+++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/XRefsQueryHandler.java
@@ -23,6 +23,7 @@ import org.eclipse.emf.cdo.common.model.CDOPackageRegistry;
 import org.eclipse.emf.cdo.common.model.CDOPackageUnit;
 import org.eclipse.emf.cdo.common.model.CDOPackageUnit.State;
 import org.eclipse.emf.cdo.common.protocol.CDOProtocolConstants;
+import org.eclipse.emf.cdo.common.revision.CDORevision;
 import org.eclipse.emf.cdo.common.util.CDOQueryInfo;
 import org.eclipse.emf.cdo.server.IQueryContext;
 import org.eclipse.emf.cdo.server.IQueryHandler;
@@ -32,7 +33,11 @@ import org.eclipse.emf.cdo.server.IStoreAccessor;
 import org.eclipse.emf.cdo.server.IView;
 import org.eclipse.emf.cdo.server.StoreThreadLocal;
 import org.eclipse.emf.cdo.server.StoreThreadLocal.NoSessionRegisteredException;
+import org.eclipse.emf.cdo.spi.common.branch.CDOBranchUtil;
 import org.eclipse.emf.cdo.spi.common.model.InternalCDOPackageRegistry;
+import org.eclipse.emf.cdo.spi.common.revision.DetachedCDORevision;
+import org.eclipse.emf.cdo.spi.common.revision.InternalCDORevisionManager;
+import org.eclipse.emf.cdo.spi.common.revision.SyntheticCDORevision;
 import org.eclipse.emf.cdo.spi.server.InternalRepository;
 import org.eclipse.emf.cdo.spi.server.QueryHandlerFactory;
 
@@ -47,8 +52,10 @@ import org.eclipse.emf.ecore.EcorePackage;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.StringTokenizer;
 
 /**
@@ -65,20 +72,29 @@ public class XRefsQueryHandler implements IQueryHandler
     try
     {
       IStoreAccessor accessor = StoreThreadLocal.getAccessor();
-      QueryContext xrefsContext = new QueryContext(info, context);
-      accessor.queryXRefs(xrefsContext);
 
       CDOBranchPoint branchPoint = context;
       CDOBranch branch = branchPoint.getBranch();
-      int maxResults = info.getMaxResults();
 
-      while (!branch.isMainBranch() && (maxResults == CDOQueryInfo.UNLIMITED_RESULTS || context.getResultCount() < maxResults))
+      if (branch.isMainBranch())
       {
-        branchPoint = branch.getBase();
-        branch = branchPoint.getBranch();
-
-        xrefsContext.setBranchPoint(branchPoint);
+        QueryContext xrefsContext = new QueryContext(info, context);
+        accessor.queryXRefs(xrefsContext);
+      }
+      else
+      {
+        QueryContext xrefsContext = new QueryContextBranching(info, context);
         accessor.queryXRefs(xrefsContext);
+
+        int maxResults = info.getMaxResults();
+        while (!branch.isMainBranch() && (maxResults == CDOQueryInfo.UNLIMITED_RESULTS || context.getResultCount() < maxResults))
+        {
+          branchPoint = branch.getBase();
+          branch = branchPoint.getBranch();
+
+          xrefsContext.setBranchPoint(branchPoint);
+          accessor.queryXRefs(xrefsContext);
+        }
       }
     }
     catch (NoSessionRegisteredException ex)
@@ -208,7 +224,7 @@ public class XRefsQueryHandler implements IQueryHandler
    * @author Eike Stepper
    * @since 3.0
    */
-  private static final class QueryContext implements IStoreAccessor.QueryXRefsContext
+  private static class QueryContext implements IStoreAccessor.QueryXRefsContext
   {
     private CDOQueryInfo info;
 
@@ -229,26 +245,26 @@ public class XRefsQueryHandler implements IQueryHandler
       branchPoint = context;
     }
 
-    public void setBranchPoint(CDOBranchPoint branchPoint)
+    public final void setBranchPoint(CDOBranchPoint branchPoint)
     {
       this.branchPoint = branchPoint;
     }
 
-    public CDOBranch getBranch()
+    public final CDOBranch getBranch()
     {
       return branchPoint.getBranch();
     }
 
-    public long getTimeStamp()
+    public final long getTimeStamp()
     {
       return branchPoint.getTimeStamp();
     }
 
-    public Map<CDOID, EClass> getTargetObjects()
+    public final Map<CDOID, EClass> getTargetObjects()
     {
       if (targetObjects == null)
       {
-        IRepository repository = context.getView().getRepository();
+        IRepository repository = getRepository();
         IStore store = repository.getStore();
         CDOPackageRegistry packageRegistry = repository.getPackageRegistry();
 
@@ -287,7 +303,7 @@ public class XRefsQueryHandler implements IQueryHandler
       return targetObjects;
     }
 
-    public EReference[] getSourceReferences()
+    public final EReference[] getSourceReferences()
     {
       if (sourceReferences == null)
       {
@@ -297,10 +313,66 @@ public class XRefsQueryHandler implements IQueryHandler
       return sourceReferences;
     }
 
+    public final Map<EClass, List<EReference>> getSourceCandidates()
+    {
+      if (sourceCandidates == null)
+      {
+        sourceCandidates = new HashMap<EClass, List<EReference>>();
+        Collection<EClass> concreteTypes = getTargetObjects().values();
+        EReference[] sourceReferences = getSourceReferences();
+
+        if (sourceReferences.length != 0)
+        {
+          InternalRepository repository = (InternalRepository)getRepository();
+          InternalCDOPackageRegistry packageRegistry = repository.getPackageRegistry(false);
+          for (EReference eReference : sourceReferences)
+          {
+            collectSourceCandidates(eReference, concreteTypes, sourceCandidates, packageRegistry);
+          }
+        }
+        else
+        {
+          collectSourceCandidates(context.getView(), concreteTypes, sourceCandidates);
+        }
+      }
+
+      return sourceCandidates;
+    }
+
+    public final int getMaxResults()
+    {
+      return info.getMaxResults();
+    }
+
+    public final IRepository getRepository()
+    {
+      return context.getView().getRepository();
+    }
+
+    public final boolean addXRef(CDOID targetID, CDOID sourceID, EReference sourceReference, int sourceIndex)
+    {
+      if (CDOIDUtil.isNull(targetID))
+      {
+        return true;
+      }
+
+      if (isIgnoredObject(sourceID))
+      {
+        return true;
+      }
+
+      return context.addResult(new CDOIDReference(targetID, sourceID, sourceReference, sourceIndex));
+    }
+
+    protected boolean isIgnoredObject(CDOID sourceID)
+    {
+      return false;
+    }
+
     private EReference[] parseSourceReferences()
     {
       List<EReference> result = new ArrayList<EReference>();
-      CDOPackageRegistry packageRegistry = context.getView().getRepository().getPackageRegistry();
+      CDOPackageRegistry packageRegistry = getRepository().getPackageRegistry();
 
       String params = (String)info.getParameters().get(CDOProtocolConstants.QUERY_LANGUAGE_XREFS_SOURCE_REFERENCES);
       if (params == null)
@@ -322,46 +394,53 @@ public class XRefsQueryHandler implements IQueryHandler
 
       return result.toArray(new EReference[result.size()]);
     }
+  }
 
-    public Map<EClass, List<EReference>> getSourceCandidates()
-    {
-      if (sourceCandidates == null)
-      {
-        sourceCandidates = new HashMap<EClass, List<EReference>>();
-        Collection<EClass> concreteTypes = getTargetObjects().values();
-        EReference[] sourceReferences = getSourceReferences();
+  /**
+   * @author Eike Stepper
+   */
+  private static final class QueryContextBranching extends QueryContext
+  {
+    private final CDOBranchPoint originalBranchPoint;
 
-        if (sourceReferences.length != 0)
-        {
-          InternalRepository repository = (InternalRepository)context.getView().getRepository();
-          InternalCDOPackageRegistry packageRegistry = repository.getPackageRegistry(false);
-          for (EReference eReference : sourceReferences)
-          {
-            collectSourceCandidates(eReference, concreteTypes, sourceCandidates, packageRegistry);
-          }
-        }
-        else
-        {
-          collectSourceCandidates(context.getView(), concreteTypes, sourceCandidates);
-        }
-      }
+    private final Set<CDOID> ignoredObjects = new HashSet<CDOID>();
 
-      return sourceCandidates;
+    public QueryContextBranching(CDOQueryInfo info, IQueryContext context)
+    {
+      super(info, context);
+      originalBranchPoint = CDOBranchUtil.copyBranchPoint(context);
     }
 
-    public int getMaxResults()
+    @Override
+    protected boolean isIgnoredObject(CDOID sourceID)
     {
-      return info.getMaxResults();
+      if (!ignoredObjects.add(sourceID))
+      {
+        return true;
+      }
+
+      if (isDetachedObject(sourceID))
+      {
+        ignoredObjects.add(sourceID);
+        return true;
+      }
+
+      return false;
     }
 
-    public boolean addXRef(CDOID targetID, CDOID sourceID, EReference sourceReference, int sourceIndex)
+    private boolean isDetachedObject(CDOID sourceID)
     {
-      if (CDOIDUtil.isNull(targetID))
+      if (getBranch() == originalBranchPoint.getBranch())
       {
-        return true;
+        return false;
       }
 
-      return context.addResult(new CDOIDReference(targetID, sourceID, sourceReference, sourceIndex));
+      SyntheticCDORevision[] synthetics = { null };
+
+      InternalCDORevisionManager revisionManager = (InternalCDORevisionManager)getRepository().getRevisionManager();
+      revisionManager.getRevision(sourceID, originalBranchPoint, CDORevision.UNCHUNKED, CDORevision.DEPTH_NONE, true, synthetics);
+
+      return synthetics[0] instanceof DetachedCDORevision;
     }
   }
 
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_517168_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_517168_Test.java
new file mode 100644
index 0000000000..1621ee2fae
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_517168_Test.java
@@ -0,0 +1,138 @@
+/*
+ * Copyright (c) 2016 Eike Stepper (Berlin, Germany) and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    Eike Stepper - initial API and implementation
+ */
+package org.eclipse.emf.cdo.tests.bugzilla;
+
+import org.eclipse.emf.cdo.CDOObjectReference;
+import org.eclipse.emf.cdo.common.branch.CDOBranch;
+import org.eclipse.emf.cdo.eresource.CDOResource;
+import org.eclipse.emf.cdo.session.CDOSession;
+import org.eclipse.emf.cdo.tests.AbstractCDOTest;
+import org.eclipse.emf.cdo.tests.config.IRepositoryConfig;
+import org.eclipse.emf.cdo.tests.config.impl.ConfigTest.Requires;
+import org.eclipse.emf.cdo.tests.model1.OrderDetail;
+import org.eclipse.emf.cdo.tests.model1.Product1;
+import org.eclipse.emf.cdo.tests.model1.PurchaseOrder;
+import org.eclipse.emf.cdo.tests.model1.SalesOrder;
+import org.eclipse.emf.cdo.transaction.CDOTransaction;
+import org.eclipse.emf.cdo.util.CDOUtil;
+
+import org.eclipse.emf.ecore.util.EcoreUtil;
+
+import java.util.List;
+
+/**
+ * Bug 517168: Multiple revision instances are loaded during branch switchCDOView.queryXRef
+ * returns invalid values when an object is removed on a newly created CDO branch.
+ *
+ * @author Eike Stepper
+ */
+@Requires(IRepositoryConfig.CAPABILITY_BRANCHING)
+public class Bugzilla_517168_Test extends AbstractCDOTest
+{
+  public void testDeletedXRef_SingleValue() throws Exception
+  {
+    CDOSession session = openSession();
+    CDOTransaction transaction = session.openTransaction();
+
+    // Initialize main branch with two objects, an order detail with a given product.
+    Product1 product = getModel1Factory().createProduct1();
+    OrderDetail orderDetail = getModel1Factory().createOrderDetail();
+    orderDetail.setProduct(product);
+
+    CDOResource resource = transaction.createResource(getResourcePath("res"));
+    resource.getContents().add(product);
+    resource.getContents().add(orderDetail);
+    transaction.commit();
+
+    // Create a branch and open a transaction on it.
+    CDOBranch derivedBranch = transaction.getBranch().createBranch("Derived");
+    transaction = session.openTransaction(derivedBranch);
+    orderDetail = transaction.getObject(orderDetail);
+    product = transaction.getObject(product);
+
+    // Remove the order detail and commit the removal.
+    EcoreUtil.delete(orderDetail);
+    transaction.commit();
+
+    // Check XRef API on derived branch after object removal:
+    // An empty result is supposed to be retrieved at this point.
+    List<CDOObjectReference> xRefs = transaction.queryXRefs(CDOUtil.getCDOObject(product));
+    assertEquals(0, xRefs.size());
+  }
+
+  public void testDeletedXRef_ManyValue() throws Exception
+  {
+    CDOSession session = openSession();
+    CDOTransaction transaction = session.openTransaction();
+
+    // Initialize main branch with two objects, an order detail with a given product.
+    PurchaseOrder purchaseOrder = getModel1Factory().createPurchaseOrder();
+    SalesOrder salesOrder = getModel1Factory().createSalesOrder();
+    salesOrder.getPurchaseOrders().add(purchaseOrder);
+
+    CDOResource resource = transaction.createResource(getResourcePath("res"));
+    resource.getContents().add(purchaseOrder);
+    resource.getContents().add(salesOrder);
+    transaction.commit();
+
+    // Create a branch and open a transaction on it.
+    CDOBranch derivedBranch = transaction.getBranch().createBranch("Derived");
+    transaction = session.openTransaction(derivedBranch);
+    salesOrder = transaction.getObject(salesOrder);
+    purchaseOrder = transaction.getObject(purchaseOrder);
+
+    // Check XRef API on derived branch:
+    // The customer is supposed to be retrieved at this point.
+    List<CDOObjectReference> xRefs = transaction.queryXRefs(CDOUtil.getCDOObject(purchaseOrder));
+    assertEquals(1, xRefs.size());
+    assertEquals(salesOrder, xRefs.get(0).getSourceObject());
+
+    // Remove the order detail and commit the removal.
+    EcoreUtil.delete(salesOrder);
+    transaction.commit();
+
+    // Check XRef API on derived branch after object removal:
+    // An empty result is supposed to be retrieved at this point.
+    xRefs = transaction.queryXRefs(CDOUtil.getCDOObject(purchaseOrder));
+    assertEquals(0, xRefs.size());
+  }
+
+  public void testDuplicateXRef_SingleValue() throws Exception
+  {
+    CDOSession session = openSession();
+    CDOTransaction transaction = session.openTransaction();
+
+    // Initialize main branch with two objects, an order detail with a given product.
+    Product1 product = getModel1Factory().createProduct1();
+    OrderDetail orderDetail = getModel1Factory().createOrderDetail();
+    orderDetail.setProduct(product);
+
+    CDOResource resource = transaction.createResource(getResourcePath("res"));
+    resource.getContents().add(product);
+    resource.getContents().add(orderDetail);
+    transaction.commit();
+
+    // Create a branch and open a transaction on it.
+    CDOBranch derivedBranch = transaction.getBranch().createBranch("Derived");
+    transaction = session.openTransaction(derivedBranch);
+    orderDetail = transaction.getObject(orderDetail);
+    product = transaction.getObject(product);
+
+    // Change an attribute of the order detail and commit the change.
+    orderDetail.setPrice(47.11f);
+    transaction.commit();
+
+    // Check XRef API on derived branch after object removal:
+    // An empty result is supposed to be retrieved at this point.
+    List<CDOObjectReference> xRefs = transaction.queryXRefs(CDOUtil.getCDOObject(product));
+    assertEquals(1, xRefs.size());
+  }
+}