From f035d42b146333a672e6e650f9905bf37ea98b4a Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 24 Dec 2024 12:18:24 +0200 Subject: [PATCH] [Fusion] Added pre-merge validation rule "RootMutationUsedRule" (#7865) --- .../Logging/LogEntryCodes.cs | 1 + .../Logging/LogEntryHelper.cs | 10 ++ .../PreMergeValidation/Events.cs | 2 + .../PreMergeValidation/PreMergeValidator.cs | 2 + .../Rules/RootMutationUsedRule.cs | 31 ++++++ .../CompositionResources.Designer.cs | 9 ++ .../Properties/CompositionResources.resx | 3 + .../Fusion.Composition/SourceSchemaMerger.cs | 3 +- .../Fusion.Composition/WellKnownTypeNames.cs | 6 ++ .../Rules/RootMutationUsedRuleTests.cs | 97 +++++++++++++++++++ 10 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/RootMutationUsedRule.cs create mode 100644 src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownTypeNames.cs create mode 100644 src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/RootMutationUsedRuleTests.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 124895c8782..113858a0ff0 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs @@ -7,4 +7,5 @@ public static class LogEntryCodes public const string ExternalMissingOnBase = "EXTERNAL_MISSING_ON_BASE"; public const string ExternalUnused = "EXTERNAL_UNUSED"; 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 2a775985a2f..59dca6e0ba8 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs @@ -164,4 +164,14 @@ public static LogEntry OutputFieldTypesNotMergeable( field, schemaA); } + + public static LogEntry RootMutationUsed(SchemaDefinition schema) + { + return new LogEntry( + string.Format(LogEntryHelper_RootMutationUsed, schema.Name), + LogEntryCodes.RootMutationUsed, + severity: LogSeverity.Error, + member: schema, + schema: schema); + } } 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 cd5be8eb6a1..2ad878c72e1 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Events.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Events.cs @@ -35,6 +35,8 @@ internal record OutputFieldGroupEvent( ImmutableArray FieldGroup, string TypeName) : IEvent; +internal record SchemaEvent(SchemaDefinition Schema) : IEvent; + internal record TypeEvent( INamedTypeDefinition Type, SchemaDefinition Schema) : IEvent; 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 e630a9edc13..8af32e40155 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/PreMergeValidator.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/PreMergeValidator.cs @@ -26,6 +26,8 @@ private void PublishEvents(CompositionContext context) foreach (var schema in context.SchemaDefinitions) { + PublishEvent(new SchemaEvent(schema), context); + foreach (var type in schema.Types) { PublishEvent(new TypeEvent(type, schema), context); diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/RootMutationUsedRule.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/RootMutationUsedRule.cs new file mode 100644 index 00000000000..90fa25f6f15 --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/RootMutationUsedRule.cs @@ -0,0 +1,31 @@ +using HotChocolate.Fusion.Events; +using static HotChocolate.Fusion.Logging.LogEntryHelper; + +namespace HotChocolate.Fusion.PreMergeValidation.Rules; + +/// +/// This rule enforces that, for any source schema, if a root mutation type is defined, it must be +/// named Mutation. Defining a root mutation type with a name other than Mutation or +/// using a differently named type alongside a type explicitly named Mutation creates +/// inconsistencies in schema design and violates the composite schema specification. +/// +/// +/// Specification +/// +internal sealed class RootMutationUsedRule : IEventHandler +{ + public void Handle(SchemaEvent @event, CompositionContext context) + { + var schema = @event.Schema; + var rootMutation = schema.MutationType; + + if (rootMutation is not null && rootMutation.Name != WellKnownTypeNames.Mutation) + { + context.Log.Write(RootMutationUsed(schema)); + } + + // An object type named 'Mutation' will be set as the root mutation type if it has not yet + // been defined, so it's not necessary to check for this type in the absence of a root + // mutation type. + } +} 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 12a930d9e85..5faca5a8808 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 @@ -148,5 +148,14 @@ internal static string LogEntryHelper_OutputFieldTypesNotMergeable { return ResourceManager.GetString("LogEntryHelper_OutputFieldTypesNotMergeable", resourceCulture); } } + + /// + /// Looks up a localized string similar to The root mutation type in schema '{0}' must be named 'Mutation'.. + /// + internal static string LogEntryHelper_RootMutationUsed { + get { + return ResourceManager.GetString("LogEntryHelper_RootMutationUsed", resourceCulture); + } + } } } 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 f7b806e4753..fb139df4084 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx @@ -48,4 +48,7 @@ Field '{0}' has a different type shape in schema '{1}' than it does in schema '{2}'. + + The root mutation type in schema '{0}' must be named 'Mutation'. + diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs index c87eb7d4731..588def2ef76 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 OutputFieldTypesMergeableRule() + new OutputFieldTypesMergeableRule(), + new RootMutationUsedRule() ]; } diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownTypeNames.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownTypeNames.cs new file mode 100644 index 00000000000..41e6215047b --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownTypeNames.cs @@ -0,0 +1,6 @@ +namespace HotChocolate.Fusion; + +internal static class WellKnownTypeNames +{ + public const string Mutation = "Mutation"; +} diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/RootMutationUsedRuleTests.cs b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/RootMutationUsedRuleTests.cs new file mode 100644 index 00000000000..cd4ba5a35d9 --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/RootMutationUsedRuleTests.cs @@ -0,0 +1,97 @@ +using HotChocolate.Fusion.Logging; +using HotChocolate.Fusion.PreMergeValidation; +using HotChocolate.Fusion.PreMergeValidation.Rules; + +namespace HotChocolate.Composition.PreMergeValidation.Rules; + +public sealed class RootMutationUsedRuleTests : CompositionTestBase +{ + private readonly PreMergeValidator _preMergeValidator = new([new RootMutationUsedRule()]); + + [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 == "ROOT_MUTATION_USED")); + Assert.True(context.Log.All(e => e.Severity == LogSeverity.Error)); + } + + public static TheoryData ValidExamplesData() + { + return new TheoryData + { + // Valid example. + { + [ + """ + schema { + mutation: Mutation + } + + type Mutation { + createProduct(name: String): Product + } + + type Product { + id: ID! + name: String + } + """ + ] + } + }; + } + + public static TheoryData InvalidExamplesData() + { + return new TheoryData + { + // The following example violates the rule because `RootMutation` is used as the root + // mutation type, but a type named `Mutation` is also defined. + { + [ + """ + schema { + mutation: RootMutation + } + + type RootMutation { + createProduct(name: String): Product + } + + type Mutation { + deprecatedField: String + } + """ + ], + [ + "The root mutation type in schema 'A' must be named 'Mutation'." + ] + } + }; + } +}