InvalidOperationException when saving an object that contains two properties that refer to the same object.

[JanKupke1]

Good afternoon,

I have a problem using your service. The problem is that automapper always creates a new instance of an object, i.e. it makes two instances of one object.

I have done a lot of internet research, but I can't get to the point where it works for me.

As a summary, one can say that in the service: entity = _mapper.Map(studentclass); is used, but it should be _mapper.Map(studentclass, entity);. But to implement this cleverly I did not manage to do so far. It would be really great if you had a hint for me.

Thanks for your efforts: Jan Kupke

public class TestDbContext : DbContext
    {
        public DbSet<HasTwoNormalEntity> HasTwoNormalEntities { get; set; }
        public DbSet<NormalEntity> NormalEntities { get; set; }
}

namespace Tests.EfClasses
{
    public class HasTwoNormalEntity
    {
        public int Id { get; set; }
        public int NormalEntity1Id { get; set; }
        public NormalEntity NormalEntity1 { get; set; }
        public int NormalEntity2Id { get; set; }
        public NormalEntity NormalEntity2 { get; set; }
    }
}

using GenericServices;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Tests.EfClasses;

namespace Tests.Dtos
{
    public class HasTwoNormalEntityDto : ILinkToEntity<HasTwoNormalEntity>
    {
        public int Id { get; set; }

        public int NormalEntity1Id { get; set; }
        public NormalEntityDto NormalEntity1 { get; set; }

        public int NormalEntity2Id { get; set; }
        public NormalEntityDto NormalEntity2 { get; set; }
    }
}

using System;
using System.Linq;
using System.Threading.Tasks;
using GenericServices;
using GenericServices.PublicButHidden;
using GenericServices.Setup;
using Tests.Dtos;
using Tests.EfClasses;
using Tests.EfCode;
using TestSupport.EfHelpers;
using Xunit;
using Xunit.Extensions.AssertExtensions;

namespace Tests.UnitTests.TestIssues
{
    public class TestSaveOneNewItemWithTheSameExistingItem
    {

        /// <summary>
        /// SaveOneNewItemWithTheSameExistingItem
        /// </summary>
        /// <exception cref="System.InvalidOperationException"></exception>
        [Fact]
        public void ServiceCreateAndSave_SaveANewItemThatContainsTwoReferencesToAnExistingItem_ThrowsExeption()
        {

            var options = SqliteInMemory.CreateOptions<TestDbContext>();
            using (var context = new TestDbContext(options))
            {
                //SETUP
                context.Database.EnsureCreated();
                var utData = context.SetupSingleDtoAndEntities< NormalEntityDto>();
                utData.AddSingleDto<HasTwoNormalEntityDto>();
                var service = new CrudServices(context, utData.ConfigAndMapper);

                var entity = new NormalEntityDto { MyString = "42" };
                service.CreateAndSave(entity);
                //VERIFY
                service.IsValid.ShouldBeTrue();

                //ATTEMPT
                var AllNormals = service.ReadManyNoTracked<NormalEntityDto>();
                var fistNormal = AllNormals.First(); //In my Case selected by a Combobox

                HasTwoNormalEntityDto hasTwoNormalEntityDto = new HasTwoNormalEntityDto();
                hasTwoNormalEntityDto.NormalEntity1 = fistNormal;
                hasTwoNormalEntityDto.NormalEntity2 = fistNormal;

                Action act = () => service.CreateAndSave(hasTwoNormalEntityDto);
                InvalidOperationException exception = Assert.Throws<InvalidOperationException>(act);
            }          
        }

