Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collection change tracking for primitive collections #31480

Merged
merged 2 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,10 @@ protected CosmosTypeMapping(CoreTypeMappingParameters parameters)
/// </summary>
public override CoreTypeMapping Clone(
ValueConverter? converter,
ValueComparer? comparer = null,
CoreTypeMapping? elementMapping = null,
JsonValueReaderWriter? jsonValueReaderWriter = null)
=> new CosmosTypeMapping(Parameters.WithComposedConverter(converter, elementMapping, jsonValueReaderWriter));
=> new CosmosTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ private InMemoryTypeMapping(CoreTypeMappingParameters parameters)
/// </summary>
public override CoreTypeMapping Clone(
ValueConverter? converter,
ValueComparer? comparer = null,
CoreTypeMapping? elementMapping = null,
JsonValueReaderWriter? jsonValueReaderWriter = null)
=> new InMemoryTypeMapping(Parameters.WithComposedConverter(converter, elementMapping, jsonValueReaderWriter));
=> new InMemoryTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
7 changes: 5 additions & 2 deletions src/EFCore.Relational/Storage/RelationalTypeMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -230,15 +230,17 @@ public RelationalTypeMappingParameters WithScale(int? scale)
/// converter composed with any existing converter and set on the new parameter object.
/// </summary>
/// <param name="converter">The converter.</param>
/// <param name="comparer">The comparer.</param>
/// <param name="elementMapping">The element mapping, or <see langword="null" /> for non-collection mappings.</param>
/// <param name="jsonValueReaderWriter">The JSON reader/writer, or <see langword="null" /> to leave unchanged.</param>
/// <returns>The new parameter object.</returns>
public RelationalTypeMappingParameters WithComposedConverter(
ValueConverter? converter,
ValueComparer? comparer,
CoreTypeMapping? elementMapping,
JsonValueReaderWriter? jsonValueReaderWriter)
=> new(
CoreParameters.WithComposedConverter(converter, elementMapping, jsonValueReaderWriter),
CoreParameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter),
StoreType,
StoreTypePostfix,
DbType,
Expand Down Expand Up @@ -428,9 +430,10 @@ public virtual RelationalTypeMapping Clone(int? precision, int? scale)
/// <inheritdoc />
public override CoreTypeMapping Clone(
ValueConverter? converter,
ValueComparer? comparer = null,
CoreTypeMapping? elementMapping = null,
JsonValueReaderWriter? jsonValueReaderWriter = null)
=> Clone(Parameters.WithComposedConverter(converter, elementMapping, jsonValueReaderWriter));
=> Clone(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));

/// <summary>
/// Clones the type mapping to update facets from the mapping info, if needed.
Expand Down
15 changes: 11 additions & 4 deletions src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -224,20 +224,27 @@ protected override CoreTypeMapping FindMapping(in TypeMappingInfo mappingInfo)
Type modelType,
Type? providerType,
CoreTypeMapping? elementMapping)
=> TryFindJsonCollectionMapping(
{
var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!;

return TryFindJsonCollectionMapping(
info.CoreTypeMappingInfo, modelType, providerType, ref elementMapping, out var collectionReaderWriter)
? (RelationalTypeMapping)FindMapping(
info.WithConverter(
// Note that the converter info is only used temporarily here and never creates an instance.
new ValueConverterInfo(modelType, typeof(string), _ => null!)))!
.Clone(
(ValueConverter)Activator.CreateInstance(
typeof(CollectionToJsonStringConverter<>).MakeGenericType(
modelType.TryGetElementType(typeof(IEnumerable<>))!),
collectionReaderWriter!)!,
typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!,
(ValueComparer?)Activator.CreateInstance(
elementType.IsNullableValueType()
? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType())
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
elementMapping!.Comparer),
elementMapping,
collectionReaderWriter)
: null;
}

/// <summary>
/// Finds the type mapping for a given <see cref="IProperty" />.
Expand Down
144 changes: 144 additions & 0 deletions src/EFCore/ChangeTracking/ListComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.EntityFrameworkCore.ChangeTracking;

