Merge branch 'master' of http://github.com/rails/rails

6 files changed, 140 insertions, 131 deletions

diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb
index 08bc80dbc2..64f4d1d532 100644
--- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb
+++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb
@@ -64,6 +64,11 @@ module ActionDispatch
           super(key.to_s, value)
         end
 
+        def clear
+          load_for_write!
+          super
+        end
+
         def to_hash
           load_for_read!
           h = {}.replace(self)
diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb
index 401d98b663..c664fb0bc2 100644
--- a/actionpack/lib/action_dispatch/routing.rb
+++ b/actionpack/lib/action_dispatch/routing.rb
@@ -31,7 +31,7 @@ module ActionDispatch
   # Think of creating routes as drawing a map for your requests. The map tells
   # them where to go based on some predefined pattern:
   #
-  #   AppName::Applications.routes.draw do |map|
+  #   AppName::Application.routes.draw do |map|
   #     Pattern 1 tells some request to go to one place
   #     Pattern 2 tell them to go to another
   #     ...
@@ -62,7 +62,7 @@ module ActionDispatch
   #
   #   redirect_to show_item_path(:id => 25)
   #
-  # Use <tt>root</tt> as a shorthand to name a route for the root path "".
+  # Use <tt>root</tt> as a shorthand to name a route for the root path "/".
   #
   #   # In routes.rb
   #   root :to => 'blogs#index'
@@ -72,7 +72,7 @@ module ActionDispatch
   #
   #   # and provide these named routes
   #   root_url   # => 'http://www.example.com/'
-  #   root_path  # => ''
+  #   root_path  # => '/'
   #
   # Note: when using +controller+, the route is simply named after the
   # method you call on the block parameter rather than map.
@@ -91,9 +91,7 @@ module ActionDispatch
   #
   # Routes can generate pretty URLs. For example:
   #
-  #   match '/articles/:year/:month/:day', :constraints => {
-  #     :controller => 'articles',
-  #     :action     => 'find_by_date',
+  #   match '/articles/:year/:month/:day' => 'articles#find_by_id', :constraints => {
   #     :year       => /\d{4}/,
   #     :month      => /\d{1,2}/,
   #     :day        => /\d{1,2}/
@@ -167,7 +165,7 @@ module ActionDispatch
   #
   # You can reload routes if you feel you must:
   #
-  #   Rails::Application.reload_routes!
+  #   Rails.application.reload_routes!
   #
   # This will clear all named routes and reload routes.rb if the file has been modified from
   # last load. To absolutely force reloading, use <tt>reload!</tt>.
diff --git a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb
index bc3f62fb48..b3146a1c60 100644
--- a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb
@@ -19,9 +19,9 @@ module ActionDispatch
 
       def in_memory_controller_namespaces
         namespaces = Set.new
-        ActionController::Base.subclasses.each do |klass|
-          controller_name = klass.underscore
-          namespaces << controller_name.split('/')[0...-1].join('/')
+        ActionController::Base.descendants.each do |klass|
+          next if klass.anonymous?
+          namespaces << klass.name.underscore.split('/')[0...-1].join('/')
         end
         namespaces.delete('')
         namespaces
@@ -31,7 +31,7 @@ module ActionDispatch
     class DeprecatedMapper #:nodoc:
       def initialize(set) #:nodoc:
         ActiveSupport::Deprecation.warn "You are using the old router DSL which will be removed in Rails 3.1. " <<
-          "Please check how to update your router file at: http://www.engineyard.com/blog/2010/the-lowdown-on-routes-in-rails-3/"
+          "Please check how to update your routes file at: http://www.engineyard.com/blog/2010/the-lowdown-on-routes-in-rails-3/"
         @set = set
       end
 
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 0b4ba8c9f5..430f6fc5a3 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -13,6 +13,8 @@ module ActionDispatch
           end
         end
 
+        attr_reader :app
+
         def initialize(app, constraints, request)
           @app, @constraints, @request = app, constraints, request
         end
@@ -63,6 +65,16 @@ module ActionDispatch
               end
             end
 
+            if path.match(':controller')
+              raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module]
+
+              # Add a default constraint for :controller path segments that matches namespaced
+              # controllers with default routes like :controller/:action/:id(.:format), e.g:
+              # GET /admin/products/show/1
+              # => { :controller => 'admin/products', :action => 'show', :id => '1' }
+              options.reverse_merge!(:controller => /.+?/)
+            end
+
             path = normalize_path(path)
             path_without_format = path.sub(/\(\.:format\)$/, '')
 
@@ -133,8 +145,11 @@ module ActionDispatch
               defaults[:controller] ||= default_controller
               defaults[:action]     ||= default_action
 
-              defaults.delete(:controller) if defaults[:controller].blank?
-              defaults.delete(:action)     if defaults[:action].blank?
+              defaults.delete(:controller) if defaults[:controller].blank? || defaults[:controller].is_a?(Regexp)
+              defaults.delete(:action)     if defaults[:action].blank? || defaults[:action].is_a?(Regexp)
+
+              defaults[:controller] = defaults[:controller].to_s if defaults.key?(:controller)
+              defaults[:action] = defaults[:action].to_s if defaults.key?(:action)
 
               if defaults[:controller].blank? && segment_keys.exclude?("controller")
                 raise ArgumentError, "missing :controller"
@@ -183,15 +198,15 @@ module ActionDispatch
 
           def default_controller
             if @options[:controller]
-              @options[:controller].to_s
+              @options[:controller]
             elsif @scope[:controller]
-              @scope[:controller].to_s
+              @scope[:controller]
             end
           end
 
           def default_action
             if @options[:action]
-              @options[:action].to_s
+              @options[:action]
             end
           end
       end
@@ -431,12 +446,16 @@ module ActionDispatch
           end
 
           def merge_options_scope(parent, child)
-            (parent || {}).merge(child)
+            (parent || {}).except(*override_keys(child)).merge(child)
           end
 
           def merge_shallow_scope(parent, child)
             child ? true : false
           end
+
+          def override_keys(child)
+            child.key?(:only) || child.key?(:except) ? [:only, :except] : []
+          end
       end
 
       module Resources
@@ -444,7 +463,7 @@ module ActionDispatch
         # a path appended since they fit properly in their scope level.
         VALID_ON_OPTIONS = [:new, :collection, :member]
         CANONICAL_ACTIONS = [:index, :create, :new, :show, :update, :destroy]
-        MERGE_FROM_SCOPE_OPTIONS = [:shallow, :constraints]
+        RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except]
 
         class Resource #:nodoc:
           DEFAULT_ACTIONS = [:index, :create, :new, :show, :update, :destroy, :edit]
@@ -464,9 +483,9 @@ module ActionDispatch
           end
 
           def actions
-            if only = options[:only]
+            if only = @options[:only]
               Array(only).map(&:to_sym)
-            elsif except = options[:except]
+            elsif except = @options[:except]
               default_actions - Array(except).map(&:to_sym)
             else
               default_actions
@@ -489,68 +508,31 @@ module ActionDispatch
             singular
           end
 
-          alias_method :nested_name, :member_name
-
           # Checks for uncountable plurals, and appends "_index" if they're.
           def collection_name
             singular == plural ? "#{plural}_index" : plural
           end
 
-          def shallow?
-            options[:shallow] ? true : false
-          end
-
-          def constraints
-            options[:constraints] || {}
-          end
-
-          def id_constraint?
-            options[:id] && options[:id].is_a?(Regexp) || constraints[:id] && constraints[:id].is_a?(Regexp)
-          end
-
-          def id_constraint
-            options[:id] || constraints[:id]
-          end
-
-          def collection_options
-            (options || {}).dup.tap do |opts|
-              opts.delete(:id)
-              opts[:constraints] = options[:constraints].dup if options[:constraints]
-              opts[:constraints].delete(:id) if options[:constraints].is_a?(Hash)
-            end
-          end
-
-          def nested_path
-            "#{path}/:#{singular}_id"
-          end
-
-          def nested_options
-            {}.tap do |opts|
-              opts[:as] = member_name
-              opts["#{singular}_id".to_sym] = id_constraint if id_constraint?
-              opts[:options] = { :shallow => shallow? } unless options[:shallow].nil?
-            end
-          end
-
           def resource_scope
