Enforce the dependent requiredness for table splitting more strictly

Remove discriminator column if all derived types ignored Fix table splitting tests Fixes #10962 Fixes #10971
dotnet · Feb 23, 2018 · 3ab14a2 · 3ab14a2
1 parent 866cd35
commit 3ab14a2
Show file tree

Hide file tree

Showing 35 changed files with 415 additions and 179 deletions.
diff --git a/src/EFCore.Relational.Specification.Tests/Query/RelationalOwnedQueryTestBase.cs b/src/EFCore.Relational.Specification.Tests/Query/RelationalOwnedQueryTestBase.cs
@@ -0,0 +1,28 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Microsoft.EntityFrameworkCore.Diagnostics;
+using Microsoft.EntityFrameworkCore.TestUtilities;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.EntityFrameworkCore.Query
+{
+    public abstract class RelationalOwnedQueryTestBase<TFixture> : OwnedQueryTestBase<TFixture>
+        where TFixture : RelationalOwnedQueryTestBase<TFixture>.RelationalOwnedQueryFixture, new()
+    {
+        protected RelationalOwnedQueryTestBase(TFixture fixture)
+            : base(fixture)
+        {
+            fixture.TestSqlLoggerFactory.Clear();
+        }
+
+        public abstract class RelationalOwnedQueryFixture : OwnedQueryFixtureBase
+        {
+            public TestSqlLoggerFactory TestSqlLoggerFactory => (TestSqlLoggerFactory)ServiceProvider.GetRequiredService<ILoggerFactory>();
+
+            public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+                => base.AddOptions(builder).ConfigureWarnings(c => c.Log(RelationalEventId.QueryClientEvaluationWarning));
+        }
+    }
+}
diff --git a/src/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs b/src/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Linq;
 using Microsoft.EntityFrameworkCore.Diagnostics;
+using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.TestModels.TransportationModel;
 using Microsoft.EntityFrameworkCore.TestUtilities;
 using Microsoft.Extensions.DependencyInjection;
@@ -28,19 +29,35 @@ public virtual void Can_query_shared()
             {
                 using (var context = CreateContext())
                 {
-                    Assert.Equal(4, context.Set<Operator>().ToList().Count);
+                    Assert.Equal(5, context.Set<Operator>().ToList().Count);
                 }
             }
         }
 
-        [Fact(Skip = "#10791 Incorrect data")]
-        public virtual void Can_query_shared_derived()
+        [Fact]
+        public virtual void Can_query_shared_derived_hierarchy()
         {
             using (CreateTestStore(OnModelCreating))
             {
                 using (var context = CreateContext())
                 {
-                    Assert.Equal(1, context.Set<FuelTank>().ToList().Count);
+                    Assert.Equal(2, context.Set<FuelTank>().ToList().Count);
+                }
+            }
+        }
+
+        [Fact]
+        public virtual void Can_query_shared_derived_nonhierarchy()
+        {
+            using (CreateTestStore(modelBuilder =>
+            {
+                OnModelCreating(modelBuilder);
+                modelBuilder.Ignore<SolidFuelTank>();
+            }))
+            {
+                using (var context = CreateContext())
+                {
+                    Assert.Equal(2, context.Set<FuelTank>().ToList().Count);
                 }
             }
         }
@@ -51,18 +68,20 @@ public virtual void Can_use_with_redundant_relationships()
             Test_roundtrip(OnModelCreating);
         }
 
-        [Fact]
+        // #9005
+        // [Fact]
         public virtual void Can_use_with_chained_relationships()
         {
             Test_roundtrip(
                 modelBuilder =>
                     {
                         OnModelCreating(modelBuilder);
-                        modelBuilder.Entity<FuelTank>(eb => { eb.Ignore(e => e.Vehicle); });
+                        //modelBuilder.Entity<FuelTank>(eb => { eb.Ignore(e => e.Vehicle); });
                     });
         }
 
