Fix [SagaIdentityFrom] ignored when NotFound declared before Handle

src/Testing/CoreTests/Bugs/Bug_2521_saga_identity_from_ordering.cs

@@ -0,0 +1,107 @@
+using System.Diagnostics;
+using JasperFx.CodeGeneration;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Wolverine.Persistence.Sagas;
+using Wolverine.Tracking;
+using Xunit;
+
+namespace CoreTests.Bugs;
+
+/// <summary>
+/// GH-2521: [SagaIdentityFrom] attribute is ignored when NotFound handler is declared
+/// before Handle in the saga class. The code generation was only scanning the first
+/// handler method found (via DistinctBy), which would be NotFound if declared first.
+/// </summary>
+public class Bug_2521_saga_identity_from_ordering : IAsyncLifetime
+{
+    private IHost _host = null!;
+
+    public async Task InitializeAsync()
+    {
+        _host = await Host.CreateDefaultBuilder()
+            .UseWolverine(opts =>
+            {
+                opts.Discovery.DisableConventionalDiscovery()
+                    .IncludeType(typeof(SagaWithNotFoundDeclaredFirst));
+
+                opts.CodeGeneration.TypeLoadMode = TypeLoadMode.Auto;
+            }).StartAsync();
+    }
+
+    public async Task DisposeAsync()
+    {
+        await _host.StopAsync();
+        _host.Dispose();
+    }
+
+    [Fact]
+    public async Task saga_identity_from_attribute_works_when_notfound_is_declared_first()
+    {
+        var sagaId = Guid.NewGuid().ToString();
+
+        // Start the saga
+        await _host.InvokeMessageAndWaitAsync(new StartNotFoundFirstSaga(sagaId));
+
+        var persistor = _host.Services.GetRequiredService<InMemorySagaPersistor>();
+        persistor.Load<SagaWithNotFoundDeclaredFirst>(sagaId).ShouldNotBeNull();
+
+        // Reset tracking
+        SagaWithNotFoundDeclaredFirst.HandleCalled = false;
+        SagaWithNotFoundDeclaredFirst.NotFoundCalled = false;
+
+        // Send message with non-conventional property name — should find saga via [SagaIdentityFrom]
+        await _host.InvokeMessageAndWaitAsync(new NotFoundFirstMsg(sagaId));
+
+        // Handle should have been called on the existing saga, not NotFound
+        SagaWithNotFoundDeclaredFirst.HandleCalled.ShouldBeTrue(
+            "Handle() should be called when saga exists and [SagaIdentityFrom] correctly resolves the saga ID");
+        SagaWithNotFoundDeclaredFirst.NotFoundCalled.ShouldBeFalse(
+            "NotFound() should NOT be called when saga exists");
+    }
+
+    [Fact]
+    public async Task notfound_is_called_when_saga_does_not_exist()
+    {
+        SagaWithNotFoundDeclaredFirst.HandleCalled = false;
+        SagaWithNotFoundDeclaredFirst.NotFoundCalled = false;
+
+        // Send message for non-existent saga
+        await _host.InvokeMessageAndWaitAsync(new NotFoundFirstMsg(Guid.NewGuid().ToString()));
+
+        SagaWithNotFoundDeclaredFirst.NotFoundCalled.ShouldBeTrue();
+        SagaWithNotFoundDeclaredFirst.HandleCalled.ShouldBeFalse();
+    }
+}
+
+public record StartNotFoundFirstSaga(string Id);
+
+// Message with non-conventional property name (not "SagaWithNotFoundDeclaredFirstId", "SagaId", or "Id")
+public record NotFoundFirstMsg(string TargetId);
+
+// Key: NotFound is declared BEFORE Handle — this is what triggers GH-2521
+public class SagaWithNotFoundDeclaredFirst : Saga
+{
+    public string Id { get; set; } = string.Empty;
+
+    public static bool HandleCalled { get; set; }
+    public static bool NotFoundCalled { get; set; }
+
+    public static SagaWithNotFoundDeclaredFirst Start(StartNotFoundFirstSaga cmd)
+        => new() { Id = cmd.Id };
+
+    // NotFound declared FIRST — before Handle
+    public static void NotFound(NotFoundFirstMsg msg)
+    {
+        NotFoundCalled = true;
+        Debug.WriteLine($"NotFound called for TargetId {msg.TargetId}");
+    }
+
+    // Handle declared SECOND — with [SagaIdentityFrom] on a non-conventional property
+    public void Handle(
+        [SagaIdentityFrom(nameof(NotFoundFirstMsg.TargetId))] NotFoundFirstMsg msg)
+    {
+        HandleCalled = true;
+        Debug.WriteLine($"Handle called for saga {Id}");
+    }
+}

