-
-
Notifications
You must be signed in to change notification settings - Fork 139
Description
I was looking at the contribution manager code and noticed the Contribution class has a problematic equals() and hashCode() implementation. There's even a TODO comment calling this out (line 352 in Contribution.java).
The issue is both methods only look at the name field (case-insensitive), which means two contributions with the same name but different versions get treated as identical. This becomes a problem because the code puts Contribution objects in HashSets and uses ConcurrentHashMap.newKeySet() in several places.
Here's what I'm seeing in ContributionListing.java:
java
Set<Contribution> allContribs = ConcurrentHashMap.newKeySet(); // line 71
Set<AvailableContribution> availableContribs = new HashSet<>(); // line 66
And the current implementation:
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o instanceof Contribution that) {
return name.equalsIgnoreCase(that.name);
}
return false;
}
// TODO remove this because it hides actual differences in objects
@Override
public int hashCode() {
return name.toLowerCase().hashCode();
}
This means if you try to track both an installed version of "Sound" library v1.0 and an available update v1.1, the HashSet will treat them as duplicates. So contribs.add(updateVersion) might just silently not add it because it thinks it's already there.
I ran into this while tracing through replaceContribution() which does a remove() then add() - but if the hashCode is only based on name, the remove might pull out the wrong object or the add might fail
Possible fixes:
-
Include version/type in the hashCode and equals
-
Just remove the overrides and use object identity
-
Add a separate hasSameName() method for name comparisons and stop overriding equals/hashCode
Option 3 seems cleanest since the code probably needs both behaviors - sometimes you want to check "is this the same contribution?" and sometimes "does a contribution with this name exist?"
This affects Contribution.java, ContributionListing.java, and probably ListPanel.java too (it uses a Map keyed by Contribution objects),