Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
0030308Openbravo ERPA. Platformpublic2015-07-03 10:222015-09-25 10:16
caristu 
caristu 
urgentmajoralways
closedfixed 
5
 
3.0PR15Q43.0PR15Q4 
alostale
Core
Yes
0030308: Use get instead of refresh when saving or updating a record
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.
In description
Performance
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 
diff issue30308.diff (2,352) 2015-07-03 10:27
https://issues.openbravo.com/file_download.php?file_id=8260&type=bug
Issue History
2015-07-03 10:22caristuNew Issue
2015-07-03 10:22caristuAssigned To => platform
2015-07-03 10:22caristuModules => Core
2015-07-03 10:22caristuResolution time => 1436047200
2015-07-03 10:22caristuTriggers an Emergency Pack => No
2015-07-03 10:23caristuTag Attached: Performance
2015-07-03 10:23caristuRelationship addedrelated to 0029947
2015-07-03 10:24caristuAssigned Toplatform => caristu
2015-07-03 10:24caristuDescription Updatedbug_revision_view_page.php?rev_id=8972#r8972
2015-07-03 10:27caristuFile Added: issue30308.diff
2015-07-03 10:28caristuNote Added: 0078636
2015-07-03 15:45caristuIssue Monitored: alostale
2015-07-03 15:45caristuReview Assigned To => alostale
2015-07-03 15:57caristuTarget Version => 3.0PR15Q4
2015-07-03 16:05hgbotCheckin
2015-07-03 16:05hgbotNote Added: 0078640
2015-07-03 16:05hgbotStatusnew => resolved
2015-07-03 16:05hgbotResolutionopen => fixed
2015-07-03 16:05hgbotFixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/b26a24d2053e04743e90c430dc6f14098348651b [^]
2015-07-03 19:32hgbotCheckin
2015-07-03 19:32hgbotNote Added: 0078644
2015-07-03 19:42hgbotCheckin
2015-07-03 19:42hgbotNote Added: 0078645
2015-07-07 10:21alostaleNote Added: 0078699
2015-07-07 10:21alostaleStatusresolved => new
2015-07-07 10:21alostaleResolutionfixed => open
2015-07-07 10:46hgbotCheckin
2015-07-07 10:46hgbotNote Added: 0078700
2015-07-07 10:46hgbotStatusnew => resolved
2015-07-07 10:46hgbotResolutionopen => fixed
2015-07-07 10:46hgbotFixed in SCM revisionhttp://code.openbravo.com/erp/devel/pi/rev/b26a24d2053e04743e90c430dc6f14098348651b [^] => http://code.openbravo.com/erp/devel/pi/rev/9bedf4b77a0a8b7fbd9c6ea25bdf8fd881063835 [^]
2015-07-07 10:49alostaleNote Added: 0078701
2015-07-07 10:49alostaleStatusresolved => closed
2015-07-07 10:49alostaleFixed in Version => 3.0PR15Q4
2015-08-20 23:15hudsonbotCheckin
2015-08-20 23:15hudsonbotNote Added: 0079452
2015-08-20 23:15hudsonbotCheckin
2015-08-20 23:15hudsonbotNote Added: 0079455
2015-08-20 23:15hudsonbotCheckin
2015-08-20 23:15hudsonbotNote Added: 0079456
2015-08-20 23:16hudsonbotCheckin
2015-08-20 23:16hudsonbotNote Added: 0079479
2015-09-25 10:16alostaleTriggers an Emergency PackNo => Yes
2016-02-23 10:58alostaleRelationship addedcauses 0032308

Notes
(0078636)
caristu   
2015-07-03 10:28   
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   
Repository: erp/devel/pi
Changeset: ec134f647522a3f775fec72d35227deebce6fda4
Author: Carlos Aristu <carlos.aristu <at> openbravo.com>
Date: Fri Jul 03 19:41:40 2015 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/ec134f647522a3f775fec72d35227deebce6fda4 [^]

Related to issue 30308: fix in javadoc

---
M src/org/openbravo/dal/service/OBDal.java
---
(0078699)
alostale   
2015-07-07 10:21   
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
---
(0078701)
alostale   
2015-07-07 10:49   
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
(0079452)
hudsonbot   
2015-08-20 23:15   
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/8c91718397a5 [^]
Maturity status: Test
(0079455)
hudsonbot   
2015-08-20 23:15   
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/8c91718397a5 [^]
Maturity status: Test
(0079456)
hudsonbot   
2015-08-20 23:15   
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/8c91718397a5 [^]
Maturity status: Test
(0079479)
hudsonbot   
2015-08-20 23:16   
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/8c91718397a5 [^]
Maturity status: Test