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.

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

[SQL-281] CSV export #464

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a2c3ea1
Extract out query-many-statements helper function
kelvinqian00 Feb 11, 2025
6659d85
Additional refactors to statement query
kelvinqian00 Feb 11, 2025
e2c6079
Move path-related util functions to their own ns
kelvinqian00 Feb 11, 2025
92471f6
Make changes to path->jsonpath functions
kelvinqian00 Feb 11, 2025
994da69
Add required deps
kelvinqian00 Feb 11, 2025
9fe638a
Add query-all-statements function
kelvinqian00 Feb 11, 2025
247b66c
Add statements to CSV util functions
kelvinqian00 Feb 11, 2025
d60579a
Implement get-statements-csv protocol function
kelvinqian00 Feb 11, 2025
df178d6
Remove unused require
kelvinqian00 Feb 11, 2025
40530f4
Only put quotes in SQL JSON path strings when needed
kelvinqian00 Feb 12, 2025
8d06741
Change some arg names
kelvinqian00 Feb 12, 2025
1c742b3
Implement CSV download interceptor and endpoint
kelvinqian00 Feb 12, 2025
7ec555a
Disable JWT validation for /admin/csv endpoint
kelvinqian00 Feb 14, 2025
5fce0e3
Use next.jdbc/plan and test queries beyond :limit
kelvinqian00 Feb 14, 2025
131e2ef
Delete now-unused requires
kelvinqian00 Feb 14, 2025
ae04282
Add docstrings, specs, and refactors to query fn
kelvinqian00 Feb 14, 2025
d311b07
Add with-open clause to interceptor
kelvinqian00 Feb 17, 2025
e766c21
Make :limit an optional param so it can be dissoc'd
kelvinqian00 Feb 17, 2025
8ea1204
Remove stray semicolon from PG query
kelvinqian00 Feb 17, 2025
27bb811
Reinstate transaction in get-statements-csv
kelvinqian00 Feb 17, 2025
379933a
Extract bench inputs to support namespace
kelvinqian00 Feb 18, 2025
0bb7191
Add tests for inserting and querying large amounts of data
kelvinqian00 Feb 18, 2025
83e7504
Move encoding functions to util namespace
kelvinqian00 Feb 18, 2025
ce400e1
Refactor query-statements-stream a bit
kelvinqian00 Feb 18, 2025
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
7 changes: 3 additions & 4 deletions deps.edn
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
org.clojure/clojure {:mvn/version "1.11.2"}
org.clojure/tools.logging {:mvn/version "1.1.0"}
org.clojure/core.memoize {:mvn/version "1.0.250"}
org.clojure/data.csv {:mvn/version "1.1.0"}
clojure-interop/java.security {:mvn/version "1.0.5"}
org.clojure/core.async {:mvn/version "1.6.681"}
;; Util deps
Expand Down Expand Up @@ -49,13 +50,11 @@
less-awful-ssl/less-awful-ssl {:mvn/version "1.0.6"}
xyz.capybara/clamav-client {:mvn/version "2.1.2"}
;; Yet Analytics deps

com.yetanalytics/lrs
{:mvn/version "1.3.0"
:exclusions [org.clojure/clojure
org.clojure/clojurescript
com.yetanalytics/xapi-schema]}

