github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 05:50:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

🔥 Drop support for the branching preview's older databases

Browse Source 
Branches and databases from the preview's earlier shapes are not
supported, and such branches are recreated. The earlier shapes are the
copy model, where a branch file kept its own data payload, and the
tables registered as 0150-add-file-branch-table,
0152-add-file-branch-table and 0153-add-file-pull-request-tables. This
commit removes the code that existed only for them. A fresh database
ends with the same schema as before.

- 0155-add-file-branch-table.sql loses its header about the old
  registrations, every IF NOT EXISTS, and the base_branch_revn upgrade
  and backfill. CREATE TABLE file_branch already declares the column.
- 0156 and 0157 lose every IF NOT EXISTS.
- 0158-add-file-branch-change-data-version folds into 0157, and its
  file and registration go. CREATE TABLE file_branch_change declares
  data_version NOT NULL, because both writers of the op log, a branch
  save and the squash of an update from main, stamp every row.
- need-migration-to? loses its nil case, which replayed an unstamped
  op-log row at the base's version, and the branch derive loses the
  comment that described it. Stamped rows replay as before.
- The tests for a re-run of the branch table migration and for the
  replay of an unstamped row go, with the require the first one used.

A database that holds the preview's older shapes must drop its
branching tables and the file.is_branch column, and delete the
branching steps from its migrations table, before it boots this code.
A database that ran 0157 without 0158 lacks data_version and counts as
an older shape. A database that already ran 0155 to 0158 under these
names keeps working, because a migration runs once per name.

AI-assisted-by: claude-opus-5-5
This commit is contained in:
Álvaro Tejero Cantero 2026-10-04 13:57:38 +02:00
parent 6605968633
commit 1410d66ca2
No known key found for this signature in database
10 changed files with 33 additions and 174 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
backend/src/app/binfile/common.clj
View File

