Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0024326Openbravo ERPA. Platformpublic2013-07-12 14:512013-09-26 14:36
Reporteralostale 
Assigned Toalostale 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target VersionFixed in Version3.0MP28 
Merge Request Status
Review Assigned Toshankarb
OBNetwork customer
Web browser
ModulesCore
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0024326: Suboptimal parent tab getter for root tabs
DescriptionKernelUtils.getParentTab(Tab tab) method returns parent's tab for parameter tab.

In case parameter tab is a root tab it returns null.

The issue is on how root tab is calculated, instead of looking at the tab level being 0, it does the same logic as for the rest of the tabs and returns null in case nothing is found.

Even this calculation is quite fast, it is done many times (at least whenever a record is selected in grid).
Steps To Reproduce-
Proposed SolutionIn case tablevel is 0 return null without any further check
Additional Information
TagsPerformance
Relationships
related to defect 0024412 closed alostale Random failures in FIC 
related to defect 00244213.0MP28 closed alostale ApplicationDictionaryCachedStructures is not thread safe 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2013-07-12 14:51alostaleNew Issue
2013-07-12 14:51alostaleAssigned To => alostale
2013-07-12 14:51alostaleModules => Core
2013-07-12 14:51alostaleTriggers an Emergency Pack => No
2013-07-12 14:55alostaleNote Added: 0059957
2013-07-12 14:55alostaleTag Attached: Performance
2013-07-12 14:56alostaleNote Edited: 0059957bug_revision_view_page.php?bugnote_id=0059957#r4877
2013-07-12 14:58alostaleNote Added: 0059958
2013-07-12 15:02alostaleReview Assigned To => shankarb
2013-07-12 15:02alostaleSummarySuboptimal parent tab getter => Suboptimal parent tab getter for root tabs
2013-07-12 15:03hgbotCheckin
2013-07-12 15:03hgbotNote Added: 0059959
2013-07-12 15:03hgbotStatusnew => resolved
2013-07-12 15:03hgbotResolutionopen => fixed
2013-07-12 15:03hgbotFixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/b0cfc1e495435ccb1bd58f8e55bebb30bd83c634 [^]
2013-07-16 06:05shankarbNote Added: 0059988
2013-07-16 06:05shankarbStatusresolved => closed
2013-07-16 06:05shankarbFixed in Version => 3.0MP26
2013-07-16 14:26hudsonbotCheckin
2013-07-16 14:26hudsonbotNote Added: 0060038
2013-07-26 09:25egoitzNote Added: 0060297
2013-07-26 09:25egoitzStatusclosed => new
2013-07-26 09:25egoitzResolutionfixed => open
2013-07-26 09:25egoitzFixed in Version3.0MP26 =>
2013-07-26 09:29egoitzStatusnew => scheduled
2013-07-26 09:37egoitzStatusscheduled => resolved
2013-07-26 09:37egoitzResolutionopen => fixed
2013-07-26 09:37egoitzReview Assigned Toshankarb => egoitz
2013-07-26 09:37egoitzStatusresolved => closed
2013-07-26 10:48alostaleRelationship addedrelated to 0024412
2013-07-26 11:03alostaleNote Added: 0060300
2013-07-26 11:03alostaleStatusclosed => new
2013-07-26 11:03alostaleResolutionfixed => open
2013-07-26 11:08hgbotCheckin
2013-07-26 11:08hgbotNote Added: 0060301
2013-07-26 20:32hudsonbotCheckin
2013-07-26 20:32hudsonbotNote Added: 0060318
2013-07-29 12:27alostaleRelationship addedrelated to 0024421
2013-09-24 17:49hgbotCheckin
2013-09-24 17:49hgbotNote Added: 0061370
2013-09-24 17:49hgbotStatusnew => resolved
2013-09-24 17:49hgbotResolutionopen => fixed
2013-09-24 17:49hgbotFixed in SCM revisionhttp://code.openbravo.com/erp/devel/pi/rev/b0cfc1e495435ccb1bd58f8e55bebb30bd83c634 [^] => http://code.openbravo.com/erp/devel/pi/rev/d40d287ea91a0c156f9087d2d00fa03dd579a543 [^]
2013-09-25 08:26alostaleReview Assigned Toegoitz => shankarb
2013-09-25 19:56hudsonbotCheckin
2013-09-25 19:57hudsonbotNote Added: 0061394
2013-09-26 14:35shankarbNote Added: 0061414
2013-09-26 14:35shankarbStatusresolved => closed
2013-09-26 14:36shankarbFixed in Version => 3.0MP28

