Merge pull request #7422 from ernie/improvements-to-improved-routing

Allow routing concerns to accept a callable

3 files changed, 103 insertions, 14 deletions

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index b52f66faf1..ddb34a2394 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -1585,7 +1585,7 @@ module ActionDispatch
           end
       end
 
-      # Routing Concerns allows you to declare common routes that can be reused
+      # Routing Concerns allow you to declare common routes that can be reused
       # inside others resources and routes.
       #
       #   concern :commentable do
@@ -1608,13 +1608,63 @@ module ActionDispatch
       module Concerns
         # Define a routing concern using a name.
         #
-        #   concern :commentable do
-        #     resources :comments
+        # Concerns may be defined inline, using a block, or handled by
+        # another object, by passing that object as the second parameter.
+        #
+        # The concern object, if supplied, should respond to <tt>call</tt>,
+        # which will receive two parameters:
+        #
+        #   * The current mapper
+        #   * A hash of options which the concern object may use
+        #
+        # Options may also be used by concerns defined in a block by accepting
+        # a block parameter. So, using a block, you might do something as
+        # simple as limit the actions available on certain resources, passing
+        # standard resource options through the concern:
+        #
+        #   concern :commentable do |options|
+        #     resources :comments, options
         #   end
         #
-        # Any routing helpers can be used inside a concern.
-        def concern(name, &block)
-          @concerns[name] = block
+        #   resources :posts, concerns: :commentable
+        #   resources :archived_posts do
+        #     # Don't allow comments on archived posts
+        #     concerns :commentable, only: [:index, :show]
+        #   end
+        #
+        # Or, using a callable object, you might implement something more
+        # specific to your application, which would be out of place in your
+        # routes file.
+        #
+        #   # purchasable.rb
+        #   class Purchasable
+        #     def initialize(defaults = {})
+        #       @defaults = defaults
+        #     end
+        #
+        #     def call(mapper, options = {})
+        #       options = @defaults.merge(options)
+        #       mapper.resources :purchases
+        #       mapper.resources :receipts
+        #       mapper.resources :returns if options[:returnable]
+        #     end
+        #   end
+        #
+        #   # routes.rb
+        #   concern :purchasable, Purchasable.new(returnable: true)
+        #
+        #   resources :toys, concerns: :purchasable
+        #   resources :electronics, concerns: :purchasable
+        #   resources :pets do
+        #     concerns :purchasable, returnable: false
+        #   end
+        #
+        # Any routing helpers can be used inside a concern. If using a
+        # callable, they're accessible from the Mapper that's passed to
+        # <tt>call</tt>.
+        def concern(name, callable = nil, &block)
+          callable ||= lambda { |mapper, options| mapper.instance_exec(options, &block) }
+          @concerns[name] = callable
         end
 
         # Use the named concerns
@@ -1628,10 +1678,11 @@ module ActionDispatch
         #   namespace :posts do
         #     concerns :commentable
         #   end
-        def concerns(*names)
-          names.flatten.each do |name|
+        def concerns(*args)
+          options = args.extract_options!
+          args.flatten.each do |name|
             if concern = @concerns[name]
-              instance_eval(&concern)
+              concern.call(self, options)
             else
               raise ArgumentError, "No concern named #{name} was found!"
             end
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index e5054a9eb8..4f5b2895c9 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -358,6 +358,7 @@ end
 class ThreadsController  < ResourcesController; end
 class MessagesController < ResourcesController; end
 class CommentsController < ResourcesController; end
+class ReviewsController < ResourcesController; end
 class AuthorsController < ResourcesController; end
 class LogosController < ResourcesController; end
 
diff --git a/actionpack/test/dispatch/routing/concerns_test.rb b/actionpack/test/dispatch/routing/concerns_test.rb
index 21da3bd77a..9f37701656 100644
--- a/actionpack/test/dispatch/routing/concerns_test.rb
+++ b/actionpack/test/dispatch/routing/concerns_test.rb
@@ -1,18 +1,28 @@
 require 'abstract_unit'
 
 class RoutingConcernsTest < ActionDispatch::IntegrationTest
+  class Reviewable
+    def self.call(mapper, options = {})
+      mapper.resources :reviews, options
+    end
+  end
+
   Routes = ActionDispatch::Routing::RouteSet.new.tap do |app|
     app.draw do
-      concern :commentable do
-        resources :comments
+      concern :commentable do |options|
+        resources :comments, options
       end
 
       concern :image_attachable do
         resources :images, only: :index
       end
 
-      resources :posts, concerns: [:commentable, :image_attachable] do
-        resource :video, concerns: :commentable
+      concern :reviewable, Reviewable
+
+      resources :posts, concerns: [:commentable, :image_attachable, :reviewable] do
+        resource :video, concerns: :commentable do
+          concerns :reviewable, as: :video_reviews
+        end
       end
 
       resource :picture, concerns: :commentable do
@@ -20,7 +30,7 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest
       end
 
       scope "/videos" do
-        concerns :commentable
+        concerns :commentable, except: :destroy
       end
     end
   end
@@ -63,11 +73,28 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest
     assert_equal "404", @response.code
   end
 
+  def test_accessing_callable_concern_
+    get "/posts/1/reviews/1"
+    assert_equal "200", @response.code
+    assert_equal "/posts/1/reviews/1", post_review_path(post_id: 1, id: 1)
+  end
+
+  def test_callable_concerns_accept_options
+    get "/posts/1/video/reviews/1"
+    assert_equal "200", @response.code
+    assert_equal "/posts/1/video/reviews/1", post_video_video_review_path(post_id: 1, id: 1)
+  end
+
   def test_accessing_concern_from_a_scope
     get "/videos/comments"
     assert_equal "200", @response.code
   end
 
+  def test_concerns_accept_options
+    delete "/videos/comments/1"
+    assert_equal "404", @response.code
+  end
+
   def test_with_an_invalid_concern_name
     e = assert_raise ArgumentError do
       ActionDispatch::Routing::RouteSet.new.tap do |app|
@@ -79,4 +106,14 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest
 
     assert_equal "No concern named foo was found!", e.message
   end
+
+  def test_concerns_executes_block_in_context_of_current_mapper
+    mapper = ActionDispatch::Routing::Mapper.new(ActionDispatch::Routing::RouteSet.new)
+    mapper.concern :test_concern do
+      resources :things
+      return self
+    end
+
+    assert_equal mapper, mapper.concerns(:test_concern)
+  end
 end