Ticket #10572: find_conditions_with_aggregation_support.diff

activerecord/test/finder_test.rb

@@ -7 +7 @@
-require 'fixtures/entrant'
-require 'fixtures/developer'
-require 'fixtures/post'
+require 'fixtures/customer'
-
-class FinderTest < Test::Unit::TestCase
-
+, :customers
-
-  def test_find
-    assert_equal(topics(:first).title, Topic.find(1).title)
@@ -39 +40 @@
-
-    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 ]))
@@ -173 +189 @@
-    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 +262 @@
-    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!"])
@@ -361 +427 @@
-  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 +505 @@
-    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
@@ -457 +583 @@
-    assert_equal sig38, Company.find_or_create_by_name("38signals")
-    assert !sig38.new_record?
-  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_two_attributes
-    number_of_topics = Topic.count
@@ -465 +599 @@
-    assert_equal another, Topic.find_or_create_by_title_and_author_name("Another topic", "John")
-    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
@@ -477 +619 @@
-    assert_equal 23, sig38.client_of
-  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 +673 @@
-    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
@@ -520 +687 @@
-    assert_equal 23, sig38.client_of
-    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" }

activerecord/lib/active_record/base.rb

@@ -1550 +1550 @@
-          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
-
@@ -1791 +1807 @@
-          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"
@@ -1800 +1851 @@
-        #     # => "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/lib/active_record/aggregations.rb

@@ -108 +108 @@
-    # 
-    # 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.