Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack overflow on recursive schemas #8

Closed
dalewking opened this issue Dec 6, 2018 · 3 comments
Closed

Stack overflow on recursive schemas #8

dalewking opened this issue Dec 6, 2018 · 3 comments
Labels
0.2.0 Release 0.2.0 bug Something isn't working

Comments

@dalewking
Copy link

We had our own library that tried to do what swagger-brake is doing and ran into the same problem.

The solution is that you have to remember where you have been when recursing and don't recurse into a schema you have already seen on the way down.

Here is a simplified swagger spec that causes the stack overflow. Note that PaymentItemDTO contains an array of children PaymentItemDTO objects.

{
  "swagger":"2.0",
  "host":"localhost",
  "basePath":"/",
  "tags":[
    {
      "name":"audit-controller",
      "description":"Audit Controller"
    },
    {
      "name":"audit-fee-controller",
      "description":"Audit Fee Controller"
    },
    {
      "name":"operation-handler",
      "description":"Operation Handler"
    }
  ],
  "paths":{
    "/api/v1/audits/summary/{businessId}":{
      "get":{
        "tags":[
          "audit-controller"
        ],
        "summary":"fetchAuditSummaryByBusinessId",
        "operationId":"fetchAuditSummaryByBusinessIdUsingGET",
        "produces":[
          "*/*"
        ],
        "parameters":[
          {
            "name":"businessId",
            "in":"path",
            "description":"businessId",
            "required":true,
            "type":"string",
            "format":"uuid"
          }
        ],
        "responses":{
          "200":{
            "description":"OK",
            "schema":{
              "$ref":"#/definitions/AuditSummaryDTO"
            }
          },
          "401":{
            "description":"Unauthorized"
          },
          "403":{
            "description":"Forbidden"
          },
          "404":{
            "description":"Not Found"
          }
        },
        "security":[
          {
            "jwt":[
              "global"
            ]
          }
        ],
        "deprecated":false
      }
    }
  },
  "securityDefinitions":{
    "jwt":{
      "type":"apiKey",
      "name":"Authorization",
      "in":"header"
    }
  },
  "definitions":{
    "AuditSummaryDTO":{
      "type":"object",
      "properties":{
        "payoffSavings":{
          "type":"number"
        },
        "unverifiedPayoff":{
          "type":"number"
        },
        "unverifiedPayoffBreakdown":{
          "type":"array",
          "items":{
            "$ref":"#/definitions/PaymentItemDTO"
          }
        },
        "unverifiedVehicles":{
          "type":"integer",
          "format":"int64"
        }
      },
      "title":"AuditSummaryDTO"
    },
    "PaymentItemDTO":{
      "type":"object",
      "properties":{
        "amountApplied":{
          "type":"number"
        },
        "children":{
          "type":"array",
          "items":{
            "$ref":"#/definitions/PaymentItemDTO"
          }
        },
        "description":{
          "type":"string"
        },
        "recordType":{
          "type":"string",
          "enum":[
            "PRINCIPAL",
            "INTEREST",
            "CPP",
            "TRANSPORTATION_FEE",
            "AUDIT_FEE",
            "OTHER_FEE"
          ]
        }
      },
      "title":"PaymentItemDTO"
    }
  }
}

@galovics
Copy link
Member

galovics commented Dec 6, 2018

Totally valid point. I knew this will happen, I just didn't consider the recursive structure as a common use-case. I'll take a look. Thanks!

@galovics galovics added the bug Something isn't working label Dec 6, 2018
@dalewking
Copy link
Author

Here is an outline of how I fixed it in our library (which was written by an intern so we are looking to replace with swagger-brake):

I created a set to remember the path:

private Set<Object> seen = new HashSet<>();

The method to determine if 2 types are the same looks like this:

    boolean hasChangedElement = false;

    // Evaluate the map the first time we see it, otherwise ignore since it is a recursive instance
    if(seen.add(old)) {
        hasChangedElement = // determine if it is changed which includes recursion

        seen.remove(old);
    }

    return hasChangedElement;

@galovics
Copy link
Member

galovics commented Dec 9, 2018

Fixed.

@galovics galovics closed this as completed Dec 9, 2018
@galovics galovics added the 0.2.0 Release 0.2.0 label Dec 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.2.0 Release 0.2.0 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants