[365832] Issues when trying to import changes: Cannot modify a frozen exception issue

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

6 files changed, 228 insertions, 2 deletions

diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/protocol/CDODataInput.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/protocol/CDODataInput.java
index 66273cedfe..2231e70115 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/protocol/CDODataInput.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/common/protocol/CDODataInput.java
@@ -114,6 +114,11 @@ public interface CDODataInput extends ExtendedDataInput
   public CDORevision readCDORevision() throws IOException;
 
   /**
+   * @since 4.1
+   */
+  public CDORevision readCDORevision(boolean freeze) throws IOException;
+
+  /**
    * @since 4.0
    */
   public CDORevisable readCDORevisable() throws IOException;
diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/protocol/CDODataInputImpl.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/protocol/CDODataInputImpl.java
index af4a9a4a35..c37dd98f69 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/protocol/CDODataInputImpl.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/protocol/CDODataInputImpl.java
@@ -475,12 +475,22 @@ public abstract class CDODataInputImpl extends ExtendedDataInput.Delegating impl
 
   public CDORevision readCDORevision() throws IOException
   {
+    return readCDORevision(true);
+  }
+
+  public CDORevision readCDORevision(boolean freeze) throws IOException
+  {
     boolean notNull = readBoolean();
     if (notNull)
     {
       InternalCDORevision revision = (InternalCDORevision)getRevisionFactory().createRevision(null);
       revision.read(this);
-      revision.freeze();
+
+      if (freeze)
+      {
+        revision.freeze();
+      }
+
       return revision;
     }
 
diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java
index 79519ef375..96e21a207c 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java
@@ -92,6 +92,11 @@ public class CDORevisionImpl extends BaseCDORevision
 
   public void freeze()
   {
+    if (getEClass().getName().equals("Node"))
+    {
+      System.out.println();
+    }
+
     frozen = true;
 
     EStructuralFeature[] features = CDOModelUtil.getAllPersistentFeatures(getEClass());
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 6a5f2a2bb5..aa0dc2fd4c 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
@@ -248,5 +248,6 @@ public abstract class AllConfigs extends ConfigTestSuite
     testClasses.add(Bugzilla_359966_Test.class);
     testClasses.add(Bugzilla_359992_Test.class);
     testClasses.add(Bugzilla_363287_Test.class);
+    testClasses.add(Bugzilla_365832_Test.class);
   }
 }
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_365832_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_365832_Test.java
new file mode 100644
index 0000000000..508318ec44
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_365832_Test.java
@@ -0,0 +1,205 @@
+/**

+ * 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:

+ *    Eike Stepper - 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.model6.A;

+import org.eclipse.emf.cdo.tests.model6.D;

+import org.eclipse.emf.cdo.tests.model6.E;

+import org.eclipse.emf.cdo.tests.model6.F;

+import org.eclipse.emf.cdo.transaction.CDOPushTransaction;

+import org.eclipse.emf.cdo.transaction.CDOTransaction;

+import org.eclipse.emf.cdo.view.CDOView;

+

+import org.eclipse.emf.common.util.EList;

+import org.eclipse.emf.ecore.EObject;

+

+import org.eclipse.gmf.runtime.notation.Diagram;

+import org.eclipse.gmf.runtime.notation.MeasurementUnit;

+import org.eclipse.gmf.runtime.notation.Node;

+import org.eclipse.gmf.runtime.notation.NotationFactory;

+

+import java.io.File;

+import java.io.IOException;

+

+/**

+ * Bug 365832: Issues when trying to import changes: Cannot modify a frozen exception issue

+ * 

+ * @author Alex Lagarde

+ */

+public class Bugzilla_365832_Test extends AbstractCDOTest

+{

+  private static final String MODEL_LOCATION_PATH = "myResource";

+

+  public void testWithoutPushTransaction() throws Exception

+  {

+    CDOSession session = openSession();

+    CDOTransaction transaction = session.openTransaction();

+    CDOResource resource = transaction.createResource(getResourcePath(MODEL_LOCATION_PATH));

+    resource.getContents().add(createModel());

+    transaction.commit();

+

+    addNewChildren(transaction);

+    checkDiagramAsCorrectlyBeenModified(transaction);

+

+    transaction.commit();

+    checkDiagramAsCorrectlyBeenModified(transaction);

+  }

+

+  public void testWithReconstructSavepoints() throws Exception

+  {

+    importWithNewLegacyElements(true);

+  }

+

+  public void testWithoutReconstructSavepoints() throws Exception

+  {

+    importWithNewLegacyElements(false);

+  }

+

+  private void importWithNewLegacyElements(boolean reconstructSavePoints) throws Exception

+  {

+    // Step 1 : create model

+    CDOSession session = openSession();

+    CDOTransaction transaction = session.openTransaction();

+    CDOResource resource = transaction.createResource(getResourcePath(MODEL_LOCATION_PATH));

+    resource.getContents().add(createModel());

+    transaction.commit();

+    session.close();

+

+    // Step 2 : open a push transaction a save locally a modification

+    // Step 2.1 : open a push transaction

+    File fileForStoringChanges = createTempFile();

+    CDOPushTransaction pushTransaction = createPushTransaction(fileForStoringChanges, reconstructSavePoints);

+

+    // Step 2.2 : create a new element

+    addNewChildren(pushTransaction);

+

+    // => make sure that diagram is modified as expected

+    checkDiagramAsCorrectlyBeenModified(pushTransaction);

+

+    // Step 2.3 : save locally

+    pushTransaction.commit();

+    pushTransaction.getSession().close();

+

+    // Step 3 : load changes

+    pushTransaction = createPushTransaction(fileForStoringChanges, reconstructSavePoints);

+

+    // => check that diagram is modified as expected

+    checkDiagramAsCorrectlyBeenModified(pushTransaction);

+

+    // => check that diagram is modified as expected

+    checkDiagramAsCorrectlyBeenModified(pushTransaction);

+

+    // Step 4 : try to modify objects and commit

+    Diagram diagram = getDiagram(pushTransaction);

+

+    @SuppressWarnings("unchecked")

+    EList<Node> children = (EList<Node>)(EList<?>)diagram.getPersistedChildren();

+    Node newElement = children.get(1);

+    newElement.setType("newType");

+    pushTransaction.commit();

+

+    pushTransaction.getSession().close();

+  }

+

+  /**

+   * Creates a Push Transaction with the given options.

+   * 

+   * @param fileForStoringChanges

+   *          the file that CDOPushTransaction should use to store changes (can be null)

+   * @param reconstructSavePoints

+   *          if CDOSavePoints should be reconstructed when creating the PushTransaction

+   */

+  private CDOPushTransaction createPushTransaction(File fileForStoringChanges, boolean reconstructSavePoints)

+      throws IOException

+  {

+    CDOSession session = openSession();

+    CDOTransaction delegate = session.openTransaction();

+    return new CDOPushTransaction(delegate, fileForStoringChanges, reconstructSavePoints);

+  }

+

+  /**

+   * Ensures that the given diagram has correctly been modified :

+   * <ul>

+   * <li>it should contains 2 Nodes</li>

+   * <li>the Second Node should have a "type" Type and its associated element should be the diagram's container's

+   * container</li>

+   * </ul>

+   */

+  @SuppressWarnings("rawtypes")

+  private void checkDiagramAsCorrectlyBeenModified(CDOView view)

+  {

+    Diagram diagram = getDiagram(view);

+    EList children = diagram.getPersistedChildren();

+

+    assertEquals("Failure when loading changes: New elements have not been created.", 2, children.size());

+

+    Node newElement = (Node)children.get(1);

+    assertEquals("Attribute not correctly set.", "type", newElement.getType());

+    assertEquals("Cross reference not correctly set.", diagram.eContainer().eContainer(), newElement.getElement());

+  }

+

+  /**

+   * Returns the diagram model to use for this test.

+   */

+  private Diagram getDiagram(CDOView view)

+  {

+    CDOResource resource = view.getResource(getResourcePath(MODEL_LOCATION_PATH));

+    F f = (F)resource.getContents().get(0);

+    E e = f.getOwnedEs().get(0);

+    A a = e.getOwnedAs().get(0);

+    D d = a.getOwnedDs().get(0);

+    return (Diagram)d.getData();

+  }

+

+  /**

+   * Creates the test model.

+   */

+  @SuppressWarnings("unchecked")

+  private EObject createModel()

+  {

+    F f = getModel6Factory().createF();

+    E e = getModel6Factory().createE();

+    A a = getModel6Factory().createA();

+    D d = getModel6Factory().createD();

+

+    Node child1 = NotationFactory.eINSTANCE.createNode();

+    child1.setElement(a);

+

+    Diagram diagram = NotationFactory.eINSTANCE.createDiagram();

+    diagram.setMeasurementUnit(MeasurementUnit.PIXEL_LITERAL);

+    diagram.getPersistedChildren().add(child1);

+

+    d.setData(diagram);

+    a.getOwnedDs().add(d);

+    e.getOwnedAs().add(a);

+    f.getOwnedEs().add(e);

+

+    return f;

+  }

+

+  /**

+   * Modifies the model by adding a new children of type Node to the Diagram.

+   */

+  @SuppressWarnings("unchecked")

+  private void addNewChildren(CDOTransaction transaction)

+  {

+    Diagram diagram = getDiagram(transaction);

+    A a = (A)diagram.eContainer().eContainer();

+

+    Node child2 = NotationFactory.eINSTANCE.createNode();

+    child2.setElement(a);

+    child2.setType("type");

+    diagram.getPersistedChildren().add(child2);

+  }

+}

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 a25d312ab0..3c1bc55774 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
@@ -1978,7 +1978,7 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
     int size = in.readInt();
     for (int i = 0; i < size; i++)
     {
-      InternalCDORevision revision = (InternalCDORevision)in.readCDORevision();
+      InternalCDORevision revision = (InternalCDORevision)in.readCDORevision(false);
 
       CDOID oldID = revision.getID();
       if (oldID.isTemporary())