Skip to content
This repository
Browse code

Added :constructor and :converter options to composed_of and deprecat…

…ed the conversion block

Signed-off-by: Michael Koziarski <michael@koziarski.com>
  • Loading branch information...
commit 2cee51d5c1d143f6fe0096ba6cbd1db1ecbe2d90 1 parent 7c9851d
Rob Anderton rob-at-thewebfellas authored NZKoz committed
91 activerecord/lib/active_record/aggregations.rb
@@ -10,10 +10,10 @@ def clear_aggregation_cache #:nodoc:
10 10 end unless self.new_record?
11 11 end
12 12
13   - # Active Record implements aggregation through a macro-like class method called +composed_of+ for representing attributes
  13 + # Active Record implements aggregation through a macro-like class method called +composed_of+ for representing attributes
14 14 # as value objects. It expresses relationships like "Account [is] composed of Money [among other things]" or "Person [is]
15   - # composed of [an] address". Each call to the macro adds a description of how the value objects are created from the
16   - # attributes of the entity object (when the entity is initialized either as a new object or from finding an existing object)
  15 + # composed of [an] address". Each call to the macro adds a description of how the value objects are created from the
  16 + # attributes of the entity object (when the entity is initialized either as a new object or from finding an existing object)
17 17 # and how it can be turned back into attributes (when the entity is saved to the database). Example:
18 18 #
19 19 # class Customer < ActiveRecord::Base
@@ -30,10 +30,10 @@ def clear_aggregation_cache #:nodoc:
30 30 # class Money
31 31 # include Comparable
32 32 # attr_reader :amount, :currency
33   - # EXCHANGE_RATES = { "USD_TO_DKK" => 6 }
34   - #
35   - # def initialize(amount, currency = "USD")
36   - # @amount, @currency = amount, currency
  33 + # EXCHANGE_RATES = { "USD_TO_DKK" => 6 }
  34 + #
  35 + # def initialize(amount, currency = "USD")
  36 + # @amount, @currency = amount, currency
37 37 # end
38 38 #
39 39 # def exchange_to(other_currency)
@@ -56,19 +56,19 @@ def clear_aggregation_cache #:nodoc:
56 56 #
57 57 # class Address
58 58 # attr_reader :street, :city
59   - # def initialize(street, city)
60   - # @street, @city = street, city
  59 + # def initialize(street, city)
  60 + # @street, @city = street, city
61 61 # end
62 62 #
63   - # def close_to?(other_address)
64   - # city == other_address.city
  63 + # def close_to?(other_address)
  64 + # city == other_address.city
65 65 # end
66 66 #
67 67 # def ==(other_address)
68 68 # city == other_address.city && street == other_address.street
69 69 # end
70 70 # end
71   - #
  71 + #
72 72 # Now it's possible to access attributes from the database through the value objects instead. If you choose to name the
73 73 # composition the same as the attribute's name, it will be the only way to access that attribute. That's the case with our
74 74 # +balance+ attribute. You interact with the value objects just like you would any other attribute, though:
@@ -87,8 +87,8 @@ def clear_aggregation_cache #:nodoc:
87 87 # customer.address_city = "Copenhagen"
88 88 # customer.address # => Address.new("Hyancintvej", "Copenhagen")
89 89 # customer.address = Address.new("May Street", "Chicago")
90   - # customer.address_street # => "May Street"
91   - # customer.address_city # => "Chicago"
  90 + # customer.address_street # => "May Street"
  91 + # customer.address_city # => "Chicago"
92 92 #
93 93 # == Writing value objects
94 94 #
@@ -103,9 +103,9 @@ def clear_aggregation_cache #:nodoc:
103 103 # returns a new value object instead of changing its own values. Active Record won't persist value objects that have been
104 104 # changed through means other than the writer method.
105 105 #
106   - # The immutable requirement is enforced by Active Record by freezing any object assigned as a value object. Attempting to
  106 + # The immutable requirement is enforced by Active Record by freezing any object assigned as a value object. Attempting to
107 107 # change it afterwards will result in a ActiveSupport::FrozenObjectError.
108   - #
  108 + #
