Refactor/tag creator (#732)

* Refactor TagTemplateCreator and tests. * Resource name generator helper added * Introduce error messages and exception for duplicate sanitized tag's display name * Update documentation for tags creation * Combine common utilities into NamingHelper * Change path generation for templates in the CreatorExecutor. Add CreatorExecutorTests Co-authored-by: Farhad Alizada <falizada@microsoft.com>
Azure · Jun 3, 2022 · 0bdf11b · 0bdf11b
1 parent 1bd13d4
commit 0bdf11b
Show file tree

Hide file tree

Showing 25 changed files with 460 additions and 77 deletions.
diff --git a/src/ArmTemplates/Commands/Executors/CreatorExecutor.cs b/src/ArmTemplates/Commands/Executors/CreatorExecutor.cs
@@ -8,6 +8,7 @@
 using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Constants;
+using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Extensions;
 using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.FileHandlers;
 using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Templates.Abstractions;
 using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Creator.Models.Parameters;
@@ -203,9 +204,7 @@ public async Task ExecuteGenerationBasedOnConfiguration()
                 }
             }
 
-            this.logger.LogInformation("Creating tag template");
-            this.logger.LogInformation("------------------------------------------");
-            var tagTemplate = this.creatorParameters.Tags != null ? this.tagTemplateCreator.CreateTagTemplate(this.creatorParameters) : null;
+            var tagTemplate = await this.GenerateTagsTemplateAsync();
 
             // create parameters file
             var templateParameters = this.masterTemplateCreator.CreateMasterTemplateParameterValues(this.creatorParameters);
@@ -242,44 +241,59 @@ public async Task ExecuteGenerationBasedOnConfiguration()
             }
             if (globalServicePolicyTemplate != null)
             {
-                FileWriter.WriteJSONToFile(globalServicePolicyTemplate, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.GlobalServicePolicy));
+                FileWriter.WriteJSONToFile(globalServicePolicyTemplate, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.GlobalServicePolicy));
             }
             if (apiVersionSetsTemplate != null)
             {
-                FileWriter.WriteJSONToFile(apiVersionSetsTemplate, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.ApiVersionSets));
+                FileWriter.WriteJSONToFile(apiVersionSetsTemplate, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.ApiVersionSets));
             }
             if (productsTemplate != null)
             {
-                FileWriter.WriteJSONToFile(productsTemplate, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Products));
+                FileWriter.WriteJSONToFile(productsTemplate, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Products));
             }
             if (productAPIsTemplate != null)
             {
-                FileWriter.WriteJSONToFile(productAPIsTemplate, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.ProductAPIs));
+                FileWriter.WriteJSONToFile(productAPIsTemplate, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.ProductAPIs));
             }
             if (propertyTemplate != null)
             {
-                FileWriter.WriteJSONToFile(propertyTemplate, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.NamedValues));
+                FileWriter.WriteJSONToFile(propertyTemplate, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.NamedValues));
             }
             if (loggersTemplate != null)
             {
-                FileWriter.WriteJSONToFile(loggersTemplate, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Loggers));
+                FileWriter.WriteJSONToFile(loggersTemplate, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Loggers));
             }
             if (backendsTemplate != null)
             {
-                FileWriter.WriteJSONToFile(backendsTemplate, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Backends));
+                FileWriter.WriteJSONToFile(backendsTemplate, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Backends));
             }
             if (authorizationServersTemplate != null)
             {
-                FileWriter.WriteJSONToFile(authorizationServersTemplate, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.AuthorizationServers));
+                FileWriter.WriteJSONToFile(authorizationServersTemplate, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.AuthorizationServers));
             }
             if (tagTemplate != null)
             {
-                FileWriter.WriteJSONToFile(tagTemplate, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Tags));
+                FileWriter.WriteJSONToFile(tagTemplate, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Tags));
             }
 
             // write parameters to outputLocation
-            FileWriter.WriteJSONToFile(templateParameters, string.Concat(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Parameters));
+            FileWriter.WriteJSONToFile(templateParameters, Path.Combine(this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Parameters));
             this.logger.LogInformation("Templates written to output location");
         }
