Skip to content

Commit 76d5bef

Browse files
ajcvickersajcvickers
authored
Handle comparers for nullable value types in primitive collections (#35235)
1 parent 03522db commit 76d5bef

File tree

7 files changed

+1231
-399
lines changed

7 files changed

+1231
-399
lines changed

‎src/EFCore/Storage/TypeMappingSourceBase.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ protected virtual bool TryFindJsonCollectionMapping(
174174
elementReader);
175175

176176
elementComparer = (ValueComparer?)Activator.CreateInstance(
177-
elementType.IsNullableValueType()
177+
elementType.IsNullableValueType() || elementMapping.Comparer.Type.IsNullableValueType()
178178
? typeof(ListOfNullableValueTypesComparer<,>).MakeGenericType(typeToInstantiate, elementType.UnwrapNullableType())
179179
: elementType.IsValueType
180180
? typeof(ListOfValueTypesComparer<,>).MakeGenericType(typeToInstantiate, elementType)

‎test/EFCore.Cosmos.FunctionalTests/Query/PrimitiveCollectionsQueryCosmosTest.cs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2100,6 +2100,82 @@ FROM root c
21002100

21012101
#endregion Cosmos-specific tests
21022102

2103+
public override async Task Parameter_collection_of_structs_Contains_struct(bool async)
2104+
{
2105+
// Always throws for sync before getting to the exception to test.
2106+
if (async)
2107+
{
2108+
// Requires collections of converted elements
2109+
await Assert.ThrowsAsync<InvalidOperationException>(() => base.Parameter_collection_of_structs_Contains_struct(async));
2110+
2111+
AssertSql();
2112+
}
2113+
}
2114+
2115+
public override async Task Parameter_collection_of_structs_Contains_nullable_struct(bool async)
2116+
{
2117+
// Always throws for sync before getting to the exception to test.
2118+
if (async)
2119+
{
2120+
// Requires collections of converted elements
2121+
await Assert.ThrowsAsync<InvalidOperationException>(() => base.Parameter_collection_of_structs_Contains_nullable_struct(async));
2122+
2123+
AssertSql();
2124+
}
2125+
}
2126+
2127+
public override async Task Parameter_collection_of_structs_Contains_nullable_struct_with_nullable_comparer(bool async)
2128+
{
2129+
// Always throws for sync before getting to the exception to test.
2130+
if (async)
2131+
{
2132+
// Requires collections of converted elements
2133+
await Assert.ThrowsAsync<InvalidOperationException>(
2134+
() => base.Parameter_collection_of_structs_Contains_nullable_struct_with_nullable_comparer(async));
2135+
2136+
AssertSql();
2137+
}
2138+
}
2139+
2140+
public override async Task Parameter_collection_of_nullable_structs_Contains_struct(bool async)
2141+
{
2142+
// Always throws for sync before getting to the exception to test.
2143+
if (async)
2144+
{
2145+
// Requires collections of converted elements
2146+
await Assert.ThrowsAsync<InvalidOperationException>(
2147+
() => base.Parameter_collection_of_nullable_structs_Contains_struct(async));
2148+
2149+
AssertSql();
2150+
}
2151+
}
2152+
2153+
public override async Task Parameter_collection_of_nullable_structs_Contains_nullable_struct(bool async)
2154+
{
2155+
// Always throws for sync before getting to the exception to test.
2156+
if (async)
2157+
{
2158+
// Requires collections of converted elements
2159+
await Assert.ThrowsAsync<InvalidOperationException>(
2160+
() => base.Parameter_collection_of_nullable_structs_Contains_nullable_struct(async));
2161+
2162+
AssertSql();
2163+
}
2164+
}
2165+
2166+
public override async Task Parameter_collection_of_nullable_structs_Contains_nullable_struct_with_nullable_comparer(bool async)
2167+
{
2168+
// Always throws for sync before getting to the exception to test.
2169+
if (async)
2170+
{
2171+
// Requires collections of converted elements
2172+
await Assert.ThrowsAsync<InvalidOperationException>(
2173+
() => base.Parameter_collection_of_nullable_structs_Contains_nullable_struct_with_nullable_comparer(async));
2174+
2175+
AssertSql();
2176+
}
2177+
}
2178+
21032179
[ConditionalFact]
21042180
public virtual void Check_all_tests_overridden()
21052181
=> TestHelpers.AssertAllMethodsOverridden(GetType());

‎test/EFCore.Specification.Tests/Query/PrimitiveCollectionsQueryTestBase.cs‎

Lines changed: 150 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -367,14 +367,16 @@ await AssertQuery(
367367
[MemberData(nameof(IsAsyncData))]
368368
public virtual async Task Parameter_collection_of_ints_Contains_nullable_int(bool async)
369369
{
370-
var ints = new int?[] { 10, 999 };
370+
var ints = new[] { 10, 999 };
371371

372372
await AssertQuery(
373373
async,
374-
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => ints.Contains(c.NullableInt)));
374+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => ints.Contains(c.NullableInt!.Value)),
375+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => c.NullableInt != null && ints.Contains(c.NullableInt!.Value)));
375376
await AssertQuery(
376377
async,
377-
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => !ints.Contains(c.NullableInt)));
378+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => !ints.Contains(c.NullableInt!.Value)),
379+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => c.NullableInt == null || !ints.Contains(c.NullableInt!.Value)));
378380
}
379381

