fix: various quirks with how we handle accept headers (#699)

erunion · web-flow · commit e60c3717a9de · 2023-08-18T12:55:49.000-07:00
* fix(api): quirk where headers could be duped in body payloads

* fix(api): stop sending special headers in body payloads

* fix(snippet): stop putting `accept` headers in snippets if we can
diff --git a/packages/api/src/core/prepareParams.ts b/packages/api/src/core/prepareParams.ts
@@ -332,6 +332,7 @@ export default async function prepareParams(operation: Operation, body?: unknown
       if (typeof metadata === 'object' && !isEmpty(metadata)) {
         if (paramName in metadata) {
           value = metadata[paramName];
+          metadataHeaderParam = paramName;
         } else if (param.in === 'header') {
           // Headers are sent case-insensitive so we need to make sure that we're properly
           // matching them when detecting what our incoming payload looks like.
@@ -379,9 +380,7 @@ export default async function prepareParams(operation: Operation, body?: unknown
     // If there's any leftover metadata that hasn't been moved into form data for this request we
     // need to move it or else it'll get tossed.
     if (!isEmpty(metadata)) {
-      if (operation.isFormUrlEncoded()) {
-        params.formData = merge(params.formData, metadata);
-      } else if (typeof metadata === 'object') {
+      if (typeof metadata === 'object') {
         // If the user supplied an `accept` or `authorization` header themselves we should allow it
         // through. Normally these headers are automatically handled by `@readme/oas-to-har` but in
         // the event that maybe the user wants to return XML for an API that normally returns JSON
@@ -391,8 +390,14 @@ export default async function prepareParams(operation: Operation, body?: unknown
           const headerParam = Object.keys(metadata).find(m => m.toLowerCase() === headerName);
           if (headerParam) {
             params.header[headerName] = metadata[headerParam] as string;
+            // eslint-disable-next-line no-param-reassign
+            delete metadata[headerParam];
           }
         });
+      }
+
+      if (operation.isFormUrlEncoded()) {
+        params.formData = merge(params.formData, metadata);
       } else {
         // Any other remaining unused metadata will be unused because we don't know where to place
         // it in the request.
diff --git a/packages/api/test/__fixtures__/definitions/basiq.json b/packages/api/test/__fixtures__/definitions/basiq.json
@@ -0,0 +1,104 @@
+{
+  "openapi": "3.0.1",
+  "info": {
+    "title": "Core",
+    "description": "All included utility endpoints for Basiq partners",
+    "version": "3.0.0"
+  },
+  "servers": [
+    {
+      "url": "https://au-api.basiq.io/"
+    }
+  ],
+  "paths": {
+    "/token": {
+      "post": {
+        "tags": ["Authentication"],
+        "summary": "Generate an auth token",
+        "description": "Use this endpoint to retrieve a token that will be passed as authorization header for Basiq API",
+        "operationId": "postToken",
+        "parameters": [
+          {
+            "name": "basiq-version",
+            "in": "header",
+            "required": true,
+            "schema": {
+              "type": "string",
+              "example": "3.0"
+            }
+          }
+        ],
+        "requestBody": {
+          "content": {
+            "application/x-www-form-urlencoded": {
+              "schema": {
+                "properties": {
+                  "scope": {
+                    "type": "string"
+                  },
+                  "userId": {
+                    "type": "string"
+                  }
+                }
+              },
+              "examples": {
+                "client_access": {
+                  "summary": "For all client side requests",
+                  "value": {
+                    "scope": "CLIENT_ACCESS",
+                    "userId": "6dd30ce4-d4ba-11ec-9d64-0242ac120002"
+                  }
+                },
+                "server_access": {
+                  "summary": "For all server side requests",
+                  "value": {
+                    "scope": "SERVER_ACCESS"
+                  }
+                }
+              }
+            }
+          }
+        },
+        "responses": {
+          "200": {
+            "description": "OK"
+          }
+        },
+        "security": [
+          {
+            "api_key": []
+          }
+        ]
+      }
+    }
+  },
+  "components": {
+    "securitySchemes": {
+      "api_key": {
+        "type": "apiKey",
+        "name": "Authorization",
+        "in": "header",
+        "x-default": "Basic NjMxMjNmMWMtZjYxMy00ZjMyLWFiYzUtYzBhZDdhYTY2YmU1OjQ3NWYwMzhkLTBlZmItNGM1ZS1iMzQ0LTAzMzYxOTkyYTRlMw=="
+      },
+      "services_token": {
+        "type": "http",
+        "scheme": "bearer",
+        "bearerFormat": "JWT"
+      }
+    }
+  },
+  "security": [
+    {
+      "api_key": []
+    },
+    {
+      "services_token": []
+    }
+  ],
+  "x-readme": {
+    "explorer-enabled": true,
+    "proxy-enabled": true,
+    "samples-enabled": true,
+    "samples-languages": ["curl", "node", "ruby", "javascript", "python"]
+  }
+}
diff --git a/packages/api/test/core/prepareParams.test.ts b/packages/api/test/core/prepareParams.test.ts
@@ -379,6 +379,41 @@ describe('#prepareParams', () => {
       });
     });
 
+    describe('quirks', () => {
+      it('should not send special headers in body payloads', async () => {
+        const basiq = await import('../__fixtures__/definitions/basiq.json').then(Oas.init);
+        await basiq.dereference();
+
+        const operation = basiq.operation('/token', 'post');
+
+        await expect(
+          prepareParams(operation, { scope: 'SERVER_ACCESS' }, { accept: 'application/json', 'BASIQ-version': '3.0' }),
+        ).resolves.toStrictEqual({
+          formData: {
+            scope: 'SERVER_ACCESS',
+          },
+          header: {
+            accept: 'application/json',
+            'basiq-version': '3.0',
+          },
+        });
+      });
+
+      it('should not duplicate a supplied header parameter if that header casing matches the spec', async () => {
+        const basiq = await import('../__fixtures__/definitions/basiq.json').then(Oas.init);
+        await basiq.dereference();
+
+        const operation = basiq.operation('/token', 'post');
+
+        await expect(
+          prepareParams(operation, { scope: 'scope', userId: 'userid' }, { 'basiq-version': '3.0' }),
+        ).resolves.toStrictEqual({
+          formData: { scope: 'scope', userId: 'userid' },
+          header: { 'basiq-version': '3.0' },
+        });
+      });
+    });
+
     describe('`accept` header overrides', () => {
       it('should support supplying an `accept` header parameter', async () => {
         const operation = parameterStyle.operation('/anything/headers', 'get');
diff --git a/packages/httpsnippet-client-api/src/index.ts b/packages/httpsnippet-client-api/src/index.ts
@@ -5,9 +5,11 @@ import type { HttpMethods, OASDocument } from 'oas/dist/rmoas.types';
 
 import { CodeBuilder } from '@readme/httpsnippet/dist/helpers/code-builder';
 import contentType from 'content-type';
-import Oas from 'oas';
+import Oas, { utils } from 'oas';
 import stringifyObject from 'stringify-object';
 
+const { matchesMimeType } = utils;
+
 // This should really be an exported type in `oas`.
 type SecurityType = 'Basic' | 'Bearer' | 'Query' | 'Header' | 'Cookie' | 'OAuth2' | 'http' | 'apiKey';
 
@@ -224,10 +226,9 @@ const client: Client<APIOptions> = {
             return;
           }
         } else if (headerLower === 'accept') {
-          // If the `Accept` header here is not the default or first `Accept` header for the
-          // operations' request body then we should add it otherwise we can let the SDK handle it
-          // itself.
-          if (headers[header] === operation.getContentType()) {
+          // If the `Accept` header here is JSON-like header then we can remove it from the code
+          // snippet because `api` natively supports and prioritizes JSON over any other mime type.
+          if (matchesMimeType.json(headers[header] as string)) {
             delete headers[header];
             return;
           }
diff --git a/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/index.ts b/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/index.ts
@@ -81,7 +81,7 @@ const mock: SnippetMock = {
         cookie: 'bar-cookie=baz; foo-cookie=bar',
       },
       functionMatcher: (url, opts) => {
-        return opts.body === 'foo=bar&foo2=bar2&foo3=bar3&foo4=bar4&accept=application%2Fjson';
+        return opts.body === 'foo=bar&foo2=bar2&foo3=bar3&foo4=bar4';
       },
     },
     res: {
diff --git a/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/output.js b/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/output.js
@@ -10,8 +10,7 @@ sdk.postAnything({
   baz: 'abc',
   key: 'value',
   'bar-cookie': 'baz',
-  'foo-cookie': 'bar',
-  accept: 'application/json'
+  'foo-cookie': 'bar'
 })
   .then(({ data }) => console.log(data))
   .catch(err => console.error(err));
diff --git a/packages/httpsnippet-client-api/test/__datasets__/full/index.ts b/packages/httpsnippet-client-api/test/__datasets__/full/index.ts
@@ -69,7 +69,7 @@ const mock: SnippetMock = {
         cookie: 'bar-cookie=baz; foo-cookie=bar',
       },
       functionMatcher: (url, opts) => {
-        return opts.body === 'foo=bar&accept=application%2Fjson';
+        return opts.body === 'foo=bar';
       },
     },
     res: {
diff --git a/packages/httpsnippet-client-api/test/__datasets__/full/output.js b/packages/httpsnippet-client-api/test/__datasets__/full/output.js
@@ -5,8 +5,7 @@ sdk.postAnything({foo: 'bar'}, {
   baz: 'abc',
   key: 'value',
   'bar-cookie': 'baz',
-  'foo-cookie': 'bar',
-  accept: 'application/json'
+  'foo-cookie': 'bar'
 })
   .then(({ data }) => console.log(data))
   .catch(err => console.error(err));
