From c23d62bf3ae3cb61bd62aa3642137dc76d8c839b Mon Sep 17 00:00:00 2001 From: Daniel Reynolds <55194784+danielreynolds1@users.noreply.github.com> Date: Wed, 1 Jan 2025 15:45:50 +0000 Subject: [PATCH] [Fusion] Added pre-merge validation rule "ProvidesOnNonCompositeFieldRule" (#7883) --- .../Logging/LogEntryCodes.cs | 1 + .../Logging/LogEntryHelper.cs | 19 +++ .../Rules/ProvidesOnNonCompositeFieldRule.cs | 34 +++++ .../CompositionResources.Designer.cs | 129 ++++-------------- .../Properties/CompositionResources.resx | 3 + .../Fusion.Composition/SourceSchemaMerger.cs | 1 + .../Fusion.Composition/ValidationHelper.cs | 5 + .../ProvidesOnNonCompositeFieldRuleTests.cs | 126 +++++++++++++++++ 8 files changed, 215 insertions(+), 103 deletions(-) create mode 100644 src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ProvidesOnNonCompositeFieldRule.cs create mode 100644 src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ProvidesOnNonCompositeFieldRuleTests.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 7743e2093e9..da278d8ff0e 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs @@ -16,6 +16,7 @@ public static class LogEntryCodes 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"; + public const string ProvidesOnNonCompositeField = "PROVIDES_ON_NON_COMPOSITE_FIELD"; public const string QueryRootTypeInaccessible = "QUERY_ROOT_TYPE_INACCESSIBLE"; 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 8e1ea275eef..fdf447b0e7c 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs @@ -346,6 +346,25 @@ public static LogEntry ProvidesFieldsMissingExternal( schema); } + public static LogEntry ProvidesOnNonCompositeField( + OutputFieldDefinition field, + INamedTypeDefinition type, + SchemaDefinition schema) + { + var coordinate = new SchemaCoordinate(type.Name, field.Name); + + return new LogEntry( + string.Format( + LogEntryHelper_ProvidesOnNonCompositeField, + coordinate, + schema.Name), + LogEntryCodes.ProvidesOnNonCompositeField, + LogSeverity.Error, + coordinate, + field, + schema); + } + public static LogEntry QueryRootTypeInaccessible( INamedTypeDefinition type, SchemaDefinition schema) diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ProvidesOnNonCompositeFieldRule.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ProvidesOnNonCompositeFieldRule.cs new file mode 100644 index 00000000000..79531b789d2 --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ProvidesOnNonCompositeFieldRule.cs @@ -0,0 +1,34 @@ +using HotChocolate.Fusion.Events; +using HotChocolate.Fusion.Extensions; +using HotChocolate.Skimmed; +using static HotChocolate.Fusion.Logging.LogEntryHelper; + +namespace HotChocolate.Fusion.PreMergeValidation.Rules; + +/// +/// The @provides directive allows a field to “provide” additional nested fields on the +/// composite type it returns. If a field’s base type is not an object or interface type (e.g., +/// String, Int, Boolean, Enum, Union, or an Input type), it cannot hold nested fields for +/// @provides to select. Consequently, attaching @provides to such a field is +/// invalid and raises a PROVIDES_ON_NON_OBJECT_FIELD error. +/// +/// +/// Specification +/// +internal sealed class ProvidesOnNonCompositeFieldRule : IEventHandler +{ + public void Handle(OutputFieldEvent @event, CompositionContext context) + { + var (field, type, schema) = @event; + + if (ValidationHelper.HasProvidesDirective(field)) + { + var fieldType = field.Type.NamedType(); + + if (fieldType is not ComplexTypeDefinition) + { + context.Log.Write(ProvidesOnNonCompositeField(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 921b615f5e9..821344e69a3 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 @@ -11,46 +11,32 @@ namespace HotChocolate.Fusion.Properties { using System; - /// - /// A strongly-typed resource class, for looking up localized strings, etc. - /// - // This class was auto-generated by the StronglyTypedResourceBuilder - // class via a tool like ResGen or Visual Studio. - // To add or remove a member, edit your .ResX file then rerun ResGen - // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "4.0.0.0")] - [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] - [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + [System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "4.0.0.0")] + [System.Diagnostics.DebuggerNonUserCodeAttribute()] + [System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class CompositionResources { - private static global::System.Resources.ResourceManager resourceMan; + private static System.Resources.ResourceManager resourceMan; - private static global::System.Globalization.CultureInfo resourceCulture; + private static System.Globalization.CultureInfo resourceCulture; - [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] internal CompositionResources() { } - /// - /// Returns the cached ResourceManager instance used by this class. - /// - [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - internal static global::System.Resources.ResourceManager ResourceManager { + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] + internal static System.Resources.ResourceManager ResourceManager { get { - if (object.ReferenceEquals(resourceMan, null)) { - global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("HotChocolate.Fusion.Properties.CompositionResources", typeof(CompositionResources).Assembly); + if (object.Equals(null, resourceMan)) { + System.Resources.ResourceManager temp = new System.Resources.ResourceManager("HotChocolate.Fusion.Properties.CompositionResources", typeof(CompositionResources).Assembly); resourceMan = temp; } return resourceMan; } } - /// - /// Overrides the current thread's CurrentUICulture property for all - /// resource lookups using this strongly typed resource class. - /// - [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - internal static global::System.Globalization.CultureInfo Culture { + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] + internal static System.Globalization.CultureInfo Culture { get { return resourceCulture; } @@ -59,207 +45,144 @@ internal CompositionResources() { } } - /// - /// Looks up a localized string similar to Pre-merge validation failed. View the composition log for details.. - /// internal static string ErrorHelper_PreMergeValidationFailed { get { return ResourceManager.GetString("ErrorHelper_PreMergeValidationFailed", resourceCulture); } } - /// - /// Looks up a localized string similar to The built-in scalar type '{0}' in schema '{1}' is not accessible.. - /// internal static string LogEntryHelper_DisallowedInaccessibleBuiltInScalar { get { return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleBuiltInScalar", resourceCulture); } } - /// - /// Looks up a localized string similar to The built-in directive argument '{0}' in schema '{1}' is not accessible.. - /// - internal static string LogEntryHelper_DisallowedInaccessibleDirectiveArgument { + internal static string LogEntryHelper_DisallowedInaccessibleIntrospectionType { get { - return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleDirectiveArgument", resourceCulture); + return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleIntrospectionType", resourceCulture); } } - /// - /// Looks up a localized string similar to The introspection argument '{0}' in schema '{1}' is not accessible.. - /// - internal static string LogEntryHelper_DisallowedInaccessibleIntrospectionArgument { + internal static string LogEntryHelper_DisallowedInaccessibleIntrospectionField { get { - return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleIntrospectionArgument", resourceCulture); + return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleIntrospectionField", resourceCulture); } } - /// - /// Looks up a localized string similar to The introspection field '{0}' in schema '{1}' is not accessible.. - /// - internal static string LogEntryHelper_DisallowedInaccessibleIntrospectionField { + internal static string LogEntryHelper_DisallowedInaccessibleIntrospectionArgument { get { - return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleIntrospectionField", resourceCulture); + return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleIntrospectionArgument", resourceCulture); } } - /// - /// Looks up a localized string similar to The introspection type '{0}' in schema '{1}' is not accessible.. - /// - internal static string LogEntryHelper_DisallowedInaccessibleIntrospectionType { + internal static string LogEntryHelper_DisallowedInaccessibleDirectiveArgument { get { - return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleIntrospectionType", resourceCulture); + return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleDirectiveArgument", resourceCulture); } } - /// - /// Looks up a localized string similar to The argument with schema coordinate '{0}' has inconsistent default values.. - /// internal static string LogEntryHelper_ExternalArgumentDefaultMismatch { get { return ResourceManager.GetString("LogEntryHelper_ExternalArgumentDefaultMismatch", resourceCulture); } } - /// - /// Looks up a localized string similar to External field '{0}' in schema '{1}' is not defined (non-external) in any other schema.. - /// internal static string LogEntryHelper_ExternalMissingOnBase { get { return ResourceManager.GetString("LogEntryHelper_ExternalMissingOnBase", resourceCulture); } } - /// - /// Looks up a localized string similar to Interface field '{0}' in schema '{1}' must not be marked as external.. - /// internal static string LogEntryHelper_ExternalOnInterface { get { return ResourceManager.GetString("LogEntryHelper_ExternalOnInterface", resourceCulture); } } - /// - /// Looks up a localized string similar to External field '{0}' in schema '{1}' is not referenced by a @provides directive in the schema.. - /// internal static string LogEntryHelper_ExternalUnused { get { return ResourceManager.GetString("LogEntryHelper_ExternalUnused", resourceCulture); } } - /// - /// Looks up a localized string similar to A @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 A @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 A @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 A @key directive on type '{0}' in schema '{1}' references field '{2}', which does not exist.. - /// internal static string LogEntryHelper_KeyInvalidFields { get { return ResourceManager.GetString("LogEntryHelper_KeyInvalidFields", resourceCulture); } } - /// - /// Looks up a localized string similar to A @key directive on type '{0}' in schema '{1}' contains invalid syntax in the 'fields' argument.. - /// internal static string LogEntryHelper_KeyInvalidSyntax { get { return ResourceManager.GetString("LogEntryHelper_KeyInvalidSyntax", resourceCulture); } } - /// - /// Looks up a localized string similar to Field '{0}' has a different type shape in schema '{1}' than it does in schema '{2}'.. - /// internal static string LogEntryHelper_OutputFieldTypesNotMergeable { get { return ResourceManager.GetString("LogEntryHelper_OutputFieldTypesNotMergeable", 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.. - /// internal static string LogEntryHelper_ProvidesDirectiveInFieldsArgument { get { return ResourceManager.GetString("LogEntryHelper_ProvidesDirectiveInFieldsArgument", resourceCulture); } } - /// - /// Looks up a localized string similar to The @provides directive on field '{0}' in schema '{1}' references field '{2}', which must not have arguments.. - /// internal static string LogEntryHelper_ProvidesFieldsHasArguments { get { return ResourceManager.GetString("LogEntryHelper_ProvidesFieldsHasArguments", resourceCulture); } } - /// - /// Looks up a localized string similar to The @provides directive on field '{0}' in schema '{1}' references field '{2}', which must be marked as external.. - /// internal static string LogEntryHelper_ProvidesFieldsMissingExternal { get { return ResourceManager.GetString("LogEntryHelper_ProvidesFieldsMissingExternal", resourceCulture); } } - /// - /// Looks up a localized string similar to The root query type in schema '{0}' must be accessible.. - /// + internal static string LogEntryHelper_ProvidesOnNonCompositeField { + get { + return ResourceManager.GetString("LogEntryHelper_ProvidesOnNonCompositeField", resourceCulture); + } + } + internal static string LogEntryHelper_QueryRootTypeInaccessible { get { return ResourceManager.GetString("LogEntryHelper_QueryRootTypeInaccessible", 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); } } - /// - /// Looks up a localized string similar to The root query type in schema '{0}' must be named 'Query'.. - /// internal static string LogEntryHelper_RootQueryUsed { get { return ResourceManager.GetString("LogEntryHelper_RootQueryUsed", resourceCulture); } } - /// - /// Looks up a localized string similar to The root subscription type in schema '{0}' must be named 'Subscription'.. - /// internal static string LogEntryHelper_RootSubscriptionUsed { get { return ResourceManager.GetString("LogEntryHelper_RootSubscriptionUsed", 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 e57690bab1a..9033132bca4 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx @@ -75,6 +75,9 @@ The @provides directive on field '{0}' in schema '{1}' references field '{2}', which must be marked as external. + + The field '{0}' in schema '{1}' includes a @provides directive, but does not return a composite type. + The root query type in schema '{0}' must be accessible. diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs index 0d1fd80c066..c17c949c8c7 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs @@ -60,6 +60,7 @@ private CompositionResult MergeSchemaDefinitions(CompositionCo new ProvidesDirectiveInFieldsArgumentRule(), new ProvidesFieldsHasArgumentsRule(), new ProvidesFieldsMissingExternalRule(), + new ProvidesOnNonCompositeFieldRule(), new QueryRootTypeInaccessibleRule(), new RootMutationUsedRule(), new RootQueryUsedRule(), diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/ValidationHelper.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/ValidationHelper.cs index 50e6d7448de..13c7ea5a9a7 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/ValidationHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/ValidationHelper.cs @@ -5,6 +5,11 @@ namespace HotChocolate.Fusion; internal sealed class ValidationHelper { + public static bool HasProvidesDirective(IDirectivesProvider type) + { + return type.Directives.ContainsName(WellKnownDirectiveNames.Provides); + } + public static bool IsAccessible(IDirectivesProvider type) { return !type.Directives.ContainsName(WellKnownDirectiveNames.Inaccessible); diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ProvidesOnNonCompositeFieldRuleTests.cs b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ProvidesOnNonCompositeFieldRuleTests.cs new file mode 100644 index 00000000000..9ac429972be --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ProvidesOnNonCompositeFieldRuleTests.cs @@ -0,0 +1,126 @@ +using HotChocolate.Fusion.Logging; +using HotChocolate.Fusion.PreMergeValidation; +using HotChocolate.Fusion.PreMergeValidation.Rules; + +namespace HotChocolate.Composition.PreMergeValidation.Rules; + +public sealed class ProvidesOnNonCompositeFieldRuleTests : CompositionTestBase +{ + private readonly PreMergeValidator _preMergeValidator = + new([new ProvidesOnNonCompositeFieldRule()]); + + [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 == "PROVIDES_ON_NON_COMPOSITE_FIELD")); + Assert.True(context.Log.All(e => e.Severity == LogSeverity.Error)); + } + + public static TheoryData ValidExamplesData() + { + return new TheoryData + { + // Here, "profile" has an object base type "Profile". The @provides directive can + // validly specify sub-fields like "settings { theme }". + { + [ + """ + type Profile { + email: String + settings: Settings + } + + type Settings { + notificationsEnabled: Boolean + theme: String + } + + type User { + id: ID! + profile: Profile @provides(fields: "settings { theme }") + } + """ + ] + } + }; + } + + public static TheoryData InvalidExamplesData() + { + return new TheoryData + { + // In this example, "email" has a scalar base type (String). Because scalars do not + // expose sub-fields, attaching @provides to "email" triggers a + // PROVIDES_ON_NON_OBJECT_FIELD error. + { + [ + """ + type User { + id: ID! + email: String @provides(fields: "length") + } + """ + ], + [ + "The field 'User.email' in schema 'A' includes a @provides directive, but " + + "does not return a composite type." + ] + }, + // Here, the schema is defined with "email" being a non-null string. + { + [ + """ + type User { + id: ID! + email: String! @provides(fields: "length") + } + """ + ], + [ + "The field 'User.email' in schema 'A' includes a @provides directive, but " + + "does not return a composite type." + ] + }, + // Here, the schema is defined with "emails" being a non-null list of non-null strings. + { + [ + """ + type User { + id: ID! + emails: [String!]! @provides(fields: "length") + } + """ + ], + [ + "The field 'User.emails' in schema 'A' includes a @provides directive, but " + + "does not return a composite type." + ] + } + }; + } +}