Changeset 8671

Timestamp:

01/19/08 03:45:24 (4 months ago)

Author:

bitsweat

Message:

Support aggregations in finder conditions. Closes #10572.

Files:

• trunk/activerecord/CHANGELOG (modified) (1 diff)
• trunk/activerecord/lib/active_record/aggregations.rb (modified) (1 diff)
• trunk/activerecord/lib/active_record/base.rb (modified) (3 diffs)
• trunk/activerecord/test/cases/finder_test.rb (modified) (10 diffs)

trunk/activerecord/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Support aggregations in finder conditions.  #10572 [Ryan Kinderman]
-
-* Organize and clean up the Active Record test suite.  #10742 [John Barnette]

trunk/activerecord/lib/active_record/aggregations.rb

@@ -109 +109 @@
-    # 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:

trunk/activerecord/lib/active_record/base.rb

@@ -1561 +1561 @@
-        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
@@ -1802 +1818 @@
-        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 }
@@ -1811 +1862 @@
-        #   { '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

trunk/activerecord/test/cases/finder_test.rb

@@ -8 +8 @@
-require 'models/developer'
-require 'models/post'
+require 'models/customer'
-
-class FinderTest < ActiveSupport::TestCase
-
+, :customers
-
-  def test_find
@@ -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
-
@@ -174 +190 @@
-  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
@@ -237 +261 @@
-    assert_not_nil topic
-    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
-
@@ -363 +429 @@
-  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
@@ -406 +506 @@
-  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")
@@ -465 +591 @@
-    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
-
@@ -478 +612 @@
-  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
-
@@ -514 +674 @@
-  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})
@@ -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
-