Penpot/penpot
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-01-23 06:58:58 -05:00
Code Issues Activity

✨ Improve file validation process on update-file rpc method

Browse source
This commit is contained in:
Andrey Antukh 2023-11-11 00:08:29 +01:00
parent 31c46a90b4
commit 9e4ed0ea92
1 changed files with 40 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

74
backend/src/app/rpc/commands/files_update.clj
View file

@ -178,7 +178,8 @@
(l/trace :hint "update-file" :time (dt/format-duration elapsed))))))))) (l/trace :hint "update-file" :time (dt/format-duration elapsed)))))))))
(defn update-file (defn update-file
[{:keys [::db/conn ::mtx/metrics] :as cfg} {:keys [id file features changes changes-with-metadata skip-validate] :as params}] [{:keys [::db/conn ::mtx/metrics] :as cfg}
{:keys [id file features changes changes-with-metadata] :as params}]
(binding [cfeat/*current* features (binding [cfeat/*current* features
cfeat/*previous* (:features file)] cfeat/*previous* (:features file)]
(let [update-fn (cond-> update-file* (let [update-fn (cond-> update-file*
@ -188,16 +189,6 @@
(contains? features "fdata/objects-map") (contains? features "fdata/objects-map")
(wrap-with-objects-map-context)) (wrap-with-objects-map-context))
;; TODO: this ruins performance.
;; We must find some other way to do general validation.
libraries (when (and (contains? cf/flags :file-validation)
(not skip-validate))
(let [libs (->> (files/get-file-libraries conn (:id file))
(map #(get-file conn (:id %)))
(map #(update % :data blob/decode))
(d/index-by :id))]
(assoc libs (:id file) file)))
changes (if changes-with-metadata changes (if changes-with-metadata
(->> changes-with-metadata (mapcat :changes) vec) (->> changes-with-metadata (mapcat :changes) vec)
(vec changes)) (vec changes))
@ -225,9 +216,9 @@
(let [file (assoc file :features features) (let [file (assoc file :features features)
params (-> params params (-> params
(assoc :file file) (assoc :file file)
(assoc :libraries libraries)
(assoc :changes changes) (assoc :changes changes)
(assoc ::created-at (dt/now)))] (assoc ::created-at (dt/now)))]
(-> (update-fn cfg params) (-> (update-fn cfg params)
(vary-meta assoc ::audit/replace-props (vary-meta assoc ::audit/replace-props
{:id (:id file) {:id (:id file)
@ -237,12 +228,13 @@
:team-id (:team-id file)})))))) :team-id (:team-id file)}))))))
(defn- update-file* (defn- update-file*
[{:keys [::db/conn] :as cfg} {:keys [profile-id file libraries changes session-id ::created-at skip-validate] :as params}] [{:keys [::db/conn] :as cfg}
{:keys [profile-id file changes session-id ::created-at skip-validate] :as params}]
(let [;; Process the file data in the CLIMIT context; scheduling it (let [;; Process the file data in the CLIMIT context; scheduling it
;; to be executed on a separated executor for avoid to do the ;; to be executed on a separated executor for avoid to do the
;; CPU intensive operation on vthread. ;; CPU intensive operation on vthread.
file (-> (climit/configure cfg :update-file) file (-> (climit/configure cfg :update-file)
(climit/submit! (partial update-file-data file libraries changes skip-validate)))] (climit/submit! (partial update-file-data conn file changes skip-validate)))]
(db/insert! conn :file-change (db/insert! conn :file-change
{:id (uuid/next) {:id (uuid/next)
@ -276,36 +268,44 @@
(get-lagged-changes conn params)))) (get-lagged-changes conn params))))
(defn- update-file-data (defn- update-file-data
[file libraries changes skip-validate] [conn file changes skip-validate]
(let [validate (fn [file] (let [file (update file :data (fn [data]
(when (and (cf/flags :file-validation) (-> data
(not skip-validate)) (blob/decode)
(val/validate-file file libraries :throw? true)))
file (-> file
(update :revn inc)
(update :data (fn [data]
(cond-> data
:always
(-> (blob/decode)
(assoc :id (:id file)) (assoc :id (:id file))
(pmg/migrate-data)) (pmg/migrate-data))))
:always ;; WARNING: this ruins performance; maybe we need to find
(cp/process-changes changes)))) ;; some other way to do general validation
(d/tap-r validate)) libs (when (and (contains? cf/flags :file-validation)
(not skip-validate))
;; FIXME: we need properly handle pointer-map here ????
(->> (files/get-file-libraries conn (:id file))
(into [file] (map (fn [{:keys [id]}]
(binding [pmap/*load-fn* (partial files/load-pointer conn id)]
(-> (db/get conn :file {:id id})
(files/decode-row)
(files/process-pointers deref) ; ensure all pointers resolved
(pmg/migrate-file))))))
(d/index-by :id)))]
file (if (and (contains? cfeat/*current* "fdata/objects-map") (-> file
(update :revn inc)
(update :data cp/process-changes changes)
;; If `libs` is defined, then full validation is performed
(val/validate-file! libs)
(cond-> (and (contains? cfeat/*current* "fdata/objects-map")
(not (contains? cfeat/*previous* "fdata/objects-map"))) (not (contains? cfeat/*previous* "fdata/objects-map")))
(enable-objects-map file) (enable-objects-map))
file)
file (if (and (contains? cfeat/*current* "fdata/pointer-map") (cond-> (and (contains? cfeat/*current* "fdata/pointer-map")
(not (contains? cfeat/*previous* "fdata/pointer-map"))) (not (contains? cfeat/*previous* "fdata/pointer-map")))
(enable-pointer-map file) (enable-pointer-map))
file)
] (update :data blob/encode))))
(update file :data blob/encode)))
(defn- take-snapshot? (defn- take-snapshot?
"Defines the rule when file `data` snapshot should be saved." "Defines the rule when file `data` snapshot should be saved."