add handling for backwards-compatibility and update documentation

3 files changed, 67 insertions, 4 deletions

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):
   #   # <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
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