[351600] Cannot load resource on a previously cleared ResourceSet
https://bugs.eclipse.org/bugs/show_bug.cgi?id=351600
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllConfigs.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllConfigs.java
index 180f90c..0057bd5 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllConfigs.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllConfigs.java
@@ -218,6 +218,7 @@
testClasses.add(Bugzilla_337587_Test.class);
testClasses.add(Bugzilla_338779_Test.class);
testClasses.add(Bugzilla_338884_Test.class);
+ testClasses.add(Bugzilla_338921_Test.class);
testClasses.add(Bugzilla_339313_Test.class);
testClasses.add(Bugzilla_339461_Test.class);
testClasses.add(Bugzilla_339908_Test.class);
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/ResourceTest.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/ResourceTest.java
index 5026798..76529af 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/ResourceTest.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/ResourceTest.java
@@ -412,55 +412,13 @@
}
}
- public void testAttachManyResources() throws Exception
+ public void testAttachResource() throws Exception
{
CDOSession session = openSession();
CDOTransaction transaction = session.openTransaction();
- CDOResource resource1 = transaction.createResource(getResourcePath("/my/resource1"));
- CDOResource resource2 = transaction.createResource(getResourcePath("/my/resource2"));
- CDOResource resource3 = transaction.createResource(getResourcePath("/my/resource3"));
-
- List<Resource> tobeRemoved = new ArrayList<Resource>();
- tobeRemoved.add(resource1);
- tobeRemoved.add(resource3);
- assertEquals(3, transaction.getResourceSet().getResources().size());// Bug 346636
-
- transaction.getResourceSet().getResources().removeAll(tobeRemoved);
+ transaction.createResource(getResourcePath("/my/resource1"));
assertEquals(1, transaction.getResourceSet().getResources().size());// Bug 346636
- assertEquals(null, transaction.getResourceSet().getResource(resource1.getURI(), false));
- assertEquals(resource2, transaction.getResourceSet().getResource(resource2.getURI(), false));
- assertEquals(null, transaction.getResourceSet().getResource(resource3.getURI(), false));
-
- transaction.getResourceSet().getResources().addAll(tobeRemoved);
- assertEquals(3, transaction.getResourceSet().getResources().size());// Bug 346636
- assertEquals(resource1, transaction.getResourceSet().getResource(resource1.getURI(), false));
- assertEquals(resource2, transaction.getResourceSet().getResource(resource2.getURI(), false));
- assertEquals(resource3, transaction.getResourceSet().getResource(resource3.getURI(), false));
-
- transaction.commit();
- session.close();
- }
-
- public void testDetachManyResources() throws Exception
- {
- CDOSession session = openSession();
- CDOTransaction transaction = session.openTransaction();
-
- CDOResource resource1 = transaction.createResource(getResourcePath("/my/resource1"));
- CDOResource resource2 = transaction.createResource(getResourcePath("/my/resource2"));
- CDOResource resource3 = transaction.createResource(getResourcePath("/my/resource3"));
-
- List<Resource> tobeRemoved = new ArrayList<Resource>();
- tobeRemoved.add(resource1);
- tobeRemoved.add(resource3);
- assertEquals(3, transaction.getResourceSet().getResources().size());// Bug 346636
-
- transaction.getResourceSet().getResources().removeAll(tobeRemoved);
- assertEquals(1, transaction.getResourceSet().getResources().size());// Bug 346636
- assertEquals(null, transaction.getResourceSet().getResource(resource1.getURI(), false));
- assertEquals(resource2, transaction.getResourceSet().getResource(resource2.getURI(), false));
- assertEquals(null, transaction.getResourceSet().getResource(resource3.getURI(), false));
transaction.commit();
session.close();
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_338921_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_338921_Test.java
new file mode 100644
index 0000000..5887f91
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_338921_Test.java
@@ -0,0 +1,175 @@
+/**
+ * Copyright (c) 2004 - 2011 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:
+ * Victor Roldan Betancort - initial API and implementation
+ */
+package org.eclipse.emf.cdo.tests.bugzilla;
+
+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.model1.Company;
+import org.eclipse.emf.cdo.transaction.CDOTransaction;
+
+import org.eclipse.emf.common.util.URI;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.resource.Resource;
+import org.eclipse.emf.ecore.resource.ResourceSet;
+
+/**
+ * Cannot load resource on a previously cleared ResourceSet
+ * <p>
+ * See Bug 338921
+ *
+ * @author Victor Roldan Betancort
+ */
+public class Bugzilla_338921_Test extends AbstractCDOTest
+{
+ public void testLoadResourceAfterSingleRemoval() throws Exception
+ {
+ Company company = getModel1Factory().createCompany();
+
+ CDOSession session = openSession();
+ CDOTransaction transaction = session.openTransaction();
+
+ CDOResource resource = transaction.createResource(getResourcePath("/Bug338921"));
+ resource.getContents().add(company);
+
+ transaction.commit();
+
+ URI uri = resource.getURI();
+
+ ResourceSet resourceSet = resource.getResourceSet();
+ resourceSet.getResources().remove(resource);
+
+ assertInvalid(resource);
+
+ Resource resource2 = resourceSet.getResource(uri, true);
+ assertNotNull(resource2);
+ // forcing transition from PROXY to CLEAN. Revision is loaded
+ resource2.getURI();
+ assertClean((EObject)resource2, transaction);
+
+ }
+
+ public void testRemoveDirtyResourceOnResourceSetWithSingleResource() throws Exception
+ {
+ Company company = getModel1Factory().createCompany();
+
+ CDOSession session = openSession();
+ CDOTransaction transaction = session.openTransaction();
+
+ CDOResource resource = transaction.createResource(getResourcePath("/Bug338921"));
+ resource.getContents().add(company);
+
+ transaction.commit();
+
+ company.setName("foobar");
+ URI uri = resource.getURI();
+
+ ResourceSet resourceSet = resource.getResourceSet();
+ resourceSet.getResources().remove(resource);
+
+ Resource resource2 = resourceSet.getResource(uri, true);
+ assertNotNull(resource2);
+ // forcing transition from PROXY to CLEAN. Revision is loaded
+ resource2.getURI();
+ assertClean((EObject)resource2, transaction);
+
+ }
+
+ public void testRemoveDirtyResourceOnResourceSetWithMultipleResource() throws Exception
+ {
+ CDOSession session = openSession();
+ CDOTransaction transaction = session.openTransaction();
+
+ CDOResource resource1 = transaction.createResource(getResourcePath("/Bug338921/res1"));
+ CDOResource resource2 = transaction.createResource(getResourcePath("/Bug338921/res2"));
+ Company company1 = getModel1Factory().createCompany();
+ Company company2 = getModel1Factory().createCompany();
+
+ resource1.getContents().add(company1);
+ resource2.getContents().add(company2);
+
+ transaction.commit();
+
+ company1.setName("foo");
+ company2.setName("bar");
+
+ ResourceSet resourceSet = resource1.getResourceSet();
+
+ resourceSet.getResources().remove(resource1);
+ assertEquals(true, transaction.isDirty());
+ }
+
+ public void testLoadResourceAfterClearOnCleanResourceSet() throws Exception
+ {
+ Company company = getModel1Factory().createCompany();
+
+ CDOSession session = openSession();
+ CDOTransaction transaction = session.openTransaction();
+
+ CDOResource resource1 = transaction.createResource(getResourcePath("/Bug338921/res1"));
+ transaction.createResource(getResourcePath("/Bug338921/res2"));
+ resource1.getContents().add(company);
+
+ transaction.commit();
+
+ URI uri = resource1.getURI();
+ company.setName("foobar");
+
+ ResourceSet resourceSet = resource1.getResourceSet();
+ resourceSet.getResources().clear();
+
+ assertEquals(true, transaction.isDirty());
+ assertInvalid(resource1);
+
+ Resource resource3 = resourceSet.getResource(uri, true);
+ assertNotNull(resource3);
+ // forcing transition from PROXY to CLEAN. Revision is loaded
+ resource3.getURI();
+ assertClean((EObject)resource3, transaction);
+
+ }
+
+ public void testLoadResourceAfterClearOnDirtyResourceSet() throws Exception
+ {
+ Company company = getModel1Factory().createCompany();
+
+ CDOSession session = openSession();
+ CDOTransaction transaction = session.openTransaction();
+
+ CDOResource resource1 = transaction.createResource(getResourcePath("/Bug338921/res1"));
+ CDOResource resource2 = transaction.createResource(getResourcePath("/Bug338921/res2"));
+ resource1.getContents().add(company);
+
+ transaction.commit();
+
+ URI uri1 = resource1.getURI();
+ URI uri2 = resource2.getURI();
+ company.setName("foobar");
+
+ ResourceSet resourceSet = resource1.getResourceSet();
+ resourceSet.getResources().clear();
+
+ assertEquals(true, transaction.isDirty());
+ assertInvalid(resource1);
+ assertInvalid(resource2);
+
+ CDOResource resource3 = (CDOResource)resourceSet.getResource(uri1, true);
+ CDOResource resource4 = (CDOResource)resourceSet.getResource(uri2, true);
+ assertNotNull(resource3);
+ assertNotNull(resource4);
+ // forcing transition from PROXY to CLEAN. Revision is loaded
+ resource3.getURI();
+ assertClean(resource3, transaction);
+ resource4.getURI();
+ assertClean(resource4, transaction);
+ }
+
+}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/eresource/impl/CDOResourceImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/eresource/impl/CDOResourceImpl.java
index 395a2ad..3f75512 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/eresource/impl/CDOResourceImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/eresource/impl/CDOResourceImpl.java
@@ -26,9 +26,11 @@
import org.eclipse.emf.cdo.view.CDOView;
import org.eclipse.emf.cdo.view.CDOViewProvider;
import org.eclipse.emf.cdo.view.CDOViewProviderRegistry;
+import org.eclipse.emf.cdo.view.CDOViewSet;
import org.eclipse.emf.internal.cdo.bundle.OM;
import org.eclipse.emf.internal.cdo.view.CDOStateMachine;
+import org.eclipse.emf.internal.cdo.view.CDOViewSetImpl;
import org.eclipse.net4j.util.WrappedException;
import org.eclipse.net4j.util.collection.Pair;
@@ -72,6 +74,7 @@
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
+import java.util.concurrent.Callable;
/**
* <!-- begin-user-doc --> An implementation of the model object '<em><b>CDO Resource</b></em>'.
@@ -347,12 +350,9 @@
if (remote)
{
existing = false;
- cdoView().getResourceSet().getResources().remove(this);
}
- else
- {
- removeFromResourceSet();
- }
+
+ removeFromResourceSet();
}
/**
@@ -1190,10 +1190,21 @@
private void removeFromResourceSet()
{
- ResourceSet resourceSet = getResourceSet();
+ final ResourceSet resourceSet = getResourceSet();
if (resourceSet != null)
{
- resourceSet.getResources().remove(this);
+ CDOViewSet viewSet = CDOUtil.getViewSet(resourceSet);
+ if (viewSet instanceof CDOViewSetImpl)
+ {
+ ((CDOViewSetImpl)viewSet).executeWithoutNotificationHandling(new Callable<Boolean>()
+ {
+ public Boolean call() throws Exception
+ {
+ resourceSet.getResources().remove(CDOResourceImpl.this);
+ return true;
+ }
+ });
+ }
}
}
@@ -1235,10 +1246,22 @@
*/
public NotificationChain basicSetResourceSet(ResourceSet resourceSet, NotificationChain notifications)
{
- ResourceSet oldResourceSet = getResourceSet();
+ final ResourceSet oldResourceSet = getResourceSet();
if (oldResourceSet != null)
{
- notifications = ((InternalEList<Resource>)oldResourceSet.getResources()).basicRemove(this, notifications);
+ final NotificationChain finalNotifications = notifications;
+
+ CDOViewSet viewSet = CDOUtil.getViewSet(oldResourceSet);
+ if (viewSet instanceof CDOViewSetImpl)
+ {
+ notifications = ((CDOViewSetImpl)viewSet).executeWithoutNotificationHandling(new Callable<NotificationChain>()
+ {
+ public NotificationChain call() throws Exception
+ {
+ return ((InternalEList<Resource>)oldResourceSet.getResources()).basicRemove(this, finalNotifications);
+ }
+ });
+ }
}
setResourceSet(resourceSet);
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 3ef76dd..11aea04 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
@@ -100,6 +100,7 @@
import org.eclipse.emf.internal.cdo.util.IPackageClosure;
import org.eclipse.emf.internal.cdo.view.CDOStateMachine;
import org.eclipse.emf.internal.cdo.view.CDOViewImpl;
+import org.eclipse.emf.internal.cdo.view.CDOViewSetImpl;
import org.eclipse.net4j.util.ObjectUtil;
import org.eclipse.net4j.util.WrappedException;
@@ -132,6 +133,7 @@
import org.eclipse.emf.spi.cdo.InternalCDOSavepoint;
import org.eclipse.emf.spi.cdo.InternalCDOSession;
import org.eclipse.emf.spi.cdo.InternalCDOTransaction;
+import org.eclipse.emf.spi.cdo.InternalCDOViewSet;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
@@ -151,6 +153,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger;
@@ -1096,14 +1099,25 @@
cleanUp(null);
}
- private void removeObject(CDOID id, CDOObject object)
+ private void removeObject(CDOID id, final CDOObject object)
{
((InternalCDOObject)object).cdoInternalSetState(CDOState.TRANSIENT);
removeObject(id);
if (object instanceof CDOResource)
{
- getResourceSet().getResources().remove(object);
+ InternalCDOViewSet viewSet = getViewSet();
+ if (viewSet instanceof CDOViewSetImpl)
+ {
+ ((CDOViewSetImpl)viewSet).executeWithoutNotificationHandling(new Callable<Boolean>()
+ {
+ public Boolean call() throws Exception
+ {
+ getResourceSet().getResources().remove(object);
+ return true;
+ }
+ });
+ }
}
((InternalCDOObject)object).cdoInternalSetID(null);
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewSetImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewSetImpl.java
index f0cfd02..f1ad2b8 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewSetImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewSetImpl.java
@@ -8,16 +8,21 @@
* Contributors:
* Simon McDuff - initial API and implementation
* Eike Stepper - maintenance
+ * Victor Roldan Betancort - bug 338921
*/
package org.eclipse.emf.internal.cdo.view;
+import org.eclipse.emf.cdo.CDOState;
import org.eclipse.emf.cdo.common.protocol.CDOProtocolConstants;
+import org.eclipse.emf.cdo.common.util.CDOException;
import org.eclipse.emf.cdo.eresource.CDOResource;
import org.eclipse.emf.cdo.eresource.CDOResourceFactory;
import org.eclipse.emf.cdo.view.CDOView;
import org.eclipse.emf.internal.cdo.messages.Messages;
+import org.eclipse.net4j.util.WrappedException;
+
import org.eclipse.emf.common.notify.Notification;
import org.eclipse.emf.common.notify.Notifier;
import org.eclipse.emf.common.notify.impl.NotificationImpl;
@@ -26,16 +31,21 @@
import org.eclipse.emf.ecore.resource.Resource;
import org.eclipse.emf.ecore.resource.Resource.Factory.Registry;
import org.eclipse.emf.ecore.resource.ResourceSet;
+import org.eclipse.emf.spi.cdo.InternalCDOObject;
import org.eclipse.emf.spi.cdo.InternalCDOView;
import org.eclipse.emf.spi.cdo.InternalCDOViewSet;
import java.text.MessageFormat;
import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
+import java.util.concurrent.Callable;
/**
* @author Simon McDuff
@@ -53,6 +63,8 @@
private ResourceSet resourceSet;
+ private ThreadLocal<Boolean> ignoreNotifications = new InheritableThreadLocal<Boolean>();
+
public CDOViewSetImpl()
{
}
@@ -208,9 +220,137 @@
return type instanceof ResourceSet;
}
+ public synchronized <V> V executeWithoutNotificationHandling(Callable<V> callable)
+ {
+ Boolean wasIgnore = ignoreNotifications.get();
+
+ try
+ {
+ ignoreNotifications.set(true);
+ return callable.call();
+ }
+ catch (Exception ex)
+ {
+ throw WrappedException.wrap(ex);
+ }
+ finally
+ {
+ if (wasIgnore == null)
+ {
+ ignoreNotifications.remove();
+ }
+ }
+ }
+
public void notifyChanged(Notification notification)
{
- // Do nothing
// The resource <-> view association is done in CDOResourceImpl.basicSetResourceSet()
+
+ if (ignoreNotifications.get() == null)
+ {
+ // We need to deregister CDOResources from CDOView if removed from the ResourceSet, see bug 338921
+ switch (notification.getEventType())
+ {
+ case Notification.REMOVE_MANY:
+ deregisterResources((List<?>)notification.getOldValue());
+ break;
+
+ case Notification.REMOVE:
+ deregisterResources(Collections.singleton(notification.getOldValue()));
+ break;
+ }
+ }
+ }
+
+ private void deregisterResources(Collection<?> potentialResources)
+ {
+ List<CDOResource> allDirtyResources = new ArrayList<CDOResource>();
+
+ try
+ {
+ Map<CDOView, List<CDOResource>> resourcesPerView = getResourcesPerView(potentialResources);
+
+ for (Entry<CDOView, List<CDOResource>> entry : resourcesPerView.entrySet())
+ {
+ InternalCDOView view = (InternalCDOView)entry.getKey();
+ List<CDOResource> resources = entry.getValue();
+
+ if (view.isDirty())
+ {
+ List<CDOResource> dirtyResources = getDirtyResources(resources);
+ if (!dirtyResources.isEmpty())
+ {
+ allDirtyResources.addAll(dirtyResources);
+ resourceSet.getResources().addAll(resources);
+ continue;
+ }
+ }
+
+ for (CDOResource resource : resources)
+ {
+ InternalCDOObject internalResource = (InternalCDOObject)resource;
+ view.deregisterObject(internalResource);
+ internalResource.cdoInternalSetState(CDOState.INVALID);
+ }
+ }
+ }
+ finally
+ {
+ int size = allDirtyResources.size();
+ if (size == 1)
+ {
+ throw new CDOException("Attempt to remove a dirty resource from a resource set: " + allDirtyResources.get(0));
+ }
+ else if (size > 1)
+ {
+ throw new CDOException("Attempt to remove dirty resources from a resource set: " + allDirtyResources);
+ }
+ }
+ }
+
+ private List<CDOResource> getDirtyResources(List<CDOResource> resources)
+ {
+ List<CDOResource> dirtyResources = new ArrayList<CDOResource>();
+ for (CDOResource resource : resources)
+ {
+ switch (resource.cdoState())
+ {
+ case NEW:
+ case DIRTY:
+ case CONFLICT:
+ case INVALID_CONFLICT:
+ dirtyResources.addAll(resources);
+ }
+ }
+
+ return dirtyResources;
+ }
+
+ private Map<CDOView, List<CDOResource>> getResourcesPerView(Collection<?> potentialResources)
+ {
+ Map<CDOView, List<CDOResource>> resourcesPerView = new HashMap<CDOView, List<CDOResource>>();
+
+ for (Object potentialResource : potentialResources)
+ {
+ if (potentialResource instanceof CDOResource)
+ {
+ CDOResource resource = (CDOResource)potentialResource;
+ CDOView view = resource.cdoView();
+
+ if (views.contains(view))
+ {
+ List<CDOResource> resources = resourcesPerView.get(view);
+ if (resources == null)
+ {
+ resources = new ArrayList<CDOResource>();
+ resourcesPerView.put(view, resources);
+ }
+
+ resources.add(resource);
+ }
+ }
+ }
+
+ return resourcesPerView;
}
}
