From 6e42ba6fee532aae771272ff6f6f293c682236dd Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Wed, 26 Oct 2016 14:16:06 -0700 Subject: [PATCH] Add a safeguard for fks updated in recursive calls to ForeignKeyPropertyDiscoveryConvention Make InternalEntityTypeBuilder.HasKey more resilient by removing conflicting fks of lower source Reset FieldInfo when lifting properties if the field doesn't exist on the new type Improve the exception message for removing a property that is still being referenced Fixes #6867 --- .../Internal/TypeExtensions.cs | 20 ++++ .../ForeignKeyPropertyDiscoveryConvention.cs | 8 +- .../Metadata/Internal/EntityType.cs | 92 ++++++++------- .../Internal/InternalEntityTypeBuilder.cs | 48 ++++++-- .../Internal/InternalPropertyBuilder.cs | 25 +++- .../Metadata/Internal/Property.cs | 6 +- .../Metadata/Internal/PropertyBase.cs | 109 +++++++++++++----- .../Properties/CoreStrings.Designer.cs | 22 +++- .../Properties/CoreStrings.resx | 10 +- .../Metadata/Internal/EntityTypeTest.cs | 8 +- .../ModelBuilderTest/InheritanceTestBase.cs | 34 ++++++ 11 files changed, 286 insertions(+), 96 deletions(-) diff --git a/src/Microsoft.EntityFrameworkCore/Internal/TypeExtensions.cs b/src/Microsoft.EntityFrameworkCore/Internal/TypeExtensions.cs index f9c53b62959..d1ce1959a16 100644 --- a/src/Microsoft.EntityFrameworkCore/Internal/TypeExtensions.cs +++ b/src/Microsoft.EntityFrameworkCore/Internal/TypeExtensions.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Globalization; +using System.Linq; using System.Reflection; using System.Text; using JetBrains.Annotations; @@ -60,6 +61,25 @@ public static string DisplayName([NotNull] this Type type, bool fullName = true) return sb.ToString(); } + /// + /// 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. + /// + public static FieldInfo GetFieldInfo([NotNull] this Type type, [NotNull] string fieldName) + { + var typesInHierarchy = type.GetTypesInHierarchy().ToList(); + foreach (var typeInHierarchy in typesInHierarchy) + { + var fields = typeInHierarchy.GetRuntimeFields().ToDictionary(f => f.Name); + FieldInfo fieldInfo; + if (fields.TryGetValue(fieldName, out fieldInfo)) + { + return fieldInfo; + } + } + return null; + } + private static void AppendGenericArguments(Type[] args, int startIndex, int numberOfArgsToAppend, StringBuilder sb, bool fullName) { var totalArgs = args.Length; diff --git a/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/ForeignKeyPropertyDiscoveryConvention.cs b/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/ForeignKeyPropertyDiscoveryConvention.cs index d06c5223aa0..f7da9bd1ac5 100644 --- a/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/ForeignKeyPropertyDiscoveryConvention.cs +++ b/src/Microsoft.EntityFrameworkCore/Metadata/Conventions/Internal/ForeignKeyPropertyDiscoveryConvention.cs @@ -353,10 +353,12 @@ public virtual InternalKeyBuilder Apply(InternalKeyBuilder keyBuilder) /// public virtual void Apply(InternalEntityTypeBuilder entityTypeBuilder, Key key) { - foreach (var foreignKey in key.DeclaringEntityType.GetDerivedForeignKeysInclusive().ToList()) + var fks = key.DeclaringEntityType.GetDerivedForeignKeysInclusive().ToList(); + foreach (var foreignKey in fks) { - if (!foreignKey.IsUnique - || foreignKey.DeclaringEntityType.BaseType != null) + if (foreignKey.Builder != null + && (!foreignKey.IsUnique + || foreignKey.DeclaringEntityType.BaseType != null)) { Apply(foreignKey.Builder); } diff --git a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/EntityType.cs b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/EntityType.cs index b43ed3bd2e7..f161c4a5b09 100644 --- a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/EntityType.cs +++ b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/EntityType.cs @@ -465,16 +465,13 @@ public virtual Key AddKey([NotNull] IReadOnlyList properties, foreach (var property in properties) { - var currentKeys = property.Keys; - if (currentKeys == null) + if (property.Keys == null) { - property.Keys = new IKey[] { key }; + property.Keys = new List { key }; } else { - var newKeys = currentKeys.ToList(); - newKeys.Add(key); - property.Keys = newKeys; + property.Keys.Add(key); } } @@ -566,11 +563,14 @@ private Key RemoveKey([NotNull] Key key, bool runConventions) foreach (var property in key.Properties) { - property.Keys = - property.Keys != null - && property.Keys.Count > 1 - ? property.Keys.Where(k => k != key).ToList() - : null; + if (property.Keys != null) + { + property.Keys.Remove(key); + if (property.Keys.Count == 0) + { + property.Keys = null; + } + } } PropertyMetadataChanged(); @@ -673,16 +673,13 @@ public virtual ForeignKey AddForeignKey( foreach (var property in properties) { - var currentKeys = property.ForeignKeys; - if (currentKeys == null) + if (property.ForeignKeys == null) { - property.ForeignKeys = new IForeignKey[] { foreignKey }; + property.ForeignKeys = new List { foreignKey }; } else { - var newKeys = currentKeys.ToList(); - newKeys.Add(foreignKey); - property.ForeignKeys = newKeys; + property.ForeignKeys.Add(foreignKey); } } @@ -909,11 +906,14 @@ private ForeignKey RemoveForeignKey([NotNull] ForeignKey foreignKey, bool runCon foreach (var property in foreignKey.Properties) { - property.ForeignKeys = - property.ForeignKeys != null - && property.ForeignKeys.Count > 1 - ? property.ForeignKeys.Where(k => k != foreignKey).ToList() - : null; + if (property.ForeignKeys != null) + { + property.ForeignKeys.Remove(foreignKey); + if (property.ForeignKeys.Count == 0) + { + property.ForeignKeys = null; + } + } } foreignKey.PrincipalKey.ReferencingForeignKeys.Remove(foreignKey); @@ -1194,16 +1194,13 @@ public virtual Index AddIndex([NotNull] IReadOnlyList properties, foreach (var property in properties) { - var currentIndexes = property.Indexes; - if (currentIndexes == null) + if (property.Indexes == null) { - property.Indexes = new IIndex[] { index }; + property.Indexes = new List { index }; } else { - var newIndex = currentIndexes.ToList(); - newIndex.Add(index); - property.Indexes = newIndex; + property.Indexes.Add(index); } } @@ -1312,11 +1309,14 @@ private Index RemoveIndex(Index index) foreach (var property in index.Properties) { - property.Indexes = - property.Indexes != null - && property.Indexes.Count > 1 - ? property.Indexes.Where(k => k != index).ToList() - : null; + if (property.Indexes != null) + { + property.Indexes.Remove(index); + if (property.Indexes.Count == 0) + { + property.Indexes = null; + } + } } Model.ConventionDispatcher.OnIndexRemoved(Builder, index); @@ -1535,21 +1535,27 @@ private Property RemoveProperty(Property property) private void CheckPropertyNotInUse(Property property) { - CheckPropertyNotInUse(property, this); + var containingKey = property.Keys?.FirstOrDefault(); + if (containingKey != null) + { + throw new InvalidOperationException( + CoreStrings.PropertyInUseKey(property.Name, this.DisplayName(), Property.Format(containingKey.Properties))); + } - foreach (var entityType in GetDerivedTypes()) + var containingForeignKey = property.ForeignKeys?.FirstOrDefault(); + if (containingForeignKey != null) { - CheckPropertyNotInUse(property, entityType); + throw new InvalidOperationException( + CoreStrings.PropertyInUseForeignKey(property.Name, this.DisplayName(), + Property.Format(containingForeignKey.Properties), containingForeignKey.DeclaringEntityType.DisplayName())); } - } - private void CheckPropertyNotInUse(Property property, EntityType entityType) - { - if (entityType.GetDeclaredKeys().Any(k => k.Properties.Contains(property)) - || entityType.GetDeclaredForeignKeys().Any(k => k.Properties.Contains(property)) - || entityType.GetDeclaredIndexes().Any(i => i.Properties.Contains(property))) + var containingIndex = property.Indexes?.FirstOrDefault(); + if (containingIndex != null) { - throw new InvalidOperationException(CoreStrings.PropertyInUse(property.Name, this.DisplayName())); + throw new InvalidOperationException( + CoreStrings.PropertyInUseIndex(property.Name, this.DisplayName(), + Property.Format(containingIndex.Properties), containingIndex.DeclaringEntityType.DisplayName())); } } diff --git a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/InternalEntityTypeBuilder.cs b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/InternalEntityTypeBuilder.cs index 779455b79be..fc9b846759a 100644 --- a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/InternalEntityTypeBuilder.cs +++ b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/InternalEntityTypeBuilder.cs @@ -139,20 +139,47 @@ private InternalKeyBuilder HasKeyInternal(IReadOnlyList properties, Co var key = Metadata.FindDeclaredKey(actualProperties); if (key == null) { - if ((configurationSource != ConfigurationSource.Explicit) // let it throw for explicit - && (configurationSource == null - || actualProperties.Any(p => p.GetContainingForeignKeys().Any(k => k.DeclaringEntityType != Metadata)) - || actualProperties.Any(p => !p.Builder.CanSetRequired(true, configurationSource)))) + if (configurationSource == null) { return null; } + var containingForeignKeys = actualProperties + .SelectMany(p => p.GetContainingForeignKeys().Where(k => k.DeclaringEntityType != Metadata)) + .ToList(); + + if (containingForeignKeys.Any(fk => !configurationSource.Overrides(fk.GetForeignKeyPropertiesConfigurationSource()))) + { + return null; + } + + if (configurationSource != ConfigurationSource.Explicit // let it throw for explicit + && actualProperties.Any(p => !p.Builder.CanSetRequired(true, configurationSource))) + { + return null; + } + + var modifiedRelationships = containingForeignKeys + .Where(fk => fk.GetForeignKeyPropertiesConfigurationSource() != ConfigurationSource.Explicit) // let it throw for explicit + .Select(foreignKey => foreignKey.Builder.HasForeignKey(null, configurationSource, runConventions: false)) + .ToList(); + foreach (var actualProperty in actualProperties) { actualProperty.Builder.IsRequired(true, configurationSource.Value); } key = Metadata.AddKey(actualProperties, configurationSource.Value); + + foreach (var foreignKey in containingForeignKeys) + { + ModelBuilder.Metadata.ConventionDispatcher.OnForeignKeyRemoved(foreignKey.DeclaringEntityType.Builder, foreignKey); + } + + foreach (var relationship in modifiedRelationships) + { + ModelBuilder.Metadata.ConventionDispatcher.OnForeignKeyAdded(relationship); + } } else if (configurationSource.HasValue) { @@ -939,15 +966,17 @@ private static Dictionary> GroupRelations var detachedRelationships = property.GetContainingForeignKeys().ToList() .Select(DetachRelationship).ToList(); - foreach (var key in Metadata.GetKeys().Where(i => i.Properties.Contains(property)).ToList()) + var removedKeys = new List(); + foreach (var key in property.GetContainingKeys().ToList()) { detachedRelationships.AddRange(key.GetReferencingForeignKeys().ToList() .Select(DetachRelationship)); - var removed = RemoveKey(key, configurationSource); + var removed = RemoveKey(key, configurationSource, runConventions: false); Debug.Assert(removed.HasValue); + removedKeys.Add(key); } - foreach (var index in Metadata.GetIndexes().Where(i => i.Properties.Contains(property)).ToList()) + foreach (var index in property.GetContainingIndexes().ToList()) { var removed = RemoveIndex(index, configurationSource); Debug.Assert(removed.HasValue); @@ -959,6 +988,11 @@ private static Dictionary> GroupRelations Debug.Assert(removedProperty == property); } + foreach (var key in removedKeys) + { + Metadata.Model.ConventionDispatcher.OnKeyRemoved(this, key); + } + foreach (var detachedRelationship in detachedRelationships) { detachedRelationship.Attach(); diff --git a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/InternalPropertyBuilder.cs b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/InternalPropertyBuilder.cs index dcc45fce6f1..eb935bac4b4 100644 --- a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/InternalPropertyBuilder.cs +++ b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/InternalPropertyBuilder.cs @@ -132,13 +132,27 @@ public virtual bool HasValueGenerator( /// public virtual bool HasField([CanBeNull] string fieldName, ConfigurationSource configurationSource) { - if (configurationSource.Overrides(Metadata.GetFieldInfoConfigurationSource())) + if (Metadata.FieldInfo?.Name == fieldName) { Metadata.SetField(fieldName, configurationSource); return true; } - return false; + if (!configurationSource.Overrides(Metadata.GetFieldInfoConfigurationSource())) + { + return false; + } + + if (fieldName != null) + { + var fieldInfo = PropertyBase.GetFieldInfo(fieldName, Metadata.DeclaringType.ClrType, Metadata.Name, + shouldThrow: configurationSource == ConfigurationSource.Explicit); + Metadata.SetFieldInfo(fieldInfo, configurationSource); + return true; + } + + Metadata.SetField(fieldName, configurationSource); + return true; } /// @@ -147,7 +161,12 @@ public virtual bool HasField([CanBeNull] string fieldName, ConfigurationSource c /// public virtual bool HasFieldInfo([CanBeNull] FieldInfo fieldInfo, ConfigurationSource configurationSource) { - if (configurationSource.Overrides(Metadata.GetFieldInfoConfigurationSource())) + if ((configurationSource.Overrides(Metadata.GetFieldInfoConfigurationSource()) + && (fieldInfo == null + || PropertyBase.IsCompatible( + fieldInfo, Metadata.ClrType, Metadata.DeclaringType.ClrType, Metadata.Name, + shouldThrow: configurationSource == ConfigurationSource.Explicit))) + || Metadata.FieldInfo == fieldInfo) { Metadata.SetFieldInfo(fieldInfo, configurationSource); return true; diff --git a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/Property.cs b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/Property.cs index 942e0531a47..91923ab9897 100644 --- a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/Property.cs +++ b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/Property.cs @@ -533,19 +533,19 @@ public virtual PropertyIndexes PropertyIndexes /// 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. /// - public virtual IReadOnlyList Keys { get; [param: CanBeNull] set; } + public virtual List Keys { get; [param: CanBeNull] set; } /// /// 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. /// - public virtual IReadOnlyList ForeignKeys { get; [param: CanBeNull] set; } + public virtual List ForeignKeys { get; [param: CanBeNull] set; } /// /// 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. /// - public virtual IReadOnlyList Indexes { get; [param: CanBeNull] set; } + public virtual List Indexes { get; [param: CanBeNull] set; } /// /// This API supports the Entity Framework Core infrastructure and is not intended to be used diff --git a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/PropertyBase.cs b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/PropertyBase.cs index ba194c87e4a..7679bae27f6 100644 --- a/src/Microsoft.EntityFrameworkCore/Metadata/Internal/PropertyBase.cs +++ b/src/Microsoft.EntityFrameworkCore/Metadata/Internal/PropertyBase.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; using System.Linq; using System.Reflection; using JetBrains.Annotations; @@ -78,29 +79,44 @@ public virtual FieldInfo FieldInfo /// 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. /// - public virtual void SetField([CanBeNull] string fieldName, ConfigurationSource configurationSource) + public virtual void SetField([CanBeNull] string fieldName, ConfigurationSource configurationSource, bool runConventions = true) { if (fieldName == null) { - SetFieldInfo(null, configurationSource); + SetFieldInfo(null, configurationSource, runConventions); return; } - var typesInHierarchy = DeclaringType.ClrType.GetTypesInHierarchy().ToList(); + if (FieldInfo?.Name == fieldName) + { + SetFieldInfo(FieldInfo, configurationSource, runConventions); + return; + } - foreach (var type in typesInHierarchy) + var fieldInfo = GetFieldInfo(fieldName, DeclaringType.ClrType, Name, shouldThrow: true); + if (fieldInfo != null) { - var fields = type.GetRuntimeFields().ToDictionary(f => f.Name); - FieldInfo fieldInfo; - if (fields.TryGetValue(fieldName, out fieldInfo)) - { - SetFieldInfo(fieldInfo, configurationSource); - return; - } + SetFieldInfo(fieldInfo, configurationSource, runConventions); } + } - throw new InvalidOperationException( - CoreStrings.MissingBackingField(fieldName, Name, DeclaringType.DisplayName())); + /// + /// 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. + /// + public static FieldInfo GetFieldInfo([NotNull] string fieldName, [NotNull] Type type, [CanBeNull] string propertyName, bool shouldThrow) + { + Debug.Assert(propertyName != null || !shouldThrow); + + var fieldInfo = type.GetFieldInfo(fieldName); + if (fieldInfo == null + && shouldThrow) + { + throw new InvalidOperationException( + CoreStrings.MissingBackingField(fieldName, propertyName, type.ShortDisplayName())); + } + + return fieldInfo; } /// @@ -110,32 +126,69 @@ public virtual void SetField([CanBeNull] string fieldName, ConfigurationSource c public virtual void SetFieldInfo( [CanBeNull] FieldInfo fieldInfo, ConfigurationSource configurationSource, bool runConventions = true) { - if (fieldInfo != null - && !fieldInfo.FieldType.GetTypeInfo().IsAssignableFrom(ClrType.GetTypeInfo())) + if (ReferenceEquals(FieldInfo, fieldInfo)) { - throw new InvalidOperationException( - CoreStrings.BadBackingFieldType( - fieldInfo.Name, - fieldInfo.FieldType.ShortDisplayName(), - DeclaringType.DisplayName(), - Name, - ClrType.ShortDisplayName())); + UpdateFieldInfoConfigurationSource(configurationSource); + return; + } + + if (fieldInfo != null) + { + IsCompatible(fieldInfo, ClrType, DeclaringType.ClrType, Name, shouldThrow: true); } UpdateFieldInfoConfigurationSource(configurationSource); - if (!ReferenceEquals(FieldInfo, fieldInfo)) + var oldFieldInfo = FieldInfo; + _fieldInfo = fieldInfo; + + PropertyMetadataChanged(); + + if (runConventions) { - var oldFieldInfo = FieldInfo; - _fieldInfo = fieldInfo; + OnFieldInfoSet(oldFieldInfo); + } + } - PropertyMetadataChanged(); + /// + /// 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. + /// + public static bool IsCompatible( + [NotNull] FieldInfo fieldInfo, + [NotNull] Type propertyType, + [NotNull] Type entityClrType, + [CanBeNull] string propertyName, + bool shouldThrow) + { + Debug.Assert(propertyName != null || !shouldThrow); - if (runConventions) + if (!fieldInfo.FieldType.GetTypeInfo().IsAssignableFrom(propertyType.GetTypeInfo())) + { + if (shouldThrow) { - OnFieldInfoSet(oldFieldInfo); + throw new InvalidOperationException( + CoreStrings.BadBackingFieldType( + fieldInfo.Name, + fieldInfo.FieldType.ShortDisplayName(), + entityClrType.ShortDisplayName(), + propertyName, + propertyType.ShortDisplayName())); } + return false; } + + if (!fieldInfo.DeclaringType.GetTypeInfo().IsAssignableFrom(entityClrType.GetTypeInfo())) + { + if (shouldThrow) + { + throw new InvalidOperationException( + CoreStrings.MissingBackingField(fieldInfo.Name, propertyName, entityClrType.ShortDisplayName())); + } + return false; + } + + return true; } /// diff --git a/src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.Designer.cs b/src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.Designer.cs index 94e79851898..a33347f5ec7 100644 --- a/src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.Designer.cs +++ b/src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.Designer.cs @@ -449,11 +449,11 @@ public static string ClrPropertyOnShadowEntity([CanBeNull] object property, [Can } /// - /// The property '{property}' cannot be removed from entity type '{entityType}' because it is being used in an index or key. All indexes and keys must be removed or redefined before the property can be removed. + /// The property '{property}' cannot be removed from entity type '{entityType}' because it is being used in the key {key}. All containing keys must be removed or redefined before the property can be removed. /// - public static string PropertyInUse([CanBeNull] object property, [CanBeNull] object entityType) + public static string PropertyInUseKey([CanBeNull] object property, [CanBeNull] object entityType, [CanBeNull] object key) { - return string.Format(CultureInfo.CurrentCulture, GetString("PropertyInUse", "property", "entityType"), property, entityType); + return string.Format(CultureInfo.CurrentCulture, GetString("PropertyInUseKey", "property", "entityType", "key"), property, entityType, key); } /// @@ -1464,6 +1464,22 @@ public static string PropertyCalledOnNavigation([CanBeNull] object property, [Ca return string.Format(CultureInfo.CurrentCulture, GetString("PropertyCalledOnNavigation", "property", "entityType"), property, entityType); } + /// + /// The property '{property}' cannot be removed from entity type '{entityType}' because it is being used in the foreign key {foreignKey} on '{foreignKeyType}'. All containing foreign keys must be removed or redefined before the property can be removed. + /// + public static string PropertyInUseForeignKey([CanBeNull] object property, [CanBeNull] object entityType, [CanBeNull] object foreignKey, [CanBeNull] object foreignKeyType) + { + return string.Format(CultureInfo.CurrentCulture, GetString("PropertyInUseForeignKey", "property", "entityType", "foreignKey", "foreignKeyType"), property, entityType, foreignKey, foreignKeyType); + } + + /// + /// The property '{property}' cannot be removed from entity type '{entityType}' because it is being used in the index {index} on '{indexType}'. All containing indexes must be removed or redefined before the property can be removed. + /// + public static string PropertyInUseIndex([CanBeNull] object property, [CanBeNull] object entityType, [CanBeNull] object index, [CanBeNull] object indexType) + { + return string.Format(CultureInfo.CurrentCulture, GetString("PropertyInUseIndex", "property", "entityType", "index", "indexType"), property, entityType, index, indexType); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.resx b/src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.resx index 5c6b50a6bcf..faa1fb00e85 100644 --- a/src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.resx +++ b/src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.resx @@ -279,8 +279,8 @@ The property '{property}' cannot exist on type '{entityType}' because the type is marked as shadow state while the property is not. Shadow state types can only contain shadow state properties. - - The property '{property}' cannot be removed from entity type '{entityType}' because it is being used in an index or key. All indexes and keys must be removed or redefined before the property can be removed. + + The property '{property}' cannot be removed from entity type '{entityType}' because it is being used in the key {key}. All containing keys must be removed or redefined before the property can be removed. Cannot remove key {key} from entity type '{entityType}' because it is referenced by a foreign key in entity type '{dependentType}'. All foreign keys must be removed or redefined before the referenced key can be removed. @@ -660,4 +660,10 @@ Cannot call Property for the property '{property}' on entity type '{entityType}' because it is configured as a navigation property. Property can only be used to configure scalar properties. + + The property '{property}' cannot be removed from entity type '{entityType}' because it is being used in the foreign key {foreignKey} on '{foreignKeyType}'. All containing foreign keys must be removed or redefined before the property can be removed. + + + The property '{property}' cannot be removed from entity type '{entityType}' because it is being used in the index {index} on '{indexType}'. All containing indexes must be removed or redefined before the property can be removed. + \ No newline at end of file diff --git a/test/Microsoft.EntityFrameworkCore.Tests/Metadata/Internal/EntityTypeTest.cs b/test/Microsoft.EntityFrameworkCore.Tests/Metadata/Internal/EntityTypeTest.cs index 9949215b231..5b2f7324eb7 100644 --- a/test/Microsoft.EntityFrameworkCore.Tests/Metadata/Internal/EntityTypeTest.cs +++ b/test/Microsoft.EntityFrameworkCore.Tests/Metadata/Internal/EntityTypeTest.cs @@ -2402,7 +2402,7 @@ public void Cannot_remove_property_when_used_by_primary_key() entityType.GetOrSetPrimaryKey(property); Assert.Equal( - CoreStrings.PropertyInUse("Id", typeof(Customer).Name), + CoreStrings.PropertyInUseKey("Id", typeof(Customer).Name, "{'Id'}"), Assert.Throws(() => entityType.RemoveProperty(property.Name)).Message); } @@ -2416,7 +2416,7 @@ public void Cannot_remove_property_when_used_by_non_primary_key() entityType.GetOrAddKey(property); Assert.Equal( - CoreStrings.PropertyInUse("Id", typeof(Customer).Name), + CoreStrings.PropertyInUseKey("Id", typeof(Customer).Name, "{'Id'}"), Assert.Throws(() => entityType.RemoveProperty(property.Name)).Message); } @@ -2432,7 +2432,7 @@ public void Cannot_remove_property_when_used_by_foreign_key() orderType.GetOrAddForeignKey(customerFk, customerPk, customerType); Assert.Equal( - CoreStrings.PropertyInUse("CustomerId", typeof(Order).Name), + CoreStrings.PropertyInUseForeignKey("CustomerId", typeof(Order).Name, "{'CustomerId'}", typeof(Order).Name), Assert.Throws(() => orderType.RemoveProperty(customerFk.Name)).Message); } @@ -2446,7 +2446,7 @@ public void Cannot_remove_property_when_used_by_an_index() entityType.GetOrAddIndex(property); Assert.Equal( - CoreStrings.PropertyInUse("Id", typeof(Customer).Name), + CoreStrings.PropertyInUseIndex("Id", typeof(Customer).Name, "{'Id'}", typeof(Customer).Name), Assert.Throws(() => entityType.RemoveProperty(property.Name)).Message); } diff --git a/test/Microsoft.EntityFrameworkCore.Tests/ModelBuilderTest/InheritanceTestBase.cs b/test/Microsoft.EntityFrameworkCore.Tests/ModelBuilderTest/InheritanceTestBase.cs index eccf6cf7d73..ac3aabfcaaa 100644 --- a/test/Microsoft.EntityFrameworkCore.Tests/ModelBuilderTest/InheritanceTestBase.cs +++ b/test/Microsoft.EntityFrameworkCore.Tests/ModelBuilderTest/InheritanceTestBase.cs @@ -299,6 +299,40 @@ public virtual void Pulling_relationship_to_a_derived_type_with_fk_creates_relat Assert.Equal(nameof(Order.CustomerId), otherDerivedFk.Properties.Single().Name); } + [Fact] + public virtual void Removing_a_key_triggers_fk_discovery_on_derived_types() + { + var modelBuilder = CreateModelBuilder(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + + var principalEntityBuilder = modelBuilder.Entity(); + var derivedPrincipalEntityBuilder = modelBuilder.Entity(); + var dependentEntityBuilder = modelBuilder.Entity(); + dependentEntityBuilder.Ignore(nameof(Order.Customer)); + var derivedDependentEntityBuilder = modelBuilder.Entity(); + dependentEntityBuilder.Property("SpecialCustomerId"); + + derivedPrincipalEntityBuilder + .HasMany(e => (IEnumerable)e.Orders) + .WithOne(e => e.SpecialCustomer); + + dependentEntityBuilder.HasKey("SpecialCustomerId"); + dependentEntityBuilder.HasKey(o => o.OrderId); + dependentEntityBuilder.Ignore("SpecialCustomerId"); + derivedDependentEntityBuilder.Property(e => e.SpecialCustomerId); + + Assert.Null(dependentEntityBuilder.Metadata.FindProperty("SpecialCustomerId")); + Assert.NotNull(derivedDependentEntityBuilder.Metadata.FindProperty("SpecialCustomerId")); + Assert.Empty(principalEntityBuilder.Metadata.GetNavigations()); + var newFk = derivedDependentEntityBuilder.Metadata.GetDeclaredNavigations().Single().ForeignKey; + Assert.Equal(nameof(SpecialOrder.SpecialCustomer), newFk.DependentToPrincipal.Name); + Assert.Equal(nameof(SpecialCustomer.Orders), newFk.PrincipalToDependent.Name); + Assert.Same(derivedPrincipalEntityBuilder.Metadata, newFk.PrincipalEntityType); + } + [Fact] public virtual void Index_removed_when_covered_by_an_inherited_foreign_key() {