Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Parameter _total in FHIR Search #2241

Merged
merged 1 commit into from
Dec 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions .github/scripts/download-resources-query.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
14 changes: 14 additions & 0 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<url>|1` will find all versions with major version `1` and a search value of `<url>|1.2` will find all versions with major version `1` and minor version `2`. Patch versions are not supported with the `below` modifier.
Expand Down
5 changes: 4 additions & 1 deletion modules/interaction/src/blaze/interaction/search/nav.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 8 additions & 3 deletions modules/interaction/src/blaze/interaction/search/params.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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)))))
119 changes: 58 additions & 61 deletions modules/interaction/src/blaze/interaction/search_type.clj
Original file line number Diff line number Diff line change
Expand Up @@ -31,33 +31,24 @@
(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
(d/execute-query db query page-id)
(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)]
Expand Down Expand Up @@ -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:

Expand All @@ -105,23 +124,26 @@

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
:fhir/issue "incomplete"))
(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]
Expand All @@ -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."
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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}
Expand Down
51 changes: 49 additions & 2 deletions modules/interaction/test/blaze/interaction/search_type_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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]}
Expand Down Expand Up @@ -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]}
Expand Down
Loading