From 837e40dcac023319a0bfc38240761d4352b73b99 Mon Sep 17 00:00:00 2001 From: Dave Gynn Date: Sat, 26 Dec 2015 22:25:27 -0800 Subject: restore ability to pass extra options to cache stores The `cache` helper methods should pass any extra options to the cache store. For example :expires_in would be a valid option if memcache was the cache store. The change in commit da16745 broke the ability to pass any options other than :skip_digest and :virtual_path. This PR restores that functionality and adds a test for it. --- actionpack/test/controller/caching_test.rb | 12 ++++++++++++ .../functional_caching/fragment_cached_with_options.html.erb | 3 +++ 2 files changed, 15 insertions(+) create mode 100644 actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb (limited to 'actionpack/test') diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index d19b3810c2..74c78dfa8e 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -172,6 +172,9 @@ class FunctionalCachingController < CachingController def fragment_cached_without_digest end + + def fragment_cached_with_options + end end class FunctionalFragmentCachingTest < ActionController::TestCase @@ -215,6 +218,15 @@ CACHED assert_equal "

ERB

", @store.read("views/nodigest") end + def test_fragment_caching_with_options + get :fragment_cached_with_options + assert_response :success + expected_body = "\n

ERB

\n\n" + + assert_equal expected_body, @response.body + assert_equal "

ERB

", @store.read("views/with_options") + end + def test_render_inline_before_fragment_caching get :inline_fragment_cached assert_response :success diff --git a/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb b/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb new file mode 100644 index 0000000000..6865df9b7e --- /dev/null +++ b/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb @@ -0,0 +1,3 @@ + +<%= cache 'with_options', :skip_digest => true, :expires_in => 1.minute do %>

ERB

