Duplicated entries in navigation collection if Equals and GetHashCode are overridden

[leszekrogowski]

If you override Equals and GetHashCode in entity it'll cause ChangeTracker strange behaviour for navigation collection. Child entities get duplicated when you're adding new entities to the collection.

Steps to reproduce

Please consider the following code:

namespace EfCoreCollectionsTest
{
    public class EntityA
    {
        public int Id { get; set; }

        [Required]
        [StringLength(100)]
        public string Name { get; set; }

        public ICollection<EntityB> EntityBs { get; set; } = new HashSet<EntityB>();
    }

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

        [Required]
        [StringLength(100)]
        public string Name { get; set; }

        public int EntityAId { get; set; }

        public EntityA EntityA { get; set; }

        // Please comment Equals and GetHashCode methods and the "duplicate" bug will disappear
        public override bool Equals(object obj)
        {
            if (obj == null || !(obj is EntityB))
            {
                return false;
            }

            var other = (EntityB)obj;
            return Id.Equals(other.Id);
        }

        public override int GetHashCode()
        {
            return Id.GetHashCode();
        }
    }

    public class SampleDbContext : DbContext
    {
        public virtual DbSet<EntityA> EntityAs { get; set; }

        public virtual DbSet<EntityB> EntityBs { get; set; }

        public SampleDbContext()
        {
        }

        public SampleDbContext(DbContextOptions<SampleDbContext> options)
            : base(options)
        {
        }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            base.OnConfiguring(optionsBuilder);

            optionsBuilder.UseInMemoryDatabase(Guid.NewGuid().ToString());
        }

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

            modelBuilder.Entity<EntityB>()
                .HasOne(e => e.EntityA)
                .WithMany(e => e.EntityBs)
                .HasForeignKey(e => e.EntityAId);
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            using (var db = new SampleDbContext())
            {
                db.Database.EnsureDeleted();
                db.Database.EnsureCreated();

                var entityA = new EntityA();
                entityA.Name = "A";
                entityA.EntityBs.Add(new EntityB
                {
                    Name = "B"
                });
                db.EntityAs.Add(entityA);
                db.SaveChanges();
                // Shows: 2 instead of 1
                Console.WriteLine($"Number of child entities: {entityA.EntityBs.Count}");

                var entityB2 = new EntityB
                {
                    Name = "B2",
                    EntityAId = entityA.Id,
                };
                entityA.EntityBs.Add(entityB2);
                db.SaveChanges();
                // Shows: 4 instead of 2
                Console.WriteLine($"Number of child entities: {entityA.EntityBs.Count}");
            }

            Console.ReadKey();
        }
    }
}

Further technical details

EF Core version: 2.2.4
Database Provider: Checked on Microsoft.EntityFrameworkCore.InMemory and Microsoft.EntityFrameworkCore.SqlServer
Operating system:
IDE: Visual Studio 2017 15.9.11

[ajcvickers]

@leszekrogowski There are a couple of issues here. First, EF Core uses reference equality for entities, and requires that only one entity instance is tracked for any given entity identity. This means that entities that override Equals and GetHashCode must still ensure that only a single instance is tracked. Further, navigation properties that EF interacts with must also respect these semantics. In the code above, the navigation property is created as a simple HashSet, not using reference equality, and so does not honor this.

Second, for many things (including common .NET collections like HashSet) do not work correctly if the hash value for an object can change during the object's lifetime. (This is not an EF limitation.) Defining equality to use the key value satisfies this condition only if the key value for an entity instance never changes after the instance is created. This is not true for the code above since store-generated keys will be set at persistence time, thereby changing the identity of an object while it is stored in a collection.

[douglasg14b]

@ajcvickers Thanks for that explanation. I don't quite understand WHY they are duplicated in the HashSet, in memory, but I understand how to avoid it now.

[James-Jackson-South]

@ajcvickers Can we get some documentation and sample code on how to correctly implement equality operators, hashcodes when overriding in EF Core? Using the default code generation in Visual Studio only seems to make the matter worse.

[ajcvickers]

@James-Jackson-South This is a good start: https://ericlippert.com/2011/02/28/guidelines-and-rules-for-gethashcode/