src/Testing/CoreTests/Persistence/Sagas/saga_id_member_determination.cs

@@ -24,6 +24,34 @@ public void member_is_determined_by_attribute()
         SagaChain.DetermineSagaIdMember(typeof(SomeSagaMessage5), typeof(SomeSaga), method)
             !.Name.ShouldBe(nameof(SomeSagaMessage5.Hello));
     }
+
+    [Fact]
+    public void member_is_determined_by_attribute_scanning_all_methods()
+    {
+        // When multiple methods are passed, [SagaIdentityFrom] should be found
+        // regardless of which method has it — fixes GH-2521
+        var notFoundMethod = typeof(SagaWithNotFoundFirst).GetMethod(nameof(SagaWithNotFoundFirst.NotFound))!;
+        var handleMethod = typeof(SagaWithNotFoundFirst).GetMethod(nameof(SagaWithNotFoundFirst.Handle))!;
+
+        // Even when NotFound is listed first (no [SagaIdentityFrom]), the Handle method's attribute is found
+        SagaChain.DetermineSagaIdMember(typeof(NonConventionalMessage), typeof(SagaWithNotFoundFirst),
+            [notFoundMethod, handleMethod])
+            !.Name.ShouldBe(nameof(NonConventionalMessage.TargetId));
+    }
+
+    [Fact]
+    public void member_is_determined_by_attribute_even_with_single_wrong_method()
+    {
+        // When only the NotFound method is passed (old behavior), the attribute is NOT found
+        var notFoundMethod = typeof(SagaWithNotFoundFirst).GetMethod(nameof(SagaWithNotFoundFirst.NotFound))!;
+
+        // Without scanning all methods, falls back to convention — which won't find "TargetId"
+        var result = SagaChain.DetermineSagaIdMember(typeof(NonConventionalMessage), typeof(SagaWithNotFoundFirst),
+            [notFoundMethod]);
+
+        // Should NOT find TargetId since NotFound doesn't have [SagaIdentityFrom]
+        result.ShouldBeNull();
+    }
 }
 
 public record SomeSagaMessage1(Guid Id, [property: SagaIdentity] Guid RandomName);
@@ -41,4 +69,18 @@ public class SomeSaga
     public void Handle([SagaIdentityFrom(nameof(SomeSagaMessage5.Hello))] SomeSagaMessage5 message) { }
 }
 
-#endregion
+#endregion
+
+// GH-2521: Message with non-conventional property name (no convention match for "SagaWithNotFoundFirst")
+public record NonConventionalMessage(string TargetId);
+
+// GH-2521: Saga where NotFound is declared BEFORE Handle
+public class SagaWithNotFoundFirst : Saga
+{
+    public string Id { get; set; } = string.Empty;
+
+    // NotFound is declared first — this caused the bug when only the first method was scanned
+    public static void NotFound(NonConventionalMessage msg) { }
+
+    public void Handle([SagaIdentityFrom(nameof(NonConventionalMessage.TargetId))] NonConventionalMessage msg) { }
+}

src/Wolverine/Persistence/Sagas/SagaChain.cs