380382
[ConditionalTheory]
@@ -405,6 +407,114 @@ await AssertQuery(
405407
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => !nullableInts.Contains(c.NullableInt)));
406408
}
407409

410+
[ConditionalTheory]
411+
[MemberData(nameof(IsAsyncData))]
412+
public virtual async Task Parameter_collection_of_structs_Contains_struct(bool async)
413+
{
414+
var values = new List<WrappedId> { new(22), new(33) };
415+
416+
await AssertQuery(
417+
async,
418+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => values.Contains(c.WrappedId)));
419+
420+
values = new List<WrappedId> { new(11), new(44) };
421+
422+
await AssertQuery(
423+
async,
424+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => !values.Contains(c.WrappedId)));
425+
}
426+
427+
[ConditionalTheory]
428+
[MemberData(nameof(IsAsyncData))]
429+
public virtual async Task Parameter_collection_of_structs_Contains_nullable_struct(bool async)
430+
{
431+
var values = new List<WrappedId> { new(22), new(33) };
432+
433+
await AssertQuery(
434+
async,
435+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => values.Contains(c.NullableWrappedId!.Value)),
436+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => c.NullableWrappedId != null && values.Contains(c.NullableWrappedId.Value)));
437+
438+
values = new List<WrappedId> { new(11), new(44) };
439+
440+
await AssertQuery(
441+
async,
442+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => !values.Contains(c.NullableWrappedId!.Value)),
443+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => c.NullableWrappedId == null || !values.Contains(c.NullableWrappedId!.Value)));
444+
}
445+
446+
[ConditionalTheory]
447+
[MemberData(nameof(IsAsyncData))] // Issue #35117
448+
public virtual async Task Parameter_collection_of_structs_Contains_nullable_struct_with_nullable_comparer(bool async)
449+
{
450+
var values = new List<WrappedId> { new(22), new(33) };
451+
452+
await AssertQuery(
453+
async,
454+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => values.Contains(c.NullableWrappedIdWithNullableComparer!.Value)),
455+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(
456+
c => c.NullableWrappedIdWithNullableComparer != null && values.Contains(c.NullableWrappedIdWithNullableComparer.Value)));
457+
458+
values = new List<WrappedId> { new(11), new(44) };
459+
460+
await AssertQuery(
461+
async,
462+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => !values.Contains(c.NullableWrappedId!.Value)),
463+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(
464+
c => c.NullableWrappedIdWithNullableComparer == null || !values.Contains(c.NullableWrappedIdWithNullableComparer!.Value)));
465+
}
466+
467+
[ConditionalTheory]
468+
[MemberData(nameof(IsAsyncData))]
469+
public virtual async Task Parameter_collection_of_nullable_structs_Contains_struct(bool async)
470+
{
471+
var values = new List<WrappedId?> { null, new(22) };
472+
473+
await AssertQuery(
474+
async,
475+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => values.Contains(c.WrappedId)));
476+
477+
values = new List<WrappedId?> { new(11), new(44) };
478+
479+
await AssertQuery(
480+
async,
481+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => !values.Contains(c.WrappedId)));
482+
}
483+
484+
[ConditionalTheory]
485+
[MemberData(nameof(IsAsyncData))]
486+
public virtual async Task Parameter_collection_of_nullable_structs_Contains_nullable_struct(bool async)
487+
{
488+
var values = new List<WrappedId?> { null, new(22) };
489+
490+
await AssertQuery(
491+
async,
492+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => values.Contains(c.NullableWrappedId)));
493+
494+
values = new List<WrappedId?> { new(11), new(44) };
495+
496+
await AssertQuery(
497+
async,
498+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => !values.Contains(c.NullableWrappedId)));
499+
}
500+
501+
[ConditionalTheory]
502+
[MemberData(nameof(IsAsyncData))]
503+
public virtual async Task Parameter_collection_of_nullable_structs_Contains_nullable_struct_with_nullable_comparer(bool async)
504+
{
505+
var values = new List<WrappedId?> { null, new(22) };
506+
507+
await AssertQuery(
508+
async,
509+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => values.Contains(c.NullableWrappedIdWithNullableComparer)));
510+
511+
values = new List<WrappedId?> { new(11), new(44) };
512+
513+
await AssertQuery(
514+
async,
515+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => !values.Contains(c.NullableWrappedIdWithNullableComparer)));
516+
}
517+
408518
[ConditionalTheory]
409519
[MemberData(nameof(IsAsyncData))]
410520
public virtual async Task Parameter_collection_of_strings_Contains_string(bool async)
@@ -1370,7 +1480,21 @@ public Func<DbContext> GetContextCreator()
13701480
=> () => CreateContext();
13711481

