Repository navigation
Derive exported batching flags from the server's batching options - #10368
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Re-exporting an existing settings file will not update variableBatching/requestBatching because the current update path only rewrites the top-level name, leaving stale batching flags behind.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates schema export so that the generated *-settings.json accurately reflects which batching modes a subgraph server actually allows, preventing gateways from emitting batching requests a server will reject (e.g., HC0009).
Changes:
- Derive exported
variableBatching/requestBatchingfrom server-side batching configuration via an internalITransportCapabilitiesProvider(with a default fallback when absent). - Add an ASP.NET Core implementation (
TransportCapabilitiesProvider) backed by namedGraphQLServerOptionsand register it in DI. - Add new tests for both the exporter behavior and the provider’s mapping logic, plus a small doc update.
File summaries
| File | Description |
|---|---|
| website/content/docs/fusion/batching.md | Documents that exported batching flags mirror server batching options (except aliasBatching). |
| src/HotChocolate/Core/test/Types.Tests/Execution/Internal/SchemaFileExporterTests.cs | Adds exporter tests for provider fallback and capability passthrough. |
| src/HotChocolate/Core/src/Types/Execution/Internal/TransportCapabilities.cs | Introduces an internal capability model for exporter batching flags. |
| src/HotChocolate/Core/src/Types/Execution/Internal/ITransportCapabilitiesProvider.cs | Adds internal abstraction used by the exporter to obtain capabilities. |
| src/HotChocolate/Core/src/Types/Execution/Internal/SchemaFileExporter.cs | Uses provider-derived capabilities when writing new settings files. |
| src/HotChocolate/AspNetCore/test/AspNetCore.Tests/TransportCapabilitiesProviderTests.cs | Adds mapping tests for server options → transport capabilities (including multi-schema). |
| src/HotChocolate/AspNetCore/src/AspNetCore/TransportCapabilitiesProvider.cs | Implements capability lookup using IOptionsMonitor<GraphQLServerOptions>. |
| src/HotChocolate/AspNetCore/src/AspNetCore/Extensions/HotChocolateAspNetCoreServiceCollectionExtensions.cs | Registers the provider so exports can reflect per-schema server settings. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Patch coverage100.0% of changed lines covered (26/26)
Project coverage: 57.9% (288083/497753 lines) |
Summary
SchemaFileExporternow writesvariableBatchingandrequestBatchingin the generatedschema-settings.jsonfrom the schema'sGraphQLServerOptions.Batchinginstead of declaring both astrue.aliasBatchingstaystrue, since it needs no server support.AddSourceSchemaDefaults(), or otherwise allow batching, therefore exportsfalsefor both flags, and a gateway composed from that file sends one request per item instead of a variable batch the server rejects withInvalid GraphQL Request.(HC0009).ITransportCapabilitiesProviderin Core, implemented inHotChocolate.AspNetCoreon top ofIOptionsMonitor<GraphQLServerOptions>, so the command-line project keeps its Core-only dependency. Without a registered provider the exporter keeps writingtruefor both flags.Test plan
SchemaFileExporterTestscover the fallback without a provider and that provider values are written through to the settings file.TransportCapabilitiesProviderTestscover no batching,AddSourceSchemaDefaults(), a single allowed flag, and per-schema-name resolution.SchemaExportCommandTestspass with unchanged snapshots.