Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0033816Openbravo ERPA. Platformpublic2016-08-25 16:182016-12-23 08:44
Reportercaristu 
Assigned Tocaristu 
PriorityurgentSeveritymajorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target VersionFixed in Version3.0PR17Q1 
Merge Request Status
Review Assigned Toalostale
OBNetwork customer
Web browser
ModulesCore
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0033816: Find a complete solution to protect OBCriteria.initialize() against multiple calls per instance
DescriptionOBCriteria has an internal initialize() method which adds the usual client,org,isactive filters.

However that initialize() method is not protected against multiple calls per instance.

So calling it more than once adds those filters also multiple times.

Several are at least 2 sequences triggering this
a.) call .count() + .uniqueResult() manually from application as seen i.e. in WebPOS OrderLoader:
    final OBCriteria<Locator> locators = OBDal.getInstance().createCriteria(Locator.class);
    locators.add(Restrictions.eq(Locator.PROPERTY_ACTIVE, true));
    locators.add(Restrictions.eq(Locator.PROPERTY_WAREHOUSE, shipment.getWarehouse()));
    // note the count causes a query but the good thing is that it doesn't cause loading
    // additional bin locations if there are too many
    if (locators.count() == 1) {
      foundSingleBin = (Locator) locators.uniqueResult();

b.) But also .count() implementation itself calls initialize() and then uniqueResult() (calling it again).

Steps To ReproduceTrace all SQL's done by OrderLoader starting at part in createShipmentLines
    if (locators.count() == 1) {
      foundSingleBin = (Locator) locators.uniqueResult();

That SQL run by the .count() already has 3 blocks of the client+org filters happening (2 copies by this bug) + additional one reported as issue 33132.

Then the next SQL run by the manual uniqueResult added 2 more copies of the filters. (one by itself and maybe 2nd one because of that same 33132).
Proposed SolutionAdd isInitialized boolean or similar to OBCriteria.initialize(). Take into account the possible reuse of the OBCriteria instance after fired a query.

Solution applied on issue 33138 was reverted on issue 33814 as it was not complete.
Additional Information
TagsNo tags attached.
Relationships
related to defect 00338143.0PR16Q4 closed caristu Initialization in OBCriteria should not be prevented under some circumstances 
related to defect 0033138 closed alostale OBCriteria.initialize() is not protected against multiple calls per instance 
related to feature request 0033767 closed platform Add code to auto-detect 'accidental double query' on same OBQuery or OBCriteria object 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2016-08-25 16:18caristuNew Issue
2016-08-25 16:18caristuAssigned To => alostale
2016-08-25 16:18caristuModules => Core
2016-08-25 16:18caristuTriggers an Emergency Pack => No
2016-08-25 16:18caristuIssue generated from0033138
2016-08-25 16:18caristuAssigned Toalostale => platform
2016-08-25 16:18caristuRelationship addedrelated to 0033814
2016-08-26 08:07caristuRelationship addedrelated to 0033138
2016-08-26 08:08caristuSummaryOBCriteria.initialize() is not protected against multiple calls per instance => Find a complete solution to protect OBCriteria.initialize() against multiple calls per instance
2016-08-26 08:08caristuProposed Solution updated
2016-12-01 13:14alostaleStatusnew => acknowledged
2016-12-01 13:17alostalePrioritynormal => urgent
2016-12-01 14:30alostaleAssigned Toplatform => caristu
2016-12-16 14:33caristuStatusacknowledged => scheduled
2016-12-19 10:21caristuRelationship addedrelated to 0033767
2016-12-19 10:37hgbotCheckin
2016-12-19 10:37hgbotNote Added: 0092757
2016-12-19 10:37hgbotStatusscheduled => resolved
2016-12-19 10:37hgbotResolutionopen => fixed
2016-12-19 10:37hgbotFixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/8a7ca1c913edeb3aeb8ce0ed21748da90e234835 [^]
2016-12-19 10:52caristuReview Assigned To => alostale
2016-12-19 10:52caristuIssue Monitored: alostale
2016-12-19 23:17hudsonbotCheckin
2016-12-19 23:17hudsonbotNote Added: 0092792
2016-12-23 08:44alostaleNote Added: 0092957
2016-12-23 08:44alostaleStatusresolved => closed
2016-12-23 08:44alostaleFixed in Version => 3.0PR17Q1

Notes
(0092757)
hgbot   
2016-12-19 10:37   
Repository: erp/devel/pi
Changeset: 8a7ca1c913edeb3aeb8ce0ed21748da90e234835
Author: Carlos Aristu <carlos.aristu <at> openbravo.com>
Date: Mon Dec 19 10:34:03 2016 +0100
URL: http://code.openbravo.com/erp/devel/pi/rev/8a7ca1c913edeb3aeb8ce0ed21748da90e234835 [^]

fixed issue 33816: OBCriteria adds standard filters several time

In some cases (ie. in count method), OBCriteria adds the standard filters for client, org and active several times. This problem is caused because initialize method can be invoked several times on the same criteria instance adding each time those filters.

This is fixed now by using two flags:
  - initialize: is set on 1st initialization and it is used to check if the criteria instance has already been initialized.
  - modified: it is used to identify if the criteria instance has been modified (by changing some filter or the order by clause) after being initialized. If it is false, we will exit the method without executing any action. Otherwise, the initialization will be done again. In this case a warning will be thrown in the log, informing that the generated qquery will have duplicated filters.

Note that still OBCriteria is not thread safe: several parallel initialization in the same instance could clash

---
M src-test/src/org/openbravo/test/dal/DalTest.java
M src/org/openbravo/dal/service/OBCriteria.java
---
(0092792)
hudsonbot   
2016-12-19 23:17   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/8c51303887c8 [^]
Maturity status: Test
(0092957)
alostale   
2016-12-23 08:44   
code reviewed

tested:

1. normal case: just once initialization
2. after executing query config is changed, second initialization is performed (ensuring backwards compatibility) and a warning is seen in logs