diff --git a/src/Persistence/MartenTests/AggregateHandlerWorkflow/core_events_return_3941.cs b/src/Persistence/MartenTests/AggregateHandlerWorkflow/core_events_return_3941.cs new file mode 100644 index 000000000..5d351afa6 --- /dev/null +++ b/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 return. That fallback works but is positional: IEnumerable 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 givenAccount(decimal opening) + { + var streamId = Guid.NewGuid(); + await using var session = theHost.DocumentStore().LightweightSession(); + session.Events.StartStream(streamId, new AmountDeposited(opening)); + await session.SaveChangesAsync(TestContext.Current.CancellationToken); + + return streamId; + } + + private async Task loadAccount(Guid streamId) + { + await using var session = theHost.DocumentStore().LightweightSession(); + return (await session.Events.AggregateStreamAsync(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). Both are castable to + // IEnumerable, 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) Handle( + RecordAuditedDeposit command, + [WriteModel] Account account) + => (new EventsToAppend { new AmountDeposited(command.Amount) }, + new[] { $"deposit of {command.Amount} against {command.AccountId}" }); +} diff --git a/src/Wolverine/Persistence/EventSourcing/DcbModelAttribute.cs b/src/Wolverine/Persistence/EventSourcing/DcbModelAttribute.cs index 37b6b7344..3cda69ea9 100644 --- a/src/Wolverine/Persistence/EventSourcing/DcbModelAttribute.cs +++ b/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>() && !x.VariableType.CanBeCastTo()); diff --git a/src/Wolverine/Persistence/EventSourcing/EventsToAppend.cs b/src/Wolverine/Persistence/EventSourcing/EventsToAppend.cs new file mode 100644 index 000000000..f88fcc6ea --- /dev/null +++ b/src/Wolverine/Persistence/EventSourcing/EventsToAppend.cs @@ -0,0 +1,68 @@ +using Wolverine.Configuration; + +namespace Wolverine.Persistence.EventSourcing; + +/// +/// Tells Wolverine handlers that this value contains a list of events to be appended to the +/// current stream — the store-agnostic sibling of Wolverine.Marten.Events, +/// Wolverine.Polecat.Events and Wolverine.Fisher.Events. +/// +/// +/// +/// 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 +/// IReadOnlyList<object> return is picked up by the +/// IEnumerable<object> fallback in +/// — but +/// that fallback is positional and implicit. Because IEnumerable<T> is +/// covariant, every reference-typed collection in a return tuple is castable to +/// IEnumerable<object>, so a handler returning +/// (IReadOnlyList<object>, IReadOnlyList<string>) has two candidates and +/// whichever lands first in Creates silently becomes the appended events. Nothing +/// fails at codegen and nothing fails at runtime; the wrong collection just ends up in the +/// event stream. +/// +/// +/// OutgoingMessages escapes that only because it is an +/// , 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 (EventsToAppend, OutgoingMessages) is unambiguous. +/// +/// +/// The store-specific types stay exactly as they are for existing code, the same way +/// WriteAggregateAttribute was kept alongside WriteModelAttribute in GH-3907. +/// +/// +/// Why this is not just called Events, which would have mirrored the three +/// store-specific types: it cannot be. This type lives beside +/// , and [WriteModel] is what makes a +/// store-agnostic handler possible in the first place — so the handler that wants this type +/// imports Wolverine.Persistence.EventSourcing by necessity, and a real application +/// imports its store's Wolverine.Marten / .Polecat / .Fisher as well. +/// Naming this Events made those two imports collide with CS0104 on the handler's +/// return type itself, forcing a using alias onto precisely the code this exists +/// to serve. The name says when the append happens, which the bare noun did not. +/// +/// +/// Single-stream only: this is for the case where [WriteModel] pins the aggregate and +/// every event goes to its stream. Appending to a variable number of different +/// streams is a separate gap and is deliberately not addressed here. +/// +/// +public class EventsToAppend : List, IWolverineReturnType +{ + public EventsToAppend() + { + } + + public EventsToAppend(IEnumerable collection) : base(collection) + { + } + + public static EventsToAppend operator +(EventsToAppend events, object @event) + { + events.Add(@event); + return events; + } +} diff --git a/src/Wolverine/Persistence/EventSourcing/IEventSourcingFrameProvider.cs b/src/Wolverine/Persistence/EventSourcing/IEventSourcingFrameProvider.cs index da0a8c827..0849681b8 100644 --- a/src/Wolverine/Persistence/EventSourcing/IEventSourcingFrameProvider.cs +++ b/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>() && !x.VariableType.CanBeCastTo());