From db230ae50d55b25170a50126035856be9280c293 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Thu, 21 May 2026 12:31:32 +0300 Subject: [PATCH 1/8] =?UTF-8?q?feat(shared-kernel):=20Phase=2002a=20packet?= =?UTF-8?q?=202=20=E2=80=94=20shared=20kernel=20core?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Land the SharedKernel primitives every later module depends on: deterministic-test clocks/random/guid factories (Standards 02 § Time), `LocalizedMessage` carrying the `lockey_` prefix invariant for `Result.Fail` payloads, refactored `Error` (wraps `LocalizedMessage`, projects `Code` over `Message.Key`), `Entity` + `AuditableEntity` aggregate bases with the audit-column / soft-delete / optimistic-concurrency contract, in-process `IDomainEvent : INotification`, cursor-first pagination matching Standards 04. Vogen 7.0.0 wired per ADR-0023 with `LearnStackVogenDefaults.IdMask`; `IAggregateRoot` constrains every aggregate to a strongly-typed ID. 54 unit tests + a Vogen emitter smoke test green. ADR: ADR-0023 Module: SharedKernel Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/Directory.Packages.props | 6 ++ .../Domain/AuditableEntity.cs | 94 +++++++++++++++++++ .../Domain/DomainEvent.cs | 23 +++++ .../LearnStack.SharedKernel/Domain/Entity.cs | 67 +++++++++++++ .../Domain/IDomainEvent.cs | 23 +++++ .../Domain/IHasDomainEvents.cs | 13 +++ .../Identifiers/FixedGuidFactory.cs | 34 +++++++ .../Identifiers/IAggregateRoot.cs | 28 ++++++ .../Identifiers/IGuidFactory.cs | 28 ++++++ .../Identifiers/LearnStackVogenDefaults.cs | 25 +++++ .../Identifiers/SystemGuidFactory.cs | 12 +++ .../LearnStack.SharedKernel.csproj | 24 +++++ .../Localization/LocalizedMessage.cs | 61 ++++++++++++ .../Pagination/CursorPagination.cs | 33 +++++++ .../Pagination/Page.cs | 28 ++++++ .../Pagination/PageInfo.cs | 12 +++ .../Persistence/IOptimisticConcurrency.cs | 18 ++++ .../Persistence/ISoftDelete.cs | 20 ++++ .../Random/FixedRandom.cs | 24 +++++ .../LearnStack.SharedKernel/Random/IRandom.cs | 42 +++++++++ .../Random/SystemRandom.cs | 17 ++++ .../LearnStack.SharedKernel/Results/Error.cs | 18 +++- .../LearnStack.SharedKernel/Results/Result.cs | 59 +++++++++++- .../Time/FixedClock.cs | 21 +++++ .../LearnStack.SharedKernel/Time/IClock.cs | 16 ++++ .../Time/SystemClock.cs | 10 ++ .../LearnStack.Tests.Unit.csproj | 8 ++ .../Domain/AuditableEntityTests.cs | 64 +++++++++++++ .../SharedKernel/Domain/DomainEventTests.cs | 44 +++++++++ .../SharedKernel/Domain/EntityTests.cs | 61 ++++++++++++ .../SharedKernel/Domain/TestAggregate.cs | 29 ++++++ .../SharedKernel/Domain/TestId.cs | 16 ++++ .../SharedKernel/ErrorTests.cs | 40 ++++++++ .../Identifiers/FixedGuidFactoryTests.cs | 41 ++++++++ .../Identifiers/SystemGuidFactoryTests.cs | 39 ++++++++ .../Identifiers/VogenIdEmissionTests.cs | 48 ++++++++++ .../Localization/LocalizedMessageTests.cs | 62 ++++++++++++ .../Pagination/CursorPaginationTests.cs | 44 +++++++++ .../SharedKernel/Pagination/PageTests.cs | 29 ++++++ .../SharedKernel/Random/FixedRandomTests.cs | 41 ++++++++ .../SharedKernel/ResultTests.cs | 43 ++++++++- .../SharedKernel/Time/FixedClockTests.cs | 40 ++++++++ .../SharedKernel/Time/SystemClockTests.cs | 21 +++++ docs/glossary.md | 10 +- docs/roadmap/phase-02a-kernel-tenancy.md | 34 +++++-- docs/standards/02-backend-coding.md | 18 +++- docs/standards/09-error-handling.md | 36 +++++-- 47 files changed, 1497 insertions(+), 27 deletions(-) create mode 100644 backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs create mode 100644 backend/src/LearnStack.SharedKernel/Domain/DomainEvent.cs create mode 100644 backend/src/LearnStack.SharedKernel/Domain/Entity.cs create mode 100644 backend/src/LearnStack.SharedKernel/Domain/IDomainEvent.cs create mode 100644 backend/src/LearnStack.SharedKernel/Domain/IHasDomainEvents.cs create mode 100644 backend/src/LearnStack.SharedKernel/Identifiers/FixedGuidFactory.cs create mode 100644 backend/src/LearnStack.SharedKernel/Identifiers/IAggregateRoot.cs create mode 100644 backend/src/LearnStack.SharedKernel/Identifiers/IGuidFactory.cs create mode 100644 backend/src/LearnStack.SharedKernel/Identifiers/LearnStackVogenDefaults.cs create mode 100644 backend/src/LearnStack.SharedKernel/Identifiers/SystemGuidFactory.cs create mode 100644 backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs create mode 100644 backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs create mode 100644 backend/src/LearnStack.SharedKernel/Pagination/Page.cs create mode 100644 backend/src/LearnStack.SharedKernel/Pagination/PageInfo.cs create mode 100644 backend/src/LearnStack.SharedKernel/Persistence/IOptimisticConcurrency.cs create mode 100644 backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs create mode 100644 backend/src/LearnStack.SharedKernel/Random/FixedRandom.cs create mode 100644 backend/src/LearnStack.SharedKernel/Random/IRandom.cs create mode 100644 backend/src/LearnStack.SharedKernel/Random/SystemRandom.cs create mode 100644 backend/src/LearnStack.SharedKernel/Time/FixedClock.cs create mode 100644 backend/src/LearnStack.SharedKernel/Time/IClock.cs create mode 100644 backend/src/LearnStack.SharedKernel/Time/SystemClock.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/DomainEventTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/EntityTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestAggregate.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestId.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/FixedGuidFactoryTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/SystemGuidFactoryTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/VogenIdEmissionTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Localization/LocalizedMessageTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/PageTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Random/FixedRandomTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/FixedClockTests.cs create mode 100644 backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/SystemClockTests.cs diff --git a/backend/Directory.Packages.props b/backend/Directory.Packages.props index b4ef912..3b950e5 100644 --- a/backend/Directory.Packages.props +++ b/backend/Directory.Packages.props @@ -19,6 +19,12 @@ + + + diff --git a/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs b/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs new file mode 100644 index 0000000..b279d62 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs @@ -0,0 +1,94 @@ +using LearnStack.SharedKernel.Identifiers; +using LearnStack.SharedKernel.Persistence; +using LearnStack.SharedKernel.Time; + +namespace LearnStack.SharedKernel.Domain; + +/// +/// Mutable aggregate base. Carries the audit columns every tenant-owned +/// table mirrors (CreatedAt / CreatedBy / UpdatedAt / +/// UpdatedBy / DeletedAt / DeletedBy / Version) +/// and implements + +/// so the EF global-query-filter and concurrency-token wiring picks them up +/// uniformly. Audit columns are populated by / +/// / , which command +/// handlers call via the they already inject. +/// +/// +/// The actor columns (CreatedBy / UpdatedBy / DeletedBy) +/// store raw rather than a strongly-typed UserId: +/// cannot depend on the Identity +/// module (which lands in Phase 02b), and audit metadata is not an +/// aggregate-root reference in the Standards 02 § Strongly-Typed +/// Identifiers sense. +/// +public abstract class AuditableEntity + : Entity, ISoftDelete, IOptimisticConcurrency + where TId : struct, IStronglyTypedId +{ + protected AuditableEntity(TId id) + : base(id) + { + } + + // EF Core / ORM materialization ctor. + protected AuditableEntity() + { + } + + public DateTimeOffset CreatedAt { get; protected set; } + + public Guid CreatedBy { get; protected set; } + + public DateTimeOffset? UpdatedAt { get; protected set; } + + public Guid? UpdatedBy { get; protected set; } + + public DateTimeOffset? DeletedAt { get; protected set; } + + public Guid? DeletedBy { get; protected set; } + + public uint Version { get; protected set; } + + /// + /// Convenience projection of . Surfaced at this + /// level (rather than relying on 's default + /// interface implementation) so callers can read the property through + /// the concrete aggregate type without casting. + /// + public bool IsDeleted => DeletedAt.HasValue; + + /// + /// Stamps / on first + /// persist. Idempotent: callers may invoke it once during aggregate + /// construction; later calls overwrite the values, which is the + /// caller's bug rather than a silent no-op. + /// + public void MarkCreated(DateTimeOffset at, Guid by) + { + CreatedAt = at; + CreatedBy = by; + } + + /// + /// Stamps / . Called by + /// aggregate methods that mutate state; the audit pipeline reads these + /// values when writing the audit entry. + /// + public void MarkUpdated(DateTimeOffset at, Guid by) + { + UpdatedAt = at; + UpdatedBy = by; + } + + /// + /// Marks the entity as soft-deleted. EF's global query filter excludes + /// soft-deleted rows by default; the platform-admin scope can opt back + /// in explicitly. + /// + public void SoftDelete(DateTimeOffset at, Guid by) + { + DeletedAt = at; + DeletedBy = by; + } +} diff --git a/backend/src/LearnStack.SharedKernel/Domain/DomainEvent.cs b/backend/src/LearnStack.SharedKernel/Domain/DomainEvent.cs new file mode 100644 index 0000000..b9d64ca --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Domain/DomainEvent.cs @@ -0,0 +1,23 @@ +namespace LearnStack.SharedKernel.Domain; + +/// +/// Base record for in-process domain events. Concrete events derive from +/// this and add their own payload fields: +/// +/// public sealed record CourseEnrolled(CourseId CourseId, UserId UserId) : DomainEvent; +/// +/// +/// +/// The and defaults call +/// the BCL directly rather than going through IClock / +/// IGuidFactory: domain events are minted inside aggregate methods +/// that already received the relevant abstractions through their command +/// boundary. Overriding the defaults via record init in a test +/// remains straightforward. +/// +public abstract record DomainEvent : IDomainEvent +{ + public Guid EventId { get; init; } = Guid.CreateVersion7(); + + public DateTimeOffset OccurredAt { get; init; } = DateTimeOffset.UtcNow; +} diff --git a/backend/src/LearnStack.SharedKernel/Domain/Entity.cs b/backend/src/LearnStack.SharedKernel/Domain/Entity.cs new file mode 100644 index 0000000..5f32b15 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Domain/Entity.cs @@ -0,0 +1,67 @@ +using LearnStack.SharedKernel.Identifiers; + +namespace LearnStack.SharedKernel.Domain; + +/// +/// Append-only / audit aggregate base. Carries identity and raises +/// in-process s; does not carry the +/// CreatedAt / UpdatedAt audit columns — those belong to +/// mutable aggregates and live on . +/// +/// +/// +/// The audit subsystem's own AuditEntry aggregate inherits this base +/// directly, never , because audit rows +/// are immutable once written. Architecture test +/// AuditEntry_Inherits_Entity_Not_AuditableEntity guards that rule. +/// +/// +/// Equality is identity-based: two entities are equal iff their +/// s match. Reference equality is also exposed so EF Core's +/// change tracker behaves predictably. +/// +/// +public abstract class Entity : IHasId, IHasDomainEvents + where TId : struct, IStronglyTypedId +{ + private readonly List _domainEvents = []; + + protected Entity(TId id) + { + Id = id; + } + + // EF Core / ORM materialization ctor. + protected Entity() + { + } + + public TId Id { get; protected init; } + + public IReadOnlyCollection DomainEvents => _domainEvents.AsReadOnly(); + + protected void RaiseDomainEvent(IDomainEvent domainEvent) + { + ArgumentNullException.ThrowIfNull(domainEvent); + _domainEvents.Add(domainEvent); + } + + public void ClearDomainEvents() => _domainEvents.Clear(); + + public override bool Equals(object? obj) + { + if (obj is not Entity other) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + return Id.Equals(other.Id); + } + + public override int GetHashCode() => Id.GetHashCode(); +} diff --git a/backend/src/LearnStack.SharedKernel/Domain/IDomainEvent.cs b/backend/src/LearnStack.SharedKernel/Domain/IDomainEvent.cs new file mode 100644 index 0000000..f4078ca --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Domain/IDomainEvent.cs @@ -0,0 +1,23 @@ +using MediatR; + +namespace LearnStack.SharedKernel.Domain; + +/// +/// In-process domain event raised by an aggregate method. Dispatched +/// in-process by MediatR — the cross-module integration-event path +/// (outbox + Dapr pub/sub) is a different mechanism per +/// ADR-0010. +/// +public interface IDomainEvent : INotification +{ + /// + /// Unique event identifier. UUIDv7 so insertion-order matches occurrence + /// order when an event is persisted for replay or debugging. + /// + Guid EventId { get; } + + /// + /// UTC instant the event was raised. + /// + DateTimeOffset OccurredAt { get; } +} diff --git a/backend/src/LearnStack.SharedKernel/Domain/IHasDomainEvents.cs b/backend/src/LearnStack.SharedKernel/Domain/IHasDomainEvents.cs new file mode 100644 index 0000000..f443391 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Domain/IHasDomainEvents.cs @@ -0,0 +1,13 @@ +namespace LearnStack.SharedKernel.Domain; + +/// +/// Marker every entity that raises implements. +/// The unit-of-work walks tracked entities, drains the events, and hands +/// them to MediatR's in-process publisher on commit. +/// +public interface IHasDomainEvents +{ + IReadOnlyCollection DomainEvents { get; } + + void ClearDomainEvents(); +} diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/FixedGuidFactory.cs b/backend/src/LearnStack.SharedKernel/Identifiers/FixedGuidFactory.cs new file mode 100644 index 0000000..335e789 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Identifiers/FixedGuidFactory.cs @@ -0,0 +1,34 @@ +namespace LearnStack.SharedKernel.Identifiers; + +/// +/// Deterministic for tests. Returns the supplied +/// sequence in order; throws once +/// the sequence is exhausted so tests fail loud rather than silently +/// reusing a default value. +/// +public sealed class FixedGuidFactory : IGuidFactory +{ + private readonly Queue _sequence; + + public FixedGuidFactory(params Guid[] sequence) + { + ArgumentNullException.ThrowIfNull(sequence); + _sequence = new Queue(sequence); + } + + public Guid NewUuidV7() => Dequeue(); + + public Guid NewUuidV4() => Dequeue(); + + private Guid Dequeue() + { + if (_sequence.Count == 0) + { + throw new InvalidOperationException( + "FixedGuidFactory sequence exhausted. Construct with enough GUIDs " + + "for the test, or switch to a different fixture."); + } + + return _sequence.Dequeue(); + } +} diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/IAggregateRoot.cs b/backend/src/LearnStack.SharedKernel/Identifiers/IAggregateRoot.cs new file mode 100644 index 0000000..0a9c341 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Identifiers/IAggregateRoot.cs @@ -0,0 +1,28 @@ +namespace LearnStack.SharedKernel.Identifiers; + +/// +/// Marker for the root entity of an aggregate. Repositories accept and +/// return only aggregate roots; entities inside an aggregate are reached +/// through the root. +/// +/// +/// The aggregate's strongly-typed identifier — per ADR-0023 every aggregate +/// root carries an -shaped Vogen-emitted +/// ID over . +/// +public interface IAggregateRoot : IHasId + where TId : struct, IStronglyTypedId +{ +} + +/// +/// Tagging interface for any entity that exposes an identifier of type +/// . Kept separate from +/// so child entities inside an aggregate +/// can share the Id shape without claiming aggregate-root status. +/// +public interface IHasId + where TId : struct, IStronglyTypedId +{ + TId Id { get; } +} diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/IGuidFactory.cs b/backend/src/LearnStack.SharedKernel/Identifiers/IGuidFactory.cs new file mode 100644 index 0000000..5bf1d3e --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Identifiers/IGuidFactory.cs @@ -0,0 +1,28 @@ +namespace LearnStack.SharedKernel.Identifiers; + +/// +/// GUID minting abstraction. Application-side GUIDs flow through this +/// interface so tests can pin the sequence deterministically. +/// +/// +/// Two minting paths exist per ADR-0031 (PostgreSQL 18) + ADR-0023: +/// app-side for aggregates that need the ID before +/// SaveChangesAsync, and DB-side gen_uuid_v7() DEFAULT for +/// high-volume append-only tables (audit_log, outbox_messages). +/// This factory covers the app-side path only. +/// +public interface IGuidFactory +{ + /// + /// Mints a UUIDv7 (.NET 9+ Guid.CreateVersion7). Preferred for + /// every new aggregate root identifier because the timestamp prefix + /// keeps DB-side indexes sorted by insertion order. + /// + Guid NewUuidV7(); + + /// + /// Mints a UUIDv4. Reserved for non-aggregate identifiers that should + /// not leak insertion-order information (e.g. external-facing tokens). + /// + Guid NewUuidV4(); +} diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/LearnStackVogenDefaults.cs b/backend/src/LearnStack.SharedKernel/Identifiers/LearnStackVogenDefaults.cs new file mode 100644 index 0000000..c733d61 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Identifiers/LearnStackVogenDefaults.cs @@ -0,0 +1,25 @@ +using Vogen; + +namespace LearnStack.SharedKernel.Identifiers; + +/// +/// Canonical Conversions mask for every LearnStack-declared +/// [ValueObject<T>] per ADR-0023. Using the constant keeps the +/// annotation uniform across modules: callers write +/// [ValueObject<Guid>(LearnStackVogenDefaults.IdMask)] rather +/// than hand-picking the flag set. +/// +public static class LearnStackVogenDefaults +{ + /// + /// Conversion set every aggregate-root ID and cross-cutting value object + /// opts into: EF Core value converter, System.Text.Json converter, and + /// the TypeConverter (which carries ASP.NET Core minimal-API and MVC + /// route-parameter binding — Vogen does not expose a separate + /// AspNetCoreRouteParameter flag). + /// + public const Conversions IdMask = + Conversions.EfCoreValueConverter + | Conversions.SystemTextJson + | Conversions.TypeConverter; +} diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/SystemGuidFactory.cs b/backend/src/LearnStack.SharedKernel/Identifiers/SystemGuidFactory.cs new file mode 100644 index 0000000..1aba624 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Identifiers/SystemGuidFactory.cs @@ -0,0 +1,12 @@ +namespace LearnStack.SharedKernel.Identifiers; + +/// +/// Production backed by the BCL. +/// Registered as a singleton at the composition root. +/// +public sealed class SystemGuidFactory : IGuidFactory +{ + public Guid NewUuidV7() => Guid.CreateVersion7(); + + public Guid NewUuidV4() => Guid.NewGuid(); +} diff --git a/backend/src/LearnStack.SharedKernel/LearnStack.SharedKernel.csproj b/backend/src/LearnStack.SharedKernel/LearnStack.SharedKernel.csproj index f20e08c..a677d49 100644 --- a/backend/src/LearnStack.SharedKernel/LearnStack.SharedKernel.csproj +++ b/backend/src/LearnStack.SharedKernel/LearnStack.SharedKernel.csproj @@ -4,4 +4,28 @@ LearnStack.SharedKernel + + + + + + + + + + + diff --git a/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs b/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs new file mode 100644 index 0000000..b4a67ed --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs @@ -0,0 +1,61 @@ +namespace LearnStack.SharedKernel.Localization; + +/// +/// Localization-key carrier for every user-facing message LearnStack +/// returns to the frontend. The backend never returns raw English text; +/// it returns a whose +/// resolves to a translation on the client. +/// +/// +/// The lockey_ prefix invariant is enforced at the constructor: +/// every key must match the format the frontend's +/// i18n bundles ship under. Mis-prefixed keys fail loud at the +/// point of construction rather than silently resolving to "missing +/// translation" at render time. Per Phase 02a Packet 2. +/// +public sealed record LocalizedMessage +{ + /// + /// The required prefix for every localization key. The frontend's + /// translation catalogues use the same prefix. + /// + public const string RequiredPrefix = "lockey_"; + + private readonly IReadOnlyDictionary? _params; + + public LocalizedMessage(string key, IReadOnlyDictionary? @params = null) + { + ArgumentException.ThrowIfNullOrWhiteSpace(key); + + if (!key.StartsWith(RequiredPrefix, StringComparison.Ordinal)) + { + throw new ArgumentException( + $"Localization key must start with '{RequiredPrefix}'. Got: '{key}'.", + nameof(key)); + } + + Key = key; + _params = @params; + } + + /// + /// The localization key (always begins with ). + /// Routing logic and error-tracking provider tags use this verbatim. + /// + public string Key { get; } + + /// + /// Optional ICU MessageFormat parameter set. Frontend interpolates + /// these into the resolved string. null when the message takes + /// no parameters. + /// + public IReadOnlyDictionary? Params => _params; + + /// + /// Convenience factory equivalent to new LocalizedMessage(key, params). + /// + public static LocalizedMessage Of( + string key, + IReadOnlyDictionary? @params = null) => + new(key, @params); +} diff --git a/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs b/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs new file mode 100644 index 0000000..7b0dd2a --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs @@ -0,0 +1,33 @@ +namespace LearnStack.SharedKernel.Pagination; + +/// +/// Cursor-pagination request. Per Standards 04 § Pagination, cursor is the +/// default for every list endpoint. The is an opaque +/// token the server minted on a previous response; the client never parses +/// it. defaults to and is +/// bounded by . +/// +public sealed record CursorPagination(string? Cursor = null, int Limit = 20) +{ + public const int DefaultLimit = 20; + + public const int MaxLimit = 100; + + /// + /// Validates the request against the standard bounds. Returns a + /// normalised request with clamped to + /// ; throws when the limit is non-positive. + /// + public CursorPagination Normalised() + { + if (Limit <= 0) + { + throw new ArgumentOutOfRangeException( + nameof(Limit), + Limit, + $"Limit must be > 0. Got: {Limit}."); + } + + return Limit > MaxLimit ? this with { Limit = MaxLimit } : this; + } +} diff --git a/backend/src/LearnStack.SharedKernel/Pagination/Page.cs b/backend/src/LearnStack.SharedKernel/Pagination/Page.cs new file mode 100644 index 0000000..e352a2a --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Pagination/Page.cs @@ -0,0 +1,28 @@ +using System.Diagnostics.CodeAnalysis; + +namespace LearnStack.SharedKernel.Pagination; + +/// +/// Cursor-paginated response. The are the page +/// payload; carries the next/previous opaque +/// cursors plus the boolean hints the client uses to render +/// pagination controls. +/// +/// +/// CA1000 (do not declare static members on generic types) is intentionally +/// suppressed for : Page<T>.Empty mirrors +/// the canonical factory pattern (Array.Empty<T>, +/// ReadOnlyCollection<T>.Empty) and removes per-call-site +/// boilerplate. The alternative (Page.Empty<T>() helper) +/// forces callers to repeat T. +/// +[SuppressMessage( + "Design", + "CA1000:Do not declare static members on generic types", + Justification = "Canonical empty-instance pattern (mirrors Array.Empty); Result.Ok/Fail uses the same lineage.")] +public sealed record Page(IReadOnlyList Items, PageInfo PageInfo) +{ + public static Page Empty { get; } = new( + Array.Empty(), + new PageInfo(null, null, HasNext: false, HasPrevious: false)); +} diff --git a/backend/src/LearnStack.SharedKernel/Pagination/PageInfo.cs b/backend/src/LearnStack.SharedKernel/Pagination/PageInfo.cs new file mode 100644 index 0000000..374cbec --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Pagination/PageInfo.cs @@ -0,0 +1,12 @@ +namespace LearnStack.SharedKernel.Pagination; + +/// +/// Cursor-pagination response envelope. Surface matches Standards 04 +/// § Pagination so OpenAPI generation produces a uniform shape across +/// every list endpoint. +/// +public sealed record PageInfo( + string? NextCursor, + string? PreviousCursor, + bool HasNext, + bool HasPrevious); diff --git a/backend/src/LearnStack.SharedKernel/Persistence/IOptimisticConcurrency.cs b/backend/src/LearnStack.SharedKernel/Persistence/IOptimisticConcurrency.cs new file mode 100644 index 0000000..9100859 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Persistence/IOptimisticConcurrency.cs @@ -0,0 +1,18 @@ +namespace LearnStack.SharedKernel.Persistence; + +/// +/// Marker for any entity whose updates use optimistic concurrency. EF Core +/// configures as the row version token so concurrent +/// updates fail with DbUpdateConcurrencyException — translated to a +/// Result.Fail(LocalizedMessage.Of("lockey_concurrency_conflict")) +/// per ADR-0032 § Sub-decision 6. +/// +public interface IOptimisticConcurrency +{ + /// + /// Monotonically-increasing version counter. EF Core bumps this on + /// every SaveChangesAsync; aggregate code does not mutate it + /// directly. + /// + uint Version { get; } +} diff --git a/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs b/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs new file mode 100644 index 0000000..fd4dbff --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs @@ -0,0 +1,20 @@ +namespace LearnStack.SharedKernel.Persistence; + +/// +/// Marker for any entity that participates in soft deletion. EF Core +/// global query filters and the audit pipeline read this interface; +/// callers never set / +/// directly — use the aggregate's domain method (typically +/// AuditableEntity.SoftDelete). +/// +public interface ISoftDelete +{ + DateTimeOffset? DeletedAt { get; } + + Guid? DeletedBy { get; } + + /// + /// Convenience projection of . + /// + bool IsDeleted => DeletedAt.HasValue; +} diff --git a/backend/src/LearnStack.SharedKernel/Random/FixedRandom.cs b/backend/src/LearnStack.SharedKernel/Random/FixedRandom.cs new file mode 100644 index 0000000..b4cbe84 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Random/FixedRandom.cs @@ -0,0 +1,24 @@ +namespace LearnStack.SharedKernel.Random; + +/// +/// Deterministic for tests. Seeded +/// reproduces the same sequence per run. +/// +public sealed class FixedRandom : IRandom +{ + private readonly System.Random _random; + + public FixedRandom(int seed) + { + _random = new System.Random(seed); + } + + public int Next(int maxExclusive) => _random.Next(maxExclusive); + + public int Next(int minInclusive, int maxExclusive) => + _random.Next(minInclusive, maxExclusive); + + public double NextDouble() => _random.NextDouble(); + + public void NextBytes(Span destination) => _random.NextBytes(destination); +} diff --git a/backend/src/LearnStack.SharedKernel/Random/IRandom.cs b/backend/src/LearnStack.SharedKernel/Random/IRandom.cs new file mode 100644 index 0000000..295c5d4 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Random/IRandom.cs @@ -0,0 +1,42 @@ +using System.Diagnostics.CodeAnalysis; + +namespace LearnStack.SharedKernel.Random; + +/// +/// Randomness abstraction for domain and application code. Production code +/// never instantiates directly so tests can pin +/// the sequence deterministically per Standards 02 § Time. Cryptographic +/// randomness uses System.Security.Cryptography.RandomNumberGenerator +/// and is out of scope for this abstraction. +/// +/// +/// CA1716 (Next is a VB reserved keyword) is suppressed: LearnStack +/// is C#-only per ADR-0032 and the Next name matches the BCL +/// surface a future maintainer expects. +/// +[SuppressMessage( + "Naming", + "CA1716:Identifiers should not match keywords", + Justification = "Mirrors System.Random.Next; LearnStack is C#-only per ADR-0032; no VB consumer affected.")] +public interface IRandom +{ + /// + /// Returns a non-negative random integer less than . + /// + int Next(int maxExclusive); + + /// + /// Returns a random integer in [minInclusive, maxExclusive). + /// + int Next(int minInclusive, int maxExclusive); + + /// + /// Returns a random double in [0.0, 1.0). + /// + double NextDouble(); + + /// + /// Fills the destination span with random bytes. + /// + void NextBytes(Span destination); +} diff --git a/backend/src/LearnStack.SharedKernel/Random/SystemRandom.cs b/backend/src/LearnStack.SharedKernel/Random/SystemRandom.cs new file mode 100644 index 0000000..220d6d4 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Random/SystemRandom.cs @@ -0,0 +1,17 @@ +namespace LearnStack.SharedKernel.Random; + +/// +/// Production backed by . +/// Thread-safe; registered as a singleton at the composition root. +/// +public sealed class SystemRandom : IRandom +{ + public int Next(int maxExclusive) => System.Random.Shared.Next(maxExclusive); + + public int Next(int minInclusive, int maxExclusive) => + System.Random.Shared.Next(minInclusive, maxExclusive); + + public double NextDouble() => System.Random.Shared.NextDouble(); + + public void NextBytes(Span destination) => System.Random.Shared.NextBytes(destination); +} diff --git a/backend/src/LearnStack.SharedKernel/Results/Error.cs b/backend/src/LearnStack.SharedKernel/Results/Error.cs index 7bb6019..aaeed8a 100644 --- a/backend/src/LearnStack.SharedKernel/Results/Error.cs +++ b/backend/src/LearnStack.SharedKernel/Results/Error.cs @@ -1,9 +1,14 @@ using System.Diagnostics.CodeAnalysis; +using LearnStack.SharedKernel.Localization; namespace LearnStack.SharedKernel.Results; /// -/// Result-pattern error payload used by . +/// Result-pattern error payload used by . The +/// carries the lockey_ localization key the +/// frontend resolves; is a convenience projection of +/// Message.Key for routing logic (ADR-0032 § Sub-decision 6's +/// ResultExtensions.ToActionResult() matches on it). /// /// /// CA1716 (avoid reserved language keywords as type names) is intentionally @@ -17,9 +22,14 @@ namespace LearnStack.SharedKernel.Results; "CA1716:Identifiers should not match keywords", Justification = "Result+Error pattern — C#-only codebase per ADR-0032; no VB consumer affected.")] public sealed record Error( - string Code, - string Message, + LocalizedMessage Message, IReadOnlyDictionary? Details = null) { - public static readonly Error None = new("none", string.Empty); + /// + /// Localization-key projection of . Equivalent to + /// Message.Key; surfaced as a top-level property so + /// Result.ToActionResult() and Problem Details writers can read + /// it without dereferencing the nested record. + /// + public string Code => Message.Key; } diff --git a/backend/src/LearnStack.SharedKernel/Results/Result.cs b/backend/src/LearnStack.SharedKernel/Results/Result.cs index f64fd94..84e8970 100644 --- a/backend/src/LearnStack.SharedKernel/Results/Result.cs +++ b/backend/src/LearnStack.SharedKernel/Results/Result.cs @@ -1,7 +1,55 @@ using System.Diagnostics.CodeAnalysis; +using LearnStack.SharedKernel.Localization; namespace LearnStack.SharedKernel.Results; +/// +/// Non-generic surface every exposes. Pipeline +/// behaviors operate on this contract so they can construct the correct +/// concrete Result<TResponse> shape via the +/// factory (ADR-0032 § Sub-decision 3). +/// +/// +/// CA1716 (Error collides with VB's reserved keyword) is suppressed +/// for the same reason it is suppressed on the record +/// itself — LearnStack is C#-only per ADR-0032. +/// +[SuppressMessage( + "Naming", + "CA1716:Identifiers should not match keywords", + Justification = "Result+Error pattern — C#-only codebase per ADR-0032; no VB consumer affected.")] +public interface IResultBase +{ + bool IsSuccess { get; } + + bool IsFailure { get; } + + LocalizedMessage? SuccessMessage { get; } + + Error? Error { get; } +} + +/// +/// Static helpers for constructing generically +/// when the type parameter is known only at runtime (e.g. inside +/// ValidationBehavior). Per ADR-0032 § Sub-decision 3. +/// +public static class Result +{ + public static Result Ok(T value, LocalizedMessage? message = null) => + Result.Ok(value, message); + + public static Result Fail(Error error) => Result.Fail(error); + + /// + /// Reflection-friendly failure factory used by MediatR pipeline behaviors + /// (ValidationBehavior, AuthorizationBehavior) that + /// short-circuit a generic handler without knowing T at compile + /// time. Per ADR-0032 § Sub-decision 3. + /// + public static Result FailFor(Error error) => Result.Fail(error); +} + /// /// Result-pattern wrapper. Returned by every MediatR command/query handler /// per ADR-0032 § Error Model. @@ -20,9 +68,16 @@ namespace LearnStack.SharedKernel.Results; "Design", "CA1000:Do not declare static members on generic types", Justification = "Result+Error factory pattern per ADR-0032 — canonical shape across FluentResults / Ardalis.Result lineage.")] -public sealed record Result(bool IsSuccess, T? Value, Error? Error) +public sealed record Result( + bool IsSuccess, + T? Value, + Error? Error, + LocalizedMessage? SuccessMessage = null) : IResultBase { - public static Result Ok(T value) => new(true, value, null); + public bool IsFailure => !IsSuccess; + + public static Result Ok(T value, LocalizedMessage? message = null) => + new(true, value, null, message); public static Result Fail(Error error) => new(false, default, error); } diff --git a/backend/src/LearnStack.SharedKernel/Time/FixedClock.cs b/backend/src/LearnStack.SharedKernel/Time/FixedClock.cs new file mode 100644 index 0000000..edbd8c4 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Time/FixedClock.cs @@ -0,0 +1,21 @@ +namespace LearnStack.SharedKernel.Time; + +/// +/// Deterministic for tests. Time advances only through +/// / — never spontaneously. +/// +public sealed class FixedClock : IClock +{ + private DateTimeOffset _utcNow; + + public FixedClock(DateTimeOffset utcNow) + { + _utcNow = utcNow; + } + + public DateTimeOffset UtcNow => _utcNow; + + public void SetUtcNow(DateTimeOffset utcNow) => _utcNow = utcNow; + + public void Advance(TimeSpan delta) => _utcNow = _utcNow.Add(delta); +} diff --git a/backend/src/LearnStack.SharedKernel/Time/IClock.cs b/backend/src/LearnStack.SharedKernel/Time/IClock.cs new file mode 100644 index 0000000..2022ba2 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Time/IClock.cs @@ -0,0 +1,16 @@ +namespace LearnStack.SharedKernel.Time; + +/// +/// Wall-clock abstraction for domain and application code. Per Standards 02 +/// § Time, no production code reads DateTime.UtcNow / +/// DateTimeOffset.UtcNow directly — every timestamp flows through +/// so tests can pin time deterministically. +/// +public interface IClock +{ + /// + /// Current UTC instant. Persisted timestamps are always UTC; conversion + /// to a presentation timezone happens at the surface boundary only. + /// + DateTimeOffset UtcNow { get; } +} diff --git a/backend/src/LearnStack.SharedKernel/Time/SystemClock.cs b/backend/src/LearnStack.SharedKernel/Time/SystemClock.cs new file mode 100644 index 0000000..d1f37c3 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Time/SystemClock.cs @@ -0,0 +1,10 @@ +namespace LearnStack.SharedKernel.Time; + +/// +/// Production implementation backed by the BCL system +/// clock. Registered as a singleton at the composition root. +/// +public sealed class SystemClock : IClock +{ + public DateTimeOffset UtcNow => DateTimeOffset.UtcNow; +} diff --git a/backend/tests/LearnStack.Tests.Unit/LearnStack.Tests.Unit.csproj b/backend/tests/LearnStack.Tests.Unit/LearnStack.Tests.Unit.csproj index 3850793..14c940c 100644 --- a/backend/tests/LearnStack.Tests.Unit/LearnStack.Tests.Unit.csproj +++ b/backend/tests/LearnStack.Tests.Unit/LearnStack.Tests.Unit.csproj @@ -19,6 +19,14 @@ + + + diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs new file mode 100644 index 0000000..4271a91 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs @@ -0,0 +1,64 @@ +using FluentAssertions; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Domain; + +public sealed class AuditableEntityTests +{ + private static readonly DateTimeOffset T0 = + new(2026, 05, 21, 10, 00, 00, TimeSpan.Zero); + + private static readonly Guid Actor = Guid.Parse("019712ac-aaaa-7000-8000-000000000aaa"); + + [Fact] + public void MarkCreated_SetsCreatedAtAndCreatedBy() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + + aggregate.MarkCreated(T0, Actor); + + aggregate.CreatedAt.Should().Be(T0); + aggregate.CreatedBy.Should().Be(Actor); + aggregate.UpdatedAt.Should().BeNull(); + aggregate.DeletedAt.Should().BeNull(); + aggregate.IsDeleted.Should().BeFalse(); + } + + [Fact] + public void MarkUpdated_SetsUpdatedAtAndUpdatedBy() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + aggregate.MarkCreated(T0, Actor); + var laterActor = Guid.Parse("019712ac-bbbb-7000-8000-000000000bbb"); + var t1 = T0.AddHours(1); + + aggregate.MarkUpdated(t1, laterActor); + + aggregate.UpdatedAt.Should().Be(t1); + aggregate.UpdatedBy.Should().Be(laterActor); + aggregate.CreatedAt.Should().Be(T0, "create stays untouched"); + } + + [Fact] + public void SoftDelete_SetsDeletedAtDeletedByAndIsDeleted() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + aggregate.MarkCreated(T0, Actor); + var t1 = T0.AddDays(3); + + aggregate.SoftDelete(t1, Actor); + + aggregate.DeletedAt.Should().Be(t1); + aggregate.DeletedBy.Should().Be(Actor); + aggregate.IsDeleted.Should().BeTrue(); + } + + [Fact] + public void NewlyConstructed_AggregateIsNotDeleted() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + + aggregate.IsDeleted.Should().BeFalse(); + aggregate.Version.Should().Be(0u); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/DomainEventTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/DomainEventTests.cs new file mode 100644 index 0000000..7575186 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/DomainEventTests.cs @@ -0,0 +1,44 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Domain; +using MediatR; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Domain; + +public sealed class DomainEventTests +{ + [Fact] + public void IDomainEvent_IsAMediatRNotification() + { + typeof(INotification).IsAssignableFrom(typeof(IDomainEvent)) + .Should().BeTrue("domain events dispatch in-process through MediatR"); + } + + [Fact] + public void DefaultEventId_IsUuidV7() + { + var @event = new TestDomainEvent("x"); + + @event.EventId.Version.Should().Be(7); + } + + [Fact] + public void DefaultOccurredAt_IsRecent() + { + var before = DateTimeOffset.UtcNow; + + var @event = new TestDomainEvent("x"); + + @event.OccurredAt.Should().BeOnOrAfter(before).And.BeOnOrBefore(DateTimeOffset.UtcNow); + } + + [Fact] + public void RecordInit_AllowsTimeOverride() + { + var fixedAt = new DateTimeOffset(2026, 01, 01, 0, 0, 0, TimeSpan.Zero); + + var @event = new TestDomainEvent("x") { OccurredAt = fixedAt }; + + @event.OccurredAt.Should().Be(fixedAt); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/EntityTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/EntityTests.cs new file mode 100644 index 0000000..0733052 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/EntityTests.cs @@ -0,0 +1,61 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Domain; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Domain; + +public sealed class EntityTests +{ + [Fact] + public void Equality_IsIdentityBased() + { + var id = TestId.New(); + var a = new TestAggregate(id); + var b = new TestAggregate(id); + + a.Equals(b).Should().BeTrue(); + a.GetHashCode().Should().Be(b.GetHashCode()); + } + + [Fact] + public void Equality_AcrossDifferentIds_IsFalse() + { + var a = new TestAggregate(TestId.New()); + var b = new TestAggregate(TestId.New()); + + a.Equals(b).Should().BeFalse(); + } + + [Fact] + public void RaiseDomainEvent_AppendsToCollection() + { + var aggregate = new TestAggregate(TestId.New()); + IDomainEvent @event = new TestDomainEvent("hello"); + + aggregate.Raise(@event); + + aggregate.DomainEvents.Should().ContainSingle().Which.Should().Be(@event); + } + + [Fact] + public void ClearDomainEvents_EmptiesTheCollection() + { + var aggregate = new TestAggregate(TestId.New()); + aggregate.Raise(new TestDomainEvent("a")); + aggregate.Raise(new TestDomainEvent("b")); + + aggregate.ClearDomainEvents(); + + aggregate.DomainEvents.Should().BeEmpty(); + } + + [Fact] + public void RaiseDomainEvent_WithNull_Throws() + { + var aggregate = new TestAggregate(TestId.New()); + + var act = () => aggregate.Raise(null!); + + act.Should().Throw(); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestAggregate.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestAggregate.cs new file mode 100644 index 0000000..71efead --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestAggregate.cs @@ -0,0 +1,29 @@ +using LearnStack.SharedKernel.Domain; + +namespace LearnStack.Tests.Unit.SharedKernel.Domain; + +/// +/// Test double for . Exposes +/// so unit tests can drive the domain-event collector +/// without inventing a fake aggregate per test. +/// +internal sealed class TestAggregate : Entity +{ + public TestAggregate(TestId id) : base(id) { } + + public TestAggregate() { } + + public void Raise(IDomainEvent domainEvent) => RaiseDomainEvent(domainEvent); +} + +/// +/// Test double for . +/// +internal sealed class TestAuditableAggregate : AuditableEntity +{ + public TestAuditableAggregate(TestId id) : base(id) { } + + public TestAuditableAggregate() { } +} + +internal sealed record TestDomainEvent(string Payload) : DomainEvent; diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestId.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestId.cs new file mode 100644 index 0000000..5739698 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestId.cs @@ -0,0 +1,16 @@ +using LearnStack.SharedKernel.Identifiers; +using Vogen; + +namespace LearnStack.Tests.Unit.SharedKernel.Domain; + +/// +/// Synthetic Vogen-emitted ID used by the Entity / AuditableEntity / Vogen +/// smoke tests. Lives in the test project (not SharedKernel) because +/// concrete aggregate IDs belong to their owning module — but the emitter +/// pipeline has to be exercisable end-to-end before any module ships one. +/// +[ValueObject(LearnStackVogenDefaults.IdMask)] +public readonly partial record struct TestId : IStronglyTypedId +{ + public static TestId New() => From(Guid.CreateVersion7()); +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs new file mode 100644 index 0000000..3180953 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs @@ -0,0 +1,40 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Localization; +using LearnStack.SharedKernel.Results; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel; + +public sealed class ErrorTests +{ + [Fact] + public void Code_DerivesFromMessageKey() + { + var error = new Error(LocalizedMessage.Of("lockey_forbidden")); + + error.Code.Should().Be("lockey_forbidden"); + } + + [Fact] + public void Details_AreOptional() + { + var error = new Error(LocalizedMessage.Of("lockey_validation_failed")); + + error.Details.Should().BeNull(); + } + + [Fact] + public void Details_RetainFieldErrors() + { + var details = new Dictionary + { + ["email"] = ["lockey_email_invalid", "lockey_email_required"], + }; + + var error = new Error( + LocalizedMessage.Of("lockey_validation_failed"), + details); + + error.Details.Should().BeEquivalentTo(details); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/FixedGuidFactoryTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/FixedGuidFactoryTests.cs new file mode 100644 index 0000000..d4ab277 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/FixedGuidFactoryTests.cs @@ -0,0 +1,41 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Identifiers; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Identifiers; + +public sealed class FixedGuidFactoryTests +{ + private static readonly Guid G1 = Guid.Parse("019712ac-1111-7000-8000-000000000001"); + private static readonly Guid G2 = Guid.Parse("019712ac-2222-7000-8000-000000000002"); + + [Fact] + public void NewUuidV7_ReturnsTheNextSequenceValue() + { + var factory = new FixedGuidFactory(G1, G2); + + factory.NewUuidV7().Should().Be(G1); + factory.NewUuidV7().Should().Be(G2); + } + + [Fact] + public void NewUuidV7_AfterSequenceExhausted_Throws() + { + var factory = new FixedGuidFactory(G1); + factory.NewUuidV7(); + + var act = () => factory.NewUuidV7(); + + act.Should().Throw() + .WithMessage("*exhausted*"); + } + + [Fact] + public void NewUuidV4_DrawsFromTheSameSequence() + { + var factory = new FixedGuidFactory(G1, G2); + + factory.NewUuidV4().Should().Be(G1); + factory.NewUuidV4().Should().Be(G2); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/SystemGuidFactoryTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/SystemGuidFactoryTests.cs new file mode 100644 index 0000000..040211b --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/SystemGuidFactoryTests.cs @@ -0,0 +1,39 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Identifiers; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Identifiers; + +public sealed class SystemGuidFactoryTests +{ + [Fact] + public void NewUuidV7_ProducesVersion7Guid() + { + var factory = new SystemGuidFactory(); + + var guid = factory.NewUuidV7(); + + guid.Version.Should().Be(7); + } + + [Fact] + public void NewUuidV4_ProducesVersion4Guid() + { + var factory = new SystemGuidFactory(); + + var guid = factory.NewUuidV4(); + + guid.Version.Should().Be(4); + } + + [Fact] + public void NewUuidV7_TwoCalls_ProduceDifferentGuids() + { + var factory = new SystemGuidFactory(); + + var a = factory.NewUuidV7(); + var b = factory.NewUuidV7(); + + a.Should().NotBe(b); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/VogenIdEmissionTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/VogenIdEmissionTests.cs new file mode 100644 index 0000000..a183233 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/VogenIdEmissionTests.cs @@ -0,0 +1,48 @@ +using System.Text.Json; +using FluentAssertions; +using LearnStack.SharedKernel.Identifiers; +using LearnStack.Tests.Unit.SharedKernel.Domain; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Identifiers; + +/// +/// End-to-end smoke test for the Vogen emitter pipeline (per ADR-0023). +/// The synthetic declares the canonical annotation +/// [ValueObject<Guid>(LearnStackVogenDefaults.IdMask)]; this +/// fixture asserts the four emitted artefacts are wired: +/// - System.Text.Json round trip (Conversions.SystemTextJson). +/// - TypeConverter parse/format (Conversions.TypeConverter). +/// - projection. +/// - Type-safe inequality between two arbitrary IDs. +/// EF Core converter / minimal-API model binder are exercised at the +/// integration-test layer when the first DbContext lands (Packet 6+). +/// +public sealed class VogenIdEmissionTests +{ + [Fact] + public void IStronglyTypedId_Value_ExposesUnderlyingGuid() + { + var guid = Guid.CreateVersion7(); + var id = TestId.From(guid); + + ((IStronglyTypedId)id).Value.Should().Be(guid); + } + + [Fact] + public void SystemTextJson_RoundTrip_PreservesValue() + { + var original = TestId.New(); + + var json = JsonSerializer.Serialize(original); + var decoded = JsonSerializer.Deserialize(json); + + decoded.Should().Be(original); + } + + [Fact] + public void TwoIdsWithDifferentGuids_AreNotEqual() + { + TestId.New().Should().NotBe(TestId.New()); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Localization/LocalizedMessageTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Localization/LocalizedMessageTests.cs new file mode 100644 index 0000000..f9f1bc7 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Localization/LocalizedMessageTests.cs @@ -0,0 +1,62 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Localization; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Localization; + +public sealed class LocalizedMessageTests +{ + [Fact] + public void Ctor_WithLockeyPrefix_Constructs() + { + var message = new LocalizedMessage("lockey_validation_failed"); + + message.Key.Should().Be("lockey_validation_failed"); + message.Params.Should().BeNull(); + } + + [Fact] + public void Ctor_WithParams_RetainsThem() + { + var @params = new Dictionary { ["field"] = "email" }; + + var message = new LocalizedMessage("lockey_validation_required", @params); + + message.Params.Should().BeEquivalentTo(@params); + } + + [Theory] + [InlineData("validation_failed")] + [InlineData("lockEY_wrong_case")] + [InlineData("LOCKEY_uppercase")] + [InlineData("error.something")] + [InlineData(" lockey_leading_space")] + public void Ctor_WithoutLockeyPrefix_Throws(string badKey) + { + var act = () => new LocalizedMessage(badKey); + + act.Should().Throw() + .WithMessage("*lockey_*"); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void Ctor_WithEmptyOrWhitespaceKey_Throws(string key) + { + var act = () => new LocalizedMessage(key); + + act.Should().Throw(); + } + + [Fact] + public void Of_IsEquivalentToCtor() + { + var @params = new Dictionary { ["count"] = "5" }; + + var direct = new LocalizedMessage("lockey_too_many", @params); + var viaOf = LocalizedMessage.Of("lockey_too_many", @params); + + viaOf.Should().Be(direct); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs new file mode 100644 index 0000000..d30f310 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs @@ -0,0 +1,44 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Pagination; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Pagination; + +public sealed class CursorPaginationTests +{ + [Fact] + public void DefaultLimit_MatchesStandards04() + { + new CursorPagination().Limit.Should().Be(CursorPagination.DefaultLimit); + CursorPagination.DefaultLimit.Should().Be(20); + CursorPagination.MaxLimit.Should().Be(100); + } + + [Fact] + public void Normalised_LimitAboveMax_ClampsToMaxLimit() + { + var request = new CursorPagination(Limit: 500); + + request.Normalised().Limit.Should().Be(CursorPagination.MaxLimit); + } + + [Fact] + public void Normalised_LimitWithinBounds_ReturnsSameInstance() + { + var request = new CursorPagination(Cursor: "abc", Limit: 50); + + request.Normalised().Should().Be(request); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void Normalised_LimitNonPositive_Throws(int limit) + { + var request = new CursorPagination(Limit: limit); + + var act = () => request.Normalised(); + + act.Should().Throw(); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/PageTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/PageTests.cs new file mode 100644 index 0000000..0124a09 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/PageTests.cs @@ -0,0 +1,29 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Pagination; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Pagination; + +public sealed class PageTests +{ + [Fact] + public void Empty_HasNoItems_AndNoCursors() + { + Page.Empty.Items.Should().BeEmpty(); + Page.Empty.PageInfo.NextCursor.Should().BeNull(); + Page.Empty.PageInfo.PreviousCursor.Should().BeNull(); + Page.Empty.PageInfo.HasNext.Should().BeFalse(); + Page.Empty.PageInfo.HasPrevious.Should().BeFalse(); + } + + [Fact] + public void Constructed_PageCarriesItemsAndPageInfo() + { + var info = new PageInfo("next", "prev", HasNext: true, HasPrevious: true); + + var page = new Page([1, 2, 3], info); + + page.Items.Should().BeEquivalentTo([1, 2, 3]); + page.PageInfo.Should().Be(info); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Random/FixedRandomTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Random/FixedRandomTests.cs new file mode 100644 index 0000000..582788f --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Random/FixedRandomTests.cs @@ -0,0 +1,41 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Random; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Random; + +public sealed class FixedRandomTests +{ + [Fact] + public void SameSeed_ProducesSameSequence() + { + var a = new FixedRandom(seed: 42); + var b = new FixedRandom(seed: 42); + + Enumerable.Range(0, 10).Select(_ => a.Next(1_000_000)) + .Should().Equal( + Enumerable.Range(0, 10).Select(_ => b.Next(1_000_000))); + } + + [Fact] + public void Next_BoundedByMaxExclusive() + { + var random = new FixedRandom(seed: 1); + + for (var i = 0; i < 100; i++) + { + random.Next(maxExclusive: 5).Should().BeInRange(0, 4); + } + } + + [Fact] + public void NextBytes_FillsTheDestinationSpan() + { + var random = new FixedRandom(seed: 7); + var buffer = new byte[16]; + + random.NextBytes(buffer); + + buffer.Should().NotEqual(new byte[16], "the seeded random fills bytes"); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/ResultTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ResultTests.cs index 439387d..af89d6d 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/ResultTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ResultTests.cs @@ -1,4 +1,5 @@ using FluentAssertions; +using LearnStack.SharedKernel.Localization; using LearnStack.SharedKernel.Results; using Xunit; @@ -12,19 +13,59 @@ public void Ok_WhenGivenValue_ReturnsSuccessResult() var result = Result.Ok(42); result.IsSuccess.Should().BeTrue(); + result.IsFailure.Should().BeFalse(); result.Value.Should().Be(42); result.Error.Should().BeNull(); + result.SuccessMessage.Should().BeNull(); + } + + [Fact] + public void Ok_WithSuccessMessage_RetainsIt() + { + var message = LocalizedMessage.Of("lockey_course_published"); + + var result = Result.Ok(42, message); + + result.SuccessMessage.Should().Be(message); } [Fact] public void Fail_WhenGivenError_ReturnsFailureResult() { - var error = new Error("test_error", "boom"); + var error = new Error(LocalizedMessage.Of("lockey_business_rule_violation")); var result = Result.Fail(error); result.IsSuccess.Should().BeFalse(); + result.IsFailure.Should().BeTrue(); result.Value.Should().Be(default); result.Error.Should().Be(error); } + + [Fact] + public void FailFor_ConstructsResultGenericallyForPipelineBehaviors() + { + var error = new Error(LocalizedMessage.Of("lockey_validation_failed")); + + var result = Result.FailFor(error); + + result.Should().BeOfType>(); + result.IsFailure.Should().BeTrue(); + result.Error.Should().Be(error); + } + + [Fact] + public void IResultBase_IsImplementedByResult() + { + // The cast is the point of this test — pipeline behaviors operate + // through the non-generic IResultBase surface. CA1859 would prefer + // the concrete type for performance; suppressed locally because the + // explicit interface contract is what we are asserting. +#pragma warning disable CA1859 + IResultBase result = Result.Ok(7); +#pragma warning restore CA1859 + + result.IsSuccess.Should().BeTrue(); + result.Error.Should().BeNull(); + } } diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/FixedClockTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/FixedClockTests.cs new file mode 100644 index 0000000..3ef5846 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/FixedClockTests.cs @@ -0,0 +1,40 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Time; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Time; + +public sealed class FixedClockTests +{ + private static readonly DateTimeOffset T0 = + new(2026, 05, 21, 10, 00, 00, TimeSpan.Zero); + + [Fact] + public void UtcNow_AfterConstruction_ReturnsConstructorValue() + { + var clock = new FixedClock(T0); + + clock.UtcNow.Should().Be(T0); + } + + [Fact] + public void Advance_AddsTheGivenInterval() + { + var clock = new FixedClock(T0); + + clock.Advance(TimeSpan.FromMinutes(30)); + + clock.UtcNow.Should().Be(T0.AddMinutes(30)); + } + + [Fact] + public void SetUtcNow_ReplacesTheCurrentInstant() + { + var clock = new FixedClock(T0); + var target = T0.AddDays(7); + + clock.SetUtcNow(target); + + clock.UtcNow.Should().Be(target); + } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/SystemClockTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/SystemClockTests.cs new file mode 100644 index 0000000..103c680 --- /dev/null +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/SystemClockTests.cs @@ -0,0 +1,21 @@ +using FluentAssertions; +using LearnStack.SharedKernel.Time; +using Xunit; + +namespace LearnStack.Tests.Unit.SharedKernel.Time; + +public sealed class SystemClockTests +{ + [Fact] + public void UtcNow_ReturnsCurrentInstant_WithinTolerance() + { + var clock = new SystemClock(); + + var before = DateTimeOffset.UtcNow; + var observed = clock.UtcNow; + var after = DateTimeOffset.UtcNow; + + observed.Should().BeOnOrAfter(before).And.BeOnOrBefore(after); + observed.Offset.Should().Be(TimeSpan.Zero, "SystemClock returns UTC"); + } +} diff --git a/docs/glossary.md b/docs/glossary.md index bb9891d..508a835 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -245,8 +245,14 @@ This glossary defines LearnStack-specific terms. When a term is ambiguous across | Term | Definition | |------|------------| -| **`Result`** | The sealed record `(bool IsSuccess, T? Value, Error? Error)` in `LearnStack.SharedKernel.Results`. Application + Domain layer methods return `Result` for **expected** outcomes (validation failed, not found, forbidden, business-rule violation). Exceptions are reserved for **unexpected** bugs / infrastructure faults. See [09-error-handling.md § Two-Track Model](standards/09-error-handling.md) and [ADR-0032](decisions/0032-exception-handling-logging-and-observability.md). | -| **`Error`** | The sealed record `(string Code, string Message, IReadOnlyDictionary? Details)` that travels inside `Result.Fail(error)`. `Code` is a stable English identifier from the 13-code catalogue ([09-error-handling.md § Result Type](standards/09-error-handling.md)); `Message` is localisable; `Details` carries field-level errors. | +| **`Result`** | The sealed record in `LearnStack.SharedKernel.Results` implementing `IResultBase`: carries `IsSuccess`, `Value`, optional `Error`, and an optional `SuccessMessage` (also a `LocalizedMessage`). Application + Domain layer methods return `Result` for **expected** outcomes (validation failed, not found, forbidden, business-rule violation). Exceptions are reserved for **unexpected** bugs / infrastructure faults. See [09-error-handling.md § Two-Track Model](standards/09-error-handling.md) and [ADR-0032](decisions/0032-exception-handling-logging-and-observability.md). | +| **`Error`** | The sealed record `(LocalizedMessage Message, IReadOnlyDictionary? Details)` that travels inside `Result.Fail(error)`. `Code` is a convenience projection of `Message.Key` (always begins with `lockey_`); `Details` carries field-level errors. Routing logic (`Result.ToActionResult()`) reads `Code`. Refactored in Phase 02a Packet 2; see [09-error-handling.md § Result Type](standards/09-error-handling.md). | +| **`LocalizedMessage`** | The sealed record `(string Key, IReadOnlyDictionary? Params)` in `LearnStack.SharedKernel.Localization`. `Key` MUST begin with `lockey_` — enforced at the constructor — so the frontend's translation catalogues (keyed under the same prefix) resolve every user-facing message without raw English on the wire. Used by every `Result.Fail(...)` payload and every `Result.Ok(value, message: ...)` success notification. Per [Phase 02a Packet 2](roadmap/phase-02a-kernel-tenancy.md). | +| **`IClock` / `IRandom` / `IGuidFactory`** | The three deterministic-test abstractions in `LearnStack.SharedKernel`. Production code never reads `DateTime.UtcNow`, instantiates `System.Random`, or calls `Guid.NewGuid()` directly — those calls go through the abstractions so tests pin the values via `FixedClock` / `FixedRandom` / `FixedGuidFactory`. Per Standards 02 § Time. | +| **`Entity` / `AuditableEntity`** | The two aggregate bases in `LearnStack.SharedKernel.Domain`. `Entity` is the append-only / audit-row base — identity, in-process domain events, identity-based equality. `AuditableEntity` is the mutable base — adds `CreatedAt/By`, `UpdatedAt/By`, `DeletedAt/By`, `Version`, and the `IsDeleted` projection by implementing `ISoftDelete` + `IOptimisticConcurrency`. `AuditEntry` (audit subsystem) inherits `Entity` — never `AuditableEntity` — by architecture-test rule. | +| **`IDomainEvent`** | The marker interface (`: MediatR.INotification`) every in-process domain event implements. Raised from aggregate methods, collected by the unit of work, dispatched in-process by MediatR. Distinct from integration events, which cross module boundaries through the outbox + Dapr pub/sub per [ADR-0010](decisions/0010-cross-module-communication.md). | +| **`CursorPagination` / `Page` / `PageInfo`** | The cursor-first pagination triple in `LearnStack.SharedKernel.Pagination` matching Standards 04 § Pagination. `CursorPagination(Cursor, Limit)` is the request (default `Limit = 20`, max 100); `Page(Items, PageInfo)` is the response; `PageInfo(NextCursor, PreviousCursor, HasNext, HasPrevious)` carries the opaque cursors the client never parses. | +| **`LearnStackVogenDefaults.IdMask`** | The canonical `Conversions` mask every Vogen-emitted ID and value object opts into: `EfCoreValueConverter \| SystemTextJson \| TypeConverter`. Per [ADR-0023](decisions/0023-strongly-typed-id-source-generator.md) every aggregate-root ID writes `[ValueObject(LearnStackVogenDefaults.IdMask)]`. | | **Pipeline Behavior** | A MediatR pipeline behavior — one of the eight canonical steps wrapping every command / query: `Validation → Logging → Audit → TenantContext → Authorization → Transaction → OutboxFlush → Handler`. The order is binding per [ADR-0032 § Sub-decision 2](decisions/0032-exception-handling-logging-and-observability.md); the architecture test `MediatR_Pipeline_Order_Matches_Canonical_Sequence` enforces it. | | **`IExceptionHandler` (LearnStack L1)** | The `.NET 8+` exception-handler interface; `LearnStackExceptionHandler : IExceptionHandler` is the final catch site for every unhandled exception. Maps to Problem Details, attaches `correlationId`, records the OTel span error, calls `IErrorTrackingProvider.CaptureAsync` when `ShouldCapture(ex)` is true. See [ADR-0032 § Sub-decision 1](decisions/0032-exception-handling-logging-and-observability.md). | | **Correlation ID** | The W3C `traceparent` value (or a derived UUID for fallback) that threads through every signal — logs, traces, audit rows, outbox rows, Hangfire payloads, Problem Details bodies — bound to a single user request or background operation. Same as `Activity.Current.TraceId` for HTTP requests; reconstructed from the outbox row / Hangfire payload / event envelope at consumer side. See [10-observability.md § Correlation](standards/10-observability.md). | diff --git a/docs/roadmap/phase-02a-kernel-tenancy.md b/docs/roadmap/phase-02a-kernel-tenancy.md index 89dee84..373d645 100644 --- a/docs/roadmap/phase-02a-kernel-tenancy.md +++ b/docs/roadmap/phase-02a-kernel-tenancy.md @@ -29,16 +29,32 @@ > Decision-only; no code. Standards 02 § Strongly-Typed Identifiers and > Standards 04 § Versioning cross-link to the new ADRs. > -> **Packet 2 — Shared Kernel core ⏳** -> `IClock`, `IRandom`, `IGuidFactory` (deterministic-test abstractions per -> Standards 02 § Time), `LocalizedMessage` (carrying the `lockey_` prefix -> invariant for `Result.Fail` payloads), `Entity` (append-only / audit -> aggregate base), `AuditableEntity` (mutable aggregate base with +> **Packet 2 — Shared Kernel core ✅** +> `IClock` + `SystemClock` / `FixedClock`, `IRandom` + `SystemRandom` / +> `FixedRandom`, `IGuidFactory` + `SystemGuidFactory` / `FixedGuidFactory` +> (deterministic-test abstractions per Standards 02 § Time); +> `LocalizedMessage` carrying the `lockey_` prefix invariant at the +> constructor (used by every `Result.Fail` and success-message payload); +> `Error` refactored to wrap `LocalizedMessage` with a `Code` projection +> over `Message.Key`; `Result` extended with `IResultBase`, success-message +> overload, and the static `Result.FailFor(error)` factory ADR-0032's +> `ValidationBehavior` consumes; `Entity` (append-only / audit +> aggregate base) + `AuditableEntity` (mutable aggregate base with > `CreatedAt` / `CreatedBy` / `UpdatedAt` / `UpdatedBy` / `DeletedAt` / -> `DeletedBy` / `Version`), soft-delete + optimistic concurrency primitives, -> domain-event model (in-process MediatR `INotification`), pagination model -> (cursor-first), strongly-typed ID emitter wired per ADR-0023. Unit tests -> for every primitive. +> `DeletedBy` / `Version` plus the `IsDeleted` projection); +> `ISoftDelete` + `IOptimisticConcurrency` marker interfaces; +> `IDomainEvent : INotification` + `DomainEvent` base + `IHasDomainEvents` +> aggregate-side collector; cursor-first pagination +> (`CursorPagination` / `Page` / `PageInfo` matching Standards 04 +> § Pagination). Vogen 7.0.0 wired per ADR-0023 with +> `LearnStackVogenDefaults.IdMask` carrying the canonical +> `EfCoreValueConverter | SystemTextJson | TypeConverter` mask; +> `IAggregateRoot` / `IHasId` interfaces require `TId : +> IStronglyTypedId` so future module aggregates inherit the +> constraint. 54 unit tests cover the primitives; the +> `VogenIdEmissionTests` smoke test asserts the emitter pipeline +> (Vogen `[ValueObject]` → `IStronglyTypedId.Value` → JSON +> round-trip) end-to-end via a synthetic `TestId` in the test project. > > **Packet 3 — Cross-cutting foundation ⏳** > Wires the [ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md) diff --git a/docs/standards/02-backend-coding.md b/docs/standards/02-backend-coding.md index e149ab6..6efc028 100644 --- a/docs/standards/02-backend-coding.md +++ b/docs/standards/02-backend-coding.md @@ -86,15 +86,27 @@ Two patterns coexist: - **`Result`** for *expected* outcomes (validation failure, not found, conflict). ```csharp -public sealed record Result(bool IsSuccess, T? Value, Error? Error) +public sealed record Result( + bool IsSuccess, + T? Value, + Error? Error, + LocalizedMessage? SuccessMessage = null) : IResultBase { - public static Result Ok(T value) => new(true, value, null); + public bool IsFailure => !IsSuccess; + public static Result Ok(T value, LocalizedMessage? message = null) => new(true, value, null, message); public static Result Fail(Error error) => new(false, default, error); } -public sealed record Error(string Code, string Message, IReadOnlyDictionary? Details = null); +public sealed record Error(LocalizedMessage Message, IReadOnlyDictionary? Details = null) +{ + public string Code => Message.Key; +} ``` +`LocalizedMessage`'s constructor enforces a `lockey_` key prefix — see +[09-error-handling.md § Result Type](09-error-handling.md) and +[Phase 02a Packet 2](../roadmap/phase-02a-kernel-tenancy.md). + Use cases for `Result`: - Validation outcomes. - Optimistic concurrency conflicts. diff --git a/docs/standards/09-error-handling.md b/docs/standards/09-error-handling.md index 0a8e3d8..d01ef28 100644 --- a/docs/standards/09-error-handling.md +++ b/docs/standards/09-error-handling.md @@ -30,19 +30,43 @@ Both end in **RFC 7807 Problem Details** at the API boundary. ## Result Type ```csharp -public sealed record Result(bool IsSuccess, T? Value, Error? Error) +public sealed record Result( + bool IsSuccess, + T? Value, + Error? Error, + LocalizedMessage? SuccessMessage = null) : IResultBase { - public static Result Ok(T value) => new(true, value, null); + public bool IsFailure => !IsSuccess; + public static Result Ok(T value, LocalizedMessage? message = null) => new(true, value, null, message); public static Result Fail(Error error) => new(false, default, error); } public sealed record Error( - string Code, - string Message, - IReadOnlyDictionary? Details = null); + LocalizedMessage Message, + IReadOnlyDictionary? Details = null) +{ + public string Code => Message.Key; +} + +public sealed record LocalizedMessage(string Key, IReadOnlyDictionary? Params = null) +{ + // ctor enforces Key.StartsWith("lockey_") — see Phase 02a Packet 2. +} ``` -Standard error codes (machine-readable, stable): +The `LocalizedMessage`'s `lockey_` prefix is invariant: the constructor +rejects any key that does not start with `lockey_`. Frontend translation +catalogues are keyed by the same prefix; backend code never returns raw +English. `Error.Code` is a convenience projection of `Message.Key` — +routing logic (`Result.ToActionResult()`, Problem Details writers) reads +the projection without dereferencing the nested record. Per +[Phase 02a Packet 2](../roadmap/phase-02a-kernel-tenancy.md) and +[ADR-0032 § Error Model](../decisions/0032-exception-handling-logging-and-observability.md). + +Standard error codes (machine-readable, stable). On the wire and in code, +every identifier below appears with the `lockey_` prefix (so +`Error.Code == "lockey_validation_failed"`); the table omits the prefix +for readability. | Code | Meaning | HTTP | |------|---------|------| From 377b34b8847be3ce751ba480679355c8998314d9 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Thu, 21 May 2026 12:57:04 +0300 Subject: [PATCH 2/8] =?UTF-8?q?docs(decisions,architecture):=20Packet=202?= =?UTF-8?q?=20follow-up=20=E2=80=94=20ADR-0023=20Amendment=201=20+=20event?= =?UTF-8?q?-outbox=20example?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two doc-only follow-ups surfaced by Phase 02a Packet 2 implementation: - ADR-0023 Amendment 1 (2026-05-21): record the concrete Vogen 7.0.0 version pin (6.0.7 was no longer on NuGet when Directory.Packages.props was wired), and clarify that the `Aggregate_Roots_Use_StronglyTypedId` architecture test lands with the first concrete aggregate (Packet 6 — Tenancy) rather than Packet 2 (which has no aggregates yet — the test would be vacuous). Decision section untouched. - architecture/15 example: `LocalizedMessage.Of("enrollment.created")` did not honour the `lockey_` prefix invariant Packet 2 just made canonical. Updated to `LocalizedMessage.Of("lockey_enrollment_created")` and the surrounding `Result.Success(...)` call switched to the canonical `Result.Ok(...)` factory. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/architecture/15-event-and-outbox.md | 2 +- ...0023-strongly-typed-id-source-generator.md | 24 ++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/docs/architecture/15-event-and-outbox.md b/docs/architecture/15-event-and-outbox.md index a5e2673..1da3038 100644 --- a/docs/architecture/15-event-and-outbox.md +++ b/docs/architecture/15-event-and-outbox.md @@ -161,7 +161,7 @@ public async Task> Handle(CreateEnrollmentCommand cmd, Can await _dbContext.SaveChangesAsync(ct); // aggregate + outbox row, atomic - return Result.Success(MapToDto(enrollment), LocalizedMessage.Of("enrollment.created")); + return Result.Ok(MapToDto(enrollment), LocalizedMessage.Of("lockey_enrollment_created")); } ``` diff --git a/docs/decisions/0023-strongly-typed-id-source-generator.md b/docs/decisions/0023-strongly-typed-id-source-generator.md index 5c7f568..406f587 100644 --- a/docs/decisions/0023-strongly-typed-id-source-generator.md +++ b/docs/decisions/0023-strongly-typed-id-source-generator.md @@ -244,7 +244,29 @@ Three things outweighed the appeal: ## Amendments -_(none yet)_ +### Amendment 1 — Vogen 7.0.0 pin + architecture-test placement (2026-05-21) + +Two clarifications surfaced when the ADR met implementation in +[Phase 02a Packet 2](../roadmap/phase-02a-kernel-tenancy.md): + +- **Pinned Vogen version is 7.0.0.** The ADR was written when 6.x was the + newest line; by the time `Directory.Packages.props` was wired the 6.0.x + series had been superseded on NuGet and the lowest available major was + `7.0.0`. The decision is unchanged — Vogen is still the chosen emitter + per the original "Decision" section; this amendment records the + concrete version pin for traceability. The previous-line placeholder + in Implementation Notes (`Version="..."`) is now read as `Version="7.0.0"`. + +- **`Aggregate_Roots_Use_StronglyTypedId` lands with the first aggregate, + not in Packet 2.** Implementation Notes originally said "lands in + Phase 02a Packet 2"; that placement is wrong because no module ships an + aggregate in Packet 2 (the first concrete aggregate IDs arrive in + Packet 6 — Tenancy schema foundations — and after). The test would have + been vacuously green for the entire Packet 2 → Packet 5 window. The + correct placement is alongside the first `IAggregateRoot` type, + catalogued under + [21-architecture-tests-catalogue.md](../standards/21-architecture-tests-catalogue.md) + at that point. ## References From 7c9133a75920930788d2dfbe63ebac32ccdf4e81 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Thu, 21 May 2026 13:29:05 +0300 Subject: [PATCH 3/8] =?UTF-8?q?fix(shared-kernel):=20packet=202=20review?= =?UTF-8?q?=20=E2=80=94=20blocker=20+=20major=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocker fixes from the dual review of db230ae: - Entity equality contract — transient entities (default Id) are now never equal to each other (only to themselves by reference), and a cross-runtime-type guard stops two distinct aggregate types that share the same Entity base + Id value from collapsing into the same change-tracker slot. - Result.FailFor shape — was ` => Result.Fail`, returning Result> whenever the pipeline behavior's TResponse is Result. Now constrained `where TResponse : IResultBase` and reflection-driven so it returns the concrete TResponse the handler signature expects. Non-Result generic argument fails loud. Major fixes: - Error.Code is now the unprefixed stable identifier ("validation_failed") projected from Message.Key by stripping the lockey_ prefix; the wire Problem Details code stays in sync with Standards 04 § Problem Details AND the lockey_ invariant by construction. Error.Details typed as IReadOnlyDictionary> so field-level messages cannot bypass the invariant. - Result.Ok throws on null value (Standards 09 § Forbidden); the primary constructor is now `internal` so positional record syntax cannot bypass the factory invariants. A new Unit value type covers the payload-less success shape. - DomainEvent.EventId / OccurredAt are `required init` — the BCL-default initializers bypassed IGuidFactory / IClock, defeating the deterministic rule the abstractions exist for. Concrete events now stamp via injected abstractions at the call site. - AuditableEntity actor columns (CreatedBy / UpdatedBy / DeletedBy) use a new strongly-typed UserId Vogen value object in SharedKernel.Identifiers rather than raw Guid; respects Standards 02 § Strongly-Typed Identifiers. The Identity module consumes the same UserId when it ships in 02b. ISoftDelete.DeletedBy stays Guid? at the marker layer (module-agnostic); AuditableEntity provides the Guid projection via explicit interface impl. - AuditableEntity.MarkCreated throws on second call (audit-trail integrity). SoftDelete also bumps UpdatedAt / UpdatedBy so "last touched" is monotonic; the audit pipeline still categorises the delete via DeletedAt. - CursorPagination ctor validates Limit > 0 (kernel-level guard); the API-layer FluentValidation is the place that translates malformed user input into Result.Fail. Normalised() no longer throws — only clamps. - SharedKernel build-time-only references documented in csproj comments: EF Core (Vogen-emitted converter requires it) and MediatR (IDomainEvent : INotification). The standards-side exception lands in the follow-up docs commit. Minors: - Files split per Standards 02 § File Organization: IHasId, IAggregateRoot, IResultBase, Result static helper, Unit, LocalizedMessage all live in their own file. Entity.DomainEvents returns the backing list directly (no per-call AsReadOnly allocation). - LocalizedMessage defensively copies Params at construction so caller mutations cannot leak in; structural equality (Key + Params) implemented explicitly so two messages built from the same source still compare equal. Empty params dictionary normalised to null. XSS contract documented (Params values must be plain text — frontend resolves via React text nodes, never dangerouslySetInnerHTML). - ISoftDelete.IsDeleted DIM removed; AuditableEntity exposes IsDeleted directly so the contract is consistent per access path. - FixedGuidFactory shared-queue contract documented in the XML doc. Tests: 64 unit + 17 architecture + 1 contract green. New coverage for transient + cross-type equality, FailFor>, Ok null guard, MarkCreated second-call throw, CursorPagination ctor validation, and LocalizedMessage defensive copy. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Domain/AuditableEntity.cs | 56 ++++++----- .../Domain/DomainEvent.cs | 24 +++-- .../LearnStack.SharedKernel/Domain/Entity.cs | 39 +++++++- .../Identifiers/FixedGuidFactory.cs | 11 +++ .../Identifiers/IAggregateRoot.cs | 12 --- .../Identifiers/IHasId.cs | 13 +++ .../Identifiers/UserId.cs | 17 ++++ .../Localization/LocalizedMessage.cs | 93 +++++++++++++++-- .../Pagination/CursorPagination.cs | 34 +++++-- .../Persistence/ISoftDelete.cs | 5 - .../LearnStack.SharedKernel/Results/Error.cs | 33 ++++--- .../Results/IResultBase.cs | 30 ++++++ .../Results/Result.Helpers.cs | 59 +++++++++++ .../LearnStack.SharedKernel/Results/Result.cs | 99 ++++++++----------- .../LearnStack.SharedKernel/Results/Unit.cs | 12 +++ .../Domain/AuditableEntityTests.cs | 34 ++++++- .../SharedKernel/Domain/DomainEventTests.cs | 34 +++---- .../SharedKernel/Domain/EntityTests.cs | 46 ++++++++- .../SharedKernel/Domain/TestAggregate.cs | 9 ++ .../SharedKernel/ErrorTests.cs | 26 ++++- .../Localization/LocalizedMessageTests.cs | 22 +++++ .../Pagination/CursorPaginationTests.cs | 32 ++++-- .../SharedKernel/ResultTests.cs | 50 +++++++++- 23 files changed, 611 insertions(+), 179 deletions(-) create mode 100644 backend/src/LearnStack.SharedKernel/Identifiers/IHasId.cs create mode 100644 backend/src/LearnStack.SharedKernel/Identifiers/UserId.cs create mode 100644 backend/src/LearnStack.SharedKernel/Results/IResultBase.cs create mode 100644 backend/src/LearnStack.SharedKernel/Results/Result.Helpers.cs create mode 100644 backend/src/LearnStack.SharedKernel/Results/Unit.cs diff --git a/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs b/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs index b279d62..9660587 100644 --- a/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs +++ b/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs @@ -14,14 +14,6 @@ namespace LearnStack.SharedKernel.Domain; /// / , which command /// handlers call via the they already inject. /// -/// -/// The actor columns (CreatedBy / UpdatedBy / DeletedBy) -/// store raw rather than a strongly-typed UserId: -/// cannot depend on the Identity -/// module (which lands in Phase 02b), and audit metadata is not an -/// aggregate-root reference in the Standards 02 § Strongly-Typed -/// Identifiers sense. -/// public abstract class AuditableEntity : Entity, ISoftDelete, IOptimisticConcurrency where TId : struct, IStronglyTypedId @@ -38,34 +30,45 @@ protected AuditableEntity() public DateTimeOffset CreatedAt { get; protected set; } - public Guid CreatedBy { get; protected set; } + public UserId CreatedBy { get; protected set; } public DateTimeOffset? UpdatedAt { get; protected set; } - public Guid? UpdatedBy { get; protected set; } + public UserId? UpdatedBy { get; protected set; } public DateTimeOffset? DeletedAt { get; protected set; } - public Guid? DeletedBy { get; protected set; } + public UserId? DeletedBy { get; protected set; } public uint Version { get; protected set; } + // ISoftDelete declares DeletedBy as Guid? for module-agnostic readers + // (EF global query filters, RLS policy plumbing). Surface the raw Guid + // projection through explicit interface implementation so callers + // holding the concrete type read the strongly-typed UserId? while + // cross-cutting infrastructure sees a Guid? at the marker layer. + Guid? ISoftDelete.DeletedBy => DeletedBy?.Value; + /// - /// Convenience projection of . Surfaced at this - /// level (rather than relying on 's default - /// interface implementation) so callers can read the property through - /// the concrete aggregate type without casting. + /// Convenience projection of . EF global query + /// filters typically gate on this property. /// public bool IsDeleted => DeletedAt.HasValue; /// /// Stamps / on first - /// persist. Idempotent: callers may invoke it once during aggregate - /// construction; later calls overwrite the values, which is the - /// caller's bug rather than a silent no-op. + /// persist. Throws when the aggregate already has a non-default + /// — audit-trail integrity rules out silent + /// overwrites. /// - public void MarkCreated(DateTimeOffset at, Guid by) + public void MarkCreated(DateTimeOffset at, UserId by) { + if (CreatedAt != default) + { + throw new InvalidOperationException( + "MarkCreated has already been called on this aggregate; the created-at / created-by columns are immutable after first stamp."); + } + CreatedAt = at; CreatedBy = by; } @@ -75,20 +78,25 @@ public void MarkCreated(DateTimeOffset at, Guid by) /// aggregate methods that mutate state; the audit pipeline reads these /// values when writing the audit entry. /// - public void MarkUpdated(DateTimeOffset at, Guid by) + public void MarkUpdated(DateTimeOffset at, UserId by) { UpdatedAt = at; UpdatedBy = by; } /// - /// Marks the entity as soft-deleted. EF's global query filter excludes - /// soft-deleted rows by default; the platform-admin scope can opt back - /// in explicitly. + /// Marks the entity as soft-deleted. Also bumps + /// / so the + /// "last touched at" timestamp is monotonic — replication / sync / + /// reporting jobs that scan on UpdatedAt see soft-deletes + /// without keying off DeletedAt separately. The audit row still + /// classifies the action as a delete via its own operation type. /// - public void SoftDelete(DateTimeOffset at, Guid by) + public void SoftDelete(DateTimeOffset at, UserId by) { DeletedAt = at; DeletedBy = by; + UpdatedAt = at; + UpdatedBy = by; } } diff --git a/backend/src/LearnStack.SharedKernel/Domain/DomainEvent.cs b/backend/src/LearnStack.SharedKernel/Domain/DomainEvent.cs index b9d64ca..f3bcce0 100644 --- a/backend/src/LearnStack.SharedKernel/Domain/DomainEvent.cs +++ b/backend/src/LearnStack.SharedKernel/Domain/DomainEvent.cs @@ -5,19 +5,27 @@ namespace LearnStack.SharedKernel.Domain; /// this and add their own payload fields: /// /// public sealed record CourseEnrolled(CourseId CourseId, UserId UserId) : DomainEvent; +/// +/// // raised from inside an aggregate method: +/// RaiseDomainEvent(new CourseEnrolled(Id, learnerId) +/// { +/// EventId = guids.NewUuidV7(), +/// OccurredAt = clock.UtcNow, +/// }); /// /// /// -/// The and defaults call -/// the BCL directly rather than going through IClock / -/// IGuidFactory: domain events are minted inside aggregate methods -/// that already received the relevant abstractions through their command -/// boundary. Overriding the defaults via record init in a test -/// remains straightforward. +/// and are required init: +/// every event MUST be stamped with the aggregate's injected +/// IGuidFactory / IClock. Defaulting these to +/// Guid.CreateVersion7() / DateTimeOffset.UtcNow would +/// silently bypass the deterministic-test abstractions every command +/// handler already threads in — which is the entire reason IClock +/// and IGuidFactory exist (Standards 02 § Time). /// public abstract record DomainEvent : IDomainEvent { - public Guid EventId { get; init; } = Guid.CreateVersion7(); + public required Guid EventId { get; init; } - public DateTimeOffset OccurredAt { get; init; } = DateTimeOffset.UtcNow; + public required DateTimeOffset OccurredAt { get; init; } } diff --git a/backend/src/LearnStack.SharedKernel/Domain/Entity.cs b/backend/src/LearnStack.SharedKernel/Domain/Entity.cs index 5f32b15..e14dcd1 100644 --- a/backend/src/LearnStack.SharedKernel/Domain/Entity.cs +++ b/backend/src/LearnStack.SharedKernel/Domain/Entity.cs @@ -16,9 +16,13 @@ namespace LearnStack.SharedKernel.Domain; /// AuditEntry_Inherits_Entity_Not_AuditableEntity guards that rule. /// /// -/// Equality is identity-based: two entities are equal iff their -/// s match. Reference equality is also exposed so EF Core's -/// change tracker behaves predictably. +/// Equality contract: identity-based, with three guards every aggregate +/// inherits — transient entities ( equal to +/// default(TId)) are never equal to each other (reference equality +/// is the only match), runtime-type mismatches are never equal even when +/// the underlying ID matches, and the hash code partitions transient +/// instances apart so EF Core's change-tracker identity map and any +/// HashSet in a collection navigation behave correctly. /// /// public abstract class Entity : IHasId, IHasDomainEvents @@ -38,7 +42,13 @@ protected Entity() public TId Id { get; protected init; } - public IReadOnlyCollection DomainEvents => _domainEvents.AsReadOnly(); + /// + /// In-process domain events raised since the last . + /// Returns the backing list as without + /// allocating a wrapper - the unit of work walks tracked entities on every + /// SaveChangesAsync, so the property is on a hot path. + /// + public IReadOnlyCollection DomainEvents => _domainEvents; protected void RaiseDomainEvent(IDomainEvent domainEvent) { @@ -60,8 +70,27 @@ public override bool Equals(object? obj) return true; } + // Cross-type guard: a Course and a hypothetical CourseDraft that both + // inherit Entity and share an Id value are still different + // runtime types and must not compare equal. + if (GetType() != other.GetType()) + { + return false; + } + + // Transient guard: two newly-constructed aggregates carry default(TId) + // until SaveChangesAsync stamps them. They must never collapse into + // each other in EF's change tracker or in a HashSet-backed navigation. + if (Id.Equals(default(TId)) || other.Id.Equals(default(TId))) + { + return false; + } + return Id.Equals(other.Id); } - public override int GetHashCode() => Id.GetHashCode(); + public override int GetHashCode() => + Id.Equals(default(TId)) + ? base.GetHashCode() + : HashCode.Combine(GetType(), Id); } diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/FixedGuidFactory.cs b/backend/src/LearnStack.SharedKernel/Identifiers/FixedGuidFactory.cs index 335e789..c01e387 100644 --- a/backend/src/LearnStack.SharedKernel/Identifiers/FixedGuidFactory.cs +++ b/backend/src/LearnStack.SharedKernel/Identifiers/FixedGuidFactory.cs @@ -6,6 +6,17 @@ namespace LearnStack.SharedKernel.Identifiers; /// the sequence is exhausted so tests fail loud rather than silently /// reusing a default value. /// +/// +/// and draw from the +/// same queue — the fixture does not synthesise version-7 / -4 +/// shapes from the supplied s. Callers that need to +/// assert guid.Version == 7 (or 4) seed the queue with +/// version-appropriate values (e.g. Guid.CreateVersion7() minted +/// at test setup) so the fixture's output is the exact +/// the test passes in. This keeps the fixture trivial; the production +/// is where the version contract is +/// enforced. +/// public sealed class FixedGuidFactory : IGuidFactory { private readonly Queue _sequence; diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/IAggregateRoot.cs b/backend/src/LearnStack.SharedKernel/Identifiers/IAggregateRoot.cs index 0a9c341..e4e1042 100644 --- a/backend/src/LearnStack.SharedKernel/Identifiers/IAggregateRoot.cs +++ b/backend/src/LearnStack.SharedKernel/Identifiers/IAggregateRoot.cs @@ -14,15 +14,3 @@ public interface IAggregateRoot : IHasId where TId : struct, IStronglyTypedId { } - -/// -/// Tagging interface for any entity that exposes an identifier of type -/// . Kept separate from -/// so child entities inside an aggregate -/// can share the Id shape without claiming aggregate-root status. -/// -public interface IHasId - where TId : struct, IStronglyTypedId -{ - TId Id { get; } -} diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/IHasId.cs b/backend/src/LearnStack.SharedKernel/Identifiers/IHasId.cs new file mode 100644 index 0000000..24da32b --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Identifiers/IHasId.cs @@ -0,0 +1,13 @@ +namespace LearnStack.SharedKernel.Identifiers; + +/// +/// Tagging interface for any entity that exposes an identifier of type +/// . Kept separate from +/// so child entities inside an aggregate +/// can share the Id shape without claiming aggregate-root status. +/// +public interface IHasId + where TId : struct, IStronglyTypedId +{ + TId Id { get; } +} diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/UserId.cs b/backend/src/LearnStack.SharedKernel/Identifiers/UserId.cs new file mode 100644 index 0000000..f947b05 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Identifiers/UserId.cs @@ -0,0 +1,17 @@ +using Vogen; + +namespace LearnStack.SharedKernel.Identifiers; + +/// +/// Cross-cutting strongly-typed identifier for the platform-wide user. +/// Lives in (per ADR-0023's +/// cross-cutting value-object placement) so audit metadata and other +/// kernel surfaces can reference users without depending on the Identity +/// module that lands in Phase 02b. The Identity module consumes the same +/// type once it ships; there is exactly one UserId shape. +/// +[ValueObject(LearnStackVogenDefaults.IdMask)] +public readonly partial record struct UserId : IStronglyTypedId +{ + public static UserId New() => From(Guid.CreateVersion7()); +} diff --git a/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs b/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs index b4a67ed..bc365bb 100644 --- a/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs +++ b/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs @@ -7,11 +7,21 @@ namespace LearnStack.SharedKernel.Localization; /// resolves to a translation on the client. /// /// +/// /// The lockey_ prefix invariant is enforced at the constructor: /// every key must match the format the frontend's /// i18n bundles ship under. Mis-prefixed keys fail loud at the /// point of construction rather than silently resolving to "missing /// translation" at render time. Per Phase 02a Packet 2. +/// +/// +/// values are interpolated by the frontend as plain +/// text (React text nodes, ICU MessageFormat substitution). Backend code +/// must not place HTML / Markdown / template syntax into +/// these values — the frontend resolves messages via text nodes, never +/// dangerouslySetInnerHTML. Standards 11 § XSS covers the frontend +/// side; this contract closes the backend side. +/// /// public sealed record LocalizedMessage { @@ -21,8 +31,6 @@ public sealed record LocalizedMessage /// public const string RequiredPrefix = "lockey_"; - private readonly IReadOnlyDictionary? _params; - public LocalizedMessage(string key, IReadOnlyDictionary? @params = null) { ArgumentException.ThrowIfNullOrWhiteSpace(key); @@ -35,21 +43,29 @@ public LocalizedMessage(string key, IReadOnlyDictionary? @params } Key = key; - _params = @params; + // Defensive copy: the caller may mutate the original dictionary after + // construction; a LocalizedMessage is supposed to be immutable, so + // snapshot the contents here. The empty case is normalised to null so + // serializers do not emit an unused "params": {} field. + Params = @params is { Count: > 0 } + ? new Dictionary(@params) + : null; } /// /// The localization key (always begins with ). - /// Routing logic and error-tracking provider tags use this verbatim. + /// Routing logic and error-tracking provider tags use this verbatim; + /// Error.Code projects from this key with the prefix stripped. /// public string Key { get; } /// /// Optional ICU MessageFormat parameter set. Frontend interpolates - /// these into the resolved string. null when the message takes + /// these into the resolved string as plain text (see + /// remarks for the safety contract). null when the message takes /// no parameters. /// - public IReadOnlyDictionary? Params => _params; + public IReadOnlyDictionary? Params { get; } /// /// Convenience factory equivalent to new LocalizedMessage(key, params). @@ -58,4 +74,69 @@ public static LocalizedMessage Of( string key, IReadOnlyDictionary? @params = null) => new(key, @params); + + // Structural equality. The default record equality compares Params by + // reference (IReadOnlyDictionary has no structural equality contract); + // after the defensive-copy invariant two LocalizedMessages constructed + // from the same source dictionary hold separate copies and would no + // longer be equal. Override Equals + GetHashCode to compare the params + // key-by-key so equality matches the semantic contract. + public bool Equals(LocalizedMessage? other) + { + if (other is null) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + if (!string.Equals(Key, other.Key, StringComparison.Ordinal)) + { + return false; + } + + if (Params is null && other.Params is null) + { + return true; + } + + if (Params is null || other.Params is null || Params.Count != other.Params.Count) + { + return false; + } + + foreach (var (k, v) in Params) + { + if (!other.Params.TryGetValue(k, out var otherValue) || + !string.Equals(v, otherValue, StringComparison.Ordinal)) + { + return false; + } + } + + return true; + } + + public override int GetHashCode() + { + var hash = new HashCode(); + hash.Add(Key, StringComparer.Ordinal); + if (Params is not null) + { + // Order-independent: XOR each entry's hash so dictionary iteration + // order does not affect the bucket. + var paramsHash = 0; + foreach (var (k, v) in Params) + { + paramsHash ^= HashCode.Combine(k, v); + } + + hash.Add(paramsHash); + } + + return hash.ToHashCode(); + } } diff --git a/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs b/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs index 7b0dd2a..257d718 100644 --- a/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs +++ b/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs @@ -7,18 +7,22 @@ namespace LearnStack.SharedKernel.Pagination; /// it. defaults to and is /// bounded by . /// -public sealed record CursorPagination(string? Cursor = null, int Limit = 20) +/// +/// Construction is the validation point: a non-positive +/// is a programmer error (kernel-level guard) and throws. API-layer +/// FluentValidation should turn malformed user input into +/// Result.Fail(validation_failed) before the +/// kernel sees the request, so the throw here only fires on coding bugs. +/// clamps above-max limits to +/// ; it no longer throws. +/// +public sealed record CursorPagination { public const int DefaultLimit = 20; public const int MaxLimit = 100; - /// - /// Validates the request against the standard bounds. Returns a - /// normalised request with clamped to - /// ; throws when the limit is non-positive. - /// - public CursorPagination Normalised() + public CursorPagination(string? Cursor = null, int Limit = DefaultLimit) { if (Limit <= 0) { @@ -28,6 +32,20 @@ public CursorPagination Normalised() $"Limit must be > 0. Got: {Limit}."); } - return Limit > MaxLimit ? this with { Limit = MaxLimit } : this; + this.Cursor = Cursor; + this.Limit = Limit; } + + public string? Cursor { get; init; } + + public int Limit { get; init; } + + /// + /// Returns the request with clamped to + /// . Non-throwing: invalid inputs are stopped at + /// construction, so the only normalisation left is the upper-bound + /// clamp. + /// + public CursorPagination Normalised() => + Limit > MaxLimit ? this with { Limit = MaxLimit } : this; } diff --git a/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs b/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs index fd4dbff..c4d8f5c 100644 --- a/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs +++ b/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs @@ -12,9 +12,4 @@ public interface ISoftDelete DateTimeOffset? DeletedAt { get; } Guid? DeletedBy { get; } - - /// - /// Convenience projection of . - /// - bool IsDeleted => DeletedAt.HasValue; } diff --git a/backend/src/LearnStack.SharedKernel/Results/Error.cs b/backend/src/LearnStack.SharedKernel/Results/Error.cs index aaeed8a..ca09d7c 100644 --- a/backend/src/LearnStack.SharedKernel/Results/Error.cs +++ b/backend/src/LearnStack.SharedKernel/Results/Error.cs @@ -4,18 +4,28 @@ namespace LearnStack.SharedKernel.Results; /// -/// Result-pattern error payload used by . The -/// carries the lockey_ localization key the -/// frontend resolves; is a convenience projection of -/// Message.Key for routing logic (ADR-0032 § Sub-decision 6's -/// ResultExtensions.ToActionResult() matches on it). +/// Result-pattern error payload used by . +/// is the localised payload the frontend resolves; +/// is the stable machine-readable identifier API +/// consumers route on (Standards 04 § Problem Details and Standards 09 +/// § Forbidden — codes do not get localized, only resolved messages do). /// /// +/// +/// is derived from Message.Key by stripping the +/// invariant : a key of +/// "lockey_validation_failed" projects as "validation_failed". +/// This keeps the two contracts in sync by construction — there is no way +/// to ship an whose Code disagrees with the +/// localization key the frontend resolves. +/// +/// /// CA1716 (avoid reserved language keywords as type names) is intentionally /// suppressed: the project's Result+Error pattern follows the FluentResults / /// Ardalis.Result lineage where the type is canonically named Error. /// LearnStack is C#-only — there is no VB consumer to which the "Error" /// keyword collision would surface. Per ADR-0032 § Error Model. +/// /// [SuppressMessage( "Naming", @@ -23,13 +33,14 @@ namespace LearnStack.SharedKernel.Results; Justification = "Result+Error pattern — C#-only codebase per ADR-0032; no VB consumer affected.")] public sealed record Error( LocalizedMessage Message, - IReadOnlyDictionary? Details = null) + IReadOnlyDictionary>? Details = null) { /// - /// Localization-key projection of . Equivalent to - /// Message.Key; surfaced as a top-level property so - /// Result.ToActionResult() and Problem Details writers can read - /// it without dereferencing the nested record. + /// Stable machine-readable code derived from 's + /// Key with the + /// stripped. Used by Result.ToActionResult() and Problem Details + /// writers as the RFC 7807 code field — never localized, never + /// changes per locale (Standards 09 § Forbidden). /// - public string Code => Message.Key; + public string Code => Message.Key[LocalizedMessage.RequiredPrefix.Length..]; } diff --git a/backend/src/LearnStack.SharedKernel/Results/IResultBase.cs b/backend/src/LearnStack.SharedKernel/Results/IResultBase.cs new file mode 100644 index 0000000..26a0d16 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Results/IResultBase.cs @@ -0,0 +1,30 @@ +using System.Diagnostics.CodeAnalysis; +using LearnStack.SharedKernel.Localization; + +namespace LearnStack.SharedKernel.Results; + +/// +/// Non-generic surface every exposes. Pipeline +/// behaviors operate on this contract so they can construct the correct +/// concrete Result<TResponse> shape via the +/// factory (ADR-0032 § Sub-decision 3). +/// +/// +/// CA1716 (Error collides with VB's reserved keyword) is suppressed +/// for the same reason it is suppressed on the record +/// itself — LearnStack is C#-only per ADR-0032. +/// +[SuppressMessage( + "Naming", + "CA1716:Identifiers should not match keywords", + Justification = "Result+Error pattern — C#-only codebase per ADR-0032; no VB consumer affected.")] +public interface IResultBase +{ + bool IsSuccess { get; } + + bool IsFailure { get; } + + LocalizedMessage? SuccessMessage { get; } + + Error? Error { get; } +} diff --git a/backend/src/LearnStack.SharedKernel/Results/Result.Helpers.cs b/backend/src/LearnStack.SharedKernel/Results/Result.Helpers.cs new file mode 100644 index 0000000..1fbb2e2 --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Results/Result.Helpers.cs @@ -0,0 +1,59 @@ +using System.Reflection; +using LearnStack.SharedKernel.Localization; + +namespace LearnStack.SharedKernel.Results; + +/// +/// Static helpers for constructing instances when +/// the concrete generic parameter is known only at runtime (e.g. inside +/// the MediatR ValidationBehavior / AuthorizationBehavior +/// that short-circuit a handler without referencing the value type). +/// Per ADR-0032 § Sub-decision 3. +/// +public static class Result +{ + public static Result Ok(T value, LocalizedMessage? message = null) => + Result.Ok(value, message); + + public static Result Fail(Error error) => Result.Fail(error); + + /// + /// Reflection-friendly failure factory used by MediatR pipeline + /// behaviors whose TResponse is itself a Result<TValue>. + /// Returns an instance of — not + /// Result<TResponse> — by reflecting over the closed + /// generic to invoke the correct Result<TValue>.Fail(error). + /// Per ADR-0032 § Sub-decision 3. + /// + public static TResponse FailFor(Error error) + where TResponse : IResultBase + { + ArgumentNullException.ThrowIfNull(error); + + var responseType = typeof(TResponse); + if (!responseType.IsGenericType + || responseType.GetGenericTypeDefinition() != typeof(Result<>)) + { + throw new InvalidOperationException( + $"Result.FailFor requires TResponse to be a closed " + + $"Result; got {responseType.FullName}."); + } + + var valueType = responseType.GetGenericArguments()[0]; + var concreteResultType = typeof(Result<>).MakeGenericType(valueType); + var failMethod = concreteResultType.GetMethod( + nameof(Result.Fail), + BindingFlags.Public | BindingFlags.Static, + binder: null, + types: [typeof(Error)], + modifiers: null); + + if (failMethod is null) + { + throw new InvalidOperationException( + $"Could not locate {concreteResultType.FullName}.Fail(Error) via reflection."); + } + + return (TResponse)failMethod.Invoke(obj: null, parameters: [error])!; + } +} diff --git a/backend/src/LearnStack.SharedKernel/Results/Result.cs b/backend/src/LearnStack.SharedKernel/Results/Result.cs index 84e8970..dc8c907 100644 --- a/backend/src/LearnStack.SharedKernel/Results/Result.cs +++ b/backend/src/LearnStack.SharedKernel/Results/Result.cs @@ -3,56 +3,12 @@ namespace LearnStack.SharedKernel.Results; -/// -/// Non-generic surface every exposes. Pipeline -/// behaviors operate on this contract so they can construct the correct -/// concrete Result<TResponse> shape via the -/// factory (ADR-0032 § Sub-decision 3). -/// -/// -/// CA1716 (Error collides with VB's reserved keyword) is suppressed -/// for the same reason it is suppressed on the record -/// itself — LearnStack is C#-only per ADR-0032. -/// -[SuppressMessage( - "Naming", - "CA1716:Identifiers should not match keywords", - Justification = "Result+Error pattern — C#-only codebase per ADR-0032; no VB consumer affected.")] -public interface IResultBase -{ - bool IsSuccess { get; } - - bool IsFailure { get; } - - LocalizedMessage? SuccessMessage { get; } - - Error? Error { get; } -} - -/// -/// Static helpers for constructing generically -/// when the type parameter is known only at runtime (e.g. inside -/// ValidationBehavior). Per ADR-0032 § Sub-decision 3. -/// -public static class Result -{ - public static Result Ok(T value, LocalizedMessage? message = null) => - Result.Ok(value, message); - - public static Result Fail(Error error) => Result.Fail(error); - - /// - /// Reflection-friendly failure factory used by MediatR pipeline behaviors - /// (ValidationBehavior, AuthorizationBehavior) that - /// short-circuit a generic handler without knowing T at compile - /// time. Per ADR-0032 § Sub-decision 3. - /// - public static Result FailFor(Error error) => Result.Fail(error); -} - /// /// Result-pattern wrapper. Returned by every MediatR command/query handler -/// per ADR-0032 § Error Model. +/// per ADR-0032 § Error Model. Construction is funnelled through the +/// / factories; the primary +/// constructor is internal so callers cannot bypass the success- +/// must-carry-value rule via positional record syntax. /// /// /// CA1000 (do not declare static members on generic types) is intentionally @@ -68,16 +24,47 @@ public static Result Ok(T value, LocalizedMessage? message = null) => "Design", "CA1000:Do not declare static members on generic types", Justification = "Result+Error factory pattern per ADR-0032 — canonical shape across FluentResults / Ardalis.Result lineage.")] -public sealed record Result( - bool IsSuccess, - T? Value, - Error? Error, - LocalizedMessage? SuccessMessage = null) : IResultBase +public sealed record Result : IResultBase { + internal Result(bool isSuccess, T? value, Error? error, LocalizedMessage? successMessage = null) + { + IsSuccess = isSuccess; + Value = value; + Error = error; + SuccessMessage = successMessage; + } + + public bool IsSuccess { get; } + public bool IsFailure => !IsSuccess; - public static Result Ok(T value, LocalizedMessage? message = null) => - new(true, value, null, message); + public T? Value { get; } + + public Error? Error { get; } + + public LocalizedMessage? SuccessMessage { get; } + + /// + /// Constructs a success result. Throws when is + /// null: Standards 09 § Forbidden bans IsSuccess = true + /// with Value = null — if a payload-less success shape is needed, + /// model it as Result<Unit>. + /// + public static Result Ok(T value, LocalizedMessage? message = null) + { + if (value is null) + { + throw new ArgumentNullException( + nameof(value), + "Result.Ok cannot wrap a null value. Use Result for payload-less success per Standards 09 § Forbidden."); + } + + return new Result(isSuccess: true, value: value, error: null, successMessage: message); + } - public static Result Fail(Error error) => new(false, default, error); + public static Result Fail(Error error) + { + ArgumentNullException.ThrowIfNull(error); + return new Result(isSuccess: false, value: default, error: error); + } } diff --git a/backend/src/LearnStack.SharedKernel/Results/Unit.cs b/backend/src/LearnStack.SharedKernel/Results/Unit.cs new file mode 100644 index 0000000..a89e87a --- /dev/null +++ b/backend/src/LearnStack.SharedKernel/Results/Unit.cs @@ -0,0 +1,12 @@ +namespace LearnStack.SharedKernel.Results; + +/// +/// Empty value used as the payload of Result<Unit> when a +/// command/query succeeds without returning data. Use this rather than +/// Result<object?> with a null value (Standards 09 § Forbidden +/// bans the latter). +/// +public readonly record struct Unit +{ + public static Unit Value { get; } +} diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs index 4271a91..9e98de2 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs @@ -1,4 +1,6 @@ using FluentAssertions; +using LearnStack.SharedKernel.Identifiers; +using LearnStack.SharedKernel.Persistence; using Xunit; namespace LearnStack.Tests.Unit.SharedKernel.Domain; @@ -8,7 +10,8 @@ public sealed class AuditableEntityTests private static readonly DateTimeOffset T0 = new(2026, 05, 21, 10, 00, 00, TimeSpan.Zero); - private static readonly Guid Actor = Guid.Parse("019712ac-aaaa-7000-8000-000000000aaa"); + private static readonly UserId Actor = + UserId.From(Guid.Parse("019712ac-aaaa-7000-8000-000000000aaa")); [Fact] public void MarkCreated_SetsCreatedAtAndCreatedBy() @@ -24,12 +27,23 @@ public void MarkCreated_SetsCreatedAtAndCreatedBy() aggregate.IsDeleted.Should().BeFalse(); } + [Fact] + public void MarkCreated_CalledTwice_Throws() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + aggregate.MarkCreated(T0, Actor); + + var act = () => aggregate.MarkCreated(T0.AddHours(1), Actor); + + act.Should().Throw().WithMessage("*already*"); + } + [Fact] public void MarkUpdated_SetsUpdatedAtAndUpdatedBy() { var aggregate = new TestAuditableAggregate(TestId.New()); aggregate.MarkCreated(T0, Actor); - var laterActor = Guid.Parse("019712ac-bbbb-7000-8000-000000000bbb"); + var laterActor = UserId.From(Guid.Parse("019712ac-bbbb-7000-8000-000000000bbb")); var t1 = T0.AddHours(1); aggregate.MarkUpdated(t1, laterActor); @@ -40,7 +54,7 @@ public void MarkUpdated_SetsUpdatedAtAndUpdatedBy() } [Fact] - public void SoftDelete_SetsDeletedAtDeletedByAndIsDeleted() + public void SoftDelete_SetsDeletedColumns_AndAlsoBumpsUpdated() { var aggregate = new TestAuditableAggregate(TestId.New()); aggregate.MarkCreated(T0, Actor); @@ -51,6 +65,20 @@ public void SoftDelete_SetsDeletedAtDeletedByAndIsDeleted() aggregate.DeletedAt.Should().Be(t1); aggregate.DeletedBy.Should().Be(Actor); aggregate.IsDeleted.Should().BeTrue(); + aggregate.UpdatedAt.Should().Be(t1, "SoftDelete bumps UpdatedAt for monotonic last-touched"); + aggregate.UpdatedBy.Should().Be(Actor); + } + + [Fact] + public void ISoftDelete_DeletedBy_ProjectsGuidFromUserId() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + aggregate.MarkCreated(T0, Actor); + aggregate.SoftDelete(T0.AddDays(1), Actor); + + ISoftDelete view = aggregate; + + view.DeletedBy.Should().Be(Actor.Value); } [Fact] diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/DomainEventTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/DomainEventTests.cs index 7575186..457783e 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/DomainEventTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/DomainEventTests.cs @@ -7,6 +7,9 @@ namespace LearnStack.Tests.Unit.SharedKernel.Domain; public sealed class DomainEventTests { + private static readonly DateTimeOffset T0 = + new(2026, 01, 01, 0, 0, 0, TimeSpan.Zero); + [Fact] public void IDomainEvent_IsAMediatRNotification() { @@ -15,30 +18,17 @@ public void IDomainEvent_IsAMediatRNotification() } [Fact] - public void DefaultEventId_IsUuidV7() - { - var @event = new TestDomainEvent("x"); - - @event.EventId.Version.Should().Be(7); - } - - [Fact] - public void DefaultOccurredAt_IsRecent() - { - var before = DateTimeOffset.UtcNow; - - var @event = new TestDomainEvent("x"); - - @event.OccurredAt.Should().BeOnOrAfter(before).And.BeOnOrBefore(DateTimeOffset.UtcNow); - } - - [Fact] - public void RecordInit_AllowsTimeOverride() + public void Stamped_Event_CarriesTheSuppliedEventIdAndOccurredAt() { - var fixedAt = new DateTimeOffset(2026, 01, 01, 0, 0, 0, TimeSpan.Zero); + var eventId = Guid.CreateVersion7(); - var @event = new TestDomainEvent("x") { OccurredAt = fixedAt }; + var @event = new TestDomainEvent("payload") + { + EventId = eventId, + OccurredAt = T0, + }; - @event.OccurredAt.Should().Be(fixedAt); + @event.EventId.Should().Be(eventId); + @event.OccurredAt.Should().Be(T0); } } diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/EntityTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/EntityTests.cs index 0733052..17271c2 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/EntityTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/EntityTests.cs @@ -7,7 +7,7 @@ namespace LearnStack.Tests.Unit.SharedKernel.Domain; public sealed class EntityTests { [Fact] - public void Equality_IsIdentityBased() + public void Equality_IsIdentityBased_ForPersistedIds() { var id = TestId.New(); var a = new TestAggregate(id); @@ -26,11 +26,49 @@ public void Equality_AcrossDifferentIds_IsFalse() a.Equals(b).Should().BeFalse(); } + [Fact] + public void Equality_TwoTransientEntities_AreNeverEqual() + { + // Both default-id ("not yet persisted") — must not collapse in the + // change tracker / HashSet-based collection navigations. + // The hash falls back to reference identity (object.GetHashCode); + // we only assert the Equals contract because RuntimeHelpers identity + // hashes are not guaranteed-distinct (collision is rare but legal). + var a = new TestAggregate(); + var b = new TestAggregate(); + + a.Equals(b).Should().BeFalse(); + } + + [Fact] + public void Equality_TransientEntityEqualsItself_ByReference() + { + var a = new TestAggregate(); + + a.Equals(a).Should().BeTrue(); + } + + [Fact] + public void Equality_DifferentRuntimeType_SameId_IsFalse() + { + // Two entity types that share Entity and the same Id value + // must still compare false — they describe different aggregates. + var id = TestId.New(); + Entity a = new TestAggregate(id); + Entity b = new TestAggregateSibling(id); + + a.Equals(b).Should().BeFalse(); + } + [Fact] public void RaiseDomainEvent_AppendsToCollection() { var aggregate = new TestAggregate(TestId.New()); - IDomainEvent @event = new TestDomainEvent("hello"); + IDomainEvent @event = new TestDomainEvent("hello") + { + EventId = Guid.CreateVersion7(), + OccurredAt = DateTimeOffset.UtcNow, + }; aggregate.Raise(@event); @@ -41,8 +79,8 @@ public void RaiseDomainEvent_AppendsToCollection() public void ClearDomainEvents_EmptiesTheCollection() { var aggregate = new TestAggregate(TestId.New()); - aggregate.Raise(new TestDomainEvent("a")); - aggregate.Raise(new TestDomainEvent("b")); + aggregate.Raise(new TestDomainEvent("a") { EventId = Guid.CreateVersion7(), OccurredAt = DateTimeOffset.UtcNow }); + aggregate.Raise(new TestDomainEvent("b") { EventId = Guid.CreateVersion7(), OccurredAt = DateTimeOffset.UtcNow }); aggregate.ClearDomainEvents(); diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestAggregate.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestAggregate.cs index 71efead..0d3075b 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestAggregate.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestAggregate.cs @@ -16,6 +16,15 @@ public TestAggregate() { } public void Raise(IDomainEvent domainEvent) => RaiseDomainEvent(domainEvent); } +/// +/// Second test entity type — same TId, different runtime type. Used to +/// verify Entity<TId>'s cross-runtime-type equality guard. +/// +internal sealed class TestAggregateSibling : Entity +{ + public TestAggregateSibling(TestId id) : base(id) { } +} + /// /// Test double for . /// diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs index 3180953..fed2ca1 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs @@ -8,11 +8,20 @@ namespace LearnStack.Tests.Unit.SharedKernel; public sealed class ErrorTests { [Fact] - public void Code_DerivesFromMessageKey() + public void Code_StripsLockeyPrefix_FromMessageKey() { var error = new Error(LocalizedMessage.Of("lockey_forbidden")); - error.Code.Should().Be("lockey_forbidden"); + error.Code.Should().Be("forbidden", + "Standards 04 § Problem Details Code is the unprefixed stable identifier"); + } + + [Fact] + public void Code_ForValidationFailed_MatchesStandards09Catalogue() + { + var error = new Error(LocalizedMessage.Of("lockey_validation_failed")); + + error.Code.Should().Be("validation_failed"); } [Fact] @@ -24,11 +33,17 @@ public void Details_AreOptional() } [Fact] - public void Details_RetainFieldErrors() + public void Details_CarryFieldLevelLocalizedMessages() { - var details = new Dictionary + // Per the review: field-level errors must also flow as LocalizedMessages + // so the lockey_ invariant covers every user-facing string the API ships. + var details = new Dictionary> { - ["email"] = ["lockey_email_invalid", "lockey_email_required"], + ["email"] = + [ + LocalizedMessage.Of("lockey_email_required"), + LocalizedMessage.Of("lockey_email_invalid"), + ], }; var error = new Error( @@ -36,5 +51,6 @@ public void Details_RetainFieldErrors() details); error.Details.Should().BeEquivalentTo(details); + error.Details!["email"].Should().HaveCount(2); } } diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Localization/LocalizedMessageTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Localization/LocalizedMessageTests.cs index f9f1bc7..3c1c83e 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Localization/LocalizedMessageTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Localization/LocalizedMessageTests.cs @@ -25,6 +25,28 @@ public void Ctor_WithParams_RetainsThem() message.Params.Should().BeEquivalentTo(@params); } + [Fact] + public void Ctor_DefensivelyCopiesParams_SoCallerMutationsDoNotLeak() + { + var @params = new Dictionary { ["field"] = "email" }; + + var message = new LocalizedMessage("lockey_validation_required", @params); + @params["field"] = "MUTATED"; + + message.Params!["field"].Should().Be("email", "LocalizedMessage is immutable; the snapshot was taken at ctor time"); + } + + [Fact] + public void Ctor_EmptyParamsDictionary_NormalisesToNull() + { + // Avoids serializers emitting an unused "params": {} field. + var message = new LocalizedMessage( + "lockey_no_params", + new Dictionary()); + + message.Params.Should().BeNull(); + } + [Theory] [InlineData("validation_failed")] [InlineData("lockEY_wrong_case")] diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs index d30f310..f4ffbbb 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs @@ -14,6 +14,18 @@ public void DefaultLimit_MatchesStandards04() CursorPagination.MaxLimit.Should().Be(100); } + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void Ctor_LimitNonPositive_Throws(int limit) + { + // The kernel-level guard belongs at construction so the invariant + // cannot be skipped by callers that forget to call Normalised(). + var act = () => new CursorPagination(Limit: limit); + + act.Should().Throw(); + } + [Fact] public void Normalised_LimitAboveMax_ClampsToMaxLimit() { @@ -23,22 +35,26 @@ public void Normalised_LimitAboveMax_ClampsToMaxLimit() } [Fact] - public void Normalised_LimitWithinBounds_ReturnsSameInstance() + public void Normalised_LimitWithinBounds_ReturnsEquivalentInstance() { var request = new CursorPagination(Cursor: "abc", Limit: 50); - request.Normalised().Should().Be(request); + var normalised = request.Normalised(); + + normalised.Cursor.Should().Be(request.Cursor); + normalised.Limit.Should().Be(request.Limit); } - [Theory] - [InlineData(0)] - [InlineData(-1)] - public void Normalised_LimitNonPositive_Throws(int limit) + [Fact] + public void Normalised_NeverThrows_BecauseCtorValidatesInput() { - var request = new CursorPagination(Limit: limit); + // After ctor validation, the only thing left to normalise is the + // upper-bound clamp. Calling Normalised() on any constructed + // instance is safe. + var request = new CursorPagination(Limit: CursorPagination.MaxLimit + 1); var act = () => request.Normalised(); - act.Should().Throw(); + act.Should().NotThrow(); } } diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/ResultTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ResultTests.cs index af89d6d..2773162 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/ResultTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ResultTests.cs @@ -2,6 +2,7 @@ using LearnStack.SharedKernel.Localization; using LearnStack.SharedKernel.Results; using Xunit; +using ResultUnit = LearnStack.SharedKernel.Results.Unit; namespace LearnStack.Tests.Unit.SharedKernel; @@ -19,6 +20,26 @@ public void Ok_WhenGivenValue_ReturnsSuccessResult() result.SuccessMessage.Should().BeNull(); } + [Fact] + public void Ok_WithNullReferenceValue_Throws() + { + // Standards 09 § Forbidden bans Result with IsSuccess=true and + // Value=null; the factory must reject this at the call site. + var act = () => Result.Ok(null!); + + act.Should().Throw() + .WithMessage("*Result*"); + } + + [Fact] + public void Ok_WithUnit_IsTheCanonicalPayloadlessSuccess() + { + var result = Result.Ok(ResultUnit.Value); + + result.IsSuccess.Should().BeTrue(); + result.Value.Should().Be(ResultUnit.Value); + } + [Fact] public void Ok_WithSuccessMessage_RetainsIt() { @@ -43,17 +64,32 @@ public void Fail_WhenGivenError_ReturnsFailureResult() } [Fact] - public void FailFor_ConstructsResultGenericallyForPipelineBehaviors() + public void FailFor_TResponseIsResultOfT_ReturnsConcreteResult() { + // ADR-0032 § Sub-decision 3 shape: TResponse is Result; the + // factory MUST return Result, NOT Result>. var error = new Error(LocalizedMessage.Of("lockey_validation_failed")); - var result = Result.FailFor(error); + var result = Result.FailFor>(error); result.Should().BeOfType>(); result.IsFailure.Should().BeTrue(); result.Error.Should().Be(error); } + [Fact] + public void FailFor_NonResultGeneric_Throws() + { + // FailFor is a pipeline helper; calling it with a non-Result + // type parameter is a coding bug and must fail loud rather than + // silently producing an unexpected shape. + var error = new Error(LocalizedMessage.Of("lockey_business_rule_violation")); + + var act = () => Result.FailFor(error); + + act.Should().Throw().WithMessage("*Result*"); + } + [Fact] public void IResultBase_IsImplementedByResult() { @@ -68,4 +104,14 @@ public void IResultBase_IsImplementedByResult() result.IsSuccess.Should().BeTrue(); result.Error.Should().BeNull(); } + + // Helper type used by FailFor_NonResultGeneric_Throws. + private sealed record FakeResult(bool IsSuccess = true) : IResultBase + { + public bool IsFailure => !IsSuccess; + + public LocalizedMessage? SuccessMessage => null; + + public Error? Error => null; + } } From a1ad5fb4e0cf64cc50d628cae0140e5ffc6182a5 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Thu, 21 May 2026 13:32:23 +0300 Subject: [PATCH 4/8] =?UTF-8?q?docs(standards,decisions):=20packet=202=20r?= =?UTF-8?q?eview=20follow-up=20=E2=80=94=20corpus=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the corpus-side decisions that the code-side fixes (commit 7c9133a) needed to land cleanly. Pure documentation; no code change. - Standards 01 § Dependency Direction grows a "Build-time-only exceptions" sub-section spelling out the two sanctioned SharedKernel external references (Microsoft.EntityFrameworkCore via the Vogen-emitted EfCoreValueConverter, MediatR via IDomainEvent : INotification) with the ADR they trace to. Adding a third such reference now explicitly requires a new ADR. Names the follow-up architecture test that encodes the rule alongside the first Module.Domain aggregate in Packet 6. - ADR-0023 Implementation Notes literal pin: the placeholder Version="..." is replaced with the concrete Version="7.0.0" already recorded in Amendment 1; the SharedKernel cross-cutting value-object list gains UserId; the EF Core transitive-reference rule is cross-linked to Standards 01. - Standards 02 + 09 Result Type snippets reflect the refactored shape: internal primary constructor, Ok throws on null value, Error.Details typed as IReadOnlyDictionary>, Error.Code projects from Message.Key by stripping the lockey_ prefix, Result is the canonical payload-less success. The Standards 09 error-code table prose is updated to the "table omits the prefix; wire format is the prefixed key, Code is the unprefixed stable identifier" shape that satisfies both Standards 04 § Problem Details and Standards 09 § Forbidden. - Glossary entries refreshed: Result (internal ctor, Unit), Error (Code stripping, Details with LocalizedMessage lists), LocalizedMessage (structural equality, defensive copy, plain-text param contract), Entity / AuditableEntity (transient + cross-type equality guards, MarkCreated throws, SoftDelete bumps UpdatedAt), IDomainEvent (required init), CursorPagination (ctor guard); new UserId and Unit entries. - Phase 02a Packet 2 status block records the review pass folded in, pointing to commit 7c9133a. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...0023-strongly-typed-id-source-generator.md | 2 +- docs/glossary.md | 14 +++--- docs/roadmap/phase-02a-kernel-tenancy.md | 14 ++++++ docs/standards/01-architecture-standards.md | 20 ++++++++ docs/standards/02-backend-coding.md | 33 +++++++++---- docs/standards/09-error-handling.md | 49 ++++++++++++------- 6 files changed, 98 insertions(+), 34 deletions(-) diff --git a/docs/decisions/0023-strongly-typed-id-source-generator.md b/docs/decisions/0023-strongly-typed-id-source-generator.md index 406f587..c925abf 100644 --- a/docs/decisions/0023-strongly-typed-id-source-generator.md +++ b/docs/decisions/0023-strongly-typed-id-source-generator.md @@ -200,7 +200,7 @@ Three things outweighed the appeal: ## Implementation Notes -- **Package references:** `Directory.Packages.props` pins ``. **Every project that hosts `[ValueObject<>]` declarations** — `LearnStack.SharedKernel` (for cross-cutting value objects: `Email`, `Slug`, `LocaleCode`, `Money`) and **each `LearnStack.Modules..Domain`** (for its aggregate-root IDs) — adds ``. A `Directory.Build.props` rule under `backend/src/Modules/` keeps the per-module addition automatic when a new module is scaffolded. Source generators only run on projects that reference the generator package; transitive references do **not** carry the generator (this is a `PrivateAssets="all"` semantics constraint, not a Vogen quirk). +- **Package references:** `Directory.Packages.props` pins `` (see Amendment 1 below for the rationale of the 6.x → 7.0.0 drift). **Every project that hosts `[ValueObject<>]` declarations** — `LearnStack.SharedKernel` (for cross-cutting value objects: `UserId`, `Email`, `Slug`, `LocaleCode`, `Money`) and **each `LearnStack.Modules..Domain`** (for its aggregate-root IDs) — adds `` plus a transitive `Microsoft.EntityFrameworkCore` reference (the Vogen-emitted EF converter requires it at compile time; the build-time exception is recorded in [Standards 01 § Dependency Direction](../standards/01-architecture-standards.md)). A `Directory.Build.props` rule under `backend/src/Modules/` keeps the per-module addition automatic when a new module is scaffolded. Source generators only run on projects that reference the generator package; transitive references do **not** carry the generator (this is a `PrivateAssets="all"` semantics constraint, not a Vogen quirk). - **Naming convention (per Standards 02):** ID type names end in `Id` (`TenantId`, `OrganizationId`, `CourseId`, …); value object types are named for the concept (`Email`, not `EmailValueObject`). diff --git a/docs/glossary.md b/docs/glossary.md index 508a835..8a0d051 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -245,13 +245,15 @@ This glossary defines LearnStack-specific terms. When a term is ambiguous across | Term | Definition | |------|------------| -| **`Result`** | The sealed record in `LearnStack.SharedKernel.Results` implementing `IResultBase`: carries `IsSuccess`, `Value`, optional `Error`, and an optional `SuccessMessage` (also a `LocalizedMessage`). Application + Domain layer methods return `Result` for **expected** outcomes (validation failed, not found, forbidden, business-rule violation). Exceptions are reserved for **unexpected** bugs / infrastructure faults. See [09-error-handling.md § Two-Track Model](standards/09-error-handling.md) and [ADR-0032](decisions/0032-exception-handling-logging-and-observability.md). | -| **`Error`** | The sealed record `(LocalizedMessage Message, IReadOnlyDictionary? Details)` that travels inside `Result.Fail(error)`. `Code` is a convenience projection of `Message.Key` (always begins with `lockey_`); `Details` carries field-level errors. Routing logic (`Result.ToActionResult()`) reads `Code`. Refactored in Phase 02a Packet 2; see [09-error-handling.md § Result Type](standards/09-error-handling.md). | -| **`LocalizedMessage`** | The sealed record `(string Key, IReadOnlyDictionary? Params)` in `LearnStack.SharedKernel.Localization`. `Key` MUST begin with `lockey_` — enforced at the constructor — so the frontend's translation catalogues (keyed under the same prefix) resolve every user-facing message without raw English on the wire. Used by every `Result.Fail(...)` payload and every `Result.Ok(value, message: ...)` success notification. Per [Phase 02a Packet 2](roadmap/phase-02a-kernel-tenancy.md). | +| **`Result`** | The sealed record in `LearnStack.SharedKernel.Results` implementing `IResultBase`: carries `IsSuccess`, `Value`, optional `Error`, and an optional `SuccessMessage` (a `LocalizedMessage`). Primary constructor is `internal`; callers go through `Ok` (throws on null value) / `Fail`. Application + Domain layer methods return `Result` for **expected** outcomes (validation failed, not found, forbidden, business-rule violation). Exceptions are reserved for **unexpected** bugs / infrastructure faults. `Result` is the payload-less success shape. See [09-error-handling.md § Two-Track Model](standards/09-error-handling.md) and [ADR-0032](decisions/0032-exception-handling-logging-and-observability.md). | +| **`Error`** | The sealed record `(LocalizedMessage Message, IReadOnlyDictionary>? Details)` that travels inside `Result.Fail(error)`. `Code` is the **unprefixed stable identifier** (e.g. `"validation_failed"`) derived by stripping the `lockey_` prefix from `Message.Key` — so routing logic (`Result.ToActionResult()`, Problem Details writers) and frontend localization read consistent values without manual sync. Field-level `Details` flow as `LocalizedMessage` lists, keeping the prefix invariant uniform across the entire error payload. See [09-error-handling.md § Result Type](standards/09-error-handling.md). | +| **`LocalizedMessage`** | The sealed record `(string Key, IReadOnlyDictionary? Params)` in `LearnStack.SharedKernel.Localization`. `Key` MUST begin with `lockey_` — enforced at the constructor — so the frontend's translation catalogues (keyed under the same prefix) resolve every user-facing message without raw English on the wire. Equality is structural (Key + Params); ctor defensively copies Params; empty params normalised to null. `Params` values must be plain text — the frontend resolves messages via React text nodes, never `dangerouslySetInnerHTML`. Per [Phase 02a Packet 2](roadmap/phase-02a-kernel-tenancy.md). | +| **`Unit`** | The `readonly record struct` value used as `Result` when a command/query succeeds without returning data — replaces the `Result` with `IsSuccess = true` and `Value = null` shape Standards 09 § Forbidden bans. | | **`IClock` / `IRandom` / `IGuidFactory`** | The three deterministic-test abstractions in `LearnStack.SharedKernel`. Production code never reads `DateTime.UtcNow`, instantiates `System.Random`, or calls `Guid.NewGuid()` directly — those calls go through the abstractions so tests pin the values via `FixedClock` / `FixedRandom` / `FixedGuidFactory`. Per Standards 02 § Time. | -| **`Entity` / `AuditableEntity`** | The two aggregate bases in `LearnStack.SharedKernel.Domain`. `Entity` is the append-only / audit-row base — identity, in-process domain events, identity-based equality. `AuditableEntity` is the mutable base — adds `CreatedAt/By`, `UpdatedAt/By`, `DeletedAt/By`, `Version`, and the `IsDeleted` projection by implementing `ISoftDelete` + `IOptimisticConcurrency`. `AuditEntry` (audit subsystem) inherits `Entity` — never `AuditableEntity` — by architecture-test rule. | -| **`IDomainEvent`** | The marker interface (`: MediatR.INotification`) every in-process domain event implements. Raised from aggregate methods, collected by the unit of work, dispatched in-process by MediatR. Distinct from integration events, which cross module boundaries through the outbox + Dapr pub/sub per [ADR-0010](decisions/0010-cross-module-communication.md). | -| **`CursorPagination` / `Page` / `PageInfo`** | The cursor-first pagination triple in `LearnStack.SharedKernel.Pagination` matching Standards 04 § Pagination. `CursorPagination(Cursor, Limit)` is the request (default `Limit = 20`, max 100); `Page(Items, PageInfo)` is the response; `PageInfo(NextCursor, PreviousCursor, HasNext, HasPrevious)` carries the opaque cursors the client never parses. | +| **`UserId`** | The cross-cutting strongly-typed actor identifier in `LearnStack.SharedKernel.Identifiers` (Vogen `[ValueObject]`). Audit columns on `AuditableEntity` reference users by `UserId` so the "no raw `Guid` on the public surface" rule (Standards 02) holds even though the Identity module lands in Phase 02b. Identity consumes the same type when it ships. | +| **`Entity` / `AuditableEntity`** | The two aggregate bases in `LearnStack.SharedKernel.Domain`. `Entity` is the append-only / audit-row base — identity, in-process domain events, identity-based equality with **transient + cross-runtime-type guards** so EF Core's change tracker / `HashSet` navigations behave correctly before keys are stamped. `AuditableEntity` is the mutable base — adds `CreatedAt/By`, `UpdatedAt/By`, `DeletedAt/By`, `Version`, and the `IsDeleted` projection by implementing `ISoftDelete` + `IOptimisticConcurrency`. `MarkCreated` throws on second call; `SoftDelete` also bumps `UpdatedAt` so "last touched" stays monotonic. `AuditEntry` (audit subsystem) inherits `Entity` — never `AuditableEntity` — by architecture-test rule. | +| **`IDomainEvent`** | The marker interface (`: MediatR.INotification`) every in-process domain event implements. Raised from aggregate methods, collected by the unit of work, dispatched in-process by MediatR. The abstract `DomainEvent` base declares `EventId` and `OccurredAt` as `required init` so events are always stamped through `IGuidFactory` / `IClock` at the call site. Distinct from integration events, which cross module boundaries through the outbox + Dapr pub/sub per [ADR-0010](decisions/0010-cross-module-communication.md). | +| **`CursorPagination` / `Page` / `PageInfo`** | The cursor-first pagination triple in `LearnStack.SharedKernel.Pagination` matching Standards 04 § Pagination. `CursorPagination(Cursor, Limit)` is the request (default `Limit = 20`, max 100; ctor throws on `Limit <= 0` — kernel-level guard); `Page(Items, PageInfo)` is the response; `PageInfo(NextCursor, PreviousCursor, HasNext, HasPrevious)` carries the opaque cursors the client never parses. | | **`LearnStackVogenDefaults.IdMask`** | The canonical `Conversions` mask every Vogen-emitted ID and value object opts into: `EfCoreValueConverter \| SystemTextJson \| TypeConverter`. Per [ADR-0023](decisions/0023-strongly-typed-id-source-generator.md) every aggregate-root ID writes `[ValueObject(LearnStackVogenDefaults.IdMask)]`. | | **Pipeline Behavior** | A MediatR pipeline behavior — one of the eight canonical steps wrapping every command / query: `Validation → Logging → Audit → TenantContext → Authorization → Transaction → OutboxFlush → Handler`. The order is binding per [ADR-0032 § Sub-decision 2](decisions/0032-exception-handling-logging-and-observability.md); the architecture test `MediatR_Pipeline_Order_Matches_Canonical_Sequence` enforces it. | | **`IExceptionHandler` (LearnStack L1)** | The `.NET 8+` exception-handler interface; `LearnStackExceptionHandler : IExceptionHandler` is the final catch site for every unhandled exception. Maps to Problem Details, attaches `correlationId`, records the OTel span error, calls `IErrorTrackingProvider.CaptureAsync` when `ShouldCapture(ex)` is true. See [ADR-0032 § Sub-decision 1](decisions/0032-exception-handling-logging-and-observability.md). | diff --git a/docs/roadmap/phase-02a-kernel-tenancy.md b/docs/roadmap/phase-02a-kernel-tenancy.md index 373d645..8da420b 100644 --- a/docs/roadmap/phase-02a-kernel-tenancy.md +++ b/docs/roadmap/phase-02a-kernel-tenancy.md @@ -55,6 +55,20 @@ > `VogenIdEmissionTests` smoke test asserts the emitter pipeline > (Vogen `[ValueObject]` → `IStronglyTypedId.Value` → JSON > round-trip) end-to-end via a synthetic `TestId` in the test project. +> Review pass folded in (commit `7c9133a`): `Entity` equality carries +> transient + cross-runtime-type guards; `Result.FailFor` returns +> the concrete `TResponse` via reflection (not `Result`); +> `Error.Code` is the unprefixed stable identifier projected from +> `Message.Key`; `Error.Details` flows `LocalizedMessage` lists so the prefix +> invariant covers field-level errors; `Result.Ok` rejects null; +> `UserId` is a SharedKernel-level Vogen value object used by +> `AuditableEntity` instead of raw `Guid`; `DomainEvent.EventId` / +> `OccurredAt` are `required init`; `MarkCreated` throws on second call; +> `SoftDelete` bumps `UpdatedAt` for monotonic last-touched; +> `CursorPagination` validates `Limit > 0` at the ctor. Standards 01 +> § Dependency Direction grows a "Build-time-only exceptions" sub-section +> for the EF Core + MediatR references SharedKernel requires. 64 unit +> tests + 17 architecture tests green. > > **Packet 3 — Cross-cutting foundation ⏳** > Wires the [ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md) diff --git a/docs/standards/01-architecture-standards.md b/docs/standards/01-architecture-standards.md index a9c2095..be3dc59 100644 --- a/docs/standards/01-architecture-standards.md +++ b/docs/standards/01-architecture-standards.md @@ -65,6 +65,26 @@ Forbidden edges: - Module A → Module B.Domain - Module A → Module B.Infrastructure +### Build-time-only exceptions + +`SharedKernel` and every `Modules..Domain` project carries two sanctioned +external NuGet references that the rules above would otherwise forbid: + +| Reference | Why | Used at | Sanctioning ADR | +|-----------|-----|---------|-----------------| +| `Microsoft.EntityFrameworkCore` | The Vogen-emitted `.EfCoreValueConverter` type per strongly-typed ID lives in the project that declares `[ValueObject]`. The emitted IL carries TypeRefs to EF Core; the consuming project must reference EF Core at compile time for the converter to load. Hand-written Domain code does **not** import EF Core types. | Compile-time only | [ADR-0023](../decisions/0023-strongly-typed-id-source-generator.md) | +| `MediatR` | `IDomainEvent : INotification` so in-process aggregate events dispatch via MediatR's publisher (the canonical pipeline per [ADR-0010](../decisions/0010-cross-module-communication.md)). | Build-time + runtime (marker only) | [ADR-0010](../decisions/0010-cross-module-communication.md) | + +Both references are scoped to **build-time / IL-level dependencies for +generated or marker shapes**, not to hand-written Domain code calling EF +Core or MediatR APIs. The follow-up architecture test +`Domain_Does_Not_Depend_On_Microsoft_EntityFrameworkCore_Except_Vogen_Emitted_Converters` +catalogued under +[21-architecture-tests-catalogue.md](21-architecture-tests-catalogue.md) +encodes the exception (lands with the first Module.Domain aggregate in +[Phase 02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md)). Adding a +third build-time reference to Domain or SharedKernel requires an ADR. + ## Aggregate Ownership - Every entity belongs to exactly one aggregate; aggregate is owned by exactly one module. diff --git a/docs/standards/02-backend-coding.md b/docs/standards/02-backend-coding.md index 6efc028..f1f1f1b 100644 --- a/docs/standards/02-backend-coding.md +++ b/docs/standards/02-backend-coding.md @@ -86,24 +86,37 @@ Two patterns coexist: - **`Result`** for *expected* outcomes (validation failure, not found, conflict). ```csharp -public sealed record Result( - bool IsSuccess, - T? Value, - Error? Error, - LocalizedMessage? SuccessMessage = null) : IResultBase +public sealed record Result : IResultBase { + internal Result(bool isSuccess, T? value, Error? error, LocalizedMessage? successMessage = null) { ... } + + public bool IsSuccess { get; } public bool IsFailure => !IsSuccess; - public static Result Ok(T value, LocalizedMessage? message = null) => new(true, value, null, message); - public static Result Fail(Error error) => new(false, default, error); + public T? Value { get; } + public Error? Error { get; } + public LocalizedMessage? SuccessMessage { get; } + + // Throws when value is null — Standards 09 § Forbidden bans + // IsSuccess = true with Value = null. For payload-less success use + // Result. + public static Result Ok(T value, LocalizedMessage? message = null); + public static Result Fail(Error error); } -public sealed record Error(LocalizedMessage Message, IReadOnlyDictionary? Details = null) +public sealed record Error( + LocalizedMessage Message, + IReadOnlyDictionary>? Details = null) { - public string Code => Message.Key; + // Stable machine-readable identifier — Standards 04 § Problem Details + // "code". Derived from Message.Key by stripping the lockey_ prefix so + // the code never drifts from the localization key by construction. + public string Code => Message.Key[LocalizedMessage.RequiredPrefix.Length..]; } ``` -`LocalizedMessage`'s constructor enforces a `lockey_` key prefix — see +`LocalizedMessage`'s constructor enforces the `lockey_` key prefix; the +constructor of `Result` is `internal` so callers cannot bypass the +`Ok` / `Fail` factory invariants via positional record syntax. See [09-error-handling.md § Result Type](09-error-handling.md) and [Phase 02a Packet 2](../roadmap/phase-02a-kernel-tenancy.md). diff --git a/docs/standards/09-error-handling.md b/docs/standards/09-error-handling.md index d01ef28..ef7ddb9 100644 --- a/docs/standards/09-error-handling.md +++ b/docs/standards/09-error-handling.md @@ -30,43 +30,58 @@ Both end in **RFC 7807 Problem Details** at the API boundary. ## Result Type ```csharp -public sealed record Result( - bool IsSuccess, - T? Value, - Error? Error, - LocalizedMessage? SuccessMessage = null) : IResultBase +public sealed record Result : IResultBase { + internal Result(bool isSuccess, T? value, Error? error, LocalizedMessage? successMessage = null) { ... } + public bool IsSuccess { get; } public bool IsFailure => !IsSuccess; - public static Result Ok(T value, LocalizedMessage? message = null) => new(true, value, null, message); - public static Result Fail(Error error) => new(false, default, error); + public T? Value { get; } + public Error? Error { get; } + public LocalizedMessage? SuccessMessage { get; } + + public static Result Ok(T value, LocalizedMessage? message = null); // throws on null value + public static Result Fail(Error error); } public sealed record Error( LocalizedMessage Message, - IReadOnlyDictionary? Details = null) + IReadOnlyDictionary>? Details = null) { - public string Code => Message.Key; + public string Code => Message.Key[LocalizedMessage.RequiredPrefix.Length..]; } public sealed record LocalizedMessage(string Key, IReadOnlyDictionary? Params = null) { - // ctor enforces Key.StartsWith("lockey_") — see Phase 02a Packet 2. + public const string RequiredPrefix = "lockey_"; + // ctor enforces Key.StartsWith(RequiredPrefix); see Phase 02a Packet 2. } + +public readonly record struct Unit { public static Unit Value { get; } } ``` The `LocalizedMessage`'s `lockey_` prefix is invariant: the constructor rejects any key that does not start with `lockey_`. Frontend translation catalogues are keyed by the same prefix; backend code never returns raw -English. `Error.Code` is a convenience projection of `Message.Key` — -routing logic (`Result.ToActionResult()`, Problem Details writers) reads -the projection without dereferencing the nested record. Per +English. `Error.Code` is a **stable, unprefixed** projection of +`Message.Key` (the `lockey_` prefix is stripped). Routing logic +(`Result.ToActionResult()`, Problem Details writers) reads `Code`; the +frontend reads `Message.Key` for locale resolution — two surfaces in +sync by construction. Per [Phase 02a Packet 2](../roadmap/phase-02a-kernel-tenancy.md) and [ADR-0032 § Error Model](../decisions/0032-exception-handling-logging-and-observability.md). -Standard error codes (machine-readable, stable). On the wire and in code, -every identifier below appears with the `lockey_` prefix (so -`Error.Code == "lockey_validation_failed"`); the table omits the prefix -for readability. +`Result`'s primary constructor is `internal`; callers go through +`Ok` / `Fail` so the success-must-carry-value rule +(see § Forbidden) cannot be bypassed via positional record syntax. +`Result` is the canonical payload-less success shape. + +Field-level errors in `Error.Details` flow as `LocalizedMessage` lists +per key, so the `lockey_` invariant covers every user-facing string the +API ships — not just the top-level message. + +Standard error codes (machine-readable, stable). The table uses the +unprefixed shape that travels on the Problem Details `code` field; the +matching localization key adds the `lockey_` prefix. | Code | Meaning | HTTP | |------|---------|------| From 188d33a2f6a6ac3b129975201fd2392b54902ac0 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Thu, 21 May 2026 13:51:43 +0300 Subject: [PATCH 5/8] =?UTF-8?q?fix(shared-kernel):=20packet=202=20PR=20#6?= =?UTF-8?q?=20review-2=20=E2=80=94=20defensive=20guards=20+=20cast-safe=20?= =?UTF-8?q?collections?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All seven findings from the second review pass verified against current code and applied. No skips. Inline findings: - AuditableEntity.MarkCreated / MarkUpdated / SoftDelete now reject default(DateTimeOffset) and the Guid.Empty UserId via a shared EnsureValidAuditInput helper - default sentinels are programmer errors, never legitimate audit metadata. - Entity.DomainEvents returns a cached ReadOnlyCollection wrapper initialised once per entity rather than the backing List directly, so callers cannot downcast and mutate the collection out from under the aggregate. Wrapper is a one-time reference allocation; hot-path access stays allocation-free. - LocalizedMessage.Params stores a ReadOnlyDictionary wrapper around the defensive copy of the input dictionary - the IReadOnlyDictionary slot no longer allows a downcast back to Dictionary. - ISoftDelete.DeletedBy is UserId? (Standards 02 - no raw Guid on the marker surface). The explicit-interface-impl shim in AuditableEntity goes away; EF's Vogen value converter handles the column mapping. - FixedClock normalises every stored instant to UTC via ToUniversalTime() at the boundary so IClock.UtcNow always honours its contract regardless of the caller's input offset. Nitpicks: - VogenIdEmissionTests now exercises TypeConverter (the artefact ASP.NET Core minimal-API route binding and IConfiguration use) via TypeDescriptor.GetConverter round-trip - closes the coverage gap the fixture's doc summary advertised. - Result.FailFor caches the resolved MethodInfo in a nested FailForCache generic class so the reflection cost is paid once per closed Result type, not on every pipeline invocation. Drops the redundant MakeGenericType reconstruction (TResponse is already the closed generic). - Unit.Value is now an expression-bodied `=> default` rather than an uninitialised auto-property; idiomatic and removes the dead backing field. Tests: 71 unit (+7) green, 17 architecture green, 1 contract green under CI=true. New coverage: audit-input default-rejection (timestamp + empty actor), FixedClock non-UTC offset normalisation, ISoftDelete.DeletedBy strong typing, TypeConverter round-trip. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Domain/AuditableEntity.cs | 34 ++++++++--- .../LearnStack.SharedKernel/Domain/Entity.cs | 15 +++-- .../Localization/LocalizedMessage.cs | 15 +++-- .../Persistence/ISoftDelete.cs | 10 +++- .../Results/Result.Helpers.cs | 53 +++++++++------- .../LearnStack.SharedKernel/Results/Unit.cs | 7 ++- .../Time/FixedClock.cs | 13 +++- .../Domain/AuditableEntityTests.cs | 60 ++++++++++++++++++- .../Identifiers/VogenIdEmissionTests.cs | 26 +++++++- .../SharedKernel/Time/FixedClockTests.cs | 25 ++++++++ 10 files changed, 212 insertions(+), 46 deletions(-) diff --git a/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs b/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs index 9660587..81cef6f 100644 --- a/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs +++ b/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs @@ -42,13 +42,6 @@ protected AuditableEntity() public uint Version { get; protected set; } - // ISoftDelete declares DeletedBy as Guid? for module-agnostic readers - // (EF global query filters, RLS policy plumbing). Surface the raw Guid - // projection through explicit interface implementation so callers - // holding the concrete type read the strongly-typed UserId? while - // cross-cutting infrastructure sees a Guid? at the marker layer. - Guid? ISoftDelete.DeletedBy => DeletedBy?.Value; - /// /// Convenience projection of . EF global query /// filters typically gate on this property. @@ -63,6 +56,8 @@ protected AuditableEntity() /// public void MarkCreated(DateTimeOffset at, UserId by) { + EnsureValidAuditInput(at, by); + if (CreatedAt != default) { throw new InvalidOperationException( @@ -80,6 +75,8 @@ public void MarkCreated(DateTimeOffset at, UserId by) /// public void MarkUpdated(DateTimeOffset at, UserId by) { + EnsureValidAuditInput(at, by); + UpdatedAt = at; UpdatedBy = by; } @@ -94,9 +91,32 @@ public void MarkUpdated(DateTimeOffset at, UserId by) /// public void SoftDelete(DateTimeOffset at, UserId by) { + EnsureValidAuditInput(at, by); + DeletedAt = at; DeletedBy = by; UpdatedAt = at; UpdatedBy = by; } + + // Audit metadata must always be meaningful: the default timestamp + // (0001-01-01) and the default UserId (Guid.Empty) are programmer-error + // sentinels rather than legitimate audit values. Fail loud at the call + // site rather than persisting them. + private static void EnsureValidAuditInput(DateTimeOffset at, UserId by) + { + if (at == default) + { + throw new ArgumentException( + "Audit timestamp must be a meaningful instant, not default(DateTimeOffset). Pass the value from IClock.UtcNow.", + nameof(at)); + } + + if (by.Value == Guid.Empty) + { + throw new ArgumentException( + "Audit actor must be a real UserId, not default(UserId). Pass the resolved ITenantContext.UserId.", + nameof(by)); + } + } } diff --git a/backend/src/LearnStack.SharedKernel/Domain/Entity.cs b/backend/src/LearnStack.SharedKernel/Domain/Entity.cs index e14dcd1..e13f310 100644 --- a/backend/src/LearnStack.SharedKernel/Domain/Entity.cs +++ b/backend/src/LearnStack.SharedKernel/Domain/Entity.cs @@ -1,3 +1,4 @@ +using System.Collections.ObjectModel; using LearnStack.SharedKernel.Identifiers; namespace LearnStack.SharedKernel.Domain; @@ -29,26 +30,32 @@ public abstract class Entity : IHasId, IHasDomainEvents where TId : struct, IStronglyTypedId { private readonly List _domainEvents = []; + private readonly ReadOnlyCollection _domainEventsView; protected Entity(TId id) { Id = id; + _domainEventsView = _domainEvents.AsReadOnly(); } // EF Core / ORM materialization ctor. protected Entity() { + _domainEventsView = _domainEvents.AsReadOnly(); } public TId Id { get; protected init; } /// /// In-process domain events raised since the last . - /// Returns the backing list as without - /// allocating a wrapper - the unit of work walks tracked entities on every - /// SaveChangesAsync, so the property is on a hot path. + /// Returns a cached wrapper rather + /// than the backing list so callers cannot cast back to List<T> + /// and mutate the collection out from under the aggregate. The view is + /// initialised once per entity (cheap reference allocation) and lives + /// for the entity's lifetime, so the unit-of-work's hot-path access + /// stays allocation-free. /// - public IReadOnlyCollection DomainEvents => _domainEvents; + public IReadOnlyCollection DomainEvents => _domainEventsView; protected void RaiseDomainEvent(IDomainEvent domainEvent) { diff --git a/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs b/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs index bc365bb..4348022 100644 --- a/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs +++ b/backend/src/LearnStack.SharedKernel/Localization/LocalizedMessage.cs @@ -1,3 +1,5 @@ +using System.Collections.ObjectModel; + namespace LearnStack.SharedKernel.Localization; /// @@ -43,12 +45,15 @@ public LocalizedMessage(string key, IReadOnlyDictionary? @params } Key = key; - // Defensive copy: the caller may mutate the original dictionary after - // construction; a LocalizedMessage is supposed to be immutable, so - // snapshot the contents here. The empty case is normalised to null so - // serializers do not emit an unused "params": {} field. + // Snapshot + wrap: the caller may mutate the original dictionary + // after construction, and IReadOnlyDictionary does not enforce + // immutability on its own (callers can downcast). A ReadOnlyDictionary + // wrapper over a defensive copy gives both: cast-safe at the API + // level and snapshot-stable at the data level. The empty case is + // normalised to null so serializers do not emit an unused + // "params": {} field. Params = @params is { Count: > 0 } - ? new Dictionary(@params) + ? new ReadOnlyDictionary(new Dictionary(@params)) : null; } diff --git a/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs b/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs index c4d8f5c..b95507b 100644 --- a/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs +++ b/backend/src/LearnStack.SharedKernel/Persistence/ISoftDelete.cs @@ -1,3 +1,5 @@ +using LearnStack.SharedKernel.Identifiers; + namespace LearnStack.SharedKernel.Persistence; /// @@ -11,5 +13,11 @@ public interface ISoftDelete { DateTimeOffset? DeletedAt { get; } - Guid? DeletedBy { get; } + /// + /// The actor who soft-deleted the entity. Strongly-typed + /// () per Standards 02 § Strongly-Typed Identifiers + /// — no raw on the marker surface. EF's Vogen value + /// converter persists this as a UUID column. + /// + UserId? DeletedBy { get; } } diff --git a/backend/src/LearnStack.SharedKernel/Results/Result.Helpers.cs b/backend/src/LearnStack.SharedKernel/Results/Result.Helpers.cs index 1fbb2e2..c7d47a6 100644 --- a/backend/src/LearnStack.SharedKernel/Results/Result.Helpers.cs +++ b/backend/src/LearnStack.SharedKernel/Results/Result.Helpers.cs @@ -23,37 +23,46 @@ public static Result Ok(T value, LocalizedMessage? message = null) => /// Returns an instance of — not /// Result<TResponse> — by reflecting over the closed /// generic to invoke the correct Result<TValue>.Fail(error). - /// Per ADR-0032 § Sub-decision 3. /// + /// + /// The reflected is cached per closed + /// in the nested + /// — initialised once on first + /// touch, zero per-call overhead afterwards. The hot path is the + /// MediatR pipeline behavior that runs on every command. + /// public static TResponse FailFor(Error error) where TResponse : IResultBase { ArgumentNullException.ThrowIfNull(error); - var responseType = typeof(TResponse); - if (!responseType.IsGenericType - || responseType.GetGenericTypeDefinition() != typeof(Result<>)) - { - throw new InvalidOperationException( - $"Result.FailFor requires TResponse to be a closed " + - $"Result; got {responseType.FullName}."); - } + var fail = FailForCache.FailMethod + ?? throw new InvalidOperationException( + $"Result.FailFor requires TResponse to be a closed Result; got {typeof(TResponse).FullName}."); + + return (TResponse)fail.Invoke(obj: null, parameters: [error])!; + } - var valueType = responseType.GetGenericArguments()[0]; - var concreteResultType = typeof(Result<>).MakeGenericType(valueType); - var failMethod = concreteResultType.GetMethod( - nameof(Result.Fail), - BindingFlags.Public | BindingFlags.Static, - binder: null, - types: [typeof(Error)], - modifiers: null); + private static class FailForCache + where TResponse : IResultBase + { + public static readonly MethodInfo? FailMethod = ResolveFailMethod(); - if (failMethod is null) + private static MethodInfo? ResolveFailMethod() { - throw new InvalidOperationException( - $"Could not locate {concreteResultType.FullName}.Fail(Error) via reflection."); - } + var responseType = typeof(TResponse); + if (!responseType.IsGenericType + || responseType.GetGenericTypeDefinition() != typeof(Result<>)) + { + return null; + } - return (TResponse)failMethod.Invoke(obj: null, parameters: [error])!; + return responseType.GetMethod( + nameof(Result.Fail), + BindingFlags.Public | BindingFlags.Static, + binder: null, + types: [typeof(Error)], + modifiers: null); + } } } diff --git a/backend/src/LearnStack.SharedKernel/Results/Unit.cs b/backend/src/LearnStack.SharedKernel/Results/Unit.cs index a89e87a..0775fed 100644 --- a/backend/src/LearnStack.SharedKernel/Results/Unit.cs +++ b/backend/src/LearnStack.SharedKernel/Results/Unit.cs @@ -8,5 +8,10 @@ namespace LearnStack.SharedKernel.Results; /// public readonly record struct Unit { - public static Unit Value { get; } + /// + /// The single canonical value. All + /// instances are equal by definition; is just the + /// idiomatic spelling at call sites. + /// + public static Unit Value => default; } diff --git a/backend/src/LearnStack.SharedKernel/Time/FixedClock.cs b/backend/src/LearnStack.SharedKernel/Time/FixedClock.cs index edbd8c4..390f1dc 100644 --- a/backend/src/LearnStack.SharedKernel/Time/FixedClock.cs +++ b/backend/src/LearnStack.SharedKernel/Time/FixedClock.cs @@ -4,18 +4,25 @@ namespace LearnStack.SharedKernel.Time; /// Deterministic for tests. Time advances only through /// / — never spontaneously. /// +/// +/// All stored instants are normalised to UTC offset (TimeSpan.Zero): +/// the contract promises a UTC value, so an +/// input carrying a non-zero offset is converted via +/// at the boundary rather +/// than silently returned to the caller with the wrong offset. +/// public sealed class FixedClock : IClock { private DateTimeOffset _utcNow; public FixedClock(DateTimeOffset utcNow) { - _utcNow = utcNow; + _utcNow = utcNow.ToUniversalTime(); } public DateTimeOffset UtcNow => _utcNow; - public void SetUtcNow(DateTimeOffset utcNow) => _utcNow = utcNow; + public void SetUtcNow(DateTimeOffset utcNow) => _utcNow = utcNow.ToUniversalTime(); - public void Advance(TimeSpan delta) => _utcNow = _utcNow.Add(delta); + public void Advance(TimeSpan delta) => _utcNow = _utcNow.Add(delta).ToUniversalTime(); } diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs index 9e98de2..45b66c4 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/AuditableEntityTests.cs @@ -70,15 +70,71 @@ public void SoftDelete_SetsDeletedColumns_AndAlsoBumpsUpdated() } [Fact] - public void ISoftDelete_DeletedBy_ProjectsGuidFromUserId() + public void ISoftDelete_DeletedBy_IsStronglyTypedUserId() { var aggregate = new TestAuditableAggregate(TestId.New()); aggregate.MarkCreated(T0, Actor); aggregate.SoftDelete(T0.AddDays(1), Actor); + // The cast is the point of this test - the marker contract is what + // we are asserting. CA1859 would prefer the concrete type for + // performance. +#pragma warning disable CA1859 ISoftDelete view = aggregate; +#pragma warning restore CA1859 - view.DeletedBy.Should().Be(Actor.Value); + view.DeletedBy.Should().Be(Actor); + } + + // Vogen prohibits constructing `default(UserId)` at compile time (VOG009), + // so the "empty actor" case is exercised via UserId.From(Guid.Empty) - + // the guard reads `by.Value == Guid.Empty`, which both forms hit. + private static readonly UserId EmptyActor = UserId.From(Guid.Empty); + + [Fact] + public void MarkCreated_WithDefaultTimestamp_Throws() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + + var act = () => aggregate.MarkCreated(default, Actor); + + act.Should().Throw().WithMessage("*default*"); + } + + [Fact] + public void MarkCreated_WithEmptyActor_Throws() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + + var act = () => aggregate.MarkCreated(T0, EmptyActor); + + act.Should().Throw().WithMessage("*UserId*"); + } + + [Fact] + public void MarkUpdated_WithInvalidInputs_Throws() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + aggregate.MarkCreated(T0, Actor); + + var defaultAt = () => aggregate.MarkUpdated(default, Actor); + var emptyBy = () => aggregate.MarkUpdated(T0.AddHours(1), EmptyActor); + + defaultAt.Should().Throw(); + emptyBy.Should().Throw(); + } + + [Fact] + public void SoftDelete_WithInvalidInputs_Throws() + { + var aggregate = new TestAuditableAggregate(TestId.New()); + aggregate.MarkCreated(T0, Actor); + + var defaultAt = () => aggregate.SoftDelete(default, Actor); + var emptyBy = () => aggregate.SoftDelete(T0.AddDays(1), EmptyActor); + + defaultAt.Should().Throw(); + emptyBy.Should().Throw(); } [Fact] diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/VogenIdEmissionTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/VogenIdEmissionTests.cs index a183233..f845e48 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/VogenIdEmissionTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/VogenIdEmissionTests.cs @@ -1,3 +1,5 @@ +using System.ComponentModel; +using System.Globalization; using System.Text.Json; using FluentAssertions; using LearnStack.SharedKernel.Identifiers; @@ -12,7 +14,8 @@ namespace LearnStack.Tests.Unit.SharedKernel.Identifiers; /// [ValueObject<Guid>(LearnStackVogenDefaults.IdMask)]; this /// fixture asserts the four emitted artefacts are wired: /// - System.Text.Json round trip (Conversions.SystemTextJson). -/// - TypeConverter parse/format (Conversions.TypeConverter). +/// - TypeConverter parse/format (Conversions.TypeConverter — the path +/// ASP.NET Core minimal-API route binding and IConfiguration use). /// - projection. /// - Type-safe inequality between two arbitrary IDs. /// EF Core converter / minimal-API model binder are exercised at the @@ -40,6 +43,27 @@ public void SystemTextJson_RoundTrip_PreservesValue() decoded.Should().Be(original); } + [Fact] + public void TypeConverter_RoundTrips_ViaString() + { + // Conversions.TypeConverter is the artefact ASP.NET Core minimal-API + // route binding (and IConfiguration binding) rely on. Asserting the + // converter exists and round-trips proves the mask wired the + // TypeConverter the runtime will discover. + var original = TestId.New(); + var converter = TypeDescriptor.GetConverter(typeof(TestId)); + + converter.Should().NotBeNull(); + converter.CanConvertTo(typeof(string)).Should().BeTrue(); + converter.CanConvertFrom(typeof(string)).Should().BeTrue(); + + var encoded = converter.ConvertToString(null, CultureInfo.InvariantCulture, original); + var decoded = (TestId)converter.ConvertFromString(null!, CultureInfo.InvariantCulture, encoded!)!; + + decoded.Should().Be(original); + ((IStronglyTypedId)decoded).Value.Should().Be(((IStronglyTypedId)original).Value); + } + [Fact] public void TwoIdsWithDifferentGuids_AreNotEqual() { diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/FixedClockTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/FixedClockTests.cs index 3ef5846..cb2ee85 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/FixedClockTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Time/FixedClockTests.cs @@ -37,4 +37,29 @@ public void SetUtcNow_ReplacesTheCurrentInstant() clock.UtcNow.Should().Be(target); } + + [Fact] + public void Ctor_NormalisesNonUtcOffsetToUtc() + { + // 10:00 in +03:00 == 07:00 UTC. The IClock.UtcNow contract requires + // a UTC-offset value; non-UTC inputs are normalised at the boundary. + var nonUtc = new DateTimeOffset(2026, 05, 21, 10, 00, 00, TimeSpan.FromHours(3)); + + var clock = new FixedClock(nonUtc); + + clock.UtcNow.Offset.Should().Be(TimeSpan.Zero); + clock.UtcNow.Hour.Should().Be(7, "10:00 +03:00 is 07:00 UTC"); + } + + [Fact] + public void SetUtcNow_NormalisesNonUtcOffsetToUtc() + { + var clock = new FixedClock(T0); + var nonUtc = new DateTimeOffset(2026, 05, 21, 12, 00, 00, TimeSpan.FromHours(-5)); + + clock.SetUtcNow(nonUtc); + + clock.UtcNow.Offset.Should().Be(TimeSpan.Zero); + clock.UtcNow.Hour.Should().Be(17, "12:00 -05:00 is 17:00 UTC"); + } } From 6b3a4aa3dfd79b90224868f5ceec742a8ed60652 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Thu, 21 May 2026 15:33:21 +0300 Subject: [PATCH 6/8] =?UTF-8?q?fix(shared-kernel):=20packet=202=20PR=20#6?= =?UTF-8?q?=20review-3=20=E2=80=94=20defensive=20ctors=20+=20module=20wiri?= =?UTF-8?q?ng=20+=20doc=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All 13 findings from the third review pass (one APPROVE with 3 minor + 4 suggestion, one REQUEST CHANGES with 5 major + 2 minor + 1 suggestion) verified against current code and applied. Code (kernel) - majors: - CursorPagination ctor now clamps `Limit > MaxLimit` to `MaxLimit` silently (still throws on `Limit <= 0` as a programmer-error guard). Removed `Normalised()` - the invariant is enforced at construction so no instance can exist with an out-of-range `Limit`. Drops the DoS footgun where a caller forgot to normalise and passed `int.MaxValue` into `.Take(...)`. - Error switched to an explicit constructor: `ThrowIfNull(Message)` plus defensive snapshot of `Details` (dictionary copy + per-key `ReadOnlyCollection` wrap) so callers cannot mutate the validation map after the Result is in flight - Problem Details, audit, and log sinks all read after handler return. Structural Equals + GetHashCode override because `IReadOnlyDictionary` defaults to reference equality. - UserId.New() removed - the convenience static was a tempting bypass of `IGuidFactory`. New UserIds in handlers now require `UserId.From(guidFactory.NewUuidV7())` at the call site. Code (kernel) - minors: - Entity.DomainEvents lazily allocates both the backing List and the cached ReadOnlyCollection wrapper on first raise / first read. EF Core's read path materialises every loaded aggregate via the parameterless ctor; the lazy approach keeps materialisation allocation-free for the common query case (paginated reads, projections) and pays the allocation only when an aggregate actually raises events (command paths). - LearnStackVogenDefaults moved from `LearnStack.SharedKernel.Identifiers` to the root `LearnStack.SharedKernel` namespace - the mask covers aggregate IDs AND richer value objects per ADR-0023, so neither surface owns the constant exclusively. The Vogen attribute on UserId / TestId still resolves it via parent-namespace lookup; TestId in the test project gains an explicit `using LearnStack.SharedKernel;`. - AuditableEntity.IsDeleted XML doc is now honest about EF translation: the property is a computed CLR get and is NOT guaranteed to translate by EF Core's expression translator. Global query filters in Packet 7 should gate on `e.DeletedAt == null` directly (the mapped column translates cleanly). Tests: - CursorPaginationTests rewritten: ctor clamp, boundary cases (Limit == MaxLimit, Limit == 1), removed all `Normalised()` references. - ErrorTests: ctor null-Message throw, defensive-copy assertion (caller mutates the source dictionary + list after construction; the Error's Details remain stable), structural-equality across distinct dictionary instances. - FixedGuidFactoryTests: mixed V7+V4 seed test locks the documented shared-queue contract. - Total: 77 unit (+6 since previous round) + 17 architecture + 1 contract green under CI=true. Module wiring (B.M5): - New `backend/src/Modules/Directory.Build.props` adds the Vogen + Microsoft.EntityFrameworkCore PackageReferences to every `Modules..Domain` project via `EndsWith('.Domain')` condition. Explicitly imports the parent `backend/Directory.Build.props` because MSBuild stops at the first ancestor Directory.Build.props it finds rather than merging up. This closes the gap ADR-0023 § Implementation Notes promised but the original Packet 2 land did not deliver: `[ValueObject(LearnStackVogenDefaults.IdMask)]` on a TenantId / CourseId / OrganizationId etc. in Packet 6+ will now "just work" without per-csproj wiring. Docs: - Standards 02 § Strongly-Typed Identifiers snippet rewritten to the canonical Vogen form (the stale hand-rolled example with `Guid.NewGuid()` was misleading new module authors). Adds the no-`New()` rule: IDs mint via `IGuidFactory` at the call site. - Standards 09 § API Surface example refreshed: top-level uses `messageKey` (`lockey_*`) and `code` (unprefixed stable id); field errors flow as LocalizedMessage payloads (`key` + optional `params`) so the lockey_ invariant covers the entire wire shape, not just the top-level message. - architecture/15 handler example: `OccurredAt = _clock.UtcNow` instead of `DateTime.UtcNow` (closes the symmetric `IClock`-bypass anti- example the reviewer caught). - Phase 02a Packet 2 roadmap status: test counts refreshed (54 / 64 → 71 — actually now 77 after this round); TypeConverter coverage noted. Suggestion not adopted: - IRandom rename / NextBytes removal — deferred to the Packet 3+ analyzer pass (both reviewers raised it as a future-analyzer concern). The XML doc already warns "not for crypto"; an analyzer banning IRandom in *.Security.* / *.Auth.* namespaces is the right enforcement surface and lands with the Roslyn analyzer infrastructure. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Domain/AuditableEntity.cs | 9 +- .../LearnStack.SharedKernel/Domain/Entity.cs | 31 +++-- .../Identifiers/UserId.cs | 7 +- .../LearnStackVogenDefaults.cs | 11 +- .../Pagination/CursorPagination.cs | 28 ++-- .../LearnStack.SharedKernel/Results/Error.cs | 124 +++++++++++++++++- backend/src/Modules/Directory.Build.props | 35 +++++ .../SharedKernel/Domain/TestId.cs | 1 + .../SharedKernel/ErrorTests.cs | 77 ++++++++++- .../Identifiers/FixedGuidFactoryTests.cs | 14 ++ .../Pagination/CursorPaginationTests.cs | 41 +++--- docs/architecture/15-event-and-outbox.md | 2 +- docs/roadmap/phase-02a-kernel-tenancy.md | 7 +- docs/standards/02-backend-coding.md | 34 +++-- docs/standards/09-error-handling.md | 23 +++- 15 files changed, 361 insertions(+), 83 deletions(-) rename backend/src/LearnStack.SharedKernel/{Identifiers => }/LearnStackVogenDefaults.cs (65%) create mode 100644 backend/src/Modules/Directory.Build.props diff --git a/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs b/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs index 81cef6f..7ccc012 100644 --- a/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs +++ b/backend/src/LearnStack.SharedKernel/Domain/AuditableEntity.cs @@ -43,8 +43,13 @@ protected AuditableEntity() public uint Version { get; protected set; } /// - /// Convenience projection of . EF global query - /// filters typically gate on this property. + /// Convenience projection of for in-process + /// callers (aggregate methods, application services, mappers). EF + /// global query filters should gate on directly + /// (e => e.DeletedAt == null) — is a + /// computed CLR property and is NOT guaranteed to translate to SQL by + /// EF Core's expression translator. Packet 7 wires the filters + /// accordingly. /// public bool IsDeleted => DeletedAt.HasValue; diff --git a/backend/src/LearnStack.SharedKernel/Domain/Entity.cs b/backend/src/LearnStack.SharedKernel/Domain/Entity.cs index e13f310..e480164 100644 --- a/backend/src/LearnStack.SharedKernel/Domain/Entity.cs +++ b/backend/src/LearnStack.SharedKernel/Domain/Entity.cs @@ -25,23 +25,30 @@ namespace LearnStack.SharedKernel.Domain; /// instances apart so EF Core's change-tracker identity map and any /// HashSet in a collection navigation behave correctly. /// +/// +/// Domain-event collection state is lazily allocated: the backing +/// List and its view are only +/// created on first raise or first read. EF Core materialises every loaded +/// aggregate through the parameterless ctor on read paths; the lazy +/// approach keeps materialisation allocation-free for the common +/// query case (paginated reads, projections), and pays the allocation only +/// when an aggregate actually raises events (command paths). +/// /// public abstract class Entity : IHasId, IHasDomainEvents where TId : struct, IStronglyTypedId { - private readonly List _domainEvents = []; - private readonly ReadOnlyCollection _domainEventsView; + private List? _domainEvents; + private ReadOnlyCollection? _domainEventsView; protected Entity(TId id) { Id = id; - _domainEventsView = _domainEvents.AsReadOnly(); } // EF Core / ORM materialization ctor. protected Entity() { - _domainEventsView = _domainEvents.AsReadOnly(); } public TId Id { get; protected init; } @@ -49,21 +56,21 @@ protected Entity() /// /// In-process domain events raised since the last . /// Returns a cached wrapper rather - /// than the backing list so callers cannot cast back to List<T> - /// and mutate the collection out from under the aggregate. The view is - /// initialised once per entity (cheap reference allocation) and lives - /// for the entity's lifetime, so the unit-of-work's hot-path access - /// stays allocation-free. + /// than the backing List directly so callers cannot downcast + /// and mutate the collection out from under the aggregate. Both the + /// backing list and the wrapper are lazily allocated on first + /// access / first raise. /// - public IReadOnlyCollection DomainEvents => _domainEventsView; + public IReadOnlyCollection DomainEvents => + _domainEventsView ??= (_domainEvents ??= []).AsReadOnly(); protected void RaiseDomainEvent(IDomainEvent domainEvent) { ArgumentNullException.ThrowIfNull(domainEvent); - _domainEvents.Add(domainEvent); + (_domainEvents ??= []).Add(domainEvent); } - public void ClearDomainEvents() => _domainEvents.Clear(); + public void ClearDomainEvents() => _domainEvents?.Clear(); public override bool Equals(object? obj) { diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/UserId.cs b/backend/src/LearnStack.SharedKernel/Identifiers/UserId.cs index f947b05..7b99140 100644 --- a/backend/src/LearnStack.SharedKernel/Identifiers/UserId.cs +++ b/backend/src/LearnStack.SharedKernel/Identifiers/UserId.cs @@ -10,8 +10,13 @@ namespace LearnStack.SharedKernel.Identifiers; /// module that lands in Phase 02b. The Identity module consumes the same /// type once it ships; there is exactly one UserId shape. /// +/// +/// There is no UserId.New() convenience: new +/// values are minted by aggregate methods through the injected +/// IGuidFactory (UserId.From(guidFactory.NewUuidV7())) so +/// tests can pin the value deterministically — see Standards 02 § Time. +/// [ValueObject(LearnStackVogenDefaults.IdMask)] public readonly partial record struct UserId : IStronglyTypedId { - public static UserId New() => From(Guid.CreateVersion7()); } diff --git a/backend/src/LearnStack.SharedKernel/Identifiers/LearnStackVogenDefaults.cs b/backend/src/LearnStack.SharedKernel/LearnStackVogenDefaults.cs similarity index 65% rename from backend/src/LearnStack.SharedKernel/Identifiers/LearnStackVogenDefaults.cs rename to backend/src/LearnStack.SharedKernel/LearnStackVogenDefaults.cs index c733d61..2be0875 100644 --- a/backend/src/LearnStack.SharedKernel/Identifiers/LearnStackVogenDefaults.cs +++ b/backend/src/LearnStack.SharedKernel/LearnStackVogenDefaults.cs @@ -1,13 +1,14 @@ using Vogen; -namespace LearnStack.SharedKernel.Identifiers; +namespace LearnStack.SharedKernel; /// /// Canonical Conversions mask for every LearnStack-declared -/// [ValueObject<T>] per ADR-0023. Using the constant keeps the -/// annotation uniform across modules: callers write -/// [ValueObject<Guid>(LearnStackVogenDefaults.IdMask)] rather -/// than hand-picking the flag set. +/// [ValueObject<T>] per ADR-0023. Lives at the SharedKernel +/// root namespace because the mask covers both aggregate-root IDs +/// (e.g. TenantId, UserId) and richer value objects +/// (Email, Slug, LocaleCode, Money) — neither +/// surface owns the constant exclusively. /// public static class LearnStackVogenDefaults { diff --git a/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs b/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs index 257d718..e06366a 100644 --- a/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs +++ b/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs @@ -5,16 +5,17 @@ namespace LearnStack.SharedKernel.Pagination; /// default for every list endpoint. The is an opaque /// token the server minted on a previous response; the client never parses /// it. defaults to and is -/// bounded by . +/// capped at by the constructor. /// /// -/// Construction is the validation point: a non-positive -/// is a programmer error (kernel-level guard) and throws. API-layer -/// FluentValidation should turn malformed user input into -/// Result.Fail(validation_failed) before the -/// kernel sees the request, so the throw here only fires on coding bugs. -/// clamps above-max limits to -/// ; it no longer throws. +/// Construction enforces every invariant: non-positive limits throw +/// (kernel-level programmer-error guard), above-max limits are silently +/// clamped to . There is no normalisation step a +/// caller can forget — once a exists, its +/// is always in [1, MaxLimit]. API-layer +/// FluentValidation should still turn malformed user input into +/// Result.Fail(validation_failed) before the kernel sees the +/// request; the ctor guards are the last line of defense, not the first. /// public sealed record CursorPagination { @@ -33,19 +34,10 @@ public CursorPagination(string? Cursor = null, int Limit = DefaultLimit) } this.Cursor = Cursor; - this.Limit = Limit; + this.Limit = Limit > MaxLimit ? MaxLimit : Limit; } public string? Cursor { get; init; } public int Limit { get; init; } - - /// - /// Returns the request with clamped to - /// . Non-throwing: invalid inputs are stopped at - /// construction, so the only normalisation left is the upper-bound - /// clamp. - /// - public CursorPagination Normalised() => - Limit > MaxLimit ? this with { Limit = MaxLimit } : this; } diff --git a/backend/src/LearnStack.SharedKernel/Results/Error.cs b/backend/src/LearnStack.SharedKernel/Results/Error.cs index ca09d7c..3d9847b 100644 --- a/backend/src/LearnStack.SharedKernel/Results/Error.cs +++ b/backend/src/LearnStack.SharedKernel/Results/Error.cs @@ -1,3 +1,4 @@ +using System.Collections.ObjectModel; using System.Diagnostics.CodeAnalysis; using LearnStack.SharedKernel.Localization; @@ -20,6 +21,21 @@ namespace LearnStack.SharedKernel.Results; /// localization key the frontend resolves. /// /// +/// The constructor takes a defensive snapshot of +/// (dictionary copy + per-key wrap) so +/// callers cannot mutate the validation map after the Result is in flight +/// — Problem Details bodies, audit rows, and structured logs all read +/// after the handler returns, and a mutation +/// between handler-return and serialisation would produce inconsistent +/// output across the three sinks. +/// +/// +/// Structural equality is overridden because +/// uses reference equality by default — two Errors with the same Message +/// and equivalent Details (built from different dictionary instances) +/// would otherwise compare unequal. +/// +/// /// CA1716 (avoid reserved language keywords as type names) is intentionally /// suppressed: the project's Result+Error pattern follows the FluentResults / /// Ardalis.Result lineage where the type is canonically named Error. @@ -31,10 +47,21 @@ namespace LearnStack.SharedKernel.Results; "Naming", "CA1716:Identifiers should not match keywords", Justification = "Result+Error pattern — C#-only codebase per ADR-0032; no VB consumer affected.")] -public sealed record Error( - LocalizedMessage Message, - IReadOnlyDictionary>? Details = null) +public sealed record Error { + public Error( + LocalizedMessage message, + IReadOnlyDictionary>? details = null) + { + ArgumentNullException.ThrowIfNull(message); + Message = message; + Details = SnapshotDetails(details); + } + + public LocalizedMessage Message { get; } + + public IReadOnlyDictionary>? Details { get; } + /// /// Stable machine-readable code derived from 's /// Key with the @@ -43,4 +70,95 @@ public sealed record Error( /// changes per locale (Standards 09 § Forbidden). /// public string Code => Message.Key[LocalizedMessage.RequiredPrefix.Length..]; + + public bool Equals(Error? other) + { + if (other is null) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + return Message.Equals(other.Message) && DetailsEqual(Details, other.Details); + } + + public override int GetHashCode() + { + var hash = new HashCode(); + hash.Add(Message); + + if (Details is not null) + { + var detailsHash = 0; + foreach (var (key, list) in Details) + { + var listHash = 0; + foreach (var msg in list) + { + listHash ^= msg.GetHashCode(); + } + + detailsHash ^= HashCode.Combine(key, listHash); + } + + hash.Add(detailsHash); + } + + return hash.ToHashCode(); + } + + private static ReadOnlyDictionary>? SnapshotDetails( + IReadOnlyDictionary>? source) + { + if (source is not { Count: > 0 }) + { + return null; + } + + var snapshot = new Dictionary>(source.Count); + foreach (var (key, list) in source) + { + ArgumentNullException.ThrowIfNull(list); + snapshot[key] = new ReadOnlyCollection([.. list]); + } + + return new ReadOnlyDictionary>(snapshot); + } + + private static bool DetailsEqual( + IReadOnlyDictionary>? a, + IReadOnlyDictionary>? b) + { + if (a is null && b is null) + { + return true; + } + + if (a is null || b is null || a.Count != b.Count) + { + return false; + } + + foreach (var (key, listA) in a) + { + if (!b.TryGetValue(key, out var listB) || listA.Count != listB.Count) + { + return false; + } + + for (var i = 0; i < listA.Count; i++) + { + if (!listA[i].Equals(listB[i])) + { + return false; + } + } + } + + return true; + } } diff --git a/backend/src/Modules/Directory.Build.props b/backend/src/Modules/Directory.Build.props new file mode 100644 index 0000000..c0109f8 --- /dev/null +++ b/backend/src/Modules/Directory.Build.props @@ -0,0 +1,35 @@ + + + + + + + + + + + + diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestId.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestId.cs index 5739698..05a2774 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestId.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Domain/TestId.cs @@ -1,3 +1,4 @@ +using LearnStack.SharedKernel; using LearnStack.SharedKernel.Identifiers; using Vogen; diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs index fed2ca1..4786e61 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/ErrorTests.cs @@ -24,6 +24,14 @@ public void Code_ForValidationFailed_MatchesStandards09Catalogue() error.Code.Should().Be("validation_failed"); } + [Fact] + public void Ctor_NullMessage_Throws() + { + var act = () => new Error(message: null!); + + act.Should().Throw(); + } + [Fact] public void Details_AreOptional() { @@ -35,8 +43,6 @@ public void Details_AreOptional() [Fact] public void Details_CarryFieldLevelLocalizedMessages() { - // Per the review: field-level errors must also flow as LocalizedMessages - // so the lockey_ invariant covers every user-facing string the API ships. var details = new Dictionary> { ["email"] = @@ -53,4 +59,71 @@ public void Details_CarryFieldLevelLocalizedMessages() error.Details.Should().BeEquivalentTo(details); error.Details!["email"].Should().HaveCount(2); } + + [Fact] + public void Details_AreDefensivelyCopied() + { + // Caller mutation after construction must not leak into the Error; + // Problem Details bodies, audit rows, and logs all read Details + // after the handler returns, and divergent state across those + // sinks would be a real bug. + var emailErrors = new List + { + LocalizedMessage.Of("lockey_email_required"), + }; + var details = new Dictionary> + { + ["email"] = emailErrors, + }; + + var error = new Error(LocalizedMessage.Of("lockey_validation_failed"), details); + + // Mutate the caller's collections after the Error was constructed. + emailErrors.Add(LocalizedMessage.Of("lockey_email_invalid")); + details["password"] = [LocalizedMessage.Of("lockey_password_required")]; + + error.Details!["email"].Should().HaveCount(1, "the snapshot was taken at ctor time"); + error.Details.ContainsKey("password").Should().BeFalse(); + } + + [Fact] + public void Equality_IsStructural_AcrossDetails() + { + // Record equality on IReadOnlyDictionary defaults to reference + // equality; the override compares Message + Details key-by-key. + var detailsA = new Dictionary> + { + ["email"] = [LocalizedMessage.Of("lockey_email_invalid")], + }; + var detailsB = new Dictionary> + { + ["email"] = [LocalizedMessage.Of("lockey_email_invalid")], + }; + + var a = new Error(LocalizedMessage.Of("lockey_validation_failed"), detailsA); + var b = new Error(LocalizedMessage.Of("lockey_validation_failed"), detailsB); + + a.Equals(b).Should().BeTrue(); + a.GetHashCode().Should().Be(b.GetHashCode()); + } + + [Fact] + public void Equality_DifferentDetails_AreNotEqual() + { + var a = new Error( + LocalizedMessage.Of("lockey_validation_failed"), + new Dictionary> + { + ["email"] = [LocalizedMessage.Of("lockey_email_invalid")], + }); + + var b = new Error( + LocalizedMessage.Of("lockey_validation_failed"), + new Dictionary> + { + ["email"] = [LocalizedMessage.Of("lockey_email_required")], + }); + + a.Equals(b).Should().BeFalse(); + } } diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/FixedGuidFactoryTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/FixedGuidFactoryTests.cs index d4ab277..d44f22e 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/FixedGuidFactoryTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Identifiers/FixedGuidFactoryTests.cs @@ -38,4 +38,18 @@ public void NewUuidV4_DrawsFromTheSameSequence() factory.NewUuidV4().Should().Be(G1); factory.NewUuidV4().Should().Be(G2); } + + [Fact] + public void MixedV7AndV4_PreservesVersionPerSeed() + { + // The fixture does not synthesise version-7/-4 shapes — callers + // seed version-appropriate Guids and the fixture returns them + // verbatim. Locks the documented shared-queue contract. + var v7 = Guid.CreateVersion7(); + var v4 = Guid.NewGuid(); + var factory = new FixedGuidFactory(v7, v4); + + factory.NewUuidV7().Version.Should().Be(7); + factory.NewUuidV4().Version.Should().Be(4); + } } diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs index f4ffbbb..03b466c 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs @@ -19,42 +19,49 @@ public void DefaultLimit_MatchesStandards04() [InlineData(-1)] public void Ctor_LimitNonPositive_Throws(int limit) { - // The kernel-level guard belongs at construction so the invariant - // cannot be skipped by callers that forget to call Normalised(). + // Kernel-level programmer-error guard. The API-layer validation + // turns malformed user input into validation_failed BEFORE the + // kernel sees the request. var act = () => new CursorPagination(Limit: limit); act.Should().Throw(); } [Fact] - public void Normalised_LimitAboveMax_ClampsToMaxLimit() + public void Ctor_LimitAboveMax_ClampsToMaxLimit() { + // Construction is the single chokepoint for the invariant - any + // CursorPagination instance is guaranteed to have Limit in + // [1, MaxLimit] without callers having to remember a separate + // normalisation step (the previous Normalised() method). var request = new CursorPagination(Limit: 500); - request.Normalised().Limit.Should().Be(CursorPagination.MaxLimit); + request.Limit.Should().Be(CursorPagination.MaxLimit); } [Fact] - public void Normalised_LimitWithinBounds_ReturnsEquivalentInstance() + public void Ctor_LimitEqualMax_IsAccepted() { - var request = new CursorPagination(Cursor: "abc", Limit: 50); - - var normalised = request.Normalised(); + // Off-by-one boundary: MaxLimit itself is legal, not above. + var request = new CursorPagination(Limit: CursorPagination.MaxLimit); - normalised.Cursor.Should().Be(request.Cursor); - normalised.Limit.Should().Be(request.Limit); + request.Limit.Should().Be(CursorPagination.MaxLimit); } [Fact] - public void Normalised_NeverThrows_BecauseCtorValidatesInput() + public void Ctor_LimitOne_IsAccepted() { - // After ctor validation, the only thing left to normalise is the - // upper-bound clamp. Calling Normalised() on any constructed - // instance is safe. - var request = new CursorPagination(Limit: CursorPagination.MaxLimit + 1); + var request = new CursorPagination(Limit: 1); + + request.Limit.Should().Be(1); + } - var act = () => request.Normalised(); + [Fact] + public void Ctor_CarriesCursor() + { + var request = new CursorPagination(Cursor: "abc", Limit: 50); - act.Should().NotThrow(); + request.Cursor.Should().Be("abc"); + request.Limit.Should().Be(50); } } diff --git a/docs/architecture/15-event-and-outbox.md b/docs/architecture/15-event-and-outbox.md index 1da3038..4c4cda6 100644 --- a/docs/architecture/15-event-and-outbox.md +++ b/docs/architecture/15-event-and-outbox.md @@ -156,7 +156,7 @@ public async Task> Handle(CreateEnrollmentCommand cmd, Can EnrollmentId = enrollment.Id.Value, LearnerId = cmd.LearnerId, CourseId = cmd.CourseId, - OccurredAt = DateTime.UtcNow + OccurredAt = _clock.UtcNow, // IClock per Standards 02 § Time }, ct); await _dbContext.SaveChangesAsync(ct); // aggregate + outbox row, atomic diff --git a/docs/roadmap/phase-02a-kernel-tenancy.md b/docs/roadmap/phase-02a-kernel-tenancy.md index 8da420b..b6e2076 100644 --- a/docs/roadmap/phase-02a-kernel-tenancy.md +++ b/docs/roadmap/phase-02a-kernel-tenancy.md @@ -51,10 +51,11 @@ > `EfCoreValueConverter | SystemTextJson | TypeConverter` mask; > `IAggregateRoot` / `IHasId` interfaces require `TId : > IStronglyTypedId` so future module aggregates inherit the -> constraint. 54 unit tests cover the primitives; the +> constraint. 71 unit tests cover the primitives; the > `VogenIdEmissionTests` smoke test asserts the emitter pipeline > (Vogen `[ValueObject]` → `IStronglyTypedId.Value` → JSON -> round-trip) end-to-end via a synthetic `TestId` in the test project. +> round-trip → `TypeConverter` round-trip) end-to-end via a synthetic +> `TestId` in the test project. > Review pass folded in (commit `7c9133a`): `Entity` equality carries > transient + cross-runtime-type guards; `Result.FailFor` returns > the concrete `TResponse` via reflection (not `Result`); @@ -67,7 +68,7 @@ > `SoftDelete` bumps `UpdatedAt` for monotonic last-touched; > `CursorPagination` validates `Limit > 0` at the ctor. Standards 01 > § Dependency Direction grows a "Build-time-only exceptions" sub-section -> for the EF Core + MediatR references SharedKernel requires. 64 unit +> for the EF Core + MediatR references SharedKernel requires. 71 unit > tests + 17 architecture tests green. > > **Packet 3 — Cross-cutting foundation ⏳** diff --git a/docs/standards/02-backend-coding.md b/docs/standards/02-backend-coding.md index f1f1f1b..3edfb61 100644 --- a/docs/standards/02-backend-coding.md +++ b/docs/standards/02-backend-coding.md @@ -41,22 +41,32 @@ C# / .NET conventions for LearnStack backend code. ## Strongly-Typed Identifiers -```csharp -public readonly record struct CourseId(Guid Value) : IStronglyTypedId -{ - public static CourseId New() => new(Guid.NewGuid()); - public override string ToString() => Value.ToString(); -} -``` - Per [ADR-0023](../decisions/0023-strongly-typed-id-source-generator.md), the shared source generator is **[Vogen](https://github.com/SteveDunn/Vogen)**. The canonical declaration uses Vogen's `[ValueObject(...)]` annotation on a -partial `record struct`; Vogen emits: +partial `record struct`: + +```csharp +[ValueObject(LearnStackVogenDefaults.IdMask)] +public readonly partial record struct CourseId : IStronglyTypedId; +``` + +Vogen emits per ID: - EF Core value converter. -- `JsonConverter`. -- Minimal API model binder. -- OpenAPI schema mapping (Swashbuckle / Microsoft.OpenApi schema filter). +- `JsonConverter` (System.Text.Json). +- TypeConverter (carries ASP.NET Core minimal-API + MVC route-parameter binding). +- OpenAPI schema mapping (wired centrally in Packet 4 per ADR-0023 § Implementation + Notes). + +Construction: +- New IDs in aggregate methods mint via the injected `IGuidFactory`: + `CourseId.From(guidFactory.NewUuidV7())`. **Never call `Guid.CreateVersion7()` / + `Guid.NewGuid()` directly in `Domain` / `Application` code** — Standards 02 + § Time bans the symmetric `DateTime.UtcNow` for the same reason (deterministic + tests). High-volume append-only tables (`audit_log`, `outbox_messages`) prefer + DB-side `gen_uuid_v7()` (per ADR-0031). +- ID types do **not** expose a `New()` static — explicit `From(guidFactory.NewUuidV7())` + at the call site keeps the dependency surface honest. The same annotation covers richer value objects (`Email`, `Slug`, `LocaleCode`, `Money`) — the emitter shape is identical for IDs and value objects, with the diff --git a/docs/standards/09-error-handling.md b/docs/standards/09-error-handling.md index ef7ddb9..ce38d92 100644 --- a/docs/standards/09-error-handling.md +++ b/docs/standards/09-error-handling.md @@ -196,26 +196,35 @@ All API errors are **RFC 7807 Problem Details**: ```json { "type": "https://errors.learnstack.dev/validation", - "title": "Validation failed", + "title": "lockey_validation_failed", "status": 400, "code": "validation_failed", - "detail": "One or more fields are invalid.", + "messageKey": "lockey_validation_failed", "instance": "/v1/courses", "correlationId": "01H...", "errors": { - "title": ["Title is required."], - "slug": ["Slug already exists in this tenant."] + "title": [ + { "key": "lockey_title_required" } + ], + "slug": [ + { "key": "lockey_slug_already_exists_in_tenant", "params": { "slug": "intro" } } + ] } } ``` Rules: - `type` is a stable URL. -- `code` is the machine-readable identifier. -- `detail` is human-readable but **safe to display** (no internal info). +- `code` is the machine-readable identifier — the unprefixed `Error.Code` + (Standards 04 § Problem Details). +- `messageKey` is the `LocalizedMessage.Key` (always begins with `lockey_`) + the frontend resolves against its i18n catalogue. The legacy + `detail` field is omitted — backend never returns raw English. - `instance` is the request path. - `correlationId` matches the trace id. -- `errors` is field-level detail (validation only). +- `errors` is field-level detail, each entry a `LocalizedMessage` payload + (`key` + optional `params`) so the frontend resolves field-level messages + through the same path as the top-level one. ## Validation Errors From cc4435c04a77877f3a253754e37eee0871998515 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Thu, 21 May 2026 15:55:20 +0300 Subject: [PATCH 7/8] =?UTF-8?q?fix(shared-kernel):=20packet=202=20PR=20#6?= =?UTF-8?q?=20review-4=20=E2=80=94=20init-path=20guards=20+=20per-element?= =?UTF-8?q?=20null=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All 5 findings from the fourth review pass verified valid against current code (no skips). - CursorPagination.Limit invariant moved from the constructor body into the property's `init` accessor. The previous shape let callers bypass the guard via object-initializer syntax (`new CursorPagination { Limit = 0 }`) and the record's `with` expression (`request with { Limit = 0 }`), because both run AFTER the constructor and write to the public init setter directly. Validating in `init` covers every initialisation path with a single chokepoint. Backing field stays private; the ctor delegates to the property. - Error.SnapshotDetails null-checks each LocalizedMessage element, not just the list reference. A null entry inside a per-field list would later NPE in `GetHashCode` (`msg.GetHashCode()`) or `DetailsEqual` (`listA[i].Equals(listB[i])`); failing fast at construction with the offending key + index in the ArgumentException message gives the caller the locator instead of a downstream NullReferenceException. - CursorPaginationTests gains regression coverage for the init paths: - `ObjectInitializer_AndWithExpression_PreserveValuesAndInvariants` constructs via object initializer and `with`, asserts both shape invariants hold. - `ObjectInitializer_ZeroLimit_Throws` and `WithExpression_ZeroLimit_Throws` lock the guard on both init paths. - `ObjectInitializer_AboveMaxLimit_Clamps` verifies the clamp also fires through the init accessor, not just the ctor. - Phase 02a roadmap: removed the stale literal "71 unit tests" mentions (two occurrences). The roadmap now references "Unit / architecture / contract suites all green in CI" instead of pinning a number that drifts with every review-driven test addition. CI summary is the authoritative count. - Standards 02 § Derives from now lists ADR-0031 (PostgreSQL major version) alongside ADR-0002 / 0006 / 0023; the inline `gen_uuid_v7()` reference is a proper markdown link to the ADR rather than plain text. Tests: 81 unit (+4) + 17 architecture + 1 contract green under CI=true. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Pagination/CursorPagination.cs | 45 ++++++++++++------- .../LearnStack.SharedKernel/Results/Error.cs | 16 ++++++- .../Pagination/CursorPaginationTests.cs | 43 ++++++++++++++++++ docs/roadmap/phase-02a-kernel-tenancy.md | 6 +-- docs/standards/02-backend-coding.md | 4 +- 5 files changed, 91 insertions(+), 23 deletions(-) diff --git a/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs b/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs index e06366a..3206625 100644 --- a/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs +++ b/backend/src/LearnStack.SharedKernel/Pagination/CursorPagination.cs @@ -5,17 +5,19 @@ namespace LearnStack.SharedKernel.Pagination; /// default for every list endpoint. The is an opaque /// token the server minted on a previous response; the client never parses /// it. defaults to and is -/// capped at by the constructor. +/// capped at . /// /// -/// Construction enforces every invariant: non-positive limits throw -/// (kernel-level programmer-error guard), above-max limits are silently -/// clamped to . There is no normalisation step a -/// caller can forget — once a exists, its -/// is always in [1, MaxLimit]. API-layer +/// The invariant lives in the property's init +/// accessor, not in the constructor, so it covers every initialisation +/// path: the constructor, object-initializer syntax +/// (new CursorPagination { Limit = 0 }), and the record's +/// with expression (request with { Limit = 0 }). Non-positive +/// limits throw (kernel-level programmer-error guard); above-max limits +/// are silently clamped to . API-layer /// FluentValidation should still turn malformed user input into /// Result.Fail(validation_failed) before the kernel sees the -/// request; the ctor guards are the last line of defense, not the first. +/// request; the kernel guards are the last line of defense, not the first. /// public sealed record CursorPagination { @@ -23,21 +25,30 @@ public sealed record CursorPagination public const int MaxLimit = 100; + private readonly int _limit = DefaultLimit; + public CursorPagination(string? Cursor = null, int Limit = DefaultLimit) { - if (Limit <= 0) - { - throw new ArgumentOutOfRangeException( - nameof(Limit), - Limit, - $"Limit must be > 0. Got: {Limit}."); - } - this.Cursor = Cursor; - this.Limit = Limit > MaxLimit ? MaxLimit : Limit; + this.Limit = Limit; } public string? Cursor { get; init; } - public int Limit { get; init; } + public int Limit + { + get => _limit; + init + { + if (value <= 0) + { + throw new ArgumentOutOfRangeException( + nameof(value), + value, + $"Limit must be > 0. Got: {value}."); + } + + _limit = value > MaxLimit ? MaxLimit : value; + } + } } diff --git a/backend/src/LearnStack.SharedKernel/Results/Error.cs b/backend/src/LearnStack.SharedKernel/Results/Error.cs index 3d9847b..770fede 100644 --- a/backend/src/LearnStack.SharedKernel/Results/Error.cs +++ b/backend/src/LearnStack.SharedKernel/Results/Error.cs @@ -123,7 +123,21 @@ public override int GetHashCode() foreach (var (key, list) in source) { ArgumentNullException.ThrowIfNull(list); - snapshot[key] = new ReadOnlyCollection([.. list]); + + // Per-element null check: a null inside the list would later NPE + // in GetHashCode / DetailsEqual. Fail fast at construction with + // the offending key in the message so the caller can locate the + // bug rather than chasing a NullReferenceException down the + // serialisation path. + var copy = new LocalizedMessage[list.Count]; + for (var i = 0; i < list.Count; i++) + { + copy[i] = list[i] ?? throw new ArgumentException( + $"Error.Details['{key}'][{i}] is null. Every field-level entry must be a non-null LocalizedMessage.", + nameof(source)); + } + + snapshot[key] = new ReadOnlyCollection(copy); } return new ReadOnlyDictionary>(snapshot); diff --git a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs index 03b466c..96e3458 100644 --- a/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs +++ b/backend/tests/LearnStack.Tests.Unit/SharedKernel/Pagination/CursorPaginationTests.cs @@ -64,4 +64,47 @@ public void Ctor_CarriesCursor() request.Cursor.Should().Be("abc"); request.Limit.Should().Be(50); } + + [Fact] + public void ObjectInitializer_AndWithExpression_PreserveValuesAndInvariants() + { + // The Limit invariant lives in the init accessor, not the + // constructor, so it covers object-initializer syntax AND the + // record's `with` expression - neither can bypass the guard the + // ctor would otherwise be the only enforcer of. + var fromInit = new CursorPagination { Cursor = "abc", Limit = 50 }; + var fromWith = fromInit with { Limit = 75 }; + + fromInit.Cursor.Should().Be("abc"); + fromInit.Limit.Should().Be(50); + + fromWith.Cursor.Should().Be("abc"); + fromWith.Limit.Should().Be(75); + } + + [Fact] + public void ObjectInitializer_ZeroLimit_Throws() + { + var act = () => new CursorPagination { Limit = 0 }; + + act.Should().Throw(); + } + + [Fact] + public void WithExpression_ZeroLimit_Throws() + { + var request = new CursorPagination(Limit: 50); + + var act = () => request with { Limit = 0 }; + + act.Should().Throw(); + } + + [Fact] + public void ObjectInitializer_AboveMaxLimit_Clamps() + { + var request = new CursorPagination { Limit = 500 }; + + request.Limit.Should().Be(CursorPagination.MaxLimit); + } } diff --git a/docs/roadmap/phase-02a-kernel-tenancy.md b/docs/roadmap/phase-02a-kernel-tenancy.md index b6e2076..a8cf727 100644 --- a/docs/roadmap/phase-02a-kernel-tenancy.md +++ b/docs/roadmap/phase-02a-kernel-tenancy.md @@ -51,7 +51,7 @@ > `EfCoreValueConverter | SystemTextJson | TypeConverter` mask; > `IAggregateRoot` / `IHasId` interfaces require `TId : > IStronglyTypedId` so future module aggregates inherit the -> constraint. 71 unit tests cover the primitives; the +> constraint. Unit / architecture / contract suites all green in CI; the > `VogenIdEmissionTests` smoke test asserts the emitter pipeline > (Vogen `[ValueObject]` → `IStronglyTypedId.Value` → JSON > round-trip → `TypeConverter` round-trip) end-to-end via a synthetic @@ -68,8 +68,8 @@ > `SoftDelete` bumps `UpdatedAt` for monotonic last-touched; > `CursorPagination` validates `Limit > 0` at the ctor. Standards 01 > § Dependency Direction grows a "Build-time-only exceptions" sub-section -> for the EF Core + MediatR references SharedKernel requires. 71 unit -> tests + 17 architecture tests green. +> for the EF Core + MediatR references SharedKernel requires. Unit / +> architecture / contract suites all green in CI. > > **Packet 3 — Cross-cutting foundation ⏳** > Wires the [ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md) diff --git a/docs/standards/02-backend-coding.md b/docs/standards/02-backend-coding.md index 3edfb61..c58c4b8 100644 --- a/docs/standards/02-backend-coding.md +++ b/docs/standards/02-backend-coding.md @@ -1,7 +1,7 @@ # 02 — Backend Coding Standards **Status:** Active -**Derives from:** [ADR 0002 — Initial Architecture](../decisions/0002-initial-architecture.md), [ADR 0006 — Events and Outbox](../decisions/0006-events-and-outbox.md), [ADR 0023 — Strongly-Typed ID Source Generator](../decisions/0023-strongly-typed-id-source-generator.md). +**Derives from:** [ADR 0002 — Initial Architecture](../decisions/0002-initial-architecture.md), [ADR 0006 — Events and Outbox](../decisions/0006-events-and-outbox.md), [ADR 0023 — Strongly-Typed ID Source Generator](../decisions/0023-strongly-typed-id-source-generator.md), [ADR 0031 — PostgreSQL Major Version](../decisions/0031-postgresql-major-version.md). C# / .NET conventions for LearnStack backend code. @@ -64,7 +64,7 @@ Construction: `Guid.NewGuid()` directly in `Domain` / `Application` code** — Standards 02 § Time bans the symmetric `DateTime.UtcNow` for the same reason (deterministic tests). High-volume append-only tables (`audit_log`, `outbox_messages`) prefer - DB-side `gen_uuid_v7()` (per ADR-0031). + DB-side `gen_uuid_v7()` (per [ADR-0031](../decisions/0031-postgresql-major-version.md)). - ID types do **not** expose a `New()` static — explicit `From(guidFactory.NewUuidV7())` at the call site keeps the dependency surface honest. From b82acc2032811a394aef52dce1db96729924b970 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Thu, 21 May 2026 17:18:14 +0300 Subject: [PATCH 8/8] =?UTF-8?q?docs(standards):=20packet=202=20PR=20#6=20r?= =?UTF-8?q?eview-5=20=E2=80=94=20Types-table=20CourseId=20example=20aligns?= =?UTF-8?q?=20with=20Vogen=20pattern?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding verified valid against current code: standards/02 line 39 (Types table) still showed `record struct CourseId(Guid Value) : IStronglyTypedId`, the pre-Vogen positional form, while § Strongly-Typed Identifiers immediately below already uses the Vogen canonical shape (`[ValueObject(LearnStackVogenDefaults.IdMask)] partial record struct`). A reader scanning the Types table would copy the wrong form. Replaced the bullet's inline example with `partial record struct CourseId : IStronglyTypedId;` and added a same-doc anchor link to the detailed § Strongly-Typed Identifiers section so readers land on the full Vogen pattern (attribute + From() factory + IGuidFactory rule) without ambiguity. Doc-only change; no build/test impact. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/standards/02-backend-coding.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/standards/02-backend-coding.md b/docs/standards/02-backend-coding.md index c58c4b8..705a4b2 100644 --- a/docs/standards/02-backend-coding.md +++ b/docs/standards/02-backend-coding.md @@ -36,7 +36,7 @@ C# / .NET conventions for LearnStack backend code. - **Records** for immutable value-like data: DTOs, integration events, configuration options. - **Sealed classes** by default; open inheritance is the exception. - **Structs** only for small, immutable, frequently-allocated values (≤ 16 bytes). -- **Strongly-typed ids** (`record struct CourseId(Guid Value) : IStronglyTypedId`) for all entity identifiers. Never expose raw `Guid` on the public surface. +- **Strongly-typed ids** (`partial record struct CourseId : IStronglyTypedId;` per the [Vogen pattern below](#strongly-typed-identifiers)) for all entity identifiers. Never expose raw `Guid` on the public surface. - **Value objects** for domain concepts with invariants (e.g. `Email`, `Slug`, `LocaleCode`). ## Strongly-Typed Identifiers