From a265c91450e7c8bbaa55d85badeaaec6fbf0bd86 Mon Sep 17 00:00:00 2001 From: Glen Date: Fri, 27 Dec 2024 12:12:00 +0200 Subject: [PATCH] [Fusion] Added pre-merge validation rule "KeyFieldsSelectInvalidTypeRule" (#7871) --- dictionary.txt | 1 + .../Logging/LogEntryCodes.cs | 1 + .../Logging/LogEntryHelper.cs | 20 ++ .../PreMergeValidation/Events.cs | 7 + .../PreMergeValidation/PreMergeValidator.cs | 88 +++++++ .../Rules/KeyFieldsSelectInvalidTypeRule.cs | 36 +++ .../CompositionResources.Designer.cs | 9 + .../Properties/CompositionResources.resx | 3 + .../Fusion.Composition/SourceSchemaMerger.cs | 1 + .../WellKnownDirectiveNames.cs | 1 + .../KeyFieldsSelectInvalidTypeRuleTests.cs | 228 ++++++++++++++++++ 11 files changed, 395 insertions(+) create mode 100644 src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyFieldsSelectInvalidTypeRule.cs create mode 100644 src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyFieldsSelectInvalidTypeRuleTests.cs diff --git a/dictionary.txt b/dictionary.txt index 0cdce2781d9..4427a17decc 100644 --- a/dictionary.txt +++ b/dictionary.txt @@ -153,6 +153,7 @@ Storeless strawberryshake streamable Structs +subgraphs sublicensable supergraph Swashbuckle 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 dcbadd4f986..20fbc87cda2 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 KeyFieldsSelectInvalidType = "KEY_FIELDS_SELECT_INVALID_TYPE"; public const string OutputFieldTypesNotMergeable = "OUTPUT_FIELD_TYPES_NOT_MERGEABLE"; public const string RootMutationUsed = "ROOT_MUTATION_USED"; public const string RootQueryUsed = "ROOT_QUERY_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 8ced6e05df8..695196247c2 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs @@ -144,6 +144,26 @@ public static LogEntry ExternalUnused( schema); } + public static LogEntry KeyFieldsSelectInvalidType( + string entityTypeName, + Directive keyDirective, + string fieldName, + string typeName, + SchemaDefinition schema) + { + return new LogEntry( + string.Format( + LogEntryHelper_KeyFieldsSelectInvalidType, + entityTypeName, + schema.Name, + new SchemaCoordinate(typeName, fieldName)), + LogEntryCodes.KeyFieldsSelectInvalidType, + LogSeverity.Error, + new SchemaCoordinate(entityTypeName), + keyDirective, + schema); + } + public static LogEntry OutputFieldTypesNotMergeable( OutputFieldDefinition field, string typeName, 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 2ad878c72e1..f1062f07d13 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Events.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Events.cs @@ -25,6 +25,13 @@ internal record FieldArgumentGroupEvent( string FieldName, string TypeName) : IEvent; +internal record KeyFieldEvent( + ComplexTypeDefinition EntityType, + Directive KeyDirective, + OutputFieldDefinition Field, + ComplexTypeDefinition Type, + 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 8af32e40155..cc7c29e13d7 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/PreMergeValidator.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/PreMergeValidator.cs @@ -3,7 +3,9 @@ using HotChocolate.Fusion.Errors; using HotChocolate.Fusion.Events; using HotChocolate.Fusion.Results; +using HotChocolate.Language; using HotChocolate.Skimmed; +using static HotChocolate.Language.Utf8GraphQLParser; namespace HotChocolate.Fusion.PreMergeValidation; @@ -36,6 +38,11 @@ private void PublishEvents(CompositionContext context) if (type is ComplexTypeDefinition complexType) { + if (complexType.Directives.ContainsName(WellKnownDirectiveNames.Key)) + { + PublishEntityEvents(complexType, schema, context); + } + foreach (var field in complexType.Fields) { PublishEvent(new OutputFieldEvent(field, type, schema), context); @@ -108,6 +115,87 @@ private void PublishEvents(CompositionContext context) } } + private void PublishEntityEvents( + ComplexTypeDefinition entityType, + SchemaDefinition schema, + CompositionContext context) + { + var keyDirectives = + entityType.Directives + .Where(d => d.Name == WellKnownDirectiveNames.Key) + .ToArray(); + + foreach (var keyDirective in keyDirectives) + { + if ( + !keyDirective.Arguments.TryGetValue(WellKnownArgumentNames.Fields, out var f) + || f is not StringValueNode fields) + { + continue; + } + + try + { + var selectionSet = Syntax.ParseSelectionSet($"{{{fields.Value}}}"); + + PublishKeyFieldEvents( + selectionSet, + entityType, + keyDirective, + entityType, + schema, + context); + } + catch (SyntaxException) + { + // Ignore. + } + } + } + + private void PublishKeyFieldEvents( + SelectionSetNode selectionSet, + ComplexTypeDefinition entityType, + Directive keyDirective, + ComplexTypeDefinition parentType, + SchemaDefinition schema, + CompositionContext context) + { + foreach (var selection in selectionSet.Selections) + { + if (selection is FieldNode fieldNode) + { + if (parentType.Fields.TryGetField(fieldNode.Name.Value, out var field)) + { + PublishEvent( + new KeyFieldEvent( + entityType, + keyDirective, + field, + parentType, + schema), + context); + + if (field.Type.NullableType() is ComplexTypeDefinition fieldType) + { + parentType = fieldType; + } + } + + if (fieldNode.SelectionSet is not null) + { + PublishKeyFieldEvents( + fieldNode.SelectionSet, + entityType, + keyDirective, + parentType, + schema, + context); + } + } + } + } + private void PublishEvent(TEvent @event, CompositionContext context) where TEvent : IEvent { diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyFieldsSelectInvalidTypeRule.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyFieldsSelectInvalidTypeRule.cs new file mode 100644 index 00000000000..ef29a45f3ea --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/KeyFieldsSelectInvalidTypeRule.cs @@ -0,0 +1,36 @@ +using HotChocolate.Fusion.Events; +using HotChocolate.Skimmed; +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 reference scalars or object types to ensure a valid and consistent +/// representation of the entity across subgraphs. Fields of types List, Interface, or +/// Union cannot be part of a @key because they do not have a well-defined unique +/// value. +/// +/// +/// Specification +/// +internal sealed class KeyFieldsSelectInvalidTypeRule : IEventHandler +{ + public void Handle(KeyFieldEvent @event, CompositionContext context) + { + var (entityType, keyDirective, field, type, schema) = @event; + + var fieldType = field.Type.NullableType(); + + if (fieldType is InterfaceTypeDefinition or ListTypeDefinition or UnionTypeDefinition) + { + context.Log.Write( + KeyFieldsSelectInvalidType( + 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 08894be8258..c41bbad9329 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 be a list, interface, or union type.. + /// + internal static string LogEntryHelper_KeyFieldsSelectInvalidType { + get { + return ResourceManager.GetString("LogEntryHelper_KeyFieldsSelectInvalidType", resourceCulture); + } + } + /// /// Looks up a localized string similar to Field '{0}' has a different type shape in schema '{1}' than it does in schema '{2}'.. /// 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 8fb5e617b33..a2ef422c968 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 be a list, interface, or union type. + Field '{0}' has a different type shape in schema '{1}' than it does in schema '{2}'. diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs index 01222282b29..1183281bdda 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 KeyFieldsSelectInvalidTypeRule(), new OutputFieldTypesMergeableRule(), new RootMutationUsedRule(), new RootQueryUsedRule(), diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs index 628d7f00a25..80bc9578513 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs @@ -4,5 +4,6 @@ internal static class WellKnownDirectiveNames { public const string External = "external"; public const string Inaccessible = "inaccessible"; + public const string Key = "key"; public const string Provides = "provides"; } diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyFieldsSelectInvalidTypeRuleTests.cs b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyFieldsSelectInvalidTypeRuleTests.cs new file mode 100644 index 00000000000..7e72915c04f --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/KeyFieldsSelectInvalidTypeRuleTests.cs @@ -0,0 +1,228 @@ +using HotChocolate.Fusion.Logging; +using HotChocolate.Fusion.PreMergeValidation; +using HotChocolate.Fusion.PreMergeValidation.Rules; + +namespace HotChocolate.Composition.PreMergeValidation.Rules; + +public sealed class KeyFieldsSelectInvalidTypeRuleTests : CompositionTestBase +{ + private readonly PreMergeValidator _preMergeValidator = + new([new KeyFieldsSelectInvalidTypeRule()]); + + [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_SELECT_INVALID_TYPE")); + Assert.True(context.Log.All(e => e.Severity == LogSeverity.Error)); + } + + public static TheoryData ValidExamplesData() + { + return new TheoryData + { + // In this example, the Product type has a valid @key directive referencing the scalar + // field `sku`. + { + [ + """ + type Product @key(fields: "sku") { + sku: String! + name: String + } + """ + ] + } + }; + } + + public static TheoryData InvalidExamplesData() + { + return new TheoryData + { + // In the following example, the Product type has an invalid @key directive referencing + // a field (featuredItem) whose type is an interface, violating the rule. + { + [ + """ + type Product @key(fields: "featuredItem { id }") { + featuredItem: Node! + sku: String! + } + + interface Node { + id: ID! + } + """ + ], + [ + "An @key directive on type 'Product' in schema 'A' references field " + + "'Product.featuredItem', which must not be a list, interface, or union type." + ] + }, + // In this example, the @key directive references a field (tags) of type List, which is + // also not allowed. + { + [ + """ + type Product @key(fields: "tags") { + tags: [String!]! + sku: String! + } + """ + ], + [ + "An @key directive on type 'Product' in schema 'A' references field " + + "'Product.tags', which must not be a list, interface, or union type." + ] + }, + // In this example, the @key directive references a field (relatedItems) of type Union, + // which violates the rule. + { + [ + """ + type Product @key(fields: "relatedItems") { + relatedItems: Related! + sku: String! + } + + union Related = Product | Service + + type Service { + id: ID! + } + """ + ], + [ + "An @key directive on type 'Product' in schema 'A' references field " + + "'Product.relatedItems', which must not be a list, interface, or union type." + ] + }, + // Nested interface. + { + [ + """ + type Product @key(fields: "info { featuredItem { id } }") { + info: ProductInfo! + } + + type ProductInfo { + featuredItem: Node! + } + + interface Node { + id: ID! + } + """ + ], + [ + "An @key directive on type 'Product' in schema 'A' references field " + + "'ProductInfo.featuredItem', which must not be a list, interface, or union " + + "type." + ] + }, + // Nested list. + { + [ + """ + type Product @key(fields: "info { tags }") { + info: ProductInfo! + } + + type ProductInfo { + tags: [String!]! + } + """ + ], + [ + "An @key directive on type 'Product' in schema 'A' references field " + + "'ProductInfo.tags', which must not be a list, interface, or union type." + ] + }, + // Nested union. + { + [ + """ + type Product @key(fields: "info { relatedItems }") { + info: ProductInfo! + } + + type ProductInfo { + relatedItems: Related! + } + + union Related = Product | Service + + type Service { + id: ID! + } + """ + ], + [ + "An @key directive on type 'Product' in schema 'A' references field " + + "'ProductInfo.relatedItems', which must not be a list, interface, or union " + + "type." + ] + }, + // Multiple keys. + { + [ + """ + type Product + @key(fields: "featuredItem { id }") + @key(fields: "tags") + @key(fields: "relatedItems") { + featuredItem: Node! + tags: [String!]! + relatedItems: Related! + } + + interface Node { + id: ID! + } + + union Related = Product | Service + + type Service { + id: ID! + } + """ + ], + [ + "An @key directive on type 'Product' in schema 'A' references field " + + "'Product.featuredItem', which must not be a list, interface, or union type.", + + "An @key directive on type 'Product' in schema 'A' references field " + + "'Product.tags', which must not be a list, interface, or union type.", + + "An @key directive on type 'Product' in schema 'A' references field " + + "'Product.relatedItems', which must not be a list, interface, or union type." + ] + } + }; + } +}