[DurableTask.ServiceBus] Fix blob container listing and purge for hub names with dashes and culture-dependent container names

src/DurableTask.ServiceBus/ServiceBusOrchestrationService.cs

@@ -431,9 +431,9 @@ internal async Task<Dictionary<string, int>> GetHubQueueMaxDeliveryCountsAsync()
             var queues =
                 (await managementClient.GetQueuesAsync()).Where(x => x.Path.StartsWith(this.hubName)).ToList();
 
-            result.Add("TaskOrchestration", queues.Single(q => string.Equals(q.Path, this.orchestratorEntityName))?.MaxDeliveryCount ?? -1);
-            result.Add("TaskActivity", queues.Single(q => string.Equals(q.Path, this.workerEntityName))?.MaxDeliveryCount ?? -1);
-            result.Add("Tracking", queues.Single(q => string.Equals(q.Path, this.trackingEntityName))?.MaxDeliveryCount ?? -1);
+            result.Add("TaskOrchestration", queues.SingleOrDefault(q => string.Equals(q.Path, this.orchestratorEntityName))?.MaxDeliveryCount ?? -1);
+            result.Add("TaskActivity", queues.SingleOrDefault(q => string.Equals(q.Path, this.workerEntityName))?.MaxDeliveryCount ?? -1);
+            result.Add("Tracking", queues.SingleOrDefault(q => string.Equals(q.Path, this.trackingEntityName))?.MaxDeliveryCount ?? -1);
 
             return result;
         }

src/DurableTask.ServiceBus/Tracking/BlobStorageClient.cs

@@ -59,7 +59,7 @@ public BlobStorageClient(string hubName, string connectionString)
 
             // make the hub name lower case since it will be used as part of the prefix of the container name,
             // which only allows lower case letters
-            this.containerNamePrefix = BlobStorageClientHelper.BuildContainerNamePrefix(hubName.ToLower());
+            this.containerNamePrefix = BlobStorageClientHelper.BuildContainerNamePrefix(hubName);
         }
 
         /// <summary>
@@ -89,7 +89,7 @@ public BlobStorageClient(string hubName, Uri endpoint, TokenCredential credentia
 
             // make the hub name lower case since it will be used as part of the prefix of the container name,
             // which only allows lower case letters
-            this.containerNamePrefix = BlobStorageClientHelper.BuildContainerNamePrefix(hubName.ToLower());
+            this.containerNamePrefix = BlobStorageClientHelper.BuildContainerNamePrefix(hubName);
         }
 
         /// <summary>
@@ -130,19 +130,21 @@ private async Task<BlobClient> GetCloudBlobClientAsync(string containerNameSuffi
         }
 
         /// <summary>
