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

Added ancestor tracking to example JSON objects to avoid infinite loop #2919

Merged
merged 2 commits into from Jan 22, 2021

Conversation

CodeWookiee
Copy link
Contributor

While attempting to import Swagger JSON for a project, Insomnia continuous failed. I tracked down the culprit to some endpoints that had objects that could contain children objects of the same type. The code that generates the example JSON body for the calls was ending up in an infinite loop that lead to a "Maximum call stack size exceeded" error.

This addresses that problem by introducing an array that tracks the schema's as it walks down the object tree. If it encounters an object who's schema matches one of the schemas of the object's "ancestors," it will skip generating an example for the object.

The array of "ancestor schemas" is passed down whether the current node being processed is an object or array of objects. However, since the array of objects is considered a level, with the object a level underneath it, the "ancestor schema" is only pushed onto the array when diving down into an object, not an array. After the recursion returns from generating a child object, it pops the current object off the array and moves on.

Potential shortcomings include situations where large schemas have deep layering, in which case the memory usage might grow out of hand. Additionally, the lack of example for the first recursion of an object in the tree might be ambiguous. I could not figure out an easy way to output something in the JSON example explaining the recursive object reference.

Hope this all makes sense and this wasn't a waste of time, but I figured I couldn't be the only person to end up unable to import swagger files as a result of the infinite loop.

@CLAassistant
Copy link

CLAassistant commented Dec 7, 2020

CLA assistant check
All committers have signed the CLA.

@develohpanda
Copy link
Contributor

Hi @CodeWookiee, thanks for a putting a PR up to address this and for such a detailed description! Would you be able to share an example spec which fails in the live app and works with this PR? I will need to spend some time reviewing this area but a spec to test with will be very useful 🙂

@CodeWookiee
Copy link
Contributor Author

Hi @CodeWookiee, thanks for a putting a PR up to address this and for such a detailed description! Would you be able to share an example spec which fails in the live app and works with this PR? I will need to spend some time reviewing this area but a spec to test with will be very useful 🙂

No problem. I'm glad to be able to offer my help, Insomnia is a wonderfully useful tool! I can't share the actual swagger file I had trouble with, but I generated another one from a dummy endpoint that trips up the importer in the same way. It's attached (in a zip file).
TestSwagger.zip

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, and thank you for the example spec as well.

Just looking at the code, it seems like this will be an issue for the openapi3 importer as well. @CodeWookiee would you be willing to tackle that one, either in this PR or another PR? 😄

@CodeWookiee
Copy link
Contributor Author

Looks great, and thank you for the example spec as well.

Just looking at the code, it seems like this will be an issue for the openapi3 importer as well. @CodeWookiee would you be willing to tackle that one, either in this PR or another PR? 😄

I would be, except that I have no experience with openapi3 (or whichever program that is associated with).

@netlify
Copy link

netlify bot commented Jan 22, 2021

Deploy preview for insomnia-storybook ready!

Built with commit 04e5378

https://deploy-preview-2919--insomnia-storybook.netlify.app

@develohpanda
Copy link
Contributor

develohpanda commented Jan 22, 2021

I would be, except that I have no experience with openapi3 (or whichever program that is associated with).

OpenAPI3 is just the next iteration of Swagger2 - the naming is different but they are very similar. I'll merge this PR for now to not block it, but feel free to explore it and raise another PR. 🙌🏽

@develohpanda develohpanda merged commit 139095d into Kong:develop Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants