Skip to content

Commit

Permalink
Propagate original values for shared columns to added dependents in t…
Browse files Browse the repository at this point in the history 
…able splitting (#22059)

Use public types in OwnedNavigationBuilder constructor

Fixes #21465
  • Loading branch information
@AndriySvyryd @ajcvickers
AndriySvyryd authored and ajcvickers committed Aug 14, 2020
1 parent 9b11ad1 commit 022b0ff
Show file tree
Hide file tree
Showing 10 changed files with 147 additions and 91 deletions.
39 changes: 24 additions & 15 deletions src/EFCore.Relational/Update/ColumnModification.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,13 @@ public ColumnModification(
/// The parameter name to use for the current value parameter (<see cref="UseCurrentValueParameter" />), if needed.
/// </summary>
public virtual string ParameterName
=> _parameterName ?? (_parameterName = UseCurrentValueParameter ? _generateParameterName() : null);
=> _parameterName ??= UseCurrentValueParameter ? _generateParameterName() : null;

/// <summary>
/// The parameter name to use for the original value parameter (<see cref="UseOriginalValueParameter" />), if needed.
/// </summary>
public virtual string OriginalParameterName
=> _originalParameterName ?? (_originalParameterName = UseOriginalValueParameter ? _generateParameterName() : null);
=> _originalParameterName ??= UseOriginalValueParameter ? _generateParameterName() : null;

/// <summary>
/// The name of the column.
Expand Down Expand Up @@ -392,25 +392,34 @@ public virtual void AddSharedColumnModification([NotNull] ColumnModification mod
if (UseOriginalValueParameter
&& !StructuralComparisons.StructuralEqualityComparer.Equals(OriginalValue, modification.OriginalValue))
{
if (_sensitiveLoggingEnabled)
if (Entry.EntityState == EntityState.Modified
&& modification.Entry.EntityState == EntityState.Added
&& modification.Entry.SharedIdentityEntry == null)
{
modification.Entry.SetOriginalValue(modification.Property, OriginalValue);
}
else
{
if (_sensitiveLoggingEnabled)
{
throw new InvalidOperationException(
RelationalStrings.ConflictingOriginalRowValuesSensitive(
Entry.EntityType.DisplayName(),
modification.Entry.EntityType.DisplayName(),
Entry.BuildCurrentValuesString(Entry.EntityType.FindPrimaryKey().Properties),
Entry.BuildOriginalValuesString(new[] { Property }),
modification.Entry.BuildOriginalValuesString(new[] { modification.Property }),
"{'" + ColumnName + "'}"));
}

throw new InvalidOperationException(
RelationalStrings.ConflictingOriginalRowValuesSensitive(
RelationalStrings.ConflictingOriginalRowValues(
Entry.EntityType.DisplayName(),
modification.Entry.EntityType.DisplayName(),
Entry.BuildCurrentValuesString(Entry.EntityType.FindPrimaryKey().Properties),
Entry.BuildOriginalValuesString(new[] { Property }),
modification.Entry.BuildOriginalValuesString(new[] { modification.Property }),
new[] { Property }.Format(),
new[] { modification.Property }.Format(),
"{'" + ColumnName + "'}"));
}

throw new InvalidOperationException(
RelationalStrings.ConflictingOriginalRowValues(
Entry.EntityType.DisplayName(),
modification.Entry.EntityType.DisplayName(),
new[] { Property }.Format(),
new[] { modification.Property }.Format(),
"{'" + ColumnName + "'}"));
}

_sharedColumnModifications.Add(modification);
Expand Down
66 changes: 38 additions & 28 deletions src/EFCore.Relational/Update/ModificationCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Diagnostics;
using System.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
Expand Down Expand Up @@ -255,12 +254,12 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
var adding = state == EntityState.Added;
var updating = state == EntityState.Modified;
var columnModifications = new List<ColumnModification>();
Dictionary<string, ColumnValuePropagator> sharedColumnMap = null;
Dictionary<string, ColumnValuePropagator> sharedTableColumnMap = null;

if (_entries.Count > 1
|| (_entries.Count == 1 && _entries[0].SharedIdentityEntry != null))
{
sharedColumnMap = new Dictionary<string, ColumnValuePropagator>();
sharedTableColumnMap = new Dictionary<string, ColumnValuePropagator>();

if (_comparer != null)
{
Expand All @@ -269,12 +268,22 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()

foreach (var entry in _entries)
{
var tableMapping = GetTableMapping(entry.EntityType);
if (tableMapping == null)
{
continue;
}

if (entry.SharedIdentityEntry != null)
{
InitializeSharedColumns(entry.SharedIdentityEntry, updating, sharedColumnMap);
var sharedTableMapping = GetTableMapping(entry.SharedIdentityEntry.EntityType);
if (sharedTableMapping != null)
{
InitializeSharedColumns(entry.SharedIdentityEntry, sharedTableMapping, updating, sharedTableColumnMap);
}
}

InitializeSharedColumns(entry, updating, sharedColumnMap);
InitializeSharedColumns(entry, tableMapping, updating, sharedTableColumnMap);
}
}

Expand All @@ -284,18 +293,7 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
&& (entry.EntityState == EntityState.Deleted
|| entry.EntityState == EntityState.Added);

ITableMappingBase tableMapping = null;
foreach (var mapping in entry.EntityType.GetTableMappings())
{
var table = ((ITableMappingBase)mapping).Table;
if (table.Name == TableName
&& table.Schema == Schema)
{
tableMapping = mapping;
break;
}
}

var tableMapping = GetTableMapping(entry.EntityType);
if (tableMapping == null)
{
continue;
Expand All @@ -308,11 +306,9 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
var isKey = property.IsPrimaryKey();
var isCondition = !adding && (isKey || property.IsConcurrencyToken);
var readValue = state != EntityState.Deleted && entry.IsStoreGenerated(property);

ColumnValuePropagator columnPropagator = null;
if (sharedColumnMap != null)
{
columnPropagator = sharedColumnMap[column.Name];
}
sharedTableColumnMap?.TryGetValue(column.Name, out columnPropagator);

var writeValue = false;
if (!readValue)
Expand Down Expand Up @@ -350,7 +346,8 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
isCondition,
_sensitiveLoggingEnabled);

if (columnPropagator != null)
if (columnPropagator != null
&& column.PropertyMappings.Count() != 1)
{
if (columnPropagator.ColumnModification != null)
{
Expand All @@ -370,16 +367,29 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
return columnModifications;
}

private void InitializeSharedColumns(IUpdateEntry entry, bool updating, Dictionary<string, ColumnValuePropagator> columnMap)
private ITableMappingBase GetTableMapping(IEntityType entityType)
{
foreach (var property in entry.EntityType.GetProperties())
ITableMappingBase tableMapping = null;
foreach (var mapping in entityType.GetTableMappings())
{
var columnName = property.FindColumn(StoreObjectIdentifier.Table(TableName, Schema))?.Name;
if (columnName == null)
var table = ((ITableMappingBase)mapping).Table;
if (table.Name == TableName
&& table.Schema == Schema)
{
continue;
tableMapping = mapping;
break;
}
}

return tableMapping;
}

private void InitializeSharedColumns(
IUpdateEntry entry, ITableMappingBase tableMapping, bool updating, Dictionary<string, ColumnValuePropagator> columnMap)
{
foreach (var columnMapping in tableMapping.ColumnMappings)
{
var columnName = columnMapping.Column.Name;
if (!columnMap.TryGetValue(columnName, out var columnPropagator))
{
columnPropagator = new ColumnValuePropagator();
Expand All @@ -388,7 +398,7 @@ private void InitializeSharedColumns(IUpdateEntry entry, bool updating, Dictiona

if (updating)
{
columnPropagator.RecordValue(property, entry);
columnPropagator.RecordValue(columnMapping.Property, entry);
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/EFCore/ChangeTracking/Internal/OriginalValues.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public OriginalValues(InternalEntityEntry entry)
public object GetValue(InternalEntityEntry entry, IProperty property)
{
var index = property.GetOriginalValueIndex();

if (index == -1)
{
throw new InvalidOperationException(
Expand Down
10 changes: 2 additions & 8 deletions src/EFCore/Metadata/Builders/EntityTypeBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,10 +496,7 @@ private OwnedNavigationBuilder OwnsOneBuilder(in TypeIdentity ownedType, string
relationship.IsUnique(true, ConfigurationSource.Explicit);
}

return new OwnedNavigationBuilder(
Builder.Metadata,
relationship.Metadata.DeclaringEntityType,
relationship);
return new OwnedNavigationBuilder(relationship.Metadata);
}

/// <summary>
Expand Down Expand Up @@ -712,10 +709,7 @@ private OwnedNavigationBuilder OwnsManyBuilder(in TypeIdentity ownedType, string
relationship.IsUnique(false, ConfigurationSource.Explicit);
}

return new OwnedNavigationBuilder(
Builder.Metadata,
relationship.Metadata.DeclaringEntityType,
relationship);
return new OwnedNavigationBuilder(relationship.Metadata);
}

/// <summary>
Expand Down
10 changes: 2 additions & 8 deletions src/EFCore/Metadata/Builders/EntityTypeBuilder`.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,10 +698,7 @@ private OwnedNavigationBuilder<TEntity, TRelatedEntity> OwnsOneBuilder<TRelatedE
relationship = (InternalForeignKeyBuilder)batch.Run(relationship.Metadata).Builder;
}

return new OwnedNavigationBuilder<TEntity, TRelatedEntity>(
Builder.Metadata,
relationship.Metadata.DeclaringEntityType,
relationship);
return new OwnedNavigationBuilder<TEntity, TRelatedEntity>(relationship.Metadata);
}

/// <summary>
Expand Down Expand Up @@ -1080,10 +1077,7 @@ private OwnedNavigationBuilder<TEntity, TRelatedEntity> OwnsManyBuilder<TRelated
relationship = (InternalForeignKeyBuilder)batch.Run(relationship.Metadata).Builder;
}

return new OwnedNavigationBuilder<TEntity, TRelatedEntity>(
Builder.Metadata,
relationship.Metadata.DeclaringEntityType,
relationship);
return new OwnedNavigationBuilder<TEntity, TRelatedEntity>(relationship.Metadata);
}

/// <summary>
Expand Down
23 changes: 8 additions & 15 deletions src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,23 @@ public class OwnedNavigationBuilder : IInfrastructure<IConventionEntityTypeBuild
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
public OwnedNavigationBuilder(
[NotNull] EntityType principalEntityType,
[NotNull] EntityType dependentEntityType,
[NotNull] InternalForeignKeyBuilder builder)
public OwnedNavigationBuilder([NotNull] IMutableForeignKey ownership)
{
PrincipalEntityType = principalEntityType;
DependentEntityType = dependentEntityType;
_builder = builder;
PrincipalEntityType = (EntityType)ownership.PrincipalEntityType;
DependentEntityType = (EntityType)ownership.DeclaringEntityType;
_builder = ((ForeignKey)ownership).Builder;
}

/// <summary>
/// Gets the principal entity type used to configure this relationship.
/// </summary>
[EntityFrameworkInternal]
protected virtual EntityType PrincipalEntityType { get; }

/// <summary>
/// Gets the dependent entity type used to configure this relationship.
/// </summary>
[EntityFrameworkInternal]
protected virtual EntityType DependentEntityType { get; }

/// <summary>
Expand Down Expand Up @@ -527,10 +526,7 @@ private OwnedNavigationBuilder OwnsOneBuilder(in TypeIdentity ownedType, string
relationship.IsUnique(true, ConfigurationSource.Explicit);
}

return new OwnedNavigationBuilder(
DependentEntityType,
relationship.Metadata.DeclaringEntityType,
relationship);
return new OwnedNavigationBuilder(relationship.Metadata);
}

/// <summary>
Expand Down Expand Up @@ -752,10 +748,7 @@ private OwnedNavigationBuilder OwnsManyBuilder(in TypeIdentity ownedType, string
relationship.IsUnique(false, ConfigurationSource.Explicit);
}

return new OwnedNavigationBuilder(
DependentEntityType,
relationship.Metadata.DeclaringEntityType,
relationship);
return new OwnedNavigationBuilder(relationship.Metadata);
}

/// <summary>
Expand Down
17 changes: 4 additions & 13 deletions src/EFCore/Metadata/Builders/OwnedNavigationBuilder`.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,8 @@ public class OwnedNavigationBuilder<TEntity, TDependentEntity> : OwnedNavigation
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
public OwnedNavigationBuilder(
[NotNull] EntityType principalEntityType,
[NotNull] EntityType dependentEntityType,
[NotNull] InternalForeignKeyBuilder builder)
: base(principalEntityType, dependentEntityType, builder)
public OwnedNavigationBuilder([NotNull] IMutableForeignKey ownership)
: base(ownership)
{
}

Expand Down Expand Up @@ -638,10 +635,7 @@ private OwnedNavigationBuilder<TDependentEntity, TNewDependentEntity> OwnsOneBui
relationship = (InternalForeignKeyBuilder)batch.Run(relationship.Metadata).Builder;
}

return new OwnedNavigationBuilder<TDependentEntity, TNewDependentEntity>(
relationship.Metadata.PrincipalEntityType,
relationship.Metadata.DeclaringEntityType,
relationship);
return new OwnedNavigationBuilder<TDependentEntity, TNewDependentEntity>(relationship.Metadata);
}

/// <summary>
Expand Down Expand Up @@ -1033,10 +1027,7 @@ private OwnedNavigationBuilder<TDependentEntity, TNewRelatedEntity> OwnsManyBuil
relationship = (InternalForeignKeyBuilder)batch.Run(relationship.Metadata).Builder;
}

return new OwnedNavigationBuilder<TDependentEntity, TNewRelatedEntity>(
DependentEntityType,
relationship.Metadata.DeclaringEntityType,
relationship);
return new OwnedNavigationBuilder<TDependentEntity, TNewRelatedEntity>(relationship.Metadata);
}

/// <summary>
Expand Down
5 changes: 5 additions & 0 deletions test/EFCore.Relational.Specification.Tests/TPTTableSplittingTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ protected TPTTableSplittingTestBase(ITestOutputHelper testOutputHelper)
{
}

public override void Can_use_optional_dependents_with_shared_concurrency_tokens()
{
// TODO: Issue #22060
}

protected override string DatabaseName { get; } = "TPTTableSplittingTest";

protected override void OnModelCreating(ModelBuilder modelBuilder)
Expand Down
Loading

0 comments on commit 022b0ff

Please sign in to comment.