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 298f766cf33..cf1339ee726 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs @@ -3,5 +3,6 @@ namespace HotChocolate.Fusion.Logging; public static class LogEntryCodes { public const string DisallowedInaccessible = "DISALLOWED_INACCESSIBLE"; + public const string ExternalArgumentDefaultMismatch = "EXTERNAL_ARGUMENT_DEFAULT_MISMATCH"; public const string OutputFieldTypesNotMergeable = "OUTPUT_FIELD_TYPES_NOT_MERGEABLE"; } 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 15bda6cc934..c59331ee55c 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs @@ -76,6 +76,16 @@ public static LogEntry DisallowedInaccessibleDirectiveArgument( new SchemaCoordinate(directiveName, argumentName: argument.Name, ofDirective: true), schema: schema); + public static LogEntry ExternalArgumentDefaultMismatch(string fieldName, string typeName) + => new( + string.Format( + LogEntryHelper_ExternalArgumentDefaultMismatch, + fieldName, + typeName), + LogEntryCodes.ExternalArgumentDefaultMismatch, + LogSeverity.Error, + new SchemaCoordinate(typeName, fieldName)); + public static LogEntry OutputFieldTypesNotMergeable(string fieldName, string typeName) => new( string.Format( diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ExternalArgumentsDefaultMismatchRule.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ExternalArgumentsDefaultMismatchRule.cs index 799641a5e6c..8753db86e41 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ExternalArgumentsDefaultMismatchRule.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ExternalArgumentsDefaultMismatchRule.cs @@ -1,85 +1,59 @@ using HotChocolate.Fusion.Events; +using HotChocolate.Language; using HotChocolate.Skimmed; using static HotChocolate.Fusion.Logging.LogEntryHelper; namespace HotChocolate.Fusion.PreMergeValidation.Rules; /// -/// This rule ensures that certain essential elements of a GraphQL schema, particularly built-in -/// scalars, directive arguments, and introspection types, cannot be marked as @inaccessible. These -/// types are fundamental to GraphQL. Making these elements inaccessible would break core GraphQL -/// functionality. +/// This rule ensures that arguments on fields marked as @external have default values compatible +/// with the corresponding arguments on fields from other source schemas where the field is defined (non-@external). /// -/// +/// /// Specification /// internal sealed class ExternalArgumentsDefaultMismatchRule - : IEventHandler - , IEventHandler - , IEventHandler - , IEventHandler + : IEventHandler { - public void Handle(TypeEvent @event, CompositionContext context) + public void Handle(OutputFieldGroupEvent @event, CompositionContext context) { - var (type, schema) = @event; + var (fieldName, fieldGroup, typeName) = @event; - // Built-in scalar types must be accessible. - if (type is ScalarTypeDefinition { IsSpecScalar: true } scalar - && !ValidationHelper.IsAccessible(scalar)) + if (fieldGroup.FirstOrDefault(i => ValidationHelper.IsExternal(i.Field)) is { } externalField) { - context.Log.Write(DisallowedInaccessibleBuiltInScalar(scalar, schema)); - } - - // Introspection types must be accessible. - if (type.IsIntrospectionType && !ValidationHelper.IsAccessible(type)) - { - context.Log.Write(DisallowedInaccessibleIntrospectionType(type, schema)); - } - } - - public void Handle(OutputFieldEvent @event, CompositionContext context) - { - var (field, type, schema) = @event; - - // Introspection fields must be accessible. - if (type.IsIntrospectionType && !ValidationHelper.IsAccessible(field)) - { - context.Log.Write( - DisallowedInaccessibleIntrospectionField( - field, - type.Name, - schema)); - } - } - - public void Handle(FieldArgumentEvent @event, CompositionContext context) - { - var (argument, field, type, schema) = @event; - - // Introspection arguments must be accessible. - if (type.IsIntrospectionType && !ValidationHelper.IsAccessible(argument)) - { - context.Log.Write( - DisallowedInaccessibleIntrospectionArgument( - argument, - field.Name, - type.Name, - schema)); - } - } - - public void Handle(DirectiveArgumentEvent @event, CompositionContext context) - { - var (argument, directive, schema) = @event; - - // Built-in directive arguments must be accessible. - if (BuiltIns.IsBuiltInDirective(directive.Name) && !ValidationHelper.IsAccessible(argument)) - { - context.Log.Write( - DisallowedInaccessibleDirectiveArgument( - argument, - directive.Name, - schema)); + var argumentNames = fieldGroup.SelectMany(fg => fg.Field.Arguments, (_, arg) => arg.Name).ToHashSet(); + foreach (var argumentName in argumentNames) + { + if (!externalField.Field.Arguments.TryGetField(argumentName, out var argumentField)) + { + // Logged in separate rule. + continue; + } + + var defaultValue = argumentField.DefaultValue; + foreach (var currentField in fieldGroup.Except([externalField])) + { + if (!currentField.Field.Arguments.TryGetField(argumentName, out argumentField)) + { + // Logged in separate rule. + continue; + } + + var currentValue = argumentField.DefaultValue; + var match = (currentValue, defaultValue) switch + { + (null, null) => true, + (not null, null) => false, + (null, not null) => false, + _ => currentValue.Value!.Equals(defaultValue.Value) + }; + + if (!match) + { + context.Log.Write(ExternalArgumentDefaultMismatch(fieldName, typeName)); + } + } + } } } } 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 8a2c9d46b34..2900d451d0c 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,67 +45,52 @@ 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}' is not accessible.. - /// internal static string LogEntryHelper_DisallowedInaccessibleBuiltInScalar { get { return ResourceManager.GetString("LogEntryHelper_DisallowedInaccessibleBuiltInScalar", resourceCulture); } } - /// - /// Looks up a localized string similar to The argument '{0}' on built-in directive type '{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}' with schema coordinate '{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}' on type '{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}' 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 Field '{0}' on type '{1}' is not mergeable.. - /// internal static string LogEntryHelper_OutputFieldTypesNotMergeable { get { return ResourceManager.GetString("LogEntryHelper_OutputFieldTypesNotMergeable", resourceCulture); } } + + internal static string LogEntryHelper_ExternalArgumentDefaultMismatch { + get { + return ResourceManager.GetString("LogEntryHelper_ExternalArgumentDefaultMismatch", 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 78fde65b49b..51125140373 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx @@ -39,4 +39,7 @@ Field '{0}' on type '{1}' is not mergeable. + + Field '{0}' on type '{1}' must have consistent default values. + diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs index 2ebe586a566..d27781211ab 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs @@ -4,5 +4,4 @@ internal static class WellKnownDirectiveNames { public const string External = "external"; public const string Inaccessible = "inaccessible"; - public const string External = "external"; } diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ExternalArgumentsDefaultMismatchRuleTests.cs b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ExternalArgumentsDefaultMismatchRuleTests.cs index 9c2baabd41e..270a8e13c37 100644 --- a/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ExternalArgumentsDefaultMismatchRuleTests.cs +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ExternalArgumentsDefaultMismatchRuleTests.cs @@ -62,6 +62,19 @@ type Product { } """ ], + () => + [ + """ + type Product { + name(language: String = "en", localization: String = "sr"): String + } + """, + """ + type Product { + name(language: String = "en", localization: String = "sr"): String @external + } + """ + ], ]; } @@ -95,7 +108,33 @@ type Product { name(language: String): String @external } """ - ] + ], + () => + [ + """ + type Product { + name(language: String = "en", localization: String = "sr"): String + } + """, + """ + type Product { + name(language: String = "en", localization: String = "sa"): String @external + } + """ + ], + () => + [ + """ + type Product { + name(language: String = "en", localization: String = "sr"): String + } + """, + """ + type Product { + name(language: String = "en", localization: String): String @external + } + """ + ], ]; } }