        [Fact]
        public void DataContextCreateAndSave_SaveANewItemThatContainsTwoReferencesToAnExistingItem_ThrowsNoExeption()
        {

            var options = SqliteInMemory.CreateOptions<TestDbContext>();
            using (var context = new TestDbContext(options))
            {
                //SETUP
                context.Database.EnsureCreated();
                var utData = context.SetupSingleDtoAndEntities<NormalEntityDto>();
                utData.AddSingleDto<HasTwoNormalEntityDto>();
                var service = new CrudServices(context, utData.ConfigAndMapper);

                var entity = new NormalEntityDto { MyString = "42" };
                service.CreateAndSave(entity);
                //VERIFY
                service.IsValid.ShouldBeTrue();

                //ATTEMPT
                var AllNormals = context.NormalEntities.ToList();
                var fistNormal = AllNormals.First(); //In my Case selected by a Combobox

                HasTwoNormalEntity hasTwoNormalEntity = new HasTwoNormalEntity();
                hasTwoNormalEntity.NormalEntity1 = fistNormal;
                hasTwoNormalEntity.NormalEntity2 = fistNormal;

                context.HasTwoNormalEntities.Add(hasTwoNormalEntity);
                var status = context.SaveChangesWithValidation();

                //VERIFY
                status.IsValid.ShouldBeTrue(status.GetAllErrors());

            }

        }

    }

}

I tried to overwrite the update method in the context class with the following code, but this did not lead to a solution:

  public override EntityEntry<TEntity> Update<TEntity>(TEntity entity) where TEntity : class
        {
            if (entity == null)
            {
                throw new System.ArgumentNullException(nameof(entity));
            }

            var type = entity.GetType();
            var et = this.Model.FindEntityType(type);
            var key = et.FindPrimaryKey();

            var keys = new object[key.Properties.Count];
            var x = 0;
            foreach (var keyName in key.Properties)
            {
                var keyProperty = type.GetProperty(keyName.Name, BindingFlags.Public | BindingFlags.Instance);
                keys[x++] = keyProperty.GetValue(entity);
            }

            var originalEntity = Find(type, keys);
            if (Entry(originalEntity).State == EntityState.Modified)
            {
                return base.Update(entity);
            }

            Entry(originalEntity).CurrentValues.SetValues(entity);
            return Entry((TEntity)originalEntity);
        }

Also I have found a class to address this problem, but I haven't found a way to integrate it into the service.

using Microsoft.EntityFrameworkCore;
using System;
using System.Collections;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Text;

namespace GenericServices.PublicButHidden
{
    public static class EfObjectUpdaterExtentions
    {
        // Used to update entities and avoid 'The instance of entity type 'X' cannot be tracked because another instance with the same key value for {'Id'} is already being tracked.' error
        public static void UpdateEntity<T>(
            this DbContext context,
            T existingEntity,
            T newEntity,
            Type[] typesToIgnore = null,
            IEnumerable<Expression<Func<T, object>>> propertiesToIgnore = null
        ) where T : class
        {
            using (var objectUpdater = new EfObjectUpdater())
            {
                objectUpdater.UpdateEntity(context, existingEntity, newEntity,
                    typesToIgnore, null);// propertiesToIgnore?.Select(p => p.GetPropertyFromExpression()).ToArray());
            }
        }

