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

View Issue DetailsJump to Notes ] Issue History ] Print ]
ID
0020129
TypeCategorySeverityReproducibilityDate SubmittedLast Update
defect[Openbravo ERP] A. Platformmajoralways2012-03-27 17:442012-05-25 12:10
ReportermiruritaView Statuspublic 
Assigned ToAugustoMauch 
PrioritynormalResolutionfixedFixed in Version3.0MP12
StatusclosedFix in branchFixed in SCM revision0a67797edc70
ProjectionnoneETAnoneTarget Version
OSAnyDatabaseAnyJava version
OS VersionDatabase versionAnt version
Product VersionpiSCM revision 
Review Assigned To
Web browser
ModulesCore
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary

0020129: OBContext class getters returning a reference to the class instance attributes (not a new instance of the attribute)

DescriptionOrganizationStructureProvider
OBContext class stores for all accessible clients the corresponding OrganizationStructureProvider.

Using the public API if you get the OrganizationStructureProvider of your client and then you query (i.e.) for the children organization the object you get is a reference of the attribute of OrganizationStructureProvider (not a new instance).

Set<String> orgIds = OBContext.getOBContext().getOrganizationStructureProvider() .getChildOrg(legalEntity.getId());

// When you add a new element to the list you are modifying the "global" list
orgIds.add(legalEntity.getId());


In OBContext class there are more examples:
 private String[] readableOrganizations;
 public String[] getReadableOrganizations()

 private String[] readableClients;
 public String[] getReadableClients()

...

Review all the attributes of OBContext class.
Steps To Reproduce1) Check the value of the following attribute of OrganizationStructureProvider class:
private Map<String, Set<String>> childByOrganizationID = new HashMap<String, Set<String>>();

2) Execute this code
Set<String> orgIds = OBContext.getOBContext().getOrganizationStructureProvider() .getChildOrg(legalEntity.getId());
orgIds.add("MY CODE");

3) Check again the value of:
private Map<String, Set<String>> childByOrganizationID = new HashMap<String, Set<String>>();

Realize that "MY CODE" is a new element of the Map.
Proposed SolutionReturn a new instance of the attribute, for example:

// BEFORE
  public Set<String> getNaturalTree(String orgId) {
    initialize();
    Set<String> result = naturalTreesByOrgID.get(orgId);
    if (result == null) {
      result = new HashSet<String>();
      result.add(orgId);
    }
    return result;
  }

// AFTER
  public Set<String> getNaturalTree(String orgId) {
    initialize();
    Set<String> result;
    if (naturalTreesByOrgID.get(orgId) == null) {
      result = new HashSet<String>();
      result.add(orgId);
    } else {
      result = new HashSet<String>(naturalTreesByOrgID.get(orgId));
    }
    return result;
  }
TagsNo tags attached.
Attached Filesdiff file icon 20129.diff [^] (1,772 bytes) 2012-03-28 07:23 [Show Content]
diff file icon issue20129.diff [^] (11,044 bytes) 2012-04-24 17:53 [Show Content]

- Relationships Relation Graph ] Dependency Graph ]
blocks defect 0020424 closedAugustoMauch Change role takes 10 secons since you press "Apply" button 

-  Notes
(0048313)
hgbot (developer)
2012-05-03 11:45

Repository: erp/devel/pi
Changeset: 0a67797edc7089222d09584f5254666e98b2a28f
Author: Augusto Mauch <augusto.mauch <at> openbravo.com>
Date: Thu May 03 11:42:58 2012 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/0a67797edc7089222d09584f5254666e98b2a28f [^]

Fixes issue 20129: OBContext and OSP return copies of attributes

Now OBContext and OrganizationStructureProvider getter methods do not return a reference to its instance attributes, but a copy.

---
M src-test/org/openbravo/test/dal/IssuesTest.java
M src/org/openbravo/dal/core/OBContext.java
M src/org/openbravo/dal/security/OrganizationStructureProvider.java
---
(0048314)
AugustoMauch (manager)
2012-05-03 11:49

Test plan:

Check that OBContext and OrganizationStructureProvider are getter methods return copies, not references.

Some tests have been included in IssuesTest.java to check this.
(0048316)
AugustoMauch (manager)
2012-05-03 12:15

This issue might be risky.

If someone was modifying the object returned by a getter taking for granted that the original object would be modified, that will no longer happen. They should use the setter methods instead.

