Move Raven ID out of persisence contract

src/ServiceControl.AcceptanceTests/Monitoring/CustomChecks/When_email_notifications_are_enabled.cs

@@ -64,16 +64,12 @@ class SetupNotificationSettings(INotificationsDataStore notificationsDataStore)
         {
             public async Task StartAsync(CancellationToken cancellationToken)
             {
-                using var notificationsManager = await notificationsDataStore.CreateNotificationsManager();
+                await using var notificationsManager = await notificationsDataStore.CreateNotificationsManager();
 
                 var settings = await notificationsManager.LoadSettings();
-                settings.Email = new EmailNotifications
-                {
-                    Enabled = true,
-                    From = "YouServiceControl@particular.net",
-                    To = "WhoeverMightBeConcerned@particular.net",
-                };
-
+                settings.Email.Enabled = true;
+                settings.Email.From = "YouServiceControl@particular.net";
+                settings.Email.To = "WhoeverMightBeConcerned@particular.net";
                 await notificationsManager.SaveChanges();
             }
 

src/ServiceControl.Persistence.EFCore/Implementation/EditFailedMessagesManager.cs

@@ -20,9 +20,10 @@ public Task SetFailedMessageAsResolved() =>
     public Task SaveChanges() =>
         throw new NotImplementedException();
 
-    public void Dispose()
+    public ValueTask DisposeAsync()
     {
         // Nothing to dispose yet
         GC.SuppressFinalize(this);
+        return ValueTask.CompletedTask;
     }
 }

src/ServiceControl.Persistence.EFCore/Implementation/NotificationsManager.cs

@@ -4,15 +4,16 @@ namespace ServiceControl.Persistence.EFCore.Implementation;
 
 public class NotificationsManager : INotificationsManager
 {
-    public Task<NotificationsSettings> LoadSettings(TimeSpan? cacheTimeout = null) =>
+    public Task<NotificationsSettings> LoadSettings() =>
         throw new NotImplementedException();
 
     public Task SaveChanges() =>
         throw new NotImplementedException();
 
-    public void Dispose()
+    public ValueTask DisposeAsync()
     {
         // Nothing to dispose yet
         GC.SuppressFinalize(this);
+        return ValueTask.CompletedTask;
     }
 }

src/ServiceControl.Persistence.RavenDB/Editing/NotificationSettingsDocument.cs

@@ -0,0 +1,9 @@
+namespace ServiceControl.Persistence.RavenDB.Editing;
+
+using Notifications;
+
+class NotificationsSettingsDocument
+{
+    public string Id { get; set; }
+    public EmailNotifications Email { get; set; } = new();
+}

src/ServiceControl.Persistence.RavenDB/Editing/NotificationsManager.cs

@@ -8,27 +8,24 @@
     class NotificationsManager(IAsyncDocumentSession session) : AbstractSessionManager(session), INotificationsManager
     {
         const string SingleDocumentId = "NotificationsSettings/All";
-        static readonly TimeSpan CacheTimeoutDefault = TimeSpan.FromMinutes(5); // Raven requires this to be at least 1 second
+        static readonly TimeSpan CacheTimeout = TimeSpan.FromMinutes(5); // Raven requires this to be at least 1 second
 
-        public async Task<NotificationsSettings> LoadSettings(TimeSpan? cacheTimeout = null)
+        public async Task<NotificationsSettings> LoadSettings()
         {
-            using var aggressivelyCacheFor = await Session.Advanced.DocumentStore.AggressivelyCacheForAsync(cacheTimeout ?? CacheTimeoutDefault);
+            using var aggressivelyCacheFor = await Session.Advanced.DocumentStore.AggressivelyCacheForAsync(CacheTimeout);
             var settings = await Session
-                .LoadAsync<NotificationsSettings>(SingleDocumentId);
+                .LoadAsync<NotificationsSettingsDocument>(SingleDocumentId);
 
-            if (settings != null)
+            if (settings == null)
             {
-                return settings;
+                settings = new NotificationsSettingsDocument { Id = SingleDocumentId };
+                await Session.StoreAsync(settings);
             }
 
-            settings = new NotificationsSettings
+            return new NotificationsSettings()
             {
-                Id = SingleDocumentId
+                Email = settings.Email
             };
-
-            await Session.StoreAsync(settings);
-
-            return settings;
         }
     }
 }

src/ServiceControl.Persistence.RavenDB/Transactions/RavenTransactionalDataStore.cs

