FetchForWriting/FetchLatest accept a strong-typed identifier (#5144)

src/EventSourcingTests/Bugs/Bug_5144_strong_typed_id_fetch_overloads.cs

@@ -0,0 +1,152 @@
+using System;
+using System.Threading.Tasks;
+using JasperFx.Events;
+using JasperFx.Events.Projections;
+using Marten.Testing.Harness;
+using Shouldly;
+using Xunit;
+
+namespace EventSourcingTests.Bugs;
+
+// #5144: FetchForWriting<T, TId> / FetchForExclusiveWriting / FetchLatest accept a strong-typed
+// identifier. Before the fix, a TId that is neither Guid nor string planned down the natural-key
+// branch, which passes a null identity strategy; the lifecycle planners matched anyway and stored
+// the null, so the call died with a bare NullReferenceException.
+public readonly record struct Bug5144PaymentId(Guid Value);
+
+public readonly record struct Bug5144InvoiceId(string Value);
+
+public record Bug5144PaymentRaised(decimal Amount);
+
+public record Bug5144PaymentSettled(decimal Amount);
+
+public class Bug5144Payment
+{
+    public Bug5144PaymentId Id { get; set; }
+    public decimal Outstanding { get; set; }
+
+    public static Bug5144Payment Create(IEvent<Bug5144PaymentRaised> e)
+        => new() { Id = new Bug5144PaymentId(e.StreamId), Outstanding = e.Data.Amount };
+
+    public void Apply(Bug5144PaymentSettled e) => Outstanding -= e.Amount;
+}
+
+public class Bug5144Invoice
+{
+    public Bug5144InvoiceId Id { get; set; }
+    public decimal Outstanding { get; set; }
+
+    public static Bug5144Invoice Create(IEvent<Bug5144PaymentRaised> e)
+        => new() { Id = new Bug5144InvoiceId(e.StreamKey!), Outstanding = e.Data.Amount };
+
+    public void Apply(Bug5144PaymentSettled e) => Outstanding -= e.Amount;
+}
+
+public class Bug_5144_strong_typed_id_fetch_overloads: OneOffConfigurationsContext
+{
+    [Theory]
+    [InlineData(SnapshotLifecycle.Inline)]
+    [InlineData(SnapshotLifecycle.Async)]
+    public async Task fetch_for_writing_by_a_guid_backed_strong_typed_id(SnapshotLifecycle lifecycle)
+    {
+        StoreOptions(opts =>
+        {
+            opts.RegisterValueType<Bug5144PaymentId>();
+            opts.Projections.Snapshot<Bug5144Payment>(lifecycle);
+        });
+
+        var streamId = theSession.Events
+            .StartStream<Bug5144Payment>(new Bug5144PaymentRaised(100m)).Id;
+        await theSession.SaveChangesAsync();
+
+        await using var session = theStore.LightweightSession();
+        var stream = await session.Events
+            .FetchForWriting<Bug5144Payment, Bug5144PaymentId>(new Bug5144PaymentId(streamId));
+
+        stream.Aggregate.ShouldNotBeNull();
+        stream.Aggregate.Id.Value.ShouldBe(streamId);
+        stream.Aggregate.Outstanding.ShouldBe(100m);
+    }
+
+    [Fact]
+    public async Task fetch_for_exclusive_writing_by_a_strong_typed_id()
+    {
+        StoreOptions(opts =>
+        {
+            opts.RegisterValueType<Bug5144PaymentId>();
+            opts.Projections.Snapshot<Bug5144Payment>(SnapshotLifecycle.Inline);
+        });
+
+        var streamId = theSession.Events
+            .StartStream<Bug5144Payment>(new Bug5144PaymentRaised(60m)).Id;
+        await theSession.SaveChangesAsync();
+
+        await using var session = theStore.LightweightSession();
+        var stream = await session.Events
+            .FetchForExclusiveWriting<Bug5144Payment, Bug5144PaymentId>(new Bug5144PaymentId(streamId));
+
+        stream.Aggregate.ShouldNotBeNull();
+        stream.Aggregate.Outstanding.ShouldBe(60m);
+    }
+
+    [Fact]
+    public async Task fetch_latest_by_a_strong_typed_id()
+    {
+        StoreOptions(opts =>
+        {
+            opts.RegisterValueType<Bug5144PaymentId>();
+            opts.Projections.Snapshot<Bug5144Payment>(SnapshotLifecycle.Inline);
+        });
+
+        var streamId = theSession.Events
+            .StartStream<Bug5144Payment>(new Bug5144PaymentRaised(100m), new Bug5144PaymentSettled(40m)).Id;
+        await theSession.SaveChangesAsync();
+
+        await using var session = theStore.LightweightSession();
+        var payment = await session.Events
+            .FetchLatest<Bug5144Payment, Bug5144PaymentId>(new Bug5144PaymentId(streamId));
+
+        payment.ShouldNotBeNull();
+        payment.Outstanding.ShouldBe(60m);
+    }
+
+    [Fact]
+    public async Task fetch_for_writing_by_a_string_backed_strong_typed_id()
+    {
+        StoreOptions(opts =>
+        {
+            opts.Events.StreamIdentity = StreamIdentity.AsString;
+            opts.RegisterValueType<Bug5144InvoiceId>();
+            opts.Projections.Snapshot<Bug5144Invoice>(SnapshotLifecycle.Inline);
+        });
+
+        var key = "invoice/" + Guid.NewGuid().ToString("N");
+        theSession.Events.StartStream<Bug5144Invoice>(key, new Bug5144PaymentRaised(25m));
+        await theSession.SaveChangesAsync();
+
+        await using var session = theStore.LightweightSession();
+        var stream = await session.Events
+            .FetchForWriting<Bug5144Invoice, Bug5144InvoiceId>(new Bug5144InvoiceId(key));
+
+        stream.Aggregate.ShouldNotBeNull();
+        stream.Aggregate.Id.Value.ShouldBe(key);
+        stream.Aggregate.Outstanding.ShouldBe(25m);
+    }
+
+    [Fact]
+    public async Task an_unknown_strong_typed_id_yields_an_empty_handle()
+    {
+        StoreOptions(opts =>
+        {
+            opts.RegisterValueType<Bug5144PaymentId>();
+            opts.Projections.Snapshot<Bug5144Payment>(SnapshotLifecycle.Inline);
+        });
+
+        await using var session = theStore.LightweightSession();
+        var stream = await session.Events
+            .FetchForWriting<Bug5144Payment, Bug5144PaymentId>(new Bug5144PaymentId(Guid.NewGuid()));
+
+        stream.Aggregate.ShouldBeNull();
+        stream.StartingVersion.ShouldBe(0);
+    }
+}

src/Marten/Events/EventStore.FetchForWriting.cs

@@ -311,6 +311,29 @@ private IAggregateFetchPlan<TDoc, TId> determineFetchPlan<TDoc, TId>(StoreOption
                     return naturalKeyPlan;
                 }
             }
