Merge branch 'master' into activestorage-guide

10 files changed, 109 insertions, 14 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 32239d202c..e01f88e902 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Make `assert_recognizes` to traverse mounted engines
+
+    *Yuichiro Kaneko*
+
 *   Remove deprecated `ActionController::ParamsParser::ParseError`.
 
     *Rafael Mendonça França*
diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
index d6cd5fd9e0..bd133f24a1 100644
--- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb
+++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
@@ -248,6 +248,7 @@ module ActionController #:nodoc:
         "If you know what you're doing, go ahead and disable forgery " \
         "protection on this action to permit cross-origin JavaScript embedding."
       private_constant :CROSS_ORIGIN_JAVASCRIPT_WARNING
+      # :startdoc:
 
       # If `verify_authenticity_token` was run (indicating that we have
       # forgery protection enabled for this request) then also verify that
diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb
index 41a47f2c82..ec86b8bc47 100644
--- a/actionpack/lib/action_dispatch/http/filter_parameters.rb
+++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb
@@ -9,7 +9,7 @@ module ActionDispatch
     # sub-hashes of the params hash to filter. Filtering only certain sub-keys
     # from a hash is possible by using the dot notation: 'credit_card.number'.
     # If a block is given, each key and value of the params hash and all
-    # sub-hashes is passed to it, the value or key can be replaced using
+    # sub-hashes is passed to it, where the value or the key can be replaced using
     # String#replace or similar method.
     #
     #   env["action_dispatch.parameter_filter"] = [:password]
@@ -48,7 +48,7 @@ module ActionDispatch
         @filtered_env ||= env_filter.filter(@env)
       end
 
-      # Reconstructed a path with all sensitive GET parameters replaced.
+      # Reconstructs a path with all sensitive GET parameters replaced.
       def filtered_path
         @filtered_path ||= query_string.empty? ? path : "#{path}?#{filtered_query_string}"
       end
diff --git a/actionpack/lib/action_dispatch/routing/endpoint.rb b/actionpack/lib/action_dispatch/routing/endpoint.rb
index e911b6537b..24dced1efd 100644
--- a/actionpack/lib/action_dispatch/routing/endpoint.rb
+++ b/actionpack/lib/action_dispatch/routing/endpoint.rb
@@ -3,10 +3,12 @@
 module ActionDispatch
   module Routing
     class Endpoint # :nodoc:
-      def dispatcher?;   false; end
-      def redirect?;     false; end
-      def matches?(req); true;  end
-      def app;           self;  end
+      def dispatcher?;                           false; end
+      def redirect?;                             false; end
+      def engine?;       rack_app.respond_to?(:routes); end
+      def matches?(req);                          true; end
+      def app;                                    self; end
+      def rack_app;                                app; end
     end
   end
 end
diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb
index b2868b7427..a2205569b4 100644
--- a/actionpack/lib/action_dispatch/routing/inspector.rb
+++ b/actionpack/lib/action_dispatch/routing/inspector.rb
@@ -15,7 +15,7 @@ module ActionDispatch
       end
 
       def rack_app
-        app.app
+        app.rack_app
       end
 
       def path
@@ -47,7 +47,7 @@ module ActionDispatch
       end
 
       def engine?
-        rack_app.respond_to?(:routes)
+        app.engine?
       end
     end
 
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index dea8387c3d..ded42adee9 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -475,6 +475,16 @@ module ActionDispatch
         #
         #      resources :users, param: :name
         #
+        #   The +users+ resource here will have the following routes generated for it:
+        #
+        #      GET       /users(.:format)
+        #      POST      /users(.:format)
+        #      GET       /users/new(.:format)
+        #      GET       /users/:name/edit(.:format)
+        #      GET       /users/:name(.:format)
+        #      PATCH/PUT /users/:name(.:format)
+        #      DELETE    /users/:name(.:format)
+        #
         #   You can override <tt>ActiveRecord::Base#to_param</tt> of a related
         #   model to construct a URL:
         #
@@ -484,8 +494,8 @@ module ActionDispatch
         #        end
         #      end
         #
-        #   user = User.find_by(name: 'Phusion')
-        #   user_path(user)  # => "/users/Phusion"
+        #      user = User.find_by(name: 'Phusion')
+        #      user_path(user)  # => "/users/Phusion"
         #
         # [:path]
         #   The path prefix for the routes.
@@ -1265,7 +1275,7 @@ module ActionDispatch
         #   POST      /profile
         #
         # === Options
-        # Takes same options as +resources+.
+        # Takes same options as resources[rdoc-ref:#resources]
         def resource(*resources, &block)
           options = resources.extract_options!.dup
 
@@ -1330,7 +1340,7 @@ module ActionDispatch
         #   DELETE    /photos/:photo_id/comments/:id
         #
         # === Options
-        # Takes same options as <tt>Base#match</tt> as well as:
+        # Takes same options as match[rdoc-ref:Base#match] as well as:
         #
         # [:path_names]
         #   Allows you to change the segment component of the +edit+ and +new+ actions.
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 71cb458112..987e709f6f 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -842,6 +842,10 @@ module ActionDispatch
         end
 
         req = make_request(env)
+        recognize_path_with_request(req, path, extras)
+      end
+
+      def recognize_path_with_request(req, path, extras)
         @router.recognize(req) do |route, params|
           params.merge!(extras)
           params.each do |key, value|
@@ -860,6 +864,9 @@ module ActionDispatch
             end
 
             return req.path_parameters
