Orphans are deleted when overlapping composite alternate key is mutated

[mrpmorris]

Given the following simple Parent/Child model, where Child is an aggregate part of Parent (and cannot exist without it).

internal class Parent
{
    public Guid Id { get; private set; } = Guid.NewGuid();
    public virtual List<Child> Children { get; private set; } = new();
}

internal class Child
{
    public Guid Id { get; private set; } = Guid.NewGuid();
    public Guid ParentId { get; set; }
    public virtual Parent Parent { get; set; } = null!;
}

I want to prohibit a Parent from being deleted if it has Children, and I also want any Childthat is removed fromParent.Children` to be automatically deleted on save.

internal class ApplicationDbContext : DbContext
{
    public DbSet<Parent> Parents { get; set; } = null!;
    private DbSet<Child> Children { get; set; } = null!;

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

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Child>()
            .HasOne(x => x.Parent)
            .WithMany(x => x.Children)
            .OnDelete(DeleteBehavior.Restrict);
    }
}

As per the documentation, this code throws an exception because Child.Parent cannot be null.

var options = new DbContextOptionsBuilder<ApplicationDbContext>()
    .UseInMemoryDatabase("Test")
    .Options;

using (var context1 = new ApplicationDbContext(options))
{
    var parent = new Parent();
    parent.Children.Add(new Child());

    context1.Parents.Add(parent);
    context1.SaveChanges();
}

using (var context2 = new ApplicationDbContext(options))
{
    var parent = context2.Parents.Include(x => x.Children).First();
    parent.Children.RemoveAt(0);
    context2.SaveChanges();
}

It seems that modelBuilder.Entity<Child>().HasAlternateKey(x => new { x.Id, x.ParentId }) has the behaviour I expected, but @ajcvickers said "I certainly wouldn't do it that way" - https://twitter.com/ajcvickers/status/1565633141880102914

What is the recommended practice to ensure the child is deleted instead?

[ajcvickers]

@mrpmorris I would set the delete behavior to ClientCascade so that cascade delete is not created in the database, then use CascadeTiming.Never so that orphans are deleted by cascading delete itself never happens.

Here's an example:

public static class Your
{
    public static string ConnectionString = @"Data Source=(LocalDb)\MSSQLLocalDB;Database=Test";
}

public class Parent
{
    public Guid Id { get; private set; } = Guid.NewGuid();
    public virtual List<Child> Children { get; private set; } = new();
}

public class Child
{
    public Guid Id { get; private set; } = Guid.NewGuid();
    public Guid ParentId { get; set; }
    public virtual Parent Parent { get; set; } = null!;
}

public class SomeDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseSqlServer(Your.ConnectionString);

    public SomeDbContext()
    {
        ChangeTracker.CascadeDeleteTiming = CascadeTiming.Never;
    }

    public DbSet<Parent> Parents { get; set; } = null!;
    private DbSet<Child> Children { get; set; } = null!;
    
    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Child>()
            .HasOne(x => x.Parent)
            .WithMany(x => x.Children)
            .OnDelete(DeleteBehavior.ClientCascade);
    }
}

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

            var parent = new Parent();
            parent.Children.Add(new Child());

            context.Parents.Add(parent);
            
            context.SaveChanges();
        }
        
        using (var context = new SomeDbContext())
        {
            context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Never;
            
            var parent = context.Parents.First();
            context.Remove(parent);

            try
            {
                context.SaveChanges();
            }
            catch (Exception e)
            {
                Console.WriteLine(e.Message);
            }
        }
        
        using (var context = new SomeDbContext())
        {
            context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Never;
            
            var parent = context.Parents.Include(x => x.Children).First();
            context.Remove(parent);

            try
            {
                context.SaveChanges();
            }
            catch (Exception e)
            {
                Console.WriteLine(e.Message);
            }
        }

        using (var context = new SomeDbContext())
        {
            context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Never;
            
            var parent = context.Parents.Include(x => x.Children).First();
            parent.Children.RemoveAt(0);
            context.SaveChanges();
        }
    }
}

[mrpmorris]

I have a UnitOfWork to abstract away the DbContext, so that setting would have to go into the constructor, and would affect everything. But obviously I want to control it per aggregate.

It seems hacky, telling EF you want to ClientCascade deletes in order to tell it not to delete.

Can you tell me why this is better than HasAlternateKey?

[ajcvickers]

I need to investigate this some more to understand why the alternate key is causing the dependent to be deleted, but it seems to be a bug.

[mrpmorris]

Damn, it makes so much sense to me.

[ajcvickers]

@mrpmorris Can you explain your logic?

[mrpmorris]

The way I saw it was that the key is the real identity, so used for referencing. Typically I have a single column for a key, because I don't like multi-part keys getting longer as I add more child tables....

So, saying I have an "Alternate Key" to me seems like I am saying "This isn't what you'd use for referential integrity, but it is another way of specifying the object's identity" - and it makes sense to me that if you destroy its identity then you want to destroy the object.

PurchaseOrder
  int Id

PurchaseOrderLine
  int Id
  int PurchaseOrderId

SomeChildOfPurchaseOrderLine
  int Id
  int PurchaseOrderLineId

PurchaseOrderLine[Id] = for other tables to reference and have a FK in the DB.
PurchaseOrderLine[Id + PurchaseOrderLineId] = This is my full identity, without which I am nothing

I just want to be able to do

PurchaseOrder po = await db.PurchaseOrders.Include(x => x.Lines).SingleAsync();
po.Lines.RemoveAt(0);
await db.SaveChangesAsync();

But, at the same time, prevent this in the DB with a prohibited delete if the order has lines

PurchaseOrder po = await db.PurchaseOrders.SingleAsync();
await db.PurchaseOrders.Remove(po);
await MyDbContext.SaveChangesAsync();

And I'd like to be able to do it by convention when defining my model with ModelBuilder rather than having to use specific code at the point of consuming the model.

[ajcvickers]

destroy its identity then you want to destroy the object.

Changing an alternate key value is supposed to throw. Once #4073 is implemented, it should cause the entity to be updated with the new key values.

[mrpmorris]

I think there needs to be a simple way of doing this.

As I am doing DDD I only have a repo for my aggregate root, so the only way of indicating a child part should be deleted is removing it from a List<Child> but, at the same time, I need to be able to specify whether deleting the root should cascade to the children or the existence of children should prevent the root from being deleted.

Maybe

builder.Entity<Child>(x =>
  x
    .HasOne<Parent>()
    .WithMany()
    .HasForeignKey(x => x.ParentId)
    .OnDelete(DeleteBehaviour.Restrict)
    .OnOrphaned(OphanedBehaviour.Delete)); // this line here

Not my final suggestion, just a seedling of an idea :)

[ajcvickers]

Tracked by #10066. Make sure to vote for that issue.

[mrpmorris]

Thanks. To vote, do I just thumb it up?

[mrpmorris]

@ajcvickers That other report is nearly 5 years old. That suggests it's not going to happen, at least in the near future :(

So my current implementation relies on a bug that will soon be fixed, and my alternatives are either to not update my version or implement something that will effect all client-deletes?

[ajcvickers]

@mrpmorris You can always override SaveChanges (or use and event/interceptor) and process the deletion of orphans there. This was the way it was done before any form of orphan deletion was available.