Fix the Oracle queue Uri identity round-trip, and finish GH-3808 (GH-3820)

src/Persistence/Oracle/OracleTests/Transport/clear_all_wolverine_storage.cs

@@ -1,5 +1,4 @@
 using IntegrationTests;
-using JasperFx.Core;
 using Oracle.ManagedDataAccess.Client;
 using Weasel.Oracle;
 using Wolverine;
@@ -77,17 +76,21 @@ protected override async Task beforeHostAsync()
     /// 30835978599) and burned this class's two retries in the run after it. Left alone the count
     /// climbs monotonically — a local repro reached 3.</para>
     ///
-    /// <para>Now that the DROP resolves a table that actually exists, ORA-00054 becomes reachable
-    /// for the first time: this class is the only provider in the suite that attaches a listener,
-    /// and a polling listener holds a TM lock on the queue table (see the matching retry in
-    /// <c>OracleQueue.TeardownAsync</c>). Hence 942 = done, 54 = wait and retry, anything else
-    /// throws. Exhausting the retries throws too, rather than returning quietly: "could not drop the
-    /// queue table" is a far better failure than a count assertion two calls later that names
-    /// neither the lock nor the leftover rows.</para>
+    /// <para>Once the DROP started resolving a table that actually exists, ORA-00054 became
+    /// reachable for the first time, because this class was then the only provider in the suite
+    /// attaching a listener and a polling listener holds a TM lock on the queue table (see the
+    /// matching retry in <c>OracleQueue.TeardownAsync</c>). GH-3820 removed that listener — the
+    /// casing bug that forced it is fixed — so the contention should be gone rather than merely
+    /// retried around. The retry stays as a belt-and-braces guard: a host from a prior class in
+    /// this collection can still be tearing its connections down. Hence 942 = done, 54 = wait and
+    /// retry, anything else throws. Exhausting the retries throws too, rather than returning
+    /// quietly: "could not drop the queue table" is a far better failure than a count assertion two
+    /// calls later that names neither the lock nor the leftover rows.</para>
     /// </summary>
     private static async Task dropTableWithRetryAsync(OracleConnection conn, string table)
     {
         const int maxAttempts = 5;
+        OracleException? lastBusy = null;
         for (var attempt = 1; attempt <= maxAttempts; attempt++)
         {
             try
@@ -102,29 +105,44 @@ private static async Task dropTableWithRetryAsync(OracleConnection conn, string
                 // ORA-00942: table or view does not exist — nothing to drop, which is the goal.
                 return;
             }
-            catch (OracleException e) when (e.Number == 54 && attempt < maxAttempts)
+            catch (OracleException e) when (e.Number == 54)
             {
                 // ORA-00054: resource busy. The prior host's connections are still tearing down.
-                await Task.Delay(TimeSpan.FromSeconds(2 * attempt));
+                //
+                // Deliberately NOT guarded by `attempt < maxAttempts`. Folding the loop counter into
+                // the `when` clause meant that on the final attempt the guard was false, the raw
+                // OracleException escaped the loop, and the descriptive throw below was unreachable
+                // dead code -- it never ran once. GH-3820.
+                lastBusy = e;
+                if (attempt < maxAttempts)
+                {
+                    await Task.Delay(TimeSpan.FromSeconds(2 * attempt));
+                }
             }
         }
 
         throw new InvalidOperationException(
             $"Could not drop {TransportSchemaName}.{table} after {maxAttempts} attempts — it is still locked by " +
-            "another session. The test would otherwise start against a table holding a previous test's rows.");
+            "another session. The test would otherwise start against a table holding a previous test's rows.",
+            lastBusy);
     }
 
     protected override void ConfigureStorage(WolverineOptions options)
     {
         options.PersistMessagesWithOracle(Servers.OracleConnectionString, SchemaName)
             .EnableMessageTransport(t => t.TransportSchemaName(TransportSchemaName));
 
-        // Registered through the listener rather than as a subscriber, unlike the other
-        // providers: Oracle uppercases queue identifiers, but Uri.Host lowercases, so the
-        // publishing.To(queue.Uri) inside ToOracleQueue() resolves a *second* endpoint over the
-        // same physical tables. The polling interval keeps the listener from draining the queue
-        // out from under the assertions.
-        options.ListenToOracleQueue(QueueName).PollingInterval(1.Hours());
+        // Subscriber only -- no listener, so nothing drains the queue out from under the
+        // assertions before the reset runs.
+        //
+        // This class used to attach a listener instead, because the publishing.To(queue.Uri)
+        // inside ToOracleQueue() resolved a *second* endpoint over the same physical tables:
+        // Oracle uppercases queue identifiers but Uri.Host lowercases, and
+        // OracleTransport.findEndpointByUri looked the name up without correcting it. That was a
+        // product bug, not a test quirk, and it is fixed in GH-3820 -- so this can now match every
+        // other provider in the suite. The listener was also the only thing holding a TM lock on
+        // the queue table, which is what made the ORA-00054 in beforeHostAsync reachable at all.
+        options.PublishAllMessages().ToOracleQueue(QueueName);
     }
 
     protected override ValueTask sendToQueueAsync(Envelope envelope)

