Project:
View Issue Details[ Jump to Notes ] | [ Issue History ] [ Print ] | |||||||
ID | ||||||||
0020129 | ||||||||
Type | Category | Severity | Reproducibility | Date Submitted | Last Update | |||
defect | [Openbravo ERP] A. Platform | major | always | 2012-03-27 17:44 | 2012-05-25 12:10 | |||
Reporter | mirurita | View Status | public | |||||
Assigned To | AugustoMauch | |||||||
Priority | normal | Resolution | fixed | Fixed in Version | 3.0MP12 | |||
Status | closed | Fix in branch | Fixed in SCM revision | 0a67797edc70 | ||||
Projection | none | ETA | none | Target Version | ||||
OS | Any | Database | Any | Java version | ||||
OS Version | Database version | Ant version | ||||||
Product Version | pi | SCM revision | ||||||
Review Assigned To | ||||||||
Web browser | ||||||||
Modules | Core | |||||||
Regression level | ||||||||
Regression date | ||||||||
Regression introduced in release | ||||||||
Regression introduced by commit | ||||||||
Triggers an Emergency Pack | No | |||||||
Summary | 0020129: OBContext class getters returning a reference to the class instance attributes (not a new instance of the attribute) | |||||||
Description | OrganizationStructureProvider 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 Reproduce | 1) 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 Solution | Return 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; } | |||||||
Tags | No tags attached. | |||||||
Attached Files | 20129.diff [^] (1,772 bytes) 2012-03-28 07:23 [Show Content]
issue20129.diff [^] (11,044 bytes) 2012-04-24 17:53 [Show Content] | |||||||
Relationships [ Relation Graph ] [ Dependency Graph ] | ||||||||
|
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 (administrator) 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 (administrator) 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 |