Skip to content

Commit

Permalink
Revert "Merge pull request #9660 from sebasoga/change_strong_paramete…
Browse files Browse the repository at this point in the history
…rs_require_behaviour"

This reverts commit c2b5a8e, reversing
changes made to 1918b12.

See: #9660 (comment)
  • Loading branch information
guilleiguaran committed Nov 2, 2013
1 parent db41eb8 commit f886fe2
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 57 deletions.
32 changes: 9 additions & 23 deletions actionpack/lib/action_controller/metal/strong_parameters.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ module ActionController
# params = ActionController::Parameters.new(a: {}) # params = ActionController::Parameters.new(a: {})
# params.fetch(:b) # params.fetch(:b)
# # => ActionController::ParameterMissing: param not found: b # # => ActionController::ParameterMissing: param not found: b
# params.require(:a)
# # => ActionController::ParameterMissing: param not found: a
class ParameterMissing < KeyError class ParameterMissing < KeyError
attr_reader :param # :nodoc: attr_reader :param # :nodoc:


Expand All @@ -19,20 +21,6 @@ def initialize(param) # :nodoc:
end end
end end


# Raised when a required parameter value is empty.
#
# params = ActionController::Parameters.new(a: {})
# params.require(:a)
# # => ActionController::EmptyParameter: value is empty for required key: a
class EmptyParameter < IndexError
attr_reader :param

def initialize(param)
@param = param
super("value is empty for required key: #{param}")
end
end

# Raised when a supplied parameter is not expected. # Raised when a supplied parameter is not expected.
# #
# params = ActionController::Parameters.new(a: "123", b: "456") # params = ActionController::Parameters.new(a: "123", b: "456")
Expand Down Expand Up @@ -169,22 +157,20 @@ def permit!
self self
end end


# Ensures that a parameter is present. If it's present and not empty, # Ensures that a parameter is present. If it's present, returns
# returns the parameter at the given +key+, if it's empty raises # the parameter at the given +key+, otherwise raises an
# an <tt>ActionController::EmptyParameter</tt> error, otherwise # <tt>ActionController::ParameterMissing</tt> error.
# raises an <tt>ActionController::ParameterMissing</tt> error.
# #
# ActionController::Parameters.new(person: { name: 'Francesco' }).require(:person) # ActionController::Parameters.new(person: { name: 'Francesco' }).require(:person)
# # => {"name"=>"Francesco"} # # => {"name"=>"Francesco"}
# #
# ActionController::Parameters.new(person: {}).require(:person) # ActionController::Parameters.new(person: nil).require(:person)
# # => ActionController::EmptyParameter: value is empty for required key: person # # => ActionController::ParameterMissing: param not found: person
# #
# ActionController::Parameters.new(name: {}).require(:person) # ActionController::Parameters.new(person: {}).require(:person)
# # => ActionController::ParameterMissing: param not found: person # # => ActionController::ParameterMissing: param not found: person
def require(key) def require(key)
raise(ActionController::ParameterMissing.new(key)) unless self.key?(key) self[key].presence || raise(ParameterMissing.new(key))
self[key].presence || raise(ActionController::EmptyParameter.new(key))
end end


# Alias of #require. # Alias of #require.
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ class ExceptionWrapper
'ActionController::InvalidAuthenticityToken' => :unprocessable_entity, 'ActionController::InvalidAuthenticityToken' => :unprocessable_entity,
'ActionDispatch::ParamsParser::ParseError' => :bad_request, 'ActionDispatch::ParamsParser::ParseError' => :bad_request,
'ActionController::BadRequest' => :bad_request, 'ActionController::BadRequest' => :bad_request,
'ActionController::ParameterMissing' => :bad_request, 'ActionController::ParameterMissing' => :bad_request
'ActionController::EmptyParameter' => :bad_request
) )


cattr_accessor :rescue_templates cattr_accessor :rescue_templates
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -2,14 +2,8 @@
require 'action_controller/metal/strong_parameters' require 'action_controller/metal/strong_parameters'


class ParametersRequireTest < ActiveSupport::TestCase class ParametersRequireTest < ActiveSupport::TestCase
test "required parameters must be present" do test "required parameters must be present not merely not nil" do
assert_raises(ActionController::ParameterMissing) do assert_raises(ActionController::ParameterMissing) do
ActionController::Parameters.new(name: {}).require(:person)
end
end

test "required parameters can't be blank" do
assert_raises(ActionController::EmptyParameter) do
ActionController::Parameters.new(person: {}).require(:person) ActionController::Parameters.new(person: {}).require(:person)
end end
end end
Expand Down
19 changes: 0 additions & 19 deletions actionpack/test/controller/required_params_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@ def create
params.require(:book).require(:name) params.require(:book).require(:name)
head :ok head :ok
end end

def update
params.require(:book)
head :ok
end
end end


class ActionControllerRequiredParamsTest < ActionController::TestCase class ActionControllerRequiredParamsTest < ActionController::TestCase
Expand All @@ -25,20 +20,6 @@ class ActionControllerRequiredParamsTest < ActionController::TestCase
end end
end end


test "empty required parameters will raise an exception" do
assert_raise ActionController::EmptyParameter do
put :update, {book: {}}
end

assert_raise ActionController::EmptyParameter do
put :update, {book: ''}
end

assert_raise ActionController::EmptyParameter do
put :update, {book: nil}
end
end

test "required parameters that are present will not raise" do test "required parameters that are present will not raise" do
post :create, { book: { name: "Mjallo!" } } post :create, { book: { name: "Mjallo!" } }
assert_response :ok assert_response :ok
Expand Down
6 changes: 0 additions & 6 deletions actionpack/test/dispatch/debug_exceptions_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ def call(env)
raise ActionController::UrlGenerationError, "No route matches" raise ActionController::UrlGenerationError, "No route matches"
when "/parameter_missing" when "/parameter_missing"
raise ActionController::ParameterMissing, :missing_param_key raise ActionController::ParameterMissing, :missing_param_key
when "/required_key_empty_value"
raise ActionController::EmptyParameter, :empty_param_key
else else
raise "puke!" raise "puke!"
end end
Expand Down Expand Up @@ -128,10 +126,6 @@ def setup
get "/parameter_missing", {}, {'action_dispatch.show_exceptions' => true} get "/parameter_missing", {}, {'action_dispatch.show_exceptions' => true}
assert_response 400 assert_response 400
assert_match(/ActionController::ParameterMissing/, body) assert_match(/ActionController::ParameterMissing/, body)

get "/required_key_empty_value", {}, {'action_dispatch.show_exceptions' => true}
assert_response 400
assert_match(/ActionController::EmptyParameter/, body)
end end


test "rescue with text error for xhr request" do test "rescue with text error for xhr request" do
Expand Down

0 comments on commit f886fe2

Please sign in to comment.