From: mmd-osm Date: Thu, 9 Jan 2020 20:09:33 +0000 (+0100) Subject: Added Accept header unit tests X-Git-Tag: live~2762^2~3 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/03ca0b2c697052c80b25f2111ab6ab718478a95a Added Accept header unit tests --- diff --git a/app/controllers/api/map_controller.rb b/app/controllers/api/map_controller.rb index 4b233c995..113554c72 100644 --- a/app/controllers/api/map_controller.rb +++ b/app/controllers/api/map_controller.rb @@ -5,7 +5,7 @@ module Api before_action :check_api_readable around_action :api_call_handle_error, :api_call_timeout - before_action :default_format_xml + before_action :set_default_request_format # This is probably the most common call of all. It is used for getting the # OSM data for a specified bounding box, usually for editing. First the diff --git a/app/controllers/api/nodes_controller.rb b/app/controllers/api/nodes_controller.rb index 08f19008f..336aebc91 100644 --- a/app/controllers/api/nodes_controller.rb +++ b/app/controllers/api/nodes_controller.rb @@ -13,7 +13,7 @@ module Api before_action :check_api_readable, :except => [:create, :update, :delete] around_action :api_call_handle_error, :api_call_timeout - before_action :default_format_xml + before_action :set_default_request_format, :except => [:create, :update, :delete] # Create a node from XML. def create diff --git a/app/controllers/api/old_controller.rb b/app/controllers/api/old_controller.rb index b68fde6b5..f672e7c49 100644 --- a/app/controllers/api/old_controller.rb +++ b/app/controllers/api/old_controller.rb @@ -16,7 +16,7 @@ module Api before_action :lookup_old_element, :except => [:history] before_action :lookup_old_element_versions, :only => [:history] - before_action :default_format_xml + before_action :set_default_request_format, :except => [:redact] def history # the .where() method used in the lookup_old_element_versions diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb index 566efe0eb..329c5e29c 100644 --- a/app/controllers/api/relations_controller.rb +++ b/app/controllers/api/relations_controller.rb @@ -11,7 +11,7 @@ module Api before_action :check_api_readable, :except => [:create, :update, :delete] around_action :api_call_handle_error, :api_call_timeout - before_action :default_format_xml + before_action :set_default_request_format, :except => [:create, :update, :delete] def create assert_method :put diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb index 95f36df53..a7c876710 100644 --- a/app/controllers/api/ways_controller.rb +++ b/app/controllers/api/ways_controller.rb @@ -11,7 +11,7 @@ module Api before_action :check_api_readable, :except => [:create, :update, :delete] around_action :api_call_handle_error, :api_call_timeout - before_action :default_format_xml + before_action :set_default_request_format, :except => [:create, :update, :delete] def create assert_method :put diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 7599568af..c0534a6fa 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -3,10 +3,49 @@ class ApiController < ApplicationController private - # Set format to xml unless client requires a specific format - def default_format_xml + ## + # Set default request format to xml unless a client requests a specific format, + # which can be done via (a) URL suffix and/or (b) HTTP Accept header, where + # the URL suffix always takes precedence over the Accept header. + def set_default_request_format unless params[:format] - request.format = "xml" unless request.format.symbol == :json + accept_header = request.headers["HTTP_ACCEPT"] + if accept_header.nil? + # e.g. unit tests don't set an Accept: header by default, force XML in this case + request.format = "xml" + return + end + + req_mimetypes = [] + + # Some clients (JOSM) send Accept headers which cannot be parsed by Rails, example: *; q=.2 + # To be fair, JOSM's Accept header doesn't adhere to RFC 7231, section 5.3.1, et al. either + # As a workaround for backwards compatibility, we're assuming XML format + begin + req_mimetypes = Mime::Type.parse(accept_header) + rescue Mime::Type::InvalidMimeType + request.format = "xml" + return + end + + # req_mimetypes contains all Accept header MIME types with descending priority + req_mimetypes.each do |mime| + if mime.symbol == :xml + request.format = "xml" + break + end + + if mime.symbol == :json + request.format = "json" + break + end + + # Any format, not explicitly requesting XML or JSON -> assume XML as default + if mime == "*/*" + request.format = "xml" + break + end + end end end diff --git a/test/controllers/api/map_controller_test.rb b/test/controllers/api/map_controller_test.rb index 54461868b..c5392c723 100644 --- a/test/controllers/api/map_controller_test.rb +++ b/test/controllers/api/map_controller_test.rb @@ -29,6 +29,78 @@ module Api ) end + ## + # test http accept headers + def test_http_accept_header + node = create(:node, :lat => 7, :lon => 7) + + minlon = node.lon - 0.1 + minlat = node.lat - 0.1 + maxlon = node.lon + 0.1 + maxlat = node.lat + 0.1 + bbox = "#{minlon},#{minlat},#{maxlon},#{maxlat}" + + # Accept: XML format -> use XML + http_accept_format("text/xml") + get :index, :params => { :bbox => bbox } + assert_response :success, "Expected success with the map call" + assert_equal "application/xml; charset=utf-8", @response.header["Content-Type"] + + # Accept: Any format -> use XML + http_accept_format("*/*") + get :index, :params => { :bbox => bbox } + assert_response :success, "Expected success with the map call" + assert_equal "application/xml; charset=utf-8", @response.header["Content-Type"] + + # Accept: Any format, .json URL suffix -> use json + http_accept_format("*/*") + get :index, :params => { :bbox => bbox, :format => "json" } + assert_response :success, "Expected success with the map call" + assert_equal "application/json; charset=utf-8", @response.header["Content-Type"] + + # Accept: Firefox header -> use XML + http_accept_format("text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8") + get :index, :params => { :bbox => bbox } + assert_response :success, "Expected success with the map call" + assert_equal "application/xml; charset=utf-8", @response.header["Content-Type"] + + # Accept: JOSM header text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2 -> use XML + # Note: JOSM's header does not comply with RFC 7231, section 5.3.1 + http_accept_format("text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2") + get :index, :params => { :bbox => bbox } + assert_response :success, "Expected success with the map call" + assert_equal "application/xml; charset=utf-8", @response.header["Content-Type"] + + # Accept: text/plain, */* -> use XML + http_accept_format("text/plain, */*") + get :index, :params => { :bbox => bbox } + assert_response :success, "Expected success with the map call" + assert_equal "application/xml; charset=utf-8", @response.header["Content-Type"] + + # Accept: text/* -> use XML + http_accept_format("text/*") + get :index, :params => { :bbox => bbox } + assert_response :success, "Expected success with the map call" + assert_equal "application/xml; charset=utf-8", @response.header["Content-Type"] + + # Accept: json, */* format -> use json + http_accept_format("application/json, */*") + get :index, :params => { :bbox => bbox } + assert_response :success, "Expected success with the map call" + assert_equal "application/json; charset=utf-8", @response.header["Content-Type"] + + # Accept: json format -> use json + http_accept_format("application/json") + get :index, :params => { :bbox => bbox } + assert_response :success, "Expected success with the map call" + assert_equal "application/json; charset=utf-8", @response.header["Content-Type"] + + # text/json is in invalid format, ActionController::UnknownFormat error is expected + http_accept_format("text/json") + get :index, :params => { :bbox => bbox } + assert_response :internal_server_error, "text/json should fail" + end + # ------------------------------------- # Test reading a bounding box. # ------------------------------------- diff --git a/test/test_helper.rb b/test/test_helper.rb index 4d9372148..5b4d0d28f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -115,6 +115,12 @@ module ActiveSupport @request.env["HTTP_AUTHORIZATION"] = format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) end + ## + # set request header for HTTP Accept + def http_accept_format(format) + @request.env["HTTP_ACCEPT"] = format + end + ## # set request readers to ask for a particular error format def error_format(format)