Permalink
Browse files

Fix merging blank conditions. Closes #10764 [mcmire, cavalle]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9082 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
jeremy committed Mar 23, 2008
1 parent e1790ef commit 0ff031992f1dc36cf36eaf26a1e476f67c29c048
Showing with 43 additions and 10 deletions.
  1. +21 −8 activerecord/lib/active_record/base.rb
  2. +22 −2 activerecord/test/cases/method_scoping_test.rb
@@ -1426,6 +1426,20 @@ def merge_includes(first, second)
(safe_to_array(first) + safe_to_array(second)).uniq
end
+ # Merges conditions so that the result is a valid +condition+
+ def merge_conditions(*conditions)
+ segments = []
+
+ conditions.each do |condition|
+ unless condition.blank?
+ sql = sanitize_sql(condition)
+ segments << sql unless sql.blank?
+ end
+ end
+
+ "(#{segments.join(') AND (')})" unless segments.empty?
+ end
+
# Object#to_a is deprecated, though it does have the desired behavior
def safe_to_array(o)
case o
@@ -1498,12 +1512,11 @@ def add_joins!(sql, options, scope = :auto)
# The optional scope argument is for the current :find scope.
def add_conditions!(sql, conditions, scope = :auto)
scope = scope(:find) if :auto == scope
- segments = []
- segments << sanitize_sql(scope[:conditions]) if scope && !scope[:conditions].blank?
- segments << sanitize_sql(conditions) unless conditions.blank?
- segments << type_condition if finder_needs_type_condition?
- segments.delete_if{|s| s.blank?}
- sql << "WHERE (#{segments.join(") AND (")}) " unless segments.empty?
+ conditions = [conditions]
+ conditions << scope[:conditions] if scope
+ conditions << type_condition if finder_needs_type_condition?
+ merged_conditions = merge_conditions(*conditions)
+ sql << "WHERE #{merged_conditions} " unless merged_conditions.blank?
end
def type_condition
@@ -1745,7 +1758,7 @@ def with_scope(method_scoping = {}, action = :merge, &block)
(hash[method].keys + params.keys).uniq.each do |key|
merge = hash[method][key] && params[key] # merge if both scopes have the same key
if key == :conditions && merge
- hash[method][key] = [params[key], hash[method][key]].collect{ |sql| "( %s )" % sanitize_sql(sql) }.join(" AND ")
+ hash[method][key] = merge_conditions(params[key], hash[method][key])
elsif key == :include && merge
hash[method][key] = merge_includes(hash[method][key], params[key]).uniq
else
@@ -1939,7 +1952,7 @@ def sanitize_sql_hash_for_conditions(attrs)
# { :status => nil, :group_id => 1 }
# # => "status = NULL , group_id = 1"
def sanitize_sql_hash_for_assignment(attrs)
- conditions = attrs.map do |attr, value|
+ attrs.map do |attr, value|
"#{connection.quote_column_name(attr)} = #{quote_bound_value(value)}"
end.join(', ')
end
@@ -150,7 +150,7 @@ def test_append_conditions
Developer.with_scope(:find => { :conditions => "name = 'David'" }) do
Developer.with_scope(:find => { :conditions => 'salary = 80000' }) do
appended_condition = Developer.instance_eval('current_scoped_methods')[:find][:conditions]
- assert_equal("( name = 'David' ) AND ( salary = 80000 )", appended_condition)
+ assert_equal("(name = 'David') AND (salary = 80000)", appended_condition)
assert_equal(1, Developer.count)
end
Developer.with_scope(:find => { :conditions => "name = 'Maiha'" }) do
@@ -163,7 +163,7 @@ def test_merge_and_append_options
Developer.with_scope(:find => { :conditions => 'salary = 80000', :limit => 10 }) do
Developer.with_scope(:find => { :conditions => "name = 'David'" }) do
merged_option = Developer.instance_eval('current_scoped_methods')[:find]
- assert_equal({ :conditions => "( salary = 80000 ) AND ( name = 'David' )", :limit => 10 }, merged_option)
+ assert_equal({ :conditions => "(salary = 80000) AND (name = 'David')", :limit => 10 }, merged_option)
end
end
end
@@ -275,6 +275,26 @@ def test_merged_scoped_find_combines_and_sanitizes_conditions
end
end
+ def test_merged_scoped_find_on_blank_conditions
+ [nil, " ", [], {}].each do |blank|
+ Developer.with_scope(:find => {:conditions => blank}) do
+ Developer.with_scope(:find => {:conditions => blank}) do
+ assert_nothing_raised { Developer.find(:first) }
+ end
+ end
+ end
+ end
+
+ def test_merged_scoped_find_on_blank_bind_conditions
+ [ [""], ["",{}] ].each do |blank|
+ Developer.with_scope(:find => {:conditions => blank}) do
+ Developer.with_scope(:find => {:conditions => blank}) do
+ assert_nothing_raised { Developer.find(:first) }
+ end
+ end
+ end
+ end
+
def test_immutable_nested_scope
options1 = { :conditions => "name = 'Jamis'" }
options2 = { :conditions => "name = 'David'" }

0 comments on commit 0ff0319

Please sign in to comment.