From ecf9247f6481e326f4821d3a51788d94deda8d7c Mon Sep 17 00:00:00 2001 From: Glen Date: Mon, 30 Dec 2024 11:33:31 +0200 Subject: [PATCH] [Fusion] Added pre-merge validation rule "KeyFieldsHasArgumentsRule" (#7873) --- .../Logging/LogEntryCodes.cs | 1 + .../Logging/LogEntryHelper.cs | 20 +++ .../Rules/KeyFieldsHasArgumentsRule.cs | 33 +++++ .../CompositionResources.Designer.cs | 9 ++ .../Properties/CompositionResources.resx | 3 + .../Fusion.Composition/SourceSchemaMerger.cs | 1 + .../Rules/KeyFieldsHasArgumentsRuleTests.cs | 122 ++++++++++++++++++ 7 files changed, 189 insertions(+) create mode 100644 src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyFieldsHasArgumentsRule.cs create mode 100644 src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyFieldsHasArgumentsRuleTests.cs diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs index 0689461b9ab..bdad96c0d58 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs @@ -7,6 +7,7 @@ public static class LogEntryCodes public const string ExternalMissingOnBase = "EXTERNAL_MISSING_ON_BASE"; public const string ExternalUnused = "EXTERNAL_UNUSED"; public const string KeyDirectiveInFieldsArg = "KEY_DIRECTIVE_IN_FIELDS_ARG"; + public const string KeyFieldsHasArgs = "KEY_FIELDS_HAS_ARGS"; public const string KeyFieldsSelectInvalidType = "KEY_FIELDS_SELECT_INVALID_TYPE"; public const string OutputFieldTypesNotMergeable = "OUTPUT_FIELD_TYPES_NOT_MERGEABLE"; public const string RootMutationUsed = "ROOT_MUTATION_USED"; diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs index 9afc2e56d65..ff88a2aca4e 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs @@ -164,6 +164,26 @@ public static LogEntry KeyDirectiveInFieldsArgument( schema); } + public static LogEntry KeyFieldsHasArguments( + string entityTypeName, + Directive keyDirective, + string fieldName, + string typeName, + SchemaDefinition schema) + { + return new LogEntry( + string.Format( + LogEntryHelper_KeyFieldsHasArguments, + entityTypeName, + schema.Name, + new SchemaCoordinate(typeName, fieldName)), + LogEntryCodes.KeyFieldsHasArgs, + LogSeverity.Error, + new SchemaCoordinate(entityTypeName), + keyDirective, + schema); + } + public static LogEntry KeyFieldsSelectInvalidType( string entityTypeName, Directive keyDirective, diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyFieldsHasArgumentsRule.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyFieldsHasArgumentsRule.cs new file mode 100644 index 00000000000..fbf5d124a0d --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyFieldsHasArgumentsRule.cs @@ -0,0 +1,33 @@ +using HotChocolate.Fusion.Events; +using static HotChocolate.Fusion.Logging.LogEntryHelper; + +namespace HotChocolate.Fusion.PreMergeValidation.Rules; + +/// +/// The @key directive is used to define the set of fields that uniquely identify an entity. +/// These fields must not include any field that is defined with arguments, as arguments introduce +/// variability that prevents consistent and valid entity resolution across subgraphs. Fields +/// included in the fields argument of the @key directive must be static and +/// consistently resolvable. +/// +/// +/// Specification +/// +internal sealed class KeyFieldsHasArgumentsRule : IEventHandler +{ + public void Handle(KeyFieldEvent @event, CompositionContext context) + { + var (entityType, keyDirective, field, type, schema) = @event; + + if (field.Arguments.Count != 0) + { + context.Log.Write( + KeyFieldsHasArguments( + entityType.Name, + keyDirective, + field.Name, + type.Name, + schema)); + } + } +} diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.Designer.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.Designer.cs index 155cfbcf01a..f0d13b497eb 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.Designer.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.Designer.cs @@ -149,6 +149,15 @@ internal static string LogEntryHelper_KeyDirectiveInFieldsArgument { } } + /// + /// Looks up a localized string similar to An @key directive on type '{0}' in schema '{1}' references field '{2}', which must not have arguments.. + /// + internal static string LogEntryHelper_KeyFieldsHasArguments { + get { + return ResourceManager.GetString("LogEntryHelper_KeyFieldsHasArguments", resourceCulture); + } + } + /// /// Looks up a localized string similar to An @key directive on type '{0}' in schema '{1}' references field '{2}', which must not be a list, interface, or union type.. /// diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx index 3f2e94e9587..ee2e0335dba 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx @@ -48,6 +48,9 @@ An @key directive on type '{0}' in schema '{1}' references field '{2}', which must not include directive applications. + + An @key directive on type '{0}' in schema '{1}' references field '{2}', which must not have arguments. + An @key directive on type '{0}' in schema '{1}' references field '{2}', which must not be a list, interface, or union type. diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs index 6b4d2db05f6..913ce4e5c5e 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs @@ -51,6 +51,7 @@ private CompositionResult MergeSchemaDefinitions(CompositionCo new ExternalMissingOnBaseRule(), new ExternalUnusedRule(), new KeyDirectiveInFieldsArgumentRule(), + new KeyFieldsHasArgumentsRule(), new KeyFieldsSelectInvalidTypeRule(), new OutputFieldTypesMergeableRule(), new RootMutationUsedRule(), diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyFieldsHasArgumentsRuleTests.cs b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyFieldsHasArgumentsRuleTests.cs new file mode 100644 index 00000000000..67d3f5f521b --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyFieldsHasArgumentsRuleTests.cs @@ -0,0 +1,122 @@ +using HotChocolate.Fusion.Logging; +using HotChocolate.Fusion.PreMergeValidation; +using HotChocolate.Fusion.PreMergeValidation.Rules; + +namespace HotChocolate.Composition.PreMergeValidation.Rules; + +public sealed class KeyFieldsHasArgumentsRuleTests : CompositionTestBase +{ + private readonly PreMergeValidator _preMergeValidator = new([new KeyFieldsHasArgumentsRule()]); + + [Theory] + [MemberData(nameof(ValidExamplesData))] + public void Examples_Valid(string[] sdl) + { + // arrange + var context = CreateCompositionContext(sdl); + + // act + var result = _preMergeValidator.Validate(context); + + // assert + Assert.True(result.IsSuccess); + Assert.True(context.Log.IsEmpty); + } + + [Theory] + [MemberData(nameof(InvalidExamplesData))] + public void Examples_Invalid(string[] sdl, string[] errorMessages) + { + // arrange + var context = CreateCompositionContext(sdl); + + // act + var result = _preMergeValidator.Validate(context); + + // assert + Assert.True(result.IsFailure); + Assert.Equal(errorMessages, context.Log.Select(e => e.Message).ToArray()); + Assert.True(context.Log.All(e => e.Code == "KEY_FIELDS_HAS_ARGS")); + Assert.True(context.Log.All(e => e.Severity == LogSeverity.Error)); + } + + public static TheoryData ValidExamplesData() + { + return new TheoryData + { + // In this example, the `User` type has a valid `@key` directive that references the + // argument-free fields `id` and `name`. + { + [ + """ + type User @key(fields: "id name") { + id: ID! + name: String + tags: [String] + } + """ + ] + } + }; + } + + public static TheoryData InvalidExamplesData() + { + return new TheoryData + { + // In this example, the `@key` directive references a field (`tags`) that is defined + // with arguments (`limit`), which is not allowed. + { + [ + """ + type User @key(fields: "id tags") { + id: ID! + tags(limit: Int = 10): [String] + } + """ + ], + [ + "An @key directive on type 'User' in schema 'A' references field " + + "'User.tags', which must not have arguments." + ] + }, + // Nested field. + { + [ + """ + type User @key(fields: "id info { tags }") { + id: ID! + info: UserInfo + } + + type UserInfo { + tags(limit: Int = 10): [String] + } + """ + ], + [ + "An @key directive on type 'User' in schema 'A' references field " + + "'UserInfo.tags', which must not have arguments." + ] + }, + // Multiple keys. + { + [ + """ + type User @key(fields: "id") @key(fields: "tags") { + id(global: Boolean = true): ID! + tags(limit: Int = 10): [String] + } + """ + ], + [ + "An @key directive on type 'User' in schema 'A' references field " + + "'User.id', which must not have arguments.", + + "An @key directive on type 'User' in schema 'A' references field " + + "'User.tags', which must not have arguments." + ] + } + }; + } +}