Skip to content

Commit

Permalink
Allow to set provider value comparer when configuring value conversion
Browse files Browse the repository at this point in the history
Use client value comparer as provider value comparer if possible
    The client value comparer will not be used if the model type is a nullable value type since we need the types to match exactly and the provider type is never nullable
Fix some API issues
Remove some unused code

Fixes #27738
Fixes #27850
Fixes #27791
Part of #27588
  • Loading branch information
AndriySvyryd authored May 5, 2022
1 parent 3f379e8 commit 3ea4803
Show file tree
Hide file tree
Showing 52 changed files with 1,334 additions and 369 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,15 @@ private void Create(
property.DeclaringEntityType.ShortName(), property.Name, nameof(PropertyBuilder.HasConversion)));
}

var providerValueComparerType = (Type?)property[CoreAnnotationNames.ProviderValueComparerType];
if (providerValueComparerType == null
&& property[CoreAnnotationNames.ProviderValueComparer] != null)
{
throw new InvalidOperationException(
DesignStrings.CompiledModelValueComparer(
property.DeclaringEntityType.ShortName(), property.Name, nameof(PropertyBuilder.HasConversion)));
}

var valueConverterType = (Type?)property[CoreAnnotationNames.ValueConverterType];
if (valueConverterType == null
&& property.GetValueConverter() != null)
Expand Down Expand Up @@ -809,6 +818,16 @@ private void Create(
.Append("()");
}

if (providerValueComparerType != null)
{
AddNamespace(providerValueComparerType, parameters.Namespaces);

mainBuilder.AppendLine(",")
.Append("providerValueComparer: new ")
.Append(_code.Reference(providerValueComparerType))
.Append("()");
}

