-
Notifications
You must be signed in to change notification settings - Fork 20
Copy over existing request query params when creating a new request #2547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Generate changelog in
|
| ListMultimap<String, String> mutable = | ||
| Multimaps.newListMultimap(new LinkedHashMap<>(), MAP_VALUE_FACTORY); | ||
| if (!queryParams.isEmpty()) { | ||
| queryParams.forEach(mutable::put); | ||
| } | ||
| queryParams = mutable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a test case that would fail before and now succeeds as a result of this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added!
|
This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days. |
| .putQueryParams("query1", "queryVal1") | ||
| .putHeaderParams("header1", "headerVal1") | ||
| .putPathParams("path1", "pathVal1") | ||
| .build(); | ||
|
|
||
| Request newRequest = Request.builder() | ||
| .from(oldRequest) | ||
| .putQueryParams("query2", "queryVal2") | ||
| .putHeaderParams("header2", "headerVal2") | ||
| .putPathParams("path2", "pathVal2") | ||
| .build(); | ||
|
|
||
| // make sure new query/header/path parameters were added in addition to whatever was in `oldRequest` | ||
| assertThat(newRequest) | ||
| .extracting(Request::queryParams) | ||
| .extracting(Multimap::entries, collection(Map.Entry.class)) | ||
| .containsExactlyInAnyOrder( | ||
| MapEntry.entry("query1", "queryVal1"), MapEntry.entry("query2", "queryVal2")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be good to also test unique query param in each of old & new requests as well as duplicate query param names (since they're a ListMultimap)
|
This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days. |
Before this PR
Request objects do not copy over query parameters when the builder is created from an existing object
After this PR
==COMMIT_MSG==
When request objects are created using a builder on an existing object, query parameters will be copied over from the previous object to match the headers and path params implementation
==COMMIT_MSG==