[http-client-java] Support collection header prefixes - #11860
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 564aeb75-0897-455b-9e8c-d8c635d43f8c
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
|
Review follow-up: the generated header collection deserializer currently matches prefixes case-sensitively: headerName.startsWith("x-ms-meta-")HTTP header names are case-insensitive, so mixed-case headers such as I plan to enhance |
There was a problem hiding this comment.
🔵 Needs a closer look
The new collectionHeaderPrefix option is only wired for response headers in the emitter and does not yet propagate to request header parameters even though generator-core supports x-ms-header-collection-prefix there.
Pull request overview
Adds support in the TypeSpec Java emitter for mapping a collectionHeaderPrefix client option onto the code model so the Java generator can deserialize prefixed header collections into Map<String, String> (validated via generator-test scenario and E2E mock test).
Changes:
- Emit
x-ms-header-collection-prefixon response headers whencollectionHeaderPrefixis set for a map-valued header. - Add a TypeSpec scenario using
@@alternateType(..., Record<string>, "java")+collectionHeaderPrefixon a header model property. - Extend the Java generator-test E2E test to validate deserialization of
x-ms-meta-*headers into a map and add a.chronusfeature entry.
File summaries
| File | Description |
|---|---|
| packages/http-client-java/generator/http-client-generator-test/tsp/response-headers.tsp | Adds a map-valued response header scenario using collectionHeaderPrefix. |
| packages/http-client-java/generator/http-client-generator-test/src/test/java/tsptest/responseheaders/ResponseHeadersTests.java | Adds E2E assertions for prefixed header collection deserialization into a Map. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/responseheaders/ResponseHeadersClient.java | Regenerated client docs to include x-ms-meta header model surface. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/responseheaders/ResponseHeadersAsyncClient.java | Regenerated async client docs to include x-ms-meta header model surface. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/responseheaders/models/ResponseHeaderOpsGetResourceMetadataHeaders.java | Regenerated headers model showing prefix-based header collection deserialization. |
| packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/responseheaders/implementation/ResponseHeaderOpsImpl.java | Regenerated impl docs to include the new header model surface. |
| packages/http-client-java/emitter/src/code-model-builder.ts | Emits x-ms-header-collection-prefix for response headers based on collectionHeaderPrefix. |
| .chronus/changes/http-client-java-collection-header-prefix-2026-09-04.md | Adds a feature changelog entry for @typespec/http-client-java. |
Review details
Suppressed comments (1)
packages/http-client-java/emitter/src/code-model-builder.ts:2348
collectionHeaderPrefixis currently only mapped onto response headers;processParameter(...)never setsx-ms-header-collection-prefixfor map-valued header parameters, even though generator-core already consumes that extension for request header serialization (e.g.ProxyParameterMapperreadsparameter.getExtensions().getXmsHeaderCollectionPrefix()in.../ProxyParameterMapper.java:48-52). This makes the new option inconsistent depending on whether it's applied to a response header vs a request header parameter.
const collectionHeaderPrefix = this.getCollectionHeaderPrefix(header);
const httpHeader = new HttpHeader(header.serializedName, schema, {
language: {
default: {
name: header.name,
- Files reviewed: 8/8 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 564aeb75-0897-455b-9e8c-d8c635d43f8c
|
Implemented in The generated deserializer now lowercases each runtime header name with The mock test now uses |
There was a problem hiding this comment.
🟡 Changes recommended
It appears to close #11859 while only implementing the response side and it lacks automated tests that exercise the emitter/generator output paths (beyond checked-in generated baselines).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
packages/http-client-java/emitter/src/code-model-builder.ts:2354
- This only maps
collectionHeaderPrefixtox-ms-header-collection-prefixfor response headers. Issue #11859 describes request header collection serialization as part of the same feature (“Request serialization using the collection prefix for map-valued header parameters”), soFixes #11859looks incomplete unless header parameters also get this extension in the code model.
This issue also appears on line 3779 of the same file.
packages/http-client-java/emitter/src/code-model-builder.ts:3782
- New behavior (
getCollectionHeaderPrefix+ emittingx-ms-header-collection-prefix) isn’t covered by the emitter’s Vitest suite; the current tests underpackages/http-client-java/emitter/testdon’t exerciseCodeModelBuilderoutput, so regressions here won’t be caught without re-running full code generation. Per the repo test guidance, add a focused emitter test that compiles a minimal TypeSpec with@@clientOption(..., "collectionHeaderPrefix", ...)and asserts the produced code model includesx-ms-header-collection-prefixon the response header.
private getCollectionHeaderPrefix(header: SdkServiceResponseHeader): string | undefined {
const value = getClientOptions(header, "collectionHeaderPrefix");
const type = getNonNullSdkType(header.type);
return type.kind === "dict" && typeof value === "string" ? value : undefined;
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The core behavior is implemented end-to-end with targeted coverage and a valid changeset, with only minor documentation wording nits remaining.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
packages/http-client-java/emitter/src/options.ts:55
- "intermittent" reads like "occasional" here; this option generates an intermediate artifact. Consider updating the description to use "intermediate" and formatting the filename with backticks for consistency with other option docs.
description: "Developer options for http-client-java emitter.",
properties: {
"generate-code-model": {
type: "boolean",
description: "Generate intermittent 'code-model.yaml' file in output directory.",
packages/http-client-java/README.md:106
- "intermittent" suggests the file is written periodically; here it’s an intermediate artifact written when the option is enabled. Consider updating the wording and formatting the filename with backticks.
| `generate-code-model` | `boolean` | | Generate intermittent 'code-model.yaml' file in output directory. |
website/src/content/docs/docs/emitters/clients/http-client-java/reference/emitter.md:71
- "intermittent" suggests the file is written periodically; here it’s an intermediate artifact written when the option is enabled. Consider updating the wording and formatting the filename with backticks.
| `generate-code-model` | `boolean` | | Generate intermittent 'code-model.yaml' file in output directory. |
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Lite
ccba166 to
f3d8e09
Compare
There was a problem hiding this comment.
🟡 Changes recommended
It claims to fix #11859 but only implements response-side header collection prefixes, leaving request-side map header serialization unaddressed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Lite
| const collectionHeaderPrefix = this.getCollectionHeaderPrefix(header); | ||
| const httpHeader = new HttpHeader(header.serializedName, schema, { | ||
| language: { | ||
| default: { | ||
| name: header.name, | ||
| description: header.summary ?? header.doc, | ||
| }, | ||
| }, | ||
| extensions: collectionHeaderPrefix | ||
| ? { "x-ms-header-collection-prefix": collectionHeaderPrefix } | ||
| : undefined, | ||
| }); |
Summary
collectionHeaderPrefixclient option tox-ms-header-collection-prefixfor response headersMap<String, String>Testing
npm run buildnpm run formatnpm run lintnpm run test:emittermvn test --define "test=tsptest.responseheaders.ResponseHeadersTests" --no-transfer-progressFixes #11859