Skip to content

Commit

Permalink
Properly remove incompatible entity types with the same name.
Browse files Browse the repository at this point in the history
Fixes #22051
  • Loading branch information
AndriySvyryd committed Aug 15, 2020
1 parent 0cbbd45 commit b12713d
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 34 deletions.
5 changes: 4 additions & 1 deletion src/EFCore/Metadata/Internal/EntityType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
}
Expand Down
46 changes: 33 additions & 13 deletions src/EFCore/Metadata/Internal/InternalModelBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion src/EFCore/Metadata/Internal/Model.cs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,9 @@ public virtual EntityType FindActualEntityType([NotNull] EntityType entityType)
/// </summary>
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);
Expand Down
18 changes: 13 additions & 5 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@
<value>The entity type '{entityType}' cannot inherit from '{baseEntityType}' because '{clrType}' is not a descendant of '{baseClrType}'.</value>
</data>
<data name="PropertyWrongEntityClrType" xml:space="preserve">
<value>CLR property '{property}' cannot be added to entity type '{entityType}' because it is declared on the CLR type '{clrType}'.</value>
<value>The CLR property '{property}' cannot be added to entity type '{entityType}' because it is declared on the CLR type '{clrType}'.</value>
</data>
<data name="InvalidNavigationWithInverseProperty" xml:space="preserve">
<value>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.</value>
Expand Down Expand Up @@ -1427,7 +1427,7 @@
<value>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.</value>
</data>
<data name="ClashingMismatchedSharedType" xml:space="preserve">
<value>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.</value>
<value>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.</value>
</data>
<data name="ClashingNonSharedType" xml:space="preserve">
<value>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.</value>
Expand Down Expand Up @@ -1498,4 +1498,7 @@
<data name="ClashingNamedOwnedType" xml:space="preserve">
<value>There is already an entity type named '{ownedTypeName}'. Use a different name when configuring the ownership '{ownerEntityType}.{navigation}'.</value>
</data>
<data name="PropertyWrongEntitySharedClrType" xml:space="preserve">
<value>The CLR property '{property}' cannot be added to entity type '{entityType}'({sharedType}) because it is declared on the CLR type '{clrType}'.</value>
</data>
</root>
31 changes: 19 additions & 12 deletions test/EFCore.Tests/Metadata/Internal/InternalModelBuilderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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<InvalidOperationException>(() => 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<InvalidOperationException>(() => 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<InvalidOperationException>(() =>
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)
Expand Down

0 comments on commit b12713d

Please sign in to comment.