Upgrade Hot Chocolate to latest -rc for version 16.0.0#3480
Open
aaronburtle wants to merge 20 commits intomainfrom
Open
Upgrade Hot Chocolate to latest -rc for version 16.0.0#3480aaronburtle wants to merge 20 commits intomainfrom
aaronburtle wants to merge 20 commits intomainfrom
Conversation
Bumps the HotChocolate family of packages from 16.0.0-p.7.68 (preview) to 16.0.0-rc.1.43 (release candidate). Affected packages: - HotChocolate - HotChocolate.AspNetCore - HotChocolate.AspNetCore.Authorization - HotChocolate.ModelContextProtocol - HotChocolate.Types.NodaTime - HotChocolate.Utilities.Introspection - HotChocolate.Transport.Http - HotChocolate.Diagnostics - CookieCrumble This commit is intentionally a single-line version bump; the API surface changes required to compile against rc.1.43 are split across the following commits in this series. Note: rc.1.43 is not yet cached on the internal feed (pkgs.dev.azure.com/sqldab/data_api_builder_build_packages). The NuGetAuthenticate@1 step in the build pipeline will trigger an authenticated upstream fetch on the first restore, after which the package will be permanently cached for all subsequent builds.
Hot Chocolate v16 reshaped the scalar type API surface. This commit adopts the new pattern in DAB's custom and built-in scalar usages. Scalar override renames (Service.GraphQLBuilder/CustomScalars/SingleType.cs): ParseLiteral(IFloatValueLiteral) -> OnCoerceInputLiteral(IFloatValueLiteral) ParseValue(JsonElement) -> OnCoerceInputValue(JsonElement) ParseResult / FormatResult -> OnCoerceOutputValue(T, ResultElement) ParseValue(T runtime) -> OnValueToLiteral(T) The output methods now write into a HotChocolate.Text.Json.ResultElement buffer rather than returning a value, and OnValueToLiteral replaces the old "ValueToLiteral / ParseResult / ParseValue" trio with a single literal-producing override. Scalar invocation renames at call sites (GraphQLStoredProcedureBuilder.cs, Sql/SchemaConverter.cs): type.ParseValue(runtimeValue) -> type.ValueToLiteral(runtimeValue) type.ParseResult(runtimeValue) -> type.ValueToLiteral(runtimeValue) Type rename (GraphQLTypes/DefaultValueType.cs, GraphQLStoredProcedureBuilder.cs, Sql/SchemaConverter.cs): ByteArrayType -> Base64StringType The legacy ByteArrayType identifier is now [Obsolete] and the Base64-string-encoded byte-array scalar lives under Base64StringType. DateTime / DateTimeOffset boundary (GraphQLStoredProcedureBuilder.cs, Sql/SchemaConverter.cs): HC v16's DateTimeType.ValueToLiteral(...) accepts only DateTimeOffset. Inputs from config (System.DateTime) are converted at the boundary preserving DateTimeKind: - Unspecified -> DateTimeOffset(value, TimeSpan.Zero) - Local / Utc -> DateTimeOffset(value) The CLR-driven schema-converter path now parses datetime defaults as DateTimeOffset directly rather than via DateTime. These changes match the patterns proposed in PR #3136 (HC v16 milestone-11 migration) and continue to compile cleanly against rc.1.43.
…tData Migrates DAB's Hot Chocolate runtime integration to v16's resolver and execution-pipeline contracts. Selection -> concrete type (Resolvers/CosmosQueryStructure.cs): ISelection -> Selection The ISelection interface was removed in HC v16; the runtime always returns the concrete Selection type. Selection.SyntaxNode -> SyntaxNodes[0].Node (Resolvers/CosmosQueryStructure.cs, Resolvers/Sql Query Structures/SqlQueryStructure.cs, Services/ExecutionHelper.cs) HC v16 introduces field-merging during query analysis, so a single Selection can now be backed by multiple syntactic occurrences in the query document. The shape changed from a single FieldNode (SyntaxNode) to an immutable list (SyntaxNodes) whose first entry is the canonical node for code that does not yet handle merged fields. DAB does not currently exercise merging, so .SyntaxNodes[0].Node is the equivalent canonical node and matches PR #3136's pattern. TimeSpanType -> DurationType (Services/ExecutionHelper.cs): The TimeSpanType scalar was removed; its replacement DurationType serializes values as ISO-8601 duration strings (e.g. "PT1H30M") rather than .NET's "1.06:00:00" round-trip format. The runtime-coercion path now parses with System.Xml.XmlConvert.ToTimeSpan, which is the ISO-8601-aware parser shipped in BCL. ByteArrayType -> Base64StringType (Services/ExecutionHelper.cs): Same scalar rename as the GraphQLBuilder commit; applied here in the runtime fieldValue coercion switch. OperationResult.WithContextData(...) -> direct property setter (Services/DetermineStatusCodeMiddleware.cs): context.Result = singleResult.WithContextData(contextData.ToImmutable()); is now singleResult.ContextData = contextData.ToImmutable(); OperationResult is mutable in v16, so the WithContextData fluent helper was removed; mutate ContextData directly. The middleware no longer needs to reassign context.Result since the existing instance reference now carries the change.
The OneOf input-object directive (graphql/graphql-spec#825) is permanently enabled in Hot Chocolate v16 and the SchemaOptions.EnableOneOf toggle was removed. Drops the now-obsolete .ModifyOptions(o => o.EnableOneOf = true) configuration call from the schema builder; behavior is unchanged.
…delegate
Updates the GraphQL server wire-up in Startup.cs for three v16 API
changes:
1. DateTimeType options bag
The bool-flag overload `new DateTimeType(disableFormatCheck: bool)`
is now [Obsolete]. Replaced with `new DateTimeType(new DateTimeOptions
{ ValidateInputFormat = ... })`. Note the boolean polarity flipped:
old: disableFormatCheck = !graphQLRuntimeOptions.EnableLegacyDateTimeScalar
new: ValidateInputFormat = !graphQLRuntimeOptions.EnableLegacyDateTimeScalar
...so the resulting condition that controls validation is unchanged.
2. WithOptions accepts only Action<GraphQLServerOptions>
The MapGraphQL().WithOptions(...) overload that took a literal
GraphQLServerOptions instance was removed. Options are now configured
per-request via a delegate:
.WithOptions(options => options.Tool.Enable = IsUIEnabled(...));
This means options are evaluated lazily on each incoming request
rather than captured eagerly at startup. The behavioral difference
is exercised by ConfigurationTests.TestNoConfigReturnsServiceUnavailable;
see the test commit later in this series.
3. Nitro mapping consolidated under GraphQL server options
`endpoints.MapNitroApp().WithOptions(new GraphQLToolOptions { ... })`
was removed; GraphQLToolOptions is no longer a public type. The
Nitro / Banana Cake Pop IDE is now governed solely by
GraphQLServerOptions.Tool.Enable on the GraphQL endpoint mapping.
Dropped the explicit MapNitroApp block because Tool.Enable already
covers the dev-mode disable case.
Updates two test files affected by Hot Chocolate v16 runtime API
changes that altered observable behavior.
MultiSourceQueryExecutionUnitTests.cs - OperationResult.Data shape:
HC v16 changed `OperationResult.Data` from
`IReadOnlyDictionary<string, object>` to
`OperationResultData?` (a struct wrapping a `ResultDocument`). The
`Errors` property is also no longer nullable; it is an immutable list
with an `IsEmpty` member.
Old:
Assert.IsNull(singleResult.Errors, ...);
IReadOnlyDictionary<string, object> data = singleResult.Data;
data.TryGetValue(QUERY_NAME_1, out object queryNode1);
var firstEntry = ((IReadOnlyDictionary<string, object>)queryNode1)
.FirstOrDefault();
Assert.AreEqual("db1", firstEntry.Value, ...);
New:
Assert.IsTrue(singleResult.Errors.IsEmpty, ...);
ResultDocument document = (ResultDocument)singleResult.Data.Value.Value;
document.Data.TryGetProperty(QUERY_NAME_1, out ResultElement queryNode1);
ResultProperty firstEntry = queryNode1.EnumerateObject().FirstOrDefault();
Assert.AreEqual("db1", firstEntry.Value.GetString(), ...);
The traversal is now: OperationResultData (struct) -> .Value (object,
the ResultDocument) -> .Data (root ResultElement) -> .TryGetProperty
-> .EnumerateObject -> ResultProperty -> .Value.GetString().
ConfigurationTests.cs - exception contract for "no runtime config":
HC v16 resolves WithOptions(Action<>) per request rather than eagerly
at startup, so the "no runtime config" condition that previously
manifested as a 503 response (hosted-on-demand scenario) or
ApplicationException (CLI startup scenario) now bubbles out of the
request pipeline as DataApiBuilderException(HttpStatusCode.ServiceUnavailable).
The catch block in TestNoConfigReturnsServiceUnavailable is split into
two typed handlers so the assertion remains precise:
catch (DataApiBuilderException dabException)
Assert StatusCode == ServiceUnavailable.
(Covers all three [DataRow] cases under HC v16.)
catch (ApplicationException appException)
Assert isUpdateableRuntimeConfig == false.
Assert message matches the historical CLI startup message.
(Preserved for symmetry; not exercised under HC v16 runtime
behavior on the in-process test host but kept to avoid silently
weakening the contract for legacy hosting variants.)
Added a remarks block on the test summarizing the behavioral change.
Adds `#nullable enable annotations` to three test files that contain
nullable reference type annotations (e.g. `string?`) but live in the
Service.Tests project, which has `<Nullable>disable</Nullable>` set
project-wide.
Without an explicit annotation context the compiler emits CS8632
("The annotation for nullable reference types should only be used in
code within a '#nullable' annotations context"), which the repo-wide
`<TreatWarningsAsErrors>True</TreatWarningsAsErrors>` setting promotes
to a build error.
These errors were introduced on main by an earlier change (PR #3476)
and surfaced here only because the HC 16 upgrade pulled the build all
the way through Service.Tests. The fix is intentionally narrow:
"#nullable enable annotations" allows `?` syntax without enabling
flow-sensitive null-state checking, so it does not cascade into a
sea of unrelated nullable-state diagnostics in these legacy test
files.
Files affected:
- Service.Tests/UnitTests/RequestParserUnitTests.cs
- Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs
- Service.Tests/Configuration/RuntimeConfigLoaderTests.cs
This commit is independent of the HC 16 upgrade and could land
separately on main.
aaronburtle
requested review from
Alekhya-Polavarapu,
Aniruddh25,
JerryNixon,
RubenCerna2079,
anushakolan,
rusamant,
sourabh1007,
souvikghosh04,
stuartpa and
vadeveka
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Updates DAB’s GraphQL stack to align with Hot Chocolate v16 RC APIs, unblocking progress toward the finalized v16 surface area (per #3448) while keeping existing DAB REST/GraphQL/CLI behavior consistent.
Changes:
- Bump Hot Chocolate (and related) package versions from
16.0.0-p.7.68to16.0.0-rc.1.43. - Migrate to v16 API changes across scalars, selection/syntax access, duration handling, and server endpoint options.
- Update unit tests for new Hot Chocolate result/data traversal and address nullable-annotation build errors under warnings-as-errors.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Directory.Packages.props | Updates Hot Chocolate/CookieCrumble package versions to v16 RC. |
| src/Service/Startup.cs | Migrates DateTime scalar options and GraphQL endpoint options/Nitro handling to v16 APIs. |
| src/Core/Services/GraphQLSchemaCreator.cs | Removes explicit OneOf enablement now default in v16. |
| src/Core/Services/ExecutionHelper.cs | Updates scalar runtime conversions (Base64String, Duration) and selection syntax node access. |
| src/Core/Services/DetermineStatusCodeMiddleware.cs | Migrates from WithContextData to setting ContextData directly. |
| src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs | Updates selection syntax node access for v16 selection changes. |
| src/Core/Resolvers/CosmosQueryStructure.cs | Updates selection type and syntax node access for v16 selection changes. |
| src/Service.GraphQLBuilder/Sql/SchemaConverter.cs | Updates default value literal creation to new scalar APIs and Base64StringType. |
| src/Service.GraphQLBuilder/GraphQLTypes/DefaultValueType.cs | Replaces obsolete ByteArrayType with Base64StringType. |
| src/Service.GraphQLBuilder/GraphQLStoredProcedureBuilder.cs | Updates scalar literal creation APIs for stored proc default values. |
| src/Service.GraphQLBuilder/CustomScalars/SingleType.cs | Migrates custom scalar overrides to v16 coercion hooks and result writing. |
| src/Service.Tests/UnitTests/MultiSourceQueryExecutionUnitTests.cs | Updates assertions to v16 result/document traversal types. |
| src/Service.Tests/Configuration/ConfigurationTests.cs | Updates “no runtime config” expectations for v16 request-time option resolution. |
| src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs | Enables nullable annotations to fix CS8632 under warnings-as-errors. |
| src/Service.Tests/UnitTests/RequestParserUnitTests.cs | Enables nullable annotations to fix CS8632 under warnings-as-errors. |
| src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs | Enables nullable annotations to fix CS8632 under warnings-as-errors. |
Comments suppressed due to low confidence (1)
src/Service.Tests/Configuration/ConfigurationTests.cs:756
- The test metadata still says "Throws Application exception" (DataRow DisplayName) and the
[TestMethod]description mentions anApplicationExceptionfor the CLI scenario, but the updated assertions now acceptDataApiBuilderException(ServiceUnavailable)as well. Please update the DisplayName/description to match the new expected behavior so the test output remains accurate.
[DataRow(new string[] { }, true, DisplayName = "No config returns 503 - config file flag absent")]
[DataRow(new string[] { "--ConfigFileName=" }, true, DisplayName = "No config returns 503 - empty config file option")]
[DataRow(new string[] { }, false, DisplayName = "Throws Application exception")]
[TestMethod("Validates that queries before runtime is configured returns a 503 in hosting scenario whereas an application exception when run through CLI")]
public async Task TestNoConfigReturnsServiceUnavailable(
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why make this change?
Closes #3448
DAB has been pinned to a Hot Chocolate v16 preview (16.0.0-p.7.68) since the v16 upgrade work began. Hot Chocolate has since reached release-candidate status; this PR moves us forward to 16.0.0-rc.1.43, the latest RC at the time of writing, and lands the API migrations required by the milestones between p.7 and rc.1.
What is this change?
Update deps.
Bump 9 HC packages from
16.0.0-p.7.68to16.0.0-rc.1.43inDirectory.Packages.propsRefactor Scalar APIs.
v16 renamed scalar overrides (
ParseLiteral/ParseValuebecameOnCoerceInputLiteral/OnCoerceInputValue/OnCoerceOutputValue/OnValueToLiteral) and call-site helpers (ParseValue/ParseResultbecameValueToLiteral).ByteArrayTypeis now obsolete in favor ofBase64StringType.DateTimeType.ValueToLiteralonly acceptsDateTimeOffset, so we convert at the boundary.Refactor resolver/execution APIs.
ISelectionwas removed (use concrete Selection);Selection.SyntaxNodebecameSyntaxNodes[0].Nodeto support field-merging;TimeSpanTypewas replaced byDurationType(ISO-8601, parsed viaXmlConvert.ToTimeSpan);OperationResult.WithContextDatais gone, setsingleResult.ContextDatadirectly.Drop ModifyOptions(o => o.EnableOneOf = true)
OneOf is on by default in v16.
Refactor startup
DateTimeType(disableFormatCheck: bool)is obsolete (replaced withDateTimeOptions { ValidateInputFormat = ... }, polarity flipped);WithOptionsonly acceptsAction<GraphQLServerOptions>(per-request);MapNitroApp().WithOptions(GraphQLToolOptions)was removed. Nitro is governed byTool.Enableon the server options.Test update
Adopt v16
OperationResultData/ResultDocument/ResultElement/ResultPropertytraversal inMultiSourceQueryExecutionUnitTests. UpdateTestNoConfigReturnsServiceUnavailableto recognize that lazyWithOptionsresolution surfaces "no runtime config" asDataApiBuilderException(ServiceUnavailable)rather than a 503 response orApplicationException.Fix test errors
Pre-existing CS8632 regression; three test files use nullable annotations under
Nullable=disable+TreatWarningsAsErrors=true. Added "#nullable enable annotations" so syntax is accepted without cascading null-state warnings. Independent of the HC upgrade, included to keep the build green.Harden Selection.SyntaxNodes access
HC v16's
Selection.SyntaxNodesis aReadOnlySpan<FieldSelectionNode>(introduced for field-merging), so directSyntaxNodes[0].Nodeindexing would surface asIndexOutOfRangeExceptionat request time if the span were ever empty. AddedSelection.RequireFieldNode()inSelectionExtensions.cs(IsEmptycheck, then return SyntaxNodes[0].Node; throws a targetedDataApiBuilderExceptionotherwise). Adopted at all call sites inSqlQueryStructure,CosmosQueryStructure, andExecutionHelper`.How was this tested?
Run against current test suite.
Sample Request(s)
No user-facing API changes. Existing GraphQL queries, REST requests, and CLI usage are unchanged