From 14fc8b34521f8354a17e50cd11fa3f809e423592 Mon Sep 17 00:00:00 2001 From: Steve Klabnik Date: Fri, 15 Jun 2012 11:19:40 +0200 Subject: Removing composed_of from ActiveRecord. This feature adds a lot of complication to ActiveRecord for dubious value. Let's talk about what it does currently: class Customer < ActiveRecord::Base composed_of :balance, :class_name => "Money", :mapping => %w(balance amount) end Instead, you can do something like this: def balance @balance ||= Money.new(value, currency) end def balance=(balance) self[:value] = balance.value self[:currency] = balance.currency @balance = balance end Since that's fairly easy code to write, and doesn't need anything extra from the framework, if you use composed_of today, you'll have to add accessors/mutators like that. Closes #1436 Closes #2084 Closes #3807 --- activerecord/lib/active_record.rb | 1 - activerecord/lib/active_record/aggregations.rb | 261 --------------------- .../lib/active_record/attribute_assignment.rb | 4 +- activerecord/lib/active_record/core.rb | 2 - activerecord/lib/active_record/dynamic_matchers.rb | 2 +- activerecord/lib/active_record/model.rb | 1 - activerecord/lib/active_record/persistence.rb | 1 - activerecord/lib/active_record/reflection.rb | 45 +--- .../lib/active_record/relation/query_methods.rb | 3 +- activerecord/lib/active_record/sanitization.rb | 32 --- 10 files changed, 9 insertions(+), 343 deletions(-) delete mode 100644 activerecord/lib/active_record/aggregations.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 8526e224da..a894d83ea7 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -36,7 +36,6 @@ module ActiveRecord autoload :ConnectionNotEstablished, 'active_record/errors' autoload :ConnectionAdapters, 'active_record/connection_adapters/abstract_adapter' - autoload :Aggregations autoload :Associations autoload :AttributeMethods autoload :AttributeAssignment diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb deleted file mode 100644 index 3ae7030caa..0000000000 --- a/activerecord/lib/active_record/aggregations.rb +++ /dev/null @@ -1,261 +0,0 @@ -module ActiveRecord - # = Active Record Aggregations - module Aggregations # :nodoc: - extend ActiveSupport::Concern - - def clear_aggregation_cache #:nodoc: - @aggregation_cache.clear if persisted? - end - - # 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) and how it can be turned back into attributes (when the entity is saved to - # 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) ] - # end - # - # The customer class now has the following methods to manipulate the value objects: - # * Customer#balance, Customer#balance=(money) - # * Customer#address, Customer#address=(address) - # - # These methods will operate with value objects like the ones described below: - # - # class Money - # include Comparable - # attr_reader :amount, :currency - # EXCHANGE_RATES = { "USD_TO_DKK" => 6 } - # - # def initialize(amount, currency = "USD") - # @amount, @currency = amount, currency - # end - # - # def exchange_to(other_currency) - # exchanged_amount = (amount * EXCHANGE_RATES["#{currency}_TO_#{other_currency}"]).floor - # Money.new(exchanged_amount, other_currency) - # end - # - # def ==(other_money) - # amount == other_money.amount && currency == other_money.currency - # end - # - # def <=>(other_money) - # if currency == other_money.currency - # amount <=> other_money.amount - # else - # amount <=> other_money.exchange_to(currency).amount - # end - # end - # end - # - # class Address - # attr_reader :street, :city - # def initialize(street, city) - # @street, @city = street, city - # end - # - # 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 with any other attribute: - # - # customer.balance = Money.new(20) # sets the Money value object and the attribute - # customer.balance # => Money value object - # customer.balance.exchange_to("DKK") # => Money.new(120, "DKK") - # customer.balance > Money.new(10) # => true - # customer.balance == Money.new(20) # => true - # customer.balance < Money.new(5) # => false - # - # Value objects can also be composed of multiple attributes, such as the case of Address. The order - # of the mappings will determine the order of the parameters. - # - # customer.address_street = "Hyancintvej" - # customer.address_city = "Copenhagen" - # customer.address # => Address.new("Hyancintvej", "Copenhagen") - # - # customer.address_street = "Vesterbrogade" - # customer.address # => Address.new("Hyancintvej", "Copenhagen") - # customer.clear_aggregation_cache - # customer.address # => Address.new("Vesterbrogade", "Copenhagen") - # - # customer.address = Address.new("May Street", "Chicago") - # customer.address_street # => "May Street" - # customer.address_city # => "Chicago" - # - # == Writing value objects - # - # Value objects are immutable and interchangeable objects that represent a given value, such as - # a Money object representing $5. Two Money objects both representing $5 should be equal (through - # methods such as == and <=> from Comparable if ranking makes sense). This is - # unlike entity objects where equality is determined by identity. An entity class such as Customer can - # easily have two different objects that both have an address on Hyancintvej. Entity identity is - # determined by object or relational unique identifiers (such as primary keys). Normal - # ActiveRecord::Base classes are entity objects. - # - # It's also important to treat the value objects as immutable. Don't allow the Money object to have - # its amount changed after creation. Create a new Money object with the new value instead. The - # Money#exchange_to method is an example of this. It 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 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 - # - # == Custom constructors and converters - # - # By default value objects are initialized by calling the new constructor of the value - # class passing each of the mapped attributes, in the order specified by the :mapping - # option, as arguments. If the value class doesn't support this convention then +composed_of+ allows - # a custom constructor to be specified. - # - # When a new value is assigned to the value object, the default assumption is that the new value - # is an instance of the value class. Specifying a custom converter allows the new value to be automatically - # converted to an instance of value class if necessary. - # - # For example, the NetworkResource model has +network_address+ and +cidr_range+ attributes that - # should be aggregated using the NetAddr::CIDR value class (http://netaddr.rubyforge.org). The constructor - # for the value class is called +create+ and it expects a CIDR address string as a parameter. New - # values can be assigned to the value object using either another NetAddr::CIDR object, a string - # or an array. The :constructor and :converter options can be used to meet - # these requirements: - # - # class NetworkResource < ActiveRecord::Base - # composed_of :cidr, - # :class_name => 'NetAddr::CIDR', - # :mapping => [ %w(network_address network), %w(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) } - # end - # - # # This calls the :constructor - # network_resource = NetworkResource.new(:network_address => '192.168.0.1', :cidr_range => 24) - # - # # These assignments will both use the :converter - # network_resource.cidr = [ '192.168.2.1', 8 ] - # network_resource.cidr = '192.168.0.1/24' - # - # # This assignment won't use the :converter as the value is already an instance of the value class - # network_resource.cidr = NetAddr::CIDR.create('192.168.2.1/8') - # - # # Saving and then reloading will use the :constructor on reload - # network_resource.save - # network_resource.reload - # - # == Finding records by a value object - # - # Once a +composed_of+ relationship is specified for a model, records can be loaded from the database - # by specifying an instance of the value object in the conditions hash. The following example - # finds all customers with +balance_amount+ equal to 20 and +balance_currency+ equal to "USD": - # - # Customer.where(:balance => Money.new(20, "USD")).all - # - module ClassMethods - # Adds reader and writer methods for manipulating a value object: - # composed_of :address adds address and address=(new_address) methods. - # - # Options are: - # * :class_name - Specifies the class name of the association. Use it only if that name - # can't be inferred from the part id. So composed_of :address will by default be linked - # 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 - # order in which mappings are defined determines the order in which attributes are sent to the - # value class constructor. - # * :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. - # This defaults to +false+. - # * :constructor - A symbol specifying the name of the constructor method or a Proc that - # is called to initialize the value object. The constructor is passed all of the mapped attributes, - # in the order that they are defined in the :mapping option, as arguments and uses them - # 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 is called when a new value is assigned to the value object. The converter is - # passed the single value that is used in the assignment and is only called if the new value is - # not an instance of :class_name. If :allow_nil is set to true, the converter - # 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), - # :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 = {}) - 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 ] - mapping = [ mapping ] unless mapping.first.is_a?(Array) - allow_nil = options[:allow_nil] || false - constructor = options[:constructor] || :new - converter = options[:converter] - - reader_method(name, class_name, mapping, allow_nil, constructor) - writer_method(name, class_name, mapping, allow_nil, converter) - - create_reflection(:composed_of, part_id, options, self) - end - - private - def reader_method(name, class_name, mapping, allow_nil, constructor) - define_method(name) do - if @aggregation_cache[name].nil? && (!allow_nil || mapping.any? {|pair| !read_attribute(pair.first).nil? }) - attrs = mapping.collect {|pair| read_attribute(pair.first)} - object = constructor.respond_to?(:call) ? - constructor.call(*attrs) : - class_name.constantize.send(constructor, *attrs) - @aggregation_cache[name] = object - end - @aggregation_cache[name] - end - end - - def writer_method(name, class_name, mapping, allow_nil, converter) - define_method("#{name}=") do |part| - klass = class_name.constantize - unless part.is_a?(klass) || converter.nil? || part.nil? - part = converter.respond_to?(:call) ? converter.call(part) : klass.send(converter, part) - end - - if part.nil? && allow_nil - mapping.each { |pair| self[pair.first] = nil } - @aggregation_cache[name] = nil - else - mapping.each { |pair| self[pair.first] = part.send(pair.last) } - @aggregation_cache[name] = part.freeze - end - end - end - end - end -end diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index df4de8ac35..ab6d253ef8 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -132,7 +132,7 @@ module ActiveRecord private # Instantiates objects for all attribute classes that needs more than one constructor parameter. This is done - # by calling new on the column type or aggregation type (through composed_of) object with these parameters. + # by calling new on the column type or aggregation type object with these parameters. # So having the pairs written_on(1) = "2004", written_on(2) = "6", written_on(3) = "24", will instantiate # written_on (a date type) with Date.new("2004", "6", "24"). You can also specify a typecast character in the # parentheses to have the parameters typecasted before they're used in the constructor. Use i for Fixnum, @@ -167,7 +167,7 @@ module ActiveRecord end def read_value_from_parameter(name, values_hash_from_param) - klass = (self.class.reflect_on_aggregation(name.to_sym) || column_for_attribute(name)).klass + klass = column_for_attribute(name).klass if values_hash_from_param.values.all?{|v|v.nil?} nil elsif klass == Time diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index f5d60d11b6..90f156456e 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -266,7 +266,6 @@ module ActiveRecord @changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr]) end - @aggregation_cache = {} @association_cache = {} @attributes_cache = {} @@ -391,7 +390,6 @@ module ActiveRecord @attributes[pk] = nil unless @attributes.key?(pk) - @aggregation_cache = {} @association_cache = {} @attributes_cache = {} @previously_changed = {} diff --git a/activerecord/lib/active_record/dynamic_matchers.rb b/activerecord/lib/active_record/dynamic_matchers.rb index e278e62ce7..23aaa319d8 100644 --- a/activerecord/lib/active_record/dynamic_matchers.rb +++ b/activerecord/lib/active_record/dynamic_matchers.rb @@ -56,7 +56,7 @@ module ActiveRecord end def valid? - attribute_names.all? { |name| model.columns_hash[name] || model.reflect_on_aggregation(name.to_sym) } + attribute_names.all? { |name| model.columns_hash[name] } end def define diff --git a/activerecord/lib/active_record/model.rb b/activerecord/lib/active_record/model.rb index d51cb30ec1..7b3d926d91 100644 --- a/activerecord/lib/active_record/model.rb +++ b/activerecord/lib/active_record/model.rb @@ -89,7 +89,6 @@ module ActiveRecord include ActiveModel::SecurePassword include AutosaveAssociation include NestedAttributes - include Aggregations include Transactions include Reflection include Serialization diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 643c3f82ad..a23597be28 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -267,7 +267,6 @@ module ActiveRecord # may do e.g. record.reload(:lock => true) to reload the same record with # an exclusive row lock. def reload(options = nil) - clear_aggregation_cache clear_association_cache fresh_object = diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index ec13d27323..e9a8e90538 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -17,36 +17,17 @@ module ActiveRecord # and creates input fields for all of the attributes depending on their type # and displays the associations to other objects. # - # MacroReflection class has info for AggregateReflection and AssociationReflection - # classes. + # MacroReflection class has info for the AssociationReflection + # class. module ClassMethods def create_reflection(macro, name, options, active_record) - case macro - when :has_many, :belongs_to, :has_one, :has_and_belongs_to_many - klass = options[:through] ? ThroughReflection : AssociationReflection - reflection = klass.new(macro, name, options, active_record) - when :composed_of - reflection = AggregateReflection.new(macro, name, options, active_record) - end + klass = options[:through] ? ThroughReflection : AssociationReflection + reflection = klass.new(macro, name, options, active_record) self.reflections = self.reflections.merge(name => reflection) reflection end - # Returns an array of AggregateReflection objects for all the aggregations in the class. - def reflect_on_all_aggregations - reflections.values.grep(AggregateReflection) - end - - # Returns the AggregateReflection object for the named +aggregation+ (use the symbol). - # - # Account.reflect_on_aggregation(:balance) # => the balance AggregateReflection - # - def reflect_on_aggregation(aggregation) - reflection = reflections[aggregation] - reflection if reflection.is_a?(AggregateReflection) - end - # Returns an array of AssociationReflection objects for all the # associations in the class. If you only want to reflect on a certain # association type, pass in the symbol (:has_many, :has_one, @@ -78,24 +59,20 @@ module ActiveRecord end end - # Abstract base class for AggregateReflection and AssociationReflection. Objects of - # AggregateReflection and AssociationReflection are returned by the Reflection::ClassMethods. + # Abstract base class for AssociationReflection. Objects of AssociationReflection are returned by the Reflection::ClassMethods. class MacroReflection # Returns the name of the macro. # - # composed_of :balance, :class_name => 'Money' returns :balance # has_many :clients returns :clients attr_reader :name # Returns the macro type. # - # composed_of :balance, :class_name => 'Money' returns :composed_of # has_many :clients returns :has_many attr_reader :macro # Returns the hash of options used for the macro. # - # composed_of :balance, :class_name => 'Money' returns { :class_name => "Money" } # has_many :clients returns +{}+ attr_reader :options @@ -114,7 +91,6 @@ module ActiveRecord # Returns the class for the macro. # - # composed_of :balance, :class_name => 'Money' returns the Money class # has_many :clients returns the Client class def klass @klass ||= class_name.constantize @@ -122,7 +98,6 @@ module ActiveRecord # Returns the class name for the macro. # - # composed_of :balance, :class_name => 'Money' returns 'Money' # has_many :clients returns 'Client' def class_name @class_name ||= (options[:class_name] || derive_class_name).to_s @@ -148,16 +123,6 @@ module ActiveRecord end end - - # Holds all the meta-data about an aggregation as it was specified in the - # Active Record class. - class AggregateReflection < MacroReflection #:nodoc: - def mapping - mapping = options[:mapping] || [name, name] - mapping.first.is_a?(Array) ? mapping : [mapping] - end - end - # Holds all the meta-data about an association as it was specified in the # Active Record class. class AssociationReflection < MacroReflection #:nodoc: diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index e0bb84d55a..529ddb5e31 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -562,8 +562,7 @@ module ActiveRecord when String, Array [@klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))] when Hash - attributes = @klass.send(:expand_hash_conditions_for_aggregates, opts) - PredicateBuilder.build_from_hash(table.engine, attributes, table) + PredicateBuilder.build_from_hash(table.engine, opts, table) else [opts] end diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 5530be3219..46f6c283e3 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -43,36 +43,6 @@ module ActiveRecord end end - # Accepts a hash of SQL conditions and replaces those attributes - # that correspond to a +composed_of+ relationship with their expanded - # aggregate attribute values. - # Given: - # class Person < ActiveRecord::Base - # composed_of :address, :class_name => "Address", - # :mapping => [%w(address_street street), %w(address_city city)] - # end - # Then: - # { :address => Address.new("813 abc st.", "chicago") } - # # => { :address_street => "813 abc st.", :address_city => "chicago" } - def expand_hash_conditions_for_aggregates(attrs) - expanded_attrs = {} - attrs.each do |attr, value| - if aggregation = reflect_on_aggregation(attr.to_sym) - mapping = aggregation.mapping - mapping.each do |field_attr, aggregate_attr| - if mapping.size == 1 && !value.respond_to?(aggregate_attr) - expanded_attrs[field_attr] = value - else - expanded_attrs[field_attr] = value.send(aggregate_attr) - end - end - else - expanded_attrs[attr] = value - end - end - expanded_attrs - end - # Sanitizes a hash of attribute/value pairs into SQL conditions for a WHERE clause. # { :name => "foo'bar", :group_id => 4 } # # => "name='foo''bar' and group_id= 4" @@ -88,8 +58,6 @@ module ActiveRecord # { :address => Address.new("123 abc st.", "chicago") } # # => "address_street='123 abc st.' and address_city='chicago'" def sanitize_sql_hash_for_conditions(attrs, default_table_name = self.table_name) - attrs = expand_hash_conditions_for_aggregates(attrs) - table = Arel::Table.new(table_name).alias(default_table_name) PredicateBuilder.build_from_hash(arel_engine, attrs, table).map { |b| connection.visitor.accept b -- cgit v1.2.3