From 6e30361260205cb7029fbc78b4a98b66a884ce45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 1 Dec 2009 13:11:24 -0200 Subject: Allow ActionController::Responder to have a common entry point for all formats. Signed-off-by: Yehuda Katz --- actionpack/lib/action_controller/metal/responder.rb | 11 ++++++++--- actionpack/test/controller/mime_responds_test.rb | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index e8e88e7479..6c76c57839 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -102,9 +102,14 @@ module ActionController #:nodoc: # not defined, call to_format. # def self.call(*args) - responder = new(*args) - method = :"to_#{responder.format}" - responder.respond_to?(method) ? responder.send(method) : responder.to_format + new(*args).respond + end + + # Main entry point for responder responsible to dispatch to the proper format. + # + def respond + method = :"to_#{format}" + respond_to?(method) ? send(method) : to_format end # HTML format does not render the resource, it always attempt to render a diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index fee9cf46f9..c1fa74b8c8 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -507,6 +507,13 @@ class RespondWithController < ActionController::Base end end + def using_responder_with_respond + responder = Class.new(ActionController::Responder) do + def respond; @controller.render :text => "respond #{format}"; end + end + respond_with(Customer.new("david", 13), :responder => responder) + end + protected def _render_js(js, options) @@ -735,6 +742,16 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal "foo - #{[:html].to_s}", @controller.response_body end + def test_respond_as_responder_entry_point + @request.accept = "text/html" + get :using_responder_with_respond + assert_equal "respond html", @response.body + + @request.accept = "application/xml" + get :using_responder_with_respond + assert_equal "respond xml", @response.body + end + def test_clear_respond_to @controller = InheritedRespondWithController.new @request.accept = "text/html" -- cgit v1.2.3 From 0c4990b5f4b8c99ea656170eff9bb9c3490de642 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 1 Dec 2009 12:19:42 -0800 Subject: Fix caching test to ensure notifications are all delivered --- actionpack/test/controller/caching_test.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack') diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 54de920740..682a8f3995 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -640,6 +640,7 @@ class FragmentCachingTest < ActionController::TestCase assert fragment_computed assert_equal 'generated till now -> ', buffer + ActiveSupport::Notifications.notifier.wait assert_equal [:fragment_exist?, :write_fragment], events.map(&:first) end -- cgit v1.2.3 From 61a31f3d3dae55b3ed2a49fafcbfe45b77ea3be2 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 1 Dec 2009 14:52:19 -0600 Subject: Fix generating params with optional defaults [#3404 state:resolved] --- actionpack/lib/action_dispatch/routing/deprecated_mapper.rb | 2 +- actionpack/test/controller/routing_test.rb | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb index dd76391870..87dfaba6c7 100644 --- a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb +++ b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb @@ -175,7 +175,7 @@ module ActionDispatch optional = false elsif segment =~ /^:(\w+)$/ if defaults.has_key?($1.to_sym) - defaults.delete($1.to_sym) + defaults.delete($1.to_sym) if defaults[$1.to_sym].nil? else optional = false end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index b5effeda40..97fbd95e73 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -1639,9 +1639,7 @@ class RouteSetTest < ActiveSupport::TestCase map.connect ':controller/:action/:id' end - pending do - assert_equal '/ibocorp', set.generate({:controller => 'ibocorp', :page => 1}) - end + assert_equal '/ibocorp', set.generate({:controller => 'ibocorp', :page => 1}) end def test_generate_with_optional_params_recalls_last_request -- cgit v1.2.3 From 7fe19d415ab80727d685c163d7a0413ca6bfe585 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 1 Dec 2009 22:22:48 -0600 Subject: Make recognize try to constantize controller to see if it exists --- actionpack/lib/action_dispatch/routing/route_set.rb | 3 +++ actionpack/test/controller/routing_test.rb | 2 -- actionpack/test/lib/controller/fake_controllers.rb | 13 ++++++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 79e15edeaa..18e18c5820 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -28,6 +28,7 @@ module ActionDispatch end if env['action_controller.recognize'] + controller(params) [200, {}, params] else controller = controller(params) @@ -41,6 +42,8 @@ module ActionDispatch controller = "#{params[:controller].camelize}Controller" ActiveSupport::Inflector.constantize(controller) end + rescue NameError => e + raise ActionController::RoutingError, e.message end def merge_default_action!(params) diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 97fbd95e73..84564f4e43 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -1851,11 +1851,9 @@ class RackMountIntegrationTests < ActiveSupport::TestCase assert_equal({:controller => 'posts', :action => 'show_date', :year => '2009'}, @routes.recognize_path('/blog/2009', :method => :get)) assert_equal({:controller => 'posts', :action => 'show_date', :year => '2009', :month => '01'}, @routes.recognize_path('/blog/2009/01', :method => :get)) assert_equal({:controller => 'posts', :action => 'show_date', :year => '2009', :month => '01', :day => '01'}, @routes.recognize_path('/blog/2009/01/01', :method => :get)) - assert_raise(ActionController::ActionControllerError) { @routes.recognize_path('/blog/123456789', :method => :get) } assert_equal({:controller => 'archive', :action => 'index', :year => '2010'}, @routes.recognize_path('/archive/2010')) assert_equal({:controller => 'archive', :action => 'index'}, @routes.recognize_path('/archive')) - assert_raise(ActionController::ActionControllerError) { @routes.recognize_path('/archive/january') } assert_equal({:controller => 'people', :action => 'index'}, @routes.recognize_path('/people', :method => :get)) assert_equal({:controller => 'people', :action => 'index', :format => 'xml'}, @routes.recognize_path('/people.xml', :method => :get)) diff --git a/actionpack/test/lib/controller/fake_controllers.rb b/actionpack/test/lib/controller/fake_controllers.rb index 250327e6dc..09692f77b5 100644 --- a/actionpack/test/lib/controller/fake_controllers.rb +++ b/actionpack/test/lib/controller/fake_controllers.rb @@ -1,37 +1,48 @@ class << Object; alias_method :const_available?, :const_defined?; end class ContentController < ActionController::Base; end -class NotAController; end module Admin class << self; alias_method :const_available?, :const_defined?; end + class AccountsController < ActionController::Base; end class NewsFeedController < ActionController::Base; end class PostsController < ActionController::Base; end class StuffController < ActionController::Base; end class UserController < ActionController::Base; end + class UsersController < ActionController::Base; end end module Api + class UsersController < ActionController::Base; end class ProductsController < ActionController::Base; end end # TODO: Reduce the number of test controllers we use +class AccountController < ActionController::Base; end class AddressesController < ActionController::Base; end +class ArchiveController < ActionController::Base; end class ArticlesController < ActionController::Base; end class BarController < ActionController::Base; end +class BlogController < ActionController::Base; end class BooksController < ActionController::Base; end class BraveController < ActionController::Base; end +class CarsController < ActionController::Base; end +class CcController < ActionController::Base; end class CController < ActionController::Base; end class ElsewhereController < ActionController::Base; end class FooController < ActionController::Base; end +class GeocodeController < ActionController::Base; end class HiController < ActionController::Base; end class ImageController < ActionController::Base; end +class NewsController < ActionController::Base; end class NotesController < ActionController::Base; end class PeopleController < ActionController::Base; end class PostsController < ActionController::Base; end class SessionsController < ActionController::Base; end class StuffController < ActionController::Base; end class SubpathBooksController < ActionController::Base; end +class SymbolsController < ActionController::Base; end +class UserController < ActionController::Base; end class WeblogController < ActionController::Base; end # For speed test -- cgit v1.2.3 From 2fbd6f46fd00a334119f1a25394046963831ce3e Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 1 Dec 2009 22:48:42 -0600 Subject: Simply track controller namespaces instead of a complete list of possible controllers to route to --- actionpack/lib/action_dispatch/routing.rb | 59 ++++++---------------- .../lib/action_dispatch/routing/route_set.rb | 10 ++-- 2 files changed, 22 insertions(+), 47 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 9b977800b4..9159bb6395 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -265,6 +265,7 @@ module ActionDispatch SEPARATORS = %w( / . ? ) HTTP_METHODS = [:get, :head, :post, :put, :delete, :options] + CONTROLLER_REGEXP = /[_a-zA-Z0-9]+/ # The root paths which may contain controller files mattr_accessor :controller_paths @@ -277,7 +278,11 @@ module ActionDispatch class << self def controller_constraints - @controller_constraints ||= Regexp.union(*possible_controllers.collect { |n| Regexp.escape(n) }) + @controller_constraints ||= begin + source = controller_namespaces.map { |ns| "#{Regexp.escape(ns)}/#{CONTROLLER_REGEXP.source}" } + source << CONTROLLER_REGEXP.source + Regexp.compile(source.sort.reverse.join('|')) + end end def clear_controller_cache! @@ -285,57 +290,25 @@ module ActionDispatch end private - # Returns the array of controller names currently available to ActionController::Routing. - def possible_controllers - possible_controllers = [] + def controller_namespaces + namespaces = Set.new - # Find any controller classes already in memory + # Find any nested controllers already in memory ActionController::Base.subclasses.each do |klass| controller_name = klass.underscore - controller_name.gsub!(/_controller\Z/, '') - possible_controllers << controller_name + namespaces << controller_name.split('/')[0...-1].join('/') end - # Find controllers in controllers/ directory - paths = controller_paths.select { |path| File.directory?(path) && path != "." } - seen_paths = Hash.new {|h, k| h[k] = true; false} - normalize_paths(paths).each do |load_path| + # Find namespaces in controllers/ directory + controller_paths.each do |load_path| + load_path = File.expand_path(load_path) Dir["#{load_path}/**/*_controller.rb"].collect do |path| - next if seen_paths[path.gsub(%r{^\.[/\\]}, "")] - - controller_name = path[(load_path.length + 1)..-1] - - controller_name.gsub!(/_controller\.rb\Z/, '') - possible_controllers << controller_name + namespaces << File.dirname(path).sub(/#{load_path}\/?/, '') end end - # remove duplicates - possible_controllers.uniq! - - possible_controllers - end - - # Returns an array of paths, cleaned of double-slashes and relative path references. - # * "\\\" and "//" become "\\" or "/". - # * "/foo/bar/../config" becomes "/foo/config". - # The returned array is sorted by length, descending. - def normalize_paths(paths) - # do the hokey-pokey of path normalization... - paths = paths.collect do |path| - path = path. - gsub("//", "/"). # replace double / chars with a single - gsub("\\\\", "\\"). # replace double \ chars with a single - gsub(%r{(.)[\\/]$}, '\1') # drop final / or \ if path ends with it - - # eliminate .. paths where possible - re = %r{[^/\\]+[/\\]\.\.[/\\]} - path.gsub!(re, "") while path.match(re) - path - end - - # start with longest path, first - paths = paths.uniq.sort_by { |path| - path.length } + namespaces.delete('') + namespaces end end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 18e18c5820..c2f6531a74 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -27,11 +27,13 @@ module ActionDispatch end end + unless controller = controller(params) + return [417, {}, []] + end + if env['action_controller.recognize'] - controller(params) [200, {}, params] else - controller = controller(params) controller.action(params[:action]).call(env) end end @@ -42,8 +44,8 @@ module ActionDispatch controller = "#{params[:controller].camelize}Controller" ActiveSupport::Inflector.constantize(controller) end - rescue NameError => e - raise ActionController::RoutingError, e.message + rescue NameError + nil end def merge_default_action!(params) -- cgit v1.2.3 From 97be8537ebf33aa0c47fe1c6164e94c24acc3c19 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 1 Dec 2009 22:58:56 -0600 Subject: Fix @renderer warning --- actionpack/lib/action_view/render/partials.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 2eb88ae3e5..aeaf1ee4ff 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -181,20 +181,20 @@ module ActionView def initialize(view_context, options, block) @view = view_context @partial_names = PARTIAL_NAMES[@view.controller.class] - + key = Thread.current[:format_locale_key] @templates = TEMPLATES[key] if key - + setup(options, block) end - + def setup(options, block) partial = options[:partial] - + @options = options @locals = options[:locals] || {} @block = block - + if String === partial @object = options[:object] @path = partial @@ -240,7 +240,7 @@ module ActionView segments << template.render(@view, locals) end - + @template = template segments end @@ -294,7 +294,7 @@ module ActionView path && @templates[path] ||= _find_template(path) end end - + def _find_template(path) if controller = @view.controller prefix = controller.controller_path unless path.include?(?/) @@ -319,7 +319,7 @@ module ActionView _evaluate_assigns_and_ivars details = options[:_details] - + # Is this needed self.formats = details[:formats] if details renderer = PartialRenderer.new(self, options, nil) @@ -329,12 +329,12 @@ module ActionView end def _render_partial(options, &block) #:nodoc: - if @renderer + if defined? @renderer @renderer.setup(options, block) else @renderer = PartialRenderer.new(self, options, block) end - + @renderer.render end -- cgit v1.2.3 From 75ae5bb0228ec8d8a144030573db2fd998299042 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 1 Dec 2009 23:14:03 -0600 Subject: cache_store and page_cache_directory are already defined in caching and pages --- actionpack/lib/action_controller/metal/compatibility.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb index c251d79f4e..0c264fcd09 100644 --- a/actionpack/lib/action_controller/metal/compatibility.rb +++ b/actionpack/lib/action_controller/metal/compatibility.rb @@ -46,11 +46,8 @@ module ActionController cattr_accessor :use_accept_header self.use_accept_header = true - cattr_accessor :page_cache_directory self.page_cache_directory = defined?(Rails.public_path) ? Rails.public_path : "" - cattr_reader :cache_store - cattr_accessor :consider_all_requests_local self.consider_all_requests_local = true @@ -116,7 +113,7 @@ module ActionController details[:prefix] = nil if name =~ /\blayouts/ super end - + # Move this into a "don't run in production" module def _default_layout(details, require_layout = false) super -- cgit v1.2.3 From 70d0b7c87a492aef141859127d1b9fec6b4b09f3 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 1 Dec 2009 23:19:41 -0600 Subject: Fix parens warning in ajax test --- actionpack/test/template/ajax_test.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/ajax_test.rb b/actionpack/test/template/ajax_test.rb index 670ba92697..aeb7c09b09 100644 --- a/actionpack/test/template/ajax_test.rb +++ b/actionpack/test/template/ajax_test.rb @@ -6,7 +6,7 @@ class AjaxTestCase < ActiveSupport::TestCase def assert_html(html, matches) matches.each do |match| - assert_match Regexp.new(Regexp.escape(match)), html + assert_match(Regexp.new(Regexp.escape(match)), html) end end @@ -52,18 +52,18 @@ class LinkToRemoteTest < AjaxTestCase test "with a hash for :update" do link = link(:update => {:success => "#posts", :failure => "#error"}) - assert_match /data-update-success="#posts"/, link - assert_match /data-update-failure="#error"/, link + assert_match(/data-update-success="#posts"/, link) + assert_match(/data-update-failure="#error"/, link) end test "with positional parameters" do link = link(:position => :top, :update => "#posts") - assert_match /data\-update\-position="top"/, link + assert_match(/data\-update\-position="top"/, link) end test "with an optional method" do link = link(:method => "delete") - assert_match /data-method="delete"/, link + assert_match(/data-method="delete"/, link) end class LegacyLinkToRemoteTest < AjaxTestCase @@ -99,7 +99,7 @@ class ButtonToRemoteTest < AjaxTestCase button = button({:url => {:action => "whatnot"}}, {:class => "fine"}) [/input/, /class="fine"/, /type="button"/, /value="Remote outpost"/, /data-url="\/whatnot"/].each do |match| - assert_match match, button + assert_match(match, button) end end end -- cgit v1.2.3 From f22db809c9da89adac0be60c5b70f37f75dc8376 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 1 Dec 2009 23:27:40 -0600 Subject: Response#cache_control is defined later --- actionpack/lib/action_dispatch/http/response.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index b3ed7c9d1a..c651f21f68 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -33,7 +33,6 @@ module ActionDispatch # :nodoc: # end class Response < Rack::Response attr_accessor :request, :blank - attr_reader :cache_control attr_writer :header, :sending_file alias_method :headers=, :header= -- cgit v1.2.3 From ad26f066fe94bad4219f235a0d65190e4d8d15c8 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 1 Dec 2009 23:29:28 -0600 Subject: Response#write is defined twice (this is why -w is good) --- actionpack/lib/action_dispatch/http/response.rb | 6 ------ 1 file changed, 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index c651f21f68..32f9a0031d 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -56,12 +56,6 @@ module ActionDispatch # :nodoc: @cache_control ||= {} end - def write(str) - s = str.to_s - @writer.call s - str - end - def status=(status) @status = status.to_i end -- cgit v1.2.3 From de40bc033a64c7074e01a8e8c585225f4cdaf81e Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 2 Dec 2009 03:23:00 -0800 Subject: Ensure Cache-Control max-age is an integer --- actionpack/lib/action_dispatch/http/response.rb | 2 +- actionpack/test/controller/render_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 32f9a0031d..4f35a00247 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -270,7 +270,7 @@ module ActionDispatch # :nodoc: max_age = control[:max_age] options = [] - options << "max-age=#{max_age}" if max_age + options << "max-age=#{max_age.to_i}" if max_age options << (control[:public] ? "public" : "private") options << "must-revalidate" if control[:must_revalidate] options.concat(extras) if extras diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index b32325fa20..cffa970011 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -54,7 +54,7 @@ class TestController < ActionController::Base end def conditional_hello_with_expires_in - expires_in 1.minute + expires_in 60.1.seconds render :action => 'hello_world' end -- cgit v1.2.3 From 84be6cfb6452a23d5617cd8a8b200d8fb0431d5a Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 2 Dec 2009 12:33:33 -0600 Subject: Fork rack build nested query to support to_param --- .../lib/action_dispatch/routing/route_set.rb | 45 +++++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index c2f6531a74..f029b634d6 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -379,7 +379,8 @@ module ActionDispatch end recall[:action] = options.delete(:action) if options[:action] == 'index' - parameterize = lambda { |name, value| + opts = {} + opts[:parameterize] = lambda { |name, value| if name == :controller value elsif value.is_a?(Array) @@ -389,7 +390,22 @@ module ActionDispatch end } - path = @set.url(named_route, options, recall, :parameterize => parameterize) + unless result = @set.generate(:path_info, named_route, options, recall, opts) + raise ActionController::RoutingError, "No route matches #{options.inspect}" + end + + uri, params = result + params.each do |k, v| + if v + params[k] = v + else + params.delete(k) + end + end + + uri << "?#{build_nested_query(params)}" if uri && params.any? + path = uri + if path && method == :generate_extras uri = URI(path) extras = uri.query ? @@ -456,6 +472,31 @@ module ActionDispatch def extract_request_environment(request) { :method => request.method } end + + private + def build_nested_query(value, prefix = nil) + case value + when Array + value.map { |v| + build_nested_query(v, "#{prefix}[]") + }.join("&") + when Hash + value.map { |k, v| + build_nested_query(v, prefix ? "#{prefix}[#{k}]" : k) + }.join("&") + when String + raise ArgumentError, "value must be a Hash" if prefix.nil? + "#{Rack::Utils.escape(prefix)}=#{Rack::Utils.escape(value)}" + when NilClass + Rack::Utils.escape(prefix) + else + if value.respond_to?(:to_param) + build_nested_query(value.to_param.to_s, prefix) + else + Rack::Utils.escape(prefix) + end + end + end end end end -- cgit v1.2.3 From c0949cc8f6c73111075e2c5b41f22f4b99a8ab26 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 2 Dec 2009 12:38:25 -0600 Subject: Rackmount 0.3.0 --- actionpack/actionpack.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 936279ad12..e191a12997 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -18,7 +18,7 @@ Gem::Specification.new do |s| s.add_dependency('activemodel', '= 3.0.pre') s.add_dependency('rack', '~> 1.0.1') s.add_dependency('rack-test', '~> 0.5.0') - s.add_dependency('rack-mount', '~> 0.2.3') + s.add_dependency('rack-mount', '~> 0.3.0') s.add_dependency('erubis', '~> 2.6.5') s.require_path = 'lib' -- cgit v1.2.3 From 4dee277a9bc05083de6c831cf9aae0846849ecda Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 2 Dec 2009 12:46:14 -0600 Subject: Stop escaping "[]" in query string --- actionpack/lib/action_dispatch/routing/route_set.rb | 4 +--- actionpack/test/controller/routing_test.rb | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index f029b634d6..5e9c36bbaf 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -486,9 +486,7 @@ module ActionDispatch }.join("&") when String raise ArgumentError, "value must be a Hash" if prefix.nil? - "#{Rack::Utils.escape(prefix)}=#{Rack::Utils.escape(value)}" - when NilClass - Rack::Utils.escape(prefix) + "#{prefix}=#{Rack::Utils.escape(value)}" else if value.respond_to?(:to_param) build_nested_query(value.to_param.to_s, prefix) diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 84564f4e43..b83c5792ba 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -1621,7 +1621,7 @@ class RouteSetTest < ActiveSupport::TestCase end def test_expand_array_build_query_string - assert_uri_equal '/foo?x%5B%5D=1&x%5B%5D=2', default_route_set.generate({:controller => 'foo', :x => [1, 2]}) + assert_uri_equal '/foo?x[]=1&x[]=2', default_route_set.generate({:controller => 'foo', :x => [1, 2]}) end def test_escape_spaces_build_query_string_selected_keys @@ -2012,9 +2012,9 @@ class RackMountIntegrationTests < ActiveSupport::TestCase assert_equal '/posts', @routes.generate({:controller => 'posts'}, {:controller => 'posts', :action => 'index'}) assert_equal '/posts/create', @routes.generate({:action => 'create'}, {:controller => 'posts'}) assert_equal '/posts?foo=bar', @routes.generate(:controller => 'posts', :foo => 'bar') - assert_equal '/posts?foo%5B%5D=bar&foo%5B%5D=baz', @routes.generate(:controller => 'posts', :foo => ['bar', 'baz']) + assert_equal '/posts?foo[]=bar&foo[]=baz', @routes.generate(:controller => 'posts', :foo => ['bar', 'baz']) assert_equal '/posts?page=2', @routes.generate(:controller => 'posts', :page => 2) - assert_equal '/posts?q%5Bfoo%5D%5Ba%5D=b', @routes.generate(:controller => 'posts', :q => { :foo => { :a => 'b'}}) + assert_equal '/posts?q[foo][a]=b', @routes.generate(:controller => 'posts', :q => { :foo => { :a => 'b'}}) assert_equal '/', @routes.generate(:controller => 'news', :action => 'index') assert_equal '/', @routes.generate(:controller => 'news', :action => 'index', :format => nil) -- cgit v1.2.3 From 8db038227ca4cbcba01a86ef5fb94cb13c780463 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 2 Dec 2009 14:10:22 -0600 Subject: Move controller namespace tracking into route set so it gets reloaded in dev mode --- actionpack/lib/action_dispatch/routing.rb | 41 ---------------------- .../action_dispatch/routing/deprecated_mapper.rb | 2 +- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- .../lib/action_dispatch/routing/route_set.rb | 36 +++++++++++++++++-- 4 files changed, 36 insertions(+), 45 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 9159bb6395..e99f979197 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -265,51 +265,10 @@ module ActionDispatch SEPARATORS = %w( / . ? ) HTTP_METHODS = [:get, :head, :post, :put, :delete, :options] - CONTROLLER_REGEXP = /[_a-zA-Z0-9]+/ - - # The root paths which may contain controller files - mattr_accessor :controller_paths - self.controller_paths = [] # A helper module to hold URL related helpers. module Helpers include ActionController::PolymorphicRoutes end - - class << self - def controller_constraints - @controller_constraints ||= begin - source = controller_namespaces.map { |ns| "#{Regexp.escape(ns)}/#{CONTROLLER_REGEXP.source}" } - source << CONTROLLER_REGEXP.source - Regexp.compile(source.sort.reverse.join('|')) - end - end - - def clear_controller_cache! - @controller_constraints = nil - end - - private - def controller_namespaces - namespaces = Set.new - - # Find any nested controllers already in memory - ActionController::Base.subclasses.each do |klass| - controller_name = klass.underscore - namespaces << controller_name.split('/')[0...-1].join('/') - end - - # Find namespaces in controllers/ directory - controller_paths.each do |load_path| - load_path = File.expand_path(load_path) - Dir["#{load_path}/**/*_controller.rb"].collect do |path| - namespaces << File.dirname(path).sub(/#{load_path}\/?/, '') - end - end - - namespaces.delete('') - namespaces - end - end end end diff --git a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb index 87dfaba6c7..8ce6b2f6d5 100644 --- a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb +++ b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb @@ -113,7 +113,7 @@ module ActionDispatch end end - requirements[:controller] ||= Routing.controller_constraints + requirements[:controller] ||= @set.controller_constraints if defaults[:controller] defaults[:action] ||= 'index' diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 34d75e55b6..400039353c 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -383,7 +383,7 @@ module ActionDispatch constraints.reject! { |k, v| segment_keys.include?(k.to_s) } conditions.merge!(constraints) - requirements[:controller] ||= Routing.controller_constraints + requirements[:controller] ||= @set.controller_constraints if via = options[:via] via = Array(via).map { |m| m.to_s.upcase } diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 5e9c36bbaf..201cf462e4 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -202,10 +202,11 @@ module ActionDispatch end end - attr_accessor :routes, :named_routes, :configuration_files + attr_accessor :routes, :named_routes, :configuration_files, :controller_paths def initialize self.configuration_files = [] + self.controller_paths = [] self.routes = [] self.named_routes = NamedRouteCollection.new @@ -252,7 +253,7 @@ module ActionDispatch def load! # Clear the controller cache so we may discover new ones - Routing.clear_controller_cache! + @controller_constraints = nil load_routes! end @@ -297,6 +298,37 @@ module ActionDispatch routes_changed_at end + CONTROLLER_REGEXP = /[_a-zA-Z0-9]+/ + + def controller_constraints + @controller_constraints ||= begin + source = controller_namespaces.map { |ns| "#{Regexp.escape(ns)}/#{CONTROLLER_REGEXP.source}" } + source << CONTROLLER_REGEXP.source + Regexp.compile(source.sort.reverse.join('|')) + end + end + + def controller_namespaces + namespaces = Set.new + + # Find any nested controllers already in memory + ActionController::Base.subclasses.each do |klass| + controller_name = klass.underscore + namespaces << controller_name.split('/')[0...-1].join('/') + end + + # Find namespaces in controllers/ directory + controller_paths.each do |load_path| + load_path = File.expand_path(load_path) + Dir["#{load_path}/**/*_controller.rb"].collect do |path| + namespaces << File.dirname(path).sub(/#{load_path}\/?/, '') + end + end + + namespaces.delete('') + namespaces + end + def add_route(app, conditions = {}, requirements = {}, defaults = {}, name = nil) route = Route.new(app, conditions, requirements, defaults, name) @set.add_route(*route) -- cgit v1.2.3 From 399909b11c094ab32542d300c72940b1b263b8e6 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 2 Dec 2009 15:23:26 -0600 Subject: Use to_query in route query string generation --- .../lib/action_dispatch/routing/route_set.rb | 25 +--------------------- 1 file changed, 1 insertion(+), 24 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 201cf462e4..a8073c2105 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -435,7 +435,7 @@ module ActionDispatch end end - uri << "?#{build_nested_query(params)}" if uri && params.any? + uri << "?#{params.to_query}" if uri && params.any? path = uri if path && method == :generate_extras @@ -504,29 +504,6 @@ module ActionDispatch def extract_request_environment(request) { :method => request.method } end - - private - def build_nested_query(value, prefix = nil) - case value - when Array - value.map { |v| - build_nested_query(v, "#{prefix}[]") - }.join("&") - when Hash - value.map { |k, v| - build_nested_query(v, prefix ? "#{prefix}[#{k}]" : k) - }.join("&") - when String - raise ArgumentError, "value must be a Hash" if prefix.nil? - "#{prefix}=#{Rack::Utils.escape(value)}" - else - if value.respond_to?(:to_param) - build_nested_query(value.to_param.to_s, prefix) - else - Rack::Utils.escape(prefix) - end - end - end end end end -- cgit v1.2.3