RavenDB-21956 Address code review comments

djordjedjukic · May 20, 2024 · 5e4f43c · 5e4f43c
1 parent 517a5b4
commit 5e4f43c
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 16 deletions.
diff --git a/src/Raven.Client/Documents/Operations/ETL/Queue/AzureQueueStorageConnectionSettings.cs b/src/Raven.Client/Documents/Operations/ETL/Queue/AzureQueueStorageConnectionSettings.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Linq;
+using Raven.Client.Documents.Operations.ETL.SQL;
 using Sparrow.Json.Parsing;
 
 namespace Raven.Client.Documents.Operations.ETL.Queue;
@@ -61,31 +62,27 @@ public string GetStorageUrl()
 
     private string GetUrlFromConnectionString(string connectionString)
     {
-        var parts = connectionString.Split(';')
-            .Select(p => p.Split('='))
-            .ToDictionary(p => p[0], p => p.Length > 1 ? p[1] : "");
-
-        parts.TryGetValue("DefaultEndpointsProtocol", out string protocol);
+        var protocol = SqlConnectionStringParser.GetConnectionStringValue(connectionString, ["DefaultEndpointsProtocol"]);
         if (string.IsNullOrWhiteSpace(protocol))
         {
-            HandleConnectionStringError("Protocol not found in the connection string");
+            ThrowConnectionStringError("Protocol not found in the connection string");
         }
 
         if (protocol.Equals("http"))
         {
-            parts.TryGetValue("QueueEndpoint", out string queueEndpoint);
+            var queueEndpoint = SqlConnectionStringParser.GetConnectionStringValue(connectionString, ["QueueEndpoint"]);
             if (string.IsNullOrWhiteSpace(queueEndpoint))
             {
-                HandleConnectionStringError("Queue endpoint not found in the connection string");
+                ThrowConnectionStringError("Queue endpoint not found in the connection string");
             }
 
             return queueEndpoint;
         }
 
-        parts.TryGetValue("AccountName", out string accountName);
+        var accountName = SqlConnectionStringParser.GetConnectionStringValue(connectionString, ["AccountName"]);
         if (string.IsNullOrWhiteSpace(accountName))
         {
-            HandleConnectionStringError("Storage account name not found in the connection string");
+            ThrowConnectionStringError("Storage account name not found in the connection string");
         }
 
         return $"https://{accountName}.queue.core.windows.net/";
@@ -107,7 +104,7 @@ private string GetStorageAccountName()
         return storageAccountName;
     }
 
-    private void HandleConnectionStringError(string message)
+    private void ThrowConnectionStringError(string message)
     {
         throw new ArgumentException(message, nameof(ConnectionString));
     }
@@ -144,10 +141,10 @@ public sealed class EntraId
 
     public bool IsValid()
     {
-        return !string.IsNullOrWhiteSpace(StorageAccountName) &&
-               !string.IsNullOrWhiteSpace(TenantId) &&
-               !string.IsNullOrWhiteSpace(ClientId) &&
-               !string.IsNullOrWhiteSpace(ClientSecret);
+        return string.IsNullOrWhiteSpace(StorageAccountName) == false &&
+               string.IsNullOrWhiteSpace(TenantId) == false &&
+               string.IsNullOrWhiteSpace(ClientId) == false &&
+               string.IsNullOrWhiteSpace(ClientSecret) == false;
     }
 }
 
@@ -158,6 +155,6 @@ public sealed class Passwordless
 
     public bool IsValid()
     {
-        return !string.IsNullOrWhiteSpace(StorageAccountName);
+        return string.IsNullOrWhiteSpace(StorageAccountName) == false;
     }
 }
diff --git a/src/Raven.Server/Documents/ETL/Providers/Queue/AzureQueueStorage/AzureQueueStorageEtl.cs b/src/Raven.Server/Documents/ETL/Providers/Queue/AzureQueueStorage/AzureQueueStorageEtl.cs
@@ -63,6 +63,7 @@ protected override int PublishMessages(List<QueueWithItems<AzureQueueStorageItem
 
             foreach (AzureQueueStorageItem queueItem in queue.Items)
             {
+                CancellationToken.ThrowIfCancellationRequested();
                 string base64CloudEvent = CreateBase64CloudEvent(queueItem);
 
                 TimeSpan? timeToLive = Database.Configuration.Etl.AzureQueueStorageTimeToLive.AsTimeSpan;

diff --git a/test/SlowTests/Server/Documents/ETL/Queue/AzureQueueStorageEtlTests.cs b/test/SlowTests/Server/Documents/ETL/Queue/AzureQueueStorageEtlTests.cs
@@ -420,6 +420,52 @@ record = await dstStore.Maintenance.Server.SendAsync(new GetDatabaseRecordOperat
             Assert.Empty(record.QueueEtls);
         }
     }
+
+    [RavenFact(RavenTestCategory.Etl, AzureQueueStorageRequired = true)]
+    public void ProperUrlFromHttpConnectionString()
+    {
+        var config = new QueueEtlConfiguration
+        {
+            Name = "test",
+            ConnectionStringName = "test",
+            BrokerType = QueueBrokerType.AzureQueueStorage,
+            Transforms = { new Transformation { Name = "test", Collections = { "Orders" }, Script = @"" } }
+        };
+
+        config.Initialize(new QueueConnectionString
+        {
+            Name = "Foo",
+            BrokerType = QueueBrokerType.AzureQueueStorage,
+            AzureQueueStorageConnectionSettings =
+                new AzureQueueStorageConnectionSettings { ConnectionString = "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;QueueEndpoint=http://127.0.0.1:10001/devstoreaccount1;" }
+        });
+
+        var storageUrl = config.Connection.AzureQueueStorageConnectionSettings.GetStorageUrl();
+        Assert.Equal(storageUrl, "http://127.0.0.1:10001/devstoreaccount1");
+    }
+
+    [RavenFact(RavenTestCategory.Etl, AzureQueueStorageRequired = true)]
+    public void ProperUrlFromHttpsConnectionString()
+    {
+        var config = new QueueEtlConfiguration
+        {
+            Name = "test",
+            ConnectionStringName = "test",
+            BrokerType = QueueBrokerType.AzureQueueStorage,
+            Transforms = { new Transformation { Name = "test", Collections = { "Orders" }, Script = @"" } }
+        };
+
+        config.Initialize(new QueueConnectionString
+        {
+            Name = "Foo",
+            BrokerType = QueueBrokerType.AzureQueueStorage,
+            AzureQueueStorageConnectionSettings =
+                new AzureQueueStorageConnectionSettings { ConnectionString = "DefaultEndpointsProtocol=https;AccountName=myexamplestorage;AccountKey=myaccountkey;EndpointSuffix=core.windows.net" }
+        });
+
+        var storageUrl = config.Connection.AzureQueueStorageConnectionSettings.GetStorageUrl();
+        Assert.Equal(storageUrl, "https://myexamplestorage.queue.core.windows.net/");
+    }
 
     private class Order
     {