I have checked that this is not done in the Openbravo ERP, but I can not know if someone outside Openbravo is doing it.
(0048354)
hgbot (developer)
2012-05-03 17:10

Repository: erp/devel/pi
Changeset: 1eea578bf5892efae85d5c0f81a12c485a52ab65
Author: Augusto Mauch <augusto.mauch <at> openbravo.com>
Date: Thu May 03 17:08:44 2012 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/1eea578bf5892efae85d5c0f81a12c485a52ab65 [^]

Related to issue 20129: Tests have been cleaned.

Two tests have been removed, because finally no changes were done to the getOrganizationStructureProvider and getEntityAccessChecker methods, so they are still returning reference and not copies (this has be
en done deliberately).

Another test has been commented out, because it has not been possible to get a user context whose getWarehouse method did not return a null value.

---
M src-test/org/openbravo/test/dal/IssuesTest.java
---
(0048404)
hgbot (developer)
2012-05-07 11:18

Repository: erp/devel/pi
Changeset: 98ca8ddc5e0ef280ba10c5a94bdf3fbd00295f0d
Author: Augusto Mauch <augusto.mauch <at> openbravo.com>
Date: Mon May 07 11:17:14 2012 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/98ca8ddc5e0ef280ba10c5a94bdf3fbd00295f0d [^]

Fixes issue 20424: DalUtil copies reverted

In the fix of issue 20129 some DalUtil copies were introduced. They are very time-consuming and have a strong impact in performance, so they have been reverted.

---
M src-test/org/openbravo/test/dal/IssuesTest.java
M src/org/openbravo/dal/core/OBContext.java
---
(0048673)
alostale (manager)
2012-05-16 08:41

Code reviewed and tested on pi@d4ef642f1e2a
(0048998)
hudsonbot (developer)
2012-05-25 12:09

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/5401e185a8b0 [^]

Maturity status: Test
(0049017)
hudsonbot (developer)
2012-05-25 12:09

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/5401e185a8b0 [^]

Maturity status: Test
(0049034)
hudsonbot (developer)
2012-05-25 12:10

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/5401e185a8b0 [^]

Maturity status: Test

- Issue History
Date Modified Username Field Change
2012-03-27 17:44 mirurita New Issue
2012-03-27 17:44 mirurita Assigned To => alostale
2012-03-27 17:44 mirurita Modules => Core
2012-03-28 07:23 iperdomo File Added: 20129.diff
2012-03-28 07:40 iperdomo Target Version => 3.0MP8.2
2012-03-28 10:39 alostale Target Version 3.0MP8.2 => 3.0MP11
2012-04-18 11:29 alostale Assigned To alostale => AugustoMauch
2012-04-24 09:37 AugustoMauch Target Version 3.0MP11 =>
2012-04-24 17:53 AugustoMauch File Added: issue20129.diff
2012-05-03 11:45 hgbot Checkin
2012-05-03 11:45 hgbot Note Added: 0048313
2012-05-03 11:45 hgbot Status new => resolved
2012-05-03 11:45 hgbot Resolution open => fixed
2012-05-03 11:45 hgbot Fixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/0a67797edc7089222d09584f5254666e98b2a28f [^]
2012-05-03 11:49 AugustoMauch Note Added: 0048314
2012-05-03 12:15 AugustoMauch Note Added: 0048316
2012-05-03 17:10 hgbot Checkin
2012-05-03 17:10 hgbot Note Added: 0048354
2012-05-04 17:57 alostale Relationship added blocks 0020424
2012-05-07 11:18 hgbot Checkin
2012-05-07 11:18 hgbot Note Added: 0048404
2012-05-16 08:41 alostale Note Added: 0048673
2012-05-16 08:41 alostale Status resolved => closed
2012-05-16 08:41 alostale Fixed in Version => 3.0MP12
2012-05-25 12:09 hudsonbot Checkin
2012-05-25 12:09 hudsonbot Note Added: 0048998
2012-05-25 12:09 hudsonbot Checkin
2012-05-25 12:09 hudsonbot Note Added: 0049017
2012-05-25 12:10 hudsonbot Checkin
2012-05-25 12:10 hudsonbot Note Added: 0049034


Copyright © 2000 - 2009 MantisBT Group
Powered by Mantis Bugtracker