mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
let AR::FinderMethods#exists? return singletons in all cases [closes #11592]
This fixes a regression. The documentation said in its introduction paragraph that the method returns truthy/falsy, but then below it was said that if there were no arguments you'd get `true` or `false`. Also when the argument is exactly `false` a singleton is documented to be returned. The method was not returning the singletons so it didn't conform to those special cases. The best solution here seems to be to just return singletons in all cases. This solution is backwards compatible. Also, the contract has been revised because it has no sense that the predicate varies that way depending on the input. I bet the previous contract was just an accident, not something mixed on purpose. Conflicts: activerecord/lib/active_record/relation/finder_methods.rb activerecord/test/cases/finder_test.rb
This commit is contained in:
parent
2fcef6678a
commit
565c367d34
2 changed files with 36 additions and 34 deletions
|
@ -171,21 +171,21 @@ module ActiveRecord
|
|||
last or raise RecordNotFound
|
||||
end
|
||||
|
||||
# Returns truthy if a record exists in the table that matches the +id+ or
|
||||
# conditions given, or falsy otherwise. The argument can take six forms:
|
||||
# Returns +true+ if a record exists in the table that matches the +id+ or
|
||||
# conditions given, or +false+ otherwise. The argument can take six forms:
|
||||
#
|
||||
# * Integer - Finds the record with this primary key.
|
||||
# * String - Finds the record with a primary key corresponding to this
|
||||
# string (such as <tt>'5'</tt>).
|
||||
# * Array - Finds the record that matches these +find+-style conditions
|
||||
# (such as <tt>['color = ?', 'red']</tt>).
|
||||
# (such as <tt>['name LIKE ?', "%#{query}%"]</tt>).
|
||||
# * Hash - Finds the record that matches these +find+-style conditions
|
||||
# (such as <tt>{color: 'red'}</tt>).
|
||||
# (such as <tt>{name: 'David'}</tt>).
|
||||
# * +false+ - Returns always +false+.
|
||||
# * No args - Returns +false+ if the table is empty, +true+ otherwise.
|
||||
#
|
||||
# For more information about specifying conditions as a Hash or Array,
|
||||
# see the Conditions section in the introduction to ActiveRecord::Base.
|
||||
# For more information about specifying conditions as a hash or array,
|
||||
# see the Conditions section in the introduction to <tt>ActiveRecord::Base</tt>.
|
||||
#
|
||||
# Note: You can't pass in a condition as a string (like <tt>name =
|
||||
# 'Jamie'</tt>), since it would be sanitized and then queried against
|
||||
|
@ -213,7 +213,7 @@ module ActiveRecord
|
|||
relation = relation.where(table[primary_key].eq(conditions)) if conditions != :none
|
||||
end
|
||||
|
||||
connection.select_value(relation.arel, "#{name} Exists", relation.bind_values)
|
||||
connection.select_value(relation, "#{name} Exists", relation.bind_values) ? true : false
|
||||
end
|
||||
|
||||
# This method is called whenever no records are found with either a single
|
||||
|
|
|
@ -45,17 +45,18 @@ class FinderTest < ActiveRecord::TestCase
|
|||
end
|
||||
|
||||
def test_exists
|
||||
assert Topic.exists?(1)
|
||||
assert Topic.exists?("1")
|
||||
assert Topic.exists?(title: "The First Topic")
|
||||
assert Topic.exists?(heading: "The First Topic")
|
||||
assert Topic.exists?(:author_name => "Mary", :approved => true)
|
||||
assert Topic.exists?(["parent_id = ?", 1])
|
||||
assert !Topic.exists?(45)
|
||||
assert !Topic.exists?(Topic.new)
|
||||
assert_equal true, Topic.exists?(1)
|
||||
assert_equal true, Topic.exists?("1")
|
||||
assert_equal true, Topic.exists?(title: "The First Topic")
|
||||
assert_equal true, Topic.exists?(heading: "The First Topic")
|
||||
assert_equal true, Topic.exists?(:author_name => "Mary", :approved => true)
|
||||
assert_equal true, Topic.exists?(["parent_id = ?", 1])
|
||||
|
||||
assert_equal false, Topic.exists?(45)
|
||||
assert_equal false, Topic.exists?(Topic.new)
|
||||
|
||||
begin
|
||||
assert !Topic.exists?("foo")
|
||||
assert_equal false, Topic.exists?("foo")
|
||||
rescue ActiveRecord::StatementInvalid
|
||||
# PostgreSQL complains about string comparison with integer field
|
||||
rescue Exception
|
||||
|
@ -72,61 +73,62 @@ class FinderTest < ActiveRecord::TestCase
|
|||
end
|
||||
|
||||
def test_exists_returns_true_with_one_record_and_no_args
|
||||
assert Topic.exists?
|
||||
assert_equal true, Topic.exists?
|
||||
end
|
||||
|
||||
def test_exists_returns_false_with_false_arg
|
||||
assert !Topic.exists?(false)
|
||||
assert_equal false, Topic.exists?(false)
|
||||
end
|
||||
|
||||
# exists? should handle nil for id's that come from URLs and always return false
|
||||
# (example: Topic.exists?(params[:id])) where params[:id] is nil
|
||||
def test_exists_with_nil_arg
|
||||
assert !Topic.exists?(nil)
|
||||
assert Topic.exists?
|
||||
assert !Topic.first.replies.exists?(nil)
|
||||
assert Topic.first.replies.exists?
|
||||
assert_equal false, Topic.exists?(nil)
|
||||
assert_equal true, Topic.exists?
|
||||
|
||||
assert_equal false, Topic.first.replies.exists?(nil)
|
||||
assert_equal true, Topic.first.replies.exists?
|
||||
end
|
||||
|
||||
# ensures +exists?+ runs valid SQL by excluding order value
|
||||
def test_exists_with_order
|
||||
assert Topic.order(:id).distinct.exists?
|
||||
assert_equal true, Topic.order(:id).distinct.exists?
|
||||
end
|
||||
|
||||
def test_exists_with_includes_limit_and_empty_result
|
||||
assert !Topic.includes(:replies).limit(0).exists?
|
||||
assert !Topic.includes(:replies).limit(1).where('0 = 1').exists?
|
||||
assert_equal false, Topic.includes(:replies).limit(0).exists?
|
||||
assert_equal false, Topic.includes(:replies).limit(1).where('0 = 1').exists?
|
||||
end
|
||||
|
||||
def test_exists_with_distinct_association_includes_and_limit
|
||||
author = Author.first
|
||||
assert !author.unique_categorized_posts.includes(:special_comments).limit(0).exists?
|
||||
assert author.unique_categorized_posts.includes(:special_comments).limit(1).exists?
|
||||
assert_equal false, author.unique_categorized_posts.includes(:special_comments).limit(0).exists?
|
||||
assert_equal true, author.unique_categorized_posts.includes(:special_comments).limit(1).exists?
|
||||
end
|
||||
|
||||
def test_exists_with_distinct_association_includes_limit_and_order
|
||||
author = Author.first
|
||||
assert !author.unique_categorized_posts.includes(:special_comments).order('comments.taggings_count DESC').limit(0).exists?
|
||||
assert author.unique_categorized_posts.includes(:special_comments).order('comments.taggings_count DESC').limit(1).exists?
|
||||
assert_equal false, author.unique_categorized_posts.includes(:special_comments).order('comments.taggings_count DESC').limit(0).exists?
|
||||
assert_equal true, author.unique_categorized_posts.includes(:special_comments).order('comments.taggings_count DESC').limit(1).exists?
|
||||
end
|
||||
|
||||
def test_exists_with_empty_table_and_no_args_given
|
||||
Topic.delete_all
|
||||
assert !Topic.exists?
|
||||
assert_equal false, Topic.exists?
|
||||
end
|
||||
|
||||
def test_exists_with_aggregate_having_three_mappings
|
||||
existing_address = customers(:david).address
|
||||
assert Customer.exists?(:address => existing_address)
|
||||
assert_equal true, 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 =>
|
||||
assert_equal false, Customer.exists?(:address =>
|
||||
Address.new(existing_address.street, existing_address.city, existing_address.country + "1"))
|
||||
assert !Customer.exists?(:address =>
|
||||
assert_equal false, Customer.exists?(:address =>
|
||||
Address.new(existing_address.street, existing_address.city + "1", existing_address.country))
|
||||
assert !Customer.exists?(:address =>
|
||||
assert_equal false, Customer.exists?(:address =>
|
||||
Address.new(existing_address.street + "1", existing_address.city, existing_address.country))
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in a new issue