Convert to provider values when comparing shared columns

Fixes #28763

src/EFCore.Relational/Update/ColumnModification.cs

@@ -191,7 +191,9 @@ public virtual void AddSharedColumnModification(IColumnModification modification
         _sharedColumnModifications ??= new List<IColumnModification>();
 
         if (UseCurrentValueParameter
-            && !modification.Property.GetValueComparer().Equals(Value, modification.Value))
+            && !Property.GetProviderValueComparer().Equals(
+                Entry.GetCurrentProviderValue(Property),
+                modification.Entry.GetCurrentProviderValue(modification.Property)))
         {
             if (_sensitiveLoggingEnabled)
             {
@@ -215,13 +217,30 @@ public virtual void AddSharedColumnModification(IColumnModification modification
         }
 
         if (UseOriginalValueParameter
-            && !modification.Property.GetValueComparer().Equals(OriginalValue, modification.OriginalValue))
+            && !Property.GetProviderValueComparer().Equals(
+                Entry.SharedIdentityEntry == null
+                    ? Entry.GetOriginalProviderValue(Property)
+                    : Entry.SharedIdentityEntry.GetOriginalProviderValue(Property),
+                modification.Entry.SharedIdentityEntry == null
+                    ? modification.Entry.GetOriginalProviderValue(modification.Property)
+                    : modification.Entry.SharedIdentityEntry.GetOriginalProviderValue(modification.Property)))
         {
             if (Entry.EntityState == EntityState.Modified
                 && modification.Entry.EntityState == EntityState.Added
                 && modification.Entry.SharedIdentityEntry == null)
             {
-                modification.Entry.SetOriginalValue(modification.Property, OriginalValue);
+                var originalValue = Entry.SharedIdentityEntry == null
+                    ? Entry.GetOriginalProviderValue(Property)
+                    : Entry.SharedIdentityEntry.GetOriginalProviderValue(Property);
+
+                var typeMapping = modification.Property.GetTypeMapping();
+                var converter = typeMapping.Converter;
+                if (converter != null)
+                {
+                    originalValue = converter.ConvertFromProvider(originalValue);
+                }
+
+                modification.Entry.SetOriginalValue(modification.Property, originalValue);
             }
             else
             {

src/EFCore.Relational/Update/ModificationCommand.cs

@@ -976,7 +976,7 @@ public bool TryPropagate(IColumnMappingBase mapping, IUpdateEntry entry)
                 && (entry.EntityState == EntityState.Unchanged
                     || (entry.EntityState == EntityState.Modified && !entry.IsModified(property))
                     || (entry.EntityState == EntityState.Added
-                        && mapping.Column.ProviderValueComparer.Equals(_originalValue, entry.GetCurrentValue(property)))))
+                        && mapping.Column.ProviderValueComparer.Equals(_originalValue, entry.GetCurrentProviderValue(property)))))
             {
                 if (property.GetAfterSaveBehavior() == PropertySaveBehavior.Save
                     || entry.EntityState == EntityState.Added)

test/EFCore.Relational.Specification.Tests/UpdatesRelationalTestBase.cs

@@ -174,6 +174,15 @@ 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<
                     LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWorkingCorrectlyDetails

test/EFCore.Specification.Tests/SharedStoreFixtureBase.cs

@@ -89,16 +89,16 @@ protected virtual bool ShouldLogCategory(string logCategory)
     public virtual void Reseed()
     {
         using var context = CreateContext();
-        Clean(context);
         TestStore.Clean(context);
+        Clean(context);
         Seed(context);
     }
 
     public virtual async Task ReseedAsync()
     {
         using var context = CreateContext();
-        await CleanAsync(context);
         await TestStore.CleanAsync(context);
+        await CleanAsync(context);
         await SeedAsync(context);
     }
 

test/EFCore.Specification.Tests/TestModels/UpdatesModel/Address.cs

@@ -0,0 +1,12 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+# nullable enable
+
+namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel;
+
+public class Address
+{
+    public string City { get; set; } = null!;
+    public Country Country { get; set; }
+}

test/EFCore.Specification.Tests/TestModels/UpdatesModel/Country.cs

@@ -0,0 +1,15 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+# nullable enable
+
+namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel;
+
+public enum Country
+{
+    Türkiye,
+    Czechia,
+    Montenegro,
+    Serbia,
+    Eswatini
+}

test/EFCore.Specification.Tests/TestModels/UpdatesModel/Person.cs

@@ -21,5 +21,7 @@ public Person(string name, Person? parent)
     public int PersonId { get; set; }
     public string Name { get; set; }
     public int? ParentId { get; set; }
+    public string? Country { get; set; }
     public Person? Parent { get; set; }
+    public Address? Address { get; set; }
 }

