Skip to content

Commit

Permalink
Ignore Unknown Values of the _summary Search Param
Browse files Browse the repository at this point in the history
Also make use of multiple _elements parameters.

Closes: #1863
  • Loading branch information
alexanderkiel committed Jul 3, 2024
1 parent c876040 commit 770dd5a
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 39 deletions.
23 changes: 18 additions & 5 deletions modules/interaction/src/blaze/interaction/search/params.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
109 changes: 83 additions & 26 deletions modules/interaction/test/blaze/interaction/search/params_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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))

Expand All @@ -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"))))
Expand All @@ -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"))))
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down Expand Up @@ -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\""]]
Expand Down
4 changes: 2 additions & 2 deletions modules/rest-util/src/blaze/handler/fhir/util.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
14 changes: 10 additions & 4 deletions modules/rest-util/test/blaze/handler/fhir/util_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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"
Expand Down
5 changes: 4 additions & 1 deletion modules/rest-util/test/blaze/handler/util_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down

0 comments on commit 770dd5a

Please sign in to comment.