diff --git a/backend/src/app/rpc/commands/files_thumbnails.clj b/backend/src/app/rpc/commands/files_thumbnails.clj index dd9c031232..4596dbd15f 100644 --- a/backend/src/app/rpc/commands/files_thumbnails.clj +++ b/backend/src/app/rpc/commands/files_thumbnails.clj @@ -311,11 +311,30 @@ :object-id object-id :tag tag}))) +(defn- delete-file-object-thumbnails! + "Soft-deletes multiple object thumbnails in a single UPDATE statement + with RETURNING, then touches all returned media objects." + [{:keys [::db/conn ::sto/storage]} object-ids] + (let [ids (db/create-array conn "text" (seq object-ids)) + sql (str/concat + "UPDATE file_tagged_object_thumbnail" + " SET deleted_at = now()" + " WHERE object_id = ANY(?)" + " AND deleted_at IS NULL" + " RETURNING media_id") + rows (db/exec! conn [sql ids])] + (doseq [{:keys [media-id]} rows] + (sto/touch-object! storage media-id)))) + (def ^:private schema:delete-file-object-thumbnail [:map {:title "delete-file-object-thumbnail"} [:file-id ::sm/uuid] [:object-id [:string {:max 250}]]]) +(def ^:private schema:delete-file-object-thumbnails + [:map {:title "delete-file-object-thumbnails"} + [:object-ids [:vector {:max 200} [:string {:max 250}]]]]) + (sv/defmethod ::delete-file-object-thumbnail {::doc/added "1.19" ::doc/module :files @@ -329,6 +348,30 @@ (delete-file-object-thumbnail! file-id object-id)) nil))) +(sv/defmethod ::delete-file-object-thumbnails + {::doc/added "1.19" + ::doc/module :files + ::climit/id [[:file-thumbnail-ops/by-profile ::rpc/profile-id] + [:file-thumbnail-ops/global]] + ::sm/params schema:delete-file-object-thumbnails + ::audit/skip true} + [cfg {:keys [::rpc/profile-id object-ids]}] + (when (seq object-ids) + ;; Extract unique file-ids from object-ids for permission checks + (let [file-ids (->> object-ids + (map thc/get-file-id) + (into #{}))] + ;; Check permissions for each unique file using a single connection + (db/run! cfg (fn [{:keys [::db/conn]}] + (doseq [file-id file-ids] + (files/check-edition-permissions! conn profile-id file-id)))) + ;; Delete all matching thumbnails in one transaction + (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] + (-> cfg + (update ::sto/storage sto/configure conn) + (delete-file-object-thumbnails! object-ids)) + nil))))) + ;; --- MUTATION COMMAND: create-file-thumbnail (defn- create-file-thumbnail diff --git a/backend/test/backend_tests/rpc_file_thumbnails_test.clj b/backend/test/backend_tests/rpc_file_thumbnails_test.clj index 8f24ccb580..000735df0c 100644 --- a/backend/test/backend_tests/rpc_file_thumbnails_test.clj +++ b/backend/test/backend_tests/rpc_file_thumbnails_test.clj @@ -354,3 +354,538 @@ (t/is (nil? (:error out))) (t/is (map? (:result out)))))) +;; --- delete-file-object-thumbnails (batch) + +(t/deftest delete-file-object-thumbnails-basic + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (first (get-in file [:data :pages])) + oid1 (thc/fmt-object-id (:id file) page-id (uuid/random) "frame") + oid2 (thc/fmt-object-id (:id file) page-id (uuid/random) "frame") + oid3 (thc/fmt-object-id (:id file) page-id (uuid/random) "component")] + + ;; Create three thumbnails + (doseq [oid [oid1 oid2 oid3]] + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id (:id file) + :object-id oid + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out))))) + + ;; Verify all three exist and are not soft-deleted + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {::db/remove-deleted false + :order-by [[:created-at :asc]]})] + (t/is (= 3 (count rows))) + (doseq [row rows] + (t/is (nil? (:deleted-at row))))) + + ;; Batch delete all three + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid1 oid2 oid3]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; Verify all three are now soft-deleted + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {::db/remove-deleted false + :order-by [[:created-at :asc]]})] + (t/is (= 3 (count rows))) + (doseq [row rows] + (t/is (some? (:deleted-at row))))))) + +(t/deftest delete-file-object-thumbnails-empty + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids []} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out))))) + +(t/deftest delete-file-object-thumbnails-non-existent + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (first (get-in file [:data :pages])) + oid1 (thc/fmt-object-id (:id file) page-id (uuid/random) "frame") + oid2 (thc/fmt-object-id (:id file) page-id (uuid/random) "frame")] + + ;; Batch delete non-existent object-ids (no thumbnails were created) + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid1 oid2]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))))) + +(t/deftest delete-file-object-thumbnails-mixed-exists + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (first (get-in file [:data :pages])) + oid1 (thc/fmt-object-id (:id file) page-id (uuid/random) "frame") + oid2 (thc/fmt-object-id (:id file) page-id (uuid/random) "frame") + oid3 (thc/fmt-object-id (:id file) page-id (uuid/random) "frame")] + + ;; Create only one thumbnail + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id (:id file) + :object-id oid1 + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out)))) + + ;; Batch delete mix of existing and non-existing + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid1 oid2 oid3]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; Verify oid1 is soft-deleted, others don't exist + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {::db/remove-deleted false})] + (t/is (= 1 (count rows))) + (t/is (= oid1 (:object-id (first rows)))) + (t/is (some? (:deleted-at (first rows))))))) + +(t/deftest delete-file-object-thumbnails-already-deleted + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (first (get-in file [:data :pages])) + oid (thc/fmt-object-id (:id file) page-id (uuid/random) "frame")] + + ;; Create a thumbnail + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id (:id file) + :object-id oid + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out)))) + + ;; First batch delete + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; Second batch delete (idempotent — no rows match deleted_at IS NULL) + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; Verify still 1 row, still soft-deleted, not duplicated + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {::db/remove-deleted false})] + (t/is (= 1 (count rows))) + (t/is (= oid (:object-id (first rows)))) + (t/is (some? (:deleted-at (first rows))))))) + +(t/deftest delete-file-object-thumbnails-unauthorized + (let [profile1 (th/create-profile* 1) + profile2 (th/create-profile* 2) + file (th/create-file* 1 {:profile-id (:id profile1) + :project-id (:default-project-id profile1) + :is-shared false}) + page-id (first (get-in file [:data :pages])) + oid (thc/fmt-object-id (:id file) page-id (uuid/random) "frame")] + + ;; profile1 creates a thumbnail on their file + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile1) + :file-id (:id file) + :object-id oid + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out)))) + + ;; profile2 tries to batch delete thumbnails from profile1's file + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile2) + :object-ids [oid]} + out (th/command! data)] + (t/is (some? (:error out))) + (t/is (th/ex-info? (:error out))) + (t/is (= :not-found (th/ex-type (:error out))))) + + ;; Verify the thumbnail is NOT deleted + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {::db/remove-deleted false})] + (t/is (= 1 (count rows))) + (t/is (nil? (:deleted-at (first rows))))))) + +(t/deftest delete-file-object-thumbnails-cross-file + (let [profile (th/create-profile* 1) + file1 (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + file2 (th/create-file* 2 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page1-id (first (get-in file1 [:data :pages])) + page2-id (first (get-in file2 [:data :pages])) + oid1 (thc/fmt-object-id (:id file1) page1-id (uuid/random) "frame") + oid2 (thc/fmt-object-id (:id file2) page2-id (uuid/random) "frame")] + + ;; Create thumbnails on both files + (doseq [[oid fid] [[oid1 (:id file1)] [oid2 (:id file2)]]] + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id fid + :object-id oid + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out))))) + + ;; Batch delete from both files in one call + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid1 oid2]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; Verify both are soft-deleted + (let [rows1 (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file1)} + {::db/remove-deleted false}) + rows2 (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file2)} + {::db/remove-deleted false})] + (t/is (= 1 (count rows1))) + (t/is (some? (:deleted-at (first rows1)))) + (t/is (= 1 (count rows2))) + (t/is (some? (:deleted-at (first rows2))))))) + +(t/deftest delete-file-object-thumbnails-cross-file-unauthorized + (let [profile1 (th/create-profile* 1) + profile2 (th/create-profile* 2) + file1 (th/create-file* 1 {:profile-id (:id profile1) + :project-id (:default-project-id profile1) + :is-shared false}) + file2 (th/create-file* 2 {:profile-id (:id profile2) + :project-id (:default-project-id profile2) + :is-shared false}) + page1-id (first (get-in file1 [:data :pages])) + page2-id (first (get-in file2 [:data :pages])) + oid1 (thc/fmt-object-id (:id file1) page1-id (uuid/random) "frame") + oid2 (thc/fmt-object-id (:id file2) page2-id (uuid/random) "frame")] + + ;; Create thumbnails on both files (by their respective owners) + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile1) + :file-id (:id file1) + :object-id oid1 + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out)))) + + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile2) + :file-id (:id file2) + :object-id oid2 + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out)))) + + ;; profile1 tries to batch delete thumbnails from both files + ;; (profile1 does NOT have access to file2) + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile1) + :object-ids [oid1 oid2]} + out (th/command! data)] + (t/is (some? (:error out))) + (t/is (th/ex-info? (:error out))) + (t/is (= :not-found (th/ex-type (:error out))))) + + ;; Verify NEITHER thumbnail was deleted (all-or-nothing) + (let [rows1 (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file1)} + {::db/remove-deleted false}) + rows2 (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file2)} + {::db/remove-deleted false})] + (t/is (= 1 (count rows1))) + (t/is (nil? (:deleted-at (first rows1)))) + (t/is (= 1 (count rows2))) + (t/is (nil? (:deleted-at (first rows2))))))) + +(t/deftest delete-file-object-thumbnails-media-touch + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (first (get-in file [:data :pages])) + oid1 (thc/fmt-object-id (:id file) page-id (uuid/random) "frame") + oid2 (thc/fmt-object-id (:id file) page-id (uuid/random) "frame")] + + ;; Create two thumbnails + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id (:id file) + :object-id oid1 + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out)))) + + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id (:id file) + :object-id oid2 + :media {:filename "sample.jpg" + :size 312043 + :path (th/tempfile "backend_tests/test_files/sample.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out)))) + + ;; Get media-ids for both thumbnails + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {:order-by [[:created-at :asc]]}) + mid1 (:media-id (first rows)) + mid2 (:media-id (second rows))] + + ;; Verify storage objects exist (they are created with touched-at already set) + (t/is (some? (th/db-get :storage-object {:id mid1}))) + (t/is (some? (th/db-get :storage-object {:id mid2}))) + + ;; Batch delete both thumbnails + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid1 oid2]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; After soft-delete, storage objects should STILL exist + ;; (they are only garbage-collected later by storage-gc-touched task) + (t/is (some? (th/db-get :storage-object {:id mid1}))) + (t/is (some? (th/db-get :storage-object {:id mid2})))))) + +(t/deftest delete-file-object-thumbnails-max-batch + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (first (get-in file [:data :pages])) + cnt 200 + oids (vec (repeatedly cnt + #(thc/fmt-object-id (:id file) page-id + (uuid/random) "frame")))] + + ;; Create 200 thumbnails + (doseq [oid oids] + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id (:id file) + :object-id oid + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out))))) + + ;; Verify all 200 exist + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {::db/remove-deleted false})] + (t/is (= cnt (count rows)))) + + ;; Batch delete all 200 in one call + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids oids} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; Verify all 200 are now soft-deleted + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {::db/remove-deleted false})] + (t/is (= cnt (count rows))) + (doseq [row rows] + (t/is (some? (:deleted-at row))))))) + +(t/deftest delete-file-object-thumbnails-single + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (first (get-in file [:data :pages])) + oid (thc/fmt-object-id (:id file) page-id (uuid/random) "frame")] + + ;; Create a single thumbnail + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id (:id file) + :object-id oid + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out)))) + + ;; Batch delete just one + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; Verify it's soft-deleted + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {::db/remove-deleted false})] + (t/is (= 1 (count rows))) + (t/is (some? (:deleted-at (first rows))))))) + +(t/deftest delete-file-object-thumbnails-same-object-twice-in-batch + (let [profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page-id (first (get-in file [:data :pages])) + oid (thc/fmt-object-id (:id file) page-id (uuid/random) "frame")] + + ;; Create one thumbnail + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id (:id file) + :object-id oid + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out)))) + + ;; Batch delete with the same object-id listed twice + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid oid]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; Verify it's soft-deleted (only one row) + (let [rows (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file)} + {::db/remove-deleted false})] + (t/is (= 1 (count rows))) + (t/is (some? (:deleted-at (first rows))))))) + +(t/deftest delete-file-object-thumbnails-keeps-other-files-intact + (let [profile (th/create-profile* 1) + file1 (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + file2 (th/create-file* 2 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + page1-id (first (get-in file1 [:data :pages])) + page2-id (first (get-in file2 [:data :pages])) + oid1 (thc/fmt-object-id (:id file1) page1-id (uuid/random) "frame") + oid2 (thc/fmt-object-id (:id file2) page2-id (uuid/random) "frame")] + + ;; Create thumbnails on both files + (doseq [[oid fid] [[oid1 (:id file1)] [oid2 (:id file2)]]] + (let [data {::th/type :create-file-object-thumbnail + ::rpc/profile-id (:id profile) + :file-id fid + :object-id oid + :media {:filename "sample.jpg" + :size 7923 + :path (th/tempfile "backend_tests/test_files/sample2.jpg") + :mtype "image/jpeg"}} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (map? (:result out))))) + + ;; Delete only thumbnail from file1 + (let [data {::th/type :delete-file-object-thumbnails + ::rpc/profile-id (:id profile) + :object-ids [oid1]} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) + + ;; Verify file1's thumbnail is deleted, file2's is not + (let [rows1 (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file1)} + {::db/remove-deleted false}) + rows2 (th/db-query :file-tagged-object-thumbnail + {:file-id (:id file2)} + {::db/remove-deleted false})] + (t/is (= 1 (count rows1))) + (t/is (some? (:deleted-at (first rows1)))) + (t/is (= 1 (count rows2))) + (t/is (nil? (:deleted-at (first rows2))))))) + diff --git a/frontend/src/app/main/data/workspace/thumbnails.cljs b/frontend/src/app/main/data/workspace/thumbnails.cljs index 80c83f6153..3770a24a4a 100644 --- a/frontend/src/app/main/data/workspace/thumbnails.cljs +++ b/frontend/src/app/main/data/workspace/thumbnails.cljs @@ -85,40 +85,70 @@ (let [request (create-request file-id page-id shape-id tag)] (q/enqueue-unique queue request (partial render-thumbnail state file-id page-id shape-id tag)))) +(defn- clear-thumbnail-batch + [] + (let [pending (volatile! nil)] + (ptk/reify ::clear-thumbnail-batch + ptk/UpdateEvent + (update [_ state] + (let [items (get state ::thumbnails-deletion-queue)] + (when (seq items) + (vreset! pending items)) + (dissoc state ::thumbnails-deletion-queue))) + + ptk/WatchEvent + (watch [_ _ _] + (let [items (reduce-kv (fn [acc object-id uri] + (when (str/starts-with? uri "blob:") + (tm/schedule-on-idle (partial wapi/revoke-uri uri))) + (conj acc object-id)) + [] + @pending)] + (l/dbg :hint "clear-thumbnail-batch" :total (count items)) + (->> (rx/from (partition-all 200 items)) + (rx/mapcat + (fn [batch] + (l/dbg :hint "clear-thumbnail-batch" :batch-size (count batch)) + (->> (rp/cmd! :delete-file-object-thumbnails + {:object-ids (vec batch)}) + (rx/catch rx/empty) + (rx/ignore)))))))))) + +(defn remove-from-deletion-queue + "Removes an object-id from the pending deletion queue in state. + Used by update-thumbnail to cancel a pending batched delete before + creating a new thumbnail for the same object." + [object-id] + (ptk/reify ::remove-from-deletion-queue + ptk/UpdateEvent + (update [_ state] + (update state ::thumbnails-deletion-queue dissoc object-id)))) + (defn clear-thumbnail ([file-id page-id frame-id tag] (clear-thumbnail file-id (thc/fmt-object-id file-id page-id frame-id tag))) - ([file-id object-id] - (let [pending (volatile! false)] - (ptk/reify ::clear-thumbnail - cljs.core/IDeref - (-deref [_] object-id) + ([_file-id object-id] + (ptk/reify ::clear-thumbnail + cljs.core/IDeref + (-deref [_] object-id) - ptk/UpdateEvent - (update [_ state] + ptk/UpdateEvent + (update [_ state] + (let [uri (dm/get-in state [:thumbnails object-id])] + (l/dbg :hint "clear-thumbnail" :object-id object-id :uri uri) (-> state - (update :thumbnails - (fn [thumbs] - (if-let [uri (get thumbs object-id)] - (do (vreset! pending uri) - (dissoc thumbs object-id)) - thumbs))) - (update :thumbnails-meta dissoc object-id))) + (update ::thumbnails-deletion-queue assoc object-id uri) + (update :thumbnails dissoc object-id) + (update :thumbnails-meta dissoc object-id)))) - ptk/WatchEvent - (watch [_ _ _] - (if-let [uri @pending] - (do - (l/trc :hint "clear-thumbnail" :uri uri) - (when (str/starts-with? uri "blob:") - (tm/schedule-on-idle (partial wapi/revoke-uri uri))) - - (let [params {:file-id file-id - :object-id object-id}] - (->> (rp/cmd! :delete-file-object-thumbnail params) - (rx/catch rx/empty) - (rx/ignore)))) - (rx/empty))))))) + ptk/WatchEvent + (watch [_ _ stream] + (let [stopper-s (->> stream + (rx/filter (ptk/type? ::clear-thumbnail)))] + (->> (rx/timer 200) + (rx/take 1) + (rx/map (fn [_] (clear-thumbnail-batch))) + (rx/take-until stopper-s))))))) (defn assoc-thumbnail [object-id uri] @@ -173,7 +203,8 @@ :tag (or tag "frame")}] (rx/merge - (rx/of (assoc-thumbnail object-id uri)) + (rx/of (assoc-thumbnail object-id uri) + (remove-from-deletion-queue object-id)) (->> (rp/cmd! :create-file-object-thumbnail params) (rx/catch rx/empty) (rx/ignore)))))) @@ -305,6 +336,14 @@ ;; and interrupt any ongoing update-thumbnail process ;; related to current frame-id (->> all-commits-s + ;; Ensure each clear-thumbnail event is dispatched in its + ;; own macrotask tick. Without this, multiple changes + ;; arriving on the same synchronous tick would emit + ;; several clear-thumbnail events back-to-back, causing + ;; their debounce timers (rx/take-until stopper-s) to + ;; race and potentially leave multiple clear-thumbnail-batch + ;; timers alive simultaneously. + (rx/observe-on :async) (rx/mapcat (fn [[tag frame-id]] (rx/of (clear-thumbnail file-id page-id frame-id tag))))) diff --git a/frontend/test/frontend_tests/data/workspace_thumbnails_test.cljs b/frontend/test/frontend_tests/data/workspace_thumbnails_test.cljs index f9c21e9be5..2ccf1c51fc 100644 --- a/frontend/test/frontend_tests/data/workspace_thumbnails_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_thumbnails_test.cljs @@ -6,9 +6,18 @@ (ns frontend-tests.data.workspace-thumbnails-test (:require + [app.common.thumbnails :as thc] [app.common.uuid :as uuid] [app.main.data.workspace.thumbnails :as thumbnails] - [cljs.test :as t :include-macros true])) + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.mock :as mock] + [potok.v2.core :as ptk])) + +;; The qualified keyword used internally by app.main.data.workspace.thumbnails +;; for tracking the pending deletion queue in application state. +(def ^:private deletion-queue-key + :app.main.data.workspace.thumbnails/thumbnails-deletion-queue) (t/deftest extract-frame-changes-handles-cyclic-frame-links (let [page-id (uuid/next) @@ -33,4 +42,257 @@ :component-root true}}}}}] (t/is (= #{["frame" root-id] ["component" shape-b-id]} - (#'thumbnails/extract-frame-changes page-id [event [old-data new-data]]))))) + (#'thumbnails/extract-frame-changes page-id [event [old-data new-data]])))) + + ;; --- Batch deletion queue state management --- + + (t/deftest clear-thumbnail-adds-to-deletion-queue + (let [file-id (uuid/next) + object-id (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + uri "blob:http://localhost/test-thumb" + event (thumbnails/clear-thumbnail file-id object-id) + state {:thumbnails {object-id uri}} + result (ptk/update event state)] + ;; Thumbnail removed from the map + (t/is (nil? (get-in result [:thumbnails object-id]))) + ;; Object-id added to the deletion queue with its URI + (t/is (= uri (get-in result [deletion-queue-key object-id]))))) + + (t/deftest clear-thumbnail-keeps-other-thumbnails + (let [file-id (uuid/next) + object-id1 (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + object-id2 (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + uri1 "blob:http://localhost/thumb-1" + uri2 "blob:http://localhost/thumb-2" + event (thumbnails/clear-thumbnail file-id object-id1) + state {:thumbnails {object-id1 uri1 object-id2 uri2}} + result (ptk/update event state)] + ;; Only the cleared thumbnail is removed + (t/is (nil? (get-in result [:thumbnails object-id1]))) + (t/is (= uri2 (get-in result [:thumbnails object-id2]))) + ;; Only the cleared thumbnail is queued + (t/is (= uri1 (get-in result [deletion-queue-key object-id1]))) + (t/is (nil? (get-in result [deletion-queue-key object-id2]))))) + + (t/deftest clear-thumbnail-accumulates-in-queue + (let [file-id (uuid/next) + object-id1 (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + object-id2 (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + uri1 "blob:http://localhost/thumb-1" + uri2 "blob:http://localhost/thumb-2" + event1 (thumbnails/clear-thumbnail file-id object-id1) + event2 (thumbnails/clear-thumbnail file-id object-id2) + state {:thumbnails {object-id1 uri1 object-id2 uri2}} + state1 (ptk/update event1 state) + state2 (ptk/update event2 state1)] + ;; Both removed from thumbnails + (t/is (nil? (get-in state2 [:thumbnails object-id1]))) + (t/is (nil? (get-in state2 [:thumbnails object-id2]))) + ;; Both accumulated in the queue + (t/is (= uri1 (get-in state2 [deletion-queue-key object-id1]))) + (t/is (= uri2 (get-in state2 [deletion-queue-key object-id2]))) + (t/is (= 2 (count (get state2 deletion-queue-key)))))) + + (t/deftest remove-from-deletion-queue-removes-entry + (let [file-id (uuid/next) + object-id (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + event (thumbnails/remove-from-deletion-queue object-id) + state {deletion-queue-key {object-id "blob:http://localhost/thumb"}} + result (ptk/update event state)] + (t/is (nil? (get-in result [deletion-queue-key object-id]))) + (t/is (empty? (get result deletion-queue-key))))) + + (t/deftest remove-from-deletion-queue-keeps-other-entries + (let [file-id (uuid/next) + object-id1 (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + object-id2 (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + uri1 "blob:http://localhost/thumb-1" + uri2 "blob:http://localhost/thumb-2" + event (thumbnails/remove-from-deletion-queue object-id1) + state {deletion-queue-key {object-id1 uri1 + object-id2 uri2}} + result (ptk/update event state)] + ;; Only the specified entry is removed + (t/is (nil? (get-in result [deletion-queue-key object-id1]))) + (t/is (= uri2 (get-in result [deletion-queue-key object-id2]))) + (t/is (= 1 (count (get result deletion-queue-key)))))) + + (t/deftest remove-before-clear-cancels-pending-delete + (let [file-id (uuid/next) + object-id (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + uri "blob:http://localhost/thumb" + ;; Step 1: clear-thumbnail queues the delete + state1 (ptk/update (thumbnails/clear-thumbnail file-id object-id) + {:thumbnails {object-id uri}}) + ;; Step 2: remove-from-deletion-queue cancels the pending delete + state2 (ptk/update (thumbnails/remove-from-deletion-queue object-id) + state1)] + ;; Thumbnail was removed from :thumbnails map by clear-thumbnail + (t/is (nil? (get-in state2 [:thumbnails object-id]))) + ;; But the deletion queue entry was cancelled by remove-from-deletion-queue + (t/is (nil? (get-in state2 [deletion-queue-key object-id]))) + (t/is (empty? (get state2 deletion-queue-key))))) + + (t/deftest clear-thumbnail-batch-drains-queue + (let [file-id (uuid/next) + object-id1 (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + object-id2 (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + uri1 "blob:http://localhost/thumb-1" + uri2 "blob:http://localhost/thumb-2" + ;; Build up the queue state manually (simulating accumulated clear-thumbnails) + state {deletion-queue-key {object-id1 uri1 object-id2 uri2}} + event (#'thumbnails/clear-thumbnail-batch) + result (ptk/update event state)] + ;; The queue is drained from application state + (t/is (empty? (get result deletion-queue-key))))) + + (t/deftest clear-thumbnail-batch-empty-queue-noop + (let [state {deletion-queue-key {}} + event (#'thumbnails/clear-thumbnail-batch) + result (ptk/update event state)] + ;; State unchanged when queue is already empty + (t/is (empty? (get result deletion-queue-key))) + (t/is (= state (dissoc result deletion-queue-key))))) + + (t/deftest assoc-thumbnail-adds-to-map + (let [object-id (thc/fmt-object-id (uuid/next) (uuid/next) (uuid/next) "frame") + uri "blob:http://localhost/new-thumb" + event (#'thumbnails/assoc-thumbnail object-id uri) + state {:thumbnails {}} + result (ptk/update event state)] + (t/is (= uri (get-in result [:thumbnails object-id]))))) + + (t/deftest duplicate-thumbnail-copies-entry + (let [old-id (thc/fmt-object-id (uuid/next) (uuid/next) (uuid/next) "frame") + new-id (thc/fmt-object-id (uuid/next) (uuid/next) (uuid/next) "frame") + uri "blob:http://localhost/dup-thumb" + event (thumbnails/duplicate-thumbnail old-id new-id) + state {:thumbnails {old-id uri}} + result (ptk/update event state)] + (t/is (= uri (get-in result [:thumbnails old-id]))) + (t/is (= uri (get-in result [:thumbnails new-id]))))) + + ;; --- Async WatchEvent tests --- + + (defn- make-obj-ids + "Helper to create n properly-formatted object-ids for a single file." + [file-id n] + (vec (repeatedly n #(thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame")))) + + (t/deftest clear-thumbnail-batch-watch-calls-rpc-with-object-ids + (t/async done + (let [file-id (uuid/next) + oids (make-obj-ids file-id 3) + state {deletion-queue-key (zipmap oids (repeat "blob:http://test"))} + event (#'thumbnails/clear-thumbnail-batch)] + (ptk/update event state) + (mock/with-mocks + {#'app.main.repo/cmd! mock/rpc-cmd!-mock + #'app.util.timers/schedule-on-idle mock/schedule-on-idle-mock} + (fn [done'] + (->> (ptk/watch event state nil) + (rx/reduce conj []) + (rx/subs! + (fn [_] nil) + (fn [err] (t/is (nil? err)) (done')) + (fn [_] + (t/is (= 1 (count @mock/rpc-calls))) + (let [[{:keys [cmd params]}] @mock/rpc-calls] + (t/is (= :delete-file-object-thumbnails cmd)) + (t/is (= (vec oids) (:object-ids params)))) + (done'))))) + done)))) + + (t/deftest clear-thumbnail-batch-watch-partitions-large-batch + (t/async done + (let [file-id (uuid/next) + oids (make-obj-ids file-id 250) + state {deletion-queue-key (zipmap oids (repeat "blob:http://test"))} + event (#'thumbnails/clear-thumbnail-batch)] + (ptk/update event state) + (mock/with-mocks + {#'app.main.repo/cmd! mock/rpc-cmd!-mock + #'app.util.timers/schedule-on-idle mock/schedule-on-idle-mock} + (fn [done'] + (->> (ptk/watch event state nil) + (rx/reduce conj []) + (rx/subs! + (fn [_] nil) + (fn [err] (t/is (nil? err)) (done')) + (fn [_] + (t/is (= 2 (count @mock/rpc-calls))) + (let [[c1 c2] @mock/rpc-calls] + (t/is (= :delete-file-object-thumbnails (:cmd c1))) + (t/is (= :delete-file-object-thumbnails (:cmd c2))) + (t/is (= 200 (count (:object-ids (:params c1))))) + (t/is (= 50 (count (:object-ids (:params c2))))) + (t/is (= (set oids) + (set (concat (:object-ids (:params c1)) + (:object-ids (:params c2))))))) + (done'))))) + done)))) + + (t/deftest clear-thumbnail-batch-watch-revokes-blob-uris + (t/async done + (let [file-id (uuid/next) + oids (make-obj-ids file-id 2) + uris ["blob:http://localhost/thumb-1" + "blob:http://localhost/thumb-2"] + state {deletion-queue-key (zipmap oids uris)} + event (#'thumbnails/clear-thumbnail-batch)] + (ptk/update event state) + (mock/with-mocks + {#'app.main.repo/cmd! (fn [_ _] (rx/of nil)) + #'app.util.webapi/revoke-uri mock/revoke-uri-mock + #'app.util.timers/schedule-on-idle mock/schedule-on-idle-mock} + (fn [done'] + (->> (ptk/watch event state nil) + (rx/reduce conj []) + (rx/subs! + (fn [_] nil) + (fn [err] (t/is (nil? err)) (done')) + (fn [_] + (t/is (= (set uris) (set @mock/revoked-uris))) + (done'))))) + done)))) + + (t/deftest clear-thumbnail-batch-watch-empty-queue-no-rpc + (t/async done + (let [event (#'thumbnails/clear-thumbnail-batch) + state {}] + (ptk/update event state) + (mock/with-mocks + {#'app.main.repo/cmd! mock/rpc-cmd!-mock + #'app.util.timers/schedule-on-idle mock/schedule-on-idle-mock} + (fn [done'] + (->> (ptk/watch event state nil) + (rx/reduce conj []) + (rx/subs! + (fn [_] nil) + (fn [err] (t/is (nil? err)) (done')) + (fn [_] + (t/is (empty? @mock/rpc-calls)) + (done'))))) + done)))) + + (t/deftest clear-thumbnail-watch-emits-batch-after-debounce + (t/async done + (let [file-id (uuid/next) + object-id (thc/fmt-object-id file-id (uuid/next) (uuid/next) "frame") + uri "blob:http://localhost/thumb" + state {:thumbnails {object-id uri}} + event (thumbnails/clear-thumbnail file-id object-id)] + (ptk/update event state) + (mock/with-mocks + {#'beicon.v2.core/timer mock/timer-mock} + (fn [done'] + (->> (ptk/watch event state nil) + (rx/reduce conj []) + (rx/subs! + (fn [_] nil) + (fn [err] (t/is (nil? err)) (done')) + (fn [events] + (t/is (= 1 (count events))) + (t/is (ptk/event? (first events))) + (done'))))) + done))))) diff --git a/frontend/test/frontend_tests/helpers/mock.cljs b/frontend/test/frontend_tests/helpers/mock.cljs new file mode 100644 index 0000000000..bce234f4cf --- /dev/null +++ b/frontend/test/frontend_tests/helpers/mock.cljs @@ -0,0 +1,103 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns frontend-tests.helpers.mock + "Async-first mocking primitives for ClojureScript tests. + + Uses `with-redefs` — the standard CLJS mechanism for rebinding Vars + within a dynamic scope. Recording atoms (`rpc-calls`, `revoked-uris`) + persist across the entire test lifecycle, making captured data + inspectable regardless of whether callbacks fire synchronously or + asynchronously. + + The `with-mocks` helper wraps the lifecycle: + 1. Reset recording atoms + 2. Install mocks via `with-redefs` + 3. Execute `(test-fn inner-done)` + 4. `inner-done` calls `outer-done` (typically `cljs.test/async`'s done) + + Since all mock functions return synchronous `rx/of` observables, + callbacks always fire within the `with-redefs` body." + (:require + [beicon.v2.core :as rx])) + +;; ═══════════════════════════════════════════════════════════════ +;; Recording atoms +;; ═══════════════════════════════════════════════════════════════ + +(def rpc-calls + "Atom accumulating mocked `rp/cmd!` calls as `{:cmd kw :params map}`." + (atom [])) + +(def revoked-uris + "Atom accumulating URIs passed to `wapi/revoke-uri`." + (atom [])) + +;; ═══════════════════════════════════════════════════════════════ +;; Mock implementations +;; ═══════════════════════════════════════════════════════════════ + +(defn rpc-cmd!-mock + "Records [cmd params] in [[rpc-calls]], returns `(rx/of nil)`." + [cmd params] + (swap! rpc-calls conj {:cmd cmd :params params}) + (rx/of nil)) + +(defn revoke-uri-mock + "Records `uri` in [[revoked-uris]]." + [uri] + (swap! revoked-uris conj uri)) + +(defn schedule-on-idle-mock + "Calls `f` immediately instead of deferring to the idle queue." + [f] + (f)) + +(defn timer-mock + "Returns `(rx/of :immediate)` so debounce timers fire instantly + during tests." + [_ms] + (rx/of :immediate)) + +;; ═══════════════════════════════════════════════════════════════ +;; Lifecycle +;; ═══════════════════════════════════════════════════════════════ + +(defn reset! + "Clear all recording atoms. Called automatically by [[with-mocks]]." + [] + (reset! rpc-calls []) + (reset! revoked-uris [])) + +;; ═══════════════════════════════════════════════════════════════ +;; Public API +;; ═══════════════════════════════════════════════════════════════ + +(defn with-mocks + "Resets recording atoms, installs `mocks` via `with-redefs`, then + calls `(test-fn inner-done)`. + + `mocks` is a map of `Var → mock-fn` (e.g. `{#'rp/cmd! mock-fn}`). + `inner-done` tears down the `with-redefs` (by returning) and calls + `outer-done` (the `cljs.test/async` `done` callback). + + Example: + + (t/deftest my-async-test + (t/async done + (mock/with-mocks + {#'rp/cmd! mock/rpc-cmd!-mock} + (fn [done'] + (->> (some-async-flow) + (rx/subs! + (fn [v] ...) + (fn [err] (done')) + (fn [] (done'))))))))" + [mocks test-fn outer-done] + (reset!) + (apply with-redefs (mapcat identity mocks) + (test-fn (fn inner-done [] + (outer-done)))))