From 20d3251a9371ca6c33713c28fa48cec1fae84499 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 30 Mar 2022 00:11:43 +0200 Subject: [PATCH] :tada: Add generic file object thumbnail abstraction As replacement to the file frame thumbnail mechanism --- backend/src/app/migrations.clj | 6 + .../sql/0070-del-frame-thumbnail-table.sql | 1 + .../0071-add-file-object-thumbnail-table.sql | 11 + backend/src/app/rpc/mutations/files.clj | 27 +- backend/src/app/rpc/queries/files.clj | 203 +++++++------- backend/src/app/tasks/file_gc.clj | 41 +-- backend/test/app/services_files_test.clj | 254 ++++++++++++++---- backend/test/app/test_helpers.clj | 3 +- common/src/app/common/data.cljc | 3 +- frontend/src/app/main/data/workspace.cljs | 27 +- .../src/app/main/data/workspace/changes.cljs | 7 +- frontend/src/app/main/ui/viewer/shapes.cljs | 2 - .../app/main/ui/workspace/context_menu.cljs | 10 +- frontend/src/app/render.cljs | 6 +- frontend/src/app/worker/thumbnails.cljs | 10 +- 15 files changed, 399 insertions(+), 212 deletions(-) create mode 100644 backend/src/app/migrations/sql/0070-del-frame-thumbnail-table.sql create mode 100644 backend/src/app/migrations/sql/0071-add-file-object-thumbnail-table.sql diff --git a/backend/src/app/migrations.clj b/backend/src/app/migrations.clj index 7f552a532..d81001a1b 100644 --- a/backend/src/app/migrations.clj +++ b/backend/src/app/migrations.clj @@ -217,6 +217,12 @@ {:name "0069-add-file-thumbnail-table" :fn (mg/resource "app/migrations/sql/0069-add-file-thumbnail-table.sql")} + + {:name "0070-del-frame-thumbnail-table" + :fn (mg/resource "app/migrations/sql/0070-del-frame-thumbnail-table.sql")} + + {:name "0071-add-file-object-thumbnail-table" + :fn (mg/resource "app/migrations/sql/0071-add-file-object-thumbnail-table.sql")} ]) diff --git a/backend/src/app/migrations/sql/0070-del-frame-thumbnail-table.sql b/backend/src/app/migrations/sql/0070-del-frame-thumbnail-table.sql new file mode 100644 index 000000000..2c4d482c5 --- /dev/null +++ b/backend/src/app/migrations/sql/0070-del-frame-thumbnail-table.sql @@ -0,0 +1 @@ +DROP TABLE file_frame_thumbnail; diff --git a/backend/src/app/migrations/sql/0071-add-file-object-thumbnail-table.sql b/backend/src/app/migrations/sql/0071-add-file-object-thumbnail-table.sql new file mode 100644 index 000000000..aeb129af9 --- /dev/null +++ b/backend/src/app/migrations/sql/0071-add-file-object-thumbnail-table.sql @@ -0,0 +1,11 @@ +CREATE TABLE file_object_thumbnail ( + file_id uuid NOT NULL REFERENCES file(id) ON DELETE CASCADE, + object_id uuid NOT NULL, + created_at timestamptz NOT NULL DEFAULT now(), + data text NULL, + + PRIMARY KEY(file_id, object_id) +); + +ALTER TABLE file_object_thumbnail + ALTER COLUMN data SET STORAGE external; diff --git a/backend/src/app/rpc/mutations/files.clj b/backend/src/app/rpc/mutations/files.clj index c3014bfc8..a7bc61238 100644 --- a/backend/src/app/rpc/mutations/files.clj +++ b/backend/src/app/rpc/mutations/files.clj @@ -476,30 +476,31 @@ :revn revn :data (blob/encode data)} {:id id}))) - nil))) +;; --- Mutation: upsert object thumbnail -;; --- Mutation: Upsert frame thumbnail - -(def sql:upsert-frame-thumbnail - "insert into file_frame_thumbnail(file_id, frame_id, data) +(def sql:upsert-object-thumbnail + "insert into file_object_thumbnail(file_id, object_id, data) values (?, ?, ?) - on conflict(file_id, frame_id) do + on conflict(file_id, object_id) do update set data = ?;") -(s/def ::data ::us/string) -(s/def ::upsert-file-frame-thumbnail - (s/keys :req-un [::profile-id ::file-id ::frame-id ::data])) +(s/def ::data (s/nilable ::us/string)) +(s/def ::object-id ::us/uuid) +(s/def ::upsert-file-object-thumbnail + (s/keys :req-un [::profile-id ::file-id ::object-id ::data])) -(sv/defmethod ::upsert-file-frame-thumbnail - [{:keys [pool] :as cfg} {:keys [profile-id file-id frame-id data]}] +(sv/defmethod ::upsert-file-object-thumbnail + [{:keys [pool] :as cfg} {:keys [profile-id file-id object-id data]}] (db/with-atomic [conn pool] (files/check-edition-permissions! conn profile-id file-id) - (db/exec-one! conn [sql:upsert-frame-thumbnail file-id frame-id data data]) + (if data + (db/exec-one! conn [sql:upsert-object-thumbnail file-id object-id data data]) + (db/delete! conn :file-object-thumbnail {:file-id file-id :object-id object-id})) nil)) -;; --- Mutation: Upsert file thumbnail +;; --- Mutation: upsert file thumbnail (def sql:upsert-file-thumbnail "insert into file_thumbnail (file_id, revn, data, props) diff --git a/backend/src/app/rpc/queries/files.clj b/backend/src/app/rpc/queries/files.clj index 9794aa6a9..3ce4846be 100644 --- a/backend/src/app/rpc/queries/files.clj +++ b/backend/src/app/rpc/queries/files.clj @@ -7,11 +7,11 @@ (ns app.rpc.queries.files (:require [app.common.data :as d] + [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.pages.helpers :as cph] [app.common.pages.migrations :as pmg] [app.common.spec :as us] - [app.common.uuid :as uuid] [app.db :as db] [app.db.sql :as sql] [app.rpc.helpers :as rpch] @@ -21,7 +21,8 @@ [app.rpc.queries.teams :as teams] [app.util.blob :as blob] [app.util.services :as sv] - [clojure.spec.alpha :as s])) + [clojure.spec.alpha :as s] + [cuerdas.core :as str])) (declare decode-row) (declare decode-row-xf) @@ -187,12 +188,30 @@ ;; --- Query: File (By ID) +(defn retrieve-object-thumbnails + ([{:keys [pool]} file-id] + (let [sql (str/concat + "select object_id, data " + " from file_object_thumbnail" + " where file_id=?")] + (->> (db/exec! pool [sql file-id]) + (d/index-by :object-id :data)))) + + ([{:keys [pool]} file-id frame-ids] + (with-open [conn (db/open pool)] + (let [sql (str/concat + "select object_id, data " + " from file_object_thumbnail" + " where file_id=? and object_id = ANY(?)") + ids (db/create-array conn "uuid" (seq frame-ids))] + (->> (db/exec! conn [sql file-id ids]) + (d/index-by :object-id :data)))))) + (defn retrieve-file [{:keys [pool] :as cfg} id] - (let [item (db/get-by-id pool :file id)] - (->> item - (decode-row) - (pmg/migrate-file)))) + (->> (db/get-by-id pool :file id) + (decode-row) + (pmg/migrate-file))) (s/def ::file (s/keys :req-un [::profile-id ::id])) @@ -202,12 +221,16 @@ [{:keys [pool] :as cfg} {:keys [profile-id id] :as params}] (let [perms (get-permissions pool profile-id id)] (check-read-permissions! perms) - (-> (retrieve-file cfg id) - (assoc :permissions perms)))) + (let [file (retrieve-file cfg id) + thumbs (retrieve-object-thumbnails cfg id)] + (-> file + (assoc :thumbnails thumbs) + (assoc :permissions perms))))) -;; --- FILE THUMBNAIL -(defn- trim-objects +;; --- QUERY: page + +(defn- prune-objects "Given the page data and the object-id returns the page data with all other not needed objects removed from the `:objects` data structure." @@ -219,64 +242,19 @@ "Given the page data, removes the `:thumbnail` prop from all shapes." [page] - (update page :objects (fn [objects] - (d/mapm #(dissoc %2 :thumbnail) objects)))) - -(defn- prune-frames-with-thumbnails - "Remove unnecesary shapes from frames that have thumbnail from page - data." - [page] - (let [filter-shape? - (fn [objects [id shape]] - (let [frame-id (:frame-id shape)] - (or (= id uuid/zero) - (= frame-id uuid/zero) - (not (some? (get-in objects [frame-id :thumbnail])))))) - - ;; We need to remove from the attribute :shapes its children because - ;; they will not be sent in the data - remove-frame-children - (fn [[id shape]] - [id (cond-> shape - (some? (:thumbnail shape)) - (assoc :shapes []))]) - - update-objects - (fn [objects] - (into {} - (comp (map remove-frame-children) - (filter (partial filter-shape? objects))) - objects))] - - (update page :objects update-objects))) - -(defn- get-thumbnail-data - [{:keys [data] :as file}] - (if-let [[page frame] (first - (for [page (-> data :pages-index vals) - frame (-> page :objects cph/get-frames) - :when (:file-thumbnail frame)] - [page frame]))] - (let [objects (->> (cph/get-children-with-self (:objects page) (:id frame)) - (d/index-by :id))] - (-> (assoc page :objects objects) - (assoc :thumbnail-frame frame))) - - (let [page-id (-> data :pages first)] - (-> (get-in data [:pages-index page-id]) - (prune-frames-with-thumbnails))))) + (update page :objects d/update-vals #(dissoc % :thumbnail))) (s/def ::page-id ::us/uuid) (s/def ::object-id ::us/uuid) -(s/def ::prune-frames-with-thumbnails ::us/boolean) -(s/def ::prune-thumbnails ::us/boolean) (s/def ::page - (s/keys :req-un [::profile-id ::file-id] - :opt-un [::page-id - ::object-id - ::prune-frames-with-thumbnails - ::prune-thumbnails])) + (s/and + (s/keys :req-un [::profile-id ::file-id] + :opt-un [::page-id ::object-id]) + (fn [obj] + (if (contains? obj :object-id) + (contains? obj :page-id) + true)))) (sv/defmethod ::page "Retrieves the page data from file and returns it. If no page-id is @@ -284,6 +262,9 @@ specified, only that object and its children will be returned in the page objects data structure. + If you specify the object-id, the page-id parameter becomes + mandatory. + Mainly used for rendering purposes." [{:keys [pool] :as cfg} {:keys [profile-id file-id page-id object-id] :as props}] (check-read-permissions! pool profile-id file-id) @@ -291,28 +272,84 @@ page-id (or page-id (-> file :data :pages first)) page (get-in file [:data :pages-index page-id])] - (cond-> page - (:prune-frames-with-thumbnails props) - (prune-frames-with-thumbnails) - - (:prune-thumbnails props) - (prune-thumbnails) - + (cond-> (prune-thumbnails page) (uuid? object-id) - (trim-objects object-id)))) + (prune-objects object-id)))) + +;; --- QUERY: file-data-for-thumbnail + +(defn- get-file-thumbnail-data + [cfg {:keys [data id] :as file}] + (letfn [;; function responsible on finding the frame marked to be + ;; used as thumbnail; the returned frame always have + ;; the :page-id set to the page that it belongs. + (get-thumbnail-frame [data] + (d/seek :use-for-thumbnail? + (for [page (-> data :pages-index vals) + frame (-> page :objects cph/get-frames)] + (assoc frame :page-id (:id page))))) + + ;; function responsible to filter objects data strucuture of + ;; all unneded shapes if a concrete frame is provided. If no + ;; frame, the objects is returned untouched. + (filter-objects [objects frame-id] + (d/index-by :id (cph/get-children-with-self objects frame-id))) + + ;; function responsible of assoc available thumbnails + ;; to frames and remove all children shapes from objects if + ;; thumbnails is available + (assoc-thumbnails [objects thumbnails] + (loop [objects objects + frames (filter cph/frame-shape? (vals objects))] + + (if-let [{:keys [id] :as frame} (first frames)] + (let [frame (if-let [thumb (get thumbnails id)] + (assoc frame :thumbnail thumb :shapes []) + (dissoc frame :thumbnail))] + (if (:thumbnail frame) + (recur (-> (assoc objects id frame) + (d/without-keys (cph/get-children-ids objects id))) + (rest frames)) + (recur (assoc objects id frame) + (rest frames)))) + + objects)))] + + (let [frame (get-thumbnail-frame data) + frame-id (:id frame) + page-id (or (:page-id frame) + (-> data :pages first)) + page (dm/get-in data [:pages-index page-id]) + + obj-ids (or (some-> frame-id list) + (map :id (cph/get-frames page))) + thumbs (retrieve-object-thumbnails cfg id obj-ids)] + + (cond-> page + ;; If we have frame, we need to specify it on the page level + ;; and remove the all other unrelated objects. + (some? frame-id) + (-> (assoc :thumbnail-frame-id frame-id) + (update :objects filter-objects frame-id)) + + ;; Assoc the available thumbnails and prune not visible shapes + ;; for avoid transfer unnecesary data. + :always + (update :objects assoc-thumbnails thumbs))))) (s/def ::file-data-for-thumbnail (s/keys :req-un [::profile-id ::file-id])) (sv/defmethod ::file-data-for-thumbnail - "Retrieves the data for generate the thumbnail of the file. Used mainly for render - thumbnails on dashboard. Returns the page data." + "Retrieves the data for generate the thumbnail of the file. Used + mainly for render thumbnails on dashboard." [{:keys [pool] :as cfg} {:keys [profile-id file-id] :as props}] (check-read-permissions! pool profile-id file-id) (let [file (retrieve-file cfg file-id)] - {:page (get-thumbnail-data file) - :file-id file-id - :revn (:revn file)})) + {:file-id file-id + :revn (:revn file) + :page (get-file-thumbnail-data cfg file)})) + ;; --- Query: Shared Library Files @@ -412,20 +449,6 @@ (teams/check-read-permissions! pool profile-id team-id) (db/exec! pool [sql:team-recent-files team-id])) -;; --- QUERY: get all file frame thumbnails - -(s/def ::file-frame-thumbnails - (s/keys :req-un [::profile-id ::file-id] - :opt-un [::frame-id])) - -(sv/defmethod ::file-frame-thumbnails - [{:keys [pool]} {:keys [profile-id file-id frame-id]}] - (check-read-permissions! pool profile-id file-id) - (let [params (cond-> {:file-id file-id} - frame-id (assoc :frame-id frame-id)) - rows (db/query pool :file-frame-thumbnail params)] - (d/index-by :frame-id :data rows))) - ;; --- QUERY: get file thumbnail (s/def ::revn ::us/integer) diff --git a/backend/src/app/tasks/file_gc.clj b/backend/src/app/tasks/file_gc.clj index b8669d96b..4591e56e8 100644 --- a/backend/src/app/tasks/file_gc.clj +++ b/backend/src/app/tasks/file_gc.clj @@ -6,18 +6,19 @@ (ns app.tasks.file-gc "A maintenance task that is responsible of: purge unused file media, - clean unused frame thumbnails and remove old file thumbnails. The + clean unused object thumbnails and remove old file thumbnails. The file is eligible to be garbage collected after some period of inactivity (the default threshold is 72h)." (:require [app.common.data :as d] [app.common.logging :as l] - [app.common.pages.helpers :as cph] [app.common.pages.migrations :as pmg] [app.db :as db] [app.util.blob :as blob] [app.util.time :as dt] + [clojure.set :as set] [clojure.spec.alpha :as s] + [cuerdas.core :as str] [integrant.core :as ig])) (declare ^:private retrieve-candidates) @@ -117,26 +118,26 @@ ;; them. (db/delete! conn :file-media-object {:id (:id mobj)})))) -(defn- collect-frames - [data] - (let [xform (comp - (map :objects) - (mapcat vals) - (filter cph/frame-shape?) - (keep :id)) - pages (concat - (vals (:pages-index data)) - (vals (:components data)))] - (into #{} xform pages))) - (defn- clean-file-frame-thumbnails! [conn file-id data] - (let [sql (str "delete from file_frame_thumbnail " - " where file_id=? and not (frame_id=ANY(?))") - ids (->> (collect-frames data) - (db/create-array conn "uuid")) - res (db/exec-one! conn [sql file-id ids])] - (l/debug :hint "delete frame thumbnails" :total (:next.jdbc/update-count res)))) + (let [stored (->> (db/query conn :file-object-thumbnail + {:file-id file-id} + {:columns [:object-id]}) + (into #{} (map :object-id))) + + using (->> (concat (vals (:pages-index data)) + (vals (:components data))) + (into #{} (comp (map :objects) + (mapcat keys)))) + + unused (set/difference stored using)] + + (when (seq unused) + (let [sql (str/concat + "delete from file_object_thumbnail " + " where file_id=? and object_id=ANY(?)") + res (db/exec-one! conn [sql file-id (db/create-array conn "uuid" unused)])] + (l/debug :hint "delete object thumbnails" :total (:next.jdbc/update-count res)))))) (defn- clean-file-thumbnails! [conn file-id revn] diff --git a/backend/test/app/services_files_test.clj b/backend/test/app/services_files_test.clj index 3977a1317..b30e2a53b 100644 --- a/backend/test/app/services_files_test.clj +++ b/backend/test/app/services_files_test.clj @@ -413,75 +413,217 @@ (t/is (= (:type error-data) :not-found)))) )) -(t/deftest query-frame-thumbnails + +(t/deftest object-thumbnails-ops (let [prof (th/create-profile* 1 {:is-active true}) file (th/create-file* 1 {:profile-id (:id prof) :project-id (:default-project-id prof) :is-shared false}) - data {::th/type :file-frame-thumbnails - :profile-id (:id prof) - :file-id (:id file) - :frame-id (uuid/next)}] + page-id (get-in file [:data :pages 0]) + frame1-id (uuid/next) + shape1-id (uuid/next) + frame2-id (uuid/next) + shape2-id (uuid/next) - ;; insert an entry on the database with a test value for the thumbnail of this frame - (th/db-insert! :file-frame-thumbnail - {:file-id (:file-id data) - :frame-id (:frame-id data) - :data "testvalue"}) + changes [{:type :add-obj + :page-id page-id + :id frame1-id + :parent-id uuid/zero + :frame-id uuid/zero + :obj {:id frame1-id + :use-for-thumbnail? true + :name "test-frame1" + :type :frame}} + {:type :add-obj + :page-id page-id + :id shape1-id + :parent-id frame1-id + :frame-id frame1-id + :obj {:id shape1-id + :name "test-shape1" + :type :rect}} + {:type :add-obj + :page-id page-id + :id frame2-id + :parent-id uuid/zero + :frame-id uuid/zero + :obj {:id frame2-id + :name "test-frame2" + :type :frame}} + {:type :add-obj + :page-id page-id + :id shape2-id + :parent-id frame2-id + :frame-id frame2-id + :obj {:id shape2-id + :name "test-shape2" + :type :rect}}]] + ;; Update the file + (th/update-file* {:file-id (:id file) + :profile-id (:id prof) + :revn 0 + :changes changes}) - (let [{:keys [result error] :as out} (th/query! data)] - ;; (th/print-result! out) - (t/is (nil? error)) - (t/is (= 1 (count result))) - (t/is (= "testvalue" (get result (:frame-id data))))))) + (t/testing "RPC page query (rendering purposes)" -(t/deftest insert-frame-thumbnails - (let [prof (th/create-profile* 1 {:is-active true}) - file (th/create-file* 1 {:profile-id (:id prof) - :project-id (:default-project-id prof) - :is-shared false}) - data {::th/type :upsert-file-frame-thumbnail - :profile-id (:id prof) - :file-id (:id file) - :frame-id (uuid/next) - :data "test insert new value"}] + ;; Query :page RPC method without passing page-id + (let [data {::th/type :page + :profile-id (:id prof) + :file-id (:id file)} + {:keys [error result] :as out} (th/query! data)] - (let [out (th/mutation! data)] - (t/is (nil? (:error out))) - (t/is (nil? (:result out))) - (let [[result] (th/db-query :file-frame-thumbnail - {:file-id (:file-id data) - :frame-id (:frame-id data)})] - (t/is (= "test insert new value" (:data result))))))) + ;; (th/print-result! out) + (t/is (map? result)) + (t/is (contains? result :objects)) + (t/is (contains? (:objects result) frame1-id)) + (t/is (contains? (:objects result) shape1-id)) + (t/is (contains? (:objects result) frame2-id)) + (t/is (contains? (:objects result) shape2-id)) + (t/is (contains? (:objects result) uuid/zero))) -(t/deftest upsert-frame-thumbnails - (let [prof (th/create-profile* 1 {:is-active true}) - file (th/create-file* 1 {:profile-id (:id prof) - :project-id (:default-project-id prof) - :is-shared false}) - data {::th/type :upsert-file-frame-thumbnail - :profile-id (:id prof) - :file-id (:id file) - :frame-id (uuid/next) - :data "updated value"}] + ;; Query :page RPC method with page-id + (let [data {::th/type :page + :profile-id (:id prof) + :file-id (:id file) + :page-id page-id} + {:keys [error result] :as out} (th/query! data)] + ;; (th/print-result! out) + (t/is (map? result)) + (t/is (contains? result :objects)) + (t/is (contains? (:objects result) frame1-id)) + (t/is (contains? (:objects result) shape1-id)) + (t/is (contains? (:objects result) frame2-id)) + (t/is (contains? (:objects result) shape2-id)) + (t/is (contains? (:objects result) uuid/zero))) - ;; insert an entry on the database with and old value for the thumbnail of this frame - (th/db-insert! :file-frame-thumbnail - {:file-id (:file-id data) - :frame-id (:frame-id data) - :data "old value"}) + ;; Query :page RPC method with page-id and object-id + (let [data {::th/type :page + :profile-id (:id prof) + :file-id (:id file) + :page-id page-id + :object-id frame1-id} + {:keys [error result] :as out} (th/query! data)] + ;; (th/print-result! out) + (t/is (map? result)) + (t/is (contains? result :objects)) + (t/is (contains? (:objects result) frame1-id)) + (t/is (contains? (:objects result) shape1-id)) + (t/is (not (contains? (:objects result) uuid/zero))) + (t/is (not (contains? (:objects result) frame2-id))) + (t/is (not (contains? (:objects result) shape2-id)))) - (let [out (th/mutation! data)] - ;; (th/print-result! out) + ;; Query :page RPC method with wrong params + (let [data {::th/type :page + :profile-id (:id prof) + :file-id (:id file) + :object-id frame1-id} + {:keys [error result] :as out} (th/query! data)] + ;; (th/print-result! out) + (t/is (= :validation (th/ex-type error))) + (t/is (= :spec-validation (th/ex-code error))))) - (t/is (nil? (:error out))) - (t/is (nil? (:result out))) + (t/testing "RPC :file-data-for-thumbnail" + ;; Insert a thumbnail data for the frame-id + (let [data {::th/type :upsert-file-object-thumbnail + :profile-id (:id prof) + :file-id (:id file) + :object-id frame1-id + :data "random-data-1"} - ;; retrieve the value from the database and check its content - (let [[result] (th/db-query :file-frame-thumbnail - {:file-id (:file-id data) - :frame-id (:frame-id data)})] - (t/is (= "updated value" (:data result))))))) + {:keys [error result] :as out} (th/mutation! data)] + (t/is (nil? error)) + (t/is (nil? result))) + + ;; Check the result + (let [data {::th/type :file-data-for-thumbnail + :profile-id (:id prof) + :file-id (:id file)} + {:keys [error result] :as out} (th/query! data)] + ;; (th/print-result! out) + (t/is (map? result)) + (t/is (contains? result :page)) + (t/is (contains? result :revn)) + (t/is (contains? result :file-id)) + + (t/is (= (:id file) (:file-id result))) + (t/is (= "random-data-1" (get-in result [:page :objects frame1-id :thumbnail]))) + (t/is (= [] (get-in result [:page :objects frame1-id :shapes])))) + + ;; Delete thumbnail data + (let [data {::th/type :upsert-file-object-thumbnail + :profile-id (:id prof) + :file-id (:id file) + :object-id frame1-id + :data nil} + {:keys [error result] :as out} (th/mutation! data)] + (t/is (nil? error)) + (t/is (nil? result))) + + ;; Check the result + (let [data {::th/type :file-data-for-thumbnail + :profile-id (:id prof) + :file-id (:id file)} + {:keys [error result] :as out} (th/query! data)] + ;; (th/print-result! out) + (t/is (map? result)) + (t/is (contains? result :page)) + (t/is (contains? result :revn)) + (t/is (contains? result :file-id)) + (t/is (= (:id file) (:file-id result))) + (t/is (nil? (get-in result [:page :objects frame1-id :thumbnail]))) + (t/is (not= [] (get-in result [:page :objects frame1-id :shapes]))))) + + (t/testing "TASK :file-gc" + + ;; insert object snapshot for known frame + (let [data {::th/type :upsert-file-object-thumbnail + :profile-id (:id prof) + :file-id (:id file) + :object-id frame1-id + :data "new-data"} + {:keys [error result] :as out} (th/mutation! data)] + (t/is (nil? error)) + (t/is (nil? result))) + + ;; Wait to file be ellegible for GC + (th/sleep 300) + + ;; run the task again + (let [task (:app.tasks.file-gc/handler th/*system*) + res (task {})] + (t/is (= 1 (:processed res)))) + + ;; check that object thumbnails are still here + (let [res (th/db-exec! ["select * from file_object_thumbnail"])] + (t/is (= 1 (count res))) + (t/is (= "new-data" (get-in res [0 :data])))) + + ;; insert object snapshot for for unknown frame + (let [data {::th/type :upsert-file-object-thumbnail + :profile-id (:id prof) + :file-id (:id file) + :object-id (uuid/next) + :data "new-data-2"} + {:keys [error result] :as out} (th/mutation! data)] + (t/is (nil? error)) + (t/is (nil? result))) + + ;; Mark file as modified + (th/db-exec! ["update file set has_media_trimmed=false where id=?" (:id file)]) + + ;; check that we have all object thumbnails + (let [res (th/db-exec! ["select * from file_object_thumbnail"])] + (t/is (= 2 (count res)))) + + ;; run the task again + (let [task (:app.tasks.file-gc/handler th/*system*) + res (task {})] + (t/is (= 1 (:processed res)))) + + ;; check that the unknown frame thumbnail is deleted + (let [res (th/db-exec! ["select * from file_object_thumbnail"])] + (t/is (= 1 (count res))) + (t/is (= "new-data" (get-in res [0 :data]))))))) (t/deftest file-thumbnail-ops diff --git a/backend/test/app/test_helpers.clj b/backend/test/app/test_helpers.clj index 5699424a6..f94e60701 100644 --- a/backend/test/app/test_helpers.clj +++ b/backend/test/app/test_helpers.clj @@ -11,6 +11,7 @@ [app.common.pages :as cp] [app.common.spec :as us] [app.common.uuid :as uuid] + [app.common.pprint :as pp] [app.config :as cf] [app.db :as db] [app.main :as main] @@ -303,7 +304,7 @@ (println "====> END ERROR")) (do (println "====> START RESPONSE") - (fipp.edn/pprint result) + (pp/pprint result) (println "====> END RESPONSE")))) (defn exception? diff --git a/common/src/app/common/data.cljc b/common/src/app/common/data.cljc index 872539945..a87466d81 100644 --- a/common/src/app/common/data.cljc +++ b/common/src/app/common/data.cljc @@ -101,7 +101,6 @@ (defn preconj [coll elem] - (assert (or (vector? coll) (nil? coll))) (into [elem] coll)) (defn enumerate @@ -176,7 +175,7 @@ [data keys] (when (map? data) (persistent! - (reduce #(dissoc! %1 %2) (transient data) keys)))) + (reduce dissoc! (transient data) keys)))) (defn remove-at-index "Takes a vector and returns a vector with an element in the diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 4f0e74ecb..bbed408dd 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -964,18 +964,23 @@ (ptk/reify ::toggle-file-thumbnail-selected ptk/WatchEvent (watch [_ state _] - (let [selected (wsh/lookup-selected state) - pages (-> state :workspace-data :pages-index vals) - extract (fn [{:keys [objects id] :as page}] - (->> (cph/get-frames objects) - (filter :file-thumbnail) - (map :id) - (remove selected) - (map (fn [frame-id] [id frame-id]))))] + (let [selected (wsh/lookup-selected state) + pages (-> state :workspace-data :pages-index vals) + get-frames (fn [{:keys [objects id] :as page}] + (->> (cph/get-frames objects) + (sequence + (comp (filter :use-for-thumbnail?) + (map :id) + (remove selected) + (map (partial vector id))))))] + (rx/concat - (rx/from (for [[page-id frame-id] (mapcat extract pages)] - (dch/update-shapes [frame-id] #(dissoc % :file-thumbnail) page-id nil))) - (rx/of (dch/update-shapes selected #(assoc % :file-thumbnail true)))))))) + (rx/from + (->> (mapcat get-frames pages) + (d/group-by first second) + (map (fn [[page-id frame-ids]] + (dch/update-shapes frame-ids #(dissoc % :use-for-thumbnail?) {:page-id page-id}))))) + (rx/of (dch/update-shapes selected #(update % :use-for-thumbnail? not)))))))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Navigation diff --git a/frontend/src/app/main/data/workspace/changes.cljs b/frontend/src/app/main/data/workspace/changes.cljs index 0336f2bab..1b298f285 100644 --- a/frontend/src/app/main/data/workspace/changes.cljs +++ b/frontend/src/app/main/data/workspace/changes.cljs @@ -32,10 +32,9 @@ (def commit-changes? (ptk/type? ::commit-changes)) (defn update-shapes - ([ids update-fn] (update-shapes ids update-fn nil nil)) - ([ids update-fn keys] (update-shapes ids update-fn nil keys)) - ([ids update-fn page-id {:keys [reg-objects? save-undo? attrs ignore-tree] - :or {reg-objects? false save-undo? true attrs nil}}] + ([ids update-fn] (update-shapes ids update-fn nil)) + ([ids update-fn {:keys [reg-objects? save-undo? attrs ignore-tree page-id] + :or {reg-objects? false save-undo? true}}] (us/assert ::coll-of-uuid ids) (us/assert fn? update-fn) diff --git a/frontend/src/app/main/ui/viewer/shapes.cljs b/frontend/src/app/main/ui/viewer/shapes.cljs index 89f4c2c05..c21622409 100644 --- a/frontend/src/app/main/ui/viewer/shapes.cljs +++ b/frontend/src/app/main/ui/viewer/shapes.cljs @@ -7,8 +7,6 @@ (ns app.main.ui.viewer.shapes "The main container for a frame in viewer mode" (:require - [app.common.geom.matrix :as gmt] - [app.common.geom.point :as gpt] [app.common.geom.shapes :as geom] [app.common.pages.helpers :as cph] [app.common.spec.interactions :as cti] diff --git a/frontend/src/app/main/ui/workspace/context_menu.cljs b/frontend/src/app/main/ui/workspace/context_menu.cljs index 28895ba49..697a6b369 100644 --- a/frontend/src/app/main/ui/workspace/context_menu.cljs +++ b/frontend/src/app/main/ui/workspace/context_menu.cljs @@ -8,6 +8,7 @@ "A workspace specific context menu (mouse right click)." (:require [app.common.data :as d] + [app.common.pages.helpers :as cph] [app.common.spec.page :as csp] [app.main.data.events :as ev] [app.main.data.modal :as modal] @@ -167,13 +168,12 @@ (mf/defc context-menu-thumbnail [{:keys [shapes]}] - (let [single? (= (count shapes) 1) - has-frame? (->> shapes (d/seek #(= :frame (:type %)))) - is-frame? (and single? has-frame?) + (let [single? (= (count shapes) 1) + has-frame? (some cph/frame-shape? shapes) do-toggle-thumbnail (st/emitf (dw/toggle-file-thumbnail-selected))] - (when is-frame? + (when (and single? has-frame?) [:* - (if (every? :file-thumbnail shapes) + (if (every? :use-for-thumbnail? shapes) [:& menu-entry {:title (tr "workspace.shape.menu.thumbnail-remove") :on-click do-toggle-thumbnail}] [:& menu-entry {:title (tr "workspace.shape.menu.thumbnail-set") diff --git a/frontend/src/app/render.cljs b/frontend/src/app/render.cljs index 38b61f544..bd89007bf 100644 --- a/frontend/src/app/render.cljs +++ b/frontend/src/app/render.cljs @@ -106,8 +106,7 @@ (repo/query! :font-variants {:file-id file-id}) (repo/query! :page {:file-id file-id :page-id page-id - :object-id object-id - :prune-thumbnails true})) + :object-id object-id})) (rx/tap (fn [[fonts]] (when (seq fonts) (st/emit! (df/fonts-fetched fonts))))) @@ -146,8 +145,7 @@ (->> (rx/zip (repo/query! :font-variants {:file-id file-id}) (repo/query! :page {:file-id file-id - :page-id page-id - :prune-thumbnails true})) + :page-id page-id})) (rx/tap (fn [[fonts]] (when (seq fonts) (st/emit! (df/fonts-fetched fonts))))) diff --git a/frontend/src/app/worker/thumbnails.cljs b/frontend/src/app/worker/thumbnails.cljs index 30455109c..e5e9d77de 100644 --- a/frontend/src/app/worker/thumbnails.cljs +++ b/frontend/src/app/worker/thumbnails.cljs @@ -63,10 +63,12 @@ (defn- render-thumbnail [{:keys [page file-id revn] :as params}] - (let [elem (if-let [frame (:thumbnail-frame page)] - (mf/element render/frame-svg #js {:objects (:objects page) :frame frame}) - (mf/element render/page-svg #js {:data page :thumbnails? true}))] - {:data (rds/renderToStaticMarkup elem) + (let [objects (:objects page) + frame (some->> page :thumbnail-frame-id (get objects)) + element (if frame + (mf/element render/frame-svg #js {:objects objects :frame frame}) + (mf/element render/page-svg #js {:data page :thumbnails? true}))] + {:data (rds/renderToStaticMarkup element) :fonts @fonts/loaded :file-id file-id :revn revn}))