diff --git a/docs/developers-guide/driver-changelog.md b/docs/developers-guide/driver-changelog.md index 2ca7afb898e0869ef1a7ddc1d2e6b102a78f51cb..704bd7040dad990359cb08c427fcbde9e782d825 100644 --- a/docs/developers-guide/driver-changelog.md +++ b/docs/developers-guide/driver-changelog.md @@ -151,7 +151,7 @@ title: Driver interface changelog - A new driver feature has been added: `:schemas`. This feature signals whether the database organizes tables in schemas (also known as namespaces) or not. Most databases have schemas so this feature is on by default. - An implemention of the multimethod `metabase.driver/database-supports?` for `:schemas` is required only if the + An implementation of the multimethod `metabase.driver/database-supports?` for `:schemas` is required only if the database doesn't store tables in schemas. - Another driver feature has been added: `:uploads`. The `:uploads` feature signals whether the database supports diff --git a/test/metabase/upload_test.clj b/test/metabase/upload_test.clj index b81454071f154c044c2371db4537d57dfb3e9dc2..2f65fe4e224f661d5b5c11bcfcbceb5c6bd8aac9 100644 --- a/test/metabase/upload_test.clj +++ b/test/metabase/upload_test.clj @@ -10,6 +10,7 @@ [java-time.api :as t] [metabase.analytics.snowplow-test :as snowplow-test] [metabase.driver :as driver] + [metabase.driver.ddl.interface :as ddl.i] [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] [metabase.driver.util :as driver.u] [metabase.models :refer [Field]] @@ -99,6 +100,10 @@ [& body] `(do-with-mysql-local-infile-off (fn [] ~@body))) +(defn- format-schema [schema-name] + (when (driver/database-supports? driver/*driver* :schemas nil) + (ddl.i/format-name driver/*driver* schema-name))) + (defn sync-upload-test-table! "Creates a table in the app db and syncs it synchronously, setting is_upload=true. Returns the table instance. The result is identical to if the table was synced with [[metabase.sync/sync-database!]], but faster because it skips @@ -475,7 +480,7 @@ (mt/with-current-user user-id (let [db (t2/select-one :model/Database db-id) schema-name (if (contains? args :schema-name) - (:schema-name args) + (format-schema (:schema-name args)) (sql.tx/session-schema driver/*driver*)) file (csv-file-with ["id, name" @@ -654,8 +659,8 @@ (mt/test-drivers (mt/normal-drivers-with-feature :uploads) (with-mysql-local-infile-on-and-off (mt/with-dynamic-redefs [driver/db-default-timezone (constantly "Z") - upload/current-database (constantly (mt/db))] - (let [table-name (mt/random-name) + upload/current-database (constantly (mt/db))] + (let [table-name (mt/random-name) datetime-pairs [["2022-01-01T12:00:00-07" "2022-01-01T19:00:00Z"] ["2022-01-01T12:00:00-07:00" "2022-01-01T19:00:00Z"] ["2022-01-01T12:00:00-07:30" "2022-01-01T19:30:00Z"] @@ -1153,9 +1158,8 @@ (:name new-field) (:display_name new-field)))))))) -(deftest csv-upload-snowplow-test - ;; Just test with h2 because snowplow should be independent of the driver - (mt/test-driver :h2 +(deftest ^:mb/once csv-upload-snowplow-test + (mt/test-drivers (mt/normal-drivers-with-feature :uploads) (snowplow-test/with-fake-snowplow-collector (with-upload-table! [_table (card->table (upload-example-csv!))] (testing "Successfully creating a CSV Upload publishes statistics to Snowplow" @@ -1182,9 +1186,8 @@ :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 +(deftest ^:mb/once csv-upload-audit-log-test + (mt/test-drivers (mt/normal-drivers-with-feature :uploads) (mt/with-premium-features #{:audit-app} (with-upload-table! [_table (card->table (upload-example-csv!))] @@ -1193,7 +1196,7 @@ :model "Table" :model_id pos? :details {:db-id pos? - :schema-name "PUBLIC" + :schema-name (format-schema "public") :table-name string? :model-id pos? :stats {:num-rows 2 @@ -1203,9 +1206,8 @@ :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 +(deftest ^:mb/once create-csv-upload!-failure-test + (mt/test-drivers (mt/normal-drivers-with-feature :uploads) (mt/with-empty-db (testing "Uploads must be enabled" (doseq [uploads-enabled-value [false nil]] @@ -1222,7 +1224,7 @@ (mt/with-dynamic-redefs [driver/database-supports? (constantly false)] (is (thrown-with-msg? java.lang.Exception - #"^Uploads are not supported on Postgres databases\." + #"^Uploads are not supported on \w+ databases\." (upload-example-csv! :schema-name "public", :table-prefix "uploaded_magic_"))))) (testing "User must have write permissions on the collection" (mt/with-non-admin-groups-no-root-collection-perms @@ -1236,8 +1238,8 @@ (= :schema-filters (keyword (:type conn-prop)))) (driver/connection-properties driver)))) -(deftest create-csv-upload!-schema-does-not-sync-test - ;; Just test with postgres because failure should be independent of the driver +(deftest ^:mb/once create-csv-upload!-schema-does-not-sync-test + ;; We only need to test this for a single driver, and the way this test has been written is coupled to Postgres (mt/test-driver :postgres (mt/with-empty-db (let [driver (driver.u/database->driver (mt/db)) @@ -1250,6 +1252,7 @@ patterns-type-prop "public"))}) (testing "Upload should fail if table can't be found after sync, for example because of schema filters" (try (upload-example-csv! {:schema-name "public"}) + (is (false? :should-not-be-reached)) (catch Exception e (is (= {:status-code 422} (ex-data e))) @@ -1259,7 +1262,7 @@ (is (false? (let [details (mt/dbdef->connection-details driver/*driver* :db {:database-name (:name (mt/db))})] (-> (jdbc/query (sql-jdbc.conn/connection-details->spec driver/*driver* details) ["SELECT EXISTS (SELECT 1 FROM information_schema.tables WHERE table_schema = 'public')"]) - first :exists))))))))) + first vals first))))))))) ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | append-csv! | @@ -1578,9 +1581,8 @@ (rows-for-table table)))) (io/delete-file file)))))) -(deftest csv-append-snowplow-test - ;; Just test with h2 because snowplow should be independent of the driver - (mt/test-driver :h2 +(deftest ^:mb/once csv-append-snowplow-test + (mt/test-drivers (mt/normal-drivers-with-feature :uploads) (snowplow-test/with-fake-snowplow-collector (with-upload-table! [table (create-upload-table!)] @@ -1618,9 +1620,8 @@ :user-id (str (mt/user->id :crowberto))} (last (snowplow-test/pop-event-data-and-user-id!)))))))))) -(deftest csv-append-audit-log-test - ;; Just test with h2 because these events are independent of the driver - (mt/test-driver :h2 +(deftest ^:mb/once csv-append-audit-log-test + (mt/test-drivers (mt/normal-drivers-with-feature :uploads) (mt/with-premium-features #{:audit-app} (with-upload-table! [table (create-upload-table!)] (let [csv-rows ["name" "Luke Skywalker"] @@ -1632,7 +1633,7 @@ :model "Table" :model_id (:id table) :details {:db-id pos? - :schema-name "PUBLIC" + :schema-name (format-schema "public") :table-name string? :stats {:num-rows 1 :num-columns 1