💥 Make legacy-db file data read-only

legacy-db is no longer a write backend: fdata/upsert! rejects it,
handle-persistence stops writing file.data and file_change.data, and
default-backend fails with :legacy-backend-read-only. Reading old
payloads keeps working, and the file-data-backend default stays db.

The configuration guide, the storage memories, the migration comments
and the partition runbook say so, with the upgrade note for deployments
that still set PENPOT_FILE_DATA_BACKEND=legacy-db.

Closes #12416

AI-assisted-by: deepseek-v4.1-flash

.serena/memories/backend/storage.md

@@ -108,8 +108,8 @@ Since `put-object!` uses backend-specific operations (`impl/resolve-backend` + `
 
 ## File Data
 
-- `file-data-backend` accepts `legacy-db`, `db`, or `storage`.
-- `legacy-db` stores main data in `file.data` and snapshots in `file_change.data`.
+- `file-data-backend` accepts `legacy-db`, `db`, or `storage`; `legacy-db` is read-only and the default is `db`. The configuration reads `legacy-db` as `db` at startup, with a warning (`normalize-file-data-backend`), so a deployment that kept the variable saves instead of failing on every write.
+- `legacy-db` reads main data from `file.data` and snapshots from `file_change.data`; it no longer writes them (`fdata/upsert!` rejects it).
 - `db` stores encoded data in `file_data.data`.
 - `storage` stores encoded data in storage subsystem with `file-data` bucket and keeps `data` nil in `file_data` table.
 - The `file_data.metadata.storage-ref-id` value points to the storage object.
@@ -121,13 +121,13 @@ Since `put-object!` uses backend-specific operations (`impl/resolve-backend` + `
 - `file_change` is partitioned by `HASH (file_id)` in 16 partitions, with the same bounds as `file_data`, so a join between them is resolved partition by partition (`enable_partitionwise_join` is set on every pooled connection).
 - The automatic migration (`0158-partition-file-change`) refuses to run when `file_change` is above 10 GiB total size or 2M rows (`pg_total_relation_size` + `pg_class.reltuples`, catalog reads). It analyzes the table first, because `reltuples` is -1 without statistics. It raises with the path of `operations/2.19.0-file-change-partition.md`, because the copy happens inside the migration transaction and a big instance would restart in a loop. Below the guard it raises `statement_timeout` to 30 minutes for the copy and the index builds, and analyzes the new partitions at the end, after the copy and the indexes, so the planner does not start with empty statistics.
 - Its primary key is `(file_id, id)`: a partitioned table needs the partition key in the primary key, so every access filters by `file_id`. From 2.19 the four snapshot commands (`update-file-snapshot`, `delete-file-snapshot`, `lock-file-snapshot`, `unlock-file-snapshot`) require the `file-id` param, because they use it to prune the partition; without it they answer `:params-validation`.
-- It has a `kind` column: `1` is a snapshot (it has a label and its data lives in `file_data`) and `2` is a revision (it has no label and it holds the changes). The values are in `app.features.file-changes`.
+- It has a `kind` column: `1` is a snapshot (it has a label and its data lives in `file_data`; the read-only `legacy-db` backend keeps old payloads in `data`) and `2` is a revision (it has no label and it holds the changes). The values are in `app.features.file-changes`.
 - `kind` is a deliberate reserve, not a leftover: today no query filters by it, the queries still tell a snapshot from a revision by its label (`label IS NOT NULL`), and `kind` takes over when they are rewritten. No constraint ties `kind` and `label` together on purpose: the day `kind` is the source of truth, that constraint would have to go.
-- Every write of the application to a row of `file_change` moves `updated_at` to the moment of the write (`lock-by!`, `unlock!`, the snapshot rename and delete, `delete-object`, the restore of a file, and the `legacy-db` snapshot write of `fdata`). The actions of the foreign keys (`ON DELETE SET NULL` on `locked_by` and `profile_id`) change the row without touching it; during the partition both tables carry the same keys, so both receive them. The runbook of the partition uses it as the watermark of its delta passes: a write of the application that leaves it behind is lost at the cutover.
+- Every write of the application to a row of `file_change` moves `updated_at` to the moment of the write (`lock-by!`, `unlock!`, the snapshot rename and delete, `delete-object`, the restore of a file). The actions of the foreign keys (`ON DELETE SET NULL` on `locked_by` and `profile_id`) change the row without touching it; during the partition both tables carry the same keys, so both receive them. The runbook of the partition uses it as the watermark of its delta passes: a write of the application that leaves it behind is lost at the cutover.
 - `file_change_legacy` is the way back of the partition: it keeps the rows and the indexes of the old table. A new instance (empty table) does not keep it. On an upgraded instance the table stays until the operator drops it by hand once the rollback window is over (`DROP TABLE file_change_legacy;`, days later, see `operations/2.19.0-file-change-partition.md`); nothing drops it automatically.
 - A revision carries only the delta of a save (the changes). The features of the file are not repeated on it: nobody read them from there. A repaired save advances the `revn` once: the changes of the request go first and the repair changes after them, in one revision, and the internal `internal/repair/<revn>` snapshot keeps the file as it was before any change or repair (not the invalid `U(O)` state the repair had to fix). That order is the one a client that saved applies on its own (it already has its changes locally), so it ends in the same state as the server. The notifications follow it too: the save message, under the session of the user, and then the repair message, under the repair session and with the user session as `:origin-session-id` (the channel hides it from the saver, which applies the repair from the `update-file` response, where it also travels). Both messages are published after the transaction commits. The convergence of the repair assumes that response arrives: a retry of a save whose response was lost can compute a second repair over the already-repaired file, and closing that hole needs idempotency by `:commit-id`.
 - A snapshot carries the features and the migrations of the file.
-- It keeps the `data` column: the `legacy-db` backend is still supported (migration `0157-migrate-legacy-file-data` moved the existing blobs, but the backend keeps reading and writing that column). It has no `data_ref_id` or `data_backend` columns: nothing read them since the file storage refactor.
+- It keeps the `data` column for reading only: migration `0157-migrate-legacy-file-data` moved the existing blobs to `file_data`, and `legacy-db` remains as a read-only backend for the payloads still in `data`. `fdata/upsert!` rejects `legacy-db` and the `file-data-backend` default is `db`. It has no `data_ref_id` or `data_backend` columns: nothing read them since the file storage refactor.
 - It has no index for the system snapshots. The old one filtered them by `data IS NOT NULL`, and with the payload living in `file_data` the column no longer tells snapshots apart. The new one could only filter them by `deleted_at IS NULL`, but the system snapshots always carry a deletion date in the future, so it would never match a row. The query that would use it filters that date with an "or", which no partial index can serve.
 - The `objects-gc` purge groups each chunk by `file_id` and locks it with `file_id = ?` and `id = ANY(?::uuid[])`: a tuple `IN ((file_id, id), ...)` gives the planner no constant partition key, so it scans all 16 partitions in every chunk. Each group is one `FOR UPDATE SKIP LOCKED` query inside the chunk transaction.
 

.serena/memories/backend/subtleties.md

@@ -78,13 +78,13 @@
 
 ## File data persistence
 
-- File data backends are `legacy-db`, `db`, and `storage`. The storage backend keeps encoded file data in storage bucket `file-data`; the DB row stores metadata with `storage-ref-id` and nil data.
+- File data backends for writing are `db` and `storage`; `legacy-db` is a read-only alias for old payloads. The storage backend keeps encoded file data in storage bucket `file-data`; the DB row stores metadata with `storage-ref-id` and nil data.
 - `fdata/upsert!` touches any storage object referenced by incoming metadata before storing the new row/blob.
 - Pointer-map fragments are persisted separately as type `fragment`, and only modified pointer maps are written.
 - `fdata/realize` combines pointer realization and object-map realization. Use it before operations that need complete in-memory file data instead of pointer placeholders.
 - A `file_data` row is one of three classes, told apart by `type`: `main` is the file's current state and carries `id = file_id`; `snapshot` is a point-in-time copy whose `id` is the one its `file_change` row shares; `fragment` is a piece of the pointer-map, referenced from the file's data.
 - Their lifecycles differ: `main` and `snapshot` live with the file (dated when it is deleted, undated when it is restored), while a `fragment` carries no date and is collected by `file_gc` once the file stops referencing it. Neither is a session buffer, so do not read a `fragment` as an ephemeral revision.
-- File snapshots (versions) keep their metadata row in `file_change` and their payload in `file_data` (`type = 'snapshot'`). With the `db` backend `file_change.data` stays NULL: that column is only written by the `legacy-db` backend, plus the rows written before the 2025-07 file storage refactor.
+- File snapshots (versions) keep their metadata row in `file_change` and their payload in `file_data` (`type = 'snapshot'`). With the `db` backend `file_change.data` stays NULL: that column is only read by the `legacy-db` backend (it is no longer written), plus the rows written before the 2025-07 file storage refactor.
 - Tell a snapshot apart by `label IS NOT NULL` (with `created_by` / `deleted_at` as needed), never by `file_change.data IS NOT NULL`: the label holds with every file data backend, and the payload column stopped meaning "this is a snapshot" when the payload moved out. Queries that identify versions must use the label. `0157-migrate-legacy-file-data` moved the labelled leftovers to `file_data` and deleted the unlabelled payload rows, so from 2.19 on the label is the only marker there is.
 - The version quota counts of `app.rpc.quotes` (`sql:get-snapshots-per-file`, `sql:get-snapshots-per-team`) are served by the partial index `file_change__user_snapshots__idx`, whose predicate mirrors their where clause; changing one without the other drops the index only scan and the count walks the whole table again.
 - Revisions live only in `file_change` and have no `file_data` counterpart. They are born already scheduled for deletion one hour later (`app.rpc.commands.files-update`) and the snapshot paths cannot reach one, because they resolve their row through a `label IS NOT NULL` filter.

backend/src/app/config.clj

@@ -10,6 +10,7 @@
    [app.common.data :as d]
    [app.common.exceptions :as ex]
    [app.common.flags :as flags]
+   [app.common.logging :as l]
    [app.common.schema :as sm]
    [app.common.time :as ct]
    [app.common.uri :as u]
@@ -362,13 +363,29 @@
 (def explain-config
   (sm/explainer schema:config))
 
+(defn- normalize-file-data-backend
+  "Since the file data moved to `file_change`, the `legacy-db` back end is
+  read-only: a write against it fails on every save. Accepting it would
+  start a healthy instance that cannot save a single file, so it reads as
+  `db`, which serves the legacy rows too. Warn instead of aborting: the
+  operator may still be halfway through the migration."
+  [config]
+  (if (= "legacy-db" (c/get config :file-data-backend))
+    (do
+      (l/warn :hint "file-data-backend=legacy-db is read-only, using db"
+              :file-data-backend "legacy-db"
+              :replacement "db")
+      (assoc config :file-data-backend "db"))
+    config))
+
 (defn read-config
   "Reads the configuration from environment variables and decodes all
   known values."
   [& {:keys [prefix default] :or {prefix "penpot"}}]
   (->> (read-env prefix)
        (merge default)
-       (decode-config)))
+       (decode-config)
+       (normalize-file-data-backend)))
 
 (def version
   (v/parse (or (some-> (io/resource "version.txt")

backend/src/app/features/fdata.clj

@@ -147,7 +147,7 @@
                        data])))
 
 (defn- handle-persistence
-  [cfg {:keys [type backend id file-id data] :as params}]
+  [cfg {:keys [backend id file-id data] :as params}]
 
   (cond
     (= backend "storage")
@@ -179,39 +179,6 @@
           params   (assoc params :metadata metadata)]
       (upsert-in-database cfg params))
 
