Skip to content

Commit

Permalink
Updated per review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@lajones
lajones committed Apr 27, 2020
1 parent 6f59e2f commit c87f9ab
Show file tree
Hide file tree
Showing 4 changed files with 216 additions and 135 deletions.
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Metadata/Conventions/Infrastructure/RelationalConventionSetBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public override ConventionSet CreateConventionSet()
// to the generated concurrency token property
ConventionSet.AddBefore(
conventionSet.ModelFinalizingConventions,
new AddConcurrencyTokenPropertiesConvention(Dependencies, RelationalDependencies),
new TableSharingConcurrencyTokenConvention(Dependencies, RelationalDependencies),
typeof(TypeMappingConvention));
// ModelCleanupConvention would remove the entity types added by QueryableDbFunctionConvention #15898
ConventionSet.AddAfter(
Expand Down
103 changes: 56 additions & 47 deletions ...ddConcurrencyTokenPropertiesConvention.cs → ...TableSharingConcurrencyTokenConvention.cs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
// 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 System;
using System.Collections.Generic;
using System.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Internal;

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
{
Expand All @@ -17,14 +18,14 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
/// creates a shadow concurrency property mapped to that column
/// on the base-most entity type(s).
/// </summary>
public class AddConcurrencyTokenPropertiesConvention : IModelFinalizingConvention
public class TableSharingConcurrencyTokenConvention : IModelFinalizingConvention
{
/// <summary>
/// Creates a new instance of <see cref="AddConcurrencyTokenPropertiesConvention" />.
/// Creates a new instance of <see cref="TableSharingConcurrencyTokenConvention" />.
/// </summary>
/// <param name="dependencies"> Parameter object containing dependencies for this convention. </param>
/// <param name="relationalDependencies"> Parameter object containing relational dependencies for this convention. </param>
public AddConcurrencyTokenPropertiesConvention(
/// <param name="relationalDependencies"> Parameter object containing relational dependencies for this convention. </param>
public TableSharingConcurrencyTokenConvention(
[NotNull] ProviderConventionSetBuilderDependencies dependencies,
[NotNull] RelationalConventionSetBuilderDependencies relationalDependencies)
{
Expand All @@ -36,35 +37,33 @@ public AddConcurrencyTokenPropertiesConvention(
/// </summary>
protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; }

public static readonly string ConcurrencyPropertyPrefix = "_concurrency_";
public static readonly string ConcurrencyPropertyPrefix = "_TableSharingConcurrencyTokenConvention_";

/// <inheritdoc />
public virtual void ProcessModelFinalizing(
IConventionModelBuilder modelBuilder,
IConventionContext<IConventionModelBuilder> context)
{
var maxIdentifierLength = modelBuilder.Metadata.GetMaxIdentifierLength();

GetMappings(modelBuilder.Metadata,
out var tableToEntityTypes, out var concurrencyColumnsToProperties);

foreach (var table in tableToEntityTypes)
foreach (var tableToEntityType in tableToEntityTypes)
{
var tableName = table.Key;
if (!concurrencyColumnsToProperties.TryGetValue(tableName, out var concurrencyColumns))
var table = tableToEntityType.Key;
if (!concurrencyColumnsToProperties.TryGetValue(table, out var concurrencyColumns))
{
continue; // this table has no mapped concurrency columns
}

var entityTypesMappedToTable = table.Value;
var entityTypesMappedToTable = tableToEntityType.Value;

foreach (var concurrencyColumn in concurrencyColumns)
{
var concurrencyColumnName = concurrencyColumn.Key;
var propertiesMappedToConcurrencyColumn = concurrencyColumn.Value;

var entityTypesMissingConcurrencyColumn =
new Dictionary<IConventionEntityType, IProperty>();
new Dictionary<IConventionEntityType, IConventionProperty>();
foreach (var entityType in entityTypesMappedToTable)
{
var foundMappedProperty = false;
Expand Down Expand Up @@ -95,45 +94,47 @@ public virtual void ProcessModelFinalizing(
}
}

foreach(var entityTypeToExampleProperty in
BasestEntities(entityTypesMissingConcurrencyColumn))
RemoveDerivedEntityTypes(ref entityTypesMissingConcurrencyColumn);

foreach(var entityTypeToExampleProperty in entityTypesMissingConcurrencyColumn)
{
var entityType = entityTypeToExampleProperty.Key;
var exampleProperty = entityTypeToExampleProperty.Value;
var allExistingProperties =
entityType.GetProperties().Select(p => p.Name)
.Union(entityType.GetNavigations().Select(p => p.Name))
.ToDictionary(s => s, s => 0);
var concurrencyShadowPropertyName =
Uniquifier.Uniquify(ConcurrencyPropertyPrefix + concurrencyColumnName, allExistingProperties, maxIdentifierLength);
var concurrencyShadowProperty =
entityType.AddProperty(concurrencyShadowPropertyName, exampleProperty.ClrType, null);
concurrencyShadowProperty.SetColumnName(concurrencyColumnName);
concurrencyShadowProperty.SetIsConcurrencyToken(true);
concurrencyShadowProperty.SetValueGenerated(exampleProperty.ValueGenerated);
var concurrencyShadowPropertyBuilder =
#pragma warning disable EF1001 // Internal EF Core API usage.
((InternalEntityTypeBuilder)entityType.Builder).CreateUniqueProperty(
ConcurrencyPropertyPrefix + exampleProperty.Name,
exampleProperty.ClrType,
!exampleProperty.IsNullable);
concurrencyShadowPropertyBuilder.SetColumnName(concurrencyColumnName);
_ = concurrencyShadowPropertyBuilder.SetIsConcurrencyToken(true, ConfigurationSource.Convention);
_ = concurrencyShadowPropertyBuilder.SetValueGenerated(exampleProperty.ValueGenerated, ConfigurationSource.Convention);
#pragma warning restore EF1001 // Internal EF Core API usage.
}
}
}
}

private void GetMappings(IConventionModel model,
out Dictionary<string, IList<IConventionEntityType>> tableToEntityTypes,
out Dictionary<string, Dictionary<string, IList<IProperty>>> concurrencyColumnsToProperties)
out Dictionary<Tuple<string, string>, IList<IConventionEntityType>> tableToEntityTypes,
out Dictionary<Tuple<string, string>, Dictionary<string, IList<IConventionProperty>>> concurrencyColumnsToProperties)
{
tableToEntityTypes = new Dictionary<string, IList<IConventionEntityType>>();
concurrencyColumnsToProperties = new Dictionary<string, Dictionary<string, IList<IProperty>>>();
tableToEntityTypes = new Dictionary<Tuple<string, string>, IList<IConventionEntityType>>();
concurrencyColumnsToProperties = new Dictionary<Tuple<string, string>, Dictionary<string, IList<IConventionProperty>>>();
foreach (var entityType in model.GetEntityTypes())
{
var tableName = entityType.GetSchemaQualifiedTableName();
var tableName = entityType.GetTableName();
if (tableName == null)
{
continue; // unmapped entityType
}

if (!tableToEntityTypes.TryGetValue(tableName, out var mappedTypes))
var table = Tuple.Create(tableName, entityType.GetSchema());

if (!tableToEntityTypes.TryGetValue(table, out var mappedTypes))
{
mappedTypes = new List<IConventionEntityType>();
tableToEntityTypes[tableName] = mappedTypes;
tableToEntityTypes[table] = mappedTypes;
}

mappedTypes.Add(entityType);
Expand All @@ -143,16 +144,16 @@ private void GetMappings(IConventionModel model,
if (property.IsConcurrencyToken
&& (property.ValueGenerated & ValueGenerated.OnUpdate) != 0)
{
if (!concurrencyColumnsToProperties.TryGetValue(tableName, out var columnToProperties))
if (!concurrencyColumnsToProperties.TryGetValue(table, out var columnToProperties))
{
columnToProperties = new Dictionary<string, IList<IProperty>>();
concurrencyColumnsToProperties[tableName] = columnToProperties;
columnToProperties = new Dictionary<string, IList<IConventionProperty>>();
concurrencyColumnsToProperties[table] = columnToProperties;
}

var columnName = property.GetColumnName();
if (!columnToProperties.TryGetValue(columnName, out var properties))
{
properties = new List<IProperty>();
properties = new List<IConventionProperty>();
columnToProperties[columnName] = properties;
}

Expand All @@ -162,19 +163,24 @@ private void GetMappings(IConventionModel model,
}
}

// Given a (distinct) IEnumerable of EntityTypes (mapped to T),
// return only the mappings where the EntityType does not inherit
// from any other EntityType in the list.
private static IEnumerable<KeyValuePair<IConventionEntityType, T>> BasestEntities<T>(
IEnumerable<KeyValuePair<IConventionEntityType, T>> entityTypeDictionary)
// Given a Dictionary of EntityTypes (mapped to T), remove
// any mappings where the EntityType inherits from any
// other EntityType in the Dictionary.
private static void RemoveDerivedEntityTypes<T>(
ref Dictionary<IConventionEntityType, T> entityTypeDictionary)
{
var toRemove = new HashSet<KeyValuePair<IConventionEntityType, T>>();
foreach (var entityType in entityTypeDictionary)
var entityTypesWithDerivedTypes = entityTypeDictionary.Where(e => e.Key.GetDirectlyDerivedTypes().Any());
foreach (var entityType in entityTypeDictionary.Where(e => e.Key.BaseType != null))
{
var otherEntityTypes = entityTypeDictionary
.Except(new[] { entityType }).Except(toRemove);
foreach (var otherEntityType in otherEntityTypes)
foreach (var otherEntityType in entityTypesWithDerivedTypes)
{
if (toRemove.Contains(otherEntityType)
|| otherEntityType.Equals(entityType))
{
continue;
}

if (otherEntityType.Key.IsAssignableFrom(entityType.Key))
{
toRemove.Add(entityType);
Expand All @@ -183,7 +189,10 @@ private static IEnumerable<KeyValuePair<IConventionEntityType, T>> BasestEntitie
}
}

return entityTypeDictionary.Except(toRemove);
foreach (var entityType in toRemove)
{
entityTypeDictionary.Remove(entityType.Key);
}
}
}
}
87 changes: 0 additions & 87 deletions test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -961,82 +961,6 @@ public virtual void Passes_for_compatible_duplicate_index_names_within_hierarchy
Assert.Equal(index1.GetName(), index2.GetName());
}

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

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

var animal = model.FindEntityType(typeof(Animal));
var concurrencyProperty = animal.FindProperty("_concurrency_Version");
Assert.True(concurrencyProperty.IsConcurrencyToken);
Assert.True(concurrencyProperty.IsShadowProperty());
Assert.Equal("Version", concurrencyProperty.GetColumnName());
Assert.Equal(ValueGenerated.OnAddOrUpdate, concurrencyProperty.ValueGenerated);
}

[ConditionalFact]
public virtual void Missing_concurrency_token_properties_are_created_on_the_base_most_types()
{
var modelBuilder = CreateConventionalModelBuilder();
modelBuilder.Entity<Person>().ToTable(nameof(Animal)).Property<byte[]>("Version")
.HasColumnName("Version").ValueGeneratedOnUpdate().IsConcurrencyToken(true);
modelBuilder.Entity<Animal>().HasOne(a => a.FavoritePerson).WithOne().HasForeignKey<Person>(p => p.Id);
modelBuilder.Entity<Animal>().HasOne(a => a.Dwelling).WithOne().HasForeignKey<AnimalHouse>(p => p.Id);
modelBuilder.Entity<Cat>();
modelBuilder.Entity<AnimalHouse>().ToTable(nameof(Animal));
modelBuilder.Entity<TheMovie>();

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

var animal = model.FindEntityType(typeof(Animal));
var concurrencyProperty = animal.FindProperty("_concurrency_Version");
Assert.True(concurrencyProperty.IsConcurrencyToken);
Assert.True(concurrencyProperty.IsShadowProperty());
Assert.Equal("Version", concurrencyProperty.GetColumnName());
Assert.Equal(ValueGenerated.OnUpdate, concurrencyProperty.ValueGenerated);

var cat = model.FindEntityType(typeof(Cat));
Assert.DoesNotContain(cat.GetDeclaredProperties(), p => p.Name == "_concurrency_Version");

var animalHouse = model.FindEntityType(typeof(AnimalHouse));
concurrencyProperty = animalHouse.FindProperty("_concurrency_Version");
Assert.True(concurrencyProperty.IsConcurrencyToken);
Assert.True(concurrencyProperty.IsShadowProperty());
Assert.Equal("Version", concurrencyProperty.GetColumnName());
Assert.Equal(ValueGenerated.OnUpdate, concurrencyProperty.ValueGenerated);

var theMovie = model.FindEntityType(typeof(TheMovie));
Assert.DoesNotContain(theMovie.GetDeclaredProperties(), p => p.Name == "_concurrency_Version");
}

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

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

var personEntityType = model.FindEntityType(typeof(Person));
var concurrencyProperty = personEntityType.FindProperty("_concurrency_Version");
Assert.True(concurrencyProperty.IsConcurrencyToken);
Assert.True(concurrencyProperty.IsShadowProperty());
Assert.Equal("Version", concurrencyProperty.GetColumnName());
Assert.Equal(ValueGenerated.OnAddOrUpdate, concurrencyProperty.ValueGenerated);
}

[ConditionalFact]
public virtual void Passes_for_correctly_mapped_concurrency_tokens_with_table_sharing()
{
Expand Down Expand Up @@ -1164,7 +1088,6 @@ protected class Animal
public string Name { get; set; }

public Person FavoritePerson { get; set; }
public AnimalHouse Dwelling { get; set; }
}

protected class Cat : Animal
Expand All @@ -1187,16 +1110,6 @@ protected class Dog : Animal
public int Identity { get; set; }
}

protected class AnimalHouse
{
public int Id { get; set; }
}

protected class TheMovie : AnimalHouse
{
public bool CanHaveAnother { get; set; }
}

protected class Person
{
public int Id { get; set; }
Expand Down
Loading

0 comments on commit c87f9ab

Please sign in to comment.