dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.71k stars 3.17k forks source link

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

Open ghost opened 5 years ago

ghost commented 5 years ago

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 commented 5 years ago
                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 commented 5 years ago

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 commented 5 years ago

@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

ghost commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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);