Some added entities are removed from SortedSet after SaveChanges()

[iokill]

Hi!

I ran into another issue with EF Core 2.1 and SortedSet
(I did not check if this also occurs on 2.2 since I'm stuck on 2.1 for now).

Let's consider the following model where Level1.Children is a SortedSet
of Level2 entries. The set sorts its entries by Level2.Name.

public class Level1
{
    public int Id { get; set; }

    [Required]
    public ICollection<Level2> Children { get; set; } = new SortedSet<Level2>();
}

public class Level2 : IComparable<Level2>
{
    public int Id { get; set; }
    public int Radius { get; set; }
    public string Name { get; set; }

    public Level1 Level1 { get; set; }
    public int Level1Id { get; set; }

    public int CompareTo(Level2 other)
    {
        return StringComparer.InvariantCultureIgnoreCase.Compare(Name, other.Name);
    }

    public override bool Equals(object obj)
    {
        if (obj is null || GetType() != obj.GetType())
        {
            return false;
        }

        var other = (Level2)obj;
        return Id == other.Id
            && Radius == other.Radius
            && Name.Equals(other.Name, StringComparison.InvariantCultureIgnoreCase);
    }

    public override int GetHashCode()
    {
        return Tuple.Create(Id, Radius, Name).GetHashCode();
    }
}

My code loads an existing Level1 entity (with related data), does lvl1.Children.Clear()
and (re-)adds some Level2 entries. After calling SaveChanges(), lvl1.Children
is missing some entries. More specifically, it looses those entries there existed before, but were removed before adding new Level2 entries.

Here's the corresponding code:

var lvl1 = context.Level1
    .Include(l1 => l1.Children)
    .First();

/* At this point, lvl1.Children contains one Level2 entry with Name="Foo" and Radius=10 */

lvl1.Children.Clear();
lvl1.Children.Add(new Level2 { Name = "Foo", Radius = 10 });
lvl1.Children.Add(new Level2 { Name = "Quz", Radius = 30 });
context.SaveChanges();

/* Now, lvl1.Children will only contain the entry with Name="Quz".
  I would expect that it still contains both added entries from above! */

There are a few things that I found out during testing:

• If I remove the property Level2.Level1Id, the problem goes away and the collection
  contains the expected amount of entries after SaveChanges().

• If I modify Level2.CompareTo(other) such that it only ever returns 0 when this.Equals(other), the issues also goes away.

To me, this looks like an issue with change tracking in EF Core, since simply removing
the property Level2.Level1Id gets rid of the issue. But since I did not do a deep dive into the EFCore internals, I'm just guessing 😉

Steps to reproduce

I've pushed a test case here: https://github.com/iokill/EFCoreCollectionSaveBug/blob/master/EFCoreSortedSetTestCase/Program.cs

Further technical details

EF Core version: 2.1.11
Database Provider: Microsoft.EntityFrameworkCore.Sqlite
Operating system: macOS 10.14.5

[divega]

@ajcvickers does this ring any bells?

[ajcvickers]

@divega Yes. It looks like unstable, non-reference equality. See #15687

[iokill]

@ajcvickers Thanks for the hint! 😁 If I understood your comment in #15687 correctly, this should be the correct way to override Equals and GetHashCode:

public class Level1
{
    public int Id { get; set; }

    [Required]
    public ICollection<Level2> Children { get; set; } = new SortedSet<Level2>();
}

public class Level2 : IComparable<Level2>
{
    public int Id { get; set; }
    public int Radius { get; set; }
    public string Name { get; set; }

    public Level1 Level1 { get; set; }
    public int Level1Id { get; set; }

    public int CompareTo(Level2 other)
    {
        return StringComparer.InvariantCultureIgnoreCase.Compare(Name, other.Name);
    }

    public override bool Equals(object obj)
    {
        if (obj is null || GetType() != obj.GetType())
        {
            return false;
        }

        var other = (Level2)obj;
        return ReferenceEquals(this, obj)
            || Radius == other.Radius
            && Name.Equals(other.Name, StringComparison.InvariantCultureIgnoreCase);
    }

    public override int GetHashCode()
    {
        return HashCode.Combine(Radius, Name);
    }
}

This however still results in the same issue for me...

[ajcvickers]

@iokill Have you verified that EF is not given multiple instances that both represent the same entity? That is, if two instances compare equal but are both used with the same context instance, then things will break. EF can only track a single instance for a given entity.

[iokill]

@ajcvickers Hmm... that is definitely an issues with the second Level2.Equals() method I posted above!

However, I now completely removed the custom Level2.Equals() and Level2.GetHashCode() methods and the issue still persists! So am I right in assuming that something other than my broken Equals() implementation is (also) causing problems here?

Also, is there some documentation on the requirements EF Core has on Equals()? Or is it simply that in addition to the usual requirements on Equals(), a single context must not get multiple instances that compare equal?

Thanks!

[ajcvickers]

@iokill Thanks for being patient. I was able to reproduce this on 2.2.4, so it looks like a potential bug.

Or is it simply that in addition to the usual requirements on Equals(), a single context must not get multiple instances that compare equal?

Yes, this is correct. We should document it better. /cc @divega

[divega]

I have created dotnet/EntityFramework.Docs#1541. Feel free to add more details to it.