svn commit: r1621413 - in /ofbiz/trunk/framework/entity/src/org/ofbiz/entity: GenericDelegator.java test/EntityTestSuite.java

[adrianc]

Author: adrianc
Date: Sat Aug 30 02:49:22 2014
New Revision: 1621413

URL: http://svn.apache.org/r1621413
Log:
Fixed bugs in GenericDelegator.java - storeByCondition and removeByCondition did not update the cache, resulting in stale cache entries.

Modified:
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/test/EntityTestSuite.java

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=1621413&r1=1621412&r2=1621413&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java Sat Aug 30 02:49:22 2014
@@ -1201,15 +1201,6 @@ public class GenericDelegator implements
                 beganTransaction = TransactionUtil.begin();
             }
 
-            if (doCacheClear) {
-                // always clear cache before the operation
-                /*
-                 *  FIXME: This does not work - we still have a stale cache
-                 *  because the "remove by" condition might be different than
-                 *  the "find by" condition.
-                 */
-                this.clearCacheLineByCondition(entityName, condition);
-            }
             ModelEntity modelEntity = getModelReader().getModelEntity(entityName);
             GenericHelper helper = getEntityHelper(entityName);
 
@@ -1219,6 +1210,9 @@ public class GenericDelegator implements
             }
 
             int rowsAffected = helper.removeByCondition(this, modelEntity, condition);
+            if (rowsAffected > 0 && doCacheClear) {
+                this.clearCacheLine(entityName);
+            }
 
             if (testMode) {
                 for (GenericValue entity : removedEntities) {
@@ -1320,15 +1314,6 @@ public class GenericDelegator implements
                 beganTransaction = TransactionUtil.begin();
             }
 
-            if (doCacheClear) {
-                // always clear cache before the operation
-                /*
-                 *  FIXME: This does not work - we still have a stale cache
-                 *  because the "store by" condition might be different than
-                 *  the "find by" condition.
-                 */
-                this.clearCacheLineByCondition(entityName, condition);
-            }
             ModelEntity modelEntity = getModelReader().getModelEntity(entityName);
             GenericHelper helper = getEntityHelper(entityName);
 
@@ -1338,6 +1323,9 @@ public class GenericDelegator implements
             }
 
             int rowsAffected =  helper.storeByCondition(this, modelEntity, fieldsToSet, condition);
+            if (rowsAffected > 0 && doCacheClear) {
+                this.clearCacheLine(entityName);
+            }
 
             if (testMode) {
                 for (GenericValue entity : updatedEntities) {

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=1621413&r1=1621412&r2=1621413&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 Sat Aug 30 02:49:22 2014
@@ -64,6 +64,7 @@ import org.ofbiz.entity.transaction.Tran
 import org.ofbiz.entity.util.EntityFindOptions;
 import org.ofbiz.entity.util.EntityListIterator;
 import org.ofbiz.entity.util.EntitySaxReader;
+import org.ofbiz.entity.util.EntityUtil;
 import org.ofbiz.entity.util.SequenceUtil;
 
 public class EntityTestSuite extends EntityTestCase {
@@ -208,17 +209,20 @@ public class EntityTestSuite extends Ent
         testValue = delegator.findOne("TestingType", true, "testingTypeId", "TEST-CACHE-1");
         assertEquals("Retrieved from cache value has the correct description", "New Testing Type #Cache-1", testValue.getString("description"));
         // Test storeByCondition updates the cache
-        /*
         testValue = EntityUtil.getFirst(delegator.findByAnd("TestingType", UtilMisc.toMap("testingTypeId", "TEST-CACHE-1"), null, true));
         EntityCondition storeByCondition = EntityCondition.makeCondition(UtilMisc.toMap("testingTypeId", "TEST-CACHE-1",
                 "lastUpdatedStamp", testValue.get("lastUpdatedStamp")));
         int qtyChanged = delegator.storeByCondition("TestingType", UtilMisc.toMap("description", "New Testing Type #Cache-0"), storeByCondition);
-        assertTrue("Delegator.storeByCondition updated one value", qtyChanged == 1);
+        assertEquals("Delegator.storeByCondition updated one value", 1, qtyChanged);
         testValue = EntityUtil.getFirst(delegator.findByAnd("TestingType", UtilMisc.toMap("testingTypeId", "TEST-CACHE-1"), null, true));
         assertEquals("Retrieved from cache value has the correct description", "New Testing Type #Cache-0", testValue.getString("description"));
-        */
+        // Test removeByCondition updates the cache
+        qtyChanged = delegator.removeByCondition("TestingType", storeByCondition);
+        assertEquals("Delegator.removeByCondition removed one value", 1, qtyChanged);
+        testValue = EntityUtil.getFirst(delegator.findByAnd("TestingType", UtilMisc.toMap("testingTypeId", "TEST-CACHE-1"), null, true));
+        assertEquals("Retrieved from cache value is null", null, testValue);
         // Test entity value remove operation updates the cache
-        testValue = (GenericValue) testValue.clone();
+        testValue = delegator.create("TestingType", "testingTypeId", "TEST-CACHE-1", "description", "Testing Type #Cache-1");
         testValue.remove();
         testValue = delegator.findOne("TestingType", true, "testingTypeId", "TEST-CACHE-1");
         assertEquals("Retrieved from cache value is null", null, testValue);