dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.78k stars 3.19k forks source link

Moving an entity between collections sometimes deletes its children, dependent on entity load order #30634

Open DrPhil opened 1 year ago

DrPhil commented 1 year ago

File a bug

When we have several level of collection entities like this: Blog->Post->Comment, then you get inconsistent (and surprising!) behavior if you manually change what collection a middle entity is part of. Sometimes the children entities will be deleted, and sometimes they are kept. I would have assumed that they were always kept.

oldBlog.Posts.Remove(post);
newBlog.Posts.Add(post);
db.SaveChanges(); // post.Comments might be deleted, or might be untouched. It depends on entity loading order!

The documentation gives a tip:

Do not write code to manipulate all navigations and FK values each time a relationship changes. Such code is more complicated and must ensure consistent changes to foreign keys and navigations in every case. If possible, just manipulate a single navigation, or maybe both navigations. If needed, just manipulate FK values. Avoid manipulating both navigations and FK values.

In the below example we manipulate both navigations and no FK, and we still get inconsistent behavior.

Include your code

dotnet new console -o EFChangeParentDeletion
cd EFChangeParentDeletion/
dotnet add package Microsoft.EntityFrameworkCore.Sqlite
dotnet add package Microsoft.EntityFrameworkCore.Design

# first add the files below and then run the migration
dotnet ef migrations add InitialCreate
dotnet ef database update

Models.cs

#nullable enable
using Microsoft.EntityFrameworkCore;

namespace EFChangeParentDeletion;

public class BloggingContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }
    public DbSet<Post> Posts { get; set; }
    public DbSet<Comment> Comments { get; set; }

    public string DbPath { get; }

    public BloggingContext()
    {
        var folder = Environment.SpecialFolder.LocalApplicationData;
        var path = Environment.GetFolderPath(folder);
        DbPath = Path.Combine(path, "my_blogging.db");
    }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseSqlite($"Data Source={DbPath}");
}

public class Blog
{
    public int BlogId { get; set; }

    public required string Url { get; set; }
    public List<Post> Posts { get; set; } = new();
}

public class Post
{
    public int PostId { get; set; }

    public required string Title { get; set; }

    public int BlogId { get; set; }
    public Blog Blog { get; set; }

    public List<Comment> Comments { get; set; } = new();
}

public class Comment
{
    public int CommentId { get; set; }

    public required string Content;

    public int PostId { get; set; }
    public Post Post { get; set; }
}

Program.cs

using EFChangeParentDeletion;
using Microsoft.EntityFrameworkCore;

Cleanup();
SetupBlogs();

using var db = new BloggingContext();
var post = db.Posts
    .Include(p => p.Blog)
    .Include(p => p.Comments)
    .FirstOrDefault(p => p.Title == "Hello Net World")!;

var blog = db.Blogs
    .FirstOrDefault(b => b.Url == "https://example.org")!;

var firstComment = post.Comments.First();

post.Blog.Posts.Remove(post);
blog.Posts.Add(post);

db.SaveChanges();
Console.WriteLine($"The comment is in state {db.Entry(firstComment).State}!");

void SetupBlogs()
{
    using var db = new BloggingContext();
    var blogNet = new Blog() { Url = "https://example.net" };
    blogNet.Posts.Add(new Post() { Title = "Hello Net World", Comments= new()
    {
        new Comment() { Content = "This post is good." },
    }});

    var blogOrg = new Blog() { Url = "https://example.org" };
    blogOrg.Posts.Add(new Post()
    {
        Title = "Hello Org World", Comments = new()
        {
            new Comment() { Content = "I like this post!" }
        }
    });

    db.Blogs.Add(blogNet);
    db.Blogs.Add(blogOrg);
    db.SaveChanges();
}
void Cleanup()
{
    using var db = new BloggingContext();
    foreach (var blog in db.Blogs.ToList())
    {
        db.Remove(blog);
    }
    db.SaveChanges();
}

This will delete the post comments and output: The comment is in state Detached!

Changing program to fetch the blog before fetching the post will not delete the comment

var blog = db.Blogs
    .FirstOrDefault(b => b.Url == "https://example.org")!;

var post = db.Posts
    .Include(p => p.Blog)
    .Include(p => p.Comments)
    .FirstOrDefault(p => p.Title == "Hello Net World")!;

and it will output:

The comment is in state Unchanged!

Include stack traces

n/a

Include verbose output

n/a

Include provider and version information

EF Core version: 7.0.4 Database provider: I've reproduced with SqlLite and SqlServer. Target framework: net7.0 Operating system: Linux IDE: Rider

DrPhil commented 1 year ago

A workaround for this is to not remove the entity from the old collection with oldBlog.Posts.Remove(post). If you need to access the posts on the old blog before SaveChanges has happened, use a filter: oldBlog.Posts.Where(p => p.Blog == oldBlog).

ajcvickers commented 1 year ago

Note for triage: this is an interesting case where reparenting an entity with required children may cause the children to be deleted before the reparenting is complete.

DrPhil commented 1 year ago

This bug has not received so much attention, so I understand that it is low on the priority list. On the other hand this causes silent data-loss in a not so weird scenario. We were able to find one of the cases where this happens, but we think there are many more hidden in our codebase (and likely others too!). Any idea of if/when this would be prioritized?

ajcvickers commented 1 year ago

This issue is in the Backlog milestone. This means that it is not planned for the next release (EF Core 8.0). We will re-assess the backlog following the this 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. Make sure to vote (👍) for this issue if it is important to you.