dotnet / dotnet/efcore

Remove references to Deleted entities as soon as they are put in the Deleted state

Open
#29,486 2 comments 1 reaction 0 assignees View on GitHub
area-change-tracking breaking-change customer-reported
Dominant language
C#
Stars
14.8k
Forks
3.4k
PR merge metrics
PR metrics pending

Description

Base Classes

```cs
public class EntityBase : INotifyPropertyChanged, INotifyPropertyChanging
{
public event PropertyChangedEventHandler? PropertyChanged;
public event PropertyChangingEventHandler? PropertyChanging;
protected void RaisePropertyChanging(string propertyName)
=> PropertyChanging?.Invoke(this, new PropertyChangingEventArgs(propertyName));

protected void RaisePropertyChanged(string propertyName)
=> PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
}

public class EntityCollection : ICollection, IDisposable, INotifyCollectionChanged
{
private readonly ReaderWriterLockSlim _lock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
private readonly HashSet _internalSet;

public event NotifyCollectionChangedEventHandler CollectionChanged;

public EntityCollection()
{
_internalSet = new HashSet();
}

public EntityCollection(IEnumerable values)
{
_internalSet = new HashSet(values);
}

public int Count => LockRead(() => _internalSet.Count);

public bool IsReadOnly => LockRead(() => ((ICollection)_internalSet).IsReadOnly);

public bool Add(T item)
{
if (LockWrite(() => _internalSet.Add(item)))
{
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item));
return true;
}
else
return false;
}

public void Clear()
{
List removedItems = LockRead(() => _internalSet.ToList());
LockWrite(() => _internalSet.Clear());

OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removedItems));
}

public bool Contains(T item)
{
return LockRead(() => _internalSet.Contains(item));
}

public void CopyTo(T[] array, int arrayIndex)
{
LockRead(() => _internalSet.CopyTo(array, arrayIndex));
}

public IEnumerator GetEnumerator()
{
return LockRead(() => _internalSet.GetEnumerator());
}

public bool Remove(T item)
{
if (LockWrite(() => _internalSet.Remove(item)))
{
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item));
return true;
}
else
return false;
}

void ICollection.Add(T item)
{
Add(item);
}

IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}

private void LockWrite(Action a)
{
_lock.EnterWriteLock();
try
{
a();
}
finally
{
if (_lock.IsWriteLockHeld)
{
_lock.ExitWriteLock();
}
}
}

private R LockWrite(Func func)
{
_lock.EnterWriteLock();
try
{
return func();
}
finally
{
if (_lock.IsWriteLockHeld)
{
_lock.ExitWriteLock();
}
}
}

private void LockRead(Action a)
{
_lock.EnterReadLock();
try
{
a();
}
finally
{
if (_lock.IsReadLockHeld)
{
_lock.ExitReadLock();
}
}
}

private R LockRead(Func func)
{
_lock.EnterReadLock();
try
{
return func();
}
finally
{
if (_lock.IsReadLockHeld)
_lock.ExitReadLock();
}
}

private void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
{
CollectionChanged?.Invoke(this, e);
}

#region Dispose
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
if (_lock != null)
{
_lock.Dispose();
}
}
}

~EntityCollection()
{
Dispose(false);
}
#endregion
}
```

Entities and DbContext

```cs
public class MyDbContext : DbContext
{
public MyDbContext()
{
ChangeTracker.AutoDetectChangesEnabled = false;
}

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
base.OnConfiguring(optionsBuilder);
optionsBuilder
.UseSqlServer("Server=localhost;Database=EFCore_CollectionDeleteBehavior;Integrated Security=true;TrustServerCertificate=True");
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
base.OnModelCreating(modelBuilder);

modelBuilder.HasChangeTrackingStrategy(ChangeTrackingStrategy.ChangingAndChangedNotificationsWithOriginalValues);

modelBuilder.Entity();
modelBuilder.Entity(b =>
{
b.HasOne(x => x.Author)
.WithMany(x => x.Books)
.HasForeignKey(x => x.AuthorId);
});
}
public DbSet Authors { get; set; }
public DbSet Books { get; set; }
}

public sealed class Author : EntityBase
{
private int _id;
public int Id
{
get => _id;
set
{
if (_id != value)
{
RaisePropertyChanging(nameof(Id));
_id = value;
RaisePropertyChanged(nameof(Id));
}
}
}

private string? _name;
public string? Name
{
get => _name;
set
{
if (_name != value)
{
RaisePropertyChanging(nameof(Name));
_name = value;
RaisePropertyChanged(nameof(Name));
}
}
}

public EntityCollection Books { get; } = new EntityCollection();
}

public sealed class Book : EntityBase
{
private int _id;
public int Id
{
get => _id;
set
{
if (_id != value)
{
RaisePropertyChanging(nameof(Id));
_id = value;
RaisePropertyChanged(nameof(Id));
}
}
}

private string? _title;
public string? Title
{
get => _title;
set
{
if (_title != value)
{
RaisePropertyChanging(nameof(Title));
_title = value;
RaisePropertyChanged(nameof(Title));
}
}
}

private int? _authorId;
public int? AuthorId
{
get => _authorId;
set
{
if (_authorId != value)
{
RaisePropertyChanging(nameof(AuthorId));
_authorId = value;
RaisePropertyChanged(nameof(AuthorId));
}
}
}

private Author? _author;
public Author? Author
{
get => _author;
set
{
if (_author != value)
{
RaisePropertyChanging(nameof(Author));
_author = value;
RaisePropertyChanged(nameof(Author));
}
}
}
}
```

```cs
await using (MyDbContext dbContext = new MyDbContext())
{
Author author = new() { Name = "Mr. X" };
dbContext.Add(author);

Book book = new Book()
{
Title = "Book1",
Author = author
};

author.Books.Add(book);

if (dbContext.Entry(book).State == EntityState.Added)
Console.WriteLine("The Book would be added -> Expected behavior");

await dbContext.SaveChangesAsync();

dbContext.Remove(book);

if (dbContext.Entry(book).State == EntityState.Deleted)
Console.WriteLine("Book is marked as deleted -> Expected behavior"); // ✅
if (dbContext.Entry(author).State == EntityState.Unchanged)
Console.WriteLine("Author is unchanged and attached -> Expected behavior"); // ✅

if (author.Books.Contains(book))
Console.WriteLine("author.Books Collection contains deleted/detached Book! -> NOT EXPECTED -> Inconsistent behavior compared to adding!"); // ❌

if (book.Author != null)
Console.WriteLine("Navigation-Property not cleared -> NOT EXPECTED -> should be also NULL"); // ❌

await dbContext.SaveChangesAsync();

if (!author.Books.Contains(book))
Console.WriteLine("author.Books Collection do not contain the Book -> Expected behavior"); // ✅

if (book.Author != null)
Console.WriteLine("Navigation-Property not cleared -> NOT EXPECTED -> should be also NULL"); // ❌
}
```

We have noticed an inconsistent behavior in following mode: `AutoDetectChangesEnabled=false` and `modelBuilder.HasChangeTrackingStrategy(ChangeTrackingStrategy.ChangingAndChangedNotificationsWithOriginalValues)`.

When adding a new entity to an existing Navigation Property Collection, it would be attached (added) immediately to DbContext. We expect that the same behavior would be while deleting an entity. Look at my attached example.

### Include provider and version information

EF Core version: 7.0 RC2
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 6.0 / .NET 7.0
Operating system: Windows 11
IDE: Visual Studio 2022 17.3.6

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.