diff --git a/.clj-kondo/hooks/clojure/core.clj b/.clj-kondo/hooks/clojure/core.clj index 809c56cbaa307d72b269595e7daaafb0dd3af0b9..99686b6abc4e682d4a778930fbc7214be39fe4ca 100644 --- a/.clj-kondo/hooks/clojure/core.clj +++ b/.clj-kondo/hooks/clojure/core.clj @@ -68,6 +68,7 @@ metabase.db.schema-migrations-test.impl/run-migrations-in-range! metabase.db.setup/migrate! metabase.db.setup/setup-db! + metabase.db/migrate! metabase.db/setup-db! metabase.driver.mongo-test/create-database-from-row-maps! metabase.driver.postgres-test/create-enums-db! diff --git a/.dir-locals.el b/.dir-locals.el index 4a87d272f40e19034eabf23d2e0ed7cbac3d8d1b..81e6455061506ffb069e5cec9924c99de6333f20 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -60,6 +60,7 @@ (eval . (put-clojure-indent 'mt/test-driver 1)) (eval . (put-clojure-indent 'prop/for-all 1)) (eval . (put-clojure-indent 'qp.streaming/streaming-response 1)) + (eval . (put-clojure-indent 'u/profile 1)) (eval . (put-clojure-indent 'u/prog1 1)) (eval . (put-clojure-indent 'u/select-keys-when 1)) (eval . (put-clojure-indent 'with-meta '(:form))) diff --git a/dev/src/dev.clj b/dev/src/dev.clj index db40f9883f42e41b5dd6f8116987a988bd302eb5..c2e4cd306795221bcbadd064c3cf92c89a7c816d 100644 --- a/dev/src/dev.clj +++ b/dev/src/dev.clj @@ -48,9 +48,8 @@ [metabase.api.common :as api] [metabase.config :as config] [metabase.core :as mbc] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.env :as mdb.env] - [metabase.db.setup :as mdb.setup] [metabase.driver :as driver] [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute] @@ -249,8 +248,8 @@ ([] (migrate! :up)) ([direction & [version]] - (mdb.setup/migrate! (mdb.connection/db-type) (mdb.connection/data-source) - direction version))) + (mdb/migrate! (mdb/db-type) (mdb/data-source) + direction version))) (methodical/defmethod t2.connection/do-with-connection :model/Database "Support running arbitrary queries against data warehouse DBs for easy REPL debugging. Only works for SQL+JDBC drivers @@ -303,11 +302,11 @@ (or (t2/select-one Database :name "Application Database") #_:clj-kondo/ignore (let [details (#'metabase.db.env/broken-out-details - (mdb.connection/db-type) + (mdb/db-type) @#'metabase.db.env/env) app-db (first (t2/insert-returning-instances! Database {:name "Application Database" - :engine (mdb.connection/db-type) + :engine (mdb/db-type) :details details}))] (sync/sync-database! app-db) app-db)))) diff --git a/dev/src/dev/debug_qp.clj b/dev/src/dev/debug_qp.clj index 932f2b95002f3650c77a2047f6bd3564236e20a4..9101e471dbc00b4f61697b29cd156e0efd2d9676 100644 --- a/dev/src/dev/debug_qp.clj +++ b/dev/src/dev/debug_qp.clj @@ -5,7 +5,7 @@ [clojure.walk :as walk] [lambdaisland.deep-diff2 :as ddiff] [medley.core :as m] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.mbql.normalize :as mbql.normalize] [metabase.mbql.util :as mbql.u] @@ -389,7 +389,7 @@ (defn pprint-sql "Pretty print a SQL string." ([sql] - (pprint-sql (mdb.connection/db-type) sql)) + (pprint-sql (mdb/db-type) sql)) ([driver sql] #_{:clj-kondo/ignore [:discouraged-var]} (println (driver/prettify-native-form driver sql)))) diff --git a/enterprise/backend/src/metabase_enterprise/advanced_config/api/logs.clj b/enterprise/backend/src/metabase_enterprise/advanced_config/api/logs.clj index 4ceb2cd045d6522552c603a5fbb681f41e5b1640..27c087931ba7012e8bc6f61882b95d6e84a43d12 100644 --- a/enterprise/backend/src/metabase_enterprise/advanced_config/api/logs.clj +++ b/enterprise/backend/src/metabase_enterprise/advanced_config/api/logs.clj @@ -11,7 +11,7 @@ [malli.core :as mc] [malli.transform :as mtx] [metabase.api.common :as api] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.util.i18n :refer [deferred-tru]] [metabase.util.malli :as mu] [metabase.util.malli.schema :as ms] @@ -22,7 +22,7 @@ [year :- ms/PositiveInt month :- ms/PositiveInt] (let [date-part (fn [part-key part-value] - (if (= (mdb.connection/db-type) :postgres) + (if (= (mdb/db-type) :postgres) [:= [:date_part [:inline (name part-key)] :started_at] [:inline part-value]] [:= [part-key :started_at] [:inline part-value]])) results (t2/select :query_execution diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/common.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/common.clj index 3feac525ba15104239646fa98ef6374605723e5e..af33ad6106a7bb1c4860d83d51527940bd0536d2 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/common.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/common.clj @@ -8,7 +8,6 @@ [metabase-enterprise.audit-app.query-processor.middleware.handle-audit-queries :as qp.middleware.audit] [metabase.db :as mdb] - [metabase.db.connection :as mdb.connection] [metabase.db.query :as mdb.query] [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute] [metabase.driver.sql-jdbc.sync :as sql-jdbc.sync] @@ -92,7 +91,7 @@ sql-jdbc.sync/db-default-timezone :ttl/threshold (u/hours->ms 1))] (fn [] - (timezone (mdb/db-type) {:datasource mdb.connection/*application-db*})))) + (timezone (mdb/db-type) {:datasource (mdb/app-db)})))) (defn- compile-honeysql [driver honeysql-query] (try @@ -115,7 +114,7 @@ ;; instead of mocking up a chunk of regular QP pipeline. (binding [qp.timezone/*results-timezone-id-override* (application-db-default-timezone)] (try - (with-open [conn (.getConnection mdb.connection/*application-db*) + (with-open [conn (.getConnection (mdb/app-db)) stmt (sql-jdbc.execute/prepared-statement driver conn sql params) rs (sql-jdbc.execute/execute-prepared-statement! driver stmt)] (let [rsmeta (.getMetaData rs) diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/queries.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/queries.clj index 9b6a00814e794472b77bb45cd012f6f55c217181..5e1de6ee31e4c51493ded7c8de3ea220116f40f5 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/queries.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/queries.clj @@ -3,7 +3,7 @@ [metabase-enterprise.audit-app.interface :as audit.i] [metabase-enterprise.audit-app.pages.common :as common] [metabase-enterprise.audit-app.pages.common.cards :as cards] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.models.permissions :as perms])) ;; List of all failing questions @@ -38,7 +38,7 @@ error-substr [:concat [:substring :latest_qe.error - [:inline (if (= (mdb.connection/db-type) :mysql) 1 0)] + [:inline (if (= (mdb/db-type) :mysql) 1 0)] [:inline 60]] "..."] dash-count [:coalesce :dash_card.count [:inline 0]]] diff --git a/enterprise/backend/src/metabase_enterprise/audit_db.clj b/enterprise/backend/src/metabase_enterprise/audit_db.clj index 281c4026b929d118630a54254e39af939e564848..6694fb52ff98e6a02486a433d51f756d1bb217ef 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_db.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_db.clj @@ -5,8 +5,7 @@ [clojure.string :as str] [metabase-enterprise.internal-user :as ee.internal-user] [metabase-enterprise.serialization.cmd :as serialization.cmd] - [metabase.db.connection :as mdb.connection] - [metabase.db.env :as mdb.env] + [metabase.db :as mdb] [metabase.models.database :refer [Database]] [metabase.models.permissions :as perms] [metabase.models.setting :refer [defsetting]] @@ -88,7 +87,7 @@ "Returns the object from entity id and model. Memoizes from entity id. Should only be used for audit/pre-loaded objects." [model entity-id] - ((mdb.connection/memoize-for-application-db + ((mdb/memoize-for-application-db (fn [entity-id] (t2/select-one model :entity_id entity-id))) entity-id)) @@ -125,11 +124,11 @@ (defn- adjust-audit-db-to-source! [{audit-db-id :id}] ;; We need to move back to a schema that matches the serialized data - (when (contains? #{:mysql :h2} mdb.env/db-type) + (when (contains? #{:mysql :h2} (mdb/db-type)) (t2/update! :model/Database audit-db-id {:engine "postgres"}) - (when (= :mysql mdb.env/db-type) + (when (= :mysql (mdb/db-type)) (t2/update! :model/Table {:db_id audit-db-id} {:schema "public"})) - (when (= :h2 mdb.env/db-type) + (when (= :h2 (mdb/db-type)) (t2/update! :model/Table {:db_id audit-db-id} {:schema [:lower :schema] :name [:lower :name]}) (t2/update! :model/Field {:table_id @@ -142,12 +141,12 @@ (defn- adjust-audit-db-to-host! [{audit-db-id :id :keys [engine]}] - (when (not= engine mdb.env/db-type) + (when (not= engine (mdb/db-type)) ;; We need to move the loaded data back to the host db - (t2/update! :model/Database audit-db-id {:engine (name mdb.env/db-type)}) - (when (= :mysql mdb.env/db-type) + (t2/update! :model/Database audit-db-id {:engine (name (mdb/db-type))}) + (when (= :mysql (mdb/db-type)) (t2/update! :model/Table {:db_id audit-db-id} {:schema nil})) - (when (= :h2 mdb.env/db-type) + (when (= :h2 (mdb/db-type)) (t2/update! :model/Table {:db_id audit-db-id} {:schema [:upper :schema] :name [:upper :name]}) (t2/update! :model/Field {:table_id @@ -156,7 +155,7 @@ :from [(t2/table-name :model/Table)] :where [:= :db_id audit-db-id]}]} {:name [:upper :name]})) - (log/infof "Adjusted Audit DB to match host engine: %s" (name mdb.env/db-type)))) + (log/infof "Adjusted Audit DB to match host engine: %s" (name (mdb/db-type))))) (def ^:private analytics-dir-resource "A resource dir containing analytics content created by Metabase to load into the app instance on startup." @@ -270,11 +269,11 @@ (nil? audit-db) (u/prog1 ::installed (log/info "Installing Audit DB...") - (install-database! mdb.env/db-type perms/audit-db-id)) + (install-database! (mdb/db-type) perms/audit-db-id)) - (not= mdb.env/db-type (:engine audit-db)) + (not= (mdb/db-type) (:engine audit-db)) (u/prog1 ::updated - (log/infof "App DB change detected. Changing Audit DB source to match: %s." (name mdb.env/db-type)) + (log/infof "App DB change detected. Changing Audit DB source to match: %s." (name (mdb/db-type))) (adjust-audit-db-to-host! audit-db)) :else diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj b/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj index d7446411679b35a851eb0977790ba218f23924a4..0a88c7c3cce36703ab565a204b14917430a95b95 100644 --- a/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj +++ b/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj @@ -8,7 +8,7 @@ [metabase-enterprise.sandbox.api.util :as mt.api.u] [metabase-enterprise.sandbox.models.group-table-access-policy :as gtap] [metabase.api.common :as api :refer [*current-user* *current-user-id*]] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.lib.metadata :as lib.metadata] [metabase.lib.metadata.protocols :as lib.metadata.protocols] [metabase.mbql.schema :as mbql.s] @@ -30,8 +30,6 @@ (set! *warn-on-reflection* true) -(comment mdb.connection/keep-me) ; used for [[memoize/ttl]] - ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | query->gtap | ;;; +----------------------------------------------------------------------------------------------------------------+ @@ -152,7 +150,7 @@ (def ^:private ^{:arglists '([table-id])} original-table-metadata (memoize/ttl ^{::memoize/args-fn (fn [[table-id]] - [(mdb.connection/unique-identifier) table-id])} + [(mdb/unique-identifier) table-id])} (fn [table-id] (mbql-query-metadata {:source-table table-id})) :ttl/threshold (u/minutes->ms 1))) diff --git a/enterprise/backend/src/metabase_enterprise/serialization/load.clj b/enterprise/backend/src/metabase_enterprise/serialization/load.clj index 690d8c3a1cb1842deca61540687e842d05415879..d15f52323512aa634dc7b0390cd41b6d2eec3176 100644 --- a/enterprise/backend/src/metabase_enterprise/serialization/load.clj +++ b/enterprise/backend/src/metabase_enterprise/serialization/load.clj @@ -10,7 +10,7 @@ :refer [fully-qualified-name->context]] [metabase-enterprise.serialization.upsert :refer [maybe-upsert-many!]] [metabase.config :as config] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.mbql.normalize :as mbql.normalize] [metabase.mbql.util :as mbql.u] [metabase.models.card :refer [Card]] @@ -177,7 +177,7 @@ (mbql-fully-qualified-names->ids* (mbql.normalize/normalize-tokens entity))) (def ^:private ^{:arglists '([])} default-user-id - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn [] (let [user (t2/select-one-pk User :is_superuser true)] (assert user (trs "No admin users found! At least one admin user is needed to act as the owner for all the loaded entities.")) diff --git a/enterprise/backend/src/metabase_enterprise/serialization/names.clj b/enterprise/backend/src/metabase_enterprise/serialization/names.clj index 82c3916722c95daa05441ff1638cf0b85b8e5fec..2cf03d688d6a0614bcc7114557d5311d2575dca1 100644 --- a/enterprise/backend/src/metabase_enterprise/serialization/names.clj +++ b/enterprise/backend/src/metabase_enterprise/serialization/names.clj @@ -3,7 +3,7 @@ (:require [clojure.string :as str] [malli.core :as mc] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.lib.schema.id :as lib.schema.id] [metabase.models.card :refer [Card]] [metabase.models.collection :refer [Collection]] @@ -41,7 +41,7 @@ (def ^{:arglists '([entity] [model id])} fully-qualified-name "Get the logical path for entity `entity`." - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn ([entity] (fully-qualified-name* entity)) ([model id] diff --git a/enterprise/backend/src/metabase_enterprise/serialization/v2/entity_ids.clj b/enterprise/backend/src/metabase_enterprise/serialization/v2/entity_ids.clj index f031ee433807a7490c948774a5489cf846427d1b..ed39bf3b57da07fd1c85dde37b3d737c472c3767 100644 --- a/enterprise/backend/src/metabase_enterprise/serialization/v2/entity_ids.clj +++ b/enterprise/backend/src/metabase_enterprise/serialization/v2/entity_ids.clj @@ -3,7 +3,6 @@ [clojure.set :as set] [clojure.string :as str] [metabase.db :as mdb] - [metabase.db.connection :as mdb.connection] [metabase.models] [metabase.models.serialization :as serdes] [metabase.util :as u] @@ -22,11 +21,11 @@ (defn- entity-id-table-names "Return a set of lower-cased names of all application database tables that have an `entity_id` column, excluding views." [] - (with-open [conn (.getConnection mdb.connection/*application-db*)] + (with-open [conn (.getConnection (mdb/app-db))] (let [dbmeta (.getMetaData conn)] (with-open [tables-rset (.getTables dbmeta nil nil nil (into-array String ["TABLE"]))] (let [non-view-tables (into #{} (map (comp u/lower-case-en :table_name)) (resultset-seq tables-rset))] - (with-open [rset (.getColumns dbmeta nil nil nil (case (mdb.connection/db-type) + (with-open [rset (.getColumns dbmeta nil nil nil (case (mdb/db-type) :h2 "ENTITY_ID" (:mysql :postgres) "entity_id"))] (let [entity-id-tables (into #{} (map (comp u/lower-case-en :table_name)) (resultset-seq rset))] diff --git a/enterprise/backend/test/metabase_enterprise/advanced_config/file/databases_test.clj b/enterprise/backend/test/metabase_enterprise/advanced_config/file/databases_test.clj index 27d510b4e173e60583d65f636cc9654a6bbcd884..aa79920920146b9aef4a4344bd60648d8ff291fa 100644 --- a/enterprise/backend/test/metabase_enterprise/advanced_config/file/databases_test.clj +++ b/enterprise/backend/test/metabase_enterprise/advanced_config/file/databases_test.clj @@ -2,7 +2,7 @@ (:require [clojure.test :refer :all] [metabase-enterprise.advanced-config.file :as advanced-config.file] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.driver.h2 :as h2] [metabase.models :refer [Database Table]] [metabase.test :as mt] @@ -19,7 +19,7 @@ (deftest init-from-config-file-test (mt/with-temporary-setting-values [config-from-file-sync-databases true] - (let [db-type (mdb.connection/db-type) + (let [db-type (mdb/db-type) original-db (mt/with-driver db-type (mt/db))] (try (binding [advanced-config.file/*config* {:version 1 diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/entity_ids_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/entity_ids_test.clj index 573b9a311fd4e1458e4f2d6933acdfbc33e730d5..2829d4e94abd85377c0e153e80b15ed9321a73c8 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/v2/entity_ids_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/entity_ids_test.clj @@ -4,7 +4,7 @@ [clojure.test :refer :all] [metabase-enterprise.serialization.v2.backfill-ids :as serdes.backfill] [metabase-enterprise.serialization.v2.entity-ids :as v2.entity-ids] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.models :refer [Collection Dashboard]] [metabase.test :as mt] [metabase.util :as u] @@ -91,8 +91,8 @@ (doseq [m (v2.entity-ids/toucan-models) :when (serdes.backfill/has-entity-id? m) :let [table-name (cond-> (name (t2/table-name m)) - (= :h2 (:db-type mdb.connection/*application-db*)) u/upper-case-en) - column-name (if (= :h2 (:db-type mdb.connection/*application-db*)) + (= :h2 (mdb/db-type)) u/upper-case-en) + column-name (if (= :h2 (mdb/db-type)) "ENTITY_ID" "entity_id") rs (-> (.getMetaData conn) diff --git a/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj b/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj index f83253e89eb6bf0ab4aac4dccafdf70f879ef2c7..9aa861647c1abecd7918c805a949beaecae5e87b 100644 --- a/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj +++ b/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj @@ -8,7 +8,7 @@ [honey.sql :as sql] [honey.sql.helpers :as sql.helpers] [java-time.api :as t] - [metabase.db.spec :as mdb.spec] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.driver.sql-jdbc.common :as sql-jdbc.common] [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] @@ -483,7 +483,7 @@ (-> details (merge {:classname "com.facebook.presto.jdbc.PrestoDriver" :subprotocol "presto" - :subname (mdb.spec/make-subname host port (db-name catalog schema))}) + :subname (mdb/make-subname host port (db-name catalog schema))}) prepare-addl-opts (dissoc :host :port :db :catalog :schema :tunnel-enabled :engine :kerberos) sql-jdbc.common/handle-additional-options)) diff --git a/src/metabase/analytics/stats.clj b/src/metabase/analytics/stats.clj index 93ceb3b147e010b540ccf088733e60b440637f2b..cdec21e68e03f9de9e82eda1e93fecf8f61c8008 100644 --- a/src/metabase/analytics/stats.clj +++ b/src/metabase/analytics/stats.clj @@ -9,7 +9,6 @@ [metabase.analytics.snowplow :as snowplow] [metabase.config :as config] [metabase.db.query :as mdb.query] - [metabase.db.util :as mdb.u] [metabase.driver :as driver] [metabase.email :as email] [metabase.embed.settings :as embed.settings] @@ -239,9 +238,9 @@ ;; Include `WHERE` clause that includes conditions for a Table related by an FK relationship: ;; (Number of Tables per DB engine) - (db-frequencies Table (mdb.u/qualify Database :engine) - {:left-join [Database [:= (mdb.u/qualify Database :id) - (mdb.u/qualify Table :db_id)]]}) + (db-frequencies Table (mdb.query/qualify Database :engine) + {:left-join [Database [:= (mdb.query/qualify Database :id) + (mdb.query/qualify Table :db_id)]]}) ;; -> {\"googleanalytics\" 4, \"postgres\" 48, \"h2\" 9}" {:style/indent 2} [model column & [additonal-honeysql]] @@ -284,7 +283,7 @@ :num_cards_per_pulses (medium-histogram (vals (db-frequencies PulseCard :pulse_id pulse-conditions)))})) (defn- alert-metrics [] - (let [alert-conditions {:left-join [:pulse [:= :pulse.id :pulse_id]], :where [:not= (mdb.u/qualify Pulse :alert_condition) nil]}] + (let [alert-conditions {:left-join [:pulse [:= :pulse.id :pulse_id]], :where [:not= (mdb.query/qualify Pulse :alert_condition) nil]}] {:alerts (t2/count Pulse :alert_condition [:not= nil]) :with_table_cards (num-notifications-with-xls-or-csv-cards [:not= :alert_condition nil]) :first_time_only (t2/count Pulse :alert_condition [:not= nil], :alert_first_only true) diff --git a/src/metabase/api/activity.clj b/src/metabase/api/activity.clj index 33af4096b01446c7a4d2f8b413303e07d438dadf..ef3c674a059635ef71f2aae20420dde3d901f5a9 100644 --- a/src/metabase/api/activity.clj +++ b/src/metabase/api/activity.clj @@ -4,7 +4,7 @@ [compojure.core :refer [GET]] [medley.core :as m] [metabase.api.common :as api :refer [*current-user-id* define-routes]] - [metabase.db.util :as mdb.u] + [metabase.db.query :as mdb.query] [metabase.models.card :refer [Card]] [metabase.models.dashboard :refer [Dashboard]] [metabase.models.interface :as mi] @@ -32,7 +32,7 @@ :display_name :initial_sync_status :visibility_type]) (let [model-symb (symbol (str/capitalize model)) - self-qualify #(mdb.u/qualify model-symb %)] + self-qualify #(mdb.query/qualify model-symb %)] (cond-> {:where [:in (self-qualify :id) ids]} (not= model "table") (merge {:left-join [:collection [:= :collection.id (self-qualify :collection_id)]]}))))) @@ -78,7 +78,7 @@ [:%max.timestamp :max_ts]] {:group-by [:model :model_id] :where [:and - (when-not all-users? [:= (mdb.u/qualify ViewLog :user_id) *current-user-id*]) + (when-not all-users? [:= (mdb.query/qualify ViewLog :user_id) *current-user-id*]) [:in :model #{"dashboard" "table"}] [:= :bm.id nil]] :order-by [[:max_ts :desc] [:model :desc]] @@ -90,10 +90,10 @@ [:= :model_id :bm.dashboard_id]]]}) card-runs (->> (t2/select [QueryExecution [:%min.executor_id :user_id] - [(mdb.u/qualify QueryExecution :card_id) :model_id] + [(mdb.query/qualify QueryExecution :card_id) :model_id] [:%count.* :cnt] [:%max.started_at :max_ts]] - {:group-by [(mdb.u/qualify QueryExecution :card_id) :context] + {:group-by [(mdb.query/qualify QueryExecution :card_id) :context] :where [:and (when-not all-users? [:= :executor_id *current-user-id*]) [:= :context (h2x/literal :question)] @@ -103,7 +103,7 @@ :left-join [[:card_bookmark :bm] [:and [:= :bm.user_id *current-user-id*] - [:= (mdb.u/qualify QueryExecution :card_id) :bm.card_id]]]}) + [:= (mdb.query/qualify QueryExecution :card_id) :bm.card_id]]]}) (map #(dissoc % :row_count)) (map #(assoc % :model "card")))] (->> (concat card-runs dashboard-and-table-views) diff --git a/src/metabase/api/database.clj b/src/metabase/api/database.clj index ef45b9990b9ba294e82858cd63df0de3686bf5df..aa64190fca9557b3b00af4870f89e3e2972843df 100644 --- a/src/metabase/api/database.clj +++ b/src/metabase/api/database.clj @@ -8,7 +8,7 @@ [metabase.api.common :as api] [metabase.api.table :as api.table] [metabase.config :as config] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.query :as mdb.query] [metabase.driver :as driver] [metabase.driver.ddl.interface :as ddl.i] @@ -541,7 +541,7 @@ ;; e.g. search-string = "123" (and (not-empty search-id) (empty? search-name)) [:like - (h2x/cast (if (= (mdb.connection/db-type) :mysql) :char :text) :report_card.id) + (h2x/cast (if (= (mdb/db-type) :mysql) :char :text) :report_card.id) (str search-id "%")] ;; e.g. search-string = "123-foo" diff --git a/src/metabase/api/field.clj b/src/metabase/api/field.clj index 0addd42a41b62a7c52f3bc8ec84040066a3a8380..84cb467e9098005f3f7d24737fae3f2c643ede3e 100644 --- a/src/metabase/api/field.clj +++ b/src/metabase/api/field.clj @@ -4,7 +4,7 @@ [compojure.core :refer [DELETE GET POST PUT]] [metabase.api.common :as api] [metabase.db.metadata-queries :as metadata-queries] - [metabase.db.util :as mdb.u] + [metabase.db.query :as mdb.query] [metabase.lib.schema.metadata :as lib.schema.metadata] [metabase.models.dimension :refer [Dimension]] [metabase.models.field :as field :refer [Field]] @@ -298,7 +298,7 @@ update-map {:values (map first value-pairs) :human_readable_values (when human-readable-values? (map second value-pairs))} - updated-pk (mdb.u/update-or-insert! FieldValues {:field_id (u/the-id field), :type :full} + updated-pk (mdb.query/update-or-insert! FieldValues {:field_id (u/the-id field), :type :full} (constantly update-map))] (api/check-500 (pos? updated-pk)))) {:status :success}) diff --git a/src/metabase/api/public.clj b/src/metabase/api/public.clj index 04f4e1bb1d03bd02703e78106cae541b7c39b7ef..103c60156dc5c6f64cf232449d6de479173238c2 100644 --- a/src/metabase/api/public.clj +++ b/src/metabase/api/public.clj @@ -13,7 +13,7 @@ [metabase.api.dashboard :as api.dashboard] [metabase.api.dataset :as api.dataset] [metabase.api.field :as api.field] - [metabase.db.util :as mdb.u] + [metabase.db.query :as mdb.query] [metabase.events :as events] [metabase.lib.schema.id :as lib.schema.id] [metabase.mbql.util :as mbql.u] @@ -407,8 +407,8 @@ (t2/exists? Dimension :field_id field-id, :human_readable_field_id search-field-id) ;; just do a couple small queries to figure this out, we could write a fancy query to join Field against itself ;; and do this in one but the extra code complexity isn't worth it IMO - (when-let [table-id (t2/select-one-fn :table_id Field :id field-id, :semantic_type (mdb.u/isa :type/PK))] - (t2/exists? Field :id search-field-id, :table_id table-id, :semantic_type (mdb.u/isa :type/Name)))))) + (when-let [table-id (t2/select-one-fn :table_id Field :id field-id, :semantic_type (mdb.query/isa :type/PK))] + (t2/exists? Field :id search-field-id, :table_id table-id, :semantic_type (mdb.query/isa :type/Name)))))) (defn- check-field-is-referenced-by-dashboard "Check that `field-id` belongs to a Field that is used as a parameter in a Dashboard with `dashboard-id`, or throw a diff --git a/src/metabase/api/testing.clj b/src/metabase/api/testing.clj index c6bf062846969b51b43841016228bd82e18ef74c..0f3ec29f663d5c9b8365708ff5643c3a78970d25 100644 --- a/src/metabase/api/testing.clj +++ b/src/metabase/api/testing.clj @@ -6,8 +6,7 @@ [compojure.core :refer [POST]] [metabase.api.common :as api] [metabase.config :as config] - [metabase.db.connection :as mdb.connection] - [metabase.db.setup :as mdb.setup] + [metabase.db :as mdb] [metabase.util.files :as u.files] [metabase.util.log :as log] [metabase.util.malli.schema :as ms]) @@ -32,10 +31,10 @@ ;;;; SAVE (defn- save-snapshot! [snapshot-name] - (assert-h2 mdb.connection/*application-db*) + (assert-h2 (mdb/app-db)) (let [path (snapshot-path-for-name snapshot-name)] (log/infof "Saving snapshot to %s" path) - (jdbc/query {:datasource mdb.connection/*application-db*} ["SCRIPT TO ?" path])) + (jdbc/query {:datasource (mdb/app-db)} ["SCRIPT TO ?" path])) :ok) (api/defendpoint POST "/snapshot/:name" @@ -50,7 +49,7 @@ (defn- reset-app-db-connection-pool! "Immediately destroy all open connections in the app DB connection pool." [] - (let [{:keys [data-source]} mdb.connection/*application-db*] + (let [data-source (mdb/data-source)] (when (instance? PoolBackedDataSource data-source) (log/info "Destroying application database connection pool") (.hardReset ^PoolBackedDataSource data-source)))) @@ -60,7 +59,7 @@ [^String snapshot-path] (log/infof "Restoring snapshot from %s" snapshot-path) (api/check-404 (.exists (java.io.File. snapshot-path))) - (with-open [conn (.getConnection mdb.connection/*application-db*)] + (with-open [conn (.getConnection (mdb/app-db))] (doseq [sql-args [["SET LOCK_TIMEOUT 180000"] ["DROP ALL OBJECTS"] ["RUNSCRIPT FROM ?" snapshot-path]]] @@ -89,26 +88,19 @@ ;; a bunch of time initializing Liquibase and checking for unrun migrations for every test when we don't need to. -- ;; Cam (when config/is-dev? - (mdb.setup/migrate! (mdb.connection/db-type) mdb.connection/*application-db* :up))) - -(defn- increment-app-db-unique-indentifier! - "Increment the [[mdb.connection/unique-identifier]] for the Metabase application DB. This effectively flushes all - caches using it as a key (including things using [[mdb.connection/memoize-for-application-db]]) such as the Settings - cache." - [] - (alter-var-root #'mdb.connection/*application-db* assoc :id (swap! mdb.connection/application-db-counter inc))) + (mdb/migrate! (mdb/db-type) (mdb/app-db) :up))) (defn- restore-snapshot! [snapshot-name] - (assert-h2 mdb.connection/*application-db*) + (assert-h2 (mdb/app-db)) (let [path (snapshot-path-for-name snapshot-name) - ^ReentrantReadWriteLock lock (:lock mdb.connection/*application-db*)] + ^ReentrantReadWriteLock lock (:lock (mdb/app-db))] ;; acquire the application DB WRITE LOCK which will prevent any other threads from getting any new connections until ;; we release it. (try (.. lock writeLock lock) (reset-app-db-connection-pool!) (restore-app-db-from-snapshot! path) - (increment-app-db-unique-indentifier!) + (mdb/increment-app-db-unique-indentifier!) (finally (.. lock writeLock unlock)))) :ok) diff --git a/src/metabase/cmd/copy.clj b/src/metabase/cmd/copy.clj index ef640e504cee6fdec9e2a7c00503b08d5ff25fcc..40738db62ea2e24e7fb5c7966918666e3eb4749c 100644 --- a/src/metabase/cmd/copy.clj +++ b/src/metabase/cmd/copy.clj @@ -8,7 +8,7 @@ [clojure.java.jdbc :as jdbc] [honey.sql :as sql] [metabase.config :as config] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.setup :as mdb.setup] [metabase.plugins.classloader :as classloader] [metabase.util :as u] @@ -111,7 +111,7 @@ ;; should be ok now that #16344 is resolved -- we might be able to remove this code entirely now. Quoting identifiers ;; is still a good idea tho.) (let [source-keys (keys (first objs)) - quote-fn (partial mdb.setup/quote-for-application-db (mdb.connection/quoting-style target-db-type)) + quote-fn (partial mdb.setup/quote-for-application-db (mdb/quoting-style target-db-type)) dest-keys (for [k source-keys] (quote-fn (name k)))] {:cols dest-keys diff --git a/src/metabase/cmd/dump_to_h2.clj b/src/metabase/cmd/dump_to_h2.clj index 136bdbe6312944729f63532e724f866eb038c76c..57631e2abdf7f05c211509b397a3521416e992e2 100644 --- a/src/metabase/cmd/dump_to_h2.clj +++ b/src/metabase/cmd/dump_to_h2.clj @@ -16,6 +16,7 @@ [metabase.cmd.copy :as copy] [metabase.cmd.copy.h2 :as copy.h2] [metabase.cmd.rotate-encryption-key :as rotate-encryption] + [metabase.db :as mdb] [metabase.db.connection :as mdb.connection] [metabase.util.log :as log])) @@ -36,7 +37,7 @@ (log/infof "Dumping from configured Metabase db to H2 file %s" h2-filename) (when-not keep-existing? (copy.h2/delete-existing-h2-database-files! h2-filename)) - (copy/copy! (mdb.connection/db-type) (mdb.connection/data-source) :h2 h2-data-source) + (copy/copy! (mdb/db-type) (mdb/data-source) :h2 h2-data-source) (when dump-plaintext? (binding [mdb.connection/*application-db* (mdb.connection/application-db :h2 h2-data-source)] (rotate-encryption/rotate-encryption-key! nil)))))) diff --git a/src/metabase/cmd/load_from_h2.clj b/src/metabase/cmd/load_from_h2.clj index 09ae03b52175f13d1fcc8e3d94beb70cc12ec7c0..525498d132b7be0fbb2dea8fd9e25b0741887805 100644 --- a/src/metabase/cmd/load_from_h2.clj +++ b/src/metabase/cmd/load_from_h2.clj @@ -20,8 +20,7 @@ (:require [metabase.cmd.copy :as copy] [metabase.cmd.copy.h2 :as copy.h2] - [metabase.db.connection :as mdb.connection] - [metabase.db.env :as mdb.env])) + [metabase.db :as mdb])) (defn load-from-h2! "Transfer data from existing H2 database to a newly created (presumably MySQL or Postgres) DB. Intended as a tool for @@ -29,8 +28,8 @@ Defaults to using [[metabase.db.env/db-file]] as the source H2 database if `h2-filename` is `nil`." ([] - (load-from-h2! (mdb.env/db-file))) + (load-from-h2! (mdb/db-file))) ([h2-filename] (let [h2-filename (str h2-filename ";IFEXISTS=TRUE") h2-data-source (copy.h2/h2-data-source h2-filename)] - (copy/copy! :h2 h2-data-source (mdb.connection/db-type) (mdb.connection/data-source))))) + (copy/copy! :h2 h2-data-source (mdb/db-type) (mdb/data-source))))) diff --git a/src/metabase/cmd/migrate.clj b/src/metabase/cmd/migrate.clj index 7a0fe7713619886f131528f34d6bd348ef0e8bf8..97677d167ca3e5bc728c96b9aaccf7b353905d4f 100644 --- a/src/metabase/cmd/migrate.clj +++ b/src/metabase/cmd/migrate.clj @@ -1,9 +1,8 @@ (ns metabase.cmd.migrate (:require - [metabase.db.connection :as mdb.connection] - [metabase.db.setup :as mdb.setup])) + [metabase.db :as mdb])) (defn migrate! "Migrate the Metabase application DB." [direction] - (mdb.setup/migrate! (mdb.connection/db-type) (mdb.connection/data-source) (keyword direction))) + (mdb/migrate! (mdb/db-type) (mdb/data-source) (keyword direction))) diff --git a/src/metabase/cmd/rotate_encryption_key.clj b/src/metabase/cmd/rotate_encryption_key.clj index 86b8b3c36b2ca32d428d706b1dc5dafe0a24b72e..13ccc6ddf52dd8e532a517d6f9f130fd93e93855 100644 --- a/src/metabase/cmd/rotate_encryption_key.clj +++ b/src/metabase/cmd/rotate_encryption_key.clj @@ -2,8 +2,6 @@ (:require [cheshire.core :as json] [metabase.db :as mdb] - [metabase.db.connection :as mdb.connection] - [metabase.db.env :as mdb.env] [metabase.models :refer [Database Secret Setting]] [metabase.models.setting.cache :as setting.cache] [metabase.util.encryption :as encryption] @@ -17,14 +15,14 @@ (when-not (mdb/db-is-set-up?) (log/warnf "Database not found. Metabase will create a new database at %s and proceeed encrypting." "2") (mdb/setup-db!)) - (log/infof "%s: %s | %s" (trs "Connected to") mdb.env/db-type (mdb.env/db-file)) + (log/infof "%s: %s | %s" (trs "Connected to") (mdb/db-type) (mdb/db-file)) (let [make-encrypt-fn (fn [maybe-encrypt-fn] (if to-key (partial maybe-encrypt-fn (encryption/validate-and-hash-secret-key to-key)) identity)) encrypt-str-fn (make-encrypt-fn encryption/maybe-encrypt) encrypt-bytes-fn (make-encrypt-fn encryption/maybe-encrypt-bytes)] - (t2/with-transaction [t-conn {:datasource (mdb.connection/data-source)}] + (t2/with-transaction [t-conn {:datasource (mdb/data-source)}] (doseq [[id details] (t2/select-pk->fn :details Database)] (when (encryption/possibly-encrypted-string? details) (throw (ex-info (trs "Can''t decrypt app db with MB_ENCRYPTION_SECRET_KEY") {:database-id id}))) diff --git a/src/metabase/config.clj b/src/metabase/config.clj index 8a823a6cd138b94bbc3e33d2ed6e2533cd58b583..75be834396ae9f966b4f084343031d0690b90fd9 100644 --- a/src/metabase/config.clj +++ b/src/metabase/config.clj @@ -153,3 +153,7 @@ "The user-id of the internal metabase user. This is needed in the OSS edition to filter out users for setup/has-user-setup." 13371338) + +(def ^:dynamic *disable-setting-cache* + "Whether to disable database cache. Here for loading circularity reasons." + false) diff --git a/src/metabase/db.clj b/src/metabase/db.clj index d50c844b359aa0f3b789926e350a478dbbb41ede..d436d0e5ca027070a2b8945e1816f950a48f1692 100644 --- a/src/metabase/db.clj +++ b/src/metabase/db.clj @@ -1,49 +1,42 @@ (ns metabase.db - "High-level functions for setting up the Metabase application database. Additional functions can be found in - sub-namespaces: + "API namespace for the application database (app-db). - * [[metabase.db.connection]] - functions for getting the application database type (e.g. `:h2`) and a - [[clojure.java.jdbc]] spec for it; dynamic variable for rebinding it + It has a few different functions you might need: + - A connectible for the app-db [[app-db]] and [[data-source]] + - Information about the app-db: [[db-is-set-up?]], [[db-type]], [[quoting-style]] + - a few other random functions that have built up for different purposes. - * [[metabase.db.connection-pool-setup]] - functions for creating a connection pool for the application database - - * [[metabase.db.custom-migrations]] - Clojure-land data migration definitions and functions for running them - - * [[metabase.db.data-source]] - Implementations of [[javax.sql.DataSource]] for raw connection strings and - broken-out db details. See [[metabase.db.env/broken-out-details]] for more details about what 'broken-out details' - means. - - * [[metabase.db.env]] - functions for getting application database connection information from environment variables - - * [[metabase.db.jdbc-protocols]] - implementations of [[clojure.java.jdbc]] protocols for the Metabase application - database - - * [[metabase.db.liquibase]] - high-level Clojure wrapper around relevant parts of the Liquibase API - - * [[metabase.db.setup]] - code related to setting up the application DB -- verifying the connection and running - migrations -- and for setting it up as the default Toucan connection - - * [[metabase.db.spec]] - util functions for creating JDBC specs for supported application DB types from connection - details maps - - * [[metabase.db.util]] - general util functions for Toucan/HoneySQL queries against the application DB" + Namespaces outside of src/metabase/db/ should not use any metabase.db.* namespace but use this api namespace." (:require - [clojure.core.async.impl.dispatch :as a.impl.dispatch] [metabase.config :as config] [metabase.db.connection :as mdb.connection] + [metabase.db.connection-pool-setup :as mdb.connection-pool-setup] + [metabase.db.env :as mdb.env] [metabase.db.setup :as mdb.setup] - [methodical.core :as methodical] - [potemkin :as p] - [toucan2.pipeline :as t2.pipeline])) + [metabase.db.spec :as mdb.spec] + [potemkin :as p])) + +(set! *warn-on-reflection* true) -;; TODO - determine if we *actually* need to import any of these -;; -;; These are mostly here as a convenience to avoid having to rework a bunch of existing code. It's better to use these -;; functions directly where applicable. (p/import-vars [mdb.connection + quoting-style db-type - quoting-style]) + unique-identifier + data-source] + + [mdb.connection-pool-setup + recent-activity?] + + [mdb.env + db-file] + + [mdb.setup + migrate!] + + [mdb.spec + make-subname + spec]) ;; TODO -- consider whether we can just do this automatically when `getConnection` is called on ;; [[mdb.connection/*application-db*]] (or its data source) @@ -52,6 +45,12 @@ [] (= @(:status mdb.connection/*application-db*) ::setup-finished)) +(defn app-db + "The Application database. A record, but use accessors [[db-type]], [[data-source]], etc to access. Also + implements [[javax.sql.DataSource]] directly, so you can call [[.getConnection]] on it directly." + ^metabase.db.connection.ApplicationDB [] + mdb.connection/*application-db*) + (defn setup-db! "Do general preparation of database by validating that we can connect. Caller can specify if we should run any pending database migrations. If DB is already set up, this function will no-op. Thread-safe." @@ -63,18 +62,30 @@ ;; DBs. (locking mdb.connection/*application-db* (when-not (db-is-set-up?) - (let [db-type (mdb.connection/db-type) - data-source (mdb.connection/data-source) + (let [db-type (db-type) + data-source (data-source) auto-migrate? (config/config-bool :mb-db-automigrate)] (mdb.setup/setup-db! db-type data-source auto-migrate?)) (reset! (:status mdb.connection/*application-db*) ::setup-finished)))) :done) -(methodical/defmethod t2.pipeline/transduce-query :before :default - "Make sure application database calls are not done inside core.async dispatch pool threads. This is done relatively - early in the pipeline so the stacktrace when this fails isn't super enormous." - [_rf _query-typeâ‚ _modelâ‚‚ _parsed-args resolved-query] - (when (a.impl.dispatch/in-dispatch-thread?) - (throw (ex-info "Application database calls are not allowed inside core.async dispatch pool threads." - {}))) - resolved-query) +(defn memoize-for-application-db + "Like [[clojure.core/memoize]], but only memoizes for the current application database; memoized values will be + ignored if the app DB is dynamically rebound. For TTL memoization with [[clojure.core.memoize]], set + `:clojure.core.memoize/args-fn` instead; see [[metabase.driver.util/database->driver*]] for an example of how to do + this." + [f] + (let [f* (memoize (fn [_application-db-id & args] + (apply f args)))] + (fn [& args] + (apply f* (unique-identifier) args)))) + + +(defn increment-app-db-unique-indentifier! + "Increment the [[unique-identifier]] for the Metabase application DB. This effectively flushes all caches using it as + a key (including things using [[mdb/memoize-for-application-db]]) such as the Settings cache. Should only be used + for testing. Not general purpose." + [] + (assert (or (not config/is-prod?) + (config/config-bool :mb-enable-test-endpoints))) + (alter-var-root #'mdb.connection/*application-db* assoc :id (swap! mdb.connection/application-db-counter inc))) diff --git a/src/metabase/db/connection.clj b/src/metabase/db/connection.clj index 8a86999a5e168c870cb2f3eb7cc7ebd51fbd5194..8b14853aabdc0927b40576beaeaf031b1e6bcb2f 100644 --- a/src/metabase/db/connection.clj +++ b/src/metabase/db/connection.clj @@ -1,13 +1,14 @@ (ns metabase.db.connection - "Functions for getting the application database connection type and JDBC spec, or temporarily overriding them. - TODO - consider renaming this namespace `metabase.db.config`." + "Functions for getting the application database connection type and JDBC spec, or temporarily overriding them." (:require + [clojure.core.async.impl.dispatch :as a.impl.dispatch] [metabase.db.connection-pool-setup :as connection-pool-setup] [metabase.db.env :as mdb.env] [methodical.core :as methodical] [potemkin :as p] [toucan2.connection :as t2.conn] - [toucan2.jdbc.connection :as t2.jdbc.conn]) + [toucan2.jdbc.connection :as t2.jdbc.conn] + [toucan2.pipeline :as t2.pipeline]) (:import (java.util.concurrent.locks ReentrantReadWriteLock))) @@ -126,17 +127,6 @@ [] (.id *application-db*)) -(defn memoize-for-application-db - "Like [[clojure.core/memoize]], but only memoizes for the current application database; memoized values will be - ignored if the app DB is dynamically rebound. For TTL memoization with [[clojure.core.memoize]], set - `:clojure.core.memoize/args-fn` instead; see [[metabase.driver.util/database->driver*]] for an example of how to do - this." - [f] - (let [f* (memoize (fn [_application-db-id & args] - (apply f args)))] - (fn [& args] - (apply f* (unique-identifier) args)))) - (methodical/defmethod t2.conn/do-with-connection :default [_connectable f] (t2.conn/do-with-connection *application-db* f)) @@ -197,3 +187,13 @@ :else (binding [*transaction-depth* (inc *transaction-depth*)] (do-transaction connection f)))) + + +(methodical/defmethod t2.pipeline/transduce-query :before :default + "Make sure application database calls are not done inside core.async dispatch pool threads. This is done relatively + early in the pipeline so the stacktrace when this fails isn't super enormous." + [_rf _query-typeâ‚ _modelâ‚‚ _parsed-args resolved-query] + (when (a.impl.dispatch/in-dispatch-thread?) + (throw (ex-info "Application database calls are not allowed inside core.async dispatch pool threads." + {}))) + resolved-query) diff --git a/src/metabase/db/data_source.clj b/src/metabase/db/data_source.clj index 5364a7275038046a373bce3d555cfc72139a22a0..de31b16294319cc638d0c4b22d72ebe0b31962ed 100644 --- a/src/metabase/db/data_source.clj +++ b/src/metabase/db/data_source.clj @@ -1,4 +1,6 @@ (ns metabase.db.data-source + "A namespace to define a record holding a connection to the application database. The [[DataSource]] type + implements [[javax.sql.DataSource]] so you can call [[getConnection]] on it." (:require [clojure.set :as set] [clojure.string :as str] diff --git a/src/metabase/db/query.clj b/src/metabase/db/query.clj index 3253d3a0bcafcc5725577c164746e2f21d956296..f313386b08718743ba7e78def48263f92a3fc94a 100644 --- a/src/metabase/db/query.clj +++ b/src/metabase/db/query.clj @@ -22,19 +22,80 @@ (:require [clojure.string :as str] [honey.sql :as sql] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.plugins.classloader :as classloader] + [metabase.util :as u] [metabase.util.log :as log] + [metabase.util.malli :as mu] + [metabase.util.malli.schema :as ms] [toucan2.core :as t2] - [toucan2.jdbc.options :as t2.jdbc.options])) + [toucan2.jdbc.options :as t2.jdbc.options] + [toucan2.model :as t2.model]) + (:import + (clojure.lang ExceptionInfo))) (set! *warn-on-reflection* true) (defn format-sql "Return a nicely-formatted version of a `query` string with the current application db driver formatting." [sql] - (driver/prettify-native-form (mdb.connection/db-type) sql)) + (driver/prettify-native-form (mdb/db-type) sql)) + +(def ^:private NamespacedKeyword + [:and :keyword [:fn (comp seq namespace)]]) + +(mu/defn ^:private type-keyword->descendants :- [:set {:min 1} ms/NonBlankString] + "Return a set of descendents of Metabase `type-keyword`. This includes `type-keyword` itself, so the set will always + have at least one element. + + (type-keyword->descendants :Semantic/Coordinate) ; -> #{\"type/Latitude\" \"type/Longitude\" \"type/Coordinate\"}" + [type-keyword :- NamespacedKeyword] + (set (map u/qualified-name (cons type-keyword (descendants type-keyword))))) + +(defn isa + "Convenience for generating an HoneySQL `IN` clause for a keyword and all of its descendents. + Intended for use with the type hierarchy in `metabase.types`. + + (t2/select Field :semantic_type (mdb/isa :type/URL)) + -> + (t2/select Field :semantic_type [:in #{\"type/URL\" \"type/ImageURL\" \"type/AvatarURL\"}]) + + Also accepts optional `expr` for use directly in a HoneySQL `where`: + + (t2/select Field {:where (mdb/isa :semantic_type :type/URL)}) + -> + (t2/select Field {:where [:in :semantic_type #{\"type/URL\" \"type/ImageURL\" \"type/AvatarURL\"}]})" + ([type-keyword] + [:in (type-keyword->descendants type-keyword)]) + ;; when using this with an `expr` (e.g. `(isa :semantic_type :type/URL)`) just go ahead and take the results of the + ;; one-arity impl above and splice expr in as the second element + ;; + ;; [:in #{"type/URL" "type/ImageURL"}] + ;; + ;; becomes + ;; + ;; [:in :semantic_type #{"type/URL" "type/ImageURL"}] + ([expr type-keyword] + [:in expr (type-keyword->descendants type-keyword)])) + +(defn qualify + "Returns a qualified field for [modelable] with [field-name]." + ^clojure.lang.Keyword [modelable field-name] + (if (vector? field-name) + [(qualify modelable (first field-name)) (second field-name)] + (let [model (t2.model/resolve-model modelable)] + (keyword (str (name (t2.model/table-name model)) \. (name field-name)))))) + +(defn join + "Convenience for generating a HoneySQL `JOIN` clause. + + (t2/select-pks-set FieldValues + (mdb/join [FieldValues :field_id] [Field :id]) + :active true)" + [[source-entity fk] [dest-entity pk]] + {:left-join [(t2/table-name (t2.model/resolve-model dest-entity)) + [:= (qualify source-entity fk) (qualify dest-entity pk)]]}) (defmulti compile "Compile a `query` (e.g. a Honey SQL map) to `[sql & args]`." @@ -114,3 +175,87 @@ (reduce [_this rf init] (binding [t2.jdbc.options/*options* (merge t2.jdbc.options/*options* jdbc-options)] (reduce rf init (t2/reducible-query sql-args))))))) + + +(defmacro with-conflict-retry + "Retry a database mutation a single time if it fails due to concurrent insertions. + May retry for other reasons." + [& body] + `(try + ~@body + (catch ExceptionInfo e# + ;; The underlying exception thrown by the driver is database specific and opaque, so we treat any exception as a + ;; possible database conflict due to a concurrent insert. If we want to be more conservative, we would need + ;; a per-driver or driver agnostic way to test the exception. + ~@body))) + +(defn select-or-insert! + "Return a database record if it exists, otherwise create it. + + The `select-map` is used to query the `model`, and if a result is found it is immediately returned. + If no value is found, `insert-fn` is called to generate the entity to be inserted. + + Note that this generated entity must be consistent with `select-map`, if it disagrees on any keys then an exception + will be thrown. It is OK for the entity to omit fields from `select-map`, they will implicitly be added on. + + This is more general than using `UPSERT`, `MERGE` or `INSERT .. ON CONFLICT`, and it also allows one to avoid + calculating initial values that may be expensive, or require side effects. + + In the case where there is an underlying db constraint to prevent duplicates, this method takes care of handling + rejection from the database due to a concurrent insert, and will retry a single time to pick up the existing row. + This may result in `insert-fn` being called a second time. + + In the case where there is no underlying db constraint, concurrent calls may still result in duplicates. + To prevent this in a database agnostic way, during an existing non-serializable transaction, would be non-trivial." + [model select-map insert-fn] + (let [select-kvs (mapcat identity select-map) + insert-fn #(let [instance (insert-fn)] + ;; the inserted values must be consistent with the select query + (assert (not (u/conflicting-keys? select-map instance)) + "this should not be used to change any of the identifying values") + ;; for convenience, we allow insert-fn's result to omit fields in the search-map + (merge instance select-map))] + (with-conflict-retry + (or (apply t2/select-one model select-kvs) + (t2/insert-returning-instance! model (insert-fn)))))) + +(defn update-or-insert! + "Update a database record, if it exists, otherwise create it. + + The `select-map` is used to query the `model`, and if a result is found then we will update that entity, otherwise + a new entity will be created. We use `update-fn` to calculate both updates and initial values - in the first case + it will be called with the existing value, and in the second case it will be called with nil, analogous to the way + that [[clojure.core/update]] calls its function. + + Note that the generated entity must be consistent with `select-map`, if it disagrees on any keys then an exception + will be thrown. It is OK for the entity to omit fields from `select-map`, they will implicitly be added on. + + This is more general than using `UPSERT`, `MERGE` or `INSERT .. ON CONFLICT`, and it also allows one to avoid + calculating initial values that may be expensive, or require side effects. + + In the case where there is an underlying db constraint to prevent duplicates, this method takes care of handling + rejection from the database due to a concurrent insert, and will retry a single time to pick up the existing row. + This may result in `update-fn` being called a second time. + + In the case where there is no underlying db constraint, concurrent calls may still result in duplicates. + To prevent this in a database agnostic way, during an existing non-serializable transaction, would be non-trivial." + [model select-map update-fn] + (let [select-kvs (mapcat identity select-map) + pks (t2/primary-keys model) + _ (assert (= 1 (count pks)) "This helper does not currently support compound keys") + pk-key (keyword (first pks)) + update-fn (fn [existing] + (let [updated (update-fn existing)] + ;; the inserted / updated values must be consistent with the select query + (assert (not (u/conflicting-keys? select-map updated)) + "This should not be used to change any of the identifying values") + ;; For convenience, we allow the update-fn to omit fields in the search-map + (merge updated select-map)))] + (with-conflict-retry + (if-let [existing (apply t2/select-one model select-kvs)] + (let [pk (pk-key existing) + updated (update-fn existing)] + (t2/update! model pk updated) + ;; the private key may have been changed by the update, and this is OK. + (pk-key updated pk)) + (t2/insert-returning-pk! model (update-fn nil)))))) diff --git a/src/metabase/db/setup.clj b/src/metabase/db/setup.clj index 74ff1b79aec72af92e06315420b6017393fa18f8..9acd4e2cf3692d291458b44fd560f9c655145497 100644 --- a/src/metabase/db/setup.clj +++ b/src/metabase/db/setup.clj @@ -8,12 +8,11 @@ DB setup steps on arbitrary databases -- useful for functionality like the `load-from-h2` or `dump-to-h2` commands." (:require [honey.sql :as sql] + [metabase.config :as config] [metabase.db.connection :as mdb.connection] [metabase.db.custom-migrations] [metabase.db.jdbc-protocols :as mdb.jdbc-protocols] [metabase.db.liquibase :as liquibase] - [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] - [metabase.models.setting :as setting] [metabase.plugins.classloader :as classloader] [metabase.util :as u] [metabase.util.honey-sql-2] @@ -112,9 +111,9 @@ [db-type :- :keyword data-source :- (ms/InstanceOfClass javax.sql.DataSource)] (log/info (u/format-color 'cyan (trs "Verifying {0} Database Connection ..." (name db-type)))) - (classloader/require 'metabase.driver.util) + (classloader/require 'metabase.driver.sql-jdbc.connection) (let [error-msg (trs "Unable to connect to Metabase {0} DB." (name db-type))] - (try (assert (sql-jdbc.conn/can-connect-with-spec? {:datasource data-source}) error-msg) + (try (assert ((requiring-resolve 'metabase.driver.sql-jdbc.connection/can-connect-with-spec?) {:datasource data-source}) error-msg) (catch Throwable e (throw (ex-info error-msg {} e))))) (with-open [conn (.getConnection ^javax.sql.DataSource data-source)] @@ -164,8 +163,8 @@ auto-migrate? :- [:maybe :boolean]] (u/profile (trs "Database setup") (u/with-us-locale - (binding [mdb.connection/*application-db* (mdb.connection/application-db db-type data-source :create-pool? false) ; should already be a pool - setting/*disable-cache* true] + (binding [mdb.connection/*application-db* (mdb.connection/application-db db-type data-source :create-pool? false) ; should already be a pool + config/*disable-setting-cache* true] (verify-db-connection db-type data-source) (error-if-downgrade-required! data-source) (run-schema-migrations! db-type data-source auto-migrate?)))) diff --git a/src/metabase/db/util.clj b/src/metabase/db/util.clj deleted file mode 100644 index 211e8d0e46b2c59f3eaf38ec74a6bcbc273c7f1e..0000000000000000000000000000000000000000 --- a/src/metabase/db/util.clj +++ /dev/null @@ -1,153 +0,0 @@ -(ns metabase.db.util - "Utility functions for querying the application database." - (:require - [metabase.util :as u] - [metabase.util.malli :as mu] - [metabase.util.malli.schema :as ms] - [toucan2.core :as t2] - [toucan2.model :as t2.model]) - (:import - (clojure.lang ExceptionInfo))) - -(defn toucan-model? - "Check if `model` is a toucan model." - [model] - (isa? model :metabase/model)) - -(defn qualify - "Returns a qualified field for [modelable] with [field-name]." - ^clojure.lang.Keyword [modelable field-name] - (if (vector? field-name) - [(qualify modelable (first field-name)) (second field-name)] - (let [model (t2.model/resolve-model modelable)] - (keyword (str (name (t2.model/table-name model)) \. (name field-name)))))) - -(defn join - "Convenience for generating a HoneySQL `JOIN` clause. - - (t2/select-pks-set FieldValues - (mdb/join [FieldValues :field_id] [Field :id]) - :active true)" - [[source-entity fk] [dest-entity pk]] - {:left-join [(t2/table-name (t2.model/resolve-model dest-entity)) - [:= (qualify source-entity fk) (qualify dest-entity pk)]]}) - -(def ^:private NamespacedKeyword - [:and :keyword [:fn (comp seq namespace)]]) - -(mu/defn ^:private type-keyword->descendants :- [:set {:min 1} ms/NonBlankString] - "Return a set of descendents of Metabase `type-keyword`. This includes `type-keyword` itself, so the set will always - have at least one element. - - (type-keyword->descendants :Semantic/Coordinate) ; -> #{\"type/Latitude\" \"type/Longitude\" \"type/Coordinate\"}" - [type-keyword :- NamespacedKeyword] - (set (map u/qualified-name (cons type-keyword (descendants type-keyword))))) - -(defn isa - "Convenience for generating an HoneySQL `IN` clause for a keyword and all of its descendents. - Intended for use with the type hierarchy in `metabase.types`. - - (t2/select Field :semantic_type (mdb/isa :type/URL)) - -> - (t2/select Field :semantic_type [:in #{\"type/URL\" \"type/ImageURL\" \"type/AvatarURL\"}]) - - Also accepts optional `expr` for use directly in a HoneySQL `where`: - - (t2/select Field {:where (mdb/isa :semantic_type :type/URL)}) - -> - (t2/select Field {:where [:in :semantic_type #{\"type/URL\" \"type/ImageURL\" \"type/AvatarURL\"}]})" - ([type-keyword] - [:in (type-keyword->descendants type-keyword)]) - ;; when using this with an `expr` (e.g. `(isa :semantic_type :type/URL)`) just go ahead and take the results of the - ;; one-arity impl above and splice expr in as the second element - ;; - ;; [:in #{"type/URL" "type/ImageURL"}] - ;; - ;; becomes - ;; - ;; [:in :semantic_type #{"type/URL" "type/ImageURL"}] - ([expr type-keyword] - [:in expr (type-keyword->descendants type-keyword)])) - -(defmacro with-conflict-retry - "Retry a database mutation a single time if it fails due to concurrent insertions. - May retry for other reasons." - [& body] - `(try - ~@body - (catch ExceptionInfo e# - ;; The underlying exception thrown by the driver is database specific and opaque, so we treat any exception as a - ;; possible database conflict due to a concurrent insert. If we want to be more conservative, we would need - ;; a per-driver or driver agnostic way to test the exception. - ~@body))) - -(defn select-or-insert! - "Return a database record if it exists, otherwise create it. - - The `select-map` is used to query the `model`, and if a result is found it is immediately returned. - If no value is found, `insert-fn` is called to generate the entity to be inserted. - - Note that this generated entity must be consistent with `select-map`, if it disagrees on any keys then an exception - will be thrown. It is OK for the entity to omit fields from `select-map`, they will implicitly be added on. - - This is more general than using `UPSERT`, `MERGE` or `INSERT .. ON CONFLICT`, and it also allows one to avoid - calculating initial values that may be expensive, or require side effects. - - In the case where there is an underlying db constraint to prevent duplicates, this method takes care of handling - rejection from the database due to a concurrent insert, and will retry a single time to pick up the existing row. - This may result in `insert-fn` being called a second time. - - In the case where there is no underlying db constraint, concurrent calls may still result in duplicates. - To prevent this in a database agnostic way, during an existing non-serializable transaction, would be non-trivial." - [model select-map insert-fn] - (let [select-kvs (mapcat identity select-map) - insert-fn #(let [instance (insert-fn)] - ;; the inserted values must be consistent with the select query - (assert (not (u/conflicting-keys? select-map instance)) - "this should not be used to change any of the identifying values") - ;; for convenience, we allow insert-fn's result to omit fields in the search-map - (merge instance select-map))] - (with-conflict-retry - (or (apply t2/select-one model select-kvs) - (t2/insert-returning-instance! model (insert-fn)))))) - -(defn update-or-insert! - "Update a database record, if it exists, otherwise create it. - - The `select-map` is used to query the `model`, and if a result is found then we will update that entity, otherwise - a new entity will be created. We use `update-fn` to calculate both updates and initial values - in the first case - it will be called with the existing value, and in the second case it will be called with nil, analogous to the way - that [[clojure.core/update]] calls its function. - - Note that the generated entity must be consistent with `select-map`, if it disagrees on any keys then an exception - will be thrown. It is OK for the entity to omit fields from `select-map`, they will implicitly be added on. - - This is more general than using `UPSERT`, `MERGE` or `INSERT .. ON CONFLICT`, and it also allows one to avoid - calculating initial values that may be expensive, or require side effects. - - In the case where there is an underlying db constraint to prevent duplicates, this method takes care of handling - rejection from the database due to a concurrent insert, and will retry a single time to pick up the existing row. - This may result in `update-fn` being called a second time. - - In the case where there is no underlying db constraint, concurrent calls may still result in duplicates. - To prevent this in a database agnostic way, during an existing non-serializable transaction, would be non-trivial." - [model select-map update-fn] - (let [select-kvs (mapcat identity select-map) - pks (t2/primary-keys model) - _ (assert (= 1 (count pks)) "This helper does not currently support compound keys") - pk-key (keyword (first pks)) - update-fn (fn [existing] - (let [updated (update-fn existing)] - ;; the inserted / updated values must be consistent with the select query - (assert (not (u/conflicting-keys? select-map updated)) - "This should not be used to change any of the identifying values") - ;; For convenience, we allow the update-fn to omit fields in the search-map - (merge updated select-map)))] - (with-conflict-retry - (if-let [existing (apply t2/select-one model select-kvs)] - (let [pk (pk-key existing) - updated (update-fn existing)] - (t2/update! model pk updated) - ;; the private key may have been changed by the update, and this is OK. - (pk-key updated pk)) - (t2/insert-returning-pk! model (update-fn nil)))))) diff --git a/src/metabase/driver/h2.clj b/src/metabase/driver/h2.clj index 8d76795fb5af0a1840b12bb8bbf788c20843a91d..ec9836388afe303e35ac40ffe6e8aec9618e260c 100644 --- a/src/metabase/driver/h2.clj +++ b/src/metabase/driver/h2.clj @@ -4,8 +4,8 @@ [clojure.string :as str] [java-time.api :as t] [metabase.config :as config] + [metabase.db :as mdb] [metabase.db.jdbc-protocols :as mdb.jdbc-protocols] - [metabase.db.spec :as mdb.spec] [metabase.driver :as driver] [metabase.driver.common :as driver.common] [metabase.driver.h2.actions :as h2.actions] @@ -508,8 +508,8 @@ (defmethod sql-jdbc.conn/connection-details->spec :h2 [_ details] {:pre [(map? details)]} - (mdb.spec/spec :h2 (cond-> details - (string? (:db details)) (update :db connection-string-set-safe-options)))) + (mdb/spec :h2 (cond-> details + (string? (:db details)) (update :db connection-string-set-safe-options)))) (defmethod sql-jdbc.sync/active-tables :h2 [& args] diff --git a/src/metabase/driver/mysql.clj b/src/metabase/driver/mysql.clj index 6048e8423c903d5b868e6ba2f02367df7e6dcc84..10b9487e1982bd55c772198faa53b4964efa7236 100644 --- a/src/metabase/driver/mysql.clj +++ b/src/metabase/driver/mysql.clj @@ -10,7 +10,7 @@ [java-time.api :as t] [medley.core :as m] [metabase.config :as config] - [metabase.db.spec :as mdb.spec] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.driver.common :as driver.common] [metabase.driver.mysql.actions :as mysql.actions] @@ -551,7 +551,7 @@ (let [details (-> (if ssl-cert? (set/rename-keys details {:ssl-cert :serverSslCert}) details) (set/rename-keys {:dbname :db}) (dissoc :ssl))] - (-> (mdb.spec/spec :mysql details) + (-> (mdb/spec :mysql details) (maybe-add-program-name-option addl-opts-map) (sql-jdbc.common/handle-additional-options details)))))) diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj index ece69be6e07ba8b8b519c730db03a9cdf6cb754b..9073ef746d2d70f8ee48b65525e2c7b624c0145d 100644 --- a/src/metabase/driver/postgres.clj +++ b/src/metabase/driver/postgres.clj @@ -8,7 +8,7 @@ [clojure.walk :as walk] [honey.sql :as sql] [java-time.api :as t] - [metabase.db.spec :as mdb.spec] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.driver.common :as driver.common] [metabase.driver.postgres.actions :as postgres.actions] @@ -714,7 +714,7 @@ (merge disable-ssl-params props)) props (as-> props it (set/rename-keys it {:dbname :db}) - (mdb.spec/spec :postgres it) + (mdb/spec :postgres it) (sql-jdbc.common/handle-additional-options it details-map))] props)) diff --git a/src/metabase/driver/sql_jdbc/connection.clj b/src/metabase/driver/sql_jdbc/connection.clj index f04b88d778e0ded7239abcdb9529fe9d39fbf2d9..6337dde4f9007eba70b5a33c1ac190564fe9bb14 100644 --- a/src/metabase/driver/sql_jdbc/connection.clj +++ b/src/metabase/driver/sql_jdbc/connection.clj @@ -4,7 +4,7 @@ (:require [clojure.java.jdbc :as jdbc] [metabase.connection-pool :as connection-pool] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.lib.metadata :as lib.metadata] [metabase.lib.metadata.jvm :as lib.metadata.jvm] @@ -247,7 +247,7 @@ ;; connections with *application-db* and 1 less connection pool. Note: This data-source is ;; not in [[database-id->connection-pool]]. (:is-audit db) - {:datasource (mdb.connection/data-source)} + {:datasource (mdb/data-source)} (= ::not-found details) nil diff --git a/src/metabase/driver/util.clj b/src/metabase/driver/util.clj index 77a2e0d279580adcbf6b0261029d90fa8e8016e7..abfb1cf911236338b9fbee9ad0ee35040f099eeb 100644 --- a/src/metabase/driver/util.clj +++ b/src/metabase/driver/util.clj @@ -5,7 +5,7 @@ [clojure.set :as set] [clojure.string :as str] [metabase.config :as config] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.lib.metadata :as lib.metadata] [metabase.lib.metadata.protocols :as lib.metadata.protocols] @@ -111,8 +111,6 @@ (contains? message :message) (update :message str) (contains? message :errors) (update :errors update-vals str)))) -(comment mdb.connection/keep-me) ; used for [[memoize/ttl]] - ;; This is normally set via the env var `MB_DB_CONNECTION_TIMEOUT_MS` (defsetting db-connection-timeout-ms "Consider [[metabase.driver/can-connect?]] / [[can-connect-with-details?]] to have failed if they were not able to @@ -181,7 +179,7 @@ (qp.store/with-metadata-provider db-id (:engine (lib.metadata.protocols/database (qp.store/metadata-provider))))) (vary-meta assoc ::memoize/args-fn (fn [[db-id]] - [(mdb.connection/unique-identifier) db-id]))) + [(mdb/unique-identifier) db-id]))) :ttl/threshold 1000)) (mu/defn database->driver :- :keyword diff --git a/src/metabase/models/bookmark.clj b/src/metabase/models/bookmark.clj index 7d001c0fd5796e9879b80a5052c0045bf46f887d..6edc23c0339e3cea61b357153172d9b7f9bf980f 100644 --- a/src/metabase/models/bookmark.clj +++ b/src/metabase/models/bookmark.clj @@ -1,9 +1,8 @@ (ns metabase.models.bookmark (:require [clojure.string :as str] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.query :as mdb.query] - [metabase.db.util :as mdb.u] [metabase.models.card :as card :refer [Card]] [metabase.models.collection :refer [Collection]] [metabase.models.dashboard :refer [Dashboard]] @@ -68,7 +67,7 @@ (defn- bookmarks-union-query [user-id] - (let [as-null (when (= (mdb.connection/db-type) :postgres) (h2x/->integer nil))] + (let [as-null (when (= (mdb/db-type) :postgres) (h2x/->integer nil))] {:union-all [{:select [:card_id [as-null :dashboard_id] [as-null :collection_id] @@ -102,18 +101,18 @@ {:select [[:bookmark.created_at :created_at] [:bookmark.type :type] [:bookmark.item_id :item_id] - [:card.name (mdb.u/qualify Card :name)] - [:card.type (mdb.u/qualify Card :card_type)] - [:card.display (mdb.u/qualify Card :display)] - [:card.description (mdb.u/qualify Card :description)] - [:card.archived (mdb.u/qualify Card :archived)] - [:dashboard.name (mdb.u/qualify Dashboard :name)] - [:dashboard.description (mdb.u/qualify Dashboard :description)] - [:dashboard.archived (mdb.u/qualify Dashboard :archived)] - [:collection.name (mdb.u/qualify Collection :name)] - [:collection.authority_level (mdb.u/qualify Collection :authority_level)] - [:collection.description (mdb.u/qualify Collection :description)] - [:collection.archived (mdb.u/qualify Collection :archived)]] + [:card.name (mdb.query/qualify Card :name)] + [:card.type (mdb.query/qualify Card :card_type)] + [:card.display (mdb.query/qualify Card :display)] + [:card.description (mdb.query/qualify Card :description)] + [:card.archived (mdb.query/qualify Card :archived)] + [:dashboard.name (mdb.query/qualify Dashboard :name)] + [:dashboard.description (mdb.query/qualify Dashboard :description)] + [:dashboard.archived (mdb.query/qualify Dashboard :archived)] + [:collection.name (mdb.query/qualify Collection :name)] + [:collection.authority_level (mdb.query/qualify Collection :authority_level)] + [:collection.description (mdb.query/qualify Collection :description)] + [:collection.archived (mdb.query/qualify Collection :archived)]] :from [[(bookmarks-union-query user-id) :bookmark]] :left-join [[:report_card :card] [:= :bookmark.card_id :card.id] [:report_dashboard :dashboard] [:= :bookmark.dashboard_id :dashboard.id] @@ -128,7 +127,7 @@ (for [table [:card :dashboard :collection] :let [field (keyword (str (name table) "." "archived"))]] [:or [:= field false] [:= field nil]])) - :order-by [[:bookmark_ordering.ordering (case (mdb.connection/db-type) + :order-by [[:bookmark_ordering.ordering (case (mdb/db-type) ;; NULLS LAST is not supported by MySQL, but this is default ;; behavior for MySQL anyway (:postgres :h2) :asc-nulls-last diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj index 5a565b696e22f0ecc98d2f9b9d7c771af67ff211..0703dabeb27d7e170749ee3831013973a72ba72b 100644 --- a/src/metabase/models/collection.clj +++ b/src/metabase/models/collection.clj @@ -10,7 +10,7 @@ [metabase.api.common :as api :refer [*current-user-id* *current-user-permissions-set*]] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.models.collection.root :as collection.root] [metabase.models.interface :as mi] [metabase.models.permissions :as perms :refer [Permissions]] @@ -32,7 +32,6 @@ (set! *warn-on-reflection* true) (comment collection.root/keep-me) -(comment mdb.connection/keep-me) ;; for [[memoize/ttl]] (p/import-vars [collection.root root-collection root-collection-with-ui-details]) @@ -1128,7 +1127,7 @@ save a DB call for *every* API call." (memoize/ttl ^{::memoize/args-fn (fn [[user-id]] - [(mdb.connection/unique-identifier) user-id])} + [(mdb/unique-identifier) user-id])} (fn user->personal-collection-id* [user-id] (u/the-id (user->personal-collection user-id))) diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj index f345d806733336cb05ab0c2ebd8d092f76b73d2f..d767df76a6761372fa21a553739955eef310c60a 100644 --- a/src/metabase/models/database.clj +++ b/src/metabase/models/database.clj @@ -3,8 +3,8 @@ [medley.core :as m] [metabase.api.common :as api] [metabase.config :as config] - [metabase.db.connection :as mdb.connection] - [metabase.db.util :as mdb.u] + [metabase.db :as mdb] + [metabase.db.query :as mdb.query] [metabase.driver :as driver] [metabase.driver.impl :as driver.impl] [metabase.driver.util :as driver.u] @@ -318,7 +318,7 @@ [{:keys [id]}] (let [table-ids (t2/select-pks-set 'Table, :db_id id, :active true)] (when (seq table-ids) - (t2/select 'Field, :table_id [:in table-ids], :semantic_type (mdb.u/isa :type/PK))))) + (t2/select 'Field, :table_id [:in table-ids], :semantic_type (mdb.query/isa :type/PK))))) ;;; -------------------------------------------------- JSON Encoder -------------------------------------------------- @@ -423,7 +423,7 @@ (def ^{:arglists '([table-id])} table-id->database-id "Retrieve the `Database` ID for the given table-id." - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn [table-id] {:pre [(integer? table-id)]} (t2/select-one-fn :db_id :model/Table, :id table-id)))) diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj index ae4e18acca03d06e5610932a28a556a5245a73ab..3307ff0dffa3f8018d80ffbeb02b2f789c3d2b41 100644 --- a/src/metabase/models/field.clj +++ b/src/metabase/models/field.clj @@ -4,7 +4,7 @@ [clojure.string :as str] [medley.core :as m] [metabase.api.common :as api] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.lib.field :as lib.field] [metabase.lib.metadata.jvm :as lib.metadata.jvm] [metabase.models.data-permissions :as data-perms] @@ -28,8 +28,6 @@ (set! *warn-on-reflection* true) -(comment mdb.connection/keep-me) ;; for [[memoize/ttl]] - ;;; ------------------------------------------------- Type Mappings -------------------------------------------------- (def visibility-types @@ -318,7 +316,7 @@ (def ^{:arglists '([field-id])} field-id->table-id "Return the ID of the Table this Field belongs to." - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn [field-id] {:pre [(integer? field-id)]} (t2/select-one-fn :table_id Field, :id field-id)))) diff --git a/src/metabase/models/field_values.clj b/src/metabase/models/field_values.clj index 31e2af5a3a6d15c0e49dfef3e45822e9ea19a2c7..9b9eb8fc464408cc811fa642503b34c40e5b6261 100644 --- a/src/metabase/models/field_values.clj +++ b/src/metabase/models/field_values.clj @@ -27,7 +27,7 @@ [java-time.api :as t] [malli.core :as mc] [medley.core :as m] - [metabase.db.util :as mdb.u] + [metabase.db.query :as mdb.query] [metabase.models.interface :as mi] [metabase.models.serialization :as serdes] [metabase.plugins.classloader :as classloader] @@ -444,7 +444,7 @@ unwrapped-values (do (log/debug (trs "Storing FieldValues for Field {0}..." field-name)) - (mdb.u/select-or-insert! FieldValues {:field_id (u/the-id field), :type :full} + (mdb.query/select-or-insert! FieldValues {:field_id (u/the-id field), :type :full} (constantly {:has_more_values has_more_values :values values :human_readable_values human-readable-values})) diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj index fea7649da50d8c53f364d9993add02002437bfc3..df3744f7d78121d7344110ae0cd0558863c1ee0d 100644 --- a/src/metabase/models/interface.clj +++ b/src/metabase/models/interface.clj @@ -8,7 +8,6 @@ [clojure.walk :as walk] [malli.core :as mc] [malli.error :as me] - [metabase.db.connection :as mdb.connection] [metabase.mbql.normalize :as mbql.normalize] [metabase.mbql.schema :as mbql.s] [metabase.models.dispatch :as models.dispatch] @@ -380,7 +379,8 @@ max (nanosecond) resolution)." [] (classloader/require 'metabase.driver.sql.query-processor) - ((resolve 'metabase.driver.sql.query-processor/current-datetime-honeysql-form) (mdb.connection/db-type))) + (let [db-type ((requiring-resolve 'metabase.db/db-type))] + ((resolve 'metabase.driver.sql.query-processor/current-datetime-honeysql-form) db-type))) (defn- add-created-at-timestamp [obj & _] (cond-> obj diff --git a/src/metabase/models/params.clj b/src/metabase/models/params.clj index fff07e29e36eb71453c246b457448db0ef0137af..266784ab81ac224d3fffb2c3e975a1d569ce68e9 100644 --- a/src/metabase/models/params.clj +++ b/src/metabase/models/params.clj @@ -13,7 +13,7 @@ [clojure.set :as set] [malli.core :as mc] [medley.core :as m] - [metabase.db.util :as mdb.u] + [metabase.db.query :as mdb.query] [metabase.mbql.normalize :as mbql.normalize] [metabase.mbql.schema :as mbql.s] [metabase.mbql.util :as mbql.u] @@ -147,7 +147,7 @@ (when-let [table-ids (seq (map :table_id fields))] (m/index-by :table_id (-> (t2/select Field:params-columns-only :table_id [:in table-ids] - :semantic_type (mdb.u/isa :type/Name)) + :semantic_type (mdb.query/isa :type/Name)) ;; run [[metabase.lib.field/infer-has-field-values]] on these Fields so their values of ;; `has_field_values` will be consistent with what the FE expects. (e.g. we'll return ;; `:list` instead of `:auto-list`.) diff --git a/src/metabase/models/params/chain_filter.clj b/src/metabase/models/params/chain_filter.clj index 98bd4d495c6d34508643957b7654aeaab6a6091d..5f2838ab9aaa95ddd4ece513f61e6ae895496cff 100644 --- a/src/metabase/models/params/chain_filter.clj +++ b/src/metabase/models/params/chain_filter.clj @@ -66,9 +66,8 @@ [clojure.set :as set] [clojure.string :as str] [honey.sql :as sql] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.query :as mdb.query] - [metabase.db.util :as mdb.u] [metabase.driver.common.parameters.dates :as params.dates] [metabase.mbql.util :as mbql.u] [metabase.models :refer [Field FieldValues Table]] @@ -92,9 +91,6 @@ ;; so the hydration method for name_field is loaded (comment params/keep-me) -;; for [[memoize/ttl]] keys -(comment mdb.connection/keep-me) - (def Constraint "Schema for a constraint on a field." [:map @@ -122,7 +118,7 @@ the DB too much since this is unlike to change often, if ever." (memoize/ttl ^{::memoize/args-fn (fn [[field-id]] - [(mdb.connection/unique-identifier) field-id])} + [(mdb/unique-identifier) field-id])} (fn [field-id] (types/temporal-field? (t2/select-one [Field :base_type :semantic_type] :id field-id))) :ttl/threshold (u/minutes->ms 10))) @@ -232,7 +228,7 @@ the implementation of `find-joins` below." (memoize/ttl ^{::memoize/args-fn (fn [[database-id enable-reverse-joins?]] - [(mdb.connection/unique-identifier) database-id enable-reverse-joins?])} + [(mdb/unique-identifier) database-id enable-reverse-joins?])} database-fk-relationships* :ttl/threshold find-joins-cache-duration-ms)) @@ -296,7 +292,7 @@ :rhs {:table <country>, :field <country.id>}}]" (let [f (memoize/ttl ^{::memoize/args-fn (fn [[database-id source-table-id other-table-id enable-reverse-joins?]] - [(mdb.connection/unique-identifier) + [(mdb/unique-identifier) database-id source-table-id other-table-id @@ -315,7 +311,7 @@ (def ^:private ^{:arglists '([source-table other-table-ids enable-reverse-joins?])} find-all-joins* (memoize/ttl ^{::memoize/args-fn (fn [[source-table-id other-table-ids enable-reverse-joins?]] - [(mdb.connection/unique-identifier) source-table-id other-table-ids enable-reverse-joins?])} + [(mdb/unique-identifier) source-table-id other-table-ids enable-reverse-joins?])} (fn [source-table-id other-table-ids enable-reverse-joins?] (let [db-id (database/table-id->database-id source-table-id) all-joins (mapcat #(find-joins db-id source-table-id % enable-reverse-joins?) @@ -507,8 +503,8 @@ [:metabase_field :dest] [:= :dest.table_id :table.id]] :where [:and [:= :source.id field-id] - (mdb.u/isa :source.semantic_type :type/PK) - (mdb.u/isa :dest.semantic_type :type/Name)] + (mdb.query/isa :source.semantic_type :type/PK) + (mdb.query/isa :dest.semantic_type :type/Name)] :limit 1}]} :ids]] :limit 1}) diff --git a/src/metabase/models/params/field_values.clj b/src/metabase/models/params/field_values.clj index 6d7a9b3c1f1bc923164f21f676a3afdb567c9cf1..3e3c4f66f7c4fa6c736e6e176c6eee708daa6f75 100644 --- a/src/metabase/models/params/field_values.clj +++ b/src/metabase/models/params/field_values.clj @@ -3,7 +3,7 @@ values (`GET /api/field/:id/values`) endpoint; used by the chain filter endpoints under certain circumstances." (:require [medley.core :as m] - [metabase.db.util :as mdb.u] + [metabase.db.query :as mdb.query] [metabase.models.field :as field] [metabase.models.field-values :as field-values :refer [FieldValues]] [metabase.models.interface :as mi] @@ -129,7 +129,7 @@ ([fv-type field constraints] (let [hash-key (hash-key-for-advanced-field-values fv-type (:id field) constraints) select-kvs {:field_id (:id field) :type fv-type :hash_key hash-key} - fv (mdb.u/select-or-insert! :model/FieldValues select-kvs + fv (mdb.query/select-or-insert! :model/FieldValues select-kvs #(prepare-advanced-field-values fv-type field hash-key constraints))] (cond (nil? fv) nil diff --git a/src/metabase/models/permissions_group.clj b/src/metabase/models/permissions_group.clj index 247453cf8ea3c1e3e6686288893a7b8c0be8e4f0..4d6ce6f95ae94518d2db7c858e21d6686770a1fe 100644 --- a/src/metabase/models/permissions_group.clj +++ b/src/metabase/models/permissions_group.clj @@ -9,7 +9,7 @@ See documentation in [[metabase.models.permissions]] for more information about the Metabase permissions system." (:require [honey.sql.helpers :as sql.helpers] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.query :as mdb.query] [metabase.models.data-permissions :as data-perms] [metabase.models.interface :as mi] @@ -33,7 +33,7 @@ ;;; -------------------------------------------- Magic Groups Getter Fns --------------------------------------------- (defn- magic-group [group-name] - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn [] (u/prog1 (t2/select-one PermissionsGroup :name group-name) ;; normally it is impossible to delete the magic [[all-users]] or [[admin]] Groups -- see diff --git a/src/metabase/models/revision.clj b/src/metabase/models/revision.clj index 95f79810424db9dc2960f1f8f8a7a65e84e889e7..a2d208eedea21a0b5a3bd99cde90dbb4ed857a51 100644 --- a/src/metabase/models/revision.clj +++ b/src/metabase/models/revision.clj @@ -3,7 +3,6 @@ [cheshire.core :as json] [clojure.data :as data] [metabase.config :as config] - [metabase.db.util :as mdb.u] [metabase.models.interface :as mi] [metabase.models.revision.diff :refer [diff-strings*]] [metabase.util :as u] @@ -13,6 +12,11 @@ [toucan2.core :as t2] [toucan2.model :as t2.model])) +(defn toucan-model? + "Check if `model` is a toucan model." + [model] + (isa? model :metabase/model)) + (def ^:const max-revisions "Maximum number of revisions to keep for each individual object. After this limit is surpassed, the oldest revisions will be deleted." @@ -155,13 +159,13 @@ (mu/defn revisions "Get the revisions for `model` with `id` in reverse chronological order." - [model :- [:fn mdb.u/toucan-model?] + [model :- [:fn toucan-model?] id :- pos-int?] (t2/select Revision :model (name model) :model_id id {:order-by [[:id :desc]]})) (mu/defn revisions+details "Fetch `revisions` for `model` with `id` and add details." - [model :- [:fn mdb.u/toucan-model?] + [model :- [:fn toucan-model?] id :- pos-int?] (when-let [revisions (revisions model id)] (loop [acc [], [r1 r2 & more] revisions] @@ -178,7 +182,7 @@ :or {is-creation? false}} :- [:map {:closed true} [:id pos-int?] [:object :map] - [:entity [:fn mdb.u/toucan-model?]] + [:entity [:fn toucan-model?]] [:user-id pos-int?] [:is-creation? {:optional true} [:maybe :boolean]] [:message {:optional true} [:maybe :string]]]] @@ -208,7 +212,7 @@ [:id pos-int?] [:user-id pos-int?] [:revision-id pos-int?] - [:entity [:fn mdb.u/toucan-model?]]]] + [:entity [:fn toucan-model?]]]] (let [{:keys [id user-id revision-id entity]} info serialized-instance (t2/select-one-fn :object Revision :model (name entity) :model_id id :id revision-id)] (t2/with-transaction [_conn] diff --git a/src/metabase/models/serialization.clj b/src/metabase/models/serialization.clj index 3009f315dbbb968fc04e1889c158e42c4f94004c..e8b7cf41b73fe888ce1bd15df2003cedca544701 100644 --- a/src/metabase/models/serialization.clj +++ b/src/metabase/models/serialization.clj @@ -16,7 +16,7 @@ [clojure.set :as set] [clojure.string :as str] [medley.core :as m] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.lib.schema.id :as lib.schema.id] [metabase.mbql.normalize :as mbql.normalize] [metabase.mbql.util :as mbql.u] @@ -523,9 +523,9 @@ (def ^:private fields-for-table "Given a table name, returns a map of column_name -> column_type" - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn fields-for-table [table-name] - (u.conn/app-db-column-types mdb.connection/*application-db* table-name)))) + (u.conn/app-db-column-types (mdb/app-db) table-name)))) (defn- ->table-name "Returns the table name that a particular ingested entity should finally be inserted into." diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj index 17fd1e64bbdfdb2cec324cf7c2c6e9455bcac345..b418e906addda7174c8f03c65a43104205ae14a7 100644 --- a/src/metabase/models/setting.clj +++ b/src/metabase/models/setting.clj @@ -492,8 +492,6 @@ (when (seq v) v))))) -(def ^:private ^:dynamic *disable-cache* false) - (def ^:private ^:dynamic *disable-init* false) (declare get) @@ -524,7 +522,7 @@ ;; cannot use db (and cache populated from db) if db is not set up (when (and (db-is-set-up?) (allows-site-wide-values? setting)) (not-empty - (if *disable-cache* + (if config/*disable-setting-cache* (db-value setting) (do ;; gotcha - returns immediately if another process is restoring it, i.e. before it's been populated @@ -702,11 +700,11 @@ unless *disable-init* has been bound to a truthy value." [setting-definition-or-name] (let [{:keys [cache? getter enabled? default feature]} (resolve-setting setting-definition-or-name) - disable-cache? (or *disable-cache* (not cache?))] + disable-cache? (or config/*disable-setting-cache* (not cache?))] (if (or (and feature (not (has-feature? feature))) (and enabled? (not (enabled?)))) default - (binding [*disable-cache* disable-cache?] + (binding [config/*disable-setting-cache* disable-cache?] (getter))))) ;;; +----------------------------------------------------------------------------------------------------------------+ @@ -806,7 +804,7 @@ ;; Record the fact that a Setting has been updated so eventually other instances (if applicable) find out ;; about it (For Settings that don't use the Cache, don't update the `last-updated` value, because it will ;; cause other instances to do needless reloading of the cache from the DB) - (when-not *disable-cache* + (when-not config/*disable-setting-cache* (setting.cache/update-settings-last-updated!)))) ;; Now return the `new-value`. new-value)))) @@ -947,7 +945,7 @@ (when-not bypass-read-only? (when (= setter :none) (throw (UnsupportedOperationException. (tru "You cannot set {0}; it is a read-only setting." name))))) - (binding [*disable-cache* (not cache?)] + (binding [config/*disable-setting-cache* (not cache?)] (set-with-audit-logging! setting new-value bypass-read-only?)))) diff --git a/src/metabase/models/setting/cache.clj b/src/metabase/models/setting/cache.clj index acad1e4097908a41d76d1880fc8c47be65b5825c..390cbbcbcdbdf6980d5a7a2c37ca52ef0ee8eafa 100644 --- a/src/metabase/models/setting/cache.clj +++ b/src/metabase/models/setting/cache.clj @@ -4,7 +4,7 @@ (:require [clojure.core :as core] [clojure.java.jdbc :as jdbc] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.util :as u] [metabase.util.honey-sql-2 :as h2x] [metabase.util.i18n :refer [trs]] @@ -28,7 +28,7 @@ ;; Setting cache is unique to the application DB; if it's swapped out for tests or mocking or whatever then use a new ;; cache. (def ^:private ^{:arglists '([])} cache* - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn [] (doto (atom nil) (add-watch :call-on-change (fn [_key _ref old new] @@ -73,7 +73,7 @@ [] (log/debug (trs "Updating value of settings-last-updated in DB...")) ;; for MySQL, cast(current_timestamp AS char); for H2 & Postgres, cast(current_timestamp AS text) - (let [current-timestamp-as-string-honeysql (h2x/cast (if (= (mdb.connection/db-type) :mysql) :char :text) + (let [current-timestamp-as-string-honeysql (h2x/cast (if (= (mdb/db-type) :mysql) :char :text) [:raw "current_timestamp"])] ;; attempt to UPDATE the existing row. If no row exists, `t2/update!` will return 0... (or (pos? (t2/update! :setting {:key settings-last-updated-key} {:value current-timestamp-as-string-honeysql})) diff --git a/src/metabase/models/table.clj b/src/metabase/models/table.clj index a32e3bddd47e43e8dd16bf8179ed9c27b7a45218..a5c632e86d4fe8e0f6e6f9103ed553b4a9de5539 100644 --- a/src/metabase/models/table.clj +++ b/src/metabase/models/table.clj @@ -2,7 +2,7 @@ (:require [metabase.api.common :as api] [metabase.config :as config] - [metabase.db.util :as mdb.u] + [metabase.db.query :as mdb.query] [metabase.driver :as driver] [metabase.models.audit-log :as audit-log] [metabase.models.data-permissions :as data-perms] @@ -136,9 +136,9 @@ {:order-by (case (:field_order table) :custom [[:custom_position :asc]] :smart [[[:case - (mdb.u/isa :semantic_type :type/PK) 0 - (mdb.u/isa :semantic_type :type/Name) 1 - (mdb.u/isa :semantic_type :type/Temporal) 2 + (mdb.query/isa :semantic_type :type/PK) 0 + (mdb.query/isa :semantic_type :type/Name) 1 + (mdb.query/isa :semantic_type :type/Temporal) 2 :else 3] :asc] [:%lower.name :asc]] @@ -184,7 +184,7 @@ [{:keys [id]}] (t2/select-one-pk Field :table_id id - :semantic_type (mdb.u/isa :type/PK) + :semantic_type (mdb.query/isa :type/PK) :visibility_type [:not-in ["sensitive" "retired"]])) (defn- with-objects [hydration-key fetch-objects-fn tables] diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj index 956b3c85891ebf025434b775656eee312ffb7eac..9a998fd62d07ff731d691627e4c76720b2f5da79 100644 --- a/src/metabase/public_settings.clj +++ b/src/metabase/public_settings.clj @@ -40,7 +40,7 @@ [] (if *compile-files* "Metabase" - (binding [setting/*disable-cache* true] + (binding [config/*disable-setting-cache* true] (application-name)))) (defn- google-auth-enabled? [] diff --git a/src/metabase/server/middleware/log.clj b/src/metabase/server/middleware/log.clj index 638d4f497278db8bd8a67aef389ec05361e535e5..8d2904e13aed139cf163237e3f30e52301640b12 100644 --- a/src/metabase/server/middleware/log.clj +++ b/src/metabase/server/middleware/log.clj @@ -7,7 +7,7 @@ [metabase.async.streaming-response :as streaming-response] [metabase.async.streaming-response.thread-pool :as thread-pool] [metabase.async.util :as async.u] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.driver.sql-jdbc.execute.diagnostic :as sql-jdbc.execute.diagnostic] [metabase.server :as server] @@ -56,7 +56,7 @@ (defn- stats [diag-info-fn] (str - (when-let [^PoolBackedDataSource pool (let [data-source (mdb.connection/data-source)] + (when-let [^PoolBackedDataSource pool (let [data-source (mdb/data-source)] (when (instance? PoolBackedDataSource data-source) data-source))] (trs "App DB connections: {0}/{1}" diff --git a/src/metabase/server/routes.clj b/src/metabase/server/routes.clj index 20ecc1af26cccfe8bfa13cf3a04724cda461b355..e1605de5e41a4703deecc7e50a487d6b513ae29d 100644 --- a/src/metabase/server/routes.clj +++ b/src/metabase/server/routes.clj @@ -8,8 +8,7 @@ [metabase.api.routes :as api] [metabase.config :as config] [metabase.core.initialization-status :as init-status] - [metabase.db.connection :as mdb.connection] - [metabase.db.connection-pool-setup :as mdb.connection-pool-setup] + [metabase.db :as mdb] [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] [metabase.plugins.classloader :as classloader] [metabase.public-settings :as public-settings] @@ -53,8 +52,8 @@ ;; ^/api/health -> Health Check Endpoint (GET "/api/health" [] (if (init-status/complete?) - (try (if (or (mdb.connection-pool-setup/recent-activity?) - (sql-jdbc.conn/can-connect-with-spec? {:datasource (mdb.connection/data-source)})) + (try (if (or (mdb/recent-activity?) + (sql-jdbc.conn/can-connect-with-spec? {:datasource (mdb/data-source)})) {:status 200, :body {:status "ok"}} {:status 503 :body {:status "Unable to get app-db connection"}}) (catch Exception e diff --git a/src/metabase/setup.clj b/src/metabase/setup.clj index d40c06335be628d3552c052ec93d2ad6cfe70cd3..4db45c3641d7193c2e26915076dce85a4debd47d 100644 --- a/src/metabase/setup.clj +++ b/src/metabase/setup.clj @@ -2,7 +2,7 @@ (:require [environ.core :as env] [metabase.config :as config] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.models.setting :as setting :refer [defsetting Setting]] [metabase.util.i18n :refer [deferred-tru tru]] [toucan2.core :as t2])) @@ -59,9 +59,9 @@ ;; override could be false so have to check non-nil (if (some? possible-override) possible-override - (or (get @app-db-id->user-exists? (mdb.connection/unique-identifier)) + (or (get @app-db-id->user-exists? (mdb/unique-identifier)) (let [exists? (boolean (seq (t2/select :model/User {:where [:not= :id config/internal-mb-user-id]})))] - (swap! app-db-id->user-exists? assoc (mdb.connection/unique-identifier) exists?) + (swap! app-db-id->user-exists? assoc (mdb/unique-identifier) exists?) exists?)))))) :doc false :audit :never) diff --git a/src/metabase/sync/analyze/fingerprint.clj b/src/metabase/sync/analyze/fingerprint.clj index af4e80f4af1bb1c70c1056cb435ff1247e4dd637..71141a17d91fe43411eee3883368185d6d45ee92 100644 --- a/src/metabase/sync/analyze/fingerprint.clj +++ b/src/metabase/sync/analyze/fingerprint.clj @@ -5,7 +5,7 @@ [clojure.set :as set] [honey.sql.helpers :as sql.helpers] [metabase.db.metadata-queries :as metadata-queries] - [metabase.db.util :as mdb.u] + [metabase.db.query :as mdb.query] [metabase.driver :as driver] [metabase.driver.util :as driver.u] [metabase.models.field :as field :refer [Field]] @@ -157,10 +157,10 @@ [:and [:= :active true] [:or - [:not (mdb.u/isa :semantic_type :type/PK)] + [:not (mdb.query/isa :semantic_type :type/PK)] [:= :semantic_type nil]] [:not-in :visibility_type ["retired" "sensitive"]] - [:not (mdb.u/isa :base_type :type/Structured)]]) + [:not (mdb.query/isa :base_type :type/Structured)]]) (def ^:dynamic *refingerprint?* "Whether we are refingerprinting or doing the normal fingerprinting. Refingerprinting should get fields that already diff --git a/src/metabase/task.clj b/src/metabase/task.clj index 4d917ece7b6ddbf4d59e87cd2cfe0dcd86817f71..211f977e351c514de942ff891a9fb32550263867 100644 --- a/src/metabase/task.clj +++ b/src/metabase/task.clj @@ -16,7 +16,6 @@ [clojurewerkz.quartzite.scheduler :as qs] [environ.core :as env] [metabase.db :as mdb] - [metabase.db.connection :as mdb.connection] [metabase.plugins.classloader :as classloader] [metabase.util :as u] [metabase.util.log :as log] @@ -102,7 +101,7 @@ ;; in a perfect world we could just check whether we're creating a new Connection or not, and if using an existing ;; Connection, wrap it in a delegating proxy wrapper that makes `.close()` a no-op but forwards all other methods. ;; Now that would be a useful macro! - (.getConnection mdb.connection/*application-db*)) + (.getConnection (mdb/app-db))) (shutdown [_])) (when-not *compile-files* diff --git a/test/metabase/api/geojson_test.clj b/test/metabase/api/geojson_test.clj index 4bd5fdd12e38d2525ee25d55505ddbd57caeacca..30dc48163f6574331566d24247be3b79fa2211e7 100644 --- a/test/metabase/api/geojson_test.clj +++ b/test/metabase/api/geojson_test.clj @@ -3,6 +3,7 @@ [cheshire.core :as json] [clojure.test :refer :all] [metabase.api.geojson :as api.geojson] + [metabase.config :as config] [metabase.http-client :as client] [metabase.models.setting :as setting] [metabase.test :as mt] @@ -216,7 +217,7 @@ expected-value (merge (@#'api.geojson/builtin-geojson) custom-geojson)] (mt/with-temporary-setting-values [custom-geojson nil] (mt/with-temp-env-var-value! [mb-custom-geojson (json/generate-string custom-geojson)] - (binding [setting/*disable-cache* true] + (binding [config/*disable-setting-cache* true] (testing "Should parse env var custom GeoJSON and merge in" (is (= expected-value (api.geojson/custom-geojson)))) diff --git a/test/metabase/api/testing_test.clj b/test/metabase/api/testing_test.clj index 1e73e05cbf74cfdeb877600f1d69c17582e66013..ee5c12648cc5f08b5f032c7505a378d019e645b4 100644 --- a/test/metabase/api/testing_test.clj +++ b/test/metabase/api/testing_test.clj @@ -4,14 +4,14 @@ [clojure.java.jdbc :as jdbc] [clojure.test :refer :all] [metabase.api.testing :as testing] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.test :as mt] [metabase.util :as u])) (set! *warn-on-reflection* true) (deftest snapshot-test - (when (= (mdb.connection/db-type) :h2) + (when (= (mdb/db-type) :h2) (let [snapshot-name (mt/random-name)] (testing "Just make sure the snapshot endpoint doesn't crash." (let [file (io/file (#'testing/snapshot-path-for-name snapshot-name))] @@ -24,13 +24,13 @@ (.delete file)))))))) (deftest restore-test - (when (= (mdb.connection/db-type) :h2) + (when (= (mdb/db-type) :h2) (testing "Should throw Exception if file does not exist" (is (= "Not found." (mt/user-http-request :rasta :post 404 (format "testing/restore/%s" (mt/random-name)))))))) (deftest e2e-test - (when (= (mdb.connection/db-type) :h2) + (when (= (mdb/db-type) :h2) (testing "Should be able to snapshot & restore stuff" (let [snapshot-name (munge (u/qualified-name ::test-snapshot))] (try @@ -46,11 +46,11 @@ ;; `restore-app-db-from-snapshot!` for more details (let [snapshot-name (str (random-uuid))] (mt/with-temp-empty-app-db [_conn :h2] - (jdbc/execute! {:datasource mdb.connection/*application-db*} ["create table test_table (a int)"]) - (jdbc/execute! {:datasource mdb.connection/*application-db*} ["insert into test_table (a) values (1)"]) - (jdbc/execute! {:datasource mdb.connection/*application-db*} ["create or replace view test_view as select a from test_table"]) - (jdbc/execute! {:datasource mdb.connection/*application-db*} ["alter table test_table add column b int"]) + (jdbc/execute! {:datasource (mdb/app-db)} ["create table test_table (a int)"]) + (jdbc/execute! {:datasource (mdb/app-db)} ["insert into test_table (a) values (1)"]) + (jdbc/execute! {:datasource (mdb/app-db)} ["create or replace view test_view as select a from test_table"]) + (jdbc/execute! {:datasource (mdb/app-db)} ["alter table test_table add column b int"]) (#'testing/save-snapshot! snapshot-name)) (mt/with-temp-empty-app-db [_conn :h2] (#'testing/restore-snapshot! snapshot-name) - (is (= [{:a 1}] (jdbc/query {:datasource mdb.connection/*application-db*} ["select a from test_view"])))))) + (is (= [{:a 1}] (jdbc/query {:datasource (mdb/app-db)} ["select a from test_view"])))))) diff --git a/test/metabase/cmd/dump_to_h2_test.clj b/test/metabase/cmd/dump_to_h2_test.clj index b99094558fa2ac0f2c8c8ba21e631c245ad85572..f909483352859d4e1404bd845aec168a1d12e287 100644 --- a/test/metabase/cmd/dump_to_h2_test.clj +++ b/test/metabase/cmd/dump_to_h2_test.clj @@ -9,12 +9,12 @@ [metabase.cmd.dump-to-h2 :as dump-to-h2] [metabase.cmd.load-from-h2 :as load-from-h2] [metabase.cmd.test-util :as cmd.test-util] + [metabase.config :as config] + [metabase.db :as mdb] [metabase.db.connection :as mdb.connection] - [metabase.db.spec :as mdb.spec] [metabase.db.test-util :as mdb.test-util] [metabase.driver :as driver] [metabase.models :refer [Database Setting]] - [metabase.models.setting :as setting] [metabase.test :as mt] [metabase.test.data.interface :as tx] [metabase.util.encryption-test :as encryption-test] @@ -60,7 +60,7 @@ {:subprotocol "h2" :subname (format "mem:%s;DB_CLOSE_DELAY=10" db-name) :classname "org.h2.Driver"} - (mdb.spec/spec db-type (tx/dbdef->connection-details db-type :db {:database-name db-name})))] + (mdb/spec db-type (tx/dbdef->connection-details db-type :db {:database-name db-name})))] (mdb.test-util/->ClojureJDBCSpecDataSource spec))) (deftest dump-to-h2-dump-plaintext-test @@ -72,7 +72,7 @@ h2-file-default-enc (format "out-%s.db" (mt/random-name))] (mt/test-drivers #{:h2 :postgres :mysql} (with-redefs [i18n.impl/site-locale-from-setting (constantly nil)] - (binding [setting/*disable-cache* true + (binding [config/*disable-setting-cache* true mdb.connection/*application-db* (mdb.connection/application-db driver/*driver* (persistent-data-source driver/*driver* db-name))] diff --git a/test/metabase/cmd/load_and_dump_test.clj b/test/metabase/cmd/load_and_dump_test.clj index 6463b20b176ec227f1c5849d4ab7d09e63e20d79..44f8d85c43c98aed4c3def4f28a72b841dff1845 100644 --- a/test/metabase/cmd/load_and_dump_test.clj +++ b/test/metabase/cmd/load_and_dump_test.clj @@ -8,11 +8,11 @@ [metabase.cmd.dump-to-h2 :as dump-to-h2] [metabase.cmd.load-from-h2 :as load-from-h2] [metabase.cmd.test-util :as cmd.test-util] + [metabase.config :as config] + [metabase.db :as mdb] [metabase.db.connection :as mdb.connection] - [metabase.db.spec :as mdb.spec] [metabase.db.test-util :as mdb.test-util] [metabase.driver :as driver] - [metabase.models.setting :as setting] [metabase.test :as mt] [metabase.test.data.interface :as tx] [metabase.util.i18n.impl :as i18n.impl])) @@ -36,8 +36,8 @@ :subname (format "mem:%s;DB_CLOSE_DELAY=10" (mt/random-name)) :classname "org.h2.Driver"} (let [details (tx/dbdef->connection-details driver/*driver* :db {:database-name db-name})] - (mdb.spec/spec driver/*driver* details))))] - (binding [setting/*disable-cache* true + (mdb/spec driver/*driver* details))))] + (binding [config/*disable-setting-cache* true mdb.connection/*application-db* (mdb.connection/application-db driver/*driver* data-source)] (with-redefs [i18n.impl/site-locale-from-setting (constantly nil)] (when-not (= driver/*driver* :h2) diff --git a/test/metabase/cmd/rotate_encryption_key_test.clj b/test/metabase/cmd/rotate_encryption_key_test.clj index 750aad438589643b3c7574b473b58ebb2d85e59a..c66c036f97bb4b778d4df853de381ad10af4ab8e 100644 --- a/test/metabase/cmd/rotate_encryption_key_test.clj +++ b/test/metabase/cmd/rotate_encryption_key_test.clj @@ -8,11 +8,12 @@ [metabase.cmd.load-from-h2 :as load-from-h2] [metabase.cmd.rotate-encryption-key :refer [rotate-encryption-key!]] [metabase.cmd.test-util :as cmd.test-util] + [metabase.config :as config] + [metabase.db :as mdb] [metabase.db.connection :as mdb.connection] [metabase.driver :as driver] [metabase.models :refer [Database Secret Setting User]] [metabase.models.interface :as mi] - [metabase.models.setting :as setting] [metabase.test :as mt] [metabase.test.data.interface :as tx] [metabase.test.fixtures :as fixtures] @@ -29,7 +30,7 @@ (use-fixtures :once (fixtures/initialize :db)) (defn- raw-value [keyy] - (:value (first (jdbc/query {:datasource (mdb.connection/data-source)} + (:value (first (jdbc/query {:datasource (mdb/data-source)} [(if (= driver/*driver* :h2) "select \"VALUE\" from setting where setting.\"KEY\"=?;" "select value from setting where setting.key=?;") keyy])))) @@ -75,7 +76,7 @@ ;; while we're at it, disable the setting cache entirely; we are effectively creating a new app DB ;; so the cache itself is invalid and can only mask the real issues - setting/*disable-cache* true + config/*disable-setting-cache* true mdb.connection/*application-db* (mdb.connection/application-db driver/*driver* data-source)] (when-not (= driver/*driver* :h2) (tx/create-db! driver/*driver* {:database-name db-name})) diff --git a/test/metabase/db/custom_migrations_test.clj b/test/metabase/db/custom_migrations_test.clj index 5c86a57927f855b1e89dbf11003a773911c779e2..e7109d509e5b3a47f78d1e6196667dd011b549bb 100644 --- a/test/metabase/db/custom_migrations_test.clj +++ b/test/metabase/db/custom_migrations_test.clj @@ -12,7 +12,7 @@ [clojurewerkz.quartzite.scheduler :as qs] [clojurewerkz.quartzite.triggers :as triggers] [medley.core :as m] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.custom-migrations :as custom-migrations] [metabase.db.schema-migrations-test.impl :as impl] [metabase.models :refer [User]] @@ -1505,13 +1505,13 @@ (defn- table-and-column-of-type [ttype] (->> (t2/query - [(case (mdb.connection/db-type) + [(case (mdb/db-type) :postgres (format "SELECT table_name, column_name, is_nullable FROM information_schema.columns WHERE data_type = '%s' AND table_schema = 'public';" ttype) :mysql (format "SELECT table_name, column_name, is_nullable FROM information_schema.columns WHERE data_type = '%s' AND table_schema = '%s';" - ttype (-> (mdb.connection/data-source) .getConnection .getCatalog)) + ttype (-> (mdb/data-source) .getConnection .getCatalog)) :h2 (format "SELECT table_name, column_name, is_nullable FROM information_schema.columns WHERE data_type = '%s';" ttype))]) @@ -1521,7 +1521,7 @@ (deftest unify-type-of-time-columns-test (impl/test-migrations ["v49.00-054"] [migrate!] - (let [db-type (mdb.connection/db-type) + (let [db-type (mdb/db-type) datetime-type (case db-type :postgres "timestamp without time zone" :h2 "TIMESTAMP" diff --git a/test/metabase/db/fix_mysql_utf8_test.clj b/test/metabase/db/fix_mysql_utf8_test.clj index 5f7e532405d32bbc7ca0bca4a8317200c3a5b978..9de8027a1fbca6c2a58e0643b18c4a3506149768 100644 --- a/test/metabase/db/fix_mysql_utf8_test.clj +++ b/test/metabase/db/fix_mysql_utf8_test.clj @@ -2,8 +2,8 @@ (:require [clojure.java.jdbc :as jdbc] [clojure.test :refer :all] + [metabase.db :as mdb] [metabase.db.data-source :as mdb.data-source] - [metabase.db.setup :as mdb.setup] [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute] [metabase.models :refer [Database]] @@ -64,7 +64,7 @@ ;; create a new application DB and run migrations. (create-test-db!) (let [data-source (test-data-source)] - (mdb.setup/migrate! :mysql data-source :up) + (mdb/migrate! :mysql data-source :up) (is (= {:character-set "utf8mb4", :collation "utf8mb4_unicode_ci"} (db-charset) (table-charset) diff --git a/test/metabase/db/force_migration_test.clj b/test/metabase/db/force_migration_test.clj index 95baa39fdb29a51b07ed6d5d386e5eaf7084a4a0..df9ee96fc9e52e732f0e345363282692e54d6bb2 100644 --- a/test/metabase/db/force_migration_test.clj +++ b/test/metabase/db/force_migration_test.clj @@ -1,7 +1,7 @@ (ns metabase.db.force-migration-test (:require [clojure.test :refer :all] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.custom-migrations :as custom-migrations] [metabase.db.liquibase :as liquibase] [metabase.db.setup :as db.setup] @@ -37,12 +37,13 @@ (mt/test-drivers #{:h2 :mysql :postgres} (mt/with-temp-empty-app-db [conn driver/*driver*] (with-redefs [liquibase/changelog-file "force-migration.yaml"] - (let [{:keys [db-type ^javax.sql.DataSource data-source]} mdb.connection/*application-db* - database (->> (if (instance? java.sql.Connection conn) - conn - (.getConnection ^javax.sql.DataSource conn)) - (#'liquibase/liquibase-connection) - (#'liquibase/database)) + (let [db-type (mdb/db-type) + data-source (mdb/data-source) + database (->> (if (instance? java.sql.Connection conn) + conn + (.getConnection ^javax.sql.DataSource conn)) + (#'liquibase/liquibase-connection) + (#'liquibase/database)) lock-service (.getLockService (LockServiceFactory/getInstance) database)] (.acquireLock lock-service) (db.setup/migrate! db-type data-source :force) diff --git a/test/metabase/db/query_test.clj b/test/metabase/db/query_test.clj index 95290905fb6d4bcdaf6b721716eab36c6db05306..385b14039529c743ba72a2c254fd8306580124ac 100644 --- a/test/metabase/db/query_test.clj +++ b/test/metabase/db/query_test.clj @@ -1,11 +1,19 @@ (ns metabase.db.query-test (:require + [clojure.set :as set] [clojure.string :as str] [clojure.test :refer :all] [metabase.db.query :as mdb.query] + [metabase.models.setting :refer [Setting]] [metabase.query-processor :as qp] [metabase.query-processor.compile :as qp.compile] - [metabase.test :as mt])) + [metabase.test :as mt] + [metabase.util :as u] + [toucan2.core :as t2]) + (:import + (java.util.concurrent CountDownLatch))) + +(set! *warn-on-reflection* true) (defn- verify-same-query "Ensure that the formatted native query derived from an mbql query produce the same results." @@ -74,3 +82,126 @@ :breakout [[:field (mt/id :products :category) {:join-alias "Products"}]]}) :database (mt/id)}] (verify-same-query q))))) + + +(defn- repeat-concurrently [n f] + ;; Use a latch to ensure that the functions start as close to simultaneously as possible. + (let [latch (CountDownLatch. n) + futures (atom [])] + (dotimes [_ n] + (swap! futures conj (future (.countDown latch) + (.await latch) + (f)))) + (into #{} (map deref) @futures))) + +(deftest select-or-insert!-test + ;; We test both a case where the database protects against duplicates, and where it does not. + ;; Using Setting is perfect because it has only two required fields - (the primary) key & value (with no constraint). + ;; + ;; In the `:key` case using the `idempotent-insert!` rather than an `or` prevents the from application throwing an + ;; exception when there are race conditions. For `:value` it prevents us silently inserting duplicates. + ;; + ;; It's important to test both, as only the latter has a phantom read issue and thus requires serializable isolation. + (let [columns [:key :value]] + (doseq [search-col columns] + (testing (format "When the search column %s a uniqueness constraint in the db" + (if (= :key search-col) "has" "does not have")) + (let [search-value (str (random-uuid)) + other-col (first (remove #{search-col} columns))] + (try + ;; ensure there is no database detritus to trip us up + (t2/delete! Setting search-col search-value) + + (let [threads 5 + latch (CountDownLatch. threads) + thunk (fn [] + (mdb.query/select-or-insert! Setting {search-col search-value} + (fn [] + ;; Make sure all the threads are in the mutating path + (.countDown latch) + (.await latch) + {other-col (str (random-uuid))}))) + results (repeat-concurrently threads thunk) + n (count results) + latest (t2/select-one Setting search-col search-value)] + + (case search-col + :key + (do (testing "every call returns the same row" + (is (= #{latest} results))) + + (testing "we never insert any duplicates" + (is (= 1 (t2/count Setting search-col search-value)))) + + (testing "later calls just return the existing row as well" + (is (= latest (thunk))) + (is (= 1 (t2/count Setting search-col search-value))))) + + :value + (do + (testing "there may be race conditions, but we insert at least once" + (is (pos? n))) + + (testing "we returned the same values that were inserted into the database" + (is (= results (set (t2/select Setting search-col search-value))))) + + (testing "later calls just return an existing row as well" + (is (contains? results (thunk))) + (is (= results (set (t2/select Setting search-col search-value)))))))) + + ;; Since we couldn't use with-temp, we need to clean up manually. + (finally + (t2/delete! Setting search-col search-value)))))))) + +(deftest updated-or-insert!-test + ;; We test both a case where the database protects against duplicates, and where it does not. + ;; Using Setting is perfect because it has only two required fields - (the primary) key & value (with no constraint). + (let [columns [:key :value]] + (doseq [search-col columns] + (testing (format "When the search column %s a uniqueness constraint in the db" + (if (= :key search-col) "has" "does not have")) + (doseq [already-exists? [true false]] + (let [search-value (str (random-uuid)) + other-col (first (remove #{search-col} columns)) + other-value (str (random-uuid))] + (try + ;; ensure there is no database detritus to trip us up + (t2/delete! Setting search-col search-value) + + (when already-exists? + (t2/insert! Setting search-col search-value other-col other-value)) + + (let [threads 5 + latch (CountDownLatch. threads) + thunk (fn [] + (u/prog1 (str (random-uuid)) + (mdb.query/update-or-insert! Setting {search-col search-value} + (fn [_] + ;; Make sure all the threads are in the mutating path + (.countDown latch) + (.await latch) + {other-col <>})))) + values-set (repeat-concurrently threads thunk) + latest (get (t2/select-one Setting search-col search-value) other-col)] + + (testing "each update tried to set a different value" + (is (= threads (count values-set)))) + + ;; Unfortunately updates are not serialized, but we cannot show that without using a model with more + ;; than 2 fields. + (testing "the row is updated to match the last update call that resolved" + (is (not= other-value latest)) + (is (contains? values-set latest))) + + (when (or (= :key search-col) already-exists?) + (is (= 1 (count (t2/select Setting search-col search-value))))) + + (testing "After the database is created, it does not create further duplicates" + (let [count (t2/count Setting search-col search-value)] + (is (pos? count)) + (is (empty? (set/intersection values-set (repeat-concurrently threads thunk)))) + (is (= count (t2/count Setting search-col search-value)))))) + + ;; Since we couldn't use with-temp, we need to clean up manually. + (finally + (t2/delete! Setting search-col search-value))))))))) diff --git a/test/metabase/db/schema_migrations_test.clj b/test/metabase/db/schema_migrations_test.clj index af2ab6a929ba914076c82056782381c86db4f66e..9632566ddae16afffb9b89af730eea7cd730d677 100644 --- a/test/metabase/db/schema_migrations_test.clj +++ b/test/metabase/db/schema_migrations_test.clj @@ -12,7 +12,7 @@ [clojure.java.jdbc :as jdbc] [clojure.test :refer :all] [java-time.api :as t] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.custom-migrations-test :as custom-migrations-test] [metabase.db.query :as mdb.query] [metabase.db.schema-migrations-test.impl :as impl] @@ -39,9 +39,8 @@ (testing "Migrating to latest version, rolling back to v44, and then migrating up again" ;; using test-migrations to excercise all drivers (impl/test-migrations ["v46.00-001" "v46.00-002"] [migrate!] - (let [{:keys [^javax.sql.DataSource data-source]} mdb.connection/*application-db* - get-last-id (fn [] - (-> {:connection (.getConnection data-source)} + (let [get-last-id (fn [] + (-> {:datasource (mdb/app-db)} (jdbc/query ["SELECT id FROM DATABASECHANGELOG ORDER BY ORDEREXECUTED DESC LIMIT 1"]) first :id))] @@ -726,7 +725,8 @@ (is (= [{:table_name "DATABASECHANGELOGLOCK" :column_name "LOCKED"}] ;; outlier because this is liquibase's table (t2/query (format "SELECT table_name, column_name FROM information_schema.columns WHERE data_type LIKE 'tinyint%%' AND table_schema = '%s';" - (-> (mdb.connection/data-source) .getConnection .getCatalog)))))))) + (with-open [conn (-> (mdb/app-db) .getConnection)] + (.getCatalog conn))))))))) (deftest index-database-changelog-test (testing "we should have an unique constraint on databasechangelog.(id,author,filename)" @@ -735,7 +735,7 @@ (is (pos? (:count (t2/query-one - (case (mdb.connection/db-type) + (case (mdb/db-type) :postgres "SELECT COUNT(*) as count FROM pg_indexes WHERE tablename = 'databasechangelog' AND indexname = 'idx_databasechangelog_id_author_filename';" :mysql "SELECT COUNT(*) as count FROM INFORMATION_SCHEMA.STATISTICS WHERE TABLE_NAME = 'DATABASECHANGELOG' AND INDEX_NAME = 'idx_databasechangelog_id_author_filename';" diff --git a/test/metabase/db/schema_migrations_test/impl.clj b/test/metabase/db/schema_migrations_test/impl.clj index bce97d9bb0ca96cf77ec0d62f2044453623f6e13..2624544cceab983c64a060adfaaaebe35b3901cb 100644 --- a/test/metabase/db/schema_migrations_test/impl.clj +++ b/test/metabase/db/schema_migrations_test/impl.clj @@ -12,10 +12,10 @@ [clojure.java.jdbc :as jdbc] [clojure.test :refer :all] [java-time.api :as t] + [metabase.db :as mdb] [metabase.db.connection :as mdb.connection] [metabase.db.data-source :as mdb.data-source] [metabase.db.liquibase :as liquibase] - [metabase.db.setup :as mdb.setup] [metabase.db.test-util :as mdb.test-util] [metabase.driver :as driver] [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] @@ -196,7 +196,7 @@ (.generateDeploymentId change-log-service) (liquibase/update-with-change-log liquibase {:change-set-filters change-set-filters}))))) -(defn- test-migrations-for-driver [driver [start-id end-id] f] +(defn- test-migrations-for-driver! [driver [start-id end-id] f] (log/debug (u/format-color 'yellow "Testing migrations for driver %s..." driver)) (with-temp-empty-app-db [conn driver] ;; sanity check: make sure the DB is actually empty @@ -225,11 +225,11 @@ :down (do (assert (int? version), "Downgrade requires a version") - (mdb.setup/migrate! driver (mdb.connection/data-source) :down version)))))] + (mdb/migrate! driver (mdb/data-source) :down version)))))] (f migrate))) (log/debug (u/format-color 'green "Done testing migrations for driver %s." driver))) -(defn do-test-migrations +(defn do-test-migrations! [migration-range f] ;; make sure the normal Metabase application DB is set up before running the tests so things don't get confused and ;; try to initialize it while the mock DB is bound @@ -239,8 +239,9 @@ [migration-range migration-range])] (testing (format "Migrations %s thru %s" start-id (or end-id "end")) (datasets/test-drivers #{:h2 :mysql :postgres} - (test-migrations-for-driver driver/*driver* [start-id end-id] f))))) + (test-migrations-for-driver! driver/*driver* [start-id end-id] f))))) +#_{:clj-kondo/ignore [:metabase/test-helpers-use-non-thread-safe-functions]} (defmacro test-migrations "Util macro for running tests for a set of Liquibase schema migration(s). @@ -275,7 +276,7 @@ either set the `DRIVERS` env var or use [[mt/set-test-drivers!]] from the REPL." {:style/indent 2} [migration-range [migrate!-binding] & body] - `(do-test-migrations + `(do-test-migrations! ~migration-range (fn [~migrate!-binding] ~@body))) diff --git a/test/metabase/db/util_test.clj b/test/metabase/db/util_test.clj deleted file mode 100644 index 51a92092be0d9a21501f23187a45107abd4ae09f..0000000000000000000000000000000000000000 --- a/test/metabase/db/util_test.clj +++ /dev/null @@ -1,134 +0,0 @@ -(ns metabase.db.util-test - (:require - [clojure.set :as set] - [clojure.test :refer [deftest testing is]] - [metabase.db.util :as mdb.u] - [metabase.models.setting :refer [Setting]] - [metabase.util :as u] - [toucan2.core :as t2]) - (:import - (java.util.concurrent CountDownLatch))) - -(set! *warn-on-reflection* true) - -(defn- repeat-concurrently [n f] - ;; Use a latch to ensure that the functions start as close to simultaneously as possible. - (let [latch (CountDownLatch. n) - futures (atom [])] - (dotimes [_ n] - (swap! futures conj (future (.countDown latch) - (.await latch) - (f)))) - (into #{} (map deref) @futures))) - -(deftest select-or-insert!-test - ;; We test both a case where the database protects against duplicates, and where it does not. - ;; Using Setting is perfect because it has only two required fields - (the primary) key & value (with no constraint). - ;; - ;; In the `:key` case using the `idempotent-insert!` rather than an `or` prevents the from application throwing an - ;; exception when there are race conditions. For `:value` it prevents us silently inserting duplicates. - ;; - ;; It's important to test both, as only the latter has a phantom read issue and thus requires serializable isolation. - (let [columns [:key :value]] - (doseq [search-col columns] - (testing (format "When the search column %s a uniqueness constraint in the db" - (if (= :key search-col) "has" "does not have")) - (let [search-value (str (random-uuid)) - other-col (first (remove #{search-col} columns))] - (try - ;; ensure there is no database detritus to trip us up - (t2/delete! Setting search-col search-value) - - (let [threads 5 - latch (CountDownLatch. threads) - thunk (fn [] - (mdb.u/select-or-insert! Setting {search-col search-value} - (fn [] - ;; Make sure all the threads are in the mutating path - (.countDown latch) - (.await latch) - {other-col (str (random-uuid))}))) - results (repeat-concurrently threads thunk) - n (count results) - latest (t2/select-one Setting search-col search-value)] - - (case search-col - :key - (do (testing "every call returns the same row" - (is (= #{latest} results))) - - (testing "we never insert any duplicates" - (is (= 1 (t2/count Setting search-col search-value)))) - - (testing "later calls just return the existing row as well" - (is (= latest (thunk))) - (is (= 1 (t2/count Setting search-col search-value))))) - - :value - (do - (testing "there may be race conditions, but we insert at least once" - (is (pos? n))) - - (testing "we returned the same values that were inserted into the database" - (is (= results (set (t2/select Setting search-col search-value))))) - - (testing "later calls just return an existing row as well" - (is (contains? results (thunk))) - (is (= results (set (t2/select Setting search-col search-value)))))))) - - ;; Since we couldn't use with-temp, we need to clean up manually. - (finally - (t2/delete! Setting search-col search-value)))))))) - -(deftest updated-or-insert!-test - ;; We test both a case where the database protects against duplicates, and where it does not. - ;; Using Setting is perfect because it has only two required fields - (the primary) key & value (with no constraint). - (let [columns [:key :value]] - (doseq [search-col columns] - (testing (format "When the search column %s a uniqueness constraint in the db" - (if (= :key search-col) "has" "does not have")) - (doseq [already-exists? [true false]] - (let [search-value (str (random-uuid)) - other-col (first (remove #{search-col} columns)) - other-value (str (random-uuid))] - (try - ;; ensure there is no database detritus to trip us up - (t2/delete! Setting search-col search-value) - - (when already-exists? - (t2/insert! Setting search-col search-value other-col other-value)) - - (let [threads 5 - latch (CountDownLatch. threads) - thunk (fn [] - (u/prog1 (str (random-uuid)) - (mdb.u/update-or-insert! Setting {search-col search-value} - (fn [_] - ;; Make sure all the threads are in the mutating path - (.countDown latch) - (.await latch) - {other-col <>})))) - values-set (repeat-concurrently threads thunk) - latest (get (t2/select-one Setting search-col search-value) other-col)] - - (testing "each update tried to set a different value" - (is (= threads (count values-set)))) - - ;; Unfortunately updates are not serialized, but we cannot show that without using a model with more - ;; than 2 fields. - (testing "the row is updated to match the last update call that resolved" - (is (not= other-value latest)) - (is (contains? values-set latest))) - - (when (or (= :key search-col) already-exists?) - (is (= 1 (count (t2/select Setting search-col search-value))))) - - (testing "After the database is created, it does not create further duplicates" - (let [count (t2/count Setting search-col search-value)] - (is (pos? count)) - (is (empty? (set/intersection values-set (repeat-concurrently threads thunk)))) - (is (= count (t2/count Setting search-col search-value)))))) - - ;; Since we couldn't use with-temp, we need to clean up manually. - (finally - (t2/delete! Setting search-col search-value))))))))) diff --git a/test/metabase/driver/h2_test.clj b/test/metabase/driver/h2_test.clj index a289ff7446bfaaeded77215ee144bc02c0745ac8..0fe6146dbc5ce1d69ede81c63c249caf760c733b 100644 --- a/test/metabase/driver/h2_test.clj +++ b/test/metabase/driver/h2_test.clj @@ -7,7 +7,7 @@ [metabase.actions.error :as actions.error] [metabase.config :as config] [metabase.core :as mbc] - [metabase.db.spec :as mdb.spec] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.driver.h2 :as h2] [metabase.driver.h2.actions :as h2.actions] @@ -167,7 +167,7 @@ (deftest ^:parallel timestamp-with-timezone-test (testing "Make sure TIMESTAMP WITH TIME ZONEs come back as OffsetDateTimes." (is (= [{:t #t "2020-05-28T18:06-07:00"}] - (jdbc/query (mdb.spec/spec :h2 {:db "mem:test_db"}) + (jdbc/query (mdb/spec :h2 {:db "mem:test_db"}) "SELECT TIMESTAMP WITH TIME ZONE '2020-05-28 18:06:00.000 America/Los_Angeles' AS t"))))) (deftest ^:parallel native-query-parameters-test diff --git a/test/metabase/driver/sql_jdbc/connection_test.clj b/test/metabase/driver/sql_jdbc/connection_test.clj index 0828901b83c38ed17f8778e496fec99dc1aab761..ee04d0da57860c3c3491379389bf06c9a2976ed0 100644 --- a/test/metabase/driver/sql_jdbc/connection_test.clj +++ b/test/metabase/driver/sql_jdbc/connection_test.clj @@ -4,7 +4,7 @@ [clojure.test :refer :all] [metabase.config :as config] [metabase.core :as mbc] - [metabase.db.spec :as mdb.spec] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.driver.h2 :as h2] [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] @@ -56,7 +56,7 @@ (let [destroyed? (atom false) original-destroy @#'sql-jdbc.conn/destroy-pool! connection-details {:db "mem:connection_test"} - spec (mdb.spec/spec :h2 connection-details)] + spec (mdb/spec :h2 connection-details)] (with-redefs [sql-jdbc.conn/destroy-pool! (fn [id destroyed-spec] (original-destroy id destroyed-spec) (reset! destroyed? true))] diff --git a/test/metabase/models/setting_test.clj b/test/metabase/models/setting_test.clj index 6861a863539f6a3d0e0cf9abb3752a6d5f4d0c28..9a080233cc3fb72cdecc97bceab48b35f5f07c89 100644 --- a/test/metabase/models/setting_test.clj +++ b/test/metabase/models/setting_test.clj @@ -5,6 +5,7 @@ [clojure.walk :as walk] [environ.core :as env] [medley.core :as m] + [metabase.config :as config] [metabase.db :as mdb] [metabase.db.connection :as mdb.connection] [metabase.db.query :as mdb.query] @@ -1122,7 +1123,7 @@ (is (nil? (test-setting-with-question-mark?))) ;; note now question mark on the environmental setting (with-redefs [env/env {:mb-test-setting-with-question-mark "resolved"}] - (binding [setting/*disable-cache* false] + (binding [config/*disable-setting-cache* false] (is (= "resolved" (test-setting-with-question-mark?)))))) (testing "Setting a setting that would munge the same throws an error" (is (= {:existing-setting diff --git a/test/metabase/query_processor/test_util.clj b/test/metabase/query_processor/test_util.clj index 5d8c98180724e948b1fc104408df0a70ce82b66d..497b8eb6d2f95514878da3c425392a1eecdba1e2 100644 --- a/test/metabase/query_processor/test_util.clj +++ b/test/metabase/query_processor/test_util.clj @@ -11,7 +11,7 @@ [clojure.test :refer :all] [mb.hawk.init] [medley.core :as m] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.driver :as driver] [metabase.driver.test-util :as driver.tu] [metabase.lib.metadata :as lib.metadata] @@ -201,7 +201,7 @@ (declare cols) (def ^:private ^{:arglists '([db-id table-id field-id])} native-query-col* - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn [db-id table-id field-id] (first (cols diff --git a/test/metabase/sync/analyze/fingerprint_test.clj b/test/metabase/sync/analyze/fingerprint_test.clj index 47ebbbc865e7c152e925d1bafd9e2af2aa270009..5a4c0d66ac4352a121a2145b2da1d8a50f22f6b4 100644 --- a/test/metabase/sync/analyze/fingerprint_test.clj +++ b/test/metabase/sync/analyze/fingerprint_test.clj @@ -4,7 +4,7 @@ [clojure.test :refer :all] [malli.core :as mc] [malli.error :as me] - [metabase.db.util :as mdb.u] + [metabase.db.query :as mdb.query] [metabase.models.field :as field :refer [Field]] [metabase.models.table :refer [Table]] [metabase.query-processor :as qp] @@ -35,10 +35,10 @@ [:and [:= :active true] [:or - [:not (mdb.u/isa :semantic_type :type/PK)] + [:not (mdb.query/isa :semantic_type :type/PK)] [:= :semantic_type nil]] [:not-in :visibility_type ["retired" "sensitive"]] - [:not (mdb.u/isa :base_type :type/Structured)] + [:not (mdb.query/isa :base_type :type/Structured)] [:or [:and [:< :fingerprint_version 1] @@ -51,10 +51,10 @@ [:and [:= :active true] [:or - [:not (mdb.u/isa :semantic_type :type/PK)] + [:not (mdb.query/isa :semantic_type :type/PK)] [:= :semantic_type nil]] [:not-in :visibility_type ["retired" "sensitive"]] - [:not (mdb.u/isa :base_type :type/Structured)] + [:not (mdb.query/isa :base_type :type/Structured)] [:or [:and [:< :fingerprint_version 2] @@ -73,10 +73,10 @@ [:and [:= :active true] [:or - [:not (mdb.u/isa :semantic_type :type/PK)] + [:not (mdb.query/isa :semantic_type :type/PK)] [:= :semantic_type nil]] [:not-in :visibility_type ["retired" "sensitive"]] - [:not (mdb.u/isa :base_type :type/Structured)] + [:not (mdb.query/isa :base_type :type/Structured)] [:or [:and [:< :fingerprint_version 2] @@ -96,10 +96,10 @@ [:and [:= :active true] [:or - [:not (mdb.u/isa :semantic_type :type/PK)] + [:not (mdb.query/isa :semantic_type :type/PK)] [:= :semantic_type nil]] [:not-in :visibility_type ["retired" "sensitive"]] - [:not (mdb.u/isa :base_type :type/Structured)] + [:not (mdb.query/isa :base_type :type/Structured)] [:or [:and [:< :fingerprint_version 4] @@ -124,10 +124,10 @@ (is (= {:where [:and [:= :active true] [:or - [:not (mdb.u/isa :semantic_type :type/PK)] + [:not (mdb.query/isa :semantic_type :type/PK)] [:= :semantic_type nil]] [:not-in :visibility_type ["retired" "sensitive"]] - [:not (mdb.u/isa :base_type :type/Structured)]]} + [:not (mdb.query/isa :base_type :type/Structured)]]} (binding [fingerprint/*refingerprint?* true] (#'fingerprint/honeysql-for-fields-that-need-fingerprint-updating)))))) diff --git a/test/metabase/test/data/h2.clj b/test/metabase/test/data/h2.clj index cb104bdb940a53fe9b65bfdb101b2be96dc9e374..ee1657a3a012389bd705689c25959b46f995f999 100644 --- a/test/metabase/test/data/h2.clj +++ b/test/metabase/test/data/h2.clj @@ -2,7 +2,6 @@ "Code for creating / destroying an H2 database from a `DatabaseDefinition`." (:require [metabase.db :as mdb] - [metabase.db.spec :as mdb.spec] [metabase.driver.ddl.interface :as ddl.i] [metabase.driver.h2] [metabase.driver.sql.util :as sql.u] @@ -119,7 +118,7 @@ ;; Don't use the h2 driver implementation, which makes the connection string read-only & if-exists only (defmethod spec/dbdef->spec :h2 [driver context dbdef] - (mdb.spec/spec :h2 (tx/dbdef->connection-details driver context dbdef))) + (mdb/spec :h2 (tx/dbdef->connection-details driver context dbdef))) (defmethod load-data/load-data! :h2 [& args] diff --git a/test/metabase/test/data/impl.clj b/test/metabase/test/data/impl.clj index 547f7271f74e1ae10c6638d92017d04029d2234a..14288ce46b8eb98600160a41c813cff3287a46e0 100644 --- a/test/metabase/test/data/impl.clj +++ b/test/metabase/test/data/impl.clj @@ -1,7 +1,7 @@ (ns metabase.test.data.impl "Internal implementation of various helper functions in `metabase.test.data`." (:require - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.db.query :as mdb.query] [metabase.driver :as driver] [metabase.driver.util :as driver.u] @@ -64,7 +64,7 @@ That is memoized for the current application database." [] - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn [driver] (u/the-id (get-or-create-test-data-db! driver))))) @@ -144,10 +144,10 @@ :active true)) (def ^:private ^{:arglists '([database-id])} table-lookup-map - (mdb.connection/memoize-for-application-db build-table-lookup-map)) + (mdb/memoize-for-application-db build-table-lookup-map)) (def ^:private ^{:arglists '([field-lookup-map])} field-lookup-map - (mdb.connection/memoize-for-application-db build-field-lookup-map)) + (mdb/memoize-for-application-db build-field-lookup-map)) (defn- cached-table-id [db-id table-name] (get (table-lookup-map db-id) [db-id table-name])) @@ -359,7 +359,7 @@ "Impl for [[metabase.test/dataset]] macro." [dataset-definition f] (let [dbdef (tx/get-dataset-definition dataset-definition) - get-db-for-driver (mdb.connection/memoize-for-application-db + get-db-for-driver (mdb/memoize-for-application-db (fn [driver] (let [db (get-or-create-database! driver dbdef)] (assert db) diff --git a/test/metabase/test/data/one_off_dbs.clj b/test/metabase/test/data/one_off_dbs.clj index f2e42a348aa5cb3bf5ef11757aec8dbb836cdcbd..5ab8a372d285f44ed7a91de878991d706eb4891d 100644 --- a/test/metabase/test/data/one_off_dbs.clj +++ b/test/metabase/test/data/one_off_dbs.clj @@ -4,7 +4,7 @@ (:require [clojure.java.jdbc :as jdbc] [clojure.string :as str] - [metabase.db.spec :as mdb.spec] + [metabase.db :as mdb] [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute] [metabase.models.database :refer [Database]] [metabase.sync :as sync] @@ -26,7 +26,7 @@ (data/with-db db (sql-jdbc.execute/do-with-connection-with-options :h2 - (mdb.spec/spec :h2 details) + (mdb/spec :h2 details) {:write? true} (fn [^java.sql.Connection conn] (binding [*conn* {:connection conn}] diff --git a/test/metabase/test/data/users.clj b/test/metabase/test/data/users.clj index e87c38e8906e5d8ea890ae96c3fc2fbec977fa12..dc34b9b5e7f777ce3e2bf1f6f7c271add93dfb83 100644 --- a/test/metabase/test/data/users.clj +++ b/test/metabase/test/data/users.clj @@ -3,7 +3,7 @@ (:require [clojure.test :as t] [medley.core :as m] - [metabase.db.connection :as mdb.connection] + [metabase.db :as mdb] [metabase.http-client :as client] [metabase.models.permissions-group :refer [PermissionsGroup]] [metabase.models.permissions-group-membership :refer [PermissionsGroupMembership]] @@ -93,7 +93,7 @@ (user->id) ; -> {:rasta 4, ...} (user->id :rasta) ; -> 4" - (mdb.connection/memoize-for-application-db + (mdb/memoize-for-application-db (fn ([] (zipmap usernames (map user->id usernames))) diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj index e107745456260afaec9c02dae2869eddc2d9bc73..a742a818e87c6891aa2dfe6be9c21aca4f615618 100644 --- a/test/metabase/test/util.clj +++ b/test/metabase/test/util.clj @@ -14,7 +14,6 @@ [mb.hawk.parallel] [metabase.config :as config] [metabase.db.query :as mdb.query] - [metabase.db.util :as mdb.u] [metabase.models :refer [Card Dimension @@ -689,7 +688,7 @@ [:not= :id perms/audit-db-id]) (defn do-with-model-cleanup [models f] - {:pre [(sequential? models) (every? mdb.u/toucan-model? models)]} + {:pre [(sequential? models) (every? #(isa? % :metabase/model) models)]} (mb.hawk.parallel/assert-test-is-not-parallel "with-model-cleanup") (initialize/initialize-if-needed! :db) (let [model->old-max-id (into {} (for [model models] diff --git a/test/metabase/util/i18n/impl_test.clj b/test/metabase/util/i18n/impl_test.clj index ec60aa51e6a83d236f36c6db4e4cbd61e59ebb38..97f2f2287817506593680190bc528aa36cc10b36 100644 --- a/test/metabase/util/i18n/impl_test.clj +++ b/test/metabase/util/i18n/impl_test.clj @@ -1,6 +1,7 @@ (ns ^:mb/once metabase.util.i18n.impl-test (:require [clojure.test :refer :all] + [metabase.config :as config] [metabase.models.setting :as setting] [metabase.public-settings :as public-settings] [metabase.test :as mt] @@ -129,7 +130,7 @@ (encryption-test/with-secret-key "secret_key__1" ;; set `site-locale` to something encrypted with the first encryption key. (mt/with-temporary-setting-values [site-locale "en"] - (binding [setting/*disable-cache* true] + (binding [config/*disable-setting-cache* true] (is (= "en" (i18n.impl/site-locale-from-setting))) ;; rotate the encryption key, which will trigger an error being logged