From 954cecc8daf24915e626856252d90f3c8938abe6 Mon Sep 17 00:00:00 2001
From: Sean Doyle <sean.p.doyle24@gmail.com>
Date: Wed, 22 Oct 2025 12:54:14 -0400
Subject: [PATCH] Implement `Errors#from_array` in terms of `#from_hash`

This commit changes the `ActiveResource::Errors#from_array`
method to be implemented in terms of its complementary `#from_hash`
method.

Rather than modifying the `Errors` instance itself (like managing
attribute and `:base` assignment, clearing, etc), `#from_array` will
instead transform the Array instance into a Hash keyed by known
attributes. Once the Hash is constructed, the method hands it off to the
`#from_hash` method.

The goal of this change is to reduce the duplicated burden of
responsibility.

Along with the implementation change, this commit also includes explicit
coverage of the `Errors#from_xml` and `Errors#from_json` methods.
They're part of the gem's public API (since they're documented and not
marked `# :nodoc:`), but aren't explicitly tested elsewhere in the
suite.

There are two XML-focused tests. The first covers the case where the
payload has multiple `<error>` elements:

```ruby
Hash.from_xml "<errors><error>Name can't be blank</error><error>Email can't be blank</error></errors>"
 # => {"errors"=>{"error"=>["Name can't be blank", "Email can't be blank"]}}
```

The call to `Hash.from_xml` transforms each `<error>` element into an
item in the Array.

The second test covers the case where the payload has a single
payload has a single `<error>` element:

```ruby
Hash.from_xml "<errors><error>Name can't be blank</error></errors>"
 # => {"errors"=>{"error"=>"Name can't be blank"}}
```

In this case, The call to `Hash.from_xml` transforms the `<error>`
element into a single String value.

Since the `Errors#from_xml` method is part of the public API, and since
its implementation invokes `Errors#from_array`, dedicating explicit test
coverage for both scenarios can help prevent regressions or
breaks in backwards-compatibility.
---
 lib/active_resource/validations.rb | 10 +++---
 test/cases/validations_test.rb     | 58 ++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/lib/active_resource/validations.rb b/lib/active_resource/validations.rb
index fee1525d92..8c1fca8ab6 100644
--- a/lib/active_resource/validations.rb
+++ b/lib/active_resource/validations.rb
@@ -14,16 +14,18 @@ class Errors < ActiveModel::Errors
     # The second parameter directs the errors cache to be cleared (default)
     # or not (by passing true).
     def from_array(messages, save_cache = false)
-      clear unless save_cache
+      errors = Hash.new { |hash, attr_name| hash[attr_name] = [] }
       humanized_attributes = Hash[@base.known_attributes.map { |attr_name| [ attr_name.humanize, attr_name ] }]
-      messages.each do |message|
+      messages.each_with_object(errors) do |message, hash|
         attr_message = humanized_attributes.keys.sort_by { |a| -a.length }.detect do |attr_name|
           if message[0, attr_name.size + 1] == "#{attr_name} "
-            add humanized_attributes[attr_name], message[(attr_name.size + 1)..-1]
+            hash[humanized_attributes[attr_name]] << message[(attr_name.size + 1)..-1]
           end
         end
-        add(:base, message) if attr_message.nil?
+        hash["base"] << message if attr_message.nil?
       end
+
+      from_hash errors, save_cache
     end
 
     # Grabs errors from a hash of attribute => array of errors elements
diff --git a/test/cases/validations_test.rb b/test/cases/validations_test.rb
index b4e2b92803..84e1d5fbe5 100644
--- a/test/cases/validations_test.rb
+++ b/test/cases/validations_test.rb
@@ -76,3 +76,61 @@ def new_project(opts = {})
       Project.new(VALID_PROJECT_HASH.merge(opts))
     end
 end
+
+class ErrorsTest < ActiveSupport::TestCase
+  def test_from_xml_with_multiple_errors
+    errors = Project.new.errors
+
+    errors.from_xml %q(<?xml version="1.0" encoding="UTF-8"?><errors><error>Name can't be blank</error><error>Email can't be blank</error></errors>)
+
+    assert_equal [ "can't be blank" ], errors[:name]
+    assert_equal [ "can't be blank" ], errors[:email]
+  end
+
+  def test_from_xml_with_one_error
+    errors = Project.new.errors
+
+    errors.from_xml %q(<?xml version="1.0" encoding="UTF-8"?><errors><error>Name can't be blank</error></errors>)
+
+    assert_equal [ "can't be blank" ], errors[:name]
+  end
+
+  def test_from_json
+    errors = Project.new.errors
+
+    errors.from_json %q({"errors":{"name":["can't be blank"],"email":["can't be blank"]}})
+
+    assert_equal [ "can't be blank" ], errors[:name]
+    assert_equal [ "can't be blank" ], errors[:email]
+  end
+
+  def test_from_hash
+    errors = Project.new.errors
+
+    errors.from_hash(
+      "base" => [ "has an error" ],
+      "unknown" => [ "is invalid" ],
+      "name" => [ "can't be blank" ],
+      "email" => [ "can't be blank" ]
+    )
+
+    assert_equal [ "has an error", "Unknown is invalid" ], errors[:base]
+    assert_equal [ "can't be blank" ], errors[:name]
+    assert_equal [ "can't be blank" ], errors[:email]
+  end
+
+  def test_from_array
+    errors = Project.new.errors
+
+    errors.from_array [
+      "Unknown is invalid",
+      "Base has an error",
+      "Name can't be blank",
+      "Email can't be blank"
+    ]
+
+    assert_equal [ "Unknown is invalid", "Base has an error" ], errors[:base]
+    assert_equal [ "can't be blank" ], errors[:name]
+    assert_equal [ "can't be blank" ], errors[:email]
+  end
+end