Openbravo Issue Tracking System - Openbravo ERP |
| View Issue Details |
|
| ID | Project | Category | View Status | Date Submitted | Last Update |
| 0037843 | Openbravo ERP | 07. Sales management | public | 2018-02-06 15:54 | 2018-02-22 18:19 |
|
| Reporter | shuehner | |
| Assigned To | markmm82 | |
| Priority | normal | Severity | major | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | |
| Platform | | OS | 5 | OS Version | |
| Product Version | | |
| Target Version | | Fixed in Version | 3.0PR18Q2 | |
| Merge Request Status | |
| Review Assigned To | dmiguelez |
| OBNetwork customer | OBPS |
| Web browser | |
| Modules | Core |
| Support ticket | |
| Regression level | |
| Regression date | |
| Regression introduced in release | |
| Regression introduced by commit | |
| Triggers an Emergency Pack | No |
|
| Summary | 0037843: OrderEventHandler does useless and repeated queries when not needed |
| Description | That 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 Solution | Reorder checks to put the checks for the modification of the 4 columns first before doing any queries.
|
| Additional Information | |
| Tags | Performance |
| Relationships | |
| Attached Files | |
|
| 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 | OBNetwork customer | => No |
| 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 | OBNetwork customer | No => Yes |
| 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 | |
|
Notes |
|
|
|
|
Found in WBA performance work. |
|
|
|
|
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
---
|
|
|
|
|
|
|
|
|
|
|
|
|
|