Notes
(0059957)
alostale   
2013-07-12 14:55   
(edited on: 2013-07-12 14:56)
Executing this test, which calculates parent's tab of root tabs before and after the fix:

    OBCriteria<Tab> qTab = OBDal.getInstance().createCriteria(Tab.class);
    qTab.add(Restrictions.eq(Tab.PROPERTY_TABLEVEL, 0L));
    qTab.addOrderBy(Tab.PROPERTY_ID, true);
    List<String> tabIds = new ArrayList<String>();
    for (Tab tab : qTab.list()) {
      tabIds.add(tab.getId());
    }

    long t = System.currentTimeMillis();
    for (String tabId : tabIds) {
      Tab tab = OBDal.getInstance().get(Tab.class, tabId);
      KernelUtils.getInstance().getParentTab(tab);
      // clear session not to reuse cached tab, so it is more similar to actual usage
      OBDal.getInstance().getSession().clear();
    }
    System.out.println("Time: " + (System.currentTimeMillis() - t));

Before: time 1500ms
After: time 360ms

Note in both cases ~300ms is spent in DB queries before the method, so after the fix the time is ~0 vs ~1200ms
(0059958)
alostale   
2013-07-12 14:58   
Regression risk none.

Executed test plan before and after the fix:

    OBCriteria<Tab> qTab = OBDal.getInstance().createCriteria(Tab.class);
    qTab.addOrderBy(Tab.PROPERTY_ID, true);
    List<String> tabIds = new ArrayList<String>();
    for (Tab tab : qTab.list()) {
      tabIds.add(tab.getId());
    }

    for (String tabId : tabIds) {
      Tab tab = OBDal.getInstance().get(Tab.class, tabId);
      System.out.println(tab + "-" + KernelUtils.getInstance().getParentTab(tab));
    }

and compared outputs, they are identical. Meaning this change doesn't modify returned parent tab for any of the ones in the system.
(0059959)
hgbot   
2013-07-12 15:03   
Repository: erp/devel/pi
Changeset: b0cfc1e495435ccb1bd58f8e55bebb30bd83c634
Author: Asier Lostalé <asier.lostale <at> openbravo.com>
Date: Fri Jul 12 15:03:06 2013 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/b0cfc1e495435ccb1bd58f8e55bebb30bd83c634 [^]

fixed bug 24326: Suboptimal parent tab getter for root tabs

---
M modules/org.openbravo.client.kernel/src/org/openbravo/client/kernel/KernelUtils.java
---
(0059988)
shankarb   
2013-07-16 06:05   
Code reviewed and tested in pi changeset 436cd2fa79fc.
(0060038)
hudsonbot   
2013-07-16 14:26   
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/9a5d5983399f [^]

Maturity status: Test
(0060297)
egoitz   
2013-07-26 09:25   
After apply that change the following error is continuosly raised:

17ed39e2 2013-07-23 05:28:31,588 [ajp-8809-7] ERROR org.hibernate.LazyInitializationException - could not initialize proxy - no Session
(0060300)
alostale   
2013-07-26 11:03   
Reopening the bug as it creates a regression: 0024412
(0060301)
hgbot   
2013-07-26 11:08   
Repository: erp/devel/pi
Changeset: 338d1021bc54b3e3658c8c3c7eae605600d7bf81
Author: Asier Lostalé <asier.lostale <at> openbravo.com>
Date: Fri Jul 26 11:07:34 2013 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/338d1021bc54b3e3658c8c3c7eae605600d7bf81 [^]

fixed bug 24412, backouts fix for bug 24326

---
M modules/org.openbravo.client.kernel/src/org/openbravo/client/kernel/KernelUtils.java
---
(0060318)
hudsonbot   
2013-07-26 20:32   
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/b9e43ea5aa1b [^]

Maturity status: Test
(0061370)
hgbot   
2013-09-24 17:49   
Repository: erp/devel/pi
Changeset: d40d287ea91a0c156f9087d2d00fa03dd579a543
Author: Asier Lostalé <asier.lostale <at> openbravo.com>
Date: Tue Jul 30 10:36:44 2013 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/d40d287ea91a0c156f9087d2d00fa03dd579a543 [^]

fixed bug 24421, fixed bug 24326: ADCachedStructures thread safe issues

-getTab now completely initializes all tabs in the window
-it is now synchronized

Added test cases to check it

---
M modules/org.openbravo.client.application/src/org/openbravo/client/application/window/ApplicationDictionaryCachedStructures.java
M modules/org.openbravo.client.kernel/src/org/openbravo/client/kernel/KernelUtils.java
M src-test/org/openbravo/test/AllAntTaskTests.java
A src-test/org/openbravo/test/base/HiddenObjectHelper.java
A src-test/org/openbravo/test/dal/ADCachedMultiThreadTest.java
---
(0061394)
hudsonbot   
2013-09-25 19:56   
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/a06bf757c7ec [^]

Maturity status: Test
(0061414)
shankarb   
2013-09-26 14:35   
Code reviewed and verified in pi changeset f497481ca351.