From abdf546ad6d02ecb95766e73cd3c645a48c954de Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 19 Jan 2008 03:45:24 +0000 Subject: [PATCH] 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 | 2 + .../lib/active_record/aggregations.rb | 9 + activerecord/lib/active_record/base.rb | 56 ++++++ activerecord/test/cases/finder_test.rb | 178 +++++++++++++++++- 4 files changed, 244 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 6b2a30999e99d..60e4a9c2ad6a4 100644 --- a/activerecord/CHANGELOG +++ b/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] diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index f0f34e9a99898..cad8304bcf8b5 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/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: # composed_of :address adds address and address=(new_address) methods. diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 7be51df0a45f4..2d2f2297ed126 100755 --- a/activerecord/lib/active_record/base.rb +++ b/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 diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 544186f1c5d28..73b604c708226 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/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