I just noticed another problem in the GenericDelegator.findOne method.
The try-catch-finally logic is flawed. The
TransactionUtil.commit(beganTransaction) statement belongs in the try
block, and the catch clause should catch ALL exceptions.
The way things are structured now, if an exception is thrown in the try
block, the transaction will be committed. It should only commit the
transaction if the code executes successfully all the way to the end of
the try block.
Any thoughts?
Adrian Crum
Sandglass Software
www.sandglass-software.com
On 9/30/2013 9:13 AM, adri...@apache.org wrote:
Author: adrianc
Date: Mon Sep 30 16:13:03 2013
New Revision: 1527626
URL: http://svn.apache.org/r1527626
Log:
Fixed a subtle flaw in the GenericDelegator.findOne method. When a database
query returns no result, GenericValue.NULL_VALUE is put in the pk cache - so
future findOne calls will know the entity value doesn't exist. But the findOne
method never checked for GenericValue.NULL_VALUE in cache gets, so the database
was queried again for an entity value we already know doesn't exist.
https://issues.apache.org/jira/browse/OFBIZ-5332
Modified:
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.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=1527626&r1=1527625&r2=1527626&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
(original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java Mon
Sep 30 16:13:03 2013
@@ -1580,8 +1580,10 @@ public class GenericDelegator implements
EntityEcaRuleRunner<?> ecaRunner = this.getEcaRuleRunner(entityName);
if (useCache) {
ecaRunner.evalRules(EntityEcaHandler.EV_CACHE_CHECK,
EntityEcaHandler.OP_FIND, primaryKey, false);
-
- GenericValue value = this.getFromPrimaryKeyCache(primaryKey);
+ GenericValue value = cache.get(primaryKey);
+ if (value == GenericValue.NULL_VALUE) {
+ return null;
+ }
if (value != null) {
return value;
}
