diff --git a/src/Transports/AWS/Wolverine.AmazonSns.Tests/send_to_topic_and_receive_in_queue_in_aws.cs b/src/Transports/AWS/Wolverine.AmazonSns.Tests/send_to_topic_and_receive_in_queue_in_aws.cs index d67246635..f942e63e7 100644 --- a/src/Transports/AWS/Wolverine.AmazonSns.Tests/send_to_topic_and_receive_in_queue_in_aws.cs +++ b/src/Transports/AWS/Wolverine.AmazonSns.Tests/send_to_topic_and_receive_in_queue_in_aws.cs @@ -8,7 +8,6 @@ namespace Wolverine.AmazonSns.Tests; -[Trait("Category", "Flaky")] public class send_to_topic_and_receive_in_queue_in_aws : IAsyncLifetime { private IHost _host = null!; @@ -53,7 +52,11 @@ public async ValueTask DisposeAsync() _host.Dispose(); } - [Fact] + // Line-for-line the same test as send_to_topic_and_receive_in_queue, except that it points at a + // real AWS account instead of LocalStack. It is here to be run by hand when SNS fidelity is in + // question; CI has no credentials, so it can only ever fail there. Skipped rather than tagged + // Flaky (#3763) because there is nothing unstable about it. + [Fact(Skip = "Requires real AWS credentials; the LocalStack twin is send_to_topic_and_receive_in_queue")] public async Task send_to_topic_and_receive_in_queue_a_single_message() { var message = new SnsMessage("Josh Allen"); diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/AmazonSqsTransportTests.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/AmazonSqsTransportTests.cs index e70ee65c0..205f701ce 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/AmazonSqsTransportTests.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/AmazonSqsTransportTests.cs @@ -100,4 +100,83 @@ public void findEndpointByUri_should_correctly_create_endpoint_if_it_doesnt_exis transport.Queues.Count.ShouldBe(2); result2.EndpointName.ShouldBe(queueName2); } -} \ No newline at end of file +} +// GH-3763. Conventional routing derives queue names from message type names, so it can hand SQS a +// name it rejects outright -- and because broker initialization provisions every queue together, +// one bad name fails startup for every conventionally-routed host in the assembly. See GH-3786 for +// the same defect on Azure Service Bus. +public class sanitizing_sqs_queue_names +{ + // "Can only include alphanumeric characters, hyphens, or underscores. 1 to 80 in length" + [Theory] + [InlineData("Wolverine.Bugs.BatchedItem[]", "Wolverine-Bugs-BatchedItem__")] + [InlineData("Wolverine.Envelope`1[System.String]", "Wolverine-Envelope_1_System-String_")] + [InlineData("Outer+Inner", "Outer_Inner")] + [InlineData("has spaces", "has_spaces")] + public void illegal_characters_are_substituted(string identifier, string expected) + { + AmazonSqsTransport.SanitizeSqsName(identifier).ShouldBe(expected); + } + + // Substituting rather than stripping is what keeps distinct type names distinct. + [Fact] + public void sanitizing_does_not_collide_an_array_type_with_its_element_type() + { + AmazonSqsTransport.SanitizeSqsName("BatchedItem[]") + .ShouldNotBe(AmazonSqsTransport.SanitizeSqsName("BatchedItem")); + } + + // No name that works today may change: a name SQS rejects could never have been provisioned in + // the first place, so this must stay a pure no-op for legal names of legal length. + [Theory] + [InlineData("Wolverine.Bugs.BatchedItem", "Wolverine-Bugs-BatchedItem")] + [InlineData("two-dead-letter-queue", "two-dead-letter-queue")] + [InlineData("wolverine_retries_MyService", "wolverine_retries_MyService")] + [InlineData("wolverine.retries.MyService.fifo", "wolverine-retries-MyService.fifo")] + public void legal_identifiers_are_left_alone(string identifier, string expected) + { + AmazonSqsTransport.SanitizeSqsName(identifier).ShouldBe(expected); + } + + // The case that actually broke CI: "shazaam-" + a namespace-qualified type name is 81 characters. + [Fact] + public void an_overlong_name_is_brought_under_the_limit() + { + var name = AmazonSqsTransport.SanitizeSqsName( + "shazaam-Wolverine.AmazonSqs.Tests.ConventionalRouting.SqsHandlerTypeNamingMessage"); + + name.Length.ShouldBe(AmazonSqsTransport.MaximumQueueNameLength); + name.ShouldStartWith("shazaam-Wolverine-AmazonSqs-Tests-ConventionalRouting-SqsHandlerType"); + } + + // Truncation alone would collide two long names that share a prefix -- and namespace-qualified + // type names very often do. + [Fact] + public void two_overlong_names_sharing_a_prefix_stay_distinct() + { + var prefix = new string('a', AmazonSqsTransport.MaximumQueueNameLength); + + AmazonSqsTransport.SanitizeSqsName(prefix + "One") + .ShouldNotBe(AmazonSqsTransport.SanitizeSqsName(prefix + "Two")); + } + + // The digest has to be stable across processes and machines, which rules out GetHashCode(). + [Fact] + public void truncation_is_deterministic() + { + var identifier = new string('b', 200); + + AmazonSqsTransport.SanitizeSqsName(identifier) + .ShouldBe("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-aaebc35c"); + } + + // AWS requires the .fifo suffix, and it counts against the 80 character budget. + [Fact] + public void the_fifo_suffix_survives_truncation() + { + var name = AmazonSqsTransport.SanitizeSqsName(new string('c', 200) + ".fifo"); + + name.Length.ShouldBe(AmazonSqsTransport.MaximumQueueNameLength); + name.ShouldEndWith(".fifo"); + } +} diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/Bug_3633_conventional_routing_respects_named_broker.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/Bug_3633_conventional_routing_respects_named_broker.cs index f557529b4..3eef192a3 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/Bug_3633_conventional_routing_respects_named_broker.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/Bug_3633_conventional_routing_respects_named_broker.cs @@ -7,14 +7,14 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; // https://github.com/JasperFx/wolverine/issues/3633 -public class Bug_3633_conventional_routing_respects_named_broker : IDisposable +public class Bug_3633_conventional_routing_respects_named_broker : IAsyncLifetime { private static readonly BrokerName theBrokerName = new("other"); - private readonly IHost _host; + private IHost _host = null!; - public Bug_3633_conventional_routing_respects_named_broker() + public async ValueTask InitializeAsync() { - _host = Host.CreateDefaultBuilder() + _host = await Host.CreateDefaultBuilder() .UseWolverine(opts => { // A default, unnamed broker is also registered so that conventional @@ -25,11 +25,13 @@ public Bug_3633_conventional_routing_respects_named_broker() .UseConventionalRouting(x => x.IncludeTypes(t => t == typeof(RoutedMessage))) .AutoProvision() .AutoPurgeOnStartup(); - }).Start(); + }).StartAsync(); } - public void Dispose() + // StopAsync, not just Dispose -- see the note in ConventionalRoutingContext (GH-3763). + public async ValueTask DisposeAsync() { + await _host.StopAsync(); _host.Dispose(); } diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/ConventionalRoutingContext.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/ConventionalRoutingContext.cs index 2cf6f711b..e39939ac7 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/ConventionalRoutingContext.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/ConventionalRoutingContext.cs @@ -7,21 +7,53 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -public abstract class ConventionalRoutingContext : IDisposable +/// +/// Every class in this namespace listens to the same small set of LocalStack queues -- most of them +/// to sqs://routed. A host that is disposed but never stopped keeps long-polling that queue, +/// so it steals messages from whichever class runs next and the theft looks like flakiness. See +/// GH-3763. +/// +/// This type owns disposal for that reason: derived classes override +/// rather than re-declaring themselves. Re-declaring it is what let a +/// no-op DisposeAsync shadow the real one here and in the Azure Service Bus fixtures +/// (GH-3758) -- an explicit interface implementation on a derived class silently wins. +/// +public abstract class ConventionalRoutingContext : IAsyncLifetime { private IHost _host = null!; + public virtual ValueTask InitializeAsync() => ValueTask.CompletedTask; + internal async Task theRuntime() { _host ??= await WolverineHost.ForAsync(opts => - opts.UseAmazonSqsTransport().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup()); + opts.UseAmazonSqsTransportLocally().UseConventionalRouting(leaveTheEndToEndQueueAlone).AutoProvision() + .AutoPurgeOnStartup()); return _host.Services.GetRequiredService(); } - public void Dispose() + /// + /// These classes only assert on configuration, but they still stand up real listeners for every + /// handler in the assembly -- which would include the queue end_to_end_with_conventional_routing + /// is trying to receive on, in a different worker process. An ExcludeTypes rather than an + /// IncludeTypes: excludes are ANDed and includes are ORed, so an include would silently widen a + /// test's own filter in ConfigureConventions. See GH-3763. + /// + private static void leaveTheEndToEndQueueAlone(AmazonSqsMessageRoutingConvention convention) { - _host?.Dispose(); + convention.ExcludeTypes(t => t == typeof(EndToEndRoutedMessage)); + } + + public async ValueTask DisposeAsync() + { + if (_host == null) return; + + // StopAsync, not just Dispose: IHost.Dispose() tears down the container without ever + // running IHostedService.StopAsync, which leaves the SQS listeners polling. + await _host.StopAsync(); + _host.Dispose(); + _host = null!; } internal async Task ConfigureConventions(Action configure) @@ -29,7 +61,11 @@ internal async Task ConfigureConventions(Action { - opts.UseAmazonSqsTransport().UseConventionalRouting(configure).AutoProvision() + opts.UseAmazonSqsTransportLocally().UseConventionalRouting(c => + { + leaveTheEndToEndQueueAlone(c); + configure(c); + }).AutoProvision() .AutoPurgeOnStartup(); }).StartAsync(); } diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/EndToEndRoutedMessage.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/EndToEndRoutedMessage.cs new file mode 100644 index 000000000..d3bca8ab3 --- /dev/null +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/EndToEndRoutedMessage.cs @@ -0,0 +1,23 @@ +using Wolverine.Attributes; + +namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; + +/// +/// end_to_end_with_conventional_routing needs a queue nothing else in this namespace listens to. +/// +/// The CI shard runs this project across three worker PROCESSES partitioned by test class, so +/// CollectionPerAssembly only buys serialization inside one process. Every other conventional +/// routing class here stands up a host listening at sqs://routed for +/// ; a concurrent worker holding that listener receives the end-to-end +/// message and the tracked session times out waiting for a delivery that already happened +/// somewhere else. Its own message type gives it its own queue. See GH-3763. +/// +[MessageIdentity("end-to-end-routed")] +public class EndToEndRoutedMessage; + +public class EndToEndRoutedMessageHandler +{ + public void Handle(EndToEndRoutedMessage message) + { + } +} diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/conventional_listener_discovery.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/conventional_listener_discovery.cs index cab9230c5..be3014c8a 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/conventional_listener_discovery.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/conventional_listener_discovery.cs @@ -9,7 +9,6 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] public class conventional_listener_discovery : ConventionalRoutingContext { [Fact] diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/discover_with_naming_prefix.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/discover_with_naming_prefix.cs index 5c409fa28..d74352e2f 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/discover_with_naming_prefix.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/discover_with_naming_prefix.cs @@ -5,25 +5,24 @@ using Xunit; namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class discover_with_naming_prefix : IDisposable +public class discover_with_naming_prefix : IAsyncLifetime { - private readonly IHost _host; - private readonly ITestOutputHelper _output; + private IHost _host = null!; - public discover_with_naming_prefix(ITestOutputHelper output) + public async ValueTask InitializeAsync() { - _output = output; - _host = Host.CreateDefaultBuilder() + _host = await Host.CreateDefaultBuilder() .UseWolverine(opts => { - opts.UseAmazonSqsTransport().PrefixIdentifiers("zztop").UseConventionalRouting().AutoProvision() + opts.UseAmazonSqsTransportLocally().PrefixIdentifiers("zztop").UseConventionalRouting().AutoProvision() .AutoPurgeOnStartup(); - }).Start(); + }).StartAsync(); } - public void Dispose() + // StopAsync, not just Dispose -- see the note in ConventionalRoutingContext (GH-3763). + public async ValueTask DisposeAsync() { + await _host.StopAsync(); _host.Dispose(); } diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing.cs index 4ff6ceb6a..efc60f7c4 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing.cs @@ -6,8 +6,7 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class end_to_end_with_conventional_routing : IAsyncLifetime, IDisposable +public class end_to_end_with_conventional_routing : IAsyncLifetime { private IHost _receiver = null!; private IHost _sender = null!; @@ -16,24 +15,28 @@ public async ValueTask InitializeAsync() { _sender = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); + opts.UseAmazonSqsTransportLocally().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); opts.DisableConventionalDiscovery(); opts.ServiceName = "Sender"; }); _receiver = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); + opts.UseAmazonSqsTransportLocally().UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); opts.ServiceName = "Receiver"; }); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - - public void Dispose() + // StopAsync, not just Dispose: IHost.Dispose() tears down the container without ever running + // IHostedService.StopAsync, so the SQS listeners keep polling and steal messages from the next + // class in this namespace. See GH-3763. + public async ValueTask DisposeAsync() { - _sender?.Dispose(); - _receiver?.Dispose(); + await _sender.StopAsync(); + _sender.Dispose(); + + await _receiver.StopAsync(); + _receiver.Dispose(); } [Fact] @@ -43,11 +46,11 @@ public async Task send_from_one_node_to_another_all_with_conventional_routing() .AlsoTrack(_receiver) .IncludeExternalTransports() .Timeout(30.Seconds()) - .SendMessageAndWaitAsync(new RoutedMessage()); + .SendMessageAndWaitAsync(new EndToEndRoutedMessage()); var received = session .AllRecordsInOrder() - .Where(x => x.Envelope!.Message!.GetType() == typeof(RoutedMessage)) + .Where(x => x.Envelope!.Message!.GetType() == typeof(EndToEndRoutedMessage)) .Single(x => x.MessageEventType == MessageEventType.Received); received diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing_with_prefix.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing_with_prefix.cs index 7c81fe8a7..0bf481908 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing_with_prefix.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/end_to_end_with_conventional_routing_with_prefix.cs @@ -6,8 +6,7 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class end_to_end_with_conventional_routing_with_prefix : IAsyncLifetime, IDisposable +public class end_to_end_with_conventional_routing_with_prefix : IAsyncLifetime { private IHost _receiver = null!; private IHost _sender = null!; @@ -16,7 +15,7 @@ public async ValueTask InitializeAsync() { _sender = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport() + opts.UseAmazonSqsTransportLocally() .PrefixIdentifiers("shazaam") .UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); opts.DisableConventionalDiscovery(); @@ -25,19 +24,23 @@ public async ValueTask InitializeAsync() _receiver = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport() + opts.UseAmazonSqsTransportLocally() .PrefixIdentifiers("shazaam") .UseConventionalRouting().AutoProvision().AutoPurgeOnStartup(); opts.ServiceName = "Receiver"; }); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - - public void Dispose() + // StopAsync, not just Dispose: IHost.Dispose() tears down the container without ever running + // IHostedService.StopAsync, so the SQS listeners keep polling and steal messages from the next + // class in this namespace. See GH-3763. + public async ValueTask DisposeAsync() { - _sender?.Dispose(); - _receiver?.Dispose(); + await _sender.StopAsync(); + _sender.Dispose(); + + await _receiver.StopAsync(); + _receiver.Dispose(); } [Fact] diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_all_defaults.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_all_defaults.cs index dcfb834a2..6775e5c1f 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_all_defaults.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_all_defaults.cs @@ -5,19 +5,16 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class when_discovering_a_listening_endpoint_with_all_defaults : ConventionalRoutingContext, IAsyncLifetime +public class when_discovering_a_listening_endpoint_with_all_defaults : ConventionalRoutingContext { private readonly Uri theExpectedUri = "sqs://routed".ToUri(); private AmazonSqsQueue theQueue = null!; - public async ValueTask InitializeAsync() + public override async ValueTask InitializeAsync() { theQueue = (await theRuntime()).Endpoints.EndpointFor(theExpectedUri).ShouldBeOfType(); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - [Fact] public void endpoint_should_be_a_listener() { diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_overridden_queue_naming.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_overridden_queue_naming.cs index 4e1ca56ba..73c58a35e 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_overridden_queue_naming.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_listening_endpoint_with_overridden_queue_naming.cs @@ -4,13 +4,12 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class when_discovering_a_listening_endpoint_with_overridden_queue_naming : ConventionalRoutingContext, IAsyncLifetime +public class when_discovering_a_listening_endpoint_with_overridden_queue_naming : ConventionalRoutingContext { private readonly Uri theExpectedUri = "sqs://routedmessage2".ToUri(); private AmazonSqsQueue theQueue = null!; - public async ValueTask InitializeAsync() + public override async ValueTask InitializeAsync() { await ConfigureConventions(c => c.QueueNameForListener(t => t.Name.ToLower() + "2")); @@ -19,8 +18,6 @@ public async ValueTask InitializeAsync() theQueue = runtime.Endpoints.EndpointFor(theExpectedUri).ShouldBeOfType(); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - [Fact] public void endpoint_should_be_a_listener() { diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_sender_with_all_defaults.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_sender_with_all_defaults.cs index 9aa386681..404052e9a 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_sender_with_all_defaults.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_discovering_a_sender_with_all_defaults.cs @@ -6,18 +6,15 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class when_discovering_a_sender_with_all_defaults : ConventionalRoutingContext, IAsyncLifetime +public class when_discovering_a_sender_with_all_defaults : ConventionalRoutingContext { private MessageRoute theRoute = null!; - public async ValueTask InitializeAsync() + public override async ValueTask InitializeAsync() { theRoute = (await PublishingRoutesFor()).Single().As(); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - [Fact] public void should_have_exactly_one_route() { diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_using_handler_type_naming.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_using_handler_type_naming.cs index 660c81bc8..df0744c81 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_using_handler_type_naming.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/ConventionalRouting/when_using_handler_type_naming.cs @@ -10,8 +10,7 @@ namespace Wolverine.AmazonSqs.Tests.ConventionalRouting; -[Trait("Category", "Flaky")] -public class when_using_handler_type_naming : IAsyncLifetime, IDisposable +public class when_using_handler_type_naming : IAsyncLifetime { private IHost _host = null!; private IWolverineRuntime _runtime = null!; @@ -20,7 +19,7 @@ public async ValueTask InitializeAsync() { _host = await WolverineHost.ForAsync(opts => { - opts.UseAmazonSqsTransport() + opts.UseAmazonSqsTransportLocally() .UseConventionalRouting(NamingSource.FromHandlerType) .AutoProvision() .AutoPurgeOnStartup(); @@ -52,10 +51,10 @@ public void listener_should_be_active() .ShouldBeTrue($"Expected active listener containing '{expectedName}'"); } - ValueTask IAsyncDisposable.DisposeAsync() => ValueTask.CompletedTask; - - public void Dispose() + // StopAsync, not just Dispose -- see the note in ConventionalRoutingContext (GH-3763). + public async ValueTask DisposeAsync() { + await _host.StopAsync(); _host.Dispose(); } } diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/Samples/Bootstrapping.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/Samples/Bootstrapping.cs index efa7513df..ffc8f1eab 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/Samples/Bootstrapping.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/Samples/Bootstrapping.cs @@ -12,7 +12,6 @@ namespace Wolverine.AmazonSqs.Tests.Samples; -[Trait("Category", "Flaky")] public class Bootstrapping { private async Task use_named_brokers() @@ -355,8 +354,10 @@ private async Task publish_raw_json() #endregion } - [Fact] - public async Task customize_mappers() + // Compile-checked only, like every other sample in this file. The snippet is documentation, so + // it shows the real `UseAmazonSqsTransport()` a reader would write — which means running it + // would talk to a real AWS account, and CI has no credentials. + private async Task customize_mappers() { #region sample_apply_custom_sqs_mapping using var host = await Host.CreateDefaultBuilder() diff --git a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/concurrency_resilient_sharded_processing.cs b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/concurrency_resilient_sharded_processing.cs index f75298905..189b6c9d8 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs.Tests/concurrency_resilient_sharded_processing.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs.Tests/concurrency_resilient_sharded_processing.cs @@ -12,7 +12,6 @@ using Xunit; namespace Wolverine.AmazonSqs.Tests; -[Trait("Category", "Flaky")] public class concurrency_resilient_sharded_processing { private readonly ITestOutputHelper _output; diff --git a/src/Transports/AWS/Wolverine.AmazonSqs/Internal/AmazonSqsTransport.cs b/src/Transports/AWS/Wolverine.AmazonSqs/Internal/AmazonSqsTransport.cs index aaa1d58de..56e283cc1 100644 --- a/src/Transports/AWS/Wolverine.AmazonSqs/Internal/AmazonSqsTransport.cs +++ b/src/Transports/AWS/Wolverine.AmazonSqs/Internal/AmazonSqsTransport.cs @@ -1,3 +1,5 @@ +using System.Security.Cryptography; +using System.Text; using Amazon.Runtime; using Amazon.SQS; using Amazon.SQS.Model; @@ -135,23 +137,79 @@ internal AmazonSqsTransport(IAmazonSQS client) : this() /// public bool SystemQueuesEnabled { get; set; } + /// + /// The hard limit Amazon SQS puts on a queue name. The .fifo suffix counts against it. + /// + public const int MaximumQueueNameLength = 80; + + /// + /// Coerce an identifier into something Amazon SQS will actually accept: "Can only include + /// alphanumeric characters, hyphens, or underscores. 1 to 80 in length". + /// + /// Conventional routing derives queue names from message type names, so the raw input can carry + /// characters SQS rejects (Handle(Item[]), generics, nested types) or simply run past 80 + /// characters once a prefix is applied. Either one fails CreateQueue with a 400, and + /// because broker initialization provisions every queue together, one bad name takes down + /// startup for every conventionally-routed host in the assembly. See GH-3763, and GH-3786 for + /// the same defect on Azure Service Bus. + /// + /// This is a no-op for every name that works today: a name SQS rejects could never have been + /// provisioned in the first place. + /// public static string SanitizeSqsName(string identifier) { //AWS requires FIFO queues to have a `.fifo` suffix var suffixIndex = identifier.LastIndexOf(".fifo", StringComparison.OrdinalIgnoreCase); + var suffix = string.Empty; + var name = identifier; + if (suffixIndex != -1) // ".fifo" suffix found { - var prefix = identifier[..suffixIndex]; - var suffix = identifier[suffixIndex..]; + suffix = identifier[suffixIndex..]; + name = identifier[..suffixIndex]; + } + + return truncateToLimit(substituteIllegalCharacters(name), suffix); + } + + private static string substituteIllegalCharacters(string name) + { + var characters = new char[name.Length]; + + for (var i = 0; i < name.Length; i++) + { + var c = name[i]; - prefix = prefix.Replace('.', Separator); + // '.' has always mapped to the identifier separator, and plenty of existing queue names + // depend on that exact spelling. Everything else illegal becomes '_' -- substituting + // rather than stripping is what keeps Item[] separable from Item. + characters[i] = c switch + { + '.' => Separator, + '-' or '_' => c, + _ => char.IsAsciiLetterOrDigit(c) ? c : '_' + }; + } - return prefix + suffix; + return new string(characters); + } + + private static string truncateToLimit(string name, string suffix) + { + if (name.Length + suffix.Length <= MaximumQueueNameLength) + { + return name + suffix; } - // ".fifo" suffix not found - return identifier.Replace('.', Separator); + // Truncation alone would collide two long names that share a prefix, and conventionally + // routed names are namespace-qualified type names, which very often do. Append a stable + // digest of the full name so the result stays unique -- and stays the SAME across processes + // and machines, which rules out string.GetHashCode() (randomized per process on .NET Core). + var digest = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(name))).ToLowerInvariant()[..8]; + var budget = MaximumQueueNameLength - suffix.Length - digest.Length - 1; + + return string.Concat(name.AsSpan(0, budget), Separator.ToString(), digest, suffix); } public override string SanitizeIdentifier(string identifier)