diff --git a/src/Http/Wolverine.Http.Tests.EfCoreOnly/Bug3353StorageActionEndpoint.cs b/src/Http/Wolverine.Http.Tests.EfCoreOnly/Bug3353StorageActionEndpoint.cs new file mode 100644 index 000000000..7f59aa54c --- /dev/null +++ b/src/Http/Wolverine.Http.Tests.EfCoreOnly/Bug3353StorageActionEndpoint.cs @@ -0,0 +1,53 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.EntityFrameworkCore; +using Wolverine.Http; +using Wolverine.Persistence; + +namespace Wolverine.Http.Tests.EfCoreOnly; + +// This little assembly exists so that Wolverine.Http.Tests can spin up hosts that use EF Core but NOT +// Marten (see Bug_3353_lightweight_storage_action_cascade). Pinning opts.ApplicationAssembly here keeps +// HTTP endpoint discovery away from the main test assembly, whose endpoints assume Marten is registered +// ([Entity]-loaded Marten documents, aggregate handlers, ...) and blow up chain construction without it. + +public class Bug3353DbContext : DbContext +{ + public Bug3353DbContext(DbContextOptions options) : base(options) + { + } + + public DbSet Items => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(map => + { + map.ToTable("bug3353_items", "bug3353"); + map.HasKey(x => x.Id); + }); + } +} + +public class Bug3353Item +{ + public Guid Id { get; set; } + public string Name { get; set; } = string.Empty; +} + +public record Bug3353CreateItem(string Name); + +public record Bug3353ItemStored(Guid Id); + +public static class Bug3353StorageActionEndpoint +{ + // Persists ONLY through the storage action, and never injects Bug3353DbContext. That combination + // routes this chain to EFCorePersistenceFrameProvider.ApplyTransactionSupport(chain, container, + // entityType) instead of the two-argument overload that carries the GH-3291 fix - CanApply is false + // without a DbContext service dependency, so AutoApplyTransactions never claims the chain. + [WolverinePost("/bug3353/storage-action")] + public static (IResult, Insert, Bug3353ItemStored) Post(Bug3353CreateItem command) + { + var item = new Bug3353Item { Id = Guid.NewGuid(), Name = command.Name }; + return (Results.Ok(), Storage.Insert(item), new Bug3353ItemStored(item.Id)); + } +} diff --git a/src/Http/Wolverine.Http.Tests.EfCoreOnly/Wolverine.Http.Tests.EfCoreOnly.csproj b/src/Http/Wolverine.Http.Tests.EfCoreOnly/Wolverine.Http.Tests.EfCoreOnly.csproj new file mode 100644 index 000000000..31db7539d --- /dev/null +++ b/src/Http/Wolverine.Http.Tests.EfCoreOnly/Wolverine.Http.Tests.EfCoreOnly.csproj @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/src/Http/Wolverine.Http.Tests/Bug_3353_lightweight_storage_action_cascade.cs b/src/Http/Wolverine.Http.Tests/Bug_3353_lightweight_storage_action_cascade.cs new file mode 100644 index 000000000..65ac76080 --- /dev/null +++ b/src/Http/Wolverine.Http.Tests/Bug_3353_lightweight_storage_action_cascade.cs @@ -0,0 +1,170 @@ +using Alba; +using IntegrationTests; +using JasperFx; +using JasperFx.CodeGeneration; +using JasperFx.Core.Reflection; +using Microsoft.AspNetCore.Builder; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; +using Wolverine.EntityFrameworkCore; +using Wolverine.Http.Tests.EfCoreOnly; +using Wolverine.Persistence; +using Wolverine.Postgresql; +using Xunit; + +namespace Wolverine.Http.Tests; + +// Reproducer for GH-3353, the follow-up to GH-3291. The 3291 fix landed in +// EFCorePersistenceFrameProvider.ApplyTransactionSupport(chain, container) - the two-argument overload +// reached from the AutoApplyTransactions policy. There is a SECOND overload, +// ApplyTransactionSupport(chain, container, entityType), reached from the side-effect return types +// (IStorageAction, Storage.Insert/Store/Update/Delete). That overload never enlists the endpoint's +// MessageContext in the outbox. +// +// The two overloads are mutually exclusive via the UsingEfCoreTransaction chain tag: whichever runs +// first wins. AutoApplyTransactions only fires when EFCorePersistenceFrameProvider.CanApply is true, +// and CanApply requires the chain to have a DbContext service dependency. So an HTTP endpoint that +// persists purely through a storage action - and never injects the DbContext - is claimed ONLY by the +// entityType overload. In Lightweight mode its cascaded messages are then dispatched by the send-now +// branch of MessageBus.PersistOrSendAsync BEFORE SaveChangesAsync commits: a downstream handler can +// observe the database before the row that triggered the message exists. +// +// Unlike the sibling GH-3291 reproducer (Bug_3291_lightweight_http_cascade_flushes_before_commit), +// this host registers EF Core but NOT Marten - the shape of the application the issue describes. That +// also means EF Core is naturally the only persistence frame provider, so the storage action resolves +// to the DbContext without any provider-ordering games. The endpoint and DbContext live in the tiny +// Wolverine.Http.Tests.EfCoreOnly assembly, and endpoint discovery is pinned there, because the main +// test assembly's endpoints assume Marten is registered and fail chain construction without it. +// +// Like the siblings, we assert at the codegen surface rather than at runtime: the runtime symptom (a +// message dispatched pre-commit) races the durability agent and the handler scheduler. The generated +// composition is the deterministic proof. +public class Bug_3353_lightweight_storage_action_cascade +{ + private static async Task buildEfCoreOnlyLightweightHostAsync() + { + var builder = WebApplication.CreateBuilder(); + + // Wolverine-integrated DbContext supplies the outbox enrollment for EF Core + builder.Services.AddDbContextWithWolverineIntegration(x => + x.UseNpgsql(Servers.PostgresConnectionString)); + + builder.Host.UseWolverine(opts => + { + // Pin endpoint discovery to the EF-Core-only assembly; see the class comment + opts.ApplicationAssembly = typeof(Bug3353StorageActionEndpoint).Assembly; + + opts.Durability.Mode = DurabilityMode.Solo; + + // EF Core application WITHOUT Marten: the message store is the plain PostgreSQL-backed one + opts.PersistMessagesWithPostgresql(Servers.PostgresConnectionString, "bug3353"); + + // The bug is specific to Lightweight mode; Eager enrolls the DbContext in a transaction + opts.UseEntityFrameworkCoreTransactions(TransactionMiddlewareMode.Lightweight); + + opts.Policies.AutoApplyTransactions(); + opts.Policies.UseDurableLocalQueues(); + + opts.Discovery.DisableConventionalDiscovery(); + // The cascaded Bug3353ItemStored needs a routed handler so it actually buffers into Outstanding + opts.Discovery.IncludeType(); + }); + + builder.Services.AddWolverineHttp(); + + return await AlbaHost.For(builder, app => app.MapWolverineEndpoints()); + } + + [Fact] + public async Task storage_action_endpoint_enlists_outbox_and_does_not_flush_before_commit() + { + await using var host = await buildEfCoreOnlyLightweightHostAsync(); + + var graph = host.Services.GetRequiredService().Endpoints!; + var chain = graph.ChainFor("POST", "/bug3353/storage-action"); + chain.ShouldNotBeNull(); + + chain.As().InitializeSynchronously(graph.Rules, graph, host.Services); + var source = chain!.SourceCode; + source.ShouldNotBeNull(); + + // Guard: the storage action must be owned by EF Core for this test to mean anything + source.ShouldContain("Bug3353DbContext"); + + // (1) The MessageContext must be enlisted in the outbox, or the cascaded Bug3353ItemStored is + // dispatched by the send-now branch of MessageBus.PersistOrSendAsync before SaveChangesAsync + // commits. + source.ShouldContain("EnlistInOutboxAsync"); + + // (2) No standalone pre-commit flush. + chain.Postprocessors.OfType().ShouldBeEmpty( + "A Lightweight-mode HTTP endpoint that persists via a storage action and cascades a message " + + "must not flush its cascaded messages with a standalone FlushOutgoingMessages postprocessor - " + + "that dispatches the cascade without any outbox protection."); + + // (3) Ordering: enlist -> SaveChangesAsync -> post-commit flush. + // Match the actual call sites (".Method(") rather than bare names, which also appear in comments. + var enlistAt = source.IndexOf(".EnlistInOutboxAsync(", StringComparison.Ordinal); + var saveAt = source.IndexOf(".SaveChangesAsync(", StringComparison.Ordinal); + var commitAt = source.IndexOf(".CommitAsync(", StringComparison.Ordinal); + + saveAt.ShouldBeGreaterThan(enlistAt, "SaveChangesAsync must run after the outbox enrollment"); + commitAt.ShouldBeGreaterThan(saveAt, + "The outbox flush (EfCoreEnvelopeTransaction.CommitAsync) must run after SaveChangesAsync commits"); + } + + // Guard rail for the fix itself: EnlistDbContextInOutbox generates an EfCoreEnvelopeTransaction, + // whose constructor throws unless the application has database-backed message persistence. An EF + // Core application with NO message store (no outbox to protect in the first place) must keep its + // pre-existing composition - SaveChangesAsync with send-now cascades - rather than gain generated + // code that fails on every request. + [Fact] + public async Task storage_action_endpoint_without_message_persistence_is_left_alone() + { + var builder = WebApplication.CreateBuilder(); + + builder.Services.AddDbContextWithWolverineIntegration(x => + x.UseNpgsql(Servers.PostgresConnectionString)); + + builder.Host.UseWolverine(opts => + { + opts.ApplicationAssembly = typeof(Bug3353StorageActionEndpoint).Assembly; + + opts.Durability.Mode = DurabilityMode.Solo; + + // Deliberately NO PersistMessagesWithPostgresql / Marten / any message store + opts.UseEntityFrameworkCoreTransactions(TransactionMiddlewareMode.Lightweight); + + opts.Policies.AutoApplyTransactions(); + + opts.Discovery.DisableConventionalDiscovery(); + opts.Discovery.IncludeType(); + }); + + builder.Services.AddWolverineHttp(); + + await using var host = await AlbaHost.For(builder, app => app.MapWolverineEndpoints()); + + var graph = host.Services.GetRequiredService().Endpoints!; + var chain = graph.ChainFor("POST", "/bug3353/storage-action"); + chain.ShouldNotBeNull(); + + chain.As().InitializeSynchronously(graph.Rules, graph, host.Services); + var source = chain!.SourceCode; + source.ShouldNotBeNull(); + + source.ShouldContain("Bug3353DbContext"); + source.ShouldContain(".SaveChangesAsync("); + source.ShouldNotContain("EnlistInOutboxAsync"); + } +} + +// A routed handler so the cascaded Bug3353ItemStored has somewhere to go (durable local queue). Body +// is irrelevant - the test asserts the outbox enrollment/flush composition, not the handling. +public class Bug3353ItemStoredHandler +{ + public void Handle(Bug3353ItemStored _) + { + } +} diff --git a/src/Http/Wolverine.Http.Tests/Wolverine.Http.Tests.csproj b/src/Http/Wolverine.Http.Tests/Wolverine.Http.Tests.csproj index 904268836..66c954849 100644 --- a/src/Http/Wolverine.Http.Tests/Wolverine.Http.Tests.csproj +++ b/src/Http/Wolverine.Http.Tests/Wolverine.Http.Tests.csproj @@ -26,6 +26,7 @@ + diff --git a/src/Persistence/Wolverine.EntityFrameworkCore/Codegen/EFCorePersistenceFrameProvider.cs b/src/Persistence/Wolverine.EntityFrameworkCore/Codegen/EFCorePersistenceFrameProvider.cs index 6a8118b11..a18df1dac 100644 --- a/src/Persistence/Wolverine.EntityFrameworkCore/Codegen/EFCorePersistenceFrameProvider.cs +++ b/src/Persistence/Wolverine.EntityFrameworkCore/Codegen/EFCorePersistenceFrameProvider.cs @@ -14,7 +14,9 @@ using Wolverine.Configuration; using Wolverine.EntityFrameworkCore.Internals; using Wolverine.Persistence; +using Wolverine.Persistence.Durability; using Wolverine.Persistence.Sagas; +using Wolverine.RDBMS; using Wolverine.Runtime; namespace Wolverine.EntityFrameworkCore.Codegen; @@ -229,7 +231,8 @@ public void ApplyTransactionSupport(IChain chain, IServiceContainer container) else if (isHttpChain(chain) && !isMultiTenanted(container, dbContextType) && chain.RequiresOutbox() - && chain.ShouldFlushOutgoingMessages()) + && chain.ShouldFlushOutgoingMessages() + && hasDatabaseBackedMessagePersistence(container)) { // GH-3291: A Wolverine.Http endpoint has no incoming envelope, so - unlike a message handler, // whose MessageContext is enlisted by MessageContext.ReadEnvelope at runtime - its @@ -341,6 +344,17 @@ private bool isMultiTenanted(IServiceContainer container, Type dbContextType) return container.HasRegistrationFor(typeof(IDbContextBuilder<>).MakeGenericType(dbContextType)); } + // EnlistDbContextInOutbox generates a runtime EfCoreEnvelopeTransaction, whose constructor throws + // unless the application has database-backed message persistence (see + // MessageContextExtensions.TryFindMessageDatabase - this mirrors its outer type check). Without a + // message database there is no outbox to protect, so leave the chain on its pre-existing send-now + // composition rather than generate code that fails on every request. + private static bool hasDatabaseBackedMessagePersistence(IServiceContainer container) + { + var runtime = container.Services.GetRequiredService(); + return runtime.Storage is IMessageDatabase or MultiTenantedMessageStore; + } + // HttpChain is the only chain type whose Scoping is HttpEndpoints. Detected via the scoping enum // rather than a type reference so Wolverine.EntityFrameworkCore need not depend on Wolverine.Http. private static bool isHttpChain(IChain chain) @@ -394,6 +408,24 @@ public void ApplyTransactionSupport(IChain chain, IServiceContainer container, T enrolledInTransaction = true; } } + else if (isHttpChain(chain) && !isMultiTenanted(container, dbType) + && hasDatabaseBackedMessagePersistence(container)) + { + // GH-3353, the storage-action counterpart to the GH-3291 branch in the no-entity overload + // above: a Wolverine.Http endpoint has no incoming envelope, so its MessageContext.Transaction + // stays null in Lightweight mode and cascaded messages are dispatched by the send-now branch + // of MessageBus.PersistOrSendAsync BEFORE the SaveChangesAsync postprocessor commits. + // + // Unlike that branch, do NOT gate on chain.RequiresOutbox(). This overload runs from + // SideEffectPolicy, which the WolverineOptions constructor registers ahead of + // OutgoingMessagesPolicy, and HttpChain.RequiresOutbox() only reflects an injected + // IMessageBus/MessageContext - an endpoint that cascades purely through its return tuple + // (the very shape that returns a storage action instead of injecting the DbContext) can + // never satisfy it here. Enlisting when the endpoint turns out not to cascade anything is a + // no-op flush at commit time. + chain.Middleware.Insert(0, new EnlistDbContextInOutbox(dbType)); + enrolledInTransaction = true; + } var abstractionType = chain.ServiceDependencies(container, Type.EmptyTypes).FirstOrDefault(x => _abstractions.Contains(x)); if (abstractionType != null) diff --git a/wolverine.slnx b/wolverine.slnx index 49eb7f5be..b24a462c3 100644 --- a/wolverine.slnx +++ b/wolverine.slnx @@ -53,6 +53,7 @@ +