Do not return operation context to pool when tasks are still running. (…

…#2616)

src/HotChocolate/Core/src/Execution/DependencyInjection/InternalServiceCollectionExtensions.cs

@@ -99,17 +99,17 @@ internal static class InternalServiceCollectionExtensions
             this IServiceCollection services)
         {
             services.TryAddSingleton<IDocumentCache>(
-                sp => new DefaultDocumentCache());
+                _ => new DefaultDocumentCache());
             services.TryAddSingleton<IPreparedOperationCache>(
-                sp => new DefaultPreparedOperationCache());
+                _ => new DefaultPreparedOperationCache());
             return services;
         }
 
         internal static IServiceCollection TryAddDefaultDocumentHashProvider(
             this IServiceCollection services)
         {
             services.TryAddSingleton<IDocumentHashProvider>(
-                sp => new MD5DocumentHashProvider());
+                _ => new MD5DocumentHashProvider());
             return services;
         }
 

src/HotChocolate/Core/src/Execution/Processing/OperationContext.Pooling.cs

@@ -24,6 +24,8 @@ internal sealed partial class OperationContext
             _resultHelper = new ResultHelper(resultPool);
         }
 
+        public bool IsPooled => _isPooled;
+
         public void Initialize(
             IRequestContext requestContext,
             IServiceProvider scopedServices,

src/HotChocolate/Core/src/Execution/Processing/OperationContextPoolPolicy.cs

@@ -17,8 +17,18 @@ public OperationContextPoolPolicy(Func<OperationContext> factory)
 
         public bool Return(OperationContext obj)
         {
-            obj.Clean();
-            return true;
+            if (obj.IsPooled)
+            {
+                return true;
+            }
+
+            if (obj.Execution.TaskStats.IsCompleted)
+            {
+                obj.Clean();
+                return true;
+            }
+
+            return false;
         }
     }
 }

src/HotChocolate/Core/src/Execution/Processing/ResolverTask.cs

@@ -53,8 +53,9 @@ private async ValueTask TryExecuteAndCompleteAsync(CancellationToken cancellatio
                 else
                 {
                     _operationContext.Execution.TaskStats.TaskCompleted();
-                    _operationContext.Execution.TaskPool.Return(this);
                 }
+
+                _operationContext.Execution.TaskPool.Return(this);
             }
         }
 

src/HotChocolate/Core/test/Execution.Tests/CodeFirstTests.cs

@@ -268,12 +268,22 @@ public async Task ExecuteExplicitAsyncField()
         [Fact]
         public async Task CannotCreateRootValue()
         {
-            IExecutionResult result =
-                await new ServiceCollection()
-                    .AddGraphQL()
-                    .AddQueryType<QueryPrivateConstructor>()
-                    .ExecuteRequestAsync("{ hello }")
-                    .MatchSnapshotAsync();
+            await new ServiceCollection()
+                .AddGraphQL()
+                .AddQueryType<QueryPrivateConstructor>()
+                .ExecuteRequestAsync("{ hello }")
+                .MatchSnapshotAsync();
+        }
+
+        // https://github.com/ChilliCream/hotchocolate/issues/2617
+        [Fact]
+        public async Task EnsureThatFieldsWithDifferentCasingAreNotMerged()
+        {
+            await new ServiceCollection()
+                .AddGraphQL()
+                .AddQueryType<QueryFieldCasing>()
+                .BuildSchemaAsync()
+                .MatchSnapshotAsync();
         }
 
         private static Schema CreateSchema()
@@ -515,5 +525,13 @@ private QueryPrivateConstructor()
 
             public string Hello() => "Hello";
         }
+
+        public class QueryFieldCasing
+        {
+            public string YourFieldName { get; set; }
+
+            [GraphQLDeprecated("This is deprecated")]
+            public string YourFieldname { get; set; }
+        }
     }
 }

src/HotChocolate/Core/test/Execution.Tests/__snapshots__/CodeFirstTests.EnsureThatFieldsWithDifferentCasingAreNotMerged.snap

@@ -0,0 +1,23 @@
+schema {
+  query: QueryFieldCasing
+}
+
+type QueryFieldCasing {
+  yourFieldName: String
+  yourFieldname: String @deprecated(reason: "This is deprecated")
+}
+
+"The `@defer` directive may be provided for fragment spreads and inline fragments to inform the executor to delay the execution of the current fragment to indicate deprioritization of the current fragment. A query with `@defer` directive will cause the request to potentially return multiple responses, where non-deferred data is delivered in the initial response and data deferred is delivered in a subsequent response. `@include` and `@skip` take precedence over `@defer`."
+directive @defer("If this argument label has a value other than null, it will be passed on to the result of this defer directive. This label is intended to give client applications a way to identify to which fragment a deferred result belongs to." label: String "Deferred when true." if: Boolean) on FRAGMENT_SPREAD | INLINE_FRAGMENT
+
+"The @deprecated directive is used within the type system definition language to indicate deprecated portions of a GraphQL service’s schema,such as deprecated fields on a type or deprecated enum values."
+directive @deprecated("Deprecations include a reason for why it is deprecated, which is formatted using Markdown syntax (as specified by CommonMark)." reason: String = "No longer supported") on FIELD_DEFINITION | ENUM_VALUE
+
+"Directs the executor to include this field or fragment only when the `if` argument is true."
+directive @include("Included when true." if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
+
+"Directs the executor to skip this field or fragment when the `if` argument is true."
+directive @skip("Skipped when true." if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
+
+"The `@stream` directive may be provided for a field of `List` type so that the backend can leverage technology such as asynchronous iterators to provide a partial list in the initial response, and additional list items in subsequent responses. `@include` and `@skip` take precedence over `@stream`."
+directive @stream("If this argument label has a value other than null, it will be passed on to the result of this stream directive. This label is intended to give client applications a way to identify to which fragment a streamed result belongs to." label: String "The initial elements that shall be send down to the consumer." initialCount: Int! "Streamed when true." if: Boolean!) on FIELD

src/HotChocolate/Language/src/Language.SyntaxTree/StringValueNode.cs

@@ -233,19 +233,13 @@ public ReadOnlySpan<byte> AsSpan()
             return _memory.Span;
         }
 
-        public StringValueNode WithLocation(Location? location)
-        {
-            return new(location, Value, Block);
-        }
+        public StringValueNode WithLocation(Location? location) =>
+            new(location, Value, Block);
 
-        public StringValueNode WithValue(string value)
-        {
-            return new(Location, value, false);
-        }
+        public StringValueNode WithValue(string value) =>
+            new(Location, value, false);
 
-        public StringValueNode WithValue(string value, bool block)
-        {
-            return new(Location, value, block);
-        }
+        public StringValueNode WithValue(string value, bool block) =>
+            new(Location, value, block);
     }
 }