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

State of join entity entry is "Added" although entity is tracked by setting State="Modified". #28005

Open
GeroIwan opened this issue May 11, 2022 · 2 comments
Labels
area-change-tracking blocked customer-reported punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. punted-for-8.0 Originally planned for the EF Core 8.0 (EF8) release, but moved out due to resource constraints. type-bug
Milestone

Comments

@GeroIwan
Copy link

After updating from EF Core 5 to EF Core 6, some of our unit tests failed. I couldn't figure out by which of the known breaking changes this might be caused, but found the changed behavior in a method that uses stubs to remove an entry from a many-to-many relation. In EF Core 5, the state of the join entity entry changes from Added to Deleted, while in EF Core 6, it changes from Added to Detached. (The "new" behavior seems to be "more correct".) As I tried to solve our remove-problem, I wondered:
Why is the state of the join entity entry Added at all?

odd behavior

The involved stubs are only made tracked in Unchanged or Modified state. But the state of the join entity entry is Unchanged or Added. Particularly strange: Whether it is Unchanged or Added depends on the order in which the involved stubs are made tracked or whether a stub is first made Unchanged and then Modified right after.
In the following code, these three cases are covered (see foreach and switch). Furthermore, the code contains tables showing the state of the join entity entry and the final result for each of these three cases, both for EF Core 5 and EF Core 6.
Is there a "good reason" for the way these three cases behave, and that not all behave the same?

runnable code

using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.ObjectModel;
using System.Linq;

namespace MyNamespace
{
    public class Program
    {
        private static void Main(string[] args)
        {
            // Define whether to prepare user entry state assignment by another user entry state assignment
            // or whether to defer role entry state assignment after user entry state assignment.
            foreach (var defer in new[] { (Boolean?)null, false, true })
            {
                Console.WriteLine("=== prepare user entry state assignment by another user entry state assignment: {0}", defer == null);
                Console.WriteLine("=== defer role entry state assignment after user entry state assignment:        {0}", defer == true);
                Console.WriteLine();

                // Populate database.
                using (var context = new MyContext())
                {
                    context.Database.EnsureDeleted();
                    context.Database.EnsureCreated();
                    context.Users.Add(new User() { RolesChangedAt = default, Roles = { new Role(), new Role() } });
                    context.SaveChanges();
                }

                // Remove relation between user with ID 1 and role with ID 1 via stubs. (Only IDs have to be known.)
                using (var context = new MyContext())
                {
                    // Create stubs, and assign their entity entry state.
                    var role = new Role() { Id = 1 };
                    var user = new User() { Id = 1, RolesChangedAt = DateTime.UtcNow, Roles = { role } };
                    switch (defer)
                    {
                        case null:
                            context.Entry(role).State = EntityState.Unchanged;
                            context.Entry(user).State = EntityState.Unchanged;
                            context.Entry(user).State = EntityState.Modified;
                            break;
                        case false:
                            context.Entry(role).State = EntityState.Unchanged;
                            context.Entry(user).State = EntityState.Modified;
                            break;
                        case true:
                            context.Entry(user).State = EntityState.Modified;
                            context.Entry(role).State = EntityState.Unchanged;
                            break;
                    }

                    // Report entity entries before removing role.
                    context.ChangeTracker.DetectChanges();
                    Console.WriteLine("before removing role:");
                    Console.WriteLine(context.ChangeTracker.DebugView.ShortView);
                    /* The following table shows the state of the join entity entry:
                     *                | EF Core 5 | EF Core 6 |
                     * ---------------+------------------------------------------
                     * defer == null  | Unchanged | Unchanged |
                     * defer == false | Added     | Added     |   <= odd behavior: Why "Added" here?
                     * defer == true  | Unchanged | Unchanged |
                     */

                    // Remove role.
                    user.Roles.Remove(role);

                    // Report entity entries after removing role.
                    context.ChangeTracker.DetectChanges();
                    Console.WriteLine("after removing role:");
                    Console.WriteLine(context.ChangeTracker.DebugView.ShortView);
                    /* The following table shows the state of the join entity entry:
                     *                | EF Core 5 | EF Core 6 |
                     * ---------------+-----------+-----------+
                     * defer == null  | Deleted   | Deleted   |
                     * defer == false | Deleted   | None      |   <= changed behavior (for the better IMHO)
                     * defer == true  | Deleted   | Deleted   |
                     * where "None" means that there is no join entity entry any more (i.e., "None" means "Detached").
                     * (Reason: When an entity is in the Added state then removing it will disconnect it rather than mark it for deletion.)
                     */

                    context.SaveChanges();
                }

                // Check whether there is a relation between user with ID 1 and role with ID 1.
                using (var context = new MyContext())
                {
                    var user = context.Users.Include(x => x.Roles).FirstOrDefault(x => x.Id == 1);
                    Console.WriteLine("result:");
                    Console.WriteLine("date changed: {0}", user.RolesChangedAt != default);
                    Console.WriteLine("role removed: {0}", user.Roles.All(x => x.Id != 1));
                    Console.WriteLine();
                    /* The following table shows whether the role is removed from the user's role collection:
                     *                | EF Core 5 | EF Core 6 |
                     * ---------------+-----------+-----------+
                     * defer == null  | True      | True      |
                     * defer == false | True      | False     |   <= consistent result of changed behavior
                     * defer == true  | True      | True      |
                     */
                }
            }
        }

