Mark stored procedure parameters without defaults as required in OpenAPI - #3687
Mark stored procedure parameters without defaults as required in OpenAPI#3687souvikghosh04 with Copilot wants to merge 16 commits into
Conversation
|
@copilot please add tests from to support the changes |
Added tests in
Each entity uses a distinct stored procedure so per-entity config metadata doesn't cross-contaminate (entities sharing a source share the same parameter definition). Verified all rows pass with the fix and fail when it's reverted, against a local SQL Server. (79f... latest commit) |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
@copilot fix the formatting issue in StoredProcedureGeneration.cs as it fails in build /home/vsts/work/1/s/src/Service.Tests/OpenApiDocumentor/StoredProcedureGeneration.cs(151,152): error WHITESPACE: Fix whitespace formatting. Replace 18 characters with '\n\n\s\s\s\s\s\s\s\s\s\s\s\s\s\s\s\s'. [/home/vsts/work/1/s/src/Service.Tests/Azure.DataApiBuilder.Service.Tests.csproj] |
Fixed in the latest commit — corrected the indentation on the |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
This introduces a third required/default precedence rule across DAB. OpenAPI now uses |
aaronburtle
left a comment
There was a problem hiding this comment.
Good to go once the last comments are addressed!
|
@copilot review the open/unresolved comments as fix what necessary and respond to each unresolved comments. |
…rtions and GET query param tests Co-authored-by: souvikghosh04 <210500244+souvikghosh04@users.noreply.github.com>
The inconsistency across GraphQL, MCP, and OpenAPI is a valid concern. Centralizing the predicate into a shared helper would require touching the GraphQL schema builder and MCP tool generation, which is beyond the scope of this PR. I'd recommend tracking that refactor as a follow-up item (e.g., referencing #3400), so it gets proper truth-table coverage for all three surfaces in one dedicated PR.
All three open comments have been addressed in the latest commit:
|
|
/azp run |
|
Azure Pipelines: Successfully started running 6 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 6 pipeline(s). |
Why make this change?
requiredarray on the{entity}_sp_requestschema, so parameters that must be supplied (e.g.firstName,lastName) were not flagged as required.What is this change?
CreateSpRequestComponentSchema(OpenApiDocumentor.cs): a parameter is now marked required when it is explicitly flagged in config or has no default value. Previously only an explicit configrequired: truepopulated the array, which is almost never set, so the array was always empty.IsRequestBodyRequiredlogic (!HasConfigDefault), keeping body-level and property-level required semantics consistent. An explicit configrequired: falseis still honored.How was this tested?
Extended
StoredProcedureGeneration.ValidateRequestBodyContentsto assert the request body schema'srequiredset contains the expected parameters. Verified the assertion fails when the fix is reverted.Sample Request(s)
GET /api/openapi— request body schema component for a stored procedure with parameterstitle,publisher_name(neither having a default):