From de60e319eaa047867c8e33901dee36c9a31a3541 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 23 Oct 2026 17:58:07 +0000 Subject: [PATCH] :sparkles: Tell what an update-file changed in the audit event The update-file event said that a file was saved, but not what changed: the change row (xlog) was the only place with that, and the event did not point at it. A reaction of the events consumer cannot read a change it cannot name. The command now generates the id of the row it inserts and carries it in the event props, under :file-change-id. The response of the RPC does not change (the prop goes in the metadata of the result, not in the result), and the prop is a plain key with a uuid value, so clean-props keeps it. A save writes two change rows: the change itself, which has no label, and the snapshot of the previous state, which has one. The event points at the change one. AI-assisted-by: minimax-m3.1-flash --- .serena/memories/audit-log.md | 1 + .serena/memories/backend/subtleties.md | 2 +- backend/src/app/rpc/commands/files_update.clj | 13 +++++++-- backend/test/backend_tests/rpc_file_test.clj | 29 ++++++++++++++++++- 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/.serena/memories/audit-log.md b/.serena/memories/audit-log.md index e341ba745f..c31e1049c0 100644 --- a/.serena/memories/audit-log.md +++ b/.serena/memories/audit-log.md @@ -127,6 +127,7 @@ The event stream is meant to let a consumer (Nexus) rebuild what it needs **with - **Team id enrichment.** After `clean-props`, `prepare-rpc-event` calls `enrich-team-id`: when props have no `:team-id` but carry a `:project-id` or `:file-id`, it resolves the team with a local query and adds `:team-id`. It never resolves from `:id` (read commands put the entity id there, and that would add a query to every read), and it skips read events entirely (`get-*` names plus `has-file-libraries`: their events carry no consumer state). Commands that already loaded the team add `:team-id` themselves in `::audit/props` (`delete-file`, comments, media upload, `delete/update-webhook`, `clone-template`, ...), so the resolver does not run. - **Membership.** `add-profile-to-team!` emits `add-team-member` (props `:team-id`, `:member-id`, `:role`, `:is-owner`, `:is-admin`, `:can-edit`) whenever it actually inserts a row, only with `:audit-log` on, inside the caller's transaction. It covers the flows that add a member with no event of their own (`create-team` owner, invitee added directly when `:email-verification` is off, approved join-request, duplicated team). With `on-conflict-do-nothing?` a conflict returns nil and emits nothing, so an existing membership is reported once. The event belongs to the actor: the two flows where the owner adds somebody else (`create-invitation`, `add-member-to-team`) pass `:actor-id` through a separate `event-opts` arg (never in `params`, which is inserted as-is); everywhere else actor and member coincide. `:role` is derived in the emitter from the flags (inverse of `permissions-for-role`: owner > admin > editor > viewer) because the table has no role column; request context travels in `event-opts` as `:rpc-params`. - **Created entity ids.** `create-project` and `create-file` add the new row's `:id` (and `:project-id`/`:team-id`) to `::audit/props`, so a projection can key it. +- **Change ids.** `update-file` carries the id of the change row (xlog) it created in `:file-change-id`: the event says that a file was saved, but what changed lives in that row. A reaction of the events consumer reads it with `::db/remove-deleted false` (the row is born with its `deleted_at` in the future). A save writes two change rows (the change itself, without a label, and the snapshot of the previous state, with one) and the event points at the change one. - **Bulk operations.** `clone-template`, `import-binfile` and `duplicate-project` create entities through internal calls that emit nothing on their own; they emit a `create-file` (and `create-project`) per entity (only with `:audit-log`). The import paths go through `audit/submit-file-created-event`, which reads the light columns (`id`, `name`, `project-id`, timestamps, `revn`) back from the row — the file must really exist, a phantom id emits nothing. `duplicate-project` already holds the rows and uses `audit/submit-create-event` directly. Both binfile versions return the same shape from `import-files!` (`{:file-ids [...]}`). - **Deletes.** `delete-team` still returns nil but carries the deleted row (`:id`, `:name`, `:created-at`, `:modified-at`, `:deleted-at`) in `::audit/props`, like `delete-project`. `delete-profile` carries `:id`, `:email`, `:fullname`, `:created-at`, `:modified-at` and `:deleted-at` (without this its props are empty: every param is namespaced and `clean-props` drops them). `create-team` adds `:created-at` and `:modified-at`. `delete-team-member` needs nothing: the delete is physical and the params already carry `:team-id`/`:member-id`. - **Hard deletes.** `objects-gc` emits one `hard-delete-objects` event per purged chunk, carrying only that chunk's entity key (`:team-ids`, `:project-ids` or `:file-ids`; one chunk transaction purges a single entity, so one event can never span sections), with context `{:triggered-by "objects-gc"}`, only with `:audit-log` on. No team-id resolution: the live total only counts rows without `deleted_at`, which a physical purge cannot change, so no team-keyed consumer reads these events. No other GC path emits (profiles, fonts, thumbnails); those purges stay silent. `file-gc` is the exception: it emits `file-gc` (`action`, `triggered-by: cron-task`) with the cleaned `:file-id` and its `:team-id`, so the storage-usage consumer learns about trimmed media. diff --git a/.serena/memories/backend/subtleties.md b/.serena/memories/backend/subtleties.md index 85178658db..5e4076e175 100644 --- a/.serena/memories/backend/subtleties.md +++ b/.serena/memories/backend/subtleties.md @@ -110,6 +110,6 @@ - File snapshots (versions) keep their metadata row in `file_change` and their payload in `file_data` (`type = 'snapshot'`). With the `db` backend `file_change.data` stays NULL: that column is only written by the `legacy-db` backend, plus the rows written before the 2025-07 file storage refactor. - Tell a snapshot apart by `label IS NOT NULL` (with `created_by` / `deleted_at` as needed), never by `file_change.data IS NOT NULL`: the label holds with every file data backend, and the payload column stopped meaning "this is a snapshot" when the payload moved out. Queries that identify versions must use the label. - The version quota counts of `app.rpc.quotes` (`sql:get-snapshots-per-file`, `sql:get-snapshots-per-team`) are served by the partial index `file_change__user_snapshots__idx`, whose predicate mirrors their where clause; changing one without the other drops the index only scan and the count walks the whole table again. -- Revisions live only in `file_change` and have no `file_data` counterpart. They are born already scheduled for deletion one hour later (`app.rpc.commands.files-update`) and the snapshot paths cannot reach one, because they resolve their row through a `label IS NOT NULL` filter. +- Revisions live only in `file_change` and have no `file_data` counterpart. They are born already scheduled for deletion one hour later (`app.rpc.commands.files-update`) and the snapshot paths cannot reach one, because they resolve their row through a `label IS NOT NULL` filter. The revision row created by a save travels to the audit event in `:file-change-id` (the reaction reads it with `::db/remove-deleted false`, because the row is born with its `deleted_at` in the future); a save writes two rows, the revision (no `label`) and the snapshot of the previous state (with one). - A deletion date is assigned once and never moved: `restore-files` (`app.rpc.commands.files`) unmarks only durable history, the `:file` method of `app.tasks.delete-object` dates only rows that have no date yet, and `objects-gc` collects only rows that carry one. Unmarking a revision leaves it invisible to `objects-gc` for ever, because nothing sets that date again. - Binfile v1/v3 import and the v3 export `get-file` step run inside one long transaction. They raise `idle_in_transaction_session_timeout` to 30 min (`app.binfile.common/transaction-idle-timeout-ms`) instead of disabling it, so a connection left idle while the ZIP is decoded or the file data is realized is bounded; `statement_timeout` stays at 300 s. diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 99ea623b82..f63becbdfe 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -273,7 +273,13 @@ changes skip-validate)) deleted-at - (ct/plus timestamp (ct/duration {:hours 1}))] + (ct/plus timestamp (ct/duration {:hours 1})) + + ;; The id of the change row (xlog) goes to the audit event + ;; too: the reactions of the events consumer need to know what + ;; the save changed, and that row is the only place with it. + file-change-id + (uuid/next)] (when-let [file (::snapshot file)] (let [deleted-at (ct/plus timestamp (ldel/get-deletion-delay team)) @@ -289,7 +295,7 @@ ;; Insert change (xlog) with deleted_at in a future data for ;; make them automatically eleggible for GC once they expires (db/insert! conn :file-change - {:id (uuid/next) + {:id file-change-id :session-id session-id :profile-id profile-id :created-at timestamp @@ -317,6 +323,9 @@ :features (:features file) :project-id (:project-id file) :team-id (:team-id file) + ;; The id of the change row inserted above: what the save + ;; changed lives in that row, not in the event. + :file-change-id file-change-id ;; NOTE: `file` carries the post-update revn here (`revn` ;; above is the pre-update one kept for the response), and ;; `modified-at` mirrors `persist-file!`: same `::timestamp` diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 68f7850486..abe80eb770 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -23,7 +23,7 @@ [app.features.fdata :as fdata] [app.http :as http] [app.jobs :as-alias jobs] - [app.loggers.audit :as-alias audit] + [app.loggers.audit :as audit] [app.rpc :as-alias rpc] [app.rpc.commands.files :as files] [app.storage :as sto] @@ -3309,6 +3309,33 @@ (t/is (= (:id team) (get-in (meta (:result out)) [:app.loggers.audit/props :team-id]))))) +(t/deftest update-file-audit-event-carries-the-file-change-id + ;; The event says that a file was saved, but not what changed: the + ;; change row (xlog) is the only place with that, so its id travels + ;; in the event. A reaction of the events consumer needs it to read + ;; the change (with `::db/remove-deleted false`: the row is born + ;; with its `deleted_at` in the future). + (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}) + result (update-file! :file-id (:id file) + :profile-id (:id profile) + :revn 0 + :changes []) + props (get-in (meta result) [:app.loggers.audit/replace-props]) + change-id (:file-change-id props) + rows (th/db-query :file-change {:file-id (:id file)}) + ;; The save writes two change rows: the change itself (no + ;; label) and the snapshot of the previous state (with one). + changes (remove :label rows)] + ;; The event points at the change row of this save. + (t/is (= 1 (count changes))) + (t/is (= (:id (first changes)) change-id)) + ;; The prop survives `clean-props`: a plain key with a uuid value. + (t/is (= change-id + (:file-change-id (audit/clean-props props)))))) + (t/deftest get-team-shared-files-skips-files-that-fail-to-load (let [profile (th/create-profile* 1) file1 (th/create-file* 1 {:profile-id (:id profile)