Merge pull request Azure#647 from chradek/operation-spec-content-type

 [v6] add mediaType to OperationSpecs and make param ordering deterministic

Author: chradek

package-lock.json

@@ -33,7 +33,7 @@
         "@azure-tools/codemodel": "4.13.317",
         "@azure-tools/linq": "3.1.206",
         "@azure-tools/openapi": "3.0.209",
-        "@azure/core-http": "^1.1.1",
+        "@azure/core-http": "1.1.3-dev.20200520.1",
         "@azure/core-lro": "^1.0.1",
         "@azure/core-paging": "^1.0.0",
         "@azure/logger": "^1.0.0",

package.json

@@ -36,7 +36,8 @@ import {
   ChoiceSchema,
   SealedChoiceSchema,
   ConstantSchema,
-  Parameter
+  Parameter,
+  ImplementationLocation
 } from "@azure-tools/codemodel";
 import { getLanguageMetadata } from "../utils/languageHelpers";
 import { shouldImportParameters } from "./utils/importUtils";
@@ -146,16 +147,25 @@ function buildSpec(spec: OperationSpecDetails): string {
   const urlParams = buildParameters(spec, "urlParameters");
   const headerParams = buildParameters(spec, "headerParameters");
   const contentType = buildContentType(spec);
+  const mediaType = buildMediaType(spec);
+
   const isXML = spec.isXML ? "isXML: true," : "";
 
   return `{ path: "${spec.path}", httpMethod: "${
     spec.httpMethod
   }", responses: {${responses.join(
     ", "
-  )}},${requestBody}${queryParams}${urlParams}${headerParams}${isXML}${contentType}serializer
+  )}},${requestBody}${queryParams}${urlParams}${headerParams}${isXML}${contentType}${mediaType}serializer
     }`;
 }
 
+function buildMediaType({ requestBody }: OperationSpecDetails) {
+  if (requestBody?.targetMediaType) {
+    return `mediaType: '${requestBody.targetMediaType}',`;
+  }
+  return "";
+}
+
 function buildContentType({ requestBody, isXML }: OperationSpecDetails) {
   return requestBody && isXML
     ? "contentType: 'application/xml; charset=utf-8',"
@@ -415,6 +425,50 @@ type ParameterWithDescription = OptionalKind<
   }
 >;
 
+/**
+ * Sorts the list of operation parameters to match the order described by the CodeModel.
+ * @param operation Details about an operation.
+ * @param request Details about an operation overload.
+ * @param parameterDeclarations List of required parameter declarations for the provided operation overload.
+ */
+function sortOperationParameters(
+  operation: OperationDetails,
+  request: OperationRequestDetails,
+  parameterDeclarations: ParameterWithDescription[]
+): ParameterWithDescription[] {
+  // Get a sorted list of parameter names for this operation/request.
+  // Note that this may inlcude parameters that aren't displayed, e.g. constant types.
+  const expectedParameterOrdering = [
+    ...operation.parameters,
+    ...(request.parameters ?? [])
+  ]
+    // Only parameters that are implemented on the method should be considered.
+    .filter(param => param.implementation === ImplementationLocation.Method)
+    .map(param => getLanguageMetadata(param.language).name);
+
+  const orderedParameterDeclarations: typeof parameterDeclarations = [];
+  for (const parameterName of expectedParameterOrdering) {
+    const index = parameterDeclarations.findIndex(
+      p => p.name === parameterName
+    );
+    if (index === -1) {
+      // No matching parameter found.
+      // Common cases where this occurs is if a parameter
+      // is optional, or a constant.
+      continue;
+    }
+
+    orderedParameterDeclarations.push(
+      ...parameterDeclarations.splice(index, 1)
+    );
+  }
+
+  // push any remaining parameters into the ordered parameter list
+  orderedParameterDeclarations.push(...parameterDeclarations);
+
+  return orderedParameterDeclarations;
+}
+
 /**
  * Gets a list of parameter declarations for each overload the operation supports,
  * and the list of parameter declarations for the base operation.
@@ -489,6 +543,13 @@ function getOperationParameterSignatures(
       parameterDeclarations
     );
 
+    // Sort the parameter declarations to match the signature the CodeModel suggests.
+    const orderedParameterDeclarations = sortOperationParameters(
+      operation,
+      request,
+      parameterDeclarations
+    );
+
     // add optional parameter
     const optionalParameter = getOptionsParameter(
       operation,
@@ -498,9 +559,9 @@ function getOperationParameterSignatures(
         mediaType: hasMultipleOverloads ? requestMediaType : undefined
       }
     );
-    parameterDeclarations.push(optionalParameter);
+    orderedParameterDeclarations.push(optionalParameter);
 
-    overloadParameterDeclarations.push(parameterDeclarations);
+    overloadParameterDeclarations.push(orderedParameterDeclarations);
   }
 
   // Create the parameter declarations for the base method signature.

src/generators/operationGenerator.ts

@@ -22,7 +22,7 @@ export function generatePackageJson(
     version: packageDetails.version,
     dependencies: {
       ...(hasLRO && { "@azure/core-lro": "^1.0.1" }),
-      "@azure/core-http": "^1.1.1",
+      "@azure/core-http": "1.1.3-dev.20200520.1",
       tslib: "^1.9.3"
     },
     keywords: ["node", "azure", "typescript", "browser", "isomorphic"],

src/generators/static/packageFileGenerator.ts

@@ -56,6 +56,10 @@ export interface OperationDetails {
   fullName: string;
   description: string;
   apiVersions: string[];
+  /**
+   * Operation parameters that are shared across requests.
+   */
+  parameters: Parameter[];
   requests: OperationRequestDetails[];
   responses: OperationResponseDetails[];
   typeDetails: TypeDetails;

