diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG.md | 56 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/log_subscriber.rb | 4 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/instrumentation.rb | 4 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/live.rb | 51 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/request_forgery_protection.rb | 3 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/test_case.rb | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/mime_types.rb | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing.rb | 3 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/inspector.rb | 5 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/testing/integration.rb | 101 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/testing/test_response.rb | 6 | ||||
| -rw-r--r-- | actionpack/test/controller/caching_test.rb | 23 | ||||
| -rw-r--r-- | actionpack/test/controller/integration_test.rb | 66 |
13 files changed, 284 insertions, 40 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 809b735deb..931313612c 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,10 +1,60 @@ +* Add request encoding and response parsing to integration tests. + + What previously was: + + ```ruby + require 'test_helper' + + class ApiTest < ActionDispatch::IntegrationTest + test 'creates articles' do + assert_difference -> { Article.count } do + post articles_path(format: :json), + params: { article: { title: 'Ahoy!' } }.to_json, + headers: { 'Content-Type' => 'application/json' } + end + + assert_equal({ 'id' => Article.last.id, 'title' => 'Ahoy!' }, JSON.parse(response.body)) + end + end + ``` + + Can now be written as: + + ```ruby + require 'test_helper' + + class ApiTest < ActionDispatch::IntegrationTest + test 'creates articles' do + assert_difference -> { Article.count } do + post articles_path, params: { article: { title: 'Ahoy!' } }, as: :json + end + + assert_equal({ 'id' => Article.last.id, 'title' => 'Ahoy!' }, response.parsed_body) + end + end + ``` + + Passing `as: :json` to integration test request helpers will set the format, + content type and encode the parameters as JSON. + + Then on the response side, `parsed_body` will parse the body according to the + content type the response has. + + Currently JSON is the only supported MIME type. Add your own with + `ActionDispatch::IntegrationTest.register_encoder`. + + *Kasper Timm Hansen* + +* Add image/svg+xml as a default mime type. + + *DHH* + ## Rails 5.0.0.beta2 (February 01, 2016) ## -* Add `-g` and `-c` (short for _grep_ and _controller_ respectively) options - to `bin/rake routes`. These options return the url `name`, `verb` and +* Add `-g` and `-c` options to `bin/rails routes`. These options return the url `name`, `verb` and `path` field that match the pattern or match a specific controller. - Deprecate `CONTROLLER` env variable in `bin/rake routes`. + Deprecate `CONTROLLER` env variable in `bin/rails routes`. See #18902. diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index d1d6acac26..a0917b4fdb 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -25,8 +25,8 @@ module ActionController status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name) end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms" - message << " (#{additions.join(" | ".freeze)})" unless additions.blank? - message << "\n\n" if Rails.env.development? + message << " (#{additions.join(" | ".freeze)})" unless additions.empty? + message << "\n\n" if defined?(Rails.env) && Rails.env.development? message end diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 3dbf34eb2a..bf74b39ac4 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -19,9 +19,9 @@ module ActionController :controller => self.class.name, :action => self.action_name, :params => request.filtered_parameters, - :format => request.format.try(:ref), + :format => request.format.ref, :method => request.request_method, - :path => (request.fullpath rescue "unknown") + :path => request.fullpath } ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup) diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index acc4507b2d..fc20e7a421 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -238,34 +238,39 @@ module ActionController # response code and headers back up the rack stack, and still process # the body in parallel with sending data to the client new_controller_thread { - t2 = Thread.current - - # Since we're processing the view in a different thread, copy the - # thread locals from the main thread to the child thread. :'( - locals.each { |k,v| t2[k] = v } - - begin - super(name) - rescue => e - if @_response.committed? - begin - @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html - @_response.stream.call_on_error - rescue => exception - log_error(exception) - ensure - log_error(e) - @_response.stream.close + ActiveSupport::Dependencies.interlock.running do + t2 = Thread.current + + # Since we're processing the view in a different thread, copy the + # thread locals from the main thread to the child thread. :'( + locals.each { |k,v| t2[k] = v } + + begin + super(name) + rescue => e + if @_response.committed? + begin + @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html + @_response.stream.call_on_error + rescue => exception + log_error(exception) + ensure + log_error(e) + @_response.stream.close + end + else + error = e end - else - error = e + ensure + @_response.commit! end - ensure - @_response.commit! end } - @_response.await_commit + ActiveSupport::Dependencies.interlock.permit_concurrent_loads do + @_response.await_commit + end + raise error if error end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 91b3403ad5..6586985ff5 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -378,7 +378,8 @@ module ActionController #:nodoc: end def xor_byte_strings(s1, s2) - s1.bytes.zip(s2.bytes).map { |(c1,c2)| c1 ^ c2 }.pack('c*') + s2_bytes = s2.bytes + s1.bytes.map.with_index { |c1, i| c1 ^ s2_bytes[i] }.pack('c*') end # The form's authenticity parameter. Override to provide your own. diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index ac45b2e363..0c4b661214 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -17,6 +17,7 @@ module ActionController # the database on the main thread, so they could open a txn, then the # controller thread will open a new connection and try to access data # that's only visible to the main thread's txn. This is the problem in #23483 + remove_method :new_controller_thread def new_controller_thread # :nodoc: yield end diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 87715205d9..8356d1a238 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -14,6 +14,7 @@ Mime::Type.register "image/jpeg", :jpeg, [], %w(jpg jpeg jpe pjpeg) Mime::Type.register "image/gif", :gif, [], %w(gif) Mime::Type.register "image/bmp", :bmp, [], %w(bmp) Mime::Type.register "image/tiff", :tiff, [], %w(tif tiff) +Mime::Type.register "image/svg+xml", :svg Mime::Type.register "video/mpeg", :mpeg, [], %w(mpg mpeg mpe) diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 6cde5b2900..79f9283f83 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -239,8 +239,7 @@ module ActionDispatch # # rails routes # - # Target specific controllers by prefixing the command with <tt>--controller</tt> option - # - or its <tt>-c</tt> shorthand. + # Target specific controllers by prefixing the command with <tt>-c</tt> option. # module Routing extend ActiveSupport::Autoload diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index b806ee015b..983f1daeb3 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -84,14 +84,15 @@ module ActionDispatch if filter.is_a?(Hash) && filter[:controller] { controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/ } elsif filter - { controller: /#{filter}/, action: /#{filter}/ } + { controller: /#{filter}/, action: /#{filter}/, verb: /#{filter}/, name: /#{filter}/, path: /#{filter}/ } end end def filter_routes(filter) if filter @routes.select do |route| - filter.any? { |default, value| route.defaults[default] =~ value } + route_wrapper = RouteWrapper.new(route) + filter.any? { |default, value| route_wrapper.send(default) =~ value } end else @routes diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 6f51accee7..f4534b4173 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -321,7 +321,9 @@ module ActionDispatch end # Performs the actual request. - def process(method, path, params: nil, headers: nil, env: nil, xhr: false) + def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: nil) + request_encoder = RequestEncoder.encoder(as) + if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme @@ -330,14 +332,17 @@ module ActionDispatch url_host += ":#{location.port}" if default != location.port host! url_host end - path = location.query ? "#{location.path}?#{location.query}" : location.path + path = request_encoder.append_format_to location.path + path = location.query ? "#{path}?#{location.query}" : path + else + path = request_encoder.append_format_to path end hostname, port = host.split(':') request_env = { :method => method, - :params => params, + :params => request_encoder.encode_params(params), "SERVER_NAME" => hostname, "SERVER_PORT" => port || (https? ? "443" : "80"), @@ -347,7 +352,7 @@ module ActionDispatch "REQUEST_URI" => path, "HTTP_HOST" => host, "REMOTE_ADDR" => remote_addr, - "CONTENT_TYPE" => "application/x-www-form-urlencoded", + "CONTENT_TYPE" => request_encoder.content_type, "HTTP_ACCEPT" => accept } @@ -376,6 +381,7 @@ module ActionDispatch response = _mock_session.last_response @response = ActionDispatch::TestResponse.from_response(response) @response.request = @request + @response.response_parser = RequestEncoder.parser(@response.content_type) @html_document = nil @url_options = nil @@ -387,6 +393,56 @@ module ActionDispatch def build_full_uri(path, env) "#{env['rack.url_scheme']}://#{env['SERVER_NAME']}:#{env['SERVER_PORT']}#{path}" end + + class RequestEncoder # :nodoc: + @encoders = {} + + attr_reader :response_parser + + def initialize(mime_name, param_encoder, response_parser, url_encoded_form = false) + @mime = Mime[mime_name] + + unless @mime + raise ArgumentError, "Can't register a request encoder for " \ + "unregistered MIME Type: #{mime_name}. See `Mime::Type.register`." + end + + @url_encoded_form = url_encoded_form + @path_format = ".#{@mime.symbol}" unless @url_encoded_form + @response_parser = response_parser || -> body { body } + @param_encoder = param_encoder || :"to_#{@mime.symbol}".to_proc + end + + def append_format_to(path) + path << @path_format unless @url_encoded_form + path + end + + def content_type + @mime.to_s + end + + def encode_params(params) + @param_encoder.call(params) + end + + def self.parser(content_type) + mime = Mime::Type.lookup(content_type) + encoder(mime ? mime.ref : nil).response_parser + end + + def self.encoder(name) + @encoders[name] || WWWFormEncoder + end + + def self.register_encoder(mime_name, param_encoder: nil, response_parser: nil) + @encoders[mime_name] = new(mime_name, param_encoder, response_parser) + end + + register_encoder :json, response_parser: -> body { JSON.parse(body) } + + WWWFormEncoder = new(:url_encoded_form, -> params { params }, nil, true) + end end module Runner @@ -643,6 +699,39 @@ module ActionDispatch # end # end # + # You can also test your JSON API easily by setting what the request should + # be encoded as: + # + # require 'test_helper' + # + # class ApiTest < ActionDispatch::IntegrationTest + # test 'creates articles' do + # assert_difference -> { Article.count } do + # post articles_path, params: { article: { title: 'Ahoy!' } }, as: :json + # end + # + # assert_response :success + # assert_equal({ id: Arcticle.last.id, title: 'Ahoy!' }, response.parsed_body) + # end + # end + # + # The `as` option sets the format to JSON, sets the content type to + # 'application/json' and encodes the parameters as JSON. + # + # Calling `parsed_body` on the response parses the response body as what + # the last request was encoded as. If the request wasn't encoded `as` something, + # it's the same as calling `body`. + # + # For any custom MIME Types you've registered, you can even add your own encoders with: + # + # ActionDispatch::IntegrationTest.register_encoder :wibble, + # param_encoder: -> params { params.to_wibble }, + # response_parser: -> body { body } + # + # Where `param_encoder` defines how the params should be encoded and + # `response_parser` defines how the response body should be parsed through + # `parsed_body`. + # # Consult the Rails Testing Guide for more. class IntegrationTest < ActiveSupport::TestCase @@ -671,5 +760,9 @@ module ActionDispatch def document_root_element html_document.root end + + def self.register_encoder(*args) + Integration::Session::RequestEncoder.register_encoder(*args) + end end end diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index 4b79a90242..9d4b73a43d 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -18,5 +18,11 @@ module ActionDispatch # Was there a server-side error? alias_method :error?, :server_error? + + attr_writer :response_parser # :nodoc: + + def parsed_body + @parsed_body ||= @response_parser.call(body) + end end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 74c78dfa8e..7556f984f2 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -390,6 +390,11 @@ class CollectionCacheController < ActionController::Base @customers = [Customer.new('david', 1)] render partial: 'customers/commented_customer', collection: @customers, as: :customer end + + def index_with_callable_cache_key + @customers = [Customer.new('david', 1)] + render @customers, cache: -> customer { 'cached_david' } + end end class AutomaticCollectionCacheTest < ActionController::TestCase @@ -405,6 +410,7 @@ class AutomaticCollectionCacheTest < ActionController::TestCase def test_collection_fetches_cached_views get :index assert_equal 1, @controller.partial_rendered_times + assert_customer_cached 'david/1', 'david, 1' get :index assert_equal 1, @controller.partial_rendered_times @@ -412,8 +418,11 @@ class AutomaticCollectionCacheTest < ActionController::TestCase def test_preserves_order_when_reading_from_cache_plus_rendering get :index, params: { id: 2 } - get :index_ordered + assert_equal 1, @controller.partial_rendered_times + assert_select ':root', 'david, 2' + get :index_ordered + assert_equal 3, @controller.partial_rendered_times assert_select ':root', "david, 1\n david, 2\n david, 3" end @@ -430,6 +439,18 @@ class AutomaticCollectionCacheTest < ActionController::TestCase get :index_with_comment assert_equal 1, @controller.partial_rendered_times end + + def test_caching_with_callable_cache_key + get :index_with_callable_cache_key + assert_customer_cached 'cached_david', 'david, 1' + assert_customer_cached 'david/1', 'david, 1' + end + + private + def assert_customer_cached(key, content) + assert_match content, + ActionView::PartialRenderer.collection_cache.read("views/#{key}/7c228ab609f0baf0b1f2367469210937") + end end class FragmentCacheKeyTestController < CachingController diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index d0a1d1285f..ea50f05f4d 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -1126,3 +1126,69 @@ class IntegrationRequestsWithSessionSetup < ActionDispatch::IntegrationTest assert_equal({"user_name"=>"david"}, cookies.to_hash) end end + +class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest + class FooController < ActionController::Base + def foos_json + render json: params.permit(:foo) + end + + def foos_wibble + render plain: 'ok' + end + end + + def test_encoding_as_json + post_to_foos as: :json do + assert_response :success + assert_match 'foos_json.json', request.path + assert_equal 'application/json', request.content_type + assert_equal({ 'foo' => 'fighters' }, request.request_parameters) + assert_equal({ 'foo' => 'fighters' }, response.parsed_body) + end + 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, + param_encoder: -> params { params }) + + post_to_foos as: :wibble do + assert_response :success + assert_match 'foos_wibble.wibble', request.path + assert_equal 'text/wibble', request.content_type + assert_equal Hash.new, request.request_parameters # Unregistered MIME Type can't be parsed. + assert_equal 'ok', response.parsed_body + end + ensure + Mime::Type.unregister :wibble + end + + def test_parsed_body_without_as_option + with_routing do |routes| + routes.draw { get ':action' => FooController } + + get '/foos_json.json', params: { foo: 'heyo' } + + assert_equal({ 'foo' => 'heyo' }, response.parsed_body) + end + end + + private + def post_to_foos(as:) + with_routing do |routes| + routes.draw { post ':action' => FooController } + + post "/foos_#{as}", params: { foo: 'fighters' }, as: as + + yield + end + end +end |