From c146f72893514b89eba70705ea0bb857432f729b Mon Sep 17 00:00:00 2001 From: Mauro Servienti Date: Tue, 1 Apr 2025 09:47:45 +0200 Subject: [PATCH] Dirty memory check may fail connecting to RavenDB when using external mode (#4917) * Favor connection string over server URL when trying to connect to RavenDB to retrieve dirty memory details * Update comment to reflect reality * Update audit comment --------- Co-authored-by: Brandon Ording --- .../MemoryInformationRetriever.cs | 12 ++++++------ .../MemoryInformationRetriever.cs | 11 ++++++----- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/ServiceControl.Audit.Persistence.RavenDB/MemoryInformationRetriever.cs b/src/ServiceControl.Audit.Persistence.RavenDB/MemoryInformationRetriever.cs index 452546521d..5aff8c0fb7 100644 --- a/src/ServiceControl.Audit.Persistence.RavenDB/MemoryInformationRetriever.cs +++ b/src/ServiceControl.Audit.Persistence.RavenDB/MemoryInformationRetriever.cs @@ -8,12 +8,12 @@ namespace ServiceControl.Audit.Persistence.RavenDB; class MemoryInformationRetriever(DatabaseConfiguration databaseConfiguration) { - // What does a connection string look like? Is it only a URI or could it contain other stuff? - // The ?? operator is needed because ServerUrl is populated when running embedded and connection - // string when running in external mode. However, the tricky part is that when tests are run they - // behave like if it was external mode. If the connection string contain always only the server - // URL, this code is safe, otherwise it need to be adjusted to extract the server URL. - readonly HttpClient client = new() { BaseAddress = new Uri(databaseConfiguration.ServerConfiguration.ServerUrl ?? databaseConfiguration.ServerConfiguration.ConnectionString) }; + // Connection string is composed of the server URL. The ?? operator is needed because ServerUrl + // is populated when running embedded and connection string when running in external mode. + // However, the tricky part is that when tests are run they behave like if it was external mode. + // Only one of ConnectionString and ServerUrl will be non-null, so we'll check ConnectionString first + // to be consistent with the error instance implementation, where ServerUrl always has a value. + readonly HttpClient client = new() { BaseAddress = new Uri(databaseConfiguration.ServerConfiguration.ConnectionString ?? databaseConfiguration.ServerConfiguration.ServerUrl) }; record ResponseDto { diff --git a/src/ServiceControl.Persistence.RavenDB/MemoryInformationRetriever.cs b/src/ServiceControl.Persistence.RavenDB/MemoryInformationRetriever.cs index b44ba50bfc..f7db4f2557 100644 --- a/src/ServiceControl.Persistence.RavenDB/MemoryInformationRetriever.cs +++ b/src/ServiceControl.Persistence.RavenDB/MemoryInformationRetriever.cs @@ -8,11 +8,12 @@ namespace ServiceControl.Persistence.RavenDB; class MemoryInformationRetriever(RavenPersisterSettings persisterSettings) { - // What does a connection string look like? Is it only a URI or could it contain other stuff? - // The primary instance has only the concept of a connection string (vs the Audit instance having - // both a ServiceUrl and a ConnectionString). If the connection string contain always only the - // server URL, this code is safe, otherwise it need to be adjusted to extract the server URL. - readonly HttpClient client = new() { BaseAddress = new Uri(persisterSettings.ServerUrl ?? persisterSettings.ConnectionString) }; + // Connection string is composed of the server URL. The ?? operator is needed because ServerUrl + // is populated when running embedded and connection string when running in external mode. + // However, the tricky part is that when tests are run they behave like if it was external mode. + // ServerUrl is always populated by the persister settings, hence the code first checks for the + // presence of a connection string, and if null, falls back to ServerUrl + readonly HttpClient client = new() { BaseAddress = new Uri(persisterSettings.ConnectionString ?? persisterSettings.ServerUrl) }; record ResponseDto {