        public static bool IsBasicProperty(this PropertyInfo p)
        {
            return p.IsSpecialName == false && p.GetIndexParameters().Length == 0
            && p.CanRead && p.IsSpecialName == false;
        }
        public static List<Object> ToObjectList(this IEnumerable enumerable)
        {
            return enumerable.Cast<object>().ToList();
        }
    }
    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Interface, Inherited = true)]
    public sealed class PersistentAttribute :
    Attribute
    { }

    public sealed class IgnoreMappingAttribute :
  Attribute
    { }

    public class EfObjectUpdater : IDisposable
    {
        private IEnumerable<string> GetIdPropertyNames(DbContext context, object entity)
        {
            if (entity == null) throw new ArgumentException("Parameter cannot be null.");
            var entityType = entity.GetType();

            return context.Model
                .FindEntityType(entityType)
                .FindPrimaryKey().Properties
                .Select(p => p.Name)
                .ToList();
        }

        private IEnumerable<object> GetIdPropertyValues(DbContext context, object obj)
        {
            if (obj == null) throw new ArgumentException("Parameter cannot be null.");
            var objType = obj.GetType();
            var result = new List<object>();

            foreach (var idPropertyName in GetIdPropertyNames(context, obj))
            {
                result.Add(objType.GetProperty(idPropertyName).GetValue(obj));
            }

            if (Attribute.IsDefined(objType, typeof(ReadOnlyAttribute))) return result;
            return result.All(p => p == null || p.Equals(0)) ? Enumerable.Repeat((object)null, result.Count).ToList() : result;
        }

        private static bool PropertiesAreEqual(string propertyName, object obj1, object obj2)
        {
            if (obj1 == obj2) return true;
            if (obj1 == null || obj2 == null) return false;
            var obj1Property = obj1.GetType().GetProperty(propertyName);
            var obj2Property = obj2.GetType().GetProperty(propertyName);
            return obj1Property?.GetValue(obj1)?.Equals(obj2Property?.GetValue(obj2)) ?? false;
        }

        private object Find(DbContext context, IEnumerable list, object objectToFind)
        {
            if (list == null || objectToFind == null) throw new ArgumentException("Parameters cannot be null.");
            var objectToFindPropertyNames = GetIdPropertyNames(context, objectToFind);
            return list.Cast<object>().SingleOrDefault(listObject => objectToFindPropertyNames.All(p => PropertiesAreEqual(p, objectToFind, listObject)));
        }

        private void UpdateProperties(IEnumerable<string> propertyNames, object existingEntity, object newEntity)
        {
            bool ValuesAreEqual(object v1, object v2) => v1 == v2 || (v1?.Equals(v2) ?? false);
            var entityType = existingEntity.GetType();
            foreach (var propertyName in propertyNames)
            {
                var prop = entityType.GetProperty(propertyName);
                if (prop == null) continue;
                var existingEntityValue = prop.GetValue(existingEntity);
                var newEntityValue = prop.GetValue(newEntity);
                if (ValuesAreEqual(existingEntityValue, newEntityValue)) continue;
                prop.SetValue(existingEntity, newEntityValue);
            }
        }

        private readonly IDictionary<object, bool> _visited = new Dictionary<object, bool>();
        private bool WasVisited(object obj)
        {
            return _visited.ContainsKey(obj);
        }

        private void MarkAsVisited(object obj)
        {
            _visited.Add(obj, true);
        }

        public void UpdateEntity<T>(DbContext context, T existingEntity, T newEntity, Type[] typesToIgnore = null, PropertyInfo[] propertiesToIgnore = null) where T : class
        {
            if (existingEntity == null || newEntity == null) throw new NullReferenceException();
            var existingEntityType = existingEntity.GetType();
            var newEntityType = newEntity.GetType();
            if (existingEntityType != newEntityType) throw new InvalidOperationException();

            if (typesToIgnore?.Contains(existingEntityType) ?? false) return;
            if (WasVisited(existingEntity)) return;
            MarkAsVisited(existingEntity);

            var basicProperties = existingEntityType.GetProperties().Where(p =>
                !(propertiesToIgnore?.Contains(p) ?? false) && p.IsBasicProperty()
            ).Select(p => p.Name).ToList();
            var collectionProperties = existingEntityType
                .GetProperties()
                .Where(p =>
                    basicProperties.All(bp => bp != p.Name) &&
                    p.PropertyType
                        .GetInterfaces()
                        .Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ICollection<>)) &&
                    p.CustomAttributes.All(ca => ca.AttributeType != typeof(IgnoreMappingAttribute))
                )
                .ToList();
            var navigationProperties = existingEntityType
                .GetProperties()
                .Where(p =>
                    basicProperties.All(bp => bp != p.Name) &&
                    collectionProperties.All(cp => cp.Name != p.Name) &&
                    p.CustomAttributes.All(ca => ca.AttributeType != typeof(IgnoreMappingAttribute))
                ).ToList();

            //basic properties
            UpdateProperties(basicProperties, existingEntity, newEntity);

            //collection properties
            foreach (var collectionProperty in collectionProperties)
            {
                var existingCollection = (IList)collectionProperty.GetValue(existingEntity);
                if (existingCollection == null)
                {
                    var collectionType = collectionProperty.PropertyType.GenericTypeArguments.FirstOrDefault();
                    if (collectionType != null)
                    {
                        existingCollection = (IList)Activator.CreateInstance(typeof(List<>).MakeGenericType(collectionType));
                    }
                    else
                    {
                        existingCollection = new List<object>();
                    }
                    collectionProperty.SetValue(existingEntity, existingCollection);
                }
                var newCollection = ((IList)collectionProperty.GetValue(newEntity))?.Cast<object>().ToList() ?? new List<object>();
                var existingCollectionType = existingCollection.Cast<object>().FirstOrDefault()?.GetType();
                var newCollectionType = newCollection.FirstOrDefault()?.GetType();
                if (typesToIgnore?.Contains(existingCollectionType) ?? false) continue;
                if (typesToIgnore?.Contains(newCollectionType) ?? false) continue;

                foreach (var existingElement in existingCollection.Cast<object>().ToList())
                {
                    if (WasVisited(existingElement)) continue;
                    var elementExistsInTheNewList = Find(context, newCollection, existingElement) != null;
                    if (elementExistsInTheNewList) continue;
                    if (Attribute.IsDefined(existingCollectionType, typeof(PersistentAttribute)))
                    {
                        existingCollection.Remove(existingElement);
                    }
                    else
                    {
                        SetEntityState(context, existingElement, EntityState.Deleted);
                    }
                }
                var additionList = new List<object>();
                foreach (var newElement in newCollection)
                {
                    object existingElementInDbSet = null;
                    try
                    {
                        var dbSet = GetDbSet(context, newCollectionType);
                        existingElementInDbSet = InvokeMethod(dbSet, "Find",
                            GetIdPropertyValues(context, newElement).ToArray());
                    }
                    catch (InvalidOperationException)
                    {
                        var dbSet = GetDbSet(context, newCollectionType);
                        List<Object> allElements = ((IEnumerable)dbSet).ToObjectList();

                        existingElementInDbSet = Find(context, allElements, newElement);
                    }
                    if (existingElementInDbSet == null)
                    {
                        UpdateEntity(context, newElement, newElement, typesToIgnore, propertiesToIgnore);
                        additionList.Add(newElement);
                    }
                    else
                    {
                        UpdateEntity(context, existingElementInDbSet, newElement, typesToIgnore, propertiesToIgnore);
                        var existingElement = Find(context, existingCollection, newElement);
                        if (existingElement == null)
                        {
                            additionList.Add(existingElementInDbSet);
                        }
                        else
                        {
                            existingCollection[existingCollection.IndexOf(existingElement)] = existingElementInDbSet;
                        }
                    }
                }
                //Because the above UpdateEntiy call is recursive, if we add the new elemnets (with ID=0)
                //beforehand, only the first one will get to be added to the list (the other ones are with the same ID and misidentified)
                foreach (var o in additionList)
                {
                    existingCollection.Add(o);
                }
            }

            //navigation properties
            foreach (var navigationProperty in navigationProperties)
            {
                if (propertiesToIgnore?.Any(p => p.Equals(navigationProperty)) ?? false) continue;
                if (typesToIgnore?.Contains(navigationProperty.PropertyType) ?? false) continue;
                var newEntityPropertyValue = navigationProperty.GetValue(newEntity);
                var propertyEntityDbSet = GetDbSet(context, navigationProperty.PropertyType);

                if (newEntityPropertyValue == null)
                {
                    if (Attribute.IsDefined(navigationProperty.PropertyType, typeof(PersistentAttribute)))
                    {
                        navigationProperty.SetValue(existingEntity, null);
                    }
                    else
                    {
                        var existingPropertyValue = navigationProperty.GetValue(existingEntity);
                        if (existingPropertyValue != null)
                            SetEntityState(context, existingPropertyValue, EntityState.Deleted);
                    }
                }
                else
                {
                    var existingEntityPropertyValue = InvokeMethod(propertyEntityDbSet, "Find", GetIdPropertyValues(context, newEntityPropertyValue).ToArray());
                    if (existingEntityPropertyValue == null)
                    {
                        navigationProperty.SetValue(existingEntity, newEntityPropertyValue);
                    }
                    else
                    {
                        navigationProperty.SetValue(existingEntity, existingEntityPropertyValue);
                        UpdateEntity(context, existingEntityPropertyValue, newEntityPropertyValue, typesToIgnore, propertiesToIgnore);
                    }
                }
            }
        }

        private static object GetDbSet(DbContext context, Type type)
        {
            return context.GetType().GetMethod("Set").MakeGenericMethod(type).Invoke(context, new object[0]);
        }

        private static object InvokeMethod(object dbSet, string methodName, object[] parameters)
        {
            return dbSet.GetType().GetMethod(methodName).Invoke(dbSet, new object[] { parameters });
        }

        private static void SetEntityState(DbContext context, object obj, EntityState state)
        {
            context.Entry(obj).State = state;
        }

        public void Dispose()
        {
            _visited.Clear();
        }
    }
}

