diff --git a/cljfmt.edn b/cljfmt.edn index f90509483..a3d97da8b 100644 --- a/cljfmt.edn +++ b/cljfmt.edn @@ -6,6 +6,7 @@ if-ok [[:block 1]] try-one [[:block 2]] when-ok [[:block 1]] + with-open-coll [[:block 1]] do-sync [[:block 1]] do-async [[:block 1]] has-form [[:block 1]] diff --git a/modules/coll/.clj-kondo/config.edn b/modules/coll/.clj-kondo/config.edn index 78c4a3953..b2035b3a4 100644 --- a/modules/coll/.clj-kondo/config.edn +++ b/modules/coll/.clj-kondo/config.edn @@ -1,2 +1,5 @@ {:config-paths - ["../../../.clj-kondo/root"]} + ["../../../.clj-kondo/root"] + + :lint-as + {blaze.coll.core/with-open-coll clojure.core/with-open}} diff --git a/modules/coll/src/blaze/coll/core.clj b/modules/coll/src/blaze/coll/core.clj index 13553569d..5ca80e9fb 100644 --- a/modules/coll/src/blaze/coll/core.clj +++ b/modules/coll/src/blaze/coll/core.clj @@ -21,7 +21,7 @@ [coll] (identical? ::empty (reduce #(reduced %2) ::empty coll))) -(defn- inc-rf [sum _] (inc ^long sum)) +(defn inc-rf [sum _] (inc ^long sum)) (defn eduction "Like `clojure.core/eduction` but implements Counted instead of Iterable." @@ -67,4 +67,7 @@ IReduceInit (reduce [_ rf# init#] (with-open ~bindings - (reduce rf# init# ~coll))))) + (reduce rf# init# ~coll))) + Counted + (count [coll#] + (.reduce coll# inc-rf 0)))) diff --git a/modules/coll/test/blaze/coll/core_test.clj b/modules/coll/test/blaze/coll/core_test.clj index f78058ea4..3f7c64e34 100644 --- a/modules/coll/test/blaze/coll/core_test.clj +++ b/modules/coll/test/blaze/coll/core_test.clj @@ -1,11 +1,13 @@ (ns blaze.coll.core-test (:require - [blaze.coll.core :as coll] + [blaze.coll.core :as coll :refer [with-open-coll]] [blaze.test-util :as tu] [clojure.spec.test.alpha :as st] - [clojure.test :as test :refer [are deftest is testing]])) + [clojure.test :as test :refer [are deftest is testing]]) + (:import [java.lang AutoCloseable])) (st/instrument) +(set! *warn-on-reflection* true) (test/use-fixtures :each tu/fixture) @@ -93,3 +95,11 @@ [::x ::y] 0 ::x [::x ::y] 1 ::y [::x ::y] 2 ::not-found))) + +(deftest with-open-coll-test + (let [state (volatile! false) + coll (with-open-coll [_ (reify AutoCloseable (close [_] (vreset! state true)))] + (coll/eduction (map inc) (range 10)))] + (is (= 10 (count coll))) + (is (= (range 1 11) (vec coll))) + (is (true? @state)))) diff --git a/modules/cql/.clj-kondo/config.edn b/modules/cql/.clj-kondo/config.edn index ca7000645..054ef5f7b 100644 --- a/modules/cql/.clj-kondo/config.edn +++ b/modules/cql/.clj-kondo/config.edn @@ -8,8 +8,7 @@ "../../module-test-util/resources/clj-kondo.exports/blaze/module-test-util"] :lint-as - {blaze.db.impl.macros/with-open-coll clojure.core/with-open - blaze.elm.compiler.macros/defunop clojure.core/defn + {blaze.elm.compiler.macros/defunop clojure.core/defn blaze.elm.compiler.macros/defbinop clojure.core/defn blaze.elm.compiler.macros/defternop clojure.core/defn blaze.elm.compiler.macros/defnaryop clojure.core/defn diff --git a/modules/db-tx-log/src/blaze/db/tx_log/spec.clj b/modules/db-tx-log/src/blaze/db/tx_log/spec.clj index b919ac791..971618238 100644 --- a/modules/db-tx-log/src/blaze/db/tx_log/spec.clj +++ b/modules/db-tx-log/src/blaze/db/tx_log/spec.clj @@ -19,7 +19,7 @@ queue?) (s/def :blaze.db.tx-cmd/op - #{"create" "put" "keep" "delete" "conditional-delete"}) + #{"create" "put" "keep" "delete" "conditional-delete" "delete-history"}) (s/def :blaze.db.tx-cmd/type :fhir.resource/type) @@ -87,6 +87,11 @@ :blaze.db.tx-cmd/check-refs :blaze.db.tx-cmd/allow-multiple])) +(defmethod tx-cmd "delete-history" [_] + (s/keys :req-un [:blaze.db.tx-cmd/op + :blaze.db.tx-cmd/type + :blaze.resource/id])) + (s/def :blaze.db/tx-cmd (s/multi-spec tx-cmd :op)) diff --git a/modules/db-tx-log/test/blaze/db/tx_log/spec_test.clj b/modules/db-tx-log/test/blaze/db/tx_log/spec_test.clj index 792398a91..5d53f9f2e 100644 --- a/modules/db-tx-log/test/blaze/db/tx_log/spec_test.clj +++ b/modules/db-tx-log/test/blaze/db/tx_log/spec_test.clj @@ -11,7 +11,7 @@ [taoensso.timbre :as log])) (st/instrument) -(log/set-level! :trace) +(log/set-min-level! :trace) (test/use-fixtures :each tu/fixture) @@ -36,11 +36,19 @@ :id "0" :hash observation-hash-0 :refs [["Patient" "0"]]} + {:op "put" + :type "Patient" + :id "0" + :hash patient-hash-0} {:op "put" :type "Patient" :id "0" :hash patient-hash-0 :if-match 1} + {:op "keep" + :type "Patient" + :id "0" + :hash patient-hash-0} {:op "delete" :type "Patient" :id "0" @@ -48,11 +56,29 @@ {:op "delete" :type "Patient" :id "0" - :check-refs true})) + :check-refs true} + {:op "conditional-delete" + :type "Patient"} + {:op "delete-history" + :type "Patient" + :id "0"})) (testing "invalid" (are [tx-cmd] (not (s/valid? :blaze.db/tx-cmd tx-cmd)) + nil + 1 + {:op "create" + :type "Patient" + :id "0"} + {:op "put" + :type "Patient" + :id "0"} + {:op "delete" + :type "Patient"} {:op "delete" :type "Patient" :id "0" - :check-refs "i should be a boolean"}))) + :check-refs "i should be a boolean"} + {:op "conditional-delete"} + {:op "delete-history" + :type "Patient"}))) diff --git a/modules/db/src/blaze/db/api_spec.clj b/modules/db/src/blaze/db/api_spec.clj index 304f704c9..c993a507e 100644 --- a/modules/db/src/blaze/db/api_spec.clj +++ b/modules/db/src/blaze/db/api_spec.clj @@ -174,13 +174,6 @@ :start-t (s/? (s/nilable :blaze.db/t))) :ret (cs/coll-of :blaze.db/resource-handle)) -(s/fdef d/total-num-of-instance-changes - :args (s/cat :db :blaze.db/db - :type :fhir.resource/type - :id :blaze.resource/id - :since (s/? (s/nilable inst?))) - :ret nat-int?) - ;; ---- Type-Level History Functions ------------------------------------------ (s/fdef d/type-history diff --git a/modules/db/src/blaze/db/impl/batch_db.clj b/modules/db/src/blaze/db/impl/batch_db.clj index 4c1b926ce..7c5500a1b 100644 --- a/modules/db/src/blaze/db/impl/batch_db.clj +++ b/modules/db/src/blaze/db/impl/batch_db.clj @@ -146,7 +146,7 @@ (-instance-history [_ tid id start-t] (let [start-t (if (some-> start-t (<= t)) start-t t)] - (rao/instance-history snapshot tid id start-t))) + (rao/instance-history snapshot tid id t start-t))) (-total-num-of-instance-changes [_ tid id since] (let [end-t (or (some->> since (t-by-instant/t-by-instant snapshot)) 0)] @@ -156,7 +156,7 @@ (-type-history [_ tid start-t start-id] (let [start-t (if (some-> start-t (<= t)) start-t t)] - (tao/type-history snapshot tid start-t start-id))) + (tao/type-history snapshot tid t start-t start-id))) (-total-num-of-type-changes [_ type since] (let [tid (codec/tid type) @@ -168,7 +168,7 @@ (-system-history [_ start-t start-tid start-id] (let [start-t (if (some-> start-t (<= t)) start-t t)] - (sao/system-history snapshot start-t start-tid start-id))) + (sao/system-history snapshot t start-t start-tid start-id))) (-total-num-of-system-changes [_ since] (let [end-t (some->> since (t-by-instant/t-by-instant snapshot))] diff --git a/modules/db/src/blaze/db/impl/index/resource_as_of.clj b/modules/db/src/blaze/db/impl/index/resource_as_of.clj index 820e14d99..dbd266bd9 100644 --- a/modules/db/src/blaze/db/impl/index/resource_as_of.clj +++ b/modules/db/src/blaze/db/impl/index/resource_as_of.clj @@ -21,9 +21,12 @@ (def ^:private ^:const ^long max-key-size (+ except-id-key-size codec/max-id-size)) -(def ^:const ^long value-size +(def ^:const ^long min-value-size (+ hash/size codec/state-size)) +(def ^:const ^long max-value-size + (+ hash/size codec/state-size codec/t-size)) + (defn- focus-id! "Reduces the limit of `kb` in order to hide the t and focus on id solely." [kb] @@ -295,15 +298,16 @@ (rh/resource-handle! tid id (codec/descending-long (bb/get-long! kb tid-id-size)) vb))) (defn instance-history - "Returns a reducible collection of all versions of the resource with `tid` and - `id` starting at `start-t`. - - Versions are resource handles." - [snapshot tid id start-t] + "Returns a reducible collection of all historic resource handles of the + resource with `tid` and `id` of the database with the point in time `t` + starting at `start-t`." + [snapshot tid id t start-t] (let [tid-id-size (+ codec/tid-size (bs/size id))] - (i/prefix-entries snapshot :resource-as-of-index - (map (decoder tid (codec/id-string id) tid-id-size)) - tid-id-size (start-key tid id start-t)))) + (i/prefix-entries + snapshot :resource-as-of-index + (comp (map (decoder tid (codec/id-string id) tid-id-size)) + (take-while #(< (long t) (rh/purged-at %)))) + tid-id-size (start-key tid id start-t)))) (defn- resource-handle* [iter target-buf key-buf value-buf tid id t] (let [tid-id-size (+ codec/tid-size (bs/size id)) @@ -347,7 +351,7 @@ [snapshot tid id t] (let [target-buf (bb/allocate max-key-size) key-buf (bb/allocate max-key-size) - value-buf (bb/allocate value-size)] + value-buf (bb/allocate max-value-size)] (with-open [iter (kv/new-iterator snapshot :resource-as-of-index)] (resource-handle* iter target-buf key-buf value-buf tid id t)))) @@ -369,7 +373,7 @@ [snapshot t] (let [target-buf (bb/allocate max-key-size) key-buf (bb/allocate max-key-size) - value-buf (bb/allocate value-size) + value-buf (bb/allocate max-value-size) iter (kv/new-iterator snapshot :resource-as-of-index)] (comp (keep @@ -389,7 +393,7 @@ ([snapshot t tid id-extractor matcher] (let [target-buf (bb/allocate max-key-size) key-buf (bb/allocate max-key-size) - value-buf (bb/allocate value-size) + value-buf (bb/allocate max-value-size) iter (kv/new-iterator snapshot :resource-as-of-index)] (comp (keep diff --git a/modules/db/src/blaze/db/impl/index/resource_handle.clj b/modules/db/src/blaze/db/impl/index/resource_handle.clj index c78fa9974..1de20b776 100644 --- a/modules/db/src/blaze/db/impl/index/resource_handle.clj +++ b/modules/db/src/blaze/db/impl/index/resource_handle.clj @@ -11,7 +11,7 @@ (set! *warn-on-reflection* true) (set! *unchecked-math* :warn-on-boxed) -(deftype ResourceHandle [^int tid id ^long t hash ^long num-changes op] +(deftype ResourceHandle [^int tid id ^long t hash ^long num-changes op ^long purged-at] p/FhirType (-type [_] ;; TODO: maybe cache this @@ -29,6 +29,7 @@ :hash hash :num-changes num-changes :op op + :purged-at purged-at not-found)) Object @@ -61,6 +62,11 @@ :delete :put))) +(defn- get-purged-at! [vb] + (if (<= 8 (bb/remaining vb)) + (bb/get-long! vb) + Long/MAX_VALUE)) + (defn resource-handle! "Creates a new resource handle. @@ -74,7 +80,8 @@ t hash (state->num-changes state) - (state->op state)))) + (state->op state) + (get-purged-at! vb)))) (defn resource-handle? [x] (instance? ResourceHandle x)) @@ -116,6 +123,11 @@ [rh] (.-op ^ResourceHandle rh)) +(defn purged-at + {:inline (fn [rh] `(.-purged-at ~(with-meta rh {:tag `ResourceHandle})))} + [rh] + (.-purged-at ^ResourceHandle rh)) + (defn reference [rh] (str (codec/tid->type (tid rh)) "/" (id rh))) diff --git a/modules/db/src/blaze/db/impl/index/rts_as_of.clj b/modules/db/src/blaze/db/impl/index/rts_as_of.clj index 22e0bf3c9..0d4677c90 100644 --- a/modules/db/src/blaze/db/impl/index/rts_as_of.clj +++ b/modules/db/src/blaze/db/impl/index/rts_as_of.clj @@ -21,14 +21,27 @@ (identical? :create op) (Numbers/setBit 1) (identical? :delete op) (Numbers/setBit 0))) -(defn encode-value [hash num-changes op] - (-> (bb/allocate rao/value-size) - (hash/into-byte-buffer! hash) - (bb/put-long! (state num-changes op)) - bb/array)) +(defn- encode-value + ([hash num-changes op] + (-> (bb/allocate rao/min-value-size) + (hash/into-byte-buffer! hash) + (bb/put-long! (state num-changes op)) + bb/array)) + ([hash num-changes op purged-at] + (-> (bb/allocate rao/max-value-size) + (hash/into-byte-buffer! hash) + (bb/put-long! (state num-changes op)) + (bb/put-long! purged-at) + bb/array))) -(defn index-entries [tid id t hash num-changes op] - (let [value (encode-value hash num-changes op)] - [[:resource-as-of-index (rao/encode-key tid id t) value] - [:type-as-of-index (tao/encode-key tid t id) value] - [:system-as-of-index (sao/encode-key t tid id) value]])) +(defn index-entries + ([tid id t hash num-changes op] + (let [value (encode-value hash num-changes op)] + [[:resource-as-of-index (rao/encode-key tid id t) value] + [:type-as-of-index (tao/encode-key tid t id) value] + [:system-as-of-index (sao/encode-key t tid id) value]])) + ([tid id t hash num-changes op purged-at] + (let [value (encode-value hash num-changes op purged-at)] + [[:resource-as-of-index (rao/encode-key tid id t) value] + [:type-as-of-index (tao/encode-key tid t id) value] + [:system-as-of-index (sao/encode-key t tid id) value]]))) diff --git a/modules/db/src/blaze/db/impl/index/system_as_of.clj b/modules/db/src/blaze/db/impl/index/system_as_of.clj index 9774836eb..4898094d0 100644 --- a/modules/db/src/blaze/db/impl/index/system_as_of.clj +++ b/modules/db/src/blaze/db/impl/index/system_as_of.clj @@ -63,14 +63,15 @@ (Longs/toByteArray (codec/descending-long ^long start-t)))) (defn system-history - "Returns a reducible collection of all versions between `start-t` (inclusive), - `start-tid` (optional, inclusive) and `start-id` (optional, inclusive) of all - resources. - - Versions are resource handles." - [snapshot start-t start-tid start-id] - (i/entries snapshot :system-as-of-index (map (decoder)) - (bs/from-byte-array (start-key start-t start-tid start-id)))) + "Returns a reducible collection of all historic resource handles of the + database with the point in time `t` between `start-t` (inclusive), `start-tid` + (optional, inclusive) and `start-id` (optional, inclusive)." + [snapshot t start-t start-tid start-id] + (i/entries + snapshot :system-as-of-index + (comp (map (decoder)) + (filter #(< (long t) (rh/purged-at %)))) + (bs/from-byte-array (start-key start-t start-tid start-id)))) (defn changes "Returns a reducible collection of all resource handles changed at `t`." diff --git a/modules/db/src/blaze/db/impl/index/type_as_of.clj b/modules/db/src/blaze/db/impl/index/type_as_of.clj index 3066143c7..ea23f811a 100644 --- a/modules/db/src/blaze/db/impl/index/type_as_of.clj +++ b/modules/db/src/blaze/db/impl/index/type_as_of.clj @@ -54,9 +54,12 @@ bs/from-byte-buffer!))) (defn type-history - "Returns a reducible collection of all historic resource handles between - `start-t` (inclusive) and `start-id` (optional, inclusive) of resources with - `tid`." - [snapshot tid start-t start-id] - (i/prefix-entries snapshot :type-as-of-index (map (decoder tid)) - codec/tid-size (start-key tid start-t start-id))) + "Returns a reducible collection of all historic resource handles with type + `tid` of the database with the point in time `t` between `start-t` (inclusive) + and `start-id` (optional, inclusive)." + [snapshot tid t start-t start-id] + (i/prefix-entries + snapshot :type-as-of-index + (comp (map (decoder tid)) + (filter #(< (long t) (rh/purged-at %)))) + codec/tid-size (start-key tid start-t start-id))) diff --git a/modules/db/src/blaze/db/node/transaction.clj b/modules/db/src/blaze/db/node/transaction.clj index e1acda683..c16437273 100644 --- a/modules/db/src/blaze/db/node/transaction.clj +++ b/modules/db/src/blaze/db/node/transaction.clj @@ -87,6 +87,13 @@ allow-multiple-delete (assoc :allow-multiple true))}) +(defmethod prepare-op :delete-history + [_ [_ type id]] + {:blaze.db/tx-cmd + {:op "delete-history" + :type type + :id id}}) + (def ^:private split (juxt #(mapv :blaze.db/tx-cmd %) #(into {} (map :hash-resource) %))) diff --git a/modules/db/src/blaze/db/node/tx_indexer/verify.clj b/modules/db/src/blaze/db/node/tx_indexer/verify.clj index 1f3e18d87..72f3ea93d 100644 --- a/modules/db/src/blaze/db/node/tx_indexer/verify.clj +++ b/modules/db/src/blaze/db/node/tx_indexer/verify.clj @@ -207,6 +207,7 @@ refs))) (def ^:private inc-0 (fnil inc 0)) +(def ^:private minus-0 (fnil - 0)) (defmethod verify-tx-cmd "create" [_search-param-registry db-before t res {:keys [type id hash refs]}] @@ -327,6 +328,45 @@ (and op (not (identical? :delete op))) (update-in [:stats tid :total] (fnil dec 0)))))) +(def ^:private ^:const ^long delete-history-max 100000) + +(defn- too-many-history-entries-msg [type id] + (format "Deleting the history of `%s/%s` failed because there are more than %,d history entries." + type id delete-history-max)) + +(defn- too-many-history-entries-anom [type id] + (ba/conflict + (too-many-history-entries-msg type id) + :fhir/issue "too-costly")) + +(defn- instance-history [db type id] + (into [] (comp (drop 1) (take delete-history-max)) (d/instance-history db type id))) + +(defn- purge-entry [tid id t rh] + (rts/index-entries tid id (rh/t rh) (rh/hash rh) (rh/num-changes rh) (rh/op rh) t)) + +(defn- purge-entries [tid id t instance-history] + (into [] (mapcat (partial purge-entry tid id t)) instance-history)) + +(defn- add-delete-history-entries [entries tid id t instance-history] + (-> (update entries :entries into (purge-entries tid (codec/id-byte-string id) t instance-history)) + (update-in [:stats tid :num-changes] minus-0 (count instance-history)))) + +(defmethod verify-tx-cmd "delete-history" + [_ db-before t res {:keys [type id]}] + (log/trace "verify-tx-cmd :delete-history" (str type "/" id)) + (with-open [_ (prom/timer duration-seconds "verify-delete-history")] + (let [tid (codec/tid type) + instance-history (instance-history db-before type id)] + (cond + (empty? instance-history) res + + (= delete-history-max (count instance-history)) + (throw-anom (too-many-history-entries-anom type id)) + + :else + (add-delete-history-entries res tid id t instance-history))))) + (defmethod verify-tx-cmd :default [_search-param-registry _db-before _t res _tx-cmd] res) diff --git a/modules/db/src/blaze/db/node/validation.clj b/modules/db/src/blaze/db/node/validation.clj index d5514a8df..87d1d215c 100644 --- a/modules/db/src/blaze/db/node/validation.clj +++ b/modules/db/src/blaze/db/node/validation.clj @@ -23,6 +23,8 @@ (defmethod extract-type-id :conditional-delete [_]) +(defmethod extract-type-id :delete-history [_]) + (defn- duplicate-resource-anomaly [[type id]] (ba/incorrect (format "Duplicate resource `%s/%s`." type id) diff --git a/modules/db/src/blaze/db/spec.clj b/modules/db/src/blaze/db/spec.clj index d565aaf96..a04f84332 100644 --- a/modules/db/src/blaze/db/spec.clj +++ b/modules/db/src/blaze/db/spec.clj @@ -96,6 +96,11 @@ :type :fhir.resource/type :clauses (s/? (s/coll-of :blaze.db.query/search-clause :kind vector? :min-count 1)))) +(defmethod tx-op :delete-history [_] + (s/cat :op #{:delete-history} + :type :fhir.resource/type + :id :blaze.resource/id)) + (s/def :blaze.db/tx-op (s/multi-spec tx-op first)) diff --git a/modules/db/test/blaze/db/api_test.clj b/modules/db/test/blaze/db/api_test.clj index 2bd0b205b..bc3deeeac 100644 --- a/modules/db/test/blaze/db/api_test.clj +++ b/modules/db/test/blaze/db/api_test.clj @@ -203,13 +203,16 @@ medication-administrations medication-administration-gen] (concat observations encounters procedures medication-administrations)))) +(defn- pull-resource [db type id] + (d/pull db (d/resource-handle db type id))) + (deftest transact-create-test (testing "one Patient" (with-system [{:blaze.db/keys [node]} config] (given @(mtu/assoc-thread-name (d/transact node [[:create {:fhir/type :fhir/Patient :id "0"}]])) [meta :thread-name] :? mtu/common-pool-thread?) - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" @@ -224,13 +227,13 @@ :subject #fhir/Reference{:reference "Patient/0"}}] [:create {:fhir/type :fhir/Patient :id "0"}]]] - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" [meta :blaze.db/op] := :create) - (given @(d/pull node (d/resource-handle (d/db node) "Observation" "0")) + (given @(pull-resource (d/db node) "Observation" "0") :fhir/type := :fhir/Observation :id := "0" [:subject :reference] := "Patient/0" @@ -263,7 +266,7 @@ {:fhir/type :fhir/Patient :id "0"} [["identifier" "111033"]]]])] (testing "the Patient was created" - (given @(d/pull node (d/resource-handle db "Patient" "0")) + (given @(pull-resource db "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" @@ -278,7 +281,7 @@ {:fhir/type :fhir/Patient :id "1"} [["identifier" "111033"]]]])] (testing "the Patient was created" - (given @(d/pull node (d/resource-handle db "Patient" "1")) + (given @(pull-resource db "Patient" "1") :fhir/type := :fhir/Patient :id := "1" [:meta :versionId] := #fhir/id"2" @@ -345,7 +348,7 @@ [[[:put {:fhir/type :fhir/Patient :id "0"}]]] (testing "the Patient was created" - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" @@ -360,7 +363,7 @@ :value "2022"}}]]] (testing "the Patient was created" - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" @@ -378,14 +381,14 @@ [:put {:fhir/type :fhir/Patient :id "0"}]]] (testing "the Patient was created" - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" [meta :blaze.db/op] := :put)) (testing "the Observation was created" - (given @(d/pull node (d/resource-handle (d/db node) "Observation" "0")) + (given @(pull-resource (d/db node) "Observation" "0") :fhir/type := :fhir/Observation :id := "0" [:meta :versionId] := #fhir/id"1" @@ -397,7 +400,7 @@ [[[:put {:fhir/type :fhir/Patient :id "0" :gender #fhir/code"male"}]] [[:put {:fhir/type :fhir/Patient :id "0" :gender #fhir/code"female"}]]] - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"2" @@ -433,7 +436,7 @@ [[:put {:fhir/type :fhir/Patient :id "0" :gender #fhir/code"female"}]]] (testing "versionId is still 1" - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" @@ -457,27 +460,27 @@ :subject #fhir/Reference{:reference "Patient/0"}}] [:put {:fhir/type :fhir/Patient :id "0"}]]] - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" [meta :blaze.db/op] := :put) - (given @(d/pull node (d/resource-handle (d/db node) "Observation" "0")) + (given @(pull-resource (d/db node) "Observation" "0") :fhir/type := :fhir/Observation :id := "0" [:meta :versionId] := #fhir/id"1" [:subject :reference] := "Patient/0" [meta :blaze.db/op] := :put) - (given @(d/pull node (d/resource-handle (d/db node) "Observation" "1")) + (given @(pull-resource (d/db node) "Observation" "1") :fhir/type := :fhir/Observation :id := "1" [:meta :versionId] := #fhir/id"1" [:subject :reference] := "Patient/0" [meta :blaze.db/op] := :put) - (given @(d/pull node (d/resource-handle (d/db node) "List" "0")) + (given @(pull-resource (d/db node) "List" "0") :fhir/type := :fhir/List :id := "0" [:meta :versionId] := #fhir/id"1" @@ -794,6 +797,225 @@ ::anom/message := "Conditional delete of all Patients failed because more than 10,000 matches were found." :fhir/issue := "too-costly"))))) +(defn- pull-instance-history + ([db type id] + (d/pull-many db (d/instance-history db type id))) + ([db type id start-t] + (d/pull-many db (d/instance-history db type id start-t)))) + +(deftest transact-delete-history-test + (testing "one patient with one version" + (with-system-data [{:blaze.db/keys [node]} config] + [[[:create {:fhir/type :fhir/Patient :id "0"}]]] + + (let [db-before (d/db node) + db-after @(d/transact node [[:delete-history "Patient" "0"]])] + + (testing "nothing changed" + (testing "the patient is the same" + (is (= (d/resource-handle db-before "Patient" "0") + (d/resource-handle db-after "Patient" "0")))) + + (testing "the instance histories are the same" + (is (= (vec (d/instance-history db-before "Patient" "0")) + (vec (d/instance-history db-after "Patient" "0"))))) + + (testing "the type histories are the same" + (is (= (vec (d/type-history db-before "Patient")) + (vec (d/type-history db-after "Patient"))))) + + (testing "the system histories are the same" + (is (= (vec (d/system-history db-before)) + (vec (d/system-history db-after))))))))) + + (testing "one patient with two versions" + (with-system-data [{:blaze.db/keys [node]} config] + [[[:create {:fhir/type :fhir/Patient :id "0" :active false}]] + [[:put {:fhir/type :fhir/Patient :id "0" :active true}]]] + + (let [db-before (d/db node) + db-after @(d/transact node [[:delete-history "Patient" "0"]])] + + (testing "the patient" + (let [patient @(pull-resource db-after "Patient" "0")] + + (testing "is active" + (given patient + :active := true)) + + (testing "the instance history contains only that patient" + (is (= [patient] @(pull-instance-history db-after "Patient" "0")))))) + + (testing "the type history contains only one entry" + (is (= 1 (d/total-num-of-type-changes db-after "Patient"))) + (given @(d/pull-many node (d/type-history db-after "Patient")) + count := 1 + [0 :active] := true)) + + (testing "the system history contains only one entry" + (is (= 1 (d/total-num-of-system-changes db-after))) + (given @(d/pull-many node (d/system-history db-after)) + count := 1 + [0 :active] := true)) + + (testing "the instance history of db-before still contains two entries" + (given @(pull-instance-history db-before "Patient" "0") + count := 2 + [0 :active] := true + [1 :active] := false)) + + (testing "the type history of db-before still contains two entries" + (is (= 2 (d/total-num-of-type-changes db-before "Patient"))) + (given @(d/pull-many node (d/type-history db-before "Patient")) + count := 2 + [0 :active] := true + [1 :active] := false)) + + (testing "the system history of db-before still contains two entries" + (is (= 2 (d/total-num-of-system-changes db-before))) + (given @(d/pull-many node (d/system-history db-before)) + count := 2 + [0 :active] := true + [1 :active] := false))))) + + (testing "one deleted patient" + (with-system-data [{:blaze.db/keys [node]} config] + [[[:create {:fhir/type :fhir/Patient :id "0"}]] + [[:delete "Patient" "0"]]] + + (let [db-before (d/db node) + db-after @(d/transact node [[:delete-history "Patient" "0"]])] + + (testing "the patient is still deleted" + (given (d/resource-handle db-after "Patient" "0") + :op := :delete)) + + (testing "the instance history contains only one entry" + (given (vec (d/instance-history db-after "Patient" "0")) + count := 1 + [0 :op] := :delete)) + + (testing "the type history contains only one entry" + (is (= 1 (d/total-num-of-type-changes db-after "Patient"))) + (given (vec (d/type-history db-after "Patient")) + count := 1 + [0 :op] := :delete)) + + (testing "the system history contains only one entry" + (is (= 1 (d/total-num-of-system-changes db-after))) + (given (vec (d/system-history db-after)) + count := 1 + [0 :op] := :delete)) + + (testing "the instance history of db-before still contains two entries" + (given (vec (d/instance-history db-before "Patient" "0")) + count := 2 + [0 :op] := :delete + [1 :op] := :create)) + + (testing "the type history of db-before still contains two entries" + (is (= 2 (d/total-num-of-type-changes db-before "Patient"))) + (given (vec (d/type-history db-before "Patient")) + count := 2 + [0 :op] := :delete + [1 :op] := :create)) + + (testing "the system history of db-before still contains two entries" + (is (= 2 (d/total-num-of-system-changes db-before))) + (given (vec (d/system-history db-before)) + count := 2 + [0 :op] := :delete + [1 :op] := :create))))) + + (testing "adding a new version on top of a deleted history" + (with-system-data [{:blaze.db/keys [node]} config] + [[[:create {:fhir/type :fhir/Patient :id "0" :active false}]] + [[:put {:fhir/type :fhir/Patient :id "0" :active true}]] + [[:delete-history "Patient" "0"]]] + + (let [db-before (d/db node) + db-after @(d/transact node [[:put {:fhir/type :fhir/Patient :id "0" + :gender #fhir/code"male"}]])] + + (testing "the patient is male" + (given @(pull-resource db-after "Patient" "0") + :gender := #fhir/code"male")) + + (testing "the instance history contains two entries" + (given @(pull-instance-history db-after "Patient" "0") + count := 2 + [0 :gender] := #fhir/code"male" + [1 :active] := true)) + + (testing "the type history contains two entries" + (is (= 2 (d/total-num-of-type-changes db-after "Patient"))) + (given @(d/pull-many node (d/type-history db-after "Patient")) + count := 2 + [0 :gender] := #fhir/code"male" + [1 :active] := true)) + + (testing "the system history contains two entries" + (is (= 2 (d/total-num-of-system-changes db-after))) + (given @(d/pull-many node (d/system-history db-after)) + count := 2 + [0 :gender] := #fhir/code"male" + [1 :active] := true)) + + (testing "the instance history of db-before still contains only one entry" + (given @(pull-instance-history db-before "Patient" "0") + count := 1 + [0 :active] := true)) + + (testing "the type history of db-before still contains only one entry" + (is (= 1 (d/total-num-of-type-changes db-before "Patient"))) + (given @(d/pull-many node (d/type-history db-before "Patient")) + count := 1 + [0 :active] := true)) + + (testing "the system history of db-before still contains only one entry" + (is (= 1 (d/total-num-of-system-changes db-before))) + (given @(d/pull-many node (d/system-history db-before)) + count := 1 + [0 :active] := true)))))) + +(deftest transact-delete-history-too-many-test + (log/set-min-level! :info) + (st/unstrument) + (testing "works with up to 100,000 history entries" + (with-system-data [{:blaze.db/keys [node]} config] + (vec (for [_ (range 50000) + active [true false]] + [[:put {:fhir/type :fhir/Patient :id "0" :active active}]])) + + (let [db-before (d/db node) + db-after @(d/transact node [[:delete-history "Patient" "0"]])] + + (testing "the patient" + (let [patient @(pull-resource db-after "Patient" "0")] + + (testing "is not active" + (given patient + :active := false)) + + (testing "the instance history contains only that patient" + (is (= [patient] @(pull-instance-history db-after "Patient" "0")))))) + + (testing "the instance history of db-before still contains 100,000 entries" + (is (= 100000 (count (d/instance-history db-before "Patient" "0")))))))) + + (testing "fails on more then 100,000 history entries" + (with-system-data [{:blaze.db/keys [node]} config] + (into + [[[:put {:fhir/type :fhir/Patient :id "0" :active false}]]] + (for [_ (range 50000) + active [true false]] + [[:put {:fhir/type :fhir/Patient :id "0" :active active}]])) + + (given-failed-future (d/transact node [[:delete-history "Patient" "0"]]) + ::anom/category := ::anom/conflict + ::anom/message := "Deleting the history of `Patient/0` failed because there are more than 100,000 history entries." + :fhir/issue := "too-costly")))) + (deftest transact-test (testing "a transaction with duplicate resources fails" (testing "two puts" @@ -841,7 +1063,7 @@ :id := "1")) (testing "the first patient is still active" - (given @(d/pull node (d/resource-handle db "Patient" "0")) + (given @(pull-resource db "Patient" "0") :id := "0" :active := true))))) @@ -896,7 +1118,7 @@ {:fhir/type :fhir/Observation :id "0" :subject #fhir/Reference{:reference "Patient/0"}}]]] - (given @(d/pull node (d/resource-handle (d/db node) "Observation" "0")) + (given @(pull-resource (d/db node) "Observation" "0") :fhir/type := :fhir/Observation :id := "0" [:meta :versionId] := #fhir/id"1" @@ -1073,7 +1295,7 @@ [[[:create {:fhir/type :fhir/Patient :id "0"}]]] (testing "pull" - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" @@ -1093,7 +1315,7 @@ (with-system-data [{:blaze.db/keys [node]} config] [[[:put {:fhir/type :fhir/Patient :id "0"}]]] - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"1" @@ -1105,7 +1327,7 @@ [[[:put {:fhir/type :fhir/Patient :id "0"}]] [[:delete "Patient" "0"]]] - (given @(d/pull node (d/resource-handle (d/db node) "Patient" "0")) + (given @(pull-resource (d/db node) "Patient" "0") :fhir/type := :fhir/Patient :id := "0" [:meta :versionId] := #fhir/id"2" @@ -1152,7 +1374,7 @@ [[:put {:fhir/type :fhir/Patient :id "0"}]]] (testing "has one list entry" - (is (= 1 (count (vec (d/type-list (d/db node) "Patient"))))) + (is (= 1 (count (d/type-list (d/db node) "Patient")))) (is (= 1 (d/type-total (d/db node) "Patient")))))) (testing "a node with two patients in two transactions" @@ -1233,11 +1455,11 @@ [[:put {:fhir/type :fhir/Observation :id "0"}]]] (testing "has one patient list entry" - (is (= 1 (count (vec (d/type-list (d/db node) "Patient"))))) + (is (= 1 (count (d/type-list (d/db node) "Patient")))) (is (= 1 (d/type-total (d/db node) "Patient")))) (testing "has one observation list entry" - (is (= 1 (count (vec (d/type-list (d/db node) "Observation"))))) + (is (= 1 (count (d/type-list (d/db node) "Observation")))) (is (= 1 (d/type-total (d/db node) "Observation")))))) (testing "the database is immutable" @@ -4382,7 +4604,7 @@ :end #fhir/dateTime"2001-07"}}]]] (let [db (d/db node) - num-encounter #(count (vec (d/type-query db "Encounter" %)))] + num-encounter #(count (d/type-query db "Encounter" %))] (are [year n] (= n (num-encounter [["date" (format "gt%d-01-01" year)] ["date" (format "lt%d-01-01" (inc year))]])) 1999 2 @@ -4428,7 +4650,7 @@ [tx-ops] (let [db (d/db node) - num-encounter #(count (vec (d/type-query db "Encounter" %)))] + num-encounter #(count (d/type-query db "Encounter" %))] (= (num-encounter [["date" (str year)]]) (num-encounter [["date" (format "sa%d-12-31" (dec year))] ["date" (format "eb%d-01-01" (inc year))]]))))))) @@ -4441,7 +4663,7 @@ [tx-ops] (let [db (d/db node) - num-encounter #(count (vec (d/type-query db "Encounter" %)))] + num-encounter #(count (d/type-query db "Encounter" %))] (= (num-encounter [["date" (str "ap" year)]]) (num-encounter [["date" (format "ge%d-01-01" year)] ["date" (format "lt%d-01-01" (inc year))]]))))))) @@ -5826,36 +6048,28 @@ (deftest instance-history-test (testing "a new node has an empty instance history" (with-system [{:blaze.db/keys [node]} config] - (is (coll/empty? (d/instance-history (d/db node) "Patient" "0"))) - (is (zero? (d/total-num-of-instance-changes (d/db node) "Patient" "0"))))) + (is (coll/empty? (d/instance-history (d/db node) "Patient" "0"))))) (testing "a node with one patient" (with-system-data [{:blaze.db/keys [node]} config] [[[:put {:fhir/type :fhir/Patient :id "0"}]]] - (testing "has one history entry" - (is (= 1 (d/total-num-of-instance-changes (d/db node) "Patient" "0")))) - (testing "contains that patient" - (given @(d/pull-many node (d/instance-history (d/db node) "Patient" "0")) + (given @(pull-instance-history (d/db node) "Patient" "0") count := 1 [0 :fhir/type] := :fhir/Patient [0 :id] := "0" [0 :meta :versionId] := #fhir/id"1")) (testing "has an empty history on another patient" - (is (coll/empty? (d/instance-history (d/db node) "Patient" "1"))) - (is (zero? (d/total-num-of-instance-changes (d/db node) "Patient" "1")))))) + (is (coll/empty? (d/instance-history (d/db node) "Patient" "1")))))) (testing "a node with one deleted patient" (with-system-data [{:blaze.db/keys [node]} config] [[[:put {:fhir/type :fhir/Patient :id "0"}]] [[:delete "Patient" "0"]]] - (testing "has two history entries" - (is (= 2 (d/total-num-of-instance-changes (d/db node) "Patient" "0")))) - - (let [patients @(d/pull-many node (d/instance-history (d/db node) "Patient" "0"))] + (let [patients @(pull-instance-history (d/db node) "Patient" "0")] (is (= 2 (count patients))) (testing "the first history entry is the patient marked as deleted" @@ -5878,17 +6092,14 @@ [:put {:fhir/type :fhir/Patient :id "1"}]] [[:put {:fhir/type :fhir/Patient :id "0" :active false}]]] - (testing "the first patient has two history entries" - (is (= 2 (d/total-num-of-instance-changes (d/db node) "Patient" "0")))) - (testing "contains both versions in reverse transaction order" - (given @(d/pull-many node (d/instance-history (d/db node) "Patient" "0")) + (given @(pull-instance-history (d/db node) "Patient" "0") count := 2 [0 :active] := false [1 :active] := true)) (testing "it is possible to start with the older transaction" - (given @(d/pull-many node (d/instance-history (d/db node) "Patient" "0" 1)) + (given @(pull-instance-history (d/db node) "Patient" "0" 1) count := 1 [0 :active] := true)) @@ -5906,9 +6117,6 @@ db @(d/transact node [[:put {:fhir/type :fhir/Patient :id "0" :active true}]])] - (testing "has one history entry" - (is (= 1 (d/total-num-of-instance-changes db "Patient" "0" since)))) - (testing "contains the patient" (given (into [] (d/stop-history-at db since) (d/instance-history db "Patient" "0")) count := 1 @@ -5923,11 +6131,8 @@ @(d/transact node [[:put {:fhir/type :fhir/Patient :id "0" :active true}]]) (testing "the original database" - (testing "has still only one history entry" - (is (= 1 (d/total-num-of-instance-changes db "Patient" "0")))) - (testing "contains still the original patient" - (given @(d/pull-many node (d/instance-history db "Patient" "0")) + (given @(pull-instance-history db "Patient" "0") count := 1 [0 :fhir/type] := :fhir/Patient [0 :id] := "0" @@ -6355,7 +6560,7 @@ (let [db (d/db node) patient (d/resource-handle db "Patient" "0")] - (is (coll/empty? (vec (d/rev-include db patient))))))) + (is (coll/empty? (d/rev-include db patient)))))) (testing "with three resources" (with-system-data [{:blaze.db/keys [node]} config] diff --git a/modules/db/test/blaze/db/impl/index/resource_as_of_spec.clj b/modules/db/test/blaze/db/impl/index/resource_as_of_spec.clj index 4eaf1a3c8..82089ad85 100644 --- a/modules/db/test/blaze/db/impl/index/resource_as_of_spec.clj +++ b/modules/db/test/blaze/db/impl/index/resource_as_of_spec.clj @@ -34,6 +34,7 @@ :args (s/cat :snapshot ::kv/snapshot :tid :blaze.db/tid :id :blaze.db/id-byte-string + :t :blaze.db/t :start-t :blaze.db/t) :ret (cs/coll-of :blaze.db/resource-handle)) @@ -56,11 +57,3 @@ :id-extractor (s/? fn?) :matcher (s/? fn?)) :ret fn?) - -(s/fdef rao/num-of-instance-changes - :args (s/cat :snapshot ::kv/snapshot - :tid :blaze.db/tid - :id :blaze.db/id-byte-string - :start-t :blaze.db/t - :end-t :blaze.db/t) - :ret nat-int?) diff --git a/modules/db/test/blaze/db/impl/index/resource_handle_spec.clj b/modules/db/test/blaze/db/impl/index/resource_handle_spec.clj index dc37469ce..bb7a83048 100644 --- a/modules/db/test/blaze/db/impl/index/resource_handle_spec.clj +++ b/modules/db/test/blaze/db/impl/index/resource_handle_spec.clj @@ -48,6 +48,10 @@ :args (s/cat :rh rh/resource-handle?) :ret :blaze.db/op) +(s/fdef rh/purged-at + :args (s/cat :rh rh/resource-handle?) + :ret :blaze.db/t) + (s/fdef rh/reference :args (s/cat :rh rh/resource-handle?) :ret :blaze.fhir/literal-ref) diff --git a/modules/db/test/blaze/db/impl/index/resource_handle_test.clj b/modules/db/test/blaze/db/impl/index/resource_handle_test.clj index 33e302a48..54155cc47 100644 --- a/modules/db/test/blaze/db/impl/index/resource_handle_test.clj +++ b/modules/db/test/blaze/db/impl/index/resource_handle_test.clj @@ -27,7 +27,9 @@ ([tid id t hash num-changes] (resource-handle tid id t hash num-changes :create)) ([tid id t hash num-changes op] - (rh/->ResourceHandle tid id t hash num-changes op))) + (resource-handle tid id t hash num-changes op Long/MAX_VALUE)) + ([tid id t hash num-changes op purged-at] + (rh/->ResourceHandle tid id t hash num-changes op purged-at))) (deftest state->num-changes-test (are [state num-changes] (= num-changes @@ -77,6 +79,12 @@ (let [rh (resource-handle 0 "foo" 0 hash 0 op)] (= op (:op rh) (rh/op rh) (apply rh/op [rh])))))) +(deftest purged-at-test + (satisfies-prop 10 + (prop/for-all [purged-at (s/gen :blaze.db/t)] + (let [rh (resource-handle 0 "foo" 0 hash 0 :create purged-at)] + (= purged-at (:purged-at rh) (rh/purged-at rh) (apply rh/purged-at [rh])))))) + (deftest reference-test (satisfies-prop 100 (prop/for-all [id (s/gen :blaze.resource/id)] diff --git a/modules/db/test/blaze/db/impl/index/rts_as_of_spec.clj b/modules/db/test/blaze/db/impl/index/rts_as_of_spec.clj index 50ddb8a02..067a875d2 100644 --- a/modules/db/test/blaze/db/impl/index/rts_as_of_spec.clj +++ b/modules/db/test/blaze/db/impl/index/rts_as_of_spec.clj @@ -14,5 +14,6 @@ :t :blaze.db/t :hash :blaze.resource/hash :num-changes nat-int? - :op keyword?) + :op keyword? + :purged-at (s/? :blaze.db/t)) :ret (s/coll-of :blaze.db.kv/put-entry)) diff --git a/modules/db/test/blaze/db/impl/index/rts_as_of_test_util.clj b/modules/db/test/blaze/db/impl/index/rts_as_of_test_util.clj index 732e97882..8f7f267c0 100644 --- a/modules/db/test/blaze/db/impl/index/rts_as_of_test_util.clj +++ b/modules/db/test/blaze/db/impl/index/rts_as_of_test_util.clj @@ -10,6 +10,9 @@ (let [buf (bb/wrap byte-array) hash (hash/from-byte-buffer! buf) state (bb/get-long! buf)] - {:hash hash - :num-changes (rh/state->num-changes state) - :op (rh/state->op state)})) + (cond-> + {:hash hash + :num-changes (rh/state->num-changes state) + :op (rh/state->op state)} + (<= 8 (bb/remaining buf)) + (assoc :purged-at (bb/get-long! buf))))) diff --git a/modules/db/test/blaze/db/impl/index/system_as_of_spec.clj b/modules/db/test/blaze/db/impl/index/system_as_of_spec.clj index aa0769ece..7e63b795c 100644 --- a/modules/db/test/blaze/db/impl/index/system_as_of_spec.clj +++ b/modules/db/test/blaze/db/impl/index/system_as_of_spec.clj @@ -15,6 +15,7 @@ (s/fdef sao/system-history :args (s/cat :snapshot :blaze.db.kv/snapshot + :t :blaze.db/t :start-t :blaze.db/t :start-tid (s/nilable :blaze.db/tid) :start-id (s/nilable :blaze.db/id-byte-string)) diff --git a/modules/db/test/blaze/db/impl/index/type_as_of_spec.clj b/modules/db/test/blaze/db/impl/index/type_as_of_spec.clj index 915bb53b6..6ac4dc358 100644 --- a/modules/db/test/blaze/db/impl/index/type_as_of_spec.clj +++ b/modules/db/test/blaze/db/impl/index/type_as_of_spec.clj @@ -15,6 +15,7 @@ (s/fdef tao/type-history :args (s/cat :snapshot :blaze.db.kv/snapshot :tid :blaze.db/tid + :t :blaze.db/t :start-t :blaze.db/t :start-id (s/nilable :blaze.db/id-byte-string)) :ret (cs/coll-of :blaze.db/resource-handle)) diff --git a/modules/db/test/blaze/db/node/tx_indexer/verify_test.clj b/modules/db/test/blaze/db/node/tx_indexer/verify_test.clj index 487a73f31..bb4f4e097 100644 --- a/modules/db/test/blaze/db/node/tx_indexer/verify_test.clj +++ b/modules/db/test/blaze/db/node/tx_indexer/verify_test.clj @@ -1,5 +1,6 @@ (ns blaze.db.node.tx-indexer.verify-test (:require + [blaze.anomaly :as ba] [blaze.byte-string :as bs] [blaze.db.api :as d] [blaze.db.impl.index.patient-last-change-test-util :as plc-tu] @@ -23,9 +24,12 @@ [blaze.fhir.spec.type] [blaze.log] [blaze.module.test-util :refer [with-system]] - [blaze.test-util :as tu] + [blaze.test-util :as tu :refer [satisfies-prop]] + [clojure.spec.alpha :as s] + [clojure.spec.gen.alpha :as sg] [clojure.spec.test.alpha :as st] [clojure.test :as test :refer [deftest is testing]] + [clojure.test.check.properties :as prop] [cognitect.anomalies :as anom] [juxt.iota :refer [given]] [taoensso.timbre :as log])) @@ -49,6 +53,20 @@ :patient #fhir/Reference{:reference "Patient/0"}}) (deftest verify-tx-cmds-test + (testing "two commands with the same identity aren't allowed" + (let [hash (hash/generate patient-0) + op-gen (sg/such-that (complement #{"conditional-delete"}) + (s/gen :blaze.db.tx-cmd/op)) + cmd (fn [op] {:op op :type "Patient" :id "0" :hash hash})] + (with-system [{:blaze.db/keys [node]} config] + + (satisfies-prop 100 + (prop/for-all [op-1 op-gen + op-2 op-gen] + (ba/conflict? + (verify/verify-tx-cmds search-param-registry (d/db node) 1 + [(cmd op-1) (cmd op-2)]))))))) + (testing "adding one Patient to an empty store" (let [hash (hash/generate patient-0)] (doseq [op [:create :put] @@ -867,3 +885,107 @@ [7 0] := :system-stats-index [7 1 ss-tu/decode-key] := {:t 2} [7 2 ss-tu/decode-val] := {:total 0 :num-changes 4}))))))) + +(deftest verify-delete-history-test + (testing "empty database" + (with-system [{:blaze.db/keys [node]} config] + + (is (empty? (verify/verify-tx-cmds + search-param-registry + (d/db node) 1 + [{:op "delete-history" :type "Patient" :id "0"}]))))) + + (testing "one patient" + (testing "with one version" + (with-system-data [{:blaze.db/keys [node]} config] + [[[:create {:fhir/type :fhir/Patient :id "0"}]]] + + (is (empty? (verify/verify-tx-cmds + search-param-registry + (d/db node) 3 + [{:op "delete-history" :type "Patient" :id "0"}]))))) + + (testing "with two versions" + (let [patient-v1 {:fhir/type :fhir/Patient :id "0" :active false} + patient-v2 {:fhir/type :fhir/Patient :id "0" :active true} + hash-v1 (hash/generate patient-v1)] + (with-system-data [{:blaze.db/keys [node]} config] + [[[:create patient-v1]] + [[:put patient-v2]]] + + (given (verify/verify-tx-cmds + search-param-registry + (d/db node) 3 + [{:op "delete-history" :type "Patient" :id "0"}]) + + count := 5 + + [0 0] := :resource-as-of-index + [0 1 rao-tu/decode-key] := {:type "Patient" :id "0" :t 1} + [0 2 rts-tu/decode-val] := {:hash hash-v1 :num-changes 1 :op :create :purged-at 3} + + [1 0] := :type-as-of-index + [1 1 tao-tu/decode-key] := {:type "Patient" :t 1 :id "0"} + [1 2 rts-tu/decode-val] := {:hash hash-v1 :num-changes 1 :op :create :purged-at 3} + + [2 0] := :system-as-of-index + [2 1 sao-tu/decode-key] := {:t 1 :type "Patient" :id "0"} + [2 2 rts-tu/decode-val] := {:hash hash-v1 :num-changes 1 :op :create :purged-at 3} + + [3 0] := :type-stats-index + [3 1 ts-tu/decode-key] := {:type "Patient" :t 3} + [3 2 ts-tu/decode-val] := {:total 1 :num-changes 1} + + [4 0] := :system-stats-index + [4 1 ss-tu/decode-key] := {:t 3} + [4 2 ss-tu/decode-val] := {:total 1 :num-changes 1})))) + + (testing "with three versions" + (let [patient-v1 {:fhir/type :fhir/Patient :id "0" :active false} + patient-v2 {:fhir/type :fhir/Patient :id "0" :active true} + patient-v3 {:fhir/type :fhir/Patient :id "0" :active false} + hash-v1 (hash/generate patient-v1) + hash-v2 (hash/generate patient-v2)] + (with-system-data [{:blaze.db/keys [node]} config] + [[[:create patient-v1]] + [[:put patient-v2]] + [[:put patient-v3]]] + + (given (verify/verify-tx-cmds + search-param-registry + (d/db node) 4 + [{:op "delete-history" :type "Patient" :id "0"}]) + + count := 8 + + [0 0] := :resource-as-of-index + [0 1 rao-tu/decode-key] := {:type "Patient" :id "0" :t 2} + [0 2 rts-tu/decode-val] := {:hash hash-v2 :num-changes 2 :op :put :purged-at 4} + + [1 0] := :type-as-of-index + [1 1 tao-tu/decode-key] := {:type "Patient" :t 2 :id "0"} + [1 2 rts-tu/decode-val] := {:hash hash-v2 :num-changes 2 :op :put :purged-at 4} + + [2 0] := :system-as-of-index + [2 1 sao-tu/decode-key] := {:t 2 :type "Patient" :id "0"} + [2 2 rts-tu/decode-val] := {:hash hash-v2 :num-changes 2 :op :put :purged-at 4} + + [3 0] := :resource-as-of-index + [3 1 rao-tu/decode-key] := {:type "Patient" :id "0" :t 1} + [3 2 rts-tu/decode-val] := {:hash hash-v1 :num-changes 1 :op :create :purged-at 4} + + [4 0] := :type-as-of-index + [4 1 tao-tu/decode-key] := {:type "Patient" :t 1 :id "0"} + [4 2 rts-tu/decode-val] := {:hash hash-v1 :num-changes 1 :op :create :purged-at 4} + + [5 0] := :system-as-of-index + [5 1 sao-tu/decode-key] := {:t 1 :type "Patient" :id "0"} + [5 2 rts-tu/decode-val] := {:hash hash-v1 :num-changes 1 :op :create :purged-at 4} + + [6 0] := :type-stats-index + [6 1 ts-tu/decode-key] := {:type "Patient" :t 4} + [6 2 ts-tu/decode-val] := {:total 1 :num-changes 1} + + [7 0] := :system-stats-index + [7 1 ss-tu/decode-key] := {:t 4} + [7 2 ss-tu/decode-val] := {:total 1 :num-changes 1})))))) diff --git a/modules/fhir-client/Makefile b/modules/fhir-client/Makefile index d28ff329c..edadd5867 100644 --- a/modules/fhir-client/Makefile +++ b/modules/fhir-client/Makefile @@ -13,6 +13,12 @@ test: prep test-coverage: prep clojure -M:test:coverage +deps-tree: + clojure -X:deps tree + +deps-list: + clojure -X:deps list + cloc-prod: cloc src @@ -22,4 +28,4 @@ cloc-test: clean: rm -rf .clj-kondo/.cache .cpcache target -.PHONY: fmt lint prep test test-coverage cloc-prod cloc-test clean +.PHONY: fmt lint prep test test-coverage deps-tree deps-list cloc-prod cloc-test clean diff --git a/modules/fhir-client/src/blaze/fhir_client.clj b/modules/fhir-client/src/blaze/fhir_client.clj index b77fd0286..3df937e48 100644 --- a/modules/fhir-client/src/blaze/fhir_client.clj +++ b/modules/fhir-client/src/blaze/fhir_client.clj @@ -37,11 +37,17 @@ (impl/update (str base-uri "/" (name type) "/" id) resource opts)) (defn delete + "Returns a CompletableFuture that will complete with resource of `type` and + `id` deleted." + [base-uri type id & [opts]] + (impl/delete (str base-uri "/" type "/" id) opts)) + +(defn delete-history "Returns a CompletableFuture that will complete with resource of `type` and `id` deleted." {:arglists '([base-uri type id & [opts]])} [base-uri type id & [opts]] - (impl/delete (str base-uri "/" (name type) "/" id) opts)) + (impl/delete (str base-uri "/" type "/" id "/_history") opts)) (defn transact "Returns a CompletableFuture that will complete with `bundle` transacted." @@ -113,6 +119,29 @@ (flow/subscribe! src pro) dst)) +(defn history-instance-publisher + "Returns a Publisher that produces a Bundle per page of versions of resource + with `type` and `id`. + + Use `resource-processor` to transform the pages to individual resources. Use + `history-instance` if you simply want to fetch all resources." + [base-uri type id & [opts]] + (reify Flow$Publisher + (subscribe [_ subscriber] + (->> (impl/paging-subscription subscriber (str base-uri "/" type "/" id "/_history") opts) + (flow/on-subscribe! subscriber))))) + +(defn history-instance + "Returns a CompletableFuture that will complete with all versions of resource + with `type` and `id` in case of success or will complete exceptionally with an + anomaly in case of an error." + [base-uri type id & [opts]] + (let [src (history-instance-publisher base-uri type id opts) + pro (resource-processor) + dst (flow/collect pro)] + (flow/subscribe! src pro) + dst)) + (defn spit "Returns a CompletableFuture that will complete with a vector of all filenames written of all resources the `publisher` produces." diff --git a/modules/fhir-client/src/blaze/fhir_client_spec.clj b/modules/fhir-client/src/blaze/fhir_client_spec.clj index 27f6f987c..745bfbf36 100644 --- a/modules/fhir-client/src/blaze/fhir_client_spec.clj +++ b/modules/fhir-client/src/blaze/fhir_client_spec.clj @@ -35,6 +35,11 @@ :opts (s/? :blaze.fhir-client/options)) :ret ac/completable-future?) +(s/fdef fhir-client/delete-history + :args (s/cat :base-uri string? :type :fhir.resource/type :id :blaze.resource/id + :opts (s/? :blaze.fhir-client/options)) + :ret ac/completable-future?) + (s/fdef fhir-client/transact :args (s/cat :base-uri string? :bundle :blaze/resource :opts (s/? :blaze.fhir-client/options)) @@ -59,10 +64,24 @@ :opts (s/? :blaze.fhir-client/options)) :ret ac/completable-future?) +(s/fdef fhir-client/search-system-publisher + :args (s/cat :base-uri string? :opts (s/? :blaze.fhir-client/options)) + :ret flow/publisher?) + (s/fdef fhir-client/search-system :args (s/cat :base-uri string? :opts (s/? :blaze.fhir-client/options)) :ret ac/completable-future?) +(s/fdef fhir-client/history-instance-publisher + :args (s/cat :base-uri string? :type :fhir.resource/type :id :blaze.resource/id + :opts (s/? :blaze.fhir-client/options)) + :ret flow/publisher?) + +(s/fdef fhir-client/history-instance + :args (s/cat :base-uri string? :type :fhir.resource/type :id :blaze.resource/id + :opts (s/? :blaze.fhir-client/options)) + :ret ac/completable-future?) + (s/fdef fhir-client/spit :args (s/cat :dir #(instance? Path %) :publisher flow/publisher?) :ret ac/completable-future?) diff --git a/modules/fhir-client/test/blaze/fhir_client_test.clj b/modules/fhir-client/test/blaze/fhir_client_test.clj index 7729ddb3f..e44762fc8 100644 --- a/modules/fhir-client/test/blaze/fhir_client_test.clj +++ b/modules/fhir-client/test/blaze/fhir_client_test.clj @@ -225,6 +225,16 @@ {:http-client http-client}) :fhir/type := :fhir/OperationOutcome)))) +(deftest delete-history-test + (let [http-client (HttpClientMock.)] + + (-> (.onDelete http-client "http://localhost:8080/fhir/Patient/0/_history") + (.doReturnStatus 204)) + + (is (nil? @(fhir-client/delete-history "http://localhost:8080/fhir" + "Patient" "0" + {:http-client http-client}))))) + (deftest transact-test (let [http-client (HttpClientMock.) bundle {:fhir/type :fhir/Bundle @@ -373,9 +383,9 @@ (given @(fhir-client/search-system "http://localhost:8080/fhir" {:http-client http-client}) + count := 1 [0 :fhir/type] := :fhir/Patient - [0 :id] := "0" - 1 := nil))) + [0 :id] := "0"))) (testing "one bundle with two patients" (let [http-client (HttpClientMock.)] @@ -391,11 +401,11 @@ (given @(fhir-client/search-system "http://localhost:8080/fhir" {:http-client http-client}) + count := 2 [0 :fhir/type] := :fhir/Patient [0 :id] := "0" [1 :fhir/type] := :fhir/Patient - [1 :id] := "1" - 2 := nil))) + [1 :id] := "1"))) (testing "two bundles with two patients" (let [http-client (HttpClientMock.)] @@ -421,11 +431,11 @@ (given @(fhir-client/search-system "http://localhost:8080/fhir" {:http-client http-client}) + count := 2 [0 :fhir/type] := :fhir/Patient [0 :id] := "0" [1 :fhir/type] := :fhir/Patient - [1 :id] := "1" - 2 := nil))) + [1 :id] := "1"))) (testing "with query params" (let [http-client (HttpClientMock.)] @@ -441,9 +451,9 @@ (given @(fhir-client/search-system "http://localhost:8080/fhir" {:http-client http-client :query-params {"_id" "0"}}) + count := 1 [0 :fhir/type] := :fhir/Patient - [0 :id] := "0" - 1 := nil))) + [0 :id] := "0"))) (testing "Server Error without JSON response" (let [http-client (HttpClientMock.)] @@ -455,6 +465,25 @@ {:http-client http-client}) ::anom/category := ::anom/fault)))) +(deftest history-instance-test + (testing "one bundle with one patient" + (let [http-client (HttpClientMock.)] + + (-> (.onGet http-client "http://localhost:8080/fhir/Patient/0/_history") + (.doReturn + (j/write-value-as-string + {:resourceType "Bundle" + :entry + [{:resource {:resourceType "Patient" :id "0"}}]})) + (.withHeader "content-type" "application/fhir+json")) + + (given @(fhir-client/history-instance "http://localhost:8080/fhir" + "Patient" "0" + {:http-client http-client}) + count := 1 + [0 :fhir/type] := :fhir/Patient + [0 :id] := "0")))) + (def temp-dir (Files/createTempDirectory "blaze" (make-array FileAttribute 0))) (deftest spit-test diff --git a/modules/http-client/test/blaze/http_client_test.clj b/modules/http-client/test/blaze/http_client_test.clj index 415a6e200..61341dd28 100644 --- a/modules/http-client/test/blaze/http_client_test.clj +++ b/modules/http-client/test/blaze/http_client_test.clj @@ -20,7 +20,7 @@ (set! *warn-on-reflection* true) (st/instrument) -(log/set-level! :trace) +(log/set-min-level! :trace) (test/use-fixtures :each tu/fixture) diff --git a/modules/interaction/.clj-kondo/config.edn b/modules/interaction/.clj-kondo/config.edn index 522e04b8c..a70b2e5a7 100644 --- a/modules/interaction/.clj-kondo/config.edn +++ b/modules/interaction/.clj-kondo/config.edn @@ -9,13 +9,14 @@ :lint-as {blaze.interaction.create-test/with-handler clojure.core/fn blaze.interaction.delete-test/with-handler clojure.core/fn + blaze.interaction.delete-history-test/with-handler clojure.core/fn blaze.interaction.conditional-delete-type-test/with-handler clojure.core/fn blaze.interaction.conditional-delete-type-test/with-handler-allow-multiple clojure.core/fn blaze.interaction.history.instance-test/with-handler clojure.core/fn blaze.interaction.history.system-test/with-handler clojure.core/fn blaze.interaction.history.type-test/with-handler clojure.core/fn blaze.interaction.read-test/with-handler clojure.core/fn - blaze.interaction.read-test/with-vread-handler clojure.core/fn + blaze.interaction.vread-test/with-handler clojure.core/fn blaze.interaction.search-compartment-test/with-handler clojure.core/fn blaze.interaction.search-system-test/with-handler clojure.core/fn blaze.interaction.search-type-test/with-handler clojure.core/fn diff --git a/modules/interaction/src/blaze/interaction/delete_history.clj b/modules/interaction/src/blaze/interaction/delete_history.clj new file mode 100644 index 000000000..802f31a40 --- /dev/null +++ b/modules/interaction/src/blaze/interaction/delete_history.clj @@ -0,0 +1,24 @@ +(ns blaze.interaction.delete-history + "FHIR delete-history interaction. + + https://build.fhir.org/http.html#delete-history" + (:require + [blaze.async.comp :refer [do-sync]] + [blaze.db.api :as d] + [blaze.db.spec] + [blaze.module :as m] + [clojure.spec.alpha :as s] + [integrant.core :as ig] + [reitit.core :as reitit] + [ring.util.response :as ring] + [taoensso.timbre :as log])) + +(defmethod m/pre-init-spec :blaze.interaction/delete-history [_] + (s/keys :req-un [:blaze.db/node])) + +(defmethod ig/init-key :blaze.interaction/delete-history [_ {:keys [node]}] + (log/info "Init FHIR delete-history interaction handler") + (fn [{{{:fhir.resource/keys [type]} :data} ::reitit/match + {:keys [id]} :path-params}] + (do-sync [_ (d/transact node [[:delete-history type id]])] + (ring/status 204)))) diff --git a/modules/interaction/src/blaze/interaction/history/instance.clj b/modules/interaction/src/blaze/interaction/history/instance.clj index 3f3498745..603bce1c6 100644 --- a/modules/interaction/src/blaze/interaction/history/instance.clj +++ b/modules/interaction/src/blaze/interaction/history/instance.clj @@ -45,11 +45,13 @@ {:fhir/type :fhir/Bundle :id (handler-util/luid context) :type #fhir/code"history" - :total (type/->UnsignedInt total) :link [(history-util/self-link context query-params)] :entry (mapv (partial history-util/build-entry context) paged-versions)} + total + (assoc :total (type/->UnsignedInt total)) + (< page-size (count paged-version-handles)) (update :link conj (next-link (peek paged-version-handles)))))))))) diff --git a/modules/interaction/src/blaze/interaction/read.clj b/modules/interaction/src/blaze/interaction/read.clj index b23d0e117..850e733a0 100644 --- a/modules/interaction/src/blaze/interaction/read.clj +++ b/modules/interaction/src/blaze/interaction/read.clj @@ -3,40 +3,20 @@ https://www.hl7.org/fhir/http.html#read" (:require - [blaze.anomaly :as ba] - [blaze.async.comp :as ac :refer [do-sync]] + [blaze.async.comp :refer [do-sync]] [blaze.db.spec] [blaze.handler.fhir.util :as fhir-util] + [blaze.interaction.util :as iu] [integrant.core :as ig] [reitit.core :as reitit] - [ring.util.response :as ring] [taoensso.timbre :as log])) -(defn- response [resource] - (let [{:blaze.db/keys [tx]} (meta resource)] - (-> (ring/response resource) - (ring/header "Last-Modified" (fhir-util/last-modified tx)) - (ring/header "ETag" (fhir-util/etag tx))))) - (def ^:private handler (fn [{{{:fhir.resource/keys [type]} :data} ::reitit/match {:keys [id]} :path-params :blaze/keys [db]}] (do-sync [resource (fhir-util/pull db type id)] - (response resource)))) - -(defn- wrap-invalid-id [handler] - (fn [{{:keys [id]} :path-params :as request}] - (cond - (not (re-matches #"[A-Za-z0-9\-\.]{1,64}" id)) - (ac/completed-future - (ba/incorrect - (format "Resource id `%s` is invalid." id) - :fhir/issue "value" - :fhir/operation-outcome "MSG_ID_INVALID")) - - :else - (handler request)))) + (iu/response resource)))) (defmethod ig/init-key :blaze.interaction/read [_ _] (log/info "Init FHIR read interaction handler") - (wrap-invalid-id handler)) + (iu/wrap-invalid-id handler)) diff --git a/modules/interaction/src/blaze/interaction/util.clj b/modules/interaction/src/blaze/interaction/util.clj index 028936cb7..b769fa809 100644 --- a/modules/interaction/src/blaze/interaction/util.clj +++ b/modules/interaction/src/blaze/interaction/util.clj @@ -1,12 +1,15 @@ (ns blaze.interaction.util (:require [blaze.anomaly :as ba :refer [when-ok]] + [blaze.async.comp :as ac] [blaze.db.api :as d] [blaze.fhir.hash :as hash] [blaze.fhir.spec.type :as type] + [blaze.handler.fhir.util :as fhir-util] [blaze.luid :as luid] [blaze.util :as u] - [clojure.string :as str])) + [clojure.string :as str] + [ring.util.response :as ring])) (defn etag->t [etag] (let [[_ t] (re-find #"W/\"(\d+)\"" etag)] @@ -124,3 +127,22 @@ {:arglists '([tx-op])} [[op]] (identical? :keep op)) + +(defn wrap-invalid-id [handler] + (fn [{{:keys [id]} :path-params :as request}] + (cond + (not (re-matches #"[A-Za-z0-9\-\.]{1,64}" id)) + (ac/completed-future + (ba/incorrect + (format "Resource id `%s` is invalid." id) + :fhir/issue "value" + :fhir/operation-outcome "MSG_ID_INVALID")) + + :else + (handler request)))) + +(defn response [resource] + (let [{:blaze.db/keys [tx]} (meta resource)] + (-> (ring/response resource) + (ring/header "Last-Modified" (fhir-util/last-modified tx)) + (ring/header "ETag" (fhir-util/etag tx))))) diff --git a/modules/interaction/src/blaze/interaction/vread.clj b/modules/interaction/src/blaze/interaction/vread.clj new file mode 100644 index 000000000..e805cb6ca --- /dev/null +++ b/modules/interaction/src/blaze/interaction/vread.clj @@ -0,0 +1,38 @@ +(ns blaze.interaction.vread + "FHIR read interaction. + + https://www.hl7.org/fhir/http.html#read" + (:require + [blaze.anomaly :as ba] + [blaze.async.comp :as ac :refer [do-sync]] + [blaze.db.api :as d] + [blaze.db.spec] + [blaze.handler.fhir.util :as fhir-util] + [blaze.interaction.util :as iu] + [integrant.core :as ig] + [reitit.core :as reitit] + [taoensso.timbre :as log])) + +(defn- not-found-anom + ([type id] + (ba/not-found + (format "Resource `%s/%s` with the given version was not found." type id) + :http/headers [["Cache-Control" "no-cache"]])) + ([type id t] + (ba/not-found + (format "Resource `%s/%s` with version `%d` was not found." type id t) + :http/headers [["Cache-Control" "no-cache"]]))) + +(def ^:private handler + (fn [{{{:fhir.resource/keys [type]} :data} ::reitit/match + {:keys [id vid]} :path-params :blaze/keys [db]}] + (if-let [t (fhir-util/parse-nat-long vid)] + (if (<= t (d/t db)) + (do-sync [resource (fhir-util/pull-historic db type id t)] + (iu/response resource)) + (ac/completed-future (not-found-anom type id t))) + (ac/completed-future (not-found-anom type id))))) + +(defmethod ig/init-key :blaze.interaction/vread [_ _] + (log/info "Init FHIR read interaction handler") + (iu/wrap-invalid-id handler)) diff --git a/modules/interaction/test/blaze/interaction/conditional_delete_type_test.clj b/modules/interaction/test/blaze/interaction/conditional_delete_type_test.clj index 843f21a8d..d303aa1c7 100644 --- a/modules/interaction/test/blaze/interaction/conditional_delete_type_test.clj +++ b/modules/interaction/test/blaze/interaction/conditional_delete_type_test.clj @@ -1,5 +1,5 @@ (ns blaze.interaction.conditional-delete-type-test - "Specifications relevant for the FHIR update interaction: + "Specifications relevant for the FHIR conditional delete interaction: https://www.hl7.org/fhir/http.html#cdelete" (:require diff --git a/modules/interaction/test/blaze/interaction/delete_history_test.clj b/modules/interaction/test/blaze/interaction/delete_history_test.clj new file mode 100644 index 000000000..3e4932bb5 --- /dev/null +++ b/modules/interaction/test/blaze/interaction/delete_history_test.clj @@ -0,0 +1,79 @@ +(ns blaze.interaction.delete-history-test + "Specifications relevant for the FHIR delete-history interaction: + + https://build.fhir.org/http.html#delete-history" + (:require + [blaze.db.api-stub :as api-stub :refer [with-system-data]] + [blaze.db.node :refer [node?]] + [blaze.interaction.delete-history] + [blaze.log] + [blaze.test-util :as tu :refer [given-thrown]] + [clojure.spec.alpha :as s] + [clojure.spec.test.alpha :as st] + [clojure.test :as test :refer [deftest is testing]] + [integrant.core :as ig] + [reitit.core :as reitit] + [taoensso.timbre :as log])) + +(st/instrument) +(log/set-min-level! :trace) + +(test/use-fixtures :each tu/fixture) + +(deftest init-test + (testing "nil config" + (given-thrown (ig/init {:blaze.interaction/delete-history nil}) + :key := :blaze.interaction/delete-history + :reason := ::ig/build-failed-spec + [:cause-data ::s/problems 0 :pred] := `map?)) + + (testing "missing config" + (given-thrown (ig/init {:blaze.interaction/delete-history {}}) + :key := :blaze.interaction/delete-history + :reason := ::ig/build-failed-spec + [:cause-data ::s/problems 0 :pred] := `(fn ~'[%] (contains? ~'% :node)))) + + (testing "invalid node" + (given-thrown (ig/init {:blaze.interaction/delete-history {:node ::invalid}}) + :key := :blaze.interaction/delete-history + :reason := ::ig/build-failed-spec + [:cause-data ::s/problems 0 :pred] := `node? + [:cause-data ::s/problems 0 :val] := ::invalid))) + +(def config + (assoc api-stub/mem-node-config + :blaze.interaction/delete-history + {:node (ig/ref :blaze.db/node)})) + +(defmacro with-handler [[handler-binding] & more] + (let [[txs body] (api-stub/extract-txs-body more)] + `(with-system-data [{handler# :blaze.interaction/delete-history} config] + ~txs + (let [~handler-binding handler#] + ~@body)))) + +(deftest handler-test + (testing "Returns No Content on non-existing resource" + (with-handler [handler] + (let [{:keys [status body]} + @(handler + {:path-params {:id "0"} + ::reitit/match {:data {:fhir.resource/type "Patient"}}})] + + (is (= 204 status)) + + (is (nil? body))))) + + (testing "Returns No Content on successful history deletion" + (with-handler [handler] + [[[:put {:fhir/type :fhir/Patient :id "0" :active false}]] + [[:put {:fhir/type :fhir/Patient :id "0" :active true}]]] + + (let [{:keys [status body]} + @(handler + {:path-params {:id "0"} + ::reitit/match {:data {:fhir.resource/type "Patient"}}})] + + (is (= 204 status)) + + (is (nil? body)))))) diff --git a/modules/interaction/test/blaze/interaction/delete_test.clj b/modules/interaction/test/blaze/interaction/delete_test.clj index 80452ee88..e030ac0dd 100644 --- a/modules/interaction/test/blaze/interaction/delete_test.clj +++ b/modules/interaction/test/blaze/interaction/delete_test.clj @@ -1,5 +1,5 @@ (ns blaze.interaction.delete-test - "Specifications relevant for the FHIR update interaction: + "Specifications relevant for the FHIR delete interaction: https://www.hl7.org/fhir/http.html#delete" (:require @@ -33,7 +33,7 @@ :reason := ::ig/build-failed-spec [:cause-data ::s/problems 0 :pred] := `(fn ~'[%] (contains? ~'% :node)))) - (testing "invalid executor" + (testing "invalid node" (given-thrown (ig/init {:blaze.interaction/delete {:node ::invalid}}) :key := :blaze.interaction/delete :reason := ::ig/build-failed-spec @@ -43,9 +43,7 @@ (def config (assoc api-stub/mem-node-config :blaze.interaction/delete - {:node (ig/ref :blaze.db/node) - :executor (ig/ref :blaze.test/executor)} - :blaze.test/executor {})) + {:node (ig/ref :blaze.db/node)})) (defmacro with-handler [[handler-binding] & more] (let [[txs body] (api-stub/extract-txs-body more)] diff --git a/modules/interaction/test/blaze/interaction/history/instance_test.clj b/modules/interaction/test/blaze/interaction/history/instance_test.clj index 1a825f18d..6e83724af 100644 --- a/modules/interaction/test/blaze/interaction/history/instance_test.clj +++ b/modules/interaction/test/blaze/interaction/history/instance_test.clj @@ -314,6 +314,9 @@ {:path-params {:id "0"} :params {"_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/0/_history?_count=1") (link-url body "self")))) diff --git a/modules/interaction/test/blaze/interaction/read_test.clj b/modules/interaction/test/blaze/interaction/read_test.clj index 343a1f855..464496484 100644 --- a/modules/interaction/test/blaze/interaction/read_test.clj +++ b/modules/interaction/test/blaze/interaction/read_test.clj @@ -12,7 +12,7 @@ [blaze.db.spec] [blaze.interaction.read] [blaze.interaction.test-util :refer [wrap-error]] - [blaze.middleware.fhir.db :refer [wrap-db wrap-versioned-instance-db]] + [blaze.middleware.fhir.db :refer [wrap-db]] [blaze.middleware.fhir.db-spec] [blaze.test-util :as tu] [clojure.spec.test.alpha :as st] @@ -50,16 +50,6 @@ wrap-error)] ~@body)))) -(defmacro with-vread-handler [[handler-binding] & more] - (let [[txs body] (api-stub/extract-txs-body more)] - `(with-system-data [{node# :blaze.db/node - handler# :blaze.interaction/read} config] - ~txs - (let [~handler-binding (-> handler# wrap-defaults - (wrap-versioned-instance-db node# 100) - wrap-error)] - ~@body)))) - (deftest handler-test (testing "returns Not-Found on non-existing resource" (with-handler [handler] @@ -89,21 +79,6 @@ [:issue 0 :details :coding 0 :code] := #fhir/code"MSG_ID_INVALID" [:issue 0 :diagnostics] := "Resource id `A_B` is invalid.")))) - (testing "returns Bad-Request on invalid version id" - (with-vread-handler [handler] - (let [{:keys [status body]} - @(handler {:path-params {:id "0" :vid "a"}})] - - (is (= 400 status)) - - (given body - :fhir/type := :fhir/OperationOutcome - [:issue 0 :severity] := #fhir/code"error" - [:issue 0 :code] := #fhir/code"value" - [:issue 0 :details :coding 0 :system] := operation-outcome - [:issue 0 :details :coding 0 :code] := #fhir/code"MSG_ID_INVALID" - [:issue 0 :diagnostics] := "Resource versionId `a` is invalid.")))) - (testing "returns Gone on deleted resource" (with-handler [handler] [[[:put {:fhir/type :fhir/Patient :id "0"}]] @@ -151,29 +126,6 @@ (is (= 200 status)) - (testing "Transaction time in Last-Modified header" - (is (= "Thu, 1 Jan 1970 00:00:00 GMT" (get headers "Last-Modified")))) - - (testing "Version in ETag header" - ;; 1 is the T of the transaction of the resource update - (is (= "W/\"1\"" (get headers "ETag")))) - - (given body - :fhir/type := :fhir/Patient - :id := "0" - [:meta :versionId] := #fhir/id"1" - [:meta :lastUpdated] := Instant/EPOCH)))) - - (testing "returns existing resource on versioned read even if it is currently deleted" - (with-vread-handler [handler] - [[[:put {:fhir/type :fhir/Patient :id "0"}]] - [[:delete "Patient" "0"]]] - - (let [{:keys [status headers body]} - @(handler {:path-params {:id "0" :vid "1"}})] - - (is (= 200 status)) - (testing "Transaction time in Last-Modified header" (is (= "Thu, 1 Jan 1970 00:00:00 GMT" (get headers "Last-Modified")))) diff --git a/modules/interaction/test/blaze/interaction/search_type_test.clj b/modules/interaction/test/blaze/interaction/search_type_test.clj index 48e5b2b77..7d045a619 100644 --- a/modules/interaction/test/blaze/interaction/search_type_test.clj +++ b/modules/interaction/test/blaze/interaction/search_type_test.clj @@ -835,7 +835,7 @@ @(handler {:params {"active" "true" "_count" "1"}})] - (testing "their is no total count because we have clauses and we have + (testing "there is no total count because we have clauses and we have more hits than page-size" (is (nil? (:total body)))) @@ -882,7 +882,7 @@ {::reitit/match patient-search-match :params {"active" "true" "_count" "1"}})] - (testing "their is no total count because we have clauses and we have + (testing "there is no total count because we have clauses and we have more hits than page-size" (is (nil? (:total body)))) @@ -929,7 +929,7 @@ {::reitit/match patient-page-match :path-params (page-path-params page-id-cipher {"__token" "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB" "_count" "1" "__t" "1" "__page-id" "2"})})] - (testing "their is no total count because we have clauses and we have + (testing "there is no total count because we have clauses and we have more hits than page-size" (is (nil? (:total body)))) diff --git a/modules/interaction/test/blaze/interaction/vread_test.clj b/modules/interaction/test/blaze/interaction/vread_test.clj new file mode 100644 index 000000000..c5634db62 --- /dev/null +++ b/modules/interaction/test/blaze/interaction/vread_test.clj @@ -0,0 +1,194 @@ +(ns blaze.interaction.vread-test + "Specifications relevant for the FHIR read interaction: + + https://www.hl7.org/fhir/http.html#read + https://www.hl7.org/fhir/operationoutcome.html + https://www.hl7.org/fhir/http.html#ops" + (:require + [blaze.anomaly-spec] + [blaze.db.api-stub :as api-stub :refer [with-system-data]] + [blaze.db.spec] + [blaze.interaction.test-util :refer [wrap-error]] + [blaze.interaction.vread] + [blaze.middleware.fhir.db :refer [wrap-db]] + [blaze.middleware.fhir.db-spec] + [blaze.test-util :as tu] + [clojure.spec.test.alpha :as st] + [clojure.test :as test :refer [deftest is testing]] + [juxt.iota :refer [given]] + [reitit.core :as reitit] + [taoensso.timbre :as log]) + (:import + [java.time Instant])) + +(st/instrument) +(log/set-min-level! :trace) + +(test/use-fixtures :each tu/fixture) + +(def config + (assoc api-stub/mem-node-config :blaze.interaction/vread {})) + +(def match + (reitit/map->Match {:data {:fhir.resource/type "Patient"}})) + +(def operation-outcome + #fhir/uri"http://terminology.hl7.org/CodeSystem/operation-outcome") + +(defn wrap-defaults [handler] + (fn [request] + (handler (assoc request ::reitit/match match)))) + +(defmacro with-handler [[handler-binding] & more] + (let [[txs body] (api-stub/extract-txs-body more)] + `(with-system-data [{node# :blaze.db/node + handler# :blaze.interaction/vread} config] + ~txs + (let [~handler-binding (-> handler# wrap-defaults (wrap-db node# 100) + wrap-error)] + ~@body)))) + +(deftest handler-test + (with-handler [handler] + [[[:put {:fhir/type :fhir/Patient :id "0"}]] + [[:delete "Patient" "0"]]] + + (testing "initial version" + (let [{:keys [status headers body]} + @(handler {:path-params {:id "0" :vid "1"}})] + + (is (= 200 status)) + + (testing "Transaction time in Last-Modified header" + (is (= "Thu, 1 Jan 1970 00:00:00 GMT" (get headers "Last-Modified")))) + + (testing "Version in ETag header" + ;; 1 is the T of the transaction of the resource update + (is (= "W/\"1\"" (get headers "ETag")))) + + (given body + :fhir/type := :fhir/Patient + :id := "0" + [:meta :versionId] := #fhir/id"1" + [:meta :lastUpdated] := Instant/EPOCH))) + + (testing "deleted version" + (let [{:keys [status headers body]} + @(handler {:path-params {:id "0" :vid "2"}})] + + (is (= 410 status)) + + (testing "Transaction time in Last-Modified header" + (is (= "Thu, 1 Jan 1970 00:00:00 GMT" (get headers "Last-Modified")))) + + (testing "Version in ETag header" + ;; 2 is the T of the transaction of the resource deletion + (is (= "W/\"2\"" (get headers "ETag")))) + + (given body + :fhir/type := :fhir/OperationOutcome + [:issue 0 :severity] := #fhir/code"error" + [:issue 0 :code] := #fhir/code"deleted" + [:issue 0 :diagnostics] := "Resource `Patient/0` was deleted."))) + + (testing "non existing version" + (let [{:keys [status headers body]} + @(handler {:path-params {:id "0" :vid "3"}})] + + (is (= 404 status)) + + (testing "has no Last-Modified header" + (is (nil? (get headers "Last-Modified")))) + + (testing "has no ETag header" + (is (nil? (get headers "ETag")))) + + (testing "disallows caching" + (is (= "no-cache" (get headers "Cache-Control")))) + + (given body + :fhir/type := :fhir/OperationOutcome + [:issue 0 :severity] := #fhir/code"error" + [:issue 0 :code] := #fhir/code"not-found" + [:issue 0 :diagnostics] := "Resource `Patient/0` with version `3` was not found."))) + + (testing "invalid version" + (let [{:keys [status headers body]} + @(handler {:path-params {:id "0" :vid "a"}})] + + (is (= 404 status)) + + (testing "has no Last-Modified header" + (is (nil? (get headers "Last-Modified")))) + + (testing "has no ETag header" + (is (nil? (get headers "ETag")))) + + (testing "disallows caching" + (is (= "no-cache" (get headers "Cache-Control")))) + + (given body + :fhir/type := :fhir/OperationOutcome + [:issue 0 :severity] := #fhir/code"error" + [:issue 0 :code] := #fhir/code"not-found" + [:issue 0 :diagnostics] := "Resource `Patient/0` with the given version was not found.")))) + + (testing "with deleted history" + (with-handler [handler] + [[[:put {:fhir/type :fhir/Patient :id "0" :active false}]] + [[:put {:fhir/type :fhir/Patient :id "0" :active true}]] + [[:delete-history "Patient" "0"]]] + + (testing "initial version doesn't exist anymore" + (let [{:keys [status headers body]} + @(handler {:path-params {:id "0" :vid "1"}})] + + (is (= 404 status)) + + (testing "has no Last-Modified header" + (is (nil? (get headers "Last-Modified")))) + + (testing "has no ETag header" + (is (nil? (get headers "ETag")))) + + (given body + :fhir/type := :fhir/OperationOutcome + [:issue 0 :severity] := #fhir/code"error" + [:issue 0 :code] := #fhir/code"not-found" + [:issue 0 :diagnostics] := "Resource `Patient/0` with version `1` was not found."))) + + (testing "current version still exists" + (let [{:keys [status headers body]} + @(handler {:path-params {:id "0" :vid "2"}})] + + (is (= 200 status)) + + (testing "Transaction time in Last-Modified header" + (is (= "Thu, 1 Jan 1970 00:00:00 GMT" (get headers "Last-Modified")))) + + (testing "Version in ETag header" + ;; 2 is the T of the transaction of the resource deletion + (is (= "W/\"2\"" (get headers "ETag")))) + + (given body + :fhir/type := :fhir/Patient + :id := "0" + :active := true))) + + (testing "version 3 doesn't exist" + (let [{:keys [status headers body]} + @(handler {:path-params {:id "0" :vid "3"}})] + + (is (= 404 status)) + + (testing "has no Last-Modified header" + (is (nil? (get headers "Last-Modified")))) + + (testing "has no ETag header" + (is (nil? (get headers "ETag")))) + + (given body + :fhir/type := :fhir/OperationOutcome + [:issue 0 :severity] := #fhir/code"error" + [:issue 0 :code] := #fhir/code"not-found" + [:issue 0 :diagnostics] := "Resource `Patient/0` with version `3` was not found.")))))) diff --git a/modules/jepsen/Makefile b/modules/jepsen/Makefile index 82df7dac7..9190f6749 100644 --- a/modules/jepsen/Makefile +++ b/modules/jepsen/Makefile @@ -13,12 +13,24 @@ test: prep test-coverage: true +deps-tree: + clojure -X:deps tree + +deps-list: + clojure -X:deps list + register-test-fast: prep clojure -M:register test --concurrency 16 --time-limit 60 -n localhost:8080 --delta-time 0.01 register-test-slow: prep clojure -M:register test --concurrency 16 --time-limit 60 -n localhost:8080 --delta-time 0.1 +resource-history-test-fast: prep + clojure -J-Xmx4g -M:resource-history test --concurrency 16 --time-limit 60 -n localhost:8080 --delta-time 0.01 + +resource-history-test-slow: prep + clojure -J-Xmx4g -M:resource-history test --concurrency 16 --time-limit 60 -n localhost:8080 --delta-time 0.1 + cloc-prod: cloc src @@ -28,4 +40,4 @@ cloc-test: clean: rm -rf .clj-kondo/.cache .cpcache target store -.PHONY: fmt lint prep test test-coverage cloc-prod cloc-test clean +.PHONY: fmt lint prep test test-coverage deps-tree deps-list cloc-prod cloc-test clean diff --git a/modules/jepsen/deps.edn b/modules/jepsen/deps.edn index 549ac15b7..1705fe116 100644 --- a/modules/jepsen/deps.edn +++ b/modules/jepsen/deps.edn @@ -21,4 +21,7 @@ :main-opts ["-m" "kaocha.runner"]} :register - {:main-opts ["-m" "blaze.jepsen.register"]}}} + {:main-opts ["-m" "blaze.jepsen.register"]} + + :resource-history + {:main-opts ["-m" "blaze.jepsen.resource-history"]}}} diff --git a/modules/jepsen/src/blaze/jepsen/resource_history.clj b/modules/jepsen/src/blaze/jepsen/resource_history.clj new file mode 100644 index 000000000..6d27f1d90 --- /dev/null +++ b/modules/jepsen/src/blaze/jepsen/resource_history.clj @@ -0,0 +1,128 @@ +(ns blaze.jepsen.resource-history + (:require + [blaze.anomaly :as ba] + [blaze.async.comp :as ac] + [blaze.fhir-client :as fhir-client] + [blaze.fhir.spec.type :as type] + [blaze.fhir.structure-definition-repo] + [blaze.jepsen.util :as u] + [clojure.tools.logging :refer [info]] + [hato.client :as hc] + [integrant.core :as ig] + [jepsen.checker :as checker] + [jepsen.cli :as cli] + [jepsen.client :as client] + [jepsen.generator :as gen] + [jepsen.tests :as tests] + [knossos.model :as model]) + (:import + [knossos.model Model])) + +(set! *warn-on-reflection* true) + +(ig/init {:blaze.fhir/structure-definition-repo {}}) + +(defn read-history "Reads all history values." [_ _] + {:type :invoke :f :read :value nil}) + +(defn add-history "Adds a random value to the history." [_ _] + {:type :invoke :f :add :value (str (random-uuid))}) + +(defn reset-history "Resets the history to only it's first value." [_ _] + {:type :invoke :f :reset :value nil}) + +(defrecord History [values] + Model + (step [r op] + (condp identical? (:f op) + :add (History. (cons (:value op) values)) + :reset (History. (some-> (first values) list)) + :read (if (or (nil? (:value op)) ; We don't know what the read was + (= values (:value op))) ; Read was a specific value + r + (model/inconsistent + (str (pr-str values) "≠" (pr-str (:value op))))))) + + Object + (toString [_] (pr-str values))) + +(defn client-read-history [{:keys [base-uri] :as context} id] + @(-> (fhir-client/history-instance base-uri "Patient" id context) + (ac/then-apply + (fn [versions] + {:type :ok :value (map (comp :value first :identifier) versions)})) + (ac/exceptionally + (fn [e] + {:type (if (ba/not-found? e) :ok :fail) :value nil})))) + +(defn client-add-history [{:keys [base-uri] :as context} id value] + @(-> (fhir-client/update + base-uri + {:fhir/type :fhir/Patient :id id + :identifier [(type/map->Identifier {:value value})]} + context) + (ac/then-apply (constantly {:type :ok})) + (ac/exceptionally (constantly {:type :fail})))) + +(defn client-reset-history [{:keys [base-uri] :as context} id] + @(-> (fhir-client/delete-history base-uri "Patient" id context) + (ac/then-apply (constantly {:type :ok})) + (ac/exceptionally (fn [e] (prn e) {:type :fail})))) + +(defrecord Client [context] + client/Client + (open! [this _test node] + (info "Open client on node" node) + (update this :context assoc + :base-uri (str "http://" node "/fhir") + :http-client (hc/build-http-client {:connect-timeout 10000}))) + + (setup! [this _test] + this) + + (invoke! [_ test op] + (case (:f op) + :read (merge op (client-read-history context (:id test))) + :add (merge op (client-add-history context (:id test) (:value op))) + :reset (merge op (client-reset-history context (:id test))))) + + (teardown! [this _test] + this) + + (close! [_ _test])) + +(defn blaze-test + "Given an options map from the command line runner (e.g. :nodes, :ssh, + :concurrency, ...), constructs a test map." + [opts] + (merge + tests/noop-test + {:pure-generators true + :name "resource-history" + :remote (u/->Remote) + :client (->Client {}) + :checker (checker/linearizable + {:model (->History nil) + :algorithm :linear}) + :generator (->> (gen/mix [read-history add-history]) + (gen/limit 100) + (gen/then (gen/once reset-history)) + (gen/cycle) + (gen/stagger (:delta-time opts)) + (gen/nemesis []) + (gen/time-limit (:time-limit opts)))} + opts)) + +(def cli-opts + "Additional command line options." + [[nil "--id ID" "The ID of the patient to use." :default (str (random-uuid))] + [nil "--delta-time s" "The duration between requests." + :default 0.1 + :parse-fn parse-double]]) + +(defn -main + "Handles command line arguments. Can either run a test, or a web server for + browsing results." + [& args] + (cli/run! (cli/single-test-cmd {:test-fn blaze-test :opt-spec cli-opts}) + args)) diff --git a/modules/rest-api/src/blaze/rest_api/routes.clj b/modules/rest-api/src/blaze/rest_api/routes.clj index f8f2bb9c8..d390918e1 100644 --- a/modules/rest-api/src/blaze/rest_api/routes.clj +++ b/modules/rest-api/src/blaze/rest_api/routes.clj @@ -47,10 +47,6 @@ {:name :snapshot-db :wrap db/wrap-snapshot-db}) -(def ^:private wrap-versioned-instance-db - {:name :versioned-instance-db - :wrap db/wrap-versioned-instance-db}) - (def ^:private wrap-ensure-form-body {:name :ensure-form-body :wrap ensure-form-body/wrap-ensure-form-body}) @@ -197,12 +193,16 @@ :middleware [[wrap-db node db-sync-timeout] wrap-link-headers] :handler (-> interactions :history-instance - :blaze.rest-api.interaction/handler)}))] + :blaze.rest-api.interaction/handler)}) + (contains? interactions :delete-history) + (assoc :delete {:interaction "delete-history" + :handler (-> interactions :delete-history + :blaze.rest-api.interaction/handler)}))] ["/{vid}" (cond-> {:name (keyword name "versioned-instance")} (contains? interactions :vread) (assoc :get {:interaction "vread" - :middleware [[wrap-versioned-instance-db node db-sync-timeout]] + :middleware [[wrap-db node db-sync-timeout]] :handler (-> interactions :vread :blaze.rest-api.interaction/handler)}))]]] (not batch?) diff --git a/modules/rest-api/src/blaze/rest_api/spec.clj b/modules/rest-api/src/blaze/rest_api/spec.clj index 499002f45..797e9d532 100644 --- a/modules/rest-api/src/blaze/rest_api/spec.clj +++ b/modules/rest-api/src/blaze/rest_api/spec.clj @@ -46,6 +46,7 @@ :update :patch :delete + :delete-history :conditional-delete-type :history-instance :history-type diff --git a/modules/rest-api/test/blaze/rest_api/routes_test.clj b/modules/rest-api/test/blaze/rest_api/routes_test.clj index 7c18c68b9..1d6ad188f 100644 --- a/modules/rest-api/test/blaze/rest_api/routes_test.clj +++ b/modules/rest-api/test/blaze/rest_api/routes_test.clj @@ -108,6 +108,9 @@ :delete #:blaze.rest-api.interaction {:handler (handler ::delete)} + :delete-history + #:blaze.rest-api.interaction + {:handler (handler ::delete-history)} :conditional-delete-type #:blaze.rest-api.interaction {:handler (handler ::conditional-delete-type)} @@ -185,6 +188,7 @@ "/Patient/0" :put "update" "/Patient/0" :delete "delete" "/Patient/0/_history" :get "history-instance" + "/Patient/0/_history" :delete "delete-history" "/Patient/0/__history-page/0" :get "history-instance" "/Patient/0/_history/42" :get "vread" "/Patient/0/$everything" :get "operation-instance-everything" @@ -227,6 +231,7 @@ "/Patient/0" :put ::update "/Patient/0" :delete ::delete "/Patient/0/_history" :get ::history-instance + "/Patient/0/_history" :delete ::delete-history "/Patient/0/__history-page/0" :get ::history-instance "/Patient/0/_history/42" :get ::vread "/Patient/0/$everything" :get ::everything @@ -274,8 +279,9 @@ "/Patient/0" :put [:observe-request-duration :params :output :error :forwarded :sync :resource] "/Patient/0" :delete [:observe-request-duration :params :output :error :forwarded :sync] "/Patient/0/_history" :get [:observe-request-duration :params :output :error :forwarded :sync :db :link-headers] + "/Patient/0/_history" :delete [:observe-request-duration :params :output :error :forwarded :sync] "/Patient/0/__history-page/0" :get [:observe-request-duration :params :output :error :forwarded :sync :decrypt-page-id :snapshot-db :link-headers] - "/Patient/0/_history/42" :get [:observe-request-duration :params :output :error :forwarded :sync :versioned-instance-db] + "/Patient/0/_history/42" :get [:observe-request-duration :params :output :error :forwarded :sync :db] "/Patient/0/$everything" :get [:observe-request-duration :params :output :error :forwarded :sync :db] "/Patient/0/__everything-page/0" :get [:observe-request-duration :params :output :error :forwarded :sync :decrypt-page-id :snapshot-db] "/Patient/0/Condition" :get [:observe-request-duration :params :output :error :forwarded :sync :db :link-headers] @@ -308,7 +314,8 @@ "/Patient/0" :put [:observe-request-duration :params :output :error :forwarded :sync :resource] "/Patient/0" :delete [:observe-request-duration :params :output :error :forwarded :sync] "/Patient/0/_history" :get [:observe-request-duration :params :output :error :forwarded :sync :db :link-headers] - "/Patient/0/_history/42" :get [:observe-request-duration :params :output :error :forwarded :sync :versioned-instance-db] + "/Patient/0/_history" :delete [:observe-request-duration :params :output :error :forwarded :sync] + "/Patient/0/_history/42" :get [:observe-request-duration :params :output :error :forwarded :sync :db] "/Patient/0/Condition" :get [:observe-request-duration :params :output :error :forwarded :sync :db :link-headers] "/Patient/0/Observation" :get [:observe-request-duration :params :output :error :forwarded :sync :db :link-headers] "/$compact-db" :get [:observe-request-duration :params :output :error :forwarded :sync :db] diff --git a/modules/rest-util/src/blaze/handler/fhir/util.clj b/modules/rest-util/src/blaze/handler/fhir/util.clj index 721aea17e..75b6cc25e 100644 --- a/modules/rest-util/src/blaze/handler/fhir/util.clj +++ b/modules/rest-util/src/blaze/handler/fhir/util.clj @@ -4,8 +4,8 @@ (:require [blaze.anomaly :as ba :refer [if-ok]] [blaze.async.comp :as ac :refer [do-sync]] + [blaze.coll.core :as coll] [blaze.db.api :as d] - [blaze.db.impl.index.resource-handle :as rh] [blaze.fhir.spec :as fhir-spec] [blaze.fhir.spec.type :as type] [blaze.fhir.spec.type.system :as system] @@ -140,25 +140,37 @@ [{:blaze.db/keys [t]}] (str "W/\"" t "\"")) +(defn- deleted-anom [db {:fhir/keys [type] :keys [id t]}] + (let [tx (d/tx db t)] + (ba/not-found + (format "Resource `%s/%s` was deleted." (name type) id) + :http/status 410 + :http/headers + [["Last-Modified" (last-modified tx)] + ["ETag" (etag tx)]] + :fhir/issue "deleted"))) + (defn- resource-handle [db type id] (if-let [{:keys [op] :as handle} (d/resource-handle db type id)] (if (identical? :delete op) - (let [tx (d/tx db (rh/t handle))] - (ba/not-found - (format "Resource `%s/%s` was deleted." type id) - :http/status 410 - :http/headers - [["Last-Modified" (last-modified tx)] - ["ETag" (etag tx)]] - :fhir/issue "deleted")) + (deleted-anom db handle) handle) (ba/not-found (format "Resource `%s/%s` was not found." type id) :fhir/issue "not-found"))) +(defn- pull* [db resource-handle] + (if (ba/anomaly? resource-handle) + (ac/completed-future resource-handle) + (-> (d/pull db resource-handle) + (ac/exceptionally + #(assoc % + ::anom/category ::anom/fault + :fhir/issue "incomplete"))))) + (defn pull "Returns a CompletableFuture that will complete with the resource with `type` - and `id` if not deleted in `db` or an anomaly otherwise. + and `id` if not deleted in `db` or complete exceptionally. Returns a not-found anomaly if the resource was not found or is deleted. In case it is deleted, sets :http/status to 410 and :http/headers Last-Modified @@ -167,13 +179,32 @@ Functions applied after the returned future are executed on the common ForkJoinPool." [db type id] - (if-ok [resource-handle (resource-handle db type id)] - (-> (d/pull db resource-handle) - (ac/exceptionally - #(assoc % - ::anom/category ::anom/fault - :fhir/issue "incomplete"))) - ac/completed-future)) + (pull* db (resource-handle db type id))) + +(defn- historic-resource-handle-not-found-anom [type id t] + (ba/not-found + (format "Resource `%s/%s` with version `%d` was not found." type id t))) + +(defn- historic-resource-handle + "Returns a CompletableFuture that will complete with the resource with `type`, + `id` and `t` (version) if not deleted in `db` or complete exceptionally. + + Returns a not-found anomaly if the resource was not found or is deleted. In + case it is deleted, sets :http/status to 410 and :http/headers Last-Modified + and ETag to appropriate values. + + Functions applied after the returned future are executed on the common + ForkJoinPool." + [db type id t] + (if-let [handle (coll/first (d/instance-history db type id t))] + (cond + (not= t (:t handle)) (historic-resource-handle-not-found-anom type id t) + (identical? :delete (:op handle)) (deleted-anom db handle) + :else handle) + (historic-resource-handle-not-found-anom type id t))) + +(defn pull-historic [db type id t] + (pull* db (historic-resource-handle db type id t))) (defn- timeout-msg [timeout] (format "Timeout while trying to acquire the latest known database state. At least one known transaction hasn't been completed yet. Please try to lower the transaction load or increase the timeout of %d ms by setting DB_SYNC_TIMEOUT to a higher value if you see this often." timeout)) diff --git a/modules/rest-util/src/blaze/handler/fhir/util_spec.clj b/modules/rest-util/src/blaze/handler/fhir/util_spec.clj index 071d5ca87..34bc958ad 100644 --- a/modules/rest-util/src/blaze/handler/fhir/util_spec.clj +++ b/modules/rest-util/src/blaze/handler/fhir/util_spec.clj @@ -12,6 +12,10 @@ [cognitect.anomalies :as anom] [reitit.core :as reitit])) +(s/fdef fhir-util/parse-nat-long + :args (s/cat :s string?) + :ret (s/nilable nat-int?)) + (s/fdef fhir-util/t :args (s/cat :query-params (s/nilable :ring.request/query-params)) :ret (s/nilable :blaze.db/t)) @@ -69,6 +73,11 @@ :args (s/cat :db :blaze.db/db :type :fhir.resource/type :id :blaze.resource/id) :ret ac/completable-future?) +(s/fdef fhir-util/pull-historic + :args (s/cat :db :blaze.db/db :type :fhir.resource/type :id :blaze.resource/id + :t :blaze.db/t) + :ret ac/completable-future?) + (s/fdef fhir-util/sync :args (s/cat :node :blaze.db/node :t (s/? :blaze.db/t) :timeout ::rest-api/db-sync-timeout) diff --git a/modules/rest-util/src/blaze/handler/util.clj b/modules/rest-util/src/blaze/handler/util.clj index e2283b2c5..6d8076157 100644 --- a/modules/rest-util/src/blaze/handler/util.clj +++ b/modules/rest-util/src/blaze/handler/util.clj @@ -152,7 +152,9 @@ * :fhir/operation-outcome - will go into `OperationOutcome.issue.details` as code with system http://terminology.hl7.org/CodeSystem/operation-outcome - * :fhir.issue/expression - will go into `OperationOutcome.issue.expression`" + * :fhir.issue/expression - will go into `OperationOutcome.issue.expression` + * :http/status - the HTTP status to use + * :http/headers - a list of tuples of header name and header value" [error] (error-response* error diff --git a/modules/rest-util/src/blaze/handler/util_spec.clj b/modules/rest-util/src/blaze/handler/util_spec.clj index 04ec42539..b2c54fb2b 100644 --- a/modules/rest-util/src/blaze/handler/util_spec.clj +++ b/modules/rest-util/src/blaze/handler/util_spec.clj @@ -10,6 +10,10 @@ :args (s/cat :headers (s/nilable map?) :name string?) :ret (s/nilable keyword?)) +(s/fdef handler-util/error-response + :args (s/cat :error some?) + :ret map?) + (s/fdef handler-util/luid :args (s/cat :context (s/keys :req-un [:blaze/clock :blaze/rng-fn])) :ret :blaze/luid) diff --git a/modules/rest-util/src/blaze/middleware/fhir/db.clj b/modules/rest-util/src/blaze/middleware/fhir/db.clj index 13b690ee9..9ab30060f 100644 --- a/modules/rest-util/src/blaze/middleware/fhir/db.clj +++ b/modules/rest-util/src/blaze/middleware/fhir/db.clj @@ -33,20 +33,3 @@ (ac/then-compose #(handler (assoc request :blaze/db %)))) (ac/completed-future (ba/incorrect (format "Missing or invalid `__t` query param `%s`." (get params "__t")))))))) - -(defn wrap-versioned-instance-db - "Database wrapping for versioned read requests. - - The `t` of the database state is taken from the path param `vid`." - [handler node timeout] - (fn [{{:keys [vid]} :path-params :as request}] - (if (:blaze/db request) - (handler request) - (if-let [t (some-> vid fhir-util/parse-nat-long)] - (-> (fhir-util/sync node t timeout) - (ac/then-compose #(handler (assoc request :blaze/db %)))) - (ac/completed-future - (ba/incorrect - (format "Resource versionId `%s` is invalid." vid) - :fhir/issue "value" - :fhir/operation-outcome "MSG_ID_INVALID")))))) diff --git a/modules/rest-util/src/blaze/middleware/fhir/db_spec.clj b/modules/rest-util/src/blaze/middleware/fhir/db_spec.clj index 2ff115ce0..d5a3c6bdf 100644 --- a/modules/rest-util/src/blaze/middleware/fhir/db_spec.clj +++ b/modules/rest-util/src/blaze/middleware/fhir/db_spec.clj @@ -9,6 +9,3 @@ (s/fdef db/wrap-snapshot-db :args (s/cat :handler ifn? :node :blaze.db/node :timeout pos-int?)) - -(s/fdef db/wrap-versioned-instance-db - :args (s/cat :handler ifn? :node :blaze.db/node :timeout pos-int?)) diff --git a/modules/rest-util/test/blaze/middleware/fhir/db_test.clj b/modules/rest-util/test/blaze/middleware/fhir/db_test.clj index f0e3c7bb8..8b593c577 100644 --- a/modules/rest-util/test/blaze/middleware/fhir/db_test.clj +++ b/modules/rest-util/test/blaze/middleware/fhir/db_test.clj @@ -24,7 +24,6 @@ (st/instrument) (st/unstrument `db/wrap-db) (st/unstrument `db/wrap-snapshot-db) - (st/unstrument `db/wrap-versioned-instance-db) (st/unstrument `fhir-util/sync) (st/unstrument `d/sync) (st/unstrument `d/as-of) @@ -124,54 +123,3 @@ (given-failed-future ((db/wrap-snapshot-db handler ::node timeout) {:params {"__t" "114148"}}) ::anom/category := ::anom/fault ::anom/message := "msg-115945")))) - -(deftest wrap-versioned-instance-db-test - (testing "uses existing database value" - (is (= ::db @((db/wrap-versioned-instance-db handler ::node timeout) {:blaze/db ::db})))) - - (testing "with missing or invalid vid" - (doseq [vid [nil "a" "-1"]] - (given-failed-future ((db/wrap-versioned-instance-db handler ::node timeout) {:path-params {:vid vid}}) - ::anom/category := ::anom/incorrect - ::anom/message := (format "Resource versionId `%s` is invalid." vid) - :fhir/issue := "value" - :fhir/operation-outcome := "MSG_ID_INVALID"))) - - (testing "uses the vid for database value acquisition" - (with-redefs - [d/sync - (fn [node t] - (assert (= ::node node)) - (assert (= 114418 t)) - (ac/completed-future ::db)) - d/as-of - (fn [db t] - (assert (= ::db db)) - (assert (= 114418 t)) - ::as-of-db)] - - (is (= ::as-of-db @((db/wrap-versioned-instance-db handler ::node timeout) {:path-params {:vid "114418"}}))))) - - (testing "fails on timeout" - (with-redefs - [d/sync - (fn [node t] - (assert (= ::node node)) - (assert (= 213957 t)) - (ac/supply-async (constantly ::db) (ac/delayed-executor 1 TimeUnit/SECONDS)))] - - (given-failed-future ((db/wrap-versioned-instance-db handler ::node timeout) {:path-params {:vid "213957"}}) - ::anom/category := ::anom/busy - ::anom/message := "Timeout while trying to acquire the database state with t=213957. The indexer has probably fallen behind. Please try to lower the transaction load or increase the timeout of 100 ms by setting DB_SYNC_TIMEOUT to a higher value if you see this often."))) - - (testing "fails on other sync error" - (with-redefs - [d/sync - (fn [node t] - (assert (= ::node node)) - (assert (= 120213 t)) - (ac/completed-future (ba/fault "msg-120219")))] - - (given-failed-future ((db/wrap-versioned-instance-db handler ::node timeout) {:path-params {:vid "120213"}}) - ::anom/category := ::anom/fault - ::anom/message := "msg-120219")))) diff --git a/modules/spec/src/blaze/spec.clj b/modules/spec/src/blaze/spec.clj index 004ea1838..6763ea1c4 100644 --- a/modules/spec/src/blaze/spec.clj +++ b/modules/spec/src/blaze/spec.clj @@ -154,6 +154,12 @@ (s/or :coll (s/coll-of string?) :string string?)) +(s/def :http/status + (s/and int? #(<= 100 % 599))) + +(s/def :http/headers + (s/coll-of (s/tuple string? string?))) + ;; ---- Clojure --------------------------------------------------------------- (s/def :clojure/binding-form (s/or :symbol simple-symbol? diff --git a/resources/blaze.edn b/resources/blaze.edn index 720f6d144..90a8e6cd6 100644 --- a/resources/blaze.edn +++ b/resources/blaze.edn @@ -67,6 +67,9 @@ :delete #:blaze.rest-api.interaction {:handler #blaze/ref :blaze.interaction/delete} + :delete-history + #:blaze.rest-api.interaction + {:handler #blaze/ref :blaze.interaction/delete-history} :conditional-delete-type #:blaze.rest-api.interaction {:handler #blaze/ref :blaze.interaction/conditional-delete-type} @@ -142,6 +145,9 @@ :blaze.interaction/delete {:node #blaze/ref :blaze.db.main/node} + :blaze.interaction/delete-history + {:node #blaze/ref :blaze.db.main/node} + :blaze.interaction/conditional-delete-type {:node #blaze/ref :blaze.db.main/node} diff --git a/test/blaze/system_test.clj b/test/blaze/system_test.clj index b7cd5ebe3..8bfafd958 100644 --- a/test/blaze/system_test.clj +++ b/test/blaze/system_test.clj @@ -6,11 +6,13 @@ [blaze.fhir.test-util :refer [structure-definition-repo]] [blaze.interaction.conditional-delete-type] [blaze.interaction.delete] + [blaze.interaction.delete-history] [blaze.interaction.history.type] [blaze.interaction.read] [blaze.interaction.search-system] [blaze.interaction.search-type] [blaze.interaction.transaction] + [blaze.interaction.vread] [blaze.middleware.fhir.decrypt-page-id :as decrypt-page-id] [blaze.middleware.fhir.decrypt-page-id-spec] [blaze.module.test-util :refer [with-system]] @@ -127,8 +129,11 @@ :rng-fn (ig/ref :blaze.test/fixed-rng-fn) :db-sync-timeout 10000} :blaze.interaction/read {} + :blaze.interaction/vread {} :blaze.interaction/delete {:node (ig/ref :blaze.db/node)} + :blaze.interaction/delete-history + {:node (ig/ref :blaze.db/node)} :blaze.interaction/conditional-delete-type {:node (ig/ref :blaze.db/node)} :blaze.interaction/search-system @@ -172,9 +177,15 @@ {:read #:blaze.rest-api.interaction {:handler (ig/ref :blaze.interaction/read)} + :vread + #:blaze.rest-api.interaction + {:handler (ig/ref :blaze.interaction/vread)} :delete #:blaze.rest-api.interaction {:handler (ig/ref :blaze.interaction/delete)} + :delete-history + #:blaze.rest-api.interaction + {:handler (ig/ref :blaze.interaction/delete-history)} :conditional-delete-type #:blaze.rest-api.interaction {:handler (ig/ref :blaze.interaction/conditional-delete-type)} @@ -242,10 +253,56 @@ :body := nil))) (deftest read-test - (with-system [{:blaze/keys [rest-api]} config] - (given (call rest-api {:request-method :get :uri "/Patient/0"}) - :status := 404 - [:body fhir-spec/parse-json :resourceType] := "OperationOutcome"))) + (with-system-data [{:blaze/keys [rest-api]} config] + [[[:put {:fhir/type :fhir/Patient :id "0"}]]] + + (testing "success" + (given (call rest-api {:request-method :get :uri "/Patient/0"}) + :status := 200 + [:body fhir-spec/parse-json :resourceType] := "Patient")) + + (testing "not found" + (given (call rest-api {:request-method :get :uri "/Patient/1"}) + :status := 404 + [:body fhir-spec/parse-json :resourceType] := "OperationOutcome")))) + +(deftest vread-test + (with-system-data [{:blaze/keys [rest-api]} config] + [[[:put {:fhir/type :fhir/Patient :id "0" :active false}]] + [[:put {:fhir/type :fhir/Patient :id "0" :active true}]]] + + (testing "current version" + (given (call rest-api {:request-method :get :uri "/Patient/0/_history/2"}) + :status := 200 + [:body fhir-spec/parse-json :active] := true)) + + (testing "older version" + (given (call rest-api {:request-method :get :uri "/Patient/0/_history/1"}) + :status := 200 + [:body fhir-spec/parse-json :active] := false)) + + (doseq [t [0 3]] + (testing (format "version %d doesn't exist" t) + (given (call rest-api {:request-method :get :uri (format "/Patient/0/_history/%d" t)}) + :status := 404 + [:body fhir-spec/parse-json :resourceType] := "OperationOutcome")))) + + (testing "with deleted history" + (with-system-data [{:blaze/keys [rest-api]} config] + [[[:put {:fhir/type :fhir/Patient :id "0" :active false}]] + [[:put {:fhir/type :fhir/Patient :id "0" :active true}]] + [[:delete-history "Patient" "0"]]] + + (testing "current version" + (given (call rest-api {:request-method :get :uri "/Patient/0/_history/2"}) + :status := 200 + [:body fhir-spec/parse-json :active] := true)) + + (doseq [t [0 1 3]] + (testing (format "version %d doesn't exist" t) + (given (call rest-api {:request-method :get :uri (format "/Patient/0/_history/%d" t)}) + :status := 404 + [:body fhir-spec/parse-json :resourceType] := "OperationOutcome")))))) (def read-bundle {:fhir/type :fhir/Bundle @@ -302,6 +359,19 @@ :status := 410 [:body fhir-spec/parse-json :resourceType] := "OperationOutcome"))) +(deftest delete-history-test + (with-system-data [{:blaze/keys [rest-api]} config] + [[[:put {:fhir/type :fhir/Patient :id "0" :active false}]] + [[:put {:fhir/type :fhir/Patient :id "0" :active true}]]] + + (given (call rest-api {:request-method :delete :uri "/Patient/0/_history"}) + :status := 204 + :body := nil) + + (given (call rest-api {:request-method :get :uri "/Patient/0/_history/1"}) + :status := 404 + [:body fhir-spec/parse-json :resourceType] := "OperationOutcome"))) + (deftest conditional-delete-type-test (with-system [{:blaze/keys [rest-api]} config] (given (call rest-api {:request-method :delete :uri "/Patient"})