Merge pull request #649 from chradek/param-deduping

[v6] dedupe parameter mappers
Azure · May 21, 2020 · f62b2f1 · f62b2f1
2 parents f26b833 + eca44be
commit f62b2f1
Show file tree

Hide file tree

Showing 302 changed files with 4,013 additions and 12,521 deletions.
diff --git a/src/generators/modelsGenerator.ts b/src/generators/modelsGenerator.ts
@@ -36,6 +36,7 @@ import {
   getResponseTypeName,
   getAllModelsNames
 } from "./utils/responseTypeUtils";
+import { getParameterDescription } from "../utils/getParameterDescription";
 
 export function generateModels(clientDetails: ClientDetails, project: Project) {
   const modelsIndexFile = project.createSourceFile(
@@ -127,7 +128,10 @@ function writeOptionsParameter(
     operationName,
     optionalParams,
     sourceFile,
-    { mediaTypes: operationRequestMediaTypes }
+    {
+      mediaTypes: operationRequestMediaTypes,
+      operationFullName: operation.fullName
+    }
   );
 }
 
@@ -423,6 +427,11 @@ function writeUniontypes({ objects }: ClientDetails, modelsFile: SourceFile) {
 interface WriteOptionalParametersOptions {
   baseClass?: string;
   mediaTypes?: Set<KnownMediaType>;
+
+  /**
+   * Useful for getting parameter description.
+   */
+  operationFullName?: string;
 }
 
 function getOptionalGroups(
@@ -463,7 +472,7 @@ function writeOptionalParameters(
   operationName: string,
   optionalParams: ParameterDetails[],
   modelsIndexFile: SourceFile,
-  { baseClass, mediaTypes }: WriteOptionalParametersOptions
+  { baseClass, mediaTypes, operationFullName }: WriteOptionalParametersOptions
 ) {
   if (!optionalParams || !optionalParams.length) {
     return;
@@ -487,13 +496,16 @@ function writeOptionalParameters(
           ...optionalGroupDeclarations,
           ...optionalParams
             .filter(p => !p.targetMediaType || p.targetMediaType === mediaType)
-            .map<PropertySignatureStructure>(p => ({
-              name: p.name,
-              hasQuestionToken: true,
-              type: p.typeDetails.typeName,
-              docs: p.description ? [p.description] : undefined,
-              kind: StructureKind.PropertySignature
-            }))
+            .map<PropertySignatureStructure>(p => {
+              const description = getParameterDescription(p, operationFullName);
+              return {
+                name: p.name,
+                hasQuestionToken: true,
+                type: p.typeDetails.typeName,
+                docs: description ? [description] : undefined,
+                kind: StructureKind.PropertySignature
+              };
+            })
         ]
       });
     }
@@ -505,13 +517,16 @@ function writeOptionalParameters(
       extends: [baseClass || "coreHttp.OperationOptions"],
       properties: [
         ...optionalGroupDeclarations,
-        ...optionalParams.map<PropertySignatureStructure>(p => ({
-          name: p.name,
-          hasQuestionToken: true,
-          type: p.typeDetails.typeName,
-          docs: p.description ? [p.description] : undefined,
-          kind: StructureKind.PropertySignature
-        }))
+        ...optionalParams.map<PropertySignatureStructure>(p => {
+          const description = getParameterDescription(p, operationFullName);
+          return {
+            name: p.name,
+            hasQuestionToken: true,
+            type: p.typeDetails.typeName,
+            docs: description ? [description] : undefined,
+            kind: StructureKind.PropertySignature
+          };
+        })
       ]
     });
   }

diff --git a/src/generators/operationGenerator.ts b/src/generators/operationGenerator.ts
@@ -45,6 +45,7 @@ import {
   getAllModelsNames,
   getResponseTypeName
 } from "./utils/responseTypeUtils";