@ -430,11 +430,7 @@
;; A row's `data_version` is the version its changes were
;; written at; the document migrates to it before the row
;; applies, and in the steady state that check is one set
;; lookup. A NULL stamp marks a row written before the column
;; existed: it replays at the version the document holds, which
;; is the base's version, because such rows precede every
;; stamped row of their log. That is how the derive replayed
;; every row before the column existed.
;; lookup.
[doc depth]
(reduce (fn [[doc depth] {:keys [changes data-version]}]
(let [changes (blob/decode changes)

5
backend/src/app/migrations.clj
View File

@ -526,10 +526,7 @@
:fn (mg/resource "app/migrations/sql/0156-add-file-pull-request-tables.sql")}
{:name "0157-add-file-branch-change-table"
:fn (mg/resource "app/migrations/sql/0157-add-file-branch-change-table.sql")}
{:name "0158-add-file-branch-change-data-version"
:fn (mg/resource "app/migrations/sql/0158-add-file-branch-change-data-version.sql")}])
:fn (mg/resource "app/migrations/sql/0157-add-file-branch-change-table.sql")}])
(defn apply-migrations!
[pool name migrations]

37
backend/src/app/migrations/sql/0155-add-file-branch-table.sql
View File

@ -1,13 +1,7 @@
-- Idempotent on purpose: this migration was originally registered as
-- "0150-add-file-branch-table" / "0152-add-file-branch-table" during
-- development, so instances that already applied it under an old name
-- re-apply this one safely (and pick up the later-added
-- base_branch_revn column).
ALTER TABLE file
ADD COLUMN IF NOT EXISTS is_branch boolean NOT NULL DEFAULT false;
ADD COLUMN is_branch boolean NOT NULL DEFAULT false;
CREATE TABLE IF NOT EXISTS file_branch (
CREATE TABLE file_branch (
id uuid PRIMARY KEY,
created_at timestamptz NOT NULL DEFAULT clock_timestamp(),
@ -40,32 +34,11 @@ CREATE TABLE IF NOT EXISTS file_branch (
CHECK (branch_file_id != source_file_id)
);
-- upgrade path from the 0150 development shape
ALTER TABLE file_branch
ADD COLUMN IF NOT EXISTS base_branch_revn bigint NOT NULL DEFAULT 0;
-- backfill: at creation both counters coincide, so base_revn is the best
-- available approximation for rows created before the column existed.
-- The rows this is for kept the copy model, where the branch file
-- persisted its own data payload; an op-log branch stores no payload and
-- legitimately starts its own counter at 0, so the payload's existence
-- is what tells the two apart, and the statement changes nothing on a
-- re-run (a renamed re-registration re-applies the whole file).
UPDATE file_branch fb
SET base_branch_revn = fb.base_revn
FROM file f
WHERE fb.branch_file_id = f.id
AND fb.base_branch_revn = 0
AND EXISTS (SELECT 1
FROM file_data fd
WHERE fd.file_id = f.id
AND fd.deleted_at IS NULL);
CREATE INDEX IF NOT EXISTS file_branch__source_file_id__idx
CREATE INDEX file_branch__source_file_id__idx
ON file_branch(source_file_id) WHERE deleted_at IS NULL;
CREATE UNIQUE INDEX IF NOT EXISTS file_branch__branch_file_id__idx
CREATE UNIQUE INDEX file_branch__branch_file_id__idx
ON file_branch(branch_file_id);
CREATE INDEX IF NOT EXISTS file_branch__base_snapshot_id__idx
CREATE INDEX file_branch__base_snapshot_id__idx
ON file_branch(base_snapshot_id) WHERE base_snapshot_id IS NOT NULL;

12
backend/src/app/migrations/sql/0156-add-file-pull-request-tables.sql
View File

@ -1,4 +1,4 @@
CREATE TABLE IF NOT EXISTS file_pull_request (
CREATE TABLE file_pull_request (
id uuid PRIMARY KEY,
created_at timestamptz NOT NULL DEFAULT clock_timestamp(),
@ -33,17 +33,17 @@ CREATE TABLE IF NOT EXISTS file_pull_request (
);
-- at most one live pull request per branch
CREATE UNIQUE INDEX IF NOT EXISTS file_pull_request__file_branch_id__open__idx
CREATE UNIQUE INDEX file_pull_request__file_branch_id__open__idx
ON file_pull_request(file_branch_id)
WHERE status = 'open' AND deleted_at IS NULL;
CREATE INDEX IF NOT EXISTS file_pull_request__target_file_id__idx
CREATE INDEX file_pull_request__target_file_id__idx
ON file_pull_request(target_file_id) WHERE deleted_at IS NULL;
CREATE INDEX IF NOT EXISTS file_pull_request__review_snapshot_id__idx
CREATE INDEX file_pull_request__review_snapshot_id__idx
ON file_pull_request(review_snapshot_id) WHERE review_snapshot_id IS NOT NULL;
CREATE TABLE IF NOT EXISTS file_pull_request_review (
CREATE TABLE file_pull_request_review (
pull_request_id uuid NOT NULL REFERENCES file_pull_request(id) ON DELETE CASCADE DEFERRABLE,
profile_id uuid NOT NULL REFERENCES profile(id) ON DELETE CASCADE DEFERRABLE,
@ -61,5 +61,5 @@ CREATE TABLE IF NOT EXISTS file_pull_request_review (
PRIMARY KEY (pull_request_id, profile_id)
);
CREATE INDEX IF NOT EXISTS file_pull_request_review__profile_id__idx
CREATE INDEX file_pull_request_review__profile_id__idx
ON file_pull_request_review(profile_id);

18
backend/src/app/migrations/sql/0157-add-file-branch-change-table.sql
View File

@ -14,8 +14,19 @@
-- dropped (materialize-file-branch), or until the objects GC hard-deletes
-- the branch file row and the foreign keys cascade; a logical branch
-- deletion leaves them in place.
--
-- `data_version` is the data version each row was written at: the name
-- of the last file data migration
-- (`app.common.files.migrations/available-migrations`) of the backend
-- that applied the row's changes. A data version is a prefix of that
-- list, so its last name identifies it. The branch derive
-- (`app.binfile.common/branch-file-data`) replays the log in segments:
-- it migrates the document to a row's version before it applies the
-- row, the point where an ordinary file migrates across an upgrade. The
-- merge base needs no column of its own: its snapshot row
-- (`file_change.migrations`) records the migrations its data holds.
CREATE TABLE IF NOT EXISTS file_branch_change (
CREATE TABLE file_branch_change (
id uuid PRIMARY KEY,
branch_id uuid NOT NULL REFERENCES file_branch(id) ON DELETE CASCADE DEFERRABLE,
@ -26,6 +37,9 @@ CREATE TABLE IF NOT EXISTS file_branch_change (
changes bytea NOT NULL,
-- the data version the changes were written at
data_version text NOT NULL,
created_at timestamptz NOT NULL DEFAULT clock_timestamp(),
updated_at timestamptz NOT NULL DEFAULT clock_timestamp(),
deleted_at timestamptz NULL,
@ -33,5 +47,5 @@ CREATE TABLE IF NOT EXISTS file_branch_change (
UNIQUE (file_id, revn)
);
CREATE INDEX IF NOT EXISTS file_branch_change__branch_id__idx
CREATE INDEX file_branch_change__branch_id__idx
ON file_branch_change(branch_id) WHERE deleted_at IS NULL;

18
backend/src/app/migrations/sql/0158-add-file-branch-change-data-version.sql
View File

@ -1,18 +0,0 @@
-- The data version each op-log row was written at: the name of the last
-- file data migration (`app.common.files.migrations/available-migrations`)
-- of the backend that applied the row's changes. A data version is a
-- prefix of that list, so its last name identifies it. The branch derive
-- (`app.binfile.common/branch-file-data`) replays the log in segments: it
-- migrates the document to a row's version before it applies the row, the
-- point where an ordinary file migrates across an upgrade.
--
-- The merge base needs no column of its own: its snapshot row
-- (`file_change.migrations`) records the migrations its data holds.
--
-- NULL marks a row written before this column existed. The derive replays
-- such a row at the version the document holds, which is the base's
-- version because those rows precede every stamped row of their log. That
-- is how the derive replayed every row before this column existed.
ALTER TABLE file_branch_change
ADD COLUMN IF NOT EXISTS data_version text NULL;

38
backend/test/backend_tests/rpc_file_branch_oplog_test.clj
View File

@ -967,44 +967,6 @@
(t/is (= {:name "Two" :path "Brand / Two"}
(select-keys (get colors c2) [:name :path]))))))))))
(t/deftest branch-replays-an-unstamped-row-at-the-base-version
;; A row written before `file_branch_change.data_version` existed has
;; no stamp. Such rows precede every stamped row and were written over
;; the base, so the derive replays them at the base's version, as it did
;; before the column existed.
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [profile (th/create-profile* 1 {:is-active true})
proj-id (:default-project-id profile)
main (th/create-file* 1 {:profile-id (:id profile)
:project-id proj-id
:is-shared false})
ordinary (th/create-file* 2 {:profile-id (:id profile)
:project-id proj-id
:is-shared false})
upgraded (conj fmg/available-migrations test-migration)
c1 (uuid/random)
c2 (uuid/random)
old-op {:type :add-color
:color {:id c1 :name "One" :path "Brand/One" :color "#111111" :opacity 1}}
new-op {:type :add-color
:color {:id c2 :name "Two" :path "Brand / Two" :color "#222222" :opacity 1}}
branch-file-id (:branch-file-id (create-branch* profile (:id main) "unstamped"))]
(apply-change* profile branch-file-id old-op)
(apply-change* profile (:id ordinary) old-op)
(th/db-update! :file-branch-change {:data-version nil} {:file-id branch-file-id})
(with-redefs [fmg/available-migrations upgraded]
(apply-change* profile branch-file-id new-op)
(apply-change* profile (:id ordinary) new-op)
(t/is (= [nil test-migration]
(->> (oplog-rows branch-file-id)
(sort-by :revn)
(mapv :data-version))))
(t/is (= (colors-of (read-file profile (:id ordinary)))
(colors-of (read-file profile branch-file-id))))))))
(t/deftest update-from-main-conflict-sides-name-the-documents
;; `update-branch-from-main` reports its conflicts in document terms:
;; `:main` carries main's value and `:branch` the branch's, whatever the

62
backend/test/backend_tests/rpc_file_branch_test.clj
View File

@ -23,7 +23,6 @@
[app.rpc.commands.files-branch-policies :as fbp]
[app.storage :as sto]
[app.util.blob :as blob]
[app.util.migrations :as mg]
[backend-tests.helpers :as th]
[clojure.string :as str]
[clojure.test :as t]
@ -2758,67 +2757,6 @@
(compare!)
(t/is (= :ignore (get @seen :same-parent-reorder)))))))
(t/deftest re-running-the-branch-migration-leaves-oplog-branches-alone
;; The branch table migration carries a backfill for the copy model it
;; predates: rows whose branch file persisted its own data payload.
;; An op-log branch stores no payload and legitimately starts its own
;; counter at 0, so a re-run of the file (a renamed re-registration
;; re-applies every statement) must not touch it, while it still
;; repositions a copy-model row.
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [profile (th/create-profile* 1 {:is-active true})
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared false})
;; main must have work for the backfill to have anything to
;; reposition to: a fresh file's revn is 0, and the backfill
;; copies base_revn, main's revision at branch creation
_ (th/command! {::th/type :update-file
::rpc/profile-id (:id profile)
:id (:id file)
:session-id (uuid/random)
:revn (:revn (th/db-get :file {:id (:id file)}))
:vern (:vern (th/db-get :file {:id (:id file)}))
:features cfeat/supported-features
:changes [{:type :add-color
:color {:id (uuid/random)
:name "counter"
:color "#ff0000"
:opacity 1}}]})
oplog-branch (-> (th/command! {::th/type :create-file-branch
::rpc/profile-id (:id profile)
:file-id (:id file)
:name "oplog-branch"})
:result)
copy-branch (-> (th/command! {::th/type :create-file-branch
::rpc/profile-id (:id profile)
:file-id (:id file)
:name "copy-branch"})
:result)
migrate-again (fn []
((mg/resource "app/migrations/sql/0155-add-file-branch-table.sql")
th/*pool*))
counter (fn [branch-id]
(:base-branch-revn (th/db-get :file-branch {:id branch-id})))]
;; both branches start with their own counter at 0
(t/is (zero? (counter (:id oplog-branch))))
(t/is (zero? (counter (:id copy-branch))))
;; the copy-model row: its branch file carries a data payload, the
;; shape the backfill was written for
(th/db-insert! :file-data
{:file-id (:branch-file-id copy-branch)
:id (:branch-file-id copy-branch)
:type "main"
:data (blob/encode {:pages []})})
(migrate-again)
;; the op-log branch keeps its own counter; the copy-model row is
;; repositioned onto main's revision
(t/is (zero? (counter (:id oplog-branch))))
(t/is (pos? (counter (:id copy-branch)))))))
(t/deftest summary-cache-key-changes-with-policies
;; Two different policies must never share a cached summary: the key
;; hashes the policies the value was computed under.

10
common/src/app/common/files/migrations.cljc
View File

@ -109,14 +109,12 @@
(d/ordered-set))))
(defn need-migration-to?
"True when `file` has not reached data version `version`. A nil
`version` names no version and needs nothing. A file that already
holds the version's last migration has reached it, which costs one
set lookup."
"True when `file` has not reached data version `version`. A file that
already holds the version's last migration has reached it, which
costs one set lookup."
[file version]
(let [migrations (:migrations file)]
(and (some? version)
(not (contains? migrations version))
(and (not (contains? migrations version))
(boolean
(->> migrations
(set/difference (data-version-migrations version))

1
common/test/common_tests/files_migrations_test.cljc
View File

@ -62,7 +62,6 @@
ctf/check-file-data (fn [_] (throw (ex-info "checked" {})))]
(t/is (cfm/need-migration-to? file "test/2"))
(t/is (not (cfm/need-migration-to? file "test/1")))
(t/is (not (cfm/need-migration-to? file nil)))
(let [file' (cfm/migrate-file-to file nil "test/2")]
(t/is (= 2 (-> file' :data :sum)))