dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.81k stars 3.2k forks source link

Adding multiple new entities at the same time results in System.InvalidOperationException entity cannot be tracked because another instance with the same key value is already being tracked #13937

Closed ben-allamx closed 2 years ago

ben-allamx commented 6 years ago

We are trying to migrate from EF6 to EF Core 2.2

The issue comes with the Primary Key field ID. We do not allow SQL to generate the ID, we also do not generate the ID in the model. We generate the ID during the SaveChanges call which we have overridden which injects the next ID we want to use at that time.

When creating multiple new entity objects and adding them to a context an exception is thrown:

System.InvalidOperationException
  HResult=0x80131509
  Message=The instance of entity type 'Role' cannot be tracked because another instance with the same key value for {'ID'} is already being tracked. When attaching existing entities, ensure that only one entity instance with a given key value is attached. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the conflicting key values.
  Source=Microsoft.EntityFrameworkCore
  StackTrace:
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.ThrowIdentityConflict(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(TKey key, InternalEntityEntry entry, Boolean updateDuplicate)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(TKey key, InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.StartTracking(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState oldState, EntityState newState, Boolean acceptChanges)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Nullable`1 forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode node, Boolean force)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode node, TState state, Func`3 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraph(InternalEntityEntry rootEntry, EntityState entityState, Boolean forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.DbContext.SetEntityState(InternalEntityEntry entry, EntityState entityState)
   at Microsoft.EntityFrameworkCore.DbContext.SetEntityStates(IEnumerable`1 entities, EntityState entityState)
   at Microsoft.EntityFrameworkCore.DbContext.AddRange(IEnumerable`1 entities)
   at Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1.AddRange(IEnumerable`1 entities)
   at ConsoleApp1.Program.Main(String[] args) in C:\Playground\EFCoreTest\EFCoreTest2\ConsoleApp1\Program.cs:line 22

C#

Main Method:

static void Main(string[] args)
{
    List<Role> newRoles = new List<Role>();
    for (Int32 i = 1; i < 5; i++)
    {
        newRoles.Add(new Role { Name = $"New Role {i}" });
    }
    using (var ctx = new MyContext())
    {
        ctx.Roles.AddRange(newRoles);
    }
}

Model:

public class Role
{
    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.None)]
    [Display(Name = "Role ID")]
    public Int32 ID { get; set; }

    [DefaultValue(null)]
    [StringLength(100)]  //AD Spec (see User table)
    [Display(Name = "Role Name")]
    public String Name { get; set; }
}

DbContext Code highlights:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Role>().ToTable("Role");
    modelBuilder.Entity<Role>()
        .HasIndex(x => x.Name)
        .HasName("UX_Role_Name")
        .IsUnique(true);
    modelBuilder.Entity<Role>().Property(x => x.Name).IsUnicode();
}
public override Int32 SaveChanges()
{
    var dict = new Dictionary<Type, Int32>();
    foreach (var change in ChangeTracker.Entries())
    {
        if (change.State == EntityState.Added)
        {
            PropertyEntry property = null;
            try { property = change.Property("ID"); }
            catch (ArgumentException) { }  //if the ID property doesn't exist, then there's nothing to do
            if (property != null)
            {
                var id = (Int32)change.Property("ID").CurrentValue;
                if (id == 0)
                {
                    var type = change.Entity.GetType();
                    if (dict.ContainsKey(type))
                    {
                        dict[type]++;
                        id = dict[type];
                    }
                    else
                    {
                        id = GetNextID(type.Name);
                        dict.Add(type, id);
                    }
                    if (id > 0)
                        change.Property("ID").CurrentValue = id;
                }
            }
        }
    }

    var savedEntries = 0;
    try { savedEntries = base.SaveChanges(); }
    catch (DbUpdateException) {}
    catch (Exception) {}
    return savedEntries;
}
private Int32 GetNextID(String tableName)
{
    Int32 nextID = -1;
    var sqlStr = $"SELECT COALESCE(MAX([ID]), 0)+1 FROM [{tableName}]";
    var conn = this.Database.GetDbConnection();
    try
    {
        conn.Open();
        var cmd = conn.CreateCommand();
        cmd.CommandText = sqlStr;
        var reader = cmd.ExecuteReader();
        while (reader.Read())
        {
            nextID = reader.GetInt32(0);
        }
    }
    finally
    {
        conn.Close();
    }
    return nextID;
}

Steps to reproduce

Run the example solution at: https://github.com/ben-allamx/EFCoreTest2 The example project is a console project with the database model, context and the main has a simple loop that creates some random entities and then tries to add them to the context. This happens with AddRange. We started off with having the loop add the item to the context in the loop but that threw the error as well so we tried moving it to an AddRange which resulted in the same exception.

Further technical details

EF Core 2.2.0-preview3-35497 Database Provider: Microsoft.EntityFrameworkCore.SqlServer Operating system: Windows 10 Pro latest updates IDE: Visual Studio 2017 15.8.9

ajcvickers commented 6 years ago

@ben-allamx EF Core has the capability to go to the database and get a key value as soon as a new entity is tracked by the context. You might want to consider that.

The reason the code above isn't working in EF Core is because EF requires a every entity to have a key value. For entities that are going to get store-generated keys, this initial key value is temporary and created automatically. For entities that do not use store-generated keys, the actual key value should be supplied immediately. However, even though your keys are not store-generated, if you mark them as such then I believe your code will work. This is because EF will then generate temporary values for the keys, but your code will replace them with real values in SaveChanging, and these real values will be used when persisting to the store.

ben-allamx commented 6 years ago

If we remove the annotation: [DatabaseGenerated(DatabaseGeneratedOption.None)] Then the Mirgrations add Annotation("SqlServer:ValueGenerationStrategy", SqlServerValueGenerationStrategy.IdentityColumn) To the table creation. Which we do not want. Is there a middle ground or setting that can say do not set the column to be Identity in SQL server but also has it as a required field in the Model and does not force EF to have a unique value when adding new entities?

ben-allamx commented 6 years ago

In a test I tested the initial migration creation with the way we need the database created. With the data annotation stating : [DatabaseGenerated(DatabaseGeneratedOption.None)]

I then changed the model to remove this requirement so that we can add entities without problems, but in doing so when any migration is created after it includes that change in the database update and that causes the following error to be thrown: System.InvalidOperationException: 'To change the IDENTITY property of a column, the column needs to be dropped and recreated.'

ajcvickers commented 6 years ago

@ben-allamx I was reading between the lines incorrectly and assuming that the database already existed. To make sure the column in the database is not configured as store-generated, leave the DatabaseGenerated annotation as it is and configure a temp value generator explicitly. Something like:

modelBuilder
    .Entity<Role>()
    .Property(e => e.ID)
    .HasValueGenerator<TemporaryIntValueGenerator>();
ben-allamx commented 6 years ago

Thank you, that is what we are looking for.

ben-allamx commented 6 years ago

I was questioned about your very first statement:

EF Core has the capability to go to the database and get a key value as soon as a new entity is tracked by the context. You might want to consider that.

Can you explain or point me to documentation that elaborates on this concept of EF Core's capability to get a key value from the database as soon as the new entity is tracked?

Thank you.

ajcvickers commented 6 years ago

@ben-allamx Essentially, I was referring to value generators. Here is a somewhat simplistic example taking your code from above, but changing it to use a sequence in the database. You might also want to look at the HiLoValueGenerator<> classes that we ship which do some more complex stuff but in the same area.

public class Blog
{
    [DatabaseGenerated(DatabaseGeneratedOption.None)]
    public int Id { get; set; }
}

public class EntityFrameworkDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Test;ConnectRetryCount=0");

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder
            .Entity<Blog>()
            .Property(e => e.Id)
            .HasValueGenerator<MyValueGenerator>();

        modelBuilder.HasSequence<int>("SomeSequence");
    }
}

public class MyValueGenerator : ValueGenerator<int>
{
    public override int Next(EntityEntry entry)
        => GetNextID(
            entry.Context.Database.GetDbConnection(),
            entry.Metadata.Relational().TableName);

    public override bool GeneratesTemporaryValues => false;

    private Int32 GetNextID(DbConnection conn, string tableName)
    {
        Int32 nextID = -1;
        var sqlStr = "SELECT NEXT VALUE FOR [SomeSequence]";
        try
        {
            conn.Open();
            var cmd = conn.CreateCommand();
            cmd.CommandText = sqlStr;
            var reader = cmd.ExecuteReader();
            while (reader.Read())
            {
                nextID = reader.GetInt32(0);
            }
        }
        finally
        {
            conn.Close();
        }
        return nextID;
    }
}

public class Program
{
    public static void Main()
    {
        using (var context = new EntityFrameworkDbContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.AddRange(new Blog(), new Blog(), new Blog(), new Blog());

            context.SaveChanges();
        }
    }
}
wadebaird commented 1 year ago

@ajcvickers : I am encountering the same exception in the scenario that my Entity has a DbGenerated Identity column for the Id field. Is the only way around this to use the ValueGenerator that you mention above?

ajcvickers commented 1 year ago

@wadebaird This is an old issue. If you are having an issue with EF Core 7 or later, then please open a new issue and attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.