From ccb2393979e736e8bd07612c95654e7dc7e79479 Mon Sep 17 00:00:00 2001 From: Neil Carvalho Date: Tue, 23 Aug 2022 15:30:57 -0300 Subject: [PATCH] Clarify `composed_of` allows a Hash for `mapping` [`composed_of`](https://api.rubyonrails.org/classes/ActiveRecord/Aggregations/ClassMethods.html#method-i-composed_of) is a feature that is not widely used and its API is somewhat confusing, especially for beginners. It was even deprecated for a while, but 10 years after its deprecation, it's still here. The Rails documentation includes these examples including mapping: ```ruby composed_of :temperature, mapping: %w(reading celsius) composed_of :balance, class_name: "Money", mapping: %w(balance amount) composed_of :address, mapping: [ %w(address_street street), %w(address_city city) ] ``` Hashes are accepted kind-of accidentally for the `mapping` option. Using a hash, instead of an array or array of arrays makes the documentation more beginner-friendly. ```ruby composed_of :temperature, mapping: { reading: :celsius } composed_of :balance, class_name: "Money", mapping: { balance: :amount } composed_of :address, mapping: { address_street: :street, address_city: :city } ``` Before Ruby 1.9, looping through a hash didn't have deterministic order, and the mapping order is important, as that's the same order Rails uses when initializing the value object. Since Ruby 1.9, this isn't an issue anymore, so we can change the documentation to use hashes instead. This commit changes the documentation for `composed_of`, clarifying that any key-value format (both a `Hash` and an `Array`) are accepted for the `mapping` option. It also adds tests to ensure hashes are also accepted. --- .../lib/active_record/aggregations.rb | 19 ++++++++++--------- activerecord/test/cases/aggregations_test.rb | 14 ++++++++++++++ activerecord/test/models/customer.rb | 2 ++ 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index 8c789cc6bf..74a9541664 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/activerecord/lib/active_record/aggregations.rb @@ -32,8 +32,8 @@ module ActiveRecord # the database). # # class Customer < ActiveRecord::Base - # composed_of :balance, class_name: "Money", mapping: %w(balance amount) - # composed_of :address, mapping: [ %w(address_street street), %w(address_city city) ] + # composed_of :balance, class_name: "Money", mapping: { balance: :amount } + # composed_of :address, mapping: { address_street: :street, address_city: :city } # end # # The customer class now has the following methods to manipulate the value objects: @@ -150,7 +150,7 @@ module ActiveRecord # class NetworkResource < ActiveRecord::Base # composed_of :cidr, # class_name: 'NetAddr::CIDR', - # mapping: [ %w(network_address network), %w(cidr_range bits) ], + # mapping: { network_address: :network, cidr_range: :bits }, # allow_nil: true, # constructor: Proc.new { |network_address, cidr_range| NetAddr::CIDR.create("#{network_address}/#{cidr_range}") }, # converter: Proc.new { |value| NetAddr::CIDR.create(value.is_a?(Array) ? value.join('/') : value) } @@ -188,10 +188,10 @@ module ActiveRecord # to the Address class, but if the real class name is +CompanyAddress+, you'll have to specify it # with this option. # * :mapping - Specifies the mapping of entity attributes to attributes of the value - # object. Each mapping is represented as an array where the first item is the name of the - # entity attribute and the second item is the name of the attribute in the value object. The + # object. Each mapping is represented as a key-value pair where the key is the name of the + # entity attribute and the value is the name of the attribute in the value object. The # order in which mappings are defined determines the order in which attributes are sent to the - # value class constructor. + # value class constructor. The mapping can be written as a hash or as an array of pairs. # * :allow_nil - Specifies that the value object will not be instantiated when all mapped # attributes are +nil+. Setting the value object to +nil+ has the effect of writing +nil+ to all # mapped attributes. @@ -208,14 +208,15 @@ module ActiveRecord # can return +nil+ to skip the assignment. # # Option examples: - # composed_of :temperature, mapping: %w(reading celsius) - # composed_of :balance, class_name: "Money", mapping: %w(balance amount) + # composed_of :temperature, mapping: { reading: :celsius } + # composed_of :balance, class_name: "Money", mapping: { balance: :amount } + # composed_of :address, mapping: { address_street: :street, address_city: :city } # composed_of :address, mapping: [ %w(address_street street), %w(address_city city) ] # composed_of :gps_location # composed_of :gps_location, allow_nil: true # composed_of :ip_address, # class_name: 'IPAddr', - # mapping: %w(ip to_i), + # mapping: { ip: :to_i }, # constructor: Proc.new { |ip| IPAddr.new(ip, Socket::AF_INET) }, # converter: Proc.new { |ip| ip.is_a?(Integer) ? IPAddr.new(ip, Socket::AF_INET) : IPAddr.new(ip.to_s) } # diff --git a/activerecord/test/cases/aggregations_test.rb b/activerecord/test/cases/aggregations_test.rb index d270175af4..d32f3e12ba 100644 --- a/activerecord/test/cases/aggregations_test.rb +++ b/activerecord/test/cases/aggregations_test.rb @@ -150,6 +150,20 @@ class AggregationsTest < ActiveRecord::TestCase customers(:barney).fullname_no_converter = { first: "Barney", last: "Stinson" } assert_equal({ first: "Barney", last: "Stinson" }.to_s, customers(:barney).name) end + + def test_hash_mapping + assert_equal "Quiet Road", customers(:barney).address_hash_mapping.street + assert_equal "Peaceful Town", customers(:barney).address_hash_mapping.city + assert_equal "Tranquil Land", customers(:barney).address_hash_mapping.country + end + + def test_value_object_with_hash_mapping_assignment_changes_model_attributes + customers(:barney).address_hash_mapping = Address.new("Lively Street", customers(:barney).address_city, customers(:barney).address_country) + + customers(:barney).save + + assert_equal "Lively Street", customers(:barney).address_street + end end class OverridingAggregationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/models/customer.rb b/activerecord/test/models/customer.rb index 1b500eb916..cabba7239e 100644 --- a/activerecord/test/models/customer.rb +++ b/activerecord/test/models/customer.rb @@ -4,6 +4,8 @@ class Customer < ActiveRecord::Base cattr_accessor :gps_conversion_was_run composed_of :address, mapping: [ %w(address_street street), %w(address_city city), %w(address_country country) ], allow_nil: true + composed_of :address_hash_mapping, class_name: "Address", + mapping: { address_street: :street, address_city: :city, address_country: :country }, allow_nil: true composed_of :balance, class_name: "Money", mapping: %i(balance amount) composed_of :gps_location, allow_nil: true composed_of :non_blank_gps_location, class_name: "GpsLocation", allow_nil: true, mapping: %w(gps_location gps_location),