svn commit: r1471283 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java

[adrianc]

Author: adrianc
Date: Wed Apr 24 08:07:36 2013
New Revision: 1471283

URL: http://svn.apache.org/r1471283
Log:
Let's try this again...

Make sure an immutable GenericEntity really is immutable. Also fixed deserialization of the new Observable object.


Modified:
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java?rev=1471283&r1=1471282&r2=1471283&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java Wed Apr 24 08:07:36 2013
@@ -76,7 +76,8 @@ public class GenericEntity implements Ma
     public static final GenericEntity NULL_ENTITY = new NullGenericEntity();
     public static final NullField NULL_FIELD = new NullField();
 
-    private Observable observable = new Observable();
+    // Do not restore observers during deserialization. Instead, client code must add observers.
+    private transient Observable observable = new Observable();
 
     /** Name of the GenericDelegator, used to re-get the GenericDelegator when deserialized */
     protected String delegatorName = null;
@@ -146,13 +147,29 @@ public class GenericEntity implements Ma
         return newEntity;
     }
 
+    protected void assertIsMutable() {
+        if (!this.mutable) {
+            Debug.logError(new IllegalStateException("This object has been flagged as immutable (unchangeable), probably because it came from an Entity Engine cache. Cannot modify an immutable entity object."), module);
+            throw new IllegalStateException("This object has been flagged as immutable (unchangeable), probably because it came from an Entity Engine cache. Cannot modify an immutable entity object.");
+        }
+    }
+
+    private Observable getObservable() {
+        if (this.observable == null) {
+            this.observable = new Observable();
+        }
+        return this.observable;
+    }
+
     /** Creates new GenericEntity */
     protected void init(ModelEntity modelEntity) {
+        assertIsMutable();
         if (modelEntity == null) {
             throw new IllegalArgumentException("Cannot create a GenericEntity with a null modelEntity parameter");
         }
         this.modelEntity = modelEntity;
         this.entityName = modelEntity.getEntityName();
+        this.observable = new Observable();
 
         // check some things
         if (this.entityName == null) {
@@ -162,6 +179,7 @@ public class GenericEntity implements Ma
 
     /** Creates new GenericEntity from existing Map */
     protected void init(Delegator delegator, ModelEntity modelEntity, Map<String, ? extends Object> fields) {
+        assertIsMutable();
         if (modelEntity == null) {
             throw new IllegalArgumentException("Cannot create a GenericEntity with a null modelEntity parameter");
         }
@@ -169,6 +187,7 @@ public class GenericEntity implements Ma
         this.entityName = modelEntity.getEntityName();
         this.delegatorName = delegator.getDelegatorName();
         this.internalDelegator = delegator;
+        this.observable = new Observable();
         setFields(fields);
 
         // check some things
@@ -179,6 +198,7 @@ public class GenericEntity implements Ma
 
     /** Creates new GenericEntity from existing Map */
     protected void init(Delegator delegator, ModelEntity modelEntity, Object singlePkValue) {
+        assertIsMutable();
         if (modelEntity == null) {
             throw new IllegalArgumentException("Cannot create a GenericEntity with a null modelEntity parameter");
         }
@@ -189,6 +209,7 @@ public class GenericEntity implements Ma
         this.entityName = modelEntity.getEntityName();
         this.delegatorName = delegator.getDelegatorName();
         this.internalDelegator = delegator;
+        this.observable = new Observable();
         set(modelEntity.getOnlyPk().getName(), singlePkValue);
 
         // check some things
@@ -199,6 +220,7 @@ public class GenericEntity implements Ma
 
     /** Copy Constructor: Creates new GenericEntity from existing GenericEntity */
     protected void init(GenericEntity value) {
+        assertIsMutable();
         // check some things
         if (value.entityName == null) {
             throw new IllegalArgumentException("Cannot create a GenericEntity with a null entityName in the modelEntity parameter");
@@ -213,6 +235,7 @@ public class GenericEntity implements Ma
     }
 
     public void reset() {
+        assertIsMutable();
         // from GenericEntity
         this.delegatorName = null;
         this.internalDelegator = null;
@@ -224,9 +247,11 @@ public class GenericEntity implements Ma
         this.cachedHashCode = 0;
         this.mutable = true;
         this.isFromEntitySync = false;
+        this.observable = new Observable();
     }
 
     public void refreshFromValue(GenericEntity newValue) throws GenericEntityException {
+        assertIsMutable();
         if (newValue == null) {
             throw new GenericEntityException("Could not refresh value, new value not found for: " + this);
         }
@@ -235,8 +260,7 @@ public class GenericEntity implements Ma
         if (!thisPK.equals(newPK)) {
             throw new GenericEntityException("Could not refresh value, new value did not have the same primary key; this PK=" + thisPK + ", new value PK=" + newPK);
         }
-        // FIXME: This is dangerous - two instances sharing a common field Map is a bad idea.
-        this.fields = newValue.fields;
+        this.fields = new HashMap<String, Object>(newValue.fields);
         this.setDelegator(newValue.getDelegator());
         this.generateHashCode = newValue.generateHashCode;
         this.cachedHashCode = newValue.cachedHashCode;
@@ -249,10 +273,12 @@ public class GenericEntity implements Ma
     }
 
     public void synchronizedWithDatasource() {
+        assertIsMutable();
         this.modified = false;
     }
 
     public void removedFromDatasource() {
+        assertIsMutable();
         // seems kind of minimal, but should do for now...
         this.modified = true;
     }
@@ -262,7 +288,10 @@ public class GenericEntity implements Ma
     }
 
     public void setImmutable() {
-        this.mutable = false;
+        if (this.mutable) {
+            this.mutable = false;
+            this.fields = Collections.unmodifiableMap(this.fields);
+        }
     }
 
     /**
@@ -276,6 +305,7 @@ public class GenericEntity implements Ma
      * @param isFromEntitySync The isFromEntitySync to set.
      */
     public void setIsFromEntitySync(boolean isFromEntitySync) {
+        assertIsMutable();
         this.isFromEntitySync = isFromEntitySync;
     }
 
@@ -310,6 +340,7 @@ public class GenericEntity implements Ma
 
     /** Set the GenericDelegator instance that created this value object and that is responsible for it. */
     public void setDelegator(Delegator internalDelegator) {
+        assertIsMutable();
         if (internalDelegator == null) return;
         this.delegatorName = internalDelegator.getDelegatorName();
         this.internalDelegator = internalDelegator;
@@ -388,11 +419,7 @@ public class GenericEntity implements Ma
      * @param setIfNull Specifies whether or not to set the value if it is null
      */
     public Object set(String name, Object value, boolean setIfNull) {
-        if (!this.mutable) {
-            // comment this out to disable the mutable check
-            throw new IllegalStateException("This object has been flagged as immutable (unchangeable), probably because it came from an Entity Engine cache. Cannot set a value in an immutable entity object.");
-        }
-
+        assertIsMutable();
         ModelField modelField = getModelEntity().getField(name);
         if (modelField == null) {
             throw new IllegalArgumentException("[GenericEntity.set] \"" + name + "\" is not a field of " + entityName + ", must be one of: " + getModelEntity().fieldNameString());
@@ -446,9 +473,12 @@ public class GenericEntity implements Ma
     }
 
     public void dangerousSetNoCheckButFast(ModelField modelField, Object value) {
+        assertIsMutable();
         if (modelField == null) throw new IllegalArgumentException("Cannot set field with a null modelField");
         generateHashCode = true;
         this.fields.put(modelField.getName(), value);
+        this.setChanged();
+        this.notifyObservers(modelField.getName());
     }
 
     public Object dangerousGetNoCheckButFast(ModelField modelField) {
@@ -1453,35 +1483,35 @@ public class GenericEntity implements Ma
     }
 
     public void addObserver(Observer observer) {
-        observable.addObserver(observer);
+        getObservable().addObserver(observer);
     }
 
     public void clearChanged() {
-        observable.clearChanged();
+        getObservable().clearChanged();
     }
 
     public void deleteObserver(Observer observer) {
-        observable.deleteObserver(observer);
+        getObservable().deleteObserver(observer);
     }
 
     public void deleteObservers() {
-        observable.deleteObservers();
+        getObservable().deleteObservers();
     }
 
     public boolean hasChanged() {
-        return observable.hasChanged();
+        return getObservable().hasChanged();
     }
 
     public void notifyObservers() {
-        observable.notifyObservers();
+        getObservable().notifyObservers();
     }
 
     public void notifyObservers(Object arg) {
-        observable.notifyObservers(arg);
+        getObservable().notifyObservers(arg);
     }
 
     public void setChanged() {
-        observable.setChanged();
+        getObservable().setChanged();
     }
 
     public static interface NULL {