Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly remove incompatible entity types with the same name. #22075

Merged
1 commit merged into from
Aug 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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