src/models/operationDetails.ts

@@ -373,6 +373,7 @@ export async function transformOperation(
       ? operation.apiVersions.map(v => v.version)
       : [],
     description: metadata.description,
+    parameters: operation.parameters || [],
     requests,
     responses,
     mediaTypes,

src/transforms/operationTransforms.ts

@@ -138,16 +138,13 @@ const extractOperationParameters = (codeModel: CodeModel) =>
           // Operations may have multiple requests, each with their own set of parameters.
           // This is known to be the case when an operation can consume multiple media types.
           // We need to ensure that the parameters from each request (method overload) is accounted for.
-          const recordMediaType = requests.length > 1;
           const requestParams: OperationParameterDetails[] = [];
           requests.forEach(request => {
             request.parameters?.forEach(parameter => {
               requestParams.push({
                 operationName,
                 parameter,
-                targetMediaType: recordMediaType
-                  ? request.protocol.http?.knownMediaType
-                  : undefined
+                targetMediaType: request.protocol.http?.knownMediaType
               });
             });
           });

src/transforms/parameterTransforms.ts

@@ -3,7 +3,10 @@
   "author": "Microsoft Corporation",
   "description": "Test Infrastructure for AutoRest",
   "version": "1.0.0-preview1",
-  "dependencies": { "@azure/core-http": "^1.1.1", "tslib": "^1.9.3" },
+  "dependencies": {
+    "@azure/core-http": "1.1.3-dev.20200520.1",
+    "tslib": "^1.9.3"
+  },
   "keywords": ["node", "azure", "typescript", "browser", "isomorphic"],
   "license": "MIT",
   "main": "./dist/additional-properties.js",

test/integration/generated/additionalProperties/package.json

@@ -165,6 +165,7 @@ const createAPTrueOperationSpec: coreHttp.OperationSpec = {
   requestBody: Parameters.createParameters,
   urlParameters: [Parameters.$host],
   headerParameters: [Parameters.contentType],
+  mediaType: "json",
   serializer
 };
 const createCatAPTrueOperationSpec: coreHttp.OperationSpec = {
@@ -181,6 +182,7 @@ const createCatAPTrueOperationSpec: coreHttp.OperationSpec = {
   requestBody: Parameters.createParameters1,
   urlParameters: [Parameters.$host],
   headerParameters: [Parameters.contentType],
+  mediaType: "json",
   serializer
 };
 const createAPObjectOperationSpec: coreHttp.OperationSpec = {
@@ -197,6 +199,7 @@ const createAPObjectOperationSpec: coreHttp.OperationSpec = {
   requestBody: Parameters.createParameters2,
   urlParameters: [Parameters.$host],
   headerParameters: [Parameters.contentType],
+  mediaType: "json",
   serializer
 };
 const createAPStringOperationSpec: coreHttp.OperationSpec = {
@@ -213,6 +216,7 @@ const createAPStringOperationSpec: coreHttp.OperationSpec = {
   requestBody: Parameters.createParameters3,
   urlParameters: [Parameters.$host],
   headerParameters: [Parameters.contentType],
+  mediaType: "json",
   serializer
 };
 const createAPInPropertiesOperationSpec: coreHttp.OperationSpec = {
@@ -229,6 +233,7 @@ const createAPInPropertiesOperationSpec: coreHttp.OperationSpec = {
   requestBody: Parameters.createParameters4,
   urlParameters: [Parameters.$host],
   headerParameters: [Parameters.contentType],
+  mediaType: "json",
   serializer
 };
 const createAPInPropertiesWithAPStringOperationSpec: coreHttp.OperationSpec = {
@@ -245,5 +250,6 @@ const createAPInPropertiesWithAPStringOperationSpec: coreHttp.OperationSpec = {
   requestBody: Parameters.createParameters5,
   urlParameters: [Parameters.$host],
   headerParameters: [Parameters.contentType],
+  mediaType: "json",
   serializer
 };

test/integration/generated/additionalProperties/src/operations/pets.ts

@@ -3,7 +3,10 @@
   "author": "Microsoft Corporation",
   "description": "Test Infrastructure for AutoRest",
   "version": "1.0.0-preview1",
-  "dependencies": { "@azure/core-http": "^1.1.1", "tslib": "^1.9.3" },
+  "dependencies": {
+    "@azure/core-http": "1.1.3-dev.20200520.1",
+    "tslib": "^1.9.3"
+  },
   "keywords": ["node", "azure", "typescript", "browser", "isomorphic"],
   "license": "MIT",
   "main": "./dist/azure-parameter-grouping.js",

test/integration/generated/azureParameterGrouping/package.json

@@ -114,6 +114,7 @@ const postRequiredOperationSpec: coreHttp.OperationSpec = {
   queryParameters: [Parameters.query],
   urlParameters: [Parameters.$host, Parameters.path],
   headerParameters: [Parameters.contentType, Parameters.customHeader],
+  mediaType: "json",
   serializer
 };
 const postOptionalOperationSpec: coreHttp.OperationSpec = {

test/integration/generated/azureParameterGrouping/src/operations/parameterGrouping.ts

@@ -3,7 +3,10 @@
   "author": "Microsoft Corporation",
   "description": "Test Infrastructure for AutoRest",
   "version": "1.0.0-preview1",
-  "dependencies": { "@azure/core-http": "^1.1.1", "tslib": "^1.9.3" },
+  "dependencies": {
+    "@azure/core-http": "1.1.3-dev.20200520.1",
+    "tslib": "^1.9.3"
+  },
   "keywords": ["node", "azure", "typescript", "browser", "isomorphic"],
   "license": "MIT",
   "main": "./dist/zzzAzureReport.js",

test/integration/generated/azureReport/package.json

@@ -3,7 +3,10 @@
   "author": "Microsoft Corporation",
   "description": "Test Infrastructure for AutoRest",
   "version": "1.0.0-preview1",
-  "dependencies": { "@azure/core-http": "^1.1.1", "tslib": "^1.9.3" },
+  "dependencies": {
+    "@azure/core-http": "1.1.3-dev.20200520.1",
+    "tslib": "^1.9.3"
+  },
   "keywords": ["node", "azure", "typescript", "browser", "isomorphic"],
   "license": "MIT",
   "main": "./dist/azure-special-properties.js",

test/integration/generated/azureSpecialProperties/package.json

@@ -3,7 +3,10 @@
   "author": "Microsoft Corporation",
   "description": "Test Infrastructure for AutoRest Swagger BAT",
   "version": "1.0.0-preview1",
-  "dependencies": { "@azure/core-http": "^1.1.1", "tslib": "^1.9.3" },
+  "dependencies": {
+    "@azure/core-http": "1.1.3-dev.20200520.1",
+    "tslib": "^1.9.3"
+  },
   "keywords": ["node", "azure", "typescript", "browser", "isomorphic"],
   "license": "MIT",
   "main": "./dist/body-array.js",