From ccb7c466bf28e6e29fabb91a84168591f3c35b38 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Sat, 19 Nov 2022 11:02:34 +0100 Subject: [PATCH] :tada: Add lazy loading and storage/pointer-map support on viewer --- backend/src/app/rpc/commands/files.clj | 25 ++++--- backend/src/app/rpc/commands/viewer.clj | 68 +++++++++---------- backend/src/app/srepl/main.clj | 5 ++ .../test/backend_tests/rpc_viewer_test.clj | 2 - frontend/src/app/main/data/viewer.cljs | 65 +++++++++++------- frontend/src/app/main/data/workspace.cljs | 3 +- .../app/main/ui/workspace/shapes/frame.cljs | 2 +- 7 files changed, 98 insertions(+), 72 deletions(-) diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index 1d02eace1..29bb36a9c 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -116,6 +116,7 @@ :can-edit (or is-owner is-admin can-edit) :can-read true :is-logged (some? profile-id)}))) + ([conn profile-id file-id share-id] (let [perms (get-permissions conn profile-id file-id) ldata (retrieve-share-link conn file-id share-id)] @@ -128,6 +129,7 @@ (some? perms) perms (some? ldata) {:type :share-link :can-read true + :pages (:pages ldata) :is-logged (some? profile-id) :who-comment (:who-comment ldata) :who-inspect (:who-inspect ldata)})))) @@ -212,7 +214,7 @@ ;; QUERY COMMANDS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(defn- handle-file-features +(defn handle-file-features [{:keys [features] :as file} client-features] (when (and (contains? features "components/v2") (not (contains? client-features "components/v2"))) @@ -244,11 +246,11 @@ (pmg/migrate-file) (handle-file-features client-features)))) -(defn- get-minimal-file +(defn get-minimal-file [{:keys [pool] :as cfg} id] (db/get pool :file {:id id} {:columns [:id :modified-at :revn]})) -(defn- get-file-etag +(defn get-file-etag [{:keys [modified-at revn]}] (str (dt/format-instant modified-at :iso) "-" revn)) @@ -277,18 +279,23 @@ (some-> (db/get conn :file-data-fragment {:file-id file-id :id fragment-id}) (update :content blob/decode))) +(s/def ::share-id ::us/uuid) (s/def ::fragment-id ::us/uuid) + (s/def ::get-file-fragment - (s/keys :req-un [::file-id ::fragment-id ::profile-id])) + (s/keys :req-un [::file-id ::fragment-id] + :opt-un [::share-id ::profile-id])) (sv/defmethod ::get-file-fragment "Retrieve a file by its ID. Only authenticated users." - {::doc/added "1.17"} - [{:keys [pool] :as cfg} {:keys [profile-id file-id fragment-id] :as params}] + {::doc/added "1.17" + :auth false} + [{:keys [pool] :as cfg} {:keys [profile-id file-id fragment-id share-id] :as params}] (with-open [conn (db/open pool)] - (check-read-permissions! conn profile-id file-id) - (-> (get-file-fragment conn file-id fragment-id) - (rph/with-http-cache long-cache-duration)))) + (let [perms (get-permissions conn profile-id file-id share-id)] + (check-read-permissions! perms) + (-> (get-file-fragment conn file-id fragment-id) + (rph/with-http-cache long-cache-duration))))) ;; --- COMMAND QUERY: get-file-object-thumbnails diff --git a/backend/src/app/rpc/commands/viewer.clj b/backend/src/app/rpc/commands/viewer.clj index 11f1cd75b..68cd69f88 100644 --- a/backend/src/app/rpc/commands/viewer.clj +++ b/backend/src/app/rpc/commands/viewer.clj @@ -10,6 +10,7 @@ [app.db :as db] [app.rpc.commands.comments :as comments] [app.rpc.commands.files :as files] + [app.rpc.cond :as-alias cond] [app.rpc.doc :as-alias doc] [app.rpc.queries.share-link :as slnk] [app.util.services :as sv] @@ -23,57 +24,52 @@ (defn- get-bundle [conn file-id profile-id features] - (let [file (files/get-file conn file-id features) - thumbnails (files/get-object-thumbnails conn file-id) - project (get-project conn (:project-id file)) - libs (files/get-file-libraries conn file-id features) - users (comments/get-file-comments-users conn file-id profile-id) + (let [file (files/get-file conn file-id features) + project (get-project conn (:project-id file)) + libs (files/get-file-libraries conn file-id features) + users (comments/get-file-comments-users conn file-id profile-id) - links (->> (db/query conn :share-link {:file-id file-id}) - (mapv slnk/decode-share-link-row)) + links (->> (db/query conn :share-link {:file-id file-id}) + (mapv slnk/decode-share-link-row)) - fonts (db/query conn :team-font-variant - {:team-id (:team-id project) - :deleted-at nil})] - {:file (assoc file :thumbnails thumbnails) + fonts (db/query conn :team-font-variant + {:team-id (:team-id project) + :deleted-at nil})] + + {:file file :users users :fonts fonts :project project :share-links links :libraries libs})) +(defn- remove-not-allowed-pages + [data allowed] + (-> data + (update :pages (fn [pages] (filterv #(contains? allowed %) pages))) + (update :pages-index select-keys allowed))) + (defn get-view-only-bundle [conn {:keys [profile-id file-id share-id features] :as params}] - (let [slink (slnk/retrieve-share-link conn file-id share-id) - perms (files/get-permissions conn profile-id file-id share-id) - thumbs (files/get-object-thumbnails conn file-id) + (let [perms (files/get-permissions conn profile-id file-id share-id) bundle (-> (get-bundle conn file-id profile-id features) - (assoc :permissions perms) - (assoc-in [:file :thumbnails] thumbs))] + (assoc :permissions perms))] ;; When we have neither profile nor share, we just return a not ;; found response to the user. - (when (and (not profile-id) - (not slink)) + (when-not perms (ex/raise :type :not-found - :code :object-not-found)) + :code :object-not-found + :hint "object not found")) - ;; When we have only profile, we need to check read permissions - ;; on file. - (when (and profile-id (not slink)) - (files/check-read-permissions! conn profile-id file-id)) + (update bundle :file + (fn [file] + (cond-> file + (= :share-link (:type perms)) + (update :data remove-not-allowed-pages (:pages perms)) - (cond-> bundle - (some? slink) - (assoc :share slink) - - (and (some? slink) - (not (contains? (:flags slink) "view-all-pages"))) - (update-in [:file :data] (fn [data] - (let [allowed-pages (:pages slink)] - (-> data - (update :pages (fn [pages] (filterv #(contains? allowed-pages %) pages))) - (update :pages-index (fn [index] (select-keys index allowed-pages)))))))))) + :always + (update :data select-keys [:id :options :pages :pages-index])))))) (s/def ::get-view-only-bundle (s/keys :req-un [::files/file-id] @@ -83,8 +79,10 @@ (sv/defmethod ::get-view-only-bundle {:auth false + ::cond/get-object #(files/get-minimal-file %1 (:file-id %2)) + ::cond/key-fn files/get-file-etag + ::cond/reuse-key? true ::doc/added "1.17"} [{:keys [pool]} params] (with-open [conn (db/open pool)] (get-view-only-bundle conn params))) - diff --git a/backend/src/app/srepl/main.clj b/backend/src/app/srepl/main.clj index 224a62333..5a85ab684 100644 --- a/backend/src/app/srepl/main.clj +++ b/backend/src/app/srepl/main.clj @@ -137,3 +137,8 @@ :id id :update-fn update-file :save? save?))) + +(defn enable-storage-features-on-file! + [system & {:as params}] + (enable-objects-map-feature-on-file! system params) + (enable-pointer-map-feature-on-file! system params)) diff --git a/backend/test/backend_tests/rpc_viewer_test.clj b/backend/test/backend_tests/rpc_viewer_test.clj index baaa416ed..6b0ded158 100644 --- a/backend/test/backend_tests/rpc_viewer_test.clj +++ b/backend/test/backend_tests/rpc_viewer_test.clj @@ -88,7 +88,6 @@ (t/is (nil? (:error out))) (let [result (:result out)] - (t/is (contains? result :share)) (t/is (contains? result :file)) (t/is (contains? result :project))))) @@ -103,7 +102,6 @@ ;; (th/print-result! out) (let [result (:result out)] (t/is (contains? result :file)) - (t/is (contains? result :share)) (t/is (contains? result :project))))) )) diff --git a/frontend/src/app/main/data/viewer.cljs b/frontend/src/app/main/data/viewer.cljs index 26252f042..4ae8f1263 100644 --- a/frontend/src/app/main/data/viewer.cljs +++ b/frontend/src/app/main/data/viewer.cljs @@ -8,18 +8,18 @@ (:require [app.common.data :as d] [app.common.data.macros :as dm] + [app.common.files.features :as ffeat] [app.common.geom.point :as gpt] [app.common.pages.helpers :as cph] [app.common.spec :as us] + [app.common.transit :as t] [app.common.types.shape-tree :as ctt] [app.common.types.shape.interactions :as ctsi] [app.main.data.comments :as dcm] [app.main.data.fonts :as df] - [app.main.data.messages :as msg] [app.main.features :as features] [app.main.repo :as rp] [app.util.globals :as ug] - [app.util.i18n :as i18n :refer [tr]] [app.util.router :as rt] [beicon.core :as rx] [cljs.spec.alpha :as s] @@ -99,40 +99,58 @@ ;; --- Data Fetching -(s/def ::fetch-bundle-params +(s/def ::fetch-bundle (s/keys :req-un [::page-id ::file-id] :opt-un [::share-id])) -(defn fetch-bundle +(defn- fetch-bundle [{:keys [file-id share-id] :as params}] - (us/assert ::fetch-bundle-params params) - (ptk/reify ::fetch-file + (us/assert! ::fetch-bundle params) + + (ptk/reify ::fetch-bundle ptk/WatchEvent (watch [_ state _] - (let [components-v2 (features/active-feature? state :components-v2) - params' (cond-> {:file-id file-id} - (uuid? share-id) - (assoc :share-id share-id) + (let [features (cond-> ffeat/enabled + (features/active-feature? state :components-v2) + (conj "components/v2") - :always - (assoc :components-v2 components-v2))] + :always + (conj "storage/pointer-map")) + params' (cond-> {:file-id file-id :features features} + (uuid? share-id) + (assoc :share-id share-id)) + + resolve (fn [[key pointer]] + (let [params {:file-id file-id :fragment-id @pointer} + params (cond-> params + (uuid? share-id) + (assoc :share-id share-id))] + (->> (rp/cmd! :get-file-fragment params) + (rx/map :content) + (rx/map #(vector key %)))))] (->> (rp/query! :view-only-bundle params') - (rx/mapcat - (fn [{:keys [fonts] :as bundle}] - (->> (rx/of (df/fonts-fetched fonts) - (bundle-fetched (merge bundle params)))))) - (rx/catch (fn [err] - (if (and (= (:type err) :restriction) - (= (:code err) :feature-disabled)) - (rx/of (msg/error (tr "errors.components-v2") {:timeout nil})) - (rx/throw err))))))))) + (rx/mapcat + (fn [bundle] + (->> (rx/from (-> bundle :file :data :pages-index seq)) + (rx/merge-map + (fn [[_ page :as kp]] + (if (t/pointer? page) + (resolve kp) + (rx/of kp)))) + (rx/reduce conj {}) + (rx/map (fn [pages-index] + (update-in bundle [:file :data] assoc :pages-index pages-index)))))) + (rx/mapcat + (fn [{:keys [fonts] :as bundle}] + (rx/of (df/fonts-fetched fonts) + (bundle-fetched (merge bundle params)))))))))) (declare go-to-frame-auto) (defn bundle-fetched - [{:keys [project file share-links libraries users permissions] :as bundle}] - (let [pages (->> (get-in file [:data :pages]) + [{:keys [project file share-links libraries users permissions thumbnails] :as bundle}] + (let [pages (->> (dm/get-in file [:data :pages]) (map (fn [page-id] (let [data (get-in file [:data :pages-index page-id])] [page-id (assoc data @@ -150,6 +168,7 @@ :permissions permissions :project project :pages pages + :thumbnails thumbnails :file file}))) ptk/WatchEvent diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 0f16dd7d8..0753c90fe 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -215,11 +215,10 @@ (->> (rp/cmd! :get-file-libraries {:file-id id :features features}) (rx/mapcat identity) - (rx/merge-map + (rx/mapcat (fn [file] (->> (filter (comp t/pointer? val) file) (resolve-pointers file)))) - (rx/reduce conj []) (rx/map libraries-fetched))))))) (rx/take-until stoper))))))) diff --git a/frontend/src/app/main/ui/workspace/shapes/frame.cljs b/frontend/src/app/main/ui/workspace/shapes/frame.cljs index 1de768035..2ac96c661 100644 --- a/frontend/src/app/main/ui/workspace/shapes/frame.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/frame.cljs @@ -100,7 +100,7 @@ ;; when `true` we've called the mount for the frame rendered? (mf/use-var false) - disable-thumbnail? (d/not-empty? (dm/get-in modifiers [(:id shape) :modifiers])) + disable-thumbnail? (d/not-empty? (dm/get-in modifiers [frame-id :modifiers])) [on-load-frame-dom render-frame? thumbnail-renderer] (ftr/use-render-thumbnail page-id shape node-ref rendered? disable-thumbnail? @force-render)