-            [{ :controller => controller }]
+            { :controller => controller }
           end
 
           def collection_scope
-            [path, collection_options]
+            path
           end
 
           def member_scope
-            ["#{path}/:id", options]
+            "#{path}/:id"
           end
 
           def new_scope(new_path)
-            ["#{path}/#{new_path}"]
+            "#{path}/#{new_path}"
           end
 
           def nested_scope
-            [nested_path, nested_options]
+            "#{path}/:#{singular}_id"
           end
+
         end
 
         class SingletonResource < Resource #:nodoc:
@@ -559,7 +541,7 @@ module ActionDispatch
           def initialize(entities, options)
             @name       = entities.to_s
             @path       = options.delete(:path) || @name
-            @controller = (options.delete(:controller) || @name.to_s.pluralize).to_s
+            @controller = (options.delete(:controller) || plural).to_s
             @as         = options.delete(:as)
             @options    = options
           end
@@ -569,24 +551,10 @@ module ActionDispatch
           end
           alias :collection_name :member_name
 
-          def nested_path
-            path
-          end
-
-          def nested_options
-            {}.tap do |opts|
-              opts[:as] = member_name
-              opts[:options] = { :shallow => shallow? } unless @options[:shallow].nil?
-            end
-          end
-
-          def shallow?
-            false
-          end
-
           def member_scope
-            [path, options]
+            path
           end
+          alias :nested_scope :member_scope
         end
 
         def initialize(*args) #:nodoc:
@@ -693,18 +661,18 @@ module ActionDispatch
           end
 
           with_scope_level(:nested) do
-            if parent_resource.shallow?
+            if shallow?
               with_exclusive_scope do
                 if @scope[:shallow_path].blank?
-                  scope(*parent_resource.nested_scope) { yield }
+                  scope(parent_resource.nested_scope, nested_options) { yield }
                 else
                   scope(@scope[:shallow_path], :as => @scope[:shallow_prefix]) do
-                    scope(*parent_resource.nested_scope) { yield }
+                    scope(parent_resource.nested_scope, nested_options) { yield }
                   end
                 end
               end
             else
-              scope(*parent_resource.nested_scope) { yield }
+              scope(parent_resource.nested_scope, nested_options) { yield }
             end
           end
         end
@@ -723,6 +691,10 @@ module ActionDispatch
           end
         end
 
+        def shallow?
+          parent_resource.instance_of?(Resource) && @scope[:shallow]
+        end
+
         def match(*args)
           options = args.extract_options!.dup
           options[:anchor] = true unless options.key?(:anchor)
@@ -800,16 +772,17 @@ module ActionDispatch
               return true
             end
 
-            if path_names = options.delete(:path_names)
-              scope(:path_names => path_names) do
+            scope_options = options.slice!(*RESOURCE_OPTIONS)
+            unless scope_options.empty?
+              scope(scope_options) do
                 send(method, resources.pop, options, &block)
               end
               return true
             end
 
-            scope_options = @scope.slice(*MERGE_FROM_SCOPE_OPTIONS).delete_if{ |k,v| v.blank? }
-            options.reverse_merge!(scope_options) unless scope_options.empty?
-            options.reverse_merge!(@scope[:options]) unless @scope[:options].blank?
+            unless action_options?(options)
+              options.merge!(scope_action_options) if scope_action_options?
+            end
 
             if resource_scope?
               nested do
@@ -821,6 +794,18 @@ module ActionDispatch
             false
           end
 
+          def action_options?(options)
+            options[:only] || options[:except]
+          end
+
+          def scope_action_options?
+            @scope[:options].is_a?(Hash) && (@scope[:options][:only] || @scope[:options][:except])
+          end
+
+          def scope_action_options
+            @scope[:options].slice(:only, :except)
+          end
+
           def resource_scope?
             [:resource, :resources].include?(@scope[:scope_level])
           end
