Skip to content

Commit

Permalink
Allow shared columns with nullable value converters (#29778)
Browse files Browse the repository at this point in the history 
Fixes #29531
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Jan 4, 2023
1 parent 7767f3e commit 83e06a3
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 92 deletions.
19 changes: 17 additions & 2 deletions src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ namespace Microsoft.EntityFrameworkCore.Infrastructure;
/// </remarks>
public class RelationalModelValidator : ModelValidator
{
private static readonly bool QuirkEnabled29531
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29531", out var enabled) && enabled;

/// <summary>
/// Creates a new instance of <see cref="RelationalModelValidator" />.
/// </summary>
Expand Down Expand Up @@ -1406,8 +1409,20 @@ private static bool IsIdentifyingPrincipal(IEntityType dependentEntityType, IEnt
currentTypeString));
}

var currentProviderType = typeMapping.Converter?.ProviderClrType ?? typeMapping.ClrType;
var previousProviderType = duplicateTypeMapping.Converter?.ProviderClrType ?? duplicateTypeMapping.ClrType;
Type currentProviderType, previousProviderType;
if (QuirkEnabled29531)
{
currentProviderType = typeMapping.Converter?.ProviderClrType ?? typeMapping.ClrType;
previousProviderType = duplicateTypeMapping.Converter?.ProviderClrType ?? duplicateTypeMapping.ClrType;
}
else
{
currentProviderType = typeMapping.Converter?.ProviderClrType.UnwrapNullableType()
?? typeMapping.ClrType;
previousProviderType = duplicateTypeMapping.Converter?.ProviderClrType.UnwrapNullableType()
?? duplicateTypeMapping.ClrType;
}

if (currentProviderType != previousProviderType)
{
throw new InvalidOperationException(
Expand Down
15 changes: 14 additions & 1 deletion src/EFCore.Relational/Update/ModificationCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,9 @@ private sealed class ColumnValuePropagator

public IColumnModification? ColumnModification { get; set; }

private static readonly bool QuirkEnabled29531
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue29531", out var enabled) && enabled;

public void RecordValue(IColumnMapping mapping, IUpdateEntry entry)
{
var property = mapping.Property;
Expand Down Expand Up @@ -981,7 +984,17 @@ public bool TryPropagate(IColumnMappingBase mapping, IUpdateEntry entry)
if (property.GetAfterSaveBehavior() == PropertySaveBehavior.Save
|| entry.EntityState == EntityState.Added)
{
entry.SetStoreGeneratedValue(property, _currentValue);
var value = _currentValue;
if (!QuirkEnabled29531)
{
var converter = property.GetTypeMapping().Converter;
if (converter != null)
{
value = converter.ConvertFromProvider(value);
}
}

entry.SetStoreGeneratedValue(property, value);
}

return false;
Expand Down
56 changes: 47 additions & 9 deletions test/EFCore.Relational.Specification.Tests/UpdatesRelationalTestBase.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.EntityFrameworkCore.TestModels.UpdatesModel;
Expand Down Expand Up @@ -108,6 +108,39 @@ public virtual void Save_with_shared_foreign_key()
});
}

[ConditionalFact]
public virtual void Can_use_shared_columns_with_conversion()
=> ExecuteWithStrategyInTransaction(
context =>
{
var person = new Person("1", null)
{
Address = new Address { Country = Country.Eswatini, City = "Bulembu" },
Country = "Eswatini"
};
context.Add(person);
context.SaveChanges();
},
context =>
{
var person = context.Set<Person>().Single();
person.Address = new Address { Country = Country.Türkiye, City = "Konya", ZipCode = 42100 };
context.SaveChanges();
},
context =>
{
var person = context.Set<Person>().Single();
Assert.Equal(Country.Türkiye, person.Address!.Country);
Assert.Equal("Konya", person.Address.City);
Assert.Equal(42100, person.Address.ZipCode);
Assert.Equal("Türkiye", person.Country);
Assert.Equal("42100", person.ZipCode);
});

