diff --git a/CLAUDE.md b/CLAUDE.md index 0299b7c0..1c7455dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,7 +129,7 @@ See [docs/CONSTITUTION.md](docs/CONSTITUTION.md) for the authoritative reference - Module boundaries, dependencies, and data ownership - Communication patterns (contracts and events) - Endpoint, frontend, and authorization rules -- Compiler-enforced diagnostics (SM0001-SM0044) +- Compiler-enforced diagnostics (SM0001-SM0054) - Framework contributor guidelines ## Key Constraints diff --git a/docs/CONSTITUTION.md b/docs/CONSTITUTION.md index 879f774c..854cc7e1 100644 --- a/docs/CONSTITUTION.md +++ b/docs/CONSTITUTION.md @@ -462,14 +462,23 @@ All SM diagnostics are emitted by the Roslyn source generator at compile time. ` |------------|----------|------| | SM0039 | Warning | Interceptor has transitive DbContext dependency (resolve at interception time) | -### Localization +### Feature Flags | Diagnostic | Severity | Rule | |------------|----------|------| -| SM0049 | Warning | Module has `IStringLocalizer` injection but no `Locales/en.json` embedded resource | -| SM0050 | Warning | `Locales/en.json` exists but is not marked as `EmbeddedResource` in `.csproj` | +| SM0045 | Error | Feature class must be sealed | +| SM0046 | Warning | Feature field must follow `ModuleName.FeatureName` pattern | +| SM0047 | Error | No duplicate feature names across modules | +| SM0048 | Error | Feature field must be a public const string | -### Module Structure +### Endpoints + +| Diagnostic | Severity | Rule | +|------------|----------|------| +| SM0049 | Error | Each endpoint must be in its own file | +| SM0054 | Info | Endpoint should declare a `public const string Route` field | + +### Module Metadata | Diagnostic | Severity | Rule | |------------|----------|------| @@ -507,6 +516,22 @@ All SM diagnostics are emitted by the Roslyn source generator at compile time. ` - Interceptors are resolved lazily to avoid circular DI. - `ApplyModuleSchema` must handle PostgreSQL, SQL Server, and SQLite. +### Database Migrations + +- One migration history shared by all modules. Run `dotnet ef migrations add --project template/SimpleModule.Host` to create a migration. +- The unified `HostDbContext` (source-generated) owns all DbSets across modules. Migrations target this context. +- When two modules add migrations concurrently, resolve conflicts by regenerating the later migration against the merged model snapshot. +- SQLite uses table prefixes (`{ModuleName}_`) for logical isolation. PostgreSQL and SQL Server use schema isolation (`{ModuleName}.`). +- Never modify or delete existing migrations that have been applied in production. Add corrective migrations instead. + +### Logging Conventions + +- Inject `ILogger` via primary constructor. Use the module's service class as the type parameter. +- Use source-generated logging via `[LoggerMessage]` attribute for all log messages. This is required by the `partial class` pattern and produces high-performance, zero-allocation log calls. +- **Log levels**: `Debug` for lifecycle events (module started/stopped). `Information` for successful operations (entity created/updated/deleted). `Warning` for expected failures (not found, validation). `Error` for unexpected failures (exceptions, infrastructure). +- **Structured fields**: Always include entity IDs and names as named parameters (e.g., `{ProductId}`, `{ProductName}`). The runtime logging infrastructure adds correlation IDs via `System.Diagnostics.Activity.Current.TraceId`. +- Do not log sensitive data (passwords, tokens, PII). The AuditLogs module handles redaction for audit trails separately. + ### Core Framework - All `IModule` methods must have default implementations. diff --git a/framework/SimpleModule.Core/Constants/HealthCheckConstants.cs b/framework/SimpleModule.Core/Constants/HealthCheckConstants.cs index f6ab9850..35250d65 100644 --- a/framework/SimpleModule.Core/Constants/HealthCheckConstants.cs +++ b/framework/SimpleModule.Core/Constants/HealthCheckConstants.cs @@ -3,6 +3,7 @@ public static class HealthCheckConstants { public const string DatabaseCheckName = "database"; + public const string ModulesCheckName = "modules"; public const string ReadyTag = "ready"; public const string AllDatabasesReachable = "All module databases are reachable."; public const string DatabaseHealthCheckFailed = "Database health check failed."; diff --git a/framework/SimpleModule.Core/Health/ModuleHealthCheck.cs b/framework/SimpleModule.Core/Health/ModuleHealthCheck.cs new file mode 100644 index 00000000..e4f19cc9 --- /dev/null +++ b/framework/SimpleModule.Core/Health/ModuleHealthCheck.cs @@ -0,0 +1,83 @@ +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using Microsoft.Extensions.Diagnostics.HealthChecks; + +namespace SimpleModule.Core.Health; + +/// +/// Aggregates health status from all discovered modules by calling +/// on each one in parallel. +/// +public sealed class ModuleHealthCheck : IHealthCheck +{ + private readonly (IModule Module, string Name)[] _modules; + + public ModuleHealthCheck(IEnumerable modules) + { + _modules = modules.Select(m => (m, GetModuleName(m))).ToArray(); + } + + public async Task CheckHealthAsync( + HealthCheckContext context, + CancellationToken cancellationToken = default + ) + { + var tasks = _modules.Select(entry => + CheckOneAsync(entry.Module, entry.Name, cancellationToken) + ); + var results = await Task.WhenAll(tasks); + + var data = new Dictionary(results.Length); + var worstStatus = ModuleHealthStatus.Healthy; + foreach (var (name, status, detail) in results) + { + data[name] = detail; + if (status > worstStatus) + { + worstStatus = status; + } + } + + return worstStatus switch + { + ModuleHealthStatus.Healthy => HealthCheckResult.Healthy( + "All modules are healthy.", + data + ), + ModuleHealthStatus.Degraded => HealthCheckResult.Degraded( + "One or more modules are degraded.", + data: data + ), + _ => HealthCheckResult.Unhealthy("One or more modules are unhealthy.", data: data), + }; + } + + [SuppressMessage( + "Design", + "CA1031:Do not catch general exception types", + Justification = "Health check must report failures, not throw" + )] + private static async Task<( + string Name, + ModuleHealthStatus Status, + string Detail + )> CheckOneAsync(IModule module, string name, CancellationToken cancellationToken) + { + try + { + var status = await module.CheckHealthAsync(cancellationToken); + return (name, status, status.ToString()); + } + catch (Exception ex) + { + return (name, ModuleHealthStatus.Unhealthy, $"Error: {ex.Message}"); + } + } + + private static string GetModuleName(IModule module) + { + var type = module.GetType(); + var attribute = type.GetCustomAttribute(); + return attribute?.Name ?? type.Name; + } +} diff --git a/framework/SimpleModule.Core/Validation/ValidationBuilder.cs b/framework/SimpleModule.Core/Validation/ValidationBuilder.cs index fff65629..8eafb6ac 100644 --- a/framework/SimpleModule.Core/Validation/ValidationBuilder.cs +++ b/framework/SimpleModule.Core/Validation/ValidationBuilder.cs @@ -1,6 +1,8 @@ +using System.Text.RegularExpressions; + namespace SimpleModule.Core.Validation; -public sealed class ValidationBuilder +public sealed partial class ValidationBuilder { private readonly Dictionary> _errors = []; @@ -20,6 +22,108 @@ public ValidationBuilder AddErrorIf(bool condition, string field, string message return this; } + public ValidationBuilder Required(string? value, string field, string? message = null) + { + return AddErrorIf( + string.IsNullOrWhiteSpace(value), + field, + message ?? $"{field} is required." + ); + } + + public ValidationBuilder MaxLength( + string? value, + string field, + int maxLength, + string? message = null + ) + { + return AddErrorIf( + value is not null && value.Length > maxLength, + field, + message ?? $"{field} must be at most {maxLength} characters." + ); + } + + public ValidationBuilder MinLength( + string? value, + string field, + int minLength, + string? message = null + ) + { + return AddErrorIf( + value is not null && value.Length < minLength, + field, + message ?? $"{field} must be at least {minLength} characters." + ); + } + + public ValidationBuilder LengthBetween( + string? value, + string field, + int minLength, + int maxLength, + string? message = null + ) + { + return AddErrorIf( + value is not null && (value.Length < minLength || value.Length > maxLength), + field, + message ?? $"{field} must be between {minLength} and {maxLength} characters." + ); + } + + public ValidationBuilder MatchesPattern( + string? value, + string field, + string pattern, + string? message = null + ) + { + // Static Regex.IsMatch caches a small set of compiled regexes internally, + // which is fine for the handful of patterns typical callers use. + return AddErrorIf( + value is not null && !Regex.IsMatch(value, pattern, RegexOptions.Compiled), + field, + message ?? $"{field} has an invalid format." + ); + } + + public ValidationBuilder Email(string? value, string field, string? message = null) + { + return AddErrorIf( + !string.IsNullOrWhiteSpace(value) && !EmailRegex().IsMatch(value), + field, + message ?? $"{field} must be a valid email address." + ); + } + + public ValidationBuilder GreaterThan( + decimal value, + string field, + decimal min, + string? message = null + ) + { + return AddErrorIf(value <= min, field, message ?? $"{field} must be greater than {min}."); + } + + public ValidationBuilder Between( + decimal value, + string field, + decimal min, + decimal max, + string? message = null + ) + { + return AddErrorIf( + value < min || value > max, + field, + message ?? $"{field} must be between {min} and {max}." + ); + } + public ValidationResult Build() { if (_errors.Count == 0) @@ -30,4 +134,7 @@ public ValidationResult Build() var errors = _errors.ToDictionary(kvp => kvp.Key, kvp => kvp.Value.ToArray()); return ValidationResult.WithErrors(errors); } + + [GeneratedRegex(@"^[^@\s]+@[^@\s]+\.[^@\s]+$", RegexOptions.Compiled)] + private static partial Regex EmailRegex(); } diff --git a/framework/SimpleModule.Generator/Emitters/ContractRegistryEmitter.cs b/framework/SimpleModule.Generator/Emitters/ContractRegistryEmitter.cs new file mode 100644 index 00000000..234a3d59 --- /dev/null +++ b/framework/SimpleModule.Generator/Emitters/ContractRegistryEmitter.cs @@ -0,0 +1,79 @@ +using System.Collections.Generic; +using System.Linq; +using System.Text; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; + +namespace SimpleModule.Generator; + +/// +/// Emits a registry of all discovered contract interfaces and their implementations. +/// This allows test infrastructure to enumerate and resolve every contract at runtime, +/// catching circular dependencies that only manifest during DI resolution. +/// +internal sealed class ContractRegistryEmitter : IEmitter +{ + public void Emit(SourceProductionContext context, DiscoveryData data) + { + if (data.ContractImplementations.Length == 0) + return; + + // Group by interface and only include interfaces with exactly one valid implementation + // (matching what ModuleExtensionsEmitter actually registers with the DI container). + var implsByInterface = new Dictionary>(); + foreach (var impl in data.ContractImplementations) + { + if (!impl.IsPublic || impl.IsAbstract) + continue; + + if (!implsByInterface.TryGetValue(impl.InterfaceFqn, out var list)) + { + list = new List(); + implsByInterface[impl.InterfaceFqn] = list; + } + list.Add(impl); + } + + var singleImpls = implsByInterface + .Where(kvp => kvp.Value.Count == 1) + .OrderBy(kvp => kvp.Key) + .ToArray(); + + if (singleImpls.Length == 0) + return; + + var sb = new StringBuilder(); + sb.AppendLine("// "); + sb.AppendLine("#pragma warning disable"); + sb.AppendLine("#nullable enable"); + sb.AppendLine(); + sb.AppendLine("namespace SimpleModule.Core;"); + sb.AppendLine(); + sb.AppendLine("/// "); + sb.AppendLine( + "/// Registry of all discovered module contract interfaces that have exactly one" + ); + sb.AppendLine( + "/// registered implementation. Enables tests to enumerate and resolve every contract" + ); + sb.AppendLine( + "/// without having to hardcode the list, catching new circular dependencies as modules" + ); + sb.AppendLine("/// are added."); + sb.AppendLine("/// "); + sb.AppendLine("public static class ModuleContractRegistry"); + sb.AppendLine("{"); + sb.AppendLine(" public static System.Type[] All { get; } = new System.Type[]"); + sb.AppendLine(" {"); + + foreach (var kvp in singleImpls) + { + sb.AppendLine($" typeof({kvp.Key}),"); + } + + sb.AppendLine(" };"); + sb.AppendLine("}"); + + context.AddSource("ContractRegistry.g.cs", SourceText.From(sb.ToString(), Encoding.UTF8)); + } +} diff --git a/framework/SimpleModule.Generator/ModuleDiscovererGenerator.cs b/framework/SimpleModule.Generator/ModuleDiscovererGenerator.cs index edad3111..8bb49255 100644 --- a/framework/SimpleModule.Generator/ModuleDiscovererGenerator.cs +++ b/framework/SimpleModule.Generator/ModuleDiscovererGenerator.cs @@ -21,6 +21,7 @@ public class ModuleDiscovererGenerator : IIncrementalGenerator new HostDbContextEmitter(), new ValueConverterConventionsEmitter(), new DbContextRegistryEmitter(), + new ContractRegistryEmitter(), new AgentExtensionsEmitter(), new LocalizationExtensionsEmitter(), new RoutesEmitter(), diff --git a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs index 0802cc09..12bae494 100644 --- a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs +++ b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs @@ -12,6 +12,7 @@ using SimpleModule.Core.Constants; using SimpleModule.Core.Events; using SimpleModule.Core.Exceptions; +using SimpleModule.Core.Health; using SimpleModule.Core.Inertia; using SimpleModule.Core.Menu; using SimpleModule.Core.RateLimiting; @@ -77,6 +78,11 @@ public static WebApplicationBuilder AddSimpleModuleInfrastructure( builder.Services.AddSingleton(); builder.Services.AddHostedService(); builder.Services.AddScoped(); + // Lazy lets services break factory-lambda cycles + // (e.g. SettingsService ↔ AuditingEventBus via ISettingsContracts). + builder.Services.AddScoped(sp => new Lazy(() => + sp.GetRequiredService() + )); builder.Services.AddScoped(); // Required by EntityInterceptor to access the current HTTP context @@ -106,6 +112,10 @@ public static WebApplicationBuilder AddSimpleModuleInfrastructure( .AddCheck( HealthCheckConstants.DatabaseCheckName, tags: [HealthCheckConstants.ReadyTag] + ) + .AddCheck( + HealthCheckConstants.ModulesCheckName, + tags: [HealthCheckConstants.ReadyTag] ); } @@ -236,6 +246,7 @@ public static async Task UseSimpleModuleInfrastructure(this WebApplication app) new HealthCheckOptions { Predicate = check => check.Tags.Contains(HealthCheckConstants.ReadyTag), + ResponseWriter = WriteHealthCheckResponse, } ) .AllowAnonymous(); @@ -330,4 +341,31 @@ private static void UseHomePageRewrite(WebApplication app) } ); } + + private static async Task WriteHealthCheckResponse( + HttpContext context, + Microsoft.Extensions.Diagnostics.HealthChecks.HealthReport report + ) + { + context.Response.ContentType = "application/json"; + + var entries = report + .Entries.Select(e => new + { + name = e.Key, + status = e.Value.Status.ToString(), + description = e.Value.Description, + data = e.Value.Data, + }) + .ToList(); + + var response = new + { + status = report.Status.ToString(), + totalDuration = report.TotalDuration.TotalMilliseconds, + checks = entries, + }; + + await context.Response.WriteAsJsonAsync(response); + } } diff --git a/modules/FileStorage/src/SimpleModule.FileStorage.Contracts/Events/FileDeletedEvent.cs b/modules/FileStorage/src/SimpleModule.FileStorage.Contracts/Events/FileDeletedEvent.cs new file mode 100644 index 00000000..865ee61d --- /dev/null +++ b/modules/FileStorage/src/SimpleModule.FileStorage.Contracts/Events/FileDeletedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.FileStorage.Contracts.Events; + +public sealed record FileDeletedEvent(FileStorageId FileId, string FileName) : IEvent; diff --git a/modules/FileStorage/src/SimpleModule.FileStorage.Contracts/Events/FileUploadedEvent.cs b/modules/FileStorage/src/SimpleModule.FileStorage.Contracts/Events/FileUploadedEvent.cs new file mode 100644 index 00000000..08cecc30 --- /dev/null +++ b/modules/FileStorage/src/SimpleModule.FileStorage.Contracts/Events/FileUploadedEvent.cs @@ -0,0 +1,10 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.FileStorage.Contracts.Events; + +public sealed record FileUploadedEvent( + FileStorageId FileId, + string FileName, + long FileSize, + string ContentType +) : IEvent; diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageService.cs b/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageService.cs index 7e6c7c17..f74ef249 100644 --- a/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageService.cs +++ b/modules/FileStorage/src/SimpleModule.FileStorage/FileStorageService.cs @@ -1,6 +1,8 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using SimpleModule.Core.Events; using SimpleModule.FileStorage.Contracts; +using SimpleModule.FileStorage.Contracts.Events; using SimpleModule.Storage; namespace SimpleModule.FileStorage; @@ -8,6 +10,7 @@ namespace SimpleModule.FileStorage; public sealed partial class FileStorageService( FileStorageDbContext db, IStorageProvider storageProvider, + IEventBus eventBus, ILogger logger ) : IFileStorageContracts { @@ -77,6 +80,16 @@ public async Task UploadFileAsync( await db.SaveChangesAsync(); LogFileUploaded(logger, storedFile.Id, storedFile.FileName); + + await eventBus.PublishAsync( + new FileUploadedEvent( + storedFile.Id, + storedFile.FileName, + storedFile.Size, + storedFile.ContentType + ) + ); + return storedFile; } catch @@ -115,6 +128,8 @@ public async Task DeleteFileAsync(StoredFile file) } LogFileDeleted(logger, file.Id, file.FileName); + + eventBus.PublishInBackground(new FileDeletedEvent(file.Id, file.FileName)); } public async Task DownloadFileAsync(FileStorageId id) diff --git a/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs b/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs index b827bf4f..b476c89c 100644 --- a/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs +++ b/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Options; using SimpleModule.Database; using SimpleModule.FileStorage.Contracts; +using SimpleModule.Tests.Shared.Fakes; using SimpleModule.Tests.Shared.Storage; namespace SimpleModule.FileStorage.Tests; @@ -37,6 +38,7 @@ public FileStorageServiceTests() _service = new FileStorageService( _db, _storageProvider, + new TestEventBus(), NullLogger.Instance ); } @@ -353,6 +355,7 @@ public async Task UploadFileAsync_Cleans_Up_Storage_On_DB_Failure() var failingService = new FileStorageService( _db, failingProvider, + new TestEventBus(), NullLogger.Instance ); diff --git a/modules/Marketplace/src/SimpleModule.Marketplace/MarketplaceModule.cs b/modules/Marketplace/src/SimpleModule.Marketplace/MarketplaceModule.cs index dbceeee0..9ac61d4b 100644 --- a/modules/Marketplace/src/SimpleModule.Marketplace/MarketplaceModule.cs +++ b/modules/Marketplace/src/SimpleModule.Marketplace/MarketplaceModule.cs @@ -1,6 +1,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Core.Menu; namespace SimpleModule.Marketplace; @@ -29,6 +30,11 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config services.AddSingleton(); } + public void ConfigurePermissions(PermissionRegistryBuilder builder) + { + builder.AddPermissions(); + } + public void ConfigureMenu(IMenuBuilder menus) { menus.Add( diff --git a/modules/Marketplace/src/SimpleModule.Marketplace/MarketplacePermissions.cs b/modules/Marketplace/src/SimpleModule.Marketplace/MarketplacePermissions.cs new file mode 100644 index 00000000..d23073f2 --- /dev/null +++ b/modules/Marketplace/src/SimpleModule.Marketplace/MarketplacePermissions.cs @@ -0,0 +1,10 @@ +using SimpleModule.Core.Authorization; + +namespace SimpleModule.Marketplace; + +public sealed class MarketplacePermissions : IModulePermissions +{ + public const string View = "Marketplace.View"; + public const string Install = "Marketplace.Install"; + public const string Uninstall = "Marketplace.Uninstall"; +} diff --git a/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PageCreatedEvent.cs b/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PageCreatedEvent.cs new file mode 100644 index 00000000..cf333aef --- /dev/null +++ b/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PageCreatedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.PageBuilder.Contracts.Events; + +public sealed record PageCreatedEvent(PageId PageId, string Title, string Slug) : IEvent; diff --git a/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PageDeletedEvent.cs b/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PageDeletedEvent.cs new file mode 100644 index 00000000..cdaa3b1c --- /dev/null +++ b/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PageDeletedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.PageBuilder.Contracts.Events; + +public sealed record PageDeletedEvent(PageId PageId) : IEvent; diff --git a/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PagePublishedEvent.cs b/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PagePublishedEvent.cs new file mode 100644 index 00000000..4f96e35b --- /dev/null +++ b/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PagePublishedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.PageBuilder.Contracts.Events; + +public sealed record PagePublishedEvent(PageId PageId, string Title) : IEvent; diff --git a/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PageUnpublishedEvent.cs b/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PageUnpublishedEvent.cs new file mode 100644 index 00000000..4a24e8a6 --- /dev/null +++ b/modules/PageBuilder/src/SimpleModule.PageBuilder.Contracts/Events/PageUnpublishedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.PageBuilder.Contracts.Events; + +public sealed record PageUnpublishedEvent(PageId PageId, string Title) : IEvent; diff --git a/modules/PageBuilder/src/SimpleModule.PageBuilder/PageBuilderService.cs b/modules/PageBuilder/src/SimpleModule.PageBuilder/PageBuilderService.cs index 93fa344c..871eaeb9 100644 --- a/modules/PageBuilder/src/SimpleModule.PageBuilder/PageBuilderService.cs +++ b/modules/PageBuilder/src/SimpleModule.PageBuilder/PageBuilderService.cs @@ -2,13 +2,16 @@ using System.Text.RegularExpressions; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using SimpleModule.Core.Events; using SimpleModule.Core.Exceptions; using SimpleModule.PageBuilder.Contracts; +using SimpleModule.PageBuilder.Contracts.Events; namespace SimpleModule.PageBuilder; public sealed partial class PageBuilderService( PageBuilderDbContext db, + IEventBus eventBus, ILogger logger ) : IPageBuilderContracts, IPageBuilderTemplateContracts, IPageBuilderTagContracts { @@ -95,6 +98,9 @@ public async Task CreatePageAsync(CreatePageRequest request) await db.SaveChangesAsync(); LogPageCreated(logger, page.Id, page.Title); + + await eventBus.PublishAsync(new PageCreatedEvent(page.Id, page.Title, page.Slug)); + return page; } @@ -150,6 +156,8 @@ public async Task DeletePageAsync(PageId id) await db.SaveChangesAsync(); LogPageDeleted(logger, id); + + eventBus.PublishInBackground(new PageDeletedEvent(id)); } public async Task PublishPageAsync(PageId id) @@ -167,6 +175,9 @@ public async Task PublishPageAsync(PageId id) await db.SaveChangesAsync(); LogPagePublished(logger, page.Id, page.Title); + + await eventBus.PublishAsync(new PagePublishedEvent(page.Id, page.Title)); + return page; } @@ -179,6 +190,9 @@ public async Task UnpublishPageAsync(PageId id) await db.SaveChangesAsync(); LogPageUnpublished(logger, page.Id, page.Title); + + await eventBus.PublishAsync(new PageUnpublishedEvent(page.Id, page.Title)); + return page; } diff --git a/modules/PageBuilder/tests/SimpleModule.PageBuilder.Tests/PageBuilderServiceTests.cs b/modules/PageBuilder/tests/SimpleModule.PageBuilder.Tests/PageBuilderServiceTests.cs index 47b9f98a..ead7bd39 100644 --- a/modules/PageBuilder/tests/SimpleModule.PageBuilder.Tests/PageBuilderServiceTests.cs +++ b/modules/PageBuilder/tests/SimpleModule.PageBuilder.Tests/PageBuilderServiceTests.cs @@ -6,6 +6,7 @@ using SimpleModule.Database; using SimpleModule.PageBuilder; using SimpleModule.PageBuilder.Contracts; +using SimpleModule.Tests.Shared.Fakes; namespace PageBuilder.Tests; @@ -31,7 +32,11 @@ public PageBuilderServiceTests() _db = new PageBuilderDbContext(options, dbOptions); _db.Database.OpenConnection(); _db.Database.EnsureCreated(); - _sut = new PageBuilderService(_db, NullLogger.Instance); + _sut = new PageBuilderService( + _db, + new TestEventBus(), + NullLogger.Instance + ); } public void Dispose() => _db.Dispose(); diff --git a/modules/Products/src/SimpleModule.Products.Contracts/Events/ProductCreatedEvent.cs b/modules/Products/src/SimpleModule.Products.Contracts/Events/ProductCreatedEvent.cs new file mode 100644 index 00000000..299a5bcd --- /dev/null +++ b/modules/Products/src/SimpleModule.Products.Contracts/Events/ProductCreatedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.Products.Contracts.Events; + +public sealed record ProductCreatedEvent(ProductId ProductId, string Name, decimal Price) : IEvent; diff --git a/modules/Products/src/SimpleModule.Products.Contracts/Events/ProductDeletedEvent.cs b/modules/Products/src/SimpleModule.Products.Contracts/Events/ProductDeletedEvent.cs new file mode 100644 index 00000000..95257f5b --- /dev/null +++ b/modules/Products/src/SimpleModule.Products.Contracts/Events/ProductDeletedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.Products.Contracts.Events; + +public sealed record ProductDeletedEvent(ProductId ProductId) : IEvent; diff --git a/modules/Products/src/SimpleModule.Products.Contracts/Events/ProductUpdatedEvent.cs b/modules/Products/src/SimpleModule.Products.Contracts/Events/ProductUpdatedEvent.cs new file mode 100644 index 00000000..42f318d4 --- /dev/null +++ b/modules/Products/src/SimpleModule.Products.Contracts/Events/ProductUpdatedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.Products.Contracts.Events; + +public sealed record ProductUpdatedEvent(ProductId ProductId, string Name, decimal Price) : IEvent; diff --git a/modules/Products/src/SimpleModule.Products/ProductService.cs b/modules/Products/src/SimpleModule.Products/ProductService.cs index 0575609d..ee199551 100644 --- a/modules/Products/src/SimpleModule.Products/ProductService.cs +++ b/modules/Products/src/SimpleModule.Products/ProductService.cs @@ -1,11 +1,16 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using SimpleModule.Core.Events; using SimpleModule.Products.Contracts; +using SimpleModule.Products.Contracts.Events; namespace SimpleModule.Products; -public partial class ProductService(ProductsDbContext db, ILogger logger) - : IProductContracts +public partial class ProductService( + ProductsDbContext db, + IEventBus eventBus, + ILogger logger +) : IProductContracts { public async Task> GetAllProductsAsync() => await db.Products.AsNoTracking().ToListAsync(); @@ -36,6 +41,10 @@ public async Task CreateProductAsync(CreateProductRequest request) LogProductCreated(logger, product.Id, product.Name); + await eventBus.PublishAsync( + new ProductCreatedEvent(product.Id, product.Name, product.Price) + ); + return product; } @@ -54,6 +63,10 @@ public async Task UpdateProductAsync(ProductId id, UpdateProductRequest LogProductUpdated(logger, product.Id, product.Name); + await eventBus.PublishAsync( + new ProductUpdatedEvent(product.Id, product.Name, product.Price) + ); + return product; } @@ -69,6 +82,8 @@ public async Task DeleteProductAsync(ProductId id) await db.SaveChangesAsync(); LogProductDeleted(logger, id); + + eventBus.PublishInBackground(new ProductDeletedEvent(id)); } [LoggerMessage(Level = LogLevel.Warning, Message = "Product with ID {ProductId} not found")] diff --git a/modules/Products/tests/SimpleModule.Products.Tests/Unit/ProductServiceTests.cs b/modules/Products/tests/SimpleModule.Products.Tests/Unit/ProductServiceTests.cs index d6c1be7d..729279cd 100644 --- a/modules/Products/tests/SimpleModule.Products.Tests/Unit/ProductServiceTests.cs +++ b/modules/Products/tests/SimpleModule.Products.Tests/Unit/ProductServiceTests.cs @@ -6,6 +6,7 @@ using SimpleModule.Database; using SimpleModule.Products; using SimpleModule.Products.Contracts; +using SimpleModule.Tests.Shared.Fakes; namespace Products.Tests.Unit; @@ -31,7 +32,7 @@ public ProductServiceTests() _db = new ProductsDbContext(options, dbOptions); _db.Database.OpenConnection(); _db.Database.EnsureCreated(); - _sut = new ProductService(_db, NullLogger.Instance); + _sut = new ProductService(_db, new TestEventBus(), NullLogger.Instance); } public void Dispose() => _db.Dispose(); diff --git a/modules/Settings/src/SimpleModule.Settings.Contracts/Events/SettingChangedEvent.cs b/modules/Settings/src/SimpleModule.Settings.Contracts/Events/SettingChangedEvent.cs new file mode 100644 index 00000000..fea38408 --- /dev/null +++ b/modules/Settings/src/SimpleModule.Settings.Contracts/Events/SettingChangedEvent.cs @@ -0,0 +1,11 @@ +using SimpleModule.Core.Events; +using SimpleModule.Core.Settings; + +namespace SimpleModule.Settings.Contracts.Events; + +public sealed record SettingChangedEvent( + string Key, + string? OldValue, + string? NewValue, + SettingScope Scope +) : IEvent; diff --git a/modules/Settings/src/SimpleModule.Settings.Contracts/Events/SettingDeletedEvent.cs b/modules/Settings/src/SimpleModule.Settings.Contracts/Events/SettingDeletedEvent.cs new file mode 100644 index 00000000..6359935d --- /dev/null +++ b/modules/Settings/src/SimpleModule.Settings.Contracts/Events/SettingDeletedEvent.cs @@ -0,0 +1,6 @@ +using SimpleModule.Core.Events; +using SimpleModule.Core.Settings; + +namespace SimpleModule.Settings.Contracts.Events; + +public sealed record SettingDeletedEvent(string Key, SettingScope Scope) : IEvent; diff --git a/modules/Settings/src/SimpleModule.Settings/SettingsModule.cs b/modules/Settings/src/SimpleModule.Settings/SettingsModule.cs index 949a19c7..11e14e73 100644 --- a/modules/Settings/src/SimpleModule.Settings/SettingsModule.cs +++ b/modules/Settings/src/SimpleModule.Settings/SettingsModule.cs @@ -1,6 +1,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Core.Menu; using SimpleModule.Core.Settings; using SimpleModule.Database; @@ -38,6 +39,11 @@ public void ConfigureMenu(IMenuBuilder menus) ); } + public void ConfigurePermissions(PermissionRegistryBuilder builder) + { + builder.AddPermissions(); + } + public void ConfigureSettings(ISettingsBuilder settings) { settings diff --git a/modules/Settings/src/SimpleModule.Settings/SettingsPermissions.cs b/modules/Settings/src/SimpleModule.Settings/SettingsPermissions.cs new file mode 100644 index 00000000..bae594ef --- /dev/null +++ b/modules/Settings/src/SimpleModule.Settings/SettingsPermissions.cs @@ -0,0 +1,10 @@ +using SimpleModule.Core.Authorization; + +namespace SimpleModule.Settings; + +public sealed class SettingsPermissions : IModulePermissions +{ + public const string View = "Settings.View"; + public const string Update = "Settings.Update"; + public const string ManageMenus = "Settings.ManageMenus"; +} diff --git a/modules/Settings/src/SimpleModule.Settings/SettingsService.cs b/modules/Settings/src/SimpleModule.Settings/SettingsService.cs index f20175c4..8a58f38e 100644 --- a/modules/Settings/src/SimpleModule.Settings/SettingsService.cs +++ b/modules/Settings/src/SimpleModule.Settings/SettingsService.cs @@ -3,8 +3,10 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using SimpleModule.Core.Caching; +using SimpleModule.Core.Events; using SimpleModule.Core.Settings; using SimpleModule.Settings.Contracts; +using SimpleModule.Settings.Contracts.Events; namespace SimpleModule.Settings; @@ -12,6 +14,7 @@ public sealed partial class SettingsService( SettingsDbContext db, ISettingsDefinitionRegistry definitions, ICacheStore cache, + Lazy eventBus, IOptions moduleOptions, ILogger logger ) : ISettingsContracts @@ -88,6 +91,8 @@ public async Task SetSettingAsync( && (scope == SettingScope.User ? s.UserId == userId : s.UserId == null) ); + var oldValue = existing?.Value; + if (existing is not null) { existing.Value = value; @@ -108,6 +113,10 @@ public async Task SetSettingAsync( await db.SaveChangesAsync(); await cache.RemoveAsync(BuildCacheKey(key, scope, userId)); LogSettingUpdated(key, scope); + + // IEventBus is Lazy to break the SettingsService → IEventBus → AuditingEventBus + // → ISettingsContracts → SettingsService cycle at construction time. + await eventBus.Value.PublishAsync(new SettingChangedEvent(key, oldValue, value, scope)); } public async Task DeleteSettingAsync(string key, SettingScope scope, string? userId = null) @@ -124,6 +133,8 @@ public async Task DeleteSettingAsync(string key, SettingScope scope, string? use await db.SaveChangesAsync(); await cache.RemoveAsync(BuildCacheKey(key, scope, userId)); LogSettingDeleted(key, scope); + + eventBus.Value.PublishInBackground(new SettingDeletedEvent(key, scope)); } } diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/SettingsServiceTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/SettingsServiceTests.cs index 24328241..241d8a8d 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/SettingsServiceTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/SettingsServiceTests.cs @@ -4,9 +4,11 @@ using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using SimpleModule.Core.Caching; +using SimpleModule.Core.Events; using SimpleModule.Core.Settings; using SimpleModule.Database; using SimpleModule.Settings; +using SimpleModule.Tests.Shared.Fakes; namespace Settings.Tests.Unit; @@ -41,10 +43,12 @@ public SettingsServiceTests() _cache = new MemoryCache(new MemoryCacheOptions()); _cacheStore = new MemoryCacheStore(_cache); + _service = new SettingsService( _db, registry, _cacheStore, + new Lazy(() => new TestEventBus()), Options.Create(new SettingsModuleOptions()), NullLogger.Instance ); diff --git a/modules/Users/src/SimpleModule.Users.Contracts/Events/UserCreatedEvent.cs b/modules/Users/src/SimpleModule.Users.Contracts/Events/UserCreatedEvent.cs new file mode 100644 index 00000000..8cdc8023 --- /dev/null +++ b/modules/Users/src/SimpleModule.Users.Contracts/Events/UserCreatedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.Users.Contracts.Events; + +public sealed record UserCreatedEvent(UserId UserId, string Email, string DisplayName) : IEvent; diff --git a/modules/Users/src/SimpleModule.Users.Contracts/Events/UserDeletedEvent.cs b/modules/Users/src/SimpleModule.Users.Contracts/Events/UserDeletedEvent.cs new file mode 100644 index 00000000..46ee4b23 --- /dev/null +++ b/modules/Users/src/SimpleModule.Users.Contracts/Events/UserDeletedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.Users.Contracts.Events; + +public sealed record UserDeletedEvent(UserId UserId) : IEvent; diff --git a/modules/Users/src/SimpleModule.Users.Contracts/Events/UserRolesChangedEvent.cs b/modules/Users/src/SimpleModule.Users.Contracts/Events/UserRolesChangedEvent.cs new file mode 100644 index 00000000..ea55877b --- /dev/null +++ b/modules/Users/src/SimpleModule.Users.Contracts/Events/UserRolesChangedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.Users.Contracts.Events; + +public sealed record UserRolesChangedEvent(UserId UserId, IReadOnlyList Roles) : IEvent; diff --git a/modules/Users/src/SimpleModule.Users.Contracts/Events/UserUpdatedEvent.cs b/modules/Users/src/SimpleModule.Users.Contracts/Events/UserUpdatedEvent.cs new file mode 100644 index 00000000..361b0cf1 --- /dev/null +++ b/modules/Users/src/SimpleModule.Users.Contracts/Events/UserUpdatedEvent.cs @@ -0,0 +1,5 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.Users.Contracts.Events; + +public sealed record UserUpdatedEvent(UserId UserId, string Email, string DisplayName) : IEvent; diff --git a/modules/Users/src/SimpleModule.Users/UserAdminService.cs b/modules/Users/src/SimpleModule.Users/UserAdminService.cs index 73571d58..3877ad8a 100644 --- a/modules/Users/src/SimpleModule.Users/UserAdminService.cs +++ b/modules/Users/src/SimpleModule.Users/UserAdminService.cs @@ -1,13 +1,18 @@ using Microsoft.AspNetCore.Identity; using Microsoft.EntityFrameworkCore; using SimpleModule.Core; +using SimpleModule.Core.Events; using SimpleModule.Core.Exceptions; using SimpleModule.Users.Contracts; +using SimpleModule.Users.Contracts.Events; namespace SimpleModule.Users; -public sealed class UserAdminService(UserManager userManager, UsersDbContext db) - : IUserAdminContracts +public sealed class UserAdminService( + UserManager userManager, + UsersDbContext db, + IEventBus eventBus +) : IUserAdminContracts { public async Task> GetUsersPagedAsync( string? search, @@ -135,6 +140,11 @@ public async Task CreateUserWithPasswordAsync(CreateAdminUserReque } var roles = await userManager.GetRolesAsync(user); + + await eventBus.PublishAsync( + new UserCreatedEvent(UserId.From(user.Id), user.Email ?? string.Empty, user.DisplayName) + ); + return MapToAdminDto(user, roles.ToList()); } @@ -149,6 +159,10 @@ public async Task UpdateUserDetailsAsync(UserId id, UpdateAdminUserRequest reque user.EmailConfirmed = request.EmailConfirmed; await userManager.UpdateAsync(user); + + await eventBus.PublishAsync( + new UserUpdatedEvent(UserId.From(user.Id), user.Email ?? string.Empty, user.DisplayName) + ); } public async Task SetUserRolesAsync(UserId id, IEnumerable roles) @@ -171,6 +185,8 @@ public async Task SetUserRolesAsync(UserId id, IEnumerable roles) { await userManager.AddToRolesAsync(user, toAdd); } + + await eventBus.PublishAsync(new UserRolesChangedEvent(id, newRoles.ToList())); } public async Task ResetPasswordAsync(UserId id, string newPassword) diff --git a/modules/Users/src/SimpleModule.Users/UserService.cs b/modules/Users/src/SimpleModule.Users/UserService.cs index d2191b8d..080d44b0 100644 --- a/modules/Users/src/SimpleModule.Users/UserService.cs +++ b/modules/Users/src/SimpleModule.Users/UserService.cs @@ -1,13 +1,16 @@ using Microsoft.AspNetCore.Identity; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using SimpleModule.Core.Events; using SimpleModule.Users.Contracts; +using SimpleModule.Users.Contracts.Events; namespace SimpleModule.Users; public partial class UserService( UserManager userManager, RoleManager roleManager, + IEventBus eventBus, ILogger logger ) : IUserContracts { @@ -62,6 +65,10 @@ public async Task CreateUserAsync(CreateUserRequest request) LogUserCreated(logger, user.Id, user.Email); + await eventBus.PublishAsync( + new UserCreatedEvent(UserId.From(user.Id), user.Email ?? string.Empty, user.DisplayName) + ); + return MapToDto(user); } @@ -81,6 +88,10 @@ public async Task UpdateUserAsync(UserId id, UpdateUserRequest request) LogUserUpdated(logger, user.Id); + await eventBus.PublishAsync( + new UserUpdatedEvent(UserId.From(user.Id), user.Email ?? string.Empty, user.DisplayName) + ); + return MapToDto(user); } @@ -95,6 +106,8 @@ public async Task DeleteUserAsync(UserId id) await userManager.DeleteAsync(user); LogUserDeleted(logger, id); + + eventBus.PublishInBackground(new UserDeletedEvent(id)); } public async Task> GetRoleIdsByNamesAsync( diff --git a/modules/Users/src/SimpleModule.Users/UsersModule.cs b/modules/Users/src/SimpleModule.Users/UsersModule.cs index 1168807e..8af07284 100644 --- a/modules/Users/src/SimpleModule.Users/UsersModule.cs +++ b/modules/Users/src/SimpleModule.Users/UsersModule.cs @@ -3,6 +3,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Core.Menu; using SimpleModule.Core.Settings; using SimpleModule.Database; @@ -45,6 +46,11 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config services.AddSingleton, ConsoleEmailSender>(); } + public void ConfigurePermissions(PermissionRegistryBuilder builder) + { + builder.AddPermissions(); + } + public void ConfigureSettings(ISettingsBuilder settings) { settings.Add( diff --git a/modules/Users/src/SimpleModule.Users/UsersPermissions.cs b/modules/Users/src/SimpleModule.Users/UsersPermissions.cs new file mode 100644 index 00000000..b4f66ca2 --- /dev/null +++ b/modules/Users/src/SimpleModule.Users/UsersPermissions.cs @@ -0,0 +1,12 @@ +using SimpleModule.Core.Authorization; + +namespace SimpleModule.Users; + +public sealed class UsersPermissions : IModulePermissions +{ + public const string View = "Users.View"; + public const string Create = "Users.Create"; + public const string Update = "Users.Update"; + public const string Delete = "Users.Delete"; + public const string ManageRoles = "Users.ManageRoles"; +} diff --git a/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserServiceTests.cs b/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserServiceTests.cs index dc318c05..c79fa997 100644 --- a/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserServiceTests.cs +++ b/modules/Users/tests/SimpleModule.Users.Tests/Unit/UserServiceTests.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; +using SimpleModule.Tests.Shared.Fakes; using SimpleModule.Users; using SimpleModule.Users.Contracts; @@ -33,7 +34,12 @@ public UserServiceTests() null, null ); - _sut = new UserService(_userManager, _roleManager, NullLogger.Instance); + _sut = new UserService( + _userManager, + _roleManager, + new TestEventBus(), + NullLogger.Instance + ); } [Fact] diff --git a/tasks/framework-gaps-review.md b/tasks/framework-gaps-review.md new file mode 100644 index 00000000..3c3dd5dc --- /dev/null +++ b/tasks/framework-gaps-review.md @@ -0,0 +1,223 @@ +# SimpleModule Framework Gap Analysis + +**Date:** 2026-04-10 +**Scope:** Full framework review across architecture, modules, testing, frontend, security, and source generator diagnostics. + +--- + +## Executive Summary + +SimpleModule is a well-architected modular monolith with strong foundations: compile-time module discovery, a comprehensive source generator with 37 diagnostics, fine-grained permission system, and solid security posture. However, the review identified **28 gaps** across 7 categories ranging from documentation drift to missing cross-cutting features. + +--- + +## 1. Documentation & Diagnostic Drift + +### 1.1 CLAUDE.md says "SM0001-SM0044" but implementation has SM0001-SM0054 +**Severity:** Medium +The source generator implements 37 diagnostics up through SM0054, but CLAUDE.md only documents through SM0044. Nine diagnostics (SM0045-SM0054) covering feature flags, module naming, endpoint structure, and contracts are undocumented. + +### 1.2 SM0049 numbering mismatch in CONSTITUTION.md +**Severity:** High +The Constitution says SM0049 is "Module has `IStringLocalizer` injection but no `Locales/en.json` embedded resource." The actual implementation is "Multiple endpoints in a single file" (Error). The localization diagnostic was apparently renumbered or displaced. + +### 1.3 SM0050 defined in Constitution but not implemented +**Severity:** Medium +Constitution defines SM0050 as "Locales/en.json exists but is not marked as `EmbeddedResource` in `.csproj`." This rule has no enforcement in the source generator. Localization resource validation is entirely manual. + +### 1.4 Large diagnostic ID gaps suggest abandoned or deferred plans +**Severity:** Low +Gaps at SM0008-0009, SM0016-0024, SM0030, SM0036-0037, SM0051 suggest rules were planned but never implemented. No tracking of what those were intended to be. + +--- + +## 2. Cross-Module Communication Gaps + +### 2.1 Only 6 of 23 modules publish events +**Severity:** High +The event bus exists and works well (with pipeline behaviors, background dispatch, exception isolation), but only Email, Tenants, Agents, FeatureFlags, Orders, and Datasets publish events. Major modules that should publish events but don't: + +- **Products** — no event on create/update/delete (Orders module can't react to product changes) +- **Users** — no event on registration, profile update, role change, password reset +- **FileStorage** — no event on upload/delete (AuditLogs could subscribe) +- **PageBuilder** — no event on publish/unpublish +- **Permissions** — no event on role/permission changes +- **Settings** — no event on setting value changes (modules can't react to config changes) + +### 2.2 No event handler discovery pattern +**Severity:** Medium +Events are defined in Contracts projects, but there are no standalone `IEventHandler` implementation files visible. Handlers appear to be registered via lambdas in `ConfigureServices`. This makes it hard to discover which modules subscribe to which events. A convention for handler classes (like endpoint classes) would improve discoverability. + +### 2.3 No saga/orchestration pattern for multi-step workflows +**Severity:** Low (documented as out of scope, but worth noting) +Cross-module workflows (e.g., "create order -> reserve product -> send email -> log audit") rely on sequential event handlers. There's no compensating transaction or saga pattern if a step fails mid-way. + +--- + +## 3. Module Consistency Issues + +### 3.1 Inconsistent contract interface naming +**Severity:** Low +The Rag module uses `SimpleModule.Rag.Module` instead of `SimpleModule.Rag` for its implementation project. All other modules follow the `SimpleModule.{Name}` convention. + +### 3.2 Modules without permissions that probably should have them +**Severity:** Medium +8 modules have no permission definitions: Agents, Dashboard, Localization, Marketplace, Permissions (meta), Rag, Settings, Users. Some of these are reasonable (Dashboard is read-only), but: + +- **Settings** — managing system-wide settings should require admin permissions +- **Users** — user management operations should have granular permissions (the module appears to rely on role checks instead of the permission system) +- **Marketplace** — install/uninstall operations should be permission-gated + +### 3.3 FakeDataGenerators only covers 3 of 23 modules +**Severity:** Medium +The shared `FakeDataGenerators` in `SimpleModule.Tests.Shared` only has pre-built Bogus fakers for Users, Products, and Orders. The other 20 modules either create fakers inline in tests or don't have them, leading to inconsistent test data patterns. + +### 3.4 Five modules have no API endpoints (Agents, Dashboard, Localization, Permissions, Rag) +**Severity:** Low +Some are by design (Localization provides translations via shared data, Permissions is middleware-based). But Agents uses a custom `AgentEndpoints.MapAgentEndpoints()` escape hatch rather than the standard `IEndpoint` pattern, which means it bypasses source generator discovery and diagnostics. + +--- + +## 4. Frontend Gaps + +### 4.1 No form validation library +**Severity:** High +There is no client-side form validation framework integrated. All validation is server-side only, which means: +- Users don't get instant feedback on invalid input +- Every validation round-trip requires a server request +- No type-safe form handling (React Hook Form + Zod would be natural fits) + +### 4.2 Missing UI components for a full-featured framework +**Severity:** Medium +The UI library has 48 components (excellent foundation), but lacks: +- **Combobox / Async Select** — needed for entity lookups +- **Multi-select** — needed for tag/category assignment +- **File upload component** — FileStorage module exists but no upload widget +- **Date range picker** — needed for audit log filtering, reports +- **Rich text editor** — PageBuilder exists but no WYSIWYG component +- **Error page templates** — no 404/500/403 page components + +### 4.3 No network error handling +**Severity:** Medium +The global HTTP error handler converts non-Inertia errors to toasts, but: +- Network failures (offline, timeout) are not explicitly handled +- No offline detection or retry UI +- No request timeout configuration +- Users see browser-default errors for network issues + +### 4.4 No loading state management +**Severity:** Low +Individual pages handle loading states manually. No framework-level convention for: +- Optimistic updates +- Skeleton loading patterns +- Navigation progress beyond the basic progress bar + +### 4.5 Four modules have no frontend (Agents, Localization, Permissions, Rag) +**Severity:** Low +Backend-only modules are valid, but Agents and Rag could benefit from admin UIs for: +- Viewing agent execution history and tool calls +- Managing RAG document ingestion and search testing + +--- + +## 5. Testing Gaps + +### 5.1 No integration tests for event bus cross-module flows +**Severity:** High +The event bus has unit tests for publish/subscribe mechanics, but there are no integration tests verifying that Module A publishing an event correctly triggers Module B's handler in the full application context. + +### 5.2 E2E tests only run Chromium +**Severity:** Low +Playwright is configured for Chromium only. Firefox and WebKit are not tested, which could miss browser-specific rendering or JS behavior issues. + +### 5.3 No contract testing between modules +**Severity:** Medium +When a Contracts interface changes, there's no automated check that all consumers still compile and work correctly. The source generator catches missing implementations (SM0025), but not behavioral contract violations (e.g., a method now throws where it didn't before). + +### 5.4 No mutation testing +**Severity:** Low +Test coverage is measured by line/branch coverage, but there's no mutation testing (e.g., Stryker.NET) to verify that tests actually catch regressions vs. just executing code paths. + +--- + +## 6. Security Gaps + +### 6.1 Limited string input validation +**Severity:** Medium +The custom validation framework (`ValidationBuilder`) handles null/empty and numeric range checks, but there's limited evidence of: +- String length limits on user input fields +- Pattern validation (email format, phone numbers) +- HTML/script injection prevention at the input layer (relies on framework-level output encoding) + +### 6.2 No request body size limits beyond file uploads +**Severity:** Low +File uploads have configurable size limits, but general API request bodies don't appear to have explicit size limits beyond ASP.NET defaults (28.6MB). A crafted large JSON payload could consume server memory. + +### 6.3 No IP-based blocking or suspicious activity detection +**Severity:** Low +Rate limiting exists (per-IP, per-user, fixed/sliding/token-bucket), but there's no: +- IP blocklist capability +- Account lockout after failed login attempts (beyond ASP.NET Identity defaults) +- Anomaly detection for unusual request patterns + +--- + +## 7. Infrastructure & Operations Gaps + +### 7.1 No health check aggregation endpoint +**Severity:** Medium +Each module implements `CheckHealthAsync()` returning `ModuleHealthStatus`, but there's no visible `/health` endpoint that aggregates all module health statuses into a single response for load balancers/orchestrators. + +### 7.2 No structured logging convention +**Severity:** Medium +Modules use standard `ILogger` but there's no framework-level convention for: +- Structured log fields (correlation IDs, module name, endpoint name) +- Log level guidelines per module +- Centralized log configuration beyond what ASP.NET provides by default + +### 7.3 No database migration strategy documentation +**Severity:** Medium +The Constitution says "one migration history" but there's no documented pattern for: +- How modules add migrations +- How migration conflicts between modules are resolved +- Whether EF Core migrations are used vs. a different strategy + +### 7.4 No module dependency graph visualization +**Severity:** Low +The source generator detects circular dependencies (SM0010) and illegal implementation references (SM0011), but there's no tool to visualize the actual module dependency graph. This would help new developers understand the architecture. + +### 7.5 No OpenTelemetry / distributed tracing integration +**Severity:** Low +For a modular monolith that could eventually extract modules, having tracing spans per module/endpoint would be valuable for performance debugging and future extraction planning. + +--- + +## Priority Ranking + +### P0 — Fix Now (Correctness Issues) +1. **SM0049 numbering mismatch** in Constitution — misleading documentation +2. **CLAUDE.md diagnostic range** — update to SM0001-SM0054 + +### P1 — High Impact Improvements +3. **Add events to Products, Users, FileStorage, PageBuilder, Settings** — enables reactive cross-module patterns +4. **Integrate client-side form validation** (React Hook Form + Zod) — major UX gap +5. **Add integration tests for cross-module event flows** +6. **Expand FakeDataGenerators** to cover all modules + +### P2 — Important but Not Urgent +7. **Add missing UI components** (combobox, multi-select, file upload widget, date range picker) +8. **Add permissions to Settings, Users, Marketplace modules** +9. **Document string validation expectations** and add length limits +10. **Add health check aggregation endpoint** +11. **Implement SM0050** (localization resource validation) +12. **Add structured logging conventions** +13. **Add network error handling** to frontend +14. **Document database migration strategy** + +### P3 — Nice to Have +15. **Module dependency graph visualization tool** +16. **Event handler discovery convention** (standalone handler classes) +17. **Multi-browser E2E testing** +18. **OpenTelemetry integration** +19. **Error page templates** (404, 500, 403) +20. **Mutation testing setup** diff --git a/template/SimpleModule.Host/ClientApp/app.tsx b/template/SimpleModule.Host/ClientApp/app.tsx index 4da1e015..826a76de 100644 --- a/template/SimpleModule.Host/ClientApp/app.tsx +++ b/template/SimpleModule.Host/ClientApp/app.tsx @@ -97,8 +97,8 @@ router.on('finish', () => { } }); -// Handle non-Inertia error responses (404, 500, etc.) by showing a toast -// instead of the default "must receive a valid Inertia response" error. +// Non-Inertia error responses (404, 500, etc.) would otherwise trigger Inertia's +// default "must receive a valid Inertia response" full-page error. router.on('httpException', (event) => { event.preventDefault(); @@ -115,30 +115,99 @@ router.on('httpException', (event) => { parsed = body; } const message = parsed?.detail ?? parsed?.title ?? `Server error (${response.status})`; - showErrorToast(message); + showToast({ variant: 'error', title: 'Error', message, autoDismissMs: 8000 }); }); -function showErrorToast(message: string) { +router.on('exception', (event) => { + event.preventDefault(); + showToast({ + variant: 'error', + title: 'Error', + message: 'Network error. Please check your connection and try again.', + autoDismissMs: 8000, + }); +}); + +let offlineToast: HTMLDivElement | null = null; + +window.addEventListener('offline', () => { + if (offlineToast) return; + offlineToast = showToast({ + variant: 'warning', + title: 'Warning', + message: 'You are offline. Some features may be unavailable.', + onClose: () => { + offlineToast = null; + }, + }); +}); + +window.addEventListener('online', () => { + if (offlineToast) { + offlineToast.remove(); + offlineToast = null; + } + showToast({ + variant: 'success', + title: 'Success', + message: 'You are back online.', + autoDismissMs: 5000, + }); +}); + +type ToastVariant = 'error' | 'warning' | 'success'; + +interface ToastOptions { + variant: ToastVariant; + title: string; + message: string; + autoDismissMs?: number; + onClose?: () => void; +} + +const toastStyles: Record = { + error: { color: 'danger', role: 'alert' }, + warning: { color: 'warning', role: 'alert' }, + success: { color: 'success', role: 'status' }, +}; + +function showToast({ + variant, + title, + message, + autoDismissMs, + onClose, +}: ToastOptions): HTMLDivElement { + const { color, role } = toastStyles[variant]; const container = document.createElement('div'); - container.role = 'alert'; - container.className = - 'fixed bottom-4 right-4 z-[100] max-w-md rounded-xl border border-danger/20 bg-danger-bg p-4 text-danger-text shadow-lg animate-in slide-in-from-bottom-full'; + container.role = role; + container.className = `fixed bottom-4 right-4 z-[100] max-w-md rounded-xl border border-${color}/20 bg-${color}-bg p-4 text-${color}-text shadow-lg animate-in slide-in-from-bottom-full`; container.innerHTML = `
-