test/EFCore.Specification.Tests/UpdatesTestBase.cs

@@ -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.
 
 # nullable enable
@@ -371,6 +371,34 @@ public virtual void Can_add_and_remove_self_refs()
                 Assert.Equal("1", people.Single(p => p.Parent == null).Name);
             });
 
+    [ConditionalFact]
+    public virtual void Can_change_enums_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" };
+                person.Country = "Türkiye";
+
+                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("Türkiye", person.Country);
+            });
+
     [ConditionalFact]
     public virtual void Can_remove_partial()
     {
@@ -621,6 +649,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                 .WithMany()
                 .OnDelete(DeleteBehavior.Restrict);
 
+            modelBuilder.Entity<Person>()
+                .OwnsOne(p => p.Address)
+                .Property(p => p.Country)
+                .HasConversion<string>();
+
             modelBuilder.Entity<Category>().HasMany(e => e.ProductCategories).WithOne(e => e.Category)
                 .HasForeignKey(e => e.CategoryId);
 

test/EFCore.SqlServer.FunctionalTests/TestUtilities/SqlServerTestStore.cs

@@ -130,8 +130,8 @@ private bool CreateDatabase(Action<DbContext> clean)
                                 new DbContextOptionsBuilder()
                                     .EnableServiceProviderCaching(false))
                             .Options);
-                    clean?.Invoke(context);
                     Clean(context);
+                    clean?.Invoke(context);
                     return true;
                 }
 

test/EFCore.SqlServer.FunctionalTests/UpdatesSqlServerTPCTest.cs

@@ -2,23 +2,19 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Microsoft.EntityFrameworkCore.TestModels.UpdatesModel;
-using static Microsoft.EntityFrameworkCore.UpdatesSqlServerTPCTest;
 
 #nullable enable
 
 namespace Microsoft.EntityFrameworkCore;
 
