Azure.Provisioning: Move name validation to Infrastructure and expose…

… for Aspire (#46437)

Azure.Provisioning: Move name validation to Infrastructure and expose for Aspire

sdk/provisioning/Azure.Provisioning/api/Azure.Provisioning.netstandard2.0.cs

@@ -98,9 +98,12 @@ public virtual void Add(Azure.Provisioning.Primitives.Provisionable resource) {
         protected internal override System.Collections.Generic.IEnumerable<Azure.Provisioning.Expressions.Statement> Compile() { throw null; }
         protected internal System.Collections.Generic.IDictionary<string, System.Collections.Generic.IEnumerable<Azure.Provisioning.Expressions.Statement>> CompileModules(Azure.Provisioning.ProvisioningContext? context = null) { throw null; }
         public override System.Collections.Generic.IEnumerable<Azure.Provisioning.Primitives.Provisionable> GetResources() { throw null; }
+        public static bool IsValidIdentifierName(string? identifierName) { throw null; }
+        public static string NormalizeIdentifierName(string? identifierName) { throw null; }
         public virtual void Remove(Azure.Provisioning.Primitives.Provisionable resource) { }
         protected internal override void Resolve(Azure.Provisioning.ProvisioningContext? context = null) { }
         protected internal override void Validate(Azure.Provisioning.ProvisioningContext? context = null) { }
+        public static void ValidateIdentifierName(string? identifierName, string? paramName = null) { }
     }
     public partial class ProvisioningContext
     {

sdk/provisioning/Azure.Provisioning/src/Infrastructure.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Text;
 using Azure.Provisioning.Expressions;
 using Azure.Provisioning.Primitives;
 
@@ -93,6 +94,109 @@ public virtual void Remove(Provisionable resource)
         }
     }
 
