[338921] Cannot load resource on a previously cleared ResourceSet

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

7 files changed, 361 insertions, 56 deletions

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 cb167624d8..d54322cdce 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
@@ -233,5 +233,6 @@ public abstract class AllConfigs extends ConfigTestSuite
     testClasses.add(Bugzilla_350027_Test.class);
     testClasses.add(Bugzilla_351067_Test.class);
     testClasses.add(Bugzilla_351096_Test.class);
+    testClasses.add(Bugzilla_338921_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 50267988c9..76529afe4f 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 class ResourceTest extends AbstractCDOTest
     }
   }
 
-  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);
-    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);
+    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.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 0000000000..5887f91bbf
--- /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 395a2adced..da97778d5c 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
@@ -63,6 +63,7 @@ import org.eclipse.emf.spi.cdo.FSMUtil;
 import org.eclipse.emf.spi.cdo.InternalCDOObject;
 import org.eclipse.emf.spi.cdo.InternalCDOTransaction;
 import org.eclipse.emf.spi.cdo.InternalCDOView;
+import org.eclipse.emf.spi.cdo.InternalCDOViewSet;
 
 import org.eclipse.core.runtime.IProgressMonitor;
 
@@ -72,6 +73,7 @@ import java.io.OutputStream;
 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 +349,9 @@ public class CDOResourceImpl extends CDOResourceNodeImpl implements CDOResource,
     if (remote)
     {
       existing = false;
-      cdoView().getResourceSet().getResources().remove(this);
-    }
-    else
-    {
-      removeFromResourceSet();
     }
+
+    removeFromResourceSet();
   }
 
   /**
@@ -1190,10 +1189,18 @@ public class CDOResourceImpl extends CDOResourceNodeImpl implements CDOResource,
 
   private void removeFromResourceSet()
   {
-    ResourceSet resourceSet = getResourceSet();
+    final ResourceSet resourceSet = getResourceSet();
     if (resourceSet != null)
     {
-      resourceSet.getResources().remove(this);
+      InternalCDOViewSet viewSet = (InternalCDOViewSet)CDOUtil.getViewSet(resourceSet);
+      viewSet.executeWithoutNotificationHandling(new Callable<Boolean>()
+      {
+        public Boolean call() throws Exception
+        {
+          resourceSet.getResources().remove(CDOResourceImpl.this);
+          return true;
+        }
+      });
     }
   }
 
@@ -1235,10 +1242,19 @@ public class CDOResourceImpl extends CDOResourceNodeImpl implements CDOResource,
    */
   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;
+
+      InternalCDOViewSet viewSet = (InternalCDOViewSet)CDOUtil.getViewSet(oldResourceSet);
+      notifications = 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 1d6bfddf41..b83b675440 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
@@ -152,6 +152,7 @@ import java.util.List;
 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;
 
@@ -1097,14 +1098,21 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
     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);
+      getViewSet().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 f0cfd02f58..f1ad2b8080 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.EPackage;
 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 @@ public class CDOViewSetImpl extends NotifierImpl implements InternalCDOViewSet
 
   private ResourceSet resourceSet;
 
+  private ThreadLocal<Boolean> ignoreNotifications = new InheritableThreadLocal<Boolean>();
+
   public CDOViewSetImpl()
   {
   }
@@ -208,9 +220,137 @@ public class CDOViewSetImpl extends NotifierImpl implements InternalCDOViewSet
     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;
   }
 }
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOViewSet.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOViewSet.java
index c9d68bcfc3..74395ceab9 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOViewSet.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOViewSet.java
@@ -14,6 +14,8 @@ import org.eclipse.emf.cdo.view.CDOViewSet;
 
 import org.eclipse.emf.common.notify.Adapter;
 
+import java.util.concurrent.Callable;
+
 /**
  * @author Eike Stepper
  * @since 2.0
@@ -27,4 +29,9 @@ public interface InternalCDOViewSet extends CDOViewSet, Adapter
   public void remove(InternalCDOView view);
 
   public InternalCDOView resolveView(String repositoryUUID);
+
+  /**
+   * @since 4.1
+   */
+  public <V> V executeWithoutNotificationHandling(Callable<V> callable);
 }