[PORT] Do not create Index-Operations with the same name as ForeignKey constraints (#115)

* In contrast to SQL Standard, MS Access will create an index together with a the FK constrains (#114)

According to https://docs.microsoft.com/en-us/office/client-developer/access/desktop-database-reference/constraint-clause-microsoft-access-sql
this can be deactivated, however creating an index with the same name as an FK still results in an runtime error, therefor the index creation operation is skipped

Co-authored-by: Michael Steinecke <m.p.steinecke@gmail.com>

* Update to 5.0 and cleanup code and comments.

Co-authored-by: Michael Steinecke <6099045+MichaelSteinecke@users.noreply.github.com>
Co-authored-by: Michael Steinecke <m.p.steinecke@gmail.com>

src/EFCore.Jet/Diagnostics/Internal/JetLoggingDefinitions.cs

@@ -116,6 +116,14 @@ namespace EntityFrameworkCore.Jet.Diagnostics.Internal
         /// </summary>
         public EventDefinitionBase LogFoundIndex;
 
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        public EventDefinitionBase LogSkippedIndex;
+
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.Jet/Diagnostics/JetEventId.cs

@@ -62,7 +62,8 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics
             IndexFound,
             ForeignKeyFound,
             ForeignKeyPrincipalColumnMissingWarning,
-            ReflexiveConstraintIgnored
+            ReflexiveConstraintIgnored,
+            IndexSkipped,
         }
 
         private static readonly string _validationPrefix = DbLoggerCategory.Model.Validation.Name + ".";
@@ -212,6 +213,12 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics
         /// </summary>
         public static readonly EventId IndexFound = MakeScaffoldingId(Id.IndexFound);
 
+        /// <summary>
+        ///     An index was skipped.
+        ///     This event is in the <see cref="DbLoggerCategory.Scaffolding" /> category.
+        /// </summary>
+        public static readonly EventId IndexSkipped = MakeScaffoldingId(Id.IndexSkipped);
+
         /// <summary>
         ///     A foreign key was found.
         ///     This event is in the <see cref="DbLoggerCategory.Scaffolding" /> category.

src/EFCore.Jet/Internal/JetLoggerExtensions.cs

@@ -307,6 +307,28 @@ namespace EntityFrameworkCore.Jet.Internal
             // No DiagnosticsSource events because these are purely design-time messages
         }
 
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        public static void IndexSkipped(
+            [NotNull] this IDiagnosticsLogger<DbLoggerCategory.Scaffolding> diagnostics,
+            [NotNull] string indexName,
+            [NotNull] string tableName,
+            bool unique)
+        {
+            var definition = JetResources.LogFoundIndex(diagnostics);
+
+            if (diagnostics.ShouldLog(definition))
+            {
+                definition.Log(diagnostics, indexName, tableName, unique);
+            }
+
+            // No DiagnosticsSource events because these are purely design-time messages
+        }
+
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.Jet/Properties/JetStrings.Designer.cs

@@ -593,6 +593,29 @@ namespace EntityFrameworkCore.Jet.Internal
             return (EventDefinition<string, string, bool>)definition;
         }
         
+        /// <summary>
+        ///     Skipped index with name: {indexName}, table: {tableName}, is unique: {isUnique}.
+        /// </summary>
+        public static EventDefinition<string, string, bool> LogSkippedIndex([NotNull] IDiagnosticsLogger logger)
+        {
+            var definition = ((Diagnostics.Internal.JetLoggingDefinitions)logger.Definitions).LogSkippedIndex;
+            if (definition == null)
+            {
+                definition = LazyInitializer.EnsureInitialized<EventDefinitionBase>(
+                    ref ((Diagnostics.Internal.JetLoggingDefinitions)logger.Definitions).LogSkippedIndex,
+                    () => new EventDefinition<string, string, bool>(
+                        logger.Options,
+                        JetEventId.IndexSkipped,
+                        LogLevel.Debug,
+                        "JetEventId.SkippedIndex",
+                        level => LoggerMessage.Define<string, string, bool>(
+                            level,
+                            JetEventId.IndexFound,
+                            _resourceManager.GetString("LogSkippedIndex"))));
+            }
+
+            return (EventDefinition<string, string, bool>)definition;
+        }
         /// <summary>
         ///     Found primary key with name: {primaryKeyName}, table: {tableName}.
         /// </summary>

src/EFCore.Jet/Properties/JetStrings.resx

@@ -207,6 +207,10 @@
     <value>Found index with name: {indexName}, table: {tableName}, is unique: {isUnique}.</value>
     <comment>Debug JetEventId.IndexFound string string bool</comment>
   </data>
+  <data name="LogSkippedIndex" xml:space="preserve">
+    <value>Skipped index with name: {indexName}, table: {tableName}, is unique: {isUnique} as the name is equal to an existing foreign key and would result in a runtime error.</value>
+    <comment>Debug JetEventId.IndexSkipped string string bool</comment>
+  </data>
   <data name="LogFoundPrimaryKey" xml:space="preserve">
     <value>Found primary key with name: {primaryKeyName}, table: {tableName}.</value>
     <comment>Debug JetEventId.PrimaryKeyFound string string</comment>

src/EFCore.Jet/Scaffolding/Internal/JetDatabaseModelFactory.cs

@@ -168,8 +168,8 @@ namespace EntityFrameworkCore.Jet.Scaffolding.Internal
             if (tables.Count > 0)
             {
                 GetColumns(connection, tables);
-                GetIndexes(connection, tables);
                 GetRelations(connection, tables);
+                GetIndexes(connection, tables);
             }
 
             return tables;
@@ -372,7 +372,11 @@ namespace EntityFrameworkCore.Jet.Scaffolding.Internal
                             table.PrimaryKey = primaryKey;
                             indexOrKey = primaryKey;
                         }
-                        else if (indexType == "UNIQUE" &&
+                        else
+                        {
+                            var isUnique = indexType == "UNIQUE";
+
+                            if (isUnique &&
                                 !nullable)
                             {
                                 var uniqueConstraint = new DatabaseUniqueConstraint
@@ -388,11 +392,23 @@ namespace EntityFrameworkCore.Jet.Scaffolding.Internal
                             }
                             else
                             {
+                                // In contrast to SQL Standard, MS Access will implicitly create an index for every FK
+                                // constraint.
+                                // According to https://docs.microsoft.com/en-us/office/client-developer/access/desktop-database-reference/constraint-clause-microsoft-access-sql,
+                                // this behavior can be disabled, but manuall creating an index with the same name as an
+                                // FK would still results in a runtime error.
+                                // We therefore skip indexes with the same name as existing FKs. 
+                                if (table.ForeignKeys.Any(fk => fk.Name == indexName))
+                                {
+                                    _logger.IndexSkipped(indexName, tableName, isUnique);
+                                    continue;
+                                }
+                            
                                 var index = new DatabaseIndex
                                 {
                                     Table = table,
                                     Name = indexName,
-                                IsUnique = indexType == "UNIQUE",
+                                    IsUnique = isUnique,
                                 };
 
                                 _logger.IndexFound(indexName, tableName, index.IsUnique);
@@ -400,6 +416,7 @@ namespace EntityFrameworkCore.Jet.Scaffolding.Internal
                                 table.Indexes.Add(index);
                                 indexOrKey = index;
                             }
+                        }
 
                         foreach (var indexColumn in indexColumns)
                         {

test/EFCore.Jet.FunctionalTests/Scaffolding/JetDatabaseModelFactoryTest.cs

@@ -1476,6 +1476,45 @@ DROP TABLE DependentTable;
 DROP TABLE PrincipalTable;");
         }
         
+        /// <summary>
+        /// In contrast to SQL Standard, MS Access will implicitly create an index for every FK constraint.
+        /// According to https://docs.microsoft.com/en-us/office/client-developer/access/desktop-database-reference/constraint-clause-microsoft-access-sql,
+        /// this behavior can be disabled, but manuall creating an index with the same name as an FK would still results
+        /// in a runtime error.
+        /// We therefore skip indexes with the same name as existing FKs. 
+        /// </summary>
+        [ConditionalFact]
+        public void EnsureNoForeignKeyIndexCreationOperationsAreCreated()
+        {
+            Test(
+                @"
+CREATE TABLE PrincipalTable (
+    Id int PRIMARY KEY
+);
+
+CREATE TABLE DependentTable (
+    Id int PRIMARY KEY,
+    IndexColumn int,
+    ForeignKeyId int,
+    CONSTRAINT MYFK FOREIGN KEY (ForeignKeyId) REFERENCES PrincipalTable(Id) ON DELETE CASCADE
+);
+    CREATE INDEX `IX_DependentTable_IndexColumn` ON `DependentTable` (`IndexColumn`);
+",
+                Enumerable.Empty<string>(),
+                Enumerable.Empty<string>(),
+                dbModel =>
+                {
+                    var table = dbModel.Tables.Single(t => t.Name == "DependentTable");
+                    
+                    Assert.Equal(1, table.ForeignKeys.Count);
+                    Assert.Equal(1, table.Indexes.Count);
+                    Assert.False(table.Indexes.Any(i => i.Name == "MYFK"));
+                },
+                @"
+DROP TABLE DependentTable;
+DROP TABLE PrincipalTable;");
+        }
+        
         [ConditionalFact]
         public void Set_referential_action_for_foreign_key()
         {