Author: adrianc
Date: Tue Apr 23 08:23:57 2013
New Revision: 1470845
URL: http://svn.apache.org/r1470845
Log:
Reverting revision 1470712. The improved immutability is causing problems in
some places.
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=1470845&r1=1470844&r2=1470845&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java
(original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java Tue
Apr 23 08:23:57 2013
@@ -146,15 +146,8 @@ 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");
}
@@ -169,7 +162,6 @@ 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");
}
@@ -187,7 +179,6 @@ 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");
}
@@ -208,7 +199,6 @@ 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");
@@ -223,7 +213,6 @@ public class GenericEntity implements Ma
}
public void reset() {
- assertIsMutable();
// from GenericEntity
this.delegatorName = null;
this.internalDelegator = null;
@@ -238,7 +227,6 @@ 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);
}
@@ -247,7 +235,8 @@ 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);
}
- this.fields = new HashMap<String, Object>(newValue.fields);
+ // FIXME: This is dangerous - two instances sharing a common field Map
is a bad idea.
+ this.fields = newValue.fields;
this.setDelegator(newValue.getDelegator());
this.generateHashCode = newValue.generateHashCode;
this.cachedHashCode = newValue.cachedHashCode;
@@ -260,12 +249,10 @@ 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;
}
@@ -275,10 +262,7 @@ public class GenericEntity implements Ma
}
public void setImmutable() {
- if (this.mutable) {
- this.mutable = false;
- this.fields = Collections.unmodifiableMap(this.fields);
- }
+ this.mutable = false;
}
/**
@@ -292,7 +276,6 @@ public class GenericEntity implements Ma
* @param isFromEntitySync The isFromEntitySync to set.
*/
public void setIsFromEntitySync(boolean isFromEntitySync) {
- assertIsMutable();
this.isFromEntitySync = isFromEntitySync;
}
@@ -327,7 +310,6 @@ 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;
@@ -406,7 +388,11 @@ 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) {
- assertIsMutable();
+ 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.");
+ }
+
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());
@@ -460,16 +446,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) {
- 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=1470845&r1=1470844&r2=1470845&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
Tue Apr 23 08:23:57 2013
@@ -124,18 +124,12 @@ 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");
