Skip to content

Commit

Permalink
Fixes after review (WIP)
Browse files Browse the repository at this point in the history
  • Loading branch information
@allentiak
allentiak committed Feb 5, 2025
1 parent 78c8bf4 commit 1ffb7bb
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 54 deletions.
32 changes: 15 additions & 17 deletions modules/rest-util/src/blaze/middleware/fhir/resource.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,23 +99,21 @@
(assoc request :body resource))
(ba/incorrect "Missing HTTP body.")))

(def ^:private ^Base64$Encoder b64-encoder
(.withoutPadding (Base64/getUrlEncoder)))

(defn- get-binary-data [body]
(with-open [_ (prom/timer parse-duration-seconds "binary")
^InputStream input body]
(.encodeToString b64-encoder (.readAllBytes input))))
(with-open [_ (prom/timer parse-duration-seconds "binary")]
(.encodeToString ^Base64$Encoder (Base64/getEncoder) (.readAllBytes ^InputStream body))))

(defn- resource-request-binary-data [{:keys [body headers] :as request}]
(if body
(when-ok [b64-encoded-data (get-binary-data body)]
(let [content-type (get headers "content-type")]
(assoc request :body
{:fhir/type :fhir/Binary
:resourceType "Binary"
:contentType (type/code content-type)
:data (type/base64Binary b64-encoded-data)})))
;; `when-ok` is not needed here because:
;; * Binary data is not parsed nor validated, and
;; * Base64-encoding should not fail under normal circumstances.
(let [b64-encoded-data (get-binary-data body)
content-type (get headers "content-type")]
(assoc request :body
{:fhir/type :fhir/Binary
:contentType (type/code content-type)
:data (type/base64Binary b64-encoded-data)}))
(ba/incorrect "Missing HTTP body.")))

(defn- unsupported-media-type-msg [media-type]
Expand All @@ -132,16 +130,16 @@
:http/status 415))
(if (str/blank? (slurp (:body request)))
(assoc request :body nil)
(ba/incorrect "Expected Content-Type header for FHIR resources."))))
(ba/incorrect "Missing Content-Type header for FHIR resources."))))

(defn- binary-resource-request [request]
(if-let [content-type (request/content-type request)]
(cond
(json-request? content-type) (resource-request-json request)
(xml-request? content-type) (resource-request-xml request)
(str/starts-with? content-type "application/fhir+json") (resource-request-json request)
(str/starts-with? content-type "application/fhir+xml") (resource-request-xml request)
:else
(resource-request-binary-data request))
(ba/incorrect "Expected Content-Type header for binary resources.")))
(ba/incorrect "Missing Content-Type header for binary resources.")))

(defn wrap-resource
"Middleware to parse a resource from the body according the content-type
Expand Down
92 changes: 55 additions & 37 deletions modules/rest-util/test/blaze/middleware/fhir/resource_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
(:import
[java.io ByteArrayInputStream]
[java.nio.charset StandardCharsets]
[java.util Base64$Encoder]
[java.util Base64]))

(set! *warn-on-reflection* true)
Expand All @@ -31,43 +32,59 @@
(ac/exceptionally handler-util/error-response))))

(def ^:private resource-handler
"A handler which just returns the `:body` from a non-binary resource request."
"A handler which just returns the :body from a non-binary resource request."
(-> (comp ac/completed-future :body)
wrap-resource
wrap-error))

(def ^:private binary-resource-handler
"A handler which just returns the `:body` from a binary resource request."
"A handler which just returns the :body from a binary resource request."
(-> (comp ac/completed-future :body)
wrap-binary-data
wrap-error))

