From 770dd5a8ad6348d6d70544a07e3c9e86c929f74d Mon Sep 17 00:00:00 2001 From: Alexander Kiel Date: Wed, 3 Jul 2024 13:22:19 +0200 Subject: [PATCH] Ignore Unknown Values of the _summary Search Param Also make use of multiple _elements parameters. Closes: #1863 --- .../src/blaze/interaction/search/params.clj | 23 +++- .../blaze/interaction/search/params_test.clj | 109 +++++++++++++----- .../rest_api/capabilities_handler_test.clj | 4 +- .../rest-util/src/blaze/handler/fhir/util.clj | 4 +- .../test/blaze/handler/fhir/util_test.clj | 14 ++- .../test/blaze/handler/util_test.clj | 5 +- 6 files changed, 120 insertions(+), 39 deletions(-) diff --git a/modules/interaction/src/blaze/interaction/search/params.clj b/modules/interaction/src/blaze/interaction/search/params.clj index dfc8d35d2..60e2d41c9 100644 --- a/modules/interaction/src/blaze/interaction/search/params.clj +++ b/modules/interaction/src/blaze/interaction/search/params.clj @@ -7,7 +7,9 @@ [blaze.interaction.util :as iu] [blaze.page-store :as page-store] [blaze.page-store.spec] - [clojure.spec.alpha :as s])) + [blaze.util :as u] + [clojure.spec.alpha :as s] + [clojure.string :as str])) (defn- clauses [page-store {token "__token" :as query-params}] (cond @@ -27,9 +29,19 @@ (when-ok [clauses (iu/clauses query-params)] {:clauses clauses})))) +(defn- summary + "Returns true if a summary result is requested." + [handling {summary "_summary"}] + (let [value (some #{"count"} (u/to-seq summary))] + (if (and (nil? value) + (identical? :blaze.preference.handling/strict handling) + (seq (u/to-seq summary))) + (ba/unsupported (str "Unsupported _summary search param with value(s): " (str/join ", " (u/to-seq summary)))) + value))) + (defn- summary? "Returns true if a summary result is requested." - [{summary "_summary" :as query-params}] + [summary query-params] (or (zero? (fhir-util/page-size query-params)) (= "count" summary))) (defn decode @@ -41,12 +53,13 @@ :token - possibly a token encoding the query clauses" [page-store handling query-params] (do-sync [{:keys [clauses token]} (clauses page-store query-params)] - (when-ok [include-defs (include/include-defs handling query-params)] + (when-ok [include-defs (include/include-defs handling query-params) + summary (summary handling query-params)] (cond-> {:clauses clauses :include-defs include-defs - :summary? (summary? query-params) - :summary (get query-params "_summary") + :summary? (summary? summary query-params) + :summary summary :elements (fhir-util/elements query-params) :page-size (fhir-util/page-size query-params) :page-type (fhir-util/page-type query-params) diff --git a/modules/interaction/test/blaze/interaction/search/params_test.clj b/modules/interaction/test/blaze/interaction/search/params_test.clj index 66bd023e8..133681b7b 100644 --- a/modules/interaction/test/blaze/interaction/search/params_test.clj +++ b/modules/interaction/test/blaze/interaction/search/params_test.clj @@ -6,6 +6,7 @@ [blaze.interaction.search.params :as params] [blaze.interaction.search.params-spec] [blaze.page-store.protocols :as p] + [blaze.preference.handling :as-alias handling] [blaze.test-util :as tu] [clojure.spec.test.alpha :as st] [clojure.string :as str] @@ -23,21 +24,18 @@ (deftest decode-test (testing "unsupported sort parameter" (given-failed-future (params/decode page-store - :blaze.preference.handling/lenient + ::handling/lenient {"_sort" "a,b"}) ::anom/category := ::anom/unsupported)) (testing "invalid include parameter" (given-failed-future (params/decode page-store - :blaze.preference.handling/strict + ::handling/strict {"_include" "Observation"}) ::anom/category := ::anom/incorrect)) (testing "decoding clauses from query params" - (given @(params/decode - page-store - :blaze.preference.handling/strict - {"foo" "bar"}) + (given @(params/decode page-store ::handling/strict {"foo" "bar"}) :clauses := [["foo" "bar"]] :token := nil)) @@ -47,7 +45,7 @@ (-get [_ token] (assert (= (str/join (repeat 32 "A")) token)) (ac/completed-future [["foo" "bar"]]))) - :blaze.preference.handling/strict + ::handling/strict {"__token" (str/join (repeat 32 "A"))}) :clauses := [["foo" "bar"]] :token := (str/join (repeat 32 "A")))) @@ -59,38 +57,97 @@ (-get [_ token] (assert (= (str/join (repeat 32 "A")) token)) (ac/completed-future (ba/not-found "Not Found")))) - :blaze.preference.handling/strict + ::handling/strict {"__token" (str/join (repeat 32 "A"))}) ::anom/category := ::anom/not-found :http/status := nil)) (testing "decoding _elements" (testing "one element" - (given @(params/decode page-store :blaze.preference.handling/strict - {"_elements" "a"}) - :elements := [:a])) + (doseq [handling [::handling/strict ::handling/lenient nil]] + (given @(params/decode page-store handling {"_elements" "a"}) + :elements := [:a]))) (testing "two elements" - (given @(params/decode page-store :blaze.preference.handling/strict - {"_elements" "a,b"}) - :elements := [:a :b])) + (doseq [handling [::handling/strict ::handling/lenient nil]] + (given @(params/decode page-store handling + {"_elements" "a,b"}) + :elements := [:a :b]))) (testing "two elements with space after comma" - (given @(params/decode page-store :blaze.preference.handling/strict - {"_elements" "a, b"}) - :elements := [:a :b])) + (doseq [handling [::handling/strict ::handling/lenient nil]] + (given @(params/decode page-store handling + {"_elements" "a, b"}) + :elements := [:a :b]))) (testing "two elements with space before comma" - (given @(params/decode page-store :blaze.preference.handling/strict - {"_elements" "a ,b"}) - :elements := [:a :b])) + (doseq [handling [::handling/strict ::handling/lenient nil]] + (given @(params/decode page-store handling + {"_elements" "a ,b"}) + :elements := [:a :b]))) (testing "two elements with space before and after comma" - (given @(params/decode page-store :blaze.preference.handling/strict - {"_elements" "a , b"}) - :elements := [:a :b])) + (doseq [handling [::handling/strict ::handling/lenient nil]] + (given @(params/decode page-store handling + {"_elements" "a , b"}) + :elements := [:a :b]))) (testing "three elements" - (given @(params/decode page-store :blaze.preference.handling/strict - {"_elements" "a,b,c"}) - :elements := [:a :b :c])))) + (doseq [handling [::handling/strict ::handling/lenient nil]] + (given @(params/decode page-store handling + {"_elements" "a,b,c"}) + :elements := [:a :b :c]))) + + (testing "two elements parameters" + (doseq [handling [::handling/strict ::handling/lenient nil]] + (given @(params/decode page-store handling + {"_elements" ["a" "b"]}) + :elements := [:a :b])))) + + (testing "decoding _summary" + (testing "count" + (doseq [handling [::handling/strict ::handling/lenient nil]] + (given @(params/decode page-store handling {"_summary" "count"}) + :summary? := true + :summary := "count"))) + + (testing "invalid counts" + (doseq [handling [::handling/lenient nil]] + (given @(params/decode page-store handling {"_summary" "counts"}) + :summary? := false + :summary := nil)) + + (given-failed-future (params/decode page-store ::handling/strict + {"_summary" "counts"}) + ::anom/category := ::anom/unsupported + ::anom/message := "Unsupported _summary search param with value(s): counts")) + + (testing "count and invalid counts" + (doseq [handling [::handling/strict ::handling/lenient nil]] + (given @(params/decode page-store handling + {"_summary" ["count" "counts"]}) + :summary? := true + :summary := "count"))) + + (testing "unsupported true" + (doseq [handling [::handling/lenient nil]] + (given @(params/decode page-store handling {"_summary" "true"}) + :summary? := false + :summary := nil)) + + (given-failed-future (params/decode page-store ::handling/strict + {"_summary" "true"}) + ::anom/category := ::anom/unsupported + ::anom/message := "Unsupported _summary search param with value(s): true")) + + (testing "unsupported true and text" + (doseq [handling [::handling/lenient nil]] + (given @(params/decode page-store handling + {"_summary" ["true" "text"]}) + :summary? := false + :summary := nil)) + + (given-failed-future (params/decode page-store ::handling/strict + {"_summary" ["true" "text"]}) + ::anom/category := ::anom/unsupported + ::anom/message := "Unsupported _summary search param with value(s): true, text")))) diff --git a/modules/rest-api/test/blaze/rest_api/capabilities_handler_test.clj b/modules/rest-api/test/blaze/rest_api/capabilities_handler_test.clj index 8feabc558..88ef5b795 100644 --- a/modules/rest-api/test/blaze/rest_api/capabilities_handler_test.clj +++ b/modules/rest-api/test/blaze/rest_api/capabilities_handler_test.clj @@ -6,6 +6,7 @@ [blaze.fhir.test-util :refer [structure-definition-repo]] [blaze.module.test-util :refer [with-system]] [blaze.rest-api :as-alias rest-api] + [blaze.rest-api.capabilities-handler] [blaze.rest-api.header-spec] [blaze.rest-api.spec] [blaze.test-util :as tu :refer [given-thrown]] @@ -166,7 +167,8 @@ @(handler {:query-params {"_elements" (str/join "," (map name ks))} :blaze/base-url "base-url-131713"})] - (= (set (conj ks :fhir/type)) (set (keys body))))))) + (or (empty? ks) + (= (set (conj ks :fhir/type)) (set (keys body)))))))) (testing "cache validation" (doseq [if-none-match ["W/\"518da6fc\"" "W/\"518da6fc\", \"foo\""]] diff --git a/modules/rest-util/src/blaze/handler/fhir/util.clj b/modules/rest-util/src/blaze/handler/fhir/util.clj index e6c7312f4..2433d0f68 100644 --- a/modules/rest-util/src/blaze/handler/fhir/util.clj +++ b/modules/rest-util/src/blaze/handler/fhir/util.clj @@ -81,10 +81,10 @@ (defn elements "Returns a vector of keywords created from the comma separated values of - the first valid `_elements` query param or `[]` otherwise." + all `_elements` query params." {:arglists '([query-params])} [{v "_elements"}] - (mapv keyword (some-> v (str/split #"\s*,\s*")))) + (into [] (comp (mapcat #(str/split % #"\s*,\s*")) (remove str/blank?) (map keyword)) (u/to-seq v))) (defn- incorrect-date-msg [name value] (format "The value `%s` of the query param `%s` is no valid date." value name)) diff --git a/modules/rest-util/test/blaze/handler/fhir/util_test.clj b/modules/rest-util/test/blaze/handler/fhir/util_test.clj index 3d294e153..f4d95da8c 100644 --- a/modules/rest-util/test/blaze/handler/fhir/util_test.clj +++ b/modules/rest-util/test/blaze/handler/fhir/util_test.clj @@ -11,6 +11,7 @@ [blaze.handler.fhir.util-spec] [blaze.module.test-util :refer [with-system]] [blaze.test-util :as tu :refer [satisfies-prop]] + [clojure.set :as set] [clojure.spec.alpha :as s] [clojure.spec.test.alpha :as st] [clojure.string :as str] @@ -142,11 +143,16 @@ {})) (testing "_elements is present" - (tu/satisfies-prop 1000 - (prop/for-all [fields fields-gen] - (let [query-params {"_elements" (fields :string)}] + (tu/satisfies-prop 100 + (prop/for-all [fields (gen/vector fields-gen)] + (let [values (mapv :string fields) + values (cond + (< 1 (count values)) values + (empty? values) "" + :else (first values)) + query-params {"_elements" values}] (= (set (fhir-util/elements query-params)) - (set (fields :vector)))))))) + (apply set/union (map (comp set :vector) fields)))))))) (deftest date-test (testing "missing" diff --git a/modules/rest-util/test/blaze/handler/util_test.clj b/modules/rest-util/test/blaze/handler/util_test.clj index 1b4599a6a..6bffc39f8 100644 --- a/modules/rest-util/test/blaze/handler/util_test.clj +++ b/modules/rest-util/test/blaze/handler/util_test.clj @@ -31,7 +31,10 @@ (testing "handling" (are [headers res] (= res (handler-util/preference headers "handling")) {"prefer" "handling=strict"} :blaze.preference.handling/strict - {"prefer" "handling=lenient"} :blaze.preference.handling/lenient)) + {"prefer" "handling=lenient"} :blaze.preference.handling/lenient + {"prefer" ""} nil + {} nil + nil nil)) (testing "respond-async" (are [headers res] (= res (handler-util/preference headers "respond-async"))