Fix Content Negotiation

This fixes a regression introduced by #703 where we don't return JSON by default if an Accept header is present. This is relevant in situations where browsers start with text/html but also will accept application/xml with lower priority. Instead of returning a 406 in this cases, XML is delivered now. If one prefers JSON in browsers the _format query param has to be used.
samply · May 10, 2022 · f81f387 · f81f387
1 parent a2b0755
commit f81f387
Show file tree

Hide file tree

Showing 2 changed files with 148 additions and 59 deletions.
diff --git a/modules/rest-api/src/blaze/rest_api/middleware/output.clj b/modules/rest-api/src/blaze/rest_api/middleware/output.clj
@@ -4,7 +4,6 @@
     [blaze.fhir.spec :as fhir-spec]
     [clojure.data.xml :as xml]
     [clojure.java.io :as io]
-    [clojure.string :as str]
     [muuntaja.parse :as parse]
     [prometheus.alpha :as prom]
     [ring.util.response :as ring]
@@ -42,38 +41,48 @@
     (generate-xml* body)))
 
 
-(defn- encode-response-json [response]
+(defn- encode-response-json [response content-type]
   (-> (update response :body generate-json)
-      (ring/content-type "application/fhir+json;charset=utf-8")))
+      (ring/content-type content-type)))
 
 
-(defn- encode-response-xml [response]
+(defn- encode-response-xml [response content-type]
   (-> (update response :body generate-xml)
-      (ring/content-type "application/fhir+xml;charset=utf-8")))
-
-
-(defn- json-format? [format]
-  (or (str/includes? format "json") (#{"*/*" "application/*" "text/*"} format)))
+      (ring/content-type content-type)))
 
 
 (defn- format-key [format]
-  (cond
-    (json-format? format) :json
-    (str/includes? format "xml") :xml))
+  (condp = format
+    "application/fhir+json" :fhir+json
+    "application/fhir+xml" :fhir+xml
+    "application/json" :json
+    "application/xml" :xml
+    "text/json" :text-json
+    "text/xml" :text-xml
+    "*/*" :fhir+json
+    "application/*" :fhir+json
+    "text/*" :text-json
+    "json" :fhir+json
+    "xml" :fhir+xml
+    nil))
 
 
 (defn- request-format
   [{{:strs [accept]} :headers {format "_format"} :query-params}]
   (or (some-> format format-key)
-      (if-let [first-accept (first (parse-accept accept))]
-        (format-key first-accept)
-        :json)))
+      (if-let [accept (parse-accept accept)]
+        (some format-key accept)
+        :fhir+json)))
 
 
 (defn- encode-response [opts request response]
   (case (request-format request)
-    :json (encode-response-json response)
-    :xml (encode-response-xml response)
+    :fhir+json (encode-response-json response "application/fhir+json;charset=utf-8")
+    :fhir+xml (encode-response-xml response "application/fhir+xml;charset=utf-8")
+    :json (encode-response-json response "application/json;charset=utf-8")
+    :xml (encode-response-xml response "application/xml;charset=utf-8")
+    :text-json (encode-response-json response "text/json;charset=utf-8")
+    :text-xml (encode-response-xml response "text/xml;charset=utf-8")
     (when (:accept-all? opts) (dissoc response :body))))
 
 

diff --git a/modules/rest-api/test/blaze/rest_api/middleware/output_test.clj b/modules/rest-api/test/blaze/rest_api/middleware/output_test.clj
@@ -1,16 +1,17 @@
 (ns blaze.rest-api.middleware.output-test
   (:require
+    [blaze.fhir.spec :as fhir-spec]
     [blaze.fhir.spec-spec]
     [blaze.rest-api.middleware.output :refer [wrap-output]]
     [blaze.test-util :as tu]
     [blaze.test-util.ring :refer [call]]
+    [clojure.data.xml :as xml]
+    [clojure.java.io :as io]
     [clojure.spec.test.alpha :as st]
     [clojure.test :as test :refer [are deftest is testing]]
     [juxt.iota :refer [given]]
     [ring.util.response :as ring]
-    [taoensso.timbre :as log])
-  (:import
-    [java.nio.charset StandardCharsets]))
+    [taoensso.timbre :as log]))
 
 
 (st/instrument)
@@ -34,74 +35,153 @@
       (respond (ring/response {:fhir/type :fhir/Patient :id "0"})))))
 
 
-(defn- bytes->str [^bytes bs]
-  (String. bs StandardCharsets/UTF_8))
+(defn- parse-json [body]
+  (fhir-spec/conform-json (fhir-spec/parse-json body)))
 
 
 (deftest json-test
   (testing "JSON is the default"
     (testing "without accept header"
       (given (call resource-handler {})
-        [:body bytes->str] := "{\"id\":\"0\",\"resourceType\":\"Patient\"}"))
+        [:headers "Content-Type"] := "application/fhir+json;charset=utf-8"
+        [:body parse-json] := {:fhir/type :fhir/Patient :id "0"}))
 
     (testing "with accept header"
-      (are [accept] (given (call resource-handler {:headers {"accept" accept}})
-        [:body bytes->str] := "{\"id\":\"0\",\"resourceType\":\"Patient\"}")
-        "*/*"
-        "application/*"
-        "text/*")))
+      (are [accept content-type]
+        (given (call resource-handler {:headers {"accept" accept}})
+          [:headers "Content-Type"] := content-type
+          [:body parse-json] := {:fhir/type :fhir/Patient :id "0"})
+        "*/*" "application/fhir+json;charset=utf-8"
+        "application/*" "application/fhir+json;charset=utf-8"
+        "text/*" "text/json;charset=utf-8")))
 
   (testing "possible accept headers"
-    (are [accept]
+    (are [accept content-type]
       (given (call resource-handler {:headers {"accept" accept}})
-        [:body bytes->str] := "{\"id\":\"0\",\"resourceType\":\"Patient\"}")
-      "application/fhir+json"
-      "application/json"
-      "text/json"
-      "application/fhir+xml;q=0.9, application/fhir+json;q=1.0"))
+        [:headers "Content-Type"] := content-type
+        [:body parse-json] := {:fhir/type :fhir/Patient :id "0"})
+      "application/fhir+json" "application/fhir+json;charset=utf-8"
+      "application/json" "application/json;charset=utf-8"
+      "text/json" "text/json;charset=utf-8"
+      "application/fhir+xml;q=0.9, application/fhir+json;q=1.0" "application/fhir+json;charset=utf-8"))
 
   (testing "_format overrides"
-    (are [accept format]
+    (are [accept format content-type]
       (given (call resource-handler
                    {:headers {"accept" accept}
                     :query-params {"_format" format}})
-        [:body bytes->str] := "{\"id\":\"0\",\"resourceType\":\"Patient\"}")
-      "application/fhir+xml" "application/fhir+json"
-      "application/fhir+xml" "application/json"
-      "application/fhir+xml" "text/json"
-      "application/fhir+xml" "json"
-      "*/*" "application/fhir+json"
-      "*/*" "application/json"
-      "*/*" "text/json"
-      "*/*" "json")))
+        [:headers "Content-Type"] := content-type
+        [:body parse-json] := {:fhir/type :fhir/Patient :id "0"})
+      "application/fhir+xml"
+      "application/fhir+json"
+      "application/fhir+json;charset=utf-8"
+
+      "application/fhir+xml"
+      "application/json"
+      "application/json;charset=utf-8"
+
+      "application/fhir+xml"
+      "text/json"
+      "text/json;charset=utf-8"
+
+      "application/fhir+xml"
+      "json"
+      "application/fhir+json;charset=utf-8"
+
+      "*/*"
+      "application/fhir+json"
+      "application/fhir+json;charset=utf-8"
+
+      "*/*"
+      "application/json"
+      "application/json;charset=utf-8"
+
+      "*/*"
+      "text/json"
+      "text/json;charset=utf-8"
+
+      "*/*"
+      "json"
+      "application/fhir+json;charset=utf-8")))
+
+
+(defn- parse-xml [body]
+  (with-open [reader (io/reader body)]
+    (fhir-spec/conform-xml (xml/parse reader))))
 
 
 (deftest xml-test
   (testing "possible accept headers"
-    (are [accept]
+    (are [accept content-type]
       (given (call resource-handler {:headers {"accept" accept}})
-        [:body bytes->str] :=
-        "<?xml version=\"1.0\" encoding=\"UTF-8\"?><Patient xmlns=\"http://hl7.org/fhir\"><id value=\"0\"/></Patient>")
+        [:headers "Content-Type"] := content-type
+        [:body parse-xml] := {:fhir/type :fhir/Patient :id "0"})
       "application/fhir+xml"
+      "application/fhir+xml;charset=utf-8"
+
       "application/xml"
+      "application/xml;charset=utf-8"
+
       "text/xml"
-      "application/fhir+json;q=0.9, application/fhir+xml;q=1.0"))
+      "text/xml;charset=utf-8"
+
+      "application/fhir+json;q=0.9, application/fhir+xml;q=1.0"
+      "application/fhir+xml;charset=utf-8"
+
+      ;; Safari
+      "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"
+      "application/xml;charset=utf-8"
+
+      ;; Chrome
+      "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9"
+      "application/xml;charset=utf-8"
+
+      ;; Edge
+      "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9"
+      "application/xml;charset=utf-8"
+
+      ;; Firefox
+      "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8"
+      "application/xml;charset=utf-8"))
 
   (testing "_format overrides"
-    (are [accept format]
+    (are [accept format content-type]
       (given (call resource-handler
                    {:headers {"accept" accept}
                     :query-params {"_format" format}})
-        [:body bytes->str] :=
-        "<?xml version=\"1.0\" encoding=\"UTF-8\"?><Patient xmlns=\"http://hl7.org/fhir\"><id value=\"0\"/></Patient>")
-      "application/fhir+json" "application/fhir+xml"
-      "application/fhir+json" "application/xml"
-      "application/fhir+json" "text/xml"
-      "application/fhir+json" "xml"
-      "*/*" "application/fhir+xml"
-      "*/*" "application/xml"
-      "*/*" "text/xml"
-      "*/*" "xml")))
+        [:headers "Content-Type"] := content-type
+        [:body parse-xml] := {:fhir/type :fhir/Patient :id "0"})
+      "application/fhir+json"
+      "application/fhir+xml"
+      "application/fhir+xml;charset=utf-8"
+
+      "application/fhir+json"
+      "application/xml"
+      "application/xml;charset=utf-8"
+
+      "application/fhir+json"
+      "text/xml"
+      "text/xml;charset=utf-8"
+
+      "application/fhir+json"
+      "xml"
+      "application/fhir+xml;charset=utf-8"
+
+      "*/*"
+      "application/fhir+xml"
+      "application/fhir+xml;charset=utf-8"
+
+      "*/*"
+      "application/xml"
+      "application/xml;charset=utf-8"
+
+      "*/*"
+      "text/xml"
+      "text/xml;charset=utf-8"
+
+      "*/*"
+      "xml"
+      "application/fhir+xml;charset=utf-8")))
 
 
 (deftest not-acceptable-test