Repository navigation
Serve the exact-matching mock instead of a partial match - #912
Open
eastagiletracker wants to merge 1 commit into
Open
eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
BestMatchComparator was not antisymmetric, so a partial match with more matched params could sort ahead of an exact match, and returnResponse served the head of the sorted list even when it was not the exact match. Rank full matches first, then by matched param count, and serve the first exact match.
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.
This PR proposes serving the exact-matching mock instead of a partial match when a virtualized API picks its response. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/710. You can sign in with your GitHub ID to claim ownership of the project.
What was wrong
When a request fully matches one mock and only partially matches another mock that matched more params, the plugin can serve the partial one. Example: mock A has
status=available, mock B hasstatus=available&tag=x&name=q, and the request is?status=available&tag=x. A is the exact match (1 of 1 params), B is a partial match (2 of 3).VirtualServiceUtil.returnResponsechecks that an exact match exists, but thengetResponseserves the head of the list sorted byBestMatchComparator, and that comparator is not antisymmetric:compare(A, B)andcompare(B, A)both return-1, so the sort just reverses the two and B can come out first. The mocks are collected in aHashMapkeyed byMockRequest, whose LombokhashCodegoes through identity-hashed values (VirtualServiceKeyValuehas nohashCode, and the enum field), so which mock wins can change from one load to the next.The change
BestMatchComparatornow defines a consistent order: mocks whose every param matched come first, then more matched params before fewer. Ties keep their original order.returnResponseserves the first exact match from the sorted list instead of the list head, because an exact match does not always sort first (e.g. an exact match on a request with no params carries no param count). The existing publicgetResponse(Method, List)still serves the head of the list and now delegates to a newgetResponse(Method, ReturnMockResponse)overload, so existing callers behave the same.Reproduction and verification
The module had no tests, so this adds
VirtualServiceUtilTestandBestMatchComparatorTest(JUnit 5 and Mockito from the existingspring-boot-starter-testdependency).servesExactMatchThroughRealMatcherdrives the realVirtualServiceValidRequestandVirtualServiceParamComparatorwith onlyVirtualServicemocked, and repeats the request against freshly loaded mocks. On currentmaster(bdbcae6) with the new tests and no fix,mvn -B verifyfails:With only the comparator fix applied,
servesExactMatchThatHasNoParamsAheadOfPartialMatchstill fails (expected: <exact> but was: <partial>), so each half of the change has a test that needs it. With both changes,mvn -B verifyon JDK 17 passes:Tests run: 11, Failures: 0, Errors: 0, Skipped: 0,BUILD SUCCESS. The baseline build of the untouched tree was also green (it had no tests to run). The tests also cover the not-found path when no mock matches exactly and the most specific exact match winning in every input order. One side effect worth knowing about: the duplicate check inisMockAlreadyExistsalso sorts withBestMatchComparator, so it now checks the full match first too.How this was managed
This work is tracked as a story on a board imported from this repository's issues and pull requests (911 stories): https://eastagiletracker.com/projects/710/stories/653353 on the board https://eastagiletracker.com/projects/710.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com