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
Cosmos DB: Adds circular reference check for entities in graphQL schema #2192
Conversation
/azp run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some questions before merging.
src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs
Outdated
Show resolved
Hide resolved
src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs
Outdated
Show resolved
Hide resolved
src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs
Outdated
Show resolved
Hide resolved
cf2fd29
to
47264b2
Compare
/azp run |
src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs
Outdated
Show resolved
Hide resolved
/azp run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/azp run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requires circular condition check even if runtime config doesnt contain the entity
src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
/azp run |
src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs
Outdated
Show resolved
Hide resolved
/azp run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for addressing all comments!
Why make this change?
Today, if graphQL schema has circular reference of entities, DAB won't be able to load the schema and die with stackoverflow exception when it tries to traverse the schema.
With JSON data, it can get very complicated to handle all the scenarios with circular reference.
What is this change?
Adding a proper exception during the load, if DAB identifies that schema has circular reference.
If schema is found with circular reference, DAB will throw below exception.
DAB Exception
Schema with circular reference
How was this tested?