Merge pull request #4656 from metabase/more-fixes

More fixes

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

@@ -260,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.")

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)))