@@ -853,7 +838,7 @@ module ActionDispatch
 
           def resource_scope(resource)
             with_scope_level(resource.is_a?(SingletonResource) ? :resource : :resources, resource) do
-              scope(*parent_resource.resource_scope) do
+              scope(parent_resource.resource_scope) do
                 yield
               end
             end
@@ -861,7 +846,7 @@ module ActionDispatch
 
           def new_scope
             with_scope_level(:new) do
-              scope(*parent_resource.new_scope(action_path(:new))) do
+              scope(parent_resource.new_scope(action_path(:new))) do
                 yield
               end
             end
@@ -869,7 +854,7 @@ module ActionDispatch
 
           def collection_scope
             with_scope_level(:collection) do
-              scope(*parent_resource.collection_scope) do
+              scope(parent_resource.collection_scope) do
                 yield
               end
             end
@@ -877,18 +862,33 @@ module ActionDispatch
 
           def member_scope
             with_scope_level(:member) do
-              scope(*parent_resource.member_scope) do
+              scope(parent_resource.member_scope) do
                 yield
               end
             end
           end
 
+          def nested_options
+            {}.tap do |options|
+              options[:as] = parent_resource.member_name
+              options[:constraints] = { "#{parent_resource.singular}_id".to_sym => id_constraint } if id_constraint?
+            end
+          end
+
+          def id_constraint?
+            @scope[:id].is_a?(Regexp) || (@scope[:constraints] && @scope[:constraints][:id].is_a?(Regexp))
+          end
+
+          def id_constraint
+            @scope[:id] || @scope[:constraints][:id]
+          end
+
           def canonical_action?(action, flag)
             flag && CANONICAL_ACTIONS.include?(action)
           end
 
           def shallow_scoping?
-            parent_resource && parent_resource.shallow? && @scope[:scope_level] == :member
+            shallow? && @scope[:scope_level] == :member
           end
 
           def path_for_action(action, path)
@@ -932,7 +932,7 @@ module ActionDispatch
               collection_name = parent_resource.collection_name
               member_name = parent_resource.member_name
               name_prefix = "#{name_prefix}_" if name_prefix.present?
-            end 
+            end
 
             case @scope[:scope_level]
             when :collection
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 5ecad6bc04..1b1a221c60 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -26,7 +26,7 @@ module ActionDispatch
             return [404, {'X-Cascade' => 'pass'}, []]
           end
 
-          controller.action(params[:action]).call(env)
+          dispatch(controller, params[:action], env)
         end
 
         def prepare_params!(params)
@@ -34,29 +34,43 @@ module ActionDispatch
           split_glob_param!(params) if @glob_param
         end
 
-        def controller(params, raise_error=true)
+        # If this is a default_controller (i.e. a controller specified by the user)
+        # we should raise an error in case it's not found, because it usually means
+        # an user error. However, if the controller was retrieved through a dynamic
+        # segment, as in :controller(/:action), we should simply return nil and
+        # delegate the control back to Rack cascade. Besides, if this is not a default
+        # controller, it means we should respect the @scope[:module] parameter.
+        def controller(params, default_controller=true)
           if params && params.key?(:controller)
             controller_param = params[:controller]
-            unless controller = @controllers[controller_param]
-              controller_name = "#{controller_param.camelize}Controller"
-              controller = @controllers[controller_param] =
-                ActiveSupport::Dependencies.ref(controller_name)
-            end
-
-            controller.get
+            controller_reference(controller_param)
           end
         rescue NameError => e
-          raise ActionController::RoutingError, e.message, e.backtrace if raise_error
+          raise ActionController::RoutingError, e.message, e.backtrace if default_controller
         end
 
-        private
-          def merge_default_action!(params)
-            params[:action] ||= 'index'
-          end
+      private
 
