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

Use public types in OwnedNavigationBuilder constructor

Fixes #21465
  • Loading branch information
AndriySvyryd committed Aug 14, 2020
1 parent 89a30d4 commit b37aa3c
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 91 deletions.
39 changes: 24 additions & 15 deletions src/EFCore.Relational/Update/ColumnModification.cs
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
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
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
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
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
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
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
Loading

0 comments on commit b37aa3c

Please sign in to comment.