<% end %> + -- cgit v1.2.3 From 52bb2d36d3141dcd8217221065d9b5fa2b12deba Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Sat, 19 Sep 2015 00:11:18 +0200 Subject: Add `as` to encode a request as a specific mime type. Turns ``` post articles_path(format: :json), params: { article: { name: 'Ahoy!' } }.to_json, headers: { 'Content-Type' => 'application/json' } ``` into ``` post articles_path, params: { article: { name: 'Ahoy!' } }, as: :json ``` --- actionpack/test/controller/integration_test.rb | 43 ++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index d0a1d1285f..296bc1baad 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -1126,3 +1126,46 @@ class IntegrationRequestsWithSessionSetup < ActionDispatch::IntegrationTest assert_equal({"user_name"=>"david"}, cookies.to_hash) end end + +class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest + class FooController < ActionController::Base + def foos + render plain: 'ok' + end + end + + def test_encoding_as_json + assert_encoded_as :json, content_type: 'application/json' + end + + def test_encoding_as_without_mime_registration + assert_raise ArgumentError do + ActionDispatch::IntegrationTest.register_encoder :wibble + end + end + + def test_registering_custom_encoder + Mime::Type.register 'text/wibble', :wibble + + ActionDispatch::IntegrationTest.register_encoder(:wibble, &:itself) + + assert_encoded_as :wibble, content_type: 'text/wibble', + parsed_parameters: Hash.new # Unregistered MIME Type can't be parsed + ensure + Mime::Type.unregister :wibble + end + + private + def assert_encoded_as(format, content_type:, parsed_parameters: { 'foo' => 'fighters' }) + with_routing do |routes| + routes.draw { post ':action' => FooController } + + post '/foos', params: { foo: 'fighters' }, as: format + + assert_response :success + assert_match "foos.#{format}", request.path + assert_equal content_type, request.content_type + assert_equal parsed_parameters, request.request_parameters + end + end +end -- cgit v1.2.3 From 0361d8449ff1c18da041df4b7dfe648abf0f1887 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 26 Jan 2016 17:06:31 -0800 Subject: clear view path cache between tests The cache for `render file:` seems to also be used in the case of `render(string)`. If one is supposed to be a hit and the other is supposed to be a miss, and they both reference the same file, then the cache could return incorrect values. This commit clears the cache between runs so that we get non-cached behavior. --- actionpack/test/controller/render_test.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 3f569230c2..db73de6010 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -253,6 +253,11 @@ end class ExpiresInRenderTest < ActionController::TestCase tests TestController + def setup + super + ActionController::Base.view_paths.paths.each(&:clear_cache) + end + def test_dynamic_render_with_file # This is extremely bad, but should be possible to do. assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) -- cgit v1.2.3 From 00285e7cf75c96553719072a27c27e4ab7d25b40 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 26 Jan 2016 18:00:05 -0800 Subject: fix permitted? conditional for `render` calls --- actionpack/test/controller/render_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index db73de6010..f205b96ce8 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -66,6 +66,10 @@ class TestController < ActionController::Base render params[:id] # => String, AC:Params end + def dynamic_render_permit + render params[:id].permit(:file) + end + def dynamic_render_with_file # This is extremely bad, but should be possible to do. file = params[:id] # => String, AC:Params @@ -273,6 +277,13 @@ class ExpiresInRenderTest < ActionController::TestCase end end + def test_permitted_dynamic_render_file_hash + assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) + response = get :dynamic_render_permit, { id: { file: '../\\../test/abstract_unit.rb' } } + assert_equal File.read(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')), + response.body + end + def test_dynamic_render_file_hash assert_raises ArgumentError do get :dynamic_render, params: { id: { file: '../\\../test/abstract_unit.rb' } } -- cgit v1.2.3 From 3844854af109fb9eee75c90bacf8bf87eb2bf968 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 26 Jan 2016 18:01:24 -0800 Subject: add a skip for failing test --- actionpack/test/controller/render_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index f205b96ce8..ea789b6121 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -278,8 +278,9 @@ class ExpiresInRenderTest < ActionController::TestCase end def test_permitted_dynamic_render_file_hash + skip "FIXME: this test passes on 4-2-stable but not master. Why?" assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) - response = get :dynamic_render_permit, { id: { file: '../\\../test/abstract_unit.rb' } } + response = get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } } assert_equal File.read(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')), response.body end -- cgit v1.2.3 From 5262bf544abacb10828abcfeb046d8146bef4c14 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Wed, 27 Jan 2016 18:19:15 +0900 Subject: doc typo [ci skip] --- actionpack/test/controller/render_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index ea789b6121..d1b9586533 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -63,7 +63,7 @@ class TestController < ActionController::Base end def dynamic_render - render params[:id] # => String, AC:Params + render params[:id] # => String, AC::Params end def dynamic_render_permit @@ -72,7 +72,7 @@ class TestController < ActionController::Base def dynamic_render_with_file # This is extremely bad, but should be possible to do. - file = params[:id] # => String, AC:Params + file = params[:id] # => String, AC::Params render file: file end -- cgit v1.2.3 From 020d6fda29c1f04818dbf467764fc8ac16b7042f Mon Sep 17 00:00:00 2001 From: eileencodes Date: Thu, 28 Jan 2016 14:18:01 -0500 Subject: Regression test for rendering file from absolute path Test that we are not allowing you to grab a file with an absolute path outside of your application directory. This is dangerous because it could be used to retrieve files from the server like `/etc/passwd`. --- actionpack/test/controller/render_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index d1b9586533..2e1a687513 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -270,6 +270,17 @@ class ExpiresInRenderTest < ActionController::TestCase response.body end + def test_dynamic_render_with_absolute_path + file = Tempfile.new + file.write "secrets!" + file.flush + assert_raises ActionView::MissingTemplate do + response = get :dynamic_render, params: { id: file.path } + end + ensure + file.unlink + end + def test_dynamic_render assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) assert_raises ActionView::MissingTemplate do -- cgit v1.2.3 From 5181d5c283430762ea444d26e72890501a6584ff Mon Sep 17 00:00:00 2001 From: eileencodes Date: Thu, 28 Jan 2016 15:22:02 -0500 Subject: Run `file.close` before unlinking for travis This works on OSX but for some reason travis is throwing a ``` 1) Error: ExpiresInRenderTest#test_dynamic_render_with_absolute_path: NoMethodError: undefined method `unlink' for nil:NilClass ``` Looking at other tests in Railties the file has a name and we close it before unlinking, so I'm going to try that. --- actionpack/test/controller/render_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 2e1a687513..9430ab8db8 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -271,13 +271,14 @@ class ExpiresInRenderTest < ActionController::TestCase end def test_dynamic_render_with_absolute_path - file = Tempfile.new + file = Tempfile.new('name') file.write "secrets!" file.flush assert_raises ActionView::MissingTemplate do response = get :dynamic_render, params: { id: file.path } end ensure + file.close file.unlink end -- cgit v1.2.3 From 647a04cdbcb4e3abccb9487cdbc7366b4d923ea4 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Sat, 30 Jan 2016 13:40:12 +0900 Subject: remove unused variable from render test This removes the following warning. ``` rails/actionpack/test/controller/render_test.rb:278: warning: assigned but unused variable - response ``` --- actionpack/test/controller/render_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 9430ab8db8..c814d4ea54 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -275,7 +275,7 @@ class ExpiresInRenderTest < ActionController::TestCase file.write "secrets!" file.flush assert_raises ActionView::MissingTemplate do - response = get :dynamic_render, params: { id: file.path } + get :dynamic_render, params: { id: file.path } end ensure file.close -- cgit v1.2.3 From c4d85dfbc71043e2a746acd310e32f4f04db801a Mon Sep 17 00:00:00 2001 From: eileencodes Date: Tue, 26 Jan 2016 19:15:09 -0500 Subject: Handle response_body= when body is nil There are some cases when the `body` in `response_body=` can be set to nil. One of those cases is in `actionpack-action_caching` which I found while upgrading it for Rails 5. It's not possible to run `body.each` on a `nil` body so we have to return after we run `response.reset_body!`. --- actionpack/test/controller/new_base/bare_metal_test.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index c226fa57ee..ee3c498b1c 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -40,6 +40,22 @@ module BareMetalTest end end + class BareEmptyController < ActionController::Metal + def index + self.response_body = nil + end + end + + class BareEmptyTest < ActiveSupport::TestCase + test "response body is nil" do + controller = BareEmptyController.new + controller.set_request!(ActionDispatch::Request.empty) + controller.set_response!(BareController.make_response!(controller.request)) + controller.index + assert_equal nil, controller.response_body + end + end + class HeadController < ActionController::Metal include ActionController::Head -- cgit v1.2.3 From 8a436fdd98c63cc0a7a6d2c642c18d33421dc6ad Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Mon, 25 Jan 2016 01:32:44 +0530 Subject: Add options for rake routes task Add two options: `-c` and `-g`. `-g` option returns the urls name, verb and path fields that match the pattern. `-c` option returns the urls for specific controller. Fixes #18902, and Fixes #20420 [Anton Davydov & Vipul A M] --- actionpack/test/dispatch/routing/inspector_test.rb | 34 +++++++++++++++++----- 1 file changed, 27 insertions(+), 7 deletions(-) (limited to 'actionpack/test') diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index 7382c267c7..f72a87b994 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -17,10 +17,10 @@ module ActionDispatch @set = ActionDispatch::Routing::RouteSet.new end - def draw(options = {}, &block) + def draw(options = nil, &block) @set.draw(&block) inspector = ActionDispatch::Routing::RoutesInspector.new(@set.routes) - inspector.format(ActionDispatch::Routing::ConsoleFormatter.new, options[:filter]).split("\n") + inspector.format(ActionDispatch::Routing::ConsoleFormatter.new, options).split("\n") end def test_displaying_routes_for_engines @@ -297,7 +297,7 @@ module ActionDispatch end def test_routes_can_be_filtered - output = draw(filter: 'posts') do + output = draw('posts') do resources :articles resources :posts end @@ -313,6 +313,26 @@ module ActionDispatch " DELETE /posts/:id(.:format) posts#destroy"], output end + def test_routes_can_be_filtered_with_namespaced_controllers + output = draw('admin/posts') do + resources :articles + namespace :admin do + resources :posts + end + end + + assert_equal [" Prefix Verb URI Pattern Controller#Action", + " admin_posts GET /admin/posts(.:format) admin/posts#index", + " POST /admin/posts(.:format) admin/posts#create", + " new_admin_post GET /admin/posts/new(.:format) admin/posts#new", + "edit_admin_post GET /admin/posts/:id/edit(.:format) admin/posts#edit", + " admin_post GET /admin/posts/:id(.:format) admin/posts#show", + " PATCH /admin/posts/:id(.:format) admin/posts#update", + " PUT /admin/posts/:id(.:format) admin/posts#update", + " DELETE /admin/posts/:id(.:format) admin/posts#destroy"], output + end + + def test_regression_route_with_controller_regexp output = draw do get ':controller(/:action)', controller: /api\/[^\/]+/, format: false @@ -336,18 +356,18 @@ module ActionDispatch end def test_routes_with_undefined_filter - output = draw(:filter => 'Rails::MissingController') do + output = draw(controller: 'Rails::MissingController') do get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ end assert_equal [ - "The controller Rails::MissingController does not exist!", + "No routes were found for this controller", "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." ], output end def test_no_routes_matched_filter - output = draw(:filter => 'rails/dummy') do + output = draw('rails/dummy') do get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ end @@ -358,7 +378,7 @@ module ActionDispatch end def test_no_routes_were_defined - output = draw(:filter => 'Rails::DummyController') { } + output = draw('Rails::DummyController') {} assert_equal [ "You don't have any routes defined!", -- cgit v1.2.3 From a640da454fdb9cd8806a1b6bd98c2da93f1b53b9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 5 Feb 2016 11:42:06 -0800 Subject: disable controller / view thread spawning in tests Tests can (and do) access the database from the main thread. In this case they were starting a transaction, then making a request. The request would create a new thread, which would allocate a new database connection. Since the main thread started a transaction that contains data that the new thread wants to see, the new thread would not see it due to data visibility from transactions. Spawning the new thread in production is fine because middleware should not be doing database manipulation similar to the test harness. Before 603fe20c it was possible to set the database connection id based on a thread local, but 603fe20c changes the connection lookup code to never look at the "connection id" but only at the thread object itself. Without that indirection, we can't force threads to use the same connection pool as another thread. Fixes #23483 --- actionpack/test/controller/live_stream_test.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 2ef9734269..0c3884cd38 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -152,7 +152,6 @@ module ActionController def thread_locals tc.assert_equal 'aaron', Thread.current[:setting] - tc.assert_not_equal Thread.current.object_id, Thread.current[:originating_thread] response.headers['Content-Type'] = 'text/event-stream' %w{ hello world }.each do |word| @@ -261,6 +260,14 @@ module ActionController end end + def setup + super + + def @controller.new_controller_thread + Thread.new { yield } + end + end + def test_set_cookie get :set_cookie assert_equal({'hello' => 'world'}, @response.cookies) -- cgit v1.2.3