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

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9082 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

2 files changed, 43 insertions, 10 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 4531c12110..9716473a4e 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1426,6 +1426,20 @@ module ActiveRecord #:nodoc:
          (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 @@ module ActiveRecord #:nodoc:
         # 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 @@ module ActiveRecord #:nodoc:
                     (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 @@ module ActiveRecord #:nodoc:
         #   { :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
diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb
index 478050f207..4b5bd6c951 100644
--- a/activerecord/test/cases/method_scoping_test.rb
+++ b/activerecord/test/cases/method_scoping_test.rb
@@ -150,7 +150,7 @@ class NestedScopingTest < ActiveRecord::TestCase
     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 @@ class NestedScopingTest < ActiveRecord::TestCase
     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 @@ class NestedScopingTest < ActiveRecord::TestCase
     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'" }