+
+        public async Task<Template> GenerateTagsTemplateAsync()
+        {
+            if (this.creatorParameters.Tags.IsNullOrEmpty() && this.creatorParameters.Apis.All(x => x.Tags.IsNullOrEmpty()))
+            {
+                return null;    
+            }
+
+            this.logger.LogInformation("Creating tag template");
+
+            var tagTemplate = this.tagTemplateCreator.CreateTagTemplate(this.creatorParameters);
+            await FileWriter.SaveAsJsonAsync(tagTemplate, this.creatorParameters.OutputLocation, this.creatorParameters.FileNames.Tags);
+
+            return tagTemplate;
+        }
     }
 }
diff --git a/src/ArmTemplates/Common/Constants/ErrorMessages.cs b/src/ArmTemplates/Common/Constants/ErrorMessages.cs
@@ -0,0 +1,13 @@
+// --------------------------------------------------------------------------
+//  Copyright (c) Microsoft Corporation. All rights reserved.
+//  Licensed under the MIT License.
+// --------------------------------------------------------------------------
+
+namespace Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Constants
+{
+    public static class ErrorMessages
+    {
+        public const string DuplicateTagResourceNameErrorMessage = "Duplicate tag resource name found during sanitizing the display name. Please consider renaming tags: {0}, {1}. Both resulted resource name to be equal to: {2}";
+        public const string EmptyResourceNameAfterSanitizingErrorMessage = "Sanitizing the display name '{0}' resulted empty string";
+    }
+}
diff --git a/src/ArmTemplates/Common/Exceptions/DuplicateTagResourceNameException.cs b/src/ArmTemplates/Common/Exceptions/DuplicateTagResourceNameException.cs
@@ -0,0 +1,17 @@
+// --------------------------------------------------------------------------
+//  Copyright (c) Microsoft Corporation. All rights reserved.
+//  Licensed under the MIT License.
+// --------------------------------------------------------------------------
+
+using System;
+using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Constants;
+
+namespace Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Exceptions
+{
+    public class DuplicateTagResourceNameException : Exception
+    {
+        public DuplicateTagResourceNameException(string existingValue, string tagName, string resourceName): base (string.Format(ErrorMessages.DuplicateTagResourceNameErrorMessage, existingValue, tagName, resourceName))
+        {
+        }
+    }
+}
diff --git a/src/ArmTemplates/Common/Exceptions/EmptyResourceNameException.cs b/src/ArmTemplates/Common/Exceptions/EmptyResourceNameException.cs
@@ -0,0 +1,17 @@
+// --------------------------------------------------------------------------
+//  Copyright (c) Microsoft Corporation. All rights reserved.
+//  Licensed under the MIT License.
+// --------------------------------------------------------------------------
+
+using System;
+using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Constants;
+
+namespace Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Exceptions
+{
+    public class EmptyResourceNameException : Exception
+    {
+        public EmptyResourceNameException(string resourceName) : base(string.Format(ErrorMessages.EmptyResourceNameAfterSanitizingErrorMessage, resourceName))
+        {
+        }
+    }
+}
diff --git a/src/ArmTemplates/Common/Extensions/CollectionExtensions.cs b/src/ArmTemplates/Common/Extensions/CollectionExtensions.cs
@@ -8,7 +8,7 @@
 
 namespace Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Extensions
 {
-    static class CollectionExtensions
+    public static class CollectionExtensions
     {
         public static bool IsNullOrEmpty<T>(this IEnumerable<T> collection)
         {

diff --git a/...ommon/Extensions/ParameterNamingHelper.cs → ...mplates/Common/Extensions/NamingHelper.cs b/...ommon/Extensions/ParameterNamingHelper.cs → ...mplates/Common/Extensions/NamingHelper.cs
@@ -8,9 +8,10 @@
 
 namespace Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Extensions
 {
-    static class ParameterNamingHelper
+    public static class NamingHelper
     {
         static readonly Regex ExcludeOtherFromLettersAndDigitsRegex = new Regex("[^a-zA-Z0-9]");
+        static readonly Regex ExcludeOtherFromAlphaNumericsAndHyphensRegex = new Regex("[^a-zA-Z0-9-]");
 
         public static string GetSubstringBetweenTwoCharacters(char left, char right, string fullString)
         {
@@ -42,5 +43,22 @@ public static string GenerateValidParameterName(string apiName, string prefix)
                 return validApiName;
             }
         }
+
+        public static string GenerateValidResourceNameFromDisplayName(string displayName)
+        {
+            if (string.IsNullOrEmpty(displayName))
+            {
+                return string.Empty;
+            }
+
+            var trimmedDisplayName = displayName.Trim().Replace(" ", "-");
+            var resourceName = ExcludeOtherFromAlphaNumericsAndHyphensRegex.Replace(trimmedDisplayName, string.Empty);
+            return resourceName;
+        }
+
+        public static string GenerateParametrizedResourceName(string parameterName, string resourceName)
+        {
+            return $"[concat(parameters('{parameterName}'), '/{resourceName}')]";
+        }
     }
 }
diff --git a/src/ArmTemplates/Common/Templates/Logger/LoggerTemplateResources.cs b/src/ArmTemplates/Common/Templates/Logger/LoggerTemplateResources.cs
@@ -51,7 +51,7 @@ public void SetLoggerResourceIdForEachLogger()
         static string GetValidLoggerParamName(string resourceName)
         {
             var loggerNameStrs = resourceName.Split(new char[] { ',' });
-            var validLoggerName = ParameterNamingHelper.GenerateValidParameterName(loggerNameStrs[loggerNameStrs.Length - 1], ParameterPrefix.LogResourceId);
+            var validLoggerName = NamingHelper.GenerateValidParameterName(loggerNameStrs[loggerNameStrs.Length - 1], ParameterPrefix.LogResourceId);
             return validLoggerName;
         }
     }

diff --git a/src/ArmTemplates/Creator/Models/Parameters/CreatorParameters.cs b/src/ArmTemplates/Creator/Models/Parameters/CreatorParameters.cs
@@ -74,7 +74,7 @@ internal void OverrideParameters(CreateConsoleAppConfiguration configuration, Fi
             }
         }
 