@@ -46,10 +46,10 @@ public SagaChain(WolverineOptions options, IGrouping<Type, HandlerCall> grouping
     {
         // After base constructor, saga handlers may have been moved to ByEndpoint (Separated mode).
         // Check what's left in Handlers (not the original grouping).
-        var remainingSagaCalls = Handlers.Where(x => x.HandlerType.CanBeCastTo<Saga>())
-            .DistinctBy(x => x.HandlerType).ToArray();
+        var allSagaHandlers = Handlers.Where(x => x.HandlerType.CanBeCastTo<Saga>()).ToArray();
+        var distinctSagaTypes = allSagaHandlers.DistinctBy(x => x.HandlerType).ToArray();
 
-        if (remainingSagaCalls.Length == 0)
+        if (distinctSagaTypes.Length == 0)
         {
             // All sagas were separated into ByEndpoint chains — this parent is routing-only.
             var anySaga = grouping.First(x => x.HandlerType.CanBeCastTo<Saga>());
@@ -59,11 +59,13 @@ public SagaChain(WolverineOptions options, IGrouping<Type, HandlerCall> grouping
 
         try
         {
-            var saga = remainingSagaCalls.Single();
+            var saga = distinctSagaTypes.Single();
             SagaType = saga.HandlerType;
             SagaMethodInfo = saga.Method;
 
-            SagaIdMember = DetermineSagaIdMember(MessageType, SagaType, saga.Method);
+            // Pass ALL saga handler methods so [SagaIdentityFrom] is found regardless of declaration order
+            SagaIdMember = DetermineSagaIdMember(MessageType, SagaType,
+                allSagaHandlers.Select(x => x.Method).ToArray());
 
             // Automatically audit the saga id
             if (SagaIdMember != null && AuditedMembers.All(x => x.Member != SagaIdMember))
@@ -73,7 +75,7 @@ public SagaChain(WolverineOptions options, IGrouping<Type, HandlerCall> grouping
         }
         catch (Exception e)
         {
-            var handlerTypes = remainingSagaCalls
+            var handlerTypes = distinctSagaTypes
                 .Select(x => x.HandlerType).Select(x => x.FullNameInCode()).Join(", ");
 
             throw new InvalidSagaException(
@@ -132,7 +134,7 @@ public SagaChain(HandlerCall handlerCall, HandlerGraph handlerGraph, Endpoint[]
         SagaType = saga.HandlerType;
         SagaMethodInfo = saga.Method;
 
-        SagaIdMember = DetermineSagaIdMember(MessageType, SagaType, saga.Method);
+        SagaIdMember = DetermineSagaIdMember(MessageType, SagaType, [saga.Method]);
 
         // Automatically audit the saga id
         if (SagaIdMember != null && AuditedMembers.All(x => x.Member != SagaIdMember))
@@ -151,7 +153,9 @@ internal SagaChain(HandlerCall[] sagaCalls, HandlerGraph handlerGraph, Endpoint[
         SagaType = saga.HandlerType;
         SagaMethodInfo = saga.Method;
 
-        SagaIdMember = DetermineSagaIdMember(MessageType, SagaType, saga.Method);
+        // Pass ALL saga handler methods so [SagaIdentityFrom] is found regardless of declaration order
+        SagaIdMember = DetermineSagaIdMember(MessageType, SagaType,
+            sagaCalls.Select(x => x.Method).ToArray());
 
         if (SagaIdMember != null && AuditedMembers.All(x => x.Member != SagaIdMember))
         {
@@ -185,10 +189,19 @@ protected override void validateAgainstInvalidSagaMethods(IGrouping<Type, Handle
     public MethodCall[] NotFoundCalls { get; set; } = [];
 
     public static MemberInfo? DetermineSagaIdMember(Type messageType, Type sagaType, MethodInfo? sagaHandlerMethod = null)
+    {
+        return DetermineSagaIdMember(messageType, sagaType,
+            sagaHandlerMethod != null ? [sagaHandlerMethod] : null);
+    }
+
+    public static MemberInfo? DetermineSagaIdMember(Type messageType, Type sagaType, MethodInfo[]? sagaHandlerMethods)
     {
         var expectedSagaIdName = $"{sagaType.Name}Id";
 
-        var specifiedSagaIdMemberName = sagaHandlerMethod?.GetParameters()
+        // Scan ALL handler methods for [SagaIdentityFrom], not just the first one.
+        // This fixes the bug where declaration order of NotFound vs Handle matters.
+        var specifiedSagaIdMemberName = sagaHandlerMethods?
+            .SelectMany(m => m.GetParameters())
             .Select(x => x.GetCustomAttribute<SagaIdentityFromAttribute>())
             .FirstOrDefault(a => a != null)?.PropertyName;