Bug 877179 - revert changes to plugin api signature for PropertyDefinitionMap.getPropertyDefinitions()
Summary: revert changes to plugin api signature for PropertyDefinitionMap.getPropertyD...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: RHQ Project
Classification: Other
Component: Plugins, CLI, Usability
Version: 4.5
Hardware: All
OS: All
high
high
Target Milestone: ---
: ---,RHQ 4.6
Assignee: Jay Shaughnessy
QA Contact: Mike Foley
URL:
Whiteboard:
Depends On:
Blocks: 878661
TreeView+ depends on / blocked
 
Reported: 2012-11-15 21:36 UTC by Simeon Pinder
Modified: 2013-02-26 20:33 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 878661 (view as bug list)
Environment:
Last Closed: 2013-02-26 20:33:20 UTC
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 786416 0 urgent CLOSED Map configuration properties not shown in correct order 2021-02-22 00:41:40 UTC

Internal Links: 786416

Comment 1 Simeon Pinder 2012-11-15 21:40:39 UTC
Originally <=301
public Map<String, PropertyDefinition> getPropertyDefinitions() {

Now 310,311
public List<PropertyDefinition> getPropertyDefinitions() {
public Map<String, PropertyDefinition> getMap() {

Jay's Proposal (For Implementation)

Proposed:

1) Return to previous API but guarantee ordered set

// Now use a TreeMap that guarantees ordering whenyou call propDefs.keySet()
// See this before using TreeMap http://stackoverflow.com/questions/109383/how-to-sort-a-mapkey-value-on-the-values-in-java
public Map<String, PropertyDefinition> getPropertyDefinitions()
    
2) Remove (this will break new code, maybe there isn't any)
public List<PropertyDefinition> getPropertyDefinitions()

3) Keep, these are the bean getter/setters 
// does not guarantee ordering
public Map<String, PropertyDefinition> getMap()
public void setMap(@NotNull Map<String, PropertyDefinition> map)

4) Fix to actually provide ordering:
public List<PropertyDefinition> getSummaryPropertyDefinitions()

Comment 2 Jay Shaughnessy 2012-11-16 15:21:44 UTC
commit d94540025793528f2b32a7dcb07a3ed77336057f
Author: Jay Shaughnessy <jshaughn>
Date:   Fri Nov 16 10:16:24 2012 -0500

While maintaining the fix done for Bug 786416, restore the public API that
was broken in the original fix work.  Also, maintain as much of the API
introduced in that fix work just in case it is now in use.

Also fixed PropertyDefinitionMap.put(PropertyDefinition) to be more careful, and well-defined, about how it handles ordering.



TEST NOTES:
The QA for this is the same as that for Bug 786416, namely to ensure that ordering is correct for PropertyDefinitionMaps. Meaning, in general, that the top-to-bottom ordering in the plugin descriptor is the left-to-right ordering in the config editor.

Comment 3 Jay Shaughnessy 2012-11-16 17:30:45 UTC
The commit has in Comment 2 is wrong.  The correct commit hash to master is:

commit 4e2ff6a181ba78b6286353b3bbf1ad045643988d
Author: Jay Shaughnessy <jshaughn>
Date:   Fri Nov 16 10:16:24 2012 -0500

Comment 4 Lukas Krejci 2012-11-19 12:26:09 UTC
This breaks the API compatibility with RHQ 4.5.1 which was released with the changed API:

<diffreport>
  <difference binseverity="ERROR" srcseverity="ERROR" class="org.rhq.core.domain.configuration.definition.PropertyDefinitionMap" method="public java.util.List getPropertyDefinitions()">Return type of method 'public java.util.List getPropertyDefinitions()' has been changed to java.util.Map</difference>
</diffreport>

Because it looks like we want to keep the API as it was in 4.4.0, we need to make yet another intentional change xml file to not break the API check when checking against 4.5.1.

Comment 5 Jay Shaughnessy 2013-02-26 20:33:20 UTC
This work was completed a while back. Closing.


Note You need to log in before you can comment on or make changes to this bug.