Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop fixing up to Deleted entities #28249

Closed
qslcna opened this issue Jun 16, 2022 · 19 comments
Closed

Stop fixing up to Deleted entities #28249

qslcna opened this issue Jun 16, 2022 · 19 comments
Assignees
Labels
breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@qslcna
Copy link

qslcna commented Jun 16, 2022

By reading this post relationship-changes. In many-to-many relationships, I consider when removing join entities PostTags from post.PostTags the removed entity will also disapper from tag.PostTags

If query tags before ctx.ChangeTracker.DetectChanges() it performs as expected.

However call ctx.ChangeTracker.DetectChanges() before query tags, the tag{id=2}'s navigation property PostTags still has {PostId: 1, TagId: 2} which state is deleted.

It dosen't disapper although I call ctx.ChangeTracker.DetectChanges() again

how to make DetectChanges has consistent result, no matter what order of invocation.

Entitis

using System.Collections.Generic;

namespace Web.Tests
{
    public class Post
    {
        public int Id { get; set; }
        public string Title { get; set; }
        public string Content { get; set; }

        public IList<PostTag> PostTags { get; } = new List<PostTag>(); // Collection navigation
    }

    public class Tag
    {
        public int Id { get; set; }
        public string Text { get; set; }

        public IList<PostTag> PostTags { get; } = new List<PostTag>(); // Collection navigation
    }

    public class PostTag
    {
        public int PostId { get; set; } // First part of composite PK; FK to Post
        public int TagId { get; set; } // Second part of composite PK; FK to Tag

        public Post Post { get; set; } // Reference navigation
        public Tag Tag { get; set; } // Reference navigation
    }
}
using Microsoft.EntityFrameworkCore;

namespace Web.Tests
{
    public class BlogContext : DbContext
    {
        public BlogContext(DbContextOptions<BlogContext> options) : base(options) { }

        public DbSet<Post> Posts { get; set; }
        public DbSet<Tag> Tags { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Post>()
                .HasData(
                    new Post { Id = 1, Title = "How to use vs?", Content = "blababa" }
                );

            modelBuilder.Entity<Tag>()
                .HasData(
                    new Tag { Id = 1, Text = ".NET" },
                    new Tag { Id = 2, Text = "C#" }

                );
            modelBuilder.Entity<PostTag>()
                .HasKey(x => new { x.PostId, x.TagId });

            modelBuilder.Entity<PostTag>()
                .HasOne(x => x.Post)
                .WithMany(x => x.PostTags);

            modelBuilder.Entity<PostTag>()
                .HasOne(x => x.Tag)
                .WithMany(x => x.PostTags);

            modelBuilder.Entity<PostTag>()
                .HasData(
                    new PostTag { PostId = 1, TagId = 1 },
                    new PostTag { PostId = 1, TagId = 2 }
                );

            base.OnModelCreating(modelBuilder);
        }
    }
}

