mirror of
https://github.com/penpot/penpot.git
synced 2026-10-30 06:20:32 +00:00
🐛 Restore the pointer-map require the repair path still needs
The branch dropped `app.util.pointer-map` from the namespace while the auto-repair path (added to develop before this branch) still rebinds `pmap/*tracked*` around validation and repair in `repair-preexisting-damage` and `process-changes-and-validate`. The namespace did not compile, so the whole backend failed to load. Those toggles do not fit the fdata macros: `with-untracked` binds nil and `with-tracked` creates a fresh atom, while the repair path has to turn tracking off and restore the existing atom (and read it before the repair starts). Restore the require and keep the bindings explicit. Also pin the feature-off path with a test: with `fdata/pointer-map` off (the default), a save creates no fragments, file-gc writes nothing and the snapshot preview still returns plain data. Drop a stray `\n` char literal left at the end of one deftest. Update the memories with the exception to the macro rule, the snapshot FOR SHARE locking and the pre-migration automatic snapshot. Closes #12410 AI-assisted-by: deepseek-v4.1-flash
This commit is contained in:
parent
83185c3b08
commit
c1ac1bd4a4
@ -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.
|
||||
|
||||
@ -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`).
|
||||
@ -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]))
|
||||
|
||||
|
||||
@ -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])))))))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user