mirror of
https://github.com/penpot/penpot.git
synced 2026-10-28 21:40:34 +00:00
✨ Record imported file ids in the import-binfile audit event (#12491)
The RPC layer emitted the event as soon as the handler returned the SSE response, before the import ran, so the event could not say which files the import created. Emit it from inside the SSE handler once the import ends, with the ids and count of the created files and whether the import completed or failed. Closes #12485 AI-assisted-by: claude-opus-5-5
This commit is contained in:
parent
f44566963d
commit
3f8aee7933
@ -17,7 +17,7 @@
|
||||
[app.config :as cf]
|
||||
[app.db :as db]
|
||||
[app.http.sse :as sse]
|
||||
[app.loggers.audit :as-alias audit]
|
||||
[app.loggers.audit :as audit]
|
||||
[app.loggers.webhooks :as-alias webhooks]
|
||||
[app.media.validation :as media.v]
|
||||
[app.rpc :as-alias rpc]
|
||||
@ -123,6 +123,34 @@
|
||||
|
||||
result))
|
||||
|
||||
(defn- prepare-import-event
|
||||
"Build the audit event of an import. The RPC layer cannot emit it
|
||||
because the import runs later, inside the SSE response, and the
|
||||
event must carry the ids of the files it creates."
|
||||
[params manifest]
|
||||
(-> (audit/event-from-rpc-params params)
|
||||
(assoc :type "command")
|
||||
(assoc :name "import-binfile")
|
||||
(assoc :props (-> params
|
||||
(dissoc :file)
|
||||
(assoc :generated-by (:generated-by manifest))
|
||||
(assoc :referer (:referer manifest))
|
||||
(audit/clean-props)))
|
||||
(assoc ::webhooks/event? true)))
|
||||
|
||||
(defn- import-binfile-and-audit
|
||||
[cfg params event]
|
||||
(try
|
||||
(let [file-ids (import-binfile cfg params)]
|
||||
(audit/submit cfg (update event :props assoc
|
||||
:file-ids (vec file-ids)
|
||||
:file-count (count file-ids)
|
||||
:status "completed"))
|
||||
file-ids)
|
||||
(catch Throwable cause
|
||||
(audit/submit cfg (assoc-in event [:props :status] "failed"))
|
||||
(throw cause))))
|
||||
|
||||
(def ^:private schema:import-binfile
|
||||
[:and
|
||||
[:map {:title "import-binfile"}
|
||||
@ -148,6 +176,7 @@
|
||||
["2.15" "Add upload-id param for chunked upload support"]]
|
||||
|
||||
::webhooks/event? true
|
||||
::audit/skip true
|
||||
::sse/stream? true
|
||||
::sm/params schema:import-binfile
|
||||
::climit/id [[:import-binfile/by-profile ::rpc/profile-id]
|
||||
@ -164,10 +193,6 @@
|
||||
:binfile-v1 1
|
||||
:binfile-v3 3))
|
||||
|
||||
params (-> params
|
||||
(assoc :profile-id profile-id)
|
||||
(assoc :version version))
|
||||
|
||||
manifest
|
||||
(case (int version)
|
||||
1 nil
|
||||
@ -175,10 +200,10 @@
|
||||
(throw (ex-info (str "Unsupported binfile version: " version)
|
||||
{:type :validation
|
||||
:code :unsupported-version
|
||||
:version version})))]
|
||||
:version version})))
|
||||
|
||||
(with-meta
|
||||
(sse/response (partial import-binfile cfg params))
|
||||
{::audit/props {:file nil
|
||||
:generated-by (:generated-by manifest)
|
||||
:referer (:referer manifest)}})))
|
||||
params (assoc params :version version)
|
||||
event (prepare-import-event params manifest)
|
||||
params (assoc params :profile-id profile-id)]
|
||||
|
||||
(sse/response (partial import-binfile-and-audit cfg params event))))
|
||||
|
||||
@ -6,12 +6,18 @@
|
||||
|
||||
(ns backend-tests.rpc-binfile-test
|
||||
(:require
|
||||
[app.binfile.common :as bfc]
|
||||
[app.binfile.v3 :as v3]
|
||||
[app.common.schema :as sm]
|
||||
[app.common.uuid :as uuid]
|
||||
[app.rpc :as-alias rpc]
|
||||
[app.rpc.commands.binfile :as binfile]
|
||||
[app.storage.tmp :as tmp]
|
||||
[backend-tests.helpers :as th]
|
||||
[clojure.test :as t]))
|
||||
[clojure.test :as t]
|
||||
[datoteka.fs :as fs]
|
||||
[datoteka.io :as io]
|
||||
[mockery.core :refer [with-mocks]]))
|
||||
|
||||
(t/use-fixtures :once th/state-init)
|
||||
(t/use-fixtures :each th/database-reset)
|
||||
@ -63,3 +69,69 @@
|
||||
;; Version 4 should be rejected
|
||||
(t/is (false? (validator (assoc base-params :version 4)))
|
||||
"version 4 should be rejected")))
|
||||
|
||||
(defn- export-file
|
||||
[profile]
|
||||
(let [file (th/create-file* 1 {:profile-id (:id profile)
|
||||
:project-id (:default-project-id profile)
|
||||
:is-shared false})
|
||||
output (tmp/tempfile :suffix ".zip")]
|
||||
(v3/export-files!
|
||||
(-> th/*system*
|
||||
(assoc ::bfc/ids #{(:id file)})
|
||||
(assoc ::bfc/embed-assets false)
|
||||
(assoc ::bfc/include-libraries false))
|
||||
(io/output-stream output))
|
||||
output))
|
||||
|
||||
(defn- import-file
|
||||
[profile path]
|
||||
(let [out (th/command! {::th/type :import-binfile
|
||||
::rpc/profile-id (:id profile)
|
||||
:project-id (:default-project-id profile)
|
||||
:name "imported"
|
||||
:file {:filename "imported.penpot"
|
||||
:size (fs/size path)
|
||||
:path path
|
||||
:mtype "application/zip"}})]
|
||||
(t/is (nil? (:error out)))
|
||||
(th/consume-sse (:result out))))
|
||||
|
||||
(defn- import-events
|
||||
[mock]
|
||||
(->> (:call-args-list @mock)
|
||||
(map second)
|
||||
(filter #(= "import-binfile" (:name %)))))
|
||||
|
||||
(t/deftest import-binfile-audit-event-records-file-ids
|
||||
(with-mocks [mock {:target 'app.loggers.audit/submit :return nil}]
|
||||
(let [profile (th/create-profile* 1 {:is-active true})
|
||||
events (import-file profile (export-file profile))
|
||||
file-ids (-> events last second)
|
||||
[event :as audit-events] (import-events mock)]
|
||||
|
||||
(t/is (= :end (-> events last first)))
|
||||
(t/is (= 1 (count audit-events)))
|
||||
(t/is (= 1 (count file-ids)))
|
||||
(t/is (= (vec file-ids) (-> event :props :file-ids)))
|
||||
(t/is (= 1 (-> event :props :file-count)))
|
||||
(t/is (= "completed" (-> event :props :status)))
|
||||
(t/is (= (:default-project-id profile) (-> event :props :project-id)))
|
||||
(t/is (= "penpot" (-> event :props :referer)))
|
||||
(t/is (= (:id profile) (:profile-id event)))
|
||||
(t/is (= "command" (:type event)))
|
||||
(t/is (not (contains? (:props event) :file))))))
|
||||
|
||||
(t/deftest import-binfile-audit-event-records-failure
|
||||
(with-mocks [mock {:target 'app.loggers.audit/submit :return nil}]
|
||||
(let [profile (th/create-profile* 1 {:is-active true})
|
||||
path (export-file profile)
|
||||
events (with-redefs [v3/import-files!
|
||||
(fn [_] (throw (ex-info "boom" {:type :internal})))]
|
||||
(import-file profile path))
|
||||
[event :as audit-events] (import-events mock)]
|
||||
|
||||
(t/is (= :error (-> events last first)))
|
||||
(t/is (= 1 (count audit-events)))
|
||||
(t/is (= "failed" (-> event :props :status)))
|
||||
(t/is (not (contains? (:props event) :file-ids))))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user