You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ad...@apache.org on 2013/05/25 11:23:46 UTC
svn commit: r1486304 - in /ofbiz/branches/release12.04: ./
framework/entity/src/org/ofbiz/entity/
framework/entity/src/org/ofbiz/entity/test/
framework/widget/src/org/ofbiz/widget/form/
Author: adrianc
Date: Sat May 25 09:23:46 2013
New Revision: 1486304
URL: http://svn.apache.org/r1486304
Log:
Backported GenericEntity immutable fixes from the trunk.
Modified:
ofbiz/branches/release12.04/ (props changed)
ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericEntity.java
ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericValue.java
ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java
ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelForm.java
ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java
Propchange: ofbiz/branches/release12.04/
------------------------------------------------------------------------------
Merged /ofbiz/trunk:r1471283,1471687,1480407,1481287
Modified: ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericDelegator.java?rev=1486304&r1=1486303&r2=1486304&view=diff
==============================================================================
--- ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericDelegator.java (original)
+++ ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericDelegator.java Sat May 25 09:23:46 2013
@@ -1011,7 +1011,7 @@ public class GenericDelegator implements
GenericValue removedEntity = null;
if (testMode) {
- removedEntity = this.findOne(primaryKey.entityName, primaryKey, false);
+ removedEntity = this.findOne(primaryKey.getEntityName(), primaryKey, false);
}
int num = helper.removeByPrimaryKey(primaryKey);
this.saveEntitySyncRemoveInfo(primaryKey);
@@ -1083,6 +1083,8 @@ public class GenericDelegator implements
}
int num = helper.removeByPrimaryKey(value.getPrimaryKey());
+ // Need to call removedFromDatasource() here because the helper calls removedFromDatasource() on the PK instead of the GenericEntity.
+ value.removedFromDatasource();
if (testMode) {
if (removedValue != null) {
@@ -1345,7 +1347,7 @@ public class GenericDelegator implements
GenericValue updatedEntity = null;
if (testMode) {
- updatedEntity = this.findOne(value.entityName, value.getPrimaryKey(), false);
+ updatedEntity = this.findOne(value.getEntityName(), value.getPrimaryKey(), false);
}
int retVal = helper.store(value);
Modified: ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericEntity.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericEntity.java?rev=1486304&r1=1486303&r2=1486304&view=diff
==============================================================================
--- ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericEntity.java (original)
+++ ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericEntity.java Sat May 25 09:23:46 2013
@@ -26,6 +26,7 @@ import java.sql.Blob;
import java.sql.SQLException;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
@@ -76,10 +77,10 @@ public class GenericEntity extends Obser
public static final NullField NULL_FIELD = new NullField();
/** Name of the GenericDelegator, used to re-get the GenericDelegator when deserialized */
- protected String delegatorName = null;
+ private String delegatorName = null;
/** Reference to an instance of GenericDelegator used to do some basic operations on this entity value. If null various methods in this class will fail. This is automatically set by the GenericDelegator for all GenericValue objects instantiated through it. You may set this manually for objects you instantiate manually, but it is optional. */
- protected transient Delegator internalDelegator = null;
+ private transient Delegator internalDelegator = null;
/** Contains the fields for this entity. Note that this should always be a
* HashMap to allow for two things: non-synchronized reads (synchronized
@@ -88,24 +89,22 @@ public class GenericEntity extends Obser
* between desiring to set a value to null and desiring to not modify the
* current value on an update.
*/
- protected Map<String, Object> fields = FastMap.newInstance();
+ private Map<String, Object> fields = new HashMap<String, Object>();
/** Contains the entityName of this entity, necessary for efficiency when creating EJBs */
- protected String entityName = null;
+ private String entityName = null;
/** Contains the ModelEntity instance that represents the definition of this entity, not to be serialized */
- protected transient ModelEntity modelEntity = null;
+ private transient ModelEntity modelEntity = null;
- /** Denotes whether or not this entity has been modified, or is known to be out of sync with the persistent record */
- protected boolean modified = false;
- protected boolean generateHashCode = true;
- protected int cachedHashCode = 0;
+ private boolean generateHashCode = true;
+ private int cachedHashCode = 0;
/** Used to specify whether or not this representation of the entity can be changed; generally cleared when this object comes from a cache */
- protected boolean mutable = true;
+ private boolean mutable = true;
/** This is an internal field used to specify that a value has come from a sync process and that the auto-stamps should not be over-written */
- protected boolean isFromEntitySync = false;
+ private boolean isFromEntitySync = false;
/** Creates new GenericEntity - Should never be used, prefer the other options. */
protected GenericEntity() { }
@@ -143,8 +142,16 @@ public class GenericEntity extends Obser
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.");
+ }
+ }
+
/** Creates new GenericEntity */
protected void init(ModelEntity modelEntity) {
+ assertIsMutable();
if (modelEntity == null) {
throw new IllegalArgumentException("Cannot create a GenericEntity with a null modelEntity parameter");
}
@@ -159,6 +166,7 @@ public class GenericEntity extends Obser
/** 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");
}
@@ -176,6 +184,7 @@ public class GenericEntity extends Obser
/** 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");
}
@@ -196,6 +205,7 @@ public class GenericEntity extends Obser
/** 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");
@@ -209,13 +219,13 @@ public class GenericEntity extends Obser
}
public void reset() {
+ assertIsMutable();
// from GenericEntity
this.delegatorName = null;
this.internalDelegator = null;
this.fields = FastMap.newInstance();
this.entityName = null;
this.modelEntity = null;
- this.modified = false;
this.generateHashCode = true;
this.cachedHashCode = 0;
this.mutable = true;
@@ -223,6 +233,7 @@ public class GenericEntity extends Obser
}
public void refreshFromValue(GenericEntity newValue) throws GenericEntityException {
+ assertIsMutable();
if (newValue == null) {
throw new GenericEntityException("Could not refresh value, new value not found for: " + this);
}
@@ -231,24 +242,40 @@ public class GenericEntity extends Obser
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 = newValue.fields;
+ this.fields = new HashMap<String, Object>(newValue.fields);
this.setDelegator(newValue.getDelegator());
this.generateHashCode = newValue.generateHashCode;
this.cachedHashCode = newValue.cachedHashCode;
- this.modified = false;
}
+ /**
+ *
+ * @deprecated Use hasChanged()
+ */
public boolean isModified() {
- return this.modified;
+ return this.hasChanged();
}
+ /**
+ * Flags this object as being synchronized with the data source.
+ * The entity engine will call this method immediately after
+ * populating this object with data from the data source.
+ */
public void synchronizedWithDatasource() {
- this.modified = false;
+ assertIsMutable();
+ this.clearChanged();
}
+ /**
+ * Flags this object as being removed from the data source.
+ * The entity engine will call this method immediately after
+ * removing this value from the data source. Once this method is
+ * called, the object is immutable.
+ */
public void removedFromDatasource() {
- // seems kind of minimal, but should do for now...
- this.modified = true;
+ assertIsMutable();
+ this.hasChanged();
+ this.setImmutable();
}
public boolean isMutable() {
@@ -256,7 +283,10 @@ public class GenericEntity extends Obser
}
public void setImmutable() {
- this.mutable = false;
+ if (this.mutable) {
+ this.mutable = false;
+ this.fields = Collections.unmodifiableMap(this.fields);
+ }
}
/**
@@ -270,6 +300,7 @@ public class GenericEntity extends Obser
* @param isFromEntitySync The isFromEntitySync to set.
*/
public void setIsFromEntitySync(boolean isFromEntitySync) {
+ assertIsMutable();
this.isFromEntitySync = isFromEntitySync;
}
@@ -304,6 +335,7 @@ public class GenericEntity extends Obser
/** 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;
@@ -382,11 +414,7 @@ public class GenericEntity extends Obser
* @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());
@@ -430,7 +458,6 @@ public class GenericEntity extends Obser
Object old = fields.put(name, value);
generateHashCode = true;
- modified = true;
this.setChanged();
this.notifyObservers(name);
return old;
@@ -440,9 +467,12 @@ public class GenericEntity extends Obser
}
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) {
@@ -968,9 +998,7 @@ public class GenericEntity extends Obser
* @return java.util.Map
*/
public Map<String, Object> getAllFields() {
- Map<String, Object> newMap = FastMap.newInstance();
- newMap.putAll(this.fields);
- return newMap;
+ return new HashMap<String, Object>(this.fields);
}
/** Used by clients to specify exactly the fields they are interested in
@@ -1454,7 +1482,9 @@ public class GenericEntity extends Obser
}
public static class NullGenericEntity extends GenericEntity implements NULL {
- protected NullGenericEntity() { }
+ protected NullGenericEntity() {
+ this.setImmutable();
+ }
@Override
public String getEntityName() {
Modified: ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericValue.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericValue.java?rev=1486304&r1=1486303&r2=1486304&view=diff
==============================================================================
--- ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericValue.java (original)
+++ ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/GenericValue.java Sat May 25 09:23:46 2013
@@ -23,6 +23,7 @@ package org.ofbiz.entity;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Collections;
import javolution.context.ObjectFactory;
import javolution.lang.Reusable;
@@ -61,11 +62,9 @@ public class GenericValue extends Generi
/** Map to cache various related cardinality one entity collections */
public transient Map<String, GenericValue> relatedOneCache = null;
- /** This Map will contain the original field values from the database iff
- * this GenericValue came from the database. If it was made manually it will
- * no have this Map, ie it will be null to not take up memory.
+ /** A Map containing the original field values from the database.
*/
- protected Map<String, Object> originalDbValues = null;
+ private Map<String, Object> originalDbValues = null;
protected GenericValue() { }
@@ -118,7 +117,7 @@ public class GenericValue extends Generi
@Override
public void synchronizedWithDatasource() {
super.synchronizedWithDatasource();
- this.copyOriginalDbValues();
+ this.originalDbValues = Collections.unmodifiableMap(getAllFields());
}
public GenericValue create() throws GenericEntityException {
@@ -147,21 +146,12 @@ public class GenericValue extends Generi
public Object getOriginalDbValue(String name) {
if (getModelEntity().getField(name) == null) {
- throw new IllegalArgumentException("[GenericEntity.get] \"" + name + "\" is not a field of " + entityName);
+ throw new IllegalArgumentException("[GenericEntity.get] \"" + name + "\" is not a field of " + getEntityName());
}
if (originalDbValues == null) return null;
return originalDbValues.get(name);
}
- /** This should only be called by the Entity Engine once a GenericValue has
- * been read from the database so that we have a copy of the original field
- * values from the Db.
- */
- public void copyOriginalDbValues() {
- this.originalDbValues = FastMap.newInstance();
- this.originalDbValues.putAll(this.fields);
- }
-
/** Get the named Related Entity for the GenericValue from the persistent store
*@param relationName String containing the relation name which is the combination of relation.title and relation.rel-entity-name as specified in the entity XML definition file
*@return List of GenericValue instances as specified in the relation definition
Modified: ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java?rev=1486304&r1=1486303&r2=1486304&view=diff
==============================================================================
--- ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java (original)
+++ ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java Sat May 25 09:23:46 2013
@@ -110,7 +110,6 @@ public class EntityTestSuite extends Ent
assertEquals("Retrieved value has the correct description", "Testing Type #4", testValue.getString("description"));
testValue.remove();
// Test immutable
- /* Requires revision 1471283
try {
testValue.put("description", "New Testing Type #4");
fail("Modified an immutable GenericValue");
@@ -121,7 +120,6 @@ public class EntityTestSuite extends Ent
fail("Modified an immutable GenericValue");
} catch (UnsupportedOperationException e) {
}
- */
testValue = delegator.findOne("TestingType", false, "testingTypeId", "TEST-4");
assertEquals("Finding removed value returns null", null, testValue);
}
@@ -139,13 +137,11 @@ public class EntityTestSuite extends Ent
fail("Modified an immutable GenericValue");
} catch (IllegalStateException e) {
}
- /* Requires revision 1471283
try {
testValue.remove("description");
fail("Modified an immutable GenericValue");
} catch (UnsupportedOperationException e) {
}
- */
// Test entity value update operation updates the cache
/* Requires revision 1471284, 1476296
testValue = (GenericValue) testValue.clone();
Modified: ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelForm.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelForm.java?rev=1486304&r1=1486303&r2=1486304&view=diff
==============================================================================
--- ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelForm.java (original)
+++ ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelForm.java Sat May 25 09:23:46 2013
@@ -21,6 +21,7 @@ package org.ofbiz.widget.form;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
@@ -46,6 +47,7 @@ import org.ofbiz.base.util.UtilXml;
import org.ofbiz.base.util.collections.FlexibleMapAccessor;
import org.ofbiz.base.util.collections.MapStack;
import org.ofbiz.base.util.string.FlexibleStringExpander;
+import org.ofbiz.entity.GenericEntity;
import org.ofbiz.entity.GenericEntityException;
import org.ofbiz.entity.model.ModelEntity;
import org.ofbiz.entity.model.ModelField;
@@ -1518,7 +1520,13 @@ public class ModelForm extends ModelWidg
if (UtilValidate.isNotEmpty(this.getListEntryName())) {
localContext.put(this.getListEntryName(), item);
} else {
- localContext.push(itemMap);
+ if (itemMap instanceof GenericEntity) {
+ // Rendering code might try to modify the GenericEntity instance,
+ // so we make a copy of it.
+ localContext.push(new HashMap<String, Object>(itemMap));
+ } else {
+ localContext.push(itemMap);
+ }
}
// reset/remove the BshInterpreter now as well as later because chances are there is an interpreter at this level of the stack too
Modified: ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java?rev=1486304&r1=1486303&r2=1486304&view=diff
==============================================================================
--- ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java (original)
+++ ofbiz/branches/release12.04/framework/widget/src/org/ofbiz/widget/form/ModelFormField.java Sat May 25 09:23:46 2013
@@ -1620,7 +1620,9 @@ public class ModelFormField {
for (GenericValue value: values) {
// add key and description with string expansion, ie expanding ${} stuff, passing locale explicitly to expand value string because it won't be found in the Entity
MapStack<String> localContext = MapStack.create(context);
- localContext.push(value);
+ // Rendering code might try to modify the GenericEntity instance,
+ // so we make a copy of it.
+ localContext.push(new HashMap<String, Object>(value));
// expand with the new localContext, which is locale aware
String optionDesc = this.description.expandString(localContext, locale);
@@ -2177,7 +2179,9 @@ public class ModelFormField {
if (value != null) {
// expanding ${} stuff, passing locale explicitly to expand value string because it won't be found in the Entity
MapStack<String> localContext = MapStack.create(context);
- localContext.push(value);
+ // Rendering code might try to modify the GenericEntity instance,
+ // so we make a copy of it.
+ localContext.push(new HashMap<String, Object>(value));
// expand with the new localContext, which is locale aware
retVal = this.description.expandString(localContext, locale);