[ConditionalFact]
public virtual void Swap_filtered_unique_index_values()
{
Expand Down Expand Up @@ -174,14 +207,19 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con

modelBuilder.Entity<Product>().HasIndex(p => new { p.Name, p.Price }).IsUnique().HasFilter("Name IS NOT NULL");

modelBuilder.Entity<Person>()
.Property(p => p.Country)
.HasColumnName("Country");

modelBuilder.Entity<Person>()
.OwnsOne(p => p.Address)
.Property(p => p.Country)
.HasColumnName("Country");
modelBuilder.Entity<Person>(pb =>
{
pb.Property(p => p.Country)
.HasColumnName("Country");
pb.Property(p => p.ZipCode)
.HasColumnName("ZipCode");
pb.OwnsOne(p => p.Address)
.Property(p => p.Country)
.HasColumnName("Country");
pb.OwnsOne(p => p.Address)
.Property(p => p.ZipCode)
.HasColumnName("ZipCode");
});

modelBuilder
.Entity<
Expand Down
1 change: 1 addition & 0 deletions test/EFCore.Specification.Tests/TestModels/UpdatesModel/Address.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ public class Address
{
public string City { get; set; } = null!;
public Country Country { get; set; }
public int? ZipCode { get; set; }
}
1 change: 1 addition & 0 deletions test/EFCore.Specification.Tests/TestModels/UpdatesModel/Person.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public Person(string name, Person? parent)
public string Name { get; set; }
public int? ParentId { get; set; }
public string? Country { get; set; }
public string? ZipCode { get; set; }
public Person? Parent { get; set; }
public Address? Address { get; set; }
}
20 changes: 11 additions & 9 deletions test/EFCore.Specification.Tests/UpdatesTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,15 +644,17 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
.HasForeignKey(e => e.DependentId)
.HasPrincipalKey(e => e.PrincipalId);
modelBuilder.Entity<Person>()
.HasOne(p => p.Parent)
.WithMany()
.OnDelete(DeleteBehavior.Restrict);
modelBuilder.Entity<Person>()
.OwnsOne(p => p.Address)
.Property(p => p.Country)
.HasConversion<string>();
modelBuilder.Entity<Person>(pb =>
{
pb.HasOne(p => p.Parent)
.WithMany()
.OnDelete(DeleteBehavior.Restrict);
pb.OwnsOne(p => p.Address)
.Property(p => p.Country)
.HasConversion<string>();
pb.Property(p => p.ZipCode)
.HasConversion<int?>(v => v == null ? null : int.Parse(v), v => v == null ? null : v.ToString()!);
});
modelBuilder.Entity<Category>().HasMany(e => e.ProductCategories).WithOne(e => e.Category)
.HasForeignKey(e => e.CategoryId);
Expand Down
156 changes: 85 additions & 71 deletions test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,61 +29,68 @@ public override void Can_add_and_remove_self_refs()
@p0=NULL (Size = 4000)
@p1='1' (Nullable = false) (Size = 4000)
@p2=NULL (DbType = Int32)
@p3=NULL (DbType = Int32)

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
INSERT INTO [Person] ([Country], [Name], [ParentId])
INSERT INTO [Person] ([Country], [Name], [ParentId], [ZipCode])
OUTPUT INSERTED.[PersonId]
VALUES (@p0, @p1, @p2);
VALUES (@p0, @p1, @p2, @p3);
""",
//
"""
@p3=NULL (Size = 4000)
@p4='2' (Nullable = false) (Size = 4000)
@p5='1' (Nullable = true)
@p6=NULL (Size = 4000)
@p7='3' (Nullable = false) (Size = 4000)
@p8='1' (Nullable = true)
//
"""
@p4=NULL (Size = 4000)
@p5='2' (Nullable = false) (Size = 4000)
@p6='1' (Nullable = true)
@p7=NULL (DbType = Int32)
@p8=NULL (Size = 4000)
@p9='3' (Nullable = false) (Size = 4000)
@p10='1' (Nullable = true)
@p11=NULL (DbType = Int32)

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
MERGE [Person] USING (
VALUES (@p3, @p4, @p5, 0),
(@p6, @p7, @p8, 1)) AS i ([Country], [Name], [ParentId], _Position) ON 1=0
VALUES (@p4, @p5, @p6, @p7, 0),
(@p8, @p9, @p10, @p11, 1)) AS i ([Country], [Name], [ParentId], [ZipCode], _Position) ON 1=0
WHEN NOT MATCHED THEN
INSERT ([Country], [Name], [ParentId])
VALUES (i.[Country], i.[Name], i.[ParentId])
INSERT ([Country], [Name], [ParentId], [ZipCode])
VALUES (i.[Country], i.[Name], i.[ParentId], i.[ZipCode])
OUTPUT INSERTED.[PersonId], i._Position;
""",
//
"""
@p9=NULL (Size = 4000)
@p10='4' (Nullable = false) (Size = 4000)
@p11='2' (Nullable = true)
//
"""
@p12=NULL (Size = 4000)
@p13='5' (Nullable = false) (Size = 4000)
@p13='4' (Nullable = false) (Size = 4000)
@p14='2' (Nullable = true)
@p15=NULL (Size = 4000)
@p16='6' (Nullable = false) (Size = 4000)
@p17='3' (Nullable = true)
@p18=NULL (Size = 4000)
@p19='7' (Nullable = false) (Size = 4000)
@p20='3' (Nullable = true)
@p15=NULL (DbType = Int32)
@p16=NULL (Size = 4000)
@p17='5' (Nullable = false) (Size = 4000)
@p18='2' (Nullable = true)
@p19=NULL (DbType = Int32)
@p20=NULL (Size = 4000)
@p21='6' (Nullable = false) (Size = 4000)
@p22='3' (Nullable = true)
@p23=NULL (DbType = Int32)
@p24=NULL (Size = 4000)
@p25='7' (Nullable = false) (Size = 4000)
@p26='3' (Nullable = true)
@p27=NULL (DbType = Int32)

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
MERGE [Person] USING (
VALUES (@p9, @p10, @p11, 0),
(@p12, @p13, @p14, 1),
(@p15, @p16, @p17, 2),
(@p18, @p19, @p20, 3)) AS i ([Country], [Name], [ParentId], _Position) ON 1=0
VALUES (@p12, @p13, @p14, @p15, 0),
(@p16, @p17, @p18, @p19, 1),
(@p20, @p21, @p22, @p23, 2),
(@p24, @p25, @p26, @p27, 3)) AS i ([Country], [Name], [ParentId], [ZipCode], _Position) ON 1=0
WHEN NOT MATCHED THEN
INSERT ([Country], [Name], [ParentId])
VALUES (i.[Country], i.[Name], i.[ParentId])
INSERT ([Country], [Name], [ParentId], [ZipCode])
VALUES (i.[Country], i.[Name], i.[ParentId], i.[ZipCode])
OUTPUT INSERTED.[PersonId], i._Position;
""",
//
"""
//
"""
@p0='4'
@p1='5'
@p2='6'
Expand All @@ -93,6 +100,7 @@ public override void Can_add_and_remove_self_refs()
@p6=NULL (Size = 4000)
@p7='1' (Nullable = false) (Size = 4000)
@p8=NULL (DbType = Int32)
@p9=NULL (DbType = Int32)

SET NOCOUNT ON;
DELETE FROM [Person]
Expand All @@ -113,62 +121,68 @@ OUTPUT 1
DELETE FROM [Person]
OUTPUT 1
WHERE [PersonId] = @p5;
INSERT INTO [Person] ([Country], [Name], [ParentId])
INSERT INTO [Person] ([Country], [Name], [ParentId], [ZipCode])
OUTPUT INSERTED.[PersonId]
VALUES (@p6, @p7, @p8);
VALUES (@p6, @p7, @p8, @p9);
""",
//
"""
@p9='1'
@p10=NULL (Size = 4000)
@p11='2' (Nullable = false) (Size = 4000)
@p12='8' (Nullable = true)
@p13=NULL (Size = 4000)
@p14='3' (Nullable = false) (Size = 4000)
@p15='8' (Nullable = true)
//
"""
@p10='1'
@p11=NULL (Size = 4000)
@p12='2' (Nullable = false) (Size = 4000)
@p13='8' (Nullable = true)
@p14=NULL (DbType = Int32)
@p15=NULL (Size = 4000)
@p16='3' (Nullable = false) (Size = 4000)
@p17='8' (Nullable = true)
@p18=NULL (DbType = Int32)

