Fix: Don't automatically include Content-Type parameter if it is already defined in the spec as a parameter (#3916)

common/changes/@autorest/modelerfour/feature-content-type_2021-02-23-17-38.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@autorest/modelerfour",
+      "comment": "**Fix** Don't add a duplicate Content-Type parameter if it is already provided in the spec",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@autorest/modelerfour",
+  "email": "tiguerin@microsoft.com"
+}

packages/extensions/modelerfour/jest.config.js

@@ -4,7 +4,7 @@ const defaultConfig = require("../../../jest.default.config");
 
 const config = {
   ...defaultConfig,
-  testMatch: ["<rootDir>/test/**/*.test.ts"],
+  testMatch: ["<rootDir>/src/**/*.test.ts", "<rootDir>/test/**/*.test.ts"],
   setupFilesAfterEnv: ["<rootDir>/test/setupJest.ts"],
 };
 

packages/extensions/modelerfour/src/modeler/modelerfour.ts

@@ -76,6 +76,7 @@ import { Session, Channel } from "@autorest/extension-base";
 import { Interpretations, XMSEnum } from "./interpretations";
 import { fail, minimum, pascalCase, knownMediaType, KnownMediaType } from "@azure-tools/codegen";
 import { ModelerFourOptions } from "./modelerfour-options";
+import { isContentTypeParameterDefined } from "./utils";
 
 /** adds only if the item is not in the collection already
  *
@@ -1367,9 +1368,11 @@ export class ModelerFour {
         http,
       },
     });
-    this.session.log(`Options ${JSON.stringify(this.options)}`, {});
-    this.session.log(`Accept-param ${this.options["always-create-accept-parameter"]}`, {});
-    if (this.options[`always-create-content-type-parameter`] === true || http.mediaTypes.length > 1) {
+
+    const shouldIncludeContentType =
+      this.options[`always-create-content-type-parameter`] === true || http.mediaTypes.length > 1;
+
+    if (!isContentTypeParameterDefined(operation) && shouldIncludeContentType) {
       const scs = this.getContentTypeParameterSchema(http);
 
       // add the parameter for the binary upload.
@@ -1446,7 +1449,7 @@ export class ModelerFour {
       },
     });
 
-    if (this.options[`always-create-content-type-parameter`] === true) {
+    if (!isContentTypeParameterDefined(operation) && this.options[`always-create-content-type-parameter`] === true) {
       const scs = this.getContentTypeParameterSchema(http, true);
 
       // add the parameter for the binary upload.

packages/extensions/modelerfour/src/modeler/utils.test.ts

@@ -0,0 +1,61 @@
+import { Operation, Parameter, Schema, SchemaType } from "@autorest/codemodel";
+import { isContentTypeParameterDefined } from "./utils";
+
+const stringSchema = new Schema("string", "A String", SchemaType.Any);
+
+describe("Modelerfour utils", () => {
+  describe("isContentTypeParameterDefined()", () => {
+    it("returns false if there is a no parameter in the operation", () => {
+      const operation = new Operation("op-1", "Test operation 1", {});
+      expect(isContentTypeParameterDefined(operation)).toBe(false);
+    });
+
+    it("returns false if there is a no parameter with the content-type name in the operation", () => {
+      const operation = new Operation("op-1", "Test operation 1", {
+        parameters: [
+          new Parameter("OtherParm", "OtherParam header", stringSchema, {
+            language: { default: { serializedName: "Other-Param" } },
+            protocol: { http: { in: "header" } },
+          }),
+        ],
+      });
+      expect(isContentTypeParameterDefined(operation)).toBe(false);
+    });
+
+    it("returns false if there is a a parameter with the content-type name but is not a header", () => {
+      const operation = new Operation("op-1", "Test operation 1", {
+        parameters: [
+          new Parameter("ContentTypeQuery", "Content type query", stringSchema, {
+            language: { default: { serializedName: "Content-Type" } },
+            protocol: { http: { in: "query" } },
+          }),
+        ],
+      });
+      expect(isContentTypeParameterDefined(operation)).toBe(false);
+    });
+
+    it("returns true if there is a parameter with the serialized Content-Type", () => {
+      const operation = new Operation("op-1", "Test operation 1", {
+        parameters: [
+          new Parameter("ContentType", "Content type header", stringSchema, {
+            language: { default: { serializedName: "Content-Type" } },
+            protocol: { http: { in: "header" } },
+          }),
+        ],
+      });
+      expect(isContentTypeParameterDefined(operation)).toBe(true);
+    });
+
+    it("returns true if there is a parameter with the serialized Content-Type but different casing", () => {
+      const operation = new Operation("op-1", "Test operation 1", {
+        parameters: [
+          new Parameter("ContentType", "Content type header", stringSchema, {
+            language: { default: { serializedName: "conTEnt-TyPe" } },
+            protocol: { http: { in: "header" } },
+          }),
+        ],
+      });
+      expect(isContentTypeParameterDefined(operation)).toBe(true);
+    });
+  });
+});

packages/extensions/modelerfour/src/modeler/utils.ts

@@ -0,0 +1,20 @@
+import { Operation, Parameter } from "@autorest/codemodel";
+
+/**
+ * Figure out if the provided operation already define a Content-Type parameter.
+ * @param operation Operation.
+ */
+export function isContentTypeParameterDefined(operation: Operation): boolean {
+  return operation.parameters?.find(isParameterContentTypeHeader) !== undefined;
+}
+
+function isParameterContentTypeHeader(parameter: Parameter): boolean {
+  const serializedName = parameter.language.default.serializedName;
+  if (!serializedName || typeof serializedName !== "string") {
+    return false;
+  }
+  if (parameter.protocol.http?.in !== "header") {
+    return false;
+  }
+  return serializedName?.toLowerCase() === "content-type";
+}

