Fix unspecified PG database connection strings

This issue can occur if the `MB_DB_DBNAME` variable has not been
specified. The `:dbname` key has a nil value which causes Ring's
`form-encode` function to fail. This commit explicitly removes the
`:dbname` as it's extra anyway and will also remove map entries that
have a nil key or value before encoding it.

src/metabase/db/spec.clj

@@ -3,9 +3,9 @@
   Only databases that are supported as application DBs should have functions in this namespace;
   otherwise, similar functions are only needed by drivers, and belong in those namespaces."
   (:require [clojure.string :as str]
+            [medley.core :as m]
             [ring.util.codec :as codec]))
 
-
 (defn h2
   "Create a database specification for a h2 database. Opts should include a key
   for :db which is the path to the database file."
@@ -18,11 +18,14 @@
          (dissoc opts :db)))
 
 (defn- remove-required-keys [db-spec & more-keys]
-  (apply dissoc db-spec (concat [:host :port :db :user :password :additional-options]
+  (apply dissoc db-spec (concat [:host :port :db :dbname :user :password :additional-options]
                                 more-keys)))
 
+(defn- purge-nil-values [m]
+  (m/remove-kv (fn [k v] (or (nil? k) (nil? v))) m))
+
 (defn- make-subname [host port db extra-connection-params]
-  (let [query-params (codec/form-encode extra-connection-params)]
+  (let [query-params (codec/form-encode (purge-nil-values extra-connection-params))]
     (str "//" host ":" port "/" db (when-not (str/blank? query-params)
                                      (str "?" query-params)))))
 

test/metabase/db/spec_test.clj

+(ns metabase.db.spec-test
+  (:require [expectations :refer :all]
+            [metabase.db.spec :refer :all]))
+
+(defn- default-pg-spec [db]
+  {:classname "org.postgresql.Driver", :subprotocol "postgresql",
+   :subname (format "//localhost:5432/%s?OpenSourceSubProtocolOverride=true" db)})
+
+;; Basic minimal config
+(expect
+  (default-pg-spec "metabase")
+  (postgres {:host "localhost"
+             :port 5432
+             :db   "metabase"}))
+
+;; Users that don't specify a `:dbname` (and thus no `:db`) will use the user's default, we should allow that
+(expect
+  (assoc (default-pg-spec "") :dbname nil)
+  (postgres {:host   "localhost"
+             :port   5432
+             :dbname nil
+             :db     nil}))
+
+;; We should be tolerant of other random nil values sneaking through
+(expect
+  (assoc (default-pg-spec "") :dbname nil, :somethingrandom nil)
+  (postgres {:host            "localhost"
+             :port            5432
+             :dbname          nil
+             :db              nil
+             :somethingrandom nil}))
+
+;; Not specifying any of the values results in defaults
+(expect
+  (default-pg-spec "")
+  (postgres {}))