diff --git a/circle.yml b/circle.yml index cbaadb504322aca8161db11953f6bd256afb6145..b8667f69f451c8d612dc352821ef1f0f7053161a 100644 --- a/circle.yml +++ b/circle.yml @@ -3,7 +3,7 @@ machine: America/Los_Angeles java: version: - oraclejdk8 + openjdk7 node: version: 4.4.7 services: diff --git a/src/metabase/db.clj b/src/metabase/db.clj index 5b96fd6602f528c46db1c685cfc51c4fe1b70942..044bb3f922d2009129bae9deb4a715fac71dee21 100644 --- a/src/metabase/db.clj +++ b/src/metabase/db.clj @@ -17,10 +17,13 @@ [metabase.models.interface :as models] [metabase.util :as u] metabase.util.honeysql-extensions) ; this needs to be loaded so the `:h2` quoting style gets added - (:import (java.util.jar JarFile JarFile$JarEntryIterator JarFile$JarFileEntry) + (:import java.io.StringWriter java.sql.Connection - clojure.lang.Atom - com.mchange.v2.c3p0.ComboPooledDataSource)) + com.mchange.v2.c3p0.ComboPooledDataSource + liquibase.Liquibase + (liquibase.database DatabaseFactory Database) + liquibase.database.jvm.JdbcConnection + liquibase.resource.ClassLoaderResourceAccessor)) ;;; +------------------------------------------------------------------------------------------------------------------------+ @@ -107,50 +110,97 @@ ;;; | MIGRATE! | ;;; +------------------------------------------------------------------------------------------------------------------------+ -(defn- filename-without-path-or-prefix - "Strip the path and/or prefix from a migration FILENAME if it has them." - [filename] - (s/replace filename #"^(?:migrations/)?([\w\d]+)(?:\.(?:json|yaml))?$" "$1")) - -(defn- migration-files:jar - "Return the set of migration filenames (without path or prefix) inside the uberjar. - (`io/resource` doesn't work here; this approach adapted from [this StackOverflow answer](http://stackoverflow.com/a/20073154/1198455).)" - [] - ;; get path to the jar -- see this SO answer http://stackoverflow.com/a/320595/1198455 - (let [^String jar-path (s/replace (-> ComboPooledDataSource .getProtectionDomain .getCodeSource .getLocation .getPath) #"%20" " ") - ^JarFile$JarEntryIterator entries (.entries (JarFile. jar-path)) - ^Atom files (atom #{})] - (while (.hasMoreElements entries) - (let [^JarFile$JarFileEntry entry (.nextElement entries) - ^String entry-name (.getName entry)] - (when (and (.startsWith entry-name "migrations/") - (not= entry-name "migrations/")) ; skip the directory itself - (swap! files conj (filename-without-path-or-prefix entry-name))))) - @files)) - -(defn- migration-files - "Return the set of migration filenames (without path or prefix) in the `resources/migrations` directory or from the JAR." - [] - ;; unfortunately io/as-file doesn't seem to work for directories inside a JAR. Try it for local dev but fall back to hacky Java interop method if that fails - (try (set (map filename-without-path-or-prefix (.list (io/as-file (io/resource "migrations"))))) - (catch Throwable _ - (migration-files:jar)))) - -(defn- migration-entries - "Return a set of migration files (without path or prefix) that have already been run. - This is fetched from the `databasechangelog` table. - (migration-entires) -> #{\"001_initial_schema\", \"002_add_session_table\", ...}" - [] - ;; an Exception will get thrown if there is no databasechangelog table yet; just return nil in that case because nil will never equal any set - (u/ignore-exceptions - (set (for [{filename :filename} (jdbc/query (jdbc-details) [(format "SELECT %s AS filename FROM %s;" ((db/quote-fn) "filename") ((db/quote-fn) "databasechangelog"))])] - (filename-without-path-or-prefix filename))))) - -(defn- has-unrun-migration-files? - "`true` if the set of migration files is the same as the set of migrations that have already been run." - ^Boolean [] - (not= (migration-files) - (migration-entries))) +(def ^:private ^:const ^String changelog-file "liquibase.yaml") + +(defn- migrations-sql + "Return a string of SQL containing the DDL statements needed to perform unrun LIQUIBASE migrations." + ^String [^Liquibase liquibase] + (let [writer (StringWriter.)] + (.update liquibase "" writer) + (.toString writer))) + +(defn- migrations-lines + "Return a sequnce of DDL statements that should be used to perform migrations for LIQUIBASE. + + MySQL gets snippy if we try to run the entire DB migration as one single string; it seems to only like it if we run one statement at a time; + Liquibase puts each DDL statement on its own line automatically so just split by lines and filter out blank / comment lines. Even though this + is not neccesary for H2 or Postgres go ahead and do it anyway because it keeps the code simple and doesn't make a significant performance difference." + [^Liquibase liquibase] + (for [line (s/split-lines (migrations-sql liquibase)) + :when (not (or (s/blank? line) + (re-find #"^--" line)))] + line)) + +(defn- has-unrun-migrations? + "Does LIQUIBASE have migration change sets that haven't been run yet? + + It's a good idea to Check to make sure there's actually something to do before running `(migrate :up)` because `migrations-sql` will + always contain SQL to create and release migration locks, which is both slightly dangerous and a waste of time when we won't be using them." + ^Boolean [^Liquibase liquibase] + (boolean (seq (.listUnrunChangeSets liquibase nil)))) + +(defn- has-migration-lock? + "Is a migration lock in place for LIQUIBASE?" + ^Boolean [^Liquibase liquibase] + (boolean (seq (.listLocks liquibase)))) + +(defn- wait-for-migration-lock-to-be-cleared + "Check and make sure the database isn't locked. If it is, sleep for 2 seconds and then retry several times. + There's a chance the lock will end up clearing up so we can run migrations normally." + [^Liquibase liquibase] + (u/auto-retry 5 + (when (has-migration-lock? liquibase) + (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! + "Run any unrun LIQUIBASE migrations, if needed. + + This creates SQL for the migrations to be performed, then executes each DDL statement. + Running `.update` directly doesn't seem to work as we'd expect; it ends up commiting the changes made and they can't be rolled back at + the end of the transaction block. Converting the migration to SQL string and running that via `jdbc/execute!` seems to do the trick." + [conn, ^Liquibase liquibase] + (log/info "Checking if Database has unrun migrations...") + (when (has-unrun-migrations? liquibase) + (log/info "Database has unrun migrations. Waiting for migration lock to be cleared...") + (wait-for-migration-lock-to-be-cleared liquibase) + (log/info "Migration lock is cleared. Running migrations...") + (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!`: + + 1. This doesn't check to make sure the DB locks are cleared + 2. Any DDL statements that fail are ignored + + It can be used to fix situations where the database got into a weird state, as was common before the fixes made in #3295. + + Each DDL statement is ran inside a nested transaction; that way if the nested transaction fails we can roll it back without rolling back the entirety of changes + that were made. (If a single statement in a transaction fails you can't do anything futher until you clear the error state by doing something like calling `.rollback`.)" + [conn, ^Liquibase liquibase] + (when (has-unrun-migrations? liquibase) + (doseq [line (migrations-lines liquibase)] + (log/info line) + (jdbc/with-db-transaction [nested-transaction-connection conn] + (try (jdbc/execute! nested-transaction-connection [line]) + (log/info (u/format-color 'green "[SUCCESS]")) + (catch Throwable e + (.rollback (jdbc/get-connection nested-transaction-connection)) + (log/error (u/format-color 'red "[ERROR] %s" (.getMessage e))))))))) + +(def ^{:arglists '([])} ^DatabaseFactory database-factory + "Return an instance of the Liquibase `DatabaseFactory`. This is done on a background thread at launch because otherwise it adds 2 seconds to startup time." + (partial deref (future (DatabaseFactory/getInstance)))) + +(defn- conn->liquibase + "Get a `Liquibase` object from JDBC CONN." + (^Liquibase [] + (conn->liquibase (jdbc-details))) + (^Liquibase [conn] + (let [^JdbcConnection liquibase-conn (JdbcConnection. (jdbc/get-connection conn)) + ^Database database (.findCorrectDatabaseImplementation (database-factory) liquibase-conn)] + (Liquibase. changelog-file (ClassLoaderResourceAccessor.) database)))) (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`): @@ -169,9 +219,30 @@ ([direction] (migrate! @db-connection-details direction)) ([db-details direction] - ;; Loading Liquibase is slow slow slow so only do it if we actually need to - (require 'metabase.db.liquibase) - ((resolve 'metabase.db.liquibase/migrate!) (jdbc-details db-details) direction))) + (jdbc/with-db-transaction [conn (jdbc-details db-details)] + ;; Tell transaction to automatically `.rollback` instead of `.commit` when the transaction finishes + (jdbc/db-set-rollback-only! conn) + ;; Disable auto-commit. This should already be off but set it just to be safe + (.setAutoCommit (jdbc/get-connection conn) false) + ;; Set up liquibase and let it do its thing + (log/info "Setting up Liquibase...") + (try + (let [liquibase (conn->liquibase conn)] + (log/info "Liquibase is ready.") + (case direction + :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))) + ;; Migrations were successful; disable rollback-only so `.commit` will be called instead of `.rollback` + (jdbc/db-unset-rollback-only! conn) + :done + ;; If for any reason any part of the migrations fail then rollback all changes + (catch Throwable e + ;; This should already be happening as a result of `db-set-rollback-only!` but running it twice won't hurt so better safe than sorry + (.rollback (jdbc/get-connection conn)) + (throw e)))))) ;;; +------------------------------------------------------------------------------------------------------------------------+ @@ -236,14 +307,16 @@ (defn- verify-db-connection "Test connection to database with DETAILS and throw an exception if we have any troubles connecting." - [engine details] - {:pre [(keyword? engine) (map? details)]} - (log/info (u/format-color 'cyan "Verifying %s Database Connection ..." (name engine))) - (assert (binding [*allow-potentailly-unsafe-connections* true] - (require 'metabase.driver) - ((resolve 'metabase.driver/can-connect-with-details?) engine details)) - (format "Unable to connect to Metabase %s DB." (name engine))) - (log/info "Verify Database Connection ... " (u/emoji "✅"))) + ([db-details] + (verify-db-connection (:type db-details) db-details)) + ([engine details] + {:pre [(keyword? engine) (map? details)]} + (log/info (u/format-color 'cyan "Verifying %s Database Connection ..." (name engine))) + (assert (binding [*allow-potentailly-unsafe-connections* true] + (require 'metabase.driver) + ((resolve 'metabase.driver/can-connect-with-details?) engine details)) + (format "Unable to connect to Metabase %s DB." (name engine))) + (log/info "Verify Database Connection ... " (u/emoji "✅")))) (def ^:dynamic ^Boolean *disable-data-migrations* @@ -266,27 +339,20 @@ (throw (java.lang.Exception. "Database requires manual upgrade.")))) (defn- run-schema-migrations! - "Run Liquibase migrations if needed if AUTO-MIGRATE? is enabled, otherwise print migrations and quit." + "Run through our DB migration process and make sure DB is fully prepared" [auto-migrate? db-details] - (when-not auto-migrate? + (log/info "Running Database Migrations...") + (if auto-migrate? + (migrate! db-details :up) (print-migrations-and-quit! db-details)) - (log/info "Database has unrun migrations. Preparing to run migrations...") - (migrate! db-details :up) (log/info "Database Migrations Current ... " (u/emoji "✅"))) -(defn- run-schema-migrations-if-needed! - "Check and see if we need to run any schema migrations, and run them if needed." - [auto-migrate? db-details] - (log/info "Checking to see if database has unrun migrations...") - (if (has-unrun-migration-files?) - (run-schema-migrations! auto-migrate? db-details) - (log/info "Database migrations are up to date. Skipping loading Liquibase."))) - (defn- run-data-migrations! - "Do any custom code-based migrations once the DB structure is up to date." + "Do any custom code-based migrations now that the db structure is up to date." [] - (require 'metabase.db.migrations) - ((resolve 'metabase.db.migrations/run-all!))) + (when-not *disable-data-migrations* + (require 'metabase.db.migrations) + ((resolve 'metabase.db.migrations/run-all!)))) (defn setup-db! "Do general preparation of database by validating that we can connect. @@ -295,8 +361,8 @@ :or {db-details @db-connection-details auto-migrate true}}] (reset! setup-db-has-been-called? true) - (verify-db-connection (:type db-details) db-details) - (run-schema-migrations-if-needed! auto-migrate db-details) + (verify-db-connection db-details) + (run-schema-migrations! auto-migrate db-details) (create-connection-pool! (jdbc-details db-details)) (run-data-migrations!)) diff --git a/src/metabase/db/liquibase.clj b/src/metabase/db/liquibase.clj deleted file mode 100644 index 14b440d60dd1d140416339e38e9a9d1bb3cefdac..0000000000000000000000000000000000000000 --- a/src/metabase/db/liquibase.clj +++ /dev/null @@ -1,127 +0,0 @@ -(ns metabase.db.liquibase - "Internal wrapper around Liquibase migrations functionality. Used internally by `metabase.db`; don't use functions in this namespace directly." - (:require [clojure.java.jdbc :as jdbc] - [clojure.string :as s] - [clojure.tools.logging :as log] - [metabase.util :as u]) - (:import java.io.StringWriter - liquibase.Liquibase - (liquibase.database DatabaseFactory Database) - liquibase.database.jvm.JdbcConnection - liquibase.resource.ClassLoaderResourceAccessor)) - -(def ^:private ^:const ^String changelog-file "liquibase.yaml") - -(defn- migrations-sql - "Return a string of SQL containing the DDL statements needed to perform unrun LIQUIBASE migrations." - ^String [^Liquibase liquibase] - (let [writer (StringWriter.)] - (.update liquibase "" writer) - (.toString writer))) - -(defn- migrations-lines - "Return a sequnce of DDL statements that should be used to perform migrations for LIQUIBASE. - - MySQL gets snippy if we try to run the entire DB migration as one single string; it seems to only like it if we run one statement at a time; - Liquibase puts each DDL statement on its own line automatically so just split by lines and filter out blank / comment lines. Even though this - is not neccesary for H2 or Postgres go ahead and do it anyway because it keeps the code simple and doesn't make a significant performance difference." - [^Liquibase liquibase] - (for [line (s/split-lines (migrations-sql liquibase)) - :when (not (or (s/blank? line) - (re-find #"^--" line)))] - line)) - -(defn- has-unrun-migrations? - "Does LIQUIBASE have migration change sets that haven't been run yet? - - It's a good idea to Check to make sure there's actually something to do before running `(migrate :up)` because `migrations-sql` will - always contain SQL to create and release migration locks, which is both slightly dangerous and a waste of time when we won't be using them." - ^Boolean [^Liquibase liquibase] - (boolean (seq (.listUnrunChangeSets liquibase nil)))) - -(defn- has-migration-lock? - "Is a migration lock in place for LIQUIBASE?" - ^Boolean [^Liquibase liquibase] - (boolean (seq (.listLocks liquibase)))) - -(defn- wait-for-migration-lock-to-be-cleared - "Check and make sure the database isn't locked. If it is, sleep for 2 seconds and then retry several times. - There's a chance the lock will end up clearing up so we can run migrations normally." - [^Liquibase liquibase] - (u/auto-retry 5 - (when (has-migration-lock? liquibase) - (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! - "Run any unrun LIQUIBASE migrations, if needed. - - This creates SQL for the migrations to be performed, then executes each DDL statement. - Running `.update` directly doesn't seem to work as we'd expect; it ends up commiting the changes made and they can't be rolled back at - the end of the transaction block. Converting the migration to SQL string and running that via `jdbc/execute!` seems to do the trick." - [conn, ^Liquibase liquibase] - (log/info "Checking if Database has unrun migrations...") - (when (has-unrun-migrations? liquibase) - (log/info "Database has unrun migrations. Waiting for migration lock to be cleared...") - (wait-for-migration-lock-to-be-cleared liquibase) - (log/info "Migration lock is cleared. Running migrations...") - (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!`: - - 1. This doesn't check to make sure the DB locks are cleared - 2. Any DDL statements that fail are ignored - - It can be used to fix situations where the database got into a weird state, as was common before the fixes made in #3295. - - Each DDL statement is ran inside a nested transaction; that way if the nested transaction fails we can roll it back without rolling back the entirety of changes - that were made. (If a single statement in a transaction fails you can't do anything futher until you clear the error state by doing something like calling `.rollback`.)" - [conn, ^Liquibase liquibase] - (when (has-unrun-migrations? liquibase) - (doseq [line (migrations-lines liquibase)] - (log/info line) - (jdbc/with-db-transaction [nested-transaction-connection conn] - (try (jdbc/execute! nested-transaction-connection [line]) - (log/info (u/format-color 'green "[SUCCESS]")) - (catch Throwable e - (.rollback (jdbc/get-connection nested-transaction-connection)) - (log/error (u/format-color 'red "[ERROR] %s" (.getMessage e))))))))) - - -(defn- conn->liquibase - "Get a `Liquibase` object from JDBC CONN." - ^Liquibase [conn] - (let [^JdbcConnection liquibase-conn (JdbcConnection. (jdbc/get-connection conn)) - ^Database database (.findCorrectDatabaseImplementation (DatabaseFactory/getInstance) liquibase-conn)] - (Liquibase. changelog-file (ClassLoaderResourceAccessor.) database))) - - -(defn migrate! - "Migrate this database via Liquibase. This command is meant for internal use by `metabase.db/migrate!`, so see that command for documentation." - [jdbc-details direction] - (jdbc/with-db-transaction [conn jdbc-details] - ;; Tell transaction to automatically `.rollback` instead of `.commit` when the transaction finishes - (jdbc/db-set-rollback-only! conn) - ;; Disable auto-commit. This should already be off but set it just to be safe - (.setAutoCommit (jdbc/get-connection conn) false) - ;; Set up liquibase and let it do its thing - (log/info "Setting up Liquibase...") - (try - (let [liquibase (conn->liquibase conn)] - (log/info "Liquibase is ready.") - (case direction - :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))) - ;; Migrations were successful; disable rollback-only so `.commit` will be called instead of `.rollback` - (jdbc/db-unset-rollback-only! conn) - :done - ;; If for any reason any part of the migrations fail then rollback all changes - (catch Throwable e - ;; This should already be happening as a result of `db-set-rollback-only!` but running it twice won't hurt so better safe than sorry - (.rollback (jdbc/get-connection conn)) - (throw e))))) diff --git a/test/metabase/db_test.clj b/test/metabase/db_test.clj index 8b8d6fb5dabe265e399164afaf802eb3676e6fd3..d1eefb25394df7926b814d243776022198a87cd8 100644 --- a/test/metabase/db_test.clj +++ b/test/metabase/db_test.clj @@ -19,15 +19,3 @@ (expect {:type :postgres, :user "tom", :password "1234", :host "localhost", :port "5432", :dbname "toms_cool_db", :ssl "true", :sslfactory "org.postgresql.ssl.NonValidatingFactory"} (parse-connection-string "postgres://tom:1234@localhost:5432/toms_cool_db?ssl=true&sslfactory=org.postgresql.ssl.NonValidatingFactory")) - - -;; tests for filename-without-path-or-prefix - -(tu/resolve-private-vars metabase.db filename-without-path-or-prefix) - -(expect "my_migration" (filename-without-path-or-prefix "migrations/my_migration.json")) -(expect "my_migration" (filename-without-path-or-prefix "migrations/my_migration.yaml")) -(expect "my_migration" (filename-without-path-or-prefix "my_migration.json")) -(expect "my_migration" (filename-without-path-or-prefix "my_migration.yaml")) -(expect "my_migration" (filename-without-path-or-prefix "migrations/my_migration")) -(expect "my_migration" (filename-without-path-or-prefix "my_migration")) diff --git a/test/metabase/http_client.clj b/test/metabase/http_client.clj index 40bf62c06c298192b2c35223cfdd7457371e0fe1..595e351721f0372b9a6f0b80a69edfba26494855 100644 --- a/test/metabase/http_client.clj +++ b/test/metabase/http_client.clj @@ -60,7 +60,10 @@ (declare client) -(defn authenticate [{:keys [email password] :as credentials}] +(defn authenticate + "Authenticate a test user with EMAIL and PASSWORD, returning their Metabase Session token; + or throw an Exception if that fails." + [{:keys [email password], :as credentials}] {:pre [(string? email) (string? password)]} (println "Authenticating" email) ; DEBUG (try