Deprecate writing to Base#attributes

The Base class defines an `attr_accessor :attributes` that is marked
with `# :nodoc:`. Technically, this means that any interaction with
`Base#attributes` or `Base#attributes=` is not part of the public
interface, and is free to be changed or removed without breaking the
public API.

However, given the project's age and the long-term period of time with
minimal changes to the public interface, this PR proposes that there be
a public deprecation of **writing** to the Hash instance returned by the
`Base#attributes` method.

The migration to `ActiveModel::Attributes` proposed in [rails#410][] will
involve changes to the `Base#attributes` method (due to
[ActiveModel::Attributes#attributes][]). Reading from the value returned
by `ActiveModel::Attributes#attributes` will remain the same, but
writing to that value will have no affect since the value is a Hash copy
created by [ActiveModel::AttributeSet#to_hash][], and not the Hash
instance used internally.

Similarly, `ActiveModel::Attributes` does not expose a corresponding
`#attributes=` method. By deprecating `#attributes=`, changes made that
incorporate `ActiveModel::Attributes` will not need to add an otherwise
unnecessary `#attributes=` implementation.

Once deprecated and part of a release cycle, the
`ActiveResource::AttributeSet` class can be removed, and the Active
Model migration's backwards compatibility burden can be reduced.

[rails#410]: rails#410
[ActiveModel::Attributes#attributes]: https://edgeapi.rubyonrails.org/classes/ActiveModel/Attributes.html#method-i-attributes
[ActiveModel::AttributeSet#to_hash]: https://github.com/rails/rails/blob/v8.1.1/activemodel/lib/active_model/attribute_set.rb#L36-L39

Author: seanpdoyle

lib/active_resource.rb

@@ -35,6 +35,7 @@ module ActiveResource
 
   URI_PARSER = defined?(URI::RFC2396_PARSER) ? URI::RFC2396_PARSER : URI::RFC2396_Parser.new
 
+  autoload :AttributeSet
   autoload :Base
   autoload :Callbacks
   autoload :Coder

lib/active_resource/attribute_set.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module ActiveResource
+  class AttributeSet < DelegateClass(Hash) # :nodoc:
+    MESSAGE = "Writing to the attributes hash is deprecated. Set attributes directly on the instance instead."
+
+    deprecate(**[ :[]=, :store, :update, :merge! ].index_with(MESSAGE),
+      deprecator: ActiveResource.deprecator)
+
+    delegate :is_a?, to: :__getobj__
+  end
+end

lib/active_resource/base.rb

@@ -1276,9 +1276,17 @@ def split_options(options = {})
         end
     end
 
-    attr_accessor :attributes # :nodoc:
     attr_accessor :prefix_options # :nodoc:
 
+    def attributes=(value) # :nodoc:
+      ActiveResource.deprecator.warn("#attributes= is deprecated. Call #load on the instance instead.")
+      @attributes = value
+    end
+
+    def attributes # :nodoc:
+      AttributeSet.new(@attributes)
+    end
+
     # If no schema has been defined for the class (see
     # <tt>ActiveResource::schema=</tt>), the default automatic schema is
     # generated from the current instance's attributes
@@ -1385,7 +1393,7 @@ def id
 
     # Sets the <tt>\id</tt> attribute of the resource.
     def id=(id)
-      attributes[self.class.primary_key] = id
+      @attributes[self.class.primary_key] = id
     end
 
     # Test for equality. Resource are equal if and only if +other+ is the same object or
@@ -1439,7 +1447,7 @@ def hash
     #   next_invoice.customer # => That Company
     def dup
       self.class.new.tap do |resource|
-        resource.attributes     = @attributes
+        resource.send :instance_variable_set, "@attributes", @attributes
         resource.prefix_options = @prefix_options
       end
     end
@@ -1842,7 +1850,7 @@ def method_missing(method_symbol, *arguments) # :nodoc:
         if method_name =~ /(=|\?)$/
           case $1
           when "="
-            attributes[$`] = arguments.first
+            @attributes[$`] = arguments.first
           when "?"
             attributes[$`]
           end

test/cases/base_test.rb

@@ -1743,4 +1743,38 @@ def test_paths_without_format
   ensure
     ActiveResource::Base.include_format_in_path = true
   end
+
+  def test_deprecate_attributes_write
+    person = Person.find(1)
+
+    assert_deprecated("#attributes= is deprecated. Call #load on the instance instead.", ActiveResource.deprecator) do
+      person.attributes = { "name" => "changed" }
+    end
+
+    assert_equal "changed", person.name
+  end
+
+  def test_deprecate_attributes_store
+    [ :[]=, :store ].each do |store|
+      person = Person.find(1)
+
+      assert_deprecated("Writing to the attributes hash is deprecated. Set attributes directly on the instance instead.", ActiveResource.deprecator) do
+        person.attributes.send(store, "name", "changed")
+      end
+
+      assert_equal "changed", person.name
+    end
+  end
+
+  def test_deprecate_attributes_update
+    [ :update, :merge! ].each do |update|
+      person = Person.find(1)
+
+      assert_deprecated("Writing to the attributes hash is deprecated. Set attributes directly on the instance instead.", ActiveResource.deprecator) do
+        person.attributes.send(update, "name" => "changed")
+      end
+
+      assert_equal "changed", person.name
+    end
+  end
 end