Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed cache control defaults were not reliably applied. #5732

Merged
merged 2 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 42 additions & 10 deletions src/HotChocolate/Caching/src/Caching/CacheControlTypeInterceptor.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System;
using System.Collections.Generic;
using System.Linq;
using HotChocolate.Configuration;
using HotChocolate.Language;
Expand All @@ -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)
{
Expand All @@ -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();
Expand All @@ -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(
Expand All @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
66 changes: 66 additions & 0 deletions src/HotChocolate/Caching/test/Caching.Tests/SchemaTests.cs
Original file line number Diff line number Diff line change
@@ -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);
}
1 change: 0 additions & 1 deletion src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(">();");
}
Expand All @@ -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("));");
}
Expand All @@ -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(">();");
}
Expand All @@ -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(">();");
}
Expand Down
3 changes: 3 additions & 0 deletions src/HotChocolate/Core/src/Types/HotChocolate.Types.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
Expand All @@ -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));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace HotChocolate.Types.Descriptors;

public class ObjectTypeDescriptor
: DescriptorBase<ObjectTypeDefinition>
, IObjectTypeDescriptor
, IObjectTypeDescriptor
{
private readonly List<ObjectFieldDescriptor> _fields = new();

Expand Down
38 changes: 0 additions & 38 deletions src/HotChocolate/Core/src/Types/Types/ObjectTypeExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
43 changes: 43 additions & 0 deletions src/HotChocolate/Core/src/Types/Types/StaticObjectTypeExtension.cs
Original file line number Diff line number Diff line change
@@ -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();
}
}