Backport fixes to v5 (#4057)

* Diagnostic improvements and bulk insert time config setting (#4049)

* Add exception logging to DomainEvents

* Add NServiceBus reference

* Add debug logging to EndpointInstanceMonitor

* Add configurable bulk insert timeout

---------

Co-authored-by: WilliamBZA <WilliamBZA@users.noreply.github.com>

* Remove audit queue validation checks (#4048)

* Fix small validation bugs (#4046)

* Allow propogation of propertychange events

* Prevent event bubbling

* Switch back to invalid file chars

* Add notify propogation tests

* Add file path sanitization tests

---------

Co-authored-by: Mike Minutillo <mike.minutillo@particular.net>

* Fix test for primary instance convention default

Original commit is here 84f61ec

* RavenDb License refresh

---------

Co-authored-by: Szymon Pobiega <szymon.pobiega@gmail.com>
Co-authored-by: WilliamBZA <WilliamBZA@users.noreply.github.com>
Co-authored-by: John Simons <john.simons@particular.net>

src/ServiceControl.Audit.Persistence.RavenDB/DatabaseConfiguration.cs

@@ -11,14 +11,16 @@ public class DatabaseConfiguration
             TimeSpan auditRetentionPeriod,
             int maxBodySizeToStore,
             int minimumStorageLeftRequiredForIngestion,
-            ServerConfiguration serverConfiguration)
+            ServerConfiguration serverConfiguration,
+            TimeSpan bulkInsertCommitTimeout)
         {
             Name = name;
             ExpirationProcessTimerInSeconds = expirationProcessTimerInSeconds;
             EnableFullTextSearch = enableFullTextSearch;
             AuditRetentionPeriod = auditRetentionPeriod;
             MaxBodySizeToStore = maxBodySizeToStore;
             ServerConfiguration = serverConfiguration;
+            BulkInsertCommitTimeout = bulkInsertCommitTimeout;
             MinimumStorageLeftRequiredForIngestion = minimumStorageLeftRequiredForIngestion;
         }
 
@@ -37,5 +39,7 @@ public class DatabaseConfiguration
         public int MaxBodySizeToStore { get; }
 
         public int MinimumStorageLeftRequiredForIngestion { get; internal set; } //Setting for ATT only
+
+        public TimeSpan BulkInsertCommitTimeout { get; }
     }
 }

src/ServiceControl.Audit.Persistence.RavenDB/RavenLicense.json

@@ -2,18 +2,18 @@
   "Id": "64c6a174-3f3a-4e7d-ac5d-b3eedd801460",
   "Name": "ParticularNservicebus (Israel)",
   "Keys": [
-      "NKhWkUrAnwvE+M6VmSxK7J6am",
-      "D2JME/+XVMQq9xh18W4lClw+a",
-      "FDWF3ZqgA89/tIgwUyc8BZ09f",
-      "wSsdi8WzL/w01TOB00HuGQE4r",
-      "jAkg7BKr7LWIRfbg02KLhiU4M",
-      "Y1FQx8IA4BOBK0XP+X+aaWE48",
-      "fLmwuqGsSc0kg0rzFdYs9AAyU",
-      "GKEkFCisMLQ4vMAcREjM0NRYX",
-      "GhufAh8AnwIgAJ8CIwCfAiQgn",
-      "wIlIJ8CJiCfAicgnwIoIJ8CKS",
-      "CfAiognwIrIJ8CLACfAi0AnwI",
-      "uIJ8CLwCfAjAggQM2LjBDMEQG",
-      "ODk8PT6fAiEgYoFY"
+    "mGE1ppvgfJjJZXnMUHFXQXJuL",
+    "sQ1trVf0DZlVNhFSjclHNQ36p",
+    "0a9ZWIaCiqu8Mh4CDVd4koc2X",
+    "2Y18vLYbTm5JH4J91IZJhq3bP",
+    "/bzD806qeBpRcDCLt82ON0+RO",
+    "eXiLDs/DTOBU9sPsNZEPzUX+C",
+    "uMA5nm6QaaB85GEpFuahhAAyU",
+    "GKEkFCisMLQ4vMAcREjM0NRYX",
+    "GhufAh8AnwIgAJ8CIwCfAiQgn",
+    "wIlIJ8CJiCfAicgnwIoIJ8CKS",
+    "CfAiognwIrIJ8CLACfAi0AnwI",
+    "uIJ8CLwCfAjAggQM2LjBDMEQG",
+    "ODk8PT6fAiEgYoRa"
   ]
 }