SET NOCOUNT ON;
DELETE FROM [Person]
OUTPUT 1
WHERE [PersonId] = @p9;
WHERE [PersonId] = @p10;
MERGE [Person] USING (
VALUES (@p10, @p11, @p12, 0),
(@p13, @p14, @p15, 1)) AS i ([Country], [Name], [ParentId], _Position) ON 1=0
VALUES (@p11, @p12, @p13, @p14, 0),
(@p15, @p16, @p17, @p18, 1)) AS i ([Country], [Name], [ParentId], [ZipCode], _Position) ON 1=0
WHEN NOT MATCHED THEN
INSERT ([Country], [Name], [ParentId])
VALUES (i.[Country], i.[Name], i.[ParentId])
INSERT ([Country], [Name], [ParentId], [ZipCode])
VALUES (i.[Country], i.[Name], i.[ParentId], i.[ZipCode])
OUTPUT INSERTED.[PersonId], i._Position;
""",
//
"""
@p16=NULL (Size = 4000)
@p17='4' (Nullable = false) (Size = 4000)
@p18='9' (Nullable = true)
//
"""
@p19=NULL (Size = 4000)
@p20='5' (Nullable = false) (Size = 4000)
@p20='4' (Nullable = false) (Size = 4000)
@p21='9' (Nullable = true)
@p22=NULL (Size = 4000)
@p23='6' (Nullable = false) (Size = 4000)
@p24='10' (Nullable = true)
@p25=NULL (Size = 4000)
@p26='7' (Nullable = false) (Size = 4000)
@p27='10' (Nullable = true)
@p22=NULL (DbType = Int32)
@p23=NULL (Size = 4000)
@p24='5' (Nullable = false) (Size = 4000)
@p25='9' (Nullable = true)
@p26=NULL (DbType = Int32)
@p27=NULL (Size = 4000)
@p28='6' (Nullable = false) (Size = 4000)
@p29='10' (Nullable = true)
@p30=NULL (DbType = Int32)
@p31=NULL (Size = 4000)
@p32='7' (Nullable = false) (Size = 4000)
@p33='10' (Nullable = true)
@p34=NULL (DbType = Int32)

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
MERGE [Person] USING (
VALUES (@p16, @p17, @p18, 0),
(@p19, @p20, @p21, 1),
(@p22, @p23, @p24, 2),
(@p25, @p26, @p27, 3)) AS i ([Country], [Name], [ParentId], _Position) ON 1=0
VALUES (@p19, @p20, @p21, @p22, 0),
(@p23, @p24, @p25, @p26, 1),
(@p27, @p28, @p29, @p30, 2),
(@p31, @p32, @p33, @p34, 3)) AS i ([Country], [Name], [ParentId], [ZipCode], _Position) ON 1=0
WHEN NOT MATCHED THEN
INSERT ([Country], [Name], [ParentId])
VALUES (i.[Country], i.[Name], i.[ParentId])
INSERT ([Country], [Name], [ParentId], [ZipCode])
VALUES (i.[Country], i.[Name], i.[ParentId], i.[ZipCode])
OUTPUT INSERTED.[PersonId], i._Position;
""",
//
"""
SELECT [p].[PersonId], [p].[Country], [p].[Name], [p].[ParentId], [p].[Address_City], [p].[Country], [p0].[PersonId], [p0].[Country], [p0].[Name], [p0].[ParentId], [p0].[Address_City], [p0].[Country], [p1].[PersonId], [p1].[Country], [p1].[Name], [p1].[ParentId], [p1].[Address_City], [p1].[Country], [p2].[PersonId], [p2].[Country], [p2].[Name], [p2].[ParentId], [p2].[Address_City], [p2].[Country]
//
"""
SELECT [p].[PersonId], [p].[Country], [p].[Name], [p].[ParentId], [p].[ZipCode], [p].[Address_City], [p].[Country], [p].[ZipCode], [p0].[PersonId], [p0].[Country], [p0].[Name], [p0].[ParentId], [p0].[ZipCode], [p0].[Address_City], [p0].[Country], [p0].[ZipCode], [p1].[PersonId], [p1].[Country], [p1].[Name], [p1].[ParentId], [p1].[ZipCode], [p1].[Address_City], [p1].[Country], [p1].[ZipCode], [p2].[PersonId], [p2].[Country], [p2].[Name], [p2].[ParentId], [p2].[ZipCode], [p2].[Address_City], [p2].[Country], [p2].[ZipCode]
FROM [Person] AS [p]
LEFT JOIN [Person] AS [p0] ON [p].[ParentId] = [p0].[PersonId]
LEFT JOIN [Person] AS [p1] ON [p0].[ParentId] = [p1].[PersonId]
Expand Down

0 comments on commit 83e06a3

Please sign in to comment.