Merge pull request #4187 from MikeFreeman-Flagstone/fix/sequence-number-cause-argument-exception

fix(asb): add SequenceNumber to ReservedHeaders to prevent duplicate key on redelivery

Author: iancooper

src/Paramore.Brighter.MessagingGateway.AzureServiceBus/ASBConstants.cs

@@ -24,7 +24,7 @@ internal static class ASBConstants
         public const string TraceParent = "cloudevents:traceparent";
         public const string TraceState = "cloudevents:tracestate";
 
-        public static readonly string[] ReservedHeaders = [LockTokenHeaderBagKey, MessageTypeHeaderBagKey, HandledCountHeaderBagKey, ReplyToHeaderBagKey, SessionIdKey
+        public static readonly string[] ReservedHeaders = [LockTokenHeaderBagKey, SequenceNumberBagKey, MessageTypeHeaderBagKey, HandledCountHeaderBagKey, ReplyToHeaderBagKey, SessionIdKey
         ];
     }
 }

tests/Paramore.Brighter.AzureServiceBus.Tests/MessagingGateway/When_A_Deferred_Message_Is_Redelivered.cs

@@ -0,0 +1,80 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using Paramore.Brighter.AzureServiceBus.Tests.TestDoubles;
+using Paramore.Brighter.MessagingGateway.AzureServiceBus;
+using Xunit;
+
+namespace Paramore.Brighter.AzureServiceBus.Tests.MessagingGateway;
+
+/// <summary>
+/// Covers the bug where DeferMessageAction caused a requeued message to carry
+/// SequenceNumber in ApplicationProperties. On redelivery, MapToBrighterMessage
+/// would throw ArgumentException (duplicate key) because SequenceNumber was
+/// already added from the native broker property.
+/// Fix: SequenceNumber is in ASBConstants.ReservedHeaders so it is never written
+/// to ApplicationProperties during requeue.
+/// </summary>
+[Trait("Category", "ASB")]
+public class When_A_Deferred_Message_Is_Redelivered
+{
+    private readonly AzureServiceBusMesssageCreator _creator;
+
+    public When_A_Deferred_Message_Is_Redelivered()
+    {
+        var subscription = new AzureServiceBusSubscription<ASBTestCommand>(
+            subscriptionName: new SubscriptionName("test-sub"),
+            channelName: new ChannelName("test-channel"),
+            routingKey: new RoutingKey("test-topic"),
+            messagePumpType: MessagePumpType.Reactor);
+
+        _creator = new AzureServiceBusMesssageCreator(subscription);
+    }
+
+    [Fact]
+    public void When_a_deferred_message_is_redelivered_it_does_not_throw_a_duplicate_key_exception()
+    {
+        // Arrange — first delivery from ASB
+        var firstDelivery = new BrokeredMessage
+        {
+            MessageBodyValue = Encoding.UTF8.GetBytes("{\"key\":\"value\"}"),
+            ApplicationProperties = new Dictionary<string, object>
+            {
+                { "MessageType", "MT_COMMAND" }
+            },
+            LockToken = Guid.NewGuid().ToString(),
+            SequenceNumber = 100L,
+            Id = Guid.NewGuid().ToString(),
+            CorrelationId = Guid.NewGuid().ToString(),
+            ContentType = "application/json"
+        };
+
+        // First receive — handler will throw DeferMessageAction, triggering a requeue
+        var brighterMessage = _creator.MapToBrighterMessage(firstDelivery);
+
+        // Simulate requeue: Brighter calls ConvertToServiceBusMessage and republishes.
+        // SequenceNumber must NOT appear in ApplicationProperties of the requeued message.
+        var requeued = AzureServiceBusMessagePublisher.ConvertToServiceBusMessage(brighterMessage);
+        Assert.False(requeued.ApplicationProperties.ContainsKey("SequenceNumber"),
+            "SequenceNumber must not be written to ApplicationProperties on requeue — " +
+            "it is a broker-assigned property and must stay in ReservedHeaders.");
+
+        // Arrange — second delivery (redelivery of the requeued message with the same sequence number)
+        var secondDelivery = new BrokeredMessage
+        {
+            MessageBodyValue = Encoding.UTF8.GetBytes("{\"key\":\"value\"}"),
+            ApplicationProperties = requeued.ApplicationProperties
+                .ToDictionary(kvp => kvp.Key, kvp => kvp.Value),
+            LockToken = Guid.NewGuid().ToString(),
+            SequenceNumber = firstDelivery.SequenceNumber,
+            Id = firstDelivery.Id,
+            CorrelationId = firstDelivery.CorrelationId,
+            ContentType = "application/json"
+        };
+
+        // Act & Assert — must not throw ArgumentException: duplicate key SequenceNumber
+        var redelivered = _creator.MapToBrighterMessage(secondDelivery);
+        Assert.Equal(firstDelivery.SequenceNumber, redelivered.Header.Bag["SequenceNumber"]);
+    }
+}

tests/Paramore.Brighter.AzureServiceBus.Tests/MessagingGateway/When_Converting_A_Message_With_Local_Headers.cs

@@ -7,6 +7,26 @@ namespace Paramore.Brighter.AzureServiceBus.Tests.MessagingGateway;
 [Trait("Category", "ASB")]
 public class AzureServiceBusMessagePublisherLocalHeaderTests
 {
+    [Fact]
+    public void When_Converting_A_Message_The_SequenceNumber_Bag_Entry_Is_Not_Written_To_ApplicationProperties()
+    {
+        // SequenceNumber is a broker-assigned system property on every delivery.
+        // It must not round-trip through ApplicationProperties — if it did, a requeued
+        // message would arrive back with SequenceNumber in both its native property and
+        // ApplicationProperties, causing a duplicate-key crash in MapToBrighterMessage.
+        var header = new MessageHeader(
+            messageId: Guid.NewGuid().ToString(),
+            topic: new RoutingKey("test.topic"),
+            messageType: MessageType.MT_COMMAND);
+        header.Bag["SequenceNumber"] = 42L;
+
+        var message = new Message(header, new MessageBody("body"));
+
+        var asbMessage = AzureServiceBusMessagePublisher.ConvertToServiceBusMessage(message);
+
+        Assert.False(asbMessage.ApplicationProperties.ContainsKey("SequenceNumber"));
+    }
+
     [Fact]
     public void When_Converting_A_Message_The_ProducerTopic_Local_Header_Is_Stripped()
     {