Remove dependencies for `StrongParametersMatcher`
Refactor the `StrongParametersMatcher` to work without depending on bourne for spying on methods. Use an internal spying tool to provide platform independence. Other improvements include: * Standardize `failure_message` method naming * Properly detect when a matcher does not match * Add documentation for `permit` matcher.
This commit is contained in:
parent
d7eec9ccc9
commit
723b5d3552
|
@ -3,7 +3,6 @@ PATH
|
|||
specs:
|
||||
shoulda-matchers (2.5.0)
|
||||
activesupport (>= 3.0.0)
|
||||
bourne (~> 1.3)
|
||||
|
||||
GEM
|
||||
remote: https://rubygems.org/
|
||||
|
@ -140,6 +139,7 @@ DEPENDENCIES
|
|||
activerecord-jdbcsqlite3-adapter
|
||||
appraisal (~> 1.0.0.beta2)
|
||||
aruba
|
||||
bourne (~> 1.3)
|
||||
bundler (~> 1.1)
|
||||
cucumber (~> 1.1)
|
||||
jdbc-sqlite3
|
||||
|
|
30
README.md
30
README.md
|
@ -909,7 +909,7 @@ end
|
|||
|
||||
### ActionController Matchers
|
||||
|
||||
*Jump to: [filter_param](#filter_param), [redirect_to](#redirect_to), [render_template](#render_template), [render_with_layout](#render_with_layout), [rescue_from](#rescue_from), [respond_with](#respond_with), [route](#route), [set_session](#set_session), [set_the_flash](#set_the_flash), [use_after_filter / use_after_action](#use_after_filter--use_after_action), [use_around_filter / use_around_action](#use_around_filter--use_around_action), [use_before_filter / use_around_action](#use_before_filter--use_before_action)*
|
||||
*Jump to: [filter_param](#filter_param), [permit](#permit), [redirect_to](#redirect_to), [render_template](#render_template), [render_with_layout](#render_with_layout), [rescue_from](#rescue_from), [respond_with](#respond_with), [route](#route), [set_session](#set_session), [set_the_flash](#set_the_flash), [use_after_filter / use_after_action](#use_after_filter--use_after_action), [use_around_filter / use_around_action](#use_around_filter--use_around_action), [use_before_filter / use_around_action](#use_before_filter--use_before_action)*
|
||||
|
||||
#### filter_param
|
||||
|
||||
|
@ -931,6 +931,34 @@ class ApplicationControllerTest < ActionController::TestCase
|
|||
end
|
||||
```
|
||||
|
||||
#### permit
|
||||
|
||||
The `permit` matcher tests that only whitelisted parameters are permitted.
|
||||
|
||||
```ruby
|
||||
class UserController < ActionController::Base
|
||||
def create
|
||||
User.create(user_params)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def user_params
|
||||
params.require(:user).permit(:email)
|
||||
end
|
||||
end
|
||||
|
||||
# RSpec
|
||||
describe UserController do
|
||||
it { should permit(:email).for(:create) }
|
||||
end
|
||||
|
||||
# Test::Unit
|
||||
class UserControllerTest < ActionController::TestCase
|
||||
should permit(:email).for(:create)
|
||||
end
|
||||
```
|
||||
|
||||
#### redirect_to
|
||||
|
||||
The `redirect_to` matcher tests that an action redirects to a certain location.
|
||||
|
|
|
@ -3,7 +3,6 @@ PATH
|
|||
specs:
|
||||
shoulda-matchers (2.5.0)
|
||||
activesupport (>= 3.0.0)
|
||||
bourne (~> 1.3)
|
||||
|
||||
GEM
|
||||
remote: https://rubygems.org/
|
||||
|
|
|
@ -1,5 +1,3 @@
|
|||
require 'bourne'
|
||||
require 'active_support/deprecation'
|
||||
begin
|
||||
require 'strong_parameters'
|
||||
rescue LoadError
|
||||
|
@ -9,16 +7,23 @@ module Shoulda
|
|||
module Matchers
|
||||
module ActionController
|
||||
def permit(*attributes)
|
||||
attributes_and_context = attributes + [self]
|
||||
StrongParametersMatcher.new(*attributes_and_context)
|
||||
StrongParametersMatcher.new(self, attributes)
|
||||
end
|
||||
|
||||
class StrongParametersMatcher
|
||||
def initialize(*attributes_and_context)
|
||||
ActiveSupport::Deprecation.warn 'The strong_parameters matcher is deprecated and will be removed in 2.0'
|
||||
@attributes = attributes_and_context[0...-1]
|
||||
@context = attributes_and_context.last
|
||||
@permitted_params = []
|
||||
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
|
||||
|
||||
def initialize(context = nil, attributes)
|
||||
@attributes = attributes
|
||||
@context = context
|
||||
end
|
||||
|
||||
def for(action, options = {})
|
||||
|
@ -37,52 +42,53 @@ module Shoulda
|
|||
end
|
||||
|
||||
def does_not_match?(controller = nil)
|
||||
simulate_controller_action && parameters_difference.present?
|
||||
simulate_controller_action && parameters_intersection.empty?
|
||||
end
|
||||
|
||||
def failure_message
|
||||
"Expected controller to permit #{parameters_difference.to_sentence}, but it did not."
|
||||
end
|
||||
alias failure_message_for_should failure_message
|
||||
|
||||
def negative_failure_message
|
||||
"Expected controller not to permit #{parameters_difference.to_sentence}, but it did."
|
||||
def failure_message_when_negated
|
||||
"Expected controller not to permit #{parameters_intersection.to_sentence}, but it did."
|
||||
end
|
||||
alias failure_message_for_should_not failure_message_when_negated
|
||||
|
||||
private
|
||||
|
||||
attr_reader :verb, :action, :attributes, :context
|
||||
attr_accessor :permitted_params
|
||||
|
||||
def simulate_controller_action
|
||||
ensure_action_and_verb_present!
|
||||
model_attrs = stubbed_model_attributes
|
||||
stubbed_model_attributes
|
||||
|
||||
context.send(verb, action)
|
||||
|
||||
verify_permit_call(model_attrs)
|
||||
verify_permit_call
|
||||
end
|
||||
|
||||
def verify_permit_call(model_attrs)
|
||||
matcher = Mocha::API::HaveReceived.new(:permit).with do |*params|
|
||||
self.permitted_params = params
|
||||
end
|
||||
|
||||
matcher.matches?(model_attrs)
|
||||
rescue Mocha::ExpectationError
|
||||
false
|
||||
def verify_permit_call
|
||||
@model_attrs.permit_was_called
|
||||
end
|
||||
|
||||
def parameters_difference
|
||||
attributes - permitted_params
|
||||
attributes - @model_attrs.shoulda_permitted_params
|
||||
end
|
||||
|
||||
def parameters_intersection
|
||||
attributes & @model_attrs.shoulda_permitted_params
|
||||
end
|
||||
|
||||
def stubbed_model_attributes
|
||||
extend Mocha::API
|
||||
@model_attrs = self.class.stubbed_parameters_class.new(arbitrary_attributes)
|
||||
|
||||
model_attrs = ::ActionController::Parameters.new(arbitrary_attributes)
|
||||
model_attrs.stubs(:permit)
|
||||
::ActionController::Parameters.any_instance.stubs(:[]).returns(model_attrs)
|
||||
|
||||
model_attrs
|
||||
local_model_attrs = @model_attrs
|
||||
::ActionController::Parameters.class_eval do
|
||||
define_method :[] do |*args|
|
||||
local_model_attrs
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def ensure_action_and_verb_present!
|
||||
|
@ -95,15 +101,34 @@ module Shoulda
|
|||
end
|
||||
|
||||
def arbitrary_attributes
|
||||
{:any_key => 'any_value'}
|
||||
{any_key: 'any_value'}
|
||||
end
|
||||
|
||||
def verb_for_action
|
||||
verb_lookup = { :create => :post, :update => :put }
|
||||
verb_lookup = { create: :post, update: :put }
|
||||
verb_lookup[action]
|
||||
end
|
||||
end
|
||||
|
||||
module StrongParametersMatcher::StubbedParameters
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
included do
|
||||
attr_accessor :permit_was_called, :shoulda_permitted_params
|
||||
end
|
||||
|
||||
def initialize(*)
|
||||
@permit_was_called = false
|
||||
super
|
||||
end
|
||||
|
||||
def permit(*args)
|
||||
self.shoulda_permitted_params = args
|
||||
self.permit_was_called = true
|
||||
nil
|
||||
end
|
||||
end
|
||||
|
||||
class StrongParametersMatcher::ActionNotDefinedError < StandardError
|
||||
def message
|
||||
'You must specify the controller action using the #for method.'
|
||||
|
@ -113,7 +138,7 @@ module Shoulda
|
|||
class StrongParametersMatcher::VerbNotDefinedError < StandardError
|
||||
def message
|
||||
'You must specify an HTTP verb when using a non-RESTful action.' +
|
||||
' e.g. for(:authorize, :verb => :post)'
|
||||
' e.g. for(:authorize, verb: :post)'
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,142 +1,133 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Shoulda::Matchers::ActionController do
|
||||
describe ".permit" do
|
||||
it "is true when the sent parameter is allowed" do
|
||||
controller_class = controller_for_resource_with_strong_parameters do
|
||||
describe "#permit" do
|
||||
it 'matches when the sent parameter is allowed' do
|
||||
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
|
||||
params.require(:user).permit(:name)
|
||||
end
|
||||
|
||||
controller_class.should permit(:name).for(:create)
|
||||
expect(controller_class).to permit(:name).for(:create)
|
||||
end
|
||||
|
||||
it "is false when the sent parameter is not allowed" do
|
||||
controller_class = controller_for_resource_with_strong_parameters do
|
||||
it 'does not match when the sent parameter is not allowed' do
|
||||
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
|
||||
params.require(:user).permit(:name)
|
||||
end
|
||||
|
||||
controller_class.should_not permit(:admin).for(:create)
|
||||
expect(controller_class).not_to permit(:admin).for(:create)
|
||||
end
|
||||
|
||||
it "allows multiple attributes" do
|
||||
controller_class = controller_for_resource_with_strong_parameters do
|
||||
it 'matches against multiple attributes' do
|
||||
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
|
||||
params.require(:user).permit(:name, :age)
|
||||
end
|
||||
|
||||
controller_class.should permit(:name, :age).for(:create)
|
||||
expect(controller_class).to permit(:name, :age).for(:create)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe Shoulda::Matchers::ActionController::StrongParametersMatcher do
|
||||
before do
|
||||
controller_for_resource_with_strong_parameters do
|
||||
params.require(:user).permit(:name, :age)
|
||||
end
|
||||
end
|
||||
|
||||
describe "#matches?" do
|
||||
it "is true for a subset of the allowable attributes" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, self).for(:create)
|
||||
matcher.matches?.should be_true
|
||||
controller_for_resource_with_strong_parameters(action: :create) do
|
||||
params.require(:user).permit(:name)
|
||||
end
|
||||
|
||||
matcher = described_class.new([:name]).in_context(self).for(:create)
|
||||
expect(matcher.matches?).to be_true
|
||||
end
|
||||
|
||||
it "is true for all the allowable attributes" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, :age, self).for(:create)
|
||||
matcher.matches?.should be_true
|
||||
controller_for_resource_with_strong_parameters(action: :create) do
|
||||
params.require(:user).permit(:name, :age)
|
||||
end
|
||||
|
||||
matcher = described_class.new([:name, :age]).in_context(self).for(:create)
|
||||
expect(matcher.matches?).to be_true
|
||||
end
|
||||
|
||||
it "is false when any attributes are not allowed" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, :admin, self).for(:create)
|
||||
matcher.matches?.should be_false
|
||||
controller_for_resource_with_strong_parameters(action: :create) do
|
||||
params.require(:user).permit(:name)
|
||||
end
|
||||
|
||||
matcher = described_class.new([:name, :admin]).in_context(self).for(:create)
|
||||
expect(matcher.matches?).to be_false
|
||||
end
|
||||
|
||||
it "is false when permit is not called" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, self).for(:new, :verb => :get)
|
||||
matcher.matches?.should be_false
|
||||
controller_for_resource_with_strong_parameters(action: :create) {}
|
||||
|
||||
matcher = described_class.new([:name]).in_context(self).for(:create)
|
||||
expect(matcher.matches?).to be_false
|
||||
end
|
||||
|
||||
it "requires an action" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, self)
|
||||
expect{ matcher.matches? }.to raise_error(Shoulda::Matchers::ActionController::StrongParametersMatcher::ActionNotDefinedError)
|
||||
matcher = described_class.new([:name])
|
||||
expect { matcher.matches? }
|
||||
.to raise_error(Shoulda::Matchers::ActionController::StrongParametersMatcher::ActionNotDefinedError)
|
||||
end
|
||||
|
||||
it "requires a verb for non-restful action" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, self).for(:authorize)
|
||||
expect{ matcher.matches? }.to raise_error(Shoulda::Matchers::ActionController::StrongParametersMatcher::VerbNotDefinedError)
|
||||
matcher = described_class.new([:name]).for(:authorize)
|
||||
expect { matcher.matches? }
|
||||
.to raise_error(Shoulda::Matchers::ActionController::StrongParametersMatcher::VerbNotDefinedError)
|
||||
end
|
||||
end
|
||||
|
||||
describe "#does_not_match?" do
|
||||
it "it is true if any of the given attributes are allowed" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, :admin, self).for(:create)
|
||||
matcher.does_not_match?.should be_true
|
||||
end
|
||||
|
||||
it "it is false if all of the given attribtues are allowed" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, :age, self).for(:create)
|
||||
matcher.does_not_match?.should be_false
|
||||
end
|
||||
end
|
||||
|
||||
describe "#failure_message" do
|
||||
describe "failure message" do
|
||||
it "includes all missing attributes" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, :age, :city, :country, self).for(:create)
|
||||
matcher.matches?
|
||||
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
|
||||
params.require(:user).permit(:name, :age)
|
||||
end
|
||||
|
||||
matcher.failure_message.should eq("Expected controller to permit city and country, but it did not.")
|
||||
expect {
|
||||
expect(controller_class).to permit(:name, :age, :city, :country).for(:create)
|
||||
}.to fail_with_message("Expected controller to permit city and country, but it did not.")
|
||||
end
|
||||
end
|
||||
|
||||
describe "#negative_failure_message" do
|
||||
it "includes all attributes that should not have been allowed but were" do
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, :age, :city, :country, self).for(:create)
|
||||
matcher.does_not_match?.should be_true
|
||||
controller_class = controller_for_resource_with_strong_parameters(action: :create) do
|
||||
params.require(:user).permit(:name, :age)
|
||||
end
|
||||
|
||||
matcher.negative_failure_message.should eq("Expected controller not to permit city and country, but it did.")
|
||||
expect {
|
||||
expect(controller_class).not_to permit(:name, :age).for(:create)
|
||||
}.to fail_with_message("Expected controller not to permit name and age, but it did.")
|
||||
end
|
||||
end
|
||||
|
||||
describe "#for" do
|
||||
context "when given :create" do
|
||||
it "posts to the controller" do
|
||||
context = stub('context', :post => nil)
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, context).for(:create)
|
||||
context = stub('context', post: nil)
|
||||
matcher = described_class.new([:name]).in_context(context).for(:create)
|
||||
|
||||
matcher.matches?
|
||||
context.should have_received(:post).with(:create)
|
||||
expect(context).to have_received(:post).with(:create)
|
||||
end
|
||||
end
|
||||
|
||||
context "when given :update" do
|
||||
it "puts to the controller" do
|
||||
context = stub('context', :put => nil)
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, context).for(:update)
|
||||
context = stub('context', put: nil)
|
||||
matcher = described_class.new([:name]).in_context(context).for(:update)
|
||||
|
||||
matcher.matches?
|
||||
context.should have_received(:put).with(:update)
|
||||
expect(context).to have_received(:put).with(:update)
|
||||
end
|
||||
end
|
||||
|
||||
context "when given a custom action and verb" do
|
||||
it "puts to the controller" do
|
||||
context = stub('context', :delete => nil)
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, context).for(:hide, :verb => :delete)
|
||||
it "deletes to the controller" do
|
||||
context = stub('context', delete: nil)
|
||||
matcher = described_class.new([:name]).in_context(context).for(:hide, verb: :delete)
|
||||
|
||||
matcher.matches?
|
||||
context.should have_received(:delete).with(:hide)
|
||||
expect(context).to have_received(:delete).with(:hide)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "#in_context" do
|
||||
it 'sets the object the controller action is sent to' do
|
||||
context = stub('context', :post => nil)
|
||||
matcher = Shoulda::Matchers::ActionController::StrongParametersMatcher.new(:name, nil).for(:create).in_context(context)
|
||||
|
||||
matcher.matches?
|
||||
|
||||
context.should have_received(:post).with(:create)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -58,17 +58,12 @@ module ControllerBuilder
|
|||
$test_app.create_temp_view(path, contents)
|
||||
end
|
||||
|
||||
def controller_for_resource_with_strong_parameters(&block)
|
||||
def controller_for_resource_with_strong_parameters(options = {}, &block)
|
||||
define_model "User"
|
||||
controller_class = define_controller "Users" do
|
||||
def new
|
||||
@user = User.new
|
||||
render :nothing => true
|
||||
end
|
||||
|
||||
def create
|
||||
define_method options.fetch(:action) do
|
||||
@user = User.create(user_params)
|
||||
render :nothing => true
|
||||
render nothing: true
|
||||
end
|
||||
|
||||
private
|
||||
|
|
Loading…
Reference in New Issue