svn commit: r1476296 - in /ofbiz/trunk/framework/entity/
Locked
 
|
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. } /* |
«
Return to OFBiz - Commits
|
1 view|%1 views
| Free forum by Nabble | Edit this page |