diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index 2d0df2257c532b6742011afc756ddac0f23d55f1..8d087a329cd6f6e19ceec5e15525e5b9f7deee71 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -143,6 +143,9 @@ toucan.db/select-field->field {:message "Use t2/select-fn->fn instead of toucan.db/select-field->field"} toucan.db/select-field->id {:message "Use t2/select-fn->pk instead of toucan.db/select-field->id"} toucan.db/select-id->field {:message "Use t2/select-pk->field instead of toucan.db/select-id->field"} + toucan.db/simple-insert! {:message "Use t2/insert-returning-pks! with table name instead of toucan.db/simple-insert!"} + toucan.db/simple-insert-many! {:message "Use t2/insert-returning-pks! with table name instead of toucan.db/simple-insert-many!"} + toucan.db/simple-delete! {:message "Use t2/delete! with table name instead of toucan.db/simple-delete!"} toucan.db/delete! {:message "Use t2/delete! instead of toucan.db/delete!"} toucan.db/insert! {:message "Use t2/insert-returning-instances! instead of toucan.db/insert!"} toucan.db/update! {:message "Use t2/update! instead of toucan.db/update!"} @@ -154,7 +157,6 @@ toucan.db/execute! {:message "Use t2/query-one instead of toucan.db/execute!"} toucan.db/reducible-query {:message "Use mdb.query/reducible-query instead of toucan.db/reducible-query"}} - :discouraged-namespace {clojure.tools.logging {:message "Use metabase.util.log instead of clojure.tools.logging directly"} metabase.util.jvm {:message "All of metabase.util.jvm is re-exported from metabase.util; prefer that"}} diff --git a/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj index c5778e0b6a115950c443e9778cfc16d053438efd..221e3c81b83990f880ba6386bcfd888607f8faa1 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj @@ -11,7 +11,6 @@ [metabase.util :as u] [metabase.util.log :as log] [metabase.util.yaml :as yaml] - [toucan.db :as db] [toucan2.core :as t2]) (:import (java.util UUID))) @@ -30,7 +29,7 @@ ;; already does what we need) (mt/with-empty-h2-app-db ;; create a single dummy User to own a Card and a Database for it to reference - (let [user (db/simple-insert! User + (let [user (t2/insert! (t2/table-name User) :email "nobody@nowhere.com" :first_name (mt/random-name) :last_name (mt/random-name) @@ -38,14 +37,14 @@ :date_joined :%now :is_active true :is_superuser true) - db (db/simple-insert! Database + db (t2/insert! (t2/table-name Database) :name "Test Database" :engine "h2" :details "{}" :created_at :%now :updated_at :%now)] ;; then the card itself - (db/simple-insert! Card + (t2/insert! (t2/table-name Card) :name "Single Card" :display "Single Card" :database_id (u/the-id db) diff --git a/src/metabase/api/metric.clj b/src/metabase/api/metric.clj index c06a5cc5e329d76da633915521123694bc04babb..5d91c15a1880f7d8c63b175671fd1475f476429d 100644 --- a/src/metabase/api/metric.clj +++ b/src/metabase/api/metric.clj @@ -7,10 +7,9 @@ [metabase.api.query-description :as api.qd] [metabase.events :as events] [metabase.mbql.normalize :as mbql.normalize] + [metabase.models :refer [Metric MetricImportantField Table]] [metabase.models.interface :as mi] - [metabase.models.metric :as metric :refer [Metric]] [metabase.models.revision :as revision] - [metabase.models.table :refer [Table]] [metabase.related :as related] [metabase.util :as u] [metabase.util.i18n :refer [trs]] @@ -131,7 +130,7 @@ ;; delete old fields as needed (when (seq fields-to-remove) - (db/simple-delete! 'MetricImportantField {:metric_id id, :field_id [:in fields-to-remove]})) + (t2/delete! (t2/table-name MetricImportantField) {:metric_id id, :field_id [:in fields-to-remove]})) ;; add new fields as needed (db/insert-many! 'MetricImportantField (for [field-id fields-to-add] {:metric_id id, :field_id field-id})) diff --git a/src/metabase/api/table.clj b/src/metabase/api/table.clj index 736cefc43dc44b8517d66e063b48d0e876dd9177..ec2b197a53743a218eb7a3d2ea652d313d151496 100644 --- a/src/metabase/api/table.clj +++ b/src/metabase/api/table.clj @@ -24,7 +24,6 @@ [metabase.util.malli.schema :as ms] [metabase.util.schema :as su] [schema.core :as s] - [toucan.db :as db] [toucan.hydrate :refer [hydrate]] [toucan2.core :as t2])) @@ -481,7 +480,7 @@ {id ms/PositiveInt} (api/write-check (t2/select-one Table :id id)) (when-let [field-ids (t2/select-pks-set Field :table_id id)] - (db/simple-delete! FieldValues :field_id [:in field-ids])) + (t2/delete! (t2/table-name FieldValues) :field_id [:in field-ids])) {:status :success}) (api/defendpoint GET "/:id/related" diff --git a/src/metabase/models/pulse_channel.clj b/src/metabase/models/pulse_channel.clj index 6aa95907ff4cbd6e69314783430f374dd104f0d1..6a0712a82b40fade6c678222accca57f50724c6d 100644 --- a/src/metabase/models/pulse_channel.clj +++ b/src/metabase/models/pulse_channel.clj @@ -281,7 +281,7 @@ (let [vs (map #(assoc {:pulse_channel_id id} :user_id %) recipients+)] (db/insert-many! PulseChannelRecipient vs))) (when (seq recipients-) - (db/simple-delete! PulseChannelRecipient + (t2/delete! (t2/table-name PulseChannelRecipient) :pulse_channel_id id :user_id [:in recipients-])))) diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj index fd0c0845598467719f721e595df9ca264062a773..9c4d79eaee6bb4dd1288257089f13c85b8be36b4 100644 --- a/src/metabase/models/setting.clj +++ b/src/metabase/models/setting.clj @@ -90,7 +90,6 @@ [metabase.util.i18n :refer [deferred-trs deferred-tru trs tru]] [metabase.util.log :as log] [schema.core :as s] - [toucan.db :as db] [toucan.models :as models] [toucan2.core :as t2]) (:import @@ -639,7 +638,7 @@ ;; write to DB (cond (nil? new-value) - (db/simple-delete! Setting :key setting-name) + (t2/delete! (t2/table-name Setting) :key setting-name) ;; if there's a value in the cache then the row already exists in the DB; update that (contains? (setting.cache/cache) setting-name) diff --git a/src/metabase/models/setting/cache.clj b/src/metabase/models/setting/cache.clj index 85416cf86ffbf1198d0d9b52932bd1f41cef9700..e3f96d453e800ea79a8b4d31e536253087db9704 100644 --- a/src/metabase/models/setting/cache.clj +++ b/src/metabase/models/setting/cache.clj @@ -84,7 +84,7 @@ ;; and ignore that Exception if one is thrown. (try ;; Use `simple-insert!` because we do *not* want to trigger pre-insert behavior, such as encrypting `:value` - (db/simple-insert! 'Setting :key settings-last-updated-key, :value current-timestamp-as-string-honeysql) + (t2/insert! (t2/table-name (t2/resolve-model 'Setting)) :key settings-last-updated-key, :value current-timestamp-as-string-honeysql) (catch java.sql.SQLException e ;; go ahead and log the Exception anyway on the off chance that it *wasn't* just a race condition issue (log/error (trs "Error updating Settings last updated value: {0}" diff --git a/src/metabase/models/task_history.clj b/src/metabase/models/task_history.clj index 39fb66e177f6d253d27295b10a41a8f3a3ee1214..3f23978a0dd5b21f5742c70c6e6216d8203be9ce 100644 --- a/src/metabase/models/task_history.clj +++ b/src/metabase/models/task_history.clj @@ -15,7 +15,6 @@ [metabase.util.log :as log] [metabase.util.schema :as su] [schema.core :as s] - [toucan.db :as db] [toucan.models :as models] [toucan2.core :as t2])) @@ -37,9 +36,9 @@ ;; ensures we'll have a good amount of history for debugging/troubleshooting, but not grow too large and fill the ;; disk. (when-let [clean-before-date (t2/select-one-fn :ended_at TaskHistory {:limit 1 - :offset num-rows-to-keep - :order-by [[:ended_at :desc]]})] - (db/simple-delete! TaskHistory :ended_at [:<= clean-before-date]))) + :offset num-rows-to-keep + :order-by [[:ended_at :desc]]})] + (t2/delete! (t2/table-name TaskHistory) :ended_at [:<= clean-before-date]))) ;;; Permissions to read or write Task. If `advanced-permissions` is enabled it requires superusers or non-admins with ;;; monitoring permissions, Otherwise it requires superusers. diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj index d24924fffd004508ca1b0e52fdd7dc20e202833b..46d2a2c10b78f2d1bee515ca8b69a4cbb63be6fb 100644 --- a/src/metabase/models/user.clj +++ b/src/metabase/models/user.clj @@ -22,7 +22,6 @@ [metabase.util.password :as u.password] [metabase.util.schema :as su] [schema.core :as schema] - [toucan.db :as db] [toucan.models :as models] [toucan2.core :as t2]) (:import @@ -105,7 +104,7 @@ ;; stack overflow of calls between the two. TODO - could we fix this issue by using a `post-delete` method? (and (not superuser?) in-admin-group?) - (db/simple-delete! PermissionsGroupMembership + (t2/delete! (t2/table-name PermissionsGroupMembership) :group_id (u/the-id (perms-group/admin)) :user_id id)))) ;; make sure email and locale are valid if set @@ -356,7 +355,7 @@ by [[pre-insert]] or [[pre-update]])" [user-id password] ;; when changing/resetting the password, kill any existing sessions - (db/simple-delete! Session :user_id user-id) + (t2/delete! (t2/table-name Session) :user_id user-id) ;; NOTE: any password change expires the password reset token (t2/update! User user-id {:password password diff --git a/src/metabase/query_processor/middleware/cache_backend/db.clj b/src/metabase/query_processor/middleware/cache_backend/db.clj index d8fe0bbc6a2b73c9d9327064bae92c3ec03bcf25..a005b7b034145aa245aeed1203b049ac1553d668 100644 --- a/src/metabase/query_processor/middleware/cache_backend/db.clj +++ b/src/metabase/query_processor/middleware/cache_backend/db.clj @@ -76,8 +76,8 @@ {:pre [(number? max-age-seconds)]} (log/tracef "Purging old cache entries.") (try - (db/simple-delete! QueryCache - :updated_at [:<= (seconds-ago max-age-seconds)]) + (t2/delete! (t2/table-name QueryCache) + :updated_at [:<= (seconds-ago max-age-seconds)]) (catch Throwable e (log/error e (trs "Error purging old cache entries")))) nil) diff --git a/test/metabase/api/revision_test.clj b/test/metabase/api/revision_test.clj index e8deff718ec7dfcc28a216be4da4661e1234a1fd..c5a9164d2b34c6f3294dde426356aec604927dfd 100644 --- a/test/metabase/api/revision_test.clj +++ b/test/metabase/api/revision_test.clj @@ -10,7 +10,6 @@ [metabase.test.data.users :as test.users] [metabase.test.fixtures :as fixtures] [metabase.util :as u] - [toucan.db :as db] [toucan.util.test :as tt] [toucan2.core :as t2])) @@ -128,7 +127,7 @@ :col 0))] (is (=? {:id id} (create-dashboard-revision! dash false :rasta))) - (is (true? (db/simple-delete! DashboardCard, :id (:id dashcard))))) + (is (pos? (t2/delete! (t2/table-name DashboardCard) :id (:id dashcard))))) (is (=? {:id id} (create-dashboard-revision! dash false :rasta))) (testing "Revert to the previous revision, allowed because rasta has permissions on parent collection" diff --git a/test/metabase/db/schema_migrations_test.clj b/test/metabase/db/schema_migrations_test.clj index ff28354e1bef0ab59d26a25790a05c109ab064a4..7b42749c07967343aa259e047c686986857630ac 100644 --- a/test/metabase/db/schema_migrations_test.clj +++ b/test/metabase/db/schema_migrations_test.clj @@ -39,7 +39,6 @@ [metabase.test.fixtures :as fixtures] [metabase.test.util.random :as tu.random] [metabase.util :as u] - [toucan.db :as db] [toucan2.core :as t2] [toucan2.execute :as t2.execute]) (:import @@ -75,11 +74,11 @@ (testing "Migration 165: add `database_position` to Field" (impl/test-migrations 165 [migrate!] ;; create a Database with a Table with 2 Fields - (db/simple-insert! Database {:name "DB", :engine "h2", :created_at :%now, :updated_at :%now}) - (db/simple-insert! Table {:name "Table", :db_id 1, :created_at :%now, :updated_at :%now, :active true}) + (t2/insert! (t2/table-name Database) {:name "DB", :engine "h2", :created_at :%now, :updated_at :%now}) + (t2/insert! (t2/table-name Table) {:name "Table", :db_id 1, :created_at :%now, :updated_at :%now, :active true}) (let [mock-field {:table_id 1, :created_at :%now, :updated_at :%now, :base_type "type/Text", :database_type "VARCHAR"}] - (db/simple-insert! Field (assoc mock-field :name "Field 1")) - (db/simple-insert! Field (assoc mock-field :name "Field 2"))) + (t2/insert! (t2/table-name Field) (assoc mock-field :name "Field 1")) + (t2/insert! (t2/table-name Field) (assoc mock-field :name "Field 2"))) (testing "sanity check: Fields should not have a `:database_position` column yet" (is (not (contains? (t2/select-one Field :id 1) :database_position)))) ;; now run migration 165 @@ -93,14 +92,14 @@ (defn- create-raw-user! "create a user but skip pre and post insert steps" [email] - (db/simple-insert! User - :email email - :first_name (tu.random/random-name) - :last_name (tu.random/random-name) - :password (str (UUID/randomUUID)) - :date_joined :%now - :is_active true - :is_superuser false)) + (first (t2/insert-returning-instances! (t2/table-name User) + :email email + :first_name (tu.random/random-name) + :last_name (tu.random/random-name) + :password (str (UUID/randomUUID)) + :date_joined :%now + :is_active true + :is_superuser false))) (deftest email-lowercasing-test (testing "Migration 268-272: basic lowercasing `email` in `core_user`" @@ -120,11 +119,11 @@ (impl/test-migrations [283 296] [migrate!] ;; by name hoists results into a map by name so diffs are easier to read than sets. (let [by-name #(into {} (map (juxt :name identity)) %) - db-id (db/simple-insert! Database {:name "DB", :engine "h2", :created_at :%now, :updated_at :%now}) - table-id (db/simple-insert! Table {:name "Table", :db_id db-id, :created_at :%now, :updated_at :%now, :active true})] - ;; Using [[db/simple-insert-many!]] instead of [[db/insert-many!]] so we don't run into the field type validation + db-id (first (t2/insert-returning-pks! (t2/table-name Database) {:name "DB", :engine "h2", :created_at :%now, :updated_at :%now})) + table-id (first (t2/insert-returning-pks! (t2/table-name Table) {:name "Table", :db_id db-id, :created_at :%now, :updated_at :%now, :active true}))] + ;; Using insert with table-name, so we don't run into the field type validation ;; added in 38 - (db/simple-insert-many! Field + (t2/insert! (t2/table-name Field) (for [field [{:base_type :type/Text :semantic_type :type/Address :name "address" @@ -349,16 +348,16 @@ ;; we're using `simple-insert!` here instead of `with-temp` because Toucan `post-insert` for the DB will ;; normally try to add it to the magic Permissions Groups which don't exist yet. They're added in later ;; migrations - (let [db (db/simple-insert! Database {:name "Legacy BigQuery driver DB" - :engine "bigquery" - :details (json/generate-string {:service-account-json "{\"fake_key\": 14}"}) - :created_at :%now - :updated_at :%now})] + (let [db-id (first (t2/insert-returning-pks! (t2/table-name Database) {:name "Legacy BigQuery driver DB" + :engine "bigquery" + :details (json/generate-string {:service-account-json "{\"fake_key\": 14}"}) + :created_at :%now + :updated_at :%now}))] (migrate!) (is (= [{:engine "bigquery-cloud-sdk"}] - (mdb.query/query {:select [:engine], :from [:metabase_database], :where [:= :id (u/the-id db)]})))) + (mdb.query/query {:select [:engine], :from [:metabase_database], :where [:= :id db-id]})))) (finally - (db/simple-delete! Database :name "Legacy BigQuery driver DB")))))) + (t2/delete! (t2/table-name Database) :name "Legacy BigQuery driver DB")))))) (deftest create-root-permissions-entry-for-admin-test (testing "Migration v0.43.00-006: Add root permissions entry for 'Administrators' magic group" @@ -371,8 +370,8 @@ (is (integer? admin-group-id)) (when existing-entry? (t2/query-one {:insert-into :permissions - :values [{:object "/" - :group_id admin-group-id}]})) + :values [{:object "/" + :group_id admin-group-id}]})) (migrate!) (is (= [{:object "/"}] (mdb.query/query {:select [:object] @@ -385,15 +384,15 @@ (testing (format "With existing data migration? %s" (pr-str with-existing-data-migration?)) (impl/test-migrations "v43.00-007" [migrate!] (t2/query-one {:insert-into :metabase_database - :values [{:name "My DB" - :engine "h2" - :created_at :%now - :updated_at :%now - :details "{}"}]}) + :values [{:name "My DB" + :engine "h2" + :created_at :%now + :updated_at :%now + :details "{}"}]}) (when with-existing-data-migration? (t2/query-one {:insert-into :data_migrations - :values [{:id "add-databases-to-magic-permissions-groups" - :timestamp :%now}]})) + :values [{:id "add-databases-to-magic-permissions-groups" + :timestamp :%now}]})) (migrate!) (is (= (if with-existing-data-migration? [] @@ -407,8 +406,8 @@ (testing "Migration v43.00-008: migrate legacy `-site-url` Setting to `site-url`; remove trailing slashes (#4123, #4188, #20402)" (impl/test-migrations ["v43.00-008"] [migrate!] (t2/query-one {:insert-into :setting - :values [{:key "-site-url" - :value "http://localhost:3000/"}]}) + :values [{:key "-site-url" + :value "http://localhost:3000/"}]}) (migrate!) (is (= [{:key "site-url", :value "http://localhost:3000"}] (mdb.query/query {:select [:*], :from [:setting], :where [:= :key "site-url"]})))))) @@ -417,16 +416,16 @@ (testing "Migration v43.00-009: ensure `site-url` Setting starts with a protocol (#20403)" (impl/test-migrations ["v43.00-009"] [migrate!] (t2/query-one {:insert-into :setting - :values [{:key "site-url" - :value "localhost:3000"}]}) + :values [{:key "site-url" + :value "localhost:3000"}]}) (migrate!) (is (= [{:key "site-url", :value "http://localhost:3000"}] (mdb.query/query {:select [:*], :from [:setting], :where [:= :key "site-url"]})))))) (defn- add-legacy-data-migration-entry! [migration-name] (t2/query-one {:insert-into :data_migrations - :values [{:id migration-name - :timestamp :%now}]})) + :values [{:id migration-name + :timestamp :%now}]})) (defn- add-migrated-collections-data-migration-entry! [] (add-legacy-data-migration-entry! "add-migrated-collections")) @@ -435,54 +434,54 @@ (testing "Migrations v43.00-014 - v43.00-019" (letfn [(create-user! [] (t2/query-one {:insert-into :core_user - :values [{:first_name "Cam" - :last_name "Era" - :email "cam@era.com" - :password "abc123" - :date_joined :%now}]}))] + :values [{:first_name "Cam" + :last_name "Era" + :email "cam@era.com" + :password "abc123" + :date_joined :%now}]}))] (doseq [{:keys [model collection-name create-instance!]} [{:model Dashboard :collection-name "Migrated Dashboards" :create-instance! (fn [] (create-user!) (t2/query-one {:insert-into :report_dashboard - :values [{:name "My Dashboard" - :created_at :%now - :updated_at :%now - :creator_id 1 - :parameters "[]" - :collection_id nil}]}))} + :values [{:name "My Dashboard" + :created_at :%now + :updated_at :%now + :creator_id 1 + :parameters "[]" + :collection_id nil}]}))} {:model Pulse :collection-name "Migrated Pulses" :create-instance! (fn [] (create-user!) (t2/query-one {:insert-into :pulse - :values [{:name "My Pulse" - :created_at :%now - :updated_at :%now - :creator_id 1 - :parameters "[]" - :collection_id nil}]}))} + :values [{:name "My Pulse" + :created_at :%now + :updated_at :%now + :creator_id 1 + :parameters "[]" + :collection_id nil}]}))} {:model Card :collection-name "Migrated Questions" :create-instance! (fn [] (create-user!) (t2/query-one {:insert-into :metabase_database - :values [{:name "My DB" - :engine "h2" - :details "{}" - :created_at :%now - :updated_at :%now}]}) + :values [{:name "My DB" + :engine "h2" + :details "{}" + :created_at :%now + :updated_at :%now}]}) (t2/query-one {:insert-into :report_card - :values [{:name "My Saved Question" - :created_at :%now - :updated_at :%now - :creator_id 1 - :display "table" - :dataset_query "{}" - :visualization_settings "{}" - :database_id 1 - :collection_id nil}]}))}] + :values [{:name "My Saved Question" + :created_at :%now + :updated_at :%now + :creator_id 1 + :display "table" + :dataset_query "{}" + :visualization_settings "{}" + :database_id 1 + :collection_id nil}]}))}] :let [table-name-keyword (t2/table-name model)]] (testing (format "create %s Collection for %s in the Root Collection" @@ -522,7 +521,7 @@ (impl/test-migrations ["v43.00-014" "v43.00-019"] [migrate!] (create-instance!) (t2/query-one {:insert-into :collection - :values [{:name collection-name, :slug "existing_collection", :color "#abc123"}]}) + :values [{:name collection-name, :slug "existing_collection", :color "#abc123"}]}) (migrate!) (is (= [{:name collection-name, :slug "existing_collection"}] (collections))) @@ -559,8 +558,8 @@ (testing "entry already exists: don't create an entry" (impl/test-migrations ["v43.00-020" "v43.00-021"] [migrate!] (t2/query-one {:insert-into :permissions - :values [{:object "/collection/root/" - :group_id (all-users-group-id)}]}) + :values [{:object "/collection/root/" + :group_id (all-users-group-id)}]}) (migrate!) (is (= [{:object "/collection/root/"}] (all-user-perms)))))))) @@ -569,20 +568,20 @@ (testing "Migration v43.00-029: clear password and password_salt for LDAP users" (impl/test-migrations ["v43.00-029"] [migrate!] (t2/query-one {:insert-into :core_user - :values [{:first_name "Cam" - :last_name "Era" - :email "cam@era.com" - :date_joined :%now - :password "password" - :password_salt "and pepper" - :ldap_auth false} - {:first_name "LDAP Cam" - :last_name "Era" - :email "ldap_cam@era.com" - :date_joined :%now - :password "password" - :password_salt "and pepper" - :ldap_auth true}]}) + :values [{:first_name "Cam" + :last_name "Era" + :email "cam@era.com" + :date_joined :%now + :password "password" + :password_salt "and pepper" + :ldap_auth false} + {:first_name "LDAP Cam" + :last_name "Era" + :email "ldap_cam@era.com" + :date_joined :%now + :password "password" + :password_salt "and pepper" + :ldap_auth true}]}) (migrate!) (is (= [{:first_name "Cam", :password "password", :password_salt "and pepper", :ldap_auth false} {:first_name "LDAP Cam", :password nil, :password_salt nil, :ldap_auth true}] @@ -594,11 +593,11 @@ (testing "Migration v43.00-042: grant download permissions to All Users permissions group" (impl/test-migrations ["v43.00-042" "v43.00-043"] [migrate!] (t2/query-one {:insert-into :metabase_database - :values [{:name "My DB" - :engine "h2" - :created_at :%now - :updated_at :%now - :details "{}"}]}) + :values [{:name "My DB" + :engine "h2" + :created_at :%now + :updated_at :%now + :details "{}"}]}) (migrate!) (is (= [{:object "/collection/root/"} {:object "/download/db/1/"}] (mdb.query/query {:select [:p.object] @@ -631,21 +630,21 @@ (testing "Migration v44.00-022: Add parameters to report_card" (impl/test-migrations ["v44.00-022" "v44.00-024"] [migrate!] (let [user-id - (db/simple-insert! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) - database-id (db/simple-insert! Database {:name "DB", :engine "h2", :created_at :%now, :updated_at :%now}) - card-id (db/simple-insert! Card {:name "My Saved Question" - :created_at :%now - :updated_at :%now - :creator_id user-id - :display "table" - :dataset_query "{}" - :visualization_settings "{}" - :database_id database-id - :collection_id nil})] + (first (t2/insert-returning-pks! (t2/table-name User) {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now})) + database-id (first (t2/insert-returning-pks! (t2/table-name Database) {:name "DB", :engine "h2", :created_at :%now, :updated_at :%now})) + card-id (first (t2/insert-returning-pks! (t2/table-name Card) {:name "My Saved Question" + :created_at :%now + :updated_at :%now + :creator_id user-id + :display "table" + :dataset_query "{}" + :visualization_settings "{}" + :database_id database-id + :collection_id nil}))] (migrate!) (is (= nil (:parameters (first (t2/select (t2/table-name Card) {:where [:= :id card-id]}))))))))) @@ -654,23 +653,23 @@ (testing "Migration v44.00-024: Add parameter_mappings to cards" (impl/test-migrations ["v44.00-024" "v44.00-026"] [migrate!] (let [user-id - (db/simple-insert! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) + (first (t2/insert-returning-pks! (t2/table-name User) {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now})) database-id - (db/simple-insert! Database {:name "DB", :engine "h2", :created_at :%now, :updated_at :%now}) + (first (t2/insert-returning-pks! (t2/table-name Database) {:name "DB", :engine "h2", :created_at :%now, :updated_at :%now})) card-id - (db/simple-insert! Card {:name "My Saved Question" - :created_at :%now - :updated_at :%now - :creator_id user-id - :display "table" - :dataset_query "{}" - :visualization_settings "{}" - :database_id database-id - :collection_id nil})] + (first (t2/insert-returning-pks! (t2/table-name Card) {:name "My Saved Question" + :created_at :%now + :updated_at :%now + :creator_id user-id + :display "table" + :dataset_query "{}" + :visualization_settings "{}" + :database_id database-id + :collection_id nil}))] (migrate!) (is (= nil (:parameter_mappings (first (t2/select (t2/table-name Card) {:where [:= :id card-id]}))))))))) @@ -695,8 +694,8 @@ (testing "Should run for new EE instances" (impl/test-migrations ["v44.00-033" "v44.00-034"] [migrate!] - (db/simple-insert! Setting {:key "premium-embedding-token" - :value "fake-key"}) + (t2/insert! (t2/table-name Setting) {:key "premium-embedding-token" + :value "fake-key"}) (migrate!) (is (= ["All Users"] (get-perms))))) @@ -709,24 +708,24 @@ (testing "Should not run for existing EE instances" (impl/test-migrations ["v44.00-033" "v44.00-034"] [migrate!] (create-raw-user! "ngoc@metabase.com") - (db/simple-insert! Setting {:key "premium-embedding-token" - :value "fake-key"}) + (t2/insert! (t2/table-name Setting) {:key "premium-embedding-token" + :value "fake-key"}) (migrate!) (is (= [] (get-perms))))) (testing "Should not fail if permissions already exist" (impl/test-migrations ["v44.00-033" "v44.00-034"] [migrate!] (t2/query-one {:insert-into :permissions - :values [{:object (perms-path) - :group_id (all-users-group-id)}]}) + :values [{:object (perms-path) + :group_id (all-users-group-id)}]}) (migrate!) (is (= ["All Users"] (get-perms)))))))) (deftest make-database-details-not-null-test (testing "Migrations v45.00-042 and v45.00-043: set default value of '{}' for Database rows with NULL details" (impl/test-migrations ["v45.00-042" "v45.00-043"] [migrate!] - (let [database-id (db/simple-insert! Database (-> (dissoc (mt/with-temp-defaults Database) :details) - (assoc :engine "h2")))] + (let [database-id (first (t2/insert-returning-pks! (t2/table-name Database) (-> (dissoc (mt/with-temp-defaults Database) :details) + (assoc :engine "h2"))))] (is (partial= {:details nil} (t2/select-one Database :id database-id))) (migrate!) @@ -736,43 +735,43 @@ (deftest populate-collection-created-at-test (testing "Migrations v45.00-048 thru v45.00-050: add Collection.created_at and populate it" (impl/test-migrations ["v45.00-048" "v45.00-050"] [migrate!] - (let [database-id (db/simple-insert! Database {:details "{}" - :engine "h2" - :is_sample false - :name "populate-collection-created-at-test-db"}) - user-id (db/simple-insert! User {:first_name "Cam" - :last_name "Era" - :email "cam@example.com" - :password "123456" - :date_joined #t "2022-10-20T02:09Z"}) - personal-collection-id (db/simple-insert! Collection {:name "Cam Era's Collection" - :personal_owner_id user-id - :color "#ff0000" - :slug "personal_collection"}) - impersonal-collection-id (db/simple-insert! Collection {:name "Regular Collection" - :color "#ff0000" - :slug "regular_collection"}) - empty-collection-id (db/simple-insert! Collection {:name "Empty Collection" - :color "#ff0000" - :slug "empty_collection"}) - _ (db/simple-insert! Card {:collection_id impersonal-collection-id - :name "Card 1" - :display "table" - :dataset_query "{}" - :visualization_settings "{}" - :creator_id user-id - :database_id database-id - :created_at #t "2022-10-20T02:09Z" - :updated_at #t "2022-10-20T02:09Z"}) - _ (db/simple-insert! Card {:collection_id impersonal-collection-id - :name "Card 2" - :display "table" - :dataset_query "{}" - :visualization_settings "{}" - :creator_id user-id - :database_id database-id - :created_at #t "2021-10-20T02:09Z" - :updated_at #t "2022-10-20T02:09Z"})] + (let [database-id (first (t2/insert-returning-pks! (t2/table-name Database) {:details "{}" + :engine "h2" + :is_sample false + :name "populate-collection-created-at-test-db"})) + user-id (first (t2/insert-returning-pks! (t2/table-name User) {:first_name "Cam" + :last_name "Era" + :email "cam@example.com" + :password "123456" + :date_joined #t "2022-10-20T02:09Z"})) + personal-collection-id (first (t2/insert-returning-pks! (t2/table-name Collection) {:name "Cam Era's Collection" + :personal_owner_id user-id + :color "#ff0000" + :slug "personal_collection"})) + impersonal-collection-id (first (t2/insert-returning-pks! (t2/table-name Collection) {:name "Regular Collection" + :color "#ff0000" + :slug "regular_collection"})) + empty-collection-id (first (t2/insert-returning-pks! (t2/table-name Collection) {:name "Empty Collection" + :color "#ff0000" + :slug "empty_collection"})) + _ (t2/insert! (t2/table-name Card) {:collection_id impersonal-collection-id + :name "Card 1" + :display "table" + :dataset_query "{}" + :visualization_settings "{}" + :creator_id user-id + :database_id database-id + :created_at #t "2022-10-20T02:09Z" + :updated_at #t "2022-10-20T02:09Z"}) + _ (t2/insert! (t2/table-name Card) {:collection_id impersonal-collection-id + :name "Card 2" + :display "table" + :dataset_query "{}" + :visualization_settings "{}" + :creator_id user-id + :database_id database-id + :created_at #t "2021-10-20T02:09Z" + :updated_at #t "2022-10-20T02:09Z"})] (migrate!) (testing "A personal Collection should get created_at set by to the date_joined from its owner" (is (= (t/offset-date-time #t "2022-10-20T02:09Z") @@ -790,42 +789,42 @@ (deftest deduplicate-dimensions-test (testing "Migrations v46.00-029 thru v46.00-031: make Dimension field_id unique instead of field_id + name" (impl/test-migrations ["v46.00-029" "v46.00-031"] [migrate!] - (let [database-id (db/simple-insert! Database {:details "{}" - :engine "h2" - :is_sample false - :name "populate-collection-created-at-test-db"}) - table-id (db/simple-insert! Table {:db_id database-id - :name "Table" - :created_at :%now - :updated_at :%now - :active true}) - field-1-id (db/simple-insert! Field {:name "F1" - :table_id table-id - :base_type "type/Text" - :database_type "TEXT" - :created_at :%now - :updated_at :%now}) - field-2-id (db/simple-insert! Field {:name "F2" - :table_id table-id - :base_type "type/Text" - :database_type "TEXT" - :created_at :%now - :updated_at :%now}) - _ (db/simple-insert! Dimension {:field_id field-1-id - :name "F1 D1" - :type "internal" - :created_at #t "2022-12-07T18:30:30.000-08:00" - :updated_at #t "2022-12-07T18:30:30.000-08:00"}) - _ (db/simple-insert! Dimension {:field_id field-1-id - :name "F1 D2" - :type "internal" - :created_at #t "2022-12-07T18:45:30.000-08:00" - :updated_at #t "2022-12-07T18:45:30.000-08:00"}) - _ (db/simple-insert! Dimension {:field_id field-2-id - :name "F2 D1" - :type "internal" - :created_at #t "2022-12-07T18:45:30.000-08:00" - :updated_at #t "2022-12-07T18:45:30.000-08:00"})] + (let [database-id (first (t2/insert-returning-pks! (t2/table-name Database) {:details "{}" + :engine "h2" + :is_sample false + :name "populate-collection-created-at-test-db"})) + table-id (first (t2/insert-returning-pks! (t2/table-name Table) {:db_id database-id + :name "Table" + :created_at :%now + :updated_at :%now + :active true})) + field-1-id (first (t2/insert-returning-pks! (t2/table-name Field) {:name "F1" + :table_id table-id + :base_type "type/Text" + :database_type "TEXT" + :created_at :%now + :updated_at :%now})) + field-2-id (first (t2/insert-returning-pks! (t2/table-name Field) {:name "F2" + :table_id table-id + :base_type "type/Text" + :database_type "TEXT" + :created_at :%now + :updated_at :%now})) + _ (t2/insert! (t2/table-name Dimension) {:field_id field-1-id + :name "F1 D1" + :type "internal" + :created_at #t "2022-12-07T18:30:30.000-08:00" + :updated_at #t "2022-12-07T18:30:30.000-08:00"}) + _ (t2/insert! (t2/table-name Dimension) {:field_id field-1-id + :name "F1 D2" + :type "internal" + :created_at #t "2022-12-07T18:45:30.000-08:00" + :updated_at #t "2022-12-07T18:45:30.000-08:00"}) + _ (t2/insert! (t2/table-name Dimension) {:field_id field-2-id + :name "F2 D1" + :type "internal" + :created_at #t "2022-12-07T18:45:30.000-08:00" + :updated_at #t "2022-12-07T18:45:30.000-08:00"})] (is (= #{"F1 D1" "F1 D2" "F2 D1"} @@ -840,74 +839,74 @@ (testing "Migrations v46.00-064 to v46.00-067: rename `group_table_access_policy` table, add `permission_id` FK, and clean up orphaned rows" (impl/test-migrations ["v46.00-064" "v46.00-067"] [migrate!] - (let [db-id (db/simple-insert! Database {:name "DB" - :engine "h2" - :created_at :%now - :updated_at :%now - :details "{}"}) - table-id (db/simple-insert! Table {:db_id db-id - :name "Table" - :created_at :%now - :updated_at :%now - :active true}) + (let [db-id (first (t2/insert-returning-pks! (t2/table-name Database) {:name "DB" + :engine "h2" + :created_at :%now + :updated_at :%now + :details "{}"})) + table-id (first (t2/insert-returning-pks! (t2/table-name Table) {:db_id db-id + :name "Table" + :created_at :%now + :updated_at :%now + :active true})) _ (t2/query-one {:insert-into :group_table_access_policy - :values [{:group_id 1 - :table_id table-id - :attribute_remappings "{\"foo\", 1}"} - {:group_id 2 - :table_id table-id - :attribute_remappings "{\"foo\", 1}"}]}) - perm-id (db/simple-insert! Permissions {:group_id 1 - :object "/db/1/schema/PUBLIC/table/1/query/segmented/"})] - ;; Two rows are present in `group_table_access_policy` - (is (= [{:id 1, :group_id 1, :table_id table-id, :card_id nil, :attribute_remappings "{\"foo\", 1}"} - {:id 2, :group_id 2, :table_id table-id, :card_id nil, :attribute_remappings "{\"foo\", 1}"}] - (mdb.query/query {:select [:*] :from [:group_table_access_policy]}))) - (migrate!) - ;; Only the sandbox with a corresponding `Permissions` row is present, and the table is renamed to `sandboxes` - (is (= [{:id 1, :group_id 1, :table_id table-id, :card_id nil, :attribute_remappings "{\"foo\", 1}", :permission_id perm-id}] - (mdb.query/query {:select [:*] :from [:sandboxes]}))))))) + :values [{:group_id 1 + :table_id table-id + :attribute_remappings "{\"foo\", 1}"} + {:group_id 2 + :table_id table-id + :attribute_remappings "{\"foo\", 1}"}]}) + perm-id (first (t2/insert-returning-pks! (t2/table-name Permissions) {:group_id 1 + :object "/db/1/schema/PUBLIC/table/1/query/segmented/"}))] + ;; Two rows are present in `group_table_access_policy` + (is (= [{:id 1, :group_id 1, :table_id table-id, :card_id nil, :attribute_remappings "{\"foo\", 1}"} + {:id 2, :group_id 2, :table_id table-id, :card_id nil, :attribute_remappings "{\"foo\", 1}"}] + (mdb.query/query {:select [:*] :from [:group_table_access_policy]}))) + (migrate!) + ;; Only the sandbox with a corresponding `Permissions` row is present, and the table is renamed to `sandboxes` + (is (= [{:id 1, :group_id 1, :table_id table-id, :card_id nil, :attribute_remappings "{\"foo\", 1}", :permission_id perm-id}] + (mdb.query/query {:select [:*] :from [:sandboxes]}))))))) (deftest able-to-delete-db-with-actions-test (testing "Migrations v46.00-084 and v46.00-085 set delete CASCADE for action.model_id to fix the bug of unable to delete database with actions" (impl/test-migrations ["v46.00-084" "v46.00-085"] [migrate!] - (let [user-id (db/simple-insert! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) - db-id (db/simple-insert! Database {:name "db" - :engine "postgres" - :created_at :%now - :updated_at :%now - :settings "{\"database-enable-actions\":true}" - :details "{}"}) - table-id (db/simple-insert! Table {:db_id db-id - :name "Table" - :created_at :%now - :updated_at :%now - :active true}) - model-id (db/simple-insert! Card {:name "My Saved Question" - :created_at :%now - :updated_at :%now - :creator_id user-id - :table_id table-id - :display "table" - :dataset_query "{}" - :visualization_settings "{}" - :database_id db-id - :collection_id nil}) - _ (db/simple-insert! Action {:name "Update user name" - :type "implicit" - :model_id model-id - :archived false - :created_at :%now - :updated_at :%now})] - (is (thrown? clojure.lang.ExceptionInfo - (t2/delete! Database :id db-id))) - (migrate!) - (is (t2/delete! Database :id db-id)))))) + (let [user-id (first (t2/insert-returning-pks! (t2/table-name User) {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now})) + db-id (first (t2/insert-returning-pks! (t2/table-name Database) {:name "db" + :engine "postgres" + :created_at :%now + :updated_at :%now + :settings "{\"database-enable-actions\":true}" + :details "{}"})) + table-id (first (t2/insert-returning-pks! (t2/table-name Table) {:db_id db-id + :name "Table" + :created_at :%now + :updated_at :%now + :active true})) + model-id (first (t2/insert-returning-pks! (t2/table-name Card) {:name "My Saved Question" + :created_at :%now + :updated_at :%now + :creator_id user-id + :table_id table-id + :display "table" + :dataset_query "{}" + :visualization_settings "{}" + :database_id db-id + :collection_id nil})) + _ (t2/insert! (t2/table-name Action) {:name "Update user name" + :type "implicit" + :model_id model-id + :archived false + :created_at :%now + :updated_at :%now})] + (is (thrown? clojure.lang.ExceptionInfo + (t2/delete! Database :id db-id))) + (migrate!) + (is (t2/delete! Database :id db-id)))))) (deftest split-data-permission-test (testing "Migration v46.00-080: split existing v1 data permission paths into v2 data and query permission paths" diff --git a/test/metabase/db/schema_migrations_test/impl.clj b/test/metabase/db/schema_migrations_test/impl.clj index 432bc2193f3dc47fdde16c25404181bb40127187..cfb8ad85f79f8d68515e3870f3a56aadfbb82517 100644 --- a/test/metabase/db/schema_migrations_test/impl.clj +++ b/test/metabase/db/schema_migrations_test/impl.clj @@ -223,7 +223,7 @@ this order: 1. Load data and check any preconditions before running migrations you're testing. - Prefer [[toucan.db/simple-insert!]] or plain SQL for loading data to avoid dependencies on the current state of + Prefer [[t2/insert!]] with a table name or plain SQL for loading data to avoid dependencies on the current state of the schema that may be present in Toucan `pre-insert` functions and the like. 2. Call `(migrate!)` to run migrations in range of `start-id` -> `end-id` (inclusive) diff --git a/test/metabase/events/revision_test.clj b/test/metabase/events/revision_test.clj index 965a615a1f7e6f6ec895bfb134cc4e2fabc76c1d..2416c4b790db11c99371a15f940e9ab2b859d051 100644 --- a/test/metabase/events/revision_test.clj +++ b/test/metabase/events/revision_test.clj @@ -6,7 +6,6 @@ :refer [Card Dashboard DashboardCard Database Metric Revision Segment Table]] [metabase.test :as mt] [metabase.util :as u] - [toucan.db :as db] [toucan2.core :as t2])) (defn- card-properties @@ -138,7 +137,7 @@ (mt/with-temp* [Dashboard [{dashboard-id :id, :as dashboard}] Card [{card-id :id} (card-properties)] DashboardCard [dashcard {:card_id card-id, :dashboard_id dashboard-id}]] - (db/simple-delete! DashboardCard, :id (:id dashcard)) + (t2/delete! (t2/table-name DashboardCard), :id (:id dashcard)) (revision/process-revision-event! {:topic :dashboard-remove-cards :item {:id dashboard-id :actor_id (mt/user->id :rasta) diff --git a/test/metabase/models/setting/cache_test.clj b/test/metabase/models/setting/cache_test.clj index 1d0436f1220cc8938811e7156fcec688d278e5a3..204ee76335bf04db5a6f5ef89ad562004b8e945b 100644 --- a/test/metabase/models/setting/cache_test.clj +++ b/test/metabase/models/setting/cache_test.clj @@ -38,7 +38,7 @@ (defn- simulate-another-instance-updating-setting! [setting-name new-value] (if new-value (db/update-where! Setting {:key (name setting-name)} :value new-value) - (db/simple-delete! Setting {:key (name setting-name)})) + (t2/delete! (t2/table-name Setting) {:key (name setting-name)})) (update-settings-last-updated-value-in-db!)) (defn reset-last-update-check! diff --git a/test/metabase/models/setting_test.clj b/test/metabase/models/setting_test.clj index 923356c2621aed7d66fba1f9e0386918ca426ba8..918617ebc90611e09535a1a7cd4bd338fe60bd24 100644 --- a/test/metabase/models/setting_test.clj +++ b/test/metabase/models/setting_test.clj @@ -514,7 +514,7 @@ (defn clear-settings-last-updated-value-in-db! "Deletes the timestamp for the last updated setting from the DB." [] - (db/simple-delete! Setting {:key setting.cache/settings-last-updated-key})) + (t2/delete! (t2/table-name Setting) :key setting.cache/settings-last-updated-key)) (defn settings-last-updated-value-in-db "Fetches the timestamp of the last updated setting." @@ -571,7 +571,7 @@ (deftest cache-sync-test (testing "make sure that if for some reason the cache gets out of sync it will reset so we can still set new settings values (#4178)" ;; clear out any existing values of `toucan-name` - (db/simple-delete! setting/Setting {:key "toucan-name"}) + (t2/delete! (t2/table-name setting/Setting) :key "toucan-name") ;; restore the cache (setting.cache/restore-cache-if-needed!) ;; now set a value for the `toucan-name` setting the wrong way diff --git a/test/metabase/server/middleware/session_test.clj b/test/metabase/server/middleware/session_test.clj index 437a5cf96909f9e49a1923bf520a41b54c1ff268..5d43fc4648f2029bcf479d927f1ff3c76342fbab 100644 --- a/test/metabase/server/middleware/session_test.clj +++ b/test/metabase/server/middleware/session_test.clj @@ -117,7 +117,7 @@ (testing (format "\n%s %s be expired." msg (if expected "SHOULD" "SHOULD NOT")) (mt/with-temp User [{user-id :id}] (let [session-id (str (UUID/randomUUID))] - (db/simple-insert! Session {:id session-id, :user_id user-id, :created_at created-at}) + (t2/insert! (t2/table-name Session) {:id session-id, :user_id user-id, :created_at created-at}) (let [session (#'mw.session/current-user-info-for-session session-id nil)] (if expected (is (= nil diff --git a/test/metabase/sync/field_values_test.clj b/test/metabase/sync/field_values_test.clj index 170a5a4dc8251a77eea01b4596cce7546202cfda..20ec9427e28321f14dee9053446ac2e19471027e 100644 --- a/test/metabase/sync/field_values_test.clj +++ b/test/metabase/sync/field_values_test.clj @@ -12,7 +12,6 @@ [metabase.test :as mt] [metabase.test.data :as data] [metabase.test.data.one-off-dbs :as one-off-dbs] - [toucan.db :as db] [toucan2.core :as t2])) (defn- venues-price-field-values [] @@ -93,8 +92,8 @@ valid-sandbox-id valid-linked-filter-id old-full-id - new-full-id] (db/simple-insert-many! - FieldValues + new-full-id] (t2/insert-returning-pks! + (t2/table-name FieldValues) [;; expired sandbox fieldvalues {:field_id field-id :type "sandbox" diff --git a/test/metabase/test.clj b/test/metabase/test.clj index f3d6380e10264404383e5adf1f002152f440aeb4..379c818d7112c3b1393f979674909824eb56eaff 100644 --- a/test/metabase/test.clj +++ b/test/metabase/test.clj @@ -309,7 +309,7 @@ (defn do-with-single-admin-user [attributes thunk] (let [existing-admin-memberships (t2/select PermissionsGroupMembership :group_id (:id (perms-group/admin))) - _ (db/simple-delete! PermissionsGroupMembership :group_id (:id (perms-group/admin))) + _ (t2/delete! (t2/table-name PermissionsGroupMembership) :group_id (:id (perms-group/admin))) existing-admin-ids (t2/select-pks-set User :is_superuser true) _ (when (seq existing-admin-ids) (db/update-where! User {:id [:in existing-admin-ids]} :is_superuser false))