Fix assert_recognizes with block constraints [#5805 state:resolved]

4 files changed, 116 insertions, 6 deletions

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index f3f7cb6507..2f12192af4 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -22,18 +22,22 @@ module ActionDispatch
           @app, @constraints, @request = app, constraints, request
         end
 
-        def call(env)
+        def matches?(env)
           req = @request.new(env)
 
           @constraints.each { |constraint|
             if constraint.respond_to?(:matches?) && !constraint.matches?(req)
-              return [ 404, {'X-Cascade' => 'pass'}, [] ]
+              return false
             elsif constraint.respond_to?(:call) && !constraint.call(*constraint_args(constraint, req))
-              return [ 404, {'X-Cascade' => 'pass'}, [] ]
+              return false
             end
           }
 
-          @app.call(env)
+          return true
+        end
+
+        def call(env)
+          matches?(env) ? @app.call(env) : [ 404, {'X-Cascade' => 'pass'}, [] ]
         end
 
         private
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 683fa19380..4b4e9da173 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -540,7 +540,9 @@ module ActionDispatch
           end
 
           dispatcher = route.app
-          dispatcher = dispatcher.app while dispatcher.is_a?(Mapper::Constraints)
+          while dispatcher.is_a?(Mapper::Constraints) && dispatcher.matches?(env) do
+            dispatcher = dispatcher.app
+          end
 
           if dispatcher.is_a?(Dispatcher) && dispatcher.controller(params, false)
             dispatcher.prepare_params!(params)
diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
index 1390b74a95..f440e74691 100644
--- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb
+++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
@@ -124,7 +124,8 @@ module ActionDispatch
           options[:controller] = "/#{controller}"
         end
 
-        assert_generates(path.is_a?(Hash) ? path[:path] : path, options, defaults, extras, message)
+        generate_options = options.dup.delete_if{ |k,v| defaults.key?(k) }
+        assert_generates(path.is_a?(Hash) ? path[:path] : path, generate_options, defaults, extras, message)
       end
 
       # A helper to make it easier to test different route configurations.
diff --git a/actionpack/test/dispatch/routing_assertions_test.rb b/actionpack/test/dispatch/routing_assertions_test.rb
new file mode 100644
index 0000000000..9f95d82129
--- /dev/null
+++ b/actionpack/test/dispatch/routing_assertions_test.rb
@@ -0,0 +1,103 @@
+require 'abstract_unit'
+require 'controller/fake_controllers'
+
+class SecureArticlesController < ArticlesController; end
+class BlockArticlesController < ArticlesController; end
+
+class RoutingAssertionsTest < ActionController::TestCase
+
+  def setup
+    @routes = ActionDispatch::Routing::RouteSet.new
+    @routes.draw do
+      resources :articles
+
+      scope 'secure', :constraints => { :protocol => 'https://' } do
+        resources :articles, :controller => 'secure_articles'
+      end
+
+      scope 'block', :constraints => lambda { |r| r.ssl? } do
+        resources :articles, :controller => 'block_articles'
+      end
+    end
+  end
+
+  def test_assert_generates
+    assert_generates('/articles', { :controller => 'articles', :action => 'index' })
+    assert_generates('/articles/1', { :controller => 'articles', :action => 'show', :id => '1' })
+  end
+
+  def test_assert_generates_with_defaults
+    assert_generates('/articles/1/edit', { :controller => 'articles', :action => 'edit' }, { :id => '1' })
+  end
+
+  def test_assert_generates_with_extras
+    assert_generates('/articles', { :controller => 'articles', :action => 'index', :page => '1' }, {}, { :page => '1' })
+  end
+
+  def test_assert_recognizes
+    assert_recognizes({ :controller => 'articles', :action => 'index' }, '/articles')
+    assert_recognizes({ :controller => 'articles', :action => 'show', :id => '1' }, '/articles/1')
+  end
+
+  def test_assert_recognizes_with_extras
+    assert_recognizes({ :controller => 'articles', :action => 'index', :page => '1' }, '/articles', { :page => '1' })
+  end
+ 
+  def test_assert_recognizes_with_method
+    assert_recognizes({ :controller => 'articles', :action => 'create' }, { :path => '/articles', :method => :post })
+    assert_recognizes({ :controller => 'articles', :action => 'update', :id => '1' }, { :path => '/articles/1', :method => :put })
+  end
+
+  def test_assert_recognizes_with_hash_constraint
+    assert_raise(ActionController::RoutingError) do
+      assert_recognizes({ :controller => 'secure_articles', :action => 'index' }, 'http://test.host/secure/articles')
+    end
+    assert_recognizes({ :controller => 'secure_articles', :action => 'index' }, 'https://test.host/secure/articles')
+  end
+
+  def test_assert_recognizes_with_block_constraint
+    assert_raise(ActionController::RoutingError) do
+      assert_recognizes({ :controller => 'block_articles', :action => 'index' }, 'http://test.host/block/articles')
+    end
+    assert_recognizes({ :controller => 'block_articles', :action => 'index' }, 'https://test.host/block/articles')
+  end
+
+  def test_assert_routing
+    assert_routing('/articles', :controller => 'articles', :action => 'index')
+  end
+
+  def test_assert_routing_with_defaults
+    assert_routing('/articles/1/edit', { :controller => 'articles', :action => 'edit', :id => '1' }, { :id => '1' })
+  end
+
+  def test_assert_routing_with_extras
+    assert_routing('/articles', { :controller => 'articles', :action => 'index', :page => '1' }, { }, { :page => '1' })
+  end
+
+  def test_assert_routing_with_hash_constraint
+    assert_raise(ActionController::RoutingError) do
+      assert_routing('http://test.host/secure/articles', { :controller => 'secure_articles', :action => 'index' })
+    end
+    assert_routing('https://test.host/secure/articles', { :controller => 'secure_articles', :action => 'index' })
+  end
+
+  def test_assert_routing_with_block_constraint
+    assert_raise(ActionController::RoutingError) do
+      assert_routing('http://test.host/block/articles', { :controller => 'block_articles', :action => 'index' })
+    end
+    assert_routing('https://test.host/block/articles', { :controller => 'block_articles', :action => 'index' })
+  end
+
+  def test_with_routing
+    with_routing do |routes|
+      routes.draw do
+        resources :articles, :path => 'artikel'
+      end
+
+      assert_routing('/artikel', :controller => 'articles', :action => 'index')
+      assert_raise(ActionController::RoutingError) do
+        assert_routing('/articles', { :controller => 'articles', :action => 'index' })
+      end
+    end
+  end
+end