zzzprojects / GraphDiff

GraphDiff is a library that allows the automatic update of a detached graph using Entity Framework code first.
https://entityframework-graphdiff.net/overview
MIT License
333 stars 102 forks source link

Navigation Properties updated in incoming entity during UpdateGraph #171

Closed fdahlberg closed 6 years ago

fdahlberg commented 6 years ago

Hi,

I have model that looks like this:

    public class CostModel
    {
        public CostModel()
        {
            CostElements = new List<CostElement>();
        }

        public Guid Id { get; set; }

        public virtual List<CostElement> CostElements { get; set; }

    }
    public class CostElement
    {
        public Guid Id { get; set; }

        public string Name { get; set; }

        [Required]
        public virtual CostModel CostModel { get; set; }
    }

I've created a method for updating the cost model using graphdiff. It looks like this:

        internal static void UpdateCostModel(CostModel model)
        {
            using (var context = new GraphDiffTestDbContext())
            {
                //context.Database.Log = s => System.Diagnostics.Debug.WriteLine(s);

                var updatedModel = context.UpdateGraph<CostModel>(model, map => map
                .OwnedCollection
                (
                    p => p.CostElements
                )
                );

                if(model.CostElements.First().CostModel == model)
                {
                    Debug.WriteLine("Model is the same after update!");
                }
                else
                {
                    Debug.WriteLine("Model is changed after update!");
                }

                context.SaveChanges();
            }
        }

The problem is this: If the costmodel already exist in the database, the navigation property (CostModel) of CostElement is updated in the incoming model. It seems weird to me that the incoming entity is updated. Is this by design, and in that case why, or is this a bug?

Kind Regards Fredrik Dahlberg

JonathanMagnan commented 6 years ago

Hello @fdahlberg ,

Thank you for reporting,

We will investigate this issue very soon and give you an answer.

Best Regards,

Jonathan


Help us to keep this library free: Donate

JonathanMagnan commented 6 years ago

Hello @fdahlberg ,

We have a lot investigated this issue to better understand why this is happening.

We find out that the issue is caused by this method: https://github.com/zzzprojects/GraphDiff/blob/master/GraphDiff/GraphDiff/Internal/Graph/GraphNode.cs#L106-L122

        protected static void AttachCyclicNavigationProperty(IObjectContextAdapter context, object parent, object child)
        {
            if (parent == null || child == null) return;

            var parentType = ObjectContext.GetObjectType(parent.GetType());
            var childType = ObjectContext.GetObjectType(child.GetType());

            var navigationProperties = context.GetNavigationPropertiesForType(childType);

            var parentNavigationProperty = navigationProperties
                    .Where(navigation => navigation.TypeUsage.EdmType.Name == parentType.Name)
                    .Select(navigation => childType.GetProperty(navigation.Name, BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public))
                    .FirstOrDefault();

            if (parentNavigationProperty != null)
                parentNavigationProperty.SetValue(child, parent, null);
        }

The child is an object of the original entity which gets assigned the parent of the new entity.

We tested some scenario which could make existing child entities work but unfortunately for new entities, there is nothing we have found that we can do for now.

Removing this code will lead to more issue since some navigation property will not be correctly copied.

We can leave this issue open but I don't think we will do something until we choose to re-write this library.

Let me know if you need a better explanation of the issue.

Best Regards,

Jonathan


Help us to keep this library free: Donate

fdahlberg commented 6 years ago

Hi Jonathan,

Thanks for investigating. It's a shame it can't be fixed but I can live with this. As a workaround I create a clone of the incoming object before using UpdateGraph.

Thanks Fred