Skip to content

Fix missing parameters and headers in generated OpenAPI specs - #2033

Open
waldekmastykarz wants to merge 6 commits into
dotnet:mainfrom
waldekmastykarz:fix-openapi-null-collections
Open

waldekmastykarz wants to merge 6 commits into
dotnet:mainfrom
waldekmastykarz:fix-openapi-null-collections

Conversation

@waldekmastykarz

Copy link
Copy Markdown
Collaborator

Closes #2032

Microsoft.OpenApi 3.x leaves OpenApiOperation.Parameters, OpenApiPathItem.Parameters, OpenApiResponse.Headers and OpenApiResponse.Content null by default, so the plugin's ?.Add(...) calls silently did nothing.

  • Initialize these collections when creating path items, operations and responses
  • Use ??= when merging parameters into an existing operation
  • Also restores response body schemas (OpenApiResponse.Content had the same problem)
  • Add integration tests for query/header/path parameters, response headers and body, and parameter merging across requests (they fail without the fix)

…otnet#2032

Microsoft.OpenApi 3.x leaves Parameters, Headers and Content collections
null by default, so the null-conditional adds in OpenApiSpecGeneratorPlugin
were no-ops. Initialize the collections so query, header and path
parameters, response headers and response bodies are emitted again.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 31, 2026 09:13
@waldekmastykarz
waldekmastykarz requested a review from a team as a code owner October 31, 2026 09:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Generated OpenAPI specs can contain invalid media-type keys and duplicate header parameters, while reversed schema merging can omit fields from repeated responses.

3 open issues
What changed in this PR

Fixes missing data in OpenAPI specs generated from recorded traffic by initializing collections that Microsoft.OpenApi 3.x leaves null, addressing #2032.

Changes:

  • Initializes parameter collections and safely adds parameters when merging operations.
  • Initializes response headers and content to restore header definitions and body schemas.
  • Adds integration tests for generated parameters, response headers/content, and parameter merging.
File Description
DevProxy.Plugins/​Generation/​OpenApiSpecGeneratorPlugin.cs Initializes OpenAPI collections and handles null parameters during merges.
DevProxy.Integration.Tests/​GenerationPluginsIntegrationTests.cs Adds a spec-generation helper and regression tests for previously omitted data.

Review effort: Balanced

- De-duplicate request/response headers case-insensitively
- Fix reversed MergeSchema arguments for request and response bodies
- Skip response content when the response has no Content-Type

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Unresolved merge and path-parameter generation defects omit observed response headers and produce duplicate header and path parameters in the generated OpenAPI spec.

No open issues 🎉

3 previously missed issues

In code that hasn't changed since last review

Medium severity Generate unique names for consecutive path parameters

DevProxy.Plugins/​Generation/​OpenApiSpecGeneratorPlugin.cs:234

Initializing path parameters exposes duplicate entries for consecutive numeric or GUID segments: /tiles/12/3456/7890 becomes /tiles/{tiles-id}/{tiles-id}/{tiles-id} with three parameters named tiles-id. This violates OpenAPI's requirement for unique name/location pairs in a parameter list. Generate distinct names in ParametrizePath (for example, by suffixing repeated names), and add a regression test with consecutive parametrizable segments.

Medium severity Merge response headers across recordings

DevProxy.Plugins/​Generation/​OpenApiSpecGeneratorPlugin.cs:398

For an existing operation/status, AddOrMergeResponse merges only content and never copies apiResponse.Headers into the retained response. Recording two GETs returning 200, with no custom header on the first and X-Custom-Response on the second, still omits that observed header from the generated spec. Merge incoming headers case-insensitively before the no-content early return, and add a multi-recording regression test so headers from bodyless responses are retained too.

Medium severity Deduplicate request headers case-insensitively

DevProxy.Plugins/​Generation/​OpenApiSpecGeneratorPlugin.cs:574

