Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0037843Openbravo ERP07. Sales managementpublic2018-02-06 15:542018-02-22 18:19
Reportershuehner 
Assigned Tomarkmm82 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target VersionFixed in Version3.0PR18Q2 
Merge Request Status
Review Assigned Todmiguelez
OBNetwork customer
Web browser
ModulesCore
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0037843: OrderEventHandler does useless and repeated queries when not needed
DescriptionThat EventHandler essentially watches changes to 4 columns of c_order table:
- newOrderDate
- newScheduledDate
- newWarehouseId
- newBPId

However it does 3 queries
- 2 ad_preference read
- c_orderline.count (related to order)

always without checking of those above columns are even modified.

Apart if several fields are modified together the code runs the orderLine.list() qeries up to 3 times (apart of the .count) above.

Steps To Reproduce- prepare to trace all queries
  - i.e. install org.openbravo.util.db module to trace
Do an c_order update via DAL of some unrelated columns
example:
a5d90620 7964475 [Import Entry - 6] INFO org.openbravo.util.db.StatementInvocationHandler - executeBatch --- SQL:
update C_Order set Updated=?, EM_Horcus_Status=? where C_Order_ID=?

Notice the extra queries:
   org.openbravo.event.OrderEventHandler.onUpdate(OrderEventHandler.java:72)
   org.openbravo.event.OrderEventHandler.onUpdate(OrderEventHandler.java:80)
   org.openbravo.event.OrderEventHandler.onUpdate(OrderEventHandler.java:90)

None of those should happen in that case.
Proposed SolutionReorder checks to put the checks for the modification of the 4 columns first before doing any queries.
Additional Information
TagsPerformance
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2018-02-06 15:54shuehnerNew Issue
2018-02-06 15:54shuehnerAssigned To => Triage Finance
2018-02-06 15:54shuehnerModules => Core
2018-02-06 15:54shuehnerTriggers an Emergency Pack => No
2018-02-06 15:54shuehnerResolution time => 1519686000
2018-02-06 15:54shuehnerNote Added: 0102221
2018-02-06 16:39shuehnerTag Attached: Performance
2018-02-06 22:44markmm82Assigned ToTriage Finance => markmm82
2018-02-06 22:44markmm82Statusnew => acknowledged
2018-02-08 22:36markmm82Statusacknowledged => scheduled
2018-02-09 17:48markmm82Note Added: 0102305
2018-02-12 09:57hgbotCheckin
2018-02-12 09:57hgbotNote Added: 0102321
2018-02-12 09:57hgbotStatusscheduled => resolved
2018-02-12 09:57hgbotResolutionopen => fixed
2018-02-12 09:57hgbotFixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/81e6797151023319bbc028198e33eba89e61698e [^]
2018-02-12 09:57hgbotCheckin
2018-02-12 09:57hgbotNote Added: 0102322
2018-02-12 10:00dmiguelezReview Assigned To => dmiguelez
2018-02-12 10:00dmiguelezNote Added: 0102323
2018-02-12 10:00dmiguelezStatusresolved => closed
2018-02-12 10:00dmiguelezFixed in Version => 3.0PR18Q2
2018-02-22 18:19hudsonbotCheckin
2018-02-22 18:19hudsonbotNote Added: 0102744
2018-02-22 18:19hudsonbotCheckin
2018-02-22 18:19hudsonbotNote Added: 0102745

Notes
(0102221)
shuehner   
2018-02-06 15:54   
Found in WBA performance work.
(0102305)
markmm82   
2018-02-09 17:48   
Test Plan:
Create a new Order header:
  Organization = F&B España, S.A
  Document No = 1000234
  Order Date = 09-02-2018
  Scheduled Delivery Date = 09-02-2018
  Business Partner = Alimentos y Supermercados, S.A
  Warehouse = España Región Norte
Save the Order header.
  Notice OrderEventHandler doesn't executes any query.
Create a new line, select any product and quantity.
Create a new line, select any product and quantity.
Go to header and change the Order date, for instance = 10-02-2018
  Notice:
   Just was executed one query to get the DoNotSyncDateOrdered preference was enabled or not.
   Just the orderDate column of the order lines was updated.
Go to header and change the Scheduled Delivery Date, for instance = 12-02-2018
  Notice:
   Just was executed one query to get the DoNotSyncDateDelivered preference was enabled or not.
   Just the scheduledDeliveryDate column of the order lines was updated.
Go to header and change the Warehouse, for instance = España Región Sur
  Notice:
   Just was executed one query to get the DoNotSyncWarehouse preference was enabled or not.
   Just the warehouse column of the order lines was updated.
Go to header and change the Business Partner, for instance = Hoteles Buenas Noches, S.A.
  Notice:
   Just was executed one query to remove discount information from the order.
   An error was thrown:
     Error
     Saving failed. Cannot change business partner or price list if there are lines.
  
Go to header and change any field different than Order date, Scheduled Delivery Date, Warehouse or Business Partner.
For instance Document No. = 1000235
  Notice:
   Any query has been executed.
   Any line has been updated.
  
Go to header and change Order date, Scheduled Delivery Date and Warehouse.
  Notice:
   Were executed three queries to get the value of DoNotSyncDateOrdered, DoNotSyncDateDelivered and DoNotSyncWarehouse preferences
   The three columns of each order lines have been updated.
(0102321)
hgbot   
2018-02-12 09:57   
Repository: erp/devel/pi
Changeset: 81e6797151023319bbc028198e33eba89e61698e
Author: Mark <markmm82 <at> gmail.com>
Date: Thu Feb 08 16:23:59 2018 -0500
URL: http://code.openbravo.com/erp/devel/pi/rev/81e6797151023319bbc028198e33eba89e61698e [^]

Fixes issue 37843: OrderEventHandler does useless and repeated queries when not
needed.

OrderEventHandler class has been refactorized:
- Removed unused vars.
- Renamed newWarehouseId to newWarehouse as it store a Warehouse object instead of an ID
- Renamed oldWarehouseId to oldWarehouse as it store a Warehouse object instead of an ID
- Only executed query to get preferences after verify that values have changed. This way
  it avoids to execute unneeded queries.
- Saved the orderLineCriteria.list() in a variable to avoid execute the same query more than once.
- Merged the three used loops updating fields in only one that updates fields if needed.
- Only loop is executed if at least one of the expected fields is changed and it is not
  activated the related preference.
- Created new methods to make code more cleaned and reuse in similar cases.

---
M src/org/openbravo/event/OrderEventHandler.java
---
(0102322)
hgbot   
2018-02-12 09:57   
Repository: erp/devel/pi
Changeset: 3caf66446c5c3f5adea8cda2c20835304a7d5519
Author: David Miguelez <david.miguelez <at> openbravo.com>
Date: Fri Feb 09 12:43:18 2018 +0100
URL: http://code.openbravo.com/erp/devel/pi/rev/3caf66446c5c3f5adea8cda2c20835304a7d5519 [^]

Related to Issue 37843. Code Review changes.

Refactor code to improve redability:
* Encapsulated order parameters into a sub class that can be
  shared between methods
* Split code of methods to reduce cognitive complexity
* Removed comments since the code is self explanatory enough

---
M src/org/openbravo/event/OrderEventHandler.java
---
(0102323)
dmiguelez   
2018-02-12 10:00   
Code Review + Testing Ok
(0102744)
hudsonbot   
2018-02-22 18:19   
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/980a6ad5bbf5 [^]
Maturity status: Test
(0102745)
hudsonbot   
2018-02-22 18:19   
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/980a6ad5bbf5 [^]
Maturity status: Test