[Configuration] Make optional scalar properties nullable - #3785
[Configuration] Make optional scalar properties nullable#3785Aryadeepta (Aryadeepta) wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR updates the configuration object model and JSON converters so that optional REST/GraphQL scalar runtime settings (e.g., enabled, path, allow-introspection, request-body-strict) can remain omitted (serialized as absent) when not explicitly configured, preventing higher-precedence configs from unintentionally overriding base config values during merge. It preserves effective runtime defaults via RuntimeConfig accessor properties and adds regression tests to validate the new serialization/merge behavior.
Changes:
- Make REST and GraphQL runtime scalar options nullable in the object model and adjust
RuntimeConfigaccessors to apply effective defaults. - Update REST/GraphQL runtime options JSON converters to only write scalar properties when explicitly set.
- Add/adjust tests (including hot-reload) to validate omission-preserving serialization and non-overriding merge behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs | Adds regression tests for omission-preserving serialization and merge behavior while asserting effective defaults via RuntimeConfig accessors. |
| src/Service.Tests/Configuration/HotReload/ConfigurationHotReloadTests.cs | Updates hot-reload assertions to use effective RuntimeConfig accessors rather than nullable underlying options. |
| src/Config/ObjectModel/RuntimeConfig.cs | Updates accessors (IsRestEnabled, IsGraphQLEnabled, paths, etc.) to interpret nullable scalar options with correct effective defaults. |
| src/Config/ObjectModel/RestRuntimeOptions.cs | Makes REST scalar options nullable to distinguish omitted vs explicitly configured values. |
| src/Config/ObjectModel/GraphQLRuntimeOptions.cs | Makes key GraphQL scalar options nullable to distinguish omitted vs explicitly configured values. |
| src/Config/Converters/RestRuntimeOptionsConverterFactory.cs | Writes REST scalar properties only when non-null; preserves shorthand true/false behavior. |
| src/Config/Converters/GraphQLRuntimeOptionsConverterFactory.cs | Writes GraphQL scalar properties only when non-null; preserves shorthand true/false behavior. |
| src/Cli/ConfigGenerator.cs | Keeps dab init output stable by explicitly supplying GraphQL defaults where needed. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
4107715 to
e0538f3
Compare
|
@microsoft-github-policy-service agree |
Why make this change?
What is this change?
RuntimeConfigaccessors.trueandfalsevalues for REST/GraphQL shorthand configuration.dab initoutput by explicitly supplying generated GraphQL defaults.How was this tested?
Validated with:
dotnet build src/Azure.DataApiBuilder.sln --no-restoreRuntimeConfigLoaderJsonDeserializerTestsConfigGeneratorTestsConfigureOptionsTestsdotnet format src/Azure.DataApiBuilder.sln --verify-no-changesThe full Service test suite was also attempted locally, but environment-dependent tests require external database instances and exceeded the local WSL inotify watcher limit.
Sample Request(s)
Not applicable. This change affects configuration serialization and merge behavior rather than REST or GraphQL request behavior.