Update of owned entity fails

[dweggemans]

If I update an owned entity a call to SaveChanges crashes with the exception below.

System.InvalidOperationException: The instance of entity type 'Person.Address#Address' cannot be tracked because another instance with the same key value for {'PersonId'} is already being tracked. When attaching existing entities, ensure that only one entity instance with a given key value is attached. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the conflicting key values.
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(TKey key, InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.StartTracking(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState oldState, EntityState newState, Boolean acceptChanges)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Boolean forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode node)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph(EntityEntryGraphNode node, Func`2 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraph(InternalEntityEntry rootEntry, EntityState entityState, Boolean forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.NavigationFixer.NavigationReferenceChanged(InternalEntityEntry entry, INavigation navigation, Object oldValue, Object newValue)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntryNotifier.NavigationReferenceChanged(InternalEntityEntry entry, INavigation navigation, Object oldValue, Object newValue)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.ChangeDetector.DetectNavigationChange(InternalEntityEntry entry, INavigation navigation)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.ChangeDetector.DetectChanges(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.ChangeDetector.DetectChanges(IStateManager stateManager)
   at Microsoft.EntityFrameworkCore.ChangeTracking.ChangeTracker.DetectChanges()
   at Microsoft.EntityFrameworkCore.DbContext.TryDetectChanges()
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()

class Program
    {
        static void Main(string[] args)
        {
            try
            {
                int id;
                using (var context = new TestDbContext())
                {
                    context.Database.Migrate();

                    var result = context.Persons.Add(new Person("John Doe", new Address("SomeStreet")));
                    context.SaveChanges();
                    id = result.Entity.Id;
                }

                using (var context = new TestDbContext())
                {
                    var person = context.Find<Person>(id);

                    person.Move(new Address("OtherStreet"));

                    context.SaveChanges();
                }
            }
            catch (Exception e)
            {
                Console.WriteLine(e);
                Console.ReadLine();
            }
        }
    }

    public class TestDbContext : DbContext
    {
        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer("Server=.;Database=OwnOneTest;Integrated Security=SSPI");
            base.OnConfiguring(optionsBuilder);
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Person>().OwnsOne(x => x.Address);

            base.OnModelCreating(modelBuilder);
        }

        public DbSet<Person> Persons { get; set; }
    }

    public class Person
    {
        [Obsolete("ORM constructor", true)]
        protected Person() {}

        public Person(string name, Address address)
        {
            Name = name;
            Address = address;
        }

        public int Id { get; private set; }

        public string Name { get; private set; }
        public Address Address { get; private set; }

        public void Move(Address address)
        {
            Address = address ?? throw new ArgumentNullException(nameof(address));
        }
    }

    public class Address
    {
        [Obsolete("ORM constructor", true)]
        protected Address() { }

        public Address(string street)
        {
            Street = street;
        }

        public string Street { get; private set; }
    }

If this is not the correct way to map DDD Entities and Value Objects, then how should I do it?

[AndriySvyryd]

This is a subcase of #7340

[AndriySvyryd]

Impact: This is a common scenario for owned entities and the way to fix it is not obvious
Risk: None. The fix only changes the exception thrown.

[ajcvickers]

EF Triage: we recognize that this is an important scenario, but unfortunately there is too much risk of regressions to try to fix in a patch release. Therefore, we plan to address it in 2.1 via #7340. The workarounds for now are

• To mutate the existing owned object rather than setting a new one. For example, on Person:

public void Move(Address address)
{
    if (address == null)
    {
        throw new ArgumentNullException(nameof(address));
    }
    
    Address.UpdateFrom(address);
}
And on Address:
```C#
internal void UpdateFrom(Address other)
{
    Street = other.Street;
}

• To manually detach the existing owned object before setting a new one. For example:

context.Entry(person.Address).State = EntityState.Detached;
person.Move(new Address("OtherStreet"));
context.Entry(person.Address).State = EntityState.Modified;

[Eilon]

This patch bug is approved for the 2.0.x patch. Please send a PR to the feature/2.0.1 branch and get it reviewed and merged. When we have the rel/2.0.1 branches ready please port the commit to that branch.

[andriysavin]

Could somebody clarify what exactly will be fixed in the 2.0.x patch? Will it be a real fix or just "throwing better exception when replacing an owned entity"?
BTW, in EF6 this scenario works as expected (with complex types).

[ajcvickers]

@andriysavin The patch will only include a better exception message. We recognize that there is a significant gap in functionality here, but unfortunately it can't be fixed in a patch release. We're working on it for 2.1.

[julielerman]

ok, at least I know I'm not going crazy ... I'll just simplify my demo for now.
Also, FWIW, using the example above, I'm still getting that error when trying to manually detach. But I'm going to let this one go for now.

[andriysavin]

Just to note: while workaround @ajcvickers suggested may work, it breaks Value Object immutability, which is the key trait of this pattern in DDD. Some assumptions made by consuming code may break after such change.

[julielerman]

@andriysavin absolutely. I tried the second one - manual detach . Did not go turn the value object into a franken-thingy :)

[andriysavin]

@julielerman I don't even consider detaching approach since it requires mixing domain/business logic with repository implementation details. Though in many cases this may be less evil than making value object mutable and allowing to spread dependency on this.

[julielerman]

separation of concerns for the win. I'm doing it in a service. EIther way, it didn't work.

[Eilon]

Hi, we have a public test feed that you can use to try out the ASP.NET/EF Core 2.0.3 patch!

To try out the pre-release patch, please refer to the following guide:

• ASP.NET/EF Core 2.0.3 patch pre-release test feed aspnet/Announcements#274 (info)
• ASP.NET/EF Core 2.0.3 patch pre-release test feed - discussion aspnetcore#2254 (discussions and questions)

We are looking for feedback on this patch. We'd like to know if you have any issues with this patch by updating your apps and libraries to the latest packages and seeing if it fixes the issues you've had, or if it introduces any new issues. If you have any issues or questions, please reply on this issue to let us know as soon as possible.

Thanks,
Eilon

[andriysavin]

Hi @Eilon , this feature seems to be working ok in EF Core 2.1 RC1 with 1 level of owned types nesting. However, if you have 2 nested owned types, EF discards changes on the entity property. Its better to provide an example:

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;

namespace TestEfCtorInjectionWithOwnedTypes
{
    public class MyEntity
    {
        public int Id { get; set; }
        public OwnedEntity Owned { get; set; }
        private MyEntity() { }
        public MyEntity(OwnedEntity owned) => Owned = owned;
    }

    public class OwnedEntity
    {
        public OwnedOwnedEntity OwnedOwnedProperty { get; private set; }
        private OwnedEntity() { }
        public OwnedEntity(OwnedOwnedEntity ownedOwnedProperty)
            => OwnedOwnedProperty = ownedOwnedProperty;
    }

    public class OwnedOwnedEntity
    {
        public double MyProperty { get; private set; }
        private OwnedOwnedEntity() { }
        public OwnedOwnedEntity(double myProperty) => MyProperty = myProperty;
    }

    class MyDbContext : DbContext
    {
        public MyDbContext(DbContextOptions options) : base(options) { }

        public DbSet<MyEntity> MyEntities { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<MyEntity>(cb =>
            {
                cb.OwnsOne(seg => seg.Owned, b =>
                {
                    b.OwnsOne(e => e.OwnedOwnedProperty);
                });
            });
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            var services = new ServiceCollection();

            services.AddDbContext<MyDbContext>(options =>
                options.UseInMemoryDatabase("MyDatabase"));

            using (var sp = services.BuildServiceProvider())
            {
                int entityId;

                using (var scope = sp.CreateScope())
                {
                    var dbContext = scope.ServiceProvider.GetRequiredService<MyDbContext>();

                    var entity = new MyEntity(new OwnedEntity(new OwnedOwnedEntity(4242)));

                    dbContext.MyEntities.Add(entity);

                    dbContext.SaveChanges();

                    entityId = entity.Id;
                }

                using (var scope = sp.CreateScope())
                {
                    var dbContext = scope.ServiceProvider.GetRequiredService<MyDbContext>();

                    var entity = dbContext.MyEntities.Find(entityId);

                    System.Console.WriteLine("MyProperty before changing: " +
                        entity.Owned.OwnedOwnedProperty.MyProperty);

                    entity.Owned = new OwnedEntity(new OwnedOwnedEntity(4848));

                    System.Console.WriteLine("MyProperty before saving changes: " + 
                        entity.Owned.OwnedOwnedProperty.MyProperty);

                    dbContext.SaveChanges();

                    System.Console.WriteLine("MyProperty after saving changes: " +
                        entity.Owned.OwnedOwnedProperty.MyProperty);
                }
            }
        }
    }
}

The output I'm getting:

MyProperty before changing: 4242
MyProperty before saving changes: 4848
MyProperty after saving changes: 4242

[ahmedtolba1984]

Hi @andriysavin ,
I am in the exact issue, Have you solved it ? or did you find a workaround for this issue as i need to solve it as soon as possible :)

[andriysavin]

@ahmedtolba1984 nope, all hope to MS guys :)

[julielerman]

If it's any use, I wrote an article with help from @AndriySvyryd (on the EF team) about a workaround . It's here: https://msdn.microsoft.com/magazine/mt846463. Note that the workaround doesn't work with InMemory, however. :(

[andriysavin]

@julielerman Thanks, the article is great! However, you're covering EF Core 2.0, where owned types replacement wasn't claimed to work. And I'm taking about v2.1, which as I understood should have fixed it. And looks like it has, but only partially. Replacement works without any workarounds, but only if there is no nesting of owned types.

[divega]

@AndriySvyryd thoughts?

[divega]

@andriysavin thanks for reaching out. Could you please create a new self-contained issue about values in nested owned types not being taken into account after replacement?

[andriysavin]

@divega Yes, I will.

[andriysavin]

Here it is #12118

[divega]

@andriysavin thanks!

[HassanHashemi]

@divega Is this fixed already? i see the label closed fixed?

[ajcvickers]

@HassanHashemi Yes, this issue was fixed in the 2.0.3 patch release--that's what the combination of a closed issue in a milestone means. However, there is a related issue #12118 that is still open.

[chanido]

I've had this issue and written a post on how we resolved it.

I hope this helps someone else.

[stg609]

Is the issue fixed? Why I still have the same problem using EF Core 2.2.6? Would you please point me some guide line on how to fix it?
@ajcvickers

[ajcvickers]

@stg609 If you are still hitting a problem, then please open a new issue and include a small, runnable project/solution or complete code listing that reproduces the behavior you are seeing.