GH-3941: a store-agnostic EventsToAppend return type in core

src/Persistence/MartenTests/AggregateHandlerWorkflow/core_events_return_3941.cs

@@ -0,0 +1,128 @@
+using IntegrationTests;
+using Marten;
+using Microsoft.Extensions.Hosting;
+using Shouldly;
+using Wolverine;
+using Wolverine.Marten;
+using Wolverine.Persistence.EventSourcing;
+
+namespace MartenTests.AggregateHandlerWorkflow;
+
+// GH-3941: the store-agnostic EventsToAppend return type. Wolverine.Marten.Events,
+// Wolverine.Polecat.Events and Wolverine.Fisher.Events are identical and store-named, so a handler
+// compiled against more than one store could not name any of them and had to fall back to a bare
+// IEnumerable<object> return. That fallback works but is positional: IEnumerable<T> is covariant, so
+// EVERY reference-typed collection in a return tuple is a candidate and FirstOrDefault takes
+// whichever lands first in Creates. Nothing fails at codegen and nothing fails at runtime - the wrong
+// collection just becomes the events. ambiguous_sibling_collection_does_not_become_the_events is the
+// fact that matters here; the other two would pass against the old fallback too.
+//
+// Note that this file imports BOTH Wolverine.Marten and Wolverine.Persistence.EventSourcing with no
+// using alias, which is the combination a real store-agnostic handler needs -- the store integration
+// plus [WriteModel]. That compiles only because the core type is not also called Events: naming it
+// so collided with CS0104 on the two handler signatures at the bottom of this file.
+public class core_events_return_3941 : IAsyncLifetime
+{
+    private IHost theHost = null!;
+
+    public async ValueTask InitializeAsync()
+    {
+        theHost = await Host.CreateDefaultBuilder()
+            .UseWolverine(opts =>
+            {
+                opts.Discovery.DisableConventionalDiscovery()
+                    .IncludeType(typeof(RecordCoreDepositHandler))
+                    .IncludeType(typeof(RecordAuditedDepositHandler));
+
+                opts.Durability.Mode = DurabilityMode.Solo;
+                opts.Services.AddMarten(m =>
+                {
+                    m.Connection(Servers.PostgresConnectionString);
+                    m.DatabaseSchemaName = "core_events_return_3941";
+                }).IntegrateWithWolverine();
+            }).StartAsync();
+    }
+
+    public async ValueTask DisposeAsync()
+    {
+        await theHost.StopAsync();
+        theHost.Dispose();
+    }
+
+    private async Task<Guid> givenAccount(decimal opening)
+    {
+        var streamId = Guid.NewGuid();
+        await using var session = theHost.DocumentStore().LightweightSession();
+        session.Events.StartStream<Account>(streamId, new AmountDeposited(opening));
+        await session.SaveChangesAsync(TestContext.Current.CancellationToken);
+
+        return streamId;
+    }
+
+    private async Task<Account> loadAccount(Guid streamId)
+    {
+        await using var session = theHost.DocumentStore().LightweightSession();
+        return (await session.Events.AggregateStreamAsync<Account>(streamId,
+            token: TestContext.Current.CancellationToken))!;
+    }
+
+    [Fact]
+    public async Task the_core_events_type_is_appended_to_the_stream()
+    {
+        var streamId = await givenAccount(100m);
+
+        await theHost.InvokeAsync(new RecordCoreDeposit(streamId, 25m));
+
+        (await loadAccount(streamId)).Balance.ShouldBe(125m);
+    }
+
+    [Fact]
+    public async Task every_event_in_the_collection_is_appended()
+    {
+        var streamId = await givenAccount(0m);
+
+        await theHost.InvokeAsync(new RecordCoreDeposit(streamId, 10m, Repeat: 3));
+
+        (await loadAccount(streamId)).Balance.ShouldBe(30m);
+    }
+
+    [Fact]
+    public async Task ambiguous_sibling_collection_does_not_become_the_events()
+    {
+        var streamId = await givenAccount(50m);
+
+        // The handler returns (EventsToAppend, IReadOnlyList<string>). Both are castable to
+        // IEnumerable<object>, so under the old fallback alone the audit lines could be appended as
+        // events instead - which throws nothing and simply corrupts the stream. Declaring EventsToAppend is
+        // what makes the choice deterministic.
+        await theHost.InvokeAsync(new RecordAuditedDeposit(streamId, 5m));
+
+        (await loadAccount(streamId)).Balance.ShouldBe(55m);
+
+        await using var session = theHost.DocumentStore().LightweightSession();
+        var events = await session.Events.FetchStreamAsync(streamId,
+            token: TestContext.Current.CancellationToken);
+
+        events.Count.ShouldBe(2);
+        events.ShouldAllBe(x => x.Data is AmountDeposited);
+    }
+}
+
+public record RecordCoreDeposit(Guid AccountId, decimal Amount, int Repeat = 1);
+
+public record RecordAuditedDeposit(Guid AccountId, decimal Amount);
+
+public static class RecordCoreDepositHandler
+{
+    public static EventsToAppend Handle(RecordCoreDeposit command, [WriteModel] Account account)
+        => new(Enumerable.Range(0, command.Repeat).Select(object (_) => new AmountDeposited(command.Amount)));
+}
+
+public static class RecordAuditedDepositHandler
+{
+    public static (EventsToAppend, IReadOnlyList<string>) Handle(
+        RecordAuditedDeposit command,
+        [WriteModel] Account account)
+        => (new EventsToAppend { new AmountDeposited(command.Amount) },
+            new[] { $"deposit of {command.Amount} against {command.AccountId}" });
+}

