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

InvalidOperationException updating an entity with CurrentValues.SetValues throws InvalidOperationException when the SetValues parameter does not contain a required field data. #8465

Closed
alexdrl opened this issue May 15, 2017 · 0 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@alexdrl
Copy link

alexdrl commented May 15, 2017

I am writing a custom save generic method for our framework in EF Core. The idea is to have a method that can make partial updates

The method is working with simple entities but throws an InvalidOperationException when the entity in entitiesAndProperties does not contain a required field data. It should not be a problem, because the entity already exists in database with the required field value, contextEntity reflects that, but seems the CurrentValues.SetValues overrides this.

If you are seeing an exception, include the full exceptions details (message and stack trace).

Exception message:
'The property 'Name' on entity type 'User' is marked as null, but this cannot be saved because the property is marked as required.'

Stack trace:
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.HandleConceptualNulls()
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.GetEntriesToSave()
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)

Steps to reproduce

Those are the custom entity method and clases.

public IEnumerable<T> UpdateEntitiesByProperties(IEnumerable<UpdateByPropertiesData<T>> entitiesAndProperties)
        {
            List<T> entities = new List<T>();

            // TODO: (Error Management) Decide what to do with null parameters
            if (entitiesAndProperties?.Any() != true)
            {
                return entities;
            }

            foreach (UpdateByPropertiesData<T> entityAndProperties in entitiesAndProperties)
            {
                T contextEntity = this.context.Find<T>(entityAndProperties.Entity.Id);

                EntityEntry<T> changeTrackerEntityEntry = this.context.Entry(contextEntity);

                changeTrackerEntityEntry.CurrentValues.SetValues(entityAndProperties.Entity);

                // TODO: Review the possibility of having a local copy of the entityEntry modified
                // properties, for not setting them to false in this foreach. This will fix the edge
                // case when we call this method two times with the same entity and different data,
                // in which case, data is lost.
                foreach (PropertyEntry property in changeTrackerEntityEntry.Properties.Where(p => !p.Metadata.IsConcurrencyToken))
                {
                    property.IsModified = entityAndProperties.UpdatedProperties.Contains(property.Metadata.Name);
                }

                entities.Add(contextEntity);
            }

            return entities;
        }


 public class UpdateByPropertiesData<T>
    {
        /// <summary> Initializes a new instance of the <see cref="UpdateByPropertiesData{T}"/> class. </summary>
        public UpdateByPropertiesData()
        {
        }

        /// <summary> Initializes a new instance of the <see cref="UpdateByPropertiesData{T}"/> class. </summary>
        /// <param name="entity">
        ///     The entity.
        /// </param>
        /// <param name="properties">
        ///     The updated properties.
        /// </param>
        public UpdateByPropertiesData(T entity, IEnumerable<string> properties)
        {
            this.Entity = entity;
            this.UpdatedProperties = properties;
        }

        /// <summary> Gets or sets the entity with the updated properties. </summary>
        /// <value> The entity with the updated properties. </value>
        public T Entity { get; set; }

        /// <summary> Gets or sets the updated properties in string format. </summary>
        /// <value> The updated properties in string format. </value>
        public IEnumerable<string> UpdatedProperties { get; set; }
    }

If I change the code to this, it is working properly, but it uses reflection, and not seems that Smart as SetValues. With the SetValues documentation I thought that I could do what I did before... I thought that the ChangeTracker only updated the IsModified properties...

public IEnumerable<T> UpdateEntitiesByProperties(IEnumerable<UpdateByPropertiesData<T>> entitiesAndProperties)
        {
            List<T> entities = new List<T>();

            // TODO: (Error Management) Decide what to do with null parameters
            if (entitiesAndProperties?.Any() != true)
            {
                return entities;
            }

            foreach (UpdateByPropertiesData<T> entityAndProperties in entitiesAndProperties)
            {
                T contextEntity = this.context.Find<T>(entityAndProperties.Entity.Id);

                EntityEntry<T> changeTrackerEntityEntry = this.context.Entry(contextEntity);

                changeTrackerEntityEntry.CurrentValues.SetValues(entityAndProperties.Entity);

                // TODO: Review the possibility of having a local copy of the entityEntry modified
                // properties, for not setting them to false in this foreach. This will fix the edge
                // case when we call this method two times with the same entity and different data,
                // in which case, data is lost.
                foreach (PropertyEntry property in changeTrackerEntityEntry.Properties.Where(p => !p.Metadata.IsConcurrencyToken))
                {
                    if (entityAndProperties.UpdatedProperties.Contains(property.Metadata.Name))
                    {
                        property.CurrentValue = typeof(T).GetTypeInfo().GetDeclaredProperty(property.Metadata.Name).GetValue(entityAndProperties.Entity);
                    }
                }

                entities.Add(contextEntity);
            }

            return entities;
        }

This is the reproducing code. It seems that it is simple to reproduce, but if you really need i could try to make a simple .NET Core console app which reproduces this.

using (BaseContext context = new BaseContext(connectionProvider))
            using (IUnitOfWork unitOfWork = new UnitOfWork<BaseContext>(context))
            {
                User changedUser = new User(Guid.Parse("11111111-1111-1111-1111-111111111111"), Guid.NewGuid());
                changedUser.LongName = "NewLongName";

                UpdateByPropertiesData<User> ubpdData = new UpdateByPropertiesData<User>(changedUser, new string[] { "LongName" });

                entitiesList.AddRange(unitOfWork.GetRepository<User>().UpdateEntitiesByProperties(new UpdateByPropertiesData<User>[] { ubpdData }));

                unitOfWork.SaveChanges();
            }

Further technical details

EF Core version: 1.1.2
Database Provider: Microsoft.EntityFrameworkCore.SqlServe
Operating system:
IDE: Visual Studio 2017 15.2

@ajcvickers ajcvickers self-assigned this May 17, 2017
@ajcvickers ajcvickers added this to the 2.0.0 milestone May 17, 2017
ajcvickers added a commit that referenced this issue Jun 23, 2017
Issues #7798, #8265, #8465

These issues were due to incorrect handling of nulls/conceptual nulls for required but nullable properties, in particular when attaching an entity with a required property that starts off null.
@ajcvickers ajcvickers added type-bug closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed type-investigation labels Jun 23, 2017
ajcvickers added a commit that referenced this issue Jun 26, 2017
Issues #7798, #8265, #8465

These issues were due to incorrect handling of nulls/conceptual nulls for required but nullable properties, in particular when attaching an entity with a required property that starts off null.
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. type-bug
Projects
None yet
Development

No branches or pull requests

2 participants