Base: implement read_attribute and write_attribute

Replace all access of `@attributes` and `attributes` key-value pairs
with calls to `read_attribute` and `write_attribute`.

The `read_attribute` and `write_attribute` implementations draw
inspiration from [ActiveRecord::AttributeMethods::Read][] and
[ActiveRecord::AttributeMethods::Write][], respectively.

[rails/rails#53886][] proposes implementing each method at the Active
Model layer. While that proposal is considered, this commit implements
each method in terms of accessing the underlying `@attributes` hash
instance. This change is also in support of a first-party integration
with [ActiveModel::Attributes][] proposed in [#410][], and aims to be
compatible with its attribute reading and writing interfaces.

```ruby
person = Person.find(1)
person.read_attribute("name")           # => "Matz"
person.name                             # => "Matz"

person.write_attribute("name", "matz")
person.name                             # => "matz"
```

[ActiveRecord::AttributeMethods::Read]: https://edgeapi.rubyonrails.org/classes/ActiveRecord/AttributeMethods/Read.html#method-i-read_attribute
[ActiveRecord::AttributeMethods::Write]: https://edgeapi.rubyonrails.org/classes/ActiveRecord/AttributeMethods/Write.html#method-i-write_attribute
[rails/rails#53886]: rails/rails#53886
[ActiveModel::Attributes]: https://edgeapi.rubyonrails.org/classes/ActiveModel/Attributes.html
[#410]: #410

Author: seanpdoyle

lib/active_resource/associations.rb

@@ -131,7 +131,7 @@ def defines_belongs_to_finder_method(reflection)
       if instance_variable_defined?(ivar_name)
         instance_variable_get(ivar_name)
       elsif attributes.include?(method_name)
-        attributes[method_name]
+        read_attribute(method_name)
       elsif association_id = send(reflection.foreign_key)
         instance_variable_set(ivar_name, reflection.klass.find(association_id))
       end
@@ -146,7 +146,7 @@ def defines_has_many_finder_method(reflection)
       if instance_variable_defined?(ivar_name)
         instance_variable_get(ivar_name)
       elsif attributes.include?(method_name)
-        attributes[method_name]
+        read_attribute(method_name)
       elsif !new_record?
         instance_variable_set(ivar_name, reflection.klass.find(:all, params: { "#{self.class.element_name}_id": self.id }))
       else
@@ -164,7 +164,7 @@ def defines_has_one_finder_method(reflection)
       if instance_variable_defined?(ivar_name)
         instance_variable_get(ivar_name)
       elsif attributes.include?(method_name)
-        attributes[method_name]
+        read_attribute(method_name)
       elsif reflection.klass.respond_to?(:singleton_name)
         instance_variable_set(ivar_name, reflection.klass.find(params: { "#{self.class.element_name}_id": self.id }))
       else

lib/active_resource/base.rb

@@ -1380,12 +1380,12 @@ def persisted?
 
     # Gets the <tt>\id</tt> attribute of the resource.
     def id
-      attributes[self.class.primary_key]
+      read_attribute(self.class.primary_key)
     end
 
     # Sets the <tt>\id</tt> attribute of the resource.
     def id=(id)
-      attributes[self.class.primary_key] = id
+      write_attribute(self.class.primary_key, id)
     end
 
     # Test for equality. Resource are equal if and only if +other+ is the same object or
@@ -1596,7 +1596,7 @@ def load(attributes, remove_root = false, persisted = false)
       attributes = Formats.remove_root(attributes) if remove_root
 
       attributes.each do |key, value|
-        @attributes[key.to_s] =
+        write_attribute(key.to_s,
           case value
           when Array
             resource = nil
@@ -1614,6 +1614,7 @@ def load(attributes, remove_root = false, persisted = false)
           else
             value.duplicable? ? value.dup : value
           end
+        )
       end
       self
     end
@@ -1693,13 +1694,29 @@ def to_xml(options = {})
     end
 
     def read_attribute_for_serialization(n)
-      if !attributes[n].nil?
-        attributes[n]
+      value = read_attribute(n)
+
+      if !value.nil?
+        value
       elsif respond_to?(n)
         send(n)
       end
     end
 
+    def read_attribute(attr_name)
+      name = attr_name.to_s
+
+      name = self.class.primary_key if name == "id" && self.class.primary_key
+      @attributes[name]
+    end
+
+    def write_attribute(attr_name, value)
+      name = attr_name.to_s
+
+      name = self.class.primary_key if name == "id" && self.class.primary_key
+      @attributes[name] = value
+    end
+
     protected
       def connection(refresh = false)
         self.class.connection(refresh)
@@ -1842,12 +1859,12 @@ def method_missing(method_symbol, *arguments) # :nodoc:
         if method_name =~ /(=|\?)$/
           case $1
           when "="
-            attributes[$`] = arguments.first
+            write_attribute($`, arguments.first)
           when "?"
-            attributes[$`]
+            read_attribute($`)
           end
         else
-          return attributes[method_name] if attributes.include?(method_name)
+          return read_attribute(method_name) if attributes.include?(method_name)
           # not set right now but we know about it
           return nil if known_attributes.include?(method_name)
           super

test/cases/attribute_methods_test.rb

@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+require "abstract_unit"
+require "fixtures/person"
+
+class AttributeMethodsTest < ActiveSupport::TestCase
+  setup :setup_response
+
+  test "write_attribute string" do
+    matz = Person.find(1)
+
+    assert_changes -> { matz.name }, to: "matz" do
+      matz.write_attribute("name", "matz")
+    end
+  end
+
+  test "write_attribute symbol" do
+    matz = Person.find(1)
+
+    assert_changes -> { matz.name }, to: "matz" do
+      matz.write_attribute(:name, "matz")
+    end
+  end
+
+  test "write_attribute id" do
+    matz = Person.find(1)
+
+    assert_changes -> { matz.id }, from: 1, to: "2" do
+      matz.write_attribute(:id, "2")
+    end
+  end
+
+  test "write_attribute primary key" do
+    previous_primary_key = Person.primary_key
+    Person.primary_key = "pk"
+    matz = Person.find(1)
+
+    assert_changes -> { matz.id }, from: 1, to: "2" do
+      matz.write_attribute(:id, "2")
+    end
+    assert_changes -> { matz.id }, from: "2", to: 1 do
+      matz.write_attribute("pk", 1)
+    end
+    assert_changes -> { matz.id }, from: 1, to: "2" do
+      matz.id = "2"
+    end
+  ensure
+    Person.primary_key = previous_primary_key
+  end
+
+  test "write_attribute an unknown attribute" do
+    person = Person.new
+
+    person.write_attribute("unknown", true)
+
+    assert_predicate person, :unknown
+  end
+
+  test "read_attribute" do
+    matz = Person.find(1)
+
+    assert_equal "Matz", matz.read_attribute("name")
+    assert_equal "Matz", matz.read_attribute(:name)
+  end
+
+  test "read_attribute id" do
+    matz = Person.find(1)
+
+    assert_equal 1, matz.read_attribute("id")
+    assert_equal 1, matz.read_attribute(:id)
+  end
+
+  test "read_attribute primary key" do
+    previous_primary_key = Person.primary_key
+    Person.primary_key = "pk"
+    matz = Person.find(1)
+
+    assert_equal 1, matz.id
+    assert_equal 1, matz.read_attribute("pk")
+    assert_equal 1, matz.read_attribute(:pk)
+  ensure
+    Person.primary_key = previous_primary_key
+  end
+
+  test "read_attribute unknown attribute" do
+    person = Person.new
+
+    person.read_attribute("unknown")
+  end
+end