Fix FlowRuleComparator violating the Comparator contract#3629
Open
vasiliy-mikhailov wants to merge 1 commit into
Open
Fix FlowRuleComparator violating the Comparator contract#3629vasiliy-mikhailov wants to merge 1 commit into
vasiliy-mikhailov wants to merge 1 commit into
Conversation
FlowRuleComparator.compare returned 0 (equal) for rules that are not equal: - when one limitApp is null and the other is not (the early o1.getLimitApp() == null check returned 0 regardless of o2), and - when both limitApps are non-null, non-default and different (the final else returned 0). Returning 0 for unequal elements breaks the Comparator contract and can drop rules when they are stored in a sorted structure. Handle the null cases explicitly and compare two different non-default limitApps with String.compareTo.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
FlowRuleComparator.comparereturns0(equal) for rules that are not actually equal:So a rule whose
limitAppisnullcompares equal to any other rule, and two rules with different non-defaultlimitAppvalues (e.g.originAvsoriginB) compare equal. Returning0for unequal elements violates theComparatorcontract and can cause rules to be dropped when stored in a sorted structure.Fix
0only when bothlimitApps arenull; otherwise order anullconsistently against a non-null value.limitApps withString.compareToinstead of returning0.Test
Adds
testNullVsNonNullLimitAppandtestDifferentNonDefaultLimitAppstoFlowRuleComparatorTest, each also checking antisymmetry. Both fail on1.8(comparereturns0for unequal rules) and pass with this change.