From 30b308a791f8b7b5b99f78d19b2928285681cee0 Mon Sep 17 00:00:00 2001 From: Milton Reder Date: Mon, 10 Aug 2026 15:02:39 -0400 Subject: [PATCH 1/3] extend ::document/precondition-failed to all applicable actions and routes See https://github.com/yetanalytics/lrs/pull/106 from @chomatdam for more info --- .../lrs/pedestal/routes/documents.cljc | 16 ++--- .../com/yetanalytics/lrs/xapi/document.cljc | 14 ++++ .../lrs/pedestal/routes/documents_test.cljc | 69 +++++++++++++++++++ src/test/com/yetanalytics/test_runner.cljc | 2 + 4 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 src/test/com/yetanalytics/lrs/pedestal/routes/documents_test.cljc diff --git a/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc b/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc index f7074791..084ef947 100644 --- a/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc +++ b/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc @@ -4,6 +4,7 @@ [com.yetanalytics.lrs.protocol :as p] [com.yetanalytics.lrs.pedestal.interceptor :as i] [com.yetanalytics.lrs.pedestal.interceptor.xapi :as xi] + [com.yetanalytics.lrs.xapi.document :as doc] [clojure.spec.alpha :as s :include-macros true] [clojure.core.async :as a :include-macros true] #?(:clj [cheshire.core :as json]))) @@ -237,11 +238,9 @@ (defn put-response [ctx {:keys [error]}] (if error - (let [exd (ex-data error)] - (if (#{:com.yetanalytics.lrs.xapi.document/precondition-failed} - (:type exd)) - (assoc ctx :response {:status 412}) - (assoc ctx :io.pedestal.interceptor.chain/error error))) + (if (doc/precondition-failed? error) + (assoc ctx :response {:status 412}) + (assoc ctx :io.pedestal.interceptor.chain/error error)) (assoc ctx :response {:status 204}))) @@ -380,8 +379,7 @@ (if error (let [exd (ex-data error)] (cond - (#{:com.yetanalytics.lrs.xapi.document/precondition-failed} - (:type exd)) + (doc/precondition-failed? error) (assoc ctx :response {:status 412}) (#{:com.yetanalytics.lrs.xapi.document/json-read-error @@ -439,7 +437,9 @@ (defn delete-response [ctx {:keys [error]}] (if error - (assoc ctx :io.pedestal.interceptor.chain/error error) + (if (doc/precondition-failed? error) + (assoc ctx :response {:status 412}) + (assoc ctx :io.pedestal.interceptor.chain/error error)) (assoc ctx :response {:status 204}))) (def handle-delete diff --git a/src/main/com/yetanalytics/lrs/xapi/document.cljc b/src/main/com/yetanalytics/lrs/xapi/document.cljc index 440b0e04..3e79c1e5 100644 --- a/src/main/com/yetanalytics/lrs/xapi/document.cljc +++ b/src/main/com/yetanalytics/lrs/xapi/document.cljc @@ -89,6 +89,20 @@ (sgen/one-of [(document-gen-fn) (json-document-gen-fn)])))) +(defn precondition-failed-error + "Return a document operation error indicating that an ETag precondition + failed. Optional `data` is included in the exception data." + ([] + (precondition-failed-error {})) + ([data] + {:error (ex-info "Document precondition failed" + (assoc data :type ::precondition-failed))})) + +(defn precondition-failed? + "Return true when `error` represents a document precondition failure." + [error] + (= ::precondition-failed (:type (ex-data error)))) + (defn updated-stamp [document] (or diff --git a/src/test/com/yetanalytics/lrs/pedestal/routes/documents_test.cljc b/src/test/com/yetanalytics/lrs/pedestal/routes/documents_test.cljc new file mode 100644 index 00000000..24685ed6 --- /dev/null +++ b/src/test/com/yetanalytics/lrs/pedestal/routes/documents_test.cljc @@ -0,0 +1,69 @@ +(ns com.yetanalytics.lrs.pedestal.routes.documents-test + (:require [clojure.test :refer [deftest is testing] :include-macros true] + [clojure.spec.alpha :as s :include-macros true] + [com.yetanalytics.lrs.pedestal.routes.documents :as routes] + [com.yetanalytics.lrs.protocol :as p] + [com.yetanalytics.lrs.xapi.document :as doc])) + +(def ctx + {:context-value ::preserved}) + +(def unexpected-error + (ex-info "Unexpected document error" {:type ::unexpected})) + +(deftest precondition-failed-error-test + (let [{:keys [error] :as result} + (doc/precondition-failed-error {:operation :put})] + (is (doc/precondition-failed? error)) + (is (= {:type ::doc/precondition-failed + :operation :put} + (ex-data error))) + (is (s/valid? ::p/set-document-ret result)) + (is (s/valid? ::p/delete-document-ret result)) + (is (s/valid? ::p/delete-documents-ret result)))) + +(deftest put-response-test + (testing "success" + (is (= (assoc ctx :response {:status 204}) + (routes/put-response ctx {})))) + (testing "precondition failure" + (is (= (assoc ctx :response {:status 412}) + (routes/put-response ctx + (doc/precondition-failed-error))))) + (testing "unexpected implementation error" + (is (= (assoc ctx :io.pedestal.interceptor.chain/error unexpected-error) + (routes/put-response ctx {:error unexpected-error}))))) + +(deftest post-response-test + (testing "success" + (is (= (assoc ctx :response {:status 204}) + (routes/post-response ctx {})))) + (testing "precondition failure" + (is (= (assoc ctx :response {:status 412}) + (routes/post-response ctx + (doc/precondition-failed-error))))) + (testing "existing merge errors" + (doseq [error-type [::doc/json-read-error + ::doc/json-not-object-error + ::doc/invalid-merge]] + (is (= (assoc ctx :response {:status 400}) + (routes/post-response + ctx + {:error (ex-info "Invalid merge" {:type error-type})}))))) + (testing "unexpected implementation error" + (is (= (assoc ctx :io.pedestal.interceptor.chain/error unexpected-error) + (routes/post-response ctx {:error unexpected-error}))))) + +(deftest delete-response-test + (testing "success" + (is (= (assoc ctx :response {:status 204}) + (routes/delete-response ctx {})))) + (testing "single and multiple document precondition failures" + (doseq [operation [:delete-document :delete-documents]] + (is (= (assoc ctx :response {:status 412}) + (routes/delete-response + ctx + (doc/precondition-failed-error {:operation operation})))))) + (testing "unexpected implementation error" + (is (= (assoc ctx :io.pedestal.interceptor.chain/error unexpected-error) + (routes/delete-response ctx {:error unexpected-error}))))) diff --git a/src/test/com/yetanalytics/test_runner.cljc b/src/test/com/yetanalytics/test_runner.cljc index 8236a443..1c5a3201 100644 --- a/src/test/com/yetanalytics/test_runner.cljc +++ b/src/test/com/yetanalytics/test_runner.cljc @@ -15,6 +15,7 @@ com.yetanalytics.lrs.xapi.statements.html-test com.yetanalytics.lrs.impl.memory-test com.yetanalytics.lrs.pedestal.http.multipart-mixed-test + com.yetanalytics.lrs.pedestal.routes.documents-test com.yetanalytics.lrs.auth-test com.yetanalytics.lrs.pedestal.interceptor.xapi.statements.attachment-test com.yetanalytics.lrs.pedestal.interceptor.xapi.statements.attachment.response-test @@ -58,6 +59,7 @@ 'com.yetanalytics.lrs.xapi.statements.html-test 'com.yetanalytics.lrs.impl.memory-test 'com.yetanalytics.lrs.pedestal.http.multipart-mixed-test + 'com.yetanalytics.lrs.pedestal.routes.documents-test 'com.yetanalytics.lrs.auth-test 'com.yetanalytics.lrs.pedestal.interceptor.xapi.statements.attachment-test 'com.yetanalytics.lrs.pedestal.interceptor.xapi.statements.attachment.response-test))) From d2795e35cdf9584ab6f0feab9e70e6d1589d052e Mon Sep 17 00:00:00 2001 From: Milton Reder Date: Mon, 10 Aug 2026 15:35:08 -0400 Subject: [PATCH 2/3] refactor etag helpers and add normalized preconditions to ctx --- .../lrs/pedestal/interceptor.cljc | 6 +- .../lrs/pedestal/routes/documents.cljc | 57 ++++++------ .../com/yetanalytics/lrs/xapi/document.cljc | 50 +++++++++++ .../yetanalytics/lrs/xapi/document_test.cljc | 45 +++++++++- src/test/com/yetanalytics/lrs_test.clj | 86 +++++++++++++++++++ src/test/com/yetanalytics/test_support.cljc | 2 +- 6 files changed, 208 insertions(+), 38 deletions(-) diff --git a/src/main/com/yetanalytics/lrs/pedestal/interceptor.cljc b/src/main/com/yetanalytics/lrs/pedestal/interceptor.cljc index cefed053..f851ebed 100644 --- a/src/main/com/yetanalytics/lrs/pedestal/interceptor.cljc +++ b/src/main/com/yetanalytics/lrs/pedestal/interceptor.cljc @@ -10,6 +10,7 @@ [io.pedestal.http.body-params :as body-params] [io.pedestal.http.ring-middlewares :as middlewares] [com.yetanalytics.lrs.pedestal.interceptor.xapi :as xapi] + [com.yetanalytics.lrs.xapi.document :as doc] [com.yetanalytics.lrs.util.hash :refer [sha-1]] [com.yetanalytics.lrs.pedestal.interceptor.xapi.statements :as si] [xapi-schema.spec :as xs :include-macros true] @@ -182,13 +183,12 @@ (defn calculate-etag [x] (sha-1 x)) -;; TODO: handle weak etags (def etag-string-pattern - #"\w+") + doc/etag-string-pattern) (defn etag-header->etag-set [etag-header] - (into #{} (re-seq etag-string-pattern etag-header))) + (doc/etag-header->etag-set etag-header)) (defn- quote-etag [etag] (str "\"" etag "\"")) diff --git a/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc b/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc index 084ef947..53cf4b22 100644 --- a/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc +++ b/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc @@ -157,36 +157,19 @@ (defn etags-preproc "Process if-match rules and etags for the handler. Will call `handle-get` - to check doc state." + to check doc state, then pass normalized preconditions to the document + implementation." [enter-fn] (fn wrap-enter [{:keys [xapi request com.yetanalytics/lrs] :as ctx}] - (let [;; Destructuring - {:keys [headers]} request - ;; VSCode incorrectly marks `if-match` and `if-none-match` as - ;; if macros - {hif-match "if-match" - hif-none-match "if-none-match"} headers - ;; Helper fns - hif-match-ok? - (fn [ctx hif-match] - (case hif-match - nil true - "*" (= 200 (get-in ctx [:response :status])) - ;; else - (contains? (i/etag-header->etag-set hif-match) - (::i/etag ctx)))) - hif-none-match-ok? - (fn [ctx hif-none-match] - (case hif-none-match - nil true - "*" (= 404 (get-in ctx [:response :status])) - ;; else - (not (contains? (i/etag-header->etag-set hif-none-match) - (::i/etag ctx)))))] - (if (= nil hif-match hif-none-match) + (let [preconditions (doc/parse-etag-preconditions + (get request :headers)) + operation-ctx (cond-> ctx + (seq preconditions) + (assoc ::doc/preconditions preconditions))] + (if (empty? preconditions) ;; If no headers provided, go ahead (enter-fn ctx) (let [;; TODO: Params overhaul, very silly rn @@ -203,10 +186,14 @@ get-params-enter get-enter a/etag-set + "Parse an ETag header value into a set of unquoted ETags." + [etag-header] + (into #{} (re-seq etag-string-pattern etag-header))) + +(s/def ::etag-condition + (s/or :wildcard #{:*} + :etags (s/coll-of string? :kind set?))) + +(s/def ::if-match ::etag-condition) +(s/def ::if-none-match ::etag-condition) + +(s/def ::preconditions + (s/keys :opt-un [::if-match ::if-none-match])) + +(defn parse-etag-preconditions + "Parse If-Match and If-None-Match request headers into normalized + preconditions. Wildcards are represented by `:*`; other values are sets of + unquoted ETags." + [{if-match "if-match" + if-none-match "if-none-match"}] + (cond-> {} + if-match + (assoc :if-match (if (= "*" if-match) + :* + (etag-header->etag-set if-match))) + if-none-match + (assoc :if-none-match (if (= "*" if-none-match) + :* + (etag-header->etag-set if-none-match))))) + +(defn etag-preconditions-met? + "Return true when normalized ETag `preconditions` are satisfied by the + current resource state. `exists?` indicates whether the resource exists; + `etag` is its unquoted ETag when available." + [{:keys [if-match if-none-match]} + {:keys [exists? etag]}] + (and (case if-match + nil true + :* exists? + (contains? if-match etag)) + (case if-none-match + nil true + :* (not exists?) + (not (contains? if-none-match etag))))) + (defn precondition-failed-error "Return a document operation error indicating that an ETag precondition failed. Optional `data` is included in the exception data." diff --git a/src/test/com/yetanalytics/lrs/xapi/document_test.cljc b/src/test/com/yetanalytics/lrs/xapi/document_test.cljc index 7fa9739d..c288a8d5 100644 --- a/src/test/com/yetanalytics/lrs/xapi/document_test.cljc +++ b/src/test/com/yetanalytics/lrs/xapi/document_test.cljc @@ -1,9 +1,52 @@ (ns com.yetanalytics.lrs.xapi.document-test - (:require [clojure.test :refer [deftest is] :include-macros true] + (:require [clojure.test :refer [deftest is testing] :include-macros true] [clojure.spec.test.alpha :as stest :include-macros true] [com.yetanalytics.test-support :refer [failures stc-opts]] [com.yetanalytics.lrs.xapi.document :as doc])) +(deftest parse-etag-preconditions-test + (testing "no preconditions" + (is (= {} (doc/parse-etag-preconditions {})))) + (testing "wildcards" + (is (= {:if-match :* + :if-none-match :*} + (doc/parse-etag-preconditions + {"if-match" "*" + "if-none-match" "*"})))) + (testing "ETag sets" + (is (= {:if-match #{"abc" "def"} + :if-none-match #{"ghi" "jkl"}} + (doc/parse-etag-preconditions + {"if-match" "\"abc\", \"def\"" + "if-none-match" "\"ghi\", \"jkl\""}))))) + +(deftest etag-preconditions-met-test + (testing "no preconditions" + (is (doc/etag-preconditions-met? {} {:exists? false})) + (is (doc/etag-preconditions-met? {} {:exists? true :etag "abc"}))) + (testing "If-Match" + (is (doc/etag-preconditions-met? {:if-match :*} + {:exists? true :etag "abc"})) + (is (not (doc/etag-preconditions-met? {:if-match :*} + {:exists? false}))) + (is (doc/etag-preconditions-met? {:if-match #{"abc"}} + {:exists? true :etag "abc"})) + (is (not (doc/etag-preconditions-met? {:if-match #{"stale"}} + {:exists? true :etag "abc"})))) + (testing "If-None-Match" + (is (doc/etag-preconditions-met? {:if-none-match :*} + {:exists? false})) + (is (not (doc/etag-preconditions-met? {:if-none-match :*} + {:exists? true :etag "abc"}))) + (is (doc/etag-preconditions-met? {:if-none-match #{"stale"}} + {:exists? true :etag "abc"})) + (is (not (doc/etag-preconditions-met? {:if-none-match #{"abc"}} + {:exists? true :etag "abc"})))) + (testing "both headers" + (is (doc/etag-preconditions-met? + {:if-match #{"abc"} :if-none-match #{"stale"}} + {:exists? true :etag "abc"})))) + (deftest updated-inst-test (is (empty? (failures diff --git a/src/test/com/yetanalytics/lrs_test.clj b/src/test/com/yetanalytics/lrs_test.clj index e76cb339..286f9fc5 100644 --- a/src/test/com/yetanalytics/lrs_test.clj +++ b/src/test/com/yetanalytics/lrs_test.clj @@ -3,6 +3,8 @@ [com.yetanalytics.test-support :as support :refer [deftest-check-ns]] [com.yetanalytics.lrs.impl.memory :as mem] [com.yetanalytics.lrs :as lrs] + [com.yetanalytics.lrs.util.hash :as hash] + [com.yetanalytics.lrs.xapi.document :as doc] [clojure.string :as cs] [com.yetanalytics.datasim.input :as sim-input] [com.yetanalytics.datasim.sim :as sim] @@ -284,3 +286,87 @@ "2.0.0" 200) (finally (http/stop lrs)))))) + +(deftest document-etag-precondition-handoff-test + (doseq [[lrs-mode port] [[:sync 8081] + [:async 8082]]] + (testing (str "memory LRS mode " (name lrs-mode)) + (let [server (support/test-server :port port + :lrs-mode lrs-mode) + original-set-document lrs/set-document + original-set-document-async lrs/set-document-async + implementation-calls (atom []) + initial-body "{\"value\":1}" + updated-body "{\"value\":2}" + initial-etag (hash/sha-1 initial-body) + url (str "http://localhost:" port + "/xapi/activities/profile") + query-params {"activityId" "http://example.com/activity" + "profileId" "etag-handoff"} + request-opts (fn [body headers] + {:basic-auth ["username" "password"] + :headers (merge + {"X-Experience-API-Version" + "1.0.3" + "Content-Type" + "application/json"} + headers) + :query-params query-params + :body body + :throw false}) + get-opts {:basic-auth ["username" "password"] + :headers {"X-Experience-API-Version" + "1.0.3"} + :query-params query-params + :throw false}] + (with-redefs + [lrs/set-document + (fn [impl ctx auth-identity params document merge?] + (swap! implementation-calls conj (::doc/preconditions ctx)) + (original-set-document impl + ctx + auth-identity + params + document + merge?)) + lrs/set-document-async + (fn [impl ctx auth-identity params document merge?] + (swap! implementation-calls conj (::doc/preconditions ctx)) + (original-set-document-async impl + ctx + auth-identity + params + document + merge?))] + (try + (http/start server) + (testing "passes normalized If-None-Match wildcard" + (is (= 204 + (:status + (curl/put + url + (request-opts initial-body {"If-None-Match" "*"}))))) + (is (= [{:if-none-match :*}] + @implementation-calls))) + (testing "preliminary check still rejects stale If-Match" + (is (= 412 + (:status + (curl/put + url + (request-opts updated-body + {"If-Match" "\"stale\""}))))) + (is (= 1 (count @implementation-calls))) + (is (= initial-body (:body (curl/get url get-opts))))) + (testing "passes normalized matching If-Match ETag set" + (is (= 204 + (:status + (curl/put + url + (request-opts updated-body + {"If-Match" + (str "\"" initial-etag "\"")}))))) + (is (= {:if-match #{initial-etag}} + (last @implementation-calls))) + (is (= updated-body (:body (curl/get url get-opts))))) + (finally + (http/stop server)))))))) diff --git a/src/test/com/yetanalytics/test_support.cljc b/src/test/com/yetanalytics/test_support.cljc index dfc2067a..19892e5b 100644 --- a/src/test/com/yetanalytics/test_support.cljc +++ b/src/test/com/yetanalytics/test_support.cljc @@ -75,7 +75,7 @@ :or {port 8080 lrs-mode :sync route-opts {}}}] - (let [lrs (mem/new-lrs {}) + (let [lrs (mem/new-lrs {:mode lrs-mode}) service {:env :dev ::lrs lrs ::http/routes (r/build From 237a01282a1600d3ac6fac7a139d46cbe6294f8c Mon Sep 17 00:00:00 2001 From: Milton Reder Date: Thu, 13 Aug 2026 11:23:20 -0400 Subject: [PATCH 3/3] let implementations declare atomic powers --- .../lrs/pedestal/routes/documents.cljc | 14 ++-- src/main/com/yetanalytics/lrs/protocol.cljc | 16 +++++ .../yetanalytics/lrs/impl/memory_test.cljc | 9 ++- .../lrs/pedestal/routes/documents_test.cljc | 68 +++++++++++++++++++ 4 files changed, 99 insertions(+), 8 deletions(-) diff --git a/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc b/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc index 53cf4b22..837adcb5 100644 --- a/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc +++ b/src/main/com/yetanalytics/lrs/pedestal/routes/documents.cljc @@ -156,9 +156,9 @@ :xapi.agents.profile.GET.request/params)))) (defn etags-preproc - "Process if-match rules and etags for the handler. Will call `handle-get` - to check doc state, then pass normalized preconditions to the document - implementation." + "Normalize ETag preconditions for document mutations. Implementations that + opt into atomic validation receive them directly; other implementations use + the preliminary `handle-get` check before receiving them." [enter-fn] (fn wrap-enter [{:keys [xapi @@ -169,9 +169,11 @@ operation-ctx (cond-> ctx (seq preconditions) (assoc ::doc/preconditions preconditions))] - (if (empty? preconditions) - ;; If no headers provided, go ahead - (enter-fn ctx) + (if (or (empty? preconditions) + (p/atomic-document-preconditions? lrs)) + ;; No condition to validate, or the implementation validates it + ;; authoritatively while applying the mutation. + (enter-fn operation-ctx) (let [;; TODO: Params overhaul, very silly rn get-params-enter (get-params-enter-fn xapi) {get-enter :enter diff --git a/src/main/com/yetanalytics/lrs/protocol.cljc b/src/main/com/yetanalytics/lrs/protocol.cljc index a01983e2..78be3831 100644 --- a/src/main/com/yetanalytics/lrs/protocol.cljc +++ b/src/main/com/yetanalytics/lrs/protocol.cljc @@ -114,6 +114,22 @@ (s/def ::document-resource-instance document-resource?) +(defprotocol AtomicDocumentPreconditions + "Optional capability for document implementations that atomically validate + ETag preconditions while applying mutations." + (-atomic-document-preconditions? [this] + "Return true when document mutation preconditions are validated atomically + by this implementation.")) + +(defn atomic-document-preconditions? + "Return true when `lrs` opts into authoritative atomic document precondition + validation. Implementations that do not implement the optional capability + return false." + [lrs] + (boolean + (and (satisfies? AtomicDocumentPreconditions lrs) + (-atomic-document-preconditions? lrs)))) + (s/def ::set-document-params (s/or :state (sc/with-conform-gen :xapi.document.state/id-params) diff --git a/src/test/com/yetanalytics/lrs/impl/memory_test.cljc b/src/test/com/yetanalytics/lrs/impl/memory_test.cljc index cdce7574..9fdfa3c7 100644 --- a/src/test/com/yetanalytics/lrs/impl/memory_test.cljc +++ b/src/test/com/yetanalytics/lrs/impl/memory_test.cljc @@ -2,7 +2,8 @@ (:require [clojure.test :as test :refer [deftest is] :include-macros true] [clojure.spec.test.alpha :as stest :include-macros true] [com.yetanalytics.test-support :refer [failures stc-opts]] - [com.yetanalytics.lrs.impl.memory :as mem])) + [com.yetanalytics.lrs.impl.memory :as mem] + [com.yetanalytics.lrs.protocol :as p])) (deftest store-ref-test (is (empty? @@ -95,4 +96,8 @@ (is (empty? (failures (stest/check `mem/new-lrs - {stc-opts {:num-tests 1}}))))) + {stc-opts {:num-tests 1}})))) + (doseq [mode [:sync :async :both]] + (is (false? (p/atomic-document-preconditions? + (mem/new-lrs {:mode mode}))) + (str "memory LRS mode " (name mode) " remains unopted")))) diff --git a/src/test/com/yetanalytics/lrs/pedestal/routes/documents_test.cljc b/src/test/com/yetanalytics/lrs/pedestal/routes/documents_test.cljc index 24685ed6..5cecce1e 100644 --- a/src/test/com/yetanalytics/lrs/pedestal/routes/documents_test.cljc +++ b/src/test/com/yetanalytics/lrs/pedestal/routes/documents_test.cljc @@ -1,6 +1,8 @@ (ns com.yetanalytics.lrs.pedestal.routes.documents-test (:require [clojure.test :refer [deftest is testing] :include-macros true] + [clojure.core.async :as a :include-macros true] [clojure.spec.alpha :as s :include-macros true] + [com.yetanalytics.test-support :as support] [com.yetanalytics.lrs.pedestal.routes.documents :as routes] [com.yetanalytics.lrs.protocol :as p] [com.yetanalytics.lrs.xapi.document :as doc])) @@ -11,6 +13,72 @@ (def unexpected-error (ex-info "Unexpected document error" {:type ::unexpected})) +(defn- atomic-sync-lrs + [enabled?] + (reify + p/AtomicDocumentPreconditions + (-atomic-document-preconditions? [_] + enabled?) + p/DocumentResource + (-set-document [_ _ _ _ _ _] {}) + (-get-document [_ _ _ _] + (throw (ex-info "Preliminary GET must be skipped" {}))) + (-get-document-ids [_ _ _ _] + (throw (ex-info "Preliminary GET must be skipped" {}))) + (-delete-document [_ _ _ _] {}) + (-delete-documents [_ _ _ _] {}))) + +(defn- atomic-async-lrs + [enabled?] + (reify + p/AtomicDocumentPreconditions + (-atomic-document-preconditions? [_] + enabled?) + p/DocumentResourceAsync + (-set-document-async [_ _ _ _ _ _] (a/go {})) + (-get-document-async [_ _ _ _] + (throw (ex-info "Preliminary GET must be skipped" {}))) + (-get-document-ids-async [_ _ _ _] + (throw (ex-info "Preliminary GET must be skipped" {}))) + (-delete-document-async [_ _ _ _] (a/go {})) + (-delete-documents-async [_ _ _ _] (a/go {})))) + +(deftest atomic-document-preconditions-capability-test + (testing "unimplemented and disabled capabilities are false" + (is (false? (p/atomic-document-preconditions? nil))) + (is (false? (p/atomic-document-preconditions? + (atomic-sync-lrs false))))) + (testing "enabled synchronous and asynchronous capabilities are true" + (is (true? (p/atomic-document-preconditions? + (atomic-sync-lrs true)))) + (is (true? (p/atomic-document-preconditions? + (atomic-async-lrs true)))))) + +(deftest atomic-etag-precondition-handoff-test + (let [preconditions {:if-match #{"abc" "def"} + :if-none-match :*} + request {:headers {"if-match" "\"abc\", \"def\"" + "if-none-match" "*"}} + enter-fn (fn [ctx] + (assoc ctx :response + {:preconditions (::doc/preconditions ctx)}))] + (testing "synchronous implementation skips preliminary GET" + (let [result ((routes/etags-preproc enter-fn) + {:request request + :com.yetanalytics/lrs (atomic-sync-lrs true)})] + (is (= preconditions + (get-in result [:response :preconditions]))))) + (testing "asynchronous implementation skips preliminary GET" + (support/test-async + (a/go + (let [result (a/