Merged back the Routing branch

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

actionpack/lib/action_controller/assertions/action_pack_assertions.rb

@@ -141,7 +141,7 @@ def assert_redirected_to(options = {}, message=nil)
           end
         end
       end
-
+      
       # ensure our redirection url is an exact match
       def assert_redirect_url(url=nil, message=nil)
         assert_redirect(message)
@@ -158,6 +158,36 @@ def assert_redirect_url_match(pattern=nil, message=nil)
         assert_block(msg) { response.redirect_url_match?(pattern) }
       end
 
+      # -- routing assertions --------------------------------------------------
+
+      # Asserts that the routing of the given path is handled correctly and that the parsed options match.
+      # Also verifies that the provided options can be used to generate the provided path.
+      def assert_routing(path, options, defaults={}, extras={}, message=nil)
+        defaults[:controller] ||= options[:controller] # Assume given controller,
+        request = ActionController::TestRequest.new({}, {}, nil)
+        request.path_parameters = defaults
+
+        ActionController::Routing::Routes.reload if ActionController::Routing::Routes.empty? # Load routes.rb if it hasn't been loaded.
+
+        generated_path, found_extras = ActionController::Routing::Routes.generate(defaults.merge(options), request)
+        generated_path = generated_path.join('/')
+        msg = build_message(message, "found extras <?>, not <?>", found_extras, extras)
+        assert_block(msg) { found_extras == extras }
+
+        msg = build_message(message, "The generated path <?> did not match <?>", generated_path, path)
+        assert_block(msg) { path == generated_path }
+
+        request = ActionController::TestRequest.new({}, {}, nil)
+        request.path = path
+        ActionController::Routing::Routes.recognize!(request)
+
+        expected_options = options.clone
+        extras.each {|k,v| expected_options.delete k}
+
+        msg = build_message(message, "The recognized options <?> did not match <?>", request.path_parameters, expected_options)
+        assert_block(msg) { request.path_parameters == expected_options }
+      end
+
       # -- template assertions ------------------------------------------------
 
       # ensure that a template object with the given name exists

actionpack/lib/action_controller/base.rb

@@ -13,6 +13,13 @@ class SessionRestoreError < ActionControllerError #:nodoc:
   end
   class MissingTemplate < ActionControllerError #:nodoc:
   end
+  class RoutingError < ActionControllerError
+    attr_reader :failures
+    def initialize(message, failures=[])
+      super(message)
+      @failures = failures
+    end
+  end
   class UnknownAction < ActionControllerError #:nodoc:
   end
   class MissingFile < ActionControllerError #:nodoc:
@@ -205,6 +212,12 @@ class Base
     # should instead be implemented in the controller to determine when debugging screens should be shown.
     @@consider_all_requests_local = true
     cattr_accessor :consider_all_requests_local
+
+    # Enable or disable the collection of failure information for RoutingErrors.
+    # This information can be extremely useful when tweaking custom routes, but is
+    # pointless once routes have been tested and verified.
+    @@debug_routes = true
+    cattr_accessor :debug_routes
 
     # Template root determines the base from which template references will be made. So a call to render("test/template")
     # will be converted to "#{template_root}/test/template.rhtml".
@@ -261,6 +274,14 @@ def controller_class_name
       def controller_name
         Inflector.underscore(controller_class_name.sub(/Controller/, ""))
       end
+
+      # Convert the class name from something like "OneModule::TwoModule::NeatController" to "one_module/two_module/neat".
+      def controller_path
+        components = self.name.to_s.split('::').collect { |name| name.underscore }
+        components[-1] = $1 if /^(.*)_controller$/ =~ components[-1]
+        components.shift if components.first == 'controllers' # Transitional conditional to accomodate root Controllers module
+        components.join('/')
+      end
     end
 
     public
@@ -337,10 +358,6 @@ def url_for(options = {}, *parameters_for_method_reference) #:doc:
         end
       end
 
