4

I have this method in my SurveyController class:

public ActionResult AddProperties(int id, int[] propertyids, int page = 1)
{
    var survey = _uow.SurveyRepository.Find(id);
    if (propertyids == null)
        return GetPropertiesTable(survey, page);

    var repo = _uow.PropertySurveyRepository;

    propertyids.Select(propertyid => new PropertySurvey
                {
                    //Setting the Property rather than the PropertyID 
                    //prevents the error occurring later
                    //Property = _uow.PropertyRepository.Find(propertyid),
                    PropertyID = propertyid,
                    SurveyID = id
                })
                .ForEach(x => repo.InsertOrUpdate(x));
    _uow.Save();

    return GetPropertiesTable(survey, page);
}

The GetPropertiesTable redisplays Properties but PropertySurvey.Property is marked virtual and I have created the entity using the new operator, so a proxy to support lazy loading was never created and it is null when I access it. When we have access direct to the DbContext we can use the Create method to explicitly create the proxy. But I have a unit of work and repository pattern here. I guess I could expose the context.Create method via a repository.Create method and then I need to remember to use that instead of the new operator when I add an entity . But wouldn't it be better to encapsulate the problem in my InsertOrUpdate method? Is there some way to detect that the entity being added is not a proxy when it should be and substitute a proxy? This is my InsertOrUpdate method in my base repository class:

    protected virtual void InsertOrUpdate(T e, int id)
    {
        if (id == default(int))
        {
            // New entity
            context.Set<T>().Add(e);
        }
        else
        {
            // Existing entity
            context.Entry(e).State = EntityState.Modified;
        }
    }
Colin
  • 22,328
  • 17
  • 103
  • 197

2 Answers2

4

Based on the answer supplied by qujck. Here is how you can do it without having to employ automapper:

Edited to always check for proxy - not just during insert - as suggested in comments

Edited again to use a different way of checking whether a proxy was passed in to the method. The reason for changing the technique is that I ran into a problem when I introduced an entity that inherited from another. In that case an inherited entity can fail the entity.e.GetType().Equals(instance.GetType() check even if it is a proxy. I got the new technique from this answer

public virtual T InsertOrUpdate(T e)
{
    DbSet<T> dbSet = Context.Set<T>();

    DbEntityEntry<T> entry;
    if (e.GetType().BaseType != null 
        && e.GetType().Namespace == "System.Data.Entity.DynamicProxies")
    {
        //The entity being added is already a proxy type that supports lazy 
        //loading - just get the context entry
        entry = Context.Entry(e);
    }
    else
    {
        //The entity being added has been created using the "new" operator. 
        //Generate a proxy type to support lazy loading  and attach it
        T instance = dbSet.Create();
        instance.ID = e.ID;
        entry = Context.Entry(instance);
        dbSet.Attach(instance);

        //and set it's values to those of the entity
        entry.CurrentValues.SetValues(e);
        e = instance;
    }

    entry.State = e.ID == default(int) ?
                            EntityState.Added :
                            EntityState.Modified;

    return e;
}

public abstract class ModelBase
{
    public int ID { get; set; }
}
Community
  • 1
  • 1
Colin
  • 22,328
  • 17
  • 103
  • 197
  • You code is naively assuming that any entity with an id value is a proxy object – qujck Jun 19 '13 at 10:11
  • All entities that have an id that is not zero are being updated. So they must have been retrieved from the DbContext in the first place. So how would they not be proxies? – Colin Jun 19 '13 at 13:01
  • In the code I'm working on at the moment the updates come from the web layer calling into WebAPI services - the deserialized instances are new'ed up entities. – qujck Jun 19 '13 at 13:13
  • I may be wrong but CurrentValues.SetValues(e) only sets first-level properties. If you're adding navigation objects there in the model, they won't be created. (example: M:M properties) – Isaac Llopis Dec 24 '15 at 10:55
1

I agree with you that this should be handled in one place and the best place to catch all looks to be your repository. You can compare the type of T with an instance created by the context and use something like Automapper to quickly transfer all of the values if the types do not match.

private bool mapCreated = false;

protected virtual void InsertOrUpdate(T e, int id)
{
    T instance = context.Set<T>().Create();
    if (e.GetType().Equals(instance.GetType()))
        instance = e;
    else
    {
        //this bit should really be managed somewhere else
        if (!mapCreated)
        {
            Mapper.CreateMap(e.GetType(), instance.GetType());
            mapCreated = true;
        }
        instance = Mapper.Map(e, instance);
    }

    if (id == default(int))
        context.Set<T>().Add(instance);
    else
        context.Entry(instance).State = EntityState.Modified;
}
qujck
  • 14,388
  • 4
  • 45
  • 74
  • Superb! 2 things I'd change. 1. We don't need to check the instance when updating. 2. Instead of using automapper we can use DBEntityEntry like this: DbEntityEntry entry = context.Entry(instance); entry.State = EntityState.Added; entry.CurrentValues.SetValues(e); – Colin May 24 '13 at 14:50
  • 1
    Comparing the type of T with an instance created by the context worked for me until I introduced an entity that inherited from another entity. Then that failed the test too, so I changed the technique used to test for a proxy - see the edit in my answer – Colin Sep 10 '13 at 14:44