Changes in parent owned collection cause unexpected behavior in nested owned collection

[AndriySvyryd]

In the following scenario the Toy is not deleted:

var options = Fixture.CreateOptions(seed: false);
using (var context = new LiveSiteDbContext(options))
{
    context.Database.EnsureCreatedResiliently();
    var family = new Family
    {
        Id = "familyId",
        PartitionKey = "partitionA",
        Kids = new List<Kid> { new Kid { Toys = new List<Toy> { new Toy() } } }
    };
    context.Families.Add(family);

    await context.SaveChangesAsync();
}

using (var context = new LiveSiteDbContext(options))
{
    var family = await context.Families.Where(f => f.Id == "familyId").FirstOrDefaultAsync();

    var newKid = new Kid
    {
        Toys = new List<Toy>()
    };
    family.Kids.First().Name = "name changedd";
    family.Kids.Add(newKid);
    family.Kids.First().Toys.RemoveAt(0);
    family.Kids.First().Toys.Add(new Toy());
    family.Kids.First().Toys.Add(new Toy());
            
    await context.SaveChangesAsync();
}

using (var context = new LiveSiteDbContext(options))
{
    var family = await context.Families.Where(f => f.Id == "familyId").FirstOrDefaultAsync();
    Assert.Equal(2, family.Kids.First().Toys.Count); // This is 3
}

Model

public class LiveSiteDbContext : DbContext
{        
    public DbSet<Family> Families { get; set; }


    public LiveSiteDbContext(DbContextOptions options) : base(options)
    {
    }

    protected override void OnModelCreating(ModelBuilder builder)
    {
        builder.Entity<Family>(b =>
        {
            b.HasNoDiscriminator()
                .HasPartitionKey(s => s.PartitionKey)
                .HasKey(s => new { s.Id, s.PartitionKey });

            b.Property(s => s.Id).ToJsonProperty("id");

            b.OwnsMany(s => s.Kids, kb =>
            {
                kb.OwnsMany(k => k.Toys, tb => {
                    tb.HasKey(t => t.ToyId);
                });
            });
        });
    }
}
    
public class Family
{
    public string Id { get; set; }
    public string PartitionKey { get; set; }
    public string Name { get; set; }

    public ICollection<Kid> Kids { get; set; }
}

public class Kid
{
    public string Name { get; set; }

    public List<Toy> Toys { get; set; }
}

public class Toy
{
    public string ToyId { get; set; }
    public string Name { get; set; }

    public Toy()
    {
        ToyId = Guid.NewGuid().ToString();
        Name = "New Toy";
    }
}

Additionally if Toy.ToyId is removed the following exception is thrown:

InvalidOperationException

System.InvalidOperationException : The property 'Toy.KidId' is part of a key and so cannot be modified or marked as modified. To change the principal of an existing entity with an identifying foreign key, first delete the dependent and invoke 'SaveChanges', and then associate the dependent with the new principal.
ChangeDetector.ThrowIfKeyChanged(InternalEntityEntry entry, IProperty property) line 74
ChangeDetector.PropertyChanged(InternalEntityEntry entry, IPropertyBase propertyBase, Boolean setModified) line 57
InternalEntityEntryNotifier.PropertyChanged(InternalEntityEntry entry, IPropertyBase property, Boolean setModified) line 115
InternalEntityEntry.SetProperty(IPropertyBase propertyBase, Object value, Boolean isMaterialization, Boolean setModified, Boolean isCascadeDelete, CurrentValueType valueType) line 1351
InternalEntityEntry.SetTemporaryValue(IProperty property, Object value, Boolean setModified) line 706
InternalEntityEntry.PropagateValue(InternalEntityEntry principalEntry, IProperty principalProperty, IProperty dependentProperty, Boolean isMaterialization, Boolean setModified) line 636
NavigationFixer.SetForeignKeyProperties(InternalEntityEntry dependentEntry, InternalEntityEntry principalEntry, IForeignKey foreignKey, Boolean setModified, Boolean fromQuery) line 1254
NavigationFixer.KeyPropertyChanged(InternalEntityEntry entry, IProperty property, IEnumerable1 containingPrincipalKeys, IEnumerable1 containingForeignKeys, Object oldValue, Object newValue) line 524
InternalEntityEntryNotifier.KeyPropertyChanged(InternalEntityEntry entry, IProperty property, IEnumerable1 keys, IEnumerable1 foreignKeys, Object oldValue, Object newValue) line 106
ChangeDetector.DetectKeyChange(InternalEntityEntry entry, IProperty property) line 270
ChangeDetector.PropertyChanged(InternalEntityEntry entry, IPropertyBase propertyBase, Boolean setModified) line 60
InternalEntityEntryNotifier.PropertyChanged(InternalEntityEntry entry, IPropertyBase property, Boolean setModified) line 115
InternalEntityEntry.SetProperty(IPropertyBase propertyBase, Object value, Boolean isMaterialization, Boolean setModified, Boolean isCascadeDelete, CurrentValueType valueType) line 1351
InternalEntityEntry.SetTemporaryValue(IProperty property, Object value, Boolean setModified) line 706
DocumentSource.UpdateDocument(JObject document, IUpdateEntry entry, Nullable1 ordinal) line 268 DocumentSource.UpdateDocument(JObject document, IUpdateEntry entry) line 149 CosmosDatabaseWrapper.SaveAsync(IUpdateEntry entry, CancellationToken cancellationToken) line 284 CosmosDatabaseWrapper.SaveChangesAsync(IList1 entries, CancellationToken cancellationToken) line 167
StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken) line 1125
StateManager.SaveChangesAsync(StateManager stateManager, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken) line 1224
<b__0>d.MoveNext() line 311

