Improve Parameter Coercion in Terminology Service

modules/fhir-structure/src/blaze/fhir/util.clj

@@ -150,6 +150,53 @@
     (str (subs s 0 (dec (count s))) "ies")
     (str s "s")))
 
+(defn coerce-integer
+  "Returns the int value of the integer `x`.
+
+  Returns an anomaly if `x` isn't an integer or has no value."
+  [x]
+  (if (type/integer? x)
+    (if-some [value (:value x)]
+      value
+      (ba/incorrect "Missing value."))
+    (ba/incorrect "Has to be an integer.")))
+
+(defn coerce-boolean
+  "Returns the boolean value of the boolean `x`.
+
+  Returns an anomaly if `x` isn't a boolean or has no value."
+  [x]
+  (if (type/boolean? x)
+    (if-some [value (:value x)]
+      value
+      (ba/incorrect "Missing value."))
+    (ba/incorrect "Has to be a boolean.")))
+
+(defn coerce-string
+  "Returns the string value of the string `x`.
+
+  Returns an anomaly if `x` isn't a string or has no value."
+  [x]
+  (if (type/string? x)
+    (if-some [value (:value x)]
+      value
+      (ba/incorrect "Missing value."))
+    (ba/incorrect "Has to be a string.")))
+
+(defn coerce-uri
+  "Returns the string value of `x`.
+
+  Accepts any FHIR type with a string-valued value, not just uri, for
+  robustness reasons.
+
+  Returns an anomaly if `x` doesn't have a string value."
+  [x]
+  (if-some [value (:value x)]
+    (if (string? value)
+      value
+      (ba/incorrect "Has to be a uri."))
+    (ba/incorrect "Missing value.")))
+
 (defn- assoc-via [params {:keys [cardinality]} name value]
   (if (identical? :many cardinality)
     (update params (keyword (plural (camel->kebab name))) (fnil into []) (if (sequential? value) value [value]))
@@ -226,37 +273,3 @@
   (when-ok [new-params (coerce-params* specs params)
             _ (check-required-params specs params)]
     new-params))
-
-(defn coerce-boolean [name value]
-  (if-some [value (parse-boolean value)]
-    (type/boolean value)
-    (ba/incorrect (format "Invalid value for parameter `%s`. Has to be a boolean." name))))
-
-(defn coerce-integer [name value]
-  (if-let [value (parse-long value)]
-    (type/integer value)
-    (ba/incorrect (format "Invalid value for parameter `%s`. Has to be an integer." name))))
-
-(defn- validate-query-params* [parameter-specs params]
-  (reduce-kv
-   (fn [new-params name value]
-     (if-let [{:keys [action coerce]} (parameter-specs name)]
-       (case action
-         :copy
-         (if-ok [value (coerce name value)]
-           (conj new-params (parameter name value))
-           reduced)
-
-         :complex
-         (reduced (ba/unsupported (format "Unsupported parameter `%s` in GET request. Please use POST." name)
-                                  :http/status 400))
-
-         (reduced (ba/unsupported (format "Unsupported parameter `%s`." name)
-                                  :http/status 400)))
-       new-params))
-   []
-   params))
-
-(defn validate-query-params [parameter-specs query-params]
-  (when-ok [params (validate-query-params* parameter-specs query-params)]
-    {:fhir/type :fhir/Parameters :parameter params}))

modules/fhir-structure/src/blaze/fhir/util_spec.clj

@@ -2,7 +2,8 @@
   (:require
    [blaze.fhir.spec.spec]
    [blaze.fhir.util :as fu]
-   [clojure.spec.alpha :as s]))
+   [clojure.spec.alpha :as s]
+   [cognitect.anomalies :as anom]))
 
 (s/fdef fu/subsetted?
   :args (s/cat :coding map?)
@@ -22,4 +23,20 @@
 
 (s/fdef fu/coerce-params
   :args (s/cat :specs map? :parameters :fhir/Parameters)
-  :ret map?)
+  :ret (s/or :params (s/map-of simple-keyword? any?) :anomaly ::anom/anomaly))
+
+(s/fdef fu/coerce-integer
+  :args (s/cat :x any?)
+  :ret (s/or :value int? :anomaly ::anom/anomaly))
+
+(s/fdef fu/coerce-boolean
+  :args (s/cat :x any?)
+  :ret (s/or :value boolean? :anomaly ::anom/anomaly))
+
+(s/fdef fu/coerce-string
+  :args (s/cat :x any?)
+  :ret (s/or :value string? :anomaly ::anom/anomaly))
+
+(s/fdef fu/coerce-uri
+  :args (s/cat :x any?)
+  :ret (s/or :value string? :anomaly ::anom/anomaly))