src/ServiceControl.Audit.Persistence.RavenDB/RavenPersistenceConfiguration.cs

@@ -14,6 +14,7 @@ public class RavenPersistenceConfiguration : IPersistenceConfiguration
         public const string LogPathKey = "LogPath";
         public const string RavenDbLogLevelKey = "RavenDBLogLevel";
         public const string MinimumStorageLeftRequiredForIngestionKey = "MinimumStorageLeftRequiredForIngestion";
+        public const string BulkInsertCommitTimeoutInSecondsKey = "BulkInsertCommitTimeoutInSeconds";
 
         public IEnumerable<string> ConfigurationKeys => new[]{
             DatabaseNameKey,
@@ -23,7 +24,8 @@ public class RavenPersistenceConfiguration : IPersistenceConfiguration
             ExpirationProcessTimerInSecondsKey,
             LogPathKey,
             RavenDbLogLevelKey,
-            MinimumStorageLeftRequiredForIngestionKey
+            MinimumStorageLeftRequiredForIngestionKey,
+            BulkInsertCommitTimeoutInSecondsKey
         };
 
         public string Name => "RavenDB";
@@ -98,14 +100,17 @@ internal static DatabaseConfiguration GetDatabaseConfiguration(PersistenceSettin
 
             var expirationProcessTimerInSeconds = GetExpirationProcessTimerInSeconds(settings);
 
+            var bulkInsertTimeout = TimeSpan.FromSeconds(GetBulkInsertCommitTimeout(settings));
+
             return new DatabaseConfiguration(
                 databaseName,
                 expirationProcessTimerInSeconds,
                 settings.EnableFullTextSearchOnBodies,
                 settings.AuditRetentionPeriod,
                 settings.MaxBodySizeToStore,
                 minimumStorageLeftRequiredForIngestion,
-                serverConfiguration);
+                serverConfiguration,
+                bulkInsertTimeout);
         }
 
         static int GetExpirationProcessTimerInSeconds(PersistenceSettings settings)
@@ -132,8 +137,33 @@ static int GetExpirationProcessTimerInSeconds(PersistenceSettings settings)
             return expirationProcessTimerInSeconds;
         }
 
+        static int GetBulkInsertCommitTimeout(PersistenceSettings settings)
+        {
+            var bulkInsertCommitTimeoutInSeconds = BulkInsertCommitTimeoutInSecondsDefault;
+
+            if (settings.PersisterSpecificSettings.TryGetValue(BulkInsertCommitTimeoutInSecondsKey, out var bulkInsertCommitTimeoutString))
+            {
+                bulkInsertCommitTimeoutInSeconds = int.Parse(bulkInsertCommitTimeoutString);
+            }
+
+            if (bulkInsertCommitTimeoutInSeconds < 0)
+            {
+                Logger.Error($"BulkInsertCommitTimeout cannot be negative. Defaulting to {BulkInsertCommitTimeoutInSecondsDefault}");
+                return BulkInsertCommitTimeoutInSecondsDefault;
+            }
+
+            if (bulkInsertCommitTimeoutInSeconds > TimeSpan.FromHours(1).TotalSeconds)
+            {
+                Logger.Error($"BulkInsertCommitTimeout cannot be larger than {TimeSpan.FromHours(1).TotalSeconds}. Defaulting to {BulkInsertCommitTimeoutInSecondsDefault}");
+                return BulkInsertCommitTimeoutInSecondsDefault;
+            }
+
+            return bulkInsertCommitTimeoutInSeconds;
+        }
+
         static readonly ILog Logger = LogManager.GetLogger(typeof(RavenPersistenceConfiguration));
 
         const int ExpirationProcessTimerInSecondsDefault = 600;
+        const int BulkInsertCommitTimeoutInSecondsDefault = 60;
     }
 }

