diff options
Diffstat (limited to 'actionpack')
-rw-r--r-- | actionpack/CHANGELOG.md | 6 | ||||
-rw-r--r-- | actionpack/lib/abstract_controller/base.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/renderers.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/http/url.rb | 20 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/journey/route.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/journey/router.rb | 4 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/inspector.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 9 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/testing/integration.rb | 1 | ||||
-rw-r--r-- | actionpack/test/controller/new_base/bare_metal_test.rb | 4 | ||||
-rw-r--r-- | actionpack/test/dispatch/url_generation_test.rb | 18 |
13 files changed, 55 insertions, 19 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 92e94ee463..86e98c011c 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix URL generation with `:trailing_slash` such that it does not add + a trailing slash after `.:format` + + *Dan Langevin* + * Build full URI as string when processing path in integration tests for performance reasons. @@ -131,4 +136,5 @@ *Tony Wooster* + Please check [4-1-stable](https://github.com/rails/rails/blob/4-1-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index c00f0d0c6f..acdfb33efa 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -255,7 +255,7 @@ module AbstractController # Checks if the action name is valid and returns false otherwise. def _valid_action_name?(action_name) - action_name.to_s !~ Regexp.new(File::SEPARATOR) + action_name !~ Regexp.new(File::SEPARATOR) end end end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 70ca99f01c..3bfc8d6460 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -226,7 +226,7 @@ module ActionController # Returns a Rack endpoint for the given action name. def self.action(name, klass = ActionDispatch::Request) - middleware_stack.build(name.to_s) do |env| + middleware_stack.build(name) do |env| new.dispatch(name, klass.new(env)) end end diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index 29ce5abd55..46405cef55 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -78,7 +78,7 @@ module ActionController # respond_to do |format| # format.html # format.csv { render csv: @csvable, filename: @csvable.name } - # } + # end # end # To use renderers and their mime types in more concise ways, see # <tt>ActionController::MimeResponds::ClassMethods.respond_to</tt> and diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index a6c17f50a5..4cba4f5f37 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -37,13 +37,7 @@ module ActionDispatch path = options[:script_name].to_s.chomp("/") path << options[:path].to_s - if options[:trailing_slash] - if path.include?('?') - path.sub!(/\?/, '/\&') - else - path.sub!(/[^\/]\z|\A\z/, '\&/') - end - end + add_trailing_slash(path) if options[:trailing_slash] result = path @@ -66,6 +60,18 @@ module ActionDispatch private + def add_trailing_slash(path) + # includes querysting + if path.include?('?') + path.sub!(/\?/, '/\&') + # does not have a .format + elsif !path.include?(".") + path.sub!(/[^\/]\z|\A\z/, '\&/') + end + + path + end + def build_host_url(options) if match = options[:host].match(HOST_REGEXP) options[:protocol] ||= match[1] unless options[:protocol] == false diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index cc3c7f20cb..1ba91d548e 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -19,7 +19,7 @@ module ActionDispatch # Unwrap any constraints so we can see what's inside for route generation. # This allows the formatter to skip over any mounted applications or redirects # that shouldn't be matched when using a url_for without a route name. - while app.is_a?(Routing::Mapper::Constraints) do + if app.is_a?(Routing::Mapper::Constraints) app = app.app end @dispatcher = app.is_a?(Routing::RouteSet::Dispatcher) diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 2ead6a4eb3..c0317e3ad2 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -115,7 +115,7 @@ module ActionDispatch def get_routes_as_head(routes) precedence = (routes.map(&:precedence).max || 0) + 1 - routes = routes.select { |r| + routes.select { |r| r.verb === "GET" && !(r.verb === "HEAD") }.map! { |r| Route.new(r.name, @@ -126,8 +126,6 @@ module ActionDispatch route.precedence = r.precedence + precedence end } - routes.flatten! - routes end end end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 71a0c5e826..2135b280da 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -16,7 +16,7 @@ module ActionDispatch @rack_app ||= begin class_name = app.class.name.to_s if class_name == "ActionDispatch::Routing::Mapper::Constraints" - rack_app(app.app) + app.app elsif ActionDispatch::Routing::Redirect === app || class_name !~ /^ActionDispatch::Routing/ app end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index f39fd1ea35..b33c5e0dfd 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -19,6 +19,15 @@ module ActionDispatch attr_reader :app, :constraints def initialize(app, constraints, request) + # Unwrap Constraints objects. I don't actually think it's possible + # to pass a Constraints object to this constructor, but there were + # multiple places that kept testing children of this object. I + # *think* they were just being defensive, but I have no idea. + while app.is_a?(self.class) + constraints += app.constraints + app = app.app + end + @app, @constraints, @request = app, constraints, request end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 40c767e685..924455bce2 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -704,7 +704,7 @@ module ActionDispatch old_params = req.path_parameters req.path_parameters = old_params.merge params dispatcher = route.app - while dispatcher.is_a?(Mapper::Constraints) && dispatcher.matches?(env) do + if dispatcher.is_a?(Mapper::Constraints) && dispatcher.matches?(env) dispatcher = dispatcher.app end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index d900f3c7a9..107e62d20f 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -3,7 +3,6 @@ require 'uri' require 'active_support/core_ext/kernel/singleton_class' require 'active_support/core_ext/object/try' require 'rack/test' -require 'minitest' module ActionDispatch module Integration #:nodoc: diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index 7396c850ad..2ddc07ef72 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -81,8 +81,8 @@ module BareMetalTest assert_nil headers['Content-Length'] end - test "head :continue (101) does not return a content-type header" do - headers = HeadController.action(:continue).call(Rack::MockRequest.env_for("/")).second + test "head :switching_protocols (101) does not return a content-type header" do + headers = HeadController.action(:switching_protocols).call(Rack::MockRequest.env_for("/")).second assert_nil headers['Content-Type'] assert_nil headers['Content-Length'] end diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index 910ff8a80f..a4dfd0a63d 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -15,6 +15,8 @@ module TestUrlGeneration Routes.draw do get "/foo", :to => "my_route_generating#index", :as => :foo + resources :bars + mount MyRouteGeneratingController.action(:index), at: '/bar' end @@ -109,6 +111,22 @@ module TestUrlGeneration test "omit subdomain when key is blank" do assert_equal "http://example.com/foo", foo_url(subdomain: "") end + + test "generating URLs with trailing slashes" do + assert_equal "/bars.json", bars_path( + trailing_slash: true, + format: 'json' + ) + end + + test "generating URLS with querystring and trailing slashes" do + assert_equal "/bars.json?a=b", bars_path( + trailing_slash: true, + a: 'b', + format: 'json' + ) + end + end end |