Penpot/penpot
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-04-12 15:01:28 -05:00
Code Issues Activity

🐛 Fix incorrect data id assignation on creating a snapshot ()

Browse source 
* 📎 Set proper name to relink-refs mechanism function

* 🐛 Fix incorrect id assignation on snapshot file resolution

* ♻️ Use uniform api for file retrieval on file snapshot code
This commit is contained in:
Andrey Antukh 2025-02-24 11:05:16 +01:00 • committed by GitHub
parent 786383c25d
commit 15d09eb0d4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 120 additions and 97 deletions
backend
src/app
binfile
common.cljmigrations.cljv1.cljv3.clj
rpc/commands
files_snapshot.clj
srepl
helpers.clj
test/backend_tests
rpc_file_snapshot_test.clj
common/src/app/common/files
helpers.cljc

35
backend/src/app/binfile/common.clj
View file

@ -20,7 +20,6 @@
[app.config :as cf]
[app.db :as db]
[app.db.sql :as sql]
[app.features.components-v2 :as feat.compv2]
[app.features.fdata :as feat.fdata]
[app.features.file-migrations :as feat.fmigr]
[app.loggers.audit :as-alias audit]
@ -307,7 +306,7 @@
update-shapes
(fn [data {:keys [page-id shape-id]}]
(d/update-in-when data [:pages-index page-id :objects shape-id] cfh/relink-media-refs lookup-index))
(d/update-in-when data [:pages-index page-id :objects shape-id] cfh/relink-refs lookup-index))
file
(update file :data #(reduce update-shapes % media-refs))]
@ -375,7 +374,7 @@
replace the old :component-file reference with the new
ones, using the provided file-index."
[data]
(cfh/relink-media-refs data lookup-index))
(cfh/relink-refs data lookup-index))
(defn- relink-media
"A function responsible of process the :media attr of file data and
@ -523,33 +522,3 @@
(l/error :hint "file schema validation error" :cause result))))
(insert-file! cfg file opts)))
(defn register-pending-migrations!
"All features that are enabled and requires explicit migration are
added to the state for a posterior migration step."
[cfg {:keys [id features] :as file}]
(doseq [feature (-> (::features cfg)
(set/difference cfeat/no-migration-features)
(set/difference cfeat/backend-only-features)
(set/difference features))]
(vswap! *state* update :pending-to-migrate (fnil conj []) [feature id]))
file)
(defn apply-pending-migrations!
"Apply alredy registered pending migrations to files"
[cfg]
(doseq [[feature file-id] (-> *state* deref :pending-to-migrate)]
(case feature
"components/v2"
(feat.compv2/migrate-file! cfg file-id
:validate? (::validate cfg true)
:skip-on-graphic-error? true)
"fdata/shape-data-type"
nil
(ex/raise :type :internal
:code :no-migration-defined
:hint (str/ffmt "no migation for feature '%' on file importation" feature)
:feature feature))))

45
backend/src/app/binfile/migrations.clj Normal file
View file

