Skip to content

Commit

Permalink
Default to the principal value generation strategy for the primary ke…
Browse files Browse the repository at this point in the history
…y columns for entity types sharing the same table.

Validate that properties sharing the same column have the same value generation strategy.

Fixes #9652
  • Loading branch information
AndriySvyryd committed Sep 14, 2017
1 parent a866d1c commit 92dcb80
Show file tree
Hide file tree
Showing 12 changed files with 230 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ protected override void Configure(ReferenceOwnershipBuilder<Level3, Level4> l4)

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder).ConfigureWarnings(
c => c
.Log(RelationalEventId.QueryClientEvaluationWarning));
c => c.Log(RelationalEventId.QueryClientEvaluationWarning));
}
}
59 changes: 59 additions & 0 deletions src/EFCore.Relational/Metadata/Internal/PropertyExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Internal;

namespace Microsoft.EntityFrameworkCore.Metadata.Internal
{
/// <summary>
/// Extension methods for <see cref="IProperty" /> for relational database metadata.
/// </summary>
public static class PropertyExtensions
{
/// <summary>
/// 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.
/// </summary>
public static IProperty FindSharedTablePrincipalPrimaryKeyProperty([NotNull] this IProperty property)
{
var linkingRelationship = property.FindSharedTableLink();
return linkingRelationship?.PrincipalKey.Properties[linkingRelationship.Properties.IndexOf(property)];
}

/// <summary>
/// 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.
/// </summary>
public static IForeignKey FindSharedTableLink([NotNull] this IProperty property)
{
var pk = property.GetContainingPrimaryKey();
if (pk == null)
{
return null;
}

var entityType = property.DeclaringEntityType;

foreach (var fk in entityType.FindForeignKeys(pk.Properties))
{
if (!fk.PrincipalKey.IsPrimaryKey()
|| fk.PrincipalEntityType == fk.DeclaringEntityType)
{
continue;
}

var principalEntityType = fk.PrincipalEntityType;
var entityTypeAnnotations = fk.DeclaringEntityType.Relational();
var principalTypeAnnotations = principalEntityType.Relational();
if (entityTypeAnnotations.TableName == principalTypeAnnotations.TableName
&& entityTypeAnnotations.Schema == principalTypeAnnotations.Schema)
{
return fk;
}
}

return null;
}
}
}
80 changes: 31 additions & 49 deletions src/EFCore.Relational/Metadata/RelationalPropertyAnnotations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,67 +93,49 @@ public virtual string ColumnName

private string GetDefaultColumnName()
{
var sharedTablePrincipalPrimaryKeyProperty = Property.FindSharedTablePrincipalPrimaryKeyProperty();
if (sharedTablePrincipalPrimaryKeyProperty != null)
{
return GetAnnotations(sharedTablePrincipalPrimaryKeyProperty).ColumnName;
}

var entityType = Property.DeclaringEntityType;
var pk = Property.GetContainingPrimaryKey();
if (pk != null)
StringBuilder builder = null;
do
{
foreach (var fk in entityType.FindForeignKeys(pk.Properties))
var ownership = entityType.GetForeignKeys().SingleOrDefault(fk => fk.IsOwnership);
if (ownership == null)
{
if (!fk.PrincipalKey.IsPrimaryKey()
|| fk.PrincipalKey == pk)
{
continue;
}

var principalEntityType = fk.PrincipalEntityType;
var entityTypeAnnotations = GetAnnotations(entityType);
var principalTypeAnnotations = GetAnnotations(principalEntityType);
if (entityTypeAnnotations.TableName == principalTypeAnnotations.TableName
&& entityTypeAnnotations.Schema == principalTypeAnnotations.Schema)
{
return GetAnnotations(principalEntityType.FindPrimaryKey().Properties[pk.IndexOf(Property)]).ColumnName;
}
entityType = null;
}
}
else
{
StringBuilder builder = null;
do
else
{
var ownership = entityType.GetForeignKeys().SingleOrDefault(fk => fk.IsOwnership);
if (ownership == null)
var ownerType = ownership.PrincipalEntityType;
var entityTypeAnnotations = GetAnnotations(entityType);
var ownerTypeAnnotations = GetAnnotations(ownerType);
if (entityTypeAnnotations.TableName == ownerTypeAnnotations.TableName
&& entityTypeAnnotations.Schema == ownerTypeAnnotations.Schema)
{
entityType = null;
if (builder == null)
{
builder = new StringBuilder();
}
builder.Insert(0, "_");
builder.Insert(0, ownership.PrincipalToDependent.Name);
entityType = ownerType;
}
else
{
var ownerType = ownership.PrincipalEntityType;
var entityTypeAnnotations = GetAnnotations(entityType);
var ownerTypeAnnotations = GetAnnotations(ownerType);
if (entityTypeAnnotations.TableName == ownerTypeAnnotations.TableName
&& entityTypeAnnotations.Schema == ownerTypeAnnotations.Schema)
{
if (builder == null)
{
builder = new StringBuilder();
}
builder.Insert(0, "_");
builder.Insert(0, ownership.PrincipalToDependent.Name);
entityType = ownerType;
}
else
{
entityType = null;
}
entityType = null;
}
}
while (entityType != null);
}
while (entityType != null);

if (builder != null)
{
builder.Append(Property.Name);
return builder.ToString();
}
if (builder != null)
{
builder.Append(Property.Name);
return builder.ToString();
}

return Property.Name;
Expand Down
10 changes: 2 additions & 8 deletions src/EFCore.Relational/Metadata/RelationalPropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,8 @@ public static bool IsColumnNullable([NotNull] this IProperty property)
return false;
}

