1
0
Fork 0
mirror of https://github.com/thoughtbot/shoulda-matchers.git synced 2022-11-09 12:01:38 -05:00

Permit matcher now supports subparameters

Previously we were taking ActionController::Parameters and completely
overriding #require, forcing it to return `self`, i.e, the entire
ActionController::Parameters object. This meant that we broke its
functionality, which is to return a slice of the params hash instead.
The consequence of this is that attempting to call #permit on a slice of
the params hash obtained via #require would not work:

``` ruby
params = ActionController::Parameters.new(
  { "course" => { "foo" => "bar" } }
)
params.require(:course)
params.require(:course).permit(:foo)
```

This commit fixes the permit matcher so that #require is proxied
instead, retaining the existing behavior.

This commit also adds a qualifier, #on, for asserting that your action
places a restriction on a slice of the params hash. The `permit` matcher
will properly track calls on child `params` instances. For example:

``` ruby
class UsersController < ActionController::Base
  def create
    User.create!(user_params)
    ...
  end

  private

  def user_params
    params.require(:user).permit(:name, :age)
  end
end

describe UsersController do
  it { should permit(:name, :age).for(:create).on(:user) }
end
```

If this fails, you'll get the following error message:

```
Expected POST #create to restrict parameters for :user to :name and :age,
but restricted parameters were :first_name and :last_name.
```
This commit is contained in:
Elliot Winkler 2015-02-19 11:35:45 -07:00
parent 3e2c4e11ae
commit b33f5de55a
4 changed files with 331 additions and 97 deletions

12
NEWS.md
View file