src/ServiceControl.Audit.Persistence.RavenDB/UnitOfWork/RavenAuditUnitOfWorkFactory.cs

@@ -20,7 +20,7 @@ class RavenAuditIngestionUnitOfWorkFactory : IAuditIngestionUnitOfWorkFactory
 
         public IAuditIngestionUnitOfWork StartNew(int batchSize)
         {
-            var timedCancellationSource = new CancellationTokenSource(TimeSpan.FromMinutes(1));
+            var timedCancellationSource = new CancellationTokenSource(databaseConfiguration.BulkInsertCommitTimeout);
             var bulkInsert = documentStoreProvider.GetDocumentStore()
                 .BulkInsert(new BulkInsertOptions { SkipOverwriteIfUnchanged = true, }, timedCancellationSource.Token);
 

src/ServiceControl.Audit.Persistence.Tests.RavenDB/SharedEmbeddedServer.cs

@@ -27,7 +27,7 @@ public static async Task<EmbeddedDatabase> GetInstance(CancellationToken cancell
                 var logsMode = "Operations";
                 var serverUrl = $"http://localhost:{PortUtility.FindAvailablePort(33334)}";
 
-                embeddedDatabase = EmbeddedDatabase.Start(new DatabaseConfiguration("audit", 60, true, TimeSpan.FromMinutes(5), 120000, 5, new ServerConfiguration(dbPath, serverUrl, logPath, logsMode)));
+                embeddedDatabase = EmbeddedDatabase.Start(new DatabaseConfiguration("audit", 60, true, TimeSpan.FromMinutes(5), 120000, 5, new ServerConfiguration(dbPath, serverUrl, logPath, logsMode), TimeSpan.FromSeconds(60)));
 
                 //make sure that the database is up
                 while (true)

src/ServiceControl.Config.Tests/AddInstance/AddErrorInstance/PathNotifyPropertyChanges.cs

@@ -0,0 +1,129 @@
+namespace ServiceControl.Config.Tests.AddInstance.AddErrorInstance
+{
+    using NUnit.Framework;
+    using ServiceControl.Config.UI.InstanceAdd;
+
+    public class PathNotifyPropertyChangesTests
+    {
+        public ServiceControlAddViewModel Given_adding_error_instance()
+        {
+            var viewModel = new ServiceControlAddViewModel();
+
+            return viewModel;
+        }
+
+        [Test]
+        public void ChangesTo_ErrorSharedServiceControlEditorViewModel_DbPath_Notifies_ServiceControlAddViewModel_DbPath()
+        {
+            var viewModel = Given_adding_error_instance();
+
+            var propertyNotified = false;
+            viewModel.PropertyChanged += (s, e) =>
+            {
+                if (e.PropertyName == nameof(viewModel.ErrorDatabasePath))
+                {
+                    propertyNotified = true;
+                }
+            };
+
+            viewModel.ServiceControl.DatabasePath = "NewValue";
+
+            Assert.IsTrue(propertyNotified, "Changes to DatabasePath did not notify ServiceControlAddViewModel");
+        }
+
+        [Test]
+        public void ChangesTo_ErrorSharedServiceControlEditorViewModel_LogPath_Notifies_ServiceControlAddViewModel_LogPath()
+        {
+            var viewModel = Given_adding_error_instance();
+
+            var propertyNotified = false;
+            viewModel.PropertyChanged += (s, e) =>
+            {
+                if (e.PropertyName == nameof(viewModel.ErrorLogPath))
+                {
+                    propertyNotified = true;
+                }
+            };
+
+            viewModel.ServiceControl.LogPath = "NewValue";
+
+            Assert.IsTrue(propertyNotified, "Changes to LogPath did not notify ServiceControlAddViewModel");
+        }
+
+        [Test]
+        public void ChangesTo_ErrorSharedServiceControlEditorViewModel_DestinationPath_Notifies_ServiceControlAddViewModel_DestinationPath()
+        {
+            var viewModel = Given_adding_error_instance();
+
+            var propertyNotified = false;
+            viewModel.PropertyChanged += (s, e) =>
+            {
+                if (e.PropertyName == nameof(viewModel.ErrorDestinationPath))
+                {
+                    propertyNotified = true;
+                }
+            };
+
+            viewModel.ServiceControl.DestinationPath = "NewValue";
+
+            Assert.IsTrue(propertyNotified, "Changes to DestinationPath did not notify ServiceControlAddViewModel");
+        }
+
+        [Test]
+        public void ChangesTo_AuditSharedServiceControlEditorViewModel_DbPath_Notifies_ServiceControlAddViewModel_DbPath()
+        {
+            var viewModel = Given_adding_error_instance();
+
+            var propertyNotified = false;
+            viewModel.PropertyChanged += (s, e) =>
+            {
+                if (e.PropertyName == nameof(viewModel.AuditDatabasePath))
+                {
+                    propertyNotified = true;
+                }
+            };
+
+            viewModel.ServiceControlAudit.DatabasePath = "NewValue";
+
+            Assert.IsTrue(propertyNotified, "Changes to DatabasePath did not notify ServiceControlAddViewModel");
+        }
+
+        [Test]
+        public void ChangesTo_AuditSharedServiceControlEditorViewModel_LogPath_Notifies_ServiceControlAddViewModel_LogPath()
+        {
+            var viewModel = Given_adding_error_instance();
+
+            var propertyNotified = false;
+            viewModel.PropertyChanged += (s, e) =>
+            {
+                if (e.PropertyName == nameof(viewModel.AuditLogPath))
+                {
+                    propertyNotified = true;
+                }
+            };
+
+            viewModel.ServiceControlAudit.LogPath = "NewValue";
+
+            Assert.IsTrue(propertyNotified, "Changes to LogPath did not notify ServiceControlAddViewModel");
+        }
+
+        [Test]
+        public void ChangesTo_AuditSharedServiceControlEditorViewModel_DestinationPath_Notifies_ServiceControlAddViewModel_DestinationPath()
+        {
+            var viewModel = Given_adding_error_instance();
+
+            var propertyNotified = false;
+            viewModel.PropertyChanged += (s, e) =>
+            {
+                if (e.PropertyName == nameof(viewModel.AuditDestinationPath))
+                {
+                    propertyNotified = true;
+                }
+            };
+
+            viewModel.ServiceControlAudit.DestinationPath = "NewValue";
+
+            Assert.IsTrue(propertyNotified, "Changes to DestinationPath did not notify ServiceControlAddViewModel");
+        }
+    }
+}

src/ServiceControl.Config.Tests/FilePathExtensionsTests.cs

@@ -0,0 +1,25 @@
+namespace ServiceControl.Config.Tests
+{
+    using NUnit.Framework;
+    using ServiceControl.Config.Extensions;
+
+    class FilePathExtensionsTests
+    {
+        [TestCase(null)]
+        [TestCase(@"C:\")]
+        [TestCase(@"C:")]
+        [TestCase(@"C:\foo\bar")]
+        //[TestCase(@"/foo/bar", @"\foo\bar")] // NOTE: Returns \foobar
+        [TestCase(@"C:\foo\bar", @"C:\foo\bar")]
+        [TestCase(@"\\foo\bar", @"\foo\bar")]
+        [TestCase(@"C:\foo\bar\", @"C:\foo\bar\")]
+        [TestCase(@"C:\foo:bar", @"C:\foobar")]
+        [TestCase(@"C:\foo|bar", @"C:\foobar")]
+        public void TestSanitization(string original, string sanitized = null)
+        {
+            var converted = FilePathExtensions.SanitizeFilePath(original);
+
+            Assert.That(converted, Is.EqualTo(sanitized ?? original));
+        }
+    }
+}

src/ServiceControl.Config.Tests/ServiceControlAddScreenLoadedTests.cs

@@ -1,5 +1,6 @@
 namespace ServiceControl.Config.Tests
 {
+    using System;
     using System.ComponentModel;
     using NUnit.Framework;
     using ServiceControlInstaller.Engine.Configuration.ServiceControl;
@@ -8,6 +9,9 @@
 
     class AddErrorInstanceScreenLoadedTests
     {
+        static readonly string programDataPath = Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData);
+        static readonly string programX86Path = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86);
+
         [Test]
         public void Error_and_Audit_Instances_are_selected_for_install()
         {
@@ -148,11 +152,11 @@ public void Destination_path_is_null()
 
             var errorInfo = (INotifyDataErrorInfo)viewModel;
 
-            Assert.IsNull(viewModel.ErrorDestinationPath);
+            Assert.That(viewModel.ErrorDestinationPath, Is.EqualTo($@"{programX86Path}\Particular Software\Particular.ServiceControl"));
 
             Assert.IsEmpty(errorInfo.GetErrors(nameof(viewModel.ErrorDestinationPath)));
 
-            Assert.IsNull(viewModel.AuditDestinationPath);
+            Assert.That(viewModel.AuditDestinationPath, Is.EqualTo($@"{programX86Path}\Particular Software\Particular.ServiceControl.Audit"));
 
             Assert.IsEmpty(errorInfo.GetErrors(nameof(viewModel.AuditDestinationPath)));
         }
@@ -164,11 +168,11 @@ public void Log_path_is_null()
 
             var errorInfo = (INotifyDataErrorInfo)viewModel;
 
-            Assert.IsNull(viewModel.ErrorLogPath);
+            Assert.That(viewModel.ErrorLogPath, Is.EqualTo($@"{programDataPath}\Particular\ServiceControl\Particular.ServiceControl\Logs"));
 
             Assert.IsEmpty(errorInfo.GetErrors(nameof(viewModel.ErrorLogPath)));
 
-            Assert.IsNull(viewModel.AuditLogPath);
+            Assert.That(viewModel.AuditLogPath, Is.EqualTo($@"{programDataPath}\Particular\ServiceControl\Particular.ServiceControl.Audit\Logs"));
 
             Assert.IsEmpty(errorInfo.GetErrors(nameof(viewModel.AuditLogPath)));
         }
@@ -181,11 +185,11 @@ public void Database_path_is_null()
 
             var errorInfo = (INotifyDataErrorInfo)viewModel;
 
-            Assert.IsNull(viewModel.ErrorDatabasePath);
+            Assert.That(viewModel.ErrorDatabasePath, Is.EqualTo($@"{programDataPath}\Particular\ServiceControl\Particular.ServiceControl\DB"));
 
             Assert.IsEmpty(errorInfo.GetErrors(nameof(viewModel.ErrorDatabasePath)));
 
-            Assert.IsNull(viewModel.AuditDatabasePath);
+            Assert.That(viewModel.AuditDatabasePath, Is.EqualTo($@"{programDataPath}\Particular\ServiceControl\Particular.ServiceControl.Audit\DB"));
 
             Assert.IsEmpty(errorInfo.GetErrors(nameof(viewModel.AuditDatabasePath)));
         }

src/ServiceControl.Config.Tests/Validation/AddAuditInstanceValidationTests.cs

@@ -30,9 +30,9 @@ public void Convention_name_cannot_be_empty_when_instance_names_are_not_provided
 
             var notifyErrorInfo = GetNotifyErrorInfo(viewModel);
 
-            Assert.IsFalse(instanceNamesProvided);
+            Assert.That(instanceNamesProvided); // Provided because the convention auto-fills them on instantiation
 
-            Assert.IsNotEmpty(notifyErrorInfo.GetErrors(nameof(viewModel.ConventionName)));
+            Assert.IsEmpty(notifyErrorInfo.GetErrors(nameof(viewModel.ConventionName)));
         }
 
         [Test]

src/ServiceControl.Config.Tests/Validation/AddErrorInstanceValidationTests.cs

@@ -30,9 +30,9 @@ public void Convention_name_cannot_be_empty_when_instance_names_are_not_provided
 
             var notifyErrorInfo = GetNotifyErrorInfo(viewModel);
 
-            Assert.IsFalse(instanceNamesProvided);
+            Assert.That(instanceNamesProvided); // Provided because the convention default auto-fills them on instantiation
 
-            Assert.IsNotEmpty(notifyErrorInfo.GetErrors(nameof(viewModel.ConventionName)));
+            Assert.IsEmpty(notifyErrorInfo.GetErrors(nameof(viewModel.ConventionName)));
         }
 
         [Test]