-          def split_glob_param!(params)
-            params[@glob_param] = params[@glob_param].split('/').map { |v| URI.unescape(v) }
+        def controller_reference(controller_param)
+          unless controller = @controllers[controller_param]
+            controller_name = "#{controller_param.camelize}Controller"
+            controller = @controllers[controller_param] =
+              ActiveSupport::Dependencies.ref(controller_name)
           end
+          controller.get
+        end
+
+        def dispatch(controller, action, env)
+          controller.action(action).call(env)
+        end
+
+        def merge_default_action!(params)
+          params[:action] ||= 'index'
+        end
+
+        def split_glob_param!(params)
+          params[@glob_param] = params[@glob_param].split('/').map { |v| URI.unescape(v) }
+        end
       end
 
       # A NamedRouteCollection instance is a collection of named routes, and also
@@ -268,10 +282,10 @@ module ActionDispatch
             # Yes plz - JP
             included do
               routes.install_helpers(self)
-              singleton_class.send(:define_method, :_router) { routes }
+              singleton_class.send(:define_method, :_routes) { routes }
             end
 
-            define_method(:_router) { routes }
+            define_method(:_routes) { routes }
           end
 
           helpers
@@ -302,7 +316,6 @@ module ActionDispatch
           @extras      = extras
 
           normalize_options!
-          normalize_recall!
           normalize_controller_action_id!
           use_relative_controller!
           controller.sub!(%r{^/}, '') if controller
@@ -319,7 +332,11 @@ module ActionDispatch
 
         def use_recall_for(key)
           if @recall[key] && (!@options.key?(key) || @options[key] == @recall[key])
-            @options[key] = @recall.delete(key)
+            if named_route_exists?
+              @options[key] = @recall.delete(key) if segment_keys.include?(key)
+            else
+              @options[key] = @recall.delete(key)
+            end
           end
         end
 
@@ -343,15 +360,6 @@ module ActionDispatch
           end
         end
 
-        def normalize_recall!
-          # If the target route is not a standard route then remove controller and action
-          # from the options otherwise they will appear in the url parameters
-          if block_or_proc_route_target?
-            recall.delete(:controller) unless segment_keys.include?(:controller)
-            recall.delete(:action) unless segment_keys.include?(:action)
-          end
-        end
-
         # This pulls :controller, :action, and :id out of the recall.
         # The recall key is only used if there is no key in the options
         # or if the key in the options is identical. If any of
@@ -424,12 +432,8 @@ module ActionDispatch
             named_route && set.named_routes[named_route]
           end
 
-          def block_or_proc_route_target?
-            named_route_exists? && !set.named_routes[named_route].app.is_a?(Dispatcher)
-          end
-
           def segment_keys
-            named_route_exists? ? set.named_routes[named_route].segment_keys : []
+            set.named_routes[named_route].segment_keys
           end
       end
 
@@ -474,7 +478,7 @@ module ActionDispatch
         path_options = yield(path_options) if block_given?
         path = generate(path_options, path_segments || {})
 
-        # ROUTES TODO: This can be called directly, so script_name should probably be set in the router
+        # ROUTES TODO: This can be called directly, so script_name should probably be set in the routes
         rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
         rewritten_url << "##{Rack::Mount::Utils.escape_uri(options[:anchor].to_param.to_s)}" if options[:anchor]
 
@@ -506,6 +510,8 @@ module ActionDispatch
           end
 
           dispatcher = route.app
+          dispatcher = dispatcher.app while dispatcher.is_a?(Mapper::Constraints)
+
           if dispatcher.is_a?(Dispatcher) && dispatcher.controller(params, false)
             dispatcher.prepare_params!(params)
             return params
diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb
index db17615d92..980abd44df 100644
--- a/actionpack/lib/action_dispatch/routing/url_for.rb
+++ b/actionpack/lib/action_dispatch/routing/url_for.rb
@@ -128,7 +128,7 @@ module ActionDispatch
         when String
           options
         when nil, Hash
-          _router.url_for(url_options.merge((options || {}).symbolize_keys))
+          _routes.url_for(url_options.merge((options || {}).symbolize_keys))
         else
           polymorphic_url(options)
         end