@@ -8,6 +8,10 @@ abstract class AbstractSessionManager(IAsyncDocumentSession session) : IDataSess
         protected IAsyncDocumentSession Session { get; } = session;
 
         public Task SaveChanges() => Session.SaveChangesAsync();
-        public void Dispose() => Session.Dispose();
+        public ValueTask DisposeAsync()
+        {
+            Session.Dispose();
+            return ValueTask.CompletedTask;
+        }
     }
 }

src/ServiceControl.Persistence.Tests.PostgreSql/ServiceControl.Persistence.Tests.PostgreSql.csproj

@@ -39,6 +39,7 @@
     <Compile Remove="..\ServiceControl.Persistence.Tests\Throughput\EndpointsTests.cs" />
     <Compile Remove="..\ServiceControl.Persistence.Tests\Throughput\ReportMasksTests.cs" />
     <Compile Remove="..\ServiceControl.Persistence.Tests\RetryStateTests.cs" />
+    <Compile Remove="..\ServiceControl.Persistence.Tests\NotificationsDataStoreTests.cs" />
   </ItemGroup>
 
 </Project>

src/ServiceControl.Persistence.Tests.SqlServer/ServiceControl.Persistence.Tests.SqlServer.csproj

@@ -39,6 +39,7 @@
     <Compile Remove="..\ServiceControl.Persistence.Tests\Throughput\EndpointsTests.cs" />
     <Compile Remove="..\ServiceControl.Persistence.Tests\Throughput\ReportMasksTests.cs" />
     <Compile Remove="..\ServiceControl.Persistence.Tests\RetryStateTests.cs" />
+    <Compile Remove="..\ServiceControl.Persistence.Tests\NotificationsDataStoreTests.cs" />
   </ItemGroup>
 
 </Project>

src/ServiceControl.Persistence.Tests/NotificationsDataStoreTests.cs

