From 02859745cc0db02fcb1af27da345456a3b5e1eb2 Mon Sep 17 00:00:00 2001 From: Matt Jones Date: Sat, 4 Feb 2012 16:02:52 -0600 Subject: add handling for backwards-compatibility and update documentation --- activeresource/lib/active_resource/base.rb | 13 ++++++++-- activeresource/lib/active_resource/validations.rb | 28 +++++++++++++++++++-- activeresource/test/cases/base_errors_test.rb | 30 +++++++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) (limited to 'activeresource') diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index c0d51797ee..5ef50b6e03 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -234,7 +234,7 @@ 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: @@ -242,12 +242,21 @@ module ActiveResource # # Response (422): # # First cannot be empty # # 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 diff --git a/activeresource/lib/active_resource/validations.rb b/activeresource/lib/active_resource/validations.rb index c11908eb86..028acb8bce 100644 --- a/activeresource/lib/active_resource/validations.rb +++ b/activeresource/lib/active_resource/validations.rb @@ -25,6 +25,12 @@ module ActiveResource 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 @@ -32,7 +38,11 @@ module ActiveResource 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 @@ -41,8 +51,22 @@ module ActiveResource # Grabs errors from a json response. def from_json(json, save_cache = false) - hash = ActiveSupport::JSON.decode(json)['errors'] || {} rescue {} - from_hash hash, 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. diff --git a/activeresource/test/cases/base_errors_test.rb b/activeresource/test/cases/base_errors_test.rb index 98fef5fa73..88ac2de96e 100644 --- a/activeresource/test/cases/base_errors_test.rb +++ b/activeresource/test/cases/base_errors_test.rb @@ -93,6 +93,36 @@ class BaseErrorsTest < ActiveSupport::TestCase 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 -- cgit v1.2.3