Repository: RobinTail/express-zod-api (ts)

Issue:
Assigning a singular `Security` schema to a `Middleware` leads to an error.
### Discussed in https://github.com/RobinTail/express-zod-api/discussions/803

<div type='discussions-op-text'>

<sup>Originally posted by **McMerph** February  8, 2023</sup>
I want to specify that a particular middleware uses JWT Bearer Authentication.
So, I added:
```
security: { type: "bearer", format: "JWT" }
```
to `createMiddleware`.
Unfortunately it didn't work -(
Error is:
```
TypeError: methods[security.type] is not a function
    at {{PROJECT_DIR}}/node_modules/express-zod-api/src/open-api-helpers.ts:801:69
    at {{PROJECT_DIR}}/node_modules/express-zod-api/src/logical-container.ts:40:15
    at Array.map (<anonymous>)
    at mapLogicalContainer ({{PROJECT_DIR}}/node_modules/express-zod-api/src/logical-container.ts:37:28)
    at depictSecurity ({{PROJECT_DIR}}/node_modules/express-zod-api/src/open-api-helpers.ts:800:29)
    at onEndpoint ({{PROJECT_DIR}}/node_modules/express-zod-api/src/open-api.ts:120:25)
    at {{PROJECT_DIR}}/node_modules/express-zod-api/src/routing-walker.ts:46:9
    at Array.forEach (<anonymous>)
    at {{PROJECT_DIR}}/node_modules/express-zod-api/src/routing-walker.ts:45:15
    at Array.forEach (<anonymous>)
```
What am I doing wrong?

---
```
security: { type: "bearer" }
```
doesn't work either.

Actually
```
security: { or: [{ type: "bearer", format: "JWT" }] }
```
or
```
security: { and: [{ type: "bearer", format: "JWT" }] }
```
both work but https://editor.swagger.io/ reports structural errors in this case (screenshot provided).
Both the middleware and the endpoint have an empty input (`input: z.object({})`) if that matters.
<img width="1725" alt="image" src="https://user-images.githubusercontent.com/29835709/217489320-92c95070-6739-49a9-8c65-3f3a6308f800.png">
</div>

Pull request description:
Fix OpenAPI bearer security
This relates to discussion https://github.com/RobinTail/express-zod-api/discussions/803
This PR fixes #816 

---
## OpenAPI spec generation crashes when using bearer security

### Steps to reproduce:
- add middleware like this:
```
createMiddleware({
  security: { type: "bearer" },
...
```
to any endpoint
- call `new OpenAPI({...}).getSpecAsYaml()`

### Actual behaviour:
```
TypeError: methods[security.type] is not a function
    at {{PROJECT_DIR}}/node_modules/express-zod-api/src/open-api-helpers.ts:801:69
    at {{PROJECT_DIR}}/node_modules/express-zod-api/src/logical-container.ts:40:15
    at Array.map (<anonymous>)
    at mapLogicalContainer ({{PROJECT_DIR}}/node_modules/express-zod-api/src/logical-container.ts:37:28)
    at depictSecurity ({{PROJECT_DIR}}/node_modules/express-zod-api/src/open-api-helpers.ts:800:29)
    at onEndpoint ({{PROJECT_DIR}}/node_modules/express-zod-api/src/open-api.ts:120:25)
    at {{PROJECT_DIR}}/node_modules/express-zod-api/src/routing-walker.ts:46:9
    at Array.forEach (<anonymous>)
    at {{PROJECT_DIR}}/node_modules/express-zod-api/src/routing-walker.ts:45:15
    at Array.forEach (<anonymous>)
```

### Expected behaviour
OpenApi spec generates without errors


