From 57bccb11627b39b1e1f2822add48a5765b967313 Mon Sep 17 00:00:00 2001 From: Rob Anderton Date: Mon, 18 Aug 2008 19:13:01 +0100 Subject: [PATCH] Added :constructor and :converter options to composed_of and deprecated the conversion block --- activerecord/lib/active_record/aggregations.rb | 91 ++++++++++++++++-------- activerecord/test/cases/aggregations_test.rb | 35 +++++++++ activerecord/test/fixtures/customers.yml | 11 +++- activerecord/test/models/customer.rb | 20 +++++- 4 files changed, 124 insertions(+), 33 deletions(-) diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index a5d3a50..f3d7359 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/activerecord/lib/active_record/aggregations.rb @@ -10,10 +10,10 @@ module ActiveRecord end unless self.new_record? end - # Active Record implements aggregation through a macro-like class method called +composed_of+ for representing attributes + # Active Record implements aggregation through a macro-like class method called +composed_of+ for representing attributes # as value objects. It expresses relationships like "Account [is] composed of Money [among other things]" or "Person [is] - # composed of [an] address". Each call to the macro adds a description of how the value objects are created from the - # attributes of the entity object (when the entity is initialized either as a new object or from finding an existing object) + # composed of [an] address". Each call to the macro adds a description of how the value objects are created from the + # attributes of the entity object (when the entity is initialized either as a new object or from finding an existing object) # and how it can be turned back into attributes (when the entity is saved to the database). Example: # # class Customer < ActiveRecord::Base @@ -30,10 +30,10 @@ module ActiveRecord # class Money # include Comparable # attr_reader :amount, :currency - # EXCHANGE_RATES = { "USD_TO_DKK" => 6 } - # - # def initialize(amount, currency = "USD") - # @amount, @currency = amount, currency + # EXCHANGE_RATES = { "USD_TO_DKK" => 6 } + # + # def initialize(amount, currency = "USD") + # @amount, @currency = amount, currency # end # # def exchange_to(other_currency) @@ -56,19 +56,19 @@ module ActiveRecord # # class Address # attr_reader :street, :city - # def initialize(street, city) - # @street, @city = street, city + # def initialize(street, city) + # @street, @city = street, city # end # - # def close_to?(other_address) - # city == other_address.city + # def close_to?(other_address) + # city == other_address.city # end # # def ==(other_address) # city == other_address.city && street == other_address.street # end # end - # + # # Now it's possible to access attributes from the database through the value objects instead. If you choose to name the # composition the same as the attribute's name, it will be the only way to access that attribute. That's the case with our # +balance+ attribute. You interact with the value objects just like you would any other attribute, though: @@ -87,8 +87,8 @@ module ActiveRecord # customer.address_city = "Copenhagen" # customer.address # => Address.new("Hyancintvej", "Copenhagen") # customer.address = Address.new("May Street", "Chicago") - # customer.address_street # => "May Street" - # customer.address_city # => "Chicago" + # customer.address_street # => "May Street" + # customer.address_city # => "Chicago" # # == Writing value objects # @@ -103,9 +103,9 @@ module ActiveRecord # returns a new value object instead of changing its own values. Active Record won't persist value objects that have been # changed through means other than the writer method. # - # The immutable requirement is enforced by Active Record by freezing any object assigned as a value object. Attempting to + # The immutable requirement is enforced by Active Record by freezing any object assigned as a value object. Attempting to # change it afterwards will result in a ActiveSupport::FrozenObjectError. - # + # # Read more about value objects on http://c2.com/cgi/wiki?ValueObject and on the dangers of not keeping value objects # immutable on http://c2.com/cgi/wiki?ValueObjectsShouldBeImmutable # @@ -130,39 +130,59 @@ module ActiveRecord # * :allow_nil - specifies that the aggregate object will not be instantiated when all mapped # attributes are +nil+. Setting the aggregate class to +nil+ has the effect of writing +nil+ to all mapped attributes. # This defaults to +false+. - # - # An optional block can be passed to convert the argument that is passed to the writer method into an instance of - # :class_name. The block will only be called if the argument is not already an instance of :class_name. + # * :constructor - a symbol specifying the name of the constructor method or a Proc that will be used to convert the + # attributes that are mapped to the aggregation to instantiate a :class_name object. The default is +:new+. + # * :converter - a symbol specifying the name of a class method of :class_name or a Proc that will be used to convert + # the argument that is passed to the writer method into an instance of :class_name. The converter will only be called + # if the argument is not already an instance of :class_name. # # Option examples: # composed_of :temperature, :mapping => %w(reading celsius) - # composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) {|balance| balance.to_money } + # composed_of :balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| balance.to_money } # 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), + # :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) } # def composed_of(part_id, options = {}, &block) - options.assert_valid_keys(:class_name, :mapping, :allow_nil) + options.assert_valid_keys(:class_name, :mapping, :allow_nil, :constructor, :converter) name = part_id.id2name - class_name = options[:class_name] || name.camelize - mapping = options[:mapping] || [ name, name ] + class_name = options[:class_name] || name.camelize + mapping = options[:mapping] || [ name, name ] mapping = [ mapping ] unless mapping.first.is_a?(Array) - allow_nil = options[:allow_nil] || false + allow_nil = options[:allow_nil] || false + constructor = options[:constructor] || :new + converter = options[:converter] || block + + ActiveSupport::Deprecation.warn('The conversion block has been deprecated, use the :converter option instead.', caller) if block_given? + + reader_method(name, class_name, mapping, allow_nil, constructor) + writer_method(name, class_name, mapping, allow_nil, converter) - reader_method(name, class_name, mapping, allow_nil) - writer_method(name, class_name, mapping, allow_nil, block) - create_reflection(:composed_of, part_id, options, self) end private - def reader_method(name, class_name, mapping, allow_nil) + def reader_method(name, class_name, mapping, allow_nil, constructor) module_eval do define_method(name) do |*args| force_reload = args.first || false if (instance_variable_get("@#{name}").nil? || force_reload) && (!allow_nil || mapping.any? {|pair| !read_attribute(pair.first).nil? }) - instance_variable_set("@#{name}", class_name.constantize.new(*mapping.collect {|pair| read_attribute(pair.first)})) + attrs = mapping.collect {|pair| read_attribute(pair.first)} + object = case constructor + when Symbol + class_name.constantize.send(constructor, *attrs) + when Proc, Method + constructor.call(*attrs) + else + raise ArgumentError, 'Constructor must be a symbol denoting the constructor method to call or a Proc to be invoked.' + end + instance_variable_set("@#{name}", object) end instance_variable_get("@#{name}") end @@ -170,14 +190,23 @@ module ActiveRecord end - def writer_method(name, class_name, mapping, allow_nil, conversion) + def writer_method(name, class_name, mapping, allow_nil, converter) module_eval do define_method("#{name}=") do |part| if part.nil? && allow_nil mapping.each { |pair| self[pair.first] = nil } instance_variable_set("@#{name}", nil) else - part = conversion.call(part) unless part.is_a?(class_name.constantize) || conversion.nil? + unless part.is_a?(class_name.constantize) || converter.nil? + part = case converter + when Symbol + class_name.constantize.send(converter, part) + when Proc, Method + converter.call(part) + else + raise ArgumentError, 'Converter must be a symbol denoting the converter method to call or a Proc to be invoked.' + end + end mapping.each { |pair| self[pair.first] = part.send(pair.last) } instance_variable_set("@#{name}", part.freeze) end diff --git a/activerecord/test/cases/aggregations_test.rb b/activerecord/test/cases/aggregations_test.rb index 75d1f27..b6656c8 100644 --- a/activerecord/test/cases/aggregations_test.rb +++ b/activerecord/test/cases/aggregations_test.rb @@ -107,6 +107,41 @@ class AggregationsTest < ActiveRecord::TestCase customers(:david).gps_location = nil assert_equal nil, customers(:david).gps_location end + + def test_custom_constructor + assert_equal 'Barney GUMBLE', customers(:barney).fullname.to_s + assert_kind_of Fullname, customers(:barney).fullname + end + + def test_custom_converter + customers(:barney).fullname = 'Barnoit Gumbleau' + assert_equal 'Barnoit GUMBLEAU', customers(:barney).fullname.to_s + assert_kind_of Fullname, customers(:barney).fullname + end +end + +class DeprecatedAggregationsTest < ActiveRecord::TestCase + class Person < ActiveRecord::Base; end + + def test_conversion_block_is_deprecated + assert_deprecated 'conversion block has been deprecated' do + Person.composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money } + end + end + + def test_conversion_block_used_when_converter_option_is_nil + Person.composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money } + assert_raise(NoMethodError) { Person.new.balance = 5 } + end + + def test_converter_option_overrides_conversion_block + Person.composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| Money.new(balance) }) { |balance| balance.to_money } + + person = Person.new + assert_nothing_raised { person.balance = 5 } + assert_equal 5, person.balance.amount + assert_kind_of Money, person.balance + end end class OverridingAggregationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/fixtures/customers.yml b/activerecord/test/fixtures/customers.yml index f802aac..0399ff8 100644 --- a/activerecord/test/fixtures/customers.yml +++ b/activerecord/test/fixtures/customers.yml @@ -6,7 +6,7 @@ david: address_city: Scary Town address_country: Loony Land gps_location: 35.544623640962634x-105.9309951055148 - + zaphod: id: 2 name: Zaphod @@ -14,4 +14,13 @@ zaphod: address_street: Avenue Road address_city: Hamlet Town address_country: Nation Land + gps_location: NULL + +barney: + id: 3 + name: Barney Gumble + balance: 1 + address_street: Quiet Road + address_city: Peaceful Town + address_country: Tranquil Land gps_location: NULL \ No newline at end of file diff --git a/activerecord/test/models/customer.rb b/activerecord/test/models/customer.rb index 030bbc6..e258ccd 100644 --- a/activerecord/test/models/customer.rb +++ b/activerecord/test/models/customer.rb @@ -1,7 +1,8 @@ class Customer < ActiveRecord::Base composed_of :address, :mapping => [ %w(address_street street), %w(address_city city), %w(address_country country) ], :allow_nil => true - composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money } + composed_of :balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| balance.to_money } composed_of :gps_location, :allow_nil => true + composed_of :fullname, :mapping => %w(name to_s), :constructor => Proc.new { |name| Fullname.parse(name) }, :converter => :parse end class Address @@ -53,3 +54,20 @@ class GpsLocation self.latitude == other.latitude && self.longitude == other.longitude end end + +class Fullname + attr_reader :first, :last + + def self.parse(str) + return nil unless str + new(*str.to_s.split) + end + + def initialize(first, last = nil) + @first, @last = first, last + end + + def to_s + "#{first} #{last.upcase}" + end +end \ No newline at end of file -- 1.5.5.1015.g9d258