+          elsif app.matches?(req) && app.engine?
+            path_parameters = app.rack_app.routes.recognize_path_with_request(req, path, extras)
+            return path_parameters
           end
         end
 
diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb
index 770fbde74e..2687772b4b 100644
--- a/actionpack/lib/action_dispatch/system_testing/driver.rb
+++ b/actionpack/lib/action_dispatch/system_testing/driver.rb
@@ -59,7 +59,7 @@ module ActionDispatch
 
         def register_webkit(app)
           Capybara::Webkit::Driver.new(app, Capybara::Webkit::Configuration.to_hash.merge(@options)).tap do |driver|
-            driver.resize_window(*@screen_size)
+            driver.resize_window_to(driver.current_window_handle, *@screen_size)
           end
         end
 
diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb
index f0f106c8ba..be455642de 100644
--- a/actionpack/test/controller/log_subscriber_test.rb
+++ b/actionpack/test/controller/log_subscriber_test.rb
@@ -98,7 +98,7 @@ class ACLogSubscriberTest < ActionController::TestCase
 
     @old_logger = ActionController::Base.logger
 
-    @cache_path = File.join Dir.tmpdir, Dir::Tmpname.make_tmpname("tmp", "cache")
+    @cache_path = Dir.mktmpdir(%w[tmp cache])
     @controller.cache_store = :file_store, @cache_path
     ActionController::LogSubscriber.attach_to :action_controller
   end
diff --git a/actionpack/test/dispatch/routing_assertions_test.rb b/actionpack/test/dispatch/routing_assertions_test.rb
index e492a56653..a5198f2f13 100644
--- a/actionpack/test/dispatch/routing_assertions_test.rb
+++ b/actionpack/test/dispatch/routing_assertions_test.rb
@@ -1,14 +1,40 @@
 # frozen_string_literal: true
 
 require "abstract_unit"
+require "rails/engine"
 require "controller/fake_controllers"
 
 class SecureArticlesController < ArticlesController; end
 class BlockArticlesController < ArticlesController; end
 class QueryArticlesController < ArticlesController; end
 
+class SecureBooksController < BooksController; end
+class BlockBooksController < BooksController; end
+class QueryBooksController < BooksController; end
+
 class RoutingAssertionsTest < ActionController::TestCase
   def setup
+    engine = Class.new(Rails::Engine) do
+      def self.name
+        "blog_engine"
+      end
+    end
+    engine.routes.draw do
+      resources :books
+
+      scope "secure", constraints: { protocol: "https://" } do
+        resources :books, controller: "secure_books"
+      end
+
+      scope "block", constraints: lambda { |r| r.ssl? } do
+        resources :books, controller: "block_books"
+      end
+
+      scope "query", constraints: lambda { |r| r.params[:use_query] == "true" } do
+        resources :books, controller: "query_books"
+      end
+    end
+
     @routes = ActionDispatch::Routing::RouteSet.new
     @routes.draw do
       resources :articles
@@ -24,6 +50,8 @@ class RoutingAssertionsTest < ActionController::TestCase
       scope "query", constraints: lambda { |r| r.params[:use_query] == "true" } do
         resources :articles, controller: "query_articles"
       end
+
+      mount engine => "/shelf"
     end
   end
 
@@ -83,6 +111,49 @@ class RoutingAssertionsTest < ActionController::TestCase
     assert_match err.message, "This is a really bad msg"
   end
 
+  def test_assert_recognizes_with_engine
+    assert_recognizes({ controller: "books", action: "index" }, "/shelf/books")
+    assert_recognizes({ controller: "books", action: "show", id: "1" }, "/shelf/books/1")
+  end
+
+  def test_assert_recognizes_with_engine_and_extras
+    assert_recognizes({ controller: "books", action: "index", page: "1" }, "/shelf/books", page: "1")
+  end
+
+  def test_assert_recognizes_with_engine_and_method
+    assert_recognizes({ controller: "books", action: "create" }, { path: "/shelf/books", method: :post })
+    assert_recognizes({ controller: "books", action: "update", id: "1" }, { path: "/shelf/books/1", method: :put })
+  end
+
+  def test_assert_recognizes_with_engine_and_hash_constraint
+    assert_raise(Assertion) do
+      assert_recognizes({ controller: "secure_books", action: "index" }, "http://test.host/shelf/secure/books")
+    end
+    assert_recognizes({ controller: "secure_books", action: "index", protocol: "https://" }, "https://test.host/shelf/secure/books")
+  end
+
+  def test_assert_recognizes_with_engine_and_block_constraint
+    assert_raise(Assertion) do
+      assert_recognizes({ controller: "block_books", action: "index" }, "http://test.host/shelf/block/books")
+    end
+    assert_recognizes({ controller: "block_books", action: "index" }, "https://test.host/shelf/block/books")
+  end
+
+  def test_assert_recognizes_with_engine_and_query_constraint
+    assert_raise(Assertion) do
+      assert_recognizes({ controller: "query_books", action: "index", use_query: "false" }, "/shelf/query/books", use_query: "false")
+    end
+    assert_recognizes({ controller: "query_books", action: "index", use_query: "true" }, "/shelf/query/books", use_query: "true")
+  end
+
+  def test_assert_recognizes_raises_message_with_engine
+    err = assert_raise(Assertion) do
+      assert_recognizes({ controller: "secure_books", action: "index" }, "http://test.host/shelf/secure/books", {}, "This is a really bad msg")
+    end
+
+    assert_match err.message, "This is a really bad msg"
+  end
+
   def test_assert_routing
     assert_routing("/articles", controller: "articles", action: "index")
   end