На Хабре уже была статья, посвящённая Dependency Injection в Ruby, но упор в ней был больше на использование паттерна IoC-container с помощью гемов dry-container и dry-auto_inject. А ведь для использования преимуществ внедрения зависимостей совершенно необязательно городить контейнеры или подключать библиотеки. Сегодня расскажу о том, как по-быстрому реализовать DI своими руками.


Описание подхода


Для чего люди используют DI? Обычно для того, чтобы во время тестов менять поведение кода, избегая вызовов ко внешним сервисам или просто для тестирования объекта в изоляции от окружения. Конечно, DHH говорит, что мы можем застабить Time.now, и наслаждаться зелёными точками тестов без лишних телодвижений, но не стоит слепо верить всему, что говорит DHH. Лично мне больше нравится точка зрения Piotr Solnica, изложенная в этом посте. Он приводит такой пример:


class Hacker
  def self.build(layout = 'us')
    new(Keyboard.new(layout: layout))
  end

  def initialize(keyboard)
    @keyboard = keyboard
  end
  # stuff
end


Параметр keyboard в конструкторе — и есть внедрение зависимости. Подобный подход позволяет тестировать класс Hacker, передавая вместо реального инстанса Keyboard моки. Изоляция, все дела:


describe Hacker do
  let(:keyboard) { mock('keyboard') }

  it 'writes awesome ruby code' do
    hacker = Hacker.new(keyboard)
    # some expectations
  end
end

Но что мне в нравится примере выше, так это изящный трюк с методом .build, в котором происходит инициализация keyboard. В обсуждениях DI я видел немало советов, в которых предлагалось инициализацию зависимостей выносить в вызывающий код, например, в контроллеры. Ага, и потом искать по всему проекту вхождения Hacker, чтобы посмотреть, какой конкретно класс используется для клавиатуры, ну-ну. То ли дело .build: дефолтный usecase на видном месте, ничего не нужно искать.


Тестирование вызывающего кода


Рассмотрим следующий пример:


class ExternalService
  def self.build
    options = Config.connector_options
    new(ExternalServiceConnector.new(options))
  end

  def initialize(connector)
    @connector = connector
  end

  def accounts
    @connector.do_some_api_call
  end
end

class SomeController
  def index
    authorize!
    ExternalService.build.accounts
  end
end

Видно, что контроллер создаёт ExternalService, используя реальные объекты (хоть это и скрыто в методе ExternalService.build), чего мы стараемся избежать, внедряя DI. Как справиться с этой ситуацией?


  1. Не тестировать вызывающий код вообще. Так себе вариант, решил записать его для полноты картины.
  2. Подменять ExternalService.build. Фактически то, о чём говорил DHH, но есть один важный момент: заменяя .build, мы не меняем поведение инстансов класса, только обёртку. Пример на RSpec:


    connector = instance_double(ExternalServiceConnector, do_some_api_call: [])
    allow(ExternalService).to receive(:build) { ExternalService.new(connector) }

  3. Тестировать контроллеры с помощью интеграционных тестов на CI-сервере. Плюсы: тестируется production-код, повышая вероятность того, что потенциальный баг отловят тесты, а не пользователи. Минусы: сложнее тестировать исключительные ситуации ("сторонний сервис упал") и не всегда у сторонних сервисов есть аккаунт-песочница, на котором можно без опаски гонять тесты.
  4. Использовать-таки IoC-контейнеры.

Мне кажется, что наиболее эффективным является сочетание второго и третьего подходов: с помощью второго тестируем исключительные ситуации, с помощью третьего убеждаемся в том, что нет ошибок в коде, который инстанцирует объекты.


Выводы


Несмотря на написанное выше, я не против применения IoC-контейнеров в целом; просто полезно помнить, что существуют альтернативы.


Ссылки, используемые в посте:


Поделиться с друзьями
-->

