Make enhanced routing Concerns more tell-don't-ask

2 files changed, 70 insertions, 37 deletions

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 8573f4d80b..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
@@ -1606,32 +1606,65 @@ module ActionDispatch
       #     concerns :commentable
       #   end
       module Concerns
-        # Define a routing concern using a name. If a second parameter is
-        # supplied, it should respond to call, which will receive the mapper
-        # as a parameter, allowing for customized behavior based on the current
-        # scope.
+        # 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
+        #
+        #   resources :posts, concerns: :commentable
+        #   resources :archived_posts do
+        #     # Don't allow comments on archived posts
+        #     concerns :commentable, only: [:index, :show]
         #   end
         #
-        #   # - or -
+        # Or, using a callable object, you might implement something more
+        # specific to your application, which would be out of place in your
+        # routes file.
         #
-        #   class Commentable
-        #     def self.call(mapper)
-        #       if mapper.current_scope[:controller] == 'videos'
-        #         mapper.resources :video_comments, as: :comments
-        #       else
-        #         mapper.resources :comments
-        #       end
+        #   # 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
         #
-        #   concern :commentable, Commentable
+        #   # 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.
+        # 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)
-          @concerns[name] = callable || lambda { |m| m.instance_eval(&block) }
+          callable ||= lambda { |mapper, options| mapper.instance_exec(options, &block) }
+          @concerns[name] = callable
         end
 
         # Use the named concerns
@@ -1645,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]
-              concern.call(self)
+              concern.call(self, options)
             else
               raise ArgumentError, "No concern named #{name} was found!"
             end
@@ -1662,10 +1696,6 @@ module ActionDispatch
         @concerns = {}
       end
 
-      def current_scope
-        @scope
-      end
-
       include Base
       include HttpHelpers
       include Redirection
diff --git a/actionpack/test/dispatch/routing/concerns_test.rb b/actionpack/test/dispatch/routing/concerns_test.rb
index 0289f38ba7..9f37701656 100644
--- a/actionpack/test/dispatch/routing/concerns_test.rb
+++ b/actionpack/test/dispatch/routing/concerns_test.rb
@@ -2,19 +2,15 @@ require 'abstract_unit'
 
 class RoutingConcernsTest < ActionDispatch::IntegrationTest
   class Reviewable
-    def self.call(mapper)
-      if mapper.current_scope[:controller] == 'posts'
-        mapper.resources :reviews
-      elsif mapper.current_scope[:controller] == 'videos'
-        mapper.resources :reviews, as: :video_reviews
-      end
+    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
@@ -24,7 +20,9 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest
       concern :reviewable, Reviewable
 
       resources :posts, concerns: [:commentable, :image_attachable, :reviewable] do
-        resource :video, concerns: [:commentable, :reviewable]
+        resource :video, concerns: :commentable do
+          concerns :reviewable, as: :video_reviews
+        end
       end
 
       resource :picture, concerns: :commentable do
@@ -32,7 +30,7 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest
       end
 
       scope "/videos" do
-        concerns :commentable
+        concerns :commentable, except: :destroy
       end
     end
   end
@@ -75,13 +73,13 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest
     assert_equal "404", @response.code
   end
 
-  def test_accessing_callable_concern_from_resources
+  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_concern_can_adapt_to_mapper
+  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)
@@ -92,6 +90,11 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest
     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|