Fix Sorting by _lastUpdated

Closes: #904

Author: alexanderkiel

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

@@ -128,7 +128,10 @@
   (-sorted-index-handles
     [search-param batch-db tid direction]
     [search-param batch-db tid direction start-id]
-    "Returns a reducible collection of index handles sorted by the sort clause.")
+    "Returns a reducible collection of index handles sorted by the sort clause.
+
+    The `start-id` is only a performance hint. Search params are allowed to
+    return all index handles, even if a `start-id` is supplied.")
   (-supports-ordered-compartment-index-handles [search-param values]
     "Returns true if `search-param` supports fetching ordered compartment index handles with `values`.")
   (-ordered-compartment-index-handles

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

@@ -4,7 +4,6 @@
    [blaze.anomaly :as ba :refer [if-ok when-ok]]
    [blaze.async.comp :as ac :refer [do-sync]]
    [blaze.coll.core :as coll]
-   [blaze.db.impl.codec :as codec]
    [blaze.db.impl.index.index-handle :as ih]
    [blaze.db.impl.index.plan :as plan]
    [blaze.db.impl.protocols :as p]
@@ -261,10 +260,9 @@
       (comp (resource-handle-mapper batch-db tid clauses other-clauses)
             (distinct))
       (index-handles* batch-db tid (first first-clause) start-id))
