Add exclamation points to some DB fn names to denote side effects

reset_password/metabase/reset_password/core.clj

@@ -12,7 +12,7 @@
 
 (defn -main
   [email-address]
-  (db/setup-db)
+  (db/setup-db!)
   (println (format "Resetting password for %s..." email-address))
   (try
     (println (format "OK [[[%s]]]" (set-reset-token! email-address)))

src/metabase/cmd/load_from_h2.clj

@@ -194,7 +194,7 @@
 
    Defaults to using `@metabase.db/db-file` as the connection string."
   [h2-connection-string-or-nil]
-  (db/setup-db)
+  (db/setup-db!)
   (jdbc/with-db-transaction [target-db-conn (db/jdbc-details)]
     (jdbc/db-set-rollback-only! target-db-conn)
     (disable-db-constraints! target-db-conn)

src/metabase/core.clj

@@ -108,7 +108,7 @@
   (reset! metabase-initialization-progress 0.4)
 
   ;; startup database.  validates connection & runs any necessary migrations
-  (db/setup-db :auto-migrate (config/config-bool :mb-db-automigrate))
+  (db/setup-db! :auto-migrate (config/config-bool :mb-db-automigrate))
   (reset! metabase-initialization-progress 0.5)
 
   ;; run a very quick check to see if we are doing a first time installation
@@ -203,7 +203,7 @@
 
 (def ^:private cmd->fn
   {:migrate      (fn [direction]
-                   (db/migrate @db/db-connection-details (keyword direction)))
+                   (db/migrate! @db/db-connection-details (keyword direction)))
    :load-from-h2 (fn [& [h2-connection-string-or-nil]]
                    (require 'metabase.cmd.load-from-h2)
                    ((resolve 'metabase.cmd.load-from-h2/load-from-h2!) h2-connection-string-or-nil))})

src/metabase/db.clj

@@ -149,7 +149,7 @@
       (Thread/sleep 2000)
       (throw (Exception. "Database has migration lock; cannot run migrations. You can force-release these locks by running `java -jar metabase.jar migrate release-locks`.")))))
 
-(defn- migrate-up-if-needed
+(defn- migrate-up-if-needed!
   "Run any unran LIQUIBASE migrations, if needed.
 
    This creates SQL for the migrations to be performed, then executes each DDL statement.
@@ -161,8 +161,8 @@
     (doseq [line (migrations-lines liquibase)]
       (jdbc/execute! conn [line]))))
 
-(defn- force-migrate-up-if-needed
-  "Force migrating up. This does two things differently from `migrate-up-if-needed`:
+(defn- force-migrate-up-if-needed!
+  "Force migrating up. This does two things differently from `migrate-up-if-needed!`:
 
    1.  This doesn't check to make sure the DB locks are cleared
    2.  Any DDL statements that fail are ignored
@@ -191,7 +191,7 @@
          ^Database       database       (.findCorrectDatabaseImplementation (DatabaseFactory/getInstance) liquibase-conn)]
      (Liquibase. changelog-file (ClassLoaderResourceAccessor.) database))))
 
-(defn migrate
+(defn migrate!
   "Migrate the database (this can also be ran via command line like `java -jar metabase.jar migrate up` or `lein run migrate up`):
 
    *  `:up`            - Migrate up
@@ -199,11 +199,14 @@
    *  `:down-one`      - Rollback a single migration
    *  `:print`         - Just print the SQL for running the migrations, don't actually run them.
    *  `:release-locks` - Manually release migration locks left by an earlier failed migration.
-                         (This shouldn't be necessary now that we run migrations inside a transaction, but is available just in case)."
+                         (This shouldn't be necessary now that we run migrations inside a transaction, but is available just in case).
+
+   Note that this only performs *schema migrations*, not data migrations. Data migrations are handled separately by `metabase.db.migrations/run-all`.
+   (`setup-db!`, below, calls both this function and `run-all`)."
   ([]
-   (migrate :up))
+   (migrate! :up))
   ([direction]
-   (migrate @db-connection-details direction))
+   (migrate! @db-connection-details direction))
   ([db-details direction]
    (jdbc/with-db-transaction [conn (jdbc-details db-details)]
      ;; Tell transaction to automatically `.rollback` instead of `.commit` when the transaction finishes
@@ -214,8 +217,8 @@
      (try
        (let [liquibase (conn->liquibase conn)]
          (case direction
-           :up            (migrate-up-if-needed conn liquibase)
-           :force         (force-migrate-up-if-needed conn liquibase)
+           :up            (migrate-up-if-needed! conn liquibase)
+           :force         (force-migrate-up-if-needed! conn liquibase)
            :down-one      (.rollback liquibase 1 "")
            :print         (println (migrations-sql liquibase))
            :release-locks (.forceReleaseLocks liquibase)))
@@ -268,12 +271,12 @@
   "Transaction connection to the *Metabase* backing DB connection pool. Used internally by `transaction`."
   nil)
 
-(declare setup-db-if-needed)
+(declare setup-db-if-needed!)
 
 (defn- db-connection
   "Get a JDBC connection spec for the Metabase DB."
   []
-  (setup-db-if-needed)
+  (setup-db-if-needed!)
   (or *transaction-connection*
       @db-connection-pool
       (throw (Exception. "DB is not setup."))))
@@ -327,7 +330,7 @@
     (format "Unable to connect to Metabase %s DB." (name engine)))
   (log/info "Verify Database Connection ... ✅"))
 
-(defn setup-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."
   [& {:keys [db-details auto-migrate]
@@ -339,10 +342,10 @@
 
   ;; Run through our DB migration process and make sure DB is fully prepared
   (if auto-migrate
-    (migrate db-details :up)
+    (migrate! db-details :up)
     ;; if we are not doing auto migrations then print out migration sql for user to run manually
     ;; then throw an exception to short circuit the setup process and make it clear we can't proceed
-    (let [sql (migrate db-details :print)]
+    (let [sql (migrate! db-details :print)]
       (log/info (str "Database Upgrade Required\n\n"
                      "NOTICE: Your database requires updates to work with this version of Metabase.  "
                      "Please execute the following sql commands on your database before proceeding.\n\n"
@@ -360,11 +363,11 @@
   (require 'metabase.db.migrations)
   ((resolve 'metabase.db.migrations/run-all)))
 
-(defn setup-db-if-needed
-  "Call `setup-db` if DB is not already setup; otherwise no-op."
+(defn setup-db-if-needed!
+  "Call `setup-db!` if DB is not already setup; otherwise this does nothing."
   [& args]
   (when-not @setup-db-has-been-called?
-    (apply setup-db args)))
+    (apply setup-db! args)))
 
 
 ;;; +------------------------------------------------------------------------------------------------------------------------+

test/metabase/test/data/interface.clj

@@ -68,7 +68,7 @@
   (metabase-instance [{:keys [database-name]} engine-kw]
     (assert (string? database-name))
     (assert (keyword? engine-kw))
-    (db/setup-db-if-needed, :auto-migrate true)
+    (db/setup-db-if-needed!, :auto-migrate true)
     (Database :name database-name, :engine (name engine-kw))))
 
 

test/metabase/test_setup.clj

@@ -77,7 +77,7 @@
 
     (try
       (log/info "Setting up test DB and running migrations...")
-      (db/setup-db :auto-migrate true)
+      (db/setup-db! :auto-migrate true)
       (setting/set! :site-name "Metabase Test")
       (core/initialization-complete!)