From b12713d094ad59d94e9ce7d247b0596548c988e3 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Fri, 14 Aug 2020 16:35:26 -0700 Subject: [PATCH] Properly remove incompatible entity types with the same name. Fixes #22051 --- src/EFCore/Metadata/Internal/EntityType.cs | 5 +- .../Metadata/Internal/InternalModelBuilder.cs | 46 +++++++++++++------ src/EFCore/Metadata/Internal/Model.cs | 4 +- src/EFCore/Properties/CoreStrings.Designer.cs | 18 ++++++-- src/EFCore/Properties/CoreStrings.resx | 7 ++- .../Internal/InternalModelBuilderTest.cs | 31 ++++++++----- 6 files changed, 77 insertions(+), 34 deletions(-) diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs index d5b535e8e82..0a82e70cb06 100644 --- a/src/EFCore/Metadata/Internal/EntityType.cs +++ b/src/EFCore/Metadata/Internal/EntityType.cs @@ -2365,7 +2365,10 @@ public virtual Property AddProperty( if (memberInfo.DeclaringType?.IsAssignableFrom(ClrType) != true) { throw new InvalidOperationException( - CoreStrings.PropertyWrongEntityClrType( + HasSharedClrType + ? CoreStrings.PropertyWrongEntitySharedClrType( + memberInfo.Name, this.DisplayName(), ClrType.ShortDisplayName(), memberInfo.DeclaringType?.ShortDisplayName()) + : CoreStrings.PropertyWrongEntityClrType( memberInfo.Name, this.DisplayName(), memberInfo.DeclaringType?.ShortDisplayName())); } } diff --git a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs index bbc872bbc1e..53f49c51bdf 100644 --- a/src/EFCore/Metadata/Internal/InternalModelBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalModelBuilder.cs @@ -82,13 +82,25 @@ private InternalEntityTypeBuilder Entity( EntityType entityType; if (type.IsNamed) { - if (type.Type != null - && (Metadata.FindEntityType(Metadata.GetDisplayName(type.Type)) != null - || Metadata.HasEntityTypeWithDefiningNavigation(type.Type))) + if (type.Type != null) { - return configurationSource == ConfigurationSource.Explicit - ? throw new InvalidOperationException(CoreStrings.ClashingNonSharedType(type.Name, type.Type.DisplayName())) - : (InternalEntityTypeBuilder)null; + var nonSharedTypes = Metadata.GetEntityTypes(Metadata.GetDisplayName(type.Type)); + foreach (var nonSharedType in nonSharedTypes) + { + if (configurationSource.OverridesStrictly(nonSharedType.GetConfigurationSource())) + { + continue; + } + + return configurationSource == ConfigurationSource.Explicit + ? throw new InvalidOperationException(CoreStrings.ClashingNonSharedType(type.Name, type.Type.DisplayName())) + : (InternalEntityTypeBuilder)null; + } + + foreach (var nonSharedType in nonSharedTypes) + { + HasNoEntityType(nonSharedType, configurationSource); + } } entityType = Metadata.FindEntityType(type.Name); @@ -139,16 +151,24 @@ private InternalEntityTypeBuilder Entity( if (entityType != null) { - if (type.IsNamed && type.Type != null) + if (type.Type == null + || entityType.ClrType == type.Type) { - if (entityType.ClrType != type.Type) - { - throw new InvalidOperationException(CoreStrings.ClashingMismatchedSharedType(type.Name)); - } + entityType.UpdateConfigurationSource(configurationSource); + return entityType.Builder; } - entityType.UpdateConfigurationSource(configurationSource); - return entityType.Builder; + if (configurationSource.OverridesStrictly(entityType.GetConfigurationSource())) + { + HasNoEntityType(entityType, configurationSource); + } + else + { + return configurationSource == ConfigurationSource.Explicit + ? throw new InvalidOperationException( + CoreStrings.ClashingMismatchedSharedType(type.Name, entityType.ClrType?.ShortDisplayName())) + : (InternalEntityTypeBuilder)null; + } } Metadata.RemoveIgnored(type.Name); diff --git a/src/EFCore/Metadata/Internal/Model.cs b/src/EFCore/Metadata/Internal/Model.cs index 6cd80b1e42a..31232c3b1f5 100644 --- a/src/EFCore/Metadata/Internal/Model.cs +++ b/src/EFCore/Metadata/Internal/Model.cs @@ -568,7 +568,9 @@ public virtual EntityType FindActualEntityType([NotNull] EntityType entityType) /// public virtual Type FindClrType([NotNull] string name) => _entityTypes.TryGetValue(name, out var entityType) - ? entityType.ClrType + ? entityType.HasSharedClrType + ? null + : entityType.ClrType : (_entityTypesWithDefiningNavigation.TryGetValue(name, out var entityTypesWithSameType) ? entityTypesWithSameType.FirstOrDefault()?.ClrType : null); diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 710b441d838..232603a907c 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -1057,7 +1057,7 @@ public static string NotAssignableClrBaseType([CanBeNull] object entityType, [Ca entityType, baseEntityType, clrType, baseClrType); /// - /// CLR property '{property}' cannot be added to entity type '{entityType}' because it is declared on the CLR type '{clrType}'. + /// The CLR property '{property}' cannot be added to entity type '{entityType}' because it is declared on the CLR type '{clrType}'. /// public static string PropertyWrongEntityClrType([CanBeNull] object property, [CanBeNull] object entityType, [CanBeNull] object clrType) => string.Format( @@ -2701,12 +2701,12 @@ public static string PrincipalKeylessType([CanBeNull] object entityType, [CanBeN entityType, firstNavigationSpecification, secondNavigationSpecification); /// - /// The shared type entity type '{entityType}' cannot be added to the model because a shared entity type with the same name but different clr type already exists. + /// The shared type entity type '{entityType}' cannot be added to the model because a shared entity type with the same name but different clr type '{otherClrType}' already exists. /// - public static string ClashingMismatchedSharedType([CanBeNull] object entityType) + public static string ClashingMismatchedSharedType([CanBeNull] object entityType, [CanBeNull] object otherClrType) => string.Format( - GetString("ClashingMismatchedSharedType", nameof(entityType)), - entityType); + GetString("ClashingMismatchedSharedType", nameof(entityType), nameof(otherClrType)), + entityType, otherClrType); /// /// The shared type entity type '{entityType}' with clr type '{type}' cannot be added to the model because a non shared entity type with the same clr type already exists. @@ -2828,6 +2828,14 @@ public static string ClashingNamedOwnedType([CanBeNull] object ownedTypeName, [C GetString("ClashingNamedOwnedType", nameof(ownedTypeName), nameof(ownerEntityType), nameof(navigation)), ownedTypeName, ownerEntityType, navigation); + /// + /// The CLR property '{property}' cannot be added to entity type '{entityType}'({sharedType}) because it is declared on the CLR type '{clrType}'. + /// + public static string PropertyWrongEntitySharedClrType([CanBeNull] object property, [CanBeNull] object entityType, [CanBeNull] object sharedType, [CanBeNull] object clrType) + => string.Format( + GetString("PropertyWrongEntitySharedClrType", nameof(property), nameof(entityType), nameof(sharedType), nameof(clrType)), + property, entityType, sharedType, clrType); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 65bfe08cbe5..21ae743bc56 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -652,7 +652,7 @@ The entity type '{entityType}' cannot inherit from '{baseEntityType}' because '{clrType}' is not a descendant of '{baseClrType}'. - CLR property '{property}' cannot be added to entity type '{entityType}' because it is declared on the CLR type '{clrType}'. + The CLR property '{property}' cannot be added to entity type '{entityType}' because it is declared on the CLR type '{clrType}'. The InversePropertyAttribute on property '{property}' on type '{entityType}' is not valid. The property '{referencedProperty}' is not a valid navigation property on the related type '{referencedEntityType}'. Ensure that the property exists and is a valid reference or collection navigation property. @@ -1427,7 +1427,7 @@ The entity type '{entityType}' cannot be on the principal end of the relationship between '{firstNavigationSpecification}' and '{secondNavigationSpecification}'. The principal entity type must have a key. - The shared type entity type '{entityType}' cannot be added to the model because a shared entity type with the same name but different clr type already exists. + The shared type entity type '{entityType}' cannot be added to the model because a shared entity type with the same name but different clr type '{otherClrType}' already exists. The shared type entity type '{entityType}' with clr type '{type}' cannot be added to the model because a non shared entity type with the same clr type already exists. @@ -1498,4 +1498,7 @@ There is already an entity type named '{ownedTypeName}'. Use a different name when configuring the ownership '{ownerEntityType}.{navigation}'. + + The CLR property '{property}' cannot be added to entity type '{entityType}'({sharedType}) because it is declared on the CLR type '{clrType}'. + \ No newline at end of file diff --git a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs index 2d82b8ad79b..6de3dc6e53b 100644 --- a/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs @@ -31,20 +31,22 @@ public void Entity_returns_same_instance_for_entity_clr_type() Assert.NotNull(entityBuilder); Assert.NotNull(model.FindEntityType(typeof(Customer))); - Assert.Same(entityBuilder, modelBuilder.Entity(typeof(Customer).FullName, ConfigurationSource.DataAnnotation)); + Assert.Same(entityBuilder, modelBuilder.Entity(typeof(Customer).DisplayName(), ConfigurationSource.DataAnnotation)); + Assert.NotNull(model.FindEntityType(typeof(Customer)).ClrType); } [ConditionalFact] - public void Entity_returns_same_instance_for_entity_type_name() + public void Entity_creates_new_instance_for_entity_type_name() { var model = new Model(); var modelBuilder = CreateModelBuilder(model); - var entityBuilder = modelBuilder.Entity(typeof(Customer).FullName, ConfigurationSource.DataAnnotation); + var entityBuilder = modelBuilder.Entity(typeof(Customer).DisplayName(), ConfigurationSource.DataAnnotation); Assert.NotNull(entityBuilder); - Assert.NotNull(model.FindEntityType(typeof(Customer).FullName)); - Assert.Same(entityBuilder, modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit)); + Assert.NotNull(model.FindEntityType(typeof(Customer).DisplayName())); + Assert.NotSame(entityBuilder, modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit)); + Assert.NotNull(model.FindEntityType(typeof(Customer)).ClrType); } [ConditionalFact] @@ -469,27 +471,32 @@ public void Can_add_shared_type() var model = new Model(); var modelBuilder = CreateModelBuilder(model); - var entityBuilder = modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit); var sharedTypeName = "SpecialDetails"; Assert.NotNull(modelBuilder.SharedTypeEntity(sharedTypeName, typeof(Details), ConfigurationSource.Convention)); Assert.True(model.FindEntityType(sharedTypeName).HasSharedClrType); - Assert.Equal( - CoreStrings.ClashingMismatchedSharedType("SpecialDetails"), - Assert.Throws(() => modelBuilder.SharedTypeEntity(sharedTypeName, typeof(Product), ConfigurationSource.DataAnnotation)).Message); + Assert.Null(modelBuilder.SharedTypeEntity(sharedTypeName, typeof(Product), ConfigurationSource.Convention)); Assert.NotNull(modelBuilder.Entity(typeof(Product), ConfigurationSource.DataAnnotation)); Assert.Null(modelBuilder.SharedTypeEntity(typeof(Product).DisplayName(), typeof(Product), ConfigurationSource.DataAnnotation)); - Assert.NotNull(modelBuilder.Entity(typeof(Product), ConfigurationSource.Explicit)); + Assert.NotNull(modelBuilder.SharedTypeEntity(sharedTypeName, typeof(Product), ConfigurationSource.Explicit)); + + Assert.Equal(typeof(Product), model.FindEntityType(sharedTypeName).ClrType); + + Assert.Equal( + CoreStrings.ClashingMismatchedSharedType("SpecialDetails", nameof(Product)), + Assert.Throws(() => modelBuilder.SharedTypeEntity(sharedTypeName, typeof(Details), ConfigurationSource.Explicit)).Message); + + Assert.NotNull(modelBuilder.Entity(typeof(Customer), ConfigurationSource.Explicit)); Assert.Equal( - CoreStrings.ClashingNonSharedType(typeof(Product).DisplayName(), typeof(Product).DisplayName()), + CoreStrings.ClashingNonSharedType(typeof(Customer).DisplayName(), typeof(Customer).DisplayName()), Assert.Throws(() => - modelBuilder.SharedTypeEntity(typeof(Product).DisplayName(), typeof(Product), ConfigurationSource.Explicit)).Message); + modelBuilder.SharedTypeEntity(typeof(Customer).DisplayName(), typeof(Customer), ConfigurationSource.Explicit)).Message); } private static void Cleanup(InternalModelBuilder modelBuilder)