From f9a7adb3b645dde55bd84c6e8cc525d21b76d0da Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 7 Jan 2025 14:26:23 +0200 Subject: [PATCH] [Fusion] Added pre-merge validation rule "OverrideFromSelfRule" --- .../Logging/LogEntryCodes.cs | 1 + .../Logging/LogEntryHelper.cs | 17 ++++ .../Rules/OverrideFromSelfRule.cs | 33 +++++++ .../CompositionResources.Designer.cs | 9 ++ .../Properties/CompositionResources.resx | 3 + .../Fusion.Composition/SourceSchemaMerger.cs | 1 + .../WellKnownArgumentNames.cs | 1 + .../WellKnownDirectiveNames.cs | 1 + .../Rules/OverrideFromSelfRuleTests.cs | 94 +++++++++++++++++++ 9 files changed, 160 insertions(+) create mode 100644 src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/OverrideFromSelfRule.cs create mode 100644 src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/OverrideFromSelfRuleTests.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 a37f8b4d3a9..905b652bb14 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs @@ -18,6 +18,7 @@ public static class LogEntryCodes public const string LookupMustNotReturnList = "LOOKUP_MUST_NOT_RETURN_LIST"; public const string LookupShouldHaveNullableReturnType = "LOOKUP_SHOULD_HAVE_NULLABLE_RETURN_TYPE"; public const string OutputFieldTypesNotMergeable = "OUTPUT_FIELD_TYPES_NOT_MERGEABLE"; + public const string OverrideFromSelf = "OVERRIDE_FROM_SELF"; public const string ProvidesDirectiveInFieldsArg = "PROVIDES_DIRECTIVE_IN_FIELDS_ARG"; public const string ProvidesFieldsHasArgs = "PROVIDES_FIELDS_HAS_ARGS"; public const string ProvidesFieldsMissingExternal = "PROVIDES_FIELDS_MISSING_EXTERNAL"; 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 269559d545d..2546e10ef47 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs @@ -381,6 +381,23 @@ public static LogEntry OutputFieldTypesNotMergeable( schemaA); } + public static LogEntry OverrideFromSelf( + Directive overrideDirective, + OutputFieldDefinition field, + INamedTypeDefinition type, + SchemaDefinition schema) + { + var coordinate = new SchemaCoordinate(type.Name, field.Name); + + return new LogEntry( + string.Format(LogEntryHelper_OverrideFromSelf, coordinate, schema.Name), + LogEntryCodes.OverrideFromSelf, + LogSeverity.Error, + coordinate, + overrideDirective, + schema); + } + public static LogEntry ProvidesDirectiveInFieldsArgument( ImmutableArray fieldNamePath, Directive providesDirective, diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/OverrideFromSelfRule.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/OverrideFromSelfRule.cs new file mode 100644 index 00000000000..61acbb466e0 --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/OverrideFromSelfRule.cs @@ -0,0 +1,33 @@ +using HotChocolate.Fusion.Events; +using HotChocolate.Language; +using static HotChocolate.Fusion.Logging.LogEntryHelper; +using static HotChocolate.Fusion.WellKnownArgumentNames; +using static HotChocolate.Fusion.WellKnownDirectiveNames; + +namespace HotChocolate.Fusion.PreMergeValidation.Rules; + +/// +/// When using @override, the from argument indicates the name of the source schema +/// that originally owns the field. Overriding from the same schema creates a contradiction, +/// as it implies both local and transferred ownership of the field within one schema. If the +/// from value matches the local schema name, it triggers an OVERRIDE_FROM_SELF error. +/// +/// +/// Specification +/// +internal sealed class OverrideFromSelfRule : IEventHandler +{ + public void Handle(OutputFieldEvent @event, CompositionContext context) + { + var (field, type, schema) = @event; + + var overrideDirective = field.Directives[Override].FirstOrDefault(); + + if ( + overrideDirective?.Arguments[From] is StringValueNode from + && from.Value == schema.Name) + { + context.Log.Write(OverrideFromSelf(overrideDirective, field, type, 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 1e7f11d4fa0..4821b3ad61a 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 @@ -248,6 +248,15 @@ internal static string LogEntryHelper_OutputFieldTypesNotMergeable { } } + /// + /// Looks up a localized string similar to The @override directive on field '{0}' in schema '{1}' must not reference the same schema.. + /// + internal static string LogEntryHelper_OverrideFromSelf { + get { + return ResourceManager.GetString("LogEntryHelper_OverrideFromSelf", resourceCulture); + } + } + /// /// Looks up a localized string similar to The @provides directive on field '{0}' in schema '{1}' references field '{2}', which must not include directive applications.. /// 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 fd7394c6db5..fa0f4b5ba8e 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx @@ -81,6 +81,9 @@ Field '{0}' has a different type shape in schema '{1}' than it does in schema '{2}'. + + The @override directive on field '{0}' in schema '{1}' must not reference the same schema. + The @provides directive on field '{0}' in schema '{1}' references field '{2}', which must not include directive applications. diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs index ba89727622d..afa679781e0 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs @@ -62,6 +62,7 @@ private CompositionResult MergeSchemaDefinitions(CompositionCo new LookupMustNotReturnListRule(), new LookupShouldHaveNullableReturnTypeRule(), new OutputFieldTypesMergeableRule(), + new OverrideFromSelfRule(), new ProvidesDirectiveInFieldsArgumentRule(), new ProvidesFieldsHasArgumentsRule(), new ProvidesFieldsMissingExternalRule(), diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownArgumentNames.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownArgumentNames.cs index 82f03e1d481..a470acd44a6 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownArgumentNames.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownArgumentNames.cs @@ -3,4 +3,5 @@ namespace HotChocolate.Fusion; internal static class WellKnownArgumentNames { public const string Fields = "fields"; + public const string From = "from"; } diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs index b627d9d5797..9ef107ac346 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs @@ -6,6 +6,7 @@ internal static class WellKnownDirectiveNames public const string Inaccessible = "inaccessible"; public const string Key = "key"; public const string Lookup = "lookup"; + public const string Override = "override"; public const string Provides = "provides"; public const string Require = "require"; } diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/OverrideFromSelfRuleTests.cs b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/OverrideFromSelfRuleTests.cs new file mode 100644 index 00000000000..6ad9b1435a6 --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/OverrideFromSelfRuleTests.cs @@ -0,0 +1,94 @@ +using HotChocolate.Fusion.Logging; +using HotChocolate.Fusion.PreMergeValidation; +using HotChocolate.Fusion.PreMergeValidation.Rules; + +namespace HotChocolate.Composition.PreMergeValidation.Rules; + +public sealed class OverrideFromSelfRuleTests : CompositionTestBase +{ + private readonly PreMergeValidator _preMergeValidator = new([new OverrideFromSelfRule()]); + + [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 == "OVERRIDE_FROM_SELF")); + Assert.True(context.Log.All(e => e.Severity == LogSeverity.Error)); + } + + public static TheoryData ValidExamplesData() + { + return new TheoryData + { + // In the following example, Schema B overrides the field "amount" from Schema A. The + // two schema names are different, so no error is raised. + { + [ + """ + # Source Schema A + type Bill { + id: ID! + amount: Int + } + """, + """ + # Source Schema B + type Bill { + id: ID! + amount: Int @override(from: "A") + } + """ + ] + } + }; + } + + public static TheoryData InvalidExamplesData() + { + return new TheoryData + { + // In the following example, the local schema is also "A", and the "from" argument is + // "A". Overriding a field from the same schema is not allowed, causing an + // OVERRIDE_FROM_SELF error. + { + [ + """ + # Source Schema A (named "A") + type Bill { + id: ID! + amount: Int @override(from: "A") + } + """ + ], + [ + "The @override directive on field 'Bill.amount' in schema 'A' must not " + + "reference the same schema." + ] + } + }; + } +}