Anonymous | Login
Project:
RSS
  
News | My View | View Issues | Roadmap | Summary

View Issue Details[ Jump to Notes ] [ Issue History ] [ Print ]
ID
0037843
TypeCategorySeverityReproducibilityDate SubmittedLast Update
defect[Openbravo ERP] 07. Sales managementmajorhave not tried2018-02-06 15:542018-02-22 18:19
ReportershuehnerView Statuspublic 
Assigned Tomarkmm82 
PrioritynormalResolutionfixedFixed in Version3.0PR18Q2
StatusclosedFix in branchFixed in SCM revision81e679715102
ProjectionnoneETAnoneTarget Version
OSAnyDatabaseAnyJava version
OS VersionDatabase versionAnt version
Product VersionSCM revision 
Review Assigned Todmiguelez
Web browser
ModulesCore
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary

0037843: 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.
TagsPerformance
Attached Files

- Relationships [ Relation Graph ] [ Dependency Graph ]

-  Notes
(0102221)
shuehner (administrator)
2018-02-06 15:54

 Found in WBA performance work.
(0102305)
markmm82 (developer)
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 (developer)
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 (developer)
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 (developer)
2018-02-12 10:00

 Code Review + Testing Ok
(0102744)
hudsonbot (developer)
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 (developer)
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

- Issue History
Date Modified Username Field Change
2018-02-06 15:54 shuehner New Issue
2018-02-06 15:54 shuehner Assigned To => Triage Finance
2018-02-06 15:54 shuehner Modules => Core
2018-02-06 15:54 shuehner Triggers an Emergency Pack => No
2018-02-06 15:54 shuehner Resolution time => 1519686000
2018-02-06 15:54 shuehner Note Added: 0102221
2018-02-06 16:39 shuehner Tag Attached: Performance
2018-02-06 22:44 markmm82 Assigned To Triage Finance => markmm82
2018-02-06 22:44 markmm82 Status new => acknowledged
2018-02-08 22:36 markmm82 Status acknowledged => scheduled
2018-02-09 17:48 markmm82 Note Added: 0102305
2018-02-12 09:57 hgbot Checkin
2018-02-12 09:57 hgbot Note Added: 0102321
2018-02-12 09:57 hgbot Status scheduled => resolved
2018-02-12 09:57 hgbot Resolution open => fixed
2018-02-12 09:57 hgbot Fixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/81e6797151023319bbc028198e33eba89e61698e [^]
2018-02-12 09:57 hgbot Checkin
2018-02-12 09:57 hgbot Note Added: 0102322
2018-02-12 10:00 dmiguelez Review Assigned To => dmiguelez
2018-02-12 10:00 dmiguelez Note Added: 0102323
2018-02-12 10:00 dmiguelez Status resolved => closed
2018-02-12 10:00 dmiguelez Fixed in Version => 3.0PR18Q2
2018-02-22 18:19 hudsonbot Checkin
2018-02-22 18:19 hudsonbot Note Added: 0102744
2018-02-22 18:19 hudsonbot Checkin
2018-02-22 18:19 hudsonbot Note Added: 0102745

Copyright © 2000 - 2009 MantisBT Group
Powered by Mantis Bugtracker