+
+            // #5144: not a natural key, but possibly a strong-typed identifier wrapping the stream
+            // identity -- PaymentId(Guid), InvoiceId(string) and friends. That *is* the stream id,
+            // so unwrap it and reuse the plan for the underlying type rather than inventing a
+            // parallel one. Only Guid and string backings can address a stream.
+            var valueType = options.TryFindValueType(typeof(TId));
+            if (valueType != null)
+            {
+                if (valueType.SimpleType == typeof(Guid))
+                {
+                    return new UnwrappedIdentityFetchPlan<TDoc, TId, Guid>(
+                        FindFetchPlan<TDoc, Guid>(), valueType.UnWrapper<TId, Guid>());
+                }
+
+                if (valueType.SimpleType == typeof(string))
+                {
+                    return new UnwrappedIdentityFetchPlan<TDoc, TId, string>(
+                        FindFetchPlan<TDoc, string>(), valueType.UnWrapper<TId, string>());
+                }
+
+                throw new InvalidOperationException(
+                    $"The strong-typed identifier {typeof(TId).FullNameInCode()} wraps {valueType.SimpleType.FullNameInCode()}, which cannot identify an event stream. Only Guid and string backed identifiers are supported here.");
+            }
         }
         else
         {

src/Marten/Events/Fetching/AsyncFetchPlanner.cs

@@ -16,6 +16,15 @@ public bool TryMatch<TDoc, TId>(IEventIdentityStrategy<TId> identity,
         StoreOptions options,
         [NotNullWhen(true)] out IAggregateFetchPlan<TDoc, TId>? plan) where TDoc : class where TId : notnull
     {
+        // #5144: a null identity strategy means planning came down the natural-key branch. These
+        // lifecycle planners match on the projection alone, so without this they would happily hand
+        // back a plan holding that null and fail later with a bare NullReferenceException.
+        if (identity is null)
+        {
+            plan = null;
+            return false;
+        }
+
         if (options.Projections.TryFindAggregate(typeof(TDoc), out var projection))
         {
             if (projection is MultiStreamProjection<TDoc, TId>)

src/Marten/Events/Fetching/InlineFetchPlanner.cs

@@ -11,6 +11,15 @@ public bool TryMatch<TDoc, TId>(IEventIdentityStrategy<TId> identity,
         StoreOptions options,
         [NotNullWhen(true)] out IAggregateFetchPlan<TDoc, TId>? plan) where TDoc : class where TId : notnull
     {
+        // #5144: a null identity strategy means planning came down the natural-key branch. These
+        // lifecycle planners match on the projection alone, so without this they would happily hand
+        // back a plan holding that null and fail later with a bare NullReferenceException.
+        if (identity is null)
+        {
+            plan = null;
+            return false;
+        }
+
         if (options.Projections.TryFindAggregate(typeof(TDoc), out var projection))
         {
             if (projection.Lifecycle == ProjectionLifecycle.Inline)

src/Marten/Events/Fetching/LiveFetchPlanner.cs

@@ -10,6 +10,15 @@ internal class LiveFetchPlanner: IFetchPlanner
     public bool TryMatch<TDoc, TId>(IEventIdentityStrategy<TId> identity,
         StoreOptions options, [NotNullWhen(true)] out IAggregateFetchPlan<TDoc, TId>? plan) where TDoc : class where TId : notnull
     {
+        // #5144: a null identity strategy means planning came down the natural-key branch. These
+        // lifecycle planners match on the projection alone, so without this they would happily hand
+        // back a plan holding that null and fail later with a bare NullReferenceException.
+        if (identity is null)
+        {
+            plan = null;
+            return false;
+        }
+
         IIdentitySetter<TDoc, TId> identitySetter = new NulloIdentitySetter<TDoc, TId>();
 
         // Yeah, this is smelly, but at least it would only happen *once* at runtime

src/Marten/Events/Fetching/UnwrappedIdentityFetchPlan.cs

@@ -0,0 +1,76 @@
+#nullable enable
+using System;
+using System.IO;
+using System.Threading;
+using System.Threading.Tasks;
+using JasperFx.Events;
+using JasperFx.Events.Projections;
+using Marten.Internal.Sessions;
+using Marten.Linq.QueryHandlers;
+
+namespace Marten.Events.Fetching;
+
+/// <summary>
+/// Adapts a fetch plan built for the raw stream identity so it can be addressed by a strong-typed
+/// identifier wrapping that identity.
+/// </summary>
+/// <remarks>
+/// <para>
+/// A strong-typed id such as <c>readonly record struct PaymentId(Guid Value)</c> is not a natural
+/// key — it *is* the stream identity, just wrapped. Before #5144 the generic
+/// <c>FetchForWriting&lt;T, TId&gt;</c> overloads had nowhere to put it: <c>TId</c> is neither
+/// <c>Guid</c> nor <c>string</c>, so planning fell into the natural-key branch, which passes a null
+/// identity strategy, and the lifecycle planners matched anyway and stored the null. The result was
+/// a bare <see cref="NullReferenceException"/> from inside the plan.
+/// </para>
+/// <para>
+/// Everything about the fetch is identical once the identity is unwrapped, so this forwards to the
+/// plan for the underlying type rather than duplicating any of it.
+/// <see cref="IAggregateFetchPlan{TDoc,TId}"/> is contravariant in <c>TId</c> and every member takes
+/// the identity as input, which is what makes a pure forwarder sufficient.
+/// </para>
+/// </remarks>
+internal class UnwrappedIdentityFetchPlan<TDoc, TId, TInner>: IAggregateFetchPlan<TDoc, TId>
+    where TDoc : notnull
+    where TId : notnull
+    where TInner : notnull
+{
+    private readonly IAggregateFetchPlan<TDoc, TInner> _inner;
+    private readonly Func<TId, TInner> _unwrap;
+
+    public UnwrappedIdentityFetchPlan(IAggregateFetchPlan<TDoc, TInner> inner, Func<TId, TInner> unwrap)
+    {
+        _inner = inner;
+        _unwrap = unwrap;
+    }
+
+    public ProjectionLifecycle Lifecycle => _inner.Lifecycle;
+
+    public Task<IEventStream<TDoc>> FetchForWriting(DocumentSessionBase session, TId id, bool forUpdate,
+        CancellationToken cancellation = default)
+        => _inner.FetchForWriting(session, _unwrap(id), forUpdate, cancellation);
+
+    public Task<IEventStream<TDoc>> FetchForWriting(DocumentSessionBase session, TId id,
+        long expectedStartingVersion, CancellationToken cancellation = default)
+        => _inner.FetchForWriting(session, _unwrap(id), expectedStartingVersion, cancellation);
+
+    public ValueTask<TDoc?> FetchForReading(DocumentSessionBase session, TId id, CancellationToken cancellation)
+        => _inner.FetchForReading(session, _unwrap(id), cancellation);
+
+    public ValueTask<TDoc?> ProjectLatest(DocumentSessionBase session, TId id, CancellationToken cancellation)
+        => _inner.ProjectLatest(session, _unwrap(id), cancellation);
+
+    public Task<bool> StreamForReading(DocumentSessionBase session, TId id, Stream destination,
+        CancellationToken cancellation)
+        => _inner.StreamForReading(session, _unwrap(id), destination, cancellation);
+
+    public IQueryHandler<IEventStream<TDoc>> BuildQueryHandler(QuerySession session, TId id,
+        long expectedStartingVersion)
+        => _inner.BuildQueryHandler(session, _unwrap(id), expectedStartingVersion);
+
+    public IQueryHandler<IEventStream<TDoc>> BuildQueryHandler(QuerySession session, TId id, bool forUpdate)
+        => _inner.BuildQueryHandler(session, _unwrap(id), forUpdate);
+
+    public IQueryHandler<TDoc?> BuildQueryHandler(QuerySession session, TId id)
+        => _inner.BuildQueryHandler(session, _unwrap(id));
+}