UnitTest1.cs

        [Fact]
        public void Test1()
        {
            var ctx = CreateContext();
            var post = ctx.Posts.Include(x => x.PostTags).SingleOrDefault(x => x.Id == 1);
            if (post != null)
            {
                post.PostTags.RemoveAt(1);
            }

            
            ctx.ChangeTracker.DetectChanges();
            Debug.WriteLine(ctx.ChangeTracker.DebugView.LongView);

            var tag = ctx.Tags.Include(x => x.PostTags).ToList();
            Debug.WriteLine(ctx.ChangeTracker.DebugView.LongView);

            ctx.ChangeTracker.DetectChanges();
            Debug.WriteLine(ctx.ChangeTracker.DebugView.LongView);


            ctx.SaveChanges();
        }
 Post {Id: 1} Unchanged
   Id: 1 PK
   Content: 'blababa'
   Title: 'How to use vs?'
   PostTags: [{PostId: 1, TagId: 1}]
 PostTag {PostId: 1, TagId: 1} Unchanged
   PostId: 1 PK FK
   TagId: 1 PK FK
   Post: {Id: 1}
   Tag: <null>
 PostTag {PostId: 1, TagId: 2} Deleted
   PostId: 1 PK FK
   TagId: 2 PK FK
   Post: <null>
   Tag: <null>
 Post {Id: 1} Unchanged
   Id: 1 PK
   Content: 'blababa'
   Title: 'How to use vs?'
   PostTags: [{PostId: 1, TagId: 1}]
 PostTag {PostId: 1, TagId: 1} Unchanged
   PostId: 1 PK FK
   TagId: 1 PK FK
   Post: {Id: 1}
   Tag: {Id: 1}
 PostTag {PostId: 1, TagId: 2} Deleted
   PostId: 1 PK FK
   TagId: 2 PK FK
   Post: <null>
   Tag: {Id: 2}
 Tag {Id: 1} Unchanged
   Id: 1 PK
   Text: '.NET'
   PostTags: [{PostId: 1, TagId: 1}]
 Tag {Id: 2} Unchanged
   Id: 2 PK
   Text: 'C#'
   PostTags: [{PostId: 1, TagId: 2}]
Post {Id: 1} Unchanged
  Id: 1 PK
  Content: 'blababa'
  Title: 'How to use vs?'
  PostTags: [{PostId: 1, TagId: 1}]
PostTag {PostId: 1, TagId: 1} Unchanged
  PostId: 1 PK FK
  TagId: 1 PK FK
  Post: {Id: 1}
  Tag: {Id: 1}
PostTag {PostId: 1, TagId: 2} Deleted
  PostId: 1 PK FK
  TagId: 2 PK FK
  Post: <null>
  Tag: {Id: 2}
Tag {Id: 1} Unchanged
  Id: 1 PK
  Text: '.NET'
  PostTags: [{PostId: 1, TagId: 1}]
Tag {Id: 2} Unchanged
  Id: 2 PK
  Text: 'C#'
  PostTags: [{PostId: 1, TagId: 2}]

Include provider and version information

EF Core version:5.0.12
Database provider: Pomelo.EntityFrameworkCore.MySql
Target framework:NET 5.0
Operating system:
IDE: vscode latest

@ajcvickers
Copy link
Member

@qslcna EF Core 5.0 is no longer supported. Please update to EF Core 6.0 and report back if yo are still seeing the issue.

@qslcna
Copy link
Author

qslcna commented Jun 16, 2022

@ajcvickers I tried EF Core 6.0.6. It has same issue.

@qslcna
Copy link
Author

qslcna commented Jun 16, 2022

I want achive like this.

        [Fact]
        public void Test1()
        {
            var ctx = CreateContext();
            var post = ctx.Posts.Include(x => x.PostTags).SingleOrDefault(x => x.Id == 1);
            if (post != null)
            {
                post.PostTags.RemoveAt(1);
            }

            //ctx.ChangeTracker.DetectChanges();
            //Debug.WriteLine(ctx.ChangeTracker.DebugView.LongView);

           // But In my real project. I call  _context.ChangeTracker.Entries here. Which will trigger DetectChanges.

            var tag = ctx.Tags.Include(x => x.PostTags).ToList();
            Debug.WriteLine(ctx.ChangeTracker.DebugView.LongView);

            ctx.ChangeTracker.DetectChanges();
            Debug.WriteLine(ctx.ChangeTracker.DebugView.LongView);


            ctx.SaveChanges();
        }
Post {Id: 1} Unchanged
  Id: 1 PK
  Content: 'blababa'
  Title: 'How to use vs?'
  PostTags: [{PostId: 1, TagId: 1}]
PostTag {PostId: 1, TagId: 1} Unchanged
  PostId: 1 PK FK
  TagId: 1 PK FK
  Post: {Id: 1}
  Tag: {Id: 1}
