Skip to content

Commit b2bc3ad

Browse files
committed
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 [#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. Once deprecated and part of a release cycle, the `ActiveResource::ReadOnlyHash` class can be removed, and the Active Model migration's backwards compatibility burden can be reduced. [#410]: #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
1 parent e577c6d commit b2bc3ad

File tree

4 files changed

+55
-3
lines changed

4 files changed

+55
-3
lines changed

lib/active_resource.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ module ActiveResource
4242
autoload :CustomMethods
4343
autoload :Formats
4444
autoload :HttpMock
45+
autoload :ReadOnlyHash
4546
autoload :Rescuable
4647
autoload :Schema
4748
autoload :Serialization

lib/active_resource/base.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,9 +1276,13 @@ def split_options(options = {})
12761276
end
12771277
end
12781278

1279-
attr_accessor :attributes # :nodoc:
1279+
attr_writer :attributes # :nodoc:
12801280
attr_accessor :prefix_options # :nodoc:
12811281

1282+
def attributes # :nodoc:
1283+
ReadOnlyHash.new(@attributes)
1284+
end
1285+
12821286
# If no schema has been defined for the class (see
12831287
# <tt>ActiveResource::schema=</tt>), the default automatic schema is
12841288
# generated from the current instance's attributes
@@ -1385,7 +1389,7 @@ def id
13851389

13861390
# Sets the <tt>\id</tt> attribute of the resource.
13871391
def id=(id)
1388-
attributes[self.class.primary_key] = id
1392+
@attributes[self.class.primary_key] = id
13891393
end
13901394

13911395
# Test for equality. Resource are equal if and only if +other+ is the same object or
@@ -1842,7 +1846,7 @@ def method_missing(method_symbol, *arguments) # :nodoc:
18421846
if method_name =~ /(=|\?)$/
18431847
case $1
18441848
when "="
1845-
attributes[$`] = arguments.first
1849+
@attributes[$`] = arguments.first
18461850
when "?"
18471851
attributes[$`]
18481852
end
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# frozen_string_literal: true
2+
3+
module ActiveResource
4+
class ReadOnlyHash < DelegateClass(Hash) # :nodoc:
5+
MESSAGE = "Writing the attributes hash is deprecated. Set attributes directly on the instance instead."
6+
7+
def []=(key, value)
8+
ActiveResource.deprecator.warn(MESSAGE)
9+
super
10+
end
11+
alias_method :store, :[]=
12+
13+
def update(*other_hashes)
14+
ActiveResource.deprecator.warn(MESSAGE)
15+
super
16+
end
17+
alias_method :merge!, :update
18+
19+
def is_a?(other)
20+
__getobj__.is_a?(other)
21+
end
22+
end
23+
end

test/cases/base_test.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1743,4 +1743,28 @@ def test_paths_without_format
17431743
ensure
17441744
ActiveResource::Base.include_format_in_path = true
17451745
end
1746+
1747+
def test_deprecate_attributes_store
1748+
[ :[]=, :store ].each do |store|
1749+
person = Person.find(1)
1750+
1751+
assert_deprecated("Writing the attributes hash is deprecated. Set attributes directly on the instance instead.", ActiveResource.deprecator) do
1752+
person.attributes.send(store, "name", "changed")
1753+
end
1754+
1755+
assert_equal "changed", person.name
1756+
end
1757+
end
1758+
1759+
def test_deprecate_attributes_update
1760+
[ :update, :merge! ].each do |update|
1761+
person = Person.find(1)
1762+
1763+
assert_deprecated("Writing the attributes hash is deprecated. Set attributes directly on the instance instead.", ActiveResource.deprecator) do
1764+
person.attributes.send(update, "name" => "changed")
1765+
end
1766+
1767+
assert_equal "changed", person.name
1768+
end
1769+
end
17461770
end

0 commit comments

Comments
 (0)