@ -52,6 +52,16 @@
vein, passing a pluralized version of the attribute name to `define_enum_for`
would erroneously pass, and now it fails. ([#641])
* Fix `permit` so that it does not break the functionality of
ActionController::Parameters#require. ([#648], [#675])
### Features
* Add `on` qualifier to `permit`. This allows you to make an assertion that
a restriction was placed on a slice of the `params` hash and not the entire
`params` hash. Although we don't require you to use this qualifier, we do
recommend it, as it's a more precise check. ([#675])
[#402]: https://github.com/thoughtbot/shoulda-matchers/pull/402
[#587]: https://github.com/thoughtbot/shoulda-matchers/pull/587
[#662]: https://github.com/thoughtbot/shoulda-matchers/pull/662
@ -59,6 +69,8 @@
[#641]: https://github.com/thoughtbot/shoulda-matchers/pull/641
[#521]: https://github.com/thoughtbot/shoulda-matchers/pull/521
[#607]: https://github.com/thoughtbot/shoulda-matchers/pull/607
[#648]: https://github.com/thoughtbot/shoulda-matchers/pull/648
[#675]: https://github.com/thoughtbot/shoulda-matchers/pull/675
# 2.8.0

View file

@ -38,14 +38,16 @@ module Shoulda
# describe UsersController do
# it do
# should permit(:first_name, :last_name, :email, :password).
# for(:create)
# for(:create).
# on(:user)
# end
# end
#
# # Test::Unit
# class UsersControllerTest < ActionController::TestCase
# should permit(:first_name, :last_name, :email, :password).
# for(:create)
# for(:create).
# on(:user)
# end
#
# If your action requires query parameters in order to work, then you'll
@ -82,7 +84,8 @@ module Shoulda
#
# it do
# should permit(:first_name, :last_name, :email, :password).
# for(:update, params: { id: 1 })
# for(:update, params: { id: 1 }).
# on(:user)
# end
# end
#
@ -93,7 +96,8 @@ module Shoulda
# end
#
# should permit(:first_name, :last_name, :email, :password).
# for(:update, params: { id: 1 })
# for(:update, params: { id: 1 }).
# on(:user)
# end
#
# Finally, if you have an action that isn't one of the seven resourceful
@ -132,10 +136,9 @@ module Shoulda
# end
#
# it do
# should permit(:activated).for(:toggle,
# params: { id: 1 },
# verb: :put
# )
# should permit(:activated).
# for(:toggle, params: { id: 1 }, verb: :put).
# on(:user)
# end
# end
#
@ -145,10 +148,9 @@ module Shoulda
# create(:user, id: 1)
# end
#
# should permit(:activated).for(:toggle,
# params: { id: 1 },
# verb: :put
# )
# should permit(:activated).
# for(:toggle, params: { id: 1 }, verb: :put).
# on(:user)
# end
#
# @return [PermitMatcher]
@ -162,11 +164,12 @@ module Shoulda
attr_writer :stubbed_params
def initialize(expected_permitted_params)
@expected_permitted_params = expected_permitted_params
@action = nil
@verb = nil
@request_params = {}
@expected_permitted_params = expected_permitted_params
set_double_collection
@subparameter = nil
@parameters_doubles = ParametersDoubles.new
end
def for(action, options = {})
@ -176,19 +179,32 @@ module Shoulda
self
end
def add_params(params)
request_params.merge!(params)
self
end
def on(subparameter)
@subparameter = subparameter
@parameters_doubles = SliceOfParametersDoubles.new(subparameter)
self
end
def in_context(context)
@context = context
self
end
def description
"permit #{verb.upcase} ##{action} to receive parameters #{param_names_as_sentence}"
"(on #{verb.upcase} ##{action}) " + expectation
end
def matches?(controller)
@controller = controller
ensure_action_and_verb_present!
parameters_doubles.register
Doublespeak.with_doubles_activated do
context.__send__(verb, action, request_params)
end
@ -197,32 +213,49 @@ module Shoulda
end
def failure_message
"Expected controller to permit #{unpermitted_params.to_sentence}, but it did not."
"Expected #{verb.upcase} ##{action} to #{expectation},\nbut #{reality}."
end
alias failure_message_for_should failure_message
def failure_message_when_negated
"Expected controller not to permit #{verified_permitted_params.to_sentence}, but it did."
"Expected #{verb.upcase} ##{action} not to #{expectation},\nbut it did."
end
alias failure_message_for_should_not failure_message_when_negated
protected
attr_reader :controller, :double_collection, :action, :verb,
:request_params, :expected_permitted_params, :context
attr_reader :controller, :double_collections_by_param, :action, :verb,
:request_params, :expected_permitted_params, :context, :subparameter,
:parameters_doubles
def set_double_collection
@double_collection =
Doublespeak.double_collection_for(::ActionController::Parameters)
def expectation
message = 'restrict parameters '
@double_collection.register_stub(:require).to_return(&:object)
@double_collection.register_proxy(:permit)
if subparameter
message << " for #{subparameter.inspect}"
end
message << 'to ' + format_param_names(expected_permitted_params)
message
end
def reality
if actual_permitted_params.empty?
'it did not restrict any parameters'
else
'the restricted parameters were ' +
format_param_names(actual_permitted_params) +
' instead'
end
end
def format_param_names(param_names)
param_names.map(&:inspect).to_sentence
end
def actual_permitted_params
double_collection.calls_to(:permit).inject([]) do |all_param_names, call|
all_param_names + call.args
end.flatten
parameters_doubles.permitted_params
end
def permit_called?
@ -258,6 +291,90 @@ module Shoulda
expected_permitted_params.map(&:inspect).to_sentence
end
# @private
class ParametersDoubles
def self.permitted_params_within(double_collection)
double_collection.calls_to(:permit).map(&:args).flatten
end
def initialize
klass = ::ActionController::Parameters
@double_collection = Doublespeak.double_collection_for(klass)
end
def register
double_collection.register_proxy(:permit)
end
def permitted_params
ParametersDoubles.permitted_params_within(double_collection)
end
protected
attr_reader :double_collection
end
# @private
class SliceOfParametersDoubles
TOP_LEVEL = Object.new
def initialize(subparameter)
klass = ::ActionController::Parameters
@subparameter = subparameter
@double_collections_by_param = {
TOP_LEVEL => Doublespeak.double_collection_for(klass)
}
end
def register
top_level_collection = double_collections_by_param[TOP_LEVEL]
double_permit_on(top_level_collection)
double_require_on(top_level_collection)
end
def permitted_params
if double_collections_by_param.key?(subparameter)
ParametersDoubles.permitted_params_within(
double_collections_by_param[subparameter]
)
else
[]
end
end
protected
attr_reader :subparameter, :double_collections_by_param
private
def double_permit_on(double_collection)
double_collection.register_proxy(:permit)
end
def double_require_on(double_collection)
double_collections_by_param = @double_collections_by_param
require_double = double_collection.register_proxy(:require)
require_double.to_return do |call|
param_name = call.args.first
params = call.return_value
double_collections_by_param[param_name] ||=
double_permit_against(params)
end
end
def double_permit_against(params)
klass = params.singleton_class
Doublespeak.double_collection_for(klass).tap do |double_collection|
double_permit_on(double_collection)
end
end
end
# @private
class ActionNotDefinedError < StandardError
def message

View file

@ -13,6 +13,16 @@ module Shoulda
def world
@_world ||= World.new
end
def debugging_enabled?
ENV['DEBUG_DOUBLESPEAK'] == '1'
end
def debug(&block)
if debugging_enabled?
puts block.call
end
end
end
end
end

View file

@ -1,16 +1,86 @@
require 'unit_spec_helper'
describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller do
shared_examples 'basic tests' do
it 'accepts a subset of the permitted attributes' do
define_controller_with_strong_parameters(action: :create) do |ctrl|
params_with_conditional_require(ctrl.params).permit(:name, :age)
end
expect(controller).to permit_with_conditional_params(
permit(:name).for(:create)
)
end
it 'accepts all of the permitted attributes' do
define_controller_with_strong_parameters(action: :create) do |ctrl|
params_with_conditional_require(ctrl.params).permit(:name, :age)
end
expect(controller).to permit_with_conditional_params(
permit(:name, :age).for(:create)
)
end
it 'rejects attributes that have not been permitted' do
define_controller_with_strong_parameters(action: :create) do |ctrl|
params_with_conditional_require(ctrl.params).permit(:name)
end
expect(controller).not_to permit_with_conditional_params(
permit(:name, :admin).for(:create)
)
end
it 'rejects when #permit has not been called' do
define_controller_with_strong_parameters(action: :create)
expect(controller).not_to permit_with_conditional_params(
permit(:name).for(:create)
)
end
it 'tracks multiple calls to #permit' do
sets_of_attributes = [
[:eta, :diner_id],
[:phone_number, :address_1, :address_2, :city, :state, :zip]
]
params = {
order: { some: 'value' },
diner: { some: 'value' }
}
define_controller_with_strong_parameters(action: :create) do |ctrl|
params_with_conditional_require(ctrl.params, :order).
permit(sets_of_attributes[0])
params_with_conditional_require(ctrl.params, :diner).
permit(sets_of_attributes[1])
end
expect(controller).to permit_with_conditional_params(
permit(*sets_of_attributes[0]).for(:create),
params
)
expect(controller).to permit_with_conditional_params(
permit(*sets_of_attributes[1]).for(:create),
params
)
end
end
it 'requires an action' do
assertion = -> { expect(controller).to permit(:name) }
define_controller_for_resource_with_strong_parameters
define_controller_with_strong_parameters
expect(&assertion).to raise_error(described_class::ActionNotDefinedError)
end
it 'requires a verb for a non-restful action' do
define_controller_for_resource_with_strong_parameters
define_controller_with_strong_parameters
assertion = lambda do
expect(controller).to permit(:name).for(:authorize)
@ -19,54 +89,64 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
expect(&assertion).to raise_error(described_class::VerbNotDefinedError)
end
it 'accepts a subset of the permitted attributes' do
define_controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name, :age)
end
context 'when operating on the entire params hash' do
include_context 'basic tests' do
def permit_with_conditional_params(permit, _params = {})
permit
end
expect(controller).to permit(:name).for(:create)
def params_with_conditional_require(params, *filters)
params
end
end
end
it 'accepts all of the permitted attributes' do
define_controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name, :age)
context 'when operating on a slice of the params hash' do
include_context 'basic tests' do
def permit_with_conditional_params(permit, params = nil)
params ||= { user: { some: 'value' } }
permit.add_params(params)
end
def params_with_conditional_require(params, *filters)
if filters.none?
filters = [:user]
end
params.require(*filters)
end
end
expect(controller).to permit(:name, :age).for(:create)
end
it 'rejects if asserting that parameters were not permitted, but on the wrong slice' do
define_controller_with_strong_parameters(action: :create) do
params.require(:order).permit(:eta, :diner_id)
end
it 'rejects attributes that have not been permitted' do
define_controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name)
expect(controller).
not_to permit(:eta, :diner_id).
for(:create, params: { order: { some: 'value' } }).
on(:something_else)
end
expect(controller).not_to permit(:name, :admin).for(:create)
end
it 'rejects when #permit has not been called' do
define_controller_for_resource_with_strong_parameters(action: :create)
expect(controller).not_to permit(:name).for(:create)
end
it 'can be used more than once in the same test' do
define_controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name)
define_controller_with_strong_parameters(action: :create) do
params.permit(:name)
end
expect(controller).to permit(:name).for(:create)
expect(controller).not_to permit(:admin).for(:create)
end
it 'works with routes that require extra params' do
it 'allows extra parameters to be passed to the action if it requires them' do
options = {
controller_name: 'Posts',
action: :show,
routes: -> { get '/posts/:slug', to: 'posts#show' }
}
define_controller_for_resource_with_strong_parameters(options) do
params.require(:user).permit(:name)
define_controller_with_strong_parameters(options) do
params.permit(:name)
end
expect(controller).
@ -75,8 +155,8 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
end
it 'works with #update specifically' do
define_controller_for_resource_with_strong_parameters(action: :update) do
params.require(:user).permit(:name)
define_controller_with_strong_parameters(action: :update) do
params.permit(:name)
end
expect(controller).
@ -84,27 +164,12 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
for(:update, params: { id: 1 })
end
it 'tracks multiple calls to #permit' do
sets_of_attributes = [
[:eta, :diner_id],
[:phone_number, :address_1, :address_2, :city, :state, :zip]
]
define_controller_for_resource_with_strong_parameters(action: :create) do
params.require(:order).permit(sets_of_attributes[0])
params.require(:diner).permit(sets_of_attributes[1])
end
expect(controller).to permit(*sets_of_attributes[0]).for(:create)
expect(controller).to permit(*sets_of_attributes[1]).for(:create)
end
describe '#matches?' do
it 'does not raise an error when #fetch was used instead of #require (issue #495)' do
matcher = permit(:eta, :diner_id).for(:create)
matching = -> { matcher.matches?(controller) }
define_controller_for_resource_with_strong_parameters(action: :create) do
define_controller_with_strong_parameters(action: :create) do
params.fetch(:order, {}).permit(:eta, :diner_id)
end
@ -120,12 +185,12 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
params: { user: { some: 'params' } }
)
define_controller_for_resource_with_strong_parameters(action: :create) do
define_controller_with_strong_parameters(action: :create) do
params[:foo] = 'bar'
actual_foo_param = params[:foo]
actual_user_params = params[:user]
params.require(:user).permit(:name)
params.permit(:name)
end
matcher.matches?(controller)
@ -134,11 +199,32 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
expect(actual_foo_param).to eq 'bar'
end
it 'still allows #require to return a slice of the params' do
expected_user_params = { 'foo' => 'bar' }
actual_user_params = nil
matcher = permit(:name).for(
:update,
params: { id: 1, user: expected_user_params }
)
define_controller_with_strong_parameters(action: :update) do
actual_user_params = params.require(:user)
begin
actual_user_params.permit(:name)
rescue
end
end
matcher.matches?(controller)
expect(actual_user_params).to eq expected_user_params
end
it 'does not permanently stub the params hash' do
matcher = permit(:name).for(:create)
params_access = -> { controller.params.require(:user) }
define_controller_for_resource_with_strong_parameters(action: :create)
define_controller_with_strong_parameters(action: :create)
matcher.matches?(controller)
@ -167,36 +253,38 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
it 'returns the correct string' do
options = { action: :create, method: :post }
define_controller_for_resource_with_strong_parameters(options) do
define_controller_with_strong_parameters(options) do
params.permit(:name, :age)
end
matcher = described_class.new([:name, :age, :height]).for(:create)
expect(matcher.description).
to eq 'permit POST #create to receive parameters :name, :age, and :height'
expect(matcher.description).to eq(
'(on POST #create) restrict parameters to :name, :age, and :height'
)
end
context 'when a verb is specified' do
it 'returns the correct string' do
options = { action: :some_action }
define_controller_for_resource_with_strong_parameters(options) do
define_controller_with_strong_parameters(options) do
params.permit(:name, :age)
end
matcher = described_class.
new([:name]).
for(:some_action, verb: :put)
expect(matcher.description).
to eq 'permit PUT #some_action to receive parameters :name'
expect(matcher.description).to eq(
'(on PUT #some_action) restrict parameters to :name'
)
end
end
end
describe 'positive failure message' do
it 'includes all missing attributes' do
define_controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name, :age)
define_controller_with_strong_parameters(action: :create) do
params.permit(:name, :age)
end
assertion = lambda do
@ -205,25 +293,31 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
for(:create)
end
expect(&assertion).to fail_with_message(
'Expected controller to permit city and country, but it did not.'
)
message =
"Expected POST #create to restrict parameters to " +
":name, :age, :city, and :country,\n" +
"but the restricted parameters were :name and :age instead."
expect(&assertion).to fail_with_message(message)
end
end
describe 'negative failure message' do
it 'includes all attributes that should not have been permitted but were' do
define_controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name, :age)
define_controller_with_strong_parameters(action: :create) do
params.permit(:name, :age)
end
assertion = lambda do
expect(controller).not_to permit(:name, :age).for(:create)
end
expect(&assertion).to fail_with_message(
'Expected controller not to permit name and age, but it did.'
)
message =
"Expected POST #create not to restrict parameters to " +
":name and :age,\n" +
"but it did."
expect(&assertion).to fail_with_message(message)
end
end
@ -283,10 +377,7 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
Class.new(StandardError)
end
def define_controller_for_resource_with_strong_parameters(
options = {},
&action_body
)
def define_controller_with_strong_parameters(options = {}, &action_body)
model_name = options.fetch(:model_name, 'User')
controller_name = options.fetch(:controller_name, 'UsersController')
collection_name = controller_name.
@ -300,7 +391,11 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
controller_class = define_controller(controller_name) do
define_method action_name do
if action_body
instance_eval(&action_body)
if action_body.arity == 0
instance_eval(&action_body)
else
action_body.call(self)
end
end
render nothing: true