PostTag {PostId: 1, TagId: 2} Unchanged
  PostId: 1 PK FK
  TagId: 2 PK FK
  Post: {Id: 1}
  Tag: {Id: 2}
Tag {Id: 1} Unchanged
  Id: 1 PK
  Text: '.NET'
  PostTags: [{PostId: 1, TagId: 1}]
Tag {Id: 2} Unchanged
  Id: 2 PK
  Text: 'C#'
  PostTags: [{PostId: 1, TagId: 2}]
 Post {Id: 1} Unchanged
  Id: 1 PK
  Content: 'blababa'
  Title: 'How to use vs?'
  PostTags: [{PostId: 1, TagId: 1}]
PostTag {PostId: 1, TagId: 1} Unchanged
  PostId: 1 PK FK
  TagId: 1 PK FK
  Post: {Id: 1}
  Tag: {Id: 1}
PostTag {PostId: 1, TagId: 2} Deleted
  PostId: 1 PK FK
  TagId: 2 PK FK
  Post: <null>
  Tag: {Id: 2}
Tag {Id: 1} Unchanged
  Id: 1 PK
  Text: '.NET'
  PostTags: [{PostId: 1, TagId: 1}]
Tag {Id: 2} Unchanged
  Id: 2 PK
  Text: 'C#'
  PostTags: []               // this is the point.

@ajcvickers
Copy link
Member

Note for triage: EF is fixing up navigations to Deleted entities when queries are executed. I think there are cases where this is desirable, but it'll take some investigation for me to remember.

@qslcna
Copy link
Author

qslcna commented Jun 16, 2022

@ajcvickers EF always fixing up added entities. why it behaves differently when handle deleted entities?

@qslcna
Copy link
Author

qslcna commented Jun 17, 2022

@ajcvickers How is the investigation?

@ajcvickers
Copy link
Member

@qslcna Are you saying that you want fixup to Deleted entities happen or not happen when a query is executed?

@qslcna
Copy link
Author

qslcna commented Jun 17, 2022

@ajcvickers I want it happend. The point is in many to many relationship, remove form post navigation property post.PostTags.
it should fix up tag navigation property tag.PostTags after I query tags or after I query tags call DetectChanges manually.

@ajcvickers
Copy link
Member

@qslcna You need to call DetectChanges after manipulating the collection, otherwise EF doesn't know that the collection has changed and that the entity should be marked as Deleted. In the code you posted originally this call was present, and I thought you were suggesting that fixup to Deleted entities should not occur, since conceptually, they are no longer in the graph.

See the docs on change detection: https://docs.microsoft.com/en-us/ef/core/change-tracking/change-detection

@qslcna
Copy link
Author

qslcna commented Jun 17, 2022

I'm confused for the meaning of fixup now.
For example,

      var post = ctx.Posts.Include(x => x.PostTags).SingleOrDefault(x => x.Id == 1);
      if (post != null)
      {
          post.PostTags.RemoveAt(1);
      }

I removed PostTag {PostId: 1, TagId: 2}

Then I query tags;

    var tag = ctx.Tags.Include(x => x.PostTags).ToList();

tag.PostTags should not contain the entity I removed.

In this scenario fix up happend or not?

@ajcvickers
Copy link
Member

You need to call DetectChanges after calling post.PostTags.RemoveAt(1);.

@qslcna
Copy link
Author

qslcna commented Jun 17, 2022

Please check the original code (at the top of page). I did that. But the result of query tags still has the entity PostTag {PostId: 1, TagId: 2} which state already mark as deleted. And I want the tag.PostTags should have no PostTags which state is Deleted.

@qslcna
Copy link
Author

qslcna commented Jun 17, 2022

Have I made it clear?

@qslcna
Copy link
Author

qslcna commented Jun 17, 2022

I thought you were suggesting that fixup to Deleted entities should not occur, since conceptually, they are no longer in the graph.