modules/fhir-structure/test/blaze/fhir/util_test.clj

@@ -1,7 +1,7 @@
 (ns blaze.fhir.util-test
   (:require
    [blaze.anomaly :as ba]
-   [blaze.fhir.spec.type :as type]
+   [blaze.fhir.spec.type]
    [blaze.fhir.structure-definition-repo]
    [blaze.fhir.util :as fu]
    [blaze.fhir.util-spec]
@@ -198,6 +198,81 @@
              [{:fhir/type :fhir/CodeSystem :id "2"}
               {:fhir/type :fhir/CodeSystem :id "1"}])))))
 
+(deftest coerce-integer-test
+  (testing "valid"
+    (is (= 1 (fu/coerce-integer #fhir/integer 1))))
+
+  (testing "invalid"
+    (doseq [x [#fhir/string "1" #fhir/boolean true nil]]
+      (given (fu/coerce-integer x)
+        ::anom/category := ::anom/incorrect
+        ::anom/message := "Has to be an integer.")))
+
+  (testing "missing value"
+    (given (fu/coerce-integer #fhir/integer{:id "0"})
+      ::anom/category := ::anom/incorrect
+      ::anom/message := "Missing value.")))
+
+(deftest coerce-boolean-test
+  (testing "valid"
+    (is (true? (fu/coerce-boolean #fhir/boolean true)))
+
+    (testing "a false value isn't confused with a missing one"
+      (is (false? (fu/coerce-boolean #fhir/boolean false)))))
+
+  (testing "invalid"
+    (doseq [x [#fhir/string "true" #fhir/integer 1 nil]]
+      (given (fu/coerce-boolean x)
+        ::anom/category := ::anom/incorrect
+        ::anom/message := "Has to be a boolean.")))
+
+  (testing "missing value"
+    (given (fu/coerce-boolean #fhir/boolean{:id "0"})
+      ::anom/category := ::anom/incorrect
+      ::anom/message := "Missing value.")))
+
+(deftest coerce-string-test
+  (testing "valid"
+    (is (= "1" (fu/coerce-string #fhir/string "1"))))
+
+  (testing "invalid"
+    (doseq [x [#fhir/integer 1 #fhir/uri "1" nil]]
+      (given (fu/coerce-string x)
+        ::anom/category := ::anom/incorrect
+        ::anom/message := "Has to be a string.")))
+
+  (testing "missing value"
+    (given (fu/coerce-string #fhir/string{:id "0"})
+      ::anom/category := ::anom/incorrect
+      ::anom/message := "Missing value.")))
+
+(deftest coerce-uri-test
+  (testing "valid"
+    (testing "any FHIR type with a string-valued value is accepted, not just
+              uri, for robustness reasons"
+      (are [x s] (= s (fu/coerce-uri x))
+        #fhir/uri "1" "1"
+        #fhir/url "1" "1"
+        #fhir/canonical "1" "1"
+        #fhir/code "1" "1"
+        #fhir/id "1" "1"
+        #fhir/oid "urn:oid:1.2.3" "urn:oid:1.2.3"
+        #fhir/uuid "urn:uuid:53fefa32-fcbb-4ff8-8a92-55ee120877b7" "urn:uuid:53fefa32-fcbb-4ff8-8a92-55ee120877b7"
+        #fhir/markdown "1" "1"
+        #fhir/string "1" "1")))
+
+  (testing "invalid"
+    (doseq [x [#fhir/integer 1 #fhir/boolean true]]
+      (given (fu/coerce-uri x)
+        ::anom/category := ::anom/incorrect
+        ::anom/message := "Has to be a uri.")))
+
+  (testing "missing value"
+    (doseq [x [#fhir/uri{:id "0"} nil]]
+      (given (fu/coerce-uri x)
+        ::anom/category := ::anom/incorrect
+        ::anom/message := "Missing value."))))
+
 (deftest coerce-params-test
   (testing "simple copy"
     (given (fu/coerce-params
@@ -283,72 +358,3 @@
         ::anom/category := ::anom/incorrect
         ::anom/message := "Missing required parameter `a`."
         :http/status := 400))))
-
-(deftest validate-query-params-test
-  (testing "empty parameter spec"
-    (given (fu/validate-query-params
-            {}
-            {"param" "param-165900"})
-      [:parameter count] := 0))
-
-  (testing "param not in spec ignored"
-    (given (fu/validate-query-params
-            {"code" {:action :copy :coerce #(type/code %2)}}
-            {"different-param" "code-165900"})
-      [:parameter count] := 0))
-
-  (testing "param action :complex not supported with GET"
-    (given (fu/validate-query-params
-            {"coding" {:action :complex}}
-            {"coding" #fhir/Coding {:system #fhir/uri "system-115910"
-                                    :version #fhir/string "version-152300"
-                                    :code #fhir/code "code-115927"}})
-      ::anom/category := ::anom/unsupported
-      ::anom/message := "Unsupported parameter `coding` in GET request. Please use POST."))
-
-  (testing "param not supported"
-    (given (fu/validate-query-params
-            {"code" {}}
-            {"code" "code-165900"})
-      ::anom/category := ::anom/unsupported
-      ::anom/message := "Unsupported parameter `code`."))
-
-  (testing "type/code"
-    (given (fu/validate-query-params
-            {"code" {:action :copy :coerce #(type/code %2)}}
-            {"code" "code-165900"})
-      [:parameter count] := 1
-      [:parameter 0 :name] := #fhir/string "code"
-      [:parameter 0 :value] := #fhir/code "code-165900"))
-
-  (testing "type/boolean"
-    (doseq [value [true false]]
-      (given (fu/validate-query-params
-              {"param-boolean" {:action :copy :coerce fu/coerce-boolean}}
-              {"param-boolean" (str value)})
-        [:parameter count] := 1
-        [:parameter 0 :name] := #fhir/string "param-boolean"
-        [:parameter 0 :value] := (type/boolean value)))
-
-    (doseq [value ["True" "False" "123" "1.5" "nil"]]
-      (given (fu/validate-query-params
-              {"param-boolean" {:action :copy :coerce fu/coerce-boolean}}
-              {"param-boolean" value})
-        ::anom/category := ::anom/incorrect
-        ::anom/message := "Invalid value for parameter `param-boolean`. Has to be a boolean.")))
-
-  (testing "type/integer"
-    (doseq [value [123 -123]]
-      (given (fu/validate-query-params
-              {"param-integer" {:action :copy :coerce fu/coerce-integer}}
-              {"param-integer" (str value)})
-        [:parameter count] := 1
-        [:parameter 0 :name] := #fhir/string "param-integer"
-        [:parameter 0 :value] := (type/integer value)))
-
-    (doseq [value ["true" "false" "1.5" "nil"]]
-      (given (fu/validate-query-params
-              {"param-integer" {:action :copy :coerce fu/coerce-integer}}
-              {"param-integer" value})
-        ::anom/category := ::anom/incorrect
-        ::anom/message := "Invalid value for parameter `param-integer`. Has to be an integer."))))

modules/operation-code-system-lookup/src/blaze/fhir/operation/code_system/lookup.clj

@@ -30,7 +30,7 @@
 (defn- validate-params* [{:keys [request-method body query-params]}]
   (if (= :post request-method)
     body
-    (fu/validate-query-params parameter-specs query-params)))
+    (fhir-util/validate-query-params parameter-specs query-params)))
 
 (defn- validate-params [{{:keys [id]} :path-params :blaze/keys [db] :as request}]
   (if-ok [params (validate-params* request)]

modules/operation-code-system-validate-code/src/blaze/fhir/operation/code_system/validate_code.clj

@@ -32,7 +32,7 @@
 (defn- validate-params* [{:keys [request-method body query-params]}]
   (if (= :post request-method)
     body
-    (fu/validate-query-params parameter-specs query-params)))
+    (fhir-util/validate-query-params parameter-specs query-params)))
 
 (defn- validate-params [{{:keys [id]} :path-params :blaze/keys [db] :as request}]
   (if-ok [params (validate-params* request)]

modules/operation-value-set-expand/src/blaze/fhir/operation/value_set/expand.clj

@@ -23,14 +23,14 @@
    "contextDirection" {}
    "filter" {:action :copy :coerce #(type/string %2)}
    "date" {}
-   "offset" {:action :copy :coerce fu/coerce-integer}
-   "count" {:action :copy :coerce fu/coerce-integer}
-   "includeDesignations" {:action :copy :coerce fu/coerce-boolean}
+   "offset" {:action :copy :coerce fhir-util/coerce-query-integer}
+   "count" {:action :copy :coerce fhir-util/coerce-query-integer}
+   "includeDesignations" {:action :copy :coerce fhir-util/coerce-query-boolean}
    "designation" {}
-   "includeDefinition" {:action :copy :coerce fu/coerce-boolean}
-   "activeOnly" {:action :copy :coerce fu/coerce-boolean}
+   "includeDefinition" {:action :copy :coerce fhir-util/coerce-query-boolean}
+   "activeOnly" {:action :copy :coerce fhir-util/coerce-query-boolean}
    "useSupplement" {}
-   "excludeNested" {:action :copy :coerce fu/coerce-boolean}
+   "excludeNested" {:action :copy :coerce fhir-util/coerce-query-boolean}
    "excludeNotForUI" {}
    "excludePostCoordinated" {}
    "displayLanguage" {:action :copy :coerce #(type/code %2)}
@@ -44,7 +44,7 @@
 (defn- validate-params* [{:keys [request-method body query-params]}]
   (if (= :post request-method)
     body
-    (fu/validate-query-params parameter-specs query-params)))
+    (fhir-util/validate-query-params parameter-specs query-params)))
 
 (defn- validate-params [{{:keys [id]} :path-params :blaze/keys [db] :as request}]
   (if-ok [params (validate-params* request)]

modules/operation-value-set-validate-code/src/blaze/fhir/operation/value_set/validate_code.clj

@@ -31,7 +31,7 @@
    "abstract" {}
    "displayLanguage" {:action :copy :coerce #(type/string %2)}
    "useSupplement" {}
-   "inferSystem" {:action :copy :coerce fu/coerce-boolean}
+   "inferSystem" {:action :copy :coerce fhir-util/coerce-query-boolean}
    "system-version" {:action :copy :coerce #(type/canonical %2)}
    "tx-resource" {:action :complex}})
 
@@ -53,7 +53,7 @@
 (defn- validate-params* [{:keys [request-method] :as request}]
   (if (= :post request-method)
     (body-params request)
-    (fu/validate-query-params parameter-specs (query-params request))))
+    (fhir-util/validate-query-params parameter-specs (query-params request))))
 
 (defn- validate-params [{{:keys [id]} :path-params :blaze/keys [db] :as request}]
   (if-ok [params (validate-params* request)]