From b912ad38e21d35a354d26e863462c2ec15bc16c4 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Fri, 15 Dec 2023 15:16:48 +0530 Subject: [PATCH 1/8] Adding validation to disallow multiple relationships between same source and target entities --- src/Core/Configurations/RuntimeConfigValidator.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index 8235f3789a..19de9832ce 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -651,9 +651,20 @@ public void ValidateRelationshipsInConfig(RuntimeConfig runtimeConfig, IMetadata string databaseName = runtimeConfig.GetDataSourceNameFromEntityName(entityName); ISqlMetadataProvider sqlMetadataProvider = sqlMetadataProviderFactory.GetMetadataProvider(databaseName); - + Dictionary targetEntityNameToRelationshipName = new(); foreach ((string relationshipName, EntityRelationship relationship) in entity.Relationships!) { + string targetEntityName = relationship.TargetEntity; + if (targetEntityNameToRelationshipName.TryGetValue(targetEntityName, out string? duplicateRelationshipName)) + { + throw new DataApiBuilderException( + message: $"Found multiple relationships: {duplicateRelationshipName}, {relationshipName} between source entity: {entityName} and target entity: {targetEntityName}", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError); + } + + // Add entry for this relationship to the dictionary tracking all the relationships for this entity. + targetEntityNameToRelationshipName[targetEntityName] = relationshipName; // Validate if entity referenced in relationship is defined in the config. if (!runtimeConfig.Entities.ContainsKey(relationship.TargetEntity)) { From 60ff4a965e2209e28ade1815aa8f08f75a422bf4 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Fri, 15 Dec 2023 15:26:20 +0530 Subject: [PATCH 2/8] updating comment --- src/Core/Configurations/RuntimeConfigValidator.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index 19de9832ce..e05c051608 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -651,6 +651,10 @@ public void ValidateRelationshipsInConfig(RuntimeConfig runtimeConfig, IMetadata string databaseName = runtimeConfig.GetDataSourceNameFromEntityName(entityName); ISqlMetadataProvider sqlMetadataProvider = sqlMetadataProviderFactory.GetMetadataProvider(databaseName); + + // Dictionary to store mapping from target entity's name to relationship name. Whenever we encounter that we + // are getting more than 1 entry for a target entity, we throw a validation error as it indicates the user has + // defined multiple relationships between the same source and target entities. Dictionary targetEntityNameToRelationshipName = new(); foreach ((string relationshipName, EntityRelationship relationship) in entity.Relationships!) { @@ -665,6 +669,7 @@ public void ValidateRelationshipsInConfig(RuntimeConfig runtimeConfig, IMetadata // Add entry for this relationship to the dictionary tracking all the relationships for this entity. targetEntityNameToRelationshipName[targetEntityName] = relationshipName; + // Validate if entity referenced in relationship is defined in the config. if (!runtimeConfig.Entities.ContainsKey(relationship.TargetEntity)) { From f016528663cd49f392a332648e741905cda92b8b Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Fri, 15 Dec 2023 16:00:28 +0530 Subject: [PATCH 3/8] adding test --- .../Unittests/ConfigValidationUnitTests.cs | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index 82d57dc20b..3ad81d50b7 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -333,6 +333,110 @@ public void TestAddingRelationshipWithDisabledGraphQL() Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); } + /// + /// Test to validate that we throw a validation error when GraphQL is enabled on the source entity and + /// the user defines multiple relationships between the same source and target entities. + /// + [DataTestMethod] + [DataRow(true, DisplayName = "Validate that exception is thrown when GQL is enabled and user defines multiple relationship between source and target entities.")] + [DataRow(false, DisplayName = "Validate that no exception is thrown when GQL is disabled and user defines multiple relationship between source and target entities.")] + public void TestMultipleRelationshipsBetweenSourceAndTargetEntities(bool isGQLEnabledForSource) + { + string sourceEntityName = "SourceEntity", targetEntityName = "TargetEntity"; + + // Create relationship between source and target entities. + EntityRelationship relationship = new( + Cardinality: Cardinality.One, + TargetEntity: targetEntityName, + SourceFields: new string[]{ "abc" }, + TargetFields: new string[] { "xyz" }, + LinkingObject: null, + LinkingSourceFields: null, + LinkingTargetFields: null + ); + + // Add another relationship between the same source and target entities. + EntityRelationship duplicateRelationship = new( + Cardinality: Cardinality.Many, + TargetEntity: targetEntityName, + SourceFields: null, + TargetFields: null, + LinkingObject: null, + LinkingSourceFields: null, + LinkingTargetFields: null + ); + + string relationshipName = "relationship", duplicateRelationshipName = "duplicateRelationship"; + Dictionary relationshipMap = new() + { + { relationshipName, relationship }, + { duplicateRelationshipName, duplicateRelationship } + }; + + // Creating source entity with enabled graphQL + Entity sourceEntity = GetSampleEntityUsingSourceAndRelationshipMap( + source: "TEST_SOURCE1", + relationshipMap: relationshipMap, + graphQLDetails: new(Singular: "", Plural: "", Enabled: isGQLEnabledForSource) + ); + + // Creating target entity. + Entity targetEntity = GetSampleEntityUsingSourceAndRelationshipMap( + source: "TEST_SOURCE2", + relationshipMap: null, + graphQLDetails: new("", "", true) + ); + + Dictionary entityMap = new() + { + { sourceEntityName, sourceEntity }, + { targetEntityName, targetEntity } + }; + + RuntimeConfig runtimeConfig = new( + Schema: "UnitTestSchema", + DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, "", Options: null), + Runtime: new( + Rest: new(), + GraphQL: new(), + Host: new(null, null) + ), + Entities: new(entityMap) + ); + + RuntimeConfigValidator configValidator = InitializeRuntimeConfigValidator(); + Mock _sqlMetadataProvider = new(); + Dictionary mockDictionaryForEntityDatabaseObject = new() + { + { + sourceEntityName, + new DatabaseTable("dbo", "TEST_SOURCE1") + }, + + { + targetEntityName, + new DatabaseTable("dbo", "TEST_SOURCE2") + } + }; + + _sqlMetadataProvider.Setup(x => x.EntityToDatabaseObject).Returns(mockDictionaryForEntityDatabaseObject); + Mock _metadataProviderFactory = new(); + _metadataProviderFactory.Setup(x => x.GetMetadataProvider(It.IsAny())).Returns(_sqlMetadataProvider.Object); + + if (isGQLEnabledForSource) + { + // Assert for expected exception. + DataApiBuilderException ex = Assert.ThrowsException(() => + configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object)); + Assert.AreEqual($"Found multiple relationships: {relationshipName}, {duplicateRelationshipName} between source entity: {sourceEntityName} and target entity: {targetEntityName}", ex.Message); + Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); + } + else + { + configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object); + } + } + /// /// Test method to check that an exception is thrown when LinkingObject was provided /// while either LinkingSourceField or SourceField is null, and either targetFields or LinkingTargetField is null. From 49ffde512193c007ca2d7e4e8390ad40765bd2eb Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Sat, 16 Dec 2023 12:34:46 +0530 Subject: [PATCH 4/8] fixing indentation --- src/Service.Tests/Unittests/ConfigValidationUnitTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index 3ad81d50b7..57a9e301c0 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -427,7 +427,7 @@ public void TestMultipleRelationshipsBetweenSourceAndTargetEntities(bool isGQLEn { // Assert for expected exception. DataApiBuilderException ex = Assert.ThrowsException(() => - configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object)); + configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object)); Assert.AreEqual($"Found multiple relationships: {relationshipName}, {duplicateRelationshipName} between source entity: {sourceEntityName} and target entity: {targetEntityName}", ex.Message); Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); } From 320ed1cc2c6b6d80c29fa859f87db2b156e9cf2a Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Sun, 4 Feb 2024 10:51:35 +0530 Subject: [PATCH 5/8] updating error message --- src/Core/Configurations/RuntimeConfigValidator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index 83d891b529..57bdef72cc 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -820,7 +820,7 @@ public void ValidateRelationshipsInConfig(RuntimeConfig runtimeConfig, IMetadata if (targetEntityNameToRelationshipName.TryGetValue(targetEntityName, out string? duplicateRelationshipName)) { HandleOrRecordException(new DataApiBuilderException( - message: $"Found multiple relationships: {duplicateRelationshipName}, {relationshipName} between source entity: {entityName} and target entity: {targetEntityName}", + message: $"Defining multiple relationships: {duplicateRelationshipName}, {relationshipName} between source entity: {entityName} and target entity: {targetEntityName} is not supported.", statusCode: HttpStatusCode.ServiceUnavailable, subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); } From 5c285e3f6964c8222f593751b2fe5a2e006e3bec Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 7 Feb 2024 15:54:09 +0530 Subject: [PATCH 6/8] updating method summary --- src/Service.Tests/Unittests/ConfigValidationUnitTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index 8e5720ad0d..64104396d5 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -337,8 +337,8 @@ public void TestAddingRelationshipWithDisabledGraphQL() } /// - /// Test to validate that we throw a validation error when GraphQL is enabled on the source entity and - /// the user defines multiple relationships between the same source and target entities. + /// Testing the RuntimeCOnfigValidator.ValidateRelationshipsInConfig() method to ensure that we throw a validation error + /// when GraphQL is enabled on the source entity and the user defines multiple relationships between the same source and target entities. /// [DataTestMethod] [DataRow(true, DisplayName = "Validate that an exception is thrown when GQL is enabled and user defines multiple relationship between source and target entities.")] From a8c987e7e65b3f58fb925bbf89477a6ec6bedd3f Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 7 Feb 2024 17:22:46 +0530 Subject: [PATCH 7/8] formatting fix --- src/Service.Tests/Unittests/ConfigValidationUnitTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index 64104396d5..167452920f 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -351,7 +351,7 @@ public void TestMultipleRelationshipsBetweenSourceAndTargetEntities(bool isGQLEn EntityRelationship relationship = new( Cardinality: Cardinality.One, TargetEntity: targetEntityName, - SourceFields: new string[]{ "abc" }, + SourceFields: new string[] { "abc" }, TargetFields: new string[] { "xyz" }, LinkingObject: null, LinkingSourceFields: null, From 1c18c1445f39adc6ad3c61abdeecb1202bf2564b Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 7 Feb 2024 17:38:24 +0530 Subject: [PATCH 8/8] updating test --- src/Service.Tests/Unittests/ConfigValidationUnitTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs index 167452920f..b367131d55 100644 --- a/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/Unittests/ConfigValidationUnitTests.cs @@ -431,7 +431,7 @@ public void TestMultipleRelationshipsBetweenSourceAndTargetEntities(bool isGQLEn // Assert for expected exception. DataApiBuilderException ex = Assert.ThrowsException(() => configValidator.ValidateRelationshipsInConfig(runtimeConfig, _metadataProviderFactory.Object)); - Assert.AreEqual($"Found multiple relationships: {relationshipName}, {duplicateRelationshipName} between source entity: {sourceEntityName} and target entity: {targetEntityName}", ex.Message); + Assert.AreEqual($"Defining multiple relationships: {relationshipName}, {duplicateRelationshipName} between source entity: {sourceEntityName} and target entity: {targetEntityName} is not supported.", ex.Message); Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode); } else