Комментарии (18)


  1. manyrus
    22.08.2016 13:05
    +1

    А что мешает в контроллере сделать вот так:

    class SomeController
      attr_writer :external_service
    
      def index
        authorize!
        external_service.accounts
      end
    
      def external_service
        @external_service ||= ExternalService.build
      end
    end
    


    1. HedgeSky
      22.08.2016 13:33

      Хороший подход, вполне позволяет писать тесты на контроллер. Но есть и минусы: когда в публичном интерфейсе класса есть attr_writer, можно решить, что где-то кто-то собирается поменять значение поля (или, по крайней мере, может это сделать). Особенно эти проблемы проявляют себя тогда, когда код сервиса начинает расширяться с помощью плагинов, хранящихся в отдельных репозиториях (например, при использовании модификаций одного и того же сервиса для разных компаний-заказчиков).
      В общем, писать код с оглядкой на тесты — это одно. Писать код, который будет использоваться только в тестах, уже другое.


    1. iqiaqqivik
      22.08.2016 16:37
      +1

      Мешает, например, то, что рельсы открывают файлы в одном им ведомом порядке, и рано или поздно придется или вручную указать порядок открытия (что плохо), или помнить и тщательно следить, чтобы вызов external_service=() случился раньше, чем вызов external_service(), что вообще ни в какие ворота.


      На #||= нужно полагаться с очень большой осторожностью, тут Пиотр очень прав. Лучше его вообще избегать, в случае ожидаемого постороннего вызова external_service=(). Все эти рельсовые присвоения в before_*** — ровно отсюда выросли.


    1. Kane
      22.08.2016 22:25

      Недостаток такого подхода в том, что он не очень хорошо работает, когда в контроллере более одного экшена. Код становится сложно понимать.


  1. iqiaqqivik
    22.08.2016 16:30
    +2

    Я очень люблю читать полемику Пиотра и Давида: оба настолько яростные евангелисты своего подхода, что за щепками, летящими от деревьев, там уже леса не видно вовсе.


    Эта реализация Hacker ужасна. Хотя бы потому, что build принимает строку, а new — экземпляр Keyboard. В языке, поддерживающем pattern matching из коробки такой проблемы бы не было, но и в руби ее решить довольно просто:


    class Hacker
      def self.build(keyboard_or_layout = 'us')
        new(keyboard_or_layout)
      end
    
      private_class_method :new
    
      def initialize(keyboard_or_layout)
        @keyboard = Keyboard === keyboard_or_layout ? keyboard_or_layout : Keyboard.new(layout: keyboard_or_layout)
      end
    end

    Если уж мы определили фабрику (тут-то зачем?! — ну ладно, типа, для примера) — ну и давайте тогда, пожалуйста, пользоваться фабрикой. Кому очень надо, вызовет Hacker.send(:new), тут же, напоминаю, руби.


    Вообще, ничего плохого в открытых классах и monkey-patching’е нет, тут я целиком на стороне DHH. Кому не нравится — есть масса языков для перехода. Но код на руби должен выглядеть, как код на руби, квакать, как код на руби, и быть руби-кодом. А не php, записанным с использованием руби-синтаксиса. Все эти аргументы, мол, ой, ведь конфликты же — это вообще какой-то признак начинающейся шизофрении. Кто все эти люди, которые боятся, что 5.days.ago будет с чем-нибудь конфликтовать? С чем, стесняюсь спросить? С гемом, который переопределит Integer#days метод, чтобы он возвращал промилли? Так с этим гемом тогда наверняка есть еще 100500 гораздо более серьезных проблем.


    Что же до аргументов Пиотра (точка зрения которого мне на самом деле ближе, я даже контрибьютор dry-transation), то DI — это очень хорошо, но не в приведенном примере. Потому что тест он на то и тест, чтобы mock. А mock в rspec — это monkey-patching. Оп-па.


    1. j_wayne
      23.08.2016 13:19
      +1

      Ну скажем, много лет назад натыкались, что mongoid что-то перекрыл в Object и nil? возвращал true на объекте (не NilClass).

      Сейчас есть проблемка с гемом packable (да, я понимаю, что он не очень популярен, но все же факт есть факт).
      Отдельно отлично работает.
      В рельсовом проекте с большим количеством гемов, при некоторых условиях, есть исключения (причем сам packable, там казалось бы, ни к чему, но поскольку манкипатч… ну вы поняли). Причем выкурить, с чем именно конфликтует не удалось, т.к. усилий нужно неоправданно много, запихнул в group :manual_load в Gemfile и подключаю вручную там, где нужен. Пока работает.

      Я отдаю себе отчет в этом:

      > А mock в rspec — это monkey-patching. Оп-па.

      И лично считаю rspec лучшим тулом для TDD. Хотя я скорее явист, чем рубист.
      Минусы истекают из плюсов и наоборот.

      ИМХО манкипатч 5.days.ago (в платформе!) это одно дело, и куда бы ни шло, а вот когда мелкие гемы лезут со своей ерундой в стандартные классы — совсем другое. Но тут уж ничего не поделаешь.


      1. iqiaqqivik
        23.08.2016 13:38

        mongoid что-то перекрыл в Object и nil? возвращал true на объекте (не NilClass)
        с большим количеством гемов, при некоторых условиях, есть исключения

        Есть, конечно, исключения. Я сам умею в продакшене GI в корку уложить (точнее, сумел разок). Но это же все из серии «давайте запретим ножи, они острые».


        когда мелкие гемы лезут со своей ерундой в стандартные классы

        Смотря как лезут. Вот пример прямо из печки: https://github.com/am-kantox/iteraptor — добавляет пару методов для рекурсивной/глубокой итерации, маппинга и редьюса прямо в Hash и Array. В исходниках можно увидеть атавизм: реверанс в сторону «ни-ни-ни, не трожь базовые классы». А именно, сначала гем требовал эксплицитного require 'greedy', чтобы открыть стандартные классы. Уже в версии 2.0 — это дефолтное поведение. Потому что так удобнее, и потому что названия функций являются точными аналогами стандартных, только именованы на испанском языке.


        1. j_wayne
          23.08.2016 13:43

          >> Но это же все из серии «давайте запретим ножи, они острые».

          Разумеется, просто неправильно выбранный инструмент.


    1. getElementById
      25.08.2016 01:29

      Проблема манки-патчинга не в том, что после его использования у вас сразу выпадут волосы и зубы, а в том, что манки-патчинг по своей сути это слабо управляемый инструмент. Его не стоит сравнивать с ножом, скорее с топором, которым вы пытаетесь разрезать торт. Цели своей вы добъетесь, но выглядеть это будет диковато, даже если топор наточен очень остро. Но допустим, что руби это такой странный мир людей, предпочитающих такие методы решения проблем.
      Манки-патчинг это большая головная боль для разработчиков гемов, потому что получается, что кто угодно может непредсказуемо поменять рантайм языка, в итоге выйдет, что какие-то гемы окажутся несовместимыми. Это вовсе не мифы, чем дольше руби развивается, тем больше таких проблем происходит. Вот пример с переполнением стека из-за переопределения базовых классов: https://github.com/txus/kleisli/pull/17. Хочу заметить, что ошибки с переполнением стека очень неудобно отлаживать, буквально на днях такая ошибка уронила у меня интерпретатор с ошибкой адресации, никакого руби-стека я не увидел вообще.
      Дальше — больше. Есть рельсы, которые построены на activesupport. Если какое-то из поведений в AS ломает ваш гем, то вам ничего не остается, кроме как поменять вашу библиотеку для совместимости с AS, потому что иначе никто не будет ей пользоваться. Это уже не ножи, это пики из задачи двух стульев.
      Стоит присмотреться повнимательнее какую же проблему на самом деле решает манки-патчинг. На деле это оказывается банальное отсутствие нужных абстракций или инструментов, вместо создания которых, люди патчат рантайм. Точка зрения dry-rb-коммьюнити заключается в том, что так быть не должно. Нужно разрабатывать библиотеки и подходы, которые будут удобны и функциональны без необходимости изменять поведение стандартных классов, для этого в руби есть все необходимое. Мы неоднократно видели, как продуманный подход преподносил много приятных сюрпризов впоследствии, что еще больше усилило нашу уверенность в правильности выбранного пути. Простите, если это звучит слишком пафосно :)


      1. iqiaqqivik
        25.08.2016 07:56

        Я не знаю, что вы понимаете под “dry-rb community”, на всякий случай повторю: мой лично код есть в dry-transaction. Так что не надо мне про продуманные подходы, я в курсе. Но это не значит, что я буду слепо повторять за Пиотром все, что он в запале скажет. Кроме того, если вы приглядитесь к коду dry-rb, вы внезапно увидите всю мощь наплевательского отношения на общепринятые стандарты (например, прямое наследование от BasicObject, которое ломает дебаггер и прочие смешные вещи, которые явно выросли из мощи языковых средств и неумения/нежелания сделать по уму).

        Инструмент для «безопасного» манкипатчинга в руби есть аж с 2.0, refinements называется, и никто им не пользуется. Потому что не удобно, и ничем не отличается от создания инстансов через `Class.new(BaseClass) do |c| c.prepend Patch… end`.

        > На деле это оказывается банальное отсутствие нужных абстракций или инструментов,
        > вместо создания которых, люди патчат рантайм.

        Ерунда. Открытые классы — мощнейший инструмент, который позволяет сделать очень много, если делать это по уму. Если же делать что-нибудь не по уму, то даже в насквозь казалось бы безопасном Delphi в 1994 году мне удалось наступить на грабли с левой библиотекой, которая обращалась к приватным полям чужих объектов по смещению. В любом языке можно отравить жизнь всем своим пользователям, и проблема не в манкипатчинге.

        Не сто?ит повторять за мэтрами все, что они скажут. Истина посередине.


        1. getElementById
          25.08.2016 10:37

          Странно, что вы отказываете мне в наличии собственного мнения. В любом случае, я имел в первую очередь позицию core team.

          BasicObject создан для использования в качестве контекста для DSL, так он и используется, разве нет? Ну, конечно, если что-то не так с этим подходом, и вы знаете об этом, нет смысла держать в себе, откройте тикет, где это можно будет обсудить в «рабочем порядке» :) Равно как и про «остальные смешные вещи».

          refinements не взлетели потому что они на самом деле не решают основную проблему — отсутствие абстракций. Ну, я не исключаю, что если бы они работали как-то по-другому, то это позволило бы хотя бы снять вопрос об изменении рантайма.

          Ерунда. Открытые классы — мощнейший инструмент, который позволяет сделать очень много, если делать это по уму. Если же делать что-нибудь не по уму, то даже в насквозь казалось бы безопасном Delphi в 1994 году мне удалось наступить на грабли с левой библиотекой, которая обращалась к приватным полям чужих объектов по смещению. В любом языке можно отравить жизнь всем своим пользователям, и проблема не в манкипатчинге.

          Это пример ложной дилеммы.


          1. iqiaqqivik
            25.08.2016 10:59

            > Странно, что вы отказываете мне в наличии собственного мнения.

            Это где это? Если бы я отказывал вам в наличии собственного мнения, мне было бы неинтересно что-нибудь писать в ответ, по-моему это достаточно очевидно.

            > BasicObject создан для использования в качестве контекста для DSL, так он и используется, разве нет?

            Кхм. Ну, допустим. Хотя избавляться от неймспесинга и внедренных зависимостей можно и поизящнее, не разламывая в щепы pry, которая не ожидает от объектов такой подлости, как отсутствие `puts`. Делегировать пару методов из `Kernel` уж точно можно было бы. Впрочем, я в целом не понимаю, в чем прелесть `BasicObject` именно там, кроме демонстрации «идеологии».
            Я не открываю тикеты потому, что не хочу лаять на Пиотра так, как он лает на DHH. Мне [местами] нравится подход dry, [местами] нравится подход rails и я хочу оставить их авторам полную свободу самовыражения. Они прекрасно смотрятся в паре.
            Я не прошу/заставляю женщин сделать пластическую операцию ради меня, я выбираю тех, которые нравятся от входа.

            > Это пример ложной дилеммы.

            Нет, это вам кажется. Никакой дилеммы тут нет вообще. Мы обсуждаем манкипатчинг. Вы приводите примеры, в которых манкипатчинг нагадил. Я утверждаю, что нагадить в нашей специальности — не проблема вообще.

            На совсем тривиальном примере: Петя отравился гнилым яблоком. Петя утверждает, что яблоки есть вредно. Я утверждаю, что отравиться можно чем угодно, но это не является причиной для отказа от собственно яблок (точнее, ниоткуда не следует, что отказ от яблок приведет к понижению вероятности последующих отравлений).


            1. getElementById
              25.08.2016 12:25

              Кхм. Ну, допустим. Хотя избавляться от неймспесинга и внедренных зависимостей можно и поизящнее, не разламывая в щепы pry, которая не ожидает от объектов такой подлости, как отсутствие `puts`. Делегировать пару методов из `Kernel` уж точно можно было бы. Впрочем, я в целом не понимаю, в чем прелесть `BasicObject` именно там, кроме демонстрации «идеологии».

              Я все же настаиваю на обсуждении этого на гитхабе, там более подходящее место. Но насчет пробрасывания методов из Kernel идея хорошая.

              Что касается остального обсуждения, то я написал тут не с целью переубедить кого-то, а чтобы обозначить позицию. Манкипатчинг по нашим представлениям крайне редко решает проблему адекватно задаче, при этом создает значительные неудобства, потому что люди не могут предвидеть последствия изменения стандартных классов, это слишком сложная задача для человека. Поэтому мы пользуемся другими инструментами для решения задач.


              1. iqiaqqivik
                25.08.2016 12:56

                Дык а я обозначаю свою позицию: вы ошибаетесь :)

                Это не значит, что кто-то прав, кстати. Я прекрасно понимаю, в чем сухие монады чище кляйсли. Я ни в коем случае не призываю открывать `NilClass` каждый раз, когда нам лень проверить операнд (я вообще считаю `try` наибольшим злом с точки зрения воздействия на неокрепшие умы, принесенным рельсами).

                В то же самое время, как мне кажется, `5.days` не может ничего сломать, нигде. Нас приглашают поучаствовать в разработке ruby core, через открытые классы — зачем отказываться? Матц напрямую неоднократно говорил, что да, вот такая цель. Да, можно создать гем, который всем все сломает — так есть 100500 гораздо более простых способов все сломать. Не нужно тянуть в рот всякую гадость, не нужно устанавливать `leftpad`, но все хорошо в меру.

                Я убежден, что ваша команда пишет внятный работоспособный код не потому, что вы решили отказаться от манкипатчинга.


                1. getElementById
                  27.08.2016 20:20

                  5.days может сломать, я разбирался с такой проблемой, когда Duration притворялся числом, его нельзя было распознать никак, кроме как вызвать метод parts, который внезапно оказывался/не оказывался у объекта, так можно было отличить целое число от Duration. Это было в рельсах 3.2, с тех пор этот момент улучшили, но я лично потратил час или два на разбирательства с той проблемой, какой-нибудь новичок, по-моему просто погиб бы. Но это я в качестве примера, на самом деле я согласен, что патчинг бывает разный, но беда в том, что вместе в рельсами вы получаете все и сразу, нет способа отказаться. А когда вы на это жалуетесь, вам рекомендуют использовать другой язык программирования под предлогом того, что вы не умеете в руби, такие дела.


                  1. iqiaqqivik
                    29.08.2016 09:08

                    > беда в том, что вместе в рельсами вы получаете все и сразу, нет способа отказаться

                    > когда вы на это жалуетесь, вам рекомендуют использовать другой язык программирования
                    > под предлогом того, что вы не умеете в руби

                    Вот не нужно передергивать, пожалуйста. Когда вы жалуетесь (вполне обоснованно) на рельсы — вам рекомендуют не использовать рельсы.

                    А вот когда вы называете манкипатчинг глобальной проблемой руби — тут вам рекомендуют сменить язык, потому что в руби открытые классы, манкипатчинг, неприватные приватные методы, метапрограммирование — ядро, ради которого все задумано (насколько я понимаю). Идея такая: чтобы хороший разработчик не отстрелил себе ногу, нужно не пистолеты запрещать, а курсы стрельбы пропагандировать.

                    Не нужно проблемы, созданные увлекшимся игрой в «давайте запатчим все на свете» Давидом, переносить на руби. Не нужно объявлять манкипатчинг — злейшим злом _языка_. Я ведь только об этом говорю.


                  1. iqiaqqivik
                    29.08.2016 09:26

                    И это, если вас не затруднит, а можно поподробнее (линк на неработающий код пойдет), как Duration, притворившийся числом, все поломал? Зачем мне знать, инстанс чего это у меня, если он квакает подходящим образом?


                    1. getElementById
                      29.08.2016 20:20

                      Да я не про вас, это аргумент dhh :)

                      Про duration я ничего уже не помню, но проблема при дебаге была в том, что квакание там было неподходящим, is_a? и .class тоже не работали, выдавая Fixnum, на самом деле им не являясь. Конечно, если там поразбираться, наверняка была проблема в некорректном обращении с типами в руби, но вот разбираться с такими вещами тяжеловато.