13721482
protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
1373-
=> modelBuilder.Entity<PrimitiveCollectionsEntity>().Property(p => p.Id).ValueGeneratedNever();
1483+
=> modelBuilder.Entity<PrimitiveCollectionsEntity>(
1484+
b =>
1485+
{
1486+
b.Property(e => e.Id).ValueGeneratedNever();
1487+
b.Property(e => e.WrappedId).HasConversion<WrappedIdConverter>();
1488+
b.Property(e => e.NullableWrappedId).HasConversion<WrappedIdConverter>();
1489+
b.Property(e => e.NullableWrappedIdWithNullableComparer).HasConversion<NullableWrappedIdConverter>();
1490+
});
1491+
1492+
protected class WrappedIdConverter() : ValueConverter<WrappedId, int>(v => v.Value, v => new(v));
1493+
1494+
// Note that value comparers over nullable value types are not a good idea, unless the comparer is handling nulls itself.
1495+
protected class NullableWrappedIdConverter() : ValueConverter<WrappedId?, int?>(
1496+
id => id == null ? null : id.Value.Value,
1497+
value => value == null ? null : new WrappedId(value.Value));
13741498

13751499
protected override Task SeedAsync(PrimitiveCollectionsContext context)
13761500
{
@@ -1420,9 +1544,12 @@ public class PrimitiveCollectionsEntity
14201544
public int Int { get; set; }
14211545
public DateTime DateTime { get; set; }
14221546
public bool Bool { get; set; }
1547+
public WrappedId WrappedId { get; set; }
14231548
public MyEnum Enum { get; set; }
14241549
public int? NullableInt { get; set; }
14251550
public string? NullableString { get; set; }
1551+
public WrappedId? NullableWrappedId { get; set; }
1552+
public WrappedId? NullableWrappedIdWithNullableComparer { get; set; }
14261553

14271554
public required string[] Strings { get; set; }
14281555
public required int[] Ints { get; set; }
@@ -1433,6 +1560,8 @@ public class PrimitiveCollectionsEntity
14331560
public required string?[] NullableStrings { get; set; }
14341561
}
14351562

1563+
public readonly record struct WrappedId(int Value);
1564+
14361565
public enum MyEnum { Value1, Value2, Value3, Value4 }
14371566

