Add MicroElements.AspNetCore.OpenApi.FluentValidation package (Issue #149)

[avgalex]

Summary

• New package MicroElements.AspNetCore.OpenApi.FluentValidation integrating FluentValidation with Microsoft.AspNetCore.OpenApi (IOpenApiSchemaTransformer)
• Targets .NET 9 and .NET 10, no dependency on Swashbuckle
• Supports all FluentValidation rule types: Required, NotEmpty, Length, Pattern, Email, Comparison, Between
• Handles AllOf/OneOf/AnyOf sub-schemas for polymorphic/inheritance models
• Version bumped to 7.1.0-beta.1

New files

• src/MicroElements.AspNetCore.OpenApi.FluentValidation/ — new package (22 files)
• samples/SampleAspNetCoreOpenApi/ — sample Minimal API project
• docs/adr/ADR-001-aspnetcore-openapi-support.md — architecture decision record

User-facing API

```csharp
// Register services
services.AddFluentValidationRulesToOpenApi();

// Add transformer to OpenApi
services.AddOpenApi(options => options.AddFluentValidationRules());
```

Migration from Swashbuckle

```diff

• MicroElements.Swashbuckle.FluentValidation

• MicroElements.AspNetCore.OpenApi.FluentValidation

• services.AddFluentValidationRulesToSwagger();

• services.AddFluentValidationRulesToOpenApi();
• services.AddOpenApi(options => options.AddFluentValidationRules());
  ```

Test plan

• `dotnet build` — 0 errors
• `dotnet test` — all existing tests pass (54 net8.0, 54 net9.0, 51 net10.0)
• Sample app verified: all validation constraints appear in OpenAPI JSON
• No transitive Swashbuckle dependency
• Code reviewed: AllOf/OneOf/AnyOf traversal, sync-over-async safety

Related to #149

🤖 Generated with Claude Code

[avgalex]

@claude review this PR

[claude]

Claude finished @avgalex's task in 3m 40s —— View job

---

PR Review: Add MicroElements.AspNetCore.OpenApi.FluentValidation package

• Gather context and read changed files
• Review project structure and configuration
• Review core implementation files
• Review API surface and DI extensions
• Review OpenAPI compatibility layer
• Review sample project
• Compile and post final review

---

Overall Assessment

This is a well-structured addition that fills a real gap (.NET 9+ users migrating away from Swashbuckle). The architecture is clean, the DI registration is idiomatic, and the #if OPENAPI_V2 approach for handling Microsoft.OpenApi 1.x vs 2.x differences is the right call. A few issues need attention before merging.

---

Bugs / Issues

1. Potential NullReferenceException in ReflectionDependencyInjectionExtensions

Assembly.FullName can return null for dynamic/transient assemblies in .NET. Line 24 dereferences it unconditionally:

// ReflectionDependencyInjectionExtensions.cs:24
.Where(assembly => assembly.FullName.Contains("Microsoft"))

Fix: assembly.FullName?.Contains("Microsoft") == true

---

2. Silent exception swallowing in AspNetCoreSchemaProvider

// AspNetCoreSchemaProvider.cs:48-51
catch (Exception)
{
    // Fallback to empty schema if sub-schema resolution fails.
    return new OpenApiSchema();
}

All exceptions are silently swallowed. If GetOrCreateSchemaAsync fails for a non-obvious reason (e.g., unregistered type, scoping issue), the user gets no signal. At minimum, the caught exception should be logged at Debug or Warning level. The AspNetCoreSchemaProvider currently has no access to a logger, but one could be passed via constructor injection or a LoggerFactory parameter (similar to how FluentValidationSchemaTransformer accepts it). Fix this →

---

Design / Performance Concerns

3. Reflection-heavy GetByFullName called on every transient resolution

GetByFullName scans ALL assemblies in the AppDomain on every invocation. In ServiceCollectionExtensions.cs, this is wired into TryAddTransient factories:

// ServiceCollectionExtensions.cs:39
services.TryAddTransient<AspNetJsonSerializerOptions>(
    provider => new AspNetJsonSerializerOptions(provider.GetJsonSerializerOptionsOrDefault()));

Since GetJsonSerializerOptionsOrDefault calls GetJsonSerializerOptions which calls GetByFullName, this AppDomain scan runs on every request that resolves AspNetJsonSerializerOptions. The result of GetByFullName("Microsoft.AspNetCore.Mvc.JsonOptions") should be cached in a static readonly field. Fix this →

---

4. Dead code: ConfigureJsonOptionsForAspNetCore is never called

ReflectionDependencyInjectionExtensions.ConfigureJsonOptionsForAspNetCore (line 52) has no callers in this package — it appears to have been copied from the Swashbuckle package but unused here. It adds to the compiled surface area of an internal class, which is fine, but the dead code could cause confusion during future maintenance. Worth either removing or marking with a // Reserved for future use comment.

---

5. JsonSerializerOptions registered as a bare service

// ServiceCollectionExtensions.cs:40
services.TryAddTransient<JsonSerializerOptions>(
    provider => provider.GetService<AspNetJsonSerializerOptions>()?.Value!);

Registering JsonSerializerOptions directly (not behind a named options or typed wrapper) can conflict with other libraries that do the same. This isn't necessarily wrong since TryAddTransient won't override an existing registration, but if another library registers it first with different settings, SystemTextJsonNameResolver will silently use those. Consider documenting this assumption or considering whether direct JsonSerializerOptions registration is necessary given that AspNetJsonSerializerOptions already wraps it.

---

6. Shallow ProcessAllSchemas traversal

// FluentValidationSchemaTransformer.cs:137-159
private static void ProcessAllSchemas(OpenApiSchema schema, List<OpenApiSchema> schemas)

The method collects AllOf/OneOf/AnyOf schemas one level deep. For inheritance chains deeper than one level (e.g., A → B → C), inner schemas won't be processed. This may be acceptable as a v7.1 limitation, but it should be documented alongside the existing .NET 9 nested validator limitation in the ADR and XML docs.

---

7. Exact version pinning for Microsoft.AspNetCore.OpenApi

<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="9.0.0" />
<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="10.0.0" />

Using exact versions means patch releases (bug fixes, security fixes) won't be consumed unless the package is explicitly updated. This is generally safe for FrameworkReference-backed packages, but consider [9.0.0,10.0.0) for net9.0 to allow patch-level updates automatically.

---

Code Quality / Maintenance

8. ADR written in Russian

docs/adr/ADR-001-aspnetcore-openapi-support.md is written entirely in Russian. For an open-source project with international contributors, ADRs should be in English. The document is very well-structured — translating it would make it accessible to contributors who don't read Russian. Fix this →

---

9. Inconsistent namespaces for copied files

OpenApiSchemaCompatibility.cs and OpenApiExtensions.cs declare namespace MicroElements.OpenApi (matching their Swashbuckle source), while the rest of the new package uses namespace MicroElements.AspNetCore.OpenApi.FluentValidation. This inconsistency is inherited from the copy-paste approach. It won't break anything (the types are internal), but it's confusing during maintenance — especially since the ADR acknowledges these files as "copies" to be unified in Phase 2 (v7.2). A comment at the top of each copied file explaining the copy origin and the Phase 2 plan would help.

---

10. No dedicated tests for the new package

The PR notes that existing tests (54/54/51 across net8.0/net9.0/net10.0) all pass, but those tests cover the Swashbuckle package. There are no tests for FluentValidationSchemaTransformer, AspNetCoreSchemaGenerationContext, AspNetCoreSchemaProvider, or the DI extension methods. The sample app serves as a manual smoke test, but automated tests for the new package are important — especially given the acknowledged code duplication where bug fixes need to be applied to both packages.

---

Minor Notes

• FluentValidationSchemaTransformer constructor: all parameters have = null defaults, but validatorRegistry immediately throws ArgumentNullException if null (line 58). The defaults suggest the type is usable without DI, but that's inconsistent. The defaults are fine for DI (where the framework injects nulls for unregistered optional services), but a comment noting this intent would help.
• OpenApiRuleContext.Property getter throws ApplicationException when a property isn't found in the schema. This is inherited from Swashbuckle. A log-and-skip approach would be more resilient for cases where a validator covers properties not present in the schema.
• The sample project is well-done — covers all rule types and [AsParameters]. Good addition.

---

Summary: The core approach is sound. Before merging, the most important items are: (3) the per-request AppDomain scan performance issue, (2) the silent exception swallowing in the schema provider, and (1) the NullReferenceException risk. Items (8) and (10) (English ADR and dedicated tests) are also strongly recommended.
| Branch: feature/aspnetcore-openapi-support

[avgalex]

@claude review Fix CI review findings for AspNetCore.OpenApi package

[claude]

Claude finished @avgalex's task in 1m 40s —— View job

---

Review: CI Findings Fixes for MicroElements.AspNetCore.OpenApi.FluentValidation

• Gather context and read changed files
• Review fix for NullReferenceException in ReflectionDependencyInjectionExtensions
• Review fix for silent exception swallowing in AspNetCoreSchemaProvider
• Review fix for per-request AppDomain scan performance issue
• Review dead code removal (ConfigureJsonOptionsForAspNetCore)
• Review ADR translation to English
• Check remaining findings (namespaces, version pinning, ProcessAllSchemas, tests)
• Post final review

---

Summary

All high-priority and most medium-priority findings from the previous review have been addressed. The fixes are clean and correct. Three lower-priority findings remain open.

---

Fixed — Verified ✓

Finding 1 — NullReferenceException in ReflectionDependencyInjectionExtensions
Fixed correctly. assembly.FullName?.Contains("Microsoft") == true (line 29). The static keyword on the lambda also prevents accidental closure captures — good addition.

Finding 2 — Silent exception swallowing in AspNetCoreSchemaProvider
Fixed correctly. ILogger injected via constructor with NullLogger.Instance fallback, and _logger.LogWarning(ex, ...) is called before the fallback return. The logger is forwarded from FluentValidationSchemaTransformer via new AspNetCoreSchemaProvider(context, _logger) (line 99 of transformer).

Finding 3 — Per-request AppDomain scan performance
Fixed correctly. A static readonly ConcurrentDictionary<string, Type?> _typeCache with GetOrAdd ensures the expensive AppDomain scan happens at most once per unique type name. The cache is correct to be static — type lookups are deterministic within a process.

Minor note: The cache has no eviction/refresh mechanism. This is fine for statically-loaded assemblies, but if the application uses dynamic assembly loading (plugins), a previously-cached null for a type that was later dynamically loaded won't be refreshed. This is an acceptable edge-case trade-off for a library targeting standard ASP.NET Core apps.

Finding 4 — Dead code ConfigureJsonOptionsForAspNetCore
Removed entirely. Clean.

Finding 8 — ADR written in Russian
Translated to English. All section headers, table content, and inline text correctly translated. Structure and content preserved.

Finding 7 — Exact version pinning
Fixed. Changed to [9.0.0,10.0.0) and [10.0.0,11.0.0). This allows patch-level updates to flow through while still preventing major version jumps. Correct approach.

---

Remaining Open Findings

Finding 5 — JsonSerializerOptions registered as a bare service (not addressed)

ServiceCollectionExtensions.cs line 40 still registers JsonSerializerOptions directly:

services.TryAddTransient<JsonSerializerOptions>(
    provider => provider.GetService<AspNetJsonSerializerOptions>()?.Value!);

The TryAdd guard prevents overriding an existing registration, so this is not a breaking issue — but it remains a potential source of silent misconfiguration if another library registers JsonSerializerOptions first. Consider adding an XML doc comment on this line explaining the intent and the TryAdd guard, so future maintainers understand the registration is intentionally loose.

Finding 6 — Shallow ProcessAllSchemas traversal (not addressed)

FluentValidationSchemaTransformer.ProcessAllSchemas (line 137–160) still collects sub-schemas only one level deep. No documentation was added to the method, the ADR, or the XML docs to note this limitation. A single /// <remarks> comment on the method noting "traversal is one level deep; deeper inheritance chains (A → B → C) are not supported in v7.1" would be sufficient to close this finding.

Finding 9 — Inconsistent namespaces on copied files (not addressed)

OpenApiSchemaCompatibility.cs and OpenApiExtensions.cs still declare namespace MicroElements.OpenApi while the rest of the package uses namespace MicroElements.AspNetCore.OpenApi.FluentValidation. No explanatory comment was added. As noted in the original review this won't break anything (both are internal), but a single-line comment at the top of each file (e.g., // Copied from Swashbuckle package; namespace preserved for Phase 2 (v7.2) unification.) would prevent future confusion.

Finding 10 — No dedicated tests (not addressed)

No tests for the new package were added in this commit. This remains the most significant open gap — the Swashbuckle tests don't exercise FluentValidationSchemaTransformer, AspNetCoreSchemaProvider, or the DI extensions. Given the acknowledged duplication, automated tests are important to catch regressions when bug fixes are applied to both packages.

---

Verdict

The three critical fixes (NullRef, exception swallowing, AppDomain scan) are correctly implemented. The PR is in substantially better shape. The remaining open items (5, 6, 9) are documentation/comment-level issues that are low risk; finding 10 (tests) is the one item worth tracking as a follow-up issue before general availability.

---

| Branch: feature/aspnetcore-openapi-support

[avgalex]

@claude review Fix remaining CI review findings (5, 6, 9) for AspNetCore.OpenApi package

[claude]

Claude finished @avgalex's task in 50s —— View job

---

Review: Fix Remaining CI Findings (5, 6, 9) for AspNetCore.OpenApi

• Gather context
• Review fix for Finding 5 — JsonSerializerOptions bare service registration
• Review fix for Finding 6 — Shallow ProcessAllSchemas traversal
• Review fix for Finding 9 — Inconsistent namespaces on copied files
• Post final review

---

All three remaining findings have been addressed. The changes are minimal and correct.

---

Fixed — Verified ✓

Finding 5 — JsonSerializerOptions bare service registration

A three-line comment was added above the TryAddTransient<JsonSerializerOptions> call in ServiceCollectionExtensions.cs:

// TryAdd ensures we only register a bare JsonSerializerOptions fallback when the host
// hasn't already registered one (e.g. via AddControllers or AddJsonOptions).
// This keeps the library non-intrusive while still providing a default for minimal APIs.

The comment accurately describes the intent and the TryAdd guard semantics. Future maintainers will understand why the registration is intentionally loose. Finding closed.

Finding 6 — Shallow ProcessAllSchemas traversal

An XML doc comment with a <remarks> block was added to FluentValidationSchemaTransformer.ProcessAllSchemas:

/// Traversal is one level deep: only direct children of AllOf/OneOf/AnyOf that contain
/// properties are included. Deeply nested composition hierarchies are not recursed.
/// This is sufficient for the polymorphic/inheritance models produced by Microsoft.AspNetCore.OpenApi
/// but may need to be extended if deeper nesting is encountered.

The remarks are accurate and clearly signal the known limitation and when an extension would be needed. Finding closed.

Finding 9 — Inconsistent namespaces on copied files

Both OpenApiExtensions.cs and OpenApiSchemaCompatibility.cs now have the same explanatory line immediately after the copyright header:

// Copied from Swashbuckle package; namespace preserved for Phase 2 (v7.2) unification.

This precisely matches the suggested wording from the original review and correctly explains both the copy origin and the Phase 2 plan. Finding closed.

---

Outstanding Items

Finding 10 — No dedicated tests remains unaddressed, as it was from the previous review. This is the one item worth tracking. Recommend opening a follow-up issue to add automated tests for FluentValidationSchemaTransformer, AspNetCoreSchemaProvider, and the DI extensions before the v7.1 GA release.

---

Verdict

All documentation/comment-level findings (5, 6, 9) are correctly resolved. The PR is ready to merge modulo the tracked follow-up for tests (Finding 10). The core implementation, DI registration, and critical bug fixes are all in good shape.
| Branch: feature/aspnetcore-openapi-support