Conversation
github-actions
Bot
added
📚 documentation
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Moderate issues remain with wildcard Vary handling and schema-download negotiation.
Review effort: Lite
Findings: None
What changed in this PR
Adds Vary: Accept handling across GraphQL HTTP transports, persisted operations, Azure Functions, caching, documentation, and regression tests.
Changes:
- Adds content-negotiation middleware.
- Merges cache-control
Varyvalues while preserving existing headers. - Updates documentation, tests, and response snapshots.
Review notes: changes are still needed for Vary: *, schema-download handling, and WebSocket documentation accuracy.
| File | Reviewed change |
|---|---|
website/content/docs/hotchocolate/server/http-transport.md |
Documents Vary behavior. |
website/content/docs/hotchocolate/server/cache-control.md |
Documents cache-control integration. |
website/content/docs/hotchocolate/migrating/migrate-from-16-6-to-16-7.md |
Adds migration guidance. |
website/content/docs/fusion/cache-control.md |
Documents Fusion cache behavior. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/VariableBatchingTests.cs |
Updates response snapshots. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.Standard_Query_Not_Allowed.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.Standard_Query_Not_Allowed_Override_Per_Request.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.Standard_Query_Not_Allowed_Even_When_Persisted.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.Standard_Query_Not_Allowed_Custom_Error.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.Standard_Query_By_Default_Works.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.Standard_Query_Allowed_When_Persisted.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.HotChocolateStyle_Sha256Hash_Success.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.HotChocolateStyle_Sha256Hash_Query_Empty_String_Success.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.HotChocolateStyle_Sha1Hash_Success.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.HotChocolateStyle_MD5Hash_Success.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.HotChocolateStyle_MD5Hash_NotFound.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.ApolloStyle_Sha256Hash_Success.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.ApolloStyle_Sha1Hash_Success.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.ApolloStyle_MD5Hash_Success.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/PersistedOperationTests.ApolloStyle_MD5Hash_NotFound.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DefaultSecurityTests.AllowOperationPlanRequests_True_Without_OperationPlanHeader_Should_Omit_OperationPlan.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DefaultSecurityTests.AllowOperationPlanRequests_True_With_OperationPlanHeader_Should_Include_OperationPlan.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DefaultSecurityTests.AllowOperationPlanRequests_False_With_PerRequestOverride_Without_OperationPlanHeader_Should_Omit_OperationPlan.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DefaultSecurityTests.AllowOperationPlanRequests_False_With_PerRequestOverride_And_OperationPlanHeader_Should_Include_OperationPlan.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DefaultSecurityTests.AllowOperationPlanRequests_False_With_OperationPlanHeader_Should_Omit_OperationPlan.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/CostReportingTests.RejectedRequest_Should_ReturnHttp400_When_AcceptIsGraphQLResponseJson.snap |
Updates expected headers. |
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/CostReportingTests.RejectedRequest_Should_ReturnHttp200_When_AcceptIsLegacyJson.snap |
Updates expected headers. |
src/HotChocolate/Caching/test/Caching.Tests/HttpCachingTests.cs |
Tests merged Vary values. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.SharedMaxAgeAndVary_Should_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.SharedMaxAgeAndVary_Multiple_Should_Cache_And_Combine.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.SharedMaxAgeAndScope_Should_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.SharedMaxAge_Multiple_Combine_Public_Private_Caches_Private.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.SharedMaxAge_MaxAge_Combine_Produces_Resolved_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.QueryError_Should_Not_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.No_Applied_Defaults_Should_Not_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.MaxAgeAndScope_Should_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.MaxAge_Zero_Should_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.MaxAge_SharedMaxAge_Combine_Produces_Resolved_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.MaxAge_NonZero_Should_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.MaxAge_Multiple_Should_Cache_Shortest_Time.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.MaxAge_Multiple_Combine_Public_Private_Caches_Private.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.JustScope_Should_Not_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.Just_Defaults_Should_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.Default_Scope_Should_Apply_And_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/Caching/test/Caching.Tests/__snapshots__/HttpCachingTests.Default_Max_Age_Should_Apply_And_Cache.snap |
Updates caching snapshot. |
src/HotChocolate/AzureFunctions/test/HotChocolate.AzureFunctions.Tests/InProcessEndToEndTests.cs |
Tests in-process headers. |
src/HotChocolate/AzureFunctions/test/HotChocolate.AzureFunctions.IsolatedProcess.Tests/IsolatedProcessEndToEndTests.cs |
Tests isolated-process headers. |
src/HotChocolate/AzureFunctions/src/HotChocolate.AzureFunctions/Extensions/HotChocolateAzureFunctionServiceCollectionExtensions.cs |
Registers negotiation middleware. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/PersistedOperationMiddlewareTests.cs |
Tests persisted-operation headers. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/HttpQueryMiddlewareTests.cs |
Updates QUERY expectations. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/HttpPostMiddlewareTests.cs |
Updates POST expectations. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/HttpGetSchemaMiddlewareTests.cs |
Updates schema expectations. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/HttpContentNegotiationMiddlewareTests.cs |
Tests negotiation rules. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/GraphQLOverHttpSpecTests.cs |
Tests transport-wide headers. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/Extensions/HttpResponseExtensionsTests.cs |
Tests Vary merging. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/__snapshots__/IntrospectionTests.Introspection_Request_With_Rule_Removed_Fail.md |
Updates response headers. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/__snapshots__/IntrospectionTests.Introspection_Request_When_NOT_Development_Fail.md |
Updates response headers. |
src/HotChocolate/AspNetCore/test/AspNetCore.Tests/__snapshots__/IntrospectionTests.Introspection_Request_When_Development_Success.md |
Updates response headers. |
src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/PersistedOperationMiddleware.cs |
Applies negotiation to persisted routes. |
src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/MiddlewareFactory.cs |
Creates negotiation middleware. |
src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/HttpUnsupportedRequestMiddleware.cs |
Reuses endpoint matching. |
src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/HttpContentNegotiationMiddleware.cs |
Adds Vary: Accept. |
src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/Formatters/DefaultHttpResponseFormatter.cs |
Preserves Vary values. |
src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/Extensions/HttpResponseExtensions.cs |
Implements Vary merging. |
src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/Extensions/HttpRequestExtensions.cs |
Adds endpoint matching. |
src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/Extensions/EndpointRouteBuilderExtensions.cs |
Integrates middleware. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Patch coverage100.0% of changed lines covered (95/95)
Project coverage: 57.8% (299462/518267 lines) |
This branch has not been deployed
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.
Summary
AcceptinVary, whatever its status code and under every transport version. This coversMapGraphQL,MapGraphQLHttp,MapGraphQLPersistedOperations, and the Azure Functions pipeline. The endpoints select the response byAccept(the JSON media type, a single result as SSE or multipart when the client names it, and406) and copy@cacheControl'sCache-Controlonto it, so withoutVarya shared or browser cache could hand one client's format to another.HttpContentNegotiationMiddlewareadds the header before any middleware writes a response; the persisted-operation routes get the same middleware through a route-group convention.DefaultHttpResponseFormatternow adds the@cacheControl(vary: …)names besideAcceptinstead of replacing the header, soVaryvalues set earlier by application middleware are kept, and a header that already lists*is left as it is. The isolated-process Azure Functions adapter now replaces a response header that is set again, where it used to add a second copy of a multi-valued header and throw for a single-valued one such asContent-Length.Varylists anything other thanAccept-Encoding, with the workarounds. The transport and cache-control pages describe the header and how a custom formatter adds to it. No public API changes.Test plan
HttpContentNegotiationMiddlewareTestsandHttpResponseExtensionsTestscover the method and path rules and the merge with existingVaryvalues.GraphQLOverHttpSpecTests, underLegacy,Draft20250508, andDraft20260903: results over GET, HEAD, POST, and QUERY, a406, a document syntax error, and a refused GET carryVary: Accept; OPTIONS, PUT, and a WebSocket upgrade do not. Rows forMapGraphQLHttpand for persisted operation GET, POST, and QUERY.HttpCachingTests:@cacheControl(vary: ["Accept", "X-foo"])yieldsVary: Accept, x-foowith a singleAccept.Vary: Accept.AzureHeaderDictionaryTests: a header orContent-Lengthset twice keeps only the second value.Vary: Accept.HotChocolate.AspNetCore.Tests,HotChocolate.Caching.Tests, both Azure Functions test projects,HotChocolate.Fusion.AspNetCore.Tests,HotChocolate.Fusion.Caching.Tests, andHotChocolate.Adapters.OpenApi.Testspass onnet10.0.