diff --git a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidationFormatter.cs b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidationFormatter.cs index d4273452cb3..025ebb4ed4c 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidationFormatter.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidationFormatter.cs @@ -111,23 +111,25 @@ private static void AppendFinding( string indent, NitroSchemaValidationFinding finding) { - builder.Append(indent) - .Append("- ") - .Append(finding.Message); + builder.Append(indent); - if (finding.Code is not null) + for (var level = 0; level < finding.Depth; level++) { - builder.Append(" [code: ").Append(finding.Code).Append(']'); + builder.Append(" "); } + builder.Append("- "); + if (finding.Severity is not null) { - builder.Append(" [severity: ").Append(finding.Severity).Append(']'); + builder.Append(SeverityMarker(finding.Severity)).Append(' '); } - if (finding.Coordinate is not null) + builder.Append(finding.Message); + + if (finding.Code is not null) { - builder.Append(" [coordinate: ").Append(finding.Coordinate).Append(']'); + builder.Append(" [code: ").Append(finding.Code).Append(']'); } if (finding.Path is not null) @@ -146,4 +148,13 @@ private static void AppendFinding( builder.AppendLine(); } + + private static string SeverityMarker(string severity) + => severity switch + { + "BREAKING" => "✕", + "DANGEROUS" => "!", + "SAFE" => "✓", + _ => $"[{severity}]" + }; } diff --git a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidationReport.cs b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidationReport.cs index 15e108dbc1f..80cbba9caa3 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidationReport.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidationReport.cs @@ -114,7 +114,8 @@ internal sealed record NitroSchemaValidationFinding( int? Line = null, int? Column = null, string? Identity = null, - string? Severity = null); + string? Severity = null, + int Depth = 0); internal enum NitroSchemaValidationStatus { diff --git a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidator.cs b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidator.cs index e919cc0874e..80086c3a490 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidator.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/NitroSchemaValidator.cs @@ -596,7 +596,7 @@ private static void ParseSchemaChanges( foreach (var change in changes.EnumerateArray()) { - ParseSchemaChange(change, findings, null, ref findingCount); + ParseSchemaChange(change, findings, null, 0, ref findingCount); } } @@ -604,6 +604,7 @@ private static void ParseSchemaChange( JsonElement change, List findings, string? parentCoordinate, + int depth, ref int findingCount) { if (findingCount >= MaxParsedFindings) @@ -613,9 +614,21 @@ private static void ParseSchemaChange( } var coordinate = GetString(change, "coordinate") ?? parentCoordinate; + var typeName = GetString(change, "__typename") ?? "SchemaChange"; + var severity = GetString(change, "severity"); + findings.Add( + new NitroSchemaValidationFinding( + "Schema change violations", + typeName, + DescribeSchemaChange(change, typeName), + Coordinate: coordinate, + Identity: severity, + Severity: severity, + Depth: depth)); + findingCount++; + if (change.TryGetProperty("changes", out var nestedChanges) - && nestedChanges.ValueKind is JsonValueKind.Array - && nestedChanges.GetArrayLength() > 0) + && nestedChanges.ValueKind is JsonValueKind.Array) { foreach (var nestedChange in nestedChanges.EnumerateArray()) { @@ -623,81 +636,77 @@ private static void ParseSchemaChange( nestedChange, findings, coordinate, + depth + 1, ref findingCount); } - - return; } - - var typeName = GetString(change, "__typename") ?? "SchemaChange"; - var severity = GetString(change, "severity"); - findings.Add( - new NitroSchemaValidationFinding( - "Schema change violations", - typeName, - DescribeSchemaChange(change, typeName), - Coordinate: coordinate, - Identity: severity, - Severity: severity)); - findingCount++; } private static string DescribeSchemaChange(JsonElement change, string typeName) => typeName switch { - "TypeSystemMemberAddedChange" => "Type system member was added.", - "TypeSystemMemberRemovedChange" => "Type system member was removed.", - "FieldAddedChange" => "Field was added.", - "FieldRemovedChange" => "Field was removed.", - "InputFieldChanged" => "Input field was changed.", - "OutputFieldChanged" => "Output field was changed.", + "TypeSystemMemberAddedChange" => + $"Type system member {Coordinate(change)} was added.", + "TypeSystemMemberRemovedChange" => + $"Type system member {Coordinate(change)} was removed.", + "TypeSystemMemberModifiedChange" => + $"Type system member {Coordinate(change)} was modified.", + "ObjectModifiedChange" => $"Object type {Coordinate(change)} was modified.", + "InputObjectModifiedChange" => + $"Input object type {Coordinate(change)} was modified.", + "InterfaceModifiedChange" => $"Interface type {Coordinate(change)} was modified.", + "UnionModifiedChange" => $"Union {Coordinate(change)} was modified.", + "EnumModifiedChange" => $"Enum {Coordinate(change)} was modified.", + "ScalarModifiedChange" => $"Scalar {Coordinate(change)} was modified.", + "DirectiveModifiedChange" => $"Directive {Coordinate(change)} was modified.", + "FieldAddedChange" => + $"Field {Coordinate(change)} of type {Name(change, "typeName")} was added.", + "FieldRemovedChange" => + $"Field {Coordinate(change)} of type {Name(change, "typeName")} was removed.", + "InputFieldChanged" or "OutputFieldChanged" => + $"Field {Coordinate(change)} was modified.", + "ArgumentAdded" => $"The argument {Coordinate(change)} was added.", + "ArgumentRemoved" => $"The argument {Coordinate(change)} was removed.", + "ArgumentChanged" => $"The argument {Coordinate(change)} has changed.", "InterfaceImplementationAdded" => - $"Interface '{GetString(change, "interfaceName") ?? "unknown"}' was implemented.", + $"Interface implementation {Name(change, "interfaceName")} was added.", "InterfaceImplementationRemoved" => - $"Interface '{GetString(change, "interfaceName") ?? "unknown"}' was removed.", - "PossibleTypeAdded" => - $"Possible type '{GetString(change, "typeName") ?? "unknown"}' was added.", - "PossibleTypeRemoved" => - $"Possible type '{GetString(change, "typeName") ?? "unknown"}' was removed.", - "UnionMemberAdded" => - $"Union member '{GetString(change, "typeName") ?? "unknown"}' was added.", + $"Interface implementation {Name(change, "interfaceName")} was removed.", + "PossibleTypeAdded" => $"Possible type {Name(change, "typeName")} was added.", + "PossibleTypeRemoved" => $"Possible type {Name(change, "typeName")} was removed.", + "UnionMemberAdded" => $"Type {Name(change, "typeName")} was added to the union.", "UnionMemberRemoved" => - $"Union member '{GetString(change, "typeName") ?? "unknown"}' was removed.", - "ArgumentAdded" => DescribeArgumentChange(change, "added"), - "ArgumentRemoved" => DescribeArgumentChange(change, "removed"), - "ArgumentChanged" => DescribeArgumentChange(change, "changed"), + $"Type {Name(change, "typeName")} was removed from the union.", "DirectiveLocationAdded" => - $"Directive location '{GetString(change, "location") ?? "unknown"}' was added.", + $"Directive location {Name(change, "location")} was added.", "DirectiveLocationRemoved" => - $"Directive location '{GetString(change, "location") ?? "unknown"}' was removed.", - "EnumValueAdded" => "Enum value was added.", - "EnumValueRemoved" => "Enum value was removed.", - "EnumValueChanged" => "Enum value was changed.", - "DescriptionChanged" => "Description was changed.", + $"Directive location {Name(change, "location")} was removed.", + "EnumValueAdded" => $"Enum value {Coordinate(change)} was added.", + "EnumValueRemoved" => $"Enum value {Coordinate(change)} was removed.", + "EnumValueChanged" => $"Enum value {Coordinate(change)} was modified.", + "DescriptionChanged" => DescribeDescriptionChange(change), "DeprecatedChange" => GetString(change, "deprecationReason") is { } reason - ? $"Deprecation changed to '{reason}'." - : "Deprecation was removed.", - "TypeChanged" => DescribeTypeChange(change), - _ => $"{typeName} was detected." + ? $"The member was deprecated with the reason '{reason}'." + : "The member was deprecated.", + "TypeChanged" => + $"Type changed from {Name(change, "oldType")} to {Name(change, "newType")}.", + _ => $"Unknown change '{typeName}' was detected." }; - private static string DescribeArgumentChange(JsonElement change, string action) - { - var name = GetString(change, "name") ?? "unknown"; - var type = GetString(change, "typeName"); - - return type is null - ? $"Argument '{name}' was {action}." - : $"Argument '{name}' of type '{type}' was {action}."; - } + private static string DescribeDescriptionChange(JsonElement change) + => (GetString(change, "old"), GetString(change, "new")) switch + { + ({ } old, { } @new) => $"Description changed from '{old}' to '{@new}'.", + (null, { } @new) => $"Description added: '{@new}'.", + ({ } old, null) => $"Description removed: '{old}'.", + _ => "Description was changed." + }; - private static string DescribeTypeChange(JsonElement change) - { - var oldType = GetString(change, "oldType") ?? "unknown"; - var newType = GetString(change, "newType") ?? "unknown"; + private static string Coordinate(JsonElement change) + => GetString(change, "coordinate") ?? "unknown"; - return $"Type changed from '{oldType}' to '{newType}'."; - } + private static string Name(JsonElement change, string propertyName) + => GetString(change, propertyName) ?? "unknown"; private static void ParseNestedErrors( JsonElement error, diff --git a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/Operations/PollNitroSchemaValidation.graphql b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/Operations/PollNitroSchemaValidation.graphql index d73d85d9ffc..044f8cd2c26 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/Operations/PollNitroSchemaValidation.graphql +++ b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/Operations/PollNitroSchemaValidation.graphql @@ -191,7 +191,7 @@ fragment SchemaChangeSummary on SchemaChangeLogEntry { } } -fragment SchemaChangeDetail on SchemaChangeLogEntry { +fragment SchemaChangeDetail on SchemaChange { ...SchemaChangeLeaf ... on InputFieldChanged { changes { @@ -220,8 +220,9 @@ fragment SchemaChangeDetail on SchemaChangeLogEntry { } } -fragment SchemaChangeLeaf on SchemaChangeLogEntry { - ...SchemaChangeSummary +fragment SchemaChangeLeaf on SchemaChange { + __typename + severity ... on InterfaceImplementationAdded { interfaceName } diff --git a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/Operations/PollNitroSchemaValidation.graphql.sha256 b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/Operations/PollNitroSchemaValidation.graphql.sha256 index bb9e860ad5e..2db8a929079 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/Operations/PollNitroSchemaValidation.graphql.sha256 +++ b/src/HotChocolate/Fusion/src/Fusion.Aspire/Nitro/Operations/PollNitroSchemaValidation.graphql.sha256 @@ -1 +1 @@ -0fcb0a11b85be6ede7bb6141f0cdcc34a3fc7eaa611747d013d02b9374e48e22 +9c6725ca85cf073343e4136f6a9b53353328156eb4335363fe624bfefc86bdd3 diff --git a/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroOperationDocumentsTests.cs b/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroOperationDocumentsTests.cs index c36d96bbd3e..f6f2216f8b2 100644 --- a/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroOperationDocumentsTests.cs +++ b/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroOperationDocumentsTests.cs @@ -65,6 +65,26 @@ public void ValidationOperationName_Should_MatchTheOperationInTheDocument( Assert.Contains(expectedDeclaration, document, StringComparison.Ordinal); } + [Fact] + public void GetPollSchemaValidationDocument_Should_RequestNestedSchemaChangeDetails() + { + // act + var document = NitroOperationDocuments.GetPollSchemaValidationDocument(); + + // assert + // the fragments must be defined and actually spread into the selections + Assert.Contains( + "fragment SchemaChangeDetail on SchemaChange", + document, + StringComparison.Ordinal); + Assert.Contains( + "fragment SchemaChangeLeaf on SchemaChange", + document, + StringComparison.Ordinal); + Assert.Contains("...SchemaChangeDetail", document, StringComparison.Ordinal); + Assert.Contains("...SchemaChangeLeaf", document, StringComparison.Ordinal); + } + [Theory] [InlineData("GetNitroStageVersion", "query GetNitroStageVersion(")] [InlineData("WatchNitroStage", "subscription WatchNitroStage(")] @@ -114,7 +134,7 @@ public void GetCompositionSettingsOperationId_Should_ReturnTheEmbeddedHash() "89e8800b8720401041061528aef1101683b241d7d24c7f0912928a1a901def02")] [InlineData( "PollNitroSchemaValidation", - "0fcb0a11b85be6ede7bb6141f0cdcc34a3fc7eaa611747d013d02b9374e48e22")] + "9c6725ca85cf073343e4136f6a9b53353328156eb4335363fe624bfefc86bdd3")] public void GetValidationOperationId_Should_ReturnTheEmbeddedHash( string operationName, string expectedOperationId) diff --git a/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroSchemaValidationFormatterTests.cs b/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroSchemaValidationFormatterTests.cs index c565fe9bf4d..c580dd6e908 100644 --- a/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroSchemaValidationFormatterTests.cs +++ b/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroSchemaValidationFormatterTests.cs @@ -90,12 +90,26 @@ public void Format_Should_PreserveHierarchyWithoutIncludingReportMetadata() ]) ], [ + new NitroSchemaValidationFinding( + "Schema change violations", + "ObjectModifiedChange", + "Object type Product was modified.", + Coordinate: "Product", + Severity: "BREAKING"), new NitroSchemaValidationFinding( "Schema change violations", "FieldRemovedChange", - "Field was removed.", + "Field Product.name of type Product was removed.", Coordinate: "Product.name", - Severity: "BREAKING") + Severity: "BREAKING", + Depth: 1), + new NitroSchemaValidationFinding( + "Schema change violations", + "FieldAddedChange", + "Field Product.price of type Product was added.", + Coordinate: "Product.price", + Severity: "SAFE", + Depth: 1) ], null, DateTimeOffset.UtcNow); @@ -106,12 +120,14 @@ public void Format_Should_PreserveHierarchyWithoutIncludingReportMetadata() // assert output.MatchInlineSnapshot( """ - Nitro schema validation found 2 violations; 1 client and 1 operation are affected. + Nitro schema validation found 4 violations; 1 client and 1 operation are affected. Client: Inventory UI (client-id) Operation: operation-hash [tags: production, canary] - Field productName does not exist. [code: HC001] [path: query.productName] [line: 4, column: 7] Schema change violations: - - Field was removed. [severity: BREAKING] [coordinate: Product.name] + - ✕ Object type Product was modified. + - ✕ Field Product.name of type Product was removed. + - ✓ Field Product.price of type Product was added. """); } } diff --git a/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroSchemaValidatorTests.cs b/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroSchemaValidatorTests.cs index f288a711c44..75dda5cd66f 100644 --- a/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroSchemaValidatorTests.cs +++ b/src/HotChocolate/Fusion/test/Fusion.Aspire.Tests/Nitro/NitroSchemaValidatorTests.cs @@ -133,7 +133,7 @@ public async Task ValidateAsync_Should_UseClientIdAndQueryMessage_When_ClientIsN Assert.Equal( "Violations|more=True|clients=client-fallback/unknown>" + "operation-fallback[production]>" - + "Client contract violations>PersistedQueryValidationError|-|-|-|-:-|-|" + + "Client contract violations>PersistedQueryValidationError|-|-|-|-:-|-|d0|" + "The operation no longer validates." + "|findings=-", DescribeViolations(report)); @@ -301,7 +301,7 @@ public async Task ValidateAsync_Should_KeepKnownFindings_When_FailureAlsoContain // assert Assert.Equal( "Violations|more=False|clients=-|findings=Schema syntax errors>" - + "SchemaVersionSyntaxError|-|-|-|7:9|-|Unexpected token.", + + "SchemaVersionSyntaxError|-|-|-|7:9|-|d0|Unexpected token.", DescribeViolations(report)); } @@ -432,7 +432,7 @@ public static TheoryData KnownTerminalFailureData() """, "Violations|more=False|clients=client-1/Web>operation-1[production]>" + "Client contract violations>PersistedQueryValidationError|HC001|-|" - + "query.product|2:3|-|Unknown field." + + "query.product|2:3|-|d0|Unknown field." + "|findings=-" }, { @@ -480,9 +480,16 @@ public static TheoryData KnownTerminalFailureData() } """, "Violations|more=False|clients=-|findings=Schema change violations>" - + "FieldRemovedChange|-|Product.name|-|-:-|BREAKING|Field was removed.," - + "Schema change violations>TypeChanged|-|Query.product|-|-:-|BREAKING|" - + "Type changed from 'Product' to 'Product!'." + + "ObjectModifiedChange|-|Product|-|-:-|BREAKING|d0|" + + "Object type Product was modified.," + + "Schema change violations>FieldRemovedChange|-|Product.name|-|-:-|BREAKING|d1|" + + "Field Product.name of type Product was removed.," + + "Schema change violations>ObjectModifiedChange|-|Query|-|-:-|BREAKING|d0|" + + "Object type Query was modified.," + + "Schema change violations>OutputFieldChanged|-|Query.product|-|-:-|BREAKING|d1|" + + "Field Query.product was modified.," + + "Schema change violations>TypeChanged|-|Query.product|-|-:-|BREAKING|d2|" + + "Type changed from Product to Product!." }, { """ @@ -498,7 +505,7 @@ public static TheoryData KnownTerminalFailureData() } """, "Violations|more=False|clients=-|findings=Invalid GraphQL schema>" - + "InvalidGraphQLSchemaError|HC002|-|-|-:-|-|Type Query is missing." + + "InvalidGraphQLSchemaError|HC002|-|-|-:-|-|d0|Type Query is missing." }, { """ @@ -533,7 +540,7 @@ public static TheoryData KnownTerminalFailureData() """, "Violations|more=False|clients=-|findings=OpenAPI>" + "OpenApiCollectionValidationDocumentError|OA001|-|paths./products|10:5|" - + "openapi-1|GET /products|Store API, GET /products: The endpoint is missing." + + "openapi-1|GET /products|d0|Store API, GET /products: The endpoint is missing." }, { """ @@ -567,7 +574,7 @@ public static TheoryData KnownTerminalFailureData() """, "Violations|more=False|clients=-|findings=MCP>" + "McpFeatureCollectionValidationDocumentError|MCP001|-|tools.findProduct|4:2|" - + "mcp-1|findProduct|Store tools, findProduct: The tool is invalid." + + "mcp-1|findProduct|d0|Store tools, findProduct: The tool is invalid." }, { """ @@ -580,7 +587,7 @@ public static TheoryData KnownTerminalFailureData() } """, "Violations|more=False|clients=-|findings=Schema syntax errors>" - + "SchemaVersionSyntaxError|-|-|-|7:9|-|Unexpected token." + + "SchemaVersionSyntaxError|-|-|-|7:9|-|d0|Unexpected token." }, { """ @@ -591,7 +598,7 @@ public static TheoryData KnownTerminalFailureData() } """, "Violations|more=False|clients=-|findings=Operation policy errors>" - + "OperationsAreNotAllowedError|-|-|-|-:-|ForbiddenOperation|" + + "OperationsAreNotAllowedError|-|-|-|-:-|ForbiddenOperation|d0|" + "Operations are forbidden." }, { @@ -602,7 +609,7 @@ public static TheoryData KnownTerminalFailureData() } """, "Violations|more=False|clients=-|findings=Processing errors>" - + "ProcessingTimeoutError|-|-|-|-:-|-|Processing timed out." + + "ProcessingTimeoutError|-|-|-|-:-|-|d0|Processing timed out." }, { """ @@ -612,7 +619,36 @@ public static TheoryData KnownTerminalFailureData() } """, "Violations|more=False|clients=-|findings=Processing errors>" - + "ReadyTimeoutError|-|-|-|-:-|-|The task did not become ready." + + "ReadyTimeoutError|-|-|-|-:-|-|d0|The task did not become ready." + }, + { + """ + { + "__typename": "SchemaVersionChangeViolationError", + "message": "Non-breaking schema changes.", + "changes": [ + { + "__typename": "ObjectModifiedChange", + "severity": "SAFE", + "coordinate": "Product", + "changes": [ + { + "__typename": "FieldAddedChange", + "severity": "SAFE", + "coordinate": "Product.price", + "typeName": "Product", + "fieldName": "price" + } + ] + } + ] + } + """, + "Violations|more=False|clients=-|findings=Schema change violations>" + + "ObjectModifiedChange|-|Product|-|-:-|SAFE|d0|" + + "Object type Product was modified.," + + "Schema change violations>FieldAddedChange|-|Product.price|-|-:-|SAFE|d1|" + + "Field Product.price of type Product was added." }, { """ @@ -622,7 +658,7 @@ public static TheoryData KnownTerminalFailureData() } """, "Violations|more=False|clients=-|findings=Processing errors>" - + "UnexpectedProcessingError|-|-|-|-:-|-|Processing failed unexpectedly." + + "UnexpectedProcessingError|-|-|-|-:-|-|d0|Processing failed unexpectedly." } }; @@ -705,7 +741,8 @@ private static string DescribeViolations(NitroSchemaValidationReport report) private static string DescribeFinding(NitroSchemaValidationFinding finding) => $"{finding.Group}>{finding.Kind}|{finding.Code ?? "-"}|{finding.Coordinate ?? "-"}|" + $"{finding.Path ?? "-"}|{finding.Line?.ToString() ?? "-"}:" - + $"{finding.Column?.ToString() ?? "-"}|{finding.Identity ?? "-"}|{finding.Message}"; + + $"{finding.Column?.ToString() ?? "-"}|{finding.Identity ?? "-"}|d{finding.Depth}|" + + finding.Message; private static string DescribeUnavailable( NitroSchemaValidationReport report,