Penpot/penpot
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-02-12 18:18:24 -05:00
Code Issues Activity

🐛 Fix problem with duplicated ids for thumbnails

Browse source
This commit is contained in:
alonso.torres 2022-05-09 12:35:34 +02:00
parent 4cdf8cec4e
commit 8cd7f61150
13 changed files with 74 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
backend/src/app/migrations.clj
View file

@ -223,6 +223,9 @@
{:name "0071-add-file-object-thumbnail-table"
:fn (mg/resource "app/migrations/sql/0071-add-file-object-thumbnail-table.sql")}
{:name "0072-mod-file-object-thumbnail-table"
:fn (mg/resource "app/migrations/sql/0072-mod-file-object-thumbnail-table.sql")}
])

4
backend/src/app/migrations/sql/0072-mod-file-object-thumbnail-table.sql Normal file
View file

@ -0,0 +1,4 @@
TRUNCATE TABLE file_object_thumbnail;
ALTER TABLE file_object_thumbnail
ALTER COLUMN object_id TYPE text;

2
backend/src/app/rpc/mutations/files.clj
View file

@ -487,7 +487,7 @@
update set data = ?;")
(s/def ::data (s/nilable ::us/string))
(s/def ::object-id ::us/uuid)
(s/def ::object-id ::us/string)
(s/def ::upsert-file-object-thumbnail
(s/keys :req-un [::profile-id ::file-id ::object-id ::data]))

27
backend/src/app/rpc/queries/files.clj
View file

@ -197,13 +197,13 @@
(->> (db/exec! pool [sql file-id])
(d/index-by :object-id :data))))
([{:keys [pool]} file-id frame-ids]
([{:keys [pool]} file-id object-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))]
ids (db/create-array conn "text" (seq object-ids))]
(->> (db/exec! conn [sql file-id ids])
(d/index-by :object-id :data))))))
@ -298,19 +298,21 @@
;; function responsible of assoc available thumbnails
;; to frames and remove all children shapes from objects if
;; thumbnails is available
(assoc-thumbnails [objects thumbnails]
(assoc-thumbnails [objects page-id 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)]
(if-let [frame (-> frames first)]
(let [frame-id (:id frame)
object-id (str page-id frame-id)
frame (if-let [thumb (get thumbnails object-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)))
(recur (-> (assoc objects frame-id frame)
(d/without-keys (cph/get-children-ids objects frame-id)))
(rest frames))
(recur (assoc objects id frame)
(recur (assoc objects frame-id frame)
(rest frames))))
objects)))]
@ -319,10 +321,11 @@
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)))
page (dm/get-in data [:pages-index page-id])
frame-ids (if (some? frame) (list frame-id) (map :id (cph/get-frames (:objects page))))
obj-ids (map #(str page-id %) frame-ids)
thumbs (retrieve-object-thumbnails cfg id obj-ids)]
(cond-> page
@ -335,7 +338,7 @@
;; Assoc the available thumbnails and prune not visible shapes
;; for avoid transfer unnecesary data.
:always
(update :objects assoc-thumbnails thumbs)))))
(update :objects assoc-thumbnails page-id thumbs)))))
(s/def ::file-data-for-thumbnail
(s/keys :req-un [::profile-id ::file-id]))

15
backend/src/app/tasks/file_gc.clj
View file