packages/extensions/modelerfour/test/scenarios/expected/body-binary-json/modeler.yaml

@@ -11,8 +11,19 @@ schemas: !Schemas
           name: string
           description: simple string
       protocol: !Protocols {}
+  constants:
+    - !ConstantSchema &ref_1
+      type: constant
+      value: !ConstantValue 
+        value: application/json
+      valueType: *ref_0
+      language: !Languages 
+        default:
+          name: application/json
+          description: Content Type 'application/json'
+      protocol: !Protocols {}
   binaries:
-    - !BinarySchema &ref_1
+    - !BinarySchema &ref_2
       type: binary
       language: !Languages 
         default:
@@ -20,7 +31,7 @@ schemas: !Schemas
           description: ''
       protocol: !Protocols {}
 globalParameters:
-  - !Parameter &ref_3
+  - !Parameter &ref_4
     schema: *ref_0
     clientDefaultValue: ''
     implementation: Client
@@ -45,13 +56,26 @@ operationGroups:
           - !ApiVersion 
             version: 1.0.0
         parameters:
-          - *ref_3
+          - *ref_4
         requests:
           - !Request 
             parameters:
-              - !Parameter &ref_2
+              - !Parameter 
                 schema: *ref_1
                 implementation: Method
+                origin: modelerfour:synthesized/content-type
+                required: true
+                language: !Languages 
+                  default:
+                    name: content-type
+                    description: Upload file type
+                    serializedName: Content-Type
+                protocol: !Protocols 
+                  http: !HttpParameter 
+                    in: header
+              - !Parameter &ref_3
+                schema: *ref_2
+                implementation: Method
                 required: true
                 language: !Languages 
                   default:
@@ -62,7 +86,7 @@ operationGroups:
                     in: body
                     style: binary
             signatureParameters:
-              - *ref_2
+              - *ref_3
             language: !Languages 
               default:
                 name: ''

packages/extensions/modelerfour/test/scenarios/expected/custom-content-type-parm/modeler.yaml

