From c844755e5a0c3d4edfcc78f9c30ef91fa0de550a Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 15 Feb 2005 00:51:02 +0000 Subject: Merged back the Routing branch git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@614 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- .../assertions/action_pack_assertions.rb | 32 +++++++++++++++++++- actionpack/lib/action_controller/base.rb | 27 +++++++++++++---- actionpack/lib/action_controller/cgi_process.rb | 10 ++++++- actionpack/lib/action_controller/helpers.rb | 35 ++++++++++------------ actionpack/lib/action_controller/request.rb | 15 ++++++++-- actionpack/lib/action_controller/rescue.rb | 16 ++++++++-- actionpack/lib/action_controller/scaffolding.rb | 2 +- .../templates/rescues/routing_error.rhtml | 8 +++++ actionpack/lib/action_controller/test_process.rb | 22 ++++++++++++-- actionpack/lib/action_controller/url_rewriter.rb | 23 +++++++------- 10 files changed, 145 insertions(+), 45 deletions(-) create mode 100644 actionpack/lib/action_controller/templates/rescues/routing_error.rhtml (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/assertions/action_pack_assertions.rb b/actionpack/lib/action_controller/assertions/action_pack_assertions.rb index c26941cd6b..7d27240244 100644 --- a/actionpack/lib/action_controller/assertions/action_pack_assertions.rb +++ b/actionpack/lib/action_controller/assertions/action_pack_assertions.rb @@ -141,7 +141,7 @@ module Test #:nodoc: 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 @@ module Test #:nodoc: 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 diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index cfcd46d985..a30f3b94d7 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -13,6 +13,13 @@ module ActionController #: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 @@ module ActionController #:nodoc: # 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 @@ module ActionController #:nodoc: 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 @@ module ActionController #:nodoc: 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 @@ module ActionController #:nodoc: 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 diff --git a/actionpack/lib/action_controller/cgi_process.rb b/actionpack/lib/action_controller/cgi_process.rb index 92b19fbf18..c301b322e5 100644 --- a/actionpack/lib/action_controller/cgi_process.rb +++ b/actionpack/lib/action_controller/cgi_process.rb @@ -46,8 +46,16 @@ module ActionController #:nodoc: 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 diff --git a/actionpack/lib/action_controller/helpers.rb b/actionpack/lib/action_controller/helpers.rb index 1201d31946..a97fd93410 100644 --- a/actionpack/lib/action_controller/helpers.rb +++ b/actionpack/lib/action_controller/helpers.rb @@ -48,25 +48,22 @@ module ActionController #: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 @@ module ActionController #:nodoc: 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 diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 3e2344c3cb..2ac6081a96 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/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 @@ module ActionController 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 @@ module ActionController end def path - request_uri ? request_uri.split('?').first : '' + path = request_uri ? request_uri.split('?').first : '' end def port @@ -100,7 +100,16 @@ module ActionController 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 #++ diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index eb7f614de0..9eb64ba7cc 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -48,7 +48,11 @@ module ActionController #:nodoc: # Overwrite to implement public exception handling (for requests answering false to local_request?). def rescue_action_in_public(exception) #:doc: - render_text "

Application error (Rails)

" + case exception + when RoutingError, UnknownAction then + render_text(IO.read(File.join(RAILS_ROOT, 'public', '404.html')), "404 Not Found") + else render_text "

Application error (Rails)

" + 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 @@ module ActionController #:nodoc: 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 diff --git a/actionpack/lib/action_controller/scaffolding.rb b/actionpack/lib/action_controller/scaffolding.rb index 9c1311efa3..140df73972 100644 --- a/actionpack/lib/action_controller/scaffolding.rb +++ b/actionpack/lib/action_controller/scaffolding.rb @@ -149,7 +149,7 @@ module ActionController 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} diff --git a/actionpack/lib/action_controller/templates/rescues/routing_error.rhtml b/actionpack/lib/action_controller/templates/rescues/routing_error.rhtml new file mode 100644 index 0000000000..82c01e10c9 --- /dev/null +++ b/actionpack/lib/action_controller/templates/rescues/routing_error.rhtml @@ -0,0 +1,8 @@ +

Routing Error

+

<%=h @exception.message %>

+<% unless @exception.failures.empty? %>

+

Failure reasons:

+ <% @exception.failures.each do |route, reason| %> + <%=h route.inspect.gsub('\\', '') %> failed because <%=h reason.downcase %>
+ <% end %> +

<% end %> diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index d4dfe7933d..3223da198c 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -31,8 +31,8 @@ module ActionController #: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 @@ module ActionController #:nodoc: @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 @@ module Test 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) diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb index 3451e6acc9..3364262e4c 100644 --- a/actionpack/lib/action_controller/url_rewriter.rb +++ b/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 @@ module ActionController 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 @@ module ActionController 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 -- cgit v1.2.3