github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-30 06:20:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

♻️ Simplify storage target helpers and refusal logging

Browse Source 
Drop the bang from the pure storage-routes validators,
inline the refusal log into park-unresolvable in both GC
tasks, and read the storage target from object metadata
only. Tests now use the production object shape.

AI-assisted-by: muse-spark-1.3-contributor
This commit is contained in:
Andrey Antukh 2026-09-11 17:25:03 +00:00
parent e96b8a4798
commit 334322e6db
5 changed files with 37 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
backend/src/app/storage/config.clj
View File

@ -65,7 +65,7 @@
:path (str path)
:cause cause))))
(defn- assert-backend-s3!
(defn- assert-backend-s3
"The routing only makes sense on top of the S3 backend."
[]
(let [backend (or (sto/get-legacy-backend)
@ -77,7 +77,7 @@
:hint "storage routes file requires the :s3 backend"
:backend (keyword backend)))))
(defn- validate!
(defn- validate
[data path]
(when-not (valid-file? data)
(ex/raise :type :validation
@ -135,8 +135,8 @@
configuration key is unset."
[]
(if-let [path (not-empty (cf/get :objects-storage-s3-routes-file))]
(let [data (-> (read-file path) (validate! path))]
(assert-backend-s3!)
(let [data (-> (read-file path) (validate path))]
(assert-backend-s3)
{:targets (normalize-targets (:targets data))
:routes (:routes data)})
{:targets nil

13
backend/src/app/storage/gc_deleted.clj
View File

@ -78,14 +78,6 @@
(let [ids (db/create-array conn "uuid" ids)]
(db/exec-one! conn [sql:delete-give-up ids max-attempts])))
(defn- log-refusal!
"Indirection over the error log so tests can capture the refusal payload."
[backend-id target ids]
(l/err :hint "storage target is not configured, deletion refused"
:backend (name backend-id)
:target target
:ids (mapv str ids)))
(def ^:private sql:defer-unresolvable
"UPDATE storage_object
SET deleted_at = NOW() + INTERVAL '1 day'
@ -97,7 +89,10 @@
selection window without being deleted and without counting a deletion
attempt (they are therefore never subject to the give-up window)."
[conn backend-id target ids]
(log-refusal! backend-id target ids)
(l/err :hint "storage target is not configured, deletion refused"
:backend (name backend-id)
:target target
:ids (mapv str ids))
(let [ids (db/create-array conn "uuid" ids)]
(db/exec-one! conn [sql:defer-unresolvable ids])))

13
backend/src/app/storage/pending_gc.clj
View File

@ -38,14 +38,6 @@
(def ^:private sql:delete-pending-sobject
"DELETE FROM storage_object WHERE id = ? AND status = 'pending'")
(defn- log-refusal!
"Indirection over the error log so tests can capture the refusal payload."
[backend-id target ids]
(l/err :hint "storage target is not configured, deletion refused"
:backend (name backend-id)
:target target
:ids (mapv str ids)))
(def ^:private sql:park-unresolvable
"UPDATE storage_object
SET deleted_at = NOW() + INTERVAL '1 day'
@ -57,7 +49,10 @@
misconfiguration and pushes `deleted_at` forward so the rows are excluded
from the next selection without being deleted."
[conn backend-id target ids]
(log-refusal! backend-id target ids)
(l/err :hint "storage target is not configured, deletion refused"
:backend (name backend-id)
:target target
:ids (mapv str ids))
(let [ids (db/create-array conn "uuid" ids)]
(db/exec-one! conn [sql:park-unresolvable ids])))

3
backend/src/app/storage/s3.clj
View File

@ -209,8 +209,7 @@
"Returns the configured target id stored on the object, or the default
target name when the object predates the routing feature."
[backend object]
(or (:storage-target object)
(some-> (meta object) :storage-target)
(or (some-> (meta object) :storage-target)
(name (::default-target backend))))
(defn- resolve-target-by-id

60
backend/test/backend_tests/storage_test.clj
View File

@ -897,8 +897,8 @@
(t/deftest s3-get-object-url-rejects-unknown-target
(let [backend (fake-s3-backend-with-targets)
ex (try
(impl/get-object-url backend {:id (uuid/next)
:storage-target "ghost"} {})
(impl/get-object-url backend (with-meta {:id (uuid/next)}
{:storage-target "ghost"}) {})
nil
(catch Throwable cause cause))]
(t/is (some? ex))
@ -907,9 +907,8 @@
(t/deftest s3-get-object-data-rejects-unknown-target
(let [backend (fake-s3-backend-with-targets)
ex (try
(impl/get-object-data backend {:id (uuid/next)
:size 1
:storage-target "ghost"})
(impl/get-object-data backend (with-meta {:id (uuid/next) :size 1}
{:storage-target "ghost"}))
nil
(catch Throwable cause cause))]
(t/is (some? ex))
@ -1183,8 +1182,7 @@
(t/deftest gc-deleted-refuses-unknown-target-and-keeps-row
(let [storage (storage-with-s3-targets)
cfg {::db/pool th/*pool* ::sto/storage storage}
id (uuid/next)
logged (atom nil)]
id (uuid/next)]
(th/db-exec! ["insert into storage_object (id, size, backend, metadata, deleted_at, status)
values (?, 1, 's3', ?, ?, 'valid')"
id
@ -1192,20 +1190,16 @@
(ct/in-past {:minutes 1})])
(with-mocks [mock {:target 'app.storage.impl/del-objects-in-bulk
:return (fn [_ _ _] #{})}]
(with-redefs [app.storage.gc-deleted/log-refusal!
(fn [backend-id target ids]
(reset! logged [backend-id target (vec ids)]))]
(let [result (#'sto.gc-deleted/clean-deleted! cfg)
row (th/db-exec-one!
["select status, deleted_at, deletion_attempts
(let [result (#'sto.gc-deleted/clean-deleted! cfg)
row (th/db-exec-one!
["select status, deleted_at, deletion_attempts
from storage_object where id = ?" id])]
(t/is (= 0 (:deleted result)))
(t/is (= 1 (:parked result)))
(t/is (= 0 (:call-count @mock)))
(t/is (= "valid" (:status row)))
(t/is (ct/is-after? (:deleted-at row) (ct/now)))
(t/is (= 0 (:deletion-attempts row)))
(t/is (= [:s3 "ghost" [id]] @logged)))))))
(t/is (= 0 (:deleted result)))
(t/is (= 1 (:parked result)))
(t/is (= 0 (:call-count @mock)))
(t/is (= "valid" (:status row)))
(t/is (ct/is-after? (:deleted-at row) (ct/now)))
(t/is (= 0 (:deletion-attempts row)))))))
(t/deftest gc-deleted-give-up-not-applied-to-unknown-target
(let [storage (storage-with-s3-targets)
@ -1218,8 +1212,7 @@
(ct/in-past {:minutes 1})])
(with-mocks [_mock {:target 'app.storage.impl/del-objects-in-bulk
:return (fn [_ _ _] #{})}]
(with-redefs [app.storage.gc-deleted/log-refusal! (fn [& _] nil)]
(#'sto.gc-deleted/clean-deleted! cfg)))
(#'sto.gc-deleted/clean-deleted! cfg))
(let [row (th/db-exec-one! ["select count(*) from storage_object where id = ?" id])]
(t/is (= 1 (:count row))))))
@ -1278,7 +1271,6 @@
(let [storage (storage-with-s3-targets)
cfg {::db/pool th/*pool* ::sto/storage storage}
id (uuid/next)
logged (atom nil)
captured (atom nil)]
(th/db-exec! ["insert into storage_object (id, size, backend, metadata, created_at, status)
values (?, 1, 's3', ?, ?, 'pending')"
@ -1289,19 +1281,15 @@
:return (fn [_ object]
(reset! captured object)
nil)}]
(with-redefs [app.storage.pending-gc/log-refusal!
(fn [backend-id target ids]
(reset! logged [backend-id target (vec ids)]))]
(let [result (#'sto.pending-gc/process! cfg)
row (th/db-exec-one!
["select status, deleted_at from storage_object where id = ?" id])]
(t/is (= 0 (:processed result)))
(t/is (= 1 (:parked result)))
(t/is (= 0 (:call-count @mock)))
(t/is (nil? @captured))
(t/is (= "pending" (:status row)))
(t/is (ct/is-after? (:deleted-at row) (ct/now)))
(t/is (= [:s3 "ghost" [id]] @logged)))))))
(let [result (#'sto.pending-gc/process! cfg)
row (th/db-exec-one!
["select status, deleted_at from storage_object where id = ?" id])]
(t/is (= 0 (:processed result)))
(t/is (= 1 (:parked result)))
(t/is (= 0 (:call-count @mock)))
(t/is (nil? @captured))
(t/is (= "pending" (:status row)))
(t/is (ct/is-after? (:deleted-at row) (ct/now)))))))
(t/deftest gc-deleted-legacy-rows-delete-from-default-target
(let [storage (storage-with-s3-targets)