[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
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 fff86f0..459ef34 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.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.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 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,21 +72,30 @@
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 @@
* @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 @@
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 @@
return targetObjects;
}
- public EReference[] getSourceReferences()
+ public final EReference[] getSourceReferences()
{
if (sourceReferences == null)
{
@@ -297,10 +313,66 @@
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 @@
return result.toArray(new EReference[result.size()]);
}
+ }
- public Map<EClass, List<EReference>> getSourceCandidates()
+ /**
+ * @author Eike Stepper
+ */
+ private static final class QueryContextBranching extends QueryContext
+ {
+ private final CDOBranchPoint originalBranchPoint;
+
+ private final Set<CDOID> ignoredObjects = new HashSet<CDOID>();
+
+ public QueryContextBranching(CDOQueryInfo info, IQueryContext context)
{
- if (sourceCandidates == null)
- {
- sourceCandidates = new HashMap<EClass, List<EReference>>();
- Collection<EClass> concreteTypes = getTargetObjects().values();
- EReference[] sourceReferences = getSourceReferences();
-
- 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);
- }
- }
-
- return sourceCandidates;
+ super(info, context);
+ originalBranchPoint = CDOBranchUtil.copyBranchPoint(context);
}
- public int getMaxResults()
+ @Override
+ protected boolean isIgnoredObject(CDOID sourceID)
{
- return info.getMaxResults();
- }
-
- public boolean addXRef(CDOID targetID, CDOID sourceID, EReference sourceReference, int sourceIndex)
- {
- if (CDOIDUtil.isNull(targetID))
+ if (!ignoredObjects.add(sourceID))
{
return true;
}
- return context.addResult(new CDOIDReference(targetID, sourceID, sourceReference, sourceIndex));
+ if (isDetachedObject(sourceID))
+ {
+ ignoredObjects.add(sourceID);
+ return true;
+ }
+
+ return false;
+ }
+
+ private boolean isDetachedObject(CDOID sourceID)
+ {
+ if (getBranch() == originalBranchPoint.getBranch())
+ {
+ return false;
+ }
+
+ 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 0000000..1621ee2
--- /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());
+ }
+}
