Permalink
Browse files

Merge pull request #4881 from al2o3cr/compatible_json_errors

Make ActiveResource error parsing interoperate with ActionController. Closes #4881.
  • Loading branch information...
2 parents d36cfe2 + 0285974 commit 681faad7f39b13421be6a6c8be592fbe137adfa8 @jeremy jeremy committed Feb 5, 2012
@@ -234,20 +234,29 @@ module ActiveResource
# ryan.save # => false
#
# # When
- # # PUT https://api.people.com/people/1.json
+ # # PUT https://api.people.com/people/1.xml
# # or
# # PUT https://api.people.com/people/1.json
# # is requested with invalid values, the response is:
# #
# # Response (422):
# # <errors><error>First cannot be empty</error></errors>
# # or
- # # {"errors":["First cannot be empty"]}
+ # # {"errors":{"first":["cannot be empty"]}}
# #
#
# ryan.errors.invalid?(:first) # => true
# ryan.errors.full_messages # => ['First cannot be empty']
#
+ # For backwards-compatibility with older endpoints, the following formats are also supported in JSON responses:
+ #
+ # # {"errors":['First cannot be empty']}
+ # # This was the required format for previous versions of ActiveResource
+ # # {"first":["cannot be empty"]}
+ # # This was the default format produced by respond_with in ActionController <3.2.1
+ #
+ # Parsing either of these formats will result in a deprecation warning.
+ #
# Learn more about Active Resource's validation features in the ActiveResource::Validations documentation.
#
# === Timeouts
@@ -25,10 +25,48 @@ def from_array(messages, save_cache = false)
end
end
+ # Grabs errors from a hash of attribute => array of errors elements
+ # The second parameter directs the errors cache to be cleared (default)
+ # or not (by passing true)
+ #
+ # Unrecognized attribute names will be humanized and added to the record's
+ # base errors.
+ def from_hash(messages, save_cache = false)
+ clear unless save_cache
+
+ messages.each do |(key,errors)|
+ errors.each do |error|
+ if @base.attributes.keys.include?(key)
+ add key, error
+ elsif key == 'base'
+ self[:base] << error
+ else
+ # reporting an error on an attribute not in attributes
+ # format and add them to base
+ self[:base] << "#{key.humanize} #{error}"
+ end
+ end
+ end
+ end
+
# Grabs errors from a json response.
def from_json(json, save_cache = false)
- array = Array.wrap(ActiveSupport::JSON.decode(json)['errors']) rescue []
- from_array array, save_cache
+ decoded = ActiveSupport::JSON.decode(json) || {} rescue {}
+ if decoded.kind_of?(Hash) && (decoded.has_key?('errors') || decoded.empty?)
+ errors = decoded['errors'] || {}
+ if errors.kind_of?(Array)
+ # 3.2.1-style with array of strings
+ ActiveSupport::Deprecation.warn('Returning errors as an array of strings is deprecated.')
+ from_array errors, save_cache
+ else
+ # 3.2.2+ style
+ from_hash errors, save_cache
+ end
+ else
+ # <3.2-style respond_with - lacks 'errors' key
+ ActiveSupport::Deprecation.warn('Returning errors as a hash without a root "errors" key is deprecated.')
+ from_hash decoded, save_cache
+ end
end
# Grabs errors from an XML response.
@@ -5,7 +5,7 @@ class BaseErrorsTest < ActiveSupport::TestCase
def setup
ActiveResource::HttpMock.respond_to do |mock|
mock.post "/people.xml", {}, %q(<?xml version="1.0" encoding="UTF-8"?><errors><error>Age can't be blank</error><error>Name can't be blank</error><error>Name must start with a letter</error><error>Person quota full for today.</error></errors>), 422, {'Content-Type' => 'application/xml; charset=utf-8'}
- mock.post "/people.json", {}, %q({"errors":["Age can't be blank","Name can't be blank","Name must start with a letter","Person quota full for today."]}), 422, {'Content-Type' => 'application/json; charset=utf-8'}
+ mock.post "/people.json", {}, %q({"errors":{"age":["can't be blank"],"name":["can't be blank", "must start with a letter"],"person":["quota full for today."]}}), 422, {'Content-Type' => 'application/json; charset=utf-8'}
end
end
@@ -83,7 +83,7 @@ def test_should_format_full_errors
def test_should_mark_as_invalid_when_content_type_is_unavailable_in_response_header
ActiveResource::HttpMock.respond_to do |mock|
mock.post "/people.xml", {}, %q(<?xml version="1.0" encoding="UTF-8"?><errors><error>Age can't be blank</error><error>Name can't be blank</error><error>Name must start with a letter</error><error>Person quota full for today.</error></errors>), 422, {}
- mock.post "/people.json", {}, %q({"errors":["Age can't be blank","Name can't be blank","Name must start with a letter","Person quota full for today."]}), 422, {}
+ mock.post "/people.json", {}, %q({"errors":{"age":["can't be blank"],"name":["can't be blank", "must start with a letter"],"person":["quota full for today."]}}), 422, {}
end
[ :json, :xml ].each do |format|
@@ -93,6 +93,36 @@ def test_should_mark_as_invalid_when_content_type_is_unavailable_in_response_hea
end
end
+ def test_should_parse_json_string_errors_with_an_errors_key
+ ActiveResource::HttpMock.respond_to do |mock|
+ mock.post "/people.json", {}, %q({"errors":["Age can't be blank", "Name can't be blank", "Name must start with a letter", "Person quota full for today."]}), 422, {'Content-Type' => 'application/json; charset=utf-8'}
+ end
+
+ assert_deprecated(/as an array/) do
+ invalid_user_using_format(:json) do
+ assert @person.errors[:name].any?
+ assert_equal ["can't be blank"], @person.errors[:age]
+ assert_equal ["can't be blank", "must start with a letter"], @person.errors[:name]
+ assert_equal ["Person quota full for today."], @person.errors[:base]
+ end
+ end
+ end
+
+ def test_should_parse_3_1_style_json_errors
+ ActiveResource::HttpMock.respond_to do |mock|
+ mock.post "/people.json", {}, %q({"age":["can't be blank"],"name":["can't be blank", "must start with a letter"],"person":["quota full for today."]}), 422, {'Content-Type' => 'application/json; charset=utf-8'}
+ end
+
+ assert_deprecated(/without a root/) do
+ invalid_user_using_format(:json) do
+ assert @person.errors[:name].any?
+ assert_equal ["can't be blank"], @person.errors[:age]
+ assert_equal ["can't be blank", "must start with a letter"], @person.errors[:name]
+ assert_equal ["Person quota full for today."], @person.errors[:base]
+ end
+ end
+ end
+
private
def invalid_user_using_format(mime_type_reference)
previous_format = Person.format

0 comments on commit 681faad

Please sign in to comment.