Patch:
diff --git a/src/logical-container.ts b/src/logical-container.ts
index 4d85af0a..77356b77 100644
--- a/src/logical-container.ts
+++ b/src/logical-container.ts
@@ -82,29 +82,40 @@ export const combineContainers = <T>(
   a: LogicalContainer<T>,
   b: LogicalContainer<T>
 ): LogicalContainer<T> => {
-  if (isObject(a) && isObject(b)) {
+  if (isObject(a)) {
     if ("and" in a) {
-      if ("and" in b) {
-        return flattenAnds([a, b]);
-      }
-      if ("or" in b) {
-        return combineContainers(andToOr(a), b);
+      if (isObject(b)) {
+        if ("and" in b) {
+          return flattenAnds([a, b]);
+        }
+        if ("or" in b) {
+          return combineContainers(andToOr(a), b);
+        }
       }
+      return flattenAnds([a, b]);
     }
     if ("or" in a) {
-      if ("and" in b) {
-        return combineContainers(b, a);
-      }
-      if ("or" in b) {
-        const combs = combinations(a.or, b.or);
-        return {
-          or:
-            combs.type === "single"
-              ? combs.value
-              : combs.value.map(flattenAnds),
-        };
+      if (isObject(b)) {
+        if ("and" in b) {
+          return combineContainers(b, a);
+        }
+        if ("or" in b) {
+          const combs = combinations(a.or, b.or);
+          return {
+            or:
+              combs.type === "single"
+                ? combs.value
+                : combs.value.map(flattenAnds),
+          };
+        }
       }
+      return combineContainers(a, { and: [b] });
     }
   }
+
+  if (isObject(b) && ("and" in b || "or" in b)) {
+    return combineContainers(b, a);
+  }
+
   return { and: [a as T, b as T] };
 };
diff --git a/src/open-api-helpers.ts b/src/open-api-helpers.ts
index 65420e56..73e86f61 100644
--- a/src/open-api-helpers.ts
+++ b/src/open-api-helpers.ts
@@ -188,18 +188,21 @@ export const depictObject: Depicter<z.AnyZodObject> = ({
   schema,
   isResponse,
   next,
-}) => ({
-  type: "object",
-  properties: depictObjectProperties({ schema, isResponse, next }),
-  required: Object.keys(schema.shape).filter((key) => {
+}) => {
+  const required = Object.keys(schema.shape).filter((key) => {
     const prop = schema.shape[key];
     const isOptional =
       isResponse && hasCoercion(prop)
         ? prop instanceof z.ZodOptional
         : prop.isOptional();
     return !isOptional;
-  }),
-});
+  });
+  return {
+    type: "object",
+    properties: depictObjectProperties({ schema, isResponse, next }),
+    ...(required.length ? { required } : {}),
+  };
+};
 
 /**
  * @see https://swagger.io/docs/specification/data-models/data-types/
@@ -281,7 +284,7 @@ export const depictRecord: Depicter<z.ZodRecord<z.ZodTypeAny>> = ({
         isResponse,
         next,
       }),
-      required: keys,
+      ...(keys.length ? { required: keys } : {}),
     };
   }
   if (keySchema instanceof z.ZodLiteral) {


Test patch:
diff --git a/tests/unit/__snapshots__/open-api-helpers.spec.ts.snap b/tests/unit/__snapshots__/open-api-helpers.spec.ts.snap
index afdc6e06..5a06738a 100644
--- a/tests/unit/__snapshots__/open-api-helpers.spec.ts.snap
+++ b/tests/unit/__snapshots__/open-api-helpers.spec.ts.snap
@@ -507,7 +507,6 @@ exports[`Open API helpers depictObject() should type:object, properties and requ
       "type": "string",
     },
   },
-  "required": [],
   "type": "object",
 }
 `;
diff --git a/tests/unit/__snapshots__/open-api.spec.ts.snap b/tests/unit/__snapshots__/open-api.spec.ts.snap
index 8eef9e60..c70afc58 100644
--- a/tests/unit/__snapshots__/open-api.spec.ts.snap
+++ b/tests/unit/__snapshots__/open-api.spec.ts.snap
@@ -358,7 +358,6 @@ paths:
                   data:
                     type: object
                     properties: {}
-                    required: []
                 required:
                   - status
                   - data
@@ -397,14 +396,71 @@ paths:
               allOf:
                 - type: object
                   properties: {}
-                  required: []
                 - type: object
                   properties: {}
-                  required: []
       security:
         - HTTP_1: []
           OAUTH2_1:
             - write
+  /v1/updateSomething:
+    put:
+      responses:
+        "200":
+          description: PUT /v1/updateSomething Successful response
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  status:
+                    type: string
+                    enum:
+                      - success
+                  data:
+                    type: object
+                    properties: {}
+                required:
+                  - status
+                  - data
+        "400":
+          description: PUT /v1/updateSomething Error response
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  status:
+                    type: string
+                    enum:
+                      - error
+                  error:
+                    type: object
+                    properties:
+                      message:
+                        type: string
+                    required:
+                      - message
+                required:
+                  - status
+                  - error
+              examples:
+                example1:
+                  value:
+                    status: error
+                    error:
+                      message: Sample error message
+      requestBody:
+        content:
+          application/json:
+            schema:
+              description: PUT /v1/updateSomething request body
+              allOf:
+                - type: object
+                  properties: {}
+                - type: object
+                  properties: {}
+      security:
+        - HTTP_2: []
 components:
   schemas: {}
   responses: {}
@@ -428,6 +484,10 @@ components:
           scopes:
             read: read something
             write: write something
+    HTTP_2:
+      type: http
+      scheme: bearer
+      bearerFormat: JWT
   links: {}
   callbacks: {}
 tags: []
@@ -927,7 +987,6 @@ paths:
                   data:
                     type: object
                     properties: {}
-                    required: []
                 required:
                   - status
                   - data
@@ -1016,7 +1075,6 @@ paths:
                   data:
                     type: object
                     properties: {}
-                    required: []
                 required:
                   - status
                   - data
@@ -2622,7 +2680,6 @@ paths:
               description: POST /v1/getSomething request body
               type: object
               properties: {}
-              required: []
 components:
   schemas: {}
   responses: {}
@@ -2882,7 +2939,6 @@ paths:
                       any:
                         format: any
                         nullable: true
-                    required: []
                 required:
                   - status
                   - data
@@ -2963,7 +3019,6 @@ paths:
                       boolean:
                         type: boolean
                         nullable: true
-                    required: []
                 required:
                   - status
                   - data
diff --git a/tests/unit/logical-container.spec.ts b/tests/unit/logical-container.spec.ts
index 5c1ef2f6..8f067039 100644
--- a/tests/unit/logical-container.spec.ts
+++ b/tests/unit/logical-container.spec.ts
@@ -1,4 +1,5 @@
 import {
+  LogicalContainer,
   andToOr,
   combineContainers,
   mapLogicalContainer,
@@ -12,7 +13,7 @@ describe("LogicalContainer", () => {
           { and: [1, 2, { or: [3, 4] }, 5] },
           (value) => value * 2
         )
-      ).toEqual({ and: [2, 4, { or: [6, 8] }, 10] });
+      ).toEqual<LogicalContainer<number>>({ and: [2, 4, { or: [6, 8] }, 10] });
     });
     test("should map LogicalOr", () => {
       expect(
@@ -20,7 +21,7 @@ describe("LogicalContainer", () => {
           { or: [1, 2, { and: [3, 4] }, 5] },
           (value) => value * 2
         )
-      ).toEqual({ or: [2, 4, { and: [6, 8] }, 10] });
+      ).toEqual<LogicalContainer<number>>({ or: [2, 4, { and: [6, 8] }, 10] });
     });
     test("should accept plain value", () => {
       expect(mapLogicalContainer(1, (value) => value * 2)).toBe(2);
@@ -29,17 +30,21 @@ describe("LogicalContainer", () => {
 
   describe("andToOr()", () => {
     test("should convert LogicalAnd of plains", () => {
-      expect(andToOr({ and: [1, 2, 3] })).toEqual({
+      expect(andToOr({ and: [1, 2, 3] })).toEqual<LogicalContainer<number>>({
         or: [{ and: [1, 2, 3] }],
       });
     });
     test("should handle LogicalOr inside", () => {
-      expect(andToOr({ and: [1, { or: [2, 3] }, 4] })).toEqual({
+      expect(andToOr({ and: [1, { or: [2, 3] }, 4] })).toEqual<
+        LogicalContainer<number>
+      >({
         or: [{ and: [1, 2, 4] }, { and: [1, 3, 4] }],
       });
     });
     test("should handle multiple LogicalOr", () => {
-      expect(andToOr({ and: [1, { or: [2, 3] }, { or: [4, 5] }, 6] })).toEqual({
+      expect(andToOr({ and: [1, { or: [2, 3] }, { or: [4, 5] }, 6] })).toEqual<
+        LogicalContainer<number>
+      >({
         or: [
           { and: [1, 2, 4, 6] },
           { and: [1, 2, 5, 6] },
@@ -52,10 +57,14 @@ describe("LogicalContainer", () => {
 
   describe("combineContainers()", () => {
     test("should combine flats", () => {
-      expect(combineContainers(1, 2)).toEqual({ and: [1, 2] });
+      expect(combineContainers(1, 2)).toEqual<LogicalContainer<number>>({
+        and: [1, 2],
+      });
     });
     test("should combine two LogicalAnd", () => {
-      expect(combineContainers({ and: [1, 2] }, { and: [3, 4] })).toEqual({
+      expect(combineContainers({ and: [1, 2] }, { and: [3, 4] })).toEqual<
+        LogicalContainer<number>
+      >({
         and: [1, 2, 3, 4],
       });
       expect(
@@ -63,12 +72,14 @@ describe("LogicalContainer", () => {
           { and: [{ or: [1, 2] }, { or: [3, 4] }] },
           { and: [{ or: [5, 6] }, { or: [7, 8] }] }
         )
-      ).toEqual({
+      ).toEqual<LogicalContainer<number>>({
         and: [{ or: [1, 2] }, { or: [3, 4] }, { or: [5, 6] }, { or: [7, 8] }],
       });
     });
     test("should combine two LogicalOr", () => {
-      expect(combineContainers({ or: [1, 2] }, { or: [3, 4] })).toEqual({
+      expect(combineContainers({ or: [1, 2] }, { or: [3, 4] })).toEqual<
+        LogicalContainer<number>
+      >({
         or: [
           { and: [1, 3] },
           { and: [1, 4] },
@@ -78,7 +89,9 @@ describe("LogicalContainer", () => {
       });
     });
     test("should handle empty and non-empty LogicalOr", () => {
-      expect(combineContainers({ or: [1, 2] }, { or: [] })).toEqual({
+      expect(combineContainers({ or: [1, 2] }, { or: [] })).toEqual<
+        LogicalContainer<number>
+      >({
         or: [1, 2],
       });
     });
@@ -88,7 +101,7 @@ describe("LogicalContainer", () => {
           { or: [{ and: [1, 2] }, { and: [3, 4] }] },
           { or: [{ and: [5, 6] }, { and: [7, 8] }] }
         )
-      ).toEqual({
+      ).toEqual<LogicalContainer<number>>({
         or: [
           { and: [1, 2, 5, 6] },
           { and: [1, 2, 7, 8] },
@@ -98,14 +111,57 @@ describe("LogicalContainer", () => {
       });
     });
     test("should combine LogicalAnd with LogicalOr", () => {
-      expect(combineContainers({ and: [1, 2] }, { or: [3, 4] })).toEqual({
-        or: [{ and: [1, 2, 3] }, { and: [1, 2, 4] }],
-      });
+      expect(combineContainers({ and: [1, 2] }, { or: [3, 4] })).toEqual<
+        LogicalContainer<number>
+      >({ or: [{ and: [1, 2, 3] }, { and: [1, 2, 4] }] });
     });
     test("should handle reverse order", () => {
-      expect(combineContainers({ or: [1, 2] }, { and: [3, 4] })).toEqual({
-        or: [{ and: [3, 4, 1] }, { and: [3, 4, 2] }],
-      });
+      expect(combineContainers({ or: [1, 2] }, { and: [3, 4] })).toEqual<
+        LogicalContainer<number>
+      >({ or: [{ and: [3, 4, 1] }, { and: [3, 4, 2] }] });
+    });
+    test("should combine LogicalAnd and flat", () => {
+      expect(combineContainers({ and: [] }, 1)).toEqual<
+        LogicalContainer<number>
+      >({ and: [1] });
+      expect(combineContainers(1, { and: [] })).toEqual<
+        LogicalContainer<number>
+      >({ and: [1] });
+      expect(combineContainers({ and: [1] }, 2)).toEqual<
+        LogicalContainer<number>
+      >({ and: [1, 2] });
+      expect(combineContainers(1, { and: [2] })).toEqual<
+        LogicalContainer<number>
+      >({ and: [2, 1] });
+      expect(combineContainers({ and: [1, 2] }, 3)).toEqual<
+        LogicalContainer<number>
+      >({ and: [1, 2, 3] });
+    });
+    test("should combine LogicalOr and flat", () => {
+      expect(combineContainers({ or: [] }, 1)).toEqual<
+        LogicalContainer<number>
+      >({ or: [1] });
+      expect(combineContainers(1, { or: [] })).toEqual<
+        LogicalContainer<number>
+      >({ or: [1] });
+      expect(combineContainers({ or: [1] }, 2)).toEqual<
+        LogicalContainer<number>
+      >({ or: [{ and: [2, 1] }] });
+      expect(combineContainers(1, { or: [2] })).toEqual<
+        LogicalContainer<number>
+      >({ or: [{ and: [1, 2] }] });
+      expect(combineContainers({ or: [{ and: [1, 2] }] }, 3)).toEqual<
+        LogicalContainer<number>
+      >({ or: [{ and: [3, 1, 2] }] });
+    });
+
+    test("Issue #816: combining empty LogicalAnd with a flat that is an object", () => {
+      expect(
+        combineContainers({ and: [] }, { type: "bearer", format: "JWT" })
+      ).toEqual({ and: [{ type: "bearer", format: "JWT" }] });
+      expect(
+        combineContainers({ type: "bearer", format: "JWT" }, { and: [] })
+      ).toEqual({ and: [{ type: "bearer", format: "JWT" }] });
     });
   });
 });
diff --git a/tests/unit/open-api.spec.ts b/tests/unit/open-api.spec.ts
index 2904dbff..5344d536 100644
--- a/tests/unit/open-api.spec.ts
+++ b/tests/unit/open-api.spec.ts
@@ -821,6 +821,11 @@ describe("Open API generator", () => {
         input: z.object({}),
         middleware: jest.fn(),
       });
+      const mw3 = createMiddleware({
+        security: { type: "bearer", format: "JWT" },
+        input: z.object({}),
+        middleware: jest.fn(),
+      });
       const spec = new OpenAPI({
         config: sampleConfig,
         routing: {
@@ -843,6 +848,13 @@ describe("Open API generator", () => {
               output: z.object({}),
               handler: async () => ({}),
             }),
+            updateSomething: defaultEndpointsFactory.addMiddleware(mw3).build({
+              scopes: ["this should be omitted"],
+              method: "put",
+              input: z.object({}),
+              output: z.object({}),
+              handler: async () => ({}),
+            }),
           },
         },
         version: "3.4.5",