Efcore: InMemory: Cascade Delete Support

[hkoestin]

Dear all

During my tests with EF RC1, I found, that the EF.InMemory is lacking a quite important feature (IMHO). Cascading deletes are not supported by EF.InMemory!

Having the following context and classes, you can reproduce the issue quite easily:

``` c#
public class DatabaseContext : DbContext
{
public DatabaseContext(DbContextOptions options)
: base(options)
{
}

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        var factory = Database.GetService<ILoggerFactory>();
        factory.AddDebug(LogLevel.Verbose);

        modelBuilder.Entity<Apple>().HasKey(x => x.Identity);
        modelBuilder.Entity<Apple>().Property(x => x.Identity).ValueGeneratedNever();
        modelBuilder.Entity<Apple>().Property(x => x.Version).IsConcurrencyToken();

        modelBuilder.Entity<AggregateIndexes<Apple>>().HasKey(x => new {x.AggregateIdentity, x.AggregateVersion});

        modelBuilder.Entity(typeof (MapIndex)).HasKey("AggregateIdentity", "AggregateVersion");
        modelBuilder.Entity(typeof (MapIndex)).Property<Guid>("AggregateIdentity").ValueGeneratedNever();
        modelBuilder.Entity(typeof (MapIndex)).Property<Guid>("AggregateVersion").ValueGeneratedNever();

        modelBuilder.Entity(typeof (MapIndex))
            .HasOne(typeof (AggregateIndexes<Apple>))
            .WithOne()
            .HasForeignKey(typeof (MapIndex), "AggregateIdentity", "AggregateVersion")
            .IsRequired()
            .OnDelete(DeleteBehavior.Cascade);

        modelBuilder.Entity<ComplexMapIndex>().HasKey(x => x.Key);
        modelBuilder.Entity<ComplexMapIndex>().Property(x => x.Key).ValueGeneratedNever();

        modelBuilder.Entity(typeof (ComplexMapIndex)).HasKey("Key");
        modelBuilder.Entity(typeof (ComplexMapIndex)).Property<Guid>("Key").ValueGeneratedNever();

        modelBuilder.Entity(typeof (ComplexMapIndex))
            .HasOne(typeof (AggregateIndexes<Apple>))
            .WithMany()
            .HasForeignKey("AggregateIdentity", "AggregateVersion")
            .IsRequired()
            .OnDelete(DeleteBehavior.Cascade);
    }
}

public abstract class Aggregate
{
    public Guid Identity { get; set; }
    public Guid Version { get; set; }
}

public class Apple : Aggregate
{
    public string Color { get; set; }        
}

public class AggregateIndexes<T> where T: Aggregate
{
    public Guid AggregateIdentity { get; set; }
    public Guid AggregateVersion { get; set; } 
}

public class MapIndex
{
    public Guid AggregateIdentity { get; set; }
    public Guid AggregateVersion { get; set; }

    public string IndexValue { get; set; }
}

public class ComplexMapIndex
{
    public Guid Key { get; set; }

    public Guid AggregateIdentity { get; set; }
    public Guid AggregateVersion { get; set; }

    public string IndexValue { get; set; }
}

With the following lines of code, you can see the results:

``` c#
        private static void Main(string[] args)
        {
            var builder = new DbContextOptionsBuilder();
//            builder.UseInMemoryDatabase();
            builder.UseSqlCe(@"Data Source=AggregatesIndexingWithDeleteCascade.sdf");

            var appleIdentity = Guid.NewGuid();

            // fill the context with some data
            using (var context = new DatabaseContext(builder.Options))
            {
                context.Database.EnsureCreated();

                var apple = new Apple
                {
                    Identity = appleIdentity,
                    Version = Guid.NewGuid(),
                    Color = "Red"
                };

                var indexes = new AggregateIndexes<Apple>()
                {
                    AggregateIdentity = apple.Identity,
                    AggregateVersion = apple.Version
                };


                context.Set<Aggregate>().Add(apple);
                context.Set<AggregateIndexes<Apple>>().Add(indexes);

                var mapIndex = new MapIndex
                {
                    AggregateIdentity = apple.Identity,
                    AggregateVersion = apple.Version,
                    IndexValue = "index-value-for-the-red-apple"
                };
                context.Set<MapIndex>().Add(mapIndex);

                for (int i = 0; i < 5; i++)
                {
                    var index = new ComplexMapIndex()
                    {
                        Key = Guid.NewGuid(),
                        AggregateIdentity = apple.Identity,
                        AggregateVersion = apple.Version,
                        IndexValue = "complex-index-value-for-the-red-apple"
                    };
                    context.Set<ComplexMapIndex>().Add(index);
                }

                context.SaveChanges();
            }

            // now we delete the AggregateIndexes<Apple> again,
            // the cascade should remove all entries now (map-index, complex-map-index)
            using (var context = new DatabaseContext(builder.Options))
            {
                var appleIndexes = context.Set<AggregateIndexes<Apple>>().Single(x => x.AggregateIdentity == appleIdentity);
                context.Remove(appleIndexes);
                context.SaveChanges();
            }

            // all entries should now be gone, as they are deleted by the cascade
            using (var context = new DatabaseContext(builder.Options))
            {
                var appleIndexes = 
                    context.Set<AggregateIndexes<Apple>>().SingleOrDefault(x => x.AggregateIdentity == appleIdentity);
                Console.WriteLine("AggregateIndexes<Apple> gone: " + (appleIndexes == null));

                var mapIndex = context.Set<MapIndex>().SingleOrDefault(x => x.AggregateIdentity == appleIdentity);
                Console.WriteLine("MapIndex gone: " + (mapIndex == null));

                var complexMapIndex = context.Set<ComplexMapIndex>().Where(x => x.AggregateIdentity == appleIdentity);
                Console.WriteLine("ComplexMapIndex gone: " + (!complexMapIndex.Any()));
            }

            Console.WriteLine("Finished ... Press any key to continue");
            Console.ReadLine();
        }