        public class MyContext : DbContext
        {
            public DbSet<User> Users { get; private set; }
            public DbSet<Role> Roles { get; private set; }

            protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder.UseSqlServer(@"Server=(LocalDB)\MSSQLLocalDB;Database=EfCore6-Problem-Db");
        }

        public class User
        {
            public Int32 Id { get; set; }
            public DateTime RolesChangedAt { get; set; }
            public Collection<Role> Roles { get; } = new();
        }

        public class Role
        {
            public Int32 Id { get; set; }
            public Collection<User> Users { get; } = new();
        }
    }
}

console output

=== prepare user entry state assignment by another user entry state assignment: True
=== defer role entry state assignment after user entry state assignment:        False

before removing role:
Role {Id: 1} Unchanged
User {Id: 1} Modified
RoleUser (Dictionary<string, object>) {RolesId: 1, UsersId: 1} Unchanged FK {RolesId: 1} FK {UsersId: 1}

after removing role:
Role {Id: 1} Unchanged
User {Id: 1} Modified
RoleUser (Dictionary<string, object>) {RolesId: 1, UsersId: 1} Deleted FK {RolesId: 1} FK {UsersId: 1}

result:
date changed: True
role removed: True

=== prepare user entry state assignment by another user entry state assignment: False
=== defer role entry state assignment after user entry state assignment:        False

before removing role:
Role {Id: 1} Unchanged
User {Id: 1} Modified
RoleUser (Dictionary<string, object>) {RolesId: 1, UsersId: 1} Added FK {RolesId: 1} FK {UsersId: 1}

after removing role:
Role {Id: 1} Unchanged
User {Id: 1} Modified

result:
date changed: True
role removed: False

=== prepare user entry state assignment by another user entry state assignment: False
=== defer role entry state assignment after user entry state assignment:        True

before removing role:
Role {Id: 1} Unchanged
User {Id: 1} Modified
RoleUser (Dictionary<string, object>) {RolesId: 1, UsersId: 1} Unchanged FK {RolesId: 1} FK {UsersId: 1}

after removing role:
Role {Id: 1} Unchanged
User {Id: 1} Modified
RoleUser (Dictionary<string, object>) {RolesId: 1, UsersId: 1} Deleted FK {RolesId: 1} FK {UsersId: 1}

result:
date changed: True
role removed: True

provider and version information

EF Core version: 6.0.5
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 6.0
Operating system: Windows 11
IDE: Visual Studio 2022 17.1.6

@GeroIwan
Copy link
Author

Could this be related to #27555?

@ajcvickers
Copy link
Contributor

Notes for triage: after investigating this I came to the conclusion that while there is a bug here, fixing it is tricky, and likely requires #27677 to be implemented.

The main problem is determining what state join entities should be in when there are synthesized from attached entities on either end.

If either end is Added, then the join entity must also be Added, since it can't already exist in the database if one or other end does not exist.

If both ends are Unchanged or Modified, then we don't know if the join entity already exists in the database or not. If we assume that it doesn't exist and mark the join entity as Unchanged, then, if we assuming wrongly, we will fail to create the relationship in the database. If we assume that that it does not exist and mark the join entity has Added, then SaveChanges will fail if we got this wrong.

Currently, we use the Modified state of the entities on either end to choose either Unchanged or Added for the join entity. However, there are two problems with this:

  • The join entity is only Added if the entity it is discovered from is Modified, not if the entity on the other end is Modified. This is a bug, and is what this issue specifically is tracking.
  • There is nothing about the Modified state of either end that gives a good indication that the join entity is new. This is bad logic

Ideally, we should do an upsert here (#4526). That is, put the join entity into a state where is is inserted if it does not exist, and updated (which will usually actually be a no-op) if it does exist.

If we change the behavior here, we will certainly break people:

  • Code that currently works because the state of the join entity is set to Unchanged will break if after we "fix" this issue it becomes Added.
  • Code that currently works because the state of the join entity is set to Added will break if after we "fix" this issue it becomes Unchanged.

So, I think we should leave the behavior as is for 7.0, and revisit as part of #27677 and possibly #4526 in 8.0.

@ajcvickers ajcvickers removed this from the 7.0.0 milestone Sep 9, 2022
@ajcvickers ajcvickers added this to the Backlog milestone Sep 13, 2022
@ajcvickers ajcvickers added punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. and removed propose-punt labels Sep 13, 2022
@ajcvickers ajcvickers modified the milestones: Backlog, 8.0.0 Dec 2, 2022
@ajcvickers ajcvickers added the punted-for-8.0 Originally planned for the EF Core 8.0 (EF8) release, but moved out due to resource constraints. label Sep 6, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0, Backlog Sep 6, 2023
@ajcvickers ajcvickers removed their assignment Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-change-tracking blocked customer-reported punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. punted-for-8.0 Originally planned for the EF Core 8.0 (EF8) release, but moved out due to resource constraints. type-bug
Projects
None yet
Development

No branches or pull requests

2 participants