Added Accept header unit tests

diff --git a/app/controllers/api/map_controller.rb b/app/controllers/api/map_controller.rb
index 4b233c99584e9924e86217e164ddca08a78c14e4..113554c72fb38ad590fcbfcf869ce8d7a20f4747 100644 (file)
--- 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 08f19008f730dc931e251b421ba2ca4dca134ca3..336aebc915793f05f4f7429214cb6ba74b169eb5 100644 (file)
--- 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 b68fde6b55bd5a72e9c101e6d11a5f385d48ea4e..f672e7c4960d9c66c1d0e94c9dd2eac272c53a0e 100644 (file)
--- 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 566efe0eba88b2f3e289f1cbcfd44d0de17b4b5f..329c5e29c7fa7adfc504da3ab7efcd719701adc7 100644 (file)
--- 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 95f36df532a9ac1e69688459dd0b8a8850690c5c..a7c876710ce4c8eafbbab30566d63f42a71a48ca 100644 (file)
--- 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 7599568af225e2d35e1f16cd3fa58e531353cb6d..c0534a6faef18b4b56abf0d33a6cf66d5423763f 100644 (file)
--- 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 54461868b7934eff2d22813dc93f20e7673c48fb..c5392c723e776b54f03cae162065035c615d0c0e 100644 (file)
--- 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 4d9372148f270cd447df619f1c438f2f4335b238..5b4d0d28f31b5c453cdc09ef4bb508caba347e66 100644 (file)
--- 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)