Output for SQL CE Provider (and all other relational providers)

``` c#
AggregateIndexes gone: True
MapIndex gone: True
ComplexMapIndex gone: True
Finished ... Press any key to continue

Output for EF.InMemory

``` c#
AggregateIndexes<Apple> gone: True
MapIndex gone: False
ComplexMapIndex gone: False
Finished ... Press any key to continue

The results for the actual providers is correct (and as expected). The output for EF.InMemory however isn't - or at least to me, it is surprising.

In your documentation I found the following:

Note
This cascading behavior is only applied to entities that are being tracked by the context. A corresponding cascade behavior should be setup in the database to ensure data that is not being tracked by the context has the same action applied. If you use EF to create the database, this cascade behavior will be setup for you.

Whereas I totally get this for a real context, which would need to evaluate these FK relations in-memory and therefore fetch the entries from the store, I actually cannot understand, why EF.InMemory can't do this for me. It already has the whole graph in-memory and would only need to evaluate the FK relations, which are already in place.

Actually, the EF.Core infrastructure almost does all the things, and handles the delete in the graph correctly. It propagates the DELETE calls along the graph-relations.

• https://github.com/aspnet/EntityFramework/blob/48e365d3433eef53b210a3f132676f1f1a1f1c4c/src/EntityFramework.Core/ChangeTracking/Internal/InternalEntityEntry.cs#L488
• https://github.com/aspnet/EntityFramework/blob/3b4bc7e508b33a6564cecffedcb1bd1bc920c97a/test/EntityFramework.Core.FunctionalTests/GraphUpdatesTestBase.cs#L1828

Unfortunately, EF.InMemory does not really support this, as shown in these tests.

• https://github.com/aspnet/EntityFramework/blob/51153628eeb9dac58919a17c20de5db1b3f352b4/test/EntityFramework.InMemory.FunctionalTests/GraphUpdatesInMemoryTest.cs

Why can't EF.InMemory handle this? It would be quite easy to implement I guess.

And why can it delete entries, if they are in the current context? At that point, it already CAN evaluate the FK relations and the cascading deletes. So it could also do for all entries in the store.
For the in-memory case, all entries are always in memory (and thus close to the context). It would be easy to fetch them, in case of a delete call to a cascading FK relation.

Any input to this topic?

Thanks a lot,
Harald

[zpbappi]

FYI, I am grabbing this one.

[zpbappi]

