Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Property with HasJsonConversion on owned entity always sent as database Update if owned entity is replaced #5

Open
ghost opened this issue Mar 7, 2019 · 2 comments

Comments

@ghost
Copy link

ghost commented Mar 7, 2019

I'm not sure if this is an issue with the library or EF Core itself, but it may be worth being aware of at least.

If you use HasJsonConversion()on a property of an owned type and you replace the owned type (e.g. because you're using immutable value objects, so you cannot modify the properties directly), the Json property is always recognized as a change even if its property value is the same as the original so the field is always included in an update to the database (I'm using SQL Server for storage).

Extremely basic example:

public class ApplicationDbContext : DbContext
{
    public ApplicationDbContext(DbContextOptions<ApplicationDbContext> options)
        : base(options)
    { }

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

        modelBuilder.Entity<Entity>().Property(e => e.Items).HasJsonValueConversion();

        modelBuilder.Entity<Entity>().OwnsOne(e => e.Owned, b =>
        {
            b.Property(o => o.Items).HasJsonValueConversion();
        });
    }

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

public class Entity
{
    public int Id { get; set; }
    public string Field { get; set; }
    public List<string> Items { get; set; }
    public Owned Owned { get; set; }
}

public class Owned
{
    public string Field { get; set; }
    public List<string> Items { get; set; }
}

Assume you already have an entity saved in the database that is equivalent to:

new Entity 
{
    Id = 1, 
    Field = "string", 
    Items = new List<string>() { "Hello", "World!" },
    Owned = new Owned
    {
        Field = "string", 
        Items = new List<string>() { "Homer", "Simpson" }
    }
};

On the root entity itself, everything works as expected when assigning an equivalent value to the property:

// not sent as update, db value is the same
entity.Items = new List<string>() { "Hello", "World!" }; 

On the owned entity, if you reassign the property without replacing the owned object itself, again everything works as expected:

// not sent as update, db value is the same
entity.Owned.Items = new List<string>() { "Homer", "Simpson" }; 

If you replace the owned entity, then the value is always considered to be modified, even though regular values (string Field) are correctly handled:

entity.Owned = new Owned
{
    // not sent as update even though owned entity is replaced, db value is the same
    Field = "string" ,
    // SENT AS UPDATE, db value is same
    Items = new List<string>() { "Homer", "Simpson" };
}

Even if you reassign the value to the exact same reference, it's still treated as modified:

var owned = entity.Owned;
entity.Owned = new Owned
{
    // not sent as update even though owned entity is replaced, db value is the same
    Field = owned.Field,
    // SENT AS UPDATE, db value is same
    Items = owned.Items 
}

It's almost like the change tracker is not taking into consideration the ValueComparer when fixing up the owned object's properties after a reassignment. Do you think there is anything the library could do to help here?

If I could find out which properties use the JsonValueComparer/JsonValueConverter while looping through ChangeTracker.Entries I could probably fix up the properties, but I don't know how to find out if a property has a ValueConverter/ValueComparer with the metadata that is available at that point. Finding the properties with a JasonValueComparer is easy enough, but the properties are not marked as modified, so I'm not sure why they are still flowing to the database update.

@AlexEngblom
Copy link
Member

AlexEngblom commented Mar 8, 2019

Interesting. I've experienced a lot of weird activity around the EF Core owned types and their tracking, and have tried to avoid using them altogether so far. Are you able to repro this on inmemory unit test?

@ghost
Copy link
Author

ghost commented Mar 8, 2019

The owned types are indeed proving to be challenging. InMemory is hard to tell. I'm not aware of anything equivalent to a SQL statement that shows how the objects are being persisted to the in-memory database.

The pointless update call is probably not even a big deal for most cases. But it could cause false positives for update triggers or, in my case, logic that is intended to fire if any properties are modified. I think I might be able to work around that, but the original issue is still an oddity. There are other unrelated issues with nested owned types so maybe they just aren't quite ready for prime time.

I guess there's not really any action you can take on your end, other than just being aware of potential issues with owned types. I don't think the the converter or comparer are the source of this, given that everything works as expected on the root entity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant