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

Replacing a property with owned type doesn't work if there is owned types nesting in v2.1 RC1 #12118

Closed
andriysavin opened this issue May 23, 2018 · 24 comments
Labels
breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@andriysavin
Copy link

andriysavin commented May 23, 2018

I'm trying to use replacing properties with owned types since it was promised to fix in 2.1. This feature seems to be working ok 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
@andriysavin
Copy link
Author

andriysavin commented May 26, 2018

Just to give a more real life example, here are some of my models (simplified).
GeoPosition and GeoSegment are Owned Types. And assigning MyEntity.Location = new GeoSegment(new GeoPosition(...), new GeoPosition(...)) is not saved, as described earlier.

 public sealed class GeoPosition
 {
        public double Latitude { get; private set; }
        public double Longitude { get; private set; }

        private GeoPosition(){}

        public GeoPosition(double latitude, double longitude)
        {
            Latitude = latitude;
            Longitude = longitude;
        }
}

public sealed class GeoSegment
{
        public GeoPosition Start { get; private set; }
        public GeoPosition End { get; private set; }

        private GeoSegment(){}

        public GeoSegment(GeoPosition start, GeoPosition end)
        {
            Start = start;
            End = end;
        }
}

public class MyEntity
{
        public int Id { get; set; }
        public GeoSegment Location { get; set; }

        public MyEntity(GeoSegment location)
        {
            Location = location;
        }
}

@ajcvickers
Copy link
Member

@AndriySvyryd Any update on the investigation here? Specifically, is it something we might want to patch?

@AndriySvyryd
Copy link
Member

@ajcvickers The issue here is that when we attach OwnedEntity the initial fixup finds the existing OwnedOwnedEntity and overrides the new navigation with the old dependent. Fixing this is potentially a breaking change.

The workaround is to set the dependent after the new owned entity is fixed up:

