Author: adrianc
Date: Mon Apr 22 20:46:36 2013
New Revision: 1470712
URL: http://svn.apache.org/r1470712
Log:
Make sure an immutable GenericEntity really is immutable.
Modified:
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.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=1470712&r1=1470711&r2=1470712&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java
(original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java Mon
Apr 22 20:46:36 2013
@@ -144,8 +144,15 @@ public class GenericEntity implements Ma
return newEntity;
}
+ protected void assertIsMutable() {
+ if (!this.mutable) {
+ 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.");
+ }
+ }
+
/** Creates new GenericEntity */
protected void init(ModelEntity modelEntity) {
+ assertIsMutable();
if (modelEntity == null) {
throw new IllegalArgumentException("Cannot create a GenericEntity
with a null modelEntity parameter");
}
@@ -160,6 +167,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");
}
@@ -177,6 +185,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");
}
@@ -197,6 +206,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");
@@ -211,6 +221,7 @@ public class GenericEntity implements Ma
}
public void reset() {
+ assertIsMutable();
// from GenericEntity
this.delegatorName = null;
this.internalDelegator = null;
@@ -225,6 +236,7 @@ public class GenericEntity implements Ma
}
public void refreshFromValue(GenericEntity newValue) throws
GenericEntityException {
+ assertIsMutable();
if (newValue == null) {
throw new GenericEntityException("Could not refresh value, new
value not found for: " + this);
}
@@ -233,8 +245,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;
@@ -247,10 +258,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;
}
@@ -260,7 +273,10 @@ public class GenericEntity implements Ma
}
public void setImmutable() {
- this.mutable = false;
+ if (this.mutable) {
+ this.mutable = false;
+ this.fields = Collections.unmodifiableMap(this.fields);
+ }
}
/**
@@ -274,6 +290,7 @@ public class GenericEntity implements Ma
* @param isFromEntitySync The isFromEntitySync to set.
*/
public void setIsFromEntitySync(boolean isFromEntitySync) {
+ assertIsMutable();
this.isFromEntitySync = isFromEntitySync;
}
@@ -308,6 +325,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;
@@ -386,11 +404,7 @@ public class GenericEntity implements Ma
* @param setIfNull Specifies whether or not to set the value if it is null
*/
public synchronized 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());
@@ -444,12 +458,16 @@ 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) {
+ assertIsMutable();
if (modelField == null) throw new IllegalArgumentException("Cannot get
field with a null modelField");
return this.fields.get(modelField.getName());
}
Modified:
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java?rev=1470712&r1=1470711&r2=1470712&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java
(original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java
Mon Apr 22 20:46:36 2013
@@ -122,12 +122,18 @@ public class EntityTestSuite extends Ent
// Test primary key cache
GenericValue testValue = delegator.findOne("TestingType", true,
"testingTypeId", "TEST-2");
assertEquals("Retrieved from cache value has the correct description",
"Testing Type #2", testValue.getString("description"));
+ // Test immutable
try {
testValue.put("description", "New Testing Type #2");
testValue.store();
fail("Modified an immutable GenericValue");
} catch (IllegalStateException e) {
}
+ try {
+ testValue.remove("description");
+ fail("Modified an immutable GenericValue");
+ } catch (UnsupportedOperationException e) {
+ }
// Test entity condition cache
/* Commenting this out for now because the tests fail due to flaws in
the EntityListCache implementation.
EntityCondition testCondition =
EntityCondition.makeCondition("description", EntityOperator.EQUALS, "Testing
Type #2");
