diff --git a/dictionary.txt b/dictionary.txt index 2875c750db8..655e32b0219 100644 --- a/dictionary.txt +++ b/dictionary.txt @@ -38,13 +38,13 @@ CCPA chillicream Codespaces colocated -csclient combinators contentfiles Contoso conv CQRS creds +csclient CUST dataloaders debuggable @@ -224,6 +224,7 @@ vsix VXNlcjox websockets Wilhuff +worklist Wunder xact xunit diff --git a/src/HotChocolate/Fusion/src/Fusion.Composition/Extensions/PathNodeExtensions.cs b/src/HotChocolate/Fusion/src/Fusion.Composition/Extensions/PathNodeExtensions.cs new file mode 100644 index 00000000000..f473a1543c9 --- /dev/null +++ b/src/HotChocolate/Fusion/src/Fusion.Composition/Extensions/PathNodeExtensions.cs @@ -0,0 +1,42 @@ +using HotChocolate.Fusion.Collections; +using HotChocolate.Fusion.Satisfiability; + +namespace HotChocolate.Fusion.Extensions; + +internal static class PathNodeExtensions +{ + extension(PathNode? path) + { + public bool ContainsItem(SatisfiabilityPathItem item) + { + for (var node = path; node is not null; node = node.Parent) + { + if (node.Item == item) + { + return true; + } + } + + return false; + } + + public string ToPathString() + { + if (path is null) + { + return string.Empty; + } + + var items = new List(); + + for (var node = path; node is not null; node = node.Parent) + { + items.Add(node.Item); + } + + items.Reverse(); + + return string.Join(" -> ", items); + } + } +} diff --git a/src/HotChocolate/Fusion/src/Fusion.Composition/Properties/CompositionResources.Designer.cs b/src/HotChocolate/Fusion/src/Fusion.Composition/Properties/CompositionResources.Designer.cs index 52a29be6614..d34e4766422 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Composition/Properties/CompositionResources.Designer.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Composition/Properties/CompositionResources.Designer.cs @@ -1501,15 +1501,6 @@ internal static string SatisfiabilityValidator_CycleDetected { } } - /// - /// Looks up a localized string similar to Satisfiability validation reached the maximum recursion depth ({0}) while visiting type '{1}'. Validation of deeply nested fields may be incomplete.. - /// - internal static string SatisfiabilityValidator_MaxRecursionDepthReached { - get { - return ResourceManager.GetString("SatisfiabilityValidator_MaxRecursionDepthReached", resourceCulture); - } - } - /// /// Looks up a localized string similar to Type '{0}' implements the 'Node' interface, but no source schema provides a non-internal 'Query.node<Node>' lookup field for this type.. /// diff --git a/src/HotChocolate/Fusion/src/Fusion.Composition/Properties/CompositionResources.resx b/src/HotChocolate/Fusion/src/Fusion.Composition/Properties/CompositionResources.resx index 8748bfaafc9..206ffcea0d6 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Composition/Properties/CompositionResources.resx +++ b/src/HotChocolate/Fusion/src/Fusion.Composition/Properties/CompositionResources.resx @@ -495,9 +495,6 @@ Cycle detected: {0} -> {1}. - - Satisfiability validation reached the maximum recursion depth ({0}) while visiting type '{1}'. Validation of deeply nested fields may be incomplete. - Type '{0}' implements the 'Node' interface, but no source schema provides a non-internal 'Query.node<Node>' lookup field for this type. diff --git a/src/HotChocolate/Fusion/src/Fusion.Composition/Satisfiability/PathNode.cs b/src/HotChocolate/Fusion/src/Fusion.Composition/Satisfiability/PathNode.cs new file mode 100644 index 00000000000..b6c52c1a088 --- /dev/null +++ b/src/HotChocolate/Fusion/src/Fusion.Composition/Satisfiability/PathNode.cs @@ -0,0 +1,10 @@ +using HotChocolate.Fusion.Collections; + +namespace HotChocolate.Fusion.Satisfiability; + +internal sealed class PathNode(SatisfiabilityPathItem item, PathNode? parent) +{ + public SatisfiabilityPathItem Item { get; } = item; + + public PathNode? Parent { get; } = parent; +} diff --git a/src/HotChocolate/Fusion/src/Fusion.Composition/Satisfiability/WorkItem.cs b/src/HotChocolate/Fusion/src/Fusion.Composition/Satisfiability/WorkItem.cs new file mode 100644 index 00000000000..686d2bed2be --- /dev/null +++ b/src/HotChocolate/Fusion/src/Fusion.Composition/Satisfiability/WorkItem.cs @@ -0,0 +1,7 @@ +using HotChocolate.Types.Mutable; + +namespace HotChocolate.Fusion.Satisfiability; + +internal readonly record struct WorkItem( + MutableObjectTypeDefinition ObjectType, + PathNode? Path); diff --git a/src/HotChocolate/Fusion/src/Fusion.Composition/SatisfiabilityValidator.cs b/src/HotChocolate/Fusion/src/Fusion.Composition/SatisfiabilityValidator.cs index 1327b454440..ef914b839ff 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Composition/SatisfiabilityValidator.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Composition/SatisfiabilityValidator.cs @@ -18,8 +18,6 @@ namespace HotChocolate.Fusion; internal sealed class SatisfiabilityValidator { - private const int MaxRecursionDepth = 500; - private readonly SatisfiabilityOptions _options; private readonly RequirementsValidator _requirementsValidator; private readonly MutableSchemaDefinition _schema; @@ -38,7 +36,8 @@ public SatisfiabilityValidator( public CompositionResult Validate() { - var context = new SatisfiabilityValidatorContext(); + var visited = new HashSet<(MutableObjectTypeDefinition, string?)>(); + var worklist = new Queue(); MutableObjectTypeDefinition?[] rootTypes = [_schema.QueryType, _schema.MutationType, _schema.SubscriptionType]; @@ -47,10 +46,23 @@ public CompositionResult Validate() { if (rootType is not null) { - VisitObjectType(rootType, context); + worklist.Enqueue(new WorkItem(rootType, null)); } } + while (worklist.Count > 0) + { + var work = worklist.Dequeue(); + var prevSchema = work.Path?.Item.SchemaName; + + if (!visited.Add((work.ObjectType, prevSchema))) + { + continue; + } + + VisitObjectType(work.ObjectType, work.Path, worklist, visited); + } + return _log.HasErrors ? ErrorHelper.SatisfiabilityValidationFailed() : CompositionResult.Success(); @@ -58,31 +70,10 @@ public CompositionResult Validate() private void VisitObjectType( MutableObjectTypeDefinition objectType, - SatisfiabilityValidatorContext context) + PathNode? path, + Queue worklist, + HashSet<(MutableObjectTypeDefinition, string?)> visited) { - if (context.Depth >= MaxRecursionDepth) - { - if (!context.DepthLimitReached) - { - context.DepthLimitReached = true; - - _log.Write( - LogEntryBuilder.New() - .SetMessage( - SatisfiabilityValidator_MaxRecursionDepthReached, - MaxRecursionDepth, - objectType.Name) - .SetCode(LogEntryCodes.Unsatisfiable) - .SetSeverity(LogSeverity.Warning) - .Build()); - } - - return; - } - - context.Depth++; - context.TypeContext.Push(objectType); - foreach (var field in objectType.Fields) { if (field.HasFusionInaccessibleDirective()) @@ -104,32 +95,23 @@ private void VisitObjectType( continue; } - VisitNodeField(objectType, field, nodeType, context); + VisitNodeField(objectType, field, nodeType, path, worklist); continue; } - VisitOutputField(field, context); + VisitOutputField(field, objectType, path, worklist, visited); } - - context.TypeContext.Pop(); - context.Depth--; } private void VisitOutputField( MutableOutputFieldDefinition field, - SatisfiabilityValidatorContext context) + MutableObjectTypeDefinition type, + PathNode? path, + Queue worklist, + HashSet<(MutableObjectTypeDefinition, string?)> visited) { - var type = context.TypeContext.Peek(); - var previousPathItem = context.Path.TryPeek(out var item) ? item : null; - var previousSchemaName = previousPathItem?.SchemaName; - var cacheKey = new FieldAccessCacheKey(field, type, previousSchemaName); - - if (context.FieldAccessCache.Contains(cacheKey)) - { - return; - } - + var previousSchemaName = path?.Item.SchemaName; var schemaNames = field.GetSchemaNames(first: previousSchemaName); var cycle = false; var errors = new List(); @@ -141,15 +123,16 @@ private void VisitOutputField( // If the field is marked as partial, it must be provided by the current schema for it // to be an option. if (field.IsPartial(schemaName) - && previousPathItem?.Provides(field, type, schemaName, _schema) != true) + && path?.Item.Provides(field, type, schemaName, _schema) != true) { continue; } var pathItem = new SatisfiabilityPathItem(field, type, schemaName); - // Validate that we are not in a cycle. - if (!context.CycleDetectionPath.Push(pathItem)) + // Validate that we are not in a cycle by checking if this path item + // already appears in the ancestor chain. + if (path.ContainsItem(pathItem)) { cycle = true; continue; @@ -158,7 +141,7 @@ private void VisitOutputField( // Validate transition between source schemas. if (previousSchemaName is not null && previousSchemaName != schemaName) { - var transitionErrors = ValidateSourceSchemaTransition(type, context, schemaName); + var transitionErrors = ValidateSourceSchemaTransition(type, path, schemaName); if (transitionErrors.Any()) { @@ -171,8 +154,6 @@ private void VisitOutputField( pathItem), transitionErrors)); - context.CycleDetectionPath.Pop(); - continue; } } @@ -186,7 +167,7 @@ private void VisitOutputField( _requirementsValidator.Validate( requirements, type, - previousPathItem, + path?.Item, excludeSchemaName: schemaName); if (requirementErrors.Length != 0) @@ -199,30 +180,27 @@ private void VisitOutputField( pathItem), requirementErrors)); - context.CycleDetectionPath.Pop(); - continue; } } optionCount++; - context.Path.Push(pathItem); - - // Visit each of the possible types that the field may return. + // Enqueue child types for later processing. if (fieldType is MutableComplexTypeDefinition or MutableUnionTypeDefinition) { + var fieldPath = new PathNode(pathItem, path); var possibleTypes = fieldType.GetPossibleTypes(schemaName, _schema); foreach (var possibleType in possibleTypes) { - VisitObjectType(possibleType, context); + if (!visited.Contains((possibleType, schemaName))) + { + worklist.Enqueue(new WorkItem(possibleType, fieldPath)); + } } } - context.Path.Pop(); - context.CycleDetectionPath.Pop(); - // When we reach a leaf field, we can break early as we only need a single option. if (fieldType.IsLeafType()) { @@ -230,8 +208,6 @@ private void VisitOutputField( } } - context.FieldAccessCache.Add(cacheKey); - // Log an error if there are no options for accessing the field, and there is no cycle // (which would imply an option for accessing the same field repeatedly). // (f.e. relatedProduct.relatedProduct.relatedProduct) @@ -240,7 +216,7 @@ private void VisitOutputField( var qualifiedFieldName = $"{type.Name}.{field.Name}"; if (_options.IgnoredNonAccessibleFields.TryGetValue(qualifiedFieldName, out var ignoredPaths) - && ignoredPaths.Contains(context.Path.ToString())) + && ignoredPaths.Contains(path.ToPathString())) { return; } @@ -251,7 +227,7 @@ private void VisitOutputField( SatisfiabilityValidator_UnableToAccessFieldOnPath, type.Name, field.Name, - context.Path) + path.ToPathString()) : string.Format( SatisfiabilityValidator_UnableToAccessField, type.Name, @@ -273,7 +249,8 @@ private void VisitNodeField( MutableObjectTypeDefinition queryType, MutableOutputFieldDefinition nodeField, IInterfaceTypeDefinition nodeType, - SatisfiabilityValidatorContext context) + PathNode? path, + Queue worklist) { foreach (var possibleType in _schema.GetPossibleTypes(nodeType)) { @@ -283,7 +260,7 @@ private void VisitNodeField( var hasNodeLookup = false; var nodePathItem = new SatisfiabilityPathItem(nodeField, queryType, "*"); - context.Path.Push(nodePathItem); + var nodePath = new PathNode(nodePathItem, path); foreach (var lookup in byIdLookups) { @@ -307,11 +284,9 @@ private void VisitNodeField( var lookupField = new MutableOutputFieldDefinition(lookupFieldDefinition.Name.Value, lookupFieldType); var lookupPathItem = new SatisfiabilityPathItem(lookupField, queryType, schemaName); - context.Path.Push(lookupPathItem); + var lookupPath = new PathNode(lookupPathItem, nodePath); - VisitObjectType(possibleType, context); - - context.Path.Pop(); + worklist.Enqueue(new WorkItem(possibleType, lookupPath)); } if (!hasNodeLookup) @@ -327,14 +302,12 @@ private void VisitNodeField( .SetExtension("error", error) .Build()); } - - context.Path.Pop(); } } private ImmutableArray ValidateSourceSchemaTransition( MutableObjectTypeDefinition type, - SatisfiabilityValidatorContext context, + PathNode? path, string transitionToSchemaName) { var errors = new List(); @@ -342,7 +315,7 @@ private ImmutableArray ValidateSourceSchemaTransition( var lookupDirectives = _schema.GetPossibleFusionLookupDirectives(type, transitionToSchemaName); - if (!lookupDirectives.Any() && !CanTransitionToSchemaThroughPath(context.Path, transitionToSchemaName)) + if (lookupDirectives.Count == 0 && !CanTransitionToSchemaThroughPath(path, transitionToSchemaName)) { errors.Add( new SatisfiabilityError( @@ -368,7 +341,7 @@ private ImmutableArray ValidateSourceSchemaTransition( _requirementsValidator.Validate( lookupRequirements, type, - context.Path.Peek(), + path?.Item, excludeSchemaName: transitionToSchemaName); if (requirementErrors.IsEmpty) @@ -399,18 +372,18 @@ private ImmutableArray ValidateSourceSchemaTransition( /// on the given schema. /// private bool CanTransitionToSchemaThroughPath( - SatisfiabilityPath path, + PathNode? path, string schemaName) { - foreach (var pathItem in path) + for (var node = path; node is not null; node = node.Parent) { var lookupDirectives = _schema.GetPossibleFusionLookupDirectives( - pathItem.Type, + node.Item.Type, schemaName); var hasLookups = lookupDirectives.Count > 0; - var fieldExists = pathItem.Field.ExistsInSchema(schemaName); + var fieldExists = node.Item.Field.ExistsInSchema(schemaName); if (hasLookups && fieldExists) { @@ -441,18 +414,3 @@ private static IType CreateType(ITypeNode typeNode, ITypeDefinition namedType) return namedType; } } - -internal sealed class SatisfiabilityValidatorContext -{ - public Stack TypeContext { get; } = []; - - public SatisfiabilityPath Path { get; } = []; - - public SatisfiabilityPath CycleDetectionPath { get; } = []; - - public HashSet FieldAccessCache { get; } = []; - - public int Depth { get; set; } - - public bool DepthLimitReached { get; set; } -} diff --git a/src/HotChocolate/Fusion/test/Fusion.Composition.Tests/SatisfiabilityValidatorTests.cs b/src/HotChocolate/Fusion/test/Fusion.Composition.Tests/SatisfiabilityValidatorTests.cs index 449784350a5..159b3e7fcbc 100644 --- a/src/HotChocolate/Fusion/test/Fusion.Composition.Tests/SatisfiabilityValidatorTests.cs +++ b/src/HotChocolate/Fusion/test/Fusion.Composition.Tests/SatisfiabilityValidatorTests.cs @@ -377,6 +377,10 @@ type Category @key(fields: "id") { Assert.True(result.IsFailure); string.Join("\n\n", log.Select(e => e.Message)).MatchInlineSnapshot( """ + Unable to access the field 'Category.name' on path 'B:Query.categoryById'. + Unable to transition between schemas 'B' and 'A' for access to field 'A:Category.name'. + No lookups found for type 'Category' in schema 'A'. + Unable to access the field 'Category.id' on path 'A:Query.productById -> A:Product.category'. Unable to transition between schemas 'A' and 'B' for access to field 'B:Category.id'. Unable to satisfy the requirement '{ id }' for lookup 'categoryById' in schema 'B'. @@ -390,10 +394,6 @@ No other schemas contain the field 'Category.id'. Unable to satisfy the requirement 'id'. Unable to access the required field 'Category.id' on path 'A:Product.category'. No other schemas contain the field 'Category.id'. - - Unable to access the field 'Category.name' on path 'B:Query.categoryById'. - Unable to transition between schemas 'B' and 'A' for access to field 'A:Category.name'. - No lookups found for type 'Category' in schema 'A'. """); } @@ -2135,14 +2135,6 @@ Unable to satisfy the requirement 'section { name }'. Unable to transition between schemas 'B' and 'C' for access to required field 'C:Section.name'. No lookups found for type 'Section' in schema 'C'. - Unable to access the field 'Category.name' on path 'A:Query.productById -> B:Product.category'. - Unable to transition between schemas 'B' and 'C' for access to field 'C:Category.name'. - No lookups found for type 'Category' in schema 'C'. - - Unable to access the field 'Section.name' on path 'A:Query.productById -> B:Product.section
'. - Unable to transition between schemas 'B' and 'C' for access to field 'C:Section.name'. - No lookups found for type 'Section' in schema 'C'. - Unable to access the field 'Product.title' on path 'B:Query.productById'. Unable to satisfy the requirement '{ category { name } section { name } }' on field 'A:Product.title'. Unable to satisfy the requirement 'category { name }'. @@ -2155,6 +2147,14 @@ Unable to satisfy the requirement 'section { name }'. Unable to access the required field 'Section.name' on path 'B:Query.productById -> B:Product.section
'. Unable to transition between schemas 'B' and 'C' for access to required field 'C:Section.name'. No lookups found for type 'Section' in schema 'C'. + + Unable to access the field 'Category.name' on path 'A:Query.productById -> B:Product.category'. + Unable to transition between schemas 'B' and 'C' for access to field 'C:Category.name'. + No lookups found for type 'Category' in schema 'C'. + + Unable to access the field 'Section.name' on path 'A:Query.productById -> B:Product.section
'. + Unable to transition between schemas 'B' and 'C' for access to field 'C:Section.name'. + No lookups found for type 'Section' in schema 'C'. """); } @@ -2570,6 +2570,55 @@ type Query { Assert.True(result.IsSuccess); } + [Fact] + public void LargeTypeCycle_DoesNotCauseStackOverflow() + { + // arrange + // Creates a long type cycle (T1 → T2 → … → T500 → T1) across 5 identical schemas, + // where every schema provides every type and field. + const int typeCount = 500; + const int schemaCount = 5; + var schemas = new string[schemaCount]; + + var sb = new StringBuilder(); + sb.AppendLine("type Query {"); + + for (var t = 1; t <= typeCount; t++) + { + sb.AppendLine($" t{t}ById(id: ID!): T{t} @lookup"); + } + + sb.AppendLine("}"); + + for (var t = 1; t <= typeCount; t++) + { + var next = (t % typeCount) + 1; + sb.AppendLine( + $"type T{t} @key(fields: \"id\") {{ id: ID! @shareable next: T{next} @shareable }}"); + } + + var sdl = sb.ToString(); + + for (var i = 0; i < schemaCount; i++) + { + schemas[i] = sdl; + } + + var merger = new SourceSchemaMerger( + CreateSchemaDefinitions(schemas), + new SourceSchemaMergerOptions { AddFusionDefinitions = false }); + + var schema = merger.Merge().Value; + var log = new CompositionLog(); + var satisfiabilityValidator = new SatisfiabilityValidator(schema, log); + + // act + var result = satisfiabilityValidator.Validate(); + + // assert + Assert.True(result.IsSuccess); + } + [Theory] [MemberData(nameof(GlobalObjectIdentificationExamplesData))] public void GlobalObjectIdentification_Examples(string[] sdl, bool success, string? logs = null) @@ -2900,62 +2949,4 @@ type Cat implements Node { } }; } - - [Fact] - public void LargeTypeCycle_DoesNotCauseStackOverflow() - { - // arrange - // Creates a long type cycle (T1 → T2 → … → T500 → T1) across 5 identical schemas. - // Each schema provides every type and field. Without the recursion depth limit - // in VisitObjectType, the recursion depth is O(types × schemas) = 2500+ frames, - // which overflows the default 1 MB stack. - const int typeCount = 500; - const int schemaCount = 5; - var schemas = new string[schemaCount]; - - var sb = new StringBuilder(); - sb.AppendLine("type Query {"); - - for (var t = 1; t <= typeCount; t++) - { - sb.AppendLine($" t{t}ById(id: ID!): T{t} @lookup"); - } - - sb.AppendLine("}"); - - for (var t = 1; t <= typeCount; t++) - { - var next = (t % typeCount) + 1; - sb.AppendLine( - $"type T{t} @key(fields: \"id\") {{ id: ID! @shareable next: T{next} @shareable }}"); - } - - var sdl = sb.ToString(); - - for (var i = 0; i < schemaCount; i++) - { - schemas[i] = sdl; - } - - var merger = new SourceSchemaMerger( - CreateSchemaDefinitions(schemas), - new SourceSchemaMergerOptions { AddFusionDefinitions = false }); - - var schema = merger.Merge().Value; - var log = new CompositionLog(); - var satisfiabilityValidator = new SatisfiabilityValidator(schema, log); - - // act - var result = satisfiabilityValidator.Validate(); - - // assert - Assert.True(result.IsSuccess); - var logEntry = Assert.Single(log); - Assert.Equal(LogSeverity.Warning, logEntry.Severity); - Assert.Equal(LogEntryCodes.Unsatisfiable, logEntry.Code); - Assert.Equal( - "Satisfiability validation reached the maximum recursion depth (500) " - + "while visiting type 'T500'. Validation of deeply nested fields may be incomplete.", - logEntry.Message); - } }