3
\$\begingroup\$

I have a DbContext with a SoftDelete implementation inside it. To make it more clear and maintainable, I wanted to extract it to separate class and use as a component of DbContext or something similar. After first try, it looks it is more complicated than I expected. I also don't want to make it more complex.

The problem is that SoftDelete needs the DbContext itself, like Database and ObjectContext.

I had few ideas so far, using DbContext as a partial class and add SoftDelete partial class. Another is just simply creating a new class and injecting an instance to the DbContexts constructor. Since I am using DI and SoftDelete needs DbContext, it gets into endless loop and making workarounds feels like it's the wrong path to go. For now, partial class is in my mind.

Maybe it is not even worth extracting?

public class ApplicationDb : DbContext, IDbContext, IDisposable
{

    public ApplicationDb()
        : base("DefaultConnection")
    {
        Configuration.LazyLoadingEnabled = false;
        Configuration.ProxyCreationEnabled = false;
    }

    public override int SaveChanges()
    {
        UpdateEntityMetadata(ChangeTracker.Entries());
        ExecuteSoftDelete(ChangeTracker.Entries());
        return base.SaveChanges();
    }

    private void UpdateEntityMetadata(IEnumerable<DbEntityEntry> entries)
    {
        var trackableItems = entries.Where(p => p.Entity is ITrackable);

        foreach (var entry in trackableItems)
        {
            var trackableEntry = entry.Entity as ITrackable;

            if (trackableEntry != null)
            {
                string userId = string.Empty;

                if (HttpContext.Current != null && HttpContext.Current.User != null)
                {
                    userId = HttpContext.Current.User.Identity.GetUserId();
                }

                if (entry.State == EntityState.Added)
                {
                    trackableEntry.Created = DateTime.UtcNow;
                    trackableEntry.CreatedBy = userId;
                }

                trackableEntry.Modified = DateTime.UtcNow;
                trackableEntry.ModifiedBy = userId;
            }
        }
    }

    private void HandleSoftDelete(IEnumerable<DbEntityEntry> entries)
    {
        var deletedItems = entries.Where(p => p.State == EntityState.Deleted && p.Entity is ISoftDelete);

        foreach (var entry in deletedItems)
        {
             var e = entry.Entity;
            string id = string.Empty;

            if (e is IdentityUser || e is ApplicationRole)
            {
                id = ((IdentityUser)e).Id;
            }
            else if (e is BaseModel)
            {
                id = ((BaseModel)e).Id.ToString();
            }

            if (string.IsNullOrEmpty(id))
            {
                throw new ArgumentException("Id not found in SoftDelete() method", id);
            }

            string tableName = GetTableName(e.GetType());
            Database.ExecuteSqlCommand(string.Format("UPDATE {0} SET IsDeleted = 1 WHERE ID = @id", tableName), new SqlParameter("id", id));

            // Marking it Unchanged prevents the hard delete - entry.State = EntityState.Unchanged;
            // So does setting it to Detached and that is what EF does when it deletes an item: http://msdn.microsoft.com/en-us/data/jj592676.aspx
            entry.State = EntityState.Detached;
        }
    }

    private EntitySetBase GetEntitySet(Type type)
    {
        if (mappingCache.ContainsKey(type))
        {
            return mappingCache[type];
        }

        type = GetObjectType(type);
        string baseTypeName = type.BaseType.Name;
        string typeName = type.Name;

        ObjectContext context = ObjectContext;
        var entitySet = context.MetadataWorkspace
                        .GetItemCollection(DataSpace.SSpace)
                        .GetItems<EntityContainer>()
                        .SelectMany(c => c.BaseEntitySets.Where(e => e.Name == typeName || e.Name == baseTypeName))
                        .FirstOrDefault();

        if (entitySet == null)
        {
            throw new ArgumentException("Entity type not found in GetEntitySet() method", typeName);
        }

        return entitySet;
    }

    internal Type GetObjectType(Type type)
    {
        return ObjectContext.GetObjectType(type);
    }

    internal string GetTableName(Type type)
    {
        EntitySetBase entitySet = GetEntitySet(type);

        return string.Format("[{0}].[{1}]", entitySet.Schema, entitySet.Table);
    }
}

To make it clearer, I have removed DbSets and other unrelated methods.

\$\endgroup\$
  • \$\begingroup\$ I think "removing the fluff" might be depriving you of additional tips, and reviewers from additional context. \$\endgroup\$ – Mathieu Guindon Oct 16 '15 at 16:13
  • \$\begingroup\$ Why not write the sprocs and have EF use those instead of the generated SQL so that your code doesn't need to even know that they're soft deletes? \$\endgroup\$ – RubberDuck Oct 16 '15 at 16:37
1
\$\begingroup\$
  1. With a bit more consistent use of LINQ you can simplify UpdateEntityMetadata a little bit:

    private void UpdateEntityMetadata(IEnumerable<DbEntityEntry> entries)
    {
        foreach (var trackableEntry in entries.Select(e => e.Entity).OfType<ITrackable>())
        {
            string userId = string.Empty;
    
            if (HttpContext.Current != null && HttpContext.Current.User != null)
            {
                userId = HttpContext.Current.User.Identity.GetUserId();
            }
    
            if (entry.State == EntityState.Added)
            {
                trackableEntry.Created = DateTime.UtcNow;
                trackableEntry.CreatedBy = userId;
            }
    
            trackableEntry.Modified = DateTime.UtcNow;
            trackableEntry.ModifiedBy = userId;
        }
    }
    
  2. In here you change the value of the method parameter:

    private EntitySetBase GetEntitySet(Type type)
    {
        if (mappingCache.ContainsKey(type))
        {
            return mappingCache[type];
        }
    
        type = GetObjectType(type);
        ...
    

    It's usually better to treat method parameters as readonly (unless they are out or ref) since it makes debugging things a bit easier (plus you may find you want to add some better logging later on and then you have no access to the original type anymore). You won't loose much by creating a new local variable instead:

    var actualType = GetObjectType(type);
    
  3. Also in GetEntitySet you use a mappingCache which seems to be a class member (property or field) and hence by convention should be PascalCase or _PascalCase - camelCase is traditionally used for local variable or method parameters.

  4. This:

        if (e is IdentityUser || e is ApplicationRole)
        {
            id = ((IdentityUser)e).Id;
        }
        else if (e is BaseModel)
        {
            id = ((BaseModel)e).Id.ToString();
        }
    

    has a bit of a design smell on it. All this type checking seems not quite right but without knowing a bit more about the design it's a bit hard to suggest something better.

  5. I can't see where and how HandleSoftDelete is actually called but I do start to get the impression that you are overloading the ApplicationDb class with too many responsibilities. I don't see any reason why it could not be extracted. Most things it relies on could be passed into it or are public properties of the DbContext.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.