From 7e5bdf8decd86741897980c7b1fb8e4dd9539f40 Mon Sep 17 00:00:00 2001 From: Glen Date: Mon, 2 Sep 2024 10:51:51 +0200 Subject: [PATCH] Omitted requireOneSlicingArgument if not set in listSize directive --- .../CostAnalysis/Types/ListSizeAttribute.cs | 9 ++- .../CostAnalysis/Types/ListSizeDirective.cs | 8 +-- .../Types/ListSizeDirectiveType.cs | 5 +- .../test/CostAnalysis.Tests/AttributeTests.cs | 57 ++++++++++++++++++- 4 files changed, 69 insertions(+), 10 deletions(-) diff --git a/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeAttribute.cs b/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeAttribute.cs index 16bf14efee0..f9d3901b048 100644 --- a/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeAttribute.cs +++ b/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeAttribute.cs @@ -13,6 +13,7 @@ namespace HotChocolate.CostAnalysis.Types; public sealed class ListSizeAttribute : ObjectFieldDescriptorAttribute { private readonly int? _assumedSize; + private readonly bool? _requireOneSlicingArgument; /// /// The maximum length of the list returned by this field. @@ -38,7 +39,11 @@ public int AssumedSize /// Whether to require a single slicing argument in the query. If that is not the case (i.e., if /// none or multiple slicing arguments are present), the static analysis will throw an error. /// - public bool RequireOneSlicingArgument { get; init; } = true; + public bool RequireOneSlicingArgument + { + get => _requireOneSlicingArgument ?? true; + init => _requireOneSlicingArgument = value; + } protected override void OnConfigure( IDescriptorContext context, @@ -50,6 +55,6 @@ protected override void OnConfigure( _assumedSize, SlicingArguments?.ToImmutableArray(), SizedFields?.ToImmutableArray(), - RequireOneSlicingArgument)); + _requireOneSlicingArgument)); } } diff --git a/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeDirective.cs b/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeDirective.cs index ceace987f75..98f1ddf37fa 100644 --- a/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeDirective.cs +++ b/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeDirective.cs @@ -28,11 +28,7 @@ public ListSizeDirective(int? assumedSize = null, AssumedSize = assumedSize; SlicingArguments = slicingArguments ?? ImmutableArray.Empty; SizedFields = sizedFields ?? ImmutableArray.Empty; - - // https://ibm.github.io/graphql-specs/cost-spec.html#sec-requireOneSlicingArgument - // Per default, requireOneSlicingArgument is enabled, - // and has to be explicitly disabled if not desired for a field. - RequireOneSlicingArgument = SlicingArguments is { Length: > 0 } && (requireOneSlicingArgument ?? true); + RequireOneSlicingArgument = requireOneSlicingArgument; } /// @@ -73,5 +69,5 @@ public ListSizeDirective(int? assumedSize = null, /// /// Specification URL /// - public bool RequireOneSlicingArgument { get; } + public bool? RequireOneSlicingArgument { get; } } diff --git a/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeDirectiveType.cs b/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeDirectiveType.cs index c20b9f8a55d..53454f18e4e 100644 --- a/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeDirectiveType.cs +++ b/src/HotChocolate/CostAnalysis/src/CostAnalysis/Types/ListSizeDirectiveType.cs @@ -144,7 +144,10 @@ private static DirectiveNode FormatValue(object value) arguments.Add(new ArgumentNode(SizedFields, directive.SizedFields.ToListValueNode())); } - arguments.Add(new ArgumentNode(RequireOneSlicingArgument, directive.RequireOneSlicingArgument)); + if (directive.RequireOneSlicingArgument is not null) + { + arguments.Add(new ArgumentNode(RequireOneSlicingArgument, directive.RequireOneSlicingArgument.Value)); + } return new DirectiveNode(_name, arguments.ToImmutableArray()); } diff --git a/src/HotChocolate/CostAnalysis/test/CostAnalysis.Tests/AttributeTests.cs b/src/HotChocolate/CostAnalysis/test/CostAnalysis.Tests/AttributeTests.cs index c41d68fe4c1..08dddfc9d7d 100644 --- a/src/HotChocolate/CostAnalysis/test/CostAnalysis.Tests/AttributeTests.cs +++ b/src/HotChocolate/CostAnalysis/test/CostAnalysis.Tests/AttributeTests.cs @@ -1,4 +1,6 @@ +using CookieCrumble; using HotChocolate.CostAnalysis.Types; +using HotChocolate.Language.Utilities; using HotChocolate.Types; namespace HotChocolate.CostAnalysis; @@ -112,6 +114,36 @@ public void ListSize_ObjectFieldAttribute_AppliesDirective() Assert.False(costDirective.RequireOneSlicingArgument); } + [Fact] + public void ListSize_ObjectFieldAttribute_AppliesRequireOneSlicingArgumentCorrectly() + { + // arrange & act + var query = CreateSchema().GetType(OperationTypeNames.Query); + + var listSizeDirective1Sdl = query.Fields["examplesAssumedSizeOnly"] + .Directives + .Single(d => d.Type.Name == "listSize") + .AsSyntaxNode() + .Print(); + + var listSizeDirective2Sdl = query.Fields["examplesRequireOneSlicingArgumentTrue"] + .Directives + .Single(d => d.Type.Name == "listSize") + .AsSyntaxNode() + .Print(); + + var listSizeDirective3Sdl = query.Fields["examplesRequireOneSlicingArgumentFalse"] + .Directives + .Single(d => d.Type.Name == "listSize") + .AsSyntaxNode() + .Print(); + + // assert + listSizeDirective1Sdl.MatchInlineSnapshot("@listSize(assumedSize: 10)"); + listSizeDirective2Sdl.MatchInlineSnapshot("@listSize(requireOneSlicingArgument: true)"); + listSizeDirective3Sdl.MatchInlineSnapshot("@listSize(requireOneSlicingArgument: false)"); + } + private static ISchema CreateSchema() { return SchemaBuilder.New() @@ -128,6 +160,8 @@ private static ISchema CreateSchema() [QueryType] private static class Queries { + private static readonly List List = [new Example(ExampleEnum.Member)]; + [ListSize( AssumedSize = 10, SlicingArguments = ["first", "last"], @@ -137,7 +171,28 @@ private static class Queries // ReSharper disable once UnusedMember.Local public static List GetExamples([Cost(8.0)] ExampleInput _) { - return [new Example(ExampleEnum.Member)]; + return List; + } + + [ListSize(AssumedSize = 10)] + // ReSharper disable once UnusedMember.Local + public static List GetExamplesAssumedSizeOnly() + { + return List; + } + + [ListSize(RequireOneSlicingArgument = true)] + // ReSharper disable once UnusedMember.Local + public static List GetExamplesRequireOneSlicingArgumentTrue() + { + return List; + } + + [ListSize(RequireOneSlicingArgument = false)] + // ReSharper disable once UnusedMember.Local + public static List GetExamplesRequireOneSlicingArgumentFalse() + { + return List; } }