Author: adrianc
Date: Fri Apr 26 17:04:49 2013
New Revision: 1476296
URL: http://svn.apache.org/r1476296
Log:
Some bug fixes for entity engine caches. GenericDelegator was inconsistent in
clearing caches - some methods cleared the cache before the entity operation
(wrong) while others cleared the cache after the entity operation (right).
Also, I updated the Delegator JavaDocs to warn against skipping the cache
clearing step - which shouldn't be done. Finally, the
AbstractEntityConditionCache.storeHook method doesn't work (for a number of
reasons) so I bypassed it.
Modified:
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/Cache.java
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/eca/EntityEcaHandler.java
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java
Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java?rev=1476296&r1=1476295&r2=1476296&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java Fri Apr 26
17:04:49 2013
@@ -159,8 +159,9 @@ public interface Delegator {
* @param primaryKey
* The GenericPK to create a value in the datasource from
* @param doCacheClear
- * boolean that specifies whether to clear related cache entries
- * for this primaryKey to be created
+ * boolean that specifies whether or not to automatically clear
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return GenericValue instance containing the new instance
*/
public GenericValue create(GenericPK primaryKey, boolean doCacheClear)
throws GenericEntityException;
@@ -183,7 +184,8 @@ public interface Delegator {
* The GenericValue to create a value in the datasource from
* @param doCacheClear
* boolean that specifies whether or not to automatically clear
- * cache entries related to this operation
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return GenericValue instance containing the new instance
*/
public GenericValue create(GenericValue value, boolean doCacheClear)
throws GenericEntityException;
@@ -222,7 +224,8 @@ public interface Delegator {
* instance
* @param doCacheClear
* boolean that specifies whether or not to automatically clear
- * cache entries related to this operation
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return GenericValue instance containing the new or updated instance
*/
public GenericValue createOrStore(GenericValue value, boolean
doCacheClear) throws GenericEntityException;
@@ -950,7 +953,8 @@ public interface Delegator {
* GenericValue instance containing the entity to refresh
* @param doCacheClear
* boolean that specifies whether or not to automatically clear
- * cache entries related to this operation
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
*/
public void refresh(GenericValue value, boolean doCacheClear) throws
GenericEntityException;
@@ -999,7 +1003,8 @@ public interface Delegator {
* or by and fields to remove
* @param doCacheClear
* boolean that specifies whether or not to automatically clear
- * cache entries related to this operation
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return int representing number of rows effected by this operation
*/
public int removeAll(List<? extends GenericEntity> dummyPKs, boolean
doCacheClear) throws GenericEntityException;
@@ -1013,8 +1018,9 @@ public interface Delegator {
* @param entityName
* The Name of the Entity as defined in the entity XML file
* @param doCacheClear
- * boolean that specifies whether to clear cache entries for
this
- * value to be removed
+ * boolean that specifies whether or not to automatically clear
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @param fields
* The fields of the named entity to query by with their
* corresponding values
@@ -1045,8 +1051,9 @@ public interface Delegator {
* The fields of the named entity to query by with their
* corresponding values
* @param doCacheClear
- * boolean that specifies whether to clear cache entries for
this
- * value to be removed
+ * boolean that specifies whether or not to automatically clear
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return int representing number of rows effected by this operation
*/
public int removeByAnd(String entityName, Map<String, ? extends Object>
fields, boolean doCacheClear) throws GenericEntityException;
@@ -1083,8 +1090,9 @@ public interface Delegator {
* @param condition
* The condition used to restrict the removing
* @param doCacheClear
- * boolean that specifies whether to clear cache entries for
this
- * value to be removed
+ * boolean that specifies whether or not to automatically clear
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return int representing number of rows effected by this operation
*/
public int removeByCondition(String entityName, EntityCondition condition,
boolean doCacheClear) throws GenericEntityException;
@@ -1104,8 +1112,9 @@ public interface Delegator {
* @param primaryKey
* The primary key of the entity to remove.
* @param doCacheClear
- * boolean that specifies whether to clear cache entries for
this
- * primaryKey to be removed
+ * boolean that specifies whether or not to automatically clear
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return int representing number of rows effected by this operation
*/
public int removeByPrimaryKey(GenericPK primaryKey, boolean doCacheClear)
throws GenericEntityException;
@@ -1135,8 +1144,9 @@ public interface Delegator {
* @param value
* GenericValue instance containing the entity
* @param doCacheClear
- * boolean that specifies whether to clear cache entries for
this
- * value to be removed
+ * boolean that specifies whether or not to automatically clear
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return int representing number of rows effected by this operation
*/
public int removeRelated(String relationName, GenericValue value, boolean
doCacheClear) throws GenericEntityException;
@@ -1156,8 +1166,9 @@ public interface Delegator {
* @param value
* The GenericValue object of the entity to remove.
* @param doCacheClear
- * boolean that specifies whether to clear cache entries for
this
- * value to be removed
+ * boolean that specifies whether or not to automatically clear
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return int representing number of rows effected by this operation
*/
public int removeValue(GenericValue value, boolean doCacheClear) throws
GenericEntityException;
@@ -1199,7 +1210,8 @@ public interface Delegator {
* GenericValue instance containing the entity
* @param doCacheClear
* boolean that specifies whether or not to automatically clear
- * cache entries related to this operation
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return int representing number of rows effected by this operation
*/
public int store(GenericValue value, boolean doCacheClear) throws
GenericEntityException;
@@ -1236,7 +1248,8 @@ public interface Delegator {
* store
* @param doCacheClear
* boolean that specifies whether or not to automatically clear
- * cache entries related to this operation
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return int representing number of rows effected by this operation
*/
public int storeAll(List<GenericValue> values, boolean doCacheClear)
throws GenericEntityException;
@@ -1256,7 +1269,8 @@ public interface Delegator {
* store
* @param doCacheClear
* boolean that specifies whether or not to automatically clear
- * cache entries related to this operation
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @param createDummyFks
* boolean that specifies whether or not to automatically create
* "dummy" place holder FKs
@@ -1288,8 +1302,9 @@ public interface Delegator {
* @param condition
* The condition that restricts the list of stored values
* @param doCacheClear
- * boolean that specifies whether to clear cache entries for
- * these values
+ * boolean that specifies whether or not to automatically clear
+ * cache entries related to this operation. This should always
be
+ * <code>true</code> - otherwise you will lose data integrity.
* @return int representing number of rows effected by this operation
* @throws GenericEntityException
*/
Modified:
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java?rev=1476296&r1=1476295&r2=1476296&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
(original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java Fri
Apr 26 17:04:49 2013
@@ -995,12 +995,6 @@ public class GenericDelegator implements
GenericHelper helper = getEntityHelper(primaryKey.getEntityName());
- if (doCacheClear) {
- // always clear cache before the operation
- ecaRunner.evalRules(EntityEcaHandler.EV_CACHE_CLEAR,
EntityEcaHandler.OP_REMOVE, primaryKey, false);
- this.clearCacheLine(primaryKey);
- }
-
ecaRunner.evalRules(EntityEcaHandler.EV_RUN,
EntityEcaHandler.OP_REMOVE, primaryKey, false);
// if audit log on for any fields, save old value before removing
so it's still there
@@ -1013,6 +1007,11 @@ public class GenericDelegator implements
removedEntity = this.findOne(primaryKey.getEntityName(),
primaryKey, false);
}
int num = helper.removeByPrimaryKey(primaryKey);
+ if (doCacheClear) {
+ ecaRunner.evalRules(EntityEcaHandler.EV_CACHE_CLEAR,
EntityEcaHandler.OP_REMOVE, primaryKey, false);
+ this.clearCacheLine(primaryKey);
+ }
+
this.saveEntitySyncRemoveInfo(primaryKey);
if (testMode) {
@@ -1064,11 +1063,6 @@ public class GenericDelegator implements
GenericHelper helper = getEntityHelper(value.getEntityName());
- if (doCacheClear) {
- ecaRunner.evalRules(EntityEcaHandler.EV_CACHE_CLEAR,
EntityEcaHandler.OP_REMOVE, value, false);
- this.clearCacheLine(value);
- }
-
ecaRunner.evalRules(EntityEcaHandler.EV_RUN,
EntityEcaHandler.OP_REMOVE, value, false);
// if audit log on for any fields, save old value before actual
remove
@@ -1084,6 +1078,11 @@ 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 (doCacheClear) {
+ ecaRunner.evalRules(EntityEcaHandler.EV_CACHE_CLEAR,
EntityEcaHandler.OP_REMOVE, value, false);
+ this.clearCacheLine(value);
+ }
+
if (testMode) {
if (removedValue != null) {
@@ -1329,12 +1328,6 @@ public class GenericDelegator implements
ecaRunner.evalRules(EntityEcaHandler.EV_VALIDATE,
EntityEcaHandler.OP_STORE, value, false);
GenericHelper helper = getEntityHelper(value.getEntityName());
- if (doCacheClear) {
- // always clear cache before the operation
- ecaRunner.evalRules(EntityEcaHandler.EV_CACHE_CLEAR,
EntityEcaHandler.OP_STORE, value, false);
- this.clearCacheLine(value);
- }
-
ecaRunner.evalRules(EntityEcaHandler.EV_RUN,
EntityEcaHandler.OP_STORE, value, false);
this.encryptFields(value);
@@ -1350,6 +1343,10 @@ public class GenericDelegator implements
}
int retVal = helper.store(value);
+ if (doCacheClear) {
+ ecaRunner.evalRules(EntityEcaHandler.EV_CACHE_CLEAR,
EntityEcaHandler.OP_STORE, value, false);
+ this.clearCacheLine(value);
+ }
if (testMode) {
storeForTestRollback(new TestOperation(OperationType.UPDATE,
updatedEntity));
@@ -2192,11 +2189,6 @@ public class GenericDelegator implements
* @see
org.ofbiz.entity.Delegator#clearCacheLine(org.ofbiz.entity.GenericValue,
boolean)
*/
public void clearCacheLine(GenericValue value, boolean distribute) {
- // TODO: make this a bit more intelligent by passing in the operation
being done (create, update, remove) so we can not do unnecessary cache clears...
- // for instance:
- // on create don't clear by primary cache (and won't clear original
values because there won't be any)
- // on remove don't clear by and for new values, but do for original
values
-
// Debug.logInfo("running clearCacheLine for value: " + value + ",
distribute: " + distribute, module);
if (value == null) {
return;
Modified:
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java?rev=1476296&r1=1476295&r2=1476296&view=diff
==============================================================================
---
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java
(original)
+++
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/AbstractEntityConditionCache.java
Fri Apr 26 17:04:49 2013
@@ -59,6 +59,21 @@ public abstract class AbstractEntityCond
return conditionCache.put(key, value);
}
+ /**
+ * Removes all condition caches that include the specified entity.
+ */
+ public void remove(GenericEntity entity) {
+ UtilCache.clearCache(getCacheName(entity.getEntityName()));
+ ModelEntity model = entity.getModelEntity();
+ if (model != null) {
+ Iterator<String> it = model.getViewConvertorsIterator();
+ while (it.hasNext()) {
+ String targetEntityName = it.next();
+ UtilCache.clearCache(getCacheName(targetEntityName));
+ }
+ }
+ }
+
public void remove(String entityName, EntityCondition condition) {
UtilCache<EntityCondition, ConcurrentMap<K, V>> cache =
getCache(entityName);
if (cache == null) return;
Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/Cache.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/Cache.java?rev=1476296&r1=1476295&r2=1476296&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/Cache.java
(original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/cache/Cache.java Fri Apr
26 17:04:49 2013
@@ -112,16 +112,22 @@ public class Cache {
public GenericValue remove(GenericEntity entity) {
if (Debug.verboseOn()) Debug.logVerbose("Cache remove GenericEntity: "
+ entity, module);
GenericValue oldEntity = entityCache.remove(entity.getPrimaryKey());
- entityListCache.storeHook(entity, null);
- entityObjectCache.storeHook(entity, null);
+ // Workaround because AbstractEntityConditionCache.storeHook doesn't
work.
+ entityListCache.remove(entity);
+ entityObjectCache.remove(entity);
+ // entityListCache.storeHook(entity, null);
+ // entityObjectCache.storeHook(entity, null);
return oldEntity;
}
public GenericValue remove(GenericPK pk) {
if (Debug.verboseOn()) Debug.logVerbose("Cache remove GenericPK: " +
pk, module);
GenericValue oldEntity = entityCache.remove(pk);
- entityListCache.storeHook(pk, null);
- entityObjectCache.storeHook(pk, null);
+ // Workaround because AbstractEntityConditionCache.storeHook doesn't
work.
+ entityListCache.remove(pk);
+ entityObjectCache.remove(pk);
+ // entityListCache.storeHook(pk, null);
+ // entityObjectCache.storeHook(pk, null);
return oldEntity;
}
}
Modified:
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/eca/EntityEcaHandler.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/eca/EntityEcaHandler.java?rev=1476296&r1=1476295&r2=1476296&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/eca/EntityEcaHandler.java
(original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/eca/EntityEcaHandler.java
Fri Apr 26 17:04:49 2013
@@ -34,6 +34,9 @@ public interface EntityEcaHandler<T> {
public static final String EV_VALIDATE = "validate";
public static final String EV_RUN = "run";
public static final String EV_RETURN = "return";
+ /**
+ * Invoked after the entity operation, but before the cache is cleared.
+ */
public static final String EV_CACHE_CLEAR = "cache-clear";
public static final String EV_CACHE_CHECK = "cache-check";
public static final String EV_CACHE_PUT = "cache-put";
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=1476296&r1=1476295&r2=1476296&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
Fri Apr 26 17:04:49 2013
@@ -107,7 +107,7 @@ public class EntityTestSuite extends Ent
observer.arg = null;
GenericValue clonedValue = (GenericValue) testValue.clone();
clonedValue.put("description", "New Testing Type #1");
- assertTrue("Observable has changed", testValue.hasChanged());
+ assertTrue("Cloned Observable has changed", clonedValue.hasChanged());
assertEquals("Observer called with cloned GenericValue field name",
"description", observer.arg);
// now store it
testValue.store();
@@ -142,12 +142,11 @@ public class EntityTestSuite extends Ent
*/
public void testEntityCache() throws Exception {
// 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"));
+ GenericValue testValue = delegator.findOne("TestingType", true, "testingTypeId",
"TEST-3");
+ assertEquals("Retrieved from cache value has the correct description", "Testing Type
#3", testValue.getString("description"));
// Test immutable
try {
- testValue.put("description", "New Testing Type #2");
- testValue.store();
+ testValue.put("description", "New Testing Type #3");
fail("Modified an immutable GenericValue");
} catch (IllegalStateException e) {
}
@@ -156,6 +155,17 @@ public class EntityTestSuite extends Ent
fail("Modified an immutable GenericValue");
} catch (UnsupportedOperationException e) {
}
+ // Test entity value update operation updates the cache
+ testValue = (GenericValue) testValue.clone();
+ testValue.put("description", "New Testing Type #3");
+ testValue.store();
+ testValue = delegator.findOne("TestingType", true, "testingTypeId",
"TEST-3");
+ assertEquals("Retrieved from cache value has the correct description", "New Testing
Type #3", testValue.getString("description"));
+ // Test entity value remove operation updates the cache
+ testValue = (GenericValue) testValue.clone();
+ testValue.remove();
+ testValue = delegator.findOne("TestingType", true, "testingTypeId",
"TEST-3");
+ assertEquals("Retrieved from cache value is null", null, testValue);
// Test entity condition cache
EntityCondition testCondition = EntityCondition.makeCondition("description",
EntityOperator.EQUALS, "Testing Type #2");
List<GenericValue> testList = delegator.findList("TestingType",
testCondition, null, null, null, true);
@@ -165,7 +175,6 @@ public class EntityTestSuite extends Ent
// Test immutable
try {
testValue.put("description", "New Testing Type #2");
- testValue.store();
fail("Modified an immutable GenericValue");
} catch (IllegalStateException e) {
}
@@ -174,13 +183,24 @@ public class EntityTestSuite extends Ent
fail("Modified an immutable GenericValue");
} catch (UnsupportedOperationException e) {
}
- /* Commenting this out for now because the tests fail due to flaws in
the EntityListCache implementation.
+ // Test entity value create operation updates the cache
testValue = (GenericValue) testValue.clone();
+ testValue.put("testingTypeId", "TEST-9");
+ testValue.create();
+ testList = delegator.findList("TestingType", testCondition, null,
null, null, true);
+ assertEquals("Delegator findList returned two values", 2,
testList.size());
+ // Test entity value update operation updates the cache
testValue.put("description", "New Testing Type #2");
testValue.store();
testList = delegator.findList("TestingType", testCondition, null,
null, null, true);
+ assertEquals("Delegator findList returned one value", 1,
testList.size());
+ // Test entity value remove operation updates the cache
+ testValue = testList.get(0);
+ testValue = (GenericValue) testValue.clone();
+ testValue.remove();
+ testList = delegator.findList("TestingType", testCondition, null,
null, null, true);
assertEquals("Delegator findList returned empty list", 0,
testList.size());
- */
+ // TODO: Test view entities.
}
/*
