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

Allow optional dependents to be added later when using table splitting #17422

Closed
AndriySvyryd opened this issue Aug 26, 2019 · 14 comments · Fixed by #17951
Closed

Allow optional dependents to be added later when using table splitting #17422

AndriySvyryd opened this issue Aug 26, 2019 · 14 comments · Fixed by #17951
Labels
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

@AndriySvyryd
Copy link
Member

Reported on https://stackoverflow.com/questions/57628778/efcore-3-and-owned-type-in-same-table-how-do-you-set-owned-instance#_=_

'The entity of type 'Owned' is sharing the table 'Principals' with entities of type 'Principal', but there is no entity of this type with the same key value that has been marked as 'Added'.

using System;
using System.Linq;
using Microsoft.EntityFrameworkCore;

namespace TestEF
{
  class Program
  {
    static void Main(string[] args)
    {
      var id = Guid.NewGuid();

      using (var db = new Ctx())
      {
        db.Database.EnsureDeleted();
        db.Database.EnsureCreated();

        var p = new Principal {Id = id};
        db.Principals.Add(p);
        db.SaveChanges();
      }

      using (var db = new Ctx())
      {
        var p = db.Principals.Single(o => o.Id == id);
        p.Child = new Owned();
        p.Child.Prop1 = "Test2";
        p.Child.Prop2 = "Test2";
        db.SaveChanges();
      }
    }

    public class Principal
    {
      public Guid Id { get; set; }
      public Owned Child { get; set; }
    }

    public class Owned
    {
      public string Prop1 { get; set; }
      public string Prop2 { get; set; }
    }

    public class Ctx : DbContext
    {
      public DbSet<Principal> Principals { get; set; }

      protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
      {
        optionsBuilder.UseSqlServer("Data Source=.;Initial Catalog=TestEF;Trusted_Connection=True;Persist Security Info=true");
      }

      protected override void OnModelCreating(ModelBuilder mb)
      {
        var emb = mb.Entity<Principal>();
        emb
          .OwnsOne(o => o.Child, cfg =>
          {
            cfg.Property(o => o.Prop1).HasMaxLength(30);
            //cfg.WithOwner();
          });
      }
    }
  }
}
@AndriySvyryd
Copy link
Member Author

As a workaround you could make the child appear as modified:

db.ChangeTracker.DetectChanges();
var childEntry = db.Entry(p.Child);
childEntry.State = EntityState.Modified;
db.SaveChanges();

@julielerman
Copy link

Hi @AndriySvyryd , This workaround is rough for normal scenarios because it assumes you are working directly with the change tracker. I want to be able to write my business logic : e.g., retrieve the entity with its null dependent (in my case an owned entity), then set a new object to that property. And on savechanges, I want the context to figure out that the mistakently Added entity needs to be changed to modified.

If the shadow property that represents the inferred key is an int then I can test for <0 to know it's truly being Added. Hacky but ok for a short-term workaround. But If we're using Guids and setting them in code that won't really work.

Is there something consistent that can be a differentiator after retrieving a null dependent and then replacing it? Since detectchanges thought it was added, the original values are the same as the current values. Hope my question is making sense. thanks!

@AndriySvyryd
Copy link
Member Author

@julielerman You can examine the state of the parent. I have not tested this, but it might do the trick:

if (entry.State == EntityState.Added)
{
    var ownership = entry.Metadata.FindOwnership();
    if (ownership != null)
    {
        var parentKey = ownership.Properties.Select(p => entry.Property(p.Name).CurrentValue).ToArray();
        var parent = context.Find(ownership.PrincipalEntityType.ClrType, parentKey);
        if (parent != null)
        {
            var parentEntry = context.Entry(parent);
            if (parentEntry.State != EntityState.Added)
            {
                entry.State = EntityState.Modified;
            }
        }
    }
}

And you can make this recursive to handle deeper levels of owned types.

@julielerman
Copy link

aha! That's the ticket! If the parent is not new then the owned entity can't be either! Why didn't I thik of that? :( THANKS!

