Add event publishing to core modules and improve framework testing#105
Merged
antosubash merged 5 commits intomainfrom Apr 11, 2026
Merged
Add event publishing to core modules and improve framework testing#105antosubash merged 5 commits intomainfrom
antosubash merged 5 commits intomainfrom
Conversation
Review of all 23 modules, core architecture, source generator diagnostics, frontend, testing infrastructure, and security patterns. Identifies 28 gaps across 7 categories with priority-ranked recommendations.
P1 — High Impact: - Add domain events to Products, Users, FileStorage, PageBuilder, Settings modules (ProductCreated/Updated/Deleted, UserCreated/ Updated/Deleted/RolesChanged, FileUploaded/Deleted, PageCreated/ Published/Unpublished/Deleted, SettingChanged/Deleted) - Wire up IEventBus publishing in each service's create/update/ delete flows, using PublishAsync for creates/updates and PublishInBackground for deletes - Add EventBusIntegrationTests covering cross-module flows (handlers, pipeline behaviors, partial failures, background dispatch) — 5 new tests - Fix circular dependency: SettingsService resolves IEventBus lazily via IServiceProvider, breaking the cycle created by AuditingEventBus decorator which itself depends on ISettingsContracts P2 — Important: - Add permissions classes and ConfigurePermissions wiring for Settings, Users, and Marketplace modules - Extend ValidationBuilder with Required, MinLength, MaxLength, LengthBetween, MatchesPattern, Email, GreaterThan, Between helpers - Add ModuleHealthCheck that aggregates per-module CheckHealthAsync results into a single aggregate health check registered alongside DatabaseHealthCheck - Add network error handling to frontend app.tsx: online/offline detection via window events, persistent offline toast, network exception handler on router - Document structured logging conventions and database migration strategy in Constitution - Fix CLAUDE.md diagnostic range (SM0001-SM0054) - Restructure Constitution diagnostic sections to match actual source generator implementation (Feature Flags, Endpoints, Module Metadata)
The circular dependency that hung every integration test (SettingsService → IEventBus → AuditingEventBus → ISettingsContracts) was invisible to the source generator's SM0010 check because: 1. SM0010 only sees module-level dependencies derived from project references, not runtime DI graphs. 2. AuditingEventBus is registered via a factory lambda (AddScoped<IEventBus>(sp => new AuditingEventBus(...))) — static analysis cannot reliably look inside lambda bodies to see the ISettingsContracts dependency. 3. .NET's built-in DI cannot track re-entry through factory lambdas, so the cycle manifests as infinite recursion / hang rather than a clean InvalidOperationException. Add runtime defenses: - New ContractRegistryEmitter generates ModuleContractRegistry.All listing every discovered contract interface with exactly one registered implementation (mirrors the existing ModuleDbContextRegistry pattern). - New theory test ContractInterface_CanBeResolved_WithoutHanging iterates ModuleContractRegistry.All and resolves each contract in a fresh scope with a 5-second timeout. - New fact EventBus_CanBeResolved_WithoutHanging resolves IEventBus directly (the decorator chain's focal point). - New fact EventBus_CanPublishEvent_WithoutHanging publishes a test event end-to-end to catch cycles that only manifest during handler dispatch. All tests run the resolution on a thread-pool thread with Task.WhenAny against a Task.Delay, because GetService is synchronous and cannot be cancelled. A hang fails fast with a clear message pointing at the IEventBus → AuditingEventBus → ISettingsContracts pattern. Verified the tests fail correctly when the bug is reintroduced: 14/28 contract resolutions hang, and both IEventBus tests fail within 11 seconds with the diagnostic message.
Code reuse:
- Lift `TestEventBus` into `SimpleModule.Tests.Shared/Fakes/` so the
5 module test projects stop duplicating an identical nested class.
The canonical fake now records `PublishedEvents` so tests can
assert on publishing behaviour.
- Wire up the existing but unreferenced `WriteHealthCheckResponse`
on the `/health/ready` endpoint.
Code quality:
- `ModuleHealthCheck`: replace fragile `.Replace("Module", "")`
name extraction with `ModuleAttribute.Name`, cache the
`(module, name)` map once in the constructor, and drop the
dead `IModuleLifecycle` branch (the interface does not hide
`IModule.CheckHealthAsync`).
- `EventBusIntegrationTests`: collapse three byte-identical
`TrackingHandler` classes into one generic
`TrackingHandler<TSlot>` with empty marker types, drop the
unused `AnotherEvent` and `WasCalled` derived property, and
replace the polling loop in the background-dispatch test with
a deterministic `TaskCompletionSource` + `WaitAsync`.
- `WebApplicationFactoryTests`: delete the 14-line narration
block; leave a single short "why" comment about SM0010 not
catching factory-lambda cycles. Replace the manual
`Task.WhenAny(task, Task.Delay(...))` timeout pattern with
`Task.WaitAsync(timeout)` + `TimeoutException` catch.
- `app.tsx`: collapse three near-identical `showErrorToast`/
`showPersistentWarningToast`/`showSuccessToast` functions
into a single `showToast({ variant, title, message, ... })`
helper driven by a `toastStyles` map.
- `SettingsService`: replace the `IServiceProvider` service-
locator anti-pattern with `Lazy<IEventBus>` to break the
`SettingsService ↔ AuditingEventBus ↔ ISettingsContracts`
cycle cleanly. A new framework-level registration provides
`Lazy<IEventBus>` out of the box.
- `HealthCheckConstants.ModulesCheckName` constant replaces the
raw `"modules"` string literal.
Efficiency:
- `ModuleHealthCheck`: dispatch to all modules in parallel via
`Task.WhenAll` instead of sequential `await` in a foreach.
Readiness probes called by load balancers no longer pay the
sum of all module latencies.
- `UserAdminService.SetUserRolesAsync`: drop the redundant
`GetRolesAsync` round-trip after the diff — the new role set
is already in memory.
- `ValidationBuilder.MatchesPattern`: pass `RegexOptions.Compiled`
so the static regex cache stores compiled patterns.
Verified: `dotnet build` clean (0/0), `dotnet test` passes
all 1208 tests across 24 projects, `biome check` clean.
…rk-gaps-py59A # Conflicts: # modules/FileStorage/tests/SimpleModule.FileStorage.Tests/FileStorageServiceTests.cs # modules/Settings/src/SimpleModule.Settings/SettingsService.cs
Deploying simplemodule-website with
|
| Latest commit: |
fd66bcd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4f7e1d2b.simplemodule-website.pages.dev |
| Branch Preview URL: | https://claude-review-framework-gaps.simplemodule-website.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses critical gaps in cross-module communication and testing infrastructure identified in the framework gap analysis. It adds event publishing to five core modules (Users, Products, FileStorage, PageBuilder, Settings), introduces comprehensive event bus integration tests, enhances validation capabilities, and improves runtime circular dependency detection.
Key Changes
Event Publishing & Cross-Module Communication
UserCreatedEvent,UserUpdatedEvent,UserDeletedEvent,UserRolesChangedEventwith publishing inUserServiceandUserAdminServiceProductCreatedEvent,ProductUpdatedEvent,ProductDeletedEventwith publishing inProductServiceFileUploadedEvent,FileDeletedEventwith publishing inFileStorageServicePageCreatedEvent,PagePublishedEvent,PageUnpublishedEvent,PageDeletedEventwith publishing inPageBuilderServiceSettingChangedEvent,SettingDeletedEventwith publishing inSettingsServiceTesting Infrastructure
BackgroundEventDispatcherAuthorization & Permissions
IModulePermissionsimplementationsValidation Enhancements
Required(),MaxLength(),MinLength(),LengthBetween()MatchesPattern()with compiled regex supportEmail()with generated regex patternGreaterThan(),Between()for numeric validationFramework Infrastructure
IModule.CheckHealthAsync()ModuleContractRegistryfor test enumeration of all contract interfacesModulesCheckNameconstantFrontend Improvements
showErrorToast()toshowToast()supporting error, warning, and success variants with configurable auto-dismiss and close callbacksDocumentation
Notable Implementation Details
IEventHandler<T>interface, enabling automatic handler discoveryAggregateExceptionto isolate handler failures while allowing other handlers to executeRegex.IsMatch()static method which benefits from .NET's internal regex cache for common patternshttps://claude.ai/code/session_01PRezWVEFAC7Gzx88WQJ4qA