Added support for nested scopes (closes #3407) [anna@wota.jp]

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

activerecord/CHANGELOG

@@ -1,5 +1,21 @@
 *SVN*
 
+* Added support for nested scopes #3407 [anna@wota.jp]. Examples:
+
+    Developer.with_scope(:find => { :conditions => "salary > 10000", :limit => 10 }) do
+      Developer.find(:all)     # => SELECT * FROM developers WHERE (salary > 10000) LIMIT 10
+
+      # inner rule is used. (all previous parameters are ignored)
+      Developer.with_exclusive_scope(:find => { :conditions => "name = 'Jamis'" }) do
+        Developer.find(:all)   # => SELECT * FROM developers WHERE (name = 'Jamis')
+      end
+
+      # parameters are merged
+      Developer.with_scope(:find => { :conditions => "name = 'Jamis'" }) do
+        Developer.find(:all)   # => SELECT * FROM developers WHERE (( salary > 10000 ) AND ( name = 'Jamis' )) LIMIT 10
+      end
+    end
+
 * Fixed db2 connection with empty user_name and auth options #3622 [phurley@gmail.com]
 
 * Fixed validates_length_of to work on UTF-8 strings by using characters instead of bytes #3699 [Masao Mutoh]

activerecord/lib/active_record/base.rb

@@ -828,20 +828,40 @@ def silence
       end
 
       # Scope parameters to method calls within the block.  Takes a hash of method_name => parameters hash.
-      # method_name may be :find or :create.
-      # :find parameters may include the <tt>:conditions</tt>, <tt>:joins</tt>,
-      # <tt>:offset</tt>, <tt>:limit</tt>, and <tt>:readonly</tt> options.
-      # :create parameters are an attributes hash.
+      # method_name may be :find or :create. :find parameters may include the <tt>:conditions</tt>, <tt>:joins</tt>,
+      # <tt>:offset</tt>, <tt>:limit</tt>, and <tt>:readonly</tt> options. :create parameters are an attributes hash.
       #
       #   Article.with_scope(:find => { :conditions => "blog_id = 1" }, :create => { :blog_id => 1 }) do
       #     Article.find(1) # => SELECT * from articles WHERE blog_id = 1 AND id = 1
       #     a = Article.create(1)
-      #     a.blog_id == 1
+      #     a.blog_id # => 1
       #   end
-      def with_scope(method_scoping = {})
+      #
+      # In nested scopings, all previous parameters are overwritten by inner rule
+      # except :conditions in :find, that are merged as hash.
+      #
+      #   Article.with_scope(:find => { :conditions => "blog_id = 1", :limit => 1 }, :create => { :blog_id => 1 }) do
+      #     Article.with_scope(:find => { :limit => 10})
+      #       Article.find(:all) # => SELECT * from articles WHERE blog_id = 1 LIMIT 10
+      #     end
+      #     Article.with_scope(:find => { :conditions => "author_id = 3" })
+      #       Article.find(:all) # => SELECT * from articles WHERE blog_id = 1 AND author_id = 3 LIMIT 1
+      #     end
+      #   end
+      #
+      # You can ignore any previous scopings by using <tt>with_exclusive_scope</tt> method.
+      #
+      #   Article.with_scope(:find => { :conditions => "blog_id = 1", :limit => 1 }) do
+      #     Article.with_exclusive_scope(:find => { :limit => 10 })
+      #       Article.find(:all) # => SELECT * from articles LIMIT 10
+      #     end
+      #   end
+      def with_scope(method_scoping = {}, action = :merge, &block)
+        method_scoping = method_scoping.method_scoping if method_scoping.respond_to?(:method_scoping)
+
         # Dup first and second level of hash (method and params).
         method_scoping = method_scoping.inject({}) do |hash, (method, params)|
-          hash[method] = params.dup
+          hash[method] = (params == true) ? params : params.dup
           hash
         end
 
@@ -852,12 +872,40 @@ def with_scope(method_scoping = {})
           f[:readonly] = true if !f[:joins].blank? && !f.has_key?(:readonly)
         end
 
-        raise ArgumentError, "Nested scopes are not yet supported: #{scoped_methods.inspect}" unless scoped_methods.nil?
+        # Merge scopings
+        if action == :merge && current_scoped_methods
+          method_scoping = current_scoped_methods.inject(method_scoping) do |hash, (method, params)|
+            case hash[method]
+              when Hash
+                if method == :find && hash[method][:conditions] && params[:conditions]
+                  (hash[method].keys + params.keys).uniq.each do |key|
+                    if key == :conditions
+                      hash[method][key] = [params[key], hash[method][key]].collect{|sql| "( %s )" % sanitize_sql(sql)}.join(" AND ")
+                    else
+                      hash[method][key] = hash[method][key] || params[key]
+                    end
+                  end
+                else
+                  hash[method] = params.merge(hash[method])
+                end
+              else
+                hash[method] = params
+            end
+            hash
+          end
+        end
 
-        self.scoped_methods = method_scoping
-        yield
-      ensure 
-        self.scoped_methods = nil
+        self.scoped_methods << method_scoping
+
+        begin
+          yield
+        ensure 
+          self.scoped_methods.pop
+        end
+      end
+
+      def with_exclusive_scope(method_scoping = {}, &block)
+        with_scope(method_scoping, :overwrite, &block)
       end
 
       # Overwrite the default class equality method to provide support for association proxies.
@@ -1076,32 +1124,27 @@ def subclasses
 
         # Test whether the given method and optional key are scoped.
         def scoped?(method, key = nil)
-          scoped_methods and scoped_methods.has_key?(method) and (key.nil? or scope(method).has_key?(key))
+          current_scoped_methods && current_scoped_methods.has_key?(method) && (key.nil? || scope(method).has_key?(key))
         end
 
         # Retrieve the scope for the given method and optional key.
         def scope(method, key = nil)
-          if scoped_methods and scope = scoped_methods[method]
+          if current_scoped_methods && scope = current_scoped_methods[method]
             key ? scope[key] : scope
           end
         end
 
         def scoped_methods
           if allow_concurrency
             Thread.current[:scoped_methods] ||= {}
-            Thread.current[:scoped_methods][self] ||= nil
+            Thread.current[:scoped_methods][self] ||= []
           else
-            @scoped_methods ||= nil
+            @scoped_methods ||= []
           end
         end
 
-        def scoped_methods=(value)
-          if allow_concurrency
-            Thread.current[:scoped_methods] ||= {}
-            Thread.current[:scoped_methods][self] = value
-          else
-            @scoped_methods = value
-          end
+        def current_scoped_methods
+          scoped_methods.last
         end
 
         # Returns the class type of the record using the current module as a prefix. So descendents of

activerecord/test/method_scoping_test.rb

@@ -9,7 +9,7 @@ class MethodScopingTest < Test::Unit::TestCase
 
   def test_set_conditions
     Developer.with_scope(:find => { :conditions => 'just a test...' }) do
-      assert_equal 'just a test...', Thread.current[:scoped_methods][Developer][:find][:conditions]
+      assert_equal 'just a test...', Thread.current[:scoped_methods][Developer][-1][:find][:conditions]
     end
   end
 
@@ -64,7 +64,7 @@ def test_scoped_create
     new_comment = nil
 
     VerySpecialComment.with_scope(:create => { :post_id => 1 }) do
-      assert_equal({ :post_id => 1 }, Thread.current[:scoped_methods][VerySpecialComment][:create])
+      assert_equal({ :post_id => 1 }, Thread.current[:scoped_methods][VerySpecialComment][-1][:create])
       new_comment = VerySpecialComment.create :body => "Wonderful world"
     end
 
@@ -87,11 +87,167 @@ def test_immutable_scope
     end
   end
 
-  def test_raise_on_nested_scope
-    Developer.with_scope(:find => { :conditions => '1=1' }) do
-      assert_raise(ArgumentError) do
-        Developer.with_scope(:find => { :conditions => '2=2' }) { }
+  def test_scoped_with_duck_typing
+    scoping = Struct.new(:method_scoping).new(:find => { :conditions => ["name = ?", 'David'] })
+    Developer.with_scope(scoping) do
+       assert_equal %w(David), Developer.find(:all).map { |d| d.name }
+    end
+  end
+
+  def test_ensure_that_method_scoping_is_correctly_restored
+    scoped_methods = Developer.instance_eval('current_scoped_methods')
+
+    begin
+      Developer.with_scope(:find => { :conditions => "name = 'Jamis'" }) do
+        raise "an exception"
+      end
+    rescue
+    end
+    assert_equal scoped_methods, Developer.instance_eval('current_scoped_methods')
+  end
+end
+
+class NestedScopingTest < Test::Unit::TestCase
+  fixtures :developers, :comments, :posts
+
+  def test_merge_options
+    Developer.with_scope(:find => { :conditions => 'salary = 80000' }) do
+      Developer.with_scope(:find => { :limit => 10 }) do
+        merged_option = Developer.instance_eval('current_scoped_methods')[:find]
+        assert_equal({ :conditions => 'salary = 80000', :limit => 10 }, merged_option)
+      end
+    end
+  end
+
+  def test_replace_options
+    Developer.with_scope(:find => { :conditions => "name = 'David'" }) do
+      Developer.with_exclusive_scope(:find => { :conditions => "name = 'Jamis'" }) do
+        assert_equal({:find => { :conditions => "name = 'Jamis'" }}, Developer.instance_eval('current_scoped_methods'))
+        assert_equal({:find => { :conditions => "name = 'Jamis'" }}, Thread.current[:scoped_methods][Developer][-1])
+      end
+    end
+  end
+
+  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(1, Developer.count)
+      end
+      Developer.with_scope(:find => { :conditions => "name = 'Maiha'" }) do
+        assert_equal(0, Developer.count)
+      end
+    end
+  end
+
+  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)
+      end
+    end
+  end
+
+  def test_nested_scoped_find
+    Developer.with_scope(:find => { :conditions => "name = 'Jamis'" }) do
+      Developer.with_exclusive_scope(:find => { :conditions => "name = 'David'" }) do
+        assert_nothing_raised { Developer.find(1) }
+        assert_equal('David', Developer.find(:first).name)
+      end
+      assert_equal('Jamis', Developer.find(:first).name)
+    end
+  end
+
+  def test_three_level_nested_exclusive_scoped_find
+    Developer.with_scope(:find => { :conditions => "name = 'Jamis'" }) do
+      assert_equal('Jamis', Developer.find(:first).name)
+
+      Developer.with_exclusive_scope(:find => { :conditions => "name = 'David'" }) do
+        assert_equal('David', Developer.find(:first).name)
+
+        Developer.with_exclusive_scope(:find => { :conditions => "name = 'Maiha'" }) do
+          assert_equal(nil, Developer.find(:first))
+        end
+
+        # ensure that scoping is restored
+        assert_equal('David', Developer.find(:first).name)
+      end
+
+      # ensure that scoping is restored
+      assert_equal('Jamis', Developer.find(:first).name)
+    end
+  end
+
+  def test_merged_scoped_find
+    poor_jamis = developers(:poor_jamis)
+    Developer.with_scope(:find => { :conditions => "salary < 100000" }) do
+      Developer.with_scope(:find => { :offset => 1 }) do
+        assert_equal(poor_jamis, Developer.find(:first))
+      end
+    end
+  end
+
+  def test_merged_scoped_find_sanitizes_conditions
+    Developer.with_scope(:find => { :conditions => ["name = ?", 'David'] }) do
+      Developer.with_scope(:find => { :conditions => ['salary = ?', 9000] }) do
+        assert_raise(ActiveRecord::RecordNotFound) { developers(:poor_jamis) }
+      end
+    end
+  end
+
+  def test_nested_scoped_find_combines_and_sanitizes_conditions
+    Developer.with_scope(:find => { :conditions => ["name = ?", 'David'] }) do
+      Developer.with_exclusive_scope(:find => { :conditions => ['salary = ?', 9000] }) do
+        assert_equal developers(:poor_jamis), Developer.find(:first)
+        assert_equal developers(:poor_jamis), Developer.find(:first, :conditions => ['name = ?', 'Jamis'])
+      end
+    end
+  end
+
+  def test_merged_scoped_find_combines_and_sanitizes_conditions
+    Developer.with_scope(:find => { :conditions => ["name = ?", 'David'] }) do
+      Developer.with_scope(:find => { :conditions => ['salary > ?', 9000] }) do
+        assert_equal %w(David), Developer.find(:all).map { |d| d.name }
+      end
+    end
+  end
+
+  def test_immutable_nested_scope
+    options1 = { :conditions => "name = 'Jamis'" }
+    options2 = { :conditions => "name = 'David'" }
+    Developer.with_scope(:find => options1) do
+      Developer.with_exclusive_scope(:find => options2) do
+        assert_equal %w(David), Developer.find(:all).map { |d| d.name }
+        options1[:conditions] = options2[:conditions] = nil
+        assert_equal %w(David), Developer.find(:all).map { |d| d.name }
+      end
+    end
+  end
+
+  def test_immutable_merged_scope
+    options1 = { :conditions => "name = 'Jamis'" }
+    options2 = { :conditions => "salary > 10000" }
+    Developer.with_scope(:find => options1) do
+      Developer.with_scope(:find => options2) do
+        assert_equal %w(Jamis), Developer.find(:all).map { |d| d.name }
+        options1[:conditions] = options2[:conditions] = nil
+        assert_equal %w(Jamis), Developer.find(:all).map { |d| d.name }
+      end
+    end
+  end
+
+  def test_ensure_that_method_scoping_is_correctly_restored
+    Developer.with_scope(:find => { :conditions => "name = 'David'" }) do
+      scoped_methods = Developer.instance_eval('current_scoped_methods')
+      begin
+        Developer.with_scope(:find => { :conditions => "name = 'Maiha'" }) do
+          raise "an exception"
+        end
+      rescue
       end
+      assert_equal scoped_methods, Developer.instance_eval('current_scoped_methods')
     end
   end
 end
@@ -118,9 +274,9 @@ def test_forwarding_to_dynamic_finders
     assert_equal 2, @welcome.comments.find_all_by_type('Comment').size
   end
 
-  def test_raise_on_nested_scope
+  def test_nested_scope
     Comment.with_scope(:find => { :conditions => '1=1' }) do
-      assert_raise(ArgumentError) { @welcome.comments.what_are_you }
+      assert_equal 'a comment...', @welcome.comments.what_are_you
     end
   end
 end
@@ -144,9 +300,9 @@ def test_forwarding_to_dynamic_finders
     assert_equal 2, @welcome.categories.find_all_by_type('Category').size
   end
 
-  def test_raise_on_nested_scope
+  def test_nested_scope
     Category.with_scope(:find => { :conditions => '1=1' }) do
-      assert_raise(ArgumentError) { @welcome.categories.what_are_you }
+      assert_equal 'a comment...', @welcome.comments.what_are_you
     end
   end
 end