com.yetanalytics/xapi-schema
{:mvn/version "1.4.0"
:exclusions [org.clojure/clojure
Expand All @@ -64,15 +63,15 @@
{:mvn/version "0.1.4"
:exclusions [org.clojure/clojure
org.clojure/clojurescript]}
com.yetanalytics/pathetic
{:mvn/version "0.5.0"}
com.yetanalytics/pedestal-oidc
{:mvn/version "0.0.8"
:exclusions [org.clojure/clojure
buddy/buddy-sign]}

com.yetanalytics/lrs-reactions
{:mvn/version "0.0.1"
:exclusions [org.clojure/clojure]}

com.yetanalytics/gen-openapi
{:mvn/version "0.0.4"
:exclusions [org.clojure/clojure
Expand Down
9 changes: 9 additions & 0 deletions src/db/postgres/lrsql/postgres/record.clj
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
(ns lrsql.postgres.record
(:require [com.stuartsierra.component :as cmp]
[hugsql.core :as hug]
[next.jdbc :as jdbc]
[lrsql.backend.data :as bd]
[lrsql.backend.protocol :as bp]
[lrsql.init :refer [init-hugsql-adapter!]]
Expand All @@ -18,6 +19,8 @@
(hug/def-db-fns "lrsql/postgres/sql/update.sql")
(hug/def-db-fns "lrsql/postgres/sql/delete.sql")

(hug/def-sqlvec-fns "lrsql/postgres/sql/query.sql")

;; Define record
#_{:clj-kondo/ignore [:unresolved-symbol]} ; Shut up VSCode warnings
(defrecord PostgresBackend [tuning]
Expand Down Expand Up @@ -102,6 +105,12 @@
(query-statement-exists tx input))
(-query-statement-descendants [_ tx input]
(query-statement-descendants tx input))
(-query-statements-lazy [_ tx input]
(let [sqlvec (query-statements-sqlvec input)]
(jdbc/plan tx sqlvec {:fetch-size 4000
:concurrency :read-only
:cursors :close
:result-type :forward-only})))

bp/ActorBackend
(-insert-actor! [_ tx input]
Expand Down
6 changes: 3 additions & 3 deletions src/db/postgres/lrsql/postgres/sql/query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ WHERE stmt.is_voided = FALSE
--~ (when (:registration params) "AND stmt.registration = :registration")
--~ (when (:authority-ifis params) "AND :frag:postgres-auth-subquery")
--~ (if (:ascending? params) "ORDER BY stmt.id ASC" "ORDER BY stmt.id DESC")
LIMIT :limit
--~ (when (:limit params) "LIMIT :limit")

/* Note: We sort by both the PK and statement ID in order to force the query
planner to avoid scanning on `stmt_a.id` first, which is much slower than
Expand All @@ -92,7 +92,7 @@ WHERE stmt_a.is_voided = FALSE
--~ (when (:authority-ifis params) "AND :frag:postgres-auth-subquery")
/*~ (if (:ascending? params) "ORDER BY (stmt_a.id, stmt_a.statement_id) ASC"
"ORDER BY (stmt_a.id, stmt_a.statement_id) DESC") ~*/
LIMIT :limit
--~ (when (:limit params) "LIMIT :limit")

-- :name query-statements
-- :command :query
Expand All @@ -107,7 +107,7 @@ FROM (
(:frag:postgres-stmt-ref-subquery-frag))
AS all_stmt
--~ (if (:ascending? params) "ORDER BY all_stmt.id ASC" "ORDER BY all_stmt.id DESC")
LIMIT :limit;
--~ (when (:limit params) "LIMIT :limit")

/* Statement Object Queries */

Expand Down
13 changes: 9 additions & 4 deletions src/db/sqlite/lrsql/sqlite/record.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
(:require [clojure.tools.logging :as log]
[com.stuartsierra.component :as cmp]
[hugsql.core :as hug]
[next.jdbc :as jdbc]
[lrsql.backend.protocol :as bp]
[lrsql.backend.data :as bd]
[lrsql.init :refer [init-hugsql-adapter!]]
[lrsql.sqlite.data :as sd]
[lrsql.util.reaction :as ru])
[lrsql.util.path :refer [path->sqlpath-string]])
(:import [org.sqlite SQLiteException SQLiteErrorCode]))

;; Init HugSql functions
Expand All @@ -19,6 +20,8 @@
(hug/def-db-fns "lrsql/sqlite/sql/update.sql")
(hug/def-db-fns "lrsql/sqlite/sql/delete.sql")

(hug/def-sqlvec-fns "lrsql/sqlite/sql/query.sql")

;; Schema Update Helpers

#_{:clj-kondo/ignore [:unresolved-symbol]}
Expand Down Expand Up @@ -135,6 +138,9 @@
(query-statement-exists tx input))
(-query-statement-descendants [_ tx input]
(query-statement-descendants tx input))
(-query-statements-lazy [_ tx input]
(let [sqlvec (query-statements-sqlvec input)]
(jdbc/plan tx sqlvec)))

bp/ActorBackend
(-insert-actor! [_ tx input]
Expand All @@ -151,7 +157,6 @@
(delete-actor-agent-profile tx input)
(delete-actor-state-document tx input)
(delete-actor-actor tx input))

(-query-actor [_ tx input]
(query-actor tx input))

Expand Down Expand Up @@ -313,7 +318,7 @@
(-error-reaction! [_ tx params]
(error-reaction! tx params))
(-snip-json-extract [_ params]
(snip-json-extract (update params :path ru/path->string)))
(snip-json-extract (update params :path path->sqlpath-string)))
(-snip-val [_ params]
(snip-val params))
(-snip-col [_ params]
Expand All @@ -327,7 +332,7 @@
(-snip-not [_ params]
(snip-not params))
(-snip-contains [_ params]
(snip-contains (update params :path ru/path->string)))
(snip-contains (update params :path path->sqlpath-string)))
(-snip-query-reaction [_ params]
(snip-query-reaction params))
(-query-reaction [_ tx params]
Expand Down
3 changes: 1 addition & 2 deletions src/db/sqlite/lrsql/sqlite/sql/query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ TRUE
--~ (when (:authority-ifis params) "AND :frag:sqlite-auth-ref-subquery")
))
--~ (if (:ascending? params) "ORDER BY stmt.id ASC" "ORDER BY stmt.id DESC")
LIMIT :limit

--~ (when (:limit params) "LIMIT :limit")

/* Statement Object Queries */

Expand Down
67 changes: 66 additions & 1 deletion src/main/lrsql/admin/interceptors/lrs_management.clj
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
(ns lrsql.admin.interceptors.lrs-management
(:require [clojure.spec.alpha :as s]
[clojure.edn :as edn]
[clojure.java.io :as io]
[io.pedestal.interceptor :refer [interceptor]]
[io.pedestal.interceptor.chain :as chain]
[lrsql.admin.protocol :as adp]
[lrsql.spec.admin :as ads]))
[lrsql.spec.admin :as ads]
[com.yetanalytics.lrs.pedestal.interceptor.xapi :as i-xapi]
[com.yetanalytics.lrs-reactions.spec :as rs])
(:import [javax.servlet ServletOutputStream]))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Actor Delete
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(def validate-delete-actor-params
(interceptor
Expand Down Expand Up @@ -31,3 +40,59 @@
(assoc ctx
:response {:status 200
:body params})))}))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; CSV Download
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(def validate-property-paths
(interceptor
{:name ::validate-property-paths
:enter
(fn validate-property-paths [ctx]
(let [property-paths (-> ctx
(get-in [:request
:params
:property-paths])
edn/read-string)]
(if-some [e (s/explain-data (s/every ::rs/path) property-paths)]
(assoc (chain/terminate ctx)
:response
{:status 400
:body {:error (format "Invalid property paths:\n%s"
(-> e s/explain-out with-out-str))}})
;; Need to dissoc since lrs.pedestal.interceptor.xapi/params-interceptor
;; restricts allowed keys in the query param map.
(-> ctx
(update-in [:request :params] dissoc :property-paths)
(update-in [:request :query-params] dissoc :property-paths)
(assoc-in [:request :property-paths] property-paths)))))}))

