Merge pull request #4678 from metabase/merge-release-0.23.1

Merge release 0.23.1

docs/operations-guide/start.md

@@ -70,15 +70,33 @@ Step-by-step instructions on how to upgrade Metabase running on Heroku.
 
 # Troubleshooting Common Problems
 
-### Metabase fails to startup
+### Metabase fails to start due to database locks
 
-Sometimes Metabase will fail to complete its startup due to a database lock that was not cleared properly.
+Sometimes Metabase will fail to complete its startup due to a database lock that was not cleared properly. The error message will look something like:
+
+    liquibase.exception.DatabaseException: liquibase.exception.LockException: Could not acquire change log lock.
 
 When this happens, go to a terminal where Metabase is installed and run:
 
     java -jar metabase.jar migrate release-locks
 
-in the command line to manually clear the locks.  Then restart your Metabase instance.
+in the command line to manually clear the locks. Then restart your Metabase instance.
+
+### Metabase fails to start due to OutOfMemoryErrors
+
+On Java 7, Metabase may fail to launch with a message like
+
+    java.lang.OutOfMemoryError: PermGen space
+
+or one like
+
+    Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler
+
+If this happens, setting a few JVM options should fix your issue:
+
+    java -XX:+CMSClassUnloadingEnabled -XX:+UseConcMarkSweepGC -XX:MaxPermSize=256m -jar target/uberjar/metabase.jar
+
+Alternatively, you can upgrade to Java 8 instead, which will fix the issue as well.
 
 
 # Configuring the Metabase Application Database

project.clj

@@ -27,7 +27,7 @@
                  [amalloy/ring-gzip-middleware "0.1.3"]               ; Ring middleware to GZIP responses if client can handle it
                  [aleph "0.4.1"]                                      ; Async HTTP library; WebSockets
                  [buddy/buddy-core "1.2.0"]                           ; various cryptograhpic functions