-    (= backend "legacy-db")
-    (cond
-      (= type "main")
-      (do
-        (db/delete! cfg :file-data
-                    {:id id :file-id file-id :type "main"}
-                    {::db/return-keys false})
-        (db/update! cfg :file
-                    {:data data}
-                    {:id file-id}
-                    {::db/return-keys false}))
-
-      (= type "snapshot")
-      (do
-        (db/delete! cfg :file-data
-                    {:id id :file-id file-id :type "snapshot"}
-                    {::db/return-keys false})
-        ;; every mutation of `file_change` moves `updated_at`: the partition
-        ;; runbook uses it as the watermark of the delta passes
-        (db/update! cfg :file-change
-                    {:data data
-                     :updated-at (ct/now)}
-                    {:file-id file-id :id id}
-                    {::db/return-keys false}))
-
-      (= type "fragment")
-      (upsert-in-database cfg
-                          (-> (dissoc params :metadata)
-                              (assoc :backend "db")))
-
-      :else
-      (throw (RuntimeException. "not implemented")))
-
     :else
     (throw (IllegalArgumentException.
             (str "backend '" backend "' not supported")))))
@@ -224,7 +191,13 @@
 
 (defn- default-backend
   [backend]
-  (or backend (cf/get :file-data-backend)))
+  (let [backend (or backend (cf/get :file-data-backend))]
+    (when (= "legacy-db" backend)
+      (ex/raise :type :validation
+                :code :legacy-backend-read-only
+                :hint "the legacy-db file data backend is read-only; set the file data backend to db or storage"
+                :backend backend))
+    backend))
 
 (defn decode-metadata
   [metadata]
@@ -237,7 +210,7 @@
    [:id ::sm/uuid]
    [:type [:enum "main" "snapshot" "fragment"]]
    [:file-id ::sm/uuid]
-   [:backend {:optional true} [:enum "db" "legacy-db" "storage"]]
+   [:backend {:optional true} [:enum "db" "storage"]]
    [:metadata {:optional true} ctf/schema:file-metadata]
    [:data {:optional true} bytes?]
    [:created-at {:optional true} ::ct/inst]

backend/test/backend_tests/config_test.clj

@@ -2,11 +2,13 @@
 ;; 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 SUBSIDIARY SL
+;; Copyright (c) KALEIDOS INC Sucursal en España SL
 
 (ns backend-tests.config-test
   (:require
+   [app.common.logging :as l]
    [app.config :as cf]
+   [clojure.core :as c]
    [clojure.test :as t]))
 
 (t/deftest get-public-uri-normalizes-base
@@ -38,3 +40,50 @@
   (t/testing "a leading slash would drop the subpath, so it fails fast"
     (t/is (thrown? AssertionError
                    (cf/join-uri "https://example.com/penpot" "/assets/by-id/123")))))
+
+;; `legacy-db` is read-only since the file data moved to `file_change`: an
+;; instance that keeps it starts healthy and then fails on every save. It is
+;; read as `db` instead, which also serves the legacy rows.
+
+(defn- read-config
+  "`read-config` with the environment replaced by `env`."
+  [env & {:keys [default] :or {default {}}}]
+  (with-redefs [cf/read-env (constantly env)]
+    (cf/read-config :default default)))
+
+(defn- read-config-with-warning
+  "Reads the configuration and collects the warnings it emits. Logging is
+  emitted asynchronously, so the record is awaited (bounded)."
+  [env]
+  (let [records (atom [])]
+    (add-watch l/log-record ::capture
+               (fn [_ _ _ record] (swap! records conj record)))
+    (try
+      (let [config (read-config env)]
+        [(loop [remaining 100]
+           (let [warns (filter #(= :warn (::l/level %)) @records)]
+             (cond
+               (seq warns) warns
+               (zero? remaining) warns
+               :else (do (Thread/sleep 10) (recur (dec remaining))))))
+         config])
+      (finally
+        (remove-watch l/log-record ::capture)))))
+
+(t/deftest read-config-replaces-the-read-only-legacy-db
+  (let [[warns config] (read-config-with-warning {:file-data-backend "legacy-db"})]
+    (t/testing "the configuration writes through db"
+      (t/is (= "db" (c/get config :file-data-backend))))
+    (t/testing "the operator is warned"
+      (t/is (contains? (into #{} (map #(c/get (::l/props %) :hint)) warns)
+                       "file-data-backend=legacy-db is read-only, using db")))))
+
+(t/deftest read-config-keeps-the-writable-back-ends
+  (doseq [backend ["db" "storage"]]
+    (t/testing backend
+      (t/is (= backend (c/get (read-config {:file-data-backend backend})
+                              :file-data-backend))))))
+
+(t/deftest read-config-takes-the-default-back-end
+  (t/is (= "db" (c/get (read-config {} :default cf/default)
+                       :file-data-backend))))

backend/test/backend_tests/fdata_upsert_test.clj

@@ -5,51 +5,50 @@
 ;; Copyright (c) KALEIDOS SUBSIDIARY SL
 
 (ns backend-tests.fdata-upsert-test
-  "Covers the invariants of `fdata/upsert!` that the partition runbook relies
-  on."
+  "Covers the write contract of `fdata/upsert!` after the `legacy-db` backend
+  became read-only."
 
   (:require
-   [app.common.time :as ct]
    [app.common.uuid :as uuid]
-   [app.db :as db]
+   [app.config :as cf]
    [app.features.fdata :as fdata]
-   [app.features.file-changes :as fchanges]
    [backend-tests.helpers :as th]
    [clojure.test :as t]))
 
 (t/use-fixtures :once th/state-init)
 (t/use-fixtures :each th/database-reset)
 
-(t/deftest legacy-db-snapshot-moves-the-updated-at-watermark
-  ;; The runbook of the partition picks the rows changed since the bulk copy
-  ;; with `updated_at`, so every mutation of `file_change` has to move it. The
-  ;; `legacy-db` snapshot path writes the payload into the row, and it used to
-  ;; leave the watermark behind.
-  (let [profile  (th/create-profile* 1)
-        file     (th/create-file* 1 {:profile-id (:id profile)
-                                     :project-id (:default-project-id profile)})
-        id       (uuid/next)
-        past     (ct/in-past "2h")
-        payload  (byte-array [1 2 3])]
+(defn- upsert-cause
+  [params]
+  (try
+    (fdata/upsert! th/*system* params)
+    nil
+    (catch Exception cause
+      cause)))
 
-    (th/db-insert! :file-change
-                   {:id         id
-                    :file-id    (:id file)
-                    :revn       1
-                    :label      "v1"
-                    :kind       fchanges/snapshot-kind
-                    :updated-at past})
+(t/deftest legacy-db-param-is-rejected
+  ;; `legacy-db` is not a write backend anymore: the schema refuses it before
+  ;; touching the database.
+  (let [cause (upsert-cause {:id (uuid/next)
+                             :file-id (uuid/next)
+                             :type "snapshot"
+                             :backend "legacy-db"
+                             :data (byte-array [1 2 3])})]
+    (t/is (some? cause))
+    (t/is (= :assertion (:type (ex-data cause))))
+    (t/is (= :data-validation (:code (ex-data cause))))))
 
-    (t/is (true? (fdata/upsert! th/*system*
-                                {:id      id
-                                 :file-id (:id file)
-                                 :type    "snapshot"
-                                 :backend "legacy-db"
-                                 :data    payload})))
+(t/deftest legacy-db-config-is-rejected-as-read-only
+  ;; The configuration value is kept for reading old data, but a write with
+  ;; it as the default backend fails with a clear message.
+  (with-redefs [cf/config (assoc cf/config :file-data-backend "legacy-db")]
+    (let [cause (upsert-cause {:id (uuid/next)
+                               :file-id (uuid/next)
+                               :type "snapshot"
+                               :data (byte-array [1 2 3])})]
+      (t/is (some? cause))
+      (t/is (= :validation (:type (ex-data cause))))
+      (t/is (= :legacy-backend-read-only (:code (ex-data cause)))))))
 
-    (let [row (th/db-get :file-change {:id id} {::db/remove-deleted false})]
-      (t/testing "the payload lands in the legacy column"
-        (t/is (= (seq payload) (seq ^bytes (:data row)))))
-
-      (t/testing "the watermark moves to the moment of the mutation"
-        (t/is (pos? (compare (inst-ms (:updated-at row)) (inst-ms past))))))))
+(t/deftest file-data-backend-defaults-to-db
+  (t/is (= "db" (cf/get :file-data-backend))))

docs/technical-guide/configuration.md

@@ -776,12 +776,13 @@ These settings are equally useful if you have a Minio storage system.
 
 __Since version 2.11.0__
 
-You can change the default file data storage backend with `PENPOT_FILE_DATA_BACKEND` environment variable. Possible values are:
+You can change the file data storage backend with `PENPOT_FILE_DATA_BACKEND` environment variable. Possible values are:
 
-- `legacy-db`: the current default backend, continues storing the file data of files and snapshots in the same location as previous versions of Penpot (< 2.11.0), this is a conservative default behaviour and will be changed to `db` in next versions.
-- `db`: stores the file data on an specific table (the future default backend).
+- `db`: the default. Stores the file data on an specific table.
 - `storage`: stores the file data using the objects storage system (S3 or FS, depending on which one is configured)
 
+The `legacy-db` value is still accepted as a **read-only** alias for the payloads written by versions before 2.11.0: the backend reads them, but it no longer writes file data there. **If you upgrade a deployment with `PENPOT_FILE_DATA_BACKEND=legacy-db`, remove the variable (or set it to `db`) before the upgrade**: the backend reads `legacy-db` as `db` and logs a warning, so saves keep working, but remove the variable anyway so the deployment does not claim a read-only backend.
+
 This also comes with an additional feature that allows offload the "inactive" files on file storage backend and leaves the database only for the active files. To enable it, you should use the `enable-tiered-file-data-storage` flag and `db` as file data storage backend.
 
 ```bash