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

Validate that derived types in TPT aren't sharing table as dependants #21155

Merged
merged 1 commit into from
Jun 8, 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
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public static string GetDefaultName([NotNull] this IForeignKey foreignKey)
.Append("_")
.Append(principalTableName)
.Append("_")
.AppendJoin(foreignKey.Properties.Select(p => p.GetColumnName(tableName, schema)), "_")
.AppendJoin(foreignKey.Properties.Select(p => p.GetColumnName()), "_")
.ToString();

return Uniquifier.Truncate(name, foreignKey.DeclaringEntityType.Model.GetMaxIdentifierLength());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public static string GetDefaultDatabaseName([NotNull] this IIndex index)
.Append("IX_")
.Append(tableName)
.Append("_")
.AppendJoin(index.Properties.Select(p => p.GetColumnName(tableName, schema)), "_")
.AppendJoin(index.Properties.Select(p => p.GetColumnName()), "_")
.ToString();

return Uniquifier.Truncate(baseName, index.DeclaringEntityType.Model.GetMaxIdentifierLength());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public static string GetDefaultName([NotNull] this IKey key)
.Append("AK_")
.Append(tableName)
.Append("_")
.AppendJoin(key.Properties.Select(p => p.GetColumnName(tableName, key.DeclaringEntityType.GetSchema())), "_")
.AppendJoin(key.Properties.Select(p => p.GetColumnName()), "_")
.ToString();
}

