diff --git a/project.clj b/project.clj index e76a22af776fb07990e21577b3d2f46e54c34e7f..f1c638bc30bf46a6ef993c65d3d50064f3475c0a 100644 --- a/project.clj +++ b/project.clj @@ -97,7 +97,7 @@ [net.sf.cssbox/cssbox "4.12" :exclusions [org.slf4j/slf4j-api]] ; HTML / CSS rendering [org.clojars.pntblnk/clj-ldap "0.0.16"] ; LDAP client [org.flatland/ordered "1.5.7"] ; ordered maps & sets - [org.liquibase/liquibase-core "3.6.2" ; migration management (Java lib) + [org.liquibase/liquibase-core "3.6.3" ; migration management (Java lib) :exclusions [ch.qos.logback/logback-classic]] [org.mariadb.jdbc/mariadb-java-client "2.3.0"] ; MySQL/MariaDB driver [org.postgresql/postgresql "42.2.5"] ; Postgres driver diff --git a/src/metabase/db.clj b/src/metabase/db.clj index 29fc0b55a34da13ae6817e919ef685bcf9392e5a..659a51fe61ed204305fea54a413112a4fe686d98 100644 --- a/src/metabase/db.clj +++ b/src/metabase/db.clj @@ -125,7 +125,7 @@ ;;; | MIGRATE! | ;;; +----------------------------------------------------------------------------------------------------------------+ -(def ^:private ^:const ^String changelog-file "liquibase.yaml") +(def ^:private ^String changelog-file "liquibase.yaml") (defn- migrations-sql "Return a string of SQL containing the DDL statements needed to perform unrun `liquibase` migrations." @@ -450,11 +450,12 @@ [& {:keys [db-details auto-migrate] :or {db-details @db-connection-details auto-migrate true}}] - (verify-db-connection db-details) - (run-schema-migrations! auto-migrate db-details) - (create-connection-pool! (jdbc-details db-details)) - (run-data-migrations!) - (reset! setup-db-has-been-called? true)) + (u/with-us-locale + (verify-db-connection db-details) + (run-schema-migrations! auto-migrate db-details) + (create-connection-pool! (jdbc-details db-details)) + (run-data-migrations!) + (reset! setup-db-has-been-called? true))) (defn setup-db-if-needed! "Call `setup-db!` if DB is not already setup; otherwise this does nothing." diff --git a/src/metabase/util.clj b/src/metabase/util.clj index 5332955040eaef08c781f48abfd4b6269e8e9da0..372af039e3aa8885d836722f307532c4708582a9 100644 --- a/src/metabase/util.clj +++ b/src/metabase/util.clj @@ -16,7 +16,8 @@ [ring.util.codec :as codec]) (:import [java.net InetAddress InetSocketAddress Socket] [java.text Normalizer Normalizer$Form] - java.util.concurrent.TimeoutException)) + java.util.concurrent.TimeoutException + java.util.Locale)) ;; This is the very first log message that will get printed. ;; It's here because this is one of the very first namespaces that gets loaded, and the first that has access to the logger @@ -652,7 +653,56 @@ [arg])) (defmacro varargs - "Make a properly-tagged Java interop varargs argument." + "Make a properly-tagged Java interop varargs argument. + + (u/varargs String) + (u/varargs String [\"A\" \"B\"])" + {:style/indent 1} [klass & [objects]] (vary-meta `(into-array ~klass ~objects) assoc :tag (format "[L%s;" (.getCanonicalName ^Class (ns-resolve *ns* klass))))) + +(def ^:private do-with-us-locale-lock (Object.)) + +(defn do-with-us-locale + "Implementation for `with-us-locale` macro; see below." + [f] + ;; Since I'm 99% sure default Locale isn't thread-local we better put a lock in place here so we don't end up with + ;; the following race condition: + ;; + ;; Thread 1 ....*.............................*........................*...........* + ;; ^getDefault() -> Turkish ^setDefault(US) ^(f) ^setDefault(Turkish) + ;; Thread 2 ....................................*....................*................*......* + ;; ^getDefault() -> US ^setDefault(US) ^(f) ^setDefault(US) + (locking do-with-us-locale-lock + (let [original-locale (Locale/getDefault)] + (try + (Locale/setDefault Locale/US) + (f) + (finally + (Locale/setDefault original-locale)))))) + +(defmacro with-us-locale + "Execute `body` with the default system locale temporarily set to `locale`. Why would you want to do this? Tons of + code relies on `String/toUpperCase` which converts a string to uppercase based on the default locale. Normally, this + does what you'd expect, but when the default locale is Turkish, all hell breaks loose: + + ;; Locale is Turkish / -Duser.language=tr + (.toUpperCase \"filename\") ;; -> \"FÄ°LENAME\" + + Rather than submit PRs to every library in the world to use `(.toUpperCase <str> Locale/US)`, it's simpler just to + temporarily bind the default Locale to something predicatable (i.e. US English) when doing something important that + tends to break like running Liquibase migrations.) + + Note that because `Locale/setDefault` and `Locale/getDefault` aren't thread-local (as far as I know) I've had to put + a lock in place to prevent race conditions where threads simulataneously attempt to fetch and change the default + Locale. Thus this macro should be used sparingly, and only in places that are already single-threaded (such as the + launch code that runs Liquibase). + + DO NOT use this macro in API endpoints or other places that are multithreaded or performance will be negatively + impacted. (You shouldn't have a good reason for using this there anyway. Rewrite your code to pass `Locale/US` when + you call `.toUpperCase` or `str/upper-case`. Only use this macro if the calls in question are part of a 3rd-party + library.)" + {:style/indent 0} + [& body] + `(do-with-us-locale (fn [] ~@body)))