diff --git a/CHANGELOG.md b/CHANGELOG.md index 79c6c24..7922a5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# Changes in 7.1.10 +- Fixed: the same `[FromQuery]`/`[AsParameters]` DTO shared by more than one endpoint lost its FluentValidation rules on every endpoint after the first (Issue #223) + - `FluentValidationOperationFilter` runs once per operation. The Issue #180 cleanup removes the temporary container schema from `SchemaRepository.Schemas`, but Swashbuckle keeps the type in its internal reserved-ids map, which the cleanup does not touch. For the 2nd+ endpoint, `GetSchemaForType` → `GenerateSchema` then returns a bare `$ref` (no `Properties`) and the schema is no longer in `Schemas`, so the "has properties" guard was skipped and no rules were applied. Only reproduces with the default `RemoveUnusedQuerySchemas = true` + - Fix: in `SwashbuckleSchemaProvider.GetSchemaForType`, when the returned schema has no properties and the id is absent from the shared repository (the reserved-but-removed state), the concrete schema is recovered by generating it into a throwaway `SchemaRepository`. Fully isolated — it never mutates the shared repository or its reserved-id state — so the Issue #180 cleanup and all other `OperationFilter` behavior (required marking #209, nested `[FromQuery]` #211/#213, request bodies, #216) are preserved + # Changes in 7.1.9 - Fixed: numeric bounds from `short`/`byte`/`ushort`/`uint`/`ulong`/`sbyte`-typed rules were silently dropped (Issue #222) - `IsNumeric` in the shared core (`MicroElements.OpenApi.FluentValidation`) only recognized `int`/`long`/`float`/`double`/`decimal`/`BigInteger`, so a `Between`/`Comparison` rule whose bound was a small integer type (e.g. `InclusiveBetween((short)1, (short)99)`) matched but produced no `minimum`/`maximum` — even though the generator emits `"type": "integer"` for the property. This could not be worked around at the validator definition site because those rule overloads only accept bounds of the property's own type diff --git a/src/MicroElements.Swashbuckle.FluentValidation/Swashbuckle/SwashbuckleSchemaProvider.cs b/src/MicroElements.Swashbuckle.FluentValidation/Swashbuckle/SwashbuckleSchemaProvider.cs index fcc6650..4bf3b65 100644 --- a/src/MicroElements.Swashbuckle.FluentValidation/Swashbuckle/SwashbuckleSchemaProvider.cs +++ b/src/MicroElements.Swashbuckle.FluentValidation/Swashbuckle/SwashbuckleSchemaProvider.cs @@ -66,6 +66,25 @@ public OpenApiSchema GetSchemaForType(Type type) } #endif + // Issue #223: when the same [FromQuery] DTO is shared by several endpoints, the per-operation + // Issue #180 cleanup removes the container schema from SchemaRepository.Schemas, but Swashbuckle + // still keeps the type in its internal reserved-ids map. For the 2nd+ endpoint, GenerateSchema then + // returns a bare $ref (no Properties) and the schema is no longer in Schemas, so the rules would be + // skipped. Recover the concrete schema by generating it into a throwaway repository — this is fully + // isolated: it never touches the shared repository or its reserved-id state. + if ((schema.Properties == null || schema.Properties.Count == 0) && + !_schemaRepository.Schemas.ContainsKey(schemaId)) + { + var throwawayRepository = new SchemaRepository(); + _schemaGenerator.GenerateSchema(type, throwawayRepository); + if (throwawayRepository.Schemas.TryGetValue(schemaId, out var concrete) + && concrete is OpenApiSchema concreteSchema + && concreteSchema.Properties is { Count: > 0 }) + { + schema = concreteSchema; + } + } + return schema; } diff --git a/test/MicroElements.Swashbuckle.FluentValidation.Tests/SharedDtoAcrossEndpointsTest.cs b/test/MicroElements.Swashbuckle.FluentValidation.Tests/SharedDtoAcrossEndpointsTest.cs new file mode 100644 index 0000000..938b557 --- /dev/null +++ b/test/MicroElements.Swashbuckle.FluentValidation.Tests/SharedDtoAcrossEndpointsTest.cs @@ -0,0 +1,132 @@ +// Copyright (c) MicroElements. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Generic; +using FluentAssertions; +using FluentValidation; +using MicroElements.OpenApi.FluentValidation; +using MicroElements.Swashbuckle.FluentValidation.Generation; +using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Options; +#if OPENAPI_V2 +using Microsoft.OpenApi; +#else +using Microsoft.OpenApi.Models; +#endif +using Swashbuckle.AspNetCore.SwaggerGen; +using Xunit; + +namespace MicroElements.Swashbuckle.FluentValidation.Tests +{ + /// + /// Issue #223: when the same [FromQuery] DTO is bound by more than one endpoint, only the FIRST + /// operation receives the FluentValidation rules; the rest lose them. + /// + /// Root cause: the Issue #180 cleanup removes the container schema from + /// but NOT from Swashbuckle's internal reserved-ids map. Because + /// runs once per operation, the second operation's GetSchemaForType hits the reserved-but-removed type and + /// gets back a bare $ref schema (no Properties), so the rule-application guard is skipped. + /// + /// Repro requires RemoveUnusedQuerySchemas = true (the default) AND a SchemaRepository shared across + /// operations — exactly how the real Swagger pipeline works. + /// https://github.com/micro-elements/MicroElements.Swashbuckle.FluentValidation/issues/223 + /// + public class SharedDtoAcrossEndpointsTest : UnitTestBase + { + public class HelloRequest + { + public string? Name { get; set; } + } + + public class HelloRequestValidator : AbstractValidator + { + public HelloRequestValidator() + { + RuleFor(x => x.Name).NotEmpty().MaximumLength(10); + } + } + +// OperationFilter integration tests require framework-specific OpenApi types (excluded on net10.0/OPENAPI_V2). +#if !OPENAPI_V2 + [Fact] + public void OperationFilter_Should_Apply_Rules_To_Every_Endpoint_Sharing_The_Same_FromQuery_Dto() + { + // Arrange — a SINGLE SchemaRepository + SchemaGenerator shared across both operations, + // exactly like the real Swagger pipeline where SchemaRepository persists for the whole document. + var schemaGeneratorOptions = new SchemaGeneratorOptions(); + var schemaRepository = new SchemaRepository(); + var schemaGenerator = SchemaGenerator(new HelloRequestValidator()); + + var schemaGenerationOptions = new SchemaGenerationOptions + { + NameResolver = new SystemTextJsonNameResolver(), + SchemaIdSelector = schemaGeneratorOptions.SchemaIdSelector, + RemoveUnusedQuerySchemas = true, // the default; this is the flag that triggers the bug + }; + + var validatorRegistry = new ValidatorRegistry( + new IValidator[] { new HelloRequestValidator() }, + new OptionsWrapper(schemaGenerationOptions)); + + var operationFilter = new FluentValidationOperationFilter( + validatorRegistry: validatorRegistry, + schemaGenerationOptions: new OptionsWrapper(schemaGenerationOptions)); + + // Act — two endpoints ("/api/hello" and "/api/hello2") binding the same HelloRequest via [FromQuery]. + var firstName = ApplyToEndpoint(operationFilter, schemaGenerator, schemaRepository); + var secondName = ApplyToEndpoint(operationFilter, schemaGenerator, schemaRepository); + + // Assert — the FIRST endpoint gets the rules today... + firstName.MinLength.Should().Be(1, because: "the first endpoint has always worked"); + firstName.MaxLength.Should().Be(10, because: "the first endpoint has always worked"); + + // ...and the SECOND endpoint MUST get them too (currently FAILS — Issue #223). + secondName.MinLength.Should().Be(1, + because: "every endpoint binding the shared HelloRequest must expose NotEmpty (Issue #223)"); + secondName.MaxLength.Should().Be(10, + because: "every endpoint binding the shared HelloRequest must expose MaximumLength(10) (Issue #223)"); + } + + /// + /// Runs the for one GET endpoint that binds + /// via [FromQuery], and returns the resulting "Name" parameter schema. + /// + private static OpenApiSchema ApplyToEndpoint( + FluentValidationOperationFilter operationFilter, + SchemaGenerator schemaGenerator, + SchemaRepository schemaRepository) + { + var metadataProvider = new EmptyModelMetadataProvider(); + var nameMetadata = metadataProvider.GetMetadataForProperty(typeof(HelloRequest), nameof(HelloRequest.Name)); + + var apiDescription = new ApiDescription(); + apiDescription.ParameterDescriptions.Add(new ApiParameterDescription + { + Name = "Name", + ModelMetadata = nameMetadata, + Source = BindingSource.Query, + }); + + var nameParamSchema = new OpenApiSchema { Type = "string" }; + var operation = new OpenApiOperation + { + Parameters = new List + { + new OpenApiParameter { Name = "Name", In = ParameterLocation.Query, Schema = nameParamSchema }, + }, + }; + + var context = new OperationFilterContext( + apiDescription, + schemaGenerator, + schemaRepository, + typeof(SharedDtoAcrossEndpointsTest).GetMethod(nameof(OperationFilter_Should_Apply_Rules_To_Every_Endpoint_Sharing_The_Same_FromQuery_Dto))!); + + operationFilter.Apply(operation, context); + + return nameParamSchema; + } +#endif + } +} diff --git a/version.props b/version.props index 54df8a4..ac108b0 100644 --- a/version.props +++ b/version.props @@ -1,6 +1,6 @@ - 7.1.9 + 7.1.10