(defn- input-stream
(defn- string-input-stream
([^String s]
(ByteArrayInputStream. (.getBytes s StandardCharsets/UTF_8)))
([^String s closed?]
(proxy [ByteArrayInputStream] [(.getBytes s StandardCharsets/UTF_8)]
(close []
(reset! closed? true)))))

(defn- encode-binary-data [^String data]
(.encodeToString (.withoutPadding (Base64/getUrlEncoder))
(.getBytes data StandardCharsets/UTF_8)))
(defn- binary-input-stream
([^bytes data]
(ByteArrayInputStream. data))
([^bytes data closed?]
(proxy [ByteArrayInputStream] [data]
(close []
(reset! closed? true)))))

(defn- encode-binary-data [^bytes data]
(.encodeToString ^Base64$Encoder (Base64/getEncoder) data))

(defn- bytes-of-random-binary-data [lenght]
(byte-array (repeatedly lenght #(rand-int 256))))

(defn- resource-as-json [b64-encoded-binary-data content-type]
(str "{\"data\" : \"" b64-encoded-binary-data "\", \"resourceType\" : \"Binary\", \"contentType\" : \"" content-type "\"}"))

(defn- resource-as-xml [b64-encoded-binary-data content-type]
(str "<Binary xmlns=\"http://hl7.org/fhir\"><data value=\"" b64-encoded-binary-data "\"/><contentType value=\"" content-type "\"/></Binary>"))

(deftest json-test
(testing "possible content types"
(doseq [content-type ["application/fhir+json" "text/json" "application/json"]]
(let [closed? (atom false)]
(given @(resource-handler
{:headers {"content-type" content-type}
:body (input-stream "{\"resourceType\": \"Patient\"}" closed?)})
:body (string-input-stream "{\"resourceType\": \"Patient\"}" closed?)})
fhir-spec/fhir-type := :fhir/Patient)
(is (true? @closed?)))))

(testing "empty body"
(given @(resource-handler
{:headers {"content-type" "application/fhir+json"}
:body (input-stream "")})
:body (string-input-stream "")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -77,7 +94,7 @@
(testing "body with invalid JSON"
(given @(resource-handler
{:headers {"content-type" "application/fhir+json"}
:body (input-stream "x")})
:body (string-input-stream "x")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -88,7 +105,7 @@
;; There is no XML analogy to this JSON test, since XML has no objects.
(given @(resource-handler
{:headers {"content-type" "application/fhir+json"}
:body (input-stream "1")})
:body (string-input-stream "1")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -100,7 +117,7 @@
(testing "body with invalid resource"
(given @(resource-handler
{:headers {"content-type" "application/fhir+json"}
:body (input-stream "{\"resourceType\": \"Patient\", \"gender\": {}}")})
:body (string-input-stream "{\"resourceType\": \"Patient\", \"gender\": {}}")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -111,7 +128,7 @@
(testing "body with bundle with null resource"
(given @(resource-handler
{:headers {"content-type" "application/fhir+json"}
:body (input-stream "{\"resourceType\": \"Bundle\", \"entry\": [{\"resource\": null}]}")})
:body (string-input-stream "{\"resourceType\": \"Bundle\", \"entry\": [{\"resource\": null}]}")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -122,7 +139,7 @@
(testing "body with bundle with invalid resource"
(given @(resource-handler
{:headers {"content-type" "application/fhir+json"}
:body (input-stream "{\"resourceType\": \"Bundle\", \"entry\": [{\"resource\": {\"resourceType\": \"Patient\", \"gender\": {}}}]}")})
:body (string-input-stream "{\"resourceType\": \"Bundle\", \"entry\": [{\"resource\": {\"resourceType\": \"Patient\", \"gender\": {}}}]}")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -133,7 +150,7 @@
(testing "long attribute values are allowed (JSON-wrapped Binary data)"
(given @(resource-handler
{:headers {"content-type" "application/fhir+json"}
:body (input-stream (str "{\"data\" : \"" (apply str (repeat (* 8 1024 1024) \a)) "\", \"resourceType\" : \"Binary\"}"))})
:body (string-input-stream (str "{\"data\" : \"" (apply str (repeat (* 8 1024 1024) \a)) "\", \"resourceType\" : \"Binary\"}"))})
fhir-spec/fhir-type := :fhir/Binary)))

(deftest xml-test
Expand All @@ -142,14 +159,14 @@
(let [closed? (atom false)]
(given @(resource-handler
{:headers {"content-type" content-type}
:body (input-stream "<Patient xmlns=\"http://hl7.org/fhir\"></Patient>" closed?)})
:body (string-input-stream "<Patient xmlns=\"http://hl7.org/fhir\"></Patient>" closed?)})
fhir-spec/fhir-type := :fhir/Patient)
(is (true? @closed?)))))

(testing "empty body"
(given @(resource-handler
{:headers {"content-type" "application/fhir+xml"}
:body (input-stream "")})
:body (string-input-stream "")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -162,7 +179,7 @@
(given @(resource-handler
{:request-method :post
:headers {"content-type" "application/fhir+xml"}
:body (input-stream input-string)})
:body (string-input-stream input-string)})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -172,7 +189,7 @@
(testing "body with invalid resource"
(given @(resource-handler
{:headers {"content-type" "application/fhir+xml"}
:body (input-stream "<Patient xmlns=\"http://hl7.org/fhir\"><id value=\"a_b\"/></Patient>")})
:body (string-input-stream "<Patient xmlns=\"http://hl7.org/fhir\"><id value=\"a_b\"/></Patient>")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -182,7 +199,7 @@
(testing "body with bundle with empty resource"
(given @(resource-handler
{:headers {"content-type" "application/fhir+xml"}
:body (input-stream "<Bundle xmlns=\"http://hl7.org/fhir\"><entry><resource></resource></entry></Bundle>")})
:body (string-input-stream "<Bundle xmlns=\"http://hl7.org/fhir\"><entry><resource></resource></entry></Bundle>")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -192,7 +209,7 @@
(testing "body with bundle with invalid resource"
(given @(resource-handler
{:headers {"content-type" "application/fhir+xml"}
:body (input-stream "<Bundle xmlns=\"http://hl7.org/fhir\"><entry><resource><Patient xmlns=\"http://hl7.org/fhir\"><id value=\"a_b\"/></Patient></resource></entry></Bundle>")})
:body (string-input-stream "<Bundle xmlns=\"http://hl7.org/fhir\"><entry><resource><Patient xmlns=\"http://hl7.org/fhir\"><id value=\"a_b\"/></Patient></resource></entry></Bundle>")})
:status := 400
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand All @@ -202,42 +219,43 @@
(testing "long attribute values are allowed (XML-wrapped Binary data)"
(given @(resource-handler
{:headers {"content-type" "application/fhir+xml"}
:body (input-stream (str "<Binary xmlns=\"http://hl7.org/fhir\"><data value=\"" (apply str (repeat (* 8 1024 1024) \a)) "\"/></Binary>"))})
:body (string-input-stream (str "<Binary xmlns=\"http://hl7.org/fhir\"><data value=\"" (apply str (repeat (* 8 1024 1024) \a)) "\"/></Binary>"))})
fhir-spec/fhir-type := :fhir/Binary)))

(deftest binary-test
(testing "when sending FHIR-wrapped Binary data"
(testing "both handlers should return the same for both wrappers (JSON and XML)"
(let [b64-encoded-binary-data "MTA1NjE0Cg==" ;; raw data: 105614
(let [b64-encoded-binary-data "MTA1NjE0Cg=="
binary-resource-data (type/base64Binary b64-encoded-binary-data)
binary-resource-content-type (type/code "text/plain")]
binary-resource-content-type "text/plain"]
(doseq [handler [resource-handler binary-resource-handler]
[fhir-content-type resource-string-representation]
[["application/fhir+json;charset=utf-8" (str "{\"data\" : \"" b64-encoded-binary-data "\", \"resourceType\" : \"Binary\", \"contentType\" : \"" binary-resource-content-type "\"}")]
["application/fhir+xml;charset=utf-8" (str "<Binary xmlns=\"http://hl7.org/fhir\"><data value=\"" b64-encoded-binary-data "\"/><contentType value=\"" binary-resource-content-type "\"/></Binary>")]]]
[["application/fhir+json;charset=utf-8" (resource-as-json b64-encoded-binary-data binary-resource-content-type)]
["application/fhir+xml;charset=utf-8" (resource-as-xml b64-encoded-binary-data binary-resource-content-type)]]]
(let [closed? (atom false)]
(given @(handler
{:headers {"content-type" fhir-content-type}
:body (input-stream resource-string-representation closed?)})
:body (string-input-stream resource-string-representation closed?)})
{:fhir/type :fhir/Binary
:contentType binary-resource-content-type
:contentType (type/code binary-resource-content-type)
:data binary-resource-data})
(is closed?))))))

(testing "when sending raw binary resource handling"
(let [small-data "105614"
large-data (apply str (repeat (* 8 1024 1024) \a))]
(doseq [[content-type raw-data b64-encoded-data]
[["text/plain" small-data "MTA1NjE0Cg=="]
["application/octet-stream" large-data (encode-binary-data large-data)]]]
(let [closed? (atom false)]
(let [small-data (bytes-of-random-binary-data 16)
large-data (bytes-of-random-binary-data (* 8 1024 1024))]
(doseq [[content-type raw-data]
[["text/plain" small-data]
["application/octet-stream" large-data]]]
(let [closed? (atom false)
encoded-data (encode-binary-data raw-data)]
(given @(binary-resource-handler
{:headers {"content-type" content-type}
:body (input-stream raw-data closed?)})
:body (binary-input-stream raw-data closed?)})
{:fhir/type :fhir/Binary
:resourceType "Binary"
:contentType content-type
:data (type/base64Binary b64-encoded-data)}
:data (type/base64Binary encoded-data)}
(is @closed?)))))))

(def ^:private whitespace
Expand All @@ -247,11 +265,11 @@
(testing "blank body without content type header results in a nil body"
(satisfies-prop 10
(prop/for-all [s whitespace]
(nil? @(resource-handler {:body (input-stream s)})))))
(nil? @(resource-handler {:body (string-input-stream s)})))))

(testing "other content is invalid"
(testing "with unknown content-type header"
(given @(resource-handler {:headers {"content-type" "text/plain"} :body (input-stream "foo")})
(given @(resource-handler {:headers {"content-type" "text/plain"} :body (string-input-stream "foo")})
:status := 415
[:body fhir-spec/fhir-type] := :fhir/OperationOutcome
[:body :issue 0 :severity] := #fhir/code"error"
Expand Down

0 comments on commit 1ffb7bb

Please sign in to comment.