-      def module_name
-        @params["module"]
-      end
-
       # Converts the class name from something like "OneModule::TwoModule::NeatController" to "NeatController".
       def controller_class_name
         self.class.controller_class_name
@@ -691,7 +708,7 @@ def send_file_headers!(options)
       end
 
       def default_template_name(default_action_name = action_name)
-        module_name ? "#{module_name}/#{controller_name}/#{default_action_name}" : "#{controller_name}/#{default_action_name}"
+        "#{self.class.controller_path}/#{default_action_name}"
       end
   end
 end

actionpack/lib/action_controller/cgi_process.rb

@@ -46,8 +46,16 @@ def initialize(cgi, session_options = {})
       super()
     end
 
+    def query_string
+      return @cgi.query_string unless @cgi.query_string.nil? || @cgi.query_string.empty?
+      parts = env['REQUEST_URI'].split('?')
+      parts.shift
+      return parts.join('?')
+    end
+
     def query_parameters
-      @cgi.query_string ? CGIMethods.parse_query_parameters(@cgi.query_string) : {}
+      qs = self.query_string
+      qs.empty? ? {} : CGIMethods.parse_query_parameters(query_string)
     end
 
     def request_parameters

actionpack/lib/action_controller/helpers.rb

@@ -48,25 +48,22 @@ def add_template_helper(helper_module) #:nodoc:
       def helper(*args, &block)
         args.flatten.each do |arg|
           case arg
-          when Module
-            add_template_helper(arg)
-          when String, Symbol
-            file_name  = Inflector.underscore(arg.to_s.downcase) + '_helper'
-            class_name = Inflector.camelize(file_name)
-            begin
-              require_dependency(file_name)
-            rescue LoadError => load_error
-              requiree = / -- (.*?)(\.rb)?$/.match(load_error).to_a[1]
-              if requiree == file_name
-                raise LoadError, "Missing helper file helpers/#{file_name}.rb"
-              else
-                raise LoadError, "Can't load file: #{requiree}"
+            when Module
+              add_template_helper(arg)
+            when String, Symbol
+              file_name  = arg.to_s.underscore + '_helper'
+              class_name = file_name.camelize
+
+              begin
+                require_dependency(file_name)
+              rescue LoadError => load_error
+                requiree = / -- (.*?)(\.rb)?$/.match(load_error).to_a[1]
+                raise LoadError, requiree == file_name ? "Missing helper file helpers/#{file_name}.rb" : "Can't load file: #{requiree}"
               end
-            end
-            raise ArgumentError, "Missing #{class_name} module in helpers/#{file_name}.rb" unless Object.const_defined?(class_name)
-            add_template_helper(Object.const_get(class_name))
-          else
-            raise ArgumentError, 'helper expects String, Symbol, or Module argument'
+
+              add_template_helper(class_name.constantize)
+            else
+              raise ArgumentError, 'helper expects String, Symbol, or Module argument'
           end
         end
 
@@ -95,7 +92,7 @@ def helper_attr(*attrs)
         def inherited(child)
           inherited_without_helper(child)
           begin
-            child.helper(child.controller_name)
+            child.helper(child.controller_path)
           rescue ArgumentError, LoadError
             # No default helper available for this controller
           end

actionpack/lib/action_controller/request.rb

@@ -3,7 +3,7 @@ module ActionController
   class AbstractRequest
     # Returns both GET and POST parameters in a single hash.
     def parameters
-      @parameters ||= request_parameters.update(query_parameters)
+      @parameters ||= request_parameters.merge(query_parameters).merge(path_parameters).with_indifferent_access
     end
 
     def method
@@ -73,7 +73,7 @@ def raw_post
     end
 
     def request_uri
