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)))))))))