Header de-duplication only applies within a single request. When two recordings of the same operation use X-Custom-Request and x-custom-request, AddOrMergeParameters compares their names case-sensitively and this line appends both, producing two parameters for the same HTTP header. Use an ordinal case-insensitive name comparison for header parameters when merging, while retaining case-sensitive matching for other parameter locations, and cover separate recordings in the regression test.

3 resolved since last review

Review effort: Balanced

- Merge response headers into existing responses
- Match header parameters case-insensitively when merging operations
- Suffix repeated path parameter names (e.g. {tiles-id}/{tiles-id-2})

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@waldekmastykarz

Copy link
Copy Markdown
Collaborator Author

Addressed the 3 issues from the latest Copilot review summary in 928ea66:

  • Unique path parameter names: consecutive parametrizable segments get suffixed names, e.g. /tiles/{tiles-id}/{tiles-id-2}/{tiles-id-3} (OpenApiSpecGenerator_GeneratesUniquePathParameterNames)
  • Merge response headers across recordings: headers are merged into the existing response before the content checks, so bodyless responses keep them too (OpenApiSpecGenerator_MergesHeadersAcrossRequests)
  • Case-insensitive header de-duplication across recordings: AddOrMergeParameters matches header parameters case-insensitively and other locations case-sensitively (covered by the same test)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

MergeSchema skips array schemas, causing retained collection-response schemas to lose fields; array-item merging and a regression test are needed.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Merge array item schemas to retain fields across responses

DevProxy.Plugins/​Generation/​OpenApiSpecGeneratorPlugin.cs:714

The response schemas now being retained still lose fields across collection responses because MergeSchema immediately returns for non-object schemas. For two GET /users responses containing [{"id":1}] and then [{"id":2,"name":"alpha"}], the generated response schema's items.properties contains only id. Extend MergeSchema to recurse into array Items, including array-valued object properties, and add an array-response regression test alongside the object-body test.

Review effort: Balanced

@waldekmastykarz

Copy link
Copy Markdown
Collaborator Author

Re: Merge array item schemas to retain fields across responses (latest Copilot review summary): not addressing this in this PR. MergeSchema has only ever merged object schemas. Request bodies, whose content was always populated, had the same array limitation before this change. This PR restores parameters, headers and response content that Microsoft.OpenApi 3.x left null (#2032). Adding array/items merging is a separate enhancement to schema merging and better tracked in its own issue.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@waldekmastykarz

Copy link
Copy Markdown
Collaborator Author

Update on Merge array item schemas to retain fields across responses: I'm reversing my earlier push-back. Because this PR now keeps response content, the gap shows up on list endpoints. Fixed in 2d53039: MergeSchema now recurses into array items, including array-valued object properties. Covered by OpenApiSpecGenerator_MergesArraySchemasAcrossRequests.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because a schema-merging edge case can omit fields from later responses when an empty array is recorded first.

No open issues 🎉

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@waldekmastykarz

Copy link
Copy Markdown
Collaborator Author

Addressed the edge case from the latest Copilot verdict (an empty array recorded first drops fields from later responses) in e908b6c. When the retained schema has no properties ([] or {} recorded first), MergeSchema now takes the incoming properties instead of skipping the merge. Covered by OpenApiSpecGenerator_MergesIntoEmptySchemaRecordedFirst.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because an unpublished finding indicates that empty-first array samples can produce schemas that reject later observed string values.

No open issues 🎉

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@waldekmastykarz

Copy link
Copy Markdown
Collaborator Author

Addressed the edge case from the latest Copilot verdict (empty-first array samples rejecting later string values) in fe30b6e. Empty arrays now get an untyped items: {} schema (any type) instead of {type: object}. That keeps them valid in OpenAPI 2.0 and 3.0, which require items. When a later sample has typed items, they replace the placeholder. Covered by OpenApiSpecGenerator_EmitsUntypedItemsForEmptyArray and OpenApiSpecGenerator_ReplacesUntypedItemsOfEmptyArrayRecordedFirst.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the OpenAPI specification generation fixes.

No open issues 🎉

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenApiSpecGeneratorPlugin omits parameters and response headers from generated specs

2 participants