Привет, Хабр! Давно манят меня лавры быть автором, и вот, наконец, настал тот светлый час, когда я допинал себя представить на твой суд мой небольшой опус.
Изучая на досуге Ruby и Rails, пробуя то RSpec, то вдруг Minitest, дошёл я до создания web-приложения с фронтэндом на JavaScript и бэкендом на Ruby, торчащим наружу REST API на базе обычных контроллеров Rails. Я использую Rails, хотя это совершенно не принципиально. Описанный ниже подход применить можно к чему угодно.
Тут следует сделать небольшое отступление. По натуре я человек требовательный, и всячески борюсь за доказанную стабильность кода (на словах-то уж точно). А уж когда речь заходит о безопасности пользователей в моём приложении, без тестов, хотя бы показывающих, что абы кто мои данные не получит, я чувствую себя совсем не комфортно. Начинаю грустить и вообще никакой код не писать. Даже если я — один-единственный пока пользователь.
Казалось бы, всё очень просто: берём RSpec и пишем тесты. Но это же скучно! Для каждого контроллера, для каждого поддерживаемого метода проверить, как минимум, что без выданного ранее токена пользователь получит от ворот поворот — это ж сколько одинакового кода надо написать! А дальше как? Контроллеров всё больше, тесты копировать скучно, да и в возможностях подходы менять я остаюсь ограничен. Пойди-ка все эти тесты потом перепиши, если я захочу, например, версию API передавать не в URL, а в заголовке, или наоборот. В общем, задумал я написать генератор.
Для каждого из уже имеющихся контроллеров было у меня два тест-кейса: проверка на то, что при попытке доступа без access token приложение возвращает код 401, а с несуществующим access token — код 403. При соблюдении этих правил остаётся убедиться только в том, что при правильном access token приложение отдаёт данные владельца этого токена и никакие другие, но это уже за рамками данной статьи. То есть, было что-то такое:
Ну и желание больше двух раз такое не писать.
Ruby — язык с широкими возможностями метапрограммирования. Благодаря им, в частности, существует и RSpec DSL, использование которого демонстрируется в примере выше. А что такое RSpec DSL? Сахарок для написания классов, которые библиотека потом просто запускает. А значит, можно их сгенерировать самому! К счастью, при наличии всего лишь одного базового класса для всех контроллеров решение этой задачи — уже дело техники. Немного помучив Google, StackOverflow и собственную голову (не всё же ей задачи ставить — решать их тоже надо), пришёл я вот к такому фрагменту кода:
Спешу, однако, обрадовать, что этот код не работает.
Очень просто. Всё дело в ленивой загрузке. Rails не сорит в памяти тем, что, быть может, и не понадобится. Поэтому массив descendants у Api::V2::ControllerBase пуст. К счастью, это легко лечится:
Вставленная после самого первого describe эта магическая строчка переворачивает ситуацию с ног на голову:
Да простят меня любители vim и консоли за использование RubyMine.
Изначальной идеей было спокойно творить и не беспокоиться о самых основах безопасности. Я добавляю контроллер, а количество зелёных тестов магическим образом увеличивается. Но, как показывает приведённая выше картинка, не всегда они сразу зелёные.
Есть методы, под общее правило не подпадающие. C'est la vie. В моём случае, например, это POST /api/user-sessions/, потому что глупо требовать правильный access token от метода, который за ним обращается. Не долго думая, я добавил вот это:
и это:
в код своего мета-теста. Всё сразу позеленело.
Правда, совсем уж забыть о нём теперь не получится.
Ruby великолепен своими возможностями метапрограммирования, RSpec великолепен своей понятливостью (я сомневался, что мне так просто дадут сгенерировать и тут же запустить сгенерированные тест-кейсы), ну а Рельсы просто великолепны, по определению. При должной сноровке автоматическая генерация тест-кейсов может использоваться для решения разных задач, не лишая, при этом, читабельности код тестов. Уверен, что это решение кому-нибудь пригодится.
Спасибо за внимание.
P.S. Финальный код решения я спрятал под спойлер.
P.P.S. Спасибо Elisabeth Hendrickson за идею и пример.
Изучая на досуге Ruby и Rails, пробуя то RSpec, то вдруг Minitest, дошёл я до создания web-приложения с фронтэндом на JavaScript и бэкендом на Ruby, торчащим наружу REST API на базе обычных контроллеров Rails. Я использую Rails, хотя это совершенно не принципиально. Описанный ниже подход применить можно к чему угодно.
Тут следует сделать небольшое отступление. По натуре я человек требовательный, и всячески борюсь за доказанную стабильность кода (на словах-то уж точно). А уж когда речь заходит о безопасности пользователей в моём приложении, без тестов, хотя бы показывающих, что абы кто мои данные не получит, я чувствую себя совсем не комфортно. Начинаю грустить и вообще никакой код не писать. Даже если я — один-единственный пока пользователь.
Казалось бы, всё очень просто: берём RSpec и пишем тесты. Но это же скучно! Для каждого контроллера, для каждого поддерживаемого метода проверить, как минимум, что без выданного ранее токена пользователь получит от ворот поворот — это ж сколько одинакового кода надо написать! А дальше как? Контроллеров всё больше, тесты копировать скучно, да и в возможностях подходы менять я остаюсь ограничен. Пойди-ка все эти тесты потом перепиши, если я захочу, например, версию API передавать не в URL, а в заголовке, или наоборот. В общем, задумал я написать генератор.
Постановка задачи
Для каждого из уже имеющихся контроллеров было у меня два тест-кейса: проверка на то, что при попытке доступа без access token приложение возвращает код 401, а с несуществующим access token — код 403. При соблюдении этих правил остаётся убедиться только в том, что при правильном access token приложение отдаёт данные владельца этого токена и никакие другие, но это уже за рамками данной статьи. То есть, было что-то такое:
describe Api::V2::UserSessionsController do
let (:access_token) {SecureRandom.hex(64)}
describe 'DELETE /user-sessions/:id' do
context 'without an access token' do
before { delete :destroy, id: access_token }
it 'responds with 401' do
expect(response).to have_http_status :unauthorized
end
end
context 'with non-existent access token' do
before {@request.headers['X-API-Token'] = SecureRandom.hex(64)}
before {delete :destroy, id: access_token}
it 'responds with 403' do
expect(response).to have_http_status :forbidden
end
end
end
end
Ну и желание больше двух раз такое не писать.
Что делать?
Ruby — язык с широкими возможностями метапрограммирования. Благодаря им, в частности, существует и RSpec DSL, использование которого демонстрируется в примере выше. А что такое RSpec DSL? Сахарок для написания классов, которые библиотека потом просто запускает. А значит, можно их сгенерировать самому! К счастью, при наличии всего лишь одного базового класса для всех контроллеров решение этой задачи — уже дело техники. Немного помучив Google, StackOverflow и собственную голову (не всё же ей задачи ставить — решать их тоже надо), пришёл я вот к такому фрагменту кода:
describe Api::V2::ControllerBase do
Api::V2::ControllerBase.descendants.each do |c|
describe "#{c.name}" do
Rails.application.routes.set.each do |route|
if route.defaults[:controller] == c.controller_path
action = route.defaults[:action]
request_method = /[A-Z]+/.match(route.constraints[:request_method].to_s)[0]
param_placeholders = Hash[route.parts.reject { |p| p == :format }.map { |p| [p, ":#{p}"] }]
spec_name = "#{request_method} #{route.format(param_placeholders)}"
describe "#{spec_name}" do
before { self.controller = c.new }
context 'without an access token' do
before { process(action, request_method, param_placeholders) }
it 'responds with 401' do
expect(response).to have_http_status :unauthorized
end
end
context 'with non-existent access token' do
before { @request.headers['X-API-Token'] = SecureRandom.hex(64) }
before { process(action, request_method, param_placeholders) }
it 'responds with 403' do
expect(response).to have_http_status :forbidden
end
end
end
end
end
end
end
end
Спешу, однако, обрадовать, что этот код не работает.
Как так?
Очень просто. Всё дело в ленивой загрузке. Rails не сорит в памяти тем, что, быть может, и не понадобится. Поэтому массив descendants у Api::V2::ControllerBase пуст. К счастью, это легко лечится:
Rails.application.eager_load!
Вставленная после самого первого describe эта магическая строчка переворачивает ситуацию с ног на голову:
Да простят меня любители vim и консоли за использование RubyMine.
Написал и забыл
Изначальной идеей было спокойно творить и не беспокоиться о самых основах безопасности. Я добавляю контроллер, а количество зелёных тестов магическим образом увеличивается. Но, как показывает приведённая выше картинка, не всегда они сразу зелёные.
Есть методы, под общее правило не подпадающие. C'est la vie. В моём случае, например, это POST /api/user-sessions/, потому что глупо требовать правильный access token от метода, который за ним обращается. Не долго думая, я добавил вот это:
def self.excluded_actions
{
Api::V2::UserSessionsController => [:create],
Api::V2::UserCalendarsController => [:oauth2callback_no_ajax]
}
end
и это:
next if excluded_actions.key?(c) && excluded_actions[c].include?(action.to_sym)
в код своего мета-теста. Всё сразу позеленело.
Правда, совсем уж забыть о нём теперь не получится.
Заключение
Ruby великолепен своими возможностями метапрограммирования, RSpec великолепен своей понятливостью (я сомневался, что мне так просто дадут сгенерировать и тут же запустить сгенерированные тест-кейсы), ну а Рельсы просто великолепны, по определению. При должной сноровке автоматическая генерация тест-кейсов может использоваться для решения разных задач, не лишая, при этом, читабельности код тестов. Уверен, что это решение кому-нибудь пригодится.
Спасибо за внимание.
P.S. Финальный код решения я спрятал под спойлер.
Финальный код мета-теста
describe Api::V2::ControllerBase do
Rails.application.eager_load!
def self.excluded_actions
{
Api::V2::UserSessionsController => [:create],
Api::V2::UserCalendarsController => [:oauth2callback_no_ajax]
}
end
Api::V2::ControllerBase.descendants.each do |c|
describe "#{c.name}" do
Rails.application.routes.set.each do |route|
if route.defaults[:controller] == c.controller_path
action = route.defaults[:action]
next if excluded_actions.key?(c) && excluded_actions[c].include?(action.to_sym)
request_method = /[A-Z]+/.match(route.constraints[:request_method].to_s)[0]
param_placeholders = Hash[route.parts.reject { |p| p == :format }.map { |p| [p, ":#{p}"] }]
spec_name = "#{request_method} #{route.format(param_placeholders)}"
describe "#{spec_name}" do
before { self.controller = c.new }
context 'without an access token' do
before do
process(action, request_method, param_placeholders)
end
it 'responds with 401' do
expect(response).to have_http_status :unauthorized
end
end
context 'with non-existent access token' do
before {@request.headers['X-API-Token'] = SecureRandom.hex(64)}
before { process(action, request_method, param_placeholders) }
it 'responds with 403' do
expect(response).to have_http_status :forbidden
end
end
end
end
end
end
end
end
P.P.S. Спасибо Elisabeth Hendrickson за идею и пример.
Комментарии (12)
Xergin
23.11.2015 07:55+2не слышали про shared examples?
szubtsovskiy
23.11.2015 09:52-1Слышали, но на тот момент я уже был охвачен идеей написать мета-тест. Буду благодарен, если приведёте пример, как эту же проблему решить с помощью shared examples.
Metus
23.11.2015 10:37+2Из своего опыта скажу следующее:
1. Тесты контроллеров, как правило, не окупают себя — они пишутся дольше всего и наиболее чувствительны к изменением тестируемого кода. Лично по мне — в рельсах лучше максимально отказаться от тестов контроллеров (оставить только там где это необходимо), а писать интеграционные тесты — т.е. именно слать запросы get, put, post, delete и т.д. на нужные адреса, либо кликать по ссылкам.
Вы же не тестируете каждую вьюху? Хотя такая возможность тоже есть.
2. Лучше всё же shared examples. Решается просто — весь общий код пишется, собственно, в него, а нужные переменные передаются через let:
it_behaves_like "protected controller" do let(:method) { 'hello' } end
По мне, основное и главное свойство тестов (помимо того что они тестируют нужный функционал) — это их читаемость.
Непонятные тесты, которые дают 100% покрытия — не тот результат, к которому бы я стремился. Понятные читаемые тесты, тестирующие большинство ситуаций + самые критичные места — это моя цель.szubtsovskiy
23.11.2015 11:36Спасибо, за то, что поделились своим опытом. Именно на это я и рассчитывал, публикуя сей опус. :-)
Тесты контроллеров, как правило, не окупают себя — они пишутся дольше всего и наиболее чувствительны к изменением тестируемого кода. Лично по мне — в рельсах лучше максимально отказаться от тестов контроллеров (оставить только там где это необходимо), а писать интеграционные тесты — т.е. именно слать запросы get, put, post, delete и т.д. на нужные адреса, либо кликать по ссылкам.
Полностью поддерживаю. Именно поэтому и появился этот мета-тест, ибо на мой взгляд проверка того, что приложение отфутболивает при отсутствии credentials, является crucial part, а остальное, может быть, и не надо совсем. Покроется интеграционными тестами. Другой вопрос, что и этот тест, вероятно, следовало бы сделать интеграционным, а не ориентированным на контроллеры, но суть от этого не меняется.
2. Лучше всё же shared examples. Решается просто — весь общий код пишется, собственно, в него, а нужные переменные передаются через let
Если я правильно понимаю, придётся тогда городить «расширение» этих shared examples для каждого контроллера, а я вообще не хочу писать спеки для контроллеров. Написал один раз и забыл (ну или почти забыл).
Непонятные тесты, которые дают 100% покрытия
То есть, этот тест непонятный? У меня-то как раз сложилось обратное представление: вся суть в самом последнем describe и двух context, и эти блоки почти ничем не отличаются от тех, что я писал отдельно для каждого контроллера. Разве что
before { self.controller = c.new }
может вызывать вопросы при первом взгляде, но с учётом других преимуществ (возможности не писать спек-файл для каждого нового контроллера), на мой взгляд, представляет собой неплохой компромисс.
Вопрос читаемости кода очень субъективный. И автору этого кода нечитаемость становится очевидна обычно не сразу. Мне довелось не так давно встретитить вот такую конструкцию в тестах на TestNG, например:
return new Object[][]{ {new LoginHelperTestDataProviderLoginAsUserValue("96968c29f29b96103b1c38f30d8f0fe2", 1l, 2l, Role.ROLE_SUPER)}, {new LoginHelperTestDataProviderLoginAsUserValue("d4bbe02790bdc33c5778fd8ec4e4d036", 2l, 5l, Role.ROLE_ADMIN)}, {new LoginHelperTestDataProviderLoginAsUserValue("712d3a285047d2ed252990d53cca3893", 2l, 56l, Role.ROLE_ADMIN)}, {new LoginHelperTestDataProviderLoginAsUserValue("56e659746ba7b3fdc24983352777f6d9", 3l, 232l, Role.ROLE_MANAGER)}, {new LoginHelperTestDataProviderLoginAsUserValue("56e659746ba7b3fdc24983352777f6d9", 4l, 64324l, Role.ROLE_MANAGER)}, {new LoginHelperTestDataProviderLoginAsUserValue("56e659746ba7b3fdc24983352777f6d9", 5l, 86456l, Role.ROLE_MANAGER)}, };
Никакой другой реакции кроме WAT?! на это, по-моему, быть не может, хотя, уверен, автору казалось всё кристально чистым, когда он это создавал. А мне даже статью написать захотелось на тему, как загубить идею, ведь это уже не первое подобное использование концепции DataProvider, которое я встречаю.
Extrapolator
23.11.2015 10:43Лично я бы предпочел метод self.excluded_actions заменить на константу EXCLUDED_ACTIONS. Это фиксированное значение, тут хэш не тянет никаких данных и не вызывает другие методы.
mpetrunin
23.11.2015 15:49+2По-моему, то, что получилось — очень плохо. По нескольким причинам:
1. Плохо читаемый и громоздкий код.
2. Неиспользование стандартных инструментов фреймворка.
3. Слишком много ненужного мета-программирования (что на самом деле подмножество п.1).
4. Избыточное число тестов на выходе.
Я вижу два пути:
1. Или как советовали выше много раз использовать shared examples.
2. Или вынести функционал в ApplicationController (или в любой другой класс, например, SecureController и наследовать уже от него), протестировать этот функционал в одном месте с помощью Anonymous Controller. А затем в каждом конкретном случае проверять в тесте, что этот конкретный контроллер наследуется от ApplicationController (или, соответственно, SecureController).
hats
23.11.2015 23:26+1Не очень понимаю зачем все это нужно, т.к. вы же всегда свои контроллеры для API наследуете от API::BaseController. Вот его-то и тестируйте один раз на все случаи аутентификации и авторизации.
Например так
require 'rails_helper' describe Api::BaseController, type: :controller do subject { get :index } let(:user) { Fabricate(:user) } let(:admin) { Fabricate(:admin) } controller do def index render nothing: true, status: :ok end end describe 'when user isn\'t admin' do before { sign_in user subject } it 'return 403' do expect_status '403' end end describe 'when user is admin' do before { sign_in admin subject } it 'return 200' do expect_status '200' end end end
XimikS
Жуть какая :) Зачем это для каждого контроллера проверять то? Не думаю, что вы меняете эту функциональность в каждом контроллере.
Кстати, судя по направлению, избранному командой RoR, спеки контроллеров ждёт печальная участь.
szubtsovskiy
Конечно, я не меняю эту функциональность в каждом контроллере, да и проверка в них сводится к тому, чтобы упомянуть методы в before_action. Но это надо не забыть сделать. Такой тест — хороший способ быть уверенным в том, что не забыл.
А можете просвятить, что это за направление, избранное командой RoR, из-за которого спеки контроллеров ждёт печальная участь?
XimikS
Ну можно же сделать всего одну проверку, а не на каждый контроллёр, если у них поведение одинаковое. Перестраховка?
Или вынести before_action в ApplicationController не выходит?
github.com/rails/rails/issues/18950#issuecomment-77924771
szubtsovskiy
Конечно, можно сделать одну проверку в коде или запихнуть before_action в ApplicationController (и добавить skip_before_action, где проверять не надо). Но это же implementation detail! Я хотел своего рода отчёт, который показывает, что (почти) каждый URL, которым приложение смотрит наружу, требует передачи access token. Не должны тесты знать, как устроена проверка внутри — они должны просто показывать, что всё работает, как ожидается. А как это сделать, не дёргая каждый URL?
В приведённом Вами тикете, кстати, DHH придерживается сходной точки зрения. Так что я грядущие изменения поддерживаю обеими руками.