@@ -0,0 +1,138 @@
+!CodeModel 
+info: !Info 
+  description: Acceptance test for file with custom content type.
+  title: Operation where there is a custom Content-Type parameter.
+schemas: !Schemas 
+  booleans:
+    - !BooleanSchema &ref_5
+      type: boolean
+      language: !Languages 
+        default:
+          name: response
+          description: ''
+      protocol: !Protocols {}
+  strings:
+    - !StringSchema &ref_0
+      type: string
+      language: !Languages 
+        default:
+          name: string
+          description: simple string
+      protocol: !Protocols {}
+    - !StringSchema &ref_1
+      type: string
+      language: !Languages 
+        default:
+          name: ''
+          description: ''
+      protocol: !Protocols {}
+  binaries:
+    - !BinarySchema &ref_3
+      type: binary
+      language: !Languages 
+        default:
+          name: binary
+          description: ''
+      protocol: !Protocols {}
+globalParameters:
+  - !Parameter &ref_6
+    schema: *ref_0
+    clientDefaultValue: ''
+    implementation: Client
+    origin: modelerfour:synthesized/host
+    required: true
+    extensions:
+      x-ms-skip-url-encoding: true
+    language: !Languages 
+      default:
+        name: $host
+        description: server parameter
+        serializedName: $host
+    protocol: !Protocols 
+      http: !HttpParameter 
+        in: uri
+operationGroups:
+  - !OperationGroup 
+    $key: ''
+    operations:
+      - !Operation 
+        apiVersions:
+          - !ApiVersion 
+            version: 1.0.0
+        parameters:
+          - *ref_6
+          - !Parameter &ref_2
+            schema: *ref_1
+            implementation: Method
+            required: true
+            language: !Languages 
+              default:
+                name: multipartContentType
+                description: 'Required. The value of this header must be multipart/mixed with a batch boundary. Example header value: multipart/mixed; boundary=batch_<GUID>'
+                serializedName: Content-Type
+            protocol: !Protocols 
+              http: !HttpParameter 
+                in: header
+        requests:
+          - !Request 
+            parameters:
+              - !Parameter &ref_4
+                schema: *ref_3
+                implementation: Method
+                language: !Languages 
+                  default:
+                    name: data
+                    description: ''
+                protocol: !Protocols 
+                  http: !HttpParameter 
+                    in: body
+                    style: binary
+            signatureParameters:
+              - *ref_4
+            language: !Languages 
+              default:
+                name: ''
+                description: ''
+            protocol: !Protocols 
+              http: !HttpBinaryRequest 
+                path: /api/User/HasUser
+                method: post
+                binary: true
+                knownMediaType: binary
+                mediaTypes:
+                  - application/json
+                uri: '{$host}'
+        signatureParameters:
+          - *ref_2
+        responses:
+          - !SchemaResponse 
+            schema: *ref_5
+            language: !Languages 
+              default:
+                name: ''
+                description: OK
+            protocol: !Protocols 
+              http: !HttpResponse 
+                knownMediaType: json
+                mediaTypes:
+                  - application/json
+                statusCodes:
+                  - '200'
+        language: !Languages 
+          default:
+            name: postWithCustomContentType
+            description: ''
+        protocol: !Protocols {}
+    language: !Languages 
+      default:
+        name: ''
+        description: ''
+    protocol: !Protocols {}
+security: !Security 
+  authenticationRequired: false
+language: !Languages 
+  default:
+    name: ''
+    description: ''
+protocol: !Protocols 
+  http: !HttpModel {}

packages/extensions/modelerfour/test/scenarios/inputs/custom-content-type-parm/openapi-document.json

@@ -0,0 +1,54 @@
+{
+  "openapi": "3.0.0",
+  "$schema": "https://raw.githubusercontent.com/OAI/OpenAPI-Specification/master/schemas/v3.0/schema.json",
+  "info": {
+    "title": "Operation where there is a custom Content-Type parameter.",
+    "description": "Acceptance test for file with custom content type.",
+    "version": "1.0.0"
+  },
+  "paths": {
+    "/api/User/HasUser": {
+      "x-ms-metadata": {
+        "apiVersions": ["1.0.0"]
+      },
+      "post": {
+        "operationId": "postWithCustomContentType",
+        "requestBody": {
+          "content": {
+            "application/json": {
+              "schema": {
+                "type": "object",
+                "format": "file"
+              }
+            }
+          }
+        },
+        "parameters": [
+          {
+            "name": "Content-Type",
+            "x-ms-client-name": "multipartContentType",
+            "in": "header",
+            "required": true,
+            "schema": {
+              "type": "string"
+            },
+            "x-ms-parameter-location": "method",
+            "description": "Required. The value of this header must be multipart/mixed with a batch boundary. Example header value: multipart/mixed; boundary=batch_<GUID>"
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "OK",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "type": "boolean"
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}

packages/extensions/modelerfour/test/scenarios/scenarios.test.ts

@@ -12,7 +12,7 @@ const cfg = {
     "group-parameters": true,
     "resolve-schema-name-collisons": true,
     "additional-checks": true,
-    //'always-create-content-type-parameter': true,
+    "always-create-content-type-parameter": true,
     "naming": {
       override: {
         $host: "$host",
@@ -22,7 +22,7 @@ const cfg = {
       constantParameter: "pascal",
       /*
         for when playing with python style settings :
-        
+
         parameter: 'snakecase',
         property: 'snakecase',
         operation: 'snakecase',