Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
![]() ![]() ![]() ![]() ![]() ![]() ![]() |
1903 posts
|
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, [hidden email] 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; > } > > ... [show rest of quote]
|
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
![]() ![]() ![]() ![]() ![]() ![]() ![]() |
1903 posts
|
I ran a test and confirmed this is a problem. The transaction is
committed after the exception is thrown. I will commit a fix soon. Adrian Crum Sandglass Software www.sandglass-software.com On 9/30/2013 9:36 AM, Adrian Crum wrote: > 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, [hidden email] 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; >> } >> >> ... [show rest of quote]
|
Free forum by Nabble | Edit this page |