109 109 # Read more about value objects on http://c2.com/cgi/wiki?ValueObject and on the dangers of not keeping value objects
110 110 # immutable on http://c2.com/cgi/wiki?ValueObjectsShouldBeImmutable
111 111 #
@@ -130,39 +130,59 @@ module ClassMethods
130 130 # * <tt>:allow_nil</tt> - specifies that the aggregate object will not be instantiated when all mapped
131 131 # attributes are +nil+. Setting the aggregate class to +nil+ has the effect of writing +nil+ to all mapped attributes.
132 132 # This defaults to +false+.
133   - #
134   - # An optional block can be passed to convert the argument that is passed to the writer method into an instance of
135   - # <tt>:class_name</tt>. The block will only be called if the argument is not already an instance of <tt>:class_name</tt>.
  133 + # * <tt>:constructor</tt> - a symbol specifying the name of the constructor method or a Proc that will be used to convert the
  134 + # attributes that are mapped to the aggregation to instantiate a <tt>:class_name</tt> object. The default is +:new+.
  135 + # * <tt>:converter</tt> - a symbol specifying the name of a class method of <tt>:class_name</tt> or a Proc that will be used to convert
  136 + # the argument that is passed to the writer method into an instance of <tt>:class_name</tt>. The converter will only be called
  137 + # if the argument is not already an instance of <tt>:class_name</tt>.
136 138 #
137 139 # Option examples:
138 140 # composed_of :temperature, :mapping => %w(reading celsius)
139   - # composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) {|balance| balance.to_money }
  141 + # composed_of :balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| balance.to_money }
140 142 # composed_of :address, :mapping => [ %w(address_street street), %w(address_city city) ]
141 143 # composed_of :gps_location
142 144 # composed_of :gps_location, :allow_nil => true
  145 + # composed_of :ip_address,
  146 + # :class_name => 'IPAddr',
  147 + # :mapping => %w(ip to_i),
  148 + # :constructor => Proc.new { |ip| IPAddr.new(ip, Socket::AF_INET) },
  149 + # :converter => Proc.new { |ip| ip.is_a?(Integer) ? IPAddr.new(ip, Socket::AF_INET) : IPAddr.new(ip.to_s) }
143 150 #
144 151 def composed_of(part_id, options = {}, &block)
145   - options.assert_valid_keys(:class_name, :mapping, :allow_nil)
  152 + options.assert_valid_keys(:class_name, :mapping, :allow_nil, :constructor, :converter)
146 153
147 154 name = part_id.id2name
148   - class_name = options[:class_name] || name.camelize
149   - mapping = options[:mapping] || [ name, name ]
  155 + class_name = options[:class_name] || name.camelize
  156 + mapping = options[:mapping] || [ name, name ]
150 157 mapping = [ mapping ] unless mapping.first.is_a?(Array)
151   - allow_nil = options[:allow_nil] || false
  158 + allow_nil = options[:allow_nil] || false
  159 + constructor = options[:constructor] || :new
  160 + converter = options[:converter] || block
  161 +
  162 + ActiveSupport::Deprecation.warn('The conversion block has been deprecated, use the :converter option instead.', caller) if block_given?
  163 +
  164 + reader_method(name, class_name, mapping, allow_nil, constructor)
  165 + writer_method(name, class_name, mapping, allow_nil, converter)
152 166
153   - reader_method(name, class_name, mapping, allow_nil)
154   - writer_method(name, class_name, mapping, allow_nil, block)
155   -
156 167 create_reflection(:composed_of, part_id, options, self)
157 168 end
158 169
159 170 private
160   - def reader_method(name, class_name, mapping, allow_nil)
  171 + def reader_method(name, class_name, mapping, allow_nil, constructor)
161 172 module_eval do
162 173 define_method(name) do |*args|
163 174 force_reload = args.first || false
164 175 if (instance_variable_get("@#{name}").nil? || force_reload) && (!allow_nil || mapping.any? {|pair| !read_attribute(pair.first).nil? })
165   - instance_variable_set("@#{name}", class_name.constantize.new(*mapping.collect {|pair| read_attribute(pair.first)}))
  176 + attrs = mapping.collect {|pair| read_attribute(pair.first)}
  177 + object = case constructor
  178 + when Symbol
  179 + class_name.constantize.send(constructor, *attrs)
  180 + when Proc, Method
  181 + constructor.call(*attrs)
  182 + else
  183 + raise ArgumentError, 'Constructor must be a symbol denoting the constructor method to call or a Proc to be invoked.'
  184 + end
  185 + instance_variable_set("@#{name}", object)
166 186 end
167 187 instance_variable_get("@#{name}")
168 188 end
@@ -170,14 +190,23 @@ def reader_method(name, class_name, mapping, allow_nil)
170 190
171 191 end
172 192
173   - def writer_method(name, class_name, mapping, allow_nil, conversion)
  193 + def writer_method(name, class_name, mapping, allow_nil, converter)
