Updates using custom value comparer not working when property is ValueGenerated.OnAdd

[ajcvickers]

Originally posted by @samburovkv at #17471 (comment)

---

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 This works if I remove the line .HasDefaultValue(new List<int>()). Is there a specific reason you are using this configuration?

Note for team: This fails with anything that sets the value generation; it doesn't have to be HasDefaultValue.

[ajcvickers]

This is working on the the latest daily build. Likely a duplicate of #13507.

[samburovkv]

Thank you, @ajcvickers! Yeah. There is specific reason why I used .HasDefaultValue(new List<int>()). If it uses without default value, then the migration tool will generate the following code:

 public partial class NewMigration : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.AddColumn<string>(
                name: "Numbers",
                table: "Persons",
                nullable: false,
                defaultValue: "");
        }
    }

After that, if we try to make migration, tool will write empty string for old rows in database.
And the following code will be broken:

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

Because json parser expects a valid JSON token. In my case empty array.

[ajcvickers]

@samburovkv Thanks, that makes sense.

[rioka]

@ajcvickers
Have a similar scenario.

Here is relevant code to define model and configure EF

public class Client
{
  public int Id { get; private set; }

  public string Name { get; set; }

  public IReadOnlyList<string> Currencies => _currencies.AsReadOnly();

  private List<string> _currencies { get; set; }

  public Client(string name) {

    Name = name;
    _currencies = new List<string>();
  }

  public void SetCurrencies(params string[] currencies) {

    _currencies.Clear();
    _currencies.AddRange(currencies);
  }
}

public class ClientConfiguration : IEntityTypeConfiguration<Client>
{
  public void Configure(EntityTypeBuilder<Client> builder) {

    builder
      .Property(c => c.Name)
      .IsRequired()
      .HasMaxLength(100);

    builder
      .Property<List<string>>("_currencies")
      .HasColumnName(nameof(Client.Currencies))
      .HasMaxLength(100)
      .HasConversion(
        v => JsonConvert.SerializeObject(v),
        v => JsonConvert.DeserializeObject<List<string>>(v))
      .Metadata.SetValueComparer(new ValueComparer<List<string>>(
        (l1, l2) => l1.SequenceEqual(l2),
        l => l.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
        // does not work
        l => l/*.ToList()*/));
  }
}

And then this is some sample code

var client = new Create("Client " + Guid.NewGuid());
client.SetCurrencies("EUR", "GBP");

ctx.Clients.Add(client);
ctx.SaveChanges();

// ... later

client.SetCurrencies("USD");
ctx.SaveChanges();

In this sample code, updates are not detected, unless I remove ToList() in l => l.ToList(); I was wondering... since l is already a list, why is the additional call to ToList required?

thx

[ajcvickers]

@rioka I am not able to reproduce this. I find updates are not detected unless I have the ToList call in place, as expected. (The ToList call is needed so that we get a snapshot of the list which will not change when items are added or removed from the original list.)

Note that the issue for which this is a duplicate is fixed in 5.0. If you're still seeing an issue with the latest 5.0 preview release, then please open a new issue and attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

[rioka]

@ajcvickers

I am not able to reproduce this

sorry my bad... I started writing "unless I uncomment...", then decided to write "if I remove...", but left a mix of both...

The ToList call is needed so that we get a snapshot...

This was actually what I was missing

thx

[furier]

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();

This worked for me, when the property was a List<T>

c => c.ToHashSet().ToList()

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.ToHashSet().ToList()));
    }
} 

[OlegTontici]

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();

This worked for me, when the property was a List<T>

c => c.ToHashSet().ToList()

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.ToHashSet().ToList()));
    }
} 

.ToList() helps because this way we return a new reference as snapshot.
If a deep clone is not performed you basically return the initial reference to that List, which is then - modified(add/remove), therefore when it compares them to see if there is a difference - it compares the same object(reference), obviously they will be identical.