Owned Entity marked as detached when adding to a collection when the entities primary key is set

[ChristopherHaws]

I have a simple entity which owns many children. If I add a child entity with the PK set, EF marks the added child entity as detached instead of added.
In EF Core 2.2.x this behavior worked, however it no longer seems to work in EF Core 3.0.

Steps to reproduce

async Task Main()
{
    var services = new ServiceCollection();
    services.AddDbContext<BlogContext>(ef =>
    {
        //ef.UseSqlServer("Server=(localdb)\\mssqllocaldb; Database=Test; Trusted_Connection=True; MultipleActiveResultSets=true");
        ef.UseInMemoryDatabase("InMemory");
    });

    using var container = services.BuildServiceProvider();

    using (var scope = container.CreateScope())
    {
        var db = scope.ServiceProvider.GetRequiredService<BlogContext>();
        await db.Database.EnsureDeletedAsync();
        await db.Database.EnsureCreatedAsync();

        db.Blogs.Add(new Blog());

        await db.SaveChangesAsync();
    }

    using (var scope = container.CreateScope())
    {
        var db = scope.ServiceProvider.GetRequiredService<BlogContext>();

        var blog = await db.Blogs.SingleAsync();
        blog.Post.Add(new Post
        {
            BlogId = blog.BlogId,
            PostId = Guid.NewGuid(), // Comment this line out and everything works
            Name = "Post 1"
        });

        // Prints "Detached"
        Console.WriteLine(db.Entry(blog.Post.Single()).State);

        // Throws DbUpdateConcurrencyException
        await db.SaveChangesAsync();
    }
}

public class BlogContext : DbContext
{
    public BlogContext(DbContextOptions<BlogContext> options) : base(options) {}
    
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Blog>(blog =>
        {
            blog.HasKey(x => x.BlogId).IsClustered();

            blog.OwnsMany(x => x.Post, post =>
            {
                post.HasKey(x => x.PostId).IsClustered();
                post.WithOwner().HasForeignKey(x => x.BlogId);
            });
        });
    }
}

public class Blog
{
    public Guid BlogId { get; set; }
    public ICollection<Post> Post { get; set; } = new HashSet<Post>();
}

public class Post
{
    public Guid PostId { get; set; }
    public Guid BlogId { get; set; }
    public String Name { get; set; }
}

When using InMemory database, this code produces the following exception:

