Skip to content
Snippets Groups Projects
Unverified Commit 38719125 authored by metabase-bot[bot]'s avatar metabase-bot[bot] Committed by GitHub
Browse files

Redshift support for CSV uploads (take 3) (#38567) (#38769) 


Co-authored-by: default avatarCal Herries <39073188+calherries@users.noreply.github.com>
parent 0fa6e099
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 272 additions and 273 deletions
enterprise/backend/test/metabase_enterprise/advanced_permissions/common_test.clj
+ 74
− 106
View file @ 38719125
(ns metabase-enterprise.advanced-permissions.common-test
(:require
[clojure.core.memoize :as memoize]
[clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
[metabase.api.database :as api.database]
[metabase.driver :as driver]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.models
:refer [Dashboard DashboardCard Database Field FieldValues Table]]
[metabase.models.database :as database]
......@@ -13,9 +11,9 @@
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
[metabase.permissions.test-util :as perms.test-util]
[metabase.sync :as sync]
[metabase.sync.concurrent :as sync.concurrent]
[metabase.test :as mt]
[metabase.test.data.sql :as sql.tx]
[metabase.test.fixtures :as fixtures]
[metabase.upload-test :as upload-test]
[metabase.util :as u]
......@@ -700,88 +698,62 @@
(deftest upload-csv-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads :schemas)
(testing "Uploads should be blocked without data access"
(mt/with-empty-db
(let [table-name (mt/random-name)]
(let [conn-spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
(doseq [stmt ["CREATE SCHEMA IF NOT EXISTS \"not_public\";"
(format "CREATE TABLE \"not_public\".\"%s\" (id INTEGER);" table-name)]]
(jdbc/execute! conn-spec stmt)))
(upload-test/sync-upload-test-table! :database (mt/db) :table-name table-name :schema-name "not_public")
(let [db-id (u/the-id (mt/db))
table-id (t2/select-one-pk :model/Table :db_id db-id :name table-name)
(let [schema-name (sql.tx/session-schema driver/*driver*)]
(upload-test/with-upload-table!
[table (upload-test/create-upload-table!)]
(let [db-id (mt/id)
upload-csv! (fn []
(upload-test/upload-example-csv! {:grant-permission? false
:schema-name "not_public"
:schema-name (:schema table)
:table-prefix "uploaded_magic_"}))]
(doseq [[schema-perms can-upload?] {:all true
:none false
{table-id :all} false}]
(with-all-users-data-perms! {db-id {:data {:native :none, :schemas {"public" :all
"not_public" schema-perms}}}}
(if can-upload?
(is (some? (upload-csv!)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"You don't have permissions to do that\."
(upload-csv!)))))
(with-all-users-data-perms! {db-id {:data {:native :write, :schemas ["not_public"]}}}
(doseq [[schema-perms can-upload? description]
[[:all true "Data permissions on schema should succeed"]
[:none false "No data permissions on schema should fail"]
[{(:id table) :all} false "Data permissions on table should fail"]]]
(testing description
(with-all-users-data-perms! {db-id {:data {:native :none, :schemas {"some_schema" :all
schema-name schema-perms}}}}
(if can-upload?
(is (some? (upload-csv!)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"You don't have permissions to do that\."
(upload-csv!))))))
(with-all-users-data-perms! {db-id {:data {:native :write, :schemas [schema-name]}}}
(is (some? (upload-csv!)))))))))))
(deftest append-csv-data-perms-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
(if (driver/database-supports? driver/*driver* :schemas (mt/db))
(testing "CSV appends should be blocked without data access to the schema"
(mt/with-empty-db
(jdbc/execute! (sql-jdbc.conn/db->pooled-connection-spec (mt/db)) "CREATE SCHEMA IF NOT EXISTS \"not_public\";")
(let [db-id (u/the-id (mt/db))
table-a (upload-test/create-upload-table! :schema-name "not_public")
table-b (upload-test/create-upload-table! :schema-name "not_public")
append-csv! #(upload-test/append-csv-with-defaults!
:table-id (:id table-a)
:user-id (mt/user->id :rasta))]
(doseq [[schema-perms can-append? test-string]
[[:all true "Data permissions on schema should succeed"]
[:none false "No permissions on schema should fail"]
[{(:id table-a) :all} true "Data permissions on table should succeed"]
[{(:id table-b) :all} false "Data permissions only on another table in the same schema should fail"]]]
(testing test-string
(with-all-users-data-perms! {db-id {:data {:native :none, :schemas {"public" :all
"not_public" schema-perms}}}}
(if can-append?
(is (some? (append-csv!)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"You don't have permissions to do that\."
(append-csv!))))))))))
;; else if the database doesn't support schemas
(testing "CSV appends should be blocked without data access to the database"
(mt/with-empty-db
(let [db-id (u/the-id (mt/db))
table-a (upload-test/create-upload-table!)
table-b (upload-test/create-upload-table!)
append-csv! #(upload-test/append-csv-with-defaults!
:table-id (:id table-a)
:user-id (mt/user->id :rasta))]
(doseq [[perms can-append? test-string]
[[:all true "Data permissions for database should succeed"]
[:none false "No permissions for database should fail"]
[{(:id table-a) :all} true "Data permissions for table should succeed"]
[{(:id table-b) :all} false "Data permissions only for another table in the same database should fail"]]]
(testing test-string
(with-all-users-data-perms! {db-id {:data {:native :none, :schemas {"" perms}}}}
(if can-append?
(is (some? (append-csv!)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"You don't have permissions to do that\."
(append-csv!)))))))))))))
(testing "CSV appends should be blocked without data access to the schema"
(let [schema-name (sql.tx/session-schema driver/*driver*)]
(upload-test/with-upload-table!
[table-a (upload-test/create-upload-table! :schema-name schema-name)]
(upload-test/with-upload-table!
[table-b (upload-test/create-upload-table! :schema-name schema-name)]
(let [db-id (u/the-id (mt/db))
append-csv! #(upload-test/append-csv-with-defaults!
:table-id (:id table-a)
:user-id (mt/user->id :rasta))]
(doseq [[schema-perms can-append? test-string]
[[:all true "Data permissions on schema should succeed"]
[:none false "No permissions on schema should fail"]
[{(:id table-a) :all} true "Data permissions on table should succeed"]
[{(:id table-b) :all} false "Data permissions only on another table in the same schema should fail"]]]
(testing test-string
(with-all-users-data-perms! {db-id {:data {:native :none, :schemas {(or schema-name "") schema-perms}}}}
(if can-append?
(is (some? (append-csv!)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"You don't have permissions to do that\."
(append-csv!))))))))))))))
(deftest append-csv-block-perms-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
(testing "Appends should be blocked if the user has blocked data access to the database, unless they have native query editing perms too"
(mt/with-empty-db
(upload-test/with-upload-table!
[table-a (upload-test/create-upload-table!)]
(let [db-id (u/the-id (mt/db))
table-a (upload-test/create-upload-table!)
append-csv! #(upload-test/append-csv-with-defaults!
:table-id (:id table-a)
:user-id (mt/user->id :rasta))]
......@@ -793,40 +765,36 @@
(if can-append?
(is (some? (append-csv!)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"You don't have permissions to do that\."
(append-csv!))))))))))))
clojure.lang.ExceptionInfo
#"You don't have permissions to do that\."
(append-csv!))))))))))))
(deftest get-database-can-upload-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads :schemas)
(testing "GET /api/database and GET /api/database/:id responses should include can_upload depending on unrestricted data access to the upload schema"
(mt/with-empty-db
(let [table-name (mt/random-name)]
(let [conn-spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
(doseq [stmt ["CREATE SCHEMA IF NOT EXISTS \"not_public\";"
(format "CREATE TABLE \"not_public\".\"%s\" (id INTEGER);" table-name)]]
(jdbc/execute! conn-spec stmt)))
(sync/sync-database! (mt/db))
(let [db-id (u/the-id (mt/db))
table-id (t2/select-one-pk :model/Table :db_id db-id)]
(mt/with-temporary-setting-values [uploads-enabled true
uploads-database-id db-id
uploads-schema-name "not_public"
uploads-table-prefix "uploaded_magic_"]
(doseq [[schema-perms can-upload?] {:all true
:none false
{table-id :all} false}]
(testing (format "can_upload should be %s if the user has %s access to the upload schema"
can-upload? schema-perms)
(with-all-users-data-perms! {db-id {:data {:native :none
:schemas {"public" :all
"not_public" schema-perms}}}}
(testing "GET /api/database"
(let [result (->> (mt/user-http-request :rasta :get 200 "database")
:data
(filter #(= (:id %) db-id))
first)]
(is (= can-upload? (:can_upload result)))))
(testing "GET /api/database/:id"
(let [result (mt/user-http-request :rasta :get 200 (format "database/%d" db-id))]
(is (= can-upload? (:can_upload result)))))))))))))))
(mt/with-model-cleanup [:model/Table]
(let [schema-name (sql.tx/session-schema driver/*driver*)]
(upload-test/with-upload-table!
[table (upload-test/create-upload-table! :schema-name schema-name)]
(let [db-id (u/the-id (mt/db))]
(mt/with-temporary-setting-values [uploads-enabled true
uploads-database-id db-id
uploads-schema-name schema-name
uploads-table-prefix "uploaded_magic_"]
(doseq [[schema-perms can-upload?] {:all true
:none false
{(:id table) :all} false}]
(testing (format "can_upload should be %s if the user has %s access to the upload schema"
can-upload? schema-perms)
(with-all-users-data-perms! {db-id {:data {:native :none
:schemas {"some_schema" :all
schema-name schema-perms}}}}
(testing "GET /api/database"
(let [result (->> (mt/user-http-request :rasta :get 200 "database")
:data
(filter #(= (:id %) db-id))
first)]
(is (= can-upload? (:can_upload result)))))
(testing "GET /api/database/:id"
(let [result (mt/user-http-request :rasta :get 200 (format "database/%d" db-id))]
(is (= can-upload? (:can_upload result))))))))))))))))
enterprise/backend/test/metabase_enterprise/upload_test.clj
+ 11
− 12
View file @ 38719125
......@@ -3,30 +3,29 @@
[clojure.test :refer :all]
[metabase-enterprise.test :as met]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.upload-test :as upload-test]))
(set! *warn-on-reflection* true)
(use-fixtures :once (fixtures/initialize :db :test-users))
(deftest uploads-disabled-for-sandboxed-user-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
(met/with-gtaps-for-user :rasta {:gtaps {:venues {}}}
(is (thrown-with-msg? Exception #"Uploads are not permitted for sandboxed users\."
(upload-test/upload-example-csv! {:grant-permission? false
:schema-name "not_public"
:table-prefix "uploaded_magic_"}))))))
(upload-test/upload-example-csv! {:grant-permission? false}))))))
(deftest appends-disabled-for-sandboxed-user-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
(mt/dataset (mt/dataset-definition
(mt/random-name)
["venues"
[{:field-name "name" :base-type :type/Text}]
[["something"]]])
(met/with-gtaps-for-user :rasta {:gtaps {:venues {}}}
(is (thrown-with-msg? Exception #"Uploads are not permitted for sandboxed users\."
(upload-test/append-csv-with-defaults! :user-id (mt/user->id :rasta))))))))
(met/with-gtaps-for-user :rasta {:gtaps {:venues {}}}
(is (thrown-with-msg? Exception #"Uploads are not permitted for sandboxed users\."
(upload-test/append-csv-with-defaults! :user-id (mt/user->id :rasta)))))))
(deftest based-on-upload-for-sandboxed-user-test
(mt/with-temporary-setting-values [uploads-enabled true]
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
;; FIXME: Redshift is flaking on `mt/dataset` and I don't know why, so I'm excluding it temporarily
(mt/test-drivers (disj (mt/normal-drivers-with-feature :uploads) :redshift)
(mt/dataset (mt/dataset-definition
(mt/random-name)
["venues"
......
modules/drivers/redshift/test/metabase/test/data/redshift.clj
+ 2
− 0
View file @ 38719125
......@@ -58,6 +58,8 @@
(defmethod sql.tx/pk-sql-type :redshift [_] "INTEGER IDENTITY(1,1)")
(defmethod sql.tx/session-schema :redshift [_driver] (unique-session-schema))
(defmethod sql.tx/qualified-name-components :redshift [& args]
(apply tx/single-db-qualified-name-components (unique-session-schema) args))
......
src/metabase/driver/postgres.clj
+ 2
− 2
View file @ 38719125
......@@ -63,6 +63,7 @@
:persist-models true
:table-privileges true
:schemas true
:uploads true
:connection-impersonation true}]
(defmethod driver/database-supports? [:postgres feature] [_driver _feature _db] supported?))
......@@ -73,8 +74,7 @@
;; Features that are supported by postgres only
(doseq [feature [:actions
:actions/custom
:index-info
:uploads]]
:index-info]]
(defmethod driver/database-supports? [:postgres feature]
[driver _feat _db]
(= driver :postgres)))
......
test/metabase/actions/test_util.clj
+ 3
− 1
View file @ 38719125
......@@ -111,7 +111,9 @@
(defmacro with-empty-db
"Sets the current dataset to a freshly created db that gets destroyed at the conclusion of `body`.
Use this to test destructive actions that may modify the data."
Use this to test destructive actions that may modify the data.
WARNING: this doesn't actually create and destroy a temporary database for cloud databases (like redshift) that
reuse a single database for all tests."
{:style/indent :defn}
[& body]
`(do-with-dataset-definition (tx/dataset-definition ~(str (gensym))) (fn [] ~@body)))
......
test/metabase/driver/sql_jdbc_test.clj
+ 26
− 23
View file @ 38719125
......@@ -239,26 +239,29 @@
(eduction
cat
[(orig metadata) [fake-schema-name]])))]
(is (= #{"public" fake-schema-name}
(driver/syncable-schemas driver/*driver* (mt/db))))
(let [driver (driver.u/database->driver (mt/db))
schema-filter-prop (find-schema-filters-prop driver)
filter-type-prop (keyword (str (:name schema-filter-prop) "-type"))
patterns-type-prop (keyword (str (:name schema-filter-prop) "-patterns"))]
(testing "syncable-schemas works as expected"
(testing " with an inclusion filter"
(t2.with-temp/with-temp [Database db-filtered {:engine driver
:details (-> (mt/db)
:details
(assoc filter-type-prop "inclusion"
patterns-type-prop "public"))}]
(is (= #{"public"}
(driver/syncable-schemas driver/*driver* db-filtered)))))
(testing " with an exclusion filter"
(t2.with-temp/with-temp [Database db-filtered {:engine driver
:details (-> (mt/db)
:details
(assoc filter-type-prop "exclusion"
patterns-type-prop "public"))}]
(is (= #{fake-schema-name}
(driver/syncable-schemas driver/*driver* db-filtered)))))))))))
(let [syncable (driver/syncable-schemas driver/*driver* (mt/db))]
(is (contains? syncable "public"))
(is (contains? syncable fake-schema-name))))
(let [driver (driver.u/database->driver (mt/db))
schema-filter-prop (find-schema-filters-prop driver)
filter-type-prop (keyword (str (:name schema-filter-prop) "-type"))
patterns-type-prop (keyword (str (:name schema-filter-prop) "-patterns"))]
(testing "syncable-schemas works as expected"
(testing "with an inclusion filter"
(t2.with-temp/with-temp [Database db-filtered {:engine driver
:details (-> (mt/db)
:details
(assoc filter-type-prop "inclusion"
patterns-type-prop "public"))}]
(let [syncable (driver/syncable-schemas driver/*driver* db-filtered)]
(is (contains? syncable "public"))
(is (not (contains? syncable fake-schema-name))))))
(testing "with an exclusion filter"
(t2.with-temp/with-temp [Database db-filtered {:engine driver
:details (-> (mt/db)
:details
(assoc filter-type-prop "exclusion"
patterns-type-prop "public"))}]
(let [syncable (driver/syncable-schemas driver/*driver* db-filtered)]
(is (not (contains? syncable "public")))
(is (not (contains? syncable fake-schema-name)))))))))))
test/metabase/test/data/h2.clj
+ 2
− 0
View file @ 38719125
......@@ -132,3 +132,5 @@
(defmethod sql.tx/standalone-table-comment-sql :h2
[& args]
(apply sql.tx/standard-standalone-table-comment-sql args))
(defmethod sql.tx/session-schema :h2 [_driver] "PUBLIC")
test/metabase/test/data/postgres.clj
+ 2
− 0
View file @ 38719125
......@@ -102,3 +102,5 @@
(defmethod sql.tx/standalone-table-comment-sql :postgres [& args]
(apply sql.tx/standard-standalone-table-comment-sql args))
(defmethod sql.tx/session-schema :postgres [_driver] "public")
test/metabase/test/data/sql.clj
+ 12
− 1
View file @ 38719125
......@@ -34,7 +34,6 @@
(defmethod pk-field-name :sql/test-extensions [_] "id")
;; TODO - WHAT ABOUT SCHEMA NAME???
(defmulti qualified-name-components
"Return a vector of String names that can be used to refer to a Database, Table, or Field. This is provided so drivers
......@@ -312,3 +311,15 @@
{:keys [query]} (qp/compile mbql-query)
query (str/replace query (re-pattern #"WHERE .* = .*") (format "WHERE {{%s}}" (name field)))]
{:query query})))
(defmulti session-schema
"Return the unquoted schema name for the current test session, if any. This can be used in test code that needs
to use the schema to create tables outside the regular test data setup. Test code that uses this should assume that
the schema is already created during initialization, and that the tables inside it will be cleaned up between test
runs in CI. Returns nil by default if there is no session schema, or the database doesn't support schemas.
For non-cloud drivers, this is typically the default schema that the driver uses when no schema is specified."
{:arglists '([driver])}
tx/dispatch-on-driver-with-test-extensions
:hierarchy #'driver/hierarchy)
(defmethod session-schema :sql/test-extensions [_] nil)
test/metabase/upload_test.clj
+ 138
− 128
View file @ 38719125
......@@ -19,6 +19,7 @@
[metabase.query-processor :as qp]
[metabase.sync.sync-metadata.tables :as sync-tables]
[metabase.test :as mt]
[metabase.test.data.sql :as sql.tx]
[metabase.upload :as upload]
[metabase.upload.parsing :as upload-parsing]
[metabase.util :as u]
......@@ -447,11 +448,7 @@
(let [db (t2/select-one :model/Database db-id)
schema-name (if (contains? args :schema-name)
(:schema-name args)
(when (driver/database-supports? driver/*driver* :schemas db)
;; make sure not_public schema is created, then use it by default
(let [spec (sql-jdbc.conn/connection-details->spec driver/*driver* (:details db))]
(jdbc/execute! spec "CREATE SCHEMA IF NOT EXISTS \"not_public\";")
"not_public")))
(sql.tx/session-schema driver/*driver*))
file (csv-file-with
["id, name"
"1, Luke Skywalker"
......@@ -483,14 +480,20 @@
(defmacro with-uploads-allowed [& body]
`(do-with-uploads-allowed (fn [] ~@body)))
(defn- do-with-upload-table! [table thunk]
(defn do-with-upload-table! [table thunk]
(try (thunk table)
(finally
(driver/drop-table! driver/*driver*
(mt/id)
(:db_id table)
(#'upload/table-identifier table)))))
(defmacro ^:private with-upload-table!
(defn- table->card [table]
(t2/select-one :model/Card :table_id (:id table)))
(defn- card->table [card]
(t2/select-one :model/Table (:table_id card)))
(defmacro with-upload-table!
"Execute `body` with a table created by evaluating the expression `create-table-expr`. `create-table-expr` must evaluate
to a toucan Table instance. The instance is bound to `table-sym` in `body`. The table is cleaned up from both the test
and app DB after the body executes.
......@@ -506,16 +509,17 @@
(deftest load-from-csv-table-name-test
(testing "Can upload two files with the same name"
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
(mt/with-empty-db
(mt/with-current-user (mt/user->id :crowberto)
(let [csv-file-prefix (mt/random-name)
model-1 (upload-example-csv! :csv-file-prefix csv-file-prefix)
model-2 (upload-example-csv! :csv-file-prefix csv-file-prefix)]
(testing "tables are different between the two uploads"
(is (some? (:table_id model-1)))
(is (some? (:table_id model-2)))
(is (not= (:table_id model-1)
(:table_id model-2))))))))))
(let [csv-file-prefix "some file prefix"]
(with-upload-table!
[table-1 (card->table (upload-example-csv! :csv-file-prefix csv-file-prefix))]
(with-upload-table!
[table-2 (card->table (upload-example-csv! :csv-file-prefix csv-file-prefix))]
(mt/with-current-user (mt/user->id :crowberto)
(testing "tables are different between the two uploads"
(is (some? (:id table-1)))
(is (some? (:id table-2)))
(is (not= (:id table-1)
(:id table-2)))))))))))
(defn- query-table
[table]
......@@ -1022,79 +1026,75 @@
(deftest create-csv-upload!-schema-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads :schemas)
(mt/with-empty-db
(let [db (mt/db)
db-id (u/the-id db)
original-sync-values (select-keys db [:is_on_demand :is_full_sync])
in-future? (atom false)
_ (t2/update! :model/Database db-id {:is_on_demand false
:is_full_sync false})]
(try
(with-redefs [ ;; do away with the `future` invocation since we don't want race conditions in a test
future-call (fn [thunk]
(swap! in-future? (constantly true))
(thunk))]
(testing "Happy path with schema, and without table-prefix"
;; create not_public schema in the db
(let [details (mt/dbdef->connection-details driver/*driver* :db {:database-name (:name (mt/db))})]
(jdbc/execute! (sql-jdbc.conn/connection-details->spec driver/*driver* details)
["CREATE SCHEMA IF NOT EXISTS \"not_public\";"]))
(let [new-model (upload-example-csv! :schema-name "not_public" :sync-synchronously? false)
new-table (t2/select-one :model/Table (:table_id new-model))]
(is (=? {:display :table
:database_id db-id
:dataset_query {:database db-id
:query {:source-table (:id new-table)}
:type :query}
:creator_id (mt/user->id :rasta)
:name #"(?i)example csv file(.*)"
:collection_id nil}
new-model)
"A new model is created")
(is (=? {:name #"(?i)example(.*)"
:schema #"(?i)not_public"
:is_upload true}
new-table)
"A new table is created")
(is (= "complete"
(:initial_sync_status new-table))
"The table is synced and marked as complete")
(is (= #{["_mb_row_id" :type/PK]
["id" :type/PK]
["name" :type/Name]}
(->> (t2/select Field :table_id (:id new-table))
(map (fn [field] [(u/lower-case-en (:name field))
(:semantic_type field)]))
set))
"The sync actually runs")
(is (true? @in-future?)
"Table has been synced in a separate thread"))))
(finally
(t2/update! :model/Database db-id original-sync-values)))))))
(let [db (mt/db)
db-id (u/the-id db)
original-sync-values (select-keys db [:is_on_demand :is_full_sync])
in-future? (atom false)
schema-name (or (sql.tx/session-schema driver/*driver*) "public")
_ (t2/update! :model/Database db-id {:is_on_demand false
:is_full_sync false})]
(try
(with-redefs [ ;; do away with the `future` invocation since we don't want race conditions in a test
future-call (fn [thunk]
(swap! in-future? (constantly true))
(thunk))]
(testing "Happy path with schema, and without table-prefix"
;; make sure the schema exists
(let [sql (format "CREATE SCHEMA IF NOT EXISTS %s;" (sql.tx/qualify-and-quote driver/*driver* schema-name))]
(jdbc/execute! (sql-jdbc.conn/db->pooled-connection-spec db) sql))
(with-upload-table!
[new-table (card->table (upload-example-csv! :schema-name schema-name :sync-synchronously? false))]
(is (=? {:display :table
:database_id db-id
:dataset_query {:database db-id
:query {:source-table (:id new-table)}
:type :query}
:creator_id (mt/user->id :rasta)
:name #"(?i)example csv file(.*)"
:collection_id nil}
(t2/select-one :model/Card :table_id (:id new-table)))
"A new model is created")
(is (=? {:name #"(?i)example(.*)"
:schema (re-pattern (str "(?i)" schema-name))
:is_upload true}
new-table)
"A new table is created")
(is (= "complete"
(:initial_sync_status new-table))
"The table is synced and marked as complete")
(is (= #{["_mb_row_id" :type/PK]
["id" :type/PK]
["name" :type/Name]}
(->> (t2/select Field :table_id (:id new-table))
(map (fn [field] [(u/lower-case-en (:name field))
(:semantic_type field)]))
set))
"The sync actually runs")
(is (true? @in-future?)
"Table has been synced in a separate thread"))))
(finally
(t2/update! :model/Database db-id original-sync-values))))))
(deftest create-csv-upload!-table-prefix-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
(mt/with-empty-db
(testing "Happy path with table prefix, and without schema"
(if (driver/database-supports? driver/*driver* :schemas (mt/db))
(is (thrown-with-msg?
java.lang.Exception
#"^A schema has not been set."
(upload-example-csv! :table-prefix "uploaded_magic_" :schema-name nil)))
(let [new-model (upload-example-csv! :table-prefix "uploaded_magic_")
new-table (t2/select-one :model/Table (:table_id new-model))]
(is (=? {:name #"(?i)example csv file(.*)"}
new-model))
(is (=? {:name #"(?i)uploaded_magic_example(.*)"}
new-table))
(is (nil? (:schema new-table)))))))))
(testing "Happy path with table prefix, and without schema"
(if (driver/database-supports? driver/*driver* :schemas (mt/db))
(is (thrown-with-msg?
java.lang.Exception
#"^A schema has not been set."
(upload-example-csv! :table-prefix "uploaded_magic_" :schema-name nil)))
(with-upload-table! [table (card->table (upload-example-csv! :table-prefix "uploaded_magic_"))]
(is (=? {:name #"(?i)example csv file(.*)"}
(table->card table)))
(is (=? {:name #"(?i)uploaded_magic_example(.*)"}
table))
(is (nil? (:schema table))))))))
(deftest create-csv-upload!-auto-pk-column-display-name-test
(testing "The auto-generated column display_name should be the same as its name"
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
(mt/with-empty-db
(let [new-model (upload-example-csv!)
new-field (t2/select-one Field :table_id (:table_id new-model) :name "_mb_row_id")]
(with-upload-table! [table (card->table (upload-example-csv!))]
(let [new-field (t2/select-one Field :table_id (:id table) :name "_mb_row_id")]
(is (= "_mb_row_id"
(:name new-field)
(:display_name new-field))))))))
......@@ -1102,9 +1102,9 @@
(deftest csv-upload-snowplow-test
;; Just test with h2 because snowplow should be independent of the driver
(mt/test-driver :h2
(mt/with-empty-db
(snowplow-test/with-fake-snowplow-collector
(upload-example-csv!)
(snowplow-test/with-fake-snowplow-collector
(with-upload-table!
[_table (card->table (upload-example-csv!))]
(is (=? {:data {"model_id" pos?
"size_mb" 3.910064697265625E-5
"num_columns" 2
......@@ -1195,15 +1195,14 @@
Defaults to a table with an auto-incrementing integer ID column, and a name column."
[& {:keys [schema-name table-name col->upload-type rows]
:or {table-name (mt/random-name)
schema-name (sql.tx/session-schema driver/*driver*)
col->upload-type (ordered-map/ordered-map
upload/auto-pk-column-keyword ::upload/auto-incrementing-int-pk
:name ::upload/varchar-255)
rows [["Obi-Wan Kenobi"]]}}]
(let [driver driver/*driver*
db-id (mt/id)
schema+table-name (if schema-name
(str schema-name "." table-name)
table-name)
schema+table-name (#'upload/table-identifier {:schema schema-name :name table-name})
insert-col-names (remove #{upload/auto-pk-column-keyword} (keys col->upload-type))
col-definitions (#'upload/column-definitions driver col->upload-type)
_ (driver/create-table! driver/*driver*
......@@ -1216,8 +1215,14 @@
_ (driver/insert-into! driver db-id schema+table-name insert-col-names rows)]
(sync-upload-test-table! :database (mt/db) :table-name table-name :schema-name schema-name)))
(defmacro maybe-apply-macro
[flag macro-fn & body]
`(if ~flag
(~macro-fn ~@body)
~@body))
(defn append-csv-with-defaults!
"Upload a small CSV file to a newly created default table, or an existing table if `table-id` is provided. Default args can be overridden"
"Upload a small CSV file to a newly created default table, or an existing table if `table-id` is provided. Default args can be overridden."
[& {:keys [uploads-enabled user-id file table-id is-upload]
:or {uploads-enabled true
user-id (mt/user->id :crowberto)
......@@ -1227,15 +1232,21 @@
"Darth Vader"]
(mt/random-name))
is-upload true}}]
(try
(mt/with-temporary-setting-values [uploads-enabled uploads-enabled]
(mt/with-current-user user-id
(let [table-id (or table-id (:id (create-upload-table!)))]
(mt/with-temporary-setting-values [uploads-enabled uploads-enabled]
(mt/with-current-user user-id
(mt/with-model-cleanup [:model/Table]
(let [new-table (when (nil? table-id)
(create-upload-table!))
table-id (or table-id (:id new-table))]
(t2/update! :model/Table table-id {:is_upload is-upload})
(append-csv! {:table-id table-id
:file file}))))
(finally
(io/delete-file file))))
(try (append-csv! {:table-id table-id
:file file})
(finally
;; Drop the table in the testdb if a new one was created.
(when new-table
(driver/drop-table! driver/*driver*
(mt/id)
(#'upload/table-identifier new-table))))))))))
(defn catch-ex-info* [f]
(try
......@@ -1249,36 +1260,35 @@
(deftest can-append-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
(mt/with-empty-db
(testing "Happy path"
(is (= {:row-count 2}
(append-csv-with-defaults!))))
(testing "Even if the uploads database, schema and table prefix is not set, appends succeed"
(mt/with-temporary-setting-values [uploads-database-id nil
uploads-schema-name nil
uploads-table-prefix nil]
(is (some? (append-csv-with-defaults!)))))
(testing "Uploads must be enabled"
(is (= {:message "Uploads are not enabled."
:data {:status-code 422}}
(catch-ex-info (append-csv-with-defaults! :uploads-enabled false)))))
(testing "The table must exist"
(is (= {:message "Not found."
:data {:status-code 404}}
(catch-ex-info (append-csv-with-defaults! :table-id Integer/MAX_VALUE)))))
(testing "The table must be an uploaded table"
(is (= {:message "The table must be an uploaded table."
:data {:status-code 422}}
(catch-ex-info (append-csv-with-defaults! :is-upload false)))))
(testing "The CSV file must not be empty"
(is (= {:message "The CSV file is missing columns that are in the table: \"name\".",
(testing "Happy path"
(is (= {:row-count 2}
(append-csv-with-defaults!))))
(testing "Even if the uploads database, schema and table prefix are not set, appends succeed"
(mt/with-temporary-setting-values [uploads-database-id nil
uploads-schema-name nil
uploads-table-prefix nil]
(is (some? (append-csv-with-defaults!)))))
(testing "Uploads must be enabled"
(is (= {:message "Uploads are not enabled."
:data {:status-code 422}}
(catch-ex-info (append-csv-with-defaults! :uploads-enabled false)))))
(testing "The table must exist"
(is (= {:message "Not found."
:data {:status-code 404}}
(catch-ex-info (append-csv-with-defaults! :table-id Integer/MAX_VALUE)))))
(testing "The table must be an uploaded table"
(is (= {:message "The table must be an uploaded table."
:data {:status-code 422}}
(catch-ex-info (append-csv-with-defaults! :is-upload false)))))
(testing "The CSV file must not be empty"
(is (= {:message "The CSV file is missing columns that are in the table: \"name\".",
:data {:status-code 422}}
(catch-ex-info (append-csv-with-defaults! :file (csv-file-with [] (mt/random-name)))))))
(testing "Uploads must be supported"
(with-redefs [driver/database-supports? (constantly false)]
(is (= {:message (format "Uploads are not supported on %s databases." (str/capitalize (name driver/*driver*)))
:data {:status-code 422}}
(catch-ex-info (append-csv-with-defaults! :file (csv-file-with [] (mt/random-name)))))))
(testing "Uploads must be supported"
(with-redefs [driver/database-supports? (constantly false)]
(is (= {:message (format "Uploads are not supported on %s databases." (str/capitalize (name driver/*driver*)))
:data {:status-code 422}}
(catch-ex-info (append-csv-with-defaults!)))))))))
(catch-ex-info (append-csv-with-defaults!))))))))
(deftest append-column-match-test
(mt/test-drivers (mt/normal-drivers-with-feature :uploads)
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment