[335675] CDOClassInfoImpl broken for EClasses with transient features

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

3 files changed, 198 insertions, 9 deletions

diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/model/CDOClassInfoImpl.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/model/CDOClassInfoImpl.java
index 8cfcd8561b..8995c32513 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/model/CDOClassInfoImpl.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/model/CDOClassInfoImpl.java
@@ -16,10 +16,12 @@ import org.eclipse.emf.cdo.common.model.EMFUtil;
 
 import org.eclipse.emf.common.notify.Notifier;
 import org.eclipse.emf.common.notify.impl.AdapterImpl;
+import org.eclipse.emf.common.util.EList;
 import org.eclipse.emf.ecore.EClass;
 import org.eclipse.emf.ecore.EStructuralFeature;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -27,6 +29,8 @@ import java.util.List;
  */
 public class CDOClassInfoImpl extends AdapterImpl implements CDOClassInfo
 {
+  private static final int NOT_MAPPED = -1;
+
   private EStructuralFeature[] allPersistentFeatures;
 
   private int[] featureIDMappings;
@@ -88,25 +92,31 @@ public class CDOClassInfoImpl extends AdapterImpl implements CDOClassInfo
 
   public int getFeatureIndex(int featureID)
   {
-    return featureIDMappings[featureID];
+    int index = featureIDMappings[featureID];
+    if (index == NOT_MAPPED)
+    {
+      throw new IllegalArgumentException("Feature not mapped: " + getEClass().getEStructuralFeature(featureID)); //$NON-NLS-1$
+    }
+
+    return index;
   }
 
   private void init(EClass eClass)
   {
-    int maxID = 0;
-    List<EStructuralFeature> features = new ArrayList<EStructuralFeature>();
-    for (EStructuralFeature feature : eClass.getEAllStructuralFeatures())
+    List<EStructuralFeature> persistentFeatures = new ArrayList<EStructuralFeature>();
+    EList<EStructuralFeature> allFeatures = eClass.getEAllStructuralFeatures();
+    for (EStructuralFeature feature : allFeatures)
     {
       if (EMFUtil.isPersistent(feature))
       {
-        features.add(feature);
-        int featureID = eClass.getFeatureID(feature);
-        maxID = Math.max(maxID, featureID);
+        persistentFeatures.add(feature);
       }
     }
 
-    allPersistentFeatures = features.toArray(new EStructuralFeature[features.size()]);
-    featureIDMappings = new int[maxID + 1];
+    allPersistentFeatures = persistentFeatures.toArray(new EStructuralFeature[persistentFeatures.size()]);
+    featureIDMappings = new int[allFeatures.size()];
+    Arrays.fill(featureIDMappings, NOT_MAPPED);
+
     for (int i = 0; i < allPersistentFeatures.length; i++)
     {
       EStructuralFeature feature = allPersistentFeatures[i];
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 cbd0aca2a7..e4603d6051 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
@@ -194,6 +194,7 @@ public abstract class AllConfigs extends ConfigTestSuite
     testClasses.add(Bugzilla_333299_Test.class);
     testClasses.add(Bugzilla_333950_Test.class);
     testClasses.add(Bugzilla_334608_Test.class);
+    testClasses.add(Bugzilla_335675_Test.class);
 
     // TODO testClasses.add(NonCDOResourceTest.class);
     // TODO testClasses.add(GeneratedEcoreTest.class);
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_335675_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_335675_Test.java
new file mode 100644
index 0000000000..f6fc94ebcb
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_335675_Test.java
@@ -0,0 +1,178 @@
+/**
+ * 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:
+ *    Caspar De Groot - initial API and implementation
+ */
+package org.eclipse.emf.cdo.tests.bugzilla;
+
+import org.eclipse.emf.cdo.CDOObject;
+import org.eclipse.emf.cdo.eresource.CDOResource;
+import org.eclipse.emf.cdo.session.CDOSession;
+import org.eclipse.emf.cdo.spi.common.revision.BaseCDORevision;
+import org.eclipse.emf.cdo.tests.AbstractCDOTest;
+import org.eclipse.emf.cdo.tests.model1.Model1Factory;
+import org.eclipse.emf.cdo.tests.model1.Model1Package;
+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.VAT;
+import org.eclipse.emf.cdo.transaction.CDOTransaction;
+import org.eclipse.emf.cdo.util.CDOUtil;
+import org.eclipse.emf.cdo.util.CommitException;
+
+import org.eclipse.emf.ecore.EAttribute;
+import org.eclipse.emf.ecore.EClass;
+import org.eclipse.emf.ecore.EDataType;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EPackage;
+import org.eclipse.emf.ecore.EcoreFactory;
+import org.eclipse.emf.ecore.EcorePackage;
+import org.eclipse.emf.ecore.util.EcoreUtil;
+
+/**
+ * @author Caspar De Groot
+ */
+public class Bugzilla_335675_Test extends AbstractCDOTest
+{
+  public void test0() throws Exception
+  {
+    EPackage pkg = null;
+    EAttribute attr1 = null;
+    EAttribute attr2 = null;
+    EAttribute attr3 = null;
+
+    final int TRANSIENT_VALUE = 2;
+
+    EClass classA = null;
+    {
+      pkg = EcoreFactory.eINSTANCE.createEPackage();
+      pkg.setNsURI("http://test.com/custom");
+      pkg.setName("test");
+      pkg.setNsPrefix("t");
+
+      classA = EcoreFactory.eINSTANCE.createEClass();
+      classA.setName("A");
+
+      attr1 = createAttribute("attr1", EcorePackage.eINSTANCE.getEInt(), false);
+      attr2 = createAttribute("attr2", EcorePackage.eINSTANCE.getEInt(), true);
+      attr3 = createAttribute("attr3", EcorePackage.eINSTANCE.getEInt(), false);
+
+      classA.getEStructuralFeatures().add(attr1);
+      classA.getEStructuralFeatures().add(attr2);
+      classA.getEStructuralFeatures().add(attr3);
+
+      pkg.getEClassifiers().add(classA);
+    }
+
+    {
+      CDOSession openSession = openSession();
+      CDOTransaction transaction = openSession.openTransaction();
+      CDOResource resource = transaction.createResource("test");
+
+      classA = (EClass)pkg.getEClassifier("A");
+      EObject instanceA = EcoreUtil.create(classA);
+      attr1 = (EAttribute)classA.getEStructuralFeature("attr1");
+      attr2 = (EAttribute)classA.getEStructuralFeature("attr2");
+      attr3 = (EAttribute)classA.getEStructuralFeature("attr3");
+      instanceA.eSet(attr1, 1);
+      instanceA.eSet(attr2, TRANSIENT_VALUE);
+      instanceA.eSet(attr3, 3);
+
+      resource.getContents().add(instanceA);
+      transaction.commit();
+      openSession.close();
+    }
+
+    {
+      CDOSession session = openSession();
+      CDOTransaction transaction = session.openTransaction();
+      CDOResource resource = transaction.getResource("test");
+      CDOObject cdoObject = CDOUtil.getCDOObject(resource.getContents().get(0));
+      classA = cdoObject.eClass();
+      attr1 = (EAttribute)classA.getEStructuralFeature("attr1");
+      attr2 = (EAttribute)classA.getEStructuralFeature("attr2");
+      attr3 = (EAttribute)classA.getEStructuralFeature("attr3");
+
+      BaseCDORevision rev = (BaseCDORevision)cdoObject.cdoRevision();
+      // int attr1Val = (Integer)rev.getValue(attr1);
+      // int attr3Val = (Integer)rev.getValue(attr3);
+      try
+      {
+        int attr2Val = (Integer)rev.getValue(attr2);
+        fail("Should have thrown an exception, but fetched value " + attr2Val);
+      }
+      catch (IllegalArgumentException ex)
+      {
+        // Good
+      }
+
+      session.close();
+    }
+  }
+
+  public void test1() throws CommitException
+  {
+    EAttribute nameAttr = Model1Package.eINSTANCE.getProduct1_Name();
+    assertEquals(false, nameAttr.isTransient());
+
+    EAttribute descriptionAttr = Model1Package.eINSTANCE.getProduct1_Description();
+    assertEquals(true, descriptionAttr.isTransient());
+
+    {
+      CDOSession openSession = openSession();
+      CDOTransaction transaction = openSession.openTransaction();
+      CDOResource resource = transaction.createResource("test");
+
+      Product1 product = Model1Factory.eINSTANCE.createProduct1();
+      product.setName("name");
+      product.setDescription("description");
+      product.setVat(VAT.VAT7);
+      resource.getContents().add(product);
+
+      PurchaseOrder order = Model1Factory.eINSTANCE.createPurchaseOrder();
+      OrderDetail detail = Model1Factory.eINSTANCE.createOrderDetail();
+      order.getOrderDetails().add(detail);
+      resource.getContents().add(order);
+
+      detail.setProduct(product);
+
+      transaction.commit();
+      openSession.close();
+    }
+
+    {
+      CDOSession session = openSession();
+      CDOTransaction transaction = session.openTransaction();
+      CDOResource resource = transaction.getResource("test");
+      CDOObject cdoObject = CDOUtil.getCDOObject(resource.getContents().get(0));
+
+      BaseCDORevision rev = (BaseCDORevision)cdoObject.cdoRevision();
+      // String name = (String)rev.getValue(nameAttr);
+      try
+      {
+        String desc = (String)rev.getValue(descriptionAttr);
+        fail("Should have thrown an exception, but fetched value: " + desc);
+      }
+      catch (Exception ex)
+      {
+        // Good
+      }
+
+      session.close();
+    }
+  }
+
+  private EAttribute createAttribute(String name, EDataType type, boolean tranzient)
+  {
+    EAttribute attr = EcoreFactory.eINSTANCE.createEAttribute();
+    attr.setName(name);
+    attr.setEType(type);
+    attr.setTransient(tranzient);
+    return attr;
+  }
+}