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

Fix the permit matcher to be less intrusive

Browse source 
Within the `permit` matcher, we need to keep track of when #permit was
called and which arguments it was called with. Previously we did this by
overriding ActionController::Parameters#[] to return an instance of
StubbedParameters (which, when #permit was called on it, would then
track the call).

This has three problems:

* We're overriding something in a global class. We can simply override
  something on the controller that's being tested instead.
* Overriding #[] is kind of magical. How does this even end up doing
  what we want? Really we should just replace the entire params hash.
* Controller tests involve hitting actions on real controllers. Those
  actions may need to access the params hash. Completely changing how
  the params hash works prevents them from being able to do that.

To solve this, we override the params hash on the controller under test
with an instance of StubbedParameters, which decorates the original
params hash. We then override #permit to track the call, but then we let
it do its thing.
This commit is contained in:
Elliot Winkler 2014-04-16 02:58:05 -06:00
parent 025dfdc432
commit 2d52f537f9
4 changed files with 134 additions and 94 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
NEWS.md
View file

@ -1,7 +1,7 @@
# 2.6.1
* Fix `ComparisonMatcher` so that `validate_numericality_of` comparison matchers
work with large numbers (#483).
work with large numbers.
* Fix so that ActiveRecord matchers aren't included when ActiveRecord
isn't defined (i.e. if you are using ActiveModel only).
@ -11,6 +11,9 @@
`validate_presence_of` when used in conjunction with `has_secure_password`.
That fix has been updated so that it does not affect `allow_value`.
* Fix `permit` so that it does not interfere with the existing `params` hash of
the controller you're testing.
# 2.6.0
* The boolean argument to `have_db_index`'s `unique` option is now optional, for

110
lib/shoulda/matchers/action_controller/strong_parameters_matcher.rb
View file

@ -1,8 +1,12 @@
require 'delegate'
begin
require 'strong_parameters'
rescue LoadError
end
require 'active_support/hash_with_indifferent_access'
module Shoulda
module Matchers
module ActionController
@ -11,19 +15,12 @@ module Shoulda
end
class StrongParametersMatcher
def self.stubbed_parameters_class
@stubbed_parameters_class ||= build_stubbed_parameters_class
end
def self.build_stubbed_parameters_class
Class.new(::ActionController::Parameters) do
include StubbedParameters
end
end
attr_writer :stubbed_params
def initialize(context = nil, attributes)
@attributes = attributes
@context = context
@stubbed_params = NullStubbedParameters.new
end
def for(action, options = {})
@ -41,11 +38,13 @@ module Shoulda
"permit #{verb.upcase} ##{action} to receive parameters #{attributes_as_sentence}"
end
def matches?(controller = nil)
def matches?(controller)
@controller = controller
simulate_controller_action && parameters_difference.empty?
end
def does_not_match?(controller = nil)
def does_not_match?(controller)
@controller = controller
simulate_controller_action && parameters_intersection.empty?
end
@ -61,50 +60,48 @@ module Shoulda
private
attr_reader :verb, :action, :attributes, :context
attr_reader :controller, :verb, :action, :attributes, :context
def simulate_controller_action
ensure_action_and_verb_present!
stub_model_attributes
stub_params
begin
context.send(verb, action)
ensure
unstub_model_attributes
unstub_params
end
verify_permit_call
end
def verify_permit_call
@model_attrs.permit_was_called
@stubbed_params.permit_was_called
end
def parameters_difference
attributes - @model_attrs.shoulda_permitted_params
attributes - @stubbed_params.shoulda_permitted_params
end
def parameters_intersection
attributes & @model_attrs.shoulda_permitted_params
attributes & @stubbed_params.shoulda_permitted_params
end
def stub_model_attributes
@model_attrs = self.class.stubbed_parameters_class.new(arbitrary_attributes)
def stub_params
matcher = self
local_model_attrs = @model_attrs
::ActionController::Parameters.class_eval do
alias_method :'shoulda_original_[]', :[]
controller.singleton_class.class_eval do
alias_method :__shoulda_original_params__, :params
define_method :[] do |*args|
local_model_attrs
define_method :params do
matcher.stubbed_params = StubbedParameters.new(__shoulda_original_params__)
end
end
end
def unstub_model_attributes
::ActionController::Parameters.class_eval do
alias_method :[], :'shoulda_original_[]'
undef_method :'shoulda_original_[]'
def unstub_params
controller.singleton_class.class_eval do
alias_method :params, :__shoulda_original_params__
end
end
@ -117,10 +114,6 @@ module Shoulda
end
end
def arbitrary_attributes
{any_key: 'any_value'}
end
def verb_for_action
verb_lookup = { create: :post, update: :put }
verb_lookup[action]
@ -129,37 +122,44 @@ module Shoulda
def attributes_as_sentence
attributes.map(&:inspect).to_sentence
end
end
module StrongParametersMatcher::StubbedParameters
extend ActiveSupport::Concern
class StubbedParameters < SimpleDelegator
attr_reader :permit_was_called, :shoulda_permitted_params
included do
attr_accessor :permit_was_called, :shoulda_permitted_params
def initialize(original_params)
super(original_params)
@permit_was_called = false
end
def require(*args)
self
end
def permit(*args)
@shoulda_permitted_params = args
@permit_was_called = true
super(*args)
end
end
def initialize(*)
@permit_was_called = false
super
class NullStubbedParameters < ActiveSupport::HashWithIndifferentAccess
def permit_was_called; false; end
def shoulda_permitted_params; self; end
def require(*); self; end
def permit(*); self; end
end
def permit(*args)
self.shoulda_permitted_params = args
self.permit_was_called = true
nil
class ActionNotDefinedError < StandardError
def message
'You must specify the controller action using the #for method.'
end
end
end
class StrongParametersMatcher::ActionNotDefinedError < StandardError
def message
'You must specify the controller action using the #for method.'
end
end
class StrongParametersMatcher::VerbNotDefinedError < StandardError
def message
'You must specify an HTTP verb when using a non-RESTful action.' +
' e.g. for(:authorize, verb: :post)'
class VerbNotDefinedError < StandardError
def message
'You must specify an HTTP verb when using a non-RESTful action.' +
' e.g. for(:authorize, verb: :post)'
end
end
end
end

101
spec/shoulda/matchers/action_controller/strong_parameters_matcher_spec.rb
View file

@ -3,27 +3,27 @@ require 'spec_helper'
describe Shoulda::Matchers::ActionController do
describe "#permit" do
it 'matches when the sent parameter is allowed' do
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name)
end
expect(controller_class).to permit(:name).for(:create)
expect(@controller).to permit(:name).for(:create)
end
it 'does not match when the sent parameter is not allowed' do
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name)
end
expect(controller_class).not_to permit(:admin).for(:create)
expect(@controller).not_to permit(:admin).for(:create)
end
it 'matches against multiple attributes' do
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name, :age)
end
expect(controller_class).to permit(:name, :age).for(:create)
expect(@controller).to permit(:name, :age).for(:create)
end
end
end
@ -63,7 +63,7 @@ describe Shoulda::Matchers::ActionController::StrongParametersMatcher do
end
matcher = described_class.new([:name]).in_context(self).for(:create)
expect(matcher.matches?).to be_true
expect(matcher.matches?(@controller)).to be_true
end
it "is true for all the allowable attributes" do
@ -72,7 +72,7 @@ describe Shoulda::Matchers::ActionController::StrongParametersMatcher do
end
matcher = described_class.new([:name, :age]).in_context(self).for(:create)
expect(matcher.matches?).to be_true
expect(matcher.matches?(@controller)).to be_true
end
it "is false when any attributes are not allowed" do
@ -81,76 +81,104 @@ describe Shoulda::Matchers::ActionController::StrongParametersMatcher do
end
matcher = described_class.new([:name, :admin]).in_context(self).for(:create)
expect(matcher.matches?).to be_false
expect(matcher.matches?(@controller)).to be_false
end
it "is false when permit is not called" do
controller_for_resource_with_strong_parameters(action: :create) {}
controller_for_resource_with_strong_parameters(action: :create)
matcher = described_class.new([:name]).in_context(self).for(:create)
expect(matcher.matches?).to be_false
expect(matcher.matches?(@controller)).to be_false
end
it "requires an action" do
controller_for_resource_with_strong_parameters
matcher = described_class.new([:name])
expect { matcher.matches? }
expect { matcher.matches?(@controller) }
.to raise_error(Shoulda::Matchers::ActionController::StrongParametersMatcher::ActionNotDefinedError)
end
it "requires a verb for non-restful action" do
controller_for_resource_with_strong_parameters
matcher = described_class.new([:name]).for(:authorize)
expect { matcher.matches? }
expect { matcher.matches?(@controller) }
.to raise_error(Shoulda::Matchers::ActionController::StrongParametersMatcher::VerbNotDefinedError)
end
context 'Stubbing ActionController::Parameters#[]' do
it "does not permanently stub []" do
context 'stubbing params on the controller' do
it 'still allows the original params to be set and accessed' do
actual_value = nil
controller_for_resource_with_strong_parameters(action: :create) do
params[:foo] = 'bar'
actual_value = params[:foo]
params.require(:user).permit(:name)
end
matcher = described_class.new([:name]).in_context(self).for(:create)
matcher.matches?(@controller)
expect(actual_value).to eq 'bar'
end
it 'stubs the params while the controller action is being run' do
params_class = nil
controller_for_resource_with_strong_parameters(action: :create) do
params_class = params.class
params.require(:user).permit(:name)
end
matcher = described_class.new([:name]).in_context(self).for(:create)
matcher.matches?(@controller)
expect(params_class).to be described_class::StubbedParameters
end
it 'does not permanently stub params' do
controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name)
end
described_class.new([:name]).in_context(self).for(:create).matches?
matcher = described_class.new([:name]).in_context(self).for(:create)
matcher.matches?(@controller)
param = ActionController::Parameters.new(name: 'Ralph')[:name]
expect(param.singleton_class).not_to include(
Shoulda::Matchers::ActionController::StrongParametersMatcher::StubbedParameters
)
expect(@controller.params).to be_a(ActionController::Parameters)
end
it 'prevents permanently overwriting [] on error' do
it 'prevents permanently stubbing params on error' do
stub_controller_with_exception
begin
described_class.new([:name]).in_context(self).for(:create).matches?
matcher = described_class.new([:name]).in_context(self).for(:create)
matcher.matches?(@controller)
rescue SimulatedError
end
param = ActionController::Parameters.new(name: 'Ralph')[:name]
expect(param.singleton_class).not_to include(
Shoulda::Matchers::ActionController::StrongParametersMatcher::StubbedParameters
)
expect(@controller.params).to be_a(ActionController::Parameters)
end
end
end
describe "failure message" do
it "includes all missing attributes" do
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name, :age)
end
expect {
expect(controller_class).to permit(:name, :age, :city, :country).for(:create)
expect(@controller).to permit(:name, :age, :city, :country).for(:create)
}.to fail_with_message("Expected controller to permit city and country, but it did not.")
end
it "includes all attributes that should not have been allowed but were" do
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
controller_for_resource_with_strong_parameters(action: :create) do
params.require(:user).permit(:name, :age)
end
expect {
expect(controller_class).not_to permit(:name, :age).for(:create)
expect(@controller).not_to permit(:name, :age).for(:create)
}.to fail_with_message("Expected controller not to permit name and age, but it did.")
end
end
@ -158,43 +186,46 @@ describe Shoulda::Matchers::ActionController::StrongParametersMatcher do
describe "#for" do
context "when given :create" do
it "posts to the controller" do
controller = ActionController::Base.new
context = stub('context', post: nil)
matcher = described_class.new([:name]).in_context(context).for(:create)
matcher.matches?
matcher.matches?(controller)
expect(context).to have_received(:post).with(:create)
end
end
context "when given :update" do
it "puts to the controller" do
controller = ActionController::Base.new
context = stub('context', put: nil)
matcher = described_class.new([:name]).in_context(context).for(:update)
matcher.matches?
matcher.matches?(controller)
expect(context).to have_received(:put).with(:update)
end
end
context "when given a custom action and verb" do
it "deletes to the controller" do
controller = ActionController::Base.new
context = stub('context', delete: nil)
matcher = described_class.new([:name]).in_context(context).for(:hide, verb: :delete)
matcher.matches?
matcher.matches?(controller)
expect(context).to have_received(:delete).with(:hide)
end
end
end
def stub_controller_with_exception
controller = define_controller('Examples') do
controller_class = define_controller('Examples') do
def create
raise SimulatedError
end
end
setup_rails_controller_test(controller)
setup_rails_controller_test(controller_class)
define_routes do
get 'examples', to: 'examples#create'

12
spec/support/controller_builder.rb
View file

@ -59,9 +59,12 @@ module ControllerBuilder
end
def controller_for_resource_with_strong_parameters(options = {}, &block)
controller_name = 'Users'
block ||= -> { {} }
define_model "User"
controller_class = define_controller "Users" do
define_method options.fetch(:action) do
controller_class = define_controller controller_name do
define_method options.fetch(:action, 'some_action') do
@user = User.create(user_params)
render nothing: true
end
@ -72,7 +75,10 @@ module ControllerBuilder
setup_rails_controller_test(controller_class)
define_routes { resources :users }
collection_name = controller_name.
to_s.sub(/Controller$/, '').underscore.
to_sym
define_routes { resources(collection_name) }
controller_class
end