-        /// List all containers of the blob storage, whose prefix is containerNamePrefix, i.e., {hubName}-dtfx.
+        /// List all containers of this hub, i.e. containers named {hubName}-dtfx-{type}-{yyyyMMdd}.
+        /// Containers of other hubs whose names share this hub's prefix (e.g. hub "alpha-dtfx-backup" for hub "alpha")
+        /// are not returned.
         /// </summary>
         /// <returns>A list of Azure blob containers</returns>
         public async Task<IEnumerable<BlobContainerItem>> ListContainersAsync()
         {
             List<BlobContainerItem> results = new List<BlobContainerItem>();
             // Use the service client's prefix parameter to only enumerate containers that start with the
-            // configured containerNamePrefix. Also apply a defensive in-memory filter in case of any
-            // unexpected differences in casing or service behavior.
+            // configured containerNamePrefix, then keep only the containers that belong to this hub, since
+            // hub names may contain dashes and another hub's containers can share the same prefix.
             var response = this.blobServiceClient.GetBlobContainersAsync(prefix: this.containerNamePrefix);
             await foreach (var container in response)
             {
-                if (container.Name != null && container.Name.StartsWith(this.containerNamePrefix, StringComparison.OrdinalIgnoreCase))
+                if (container.Name != null && BlobStorageClientHelper.IsContainerOwnedByHub(container.Name, this.containerNamePrefix))
                 {
                     results.Add(container);
                 }
@@ -152,7 +154,7 @@ public async Task<IEnumerable<BlobContainerItem>> ListContainersAsync()
 
 
         /// <summary>
-        /// Delete all containers that are older than the input threshold date.
+        /// Delete all containers of this hub that are older than the input threshold date.
         /// </summary>
         /// <param name="thresholdDateTimeUtc">The specified date threshold</param>
         /// <returns></returns>
@@ -164,7 +166,7 @@ public async Task DeleteExpiredContainersAsync(DateTime thresholdDateTimeUtc)
         }
 
         /// <summary>
-        /// Delete blob containers with the containerNamePrefix as prefix.
+        /// Delete all containers of this hub.
         /// </summary>
         /// <returns></returns>
         public async Task DeleteBlobStoreContainersAsync()

src/DurableTask.ServiceBus/Tracking/BlobStorageClientHelper.cs

@@ -27,6 +27,8 @@ public class BlobStorageClientHelper
     {
         static readonly string DateFormat = "yyyyMMdd";
         static readonly char ContainerNameDelimiter = '-';
+        static readonly string ContainerNameMarker = "dtfx";
+        static readonly Regex ContainerNameSuffixRegex = new Regex(@"^[a-z0-9-]+\z", RegexOptions.Compiled | RegexOptions.CultureInvariant);
 
         /// <summary>
         /// the blob storage access key is in the format of {DateTime}|{blobName}
@@ -60,7 +62,7 @@ public static string BuildMessageBlobKey(string instanceId, string executionId,
 
         static string BuildContainerNameSuffix(string containerType, DateTime blobCreationTime)
         {
-            return $"{containerType.ToLower()}{ContainerNameDelimiter}{GetDateStringForContainerName(blobCreationTime)}";
+            return $"{containerType.ToLowerInvariant()}{ContainerNameDelimiter}{GetDateStringForContainerName(blobCreationTime)}";
         }
 
         /// <summary>
@@ -72,7 +74,7 @@ static string BuildContainerNameSuffix(string containerType, DateTime blobCreati
         /// <returns>The container name prefix</returns>
         public static string BuildContainerNamePrefix(string hubName)
         {
-            return $"{hubName.ToLower()}{ContainerNameDelimiter}dtfx";
+            return $"{hubName.ToLowerInvariant()}{ContainerNameDelimiter}{ContainerNameMarker}";
         }
 
         /// <summary>
@@ -91,8 +93,8 @@ public static string BuildSessionBlobKey(string sessionId)
         static string GetDateStringForContainerName(DateTime messageFireTime)
         {
             return messageFireTime.IsSet() ?
-                messageFireTime.ToString(DateFormat) :
-                DateTime.UtcNow.ToString(DateFormat);
+                messageFireTime.ToString(DateFormat, CultureInfo.InvariantCulture) :
+                DateTime.UtcNow.ToString(DateFormat, CultureInfo.InvariantCulture);
         }
 
         /// <summary>
@@ -129,8 +131,7 @@ public static void ParseKey(string key, out string containerNameSuffix, out stri
         /// <returns>True if the container name suffix is valid.</returns>
         static bool IsValidContainerNameSuffix(string containerNameSuffix)
         {
-            var regex = new Regex(@"^[a-z0-9\\-]+$");
-            return regex.Match(containerNameSuffix).Success;
+            return ContainerNameSuffixRegex.IsMatch(containerNameSuffix);
         }
 
         /// <summary>
@@ -141,26 +142,21 @@ static bool IsValidContainerNameSuffix(string containerNameSuffix)
         /// <returns></returns>
         public static bool IsContainerExpired(string containerName, DateTime thresholdDateTimeUtc)
         {
+            // Container names are {hubName}-dtfx-{type}-{yyyyMMdd}; the hub name may itself contain dashes,
+            // so the fixed segments are read from the end.
             string[] segments = containerName.Split(ContainerNameDelimiter);
-            if (segments.Length != 4)
+            if (segments.Length < 4 || !string.Equals(segments[segments.Length - 3], ContainerNameMarker, StringComparison.Ordinal))
             {
                 TraceHelper.Trace(
                     TraceEventType.Warning,
                     "BlobStorageClientHelper-IsContainerExpired-ContainerIgnored",
-                    $"Container name {containerName} does not contain required 4 segments. Container {containerName} is ignored.");
+                    $"Container name {containerName} is not in the format {{hubName}}-{ContainerNameMarker}-{{type}}-{{date}}. Container {containerName} is ignored.");
 
                 return false;
             }
 
             string dateString = segments[segments.Length - 1];
-            bool parseSucceeded = DateTime.TryParseExact(
-                dateString,
-                DateFormat,
-                System.Globalization.CultureInfo.InvariantCulture,
-                DateTimeStyles.None,
-                out DateTime containerDateTime);
-
-            if (!parseSucceeded)
+            if (!TryParseContainerDate(dateString, out DateTime containerDateTime))
             {
                 TraceHelper.Trace(
                     TraceEventType.Warning,
@@ -173,6 +169,39 @@ public static bool IsContainerExpired(string containerName, DateTime thresholdDa
             return containerDateTime < thresholdDateTimeUtc;
         }
 
+        /// <summary>
+        /// Check if the container belongs to the hub with the given container name prefix.
+        /// Container names are {hubName}-dtfx-{type}-{yyyyMMdd}; because hub names may contain dashes, a different hub
+        /// (e.g. "alpha-dtfx-backup") can share the prefix "alpha-dtfx". The container belongs to the hub only if the
+        /// whole name without its trailing "-{type}-{yyyyMMdd}" segments equals the hub's container name prefix,
+        /// the type is not empty, and the last segment is a valid yyyyMMdd date.
+        /// </summary>
+        /// <param name="containerName">The container name</param>
+        /// <param name="containerNamePrefix">The hub's container name prefix, i.e. {hubName}-dtfx</param>
+        /// <returns>True if the container belongs to the hub.</returns>
+        internal static bool IsContainerOwnedByHub(string containerName, string containerNamePrefix)
+        {
+            int dateDelimiterIndex = containerName.LastIndexOf(ContainerNameDelimiter);
+            if (dateDelimiterIndex <= 0)
+            {
+                return false;
+            }
+
+            int typeDelimiterIndex = containerName.LastIndexOf(ContainerNameDelimiter, dateDelimiterIndex - 1);
+            if (typeDelimiterIndex <= 0 || typeDelimiterIndex == dateDelimiterIndex - 1)
+            {
+                return false;
+            }
+
+            return string.Equals(containerName.Substring(0, typeDelimiterIndex), containerNamePrefix, StringComparison.OrdinalIgnoreCase) &&
+                TryParseContainerDate(containerName.Substring(dateDelimiterIndex + 1), out _);
+        }
+
+        static bool TryParseContainerDate(string dateString, out DateTime date)
+        {
+            return DateTime.TryParseExact(dateString, DateFormat, CultureInfo.InvariantCulture, DateTimeStyles.None, out date);
+        }
+
         /// <summary>
         /// Build a container name using prefix and suffix.
         /// </summary>

test/DurableTask.ServiceBus.Tests/BlobStorageClientHelperTest.cs

@@ -15,6 +15,7 @@ namespace DurableTask.ServiceBus.Tests
 {
     using System;
     using System.Diagnostics.CodeAnalysis;
+    using System.Globalization;
     using System.Text.RegularExpressions;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
     using DurableTask.ServiceBus.Tracking;
@@ -42,6 +43,84 @@ public void IsContainerExpiredTest()
             Assert.IsFalse(BlobStorageClientHelper.IsContainerExpired("hubname-dtfx-message-20146789", DateTime.UtcNow));
         }
 
+        [TestMethod]
+        [SuppressMessage("ReSharper", "StringLiteralTypo")]
+        public void IsContainerExpiredHubNameWithDashesTest()
+        {
+            var dateTime = new DateTime(2015, 05, 17);
+            Assert.IsTrue(BlobStorageClientHelper.IsContainerExpired("my-hub-dtfx-message-20150516", dateTime));
+            Assert.IsTrue(BlobStorageClientHelper.IsContainerExpired("my-test-hub-dtfx-session-20140518", dateTime));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerExpired("my-hub-dtfx-message-20150518", dateTime));
+
+            // containers not created by the framework are ignored
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerExpired("my-hub-other-message-20100101", DateTime.UtcNow));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerExpired("dtfx-message-20100101", DateTime.UtcNow));
+        }
+
+        [TestMethod]
+        [SuppressMessage("ReSharper", "StringLiteralTypo")]
+        public void ContainerNamesAreCultureInvariantTest()
+        {
+            CultureInfo originalCulture = CultureInfo.CurrentCulture;
+            try
+            {
+                // th-TH uses the Buddhist calendar (year 2558 for 2015)
+                CultureInfo.CurrentCulture = new CultureInfo("th-TH");
+                string key = BlobStorageClientHelper.BuildMessageBlobKey("aa", "bb", new DateTime(2015, 05, 17));
+                StringAssert.StartsWith(key, "message-20150517|");
+
+                // tr-TR lower-cases 'I' to a dotless 'ı', which is invalid in a container name
+                CultureInfo.CurrentCulture = new CultureInfo("tr-TR");
+                Assert.AreEqual("myhubid-dtfx", BlobStorageClientHelper.BuildContainerNamePrefix("MYHUBID"));
+            }
+            finally
+            {
+                CultureInfo.CurrentCulture = originalCulture;
+            }
+        }
+
+        [TestMethod]
+        public void ParseKeyRejectsInvalidSuffixCharactersTest()
+        {
+            Assert.ThrowsException<ArgumentException>(
+                () => BlobStorageClientHelper.ParseKey(@"message\-20100319|aa/bb/cc", out _, out _));
+            Assert.ThrowsException<ArgumentException>(
+                () => BlobStorageClientHelper.ParseKey("message-20100319\n|aa/bb/cc", out _, out _));
+        }
+
+        [TestMethod]
+        [SuppressMessage("ReSharper", "StringLiteralTypo")]
+        public void IsContainerOwnedByHubTest()
+        {
+            Assert.IsTrue(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-session-20150516", "alpha-dtfx"));
+            Assert.IsTrue(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-backup-dtfx-session-20150516", "alpha-dtfx-backup-dtfx"));
+            Assert.IsTrue(BlobStorageClientHelper.IsContainerOwnedByHub("my-hub-dtfx-message-20150516", "my-hub-dtfx"));
+
+            // another hub whose container names share this hub's prefix
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-backup-dtfx-session-20150516", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-session-20150516", "alpha-dtfx-backup-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-extra-session-20150516", "alpha-dtfx"));
+
+            // malformed names
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-20150516", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("-session-20150516", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("invalidContainerName", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx--20150516", "alpha-dtfx"));
+
+            // non-date suffixes
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-session-backup", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-session-2015051", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-session-201505160", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-session-", "alpha-dtfx"));
+
+            // invalid calendar dates
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-session-20150230", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-session-20151301", "alpha-dtfx"));
+            Assert.IsFalse(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-message-20150001", "alpha-dtfx"));
+            Assert.IsTrue(BlobStorageClientHelper.IsContainerOwnedByHub("alpha-dtfx-message-20160229", "alpha-dtfx"));
+        }
+
         [TestMethod]
         public void BuildMessageBlobKeyTest()
         {