var pk = property.DeclaringEntityType.FindPrimaryKey();
return pk != null
&& property.DeclaringEntityType.FindForeignKeys(pk.Properties)
.Any(
fk => fk.PrincipalKey.IsPrimaryKey()
&& fk.PrincipalEntityType.BaseType != null
&& fk.DeclaringEntityType.Relational().TableName == fk.PrincipalEntityType.Relational().TableName
&& fk.DeclaringEntityType.Relational().Schema == fk.PrincipalEntityType.Relational().Schema);
return property.DeclaringEntityType.FindPrimaryKey()?.Properties.First()
?.FindSharedTableLink()?.PrincipalEntityType.BaseType != null;
}
}
}
38 changes: 34 additions & 4 deletions src/EFCore.SqlServer/Internal/SqlServerModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,40 @@ protected override void ValidateSharedColumnsCompatibility(IReadOnlyList<IEntity
{
base.ValidateSharedColumnsCompatibility(mappedTypes, tableName);

var identityColumns = mappedTypes.SelectMany(et => et.GetDeclaredProperties())
.Where(p => p.SqlServer().ValueGenerationStrategy == SqlServerValueGenerationStrategy.IdentityColumn)
.Distinct((p1, p2) => p1.Name == p2.Name)
.ToList();
var identityColumns = new List<IProperty>();
var propertyMappings = new Dictionary<string, IProperty>();

foreach (var property in mappedTypes.SelectMany(et => et.GetDeclaredProperties()))
{
var propertyAnnotations = property.Relational();
var columnName = propertyAnnotations.ColumnName;
if (propertyMappings.TryGetValue(columnName, out var duplicateProperty))
{
var propertyStrategy = property.SqlServer().ValueGenerationStrategy;
var duplicatePropertyStrategy = duplicateProperty.SqlServer().ValueGenerationStrategy;
if (propertyStrategy != duplicatePropertyStrategy
&& (propertyStrategy == SqlServerValueGenerationStrategy.IdentityColumn
|| duplicatePropertyStrategy == SqlServerValueGenerationStrategy.IdentityColumn))
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnNameValueGenerationStrategyMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
tableName));
}
}
else
{
propertyMappings[columnName] = property;
if (property.SqlServer().ValueGenerationStrategy == SqlServerValueGenerationStrategy.IdentityColumn)
{
identityColumns.Add(property);
}
}
}

if (identityColumns.Count > 1)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,18 @@ public virtual SqlServerValueGenerationStrategy? ValueGenerationStrategy