@@ -0,0 +1,167 @@
+namespace ServiceControl.Persistence.Tests;
+
+using System.Threading.Tasks;
+using NUnit.Framework;
+
+class NotificationsDataStoreTests : PersistenceTestBase
+{
+    [Test, CancelAfter(30_000)]
+    public async Task LoadSettings_returns_defaults_when_no_settings_exist()
+    {
+        await using var manager = await NotificationsStore.CreateNotificationsManager();
+
+        var settings = await manager.LoadSettings();
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(settings, Is.Not.Null);
+            Assert.That(settings.Email, Is.Not.Null);
+            Assert.That(settings.Email.Enabled, Is.False);
+            Assert.That(settings.Email.SmtpServer, Is.Null);
+            Assert.That(settings.Email.SmtpPort, Is.Null);
+            Assert.That(settings.Email.EnableTLS, Is.False);
+            Assert.That(settings.Email.From, Is.Null);
+            Assert.That(settings.Email.To, Is.Null);
+            Assert.That(settings.Email.AuthenticationAccount, Is.Null);
+            Assert.That(settings.Email.AuthenticationPassword, Is.Null);
+        }
+    }
+
+    [Test, CancelAfter(30_000)]
+    public async Task SaveChanges_persists_email_settings_round_trip()
+    {
+        await using (var manager = await NotificationsStore.CreateNotificationsManager())
+        {
+            var settings = await manager.LoadSettings();
+
+            settings.Email.Enabled = true;
+            settings.Email.SmtpServer = "smtp.example.com";
+            settings.Email.SmtpPort = 587;
+            settings.Email.EnableTLS = true;
+            settings.Email.From = "sc@example.com";
+            settings.Email.To = "ops@example.com";
+            settings.Email.AuthenticationAccount = "user";
+            settings.Email.AuthenticationPassword = "p@ssw0rd";
+
+            await manager.SaveChanges();
+        }
+
+        await CompleteDatabaseOperation();
+
+        await using var verifyManager = await NotificationsStore.CreateNotificationsManager();
+        var loaded = await verifyManager.LoadSettings();
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(loaded.Email.Enabled, Is.True);
+            Assert.That(loaded.Email.SmtpServer, Is.EqualTo("smtp.example.com"));
+            Assert.That(loaded.Email.SmtpPort, Is.EqualTo(587));
+            Assert.That(loaded.Email.EnableTLS, Is.True);
+            Assert.That(loaded.Email.From, Is.EqualTo("sc@example.com"));
+            Assert.That(loaded.Email.To, Is.EqualTo("ops@example.com"));
+            Assert.That(loaded.Email.AuthenticationAccount, Is.EqualTo("user"));
+            Assert.That(loaded.Email.AuthenticationPassword, Is.EqualTo("p@ssw0rd"));
+        }
+    }
+
+    [Test, CancelAfter(30_000)]
+    public async Task Toggling_enabled_is_persisted()
+    {
+        await using (var manager = await NotificationsStore.CreateNotificationsManager())
+        {
+            var settings = await manager.LoadSettings();
+            settings.Email.Enabled = true;
+            await manager.SaveChanges();
+        }
+
+        await CompleteDatabaseOperation();
+
+        await using (var manager = await NotificationsStore.CreateNotificationsManager())
+        {
+            var settings = await manager.LoadSettings();
+            Assert.That(settings.Email.Enabled, Is.True);
+
+            settings.Email.Enabled = false;
+            await manager.SaveChanges();
+        }
+
+        await CompleteDatabaseOperation();
+
+        await using var verifyManager = await NotificationsStore.CreateNotificationsManager();
+        var final = await verifyManager.LoadSettings();
+        Assert.That(final.Email.Enabled, Is.False);
+    }
+
+    [Test, CancelAfter(30_000)]
+    public async Task LoadSettings_returns_previously_saved_settings()
+    {
+        await using (var manager = await NotificationsStore.CreateNotificationsManager())
+        {
+            var settings = await manager.LoadSettings();
+            settings.Email.SmtpServer = "configured.server";
+            settings.Email.SmtpPort = 2525;
+            await manager.SaveChanges();
+        }
+
+        await CompleteDatabaseOperation();
+
+        await using var manager2 = await NotificationsStore.CreateNotificationsManager();
+        var loaded = await manager2.LoadSettings();
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(loaded.Email.SmtpServer, Is.EqualTo("configured.server"));
+            Assert.That(loaded.Email.SmtpPort, Is.EqualTo(2525));
+            // Untouched fields keep their defaults
+            Assert.That(loaded.Email.Enabled, Is.False);
+            Assert.That(loaded.Email.EnableTLS, Is.False);
+        }
+    }
+
+    [Test, CancelAfter(30_000)]
+    public async Task Updating_individual_fields_preserves_others()
+    {
+        await using (var manager = await NotificationsStore.CreateNotificationsManager())
+        {
+            var settings = await manager.LoadSettings();
+            settings.Email.Enabled = true;
+            settings.Email.SmtpServer = "original.smtp";
+            settings.Email.SmtpPort = 25;
+            settings.Email.EnableTLS = false;
+            settings.Email.From = "from@orig";
+            settings.Email.To = "to@orig";
+            settings.Email.AuthenticationAccount = "acct";
+            settings.Email.AuthenticationPassword = "secret";
+            await manager.SaveChanges();
+        }
+
+        await CompleteDatabaseOperation();
+
+        await using (var manager = await NotificationsStore.CreateNotificationsManager())
+        {
+            var settings = await manager.LoadSettings();
+            settings.Email.SmtpServer = "updated.smtp";
+            settings.Email.EnableTLS = true;
+            await manager.SaveChanges();
+        }
+
+        await CompleteDatabaseOperation();
+
+        await using var verifyManager = await NotificationsStore.CreateNotificationsManager();
+        var loaded = await verifyManager.LoadSettings();
+
+        using (Assert.EnterMultipleScope())
+        {
+            // Updated fields
+            Assert.That(loaded.Email.SmtpServer, Is.EqualTo("updated.smtp"));
+            Assert.That(loaded.Email.EnableTLS, Is.True);
+            // Preserved fields
+            Assert.That(loaded.Email.Enabled, Is.True);
+            Assert.That(loaded.Email.SmtpPort, Is.EqualTo(25));
+            Assert.That(loaded.Email.From, Is.EqualTo("from@orig"));
+            Assert.That(loaded.Email.To, Is.EqualTo("to@orig"));
+            Assert.That(loaded.Email.AuthenticationAccount, Is.EqualTo("acct"));
+            Assert.That(loaded.Email.AuthenticationPassword, Is.EqualTo("secret"));
+        }
+    }
+}

src/ServiceControl.Persistence.Tests/Recoverability/EditMessageTests.cs

@@ -76,7 +76,7 @@ public async Task Should_discard_edit_when_different_edit_already_exists()
 
             _ = await CreateAndStoreFailedMessage(failedMessageId);
 
