From e8c7140a04eb40deb9e5ea5d03ed720180497863 Mon Sep 17 00:00:00 2001
From: Jeff Evans <jeff303@users.noreply.github.com>
Date: Fri, 20 Aug 2021 17:04:13 -0500
Subject: [PATCH] Fix serialization P2 issues (#17388)
* Fix serialization dump error when there are no collections
Update `select-collections` to correctly handle the case where there are no collections
Adding new test that removes all collections, then ensures that dump works with no errors
* Fix serialization load error into empty/blank target DB
Rethrowing exception in cmd when overall load fails
Add test to ensure that a dump containing a user can be loaded into a blank target app DB successfully
Adding a few missing bindings to the `with-temp-empty-app-db` code to set the connection vars under metabase.db.connection
In upsert, add hooks for a :pre-insert-fn and :post-insert-fn to be invoked for new entity instances created by the upsert process, since whether an entity will be an insert or update isn't necessarily known by the load process (only by upsert once identity-condition is checked for each)
In load, setting the pre and post insert functions for a User instance to initialize :password with a random value, and to generate and send a password reset email to the newly inserted user's email, respectively
NOTE: this post insert fn (to send a password reset email for newly inserted users) is NOT hooked up for User as of x.41 release, since it is considered a bugfix, but this can be enabled in a future release)
Adding new defs for the magic permission group names to make those easier to override from tests that might need to (such as the one added for this commit)
* Fix serialization objects being incorrectly updated on skip
Remove `maybe-fixup-card-template-ids!`, which was forcing mode :update, since the existing retry logic should cover what it was trying to do
Update Card and Metric models to delete any Dependency instances for which they are the `:model_id` (to make serialization tests after other tests created temporary Card/Dependency pairs)
Adding missing assertions for Dependency serialization
* Serialization: Fix reload entity logic
Now that the `:mode` is always respected instead of being ignored sometimes, we need to update our "second pass" reload functions to always make the mode `:update` on the second pass, or else that entity would just be skipped, which is bad
Also updated the test to use `:mode` `:skip` from the beginning to be more stringent
---
.../metabase_enterprise/serialization/cmd.clj | 19 +++--
.../serialization/load.clj | 56 +++++++++-----
.../serialization/upsert.clj | 29 ++++----
.../serialization/cmd_test.clj | 73 +++++++++++++++++++
.../serialization/load_test.clj | 4 +-
.../serialization/serialize_test.clj | 7 +-
.../serialization/test_util.clj | 9 ++-
src/metabase/cmd.clj | 4 +-
src/metabase/models/card.clj | 5 +-
src/metabase/models/metric.clj | 8 +-
src/metabase/models/permissions_group.clj | 21 +++++-
.../db/schema_migrations_test/impl.clj | 24 ++++--
12 files changed, 197 insertions(+), 62 deletions(-)
create mode 100644 enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj b/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
index f141b1ce8bd..cf9a946a969 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
@@ -61,7 +61,8 @@
(reload-fn)))
(log/info (trs "END LOAD from {0} with context {1}" path context))))
(catch Throwable e
- (log/error e (trs "ERROR LOAD from {0}: {1}" path (.getMessage e)))))))
+ (log/error e (trs "ERROR LOAD from {0}: {1}" path (.getMessage e)))
+ (throw e)))))
(defn- select-entities-in-collections
([model collections]
@@ -104,13 +105,15 @@
[:= :personal_owner_id
(some-> users first u/the-id)]]
state-filter]})]
- (-> (db/select Collection
- {:where [:and
- (reduce (fn [acc coll]
- (conj acc [:like :location (format "/%d/%%" (:id coll))]))
- [:or] base-collections)
- state-filter]})
- (into base-collections)))))
+ (if (empty? base-collections)
+ []
+ (-> (db/select Collection
+ {:where [:and
+ (reduce (fn [acc coll]
+ (conj acc [:like :location (format "/%d/%%" (:id coll))]))
+ [:or] base-collections)
+ state-filter]})
+ (into base-collections))))))
(defn dump
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/load.clj b/enterprise/backend/src/metabase_enterprise/serialization/load.clj
index f8b89bde4ee..83546fa60ed 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/load.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/load.clj
@@ -6,7 +6,7 @@
[clojure.tools.logging :as log]
[medley.core :as m]
[metabase-enterprise.serialization.names :as names :refer [fully-qualified-name->context]]
- [metabase-enterprise.serialization.upsert :refer [maybe-fixup-card-template-ids! maybe-upsert-many!]]
+ [metabase-enterprise.serialization.upsert :refer [maybe-upsert-many!]]
[metabase.config :as config]
[metabase.mbql.normalize :as mbql.normalize]
[metabase.mbql.util :as mbql.util]
@@ -28,7 +28,7 @@
[metabase.models.segment :refer [Segment]]
[metabase.models.setting :as setting]
[metabase.models.table :refer [Table]]
- [metabase.models.user :refer [User]]
+ [metabase.models.user :as user :refer [User]]
[metabase.shared.models.visualization-settings :as mb.viz]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
@@ -36,7 +36,8 @@
[toucan.db :as db]
[yaml.core :as yaml]
[yaml.reader :as y.reader])
- (:import java.time.temporal.Temporal))
+ (:import java.time.temporal.Temporal
+ java.util.UUID))
(extend-type Temporal y.reader/YAMLReader
(decode [data]
@@ -498,7 +499,7 @@
"Retrying dashboards for collection %s: %s"
(or (:collection context) "root")
(str/join ", " (map :name revisit-dashboards)))
- (load-dashboards context revisit-dashboards)))))))
+ (load-dashboards (assoc context :mode :update) revisit-dashboards)))))))
(defmethod load "dashboards"
[path context]
@@ -545,7 +546,7 @@
(fn []
(log/infof "Reloading pulses from collection %d" (:collection context))
(let [pulse-indexes (map ::pulse-index revisit)]
- (load-pulses (map (partial nth pulses) pulse-indexes) context)))))))
+ (load-pulses (map (partial nth pulses) pulse-indexes) (assoc context :mode :update))))))))
(defmethod load "pulses"
[path context]
@@ -639,16 +640,8 @@
{::revisit [] ::revisit-index #{} ::process []}
(vec resolved-cards))
dummy-insert-cards (not-empty (::revisit grouped-cards))
- process-cards (::process grouped-cards)
- touched-card-ids (maybe-upsert-many!
- context Card
- process-cards)]
- (maybe-fixup-card-template-ids!
- (assoc context :mode :update)
- Card
- (for [card (slurp-many paths)] (resolve-card card (assoc context :mode :update)))
- touched-card-ids)
-
+ process-cards (::process grouped-cards)]
+ (maybe-upsert-many! context Card process-cards)
(when dummy-insert-cards
(let [dummy-inserted-ids (maybe-upsert-many!
context
@@ -664,17 +657,46 @@
(fn []
(log/infof "Attempting to reload cards in collection %d" (:collection context))
(let [revisit-indexes (::revisit-index grouped-cards)]
- (load-cards context paths (mapv (partial nth cards) revisit-indexes))))))))
+ (load-cards (assoc context :mode :update) paths (mapv (partial nth cards) revisit-indexes))))))))
(defmethod load "cards"
[path context]
(binding [names/*suppress-log-name-lookup-exception* true]
(load-cards context (list-dirs path) nil)))
+(defn- pre-insert-user
+ "A function called on each User instance before it is inserted (via upsert)."
+ [user]
+ (log/infof "User with email %s is new to target DB; setting a random password" (:email user))
+ (assoc user :password (str (UUID/randomUUID))))
+
+;; leaving comment out for now (deliberately), because this will send a password reset email to newly inserted users
+;; when enabled in a future release; see `defmethod load "users"` below
+#_(defn- post-insert-user
+ "A function called on the ID of each `User` instance after it is inserted (via upsert)."
+ [user-id]
+ (when-let [{email :email, google-auth? :google_auth, is-active? :is_active}
+ (db/select-one [User :email :google_auth :is_active] :id user-id)]
+ (let [reset-token (user/set-password-reset-token! user-id)
+ site-url (public-settings/site-url)
+ password-reset-url (str site-url "/auth/reset_password/" reset-token)
+ ;; in a web server context, the server-name ultimately comes from ServletRequest/getServerName
+ ;; (i.e. the Java class, via Ring); this is the closest approximation in our batch context
+ server-name (.getHost (URL. site-url))]
+ (let [email-res (email/send-password-reset-email! email google-auth? server-name password-reset-url is-active?)]
+ (if (:error email-res)
+ (log/infof "Failed to send password reset email generated for user ID %d (%s): %s"
+ user-id
+ email
+ (:message email-res))
+ (log/infof "Password reset email generated for user ID %d (%s)" user-id email)))
+ user-id)))
+
(defmethod load "users"
[path context]
;; Currently we only serialize the new owner user, so it's fine to ignore mode setting
- (maybe-upsert-many! context User
+ ;; add :post-insert-fn post-insert-user back to start sending password reset emails
+ (maybe-upsert-many! (assoc context :pre-insert-fn pre-insert-user) User
(for [user (slurp-dir path)]
(dissoc user :password))))
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/upsert.clj b/enterprise/backend/src/metabase_enterprise/serialization/upsert.clj
index 061c53a3f7b..3e686877cf2 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/upsert.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/upsert.clj
@@ -117,8 +117,19 @@
:insert)))))))
(defn maybe-upsert-many!
- "Batch upsert-or-skip"
- [{:keys [mode on-error] :as context} model entities]
+ "Batch upsert many entities.
+
+ Within the `context` map, the following keys are recognized:
+ `mode` indicates mode of operation for existing entities (`:upsert` or `:skip`), as per the `identity-condition`
+ `on-error` indicates what to do in case of upsert error (`:continue` or `:abort`)
+ `pre-insert-fn` (optional) is a function to call on each entity to be inserted, before it is inserted
+ `post-insert-fn` (optional) is a function to call on each entity to be inserted, after it is inserted"
+ [{:keys [mode on-error pre-insert-fn post-insert-fn]
+ :or {pre-insert-fn identity
+ post-insert-fn identity}
+ :as context}
+ model
+ entities]
(let [{:keys [update insert skip]} (group-by-action context model entities)]
(doseq [[_ entity _] insert]
(log/info (trs "Inserting {0}" (name-for-logging (name model) entity))))
@@ -131,7 +142,8 @@
(->> (concat (for [[position _ existing] skip]
[(u/the-id existing) position])
- (map vector (maybe-insert-many! model on-error (map second insert))
+ (map vector (map post-insert-fn
+ (maybe-insert-many! model on-error (map (comp pre-insert-fn second) insert)))
(map first insert))
(for [[position entity existing] update]
(let [id (u/the-id existing)]
@@ -143,14 +155,3 @@
[id position])))
(sort-by second)
(map first))))
-
-(defn maybe-fixup-card-template-ids!
- "Upserts `entities` that are in `selected-ids`. Cards with template-tags that refer to other cards need a second pass
- of fixing the card-ids. To not overwrite cards that were skipped in previous step, classify entities and validate
- against the ones that were just modified."
- [context model entities selected-ids]
- (let [{:keys [update _ _]} (group-by-action context model entities)
- id-set (set selected-ids)
- final-ents (filter #(id-set (:id (nth % 2))) update)]
- (maybe-upsert-many! context model
- (map second final-ents))))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
new file mode 100644
index 00000000000..4827803f64c
--- /dev/null
+++ b/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
@@ -0,0 +1,73 @@
+(ns metabase-enterprise.serialization.cmd-test
+ (:require [clojure.test :as t]
+ [clojure.tools.logging :as log]
+ [metabase-enterprise.serialization.load :as load]
+ [metabase.cmd :as cmd]
+ [metabase.db.schema-migrations-test.impl :as schema-migrations-test.impl]
+ [metabase.models :refer [Card User]]
+ [metabase.models.permissions-group :as group]
+ [metabase.test :as mt]
+ [metabase.test.fixtures :as fixtures]
+ [metabase.util :as u]
+ [toucan.db :as db])
+ (:import java.util.UUID))
+
+(t/use-fixtures :once (fixtures/initialize :db :test-users))
+
+(defmacro ^:private with-empty-h2-app-db
+ "Runs `body` under a new, blank, H2 application database (randomly named), in which all model tables have been
+ created via Liquibase schema migrations. After `body` is finished, the original app DB bindings are restored.
+
+ Makes use of functionality in the `metabase.db.schema-migrations-test.impl` namespace since that already does
+ what we need."
+ [& body]
+ `(schema-migrations-test.impl/with-temp-empty-app-db [conn# :h2]
+ (schema-migrations-test.impl/run-migrations-in-range! conn# [0 999999]) ; this should catch all migrations)
+ ;; since the actual group defs are not dynamic, we need with-redefs to change them here
+ (with-redefs [group/all-users (#'group/get-or-create-magic-group! group/all-users-group-name)
+ group/admin (#'group/get-or-create-magic-group! group/admin-group-name)
+ group/metabot (#'group/get-or-create-magic-group! group/metabot-group-name)]
+ ~@body)))
+
+(t/deftest no-collections-test
+ (t/testing "Dumping a card when there are no active collection should work properly (#16931)"
+ ;; we need a blank H2 app db, temporarily, in order to run this test (to ensure we have no collections present,
+ ;; while also not deleting or messing with any existing user personal collections that the real app DB might have,
+ ;; since that will interfere with other tests)
+ ;; making use of the functionality in the metabase.db.schema-migrations-test.impl namespace for this (since it
+ ;; already does what we need)
+ (with-empty-h2-app-db
+ ;; create a single dummy user, to own a card
+ (let [user (db/simple-insert! User
+ :email "nobody@nowhere.com"
+ :first_name (mt/random-name)
+ :last_name (mt/random-name)
+ :password (str (UUID/randomUUID))
+ :date_joined :%now
+ :is_active true
+ :is_superuser true)]
+ ;; then the card itself
+ (db/simple-insert! Card
+ :name "Single Card"
+ :display "Single Card"
+ :dataset_query {}
+ :creator_id (u/the-id user)
+ :visualization_settings "{}"
+ :created_at :%now
+ :updated_at :%now)
+ ;; serialize "everything" (which should just be the card and user), which should succeed if #16931 is fixed
+ (cmd/dump (str (System/getProperty "java.io.tmpdir") "/" (mt/random-name)))))))
+
+(t/deftest blank-target-db-test
+ (t/testing "Loading a dump into an empty app DB still works (#16639)"
+ (let [dump-dir (str (System/getProperty "java.io.tmpdir") "/" (mt/random-name))
+ user-pre-insert-called? (atom false)]
+ (log/infof "Dumping to %s" dump-dir)
+ (cmd/dump dump-dir "--user" "crowberto@metabase.com")
+ (with-empty-h2-app-db
+ (with-redefs [load/pre-insert-user (fn [user]
+ (reset! user-pre-insert-called? true)
+ (assoc user :password "test-password"))]
+ (cmd/load dump-dir "--mode" :update
+ "--on-error" :abort)
+ (t/is (true? @user-pre-insert-called?)))))))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj
index 8808737abe4..8f7ef6a9bd5 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj
@@ -262,7 +262,7 @@
[entity _]
entity)
-(deftest dump-load-entities-testw
+(deftest dump-load-entities-test
(try
;; in case it already exists
(u/ignore-exceptions
@@ -372,7 +372,7 @@
[Card (Card card-id-with-native-snippet)]
[Card (Card card-join-card-id)]]})]
(with-world-cleanup
- (load dump-dir {:on-error :continue :mode :update})
+ (load dump-dir {:on-error :continue :mode :skip})
(mt/with-db (db/select-one Database :name ts/temp-db-name)
(doseq [[model entity] (:entities fingerprint)]
(testing (format "%s \"%s\"" (type model) (:name entity))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/serialize_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/serialize_test.clj
index d79f62834f8..0ada5e3610e 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/serialize_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/serialize_test.clj
@@ -3,8 +3,8 @@
[clojure.test :refer :all]
[metabase-enterprise.serialization.serialize :as serialize]
[metabase-enterprise.serialization.test-util :as ts]
- [metabase.models :refer [Card Collection Dashboard Database Field Metric NativeQuerySnippet Segment
- Table]]))
+ [metabase.models :refer [Card Collection Dashboard Database Dependency Field Metric NativeQuerySnippet
+ Segment Table]]))
(defn- all-ids-are-fully-qualified-names?
[m]
@@ -31,7 +31,8 @@
[Table table-id]
[Field numeric-field-id]
[Database db-id]
- [NativeQuerySnippet snippet-id]]]
+ [NativeQuerySnippet snippet-id]
+ [Dependency dependency-id]]]
(testing (name model)
(let [serialization (serialize/serialize (model id))]
(testing (format "\nserialization = %s" (pr-str serialization))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj b/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj
index 1642f1bdff2..4a00d54bdbf 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj
@@ -1,7 +1,7 @@
(ns metabase-enterprise.serialization.test-util
(:require [metabase-enterprise.serialization.names :as names]
- [metabase.models :refer [Card Collection Dashboard DashboardCard DashboardCardSeries Database Field Metric
- NativeQuerySnippet Pulse PulseCard Segment Table User]]
+ [metabase.models :refer [Card Collection Dashboard DashboardCard DashboardCardSeries Database Dependency
+ Field Metric NativeQuerySnippet Pulse PulseCard Segment Table User]]
[metabase.models.collection :as collection]
[metabase.query-processor.store :as qp.store]
[metabase.shared.models.visualization-settings :as mb.viz]
@@ -121,6 +121,11 @@
[:field
~'category-pk-field-id
{:join-alias "cat"}]]}]}}}]
+ Dependency [{~'dependency-id :id} {:model "Card"
+ :model_id ~'card-id
+ :dependent_on_model "Segment"
+ :dependent_on_id ~'segment-id
+ :created_at :%now}]
Card [{~'card-arch-id :id}
{;:archived true
:table_id ~'table-id
diff --git a/src/metabase/cmd.clj b/src/metabase/cmd.clj
index 0e5cc98a7bb..a322801e577 100644
--- a/src/metabase/cmd.clj
+++ b/src/metabase/cmd.clj
@@ -52,7 +52,7 @@
(classloader/require 'metabase.cmd.dump-to-h2)
(try
(let [options {:keep-existing? (boolean (some #{"--keep-existing"} opts))
- :dump-plaintext? (boolean (some #{"--dump-plaintext"} opts)) }]
+ :dump-plaintext? (boolean (some #{"--dump-plaintext"} opts))}]
((resolve 'metabase.cmd.dump-to-h2/dump-to-h2!) h2-filename options)
(println "Dump complete")
(system-exit! 0))
@@ -140,7 +140,7 @@
(defn ^:command load
"Load serialized metabase instance as created by `dump` command from directory `path`.
- `mode` can be one of `:update` or `:skip` (default)."
+ `--mode` can be one of `:update` or `:skip` (default). `--on-error` can be `:abort` or `:continue` (default)."
([path] (load path {"--mode" :skip
"--on-error" :continue}))
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index 6a1cecc341e..5edecdeddc4 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -6,7 +6,7 @@
[metabase.mbql.normalize :as normalize]
[metabase.mbql.util :as mbql.u]
[metabase.models.collection :as collection]
- [metabase.models.dependency :as dependency]
+ [metabase.models.dependency :as dependency :refer [Dependency]]
[metabase.models.field-values :as field-values]
[metabase.models.interface :as i]
[metabase.models.params :as params]
@@ -203,7 +203,8 @@
;; Cards don't normally get deleted (they get archived instead) so this mostly affects tests
(defn- pre-delete [{:keys [id]}]
(db/delete! 'ModerationReview :moderated_item_type "card", :moderated_item_id id)
- (db/delete! 'Revision :model "Card", :model_id id))
+ (db/delete! 'Revision :model "Card", :model_id id)
+ (db/delete! 'Dependency :model "Card", :model_id id))
(defn- result-metadata-out
"Transform the Card result metadata as it comes out of the DB. Convert columns to keywords where appropriate."
diff --git a/src/metabase/models/metric.clj b/src/metabase/models/metric.clj
index 2e9fe7be533..a97a21af73f 100644
--- a/src/metabase/models/metric.clj
+++ b/src/metabase/models/metric.clj
@@ -4,7 +4,7 @@
clauses."
(:require [medley.core :as m]
[metabase.mbql.util :as mbql.u]
- [metabase.models.dependency :as dependency]
+ [metabase.models.dependency :as dependency :refer [Dependency]]
[metabase.models.interface :as i]
[metabase.models.revision :as revision]
[metabase.util :as u]
@@ -17,6 +17,9 @@
(models/defmodel Metric :metric)
+(defn- pre-delete [{:keys [id]}]
+ (db/delete! 'Dependency :model "Metric", :model_id id))
+
(defn- pre-update [{:keys [creator_id id], :as updates}]
(u/prog1 updates
;; throw an Exception if someone tries to update creator_id
@@ -35,7 +38,8 @@
models/IModelDefaults
{:types (constantly {:definition :metric-segment-definition})
:properties (constantly {:timestamped? true})
- :pre-update pre-update})
+ :pre-update pre-update
+ :pre-delete pre-delete})
i/IObjectPermissions
(merge
i/IObjectPermissionsDefaults
diff --git a/src/metabase/models/permissions_group.clj b/src/metabase/models/permissions_group.clj
index 2c98c873896..39af0e75a9f 100644
--- a/src/metabase/models/permissions_group.clj
+++ b/src/metabase/models/permissions_group.clj
@@ -34,20 +34,35 @@
(fn []
(f (mdb.connection/db-type) (mdb.connection/jdbc-spec)))))
+(def ^{:const true
+ :doc "The name of the \"All Users\" magic group."
+ :added "0.41.0"} all-users-group-name
+ "All Users")
+
(def ^{:arglists '([])} ^metabase.models.permissions_group.PermissionsGroupInstance
all-users
"Fetch the `All Users` permissions group, creating it if needed."
- (get-or-create-magic-group! "All Users"))
+ (get-or-create-magic-group! all-users-group-name))
+
+(def ^{:const true
+ :doc "The name of the \"Administrators\" magic group."
+ :added "0.41.0"} admin-group-name
+ "Administrators")
(def ^{:arglists '([])} ^metabase.models.permissions_group.PermissionsGroupInstance
admin
"Fetch the `Administators` permissions group, creating it if needed."
- (get-or-create-magic-group! "Administrators"))
+ (get-or-create-magic-group! admin-group-name))
+
+(def ^{:const true
+ :doc "The name of the \"MetaBot\" magic group."
+ :added "0.41.0"} metabot-group-name
+ "MetaBot")
(def ^{:arglists '([])} ^metabase.models.permissions_group.PermissionsGroupInstance
metabot
"Fetch the `MetaBot` permissions group, creating it if needed."
- (get-or-create-magic-group! "MetaBot"))
+ (get-or-create-magic-group! metabot-group-name))
;;; --------------------------------------------------- Validation ---------------------------------------------------
diff --git a/test/metabase/db/schema_migrations_test/impl.clj b/test/metabase/db/schema_migrations_test/impl.clj
index e2c96c805da..e245ecefec2 100644
--- a/test/metabase/db/schema_migrations_test/impl.clj
+++ b/test/metabase/db/schema_migrations_test/impl.clj
@@ -12,6 +12,7 @@
[clojure.test :refer :all]
[clojure.tools.logging :as log]
[metabase.db :as mdb]
+ [metabase.db.connection :as mdb.conn]
[metabase.db.liquibase :as liquibase]
[metabase.driver :as driver]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
@@ -23,10 +24,10 @@
(:import [liquibase Contexts Liquibase]
[liquibase.changelog ChangeSet DatabaseChangeLog]))
-(defmulti ^:private do-with-temp-empty-app-db*
+(defmulti do-with-temp-empty-app-db*
"Create a new completely empty app DB for `driver`, then call `(f jdbc-spec)` with a spec for that DB. Should clean up
before and after running `f` as needed."
- {:arglists '([driver f])}
+ {:added "0.41.0", :arglists '([driver f])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
@@ -55,25 +56,34 @@
(let [jdbc-spec {:subprotocol "h2", :subname "mem:schema-migrations-test-db", :classname "org.h2.Driver"}]
(f jdbc-spec)))
-(defn- do-with-temp-empty-app-db [driver f]
+(defn do-with-temp-empty-app-db
+ "The function invoked by `with-temp-empty-app-db` to execute a thunk `f` in a temporary, empty app DB. Use the macro
+ instead: `with-temp-empty-app-db`."
+ {:added "0.41.0"}
+ [driver f]
(do-with-temp-empty-app-db*
driver
(fn [jdbc-spec]
(with-open [conn (jdbc/get-connection jdbc-spec)]
(binding [toucan.db/*db-connection* {:connection conn}
- toucan.db/*quoting-style* (mdb/quoting-style driver)]
+ toucan.db/*quoting-style* (mdb/quoting-style driver)
+ mdb.conn/*db-type* driver
+ mdb.conn/*jdbc-spec* jdbc-spec]
(f conn))))))
-(defmacro ^:private with-temp-empty-app-db
+(defmacro with-temp-empty-app-db
"Create a new temporary application DB of `db-type` and execute `body` with `conn-binding` bound to a
`java.sql.Connection` to the database. Toucan `*db-connection*` is also bound, which means Toucan functions like
- `select` or `update!` will operate against this database."
+ `select` or `update!` will operate against this database.
+
+ Made public as of x.41."
[[conn-binding db-type] & body]
`(do-with-temp-empty-app-db ~db-type (fn [~(vary-meta conn-binding assoc :tag 'java.sql.Connection)] ~@body)))
-(defn- run-migrations-in-range!
+(defn run-migrations-in-range!
"Run Liquibase migrations from our migrations YAML file in the range of `start-id` -> `end-id` (inclusive) against a
DB with `jdbc-spec`."
+ {:added "0.41.0"}
[^java.sql.Connection conn [start-id end-id]]
(liquibase/with-liquibase [liquibase conn]
(let [change-log (.getDatabaseChangeLog liquibase)
--
GitLab