Openbravo Issue Tracking System - Openbravo ERP |
| View Issue Details |
|
| ID | Project | Category | View Status | Date Submitted | Last Update |
| 0030308 | Openbravo ERP | A. Platform | public | 2015-07-03 10:22 | 2015-09-25 10:16 |
|
| Reporter | caristu | |
| Assigned To | caristu | |
| Priority | urgent | Severity | major | Reproducibility | always |
| Status | closed | Resolution | fixed | |
| Platform | | OS | 5 | OS Version | |
| Product Version | | |
| Target Version | 3.0PR15Q4 | Fixed in Version | 3.0PR15Q4 | |
| Merge Request Status | |
| Review Assigned To | alostale |
| OBNetwork customer | |
| Web browser | |
| Modules | Core |
| Support ticket | |
| Regression level | |
| Regression date | |
| Regression introduced in release | |
| Regression introduced by commit | |
| Triggers an Emergency Pack | Yes |
|
| Summary | 0030308: Use get instead of refresh when saving or updating a record |
| Description | When saving or updating a record trough the UI, currently the OBDal.getInstance().getSession().refresh method is being executed in order to retrieve the objects from the db as they can have changed because of the execution of business event handler or database triggers.
Using OBDal.getInstance().get() performs better than OBDal.getInstance().getSession().refresh because of the query being executed currently in each case.
So this refresh() call should be replaced with get() when saving or updating a record. |
| Steps To Reproduce | In description |
| Proposed Solution | |
| Additional Information | |
| Tags | Performance |
| Relationships | | related to | design defect | 0029947 | | acknowledged | Triage Platform Base | Wrong query launched when OBDal.getInstance.getSession.refresh(Object) | | causes | defect | 0032308 | | closed | alostale | error on update if entity persistence observer loaded current object in memory |
|
| Attached Files |  issue30308.diff (2,352) 2015-07-03 10:27
https://issues.openbravo.com/file_download.php?file_id=8260&type=bug |
|
| Issue History |
| Date Modified | Username | Field | Change |
| 2015-07-03 10:22 | caristu | New Issue | |
| 2015-07-03 10:22 | caristu | Assigned To | => platform |
| 2015-07-03 10:22 | caristu | Modules | => Core |
| 2015-07-03 10:22 | caristu | Resolution time | => 1436047200 |
| 2015-07-03 10:22 | caristu | Triggers an Emergency Pack | => No |
| 2015-07-03 10:23 | caristu | Tag Attached: Performance | |
| 2015-07-03 10:23 | caristu | Relationship added | related to 0029947 |
| 2015-07-03 10:24 | caristu | Assigned To | platform => caristu |
| 2015-07-03 10:24 | caristu | Description Updated | bug_revision_view_page.php?rev_id=8972#r8972 |
| 2015-07-03 10:27 | caristu | File Added: issue30308.diff | |
| 2015-07-03 10:28 | caristu | Note Added: 0078636 | |
| 2015-07-03 15:45 | caristu | Issue Monitored: alostale | |
| 2015-07-03 15:45 | caristu | Review Assigned To | => alostale |
| 2015-07-03 15:57 | caristu | Target Version | => 3.0PR15Q4 |
| 2015-07-03 16:05 | hgbot | Checkin | |
| 2015-07-03 16:05 | hgbot | Note Added: 0078640 | |
| 2015-07-03 16:05 | hgbot | Status | new => resolved |
| 2015-07-03 16:05 | hgbot | Resolution | open => fixed |
| 2015-07-03 16:05 | hgbot | Fixed in SCM revision | => http://code.openbravo.com/erp/devel/pi/rev/b26a24d2053e04743e90c430dc6f14098348651b [^] |
| 2015-07-03 19:32 | hgbot | Checkin | |
| 2015-07-03 19:32 | hgbot | Note Added: 0078644 | |
| 2015-07-03 19:42 | hgbot | Checkin | |
| 2015-07-03 19:42 | hgbot | Note Added: 0078645 | |
| 2015-07-07 10:21 | alostale | Note Added: 0078699 | |
| 2015-07-07 10:21 | alostale | Status | resolved => new |
| 2015-07-07 10:21 | alostale | Resolution | fixed => open |
| 2015-07-07 10:46 | hgbot | Checkin | |
| 2015-07-07 10:46 | hgbot | Note Added: 0078700 | |
| 2015-07-07 10:46 | hgbot | Status | new => resolved |
| 2015-07-07 10:46 | hgbot | Resolution | open => fixed |
| 2015-07-07 10:46 | hgbot | Fixed in SCM revision | http://code.openbravo.com/erp/devel/pi/rev/b26a24d2053e04743e90c430dc6f14098348651b [^] => http://code.openbravo.com/erp/devel/pi/rev/9bedf4b77a0a8b7fbd9c6ea25bdf8fd881063835 [^] |
| 2015-07-07 10:49 | alostale | Note Added: 0078701 | |
| 2015-07-07 10:49 | alostale | Status | resolved => closed |
| 2015-07-07 10:49 | alostale | Fixed in Version | => 3.0PR15Q4 |
| 2015-08-20 23:15 | hudsonbot | Checkin | |
| 2015-08-20 23:15 | hudsonbot | Note Added: 0079452 | |
| 2015-08-20 23:15 | hudsonbot | Checkin | |
| 2015-08-20 23:15 | hudsonbot | Note Added: 0079455 | |
| 2015-08-20 23:15 | hudsonbot | Checkin | |
| 2015-08-20 23:15 | hudsonbot | Note Added: 0079456 | |
| 2015-08-20 23:16 | hudsonbot | Checkin | |
| 2015-08-20 23:16 | hudsonbot | Note Added: 0079479 | |
| 2015-09-25 10:16 | alostale | Triggers an Emergency Pack | No => Yes |
| 2016-02-23 10:58 | alostale | Relationship added | causes 0032308 |
|
Notes |
|
|
|
|
Proposed solution pushed to try. Attached the file with that solution. |
|
|
|
(0078640)
|
|
hgbot
|
|
2015-07-03 16:05
|
|
Repository: erp/devel/pi
Changeset: b26a24d2053e04743e90c430dc6f14098348651b
Author: Carlos Aristu <carlos.aristu <at> openbravo.com>
Date: Fri Jul 03 16:04:19 2015 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/b26a24d2053e04743e90c430dc6f14098348651b [^]
Fixes issue 30308: Use get instead of refresh when saving or updating a record
We use now OBDal.getInstance().get() instead of OBDal.getInstance().getSession().refresh(). To ensure that the get() methods retrieves the last changes in the object from database, we call evict() to remove the bob from the cache
---
M modules/org.openbravo.service.json/src/org/openbravo/service/json/DefaultJsonDataService.java
---
|
|
|
|
(0078644)
|
|
hgbot
|
|
2015-07-03 19:32
|
|
Repository: erp/devel/pi
Changeset: 2268a549839c8b21ffd5c39f0de3056a721c5887
Author: Carlos Aristu <carlos.aristu <at> openbravo.com>
Date: Fri Jul 03 19:31:11 2015 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/2268a549839c8b21ffd5c39f0de3056a721c5887 [^]
Related to issue 30308: created new refresh method in OBDal
Place the solution in a new method in OBDal, to allow the usage of this approach in similar cases
---
M modules/org.openbravo.service.json/src/org/openbravo/service/json/DefaultJsonDataService.java
M src/org/openbravo/dal/service/OBDal.java
---
|
|
|
|
(0078645)
|
|
hgbot
|
|
2015-07-03 19:42
|
|
|
|
|
|
Reopening due to code review issues:
New OBDal.refresh(BaseOBObject, boolean) has some problems:
* Unlike OBDal.refresh(Object) method, it does not refresh current Object instance but creates a new one which is returned
* Parameter object instance can be evicted from DAL's cache, so reusing it after calling this method can result in a LazyInitializationException
* If parameter object is already in DAL's cache, refreshing with useCache=false has no effect
* If parameter object is not yet initialized bob.getId() might result in a LazyInitializationException |
|
|
|
(0078700)
|
|
hgbot
|
|
2015-07-07 10:46
|
|
Repository: erp/devel/pi
Changeset: 9bedf4b77a0a8b7fbd9c6ea25bdf8fd881063835
Author: Asier Lostalé <asier.lostale <at> openbravo.com>
Date: Tue Jul 07 10:45:53 2015 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/9bedf4b77a0a8b7fbd9c6ea25bdf8fd881063835 [^]
fixed issue 30308: use get to reload grid object on save/update
Backed out changesets: 2268a549839c2268a549839c & ec134f647522ec134f647522
because New OBDal.refresh(BaseOBObject, boolean) had some problems:
* Unlike OBDal.refresh(Object) method, it does not refresh current Object
instance but creates a new one which is returned
* Parameter object instance can be evicted from DAL's cache, so reusing it
after calling this method can result in a LazyInitializationException
* If parameter object is already in DAL's cache, refreshing with useCache=false
has no effect
* If parameter object is not yet initialized bob.getId() might result in a
LazyInitializationException
After those backouts code remains as it was originally doing the evict and get
within DefaultJsonDataService.update method.
---
M modules/org.openbravo.service.json/src/org/openbravo/service/json/DefaultJsonDataService.java
M src/org/openbravo/dal/service/OBDal.java
---
|
|
|
|
|
code reviewed
closing after reverting last 2 changesets
tested for update and insert, in both cases the record is retrieved from DB without extra joins |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|