diff --git a/src/Persistence/PolecatTests/Bugs/Bug_3942_nullable_aggregate_id_type.cs b/src/Persistence/PolecatTests/Bugs/Bug_3942_nullable_aggregate_id_type.cs new file mode 100644 index 0000000000..6dfa6024fa --- /dev/null +++ b/src/Persistence/PolecatTests/Bugs/Bug_3942_nullable_aggregate_id_type.cs @@ -0,0 +1,121 @@ +using Shouldly; +using Wolverine.Polecat.Persistence.Sagas; +using Wolverine.Polecat.Requirements; + +namespace PolecatTests.Bugs; + +/// +/// GH-3942: the two stores have to agree on an aggregate's id type. +/// +/// +/// WriteModelAttribute.Modify asks the provider for the aggregate's id type and +/// FindIdentity branches on whether the answer is primitive. Marten answers with the +/// configured document id type, which is never nullable; Polecat reflected the Id +/// property verbatim, so public AlertId? Id { get; set; } answered +/// Nullable<AlertId>. That is not primitive, so the IdentifiedBy<T> +/// escape hatch was skipped and the message was scanned for a Nullable<AlertId> +/// member — which exists in no codebase. Every such chain failed to build under Polecat while +/// the identical shared source worked under Marten. +/// +/// These are deliberately reflection-level rather than integration tests: the failure is at +/// codegen time, and under TypeLoadMode.Dynamic a chain only fails when its message first +/// arrives — so an integration test would have to dispatch every message type to see it. +/// +/// +public class Bug_3942_nullable_aggregate_id_type +{ + private static Type sagaIdType() => + new PolecatPersistenceFrameProvider().DetermineSagaIdType(typeof(T), null!); + + // ===== The reported case ===== + + [Fact] + public void nullable_strong_typed_id_is_unwrapped() + { + sagaIdType().ShouldBe(typeof(AlertId)); + } + + [Fact] + public void nullable_primitive_id_is_unwrapped() + { + sagaIdType().ShouldBe(typeof(Guid)); + } + + // ===== Everything else is untouched ===== + + [Fact] + public void non_nullable_strong_typed_id_is_unchanged() + { + sagaIdType().ShouldBe(typeof(AlertId)); + } + + [Fact] + public void non_nullable_primitive_id_is_unchanged() + { + sagaIdType().ShouldBe(typeof(Guid)); + } + + // A string id is already a reference type, so it was never wrapped -- and `string?` erases to + // string at runtime, which is exactly why the bug only bit value-type ids. + [Fact] + public void string_id_is_unchanged() + { + sagaIdType().ShouldBe(typeof(string)); + } + + [Fact] + public void a_type_with_no_id_property_still_falls_back_to_guid() + { + sagaIdType().ShouldBe(typeof(Guid)); + } + + // ===== DocumentExistsAttribute.ResolveIdType is kept in lockstep ===== + + [Fact] + public void document_id_resolution_agrees_with_the_saga_provider() + { + DocumentExistsAttribute + .ResolveIdType(typeof(NullableStrongTypedIdAggregate)).ShouldBe(typeof(AlertId)); + + DocumentExistsAttribute + .ResolveIdType(typeof(NullableGuidAggregate)).ShouldBe(typeof(Guid)); + + DocumentExistsAttribute + .ResolveIdType(typeof(GuidAggregate)).ShouldBe(typeof(Guid)); + + DocumentExistsAttribute + .ResolveIdType(typeof(NoIdAggregate)).ShouldBe(typeof(Guid)); + } +} + +public readonly record struct AlertId(string Value); + +public class NullableStrongTypedIdAggregate +{ + public AlertId? Id { get; set; } +} + +public class StrongTypedIdAggregate +{ + public AlertId Id { get; set; } +} + +public class NullableGuidAggregate +{ + public Guid? Id { get; set; } +} + +public class GuidAggregate +{ + public Guid Id { get; set; } +} + +public class StringAggregate +{ + public string Id { get; set; } = string.Empty; +} + +public class NoIdAggregate +{ + public string Name { get; set; } = string.Empty; +} diff --git a/src/Persistence/Wolverine.Polecat/Persistence/Sagas/PolecatPersistenceFrameProvider.cs b/src/Persistence/Wolverine.Polecat/Persistence/Sagas/PolecatPersistenceFrameProvider.cs index 6e8202c042..e77535bbc1 100644 --- a/src/Persistence/Wolverine.Polecat/Persistence/Sagas/PolecatPersistenceFrameProvider.cs +++ b/src/Persistence/Wolverine.Polecat/Persistence/Sagas/PolecatPersistenceFrameProvider.cs @@ -33,7 +33,16 @@ public bool CanPersist(Type entityType, IServiceContainer container, out Type pe public Type DetermineSagaIdType(Type sagaType, IServiceContainer container) { var idProp = sagaType.GetProperty("Id", BindingFlags.Public | BindingFlags.Instance); - return idProp?.PropertyType ?? typeof(Guid); + if (idProp == null) return typeof(Guid); + + // GH-3942: unwrap Nullable, so the two stores agree on an aggregate's id type. + // Marten answers with the *configured document id type*, which is never nullable. Reflecting + // the property verbatim answered Nullable for `public AlertId? Id { get; set; }`, + // and WriteModelAttribute.FindIdentity treats that as non-primitive -- so it skipped the + // IdentifiedBy escape hatch entirely and went looking for a Nullable member on + // the message, which exists nowhere. Every such chain failed to build under Polecat while + // the identical source worked under Marten. + return Nullable.GetUnderlyingType(idProp.PropertyType) ?? idProp.PropertyType; } public void ApplyTransactionSupport(IChain chain, IServiceContainer container) diff --git a/src/Persistence/Wolverine.Polecat/Requirements/DocumentExistsAttribute.cs b/src/Persistence/Wolverine.Polecat/Requirements/DocumentExistsAttribute.cs index 2d51f22b23..f1c4547bdf 100644 --- a/src/Persistence/Wolverine.Polecat/Requirements/DocumentExistsAttribute.cs +++ b/src/Persistence/Wolverine.Polecat/Requirements/DocumentExistsAttribute.cs @@ -69,13 +69,18 @@ public override void Modify(IChain chain, GenerationRules rules, IServiceContain /// /// Resolve the identity type for a Polecat document type by reflecting on the public - /// instance Id property. Falls back to when the property is - /// missing — matching . + /// instance Id property. A nullable id property yields its underlying type, and a + /// missing one falls back to — matching + /// . /// internal static Type ResolveIdType(Type docType) { var idProp = docType.GetProperty("Id", BindingFlags.Public | BindingFlags.Instance); - return idProp?.PropertyType ?? typeof(Guid); + if (idProp == null) return typeof(Guid); + + // GH-3942: kept in lockstep with DetermineSagaIdType. See the note there for why the + // verbatim property type is wrong for `public MyId? Id { get; set; }`. + return Nullable.GetUnderlyingType(idProp.PropertyType) ?? idProp.PropertyType; } internal static bool TryFindIdentityVariable(IChain chain, string? argumentName, Type docType, Type idType,