-                 [buddy/buddy-sign "1.1.0"]                           ; JSON Web Tokens; High-Level message signing library
+                 [buddy/buddy-sign "1.5.0"]                           ; JSON Web Tokens; High-Level message signing library
                  [cheshire "5.7.0"]                                   ; fast JSON encoding (used by Ring JSON middleware)
                  [clj-http "3.4.1"                                    ; HTTP client
                   :exclusions [commons-codec

src/metabase/api/routes.clj

@@ -60,9 +60,6 @@
   (context "/field"           [] (+auth field/routes))
   (context "/getting_started" [] (+auth getting-started/routes))
   (context "/geojson"         [] (+auth geojson/routes))
-  (GET     "/health"          [] (if ((resolve 'metabase.core/initialized?))
-                                   {:status 200, :body {:status "ok"}}
-                                   {:status 503, :body {:status "initializing", :progress ((resolve 'metabase.core/initialization-progress))}}))
   (context "/label"           [] (+auth label/routes))
   (context "/metric"          [] (+auth metric/routes))
   (context "/notify"          [] (+apikey notify/routes))

src/metabase/core.clj

 ;; -*- comment-column: 35; -*-
 (ns metabase.core
   (:gen-class)
-  (:require [clojure.string :as s]
+  (:require (clojure [pprint :as pprint]
+                     [string :as s])
             [clojure.tools.logging :as log]
             environ.core
             [ring.adapter.jetty :as ring-jetty]
@@ -14,8 +15,9 @@
                              [session :refer [wrap-session]])
             [medley.core :as m]
             [toucan.db :as db]
-            (metabase [config :as config]
-                      [db :as mdb]
+            [metabase.config :as config]
+            [metabase.core.initialization-status :as init-status]
+            (metabase [db :as mdb]
                       [driver :as driver]
                       [events :as events]
                       [logger :as logger]
@@ -54,23 +56,7 @@
 
 ;;; ## ---------------------------------------- LIFECYCLE ----------------------------------------
 
-(defonce ^:private metabase-initialization-progress
-  (atom 0))
 
-(defn initialized?
-  "Is Metabase initialized and ready to be served?"
-  []
-  (= @metabase-initialization-progress 1.0))
-
-(defn initialization-progress
-  "Get the current progress of Metabase initialization."
-  []
-  @metabase-initialization-progress)
-
-(defn initialization-complete!
-  "Complete the Metabase initialization by setting its progress to 100%."
-  []
-  (reset! metabase-initialization-progress 1.0))
 
 (defn- -init-create-setup-token
   "Create and set a new setup token and log it."
@@ -97,46 +83,47 @@
   []
   (log/info (format "Starting Metabase version %s ..." config/mb-version-string))
   (log/info (format "System timezone is '%s' ..." (System/getProperty "user.timezone")))
-  (reset! metabase-initialization-progress 0.1)
+  (init-status/set-progress! 0.1)
 
   ;; First of all, lets register a shutdown hook that will tidy things up for us on app exit
   (.addShutdownHook (Runtime/getRuntime) (Thread. ^Runnable destroy!))
-  (reset! metabase-initialization-progress 0.2)
+  (init-status/set-progress! 0.2)
 
   ;; load any plugins as needed
   (plugins/load-plugins!)
-  (reset! metabase-initialization-progress 0.3)
+  (init-status/set-progress! 0.3)
 
   ;; Load up all of our Database drivers, which are used for app db work
   (driver/find-and-load-drivers!)
-  (reset! metabase-initialization-progress 0.4)
+  (init-status/set-progress! 0.4)
 
   ;; startup database.  validates connection & runs any necessary migrations
+  (log/info "Setting up and migrating Metabase DB. Please sit tight, this may take a minute...")
   (mdb/setup-db! :auto-migrate (config/config-bool :mb-db-automigrate))
-  (reset! metabase-initialization-progress 0.5)
+  (init-status/set-progress! 0.5)
 
   ;; run a very quick check to see if we are doing a first time installation
   ;; the test we are using is if there is at least 1 User in the database
-  (let [new-install (not (db/exists? User))]
+  (let [new-install? (not (db/exists? User))]
 
     ;; Bootstrap the event system
     (events/initialize-events!)
-    (reset! metabase-initialization-progress 0.7)
+    (init-status/set-progress! 0.7)
 
     ;; Now start the task runner
     (task/start-scheduler!)
-    (reset! metabase-initialization-progress 0.8)
+    (init-status/set-progress! 0.8)
 
-    (when new-install
+    (when new-install?
       (log/info "Looks like this is a new installation ... preparing setup wizard")
       ;; create setup token
       (-init-create-setup-token)
       ;; publish install event
       (events/publish-event! :install {}))
-    (reset! metabase-initialization-progress 0.9)
+    (init-status/set-progress! 0.9)
 
     ;; deal with our sample dataset as needed
-    (if new-install
+    (if new-install?
       ;; add the sample dataset DB for fresh installs
       (sample-data/add-sample-dataset!)
       ;; otherwise update if appropriate
@@ -145,7 +132,7 @@
     ;; start the metabot thread
     (metabot/start-metabot!))
 
-  (initialization-complete!)
+  (init-status/set-complete!)
   (log/info "Metabase Initialization COMPLETE"))
 
 
@@ -173,7 +160,8 @@
                              (config/config-str :mb-jetty-daemon) (assoc :daemon? (config/config-bool :mb-jetty-daemon))
                              (config/config-str :mb-jetty-ssl)    (-> (assoc :ssl? true)
                                                                       (merge jetty-ssl-config)))]
-      (log/info "Launching Embedded Jetty Webserver with config:\n" (with-out-str (clojure.pprint/pprint (m/filter-keys (fn [k] (not (re-matches #".*password.*" (str k)))) jetty-config))))
+      (log/info "Launching Embedded Jetty Webserver with config:\n" (with-out-str (pprint/pprint (m/filter-keys #(not (re-matches #".*password.*" (str %)))
+                                                                                                                jetty-config))))
       ;; NOTE: we always start jetty w/ join=false so we can start the server first then do init in the background
       (->> (ring-jetty/run-jetty app (assoc jetty-config :join? false))
            (reset! jetty-instance)))))

src/metabase/core/initialization_status.clj

+(ns metabase.core.initialization-status
+  "Code related to tracking the progress of metabase initialization.
+   This is kept in a separate, tiny namespace so it can be loaded right away when the application launches
+   (and so we don't need to wait for `metabase.core` to load to check the status).")
+
+(defonce ^:private progress-atom
+  (atom 0))
+
+(defn complete?
+  "Is Metabase initialized and ready to be served?"
+  []
+  (= @progress-atom 1.0))
+
+(defn progress
+  "Get the current progress of Metabase initialization."
+  []
+  @progress-atom)
+
+(defn set-progress!
+  "Update the Metabase initialization progress to a new value, a floating-point value between `0` and `1`."
+  [^Float new-progress]
+  {:pre [(float? new-progress) (<= 0.0 new-progress 1.0)]}
+  (reset! progress-atom new-progress))
+
+(defn set-complete!
+  "Complete the Metabase initialization by setting its progress to 100%."
+  []
+  (set-progress! 1.0))

src/metabase/db.clj

@@ -289,6 +289,11 @@
 (def ^:private setup-db-has-been-called?
   (atom false))
 
+(defn db-is-setup?
+  "True if the Metabase DB is setup and ready."
+  ^Boolean []
+  @setup-db-has-been-called?)
+
 (def ^:dynamic *allow-potentailly-unsafe-connections*
   "We want to make *every* database connection made by the drivers safe -- read-only, only connect if DB file exists, etc.
    At the same time, we'd like to be able to use driver functionality like `can-connect-with-details?` to check whether we can
@@ -360,11 +365,11 @@
   [& {:keys [db-details auto-migrate]
       :or   {db-details   @db-connection-details
              auto-migrate true}}]
-  (reset! setup-db-has-been-called? true)
   (verify-db-connection db-details)
   (run-schema-migrations! auto-migrate db-details)
   (create-connection-pool! (jdbc-details db-details))
-  (run-data-migrations!))
+  (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."

src/metabase/middleware.clj

@@ -9,6 +9,7 @@
             [metabase.api.common :refer [*current-user* *current-user-id* *is-superuser?* *current-user-permissions-set*]]
             [metabase.api.common.internal :refer [*automatically-catch-api-exceptions*]]
             [metabase.config :as config]
+            [metabase.core.initialization-status :as init-status]
             [metabase.db :as mdb]
             (metabase.models [session :refer [Session]]
                              [setting :refer [defsetting]]
@@ -89,9 +90,7 @@
 (defn- current-user-info-for-session
   "Return User ID and superuser status for Session with SESSION-ID if it is valid and not expired."
   [session-id]
-  (when (and session-id (or ((resolve 'metabase.core/initialized?))
-                            (println "Metabase is not initialized!") ; NOCOMMIT
-                            ))
+  (when (and session-id (init-status/complete?))
     (when-let [session (or (session-with-id session-id)
                            (println "no matching session with ID") ; NOCOMMIT
                            )]
@@ -101,7 +100,7 @@
          :is-superuser?    (:is_superuser session)}))))
 
 (defn- add-current-user-info [{:keys [metabase-session-id], :as request}]
-  (when-not ((resolve 'metabase.core/initialized?))
+  (when-not (init-status/complete?)
     (println "Metabase is not initialized yet!")) ; DEBUG
   (merge request (current-user-info-for-session metabase-session-id)))
 
@@ -261,10 +260,11 @@
   "Middleware to set the `site-url` Setting if it's unset the first time a request is made."
   [handler]
   (fn [{{:strs [origin host] :as headers} :headers, :as request}]
-    (when-not (public-settings/site-url)
-      (when-let [site-url (or origin host)]
-        (log/info "Setting Metabase site URL to" site-url)
-        (public-settings/site-url site-url)))
+    (when (mdb/db-is-setup?)
+      (when-not (public-settings/site-url)
+        (when-let [site-url (or origin host)]
+          (log/info "Setting Metabase site URL to" site-url)
+          (public-settings/site-url site-url))))
     (handler request)))
 
 

src/metabase/public_settings.clj

@@ -23,11 +23,13 @@
   :default "Metabase")
 
 ;; This value is *guaranteed* to never have a trailing slash :D
+;; It will also prepend `http://` to the URL if there's not protocol when it comes in
 (defsetting site-url
   "The base URL of this Metabase instance, e.g. \"http://metabase.my-company.com\"."
   :setter (fn [new-value]
             (setting/set-string! :site-url (when new-value
-                                             (s/replace new-value #"/$" "")))))
+                                             (cond->> (s/replace new-value #"/$" "")
+                                               (not (s/starts-with? new-value "http")) (str "http://"))))))
 
 (defsetting admin-email
   "The email address users should be referred to if they encounter a problem.")

src/metabase/routes.clj

@@ -6,13 +6,14 @@
             [ring.util.response :as resp]
             [stencil.core :as stencil]
             [metabase.api.routes :as api]
+            [metabase.core.initialization-status :as init-status]
             [metabase.public-settings :as public-settings]
             [metabase.util :as u]
             [metabase.util.embed :as embed]))
 
 
 (defn- entrypoint [entry embeddable? {:keys [uri]}]
-  (-> (if ((resolve 'metabase.core/initialized?))
+  (-> (if (init-status/complete?)
         (stencil/render-string (slurp (or (io/resource (str "frontend_client/" entry ".html"))
                                           (throw (Exception. (str "Cannot find './resources/frontend_client/" entry ".html'. Did you remember to build the Metabase frontend?")))))
                                {:bootstrap_json (json/generate-string (public-settings/public-settings))
@@ -40,9 +41,17 @@
   ;; ^/$ -> index.html
   (GET "/" [] index)
   (GET "/favicon.ico" [] (resp/resource-response "frontend_client/favicon.ico"))
-  ;; ^/api/ -> API routes
-  (context "/api" [] api/routes)
-  ; ^/app/ -> static files under frontend_client/app
+  ;; ^/api/health -> Health Check Endpoint
+  (GET "/api/health" [] (if (init-status/complete?)
+                          {:status 200, :body {:status "ok"}}
+                          {:status 503, :body {:status "initializing", :progress (init-status/progress)}}))
+  ;; ^/api/ -> All other API routes
+  (context "/api" [] (fn [& args]
+                       ;; if Metabase is not finished initializing, return a generic error message rather than something potentially confusing like "DB is not set up"
+                       (if-not (init-status/complete?)
+                         {:status 503, :body "Metabase is still initializing. Please sit tight..."}
+                         (apply api/routes args))))
+  ;; ^/app/ -> static files under frontend_client/app
   (context "/app" []
     (route/resources "/" {:root "frontend_client/app"})
     ;; return 404 for anything else starting with ^/app/ that doesn't exist

test/metabase/publc_settings_test.clj

@@ -3,9 +3,35 @@
             [metabase.public-settings :as public-settings]
             [metabase.test.util :as tu]))
 
-;; double-check that setting the `site-url` setting will automatically strip off trailing slashes
+ ;; double-check that setting the `site-url` setting will automatically strip off trailing slashes
 (expect
   "http://localhost:3000"
   (tu/with-temporary-setting-values [site-url nil]
     (public-settings/site-url "http://localhost:3000/")
     (public-settings/site-url)))
+
+ ;; double-check that setting the `site-url` setting will prepend `http://` if no protocol was specified
+(expect
+  "http://localhost:3000"
+  (tu/with-temporary-setting-values [site-url nil]
+    (public-settings/site-url "localhost:3000")
+    (public-settings/site-url)))
+
+(expect
+  "http://localhost:3000"
+  (tu/with-temporary-setting-values [site-url nil]
+    (public-settings/site-url "localhost:3000")
+    (public-settings/site-url)))
+
+(expect
+  "http://localhost:3000"
+  (tu/with-temporary-setting-values [site-url nil]
+    (public-settings/site-url "http://localhost:3000")
+    (public-settings/site-url)))
+
+;; if https:// was specified it should keep it
+(expect
+  "https://localhost:3000"
+  (tu/with-temporary-setting-values [site-url nil]
+    (public-settings/site-url "https://localhost:3000")
+    (public-settings/site-url)))

test/metabase/test/data/users.clj

@@ -3,8 +3,8 @@
   ;; TODO - maybe this namespace should just be `metabase.test.users`.
   (:require [medley.core :as m]
             [toucan.db :as db]
-            (metabase [config :as config]
-                      [core :as core])
+            [metabase.config :as config]
+            [metabase.core.initialization-status :as init-status]
             [metabase.http-client :as http]
             (metabase.models [permissions-group :as perms-group]
                              [user :refer [User]])
@@ -56,7 +56,7 @@
    ;; only need to wait when running unit tests. When doing REPL dev and using the test users we're probably
    ;; the server is probably a separate process (`lein ring server`)
    (when config/is-test?
-     (when-not (core/initialized?)
+     (when-not (init-status/complete?)
        (when (<= max-wait-seconds 0)
          (throw (Exception. "Metabase still hasn't finished initializing.")))
        (println (format "Metabase is not yet initialized, waiting 1 second (max wait remaining: %d seconds)..." max-wait-seconds))

test/metabase/test_setup.clj

@@ -5,8 +5,9 @@
             [clojure.set :as set]
             [clojure.tools.logging :as log]
             [expectations :refer :all]
-            (metabase [core :as core]
-                      [db :as mdb]
+            [metabase.core :as core]
+            [metabase.core.initialization-status :as init-status]
+            (metabase [db :as mdb]
                       [driver :as driver])
             (metabase.models [setting :as setting]
                              [table :refer [Table]])
@@ -79,7 +80,7 @@
       (log/info (format "Setting up %s test DB and running migrations..." (name (mdb/db-type))))
       (mdb/setup-db! :auto-migrate true)
       (setting/set! :site-name "Metabase Test")
-      (core/initialization-complete!)
+      (init-status/set-complete!)
 
       ;; make sure the driver test extensions are loaded before running the tests. :reload them because otherwise we get wacky 'method in protocol not implemented' errors
       ;; when running tests against an individual namespace