summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorCaspar De Groot2010-04-27 05:18:46 (EDT)
committerCaspar De Groot2010-04-27 05:18:46 (EDT)
commita93e2710f850c334f5d2c9823db0889dc9e7aaee (patch)
tree662f7104926f6864b2632a8358e70566afbb8812
parent9f2732ad704b9cedcf49f3cdfde0b7e685bd43b7 (diff)
downloadcdo-a93e2710f850c334f5d2c9823db0889dc9e7aaee.zip
cdo-a93e2710f850c334f5d2c9823db0889dc9e7aaee.tar.gz
cdo-a93e2710f850c334f5d2c9823db0889dc9e7aaee.tar.bz2
[303466] CDO not robust when using dynamic packages
https://bugs.eclipse.org/bugs/show_bug.cgi?id=303466
-rw-r--r--plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/CDOModelUtil.java70
-rw-r--r--plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/EMFUtil.java43
-rw-r--r--plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/CommitTransactionIndication.java26
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java2
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/PackageRegistryTest.java2
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_303466_Test.java149
6 files changed, 288 insertions, 4 deletions
diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/CDOModelUtil.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/CDOModelUtil.java
index 6b5f696..24ee4ba 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/CDOModelUtil.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/CDOModelUtil.java
@@ -22,14 +22,18 @@ import org.eclipse.net4j.util.io.ExtendedDataOutput;
import org.eclipse.emf.common.notify.Adapter;
import org.eclipse.emf.common.util.EList;
import org.eclipse.emf.common.util.Enumerator;
+import org.eclipse.emf.common.util.TreeIterator;
import org.eclipse.emf.ecore.EClass;
import org.eclipse.emf.ecore.EClassifier;
import org.eclipse.emf.ecore.EDataType;
import org.eclipse.emf.ecore.EEnum;
import org.eclipse.emf.ecore.EEnumLiteral;
+import org.eclipse.emf.ecore.EGenericType;
+import org.eclipse.emf.ecore.EObject;
import org.eclipse.emf.ecore.EPackage;
import org.eclipse.emf.ecore.EStructuralFeature;
import org.eclipse.emf.ecore.EcorePackage;
+import org.eclipse.emf.ecore.InternalEObject;
import org.eclipse.emf.ecore.resource.ResourceSet;
import org.eclipse.emf.ecore.util.EcoreUtil;
import org.eclipse.emf.ecore.util.FeatureMapUtil;
@@ -377,6 +381,8 @@ public final class CDOModelUtil
public static void writePackage(ExtendedDataOutput out, EPackage ePackage, boolean zipped,
EPackage.Registry packageRegistry) throws IOException
{
+ checkCrossResourceURIs(ePackage);
+
byte[] bytes = EMFUtil.getEPackageBytes(ePackage, zipped, packageRegistry);
out.writeString(ePackage.getNsURI());
out.writeBoolean(zipped);
@@ -386,6 +392,70 @@ public final class CDOModelUtil
/**
* @since 3.0
*/
+ public static void checkCrossResourceURIs(EPackage ePackage)
+ {
+ TreeIterator<EObject> it = ePackage.eAllContents();
+ while (it.hasNext())
+ {
+ EObject e = it.next();
+ for (EObject r : e.eCrossReferences())
+ {
+ if (r.eIsProxy())
+ {
+ String msg = "Package '%s' contains unresolved proxy '%s'";
+ msg = String.format(msg, ePackage.getNsURI(), ((InternalEObject)r).eProxyURI());
+ throw new IllegalStateException(msg);
+ }
+
+ if (r.eResource() != null && r.eResource() != e.eResource())
+ {
+ // It's a ref into another resource
+ EPackage pkg = null;
+ if (r instanceof EClassifier)
+ {
+ pkg = ((EClassifier)r).getEPackage();
+ }
+ else if (r instanceof EStructuralFeature)
+ {
+ EStructuralFeature feature = (EStructuralFeature)r;
+ EClass ownerClass = (EClass)feature.eContainer();
+ pkg = ownerClass.getEPackage();
+ }
+ else if (r instanceof EGenericType)
+ {
+ EGenericType genType = (EGenericType)r;
+ EClassifier c = genType.getEClassifier();
+ if (c != null)
+ {
+ pkg = c.getEPackage();
+ }
+ }
+
+ if (pkg == null)
+ {
+ continue;
+ }
+
+ while (pkg.getESuperPackage() != null)
+ {
+ pkg = pkg.getESuperPackage();
+ }
+
+ String resourceURI = r.eResource().getURI().toString();
+ if (!resourceURI.toString().equals(pkg.getNsURI()))
+ {
+ String msg = "URI of the resource (%s) does not match the nsURI (%s) of the top-level package";
+ msg = String.format(msg, resourceURI, pkg.getNsURI());
+ throw new IllegalStateException(msg);
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * @since 3.0
+ */
public static EPackage readPackage(ExtendedDataInput in, ResourceSet resourceSet, boolean lookForResource)
throws IOException
{
diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/EMFUtil.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/EMFUtil.java
index 23b6063..e455d03 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/EMFUtil.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/model/EMFUtil.java
@@ -17,6 +17,7 @@ import org.eclipse.net4j.util.WrappedException;
import org.eclipse.emf.common.notify.Adapter;
import org.eclipse.emf.common.notify.Notifier;
import org.eclipse.emf.common.util.EList;
+import org.eclipse.emf.common.util.TreeIterator;
import org.eclipse.emf.common.util.URI;
import org.eclipse.emf.ecore.EAttribute;
import org.eclipse.emf.ecore.EClass;
@@ -27,6 +28,7 @@ import org.eclipse.emf.ecore.EReference;
import org.eclipse.emf.ecore.EStructuralFeature;
import org.eclipse.emf.ecore.EcoreFactory;
import org.eclipse.emf.ecore.EcorePackage;
+import org.eclipse.emf.ecore.InternalEObject;
import org.eclipse.emf.ecore.impl.EPackageImpl;
import org.eclipse.emf.ecore.resource.Resource;
import org.eclipse.emf.ecore.resource.ResourceSet;
@@ -40,6 +42,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -377,4 +380,44 @@ public final class EMFUtil
{
return newEcoreResourceSet(EPackage.Registry.INSTANCE);
}
+
+ /**
+ * @since 3.0
+ */
+ public static EObject safeResolve(EObject proxy, ResourceSet resourceSet)
+ {
+ if (!proxy.eIsProxy())
+ {
+ return proxy;
+ }
+
+ EObject resolved = EcoreUtil.resolve(proxy, resourceSet);
+ if (resolved == proxy)
+ {
+ throw new IllegalStateException("Unresolvable proxy: " + ((InternalEObject)proxy).eProxyURI());
+ }
+
+ return resolved;
+ }
+
+ /**
+ * @since 3.0
+ */
+ public static void safeResolveAll(ResourceSet resourceSet)
+ {
+ TreeIterator<Notifier> it = resourceSet.getAllContents();
+ while (it.hasNext())
+ {
+ Notifier notifier = it.next();
+ if (notifier instanceof EObject)
+ {
+ EMFUtil.safeResolve((EObject)notifier, resourceSet);
+ Iterator<EObject> it2 = ((EObject)notifier).eCrossReferences().iterator();
+ while (it2.hasNext())
+ {
+ EMFUtil.safeResolve(it2.next(), resourceSet);
+ }
+ }
+ }
+ }
}
diff --git a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/CommitTransactionIndication.java b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/CommitTransactionIndication.java
index 8ddcbfd..34c475b 100644
--- a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/CommitTransactionIndication.java
+++ b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/CommitTransactionIndication.java
@@ -50,8 +50,10 @@ import org.eclipse.net4j.util.om.monitor.OMMonitor;
import org.eclipse.net4j.util.om.monitor.ProgressDistributor;
import org.eclipse.net4j.util.om.trace.ContextTracer;
+import org.eclipse.emf.ecore.resource.Resource;
import org.eclipse.emf.ecore.resource.ResourceSet;
-import org.eclipse.emf.ecore.util.EcoreUtil;
+import org.eclipse.emf.ecore.resource.impl.ResourceSetImpl;
+import org.eclipse.emf.ecore.xmi.impl.EcoreResourceFactoryImpl;
import java.io.IOException;
import java.util.List;
@@ -201,7 +203,7 @@ public class CommitTransactionIndication extends IndicationWithMonitoring
}
InternalCDOPackageRegistry packageRegistry = commitContext.getPackageRegistry();
- ResourceSet resourceSet = EMFUtil.newEcoreResourceSet(packageRegistry);
+ ResourceSet resourceSet = createResourceSet(packageRegistry);
for (int i = 0; i < newPackageUnits.length; i++)
{
newPackageUnits[i] = (InternalCDOPackageUnit)in.readCDOPackageUnit(resourceSet);
@@ -210,7 +212,8 @@ public class CommitTransactionIndication extends IndicationWithMonitoring
}
// When all packages are deserialized and registered, resolve them
- EcoreUtil.resolveAll(resourceSet);
+ // Note: EcoreUtil.resolveAll(resourceSet) does *not* do the trick
+ EMFUtil.safeResolveAll(resourceSet);
// New objects
if (TRACER.isEnabled())
@@ -254,6 +257,23 @@ public class CommitTransactionIndication extends IndicationWithMonitoring
}
}
+ private ResourceSet createResourceSet(InternalCDOPackageRegistry packageRegistry)
+ {
+ ResourceSet resourceSet = new ResourceSetImpl()
+ {
+ @Override
+ protected void demandLoad(Resource resource) throws IOException
+ {
+ // Do nothing: we don't want this ResourceSet to attempt demandloads.
+ }
+ };
+
+ Resource.Factory resourceFactory = new EcoreResourceFactoryImpl();
+ resourceSet.getResourceFactoryRegistry().getExtensionToFactoryMap().put("*", resourceFactory); //$NON-NLS-1$
+ resourceSet.setPackageRegistry(packageRegistry);
+ return resourceSet;
+ }
+
protected void initializeCommitContext(CDODataInput in) throws Exception
{
int viewID = in.readInt();
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java
index e95c623..85ddd68 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java
@@ -66,6 +66,7 @@ import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_294859_Test;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_298561_Test;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_299190_Test;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_302233_Test;
+import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_303466_Test;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_303807_Test;
import org.eclipse.emf.cdo.tests.config.impl.ConfigTest;
import org.eclipse.emf.cdo.tests.config.impl.ConfigTestSuite;
@@ -193,6 +194,7 @@ public abstract class AllTestsAllConfigs extends ConfigTestSuite
testClasses.add(Bugzilla_298561_Test.class);
testClasses.add(Bugzilla_299190_Test.class);
testClasses.add(Bugzilla_302233_Test.class);
+ testClasses.add(Bugzilla_303466_Test.class);
testClasses.add(Bugzilla_303807_Test.class);
// TODO testClasses.add(NonCDOResourceTest.class);
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/PackageRegistryTest.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/PackageRegistryTest.java
index f9cc7aa..3cab2e5 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/PackageRegistryTest.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/PackageRegistryTest.java
@@ -678,7 +678,7 @@ public class PackageRegistryTest extends AbstractCDOTest
assertEquals("Eike", company.getName());
}
- private static EPackage loadModel(String fileName) throws IOException
+ public static EPackage loadModel(String fileName) throws IOException
{
URI uri = URI.createURI("file://" + fileName);
XMIResource resource = new XMIResourceImpl(uri);
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_303466_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_303466_Test.java
new file mode 100644
index 0000000..98fe5c2
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_303466_Test.java
@@ -0,0 +1,149 @@
+/**
+ * Copyright (c) 2004 - 2010 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:
+ * Caspar De Groot
+ */
+package org.eclipse.emf.cdo.tests.bugzilla;
+
+import org.eclipse.emf.cdo.common.model.EMFUtil;
+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.PackageRegistryTest;
+import org.eclipse.emf.cdo.transaction.CDOTransaction;
+
+import org.eclipse.net4j.signal.RemoteException;
+import org.eclipse.net4j.util.transaction.TransactionException;
+
+import org.eclipse.emf.common.util.URI;
+import org.eclipse.emf.ecore.EClass;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EPackage;
+import org.eclipse.emf.ecore.resource.Resource;
+import org.eclipse.emf.ecore.resource.ResourceSet;
+
+import java.io.IOException;
+import java.util.Map;
+
+/**
+ * CDO not robust when using dynamic packages
+ *
+ * @author Caspar De Groot
+ */
+public class Bugzilla_303466_Test extends AbstractCDOTest
+{
+ public void test_missingDependency() throws IOException
+ {
+ CDOSession session = openSession();
+
+ ResourceSet resourceSet = EMFUtil.newEcoreResourceSet();
+ EPackage derivedPkg = PackageRegistryTest.loadModel("model/derived.ecore");
+ Resource resource = derivedPkg.eResource();
+ resourceSet.getResources().add(resource);
+ resource.load(null);
+
+ session.getPackageRegistry().putEPackage(derivedPkg);
+ assertEquals(3, session.getPackageRegistry().size());
+
+ EClass derivedClass = (EClass)derivedPkg.getEClassifier("DerivedClass");
+ EObject derived = derivedPkg.getEFactoryInstance().create(derivedClass);
+
+ // Verify that the feature inherited from the base class is missing
+ assertNull(derivedClass.getEStructuralFeature("couter"));
+
+ CDOTransaction tx = session.openTransaction();
+ CDOResource resource2 = tx.createResource("/resource1");
+ resource2.getContents().add(derived);
+
+ try
+ {
+ tx.commit();
+ fail("Should have thrown an exception because of the missing base package");
+ }
+ catch (TransactionException e)
+ {
+ if (e.getCause() instanceof IllegalStateException)
+ {
+ // Good
+ }
+ else if (e.getCause() instanceof RemoteException)
+ {
+ fail("Problem should have been detected on the client side");
+ }
+ else
+ {
+ throw e;
+ }
+ }
+ finally
+ {
+ tx.close();
+ session.close();
+ }
+ }
+
+ public void test_badUris() throws IOException
+ {
+ CDOSession session = openSession();
+
+ ResourceSet resourceSet = EMFUtil.newEcoreResourceSet();
+
+ EPackage basePkg = PackageRegistryTest.loadModel("model/base.ecore");
+ Resource resource1 = basePkg.eResource();
+ resourceSet.getResources().add(resource1);
+ resourceSet.getPackageRegistry().put(basePkg.getNsURI(), basePkg);
+
+ EPackage derivedPkg = PackageRegistryTest.loadModel("model/derived.ecore");
+ Resource resource2 = derivedPkg.eResource();
+ resourceSet.getResources().add(resource2);
+ resourceSet.getPackageRegistry().put(basePkg.getNsURI(), derivedPkg);
+
+ Map<URI, URI> uriMap = resourceSet.getURIConverter().getURIMap();
+ URI modelUri3 = URI.createFileURI("base.ecore");
+ uriMap.put(modelUri3, resource1.getURI());
+
+ EMFUtil.safeResolveAll(resourceSet);
+
+ session.getPackageRegistry().putEPackage(basePkg);
+ session.getPackageRegistry().putEPackage(derivedPkg);
+
+ EClass derivedClass = (EClass)derivedPkg.getEClassifier("DerivedClass");
+ EObject derived = derivedPkg.getEFactoryInstance().create(derivedClass);
+
+ // Verify that the feature inherited from the base class is NOT missing on the client side
+ assertNotNull(derivedClass.getEStructuralFeature("couter"));
+
+ CDOTransaction tx = session.openTransaction();
+ CDOResource resource3 = tx.createResource("/resource1");
+ resource3.getContents().add(derived);
+
+ try
+ {
+ tx.commit();
+ fail("Should have thrown an exception because of the file URIs in the dependent package");
+ }
+ catch (TransactionException e)
+ {
+ if (e.getCause() instanceof IllegalStateException)
+ {
+ // Good
+ }
+ else if (e.getCause() instanceof RemoteException)
+ {
+ fail("Problem should have been detected on the client side");
+ }
+ else
+ {
+ fail("Should have thrown an " + IllegalStateException.class.getName());
+ }
+ }
+
+ tx.close();
+ session.close();
+ }
+}