Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

LRS-30 Support LRS 2.0 #22

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

LRS-30 Support LRS 2.0 #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c1d65c2
LRS-30 #comment passing LRS-2.0 branch of conformance tests in clj + …
milt May 5, 2021
8928bed
LRS-30 #comment remove hack for incorrect headers in tests
milt Jul 19, 2021
202c415
Merge branch 'master' into LRS-30_2_0_compat
milt Jul 19, 2021
b573259
Merge branch 'master' into LRS-30_2_0_compat
milt Aug 9, 2021
e37b000
LRS-30 working in clj but now failing in cljs
milt Aug 9, 2021
59f8fe9
LRS-30 proper lower casing of header to fix cljs
milt Aug 9, 2021
607337e
Merge branch 'master' into LRS-30_2_0_compat
milt Aug 16, 2021
3fccf86
Merge branch 'master' into LRS-30_2_0_compat
milt Sep 2, 2021
1a07703
new version of test runner with branch selection
milt Sep 2, 2021
5e490e2
LRS-30 update conf test up checker
milt Sep 22, 2021
99a51e8
Merge branch 'master' into LRS-30_2_0_compat
milt Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions deps.edn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
org.clojure/clojurescript {:mvn/version "1.10.741"}
org.clojure/core.async {:mvn/version "1.1.587"}
com.yetanalytics/xapi-schema {:git/url "/~https://github.com/yetanalytics/xapi-schema.git"
:sha "4b97122d839e895b9111e6ed62ae68bc0fdee723"
:sha "92876f9e7c9d0df69db27c7e268d9517a880591b"
:exclusions [org.clojure/clojurescript]}
cheshire/cheshire {:mvn/version "5.10.0"}
io.pedestal/pedestal.service {:mvn/version "0.5.9"}
Expand Down Expand Up @@ -38,7 +38,7 @@
:sha "423bd6c301ce8503f5c18b43df098bbe64f8f1ab"}
com.yetanalytics/lrs-test-runner
{:git/url "/~https://github.com/yetanalytics/lrs-test-runner.git"
:sha "0fcd42854f9c79a043c13d436d629826bfc5133d"}}}
:sha "9caedae0fe5d619651f8b54fa669cec363475fef"}}}
;; bench utils for LRS implementations
:bench
{:extra-paths ["src/bench"]
Expand Down
4 changes: 3 additions & 1 deletion src/main/com/yetanalytics/lrs/impl/memory.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,7 @@
:params :xapi.document.state/context-params)
:ret :state/documents)


;; Operations on in-mem state.
;; Shorten document to doc here to avoid conflicts with existing

Expand Down Expand Up @@ -858,7 +859,8 @@
:body {:version ["1.0.0"
"1.0.1"
"1.0.2"
"1.0.3"]}})
"1.0.3"
"2.0.0"]}})

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Putting It All Together
Expand Down
100 changes: 45 additions & 55 deletions src/main/com/yetanalytics/lrs/pedestal/interceptor.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,55 +27,43 @@
:enter (fn [ctx]
(assoc ctx :com.yetanalytics/lrs lrs))}))

(def xAPIVersionRegEx
(let [suf-part "[0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*"
suffix (str "(\\.[0-9]+(?:-" suf-part ")?(?:\\+" suf-part ")?)?")
ver-str (str "^[1-2]\\.0" suffix "$")]
(re-pattern ver-str)))

(def require-xapi-version-interceptor
(i/interceptor
{:name ::require-xapi-version
:enter
(fn require-xapi-version [ctx]
;; if this is an html request, don't require this
;; browsers can't provide it
(if (si/accept-html? ctx)
(assoc-in ctx
[:request :headers "x-experience-api-version"]
"1.0.3")
(if-let [version-header
(get-in ctx [:request :headers "x-experience-api-version"])]
;; Per spec, if we accept 1.0.0,
;; 1.0 must be accepted as if 1.0.0
(if (#{"1.0"
"1.0.0"
"1.0.1"
"1.0.2"
"1.0.3"} version-header)
;; Version ok
ctx
;; Version not ok
(assoc
(chain/terminate ctx)
:response
#?(:cljs
{:status 400
:headers {"Content-Type" "application/json"}
:body
{:error {:message "X-Experience-API-Version header invalid!"}}}
;; TODO: Figure this out and dix
;; this is odd. For some reason, the conformance
;; tests intermittently fail when this error response
;; comes in w/o content-length. So we string it out
;; and set it. Who knows.
:clj
{:status 400
:headers {"Content-Type" "application/json"
"Content-Length" "66"}
:body "{\"error\": {\"message\": \"X-Experience-API-Version header invalid!\"}}"})))
(assoc
(chain/terminate ctx)
:response
{:status 400
:headers {"Content-Type" "application/json"}
:body
{:error
{:message "X-Experience-API-Version header required!"}}}))))}))
{:name ::require-xapi-version
:enter (fn [ctx]
;; if this is an html request, don't require this
;; browsers can't provide it
(if (si/accept-html? ctx)
(assoc-in ctx
[:request :headers "x-experience-api-version"]
"2.0.0")
(if-let [version-header (get-in ctx [:request :headers "x-experience-api-version"])]
(if (re-matches xAPIVersionRegEx
version-header)
(assoc ctx :com.yetanalytics.lrs/version version-header)
(assoc (chain/terminate ctx)
:response
{:status 400
:headers {#?(:cljs "Content-Type"
:clj "content-type")
"application/json"
"x-experience-api-version" "2.0.0"}
:body
{:error {:message "X-Experience-API-Version header invalid!"}}}))
(assoc (chain/terminate ctx)
:response
{:status 400
:headers {#?(:cljs "Content-Type"
:clj "content-type") "application/json"
"x-experience-api-version" "2.0.0"}
:body
{:error {:message "X-Experience-API-Version header required!"}}}))))}))