src/Persistence/Oracle/OracleTests/Transport/oracle_queue_uri_identity.cs

@@ -0,0 +1,45 @@
+using Shouldly;
+using Wolverine.Oracle.Transport;
+using Xunit;
+
+namespace OracleTests.Transport;
+
+/// <summary>
+/// GH-3820. Oracle is the only database transport whose <c>SanitizeIdentifier</c> upper-cases
+/// (Postgres and SQL Server both lower-case), and it inherited its siblings' <c>findEndpointByUri</c>
+/// verbatim: <c>Queues[uri.Host]</c>. <see cref="System.Uri"/> normalises the authority to lower
+/// case, so an uppercased queue name never came back out of its own Uri — the lookup silently
+/// created a *second* endpoint over the same physical queue tables.
+/// </summary>
+public class oracle_queue_uri_identity
+{
+    [Fact]
+    public void queue_uri_round_trips_to_the_same_endpoint()
+    {
+        var transport = new OracleTransport();
+        var queue = transport.Queues[transport.MaybeCorrectName("resetone")];
+
+        // Oracle upper-cases identifiers...
+        queue.Name.ShouldBe("RESETONE");
+
+        // ...but System.Uri lower-cases the authority, so the Uri cannot carry the casing back.
+        queue.Uri.Host.ShouldBe("resetone");
+
+        // Resolving the endpoint from its own Uri must therefore correct the name again, or it
+        // hands back a brand new queue pointed at the same tables.
+        transport.GetOrCreateEndpoint(queue.Uri).ShouldBeSameAs(queue);
+        transport.Queues.Count().ShouldBe(1);
+    }
+
+    [Fact]
+    public void queue_uri_round_trips_when_the_name_needs_dash_correction()
+    {
+        var transport = new OracleTransport();
+        var queue = transport.Queues[transport.MaybeCorrectName("reset-two")];
+
+        queue.Name.ShouldBe("RESET_TWO");
+
+        transport.GetOrCreateEndpoint(queue.Uri).ShouldBeSameAs(queue);
+        transport.Queues.Count().ShouldBe(1);
+    }
+}

src/Persistence/Oracle/Wolverine.Oracle/Transport/OracleTransport.cs

@@ -46,7 +46,16 @@ public override string SanitizeIdentifier(string identifier)
 
     protected override OracleQueue findEndpointByUri(Uri uri)
     {
-        var queueName = uri.Host;
+        // GH-3820. Unlike the Postgres and SQL Server transports this was copied from, Oracle
+        // upper-cases identifiers (see SanitizeIdentifier) while System.Uri normalises the
+        // authority to lower case. A bare Queues[uri.Host] therefore never matches the key the
+        // queue was registered under, and LightweightCache quietly mints a *second* OracleQueue
+        // over the same physical tables. Correcting the name here is safe and idempotent: the
+        // host segment already went through SanitizeIdentifier when the Uri was built, so the
+        // only thing left to undo is Uri's lower-casing. Note SanitizeIdentifier rather than
+        // MaybeCorrectName -- the host already carries any IdentifierPrefix, which
+        // MaybeCorrectName would prepend a second time.
+        var queueName = SanitizeIdentifier(uri.Host);
         return Queues[queueName];
     }