@julielerman
Copy link

ps -- looking through commits, it looks like findownership is new to 3. Definitey new to me. :) Is that correct?

@AndriySvyryd
Copy link
Member Author

It was in .Internal previously. We made a lot of methods public in 3.0 as we don't want to accidentally break someone relying on them.

AndriySvyryd added a commit that referenced this issue Sep 20, 2019
…entry (the entry that isn't a dependent of any other entry in the row). If the main entry isn't present the row is assumed to be modified.

The principal entry is now not required to be in the same state as the dependent when saving changes to a shared row.

Fixes #17422
Fixes #17935
AndriySvyryd added a commit that referenced this issue Sep 20, 2019
…entry (the entry that isn't a dependent of any other entry in the row). If the main entry isn't present the row is assumed to be modified.

The principal entry is now not required to be in the same state as the dependent when saving changes to a shared row.

Fixes #17422
Fixes #17935
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 20, 2019
@AndriySvyryd AndriySvyryd removed their assignment Sep 20, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0, 3.1.0-preview1 Oct 15, 2019
@slipdef
Copy link

slipdef commented Nov 11, 2019

Ok. The workaround is there. But I don't understand how really basic scenario for nullable owned entities doesn't work in the release version and it's not a high priority fix for 3.0.

@AndriySvyryd
Copy link
Member Author

@slipdef In short it's because 3.0 is not an LTS release.

@sygnowskip
Copy link

sygnowskip commented Nov 25, 2019

@julielerman we've found (me and @jalbrzym) a workaround for this issue, current implementation of OwnsOne in EF Core does not support standard scenarios, e.g.:

  • { value } -> { value } is working fine
  • { value } -> NULL is throwing exception
  • NULL -> { value } is throwing exception

I have described more details here: #19044 (comment)

But, we have looked how EF handle value objects in SaveChanges method and we were able to provide a fix for this, you need to override SaveChanges method and put this code there:

        public override int SaveChanges()
        {
            var ownedEntities = ChangeTracker.Entries<ValueObject>().Where(x => x.State == EntityState.Added || x.State == EntityState.Deleted).ToList();
            foreach (var ownedEntityEntry in ownedEntities)
            {
                var ownership = ownedEntityEntry.Metadata.FindOwnership();
                if (ownership != null)
                {
                    var parentKey = ownership.Properties.Select(p => ownedEntityEntry.Property(p.Name).CurrentValue).ToArray();
                    var parent = Find(ownership.PrincipalEntityType.ClrType, parentKey);
                    if (parent != null)
                    {
                        var parentEntry = Entry(parent);
                        if (ownedEntityEntry.State == EntityState.Deleted && parentEntry.State != EntityState.Deleted)
                        {
                            parentEntry.State = EntityState.Modified;
                            var navProperty = ownership.PrincipalToDependent.FieldInfo;
                            if (navProperty.GetValue(parentEntry.Entity) == null)
                                navProperty.SetValue(parentEntry.Entity, FormatterServices.GetUninitializedObject(ownedEntityEntry.Metadata.ClrType));
                        }
                        else if (ownedEntityEntry.State == EntityState.Added && parentEntry.State != EntityState.Added)
                        {
                            ownedEntityEntry.State = EntityState.Modified;
                        }
                    }
                }
            }
            return base.SaveChanges();
        }

There is one disadvantage of this workaround, we were not able to handle OwnsOne inside OwnsOne (yet).

EDIT: It's working fine on EF Core 3.1.0-preview3.19554.8 😃 ❤️

@ajcvickers ajcvickers modified the milestones: 3.1.0-preview1, 3.1.0 Dec 2, 2019
@robbaman
Copy link

robbaman commented Dec 13, 2019

@julielerman You can examine the state of the parent. I have not tested this, but it might do the trick:

if (entry.State == EntityState.Added)
{
    var ownership = entry.Metadata.FindOwnership();
    if (ownership != null)
    {
        var parentKey = ownership.Properties.Select(p => entry.Property(p.Name).CurrentValue).ToArray();
        var parent = context.Find(ownership.PrincipalEntityType.ClrType, parentKey);
        if (parent != null)
        {
            var parentEntry = context.Entry(parent);
            if (parentEntry.State != EntityState.Added)
            {
                entry.State = EntityState.Modified;
            }
        }
    }
}

And you can make this recursive to handle deeper levels of owned types.

@AndriySvyryd Do you have a sample of how this can be done when an entity has an alternate key?

We have two entities: Parent and Owned (OwnsMany). The parent has a PK which is a Guid and an alternate key that is an int identity. The relation is done on the alternate int key.

Your proposed solution for getting the parent of the Owned type works in most cases, but the call to Find(ownership.PrincipalEntityType.ClrType, parentKey) fails with exception:

System.ArgumentException: The key value at position 0 of the call to 'DbSet<Pakket>.Find' 
was of type 'int', which does not match the property type of 'Guid'.

I understand why the error occurs, but I'm not familiar enough with the inner workings of EFCore to figure out a different way to get the parent of the owned type without using the Find method like this code does.

@AndriySvyryd
Copy link
Member Author

@robbaman You can update to 3.1 and avoid any workaround.

@robbaman
Copy link

@robbaman You can update to 3.1 and avoid any workaround.

Yes, except that I'm using this technique to change the parent to Modified whenever an Owned type is Modified/Deleted/Added and the parent is Unmodified to trigger the timestamp for the parent to be updated.

If there's another way to do that, that'd be cool too.

@AndriySvyryd
Copy link
Member Author

@robbaman I see, you are hitting #18529. To make the workaround work with AKs #7391 would be needed.

An alternative workaround would be to make sure the affected owned entities have a navigation to the parent (it can be private) and implement an interface, like:

public interface IOwnedEntity
{
    object GetRoot();
}

@emi662002
Copy link

@julielerman we've found (me and @jalbrzym) a workaround for this issue, current implementation of OwnsOne in EF Core does not support standard scenarios, e.g.:

  • { value } -> { value } is working fine
  • { value } -> NULL is throwing exception
  • NULL -> { value } is throwing exception

I have described more details here: #19044 (comment)

But, we have looked how EF handle value objects in SaveChanges method and we were able to provide a fix for this, you need to override SaveChanges method and put this code there:

        public override int SaveChanges()
        {
            var ownedEntities = ChangeTracker.Entries<ValueObject>().Where(x => x.State == EntityState.Added || x.State == EntityState.Deleted).ToList();
            foreach (var ownedEntityEntry in ownedEntities)
            {
                var ownership = ownedEntityEntry.Metadata.FindOwnership();
                if (ownership != null)
                {
                    var parentKey = ownership.Properties.Select(p => ownedEntityEntry.Property(p.Name).CurrentValue).ToArray();
                    var parent = Find(ownership.PrincipalEntityType.ClrType, parentKey);
                    if (parent != null)
                    {
                        var parentEntry = Entry(parent);
                        if (ownedEntityEntry.State == EntityState.Deleted && parentEntry.State != EntityState.Deleted)
                        {
                            parentEntry.State = EntityState.Modified;
                            var navProperty = ownership.PrincipalToDependent.FieldInfo;
                            if (navProperty.GetValue(parentEntry.Entity) == null)
                                navProperty.SetValue(parentEntry.Entity, FormatterServices.GetUninitializedObject(ownedEntityEntry.Metadata.ClrType));
                        }
                        else if (ownedEntityEntry.State == EntityState.Added && parentEntry.State != EntityState.Added)
                        {
                            ownedEntityEntry.State = EntityState.Modified;
                        }
                    }
                }
            }
            return base.SaveChanges();
        }

There is one disadvantage of this workaround, we were not able to handle OwnsOne inside OwnsOne (yet).

EDIT: It's working fine on EF Core 3.1.0-preview3.19554.8 😃 ❤️

small fix for the example if somebody is using it:
parentEntry.State = EntityState.Modified;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

Successfully merging a pull request may close this issue.

7 participants