Yes if deleted entities no longer in the graph, it should disapper form any navigation collections.

@qslcna qslcna changed the title ef core call ChangeTracker.DetectChanges not fix up the navigtion property ef core query result navigation collection contains entity which has deleted state (using same dbcontext). Jun 17, 2022
@qslcna
Copy link
Author

qslcna commented Jun 17, 2022

@ajcvickers the fisrt part ( description about my question ) of original post has updated. Anxiously waiting for your reply. I've been struggling with this issue for a long time

@qslcna qslcna changed the title ef core query result navigation collection contains entity which has deleted state (using same dbcontext). ef core query result navigation collection contains entity which state mark as deleted (using same dbcontext). Jun 17, 2022
@ajcvickers
Copy link
Member

@qslcna Currently, executing a query causes the queried entities to be connected to any related Deleted entities, and vice-versa. I agree that this may not be the desired behavior. This is something we will investigate. However, that investigation will happen in due course as part of our normal work.

@ajcvickers ajcvickers changed the title ef core query result navigation collection contains entity which state mark as deleted (using same dbcontext). Consider not fixing up to Deleted entities on query Jun 20, 2022
@ajcvickers ajcvickers self-assigned this Jun 20, 2022
@ajcvickers ajcvickers added this to the Backlog milestone Jun 20, 2022
@qslcna
Copy link
Author

qslcna commented Jun 21, 2022

 public class Receipt
    {
        public DateTime Date { get; set; }
        public int? CustomerId { get; set; }
        public int? SupplierId { get; set; }
        public decimal Total { get; set; }

        public virtual Customer Customer { get; set; }
        public virtual Supplier Supplier { get; set; }

        public virtual List<WriteOff> WriteOffs { get; set; }
    }

    public class SaleOut
    {
        public DateTime Date { get; set; }
        public int? CustomerId { get; set; }
        public int? SupplierId { get; set; }

        public decimal Total { get; private set; }
        public decimal Progress { get; private set; }

        public virtual Supplier Supplier { get; set; }
        public virtual Customer Customer { get; set; }

        public virtual List<SaleOutItem> Items { get; set; }

        public virtual List<WriteOff> WriteOffs { get; private set; }

        public void WriteOff()
        {
            var writeOffsTotal = this.WriteOffs.Sum(x => x.Amount);
            if (writeOffsTotal < 0 || writeOffsTotal > this.Total)
            {
                throw new Exception();
            }
            this.Progress = writeOffsTotal;
        }
    }

    public class WriteOff
    {

        public decimal Amount { get; set; }

        public int TradeId { get; set; }
        public int OwnerId { get; set; }

        public virtual SaleOut Trade { get; set; }
        public virtual Receipt Owner { get; set; }
    }

Consider the situation, If Receipt.WriteOffs modified. I need update related SaleOut.Progress.

  1. I got those SaleOut ids by _context.ChangeTracker.Entries<WriteOff>().Select(x => x.Entity.TradeId);
  2. query thouse SaleOuts by ids form step 1, and invoke SaleOut.WriteOff() form each result.

if the query result still connected to Deleted Entities the Progress will not be right. BTW add WirteOff works fine. the SaleOut query result will always connected to Added Entities.

Do you have any advice on this.

@ajcvickers
Copy link
Member

@qslcna I'm afraid I find it very hard to understand the details of what you are trying to do, and hence don't really have any advice.

@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0-rc2 Sep 19, 2022
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed needs-design consider-for-current-release labels Sep 19, 2022
@ajcvickers ajcvickers changed the title Consider not fixing up to Deleted entities on query Stop fixing up to Deleted entities Sep 19, 2022
@panjonsen
Copy link

image
image
image
image
It has an endless loop, and its ideal result is the same as "Teacher=null", because I have no Include.
@ajcvickers

@ajcvickers ajcvickers modified the milestones: 7.0.0-rc2, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

No branches or pull requests

3 participants