Make sure the query-params are not only coerced, but also validated

Before this change, the `query-params` value was coerced,
but never validated (which was a bug).

* `get-param-value`
 - add parameter: `validator` (optional)
 - refactor
 - document

* implement and use validators:
- `validate-date`
- `validate-string`
- `validate-noop`

* implement and use coercer:
- `coercer-noop`

*rename `coerce-report-type` to
 `coerce-and-validate-report-type`.

* implement and use anomalies:
- create `invalid-string-param-anom`
- create `invalid-date-param-anom`

* create `invalid-string-param-msg`
 (`invalid-date-param-msg` already existed)

* add regression test

Misc:
* document `handler`
* add regression test
* reformat with `cljfmt`

modules/operation-measure-evaluate-measure/src/blaze/fhir/operation/evaluate_measure/middleware/params.clj

@@ -13,29 +13,75 @@
   (format "Invalid parameter `%s` with value `%s`. Should be a date in format YYYY, YYYY-MM or YYYY-MM-DD."
           name value))
 
+(defn- invalid-string-param-msg [name value]
+  (format "Invalid parameter `%s` with value `%s`. Should be a string."
+          name value))
+
+(defn- invalid-date-param-anom
+  [name value]
+  (ba/incorrect
+   (invalid-date-param-msg name value)
+   :fhir/issue "value"
+   :fhir/operation-outcome "MSG_PARAM_INVALID"
+   :fhir.issue/expression name))
+
+(defn- invalid-string-param-anom
+  [name value]
+  (ba/incorrect
+   (invalid-string-param-msg name value)
+   :fhir/issue "value"
+   :fhir/operation-outcome "MSG_PARAM_INVALID"
+   :fhir.issue/expression name))
+
+(defn- validate-noop [_name value]
+  value)
+
+(defn- validate-date [name value]
+  (if (system/date? value) value (invalid-date-param-anom name value)))
+
+(defn- validate-string [name value]
+  (if (string? value) value (invalid-string-param-anom name value)))
+
 (defn- get-param-value-from-resource [body name]
   (when (identical? :fhir/Parameters (fhir-spec/fhir-type body))
     (some #(when (= name (:name %)) (:value %)) (:parameter body))))
 
-(defn- get-param-value [{:keys [params body]} name coercer]
+(defn- get-param-value* [{:keys [params body]} name coercer]
   (or (some->> (get params name) (coercer name))
       (get-param-value-from-resource body name)))
 
-(defn- get-required-param-value [request name coercer]
-  (or (get-param-value request name coercer)
+(defn- get-param-value
+  "Given a `request`, tries to get the value of the param with `name`.
+
+  There are two types of sources for the values:
+  * query-param values, and
+  * param values from the parameters resource.
+
+  Query-param values will need to be coerced to FHIR types, since they will
+  always be strings. They will be coerced with `coercer` (a function that takes
+  `name` and the found value, and returns the coerced value or an anomaly).
+
+  Param values don't need to be coerced, since they already are FHIR types.
+
+  Both types of values will then be validated by an optional `validator` (a
+  function that takes `name` and the coerced value, returning either the
+  validated value or an anomaly)."
+  ([request name coercer]
+   (get-param-value request name coercer validate-noop))
+  ([request name coercer validator]
+   (when-ok [value (get-param-value* request name coercer)]
+     (some->> value (validator name)))))
+
+(defn- get-required-param-value [request name coercer validator]
+  (or (get-param-value request name coercer validator)
       (ba/incorrect
        (format "Missing required parameter `%s`." name)
        :fhir/issue "value"
        :fhir/operation-outcome "MSG_PARAM_INVALID"
        :fhir.issue/expression name)))
 
-(defn- invalid-date-param-anom
-  [name value]
-  (ba/incorrect
-   (invalid-date-param-msg name value)
-   :fhir/issue "value"
-   :fhir/operation-outcome "MSG_PARAM_INVALID"
-   :fhir.issue/expression name))
+(defn- coerce-noop [_name value]
+  value)
 
 (defn- coerce-date
   "Coerces `value` into a System.Date.
@@ -51,7 +97,7 @@
   (format "Invalid parameter `reportType` with value `%s`. Should be one of `subject`, `subject-list` or `population`."
           report-type))
 
-(defn- coerce-report-type [_ value]
+(defn- coerce-and-validate-report-type [_ value]
   (if-not (s/valid? ::measure/report-type value)
     (ba/incorrect (invalid-report-type-param-msg value) :fhir/issue "value")
     (type/code value)))
@@ -78,13 +124,13 @@
 
 (defn- params-request [{:keys [request-method] :as request}]
   (when-ok [period-start (get-required-param-value
-                          request "periodStart" coerce-date)
+                          request "periodStart" coerce-date validate-date)
             period-end (get-required-param-value
-                        request "periodEnd" coerce-date)
+                        request "periodEnd" coerce-date validate-date)
             measure (get-param-value
-                     request "measure" (fn [_n v] v))
+                     request "measure" coerce-noop validate-string)
             report-type (get-param-value
-                         request "reportType" coerce-report-type)
+                         request "reportType" coerce-and-validate-report-type)
             subject-ref (coerce-subject-ref-param request)]
     (let [report-type (some-> report-type type/value)]
       (if (and (= :get request-method) (= "subject-list" report-type))

modules/operation-measure-evaluate-measure/test/blaze/fhir/operation/evaluate_measure/middleware/params_test.clj

@@ -17,7 +17,13 @@
 (def operation-outcome-uri
   #fhir/uri"http://terminology.hl7.org/CodeSystem/operation-outcome")
 
-(def handler (-> (params/wrap-coerce-params ac/completed-future) wrap-error))
+(def handler
+  "This testing handler will wrap the request into a future.
+
+   If an error arises, it returns a normal ring response map.
+   If no error arises, it returns the original request."
+  (-> (params/wrap-coerce-params ac/completed-future)
+      wrap-error))
 
 (deftest wrap-coerce-params-test
   (testing "period start"
@@ -114,28 +120,54 @@
           [:period 1] := #fhir/date"2021"))))
 
   (testing "measure"
-    (doseq [request
-            [{:params
-              {"periodStart" "2015"
-               "periodEnd" "2016"
-               "measure" "measure-202606"}}
-             {:body
-              {:fhir/type :fhir/Parameters
-               :parameter
-               [{:fhir/type :fhir.Parameters/parameter
-                 :name "periodStart"
-                 :value #fhir/date"2014"}
-                {:fhir/type :fhir.Parameters/parameter
-                 :name "periodEnd"
-                 :value #fhir/date"2015"}
-                {:fhir/type :fhir.Parameters/parameter
-                 :name "measure"
-                 :value #fhir/string"measure-202606"}]}}]]
-      (let [{:blaze.fhir.operation.evaluate-measure/keys [params]}
-            @(handler request)]
+    (testing "invalid (only POST)"
+      (let [request {:request-method :post
+                     :body
+                     {:fhir/type :fhir/Parameters
+                      :parameter
+                      [{:fhir/type :fhir.Parameters/parameter
+                        :name "periodStart"
+                        :value #fhir/date"2014"}
+                       {:fhir/type :fhir.Parameters/parameter
+                        :name "periodEnd"
+                        :value #fhir/date"2015"}
+                       {:fhir/type :fhir.Parameters/parameter
+                        :name "measure"
+                        :value #fhir/date"2015"}]}}
+            {:keys [status body]} @(handler request)]
 
-        (given params
-          :measure := "measure-202606"))))
+        (is (= 400 status))
+
+        (is (= :fhir/OperationOutcome (:fhir/type body)))
+
+        (given (-> body :issue first)
+          :severity := #fhir/code"error"
+          :code := #fhir/code"value"
+          :diagnostics := "Invalid parameter `measure` with value `2015`. Should be a string.")))
+
+    (testing "valid (both GET and POST)"
+      (doseq [request
+              [{:params
+                {"periodStart" "2015"
+                 "periodEnd" "2016"
+                 "measure" "measure-202606"}}
+               {:body
+                {:fhir/type :fhir/Parameters
+                 :parameter
+                 [{:fhir/type :fhir.Parameters/parameter
+                   :name "periodStart"
+                   :value #fhir/date"2014"}
+                  {:fhir/type :fhir.Parameters/parameter
+                   :name "periodEnd"
+                   :value #fhir/date"2015"}
+                  {:fhir/type :fhir.Parameters/parameter
+                   :name "measure"
+                   :value #fhir/string"measure-202606"}]}}]]
+        (let [{:blaze.fhir.operation.evaluate-measure/keys [params]}
+              @(handler request)]
+
+          (given params
+            :measure := "measure-202606")))))
 
   (testing "report type"
     (testing "invalid"