-      env['REQUEST_URI']
+      (%r{^\w+\://[^/]+(/.*|$)$} =~ env['REQUEST_URI']) ? $1 : env['REQUEST_URI'] # Remove domain, which webrick puts into the request_uri.
     end
 
     def protocol
@@ -85,7 +85,7 @@ def ssl?
     end
 
     def path
-      request_uri ? request_uri.split('?').first : ''
+      path = request_uri ? request_uri.split('?').first : ''
     end
 
     def port
@@ -100,7 +100,16 @@ def port_string
     def host_with_port
       env['HTTP_HOST'] || host + port_string
     end
+
+    def path_parameters=(parameters)
+      @path_parameters = parameters
+      @parameters = nil
+    end
 
+    def path_parameters
+      @path_parameters ||= {}
+    end
+
     #--
     # Must be implemented in the concrete request
     #++

actionpack/lib/action_controller/rescue.rb

@@ -48,7 +48,11 @@ def log_error(exception) #:doc:
 
       # Overwrite to implement public exception handling (for requests answering false to <tt>local_request?</tt>).
       def rescue_action_in_public(exception) #:doc:
-        render_text "<html><body><h1>Application error (Rails)</h1></body></html>"
+        case exception
+          when RoutingError, UnknownAction then
+            render_text(IO.read(File.join(RAILS_ROOT, 'public', '404.html')), "404 Not Found")
+          else render_text "<html><body><h1>Application error (Rails)</h1></body></html>"
+        end
       end
 
       # Overwrite to expand the meaning of a local request in order to show local rescues on other occurences than
@@ -110,13 +114,21 @@ def template_path_for_local_rescue(exception)
         rescues_path(
           case exception
             when MissingTemplate then "missing_template"
+            when RoutingError then "routing_error"
             when UnknownAction   then "unknown_action"
             when ActionView::TemplateError then "template_error"
-            else "diagnostics"
+            else raise ;"diagnostics"
           end
         )
       end
 
+      def response_code_for_rescue(exception)
+        case exception
+          when UnknownAction, RoutingError then "404 Page Not Found"
+          else "500 Internal Error"
+        end
+      end
+
       def clean_backtrace(exception)
         exception.backtrace.collect { |line| Object.const_defined?(:RAILS_ROOT) ? line.gsub(RAILS_ROOT, "") : line }
       end

actionpack/lib/action_controller/scaffolding.rb

@@ -149,7 +149,7 @@ def update#{suffix}
           
           private
             def render#{suffix}_scaffold(action = caller_method_name(caller))
-              if template_exists?("\#{controller_name}/\#{action}")
+              if template_exists?("\#{self.class.controller_path}/\#{action}")
                 render_action(action)
               else
                 @scaffold_class = #{class_name}

actionpack/lib/action_controller/templates/rescues/routing_error.rhtml

@@ -0,0 +1,8 @@
+<h1>Routing Error</h1>
+<p><%=h @exception.message %></p>
+<% unless @exception.failures.empty? %><p>
+	<h2>Failure reasons:</h2>
+	<% @exception.failures.each do |route, reason| %>
+		<%=h route.inspect.gsub('\\', '') %> failed because <%=h reason.downcase %><br />
+  <% end %>
+</p><% end %>

actionpack/lib/action_controller/test_process.rb

@@ -31,8 +31,8 @@ def process_test(request) #:nodoc:
 
   class TestRequest < AbstractRequest #:nodoc:
     attr_accessor :cookies
-    attr_accessor :query_parameters, :request_parameters, :session, :env
-    attr_accessor :host, :path, :request_uri, :remote_addr
+    attr_accessor :query_parameters, :request_parameters, :path, :session, :env
+    attr_accessor :host, :remote_addr
 
     def initialize(query_parameters = nil, request_parameters = nil, session = nil)
       @query_parameters   = query_parameters || {}
@@ -58,11 +58,28 @@ def action=(action_name)
       @parameters = nil
     end
 
+    # Used to check AbstractRequest's request_uri functionality.
+    # Disables the use of @path and @request_uri so superclass can handle those.
+    def set_REQUEST_URI(value)
+      @env["REQUEST_URI"] = value
+      @request_uri = nil
+      @path = nil
+    end
+
     def request_uri=(uri)
       @request_uri = uri
       @path = uri.split("?").first
     end
 
+    def request_uri
+      @request_uri || super()
+    end
+
+    def path
+      @path || super()
+    end
+
+
     private
       def initialize_containers
         @env, @cookies = {}, {}
@@ -237,6 +254,7 @@ class TestCase #:nodoc:
         def process(action, parameters = nil, session = nil)
           @request.env['REQUEST_METHOD'] ||= "GET"
           @request.action = action.to_s
+          @request.path_parameters = { :controller => @controller.class.controller_path }
           @request.parameters.update(parameters) unless parameters.nil?
           @request.session = ActionController::TestSession.new(session) unless session.nil?
           @controller.process(@request, @response)

actionpack/lib/action_controller/url_rewriter.rb

@@ -1,10 +1,9 @@
 module ActionController
   # Rewrites URLs for Base.redirect_to and Base.url_for in the controller.
   class UrlRewriter #:nodoc:
-    VALID_OPTIONS = [:action, :action_prefix, :action_suffix, :application_prefix, :module, :controller, :controller_prefix, :anchor, :params, :path_params, :id, :only_path, :overwrite_params, :host, :protocol ]
-
-    def initialize(request, controller, action)
-      @request, @controller, @action = request, controller, action
+    RESERVED_OPTIONS = [:anchor, :params, :path_params, :only_path, :host, :protocol]
+    def initialize(request, parameters)
+      @request, @parameters = request, parameters
       @rewritten_path = @request.path ? @request.path.dup : ""
     end
 
@@ -22,7 +21,7 @@ def to_s
     end
 
     def to_str
-      "#{@request.protocol}, #{@request.host_with_port}, #{@request.path}, #{@controller}, #{@action}, #{@request.parameters.inspect}"
+  		"#{@request.protocol}, #{@request.host_with_port}, #{@request.path}, #{@parameters[:controller]}, #{@parameters[:action]}, #{@request.parameters.inspect}"
     end
 
     private
@@ -48,12 +47,14 @@ def rewrite_url(path, options)
         return rewritten_url
       end
 
-      def rewrite_path(path, options)
-        include_id_in_path_params(options)
-
-        path = rewrite_action(path, options)      if options[:action] || options[:action_prefix]
-        path = rewrite_path_params(path, options) if options[:path_params]
-        path = rewrite_controller(path, options)  if options[:controller] || options[:controller_prefix]
+      def rewrite_path(options)
+        options = options.symbolize_keys
+        RESERVED_OPTIONS.each {|k| options.delete k}
+
+        path, extras = Routing::Routes.generate(options, @request)
+        path = "/#{path.join('/')}"
+        path += build_query_string(extras)
+
         return path
       end
 

actionpack/lib/action_view/partials.rb

@@ -60,7 +60,7 @@ def partial_pieces(partial_path)
         if partial_path.include?('/')
           return File.dirname(partial_path), File.basename(partial_path)
         else
-          return controller.send(:controller_name), partial_path
+          return controller.class.controller_path, partial_path
         end
       end
 

actionpack/test/controller/cookie_test.rb

@@ -28,10 +28,6 @@ def set_multiple_cookies
       render_text "hello world"
     end
 
-    def access_frozen_cookies
-      @cookies["wont"] = "work"
-    end
-
     def rescue_action(e) raise end
   end
 
@@ -67,11 +63,6 @@ def test_multiple_cookies
     assert_equal 2, process_request.headers["cookie"].size
   end
 
-  def test_setting_cookie_on_frozen_instance_variable
-    @request.action = "access_frozen_cookies"
-    assert_raises(TypeError) { process_request }
-  end
-
   private
     def process_request
       TestController.process(@request, @response)