Efcore: Key with default value are not generated with EF Core 3.1

Created on 11 Jan 2020  路  8Comments  路  Source: dotnet/efcore

Before EF Core 3, with ValueGeneratedOnAdd, when a Key had a CLR default value, it was automatically generated. If a Key had a different value than default, this value was used.

This is the documented behavior, which is not up to date with EF Core 3 in: value-generated-on-add

In the breaking changes of EF Core 3, it is documented that you have to use ValueGeneratedNever to work with old behavior and insert keys with explicit value: detectchanges-honors-store-generated-key-values

The problem is that I can't have the old behavior with ValueGeneratedNever and generate a Key when it has a default value and use the one provided if it's different from the default value.

Is it possible to generate a value when the Key has a default value?

Steps to reproduce

````cs
public class ValueGeneratedTests
{
[Fact]
public void AddOnValueGeneratedOnAddContext()
{
var options = new DbContextOptionsBuilder()
.UseInMemoryDatabase(databaseName: "InMemoryTest")
.Options;

    using var context = new ValueGeneratedOnAddContext(options);
    context.SimpleModels.Add(new SimpleModel
    {
        Id = Guid.NewGuid(),
        SimpleDepModels =
        {
            new SimpleDepModel {Id = default},
            new SimpleDepModel {Id = default},
            new SimpleDepModel {Id = Guid.NewGuid()}
        }
    });
    context.SaveChanges();

    var model = context.SimpleModels.Single();
    // EF Core 3 with ValueGeneratedOnAdd => ERROR Microsoft.EntityFrameworkCore.DbUpdateConcurrencyException : Attempted to update or delete an entity that does not exist in the store.
    // EF Core 2 with ValueGeneratedOnAdd => SUCCESS defaultValue is Generated, notDefaultValue is not Generated
    var defaultValue = default(Guid);
    var notDefaultValue = Guid.NewGuid();
    model.SimpleDepModels.Add(new SimpleDepModel { Id = defaultValue });
    model.SimpleDepModels.Add(new SimpleDepModel { Id = defaultValue });
    model.SimpleDepModels.Add(new SimpleDepModel { Id = notDefaultValue });
    context.SaveChanges();

    // Assert
    Assert.Contains(context.SimpleDepModels.ToList(), m => m.Id == notDefaultValue);
    Assert.DoesNotContain(context.SimpleDepModels.ToList(), m => m.Id == defaultValue);
}

[Fact]
public void AddOnValueGeneratedNeverContext()
{
    var options = new DbContextOptionsBuilder<TestContext>()
        .UseInMemoryDatabase(databaseName: "InMemoryTest")
        .Options;

    using var context = new ValueGeneratedNeverContext(options);
    // EF Core 2 & 3 with ValueGeneratedNever => ERROR System.InvalidOperationException : The instance of entity type 'SimpleDepModel' cannot be tracked because another instance with the same key value for {'Id'} is already being tracked.
    // => default Guid value is considered duplicate Key
    context.SimpleModels.Add(new SimpleModel
    {
        Id = Guid.NewGuid(),
        SimpleDepModels =
        {
            new SimpleDepModel {Id = default},
            new SimpleDepModel {Id = default},
            new SimpleDepModel {Id = Guid.NewGuid()}
        }
    });
    context.SaveChanges();

    var model = context.SimpleModels.Single();
    var defaultValue = default(Guid);
    var notDefaultValue = Guid.NewGuid();
    model.SimpleDepModels.Add(new SimpleDepModel { Id = defaultValue });
    model.SimpleDepModels.Add(new SimpleDepModel { Id = defaultValue });
    model.SimpleDepModels.Add(new SimpleDepModel { Id = notDefaultValue });
    context.SaveChanges();

    // Assert
    Assert.Contains(context.SimpleDepModels.ToList(), m => m.Id == notDefaultValue);
    Assert.DoesNotContain(context.SimpleDepModels.ToList(), m => m.Id == defaultValue);
}

}

public class TestContext : DbContext
{
public TestContext(DbContextOptions options)
: base(options)
{

}

public DbSet<SimpleModel> SimpleModels { get; set; }
public DbSet<SimpleDepModel> SimpleDepModels { get; set; }

}

public class ValueGeneratedOnAddContext : TestContext
{
public ValueGeneratedOnAddContext(DbContextOptions options) : base(options)
{

}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    base.OnModelCreating(modelBuilder);

    modelBuilder.Entity<SimpleModel>(builder =>
    {
        builder.Property(e => e.Id).ValueGeneratedOnAdd();
    });
    modelBuilder.Entity<SimpleDepModel>(builder =>
    {
        builder.Property(e => e.Id).ValueGeneratedOnAdd();
    });
}

}

public class ValueGeneratedNeverContext : TestContext
{
public ValueGeneratedNeverContext(DbContextOptions options) : base(options)
{

}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    base.OnModelCreating(modelBuilder);

    modelBuilder.Entity<SimpleModel>(builder =>
    {
        builder.Property(e => e.Id).ValueGeneratedNever();
    });
    modelBuilder.Entity<SimpleDepModel>(builder =>
    {
        builder.Property(e => e.Id).ValueGeneratedNever();
    });
}

}