-public class UpdatesSqlServerTPCTest : UpdatesRelationalTestBase<UpdatesSqlServerTPTFixture>
+public class UpdatesSqlServerTPCTest : UpdatesSqlServerTestBase<UpdatesSqlServerTPCTest.UpdatesSqlServerTPCFixture>
 {
     // ReSharper disable once UnusedParameter.Local
-    public UpdatesSqlServerTPCTest(UpdatesSqlServerTPTFixture fixture, ITestOutputHelper testOutputHelper)
-        : base(fixture)
+    public UpdatesSqlServerTPCTest(UpdatesSqlServerTPCFixture fixture, ITestOutputHelper testOutputHelper)
+        : base(fixture, testOutputHelper)
     {
-        //Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
-        Fixture.TestSqlLoggerFactory.Clear();
     }
 
-    [ConditionalFact]
     public override void Save_with_shared_foreign_key()
     {
         base.Save_with_shared_foreign_key();
@@ -43,58 +39,6 @@ OUTPUT INSERTED.[Id]
 VALUES (@p0, @p1);");
     }
 
-    [ConditionalFact]
-    public override void Can_add_and_remove_self_refs()
-    {
-        base.Can_add_and_remove_self_refs();
-
-        AssertContainsSql(
-            @"@p0='1' (Nullable = false) (Size = 4000)
-@p1=NULL (DbType = Int32)
-
-SET IMPLICIT_TRANSACTIONS OFF;
-SET NOCOUNT ON;
-INSERT INTO [Person] ([Name], [ParentId])
-OUTPUT INSERTED.[PersonId]
-VALUES (@p0, @p1);",
-            //
-            @"@p2='2' (Nullable = false) (Size = 4000)
-@p3='1' (Nullable = true)
-@p4='3' (Nullable = false) (Size = 4000)
-@p5='1' (Nullable = true)
-
-SET IMPLICIT_TRANSACTIONS OFF;
-SET NOCOUNT ON;
-MERGE [Person] USING (
-VALUES (@p2, @p3, 0),
-(@p4, @p5, 1)) AS i ([Name], [ParentId], _Position) ON 1=0
-WHEN NOT MATCHED THEN
-INSERT ([Name], [ParentId])
-VALUES (i.[Name], i.[ParentId])
-OUTPUT INSERTED.[PersonId], i._Position;",
-            //
-            @"@p6='4' (Nullable = false) (Size = 4000)
-@p7='2' (Nullable = true)
-@p8='5' (Nullable = false) (Size = 4000)
-@p9='2' (Nullable = true)
-@p10='6' (Nullable = false) (Size = 4000)
-@p11='3' (Nullable = true)
-@p12='7' (Nullable = false) (Size = 4000)
-@p13='3' (Nullable = true)
-
-SET IMPLICIT_TRANSACTIONS OFF;
-SET NOCOUNT ON;
-MERGE [Person] USING (
-VALUES (@p6, @p7, 0),
-(@p8, @p9, 1),
-(@p10, @p11, 2),
-(@p12, @p13, 3)) AS i ([Name], [ParentId], _Position) ON 1=0
-WHEN NOT MATCHED THEN
-INSERT ([Name], [ParentId])
-VALUES (i.[Name], i.[ParentId])
-OUTPUT INSERTED.[PersonId], i._Position;");
-    }
-
     public override void Save_replaced_principal()
     {
         base.Save_replaced_principal();
@@ -140,80 +84,22 @@ FROM [ProductBase] AS [p]
 WHERE [p].[Discriminator] = N'Product' AND [p].[DependentId] = @__category_PrincipalId_0");
     }
 
-    public override void Identifiers_are_generated_correctly()
-    {
-        using var context = CreateContext();
-        var entityType = context.Model.FindEntityType(
-            typeof(
-                LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWorkingCorrectly
-            ))!;
-        Assert.Equal(
-            "LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWorking~",
-            entityType.GetTableName());
-        Assert.Equal(
-            "PK_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWork~",
-            entityType.GetKeys().Single().GetName());
-        Assert.Equal(
-            "FK_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWork~",
-            entityType.GetForeignKeys().Single().GetConstraintName());
-        Assert.Equal(
-            "IX_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWork~",
-            entityType.GetIndexes().Single().GetDatabaseName());
-
-        var entityType2 = context.Model.FindEntityType(
-            typeof(
-                LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWorkingCorrectlyDetails
-            ))!;
-
-        Assert.Equal(
-            "LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWorkin~1",
-            entityType2.GetTableName());
-        Assert.Equal(
-            "PK_LoginDetails",
-            entityType2.GetKeys().Single().GetName());
-        Assert.Equal(
-            "ExtraPropertyWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWorkingCo~",
-            entityType2.GetProperties().ElementAt(1).GetColumnName(StoreObjectIdentifier.Table(entityType2.GetTableName()!)));
-        Assert.Equal(
-            "ExtraPropertyWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWorkingC~1",
-            entityType2.GetProperties().ElementAt(2).GetColumnName(StoreObjectIdentifier.Table(entityType2.GetTableName()!)));
-        Assert.Equal(
-            "IX_LoginEntityTypeWithAnExtremelyLongAndOverlyConvolutedNameThatIsUsedToVerifyThatTheStoreIdentifierGenerationLengthLimitIsWork~",
-            entityType2.GetIndexes().Single().GetDatabaseName());
-    }
-
-    private void AssertSql(params string[] expected)
-        => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
-
-    protected void AssertContainsSql(params string[] expected)
-        => Fixture.TestSqlLoggerFactory.AssertBaseline(expected, assertOrder: false);
-
-    public class UpdatesSqlServerTPTFixture : UpdatesRelationalFixture
+    public class UpdatesSqlServerTPCFixture : UpdatesSqlServerFixtureBase
     {
-        protected override ITestStoreFactory TestStoreFactory
-            => SqlServerTestStoreFactory.Instance;
-
         protected override string StoreName
             => "UpdateTestTPC";
 
         public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
             => base.AddOptions(builder).ConfigureWarnings(
                 w =>
                 {
-                    w.Log(SqlServerEventId.DecimalTypeKeyWarning);
                     w.Log(RelationalEventId.ForeignKeyTpcPrincipalWarning);
                 });
 
-        protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
-            => configurationBuilder.Properties<decimal>().HaveColumnType("decimal(18, 2)");
-
         protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
         {
             base.OnModelCreating(modelBuilder, context);
 
-            modelBuilder.Entity<ProductBase>()
-                .Property(p => p.Id).HasDefaultValueSql("NEWID()");
-
             modelBuilder.Entity<Category>()
                 .UseTpcMappingStrategy();
         }