Reworked Persisted Operation Options (#7560)

src/HotChocolate/AspNetCore/test/AspNetCore.Tests/PersistedOperationTests.cs

@@ -289,7 +289,7 @@ public async Task Standard_Query_Not_Allowed()
         var server = CreateStarWarsServer(
             configureServices: s => s
                 .AddGraphQL("StarWars")
-                .ModifyRequestOptions(o => o.PersistedOperationOptions = OnlyPersistedOperations)
+                .ModifyRequestOptions(o => o.PersistedOperations.OnlyAllowPersistedDocuments = true)
                 .ConfigureSchemaServices(c => c.AddSingleton<IOperationDocumentStorage>(storage))
                 .UsePersistedOperationPipeline());
 
@@ -316,7 +316,7 @@ public async Task Standard_Query_Not_Allowed_Even_When_Persisted()
         var server = CreateStarWarsServer(
             configureServices: s => s
                 .AddGraphQL("StarWars")
-                .ModifyRequestOptions(o => o.PersistedOperationOptions |= OnlyPersistedOperations)
+                .ModifyRequestOptions(o => o.PersistedOperations.OnlyAllowPersistedDocuments = true)
                 .ConfigureSchemaServices(c => c.AddSingleton<IOperationDocumentStorage>(storage))
                 .UsePersistedOperationPipeline());
 
@@ -344,9 +344,10 @@ public async Task Standard_Query_Allowed_When_Persisted()
             configureServices: s => s
                 .AddGraphQL("StarWars")
                 .ModifyRequestOptions(o =>
-                    o.PersistedOperationOptions =
-                        OnlyPersistedOperations
-                        | MatchStandardDocument)
+                {
+                    o.PersistedOperations.OnlyAllowPersistedDocuments = true;
+                    o.PersistedOperations.AllowDocumentBody = true;
+                })
                 .ConfigureSchemaServices(c => c.AddSingleton<IOperationDocumentStorage>(storage))
                 .UsePersistedOperationPipeline());
 
