Fixed cache control defaults were not reliably applied (#5732)

src/HotChocolate/Caching/src/Caching/CacheControlTypeInterceptor.cs

@@ -1,3 +1,5 @@
+using System;
+using System.Collections.Generic;
 using System.Linq;
 using HotChocolate.Configuration;
 using HotChocolate.Language;
@@ -10,14 +12,16 @@ namespace HotChocolate.Caching;
 
 internal sealed class CacheControlTypeInterceptor : TypeInterceptor
 {
+    private readonly List<(RegisteredType Type, ObjectTypeDefinition TypeDef)> _types = new();
     private readonly ICacheControlOptions _cacheControlOptions;
+    private TypeDependency? _cacheControlDependency;
 
     public CacheControlTypeInterceptor(ICacheControlOptionsAccessor accessor)
     {
         _cacheControlOptions = accessor.CacheControl;
     }
 
-    public override void OnBeforeCompleteType(
+    public override void OnBeforeCompleteName(
         ITypeCompletionContext completionContext,
         DefinitionBase definition)
     {
@@ -26,19 +30,42 @@ public CacheControlTypeInterceptor(ICacheControlOptionsAccessor accessor)
             return;
         }
 
-        if (completionContext.IsIntrospectionType ||
-            completionContext.IsSubscriptionType == true ||
-            completionContext.IsMutationType == true)
+        if (completionContext.Type is ObjectType && definition is ObjectTypeDefinition typeDef)
+        {
+            _types.Add(((RegisteredType)completionContext, typeDef));
+        }
+    }
+
+    public override void OnAfterMergeTypeExtensions()
+    {
+        foreach (var item in _types)
+        {
+            TryApplyDefaults(item.Type, item.TypeDef);
+        }
+    }
+
+    private void TryApplyDefaults(RegisteredType type, ObjectTypeDefinition objectDef)
+    {
+        if (!_cacheControlOptions.Enable || !_cacheControlOptions.ApplyDefaults)
         {
             return;
         }
 
-        if (definition is not ObjectTypeDefinition objectDef)
+        if (type.IsIntrospectionType ||
+            type.IsSubscriptionType == true ||
+            type.IsMutationType == true)
         {
             return;
         }
 
+        _cacheControlDependency ??= new TypeDependency(
+            new ExtendedTypeDirectiveReference(
+                type.TypeInspector.GetType(
+                    typeof(CacheControlDirectiveType))),
+            TypeDependencyFulfilled.Completed);
+
         var length = objectDef.Fields.Count;
+        var appliedDefaults = false;
 
 #if NET6_0_OR_GREATER
         var fields = ((BindableList<ObjectFieldDefinition>)objectDef.Fields).AsSpan();
@@ -63,17 +90,24 @@ public CacheControlTypeInterceptor(ICacheControlOptionsAccessor accessor)
                 continue;
             }
 
-            if (completionContext.IsQueryType == true ||
+            if (type.IsQueryType == true ||
                 CostTypeInterceptor.IsDataResolver(field))
             {
                 // Each field on the query type or data resolver fields
                 // are treated as fields that need to be explicitly cached.
                 ApplyCacheControlWithDefaultMaxAge(field);
+                appliedDefaults = true;
             }
         }
+
+        if (appliedDefaults)
+        {
+            type.Dependencies.Add(_cacheControlDependency);
+        }
     }
 
-    private void ApplyCacheControlWithDefaultMaxAge(OutputFieldDefinitionBase field)
+    private void ApplyCacheControlWithDefaultMaxAge(
+        OutputFieldDefinitionBase field)
     {
         field.Directives.Add(
             new DirectiveDefinition(
@@ -85,9 +119,7 @@ private void ApplyCacheControlWithDefaultMaxAge(OutputFieldDefinitionBase field)
     }
 
     private static bool HasCacheControlDirective(ObjectFieldDefinition field)
-    {
-        return field.Directives.Any(IsCacheControlDirective);
-    }
+        => field.Directives.Any(static d => IsCacheControlDirective(d));
 
     private static bool IsCacheControlDirective(DirectiveDefinition directive)
     {

src/HotChocolate/Caching/src/Caching/Types/CacheControlScope.cs

@@ -11,6 +11,7 @@ public enum CacheControlScope
     /// be only cached for and accessible to that user.
     /// </summary>
     Private,
+
     /// <summary>
     /// Public query results contain data
     /// that is not scoped to a particular user and can

src/HotChocolate/Caching/test/Caching.Tests/SchemaTests.cs

@@ -0,0 +1,66 @@
+using System.Threading.Tasks;
+using CookieCrumble;
+using HotChocolate.Execution;
+using HotChocolate.Types;
+using Microsoft.Extensions.DependencyInjection;
+using Xunit;
+
+namespace HotChocolate.Caching.Tests;
+
+public class SchemaTests
+{
+    [Fact]
+    public async Task Allow_CacheControl_On_FieldDefinition()
+    {
+        var schema =
+            await new ServiceCollection()
+                .AddGraphQLServer()
+                .AddTypeExtension(typeof(Query))
+                .ConfigureSchema(
+                    b => b.TryAddRootType(
+                        () => new ObjectType(
+                            d => d.Name(OperationTypeNames.Query)),
+                        Language.OperationType.Query))
+                .AddCacheControl()
+                .BuildSchemaAsync();
+
+        schema.MatchInlineSnapshot(
+            """
+            schema {
+              query: Query
+            }
+
+            type Book {
+              title: String! @cacheControl(maxAge: 5000)
+              description: String!
+            }
+
+            type Query {
+              book: Book! @cacheControl(maxAge: 0)
+            }
+
+            "The scope of a cache hint."
+            enum CacheControlScope {
+              "The value to cache is not tied to a single user."
+              PUBLIC
+              "The value to cache is specific to a single user."
+              PRIVATE
+            }
+
+            "The `@cacheControl` directive may be provided for individual fields or entire object, interface or union types to provide caching hints to the executor."
+            directive @cacheControl("The maximum amount of time this field's cached value is valid, in seconds." maxAge: Int "If `PRIVATE`, the field's value is specific to a single user. The default value is `PUBLIC`, which means the field's value is not tied to a single user." scope: CacheControlScope "If `true`, the field inherits the `maxAge` of its parent field." inheritMaxAge: Boolean) on OBJECT | FIELD_DEFINITION | INTERFACE | UNION
+            """);
+    }
+
+    [QueryType]
+    public static class Query
+    {
+        public static Book GetBook()
+            => new Book("C# in depth.", "abc");
+
+}
+
+    public record Book(
+        [property: CacheControl(5000)] string Title,
+        string Description);
+}

src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs

@@ -197,7 +197,6 @@ public static class Schema
         public const string UnresolvedTypes = "TS_UNRESOLVED_TYPES";
         public const string NoName = "TS_NO_NAME_DEFINED";
         public const string NoFieldType = "TS_NO_FIELD_TYPE";
-        public const string ArgumentValueTypeWrong = "TS_ARG_VALUE_TYPE_WRONG";
         public const string InvalidArgument = "TS_INVALID_ARG";
         public const string InterfaceNotImplemented = "SCHEMA_INTERFACE_NO_IMPL";
         public const string DuplicateTypeName = "HC0065";

src/HotChocolate/Core/src/Types.Analyzers/Generators/ModuleGenerator.cs

@@ -85,7 +85,7 @@ public bool Consume(ISyntaxInfo syntaxInfo)
                         sourceText.Append(Indent)
                             .Append(Indent)
                             .Append(Indent)
-                            .Append("builder.AddType<")
+                            .Append("builder.AddType<global::")
                             .Append(type.Name)
                             .AppendLine(">();");
                     }
@@ -100,7 +100,7 @@ public bool Consume(ISyntaxInfo syntaxInfo)
                             sourceText.Append(Indent)
                                 .Append(Indent)
                                 .Append(Indent)
-                                .Append("builder.AddTypeExtension(typeof(")
+                                .Append("builder.AddTypeExtension(typeof(global::")
                                 .Append(extension.Name)
                                 .AppendLine("));");
                         }
@@ -109,7 +109,7 @@ public bool Consume(ISyntaxInfo syntaxInfo)
                             sourceText.Append(Indent)
                                 .Append(Indent)
                                 .Append(Indent)
-                                .Append("builder.AddTypeExtension<")
+                                .Append("builder.AddTypeExtension<global::")
                                 .Append(extension.Name)
                                 .AppendLine(">();");
                         }
@@ -129,7 +129,7 @@ public bool Consume(ISyntaxInfo syntaxInfo)
                         sourceText.Append(Indent)
                             .Append(Indent)
                             .Append(Indent)
-                            .Append("builder.AddDataLoader<")
+                            .Append("builder.AddDataLoader<global::")
                             .Append(dataLoader.Name)
                             .AppendLine(">();");
                     }

