From bdcb979211f7a4080107eda1dcd2225572627cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Tejero=20Cantero?= Date: Fri, 14 Aug 2026 00:59:17 +0200 Subject: [PATCH] :sparkles: Let any branch become an ordinary file ::materialize-file-branch persists a branch's derived state as the file's own data payload, drops the op log, releases the pinned base snapshot, closes any pull request over it and clears the branch marker, so every read and write path takes the ordinary route from then on. That is the preview's exit door: the flag can go off and the engine can be replaced without stranding anybody with an unreadable file. It keys on the branch file rather than the branch metadata, so calling it twice is a reported no-op rather than an error. --- backend/src/app/rpc/commands/files_branch.clj | 94 +++++++++++++++++++ .../backend_tests/rpc_file_branch_test.clj | 78 +++++++++++++++ 2 files changed, 172 insertions(+) diff --git a/backend/src/app/rpc/commands/files_branch.clj b/backend/src/app/rpc/commands/files_branch.clj index 9a68c4fd2e..45648b2a71 100644 --- a/backend/src/app/rpc/commands/files_branch.clj +++ b/backend/src/app/rpc/commands/files_branch.clj @@ -1005,6 +1005,100 @@ {:status :updated :revn (:revn updated)})))))))))))) +;; --- COMMAND: materialize-file-branch + +(def ^:private schema:materialize-file-branch + [:map {:title "materialize-file-branch"} + [:file-id ::sm/uuid]]) + +(sv/defmethod ::materialize-file-branch + "Turn a branch file into an ordinary file: persist its derived state as + the file's own data payload, drop the op log, release the pinned base + snapshot, close any pull request over it, and clear the branch marker so + every read and write path takes the ordinary route from then on + (`binfile::get-file*` and `files-update::update-file*` both switch on + that flag). + + This is the preview's exit door: with it the `:branching` flag can go + off and the engine can be replaced without stranding anybody with a file + nobody can open. + + It keys on the branch FILE rather than on the branch metadata, which is + what makes it idempotent: a file carrying no live branch row is already + materialised, and the command reports that without touching anything." + {::doc/added "2.16" + ::webhooks/event? true + ::sm/params schema:materialize-file-branch + ::climit/id [[:materialize-file-branch/global]]} + [cfg {:keys [::rpc/profile-id file-id]}] + (check-branching-enabled!) + (files/check-edition-permissions! cfg profile-id file-id) + (db/tx-run! + cfg + (fn [{:keys [::db/conn] :as cfg}] + ;; the advisory lock every save on this file takes: a concurrent + ;; branch save must not land between the derive and the persist, or + ;; its op would be dropped together with the log + (db/xact-lock! conn file-id) + + (let [branch (db/get* conn :file-branch {:branch-file-id file-id})] + (if (nil? branch) + {:status :materialized :file-id file-id :changed false} + (let [file (bfc/get-file cfg file-id :realize? true) + team (teams/get-team conn :profile-id profile-id :file-id file-id) + delay (ldel/get-deletion-delay team) + ts (ct/now)] + + (binding [pmap/*tracked* (pmap/create-tracked) + pmap/*load-fn* (partial fdata/load-pointer cfg file-id) + cfeat/*current* (:features file) + cfeat/*previous* (:features file)] + + (let [libs (bfc/get-resolved-file-libraries cfg file) + errors (not-empty (cfv/validate-file file libs)) + file (if errors + (update file :data cpc/process-changes + (cfr/repair-file file libs errors)) + file)] + + ;; the derived state becomes the file's own payload + (fupd/persist-file! (assoc cfg ::fupd/timestamp ts) file) + + ;; from here the ordinary paths apply + (db/update! conn :file + {:is-branch false} + {:id file-id} + {::db/return-keys false}) + + ;; the payload replaced the log + (db/delete! conn :file-branch-change {:branch-id (:id branch)}) + + ;; a review sandbox over a file that is no longer a branch + ;; has nothing left to review + (close-branch-pull-requests! cfg (:id branch) + {:profile-id profile-id + :status "closed" + :deleted-at (ct/in-future delay)}) + + ;; nothing derives from the base any more, so the pin goes + (release-base-snapshot! cfg branch (ct/in-future delay)) + + ;; The branch row is archived rather than deleted: its pull + ;; requests reference it with ON DELETE CASCADE, so deleting + ;; it would take the review history along, and `archived` is + ;; a status the schema already allows. + (db/update! conn :file-branch + {:status "archived" + :deleted-at (ct/in-future delay) + :updated-at ts} + {:id (:id branch)} + {::db/return-keys false}) + + {:status :materialized + :file-id file-id + :changed true + :revn (:revn file)})))))))) + ;; --- COMMAND QUERY: get-file-branch-info (def ^:private schema:get-file-branch-info diff --git a/backend/test/backend_tests/rpc_file_branch_test.clj b/backend/test/backend_tests/rpc_file_branch_test.clj index 4beedb8d1e..e42a8f3b7d 100644 --- a/backend/test/backend_tests/rpc_file_branch_test.clj +++ b/backend/test/backend_tests/rpc_file_branch_test.clj @@ -6,8 +6,10 @@ (ns backend-tests.rpc-file-branch-test (:require + [app.binfile.common :as bfc] [app.common.features :as cfeat] [app.common.files.branch-merge :as bm] + [app.common.files.validate :as cfv] [app.common.time :as ct] [app.common.types.shape :as cts] [app.common.uuid :as uuid] @@ -922,3 +924,79 @@ (let [row (th/db-get :file-media-object {:id (:id added)})] (t/is (= (:id file) (:file-id row))) (t/is (= (:id sobj2) (:media-id row))))))))) + +(t/deftest materialize-branch-into-an-ordinary-file + (with-redefs [cf/flags (conj cf/flags :branching)] + (let [profile (th/create-profile* 1 {:is-active true}) + proj-id (:default-project-id profile) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id proj-id + :is-shared false}) + create (:result (th/command! {::th/type :create-file-branch + ::rpc/profile-id (:id profile) + :file-id (:id file) + :name "exit-door"})) + branch-id (:id create) + branch-file-id (:branch-file-id create) + page-1 (uuid/random) + page-2 (uuid/random)] + + (t/testing "a non-trivial op log: two saves on the branch" + (doseq [[page-id nm] [[page-1 "log-one"] [page-2 "log-two"]]] + (let [bf (th/db-get :file {:id branch-file-id}) + out (th/command! {::th/type :update-file + ::rpc/profile-id (:id profile) + :id branch-file-id + :session-id (uuid/random) + :revn (:revn bf) + :vern (:vern bf) + :features cfeat/supported-features + :changes [{:type :add-page :id page-id :name nm}]})] + (t/is (nil? (:error out))))) + (t/is (= 2 (count (th/db-query :file-branch-change {:branch-id branch-id}))))) + + (let [before (:result (th/command! {::th/type :get-file + ::rpc/profile-id (:id profile) + :id branch-file-id}))] + + (t/testing "materialize persists the derived state and drops the branch" + (let [out (th/command! {::th/type :materialize-file-branch + ::rpc/profile-id (:id profile) + :file-id branch-file-id})] + (t/is (nil? (:error out))) + (t/is (= :materialized (-> out :result :status))) + (t/is (true? (-> out :result :changed)))) + + (let [row (th/db-get :file {:id branch-file-id})] + (t/is (false? (:is-branch row))) + (t/is (some? (:data row)))) + + (t/is (empty? (th/db-query :file-branch-change {:branch-id branch-id}))) + + (let [row (th/db-get :file-branch {:id branch-id})] + (t/is (= "archived" (:status row))) + (t/is (some? (:deleted-at row))))) + + (t/testing "the file still opens with the state the log produced" + (let [after (:result (th/command! {::th/type :get-file + ::rpc/profile-id (:id profile) + :id branch-file-id})) + pages (-> after :data :pages-index)] + (t/is (contains? pages page-1)) + (t/is (contains? pages page-2)) + (t/is (= (-> before :data :pages) (-> after :data :pages))))) + + (t/testing "the materialised file validates against its libraries" + (let [errors (db/run! th/*system* + (fn [cfg] + (let [f (bfc/get-file cfg branch-file-id :realize? true)] + (cfv/validate-file f (bfc/get-resolved-file-libraries cfg f)))))] + (t/is (empty? errors)))) + + (t/testing "materializing again is a no-op" + (let [out (th/command! {::th/type :materialize-file-branch + ::rpc/profile-id (:id profile) + :file-id branch-file-id})] + (t/is (nil? (:error out))) + (t/is (= :materialized (-> out :result :status))) + (t/is (false? (-> out :result :changed)))))))))