feat: add QueryParams multimap and OperationParams input-projection SPI#200
Merged
Conversation
Introduce a structured query-string model and the operation-input projection
seam, replacing the placeholder query type and the split('&') string surgery in
pagination.
- QueryParams (http.common): immutable, insertion-ordered, multi-valued query
model with RFC 3986 encoding (space -> %20, literal + -> %2B) and an
order-sensitive equals that agrees with encode(). It is an origination model
for building queries, not a fidelity-preserving URL editor.
- PercentEncoding (http.common): shared RFC 3986 URL-component codec used for
both query components and path segments (/ -> %2F, so a path value cannot
inject extra segments).
- OperationParams (operation): SPI that projects an operation's typed inputs
(path / query / header / body) into a Request and the context chain, via
toRequest(baseUrl) and toRequestContext(baseUrl, dispatch). Path templates
(/pets/{id}) are substituted with path-segment encoding; a missing variable
fails fast. Execution stays the pipeline's job.
- RequestRebuilder: query edits now splice the raw query string, preserving
untouched parameters byte-for-byte (a value-less ?flag stays value-less,
reserved characters are not rewritten) and encoding only the targeted
parameter; reads go through QueryParams.
- Remove the dead QueryParam placeholder stub and its test.
- Record the request-URL model decision (keep java.net.URL, layer QueryParams)
and document the new types in docs/http.md and the README package map.
Closes #28
Closes #29
Closes #57
Add a Request URL Model subsection (and TOC entry) under Cross-Cutting Design Decisions, summarising the keep-java.net.URL-plus-QueryParams choice and the preserved DNS-free equality, with a pointer to the full rationale in http.md. Relates to #29.
QueryParams.build() retained a name whose value list was empty, leaving an entry that reported contains() == true but rendered nothing from encode(). Two instances that encode() to the same string could therefore compare unequal and hash differently, breaking the documented "equal iff encode() identical" invariant. Skip names with no values when building, so a name added with an empty list never persists as a phantom entry. A value-less parameter keeps its single empty-string value and is unaffected.
OperationRequestAssembler appended the resolved path and query to the base URL verbatim, with no handling for a base URL that already carried a query. A signed base such as https://host/container?sig=... therefore had the path spliced into the query value (https://host/container?sig=.../pets?limit=20), corrupting both the path and the signature, and java.net.URL accepted the result so the breakage only surfaced at the server. Split any query off the base URL first, insert the resolved path before it, and append the operation query after it, so base + path + query compose correctly. Reject a fragment on the base URL (it cannot be composed with a path/query and is never sent on the wire), and translate a malformed base URL into IllegalArgumentException instead of leaking the checked MalformedURLException out of toRequest().
…position - encode reserved '*' as %2A and keep unreserved '~' literal in PercentEncoding - preserve a significant trailing slash and avoid a doubled '&' when composing operation request URLs over a base URL that already carries path/query - read a query param via a direct scan instead of building a throwaway QueryParams multimap per page
OperationParams exposes an operationName for instrumentation, but toRequestContext discarded it — it promoted the DispatchContext without passing the name on, so the value never reached the context chain that the HttpTracerFactory.newTracer(operationName, ...) seam reads from. A service that set operationName still produced unlabelled contexts. Thread operationName through the promotion chain: - RequestContext and ExchangeContext gain an optional operationName, forwarded on promotion (RequestContext -> ExchangeContext). - DispatchContext.toRequestContext takes an optional operationName; @jvmoverloads keeps the existing single-argument method. - OperationParams.toRequestContext passes its operationName, so a named operation labels its context for tracing without affecting the assembled request's URL, headers, or body. Regenerate the sdk-core API snapshot and update the Operation Input Projection docs. Tests cover propagation through each link of the chain.
OperationRequestAssembler left a trailing '&' from the base URL's query
in place when the operation contributed no query of its own, emitting a
dangling `?a=1&`. Trim a single trailing separator before merging so the
base query joins cleanly whether or not an operation query follows.
Widen the path-template variable pattern from `[^/}]` to `[^}]` so a
malformed `{a/b}` placeholder is captured whole and fails fast as a
missing path parameter, instead of leaking an unsubstituted `{a/b}`
literal into the assembled URL.
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
1 participant
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.
Summary
Adds a structured query-string model (
QueryParams) and the operation-input projection SPI (OperationParams), and routes pagination's query manipulation through proper primitives instead of placeholder /split('&')code.Motivation
QueryParamwas a placeholder stub, andRequestRebuildermanipulated query strings by splitting on&(single-valued only). There was also no structured way to project an operation's inputs — path, query, header, body — onto a request, which thin generated services and typed pagination need.What changed
QueryParams(http.common) — immutable, insertion-ordered, multi-valued query model (mirrorsHeaders):add/set/remove/addAll,get/values/names/entries/encode/parse. Encoding follows RFC 3986 (space →%20, literal+→%2B), notapplication/x-www-form-urlencoded. Equality is order-sensitive so it agrees withencode(). It is an origination model for building queries, not a fidelity-preserving URL editor.PercentEncoding(http.common) — shared RFC 3986 URL-component percent codec, used for both query components and path segments (/→%2F, so a path-parameter value cannot inject extra segments).OperationParams(operation) — SPI a service implements per operation to declare where each typed input belongs on the wire (path / query / header / body). The runtime assembles theRequest(toRequest(baseUrl)) and feeds it into the context chain (toRequestContext(baseUrl, dispatch)). Path templates (/pets/{id}) are substituted with path-segment encoding; a missing variable fails fast. Execution stays the pipeline's job — the SPI stops at producing the request/context.RequestRebuilder— query edits now splice the raw query string and copy untouched parameters byte-for-byte (a value-less?flagstays value-less; reserved characters are not rewritten), encoding only the targeted parameter; reads go throughQueryParams.Removed the dead
QueryParamplaceholder stub and its test.Docs / API —
docs/http.mdgains theQueryParams, Operation Input Projection, and Request URL Model sections; the README package map is updated;sdk-core.apiregenerated.Request URL model decision
Requestkeeps a resolvedjava.net.URL, withQueryParamslayered for query manipulation. This preserves the existing DNS-free equality (url.toExternalForm()) and puts a structured model where the manipulation pressure is. A deconstructed URL model /java.net.URImigration remains deferred until richer path handling earns it.Testing
All green. 66 new tests across
QueryParamsTest,PercentEncodingTest,OperationParamsTest, andRequestRebuilderTest. (The R8 shrink-survival guard in:sdk-shrink-testruns as part of a full./gradlew build/ CI.)Closes #28
Closes #29
Closes #57
Supersedes #147.