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

Make EntityEntry methods (GetDatabaseValues, etc) aggregate friendly #13890

Open
Tracked by #1985
ghost opened this issue Nov 5, 2018 · 7 comments
Open
Tracked by #1985

Make EntityEntry methods (GetDatabaseValues, etc) aggregate friendly #13890

ghost opened this issue Nov 5, 2018 · 7 comments

Comments

@ghost
Copy link

ghost commented Nov 5, 2018

The proposal is to add the following methods:

EntityEntry.GetOriginalValues(includeOwnedTypes: true);
EntityEntry.GetCurrentValues(includeOwnedTypes: true);
EntityEntry.GetDatabaseValues(includeOwnedTypes: true);
EntityEntry.Reload(includeOwnedTypes: true);

And to make nested access work:

EntityEntry<TEntity>.Property(e => e.CustomerName.FirstName);
EntityEntry<TEntity>.Property(e => e.Addresses[2].City);

Original issue below:


I attach an entity which has an owned type as its property.
Then I set original values which are different from the current values (on the owned type properties)

Steps to reproduce

   //  the owned type
    public class CustomerName
    {
        public CustomerName()
        {
        }
        public string FirstName { get; set; }
        public string MiddleName { get; set; }
        public string LastName { get; set; }
    }

    public partial class Customer
    {
        public Customer()
        {
            CustomerAddress = new HashSet<CustomerAddress>();
            SalesOrderHeader = new HashSet<SalesOrderHeader>();
            CustomerName = new CustomerName();
        }

        public int CustomerId { get; set; }
        public bool NameStyle { get; set; }
        public string Title { get; set; }
        // this is the property with the owned type
        public CustomerName CustomerName { get; set; }

        public ICollection<CustomerAddress> CustomerAddress { get; set; }
        public ICollection<SalesOrderHeader> SalesOrderHeader { get; set; }
    }

      //  the code for update (first name is different in the current and original owned type version )
        public void UpdateCustomer(Customer customer)
        {
            customer.ModifiedDate = DateTime.Now;
            var orig = this.GetOriginal<Customer>();
            var entry = DB.Customer.Attach(customer);
           // the change is not detected (although properties are different)
            entry.OriginalValues.SetValues(orig);
        }

the entity framework does not detect changes on SaveChanges the owned type is not updated (without error)
but if i update property on the owned type after attaching the entity

        public void UpdateCustomer(Customer customer)
        {
            customer.ModifiedDate = DateTime.Now;
            var orig = this.GetOriginal<Customer>();
            var entry = DB.Customer.Attach(customer);
            // change property on the owned type after attaching
            customer.CustomerName.FirstName = "Dummy Name";
            entry.OriginalValues.SetValues(orig);
        }

then this change is detected and Saved to Database

Further technical details

EF Core version: Microsoft.EntityFrameworkCore.2.1.4
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Visual Studio 2017 15.8.8

@ghost
Copy link
Author

ghost commented Nov 5, 2018

                entity.OwnsOne(x => x.CustomerName).Property(c => c.FirstName).HasColumnName("FirstName").IsRequired()
                    .HasColumnType("Name")
                    .HasMaxLength(50);

                entity.OwnsOne(x => x.CustomerName).Property(c => c.MiddleName).HasColumnName("MiddleName")
                    .HasColumnType("Name")
                    .HasMaxLength(50);

                entity.OwnsOne(x => x.CustomerName).Property(c => c.LastName).HasColumnName("LastName").IsRequired()
                    .HasColumnType("Name")
                    .HasMaxLength(50);

@ghost
Copy link
Author

ghost commented Nov 5, 2018

You can try it on this project https://github.com/BBGONE/JRIApp.Core
which uses Entity Framework Core 2.1.4