/// <summary>
/// A <see cref="ValueComparer{T}"/> for lists of primitive items. The list can be typed as <see cref="IEnumerable{T}"/>,
/// but can only be used with instances that implement <see cref="IList{T}"/>.
/// </summary>
/// <remarks>
/// <para>
/// This comparer should be used for reference types and non-nullable value types. Use
/// <see cref="NullableValueTypeListComparer{TElement}"/> for nullable value types.
/// </para>
/// <para>
/// See <see href="https://aka.ms/efcore-docs-value-comparers">EF Core value comparers</see> for more information and examples.
/// </para>
/// </remarks>
/// <typeparam name="TElement">The element type.</typeparam>
public sealed class ListComparer<TElement> : ValueComparer<IEnumerable<TElement>>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this replace the equivalent one in Cosmos?

{
/// <summary>
/// Creates a new instance of the list comparer.
/// </summary>
/// <param name="elementComparer">The comparer to use for comparing elements.</param>
public ListComparer(ValueComparer<TElement> elementComparer)
: base(
(a, b) => Compare(a, b, elementComparer),
o => GetHashCode(o, elementComparer),
source => Snapshot(source, elementComparer))
{
}

private static bool Compare(IEnumerable<TElement>? a, IEnumerable<TElement>? b, ValueComparer<TElement> elementComparer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be compatible with compiled models these methods need to be public.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, they shouldn't use captured variables. Instead, in-line the element comparer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave this for RC2. I can see it fails with compiled models, but I can't see the path forward where we can get the element comparer without capturing it in some way. Will file an issue. We can discuss.

System.NotSupportedException
Encountered a constant of unsupported type '<>c__DisplayClass0_0'. Only primitive constant nodes are supported.
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.<VisitConstant>g__GenerateValue|36_0(Object value, <>c__DisplayClass36_0&) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 949
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitConstant(ConstantExpression constant) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 878
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Visit(Expression node) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 204
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Translate[T](Expression node) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 158
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitMember(MemberExpression member) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 1350
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Visit(Expression node) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 204
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Translate[T](Expression node) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 158
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateList(IReadOnlyList`1 list) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 2175
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateMethodArguments(ParameterInfo[] parameters, IReadOnlyList`1 arguments) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 2135
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitMethodCall(MethodCallExpression call) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 1393
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Visit(Expression node) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 204
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Translate(Expression node) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 145
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitLambda[T](Expression`1 lambda) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 1228
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Visit(Expression node) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 204
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateCore(Expression node, ISet`1 collectedNamespaces, Boolean statementContext) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 117
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateExpression(Expression node, ISet`1 collectedNamespaces) in C:\github\efcore\src\EFCore.Design\Query\Internal\LinqToCSharpSyntaxTranslator.cs:line 101
   at Microsoft.EntityFrameworkCore.Design.Internal.CSharpHelper.Expression(Expression node, ISet`1 collectedNamespaces) in C:\github\efcore\src\EFCore.Design\Design\Internal\CSharpHelper.cs:line 1545
   at Microsoft.EntityFrameworkCore.Design.Internal.CSharpRuntimeAnnotationCodeGenerator.Create(ValueComparer comparer, CSharpRuntimeAnnotationCodeGeneratorParameters parameters, ICSharpHelper codeHelper) in C:\github\efcore\src\EFCore\Design\Internal\CSharpRuntimeAnnotationCodeGenerator.cs:line 394
   at Microsoft.EntityFrameworkCore.Design.Internal.RelationalCSharpRuntimeAnnotationCodeGenerator.Create(CoreTypeMapping typeMapping, CSharpRuntimeAnnotationCodeGeneratorParameters parameters, ValueComparer valueComparer, ValueComparer keyValueComparer, ValueComparer providerValueComparer) in C:\github\efcore\src\EFCore.Relational\Design\Internal\RelationalCSharpRuntimeAnnotationCodeGenerator.cs:line 2128
   at Microsoft.EntityFrameworkCore.Design.Internal.ICSharpRuntimeAnnotationCodeGenerator.Create(CoreTypeMapping typeMapping, IProperty property, CSharpRuntimeAnnotationCodeGeneratorParameters parameters) in C:\github\efcore\src\EFCore\Design\Internal\ICSharpRuntimeAnnotationCodeGenerator.cs:line 117
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.Create(IProperty property, String variableName, Dictionary`2 propertyVariables, CSharpRuntimeAnnotationCodeGeneratorParameters parameters) in C:\github\efcore\src\EFCore.Design\Scaffolding\Internal\CSharpRuntimeModelCodeGenerator.cs:line 861
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.Create(IProperty property, Dictionary`2 propertyVariables, CSharpRuntimeAnnotationCodeGeneratorParameters parameters) in C:\github\efcore\src\EFCore.Design\Scaffolding\Internal\CSharpRuntimeModelCodeGenerator.cs:line 690
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.CreateEntityType(IEntityType entityType, IndentedStringBuilder mainBuilder, IndentedStringBuilder methodBuilder, SortedSet`1 namespaces, String className, Boolean nullable) in C:\github\efcore\src\EFCore.Design\Scaffolding\Internal\CSharpRuntimeModelCodeGenerator.cs:line 540
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.GenerateEntityType(IEntityType entityType, String namespace, String className, Boolean nullable) in C:\github\efcore\src\EFCore.Design\Scaffolding\Internal\CSharpRuntimeModelCodeGenerator.cs:line 462
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.GenerateModel(IModel model, CompiledModelCodeGenerationOptions options) in C:\github\efcore\src\EFCore.Design\Scaffolding\Internal\CSharpRuntimeModelCodeGenerator.cs:line 75
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGeneratorTest.Test(DbContext context, CompiledModelCodeGenerationOptions options, Action`1 assertScaffold, Action`1 assertModel, Type additionalDesignTimeServices, Action`1 useContext, String expectedExceptionMessage) in C:\github\efcore\test\EFCore.Design.Tests\Scaffolding\Internal\CSharpRuntimeModelCodeGeneratorTest.cs:line 12338
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGeneratorTest.BigModel_with_JSON_columns() in C:\github\efcore\test\EFCore.Design.Tests\Scaffolding\Internal\CSharpRuntimeModelCodeGeneratorTest.cs:line 4439
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

{
if (ReferenceEquals(a, b))
{
return true;
}

if (a is null)
{
return b is null;
}

if (b is null)
{
return false;
}

if (a is IList<TElement> aList && b is IList<TElement> bList)
{
if (aList.Count != bList.Count)
{
return false;
}

for (var i = 0; i < aList.Count; i++)
{
var (el1, el2) = (aList[i], bList[i]);
if (el1 is null)
{
if (el2 is null)
{
continue;
}

return false;
}

if (el2 is null)
{
return false;
}

if (!elementComparer.Equals(el1, el2))
{
return false;
}
}

return true;
}

throw new InvalidOperationException(
CoreStrings.BadListType(
(a is IList<TElement?> ? b : a).GetType().ShortDisplayName(),
typeof(ListComparer<TElement?>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName()));
}

private static int GetHashCode(IEnumerable<TElement> source, ValueComparer<TElement> elementComparer)
{
var hash = new HashCode();

foreach (var el in source)
{
hash.Add(el == null ? 0 : elementComparer.GetHashCode(el));
}

return hash.ToHashCode();
}

private static IList<TElement> Snapshot(IEnumerable<TElement> source, ValueComparer<TElement> elementComparer)
{
if (!(source is IList<TElement> sourceList))
{
throw new InvalidOperationException(
CoreStrings.BadListType(
source.GetType().ShortDisplayName(),
typeof(ListComparer<TElement?>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName()));
}

if (sourceList.IsReadOnly)
{
var snapshot = new TElement[sourceList.Count];

for (var i = 0; i < sourceList.Count; i++)
{
var instance = sourceList[i];
if (instance != null)
{
snapshot[i] = elementComparer.Snapshot(instance);
}
}

return snapshot;
}
else
{
var snapshot = (source is List<TElement> || sourceList.IsReadOnly)
? new List<TElement>(sourceList.Count)
: (IList<TElement>)Activator.CreateInstance(source.GetType())!;

foreach (var e in sourceList)
{
snapshot.Add(e == null ? (TElement)(object)null! : elementComparer.Snapshot(e));
}

return snapshot;
}
}
}
142 changes: 142 additions & 0 deletions src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.EntityFrameworkCore.ChangeTracking;

/// <summary>
/// A <see cref="ValueComparer{T}"/> for lists of primitive items. The list can be typed as <see cref="IEnumerable{T}"/>,
/// but can only be used with instances that implement <see cref="IList{T}"/>.
/// </summary>
/// <remarks>
/// <para>
/// This comparer should be used for nullable value types. Use <see cref="NullableValueTypeListComparer{TElement}"/> for reference
/// types and non-nullable value types.
/// </para>
/// <para>
/// See <see href="https://aka.ms/efcore-docs-value-comparers">EF Core value comparers</see> for more information and examples.
/// </para>
/// </remarks>
/// <typeparam name="TElement">The element type.</typeparam>
public sealed class NullableValueTypeListComparer<TElement> : ValueComparer<IEnumerable<TElement?>>
where TElement : struct
{
/// <summary>
/// Creates a new instance of the list comparer.
/// </summary>
/// <param name="elementComparer">The comparer to use for comparing elements.</param>
public NullableValueTypeListComparer(ValueComparer<TElement> elementComparer)
: base(
(a, b) => Compare(a, b, elementComparer),
o => GetHashCode(o, elementComparer),
source => Snapshot(source, elementComparer))
{
}

private static bool Compare(IEnumerable<TElement?>? a, IEnumerable<TElement?>? b, ValueComparer<TElement> elementComparer)
{
if (ReferenceEquals(a, b))
{
return true;
}

if (a is null)
{
return b is null;
}

if (b is null)
{
return false;
}

if (a is IList<TElement?> aList && b is IList<TElement?> bList)
{
if (aList.Count != bList.Count)
{
return false;
}

for (var i = 0; i < aList.Count; i++)
{
var (el1, el2) = (aList[i], bList[i]);
if (el1 is null)
{
if (el2 is null)
{
continue;
}

return false;
}

if (el2 is null)
{
return false;
}

if (!elementComparer.Equals(el1, el2))
{
return false;
}
}

return true;
}

throw new InvalidOperationException(
CoreStrings.BadListType(
(a is IList<TElement?> ? b : a).GetType().ShortDisplayName(),
typeof(NullableValueTypeListComparer<TElement>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type.MakeNullable()).ShortDisplayName()));
}

private static int GetHashCode(IEnumerable<TElement?> source, ValueComparer<TElement> elementComparer)
{
var hash = new HashCode();

foreach (var el in source)
{
hash.Add(el == null ? 0 : elementComparer.GetHashCode(el));
}

return hash.ToHashCode();
}

private static IList<TElement?> Snapshot(IEnumerable<TElement?> source, ValueComparer<TElement> elementComparer)
{
if (!(source is IList<TElement?> sourceList))
{
throw new InvalidOperationException(
CoreStrings.BadListType(
source.GetType().ShortDisplayName(),
typeof(NullableValueTypeListComparer<TElement>).ShortDisplayName(),
typeof(IList<>).MakeGenericType(elementComparer.Type.MakeNullable()).ShortDisplayName()));
}

if (sourceList.IsReadOnly)
{
var snapshot = new TElement?[sourceList.Count];

for (var i = 0; i < sourceList.Count; i++)
{
var instance = sourceList[i];
snapshot[i] = instance == null ? null : (TElement?)elementComparer.Snapshot(instance);
}

return snapshot;
}
else
{
var snapshot = source is List<TElement?> || sourceList.IsReadOnly
? new List<TElement?>(sourceList.Count)
: (IList<TElement?>)Activator.CreateInstance(source.GetType())!;

foreach (var e in sourceList)
{
snapshot.Add(e == null ? null : (TElement?)elementComparer.Snapshot(e));
}

return snapshot;
}
}
}
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/Property.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@ public virtual CoreTypeMapping? TypeMapping
/// </summary>
public virtual string? CheckValueComparer(ValueComparer? comparer)
=> comparer != null
&& comparer.Type.UnwrapNullableType() != ClrType.UnwrapNullableType()
&& !comparer.Type.UnwrapNullableType().IsAssignableFrom(ClrType.UnwrapNullableType())
? CoreStrings.ComparerPropertyMismatch(
comparer.Type.ShortDisplayName(),
DeclaringType.DisplayName(),
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading