Complete synchronized storage session before storing outbox data (#195)

* Complete synchronized storage session before storing outbox data (#191) * Fix packages not uploading on CI runs * update testing references
Particular · May 1, 2023 · ef94e7e · ef94e7e
1 parent e932106
commit ef94e7e
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 12 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -22,7 +22,7 @@ jobs:
           fail-fast: false
       steps:
       - name: Checkout
-        uses: actions/checkout@v3.2.0
+        uses: actions/checkout@v3.5.2
         with:
           fetch-depth: 0
       - name: Setup .NET SDK
@@ -34,11 +34,11 @@ jobs:
       - name: Build
         run: dotnet build src --configuration Release
       - name: Upload packages
-        if: matrix.upload-packages && runner.os == 'Windows'
-        uses: actions/upload-artifact@v3.1.1
+        if: runner.os == 'Windows'
+        uses: actions/upload-artifact@v3.1.2
         with:
           name: NuGet packages
           path: nugets/
           retention-days: 7
       - name: Run tests
-        uses: Particular/run-tests-action@v1.4.0
+        uses: Particular/run-tests-action@v1.5.1
diff --git a/...nsactionalSession.AcceptanceTests/NServiceBus.TransactionalSession.AcceptanceTests.csproj b/...nsactionalSession.AcceptanceTests/NServiceBus.TransactionalSession.AcceptanceTests.csproj
@@ -9,9 +9,9 @@
 
   <ItemGroup>
     <PackageReference Include="GitHubActionsTestLogger" Version="2.0.1" />
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.1" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.5.0" />
     <PackageReference Include="NUnit" Version="3.13.3" />
-    <PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
+    <PackageReference Include="NUnit3TestAdapter" Version="4.4.2" />
     <PackageReference Include="NServiceBus.AcceptanceTesting" Version="7.8.0" />
   </ItemGroup>
 

diff --git a/src/NServiceBus.TransactionalSession.Tests/NServiceBus.TransactionalSession.Tests.csproj b/src/NServiceBus.TransactionalSession.Tests/NServiceBus.TransactionalSession.Tests.csproj
@@ -9,10 +9,10 @@
 
   <ItemGroup>
     <PackageReference Include="GitHubActionsTestLogger" Version="2.0.1" />
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.1" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.5.0" />
     <PackageReference Include="NServiceBus.Testing" Version="7.4.1" />
     <PackageReference Include="NUnit" Version="3.13.3" />
-    <PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
+    <PackageReference Include="NUnit3TestAdapter" Version="4.4.2" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/NServiceBus.TransactionalSession.Tests/OutboxTransactionalSessionTests.cs b/src/NServiceBus.TransactionalSession.Tests/OutboxTransactionalSessionTests.cs
@@ -178,8 +178,25 @@ public async Task Commit_should_send_control_message_when_outbox_fails()
             Assert.AreEqual(bool.TrueString, controlMessage.Message.Headers[Headers.ControlMessageHeader]);
 
             var outboxTransaction = outboxStorage.StartedTransactions.Single();
-            Assert.IsFalse(fakeSynchronizedStorage.Completed, "should not have completed synchronized storage session");
-            Assert.IsFalse(outboxTransaction.Commited, "should not have commited outbox operations");
+            Assert.IsFalse(outboxTransaction.Commited, "should not have committed outbox operations");
+        }
+
+        [Test]
+        public async Task Commit_should_complete_synchronized_storage_session_before_outbox_store()
+        {
+            var fakeSynchronizedStorage = new FakeSynchronizedStorage();
+            var messageSession = new FakeMessageSession();
+            var dispatcher = new FakeDispatcher();
+            var outboxStorage = new FakeOutboxStorage { StoreCallback = (_, _, _) => throw new Exception("some error") };
+            using var session = new OutboxTransactionalSession(outboxStorage, new CompletableSynchronizedStorageSessionAdapter(fakeSynchronizedStorage, fakeSynchronizedStorage), messageSession, dispatcher, Enumerable.Empty<IOpenSessionOptionsCustomization>(), "queue address");
+
+            await session.Open(new FakeOpenSessionOptions());
+            await session.Send(new object());
+            Assert.ThrowsAsync<Exception>(async () => await session.Commit());
+
+            var outboxTransaction = outboxStorage.StartedTransactions.Single();
+            Assert.IsTrue(fakeSynchronizedStorage.Completed, "should have completed synchronized storage session to match the receive pipeline behavior");
+            Assert.IsFalse(outboxTransaction.Commited, "should not have committed outbox operations");
         }
 
         [Test]

diff --git a/src/NServiceBus.TransactionalSession/OutboxTransactionalSession.cs b/src/NServiceBus.TransactionalSession/OutboxTransactionalSession.cs
@@ -49,13 +49,13 @@ protected override async Task CommitInternal(CancellationToken cancellationToken
             var outgoingMessages = new TransportOperations(new TransportTransportOperation(message, new UnicastAddressTag(physicalQueueAddress)));
             await dispatcher.Dispatch(outgoingMessages, new TransportTransaction(), new ContextBag()).ConfigureAwait(false);
 
+            await synchronizedStorageSession.CompleteAsync().ConfigureAwait(false);
+
             var outboxMessage =
                 new OutboxMessage(SessionId, ConvertToOutboxOperations(pendingOperations.Operations));
             await outboxStorage.Store(outboxMessage, outboxTransaction, Context)
                 .ConfigureAwait(false);
 
-            await synchronizedStorageSession.CompleteAsync().ConfigureAwait(false);
-
             await outboxTransaction.Commit().ConfigureAwait(false);
         }