src/HotChocolate/Core/src/Types/HotChocolate.Types.csproj

@@ -99,6 +99,9 @@
     <Compile Update="Configuration\RegisteredType.DiscoveryContext.cs">
       <DependentUpon>RegisteredType.cs</DependentUpon>
     </Compile>
+    <Compile Update="Types\StaticObjectTypeExtension.cs">
+      <DependentUpon>ObjectTypeExtension.cs</DependentUpon>
+    </Compile>
   </ItemGroup>
 
   <ItemGroup>

src/HotChocolate/Core/src/Types/Internal/TypeDependencyHelper.cs

@@ -185,7 +185,7 @@ public static class TypeDependencyHelper
         {
             foreach (var directive in definition.Directives)
             {
-                dependencies.Add(new(directive.Type, Completed));
+                dependencies.Add(new TypeDependency(directive.Type, Completed));
             }
         }
     }
@@ -198,7 +198,7 @@ public static class TypeDependencyHelper
         {
             foreach (var directive in definition.Directives)
             {
-                dependencies.Add(new(directive.Type, Completed));
+                dependencies.Add(new TypeDependency(directive.Type, Completed));
             }
         }
     }

src/HotChocolate/Core/src/Types/Internal/TypeDiscoveryHandler.cs

@@ -6,7 +6,7 @@
 namespace HotChocolate.Internal;
 
 /// <summary>
