Add more tests for keyless entity types

Fixes #11802
dotnet · Nov 9, 2020 · 2771595 · 2771595
1 parent fbc43f6
commit 2771595
Show file tree

Hide file tree

Showing 10 changed files with 111 additions and 79 deletions.
diff --git a/src/EFCore.Relational/Extensions/RelationalQueryableExtensions.cs b/src/EFCore.Relational/Extensions/RelationalQueryableExtensions.cs
@@ -152,7 +152,7 @@ private static FromSqlQueryRootExpression GenerateFromSqlQueryRoot(
             IQueryable source,
             string sql,
             object[] arguments,
-            [CallerMemberName] string memberName = null)
+            [CallerMemberName] string memberName = null!)
         {
             var queryRootExpression = (QueryRootExpression)source.Expression;
 

diff --git a/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
@@ -2744,6 +2744,45 @@ private InternalForeignKeyBuilder HasRelationship(
                     Metadata, null, navigationToTarget, !setTargetAsPrincipal, configurationSource, required);
             }
 
+            if (configurationSource == ConfigurationSource.Explicit
+                && setTargetAsPrincipal.HasValue)
+            {
+                if (setTargetAsPrincipal.Value)
+                {
+                    if (targetEntityType.IsKeyless
+                        && targetEntityType.GetIsKeylessConfigurationSource() == ConfigurationSource.Explicit)
+                    {
+                        throw new InvalidOperationException(CoreStrings.PrincipalKeylessType(
+                            targetEntityType.DisplayName(),
+                            targetEntityType.DisplayName()
+                            + (inverseNavigation == null
+                                ? ""
+                                : "." + inverseNavigation.Value.Name),
+                            Metadata.DisplayName()
+                            + (navigationToTarget == null
+                                ? ""
+                                : "." + navigationToTarget.Value.Name)));
+                    }
+                }
+                else
+                {
+                    if (Metadata.IsKeyless
+                        && Metadata.GetIsKeylessConfigurationSource() == ConfigurationSource.Explicit)
+                    {
+                        throw new InvalidOperationException(CoreStrings.PrincipalKeylessType(
+                            Metadata.DisplayName(),
+                            Metadata.DisplayName()
+                            + (navigationToTarget == null
+                                ? ""
+                                : "." + navigationToTarget.Value.Name),
+                            targetEntityType.DisplayName()
+                            + (inverseNavigation == null
+                                ? ""
+                                : "." + inverseNavigation.Value.Name)));
+                    }
+                }
+            }
+
             var existingRelationship = InternalForeignKeyBuilder.FindCurrentForeignKeyBuilder(
                 targetEntityType,
                 Metadata,
@@ -2908,6 +2947,11 @@ private InternalForeignKeyBuilder HasRelationship(
                     }
 
                     relationship = newRelationship;
+
+                    if (relationship == null)
+                    {
+                        return null;
+                    }
                 }
 
                 if (setTargetAsPrincipal == true)
@@ -3616,7 +3660,12 @@ private ForeignKey SetOrAddForeignKey(
                         1, null, new[] { typeof(int) }, new[] { "TempId" }, isRequired: true, baseName: "").Item2;
 
                     principalKey = principalBaseEntityTypeBuilder.HasKeyInternal(
-                        principalKeyProperties, ConfigurationSource.Convention).Metadata;
+                        principalKeyProperties, ConfigurationSource.Convention)?.Metadata;
+
+                    if (principalKey == null)
+                    {
+                        return null;
+                    }
                 }
 
                 if (foreignKey != null)

diff --git a/src/EFCore/Metadata/Internal/SkipNavigation.cs b/src/EFCore/Metadata/Internal/SkipNavigation.cs
@@ -166,7 +166,10 @@ public virtual EntityType? JoinEntityType
             var oldForeignKey = ForeignKey;
             var isChanging = foreignKey != ForeignKey;
 
-            oldForeignKey?.ReferencingSkipNavigations!.Remove(this);
+            if (oldForeignKey != null)
+            {
+                oldForeignKey.ReferencingSkipNavigations!.Remove(this);
+            }
 
             if (foreignKey == null)
             {

diff --git a/test/EFCore.Cosmos.Tests/ModelBuilding/CosmosModelBuilderGenericTest.cs b/test/EFCore.Cosmos.Tests/ModelBuilding/CosmosModelBuilderGenericTest.cs
@@ -214,11 +214,5 @@ public class CosmosGenericOwnedTypes : GenericOwnedTypes
             protected override TestModelBuilder CreateModelBuilder()
                 => CreateTestModelBuilder(CosmosTestHelpers.Instance);
         }
-
-        public class CosmosGenericKeylessEntities : GenericKeylessEntities
-        {
-            protected override TestModelBuilder CreateModelBuilder()
-                => CreateTestModelBuilder(CosmosTestHelpers.Instance);
-        }
     }
 }