[JonPSmith]

Thank you for taking the time to add your code.

The problem is that you have is because you are trying to create a entity with relationships. What was happening was it was creating NEW HasTwoNormalEntity (which you wanted) and NEW NormalEntity (which I'm sure isn't what you wanted). You then got an exception because the NormalEntityDto had the Id of the NormalEntity in the database so when it tried to add a new NormalEntity with the same key then it throw the exception.

So, one good solution (there are others) is to use the NormalEntity in your HasTwoNormalEntityDto instead of NormalEntityDto and assign the tracked version from the NormalEntity to the two navigational properties. Here is the fixed code.

Change to the HasTwoNormalEntityDto

Note I also took out the NormalEntity1/2Id out too. You have either the the navigational property OR the foreign keys, but not both.

public class HasTwoNormalEntityDto : ILinkToEntity<HasTwoNormalEntity>
{
    public int Id { get; set; }

    public NormalEntity NormalEntity1 { get; set; }

    public NormalEntity NormalEntity2 { get; set; }
}

Changed unit test to show

[Fact]
public void ServiceCreateAndSave_SaveANewItemThatContainsTwoReferencesToAnExistingItem_ThrowsExeption()
{
        //... other parts of the test same as before 

        //ATTEMPT
        var fistNormal = context.NormalEntities.First(); //In my Case selected by a Combobox

        HasTwoNormalEntityDto hasTwoNormalEntityDto = new HasTwoNormalEntityDto();
        hasTwoNormalEntityDto.NormalEntity1 = fistNormal;
        hasTwoNormalEntityDto.NormalEntity2 = fistNormal;

        //.... no exception 
    }
}

Couple of things to say at the end:

• Handling setting up relationships are difficult - do you want a new entity in the relationship or an existing entity from the database? The best solution for adding a existing entity to a relationship is to assign a tracked version of the entity. If you provide a new version then it will create a new version of that entity.
• I don't recommend EfCore.GenericServices for setting up relationships (I use direct EF Core code for that). EfCore.GenericServices is great are reading data and simple updates, which is what I designed if for.
• You were great at giving the code, but the normal was is to provide each class in a separate section, OR build a Console App that contains all the code in one class. It was fine for me, but that's the normal way to handle issues.