public class SimpleModel
{
public Guid Id { get; set; }

public ICollection<SimpleDepModel> SimpleDepModels { get; set; } = new HashSet<SimpleDepModel>();

}

public class SimpleDepModel
{
public Guid Id { get; set; }
public Guid SimpleModelId { get; set; }
}
````

Further technical details

EF Core version: 3.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.1
Operating system: Windows 10.0.18362
IDE: Visual Studio 2019 16.4.2

closed-by-design customer-reported

Most helpful comment

This is perfectly fine as ValueGeneratedNever refers to value generation in the store. By convention we provide a ValueGenerator for key properties that are store-generated, but one specified explicitly will always be preferred.

All 8 comments

@YZahringer The change in behavior here is due to the breaking change in 3.0 that changes the way changes are detected when using generated keys.

Let me explain what is going on in the your examples. Consider this code with ValueGeneratedOnAdd:

```C#
model.SimpleDepModels.Add(new SimpleDepModel { Id = defaultValue });
model.SimpleDepModels.Add(new SimpleDepModel { Id = defaultValue });
model.SimpleDepModels.Add(new SimpleDepModel { Id = notDefaultValue });

    context.SaveChanges();
SaveChanges calls DetectChanges to look for changes in the graph of tracked entities. It finds three entities that were not present in the graph before. Since the key is configured as generated, in means the ones with default values are assumed to be new (since they don't have a key) and the one that already has a key value set is assumed to be existing.

You could instead be explicit about calling Add on the context/set:

```C#
context.Add(new SimpleDepModel { Id = notDefaultValue });

or set the state explicitly using context.Entry, in which case the entities will be in the Added state and will get a generated GUID unless one was supplied, just as in previous versions.

See #17747 for some more discussion.

@ajcvickers Thank you for the answer and the explanations.

I understand the new behavior of EF Core 3. The problem is that I am trying to have the same behavior that was supported by EF Core 2. It is documented that I have to use ValueGeneratedNever, but it is not sufficient to reproduce the old behavior was supported.

I've been trying to migrate a large EF Core 2 project to EF Core 3 for several days now. I've had several bugs/regressions that can be fixed temporarily with a workaround and that are already tracked and fixed with EF Core 5, awesome.

Now I'm blocking on this one, which is a breaking change, but I can't get the old supported behavior. I'd like to avoid changing all the code in the project, it's a large project in production with almost a hundred entities and a lot of business logic.

There is no workaround to keep EF Core 2 behavior?

@YZahringer There isn't a built-in way to get back to exactly the 2.x behavior. I will investigate what workarounds might be suitable for your case.

@ajcvickers I continued to investigate. I found two solutions:

  • Subscribe to ChangeTracker.Tracked event and generate new Guid if the current one has the default value.
  • Create a ValueGenerator that generates new Guid if the current one has the default value.

Both solutions works, I prefer the ValueGenerator which is more focused.

Here is the ValueGenerator that I use. Do you think this is a valid solution? No performance issues?
Do you have another idea?
````cs
public class GuidOnDefaultValueGenerator : ValueGenerator
{
private IProperty Property { get; }
private IEntityType EntityType { get; }

public GuidOnDefaultValueGenerator(IProperty property, IEntityType entityType)
{
    Property = property ?? throw new ArgumentNullException(nameof(property));
    EntityType = entityType ?? throw new ArgumentNullException(nameof(entityType));
}

public override Guid Next(EntityEntry entry)
{
    if (entry.Metadata == EntityType)
    {
        var value = entry.CurrentValues.GetValue<Guid>(Property);
        return value == default ? Guid.NewGuid() : value;
    }

    // Return current value if EntityType is not same (e.g. when is ForeignKey property)
    return entry.CurrentValues.GetValue<Guid>(Property.Name);
}

public override bool GeneratesTemporaryValues => false;

}

public static class PropertyBuilderExtensions
{
public static PropertyBuilder ValueGeneratedWhenDefault(this PropertyBuilder propertyBuilder)
{
propertyBuilder.ValueGeneratedNever();
propertyBuilder.HasValueGenerator((property, type) => new GuidOnDefaultValueGenerator(property, type));
return propertyBuilder;
}
}
````

@YZahringer When I first read your solution I didn't expect it to work! But it does because the value is not treated as a generated key anymore (ValueGeneratedNever) and yet it still has a value generator associated with it so it gets a generated value like a non-key property would. After having thought about it a bit I think this is probably okay, but I'd like to discuss with the team.

@AndriySvyryd Does this solution look like something that might cause problems?

This is perfectly fine as ValueGeneratedNever refers to value generation in the store. By convention we provide a ValueGenerator for key properties that are store-generated, but one specified explicitly will always be preferred.

@ajcvickers I'm experiencing the same issue with an autogenerated int identity.

@jklaus did you find a solution for autogenerated int identity? Thanks

Was this page helpful?
0 / 5 - 0 ratings