Incorrect tracking state of loaded Entities when using ChangeTrackingStrategy.ChangedNotifications

[rwg0]

When working with ChangeTrackingStrategy.ChangedNotifications, loading related entities using

table.Where(predicate).Include(x => x.Relations).ToList()

leads to the loaded related entities having a tracking state of 'Added' rather than 'Unchanged'. Trying to SaveChanges after this has happened leads to an exception due to unique constraint failed (trying to insert the same loaded entity into the table with it's existing primary key value).

Unhandled Exception: Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details. ---> Microsoft.Data.Sqlite.SqliteException: SQLite Error 19: 'UNIQUE constraint failed: Widgets.Id'.
   at Microsoft.Data.Sqlite.Interop.MarshalEx.ThrowExceptionForRC(Int32 rc, Sqlite3Handle db)
   at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.Data.Sqlite.SqliteCommand.ExecuteDbDataReader(CommandBehavior behavior)
   at System.Data.Common.DbCommand.ExecuteReader()
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, String executeMethod, IReadOnlyDictionary`2 parameterValues, Boolean closeConnection)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteReader(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
   --- End of inner exception stack trace ---
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(Tuple`2 parameters)
   at Microsoft.EntityFrameworkCore.Storage.Internal.NoopExecutionStrategy.Execute[TState,TResult](Func`2 operation, Func`2 verifySucceeded, TState state)
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState,TResult](IExecutionStrategy strategy, Func`2 operation, TState state)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IReadOnlyList`1 entries)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()

Steps to reproduce

The following code reproduces the issue. Full code attached.

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Text;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata;

namespace ConsoleApplication10
{
    public class WidgetContext : DbContext
    {
        private static string name = Guid.NewGuid().ToString();

        public DbSet<Widget> Widgets { get; set; }
        public DbSet<Gizmo> Gizmos { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.HasChangeTrackingStrategy(ChangeTrackingStrategy.ChangedNotifications);
            base.OnModelCreating(modelBuilder);
        }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlite($"Filename={name}.db");
            base.OnConfiguring(optionsBuilder);
        }
    }

    public class Widget : INotifyPropertyChanged
    {
        private int _id;
        private string _name;
        private int _gizmoId;
        private Gizmo _gizmo;

        public int Id
        {
            get { return _id; }
            set
            {
                if (value == _id) return;
                _id = value;
                OnPropertyChanged();
            }
        }

        public string Name
        {
            get { return _name; }
            set
            {
                if (value == _name) return;
                _name = value;
                OnPropertyChanged();
            }
        }

        public int GizmoId
        {
            get { return _gizmoId; }
            set
            {
                if (value == _gizmoId) return;
                _gizmoId = value;
                OnPropertyChanged();
            }
        }

        public Gizmo Gizmo
        {
            get { return _gizmo; }
            set
            {
                if (Equals(value, _gizmo)) return;
                _gizmo = value;
                OnPropertyChanged();
            }
        }

        public event PropertyChangedEventHandler PropertyChanged;

        protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }
    }

    public class Gizmo : INotifyPropertyChanged
    {
        private int _id;
        private ObservableCollection<Widget> _widgets;

        public int Id
        {
            get { return _id; }
            set
            {
                if (value == _id) return;
                _id = value;
                OnPropertyChanged();
            }
        }

        public ObservableCollection<Widget> Widgets
        {
            get { return _widgets; }
            set
            {
                if (Equals(value, _widgets)) return;
                _widgets = value;
                OnPropertyChanged();
            }
        }

        public event PropertyChangedEventHandler PropertyChanged;

        protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }
    }


    class Program
    {
        static void Main(string[] args)
        {

            using (var context = new WidgetContext())
            {
                context.Database.Migrate();

                var widget = new Widget() {Name = "Widget1"};
                var gizmo = new Gizmo();
                widget.Gizmo = gizmo;
                context.Widgets.Add(widget);
                context.Gizmos.Add(gizmo);
                context.SaveChanges();
            }

            using (var context = new WidgetContext())
            {
                // Select the Gizmo we just made
                var gizmo = context.Gizmos.First();
                // now fetch the widgets that came with it
                var widgets = context.Gizmos.Where(x => x.Id == gizmo.Id).Include(x => x.Widgets).ToList();

                foreach (var item in context.ChangeTracker.Entries())
                {
                    Console.WriteLine($"{item.Entity.GetType()} in state {item.State}");
                }


                // oops - EF thinks the widget is added to the context
                context.SaveChanges();
            }

        }
    }
}

ConsoleApplication10.zip

Further technical details

EF Core version: 1.1.1
Database Provider: Microsoft.EntityFrameworkCore.Sqlite
Operating system: Windows 10
IDE: Visual Studio 2015

[ajcvickers]

Notes for triage: confirmed this is a bug, but not a regression.

Workaround: Stop using ChangeTrackingStrategy.ChangedNotifications for now.

[rwg0]

Using the Snapshot tracking strategy leads to parts of our code spending 95% of time in DetectChanges (we load a lot of objects to perform calculations, but only end up changing a small number of them).

I came up with this rather ugly workaround

// after loading the related Widgets
                foreach (var item in gizmo.Widgets)
                {
                    var entry = context.Entry(item);
                    if (entry.State == EntityState.Added && item.Id > 0) // state Added and a valid primary key indicates the state is incorrect for this item
                        entry.State = EntityState.Unchanged;
                }

I believe it's valid because entities that are already loaded don't seem to get their state corrupted.

[ajcvickers]

@rwg0 That seems like a reasonable workaround as well.

[ajcvickers]

Verified that this repros in 1.2, but is fixed in 2.0.