From ac754e617d66ff24378bdca09755862479f6eec9 Mon Sep 17 00:00:00 2001 From: matthchr Date: Thu, 12 May 2016 18:42:04 -0700 Subject: [PATCH] Refactor parameter grouping and enable better comments - Parameter grouping code is clearer. - Models for parameter grouping now more clearly state what operations they are used on. --- .../Azure.Extensions/AzureExtensions.cs | 2 +- .../AutoRest.Generator.Extensions.csproj | 1 + .../Extensions/Extensions/Extensions.cs | 137 +---------- .../ParameterGroupExtensionHelper.cs | 229 ++++++++++++++++++ .../Azure.Python/AzurePythonCodeGenerator.cs | 2 +- 5 files changed, 233 insertions(+), 138 deletions(-) create mode 100644 AutoRest/Generators/Extensions/Extensions/ParameterGroupExtensionHelper.cs diff --git a/AutoRest/Generators/Extensions/Azure.Extensions/AzureExtensions.cs b/AutoRest/Generators/Extensions/Azure.Extensions/AzureExtensions.cs index c38246fdccd61..d4863ccf8beb9 100644 --- a/AutoRest/Generators/Extensions/Azure.Extensions/AzureExtensions.cs +++ b/AutoRest/Generators/Extensions/Azure.Extensions/AzureExtensions.cs @@ -77,7 +77,7 @@ public static void NormalizeAzureClientModel(ServiceClient serviceClient, Settin ParseODataExtension(serviceClient); FlattenModels(serviceClient); FlattenMethodParameters(serviceClient, settings); - AddParameterGroups(serviceClient); + ParameterGroupExtensionHelper.AddParameterGroups(serviceClient); AddLongRunningOperations(serviceClient); AddAzureProperties(serviceClient); SetDefaultResponses(serviceClient); diff --git a/AutoRest/Generators/Extensions/Extensions/AutoRest.Generator.Extensions.csproj b/AutoRest/Generators/Extensions/Extensions/AutoRest.Generator.Extensions.csproj index b5ca288802491..baeb25e146303 100644 --- a/AutoRest/Generators/Extensions/Extensions/AutoRest.Generator.Extensions.csproj +++ b/AutoRest/Generators/Extensions/Extensions/AutoRest.Generator.Extensions.csproj @@ -27,6 +27,7 @@ + True diff --git a/AutoRest/Generators/Extensions/Extensions/Extensions.cs b/AutoRest/Generators/Extensions/Extensions/Extensions.cs index 44b69bfea2122..2016e53f1fb60 100644 --- a/AutoRest/Generators/Extensions/Extensions/Extensions.cs +++ b/AutoRest/Generators/Extensions/Extensions/Extensions.cs @@ -44,7 +44,7 @@ public static void NormalizeClientModel(ServiceClient serviceClient, Settings se { FlattenModels(serviceClient); FlattenMethodParameters(serviceClient, settings); - AddParameterGroups(serviceClient); + ParameterGroupExtensionHelper.AddParameterGroups(serviceClient); ProcessParameterizedHost(serviceClient, settings); } @@ -333,141 +333,6 @@ public static void RemoveUnreferencedTypes(ServiceClient serviceClient, HashSet< } } - /// - /// Adds the parameter groups to operation parameters. - /// - /// - public static void AddParameterGroups(ServiceClient serviceClient) - { - if (serviceClient == null) - { - throw new ArgumentNullException("serviceClient"); - } - - HashSet generatedParameterGroups = new HashSet(); - - foreach (Method method in serviceClient.Methods) - { - //Copy out flattening transformations as they should be the last - List flatteningTransformations = method.InputParameterTransformation.ToList(); - method.InputParameterTransformation.Clear(); - - //This group name is normalized by each languages code generator later, so it need not happen here. - Dictionary> parameterGroups = new Dictionary>(); - - foreach (Parameter parameter in method.Parameters) - { - if (parameter.Extensions.ContainsKey(ParameterGroupExtension)) - { - JContainer extensionObject = parameter.Extensions[ParameterGroupExtension] as JContainer; - if (extensionObject != null) - { - string specifiedGroupName = extensionObject.Value("name"); - string parameterGroupName; - if (specifiedGroupName == null) - { - string postfix = extensionObject.Value("postfix") ?? "Parameters"; - parameterGroupName = method.Group + "-" + method.Name + "-" + postfix; - } - else - { - parameterGroupName = specifiedGroupName; - } - - if (!parameterGroups.ContainsKey(parameterGroupName)) - { - parameterGroups.Add(parameterGroupName, new Dictionary()); - } - - Property groupProperty = new Property() - { - IsReadOnly = false, //Since these properties are used as parameters they are never read only - Name = parameter.Name, - IsRequired = parameter.IsRequired, - DefaultValue = parameter.DefaultValue, - //Constraints = parameter.Constraints, Omit these since we don't want to perform parameter validation - Documentation = parameter.Documentation, - Type = parameter.Type, - SerializedName = null //Parameter is never serialized directly - }; - // Copy over extensions - foreach (var key in parameter.Extensions.Keys) - { - groupProperty.Extensions[key] = parameter.Extensions[key]; - } - - parameterGroups[parameterGroupName].Add(groupProperty, parameter); - } - } - } - - foreach (string parameterGroupName in parameterGroups.Keys) - { - CompositeType parameterGroupType = - generatedParameterGroups.FirstOrDefault(item => item.Name == parameterGroupName); - if (parameterGroupType == null) - { - parameterGroupType = new CompositeType - { - Name = parameterGroupName, - Documentation = "Additional parameters for one or more operations" - }; - generatedParameterGroups.Add(parameterGroupType); - - //Add to the service client - serviceClient.ModelTypes.Add(parameterGroupType); - } - foreach (Property property in parameterGroups[parameterGroupName].Keys) - { - Property matchingProperty = parameterGroupType.Properties.FirstOrDefault( - item => item.Name == property.Name && - item.IsReadOnly == property.IsReadOnly && - item.DefaultValue == property.DefaultValue && - item.SerializedName == property.SerializedName); - if (matchingProperty == null) - { - parameterGroupType.Properties.Add(property); - } - } - - bool isGroupParameterRequired = parameterGroupType.Properties.Any(p => p.IsRequired); - - //Create the new parameter object based on the parameter group type - Parameter parameterGroup = new Parameter() - { - Name = parameterGroupName, - IsRequired = isGroupParameterRequired, - Location = ClientModel.ParameterLocation.None, - SerializedName = string.Empty, - Type = parameterGroupType, - Documentation = "Additional parameters for the operation" - }; - - method.Parameters.Add(parameterGroup); - - //Link the grouped parameters to their parent, and remove them from the method parameters - foreach (Property property in parameterGroups[parameterGroupName].Keys) - { - Parameter p = parameterGroups[parameterGroupName][property]; - - var parameterTransformation = new ParameterTransformation - { - OutputParameter = p - }; - parameterTransformation.ParameterMappings.Add(new ParameterMapping - { - InputParameter = parameterGroup, - InputParameterProperty = property.GetClientName() - }); - method.InputParameterTransformation.Add(parameterTransformation); - method.Parameters.Remove(p); - } - } - - // Copy back flattening transformations if any - flatteningTransformations.ForEach(t => method.InputParameterTransformation.Add(t)); - } - } /// /// Flattens the request payload if the number of properties of the diff --git a/AutoRest/Generators/Extensions/Extensions/ParameterGroupExtensionHelper.cs b/AutoRest/Generators/Extensions/Extensions/ParameterGroupExtensionHelper.cs new file mode 100644 index 0000000000000..5a97e2619f79f --- /dev/null +++ b/AutoRest/Generators/Extensions/Extensions/ParameterGroupExtensionHelper.cs @@ -0,0 +1,229 @@ +namespace Microsoft.Rest.Generator +{ + using System; + using System.Collections.Generic; + using System.Globalization; + using System.Linq; + using System.Text; + using System.Threading.Tasks; + using ClientModel; + using Newtonsoft.Json.Linq; + + public static class ParameterGroupExtensionHelper + { + private class ParameterGroup + { + public string Name { get; } + + public Dictionary ParameterMapping { get; } + + public ParameterGroup(string name, Dictionary parameterMapping) + { + this.Name = name; + this.ParameterMapping = parameterMapping; + } + } + + private static Property CreateParameterGroupProperty(Parameter parameter) + { + Property groupProperty = new Property() + { + IsReadOnly = false, //Since these properties are used as parameters they are never read only + Name = parameter.Name, + IsRequired = parameter.IsRequired, + DefaultValue = parameter.DefaultValue, + //Constraints = parameter.Constraints, Omit these since we don't want to perform parameter validation + Documentation = parameter.Documentation, + Type = parameter.Type, + SerializedName = null //Parameter is never serialized directly + }; + + // Copy over extensions + foreach (var key in parameter.Extensions.Keys) + { + groupProperty.Extensions[key] = parameter.Extensions[key]; + } + + return groupProperty; + } + + private static ParameterGroup BuildParameterGroup(string parameterGroupName, Method method) + { + Dictionary parameterMapping = method.Parameters.Where( + p => GetParameterGroupName(method.Group, method.Name, p) == parameterGroupName).ToDictionary( + CreateParameterGroupProperty, + p => p); + + return new ParameterGroup(parameterGroupName, parameterMapping); + } + + private static string GetParameterGroupName(string methodGroupName, string methodName, Parameter parameter) + { + if (parameter.Extensions.ContainsKey(Extensions.ParameterGroupExtension)) + { + JContainer extensionObject = parameter.Extensions[Extensions.ParameterGroupExtension] as JContainer; + if (extensionObject != null) + { + string specifiedGroupName = extensionObject.Value("name"); + string parameterGroupName; + if (specifiedGroupName == null) + { + string postfix = extensionObject.Value("postfix") ?? "Parameters"; + parameterGroupName = methodGroupName + "-" + methodName + "-" + postfix; + } + else + { + parameterGroupName = specifiedGroupName; + } + return parameterGroupName; + } + } + + return null; + } + + private static IEnumerable ExtractParameterGroupNames(Method method) + { + return method.Parameters.Select(p => GetParameterGroupName(method.Group, method.Name, p)).Where(name => !string.IsNullOrEmpty(name)).Distinct(); + } + + private static IEnumerable ExtractParameterGroups(Method method) + { + IEnumerable parameterGroupNames = ExtractParameterGroupNames(method); + + return parameterGroupNames.Select(parameterGroupName => BuildParameterGroup(parameterGroupName, method)); + } + + private static IEnumerable GetMethodsUsingParameterGroup(IEnumerable methods, ParameterGroup parameterGroup) + { + return methods.Where(m => ExtractParameterGroupNames(m).Contains(parameterGroup.Name)); + } + + private static string GenerateParameterGroupModelText(IEnumerable methodsWhichUseGroup) + { + Func createOperationDisplayString = (group, name) => + { + return string.IsNullOrEmpty(group) ? name : string.Format(CultureInfo.InvariantCulture, "{0}_{1}", group, name); + }; + + List methodList = methodsWhichUseGroup.ToList(); + if (methodList.Count == 1) + { + Method method = methodList.Single(); + return string.Format(CultureInfo.InvariantCulture, "Additional parameters for the {0} operation.", + createOperationDisplayString(method.Group, method.Name)); + } + else if (methodList.Count <= 4) + { + string operationsString = string.Join(", ", methodList.Select( + m => string.Format(CultureInfo.InvariantCulture, createOperationDisplayString(m.Group, m.Name)))); + + return string.Format(CultureInfo.InvariantCulture, "Additional parameters for a set of operations, such as: {0}.", operationsString); + } + else + { + return "Additional parameters for a set of operations."; + } + } + + /// + /// Adds the parameter groups to operation parameters. + /// + /// + public static void AddParameterGroups(ServiceClient serviceClient) + { + if (serviceClient == null) + { + throw new ArgumentNullException("serviceClient"); + } + + HashSet generatedParameterGroups = new HashSet(); + + foreach (Method method in serviceClient.Methods) + { + //Copy out flattening transformations as they should be the last + List flatteningTransformations = method.InputParameterTransformation.ToList(); + method.InputParameterTransformation.Clear(); + + //This group name is normalized by each languages code generator later, so it need not happen here. + IEnumerable parameterGroups = ExtractParameterGroups(method); + + List parametersToAddToMethod = new List(); + List parametersToRemoveFromMethod = new List(); + + foreach (ParameterGroup parameterGroup in parameterGroups) + { + CompositeType parameterGroupType = + generatedParameterGroups.FirstOrDefault(item => item.Name == parameterGroup.Name); + + if (parameterGroupType == null) + { + IEnumerable methodsWhichUseGroup = GetMethodsUsingParameterGroup(serviceClient.Methods, parameterGroup); + + parameterGroupType = new CompositeType + { + Name = parameterGroup.Name, + Documentation = GenerateParameterGroupModelText(methodsWhichUseGroup) + }; + generatedParameterGroups.Add(parameterGroupType); + + //Add to the service client + serviceClient.ModelTypes.Add(parameterGroupType); + } + + foreach (Property property in parameterGroup.ParameterMapping.Keys) + { + Property matchingProperty = parameterGroupType.Properties.FirstOrDefault( + item => item.Name == property.Name && + item.IsReadOnly == property.IsReadOnly && + item.DefaultValue == property.DefaultValue && + item.SerializedName == property.SerializedName); + if (matchingProperty == null) + { + parameterGroupType.Properties.Add(property); + } + } + + bool isGroupParameterRequired = parameterGroupType.Properties.Any(p => p.IsRequired); + + //Create the new parameter object based on the parameter group type + Parameter newParameter = new Parameter() + { + Name = parameterGroup.Name, + IsRequired = isGroupParameterRequired, + Location = ClientModel.ParameterLocation.None, + SerializedName = string.Empty, + Type = parameterGroupType, + Documentation = "Additional parameters for the operation" + }; + parametersToAddToMethod.Add(newParameter); + + //Link the grouped parameters to their parent, and remove them from the method parameters + foreach (Property property in parameterGroup.ParameterMapping.Keys) + { + Parameter p = parameterGroup.ParameterMapping[property]; + + var parameterTransformation = new ParameterTransformation + { + OutputParameter = p + }; + + parameterTransformation.ParameterMappings.Add(new ParameterMapping + { + InputParameter = newParameter, + InputParameterProperty = property.GetClientName() + }); + method.InputParameterTransformation.Add(parameterTransformation); + parametersToRemoveFromMethod.Add(p); + } + } + + method.Parameters.RemoveAll(p => parametersToRemoveFromMethod.Contains(p)); + method.Parameters.AddRange(parametersToAddToMethod); + + // Copy back flattening transformations if any + flatteningTransformations.ForEach(t => method.InputParameterTransformation.Add(t)); + } + } + } +} diff --git a/AutoRest/Generators/Python/Azure.Python/AzurePythonCodeGenerator.cs b/AutoRest/Generators/Python/Azure.Python/AzurePythonCodeGenerator.cs index eeb311593bb52..2bc8932aa11fc 100644 --- a/AutoRest/Generators/Python/Azure.Python/AzurePythonCodeGenerator.cs +++ b/AutoRest/Generators/Python/Azure.Python/AzurePythonCodeGenerator.cs @@ -62,7 +62,7 @@ public override void NormalizeClientModel(ServiceClient serviceClient) AzureExtensions.UpdateHeadMethods(serviceClient); AzureExtensions.ParseODataExtension(serviceClient); Extensions.FlattenModels(serviceClient); - Extensions.AddParameterGroups(serviceClient); + ParameterGroupExtensionHelper.AddParameterGroups(serviceClient); AzureExtensions.AddAzureProperties(serviceClient); AzureExtensions.SetDefaultResponses(serviceClient); CorrectFilterParameters(serviceClient);