DbUpdateConcurrencyException
Attempted to update or delete an entity that does not exist in the store.
   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.SaveChangesAsync(IList`1 entries, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at UserQuery.Main(), line 40

When using SqlServer, it produces the following exception:

DbUpdateConcurrencyException
Database operation expected to affect 1 row(s) but actually affected 0 row(s). Data may have been modified or deleted since entities were loaded. See http://go.microsoft.com/fwlink/?LinkId=527962 for information on understanding and handling optimistic concurrency exceptions.
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ThrowAggregateUpdateConcurrencyException(Int32 commandIndex, Int32 expectedRowsAffected, Int32 rowsAffected)
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeResultSetWithoutPropagationAsync(Int32 commandIndex, RelationalDataReader reader, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeAsync(RelationalDataReader reader, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(DbContext _, ValueTuple`2 parameters, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(DbContext _, ValueTuple`2 parameters, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at UserQuery.Main(), line 40

Further technical details

EF Core version: 3.0
Database provider: Microsoft.EntityFrameworkCore.SqlServer and Microsoft.EntityFrameworkCore.InMemory
Target framework: .NET Core 3.0
Operating system: Windows 10 1809
IDE: Visual Studio 2019 16.3, Visual Studio 2019 16.4 Preview 1, and LINQPad 6.

[smitpatel]

@AndriySvyryd - The state printed is actually detached.

The issue is ran into due to https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#dc
Calling add before adding to collection makes it working

var query = db.Blogs.ToList();
                var blog = db.Blogs.Single();
                var newPost = new Post
                {
                    BlogId = blog.BlogId,
                    PostId = Guid.NewGuid(), // Comment this line out and everything works
                    Name = "Post 1"
                };
                db.Add(newPost);
                blog.Post.Add(newPost);

                // Prints "Detached"
                Console.WriteLine(db.Entry(blog.Post.Single()).State);
                // Throws DbUpdateConcurrencyException
                db.SaveChanges();

[AndriySvyryd]

I don't think this is the expected behavior.

[MarTeezy]

This is an unexpected behavior. In some cases we would like to let the database generate our identifier, and in others we need to accept an identifier to synchronize entities across services.
I understand the workaround @smitpatel. However, this means that we must explicitly detect whether the entities in a collection are being added or updated, something that EF Core has abstracted for us in the past.

The mitigation would be to not let the database generate the identifier and ensure we always create our own?

[samisq]

We're also encountering this exact issue, not just with owned entities, but also with one-to-many relationships. I see this issue has been tagged with 3.1.0 release. If there any plan to release a patch before then? We rely heavily on this behavior. Is there a recommended workaround?

[ajcvickers]

@samisq We don't plan to patch this. 3.1 is scheduled for release on December 3, and any patch would be unlikely to ship much before this anyway.

[ajcvickers]

Note for triage: this seems to be working as designed with current bits. Specifically:

• PostId is configured to use generated values
• If the value is explicitly set, then DetectChanges now honors this and treats the entity as existing
• Not sure why @smitpatel was seeing Detached state, but with current bits the state is Modified as expected. This may have been fixed since 3.0.

The solution is to mark the property to not use generated key values:

post.Property(e => e.PostId).ValueGeneratedNever();

Bringing back to triage before closing to see if I missed anything.

[AndriySvyryd]

I think 0259104 fixed it

[ChristopherHaws]

Just as an FYI, I have been working on migrating an app from EF Core 2.2 to 3.0 and the number one runtime issue we have been running into is what I describe in this issue. We have a lot of places where we manually set PKs as Guid's in C# code instead of generating it on the SQL server, which behaves completely different in 3.0. I have been able to find most of the issues thanks to our unit tests, but we only have about 70% coverage so I am not confident that I have found them all.

I do understand that it is specifically called out in the upgrade docs, but to me this seems to contradict the "why" section of those docs. It seems like it less consistent now than it was before. For example, some types (string, datetime, etc) behave differently now than other types (int, guid, etc) when they are part of the entity's key. Having to remember which types will and wont be generated by the backing store and remembering to add ValueGeneratedNever for types that are is more confusing to me than simply configuring the property to be ValueGeneratedOnAdd or having some global configuration that allows you to specify which types are store generated by default. The fact that if BlogId and PostId were both strings, the behavior of Add is completely different to if they were guid's is easy to mess up.

[ajcvickers]

@ChristopherHaws I think it is fair to say that it is now more important than before to understand whether a key is configured to use generated values or not. However, Guid and int keys have always been configured by default to use generated values--this hasn't changed. It also isn't new that entity types with generated key values behave differently. The only thing that has changed in 3.0 is that this different behavior now also applies when DetectChanges is called. So previously when Attach encountered a type configured to use generated key values, and the key value was set, then it was determined to be an existing entity rather than a new entity. However, DetectChanges did not do this, bringing in an entity with a key value set as new regardless. Now DetectChanges and attach both do the same thing.

I added a specific note to dotnet/EntityFramework.Docs#1046 (comment) to ensure that this is well documented.

[MarTeezy]

Thanks for the clarifications @ajcvickers
For anyone interested, this is how I have solved this.

I have created an interface IManuallyGeneratedIdEntity (name pending) and attached it to many of our entities.
I have created a ModelBuilder extension to set the ValueGenerated property on Id for these entities.

public static ModelBuilder ChildEntityValueGeneratedNever(this ModelBuilder modelBuilder)
{
    foreach (var entity in modelBuilder.Model.GetEntityTypes().Where(m => typeof(IManuallyGeneratedIdEntity).IsAssignableFrom(m.ClrType)))
    {
        var property = entity.GetProperties().Single(p => p.IsKey() && p.Name == "Id");
        property.ValueGenerated = ValueGenerated.Never;
    }

    return modelBuilder;
}

Finally, we have a RepositoryBase which encapsulates our DbContext. Our SaveChangesAsync method performs some pre-save and post-save actions. I've added a pre-save action:

private void GenerateEntityIds()
{
    var entities = Context.ChangeTracker.Entries().Where(e => e.Entity is IManuallyGeneratedIdEntity).ToList();

    foreach (var entry in entities)
    {
        var entity = (IManuallyGeneratedIdEntity) entry.Entity;
        
        if (entity.Id == default)
        {
            entity.Id = Guid.NewGuid();
        }
    }
}