diff --git a/src/HotChocolate/Adapters/src/Adapters.OpenApi.AspNetCore/DynamicOpenApiDocumentTransformer.cs b/src/HotChocolate/Adapters/src/Adapters.OpenApi.AspNetCore/DynamicOpenApiDocumentTransformer.cs index f3ab58b737d..486ee4ac773 100644 --- a/src/HotChocolate/Adapters/src/Adapters.OpenApi.AspNetCore/DynamicOpenApiDocumentTransformer.cs +++ b/src/HotChocolate/Adapters/src/Adapters.OpenApi.AspNetCore/DynamicOpenApiDocumentTransformer.cs @@ -58,10 +58,11 @@ public void AddDefinitions( OpenApiEndpointDefinition[] endpoints, OpenApiModelDefinition[] models, IDictionary modelsByName, - ISchemaDefinition schema) + ISchemaDefinition schema, + IOpenApiDiagnosticEvents events) { var (endpointDescriptors, modelDescriptors, inputComponents) = - new DocumentBuilder(schema, modelsByName).Build(endpoints, models); + new DocumentBuilder(schema, modelsByName, events).Build(endpoints, models); _endpoints = endpointDescriptors; _models = modelDescriptors; @@ -180,10 +181,12 @@ public void Add(OpenApiEndpointDefinitionParameter parameter) // ambient values (schema, models) are read from fields instead of threaded as parameters. private sealed class DocumentBuilder( ISchemaDefinition schema, - IDictionary modelsByName) + IDictionary modelsByName, + IOpenApiDiagnosticEvents events) { private readonly ISchemaDefinition _schema = schema; private readonly IDictionary _modelsByName = modelsByName; + private readonly IOpenApiDiagnosticEvents _events = events; private readonly Dictionary _operationIdUsages = []; private readonly Dictionary _inputComponents = []; private readonly HashSet _registeredInputNames = []; @@ -201,9 +204,16 @@ private sealed class DocumentBuilder( var endpointDescriptor = CreateEndpointDescriptor(endpoint); endpointDescriptors.Add(endpointDescriptor); } - catch + catch (Exception exception) { - // If the construction of an endpoint descriptor fails, we just skip over it. + // If the construction of an endpoint descriptor fails, we skip over it and + // report why the endpoint is missing from the OpenAPI document. + var endpointName = endpoint.OperationDefinition.Name?.Value; + + ReportError( + $"Endpoint '{endpointName}' could not be added to the OpenAPI document: " + + exception.Message, + endpoint); } } @@ -214,9 +224,14 @@ private sealed class DocumentBuilder( var modelDescriptor = CreateModelDescriptor(model); modelDescriptors.Add(modelDescriptor); } - catch + catch (Exception exception) { - // If the construction of an model descriptor fails, we just skip over it. + // If the construction of an model descriptor fails, we skip over it and + // report why the model is missing from the OpenAPI document. + ReportError( + $"Model '{model.Name}' could not be added to the OpenAPI document: " + + exception.Message, + model); } } @@ -537,9 +552,22 @@ private OpenApiSchemaAbstraction CreateOpenApiSchemaForSelectionSet( var fieldName = field.Name.Value; var responseName = field.Alias?.Value ?? fieldName; - var fieldType = fieldName == IntrospectionFieldNames.TypeName ? - new NonNullType(_schema.Types["String"]) : - ((IComplexTypeDefinition)namedType).Fields[fieldName].Type; + IOutputType fieldType; + + if (fieldName == IntrospectionFieldNames.TypeName) + { + fieldType = new NonNullType(_schema.Types["String"]); + } + else if (namedType is IComplexTypeDefinition complexType + && complexType.Fields.TryGetField(fieldName, out var fieldDefinition)) + { + fieldType = fieldDefinition.Type; + } + else + { + throw new InvalidOperationException( + $"Expected to find field '{fieldName}' on type '{namedType.Name}'."); + } OpenApiSchemaAbstraction typeSchema; if (field.SelectionSet is not null) @@ -622,7 +650,12 @@ private OpenApiSchemaAbstraction CreateOpenApiSchemaForSelectionSet( } else { - var fragment = localFragmentLookup[fragmentName]; + if (!localFragmentLookup.TryGetValue(fragmentName, out var fragment)) + { + throw new InvalidOperationException( + $"Expected to find a definition for fragment '{fragmentName}'."); + } + var typeCondition = _schema.Types[fragment.TypeCondition.Name.Value]; var isDifferentType = !typeCondition.IsAssignableFrom(namedType); var typeName = typeCondition.Name; @@ -1301,5 +1334,10 @@ private static string GetOperationId(string? operationName) span[0] = char.ToLowerInvariant(span[0]); }); } + + private void ReportError(string message, IOpenApiDefinition definition) + { + _events.ValidationErrors([new OpenApiDefinitionValidationError(message, definition)]); + } } } diff --git a/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointDescriptor.cs b/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointDescriptor.cs index 379e6c5a596..73142936a3e 100644 --- a/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointDescriptor.cs +++ b/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointDescriptor.cs @@ -6,12 +6,15 @@ namespace HotChocolate.Adapters.OpenApi; internal sealed record OpenApiEndpointDescriptor( DocumentNode Document, - bool HasValidDocument, + IReadOnlyList DocumentErrors, string HttpMethod, RoutePattern Route, VariableValueInsertionTrie ParameterTrie, string? VariableFilledThroughBody, - OpenApiResponseBodySelection ResponseBodySelection); + OpenApiResponseBodySelection ResponseBodySelection) +{ + public bool HasValidDocument => DocumentErrors.Count == 0; +} internal interface IVariableValueInsertionTrieSegment; diff --git a/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointFactory.cs b/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointFactory.cs index 9d9a54b6359..367df6e2d3e 100644 --- a/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointFactory.cs +++ b/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/Execution/OpenApiEndpointFactory.cs @@ -73,11 +73,10 @@ public static OpenApiEndpointDescriptor CreateEndpointDescriptor( var documentValidator = schema.Services.GetRequiredService(); var validationResult = documentValidator.Validate(schema, document); - var hasValidDocument = !validationResult.HasErrors; return new OpenApiEndpointDescriptor( document, - hasValidDocument, + validationResult.Errors, endpointDefinition.HttpMethod, route, parameterTrie, diff --git a/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/IDynamicOpenApiDocumentTransformer.cs b/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/IDynamicOpenApiDocumentTransformer.cs index 6e061261a9c..c8eecf69eff 100644 --- a/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/IDynamicOpenApiDocumentTransformer.cs +++ b/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/IDynamicOpenApiDocumentTransformer.cs @@ -6,5 +6,6 @@ void AddDefinitions( OpenApiEndpointDefinition[] endpoints, OpenApiModelDefinition[] models, IDictionary modelsByName, - ISchemaDefinition schema); + ISchemaDefinition schema, + IOpenApiDiagnosticEvents events); } diff --git a/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/OpenApiDefinitionRegistry.cs b/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/OpenApiDefinitionRegistry.cs index b0e32246bf6..e05167f8c99 100644 --- a/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/OpenApiDefinitionRegistry.cs +++ b/src/HotChocolate/Adapters/src/Adapters.OpenApi.Core/OpenApiDefinitionRegistry.cs @@ -153,12 +153,13 @@ private void UpdateAllDefinitions( validDefinitions.Add(definition); } - UpdateEndpointsAndOpenApiDefinitions(validDefinitions, schema); + UpdateEndpointsAndOpenApiDefinitions(validDefinitions, schema, events); } private void UpdateEndpointsAndOpenApiDefinitions( List definitions, - ISchemaDefinition schema) + ISchemaDefinition schema, + IOpenApiDiagnosticEvents events) { var endpoints = definitions .OfType() @@ -191,14 +192,23 @@ private void UpdateEndpointsAndOpenApiDefinitions( continue; } + var endpointName = endpoint.OperationDefinition.Name!.Value; + OpenApiEndpointDescriptor descriptor; try { descriptor = OpenApiEndpointFactory.CreateEndpointDescriptor(endpoint, modelsByName, schema); } - catch + catch (Exception exception) { + events.ValidationErrors( + [ + new OpenApiDefinitionValidationError( + $"Endpoint '{endpointName}' could not be initialized: {exception.Message}", + endpoint) + ]); + continue; } @@ -217,18 +227,35 @@ private void UpdateEndpointsAndOpenApiDefinitions( keyHasValid.Add(key); } - else if (!keyToIndex.ContainsKey(key)) + else { - keyToIndex[key] = chosenEndpoints.Count; - chosenEndpoints.Add((endpoint, descriptor)); + events.ValidationErrors( + [ + .. descriptor.DocumentErrors.Select( + error => new OpenApiDefinitionValidationError( + $"Endpoint '{endpointName}' has an invalid document: {error.Message}", + endpoint)) + ]); + + if (!keyToIndex.ContainsKey(key)) + { + keyToIndex[key] = chosenEndpoints.Count; + chosenEndpoints.Add((endpoint, descriptor)); + } } } + // Endpoints with an invalid document cannot execute, so they are left out of the + // OpenAPI document rather than described as callable. _transformer.AddDefinitions( - chosenEndpoints.Select(e => e.Definition).ToArray(), + chosenEndpoints + .Where(e => e.Descriptor.HasValidDocument) + .Select(e => e.Definition) + .ToArray(), models, modelsByName, - schema); + schema, + events); var httpEndpoints = new List(); diff --git a/src/HotChocolate/Adapters/test/Adapters.OpenApi.Tests/Validation/ValidationTestBase.cs b/src/HotChocolate/Adapters/test/Adapters.OpenApi.Tests/Validation/ValidationTestBase.cs index ea9a1449dda..956420b8986 100644 --- a/src/HotChocolate/Adapters/test/Adapters.OpenApi.Tests/Validation/ValidationTestBase.cs +++ b/src/HotChocolate/Adapters/test/Adapters.OpenApi.Tests/Validation/ValidationTestBase.cs @@ -95,10 +95,97 @@ preferences @responseBody { eventListener.HasReportedErrors.Wait(cts.Token); + // assert + Assert.Collection( + eventListener.Errors, + error => Assert.Equal( + "OpenAPI models cannot contain the '@responseBody' directive.", + error.Message), + error => Assert.Equal( + "Endpoint 'GetUser' has an invalid document: " + + "The specified fragment `UserPreferences` does not exist.", + error.Message)); + } + + [Fact] + public async Task Model_Type_Condition_Not_In_Schema_RaisesError() + { + // arrange + using var cts = new CancellationTokenSource(s_testTimeout); + var storage = new TestOpenApiDefinitionStorage( + """ + fragment User on NonExistentType { + id + } + """); + var eventListener = new TestOpenApiDiagnosticEventListener(); + var server = CreateTestServer(storage, eventListener); + + // act + await server.Services.GetRequestExecutorAsync(cancellationToken: cts.Token); + + eventListener.HasReportedErrors.Wait(cts.Token); + // assert var error = Assert.Single(eventListener.Errors); Assert.Equal( - "OpenAPI models cannot contain the '@responseBody' directive.", + "Model 'User' could not be added to the OpenAPI document: " + + "Expected to find type condition type in the schema.", + error.Message); + } + + [Fact] + public async Task Model_Selects_Unknown_Field_RaisesError() + { + // arrange + using var cts = new CancellationTokenSource(s_testTimeout); + var storage = new TestOpenApiDefinitionStorage( + """ + fragment User on User { + nonExistentField + } + """); + var eventListener = new TestOpenApiDiagnosticEventListener(); + var server = CreateTestServer(storage, eventListener); + + // act + await server.Services.GetRequestExecutorAsync(cancellationToken: cts.Token); + + eventListener.HasReportedErrors.Wait(cts.Token); + + // assert + var error = Assert.Single(eventListener.Errors); + Assert.Equal( + "Model 'User' could not be added to the OpenAPI document: " + + "Expected to find field 'nonExistentField' on type 'User'.", + error.Message); + } + + [Fact] + public async Task Model_References_Missing_Fragment_RaisesError() + { + // arrange + using var cts = new CancellationTokenSource(s_testTimeout); + var storage = new TestOpenApiDefinitionStorage( + """ + fragment User on User { + id + ...MissingHelper + } + """); + var eventListener = new TestOpenApiDiagnosticEventListener(); + var server = CreateTestServer(storage, eventListener); + + // act + await server.Services.GetRequestExecutorAsync(cancellationToken: cts.Token); + + eventListener.HasReportedErrors.Wait(cts.Token); + + // assert + var error = Assert.Single(eventListener.Errors); + Assert.Equal( + "Model 'User' could not be added to the OpenAPI document: " + + "Expected to find a definition for fragment 'MissingHelper'.", error.Message); } @@ -759,5 +846,104 @@ query GetUsers { Assert.Equal($"Endpoint has invalid route pattern '{route}'.", error.Message); } + [Fact] + public async Task Endpoint_References_Missing_Model_RaisesError() + { + // arrange + using var cts = new CancellationTokenSource(s_testTimeout); + var storage = new TestOpenApiDefinitionStorage( + """ + query GetUser @http(method: GET, route: "/user") { + userById(id: "1") { + ...User + } + } + """); + var eventListener = new TestOpenApiDiagnosticEventListener(); + var server = CreateTestServer(storage, eventListener); + + // act + await server.Services.GetRequestExecutorAsync(cancellationToken: cts.Token); + + eventListener.HasReportedErrors.Wait(cts.Token); + + // assert + var error = Assert.Single(eventListener.Errors); + Assert.Equal( + "Endpoint 'GetUser' has an invalid document: " + + "The specified fragment `User` does not exist.", + error.Message); + } + + [Fact] + public async Task Endpoint_References_Local_Fragment_Of_Model_RaisesError() + { + // arrange + // 'UserContact' is a local fragment of the 'User' model, so it cannot be spread + // from another definition. + using var cts = new CancellationTokenSource(s_testTimeout); + var storage = new TestOpenApiDefinitionStorage( + """ + query GetUser @http(method: GET, route: "/user") { + userById(id: "1") { + ...UserContact + } + } + """, + """ + fragment User on User { + id + name + } + + fragment UserContact on User { + email + } + """); + var eventListener = new TestOpenApiDiagnosticEventListener(); + var server = CreateTestServer(storage, eventListener); + + // act + await server.Services.GetRequestExecutorAsync(cancellationToken: cts.Token); + + eventListener.HasReportedErrors.Wait(cts.Token); + + // assert + var error = Assert.Single(eventListener.Errors); + Assert.Equal( + "Endpoint 'GetUser' has an invalid document: " + + "The specified fragment `UserContact` does not exist.", + error.Message); + } + + [Fact] + public async Task Endpoint_Selects_Unknown_Field_RaisesError() + { + // arrange + using var cts = new CancellationTokenSource(s_testTimeout); + var storage = new TestOpenApiDefinitionStorage( + """ + query GetUser @http(method: GET, route: "/user") { + userById(id: "1") { + nonExistentField + } + } + """); + var eventListener = new TestOpenApiDiagnosticEventListener(); + var server = CreateTestServer(storage, eventListener); + + // act + await server.Services.GetRequestExecutorAsync(cancellationToken: cts.Token); + + eventListener.HasReportedErrors.Wait(cts.Token); + + // assert + var error = Assert.Single(eventListener.Errors); + Assert.Equal( + "Endpoint 'GetUser' has an invalid document: " + + "The field `nonExistentField` does not exist on the type `User`.", + error.Message); + } + #endregion } diff --git a/website/content/docs/hotchocolate/build/adapters/openapi.md b/website/content/docs/hotchocolate/build/adapters/openapi.md index ca9fa487852..25d042ea404 100644 --- a/website/content/docs/hotchocolate/build/adapters/openapi.md +++ b/website/content/docs/hotchocolate/build/adapters/openapi.md @@ -221,7 +221,7 @@ To make a fragment referenceable from another document, give it a document of it Fragment names must be unique across every document that contributes to a single endpoint, which is the endpoint's own document plus each model it spreads, transitively. When two of those documents define a fragment with the same name, the endpoint's composed document is invalid and the endpoint responds with HTTP 500. Private helper fragments are not namespaced by their document, so this applies to them as well. -An endpoint that spreads a fragment no document defines is also invalid, and likewise responds with HTTP 500. In both cases the endpoint is still routed, and it is omitted from the OpenAPI specification. +An endpoint that spreads a fragment no document defines is also invalid, and likewise responds with HTTP 500. In both cases the endpoint is still routed, and it is omitted from the OpenAPI specification. Register a diagnostic event listener (see [Diagnostics](#diagnostics)) to see the reason. # Storage @@ -277,6 +277,41 @@ builder The storage implements `IObservable`. When you push `Updated` or `Removed` events through this observable, the adapter picks up changes at runtime, adding, updating, or removing HTTP endpoints without a restart. This hot-reload behavior extends to the OpenAPI specification. +# Diagnostics + +A definition that the adapter cannot use is skipped rather than throwing at startup, so an endpoint can be routed and still fail every call with HTTP 500. Derive from `OpenApiDiagnosticEventListener` to observe why: + +```csharp +using HotChocolate.Adapters.OpenApi; + +public class LoggingOpenApiDiagnosticEventListener( + ILogger logger) + : OpenApiDiagnosticEventListener +{ + public override void ValidationErrors( + IReadOnlyList errors) + { + foreach (var error in errors) + { + logger.LogError("{ValidationError}", error.Message); + } + } +} +``` + +Register it on the GraphQL server. The listener is activated from the schema services, so any application service it takes as a constructor argument, `ILogger` included, must be made available with `AddApplicationService()`: + +```csharp +builder + .AddGraphQL() + .AddQueryType() + .AddOpenApi() + .AddApplicationService>() + .AddDiagnosticEventListener(); +``` + +The listener receives an error for every definition the adapter cannot use: one that fails a validation rule, an endpoint whose composed document does not validate against the schema, an endpoint that could not be initialized, and a definition that could not be added to the OpenAPI document. Errors are reported whenever definitions are loaded or reloaded, so a hot-reload that breaks an endpoint reports it at that moment. + # OpenAPI Specification The adapter integrates with ASP.NET Core's built-in OpenAPI support. After you register `AddOpenApi(options => options.AddGraphQLTransformer())`, the generated endpoints appear in the OpenAPI document at `/openapi/v1.json`.