src/Wolverine/Persistence/EventSourcing/DcbModelAttribute.cs

@@ -236,7 +236,11 @@ internal static void DetermineEventCaptureHandling(IChain chain, Type modelType,
             return;
         }
 
+        // Matched explicitly and ahead of the fallback for the same reason as the single-stream path
+        // in IEventSourcingFrameProvider — the core Events is an IWolverineReturnType, which the
+        // fallback excludes (GH-3941).
         var eventsVariable = firstCall.Creates.FirstOrDefault(x => x.VariableType == provider.EventsCollectionType) ??
+                             firstCall.Creates.FirstOrDefault(x => x.VariableType == typeof(EventsToAppend)) ??
                              firstCall.Creates.FirstOrDefault(x =>
                                  x.VariableType.CanBeCastTo<IEnumerable<object>>() &&
                                  !x.VariableType.CanBeCastTo<IWolverineReturnType>());

src/Wolverine/Persistence/EventSourcing/EventsToAppend.cs

@@ -0,0 +1,68 @@
+using Wolverine.Configuration;
+
+namespace Wolverine.Persistence.EventSourcing;
+
+/// <summary>
+///     Tells Wolverine handlers that this value contains a list of events to be appended to the
+///     current stream — the store-agnostic sibling of <c>Wolverine.Marten.Events</c>,
+///     <c>Wolverine.Polecat.Events</c> and <c>Wolverine.Fisher.Events</c>.
+/// </summary>
+/// <remarks>
+///     <para>
+///         The three store-specific types are identical and store-named, so a handler that wants to
+///         be store-agnostic could not use any of them. The store-agnostic path did exist — a bare
+///         <c>IReadOnlyList&lt;object&gt;</c> return is picked up by the
+///         <c>IEnumerable&lt;object&gt;</c> fallback in
+///         <see cref="EventSourcingFrameProviderExtensions.DetermineEventCaptureHandling" /> — but
+///         that fallback is <b>positional and implicit</b>. Because <c>IEnumerable&lt;T&gt;</c> is
+///         covariant, every reference-typed collection in a return tuple is castable to
+///         <c>IEnumerable&lt;object&gt;</c>, so a handler returning
+///         <c>(IReadOnlyList&lt;object&gt;, IReadOnlyList&lt;string&gt;)</c> has two candidates and
+///         whichever lands first in <c>Creates</c> silently becomes the appended events. Nothing
+///         fails at codegen and nothing fails at runtime; the wrong collection just ends up in the
+///         event stream.
+///     </para>
+///     <para>
+///         <c>OutgoingMessages</c> escapes that only because it is an
+///         <see cref="IWolverineReturnType" />, which the fallback explicitly excludes — a happy
+///         accident of an unrelated marker rather than a designed guarantee, and one that does not
+///         extend to a user's own collection type. Returning this type instead makes the intent
+///         declared, so <c>(EventsToAppend, OutgoingMessages)</c> is unambiguous.
+///     </para>
+///     <para>
+///         The store-specific types stay exactly as they are for existing code, the same way
+///         <c>WriteAggregateAttribute</c> was kept alongside <c>WriteModelAttribute</c> in GH-3907.
+///     </para>
+///     <para>
+///         <b>Why this is not just called <c>Events</c>,</b> which would have mirrored the three
+///         store-specific types: it cannot be. This type lives beside
+///         <see cref="WriteModelAttribute" />, and <c>[WriteModel]</c> is what makes a
+///         store-agnostic handler possible in the first place — so the handler that wants this type
+///         imports <c>Wolverine.Persistence.EventSourcing</c> by necessity, and a real application
+///         imports its store's <c>Wolverine.Marten</c> / <c>.Polecat</c> / <c>.Fisher</c> as well.
+///         Naming this <c>Events</c> made those two imports collide with CS0104 <em>on the handler's
+///         return type itself</em>, forcing a <c>using</c> alias onto precisely the code this exists
+///         to serve. The name says when the append happens, which the bare noun did not.
+///     </para>
+///     <para>
+///         Single-stream only: this is for the case where <c>[WriteModel]</c> pins the aggregate and
+///         every event goes to its stream. Appending to a variable number of <em>different</em>
+///         streams is a separate gap and is deliberately not addressed here.
+///     </para>
+/// </remarks>
+public class EventsToAppend : List<object>, IWolverineReturnType
+{
+    public EventsToAppend()
+    {
+    }
+
+    public EventsToAppend(IEnumerable<object> collection) : base(collection)
+    {
+    }
+
+    public static EventsToAppend operator +(EventsToAppend events, object @event)
+    {
+        events.Add(@event);
+        return events;
+    }
+}

src/Wolverine/Persistence/EventSourcing/IEventSourcingFrameProvider.cs

@@ -147,7 +147,11 @@ public static void DetermineEventCaptureHandling(this IEventSourcingFrameProvide
             return;
         }
 
+        // The core Events has to be matched explicitly and ahead of the fallback: it is an
+        // IWolverineReturnType, which is exactly what the fallback excludes, so leaving it to be
+        // picked up implicitly would skip it (GH-3941).
         var eventsVariable = firstCall.Creates.FirstOrDefault(x => x.VariableType == provider.EventsCollectionType) ??
+                             firstCall.Creates.FirstOrDefault(x => x.VariableType == typeof(EventsToAppend)) ??
                              firstCall.Creates.FirstOrDefault(x =>
                                  x.VariableType.CanBeCastTo<IEnumerable<object>>() &&
                                  !x.VariableType.CanBeCastTo<IWolverineReturnType>());