Could be related to #28600

[brasky]

I'm not sure if this is a separate issue, but I encountered another issue which is very similar to this.

Lets say you have an ICollection<Book> Books in your DbContext, and your Book class has an owned ICollection<Page> Pages, now imagine this scenario:

var book1 = context.Books.Where(b => b.Id == someId).First();

book1.Pages.Add(new Page());

await context.SaveChangesAsync()

var book2 = context.Books.Where(b => b.Id == someOtherId).First();

book2.Pages.Add(new Page());

await context.SaveChangesAsync()

This will throw an InvalidOperationException stating that Page.Id is readonly and has been changed or marked as changed.

Two things will solve this:

1. after the first SaveChangesAsync(), if you clear the change tracker, everything will work fine
2. Only call SaveChangesAsync once, which I guess you should almost always do, but in our case it kind of made sense to call it twice.

Curious if this is the same issue/root cause or if it's a different issue entirely, or even expected behavior... Hard to know sometimes with the Cosmos DB provider.

[brasky]

Actually once I wrote it out fully I realized that it's probably not correct to re-use the context to call SaveChangesAsync multiple times. Correct me if I'm wrong but I'm guessing you will say that the following code is expected to fail (it does):

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;

class TestContext : DbContext
{
    public TestContext()
    {

    }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseCosmos("https://localhost:8081", "C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==", "TestDb");
    }

    public DbSet<Book> Books { get; set; }
}

class Book
{
    public int Id { get; set; }
    public ICollection<Page> Pages { get; set; }
}

[Owned]
class Page
{
    public string Text { get; set; } = "";
}

class Program
{

    static async Task Main(string[] args)
    {

        using (var context = new TestContext())
        {
            await context.Database.EnsureDeletedAsync();
            await context.Database.EnsureCreatedAsync();
            context.Books.Add(
                new Book()
                {
                    Id = 1,
                    Pages = new List<Page>() { new Page() }
                });

            context.Books.Add(
                new Book()
                {
                    Id = 2,
                    Pages = new List<Page>() { new Page() } 
                });

            await context.SaveChangesAsync();
        }

        using(var context = new TestContext())
        {
            var book1 = context.Books.Where(b => b.Id == 1).First();
            book1.Pages.Add(new Page());
            await context.SaveChangesAsync();

            var book2 = context.Books.Where(b => b.Id == 2).First();
            book2.Pages.Add(new Page());
            await context.SaveChangesAsync();
        }
    }
}

[AndriySvyryd]

@brasky Reusing the context is not recommended, but it should work fine once the issue is fixed

[mnguyen36]

@AndriySvyryd I was the one that you opened this bug for (Minh Nguyen), I am wondering if this is related to nested properties not getting saved, i.e. if in the family/kid/toy model, if we add a new nested property to toy

public class Toy
{
    public string ToyId { get; set; }
    public string Name { get; set; } 
    public ToyMeta ToyMeta {get; set}

    public Toy()
    {
        ToyId = Guid.NewGuid().ToString();
        Name = "New Toy";
    }
}
public class ToyMeta 
{
    public string Description {get;set;}
}

adding this new property and updating it will not persist