This file has the update code:
https://github.com/BBGONE/JRIApp.Core/blob/master/DEMOS/RIAppDemoMVC/RIApp.BLL/DataServices/RIAppDemoServiceEF.cs

        [Authorize(Roles` = new[] {ADMINS_ROLE})]
        [Update]
        public void UpdateCustomer(Customer customer)
        {
            customer.ModifiedDate = DateTime.Now;
            var orig = this.GetOriginal<Customer>();
            var entry = DB.Customer.Attach(customer);
            /*
           var dbValues = entry.GetDatabaseValues();
           entry.OriginalValues.SetValues(dbValues);
           */
            entry.OriginalValues.SetValues(orig);
}

@ajcvickers
Copy link
Member

@BBGONE The methods like GetDatabaseValues, SetValues etc work on a single entity instance. Since the owned type is really just an entity type, these methods need to be called both for the owner instance and owned instance.

This is currently by design behavior, but isn't ideal when using aggregates, so we will use this issue to track making the experience better, with reference to #1985

@ajcvickers ajcvickers changed the title Owned types are not updated Make EntityEntry methods (GetDatabaseValues, etc) aggregate friendly Nov 5, 2018
@ajcvickers ajcvickers added this to the 3.0.0 milestone Nov 5, 2018
@ajcvickers ajcvickers self-assigned this Nov 5, 2018
@ghost
Copy link
Author

ghost commented Nov 6, 2018

Yeah, that's better be done internally by the framework as it was with complex type in EF6- because how the user (consumer) can know how the entity is mapped - she (he) just uses it as a single piece.

@ghost
Copy link
Author

ghost commented Nov 10, 2018

I managed to update owned types using very convoluted update: this is a current workaround for this issue

        public override void Update(Customer customer)
        {
            customer.ModifiedDate = DateTime.Now;
            var orig = this.GetOriginal<Customer>();
            var entry = DB.Customer.Attach(customer);
            var _entry2 = DB.Entry<CustomerName>(customer.CustomerName);
            var _entry3 = DB.Entry<CustomerContact>(customer.CustomerName.Contact);

            entry.OriginalValues.SetValues(orig);
            _entry2.OriginalValues.SetValues(orig.CustomerName);
            _entry3.OriginalValues.SetValues(orig.CustomerName.Contact);
        }

@ghost
Copy link
Author

ghost commented Nov 10, 2018

I added a generic extension for updating original values on owned types.
https://github.com/BBGONE/JRIApp.Core/blob/master2/FRAMEWORK/SERVER/RIAPP.DataService.EFCore/Utils/EntityUpdateHelper.cs
You can reuse it as you wish.
Now the code for update looks as:

        public override void Update(Customer customer)
        {
            customer.ModifiedDate = DateTime.Now;
            var orig = this.GetOriginal<Customer>();
            var entry = DB.Customer.Attach(customer);
            // Using custom extension method - This is a workaround to update owned entities https://github.com/aspnet/EntityFrameworkCore/issues/13890
            entry.SetOriginalValues(orig);
        }

The extension implemented as:

    public static class EntityUpdateHelper
    {
        private class EntryValue
        {
            public int Level { get; set; }
            public ReferenceEntry Reference { get; set; }
            public EntityEntry Entry { get; set; }
            public object parentEntity { get; set; }
            public object Entity { get; set; }
        }

        private static EntryValue[] _GetOwnedEntryValues(EntryValue entryValue, List<EntryValue> entryList=null, int level = 0)
        {
            if (entryList== null)
            {
                entryList = new List<EntryValue>();
            }
            entryList.Add(entryValue);

            MemberEntry[] members = entryValue.Entry.Members.ToArray();
            ReferenceEntry[] references = members.OfType<ReferenceEntry>().ToArray();
            foreach (var _reference in references)
            {
                var currentEntity = _reference.Metadata.PropertyInfo.GetValue(entryValue.Entity);
                int nextLevel = level + 1;
                var currentEntryValue = new EntryValue { Level = nextLevel, Entry = _reference.TargetEntry, Entity = currentEntity, Reference = _reference, parentEntity = entryValue.Entity };
                _GetOwnedEntryValues(currentEntryValue, entryList, nextLevel);
            }

            return entryList.ToArray();
        }

        private static void _SetValuesAtLevel(int lvl, ILookup<int, EntryValue> entryLookUp, int maxLvl, Action<EntityEntry, object> setValuesAction)
        {
            foreach (var ev in entryLookUp[lvl])
            {
                var metadata = ev.Reference.Metadata;
                // Type currentValueType = metadata.ClrType;
                // string name = metadata.Name;
                var currentValue = metadata.PropertyInfo.GetValue(ev.parentEntity);
                if (currentValue != null)
                {
                    setValuesAction(ev.Entry, currentValue);
                    int nextLvl = lvl + 1;
                    if (maxLvl >= nextLvl)
                    {
                        _SetValuesAtLevel(nextLvl, entryLookUp, maxLvl, setValuesAction);
                    }
                }
            }
        }

        /// <summary>
        ///  This is a workaround to update owned entities https://github.com/aspnet/EntityFrameworkCore/issues/13890
        /// </summary>
        /// <param name="entry">Top level entry on which to set the values</param>
        /// <param name="entity">the Values in the form of an object</param>
        public static void _SetRootValues(EntityEntry entry, object entity, Action<EntityEntry, object> setValuesAction)
        {
            setValuesAction(entry, entity);
            var entryValue = new EntryValue { Level = 0, Entry = entry, Entity = entity, Reference = null, parentEntity = null };
            var entryValues = _GetOwnedEntryValues(entryValue);
            var levels = entryValues.Select(e => e.Level).ToArray();
            var maxLvl = levels.Max();
            if (maxLvl >= 1)
            {
                var entriesLookUp = entryValues.ToLookup(e => e.Level);
                _SetValuesAtLevel(1, entriesLookUp, maxLvl, setValuesAction);
            }
        }

        /// <summary>
        /// Extension method to set original values on entity entries
        /// </summary>
        /// <param name="entry"></param>
        /// <param name="origValues"></param>
        public static void SetOriginalValues(this EntityEntry entry, object origValues)
        {
            EntityUpdateHelper._SetRootValues(entry, origValues, (entryParam, entityParam) => entryParam.OriginalValues.SetValues(entityParam));
        }
    }

@ghost
Copy link
Author

ghost commented Nov 11, 2018

P.S. I suggest, when you implement the fix in EFCore 3.0, you would implement this in a backward compatible way. For example adding optional parameters to the methods. Because developers would start using workarounds before you implement this, and that will break their code.

OriginalValues.SetValues(entity, includeOwnedTypes: true);

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

No branches or pull requests

2 participants