Support aggregations in finder conditions. Closes #10572.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8671 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Support aggregations in finder conditions.  #10572 [Ryan Kinderman]
+
 * Organize and clean up the Active Record test suite.  #10742 [John Barnette]
 
 * Ensure that modifying has_and_belongs_to_many actions clear the query cache.  Closes #10840 [john.andrews]

activerecord/lib/active_record/aggregations.rb

@@ -108,6 +108,15 @@ def clear_aggregation_cache #:nodoc:
     # 
     # 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
+    #
+    # == 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.find(:all, :conditions => {:balance => Money.new(20, "USD")})
+    #
     module ClassMethods
       # Adds reader and writer methods for manipulating a value object:
       # <tt>composed_of :address</tt> adds <tt>address</tt> and <tt>address=(new_address)</tt> methods.

activerecord/lib/active_record/base.rb

@@ -1560,7 +1560,23 @@ def construct_attributes_from_arguments(attribute_names, arguments)
           attributes
         end
 
+        # Similar in purpose to +expand_hash_conditions_for_aggregates+.
+        def expand_attribute_names_for_aggregates(attribute_names)
+          expanded_attribute_names = []
+          attribute_names.each do |attribute_name|
+            unless (aggregation = reflect_on_aggregation(attribute_name.to_sym)).nil?
+              aggregate_mapping(aggregation).each do |field_attr, aggregate_attr|
+                expanded_attribute_names << field_attr
+              end
+            else
+              expanded_attribute_names << attribute_name
+            end
+          end
+          expanded_attribute_names
+        end
+
         def all_attributes_exists?(attribute_names)
+          attribute_names = expand_attribute_names_for_aggregates(attribute_names)
           attribute_names.all? { |name| column_methods_hash.include?(name.to_sym) }
         end
 
@@ -1801,6 +1817,41 @@ def sanitize_sql_for_assignment(assignments)
           end
         end
 
+        def aggregate_mapping(reflection)
+          mapping = reflection.options[:mapping] || [reflection.name, reflection.name]
+          mapping.first.is_a?(Array) ? mapping : [mapping]
+        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|
+            unless (aggregation = reflect_on_aggregation(attr.to_sym)).nil?
+              mapping = aggregate_mapping(aggregation)
+              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"
@@ -1810,7 +1861,12 @@ def sanitize_sql_for_assignment(assignments)
         #     # => "age BETWEEN 13 AND 18"
         #   { 'other_records.id' => 7 }
         #     # => "`other_records`.`id` = 7"
+        # And for value objects on a composed_of relationship:
+        #   { :address => Address.new("123 abc st.", "chicago") }
+        #     # => "address_street='123 abc st.' and address_city='chicago'"
         def sanitize_sql_hash_for_conditions(attrs)
+          attrs = expand_hash_conditions_for_aggregates(attrs)
+
           conditions = attrs.map do |attr, value|
             attr = attr.to_s
 

activerecord/test/cases/finder_test.rb

@@ -7,9 +7,10 @@
 require 'models/entrant'
 require 'models/developer'
 require 'models/post'
+require 'models/customer'
 
 class FinderTest < ActiveSupport::TestCase
-  fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors
+  fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers
 
   def test_find
     assert_equal(topics(:first).title, Topic.find(1).title)
@@ -40,6 +41,21 @@ def test_exists
     assert_raise(NoMethodError) { Topic.exists?([1,2]) }
   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_find_by_array_of_one_id
     assert_kind_of(Array, Topic.find([ 1 ]))
     assert_equal(1, Topic.find([ 1 ]).length)
@@ -173,6 +189,14 @@ def test_find_on_hash_conditions_with_explicit_table_name
     assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { 'topics.approved' => true }) }
   end
 
+  def test_find_on_hash_conditions_with_explicit_table_name_and_aggregate
+    david = customers(:david)
+    assert Customer.find(david.id, :conditions => { 'customers.name' => david.name, :address => david.address })
+    assert_raises(ActiveRecord::RecordNotFound) {
+      Customer.find(david.id, :conditions => { 'customers.name' => david.name + "1", :address => david.address })
+    }
+  end
+
   def test_find_on_association_proxy_conditions
     assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10], Comment.find_all_by_post_id(authors(:david).posts).map(&:id).sort
   end
@@ -238,6 +262,48 @@ def test_hash_condition_find_with_nil
     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.find(:first, :conditions => {:balance => balance})
+    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.find(:first, :conditions => {:gps_location => gps_location})
+    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.find(:first, :conditions => {:balance => balance.amount})
+    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.find(:first, :conditions => {:gps_location => gps_location.gps_location})
+    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.find(:first, :conditions => {:address => address})
+    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.find(:first, :conditions => {:address => address, :name => customers(:david).name})
+    assert_equal customers(:david), found_customer
+  end
+
   def test_bind_variables
     assert_kind_of Firm, Company.find(:first, :conditions => ["name = ?", "37signals"])
     assert_nil Company.find(:first, :conditions => ["name = ?", "37signals!"])
@@ -362,6 +428,40 @@ def test_find_by_one_attribute_with_conditions
     assert_equal accounts(:rails_core_account), Account.find_by_credit_limit(50, :conditions => ['firm_id = ?', 6])
   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_caches_method
     # ensure this test can run independently of order
     class << Account; self; end.send(:remove_method, :find_by_credit_limit) if Account.respond_to?(:find_by_credit_limit)
@@ -405,6 +505,32 @@ def test_find_all_by_one_attribute
     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 topics(:first), topics.last
@@ -466,6 +592,14 @@ def test_find_or_create_from_two_attributes
     assert !another.new_record?
   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.new_record?
+  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})
@@ -477,12 +611,38 @@ def test_find_or_create_from_one_attribute_and_hash
     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.new_record?
+  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.new_record?
+    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.new_record?
   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.new_record?
+  end
+
   def test_find_or_initialize_from_one_attribute_should_set_attribute_even_when_protected
     c = Company.find_or_initialize_by_name_and_rating("Fortune 1000", 1000)
     assert_equal "Fortune 1000", c.name
@@ -513,6 +673,13 @@ def test_find_or_initialize_from_two_attributes
     assert another.new_record?
   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.new_record?
+  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
@@ -521,6 +688,15 @@ def test_find_or_initialize_from_one_attribute_and_hash
     assert sig38.new_record?
   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.new_record?
+  end
+
   def test_find_with_bad_sql
     assert_raises(ActiveRecord::StatementInvalid) { Topic.find_by_sql "select 1 from badtable" }
   end