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 20fbc87cda2..0689461b9ab 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs @@ -6,6 +6,7 @@ public static class LogEntryCodes public const string ExternalArgumentDefaultMismatch = "EXTERNAL_ARGUMENT_DEFAULT_MISMATCH"; 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 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 695196247c2..9afc2e56d65 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs @@ -1,3 +1,4 @@ +using System.Collections.Immutable; using HotChocolate.Skimmed; using static HotChocolate.Fusion.Properties.CompositionResources; @@ -144,6 +145,25 @@ public static LogEntry ExternalUnused( schema); } + public static LogEntry KeyDirectiveInFieldsArgument( + string entityTypeName, + Directive keyDirective, + ImmutableArray fieldNamePath, + SchemaDefinition schema) + { + return new LogEntry( + string.Format( + LogEntryHelper_KeyDirectiveInFieldsArgument, + entityTypeName, + schema.Name, + string.Join(".", fieldNamePath)), + LogEntryCodes.KeyDirectiveInFieldsArg, + 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/Events.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Events.cs index f1062f07d13..1ce5bdc790a 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Events.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Events.cs @@ -1,5 +1,6 @@ using System.Collections.Immutable; using HotChocolate.Fusion.Events; +using HotChocolate.Language; using HotChocolate.Skimmed; namespace HotChocolate.Fusion.PreMergeValidation; @@ -32,6 +33,13 @@ internal record KeyFieldEvent( ComplexTypeDefinition Type, SchemaDefinition Schema) : IEvent; +internal record KeyFieldNodeEvent( + ComplexTypeDefinition EntityType, + Directive KeyDirective, + FieldNode FieldNode, + ImmutableArray FieldNamePath, + SchemaDefinition Schema) : IEvent; + internal record OutputFieldEvent( OutputFieldDefinition Field, INamedTypeDefinition Type, diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/PreMergeValidator.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/PreMergeValidator.cs index cc7c29e13d7..c80d77f81a4 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/PreMergeValidator.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/PreMergeValidator.cs @@ -142,6 +142,7 @@ private void PublishEntityEvents( selectionSet, entityType, keyDirective, + [], entityType, schema, context); @@ -157,6 +158,7 @@ private void PublishKeyFieldEvents( SelectionSetNode selectionSet, ComplexTypeDefinition entityType, Directive keyDirective, + List fieldNamePath, ComplexTypeDefinition parentType, SchemaDefinition schema, CompositionContext context) @@ -165,6 +167,17 @@ private void PublishKeyFieldEvents( { if (selection is FieldNode fieldNode) { + fieldNamePath.Add(fieldNode.Name.Value); + + PublishEvent( + new KeyFieldNodeEvent( + entityType, + keyDirective, + fieldNode, + [.. fieldNamePath], + schema), + context); + if (parentType.Fields.TryGetField(fieldNode.Name.Value, out var field)) { PublishEvent( @@ -188,10 +201,13 @@ private void PublishKeyFieldEvents( fieldNode.SelectionSet, entityType, keyDirective, + fieldNamePath, parentType, schema, context); } + + fieldNamePath = []; } } } diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyDirectiveInFieldsArgumentRule.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyDirectiveInFieldsArgumentRule.cs new file mode 100644 index 00000000000..a283626ed88 --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyDirectiveInFieldsArgumentRule.cs @@ -0,0 +1,30 @@ +using HotChocolate.Fusion.Events; +using static HotChocolate.Fusion.Logging.LogEntryHelper; + +namespace HotChocolate.Fusion.PreMergeValidation.Rules; + +/// +/// The @key directive specifies the set of fields used to uniquely identify an entity. The +/// fields argument must consist of a valid GraphQL selection set that does not include any +/// directive applications. Directives in the fields argument are not supported. +/// +/// +/// Specification +/// +internal sealed class KeyDirectiveInFieldsArgumentRule : IEventHandler +{ + public void Handle(KeyFieldNodeEvent @event, CompositionContext context) + { + var (entityType, keyDirective, fieldNode, fieldNamePath, schema) = @event; + + if (fieldNode.Directives.Count != 0) + { + context.Log.Write( + KeyDirectiveInFieldsArgument( + entityType.Name, + keyDirective, + fieldNamePath, + 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 c41bbad9329..155cfbcf01a 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 @@ -140,6 +140,15 @@ internal static string LogEntryHelper_ExternalUnused { } } + /// + /// Looks up a localized string similar to An @key directive on type '{0}' in schema '{1}' references field '{2}', which must not include directive applications.. + /// + internal static string LogEntryHelper_KeyDirectiveInFieldsArgument { + get { + return ResourceManager.GetString("LogEntryHelper_KeyDirectiveInFieldsArgument", 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 a2ef422c968..3f2e94e9587 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx @@ -45,6 +45,9 @@ External field '{0}' in schema '{1}' is not referenced by an @provides directive in the schema. + + 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 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 1183281bdda..6b4d2db05f6 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs @@ -50,6 +50,7 @@ private CompositionResult MergeSchemaDefinitions(CompositionCo new ExternalArgumentDefaultMismatchRule(), new ExternalMissingOnBaseRule(), new ExternalUnusedRule(), + new KeyDirectiveInFieldsArgumentRule(), new KeyFieldsSelectInvalidTypeRule(), new OutputFieldTypesMergeableRule(), new RootMutationUsedRule(), diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyDirectiveInFieldsArgumentRuleTests.cs b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyDirectiveInFieldsArgumentRuleTests.cs new file mode 100644 index 00000000000..f24d5419779 --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyDirectiveInFieldsArgumentRuleTests.cs @@ -0,0 +1,130 @@ +using HotChocolate.Fusion.Logging; +using HotChocolate.Fusion.PreMergeValidation; +using HotChocolate.Fusion.PreMergeValidation.Rules; + +namespace HotChocolate.Composition.PreMergeValidation.Rules; + +public sealed class KeyDirectiveInFieldsArgumentRuleTests : CompositionTestBase +{ + private readonly PreMergeValidator _preMergeValidator = + new([new KeyDirectiveInFieldsArgumentRule()]); + + [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_DIRECTIVE_IN_FIELDS_ARG")); + Assert.True(context.Log.All(e => e.Severity == LogSeverity.Error)); + } + + public static TheoryData ValidExamplesData() + { + return new TheoryData + { + // In this example, the `fields` argument of the `@key` directive does not include any + // directive applications, satisfying the rule. + { + [ + """ + type User @key(fields: "id name") { + id: ID! + name: String + } + """ + ] + } + }; + } + + public static TheoryData InvalidExamplesData() + { + return new TheoryData + { + // In this example, the `fields` argument of the `@key` directive includes a directive + // application `@lowercase`, which is not allowed. + { + [ + """ + directive @lowercase on FIELD_DEFINITION + + type User @key(fields: "id name @lowercase") { + id: ID! + name: String + } + """ + ], + [ + "An @key directive on type 'User' in schema 'A' references field 'name', " + + "which must not include directive applications." + ] + }, + // In this example, the `fields` argument includes a directive application `@lowercase` + // nested inside the selection set, which is also invalid. + { + [ + """ + directive @lowercase on FIELD_DEFINITION + + type User @key(fields: "id name { firstName @lowercase }") { + id: ID! + name: FullName + } + + type FullName { + firstName: String + lastName: String + } + """ + ], + [ + "An @key directive on type 'User' in schema 'A' references field " + + "'name.firstName', which must not include directive applications." + ] + }, + // Multiple keys. + { + [ + """ + directive @example on FIELD_DEFINITION + + type User @key(fields: "id @example") @key(fields: "name @example") { + id: ID! + name: String + } + """ + ], + [ + "An @key directive on type 'User' in schema 'A' references field 'id', " + + "which must not include directive applications.", + + "An @key directive on type 'User' in schema 'A' references field 'name', " + + "which must not include directive applications." + ] + } + }; + } +}