[296450] isSet method returns incorrect value after load

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

2 files changed, 93 insertions, 55 deletions

diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOObjectImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOObjectImpl.java
index 4639e9ce25..261d7350ec 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOObjectImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOObjectImpl.java
@@ -325,18 +325,20 @@ public class CDOObjectImpl extends EStoreEObjectImpl implements InternalCDOObjec
       revision.setResourceID(cdoResource.cdoID());
     }
 
-    EClass eClass = eClass();
-    for (int i = 0; i < eClass.getFeatureCount(); i++)
+    if (cdoSettings != null)
     {
-      EStructuralFeature eFeature = cdoInternalDynamicFeature(i);
-      if (EMFUtil.isPersistent(eFeature))
+      EClass eClass = eClass();
+      for (int i = 0; i < eClass.getFeatureCount(); i++)
       {
-        Object setting = cdoBasicSettings() != null ? cdoSettings()[i] : null;
-        instanceToRevisionFeature(view, this, eFeature, setting);
+        EStructuralFeature eFeature = cdoInternalDynamicFeature(i);
+        if (EMFUtil.isPersistent(eFeature))
+        {
+          instanceToRevisionFeature(view, this, eFeature, cdoSettings[i]);
+        }
       }
-    }
 
-    cdoSettings = null;
+      cdoSettings = null;
+    }
   }
 
   /**
@@ -541,8 +543,8 @@ public class CDOObjectImpl extends EStoreEObjectImpl implements InternalCDOObjec
   @Override
   protected boolean eDynamicIsSet(int dynamicFeatureID, EStructuralFeature eFeature)
   {
-    return dynamicFeatureID < 0 ? eOpenIsSet(eFeature) : eSettingDelegate(eFeature).dynamicIsSet(this, eSettings(),
-        dynamicFeatureID);
+    return dynamicFeatureID < 0 ? eOpenIsSet(eFeature) : !EMFUtil.isPersistent(eFeature) ? eSettingDelegate(eFeature)
+        .dynamicIsSet(this, eSettings(), dynamicFeatureID) : eStore().isSet(this, eFeature);
   }
 
   /**
@@ -772,7 +774,14 @@ public class CDOObjectImpl extends EStoreEObjectImpl implements InternalCDOObjec
     if (cdoSettings == null)
     {
       int size = eClass().getFeatureCount() - eStaticFeatureCount();
-      cdoSettings = size == 0 ? ENO_SETTINGS : new Object[size];
+      if (size == 0)
+      {
+        cdoSettings = ENO_SETTINGS;
+      }
+      else
+      {
+        cdoSettings = new Object[size];
+      }
     }
 
     return cdoSettings;
@@ -1223,7 +1232,8 @@ public class CDOObjectImpl extends EStoreEObjectImpl implements InternalCDOObjec
 
     protected Object getValue(InternalEObject eObject, int dynamicFeatureID)
     {
-      return ((CDOObjectImpl)eObject).cdoSettings()[dynamicFeatureID];
+      Object value = ((CDOObjectImpl)eObject).cdoSettings()[dynamicFeatureID];
+      return value;
     }
 
     protected EList<Object> getValueAsList(InternalEObject eObject, int dynamicFeatureID)
@@ -1241,16 +1251,10 @@ public class CDOObjectImpl extends EStoreEObjectImpl implements InternalCDOObjec
 
     protected Object setValue(InternalEObject eObject, int dynamicFeatureID, Object newValue)
     {
-      Object eSettings[] = ((CDOObjectImpl)eObject).cdoSettings();
-
-      try
-      {
-        return eSettings[dynamicFeatureID];
-      }
-      finally
-      {
-        eSettings[dynamicFeatureID] = newValue;
-      }
+      Object settings[] = ((CDOObjectImpl)eObject).cdoSettings();
+      Object oldSetting = settings[dynamicFeatureID];
+      settings[dynamicFeatureID] = newValue;
+      return oldSetting;
     }
 
     protected int eDynamicFeatureID(InternalEObject eObject, EStructuralFeature feature)
@@ -1364,8 +1368,7 @@ public class CDOObjectImpl extends EStoreEObjectImpl implements InternalCDOObjec
 
     public EStructuralFeature getContainingFeature(InternalEObject eObject)
     {
-      // This should never be called.
-      throw new UnsupportedOperationException();
+      throw new UnsupportedOperationException("Should never be called");
     }
 
     public EObject create(EClass eClass)
@@ -1375,14 +1378,39 @@ public class CDOObjectImpl extends EStoreEObjectImpl implements InternalCDOObjec
 
     public boolean isSet(InternalEObject eObject, EStructuralFeature feature)
     {
+      if (!feature.isUnsettable())
+      {
+        return true;
+      }
+
+      Object[] settings = ((CDOObjectImpl)eObject).cdoBasicSettings();
+      if (settings == null)
+      {
+        return false;
+      }
+
       int dynamicFeatureID = eDynamicFeatureID(eObject, feature);
-      return getValue(eObject, dynamicFeatureID) != null;
+      return settings[dynamicFeatureID] != null;
     }
 
     public void unset(InternalEObject eObject, EStructuralFeature feature)
     {
+      Object[] settings = ((CDOObjectImpl)eObject).cdoBasicSettings();
+      if (settings == null)
+      {
+        // Is already unset
+        return;
+      }
+
       int dynamicFeatureID = eDynamicFeatureID(eObject, feature);
-      setValue(eObject, dynamicFeatureID, null);
+      if (feature.isUnsettable())
+      {
+        settings[dynamicFeatureID] = null;
+      }
+      else
+      {
+        settings[dynamicFeatureID] = feature.getDefaultValue();
+      }
     }
   }
 
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java
index 59bde6a455..5eefb6c953 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java
@@ -19,6 +19,7 @@ import org.eclipse.emf.cdo.common.model.CDOModelUtil;
 import org.eclipse.emf.cdo.common.model.CDOType;
 import org.eclipse.emf.cdo.common.revision.CDOList;
 import org.eclipse.emf.cdo.common.revision.CDORevision;
+import org.eclipse.emf.cdo.common.revision.CDORevisionData;
 import org.eclipse.emf.cdo.common.revision.CDORevisionUtil;
 import org.eclipse.emf.cdo.common.revision.delta.CDOFeatureDelta;
 import org.eclipse.emf.cdo.eresource.CDOResource;
@@ -37,7 +38,6 @@ import org.eclipse.emf.cdo.view.CDORevisionPrefetchingPolicy;
 import org.eclipse.emf.internal.cdo.bundle.OM;
 import org.eclipse.emf.internal.cdo.util.FSMUtil;
 
-import org.eclipse.net4j.util.ImplementationError;
 import org.eclipse.net4j.util.om.trace.ContextTracer;
 
 import org.eclipse.emf.ecore.EClass;
@@ -171,18 +171,30 @@ public final class CDOStore implements EStore
     view.getFeatureAnalyzer().preTraverseFeature(cdoObject, feature, index);
     InternalCDORevision revision = getRevisionForReading(cdoObject);
 
-    Object value = revision.basicGet(feature, index);
+    Object value = revision.getValue(feature, index);
     value = convertToEMF(eObject, revision, feature, index, value);
 
     view.getFeatureAnalyzer().postTraverseFeature(cdoObject, feature, index, value);
     return value;
   }
 
-  @Deprecated
   public boolean isSet(InternalEObject eObject, EStructuralFeature feature)
   {
-    // Should not be called
-    throw new ImplementationError();
+    if (!feature.isUnsettable())
+    {
+      return true;
+    }
+
+    InternalCDOObject cdoObject = getCDOObject(eObject);
+    if (TRACER.isEnabled())
+    {
+      TRACER.format("isSet({0}, {1})", cdoObject, feature); //$NON-NLS-1$
+    }
+
+    InternalCDORevision revision = getRevisionForReading(cdoObject);
+
+    Object value = revision.getValue(feature, NO_INDEX);
+    return value != null;
   }
 
   public int size(InternalEObject eObject, EStructuralFeature feature)
@@ -333,12 +345,12 @@ public final class CDOStore implements EStore
     // TODO Clarify feature maps
     if (feature instanceof EReference)
     {
-      Object oldValue = revision.basicGet(feature, index);
+      Object oldValue = revision.getValue(feature, index);
       oldValue = resolveProxy(revision, feature, index, oldValue);
       value = cdoObject.cdoView().convertObjectToID(value, true);
     }
 
-    Object oldValue = revision.basicSet(feature, index, value);
+    Object oldValue = revision.setValue(feature, index, value);
     oldValue = convertToEMF(eObject, revision, feature, index, oldValue);
     return oldValue;
   }
@@ -354,8 +366,15 @@ public final class CDOStore implements EStore
     CDOFeatureDelta delta = new CDOUnsetFeatureDeltaImpl(feature);
     InternalCDORevision revision = getRevisionForWriting(cdoObject, delta);
 
-    // TODO Handle containment remove!!!
-    revision.set(feature, 0, null);
+    if (feature.isUnsettable())
+    {
+      revision.unset(feature);
+    }
+    else
+    {
+      Object defaultValue = convertToCDO(cdoObject, feature, feature.getDefaultValue());
+      revision.set(feature, NO_INDEX, defaultValue);
+    }
   }
 
   public void add(InternalEObject eObject, EStructuralFeature feature, int index, Object value)
@@ -449,26 +468,18 @@ public final class CDOStore implements EStore
    */
   public Object convertToCDO(InternalCDOObject object, EStructuralFeature feature, Object value)
   {
+    if (value == EStoreEObjectImpl.NIL)
+    {
+      return CDORevisionData.NIL;
+    }
+
     if (value != null)
     {
-      if (value == EStoreEObjectImpl.NIL)
-      {
-        value = InternalCDORevision.NIL;
-      }
-      else if (feature instanceof EReference)
+      if (feature instanceof EReference)
       {
         // The EReference condition should be in the CDOType.convertToCDO. Since common package do not have access to
         // InternalCDOView I kept it here.
         value = view.convertObjectToID(value, true);
-        // TTT if (value instanceof InternalEObject)
-        // {
-        // CDOIDDangling id = view.convertDanglingObjectToID(object, feature, (InternalEObject)value);
-        // if (id != null)
-        // {
-        // // TODO assign at once from convertDanglingObjectToID() if dangling IDs are fully implemented
-        // value = id;
-        // }
-        // }
       }
       else if (FeatureMapUtil.isFeatureMap(feature))
       {
@@ -500,13 +511,13 @@ public final class CDOStore implements EStore
   public Object convertToEMF(EObject eObject, InternalCDORevision revision, EStructuralFeature feature, int index,
       Object value)
   {
-    if (value != null)
+    if (value == CDORevisionData.NIL)
     {
-      if (value == InternalCDORevision.NIL)
-      {
-        return EStoreEObjectImpl.NIL;
-      }
+      return EStoreEObjectImpl.NIL;
+    }
 
+    if (value != null)
+    {
       if (feature.isMany() && index != EStore.NO_INDEX)
       {
         value = resolveProxy(revision, feature, index, value);
@@ -525,7 +536,6 @@ public final class CDOStore implements EStore
         }
       }
 
-      // TODO Clarify feature maps
       if (feature instanceof EReference)
       {
         value = convertIdToObject(view, eObject, feature, index, value);