@ -0,0 +1,45 @@
;; 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 app.binfile.migrations
"A binfile related migrations handling"
(:require
[app.binfile.common :as bfc]
[app.common.exceptions :as ex]
[app.common.features :as cfeat]
[app.features.components-v2 :as feat.compv2]
[clojure.set :as set]
[cuerdas.core :as str]))
(defn register-pending-migrations!
"All features that are enabled and requires explicit migration are
added to the state for a posterior migration step."
[cfg {:keys [id features] :as file}]
(doseq [feature (-> (::features cfg)
(set/difference cfeat/no-migration-features)
(set/difference cfeat/backend-only-features)
(set/difference features))]
(vswap! bfc/*state* update :pending-to-migrate (fnil conj []) [feature id]))
file)
(defn apply-pending-migrations!
"Apply alredy registered pending migrations to files"
[cfg]
(doseq [[feature file-id] (-> bfc/*state* deref :pending-to-migrate)]
(case feature
"components/v2"
(feat.compv2/migrate-file! cfg file-id
:validate? (::validate cfg true)
:skip-on-graphic-error? true)
"fdata/shape-data-type"
nil
(ex/raise :type :internal
:code :no-migration-defined
:hint (str/ffmt "no migation for feature '%' on file importation" feature)
:feature feature))))

3
backend/src/app/binfile/v1.clj
View file

@ -9,6 +9,7 @@
(:refer-clojure :exclude [assert])
(:require
[app.binfile.common :as bfc]
[app.binfile.migrations :as bfm]
[app.common.data :as d]
[app.common.data.macros :as dm]
[app.common.exceptions :as ex]
@ -473,7 +474,7 @@
(read-section options))))
[:v1/metadata :v1/files :v1/rels :v1/sobjects])
(bfc/apply-pending-migrations! cfg)
(bfm/apply-pending-migrations! cfg)
;; Knowing that the ids of the created files are in index,
;; just lookup them and return it as a set

5
backend/src/app/binfile/v3.clj
View file

@ -9,6 +9,7 @@
(:refer-clojure :exclude [read])
(:require
[app.binfile.common :as bfc]
[app.binfile.migrations :as bfm]
[app.common.data :as d]
[app.common.data.macros :as dm]
[app.common.exceptions :as ex]
@ -735,7 +736,7 @@
(bfc/process-file))]
(bfc/register-pending-migrations! cfg file)
(bfm/register-pending-migrations! cfg file)
(bfc/save-file! cfg file ::db/return-keys false)
file-id')))
@ -915,7 +916,7 @@
(import-file-media cfg)
(import-file-thumbnails cfg)
(bfc/apply-pending-migrations! cfg)
(bfm/apply-pending-migrations! cfg)
ids)))))))

104
backend/src/app/rpc/commands/files_snapshot.clj
View file

@ -6,6 +6,7 @@
(ns app.rpc.commands.files-snapshot
(:require
[app.binfile.common :as bfc]
[app.common.exceptions :as ex]
[app.common.logging :as l]
[app.common.schema :as sm]
@ -22,7 +23,6 @@
[app.rpc.quotes :as quotes]
[app.storage :as sto]
[app.util.blob :as blob]
[app.util.pointer-map :as pmap]
[app.util.services :as sv]
[app.util.time :as dt]
[cuerdas.core :as str]))
@ -58,26 +58,6 @@
(files/check-read-permissions! conn profile-id file-id)
(get-file-snapshots conn file-id))))
(def ^:private sql:get-file
"SELECT f.*,
p.id AS project_id,
p.team_id AS team_id
FROM file AS f
INNER JOIN project AS p ON (p.id = f.project_id)
WHERE f.id = ?")
(defn- get-file
[cfg file-id]
(let [file (->> (db/exec-one! cfg [sql:get-file file-id])
(feat.fdata/resolve-file-data cfg))]
(binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg file-id)]
(-> file
(update :data blob/decode)
(update :data feat.fdata/process-pointers deref)
(update :data feat.fdata/process-objects (partial into {}))
(update :data assoc ::id file-id)
(update :data blob/encode)))))
(defn- generate-snapshot-label
[]
(let [ts (-> (dt/now)
@ -87,49 +67,53 @@
(str "snapshot-" ts)))
(defn create-file-snapshot!
[cfg profile-id file-id label]
(let [file (get-file cfg file-id)
[cfg file & {:keys [label created-by deleted-at profile-id]
:or {deleted-at :default
created-by :system}}]
(assert (#{:system :user :admin} created-by)
"expected valid keyword for created-by")
(let [conn
(db/get-connection cfg)
;; NOTE: final user never can provide label as `:system`
;; keyword because the validator implies label always as
;; string; keyword is used for signal a special case
created-by
(if (= label :system)
"system"
"user")
(name created-by)
deleted-at
(if (= label :system)
(cond
(= deleted-at :default)
(dt/plus (dt/now) (cf/get-deletion-delay))
(dt/instant? deleted-at)
deleted-at
:else
nil)
label
(if (= label :system)
(str "internal/snapshot/" (:revn file))
(or label (generate-snapshot-label)))
(or label (generate-snapshot-label))
snapshot-id
(uuid/next)]
(uuid/next)
(-> cfg
(assoc ::quotes/profile-id profile-id)
(assoc ::quotes/project-id (:project-id file))
(assoc ::quotes/team-id (:team-id file))
(assoc ::quotes/file-id (:id file))
(quotes/check! {::quotes/id ::quotes/snapshots-per-file}
{::quotes/id ::quotes/snapshots-per-team}))
data
(blob/encode (:data file))
features
(db/encode-pgarray (:features file) conn "text")]
(l/debug :hint "creating file snapshot"
:file-id (str file-id)
:file-id (str (:id file))
:id (str snapshot-id)
:label label)
(db/insert! cfg :file-change
{:id snapshot-id
:revn (:revn file)
:data (:data file)
:data data
:version (:version file)
:features (:features file)
:features features
:profile-id profile-id
:file-id (:id file)
:label label
@ -146,12 +130,25 @@
(sv/defmethod ::create-file-snapshot
{::doc/added "1.20"
::sm/params schema:create-file-snapshot}
[cfg {:keys [::rpc/profile-id file-id label]}]
(db/tx-run! cfg
(fn [{:keys [::db/conn] :as cfg}]
(files/check-edition-permissions! conn profile-id file-id)
(create-file-snapshot! cfg profile-id file-id label))))
::sm/params schema:create-file-snapshot
::db/transaction true}
[{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id file-id label]}]
(files/check-edition-permissions! conn profile-id file-id)
(let [file (bfc/get-file cfg file-id)
project (db/get-by-id cfg :project (:project-id file))]
(-> cfg
(assoc ::quotes/profile-id profile-id)
(assoc ::quotes/project-id (:project-id file))
(assoc ::quotes/team-id (:team-id project))
(assoc ::quotes/file-id (:id file))
(quotes/check! {::quotes/id ::quotes/snapshots-per-file}
{::quotes/id ::quotes/snapshots-per-team}))
(create-file-snapshot! cfg file
{:label label
:profile-id profile-id
:created-by :user})))
(defn restore-file-snapshot!
[{:keys [::db/conn ::mbus/msgbus] :as cfg} file-id snapshot-id]
@ -237,8 +234,11 @@
(db/tx-run! cfg
(fn [{:keys [::db/conn] :as cfg}]
(files/check-edition-permissions! conn profile-id file-id)
(create-file-snapshot! cfg profile-id file-id :system)
(restore-file-snapshot! cfg file-id id))))
(let [file (bfc/get-file cfg file-id)]
(create-file-snapshot! cfg file
{:profile-id profile-id
:created-by :system})
(restore-file-snapshot! cfg file-id id)))))
(def ^:private schema:update-file-snapshot
[:map {:title "update-file-snapshot"}

15
backend/src/app/srepl/helpers.clj
View file

@ -15,7 +15,8 @@
[app.features.components-v2 :as feat.comp-v2]
[app.main :as main]
[app.rpc.commands.files :as files]
[app.rpc.commands.files-snapshot :as fsnap]))
[app.rpc.commands.files-snapshot :as fsnap]
[app.util.time :as dt]))
(def ^:dynamic *system* nil)
@ -96,8 +97,11 @@
(let [conn (db/get-connection system)]
(->> (feat.comp-v2/get-and-lock-team-files conn team-id)
(reduce (fn [result file-id]
(fsnap/create-file-snapshot! system nil file-id label)
(inc result))
(let [file (fsnap/get-file-snapshots system file-id)]
(fsnap/create-file-snapshot! system file
{:label label
:created-by :admin})
(inc result)))
0))))
(defn restore-team-snapshot!
@ -143,7 +147,10 @@
(cfv/validate-file-schema! file'))
(when (string? label)
(fsnap/create-file-snapshot! system nil file-id label))
(fsnap/create-file-snapshot! system file
{:label label
:deleted-at (dt/in-future {:days 30})
:created-by :admin}))
(let [file' (update file' :revn inc)]
(bfc/update-file! system file')

1
backend/test/backend_tests/rpc_file_snapshot_test.clj
View file

@ -97,6 +97,7 @@
(th/db-query :file-change
{:file-id (:id file)}
{:order-by [:created-at]})]
(t/is (= 2 (count rows)))
(t/is (= "user" (:created-by row1)))
(t/is (= "system" (:created-by row2)))))

9
common/src/app/common/files/helpers.cljc
View file

@ -570,10 +570,9 @@
(into xform:collect-media-refs (vals (:components data)))
(into (keys (:media data)))))
(defn relink-media-refs
"A function responsible to analyze all file data and replace the
old :component-file reference with the new ones, using the provided
file-index."
(defn relink-refs
"A function responsible to analyze the file data or shape for references
and apply lookup-index on it."
[data lookup-index]
(letfn [(process-map-form [form]
(cond-> form
@ -724,7 +723,7 @@
(defn split-by-last-period
"Splits a string into two parts:
the text before and including the last period,
the text before and including the last period,
and the text after the last period."
[s]
(if-let [last-period (str/last-index-of s ".")]