More cleanup

src/metabase/models/database.clj

@@ -45,12 +45,9 @@
   "Return a *sorted set* of schema names (as strings) associated with this `Database`."
   [{:keys [id]}]
   (when id
-    (apply sorted-set (sort-by (fn [schema-name]
-                                 (when schema-name
-                                   (s/lower-case schema-name)))
-                               (db/select-field :schema 'Table
-                                 :db_id id
-                                 {:modifiers [:DISTINCT]})))))
+    (apply sorted-set (db/select-field :schema 'Table
+                        :db_id id
+                        {:modifiers [:DISTINCT]}))))
 
 (defn schema-exists?
   "Does DATABASE have any tables with SCHEMA?"

src/metabase/util.clj

@@ -344,15 +344,28 @@
        (every? map? coll)))
 
 (defn maybe?
-  "True if X is `nil`, or if it satisfies PRED. (PRED is only called if X is non-nil.)
+  "Returns `true` if X is `nil`, otherwise calls (F X).
+   This can be used to see something is either `nil` or statisfies a predicate function:
 
-     (string? nil)        -> false
-     (string? \"A\")      -> true
-     (maybe? string? nil) -> true
-     (maybe? string? \"A\") -> true"
-  [pred x]
+     (string? nil)          -> false
+     (string? \"A\")        -> true
+     (maybe? string? nil)   -> true
+     (maybe? string? \"A\") -> true
+
+   It can also be used to make sure a given function won't throw a `NullPointerException`:
+
+     (s/lower-case nil)            -> NullPointerException
+     (s/lower-case \"ABC\")        -> \"abc\"
+     (maybe? s/lower-case nil)     -> true
+     (maybe? s/lower-case \"ABC\") -> \"abc\"
+
+   The latter use-case can be useful for things like sorting where some values in a collection
+   might be `nil`:
+
+     (sort-by (partial maybe? s/lower-case) some-collection)"
+  [f x]
   (or (nil? x)
-      (pred x)))
+      (f x)))
 
 
 (def ^:private ^:const host-up-timeout

test/metabase/test/data/sqlserver.clj

@@ -24,7 +24,7 @@
    :TimeField       "TIME"})
 
 (def ^:private db-name-counter
-  "We destroy and create the same temporary databases serveral times when running our query processor tests.
+  "We destroy and create the same temporary databases serveral times when running our query processor tests. (why?)
 
    To kick other users off of the database when we destroy it, we `ALTER DATABASE SET SINGLE_USER ROLLBACK IMMEDIATE`.
    This has the side effect of preventing any other connections to the database. If our tests barf for any reason,
@@ -103,7 +103,7 @@
             :engine                       (constantly :sqlserver)})))
 
 
-(defn- cleanup-leftover-dbs
+(defn- cleanup-leftover-dbs!
   "Clean up any leftover DBs that weren't destroyed by the last test run (eg, if it failed for some reason).
    This is important because we're limited to a quota of 30 DBs on RDS."
   {:expectations-options :before-run}