diff --git a/backend/src/app/rpc/commands/files_thumbnails.clj b/backend/src/app/rpc/commands/files_thumbnails.clj index 1a821c4027..1ff4e46d40 100644 --- a/backend/src/app/rpc/commands/files_thumbnails.clj +++ b/backend/src/app/rpc/commands/files_thumbnails.clj @@ -15,10 +15,12 @@ [app.common.schema :as sm] [app.common.thumbnails :as thc] [app.common.time :as ct] + [app.common.types.objects-map :as omap] [app.common.types.shape-tree :as ctt] [app.config :as cf] [app.db :as db] [app.db.sql :as-alias sql] + [app.features.fdata :as fdata] [app.loggers.audit :as-alias audit] [app.loggers.webhooks :as-alias webhooks] [app.media.validation :as media.v] @@ -92,26 +94,38 @@ ;; --- COMMAND QUERY: get-file-data-for-thumbnail -;; We need to improve how we set frame for thumbnail in order to avoid -;; loading all pages into memory for find the frame set for thumbnail. +(defn- get-thumbnail-frame-candidates + "Only decode the objects whose encoded form can carry the flag." + [objects] + (if (omap/objects-map? objects) + (keep (fn [[id encoded]] + (when (str/includes? encoded "use-for-thumbnail") + (get objects id))) + (omap/get-data objects)) + (vals objects))) + +(defn- get-thumbnail-frame + "Find the frame marked to be used as thumbnail, loading pages lazily. + The returned frame has the :page-id assoc'ed." + [data] + (d/seek some? + (for [page (-> data :pages-index vals) + :let [page (cond-> page (pmap/pointer-map? page) deref)]] + (some-> (d/seek #(and (cfh/frame-shape? %) + (or (:use-for-thumbnail %) + (:use-for-thumbnail? %))) ; NOTE: backward comp (remove on v1.21) + (get-thumbnail-frame-candidates (:objects page))) + (assoc :page-id (:id page)))))) (defn get-file-data-for-thumbnail [{:keys [::db/conn] :as cfg} {:keys [data id] :as file} strip-frames-with-thumbnails] - (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 [{:keys [data]}] - (d/seek #(or (:use-for-thumbnail %) - (:use-for-thumbnail? %)) ; NOTE: backward comp (remove on v1.21) - (for [page (-> data :pages-index vals) - frame (-> page :objects ctt/get-frames)] - (assoc frame :page-id (:id page))))) - - ;; function responsible to filter objects data structure of + (letfn [;; function responsible to filter objects data structure of ;; all unneeded shapes if a concrete frame is provided. If no - ;; frame, the objects is returned untouched. + ;; frame, all objects are returned as a plain map. (filter-objects [objects frame-id] - (d/index-by :id (cfh/get-children-with-self objects frame-id))) + (if (some? frame-id) + (d/index-by :id (cfh/get-children-with-self objects frame-id)) + (into {} objects))) ;; function responsible of assoc available thumbnails ;; to frames and remove all children shapes from objects if @@ -152,29 +166,28 @@ objects)))] - (let [frame (get-thumbnail-frame file) - frame-id (:id frame) - page-id (or (:page-id frame) - (-> data :pages first)) + (binding [pmap/*load-fn* (partial fdata/load-pointer cfg id)] + (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]) - page (cond-> page (pmap/pointer-map? page) deref) - frame-ids (if (some? frame) (list frame-id) (map :id (ctt/get-frames (:objects page)))) + page (dm/get-in data [:pages-index page-id]) + page (cond-> page (pmap/pointer-map? page) deref) + frame-ids (if (some? frame) (list frame-id) (map :id (ctt/get-frames (:objects page)))) - obj-ids (map #(thc/fmt-object-id (:id file) page-id % "frame") frame-ids) - thumbs (get-object-thumbnails conn id obj-ids)] + obj-ids (map #(thc/fmt-object-id (:id file) page-id % "frame") frame-ids) + thumbs (get-object-thumbnails conn 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)) + (cond-> (update page :objects filter-objects frame-id) + ;; If we have frame, we need to specify it on the page level + (some? frame-id) + (assoc :thumbnail-frame-id frame-id) - ;; Assoc the available thumbnails and prune not visible shapes - ;; for avoid transfer unnecessary data. - strip-frames-with-thumbnails - (update :objects assoc-thumbnails page-id thumbs))))) + ;; Assoc the available thumbnails and prune not visible shapes + ;; for avoid transfer unnecessary data. + strip-frames-with-thumbnails + (update :objects assoc-thumbnails page-id thumbs)))))) (def ^:private schema:get-file-data-for-thumbnail @@ -204,7 +217,6 @@ :file-id file-id) file (bfc/get-file cfg file-id :include-deleted? true - :realize? true :read-only? true) strip-frames-with-thumbnails (or (nil? strip-frames-with-thumbnails) ;; if not present, default to true diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 2ac2bf8424..e2f7f631a0 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -1373,6 +1373,73 @@ ;; (app.common.pprint/pprint rows) (t/is (= 1 (count rows))))))) +(t/deftest file-data-for-thumbnail-frame-on-other-page + (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}) + page1-id (get-in file [:data :pages 0]) + page2-id (uuid/next) + rect-id (uuid/next) + frame-id (uuid/next) + + get-data (fn [] + (th/command! {::th/type :get-file-data-for-thumbnail + ::rpc/profile-id (:id prof) + :file-id (:id file) + :features cfeat/supported-features}))] + + (th/update-file* {:file-id (:id file) + :profile-id (:id prof) + :revn 0 + :vern 0 + :changes [{:type :add-obj + :page-id page1-id + :id rect-id + :parent-id uuid/zero + :frame-id uuid/zero + :obj (cts/setup-shape + {:id rect-id + :name "rect" + :type :rect})} + {:type :add-page + :id page2-id + :name "page 2"} + {:type :add-obj + :page-id page2-id + :id frame-id + :parent-id uuid/zero + :frame-id uuid/zero + :obj (cts/setup-shape + {:id frame-id + :name "frame" + :type :frame})}]}) + + (t/testing "no marked frame falls back to the first page" + (let [{:keys [error result]} (get-data)] + (t/is (nil? error)) + (t/is (= page1-id (get-in result [:page :id]))) + (t/is (nil? (get-in result [:page :thumbnail-frame-id]))) + (t/is (contains? (get-in result [:page :objects]) rect-id)))) + + (th/update-file* {:file-id (:id file) + :profile-id (:id prof) + :revn 1 + :vern 0 + :changes [{:type :mod-obj + :page-id page2-id + :id frame-id + :operations [{:type :set + :attr :use-for-thumbnail + :val true}]}]}) + + (t/testing "marked frame on another page is used" + (let [{:keys [error result]} (get-data)] + (t/is (nil? error)) + (t/is (= page2-id (get-in result [:page :id]))) + (t/is (= frame-id (get-in result [:page :thumbnail-frame-id]))) + (t/is (contains? (get-in result [:page :objects]) frame-id)))))) + (t/deftest file-thumbnail-ops (let [prof (th/create-profile* 1 {:is-active true}) file (th/create-file* 1 {:profile-id (:id prof)