diff --git a/.serena/memories/backend/subtleties.md b/.serena/memories/backend/subtleties.md index b6c3c9afb3..67d049fe3c 100644 --- a/.serena/memories/backend/subtleties.md +++ b/.serena/memories/backend/subtleties.md @@ -94,10 +94,11 @@ - Pointer-map fragments are persisted separately as type `fragment`, and only modified pointer maps are written. A pointer keeps its id while nothing changes; content (`assoc`/`without`) and metadata (`with-meta`) changes produce a fresh id, so a fragment row is never rewritten after it is written. Re-assoc with an identical value returns the same map in array and hash maps alike, so a save keeps the fragment id of every page it does not change. - `enable-objects-map` skips unloaded pointer pages: their fragment was already wrapped when it was written, and loading it only to wrap it again costs one page read per untouched page on every save. A page with a plain fragment is wrapped when it is first loaded or edited; a data migration that enables `fdata/objects-map` must rewrite the data itself. - Snapshots share fragment rows with the live file (they are stored with pointers, not materialized). `file-gc` marks every fragment referenced neither by the current main file nor by an unpurged snapshot; the file is read unrealized and unchanged pages keep their ids via reattach, so a pass over an unchanged file writes no new fragments. -- A save attaches its automatic snapshot before applying pending file migrations: the snapshot keeps the pre-migration pointers and migration set, and restoring it leaves the next read to migrate the file. +- A save attaches its automatic snapshot before applying pending file migrations: the snapshot keeps the pre-migration pointers and migration set, and restoring it leaves the next read to migrate the file. This also holds for files without the feature (the snapshot keeps the pre-migration format; `get-file-snapshot` returns it as stored), while `create-file-snapshot` and the restore backup read with `migrate?` on, so those versions are stored already migrated. - The migration read path (`binfile.common/migrate-file`) captures `fdata/original-pointers` before realizing, reattaches unchanged positions after applying the migrations, and persists the reattached file; it returns the realized file because reads expose plain data. A migration that adds `fdata/objects-map` skips the reattach so the pages get wrapped. - `fdata/load-pointer` reads rows that a previous `file-gc` pass marked on purpose (explicit `::db/remove-deleted false`): the fragment is only gone once `objects-gc` purges it. The `get-file-fragment` RPC keeps rejecting marked rows. -- Bind the fragment loader through `fdata/with-pointers` (reads), `fdata/with-tracked-pointers` (writes with a loader) or `fdata/with-tracked` (writes with nothing to load yet, e.g. file creation), and `fdata/with-untracked` (validation passes that must track nothing) instead of hand-rolled `binding` forms; `file-snapshots/reduce-snapshots` already binds its loader while walking snapshot content. +- Bind the fragment loader through `fdata/with-pointers` (reads), `fdata/with-tracked-pointers` (writes with a loader) or `fdata/with-tracked` (writes with nothing to load yet, e.g. file creation), and `fdata/with-untracked` (validation passes that must track nothing) instead of hand-rolled `binding` forms; `file-snapshots/reduce-snapshots` already binds its loader while walking snapshot content. The fine-grained toggles of an existing tracked set live in `app.rpc.commands.files-update` (`repair-preexisting-damage` and `process-changes-and-validate` rebind `pmap/*tracked*` around validation and repair), so that namespace still requires `app.util.pointer-map` directly. +- Snapshot creation takes `FOR SHARE` on the file row: both `create-file-snapshot` and the backup read of `restore-file-snapshot` hold it from the read to the insert, so they serialize with a `file-gc` pass (`FOR UPDATE ... SKIP LOCKED`) instead of pinning fragments the pass is about to mark. `get-file-snapshot` realizes the snapshot pointers with the live file id (fragment rows are keyed by file, not by snapshot). - `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. diff --git a/.serena/memories/frontend/dashboard-viewer-subtleties.md b/.serena/memories/frontend/dashboard-viewer-subtleties.md index 9c753d8875..c8dba9b2d8 100644 --- a/.serena/memories/frontend/dashboard-viewer-subtleties.md +++ b/.serena/memories/frontend/dashboard-viewer-subtleties.md @@ -13,6 +13,6 @@ - `app.main.ui.ui` parses the viewer's `file-id` query param with `uuid/parse*`, so a missing or malformed id arrives as nil. `viewer-page*` calls `check-file-id!` (same namespace) during render, before mounting `viewer*`: it raises `ex/raise :type :not-found :code :missing-file-id`, which the error boundary turns into the not-found page. Without the guard the `dv/initialize` assertion fails on the nil file-id and the user gets an internal-error page. Test: `frontend-tests.ui.viewer-test/check-file-id-raises-not-found-for-invalid-file-id`. - Viewer initialization sets `:current-file-id`, `:current-share-id`, and `:viewer-local`, then fetches the view-only bundle. Comment threads are fetched only for logged-in users. - Viewer bundle fetch sends the full supported feature set because anonymous shared viewers may not know team-enabled features. -- View-only bundles can contain pointer values in `:pages-index` and file data. Viewer resolves those fragments with `:get-file-fragment` before storing the bundle. +- View-only bundles can contain pointer values in `:pages-index` and file data. Viewer resolves those fragments with `:get-file-fragment` before storing the bundle, through `app.main.features.pointer-map/resolve-pointer` (share-link scoped; the workspace and library loaders use `resolve-file`, which resolves the same two levels). Both fail loudly: a fragment that cannot be resolved rejects the stream and is logged with file id and fragment id instead of silently dropping the page. - `bundle-fetched` indexes pages and precomputes viewer frames/all-frames, stores libraries/users/thumbnails/permissions under `:viewer`, then navigates to frame id, query index, or auto-selected frame. - Viewer zoom and interaction mode changes update both `:viewer-local` and the `:viewer` route query params. `update-zoom-querystring` guards with a query-param comparison (`not= current expected`) and uses `::rt/replace`; without that guard the load sequence (`bundle-fetched` → `zoom-to-fill` → `update-zoom-querystring` → `rt/nav`) re-runs on every navigation and crashes with React "maximum update depth exceeded" (2.18.0-RC5 regression, fixed in `31b73460c3`; regression test: `bundle-fetched-with-zoom-fill-url-does-not-navigate`). \ No newline at end of file diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index dc4e7ea83c..34902922bf 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -36,6 +36,7 @@ [app.rpc.doc :as-alias doc] [app.rpc.helpers :as rph] [app.util.blob :as blob] + [app.util.pointer-map :as pmap] [app.util.services :as sv] [clojure.set :as set])) diff --git a/backend/test/backend_tests/file_pointer_snapshot_test.clj b/backend/test/backend_tests/file_pointer_snapshot_test.clj index cf30b135c7..8d1fcd34ab 100644 --- a/backend/test/backend_tests/file_pointer_snapshot_test.clj +++ b/backend/test/backend_tests/file_pointer_snapshot_test.clj @@ -29,14 +29,16 @@ (t/use-fixtures :each th/database-reset) (defn- update-file! - [& {:keys [profile-id file-id changes revn] :or {revn 0}}] + [& {:keys [profile-id file-id changes revn features] + :or {revn 0 + features cfeat/supported-features}}] (let [params {::th/type :update-file ::rpc/profile-id profile-id :id file-id :session-id (uuid/random) :revn revn :vern 0 - :features cfeat/supported-features + :features features :changes changes} out (th/command! params)] (t/is (nil? (:error out))) @@ -516,7 +518,8 @@ (t/is (contains? @pmap/*tracked* (pmap/get-id wrapped))) (t/is (contains? @pmap/*tracked* (pmap/get-id unloaded))) (t/is (= {:a 1} @wrapped)) - (t/is (thrown? UnsupportedOperationException @unloaded))))) \n + (t/is (thrown? UnsupportedOperationException @unloaded))))) + (t/deftest legacy-snapshot-does-not-break-liveness ;; Snapshots written before the feature carry materialized data in ;; file_change.data (backend "legacy-db") instead of a file_data row. @@ -954,3 +957,49 @@ (fdata/realize-entry))))))] (t/is (= lib-page (:id page))) (t/is (contains? (:objects page) uuid/zero))))) + +(t/deftest feature-off-keeps-plain-snapshots-and-gc + ;; The feature is disabled by default (`fdata/pointer-map` is not in + ;; `cfeat/default-features`, only an explicit flag or team feature + ;; enables it). Pin that the paths this branch touches stay plain for + ;; such a file: a save takes a snapshot without pointers, no fragment + ;; rows appear, file-gc writes nothing, and the preview still resolves. + (with-redefs [cfeat/get-enabled-features (fn [& _] cfeat/default-features)] + (let [features (disj cfeat/supported-features + "fdata/pointer-map" + "fdata/objects-map") + profile (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + file-id (:id file) + page-id (first (get-in file [:data :pages]))] + + (t/is (not (contains? (:features file) "fdata/pointer-map"))) + (t/is (not (contains? (:features file) "fdata/objects-map"))) + (t/is (empty? (fragment-rows file-id))) + + (update-file! :profile-id (:id profile) + :file-id file-id + :revn 0 + :features features + :changes (add-rect-change page-id (uuid/random))) + + (t/is (empty? (fragment-rows file-id))) + + (let [snapshot (first (snapshots-by-revn file-id 0)) + snap (fsnap/get-snapshot th/*system* file-id (:id snapshot))] + (t/is (uuid? (:id snapshot))) + (t/is (empty? (fdata/get-used-pointer-ids (:data snap)))) + (t/is (true? (th/run-task! :file-gc {:file-id file-id}))) + (t/is (empty? (fragment-rows file-id))) + + (let [out (th/command! {::th/type :get-file-snapshot + ::rpc/profile-id (:id profile) + :file-id file-id + :id (:id snapshot) + :features features})] + (t/is (nil? (:error out))) + (t/is (map? (get-in out [:result :data]))) + (t/is (empty? (fdata/get-used-pointer-ids + (get-in out [:result :data])))))))))