From edca15240e3899ac00ede95be436291cea891307 Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Mon, 29 Apr 2024 17:06:38 +0530 Subject: [PATCH 1/8] Add circular refrence check --- .../CosmosSqlMetadataProvider.cs | 27 +++++++++- .../Configuration/ConfigurationTests.cs | 49 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs index 3c7d9fc42e..ff5eaff270 100644 --- a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs @@ -195,18 +195,40 @@ private void ProcessSchema(IReadOnlyList fields, Dictionary schemaDocument, string currentPath, IncrementingInteger tableCounter, - EntityDbPolicyCosmosModel? previousEntity = null) + EntityDbPolicyCosmosModel? previousEntity = null, + HashSet? visitedEntity = null) { // Traverse the fields and add them to the path foreach (FieldDefinitionNode field in fields) { + // Create a tracker to keep track of visited entities to detect circular references + HashSet trackerForField = new(); + if (visitedEntity is not null) + { + trackerForField = visitedEntity; + } + string entityType = field.Type.NamedType().Name.Value; + // If the entity is not in the runtime config, skip it if (!_runtimeConfig.Entities.ContainsKey(entityType)) { continue; } + // If the entity is already visited, then it is a circular reference + if (trackerForField.Contains(entityType)) + { + throw new DataApiBuilderException( + message: $"Circular reference detected in the schema for entity '{entityType}'.", + statusCode: System.Net.HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); + } + else + { + trackerForField.Add(entityType); + } + string? alias = null; bool isArrayType = field.Type is ListTypeNode; if (isArrayType) @@ -253,7 +275,8 @@ private void ProcessSchema(IReadOnlyList fields, schemaDocument: schemaDocument, currentPath: isArrayType ? $"{alias}" : $"{currentPath}.{field.Name.Value}", tableCounter: tableCounter, - previousEntity: isArrayType ? currentEntity : null); + previousEntity: isArrayType ? currentEntity : null, + visitedEntity: trackerForField); } } diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index fa724a2ab4..4542a4ed92 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -425,6 +425,27 @@ public class ConfigurationTests } }"; + internal const string GRAPHQL_SCHEMA_WITH_CYCLE = @" +type Character { + id : ID, + name : String, + star: Planet, + moons: [Moon], +} + +type Planet @model(name:""Planet"") { + id : ID!, + name : String, + character: Character +} + +type Moon { + id : ID, + name : String, + details : String, + character: Character +} +"; [TestCleanup] public void CleanupAfterEachTest() { @@ -2938,6 +2959,34 @@ public async Task TestEngineSupportViewsWithoutKeyFieldsInConfigForMsSQL() } } + [TestMethod, TestCategory(TestCategory.COSMOSDBNOSQL)] + public void ValidateGraphQLSchemaForCircularReference() + { + // Read the base config from the file system + TestHelper.SetupDatabaseEnvironment(TestCategory.COSMOSDBNOSQL); + FileSystemRuntimeConfigLoader baseLoader = TestHelper.GetRuntimeConfigLoader(); + if (!baseLoader.TryLoadKnownConfig(out RuntimeConfig baseConfig)) + { + throw new ApplicationException("Failed to load the default CosmosDB_NoSQL config and cannot continue with tests."); + } + + // Setup a mock file system, and use that one with the loader/provider for the config + MockFileSystem fileSystem = new(new Dictionary() + { + { @"../schema.gql", new MockFileData(GRAPHQL_SCHEMA_WITH_CYCLE) }, + { FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME, + new MockFileData(baseConfig.ToJson()) } + }); + FileSystemRuntimeConfigLoader loader = new(fileSystem); + RuntimeConfigProvider provider = new(loader); + + DataApiBuilderException exception = + Assert.ThrowsException(() => new CosmosSqlMetadataProvider(provider, fileSystem)); + Assert.AreEqual("Circular reference detected in the schema for entity 'Character'.", exception.Message); + Assert.AreEqual(System.Net.HttpStatusCode.InternalServerError, exception.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ErrorInInitialization, exception.SubStatusCode); + } + [TestMethod, TestCategory(TestCategory.COSMOSDBNOSQL)] public async Task TestErrorMessageWithoutKeyFieldsInConfig() { From 47264b2223962ac35f10fa6edee1944965a7195b Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Tue, 30 Apr 2024 11:59:42 +0530 Subject: [PATCH 2/8] add more docs --- .../CosmosSqlMetadataProvider.cs | 23 +++++------ .../Configuration/ConfigurationTests.cs | 39 ++++++++++++++++--- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs index ff5eaff270..cace90af39 100644 --- a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs @@ -187,11 +187,16 @@ private void ParseSchemaGraphQLFieldsForJoins() /// 4. Check if we get previous entity with join information, if yes append it to the current entity also /// 5. Recursively call this function, to process the schema /// - /// - /// - /// - /// indicates the parent entity for which we are processing the schema. - private void ProcessSchema(IReadOnlyList fields, + /// All the fields of an entity + /// Schema Documents, useful to get fields information of an entity + /// Generated path of an entity + /// Counter used to generate table alias + /// indicates the parent entity for which we are processing the schema. + /// It is useful to get the JOIN statement information and create further new statements + /// Keeps a track of the path in an entity, to detect circular reference + /// It detects the circular reference in the schema while processing the schema and throws + private void ProcessSchema( + IReadOnlyList fields, Dictionary schemaDocument, string currentPath, IncrementingInteger tableCounter, @@ -217,17 +222,13 @@ private void ProcessSchema(IReadOnlyList fields, } // If the entity is already visited, then it is a circular reference - if (trackerForField.Contains(entityType)) + if (!trackerForField.Add(entityType)) { throw new DataApiBuilderException( - message: $"Circular reference detected in the schema for entity '{entityType}'.", + message: $"Circular reference detected in the provided GraphQL schema for entity '{entityType}'.", statusCode: System.Net.HttpStatusCode.InternalServerError, subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); } - else - { - trackerForField.Add(entityType); - } string? alias = null; bool isArrayType = field.Type is ListTypeNode; diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 4542a4ed92..c238c84a08 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -425,11 +425,10 @@ public class ConfigurationTests } }"; - internal const string GRAPHQL_SCHEMA_WITH_CYCLE = @" + internal const string GRAPHQL_SCHEMA_WITH_CYCLE_ARRAY = @" type Character { id : ID, name : String, - star: Planet, moons: [Moon], } @@ -446,6 +445,28 @@ type Moon { character: Character } "; + + internal const string GRAPHQL_SCHEMA_WITH_CYCLE_OBJECT = @" +type Character { + id : ID, + name : String, + moons: Moon, +} + +type Planet @model(name:""Planet"") { + id : ID!, + name : String, + character: Character +} + +type Moon { + id : ID, + name : String, + details : String, + character: Character +} +"; + [TestCleanup] public void CleanupAfterEachTest() { @@ -2959,8 +2980,16 @@ public async Task TestEngineSupportViewsWithoutKeyFieldsInConfigForMsSQL() } } + /// + /// In CosmosDB NoSQL, we store data in the form of JSON.Practically, JSON can be very complex. + /// But DAB doesn't support JSON with circular references e.g if 'Character.Moon' is a valid JSON Path, the + /// 'Moon.Character' should not be there, DAB would throw an exception during the load itself. + /// + /// [TestMethod, TestCategory(TestCategory.COSMOSDBNOSQL)] - public void ValidateGraphQLSchemaForCircularReference() + [DataRow(GRAPHQL_SCHEMA_WITH_CYCLE_OBJECT, DisplayName = "When Circular Reference is there with Object type (i.e. 'Moon' in 'Character' Entity")] + [DataRow(GRAPHQL_SCHEMA_WITH_CYCLE_ARRAY, DisplayName = "When Circular Reference is there with Array type (i.e. '[Moon]' in 'Character' Entity")] + public void ValidateGraphQLSchemaForCircularReference(string schema) { // Read the base config from the file system TestHelper.SetupDatabaseEnvironment(TestCategory.COSMOSDBNOSQL); @@ -2973,7 +3002,7 @@ public void ValidateGraphQLSchemaForCircularReference() // Setup a mock file system, and use that one with the loader/provider for the config MockFileSystem fileSystem = new(new Dictionary() { - { @"../schema.gql", new MockFileData(GRAPHQL_SCHEMA_WITH_CYCLE) }, + { @"../schema.gql", new MockFileData(schema) }, { FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME, new MockFileData(baseConfig.ToJson()) } }); @@ -2982,7 +3011,7 @@ public void ValidateGraphQLSchemaForCircularReference() DataApiBuilderException exception = Assert.ThrowsException(() => new CosmosSqlMetadataProvider(provider, fileSystem)); - Assert.AreEqual("Circular reference detected in the schema for entity 'Character'.", exception.Message); + Assert.AreEqual("Circular reference detected in the provided GraphQL schema for entity 'Character'.", exception.Message); Assert.AreEqual(System.Net.HttpStatusCode.InternalServerError, exception.StatusCode); Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ErrorInInitialization, exception.SubStatusCode); } From 2c6a4648f484434b09de2eaa81d243c6e1eff0ea Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Wed, 1 May 2024 06:21:29 +0530 Subject: [PATCH 3/8] rename variable --- .../MetadataProviders/CosmosSqlMetadataProvider.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs index cace90af39..2611ee7fde 100644 --- a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs @@ -191,7 +191,7 @@ private void ParseSchemaGraphQLFieldsForJoins() /// Schema Documents, useful to get fields information of an entity /// Generated path of an entity /// Counter used to generate table alias - /// indicates the parent entity for which we are processing the schema. + /// indicates the parent entity for which we are processing the schema. /// It is useful to get the JOIN statement information and create further new statements /// Keeps a track of the path in an entity, to detect circular reference /// It detects the circular reference in the schema while processing the schema and throws @@ -200,7 +200,7 @@ private void ProcessSchema( Dictionary schemaDocument, string currentPath, IncrementingInteger tableCounter, - EntityDbPolicyCosmosModel? previousEntity = null, + EntityDbPolicyCosmosModel? parentEntity = null, HashSet? visitedEntity = null) { // Traverse the fields and add them to the path @@ -258,15 +258,15 @@ private void ProcessSchema( }); } - if (previousEntity is not null) + if (parentEntity is not null) { if (string.IsNullOrEmpty(currentEntity.JoinStatement)) { - currentEntity.JoinStatement = previousEntity.JoinStatement; + currentEntity.JoinStatement = parentEntity.JoinStatement; } else { - currentEntity.JoinStatement = previousEntity.JoinStatement + " JOIN " + currentEntity.JoinStatement; + currentEntity.JoinStatement = parentEntity.JoinStatement + " JOIN " + currentEntity.JoinStatement; } } @@ -276,7 +276,7 @@ private void ProcessSchema( schemaDocument: schemaDocument, currentPath: isArrayType ? $"{alias}" : $"{currentPath}.{field.Name.Value}", tableCounter: tableCounter, - previousEntity: isArrayType ? currentEntity : null, + parentEntity: isArrayType ? currentEntity : null, visitedEntity: trackerForField); } } From 658a7527a663643c038a543bb6406fe816286f8d Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Fri, 3 May 2024 11:26:02 +0530 Subject: [PATCH 4/8] Update src/Service.Tests/Configuration/ConfigurationTests.cs Co-authored-by: Aniruddh Munde --- src/Service.Tests/Configuration/ConfigurationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index b3669c1c2c..8b88dc4046 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -2982,7 +2982,7 @@ public async Task TestEngineSupportViewsWithoutKeyFieldsInConfigForMsSQL() /// /// In CosmosDB NoSQL, we store data in the form of JSON.Practically, JSON can be very complex. - /// But DAB doesn't support JSON with circular references e.g if 'Character.Moon' is a valid JSON Path, the + /// But DAB doesn't support JSON with circular references e.g if 'Character.Moon' is a valid JSON Path, then /// 'Moon.Character' should not be there, DAB would throw an exception during the load itself. /// /// From a1d0c87514de70460fc1a4572eb8c8fe21ddb123 Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Fri, 3 May 2024 11:26:14 +0530 Subject: [PATCH 5/8] Update src/Service.Tests/Configuration/ConfigurationTests.cs Co-authored-by: Aniruddh Munde --- src/Service.Tests/Configuration/ConfigurationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 8b88dc4046..c2da680f67 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -2981,7 +2981,7 @@ public async Task TestEngineSupportViewsWithoutKeyFieldsInConfigForMsSQL() } /// - /// In CosmosDB NoSQL, we store data in the form of JSON.Practically, JSON can be very complex. + /// In CosmosDB NoSQL, we store data in the form of JSON. Practically, JSON can be very complex. /// But DAB doesn't support JSON with circular references e.g if 'Character.Moon' is a valid JSON Path, then /// 'Moon.Character' should not be there, DAB would throw an exception during the load itself. /// From 67b04165f74b9e12980cd4c04bb3f380a21d26ad Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Fri, 3 May 2024 11:39:16 +0530 Subject: [PATCH 6/8] rename variableds --- .../MetadataProviders/CosmosSqlMetadataProvider.cs | 10 +++++----- src/Service.Tests/Configuration/ConfigurationTests.cs | 5 ++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs index 2611ee7fde..cfcdd4389b 100644 --- a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs @@ -193,7 +193,7 @@ private void ParseSchemaGraphQLFieldsForJoins() /// Counter used to generate table alias /// indicates the parent entity for which we are processing the schema. /// It is useful to get the JOIN statement information and create further new statements - /// Keeps a track of the path in an entity, to detect circular reference + /// Keeps a track of the path in an entity, to detect circular reference /// It detects the circular reference in the schema while processing the schema and throws private void ProcessSchema( IReadOnlyList fields, @@ -201,16 +201,16 @@ private void ProcessSchema( string currentPath, IncrementingInteger tableCounter, EntityDbPolicyCosmosModel? parentEntity = null, - HashSet? visitedEntity = null) + HashSet? visitedEntities = null) { // Traverse the fields and add them to the path foreach (FieldDefinitionNode field in fields) { // Create a tracker to keep track of visited entities to detect circular references HashSet trackerForField = new(); - if (visitedEntity is not null) + if (visitedEntities is not null) { - trackerForField = visitedEntity; + trackerForField = visitedEntities; } string entityType = field.Type.NamedType().Name.Value; @@ -277,7 +277,7 @@ private void ProcessSchema( currentPath: isArrayType ? $"{alias}" : $"{currentPath}.{field.Name.Value}", tableCounter: tableCounter, parentEntity: isArrayType ? currentEntity : null, - visitedEntity: trackerForField); + visitedEntities: trackerForField); } } diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index c2da680f67..d0c3a85ff3 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -3003,8 +3003,7 @@ public void ValidateGraphQLSchemaForCircularReference(string schema) MockFileSystem fileSystem = new(new Dictionary() { { @"../schema.gql", new MockFileData(schema) }, - { FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME, - new MockFileData(baseConfig.ToJson()) } + { DEFAULT_CONFIG_FILE_NAME, new MockFileData(baseConfig.ToJson()) } }); FileSystemRuntimeConfigLoader loader = new(fileSystem); RuntimeConfigProvider provider = new(loader); @@ -3012,7 +3011,7 @@ public void ValidateGraphQLSchemaForCircularReference(string schema) DataApiBuilderException exception = Assert.ThrowsException(() => new CosmosSqlMetadataProvider(provider, fileSystem)); Assert.AreEqual("Circular reference detected in the provided GraphQL schema for entity 'Character'.", exception.Message); - Assert.AreEqual(System.Net.HttpStatusCode.InternalServerError, exception.StatusCode); + Assert.AreEqual(HttpStatusCode.InternalServerError, exception.StatusCode); Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ErrorInInitialization, exception.SubStatusCode); } From 985cbb6ca08196bff840b5620e80de8adf15b381 Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Fri, 3 May 2024 15:04:24 +0530 Subject: [PATCH 7/8] fix buildin check --- .../MetadataProviders/CosmosSqlMetadataProvider.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs index cfcdd4389b..e608b8a6c6 100644 --- a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs @@ -213,14 +213,13 @@ private void ProcessSchema( trackerForField = visitedEntities; } - string entityType = field.Type.NamedType().Name.Value; - - // If the entity is not in the runtime config, skip it - if (!_runtimeConfig.Entities.ContainsKey(entityType)) + // If the entity is build-in type, do not go further to check circular reference + if (GraphQLUtils.IsBuiltInType(field.Type)) { continue; } + string entityType = field.Type.NamedType().Name.Value; // If the entity is already visited, then it is a circular reference if (!trackerForField.Add(entityType)) { From 5e1380f1f2b004da8c799e098bd3a3735ee2c9e3 Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Fri, 3 May 2024 15:06:28 +0530 Subject: [PATCH 8/8] rename variable --- .../MetadataProviders/CosmosSqlMetadataProvider.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs index e608b8a6c6..795952da1c 100644 --- a/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs @@ -207,10 +207,10 @@ private void ProcessSchema( foreach (FieldDefinitionNode field in fields) { // Create a tracker to keep track of visited entities to detect circular references - HashSet trackerForField = new(); + HashSet trackerForFields = new(); if (visitedEntities is not null) { - trackerForField = visitedEntities; + trackerForFields = visitedEntities; } // If the entity is build-in type, do not go further to check circular reference @@ -221,7 +221,7 @@ private void ProcessSchema( string entityType = field.Type.NamedType().Name.Value; // If the entity is already visited, then it is a circular reference - if (!trackerForField.Add(entityType)) + if (!trackerForFields.Add(entityType)) { throw new DataApiBuilderException( message: $"Circular reference detected in the provided GraphQL schema for entity '{entityType}'.", @@ -276,7 +276,7 @@ private void ProcessSchema( currentPath: isArrayType ? $"{alias}" : $"{currentPath}.{field.Name.Value}", tableCounter: tableCounter, parentEntity: isArrayType ? currentEntity : null, - visitedEntities: trackerForField); + visitedEntities: trackerForFields); } }