Skip to content
Snippets Groups Projects
Unverified Commit e8fc1a0a authored by Chris Truter's avatar Chris Truter Committed by GitHub
Browse files

Publish audit logs for CSV uploads (#39821)

parent f7103f08
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/events/audit_log.clj
+ 8
− 0
View file @ e8fc1a0a
......@@ -209,3 +209,11 @@
(methodical/defmethod events/publish-event! ::api-key-event
[topic event]
(audit-log/record-event! topic event))
(derive ::upload-event ::event)
(derive :event/upload-create ::upload-event)
(derive :event/upload-append ::upload-event)
(methodical/defmethod events/publish-event! ::upload-event
[topic event]
(audit-log/record-event! topic event))
src/metabase/upload.clj
+ 47
− 13
View file @ e8fc1a0a
......@@ -13,6 +13,7 @@
[metabase.driver :as driver]
[metabase.driver.sync :as driver.s]
[metabase.driver.util :as driver.u]
[metabase.events :as events]
[metabase.lib.core :as lib]
[metabase.mbql.util :as mbql.u]
[metabase.models :refer [Database]]
......@@ -366,6 +367,9 @@
auto-pk-indices
(map (partial remove-indices auto-pk-indices)))))
(defn- file-size-mb [csv-file]
(/ (.length ^File csv-file) 1048576.0))
(defn- load-from-csv!
"Loads a table from a CSV file. If the table already exists, it will throw an error.
Returns the file size, number of rows, and number of columns."
......@@ -389,8 +393,7 @@
{:num-rows (count rows)
:num-columns (count extant-columns)
:generated-columns (count generated-columns)
:size-mb (/ (.length csv-file)
1048576.0)}
:size-mb (file-size-mb csv-file)}
(catch Throwable e
(driver/drop-table! driver db-id table-name)
(throw (ex-info (ex-message e) {:status-code 400})))))))
......@@ -461,6 +464,10 @@
[db schema-name]
(nil? (can-create-upload-error db schema-name)))
(defn- start-timer [] (System/nanoTime))
(defn- since-ms [timer] (/ (- (System/nanoTime) timer) 1e6))
;;; +-----------------------------------------
;;; | public interface for creating CSV table
;;; +-----------------------------------------
......@@ -501,7 +508,7 @@
(check-can-create-upload database schema-name)
(collection/check-write-perms-for-collection collection-id)
(try
(let [start-time (System/currentTimeMillis)
(let [timer (start-timer)
driver (driver.u/database->driver database)
filename-prefix (or (second (re-matches #"(.*)\.csv$" filename))
filename)
......@@ -529,14 +536,22 @@
:name (humanization/name->human-readable-name filename-prefix)
:visualization_settings {}}
@api/*current-user*)
upload-seconds (/ (- (System/currentTimeMillis) start-time)
1000.0)]
upload-seconds (/ (since-ms timer) 1e3)
stats (assoc stats :upload-seconds upload-seconds)]
(events/publish-event! :event/upload-create
{:user-id (:id @api/*current-user*)
:model-id (:id table)
:model :model/Table
:details {:db-id db-id
:schema-name schema-name
:table-name table-name
:model-id (:id card)
:stats stats}})
(snowplow/track-event! ::snowplow/csv-upload-successful
api/*current-user-id*
(merge
{:model-id (:id card)
:upload-seconds upload-seconds}
stats))
(assoc stats :model-id (:id card)))
card)
(catch Throwable e
(let [fail-stats (with-open [reader (bom/bom-reader file)]
......@@ -622,7 +637,8 @@
(defn- append-csv!*
[database table file]
(with-open [reader (bom/bom-reader file)]
(let [[header & rows] (without-auto-pk-columns (csv/read-csv reader))
(let [timer (start-timer)
[header & rows] (without-auto-pk-columns (csv/read-csv reader))
driver (driver.u/database->driver database)
normed-name->field (m/index-by (comp normalize-column-name :name)
(t2/select :model/Field :table_id (:id table) :active true))
......@@ -646,23 +662,41 @@
(->> (changed-field->new-type fields old-column-types relaxed-types)
(alter-columns! driver database table))))
;; this will fail if any of our required relaxations were rejected.
parsed-rows (parse-rows settings new-column-types rows)]
parsed-rows (parse-rows settings new-column-types rows)
row-count (count parsed-rows)]
(try
(driver/insert-into! driver (:id database) (table-identifier table) normed-header parsed-rows)
(catch Throwable e
(throw (ex-info (ex-message e) {:status-code 422}))))
(when create-auto-pk?
(driver/add-columns! driver
(:id database)
(table-identifier table)
{auto-pk-column-keyword (driver/upload-type->database-type driver ::auto-incrementing-int-pk)}
:primary-key [auto-pk-column-keyword]))
(scan-and-sync-table! database table)
(when create-auto-pk?
(let [auto-pk-field (table-id->auto-pk-column (:id table))]
(t2/update! :model/Field (:id auto-pk-field) {:display_name (:name auto-pk-field)})))
{:row-count (count parsed-rows)})))
(events/publish-event! :event/upload-append
{:user-id (:id @api/*current-user*)
:model-id (:id table)
:model :model/Table
:details {:db-id (:id database)
:schema-name (:schema table)
:table-name (:name table)
:stats {:num-rows row-count
:num-columns (count new-column-types)
:generated-columns (if create-auto-pk? 1 0)
:size-mb (file-size-mb file)
:upload-seconds (since-ms timer)}}})
{:row-count row-count})))
(defn- can-append-error
"Returns an ExceptionInfo object if the user cannot upload to the given database and schema. Returns nil otherwise."
......
test/metabase/upload_test.clj
+ 53
− 0
View file @ e8fc1a0a
......@@ -431,6 +431,11 @@
names (repeatedly n (partial #'upload/unique-table-name driver/*driver* ""))]
(is (= 50 (count (distinct names))))))))
(defn last-audit-event [topic]
(t2/select-one [:model/AuditLog :topic :user_id :model :model_id :details]
:topic topic
{:order-by [[:id :desc]]}))
(defn upload-example-csv!
"Upload a small CSV file to the given collection ID. `grant-permission?` controls whether the
current user is granted data permissions to the database."
......@@ -1127,6 +1132,28 @@
:user-id (str (mt/user->id :rasta))}
(last (snowplow-test/pop-event-data-and-user-id!)))))))))
(deftest csv-upload-audit-log-test
;; Just test with h2 because these events are independent of the driver
(mt/test-driver :h2
(mt/with-premium-features #{:audit-app}
(with-upload-table!
[_table (card->table (upload-example-csv!))]
(is (=? {:topic :upload-create
:user_id (:id (mt/fetch-user :rasta))
:model "Table"
:model_id pos?
:details {:db-id pos?
:schema-name "PUBLIC"
:table-name string?
:model-id pos?
:stats {:num-rows 2
:num-columns 2
:generated-columns 1
:size-mb 3.910064697265625E-5
:upload-seconds pos?}}}
(last-audit-event :upload-create)))))))
(deftest create-csv-upload!-failure-test
;; Just test with postgres because failure should be independent of the driver
(mt/test-driver :postgres
......@@ -1484,6 +1511,32 @@
(rows-for-table table))))
(io/delete-file file))))))
(deftest csv-append-audit-log-test
;; Just test with h2 because these events are independent of the driver
(mt/test-driver :h2
(mt/with-premium-features #{:audit-app}
(with-upload-table! [table (create-upload-table!)]
(let [csv-rows ["name" "Luke Skywalker"]
file (csv-file-with csv-rows (mt/random-name))]
(append-csv! {:file file, :table-id (:id table)})
(is (=? {:topic :upload-append
:user_id (:id (mt/fetch-user :crowberto))
:model "Table"
:model_id (:id table)
:details {:db-id pos?
:schema-name "PUBLIC"
:table-name string?
:stats {:num-rows 1
:num-columns 1
:generated-columns 0
:size-mb 1.811981201171875E-5
:upload-seconds pos?}}}
(last-audit-event :upload-append)))
(io/delete-file file))))))
(deftest append-mb-row-id-csv-and-table-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
(testing "Append succeeds if the table has _mb_row_id and the CSV does too"
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment