Fix Paging on _id Parameter with Unsorted IDs

.github/scripts/patient-ids.sh

@@ -7,4 +7,6 @@ SCRIPT_DIR="$(dirname "$(readlink -f "$0")")"
 
 BASE="http://localhost:8080/fhir"
 PATIENT_IDENTIFIERS="${1//[[:space:]]/}"
-curl -s "$BASE/Patient?identifier=$PATIENT_IDENTIFIERS&_count=1000" | jq -r '.entry[].resource.id' | paste -sd, -
+
+# it's important to shuffle the IDs here in order to test that FHIR search works with unordered IDs
+blazectl download --server "$BASE" Patient -p -q "identifier=$PATIENT_IDENTIFIERS" | jq -r '.id' | shuf | paste -sd, -

.github/workflows/build.yml

@@ -1426,24 +1426,57 @@ jobs:
     - name: Download Patient Resources with given IDs
       env:
         PATIENT_SSNS: >-
-          999-89-9294,999-60-3949,999-40-8052,999-86-7721,999-31-6940,999-37-5253,
-          999-14-4781,999-71-5798,999-85-9014,999-88-9231,999-51-2985,999-14-2168,
-          999-51-2170,999-52-2914,999-45-6374,999-58-8110,999-92-6349,999-14-7569,
-          999-58-5645,999-89-6440,999-94-8430,999-90-6541,999-36-1192,999-30-5696,
-          999-91-6238,999-12-1519,999-44-9085,999-99-3624,999-28-6755,999-60-2281,
-          999-30-8040,999-99-3859,999-84-8058,999-70-9633,999-68-6985,999-83-6253,
-          999-24-4528,999-57-1697,999-27-7985,999-50-8738,999-21-6355,999-74-7491,
-          999-59-3737,999-54-5495,999-88-2066,999-64-2743,999-53-8490,999-18-4716,
-          999-22-4364,999-29-3271,999-11-9976,999-44-5315,999-48-9431,999-50-3850,
-          999-54-1289,999-49-1081,999-37-7151,999-25-2068,999-61-9829,999-56-2917,
-          999-37-1371,999-29-2856,999-96-2459,999-74-1202,999-74-3491,999-53-2578,
-          999-77-8967,999-53-6824,999-26-9128,999-71-5114,999-10-5138,999-58-7855,
-          999-11-1543,999-66-3180,999-39-8247,999-96-1580,999-72-5318,999-23-7555,
-          999-52-6588,999-88-2620,999-11-3074,999-89-9206,999-47-9791,999-43-4776,
-          999-84-6431,999-53-9409,999-57-9047,999-98-1131,999-23-9506,999-79-6781,
-          999-10-3828,999-31-2101,999-42-5310,999-46-3927,999-68-3132,999-45-8011,
-          999-11-7186,999-58-3121,999-48-7111,999-24-3722
-      run: .github/scripts/download-resources-query.sh Patient "_id=$(.github/scripts/patient-ids.sh "$PATIENT_SSNS")" 100
+          999-21-6355,999-13-3823,999-27-7985,999-86-3618,999-93-7964,999-73-8558,
+          999-91-8665,999-50-8738,999-13-8439,999-28-7892,999-33-7679,999-83-6485,
+          999-30-5925,999-98-7459,999-14-8063,999-59-3737,999-54-5495,999-88-2066,
+          999-53-8490,999-74-7491,999-64-2743,999-18-4716,999-29-3271,999-11-9976,
+          999-44-5315,999-22-4364,999-50-3850,999-40-8052,999-54-1289,999-37-7151,
+          999-49-1081,999-61-9829,999-25-2068,999-48-9431,999-14-4781,999-71-5798,
+          999-86-7721,999-31-6940,999-37-5253,999-51-2985,999-85-9014,999-89-9294,
+          999-60-3949,999-88-9231,999-51-2170,999-14-2168,999-52-2914,999-45-6374,
+          999-58-8110,999-58-5645,999-14-7569,999-92-6349,999-89-6440,999-94-8430,
+          999-90-6541,999-36-1192,999-84-8058,999-91-6238,999-12-1519,999-44-9085,
+          999-99-3624,999-60-2281,999-28-6755,999-30-8040,999-99-3859,999-30-5696,
+          999-68-6985,999-70-9633,999-83-6253,999-24-4528,999-57-1697,999-29-2856,
+          999-96-2459,999-74-1202,999-74-3491,999-56-2917,999-53-2578,999-37-1371,
+          999-53-6824,999-26-9128,999-71-5114,999-10-5138,999-58-7855,999-11-1543,
+          999-11-3074,999-47-9791,999-89-9206,999-66-3180,999-77-8967,999-72-5318,
+          999-23-7555,999-52-6588,999-88-2620,999-23-9506,999-98-1131,999-79-6781,
+          999-43-4776,999-84-6431,999-39-8247,999-96-1580,999-36-5527,999-99-4279,
+          999-19-7218,999-91-6146,999-58-6456,999-35-2730,999-94-5196,999-36-7625,
+          999-87-4481,999-82-5173,999-58-9303,999-27-5358,999-38-8362,999-74-4503,
+          999-60-2346,999-81-7132,999-16-5530,999-59-3296,999-96-2754,999-31-5793,
+          999-63-9944,999-84-7193,999-37-7846,999-70-6935,999-79-7963,999-21-2993,
+          999-20-5711,999-49-6953,999-60-1829,999-17-9141,999-55-2550,999-33-3340,
+          999-75-8758,999-42-2353,999-41-4960,999-72-3108,999-51-9785,999-32-4696,
+          999-87-1692,999-78-6946,999-79-5928,999-28-4726,999-78-7357,999-91-5448,
+          999-71-7680,999-72-4531,999-55-4334,999-80-4102,999-99-1126,999-77-5295,
+          999-47-8672,999-91-5681,999-11-9949,999-42-1240,999-28-2768,999-79-4394,
+          999-74-6145,999-49-1648,999-98-9296,999-71-9591,999-27-1668,999-63-3783,
+          999-66-4132,999-63-9151,999-33-3123,999-60-4007,999-41-5400,999-36-1925,
+          999-78-6396,999-25-1767,999-27-6284,999-55-5744,999-91-3357,999-46-5669,
+          999-47-4456,999-37-3279,999-69-4320,999-96-1711,999-11-7347,999-87-2660,
+          999-40-9518,999-30-5419,999-84-6225,999-35-7213,999-50-4337,999-20-6881,
+          999-88-8626,999-85-4520,999-99-7423,999-93-6074,999-73-6096,999-49-7536,
+          999-16-8660,999-97-1001,999-55-7176,999-26-5009,999-21-2567,999-70-7173,
+          999-50-8491,999-47-1432,999-74-6461,999-97-2022,999-65-4916,999-50-3056,
+          999-44-2671,999-61-3570,999-64-7627,999-53-6591,999-73-9725,999-40-8071,
+          999-48-3926,999-82-4485,999-92-8668,999-28-9692,999-24-8156,999-63-8895,
+          999-45-8011,999-48-7111,999-10-3828,999-31-2101,999-46-3927,999-53-9409,
+          999-42-5310,999-68-3132,999-57-9047,999-24-3722,999-11-7186,999-43-3852,
+          999-58-3121,999-85-3188,999-43-4015,999-50-9097,999-53-9876,999-76-8113,
+          999-28-9201,999-48-5783,999-70-7158,999-89-4529,999-70-8614,999-81-2546,
+          999-75-7830,999-83-4725,999-69-9815,999-55-7212,999-31-9171,999-25-7065,
+          999-94-1053,999-45-4290,999-88-8247,999-89-4372,999-26-3144,999-22-8920,
+          999-98-1674,999-34-1694,999-10-3128,999-42-6020,999-86-3199,999-16-8888,
+          999-60-3122,999-90-2327,999-10-4587,999-57-2528,999-72-9441,999-45-7616,
+          999-55-8093,999-31-2790,999-94-1391,999-36-9334,999-55-7518,999-17-3141,
+          999-60-6082,999-90-1527,999-40-6116,999-24-7513,999-57-8253,999-55-5363,
+          999-89-6219,999-42-1353,999-82-6179,999-84-1172,999-44-1850,999-72-7332,
+          999-68-1649,999-26-2356,999-18-5134,999-42-2500,999-95-3358,999-35-1328,
+          999-87-5740,999-97-9920,999-83-2307,999-51-4707,999-38-2717,999-91-8913,
+          999-58-5930,999-50-2058,999-87-8426,999-98-3147,999-29-1152,999-95-9757
+      run: .github/scripts/download-resources-query.sh Patient "_id=$(.github/scripts/patient-ids.sh "$PATIENT_SSNS")" 300
 
     - name: Download Vital Sign Resources
       run: .github/scripts/download-resources-query.sh Observation "category=vital-signs" 152877