+    private static bool IsAsciiLetterOrDigit(char ch) =>
+        'a' <= ch && ch <= 'z' ||
+        'A' <= ch && ch <= 'Z' ||
+        '0' <= ch && ch <= '9';
+
+    /// <summary>
+    /// Checks whether an name is a valid bicep identifier name comprised of
+    /// letters, digits, and underscores.
+    /// </summary>
+    /// <param name="identifierName">The proposed identifier name.</param>
+    /// <returns>Whether the name is a valid bicep identifier name.</returns>
+    public static bool IsValidIdentifierName(string? identifierName)
+    {
+        if (string.IsNullOrEmpty(identifierName)) { return false; }
+        if (char.IsDigit(identifierName![0])) { return false; }
+        foreach (char ch in identifierName)
+        {
+            if (!IsAsciiLetterOrDigit(ch) && ch != '_')
+            {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /// <summary>
+    /// Validates whether a given bicep identifier name is correctly formed of
+    /// letters, numbers, and underscores.
+    /// </summary>
+    /// <param name="identifierName">The proposed bicep identifier name.</param>
+    /// <param name="paramName">Optional parameter name to use for exceptions.</param>
+    /// <exception cref="ArgumentNullException">Throws if null.</exception>
+    /// <exception cref="ArgumentException">Throws if empty or invalid.</exception>
+    public static void ValidateIdentifierName(string? identifierName, string? paramName = default)
+    {
+        paramName ??= nameof(identifierName);
+        if (identifierName is null)
+        {
+            throw new ArgumentNullException(paramName, $"{paramName} cannot be null.");
+        }
+        else if (identifierName.Length == 0)
+        {
+            throw new ArgumentException($"{paramName} cannot be empty.", paramName);
+        }
+        else if (char.IsDigit(identifierName[0]))
+        {
+            throw new ArgumentException($"{paramName} cannot start with a number: \"{identifierName}\"", paramName);
+        }
+
+        foreach (var ch in identifierName)
+        {
+            if (!IsAsciiLetterOrDigit(ch) && ch != '_')
+            {
+                throw new ArgumentException($"{paramName} should only contain letters, numbers, and underscores: \"{identifierName}\"", paramName);
+            }
+        }
+    }
+
+    /// <summary>
+    /// Normalizes a proposed bicep identifier name.  Any invalid characters
+    /// will be replaced with underscores.
+    /// </summary>
+    /// <param name="identifierName">The proposed bicep identifier name.</param>
+    /// <returns>A valid bicep identifier name.</returns>
+    /// <exception cref="ArgumentNullException">Throws if null.</exception>
+    /// <exception cref="ArgumentException">Throws if empty.</exception>
+    public static string NormalizeIdentifierName(string? identifierName)
+    {
+        if (IsValidIdentifierName(identifierName))
+        {
+            return identifierName!;
+        }
+
+        if (identifierName is null)
+        {
+            // TODO: This may be relaxed in the future to generate an automatic
+            // name rather than throwing
+            throw new ArgumentNullException(nameof(identifierName), $"{nameof(identifierName)} cannot be null.");
+        }
+        else if (identifierName.Length == 0)
+        {
+            throw new ArgumentException($"{nameof(identifierName)} cannot be empty.", nameof(identifierName));
+        }
+
+        StringBuilder builder = new(identifierName.Length);
+
+        // Digits are not allowed as the first character, so prepend an
+        // underscore if the identifierName starts with a digit
+        if (char.IsDigit(identifierName[0]))
+        {
+            builder.Append('_');
+        }
+
+        foreach (char ch in identifierName)
+        {
+            // TODO: Consider opening this up to other naming strategies if
+            // someone can do something more intelligent for their usage/domain
+            builder.Append(IsAsciiLetterOrDigit(ch) ? ch : '_');
+        }
+
+        return builder.ToString();
+    }
+
     /// <inheritdoc/>
     protected internal override void Validate(ProvisioningContext? context = null)
     {

sdk/provisioning/Azure.Provisioning/src/Primitives/ProvisioningConstruct.cs

@@ -23,10 +23,15 @@ public abstract class NamedProvisioningConstruct : ProvisioningConstruct
     public string IdentifierName
     {
         get => _identifierName;
-        set => _identifierName = ValidateIdentifierName(value, nameof(value));
+        set
+        {
+            Infrastructure.ValidateIdentifierName(value, nameof(value));
+            _identifierName = value;
+        }
     }
     private string _identifierName;
-    // TODO: Listen for feedback, but discuss IdentifierName vs. ProvisioningName in the Arch Board
+    // TODO: Listen for customer feedback and discuss IdentifierName vs.
+    // ProvisioningName in the Arch Board
 
     /// <summary>
     /// Creates a named Bicep entity, like a resource or parameter.
@@ -36,32 +41,12 @@ public string IdentifierName
     /// refer to the resource in expressions, but is not the Azure name of the
     /// resource.  This value can contain letters, numbers, and underscores.
     /// </param>
-    protected NamedProvisioningConstruct(string identifierName) =>
-        _identifierName = ValidateIdentifierName(identifierName, nameof(identifierName));
-
-    // TODO: Relax this in the future when we make identifier names optional
-    private static string ValidateIdentifierName(string identifierName, string paramName)
+    protected NamedProvisioningConstruct(string identifierName)
     {
-        // TODO: Enable when Aspire is ready
-        /*
-        if (identifierName is null)
-        {
-            throw new ArgumentNullException(paramName, $"{nameof(IdentifierName)} cannot be null.");
-        }
-        else if (identifierName.Length == 0)
-        {
-            throw new ArgumentException($"{nameof(IdentifierName)} cannot be empty.", paramName);
-        }
-
-        foreach (var ch in identifierName)
-        {
-            if (!char.IsLetterOrDigit(ch) && ch != '_')
-            {
-                throw new ArgumentException($"{nameof(IdentifierName)} \"{identifierName}\" should only contain letters, numbers, and underscores.", paramName);
-            }
-        }
-        /**/
-        return identifierName;
+        // TODO: In the near future we'll make this optional and only validate
+        // if the value passed in isn't null.
+        Infrastructure.ValidateIdentifierName(identifierName, nameof(identifierName));
+        _identifierName = identifierName;
     }
 }
 

sdk/provisioning/Azure.Provisioning/tests/SampleTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using Azure.Core;
 using Azure.Core.TestFramework;
@@ -348,15 +349,32 @@ await test.Define(
     [Test]
     public void ValidNames()
     {
-        // TODO: Enable when we turn NamedProvisioningConstruct.ValidateIdentifierName back on
-        /*
+        // Check null is invalid
+        Assert.IsFalse(Infrastructure.IsValidIdentifierName(null));
+        Assert.Throws<ArgumentNullException>(() => Infrastructure.ValidateIdentifierName(null));
         Assert.Throws<ArgumentNullException>(() => new StorageAccount(null!));
-        Assert.Throws<ArgumentException>(() => new StorageAccount(""));
-        Assert.Throws<ArgumentException>(() => new StorageAccount("my-storage"));
-        Assert.Throws<ArgumentException>(() => new StorageAccount("my storage"));
-        Assert.Throws<ArgumentException>(() => new StorageAccount("my:storage"));
-        Assert.Throws<ArgumentException>(() => new StorageAccount("storage$"));
-        /**/
-        _ = new StorageAccount("ABCdef123_");
+
+        // Check invalid names
+        List<string> invalid = ["", "my-storage", "my storage", "my:storage", "storage$", "1storage", "KforKelvin"];
+        foreach (string name in invalid)
+        {
+            Assert.IsFalse(Infrastructure.IsValidIdentifierName(name));
+            Assert.Throws<ArgumentException>(() => Infrastructure.ValidateIdentifierName(name));
+            if (!string.IsNullOrEmpty(name))
+            {
+                Assert.AreNotEqual(name, Infrastructure.NormalizeIdentifierName(name));
+            }
+            Assert.Throws<ArgumentException>(() => new StorageAccount(name));
+        }
+
+        // Check valid names
+        List<string> valid = ["foo", "FOO", "Foo", "f", "_foo", "_", "foo123", "ABCdef123_"];
+        foreach (string name in valid)
+        {
+            Assert.IsTrue(Infrastructure.IsValidIdentifierName(name));
+            Assert.DoesNotThrow(() => Infrastructure.ValidateIdentifierName(name));
+            Assert.AreEqual(name, Infrastructure.NormalizeIdentifierName(name));
+            Assert.DoesNotThrow(() => new StorageAccount(name));
+        }
     }
 }