Итак, суть проблемы. На проекте мы используем OData WebApi и все контроллеры наследуются от базового, используют метод GetService из базового класса который вытягивает зависимости через статический класс ApiControllerScopeContextMediator.
public abstract class ODataControllerBase : ODataController
{
protected T GetService<T>()
{
return ApiControllerScopeContextMediator.GetService<T>(this);
}
}
internal static class ApiControllerScopeContextMediator
{
internal static T GetService<T>(ApiController controller)
{
return (T) controller.Configuration.DependencyResolver.GetService(typeof (T));
}
}
А в Global.asax конфигурируем подтягивание зависимостей для OData через StructureMap:
GlobalConfiguration.Configuration.DependencyResolver = new StructureMapDependencyResolver(container);
Во всех action у контроллеров повсеместно используется метод GetService, как, например, здесь:
public class DisconnectedAppsController : ODataControllerBase
{
public IHttpActionResult Get()
{
var query = GetService<IQuery<IQueryable<DisconnectedAppDomain>, DisconnectedAppFilter>>();
}
}
Но почему? Ведь можно было бы просто использовать constructor injection:
public DisconnectedAppsController(IQuery<IQueryable<DisconnectedAppDomain>, DisconnectedAppFilter> query){
_query = query;
}
Так что же все-таки: «Таити, Таити» (Constructor Injection) или «нас и здесь неплохо кормят» (GetService)?
Какие проблемы с этим кодом я вижу:
- Контроллеры нельзя покрыть unit тестами (без инициализации IoC контейнера)
- Согласно принципу KISS нам не нужна дополнительная сложность, а согласно YAGNI дополнительная завязка на System.Web.Http.Configuration.DependencyResolver, которая вылезет нам боком если мы захотим перейти с MVC5 на MVC6, у которого в корне изменится работы с Dependency Injection, а ApiController в бета версии уже лишился Configuration
- Взглянув на класс мы не видим явно все зависимости класса
- Использование GetService у DependencyResolver — это по сути использование реализации IoC при помощи Service Locator, что является анти-паттерном
- а Service Locator нарушает принципы SOLID
- Мы нарушаем фундаментальный принцип ООП инкапсуляцию
Какие аргументы «против» мне довелось услышать:
- В идеале контроллеры должны быть слишком тупы, чтобы их покрывать тестами, нам это никогда не понадобится, если и покрывать их, то интеграционными тестами
- Отказываться от текущей реализации основываясь на ссылках в гугле слишком идеалистично, это не принесет пользы
- Удобно для разработчика, меньше кода писать
- Изменение кода, который используется подобным образом во многих местах, внесет энтропию в проект
- Метод YAGNI в другой плоскости — а зачем нам менять что-то, если нет очевидной ежеминутной выгоды
- Constructor injection в контроллерах банально неудобен
Пару лет назад прочитал книгу Марка Симана «Dependency Injection». Сижу и думаю, так что у меня с DI: любовь или брак по расчету?
Использованные материалы:
Mark Seeman «Dependency Injection»
Mark Seeman's blog
Microsoft MVC6 github open source project
SOLID wiki page
YAGNI wiki page
KISS wiki page
Комментарии (78)
lair
27.11.2015 15:57+1Constructor injection в контроллерах банально неудобен
Почему?- 27.11.2015 17:18+1
На мой взгляд Constructor injection вполне себе удобен, но в качестве контраргумента довелось слышать, что конструктор разрастается при и при 5 и больше зависимостях выглядит тяжеловесно. К тому же переданные в конструктор параметры нужно инкапсулировать в классе, что добавляет телодвижений.
Но на мой взгляд простота, наглядность и очевидность зависимостей класса в купе с другими преимуществами являются достаточным мотивом для рефакторинга такого кода.lair
27.11.2015 17:21+5конструктор разрастается при и при 5 и больше зависимостях выглядит тяжеловесно
Обычно в таких случаях принято говорить «а не слишком ли много у вас ответственностей, если вам нужно 5+ зависимостей?»
К тому же переданные в конструктор параметры нужно инкапсулировать в классе, что добавляет телодвижений.
Ничем не тяжелее явного запроса зависимости при каждой необходимости.
Razaz
27.11.2015 21:24Сделайте агрегированный сервис. В том же Autofac — Aggregate Services.
Нажать Alt-Enter + Enter для автоматического создания филда и присваивания ему значения тяжело? Как же вы код то пишете?)
Где у вас очевидность зависимостей то?- 27.11.2015 22:15
Где у вас очевидность зависимостей то?
Когда зависимости передают параметрами в конструктор они становятся очевидными, не нужно заведомо знать, что где-то используется GetService зависящий от чего-то.
Фишка решарпера Ctrl+Alt+D (при схеме горячих клавиш JetBrains) на ура решает проблему вынесения параметра конструктора в поле класса.
Поэтому я двумя руками «за» Constructor Injection. Для меня это весомый фактор, мотивирующий на рефакторинг существующего большого количества контроллеров. Ищу способы замотивировать, донести плюсы такого подхода коллегам.Razaz
27.11.2015 23:31Я и говорю, что GetService — порочная практика, как бы ни жаловались на количество параметров конструктора. Всегда есть способы уменьшить кол-во параметров, да и в принципе, этот код, с высокой вероятностью, будет первым кандидатом на декомпозицию.
GetService можно использовать только в спец. случаях, и то, это можно обойти при желании в большинстве случаев.
Вы как -то путаетесь в показаниях. То «Constructor injection в контроллерах банально неудобен», то «На мой взгляд Constructor injection вполне себе удобен» ;)- 28.11.2015 00:02
Моё-то как раз мнение, как в самой статье, так и в комментариях — одинаково. Использовать так GetService это порочная практика.
Какие проблемы с этим кодом я вижу:
речь как раз идет о коде с GetService
На мой взгляд Constructor injection вполне себе удобен, но в качестве контраргумента довелось слышать, что конструктор разрастается при и при 5 и больше зависимостях выглядит тяжеловесно. К тому же переданные в конструктор параметры нужно инкапсулировать в классе, что добавляет телодвижений.
Тем не менее этот контраргумент вовсе не перевешивает «плюсы» Constructor injection.Razaz
28.11.2015 17:17А о чем тогда статья?) Показать как пользоваться тем, что уже давно не рекомендуется использовать? :)
Delphinum
28.11.2015 04:03Потому, что удобнее run method injection ;)
lair
28.11.2015 09:35Аргументируйте.
Delphinum
28.11.2015 14:41+2Нет необходимости определять конструктор и свойства контроллера.
Каждый конкретный обработчик конструктора может иметь отличные от других обработчиков зависимости.lair
28.11.2015 14:57+2Каждый конкретный обработчик конструктора может иметь отличные от других обработчиков зависимости.
Наверное, вы имели в виду, обработчик контроллера, проще говоря — action?
Во-первых, часто ли вам надо иметь у них разные зависимости? А если у них у всех есть одна и та же зависимость, вы получите неслабое дублирование кода (причем если у вас внутри контроллера есть общие методы, то зависимость придется прокидывать и в них).
Во-вторых, обычно параметрами action являются данные, пришедшие в запросе — как вы планируете их разделять?
Ну и в-четвертых, неудобно иметь разные DI-методологии в разных частях системы.mayorovp
28.11.2015 15:06Ну, подходы же можно и объединить: некоторые зависимости — в конструктор, а некоторые — в action.
Разделять данные и сервисы можно атрибутами. Или просто по наличию сервиса в контейнере. Лишь бы не по наличию данных в запросе :)
По поводу же разных методологий в разных частях системы — что именно в этом неудобного? Если, конечно, таких методологий 2-3, а не 10-20.lair
28.11.2015 22:12-2Ну, подходы же можно и объединить: некоторые зависимости — в конструктор, а некоторые — в action.
См. ниже про разные методологии.
Разделять данные и сервисы можно атрибутами.
Лишняя маркировка.
Или просто по наличию сервиса в контейнере.
Можно случайно получить непредсказуемое поведение.
По поводу же разных методологий в разных частях системы — что именно в этом неудобного? Если, конечно, таких методологий 2-3, а не 10-20.
То что теперь, когда я хочу это протестировать, мне надо посмотреть в два-три места вместо одного. Вообще, любая неуниформность увеличивает стоимость поддержки.
Delphinum
28.11.2015 15:07Наверное, вы имели в виду, обработчик контроллера, проще говоря — action?
Совершенно верно, спросони опечатался.
Во-первых, часто ли вам надо иметь у них разные зависимости? А если у них у всех есть одна и та же зависимость, вы получите неслабое дублирование кода
С одной стороны — да, мне частенько нужны различные зависимости в разных методах контроллера. Как минимум очень часто в методах используются разные Mapper'ы. Если же большинство методов использует одну зависимость, то не исключается применение constructor injection именно для этой зависимости.
Во-вторых, обычно параметрами action являются данные, пришедшие в запросе — как вы планируете их разделять?
У меня это обычно делается так:
public function editAction($require, $urlParams){ ... }
то есть части URL, выделенные роутером хранятся в зависимости $urlParams, а параметры запроса, переданные клиентом, в $require. И то, и другое хранится в локаторе, что позволяет разрешить их с помощью DI.
Ну и в-четвертых, неудобно иметь разные DI-методологии в разных частях системы
Почему же?Razaz
28.11.2015 17:16А нету чего-то типа this.Request или this.Request.GetRouteData()? ;) Мапинг вообще делается обычно до вызова метода контроллера(ну во вселенной Asp.Net MVC/WebApi). Просто инъекция в аргументы метода как-то вообще странно выглядит.
Delphinum
28.11.2015 18:02А нету чего-то типа this.Request или this.Request.GetRouteData()?
this вещица хорошая, но она завязывает зависимости на уровне класса, а часто зависимости относятся только к методу. Приведу пример (на PHP, если вы не возражаете):
<?php class UserController{ private $session; public function __construct($session){ $this->session = $session; } public function register($require, $userTable){ $login = $require->param('login'); $pass = $require->param('pass'); ... $id = $userTable->insert(['login' => $login, 'password' => $pass]); $this->session->save(['id' => $id, 'login' => $login]); } public function logout(){ $this->session->delete(); } }
Из примера видно, что общие зависимости, такие как Сессия, разрешаются через конструктор, но частные зависимости, разрешаются для каждого метода отдельно. Здесь зависимости require и userTable не нужны в методе logout, так зачем их разрешать на уровне конструктора? На деле это только осложняет тестирование, так как придется Mock'ать их при тестировании всех методов контроллера, даже если эти зависимости не нужны.
У меня реализуется это тоже довольно просто:
<?php $manager = new Di\Manager; $manager->set('require', $require); $manager->set('session', $session); $manager->set('userTable', function(){ ... return $userTable; }); // Инстанциация контроллера через разрешение зависимостей конструктора $controller = $manager->constructInjection($controllerName); // Вызов экшена контроллера с разрешением его зависимостей $result = $manager->methodInjection($controller, $action); $response->body($result); $response->send();Razaz
28.11.2015 22:30Имхо профиту особо я не вижу, а вот проблем может добавить, особенно если баловаться с интроспекцией параметров, проксированием и тд.
С инъекцией в конструкторе все то же выглядит весьма прилично:
public AccountController( UserManager<ApplicationUser> userManager, SignInManager<ApplicationUser> signInManager) { _userManager = userManager; _signInManager = signInManager; } public async Task<IActionResult> Register(RegisterViewModel model) { if (ModelState.IsValid) { var user = new ApplicationUser {UserName = model.Email, Email = model.Email}; var result = await _userManager.CreateAsync(user, model.Password); if (result.Succeeded) { await _signInManager.SignInAsync(user, isPersistent: false); return RedirectToAction(nameof(HomeController.Index), "Home"); } } return View(model); }
По мне так логичней, что класс, выполняющий набор действий, заранее сообщает о своих зависимостях, а не в момент выполнения вдруг оказывается, что зависимость, которую он попросил, недоступна. Ну и сигнатуры методов будут огого.Delphinum
29.11.2015 13:46По мне так логичней, что класс, выполняющий набор действий, заранее сообщает о своих зависимостях
Почему же тогда я не встечаю классы вида:
class MyClass{ ... public function __construct($x, $y, $z, $a, $b, $c, ...){ ... } public function methodA(){} public function methodB(){} public function methodC(){} }
lair
28.11.2015 22:17-1да, мне частенько нужны различные зависимости в разных методах контроллера.
Вы уверены, что это не признак нарушения SRP?
Как минимум очень часто в методах используются разные Mapper'ы.
Ыыы, а у нас просто один маппер позволяет мапить все объекты.
части URL выделенные роутером хранятся в зависимости $urlParams, а параметры запроса, переданные клиентом, в $require.
Это все прекрасно, но где зависимости?
(Впрочем, вру, не прекрасно. Прекрасно — это вот так:
public async Task Put(string id, Location data) {...}
)
И то, и другое хранится в локаторе, что позволяет разрешить их с помощью DI.
Но зачем? И, главное, по каким признакам?
Почему же?
Выше уже ответил. Униформность нарушается.Delphinum
29.11.2015 13:50-1Ыыы, а у нас просто один маппер позволяет мапить все объекты
Один контроллер на все запросы не используете? )
Это все прекрасно, но где зависимости?
Внутри метода контроллера он выделяет нужные ему параметры и валидирует их.
Прекрасно — это вот так
Реализовывал подобное, да это удобно, но я вынес это на уровень аннотаций, так как в PHP все равно строго типизировать параметры не выходит.
Но зачем? И, главное, по каким признакам?
Сервисы должны хранится в локаторе.
Униформность нарушается
Тобишь если у нас есть одно рабочее решение, его нужно превращать в золотой молоток?lair
29.11.2015 18:23+2Один контроллер на все запросы не используете? )
Нет. Маппер — это сервис, у него нет бизнес-смысла, в отличие от контроллера.
Внутри метода контроллера он выделяет нужные ему параметры и валидирует их.
Это лишний повторяющийся код.
Сервисы должны хранится в локаторе.
Вот только пришедшие снаружи данные — это не сервисы. Что им делать в локаторе?
Тобишь если у нас есть одно рабочее решение, его нужно превращать в золотой молоток?
Если у нас есть рабочее решение, то не надо рядом внедрять почти такое же, но другое. Это будет путать программиста.Delphinum
29.11.2015 18:55-1Это лишний повторяющийся код
Который можно легко унифицировать и превратить в не повторящийся, что и было сделано с помощью аннотаций.
Вот только пришедшие снаружи данные — это не сервисы. Что им делать в локаторе?
Пришедшие снаружи данные можно определить как пакет, который вполне можно поместить в локатор для доступа к ним из любой части обработчика.
Если у нас есть рабочее решение, то не надо рядом внедрять почти такое же, но другое. Это будет путать программиста
Это самое «почти» имеет очень большое значение ) Превращать все частные зависимости методов в зависимости объекта, это золотой молоток.lair
29.11.2015 19:11+2Который можно легко унифицировать и превратить в не повторящийся, что и было сделано с помощью аннотаций.
Значит, это происходит не внутри метода контроллера.
Пришедшие снаружи данные можно определить как пакет, который вполне можно поместить в локатор для доступа к ним из любой части обработчика.
Зачем? Это, по факту, глобальное состояние.
Превращать все частные зависимости методов в зависимости объекта, это золотой молоток.
Понимаете ли, в чем дело: «зависимость» объекта — это его внутреннее дело. Сегодня они одни, а завтра — другие; сегодня они используются в одном методе, завтра — во всех. В то же время, параметры его методов — это его внешний контракт, чем меньше он меняется, тем лучше.Delphinum
29.11.2015 19:17Значит, это происходит не внутри метода контроллера
Все, чего бы вы не делали, можно выполнить как внутри некого метода, так и вне его. Не вижу аргументации в этом.
Зачем? Это, по факту, глобальное состояние
Чтобы можно было разрешить их как зависимость. Очевидно же.
Сегодня они одни, а завтра — другие; сегодня они используются в одном методе, завтра — во всех. В то же время, параметры его методов — это его внешний контракт, чем меньше он меняется, тем лучше
А семантика контроллера, это не контракт? )lair
29.11.2015 19:44Чтобы можно было разрешить их как зависимость. Очевидно же.
А зачем вам разрешать состояние как зависимость?
А семантика контроллера, это не контракт?
Нет.Delphinum
29.11.2015 20:02А зачем вам разрешать состояние как зависимость?
Потому что это не состояние, а пакет данных пользователя, от которого зависят некоторые методы моего контроллера.
Нет
То есть в изменении семантики контроллера вы проблем не видите, а в изменении семантики других методов видите? )lair
29.11.2015 20:16Потому что это не состояние, а пакет данных пользователя, от которого зависят некоторые методы моего контроллера.
Методы контроллера не могут зависеть от пакета данных пользователя, пакет данных — это входные данные, аргументы.
а в изменении семантики других методов видите?
Так в том-то и дело, что зависимости никак не определяют семантику метода. Поэтому меня и раздражает, что сугубо внутреннее устройство (зависимости) проползают во внешний контракт.Delphinum
29.11.2015 20:51Методы контроллера не могут зависеть от пакета данных пользователя, пакет данных — это входные данные, аргументы
Мысленно назовите всю совокупность этих данных пакетом и попробуйте сделать так, чтобы метод от них «не зависел».
Так в том-то и дело, что зависимости никак не определяют семантику метода
Еще как определяют.lair
29.11.2015 20:53Мысленно назовите всю совокупность этих данных пакетом и попробуйте сделать так, чтобы метод от них «не зависел».
У нас с вами какая-то терминологическая путаница. Функция (в математическом смысле) не «зависит» от своих аргументов. «Зависимость» метода (уже в программном смысле) — это какой-то объект, нужный ему для корректного выполнения задачи. Входные аргументы зависимостями не являются.
Еще как определяют.
Неа. Семантика метода, скажем,PutArticle— в том, чтобы сохранить статью (в некое хранилище). То, что при этом ему нужен сервис безопасности и (особенно) сервис логирования — это не его семантика.Delphinum
29.11.2015 20:58«Зависимость» метода (уже в программном смысле) — это какой-то объект, нужный ему для корректного выполнения задачи
Почему именно объект? Если я обмениваюсь данными с сервером в виде JSON, то это уже зависимости?
То, что при этом ему нужен сервис безопасности и (особенно) сервис логирования — это не его семантика
Откуда же метод получит сервис логирования?lair
29.11.2015 21:17Почему именно объект?
Потому что обычно внутрение сущности программы в ООП — это объекты.
Если я обмениваюсь данными с сервером в виде JSON, то это уже зависимости?
Нет, это входные (или выходные) данные.
Откуда же метод получит сервис логирования?
А это не имеет никакого значения для его семантики. Более того, его семантика не изменится от того, использует он метод логирования или нет.Delphinum
30.11.2015 00:21Потому что обычно внутрение сущности программы в ООП — это объекты
Нет, это входные (или выходные) данные
Которые «объекты», которые нужны для корректного выполнения задачи. Все сходится.
А это не имеет никакого значения для его семантики. Более того, его семантика не изменится от того, использует он метод логирования или нет
То есть не важно как метод получит свои зависимости, главное чтоб семантику ему не поменять? )lair
30.11.2015 02:35Которые «объекты», которые нужны для корректного выполнения задачи. Все сходится.
Не, не сходится. Входные данные — это не зависимости.
То есть не важно как метод получит свои зависимости, главное чтоб семантику ему не поменять?
Да, именно так.
Cortlendt
27.11.2015 16:54+1Суть проблемы в том, что вам нужно подружить два фреймворка, которые диктуют дизайн — DI Container и OData WebApi. Так как оба фреймворка хотят «контроллировать» конструктор, нужен какой-то "
костыльадаптер" для того чтобы их подружить.
Вы описали типичную проблему интеграции и к самим принципам DI она не имеет отношения.lair
27.11.2015 17:22А в каком месте OData WebAPI «хочет контролировать конструктор»? (собственно, к DI Container тоже относится)
Cortlendt
27.11.2015 17:30+1DI Container — «создает граф объектов», «зависимости декларируются в конструкторе (обычно)».
OData WebAPI «содает контроллеры для обработки запросов», «наследуется базовый контроллер, который может дать ограничение на конструктор».
Соотвествено реализация IDependencyResolver и является адаптером, который позволяет интегрировать оба фреймворка.lair
27.11.2015 17:40-1зависимости декларируются в конструкторе (обычно)
Могут, но не обязаны.
наследуется базовый контроллер, который может дать ограничение на конструктор.
Наследуемый базовый контроллер не налагает никаких ограничений на конструктор.
Соотвествено реализация IDependencyResolver и является адаптером, который позволяет интегрировать оба фреймворка.
Неа.IDependencyResolver(за компанию с еще некоторыми классами) — это точка расширения, позволяющая использовать в WebAPI тот DI-контейнер, который нужен программисту. Совершенно штатная.
- 27.11.2015 17:49-2
Неа. IDependencyResolver (за компанию с еще некоторыми классами) — это точка расширения, позволяющая использовать в WebAPI тот DI-контейнер, который нужен программисту. Совершенно штатная.
Соглашусь с этим утверждением. Но сразу принял бы во внимание, что в MVC6 этой точки расширения уже не будет, поскольку DI-контейнер будет интегрирован в сам фреймворкlair
27.11.2015 17:53+2Будет другая,
app.ApplicationServices. Всей-то разницы.- 28.11.2015 15:25
internal static T GetService<T>(ApiController controller) { return (T) controller.Configuration.DependencyResolver.GetService(typeof (T)); }
Но от завязки на этот код в статическом классе придется избавляться.lair
28.11.2015 22:22Если вам очень захочется, то через ControllerContext доберетесь. Но учитывая, что
ApiControllerтоже нет, вам в любом случае этот код переписывать.
Cortlendt
27.11.2015 17:50-3Неа. IDependencyResolver (за компанию с еще некоторыми классами) — это точка расширения, позволяющая использовать в WebAPI тот DI-контейнер, который нужен программисту. Совершенно штатная.
Что собственно я и написал. И нужен он потому что оба фреймворка завязаны на использование конструкторов. Вы или читать не умеете или просто некомпетентны.lair
27.11.2015 17:58Как раз наоборот, ни один из фреймворков (более того, DI-контейнер — это не фреймворк) не завязан на использование конструкторов (насколько вообще создание объектов в .net может не быть завязано на использование конструктора).
В WebAPI контроллеры создаются через IHttpControllerActivator, который делегирует создание в IDependencyResolver, а за его отсутствием вызывает Activator.CreateInstance. OData не накладывает никаких ограничений на конструктор: у стандартного конструктора нет параметров.
Весь смысл этой точки расширения в том, чтобы дать пользователю возможность использовать свой DI. Конструкторы, не конструкторы — никого уже не волнует. Хоть синглтоны.
qw1
28.11.2015 13:18Чтобы минимизировать синтаксический оверхед от contructor injection, я пользуюсь таким шаблоном:
public class HomeController : Controller { [Dependency] public IService1 Service1 { get; set; } [Dependency] public IService2 Service2 { get; set; } public HomeController(IUnityContainer uc) { uc.BuildUp(this); } public ActionResult Index() { var data = Service1.Load(); // ... } }lair
28.11.2015 14:57+3… и теперь ваш контроллер, как и его тесты, напрямую зависит от DI-контейнера.
qw1
28.11.2015 16:05Да. Но я ещё не встречал проекта, где на полпути меняли бы БД, ORM или контейнер.
Обычно в таких ситуациях переписывают с нуля «сразу правильно».Razaz
28.11.2015 17:20Тут не в смене дело. Ваш контроллер теперь узнал о существовании Unity. В том же Autofac достаточно зарегестрировать с AutowireProperties и не надо в класс контроллера добавлять такие штуки. Плюс еще и зависимость на контейнер таскать придется.
qw1
28.11.2015 19:02Нужны ли юнит-тесты на контроллеры? Я стараюсь всё сложное переносить в сервисы, и их тестировать.
Основная работа контроллера — наполнение ViewBag, зачастую код во View сложнее, чем в контроллере. Если и тестировать, то связку Controller+View, а выходной HTML из View непонятно, как тестировать (у кого-то есть опыт?).Razaz
28.11.2015 22:34Да, нужны. Content negotiation, генерация роутов, локализация(в том числе и вьюх). Например простейший тест на локализацию — поиск временных строковых токенов в html. Да даже проверить тот же ViewBag. А еще проверить обработку исключений например и их мапинг в статус коды. В общем надо:)
dmitry_pavlov
29.11.2015 21:41Razaz — в качестве альтернативы юнит тестам в этой ситуации, чтобы отловить указанные моменты в работе контроллера Web API или MVC приложения можно делать в тестах запросы через HttpClient или подобными способами — http://www.diogonunes.com/blog/webclient-vs-httpclient-vs-httpwebrequest/
Запускается в вместе юнит тестами, но по сути являются уже интеграциоными. Выглядит и работает хорошо, сигнализирует о проблеме — тоже. В response будет все, что вы ожидаете или наоборот.
Думаю, с помощью таких вот интеграционных тестов на контроллеры (покрыващих вопросы контента, маршрутизации, и тп) — вполне можно смириться с непокрытием их обычными юнит тестами. Особенно если (как справедливо заметил qw1 ) логику держать все-таки в сервисах и юнит-тестировать их.
Вполне все удобно получается. И зависимости контроллера не помешают. Для интерграционных тестов достаточно иметь поднятое окружение для тестирования. URL прописать в конфиг его для тест проекта. Локально чтобы на локальной машине гонялись, На билд сервере — на тестовом окружении.Razaz
30.11.2015 00:57Проблема как раз в том, что являются интеграционными :)
Мне как-то не нравится оставлять не покрытыми контроллеры, которые участвуют в реализации каких-либо протоколов(OAuth, OIDC, SCIM) например :) Если можно покрыть тестами, то почему нет? Плюс не всегда есть возможность на каждый билд разворачивать новый тестовый инстанс приложения.
Плюс ко всему прочему страдает атомарность тестов. При тестировании контроллеров я могу сразу понять где проблема — в контроллере, в фильтре или в middlware. При интеграционном тесте можно долго ковыряться с тем, что и где пошло не так.dmitry_pavlov
30.11.2015 09:35Если нет continuous integration и тестовое окружение не доступно на каждый билд — это, конечно, сложнее. В остальном — как раз вызовом API программно через вебклиента вы по сути лучше проверяете взаимодействие с вашим приложением извне — более честно. Ведь методы контроллера никто напрямую не вызывает как библиотечные. Соответственно, чтобы задействовать все праоверки доступа, маршрутизацию и тп — как раз и удобно подергать контроллер через URL.
Плюсов в данном подходе на мой взгляд больше, чем минусов в случае сложностей либо с непокрытыми вовсе контроллерами, либо с войной против зависисмостей, даже подмена которых для тестовых целей точно не всегда обеспечит вас уверенностью, что все что вы хотите проверить, действительно отработает в рабочем окружении.
Так не думаете?Razaz
30.11.2015 10:48Окружение доступер, но тест охватывает слишком большой кусок функциональности за раз, при этом не предоставляя возможности протестировать отдельные части. Можно все протестировать без вызова контроллера извне. Плюс с такими тестами гораздо проще отладиться, так как не надо постоянно слать запросы — достаточно просто подебажить нужный кусок.
lair
28.11.2015 22:22Но я ещё не встречал проекта, где на полпути меняли бы БД, ORM или контейнер.
Я вот пару недель назад менял на проекте DI-контейнер.
Angelina_Joulie
29.11.2015 12:09Значит проекты были не поворотливыве.
В парочке больших проектов, мы уже пару раз меняли ORM — максимум неделя, и 1-2 дня отлов багов. Это не страшно.
DI — мы ещё не меняли, не видем в этом смысла Unity хоть и тежеловат по сравнению с конкурентами, но его гибкость пока недостежимая высота для других.
mayorovp
28.11.2015 15:01Конструктор, принимающий контейнер DI — это антипаттерн, поскольку вносит в класс дополнительную зависимость, не упрощая при этом ничего.
Сравните: new HomeController(uc) и uc.Resolve<HomeController>(). Первый способ создания объекта ничуть не проще второго.
qw1
28.11.2015 16:07Я не делаю
new HomeController(uc). Если бы создание контроллера было в моём коде, я бы написал через Resolve, но поскольку ASP.NET MVC делает constructor injection, тут такой костыль.lair
28.11.2015 22:24+1поскольку ASP.NET MVC делает constructor injection, тут такой костыль.
asp.net MVC не делает constructor injection. asp.net MVC передает запрос в IDependencyResolver, а тот — вашему DI-контейнеру.qw1
29.11.2015 00:26+1Это всё SO-driven programming )) Если изучить документацию перед написанием кода, можно упростить код. Танцы с BuildUp в конструкторе не нужны.
omikad
29.11.2015 03:25+1В идеале контроллеры должны быть слишком тупы, чтобы их покрывать тестами, нам это никогда не понадобится, если и покрывать их, то интеграционными тестами
Я за этот вариант, потому что есть дополнительный плюс. Представьте что в контроллере у вас есть интересный код, который потребовался вам, ну например, в вебджобе, или в другом сайте. Что будете делать зависимость от этого веб проекта? Нет, придется утаскивать в некоторую библиотеку. Поэтому можно сразу делать все этой библиотеке, а в контроллерах оставлять только самый наитупейший код, который работает на очень высоком уровне абстрагирования, такой код и тестировать не захочется. Один минус что придется пробрасывать вызовы из контроллера в вашу либу, ну да и ладно, зато уходим от всех проблем борьбы с чужими фреймворками
Angelina_Joulie
29.11.2015 12:06Добавлю и свои 5 копеек.
1. constructor injection — это читаемость кода. Сразу видно, что необходимо для работы конкретного класса.
к примеру:
public class MyController: Controller
{
ctor(IReader reader, IWriter writer);
}
и любому читающему понятно, что тут R/W операции, не вчитываясь в остальные строчки кода.
Использование injection method — это штуковина, нужна для экзотических вещей, когда в время построения нужно преодолеть рекурсивную зависимость объектов один от другого. Ну и ещё для парочки специфических сценариев.
Если у класса есть опциональные зависимости, то такие зависимости лучше подсовывать через property injection.
Вообщем, правило должно быть такое: то, без чего работа класса не возможна — в конструктор, всё остальное property injection.Razaz
30.11.2015 01:03+1Property Injection в принципе рекомендуется только для специфических случаев. Например, если каждый класс использует ILogger или используется наследование. Ну или если конструктор нельзя трогать(Сейчас это вообще редкость).
Что это за опциональные зависимости такие? Типа есть работаю, а нет начинаю менять поведение? :)
IL_Agent
30.11.2015 10:17Что это за опциональные зависимости такие? Типа есть работаю, а нет начинаю менять поведение?
Да. Достигается с помощью создания нескольких конструкторов, либо, как было сказано выше, через property injection. Есть мнение, и я с ним согласен по большому счёту, что это антипаттерн.Razaz
30.11.2015 10:51Я понимаю, что можно баловаться всякими Constructor Selector и подобными фокусами, просто давно уже не возникала необходимость в таком хитром трюке. Полностью согласен с автором статьи.
IL_Agent
На мой взгляд единственный серьёзный аргумент «против» — использование одновременно двух подходов. Остальные — надуманны.
Чем неудобен Constructor Injection?
Писать больше кода? А если сервис нужен в нескольких местах? Писать
несколько раз?
IL_Agent
И ещё. При использовании Constructor Injection в случае невозможности разрешить зависимость вы получите ошибку сразу при создании объекта зависимого класса, а не неизвестно когда в процессе работы.
js605451
[сарказм] Именно для таких ситуаций и существует паттерн проектирования Helper/Utils!
HomoLuden
Подход с «тонкими» контроллерам используют в Rails среде, когда пишут RESTful API Application. С ним действительно упрощается тестирование.
Тестами покрываются только модели и функционирование UI. При этом, т.к. основное приложение чисто API провайдер, то UI реализуется и тестируется отдельно (например, с AngularJS либо EmberJS).
Лично я вижу «зерно» в этом доводе.