mainBuilder
.AppendLine(");")
.DecrementIndent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1318,7 +1318,7 @@ protected override void ValidateInheritanceMapping(
var mappingStrategy = (string?)entityType[RelationalAnnotationNames.MappingStrategy];
if (mappingStrategy != null)
{
ValidateMappingStrategy(mappingStrategy, entityType);
ValidateMappingStrategy(entityType, mappingStrategy);
var storeObject = entityType.GetSchemaQualifiedTableName()
?? entityType.GetSchemaQualifiedViewName()
?? entityType.GetFunctionName();
Expand Down Expand Up @@ -1389,9 +1389,9 @@ protected override void ValidateInheritanceMapping(
/// <summary>
/// Validates that the given mapping strategy is supported
/// </summary>
/// <param name="mappingStrategy">The mapping strategy.</param>
/// <param name="entityType">The entity type.</param>
protected virtual void ValidateMappingStrategy(string? mappingStrategy, IEntityType entityType)
/// <param name="mappingStrategy">The mapping strategy.</param>
protected virtual void ValidateMappingStrategy(IEntityType entityType, string? mappingStrategy)
{
switch (mappingStrategy)
{
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore.Relational/Metadata/IColumn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,14 @@ public virtual string? Collation
=> PropertyMappings.First().Property
.GetCollation(StoreObjectIdentifier.Table(Table.Name, Table.Schema));

/// <summary>
/// Gets the <see cref="ValueComparer" /> for this column.
/// </summary>
/// <returns>The comparer.</returns>
public virtual ValueComparer ProviderValueComparer
=> PropertyMappings.First().Property
.GetProviderValueComparer();

/// <summary>
/// Returns the property mapping for the given entity type.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2062,7 +2062,7 @@ protected virtual void DiffData(

var sourceValue = sourceColumnModification.OriginalValue;
var targetValue = targetColumnModification.Value;
var comparer = targetMapping.TypeMapping.ProviderValueComparer;
var comparer = targetColumn.ProviderValueComparer;
if (sourceColumn.ProviderClrType == targetColumn.ProviderClrType
&& comparer.Equals(sourceValue, targetValue))
{
Expand Down
33 changes: 16 additions & 17 deletions src/EFCore.Relational/Storage/RelationalGeometryTypeMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ protected RelationalGeometryTypeMapping(
: base(CreateRelationalTypeMappingParameters(storeType))
{
SpatialConverter = converter;
SetProviderValueComparer();
}

/// <summary>
Expand All @@ -38,25 +37,19 @@ protected RelationalGeometryTypeMapping(
protected RelationalGeometryTypeMapping(
RelationalTypeMappingParameters parameters,
ValueConverter<TGeometry, TProvider>? converter)
: base(parameters)
: base(parameters.WithCoreParameters(parameters.CoreParameters with
{
ProviderValueComparer = parameters.CoreParameters.ProviderValueComparer
?? CreateProviderValueComparer(parameters.CoreParameters.Converter?.ProviderClrType ?? parameters.CoreParameters.ClrType)
}))
{
SpatialConverter = converter;
SetProviderValueComparer();
}

private void SetProviderValueComparer()
{
var providerType = Converter?.ProviderClrType ?? ClrType;
if (providerType.IsAssignableTo(typeof(TGeometry)))
{
ProviderValueComparer = (ValueComparer)Activator.CreateInstance(typeof(GeometryValueComparer<>).MakeGenericType(providerType))!;
}
}

/// <summary>
/// The underlying Geometry converter.
/// </summary>
protected virtual ValueConverter<TGeometry, TProvider>? SpatialConverter { get; }
private static ValueComparer? CreateProviderValueComparer(Type providerType)
=> providerType.IsAssignableTo(typeof(TGeometry))
? (ValueComparer)Activator.CreateInstance(typeof(GeometryValueComparer<>).MakeGenericType(providerType))!
: null;

private static RelationalTypeMappingParameters CreateRelationalTypeMappingParameters(string storeType)
{
Expand All @@ -67,10 +60,16 @@ private static RelationalTypeMappingParameters CreateRelationalTypeMappingParame
typeof(TGeometry),
null,
comparer,
comparer),
comparer,
CreateProviderValueComparer(typeof(TGeometry))),
storeType);
}

/// <summary>
/// The underlying Geometry converter.
/// </summary>
protected virtual ValueConverter<TGeometry, TProvider>? SpatialConverter { get; }

/// <summary>
/// Creates a <see cref="DbParameter" /> with the appropriate type information configured.
/// </summary>
Expand Down
33 changes: 18 additions & 15 deletions src/EFCore.Relational/Storage/RelationalTypeMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,24 @@ public RelationalTypeMappingParameters(
/// </summary>
public StoreTypePostfix StoreTypePostfix { get; }

/// <summary>
/// Creates a new <see cref="RelationalTypeMappingParameters" /> parameter object with the given
/// core parameters.
/// </summary>
/// <param name="coreParameters">Parameters for the <see cref="CoreTypeMapping" /> base class.</param>
/// <returns>The new parameter object.</returns>
public RelationalTypeMappingParameters WithCoreParameters(in CoreTypeMappingParameters coreParameters)
=> new(
coreParameters,
StoreType,
StoreTypePostfix,
DbType,
Unicode,
Size,
FixedLength,
Precision,
Scale);

/// <summary>
/// Creates a new <see cref="RelationalTypeMappingParameters" /> parameter object with the given
/// mapping info.
Expand Down Expand Up @@ -261,8 +279,6 @@ protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters p
=> this;
}

private ValueComparer? _providerValueComparer;

/// <summary>
/// Initializes a new instance of the <see cref="RelationalTypeMapping" /> class.
/// </summary>
Expand Down Expand Up @@ -380,19 +396,6 @@ public virtual bool IsFixedLength
protected virtual string SqlLiteralFormatString
=> "{0}";

/// <summary>
/// A <see cref="ValueComparer" /> for the provider CLR type values.
/// </summary>
public virtual ValueComparer ProviderValueComparer
{
get => NonCapturingLazyInitializer.EnsureInitialized(
ref _providerValueComparer,
this,
static c => ValueComparer.CreateDefault(c.Converter?.ProviderClrType ?? c.ClrType, favorStructuralComparisons: true));

protected set => _providerValueComparer = value;
}

/// <summary>
/// Creates a copy of this mapping.
/// </summary>
Expand Down
8 changes: 4 additions & 4 deletions src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ private static bool IsModified(IReadOnlyList<IColumn> columns, IReadOnlyModifica
var column = columns[columnIndex];
object? originalValue = null;
object? currentValue = null;
RelationalTypeMapping? typeMapping = null;
ValueComparer? providerValueComparer = null;
for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++)
{
var entry = command.Entries[entryIndex];
Expand Down Expand Up @@ -641,12 +641,12 @@ private static bool IsModified(IReadOnlyList<IColumn> columns, IReadOnlyModifica
break;
}

typeMapping = columnMapping!.TypeMapping;
providerValueComparer = property.GetProviderValueComparer();
}
}

if (typeMapping != null
&& !typeMapping.ProviderValueComparer.Equals(originalValue, currentValue))
if (providerValueComparer != null
&& !providerValueComparer.Equals(originalValue, currentValue))
{
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public virtual bool TryCreateDependentKeyValue(IReadOnlyModificationCommand comm
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
protected static IEqualityComparer<object?[]> CreateEqualityComparer(IReadOnlyList<IColumn> columns)
=> new CompositeCustomComparer(columns.Select(c => c.PropertyMappings.First().TypeMapping.ProviderValueComparer).ToList());
=> new CompositeCustomComparer(columns.Select(c => c.ProviderValueComparer).ToList());

private sealed class CompositeCustomComparer : IEqualityComparer<object?[]>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ public abstract bool TryCreateDependentKeyValue(
/// </summary>
protected virtual IEqualityComparer<TKey> CreateKeyEqualityComparer(IColumn column)
#pragma warning disable EF1001 // Internal EF Core API usage.
=> NullableComparerAdapter<TKey>.Wrap(
column.PropertyMappings.First().TypeMapping.ProviderValueComparer);
=> NullableComparerAdapter<TKey>.Wrap(column.ProviderValueComparer);
#pragma warning restore EF1001 // Internal EF Core API usage.

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public SimpleRowIndexValueFactory(ITableIndex index)
_column = index.Columns.Single();
_columnAccessors = ((Column)_column).Accessors;
#pragma warning disable EF1001 // Internal EF Core API usage.
EqualityComparer = NullableComparerAdapter<TKey>.Wrap(_column.PropertyMappings.First().TypeMapping.ProviderValueComparer);
EqualityComparer = NullableComparerAdapter<TKey>.Wrap(_column.ProviderValueComparer);
#pragma warning restore EF1001 // Internal EF Core API usage.
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public SimpleRowKeyValueFactory(IUniqueConstraint constraint)
_constraint = constraint;
_column = constraint.Columns.Single();
_columnAccessors = ((Column)_column).Accessors;
EqualityComparer = new NoNullsCustomEqualityComparer(_column.PropertyMappings.First().TypeMapping.ProviderValueComparer);
EqualityComparer = new NoNullsCustomEqualityComparer(_column.ProviderValueComparer);
}

/// <summary>
Expand Down Expand Up @@ -139,14 +139,6 @@ private sealed class NoNullsCustomEqualityComparer : IEqualityComparer<TKey>

public NoNullsCustomEqualityComparer(ValueComparer comparer)
{
if (comparer.Type != typeof(TKey)
&& comparer.Type == typeof(TKey).UnwrapNullableType())
{
#pragma warning disable EF1001 // Internal EF Core API usage.
comparer = comparer.ToNonNullNullableComparer();
#pragma warning restore EF1001 // Internal EF Core API usage.
}

_equals = (Func<TKey?, TKey?, bool>)comparer.EqualsExpression.Compile();
_hashCode = (Func<TKey, int>)comparer.HashCodeExpression.Compile();
}
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Update/ModificationCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ public void RecordValue(IColumnMapping mapping, IUpdateEntry entry)
break;
case EntityState.Added:
_currentValue = entry.GetCurrentProviderValue(property);
_write = !mapping.TypeMapping.ProviderValueComparer.Equals(_originalValue, _currentValue);
_write = !mapping.Column.ProviderValueComparer.Equals(_originalValue, _currentValue);

break;
case EntityState.Deleted:
Expand All @@ -513,7 +513,7 @@ public bool TryPropagate(IColumnMapping mapping, IUpdateEntry entry)
&& (entry.EntityState == EntityState.Unchanged
|| (entry.EntityState == EntityState.Modified && !entry.IsModified(property))
|| (entry.EntityState == EntityState.Added
&& mapping.TypeMapping.ProviderValueComparer.Equals(_originalValue, entry.GetCurrentValue(property)))))
&& mapping.Column.ProviderValueComparer.Equals(_originalValue, entry.GetCurrentValue(property)))))
{
if (property.GetAfterSaveBehavior() == PropertySaveBehavior.Save
|| entry.EntityState == EntityState.Added)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,6 @@ private sealed class NoNullsCustomEqualityComparer : IEqualityComparer<TKey>

public NoNullsCustomEqualityComparer(ValueComparer comparer)
{
if (comparer.Type != typeof(TKey)
&& comparer.Type == typeof(TKey).UnwrapNullableType())
{
comparer = comparer.ToNonNullNullableComparer();
}

_equals = (Func<TKey?, TKey?, bool>)comparer.EqualsExpression.Compile();
_hashCode = (Func<TKey, int>)comparer.HashCodeExpression.Compile();
}
Expand Down
62 changes: 0 additions & 62 deletions src/EFCore/ChangeTracking/Internal/ValueComparerExtensions.cs

This file was deleted.

18 changes: 18 additions & 0 deletions src/EFCore/Infrastructure/ModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,24 @@ protected virtual void ValidateTypeMappings(
{
_ = property.GetCurrentValueComparer(); // Will throw if there is no way to compare
}

var providerComparer = property.GetProviderValueComparer();
if (providerComparer == null)
{
continue;
}

var typeMapping = property.GetTypeMapping();
var actualProviderClrType = (typeMapping.Converter?.ProviderClrType ?? typeMapping.ClrType).UnwrapNullableType();

if (providerComparer.Type.UnwrapNullableType() != actualProviderClrType)
{
throw new InvalidOperationException(CoreStrings.ComparerPropertyMismatch(
providerComparer.Type.ShortDisplayName(),
property.DeclaringEntityType.DisplayName(),
property.Name,
actualProviderClrType.ShortDisplayName()));
}
}
}
}
Expand Down
Loading

0 comments on commit 3ea4803

Please sign in to comment.