Address Error Response TODOs (#3981)

* Attempt to address error response TODOs in the ResolveMessagesController

* Make group id required

* Use model validation in PendingRetryMessagesController

* Make queueAddress required

* string.Empty should not be allowed

* Approval

* Group ID route constraints

* Cleanup ID generator

* FailedMessageId is mostly a string so use that instead of guid

* Validation ArchiveBatch

* MessageId constraint

* Datastore cleanup

* Conversation id constraint

* Fix regression

* Cleanup

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

@@ -24,7 +24,7 @@ public async Task Should_send_custom_check_status_change_emails()
         {
             var emailDropPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
             Directory.CreateDirectory(emailDropPath);
-            string[] emails = Array.Empty<string>();
+            string[] emails = [];
 
             SetSettings = settings =>
             {
@@ -48,7 +48,7 @@ public async Task Should_send_custom_check_status_change_emails()
 
             CollectionAssert.IsNotEmpty(emails);
 
-            var emailText = File.ReadAllLines(emails[0]);
+            var emailText = await File.ReadAllLinesAsync(emails[0]);
 
             Assert.AreEqual("X-Sender: YouServiceControl@particular.net", emailText[0]);
             Assert.AreEqual("X-Receiver: WhoeverMightBeConcerned@particular.net", emailText[1]);
@@ -57,18 +57,11 @@ public async Task Should_send_custom_check_status_change_emails()
             Assert.AreEqual("Subject: [Particular.ServiceControl] health check failed", emailText[6]);
         }
 
-        class SetupNotificationSettings : IHostedService
+        class SetupNotificationSettings(IErrorMessageDataStore errorMessageDataStore) : IHostedService
         {
-            readonly IErrorMessageDataStore errorMessageDataStore;
-
-            public SetupNotificationSettings(IErrorMessageDataStore errorMessageDataStore)
-            {
-                this.errorMessageDataStore = errorMessageDataStore;
-            }
-
             public async Task StartAsync(CancellationToken cancellationToken)
             {
-                var notificationsManager = await errorMessageDataStore.CreateNotificationsManager();
+                using var notificationsManager = await errorMessageDataStore.CreateNotificationsManager();
 
                 var settings = await notificationsManager.LoadSettings();
                 settings.Email = new EmailNotifications
@@ -91,22 +84,12 @@ public class MyContext : ScenarioContext
 
         public class EndpointWithFailingCustomCheck : EndpointConfigurationBuilder
         {
-            public EndpointWithFailingCustomCheck()
-            {
-                EndpointSetup<DefaultServer>(c => { c.ReportCustomChecksTo(Settings.DEFAULT_SERVICE_NAME, TimeSpan.FromSeconds(1)); });
-            }
+            public EndpointWithFailingCustomCheck() => EndpointSetup<DefaultServer>(c => { c.ReportCustomChecksTo(Settings.DEFAULT_SERVICE_NAME, TimeSpan.FromSeconds(1)); });
 
-            class FailingCustomCheck : CustomCheck
+            class FailingCustomCheck() : CustomCheck("MyCustomCheckId", "MyCategory")
             {
-                public FailingCustomCheck()
-                    : base("MyCustomCheckId", "MyCategory")
-                {
-                }
-
                 public override Task<CheckResult> PerformCheck(CancellationToken cancellationToken = default)
-                {
-                    return Task.FromResult(CheckResult.Failed("Some reason"));
-                }
+                    => Task.FromResult(CheckResult.Failed("Some reason"));
             }
         }
     }

src/ServiceControl.Persistence.RavenDB/DocumentIdGenerators/FailedMessageIdGenerator.cs

@@ -2,10 +2,7 @@
 {
     public const string CollectionName = "FailedMessages";
 
-    public static string MakeDocumentId(string messageUniqueId)
-    {
-        return $"{CollectionName}/{messageUniqueId}";
-    }
+    public static string MakeDocumentId(string messageUniqueId) => $"{CollectionName}/{messageUniqueId}";
 
     public static string GetMessageIdFromDocumentId(string failedMessageDocumentId) => failedMessageDocumentId.Substring(CollectionName.Length + 1);
 }

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

@@ -5,34 +5,29 @@
     using Notifications;
     using Raven.Client.Documents.Session;
 
-    class NotificationsManager : AbstractSessionManager, INotificationsManager
+    class NotificationsManager(IAsyncDocumentSession session) : AbstractSessionManager(session), INotificationsManager
     {
         static readonly TimeSpan CacheTimeoutDefault = TimeSpan.FromMinutes(5); // Raven requires this to be at least 1 second
 
-        public NotificationsManager(IAsyncDocumentSession session) : base(session)
-        {
-        }
-
         public async Task<NotificationsSettings> LoadSettings(TimeSpan? cacheTimeout = null)
         {
+            using var aggressivelyCacheFor = await Session.Advanced.DocumentStore.AggressivelyCacheForAsync(cacheTimeout ?? CacheTimeoutDefault);
+            var settings = await Session
+                .LoadAsync<NotificationsSettings>(NotificationsSettings.SingleDocumentId);
 
-            using (Session.Advanced.DocumentStore.AggressivelyCacheFor(cacheTimeout ?? CacheTimeoutDefault))
+            if (settings != null)
             {
-                var settings = await Session
-                    .LoadAsync<NotificationsSettings>(NotificationsSettings.SingleDocumentId);
+                return settings;
+            }
 
-                if (settings == null)
-                {
-                    settings = new NotificationsSettings
-                    {
-                        Id = NotificationsSettings.SingleDocumentId
-                    };
+            settings = new NotificationsSettings
+            {
+                Id = NotificationsSettings.SingleDocumentId
+            };
 
-                    await Session.StoreAsync(settings);
-                }
+            await Session.StoreAsync(settings);
 
-                return settings;
-            }
+            return settings;
         }
     }
 }