(Cosmos) Embedded entities (OwnsMany) throws an InvalidOperationException when calling SaveChangesAsync

[JamesBroadberry]

Hi,

I've created the following repository with a small, runnable project that demonstrates the issue we're seeing, publicly available here: https://github.com/JamesBroadberry/EFCore.Cosmos.EntityWithEmbeddedCollection

The issue could potentially to be with how the ChangeTracker works (although my knowledge of EF isn't that strong).

The README in the repository has comprehensive information about the issue we're seeing but long story short, when adding multiple items to a HashSet on an Entity and calling SaveChangesAsync, we get the following stack trace:

Unhandled exception. System.InvalidOperationException: The property 'OrderItem.Id' is defined as read-only after it has been saved, but its value has been modified or marked as modified.
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.PrepareToSave()
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.GetEntriesToSave(Boolean cascadeChanges)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(DbContext _, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteImplementationAsync[TState,TResult](Func`4 operation, Func`4 verifySucceeded, TState state, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteImplementationAsync[TState,TResult](Func`4 operation, Func`4 verifySucceeded, TState state, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at EFCore.Cosmos.EntityWithEmbeddedCollection.Program.AddOrder(CosmosStoreDbContext context, String customerName) in C:\Users\XXXX\source\repos\EFCore.Cosmos.EntityWithEmbeddedCollection\EFCore.Cosmos.EntityWithEmbeddedCollection\Program.cs:line 56
   at EFCore.Cosmos.EntityWithEmbeddedCollection.Program.Main() in C:\Users\XXXX\source\repos\EFCore.Cosmos.EntityWithEmbeddedCollection\EFCore.Cosmos.EntityWithEmbeddedCollection\Program.cs:line 39
   at EFCore.Cosmos.EntityWithEmbeddedCollection.Program.<Main>()

This could be related to (or potentially fixed by the solution to) #24803.

If you need any more information from me, please let me know and I'll do my best to help.

Thanks,
James

[JamesBroadberry]

I've just realised I originally committed the workaround we need to put in place. I've pushed a commit that fully demonstrates our issue. Sorry about that.

[ajcvickers]

@AndriySvyryd Looks like this could be a problem with the Cosmos update pipeline. The exception happens when calling SaveChanges twice, where the second call should be a no-op.

Minimal repro to be run with emulator:

public class CosmosStoreDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseCosmos(
            "AccountEndpoint=https://localhost:8081/;AccountKey=C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==",
            "ExampleScenarioForEntityWithEmbeddedCollection");

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.HasDefaultContainer("CosmosStoreDbContext");
        modelBuilder.Entity<Order>().OwnsMany(x => x.Items);
    }
}

public record OrderItem
{
    public string ItemName { get; set; }
    public decimal ItemPrice { get; set; }
}

public record Order
{
    public Guid Id { get; set; }
    public string CustomerName { get; set; }
    public ICollection<OrderItem> Items { get; set; } = new HashSet<OrderItem>();
}

public class Program
{
    public static async Task Main()
    {
        using (var dbContext = new CosmosStoreDbContext())
        {
            await dbContext.Database.EnsureCreatedAsync();
            dbContext.RemoveRange(dbContext.Set<Order>());
        }

        using (var dbContext = new CosmosStoreDbContext())
        {
            var bobOrder = dbContext.Add(new Order {CustomerName = "Bob"}).Entity;

            await dbContext.SaveChangesAsync();

            bobOrder.Items.Add(new OrderItem()
            {
                ItemName = "Foo",
                ItemPrice = 10
            });

            await dbContext.SaveChangesAsync();

            bobOrder.Items.Add(new OrderItem()
            {
                ItemName = "Bar",
                ItemPrice = 20
            });

            await dbContext.SaveChangesAsync();
            await dbContext.SaveChangesAsync();
        }
    }
}

[JamesBroadberry]

Thanks for acknowledging this and adding it to the backlog @ajcvickers

In case anyone is looking for a workaround, in the meantime we're manually adding an "Id" (initialised to a new Guid) to the embedded entity (OrderItem in this example) and setting it up like so:

modelBuilder.Entity<Order>().OwnsMany(x => x.Items).HasKey(item => item.Id);
modelBuilder.Entity<Order>().OwnsMany(x => x.Items).Property(item => item.Id).ValueGeneratedNever();

Our plan is to later remove these IDs as soon as EF supports it.
Also, the second line about ValueGeneratedNever seems to be important or the change tracker throws the following when testing on a larger scale.

Microsoft.EntityFrameworkCore.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(DbContext _, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)

[AndrewTriesToCode]

Hello! I'm running into this as well and have an observation that might help.
If you add an explicit int Id property to the owned entity and add a few things to the collection you can then query it and see in the debugger that EFCore has assigned the Id of each embedded entity with a simple increment.

However, if another embedded entity instance is added to the collection, after SavesChanges if you check the debugger these Id properties will have been zeroed out except the most recent. I believe this is then being seen as modified read only keys on subsequent SaveChanges.

I modified @ajcvickers example below to demonstrate.

public class CosmosStoreDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseCosmos(
            "AccountEndpoint=https://localhost:8081/;AccountKey=C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==",
            "ExampleScenarioForEntityWithEmbeddedCollection");

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.HasDefaultContainer("CosmosStoreDbContext");
        modelBuilder.Entity<Order>().OwnsMany(x => x.Items);
    }
}

public record OrderItem
{
    public string ItemName { get; set; }
    public decimal ItemPrice { get; set; }
}

public record Order
{
    public Guid Id { get; set; }
    public string CustomerName { get; set; }
    public ICollection<OrderItem> Items { get; set; } = new HashSet<OrderItem>();
}

public class Program
{
    public static async Task Main()
    {
        using (var dbContext = new CosmosStoreDbContext())
        {
            await dbContext.Database.EnsureCreatedAsync();
            dbContext.RemoveRange(dbContext.Set<Order>());
        }

        using (var dbContext = new CosmosStoreDbContext())
        {
            var bobOrder = dbContext.Add(new Order {CustomerName = "Bob"}).Entity;

            bobOrder.Items.Add(new OrderItem()
            {
                ItemName = "Foo",
                ItemPrice = 10
            });

            bobOrder.Items.Add(new OrderItem()
            {
                ItemName = "Bar",
                ItemPrice = 20
            });

            await dbContext.SaveChangesAsync();      
        }

        using (var dbContext = new CosmosStoreDbContext())
        {
            var bobOrder = await dbContext.SingleAsync(o => o.CustomerName == "Bob");

            bobOrder.Items.Add(new OrderItem()
            {
                ItemName = "Baz",
                ItemPrice = 30
            });

            // Debugger show's order item id's as expected
            await dbContext.SaveChangesAsync();
            // Debugger show's order item id's as all zeroes except the latest one.

            // Next save on the context throws exception because order item ids have changed.
            await dbContext.SaveChangesAsync();
        }

    }
}