(def x-forwarded-for-interceptor
(i/interceptor
Expand Down Expand Up @@ -162,11 +150,12 @@
;; Leave
(def set-xapi-version-interceptor
(i/interceptor
{:name ::set-xapi-version
:leave (fn set-xapi-version [ctx]
{:name ::set-xapi-version
:leave (fn [ctx]
(assoc-in ctx
[:response :headers "X-Experience-API-Version"]
"1.0.3"))}))
[:response :headers "x-experience-api-version"]
;; Should be latest patch version
"2.0.0"))}))

(defn calculate-etag [x]
(sha-1 x))
Expand Down Expand Up @@ -196,7 +185,7 @@
(update-in [:response :headers] merge {"ETag" (quote-etag etag)}))))

;; Combo
(def require-and-set-xapi-version-interceptor
#_(def require-and-set-xapi-version-interceptor
(i/interceptor
(merge
require-xapi-version-interceptor
Expand Down Expand Up @@ -402,9 +391,9 @@
;; The etag interceptor may mess with routes, so it's important not to have any
;; important leave stuff after it in the defaults
;; TODO: If all platforms support async/NIO responses, we can bring this back

;; (not (nil? resource-path)) (conj (middlewares/fast-resource resource-path))

#?@(:clj
[(some? resource-path) (conj (middlewares/resource resource-path))
(some? file-path) (conj (middlewares/file file-path))])
Expand All @@ -422,12 +411,13 @@
http/json-body
error-interceptor
body-params
xapi/alternate-request-syntax-interceptor
;; xapi/alternate-request-syntax-interceptor
set-xapi-version-interceptor
xapi-ltags-interceptor]))

(def doc-interceptors-base
[x-forwarded-for-interceptor
require-xapi-version-interceptor
xapi/alternate-request-syntax-interceptor
set-xapi-version-interceptor
xapi-ltags-interceptor])
Expand Down
156 changes: 82 additions & 74 deletions src/main/com/yetanalytics/lrs/pedestal/interceptor/xapi.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,80 +31,88 @@

