Header params are missing or duplicated (#1769)

Fixes #1768 1. Body depiction does not exclude header params (only path params); 2. Params depiction only depicts header params when `query` is enabled as input source.
RobinTail · May 15, 2024 · af02e67 · af02e67
1 parent 78ea9a8
commit af02e67
Show file tree

Hide file tree

Showing 7 changed files with 281 additions and 74 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,16 @@
 
 ## Version 19
 
+### v19.1.1
+
+- Fixed a bug on duplicated or missing request header parameters in the generated Documentation:
+  - The issue corresponds to the "Headers as input source" opt-in feature;
+  - When `query` was not listed in the input sources:
+    - Headers used to be missing in the documented request parameters.
+  - When `body` was listed along with `query` in the input sources:
+    - Headers used to be duplicated into the documented request body.
+  - The issue was found and reported by [@boarush](https://github.com/boarush).
+
 ### v19.1.0
 
 - Feature: customizable handling rules for your branded schemas in Documentation and Integration:

diff --git a/src/documentation-helpers.ts b/src/documentation-helpers.ts
@@ -3,6 +3,7 @@ import {
   ExamplesObject,
   MediaTypeObject,
   OAuthFlowObject,
+  ParameterLocation,
   ParameterObject,
   ReferenceObject,
   RequestBodyObject,
@@ -686,7 +687,7 @@ export const depictRequestParams = ({
   description = `${method.toUpperCase()} ${path} Parameter`,
 }: Omit<ReqResDepictHelperCommonProps, "mimeTypes"> & {
   inputSources: InputSource[];
-}): ParameterObject[] => {
+}) => {
   const { shape } = extractObjectSchema(
     schema,
     new DocumentationError({
@@ -704,39 +705,50 @@ export const depictRequestParams = ({
     areParamsEnabled && pathParams.includes(name);
   const isHeaderParam = (name: string) =>
     areHeadersEnabled && isCustomHeader(name);
-  return Object.keys(shape)
-    .filter((name) => isQueryEnabled || isPathParam(name))
-    .map((name) => {
-      const depicted = walkSchema(shape[name], {
-        rules: { ...brandHandling, ...depicters },
-        onEach,
-        onMissing,
-        ctx: {
-          isResponse: false,
-          serializer,
-          getRef,
-          makeRef,
-          path,
-          method,
-        },
-      });
-      const result =
-        composition === "components"
-          ? makeRef(makeCleanId(description, name), depicted)
-          : depicted;
-      return {
-        name,
-        in: isPathParam(name)
-          ? "path"
-          : isHeaderParam(name)
-            ? "header"
-            : "query",
-        required: !shape[name].isOptional(),
-        description: depicted.description || description,
-        schema: result,
-        examples: depictParamExamples(schema, name),
-      };
+
+  const parameters = Object.keys(shape)
+    .map<{ name: string; location?: ParameterLocation }>((name) => ({
+      name,
+      location: isPathParam(name)
+        ? "path"
+        : isHeaderParam(name)
+          ? "header"
+          : isQueryEnabled
+            ? "query"
+            : undefined,
+    }))
+    .filter(
+      (parameter): parameter is Required<typeof parameter> =>
+        parameter.location !== undefined,
+    );
+
+  return parameters.map<ParameterObject>(({ name, location }) => {
+    const depicted = walkSchema(shape[name], {
+      rules: { ...brandHandling, ...depicters },
+      onEach,
+      onMissing,
+      ctx: {
+        isResponse: false,
+        serializer,
+        getRef,
+        makeRef,
+        path,
+        method,
+      },
     });
+    const result =
+      composition === "components"
+        ? makeRef(makeCleanId(description, name), depicted)
+        : depicted;
+    return {
+      name,
+      in: location,
+      required: !shape[name].isOptional(),
+      description: depicted.description || description,
+      schema: result,
+      examples: depictParamExamples(schema, name),
+    };
+  });
 };
 
 export const depicters: HandlingRules<
@@ -828,29 +840,29 @@ export const onMissing: SchemaHandler<
 
 export const excludeParamsFromDepiction = (
   depicted: SchemaObject | ReferenceObject,
-  pathParams: string[],
+  names: string[],
 ): SchemaObject | ReferenceObject => {
   if (isReferenceObject(depicted)) {
     return depicted;
   }
   const copy = { ...depicted };
   if (copy.properties) {
-    copy.properties = omit(pathParams, copy.properties);
+    copy.properties = omit(names, copy.properties);
   }
   if (copy.examples) {
-    copy.examples = copy.examples.map((entry) => omit(pathParams, entry));
+    copy.examples = copy.examples.map((entry) => omit(names, entry));
   }
   if (copy.required) {
-    copy.required = copy.required.filter((name) => !pathParams.includes(name));
+    copy.required = copy.required.filter((name) => !names.includes(name));
   }
   if (copy.allOf) {
     copy.allOf = copy.allOf.map((entry) =>
-      excludeParamsFromDepiction(entry, pathParams),
+      excludeParamsFromDepiction(entry, names),
     );
   }
   if (copy.oneOf) {
     copy.oneOf = copy.oneOf.map((entry) =>
-      excludeParamsFromDepiction(entry, pathParams),
+      excludeParamsFromDepiction(entry, names),
     );
   }
   return copy;
@@ -1010,6 +1022,7 @@ export const depictSecurityRefs = (
   return depictSecurityRefs({ or: [container] });
 };
 
+// @todo depictBody
 export const depictRequest = ({
   method,
   path,
@@ -1020,9 +1033,11 @@ export const depictRequest = ({
   makeRef,
   composition,
   brandHandling,
+  paramNames,
   description = `${method.toUpperCase()} ${path} Request body`,
-}: ReqResDepictHelperCommonProps): RequestBodyObject => {
-  const pathParams = getRoutePathParams(path);
+}: ReqResDepictHelperCommonProps & {
+  paramNames: string[];
+}): RequestBodyObject => {
   const bodyDepiction = excludeExamplesFromDepiction(
     excludeParamsFromDepiction(
       walkSchema(schema, {
@@ -1038,15 +1053,15 @@ export const depictRequest = ({
           method,
         },
       }),
-      pathParams,
+      paramNames,
     ),
   );
   const media: MediaTypeObject = {
     schema:
       composition === "components"
         ? makeRef(makeCleanId(description), bodyDepiction)
         : bodyDepiction,
-    examples: depictExamples(schema, false, pathParams),
+    examples: depictExamples(schema, false, paramNames),
   };
   return { description, content: fromPairs(xprod(mimeTypes, [media])) };
 };

diff --git a/src/documentation.ts b/src/documentation.ts
@@ -7,6 +7,7 @@ import {
   SecuritySchemeObject,
   SecuritySchemeType,
 } from "openapi3-ts/oas31";
+import { pluck } from "ramda";
 import { z } from "zod";
 import { DocumentationError } from "./errors";
 import {
@@ -157,7 +158,7 @@ export class Documentation extends OpenApiBuilder {
       _method,
     ) => {
       const method = _method as Method;
-      const commonParams = {
+      const commons = {
         path,
         method,
         endpoint,
@@ -185,7 +186,7 @@ export class Documentation extends OpenApiBuilder {
       );
 
       const depictedParams = depictRequestParams({
-        ...commonParams,
+        ...commons,
         inputSources,
         schema: endpoint.getSchema("input"),
         description: descriptions?.requestParameter?.call(null, {
@@ -201,7 +202,7 @@ export class Documentation extends OpenApiBuilder {
         for (const { mimeTypes, schema, statusCodes } of apiResponses) {
           for (const statusCode of statusCodes) {
             responses[statusCode] = depictResponse({
-              ...commonParams,
+              ...commons,
               variant,
               schema,
               mimeTypes,
@@ -221,7 +222,8 @@ export class Documentation extends OpenApiBuilder {
 
       const requestBody = inputSources.includes("body")
         ? depictRequest({
-            ...commonParams,
+            ...commons,
+            paramNames: pluck("name", depictedParams),
             schema: endpoint.getSchema("input"),
             mimeTypes: endpoint.getMimeTypes("input"),
             description: descriptions?.requestBody?.call(null, {

diff --git a/tests/unit/__snapshots__/documentation-helpers.spec.ts.snap b/tests/unit/__snapshots__/documentation-helpers.spec.ts.snap
@@ -1317,7 +1317,7 @@ exports[`Documentation helpers > excludeParamsFromDepiction() > should handle th
 }
 `;
 
-exports[`Documentation helpers > excludeParamsFromDepiction() > should omit specified path params 0 1`] = `
+exports[`Documentation helpers > excludeParamsFromDepiction() > should omit specified params 0 1`] = `
 {
   "properties": {
     "b": {
@@ -1331,7 +1331,7 @@ exports[`Documentation helpers > excludeParamsFromDepiction() > should omit spec
 }
 `;
 
-exports[`Documentation helpers > excludeParamsFromDepiction() > should omit specified path params 1 1`] = `
+exports[`Documentation helpers > excludeParamsFromDepiction() > should omit specified params 1 1`] = `
 {
   "oneOf": [
     {
@@ -1354,7 +1354,7 @@ exports[`Documentation helpers > excludeParamsFromDepiction() > should omit spec
 }
 `;
 
-exports[`Documentation helpers > excludeParamsFromDepiction() > should omit specified path params 2 1`] = `
+exports[`Documentation helpers > excludeParamsFromDepiction() > should omit specified params 2 1`] = `
 {
   "properties": {
     "b": {
@@ -1368,7 +1368,7 @@ exports[`Documentation helpers > excludeParamsFromDepiction() > should omit spec
 }
 `;
 
-exports[`Documentation helpers > excludeParamsFromDepiction() > should omit specified path params 3 1`] = `
+exports[`Documentation helpers > excludeParamsFromDepiction() > should omit specified params 3 1`] = `
 {
   "allOf": [
     {