+import { getParameterDescription } from "../utils/getParameterDescription";
 
 /**
  * Function that writes the code for all the operations.
@@ -520,7 +521,7 @@ function getOperationParameterSignatures(
 
       const newParameter = {
         name: param.name,
-        description: param.description,
+        description: getParameterDescription(param, operation.fullName),
         type: typeName,
         hasQuestionToken: !param.required,
         isContentType: Boolean(

diff --git a/src/generators/utils/parameterUtils.ts b/src/generators/utils/parameterUtils.ts
@@ -60,7 +60,7 @@ export function filterOperationParameters(
     !!(includeGlobalParameters || !param.isGlobal);
 
   const isInOperation = (param: ParameterDetails) =>
-    !!(param.operationsIn && param.operationsIn.includes(operation.fullName));
+    !!(param.operationsIn && param.operationsIn[operation.fullName]);
 
   // We may want to filter out any parameter that is grouped by here.
   // This is so that we can place the group parameter instead.

diff --git a/src/models/parameterDetails.ts b/src/models/parameterDetails.ts
@@ -14,7 +14,7 @@ import { KnownMediaType } from "@azure-tools/codegen";
 export interface ParameterDetails {
   nameRef: string;
   defaultValue?: any;
-  description: string;
+  description?: string;
   name: string;
   serializedName: string;
   location: ParameterLocation;
@@ -23,7 +23,7 @@ export interface ParameterDetails {
   mapper: string | Mapper;
   isGlobal: boolean;
   parameter: Parameter;
-  operationsIn?: string[];
+  operationsIn?: { [operationName: string]: { description: string } };
   collectionFormat?: string;
   schemaType: AllSchemaTypes;
   implementationLocation?: ImplementationLocation;

diff --git a/src/transforms/extensions.ts b/src/transforms/extensions.ts
@@ -7,7 +7,8 @@ import {
   Parameter,
   StringSchema,
   Protocol,
-  ParameterLocation
+  ParameterLocation,
+  ImplementationLocation
 } from "@azure-tools/codemodel";
 import { cloneOperation } from "../utils/cloneOperation";
 import { extractPaginationDetails } from "../utils/extractPaginationDetails";
@@ -157,10 +158,17 @@ function addPageableMethods(codeModel: CodeModel) {
           },
           protocol: {
             http: httpProtocol
-          }
+          },
+          implementation: ImplementationLocation.Method
         }
       );
-      nextLinkMethod.requests?.[0].addParameter(nextLinkParameter);
+
+      // Ensure all overloads support the nextLink parameter.
+      for (const request of nextLinkMethod.requests ?? []) {
+        const parameters = request.parameters ?? [];
+        parameters.push(nextLinkParameter);
+        request.parameters = parameters;
+      }
 
       operationGroup.addOperation(nextLinkMethod);
     }

diff --git a/src/transforms/operationTransforms.ts b/src/transforms/operationTransforms.ts
@@ -456,7 +456,7 @@ function getGroupedParameters(
   const operationParams = parameters.filter(p => {
     // Ensure parameters are specific to the operation.
     const matchesOperation =
-      p.operationsIn && p.operationsIn.indexOf(operationFullname) > -1;
+      p.operationsIn && p.operationsIn[operationFullname];
     // Consider the media type as a match if none was provided, or they actually match.
     // This is important when an operation supports multiple media types.
     const matchesMediaType =

diff --git a/src/transforms/parameterTransforms.ts b/src/transforms/parameterTransforms.ts
@@ -216,17 +216,25 @@ export function populateOperationParameters(
 
   const sameNameParams = operationParameters.filter(p => p.name === name);
   description += getSchemaTypeDocumentation(parameter.schema);
+  const isRequired = getParameterRequired(parameter);
 
   const collectionFormat = getCollectionFormat(parameter);
   const typeDetails = getTypeForSchema(parameter.schema);
   const paramDetails: ParameterDetails = {
     nameRef: name,
-    description,
+    description:
+      isRequired && parameter.implementation === ImplementationLocation.Method
+        ? undefined
+        : description,
     name,
     serializedName,
-    operationsIn: [operationName],
+    operationsIn: {
+      [operationName]: {
+        description
+      }
+    },
     location: getParameterLocation(parameter),
-    required: getParameterRequired(parameter),
+    required: isRequired,
     schemaType: parameter.schema.type,
     parameterPath: getParameterPath(parameter),
     mapper: getMapperOrRef(
@@ -255,7 +263,8 @@ export function populateOperationParameters(
     paramDetails,
     operationParameters,
     sameNameParams,
-    operationName
+    operationName,
+    description
   );
 }
 
@@ -388,7 +397,6 @@ function getParameterName(parameter: Parameter) {
  */
 function getComparableParameter({
   name,
-  description,
   serializedName,
   location,
   required,
@@ -404,7 +412,6 @@ function getComparableParameter({
 }: ParameterDetails) {
   return {
     name,
-    description,
     serializedName,
     location,
     required,
@@ -434,7 +441,8 @@ export function disambiguateParameter(
   parameter: ParameterDetails,
   operationParameters: ParameterDetails[],
   sameNameParams: ParameterDetails[],
-  operationName: string
+  operationName: string,
+  description: string
 ) {
   const param = getComparableParameter(parameter);
   const existingParam = sameNameParams.find(p =>
@@ -443,9 +451,9 @@ export function disambiguateParameter(
 
   if (existingParam) {
     if (existingParam.operationsIn) {
-      existingParam.operationsIn.push(operationName);
+      existingParam.operationsIn[operationName] = { description };
     } else {
-      existingParam.operationsIn = [operationName];
+      existingParam.operationsIn = { [operationName]: { description } };
     }
     return;
   } else {

diff --git a/src/utils/getParameterDescription.ts b/src/utils/getParameterDescription.ts
@@ -0,0 +1,15 @@
+import { ParameterDetails } from "../models/parameterDetails";
+
+export function getParameterDescription(
+  parameter: ParameterDetails,
+  operationName?: string
+): string {
+  let description: string | undefined;
+  if (operationName) {
+    description = parameter.operationsIn?.[operationName].description;
+  }
+  if (!description) {
+    description = parameter.description ?? "";
+  }
+  return description;
+}
diff --git a/test/integration/generated/azureSpecialProperties/src/models/parameters.ts b/test/integration/generated/azureSpecialProperties/src/models/parameters.ts
@@ -66,39 +66,6 @@ export const subscriptionId1: coreHttp.OperationURLParameter = {
   }
 };
 
-export const subscriptionId2: coreHttp.OperationURLParameter = {
-  parameterPath: "subscriptionId",
-  mapper: {
-    serializedName: "subscriptionId",
-    required: true,
-    type: {
-      name: "String"
-    }
-  }
-};
-
-export const subscriptionId3: coreHttp.OperationURLParameter = {
-  parameterPath: "subscriptionId",
-  mapper: {
-    serializedName: "subscriptionId",
-    required: true,
-    type: {
-      name: "String"
-    }
-  }
-};
-
-export const subscriptionId4: coreHttp.OperationURLParameter = {
-  parameterPath: "subscriptionId",
-  mapper: {
-    serializedName: "subscriptionId",
-    required: true,
-    type: {
-      name: "String"
-    }
-  }
-};
-
 export const apiVersion1: coreHttp.OperationQueryParameter = {
   parameterPath: "apiVersion",
   mapper: {
@@ -121,18 +88,6 @@ export const apiVersion2: coreHttp.OperationQueryParameter = {
   }
 };
 
-export const apiVersion3: coreHttp.OperationQueryParameter = {
-  parameterPath: "apiVersion",
-  mapper: {
-    defaultValue: "2.0",
-    isConstant: true,
-    serializedName: "api-version",
-    type: {
-      name: "String"
-    }
-  }
-};
-
 export const unencodedPathParam: coreHttp.OperationURLParameter = {
   parameterPath: "unencodedPathParam",
   mapper: {

diff --git a/test/integration/generated/azureSpecialProperties/src/operations/apiVersionLocal.ts b/test/integration/generated/azureSpecialProperties/src/operations/apiVersionLocal.ts
@@ -142,7 +142,7 @@ const getSwaggerLocalValidOperationSpec: coreHttp.OperationSpec = {
       bodyMapper: Mappers.ErrorModel
     }
   },
-  queryParameters: [Parameters.apiVersion3],
+  queryParameters: [Parameters.apiVersion1],
   urlParameters: [Parameters.$host],
   serializer
 };
diff --git a/test/integration/generated/azureSpecialProperties/src/operations/subscriptionInMethod.ts b/test/integration/generated/azureSpecialProperties/src/operations/subscriptionInMethod.ts
@@ -130,7 +130,7 @@ const postMethodLocalNullOperationSpec: coreHttp.OperationSpec = {
       bodyMapper: Mappers.ErrorModel
     }
   },
-  urlParameters: [Parameters.$host, Parameters.subscriptionId2],
+  urlParameters: [Parameters.$host, Parameters.subscriptionId1],
   serializer
 };
 const postPathLocalValidOperationSpec: coreHttp.OperationSpec = {
@@ -143,7 +143,7 @@ const postPathLocalValidOperationSpec: coreHttp.OperationSpec = {
       bodyMapper: Mappers.ErrorModel
     }
   },
-  urlParameters: [Parameters.$host, Parameters.subscriptionId3],
+  urlParameters: [Parameters.$host, Parameters.subscriptionId1],
   serializer
 };
 const postSwaggerLocalValidOperationSpec: coreHttp.OperationSpec = {
@@ -156,6 +156,6 @@ const postSwaggerLocalValidOperationSpec: coreHttp.OperationSpec = {
       bodyMapper: Mappers.ErrorModel
     }
   },
-  urlParameters: [Parameters.$host, Parameters.subscriptionId4],
+  urlParameters: [Parameters.$host, Parameters.subscriptionId1],
   serializer
 };