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/README.rdoc | 12 - 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 --- activerecord/test/cases/aggregations_test.rb | 158 ------------- activerecord/test/cases/base_test.rb | 44 ---- .../test/cases/deprecated_dynamic_methods_test.rb | 76 ------ activerecord/test/cases/finder_test.rb | 99 -------- activerecord/test/cases/reflection_test.rb | 24 -- activerecord/test/models/customer.rb | 7 - activerecord/test/models/developer.rb | 5 - 18 files changed, 9 insertions(+), 768 deletions(-) delete mode 100644 activerecord/lib/active_record/aggregations.rb delete mode 100644 activerecord/test/cases/aggregations_test.rb (limited to 'activerecord') diff --git a/activerecord/README.rdoc b/activerecord/README.rdoc index d080e0b0f5..60965590a1 100644 --- a/activerecord/README.rdoc +++ b/activerecord/README.rdoc @@ -46,18 +46,6 @@ A short rundown of some of the major features: {Learn more}[link:classes/ActiveRecord/Associations/ClassMethods.html] -* Aggregations of value objects. - - class Account < 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 - - {Learn more}[link:classes/ActiveRecord/Aggregations/ClassMethods.html] - - * Validation rules that can differ for new or existing objects. class Account < ActiveRecord::Base 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 diff --git a/activerecord/test/cases/aggregations_test.rb b/activerecord/test/cases/aggregations_test.rb deleted file mode 100644 index 5bd8f76ba2..0000000000 --- a/activerecord/test/cases/aggregations_test.rb +++ /dev/null @@ -1,158 +0,0 @@ -require "cases/helper" -require 'models/customer' -require 'active_support/core_ext/exception' - -class AggregationsTest < ActiveRecord::TestCase - fixtures :customers - - def test_find_single_value_object - assert_equal 50, customers(:david).balance.amount - assert_kind_of Money, customers(:david).balance - assert_equal 300, customers(:david).balance.exchange_to("DKK").amount - end - - def test_find_multiple_value_object - assert_equal customers(:david).address_street, customers(:david).address.street - assert( - customers(:david).address.close_to?(Address.new("Different Street", customers(:david).address_city, customers(:david).address_country)) - ) - end - - def test_change_single_value_object - customers(:david).balance = Money.new(100) - customers(:david).save - assert_equal 100, customers(:david).reload.balance.amount - end - - def test_immutable_value_objects - customers(:david).balance = Money.new(100) - assert_raise(ActiveSupport::FrozenObjectError) { customers(:david).balance.instance_eval { @amount = 20 } } - end - - def test_inferred_mapping - assert_equal "35.544623640962634", customers(:david).gps_location.latitude - assert_equal "-105.9309951055148", customers(:david).gps_location.longitude - - customers(:david).gps_location = GpsLocation.new("39x-110") - - assert_equal "39", customers(:david).gps_location.latitude - assert_equal "-110", customers(:david).gps_location.longitude - - customers(:david).save - - customers(:david).reload - - assert_equal "39", customers(:david).gps_location.latitude - assert_equal "-110", customers(:david).gps_location.longitude - end - - def test_reloaded_instance_refreshes_aggregations - assert_equal "35.544623640962634", customers(:david).gps_location.latitude - assert_equal "-105.9309951055148", customers(:david).gps_location.longitude - - Customer.update_all("gps_location = '24x113'") - customers(:david).reload - assert_equal '24x113', customers(:david)['gps_location'] - - assert_equal GpsLocation.new('24x113'), customers(:david).gps_location - end - - def test_gps_equality - assert_equal GpsLocation.new('39x110'), GpsLocation.new('39x110') - end - - def test_gps_inequality - assert_not_equal GpsLocation.new('39x110'), GpsLocation.new('39x111') - end - - def test_allow_nil_gps_is_nil - assert_nil customers(:zaphod).gps_location - end - - def test_allow_nil_gps_set_to_nil - customers(:david).gps_location = nil - customers(:david).save - customers(:david).reload - assert_nil customers(:david).gps_location - end - - def test_allow_nil_set_address_attributes_to_nil - customers(:zaphod).address = nil - assert_nil customers(:zaphod).attributes[:address_street] - assert_nil customers(:zaphod).attributes[:address_city] - assert_nil customers(:zaphod).attributes[:address_country] - end - - def test_allow_nil_address_set_to_nil - customers(:zaphod).address = nil - customers(:zaphod).save - customers(:zaphod).reload - assert_nil customers(:zaphod).address - end - - def test_nil_raises_error_when_allow_nil_is_false - assert_raise(NoMethodError) { customers(:david).balance = nil } - end - - def test_allow_nil_address_loaded_when_only_some_attributes_are_nil - customers(:zaphod).address_street = nil - customers(:zaphod).save - customers(:zaphod).reload - assert_kind_of Address, customers(:zaphod).address - assert_nil customers(:zaphod).address.street - end - - def test_nil_assignment_results_in_nil - customers(:david).gps_location = GpsLocation.new('39x111') - assert_not_nil customers(:david).gps_location - customers(:david).gps_location = nil - assert_nil customers(:david).gps_location - end - - def test_nil_return_from_converter_is_respected_when_allow_nil_is_true - customers(:david).non_blank_gps_location = "" - customers(:david).save - customers(:david).reload - assert_nil customers(:david).non_blank_gps_location - end - - def test_nil_return_from_converter_results_in_failure_when_allow_nil_is_false - assert_raises(NoMethodError) do - customers(:barney).gps_location = "" - end - end - - def test_do_not_run_the_converter_when_nil_was_set - customers(:david).non_blank_gps_location = nil - assert_nil Customer.gps_conversion_was_run - 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 OverridingAggregationsTest < ActiveRecord::TestCase - class Name; end - class DifferentName; end - - class Person < ActiveRecord::Base - composed_of :composed_of, :mapping => %w(person_first_name first_name) - end - - class DifferentPerson < Person - composed_of :composed_of, :class_name => 'DifferentName', :mapping => %w(different_person_first_name first_name) - end - - def test_composed_of_aggregation_redefinition_reflections_should_differ_and_not_inherited - assert_not_equal Person.reflect_on_aggregation(:composed_of), - DifferentPerson.reflect_on_aggregation(:composed_of) - end -end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index c5818d5ded..e3d6871ad4 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -888,22 +888,6 @@ class BasicsTest < ActiveRecord::TestCase assert_equal Time.local(2004, 6, 24, 16, 24, 0), topic.written_on end - def test_multiparameter_assignment_of_aggregation - customer = Customer.new - address = Address.new("The Street", "The City", "The Country") - attributes = { "address(1)" => address.street, "address(2)" => address.city, "address(3)" => address.country } - customer.attributes = attributes - assert_equal address, customer.address - end - - def test_multiparameter_assignment_of_aggregation_out_of_order - customer = Customer.new - address = Address.new("The Street", "The City", "The Country") - attributes = { "address(3)" => address.country, "address(2)" => address.city, "address(1)" => address.street } - customer.attributes = attributes - assert_equal address, customer.address - end - def test_multiparameter_assignment_of_aggregation_with_missing_values ex = assert_raise(ActiveRecord::MultiparameterAssignmentErrors) do customer = Customer.new @@ -914,14 +898,6 @@ class BasicsTest < ActiveRecord::TestCase assert_equal("address", ex.errors[0].attribute) end - def test_multiparameter_assignment_of_aggregation_with_blank_values - customer = Customer.new - address = Address.new("The Street", "The City", "The Country") - attributes = { "address(1)" => "", "address(2)" => address.city, "address(3)" => address.country } - customer.attributes = attributes - assert_equal Address.new(nil, "The City", "The Country"), customer.address - end - def test_multiparameter_assignment_of_aggregation_with_large_index ex = assert_raise(ActiveRecord::MultiparameterAssignmentErrors) do customer = Customer.new @@ -1021,26 +997,6 @@ class BasicsTest < ActiveRecord::TestCase assert_equal("c", duped_topic.title) end - def test_dup_with_aggregate_of_same_name_as_attribute - dev = DeveloperWithAggregate.find(1) - assert_kind_of DeveloperSalary, dev.salary - - dup = nil - assert_nothing_raised { dup = dev.dup } - assert_kind_of DeveloperSalary, dup.salary - assert_equal dev.salary.amount, dup.salary.amount - assert !dup.persisted? - - # test if the attributes have been dupd - original_amount = dup.salary.amount - dev.salary.amount = 1 - assert_equal original_amount, dup.salary.amount - - assert dup.save - assert dup.persisted? - assert_not_equal dup.id, dev.id - end - def test_dup_does_not_copy_associations author = authors(:david) assert_not_equal [], author.posts diff --git a/activerecord/test/cases/deprecated_dynamic_methods_test.rb b/activerecord/test/cases/deprecated_dynamic_methods_test.rb index 77a609f49a..7c108b983e 100644 --- a/activerecord/test/cases/deprecated_dynamic_methods_test.rb +++ b/activerecord/test/cases/deprecated_dynamic_methods_test.rb @@ -45,32 +45,6 @@ class DeprecatedDynamicMethodsTest < ActiveRecord::TestCase assert_equal [], Topic.find_all_by_title("The First Topic!!") end - def test_find_all_by_one_attribute_that_is_an_aggregate - balance = customers(:david).balance - assert_kind_of Money, balance - found_customers = Customer.find_all_by_balance(balance) - assert_equal 1, found_customers.size - assert_equal customers(:david), found_customers.first - end - - def test_find_all_by_two_attributes_that_are_both_aggregates - balance = customers(:david).balance - address = customers(:david).address - assert_kind_of Money, balance - assert_kind_of Address, address - found_customers = Customer.find_all_by_balance_and_address(balance, address) - assert_equal 1, found_customers.size - assert_equal customers(:david), found_customers.first - end - - def test_find_all_by_two_attributes_with_one_being_an_aggregate - balance = customers(:david).balance - assert_kind_of Money, balance - found_customers = Customer.find_all_by_balance_and_name(balance, customers(:david).name) - assert_equal 1, found_customers.size - assert_equal customers(:david), found_customers.first - end - def test_find_all_by_one_attribute_with_options topics = Topic.find_all_by_content("Have a nice day", :order => "id DESC") assert_equal topics(:first), topics.last @@ -137,14 +111,6 @@ class DeprecatedDynamicMethodsTest < ActiveRecord::TestCase assert_equal 17, sig38.firm_id end - def test_find_or_create_from_two_attributes_with_one_being_an_aggregate - number_of_customers = Customer.count - created_customer = Customer.find_or_create_by_balance_and_name(Money.new(123), "Elizabeth") - assert_equal number_of_customers + 1, Customer.count - assert_equal created_customer, Customer.find_or_create_by_balance(Money.new(123), "Elizabeth") - assert created_customer.persisted? - end - def test_find_or_create_from_one_attribute_and_hash number_of_companies = Company.count sig38 = Company.find_or_create_by_name({:name => "38signals", :firm_id => 17, :client_of => 23}) @@ -167,38 +133,12 @@ class DeprecatedDynamicMethodsTest < ActiveRecord::TestCase assert_equal 23, sig38.client_of end - def test_find_or_create_from_one_aggregate_attribute - number_of_customers = Customer.count - created_customer = Customer.find_or_create_by_balance(Money.new(123)) - assert_equal number_of_customers + 1, Customer.count - assert_equal created_customer, Customer.find_or_create_by_balance(Money.new(123)) - assert created_customer.persisted? - end - - def test_find_or_create_from_one_aggregate_attribute_and_hash - number_of_customers = Customer.count - balance = Money.new(123) - name = "Elizabeth" - created_customer = Customer.find_or_create_by_balance({:balance => balance, :name => name}) - assert_equal number_of_customers + 1, Customer.count - assert_equal created_customer, Customer.find_or_create_by_balance({:balance => balance, :name => name}) - assert created_customer.persisted? - assert_equal balance, created_customer.balance - assert_equal name, created_customer.name - end - def test_find_or_initialize_from_one_attribute sig38 = Company.find_or_initialize_by_name("38signals") assert_equal "38signals", sig38.name assert !sig38.persisted? end - def test_find_or_initialize_from_one_aggregate_attribute - new_customer = Customer.find_or_initialize_by_balance(Money.new(123)) - assert_equal 123, new_customer.balance.amount - assert !new_customer.persisted? - end - def test_find_or_initialize_from_one_attribute_should_not_set_attribute_even_when_protected c = Company.find_or_initialize_by_name({:name => "Fortune 1000", :rating => 1000}) assert_equal "Fortune 1000", c.name @@ -285,13 +225,6 @@ class DeprecatedDynamicMethodsTest < ActiveRecord::TestCase assert_raise(ArgumentError) { Topic.find_or_initialize_by_title_and_author_name("Another topic") } end - def test_find_or_initialize_from_one_aggregate_attribute_and_one_not - new_customer = Customer.find_or_initialize_by_balance_and_name(Money.new(123), "Elizabeth") - assert_equal 123, new_customer.balance.amount - assert_equal "Elizabeth", new_customer.name - assert !new_customer.persisted? - end - def test_find_or_initialize_from_one_attribute_and_hash sig38 = Company.find_or_initialize_by_name({:name => "38signals", :firm_id => 17, :client_of => 23}) assert_equal "38signals", sig38.name @@ -300,15 +233,6 @@ class DeprecatedDynamicMethodsTest < ActiveRecord::TestCase assert !sig38.persisted? end - def test_find_or_initialize_from_one_aggregate_attribute_and_hash - balance = Money.new(123) - name = "Elizabeth" - new_customer = Customer.find_or_initialize_by_balance({:balance => balance, :name => name}) - assert_equal balance, new_customer.balance - assert_equal name, new_customer.name - assert !new_customer.persisted? - end - def test_find_last_by_one_attribute assert_equal Topic.last, Topic.find_last_by_title(Topic.last.title) assert_nil Topic.find_last_by_title("A title with no matches") diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index aa44307bc2..df21585961 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -79,21 +79,6 @@ class FinderTest < ActiveRecord::TestCase assert !Topic.exists? end - def test_exists_with_aggregate_having_three_mappings - existing_address = customers(:david).address - assert Customer.exists?(:address => existing_address) - end - - def test_exists_with_aggregate_having_three_mappings_with_one_difference - existing_address = customers(:david).address - assert !Customer.exists?(:address => - Address.new(existing_address.street, existing_address.city, existing_address.country + "1")) - assert !Customer.exists?(:address => - Address.new(existing_address.street, existing_address.city + "1", existing_address.country)) - assert !Customer.exists?(:address => - Address.new(existing_address.street + "1", existing_address.city, existing_address.country)) - end - def test_exists_does_not_instantiate_records Developer.expects(:instantiate).never Developer.exists? @@ -312,14 +297,6 @@ class FinderTest < ActiveRecord::TestCase assert_equal companies(:rails_core), firms.first end - def test_find_on_hash_conditions_with_explicit_table_name_and_aggregate - david = customers(:david) - assert Customer.scoped(:where => { 'customers.name' => david.name, :address => david.address }).find(david.id) - assert_raise(ActiveRecord::RecordNotFound) { - Customer.scoped(:where => { 'customers.name' => david.name + "1", :address => david.address }).find(david.id) - } - end - def test_find_on_association_proxy_conditions assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12], Comment.where(post_id: authors(:david).posts).map(&:id).sort end @@ -394,48 +371,6 @@ class FinderTest < ActiveRecord::TestCase assert_nil topic.last_read end - def test_hash_condition_find_with_aggregate_having_one_mapping - balance = customers(:david).balance - assert_kind_of Money, balance - found_customer = Customer.scoped(:where => {:balance => balance}).first - assert_equal customers(:david), found_customer - end - - def test_hash_condition_find_with_aggregate_attribute_having_same_name_as_field_and_key_value_being_aggregate - gps_location = customers(:david).gps_location - assert_kind_of GpsLocation, gps_location - found_customer = Customer.scoped(:where => {:gps_location => gps_location}).first - assert_equal customers(:david), found_customer - end - - def test_hash_condition_find_with_aggregate_having_one_mapping_and_key_value_being_attribute_value - balance = customers(:david).balance - assert_kind_of Money, balance - found_customer = Customer.scoped(:where => {:balance => balance.amount}).first - assert_equal customers(:david), found_customer - end - - def test_hash_condition_find_with_aggregate_attribute_having_same_name_as_field_and_key_value_being_attribute_value - gps_location = customers(:david).gps_location - assert_kind_of GpsLocation, gps_location - found_customer = Customer.scoped(:where => {:gps_location => gps_location.gps_location}).first - assert_equal customers(:david), found_customer - end - - def test_hash_condition_find_with_aggregate_having_three_mappings - address = customers(:david).address - assert_kind_of Address, address - found_customer = Customer.scoped(:where => {:address => address}).first - assert_equal customers(:david), found_customer - end - - def test_hash_condition_find_with_one_condition_being_aggregate_and_another_not - address = customers(:david).address - assert_kind_of Address, address - found_customer = Customer.scoped(:where => {:address => address, :name => customers(:david).name}).first - assert_equal customers(:david), found_customer - end - def test_condition_utc_time_interpolation_with_default_timezone_local with_env_tz 'America/New_York' do with_active_record_default_timezone :local do @@ -604,40 +539,6 @@ class FinderTest < ActiveRecord::TestCase assert_equal accounts(:rails_core_account), Account.where('firm_id = ?', 6).find_by_credit_limit(50) end - def test_find_by_one_attribute_that_is_an_aggregate - address = customers(:david).address - assert_kind_of Address, address - found_customer = Customer.find_by_address(address) - assert_equal customers(:david), found_customer - end - - def test_find_by_one_attribute_that_is_an_aggregate_with_one_attribute_difference - address = customers(:david).address - assert_kind_of Address, address - missing_address = Address.new(address.street, address.city, address.country + "1") - assert_nil Customer.find_by_address(missing_address) - missing_address = Address.new(address.street, address.city + "1", address.country) - assert_nil Customer.find_by_address(missing_address) - missing_address = Address.new(address.street + "1", address.city, address.country) - assert_nil Customer.find_by_address(missing_address) - end - - def test_find_by_two_attributes_that_are_both_aggregates - balance = customers(:david).balance - address = customers(:david).address - assert_kind_of Money, balance - assert_kind_of Address, address - found_customer = Customer.find_by_balance_and_address(balance, address) - assert_equal customers(:david), found_customer - end - - def test_find_by_two_attributes_with_one_being_an_aggregate - balance = customers(:david).balance - assert_kind_of Money, balance - found_customer = Customer.find_by_balance_and_name(balance, customers(:david).name) - assert_equal customers(:david), found_customer - end - def test_dynamic_finder_on_one_attribute_with_conditions_returns_same_results_after_caching # ensure this test can run independently of order class << Account; self; end.send(:remove_method, :find_by_credit_limit) if Account.public_methods.include?(:find_by_credit_limit) diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 7dd5698dcf..bac3376c9f 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -83,30 +83,6 @@ class ReflectionTest < ActiveRecord::TestCase end end - def test_aggregation_reflection - reflection_for_address = AggregateReflection.new( - :composed_of, :address, { :mapping => [ %w(address_street street), %w(address_city city), %w(address_country country) ] }, Customer - ) - - reflection_for_balance = AggregateReflection.new( - :composed_of, :balance, { :class_name => "Money", :mapping => %w(balance amount) }, Customer - ) - - reflection_for_gps_location = AggregateReflection.new( - :composed_of, :gps_location, { }, Customer - ) - - assert Customer.reflect_on_all_aggregations.include?(reflection_for_gps_location) - assert Customer.reflect_on_all_aggregations.include?(reflection_for_balance) - assert Customer.reflect_on_all_aggregations.include?(reflection_for_address) - - assert_equal reflection_for_address, Customer.reflect_on_aggregation(:address) - - assert_equal Address, Customer.reflect_on_aggregation(:address).klass - - assert_equal Money, Customer.reflect_on_aggregation(:balance).klass - end - def test_reflect_on_all_autosave_associations expected = Pirate.reflect_on_all_associations.select { |r| r.options[:autosave] } received = Pirate.reflect_on_all_autosave_associations diff --git a/activerecord/test/models/customer.rb b/activerecord/test/models/customer.rb index 7e8e82542f..594c484f20 100644 --- a/activerecord/test/models/customer.rb +++ b/activerecord/test/models/customer.rb @@ -1,12 +1,5 @@ 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 :balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| balance.to_money } - 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), - :converter => lambda { |gps| self.gps_conversion_was_run = true; gps.blank? ? nil : GpsLocation.new(gps)} - composed_of :fullname, :mapping => %w(name to_s), :constructor => Proc.new { |name| Fullname.parse(name) }, :converter => :parse end class Address diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 83482f4d07..9fefa9ad3a 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -65,11 +65,6 @@ class AuditLog < ActiveRecord::Base end DeveloperSalary = Struct.new(:amount) -class DeveloperWithAggregate < ActiveRecord::Base - self.table_name = 'developers' - composed_of :salary, :class_name => 'DeveloperSalary', :mapping => [%w(salary amount)] -end - class DeveloperWithBeforeDestroyRaise < ActiveRecord::Base self.table_name = 'developers' has_and_belongs_to_many :projects, :join_table => 'developers_projects', :foreign_key => 'developer_id' -- cgit v1.2.3