-        internal void GenerateFileNames()
+        public void GenerateFileNames()
         {
             this.FileNames = this.BaseFileName == null
                 ? FileNameGenerator.GenerateFileNames(this.ApimServiceName)

diff --git a/src/ArmTemplates/Creator/TemplateCreators/Abstractions/ITagTemplateCreator.cs b/src/ArmTemplates/Creator/TemplateCreators/Abstractions/ITagTemplateCreator.cs
@@ -3,11 +3,6 @@
 //  Licensed under the MIT License.
 // --------------------------------------------------------------------------
 
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Templates.Abstractions;
 using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Creator.Models.Parameters;
 

diff --git a/src/ArmTemplates/Creator/TemplateCreators/PropertyTemplateCreator.cs b/src/ArmTemplates/Creator/TemplateCreators/PropertyTemplateCreator.cs
@@ -53,14 +53,14 @@ public Template CreatePropertyTemplate(CreatorParameters creatorConfig)
             {
                 string value = namedValue.Value == null ? null
                    : creatorConfig.ParameterizeNamedValues
-                       ? $"[parameters('{ParameterNames.NamedValues}').{ParameterNamingHelper.GenerateValidParameterName(namedValue.DisplayName, ParameterPrefix.Property)}]"
+                       ? $"[parameters('{ParameterNames.NamedValues}').{NamingHelper.GenerateValidParameterName(namedValue.DisplayName, ParameterPrefix.Property)}]"
                        : namedValue.Value;
 
                 var keyVault = namedValue.KeyVault == null ? null
                     : creatorConfig.ParameterizeNamedValues
                         ? new NamedValueResourceKeyVaultProperties
                         {
-                            SecretIdentifier = $"[parameters('{ParameterNames.NamedValueKeyVaultSecrets}').{ParameterNamingHelper.GenerateValidParameterName(namedValue.DisplayName, ParameterPrefix.Property)}]"
+                            SecretIdentifier = $"[parameters('{ParameterNames.NamedValueKeyVaultSecrets}').{NamingHelper.GenerateValidParameterName(namedValue.DisplayName, ParameterPrefix.Property)}]"
                         }
                         : namedValue.KeyVault;
 

diff --git a/src/ArmTemplates/Creator/TemplateCreators/TagTemplateCreator.cs b/src/ArmTemplates/Creator/TemplateCreators/TagTemplateCreator.cs
@@ -10,6 +10,8 @@
 using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Templates.Builders.Abstractions;
 using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Creator.Models.Parameters;
 using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Creator.TemplateCreators.Abstractions;
+using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Extensions;
+using Microsoft.Azure.Management.ApiManagement.ArmTemplates.Common.Exceptions;
 
 namespace Microsoft.Azure.Management.ApiManagement.ArmTemplates.Creator.TemplateCreators
 {
@@ -22,53 +24,71 @@ public TagTemplateCreator(ITemplateBuilder templateBuilder)
             this.templateBuilder = templateBuilder;
         }
 
-        public Template CreateTagTemplate(CreatorParameters creatorConfig)
+        static void AddTagNameToDictionary(string tagName, Dictionary<string, string> tagsDictionary)
         {
-            // create empty template
-            Template tagTemplate = this.templateBuilder.GenerateEmptyTemplate().Build();
+            var resourceName = NamingHelper.GenerateValidResourceNameFromDisplayName(tagName);
 
-            // add parameters
-            tagTemplate.Parameters = new Dictionary<string, TemplateParameterProperties>
+            if (string.IsNullOrEmpty(resourceName)) 
             {
-                {ParameterNames.ApimServiceName, new TemplateParameterProperties(){ Type = "string" }}
-            };
+                throw new EmptyResourceNameException(tagName);
+            }
+
+            if (tagsDictionary.ContainsKey(resourceName))
+            {
+                var existingValue = tagsDictionary[resourceName];
+                if (!existingValue.Equals(tagName))
+                {
+                    throw new DuplicateTagResourceNameException(existingValue, tagName, resourceName);
+                }
+            }
+            else
+            {
+                tagsDictionary.Add(resourceName, tagName);
+            }
+        }
 
-            // aggregate all tags from apis
-            HashSet<string> tagHashset = new HashSet<string>();
-            List<ApiConfig> apis = creatorConfig.Apis;
-            if (apis != null)
+        public Template CreateTagTemplate(CreatorParameters creatorConfig)
+        {
+            var tagTemplate = this.templateBuilder.GenerateTemplateWithApimServiceNameProperty().Build();
+            var tagsDictionary = new Dictionary<string, string>();
+
+            if (!creatorConfig.Apis.IsNullOrEmpty())
             {
-                foreach (ApiConfig api in apis)
+                foreach (var api in creatorConfig.Apis)
                 {
-                    if (api.Tags != null)
+                    if (!api.Tags.IsNullOrEmpty())
                     {
-                        string[] apiTags = api.Tags.Split(", ");
-                        foreach (string apiTag in apiTags)
+                        var apiTags = api.Tags.Split(",");
+
+                        foreach (var apiTag in apiTags)
                         {
-                            tagHashset.Add(apiTag);
+                            AddTagNameToDictionary(apiTag, tagsDictionary);
                         }
                     }
                 }
             }
-            foreach (TagProperties tag in creatorConfig.Tags)
+
+            if (!creatorConfig.Tags.IsNullOrEmpty())
             {
-                tagHashset.Add(tag.DisplayName);
+                foreach (var tag in creatorConfig.Tags)
+                {
+                    AddTagNameToDictionary(tag.DisplayName, tagsDictionary);
+                }
             }
 
-            List<TemplateResource> resources = new List<TemplateResource>();
-            foreach (string tag in tagHashset)
+            var resources = new List<TemplateResource>();
+
+            foreach (var (tagResourceName, tagDisplayName) in tagsDictionary)
             {
-                // create tag resource with properties
-                TagTemplateResource tagTemplateResource = new TagTemplateResource()
+                var tagTemplateResource = new TagTemplateResource()
                 {
-                    Name = $"[concat(parameters('{ParameterNames.ApimServiceName}'), '/{tag}')]",
+                    Name = NamingHelper.GenerateParametrizedResourceName(ParameterNames.ApimServiceName, tagResourceName),
                     Type = ResourceTypeConstants.Tag,
                     ApiVersion = GlobalConstants.ApiVersion,
                     Properties = new TagProperties()
                     {
-                        DisplayName = tag
-                    },
-                    DependsOn = new string[] { }
+                        DisplayName = tagDisplayName
+                    }
                 };
                 resources.Add(tagTemplateResource);
             }

diff --git a/src/ArmTemplates/Extractor/EntityExtractors/APIExtractor.cs b/src/ArmTemplates/Extractor/EntityExtractors/APIExtractor.cs
@@ -142,14 +142,14 @@ void SetArmTemplateValuesToApiTemplateResource(ApiTemplateResource apiResource,
 
             if (extractorParameters.ParameterizeServiceUrl)
             {
-                apiResource.Properties.ServiceUrl = $"[parameters('{ParameterNames.ServiceUrl}').{ParameterNamingHelper.GenerateValidParameterName(originalServiceApiName, ParameterPrefix.Api)}]";
+                apiResource.Properties.ServiceUrl = $"[parameters('{ParameterNames.ServiceUrl}').{NamingHelper.GenerateValidParameterName(originalServiceApiName, ParameterPrefix.Api)}]";
             }
 
             if (extractorParameters.ParametrizeApiOauth2Scope)
             {
                 if (apiResource.Properties.AuthenticationSettings?.OAuth2?.Scope is not null)
                 {
-                    apiResource.Properties.AuthenticationSettings.OAuth2.Scope = $"[parameters('{ParameterNames.ApiOauth2ScopeSettings}').{ParameterNamingHelper.GenerateValidParameterName(originalServiceApiName, ParameterPrefix.ApiOauth2Scope)}]";
+                    apiResource.Properties.AuthenticationSettings.OAuth2.Scope = $"[parameters('{ParameterNames.ApiOauth2ScopeSettings}').{NamingHelper.GenerateValidParameterName(originalServiceApiName, ParameterPrefix.ApiOauth2Scope)}]";
                 }
             }
 

diff --git a/src/ArmTemplates/Extractor/EntityExtractors/ApiRevisionExtractor.cs b/src/ArmTemplates/Extractor/EntityExtractors/ApiRevisionExtractor.cs
@@ -118,7 +118,7 @@ async Task<ApiTemplateResource> GenerateApiTemplateResourceAsVersioned(string ap
 
             if (extractorParameters.ParameterizeServiceUrl)
             {
-                apiDetails.Properties.ServiceUrl = $"[parameters('{ParameterNames.ServiceUrl}').{ParameterNamingHelper.GenerateValidParameterName(apiName, ParameterPrefix.Api)}]";
+                apiDetails.Properties.ServiceUrl = $"[parameters('{ParameterNames.ServiceUrl}').{NamingHelper.GenerateValidParameterName(apiName, ParameterPrefix.Api)}]";
             }
 
             if (apiDetails.Properties.ApiVersionSetId != null)

diff --git a/src/ArmTemplates/Extractor/EntityExtractors/BackendExtractor.cs b/src/ArmTemplates/Extractor/EntityExtractors/BackendExtractor.cs
@@ -110,7 +110,7 @@ void SaveBackendApiParametersToCache()
                     }
 
                     var backendApiParameters = new BackendApiParameters();
-                    var backendValidName = ParameterNamingHelper.GenerateValidParameterName(originalBackendName, ParameterPrefix.Backend).ToLower();
+                    var backendValidName = NamingHelper.GenerateValidParameterName(originalBackendName, ParameterPrefix.Backend).ToLower();
 
                     if (!string.IsNullOrEmpty(backendResource.Properties.ResourceId))
                     {