-/// A type discover handler allows to specify how types are inferred
+/// A type discovery handler allows to specify how types are inferred
 /// from <see cref="ExtendedTypeReference"/>s.
 /// </summary>
 public abstract class TypeDiscoveryHandler

src/HotChocolate/Core/src/Types/Types/Descriptors/ObjectTypeDescriptor.cs

@@ -15,7 +15,7 @@ namespace HotChocolate.Types.Descriptors;
 
 public class ObjectTypeDescriptor
     : DescriptorBase<ObjectTypeDefinition>
-        , IObjectTypeDescriptor
+    , IObjectTypeDescriptor
 {
     private readonly List<ObjectFieldDescriptor> _fields = new();
 

src/HotChocolate/Core/src/Types/Types/ObjectTypeExtension.cs

@@ -158,41 +158,3 @@ protected override ObjectTypeDefinition CreateDefinition(ITypeDiscoveryContext c
         }
     }
 }
-
-/// <summary>
-/// This helper class is used to allow static type extensions.
-/// </summary>
-internal sealed class StaticObjectTypeExtension : ObjectTypeExtension
-{
-    private readonly Type _staticExtType;
-
-    public StaticObjectTypeExtension(Type staticExtType)
-        => _staticExtType = staticExtType;
-
-    protected override void Configure(IObjectTypeDescriptor descriptor)
-    {
-        var context = descriptor.Extend().Context;
-        var definition = descriptor.Extend().Definition;
-
-        // we are using the non-generic type extension class which would set nothing.
-        definition.Name = context.Naming.GetTypeName(_staticExtType, TypeKind.Object);
-        definition.Description = context.Naming.GetTypeDescription(_staticExtType, TypeKind.Object);
-        definition.Fields.BindingBehavior = context.Options.DefaultBindingBehavior;
-        definition.FieldBindingFlags = context.Options.DefaultFieldBindingFlags;
-        definition.FieldBindingType = _staticExtType;
-        definition.IsExtension = true;
-
-        // we set the static type as runtime type. Since this is not the actual runtime
-        // type and is replaced by the actual runtime type of the GraphQL type
-        // we do not run into any conflicts here.
-        definition.RuntimeType = _staticExtType;
-
-        // next we set the binding flags to only infer static members.
-        definition.FieldBindingFlags = FieldBindingFlags.Static;
-
-        // last we use an internal helper to force infer the GraphQL fields from the
-        // field binding type which is at this moment the runtime type that we have
-        // set above.
-        ((ObjectTypeDescriptor)descriptor).InferFieldsFromFieldBindingType();
-    }
-}

src/HotChocolate/Core/src/Types/Types/StaticObjectTypeExtension.cs

@@ -0,0 +1,43 @@
+#nullable enable
+using System;
+using HotChocolate.Types.Descriptors;
+
+namespace HotChocolate.Types;
+
+/// <summary>
+/// This helper class is used to allow static type extensions.
+/// </summary>
+internal sealed class StaticObjectTypeExtension : ObjectTypeExtension
+{
+    private readonly Type _staticExtType;
+
+    public StaticObjectTypeExtension(Type staticExtType)
+        => _staticExtType = staticExtType;
+
+    protected override void Configure(IObjectTypeDescriptor descriptor)
+    {
+        var context = descriptor.Extend().Context;
+        var definition = descriptor.Extend().Definition;
+
+        // we are using the non-generic type extension class which would set nothing.
+        definition.Name = context.Naming.GetTypeName(_staticExtType, TypeKind.Object);
+        definition.Description = context.Naming.GetTypeDescription(_staticExtType, TypeKind.Object);
+        definition.Fields.BindingBehavior = context.Options.DefaultBindingBehavior;
+        definition.FieldBindingFlags = context.Options.DefaultFieldBindingFlags;
+        definition.FieldBindingType = _staticExtType;
+        definition.IsExtension = true;
+
+        // we set the static type as runtime type. Since this is not the actual runtime
+        // type and is replaced by the actual runtime type of the GraphQL type
+        // we do not run into any conflicts here.
+        definition.RuntimeType = _staticExtType;
+
+        // next we set the binding flags to only infer static members.
+        definition.FieldBindingFlags = FieldBindingFlags.Static;
+
+        // last we use an internal helper to force infer the GraphQL fields from the
+        // field binding type which is at this moment the runtime type that we have
+        // set above.
+        ((ObjectTypeDescriptor)descriptor).InferFieldsFromFieldBindingType();
+    }
+}