From 9013a670688eb2f49a156227dcc90a8440fe512e Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sun, 24 May 2026 14:34:54 +0200 Subject: [PATCH 1/6] feat(core): add Form Request classes for typed binding + validation per endpoint (#163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces Laravel-style FormRequest base class that bundles parameter binding, authorization, validation rules, and data normalization into a single class. The endpoint handler receives an already-valid request object. Pipeline: Bind → Authorize (403) → Prepare → Validate (422) → Handler - FormRequest base + [FormRequest] attribute in SimpleModule.Core - Source generator discovers [FormRequest] types, validates shape (SM0056/SM0057), emits TypeScript interfaces (same path as [Dto]) - FormRequestEndpointFilter auto-applied to all module route groups - FluentValidation under the hood via RuleConfigurator - Validator cached per type for performance - 422 Unprocessable Entity with RFC 7807 problem+json for validation failures - Email module CreateTemplateEndpoint refactored as reference implementation - 16 new xUnit tests covering binding, validation, authorization, and prepare hooks - Constitution updated with FormRequest rules and SM0056/SM0057 diagnostics --- docs/CONSTITUTION.md | 57 ++++ .../FormRequests/FormRequest.cs | 14 + .../FormRequests/FormRequestAttribute.cs | 6 + .../FormRequests/FormRequestEndpointFilter.cs | 96 +++++++ .../FormRequests/FormRequestExtensions.cs | 14 + .../FormRequests/FormRequest{T}.cs | 31 +++ .../FormRequests/RuleConfigurator.cs | 20 ++ .../AnalyzerReleases.Unshipped.md | 2 + .../Discovery/CoreSymbols.cs | 8 + .../Discovery/DiscoveryData.cs | 4 + .../Discovery/DiscoveryDataBuilder.cs | 9 + .../Discovery/Finders/DtoFinder.cs | 22 ++ .../Discovery/Finders/FormRequestFinder.cs | 101 +++++++ .../Discovery/Records/FormRequestRecords.cs | 8 + .../Discovery/Records/WorkingTypes.cs | 8 + .../Discovery/SymbolDiscovery.cs | 13 +- .../Emitters/DiagnosticEmitter.cs | 1 + .../DiagnosticDescriptors.FormRequest.cs | 24 ++ .../Emitters/Diagnostics/FormRequestChecks.cs | 34 +++ .../Emitters/EndpointExtensionsEmitter.cs | 3 +- .../Templates/CreateTemplateEndpoint.cs | 39 ++- .../FormRequests/CreateTemplateFormRequest.cs | 64 +++++ modules/Email/src/SimpleModule.Email/types.ts | 9 + .../FormRequestEndpointFilterTests.cs | 262 ++++++++++++++++++ .../FormRequestValidationTests.cs | 125 +++++++++ .../TopologicalSortTests.cs | 3 + 26 files changed, 954 insertions(+), 23 deletions(-) create mode 100644 framework/SimpleModule.Core/FormRequests/FormRequest.cs create mode 100644 framework/SimpleModule.Core/FormRequests/FormRequestAttribute.cs create mode 100644 framework/SimpleModule.Core/FormRequests/FormRequestEndpointFilter.cs create mode 100644 framework/SimpleModule.Core/FormRequests/FormRequestExtensions.cs create mode 100644 framework/SimpleModule.Core/FormRequests/FormRequest{T}.cs create mode 100644 framework/SimpleModule.Core/FormRequests/RuleConfigurator.cs create mode 100644 framework/SimpleModule.Generator/Discovery/Finders/FormRequestFinder.cs create mode 100644 framework/SimpleModule.Generator/Discovery/Records/FormRequestRecords.cs create mode 100644 framework/SimpleModule.Generator/Emitters/Diagnostics/DiagnosticDescriptors.FormRequest.cs create mode 100644 framework/SimpleModule.Generator/Emitters/Diagnostics/FormRequestChecks.cs create mode 100644 modules/Email/src/SimpleModule.Email/FormRequests/CreateTemplateFormRequest.cs create mode 100644 tests/SimpleModule.Core.Tests/FormRequests/FormRequestEndpointFilterTests.cs create mode 100644 tests/SimpleModule.Core.Tests/FormRequests/FormRequestValidationTests.cs diff --git a/docs/CONSTITUTION.md b/docs/CONSTITUTION.md index 5321b7b2..c22dcc2f 100644 --- a/docs/CONSTITUTION.md +++ b/docs/CONSTITUTION.md @@ -258,6 +258,56 @@ This is cosmetic organization -- all modules share one connection. Override `ConfigureEndpoints` on the module class for non-standard routes. +### Form Requests + +Form Requests bundle parameter binding, authorization, validation, and data normalization into a single class. The handler receives an already-valid request object. + +```csharp +[FormRequest] +public sealed class CreateProductRequest : FormRequest +{ + public string Name { get; set; } = ""; + public decimal Price { get; set; } + + public override bool Authorize(ClaimsPrincipal user) + => user.HasPermission("Products.Create"); + + public override void Prepare() + { + Name = Name.Trim(); + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty().MaximumLength(200); + rules.RuleFor(x => x.Price).GreaterThan(0); + } +} +``` + +**Pipeline:** `Bind → Authorize → Prepare → Validate → Handler` + +- `Authorize` returns `false` → **403 Forbidden** (short-circuit) +- `Prepare` normalizes data before validation runs +- Validation fails → **422 Unprocessable Entity** with RFC 7807 problem+json: + +```json +{ + "title": "Validation Error", + "status": 422, + "detail": "One or more validation errors occurred.", + "errors": { "Name": ["'Name' must not be empty."] } +} +``` + +**Rules:** +- FormRequest classes **must be sealed** (SM0056) +- FormRequest classes **must extend `FormRequest`** (SM0057) +- `[FormRequest]` types get TypeScript interfaces auto-generated (same as `[Dto]`) +- The filter runs on all module route groups automatically +- Existing endpoints using `IValidator` + manual validation are unaffected (opt-in) +- FluentValidation is used under the hood — `RuleFor()` API is standard FluentValidation + --- ## 7. Frontend @@ -478,6 +528,13 @@ All SM diagnostics are emitted by the Roslyn source generator at compile time. ` | SM0049 | Error | Each endpoint must be in its own file | | SM0054 | Info | Endpoint should declare a `public const string Route` field | +### Form Requests + +| Diagnostic | Severity | Rule | +|------------|----------|------| +| SM0056 | Error | FormRequest class must be sealed | +| SM0057 | Error | FormRequest class must extend `FormRequest` | + ### Module Metadata | Diagnostic | Severity | Rule | diff --git a/framework/SimpleModule.Core/FormRequests/FormRequest.cs b/framework/SimpleModule.Core/FormRequests/FormRequest.cs new file mode 100644 index 00000000..6834c2f3 --- /dev/null +++ b/framework/SimpleModule.Core/FormRequests/FormRequest.cs @@ -0,0 +1,14 @@ +using System.Security.Claims; + +namespace SimpleModule.Core.FormRequests; + +public abstract class FormRequest +{ + public virtual bool Authorize(ClaimsPrincipal user) => true; + + public virtual void Prepare() { } + + internal abstract Task ValidateAsync( + CancellationToken cancellationToken + ); +} diff --git a/framework/SimpleModule.Core/FormRequests/FormRequestAttribute.cs b/framework/SimpleModule.Core/FormRequests/FormRequestAttribute.cs new file mode 100644 index 00000000..0473441b --- /dev/null +++ b/framework/SimpleModule.Core/FormRequests/FormRequestAttribute.cs @@ -0,0 +1,6 @@ +using System; + +namespace SimpleModule.Core.FormRequests; + +[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)] +public sealed class FormRequestAttribute : Attribute { } diff --git a/framework/SimpleModule.Core/FormRequests/FormRequestEndpointFilter.cs b/framework/SimpleModule.Core/FormRequests/FormRequestEndpointFilter.cs new file mode 100644 index 00000000..c8daf187 --- /dev/null +++ b/framework/SimpleModule.Core/FormRequests/FormRequestEndpointFilter.cs @@ -0,0 +1,96 @@ +using System.Text.Json; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using SimpleModule.Core.Constants; +using SimpleModule.Core.Inertia; +using SimpleModule.Core.Validation; + +namespace SimpleModule.Core.FormRequests; + +public sealed class FormRequestEndpointFilter : IEndpointFilter +{ + private static readonly JsonSerializerOptions InertiaJsonOptions = new() + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }; + + public async ValueTask InvokeAsync( + EndpointFilterInvocationContext context, + EndpointFilterDelegate next + ) + { + for (var i = 0; i < context.Arguments.Count; i++) + { + if (context.Arguments[i] is not FormRequest formRequest) + continue; + + if (!formRequest.Authorize(context.HttpContext.User)) + { + return Results.Problem( + statusCode: StatusCodes.Status403Forbidden, + title: ErrorMessages.ForbiddenTitle, + detail: ErrorMessages.DefaultForbiddenMessage + ); + } + + formRequest.Prepare(); + + var result = await formRequest.ValidateAsync(context.HttpContext.RequestAborted); + if (!result.IsValid) + { + var errors = result.ToValidationErrors(); + + if (context.HttpContext.Request.IsInertia()) + { + return WriteInertiaValidationError(context.HttpContext, errors); + } + + return Results.Problem( + statusCode: StatusCodes.Status422UnprocessableEntity, + title: ErrorMessages.ValidationErrorTitle, + detail: ErrorMessages.DefaultValidationMessage, + extensions: new Dictionary { ["errors"] = errors } + ); + } + } + + return await next(context); + } + + private static InertiaValidationResult WriteInertiaValidationError( + HttpContext httpContext, + Dictionary errors + ) + { + var component = "Error/422"; + var pageData = new + { + component, + props = new + { + status = 422, + title = ErrorMessages.ValidationErrorTitle, + message = ErrorMessages.DefaultValidationMessage, + errors, + }, + url = httpContext.Request.Path + httpContext.Request.QueryString, + version = InertiaMiddleware.Version, + }; + + return new InertiaValidationResult(pageData); + } + + private sealed class InertiaValidationResult(object pageData) : IResult + { + public async Task ExecuteAsync(HttpContext httpContext) + { + httpContext.Response.StatusCode = StatusCodes.Status422UnprocessableEntity; + httpContext.Response.Headers[InertiaHttpExtensions.InertiaHeader] = "true"; + httpContext.Response.Headers["Vary"] = InertiaHttpExtensions.InertiaHeader; + httpContext.Response.ContentType = "application/json"; + await httpContext.Response.WriteAsync( + JsonSerializer.Serialize(pageData, InertiaJsonOptions) + ); + } + } +} diff --git a/framework/SimpleModule.Core/FormRequests/FormRequestExtensions.cs b/framework/SimpleModule.Core/FormRequests/FormRequestExtensions.cs new file mode 100644 index 00000000..10711773 --- /dev/null +++ b/framework/SimpleModule.Core/FormRequests/FormRequestExtensions.cs @@ -0,0 +1,14 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; + +namespace SimpleModule.Core.FormRequests; + +public static class FormRequestExtensions +{ + public static RouteGroupBuilder AddFormRequestFilter(this RouteGroupBuilder group) + { + EndpointFilterExtensions.AddEndpointFilter(group); + return group; + } +} diff --git a/framework/SimpleModule.Core/FormRequests/FormRequest{T}.cs b/framework/SimpleModule.Core/FormRequests/FormRequest{T}.cs new file mode 100644 index 00000000..6f360b3c --- /dev/null +++ b/framework/SimpleModule.Core/FormRequests/FormRequest{T}.cs @@ -0,0 +1,31 @@ +using System.Collections.Concurrent; +using FluentValidation; +using FluentValidation.Results; + +namespace SimpleModule.Core.FormRequests; + +public abstract class FormRequest : FormRequest + where TSelf : FormRequest +{ + private static readonly ConcurrentDictionary> ValidatorCache = + new(); + + protected abstract void ConfigureRules(RuleConfigurator rules); + + internal sealed override async Task ValidateAsync( + CancellationToken cancellationToken + ) + { + var validator = ValidatorCache.GetOrAdd( + typeof(TSelf), + _ => + { + var configurator = new RuleConfigurator(); + ConfigureRules(configurator); + return configurator.Build(); + } + ); + + return await validator.ValidateAsync((TSelf)this, cancellationToken); + } +} diff --git a/framework/SimpleModule.Core/FormRequests/RuleConfigurator.cs b/framework/SimpleModule.Core/FormRequests/RuleConfigurator.cs new file mode 100644 index 00000000..b8f82eba --- /dev/null +++ b/framework/SimpleModule.Core/FormRequests/RuleConfigurator.cs @@ -0,0 +1,20 @@ +using System.Linq.Expressions; +using FluentValidation; + +namespace SimpleModule.Core.FormRequests; + +public sealed class RuleConfigurator + where T : class +{ + private readonly InlineValidator _validator = new(); + + public IRuleBuilderInitial RuleFor( + Expression> expression + ) => _validator.RuleFor(expression); + + public IRuleBuilderInitialCollection RuleForEach( + Expression>> expression + ) => _validator.RuleForEach(expression); + + internal InlineValidator Build() => _validator; +} diff --git a/framework/SimpleModule.Generator/AnalyzerReleases.Unshipped.md b/framework/SimpleModule.Generator/AnalyzerReleases.Unshipped.md index 43efbf79..33d43bae 100644 --- a/framework/SimpleModule.Generator/AnalyzerReleases.Unshipped.md +++ b/framework/SimpleModule.Generator/AnalyzerReleases.Unshipped.md @@ -40,3 +40,5 @@ SM0052 | SimpleModule.Generator | Error | Module assembly name does not follow n SM0053 | SimpleModule.Generator | Error | Module has no matching Contracts assembly SM0054 | SimpleModule.Generator | Info | Endpoint missing Route const field SM0055 | SimpleModule.Generator | Error | Entity class must live in a Contracts assembly +SM0056 | SimpleModule.Generator | Error | FormRequest class must be sealed +SM0057 | SimpleModule.Generator | Error | FormRequest class must extend FormRequest diff --git a/framework/SimpleModule.Generator/Discovery/CoreSymbols.cs b/framework/SimpleModule.Generator/Discovery/CoreSymbols.cs index be98b6b7..b3005417 100644 --- a/framework/SimpleModule.Generator/Discovery/CoreSymbols.cs +++ b/framework/SimpleModule.Generator/Discovery/CoreSymbols.cs @@ -27,6 +27,8 @@ internal readonly record struct CoreSymbols( INamedTypeSymbol? ModuleFeatures, INamedTypeSymbol? SaveChangesInterceptor, INamedTypeSymbol? ModuleOptions, + INamedTypeSymbol? FormRequestAttribute, + INamedTypeSymbol? FormRequestBase, bool HasAgentsAssembly, bool HasRagAssembly ) @@ -80,6 +82,12 @@ bool HasRagAssembly "Microsoft.EntityFrameworkCore.Diagnostics.ISaveChangesInterceptor" ), ModuleOptions: compilation.GetTypeByMetadataName("SimpleModule.Core.IModuleOptions"), + FormRequestAttribute: compilation.GetTypeByMetadataName( + "SimpleModule.Core.FormRequests.FormRequestAttribute" + ), + FormRequestBase: compilation.GetTypeByMetadataName( + "SimpleModule.Core.FormRequests.FormRequest`1" + ), HasAgentsAssembly: compilation.GetTypeByMetadataName( "SimpleModule.Agents.SimpleModuleAgentExtensions" ) diff --git a/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs b/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs index c2323c80..cbbc651c 100644 --- a/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs +++ b/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs @@ -47,6 +47,7 @@ internal readonly record struct DiscoveryData( ImmutableArray AgentDefinitions, ImmutableArray AgentToolProviders, ImmutableArray KnowledgeSources, + ImmutableArray FormRequests, ImmutableArray ContractsAssemblyNames, bool HasAgentsAssembly, bool HasRagAssembly, @@ -79,6 +80,7 @@ string HostAssemblyName ImmutableArray.Empty, ImmutableArray.Empty, ImmutableArray.Empty, + ImmutableArray.Empty, ImmutableArray.Empty, false, false, @@ -103,6 +105,7 @@ public bool Equals(DiscoveryData other) && AgentDefinitions.SequenceEqual(other.AgentDefinitions) && AgentToolProviders.SequenceEqual(other.AgentToolProviders) && KnowledgeSources.SequenceEqual(other.KnowledgeSources) + && FormRequests.SequenceEqual(other.FormRequests) && ContractsAssemblyNames.SequenceEqual(other.ContractsAssemblyNames) && HasAgentsAssembly == other.HasAgentsAssembly && HasRagAssembly == other.HasRagAssembly @@ -128,6 +131,7 @@ public override int GetHashCode() hash = HashHelper.HashArray(hash, AgentDefinitions); hash = HashHelper.HashArray(hash, AgentToolProviders); hash = HashHelper.HashArray(hash, KnowledgeSources); + hash = HashHelper.HashArray(hash, FormRequests); hash = HashHelper.HashArray(hash, ContractsAssemblyNames); hash = HashHelper.Combine(hash, HasAgentsAssembly.GetHashCode()); hash = HashHelper.Combine(hash, HasRagAssembly.GetHashCode()); diff --git a/framework/SimpleModule.Generator/Discovery/DiscoveryDataBuilder.cs b/framework/SimpleModule.Generator/Discovery/DiscoveryDataBuilder.cs index ad2e08f5..a0f1114d 100644 --- a/framework/SimpleModule.Generator/Discovery/DiscoveryDataBuilder.cs +++ b/framework/SimpleModule.Generator/Discovery/DiscoveryDataBuilder.cs @@ -29,6 +29,7 @@ internal static DiscoveryData Build( List agentDefinitions, List agentToolProviders, List knowledgeSources, + List formRequests, Dictionary contractsAssemblyMap, bool hasAgentsAssembly, bool hasRagAssembly, @@ -174,6 +175,14 @@ string hostAssemblyName knowledgeSources .Select(k => new KnowledgeSourceRecord(k.FullyQualifiedName, k.ModuleName)) .ToImmutableArray(), + formRequests + .Select(f => new FormRequestInfoRecord( + f.FullyQualifiedName, + f.IsSealed, + f.ExtendsFormRequest, + f.Location + )) + .ToImmutableArray(), contractsAssemblyMap.Keys.ToImmutableArray(), hasAgentsAssembly, hasRagAssembly, diff --git a/framework/SimpleModule.Generator/Discovery/Finders/DtoFinder.cs b/framework/SimpleModule.Generator/Discovery/Finders/DtoFinder.cs index d03e6fb6..10b1e70e 100644 --- a/framework/SimpleModule.Generator/Discovery/Finders/DtoFinder.cs +++ b/framework/SimpleModule.Generator/Discovery/Finders/DtoFinder.cs @@ -236,5 +236,27 @@ CancellationToken cancellationToken } FindDtoTypes(hostGlobalNamespace, symbols.DtoAttribute, dtoTypes, cancellationToken); + + // [FormRequest] types also get TypeScript interfaces (same path as [Dto]) + if (symbols.FormRequestAttribute is null) + return; + + foreach (var assemblySymbol in refAssemblies) + { + cancellationToken.ThrowIfCancellationRequested(); + FindDtoTypes( + assemblySymbol.GlobalNamespace, + symbols.FormRequestAttribute, + dtoTypes, + cancellationToken + ); + } + + FindDtoTypes( + hostGlobalNamespace, + symbols.FormRequestAttribute, + dtoTypes, + cancellationToken + ); } } diff --git a/framework/SimpleModule.Generator/Discovery/Finders/FormRequestFinder.cs b/framework/SimpleModule.Generator/Discovery/Finders/FormRequestFinder.cs new file mode 100644 index 00000000..2dd06839 --- /dev/null +++ b/framework/SimpleModule.Generator/Discovery/Finders/FormRequestFinder.cs @@ -0,0 +1,101 @@ +using System.Collections.Generic; +using System.Threading; +using Microsoft.CodeAnalysis; + +namespace SimpleModule.Generator; + +internal static class FormRequestFinder +{ + internal static void Discover( + IReadOnlyList refAssemblies, + INamespaceSymbol hostGlobalNamespace, + CoreSymbols symbols, + List formRequests, + CancellationToken cancellationToken + ) + { + if (symbols.FormRequestAttribute is null) + return; + + foreach (var assemblySymbol in refAssemblies) + { + cancellationToken.ThrowIfCancellationRequested(); + FindFormRequestTypes( + assemblySymbol.GlobalNamespace, + symbols, + formRequests, + cancellationToken + ); + } + + FindFormRequestTypes(hostGlobalNamespace, symbols, formRequests, cancellationToken); + } + + private static void FindFormRequestTypes( + INamespaceSymbol namespaceSymbol, + CoreSymbols symbols, + List formRequests, + CancellationToken cancellationToken + ) + { + foreach (var member in namespaceSymbol.GetMembers()) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (member is INamespaceSymbol childNamespace) + { + FindFormRequestTypes(childNamespace, symbols, formRequests, cancellationToken); + } + else if (member is INamedTypeSymbol typeSymbol) + { + var hasAttribute = false; + foreach (var attr in typeSymbol.GetAttributes()) + { + if ( + SymbolEqualityComparer.Default.Equals( + attr.AttributeClass, + symbols.FormRequestAttribute + ) + ) + { + hasAttribute = true; + break; + } + } + + if (!hasAttribute) + continue; + + var fqn = typeSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); + + var extendsFormRequest = false; + var current = typeSymbol.BaseType; + while (current is not null) + { + if ( + symbols.FormRequestBase is not null + && SymbolEqualityComparer.Default.Equals( + current.OriginalDefinition, + symbols.FormRequestBase + ) + ) + { + extendsFormRequest = true; + break; + } + current = current.BaseType; + } + + formRequests.Add( + new FormRequestInfo + { + FullyQualifiedName = fqn, + IsSealed = typeSymbol.IsSealed, + ExtendsFormRequest = extendsFormRequest, + Location = SymbolHelpers.GetSourceLocation(typeSymbol), + } + ); + } + } + } +} diff --git a/framework/SimpleModule.Generator/Discovery/Records/FormRequestRecords.cs b/framework/SimpleModule.Generator/Discovery/Records/FormRequestRecords.cs new file mode 100644 index 00000000..a7445656 --- /dev/null +++ b/framework/SimpleModule.Generator/Discovery/Records/FormRequestRecords.cs @@ -0,0 +1,8 @@ +namespace SimpleModule.Generator; + +internal readonly record struct FormRequestInfoRecord( + string FullyQualifiedName, + bool IsSealed, + bool ExtendsFormRequest, + SourceLocationRecord? Location +); diff --git a/framework/SimpleModule.Generator/Discovery/Records/WorkingTypes.cs b/framework/SimpleModule.Generator/Discovery/Records/WorkingTypes.cs index 6a584e91..755f049a 100644 --- a/framework/SimpleModule.Generator/Discovery/Records/WorkingTypes.cs +++ b/framework/SimpleModule.Generator/Discovery/Records/WorkingTypes.cs @@ -156,3 +156,11 @@ internal sealed class DiscoveredTypeInfo public string FullyQualifiedName { get; set; } = ""; public string ModuleName { get; set; } = ""; } + +internal sealed class FormRequestInfo +{ + public string FullyQualifiedName { get; set; } = ""; + public bool IsSealed { get; set; } + public bool ExtendsFormRequest { get; set; } + public SourceLocationRecord? Location { get; set; } +} diff --git a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs index d63450c6..45a42e41 100644 --- a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs +++ b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs @@ -211,7 +211,17 @@ CancellationToken cancellationToken moduleOptionsList ); - // Step 3g: Agent definitions, tool providers, knowledge sources + // Step 3g: FormRequest types + var formRequests = new List(); + FormRequestFinder.Discover( + refAssemblies, + compilation.Assembly.GlobalNamespace, + s, + formRequests, + cancellationToken + ); + + // Step 3h: Agent definitions, tool providers, knowledge sources var agentDefinitions = new List(); var agentToolProviders = new List(); var knowledgeSources = new List(); @@ -253,6 +263,7 @@ CancellationToken cancellationToken agentDefinitions, agentToolProviders, knowledgeSources, + formRequests, contractsAssemblyMap, s.HasAgentsAssembly, s.HasRagAssembly, diff --git a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs index 6a2669f9..8260e8c8 100644 --- a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs @@ -12,5 +12,6 @@ public void Emit(SourceProductionContext context, DiscoveryData data) ContractAndDtoChecks.Run(context, data); PermissionFeatureChecks.Run(context, data); EndpointChecks.Run(context, data); + FormRequestChecks.Run(context, data); } } diff --git a/framework/SimpleModule.Generator/Emitters/Diagnostics/DiagnosticDescriptors.FormRequest.cs b/framework/SimpleModule.Generator/Emitters/Diagnostics/DiagnosticDescriptors.FormRequest.cs new file mode 100644 index 00000000..15bb03da --- /dev/null +++ b/framework/SimpleModule.Generator/Emitters/Diagnostics/DiagnosticDescriptors.FormRequest.cs @@ -0,0 +1,24 @@ +using Microsoft.CodeAnalysis; + +namespace SimpleModule.Generator; + +internal static partial class DiagnosticDescriptors +{ + internal static readonly DiagnosticDescriptor FormRequestNotSealed = new( + id: "SM0056", + title: "FormRequest class must be sealed", + messageFormat: "FormRequest '{0}' is not sealed. FormRequest classes must be sealed to prevent inheritance hierarchies that break validation caching and make the pipeline unpredictable.", + category: "SimpleModule.Generator", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); + + internal static readonly DiagnosticDescriptor FormRequestDoesNotExtendBase = new( + id: "SM0057", + title: "FormRequest class must extend FormRequest", + messageFormat: "FormRequest '{0}' has the [FormRequest] attribute but does not extend FormRequest<{0}>. The class must extend FormRequest to participate in the validation pipeline.", + category: "SimpleModule.Generator", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); +} diff --git a/framework/SimpleModule.Generator/Emitters/Diagnostics/FormRequestChecks.cs b/framework/SimpleModule.Generator/Emitters/Diagnostics/FormRequestChecks.cs new file mode 100644 index 00000000..7bd5c85b --- /dev/null +++ b/framework/SimpleModule.Generator/Emitters/Diagnostics/FormRequestChecks.cs @@ -0,0 +1,34 @@ +using Microsoft.CodeAnalysis; + +namespace SimpleModule.Generator; + +internal static class FormRequestChecks +{ + internal static void Run(SourceProductionContext context, DiscoveryData data) + { + foreach (var formRequest in data.FormRequests) + { + if (!formRequest.IsSealed) + { + context.ReportDiagnostic( + Diagnostic.Create( + DiagnosticDescriptors.FormRequestNotSealed, + LocationHelper.ToLocation(formRequest.Location), + TypeMappingHelpers.StripGlobalPrefix(formRequest.FullyQualifiedName) + ) + ); + } + + if (!formRequest.ExtendsFormRequest) + { + context.ReportDiagnostic( + Diagnostic.Create( + DiagnosticDescriptors.FormRequestDoesNotExtendBase, + LocationHelper.ToLocation(formRequest.Location), + TypeMappingHelpers.StripGlobalPrefix(formRequest.FullyQualifiedName) + ) + ); + } + } + } +} diff --git a/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs b/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs index ca40d4cf..fbeed7e2 100644 --- a/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs @@ -18,6 +18,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) sb.AppendLine("using Microsoft.AspNetCore.Http;"); sb.AppendLine("using Microsoft.AspNetCore.Authorization;"); sb.AppendLine("using SimpleModule.Core.Authorization;"); + sb.AppendLine("using SimpleModule.Core.FormRequests;"); sb.AppendLine(); sb.AppendLine("namespace SimpleModule.Core;"); sb.AppendLine(); @@ -42,7 +43,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) if (!string.IsNullOrEmpty(module.RoutePrefix)) { sb.AppendLine( - $" var group = app.MapGroup(\"{module.RoutePrefix}\").WithTags(\"{module.ModuleName}\").RequireAuthorization();" + $" var group = app.MapGroup(\"{module.RoutePrefix}\").WithTags(\"{module.ModuleName}\").RequireAuthorization().AddFormRequestFilter();" ); foreach (var endpoint in module.Endpoints) { diff --git a/modules/Email/src/SimpleModule.Email/Endpoints/Templates/CreateTemplateEndpoint.cs b/modules/Email/src/SimpleModule.Email/Endpoints/Templates/CreateTemplateEndpoint.cs index 82b58bf4..adf69be1 100644 --- a/modules/Email/src/SimpleModule.Email/Endpoints/Templates/CreateTemplateEndpoint.cs +++ b/modules/Email/src/SimpleModule.Email/Endpoints/Templates/CreateTemplateEndpoint.cs @@ -1,11 +1,9 @@ -using FluentValidation; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; -using SimpleModule.Core.Authorization; using SimpleModule.Core.Endpoints; -using SimpleModule.Core.Validation; using SimpleModule.Email.Contracts; +using SimpleModule.Email.FormRequests; namespace SimpleModule.Email.Endpoints.Templates; @@ -16,24 +14,23 @@ public class CreateTemplateEndpoint : IEndpoint public void Map(IEndpointRouteBuilder app) => app.MapPost( - Route, - async ( - CreateEmailTemplateRequest request, - IValidator validator, - IEmailContracts emailContracts - ) => + Route, + async (CreateTemplateFormRequest request, IEmailContracts emailContracts) => + { + var dto = new CreateEmailTemplateRequest { - var validation = await validator.ValidateAsync(request); - if (!validation.IsValid) - throw new Core.Exceptions.ValidationException( - validation.ToValidationErrors() - ); + Name = request.Name, + Slug = request.Slug, + Subject = request.Subject, + Body = request.Body, + IsHtml = request.IsHtml, + DefaultReplyTo = request.DefaultReplyTo, + }; - return await CrudEndpoints.Create( - () => emailContracts.CreateTemplateAsync(request), - t => $"/api/email/templates/{t.Id.Value}" - ); - } - ) - .RequirePermission(EmailPermissions.ManageTemplates); + return await CrudEndpoints.Create( + () => emailContracts.CreateTemplateAsync(dto), + t => $"/api/email/templates/{t.Id.Value}" + ); + } + ); } diff --git a/modules/Email/src/SimpleModule.Email/FormRequests/CreateTemplateFormRequest.cs b/modules/Email/src/SimpleModule.Email/FormRequests/CreateTemplateFormRequest.cs new file mode 100644 index 00000000..3e73aec1 --- /dev/null +++ b/modules/Email/src/SimpleModule.Email/FormRequests/CreateTemplateFormRequest.cs @@ -0,0 +1,64 @@ +using System.Security.Claims; +using System.Text.RegularExpressions; +using FluentValidation; +using SimpleModule.Core.Authorization; +using SimpleModule.Core.Extensions; +using SimpleModule.Core.FormRequests; + +namespace SimpleModule.Email.FormRequests; + +[FormRequest] +public sealed partial class CreateTemplateFormRequest : FormRequest +{ + public string Name { get; set; } = ""; + public string Slug { get; set; } = ""; + public string Subject { get; set; } = ""; + public string Body { get; set; } = ""; + public bool IsHtml { get; set; } = true; + public string? DefaultReplyTo { get; set; } + + public override bool Authorize(ClaimsPrincipal user) => + user.HasPermission(EmailPermissions.ManageTemplates); + + public override void Prepare() + { + Name = Name.Trim(); +#pragma warning disable CA1308 // Slugs are conventionally lowercase + Slug = Slug.Trim().ToLowerInvariant(); +#pragma warning restore CA1308 + Subject = Subject.Trim(); + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules + .RuleFor(x => x.Name) + .NotEmpty() + .WithMessage("Name is required.") + .MaximumLength(200) + .WithMessage("Name must not exceed 200 characters."); + rules + .RuleFor(x => x.Slug) + .NotEmpty() + .WithMessage("Slug is required.") + .MaximumLength(200) + .WithMessage("Slug must not exceed 200 characters.") + .Must(s => string.IsNullOrWhiteSpace(s) || SlugPattern().IsMatch(s)) + .WithMessage("Slug must be lowercase alphanumeric with hyphens."); + rules + .RuleFor(x => x.Subject) + .NotEmpty() + .WithMessage("Subject is required.") + .MaximumLength(500) + .WithMessage("Subject must not exceed 500 characters."); + rules.RuleFor(x => x.Body).NotEmpty().WithMessage("Body is required."); + rules + .RuleFor(x => x.DefaultReplyTo) + .EmailAddress() + .When(x => !string.IsNullOrWhiteSpace(x.DefaultReplyTo)) + .WithMessage("Invalid email format."); + } + + [GeneratedRegex(@"^[a-z0-9]+(-[a-z0-9]+)*$")] + private static partial Regex SlugPattern(); +} diff --git a/modules/Email/src/SimpleModule.Email/types.ts b/modules/Email/src/SimpleModule.Email/types.ts index 84790408..2fb76a76 100644 --- a/modules/Email/src/SimpleModule.Email/types.ts +++ b/modules/Email/src/SimpleModule.Email/types.ts @@ -106,3 +106,12 @@ export interface UpdateEmailTemplateRequest { defaultReplyTo: string; } +export interface CreateTemplateFormRequest { + name: string; + slug: string; + subject: string; + body: string; + isHtml: boolean; + defaultReplyTo: string; +} + diff --git a/tests/SimpleModule.Core.Tests/FormRequests/FormRequestEndpointFilterTests.cs b/tests/SimpleModule.Core.Tests/FormRequests/FormRequestEndpointFilterTests.cs new file mode 100644 index 00000000..b747d9dd --- /dev/null +++ b/tests/SimpleModule.Core.Tests/FormRequests/FormRequestEndpointFilterTests.cs @@ -0,0 +1,262 @@ +using System.Security.Claims; +using System.Text.Json; +using FluentAssertions; +using FluentValidation; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.HttpResults; +using SimpleModule.Core.FormRequests; + +namespace SimpleModule.Core.Tests.FormRequests; + +public class FormRequestEndpointFilterTests +{ + private readonly FormRequestEndpointFilter _filter = new(); + + [Fact] + public async Task InvokeAsync_NoFormRequestArguments_CallsNext() + { + var nextCalled = false; + var context = CreateFilterContext("plain string argument", 42); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task InvokeAsync_ValidRequest_CallsNext() + { + var request = new ValidTestRequest { Name = "Test", Value = 10 }; + var nextCalled = false; + var context = CreateFilterContext(request); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task InvokeAsync_InvalidRequest_Returns422() + { + var request = new ValidTestRequest { Name = "", Value = -1 }; + var context = CreateFilterContext(request); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + result.Should().BeAssignableTo(); + var problem = (ProblemHttpResult)result!; + problem.StatusCode.Should().Be(422); + problem.ProblemDetails.Title.Should().Be("Validation Error"); + problem.ProblemDetails.Extensions.Should().ContainKey("errors"); + } + + [Fact] + public async Task InvokeAsync_InvalidRequest_ReturnsFieldErrors() + { + var request = new ValidTestRequest { Name = "", Value = -1 }; + var context = CreateFilterContext(request); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + var problem = (ProblemHttpResult)result!; + var errorsObj = problem.ProblemDetails.Extensions["errors"]; + var errorsJson = JsonSerializer.Serialize(errorsObj); + var errors = JsonSerializer.Deserialize>(errorsJson)!; + errors.Should().ContainKey("Name"); + errors.Should().ContainKey("Value"); + } + + [Fact] + public async Task InvokeAsync_UnauthorizedRequest_Returns403() + { + var request = new UnauthorizedTestRequest { Name = "Test" }; + var context = CreateFilterContext(request); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + result.Should().BeAssignableTo(); + var problem = (ProblemHttpResult)result!; + problem.StatusCode.Should().Be(403); + } + + [Fact] + public async Task InvokeAsync_Prepare_NormalizesDataBeforeValidation() + { + var request = new PrepareTestRequest { Sku = " abc-123 " }; + var context = CreateFilterContext(request); + + await _filter.InvokeAsync(context, _ => ValueTask.FromResult(Results.Ok())); + + request.Sku.Should().Be("ABC-123"); + } + + [Fact] + public async Task InvokeAsync_Prepare_RunsBeforeValidation() + { + var request = new PrepareTestRequest { Sku = " valid-sku " }; + var context = CreateFilterContext(request); + var nextCalled = false; + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + request.Sku.Should().Be("VALID-SKU"); + } + + [Fact] + public async Task InvokeAsync_AuthorizeChecksUser() + { + var request = new PermissionTestRequest { Name = "Test" }; + var context = CreateFilterContext(request, new Claim("permission", "Products.Create")); + var nextCalled = false; + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task InvokeAsync_AuthorizeChecksFail_WhenMissingPermission() + { + var request = new PermissionTestRequest { Name = "Test" }; + var context = CreateFilterContext(request); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + result.Should().BeAssignableTo(); + var problem = (ProblemHttpResult)result!; + problem.StatusCode.Should().Be(403); + } + + private static DefaultEndpointFilterInvocationContext CreateFilterContext( + params object[] arguments + ) + { + return CreateFilterContext(arguments, Array.Empty()); + } + + private static DefaultEndpointFilterInvocationContext CreateFilterContext( + object argument, + params Claim[] claims + ) + { + return CreateFilterContext(new[] { argument }, claims); + } + + private static DefaultEndpointFilterInvocationContext CreateFilterContext( + object[] arguments, + Claim[] claims + ) + { + var httpContext = new DefaultHttpContext(); + httpContext.Response.Body = new MemoryStream(); + + if (claims.Length > 0) + { + var identity = new ClaimsIdentity(claims, "test"); + httpContext.User = new ClaimsPrincipal(identity); + } + + return new DefaultEndpointFilterInvocationContext(httpContext, arguments); + } + + #region Test FormRequest types + + [FormRequest] + private sealed class ValidTestRequest : FormRequest + { + public string Name { get; set; } = ""; + public int Value { get; set; } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty(); + rules.RuleFor(x => x.Value).GreaterThan(0); + } + } + + [FormRequest] + private sealed class UnauthorizedTestRequest : FormRequest + { + public string Name { get; set; } = ""; + + public override bool Authorize(ClaimsPrincipal user) => false; + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty(); + } + } + + [FormRequest] + private sealed class PrepareTestRequest : FormRequest + { + public string Sku { get; set; } = ""; + + public override void Prepare() + { + Sku = Sku.Trim().ToUpperInvariant(); + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Sku).NotEmpty().Matches("^[A-Z0-9-]+$"); + } + } + + [FormRequest] + private sealed class PermissionTestRequest : FormRequest + { + public string Name { get; set; } = ""; + + public override bool Authorize(ClaimsPrincipal user) => + user.Claims.Any(c => c is { Type: "permission", Value: "Products.Create" }); + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty(); + } + } + + #endregion +} diff --git a/tests/SimpleModule.Core.Tests/FormRequests/FormRequestValidationTests.cs b/tests/SimpleModule.Core.Tests/FormRequests/FormRequestValidationTests.cs new file mode 100644 index 00000000..c89fe3a3 --- /dev/null +++ b/tests/SimpleModule.Core.Tests/FormRequests/FormRequestValidationTests.cs @@ -0,0 +1,125 @@ +using System.Security.Claims; +using FluentAssertions; +using FluentValidation; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.HttpResults; +using SimpleModule.Core.FormRequests; + +namespace SimpleModule.Core.Tests.FormRequests; + +public class FormRequestValidationTests +{ + private readonly FormRequestEndpointFilter _filter = new(); + + [Fact] + public async Task ValidRequest_PassesThrough() + { + var request = new TestFormRequest { Name = "Valid", Price = 10m }; + var nextCalled = false; + + await _filter.InvokeAsync( + CreateContext(request), + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task EmptyName_Returns422() + { + var request = new TestFormRequest { Name = "", Price = 10m }; + + var result = await _filter.InvokeAsync( + CreateContext(request), + _ => ValueTask.FromResult(Results.Ok()) + ); + + var problem = result.Should().BeOfType().Subject; + problem.StatusCode.Should().Be(422); + } + + [Fact] + public async Task NegativePrice_Returns422() + { + var request = new TestFormRequest { Name = "Valid", Price = -5m }; + + var result = await _filter.InvokeAsync( + CreateContext(request), + _ => ValueTask.FromResult(Results.Ok()) + ); + + result.Should().BeOfType(); + } + + [Fact] + public async Task MultipleViolations_ReturnsAllErrors() + { + var request = new TestFormRequest { Name = "", Price = -5m }; + + var result = await _filter.InvokeAsync( + CreateContext(request), + _ => ValueTask.FromResult(Results.Ok()) + ); + + var problem = (ProblemHttpResult)result!; + problem.ProblemDetails.Extensions.Should().ContainKey("errors"); + } + + [Fact] + public async Task ValidatorIsCached_AcrossInstances() + { + var r1 = new TestFormRequest { Name = "A", Price = 1m }; + var r2 = new TestFormRequest { Name = "B", Price = 2m }; + + var nextCount = 0; + ValueTask Next(EndpointFilterInvocationContext _) + { + nextCount++; + return ValueTask.FromResult(Results.Ok()); + } + + await _filter.InvokeAsync(CreateContext(r1), Next); + await _filter.InvokeAsync(CreateContext(r2), Next); + + nextCount.Should().Be(2); + } + + [Fact] + public void Prepare_DefaultImplementation_DoesNotThrow() + { + var request = new TestFormRequest { Name = "Test", Price = 1m }; + var act = () => request.Prepare(); + act.Should().NotThrow(); + } + + [Fact] + public void Authorize_DefaultImplementation_ReturnsTrue() + { + var request = new TestFormRequest { Name = "Test", Price = 1m }; + request.Authorize(new ClaimsPrincipal()).Should().BeTrue(); + } + + private static DefaultEndpointFilterInvocationContext CreateContext(FormRequest request) + { + var httpContext = new DefaultHttpContext { Response = { Body = new MemoryStream() } }; + return new DefaultEndpointFilterInvocationContext(httpContext, request); + } + + [FormRequest] + private sealed class TestFormRequest : FormRequest + { + public string Name { get; set; } = ""; + public decimal Price { get; set; } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty().MaximumLength(200); + rules.RuleFor(x => x.Price).GreaterThan(0); + } + } +} diff --git a/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs b/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs index 6f490b84..d016558c 100644 --- a/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs +++ b/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs @@ -247,6 +247,7 @@ public void SortModules_WithDependencies_ReordersByDependency() ImmutableArray.Empty, ImmutableArray.Empty, ImmutableArray.Empty, + ImmutableArray.Empty, ImmutableArray.Empty, false, false, @@ -323,6 +324,7 @@ public void SortModules_WithCycle_ReturnsOriginalOrder() ImmutableArray.Empty, ImmutableArray.Empty, ImmutableArray.Empty, + ImmutableArray.Empty, ImmutableArray.Empty, false, false, @@ -415,6 +417,7 @@ public void SortModules_NoDependencies_PreservesOriginalOrder() ImmutableArray.Empty, ImmutableArray.Empty, ImmutableArray.Empty, + ImmutableArray.Empty, ImmutableArray.Empty, false, false, From 48938689d848233eaf5d83ec7bbc5f91ca5b24e8 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sun, 24 May 2026 14:52:32 +0200 Subject: [PATCH 2/6] =?UTF-8?q?fix(core):=20address=20architecture=20revie?= =?UTF-8?q?w=20=E2=80=94=20filter=20coverage,=20shared=20Inertia=20errors,?= =?UTF-8?q?=20testability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix C3: modules without RoutePrefix now get a group with AddFormRequestFilter() instead of mapping directly to app (no-prefix endpoints were silently skipping the FormRequest validation pipeline) - Fix C1: ConfigureEndpoints escape hatch now wraps in a group with AddFormRequestFilter() so FormRequest types work in escape-hatch modules - Fix M4: extract shared InertiaErrorResult utility used by both FormRequestEndpointFilter and GlobalExceptionHandler (removes duplication) - Fix m2: add public ValidateRulesAsync() method on FormRequest so consumers can unit-test their validation rules without constructing filter context - Update generator tests to match new generated code shape --- .../Exceptions/GlobalExceptionHandler.cs | 41 +- .../FormRequests/FormRequest.cs | 9 +- .../FormRequests/FormRequestEndpointFilter.cs | 60 +- .../Inertia/InertiaErrorResult.cs | 51 ++ .../Emitters/EndpointExtensionsEmitter.cs | 13 +- .../Unit/CreateTemplateFormRequestTests.cs | 473 ++++++++++++ .../FormRequests/FormRequestEdgeCaseTests.cs | 699 ++++++++++++++++++ .../DiagnosticCatalogTests.cs | 6 + .../EndpointExtensionsEmitterTests.cs | 16 +- .../FormRequestDiagnosticTests.cs | 423 +++++++++++ .../Helpers/GeneratorTestHelper.cs | 19 + .../ModuleDiscovererGeneratorTests.cs | 4 +- .../ModuleExtensionsGenerationTests.cs | 2 +- 13 files changed, 1719 insertions(+), 97 deletions(-) create mode 100644 framework/SimpleModule.Core/Inertia/InertiaErrorResult.cs create mode 100644 modules/Email/tests/SimpleModule.Email.Tests/Unit/CreateTemplateFormRequestTests.cs create mode 100644 tests/SimpleModule.Core.Tests/FormRequests/FormRequestEdgeCaseTests.cs create mode 100644 tests/SimpleModule.Generator.Tests/FormRequestDiagnosticTests.cs diff --git a/framework/SimpleModule.Core/Exceptions/GlobalExceptionHandler.cs b/framework/SimpleModule.Core/Exceptions/GlobalExceptionHandler.cs index 1fb956bd..47c70416 100644 --- a/framework/SimpleModule.Core/Exceptions/GlobalExceptionHandler.cs +++ b/framework/SimpleModule.Core/Exceptions/GlobalExceptionHandler.cs @@ -1,4 +1,3 @@ -using System.Text.Json; using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -11,11 +10,6 @@ namespace SimpleModule.Core.Exceptions; public sealed class GlobalExceptionHandler(ILogger logger) : IExceptionHandler { - private static readonly JsonSerializerOptions InertiaJsonOptions = new() - { - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }; - public async ValueTask TryHandleAsync( HttpContext httpContext, Exception exception, @@ -70,7 +64,9 @@ CancellationToken cancellationToken if (httpContext.Request.IsInertia()) { - return await WriteInertiaErrorAsync(httpContext, statusCode, title, detail); + var inertiaResult = new InertiaErrorResult(statusCode, title, detail); + await inertiaResult.ExecuteAsync(httpContext); + return true; } var problemDetails = new ProblemDetails @@ -88,35 +84,4 @@ CancellationToken cancellationToken await httpContext.Response.WriteAsJsonAsync(problemDetails, cancellationToken); return true; } - - private static async ValueTask WriteInertiaErrorAsync( - HttpContext httpContext, - int statusCode, - string title, - string message - ) - { - var component = $"Error/{statusCode}"; - var props = new - { - status = statusCode, - title, - message, - }; - - var pageData = new - { - component, - props, - url = httpContext.Request.Path + httpContext.Request.QueryString, - version = InertiaMiddleware.Version, - }; - - httpContext.Response.Headers[InertiaHttpExtensions.InertiaHeader] = "true"; - httpContext.Response.Headers["Vary"] = InertiaHttpExtensions.InertiaHeader; - httpContext.Response.ContentType = "application/json"; - var json = JsonSerializer.Serialize(pageData, InertiaJsonOptions); - await httpContext.Response.WriteAsync(json); - return true; - } } diff --git a/framework/SimpleModule.Core/FormRequests/FormRequest.cs b/framework/SimpleModule.Core/FormRequests/FormRequest.cs index 6834c2f3..08446442 100644 --- a/framework/SimpleModule.Core/FormRequests/FormRequest.cs +++ b/framework/SimpleModule.Core/FormRequests/FormRequest.cs @@ -1,4 +1,5 @@ using System.Security.Claims; +using FluentValidation.Results; namespace SimpleModule.Core.FormRequests; @@ -8,7 +9,9 @@ public abstract class FormRequest public virtual void Prepare() { } - internal abstract Task ValidateAsync( - CancellationToken cancellationToken - ); + public async Task ValidateRulesAsync( + CancellationToken cancellationToken = default + ) => await ValidateAsync(cancellationToken); + + internal abstract Task ValidateAsync(CancellationToken cancellationToken); } diff --git a/framework/SimpleModule.Core/FormRequests/FormRequestEndpointFilter.cs b/framework/SimpleModule.Core/FormRequests/FormRequestEndpointFilter.cs index c8daf187..52b61498 100644 --- a/framework/SimpleModule.Core/FormRequests/FormRequestEndpointFilter.cs +++ b/framework/SimpleModule.Core/FormRequests/FormRequestEndpointFilter.cs @@ -1,6 +1,4 @@ -using System.Text.Json; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; using SimpleModule.Core.Constants; using SimpleModule.Core.Inertia; using SimpleModule.Core.Validation; @@ -9,11 +7,6 @@ namespace SimpleModule.Core.FormRequests; public sealed class FormRequestEndpointFilter : IEndpointFilter { - private static readonly JsonSerializerOptions InertiaJsonOptions = new() - { - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }; - public async ValueTask InvokeAsync( EndpointFilterInvocationContext context, EndpointFilterDelegate next @@ -26,6 +19,15 @@ EndpointFilterDelegate next if (!formRequest.Authorize(context.HttpContext.User)) { + if (context.HttpContext.Request.IsInertia()) + { + return new InertiaErrorResult( + StatusCodes.Status403Forbidden, + ErrorMessages.ForbiddenTitle, + ErrorMessages.DefaultForbiddenMessage + ); + } + return Results.Problem( statusCode: StatusCodes.Status403Forbidden, title: ErrorMessages.ForbiddenTitle, @@ -42,7 +44,12 @@ EndpointFilterDelegate next if (context.HttpContext.Request.IsInertia()) { - return WriteInertiaValidationError(context.HttpContext, errors); + return new InertiaErrorResult( + StatusCodes.Status422UnprocessableEntity, + ErrorMessages.ValidationErrorTitle, + ErrorMessages.DefaultValidationMessage, + errors + ); } return Results.Problem( @@ -56,41 +63,4 @@ EndpointFilterDelegate next return await next(context); } - - private static InertiaValidationResult WriteInertiaValidationError( - HttpContext httpContext, - Dictionary errors - ) - { - var component = "Error/422"; - var pageData = new - { - component, - props = new - { - status = 422, - title = ErrorMessages.ValidationErrorTitle, - message = ErrorMessages.DefaultValidationMessage, - errors, - }, - url = httpContext.Request.Path + httpContext.Request.QueryString, - version = InertiaMiddleware.Version, - }; - - return new InertiaValidationResult(pageData); - } - - private sealed class InertiaValidationResult(object pageData) : IResult - { - public async Task ExecuteAsync(HttpContext httpContext) - { - httpContext.Response.StatusCode = StatusCodes.Status422UnprocessableEntity; - httpContext.Response.Headers[InertiaHttpExtensions.InertiaHeader] = "true"; - httpContext.Response.Headers["Vary"] = InertiaHttpExtensions.InertiaHeader; - httpContext.Response.ContentType = "application/json"; - await httpContext.Response.WriteAsync( - JsonSerializer.Serialize(pageData, InertiaJsonOptions) - ); - } - } } diff --git a/framework/SimpleModule.Core/Inertia/InertiaErrorResult.cs b/framework/SimpleModule.Core/Inertia/InertiaErrorResult.cs new file mode 100644 index 00000000..88e34c7f --- /dev/null +++ b/framework/SimpleModule.Core/Inertia/InertiaErrorResult.cs @@ -0,0 +1,51 @@ +using System.Text.Json; +using Microsoft.AspNetCore.Http; + +namespace SimpleModule.Core.Inertia; + +public sealed class InertiaErrorResult( + int statusCode, + string title, + string message, + object? errors = null +) : IResult +{ + private static readonly JsonSerializerOptions JsonOptions = new() + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }; + + public async Task ExecuteAsync(HttpContext httpContext) + { + var component = $"Error/{statusCode}"; + var props = errors is not null + ? (object) + new + { + status = statusCode, + title, + message, + errors, + } + : new + { + status = statusCode, + title, + message, + }; + + var pageData = new + { + component, + props, + url = httpContext.Request.Path + httpContext.Request.QueryString, + version = InertiaMiddleware.Version, + }; + + httpContext.Response.StatusCode = statusCode; + httpContext.Response.Headers[InertiaHttpExtensions.InertiaHeader] = "true"; + httpContext.Response.Headers["Vary"] = InertiaHttpExtensions.InertiaHeader; + httpContext.Response.ContentType = "application/json"; + await httpContext.Response.WriteAsync(JsonSerializer.Serialize(pageData, JsonOptions)); + } +} diff --git a/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs b/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs index fbeed7e2..6c4270d5 100644 --- a/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs @@ -52,9 +52,12 @@ public void Emit(SourceProductionContext context, DiscoveryData data) } else { + sb.AppendLine( + $" var group = app.MapGroup(\"\").WithTags(\"{module.ModuleName}\").RequireAuthorization().AddFormRequestFilter();" + ); foreach (var endpoint in module.Endpoints) { - EmitEndpointRegistration(sb, endpoint, "app"); + EmitEndpointRegistration(sb, endpoint, "group"); } } @@ -95,13 +98,19 @@ public void Emit(SourceProductionContext context, DiscoveryData data) } // Manual ConfigureEndpoints (escape hatch) + // Wrap in a group with FormRequestFilter so FormRequest types work in escape-hatch modules. foreach (var module in modules.Where(m => m.HasConfigureEndpoints)) { var fieldName = TypeMappingHelpers.GetModuleFieldName(module.FullyQualifiedName); sb.AppendLine(); + sb.AppendLine(" {"); sb.AppendLine( - $" ((global::SimpleModule.Core.IModule)ModuleExtensions.{fieldName}).ConfigureEndpoints(app);" + $" var _escapeGroup = app.MapGroup(\"\").AddFormRequestFilter();" ); + sb.AppendLine( + $" ((global::SimpleModule.Core.IModule)ModuleExtensions.{fieldName}).ConfigureEndpoints(_escapeGroup);" + ); + sb.AppendLine(" }"); } sb.AppendLine(); diff --git a/modules/Email/tests/SimpleModule.Email.Tests/Unit/CreateTemplateFormRequestTests.cs b/modules/Email/tests/SimpleModule.Email.Tests/Unit/CreateTemplateFormRequestTests.cs new file mode 100644 index 00000000..238da18d --- /dev/null +++ b/modules/Email/tests/SimpleModule.Email.Tests/Unit/CreateTemplateFormRequestTests.cs @@ -0,0 +1,473 @@ +using System.Security.Claims; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.HttpResults; +using SimpleModule.Core.Authorization; +using SimpleModule.Core.FormRequests; +using SimpleModule.Email.FormRequests; + +namespace SimpleModule.Email.Tests.Unit; + +/// +/// Covers CreateTemplateFormRequest in isolation — authorization, normalization (Prepare), +/// and each validation rule. The filter pipeline is exercised via FormRequestEndpointFilter +/// directly so we do not need a running server. +/// +public class CreateTemplateFormRequestTests +{ + private readonly FormRequestEndpointFilter _filter = new(); + + // ─── Authorize ──────────────────────────────────────────────────────────── + + [Fact] + public void Authorize_UserWithManageTemplatesPermission_ReturnsTrue() + { + var user = MakeUser(EmailPermissions.ManageTemplates); + var request = ValidRequest(); + + request.Authorize(user).Should().BeTrue(); + } + + [Fact] + public void Authorize_UserWithoutPermission_ReturnsFalse() + { + var user = MakeUser("SomeOtherPermission"); + var request = ValidRequest(); + + request.Authorize(user).Should().BeFalse(); + } + + [Fact] + public void Authorize_AnonymousUser_ReturnsFalse() + { + var user = new ClaimsPrincipal(new ClaimsIdentity()); + var request = ValidRequest(); + + request.Authorize(user).Should().BeFalse(); + } + + [Fact] + public async Task Filter_UnauthorizedUser_Returns403() + { + var request = ValidRequest(); + var context = CreateContext(request); // no permission claims + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + result.Should().BeOfType().Which.StatusCode.Should().Be(403); + } + + // ─── Prepare / normalization ────────────────────────────────────────────── + + [Fact] + public void Prepare_TrimsNameAndSubject() + { + var request = new CreateTemplateFormRequest + { + Name = " Welcome Email ", + Slug = "welcome-email", + Subject = " Hello {{name}} ", + Body = "Hi there", + }; + + request.Prepare(); + + request.Name.Should().Be("Welcome Email"); + request.Subject.Should().Be("Hello {{name}}"); + } + + [Fact] + public void Prepare_NormalizesSlugToLowercase() + { + var request = new CreateTemplateFormRequest + { + Name = "Test", + Slug = " WELCOME-Email ", + Subject = "Test", + Body = "Body", + }; + + request.Prepare(); + + request.Slug.Should().Be("welcome-email"); + } + + [Fact] + public void Prepare_SlugTrimmedAndLowercased() + { + var request = new CreateTemplateFormRequest + { + Slug = " Order-Confirmation ", + Name = "n", + Subject = "s", + Body = "b", + }; + + request.Prepare(); + + request.Slug.Should().Be("order-confirmation"); + } + + // ─── Validation rules ───────────────────────────────────────────────────── + + [Fact] + public async Task ValidRequest_PassesValidation() + { + var request = ValidRequest(); + var nextCalled = false; + var context = CreateContext(request, hasPermission: true); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task EmptyName_Returns422() + { + var request = ValidRequest(); + request.Name = ""; + var context = CreateContext(request, hasPermission: true); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + AssertFieldError(result, "Name"); + } + + [Fact] + public async Task NameExceeds200Characters_Returns422() + { + var request = ValidRequest(); + request.Name = new string('A', 201); + var context = CreateContext(request, hasPermission: true); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + AssertFieldError(result, "Name"); + } + + [Fact] + public async Task Exactly200CharactersName_Passes() + { + var request = ValidRequest(); + request.Name = new string('A', 200); + var nextCalled = false; + var context = CreateContext(request, hasPermission: true); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task EmptySlug_Returns422() + { + var request = ValidRequest(); + request.Slug = ""; + var context = CreateContext(request, hasPermission: true); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + AssertFieldError(result, "Slug"); + } + + [Theory] + // These values remain invalid even after Prepare() lowercases and trims them: + [InlineData("has space")] // spaces survive lowercase (become "has space") + [InlineData("trailing-")] // trailing hyphen + [InlineData("-leading")] // leading hyphen + [InlineData("double--hyphen")] // consecutive hyphens + [InlineData("special!char")] // special characters + public async Task InvalidSlugPattern_Returns422(string slug) + { + var request = ValidRequest(); + request.Slug = slug; + var context = CreateContext(request, hasPermission: true); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + AssertFieldError(result, "Slug"); + } + + [Theory] + [InlineData("welcome")] + [InlineData("order-confirmation")] + [InlineData("a1b2c3")] + [InlineData("x")] + public async Task ValidSlugPattern_Passes(string slug) + { + var request = ValidRequest(); + request.Slug = slug; + var nextCalled = false; + var context = CreateContext(request, hasPermission: true); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue($"slug '{slug}' is valid"); + } + + [Fact] + public async Task EmptySubject_Returns422() + { + var request = ValidRequest(); + request.Subject = ""; + var context = CreateContext(request, hasPermission: true); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + AssertFieldError(result, "Subject"); + } + + [Fact] + public async Task EmptyBody_Returns422() + { + var request = ValidRequest(); + request.Body = ""; + var context = CreateContext(request, hasPermission: true); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + AssertFieldError(result, "Body"); + } + + [Fact] + public async Task ValidEmailReplyTo_Passes() + { + var request = ValidRequest(); + request.DefaultReplyTo = "support@example.com"; + var nextCalled = false; + var context = CreateContext(request, hasPermission: true); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task InvalidEmailReplyTo_Returns422() + { + var request = ValidRequest(); + request.DefaultReplyTo = "not-an-email"; + var context = CreateContext(request, hasPermission: true); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + AssertFieldError(result, "DefaultReplyTo"); + } + + [Fact] + public async Task NullReplyTo_Passes() + { + var request = ValidRequest(); + request.DefaultReplyTo = null; + var nextCalled = false; + var context = CreateContext(request, hasPermission: true); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue("null DefaultReplyTo is allowed"); + } + + [Fact] + public async Task WhitespaceReplyTo_PassesBecauseWhenConditionSkipsIt() + { + // The rule uses `.When(x => !string.IsNullOrWhiteSpace(x.DefaultReplyTo))`, + // so a whitespace-only value must not trigger the email format check. + var request = ValidRequest(); + request.DefaultReplyTo = " "; + var nextCalled = false; + var context = CreateContext(request, hasPermission: true); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue("whitespace DefaultReplyTo is skipped by When condition"); + } + + // ─── Prepare + Validate integration ────────────────────────────────────── + + [Fact] + public async Task PrepareNormalizesSlugThenValidationSees_LowercaseSlug() + { + // After Prepare(), "Welcome-Email" becomes "welcome-email" which matches the slug pattern. + // If validation ran before Prepare, the uppercase letters would fail the regex. + var request = new CreateTemplateFormRequest + { + Name = "Welcome", + Slug = "Welcome-Email", // valid after normalization + Subject = "Hello", + Body = "Body text", + }; + var nextCalled = false; + var context = CreateContext(request, hasPermission: true); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue("Prepare must run before validation"); + request.Slug.Should().Be("welcome-email"); + } + + // ─── Mapping to DTO (CreateTemplateEndpoint contract) ──────────────────── + + [Fact] + public void FormRequest_MapsAllFieldsToDto() + { + // The CreateTemplateEndpoint maps FormRequest → CreateEmailTemplateRequest. + // Verify that all fields the endpoint touches are accessible on the request. + var request = new CreateTemplateFormRequest + { + Name = "Invoice", + Slug = "invoice", + Subject = "Your invoice", + Body = "

Hello

", + IsHtml = true, + DefaultReplyTo = "billing@example.com", + }; + + // If any property was renamed or removed, this would fail to compile. + _ = request.Name; + _ = request.Slug; + _ = request.Subject; + _ = request.Body; + _ = request.IsHtml; + _ = request.DefaultReplyTo; + + request.Name.Should().Be("Invoice"); + request.Slug.Should().Be("invoice"); + request.Subject.Should().Be("Your invoice"); + request.Body.Should().Be("

Hello

"); + request.IsHtml.Should().BeTrue(); + request.DefaultReplyTo.Should().Be("billing@example.com"); + } + + [Fact] + public void FormRequest_IsHtml_DefaultsToTrue() + { + var request = new CreateTemplateFormRequest(); + request.IsHtml.Should().BeTrue(); + } + + // ─── Helpers ───────────────────────────────────────────────────────────── + + private static CreateTemplateFormRequest ValidRequest() => + new() + { + Name = "Welcome", + Slug = "welcome", + Subject = "Hello {{name}}", + Body = "

Welcome to the platform!

", + IsHtml = true, + }; + + private static ClaimsPrincipal MakeUser(string permission) + { + var claims = new[] { new Claim("permission", permission) }; + return new ClaimsPrincipal(new ClaimsIdentity(claims, "test")); + } + + private static DefaultEndpointFilterInvocationContext CreateContext( + CreateTemplateFormRequest request, + bool hasPermission = false + ) + { + var httpContext = new DefaultHttpContext { Response = { Body = new MemoryStream() } }; + + if (hasPermission) + { + var identity = new ClaimsIdentity( + [new Claim("permission", EmailPermissions.ManageTemplates)], + "test" + ); + httpContext.User = new ClaimsPrincipal(identity); + } + + return new DefaultEndpointFilterInvocationContext(httpContext, request); + } + + private static void AssertFieldError(object? result, string fieldName) + { + var problem = result.Should().BeOfType().Subject; + problem.StatusCode.Should().Be(422); + var errorsJson = System.Text.Json.JsonSerializer.Serialize( + problem.ProblemDetails.Extensions["errors"] + ); + var errors = System.Text.Json.JsonSerializer.Deserialize>( + errorsJson + )!; + errors + .Should() + .ContainKey(fieldName, $"field '{fieldName}' should have a validation error"); + } +} diff --git a/tests/SimpleModule.Core.Tests/FormRequests/FormRequestEdgeCaseTests.cs b/tests/SimpleModule.Core.Tests/FormRequests/FormRequestEdgeCaseTests.cs new file mode 100644 index 00000000..886450b4 --- /dev/null +++ b/tests/SimpleModule.Core.Tests/FormRequests/FormRequestEdgeCaseTests.cs @@ -0,0 +1,699 @@ +using System.Security.Claims; +using System.Text.Json; +using FluentAssertions; +using FluentValidation; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.HttpResults; +using SimpleModule.Core.FormRequests; +using SimpleModule.Core.Inertia; + +namespace SimpleModule.Core.Tests.FormRequests; + +/// +/// Covers the gaps not exercised by the existing 16 tests: +/// - Empty arguments list and null argument slots +/// - Multiple FormRequest arguments in one endpoint (only the first is checked — the filter loops) +/// - Empty ConfigureRules (no rules — should always pass) +/// - RuleForEach collection validation +/// - Inertia error path (X-Inertia header → 422 JSON with Inertia shape) +/// - Exact JSON shape of the API (non-Inertia) 422 response +/// - Validator caching is type-isolated (two different types get separate validators) +/// - Authorize check receives the actual HttpContext user, not a default principal +/// - Prepare mutates the instance before Validate is called +/// - CancellationToken propagation — ValidateAsync is passed the RequestAborted token +/// +public class FormRequestEdgeCaseTests +{ + private readonly FormRequestEndpointFilter _filter = new(); + + // ─── Empty / null argument slots ──────────────────────────────────────── + + [Fact] + public async Task InvokeAsync_EmptyArgumentList_CallsNext() + { + var nextCalled = false; + var context = CreateContext(); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task InvokeAsync_NullArgumentSlot_SkipsNullAndCallsNext() + { + // A null argument must not throw — the filter casts and checks with `is`, which handles null safely. + var nextCalled = false; + var context = CreateContext((object?)null!); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task InvokeAsync_MixedArgumentsWithFormRequestLast_StillValidates() + { + // The filter must scan all argument positions, not just index 0. + var request = new NoRulesRequest(); + var nextCalled = false; + var context = CreateContext("first-arg", 42, request); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + // ─── Multiple FormRequest arguments ────────────────────────────────────── + + [Fact] + public async Task InvokeAsync_TwoValidFormRequests_CallsNext() + { + var r1 = new NoRulesRequest(); + var r2 = new NoRulesRequest(); + var nextCalled = false; + var context = CreateContext(r1, r2); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task InvokeAsync_FirstFormRequestInvalid_Returns422WithoutCallingNext() + { + // The filter iterates in order; the first invalid request short-circuits. + var invalid = new SingleFieldRequest { Name = "" }; + var valid = new NoRulesRequest(); + var nextCalled = false; + var context = CreateContext(invalid, valid); + + var result = await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeFalse("short-circuit on first invalid request"); + result.Should().BeOfType().Which.StatusCode.Should().Be(422); + } + + [Fact] + public async Task InvokeAsync_FirstFormRequestUnauthorized_Returns403WithoutCheckingSecond() + { + var unauthorized = new AlwaysDenyRequest(); + var valid = new NoRulesRequest(); + var nextCalled = false; + var context = CreateContext(unauthorized, valid); + + var result = await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeFalse(); + result.Should().BeOfType().Which.StatusCode.Should().Be(403); + } + + // ─── Empty ConfigureRules ──────────────────────────────────────────────── + + [Fact] + public async Task InvokeAsync_FormRequestWithNoRules_AlwaysPasses() + { + var request = new NoRulesRequest(); + var nextCalled = false; + var context = CreateContext(request); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + // ─── RuleForEach collection validation ─────────────────────────────────── + + [Fact] + public async Task InvokeAsync_CollectionAllValid_CallsNext() + { + var request = new CollectionRequest { Tags = ["foo", "bar"] }; + var nextCalled = false; + var context = CreateContext(request); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task InvokeAsync_CollectionContainsEmptyElement_Returns422() + { + var request = new CollectionRequest { Tags = ["foo", "", "bar"] }; + var context = CreateContext(request); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + result.Should().BeOfType().Which.StatusCode.Should().Be(422); + } + + [Fact] + public async Task InvokeAsync_EmptyCollection_Returns422() + { + // RuleForEach on an empty list produces no per-element errors, but the outer + // NotEmpty() on the collection itself should catch it. + var request = new CollectionRequest { Tags = [] }; + var context = CreateContext(request); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + result.Should().BeOfType().Which.StatusCode.Should().Be(422); + } + + // ─── Inertia error path ─────────────────────────────────────────────────── + + [Fact] + public async Task InvokeAsync_InertiaRequest_InvalidRequest_Returns422WithInertiaShape() + { + var request = new SingleFieldRequest { Name = "" }; + var httpContext = new DefaultHttpContext(); + httpContext.Response.Body = new MemoryStream(); + httpContext.Request.Headers[InertiaHttpExtensions.InertiaHeader] = "true"; + + var context = new DefaultEndpointFilterInvocationContext(httpContext, request); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + // Must NOT be a ProblemHttpResult — Inertia uses its own IResult + result + .Should() + .NotBeOfType("Inertia uses a custom IResult, not ProblemDetails"); + + // Execute the result so headers and body are written + await ((IResult)result!).ExecuteAsync(httpContext); + + httpContext.Response.StatusCode.Should().Be(422); + httpContext + .Response.Headers[InertiaHttpExtensions.InertiaHeader] + .ToString() + .Should() + .Be("true"); + httpContext.Response.ContentType.Should().Contain("application/json"); + + httpContext.Response.Body.Seek(0, SeekOrigin.Begin); + string body; + using (var reader = new StreamReader(httpContext.Response.Body)) + body = await reader.ReadToEndAsync(); + using var doc = JsonDocument.Parse(body); + + // Must have component, props.status, props.errors + doc.RootElement.GetProperty("component").GetString().Should().Be("Error/422"); + var props = doc.RootElement.GetProperty("props"); + props.GetProperty("status").GetInt32().Should().Be(422); + props.GetProperty("errors").ValueKind.Should().Be(JsonValueKind.Object); + } + + [Fact] + public async Task InvokeAsync_InertiaRequest_InvalidRequest_ErrorsContainFieldName() + { + var request = new SingleFieldRequest { Name = "" }; + var httpContext = new DefaultHttpContext(); + httpContext.Response.Body = new MemoryStream(); + httpContext.Request.Headers[InertiaHttpExtensions.InertiaHeader] = "true"; + + var context = new DefaultEndpointFilterInvocationContext(httpContext, request); + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + await ((IResult)result!).ExecuteAsync(httpContext); + + httpContext.Response.Body.Seek(0, SeekOrigin.Begin); + string body; + using (var reader = new StreamReader(httpContext.Response.Body)) + body = await reader.ReadToEndAsync(); + using var doc = JsonDocument.Parse(body); + + var errors = doc.RootElement.GetProperty("props").GetProperty("errors"); + errors.EnumerateObject().Select(p => p.Name).Should().Contain("Name"); + } + + [Fact] + public async Task InvokeAsync_InertiaRequest_ValidRequest_CallsNext() + { + // Inertia header must not interfere with valid requests. + var request = new SingleFieldRequest { Name = "Alice" }; + var nextCalled = false; + var httpContext = new DefaultHttpContext(); + httpContext.Response.Body = new MemoryStream(); + httpContext.Request.Headers[InertiaHttpExtensions.InertiaHeader] = "true"; + + var context = new DefaultEndpointFilterInvocationContext(httpContext, request); + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + // ─── API (non-Inertia) 422 exact shape ─────────────────────────────────── + + [Fact] + public async Task InvokeAsync_ApiRequest_InvalidRequest_Returns422WithErrorsExtension() + { + var request = new SingleFieldRequest { Name = "" }; + var context = CreateContext(request); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + var problem = result.Should().BeOfType().Subject; + problem.StatusCode.Should().Be(422); + problem.ProblemDetails.Title.Should().Be("Validation Error"); + problem.ProblemDetails.Detail.Should().Be("One or more validation errors occurred."); + problem.ProblemDetails.Extensions.Should().ContainKey("errors"); + + var errorsJson = JsonSerializer.Serialize(problem.ProblemDetails.Extensions["errors"]); + var errors = JsonSerializer.Deserialize>(errorsJson)!; + errors.Should().ContainKey("Name"); + errors["Name"].Should().NotBeEmpty(); + } + + [Fact] + public async Task InvokeAsync_ApiRequest_403Shape_HasForbiddenTitle() + { + var request = new AlwaysDenyRequest(); + var context = CreateContext(request); + + var result = await _filter.InvokeAsync( + context, + _ => ValueTask.FromResult(Results.Ok()) + ); + + var problem = result.Should().BeOfType().Subject; + problem.StatusCode.Should().Be(403); + problem.ProblemDetails.Title.Should().Be("Forbidden"); + } + + // ─── Validator caching is type-isolated ────────────────────────────────── + + [Fact] + public async Task ValidatorCache_TwoDifferentTypes_EachGetSeparateValidator() + { + // Both types pass through valid values. If the cache incorrectly shared a validator, + // one type's rules would bleed into the other and cause unexpected failures. + var r1 = new SingleFieldRequest { Name = "valid" }; + var r2 = new NoRulesRequest(); + + var nextCount = 0; + ValueTask Next(EndpointFilterInvocationContext _) + { + nextCount++; + return ValueTask.FromResult(Results.Ok()); + } + + await _filter.InvokeAsync(CreateContext(r1), Next); + await _filter.InvokeAsync(CreateContext(r2), Next); + + nextCount.Should().Be(2, "both requests should independently pass through"); + } + + [Fact] + public async Task ValidatorCache_SameTypeTwiceWithSameFilter_BothUseTheCachedValidator() + { + // Create two instances of the same type; both should exercise the validator from cache + // (only one ConfigureRules call happens). If caching is broken, the second call might + // rebuild and create a different validator state — but the visible effect is the same, + // so this test confirms stability (no exception, correct results). + var r1 = new SingleFieldRequest { Name = "alice" }; + var r2 = new SingleFieldRequest { Name = "bob" }; + + var nextCount = 0; + ValueTask Next(EndpointFilterInvocationContext _) + { + nextCount++; + return ValueTask.FromResult(Results.Ok()); + } + + await _filter.InvokeAsync(CreateContext(r1), Next); + await _filter.InvokeAsync(CreateContext(r2), Next); + + nextCount.Should().Be(2); + } + + // ─── Authorize receives actual user ───────────────────────────────────── + + [Fact] + public async Task InvokeAsync_AuthorizeReceivesHttpContextUser_NotDefaultPrincipal() + { + ClaimsPrincipal? capturedUser = null; + var request = new CapturingAuthRequest(u => capturedUser = u) { Name = "Test" }; + + var httpContext = new DefaultHttpContext(); + httpContext.Response.Body = new MemoryStream(); + var identity = new ClaimsIdentity([new Claim("role", "admin")], "test"); + httpContext.User = new ClaimsPrincipal(identity); + + var context = new DefaultEndpointFilterInvocationContext(httpContext, request); + + await _filter.InvokeAsync(context, _ => ValueTask.FromResult(Results.Ok())); + + capturedUser.Should().NotBeNull(); + capturedUser!.Claims.Should().Contain(c => c.Type == "role" && c.Value == "admin"); + } + + // ─── Prepare always runs before Validate ───────────────────────────────── + + [Fact] + public async Task InvokeAsync_PrepareRunsBeforeValidation_SideEffectsVisible() + { + // PrepareTrackingRequest records whether Prepare was called before validation. + // The test asserts that the instance was mutated before rules ran. + var request = new PrepareTrackingRequest { Value = " trimmed " }; + var context = CreateContext(request); + + await _filter.InvokeAsync(context, _ => ValueTask.FromResult(Results.Ok())); + + request.PrepareWasCalled.Should().BeTrue(); + request.Value.Should().Be("trimmed", "Prepare should have trimmed the value"); + } + + [Fact] + public async Task InvokeAsync_PrepareNormalizes_ThenValidationSees_NormalizedValue() + { + // If validation ran on the raw value before Prepare, this would fail the regex. + // The fact that it passes confirms Prepare ran first. + var request = new PrepareNormalizeRequest { Code = " ABC 123 " }; + var context = CreateContext(request); + var nextCalled = false; + + await _filter.InvokeAsync( + context, + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue("normalized value ABC123 should pass the digit-only rule"); + request.Code.Should().Be("ABC123"); + } + + // ─── CancellationToken propagation ─────────────────────────────────────── + + [Fact] + public async Task InvokeAsync_CancelledToken_ThrowsOperationCancelledException() + { + var request = new SlowValidationRequest { Name = "test" }; + using var cts = new CancellationTokenSource(); + + var httpContext = new DefaultHttpContext(); + httpContext.Response.Body = new MemoryStream(); + httpContext.RequestAborted = cts.Token; + + var context = new DefaultEndpointFilterInvocationContext(httpContext, request); + await cts.CancelAsync(); + + var act = async () => + await _filter.InvokeAsync(context, _ => ValueTask.FromResult(Results.Ok())); + + await act.Should().ThrowAsync(); + } + + // ─── FormRequestAttribute marker ───────────────────────────────────────── + + [Fact] + public void FormRequestAttribute_IsAttributeUsageRestrictedToClass() + { + var usage = typeof(FormRequestAttribute) + .GetCustomAttributes(typeof(AttributeUsageAttribute), false) + .Cast() + .Single(); + + usage.ValidOn.Should().Be(AttributeTargets.Class); + usage.AllowMultiple.Should().BeFalse(); + usage.Inherited.Should().BeFalse(); + } + + // ─── RuleConfigurator surface area ─────────────────────────────────────── + // Build() is internal, so we exercise RuleConfigurator through the filter pipeline. + + [Fact] + public async Task RuleConfigurator_RuleFor_ValidValue_PassesThroughFilter() + { + var request = new CustomMessageRequest { Name = "valid" }; + var nextCalled = false; + + await _filter.InvokeAsync( + CreateContext(request), + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + [Fact] + public async Task RuleConfigurator_RuleFor_CustomMessage_AppearsInErrors() + { + var request = new CustomMessageRequest { Name = "" }; + + var result = await _filter.InvokeAsync( + CreateContext(request), + _ => ValueTask.FromResult(Results.Ok()) + ); + + var problem = result.Should().BeOfType().Subject; + var errorsJson = JsonSerializer.Serialize(problem.ProblemDetails.Extensions["errors"]); + var errors = JsonSerializer.Deserialize>(errorsJson)!; + errors["Name"].Should().Contain("Name is required for CustomMessageRequest."); + } + + [Fact] + public async Task RuleConfigurator_RuleForEach_ValidCollection_PassesThroughFilter() + { + // This uses CollectionRequest which has both RuleFor(Tags).NotEmpty() and RuleForEach. + var request = new CollectionRequest { Tags = ["alpha", "beta"] }; + var nextCalled = false; + + await _filter.InvokeAsync( + CreateContext(request), + _ => + { + nextCalled = true; + return ValueTask.FromResult(Results.Ok()); + } + ); + + nextCalled.Should().BeTrue(); + } + + // ─── Helpers ───────────────────────────────────────────────────────────── + + private static DefaultEndpointFilterInvocationContext CreateContext(params object?[] arguments) + { + var httpContext = new DefaultHttpContext { Response = { Body = new MemoryStream() } }; + return new DefaultEndpointFilterInvocationContext(httpContext, arguments); + } + + // ─── Test FormRequest types ─────────────────────────────────────────────── + + [FormRequest] + private sealed class NoRulesRequest : FormRequest + { + protected override void ConfigureRules(RuleConfigurator rules) + { + // Intentionally empty — must always pass validation. + } + } + + [FormRequest] + private sealed class SingleFieldRequest : FormRequest + { + public string Name { get; set; } = ""; + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty(); + } + } + + [FormRequest] + private sealed class AlwaysDenyRequest : FormRequest + { + public override bool Authorize(ClaimsPrincipal user) => false; + + protected override void ConfigureRules(RuleConfigurator rules) { } + } + + [FormRequest] + private sealed class CollectionRequest : FormRequest + { + public List Tags { get; set; } = []; + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Tags).NotEmpty().WithMessage("Tags cannot be empty"); + rules.RuleForEach(x => x.Tags).NotEmpty().WithMessage("Each tag must be non-empty"); + } + } + + [FormRequest] + private sealed class PrepareTrackingRequest : FormRequest + { + public string Value { get; set; } = ""; + public bool PrepareWasCalled { get; private set; } + + public override void Prepare() + { + PrepareWasCalled = true; + Value = Value.Trim(); + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Value).NotEmpty(); + } + } + + [FormRequest] + private sealed class PrepareNormalizeRequest : FormRequest + { + public string Code { get; set; } = ""; + + public override void Prepare() + { + Code = Code.Replace(" ", "", StringComparison.Ordinal).Trim(); + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + // Only alphanumeric — raw " ABC 123 " would fail this; normalized "ABC123" passes. + rules.RuleFor(x => x.Code).NotEmpty().Matches("^[A-Z0-9]+$"); + } + } + + [FormRequest] + private sealed class CapturingAuthRequest : FormRequest + { + private readonly Action _capture; + + public string Name { get; set; } = ""; + + public CapturingAuthRequest(Action capture) => _capture = capture; + + public override bool Authorize(ClaimsPrincipal user) + { + _capture(user); + return true; + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty(); + } + } + + [FormRequest] + private sealed class CustomMessageRequest : FormRequest + { + public string Name { get; set; } = ""; + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules + .RuleFor(x => x.Name) + .NotEmpty() + .WithMessage("Name is required for CustomMessageRequest."); + } + } + + [FormRequest] + private sealed class SlowValidationRequest : FormRequest + { + public string Name { get; set; } = ""; + + protected override void ConfigureRules(RuleConfigurator rules) + { + // MustAsync triggers actual async validation, so the cancellation token is honored. + rules + .RuleFor(x => x.Name) + .NotEmpty() + .MustAsync( + async (_, ct) => + { + await Task.Delay(TimeSpan.FromSeconds(10), ct); + return true; + } + ); + } + } +} diff --git a/tests/SimpleModule.Generator.Tests/DiagnosticCatalogTests.cs b/tests/SimpleModule.Generator.Tests/DiagnosticCatalogTests.cs index a36d8091..5bb100b7 100644 --- a/tests/SimpleModule.Generator.Tests/DiagnosticCatalogTests.cs +++ b/tests/SimpleModule.Generator.Tests/DiagnosticCatalogTests.cs @@ -177,6 +177,12 @@ private static readonly Dictionary< DiagnosticSeverity.Error, "SimpleModule.Generator" ), + ["FormRequestNotSealed"] = ("SM0056", DiagnosticSeverity.Error, "SimpleModule.Generator"), + ["FormRequestDoesNotExtendBase"] = ( + "SM0057", + DiagnosticSeverity.Error, + "SimpleModule.Generator" + ), }; [Fact] diff --git a/tests/SimpleModule.Generator.Tests/EndpointExtensionsEmitterTests.cs b/tests/SimpleModule.Generator.Tests/EndpointExtensionsEmitterTests.cs index b2a5cd6f..6df94c0c 100644 --- a/tests/SimpleModule.Generator.Tests/EndpointExtensionsEmitterTests.cs +++ b/tests/SimpleModule.Generator.Tests/EndpointExtensionsEmitterTests.cs @@ -39,7 +39,7 @@ public void Map(IEndpointRouteBuilder app) endpointExt .Should() .Contain( - "var group = app.MapGroup(\"/api/products\").WithTags(\"Products\").RequireAuthorization();" + "var group = app.MapGroup(\"/api/products\").WithTags(\"Products\").RequireAuthorization().AddFormRequestFilter();" ); endpointExt.Should().Contain("new global::TestApp.Endpoints.ListEndpoint().Map(group);"); } @@ -75,8 +75,12 @@ public void Map(IEndpointRouteBuilder app) var endpointExt = GetGeneratedSource(result, "EndpointExtensions.g.cs"); - endpointExt.Should().Contain("new global::TestApp.Endpoints.PingEndpoint().Map(app);"); - endpointExt.Should().NotContain("MapGroup"); + endpointExt + .Should() + .Contain( + "var group = app.MapGroup(\"\").WithTags(\"Misc\").RequireAuthorization().AddFormRequestFilter();" + ); + endpointExt.Should().Contain("new global::TestApp.Endpoints.PingEndpoint().Map(group);"); } [Fact] @@ -115,11 +119,11 @@ public void Map(IEndpointRouteBuilder app) // Should NOT auto-register endpoints because HasConfigureEndpoints is true endpointExt.Should().NotContain("new global::TestApp.Endpoints.AutoEndpoint()"); - // Should use the escape hatch instead + // Should use the escape hatch wrapped in a FormRequestFilter group endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_CustomModule).ConfigureEndpoints(app);" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_CustomModule).ConfigureEndpoints(_escapeGroup);" ); } @@ -149,7 +153,7 @@ public void ConfigureEndpoints(IEndpointRouteBuilder endpoints) { } endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_ManualModule).ConfigureEndpoints(app);" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_ManualModule).ConfigureEndpoints(_escapeGroup);" ); } diff --git a/tests/SimpleModule.Generator.Tests/FormRequestDiagnosticTests.cs b/tests/SimpleModule.Generator.Tests/FormRequestDiagnosticTests.cs new file mode 100644 index 00000000..7e653763 --- /dev/null +++ b/tests/SimpleModule.Generator.Tests/FormRequestDiagnosticTests.cs @@ -0,0 +1,423 @@ +using FluentAssertions; +using SimpleModule.Generator.Tests.Helpers; + +namespace SimpleModule.Generator.Tests; + +/// +/// Tests for SM0056 (FormRequest must be sealed) and SM0057 (FormRequest must extend FormRequest<TSelf>). +/// The generator only runs FormRequestChecks when at least one [Module] exists in the compilation +/// (SymbolDiscovery returns DiscoveryData.Empty when no modules are found), so every test source +/// includes a minimal module declaration. +/// +public class FormRequestDiagnosticTests +{ + #region SM0056: FormRequest class must be sealed + + [Fact] + public void SM0056_NonSealedFormRequest_ReportsError() + { + var source = """ + using SimpleModule.Core; + using SimpleModule.Core.FormRequests; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule { } + } + + namespace TestApp.FormRequests + { + [FormRequest] + public class OpenFormRequest : FormRequest + { + public string Name { get; set; } = ""; + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty(); + } + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilationWithFormRequestSupport(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Should().Contain(d => d.Id == "SM0056"); + var diag = diagnostics.First(d => d.Id == "SM0056"); + diag.GetMessage(System.Globalization.CultureInfo.InvariantCulture) + .Should() + .Contain("OpenFormRequest"); + } + + [Fact] + public void SM0056_SealedFormRequest_NoDiagnostic() + { + var source = """ + using SimpleModule.Core; + using SimpleModule.Core.FormRequests; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule { } + } + + namespace TestApp.FormRequests + { + [FormRequest] + public sealed class SealedFormRequest : FormRequest + { + public string Name { get; set; } = ""; + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty(); + } + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilationWithFormRequestSupport(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Should().NotContain(d => d.Id == "SM0056"); + } + + [Fact] + public void SM0056_MultipleNonSealedFormRequests_ReportsErrorForEach() + { + var source = """ + using SimpleModule.Core; + using SimpleModule.Core.FormRequests; + + namespace TestApp + { + [Module("Orders")] + public class OrdersModule : IModule { } + } + + namespace TestApp.FormRequests + { + [FormRequest] + public class FirstRequest : FormRequest + { + protected override void ConfigureRules(RuleConfigurator rules) { } + } + + [FormRequest] + public class SecondRequest : FormRequest + { + protected override void ConfigureRules(RuleConfigurator rules) { } + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilationWithFormRequestSupport(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Where(d => d.Id == "SM0056").Should().HaveCount(2); + } + + #endregion + + #region SM0057: FormRequest class must extend FormRequest + + [Fact] + public void SM0057_FormRequestWithoutBase_ReportsError() + { + var source = """ + using SimpleModule.Core; + using SimpleModule.Core.FormRequests; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule { } + } + + namespace TestApp.FormRequests + { + [FormRequest] + public sealed class BadFormRequest + { + public string Name { get; set; } = ""; + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilationWithFormRequestSupport(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Should().Contain(d => d.Id == "SM0057"); + var diag = diagnostics.First(d => d.Id == "SM0057"); + diag.GetMessage(System.Globalization.CultureInfo.InvariantCulture) + .Should() + .Contain("BadFormRequest"); + } + + [Fact] + public void SM0057_FormRequestWithWrongBase_ReportsError() + { + // Has [FormRequest] but extends an arbitrary class, not FormRequest + var source = """ + using SimpleModule.Core; + using SimpleModule.Core.FormRequests; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule { } + } + + namespace TestApp.FormRequests + { + public abstract class SomeBase { } + + [FormRequest] + public sealed class WrongBaseRequest : SomeBase + { + public string Name { get; set; } = ""; + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilationWithFormRequestSupport(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Should().Contain(d => d.Id == "SM0057"); + } + + [Fact] + public void SM0057_ProperFormRequest_NoDiagnostic() + { + var source = """ + using SimpleModule.Core; + using SimpleModule.Core.FormRequests; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule { } + } + + namespace TestApp.FormRequests + { + [FormRequest] + public sealed class GoodRequest : FormRequest + { + public string Value { get; set; } = ""; + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Value).NotEmpty(); + } + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilationWithFormRequestSupport(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Should().NotContain(d => d.Id == "SM0057"); + } + + [Fact] + public void SM0056_And_SM0057_BothFire_WhenClassHasAttributeButNoBaseAndIsNotSealed() + { + // A class with [FormRequest] that is neither sealed nor extending the base class should + // produce both SM0056 and SM0057 simultaneously. + var source = """ + using SimpleModule.Core; + using SimpleModule.Core.FormRequests; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule { } + } + + namespace TestApp.FormRequests + { + [FormRequest] + public class DoublyBadRequest + { + public string Name { get; set; } = ""; + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilationWithFormRequestSupport(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Should().Contain(d => d.Id == "SM0056"); + diagnostics.Should().Contain(d => d.Id == "SM0057"); + } + + #endregion + + #region No false positives on valid FormRequest types + + [Fact] + public void ValidFormRequest_ProducesNoFormRequestDiagnostics() + { + var source = """ + using SimpleModule.Core; + using SimpleModule.Core.FormRequests; + + namespace TestApp + { + [Module("Catalog")] + public class CatalogModule : IModule { } + } + + namespace TestApp.FormRequests + { + [FormRequest] + public sealed class CreateProductRequest : FormRequest + { + public string Name { get; set; } = ""; + public decimal Price { get; set; } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty().MaximumLength(200); + rules.RuleFor(x => x.Price).GreaterThan(0); + } + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilationWithFormRequestSupport(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics + .Where(d => d.Id == "SM0056" || d.Id == "SM0057") + .Should() + .BeEmpty("a correctly-defined FormRequest should produce no SM0056/SM0057 diagnostics"); + } + + [Fact] + public void TypeWithoutFormRequestAttribute_NotSealed_NoDiagnostic() + { + // A class that extends FormRequest but lacks the [FormRequest] attribute + // should not be reported — the attribute is the trigger for the finder. + var source = """ + using SimpleModule.Core; + using SimpleModule.Core.FormRequests; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule { } + } + + namespace TestApp.FormRequests + { + // No [FormRequest] attribute — not subject to SM0056/SM0057 checks. + public class UnmarkedRequest : FormRequest + { + public string Name { get; set; } = ""; + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules.RuleFor(x => x.Name).NotEmpty(); + } + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilationWithFormRequestSupport(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Should().NotContain(d => d.Id == "SM0056"); + diagnostics.Should().NotContain(d => d.Id == "SM0057"); + } + + #endregion + + #region Generator emits AddFormRequestFilter on route groups + + [Fact] + public void Module_WithRoutePrefix_GeneratesAddFormRequestFilterOnGroup() + { + var source = """ + using Microsoft.AspNetCore.Builder; + using Microsoft.AspNetCore.Routing; + using SimpleModule.Core; + + namespace TestApp + { + [Module("Orders", RoutePrefix = "/api/orders")] + public class OrdersModule : IModule { } + } + + namespace TestApp.Endpoints + { + public class ListOrdersEndpoint : IEndpoint + { + public void Map(IEndpointRouteBuilder app) + { + app.MapGet("/", () => "orders"); + } + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilation(source); + var result = GeneratorTestHelper.RunGenerator(compilation); + + var endpointExt = result + .GeneratedTrees.First(t => + t.FilePath.EndsWith("EndpointExtensions.g.cs", StringComparison.Ordinal) + ) + .GetText() + .ToString(); + + // The filter must be applied on every route group so FormRequest validation fires automatically. + endpointExt.Should().Contain(".AddFormRequestFilter()"); + } + + [Fact] + public void Module_WithoutRoutePrefix_StillGeneratesAddFormRequestFilter() + { + var source = """ + using Microsoft.AspNetCore.Builder; + using Microsoft.AspNetCore.Routing; + using SimpleModule.Core; + + namespace TestApp + { + [Module("Misc")] + public class MiscModule : IModule { } + } + + namespace TestApp.Endpoints + { + public class PingEndpoint : IEndpoint + { + public void Map(IEndpointRouteBuilder app) + { + app.MapGet("/ping", () => "pong"); + } + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilation(source); + var result = GeneratorTestHelper.RunGenerator(compilation); + + var endpointExt = result + .GeneratedTrees.First(t => + t.FilePath.EndsWith("EndpointExtensions.g.cs", StringComparison.Ordinal) + ) + .GetText() + .ToString(); + + // Even without RoutePrefix, a group is created with the FormRequest filter + endpointExt.Should().Contain(".AddFormRequestFilter()"); + } + + #endregion +} diff --git a/tests/SimpleModule.Generator.Tests/Helpers/GeneratorTestHelper.cs b/tests/SimpleModule.Generator.Tests/Helpers/GeneratorTestHelper.cs index 1a45f492..581293c8 100644 --- a/tests/SimpleModule.Generator.Tests/Helpers/GeneratorTestHelper.cs +++ b/tests/SimpleModule.Generator.Tests/Helpers/GeneratorTestHelper.cs @@ -85,6 +85,25 @@ public static CSharpCompilation CreateCompilation(params string[] sources) ); } + /// + /// Creates a compilation that includes FluentValidation and the SimpleModule.Core assembly + /// so that FormRequest<T> and RuleConfigurator<T> are resolvable by the generator. + /// + public static CSharpCompilation CreateCompilationWithFormRequestSupport(params string[] sources) + { + var compilation = CreateCompilation(sources); + + var extraRefs = new List + { + MetadataReference.CreateFromFile(typeof(FluentValidation.IValidator).Assembly.Location), + MetadataReference.CreateFromFile( + typeof(FluentValidation.AbstractValidator<>).Assembly.Location + ), + }; + + return compilation.AddReferences(extraRefs); + } + public static CSharpCompilation CreateCompilationWithEfCore(params string[] sources) { var compilation = CreateCompilation(sources); diff --git a/tests/SimpleModule.Generator.Tests/ModuleDiscovererGeneratorTests.cs b/tests/SimpleModule.Generator.Tests/ModuleDiscovererGeneratorTests.cs index b740d6d5..ee71cb96 100644 --- a/tests/SimpleModule.Generator.Tests/ModuleDiscovererGeneratorTests.cs +++ b/tests/SimpleModule.Generator.Tests/ModuleDiscovererGeneratorTests.cs @@ -56,7 +56,7 @@ public void ConfigureEndpoints(IEndpointRouteBuilder endpoints) { } endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_TestModule).ConfigureEndpoints(app)" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_TestModule).ConfigureEndpoints(_escapeGroup)" ); } @@ -96,7 +96,7 @@ public void ConfigureEndpoints(IEndpointRouteBuilder endpoints) { } endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_EndpointOnlyModule).ConfigureEndpoints(app)" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_EndpointOnlyModule).ConfigureEndpoints(_escapeGroup)" ); } diff --git a/tests/SimpleModule.Generator.Tests/ModuleExtensionsGenerationTests.cs b/tests/SimpleModule.Generator.Tests/ModuleExtensionsGenerationTests.cs index a4165d36..25b1da7f 100644 --- a/tests/SimpleModule.Generator.Tests/ModuleExtensionsGenerationTests.cs +++ b/tests/SimpleModule.Generator.Tests/ModuleExtensionsGenerationTests.cs @@ -111,7 +111,7 @@ public void ConfigureEndpoints(IEndpointRouteBuilder endpoints) { } endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_TestModule).ConfigureEndpoints(app)" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_TestModule).ConfigureEndpoints(_escapeGroup)" ); } From 32fa8b3580285a1e9be7a6e10444c696389019bc Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sun, 24 May 2026 15:56:25 +0200 Subject: [PATCH 3/6] =?UTF-8?q?fix(core):=20address=20code=20review=20find?= =?UTF-8?q?ings=20=E2=80=94=20dedup,=20auth,=20caching,=20API=20gaps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix duplicate TS interfaces when both [Dto] and [FormRequest] on same class (DtoFinder now deduplicates FQNs before the [FormRequest] scan) - Restore .RequirePermission() on CreateTemplateEndpoint for ASP.NET auth metadata visibility (OpenAPI/Swagger, policy-based audit); remove redundant FormRequest.Authorize() override from CreateTemplateFormRequest - Revert escape-hatch ConfigureEndpoints to receive WebApplication (not RouteGroupBuilder) — avoids breaking contract for modules that cast to IApplicationBuilder - ValidateRulesAsync now calls Prepare() before validation, consistent with the filter pipeline - Add When/Unless forwarding to RuleConfigurator for grouped conditional rules - Replace ConcurrentDictionary validator cache with volatile + Interlocked (single-entry-per-type, simpler, avoids redundant dictionary overhead) --- .../FormRequests/FormRequest.cs | 6 ++- .../FormRequests/FormRequest{T}.cs | 22 +++++---- .../FormRequests/RuleConfigurator.cs | 5 +++ .../Discovery/Finders/DtoFinder.cs | 17 ++++++- .../Emitters/EndpointExtensionsEmitter.cs | 8 +--- .../Templates/CreateTemplateEndpoint.cs | 36 ++++++++------- .../FormRequests/CreateTemplateFormRequest.cs | 6 --- .../Unit/CreateTemplateFormRequestTests.cs | 45 ++----------------- .../EndpointExtensionsEmitterTests.cs | 6 +-- .../ModuleDiscovererGeneratorTests.cs | 4 +- .../ModuleExtensionsGenerationTests.cs | 2 +- 11 files changed, 65 insertions(+), 92 deletions(-) diff --git a/framework/SimpleModule.Core/FormRequests/FormRequest.cs b/framework/SimpleModule.Core/FormRequests/FormRequest.cs index 08446442..fab6feb3 100644 --- a/framework/SimpleModule.Core/FormRequests/FormRequest.cs +++ b/framework/SimpleModule.Core/FormRequests/FormRequest.cs @@ -11,7 +11,11 @@ public virtual void Prepare() { } public async Task ValidateRulesAsync( CancellationToken cancellationToken = default - ) => await ValidateAsync(cancellationToken); + ) + { + Prepare(); + return await ValidateAsync(cancellationToken); + } internal abstract Task ValidateAsync(CancellationToken cancellationToken); } diff --git a/framework/SimpleModule.Core/FormRequests/FormRequest{T}.cs b/framework/SimpleModule.Core/FormRequests/FormRequest{T}.cs index 6f360b3c..a0745ce0 100644 --- a/framework/SimpleModule.Core/FormRequests/FormRequest{T}.cs +++ b/framework/SimpleModule.Core/FormRequests/FormRequest{T}.cs @@ -1,4 +1,3 @@ -using System.Collections.Concurrent; using FluentValidation; using FluentValidation.Results; @@ -7,8 +6,7 @@ namespace SimpleModule.Core.FormRequests; public abstract class FormRequest : FormRequest where TSelf : FormRequest { - private static readonly ConcurrentDictionary> ValidatorCache = - new(); + private static volatile InlineValidator? _cachedValidator; protected abstract void ConfigureRules(RuleConfigurator rules); @@ -16,15 +14,15 @@ internal sealed override async Task ValidateAsync( CancellationToken cancellationToken ) { - var validator = ValidatorCache.GetOrAdd( - typeof(TSelf), - _ => - { - var configurator = new RuleConfigurator(); - ConfigureRules(configurator); - return configurator.Build(); - } - ); + var validator = _cachedValidator; + if (validator is null) + { + var configurator = new RuleConfigurator(); + ConfigureRules(configurator); + validator = configurator.Build(); + Interlocked.CompareExchange(ref _cachedValidator, validator, null); + validator = _cachedValidator; + } return await validator.ValidateAsync((TSelf)this, cancellationToken); } diff --git a/framework/SimpleModule.Core/FormRequests/RuleConfigurator.cs b/framework/SimpleModule.Core/FormRequests/RuleConfigurator.cs index b8f82eba..70e38b51 100644 --- a/framework/SimpleModule.Core/FormRequests/RuleConfigurator.cs +++ b/framework/SimpleModule.Core/FormRequests/RuleConfigurator.cs @@ -16,5 +16,10 @@ public IRuleBuilderInitialCollection RuleForEach( Expression>> expression ) => _validator.RuleForEach(expression); + public void When(Func condition, Action action) => _validator.When(condition, action); + + public void Unless(Func condition, Action action) => + _validator.Unless(condition, action); + internal InlineValidator Build() => _validator; } diff --git a/framework/SimpleModule.Generator/Discovery/Finders/DtoFinder.cs b/framework/SimpleModule.Generator/Discovery/Finders/DtoFinder.cs index 10b1e70e..2e8cbbae 100644 --- a/framework/SimpleModule.Generator/Discovery/Finders/DtoFinder.cs +++ b/framework/SimpleModule.Generator/Discovery/Finders/DtoFinder.cs @@ -241,13 +241,20 @@ CancellationToken cancellationToken if (symbols.FormRequestAttribute is null) return; + // Build a set of already-discovered FQNs so types with both [Dto] and + // [FormRequest] are not added twice (which would cause duplicate TS exports). + var existingFqns = new HashSet(); + foreach (var d in dtoTypes) + existingFqns.Add(d.FullyQualifiedName); + + var formRequestDtos = new List(); foreach (var assemblySymbol in refAssemblies) { cancellationToken.ThrowIfCancellationRequested(); FindDtoTypes( assemblySymbol.GlobalNamespace, symbols.FormRequestAttribute, - dtoTypes, + formRequestDtos, cancellationToken ); } @@ -255,8 +262,14 @@ CancellationToken cancellationToken FindDtoTypes( hostGlobalNamespace, symbols.FormRequestAttribute, - dtoTypes, + formRequestDtos, cancellationToken ); + + foreach (var dto in formRequestDtos) + { + if (!existingFqns.Contains(dto.FullyQualifiedName)) + dtoTypes.Add(dto); + } } } diff --git a/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs b/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs index 6c4270d5..304d06a2 100644 --- a/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs @@ -98,19 +98,13 @@ public void Emit(SourceProductionContext context, DiscoveryData data) } // Manual ConfigureEndpoints (escape hatch) - // Wrap in a group with FormRequestFilter so FormRequest types work in escape-hatch modules. foreach (var module in modules.Where(m => m.HasConfigureEndpoints)) { var fieldName = TypeMappingHelpers.GetModuleFieldName(module.FullyQualifiedName); sb.AppendLine(); - sb.AppendLine(" {"); - sb.AppendLine( - $" var _escapeGroup = app.MapGroup(\"\").AddFormRequestFilter();" - ); sb.AppendLine( - $" ((global::SimpleModule.Core.IModule)ModuleExtensions.{fieldName}).ConfigureEndpoints(_escapeGroup);" + $" ((global::SimpleModule.Core.IModule)ModuleExtensions.{fieldName}).ConfigureEndpoints(app);" ); - sb.AppendLine(" }"); } sb.AppendLine(); diff --git a/modules/Email/src/SimpleModule.Email/Endpoints/Templates/CreateTemplateEndpoint.cs b/modules/Email/src/SimpleModule.Email/Endpoints/Templates/CreateTemplateEndpoint.cs index adf69be1..0ae2e7c1 100644 --- a/modules/Email/src/SimpleModule.Email/Endpoints/Templates/CreateTemplateEndpoint.cs +++ b/modules/Email/src/SimpleModule.Email/Endpoints/Templates/CreateTemplateEndpoint.cs @@ -1,6 +1,7 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Core.Endpoints; using SimpleModule.Email.Contracts; using SimpleModule.Email.FormRequests; @@ -14,23 +15,24 @@ public class CreateTemplateEndpoint : IEndpoint public void Map(IEndpointRouteBuilder app) => app.MapPost( - Route, - async (CreateTemplateFormRequest request, IEmailContracts emailContracts) => - { - var dto = new CreateEmailTemplateRequest + Route, + async (CreateTemplateFormRequest request, IEmailContracts emailContracts) => { - Name = request.Name, - Slug = request.Slug, - Subject = request.Subject, - Body = request.Body, - IsHtml = request.IsHtml, - DefaultReplyTo = request.DefaultReplyTo, - }; + var dto = new CreateEmailTemplateRequest + { + Name = request.Name, + Slug = request.Slug, + Subject = request.Subject, + Body = request.Body, + IsHtml = request.IsHtml, + DefaultReplyTo = request.DefaultReplyTo, + }; - return await CrudEndpoints.Create( - () => emailContracts.CreateTemplateAsync(dto), - t => $"/api/email/templates/{t.Id.Value}" - ); - } - ); + return await CrudEndpoints.Create( + () => emailContracts.CreateTemplateAsync(dto), + t => $"/api/email/templates/{t.Id.Value}" + ); + } + ) + .RequirePermission(EmailPermissions.ManageTemplates); } diff --git a/modules/Email/src/SimpleModule.Email/FormRequests/CreateTemplateFormRequest.cs b/modules/Email/src/SimpleModule.Email/FormRequests/CreateTemplateFormRequest.cs index 3e73aec1..ea07b9ba 100644 --- a/modules/Email/src/SimpleModule.Email/FormRequests/CreateTemplateFormRequest.cs +++ b/modules/Email/src/SimpleModule.Email/FormRequests/CreateTemplateFormRequest.cs @@ -1,8 +1,5 @@ -using System.Security.Claims; using System.Text.RegularExpressions; using FluentValidation; -using SimpleModule.Core.Authorization; -using SimpleModule.Core.Extensions; using SimpleModule.Core.FormRequests; namespace SimpleModule.Email.FormRequests; @@ -17,9 +14,6 @@ public sealed partial class CreateTemplateFormRequest : FormRequest - user.HasPermission(EmailPermissions.ManageTemplates); - public override void Prepare() { Name = Name.Trim(); diff --git a/modules/Email/tests/SimpleModule.Email.Tests/Unit/CreateTemplateFormRequestTests.cs b/modules/Email/tests/SimpleModule.Email.Tests/Unit/CreateTemplateFormRequestTests.cs index 238da18d..6d98f722 100644 --- a/modules/Email/tests/SimpleModule.Email.Tests/Unit/CreateTemplateFormRequestTests.cs +++ b/modules/Email/tests/SimpleModule.Email.Tests/Unit/CreateTemplateFormRequestTests.cs @@ -2,7 +2,6 @@ using FluentAssertions; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.HttpResults; -using SimpleModule.Core.Authorization; using SimpleModule.Core.FormRequests; using SimpleModule.Email.FormRequests; @@ -18,46 +17,16 @@ public class CreateTemplateFormRequestTests private readonly FormRequestEndpointFilter _filter = new(); // ─── Authorize ──────────────────────────────────────────────────────────── + // Authorization is handled at the endpoint level via .RequirePermission(), + // not in the FormRequest. The default Authorize() returns true. [Fact] - public void Authorize_UserWithManageTemplatesPermission_ReturnsTrue() - { - var user = MakeUser(EmailPermissions.ManageTemplates); - var request = ValidRequest(); - - request.Authorize(user).Should().BeTrue(); - } - - [Fact] - public void Authorize_UserWithoutPermission_ReturnsFalse() - { - var user = MakeUser("SomeOtherPermission"); - var request = ValidRequest(); - - request.Authorize(user).Should().BeFalse(); - } - - [Fact] - public void Authorize_AnonymousUser_ReturnsFalse() + public void Authorize_ReturnsTrue_BecauseAuthorizationIsAtEndpointLevel() { var user = new ClaimsPrincipal(new ClaimsIdentity()); var request = ValidRequest(); - request.Authorize(user).Should().BeFalse(); - } - - [Fact] - public async Task Filter_UnauthorizedUser_Returns403() - { - var request = ValidRequest(); - var context = CreateContext(request); // no permission claims - - var result = await _filter.InvokeAsync( - context, - _ => ValueTask.FromResult(Results.Ok()) - ); - - result.Should().BeOfType().Which.StatusCode.Should().Be(403); + request.Authorize(user).Should().BeTrue(); } // ─── Prepare / normalization ────────────────────────────────────────────── @@ -431,12 +400,6 @@ private static CreateTemplateFormRequest ValidRequest() => IsHtml = true, }; - private static ClaimsPrincipal MakeUser(string permission) - { - var claims = new[] { new Claim("permission", permission) }; - return new ClaimsPrincipal(new ClaimsIdentity(claims, "test")); - } - private static DefaultEndpointFilterInvocationContext CreateContext( CreateTemplateFormRequest request, bool hasPermission = false diff --git a/tests/SimpleModule.Generator.Tests/EndpointExtensionsEmitterTests.cs b/tests/SimpleModule.Generator.Tests/EndpointExtensionsEmitterTests.cs index 6df94c0c..f89128c4 100644 --- a/tests/SimpleModule.Generator.Tests/EndpointExtensionsEmitterTests.cs +++ b/tests/SimpleModule.Generator.Tests/EndpointExtensionsEmitterTests.cs @@ -119,11 +119,11 @@ public void Map(IEndpointRouteBuilder app) // Should NOT auto-register endpoints because HasConfigureEndpoints is true endpointExt.Should().NotContain("new global::TestApp.Endpoints.AutoEndpoint()"); - // Should use the escape hatch wrapped in a FormRequestFilter group + // Should use the escape hatch passing app directly endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_CustomModule).ConfigureEndpoints(_escapeGroup);" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_CustomModule).ConfigureEndpoints(app);" ); } @@ -153,7 +153,7 @@ public void ConfigureEndpoints(IEndpointRouteBuilder endpoints) { } endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_ManualModule).ConfigureEndpoints(_escapeGroup);" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_ManualModule).ConfigureEndpoints(app);" ); } diff --git a/tests/SimpleModule.Generator.Tests/ModuleDiscovererGeneratorTests.cs b/tests/SimpleModule.Generator.Tests/ModuleDiscovererGeneratorTests.cs index ee71cb96..b740d6d5 100644 --- a/tests/SimpleModule.Generator.Tests/ModuleDiscovererGeneratorTests.cs +++ b/tests/SimpleModule.Generator.Tests/ModuleDiscovererGeneratorTests.cs @@ -56,7 +56,7 @@ public void ConfigureEndpoints(IEndpointRouteBuilder endpoints) { } endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_TestModule).ConfigureEndpoints(_escapeGroup)" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_TestModule).ConfigureEndpoints(app)" ); } @@ -96,7 +96,7 @@ public void ConfigureEndpoints(IEndpointRouteBuilder endpoints) { } endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_EndpointOnlyModule).ConfigureEndpoints(_escapeGroup)" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_EndpointOnlyModule).ConfigureEndpoints(app)" ); } diff --git a/tests/SimpleModule.Generator.Tests/ModuleExtensionsGenerationTests.cs b/tests/SimpleModule.Generator.Tests/ModuleExtensionsGenerationTests.cs index 25b1da7f..a4165d36 100644 --- a/tests/SimpleModule.Generator.Tests/ModuleExtensionsGenerationTests.cs +++ b/tests/SimpleModule.Generator.Tests/ModuleExtensionsGenerationTests.cs @@ -111,7 +111,7 @@ public void ConfigureEndpoints(IEndpointRouteBuilder endpoints) { } endpointExt .Should() .Contain( - "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_TestModule).ConfigureEndpoints(_escapeGroup)" + "((global::SimpleModule.Core.IModule)ModuleExtensions.s_TestApp_TestModule).ConfigureEndpoints(app)" ); } From 58745be2be67e1f6e287767d996b7b1dd3670cfd Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sun, 24 May 2026 16:15:17 +0200 Subject: [PATCH 4/6] feat(settings): convert Settings endpoints to FormRequest pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert 4 Settings module endpoints to use FormRequest for input validation: - UpdateSettingEndpoint (PUT /api/settings) — validates key format, scope enum - UpdateMySettingEndpoint (PUT /api/settings/me) — reuses UpdateSettingFormRequest - CreateMenuItemEndpoint (POST /api/settings/menus) — validates label, URL length - UpdateMenuItemEndpoint (PUT /api/settings/menus/{id}) — validates same as create Demonstrates dual-validation: FormRequest validates shape (key format, field lengths, enum membership), service layer validates domain (setting type/range against definitions). FormRequest returns 422, service exceptions return 400. Symmetric validation: both Create and Update menu endpoints now share the same rules; both admin and user setting endpoints use the same FormRequest. Includes 16 new integration tests verifying 422 responses, Prepare normalization, RFC 7807 shape, and auth-before-validation ordering. --- .../Endpoints/Menus/CreateMenuItemEndpoint.cs | 21 +- .../Endpoints/Menus/UpdateMenuItemEndpoint.cs | 16 +- .../Settings/UpdateSettingEndpoint.cs | 3 +- .../UserSettings/UpdateMySettingEndpoint.cs | 3 +- .../FormRequests/CreateMenuItemFormRequest.cs | 58 +++ .../FormRequests/UpdateMenuItemFormRequest.cs | 56 +++ .../FormRequests/UpdateSettingFormRequest.cs | 39 ++ .../src/SimpleModule.Settings/types.ts | 59 +-- .../Integration/FormRequestEndpointTests.cs | 371 ++++++++++++++++++ 9 files changed, 594 insertions(+), 32 deletions(-) create mode 100644 modules/Settings/src/SimpleModule.Settings/FormRequests/CreateMenuItemFormRequest.cs create mode 100644 modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateMenuItemFormRequest.cs create mode 100644 modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateSettingFormRequest.cs create mode 100644 modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/CreateMenuItemEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/CreateMenuItemEndpoint.cs index 645aea42..453f8dbb 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/CreateMenuItemEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/CreateMenuItemEndpoint.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Routing; using SimpleModule.Core; using SimpleModule.Settings.Contracts; +using SimpleModule.Settings.FormRequests; using SimpleModule.Settings.Services; namespace SimpleModule.Settings.Endpoints.Menus; @@ -15,10 +16,22 @@ public class CreateMenuItemEndpoint : IEndpoint public void Map(IEndpointRouteBuilder app) => app.MapPost( Route, - async (CreateMenuItemRequest request, PublicMenuService service) => + async (CreateMenuItemFormRequest request, PublicMenuService service) => { - var entity = await service.CreateAsync(request); - var dto = new PublicMenuItemDto + var dto = new CreateMenuItemRequest + { + ParentId = request.ParentId, + Label = request.Label, + Url = request.Url, + PageRoute = request.PageRoute, + Icon = request.Icon, + CssClass = request.CssClass, + OpenInNewTab = request.OpenInNewTab, + IsVisible = request.IsVisible, + IsHomePage = request.IsHomePage, + }; + var entity = await service.CreateAsync(dto); + var result = new PublicMenuItemDto { Id = entity.Id, ParentId = entity.ParentId, @@ -32,7 +45,7 @@ public void Map(IEndpointRouteBuilder app) => IsHomePage = entity.IsHomePage, SortOrder = entity.SortOrder, }; - return TypedResults.Created($"/api/settings/menus/{entity.Id}", dto); + return TypedResults.Created($"/api/settings/menus/{entity.Id}", result); } ) .RequireAuthorization(); diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/UpdateMenuItemEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/UpdateMenuItemEndpoint.cs index eb1a2174..76c8901f 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/UpdateMenuItemEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/UpdateMenuItemEndpoint.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Routing; using SimpleModule.Core; using SimpleModule.Settings.Contracts; +using SimpleModule.Settings.FormRequests; using SimpleModule.Settings.Services; namespace SimpleModule.Settings.Endpoints.Menus; @@ -17,11 +18,22 @@ public void Map(IEndpointRouteBuilder app) => Route, async Task ( int id, - UpdateMenuItemRequest request, + UpdateMenuItemFormRequest request, PublicMenuService service ) => { - var entity = await service.UpdateAsync(PublicMenuItemId.From(id), request); + var dto = new UpdateMenuItemRequest + { + Label = request.Label, + Url = request.Url, + PageRoute = request.PageRoute, + Icon = request.Icon, + CssClass = request.CssClass, + OpenInNewTab = request.OpenInNewTab, + IsVisible = request.IsVisible, + IsHomePage = request.IsHomePage, + }; + var entity = await service.UpdateAsync(PublicMenuItemId.From(id), dto); return entity is not null ? TypedResults.NoContent() : TypedResults.NotFound(); } ) diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Settings/UpdateSettingEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Settings/UpdateSettingEndpoint.cs index 359ef328..6c239be1 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Settings/UpdateSettingEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Settings/UpdateSettingEndpoint.cs @@ -5,6 +5,7 @@ using SimpleModule.Core.Authorization; using SimpleModule.Core.Settings; using SimpleModule.Settings.Contracts; +using SimpleModule.Settings.FormRequests; namespace SimpleModule.Settings.Endpoints.Settings; @@ -16,7 +17,7 @@ public class UpdateSettingEndpoint : IEndpoint public void Map(IEndpointRouteBuilder app) => app.MapPut( Route, - async Task (UpdateSettingRequest request, ISettingsContracts settings) => + async Task (UpdateSettingFormRequest request, ISettingsContracts settings) => { if (request.Scope == SettingScope.User) { diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/UserSettings/UpdateMySettingEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/UserSettings/UpdateMySettingEndpoint.cs index 0b32a549..c84b4005 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/UserSettings/UpdateMySettingEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/UserSettings/UpdateMySettingEndpoint.cs @@ -5,6 +5,7 @@ using SimpleModule.Core; using SimpleModule.Core.Settings; using SimpleModule.Settings.Contracts; +using SimpleModule.Settings.FormRequests; namespace SimpleModule.Settings.Endpoints.UserSettings; @@ -17,7 +18,7 @@ public void Map(IEndpointRouteBuilder app) => app.MapPut( Route, async Task ( - UpdateSettingRequest request, + UpdateMySettingFormRequest request, ISettingsContracts settings, ClaimsPrincipal principal ) => diff --git a/modules/Settings/src/SimpleModule.Settings/FormRequests/CreateMenuItemFormRequest.cs b/modules/Settings/src/SimpleModule.Settings/FormRequests/CreateMenuItemFormRequest.cs new file mode 100644 index 00000000..a430dc7b --- /dev/null +++ b/modules/Settings/src/SimpleModule.Settings/FormRequests/CreateMenuItemFormRequest.cs @@ -0,0 +1,58 @@ +using FluentValidation; +using SimpleModule.Core.FormRequests; +using SimpleModule.Settings.Contracts; + +namespace SimpleModule.Settings.FormRequests; + +[FormRequest] +public sealed class CreateMenuItemFormRequest : FormRequest +{ + public PublicMenuItemId? ParentId { get; set; } + public string Label { get; set; } = ""; + + [System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1056:URI-like properties should not be strings" + )] + public string? Url { get; set; } + public string? PageRoute { get; set; } + public string Icon { get; set; } = ""; + public string? CssClass { get; set; } + public bool OpenInNewTab { get; set; } + public bool IsVisible { get; set; } = true; + public bool IsHomePage { get; set; } + + public override void Prepare() + { + Label = Label.Trim(); + Url = Url?.Trim(); + PageRoute = PageRoute?.Trim(); + Icon = Icon.Trim(); + CssClass = CssClass?.Trim(); + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules + .RuleFor(x => x.Label) + .NotEmpty() + .WithMessage("Label is required.") + .MaximumLength(200) + .WithMessage("Label must not exceed 200 characters."); + + rules + .RuleFor(x => x.Url) + .MaximumLength(2000) + .WithMessage("URL must not exceed 2000 characters."); + + rules + .RuleFor(x => x.PageRoute) + .MaximumLength(500) + .WithMessage("Page route must not exceed 500 characters."); + + rules + .RuleFor(x => x.Icon) + .MaximumLength(1000) + .WithMessage("Icon must not exceed 1000 characters."); + } +} diff --git a/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateMenuItemFormRequest.cs b/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateMenuItemFormRequest.cs new file mode 100644 index 00000000..3080a3be --- /dev/null +++ b/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateMenuItemFormRequest.cs @@ -0,0 +1,56 @@ +using FluentValidation; +using SimpleModule.Core.FormRequests; + +namespace SimpleModule.Settings.FormRequests; + +[FormRequest] +public sealed class UpdateMenuItemFormRequest : FormRequest +{ + public string Label { get; set; } = ""; + + [System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1056:URI-like properties should not be strings" + )] + public string? Url { get; set; } + public string? PageRoute { get; set; } + public string Icon { get; set; } = ""; + public string? CssClass { get; set; } + public bool OpenInNewTab { get; set; } + public bool IsVisible { get; set; } = true; + public bool IsHomePage { get; set; } + + public override void Prepare() + { + Label = Label.Trim(); + Url = Url?.Trim(); + PageRoute = PageRoute?.Trim(); + Icon = Icon.Trim(); + CssClass = CssClass?.Trim(); + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules + .RuleFor(x => x.Label) + .NotEmpty() + .WithMessage("Label is required.") + .MaximumLength(200) + .WithMessage("Label must not exceed 200 characters."); + + rules + .RuleFor(x => x.Url) + .MaximumLength(2000) + .WithMessage("URL must not exceed 2000 characters."); + + rules + .RuleFor(x => x.PageRoute) + .MaximumLength(500) + .WithMessage("Page route must not exceed 500 characters."); + + rules + .RuleFor(x => x.Icon) + .MaximumLength(1000) + .WithMessage("Icon must not exceed 1000 characters."); + } +} diff --git a/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateSettingFormRequest.cs b/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateSettingFormRequest.cs new file mode 100644 index 00000000..e309ee21 --- /dev/null +++ b/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateSettingFormRequest.cs @@ -0,0 +1,39 @@ +using System.Text.Json; +using System.Text.RegularExpressions; +using FluentValidation; +using SimpleModule.Core.FormRequests; +using SimpleModule.Core.Settings; + +namespace SimpleModule.Settings.FormRequests; + +[FormRequest] +public sealed partial class UpdateSettingFormRequest : FormRequest +{ + public string Key { get; set; } = ""; + public JsonElement Value { get; set; } + public SettingScope Scope { get; set; } + + public override void Prepare() + { + Key = Key.Trim(); + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules + .RuleFor(x => x.Key) + .NotEmpty() + .WithMessage("Setting key is required.") + .MaximumLength(256) + .WithMessage("Setting key must not exceed 256 characters.") + .Must(k => string.IsNullOrEmpty(k) || SettingKeyPattern().IsMatch(k)) + .WithMessage( + "Setting key must be lowercase alphanumeric with dots and underscores (e.g. 'app.theme')." + ); + + rules.RuleFor(x => x.Scope).IsInEnum().WithMessage("Invalid setting scope."); + } + + [GeneratedRegex(@"^[a-z][a-z0-9_.]*$")] + private static partial Regex SettingKeyPattern(); +} diff --git a/modules/Settings/src/SimpleModule.Settings/types.ts b/modules/Settings/src/SimpleModule.Settings/types.ts index c6028465..6d474db8 100644 --- a/modules/Settings/src/SimpleModule.Settings/types.ts +++ b/modules/Settings/src/SimpleModule.Settings/types.ts @@ -1,34 +1,31 @@ // Auto-generated from [Dto] types — do not edit -export interface BulkSettingUpdate { - key: string; - scope: any; - value: any; -} - -export interface BulkUpdateSettingsRequest { - updates: BulkSettingUpdate[]; -} - -export interface SettingValueDto { - key: string; - scope: any; - value: any | null; - isOverridden: boolean; - userId: string; - updatedAt: string | null; +export interface CreateMenuItemFormRequest { + parentId: any | null; + label: string; + url: string; + pageRoute: string; + icon: string; + cssClass: string; + openInNewTab: boolean; + isVisible: boolean; + isHomePage: boolean; } -export interface UserSettingValueDto { - key: string; - value: any | null; - resolvedValue: any | null; - isOverridden: boolean; +export interface UpdateMenuItemFormRequest { + label: string; + url: string; + pageRoute: string; + icon: string; + cssClass: string; + openInNewTab: boolean; + isVisible: boolean; + isHomePage: boolean; } -export interface UpdateSettingRequest { +export interface UpdateSettingFormRequest { key: string; + value: string; scope: any; - value: any; } export interface CreateMenuItemRequest { @@ -68,6 +65,14 @@ export interface ReorderItem { sortOrder: number; } +export interface Setting { + key: string; + value: string; + scope: any; + userId: string; + updatedAt: string; +} + export interface SettingsFilter { scope: any | null; group: string; @@ -84,3 +89,9 @@ export interface UpdateMenuItemRequest { isHomePage: boolean; } +export interface UpdateSettingRequest { + key: string; + value: string; + scope: any; +} + diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs new file mode 100644 index 00000000..9c82a179 --- /dev/null +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs @@ -0,0 +1,371 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using FluentAssertions; +using SimpleModule.Core.Settings; +using SimpleModule.Settings.Contracts; +using SimpleModule.Tests.Shared.Fixtures; + +namespace Settings.Tests.Integration; + +[Collection(TestCollections.Integration)] +public class FormRequestEndpointTests(SimpleModuleWebApplicationFactory factory) +{ + private static string UniqueKey(string prefix) => $"{prefix}.{Guid.NewGuid():N}"; + + // ─── UpdateSetting: valid requests ──────────────────────────────────────── + + [Fact] + public async Task UpdateSetting_ValidRequest_Returns204() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = UniqueKey("test.formrequest"), + Value = "\"hello\"", + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.NoContent); + } + + [Fact] + public async Task UpdateSetting_NullValue_Returns204() + { + // Value is string? — null means "clear the setting", which is a valid operation. + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = UniqueKey("test.nullvalue"), + Value = (string?)null, + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.NoContent); + } + + [Fact] + public async Task UpdateSetting_KeyWithWhitespace_TrimsAndSucceeds() + { + // Prepare() should trim whitespace from the key before validation runs. + var client = factory.CreateAuthenticatedClient(); + var baseKey = UniqueKey("test.trimmed"); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = $" {baseKey} ", + Value = "\"trimmed\"", + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.NoContent); + } + + // ─── UpdateSetting: validation failures → 422 ───────────────────────────── + + [Fact] + public async Task UpdateSetting_EmptyKey_Returns422WithKeyError() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = "", + Value = "\"test\"", + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + await AssertProblemHasFieldError(response, "Key"); + } + + [Fact] + public async Task UpdateSetting_InvalidKeyFormat_Returns422() + { + // Keys must be dotted lowercase identifiers (e.g. "app.theme"). + // "INVALID KEY!" contains spaces and special chars — should fail even after trim. + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = "INVALID KEY!", + Value = "\"test\"", + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + await AssertProblemHasFieldError(response, "Key"); + } + + [Fact] + public async Task UpdateSetting_KeyTooLong_Returns422() + { + var client = factory.CreateAuthenticatedClient(); + // MaximumLength(256) means 256 is valid but 257 is not. + var longKey = "a." + new string('a', 256); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = longKey, + Value = "\"test\"", + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + await AssertProblemHasFieldError(response, "Key"); + } + + [Fact] + public async Task UpdateSetting_InvalidScopeEnum_Returns422() + { + var client = factory.CreateAuthenticatedClient(); + + // Use raw JSON to send scope=99 which is not a valid SettingScope value. + using var content = new StringContent( + JsonSerializer.Serialize( + new + { + Key = "test.scope", + Value = "\"x\"", + Scope = 99, + } + ), + System.Text.Encoding.UTF8, + "application/json" + ); + + var response = await client.PutAsync("/api/settings", content); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + await AssertProblemHasFieldError(response, "Scope"); + } + + // ─── CreateMenuItem: valid requests ─────────────────────────────────────── + + [Fact] + public async Task CreateMenuItem_ValidRequest_Returns201WithLocation() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PostAsJsonAsync( + "/api/settings/menus", + new + { + Label = $"Test Menu {Guid.NewGuid():N}", + Url = "/test-page", + IsVisible = true, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.Created); + response.Headers.Location.Should().NotBeNull(); + response.Headers.Location!.ToString().Should().StartWith("/api/settings/menus/"); + } + + [Fact] + public async Task CreateMenuItem_MinimalRequest_Returns201() + { + // Only Label is required; all other fields are optional. + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PostAsJsonAsync( + "/api/settings/menus", + new { Label = "Minimal Item" } + ); + + response.StatusCode.Should().Be(HttpStatusCode.Created); + } + + [Fact] + public async Task CreateMenuItem_LabelTrimmedByPrepare_EntityHasTrimmedLabel() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PostAsJsonAsync( + "/api/settings/menus", + new { Label = " Trimmed Label ", IsVisible = true } + ); + + response.StatusCode.Should().Be(HttpStatusCode.Created); + + // Read back the created entity to verify the label was trimmed. + var body = await response.Content.ReadFromJsonAsync(); + body.GetProperty("label").GetString().Should().Be("Trimmed Label"); + } + + // ─── CreateMenuItem: validation failures → 422 ──────────────────────────── + + [Fact] + public async Task CreateMenuItem_EmptyLabel_Returns422() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PostAsJsonAsync( + "/api/settings/menus", + new { Label = "", IsVisible = true } + ); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + await AssertProblemHasFieldError(response, "Label"); + } + + [Fact] + public async Task CreateMenuItem_LabelTooLong_Returns422() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PostAsJsonAsync( + "/api/settings/menus", + new { Label = new string('X', 201), IsVisible = true } + ); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + await AssertProblemHasFieldError(response, "Label"); + } + + [Fact] + public async Task CreateMenuItem_UrlTooLong_Returns422() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PostAsJsonAsync( + "/api/settings/menus", + new + { + Label = "Valid Label", + Url = "/" + new string('u', 2001), + IsVisible = true, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + await AssertProblemHasFieldError(response, "Url"); + } + + // ─── Cross-cutting: RFC 7807 shape ──────────────────────────────────────── + + [Fact] + public async Task ValidationError_ResponseHasCorrectRfc7807Shape() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = "", + Value = "\"test\"", + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + response.Content.Headers.ContentType?.MediaType.Should().Be("application/problem+json"); + + var body = await response.Content.ReadFromJsonAsync(); + + // RFC 7807 required fields + body.TryGetProperty("status", out var statusProp).Should().BeTrue(); + statusProp.GetInt32().Should().Be(422); + + body.TryGetProperty("title", out var titleProp).Should().BeTrue(); + titleProp.GetString().Should().Be("Validation Error"); + + // The errors extension must be a dictionary of field → string[] + body.TryGetProperty("errors", out var errorsProp).Should().BeTrue(); + errorsProp.ValueKind.Should().Be(JsonValueKind.Object); + + // Each error entry should be an array of strings + foreach (var prop in errorsProp.EnumerateObject()) + { + prop.Value.ValueKind.Should().Be(JsonValueKind.Array); + foreach (var msg in prop.Value.EnumerateArray()) + { + msg.ValueKind.Should().Be(JsonValueKind.String); + } + } + } + + // ─── Cross-cutting: authentication before validation ────────────────────── + + [Fact] + public async Task UpdateSetting_Unauthenticated_Returns401NotValidationError() + { + // Auth middleware must run before the FormRequest filter. + // Even with invalid data, an unauthenticated request gets 401. + var client = factory.CreateClient(); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = "", + Value = "\"test\"", + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + } + + [Fact] + public async Task CreateMenuItem_Unauthenticated_Returns401NotValidationError() + { + var client = factory.CreateClient(); + + var response = await client.PostAsJsonAsync( + "/api/settings/menus", + new { Label = "", IsVisible = true } + ); + + response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + } + + // ─── Helpers ────────────────────────────────────────────────────────────── + + private static async Task AssertProblemHasFieldError( + HttpResponseMessage response, + string fieldName + ) + { + var body = await response.Content.ReadFromJsonAsync(); + + body.TryGetProperty("status", out var statusProp) + .Should() + .BeTrue("response should contain 'status'"); + statusProp.GetInt32().Should().Be(422); + + body.TryGetProperty("title", out var titleProp) + .Should() + .BeTrue("response should contain 'title'"); + titleProp.GetString().Should().Be("Validation Error"); + + body.TryGetProperty("errors", out var errorsProp) + .Should() + .BeTrue("response should contain 'errors' dictionary"); + errorsProp + .TryGetProperty(fieldName, out _) + .Should() + .BeTrue($"errors should contain key '{fieldName}'"); + } +} From b611f4c1bf1753ee7a7ce7be673b2e95ae0a942e Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sun, 24 May 2026 17:03:41 +0200 Subject: [PATCH 5/6] =?UTF-8?q?fix(settings):=20address=20code=20review=20?= =?UTF-8?q?=E2=80=94=20regex,=20scope,=20cascade,=20test=20semantics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix regex to allow camelCase/PascalCase keys (email.defaultFromAddress, FileStorage.MaxFileSizeMb, etc.) and enforce segment-based validation that rejects trailing dots and empty segments - Create dedicated UpdateMySettingFormRequest (Key + Value only) for PUT /api/settings/me — removes misleading Scope field that was validated but silently ignored - Fix double error messages for empty key by splitting rules with .When() guard so regex only fires on non-empty keys - Rename misleading test: NullValue "clears setting" → actually stores empty string (documents real behavior, not aspirational) - Add tests for camelCase keys and trailing-dot rejection --- .../UpdateMySettingFormRequest.cs | 39 ++++++++++++++++ .../FormRequests/UpdateSettingFormRequest.cs | 14 +++--- .../src/SimpleModule.Settings/types.ts | 5 +++ .../Integration/FormRequestEndpointTests.cs | 44 +++++++++++++++++-- .../Integration/SettingsEndpointTests.cs | 2 +- 5 files changed, 94 insertions(+), 10 deletions(-) create mode 100644 modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateMySettingFormRequest.cs diff --git a/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateMySettingFormRequest.cs b/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateMySettingFormRequest.cs new file mode 100644 index 00000000..f98c0e09 --- /dev/null +++ b/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateMySettingFormRequest.cs @@ -0,0 +1,39 @@ +using System.Text.Json; +using System.Text.RegularExpressions; +using FluentValidation; +using SimpleModule.Core.FormRequests; + +namespace SimpleModule.Settings.FormRequests; + +[FormRequest] +public sealed partial class UpdateMySettingFormRequest : FormRequest +{ + public string Key { get; set; } = ""; + public JsonElement Value { get; set; } + + public override void Prepare() + { + Key = Key.Trim(); + } + + protected override void ConfigureRules(RuleConfigurator rules) + { + rules + .RuleFor(x => x.Key) + .NotEmpty() + .WithMessage("Setting key is required.") + .MaximumLength(256) + .WithMessage("Setting key must not exceed 256 characters."); + + rules + .RuleFor(x => x.Key) + .Must(k => SettingKeyPattern().IsMatch(k)) + .WithMessage( + "Setting key must be alphanumeric segments separated by dots (e.g. 'app.theme')." + ) + .When(x => !string.IsNullOrEmpty(x.Key)); + } + + [GeneratedRegex(@"^[a-zA-Z][a-zA-Z0-9_]*(\.[a-zA-Z][a-zA-Z0-9_]*)*$")] + private static partial Regex SettingKeyPattern(); +} diff --git a/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateSettingFormRequest.cs b/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateSettingFormRequest.cs index e309ee21..e0cffee6 100644 --- a/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateSettingFormRequest.cs +++ b/modules/Settings/src/SimpleModule.Settings/FormRequests/UpdateSettingFormRequest.cs @@ -25,15 +25,19 @@ protected override void ConfigureRules(RuleConfigurator string.IsNullOrEmpty(k) || SettingKeyPattern().IsMatch(k)) + .WithMessage("Setting key must not exceed 256 characters."); + + rules + .RuleFor(x => x.Key) + .Must(k => SettingKeyPattern().IsMatch(k)) .WithMessage( - "Setting key must be lowercase alphanumeric with dots and underscores (e.g. 'app.theme')." - ); + "Setting key must be alphanumeric segments separated by dots (e.g. 'app.theme', 'email.defaultFromAddress')." + ) + .When(x => !string.IsNullOrEmpty(x.Key)); rules.RuleFor(x => x.Scope).IsInEnum().WithMessage("Invalid setting scope."); } - [GeneratedRegex(@"^[a-z][a-z0-9_.]*$")] + [GeneratedRegex(@"^[a-zA-Z][a-zA-Z0-9_]*(\.[a-zA-Z][a-zA-Z0-9_]*)*$")] private static partial Regex SettingKeyPattern(); } diff --git a/modules/Settings/src/SimpleModule.Settings/types.ts b/modules/Settings/src/SimpleModule.Settings/types.ts index 6d474db8..661edde5 100644 --- a/modules/Settings/src/SimpleModule.Settings/types.ts +++ b/modules/Settings/src/SimpleModule.Settings/types.ts @@ -22,6 +22,11 @@ export interface UpdateMenuItemFormRequest { isHomePage: boolean; } +export interface UpdateMySettingFormRequest { + key: string; + value: string; +} + export interface UpdateSettingFormRequest { key: string; value: string; diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs index 9c82a179..7aafadc0 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs @@ -11,7 +11,7 @@ namespace Settings.Tests.Integration; [Collection(TestCollections.Integration)] public class FormRequestEndpointTests(SimpleModuleWebApplicationFactory factory) { - private static string UniqueKey(string prefix) => $"{prefix}.{Guid.NewGuid():N}"; + private static string UniqueKey(string prefix) => $"{prefix}.k{Guid.NewGuid():N}"; // ─── UpdateSetting: valid requests ──────────────────────────────────────── @@ -34,9 +34,9 @@ public async Task UpdateSetting_ValidRequest_Returns204() } [Fact] - public async Task UpdateSetting_NullValue_Returns204() + public async Task UpdateSetting_NullValue_StoresEmptyString() { - // Value is string? — null means "clear the setting", which is a valid operation. + // Value is string? — null value is coerced to empty string by the endpoint. var client = factory.CreateAuthenticatedClient(); var response = await client.PutAsJsonAsync( @@ -93,10 +93,46 @@ public async Task UpdateSetting_EmptyKey_Returns422WithKeyError() await AssertProblemHasFieldError(response, "Key"); } + [Fact] + public async Task UpdateSetting_CamelCaseKey_Returns204() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = "test.camelCaseKey", + Value = "\"test\"", + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.NoContent); + } + + [Fact] + public async Task UpdateSetting_TrailingDot_Returns422() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.PutAsJsonAsync( + "/api/settings", + new + { + Key = "app.", + Value = "\"test\"", + Scope = SettingScope.Application, + } + ); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + await AssertProblemHasFieldError(response, "Key"); + } + [Fact] public async Task UpdateSetting_InvalidKeyFormat_Returns422() { - // Keys must be dotted lowercase identifiers (e.g. "app.theme"). // "INVALID KEY!" contains spaces and special chars — should fail even after trim. var client = factory.CreateAuthenticatedClient(); diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs index a5acef8a..3ce62efd 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs @@ -12,7 +12,7 @@ namespace Settings.Tests.Integration; [Collection(TestCollections.Integration)] public class SettingsEndpointTests(SimpleModuleWebApplicationFactory factory) { - private static string UniqueKey(string prefix) => $"{prefix}.{Guid.NewGuid():N}"; + private static string UniqueKey(string prefix) => $"{prefix}.k{Guid.NewGuid():N}"; private static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web); From e7e84dc8958423e00c430d82f1ae314b8d19831a Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sun, 24 May 2026 20:14:13 +0200 Subject: [PATCH 6/6] fix(settings): adapt FormRequest tests to upstream API changes Upstream PR #211 changed UpdateSettingRequest.Value from string? to JsonElement and added .RequirePermission(SettingsPermissions.Update). Update FormRequest integration tests to: - Use JsonSerializer.Deserialize() for Value fields - Pass SettingsPermissions.Update to CreateAuthenticatedClient - Replace hyphenated test keys with camelCase (regex rejects hyphens) --- .../src/SimpleModule.Settings/types.ts | 50 +++++++++++------ .../Integration/FormRequestEndpointTests.cs | 54 +++++++++---------- .../Integration/SettingsEndpointTests.cs | 6 +-- .../SettingsValidationEndpointTests.cs | 6 +-- 4 files changed, 64 insertions(+), 52 deletions(-) diff --git a/modules/Settings/src/SimpleModule.Settings/types.ts b/modules/Settings/src/SimpleModule.Settings/types.ts index 661edde5..91c24b66 100644 --- a/modules/Settings/src/SimpleModule.Settings/types.ts +++ b/modules/Settings/src/SimpleModule.Settings/types.ts @@ -1,4 +1,36 @@ // Auto-generated from [Dto] types — do not edit +export interface BulkSettingUpdate { + key: string; + scope: any; + value: any; +} + +export interface BulkUpdateSettingsRequest { + updates: BulkSettingUpdate[]; +} + +export interface SettingValueDto { + key: string; + scope: any; + value: any | null; + isOverridden: boolean; + userId: string; + updatedAt: string | null; +} + +export interface UserSettingValueDto { + key: string; + value: any | null; + resolvedValue: any | null; + isOverridden: boolean; +} + +export interface UpdateSettingRequest { + key: string; + scope: any; + value: any; +} + export interface CreateMenuItemFormRequest { parentId: any | null; label: string; @@ -24,12 +56,12 @@ export interface UpdateMenuItemFormRequest { export interface UpdateMySettingFormRequest { key: string; - value: string; + value: any; } export interface UpdateSettingFormRequest { key: string; - value: string; + value: any; scope: any; } @@ -70,14 +102,6 @@ export interface ReorderItem { sortOrder: number; } -export interface Setting { - key: string; - value: string; - scope: any; - userId: string; - updatedAt: string; -} - export interface SettingsFilter { scope: any | null; group: string; @@ -94,9 +118,3 @@ export interface UpdateMenuItemRequest { isHomePage: boolean; } -export interface UpdateSettingRequest { - key: string; - value: string; - scope: any; -} - diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs index 7aafadc0..1d75e333 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs @@ -3,6 +3,7 @@ using System.Text.Json; using FluentAssertions; using SimpleModule.Core.Settings; +using SimpleModule.Settings; using SimpleModule.Settings.Contracts; using SimpleModule.Tests.Shared.Fixtures; @@ -18,14 +19,14 @@ public class FormRequestEndpointTests(SimpleModuleWebApplicationFactory factory) [Fact] public async Task UpdateSetting_ValidRequest_Returns204() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); var response = await client.PutAsJsonAsync( "/api/settings", new { Key = UniqueKey("test.formrequest"), - Value = "\"hello\"", + Value = JsonSerializer.Deserialize("\"hello\""), Scope = SettingScope.Application, } ); @@ -34,17 +35,17 @@ public async Task UpdateSetting_ValidRequest_Returns204() } [Fact] - public async Task UpdateSetting_NullValue_StoresEmptyString() + public async Task UpdateSetting_NullJsonValue_Returns204() { - // Value is string? — null value is coerced to empty string by the endpoint. - var client = factory.CreateAuthenticatedClient(); + // Value is JsonElement — a JSON null is a valid JsonElement with ValueKind.Null. + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); var response = await client.PutAsJsonAsync( "/api/settings", new { Key = UniqueKey("test.nullvalue"), - Value = (string?)null, + Value = JsonSerializer.Deserialize("null"), Scope = SettingScope.Application, } ); @@ -56,7 +57,7 @@ public async Task UpdateSetting_NullValue_StoresEmptyString() public async Task UpdateSetting_KeyWithWhitespace_TrimsAndSucceeds() { // Prepare() should trim whitespace from the key before validation runs. - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); var baseKey = UniqueKey("test.trimmed"); var response = await client.PutAsJsonAsync( @@ -64,7 +65,7 @@ public async Task UpdateSetting_KeyWithWhitespace_TrimsAndSucceeds() new { Key = $" {baseKey} ", - Value = "\"trimmed\"", + Value = JsonSerializer.Deserialize("\"trimmed\""), Scope = SettingScope.Application, } ); @@ -77,14 +78,14 @@ public async Task UpdateSetting_KeyWithWhitespace_TrimsAndSucceeds() [Fact] public async Task UpdateSetting_EmptyKey_Returns422WithKeyError() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); var response = await client.PutAsJsonAsync( "/api/settings", new { Key = "", - Value = "\"test\"", + Value = JsonSerializer.Deserialize("\"test\""), Scope = SettingScope.Application, } ); @@ -96,14 +97,14 @@ public async Task UpdateSetting_EmptyKey_Returns422WithKeyError() [Fact] public async Task UpdateSetting_CamelCaseKey_Returns204() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); var response = await client.PutAsJsonAsync( "/api/settings", new { Key = "test.camelCaseKey", - Value = "\"test\"", + Value = JsonSerializer.Deserialize("\"test\""), Scope = SettingScope.Application, } ); @@ -114,14 +115,14 @@ public async Task UpdateSetting_CamelCaseKey_Returns204() [Fact] public async Task UpdateSetting_TrailingDot_Returns422() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); var response = await client.PutAsJsonAsync( "/api/settings", new { Key = "app.", - Value = "\"test\"", + Value = JsonSerializer.Deserialize("\"test\""), Scope = SettingScope.Application, } ); @@ -134,14 +135,14 @@ public async Task UpdateSetting_TrailingDot_Returns422() public async Task UpdateSetting_InvalidKeyFormat_Returns422() { // "INVALID KEY!" contains spaces and special chars — should fail even after trim. - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); var response = await client.PutAsJsonAsync( "/api/settings", new { Key = "INVALID KEY!", - Value = "\"test\"", + Value = JsonSerializer.Deserialize("\"test\""), Scope = SettingScope.Application, } ); @@ -153,7 +154,7 @@ public async Task UpdateSetting_InvalidKeyFormat_Returns422() [Fact] public async Task UpdateSetting_KeyTooLong_Returns422() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); // MaximumLength(256) means 256 is valid but 257 is not. var longKey = "a." + new string('a', 256); @@ -162,7 +163,7 @@ public async Task UpdateSetting_KeyTooLong_Returns422() new { Key = longKey, - Value = "\"test\"", + Value = JsonSerializer.Deserialize("\"test\""), Scope = SettingScope.Application, } ); @@ -174,18 +175,11 @@ public async Task UpdateSetting_KeyTooLong_Returns422() [Fact] public async Task UpdateSetting_InvalidScopeEnum_Returns422() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); // Use raw JSON to send scope=99 which is not a valid SettingScope value. using var content = new StringContent( - JsonSerializer.Serialize( - new - { - Key = "test.scope", - Value = "\"x\"", - Scope = 99, - } - ), + """{"Key":"test.scope","Value":"x","Scope":99}""", System.Text.Encoding.UTF8, "application/json" ); @@ -303,14 +297,14 @@ public async Task CreateMenuItem_UrlTooLong_Returns422() [Fact] public async Task ValidationError_ResponseHasCorrectRfc7807Shape() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.Update]); var response = await client.PutAsJsonAsync( "/api/settings", new { Key = "", - Value = "\"test\"", + Value = JsonSerializer.Deserialize("\"test\""), Scope = SettingScope.Application, } ); @@ -356,7 +350,7 @@ public async Task UpdateSetting_Unauthenticated_Returns401NotValidationError() new { Key = "", - Value = "\"test\"", + Value = JsonSerializer.Deserialize("\"test\""), Scope = SettingScope.Application, } ); diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs index 3ce62efd..d4e29fb9 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs @@ -208,7 +208,7 @@ public async Task BulkUpdateSettings_WithoutPermission_Returns403() [ new BulkSettingUpdate { - Key = UniqueKey("bulk-perm"), + Key = UniqueKey("bulkperm"), Scope = SettingScope.Application, Value = JsonSerializer.Deserialize("\"v\""), }, @@ -246,7 +246,7 @@ public async Task UpdateSetting_WithUserScope_Returns400() var client = CreateAdminClient(); var response = await client.PutAsJsonAsync( "/api/settings", - StringRequest(UniqueKey("user-scope"), "value", SettingScope.User), + StringRequest(UniqueKey("userscope"), "value", SettingScope.User), JsonOptions ); response.StatusCode.Should().Be(HttpStatusCode.BadRequest); @@ -264,7 +264,7 @@ public async Task BulkUpdateSettings_WithUserScopeEntry_Returns400() [ new BulkSettingUpdate { - Key = UniqueKey("bulk-user"), + Key = UniqueKey("bulkuser"), Scope = SettingScope.User, Value = JsonSerializer.Deserialize("\"v\""), }, diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsValidationEndpointTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsValidationEndpointTests.cs index f24aafd5..4fa83848 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsValidationEndpointTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsValidationEndpointTests.cs @@ -21,7 +21,7 @@ namespace Settings.Tests.Integration; [Collection(TestCollections.Integration)] public class SettingsValidationEndpointTests(SimpleModuleWebApplicationFactory factory) { - private static string UniqueKey(string prefix) => $"qa.{prefix}.{Guid.NewGuid():N}"; + private static string UniqueKey(string prefix) => $"qa.{prefix}.k{Guid.NewGuid():N}"; private static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web); @@ -318,7 +318,7 @@ public async Task UpdateSetting_WithUserScope_IsRejected() // PUT /api/settings rejects User scope at the admin endpoint to prevent ghost rows // (UserId=null). Callers must use /api/settings/me for per-user values. var client = AdminClient(); - var key = UniqueKey("user-scope-ghost"); + var key = UniqueKey("userscopeghost"); var response = await client.PutAsJsonAsync( "/api/settings", @@ -420,7 +420,7 @@ await client.PutAsJsonAsync( public async Task ScopeIsolation_SystemAndApplicationAreSeparateRows() { var client = AdminClient(); - var key = UniqueKey("scope-isolation"); + var key = UniqueKey("scopeIsolation"); // Set different values at System and Application scope await client.PutAsJsonAsync(