Logic for changing entity navigational properties States

Question

The question might seem a little bit long, but I will be very glad if you have read it till the end and told me your opinions.

Let's say I have such entity:

public class FirstEntity
{
    public int Id { get; set; }
    ...

    public virtual SecondEntity SecondEntity { get; set; }
    public int SecondEntityId { get; set; }

    public virtual ICollection<ThirdEntity> ThirdEntities { get; set; }
}

And here is the SecondEntity and ThirdEntity:

public class SecondEntity
{
    public int Id { get; set; }
    ...

    public virtual FirstEntity FirstEntity { get; set; }
}

public class ThirdEntity
{
    public int Id { get; set; }
    ...

    public virtual FirstEntity FirstEntity { get; set; }
}

While inserting entity I am setting it's state to Added:

 Context.Entry(entity).State = EntityState.Added;

In this case, all child entities states are changing to Added also.

The problem is I must not insert some entities if they already exist in the database. So, for example if some SecondEntity exists in the database, then I must change it's state to Detached, but also I must set SecondEntityId value to existing SecondEntity in the database.

IMHO, it is pretty common problem. But, I can't find any question like this in SE.

I have solved this problem. But I wonder, is my algorithm any good?

I have created two attributes: [MustBeChecked] and [ModifyState("SecondEntityId")]

Logic is that, MustBeChecked is the navigational property which includes navigational properties which must be checked. And these properties has decorated with ModifyState attribute and the name of the property which must be set to the key value of the existing row in the database.

For example, FirstEntity will look like that:

[MustBeChecked]
public class FirstEntity
{
    public int Id { get; set; }
    ...

    [ModifyState("SecondEntityId")]
    public virtual SecondEntity SecondEntity { get; set; }
    public int SecondEntityId { get; set; }

    [ModifyState]
    public virtual ICollection<ThirdEntity> ThirdEntities { get; set; }
}

Also I have one type per entity which has DefineState method. I call it with relfection in runtime. It checks if this entity exists in the database or not and then sets the property value in parent object. For example, one of them:

public class SecondEntityState : IStateDefiner<SecondEntity>
{
    public Phone DefineState(DbContext context, SecondEntity model, object parent, string propertyName)
    {
        var entity = context.Set<SecondEntity>().AsQueryable().FilterByUniqueProperties(model).SingleOrDefault();

        if (entity != null)
        {
            if (!String.IsNullOrEmpty(propertyName))
            {
                parent.GetType().GetProperty(propertyName).SetValue(parent, entity.Id);
            }

            context.Entry(model).State = System.Data.EntityState.Detached;
        }

        return model;
    }
}

And I am iterating over entity properties recursively to find such navigational properties and process over them.

public void ModifyState<TEntity>(TEntity entity)
        {
            var properties = entity.GetType()
                .GetPropertiesWithAttribute<MustBeCheckedAttribute>();

            // Iterate over attribute details
            foreach (var prop in properties)
            {
                // Check if the property is collection
                if (typeof(IEnumerable).IsAssignableFrom(prop.PropertyType))
                {
                    var collection = entity.GetType()
                        .GetProperty(prop.Name)
                        .GetValue(entity);

                    foreach (var collectionElement in collection as List<object>)
                    {
                        ModifyState(collectionElement);
                        ProcessOverProperties(collectionElement);
                    }
                }
                else
                {
                    ModifyState(entity);
                    ProcessOverProperties(entity);
                }
            }
        }

        /// <summary>
        /// Will find properties which has decorated with ModifyState attribute.
        /// And then will select property and IdContainerPropertyName value for this attribute.
        /// We are going to use these informations for detaching navigational property
        /// and setting corresponding property's value
        /// </summary>
        /// <typeparam name="TEntity">Entity type</typeparam>
        /// <param name="entity">Entity object</param>
        private void ProcessOverProperties<TEntity>(TEntity entity)
        {
            var modifyStateAttributes = entity.GetType()
                .GetPropertiesWithAttribute<ModifyStateAttribute>()
                .Select(prop =>
                    new
                    {
                        Property = prop,
                        IdContainer = (Attribute.GetCustomAttribute(prop, typeof(ModifyStateAttribute)) as ModifyStateAttribute).IdContainerPropertyName
                    });

            // Iterate over attribute details
            foreach (var attributeInformation in modifyStateAttributes)
            {
                var stateDefinerInstance = EntityStateFactory.CreateStateDefiner(attributeInformation.Property.PropertyType);

                MethodInfo methodInfo = stateDefinerInstance.GetType().GetMethod("DefineState");
                methodInfo.Invoke(stateDefinerInstance, new object[] 
                    { 
                        this, 
                        entity.GetType().GetProperty(attributeInformation.Property.Name).GetValue(entity), 
                        entity, 
                        attributeInformation.IdContainer
                    });
            }
        }

Is my logic any good?

Answer 1

I'm going to answer in the context of comparing this to GraphDiff since it was mentioned in the comments.

First, make sure you have a need to update objects in this fashion. I'm assuming you do.

Implementation-wise, the main difference between GraphDiff and this approach is that GraphDiff builds a graph object and then visits it, while this approach uses recursion for a more procedural approach.

As for the use of reflection, I think it's required to solve this problem. I can't see a way around it.

I see this as a valid approach. It requires custom attributes on entities and some boilerplate code in places, but short of writing a library that extends EF objects, I think you kind of have to do something like this.

As long as you can verify that you won't get into

1. circular reference issues
2. stack overflows from recursing too deeply

then I would say it's a valid approach. It's clean, and it appears easy enough to maintain.

Two suggestions:

1. I would suggest storing your objects you get from reflecting on types in a static dictionary so you don't have to use reflection every time, just the first time.

2. Consider making an abstract or at least a parent class for a base implementation of your State classes. I think there's some code in your DefineState methods that you that you can avoid duplicating in every method definition by pulling out into a base implementation. Can you possibly use generic parameters and/or one generic method?

Overall though, let performance be the judge. As long as your app isn't taking a significant hit from this implementation, I say you should congratulate yourself on finding a solution to a rather complex problem.