Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EfCore 5 update breaks behaviour of RowVersion with Conversions for InMemory database #23527

Closed
dg-marbl opened this issue Nov 30, 2020 · 1 comment
Labels
area-change-tracking closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Milestone

Comments

@dg-marbl
Copy link

Problem

There seems to be a new issue when upgrading from EfCore 3.x to 5.0 and using an Entity that contains a Property that uses a ValueConverter as well as a RowVersion. This issue affects the InMemory database provider.

Example

The following Code works in EfCore 3.1.10 but not in EfCore 5.0: full example

var serviceCollection = new ServiceCollection();
serviceCollection.AddDbContext<TestDbContext>(builder => CreateInMemoryDbContextOptions(builder));

var serviceProvider = serviceCollection.BuildServiceProvider();
var context = serviceProvider.GetRequiredService<TestDbContext>();
            
var dbModel = new DbModel{Value = "Test"};
context.Add(dbModel);
context.SaveChanges();

var model = context.Set<DbModel>().Single();
model.Value += "2";
context.SaveChanges();
            
var models = context.Set<DbModel>().ToImmutableList();
Console.WriteLine($"Value is {model.Value}");

where the model is

    public class DbModel
    {
        public long Id { get; set; }

        public ulong RowVersion { get; set; }
        
        public string Value { get; set; }
    }
    
    public class DbModelEntityTypeConfiguration : IEntityTypeConfiguration<DbModel>
    {
        public void Configure(EntityTypeBuilder<DbModel> builder)
        {
            builder.ToTable(name: nameof(DbModel), schema: "test");
            builder.Property(po => po.RowVersion).HasConversion(new NumberToBytesConverter<ulong>()).IsRowVersion();
        }
    }

Stacktrace

Unhandled exception. System.InvalidCastException: Unable to cast object of type 'System.Byte[]' to type 'System.UInt64'.
   at Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer`1.Equals(Object left, Object right)
   at Microsoft.EntityFrameworkCore.InMemory.Storage.Internal.InMemoryTable`1.IsConcurrencyConflict(IUpdateEntry entry, IProperty property, Object rowValue, Dictionary`2 concurrencyConflicts)
   at Microsoft.EntityFrameworkCore.InMemory.Storage.Internal.InMemoryTable`1.Update(IUpdateEntry entry)
   at Microsoft.EntityFrameworkCore.InMemory.Storage.Internal.InMemoryStore.ExecuteTransaction(IList`1 entries, IDiagnosticsLogger`1 updateLogger)
   at Microsoft.EntityFrameworkCore.InMemory.Storage.Internal.InMemoryDatabase.SaveChanges(IList`1 entries)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(DbContext _, Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.Storage.NonRetryingExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()

Provider and version information

EF Core version:
Database provider: Microsoft.EntityFrameworkCore.InMemory 5.0.0 (works with 3.1.10)
Target framework: NET 5.0
Operating system: Windows 10

@ajcvickers
Copy link
Contributor

ajcvickers commented Nov 30, 2020

Note for triage: the optimistic concurrency code in the in-memory provider assumes no value conversion. This was not an issue before 5.0 because value conversions were ignored in the in-memory database.

public class DbModel
{
    public long Id { get; set; }

    public ulong RowVersion { get; set; }
        
    public string Value { get; set; }
}

public class SomeDbContext : DbContext
{
    private static ILoggerFactory ContextLoggerFactory
        => LoggerFactory.Create(b => b.AddConsole().SetMinimumLevel(LogLevel.Information));

    public DbSet<DbModel> DbModels { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            //.UseSqlite("Data Source=test.db")
            //.UseSqlServer(Your.ConnectionString)
            .UseInMemoryDatabase("Test")
            .UseLoggerFactory(ContextLoggerFactory)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<DbModel>(builder =>
        {
            builder.ToTable(name: nameof(DbModel), schema: "test");
            builder.Property(po => po.RowVersion).HasConversion(new NumberToBytesConverter<ulong>()).IsRowVersion();
        });
    }
}

public class Program
{
    public static void Main()
    {
        using (var context = new SomeDbContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();
            
            var dbModel = new DbModel{Value = "Test"};
            context.Add(dbModel);
            context.SaveChanges();

            var model = context.Set<DbModel>().Single();
            model.Value += "2";
            context.SaveChanges();
            
            var models = context.Set<DbModel>().ToList();
            Console.WriteLine($"Value is {model.Value}");
            
            context.SaveChanges();
        }
    }
}

@ajcvickers ajcvickers added this to the 5.0.2 milestone Nov 30, 2020
@ajcvickers ajcvickers self-assigned this Nov 30, 2020
ajcvickers added a commit that referenced this issue Dec 3, 2020
…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.
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. Servicing-approved and removed Servicing-consider labels Dec 3, 2020
ajcvickers added a commit that referenced this issue Dec 10, 2020
…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.
ajcvickers added a commit that referenced this issue Dec 10, 2020
…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.
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-change-tracking closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Projects
None yet
Development

No branches or pull requests

3 participants