14381567
public class PrimitiveCollectionsData : ISetSource
@@ -1464,8 +1593,11 @@ private static IReadOnlyList<PrimitiveCollectionsEntity> CreatePrimitiveArrayEnt
14641593
DateTime = new DateTime(2020, 1, 10, 12, 30, 0, DateTimeKind.Utc),
14651594
Bool = true,
14661595
Enum = MyEnum.Value1,
1596+
WrappedId = new(22),
14671597
NullableInt = 10,
14681598
NullableString = "10",
1599+
NullableWrappedId = new(22),
1600+
NullableWrappedIdWithNullableComparer = new(22),
14691601
Ints = [1, 10],
14701602
Strings = ["1", "10"],
14711603
DateTimes =
@@ -1475,7 +1607,7 @@ private static IReadOnlyList<PrimitiveCollectionsEntity> CreatePrimitiveArrayEnt
14751607
Bools = [true, false],
14761608
Enums = [MyEnum.Value1, MyEnum.Value2],
14771609
NullableInts = [1, 10],
1478-
NullableStrings = ["1", "10"]
1610+
NullableStrings = ["1", "10"],
14791611
},
14801612
new()
14811613
{
@@ -1485,8 +1617,11 @@ private static IReadOnlyList<PrimitiveCollectionsEntity> CreatePrimitiveArrayEnt
14851617
DateTime = new DateTime(2020, 1, 11, 12, 30, 0, DateTimeKind.Utc),
14861618
Bool = false,
14871619
Enum = MyEnum.Value2,
1620+
WrappedId = new(22),
14881621
NullableInt = null,
14891622
NullableString = null,
1623+
NullableWrappedId = null,
1624+
NullableWrappedIdWithNullableComparer = null,
14901625
Ints = [1, 11, 111],
14911626
Strings = ["1", "11", "111"],
14921627
DateTimes =
@@ -1508,8 +1643,11 @@ private static IReadOnlyList<PrimitiveCollectionsEntity> CreatePrimitiveArrayEnt
15081643
DateTime = new DateTime(2022, 1, 10, 12, 30, 0, DateTimeKind.Utc),
15091644
Bool = true,
15101645
Enum = MyEnum.Value1,
1646+
WrappedId = new(22),
15111647
NullableInt = 20,
15121648
NullableString = "20",
1649+
NullableWrappedId = new(22),
1650+
NullableWrappedIdWithNullableComparer = new(22),
15131651
Ints = [1, 1, 10, 10, 10, 1, 10],
15141652
Strings = ["1", "10", "10", "1", "1"],
15151653
DateTimes =
@@ -1533,8 +1671,11 @@ private static IReadOnlyList<PrimitiveCollectionsEntity> CreatePrimitiveArrayEnt
15331671
DateTime = new DateTime(2024, 1, 11, 12, 30, 0, DateTimeKind.Utc),
15341672
Bool = false,
15351673
Enum = MyEnum.Value2,
1674+
WrappedId = new(22),
15361675
NullableInt = null,
15371676
NullableString = null,
1677+
NullableWrappedId = null,
1678+
NullableWrappedIdWithNullableComparer = null,
15381679
Ints = [1, 1, 111, 11, 1, 111],
15391680
Strings = ["1", "11", "111", "11"],
15401681
DateTimes =
@@ -1551,7 +1692,7 @@ private static IReadOnlyList<PrimitiveCollectionsEntity> CreatePrimitiveArrayEnt
15511692
Bools = [false],
15521693
Enums = [MyEnum.Value2, MyEnum.Value3],
15531694
NullableInts = [null, null],
1554-
NullableStrings = [null, null]
1695+
NullableStrings = [null, null],
15551696
},
15561697
new()
15571698
{
@@ -1561,8 +1702,11 @@ private static IReadOnlyList<PrimitiveCollectionsEntity> CreatePrimitiveArrayEnt
15611702
DateTime = new DateTime(2000, 1, 1, 0, 0, 0, DateTimeKind.Utc),
15621703
Bool = false,
15631704
Enum = MyEnum.Value1,
1705+
WrappedId = new(22),
1706+
NullableWrappedIdWithNullableComparer = null,
15641707
NullableInt = null,
15651708
NullableString = null,
1709+
NullableWrappedId = null,
15661710
Ints = [],
15671711
Strings = [],
15681712
DateTimes = [],

0 commit comments

Comments
 (0)