From c7f960d69f6c25d2186259d617ba84f168a8e487 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Wed, 16 Aug 2023 13:06:05 +0100 Subject: [PATCH 1/2] Collection change tracking for primitive collections Part of #25364 --- .../Storage/Internal/CosmosTypeMapping.cs | 3 +- .../Storage/Internal/InMemoryTypeMapping.cs | 3 +- .../Storage/RelationalTypeMapping.cs | 7 +- .../Storage/RelationalTypeMappingSource.cs | 13 +- src/EFCore/ChangeTracking/ListComparer.cs | 144 ++++ .../NullableValueTypeListComparer.cs | 142 ++++ src/EFCore/Metadata/Internal/Property.cs | 2 +- src/EFCore/Properties/CoreStrings.Designer.cs | 8 + src/EFCore/Properties/CoreStrings.resx | 3 + src/EFCore/Storage/CoreTypeMapping.cs | 6 +- src/EFCore/Storage/TypeMappingSource.cs | 13 +- .../Update/JsonUpdateTestBase.cs | 36 - .../Internal/ChangeDetectorTest.cs | 3 +- test/EFCore.Tests/CollectionComparerTest.cs | 680 ++++++++++++++++++ 14 files changed, 1016 insertions(+), 47 deletions(-) create mode 100644 src/EFCore/ChangeTracking/ListComparer.cs create mode 100644 src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs create mode 100644 test/EFCore.Tests/CollectionComparerTest.cs diff --git a/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs b/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs index 0f613a1d912..cb62bd9a9e5 100644 --- a/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs +++ b/src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs @@ -62,9 +62,10 @@ protected CosmosTypeMapping(CoreTypeMappingParameters parameters) /// 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)); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs b/src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs index 43ca2771b32..bfd8126ac82 100644 --- a/src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs +++ b/src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs @@ -55,9 +55,10 @@ private InMemoryTypeMapping(CoreTypeMappingParameters parameters) /// 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)); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs index d04f9d2d288..1f12134f7b3 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs @@ -230,15 +230,17 @@ public RelationalTypeMappingParameters WithScale(int? scale) /// converter composed with any existing converter and set on the new parameter object. /// /// The converter. + /// The comparer. /// The element mapping, or for non-collection mappings. /// The JSON reader/writer, or to leave unchanged. /// The new parameter object. 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, @@ -428,9 +430,10 @@ public virtual RelationalTypeMapping Clone(int? precision, int? scale) /// 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)); /// /// Clones the type mapping to update facets from the mapping info, if needed. diff --git a/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs b/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs index 681060684f2..70753c99635 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs @@ -224,7 +224,10 @@ 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( @@ -233,11 +236,17 @@ protected override CoreTypeMapping FindMapping(in TypeMappingInfo mappingInfo) .Clone( (ValueConverter)Activator.CreateInstance( typeof(CollectionToJsonStringConverter<>).MakeGenericType( - modelType.TryGetElementType(typeof(IEnumerable<>))!), + elementType), collectionReaderWriter!)!, + (ValueComparer?)Activator.CreateInstance( + elementType.IsNullableValueType() + ? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType()) + : typeof(ListComparer<>).MakeGenericType(elementType), + elementMapping?.Comparer), elementMapping, collectionReaderWriter) : null; + } /// /// Finds the type mapping for a given . diff --git a/src/EFCore/ChangeTracking/ListComparer.cs b/src/EFCore/ChangeTracking/ListComparer.cs new file mode 100644 index 00000000000..7982bc392a6 --- /dev/null +++ b/src/EFCore/ChangeTracking/ListComparer.cs @@ -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; + +/// +/// A for lists of primitive items. The list can be typed as , +/// but can only be used with instances that implement . +/// +/// +/// +/// This comparer should be used for reference types and non-nullable value types. Use +/// for nullable value types. +/// +/// +/// See EF Core value comparers for more information and examples. +/// +/// +/// The element type. +public sealed class ListComparer : ValueComparer> +{ + /// + /// Creates a new instance of the list comparer. + /// + /// The comparer to use for comparing elements. + public ListComparer(ValueComparer elementComparer) + : base( + (a, b) => Compare(a, b, elementComparer), + o => GetHashCode(o, elementComparer), + source => Snapshot(source, elementComparer)) + { + } + + private static bool Compare(IEnumerable? a, IEnumerable? b, ValueComparer 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 aList && b is IList 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 ? b : a).GetType().ShortDisplayName(), + typeof(ListComparer).ShortDisplayName(), + typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName())); + } + + private static int GetHashCode(IEnumerable source, ValueComparer elementComparer) + { + var hash = new HashCode(); + + foreach (var el in source) + { + hash.Add(el == null ? 0 : elementComparer.GetHashCode(el)); + } + + return hash.ToHashCode(); + } + + private static IList Snapshot(IEnumerable source, ValueComparer elementComparer) + { + if (!(source is IList sourceList)) + { + throw new InvalidOperationException( + CoreStrings.BadListType( + source.GetType().ShortDisplayName(), + typeof(ListComparer).ShortDisplayName(), + typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName())); + } + + if (sourceList.GetType().IsArray) + { + 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 + ? new List(sourceList.Count) + : (IList)Activator.CreateInstance(source.GetType())!; + + foreach (var e in sourceList) + { + snapshot.Add(e == null ? (TElement)(object)null! : elementComparer.Snapshot(e)); + } + + return snapshot; + } + } +} diff --git a/src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs b/src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs new file mode 100644 index 00000000000..657660a93b7 --- /dev/null +++ b/src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs @@ -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; + +/// +/// A for lists of primitive items. The list can be typed as , +/// but can only be used with instances that implement . +/// +/// +/// +/// This comparer should be used for nullable value types. Use for reference +/// types and non-nullable value types. +/// +/// +/// See EF Core value comparers for more information and examples. +/// +/// +/// The element type. +public sealed class NullableValueTypeListComparer : ValueComparer> + where TElement : struct +{ + /// + /// Creates a new instance of the list comparer. + /// + /// The comparer to use for comparing elements. + public NullableValueTypeListComparer(ValueComparer elementComparer) + : base( + (a, b) => Compare(a, b, elementComparer), + o => GetHashCode(o, elementComparer), + source => Snapshot(source, elementComparer)) + { + } + + private static bool Compare(IEnumerable? a, IEnumerable? b, ValueComparer 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 aList && b is IList 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 ? b : a).GetType().ShortDisplayName(), + typeof(NullableValueTypeListComparer).ShortDisplayName(), + typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName())); + } + + private static int GetHashCode(IEnumerable source, ValueComparer elementComparer) + { + var hash = new HashCode(); + + foreach (var el in source) + { + hash.Add(el == null ? 0 : elementComparer.GetHashCode(el)); + } + + return hash.ToHashCode(); + } + + private static IList Snapshot(IEnumerable source, ValueComparer elementComparer) + { + if (!(source is IList sourceList)) + { + throw new InvalidOperationException( + CoreStrings.BadListType( + source.GetType().ShortDisplayName(), + typeof(NullableValueTypeListComparer).ShortDisplayName(), + typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName())); + } + + if (sourceList.GetType().IsArray) + { + 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 + ? new List(sourceList.Count) + : (IList)Activator.CreateInstance(source.GetType())!; + + foreach (var e in sourceList) + { + snapshot.Add(e == null ? null : (TElement?)elementComparer.Snapshot(e)); + } + + return snapshot; + } + } +} diff --git a/src/EFCore/Metadata/Internal/Property.cs b/src/EFCore/Metadata/Internal/Property.cs index 2d79b84fe61..c7bd85acb51 100644 --- a/src/EFCore/Metadata/Internal/Property.cs +++ b/src/EFCore/Metadata/Internal/Property.cs @@ -1151,7 +1151,7 @@ public virtual CoreTypeMapping? TypeMapping /// 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(), diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 563afdfe7df..ffe136f9bac 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -202,6 +202,14 @@ public static string BadJsonValueReaderWriterType(object? givenType) GetString("BadJsonValueReaderWriterType", nameof(givenType)), givenType); + /// + /// The type '{givenType}' cannot be used with '{comparerType}' because it does not implement '{listType}'. Collections of primitive types must be ordered lists. + /// + public static string BadListType(object? givenType, object? comparerType, object? listType) + => string.Format( + GetString("BadListType", nameof(givenType), nameof(comparerType), nameof(listType)), + givenType, comparerType, listType); + /// /// The type '{givenType}' cannot be used as a value comparer because it does not inherit from '{expectedType}'. Make sure to inherit value comparers from '{expectedType}'. /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index b9cf6b9e15f..c56b0700f52 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -183,6 +183,9 @@ The type '{givenType}' cannot be used as a 'JsonValueReaderWriter' because it does not inherit from the generic 'JsonValueReaderWriter<TValue>'. Make sure to inherit json reader/writers from 'JsonValueReaderWriter<TValue>'. + + The type '{givenType}' cannot be used with '{comparerType}' because it does not implement '{listType}'. Collections of primitive types must be ordered lists. + The type '{givenType}' cannot be used as a value comparer because it does not inherit from '{expectedType}'. Make sure to inherit value comparers from '{expectedType}'. diff --git a/src/EFCore/Storage/CoreTypeMapping.cs b/src/EFCore/Storage/CoreTypeMapping.cs index 85d32f3c09d..f84fa623bab 100644 --- a/src/EFCore/Storage/CoreTypeMapping.cs +++ b/src/EFCore/Storage/CoreTypeMapping.cs @@ -110,11 +110,13 @@ public CoreTypeMappingParameters( /// converter composed with any existing converter and set on the new parameter object. /// /// The converter. + /// The comparer. /// The element mapping, or for non-collection mappings. /// The JSON reader/writer, or to leave unchanged. /// The new parameter object. public CoreTypeMappingParameters WithComposedConverter( ValueConverter? converter, + ValueComparer? comparer, CoreTypeMapping? elementMapping, JsonValueReaderWriter? jsonValueReaderWriter) { @@ -123,7 +125,7 @@ public CoreTypeMappingParameters WithComposedConverter( return new CoreTypeMappingParameters( ClrType, converter ?? Converter, - Comparer, + comparer ?? Comparer, KeyComparer, ProviderValueComparer, ValueGeneratorFactory, @@ -263,11 +265,13 @@ public virtual ValueComparer ProviderValueComparer /// added. /// /// The converter to use. + /// The comparer to use, or for to keep the default. /// The element mapping, or for non-collection mappings. /// The JSON reader/writer, or to leave unchanged. /// A new type mapping public abstract CoreTypeMapping Clone( ValueConverter? converter, + ValueComparer? comparer = null, CoreTypeMapping? elementMapping = null, JsonValueReaderWriter? jsonValueReaderWriter = null); diff --git a/src/EFCore/Storage/TypeMappingSource.cs b/src/EFCore/Storage/TypeMappingSource.cs index cd06b5e562e..73411238962 100644 --- a/src/EFCore/Storage/TypeMappingSource.cs +++ b/src/EFCore/Storage/TypeMappingSource.cs @@ -180,7 +180,10 @@ protected TypeMappingSource(TypeMappingSourceDependencies dependencies) Type modelType, Type? providerType, CoreTypeMapping? elementMapping) - => TryFindJsonCollectionMapping( + { + var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!; + + return TryFindJsonCollectionMapping( info, modelType, providerType, ref elementMapping, out var collectionReaderWriter) ? FindMapping( info.WithConverter( @@ -189,11 +192,17 @@ protected TypeMappingSource(TypeMappingSourceDependencies dependencies) .Clone( (ValueConverter)Activator.CreateInstance( typeof(CollectionToJsonStringConverter<>).MakeGenericType( - modelType.TryGetElementType(typeof(IEnumerable<>))!), + elementType!), collectionReaderWriter!)!, + (ValueComparer?)Activator.CreateInstance( + elementType.IsNullableValueType() + ? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType()) + : typeof(ListComparer<>).MakeGenericType(elementType), + elementMapping?.Comparer), elementMapping, collectionReaderWriter) : null; + } /// /// Finds the type mapping for a given . diff --git a/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs index be85bf18a79..abbf5190f4e 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs @@ -1703,8 +1703,6 @@ public virtual Task Edit_single_property_collection_of_char() '\\' }; entity.Collection[0].TestCharacterCollection.Add((char)0); - // TODO: This should be change-detected. - context.Entry(entity.Collection[0]).Property(e => e.TestCharacterCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -1730,9 +1728,6 @@ public virtual Task Edit_single_property_collection_of_datetime() var entity = query.Single(x => x.Id == 1); entity.Reference.TestDateTimeCollection.Add(DateTime.Parse("01/01/3000 12:34:56")); entity.Collection[0].TestDateTimeCollection.Add(DateTime.Parse("01/01/3000 12:34:56")); - // TODO: This should be change-detected. - context.Entry(entity.Reference).Property(e => e.TestDateTimeCollection).IsModified = true; - context.Entry(entity.Collection[0]).Property(e => e.TestDateTimeCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -1830,9 +1825,6 @@ public virtual Task Edit_single_property_collection_of_double() var entity = query.Single(x => x.Id == 1); entity.Reference.TestDoubleCollection.Add(-1.23579); entity.Collection[0].TestDoubleCollection.Add(-1.23579); - // TODO: This should be change-detected. - context.Entry(entity.Reference).Property(e => e.TestDoubleCollection).IsModified = true; - context.Entry(entity.Collection[0]).Property(e => e.TestDoubleCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -1933,9 +1925,6 @@ public virtual Task Edit_single_property_collection_of_int64() var entity = query.Single(x => x.Id == 1); entity.Reference.TestInt64Collection.Clear(); entity.Collection[0].TestInt64Collection.Clear(); - // TODO: This should be change-detected. - context.Entry(entity.Reference).Property(e => e.TestInt64Collection).IsModified = true; - context.Entry(entity.Collection[0]).Property(e => e.TestInt64Collection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -1986,9 +1975,6 @@ public virtual Task Edit_single_property_collection_of_single() var entity = query.Single(x => x.Id == 1); entity.Reference.TestSingleCollection.RemoveAt(0); entity.Collection[0].TestSingleCollection.RemoveAt(1); - // TODO: This should be change-detected. - context.Entry(entity.Reference).Property(e => e.TestSingleCollection).IsModified = true; - context.Entry(entity.Collection[0]).Property(e => e.TestSingleCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2014,9 +2000,6 @@ public virtual Task Edit_single_property_collection_of_timespan() var entity = query.Single(x => x.Id == 1); entity.Reference.TestTimeSpanCollection[0] = new TimeSpan(0, 10, 1, 1, 7); entity.Collection[0].TestTimeSpanCollection[1] = new TimeSpan(0, 10, 1, 1, 7); - // TODO: This should be change-detected. - context.Entry(entity.Reference).Property(e => e.TestTimeSpanCollection).IsModified = true; - context.Entry(entity.Collection[0]).Property(e => e.TestTimeSpanCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2120,8 +2103,6 @@ public virtual Task Edit_single_property_collection_of_nullable_int32() entity.Reference.TestNullableInt32Collection.Add(77); entity.Reference.TestNullableInt32Collection.Add(null); entity.Collection[0].TestNullableInt32Collection = new ObservableCollection { null, 77 }; - // TODO: This should be change-detected. - context.Entry(entity.Reference).Property(e => e.TestNullableInt32Collection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2275,9 +2256,6 @@ public virtual Task Edit_single_property_collection_of_nullable_enum_with_int_co entity.Reference.TestNullableEnumWithIntConverterCollection.RemoveAt(1); entity.Collection[0].TestNullableEnumWithIntConverterCollection.Add(JsonEnum.Two); entity.Collection[0].TestNullableEnumWithIntConverterCollection.RemoveAt(2); - // TODO: This should be change-detected. - context.Entry(entity.Reference).Property(e => e.TestNullableEnumWithIntConverterCollection).IsModified = true; - context.Entry(entity.Collection[0]).Property(e => e.TestNullableEnumWithIntConverterCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2458,8 +2436,6 @@ public virtual Task Edit_single_property_relational_collection_of_datetime() var query = await context.JsonEntitiesAllTypes.ToListAsync(); var entity = query.Single(x => x.Id == 1); entity.TestDateTimeCollection.Add(DateTime.Parse("01/01/3000 12:34:56")); - // TODO: This should be change-detected. - context.Entry(entity).Property(e => e.TestDateTimeCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2537,8 +2513,6 @@ public virtual Task Edit_single_property_relational_collection_of_double() var query = await context.JsonEntitiesAllTypes.ToListAsync(); var entity = query.Single(x => x.Id == 1); entity.TestDoubleCollection.Add(-1.23579); - // TODO: This should be change-detected. - context.Entry(entity).Property(e => e.TestDoubleCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2627,8 +2601,6 @@ public virtual Task Edit_single_property_relational_collection_of_int64() var query = await context.JsonEntitiesAllTypes.ToListAsync(); var entity = query.Single(x => x.Id == 1); entity.TestInt64Collection.Clear(); - // TODO: This should be change-detected. - context.Entry(entity).Property(e => e.TestInt64Collection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2673,8 +2645,6 @@ public virtual Task Edit_single_property_relational_collection_of_single() var query = await context.JsonEntitiesAllTypes.ToListAsync(); var entity = query.Single(x => x.Id == 1); entity.TestSingleCollection.RemoveAt(0); - // TODO: This should be change-detected. - context.Entry(entity).Property(e => e.TestSingleCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2697,8 +2667,6 @@ public virtual Task Edit_single_property_relational_collection_of_timespan() var query = await context.JsonEntitiesAllTypes.ToListAsync(); var entity = query.Single(x => x.Id == 1); entity.TestTimeSpanCollection[0] = new TimeSpan(0, 10, 1, 1, 7); - // TODO: This should be change-detected. - context.Entry(entity).Property(e => e.TestTimeSpanCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2789,8 +2757,6 @@ public virtual Task Edit_single_property_relational_collection_of_nullable_int32 var entity = query.Single(x => x.Id == 1); entity.TestNullableInt32Collection.Add(77); entity.TestNullableInt32Collection.Add(null); - // TODO: This should be change-detected. - context.Entry(entity).Property(e => e.TestNullableInt32Collection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); @@ -2925,8 +2891,6 @@ public virtual Task Edit_single_property_relational_collection_of_nullable_enum_ var entity = query.Single(x => x.Id == 1); entity.TestNullableEnumWithIntConverterCollection.Add(JsonEnum.Two); entity.TestNullableEnumWithIntConverterCollection.RemoveAt(1); - // TODO: This should be change-detected. - context.Entry(entity).Property(e => e.TestNullableEnumWithIntConverterCollection).IsModified = true; ClearLog(); await context.SaveChangesAsync(); diff --git a/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs index 8e144020806..7b9c501aee6 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs @@ -313,9 +313,10 @@ public ConcreteTypeMapping(Type clrType, ValueConverter converter, ValueComparer public override CoreTypeMapping Clone( ValueConverter converter, + ValueComparer comparer = null, CoreTypeMapping elementMapping = null, JsonValueReaderWriter jsonValueReaderWriter = null) - => new ConcreteTypeMapping(Parameters.WithComposedConverter(converter, elementMapping, jsonValueReaderWriter)); + => new ConcreteTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter)); protected override CoreTypeMapping Clone(CoreTypeMappingParameters parameters) => new ConcreteTypeMapping(parameters); diff --git a/test/EFCore.Tests/CollectionComparerTest.cs b/test/EFCore.Tests/CollectionComparerTest.cs new file mode 100644 index 00000000000..871110d10fa --- /dev/null +++ b/test/EFCore.Tests/CollectionComparerTest.cs @@ -0,0 +1,680 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +using System.Collections.ObjectModel; + +namespace Microsoft.EntityFrameworkCore; + +public class CollectionComparerTest +{ + [ConditionalFact] + public void Can_detect_changes_to_primitive_collections_using_arrays() + { + using var context = new SomeLists(); + + var entity = new Voidbringer + { + Id = 1, + + ArrayInt = new[] { 0, 1, 2 }, + ArrayNullableInt = new int?[] { 0, null, 2 }, + ArrayString = new[] { "0", "1", "2" }, + ArrayNullableString = new[] { "0", null, "2" }, + ArrayStruct = new MyStruct[] { new("0"), new("1"), new("2") }, + ArrayNullableStruct = new MyStruct?[] { new("0"), null, new("2") }, + ArrayClass = new MyClass[] { new("0"), new("1"), new("2") }, + ArrayNullableClass = new MyClass?[] { new("0"), null, new("2") }, + + EnumerableInt = new[] { 0, 1, 2 }, + EnumerableNullableInt = new int?[] { 0, null, 2 }, + EnumerableString = new[] { "0", "1", "2" }, + EnumerableNullableString = new[] { "0", null, "2" }, + EnumerableStruct = new MyStruct[] { new("0"), new("1"), new("2") }, + EnumerableNullableStruct = new MyStruct?[] { new("0"), null, new("2") }, + EnumerableClass = new MyClass[] { new("0"), new("1"), new("2") }, + EnumerableNullableClass = new MyClass?[] { new("0"), null, new("2") }, + + IListInt = new[] { 0, 1, 2 }, + IListNullableInt = new int?[] { 0, null, 2 }, + IListString = new[] { "0", "1", "2" }, + IListNullableString = new[] { "0", null, "2" }, + IListStruct = new MyStruct[] { new("0"), new("1"), new("2") }, + IListNullableStruct = new MyStruct?[] { new("0"), null, new("2") }, + IListClass = new MyClass[] { new("0"), new("1"), new("2") }, + IListNullableClass = new MyClass?[] { new("0"), null, new("2") }, + }; + + var entry = context.Add(entity); + + context.SaveChanges(); + + entity.ArrayInt[0] = 10; + entity.ArrayNullableInt[1] = 11; + entity.ArrayString[2] = "12"; + entity.ArrayNullableString[0] = null; + entity.ArrayStruct[1] = new MyStruct("14"); + entity.ArrayNullableStruct[2] = new MyStruct("15"); + entity.ArrayClass[0] = new MyClass("16"); + entity.ArrayNullableClass[2] = null; + + ((IList)entity.EnumerableInt)[2] = 20; + (((IList)entity.EnumerableNullableInt))[0] = null; + ((IList)entity.EnumerableString)[1] = "22"; + ((IList)entity.EnumerableNullableString)[2] = "23"; + ((IList)entity.EnumerableStruct)[0] = new MyStruct("24"); + ((IList)entity.EnumerableNullableStruct)[0] = null; + ((IList)entity.EnumerableClass)[2] = new MyClass("26"); + ((IList)entity.EnumerableNullableClass)[0] = new MyClass("27"); + + entity.IListInt[1] = 30; + entity.IListNullableInt[2] = 31; + entity.IListString[0] = "32"; + entity.IListNullableString[2] = null; + entity.IListStruct[2] = new MyStruct("34"); + entity.IListNullableStruct[0] = new MyStruct("35"); + entity.IListClass[1] = new MyClass("36"); + entity.IListNullableClass[2] = null; + + context.ChangeTracker.DetectChanges(); + + Assert.True(entry.Property(e => e.ArrayInt).IsModified); + Assert.True(entry.Property(e => e.ArrayNullableInt).IsModified); + Assert.True(entry.Property(e => e.ArrayString).IsModified); + Assert.True(entry.Property(e => e.ArrayNullableString).IsModified); + Assert.True(entry.Property(e => e.ArrayStruct).IsModified); + Assert.True(entry.Property(e => e.ArrayNullableStruct).IsModified); + Assert.True(entry.Property(e => e.ArrayClass).IsModified); + Assert.True(entry.Property(e => e.ArrayNullableClass).IsModified); + + Assert.True(entry.Property(e => e.EnumerableInt).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableInt).IsModified); + Assert.True(entry.Property(e => e.EnumerableString).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableString).IsModified); + Assert.True(entry.Property(e => e.EnumerableStruct).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableStruct).IsModified); + Assert.True(entry.Property(e => e.EnumerableClass).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableClass).IsModified); + + Assert.True(entry.Property(e => e.IListInt).IsModified); + Assert.True(entry.Property(e => e.IListNullableInt).IsModified); + Assert.True(entry.Property(e => e.IListString).IsModified); + Assert.True(entry.Property(e => e.IListNullableString).IsModified); + Assert.True(entry.Property(e => e.IListStruct).IsModified); + Assert.True(entry.Property(e => e.IListNullableStruct).IsModified); + Assert.True(entry.Property(e => e.IListClass).IsModified); + Assert.True(entry.Property(e => e.IListNullableClass).IsModified); + + context.SaveChanges(); + + Assert.Equal(EntityState.Unchanged, entry.State); + } + + [ConditionalFact] + public void Can_detect_changes_to_primitive_collections_using_List() + { + using var context = new SomeLists(); + + var entity = new Voidbringer + { + Id = 2, + + EnumerableInt = new List { 0, 1, 2 }, + EnumerableNullableInt = new List { 0, null, 2 }, + EnumerableString = new List { "0", "1", "2" }, + EnumerableNullableString = new List { "0", null, "2" }, + EnumerableStruct = new List { new("0"), new("1"), new("2") }, + EnumerableNullableStruct = new List { new("0"), null, new("2") }, + EnumerableClass = new List { new("0"), new("1"), new("2") }, + EnumerableNullableClass = new List { new("0"), null, new("2") }, + + IListInt = new List { 0, 1, 2 }, + IListNullableInt = new List { 0, null, 2 }, + IListString = new List { "0", "1", "2" }, + IListNullableString = new List { "0", null, "2" }, + IListStruct = new List { new("0"), new("1"), new("2") }, + IListNullableStruct = new List { new("0"), null, new("2") }, + IListClass = new List { new("0"), new("1"), new("2") }, + IListNullableClass = new List { new("0"), null, new("2") }, + + ListInt = new() { 0, 1, 2 }, + ListNullableInt = new List { 0, null, 2 }, + ListString = new() { "0", "1", "2" }, + ListNullableString = new() { "0", null, "2" }, + ListStruct = new List { new("0"), new("1"), new("2") }, + ListNullableStruct = new List { new("0"), null, new("2") }, + ListClass = new List { new("0"), new("1"), new("2") }, + ListNullableClass = new List { new("0"), null, new("2") }, + + ICollectionInt = new List { 0, 1, 2 }, + ICollectionNullableInt = new List { 0, null, 2 }, + ICollectionString = new List { "0", "1", "2" }, + ICollectionNullableString = new List { "0", null, "2" }, + ICollectionStruct = new List { new("0"), new("1"), new("2") }, + ICollectionNullableStruct = new List { new("0"), null, new("2") }, + ICollectionClass = new List { new("0"), new("1"), new("2") }, + ICollectionNullableClass = new List { new("0"), null, new("2") }, + }; + + var entry = context.Add(entity); + + context.SaveChanges(); + + ((IList)entity.EnumerableInt)[0] = 20; + (((IList)entity.EnumerableNullableInt))[0] = null; + ((IList)entity.EnumerableString)[2] = "22"; + ((IList)entity.EnumerableNullableString)[0] = "23"; + ((IList)entity.EnumerableStruct)[1] = new MyStruct("24"); + ((IList)entity.EnumerableNullableStruct)[2] = null; + ((IList)entity.EnumerableClass)[0] = new MyClass("26"); + ((IList)entity.EnumerableNullableClass)[1] = new MyClass("27"); + + entity.IListInt[0] = 30; + entity.IListNullableInt[1] = 31; + entity.IListString[2] = "32"; + entity.IListNullableString[0] = null; + entity.IListStruct[1] = new MyStruct("34"); + entity.IListNullableStruct[2] = new MyStruct("35"); + entity.IListClass[0] = new MyClass("36"); + entity.IListNullableClass[2] = null; + + entity.ListInt[2] = 40; + entity.ListNullableInt[0] = null; + entity.ListString[1] = "42"; + entity.ListNullableString[2] = "43"; + entity.ListStruct[0] = new MyStruct("44"); + entity.ListNullableStruct[0] = null; + entity.ListClass[2] = new MyClass("46"); + entity.ListNullableClass[0] = new MyClass("47"); + + ((IList)entity.ICollectionInt)[1] = 50; + (((IList)entity.ICollectionNullableInt))[2] = 51; + ((IList)entity.ICollectionString)[0] = "52"; + ((IList)entity.ICollectionNullableString)[2] = null; + ((IList)entity.ICollectionStruct)[2] = new MyStruct("54"); + ((IList)entity.ICollectionNullableStruct)[0] = new MyStruct("55"); + ((IList)entity.ICollectionClass)[1] = new MyClass("56"); + ((IList)entity.ICollectionNullableClass)[2] = null; + + context.ChangeTracker.DetectChanges(); + + Assert.True(entry.Property(e => e.EnumerableInt).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableInt).IsModified); + Assert.True(entry.Property(e => e.EnumerableString).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableString).IsModified); + Assert.True(entry.Property(e => e.EnumerableStruct).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableStruct).IsModified); + Assert.True(entry.Property(e => e.EnumerableClass).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableClass).IsModified); + + Assert.True(entry.Property(e => e.IListInt).IsModified); + Assert.True(entry.Property(e => e.IListNullableInt).IsModified); + Assert.True(entry.Property(e => e.IListString).IsModified); + Assert.True(entry.Property(e => e.IListNullableString).IsModified); + Assert.True(entry.Property(e => e.IListStruct).IsModified); + Assert.True(entry.Property(e => e.IListNullableStruct).IsModified); + Assert.True(entry.Property(e => e.IListClass).IsModified); + Assert.True(entry.Property(e => e.IListNullableClass).IsModified); + + Assert.True(entry.Property(e => e.ListInt).IsModified); + Assert.True(entry.Property(e => e.ListNullableInt).IsModified); + Assert.True(entry.Property(e => e.ListString).IsModified); + Assert.True(entry.Property(e => e.ListNullableString).IsModified); + Assert.True(entry.Property(e => e.ListStruct).IsModified); + Assert.True(entry.Property(e => e.ListNullableStruct).IsModified); + Assert.True(entry.Property(e => e.ListClass).IsModified); + Assert.True(entry.Property(e => e.ListNullableClass).IsModified); + + Assert.True(entry.Property(e => e.ICollectionInt).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableInt).IsModified); + Assert.True(entry.Property(e => e.ICollectionString).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableString).IsModified); + Assert.True(entry.Property(e => e.ICollectionStruct).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableStruct).IsModified); + Assert.True(entry.Property(e => e.ICollectionClass).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableClass).IsModified); + + context.SaveChanges(); + + Assert.Equal(EntityState.Unchanged, entry.State); + } + + [ConditionalFact] + public void Can_detect_changes_to_primitive_collections_using_Collection() + { + using var context = new SomeLists(); + + var entity = new Voidbringer + { + Id = 3, + + EnumerableInt = new Collection { 0, 1, 2 }, + EnumerableNullableInt = new Collection { 0, null, 2 }, + EnumerableString = new Collection { "0", "1", "2" }, + EnumerableNullableString = new Collection { "0", null, "2" }, + EnumerableStruct = new Collection { new("0"), new("1"), new("2") }, + EnumerableNullableStruct = new Collection { new("0"), null, new("2") }, + EnumerableClass = new Collection { new("0"), new("1"), new("2") }, + EnumerableNullableClass = new Collection { new("0"), null, new("2") }, + + IListInt = new Collection { 0, 1, 2 }, + IListNullableInt = new Collection { 0, null, 2 }, + IListString = new Collection { "0", "1", "2" }, + IListNullableString = new Collection { "0", null, "2" }, + IListStruct = new Collection { new("0"), new("1"), new("2") }, + IListNullableStruct = new Collection { new("0"), null, new("2") }, + IListClass = new Collection { new("0"), new("1"), new("2") }, + IListNullableClass = new Collection { new("0"), null, new("2") }, + + ICollectionInt = new Collection { 0, 1, 2 }, + ICollectionNullableInt = new Collection { 0, null, 2 }, + ICollectionString = new Collection { "0", "1", "2" }, + ICollectionNullableString = new Collection { "0", null, "2" }, + ICollectionStruct = new Collection { new("0"), new("1"), new("2") }, + ICollectionNullableStruct = new Collection { new("0"), null, new("2") }, + ICollectionClass = new Collection { new("0"), new("1"), new("2") }, + ICollectionNullableClass = new Collection { new("0"), null, new("2") }, + + CollectionInt = new Collection { 0, 1, 2 }, + CollectionNullableInt = new Collection { 0, null, 2 }, + CollectionString = new Collection { "0", "1", "2" }, + CollectionNullableString = new Collection { "0", null, "2" }, + CollectionStruct = new Collection { new("0"), new("1"), new("2") }, + CollectionNullableStruct = new Collection { new("0"), null, new("2") }, + CollectionClass = new Collection { new("0"), new("1"), new("2") }, + CollectionNullableClass = new Collection { new("0"), null, new("2") }, + }; + + var entry = context.Add(entity); + + context.SaveChanges(); + + ((IList)entity.EnumerableInt)[0] = 20; + (((IList)entity.EnumerableNullableInt))[0] = null; + ((IList)entity.EnumerableString)[2] = "22"; + ((IList)entity.EnumerableNullableString)[0] = "23"; + ((IList)entity.EnumerableStruct)[1] = new MyStruct("24"); + ((IList)entity.EnumerableNullableStruct)[2] = null; + ((IList)entity.EnumerableClass)[0] = new MyClass("26"); + ((IList)entity.EnumerableNullableClass)[1] = new MyClass("27"); + + entity.IListInt[2] = 30; + entity.IListNullableInt[0] = 31; + entity.IListString[1] = "32"; + entity.IListNullableString[2] = null; + entity.IListStruct[0] = new MyStruct("34"); + entity.IListNullableStruct[1] = new MyStruct("35"); + entity.IListClass[2] = new MyClass("36"); + entity.IListNullableClass[0] = null; + + ((IList)entity.ICollectionInt)[1] = 50; + (((IList)entity.ICollectionNullableInt))[2] = 51; + ((IList)entity.ICollectionString)[0] = "52"; + ((IList)entity.ICollectionNullableString)[2] = null; + ((IList)entity.ICollectionStruct)[2] = new MyStruct("54"); + ((IList)entity.ICollectionNullableStruct)[0] = new MyStruct("55"); + ((IList)entity.ICollectionClass)[1] = new MyClass("56"); + ((IList)entity.ICollectionNullableClass)[2] = null; + + ((IList)entity.CollectionInt)[0] = 60; + (((IList)entity.CollectionNullableInt))[0] = null; + ((IList)entity.CollectionString)[2] = "62"; + ((IList)entity.CollectionNullableString)[0] = "63"; + ((IList)entity.CollectionStruct)[1] = new MyStruct("64"); + ((IList)entity.CollectionNullableStruct)[2] = null; + ((IList)entity.CollectionClass)[0] = new MyClass("66"); + ((IList)entity.CollectionNullableClass)[1] = new MyClass("67"); + + context.ChangeTracker.DetectChanges(); + + Assert.True(entry.Property(e => e.EnumerableInt).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableInt).IsModified); + Assert.True(entry.Property(e => e.EnumerableString).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableString).IsModified); + Assert.True(entry.Property(e => e.EnumerableStruct).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableStruct).IsModified); + Assert.True(entry.Property(e => e.EnumerableClass).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableClass).IsModified); + + Assert.True(entry.Property(e => e.IListInt).IsModified); + Assert.True(entry.Property(e => e.IListNullableInt).IsModified); + Assert.True(entry.Property(e => e.IListString).IsModified); + Assert.True(entry.Property(e => e.IListNullableString).IsModified); + Assert.True(entry.Property(e => e.IListStruct).IsModified); + Assert.True(entry.Property(e => e.IListNullableStruct).IsModified); + Assert.True(entry.Property(e => e.IListClass).IsModified); + Assert.True(entry.Property(e => e.IListNullableClass).IsModified); + + Assert.True(entry.Property(e => e.ICollectionInt).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableInt).IsModified); + Assert.True(entry.Property(e => e.ICollectionString).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableString).IsModified); + Assert.True(entry.Property(e => e.ICollectionStruct).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableStruct).IsModified); + Assert.True(entry.Property(e => e.ICollectionClass).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableClass).IsModified); + + Assert.True(entry.Property(e => e.CollectionInt).IsModified); + Assert.True(entry.Property(e => e.CollectionNullableInt).IsModified); + Assert.True(entry.Property(e => e.CollectionString).IsModified); + Assert.True(entry.Property(e => e.CollectionNullableString).IsModified); + Assert.True(entry.Property(e => e.CollectionStruct).IsModified); + Assert.True(entry.Property(e => e.CollectionNullableStruct).IsModified); + Assert.True(entry.Property(e => e.CollectionClass).IsModified); + Assert.True(entry.Property(e => e.CollectionNullableClass).IsModified); + + context.SaveChanges(); + + Assert.Equal(EntityState.Unchanged, entry.State); + } + + [ConditionalFact] + public void Can_detect_changes_to_primitive_collections_using_ObservableCollection() + { + using var context = new SomeLists(); + + var entity = new Voidbringer + { + Id = 4, + + EnumerableInt = new ObservableCollection { 0, 1, 2 }, + EnumerableNullableInt = new ObservableCollection { 0, null, 2 }, + EnumerableString = new ObservableCollection { "0", "1", "2" }, + EnumerableNullableString = new ObservableCollection { "0", null, "2" }, + EnumerableStruct = new ObservableCollection { new("0"), new("1"), new("2") }, + EnumerableNullableStruct = new ObservableCollection { new("0"), null, new("2") }, + EnumerableClass = new ObservableCollection { new("0"), new("1"), new("2") }, + EnumerableNullableClass = new ObservableCollection { new("0"), null, new("2") }, + + IListInt = new ObservableCollection { 0, 1, 2 }, + IListNullableInt = new ObservableCollection { 0, null, 2 }, + IListString = new ObservableCollection { "0", "1", "2" }, + IListNullableString = new ObservableCollection { "0", null, "2" }, + IListStruct = new ObservableCollection { new("0"), new("1"), new("2") }, + IListNullableStruct = new ObservableCollection { new("0"), null, new("2") }, + IListClass = new ObservableCollection { new("0"), new("1"), new("2") }, + IListNullableClass = new ObservableCollection { new("0"), null, new("2") }, + + ICollectionInt = new ObservableCollection { 0, 1, 2 }, + ICollectionNullableInt = new ObservableCollection { 0, null, 2 }, + ICollectionString = new ObservableCollection { "0", "1", "2" }, + ICollectionNullableString = new ObservableCollection { "0", null, "2" }, + ICollectionStruct = new ObservableCollection { new("0"), new("1"), new("2") }, + ICollectionNullableStruct = new ObservableCollection { new("0"), null, new("2") }, + ICollectionClass = new ObservableCollection { new("0"), new("1"), new("2") }, + ICollectionNullableClass = new ObservableCollection { new("0"), null, new("2") }, + + ObservableCollectionInt = new ObservableCollection { 0, 1, 2 }, + ObservableCollectionNullableInt = new ObservableCollection { 0, null, 2 }, + ObservableCollectionString = new ObservableCollection { "0", "1", "2" }, + ObservableCollectionNullableString = new ObservableCollection { "0", null, "2" }, + ObservableCollectionStruct = new ObservableCollection { new("0"), new("1"), new("2") }, + ObservableCollectionNullableStruct = new ObservableCollection { new("0"), null, new("2") }, + ObservableCollectionClass = new ObservableCollection { new("0"), new("1"), new("2") }, + ObservableCollectionNullableClass = new ObservableCollection { new("0"), null, new("2") }, + }; + + var entry = context.Add(entity); + + context.SaveChanges(); + + ((IList)entity.EnumerableInt)[0] = 20; + (((IList)entity.EnumerableNullableInt))[2] = null; + ((IList)entity.EnumerableString)[2] = "22"; + ((IList)entity.EnumerableNullableString)[0] = "23"; + ((IList)entity.EnumerableStruct)[1] = new MyStruct("24"); + ((IList)entity.EnumerableNullableStruct)[2] = null; + ((IList)entity.EnumerableClass)[0] = new MyClass("26"); + ((IList)entity.EnumerableNullableClass)[1] = new MyClass("27"); + + entity.IListInt[2] = 30; + entity.IListNullableInt[0] = 31; + entity.IListString[1] = "32"; + entity.IListNullableString[2] = null; + entity.IListStruct[0] = new MyStruct("34"); + entity.IListNullableStruct[1] = new MyStruct("35"); + entity.IListClass[2] = new MyClass("36"); + entity.IListNullableClass[0] = null; + + ((IList)entity.ICollectionInt)[1] = 50; + (((IList)entity.ICollectionNullableInt))[2] = 51; + ((IList)entity.ICollectionString)[0] = "52"; + ((IList)entity.ICollectionNullableString)[0] = null; + ((IList)entity.ICollectionStruct)[2] = new MyStruct("54"); + ((IList)entity.ICollectionNullableStruct)[0] = new MyStruct("55"); + ((IList)entity.ICollectionClass)[1] = new MyClass("56"); + ((IList)entity.ICollectionNullableClass)[2] = null; + + ((IList)entity.ObservableCollectionInt)[0] = 70; + (((IList)entity.ObservableCollectionNullableInt))[1] = 71; + ((IList)entity.ObservableCollectionString)[2] = "72"; + ((IList)entity.ObservableCollectionNullableString)[0] = null; + ((IList)entity.ObservableCollectionStruct)[1] = new MyStruct("74"); + ((IList)entity.ObservableCollectionNullableStruct)[2] = new MyStruct("75"); + ((IList)entity.ObservableCollectionClass)[0] = new MyClass("76"); + ((IList)entity.ObservableCollectionNullableClass)[2] = null; + + context.ChangeTracker.DetectChanges(); + + Assert.True(entry.Property(e => e.EnumerableInt).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableInt).IsModified); + Assert.True(entry.Property(e => e.EnumerableString).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableString).IsModified); + Assert.True(entry.Property(e => e.EnumerableStruct).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableStruct).IsModified); + Assert.True(entry.Property(e => e.EnumerableClass).IsModified); + Assert.True(entry.Property(e => e.EnumerableNullableClass).IsModified); + + Assert.True(entry.Property(e => e.IListInt).IsModified); + Assert.True(entry.Property(e => e.IListNullableInt).IsModified); + Assert.True(entry.Property(e => e.IListString).IsModified); + Assert.True(entry.Property(e => e.IListNullableString).IsModified); + Assert.True(entry.Property(e => e.IListStruct).IsModified); + Assert.True(entry.Property(e => e.IListNullableStruct).IsModified); + Assert.True(entry.Property(e => e.IListClass).IsModified); + Assert.True(entry.Property(e => e.IListNullableClass).IsModified); + + Assert.True(entry.Property(e => e.ICollectionInt).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableInt).IsModified); + Assert.True(entry.Property(e => e.ICollectionString).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableString).IsModified); + Assert.True(entry.Property(e => e.ICollectionStruct).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableStruct).IsModified); + Assert.True(entry.Property(e => e.ICollectionClass).IsModified); + Assert.True(entry.Property(e => e.ICollectionNullableClass).IsModified); + + Assert.True(entry.Property(e => e.ObservableCollectionInt).IsModified); + Assert.True(entry.Property(e => e.ObservableCollectionNullableInt).IsModified); + Assert.True(entry.Property(e => e.ObservableCollectionString).IsModified); + Assert.True(entry.Property(e => e.ObservableCollectionNullableString).IsModified); + Assert.True(entry.Property(e => e.ObservableCollectionStruct).IsModified); + Assert.True(entry.Property(e => e.ObservableCollectionNullableStruct).IsModified); + Assert.True(entry.Property(e => e.ObservableCollectionClass).IsModified); + Assert.True(entry.Property(e => e.ObservableCollectionNullableClass).IsModified); + + context.SaveChanges(); + + Assert.Equal(EntityState.Unchanged, entry.State); + } + + [ConditionalFact] + public void List_comparer_throws_when_used_with_non_list() + { + var comparer = new ListComparer(new ValueComparer(favorStructuralComparisons: false)); + + Assert.Equal( + CoreStrings.BadListType("HashSet", "ListComparer", "IList"), + Assert.Throws(() => comparer.Equals(new List(), new HashSet())).Message); + + Assert.Equal( + CoreStrings.BadListType("HashSet", "ListComparer", "IList"), + Assert.Throws(() => comparer.Equals(new HashSet(), new List())).Message); + + Assert.Equal( + CoreStrings.BadListType("HashSet", "ListComparer", "IList"), + Assert.Throws(() => comparer.Snapshot(new HashSet())).Message); + } + + [ConditionalFact] + public void Nullable_list_comparer_throws_when_used_with_non_list() + { + var comparer = new NullableValueTypeListComparer(new ValueComparer(favorStructuralComparisons: false)); + + Assert.Equal( + CoreStrings.BadListType("HashSet", "NullableValueTypeListComparer", "IList"), + Assert.Throws(() => comparer.Equals(new List(), new HashSet())).Message); + + Assert.Equal( + CoreStrings.BadListType("HashSet", "NullableValueTypeListComparer", "IList"), + Assert.Throws(() => comparer.Equals(new HashSet(), new List())).Message); + + Assert.Equal( + CoreStrings.BadListType("HashSet", "NullableValueTypeListComparer", "IList"), + Assert.Throws(() => comparer.Snapshot(new HashSet())).Message); + } + + private class Voidbringer + { + public int Id { get; set; } + + public int[]? ArrayInt { get; set; } + public int?[]? ArrayNullableInt { get; set; } + public string[]? ArrayString { get; set; } + public string?[]? ArrayNullableString { get; set; } + public MyStruct[]? ArrayStruct { get; set; } + public MyStruct?[]? ArrayNullableStruct { get; set; } + public MyClass[]? ArrayClass { get; set; } + public MyClass?[]? ArrayNullableClass { get; set; } + + public IEnumerable? EnumerableInt { get; set; } + public IEnumerable? EnumerableNullableInt { get; set; } + public IEnumerable? EnumerableString { get; set; } + public IEnumerable? EnumerableNullableString { get; set; } + public IEnumerable? EnumerableStruct { get; set; } + public IEnumerable? EnumerableNullableStruct { get; set; } + public IEnumerable? EnumerableClass { get; set; } + public IEnumerable? EnumerableNullableClass { get; set; } + + public IList? IListInt { get; set; } + public IList? IListNullableInt { get; set; } + public IList? IListString { get; set; } + public IList? IListNullableString { get; set; } + public IList? IListStruct { get; set; } + public IList? IListNullableStruct { get; set; } + public IList? IListClass { get; set; } + public IList? IListNullableClass { get; set; } + + public List? ListInt { get; set; } + public List? ListNullableInt { get; set; } + public List? ListString { get; set; } + public List? ListNullableString { get; set; } + public List? ListStruct { get; set; } + public List? ListNullableStruct { get; set; } + public List? ListClass { get; set; } + public List? ListNullableClass { get; set; } + + public ICollection? ICollectionInt { get; set; } + public ICollection? ICollectionNullableInt { get; set; } + public ICollection? ICollectionString { get; set; } + public ICollection? ICollectionNullableString { get; set; } + public ICollection? ICollectionStruct { get; set; } + public ICollection? ICollectionNullableStruct { get; set; } + public ICollection? ICollectionClass { get; set; } + public ICollection? ICollectionNullableClass { get; set; } + + public Collection? CollectionInt { get; set; } + public Collection? CollectionNullableInt { get; set; } + public Collection? CollectionString { get; set; } + public Collection? CollectionNullableString { get; set; } + public Collection? CollectionStruct { get; set; } + public Collection? CollectionNullableStruct { get; set; } + public Collection? CollectionClass { get; set; } + public Collection? CollectionNullableClass { get; set; } + + public ObservableCollection? ObservableCollectionInt { get; set; } + public ObservableCollection? ObservableCollectionNullableInt { get; set; } + public ObservableCollection? ObservableCollectionString { get; set; } + public ObservableCollection? ObservableCollectionNullableString { get; set; } + public ObservableCollection? ObservableCollectionStruct { get; set; } + public ObservableCollection? ObservableCollectionNullableStruct { get; set; } + public ObservableCollection? ObservableCollectionClass { get; set; } + public ObservableCollection? ObservableCollectionNullableClass { get; set; } + } + + private struct MyStruct(string value) + { + public string Value { get; set; } = value; + } + + private class MyClass(string value) + { + public string Value { get; set; } = value; + } + + private class SomeLists : DbContext + { + protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseInMemoryDatabase(nameof(SomeLists)); + + protected internal override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity( + b => + { + b.PrimitiveCollection(e => e.ArrayStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ArrayNullableStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ArrayClass).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ArrayNullableClass).ElementType().HasConversion(); + + b.PrimitiveCollection(e => e.EnumerableStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.EnumerableNullableStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.EnumerableClass).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.EnumerableNullableClass).ElementType().HasConversion(); + + b.PrimitiveCollection(e => e.IListStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.IListNullableStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.IListClass).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.IListNullableClass).ElementType().HasConversion(); + + b.PrimitiveCollection(e => e.ListStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ListNullableStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ListClass).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ListNullableClass).ElementType().HasConversion(); + + b.PrimitiveCollection(e => e.ICollectionStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ICollectionNullableStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ICollectionClass).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ICollectionNullableClass).ElementType().HasConversion(); + + b.PrimitiveCollection(e => e.CollectionStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.CollectionNullableStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.CollectionClass).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.CollectionNullableClass).ElementType().HasConversion(); + + b.PrimitiveCollection(e => e.ObservableCollectionStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ObservableCollectionNullableStruct).ElementType().HasConversion(); + b.PrimitiveCollection(e => e.ObservableCollectionClass).ElementType() + .HasConversion(); + b.PrimitiveCollection(e => e.ObservableCollectionNullableClass).ElementType() + .HasConversion(); + + // var myStructListComparer = new ListComparer(new ValueComparer(favorStructuralComparisons: false)); + // var nullableMyStructListComparer = + // new NullableValueTypeListComparer(new ValueComparer(favorStructuralComparisons: false)); + // var myClassListComparer = new ListComparer(new MyClassComparer()); + // var intListComparer = new ListComparer(new ValueComparer(favorStructuralComparisons: false)); + // var nullableIntListComparer = + // new NullableValueTypeListComparer(new ValueComparer(favorStructuralComparisons: false)); + // var stringListComparer = new ListComparer(new ValueComparer(favorStructuralComparisons: false)); + }); + } + } + + private class MyClassConverter() : ValueConverter(v => v.Value, v => new(v)); + + private class MyStructConverter() : ValueConverter(v => v.Value, v => new() { Value = v }); + + private class MyClassComparer() : ValueComparer( + (l, r) => l!.Value == r!.Value, v => v.Value.GetHashCode(), v => new MyClass(v.Value)); +} From 47800f60d54e18c436cab4010a618aae6f348211 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Thu, 17 Aug 2023 10:23:12 +0100 Subject: [PATCH 2/2] Fix for object collections --- .../Storage/RelationalTypeMappingSource.cs | 8 +++----- src/EFCore/ChangeTracking/ListComparer.cs | 4 ++-- .../ChangeTracking/NullableValueTypeListComparer.cs | 8 ++++---- src/EFCore/Storage/TypeMappingSource.cs | 8 +++----- 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs b/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs index 70753c99635..258c6ed21b0 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs @@ -235,14 +235,12 @@ protected override CoreTypeMapping FindMapping(in TypeMappingInfo mappingInfo) new ValueConverterInfo(modelType, typeof(string), _ => null!)))! .Clone( (ValueConverter)Activator.CreateInstance( - typeof(CollectionToJsonStringConverter<>).MakeGenericType( - elementType), - collectionReaderWriter!)!, + typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!, (ValueComparer?)Activator.CreateInstance( elementType.IsNullableValueType() ? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType()) - : typeof(ListComparer<>).MakeGenericType(elementType), - elementMapping?.Comparer), + : typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type), + elementMapping!.Comparer), elementMapping, collectionReaderWriter) : null; diff --git a/src/EFCore/ChangeTracking/ListComparer.cs b/src/EFCore/ChangeTracking/ListComparer.cs index 7982bc392a6..2f9220722b0 100644 --- a/src/EFCore/ChangeTracking/ListComparer.cs +++ b/src/EFCore/ChangeTracking/ListComparer.cs @@ -112,7 +112,7 @@ private static IList Snapshot(IEnumerable source, ValueCompa typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName())); } - if (sourceList.GetType().IsArray) + if (sourceList.IsReadOnly) { var snapshot = new TElement[sourceList.Count]; @@ -129,7 +129,7 @@ private static IList Snapshot(IEnumerable source, ValueCompa } else { - var snapshot = source is List + var snapshot = (source is List || sourceList.IsReadOnly) ? new List(sourceList.Count) : (IList)Activator.CreateInstance(source.GetType())!; diff --git a/src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs b/src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs index 657660a93b7..d2e8ac223ee 100644 --- a/src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs +++ b/src/EFCore/ChangeTracking/NullableValueTypeListComparer.cs @@ -87,7 +87,7 @@ private static bool Compare(IEnumerable? a, IEnumerable? b CoreStrings.BadListType( (a is IList ? b : a).GetType().ShortDisplayName(), typeof(NullableValueTypeListComparer).ShortDisplayName(), - typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName())); + typeof(IList<>).MakeGenericType(elementComparer.Type.MakeNullable()).ShortDisplayName())); } private static int GetHashCode(IEnumerable source, ValueComparer elementComparer) @@ -110,10 +110,10 @@ private static int GetHashCode(IEnumerable source, ValueComparer).ShortDisplayName(), - typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName())); + typeof(IList<>).MakeGenericType(elementComparer.Type.MakeNullable()).ShortDisplayName())); } - if (sourceList.GetType().IsArray) + if (sourceList.IsReadOnly) { var snapshot = new TElement?[sourceList.Count]; @@ -127,7 +127,7 @@ private static int GetHashCode(IEnumerable source, ValueComparer + var snapshot = source is List || sourceList.IsReadOnly ? new List(sourceList.Count) : (IList)Activator.CreateInstance(source.GetType())!; diff --git a/src/EFCore/Storage/TypeMappingSource.cs b/src/EFCore/Storage/TypeMappingSource.cs index 73411238962..6c9f0b5f57f 100644 --- a/src/EFCore/Storage/TypeMappingSource.cs +++ b/src/EFCore/Storage/TypeMappingSource.cs @@ -191,14 +191,12 @@ protected TypeMappingSource(TypeMappingSourceDependencies dependencies) new ValueConverterInfo(modelType, typeof(string), _ => null!)))! .Clone( (ValueConverter)Activator.CreateInstance( - typeof(CollectionToJsonStringConverter<>).MakeGenericType( - elementType!), - collectionReaderWriter!)!, + typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!, (ValueComparer?)Activator.CreateInstance( elementType.IsNullableValueType() ? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType()) - : typeof(ListComparer<>).MakeGenericType(elementType), - elementMapping?.Comparer), + : typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type), + elementMapping!.Comparer), elementMapping, collectionReaderWriter) : null;