diff --git a/test/EFCore.InMemory.Tests/ModelBuilding/InMemoryModelBuilderGenericTest.cs b/test/EFCore.InMemory.Tests/ModelBuilding/InMemoryModelBuilderGenericTest.cs
@@ -241,11 +241,5 @@ public class InMemoryGenericOwnedTypes : GenericOwnedTypes
             protected override TestModelBuilder CreateModelBuilder()
                 => CreateTestModelBuilder(InMemoryTestHelpers.Instance);
         }
-
-        public class InMemoryGenericKeylessEntities : GenericKeylessEntities
-        {
-            protected override TestModelBuilder CreateModelBuilder()
-                => CreateTestModelBuilder(InMemoryTestHelpers.Instance);
-        }
     }
 }
diff --git a/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs b/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs
@@ -671,11 +671,5 @@ public override void Can_configure_owned_type_key()
             protected override TestModelBuilder CreateModelBuilder()
                 => CreateTestModelBuilder(SqlServerTestHelpers.Instance);
         }
-
-        public class SqlServerGenericKeylessEntities : GenericKeylessEntities
-        {
-            protected override TestModelBuilder CreateModelBuilder()
-                => CreateTestModelBuilder(SqlServerTestHelpers.Instance);
-        }
     }
 }
diff --git a/test/EFCore.Tests/ModelBuilding/KeylessEntitiesTestBase.cs b/test/EFCore.Tests/ModelBuilding/KeylessEntitiesTestBase.cs
diff --git a/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs b/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs
@@ -88,12 +88,6 @@ protected override TestModelBuilder CreateTestModelBuilder(TestHelpers testHelpe
                 => new GenericTestModelBuilder(testHelpers);
         }
 
-        public class GenericKeylessEntities : KeylessEntitiesTestBase
-        {
-            protected override TestModelBuilder CreateTestModelBuilder(TestHelpers testHelpers)
-                => new GenericTestModelBuilder(testHelpers);
-        }
-
         public class GenericOneToMany : OneToManyTestBase
         {
             protected override TestModelBuilder CreateTestModelBuilder(TestHelpers testHelpers)

diff --git a/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericTest.cs b/test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericTest.cs
@@ -49,12 +49,6 @@ protected override TestModelBuilder CreateTestModelBuilder(TestHelpers testHelpe
                 => new NonGenericTestModelBuilder(testHelpers);
         }
 
-        public class NonGenericKeylessEntities : KeylessEntitiesTestBase
-        {
-            protected override TestModelBuilder CreateTestModelBuilder(TestHelpers testHelpers)
-                => new NonGenericTestModelBuilder(testHelpers);
-        }
-
         public class NonGenericOneToMany : OneToManyTestBase
         {
 

diff --git a/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs b/test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs
@@ -908,6 +908,62 @@ public virtual void Can_use_non_PK_principal()
                 Assert.Empty(principalType.GetIndexes());
             }
 
+            [ConditionalFact]
+            public virtual void Throws_on_keyless_type_as_principal()
+            {
+                var modelBuilder = CreateModelBuilder();
+
+                modelBuilder.Ignore<CustomerDetails>();
+
+                Assert.Equal(
+                    CoreStrings.PrincipalKeylessType(
+                            nameof(Customer),
+                            nameof(Customer) + "." + nameof(Customer.Orders),
+                            nameof(Order)),
+                    Assert.Throws<InvalidOperationException>(() =>
+                        modelBuilder.Entity<Customer>().HasNoKey()
+                            .HasMany(c => c.Orders)
+                            .WithOne(o => o.Customer)).Message);
+            }
+
+            [ConditionalFact]
+            public virtual void Keyless_type_with_unmapped_collection_navigations_does_not_throw()
+            {
+                var modelBuilder = CreateModelBuilder();
+
+                modelBuilder.Ignore<CustomerDetails>();
+
+                modelBuilder.Entity<Customer>().HasNoKey();
+
+                var model = modelBuilder.FinalizeModel();
+
+                var customer = model.FindEntityType(typeof(Customer));
+                Assert.Empty(customer.GetNavigations());
+                Assert.Null(customer.FindPrimaryKey());
+                Assert.Null(model.FindEntityType(typeof(Order)));
+            }
+
+            [ConditionalFact]
+            public virtual void Keyless_type_discovered_before_referenced_entity_type_does_not_leave_temp_id()
+            {
+                var modelBuilder = CreateModelBuilder();
+
+                modelBuilder.Ignore<Order>();
+                modelBuilder.Ignore<CustomerDetails>();
+
+                modelBuilder.Entity<KeylessEntity>().HasNoKey();
+                modelBuilder.Entity<Customer>();
+
+                var model = modelBuilder.FinalizeModel();
+
+                var keyless = model.FindEntityType(typeof(KeylessEntity));
+                Assert.Null(model.FindEntityType(typeof(Customer))?.FindProperty("TempId"));
+                Assert.Null(keyless.FindPrimaryKey());
+
+                var customerNavigation = keyless.GetNavigations().Single();
+                Assert.Same(keyless, customerNavigation.ForeignKey.DeclaringEntityType);
+            }
+
             [ConditionalFact]
             public virtual void Can_have_both_convention_properties_specified()
             {