@@ -372,8 +373,8 @@ public async Task Standard_Query_Not_Allowed_Custom_Error()
                 .AddGraphQL("StarWars")
                 .ModifyRequestOptions(o =>
                 {
-                    o.PersistedOperationOptions = OnlyPersistedOperations;
-                    o.OnlyPersistedOperationsAreAllowedError =
+                    o.PersistedOperations.OnlyAllowPersistedDocuments = true;
+                    o.PersistedOperations.OperationNotAllowedError =
                         ErrorBuilder.New()
                             .SetMessage("Not allowed!")
                             .Build();
@@ -403,7 +404,7 @@ public async Task Standard_Query_Not_Allowed_Override_Per_Request()
                 .AddGraphQL("StarWars")
                 .ModifyRequestOptions(o =>
                 {
-                    o.PersistedOperationOptions = OnlyPersistedOperations;
+                    o.PersistedOperations.OnlyAllowPersistedDocuments = true;
                 })
                 .ConfigureSchemaServices(c => c.AddSingleton<IOperationDocumentStorage>(storage))
                 .UsePersistedOperationPipeline()
@@ -420,6 +421,50 @@ public async Task Standard_Query_Not_Allowed_Override_Per_Request()
         result.MatchSnapshot();
     }
 
+    [Fact]
+    public async Task Ensure_Pooled_Objects_Are_Cleared()
+    {
+        // arrange
+        // we have one operation in our storage that is allowed.
+        var storage = new OperationStorage();
+        storage.AddOperation(
+            "a73defcdf38e5891e91b9ba532cf4c36",
+            "query GetHeroName { hero { name } }");
+
+        var server = CreateStarWarsServer(
+            configureServices: s => s
+                .AddGraphQL("StarWars")
+                .ModifyRequestOptions(o =>
+                {
+                    // we only allow persisted operations but we also allow standard requests
+                    // as long as they match a persisted operation.
+                    o.PersistedOperations.OnlyAllowPersistedDocuments = true;
+                    o.PersistedOperations.AllowDocumentBody = true;
+                })
+                .ConfigureSchemaServices(c => c.AddSingleton<IOperationDocumentStorage>(storage))
+                .UsePersistedOperationPipeline());
+
+        // act
+        var result1ShouldBeOk = await server.PostAsync(
+            new ClientQueryRequest { Id = "a73defcdf38e5891e91b9ba532cf4c36" },
+            path: "/starwars");
+
+        var result2ShouldBeOk = await server.PostAsync(
+            new ClientQueryRequest { Query = "query GetHeroName { hero { name } }"},
+            path: "/starwars");
+
+        var result3ShouldFail = await server.PostAsync(
+            new ClientQueryRequest { Query = "{ __typename }" },
+            path: "/starwars");
+
+        // assert
+        await Snapshot.Create()
+            .Add(result1ShouldBeOk, "Result 1 - Should be OK")
+            .Add(result2ShouldBeOk, "Result 2 - Should be OK")
+            .Add(result3ShouldFail, "Result 3 - Should fail")
+            .MatchMarkdownAsync();
+    }
+
     private ClientQueryRequest CreateApolloStyleRequest(string hashName, string key)
         =>  new()
         {

src/HotChocolate/AspNetCore/test/AspNetCore.Tests/__snapshots__/PersistedOperationTests.Ensure_Pooled_Objects_Are_Cleared.md

@@ -0,0 +1,53 @@
+# Ensure_Pooled_Objects_Are_Cleared
+
+## Result 1 - Should be OK
+
+```json
+{
+  "ContentType": "application/graphql-response+json; charset=utf-8",
+  "StatusCode": "OK",
+  "Data": {
+    "hero": {
+      "name": "R2-D2"
+    }
+  },
+  "Errors": null,
+  "Extensions": null
+}
+```
+
+## Result 2 - Should be OK
+
+```json
+{
+  "ContentType": "application/graphql-response+json; charset=utf-8",
+  "StatusCode": "OK",
+  "Data": {
+    "hero": {
+      "name": "R2-D2"
+    }
+  },
+  "Errors": null,
+  "Extensions": null
+}
+```
+
+## Result 3 - Should fail
+
+```json
+{
+  "ContentType": "application/graphql-response+json; charset=utf-8",
+  "StatusCode": "BadRequest",
+  "Data": null,
+  "Errors": [
+    {
+      "message": "Only persisted operations are allowed.",
+      "extensions": {
+        "code": "HC0067"
+      }
+    }
+  ],
+  "Extensions": null
+}
+```
+

src/HotChocolate/Core/src/Execution/Options/IPersistedOperationOptionsAccessor.cs

@@ -6,20 +6,7 @@ namespace HotChocolate.Execution.Options;
 public interface IPersistedOperationOptionsAccessor
 {
     /// <summary>
-    /// Specifies if only persisted operations are allowed when using
-    /// the persisted operation pipeline.
+    /// Specifies the behavior of the persisted operation pipeline.
     /// </summary>
-    [Obsolete("Use PersistedOperationOptions instead.")]
-    bool OnlyAllowPersistedOperations { get; }
-
-    /// <summary>
-    /// Specifies the behavior of the persisted operation middleware.
-    /// </summary>
-    PersistedOperationOptions PersistedOperationOptions { get; set; }
-
-    /// <summary>
-    /// The error that will be thrown when only persisted
-    /// operations are allowed and a normal operation is issued.
-    /// </summary>
-    IError OnlyPersistedOperationsAreAllowedError { get; }
+    PersistedOperationOptions PersistedOperations { get; }
 }

src/HotChocolate/Core/src/Execution/Options/PersistedOperationOptions.cs

@@ -1,30 +1,41 @@
 namespace HotChocolate.Execution.Options;
 
 /// <summary>
-/// Represents the options to configure the
-/// behavior of the persisted operation middleware.
+/// Represents the persisted operation options.
 /// </summary>
-[Flags]
-public enum PersistedOperationOptions
+public sealed class PersistedOperationOptions
 {
+    private IError _operationNotAllowedError = ErrorHelper.OnlyPersistedOperationsAreAllowed();
+
     /// <summary>
-    /// Nothing is configured.
+    /// Specifies if only persisted operation documents are allowed.
     /// </summary>
-    None = 0,
+    public bool OnlyAllowPersistedDocuments { get; set; }
 
     /// <summary>
-    /// Only persisted operations are allowed.
+    /// Specifies that if <see cref="OnlyAllowPersistedDocuments"/> is switched on
+    /// whether a standard GraphQL request with document body is allowed as long as
+    /// it matches a persisted document.
     /// </summary>
-    OnlyPersistedOperations = 1,
+    public bool AllowDocumentBody { get; set; }
 
     /// <summary>
-    /// Allow standard GraphQL requests if the GraphQL document
-    /// match a persisted operation document.
+    /// Specifies if persisted operation documents
+    /// need to be validated.
     /// </summary>
-    MatchStandardDocument = 2,
+    public bool SkipPersistedDocumentValidation { get; set; }
 
     /// <summary>
-    /// Skip validation for persisted operations documents.
+    /// The error that will be thrown when only persisted
+    /// operations are allowed and a normal operation is issued.
     /// </summary>
-    SkipValidationForPersistedDocument = 4
+    public IError OperationNotAllowedError
+    {
+        get => _operationNotAllowedError;
+        set
+        {
+            _operationNotAllowedError = value
+                ?? throw new ArgumentNullException(nameof(OperationNotAllowedError));
+        }
+    }
 }

src/HotChocolate/Core/src/Execution/Options/RequestExecutorOptions.cs

@@ -11,7 +11,7 @@ public class RequestExecutorOptions : IRequestExecutorOptionsAccessor
 {
     private static readonly TimeSpan _minExecutionTimeout = TimeSpan.FromMilliseconds(100);
     private TimeSpan _executionTimeout;
-    private IError _onlyPersistedOperationsAreAllowedError = ErrorHelper.OnlyPersistedOperationsAreAllowed();
+    private PersistedOperationOptions _persistedOperations = new();
 
     /// <summary>
     /// <para>Initializes a new instance of <see cref="RequestExecutorOptions"/>.</para>
@@ -53,51 +53,20 @@ public TimeSpan ExecutionTimeout
     public bool IncludeExceptionDetails { get; set; } = Debugger.IsAttached;
 
     /// <summary>
-    /// <para>
-    /// Specifies if only persisted operations are allowed when using
-    /// the persisted operation pipeline.
-    /// </para>
-    /// <para>The default is <c>false</c>.</para>
-    /// </summary>
-    [Obsolete("Use PersistedOperationOptions instead.")]
-    public bool OnlyAllowPersistedOperations
-    {
-        get => (PersistedOperationOptions & OnlyPersistedOperations) == OnlyPersistedOperations;
-        set
-        {
-            if (value)
-            {
-                PersistedOperationOptions |= OnlyPersistedOperations;
-            }
-            else
-            {
-                PersistedOperationOptions &= ~OnlyPersistedOperations;
-            }
-        }
-    }
-
-    /// <summary>
-    /// Specifies the behavior of the persisted operation middleware.
+    /// Specifies that the transport is allowed to provide the schema SDL document as a file.
     /// </summary>
-    public PersistedOperationOptions PersistedOperationOptions { get; set; } = None;
+    public bool EnableSchemaFileSupport { get; set; } = true;
 
     /// <summary>
-    /// The error that will be thrown when only persisted
-    /// operations are allowed and a normal operation is issued.
+    /// Specifies the behavior of the persisted operation pipeline.
     /// </summary>
-    public IError OnlyPersistedOperationsAreAllowedError
+    public PersistedOperationOptions PersistedOperations
     {
-        get => _onlyPersistedOperationsAreAllowedError;
+        get => _persistedOperations;
         set
         {
-            _onlyPersistedOperationsAreAllowedError = value
-                ?? throw new ArgumentNullException(
-                    nameof(OnlyPersistedOperationsAreAllowedError));
+            _persistedOperations = value
+                ?? throw new ArgumentNullException(nameof(PersistedOperations));
         }
     }
-
-    /// <summary>
-    /// Specifies that the transport is allowed to provide the schema SDL document as a file.
-    /// </summary>
-    public bool EnableSchemaFileSupport { get; set; } = true;
 }

src/HotChocolate/Core/src/Execution/Pipeline/OnlyPersistedOperationsAllowedMiddleware.cs

@@ -30,16 +30,16 @@ private OnlyPersistedOperationsAllowedMiddleware(
             ?? throw new ArgumentNullException(nameof(diagnosticEvents));
 
         // prepare options.
-        _options = options.PersistedOperationOptions;
-        var error = options.OnlyPersistedOperationsAreAllowedError;
+        _options = options.PersistedOperations;
+        var error = options.PersistedOperations.OperationNotAllowedError;
         _errorResult =  OperationResultBuilder.CreateError(error, _statusCode);
         _exception = new GraphQLException(error);
     }
 
     public ValueTask InvokeAsync(IRequestContext context)
     {
         // if all operations are allowed we can skip this middleware.
-        if((_options & OnlyPersistedOperations) != OnlyPersistedOperations)
+        if(!_options.OnlyAllowPersistedDocuments)
         {
             return _next(context);
         }
@@ -51,7 +51,7 @@ public ValueTask InvokeAsync(IRequestContext context)
             // however this could still be a standard GraphQL request that contains a document
             // that just matches a persisted operation document.
             // either this is allowed by the configuration and we can skip this middleware
-            if ((_options & MatchStandardDocument) == MatchStandardDocument)
+            if (_options.AllowDocumentBody)
             {
                 return _next(context);
             }

src/HotChocolate/Core/src/Execution/Pipeline/ReadPersistedOperationMiddleware.cs

@@ -3,7 +3,6 @@
 using HotChocolate.Language;
 using HotChocolate.Validation;
 using Microsoft.Extensions.DependencyInjection;
-using static HotChocolate.Execution.Options.PersistedOperationOptions;
 
 namespace HotChocolate.Execution.Pipeline;
 
@@ -61,7 +60,7 @@ await _operationDocumentStorage.TryReadAsync(
                 context.ValidationResult = DocumentValidatorResult.Ok;
                 context.IsCachedDocument = true;
                 context.IsPersistedDocument = true;
-                if ((_options & SkipValidationForPersistedDocument) == SkipValidationForPersistedDocument)
+                if (_options.SkipPersistedDocumentValidation)
                 {
                     context.ValidationResult = DocumentValidatorResult.Ok;
                 }
@@ -75,7 +74,7 @@ await _operationDocumentStorage.TryReadAsync(
                 context.ValidationResult = DocumentValidatorResult.Ok;
                 context.IsCachedDocument = true;
                 context.IsPersistedDocument = true;
-                if ((_options & SkipValidationForPersistedDocument) == SkipValidationForPersistedDocument)
+                if (_options.SkipPersistedDocumentValidation)
                 {
                     context.ValidationResult = DocumentValidatorResult.Ok;
                 }
@@ -93,7 +92,7 @@ public static RequestCoreMiddleware Create()
                 next,
                 diagnosticEvents,
                 persistedOperationStore,
-                core.Options.PersistedOperationOptions);
+                core.Options.PersistedOperations);
             return context => middleware.InvokeAsync(context);
         };
 }