Skip to content

Commit ec56744

Browse files
ayush3797Aniruddh25seantleonard
authored
Validation to disallow multiple create on related entities backed by same database table (#2189)
## Why make this change? DAB doesn't support multiple-create on related entities which are backed by same database table. This PR adds a validation to catch that scenario and throw a meaningful exception to the end user. Issue to track support: #2157 ## What is this change? - Adds a validation during order determination logic in `MultipleCreateOrderHelper.GetReferencingEntityName()` method to catch the scenario. - In the method signature for `MultipleCreateOrderHelper.GetReferencingEntityName()`, a new parameter isMNRelationship is added with default value of false. The reasoning is explained in the section. #### Reason for why `MultipleCreateOrderHelper.GetReferencingEntityName()` is called even for M:N relationships when we know that we are always going to return an empty string? 1. For M:N relationships, we assume one of the source/target entity as the referencing entity when we do insertion during query execution because any of the entity can be considered as the referencing/referenced entity. However this does not hold true when the source and target entities are backed by same database tables - in which case we cannot consider any of the entity as the referenced/referencing entity. 2. The not-supported use case - that of not allowing multiple-create via relationships in which source and target entities are backed by same database tables is because DAB cannot determine a valid order of insertion. Since this concerns order determination, it is better to keep the code inside the `MultipleCreateOrderHelper.GetReferencingEntityName()` method. 3. Keeping this in `MultipleCreateOrderHelper` provides unit test coverage even for the other 2 db types i.e. PgSql,MySql. ## How was this tested? - Added test MultipleCreateOrderHelperUnitTests.TestExceptionForSelfReferencingRelationships() to test the change for the 3 dbs - MySql/MsSql/PgSql. ## Sample Request(s) 1. Config: ![image](https://github.com/Azure/data-api-builder/assets/34566234/262f5816-87e3-4a0c-acc6-291f56d3c213) Request: ![image](https://github.com/Azure/data-api-builder/assets/34566234/c8007c11-51d3-4029-a2e6-a3197d86d582) --------- Co-authored-by: Aniruddh Munde <[email protected]> Co-authored-by: Sean Leonard <[email protected]>
1 parent 5078a70 commit ec56744

File tree

6 files changed

+151
-17
lines changed

6 files changed

+151
-17
lines changed

config-generators/mysql-commands.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ init --config "dab-config.MySql.json" --database-type mysql --connection-string
22
add Publisher --config "dab-config.MySql.json" --source publishers --permissions "anonymous:read"
33
add Stock --config "dab-config.MySql.json" --source stocks --permissions "anonymous:create,read,update,delete"
44
add Book --config "dab-config.MySql.json" --source books --permissions "anonymous:create,read,update,delete" --graphql "book:books"
5+
add BookNF --config "dab-config.MySql.json" --source books --permissions "anonymous:*" --rest true --graphql "bookNF:booksNF"
56
add BookWebsitePlacement --config "dab-config.MySql.json" --source book_website_placements --permissions "anonymous:read"
67
add Author --config "dab-config.MySql.json" --source authors --permissions "anonymous:read"
78
add Review --config "dab-config.MySql.json" --source reviews --permissions "anonymous:create,read,update" --rest false --graphql "review:reviews"

src/Core/Resolvers/MultipleCreateOrderHelper.cs

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,14 @@ public class MultipleCreateOrderHelper
2424
/// returns the referencing entity's name for the pair of (source, target) entities.
2525
///
2626
/// When visualized as a graphQL mutation request,
27-
/// Source entity refers to the top level entity
27+
/// Source entity refers to the top level entity in whose configuration the relationship is defined.
2828
/// Target entity refers to the related entity.
2929
///
3030
/// This method handles the logic to determine the referencing entity for relationships from (source, target) with cardinalities:
3131
/// 1. 1:N - Target entity is the referencing entity
3232
/// 2. N:1 - Source entity is the referencing entity
3333
/// 3. 1:1 - Determined based on foreign key constraint/request input data.
34+
/// 4. M:N - None of the source/target entity is the referencing entity. Instead, linking table acts as the referencing entity.
3435
/// </summary>
3536
/// <param name="context">GraphQL request context.</param>
3637
/// <param name="relationshipName">Configured relationship name in the config file b/w source and target entity.</param>
@@ -40,6 +41,7 @@ public class MultipleCreateOrderHelper
4041
/// <param name="columnDataInSourceBody">Column name/value for backing columns present in the request input for the source entity.</param>
4142
/// <param name="targetNodeValue">Input GraphQL value for target node (could be an object or array).</param>
4243
/// <param name="nestingLevel">Nesting level of the entity in the mutation request.</param>
44+
/// <param name="isMNRelationship">true if the relationship is a Many-Many relationship.</param>
4345
public static string GetReferencingEntityName(
4446
IMiddlewareContext context,
4547
string relationshipName,
@@ -48,7 +50,8 @@ public static string GetReferencingEntityName(
4850
ISqlMetadataProvider metadataProvider,
4951
Dictionary<string, IValueNode?> columnDataInSourceBody,
5052
IValueNode? targetNodeValue,
51-
int nestingLevel)
53+
int nestingLevel,
54+
bool isMNRelationship = false)
5255
{
5356
if (!metadataProvider.GetEntityNamesAndDbObjects().TryGetValue(sourceEntityName, out DatabaseObject? sourceDbObject) ||
5457
!metadataProvider.GetEntityNamesAndDbObjects().TryGetValue(targetEntityName, out DatabaseObject? targetDbObject))
@@ -61,10 +64,39 @@ public static string GetReferencingEntityName(
6164
subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound);
6265
}
6366

67+
if (sourceDbObject.GetType() != typeof(DatabaseTable) || targetDbObject.GetType() != typeof(DatabaseTable))
68+
{
69+
throw new DataApiBuilderException(
70+
message: $"Cannot execute multiple-create for relationship: {relationshipName} at level: {nestingLevel} because currently DAB supports" +
71+
$"multiple-create only for entities backed by tables.",
72+
statusCode: HttpStatusCode.BadRequest,
73+
subStatusCode: DataApiBuilderException.SubStatusCodes.NotSupported);
74+
}
75+
76+
DatabaseTable sourceDbTable = (DatabaseTable)sourceDbObject;
77+
DatabaseTable targetDbTable = (DatabaseTable)targetDbObject;
78+
if (sourceDbTable.Equals(targetDbTable))
79+
{
80+
// DAB does not yet support multiple-create for self referencing relationships where both the source and
81+
// target entities are backed by same database table.
82+
throw new DataApiBuilderException(
83+
message: $"Multiple-create for relationship: {relationshipName} at level: {nestingLevel} is not supported because the " +
84+
$"source entity: {sourceEntityName} and the target entity: {targetEntityName} are backed by the same database table.",
85+
statusCode: HttpStatusCode.BadRequest,
86+
subStatusCode: DataApiBuilderException.SubStatusCodes.NotSupported);
87+
}
88+
89+
if (isMNRelationship)
90+
{
91+
// For M:N relationships, neither the source nor the target entity act as the referencing entity.
92+
// Instead, the linking table act as the referencing entity.
93+
return string.Empty;
94+
}
95+
6496
if (TryDetermineReferencingEntityBasedOnEntityRelationshipMetadata(
6597
sourceEntityName: sourceEntityName,
6698
targetEntityName: targetEntityName,
67-
sourceDbObject: sourceDbObject,
99+
sourceDbTable: sourceDbTable,
68100
referencingEntityName: out string? referencingEntityNameBasedOnEntityMetadata))
69101
{
70102
return referencingEntityNameBasedOnEntityMetadata;
@@ -100,18 +132,17 @@ public static string GetReferencingEntityName(
100132
/// </summary>
101133
/// <param name="sourceEntityName">Source entity name.</param>
102134
/// <param name="targetEntityName">Target entity name.</param>
103-
/// <param name="sourceDbObject">Database object for source entity.</param>
135+
/// <param name="sourceDbTable">Database table for source entity.</param>
104136
/// <param name="referencingEntityName">Stores the determined referencing entity name to be returned to the caller.</param>
105137
/// <returns>True when the referencing entity name can be determined based on the foreign key constraint defined in the database;
106138
/// else false.</returns>
107139
private static bool TryDetermineReferencingEntityBasedOnEntityRelationshipMetadata(
108140
string sourceEntityName,
109141
string targetEntityName,
110-
DatabaseObject sourceDbObject,
142+
DatabaseTable sourceDbTable,
111143
[NotNullWhen(true)] out string? referencingEntityName)
112144
{
113-
DatabaseTable sourceDbTable = (DatabaseTable)sourceDbObject;
114-
SourceDefinition sourceDefinition = sourceDbObject.SourceDefinition;
145+
SourceDefinition sourceDefinition = sourceDbTable.SourceDefinition;
115146

116147
List<ForeignKeyDefinition> targetEntityForeignKeys = sourceDefinition.SourceEntityRelationshipMap[sourceEntityName].TargetEntityToFkDefinitionMap[targetEntityName];
117148
HashSet<string> referencingEntityNames = new();

src/Core/Services/MultipleMutationInputValidator.cs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -345,15 +345,7 @@ private void ProcessRelationshipField(
345345
const string relationshipSourceIdentifier = "$";
346346
string targetEntityName = runtimeConfig.Entities![entityName].Relationships![relationshipName].TargetEntity;
347347
string? linkingObject = runtimeConfig.Entities![entityName].Relationships![relationshipName].LinkingObject;
348-
if (!string.IsNullOrWhiteSpace(linkingObject))
349-
{
350-
// The presence of a linking object indicates that an M:N relationship exists between the current entity and the target/child entity.
351-
// The linking table acts as a referencing table for both the source/target entities which act as
352-
// referenced entities. Consequently:
353-
// - Column values for the target entity can't be derived from insertion in the current entity.
354-
// - Column values for the current entity can't be derived from the insertion in the target/child entity.
355-
return;
356-
}
348+
bool isMNRelationship = !string.IsNullOrWhiteSpace(linkingObject);
357349

358350
// Determine the referencing entity for the current relationship field input.
359351
string referencingEntityName = MultipleCreateOrderHelper.GetReferencingEntityName(
@@ -364,7 +356,18 @@ private void ProcessRelationshipField(
364356
metadataProvider: metadataProvider,
365357
columnDataInSourceBody: backingColumnData,
366358
targetNodeValue: relationshipFieldValue,
367-
nestingLevel: nestingLevel);
359+
nestingLevel: nestingLevel,
360+
isMNRelationship: isMNRelationship);
361+
362+
if (isMNRelationship)
363+
{
364+
// The presence of a linking object indicates that an M:N relationship exists between the current entity and the target/child entity.
365+
// The linking table acts as a referencing table for both the source/target entities which act as
366+
// referenced entities. Consequently:
367+
// - Column values for the target entity can't be derived from insertion in the current entity.
368+
// - Column values for the current entity can't be derived from the insertion in the target/child entity.
369+
return;
370+
}
368371

369372
// Determine the referenced entity.
370373
string referencedEntityName = referencingEntityName.Equals(entityName) ? targetEntityName : entityName;

src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -766,6 +766,32 @@
766766
}
767767
}
768768
},
769+
{
770+
BookNF: {
771+
Source: {
772+
Object: books,
773+
Type: Table
774+
},
775+
GraphQL: {
776+
Singular: bookNF,
777+
Plural: booksNF,
778+
Enabled: true
779+
},
780+
Rest: {
781+
Enabled: true
782+
},
783+
Permissions: [
784+
{
785+
Role: anonymous,
786+
Actions: [
787+
{
788+
Action: *
789+
}
790+
]
791+
}
792+
]
793+
}
794+
},
769795
{
770796
BookWebsitePlacement: {
771797
Source: {

src/Service.Tests/Unittests/MultipleCreateUnitTests/MultipleCreateOrderHelperUnitTests.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
using System.Collections.Generic;
5+
using System.Net;
56
using Azure.DataApiBuilder.Core.Resolvers;
67
using Azure.DataApiBuilder.Service.Exceptions;
78
using Azure.DataApiBuilder.Service.Tests.SqlTests;
@@ -374,6 +375,23 @@ public void ValidateReferencingEntityBasedOnEntityMetadata()
374375

375376
#endregion
376377

378+
#region Order determination test for relationships having source/target entities backed by same database table
379+
380+
/// <summary>
381+
/// Test to validate that when multiple-create is executed for a relationship for which source and target entities are backed by the
382+
/// same database table, we throw an appropriate exception because DAB currently does not support multiple-create for such relationships.
383+
/// </summary>
384+
[TestMethod]
385+
public void TestExceptionForSelfReferencingRelationships()
386+
{
387+
// Identical source and target entities backed by the same database table 'books'.
388+
ValidateExceptionForSelfReferencingRelationship(sourceEntityName: "Book", targetEntityName: "Book");
389+
390+
// Different source and target entities backed by the same database table 'books'.
391+
ValidateExceptionForSelfReferencingRelationship(sourceEntityName: "Book", targetEntityName: "BookNF");
392+
}
393+
#endregion
394+
377395
#region Helpers
378396
private static void ValidateReferencingEntityForRelationship(
379397
string sourceEntityName,
@@ -395,6 +413,35 @@ private static void ValidateReferencingEntityForRelationship(
395413
nestingLevel: 1);
396414
Assert.AreEqual(expectedReferencingEntityName, actualReferencingEntityName);
397415
}
416+
417+
/// <summary>
418+
/// Helper method to validate the exception when multiple-create is executed for a self-referencing relationship where source and target
419+
/// entities are backed by the same database table.
420+
/// </summary>
421+
/// <param name="sourceEntityName">Name of the source entity.</param>
422+
/// <param name="targetEntityName">NAme of the target entity.</param>
423+
private static void ValidateExceptionForSelfReferencingRelationship(
424+
string sourceEntityName,
425+
string targetEntityName)
426+
{
427+
// Setup mock IMiddlewareContext.
428+
IMiddlewareContext context = SetupMiddlewareContext();
429+
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(() => MultipleCreateOrderHelper.GetReferencingEntityName(
430+
relationshipName: "testRelationship", // Don't need relationship name while testing determination of referencing entity using metadata.
431+
context: context,
432+
sourceEntityName: sourceEntityName,
433+
targetEntityName: targetEntityName,
434+
metadataProvider: _sqlMetadataProvider,
435+
columnDataInSourceBody: new(),
436+
targetNodeValue: null,
437+
nestingLevel: 1));
438+
439+
// Assert that the exception is as expected.
440+
Assert.AreEqual(HttpStatusCode.BadRequest, ex.StatusCode);
441+
Assert.AreEqual(DataApiBuilderException.SubStatusCodes.NotSupported, ex.SubStatusCode);
442+
Assert.AreEqual($"Multiple-create for relationship: testRelationship at level: 1 is not supported because the source entity: {sourceEntityName} and" +
443+
$" the target entity: {targetEntityName} are backed by the same database table.", ex.Message);
444+
}
398445
#endregion
399446

400447
#region Setup

src/Service.Tests/dab-config.MySql.json

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,32 @@
821821
}
822822
}
823823
},
824+
"BookNF": {
825+
"source": {
826+
"object": "books",
827+
"type": "table"
828+
},
829+
"graphql": {
830+
"enabled": true,
831+
"type": {
832+
"singular": "bookNF",
833+
"plural": "booksNF"
834+
}
835+
},
836+
"rest": {
837+
"enabled": true
838+
},
839+
"permissions": [
840+
{
841+
"role": "anonymous",
842+
"actions": [
843+
{
844+
"action": "*"
845+
}
846+
]
847+
}
848+
]
849+
},
824850
"BookWebsitePlacement": {
825851
"source": {
826852
"object": "book_website_placements",

0 commit comments

Comments
 (0)