Here is an update of the issue:

_Why cascade delete isn't working for InMemory database?_
When we delete an entry, the related entities are not loaded in StateManager (or, change tracking system), unless we specifically include them using Include(...). So, they are not marked for deletion and stays unaffected. If you include the entries that you think should be deleted or updated when you delete a parent entry, it should work fine. For example, the following test should pass, even for InMemory database:

[Fact]
void test_written_in_notepad()
{
  List<int> childIds;
  using(var context = CreateInMemoryContext())
  {
    var parent = context.Parents.Include(p => p.Children).FirstOrDefault(p => p.Id == 1);
    childIds = p.Children.Select(c => c.Id).ToList();
    context.Remove(parent);
    context.SaveChanges();
  }

  using(var context = CreateInMemoryContext())
  {
    Assert.Empty(context.Children.Where(c => childIds.Contains(c.Id));
  }
}

_Then why is it working for relational databases?_
I don't know if it was intentional or not, but _not-handling-of-dependent-entries_ behavior works pretty well for relational databases. There are three possible outcomes when you delete a parent entry in a relational database.

1. all the children get deleted
2. all the reference id of the children get updated to NULL
3. deletion of the parent is restricted as long as it has children

It turns out, related database are designed to handle that without any tweaking from the EF. All EF has to do is generate the proper SQL (on delete cascade or whatever is appropriate) when creating the tables.

_What needs to be done for InMemory database to support cascade deletion?_
Since we do not get any magical support from the database here, the easiest possible thing we can do is to load the whole object graph in memory when a query is executed, regardless of Include(...) uses. That way, we can always traverse the object graph and delete and/or update accordingly. However, in this case, users will experience different behavior from other relational databases. So, the next option is to load the dependent entries _on demand_, make sure that they are tracked by the StateManager and update the entries accordingly. _on demands_ refers to the when all the foreign keys of an entry are evaluated to check for dependent entries for cascade deletion.

_What did I do?_
I have created a service with a default implementation of _Do Nothing_ and an InMemory implementation of loading the dependent entries (if they are not loaded) and updating the EntityState properly. It appears to be demonstrating expected behavior for cascade deletion.

_What's next?_
While the tests in the VS solution related to cascade deletion feature are passing, some test's are inconclusive (i.e. failing). Tests that asserts behavior like _You cannot add a one-to-one or one-to-many relation entry without specifying the reference field value_. Here is one such failing test:

[ConditionalFact]
public virtual void Optional_One_to_one_relationships_are_one_to_one()
{
    using (var context = CreateContext())
    {
        var root = context.Roots.Single(IsTheRoot);

        root.OptionalSingle = new OptionalSingle1();

        Assert.Throws<DbUpdateException>(() => context.SaveChanges());
    }
}

To support this behavior, it is apparent that implementing cascade delete is not going to be enough. We also need to check _Added_ and _Modified_ entries for relational consistency when saving. I understand that it does not fall under the title of this issue. But, without this implementation, I cannot really make the tests pass- unless I overwrite them to be empty, as they are in the dev branch.

So, my question is, should I submit a PR for cascade delete feature only under this issue and implement relational consistency checking in another PR? Or, would you like to change the title/description of the issue so that I can submit a single PR and make all the tests passing?

[ajcvickers]

@zpbappi The support for cascade delete should be implemented purely in the in-memory database. It should not change the StateManager in any way. It should not change which entities are being loaded. This means that the in-memory database needs to understand the FK constraints that have been created and what their cascade behavior is defined as. It should then follow these constraints, independently of the state manager, when an entity is deleted and make appropriate changes or throw appropriate exceptions based on the constraint and cascade behavior. You would effectively be implementing some part of the FK support that a relational database has.

[Mardoxx]

This feature would be very welcome. Any updates?

[ajcvickers]

This issue is in the Backlog milestone. This means that it is not going to happen for the 2.0 release. We will re-assess the backlog following the 2.0 release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

[Mardoxx]

Brilliant, thank you :)

[PascalHonegger]

Any updates? I would really appreciate this feature to ensure my test in memory database behaves (more) like my production database.

[AndriySvyryd]

This depends on https://github.com/aspnet/EntityFrameworkCore/issues/2166