Error

+

-
`; - const msg = container.querySelector('p.opacity-90'); - if (msg) msg.textContent = message; - container.querySelector('button')?.addEventListener('click', () => container.remove()); + const titleEl = container.querySelector('p.font-semibold'); + if (titleEl) titleEl.textContent = title; + const msgEl = container.querySelector('p.opacity-90'); + if (msgEl) msgEl.textContent = message; + + const dismiss = () => { + container.remove(); + onClose?.(); + }; + container.querySelector('button')?.addEventListener('click', dismiss); document.body.appendChild(container); - setTimeout(() => container.remove(), 8000); + if (autoDismissMs) { + setTimeout(dismiss, autoDismissMs); + } + return container; } createInertiaApp({ diff --git a/tests/SimpleModule.Core.Tests/Events/EventBusIntegrationTests.cs b/tests/SimpleModule.Core.Tests/Events/EventBusIntegrationTests.cs new file mode 100644 index 00000000..0eb97c6f --- /dev/null +++ b/tests/SimpleModule.Core.Tests/Events/EventBusIntegrationTests.cs @@ -0,0 +1,244 @@ +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using SimpleModule.Core.Events; + +namespace SimpleModule.Core.Tests.Events; + +/// +/// Integration tests verifying cross-module event flows work correctly using the +/// full DI container and event bus pipeline (handlers, pipeline behaviors, background +/// dispatch). +/// +public sealed class EventBusIntegrationTests +{ + private sealed record TestEvent(string Value) : IEvent; + + // Distinct marker types let DI register three independent handler instances + // without copy-pasting three identical handler classes. + private interface IHandlerSlot + { + List ReceivedEvents { get; } + } + + private sealed class SlotOne : IHandlerSlot + { + public List ReceivedEvents { get; } = []; + } + + private sealed class SlotTwo : IHandlerSlot + { + public List ReceivedEvents { get; } = []; + } + + private sealed class SlotThree : IHandlerSlot + { + public List ReceivedEvents { get; } = []; + } + + private sealed class TrackingHandler(TSlot slot) : IEventHandler + where TSlot : IHandlerSlot + { + public Task HandleAsync(TestEvent @event, CancellationToken cancellationToken) + { + slot.ReceivedEvents.Add(@event); + return Task.CompletedTask; + } + } + + private sealed class ThrowingHandler : IEventHandler + { + public bool WasCalled { get; private set; } + + public Task HandleAsync(TestEvent @event, CancellationToken cancellationToken) + { + WasCalled = true; + throw new InvalidOperationException("Handler intentionally failed"); + } + } + + private sealed class SignallingHandler(TaskCompletionSource tcs) + : IEventHandler + { + public Task HandleAsync(TestEvent @event, CancellationToken cancellationToken) + { + tcs.TrySetResult(@event); + return Task.CompletedTask; + } + } + + private sealed class TrackingPipelineBehavior : IEventPipelineBehavior + { + public bool BeforeHandlerCalled { get; private set; } + public bool AfterHandlerCalled { get; private set; } + public TestEvent? ReceivedEvent { get; private set; } + + public async Task HandleAsync( + TestEvent @event, + Func next, + CancellationToken cancellationToken + ) + { + ReceivedEvent = @event; + BeforeHandlerCalled = true; + await next(); + AfterHandlerCalled = true; + } + } + + private static ServiceProvider BuildProvider(Action configure) + { + var services = new ServiceCollection(); + + services.AddSingleton(_ => new BackgroundEventChannel( + NullLogger.Instance + )); + services.AddSingleton(sp => new EventBus( + sp, + NullLogger.Instance, + sp.GetRequiredService() + )); + services.AddSingleton(sp => new BackgroundEventDispatcher( + sp.GetRequiredService(), + sp.GetRequiredService(), + NullLogger.Instance + )); + + configure(services); + + return services.BuildServiceProvider(); + } + + [Fact] + public async Task Event_PublishAsync_InvokesRegisteredHandler() + { + var slot = new SlotOne(); + await using var provider = BuildProvider(services => + { + services.AddSingleton(slot); + services.AddSingleton, TrackingHandler>(); + }); + var bus = provider.GetRequiredService(); + + await bus.PublishAsync(new TestEvent("integration-test")); + + slot.ReceivedEvents.Should().ContainSingle().Which.Value.Should().Be("integration-test"); + } + + [Fact] + public async Task Event_PublishAsync_MultipleHandlers_AllInvoked() + { + var slot1 = new SlotOne(); + var slot2 = new SlotTwo(); + var slot3 = new SlotThree(); + await using var provider = BuildProvider(services => + { + services.AddSingleton(slot1); + services.AddSingleton(slot2); + services.AddSingleton(slot3); + services.AddSingleton, TrackingHandler>(); + services.AddSingleton, TrackingHandler>(); + services.AddSingleton, TrackingHandler>(); + }); + var bus = provider.GetRequiredService(); + + await bus.PublishAsync(new TestEvent("multi-handler")); + + slot1.ReceivedEvents.Should().ContainSingle().Which.Value.Should().Be("multi-handler"); + slot2.ReceivedEvents.Should().ContainSingle().Which.Value.Should().Be("multi-handler"); + slot3.ReceivedEvents.Should().ContainSingle().Which.Value.Should().Be("multi-handler"); + } + + [Fact] + public async Task Event_PublishAsync_HandlerThrows_OtherHandlersStillRun() + { + var slotBefore = new SlotOne(); + var slotAfter = new SlotTwo(); + var throwingHandler = new ThrowingHandler(); + + await using var provider = BuildProvider(services => + { + services.AddSingleton(slotBefore); + services.AddSingleton(slotAfter); + services.AddSingleton, TrackingHandler>(); + services.AddSingleton>(throwingHandler); + services.AddSingleton, TrackingHandler>(); + }); + var bus = provider.GetRequiredService(); + + var act = () => bus.PublishAsync(new TestEvent("partial-failure")); + + var ex = await act.Should().ThrowAsync(); + ex.Which.InnerExceptions.Should() + .ContainSingle() + .Which.Should() + .BeOfType() + .Which.Message.Should() + .Be("Handler intentionally failed"); + + slotBefore + .ReceivedEvents.Should() + .ContainSingle() + .Which.Value.Should() + .Be("partial-failure"); + throwingHandler.WasCalled.Should().BeTrue(); + slotAfter + .ReceivedEvents.Should() + .ContainSingle() + .Which.Value.Should() + .Be("partial-failure"); + } + + [Fact] + public async Task Event_PublishInBackground_EventuallyInvokesHandler() + { + var tcs = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously + ); + await using var provider = BuildProvider(services => + { + services.AddSingleton(tcs); + services.AddSingleton, SignallingHandler>(); + }); + var bus = provider.GetRequiredService(); + var dispatcher = provider.GetRequiredService(); + using var cts = new CancellationTokenSource(); + await dispatcher.StartAsync(cts.Token); + + try + { + bus.PublishInBackground(new TestEvent("background-event")); + + var received = await tcs.Task.WaitAsync(TimeSpan.FromSeconds(5)); + received.Value.Should().Be("background-event"); + } + finally + { + await cts.CancelAsync(); + await dispatcher.StopAsync(CancellationToken.None); + } + } + + [Fact] + public async Task Event_PipelineBehavior_WrapsHandlerExecution() + { + var behavior = new TrackingPipelineBehavior(); + var slot = new SlotOne(); + + await using var provider = BuildProvider(services => + { + services.AddSingleton>(behavior); + services.AddSingleton(slot); + services.AddSingleton, TrackingHandler>(); + }); + var bus = provider.GetRequiredService(); + + await bus.PublishAsync(new TestEvent("pipeline-test")); + + behavior.BeforeHandlerCalled.Should().BeTrue(); + behavior.AfterHandlerCalled.Should().BeTrue(); + behavior.ReceivedEvent.Should().NotBeNull(); + behavior.ReceivedEvent!.Value.Should().Be("pipeline-test"); + slot.ReceivedEvents.Should().ContainSingle().Which.Value.Should().Be("pipeline-test"); + } +} diff --git a/tests/SimpleModule.Core.Tests/Infrastructure/WebApplicationFactoryTests.cs b/tests/SimpleModule.Core.Tests/Infrastructure/WebApplicationFactoryTests.cs index 458beb49..3d555db9 100644 --- a/tests/SimpleModule.Core.Tests/Infrastructure/WebApplicationFactoryTests.cs +++ b/tests/SimpleModule.Core.Tests/Infrastructure/WebApplicationFactoryTests.cs @@ -6,6 +6,7 @@ using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.Extensions.DependencyInjection; using SimpleModule.Core; +using SimpleModule.Core.Events; using SimpleModule.Host; using SimpleModule.Tests.Shared.Fixtures; @@ -152,6 +153,90 @@ public void AuditLogContracts_CanBeResolved() svc.Should().NotBeNull(); } + // Defends against runtime circular dependencies that SM0010 can't catch: + // the generator only sees module-level project references and cannot analyze + // factory lambdas like AddScoped(sp => new AuditingEventBus(..., sp.GetService())). + // .NET DI can't detect re-entry through factory lambdas, so such cycles hang + // the whole test run instead of throwing. These timeout tests fail fast instead. + + private static readonly TimeSpan ResolutionTimeout = TimeSpan.FromSeconds(5); + + [Theory] + [MemberData(nameof(AllContractTypes))] + public Task ContractInterface_CanBeResolved_WithoutHanging(Type contractType) => + AssertResolvesWithinTimeout(contractType); + + [Fact] + public Task EventBus_CanBeResolved_WithoutHanging() => + AssertResolvesWithinTimeout(typeof(IEventBus)); + + [Fact] + public async Task EventBus_CanPublishEvent_WithoutHanging() + { + var rootProvider = _factory.Services; + + var publishTask = Task.Run(async () => + { + using var scope = rootProvider.CreateScope(); + var bus = scope.ServiceProvider.GetRequiredService(); + await bus.PublishAsync(new NoopEvent(), CancellationToken.None); + }); + + try + { + await publishTask.WaitAsync(ResolutionTimeout); + } + catch (TimeoutException) + { + throw new InvalidOperationException( + $"IEventBus.PublishAsync hung for over {ResolutionTimeout.TotalSeconds}s. " + + "A handler, decorator, or service in the resolution chain likely has a circular dependency." + ); + } + } + + private async Task AssertResolvesWithinTimeout(Type serviceType) + { + // Force the factory to build before timing so fixture warmup isn't charged + // against the per-service budget. + var rootProvider = _factory.Services; + + var resolveTask = Task.Run(() => + { + using var scope = rootProvider.CreateScope(); + var svc = scope.ServiceProvider.GetService(serviceType); + svc.Should().NotBeNull($"{serviceType.FullName} should be registered"); + }); + + try + { + await resolveTask.WaitAsync(ResolutionTimeout); + } + catch (TimeoutException) + { + throw new InvalidOperationException( + $"Resolving '{serviceType.FullName}' hung for over {ResolutionTimeout.TotalSeconds}s. " + + "Likely a circular dependency introduced through a decorator factory " + + "(see IEventBus → AuditingEventBus → ISettingsContracts pattern)." + ); + } + } + + public static TheoryData AllContractTypes + { + get + { + var data = new TheoryData(); + foreach (var type in ModuleContractRegistry.All) + { + data.Add(type); + } + return data; + } + } + + private sealed record NoopEvent : IEvent; + // ── Authenticated client ──────────────────────────────────────── [Fact] diff --git a/tests/SimpleModule.Tests.Shared/Fakes/TestEventBus.cs b/tests/SimpleModule.Tests.Shared/Fakes/TestEventBus.cs new file mode 100644 index 00000000..fa6fc830 --- /dev/null +++ b/tests/SimpleModule.Tests.Shared/Fakes/TestEventBus.cs @@ -0,0 +1,23 @@ +using SimpleModule.Core.Events; + +namespace SimpleModule.Tests.Shared.Fakes; + +/// +/// Recording stub for unit tests. Captures every published +/// event in so tests can assert on publishing behaviour +/// without wiring up the full event pipeline. +/// +public sealed class TestEventBus : IEventBus +{ + public List PublishedEvents { get; } = []; + + public Task PublishAsync(T @event, CancellationToken cancellationToken = default) + where T : IEvent + { + PublishedEvents.Add(@event); + return Task.CompletedTask; + } + + public void PublishInBackground(T @event) + where T : IEvent => PublishedEvents.Add(@event); +}