174 194 module_eval do
175 195 define_method("#{name}=") do |part|
176 196 if part.nil? && allow_nil
177 197 mapping.each { |pair| self[pair.first] = nil }
178 198 instance_variable_set("@#{name}", nil)
179 199 else
180   - part = conversion.call(part) unless part.is_a?(class_name.constantize) || conversion.nil?
  200 + unless part.is_a?(class_name.constantize) || converter.nil?
  201 + part = case converter
  202 + when Symbol
  203 + class_name.constantize.send(converter, part)
  204 + when Proc, Method
  205 + converter.call(part)
  206 + else
  207 + raise ArgumentError, 'Converter must be a symbol denoting the converter method to call or a Proc to be invoked.'
  208 + end
  209 + end
181 210 mapping.each { |pair| self[pair.first] = part.send(pair.last) }
182 211 instance_variable_set("@#{name}", part.freeze)
183 212 end
35 activerecord/test/cases/aggregations_test.rb
@@ -107,6 +107,41 @@ def test_nil_assignment_results_in_nil
107 107 customers(:david).gps_location = nil
108 108 assert_equal nil, customers(:david).gps_location
109 109 end
  110 +
  111 + def test_custom_constructor
  112 + assert_equal 'Barney GUMBLE', customers(:barney).fullname.to_s
  113 + assert_kind_of Fullname, customers(:barney).fullname
  114 + end
  115 +
  116 + def test_custom_converter
  117 + customers(:barney).fullname = 'Barnoit Gumbleau'
  118 + assert_equal 'Barnoit GUMBLEAU', customers(:barney).fullname.to_s
  119 + assert_kind_of Fullname, customers(:barney).fullname
  120 + end
  121 +end
  122 +
  123 +class DeprecatedAggregationsTest < ActiveRecord::TestCase
  124 + class Person < ActiveRecord::Base; end
  125 +
  126 + def test_conversion_block_is_deprecated
  127 + assert_deprecated 'conversion block has been deprecated' do
  128 + Person.composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money }
  129 + end
  130 + end
  131 +
  132 + def test_conversion_block_used_when_converter_option_is_nil
  133 + Person.composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money }
  134 + assert_raise(NoMethodError) { Person.new.balance = 5 }
  135 + end
  136 +
  137 + def test_converter_option_overrides_conversion_block
  138 + Person.composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| Money.new(balance) }) { |balance| balance.to_money }
  139 +
  140 + person = Person.new
  141 + assert_nothing_raised { person.balance = 5 }
  142 + assert_equal 5, person.balance.amount
  143 + assert_kind_of Money, person.balance
  144 + end
110 145 end
111 146
112 147 class OverridingAggregationsTest < ActiveRecord::TestCase
11 activerecord/test/fixtures/customers.yml
@@ -6,7 +6,7 @@ david:
6 6 address_city: Scary Town
7 7 address_country: Loony Land
8 8 gps_location: 35.544623640962634x-105.9309951055148
9   -
  9 +
10 10 zaphod:
11 11 id: 2
12 12 name: Zaphod
@@ -14,4 +14,13 @@ zaphod:
14 14 address_street: Avenue Road
15 15 address_city: Hamlet Town
16 16 address_country: Nation Land
  17 + gps_location: NULL
  18 +
  19 +barney:
  20 + id: 3
  21 + name: Barney Gumble
  22 + balance: 1
  23 + address_street: Quiet Road
  24 + address_city: Peaceful Town
  25 + address_country: Tranquil Land
17 26 gps_location: NULL
20 activerecord/test/models/customer.rb
... ... @@ -1,7 +1,8 @@
1 1 class Customer < ActiveRecord::Base
2 2 composed_of :address, :mapping => [ %w(address_street street), %w(address_city city), %w(address_country country) ], :allow_nil => true
3   - composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money }
  3 + composed_of :balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| balance.to_money }
4 4 composed_of :gps_location, :allow_nil => true
  5 + composed_of :fullname, :mapping => %w(name to_s), :constructor => Proc.new { |name| Fullname.parse(name) }, :converter => :parse
5 6 end
6 7
7 8 class Address
@@ -53,3 +54,20 @@ def ==(other)
53 54 self.latitude == other.latitude && self.longitude == other.longitude
54 55 end
55 56 end
  57 +
  58 +class Fullname
  59 + attr_reader :first, :last
  60 +
  61 + def self.parse(str)
  62 + return nil unless str
  63 + new(*str.to_s.split)
  64 + end
  65 +
  66 + def initialize(first, last = nil)
  67 + @first, @last = first, last
  68 + end
  69 +
  70 + def to_s
  71 + "#{first} #{last.upcase}"
  72 + end
  73 +end

0 comments on commit 2cee51d

Please sign in to comment.
Something went wrong with that request. Please try again.