(def validate-query-params
(interceptor
(i-xapi/params-interceptor :xapi.statements.GET.request/params)))

(def csv-response-header
{"Content-Type" "text/csv"
"Content-Disposition" "attachment"})

(def download-statement-csv
(interceptor
{:name ::download-statement-csv
:enter
(fn download-statement-csv [ctx]
(let [{lrs :com.yetanalytics/lrs
request :request}
ctx
{:keys [property-paths query-params]}
request]
(assoc ctx
:response
{:status 200
:headers csv-response-header
:body (fn [^ServletOutputStream os]
(with-open [writer (io/writer os)]
(adp/-get-statements-csv lrs
writer
property-paths
query-params)))})))}))
9 changes: 8 additions & 1 deletion src/main/lrsql/admin/protocol.clj
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,11 @@
"Soft-delete a reaction."))

(defprotocol AdminLRSManager
(-delete-actor [this params]))
(-delete-actor [this params]
"Delete actor by `:actor-id`")
(-get-statements-csv [this writer property-paths params]
"Retrieve statements by CSV. Instead of returning a sequence of
statements, streams them to `writer` as a side effect, in order to
avoid storing them in memory. `property-paths` are defined in the
Reactions API, while `params` are the same query params for
`-get-statements`."))
12 changes: 10 additions & 2 deletions src/main/lrsql/admin/routes.clj
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,21 @@
ri/delete-reaction)
:route-name :lrsql.admin.reaction/delete]})

(defn admin-lrs-management-routes [common-interceptors jwt-secret jwt-leeway no-val-opts]
(defn admin-lrs-management-routes
[common-interceptors jwt-secret jwt-leeway no-val-opts]
#{["/admin/agents" :delete (conj common-interceptors
lm/validate-delete-actor-params
(ji/validate-jwt jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
lm/delete-actor)
:route-name :lrsql.lrs-management/delete-actor]})
:route-name :lrsql.lrs-management/delete-actor]
["/admin/csv" :get (conj common-interceptors
lm/validate-property-paths
lm/validate-query-params
#_(ji/validate-jwt jwt-secret jwt-leeway no-val-opts)
#_ji/validate-jwt-account
lm/download-statement-csv)
:route-name :lrsql.lrs-management/download-csv]})

(defn add-admin-routes
"Given a set of routes `routes` for a default LRS implementation,
Expand Down
1 change: 1 addition & 0 deletions src/main/lrsql/backend/protocol.clj
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
;; Queries
(-query-statement [this tx input])
(-query-statements [this tx input])
(-query-statements-lazy [this tx input])
(-query-statement-exists [this tx input])
(-query-statement-descendants [this tx input]))

Expand Down
Loading
Loading