-            using (var editFailedMessagesManager = await EditFailedMessagesStore.CreateEditFailedMessageManager())
+            await using (var editFailedMessagesManager = await EditFailedMessagesStore.CreateEditFailedMessageManager())
             {
                 _ = await editFailedMessagesManager.GetFailedMessage(failedMessageId);
                 await editFailedMessagesManager.SetCurrentEditingRequestId(previousEdit);
@@ -88,7 +88,7 @@ public async Task Should_discard_edit_when_different_edit_already_exists()
             // Act
             await handler.Handle(message, new TestableMessageHandlerContext());
 
-            using (var editFailedMessagesManagerAssert = await EditFailedMessagesStore.CreateEditFailedMessageManager())
+            await using (var editFailedMessagesManagerAssert = await EditFailedMessagesStore.CreateEditFailedMessageManager())
             {
                 var failedMessage = await editFailedMessagesManagerAssert.GetFailedMessage(failedMessageId);
                 var editId = await editFailedMessagesManagerAssert.GetCurrentEditingRequestId(failedMessageId);
@@ -125,18 +125,16 @@ public async Task Should_dispatch_edited_message_when_first_edit()
                 Assert.That(dispatchedMessage.Item1.Message.Headers["someKey"], Is.EqualTo("someValue"));
             }
 
-            using (var x = await EditFailedMessagesStore.CreateEditFailedMessageManager())
-            {
-                var failedMessage2 = await x.GetFailedMessage(failedMessage.UniqueMessageId);
-                Assert.That(failedMessage2, Is.Not.Null, "Edited failed message");
+            await using var x = await EditFailedMessagesStore.CreateEditFailedMessageManager();
+            var failedMessage2 = await x.GetFailedMessage(failedMessage.UniqueMessageId);
+            Assert.That(failedMessage2, Is.Not.Null, "Edited failed message");
 
-                var editId = await x.GetCurrentEditingRequestId(failedMessage2.UniqueMessageId);
+            var editId = await x.GetCurrentEditingRequestId(failedMessage2.UniqueMessageId);
 
-                using (Assert.EnterMultipleScope())
-                {
-                    Assert.That(failedMessage2.Status, Is.EqualTo(FailedMessageStatus.Resolved), "Failed message status");
-                    Assert.That(editId, Is.EqualTo(handlerContent.MessageId), "MessageId");
-                }
+            using (Assert.EnterMultipleScope())
+            {
+                Assert.That(failedMessage2.Status, Is.EqualTo(FailedMessageStatus.Resolved), "Failed message status");
+                Assert.That(editId, Is.EqualTo(handlerContent.MessageId), "MessageId");
             }
         }
 

src/ServiceControl.Persistence/IDataSessionManager.cs

@@ -3,7 +3,7 @@
     using System;
     using System.Threading.Tasks;
 
-    public interface IDataSessionManager : IDisposable
+    public interface IDataSessionManager : IAsyncDisposable
     {
         Task SaveChanges();
     }

src/ServiceControl.Persistence/INotificationsManager.cs

@@ -1,11 +1,10 @@
 namespace ServiceControl.Persistence
 {
-    using System;
     using System.Threading.Tasks;
     using Notifications;
 
     public interface INotificationsManager : IDataSessionManager
     {
-        Task<NotificationsSettings> LoadSettings(TimeSpan? cacheTimeout = null);
+        Task<NotificationsSettings> LoadSettings();
     }
 }

src/ServiceControl.Persistence/NotificationsSettings.cs

@@ -2,8 +2,6 @@
 {
     public class NotificationsSettings
     {
-        public string Id { get; set; }
-
-        public EmailNotifications Email { get; set; } = new EmailNotifications();
+        public EmailNotifications Email { get; init; } = new EmailNotifications();
     }
 }

src/ServiceControl.UnitTests/MessageFailures/EditFailedMessagesControllerAuditTests.cs

@@ -48,15 +48,13 @@ sealed class FakeEditFailedMessagesManager : IEditFailedMessagesManager
     {
         public string? CurrentEditingRequestId { get; set; }
 
-        public void Dispose()
-        {
-        }
-
         public Task SaveChanges() => Task.CompletedTask;
         public Task<FailedMessage?> GetFailedMessage(string failedMessageId) => Task.FromResult<FailedMessage?>(null);
         public Task<string?> GetCurrentEditingRequestId(string failedMessageId) => Task.FromResult(CurrentEditingRequestId);
         public Task SetCurrentEditingRequestId(string editingMessageId) => Task.CompletedTask;
         public Task SetFailedMessageAsResolved() => Task.CompletedTask;
+
+        public ValueTask DisposeAsync() => ValueTask.CompletedTask;
     }
 
     sealed class StubErrorMessageDataStore : IFailedMessageQueryDataStore, IEditFailedMessagesDataStore