Removing wrong FK definition

src/Core/Services/MetadataProviders/SqlMetadataProvider.cs

@@ -1777,52 +1777,103 @@ private void FillInferredFkInfo(
             {
                 // For each source entities, which maps to this table definition
                 // and has a relationship metadata to be filled.
-                foreach ((_, RelationshipMetadata relationshipData)
+                foreach ((string sourceEntityName, RelationshipMetadata relationshipData)
                        in sourceDefinition.SourceEntityRelationshipMap)
                 {
                     // Enumerate all the foreign keys required for all the target entities
                     // that this source is related to.
-                    IEnumerable<List<ForeignKeyDefinition>> foreignKeysForAllTargetEntities =
-                        relationshipData.TargetEntityToFkDefinitionMap.Values;
-                    // For each target, loop through each foreign key
-                    foreach (List<ForeignKeyDefinition> foreignKeysForTarget in foreignKeysForAllTargetEntities)
+                    foreach ((string targetEntityName, List<ForeignKeyDefinition> fKDefinitionsToTarget) in relationshipData.TargetEntityToFkDefinitionMap)
                     {
-                        // For each foreign key between this pair of source and target entities
-                        // which needs the referencing columns,
-                        // find the fk inferred for this pair the backend and
-                        // equate the referencing columns and referenced columns.
-                        foreach (ForeignKeyDefinition fk in foreignKeysForTarget)
-                        {
-                            // if the referencing and referenced columns count > 0,
-                            // we have already gathered this information from the runtime config.
-                            if (fk.ReferencingColumns.Count > 0 && fk.ReferencedColumns.Count > 0)
-                            {
-                                continue;
-                            }
+                        // 
+                        // Scenario 1: When a FK constraint is defined between source and target entities in the database.
+                        // In this case, there will be exactly one ForeignKeyDefinition with the right pair of Referencing and Referenced tables. 
+                        // Scenario 2: When no FK constraint is defined between source and target entities, but the relationship fields are configured through config file
+                        // In this case, two entries will be created. 
+                        // First entry: Referencing table: Source entity, Referenced table: Target entity
+                        // Second entry: Referencing table: Target entity, Referenced table: Source entity 
+                        List<ForeignKeyDefinition> validatedFKDefinitionsToTarget = GetValidatedFKs(fKDefinitionsToTarget);
+                        relationshipData.TargetEntityToFkDefinitionMap[targetEntityName] = validatedFKDefinitionsToTarget;
+                    }
+                }
+            }
+        }
 
-                            // Add the referencing and referenced columns for this foreign key definition
-                            // for the target.
-                            if (PairToFkDefinition is not null && PairToFkDefinition.TryGetValue(
-                                    fk.Pair, out ForeignKeyDefinition? inferredDefinition))
-                            {
-                                // Only add the referencing columns if they have not been
-                                // specified in the configuration file.
-                                if (fk.ReferencingColumns.Count == 0)
-                                {
-                                    fk.ReferencingColumns.AddRange(inferredDefinition.ReferencingColumns);
-                                }
+        /// <summary>
+        /// Loops over all the foreign key definitions defined for the target entity in the source entity's definition
+        /// and adds to the set of validated FK definitions:
+        /// 1. All the FK definitions which actually map to a foreign key constraint defined in the database.
+        /// In such a case, if the source/target fields are also provided in the config, they are given precedence over the FK constraint.
+        /// 2. FK definitions for custom relationships defined by the user in the configuration file where no FK constraint exists between
+        /// the pair of (source, target) entities.
+        /// </summary>
+        /// <param name="fKDefinitionsToTarget">List of FK definitions defined from source to target.</param>
+        /// <returns>List of validated FK definitions from source to target.</returns>
+        private List<ForeignKeyDefinition> GetValidatedFKs(
+            List<ForeignKeyDefinition> fKDefinitionsToTarget)
+        {
+            List<ForeignKeyDefinition> validatedFKDefinitionsToTarget = new();
+            foreach (ForeignKeyDefinition fKDefinitionToTarget in fKDefinitionsToTarget)
+            {
+                // This code block adds FK definitions between source and target entities when there is an FK constraint defined
+                // in the database, either from source->target or target->source entities or both.
 
-                                // Only add the referenced columns if they have not been
-                                // specified in the configuration file.
-                                if (fk.ReferencedColumns.Count == 0)
-                                {
-                                    fk.ReferencedColumns.AddRange(inferredDefinition.ReferencedColumns);
-                                }
-                            }
-                        }
+                // Add the referencing and referenced columns for this foreign key definition for the target.
+                if (PairToFkDefinition is not null &&
+                    PairToFkDefinition.TryGetValue(fKDefinitionToTarget.Pair, out ForeignKeyDefinition? inferredFKDefinition))
+                {
+                    // Being here indicates that we inferred an FK constraint for the current foreign key definition.
+                    // The count of referencing and referenced columns being > 0 indicates that source.fields and target.fields 
+                    // have been specified in the config file.
+                    // In this scenario, higher precedence is given to the fields configured through the config file. So, the existing FK definition is retained as is.
+                    if (fKDefinitionToTarget.ReferencingColumns.Count > 0 && fKDefinitionToTarget.ReferencedColumns.Count > 0)
+                    {
+                        validatedFKDefinitionsToTarget.Add(fKDefinitionToTarget);
+                    }
+                    // The count of referenced and referencing columns being = 0 indicates that source.fields and target.fields 
+                    // are not configured through the config file. In this case, the FK fields inferred from the database are populated.
+                    else
+                    {
+                        validatedFKDefinitionsToTarget.Add(inferredFKDefinition);
+                    }
+                }
+                else
+                {
+                    // This code block adds FK definitions between source and target entities when DAB hasn't yet identified an FK constraint.
+                    //
+                    // Being here indicates that we haven't yet found an FK constraint in the database for the current FK definition.
+                    // But this does not indicate absence of an FK constraint between the source, target entities yet.
+                    // This may happen when an FK constraint exists between two tables, but in an order opposite to the order
+                    // of referencing and referenced tables present in the current FK definition. This happens because for a relationship
+                    // with right cardinality as 1, we add FK definitions from both source->target and target->source to the source entity's definition.
+                    // because at that point we don't know if the relationship is an N:1 relationship or a 1:1 relationship.
+                    // So here, we need to remove the wrong FK definition for:
+                    // 1. N:1 relationships,
+                    // 2. 1:1 relationships where an FK constraint exists only from source->target or target->source but not both.
+                    //
+                    // E.g. for a relationship between Book-Publisher entities with cardinality 1, we would have added a Foreign key definition
+                    // from Book->Publisher and Publisher->Book to Book's source definition earlier.
+                    // Since it is an N:1 relationship, it might have been the case that the current FK definition had
+                    // 'publishers' table as the referencing table and 'books' table as the referenced table, and hence,
+                    // we did not find any FK constraint. But an FK constraint does exist where 'books' is the referencing table
+                    // while the 'publishers' is the referenced table.
+                    // (The definition for that constraint would be taken care of while adding database FKs above.)
+                    //
+                    // So, before concluding that there is no FK constraint between the source, target entities, we need
+                    // to confirm absence of FK constraint from source->target and target->source tables.
+                    RelationShipPair inverseFKPair = new(fKDefinitionToTarget.Pair.ReferencedDbTable, fKDefinitionToTarget.Pair.ReferencingDbTable);
+
+                    // Add FK definition to the set of validated FKs only if no FK constraint is defined for the source and target entities
+                    // in the database, either from source -> target or target -> source.
+                    // When a foreign key constraint is not identified using inverseFKPair and fKDefinitionToTarget.Pair, it means that
+                    // the FK constraint is only defined in the runtime config file.
+                    if (PairToFkDefinition is not null && !PairToFkDefinition.ContainsKey(inverseFKPair))
+                    {
+                        validatedFKDefinitionsToTarget.Add(fKDefinitionToTarget);
                     }
                 }
             }
+
+            return validatedFKDefinitionsToTarget;
         }
 
         /// <summary>

src/Service.Tests/Unittests/SqlMetadataProviderUnitTests.cs

@@ -6,6 +6,7 @@
 using System.IO;
 using System.Net;
 using System.Threading.Tasks;
+using Azure.DataApiBuilder.Config.DatabasePrimitives;
 using Azure.DataApiBuilder.Config.ObjectModel;
 using Azure.DataApiBuilder.Core.Authorization;
 using Azure.DataApiBuilder.Core.Configurations;
@@ -354,5 +355,131 @@ public void ValidateGraphQLReservedNaming_DatabaseColumns(string dbColumnName, s
                 actual: isViolationWithGraphQLGloballyDisabled,
                 message: "Unexpected failure. fieldName: " + dbColumnName + " | fieldMapping:" + mappedName);
         }
