Add MySQL latest, MariaDB 10.2, and MariaDB latest to CI (#11769)

* Add MySQL latest, MariaDB 10.2, and MariaDB latest to CI [ci mysql]

* Add upper limit to number of results returned by GET /api/search endpoint

* Rewrite tests in metabase.api.search-test to use new style

* Rework DB & test component init code so they can be retried after failing without having to restart REPL/redefine vars

* Log application DB product/version info; include in troubleshooting info

* Log DB version info when starting tests

.circleci/config.yml

@@ -71,6 +71,49 @@ executors:
           MB_ENCRYPTION_SECRET_KEY: << parameters.encryption-key >>
       - image: circleci/mysql:5.7.23
 
+  mysql-latest:
+    working_directory: /home/circleci/metabase/metabase/
+    docker:
+      - image: circleci/clojure:lein-2.8.1
+        environment:
+          MB_DB_TYPE: mysql
+          MB_DB_HOST: localhost
+          MB_DB_PORT: 3306
+          MB_DB_DBNAME: circle_test
+          MB_DB_USER: root
+          MB_MYSQL_TEST_USER: root
+      - image: circleci/mysql:latest
+
+  mariadb-10-2:
+    working_directory: /home/circleci/metabase/metabase/
+    docker:
+      - image: circleci/clojure:lein-2.8.1
+        environment:
+          MB_DB_TYPE: mysql
+          MB_DB_HOST: localhost
+          MB_DB_PORT: 3306
+          MB_DB_DBNAME: circle_test
+          MB_DB_USER: root
+          MB_MYSQL_TEST_USER: root
+      - image: circleci/mariadb:10.2.23
+
+  mariadb-latest:
+    working_directory: /home/circleci/metabase/metabase/
+    docker:
+      - image: circleci/clojure:lein-2.8.1
+        environment:
+          MB_DB_TYPE: mysql
+          MB_DB_HOST: localhost
+          MB_DB_PORT: 3306
+          MB_DB_DBNAME: metabase_test
+          MB_DB_USER: root
+          MB_MYSQL_TEST_USER: root
+      - image: mariadb:latest
+        environment:
+          MYSQL_DATABASE: metabase_test
+          MYSQL_USER: root
+          MYSQL_ALLOW_EMPTY_PASSWORD: yes
+
   mongo:
      working_directory: /home/circleci/metabase/metabase/
      docker:
@@ -290,6 +333,9 @@ jobs:
       auto-retry:
         type: boolean
         default: false
+      description:
+        type: string
+        default: ""
     executor: << parameters.e >>
     steps:
       - attach-workspace
@@ -319,7 +365,7 @@ jobs:
           condition: << parameters.auto-retry >>
           steps:
             - run:
-                name: Test << parameters.driver >> driver
+                name: Test << parameters.driver >> driver << parameters.description >>
                 environment:
                   DRIVERS: h2,<< parameters.driver >>
                 command: >
@@ -593,6 +639,7 @@ workflows:
 
       - test-driver:
           name: be-tests-mysql
+          description: "(MySQL 5.7)"
           requires:
             - be-tests
           e:
@@ -600,6 +647,33 @@ workflows:
             encryption-key: Orw0AAyzkO/kPTLJRxiyKoBHXa/d6ZcO+p+gpZO/wSQ=
           driver: mysql
 
+      - test-driver:
+          name: be-tests-mysql-latest
+          description: "(MySQL latest)"
+          requires:
+            - be-tests
+          e:
+            name: mysql-latest
+          driver: mysql
+
+      - test-driver:
+          name: be-tests-mariadb
+          description: "(MariaDB 10.2)"
+          requires:
+            - be-tests
+          e:
+            name: mariadb-10-2
+          driver: mysql
+
+      - test-driver:
+          name: be-tests-mariadb-latest
+          description: "(MariaDB latest)"
+          requires:
+            - be-tests
+          e:
+            name: mariadb-latest
+          driver: mysql
+
       - test-driver:
           name: be-tests-oracle
           requires:
@@ -737,6 +811,9 @@ workflows:
             - be-tests-googleanalytics
             - be-tests-mongo
             - be-tests-mysql
+            - be-tests-mysql-latest
+            - be-tests-mariadb
+            - be-tests-mariadb-latest
             - be-tests-oracle
             - be-tests-postgres
             - be-tests-presto

src/metabase/api/search.clj

 (ns metabase.api.search
   (:require [clojure.string :as str]
+            [clojure.tools.logging :as log]
             [compojure.core :refer [GET]]
             [flatland.ordered.map :as ordered-map]
             [honeysql
@@ -26,6 +27,12 @@
             [schema.core :as s]
             [toucan.db :as db]))
 
+(def ^:private ^:const search-max-results
+  "Absolute maximum number of search results to return. This number is in place to prevent massive application DB load
+  by returning tons of results; this number should probably be adjusted downward once we have UI in place to indicate
+  that results are truncated."
+  1000)
+
 (def ^:private SearchContext
   "Map with the various allowed search parameters, used to construct the SQL query"
   {:search-string      (s/maybe su/NonBlankString)
@@ -33,8 +40,15 @@
    :current-user-perms #{perms/UserPath}})
 
 (def ^:private searchable-models
+  "Models that can be searched. Results also come back in this order (i.e., all matching Cards, followed by all matching
+  Dashboards, etc.)"
   [Card Dashboard Pulse Collection Segment Metric Table])
 
+(def ^:private model->sort-position
+  (into {} (map-indexed (fn [i model]
+                          [(str/lower-case (name model)) i])
+                        searchable-models)))
+
 (def ^:private SearchableModel
   (apply s/enum searchable-models))
 
@@ -326,15 +340,24 @@
 (s/defn ^:private search
   "Builds a search query that includes all of the searchable entities and runs it"
   [search-ctx :- SearchContext]
-  (for [row (db/query {:union-all (for [model searchable-models
-                                        :let  [query (search-query-for-model model search-ctx)]
-                                        :when (seq query)]
-                                    query)})]
-    ;; MySQL returns `:favorite` as `1` or `0` so convert those to boolean as needed
-    (update row :favorite (fn [favorite]
-                            (if (integer? favorite)
-                              (not (zero? favorite))
-                              favorite)))))
+  (letfn [(bit->boolean [v]
+            (if (number? v)
+              (not (zero? v))
+              v))]
+    (let [search-query {:union-all (for [model searchable-models
+                                         :let  [query (search-query-for-model model search-ctx)]
+                                         :when (seq query)]
+                                     query)}
+          _            (log/tracef "Searching with query:\n%s" (u/pprint-to-str search-query))
+          ;; sort results by [model name]
+          results      (sort-by (juxt (comp model->sort-position :model)
+                                      :name)
+                                (db/query search-query :max-rows search-max-results))]
+      (for [row results]
+        ;; MySQL returns `:favorite` and `:archived` as `1` or `0` so convert those to boolean as needed
+        (-> row
+            (update :favorite bit->boolean)
+            (update :archived bit->boolean))))))
 
 
 ;;; +----------------------------------------------------------------------------------------------------------------+

src/metabase/api/util.clj

@@ -39,7 +39,7 @@
 (api/defendpoint GET "/bug_report_details"
   []
   (api/check-superuser)
-  {:system-info (troubleshooting/system-info)
+  {:system-info   (troubleshooting/system-info)
    :metabase-info (troubleshooting/metabase-info)})
 
 (api/define-routes)

src/metabase/db.clj

@@ -256,7 +256,7 @@
   false)
 
 (s/defn ^:private verify-db-connection
-  "Test connection to database with DETAILS and throw an exception if we have any troubles connecting."
+  "Test connection to database with `details` and throw an exception if we have any troubles connecting."
   ([db-details]
    (verify-db-connection (:type db-details) db-details))
 
@@ -266,7 +266,10 @@
              (classloader/require 'metabase.driver.util)
              ((resolve 'metabase.driver.util/can-connect-with-details?) driver details :throw-exceptions))
      (trs "Unable to connect to Metabase {0} DB." (name driver)))
-   (log/info (trs "Verify Database Connection ... ") (u/emoji "✅"))))
+   (jdbc/with-db-metadata [metadata (jdbc-spec details)]
+     (log/info (trs "Successfully verified {0} {1} application database connection."
+                    (.getDatabaseProductName metadata) (.getDatabaseProductVersion metadata))
+               (u/emoji "✅")))))
 
 (def ^:dynamic ^Boolean *disable-data-migrations*
   "Should we skip running data migrations when setting up the DB? (Default is `false`).
@@ -309,7 +312,8 @@
     ((resolve 'metabase.db.migrations/run-all!))))
 
 (defn setup-db!*
-  "Connects to db and runs migrations."
+  "Connects to db and runs migrations. Don't use this directly, unless you know what you're doing; use `setup-db!`
+  instead, which can be called more than once without issue and is thread-safe."
   [db-details auto-migrate]
   (u/profile (trs "Database setup")
     (u/with-us-locale
@@ -326,13 +330,22 @@
     (reset! db-setup-finished? true))
   nil)
 
-(defonce ^{:arglists '([]), :doc "Do general preparation of database by validating that we can connect. Caller can
-  specify if we should run any pending database migrations. If DB is already set up, this function will no-op."}
-  setup-db!
-  (partial deref (delay (setup-db-from-env!*))))
+(defonce ^:private db-setup-complete? (atom false))
+(defonce ^:private setup-db-lock (Object.))
+
+(defn setup-db!
+  "Do general preparation of database by validating that we can connect. Caller can specify if we should run any pending
+  database migrations. If DB is already set up, this function will no-op. Thread-safe."
+  []
+  (when-not @db-setup-complete?
+    (locking setup-db-lock
+      (when-not @db-setup-complete?
+        (setup-db-from-env!*)
+        (reset! db-setup-complete? true))))
+  :done)
 
 
-;;; Various convenience fns (experiMENTAL)
+;;; Various convenience fns
 
 (defn join
   "Convenience for generating a HoneySQL `JOIN` clause.

src/metabase/driver/mysql.clj

@@ -254,12 +254,10 @@
        (-> (dbspec/mysql details)
            (sql-jdbc.common/handle-additional-options details))))))
 
-
 (defmethod sql-jdbc.sync/active-tables :mysql
   [& args]
   (apply sql-jdbc.sync/post-filtered-active-tables args))
 
-
 (defmethod sql-jdbc.sync/excluded-schemas :mysql
   [_]
   #{"INFORMATION_SCHEMA"})

src/metabase/troubleshooting.clj

 (ns metabase.troubleshooting
-  (:require [metabase
+  (:require [clojure.java.jdbc :as jdbc]
+            [metabase
              [config :as mc]
              [db :as mdb]]
             [metabase.models.setting :as setting]
             [metabase.util.stats :as mus]
-            [toucan.db :as tdb]))
+            [toucan.db :as db]))
 
 (defn system-info
   "System info we ask for for bug reports"
@@ -25,9 +26,14 @@
 (defn metabase-info
   "Make it easy for the user to tell us what they're using"
   []
-  {:databases            (->> (tdb/select 'Database) (map :engine) distinct)
-   :hosting-env          (mus/environment-type)
-   :application-database (mdb/db-type)
-   :run-mode             (mc/config-kw :mb-run-mode)
-   :version              mc/mb-version-info
-   :settings             {:report-timezone (setting/get :report-timezone)}})
+  {:databases                    (->> (db/select 'Database) (map :engine) distinct)
+   :hosting-env                  (mus/environment-type)
+   :application-database         (mdb/db-type)
+   :application-database-details (jdbc/with-db-metadata [metadata (db/connection)]
+                                   {:database    {:name    (.getDatabaseProductName metadata)
+                                                  :version (.getDatabaseProductVersion metadata)}
+                                    :jdbc-driver {:name    (.getDriverName metadata)
+                                                  :version (.getDriverVersion metadata)}})
+   :run-mode                     (mc/config-kw :mb-run-mode)
+   :version                      mc/mb-version-info
+   :settings                     {:report-timezone (setting/get :report-timezone)}})

test/metabase/test/data/mysql.clj

@@ -48,9 +48,4 @@
   [& args]
   (apply load-data/load-data-all-at-once! args))
 
-#_(defmethod load-data/do-insert! :mysql
-  [driver spec table-identifier row-or-rows]
-  (jdbc/execute! spec "SET @@session.time_zone = 'UTC'");
-  ((get-method load-data/do-insert! :sql-jdbc/test-extensions) driver spec table-identifier row-or-rows))
-
 (defmethod sql.tx/pk-sql-type :mysql [_] "INTEGER NOT NULL AUTO_INCREMENT")

test/metabase/test/initialize.clj

@@ -7,45 +7,10 @@
              [util :as u]]
             [metabase.plugins.classloader :as classloader]))
 
-
-;; (def ^:private ^:dynamic *require-chain* nil)
-
-;; (defonce new-require
-;;   (let [orig-require (var-get #'clojure.core/require)]
-;;     (orig-require 'clojure.pprint)
-;;     (fn [& args]
-;;       (binding [*require-chain* (conj (vec *require-chain*) (ns-name *ns*))]
-;;         (let [require-chain-description (apply str (interpose " -> " *require-chain*))]
-;;           (println "\nin" require-chain-description)
-;;           ((resolve 'clojure.pprint/pprint) (cons 'require args))
-;;           (apply orig-require args)
-;;           (println "finished" require-chain-description))))))
-
-;; (intern 'clojure.core 'require new-require)
-
-(defmulti initialize-if-needed!
-  "Initialize one or more components.
-
-    (initialize-if-needed! :db :web-server)"
-  (fn
-    ([k]        (keyword k))
-    ([k & more] :many)))
-
-(defonce ^:private initialized (atom #{}))
-
-(defn initialized?
-  "Has this component been initialized?"
-  ([k]
-   (contains? @initialized k))
-
-  ([k & more]
-   (and (initialized? k)
-        (apply initialized? more))))
-
-(defmethod initialize-if-needed! :many
-  [& args]
-  (doseq [k args]
-    (initialize-if-needed! k)))
+(defmulti ^:private do-initialization!
+  "Perform component-specific initialization. This is guaranteed to only be called once."
+  {:arglists '([init-setp])}
+  keyword)
 
 (defn- log-init-message [task-name]
   (let [body   (format "| Initializing %s... |" task-name)
@@ -58,36 +23,57 @@
 
 (def ^:private init-timeout-ms (* 30 1000))
 
-(def ^:private ^:dynamic *initializing* [])
+(def ^:private ^:dynamic *initializing*
+  "Collection of components that are being currently initialized by the current thread."
+  [])
+
+(defonce ^:private initialized (atom #{}))
 
-(defn- deref-init-delay [task-name a-delay]
+(defn- check-for-circular-deps [step]
+  (when (contains? (set *initializing*) step)
+    (throw (Exception. (format "Circular initialization dependencies! %s"
+                               (str/join " -> " (conj *initializing* step)))))))
+
+(defn- initialize-if-needed!* [step]
   (try
-    (when (contains? (set *initializing*) task-name)
-      (throw (Exception. (format "Circular initialization dependencies! %s"
-                                 (str/join " -> " (conj *initializing* task-name))))))
-    (binding [*initializing* (conj *initializing* task-name)]
+    (log-init-message step)
+    (binding [*initializing* (conj *initializing* step)]
       (u/with-timeout init-timeout-ms
-        @a-delay))
+        (do-initialization! step)))
     (catch Throwable e
-      (println "Error initializing" task-name)
+      (println "Error initializing" step)
       (println e)
       (when config/is-test?
         (System/exit -1))
       (throw e))))
 
+(defn initialize-if-needed!
+  "Initialize one or more components.
+
+    (initialize-if-needed! :db :web-server)"
+  [& steps]
+  (doseq [step steps
+          :let [step (keyword step)]]
+    (when-not (@initialized step)
+      (check-for-circular-deps step)
+      (locking step
+        (when-not (@initialized step)
+          (initialize-if-needed!* step)
+          (swap! initialized conj step))))))
+
+(defn initialized?
+  "Has this component been initialized?"
+  ([k]
+   (contains? @initialized k))
+
+  ([k & more]
+   (and (initialized? k)
+        (apply initialized? more))))
+
 (defmacro ^:private define-initialization [task-name & body]
-  (let [delay-symb (-> (symbol (format "init-%s-%d" (name task-name) (hash &form)))
-                       (with-meta {:private true}))]
-    `(do
-       (defonce ~delay-symb
-         (delay
-           (log-init-message ~(keyword task-name))
-           (swap! initialized conj ~(keyword task-name))
-           ~@body
-           ~(keyword task-name)))
-       (defmethod initialize-if-needed! ~(keyword task-name)
-         [~'_]
-         (deref-init-delay ~(keyword task-name) ~delay-symb)))))
+  `(defmethod do-initialization! ~(keyword task-name)
+     [~'_]
+     ~@body))
 
 (define-initialization :plugins
   (classloader/require 'metabase.test.initialize.plugins)
@@ -114,5 +100,10 @@
   (classloader/require 'metabase.test.initialize.test-users-personal-collections)
   ((resolve 'metabase.test.initialize.test-users-personal-collections/init!)))
 
-(alter-meta! #'initialize-if-needed! assoc :arglists (list (into ['&] (sort (disj (set (keys (methods initialize-if-needed!)))
-                                                                                  :many)))))
+(defn- all-components
+  "Set of all components/initialization steps that are defined."
+  []
+  (set (keys (methods do-initialization!))))
+
+;; change the arglists for `initialize-if-needed!` to list all the possible args for REPL-usage convenience
+(alter-meta! #'initialize-if-needed! assoc :arglists (list (into ['&] (sort (all-components)))))

test/metabase/test/initialize/db.clj

 (ns metabase.test.initialize.db
-  (:require [metabase
+  (:require [clojure.java.jdbc :as jdbc]
+            [metabase
              [db :as mdb]
-             [task :as task]]))
+             [task :as task]
+             [util :as u]]
+            [toucan.db :as db]))
 
 (defn init! []
-  (println (format "Setting up %s test DB and running migrations..." (mdb/db-type)))
+  (println (u/format-color 'blue "Setting up %s test DB and running migrations..." (mdb/db-type)))
   (#'task/set-jdbc-backend-properties!)
-  (mdb/setup-db!))
+  (mdb/setup-db!)
+  (jdbc/with-db-metadata [metadata (db/connection)]
+    (println (u/format-color 'blue "Application DB is %s %s" (.getDatabaseProductName metadata) (.getDatabaseProductVersion metadata)))))