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/04/26 19:04:49 UTC
svn commit: r1476296 - in
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity: Delegator.java
GenericDelegator.java cache/AbstractEntityConditionCache.java
cache/Cache.java eca/EntityEcaHandler.java test/EntityTestSuite.java
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.
}
/*
Re: svn commit: r1476296 - in
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity: Delegator.java
GenericDelegator.java cache/AbstractEntityConditionCache.java
cache/Cache.java eca/EntityEcaHandler.java test/EntityTestSuite.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
(sorry for previous message; seems my email client did me a joke)
Then I would also add a warning in log.
But I'm not quite sure all this would be enough.
It'd be better to refactor it, to remove any possible confusions. Fortunately those methods seems to not be much used OOTB.
Of course this means that we are sure we will not lose anything from this refactoring. It seems you are quite sure from the javadoc, right?
Jacques
From: "Adrian Crum" <ad...@sandglass-software.com>
> We could change the code to always clear the cache, but leave the method
> signature the same and mention in the JavaDocs that the doCacheClear
> parameter will be ignored.
>
> -Adrian
>
> On 5/11/2013 12:54 PM, Jacques Le Roux wrote:
>> Adrian,
>>
>> In javadoc of numbers of methods of Delegator.java you added this sentence
>> - * 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.Now I wonder if we should no more and remove all those . What's the point of letting users not doing a cache clear there?ThanksJacquesFrom: <ad...@apache.org>
>>> 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.
>>> }
>>>
>>> /*
>>>
>>>
>
Re: svn commit: r1476296 - in /ofbiz/trunk/framework/entity/src/org/ofbiz/entity:
Delegator.java GenericDelegator.java cache/AbstractEntityConditionCache.java
cache/Cache.java eca/EntityEcaHandler.java test/EntityTestSuite.java
Posted by Adrian Crum <ad...@sandglass-software.com>.
We could change the code to always clear the cache, but leave the method
signature the same and mention in the JavaDocs that the doCacheClear
parameter will be ignored.
-Adrian
On 5/11/2013 12:54 PM, Jacques Le Roux wrote:
> Adrian,
>
> In javadoc of numbers of methods of Delegator.java you added this sentence
> - * 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.Now I wonder if we should no more and remove all those . What's the point of letting users not doing a cache clear there?ThanksJacquesFrom: <ad...@apache.org>
>> 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.
>> }
>>
>> /*
>>
>>
Re: svn commit: r1476296 - in
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity: Delegator.java
GenericDelegator.java cache/AbstractEntityConditionCache.java
cache/Cache.java eca/EntityEcaHandler.java test/EntityTestSuite.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Done in trunk at r1524704
Jacques
Jacques Le Roux wrote:
> Hi Paul,
>
> Yes that should be the way indeed. We should do it before creating R13
>
> Jacques
>
> From: "Paul Foxworthy" <pa...@cohsoft.com.au>
>> Hi Adrian and Jacques,
>>
>> If doCacheClear will have no effect, I suggest deprecating the method
>> signature with the doCacheClear. It can call another overloaded variant
>> without that parameter. All calls in trunk should be changed to the second.
>> We can leave the deprecated version for backwards compatibility for a time.
>>
>> I agree in the release branches that all we should do is add some JavaDoc
>> and log a warning.
>>
>> Cheers
>>
>> Paul Foxworthy
>>
>>
>> Jacques Le Roux wrote
>>> Jacques Le Roux wrote:
>>>> (sorry for previous message; seems my email client did me a joke)
>>>>
>>>> Then I would also add a warning in log.
>>>>
>>>> But I'm not quite sure all this would be enough.
>>>> It'd be better to refactor it, to remove any possible confusions.
>>>> Fortunately those methods seems to not be much used OOTB.
>>>
>>> Of course this would be done in trunk only. What you proposed + log
>>> warning would be in branches releases.
>>>
>>> Jacques
>>>
>>>> Of course this means that we are sure we will not lose anything from this
>>>> refactoring. It seems you are quite sure from the
>>>> javadoc, right?
>>>>
>>>> Jacques
>>>>
>>>> From: "Adrian Crum" <
>>
>>> adrian.crum@
>>
>>> >
>>>>> We could change the code to always clear the cache, but leave the method
>>>>> signature the same and mention in the JavaDocs that the doCacheClear
>>>>> parameter will be ignored.
>>>>>
>>>>> -Adrian
>>>>>
>>>>> On 5/11/2013 12:54 PM, Jacques Le Roux wrote:
>>>>>> Adrian,
>>>>>>
>>>>>> In javadoc of numbers of methods of Delegator.java you added this
>>>>>> sentence
>>>>>> - * 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.Now I wonder if we should no
>>> more and remove all
>>>>>> those . What's the point of letting users not doing a cache clear
>>>>>> there?ThanksJacquesFrom: <
>>
>>> adrianc@
>>
>>> >
>>>>>>> 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.
>>>>>>> }
>>>>>>>
>>>>>>> /*
>>
>>
>>
>>
>>
>> -----
>> --
>> Coherent Software Australia Pty Ltd
>> http://www.coherentsoftware.com.au/
>>
>> Bonsai ERP, the all-inclusive ERP system
>> http://www.bonsaierp.com.au/
>>
>> --
>> View this message in context:
>> http://ofbiz.135035.n4.nabble.com/Re-svn-commit-r1476296-in-ofbiz-trunk-framework-entity-src-org-ofbiz-entity-Delegator-java-GenericDea-tp4641196p4641328.html
>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
Re: svn commit: r1476296 - in
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity: Delegator.java
GenericDelegator.java cache/AbstractEntityConditionCache.java
cache/Cache.java eca/EntityEcaHandler.java test/EntityTestSuite.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Paul,
Yes that should be the way indeed. We should do it before creating R13
Jacques
From: "Paul Foxworthy" <pa...@cohsoft.com.au>
> Hi Adrian and Jacques,
>
> If doCacheClear will have no effect, I suggest deprecating the method
> signature with the doCacheClear. It can call another overloaded variant
> without that parameter. All calls in trunk should be changed to the second.
> We can leave the deprecated version for backwards compatibility for a time.
>
> I agree in the release branches that all we should do is add some JavaDoc
> and log a warning.
>
> Cheers
>
> Paul Foxworthy
>
>
> Jacques Le Roux wrote
>> Jacques Le Roux wrote:
>>> (sorry for previous message; seems my email client did me a joke)
>>>
>>> Then I would also add a warning in log.
>>>
>>> But I'm not quite sure all this would be enough.
>>> It'd be better to refactor it, to remove any possible confusions.
>>> Fortunately those methods seems to not be much used OOTB.
>>
>> Of course this would be done in trunk only. What you proposed + log
>> warning would be in branches releases.
>>
>> Jacques
>>
>>> Of course this means that we are sure we will not lose anything from this
>>> refactoring. It seems you are quite sure from the
>>> javadoc, right?
>>>
>>> Jacques
>>>
>>> From: "Adrian Crum" <
>
>> adrian.crum@
>
>> >
>>>> We could change the code to always clear the cache, but leave the method
>>>> signature the same and mention in the JavaDocs that the doCacheClear
>>>> parameter will be ignored.
>>>>
>>>> -Adrian
>>>>
>>>> On 5/11/2013 12:54 PM, Jacques Le Roux wrote:
>>>>> Adrian,
>>>>>
>>>>> In javadoc of numbers of methods of Delegator.java you added this
>>>>> sentence
>>>>> - * 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.Now I wonder if we should no
>> more and remove all
>>>>> those . What's the point of letting users not doing a cache clear
>>>>> there?ThanksJacquesFrom: <
>
>> adrianc@
>
>> >
>>>>>> 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.
>>>>>> }
>>>>>>
>>>>>> /*
>
>
>
>
>
> -----
> --
> Coherent Software Australia Pty Ltd
> http://www.coherentsoftware.com.au/
>
> Bonsai ERP, the all-inclusive ERP system
> http://www.bonsaierp.com.au/
>
> --
> View this message in context: http://ofbiz.135035.n4.nabble.com/Re-svn-commit-r1476296-in-ofbiz-trunk-framework-entity-src-org-ofbiz-entity-Delegator-java-GenericDea-tp4641196p4641328.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
Re: svn commit: r1476296 - in
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity: Delegator.java
GenericDelegator.java cache/AbstractEntityConditionCache.java
cache/Cache.java eca/EntityEcaHandler.java test/EntityTestSuite.java
Posted by Paul Foxworthy <pa...@cohsoft.com.au>.
Hi Adrian and Jacques,
If doCacheClear will have no effect, I suggest deprecating the method
signature with the doCacheClear. It can call another overloaded variant
without that parameter. All calls in trunk should be changed to the second.
We can leave the deprecated version for backwards compatibility for a time.
I agree in the release branches that all we should do is add some JavaDoc
and log a warning.
Cheers
Paul Foxworthy
Jacques Le Roux wrote
> Jacques Le Roux wrote:
>> (sorry for previous message; seems my email client did me a joke)
>>
>> Then I would also add a warning in log.
>>
>> But I'm not quite sure all this would be enough.
>> It'd be better to refactor it, to remove any possible confusions.
>> Fortunately those methods seems to not be much used OOTB.
>
> Of course this would be done in trunk only. What you proposed + log
> warning would be in branches releases.
>
> Jacques
>
>> Of course this means that we are sure we will not lose anything from this
>> refactoring. It seems you are quite sure from the
>> javadoc, right?
>>
>> Jacques
>>
>> From: "Adrian Crum" <
> adrian.crum@
> >
>>> We could change the code to always clear the cache, but leave the method
>>> signature the same and mention in the JavaDocs that the doCacheClear
>>> parameter will be ignored.
>>>
>>> -Adrian
>>>
>>> On 5/11/2013 12:54 PM, Jacques Le Roux wrote:
>>>> Adrian,
>>>>
>>>> In javadoc of numbers of methods of Delegator.java you added this
>>>> sentence
>>>> - * 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.Now I wonder if we should no
> more and remove all
>>>> those . What's the point of letting users not doing a cache clear
>>>> there?ThanksJacquesFrom: <
> adrianc@
> >
>>>>> 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.
>>>>> }
>>>>>
>>>>> /*
-----
--
Coherent Software Australia Pty Ltd
http://www.coherentsoftware.com.au/
Bonsai ERP, the all-inclusive ERP system
http://www.bonsaierp.com.au/
--
View this message in context: http://ofbiz.135035.n4.nabble.com/Re-svn-commit-r1476296-in-ofbiz-trunk-framework-entity-src-org-ofbiz-entity-Delegator-java-GenericDea-tp4641196p4641328.html
Sent from the OFBiz - Dev mailing list archive at Nabble.com.
Re: svn commit: r1476296 - in
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity: Delegator.java
GenericDelegator.java cache/AbstractEntityConditionCache.java
cache/Cache.java eca/EntityEcaHandler.java test/EntityTestSuite.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Adrian,
In javadoc of numbers of methods of Delegator.java you added this sentence
- * 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.Now I wonder if we should no more and remove all those . What's the point of letting users not doing a cache clear there?ThanksJacquesFrom: <ad...@apache.org>
> 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.
> }
>
> /*
>
>