api: fix inverted value comparison in ImageStoreDetailResponse.equals - #14023
Conversation
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
seems this is unused code as no error reports have ever been filed, but changed clgtm
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #14023 +/- ##
============================================
+ Coverage 16.34% 19.79% +3.44%
- Complexity 13574 20000 +6426
============================================
Files 5669 6371 +702
Lines 501368 575899 +74531
Branches 60903 70495 +9592
============================================
+ Hits 81964 113975 +32011
- Misses 410219 449491 +39272
- Partials 9185 12433 +3248
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
equals() returned false when the two responses had the SAME value and true
when the values DIFFERED (the value branch was inverted):
else if (this.getValue().equals(other.getValue()))
return false;
So two identical details were treated as unequal and two details differing
only by value were treated as equal, corrupting any Set/Map/dedup keyed on
ImageStoreDetailResponse. It also NPEd when value was null.
Compare with !Objects.equals(getValue(), other.getValue()), which restores
the correct result and is null-safe.
Adds tests for equal name+value, differing value, and differing name.
6498f93 to
1415c74
Compare
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19089 |
|
@blueorangutan test |
|
@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-16948)
|
|
not related failures in the smoke tests. |
4 participants
Description
ImageStoreDetailResponse.equals()had the value comparison inverted - itreturned
falsewhen the two responses had the same value andtruewhenthe values differed:
So two identical details were treated as unequal, and two details differing only
by value were treated as equal — which corrupts any Set/Map/dedup keyed on
ImageStoreDetailResponse. It also threw an NPE when the value was null.Fixed by comparing with
!Objects.equals(getValue(), other.getValue()), whichrestores the correct result and is null-safe.
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
How Has This Been Tested?
Added unit tests covering equal name+value, differing value, and differing name.
Also built the standard packages and deployed on a KVM advanced zone.