aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG.md6
-rw-r--r--actionpack/lib/abstract_controller/base.rb2
-rw-r--r--actionpack/lib/action_controller/metal.rb2
-rw-r--r--actionpack/lib/action_controller/metal/renderers.rb2
-rw-r--r--actionpack/lib/action_dispatch/http/url.rb20
-rw-r--r--actionpack/lib/action_dispatch/journey/route.rb2
-rw-r--r--actionpack/lib/action_dispatch/journey/router.rb4
-rw-r--r--actionpack/lib/action_dispatch/routing/inspector.rb2
-rw-r--r--actionpack/lib/action_dispatch/routing/mapper.rb9
-rw-r--r--actionpack/lib/action_dispatch/routing/route_set.rb2
-rw-r--r--actionpack/lib/action_dispatch/testing/integration.rb1
-rw-r--r--actionpack/test/controller/new_base/bare_metal_test.rb4
-rw-r--r--actionpack/test/dispatch/url_generation_test.rb18
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