-     (let [start-id (codec/id-string start-id)]
-       (coll/eduction
-        (drop-while #(not= start-id (:id %)))
-        (unordered-resource-handles batch-db tid clauses))))))
+     (coll/eduction
+      (u/drop-while-not-start-id start-id)
+      (unordered-resource-handles batch-db tid clauses)))))
 
 (defn type-query
   "Returns a reducible collection of resource handles from `batch-db` of type
@@ -273,7 +271,8 @@
   ([batch-db tid {:keys [sort-clause search-clauses]}]
    (if sort-clause
      (coll/eduction
-      (resource-handle-mapper batch-db tid search-clauses search-clauses)
+      (comp (resource-handle-mapper batch-db tid search-clauses search-clauses)
+            (distinct))
       (sorted-index-handles batch-db tid sort-clause))
      (let [[scan-clauses other-clauses] (type-query-plan* batch-db tid search-clauses)]
        (if (seq scan-clauses)
@@ -282,7 +281,9 @@
   ([batch-db tid {:keys [sort-clause search-clauses]} start-id]
    (if sort-clause
      (coll/eduction
-      (resource-handle-mapper batch-db tid search-clauses search-clauses)
+      (comp (resource-handle-mapper batch-db tid search-clauses search-clauses)
+            (distinct)
+            (u/drop-while-not-start-id start-id))
       (sorted-index-handles batch-db tid sort-clause start-id))
      (let [[scan-clauses other-clauses] (type-query-plan* batch-db tid search-clauses)]
        (if (seq scan-clauses)

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

@@ -78,6 +78,9 @@
   "Returns a reducible collection of index handles sorted by `search-param` in
   `direction`, starting with `start-id` (optional).
 
+  The `start-id` is only a performance hint. Search params are allowed to return
+  all index handles, even if a `start-id` is supplied.
+
   The index handles are not distinct and not ordered by id."
   ([search-param batch-db tid direction]
    (p/-sorted-index-handles search-param batch-db tid direction))

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

@@ -122,13 +122,12 @@
      (p/-index-handles search-param batch-db ref-tid modifier compiled-value)))
 
   (-index-handles [this batch-db tid modifier compiled-value start-id]
-    (let [start-id (codec/id-string start-id)]
-      (coll/eduction
-       (comp (u/resource-handle-xf batch-db tid)
-             (distinct)
-             (drop-while #(not= start-id (:id %)))
-             (map ih/from-resource-handle))
-       (p/-index-handles this batch-db tid modifier compiled-value))))
+    (coll/eduction
+     (comp (u/resource-handle-xf batch-db tid)
+           (distinct)
+           (u/drop-while-not-start-id start-id)
+           (map ih/from-resource-handle))
+     (p/-index-handles this batch-db tid modifier compiled-value)))
 
   (-supports-ordered-compartment-index-handles [_ _]
     false)

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

@@ -420,13 +420,9 @@
        (all-keys batch-db c-hash tid)
        (all-keys-prev batch-db c-hash tid))))
 
-  (-sorted-index-handles [_ batch-db tid direction start-id]
-    (coll/eduction
-     (comp drop-value
-           u/by-id-grouper)
-     (if (= :asc direction)
-       (all-keys batch-db c-hash tid start-id)
-       (all-keys-prev batch-db c-hash tid start-id))))
+  (-sorted-index-handles [search-param batch-db tid direction _start-id]
+   ;; starting with a particilar id isn't possible
+    (p/-sorted-index-handles search-param batch-db tid direction))
 
   (-supports-ordered-compartment-index-handles [_ _]
     false)

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

@@ -153,3 +153,7 @@
   (if (known? modifier)
     (unsupported-modifier-anom code modifier)
     (unknown-modifier-anom code modifier)))
+
+(defn drop-while-not-start-id [start-id]
+  (let [start-id (codec/id-string start-id)]
+    (drop-while #(not= start-id (:id %)))))

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

@@ -2378,22 +2378,25 @@
         [0 :fhir/type] := :fhir/Patient
         [0 :id] := "0")))
 
-  ;; TODO: fix this https://github.com/samply/blaze/issues/904
-  #_(testing "sorting by _lastUpdated returns only the newest version of the patient"
-      (with-system-data [{:blaze.db/keys [node]} (with-system-clock config)]
-        [[[:put {:fhir/type :fhir/Patient :id "0"}]]
-         [[:put {:fhir/type :fhir/Patient :id "1"}]]]
+  (testing "sorting by _lastUpdated returns every resource only once"
+    (with-system-data [{:blaze.db/keys [node]} system-clock-config]
+      [[[:put {:fhir/type :fhir/Patient :id "0"}]]
+       [[:put {:fhir/type :fhir/Patient :id "1"}]]]
 
-        ;; we have to sleep more than one second here because dates are index only with second resolution
-        (Thread/sleep 2000)
-        @(d/transact node [[:put {:fhir/type :fhir/Patient :id "0"}]])
+      ;; we have to sleep more than one second here because dates are index only with second resolution
+      (Thread/sleep 1100)
+      @(d/transact node [[:put {:fhir/type :fhir/Patient :id "0"}]])
 
-        (doseq [dir [:asc :desc]]
-          (given-type-query node "Patient" [[:sort "_lastUpdated" dir]]
-            count := 2
-            [0 :fhir/type] := :fhir/Patient
-            [0 :id] := "0"
-            [0 :active] := #fhir/boolean false))))
+      (let [clauses [[:sort "_lastUpdated" :desc]]]
+        (given-type-query node "Patient" clauses
+          count := 2
+          [0 :id] := "0"
+          [1 :id] := "1")
+
+        (testing "it is possible to start with the second patient"
+          (given (pull-type-query node "Patient" clauses "1")
+            count := 1
+            [0 :id] := "1")))))
 
   (testing "a node with three patients in one transaction"
     (with-system-data [{:blaze.db/keys [node]} config]
@@ -2402,18 +2405,16 @@
         [:put {:fhir/type :fhir/Patient :id "2" :active #fhir/boolean true}]]]
 
       (testing "two active patients will be found"
-        (given-type-query node "Patient" [["active" "true"]]
-          count := 2
-          [0 :fhir/type] := :fhir/Patient
-          [0 :id] := "0"
-          [1 :fhir/type] := :fhir/Patient
-          [1 :id] := "2"))
+        (let [clauses [["active" "true"]]]
+          (given-type-query node "Patient" clauses
+            count := 2
+            [0 :id] := "0"
+            [1 :id] := "2")
 
-      (testing "it is possible to start with the second patient"
-        (given (pull-type-query node "Patient" [["active" "true"]] "2")
-          count := 1
-          [0 :fhir/type] := :fhir/Patient
-          [0 :id] := "2"))))
+          (testing "it is possible to start with the second patient"
+            (given (pull-type-query node "Patient" clauses "2")
+              count := 1
+              [0 :id] := "2"))))))
 
   (testing "special case of _lastUpdated date search parameter"
     (testing "inequality searches do return every resource only once"
@@ -2422,7 +2423,7 @@
          [[:put {:fhir/type :fhir/Patient :id "1"}]]]
 
         ;; we have to sleep more than one second here because dates are index only with second resolution
-        (Thread/sleep 2000)
+        (Thread/sleep 1100)
         @(d/transact node [[:put {:fhir/type :fhir/Patient :id "0"}]])
 
         (given-type-query node "Patient" [["_lastUpdated" "ge2000-01-01"]]