Redoc: Recursive/circular logic not correct - 2.0.0-alpha.32

Created on 31 Jul 2018  路  8Comments  路  Source: Redocly/redoc

I've included a "full" spec that reproduces this problem. I expect Option 3 of schema "My Thing" to render, but it is labeling itself as recursive. It happens on any item in the allOf that has a reference that was already resolved elsewhere in that particular allOf object, I believe. This is taking away most of the utility behind using objects with allOf for me.

openapi: 3.0.1
info:
  version: 0.0.1
  title: My cool API

paths:
  # My Thing
  /mything:
    post:
      tags:
      - My thing
      summary: Add My Thing
      description: Add a new my thing
      operationId: api.addMyThing
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/MyThingCreate'
      responses:
        '201':
          description: Created
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/MyThingCreateResponse'

components:
  schemas:
    ThingA:
      type: object
      properties:
        thinga:
          type: string
    ThingB:
      type: object
      properties:
        thingb:
          type: string
    ThingC:
      type: object
      properties:
        thingc:
          type: string
    MyThing:
      allOf:
        - oneOf:
          - allOf:
            - title: Option 1
            - $ref: '#/components/schemas/ThingB'
          - allOf:
            - title: Option 2
            - $ref: '#/components/schemas/ThingA'
          - allOf:
            - title: Option 3
            - $ref: '#/components/schemas/ThingB'
            - $ref: '#/components/schemas/ThingA'
        - $ref: '#/components/schemas/ThingC'

    MyThingCreate:
      allOf:
        - $ref: '#/components/schemas/MyThing'
        - required:
          - thinga
          - thingb
    MyThingCreateResponse:
      type: object
      properties:
        thing1:
          type: string


  securitySchemes:
    bearerAuth:
      type: http
      scheme: bearer
      bearerFormat: JWT
security:
  - bearerAuth: []

image

Most helpful comment

@viralanomaly I checked it out and it seems I need to rewrite circular detecting logic completely. It's complex right and unmaintainable :(

I have an idea how to do it properly but it will take some time.

Expect this to be fixed at the end of the next week.

All 8 comments

If I remove ThingC and the outermost allOf within "MyThing", this renders as expected.

Is there a way to set forceCircular outside of this file? If I could just set it, my spec loads much better.

This still doesn't fix the fact that the code is assuming a circular reference just because a reference has already been visited.

diff --git a/src/services/OpenAPIParser.ts b/src/services/OpenAPIParser.ts
index 8037705..3bace31 100644
--- a/src/services/OpenAPIParser.ts
+++ b/src/services/OpenAPIParser.ts
@@ -146,7 +146,7 @@ export class OpenAPIParser {
    * @param obj object to dereference
    * @param forceCircular whether to dereference even if it is cirular ref
    */
-  deref(obj: OpenAPIRef | T, forceCircular: boolean = false): T {
+  deref(obj: OpenAPIRef | T, forceCircular: boolean = true): T {
     if (this.isRef(obj)) {
       const resolved = this.byRef(obj.$ref)!;
       const visited = this._refCounter.visited(obj.$ref);
@@ -183,7 +183,7 @@ export class OpenAPIParser {
   mergeAllOf(
     schema: OpenAPISchema,
     $ref?: string,
-    forceCircular: boolean = false,
+    forceCircular: boolean = true,
   ): MergedOpenAPISchema {
     schema = this.hoistOneOfs(schema);

@viralanomaly, thanks for detailed issue report!

Is there a way to set forceCircular outside of this file? If I could just set it, my spec loads much better.

No, I am working on a proper fix. It should be released in the next alpha.

@viralanomaly I checked it out and it seems I need to rewrite circular detecting logic completely. It's complex right and unmaintainable :(

I have an idea how to do it properly but it will take some time.

Expect this to be fixed at the end of the next week.

The circular logic looked pretty... special. Thanks for looking into it!

Have you made any progress on this, @RomanGotsiy ?

Not really. Before that, I have to make some changes to APIDevTools/json-schema-ref-parser and they are not so obvious.
I made a shortcut by patching the lib and verified it works but now I need to implement it properly and made a PR.

Already discussed possibilities with lib author so it is moving.

Thanks for reminding 馃檶

Did this fix expose forceCircular as well, or only fix the circular logic?

Was this page helpful?
0 / 5 - 0 ratings