From b897313e9481656b8b18671768ef1f7e355eba40 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sun, 26 Oct 2025 09:41:23 -0400 Subject: [PATCH 1/2] 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]: https://github.com/rails/rails/pull/53886 [ActiveModel::Attributes]: https://edgeapi.rubyonrails.org/classes/ActiveModel/Attributes.html [#410]: https://github.com/rails/activeresource/pull/410 --- lib/active_resource/associations.rb | 6 +- lib/active_resource/base.rb | 33 +++++++--- test/cases/attribute_methods_test.rb | 90 ++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 test/cases/attribute_methods_test.rb diff --git a/lib/active_resource/associations.rb b/lib/active_resource/associations.rb index 7d27bba2f8..4046f1c0eb 100644 --- a/lib/active_resource/associations.rb +++ b/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 diff --git a/lib/active_resource/base.rb b/lib/active_resource/base.rb index edf92e5c25..ee5541435b 100644 --- a/lib/active_resource/base.rb +++ b/lib/active_resource/base.rb @@ -1380,12 +1380,12 @@ def persisted? # Gets the \id attribute of the resource. def id - attributes[self.class.primary_key] + read_attribute(self.class.primary_key) end # Sets the \id 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 diff --git a/test/cases/attribute_methods_test.rb b/test/cases/attribute_methods_test.rb new file mode 100644 index 0000000000..8e4483903f --- /dev/null +++ b/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 From 0389ff5589ff646603a590451e063d46444c297b Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Tue, 26 Nov 2024 15:29:58 -0500 Subject: [PATCH 2/2] Integrate with Active Model Attributes The `schema { ... }` interface pre-dates the Active Model Attributes API (defined as early as [v5.2.0][]), but clearly draws inspiration from Active Record's Database Schema and Attribute casting (which was extracted into `ActiveModel::Attributes`). However, the type information captured in `schema { ... }` blocks or assigned as `Hash` arguments to `schema=` is purely inert metadata. Proposal --- This commit aims to integrate with [ActiveModel::Model][] and [ActiveModel::Attributes][]. Through the introduction of both modules, subclasses of `ActiveResource::Schema` can benefit from type casting attributes and constructing instances with default values. This commit makes minimally incremental changes, prioritizing backwards compatibility. The reliance on `#respond_to_missing?` and `#method_missing` is left largely unchanged. Similarly, the `Schema` interface continues to provide metadata about its attributes through the `Schema#attr` method (instead of reading from `ActiveModel::Attributes#attribute_names` or `ActiveModel::Attributes.attribute_types`). API Changes --- To cast values to their specified types, declare the Schema with the `:cast_values` set to true. ```ruby class Person < ActiveResource::Base schema cast_values: true do integer 'age' end end p = Person.new p.age = "18" p.age # => 18 ``` To configure inheriting resources to cast values, set the `cast_values` class attribute: ```ruby class ApplicationResource < ActiveResource::Base self.cast_values = true end class Person < ApplicationResource schema do integer 'age' end end p = Person.new p.age = "18" p.age # => 18 ``` To set all resources application-wide to cast values, set `config.active_resource.cast_values`: ```ruby # config/application.rb config.active_resource.cast_values = true ``` [v5.2.0]: https://api.rubyonrails.org/v5.2.0/classes/ActiveModel/Attributes/ClassMethods.html [ActiveModel::Model]: https://api.rubyonrails.org/classes/ActiveModel/Model.html [ActiveModel::Attributes]: https://api.rubyonrails.org/classes/ActiveModel/Attributes/ClassMethods.html --- lib/active_resource/base.rb | 79 +++++++++++++++++--- lib/active_resource/schema.rb | 90 +++++++++++++++-------- test/cases/base/schema_test.rb | 129 +++++++++++++++++++++++++++++++++ test/cases/base_test.rb | 36 +++++++++ 4 files changed, 291 insertions(+), 43 deletions(-) diff --git a/lib/active_resource/base.rb b/lib/active_resource/base.rb index ee5541435b..3e5c3d86c5 100644 --- a/lib/active_resource/base.rb +++ b/lib/active_resource/base.rb @@ -380,6 +380,12 @@ def self.logger=(logger) class_attribute :connection_class self.connection_class = Connection + class_attribute :cast_values, instance_accessor: false, instance_predicate: false # :nodoc: + self.cast_values = false + + class_attribute :schema_definition, instance_accessor: false, instance_predicate: false # :nodoc: + self.schema_definition = Schema + class << self include ThreadsafeAttributes threadsafe_attribute :_headers, :_connection, :_user, :_password, :_bearer_token, :_site, :_proxy @@ -430,16 +436,48 @@ class << self # # Attribute-types must be one of: string, text, integer, float, decimal, datetime, timestamp, time, date, binary, boolean # - # Note: at present the attribute-type doesn't do anything, but stay - # tuned... - # Shortly it will also *cast* the value of the returned attribute. - # ie: - # j.age # => 34 # cast to an integer - # j.weight # => '65' # still a string! + # Note: By default, the attribute-type is ignored and will not cast its + # value. + # + # To cast values to their specified types, declare the Schema with the + # +:cast_values+ set to true. + # + # class Person < ActiveResource::Base + # schema cast_values: true do + # integer 'age' + # end + # end + # + # p = Person.new + # p.age = "18" + # p.age # => 18 + # + # To configure inheriting resources to cast values, set the +cast_values+ + # class attribute: # - def schema(&block) + # class ApplicationResource < ActiveResource::Base + # self.cast_values = true + # end + # + # class Person < ApplicationResource + # schema do + # integer 'age' + # end + # end + # + # p = Person.new + # p.age = "18" + # p.age # => 18 + # + # To set all resources application-wide to cast values, set + # +config.active_resource.cast_values+: + # + # # config/application.rb + # config.active_resource.cast_values = true + def schema(cast_values: self.cast_values, &block) if block_given? - schema_definition = Schema.new + self.schema_definition = Class.new(schema_definition) + schema_definition.cast_values = cast_values schema_definition.instance_eval(&block) # skip out if we didn't define anything @@ -479,6 +517,7 @@ def schema(&block) def schema=(the_schema) unless the_schema.present? # purposefully nulling out the schema + self.schema_definition = Schema @schema = nil @known_attributes = [] return @@ -1308,6 +1347,7 @@ def known_attributes def initialize(attributes = {}, persisted = false) @attributes = {}.with_indifferent_access @prefix_options = {} + @schema = self.class.schema_definition.new @persisted = persisted load(attributes, false, persisted) end @@ -1341,6 +1381,7 @@ def clone resource = self.class.new({}) resource.prefix_options = self.prefix_options resource.send :instance_variable_set, "@attributes", cloned + resource.send :instance_variable_set, "@schema", @schema.clone resource end @@ -1674,7 +1715,7 @@ def respond_to_missing?(method, include_priv = false) method_name = method.to_s if attributes.nil? super - elsif known_attributes.include?(method_name) + elsif known_attributes.include?(method_name) || @schema.respond_to?(method) true elsif method_name =~ /(?:=|\?)$/ && known_attributes.include?($`) true @@ -1685,6 +1726,10 @@ def respond_to_missing?(method, include_priv = false) end end + def serializable_hash(options = nil) + @schema.serializable_hash(options).merge!(super) + end + def to_json(options = {}) super(include_root_in_json ? { root: self.class.element_name }.merge(options) : options) end @@ -1707,14 +1752,22 @@ def read_attribute(attr_name) name = attr_name.to_s name = self.class.primary_key if name == "id" && self.class.primary_key - @attributes[name] + if @schema.respond_to?(name) + @schema.send(name) + else + @attributes[name] + end 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 + if @schema.respond_to?("#{name}=") + @schema.send("#{name}=", value) + else + attributes[name] = value + end end protected @@ -1856,7 +1909,9 @@ def split_options(options = {}) def method_missing(method_symbol, *arguments) # :nodoc: method_name = method_symbol.to_s - if method_name =~ /(=|\?)$/ + if @schema.respond_to?(method_name) + @schema.send(method_name, *arguments) + elsif method_name =~ /(=|\?)$/ case $1 when "=" write_attribute($`, arguments.first) diff --git a/lib/active_resource/schema.rb b/lib/active_resource/schema.rb index b291c7492a..c37d792f13 100644 --- a/lib/active_resource/schema.rb +++ b/lib/active_resource/schema.rb @@ -2,13 +2,26 @@ module ActiveResource # :nodoc: class Schema # :nodoc: + include ActiveModel::Model + include ActiveModel::Attributes + include ActiveModel::Serialization + # attributes can be known to be one of these types. They are easy to # cast to/from. KNOWN_ATTRIBUTE_TYPES = %w[ string text integer float decimal datetime timestamp time date binary boolean ] # An array of attribute definitions, representing the attributes that # have been defined. - attr_accessor :attrs + class_attribute :attrs, instance_accessor: false, instance_predicate: false # :nodoc: + self.attrs = {}.freeze + + class_attribute :cast_values, instance_accessor: false, instance_predicate: false # :nodoc: + self.cast_values = false + + attribute_method_suffix "?", parameters: false + + alias_method :attribute?, :send + private :attribute? # The internals of an Active Resource Schema are very simple - # unlike an Active Record TableDefinition (on which it is based). @@ -22,39 +35,54 @@ class Schema # :nodoc: # The schema stores the name and type of each attribute. That is then # read out by the schema method to populate the schema of the actual # resource. - def initialize - @attrs = {} - end - def attribute(name, type, options = {}) - raise ArgumentError, "Unknown Attribute type: #{type.inspect} for key: #{name.inspect}" unless type.nil? || Schema::KNOWN_ATTRIBUTE_TYPES.include?(type.to_s) + class << self + def inherited(subclass) + super + subclass.attrs = attrs.dup + end - the_type = type.to_s - # TODO: add defaults - # the_attr = [type.to_s] - # the_attr << options[:default] if options.has_key? :default - @attrs[name.to_s] = the_type - self - end - - # The following are the attribute types supported by Active Resource - # migrations. - KNOWN_ATTRIBUTE_TYPES.each do |attr_type| - # def string(*args) - # options = args.extract_options! - # attr_names = args + # The internals of an Active Resource Schema are very simple - + # unlike an Active Record TableDefinition (on which it is based). + # It provides a set of convenience methods for people to define their + # schema using the syntax: + # schema do + # string :foo + # integer :bar + # end # - # attr_names.each { |name| attribute(name, 'string', options) } - # end - class_eval <<-EOV, __FILE__, __LINE__ + 1 - # frozen_string_literal: true - def #{attr_type}(*args) - options = args.extract_options! - attr_names = args - - attr_names.each { |name| attribute(name, '#{attr_type}', options) } - end - EOV + # The schema stores the name and type of each attribute. That is then + # read out by the schema method to populate the schema of the actual + # resource. + def attribute(name, type = nil, options = {}) + raise ArgumentError, "Unknown Attribute type: #{type.inspect} for key: #{name.inspect}" unless type.nil? || Schema::KNOWN_ATTRIBUTE_TYPES.include?(type.to_s) + + the_type = type&.to_s + attrs[name.to_s] = the_type + + super(name, cast_values ? type.try(:to_sym) : nil, **options) + self + end + + # The following are the attribute types supported by Active Resource + # migrations. + KNOWN_ATTRIBUTE_TYPES.each do |attr_type| + # def string(*args) + # options = args.extract_options! + # attr_names = args + # + # attr_names.each { |name| attribute(name, 'string', options) } + # end + class_eval <<-EOV, __FILE__, __LINE__ + 1 + # frozen_string_literal: true + def #{attr_type}(*args) + options = args.extract_options! + attr_names = args + + attr_names.each { |name| attribute(name, '#{attr_type}', options) } + end + EOV + end end end end diff --git a/test/cases/base/schema_test.rb b/test/cases/base/schema_test.rb index 931ccd4190..a27bf99de5 100644 --- a/test/cases/base/schema_test.rb +++ b/test/cases/base/schema_test.rb @@ -15,6 +15,7 @@ def setup def teardown Person.schema = nil # hack to stop test bleedthrough... + Person.cast_values = false # hack to stop test bleedthrough... end @@ -160,6 +161,51 @@ def teardown } end + test "classes can alias attributes for a schema they inherit from their ancestors" do + base = Class.new(ActiveResource::Base) do + schema { attribute :base_attribute } + end + person = Class.new(base) do + schema { alias_attribute :aliased_attribute, :base_attribute } + end + + resource = person.new + + assert_changes -> { resource.base_attribute }, to: "value" do + resource.aliased_attribute = "value" + end + assert_equal [ "base_attribute" ], resource.attribute_names + assert_equal "value", resource.serializable_hash["base_attribute"] + assert_not_includes resource.serializable_hash, "aliased_attribute" + end + + test "classes can extend the schema they inherit from their ancestors" do + base = Class.new(ActiveResource::Base) do + schema { attribute :created_at, :datetime } + end + cast_values = Class.new(base) do + schema(cast_values: true) { attribute :accepted_terms_and_conditions, :boolean } + end + uncast_values = Class.new(base) do + schema(cast_values: false) { attribute :line1, :string } + end + + cast_resource = cast_values.new + uncast_resource = uncast_values.new + + assert_changes -> { cast_resource.accepted_terms_and_conditions }, to: true do + cast_resource.accepted_terms_and_conditions = "1" + end + assert_changes -> { cast_resource.created_at.try(:to_date) }, from: nil, to: Date.new(2025, 1, 1) do + cast_resource.created_at = "2025-01-01" + end + assert_changes -> { uncast_resource.line1 }, to: 123 do + uncast_resource.line1 = 123 + end + assert_changes -> { uncast_resource.created_at }, from: nil, to: "2025-01-01" do + uncast_resource.created_at = "2025-01-01" + end + end ##################################################### # Using the schema syntax @@ -425,4 +471,87 @@ def teardown Person.schema = new_schema assert_equal Person.new(age: 20, name: "Matz").known_attributes, [ "age", "name" ] end + + test "clone with schema that casts values" do + Person.cast_values = true + Person.schema = { "age" => "integer" } + person = Person.new({ Person.primary_key => 1, "age" => "10" }, true) + + person_c = person.clone + + assert_predicate person_c, :new? + assert_nil person_c.send(Person.primary_key) + assert_equal 10, person_c.age + end + + test "known primary_key attributes should be cast" do + Person.schema cast_values: true do + attribute Person.primary_key, :integer + end + + person = Person.new(Person.primary_key => "1") + + assert_equal 1, person.send(Person.primary_key) + end + + test "cast_values: true supports implicit types" do + Person.schema cast_values: true do + attribute :name + end + + person = Person.new(name: "String") + + assert_equal "String", person.name + end + + test "known attributes should be cast" do + Person.schema cast_values: true do + attribute :born_on, :date + end + + person = Person.new(born_on: "2000-01-01") + + assert_equal Date.new(2000, 1, 1), person.born_on + end + + test "known boolean attributes should be cast as predicates" do + Person.schema cast_values: true do + attribute :alive, :boolean + end + + assert_predicate Person.new(alive: "1"), :alive? + assert_predicate Person.new(alive: "true"), :alive? + assert_predicate Person.new(alive: true), :alive? + assert_not_predicate Person.new, :alive? + assert_not_predicate Person.new(alive: nil), :alive? + assert_not_predicate Person.new(alive: "0"), :alive? + assert_not_predicate Person.new(alive: "false"), :alive? + assert_not_predicate Person.new(alive: false), :alive? + end + + test "known attributes should be support default values" do + Person.schema cast_values: true do + attribute :name, :string, default: "Default Name" + end + + person = Person.new + + assert_equal "Default Name", person.name + end + + test "unknown attributes should not be cast" do + Person.cast_values = true + + person = Person.new(age: "10") + + assert_equal "10", person.age + end + + test "unknown attribute type raises ArgumentError" do + assert_raises ArgumentError, match: /Unknown Attribute type: :junk/ do + Person.schema cast_values: true do + attribute :name, :junk + end + end + end end diff --git a/test/cases/base_test.rb b/test/cases/base_test.rb index 462920ac92..a28272d2b5 100644 --- a/test/cases/base_test.rb +++ b/test/cases/base_test.rb @@ -1485,6 +1485,42 @@ def test_exists_with_204_no_content assert Person.exists?(1) end + def test_serializable_hash + Person.schema do + attribute :name, :string + attribute :likes_hats, :boolean + end + resource = Person.new(id: 1, name: "Joe", likes_hats: true, non_attribute_field: "foo") + + serializable_hash = resource.serializable_hash + + assert_equal [ "id", "name", "likes_hats", "non_attribute_field" ].sort, serializable_hash.keys.sort + assert_equal 1, serializable_hash["id"] + assert_equal "Joe", serializable_hash["name"] + assert_equal true, serializable_hash["likes_hats"] + assert_equal "foo", serializable_hash["non_attribute_field"] + ensure + Person.schema = nil + end + + def test_serializable_hash_with_options + Person.schema do + attribute :name, :string + attribute :likes_hats, :boolean + end + resource = Person.new(id: 1, name: "Joe", likes_hats: true, non_attribute_field: "foo") + + serializable_hash = resource.serializable_hash(only: [ :id, :name, :non_attribute_field ]) + + assert_equal [ "id", "name", "non_attribute_field" ].sort, serializable_hash.keys.sort + assert_equal 1, serializable_hash["id"] + assert_equal "Joe", serializable_hash["name"] + assert_equal "foo", serializable_hash["non_attribute_field"] + assert_nil serializable_hash["likes_hats"] + ensure + Person.schema = nil + end + def test_read_attribute_for_serialization joe = Person.find(6) joe.singleton_class.class_eval do