Efcore: Change tracking not working for collection properties with value conversion defined

[BalintBanyasz]

My entity has a collection of SmartEnums (simplified in the code example below) which is persisted in the database using a ValueConverter in order to keep the domain model clean (I wanted to avoid introducing a wrapper entity with an extra ID property). It works correctly for retrieving data but unfortunately doesn't seem to pick up changes to the collection automatically. When calling SaveChanges, the changes are not persisted unless the entity state is manually set to EntityState.Modified beforehand.

Steps to reproduce

MySmartEnum
```c#
public class MySmartEnum
{
public string Value { get; set; }

    public static MySmartEnum FromValue(string value)
    {
        return new MySmartEnum { Value = value };
    }
}

```c#
public class Entity
    {
        public Entity()
        {
            SmartEnumCollection = new HashSet<MySmartEnum>();
        }

        public int Id { get; set; }
        public virtual ICollection<MySmartEnum> SmartEnumCollection { get; set; }
    }

Context
```c#
public class TestContext : DbContext
{
public TestContext()
{ }

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

    public virtual DbSet<Entity> Entities { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        var valueConverter = new ValueConverter<ICollection<MySmartEnum>, string>
        (
            e => string.Join(',', e.Select(c => c.Value)),
            str => new HashSet<MySmartEnum>(
                        str.Split(new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries)
                           .Select(x => MySmartEnum.FromValue(x)))
        );

        modelBuilder.Entity<Entity>()
            .Property(e => e.SmartEnumCollection)
            .HasConversion(valueConverter);
    }
}

Update method
```c#
using (var context = new TestContext(options))
            {
                var entity = context.Entities
                    .First();

                entity.SmartEnumCollection.Add(MySmartEnum.FromValue("Test"));

                // Changes are persisted only if the following line is uncommented:
                //context.Entry(entity).State = EntityState.Modified;

                context.SaveChanges();
            }

Further technical details

EF Core version: 3.0.0-rc1.19427.8
Database Provider: Microsoft.EntityFrameworkCore.SqlServer (Version 3.0.0-rc1.19427.8)
Operating system: Windows 10 (Version 10.0.18362.295)
IDE: Visual Studio 2019 16.2.3

[ajcvickers]

@BalintBanyasz The issue here is that EF Core needs to be able to create a snapshot of the current value and then compare that snapshot with the new value to see if it has changed. This requires special code when using value conversion to map a type with significant structure--in this case the ICollection<>.

The fix for this is to tell EF how to snapshot and compare these collections. For example:
```C#
var valueComparer = new ValueComparer>(
(c1, c2) => c1.SequenceEqual(c2),
c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
c => (ICollection)c.ToHashSet());

modelBuilder.Entity()
.Property(e => e.SmartEnumCollection)
.HasConversion(valueConverter)
.Metadata.SetValueComparer(valueComparer);
```

Notes for triage: we should consider:

• Documenting how to do this--filed https://github.com/aspnet/EntityFramework.Docs/issues/1680
• If a simple property is a collection (or maybe just an IEnumerable), then we could automatically use a value comparer like shown above.

[ajcvickers]

Note from triage:

• Warn about this case in 3.1
• Consider shipping well-known specific value comparers
• In future, decide if we should do this automatically and also:
  
  
  
  • Can we detect IEquatable or IComparable and use it, only generating the snapshotting?
  
  

[BalintBanyasz]

@BalintBanyasz The issue here is that EF Core needs to be able to create a snapshot of the current value and then compare that snapshot with the new value to see if it has changed. This requires special code when using value conversion to map a type with significant structure--in this case the ICollection<>.

The fix for this is to tell EF how to snapshot and compare these collections. For example:

var valueComparer = new ValueComparer<ICollection<MySmartEnum>>(
    (c1, c2) => c1.SequenceEqual(c2), 
    c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
    c => (ICollection<MySmartEnum>)c.ToHashSet());

modelBuilder.Entity<Entity>()
    .Property(e => e.SmartEnumCollection)
    .HasConversion(valueConverter)
    .Metadata.SetValueComparer(valueComparer);

Notes for triage: we should consider:

• Documenting how to do this--filed aspnet/EntityFramework.Docs#1680
• If a simple property is a collection (or maybe just an IEnumerable), then we could automatically use a value comparer like shown above.

Thank you @ajcvickers. It works perfectly with the ValueComparer and snapshotExpression configured.

[ajcvickers]

3.1 part of this (warning) split out into #18600

[petrmosna15]

Fox me the ValueComparer was not working, I had to remove ToHashSet() in the snapshotExpression

[coolhome]

+1 for better documentation and +1 for an easier solution.

I have not tested this but I think a string backing field could be used in junction with a getter/setter that will serialize/deserialize to and from the backing field.

Either way both are less than ideal imho.

[samburovkv]

I tried to follow @ajcvickers's advice. But my code doesn't work properly.
My code:

public class Person
{
    public string Id { get; set; }

    public virtual List<int> Numbers { get; set; }

    public Person()
    {
        Id = Guid.NewGuid().ToString();
        Numbers = new List<int>();
    }
}

public class PersonEntityTypeConfiguration : IEntityTypeConfiguration<Person>
    {
        internal static readonly ValueConverter<List<int>, string> Converter
            = new ValueConverter<List<int>, string>(
                v => JsonSerializer.Serialize(v, null),
                v => JsonSerializer.Deserialize<List<int>>(v, null));

        public void Configure(EntityTypeBuilder<Person> builder)
        {
            builder.HasKey(x => x.Id);
            builder
                .Property(x => x.Numbers)
                .HasDefaultValue(new List<int>())
                .IsRequired()
                .HasConversion(Converter)
                .Metadata.SetValueComparer(new ValueComparer<List<int>>(
                    (c1, c2) => c1.SequenceEqual(c2),
                    c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
                    c => c.ToList()));
        }
    }

var person = dbContext.Persons.FirstOrDefault(x => x.Id == "6c7a5476-0266-459e-8229-544483379108");
person.Numbers.Add(98);
await dbContext.SaveChangesAsync();

I get "person" entity from database then I add new item to "Numbers" list. But SaveChanges doesn't save new values in the collection to database. I have test project with test database https://github.com/samburovkv/EfSQLiteTests

I'd like to know how this feature works.
This code works:

var person = dbContext.Persons.FirstOrDefault(x => x.Id == "6c7a5476-0266-459e-8229-544483379108");

person.Numbers = new List<int>(person.Numbers);
person.Numbers.Add(98); 
await dbContext.SaveChangesAsync();

[ajcvickers]

@samburovkv I moved this to a new issue: #21986