Onion архитектура asp.net mvc core приложения

В данный момент существует множество подходов к созданию архитектуры MVC приложений (Domain Driven Design, 3d-layered architecture, onion structure, etc).

Мой любимый подход это 3х уровневая архитектура и вот почему:

1. Позволяет легко избавиться от зависиомтей используя Inversion of Control
2. Каждый из уровней отвечает за свои задачи, что позволяет придерживаться принципа Single responsibility даже на уровне слоя.

Приложение ASP.NET MVC с этим подходом выглядело бы так:

1. Data layer - библиотека, которая организовывает связь с базой данных, в неё входят UoW, Repository pattern, EF если имеется и Data models
2. Bussiness layer - библиотека отвечающая за сервисы, бизнес логику и DTO (data transfer objects которые связывают Data Models и View Models)
3. Presentation layer - собственно сам MVC, сюда входят ViewModels, View, Controllers, конфигурация IoC и т.д.

Благодаря такой архитектуре изменения в одном уровне не повлекут за собой огромных изменений (а при правильном построении SOLID вообще не изменят другие слои).

Теперь по поводу реализации и вопросов:

1. Контроллеры должны быть по принципу as thin as possible и любая бизнес логика в них говорит о том что реализацию можно улучшить и всю логику можно и нужно выносить в сервисы. В вашей реализации присутствует обработка изображений, которую по хорошему следует вынести в отдельный сервис назвав его ImageManipulationService и убрать из контроллеров #helpers
2. Любую статику очень сложно покрыть тестами в будущем, лучше всего придерживаться реализации этих паттернов как объектов - в будущем будет намного легче поддерживать приложение.
3. Интерфейс IDisposable специально был разработан для высвобождения ресурсов, которые не будут уничтожены Garbage Collector`ом (в основном это касается unsafe кода), в реализации UoW считается хорошей практикой высвобождать DbContext перед уничтожением самого UoW, так что паттерн реализован хорошо.
4. Использование Generic Repository довольно распространенная практика и это намного лучше чем писать под каждую сущность новый объект репозитория. Не вижу никаких проблем использовать этот паттерн.
5. У приложения может быть несколько контекстов базы данных в случае если приложение использует несколько БД одновременно. На практике такое случается крайне редко и если это вопрос по поводу реализации общего интерфейса для DbContext то это скорее overengeenering чем хорошая практика.

PS. Под Generic Repository хорошо подходит реализация Generic UoW:

public interface IUnitOfWork : IDisposable  
{
    T GetRepository<T>() where T : class;
    int Save();
}
public class UnitOfWork : IUnitOfWork  
{
    private Dictionary<string, object> _repositories;
    private IDbContext _dbContext;
    public UnitOfWork() : this(new AppContext())
    {
    }
    public UnitOfWork(IDbContext dbContext)
    {
        _dbContext = dbContext;
        _repositories = new Dictionary<string, object>();
    }
    /// <summary>
    /// Search for repository in dictionary and if not exists creating new.
    /// </summary>
    /// <typeparam name="T">Type of repository to create.</typeparam>
    /// <returns>Returns repository with DbContext provided by UoW.</returns>
    public T GetRepository<T>() where T : class 
    {
        if (!_repositories.ContainsKey(nameof(T)))
        {
            var repository = (T)Activator.CreateInstance(typeof(T), _dbContext);
            _repositories.Add(nameof(T), repository);
        }
        return (T)_repositories[nameof(T)];
    }
    /// <summary>
    /// Saves all pending changes.
    /// </summary>
    /// <returns>The number of objects in an Added, Modified, or Deleted state</returns>
    public int Save()
    {
        return _dbContext.SaveChanges();
    }
    /// <summary>
    /// Disposes current object.
    /// </summary>
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
    private void Dispose(bool disposing)
    {
        if (disposing)
        {
            if (_dbContext != null)
            {
                _dbContext.Dispose();
                _dbContext = null;
            }
        }
    }
}

2. Во-первых, DbContext сам по себе является и репозиторием и единицей работы. Поэтому делать обёртки поверх него нет смысла. Имхо.

Статический UnitOfWork? Это как вообще? Смысл этого паттерна в том, что создаётся юнит, выполняется какая-то работа, закрывается юнит, что приводит к сохранению (или откату) данных, освобождению занятых ресурсов и т. п. То есть он никак не должен быть статическим.

3. Как уже сказано выше, единица работы должна создаваться и удаляться по ходу работы. Реализовав интерфейс IDisposable, можно использовать удобную конструкцию using, что приведёт к закрытию юнита даже в случае исключений.

Но, как я уже сказал, DbContext не нуждается в обёртках.

5. Их может быть сколько угодно. Например, один контекст может быть для работы с данными пользователей сайта, другой - для админов. Однако, чем их больше, тем тяжелее писать, а главное - поддерживать код.

Сделав несколько контекстов, намного сложнее осуществлять миграции! Спасибо tym32167 за напоминание.