Expand Down
31 changes: 27 additions & 4 deletions src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,29 @@ protected virtual void ValidateSharedTableCompatibility(
IEntityType root = null;
foreach (var mappedType in mappedTypes)
{
if ((mappedType.BaseType != null && unvalidatedTypes.Contains(mappedType.BaseType))
|| (mappedType.FindPrimaryKey() != null
if (mappedType.BaseType != null && unvalidatedTypes.Contains(mappedType.BaseType))
{
continue;
}

if (mappedType.FindPrimaryKey() != null
&& mappedType.FindForeignKeys(mappedType.FindPrimaryKey().Properties)
.Any(fk => fk.PrincipalKey.IsPrimaryKey()
&& unvalidatedTypes.Contains(fk.PrincipalEntityType))))
&& unvalidatedTypes.Contains(fk.PrincipalEntityType)))
{
if (mappedType.BaseType != null)
{
var principalType = mappedType.FindForeignKeys(mappedType.FindPrimaryKey().Properties)
.First(fk => fk.PrincipalKey.IsPrimaryKey()
&& unvalidatedTypes.Contains(fk.PrincipalEntityType))
.PrincipalEntityType;
throw new InvalidOperationException(
RelationalStrings.IncompatibleTableDerivedRelationship(
Format(tableName, schema),
mappedType.DisplayName(),
principalType.DisplayName()));
}

continue;
}

Expand Down Expand Up @@ -352,7 +369,13 @@ protected virtual void ValidateSharedColumnsCompatibility(
storeConcurrencyTokens = new Dictionary<string, IProperty>();
}

storeConcurrencyTokens[property.GetColumnName(tableName, schema)] = property;
var columnName = property.GetColumnName(tableName, schema);
if (columnName == null)
{
continue;
}

storeConcurrencyTokens[columnName] = property;
if (missingConcurrencyTokens == null)
{
missingConcurrencyTokens = new HashSet<string>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ private static void TryUniquifyColumnNames(
foreach (var property in entityType.GetDeclaredProperties())
{
var columnName = property.GetColumnName(tableName, schema);
if (columnName == null)
{
continue;
}

if (!properties.TryGetValue(columnName, out var otherProperty))
{
properties[columnName] = property;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ private void GetMappings(IConventionModel model,
}

var columnName = property.GetColumnName(tableName, table.Schema);
if (columnName == null)
{
continue;
}

if (!columnToProperties.TryGetValue(columnName, out var properties))
{
properties = new List<IConventionProperty>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,8 @@ public static bool AreCompatible(
return false;
}

if (!foreignKey.PrincipalKey.Properties
.Select(p => p.GetColumnName(tableName, schema))
.SequenceEqual(
duplicateForeignKey.PrincipalKey.Properties
.Select(p => p.GetColumnName(tableName, schema))))
if (!foreignKey.PrincipalKey.Properties.Select(p => p.GetColumnName(tableName, schema))
.SequenceEqual(duplicateForeignKey.PrincipalKey.Properties.Select(p => p.GetColumnName(tableName, schema))))
{
if (shouldThrow)
{
Expand Down
12 changes: 10 additions & 2 deletions src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

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

3 changes: 3 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -654,4 +654,7 @@
<value>The unnamed index on the entity type '{entityType}' specifies properties {indexPropertiesList}. The property '{propertyName1}' is mapped to table(s) {tableList1}, whereas the property '{propertyName2}' is mapped to table(s) {tableList2}. All index properties must map to at least one common table.</value>
<comment>Error RelationalEventId.IndexPropertiesMappedToNonOverlappingTables string string string string string string</comment>
</data>
<data name="IncompatibleTableDerivedRelationship" xml:space="preserve">
<value>Cannot use table '{table}' for entity type '{entityType}' since it is being used for entity type '{otherEntityType}', there is a relationship between their primary keys in which '{entityType}' is the dependent and '{entityType}' has a base entity type mapped to a different table. Either map '{otherEntityType}' to a different table or invert the relationship between '{entityType}' and '{otherEntityType}'.</value>
</data>
</root>
8 changes: 7 additions & 1 deletion src/EFCore.SqlServer/Internal/SqlServerModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,13 @@ protected override void ValidateSharedColumnsCompatibility(
{
if (property.GetValueGenerationStrategy(tableName, schema) == SqlServerValueGenerationStrategy.IdentityColumn)
{
identityColumns[property.GetColumnName(tableName, schema)] = property;
var columnName = property.GetColumnName(tableName, schema);
if (columnName == null)
{
continue;
}

identityColumns[columnName] = property;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ private string CreateIndexFilter(IIndex index)
{
return null;
}

var schema = index.DeclaringEntityType.GetSchema();

var nullableColumns = index.Properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ public static bool AreCompatibleForSqlServer(
if (index.GetIncludeProperties() == null
|| duplicateIndex.GetIncludeProperties() == null
|| !index.GetIncludeProperties().Select(
p => index.DeclaringEntityType.FindProperty(p).GetColumnName(tableName, schema)).SequenceEqual(
duplicateIndex.GetIncludeProperties().Select(
p => duplicateIndex.DeclaringEntityType.FindProperty(p).GetColumnName(tableName, schema)))) {
p => index.DeclaringEntityType.FindProperty(p).GetColumnName(tableName, schema))
.SequenceEqual(
duplicateIndex.GetIncludeProperties().Select(
p => duplicateIndex.DeclaringEntityType.FindProperty(p).GetColumnName(tableName, schema))))
{
if (shouldThrow)
{
throw new InvalidOperationException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,49 @@ public virtual void Detects_view_TPT_with_discriminator()
modelBuilder.Model);
}

[ConditionalFact]
public virtual void Passes_on_valid_table_sharing_with_TPT()
{
var modelBuilder = CreateConventionalModelBuilder();

modelBuilder.Entity<Animal>()
.Ignore(a => a.FavoritePerson);

modelBuilder.Entity<Cat>(
x =>
{
x.ToTable("Cat");
x.HasOne(c => c.FavoritePerson).WithOne().HasForeignKey<Person>(c => c.Id);
});

modelBuilder.Entity<Person>().ToTable("Cat");

Validate(modelBuilder.Model);
}

[ConditionalFact]
public virtual void Detects_linking_relationship_on_derived_type_in_TPT()
{
var modelBuilder = CreateConventionalModelBuilder();

modelBuilder.Entity<Animal>()
.Ignore(a => a.FavoritePerson);

modelBuilder.Entity<Cat>(
x =>
{
x.ToTable("Cat");
x.HasOne(c => c.FavoritePerson).WithOne().HasForeignKey<Cat>(c => c.Id);
});

modelBuilder.Entity<Person>().ToTable("Cat");

VerifyError(
RelationalStrings.IncompatibleTableDerivedRelationship(
"Cat", "Cat", "Person"),
modelBuilder.Model);
}

[ConditionalFact]
public virtual void Passes_for_valid_table_overrides()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Conventions;
Expand Down Expand Up @@ -36,6 +37,23 @@ public virtual void Missing_concurrency_token_property_is_created_on_the_base_ty
Assert.Equal(ValueGenerated.OnAddOrUpdate, concurrencyProperty.ValueGenerated);
}

[ConditionalFact]
public virtual void Missing_concurrency_token_property_is_not_created_for_TPT()
{
var modelBuilder = GetModelBuilder();
modelBuilder.Entity<Animal>().Ignore(a => a.FavoritePerson)
.Property<byte[]>("Version").IsRowVersion().HasColumnName("Version");
modelBuilder.Entity<Cat>().HasOne(a => a.FavoritePerson).WithOne().HasForeignKey<Person>(p => p.Id);
modelBuilder.Entity<Cat>().ToTable(nameof(Cat));
modelBuilder.Entity<Person>().ToTable(nameof(Cat));

var model = modelBuilder.Model;
model.FinalizeModel();

var person = model.FindEntityType(typeof(Person));
Assert.DoesNotContain(person.GetProperties(), p => p.IsConcurrencyToken);
}

[ConditionalFact]
public virtual void Missing_concurrency_token_properties_are_created_on_the_base_most_types()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ protected override void BuildModel(ModelBuilder modelBuilder)

b.HasIndex("NormalizedName")
.IsUnique()
.HasName("RoleNameIndex")
.HasName("RoleNameIndex") // Don't change to HasDatabaseName
.HasFilter("[NormalizedName] IS NOT NULL");

b.ToTable("AspNetRoles");
Expand Down