Skip to content

Commit

Permalink
Fixed issue where routes with globs caused constraints on that glob to
Browse files Browse the repository at this point in the history
be ignored. A regular expression constraint gets overwritten when the
routes.rb file is processed. Changed the overwriting to an ||= instead
of an = assignment.
  • Loading branch information
maurafitz authored and pixeltrix committed Dec 4, 2012
1 parent a9dc446 commit 4243de6
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 1 deletion.
4 changes: 4 additions & 0 deletions actionpack/CHANGELOG.md
@@ -1,4 +1,8 @@
## Rails 4.0.0 (unreleased) ## ## Rails 4.0.0 (unreleased) ##
* Fixed a bug that ignores constraints on a glob route. This was caused because the constraint
regular expression is overwritten when the `routes.rb` file is processed. Fixes #7924

*Maura Fitzgerald*


* More descriptive error messages when calling `render :partial` with * More descriptive error messages when calling `render :partial` with
an invalid `:layout` argument. an invalid `:layout` argument.
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/routing/mapper.rb
Expand Up @@ -158,7 +158,7 @@ def conditions
def requirements def requirements
@requirements ||= (@options[:constraints].is_a?(Hash) ? @options[:constraints] : {}).tap do |requirements| @requirements ||= (@options[:constraints].is_a?(Hash) ? @options[:constraints] : {}).tap do |requirements|
requirements.reverse_merge!(@scope[:constraints]) if @scope[:constraints] requirements.reverse_merge!(@scope[:constraints]) if @scope[:constraints]
@options.each { |k, v| requirements[k] = v if v.is_a?(Regexp) } @options.each { |k, v| requirements[k] ||= v if v.is_a?(Regexp) }
end end
end end


Expand Down
29 changes: 29 additions & 0 deletions actionpack/test/dispatch/routing_test.rb
Expand Up @@ -3065,6 +3065,35 @@ def app; Routes end
end end
end end


class TestGlobRoutingMapper < ActionDispatch::IntegrationTest
Routes = ActionDispatch::Routing::RouteSet.new.tap do |app|
app.draw do
ok = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] }

get "/*id" => redirect("/not_cars"), :constraints => {id: /dummy/}
get "/cars" => ok
end
end

#include Routes.url_helpers
def app; Routes end

def test_glob_constraint
get "/dummy"
assert_equal "301", @response.code
assert_equal "/not_cars", @response.header['Location'].match('/[^/]+$')[0]
end

def test_glob_constraint_skip_route
get "/cars"
assert_equal "200", @response.code
end
def test_glob_constraint_skip_all
get "/missing"
assert_equal "404", @response.code
end
end

class TestOptimizedNamedRoutes < ActionDispatch::IntegrationTest class TestOptimizedNamedRoutes < ActionDispatch::IntegrationTest
Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| Routes = ActionDispatch::Routing::RouteSet.new.tap do |app|
app.draw do app.draw do
Expand Down

0 comments on commit 4243de6

Please sign in to comment.