Move required exploded array query parameters into the exploded form expansion - #8101
Conversation
…xpansion
Required query parameters were always emitted outside the RFC 6570 expression as literal name={name} pairs, ignoring the explode modifier. An array typed query parameter with explode set to true, the OpenAPI default for query parameters, therefore comma joined its values instead of repeating the key. Array typed exploded parameters now move into the form expansion with the star modifier while scalars keep the required syntax, since scalar expansion is identical either way.
Fixes microsoft#8031
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
There was a problem hiding this comment.
Pull request overview
This PR fixes URL template generation in Kiota so required query parameters that are arrays and effectively explode: true are no longer emitted in the “required literal” form (name={name}), which cannot represent exploded arrays per RFC 6570. Instead, those parameters are moved into the RFC 6570 form-style query expansion ({?name*} / {&name*}), restoring repeated-key serialization for arrays (e.g., ids=1&ids=2&ids=3) while preserving the existing “required literal” semantics for scalar required parameters.
Changes:
- Update
OpenApiUrlTreeNodeExtensions.GetUrlTemplateto treat required exploded array query parameters as part of the{?...}/{&...}expression with the*modifier. - Add/extend unit tests to cover required exploded arrays (explicit/default explode), required scalars with
explode: true, optional exploded arrays, and mixed required/optional composition ordering. - Add a
CHANGELOG.mdentry documenting the behavior change and the RFC 6570 tradeoff when a required exploded array is unset.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs |
Moves required exploded array query params into {?...} / {&...} expansions via NeedsExplodedExpansion, preserving correct exploded serialization. |
tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs |
Adds regression coverage for required exploded arrays and mixed required/optional scenarios, ensuring scalars keep required literal syntax. |
CHANGELOG.md |
Documents the fix and the intentional tradeoff for unset required exploded arrays per RFC 6570 constraints. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
om singhal (@Om-singhaI) can you solve the conflicts on the changelog please? |
# Conflicts: # CHANGELOG.md
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
Thank you for making the changes!
|
The queue failures are not coming from this branch. The typescript idempotency job against the github description passed on the PR run and then failed in both queue attempts, and the only input that changed in between is the description itself, which the job downloads fresh from github/rest-api-description main. I can reproduce it locally on unmodified main at d5f667d: generating typescript from the current api.github.com.json crashes with |
GetUrlTemplateemits every required query parameter outside the RFC 6570 expression as a literalname={name}pair and never readsExplode. For an array typed parameter withexplode: truethat template comma joins the values (?ids=1,2,3) when the spec asks for a repeated key (?ids=1&ids=2&ids=3). SinceMicrosoft.OpenApidefaultsstyletoformfor query parameters andExplodeto true forformstyle, every unannotated required array query parameter hits this, and an explicitexplode: truein the document is ignored too.Vincent Biret (@baywet) already prescribed the fix on #4487 in 2024:
That issue went stale before a PR materialized. This picks it up, with one refinement: only array typed parameters leave the required bucket. For a scalar,
name={name}and{?name}expand to the samename=valuepair, so exploding a scalar changes nothing and moving scalars would only give up the required semantics introduced by #3989 for no serialization gain. The existing tests asserting?apikey={apikey}&filter={filter}for required strings (which haveExplode == trueby default) keep passing untouched, and a new test pins the required syntax for a scalar with an explicitexplode: true.Changes
OpenApiUrlTreeNodeExtensions.GetUrlTemplate: required query parameters that are array typed and exploded now land in the{?...}/{&...}expression with the*modifier, alongside the optional parameters. The rest of the template composition is untouched.explode), required scalar with explicitexplode: truekeeping the required syntax, optional exploded array unchanged, and a mix of required scalar, required exploded array, and optional parameter composing to?apikey={apikey}{&filter*,select*}.CHANGELOG.mdentry.Behavior tradeoff
A required exploded array the caller leaves unset now vanishes from the query string entirely, where the old template emitted a bare
ids=pair. RFC 6570 offers no syntax that is both outside an expression and exploded, so the template cannot express required and exploded at the same time; correct serialization of provided values wins over the empty placeholder for missing ones. The changelog entry calls this out.Full suite is green,
dotnet format --verify-no-changesis clean on the touched files.Fixes #8031
Closes #4487
Related: https://github.com/microsoft/kiota-dotnet/issues/934