var relationalProperty = Property.Relational();
if (!fallbackToModel
|| Property.ValueGenerated != ValueGenerated.OnAdd
|| relationalProperty.DefaultValue != null
|| relationalProperty.DefaultValueSql != null
|| relationalProperty.ComputedColumnSql != null)
{
return null;
}

if (Property.ValueGenerated != ValueGenerated.OnAdd)
{
return Property.FindSharedTablePrincipalPrimaryKeyProperty()?.SqlServer().ValueGenerationStrategy;
}

var modelStrategy = Property.DeclaringEntityType.Model.SqlServer().ValueGenerationStrategy;

if (modelStrategy == SqlServerValueGenerationStrategy.SequenceHiLo
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.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.SqlServer/Properties/SqlServerStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -214,4 +214,7 @@
<value>Foreign key {fkName} is defined on table {tableName} which is not included in the selection set. Skipping.</value>
<comment>Debug SqlServerEventId.ForeignKeyTableMissingWarning string string</comment>
</data>
<data name="DuplicateColumnNameValueGenerationStrategyMismatch" xml:space="preserve">
<value>'{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}' but are configured with different value generation strategies.</value>
</data>
</root>
23 changes: 23 additions & 0 deletions src/EFCore/Internal/EnumerableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,5 +122,28 @@ public static bool StartsWith<TSource>(

return true;
}

/// <summary>
/// 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.
/// </summary>
public static int IndexOf<T>([NotNull] this IEnumerable<T> source, [NotNull] T item)
=> source.Select((x, index) =>
EqualityComparer<T>.Default.Equals(item, x) ? index : -1)
.FirstOr(x => x != -1, -1);

/// <summary>
/// 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.
/// </summary>
public static T FirstOr<T>([NotNull] this IEnumerable<T> source, [NotNull] T alternate)
=> source.DefaultIfEmpty(alternate).First();

/// <summary>
/// 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.
/// </summary>
public static T FirstOr<T>([NotNull] this IEnumerable<T> source, [NotNull] Func<T, bool> predicate, [NotNull] T alternate)
=> source.Where(predicate).FirstOr(alternate);
}
}
5 changes: 5 additions & 0 deletions test/EFCore.Relational.Tests/RelationalModelValidatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;

// ReSharper disable InconsistentNaming
namespace Microsoft.EntityFrameworkCore
{
public class RelationalModelValidatorTest : ModelValidatorTest
Expand Down Expand Up @@ -764,6 +765,8 @@ protected class Cat : Animal

[NotMapped]
public string Type { get; set; }

public int Identity { get; set; }
}

protected class Dog : Animal
Expand All @@ -772,6 +775,8 @@ protected class Dog : Animal

[NotMapped]
public int Type { get; set; }

public int Identity { get; set; }
}

protected class Person
Expand Down
35 changes: 35 additions & 0 deletions test/EFCore.SqlServer.Tests/Migrations/SqlServerModelDifferTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,41 @@ public void Alter_unique_constraint_clustering()
});
}

[Fact]
public void Create_shared_table_with_two_entity_types()
{
Execute(
_ => { },
modelBuilder =>
{
modelBuilder.Entity("Order", eb =>
{
eb.Property<int>("Id");
eb.ToTable("Orders");
});
modelBuilder.Entity("Details", eb =>
{
eb.Property<int>("Id");
eb.Property<DateTime>("Time");
eb.HasOne("Order").WithOne().HasForeignKey("Details", "Id");
eb.ToTable("Orders");
});
},
operations =>
{
Assert.Equal(1, operations.Count);

var createTableOperation = Assert.IsType<CreateTableOperation>(operations[0]);
Assert.Equal(2, createTableOperation.Columns.Count);
var idColumn = createTableOperation.Columns[0];
Assert.Equal("Id", idColumn.Name);
Assert.Equal(SqlServerValueGenerationStrategy.IdentityColumn, idColumn["SqlServer:ValueGenerationStrategy"]);
var timeColumn = createTableOperation.Columns[1];
Assert.Equal("Time", timeColumn.Name);
Assert.False(timeColumn.IsNullable);
});
}

[Fact]
public void Alter_index_clustering()
{
Expand Down
Loading

0 comments on commit 92dcb80

Please sign in to comment.