diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG.md | 4 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/head.rb | 14 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/cache.rb | 8 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/response.rb | 35 | ||||
| -rw-r--r-- | actionpack/test/controller/caching_test.rb | 1 | ||||
| -rw-r--r-- | actionpack/test/controller/log_subscriber_test.rb | 2 | ||||
| -rw-r--r-- | actionpack/test/controller/render_test.rb | 35 | ||||
| -rw-r--r-- | actionpack/test/controller/rescue_test.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/dispatch/response_test.rb | 2 |
9 files changed, 76 insertions, 29 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 05cbe472e0..cb5e7516fb 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Deprecate passing first parameter as `Hash` and default status code for `head` method. + + *Mehmet Emin İNAÇ* + * Adds`Rack::Utils::ParameterTypeError` and `Rack::Utils::InvalidParameterError` to the rescue_responses hash in `ExceptionWrapper` (Rack recommends integrators serve 400s for both of these). diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index 70f42bf565..f445094bdc 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -17,8 +17,18 @@ module ActionController # # See Rack::Utils::SYMBOL_TO_STATUS_CODE for a full list of valid +status+ symbols. def head(status, options = {}) - options, status = status, nil if status.is_a?(Hash) - status ||= options.delete(:status) || :ok + if status.is_a?(Hash) + msg = status[:status] ? 'The :status option' : 'The implicit :ok status' + options, status = status, status.delete(:status) + + ActiveSupport::Deprecation.warn(<<-MSG.squish) + #{msg} on `head` has been deprecated and will be removed in Rails 5.1. + Please pass the status as a separate parameter before the options, instead. + MSG + end + + status ||= :ok + location = options.delete(:location) content_type = options.delete(:content_type) diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 747d295261..cc1cb3f0f0 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -151,11 +151,11 @@ module ActionDispatch control.merge! @cache_control if control.empty? - headers[CACHE_CONTROL] = DEFAULT_CACHE_CONTROL + self[CACHE_CONTROL] = DEFAULT_CACHE_CONTROL elsif control[:no_cache] - headers[CACHE_CONTROL] = NO_CACHE + self[CACHE_CONTROL] = NO_CACHE if control[:extras] - headers[CACHE_CONTROL] += ", #{control[:extras].join(', ')}" + self[CACHE_CONTROL] += ", #{control[:extras].join(', ')}" end else extras = control[:extras] @@ -167,7 +167,7 @@ module ActionDispatch options << MUST_REVALIDATE if control[:must_revalidate] options.concat(extras) if extras - headers[CACHE_CONTROL] = options.join(", ") + self[CACHE_CONTROL] = options.join(", ") end end end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 9e53a0f08b..aae011fd6a 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -41,9 +41,8 @@ module ActionDispatch # :nodoc: attr_writer :sending_file # Get and set headers for this response. - attr_accessor :header + attr_reader :header - alias_method :headers=, :header= alias_method :headers, :header delegate :[], :[]=, :to => :@header @@ -61,7 +60,7 @@ module ActionDispatch # :nodoc: # The charset of the response. HTML wants to know the encoding of the # content you're giving them, so we need to send that along. - attr_accessor :charset + attr_reader :charset CONTENT_TYPE = "Content-Type".freeze SET_COOKIE = "Set-Cookie".freeze @@ -117,8 +116,9 @@ module ActionDispatch # :nodoc: super() header = merge_default_headers(header, default_headers) + @header = header - self.body, self.header, self.status = body, header, status + self.body, self.status = body, status @sending_file = false @blank = false @@ -127,7 +127,7 @@ module ActionDispatch # :nodoc: @sending = false @sent = false @content_type = nil - @charset = nil + @charset = self.class.default_charset if content_type = self[CONTENT_TYPE] type, charset = content_type.split(/;\s*charset=/) @@ -187,6 +187,15 @@ module ActionDispatch # :nodoc: @content_type = content_type.to_s end + # Sets the HTTP character set. + def charset=(charset) + if nil == charset + @charset = self.class.default_charset + else + @charset = charset + end + end + # The response code of the request. def response_code @status @@ -278,6 +287,7 @@ module ActionDispatch # :nodoc: # # status, headers, body = *response def to_a + commit! rack_response @status, @header.to_hash end alias prepare! to_a @@ -302,6 +312,9 @@ module ActionDispatch # :nodoc: private def before_committed + return if committed? + assign_default_content_type_and_charset! + handle_conditional_get! end def before_sending @@ -319,16 +332,15 @@ module ActionDispatch # :nodoc: body.respond_to?(:each) ? body : [body] end - def assign_default_content_type_and_charset!(headers) - return if headers[CONTENT_TYPE].present? + def assign_default_content_type_and_charset! + return if self[CONTENT_TYPE].present? @content_type ||= Mime::HTML - @charset ||= self.class.default_charset unless @charset == false type = @content_type.to_s.dup - type << "; charset=#{@charset}" if append_charset? + type << "; charset=#{charset}" if append_charset? - headers[CONTENT_TYPE] = type + self[CONTENT_TYPE] = type end def append_charset? @@ -372,9 +384,6 @@ module ActionDispatch # :nodoc: end def rack_response(status, header) - assign_default_content_type_and_charset!(header) - handle_conditional_get! - header[SET_COOKIE] = header[SET_COOKIE].join("\n") if header[SET_COOKIE].respond_to?(:join) if NO_CONTENT_CODES.include?(@status) diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index de697858c3..f21dd87400 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -380,6 +380,7 @@ class AutomaticCollectionCacheTest < ActionController::TestCase @controller = CollectionCacheController.new @controller.perform_caching = true @controller.partial_rendered_times = 0 + @controller.cache_store = ActiveSupport::Cache::MemoryStore.new end def test_collection_fetches_cached_views diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index ccbf336acf..7835d2768a 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -10,7 +10,7 @@ module Another end rescue_from SpecialException do - head :status => 406 + head 406 end before_action :redirector, only: :never_executed diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index c9c43de37d..43e992d432 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -138,6 +138,14 @@ class TestController < ActionController::Base fresh_when(:last_modified => Time.now.utc.beginning_of_day, :etag => [ :foo, 123 ]) end + def head_with_status_hash + head status: :created + end + + def head_with_hash_does_not_include_status + head warning: :deprecated + end + def head_created head :created end @@ -151,31 +159,31 @@ class TestController < ActionController::Base end def head_with_location_header - head :location => "/foo" + head :ok, :location => "/foo" end def head_with_location_object - head :location => Customer.new("david", 1) + head :ok, :location => Customer.new("david", 1) end def head_with_symbolic_status - head :status => params[:status].intern + head params[:status].intern end def head_with_integer_status - head :status => params[:status].to_i + head params[:status].to_i end def head_with_string_status - head :status => params[:status] + head params[:status] end def head_with_custom_header - head :x_custom_header => "something" + head :ok, :x_custom_header => "something" end def head_with_www_authenticate_header - head 'WWW-Authenticate' => 'something' + head :ok, 'WWW-Authenticate' => 'something' end def head_with_status_code_first @@ -490,6 +498,19 @@ class HeadRenderTest < ActionController::TestCase assert_response :created end + def test_passing_hash_to_head_as_first_parameter_deprecated + assert_deprecated do + get :head_with_status_hash + end + end + + def test_head_with_default_value_is_deprecated + assert_deprecated do + get :head_with_hash_does_not_include_status + assert_response :ok + end + end + def test_head_created_with_application_json_content_type post :head_created_with_application_json_content_type assert @response.body.blank? diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 4898b0c57f..2cfd0b8023 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -47,10 +47,10 @@ class RescueController < ActionController::Base rescue_from 'InvalidRequestToRescueAsString', :with => proc { |exception| render :text => exception.message } rescue_from BadGateway do - head :status => 502 + head 502 end rescue_from 'BadGatewayToRescueAsString' do - head :status => 502 + head 502 end rescue_from ResourceUnavailable do |exception| diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 5fbd19acdf..7aca251066 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -72,12 +72,14 @@ class ResponseTest < ActiveSupport::TestCase test "content type" do [204, 304].each do |c| + @response = ActionDispatch::Response.new @response.status = c.to_s _, headers, _ = @response.to_a assert !headers.has_key?("Content-Type"), "#{c} should not have Content-Type header" end [200, 302, 404, 500].each do |c| + @response = ActionDispatch::Response.new @response.status = c.to_s _, headers, _ = @response.to_a assert headers.has_key?("Content-Type"), "#{c} did not have Content-Type header" |