[5.0.2] Handle concurrency tokens with conversions in the in-memory d…

…atabase

Fixes #23527
Also fixes #12436 (MQ issue to add more testing for ulong concurrency tokes)

The fix is to ensure the current value is converted to the model type before comparing.

src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs

@@ -237,6 +237,14 @@ private static bool IsConcurrencyConflict(
                 var comparer = property.GetKeyValueComparer();
                 var originalValue = entry.GetOriginalValue(property);
 
+                var converter = property.GetValueConverter()
+                    ?? property.FindTypeMapping()?.Converter;
+
+                if (converter != null)
+                {
+                    rowValue = converter.ConvertFromProvider(rowValue);
+                }
+
                 if ((comparer != null && !comparer.Equals(rowValue, originalValue))
                     || (comparer == null && !StructuralComparisons.StructuralEqualityComparer.Equals(rowValue, originalValue)))
                 {

test/EFCore.InMemory.FunctionalTests/F1InMemoryFixture.cs

@@ -1,16 +1,28 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.TestUtilities;
 
 namespace Microsoft.EntityFrameworkCore
 {
-    public class F1InMemoryFixture : F1FixtureBase
+    public class F1InMemoryFixture : F1InMemoryFixtureBase<byte[]>
+    {
+    }
+
+    public class F1ULongInMemoryFixture : F1InMemoryFixtureBase<ulong>
+    {
+    }
+
+    public abstract class F1InMemoryFixtureBase<TRowVersion> : F1FixtureBase<TRowVersion>
     {
         public override TestHelpers TestHelpers
             => InMemoryTestHelpers.Instance;
 
         protected override ITestStoreFactory TestStoreFactory
             => InMemoryTestStoreFactory.Instance;
+
+        public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+            => base.AddOptions(builder).ConfigureWarnings(e => e.Ignore(InMemoryEventId.TransactionIgnoredWarning));
     }
 }

test/EFCore.InMemory.FunctionalTests/InMemoryComplianceTest.cs

@@ -14,7 +14,6 @@ public class InMemoryComplianceTest : ComplianceTestBase
         {
             // No in-memory tests
             typeof(FunkyDataQueryTestBase<>),
-            typeof(OptimisticConcurrencyTestBase<>),
             typeof(StoreGeneratedTestBase<>),
             typeof(ConferencePlannerTestBase<>),
             typeof(ManyToManyQueryTestBase<>),

test/EFCore.InMemory.FunctionalTests/OptimisticConcurrencyInMemoryTest.cs

@@ -0,0 +1,92 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Threading.Tasks;
+using Xunit;
+
+namespace Microsoft.EntityFrameworkCore
+{
+    public class OptimisticConcurrencyULongInMemoryTest : OptimisticConcurrencyInMemoryTestBase<F1ULongInMemoryFixture, ulong>
+    {
+        public OptimisticConcurrencyULongInMemoryTest(F1ULongInMemoryFixture fixture)
+            : base(fixture)
+        {
+        }
+    }
+
+    public class OptimisticConcurrencyInMemoryTest : OptimisticConcurrencyInMemoryTestBase<F1InMemoryFixture, byte[]>
+    {
+        public OptimisticConcurrencyInMemoryTest(F1InMemoryFixture fixture)
+            : base(fixture)
+        {
+        }
+    }
+
+    public abstract class OptimisticConcurrencyInMemoryTestBase<TFixture, TRowVersion>
+        : OptimisticConcurrencyTestBase<TFixture, TRowVersion>
+        where TFixture : F1FixtureBase<TRowVersion>, new()
+    {
+        protected OptimisticConcurrencyInMemoryTestBase(TFixture fixture)
+            : base(fixture)
+        {
+        }
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task Simple_concurrency_exception_can_be_resolved_with_store_values()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task Simple_concurrency_exception_can_be_resolved_with_client_values()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task Simple_concurrency_exception_can_be_resolved_with_new_values()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task Simple_concurrency_exception_can_be_resolved_with_store_values_using_equivalent_of_accept_changes()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task Simple_concurrency_exception_can_be_resolved_with_store_values_using_Reload()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task Updating_then_deleting_the_same_entity_results_in_DbUpdateConcurrencyException()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task
+            Updating_then_deleting_the_same_entity_results_in_DbUpdateConcurrencyException_which_can_be_resolved_with_store_values()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task
+            Change_in_independent_association_after_change_in_different_concurrency_token_results_in_independent_association_exception()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task Change_in_independent_association_results_in_independent_association_exception()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task Two_concurrency_issues_in_one_to_many_related_entities_can_be_handled_by_dealing_with_dependent_first()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Optimistic Offline Lock #2195")]
+        public override Task Two_concurrency_issues_in_one_to_one_related_entities_can_be_handled_by_dealing_with_dependent_first()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Throw DbUpdateException or DbUpdateConcurrencyException for in-memory database errors #23569")]
+        public override Task Adding_the_same_entity_twice_results_in_DbUpdateException()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Throw DbUpdateException or DbUpdateConcurrencyException for in-memory database errors #23569")]
+        public override Task Deleting_the_same_entity_twice_results_in_DbUpdateConcurrencyException()
+            => Task.FromResult(true);
+
+        [ConditionalFact(Skip = "Throw DbUpdateException or DbUpdateConcurrencyException for in-memory database errors #23569")]
+        public override Task Deleting_then_updating_the_same_entity_results_in_DbUpdateConcurrencyException()
+            => Task.FromResult(true);
+    }
+}

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

@@ -7,7 +7,7 @@
 
 namespace Microsoft.EntityFrameworkCore
 {
-    public abstract class F1RelationalFixture : F1FixtureBase
+    public abstract class F1RelationalFixture<TRowVersion> : F1FixtureBase<TRowVersion>
     {
         public TestSqlLoggerFactory TestSqlLoggerFactory
             => (TestSqlLoggerFactory)ListLoggerFactory;

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

@@ -10,7 +10,7 @@
 namespace Microsoft.EntityFrameworkCore
 {
     public abstract class PropertyEntryTestBase<TFixture> : IClassFixture<TFixture>
-        where TFixture : F1FixtureBase, new()
+        where TFixture : F1FixtureBase<byte[]>, new()
     {
         protected PropertyEntryTestBase(TFixture fixture)
             => Fixture = fixture;

test/EFCore.Specification.Tests/DatabindingTestBase.cs

@@ -13,7 +13,7 @@
 namespace Microsoft.EntityFrameworkCore
 {
     public abstract class DatabindingTestBase<TFixture> : IClassFixture<TFixture>
-        where TFixture : F1FixtureBase, new()
+        where TFixture : F1FixtureBase<byte[]>, new()
     {
         protected DatabindingTestBase(TFixture fixture)
             => Fixture = fixture;

test/EFCore.Specification.Tests/F1FixtureBase.cs

@@ -8,7 +8,7 @@
 
 namespace Microsoft.EntityFrameworkCore
 {
-    public abstract class F1FixtureBase : SharedStoreFixtureBase<F1Context>
+    public abstract class F1FixtureBase<TRowVersion> : SharedStoreFixtureBase<F1Context>
     {
         protected override string StoreName { get; } = "F1Test";
 
@@ -90,6 +90,43 @@ protected virtual void BuildModelExternal(ModelBuilder modelBuilder)
                         .HasOne(t => t.Team)
                         .WithMany())
                 .HasKey(ts => new { ts.SponsorId, ts.TeamId });
+
+            modelBuilder.Entity<Chassis>().Property<TRowVersion>("Version").IsRowVersion();
+            modelBuilder.Entity<Driver>().Property<TRowVersion>("Version").IsRowVersion();
+
+            modelBuilder.Entity<Team>().Property<TRowVersion>("Version")
+                .ValueGeneratedOnAddOrUpdate()
+                .IsConcurrencyToken();
+
+            modelBuilder.Entity<Sponsor>(
+                eb =>
+                {
+                    eb.Property<TRowVersion>("Version").IsRowVersion();
+                    eb.Property<int?>(Sponsor.ClientTokenPropertyName);
+                });
+
+            modelBuilder.Entity<TitleSponsor>()
+                .OwnsOne(
+                    s => s.Details, eb =>
+                    {
+                        eb.Property(d => d.Space);
+                        eb.Property<TRowVersion>("Version").IsRowVersion();
+                        eb.Property<int?>(Sponsor.ClientTokenPropertyName).IsConcurrencyToken();
+                    });
+
+            if (typeof(TRowVersion) != typeof(byte[]))
+            {
+                modelBuilder.Entity<Chassis>().Property<TRowVersion>("Version").HasConversion<byte[]>();
+                modelBuilder.Entity<Driver>().Property<TRowVersion>("Version").HasConversion<byte[]>();
+                modelBuilder.Entity<Team>().Property<TRowVersion>("Version").HasConversion<byte[]>();
+                modelBuilder.Entity<Sponsor>().Property<TRowVersion>("Version").HasConversion<byte[]>();
+                modelBuilder.Entity<TitleSponsor>()
+                    .OwnsOne(
+                        s => s.Details, eb =>
+                        {
+                            eb.Property<TRowVersion>("Version").IsRowVersion().HasConversion<byte[]>();
+                        });
+            }
         }
 
         protected override void Seed(F1Context context)

test/EFCore.Specification.Tests/OptimisticConcurrencyTestBase.cs

@@ -16,8 +16,8 @@
 // ReSharper disable InconsistentNaming
 namespace Microsoft.EntityFrameworkCore
 {
-    public abstract class OptimisticConcurrencyTestBase<TFixture> : IClassFixture<TFixture>
-        where TFixture : F1FixtureBase, new()
+    public abstract class OptimisticConcurrencyTestBase<TFixture, TRowVersion> : IClassFixture<TFixture>
+        where TFixture : F1FixtureBase<TRowVersion>, new()
     {
         protected OptimisticConcurrencyTestBase(TFixture fixture)
         {

test/EFCore.Specification.Tests/SerializationTestBase.cs

@@ -13,7 +13,7 @@
 namespace Microsoft.EntityFrameworkCore
 {
     public abstract class SerializationTestBase<TFixture> : IClassFixture<TFixture>
-        where TFixture : F1FixtureBase, new()
+        where TFixture : F1FixtureBase<byte[]>, new()
     {
         protected SerializationTestBase(TFixture fixture)
             => Fixture = fixture;

test/EFCore.SqlServer.FunctionalTests/F1SqlServerFixture.cs

@@ -6,7 +6,15 @@
 
 namespace Microsoft.EntityFrameworkCore
 {
-    public class F1SqlServerFixture : F1RelationalFixture
+    public class F1ULongSqlServerFixture : F1SqlServerFixtureBase<ulong>
+    {
+    }
+
+    public class F1SqlServerFixture : F1SqlServerFixtureBase<byte[]>
+    {
+    }
+
+    public abstract class F1SqlServerFixtureBase<TRowVersion> : F1RelationalFixture<TRowVersion>
     {
         protected override ITestStoreFactory TestStoreFactory
             => SqlServerTestStoreFactory.Instance;
@@ -18,27 +26,11 @@ protected override void BuildModelExternal(ModelBuilder modelBuilder)
         {
             base.BuildModelExternal(modelBuilder);
 
-            modelBuilder.Entity<Chassis>().Property<byte[]>("Version").IsRowVersion();
-            modelBuilder.Entity<Driver>().Property<byte[]>("Version").IsRowVersion();
-
-            modelBuilder.Entity<Team>().Property<byte[]>("Version")
-                .ValueGeneratedOnAddOrUpdate()
-                .IsConcurrencyToken();
-
-            modelBuilder.Entity<Sponsor>(
-                eb =>
-                {
-                    eb.Property<byte[]>("Version").IsRowVersion().HasColumnName("Version");
-                    eb.Property<int?>(Sponsor.ClientTokenPropertyName).HasColumnName(Sponsor.ClientTokenPropertyName);
-                });
             modelBuilder.Entity<TitleSponsor>()
                 .OwnsOne(
                     s => s.Details, eb =>
                     {
                         eb.Property(d => d.Space).HasColumnType("decimal(18,2)");
-                        eb.Property<byte[]>("Version").IsRowVersion().HasColumnName("Version");
-                        eb.Property<int?>(Sponsor.ClientTokenPropertyName).IsConcurrencyToken()
-                            .HasColumnName(Sponsor.ClientTokenPropertyName);
                     });
         }
     }

test/EFCore.SqlServer.FunctionalTests/OptimisticConcurrencySqlServerTest.cs

@@ -11,12 +11,30 @@
 // ReSharper disable InconsistentNaming
 namespace Microsoft.EntityFrameworkCore
 {
-    public class OptimisticConcurrencySqlServerTest : OptimisticConcurrencyTestBase<F1SqlServerFixture>
+    public class OptimisticConcurrencyULongSqlServerTest : OptimisticConcurrencySqlServerTestBase<F1ULongSqlServerFixture, ulong>
+    {
+        public OptimisticConcurrencyULongSqlServerTest(F1ULongSqlServerFixture fixture)
+            : base(fixture)
+        {
+        }
+    }
+
+    public class OptimisticConcurrencySqlServerTest : OptimisticConcurrencySqlServerTestBase<F1SqlServerFixture, byte[]>
     {
         public OptimisticConcurrencySqlServerTest(F1SqlServerFixture fixture)
             : base(fixture)
         {
         }
+    }
+
+    public abstract class OptimisticConcurrencySqlServerTestBase<TFixture, TRowVersion>
+        : OptimisticConcurrencyTestBase<TFixture, TRowVersion>
+        where TFixture : F1FixtureBase<TRowVersion>, new()
+    {
+        protected OptimisticConcurrencySqlServerTestBase(TFixture fixture)
+            : base(fixture)
+        {
+        }
 
         [ConditionalFact]
         public async Task Modifying_concurrency_token_only_is_noop()
@@ -28,22 +46,22 @@ await c.Database.CreateExecutionStrategy().ExecuteAsync(
                     using var transaction = context.Database.BeginTransaction();
                     var driver = context.Drivers.Single(d => d.CarNumber == 1);
                     driver.Podiums = StorePodiums;
-                    var firstVersion = context.Entry(driver).Property<byte[]>("Version").CurrentValue;
+                    var firstVersion = context.Entry(driver).Property<TRowVersion>("Version").CurrentValue;
                     await context.SaveChangesAsync();
 
                     using var innerContext = CreateF1Context();
                     innerContext.Database.UseTransaction(transaction.GetDbTransaction());
                     driver = innerContext.Drivers.Single(d => d.CarNumber == 1);
-                    Assert.NotEqual(firstVersion, innerContext.Entry(driver).Property<byte[]>("Version").CurrentValue);
+                    Assert.NotEqual(firstVersion, innerContext.Entry(driver).Property<TRowVersion>("Version").CurrentValue);
                     Assert.Equal(StorePodiums, driver.Podiums);
 
-                    var secondVersion = innerContext.Entry(driver).Property<byte[]>("Version").CurrentValue;
-                    innerContext.Entry(driver).Property<byte[]>("Version").CurrentValue = firstVersion;
+                    var secondVersion = innerContext.Entry(driver).Property<TRowVersion>("Version").CurrentValue;
+                    innerContext.Entry(driver).Property<TRowVersion>("Version").CurrentValue = firstVersion;
                     await innerContext.SaveChangesAsync();
                     using var validationContext = CreateF1Context();
                     validationContext.Database.UseTransaction(transaction.GetDbTransaction());
                     driver = validationContext.Drivers.Single(d => d.CarNumber == 1);
-                    Assert.Equal(secondVersion, validationContext.Entry(driver).Property<byte[]>("Version").CurrentValue);
+                    Assert.Equal(secondVersion, validationContext.Entry(driver).Property<TRowVersion>("Version").CurrentValue);
                     Assert.Equal(StorePodiums, driver.Podiums);
                 });
         }
@@ -59,8 +77,8 @@ await c.Database.CreateExecutionStrategy().ExecuteAsync(
                     var sponsor = context.Set<TitleSponsor>().Single();
                     var sponsorEntry = c.Entry(sponsor);
                     var detailsEntry = sponsorEntry.Reference(s => s.Details).TargetEntry;
-                    var sponsorVersion = sponsorEntry.Property<byte[]>("Version").CurrentValue;
-                    var detailsVersion = detailsEntry.Property<byte[]>("Version").CurrentValue;
+                    var sponsorVersion = sponsorEntry.Property<TRowVersion>("Version").CurrentValue;
+                    var detailsVersion = detailsEntry.Property<TRowVersion>("Version").CurrentValue;
 
                     Assert.Null(sponsorEntry.Property<int?>(Sponsor.ClientTokenPropertyName).CurrentValue);
                     sponsorEntry.Property<int?>(Sponsor.ClientTokenPropertyName).CurrentValue = 1;
@@ -71,8 +89,8 @@ await c.Database.CreateExecutionStrategy().ExecuteAsync(
 
                     await context.SaveChangesAsync();
 
-                    var newSponsorVersion = sponsorEntry.Property<byte[]>("Version").CurrentValue;
-                    var newDetailsVersion = detailsEntry.Property<byte[]>("Version").CurrentValue;
+                    var newSponsorVersion = sponsorEntry.Property<TRowVersion>("Version").CurrentValue;
+                    var newDetailsVersion = detailsEntry.Property<TRowVersion>("Version").CurrentValue;
 
                     Assert.Equal(newSponsorVersion, newDetailsVersion);
                     Assert.NotEqual(sponsorVersion, newSponsorVersion);
@@ -92,7 +110,7 @@ await c.Database.CreateExecutionStrategy().ExecuteAsync(
                     using var transaction = context.Database.BeginTransaction();
                     var sponsor = context.Set<TitleSponsor>().Single();
                     var sponsorEntry = c.Entry(sponsor);
-                    var sponsorVersion = sponsorEntry.Property<byte[]>("Version").CurrentValue;
+                    var sponsorVersion = sponsorEntry.Property<TRowVersion>("Version").CurrentValue;
 
                     Assert.Null(sponsorEntry.Property<int?>(Sponsor.ClientTokenPropertyName).CurrentValue);
                     sponsorEntry.Property<int?>(Sponsor.ClientTokenPropertyName).CurrentValue = 1;
@@ -106,8 +124,8 @@ await c.Database.CreateExecutionStrategy().ExecuteAsync(
 
                     await context.SaveChangesAsync();
 
-                    var newSponsorVersion = sponsorEntry.Property<byte[]>("Version").CurrentValue;
-                    var newDetailsVersion = detailsEntry.Property<byte[]>("Version").CurrentValue;
+                    var newSponsorVersion = sponsorEntry.Property<TRowVersion>("Version").CurrentValue;
+                    var newDetailsVersion = detailsEntry.Property<TRowVersion>("Version").CurrentValue;
 
                     Assert.Equal(newSponsorVersion, newDetailsVersion);
                     Assert.NotEqual(sponsorVersion, newSponsorVersion);