@ -12,6 +12,7 @@
(: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]
@ -125,10 +126,14 @@
{:columns [:object-id]})
(into #{} (map :object-id)))
using (->> (concat (vals (:pages-index data))
(vals (:components data)))
(into #{} (comp (map :objects)
(mapcat keys))))
get-objects-ids
(fn [{:keys [id objects]}]
(->> (cph/get-frames objects)
(map #(str id (:id %)))))
using (into #{}
(mapcat get-objects-ids)
(vals (:pages-index data)))
unused (set/difference stored using)]
@ -136,7 +141,7 @@
(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)])]
res (db/exec-one! conn [sql file-id (db/create-array conn "text" unused)])]
(l/debug :hint "delete object thumbnails" :total (:next.jdbc/update-count res))))))
(defn- clean-file-thumbnails!

8
backend/test/app/services_files_test.clj
View file

@ -527,7 +527,7 @@
(let [data {::th/type :upsert-file-object-thumbnail
:profile-id (:id prof)
:file-id (:id file)
:object-id frame1-id
:object-id (str page-id frame1-id)
:data "random-data-1"}
{:keys [error result] :as out} (th/mutation! data)]
@ -553,7 +553,7 @@
(let [data {::th/type :upsert-file-object-thumbnail
:profile-id (:id prof)
:file-id (:id file)
:object-id frame1-id
:object-id (str page-id frame1-id)
:data nil}
{:keys [error result] :as out} (th/mutation! data)]
(t/is (nil? error))
@ -579,7 +579,7 @@
(let [data {::th/type :upsert-file-object-thumbnail
:profile-id (:id prof)
:file-id (:id file)
:object-id frame1-id
:object-id (str page-id frame1-id)
:data "new-data"}
{:keys [error result] :as out} (th/mutation! data)]
(t/is (nil? error))
@ -602,7 +602,7 @@
(let [data {::th/type :upsert-file-object-thumbnail
:profile-id (:id prof)
:file-id (:id file)
:object-id (uuid/next)
:object-id (str page-id (uuid/next))
:data "new-data-2"}
{:keys [error result] :as out} (th/mutation! data)]
(t/is (nil? error))

40
frontend/src/app/main/data/workspace/thumbnails.cljs
View file

@ -7,6 +7,7 @@
(ns app.main.data.workspace.thumbnails
(:require
[app.common.data :as d]
[app.common.data.macros :as dm]
[app.common.pages.helpers :as cph]
[app.common.uuid :as uuid]
[app.main.data.workspace.changes :as dch]
@ -27,57 +28,44 @@
(defn update-thumbnail
"Updates the thumbnail information for the given frame `id`"
[id data]
(let [lock (uuid/next)]
[page-id frame-id data]
(let [lock (uuid/next)
object-id (dm/str page-id frame-id)]
(ptk/reify ::update-thumbnail
IDeref
(-deref [_] {:id id :data data})
(-deref [_] {:object-id object-id :data data})
ptk/UpdateEvent
(update [_ state]
(-> state
(assoc-in [:workspace-file :thumbnails id] data)
(cond-> (nil? (get-in state [::update-thumbnail-lock id]))
(assoc-in [::update-thumbnail-lock id] lock))))
(assoc-in [:workspace-file :thumbnails object-id] data)
(cond-> (nil? (get-in state [::update-thumbnail-lock object-id]))
(assoc-in [::update-thumbnail-lock object-id] lock))))
ptk/WatchEvent
(watch [_ state stream]
(when (= lock (get-in state [::update-thumbnail-lock id]))
(when (= lock (get-in state [::update-thumbnail-lock object-id]))
(let [stopper (->> stream (rx/filter (ptk/type? :app.main.data.workspace/finalize)))
params {:file-id (:current-file-id state)
:object-id id}]
:object-id object-id}]
;; Sends the first event and debounce the rest. Will only make one update once
;; the 2 second debounce is finished
(rx/merge
(->> stream
(rx/filter (ptk/type? ::update-thumbnail))
(rx/map deref)
(rx/filter #(= id (:id %)))
(rx/filter #(= object-id (:object-id %)))
(rx/debounce 2000)
(rx/take 1)
(rx/map :data)
(rx/flat-map #(rp/mutation! :upsert-file-object-thumbnail (assoc params :data %)))
(rx/map #(fn [state] (d/dissoc-in state [::update-thumbnail-lock id])))
(rx/map #(fn [state] (d/dissoc-in state [::update-thumbnail-lock object-id])))
(rx/take-until stopper))
(->> (rx/of (update-thumbnail id data))
(->> (rx/of (update-thumbnail page-id frame-id data))
(rx/observe-on :async)))))))))
(defn remove-thumbnail
[id]
(ptk/reify ::remove-thumbnail
ptk/UpdateEvent
(update [_ state]
(-> state (d/dissoc-in [:workspace-file :thumbnails id])))
ptk/WatchEvent
(watch [_ state _]
(let [params {:file-id (:current-file-id state)
:object-id id
:data nil}]
(->> (rp/mutation! :upsert-file-object-thumbnail params)
(rx/ignore))))))
(defn- extract-frame-changes
"Process a changes set in a commit to extract the frames that are changing"
[[event [old-objects new-objects]]]

7
frontend/src/app/main/refs.cljs
View file

@ -399,8 +399,11 @@
(l/derived #(dm/get-in % [:workspace-file :thumbnails] {}) st/state))
(defn thumbnail-frame-data
[frame-id]
(l/derived #(get % frame-id) thumbnail-data))
[page-id frame-id]
(l/derived
(fn [thumbnails]
(get thumbnails (dm/str page-id frame-id)))
thumbnail-data))
(def workspace-text-modifier
(l/derived :workspace-text-modifier st/state))

8
frontend/src/app/main/ui/workspace/shapes/frame.cljs
View file

@ -10,6 +10,7 @@
[app.common.data.macros :as dm]
[app.main.data.workspace.thumbnails :as dwt]
[app.main.refs :as refs]
[app.main.ui.context :as ctx]
[app.main.ui.hooks :as hooks]
[app.main.ui.shapes.embed :as embed]
[app.main.ui.shapes.frame :as frame]
@ -67,8 +68,11 @@
;; Thumbnail data
frame-id (:id shape)
thumbnail-data-ref (mf/use-memo (mf/deps frame-id) #(refs/thumbnail-frame-data frame-id))
page-id (mf/use-ctx ctx/current-page-id)
thumbnail-data-ref (mf/use-memo (mf/deps page-id frame-id) #(refs/thumbnail-frame-data page-id frame-id))
thumbnail-data (mf/deref thumbnail-data-ref)
thumbnail? (and thumbnail? (or (some? (:thumbnail shape)) (some? thumbnail-data)))
;; References to the current rendered node and the its parentn
@ -84,7 +88,7 @@
disable-thumbnail? (d/not-empty? (dm/get-in modifiers [(:id shape) :modifiers]))
[on-load-frame-dom thumb-renderer]
(ftr/use-render-thumbnail shape node-ref rendered? thumbnail? disable-thumbnail?)
(ftr/use-render-thumbnail page-id shape node-ref rendered? thumbnail? disable-thumbnail?)
on-frame-load
(fns/use-node-store thumbnail? node-ref rendered?)]

1
frontend/src/app/main/ui/workspace/shapes/frame/node_store.cljs
View file

@ -28,7 +28,6 @@
(when (and (some? node) (nil? @node-ref))
(let [content (-> (.createElementNS globals/document "http://www.w3.org/2000/svg" "g")
(dom/add-class! "frame-content"))]
;;(.appendChild node content)
(reset! node-ref content)
(reset! parent-ref node)
(swap! re-render inc)))))]

4
frontend/src/app/main/ui/workspace/shapes/frame/thumbnail_render.cljs
View file

@ -33,7 +33,7 @@
(defn use-render-thumbnail
"Hook that will create the thumbnail thata"
[{:keys [id x y width height] :as shape} node-ref rendered? thumbnail? disable?]
[page-id {:keys [id x y width height] :as shape} node-ref rendered? thumbnail? disable?]
(let [frame-canvas-ref (mf/use-ref nil)
frame-image-ref (mf/use-ref nil)
@ -60,7 +60,7 @@
img-node (mf/ref-val frame-image-ref)
thumb-data (draw-thumbnail-canvas canvas-node img-node)]
(when (some? thumb-data)
(st/emit! (dw/update-thumbnail id thumb-data))
(st/emit! (dw/update-thumbnail page-id id thumb-data))
(reset! image-url nil))))))
on-update-frame

10
frontend/src/app/worker/thumbnails.cljs
View file

@ -14,6 +14,7 @@
[app.util.http :as http]
[app.worker.impl :as impl]
[beicon.core :as rx]
[debug :refer [debug?]]
[rumext.alpha :as mf]))
(defn- handle-response
@ -115,6 +116,9 @@
(rx/map render-thumbnail)
(rx/mapcat persist-thumbnail)))]
(->> (request-thumbnail file-id revn)
(rx/catch not-found? on-cache-miss)
(rx/map on-result))))
(if (debug? :disable-thumbnail-cachee)
(->> (request-data-for-thumbnail file-id revn)
(rx/map render-thumbnail))
(->> (request-thumbnail file-id revn)
(rx/catch not-found? on-cache-miss)
(rx/map on-result)))))

3
frontend/src/debug.cljs
View file

@ -61,6 +61,9 @@
;; Show text fragments outlines
:text-outline
;; Disable thumbnail cache
:disable-thumbnail-cachee
})
;; These events are excluded when we activate the :events flag