-        [Fact(Skip = "Issue#10971")]
+        // #9005
+        // [Fact]
         public virtual void Can_use_with_fanned_relationships()
         {
             Test_roundtrip(
@@ -85,6 +104,33 @@ protected void Test_roundtrip(Action<ModelBuilder> onModelCreating)
             }
         }
 
+        [Fact]
+        public virtual void Inserting_dependent_with_just_one_parent_throws()
+        {
+            using (CreateTestStore(OnModelCreating))
+            {
+                using (var context = CreateContext())
+                {
+                    context.Add(new PoweredVehicle
+                    {
+                        Name = "Fuel transport",
+                        SeatingCapacity = 1,
+                        Operator = new LicensedOperator { Name = "Jack Jackson", LicenseType = "Class A CDC" }
+                    });
+                    context.Add(new FuelTank
+                    {
+                        Capacity = "10000 l",
+                        FuelType = "Gas",
+                        VehicleName = "Fuel transport"
+                    });
+
+                    Assert.Equal(RelationalStrings.SharedRowEntryCountMismatchSensitive(
+                        nameof(PoweredVehicle), "Vehicles", nameof(Engine), "{Name: Fuel transport}", "Added"),
+                        Assert.Throws<InvalidOperationException>(() => context.SaveChanges()).Message);
+                }
+            }
+        }
+
         [Fact]
         public virtual void Can_change_dependent_instance_non_derived()
         {

diff --git a/src/EFCore.Relational/Metadata/Conventions/Internal/RelationalConventionSetBuilder.cs b/src/EFCore.Relational/Metadata/Conventions/Internal/RelationalConventionSetBuilder.cs
@@ -50,9 +50,11 @@ public virtual ConventionSet AddConventions(ConventionSet conventionSet)
 
             var sharedTableConvention = new SharedTableConvention();
 
+            var discriminatorConvention =new DiscriminatorConvention();
             conventionSet.EntityTypeAddedConventions.Add(new RelationalTableAttributeConvention());
             conventionSet.EntityTypeAddedConventions.Add(sharedTableConvention);
-            conventionSet.BaseEntityTypeChangedConventions.Add(new DiscriminatorConvention());
+            conventionSet.EntityTypeRemovedConventions.Add(discriminatorConvention);
+            conventionSet.BaseEntityTypeChangedConventions.Add(discriminatorConvention);
             conventionSet.BaseEntityTypeChangedConventions.Add(
                 new TableNameFromDbSetConvention(Dependencies.Context?.Context, Dependencies.SetFinder));
             conventionSet.EntityTypeAnnotationChangedConventions.Add(sharedTableConvention);

diff --git a/src/EFCore.Relational/Metadata/Internal/TableMapping.cs b/src/EFCore.Relational/Metadata/Internal/TableMapping.cs
@@ -50,11 +50,14 @@ public TableMapping(
         ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
         ///     directly from your code. This API may change or be removed in future releases.
         /// </summary>
-        public virtual IEnumerable<IEntityType> GetRootTypes()
-            => EntityTypes.Where(
+        public virtual IEntityType GetRootType()
+            => EntityTypes.SingleOrDefault(
                 t => t.BaseType == null
-                    && t.FindForeignKeys(t.FindDeclaredPrimaryKey().Properties)
-                        .All(fk => t.Relational().TableName != fk.PrincipalEntityType.Relational().TableName));
+                     && t.FindForeignKeys(t.FindDeclaredPrimaryKey().Properties)
+                         .All(fk => !fk.PrincipalKey.IsPrimaryKey()
+                                    || fk.PrincipalEntityType.RootType() == t
+                                    || t.Relational().TableName != fk.PrincipalEntityType.Relational().TableName
+                                    || t.Relational().Schema != fk.PrincipalEntityType.Relational().Schema));
 
         /// <summary>
         ///     This API supports the Entity Framework Core infrastructure and is not intended to be used

diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
@@ -492,7 +492,7 @@ protected virtual IEnumerable<MigrationOperation> Diff(
                     s.Name,
                     t.Name,
                     StringComparison.OrdinalIgnoreCase),
-                (s, t, c) => s.GetRootTypes().Any(se => t.GetRootTypes().Any(te => string.Equals(se.Name, te.Name, StringComparison.OrdinalIgnoreCase))),
+                (s, t, c) => string.Equals(s.GetRootType().Name, t.GetRootType().Name, StringComparison.OrdinalIgnoreCase),
                 (s, t, c) => s.EntityTypes.Any(se => t.EntityTypes.Any(te => string.Equals(se.Name, te.Name, StringComparison.OrdinalIgnoreCase))));
 
         /// <summary>
@@ -605,8 +605,7 @@ protected virtual IEnumerable<MigrationOperation> Remove(
         }
 
         private static IEnumerable<IProperty> GetSortedProperties(TableMapping target)
-            => target.GetRootTypes()
-                .SelectMany(GetSortedProperties)
+            => GetSortedProperties(target.GetRootType())
                 .Distinct((x, y) => x.Relational().ColumnName == y.Relational().ColumnName);
 
         private static IEnumerable<IProperty> GetSortedProperties(IEntityType entityType)
@@ -642,7 +641,7 @@ private static IEnumerable<IProperty> GetSortedProperties(IEntityType entityType
 
                 var clrType = clrProperty.DeclaringType;
                 var index = clrType.GetTypeInfo().DeclaredProperties
-                    .IndexOf(clrProperty, new PropertyInfoEuqlityComparer());
+                    .IndexOf(clrProperty, PropertyInfoEqualityComparer.Instance);
 
                 types.GetOrAddNew(clrType)[index] = clrProperty;
             }
@@ -657,7 +656,7 @@ private static IEnumerable<IProperty> GetSortedProperties(IEntityType entityType
                     && fk.DeclaringEntityType.Relational().TableName == entityType.Relational().TableName
                     && fk == fk.DeclaringEntityType
                         .FindForeignKey(
-                            fk.DeclaringEntityType.FindDeclaredPrimaryKey().Properties,
+                            fk.DeclaringEntityType.FindPrimaryKey().Properties,
                             entityType.FindPrimaryKey(),
                             entityType)))
             {
@@ -674,7 +673,7 @@ private static IEnumerable<IProperty> GetSortedProperties(IEntityType entityType
 
                 var clrType = clrProperty.DeclaringType;
                 var index = clrType.GetTypeInfo().DeclaredProperties
-                    .IndexOf(clrProperty, new PropertyInfoEuqlityComparer());
+                    .IndexOf(clrProperty, PropertyInfoEqualityComparer.Instance);
 
                 types.GetOrAddNew(clrType)[index] = clrProperty;
             }
@@ -711,8 +710,14 @@ private static IEnumerable<IProperty> GetSortedProperties(IEntityType entityType
                 .Concat(entityType.GetDirectlyDerivedTypes().SelectMany(GetSortedProperties));
         }
 
-        private class PropertyInfoEuqlityComparer : IEqualityComparer<PropertyInfo>
+        private class PropertyInfoEqualityComparer : IEqualityComparer<PropertyInfo>
         {
+            private PropertyInfoEqualityComparer()
+            {
+            }
+
+            public static readonly PropertyInfoEqualityComparer Instance = new PropertyInfoEqualityComparer();
+
             public bool Equals(PropertyInfo x, PropertyInfo y)
                 => x.IsSameAs(y);
 
@@ -805,9 +810,9 @@ private static string GetDefiningNavigationName(IEntityType entityType)
             var primaryKey = entityType.FindDeclaredPrimaryKey();
             if (primaryKey != null)
             {
-                var definingForeignKey = entityType.FindForeignKeys(primaryKey.Properties)
-                    .Where(fk => fk.PrincipalEntityType.Relational().TableName == entityType.Relational().TableName)
-                    .FirstOrDefault();
+                var definingForeignKey = entityType
+                    .FindForeignKeys(primaryKey.Properties)
+                    .FirstOrDefault(fk => fk.PrincipalEntityType.Relational().TableName == entityType.Relational().TableName);
                 if (definingForeignKey?.DependentToPrincipal != null)
                 {
                     return definingForeignKey.DependentToPrincipal.Name;

diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs
diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx
@@ -350,10 +350,10 @@
     <value>The instance of entity type '{firstEntityType}' and the instance of entity type '{secondEntityType}' are mapped to the same row with the key value '{keyValue}', but have different original property values '{firstConflictingValues}' and '{secondConflictingValues}' mapped to {columns}.</value>
   </data>
   <data name="SharedRowEntryCountMismatch" xml:space="preserve">
-    <value>The entity of '{entityType}' is sharing the table '{tableName}' with '{missingEntityType}', but there is no entity of this type with the same key value that has been marked as '{state}'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values.</value>
+    <value>The entity of type '{entityType}' is sharing the table '{tableName}' with entities of type '{missingEntityType}', but there is no entity of this type with the same key value that has been marked as '{state}'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values.</value>
   </data>
   <data name="SharedRowEntryCountMismatchSensitive" xml:space="preserve">
-    <value>The entity of '{entityType}' is sharing the table '{tableName}' with '{missingEntityType}', but there is no entity of this type with the same key value '{keyValue}' that has been marked as '{state}'.</value>
+    <value>The entity of type '{entityType}' is sharing the table '{tableName}' with entities of type '{missingEntityType}', but there is no entity of this type with the same key value '{keyValue}' that has been marked as '{state}'.</value>
   </data>
   <data name="IncorrectDefaultValueType" xml:space="preserve">
     <value>Cannot set default value '{value}' of type '{valueType}' on property '{property}' of type '{propertyType}' in entity type '{entityType}'.</value>

diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs
@@ -200,15 +200,12 @@ private void Validate(Dictionary<(string Schema, string Name), SharedTableEntryM
             {
                 foreach (var command in modificationCommandIdentityMap.Values)
                 {
-                    if ((command.EntityState != EntityState.Added
+                    if (command.EntityState != EntityState.Added
                          && command.EntityState != EntityState.Deleted)
-                        || (command.Entries.Any(e => modificationCommandIdentityMap.GetPrincipals(e.EntityType).Count == 0)
-                            && command.Entries.Any(e => modificationCommandIdentityMap.GetDependents(e.EntityType).Count == 0)))
                     {
                         continue;
                     }
 
-                    var tableName = (string.IsNullOrEmpty(command.Schema) ? "" : command.Schema + ".") + command.TableName;
                     foreach (var entry in command.Entries)
                     {
                         foreach (var principalEntityType in modificationCommandIdentityMap.GetPrincipals(entry.EntityType))
@@ -217,6 +214,7 @@ private void Validate(Dictionary<(string Schema, string Name), SharedTableEntryM
                                 principalEntry => principalEntry != entry
                                                   && principalEntityType.IsAssignableFrom(principalEntry.EntityType)))
                             {
+                                var tableName = (string.IsNullOrEmpty(command.Schema) ? "" : command.Schema + ".") + command.TableName;
                                 if (_sensitiveLoggingEnabled)
                                 {
                                     throw new InvalidOperationException(
@@ -243,6 +241,7 @@ private void Validate(Dictionary<(string Schema, string Name), SharedTableEntryM
                                 dependentEntry => dependentEntry != entry
                                                   && dependentEntityType.IsAssignableFrom(dependentEntry.EntityType)))
                             {
+                                var tableName = (string.IsNullOrEmpty(command.Schema) ? "" : command.Schema + ".") + command.TableName;
                                 if (_sensitiveLoggingEnabled)
                                 {
                                     throw new InvalidOperationException(