modules/db-protocols/src/blaze/db/impl/protocols.clj

@@ -138,6 +138,7 @@
 
 (defprotocol WithOrderedIndexHandles
   (-ordered-index-handles
+    [search-param batch-db tid modifier compiled-values]
     [search-param batch-db tid modifier compiled-values start-id]))
 
 (defprotocol SearchParamRegistry

modules/db/deps.edn

@@ -63,7 +63,10 @@
 
    :extra-deps
    {blaze/fhir-test-util
-    {:local/root "../fhir-test-util"}}}
+    {:local/root "../fhir-test-util"}
+
+    org.clojure/math.combinatorics
+    {:mvn/version "0.1.6"}}}
 
   :kaocha
   {:extra-deps

modules/db/src/blaze/db/impl/search_param.clj

@@ -17,7 +17,6 @@
    [blaze.db.impl.search-param.quantity]
    [blaze.db.impl.search-param.string]
    [blaze.db.impl.search-param.token]
-   [blaze.db.impl.search-param.util :as u]
    [blaze.fhir-path :as fhir-path]
    [blaze.fhir.spec :as fhir-spec]
    [blaze.fhir.spec.references :as fsr]
@@ -87,8 +86,7 @@
   ([search-param batch-db tid modifier compiled-values]
    (if (= 1 (count compiled-values))
      (p/-index-handles search-param batch-db tid modifier (first compiled-values))
-     (let [index-handles #(p/-index-handles search-param batch-db tid modifier %)]
-       (u/union-index-handles (map index-handles compiled-values)))))
+     (p/-ordered-index-handles search-param batch-db tid modifier compiled-values)))
   ([search-param batch-db tid modifier compiled-values start-id]
    (if (= 1 (count compiled-values))
      (p/-index-handles search-param batch-db tid modifier (first compiled-values) start-id)

modules/db/src/blaze/db/impl/search_param/composite/token_token.clj

@@ -14,6 +14,11 @@
 (defrecord SearchParamCompositeTokenToken [name url type base code c-hash
                                            main-expression c1 c2]
   p/WithOrderedIndexHandles
+  (-ordered-index-handles
+    [search-param batch-db tid modifier compiled-values]
+    (let [index-handles #(p/-index-handles search-param batch-db tid modifier %)]
+      (u/union-index-handles (map index-handles compiled-values))))
+
   (-ordered-index-handles
     [search-param batch-db tid modifier compiled-values start-id]
     (let [index-handles #(p/-index-handles search-param batch-db tid modifier % start-id)]

modules/db/src/blaze/db/impl/search_param/list.clj

@@ -45,6 +45,11 @@
 
 (defrecord SearchParamList [name type code]
   p/WithOrderedIndexHandles
+  (-ordered-index-handles
+    [search-param batch-db tid modifier compiled-values]
+    (let [index-handles #(p/-index-handles search-param batch-db tid modifier %)]
+      (u/union-index-handles (map index-handles compiled-values))))
+
   (-ordered-index-handles
     [search-param batch-db tid modifier compiled-values start-id]
     (let [index-handles #(p/-index-handles search-param batch-db tid modifier % start-id)]

modules/db/src/blaze/db/impl/search_param/token.clj

@@ -160,6 +160,11 @@
 
 (defrecord SearchParamToken [name url type base code target c-hash expression]
   p/WithOrderedIndexHandles
+  (-ordered-index-handles
+    [search-param batch-db tid modifier compiled-values]
+    (let [index-handles #(p/-index-handles search-param batch-db tid modifier %)]
+      (u/union-index-handles (map index-handles compiled-values))))
+
   (-ordered-index-handles
     [search-param batch-db tid modifier compiled-values start-id]
     (let [index-handles #(p/-index-handles search-param batch-db tid modifier % start-id)]
@@ -252,6 +257,11 @@
 
 (defrecord SearchParamTokenIdentifier [name url type base code target c-hash expression]
   p/WithOrderedIndexHandles
+  (-ordered-index-handles
+    [search-param batch-db tid modifier compiled-values]
+    (let [index-handles #(p/-index-handles search-param batch-db tid modifier %)]
+      (u/union-index-handles (map index-handles compiled-values))))
+
   (-ordered-index-handles
     [search-param batch-db tid modifier compiled-values start-id]
     (let [index-handles #(p/-index-handles search-param batch-db tid modifier % start-id)]
@@ -305,13 +315,19 @@
 
 (defrecord SearchParamId [name type code]
   p/WithOrderedIndexHandles
+  (-ordered-index-handles
+    [search-param batch-db tid modifier compiled-values]
+    (let [index-handles #(p/-index-handles search-param batch-db tid modifier %)]
+      (coll/eduction (mapcat index-handles) (sort compiled-values))))
+
   (-ordered-index-handles
     [search-param batch-db tid modifier compiled-values start-id]
-    (u/union-index-handles
-     (coll/eduction
-      (comp (drop-while #(not= start-id %))
-            (map #(p/-index-handles search-param batch-db tid modifier %)))
-      compiled-values)))
+    (let [compiled-values (drop-while #(not= start-id %) (sort compiled-values))
+          index-handles #(p/-index-handles search-param batch-db tid modifier %)]
+      (condp = (count compiled-values)
+        0 []
+        1 (index-handles (first compiled-values))
+        (coll/eduction (mapcat index-handles) compiled-values))))
 
   p/SearchParam
   (-compile-value [_ _ value]
@@ -328,7 +344,7 @@
         []))
 
   (-index-handles [sp batch-db tid modifier compiled-value start-id]
-    (if (= compiled-value start-id)
+    (if (bs/<= start-id compiled-value)
       (p/-index-handles sp batch-db tid modifier compiled-value)
       []))
 

modules/db/test/blaze/db/api_test.clj

@@ -28,6 +28,7 @@
    [blaze.fhir.spec.type.system :as system]
    [blaze.module.test-util :as mtu :refer [given-failed-future with-system]]
    [blaze.test-util :as tu :refer [satisfies-prop]]
+   [clojure.math.combinatorics :as combo]
    [clojure.spec.alpha :as s]
    [clojure.spec.test.alpha :as st]
    [clojure.test :as test :refer [are deftest is testing]]
@@ -3307,20 +3308,57 @@
     (with-system-data [{:blaze.db/keys [node]} config]
       [[[:put {:fhir/type :fhir/Patient :id "0"}]
         [:put {:fhir/type :fhir/Patient :id "1"}]
-        [:put {:fhir/type :fhir/Patient :id "2"}]]]
-
-      (let [clauses [["_id" "0" "1" "2"]]]
-        (given-type-query node "Patient" clauses
-          count := 3
-          [0 :id] := "0"
-          [1 :id] := "1"
-          [2 :id] := "2")
+        [:put {:fhir/type :fhir/Patient :id "2"}]
+        [:put {:fhir/type :fhir/Patient :id "3"}]
+        [:put {:fhir/type :fhir/Patient :id "4"}]
+        [:put {:fhir/type :fhir/Patient :id "5"}]]]
 
-        (testing "it is possible to start with the second patient"
-          (given (pull-type-query node "Patient" clauses "1")
-            count := 2
+      (doseq [ids (combo/permutations ["1" "3" "4"])]
+        (let [clauses [(into ["_id"] ids)]]
+          (given-type-query node "Patient" clauses
+            count := 3
             [0 :id] := "1"
-            [1 :id] := "2"))))))
+            [1 :id] := "3"
+            [2 :id] := "4")
+
+          (testing "it is possible to start with the second patient"
+            (given (pull-type-query node "Patient" clauses "3")
+              count := 2
+              [0 :id] := "3"
+              [1 :id] := "4"))
+
+          (testing "it is possible to start with the third patient"
+            (given (pull-type-query node "Patient" clauses "4")
+              count := 1
+              [0 :id] := "4"))
+
+          (testing "using a non-matching ID doesn't fail"
+            (testing "in the middle"
+              (given (pull-type-query node "Patient" clauses "2")
+                count := 0))
+
+            (testing "at the end"
+              (given (pull-type-query node "Patient" clauses "5")
+                count := 0))))))))
+
+(deftest ^:slow type-query-id-property-test
+  (log/set-min-level! :warn)
+
+  (testing "random id's"
+    (satisfies-prop 50
+      (prop/for-all [ids (gen/set (s/gen :blaze.resource/id) {:min-elements 1 :max-elements 1000})]
+        (with-system-data [{:blaze.db/keys [node]} config]
+          [(mapv #(vector :create {:fhir/type :fhir/Patient :id %}) ids)]
+
+          (let [db (d/db node)
+                ids (shuffle (take 100 ids))
+                clauses [(into ["_id"] ids)]
+                sorted-ids (sort ids)
+                start-id (rand-nth sorted-ids)]
+            (and (= (into [] (map :id) (d/type-query db "Patient" clauses))
+                    sorted-ids)
+                 (= (into [] (map :id) (d/type-query db "Patient" clauses start-id))
+                    (drop-while #(not= start-id %) sorted-ids)))))))))
 
 (deftest type-query-sort-test
   (testing "sorting by _id"
@@ -3449,7 +3487,10 @@
           (with-system-data [{:blaze.db/keys [node]} config]
             [(mapv #(vector :create {:fhir/type :fhir/Patient :id %}) ids)]
 
-            (= (sort ids) (mapv :id (pull-type-query node "Patient" [[:sort "_id" :asc]])))))))))
+            (let [db (d/db node)
+                  clauses [[:sort "_id" :asc]]]
+              (= (into [] (map :id) (d/type-query db "Patient" clauses))
+                 (sort ids)))))))))
 
 (deftest type-query-version-test
   (testing "only the latest version matches"