From 7473ab6a775a6125935c5d54d96ac912963e9c74 Mon Sep 17 00:00:00 2001 From: Alexander Kiel Date: Sun, 8 Dec 2024 17:28:16 +0100 Subject: [PATCH] Implement Parameter _total in FHIR Search Closes: #2226 --- .github/scripts/download-resources-query.sh | 9 +- docs/api.md | 14 +++ .../src/blaze/interaction/search/nav.clj | 5 +- .../src/blaze/interaction/search/params.clj | 11 +- .../src/blaze/interaction/search_type.clj | 119 +++++++++--------- .../blaze/interaction/search_type_test.clj | 51 +++++++- 6 files changed, 140 insertions(+), 69 deletions(-) diff --git a/.github/scripts/download-resources-query.sh b/.github/scripts/download-resources-query.sh index 74253a0a8..439f17508 100755 --- a/.github/scripts/download-resources-query.sh +++ b/.github/scripts/download-resources-query.sh @@ -9,11 +9,16 @@ QUERY=$2 EXPECTED_SIZE=$3 FILE_NAME_PREFIX="$(uuidgen)" -count() { +summary_count() { curl -sH 'Prefer: handling=strict' -H 'Accept: application/fhir+json' "$BASE/$TYPE?$QUERY&_summary=count" | jq .total } -test "count size" "$(count)" "$EXPECTED_SIZE" +total_count() { + curl -sH 'Prefer: handling=strict' -H 'Accept: application/fhir+json' "$BASE/$TYPE?$QUERY&_total=accurate" | jq .total +} + +test "_summary=count count" "$(summary_count)" "$EXPECTED_SIZE" +test "_total=accurate count" "$(total_count)" "$EXPECTED_SIZE" blazectl --no-progress --server "$BASE" download "$TYPE" -q "$QUERY" -o "$FILE_NAME_PREFIX-get.ndjson" diff --git a/docs/api.md b/docs/api.md index 392aca537..5ea815d50 100644 --- a/docs/api.md +++ b/docs/api.md @@ -98,6 +98,20 @@ GET [base]/[type]?param1=value&... POST [base]/[type]/_search ``` +#### Search Result Parameters + +| Name | Description | +|------------------|-----------------------------------------------------------------| +| `_sort` | see Sorting | +| `_count` | the default page size is 50 and the maximum page size is 10.000 | +| `_include` | supported, except the wildcard `*` | +| `_revinclude` | supported, except the wildcard `*` | +| `_summary` | only `count` is supported | +| `_total` | `accurate` is supported | +| `_elements` | fully supported | +| `_contained` | not supported | +| `_containedType` | not supported | + #### _profile Search for `Resource.meta.profile` is supported using the `_profile` search param with exact match or using the `below` modifier as in `_profile:below` with major and minor version prefix match. [Semver][6] is expected for version numbers so a search value of `|1` will find all versions with major version `1` and a search value of `|1.2` will find all versions with major version `1` and minor version `2`. Patch versions are not supported with the `below` modifier. diff --git a/modules/interaction/src/blaze/interaction/search/nav.clj b/modules/interaction/src/blaze/interaction/search/nav.clj index 7851a9aa0..f01ac1bef 100644 --- a/modules/interaction/src/blaze/interaction/search/nav.clj +++ b/modules/interaction/src/blaze/interaction/search/nav.clj @@ -73,12 +73,15 @@ (seq rev-itr) (assoc "_revinclude:iterate" rev-itr)))) -(defn- merge-params [clauses-params {:keys [include-defs summary elements page-size]}] +(defn- merge-params + [clauses-params {:keys [include-defs summary total elements page-size]}] (cond-> clauses-params (seq include-defs) (merge (include-defs->query-params include-defs)) summary (assoc "_summary" summary) + total + (assoc "_total" total) (seq elements) (assoc "_elements" (str/join "," (map name elements))) page-size diff --git a/modules/interaction/src/blaze/interaction/search/params.clj b/modules/interaction/src/blaze/interaction/search/params.clj index 60e2d41c9..43423909b 100644 --- a/modules/interaction/src/blaze/interaction/search/params.clj +++ b/modules/interaction/src/blaze/interaction/search/params.clj @@ -44,6 +44,10 @@ [summary query-params] (or (zero? (fhir-util/page-size query-params)) (= "count" summary))) +(defn- total [{total "_total"}] + (when (= "accurate" total) + total)) + (defn decode "Returns a CompletableFuture that will complete with decoded params or complete exceptionally in case of errors. @@ -54,7 +58,8 @@ [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) - summary (summary handling query-params)] + summary (summary handling query-params) + total (total query-params)] (cond-> {:clauses clauses :include-defs include-defs @@ -65,5 +70,5 @@ :page-type (fhir-util/page-type query-params) :page-id (fhir-util/page-id query-params) :page-offset (fhir-util/page-offset query-params)} - token - (assoc :token token))))) + token (assoc :token token) + total (assoc :total total))))) diff --git a/modules/interaction/src/blaze/interaction/search_type.clj b/modules/interaction/src/blaze/interaction/search_type.clj index 2269a96fe..1ecf96fb3 100644 --- a/modules/interaction/src/blaze/interaction/search_type.clj +++ b/modules/interaction/src/blaze/interaction/search_type.clj @@ -31,10 +31,10 @@ (d/type-list db type page-id) (d/type-list db type))) -(defn- type-query [db type clauses page-id] - (if page-id - (d/type-query db type clauses page-id) - (d/type-query db type clauses))) +(defn- compile-type-query [db type clauses handling] + (if (identical? :blaze.preference.handling/strict handling) + (d/compile-type-query db type clauses) + (d/compile-type-query-lenient db type clauses))) (defn- execute-query [db query page-id] (if page-id @@ -42,22 +42,13 @@ (d/execute-query db query))) (defn- handles-and-clauses - [{:keys [type] :blaze.preference/keys [handling] - {:keys [clauses page-id]} :params} - db] - (cond - (empty? clauses) + [{:blaze/keys [db] :keys [type] :blaze.preference/keys [handling] + {:keys [clauses page-id]} :params}] + (if (empty? clauses) {:handles (type-list db type page-id)} - - (identical? :blaze.preference.handling/strict handling) - (when-ok [handles (type-query db type clauses page-id)] - {:handles handles - :clauses clauses}) - - :else - (when-ok [query (d/compile-type-query-lenient db type clauses)] + (when-ok [query (compile-type-query db type clauses handling)] {:handles (execute-query db query page-id) - :clauses (d/query-clauses query)}))) + :query query}))) (defn- build-matches-only-page [page-size handles] (let [handles (into [] (take (inc page-size)) handles)] @@ -95,6 +86,34 @@ (map #(d/pull-many db % elements)) (map (partial d/pull-many db)))) +(defn- total-future + "Calculates the total number of resources returned. + + If we are on the first page (page-id is nil) and we don't have a next match, + we can use number of matches. + + If we have no query (returning all resources), we can use `d/type-total`. + + If we are forced to calculate the total (total is accurate) we issue an + `d/count-query`. + + Otherwise, we don't report it." + [db type query {:keys [page-id total]} matches next-match] + (cond + ;; evaluate this criteria first, because we can potentially safe the + ;; d/type-total call + (and (nil? page-id) (nil? next-match)) + (ac/completed-future (count matches)) + + (nil? query) + (ac/completed-future (d/type-total db type)) + + (= "accurate" total) + (d/count-query db query) + + :else + (ac/completed-future nil))) + (defn- page-data "Returns a CompletableFuture that will complete with a map of: @@ -105,13 +124,16 @@ or an anomaly in case of errors." {:arglists '([context])} - [{:blaze/keys [db] {:keys [include-defs page-size] :as params} :params :as context}] - (if-ok [{:keys [handles clauses]} (handles-and-clauses context db)] + [{:blaze/keys [db] :keys [type] + {:keys [include-defs page-size] :as params} :params + :as context}] + (if-ok [{:keys [handles query]} (handles-and-clauses context)] (let [{:keys [matches includes next-match]} (build-page db include-defs page-size handles) + total-future (total-future db type query params matches next-match) match-futures (into [] (comp (partition-all 100) (pull-matches-xf params db)) matches) include-futures (into [] (comp (partition-all 100) (map (partial d/pull-many db))) includes)] - (-> (ac/all-of (into match-futures include-futures)) + (-> (ac/all-of (into (conj match-futures total-future) include-futures)) (ac/exceptionally #(assoc % ::anom/category ::anom/fault @@ -119,9 +141,9 @@ (ac/then-apply (fn [_] {:entries (entries context match-futures include-futures) - :num-matches (count matches) :next-handle next-match - :clauses clauses})))) + :total (ac/join total-future) + :clauses (some-> query d/query-clauses)})))) ac/completed-future)) (defn- link [relation url] @@ -143,24 +165,6 @@ (->> (next-link-url-fn token clauses (next-link-offset next-handle)) (link "next"))) -(defn- total - "Calculates the total number of resources returned. - - If we have no clauses (returning all resources), we can use `d/type-total`. - Secondly, if the number of entries found is not more than one page in size, - we can use that number. Otherwise, there is no cheap way to calculate the - number of matching resources, so we don't report it." - [{:keys [type] :blaze/keys [db] {:keys [clauses page-id]} :params} - num-matches next-handle] - (cond - ;; evaluate this criteria first, because we can potentially safe the - ;; d/type-total call - (and (nil? page-id) (nil? next-handle)) - num-matches - - (empty? clauses) - (d/type-total db type))) - (defn- zero-bundle "Generate a special bundle if the search results in zero matches to avoid generating a token for the first link, we don't need in this case." @@ -193,24 +197,16 @@ (defn- search-normal [context] (-> (page-data context) (ac/then-compose - (fn [{:keys [entries num-matches next-handle clauses]}] - (let [total (total context num-matches next-handle)] - (if (some-> total zero?) - (ac/completed-future (zero-bundle context clauses)) - (do-sync [token (gen-token! context clauses)] - (if next-handle - (-> (normal-bundle context token clauses entries total) - (update :link conj (next-link context token clauses next-handle))) - (normal-bundle context token clauses entries total))))))) + (fn [{:keys [entries next-handle total clauses]}] + (if (some-> total zero?) + (ac/completed-future (zero-bundle context clauses)) + (do-sync [token (gen-token! context clauses)] + (if next-handle + (-> (normal-bundle context token clauses entries total) + (update :link conj (next-link context token clauses next-handle))) + (normal-bundle context token clauses entries total)))))) (ac/then-apply ring/response))) -(defn- compile-type-query - [{:keys [type] :blaze/keys [db] :blaze.preference/keys [handling] - {:keys [clauses]} :params}] - (if (identical? :blaze.preference.handling/strict handling) - (d/compile-type-query db type clauses) - (d/compile-type-query-lenient db type clauses))) - (defn- summary-response [context total clauses] (ring/response {:fhir/type :fhir/Bundle @@ -223,10 +219,11 @@ (ac/completed-future (summary-response context (d/type-total db type) []))) (defn- search-summary - [{:blaze/keys [db] {:keys [clauses]} :params :as context}] + [{:keys [type] :blaze/keys [db] {:keys [clauses]} :params + :blaze.preference/keys [handling] :as context}] (if (empty? clauses) (no-query-summary-response context) - (if-ok [query (compile-type-query context)] + (if-ok [query (compile-type-query db type clauses handling)] (let [clauses (d/query-clauses query)] (if (empty? clauses) (no-query-summary-response context) @@ -279,8 +276,8 @@ of the next link." [{:blaze/keys [base-url db] :as request} page-id-cipher params] (fn [token clauses offset] - (nav/token-url base-url page-id-cipher (page-match request) params token clauses - (d/t db) offset))) + (nav/token-url base-url page-id-cipher (page-match request) params token + clauses (d/t db) offset))) (defn- search-context [{:keys [page-store page-id-cipher] :as context} diff --git a/modules/interaction/test/blaze/interaction/search_type_test.clj b/modules/interaction/test/blaze/interaction/search_type_test.clj index 7d045a619..8690fd38d 100644 --- a/modules/interaction/test/blaze/interaction/search_type_test.clj +++ b/modules/interaction/test/blaze/interaction/search_type_test.clj @@ -852,7 +852,30 @@ (link-url body "next")))) (testing "the bundle contains one entry" - (is (= 1 (count (:entry body))))))) + (is (= 1 (count (:entry body)))))) + + (testing "with _total=accurate" + (let [{:keys [body]} + @(handler + {:params {"active" "true" "_total" "accurate" "_count" "1"}})] + + (testing "the total count is 2" + (is (= #fhir/unsignedInt 2 (:total body)))) + + (testing "has a self link" + (is (= (str base-url context-path "/Patient?active=true&_total=accurate&_count=1") + (link-url body "self")))) + + (testing "has a first link" + (is (= (page-url page-id-cipher "Patient" {"active" ["true"] "_total" "accurate" "_count" "1" "__t" "1"}) + (link-url body "first")))) + + (testing "has a next link with search params" + (is (= (page-url page-id-cipher "Patient" {"active" ["true"] "_total" "accurate" "_count" "1" "__t" "1" "__page-id" "2"}) + (link-url body "next")))) + + (testing "the bundle contains one entry" + (is (= 1 (count (:entry body)))))))) (testing "search for inactive patients" (let [{:keys [body]} @@ -899,7 +922,31 @@ (link-url body "next")))) (testing "the bundle contains one entry" - (is (= 1 (count (:entry body))))))) + (is (= 1 (count (:entry body)))))) + + (testing "with _total=accurate" + (let [{:keys [body]} + @(handler + {::reitit/match patient-search-match + :params {"active" "true" "_total" "accurate" "_count" "1"}})] + + (testing "the total count is 2" + (is (= #fhir/unsignedInt 2 (:total body)))) + + (testing "has a self link" + (is (= (str base-url context-path "/Patient?active=true&_total=accurate&_count=1") + (link-url body "self")))) + + (testing "has a first link with token" + (is (= (page-url page-id-cipher "Patient" {"__token" "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAC" "_total" "accurate" "_count" "1" "__t" "1"}) + (link-url body "first")))) + + (testing "has a next link with token" + (is (= (page-url page-id-cipher "Patient" {"__token" "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAC" "_total" "accurate" "_count" "1" "__t" "1" "__page-id" "2"}) + (link-url body "next")))) + + (testing "the bundle contains one entry" + (is (= 1 (count (:entry body)))))))) (testing "search for inactive patients" (let [{:keys [body]}