using (var context = new MyDbContext(options))
{
    var entity = context.MyEntities.Find(entityId);

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

    entity.Owned = new OwnedEntity(null);

    context.ChangeTracker.DetectChanges();

    context.Entry(entity).Reference(e => e.Owned).TargetEntry.Reference(e => e.OwnedOwnedProperty).CurrentValue
        = new OwnedOwnedEntity(4848);

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

    context.SaveChanges();

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

@ajcvickers
Copy link
Member

@AndriySvyryd Thanks. Removing milestone to re-triage on Wednesday. /cc @divega

@ajcvickers ajcvickers modified the milestones: 2.1.1, 2.2.0 May 29, 2018
@ajcvickers
Copy link
Member

Note from triage: non-owned case with conflicting FK should throw, but low priority.

@Ranmoro
Copy link

Ranmoro commented Jul 10, 2018

The problem is repeated at the first level of the object, as well as with of one to one relationship.

@Alantoo
Copy link

Alantoo commented Sep 9, 2018

Hi everyone.

I had similar issue. I was unable to update property of owned type of an entity.
Hope it will be helpful for someone as temporary fixup until we get solution from ef team.
Here a code snipped which works for me to update entity with immutable owned types:

public static EntityEntry<TEntity> UpdateImmutable<TEntity>(this DbContext context,
            TEntity immutableEntity)
            where TEntity : class, IEntity
        {
            var oldEntity = context.ChangeTracker.Entries<TEntity>()
                .FirstOrDefault(_ => _.Entity.Id == immutableEntity.Id);

            if (oldEntity != null)
            {
                oldEntity.State = EntityState.Detached;
            }

            var updatedEntry = context.Entry(immutableEntity);

            if (oldEntity != null)
            {
                foreach (PropertyEntry property in updatedEntry.Properties)
                {
                    var oldPropertyName = property.Metadata.Name;
                    var oldProperty = oldEntity.Property(oldPropertyName);

                    if (property.Metadata.IsConcurrencyToken)
                    {
                        property.CurrentValue = oldProperty.CurrentValue;
                        property.OriginalValue = oldProperty.OriginalValue;
                    }
                }

                foreach (ReferenceEntry navigation in updatedEntry.Navigations.OfType<ReferenceEntry>())
                {
                    var oldNavigationName = navigation.Metadata.Name;
                    var oldNavigation = oldEntity.Navigation(oldNavigationName);
                    var entityEntry = context.Entry(oldNavigation.CurrentValue);
                    
                    if (entityEntry.State != EntityState.Detached && !Equals(oldNavigation.CurrentValue, navigation.CurrentValue))
                    {
                        entityEntry.State = EntityState.Detached;
                        context.Entry(navigation.CurrentValue).State = EntityState.Modified;
                    }
                }
            }

            updatedEntry.State = EntityState.Modified;

            return updatedEntry;
        }

If someone can suggest better approach it would be nice.

@ajcvickers ajcvickers modified the milestones: 2.2.0-preview2, 2.2.0 Sep 11, 2018
@smariussorin
Copy link

smariussorin commented Sep 26, 2018

@Alantoo Does it fix the nested own types?

I solved using a quick workaround ( We have only one entity with own type so this approach is simpler for us, without using reflexion)

Example for detached context:

context.Entry(entityExisting).Reference(p => p.OwnTypeA).TargetEntry.CurrentValues.SetValues(entityUpdate.OwnTypeA);
context.Entry(entityExisting).Reference(p => p.OwnTypeB).TargetEntry.CurrentValues.SetValues(entityUpdate.OwnTypeB);
context.Entry(entityExisting.OwnTypeB).Reference(p => p.OwnTypeB_SubOwnType).TargetEntry.CurrentValues.SetValues(entityUpdate.OwnTypeB.OwnTypeB_SubOwnType);
context.Entry(entityExisting).CurrentValues.SetValues(entityUpdate);

context.Entry(entityExisting).Reference(p => p.OwnTypeA).TargetEntry.State = EntityState.Modified;
context.Entry(entityExisting).Reference(p => p.OwnTypeB).TargetEntry.State = EntityState.Modified;
context.Entry(entityExisting.OwnTypeB).Reference(p => p.OwnTypeB_SubOwnType).TargetEntry.State = EntityState.Modified;
context.Entry(entityExisting).State = EntityState.Modified;

I`m waiting for a live so I can remove redundant code

@FabriDamazio
Copy link

I can confirm that @smariussorin workaround works. =))

Tested with a owned type with 5 nested owned types

@NickCook-StruMIS
Copy link

I have this problem as well, although @smariussorin solution did not work for me but @AndriySvyryd solution works perfectly. Do we know when this problem will be resolved so I can remove my temporary work around. Thanks.

@ajcvickers ajcvickers modified the milestones: 2.2.0-preview3, 2.2.0 Oct 15, 2018
@Mortana89
Copy link

Mortana89 commented Jan 8, 2019

Hi,

What's the status of this? Also facing issues with owned types with 2 levels (parent A - owned collection AB - owned collection BC), where, when adding a new entry to AB, which holds also a new entry in BC, all the entries in AB lose their BC values, except the first and the last (newly added) entry.
Cosmos DB provider that is.

You can see it clearly here (Notice the StatusHistory collections):

    "id": "51e8e0dc-ef21-46a3-b753-eca90100a005",
    "Country": "BEL",
    "Discriminator": "ServiceLocation",
    "OrganisationId": "7b3c1aec-1e14-4e33-8302-50e3c5ec6e24",
    "Location": null,
    "Services": [
        {
            "id": "1c9867c5-18a6-4384-9f14-f3b534ac2835",
            "Discriminator": "Service",
            "ExternalIdentifier": "1112332223",
            "ServiceLocationid": "51e8e0dc-ef21-46a3-b753-eca90100a005",
            "StatusHistory": [
                {
                    "id": "3473c8ee-ab48-435a-b938-d4868a383a52",
                    "Discriminator": "ServiceStatusHistory",
                    "EndDateTime": "9999-12-31T23:59:59.9999999",
                    "ServiceStatusEnum": "requested",
                    "Serviceid": "1c9867c5-18a6-4384-9f14-f3b534ac2835",
                    "StartDateTime": "2019-01-08T00:00:00Z"
                }
            ]
        },
        {
            "id": "dd6ad486-de1d-420d-809c-6f84245619c9",
            "Discriminator": "Service",
            "ExternalIdentifier": "1112332235",
            "ServiceLocationid": "51e8e0dc-ef21-46a3-b753-eca90100a005",
            "StatusHistory": []
        },
        {
            "id": "081f85ef-3a36-41a5-a5f1-d67e79fb2234",
            "Discriminator": "Service",
            "ExternalIdentifier": "1112332238",
            "ServiceLocationid": "51e8e0dc-ef21-46a3-b753-eca90100a005",
            "StatusHistory": []
        },
        {
            "id": "31b0b16e-0900-4c67-937c-eb3002563eef",
            "Discriminator": "Service",
            "ExternalIdentifier": "1112332231",
            "ServiceLocationid": "51e8e0dc-ef21-46a3-b753-eca90100a005",
            "StatusHistory": [
                {
                    "id": "baa23b25-e088-4fb0-b54c-383ed34cc00d",
                    "Discriminator": "ServiceStatusHistory",
                    "EndDateTime": "9999-12-31T23:59:59.9999999",
                    "ServiceStatusEnum": "requested",
                    "Serviceid": "31b0b16e-0900-4c67-937c-eb3002563eef",
                    "StartDateTime": "2019-01-08T00:00:00Z"
                }
            ]
        }
    ],
    "_rid": "24kMAKY2M4kFAAAAAAAAAA==",
    "_self": "dbs/24kMAA==/colls/24kMAKY2M4k=/docs/24kMAKY2M4kFAAAAAAAAAA==/",
    "_etag": "\"00000000-0000-0000-a73c-f2ea6b2e01d4\"",
    "_attachments": "attachments/",
    "_ts": 1546943331
}```

@AndriySvyryd
Copy link
Member

@Mortana89 I think that's actually another manifestation of #13578

@NickCook-StruMIS
Copy link

Yes, I am still waiting for a fix to this. I have temporary workaround code in my BaseContext that I want to remove because it is causing other knock on issues for our software now. Is this Issue being resolved in Core version 3.0?

Thanks

@ajcvickers
Copy link
Member

@NickCook-StruMIS This issue is in the 3.0 milestone, which means that it is tentatively planned to be fixed in the 3.0 release.

@chanido
Copy link

chanido commented Mar 23, 2019

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

I hope this helps someone else.

@vertonghenb
Copy link

vertonghenb commented Mar 26, 2019

Just an update, not working in Preview 3, still throws the following exception when replacing an owned owned type with a new one:
System.InvalidOperationException: The instance of entity type 'SalesOrderLineItem.Price#UnitPrice' with the key value '{SalesOrderLineItemSalesOrderLineId: 14}' is marked as 'Deleted', but the instance of entity type 'SalesOrderLineItem' with the key value '{SalesOrderLineId: 14}' is marked as 'Modified' and both are mapped to the same row.

@AndriySvyryd
Copy link
Member

@vertonghenb The fix will be shipped in 3.0 preview 4

@lonix1
Copy link

lonix1 commented Jul 7, 2019

None of the workarounds work when using UserManager<TUser> instead of DbContext.

Does someone have this use case, and has a small snippet of working code to share?

e.g.

// `User` is entity
// `User.Token` is owned type
// `User.Token.Hash` is owned-owned type
user.Token = someNewToken;
await userManager.UpdateAsync(user);
// user.Token and user.Token.Hash are not updated

I posted by use case to SO here.

@Cogax
Copy link

Cogax commented Jul 16, 2019

None of the workarounds work when using UserManager<TUser> instead of DbContext.

Does someone have this use case, and has a small snippet of working code to share?

e.g.

// User is entity
// User.Token is owned type
// User.Token.Hash is owned-owned type
user.Token = someNewToken;
await userManager.UpdateAsync(user);
// user.Token and user.Token.Hash are not updated
I posted by use case to SO here.

Try something like that:

// `User` is entity
// `User.Token` is owned type
// `User.Token.Hash` is owned-owned type
user.Token = someNewToken;

var hash = user.Token.Hash
context.ChangeTracker.DetectChanges();
context.Entry(user).Reference(x => x.Token).TargetEntry.Reference(x => x.Hash).CurrentValue = hash;
context.Entry(user).State = EntityState.Modified;
await context.SaveChangesAsync();

@m-okhovat
Copy link

@AndriySvyryd currently I'm using ef core 5 preview and still have the same issue which @andriysavin mentioned earlier.
In order to respect the separation of concern with a DDD approach, I don't have access to the dbContext in the application layer of my application, in which entity with owned type should be updated...
is there any workaround by overriding the SaveChanges method???

@AndriySvyryd
Copy link
Member

@m-okhovat This issue was fixed in 3.0.0, if you are still experiencing it in 5.0.0 file a new issue with a small repro project

@virzak
Copy link

virzak commented Mar 24, 2021

I'm not sure if this is the same issue, but EntityEntry<T>.CurrentValues does not include Owned entity properties. This results in CurrentValues.SetValues(disconnected) to skip the owned entity's properties.

Added a failing test case in #24504

@AndriySvyryd
Copy link
Member

@virzak That's by design, you need to traverse navigations by calling entityEntry.Reference("OwnedReference").EntityEntry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

No branches or pull requests