From 288ce2ff271bdde6690a352ab49df633019ac0e4 Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Mon, 26 Jun 2017 14:16:17 -0700 Subject: [PATCH] Fix table splitting for derived types in update pipeline. Make the columns nullable for dependent types involved in table splitting. When uniquifying column names use the declaring entity type name as prefix. Fixes #8907 --- .../TableSplittingTestBase.cs | 106 ++++++++++++ .../RelationalModelValidator.cs | 74 +++++---- .../Internal/SharedTableConvention.cs | 35 ++-- .../Metadata/RelationalPropertyAnnotations.cs | 21 +-- .../Metadata/RelationalPropertyExtensions.cs | 22 ++- .../Properties/RelationalStrings.Designer.cs | 16 +- .../Properties/RelationalStrings.resx | 58 +++---- .../Update/Internal/CommandBatchPreparer.cs | 34 ++-- .../ModificationCommandIdentityMap.cs | 83 ++++++---- .../Query/OwnedQueryTestBase.cs | 4 +- .../TransportationModel/CombustionEngine.cs | 17 ++ .../TestModels/TransportationModel/Engine.cs | 19 +++ .../TransportationModel/FuelTank.cs | 25 +++ .../TransportationModel/LicensedOperator.cs | 17 ++ .../TransportationModel/Operator.cs | 19 +++ .../TransportationModel/PoweredVehicle.cs | 17 ++ .../TransportationContext.cs | 151 ++++++++++++++++++ .../TestModels/TransportationModel/Vehicle.cs | 20 +++ .../Update/CommandBatchPreparerTest.cs | 6 +- .../Query/OwnedQuerySqlServerTest.cs | 15 +- .../TableSplittingSqlServerTest.cs | 49 ++++++ .../SqlServerModelBuilderGenericTest.cs | 2 +- .../Query/OwnedQuerySqliteTest.cs | 16 +- .../TableSplittingSqliteTest.cs | 41 +++++ 24 files changed, 708 insertions(+), 159 deletions(-) create mode 100644 src/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs create mode 100644 src/EFCore.Specification.Tests/TestModels/TransportationModel/CombustionEngine.cs create mode 100644 src/EFCore.Specification.Tests/TestModels/TransportationModel/Engine.cs create mode 100644 src/EFCore.Specification.Tests/TestModels/TransportationModel/FuelTank.cs create mode 100644 src/EFCore.Specification.Tests/TestModels/TransportationModel/LicensedOperator.cs create mode 100644 src/EFCore.Specification.Tests/TestModels/TransportationModel/Operator.cs create mode 100644 src/EFCore.Specification.Tests/TestModels/TransportationModel/PoweredVehicle.cs create mode 100644 src/EFCore.Specification.Tests/TestModels/TransportationModel/TransportationContext.cs create mode 100644 src/EFCore.Specification.Tests/TestModels/TransportationModel/Vehicle.cs create mode 100644 test/EFCore.SqlServer.FunctionalTests/TableSplittingSqlServerTest.cs create mode 100644 test/EFCore.Sqlite.FunctionalTests/TableSplittingSqliteTest.cs diff --git a/src/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs b/src/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs new file mode 100644 index 00000000000..f0b7fa653f4 --- /dev/null +++ b/src/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs @@ -0,0 +1,106 @@ +// 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 System; +using System.Linq; +using Microsoft.EntityFrameworkCore.TestModels.TransportationModel; +using Xunit; + +namespace Microsoft.EntityFrameworkCore +{ + public abstract class TableSplittingTestBase + where TTestStore : TestStore + { + [Fact] + public void Can_query_shared() + { + using (var store = CreateTestStore(OnModelCreating)) + { + using (var context = CreateContext(store, OnModelCreating)) + { + Assert.Equal(4, context.Set().ToList().Count); + } + } + } + + [Fact(Skip = "#8973")] + public void Can_query_shared_derived() + { + using (var store = CreateTestStore(OnModelCreating)) + { + using (var context = CreateContext(store, OnModelCreating)) + { + Assert.Equal(2, context.Set().ToList().Count); + Assert.Equal(1, context.Set().ToList().Count); + } + } + } + + [Fact] + public void Can_use_with_redundant_relationships() + { + Test_roundtrip(OnModelCreating); + } + + [Fact] + public void Can_use_with_chained_relationships() + { + Test_roundtrip(modelBuilder => + { + OnModelCreating(modelBuilder); + modelBuilder.Entity(eb => + { + eb.Ignore(e => e.Vehicle); + }); + }); + } + + [Fact] + public void Can_use_with_fanned_relationships() + { + Test_roundtrip(modelBuilder => + { + OnModelCreating(modelBuilder); + modelBuilder.Entity(eb => + { + eb.Ignore(e => e.Engine); + }); + modelBuilder.Entity(eb => + { + eb.Ignore(e => e.FuelTank); + }); + }); + } + + protected virtual void OnModelCreating(ModelBuilder modelBuilder) + { + TransportationContext.OnModelCreatingBase(modelBuilder); + + modelBuilder.Entity(eb => + { + eb.HasDiscriminator("Discriminator"); + eb.Property("Discriminator").HasColumnName("Discriminator"); + eb.ToTable("Vehicles"); + }); + + modelBuilder.Entity().ToTable("Vehicles"); + modelBuilder.Entity().ToTable("Vehicles"); + modelBuilder.Entity().ToTable("Vehicles"); + } + + protected void Test_roundtrip(Action onModelCreating) + { + using (var store = CreateTestStore(onModelCreating)) + { + using (var context = CreateContext(store, onModelCreating)) + { + context.AssertSeeded(); + } + } + } + + protected static readonly string DatabaseName = "TableSplittingTest"; + public abstract TTestStore CreateTestStore(Action onModelCreating); + public abstract TransportationContext CreateContext(TTestStore testStore, Action onModelCreating); + } +} diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs index de2bd8434a0..1627b20a6e3 100644 --- a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs +++ b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs @@ -209,40 +209,60 @@ protected virtual void ValidateSharedTableCompatibility( var firstValidatedType = mappedTypes[0]; var validatedTypes = new List { firstValidatedType }; var unvalidatedTypes = new Queue(mappedTypes.Skip(1)); - while (unvalidatedTypes.Count > 0) + var invalidTypes = new Queue(); + var doAnotherPass = true; + while (doAnotherPass) { - var entityType = unvalidatedTypes.Dequeue(); - var key = entityType.FindPrimaryKey(); - var otherKey = firstValidatedType.FindPrimaryKey(); - if (key.Relational().Name != otherKey.Relational().Name) + doAnotherPass = false; + while (unvalidatedTypes.Count > 0) { - throw new InvalidOperationException( - RelationalStrings.IncompatibleTableKeyNameMismatch( - tableName, - entityType.DisplayName(), - firstValidatedType.DisplayName(), - key.Relational().Name, - Property.Format(key.Properties), - otherKey.Relational().Name, - Property.Format(otherKey.Properties))); + var entityType = unvalidatedTypes.Dequeue(); + var relationshipFound = validatedTypes.Any(validatedType => + entityType.RootType() == validatedType.RootType() + || IsIdentifyingPrincipal(entityType, validatedType) + || IsIdentifyingPrincipal(validatedType, entityType)); + if (!relationshipFound) + { + invalidTypes.Enqueue(entityType); + continue; + } + + var key = entityType.FindPrimaryKey(); + var otherKey = firstValidatedType.FindPrimaryKey(); + if (key.Relational().Name != otherKey.Relational().Name) + { + throw new InvalidOperationException( + RelationalStrings.IncompatibleTableKeyNameMismatch( + tableName, + entityType.DisplayName(), + firstValidatedType.DisplayName(), + key.Relational().Name, + Property.Format(key.Properties), + otherKey.Relational().Name, + Property.Format(otherKey.Properties))); + } + + doAnotherPass = true; + validatedTypes.Add(entityType); } - var relationshipFound = validatedTypes.Any(validatedType => - entityType.RootType() == validatedType.RootType() - || IsIdentifyingPrincipal(entityType, validatedType) - || IsIdentifyingPrincipal(validatedType, entityType)); - if (!relationshipFound) + if (doAnotherPass) { - throw new InvalidOperationException( - RelationalStrings.IncompatibleTableNoRelationship( - tableName, - entityType.DisplayName(), - firstValidatedType.DisplayName(), - Property.Format(key.Properties), - Property.Format(otherKey.Properties))); + var temp = unvalidatedTypes; + unvalidatedTypes = invalidTypes; + invalidTypes = temp; } + } - validatedTypes.Add(entityType); + foreach (var entityType in invalidTypes) + { + throw new InvalidOperationException( + RelationalStrings.IncompatibleTableNoRelationship( + tableName, + entityType.DisplayName(), + firstValidatedType.DisplayName(), + Property.Format(entityType.FindPrimaryKey().Properties), + Property.Format(firstValidatedType.FindPrimaryKey().Properties))); } } diff --git a/src/EFCore.Relational/Metadata/Conventions/Internal/SharedTableConvention.cs b/src/EFCore.Relational/Metadata/Conventions/Internal/SharedTableConvention.cs index c3abfe442cb..2d885216081 100644 --- a/src/EFCore.Relational/Metadata/Conventions/Internal/SharedTableConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/Internal/SharedTableConvention.cs @@ -1,6 +1,7 @@ // 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 System; using System.Collections.Generic; using System.Linq; using Microsoft.EntityFrameworkCore.Infrastructure; @@ -118,31 +119,47 @@ private static void TryUniquifyColumnNames(EntityType entityType, Dictionary(string baseIdentifier, Dictionary existingIdentifiers) + private static string Uniquify(string baseIdentifier, string prefix, Dictionary existingIdentifiers) { - var finalIdentifier = baseIdentifier; + if (!baseIdentifier.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + baseIdentifier = prefix + "_" + baseIdentifier; + } + var suffix = 1; + var finalIdentifier = baseIdentifier; while (existingIdentifiers.ContainsKey(finalIdentifier)) { finalIdentifier = baseIdentifier + suffix; diff --git a/src/EFCore.Relational/Metadata/RelationalPropertyAnnotations.cs b/src/EFCore.Relational/Metadata/RelationalPropertyAnnotations.cs index c98624bb530..23702d43f6a 100644 --- a/src/EFCore.Relational/Metadata/RelationalPropertyAnnotations.cs +++ b/src/EFCore.Relational/Metadata/RelationalPropertyAnnotations.cs @@ -48,18 +48,22 @@ public virtual string ColumnName private string GetDefaultColumnName() { + var entityType = Property.DeclaringEntityType; var pk = Property.GetContainingPrimaryKey(); if (pk != null) { - var entityType = Property.DeclaringEntityType; - var ownership = entityType.GetForeignKeys().SingleOrDefault(fk => fk.IsOwnership); - if (ownership != null) + foreach (var fk in entityType.FindForeignKeys(pk.Properties)) { - var ownerType = ownership.PrincipalEntityType; + if (!fk.PrincipalKey.IsPrimaryKey()) + { + continue; + } + + var principalEntityType = fk.PrincipalEntityType; var entityTypeAnnotations = GetAnnotations(entityType); - var ownerTypeAnnotations = GetAnnotations(ownerType); - if (entityTypeAnnotations.TableName == ownerTypeAnnotations.TableName - && entityTypeAnnotations.Schema == ownerTypeAnnotations.Schema) + var principalTypeAnnotations = GetAnnotations(principalEntityType); + if (entityTypeAnnotations.TableName == principalTypeAnnotations.TableName + && entityTypeAnnotations.Schema == principalTypeAnnotations.Schema) { var index = -1; for (var i = 0; i < pk.Properties.Count; i++) @@ -71,13 +75,12 @@ private string GetDefaultColumnName() } } - return GetAnnotations(ownerType.FindPrimaryKey().Properties[index]).ColumnName; + return GetAnnotations(principalEntityType.FindPrimaryKey().Properties[index]).ColumnName; } } } else { - var entityType = Property.DeclaringEntityType; StringBuilder builder = null; do { diff --git a/src/EFCore.Relational/Metadata/RelationalPropertyExtensions.cs b/src/EFCore.Relational/Metadata/RelationalPropertyExtensions.cs index 30430962741..fd71e6ea307 100644 --- a/src/EFCore.Relational/Metadata/RelationalPropertyExtensions.cs +++ b/src/EFCore.Relational/Metadata/RelationalPropertyExtensions.cs @@ -1,13 +1,33 @@ // 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 System.Linq; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Metadata.Internal; namespace Microsoft.EntityFrameworkCore.Metadata { public static class RelationalPropertyExtensions { public static bool IsColumnNullable([NotNull] this IProperty property) - => property.DeclaringEntityType.BaseType != null || property.IsNullable; + { + if (property.DeclaringEntityType.BaseType != null + || property.IsNullable) + { + return true; + } + + if (property.IsPrimaryKey()) + { + return false; + } + + var pk = property.DeclaringEntityType.FindPrimaryKey(); + return pk != null + && property.DeclaringEntityType.FindForeignKeys(pk.Properties) + .Any(fk => fk.PrincipalKey.IsPrimaryKey() + && fk.DeclaringEntityType.Relational().TableName == fk.PrincipalEntityType.Relational().TableName + && fk.DeclaringEntityType.Relational().Schema == fk.PrincipalEntityType.Relational().Schema); + } } } diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index b18670eb3ae..033d191a476 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -635,20 +635,20 @@ public static string ConflictingOriginalRowValuesSensitive([CanBeNull] object fi firstEntityType, secondEntityType, keyValue, firstConflictingValues, secondConflictingValues, columns); /// - /// There are '{mappedEntityTypeCount}' entity types sharing the table '{tableName}', but only '{entryCount}' entries with the same key values have been marked as '{state}'. The missing entities should be assignable to {missingEntityTypes}. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values. + /// 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. /// - public static string SharedRowEntryCountMismatch([CanBeNull] object mappedEntityTypeCount, [CanBeNull] object tableName, [CanBeNull] object entryCount, [CanBeNull] object state, [CanBeNull] object missingEntityTypes) + public static string SharedRowEntryCountMismatch([CanBeNull] object entityType, [CanBeNull] object tableName, [CanBeNull] object missingEntityType, [CanBeNull] object state) => string.Format( - GetString("SharedRowEntryCountMismatch", nameof(mappedEntityTypeCount), nameof(tableName), nameof(entryCount), nameof(state), nameof(missingEntityTypes)), - mappedEntityTypeCount, tableName, entryCount, state, missingEntityTypes); + GetString("SharedRowEntryCountMismatch", nameof(entityType), nameof(tableName), nameof(missingEntityType), nameof(state)), + entityType, tableName, missingEntityType, state); /// - /// There are '{mappedEntityTypeCount}' entity types sharing the table '{tableName}', but only '{entryCount}' entries with the same key values '{keyValue}' have been marked as '{state}'. The missing entities should be assignable to {missingEntityTypes}. + /// 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}'. /// - public static string SharedRowEntryCountMismatchSensitive([CanBeNull] object mappedEntityTypeCount, [CanBeNull] object tableName, [CanBeNull] object entryCount, [CanBeNull] object keyValue, [CanBeNull] object state, [CanBeNull] object missingEntityTypes) + public static string SharedRowEntryCountMismatchSensitive([CanBeNull] object entityType, [CanBeNull] object tableName, [CanBeNull] object missingEntityType, [CanBeNull] object keyValue, [CanBeNull] object state) => string.Format( - GetString("SharedRowEntryCountMismatchSensitive", nameof(mappedEntityTypeCount), nameof(tableName), nameof(entryCount), nameof(keyValue), nameof(state), nameof(missingEntityTypes)), - mappedEntityTypeCount, tableName, entryCount, keyValue, state, missingEntityTypes); + GetString("SharedRowEntryCountMismatchSensitive", nameof(entityType), nameof(tableName), nameof(missingEntityType), nameof(keyValue), nameof(state)), + entityType, tableName, missingEntityType, keyValue, state); /// /// Cannot set default value '{value}' of type '{valueType}' on property '{property}' of type '{propertyType}' in entity type '{entityType}'. diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 9d8b50605ed..91889545a8f 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1,17 +1,17 @@  - @@ -346,10 +346,10 @@ 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}. - There are '{mappedEntityTypeCount}' entity types sharing the table '{tableName}', but only '{entryCount}' entries with the same key values have been marked as '{state}'. The missing entities should be assignable to {missingEntityTypes}. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values. + 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. - There are '{mappedEntityTypeCount}' entity types sharing the table '{tableName}', but only '{entryCount}' entries with the same key values '{keyValue}' have been marked as '{state}'. The missing entities should be assignable to {missingEntityTypes}. + 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}'. Cannot set default value '{value}' of type '{valueType}' on property '{property}' of type '{propertyType}' in entity type '{entityType}'. diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 780602f993c..4386ebcf453 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -153,13 +153,13 @@ private IReadOnlyDictionary return _tableSharingIdentityMapFactories; } - var tables = new Dictionary<(string Schema, string TableName), List>(); + var tables = new Dictionary<(string Schema, string TableName), HashSet>(); foreach (var entityType in model.GetEntityTypes()) { var fullName = (entityType.Relational().Schema, entityType.Relational().TableName); if (!tables.TryGetValue(fullName, out var mappedEntityTypes)) { - mappedEntityTypes = new List(); + mappedEntityTypes = new HashSet(); tables.Add(fullName, mappedEntityTypes); } @@ -169,28 +169,22 @@ private IReadOnlyDictionary var sharedTablesMap = new Dictionary(); foreach (var tableMapping in tables) { - var roots = new HashSet(tableMapping.Value.Where(e => e.BaseType == null)); - if (roots.Count > 1) + var entityTypes = tableMapping.Value; + if (entityTypes.Count > 1) { - var rootGraph = new Multigraph(); - rootGraph.AddVertices(roots); - foreach (var entityType in roots) + var principals = new Dictionary>(entityTypes.Count); + foreach (var entityType in entityTypes) { - foreach (var foreignKey in entityType.GetForeignKeys()) + var principalList = new List(); + foreach (var foreignKey in entityType.FindForeignKeys(entityType.FindPrimaryKey().Properties)) { - if (foreignKey.PrincipalEntityType != entityType - && roots.Contains(foreignKey.PrincipalEntityType)) + if (foreignKey.PrincipalKey.IsPrimaryKey() + && entityTypes.Contains(foreignKey.PrincipalEntityType)) { - rootGraph.AddEdge(foreignKey.PrincipalEntityType, foreignKey.DeclaringEntityType, foreignKey); + principalList.Add(foreignKey.PrincipalEntityType); } } - } - - var sortedRoots = rootGraph.TopologicalSort(); - var rootTypesOrder = new Dictionary(sortedRoots.Count); - for (var i = 0; i < sortedRoots.Count; i++) - { - rootTypesOrder[sortedRoots[i]] = i; + principals[entityType] = principalList; } var stateManager = _currentContext.GetDependencies().StateManager; @@ -202,13 +196,13 @@ ModificationCommandIdentityMap CommandIdentityMapFactory( bool sensitiveLoggingEnabled) => new ModificationCommandIdentityMap( stateManager, - rootTypesOrder, + principals, name, schema, generateParameterName, sensitiveLoggingEnabled); - foreach (var entityType in tableMapping.Value) + foreach (var entityType in entityTypes) { sharedTablesMap.Add(entityType, CommandIdentityMapFactory); } diff --git a/src/EFCore.Relational/Update/Internal/ModificationCommandIdentityMap.cs b/src/EFCore.Relational/Update/Internal/ModificationCommandIdentityMap.cs index 88549a38971..5dfb3a2567d 100644 --- a/src/EFCore.Relational/Update/Internal/ModificationCommandIdentityMap.cs +++ b/src/EFCore.Relational/Update/Internal/ModificationCommandIdentityMap.cs @@ -19,7 +19,7 @@ namespace Microsoft.EntityFrameworkCore.Update.Internal public class ModificationCommandIdentityMap { private readonly IStateManager _stateManager; - private readonly IReadOnlyDictionary _rootTypesOrder; + private readonly IReadOnlyDictionary> _principals; private readonly string _name; private readonly string _schema; private readonly Func _generateParameterName; @@ -35,19 +35,19 @@ private readonly Dictionary _sharedCom /// public ModificationCommandIdentityMap( [NotNull] IStateManager stateManager, - [NotNull] IReadOnlyDictionary rootTypesOrder, + [NotNull] IReadOnlyDictionary> principals, [NotNull] string name, [CanBeNull] string schema, [NotNull] Func generateParameterName, bool sensitiveLoggingEnabled) { _stateManager = stateManager; - _rootTypesOrder = rootTypesOrder; + _principals = principals; _name = name; _schema = schema; _generateParameterName = generateParameterName; _sensitiveLoggingEnabled = sensitiveLoggingEnabled; - _comparer = new EntryComparer(rootTypesOrder); + _comparer = new EntryComparer(principals); } /// @@ -72,7 +72,7 @@ public virtual ModificationCommand GetOrAddCommand([NotNull] IUpdateEntry entry) private InternalEntityEntry GetMainEntry(InternalEntityEntry entry) { var entityType = entry.EntityType.RootType(); - if (_rootTypesOrder[entityType] == 0) + if (_principals[entityType].Count == 0) { return entry; } @@ -80,7 +80,7 @@ private InternalEntityEntry GetMainEntry(InternalEntityEntry entry) foreach (var foreignKey in entityType.FindForeignKeys(entityType.FindPrimaryKey().Properties)) { if (foreignKey.PrincipalKey.IsPrimaryKey() - && _rootTypesOrder.ContainsKey(foreignKey.PrincipalEntityType)) + && _principals.ContainsKey(foreignKey.PrincipalEntityType)) { var principal = _stateManager.GetPrincipal(entry, foreignKey); if (principal != null) @@ -101,48 +101,65 @@ public virtual void Validate(bool sensitiveLoggingEnabled) { foreach (var command in _sharedCommands.Values) { - if ((command.EntityState == EntityState.Added - || command.EntityState == EntityState.Deleted) - && command.Entries.Count != _rootTypesOrder.Count) + if ((command.EntityState != EntityState.Added + && command.EntityState != EntityState.Deleted) + || command.Entries.Any(e => _principals[e.EntityType].Count == 0)) { - var tableName = (string.IsNullOrEmpty(command.Schema) ? "" : command.Schema + ".") + command.TableName; - - var missingEntityTypes = new HashSet(_rootTypesOrder.Keys); - foreach (var entry in command.Entries) - { - missingEntityTypes.Remove(entry.EntityType.RootType()); - } - - var missingEntityTypesString = "{" + string.Join(", ", missingEntityTypes.Select(p => "'" + p.DisplayName() + "'")) + "}"; + continue; + } - if (sensitiveLoggingEnabled) + var tableName = (string.IsNullOrEmpty(command.Schema) ? "" : command.Schema + ".") + command.TableName; + foreach (var entry in command.Entries) + { + foreach (var principalEntityType in _principals[entry.EntityType]) { - throw new InvalidOperationException(RelationalStrings.SharedRowEntryCountMismatchSensitive( - _rootTypesOrder.Count, - tableName, - command.Entries.Count, - command.Entries[0].BuildCurrentValuesString(command.Entries[0].EntityType.FindPrimaryKey().Properties), - command.EntityState, - missingEntityTypesString)); + if (!command.Entries.Any(principalEntry => principalEntry != entry + && principalEntityType.IsAssignableFrom(principalEntry.EntityType))) + { + if (sensitiveLoggingEnabled) + { + throw new InvalidOperationException(RelationalStrings.SharedRowEntryCountMismatchSensitive( + entry.EntityType.DisplayName(), + tableName, + principalEntityType.DisplayName(), + entry.BuildCurrentValuesString(entry.EntityType.FindPrimaryKey().Properties), + command.EntityState)); + } + + throw new InvalidOperationException(RelationalStrings.SharedRowEntryCountMismatch( + entry.EntityType.DisplayName(), + tableName, + principalEntityType.DisplayName(), + command.EntityState)); + } } - - throw new InvalidOperationException(RelationalStrings.SharedRowEntryCountMismatch( - _rootTypesOrder.Count, tableName, command.Entries.Count, command.EntityState, missingEntityTypesString)); } } } private class EntryComparer : IComparer { - private readonly IReadOnlyDictionary _rootTypesOrder; + private readonly IReadOnlyDictionary> _principals; - public EntryComparer(IReadOnlyDictionary rootTypesOrder) + public EntryComparer(IReadOnlyDictionary> principals) { - _rootTypesOrder = rootTypesOrder; + _principals = principals; } public int Compare(IUpdateEntry x, IUpdateEntry y) - => _rootTypesOrder[x.EntityType.RootType()] - _rootTypesOrder[y.EntityType.RootType()]; + { + if (_principals[x.EntityType].Count == 0) + { + return -1; + } + + if (_principals[y.EntityType].Count == 0) + { + return 1; + } + + return StringComparer.Ordinal.Compare(x.EntityType.Name, y.EntityType.Name); + } } } } diff --git a/src/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs b/src/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs index 5a33102bd24..15aef3e1a38 100644 --- a/src/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs +++ b/src/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs @@ -15,7 +15,7 @@ public virtual void Query_for_base_type_loads_all_owned_navs() using (var context = CreateContext()) { var people = context.Set().ToList(); - + Assert.Equal(4, people.Count); Assert.True(people.All(p => p.PersonAddress != null)); Assert.True(people.OfType().All(b => b.BranchAddress != null)); @@ -23,7 +23,7 @@ public virtual void Query_for_base_type_loads_all_owned_navs() Assert.True(people.OfType().All(b => b.LeafBAddress != null)); } } - + [Fact] public virtual void Query_for_branch_type_loads_all_owned_navs() { diff --git a/src/EFCore.Specification.Tests/TestModels/TransportationModel/CombustionEngine.cs b/src/EFCore.Specification.Tests/TestModels/TransportationModel/CombustionEngine.cs new file mode 100644 index 00000000000..a281675a689 --- /dev/null +++ b/src/EFCore.Specification.Tests/TestModels/TransportationModel/CombustionEngine.cs @@ -0,0 +1,17 @@ +namespace Microsoft.EntityFrameworkCore.TestModels.TransportationModel +{ + public class CombustionEngine : Engine + { + public FuelTank FuelTank { get; set; } + + public override bool Equals(object obj) + { + var other = obj as CombustionEngine; + return other != null + && base.Equals(other) + && Equals(FuelTank, other.FuelTank); + } + + public override int GetHashCode() => base.GetHashCode(); + } +} diff --git a/src/EFCore.Specification.Tests/TestModels/TransportationModel/Engine.cs b/src/EFCore.Specification.Tests/TestModels/TransportationModel/Engine.cs new file mode 100644 index 00000000000..6d335145bf6 --- /dev/null +++ b/src/EFCore.Specification.Tests/TestModels/TransportationModel/Engine.cs @@ -0,0 +1,19 @@ +namespace Microsoft.EntityFrameworkCore.TestModels.TransportationModel +{ + public class Engine + { + public string VehicleName { get; set; } + public string Description { get; set; } + public PoweredVehicle Vehicle { get; set; } + + public override bool Equals(object obj) + { + var other = obj as Engine; + return other != null + && VehicleName == other.VehicleName + && Description == other.Description; + } + + public override int GetHashCode() => VehicleName.GetHashCode(); + } +} diff --git a/src/EFCore.Specification.Tests/TestModels/TransportationModel/FuelTank.cs b/src/EFCore.Specification.Tests/TestModels/TransportationModel/FuelTank.cs new file mode 100644 index 00000000000..299a9b50e4a --- /dev/null +++ b/src/EFCore.Specification.Tests/TestModels/TransportationModel/FuelTank.cs @@ -0,0 +1,25 @@ +// 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. + +namespace Microsoft.EntityFrameworkCore.TestModels.TransportationModel +{ + public class FuelTank + { + public string VehicleName { get; set; } + public string FuelType { get; set; } + public string Capacity { get; set; } + public PoweredVehicle Vehicle { get; set; } + public CombustionEngine Engine { get; set; } + + public override bool Equals(object obj) + { + var other = obj as FuelTank; + return other != null + && VehicleName == other.VehicleName + && FuelType == other.FuelType + && Capacity == other.Capacity; + } + + public override int GetHashCode() => VehicleName.GetHashCode(); + } +} diff --git a/src/EFCore.Specification.Tests/TestModels/TransportationModel/LicensedOperator.cs b/src/EFCore.Specification.Tests/TestModels/TransportationModel/LicensedOperator.cs new file mode 100644 index 00000000000..3614a48a58f --- /dev/null +++ b/src/EFCore.Specification.Tests/TestModels/TransportationModel/LicensedOperator.cs @@ -0,0 +1,17 @@ +namespace Microsoft.EntityFrameworkCore.TestModels.TransportationModel +{ + public class LicensedOperator : Operator + { + public string LicenseType { get; set; } + + public override bool Equals(object obj) + { + var other = obj as LicensedOperator; + return other != null + && base.Equals(other) + && LicenseType == other.LicenseType; + } + + public override int GetHashCode() => base.GetHashCode(); + } +} diff --git a/src/EFCore.Specification.Tests/TestModels/TransportationModel/Operator.cs b/src/EFCore.Specification.Tests/TestModels/TransportationModel/Operator.cs new file mode 100644 index 00000000000..0179b98499d --- /dev/null +++ b/src/EFCore.Specification.Tests/TestModels/TransportationModel/Operator.cs @@ -0,0 +1,19 @@ +namespace Microsoft.EntityFrameworkCore.TestModels.TransportationModel +{ + public class Operator + { + public string VehicleName { get; set; } + public string Name { get; set; } + public Vehicle Vehicle { get; set; } + + public override bool Equals(object obj) + { + var other = obj as Operator; + return other != null + && VehicleName == other.VehicleName + && Name == other.Name; + } + + public override int GetHashCode() => VehicleName.GetHashCode(); + } +} diff --git a/src/EFCore.Specification.Tests/TestModels/TransportationModel/PoweredVehicle.cs b/src/EFCore.Specification.Tests/TestModels/TransportationModel/PoweredVehicle.cs new file mode 100644 index 00000000000..b4b6c383e24 --- /dev/null +++ b/src/EFCore.Specification.Tests/TestModels/TransportationModel/PoweredVehicle.cs @@ -0,0 +1,17 @@ +namespace Microsoft.EntityFrameworkCore.TestModels.TransportationModel +{ + public class PoweredVehicle : Vehicle + { + public Engine Engine { get; set; } + + public override bool Equals(object obj) + { + var other = obj as PoweredVehicle; + return other != null + && base.Equals(other) + && Equals(Engine, other.Engine); + } + + public override int GetHashCode() => base.GetHashCode(); + } +} diff --git a/src/EFCore.Specification.Tests/TestModels/TransportationModel/TransportationContext.cs b/src/EFCore.Specification.Tests/TestModels/TransportationModel/TransportationContext.cs new file mode 100644 index 00000000000..965e62e9499 --- /dev/null +++ b/src/EFCore.Specification.Tests/TestModels/TransportationModel/TransportationContext.cs @@ -0,0 +1,151 @@ +// 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 System.Collections.Generic; +using System.Linq; +using Xunit; + +namespace Microsoft.EntityFrameworkCore.TestModels.TransportationModel +{ + public class TransportationContext : DbContext + { + public TransportationContext(DbContextOptions options) + : base(options) + { + } + + public DbSet Vehicles { get; set; } + public DbSet Operators { get; set; } + + public static void OnModelCreatingBase(ModelBuilder modelBuilder) + { + modelBuilder.Entity(eb => + { + eb.HasKey(e => e.Name); + }); + modelBuilder.Entity(eb => + { + eb.HasKey(e => e.VehicleName); + eb.HasOne(e => e.Vehicle) + .WithOne(e => e.Engine) + .HasForeignKey(e => e.VehicleName); + }); + modelBuilder.Entity(); + + modelBuilder.Entity(eb => + { + eb.HasKey(e => e.VehicleName); + eb.HasOne(e => e.Vehicle) + .WithOne(e => e.Operator) + .HasForeignKey(e => e.VehicleName); + }); + modelBuilder.Entity(); + + modelBuilder.Entity(eb => + { + eb.HasKey(e => e.VehicleName); + eb.HasOne(e => e.Engine) + .WithOne(e => e.FuelTank) + .HasForeignKey(e => e.VehicleName); + eb.HasOne(e => e.Vehicle) + .WithOne() + .HasForeignKey(e => e.VehicleName); + }); + } + + public void Seed() + { + Vehicles.AddRange(CreateVehicles()); + SaveChanges(); + } + + public void AssertSeeded() + { + Assert.Equal(CreateVehicles().OrderBy(v => v.Name).ToList(), Load(Vehicles).OrderBy(v => v.Name).ToList()); + } + + // TODO: Instead use derived includes when available + public DbSet Load(DbSet vehicles) + { + foreach (var vehicle in vehicles) + { + Load(vehicle); + } + + return vehicles; + } + + private void Load(Vehicle vehicle) + { + if (vehicle != null) + { + switch (vehicle) + { + case PoweredVehicle v: + Entry(v).Reference(e => e.Engine).Load(); + Load(v.Engine); + goto default; + default: + Entry(vehicle).Reference(e => e.Operator).Load(); + break; + } + } + } + + private void Load(Engine engine) + { + if (engine != null) + { + switch (engine) + { + case CombustionEngine en: + en.FuelTank = Set().SingleOrDefault(f => f.VehicleName == en.VehicleName); + break; + } + } + } + + protected IEnumerable CreateVehicles() + { + return new List + { + new Vehicle + { + Name = "Paper plane", + SeatingCapacity = 0 + }, + new Vehicle + { + Name = "Trek Pro Fit Madone 6 Series", + SeatingCapacity = 1, + Operator = new Operator { Name = "Lance Armstrong", VehicleName = "Trek Pro Fit Madone 6 Series" } + }, + new PoweredVehicle + { + Name = "1984 California Car", + SeatingCapacity = 34, + Operator = new LicensedOperator { Name = "Albert Williams", LicenseType = "Muni Transit", VehicleName = "1984 California Car" } + }, + new PoweredVehicle + { + Name = "P85 2012 Tesla Model S Performance Edition", + SeatingCapacity = 5, + Engine = new Engine { Description = "416 hp three phase, four pole AC induction", VehicleName = "P85 2012 Tesla Model S Performance Edition" }, + Operator = new LicensedOperator { Name = "Elon Musk", LicenseType = "Driver", VehicleName = "P85 2012 Tesla Model S Performance Edition" } + }, + new PoweredVehicle + { + Name = "North American X-15A-2", + SeatingCapacity = 1, + Engine = new CombustionEngine + { + Description = "Reaction Motors XLR99 throttleable, restartable liquid-propellant rocket engine", + FuelTank = new FuelTank { FuelType = "Liquid oxygen and anhydrous ammonia", Capacity = "11250 kg", VehicleName = "North American X-15A-2" }, + VehicleName = "North American X-15A-2" + }, + Operator = new LicensedOperator { Name = "William J. Knight", LicenseType = "Air Force Test Pilot", VehicleName = "North American X-15A-2" } + } + }; + } + } +} diff --git a/src/EFCore.Specification.Tests/TestModels/TransportationModel/Vehicle.cs b/src/EFCore.Specification.Tests/TestModels/TransportationModel/Vehicle.cs new file mode 100644 index 00000000000..cb15a1fdcdb --- /dev/null +++ b/src/EFCore.Specification.Tests/TestModels/TransportationModel/Vehicle.cs @@ -0,0 +1,20 @@ +namespace Microsoft.EntityFrameworkCore.TestModels.TransportationModel +{ + public class Vehicle + { + public string Name { get; set; } + public int SeatingCapacity { get; set; } + public Operator Operator { get; set; } + + public override bool Equals(object obj) + { + var other = obj as Vehicle; + return other != null + && Name == other.Name + && SeatingCapacity == other.SeatingCapacity + && Equals(Operator, other.Operator); + } + + public override int GetHashCode() => Name.GetHashCode(); + } +} diff --git a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs index 570fd14fea3..0303537054f 100644 --- a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs +++ b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs @@ -681,20 +681,20 @@ public void BatchCommands_throws_on_incomplete_updates_for_shared_table(EntitySt var currentDbContext = CreateContextServices(CreateSharedTableModel()).GetRequiredService(); var stateManager = currentDbContext.GetDependencies().StateManager; - var first = new FakeEntity { Id = 42, Value = "Test" }; + var first = new RelatedFakeEntity { Id = 42 }; var firstEntry = stateManager.GetOrCreateEntry(first); firstEntry.SetEntityState(state); if (sensitiveLogging) { - Assert.Equal(RelationalStrings.SharedRowEntryCountMismatchSensitive(2, "FakeEntity", 1, "Id:42", state, "{'RelatedFakeEntity'}"), + Assert.Equal(RelationalStrings.SharedRowEntryCountMismatchSensitive("RelatedFakeEntity", "FakeEntity", "FakeEntity", "Id:42", state), Assert.Throws( () => CreateCommandBatchPreparer(currentDbContext: currentDbContext, sensitiveLogging: sensitiveLogging) .BatchCommands(new[] { firstEntry }).ToArray()).Message); } else { - Assert.Equal(RelationalStrings.SharedRowEntryCountMismatch(2, "FakeEntity", 1, state, "{'RelatedFakeEntity'}"), + Assert.Equal(RelationalStrings.SharedRowEntryCountMismatch("RelatedFakeEntity", "FakeEntity", "FakeEntity", state), Assert.Throws( () => CreateCommandBatchPreparer(currentDbContext: currentDbContext, sensitiveLogging: sensitiveLogging) .BatchCommands(new[] { firstEntry }).ToArray()).Message); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs index b3f58dba60f..05f5b72aecb 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs @@ -12,9 +12,10 @@ public class OwnedQuerySqlServerTest : OwnedQueryTestBase, IClassFixture + { + private readonly string _connectionString = SqlServerTestStore.CreateConnectionString(DatabaseName); + public TestSqlLoggerFactory TestSqlLoggerFactory { get; } = new TestSqlLoggerFactory(); + + public override SqlServerTestStore CreateTestStore(Action onModelCreating) + => SqlServerTestStore.GetOrCreateShared(DatabaseName, () => + { + var optionsBuilder = new DbContextOptionsBuilder() + .UseSqlServer(_connectionString, b => b.ApplyConfiguration().CommandTimeout(300)) + .UseInternalServiceProvider(BuildServiceProvider(onModelCreating)); + + using (var context = new TransportationContext(optionsBuilder.Options)) + { + context.Database.EnsureCreated(); + context.Seed(); + } + }); + + public override TransportationContext CreateContext(SqlServerTestStore testStore, Action onModelCreating) + { + var optionsBuilder = new DbContextOptionsBuilder() + .UseSqlServer(testStore.Connection, b => b.ApplyConfiguration().CommandTimeout(300)) + .UseInternalServiceProvider(BuildServiceProvider(onModelCreating)); + + var context = new TransportationContext(optionsBuilder.Options); + context.Database.UseTransaction(testStore.Transaction); + return context; + } + + private IServiceProvider BuildServiceProvider(Action onModelCreating) + => new ServiceCollection() + .AddEntityFrameworkSqlServer() + .AddSingleton(TestModelSource.GetFactory(onModelCreating)) + .AddSingleton(TestSqlLoggerFactory) + .BuildServiceProvider(); + } +} diff --git a/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs b/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs index b2f99324f70..3bfd155270d 100644 --- a/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs +++ b/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderGenericTest.cs @@ -94,7 +94,7 @@ public void Can_use_shadow_FK_that_collides_with_convention_shadow_FK_on_other_d Assert.Equal("ParentId", property1.SqlServer().ColumnName); var property2 = modelBuilder.Model.FindEntityType(typeof(DisjointChildSubclass2)).FindProperty("ParentId"); Assert.True(property2.IsForeignKey()); - Assert.Equal("ParentId1", property2.SqlServer().ColumnName); + Assert.Equal("DisjointChildSubclass2_ParentId", property2.SqlServer().ColumnName); } public class Parent diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/OwnedQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/OwnedQuerySqliteTest.cs index 88a7b556546..c6aba183041 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/OwnedQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/OwnedQuerySqliteTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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 Xunit; @@ -14,31 +14,25 @@ public OwnedQuerySqliteTest(OwnedQuerySqliteFixture fixture) _fixture = fixture; } - [Fact(Skip = "#8907")] + [Fact(Skip = "#8973")] public override void Query_for_base_type_loads_all_owned_navs() { base.Query_for_base_type_loads_all_owned_navs(); } - [Fact(Skip = "#8907")] + [Fact(Skip = "#8973")] public override void Query_for_branch_type_loads_all_owned_navs() { base.Query_for_branch_type_loads_all_owned_navs(); } - [Fact(Skip = "#8907")] - public override void Query_for_leaf_type_loads_all_owned_navs() - { - base.Query_for_leaf_type_loads_all_owned_navs(); - } - - [Fact(Skip = "#8907")] + [Fact(Skip = "#8973")] public override void Query_when_group_by() { base.Query_when_group_by(); } - [Fact(Skip = "#8907")] + [Fact(Skip = "#8973")] public override void Query_when_subquery() { base.Query_when_subquery(); diff --git a/test/EFCore.Sqlite.FunctionalTests/TableSplittingSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/TableSplittingSqliteTest.cs new file mode 100644 index 00000000000..c5bbadaf4c2 --- /dev/null +++ b/test/EFCore.Sqlite.FunctionalTests/TableSplittingSqliteTest.cs @@ -0,0 +1,41 @@ +// 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 System; +using Microsoft.EntityFrameworkCore.TestModels.TransportationModel; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.EntityFrameworkCore +{ + public class TableSplittingSqliteTest : TableSplittingTestBase + { + public override SqliteTestStore CreateTestStore(Action onModelCreating) + => SqliteTestStore.GetOrCreateShared(DatabaseName, false, true, () => + { + var optionsBuilder = new DbContextOptionsBuilder() + .UseSqlite(SqliteTestStore.CreateConnectionString(DatabaseName)) + .UseInternalServiceProvider(BuildServiceProvider(onModelCreating)); + + using (var context = new TransportationContext(optionsBuilder.Options)) + { + context.Database.EnsureClean(); + context.Seed(); + } + }); + + public override TransportationContext CreateContext(SqliteTestStore testStore, Action onModelCreating) + { + var optionsBuilder = new DbContextOptionsBuilder() + .UseSqlite(SqliteTestStore.CreateConnectionString(DatabaseName)) + .UseInternalServiceProvider(BuildServiceProvider(onModelCreating)); + + return new TransportationContext(optionsBuilder.Options); + } + + private IServiceProvider BuildServiceProvider(Action onModelCreating) + => new ServiceCollection() + .AddEntityFrameworkSqlite() + .AddSingleton(TestModelSource.GetFactory(onModelCreating)) + .BuildServiceProvider(); + } +}