+
+        /// <summary>
+        /// Test to validate successful inference of relationship data based on data provided in the config and the metadata
+        /// collected from the MsSql database.
+        /// </summary>
+        [TestMethod, TestCategory(TestCategory.MSSQL)]
+        public async Task ValidateInferredRelationshipInfoForMsSql()
+        {
+            DatabaseEngine = TestCategory.MSSQL;
+            await SetupTestFixtureAndInferMetadata();
+            ValidateInferredRelationshipInfoForTables();
+        }
+
+        /// <summary>
+        /// Test to validate successful inference of relationship data based on data provided in the config and the metadata
+        /// collected from the MySql database.
+        /// </summary>
+        [TestMethod, TestCategory(TestCategory.MYSQL)]
+        public async Task ValidateInferredRelationshipInfoForMySql()
+        {
+            DatabaseEngine = TestCategory.MYSQL;
+            await SetupTestFixtureAndInferMetadata();
+            ValidateInferredRelationshipInfoForTables();
+        }
+
+        /// <summary>
+        /// Test to validate successful inference of relationship data based on data provided in the config and the metadata
+        /// collected from the PgSql database.
+        /// </summary>
+        [TestMethod, TestCategory(TestCategory.POSTGRESQL)]
+        public async Task ValidateInferredRelationshipInfoForPgSql()
+        {
+            DatabaseEngine = TestCategory.POSTGRESQL;
+            await SetupTestFixtureAndInferMetadata();
+            ValidateInferredRelationshipInfoForTables();
+        }
+
+        /// <summary>
+        /// Helper method for test methods ValidateInferredRelationshipInfoFor{MsSql, MySql, and PgSql}.
+        /// This helper validates that an entity's relationship data is correctly inferred based on config and database supplied relationship metadata.
+        /// Each test verifies that the referencing entity is correctly determined based on the FK constraints in the database.
+        /// </summary>
+        private static void ValidateInferredRelationshipInfoForTables()
+        {
+            // Validate that when for an 1:N relationship between Book - Review, an FK constraint
+            // exists from Review->Book.
+            // DAB determines that Review is the referencing entity during startup.
+            ValidateReferencingEntitiesForRelationship(
+                sourceEntityName: "Book",
+                targetEntityName: "Review",
+                expectedReferencingEntityNames: new List<string>() { "Review" });
+
+            // Validate that when for an 1:1 relationship between Stock - stocks_price, an FK constraint
+            // exists from stocks_price -> Stock.
+            // DAB determines that stocks_price is the referencing entity during startup.
+            ValidateReferencingEntitiesForRelationship(
+                sourceEntityName: "Stock",
+                targetEntityName: "stocks_price",
+                expectedReferencingEntityNames: new List<string>() { "stocks_price" });
+
+            // Validate that when for an N:1 relationship between Book - Publisher, an FK constraint
+            // exists from Book->Publisher.
+            // DAB determiens that Book is the referencing entity during startup.
+            ValidateReferencingEntitiesForRelationship(
+                sourceEntityName: "Book",
+                targetEntityName: "Publisher",
+                expectedReferencingEntityNames: new List<string>() { "Book" });
+        }
+
+        /// <summary>
+        /// Helper method to validate that for a given pair of source and target entities, DAB correctly infers the referencing entity/entities
+        /// during startup.
+        /// 1. For relationships backed by an FK, there is only one referencing entity.
+        /// 2. For relationships not backed by an FK, there are two referencing entities because
+        /// at startup, DAB can't determine which entity is the referencing entity. DAB can only determine the referecing entity
+        /// during request execution.
+        /// </summary>
+        /// <param name="sourceEntityName">Source entity name.</param>
+        /// <param name="targetEntityName">Target entity name.</param>
+        /// <param name="expectedReferencingEntityNames">List of expected referencing entity names.</param>
+        private static void ValidateReferencingEntitiesForRelationship(
+            string sourceEntityName,
+            string targetEntityName,
+            List<string> expectedReferencingEntityNames)
+        {
+            _sqlMetadataProvider.GetEntityNamesAndDbObjects().TryGetValue(sourceEntityName, out DatabaseObject sourceDbo);
+            _sqlMetadataProvider.GetEntityNamesAndDbObjects().TryGetValue(targetEntityName, out DatabaseObject targetDbo);
+            DatabaseTable sourceTable = (DatabaseTable)sourceDbo;
+            DatabaseTable targetTable = (DatabaseTable)targetDbo;
+            List<ForeignKeyDefinition> foreignKeys = sourceDbo.SourceDefinition.SourceEntityRelationshipMap[sourceEntityName].TargetEntityToFkDefinitionMap[targetEntityName];
+            HashSet<DatabaseTable> expectedReferencingTables = new();
+            HashSet<DatabaseTable> actualReferencingTables = new();
+            foreach (string referencingEntityName in expectedReferencingEntityNames)
+            {
+                DatabaseTable referencingTable = referencingEntityName.Equals(sourceEntityName) ? sourceTable : targetTable;
+                expectedReferencingTables.Add(referencingTable);
+            }
+
+            foreach (ForeignKeyDefinition foreignKey in foreignKeys)
+            {
+                if (foreignKey.ReferencedColumns.Count == 0)
+                {
+                    continue;
+                }
+
+                DatabaseTable actualReferencingTable = foreignKey.Pair.ReferencingDbTable;
+                actualReferencingTables.Add(actualReferencingTable);
+            }
+
+            Assert.IsTrue(actualReferencingTables.SetEquals(expectedReferencingTables));
+        }
+
+        /// <summary>
+        /// Resets the database state and infers metadata for all the entities exposed in the config.
+        /// The `ResetDbStateAsync()` method executes the .sql script of the respective database type and
+        /// serves as a setup phase for this test. 
+        /// </summary>
+        private static async Task SetupTestFixtureAndInferMetadata()
+        {
+            TestHelper.SetupDatabaseEnvironment(DatabaseEngine);
+            RuntimeConfig runtimeConfig = SqlTestHelper.SetupRuntimeConfig();
+            RuntimeConfigProvider runtimeConfigProvider = TestHelper.GenerateInMemoryRuntimeConfigProvider(runtimeConfig);
+            SetUpSQLMetadataProvider(runtimeConfigProvider);
+            await ResetDbStateAsync();
+            await _sqlMetadataProvider.InitializeAsync();
+        }
     }
 }