(def alternate-request-syntax-interceptor
{:name ::alternate-request-syntax-interceptor
:enter
(fn alternate-request-syntax-interceptor-fn
[{:keys [request] :as ctx}]
(if (some-> request :params :method)
;; If this is a request with a smuggled verb
(if (not= :post (:original-request-method request))
(assoc (chain/terminate ctx)
:response
{:status 400
:body
{:error
{:message "xAPI alternate request syntax must use POST!"}}})
(if (some-> ctx :request :query-params not-empty)
;; We can't have extra query params
(assoc (chain/terminate ctx)
:response
{:status 400
:body
{:error
{:message "xAPI Alternate Syntax does not allow extra query params."}}})
(let [;; Destructuring
{:keys [form-params]} request
{:keys [content]} form-params
{:strs [content-type
content-length]
:as form-headers}
(->> valid-alt-request-headers
(select-keys (:form-params request))
(map #(update % 0 (comp cstr/lower-case name)))
(reduce conj {}))
;; Content + Params
new-params (apply dissoc
form-params
:content ; don't let content in
valid-alt-request-headers)
?content-type (or content-type
(and content
"application/json"))
?content-bytes #?(:clj (when content
(.getBytes ^String content "UTF-8"))
:cljs nil)
?content-length (or content-length
#?(:clj (and ?content-bytes
(count ?content-bytes))
:cljs (and content
(.-length content))))
;; Dummy binding to avoid clj-kondo warning, since
;; ?content-bytes is only used in clj
_ ?content-bytes
;; Corece request
request' (-> request
(dissoc :form-params)
(update :headers merge form-headers)
;; replace params with the other form params
(assoc :params new-params))
request'' (cond-> request'
?content-type
(assoc :content-type ?content-type)
?content-length
(assoc :content-length ?content-length)
;; If there's content, make it an input stream
#?(:clj ?content-bytes :cljs content)
(assoc :body #?(:clj (ByteArrayInputStream.
^bytes ?content-bytes)
:cljs content)))
;; TODO: Need to figure out body-params in cljs
parser-m #?(:clj (body-params/default-parser-map
:json-options {:key-fn str})
:cljs (body-params/default-parser-map))]
(cond-> (assoc ctx :request request'')
content
(execute-next (body-params/body-params parser-m))))))
;; No smuggled verb
ctx))})
:enter (fn [{:keys [request] :as ctx}]
;; If this is a request with a smuggled verb
;; and version is pre-2.0.x
(if (some-> request :params :method)
;; Version 2.0.0 and up do not allow this so we error
(if (.startsWith
^String (:com.yetanalytics.lrs/version
ctx)
"2.0")
(assoc (chain/terminate ctx)
:response
{:status 400
:body {:error
{:message "xAPI alternate request syntax not supported"}}})
(if (not= :post (:original-request-method request))
(assoc (chain/terminate ctx)
:response
{:status 400
:body
{:error
{:message "xAPI alternate request syntax must use POST!"}}})
(if (some-> ctx :request :query-params not-empty)
;; We can't have extra query params
(assoc (chain/terminate ctx)
:response
{:status 400
:body
{:error
{:message "xAPI Alternate Syntax does not allow extra query params."}}})
(let [;; Destructuring
{:keys [form-params]} request
{:keys [content]} form-params
{:strs [content-type
content-length]
:as form-headers}
(->> valid-alt-request-headers
(select-keys (:form-params request))
(map #(update % 0 (comp cstr/lower-case name)))
(reduce conj {}))
;; Content + Params
new-params (apply dissoc
form-params
:content ; don't let content in
valid-alt-request-headers)
?content-type (or content-type
(and content
"application/json"))
?content-bytes #?(:clj (when content
(.getBytes ^String content "UTF-8"))
:cljs nil)
?content-length (or content-length
#?(:clj (and ?content-bytes
(count ?content-bytes))
:cljs (and content
(.-length content))))
;; Dummy binding to avoid clj-kondo warning, since
;; ?content-bytes is only used in clj
_ ?content-bytes
;; Corece request
request' (-> request
(dissoc :form-params)
(update :headers merge form-headers)
;; replace params with the other form params
(assoc :params new-params))
request'' (cond-> request'
?content-type
(assoc :content-type ?content-type)
?content-length
(assoc :content-length ?content-length)
;; If there's content, make it an input stream
#?(:clj ?content-bytes :cljs content)
(assoc :body #?(:clj (ByteArrayInputStream.
^bytes ?content-bytes)
:cljs content)))
;; TODO: Need to figure out body-params in cljs
parser-m #?(:clj (body-params/default-parser-map
:json-options {:key-fn str})
:cljs (body-params/default-parser-map))]
(cond-> (assoc ctx :request request'')
content
(execute-next (body-params/body-params parser-m)))))))
ctx))})

(defn conform-cheshire [spec-kw x]
#?(:clj (binding [xsr/*read-json-fn* json/parse-string-strict
Expand Down
4 changes: 2 additions & 2 deletions src/main/com/yetanalytics/lrs/xapi/statements.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@

(defn prepare-statement
"Assign an ID, stored, timestamp, etc prior to storage"
[{:strs [id stored timestamp _version] :as statement}]
[{:strs [id stored timestamp version] :as statement}]
(let [id (or id (str #?(:clj (java.util.UUID/randomUUID)
:cljs (random-uuid))))
stored (or stored (now-stamp))
Expand All @@ -166,7 +166,7 @@
"stored" stored
"timestamp" timestamp
"authority" authority
"version" "1.0.3"))))
"version" (or version "2.0.0")))))

(s/fdef prepare-statement
:args (s/cat :statement ::xs/statement)
Expand Down
2 changes: 1 addition & 1 deletion src/test/com/yetanalytics/conformance_test/clj/async.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
[mem-lrs.server :as server]
[io.pedestal.http :as http]))

(use-fixtures :once runner/test-suite-fixture)
(use-fixtures :once #(runner/test-suite-fixture % :branch "LRS-2.0"))

(deftest test-lrs-async
(testing "clj/java async"
Expand Down
2 changes: 1 addition & 1 deletion src/test/com/yetanalytics/conformance_test/clj/sync.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
[mem-lrs.server :as server]
[io.pedestal.http :as http]))

(use-fixtures :once runner/test-suite-fixture)
(use-fixtures :once #(runner/test-suite-fixture % :branch "LRS-2.0"))

(deftest test-lrs-sync
(testing "clj/java sync"
Expand Down
4 changes: 2 additions & 2 deletions src/test/com/yetanalytics/conformance_test/cljs.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
[]
(try
(= (slurp "http://localhost:8080/xapi/about")
"{\"version\":[\"1.0.0\",\"1.0.1\",\"1.0.2\",\"1.0.3\"]}")
"{\"version\":[\"1.0.0\",\"1.0.1\",\"1.0.2\",\"1.0.3\",\"2.0.0\"]}")
(catch java.net.ConnectException _
false)))

Expand Down Expand Up @@ -42,7 +42,7 @@
{:type ::cljs-lrs-start-error
:ret ret})))))

(use-fixtures :once runner/test-suite-fixture)
(use-fixtures :once #(runner/test-suite-fixture % :branch "LRS-2.0"))

(deftest test-cljs-lrs
(testing "cljs/javascript async"
Expand Down