fix: hoistOneOf missing refs stack and improve allOf for same $ref

Co-authored-by: Roman Hotsiy <gotsijroman@gmail.com>
Redocly · Sep 6, 2022 · bb325d0 · bb325d0
1 parent 6ac1e1e
commit bb325d0
Show file tree

Hide file tree

Showing 3 changed files with 119 additions and 26 deletions.
diff --git a/src/services/OpenAPIParser.ts b/src/services/OpenAPIParser.ts
@@ -173,7 +173,7 @@ export class OpenAPIParser {
       return schema;
     }
 
-    schema = this.hoistOneOfs(schema);
+    schema = this.hoistOneOfs(schema, refsStack);
 
     if (schema.allOf === undefined) {
       return schema;
@@ -194,30 +194,34 @@ export class OpenAPIParser {
       receiver.items = { ...receiver.items };
     }
 
-    const allOfSchemas = schema.allOf
-      .map((subSchema: OpenAPISchema) => {
-        const { resolved, refsStack: subRefsStack } = this.deref(subSchema, refsStack, true);
+    const allOfSchemas = uniqByPropIncludeMissing(
+      schema.allOf
+        .map((subSchema: OpenAPISchema) => {
+          const { resolved, refsStack: subRefsStack } = this.deref(subSchema, refsStack, true);
 
-        const subRef = subSchema.$ref || undefined;
-        const subMerged = this.mergeAllOf(resolved, subRef, subRefsStack);
-        if (subMerged['x-circular-ref'] && subMerged.allOf) {
-          // if mergeAllOf is circular and still contains allOf, we should ignore it
-          return undefined;
-        }
-        if (subRef) {
-          // collect information for implicit descriminator lookup
-          receiver['x-parentRefs']?.push(...(subMerged['x-parentRefs'] || []), subRef);
-        }
-        return {
-          $ref: subRef,
-          refsStack: pushRef(subRefsStack, subRef),
-          schema: subMerged,
-        };
-      })
-      .filter(child => child !== undefined) as Array<{
-      schema: MergedOpenAPISchema;
-      refsStack: string[];
-    }>;
+          const subRef = subSchema.$ref || undefined;
+          const subMerged = this.mergeAllOf(resolved, subRef, subRefsStack);
+          if (subMerged['x-circular-ref'] && subMerged.allOf) {
+            // if mergeAllOf is circular and still contains allOf, we should ignore it
+            return undefined;
+          }
+          if (subRef) {
+            // collect information for implicit descriminator lookup
+            receiver['x-parentRefs']?.push(...(subMerged['x-parentRefs'] || []), subRef);
+          }
+          return {
+            $ref: subRef,
+            refsStack: pushRef(subRefsStack, subRef),
+            schema: subMerged,
+          };
+        })
+        .filter(child => child !== undefined) as Array<{
+        schema: MergedOpenAPISchema;
+        refsStack: string[];
+        $ref?: string;
+      }>,
+      '$ref',
+    );
 
     for (const { schema: subSchema, refsStack: subRefsStack } of allOfSchemas) {
       const {
@@ -269,7 +273,10 @@ export class OpenAPIParser {
             // merge inner properties
             const mergedProp = this.mergeAllOf(
               {
-                allOf: [receiver.properties[prop], properties[prop]],
+                allOf: [
+                  receiver.properties[prop],
+                  { ...properties[prop], 'x-refsStack': propRefsStack } as any,
+                ],
                 'x-refsStack': propRefsStack,
               },
               $ref + '/properties/' + prop,
@@ -348,7 +355,7 @@ export class OpenAPIParser {
     return res;
   }
 
-  private hoistOneOfs(schema: OpenAPISchema) {
+  private hoistOneOfs(schema: OpenAPISchema, refsStack: string[]) {
     if (schema.allOf === undefined) {
       return schema;
     }
@@ -363,6 +370,7 @@ export class OpenAPIParser {
           oneOf: sub.oneOf.map((part: OpenAPISchema) => {
             return {
               allOf: [...beforeAllOf, part, ...afterAllOf],
+              'x-refsStack': refsStack,
             };
           }),
         };
@@ -372,3 +380,15 @@ export class OpenAPIParser {
     return schema;
   }
 }
+
+/**
+ * Unique array by property, missing properties are included
+ */
+function uniqByPropIncludeMissing<T extends object>(arr: T[], prop: keyof T): T[] {
+  const seen = new Set();
+  return arr.filter(item => {
+    const k = item[prop];
+    if (!k) return true;
+    return k && !seen.has(k) && seen.add(k);
+  });
+}
diff --git a/src/services/__tests__/__snapshots__/OpenAPIParser.test.ts.snap b/src/services/__tests__/__snapshots__/OpenAPIParser.test.ts.snap
@@ -57,6 +57,7 @@ Object {
           ],
         },
       ],
+      "x-refsStack": undefined,
     },
     Object {
       "allOf": Array [
@@ -96,6 +97,7 @@ Object {
           ],
         },
       ],
+      "x-refsStack": undefined,
     },
   ],
 }

diff --git a/src/services/__tests__/models/Schema.circular.test.ts b/src/services/__tests__/models/Schema.circular.test.ts
@@ -655,5 +655,76 @@ describe('Models', () => {
                   type: <string>
         `);
     });
+
+    test('should detect and recursion with nested oneOf case same schema', () => {
+      const spec = parseYaml(outdent`
+      openapi: 3.0.0
+      components:
+        schemas:
+          Test:
+            allOf:
+              - type: object
+                required:
+                  - "@relations"
+                properties:
+                  "@relations":
+                    type: object
+                    properties:
+                      A:
+                        $ref: "#/components/schemas/A"
+              - type: object
+                required:
+                  - "@relations"
+                properties:
+                  "@relations":
+                    type: object
+                    properties:
+                      A:
+                        $ref: "#/components/schemas/A"
+          A:
+            type: object
+            description: Description
+            properties:
+              B:
+                type: array
+                items:
+                  oneOf:
+                    - type: object
+                    - title: tableLookup
+                      type: object
+                      properties:
+                        fallback:
+                          type: array
+                          default: []
+                          items:
+                            $ref: "#/components/schemas/A/properties/B"
+      `) as any;
+
+      parser = new OpenAPIParser(spec, undefined, opts);
+      const schema = new SchemaModel(
+        parser,
+        spec.components.schemas.Test,
+        '#/components/schemas/Test',
+        opts,
+      );
+
+      expect(printSchema(schema, circularDetailsPrinter)).toMatchInlineSnapshot(`
+        @relations*:
+          A:
+            B: [
+              oneOf
+                object -> <object>
+                tableLookup ->
+                  fallback: [
+                    [
+                    oneOf
+                      object -> <object>
+                      tableLookup ->
+                        fallback: [<array> !circular]
+                    ]
+                  ]
+            ]
+        `);
+    });
   });
 });