diff --git a/CHANGELOG.md b/CHANGELOG.md index 942c85a..a6fd78d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# Changes in 7.1.3 +- Fixed: `$ref` replaced with inline schema copy when using `SetValidator` with nested object types (Issue #198) + - `ResolveRefProperty` (introduced in 7.1.2 for BigInteger isolation) replaced all `$ref` properties with copies, destroying reference structure in the OpenAPI document + - Fix: snapshot `$ref` properties before rule application, restore them afterwards if no validation constraints were added by rules + - BigInteger per-model constraints (Issue #146) continue to work correctly + # Changes in 7.1.2 - Added: `BigInteger` support for min/max validation constraints in OpenAPI schema generation (Issue #146) - `IsNumeric()` and `NumericToDecimal()` now handle `BigInteger` values diff --git a/src/MicroElements.Swashbuckle.FluentValidation/OpenApi/OpenApiSchemaCompatibility.cs b/src/MicroElements.Swashbuckle.FluentValidation/OpenApi/OpenApiSchemaCompatibility.cs index 0c82b5c..d6aead7 100644 --- a/src/MicroElements.Swashbuckle.FluentValidation/OpenApi/OpenApiSchemaCompatibility.cs +++ b/src/MicroElements.Swashbuckle.FluentValidation/OpenApi/OpenApiSchemaCompatibility.cs @@ -253,6 +253,8 @@ public static bool TryGetProperty(OpenApiSchema schema, string key, out OpenApiS /// Resolves a $ref property, replaces it with an isolated shallow copy in the parent schema, /// and returns the copy. If the property is already a concrete OpenApiSchema, returns it as-is. /// This prevents validation rules from mutating the shared schema in SchemaRepository. + /// After rule application, call to restore $refs + /// for properties that were not modified by rules (Issue #198). /// public static OpenApiSchema? ResolveRefProperty(OpenApiSchema schema, string key, SchemaRepository? repository) { @@ -275,6 +277,98 @@ public static bool TryGetProperty(OpenApiSchema schema, string key, out OpenApiS return null; } + + /// + /// Snapshots all $ref properties in the schema before rule application. + /// Returns a dictionary of property key to original OpenApiSchemaReference. + /// + public static Dictionary? SnapshotRefs(OpenApiSchema schema) + { + if (schema.Properties == null) + return null; + + Dictionary? snapshot = null; + foreach (var kvp in schema.Properties) + { + if (kvp.Value is OpenApiSchemaReference schemaRef) + { + snapshot ??= new Dictionary(); + snapshot[kvp.Key] = schemaRef; + } + } + + return snapshot; + } + + /// + /// Restores $ref properties that were replaced by ResolveRefProperty but not meaningfully + /// modified by validation rules. Compares the inline copy against the original component + /// schema to detect changes. Issue #198. + /// + public static void RestoreUnmodifiedRefs( + OpenApiSchema schema, + Dictionary? snapshot, + SchemaRepository? repository) + { + if (snapshot == null || repository == null || schema.Properties == null) + return; + + foreach (var kvp in snapshot) + { + var key = kvp.Key; + var originalRef = kvp.Value; + + if (!schema.Properties.TryGetValue(key, out var currentProp)) + continue; + + // If still a ref, nothing was replaced + if (currentProp is OpenApiSchemaReference) + continue; + + if (currentProp is OpenApiSchema inlineCopy) + { + var refId = originalRef.Reference?.Id; + if (refId != null && repository.Schemas.TryGetValue(refId, out var resolved) && resolved is OpenApiSchema componentSchema) + { + if (!HasValidationConstraintChanges(inlineCopy, componentSchema)) + { + schema.Properties[key] = originalRef; + } + } + } + } + } + + /// + /// Checks if the inline copy has any validation constraint differences compared to the component schema. + /// + private static bool HasValidationConstraintChanges(OpenApiSchema copy, OpenApiSchema original) + { + if (copy.MinLength != original.MinLength) return true; + if (copy.MaxLength != original.MaxLength) return true; + if (copy.MinItems != original.MinItems) return true; + if (copy.MaxItems != original.MaxItems) return true; + if (copy.Pattern != original.Pattern) return true; + if (copy.Minimum != original.Minimum) return true; + if (copy.Maximum != original.Maximum) return true; + if (copy.ExclusiveMinimum != original.ExclusiveMinimum) return true; + if (copy.ExclusiveMaximum != original.ExclusiveMaximum) return true; + if (copy.Type != original.Type) return true; + if (copy.Format != original.Format) return true; + + // Check Required collection changes + var copyReq = copy.Required; + var origReq = original.Required; + if (copyReq?.Count != origReq?.Count) return true; + if (copyReq != null && origReq != null && !copyReq.SetEquals(origReq)) return true; + + // Check AllOf collection changes (Pattern rule with UseAllOfForMultipleRules) + var copyAllOfCount = copy.AllOf?.Count ?? 0; + var origAllOfCount = original.AllOf?.Count ?? 0; + if (copyAllOfCount != origAllOfCount) return true; + + return false; + } #endif /// diff --git a/src/MicroElements.Swashbuckle.FluentValidation/Swashbuckle/FluentValidationRules.cs b/src/MicroElements.Swashbuckle.FluentValidation/Swashbuckle/FluentValidationRules.cs index 02bb407..5c4200e 100644 --- a/src/MicroElements.Swashbuckle.FluentValidation/Swashbuckle/FluentValidationRules.cs +++ b/src/MicroElements.Swashbuckle.FluentValidation/Swashbuckle/FluentValidationRules.cs @@ -107,6 +107,13 @@ public void Apply(OpenApiSchema? schema, SchemaFilterContext context) { foreach (var oneOfSchemas in allSchemas) { +#if OPENAPI_V2 + // Issue #198: Snapshot $ref properties before rule application. + // ResolveRefProperty replaces $refs with copies for mutation isolation (Issue #146), + // but we restore unmodified refs afterwards to preserve $ref structure. + var refSnapshot = OpenApiSchemaCompatibility.SnapshotRefs(oneOfSchemas); +#endif + var validatorContext = new ValidatorContext(typeContext, validator); var schemaContext = new SchemaGenerationContext( schemaRepository: context.SchemaRepository, @@ -126,6 +133,11 @@ public void Apply(OpenApiSchema? schema, SchemaFilterContext context) { _logger.LogWarning(0, e, "Applying IncludeRules for type '{ModelType}' failed", context.Type); } + +#if OPENAPI_V2 + // Issue #198: Restore $refs for properties that were not meaningfully modified by rules. + OpenApiSchemaCompatibility.RestoreUnmodifiedRefs(oneOfSchemas, refSnapshot, context.SchemaRepository); +#endif } } } diff --git a/test/MicroElements.Swashbuckle.FluentValidation.Tests/SchemaReferenceResolutionTests.cs b/test/MicroElements.Swashbuckle.FluentValidation.Tests/SchemaReferenceResolutionTests.cs index c5b3973..d9ae91d 100644 --- a/test/MicroElements.Swashbuckle.FluentValidation.Tests/SchemaReferenceResolutionTests.cs +++ b/test/MicroElements.Swashbuckle.FluentValidation.Tests/SchemaReferenceResolutionTests.cs @@ -123,6 +123,101 @@ public ModelBValidator() } } + /// + /// Issue #198: SetValidator with nested object type should preserve $ref in parent schema. + /// https://github.com/micro-elements/MicroElements.Swashbuckle.FluentValidation/issues/198 + /// + public class PersonModel + { + public AddressModel Address { get; set; } + } + + public class AddressModel + { + public string Street { get; set; } + } + + public class AddressModelValidator : AbstractValidator + { + public AddressModelValidator() + { + RuleFor(x => x.Street).NotEmpty(); + } + } + + public class PersonModelValidator : AbstractValidator + { + public PersonModelValidator() + { + RuleFor(x => x.Address) + .NotEmpty() + .SetValidator(new AddressModelValidator()); + } + } + + [Fact] + public void SetValidator_Should_Preserve_Ref_For_Nested_Object() + { + // Arrange + var schemaRepository = new SchemaRepository(); + var schemaGenerator = SchemaGenerator(new PersonModelValidator(), new AddressModelValidator()); + + // Act + var referenceSchema = schemaGenerator.GenerateSchema(typeof(PersonModel), schemaRepository); + var personSchema = schemaRepository.GetSchema(referenceSchema.GetRefId()!); + + // Assert: Person schema should have "Address" in required + personSchema.Required.Should().Contain("Address"); + + // Assert: Address component schema should exist and have street constraints + schemaRepository.Schemas.Should().ContainKey("AddressModel"); + + // Assert: Person.properties["Address"] should remain a $ref, not an inline copy + var addressProp = personSchema.Properties["Address"]; +#if OPENAPI_V2 + addressProp.Should().BeOfType( + "Person.properties['address'] should be a $ref, not an inline copy of the Address schema"); +#else + addressProp.Reference.Should().NotBeNull( + "Person.properties['address'] should be a $ref, not an inline copy of the Address schema"); +#endif + } + + /// + /// Issue #198 follow-up: when a $ref property IS modified by a validation rule + /// (e.g., Email sets Format), the $ref should NOT be restored — the inline copy + /// with constraints must be kept. + /// + public class ModelWithEmail + { + public string ContactEmail { get; set; } + } + + public class ModelWithEmailValidator : AbstractValidator + { + public ModelWithEmailValidator() + { + RuleFor(x => x.ContactEmail).NotEmpty().EmailAddress(); + } + } + + [Fact] + public void Ref_Property_With_Constraint_Should_Not_Be_Restored() + { + // Arrange + var schemaRepository = new SchemaRepository(); + var schemaGenerator = SchemaGenerator(new ModelWithEmailValidator()); + + // Act + var referenceSchema = schemaGenerator.GenerateSchema(typeof(ModelWithEmail), schemaRepository); + var schema = schemaRepository.GetSchema(referenceSchema.GetRefId()!); + + // Assert: ContactEmail should have format=email applied (not discarded by ref restore) + var emailProp = schema.GetProperty("ContactEmail")!; + emailProp.Format.Should().Be("email", + "Email rule should set Format on the property, and it must not be discarded by $ref restoration"); + } + [Fact] public void SharedRef_Should_Not_Corrupt_Between_Models() { diff --git a/version.props b/version.props index f9cd927..9342ff1 100644 --- a/version.props +++ b/version.props @@ -1,6 +1,6 @@ - 7.1.2 + 7.1.3