From 128a4d00d083da72b50eb9396074726a1cf74d64 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Tue, 21 Jan 2020 15:30:07 -0800
Subject: [PATCH] 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 | 79 +++++-
src/metabase/api/search.clj | 41 ++-
src/metabase/api/util.clj | 2 +-
src/metabase/db.clj | 29 +-
src/metabase/driver/mysql.clj | 2 -
src/metabase/troubleshooting.clj | 22 +-
test/metabase/api/search_test.clj | 385 +++++++++++++--------------
test/metabase/test/data/mysql.clj | 5 -
test/metabase/test/initialize.clj | 113 ++++----
test/metabase/test/initialize/db.clj | 13 +-
10 files changed, 387 insertions(+), 304 deletions(-)
diff --git a/.circleci/config.yml b/.circleci/config.yml
index f3cf2230515..d25f8e644ad 100644
--- a/.circleci/config.yml
+++ b/.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
diff --git a/src/metabase/api/search.clj b/src/metabase/api/search.clj
index 524fa8fd359..d17d943cd7b 100644
--- a/src/metabase/api/search.clj
+++ b/src/metabase/api/search.clj
@@ -1,5 +1,6 @@
(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))))))
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/src/metabase/api/util.clj b/src/metabase/api/util.clj
index 2270e7428f3..118b1266794 100644
--- a/src/metabase/api/util.clj
+++ b/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)
diff --git a/src/metabase/db.clj b/src/metabase/db.clj
index 94acb70de1c..2554d22329c 100644
--- a/src/metabase/db.clj
+++ b/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.
diff --git a/src/metabase/driver/mysql.clj b/src/metabase/driver/mysql.clj
index 67d9332f08d..cc0fcd6e663 100644
--- a/src/metabase/driver/mysql.clj
+++ b/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"})
diff --git a/src/metabase/troubleshooting.clj b/src/metabase/troubleshooting.clj
index c6a30838e46..437ea8af643 100644
--- a/src/metabase/troubleshooting.clj
+++ b/src/metabase/troubleshooting.clj
@@ -1,10 +1,11 @@
(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)}})
diff --git a/test/metabase/api/search_test.clj b/test/metabase/api/search_test.clj
index 7119db87a57..24cb63b3c92 100644
--- a/test/metabase/api/search_test.clj
+++ b/test/metabase/api/search_test.clj
@@ -1,29 +1,19 @@
(ns metabase.api.search-test
(:require [clojure
[set :as set]
- [string :as str]]
- [expectations :refer [expect]]
+ [string :as str]
+ [test :refer :all]]
+ [metabase
+ [models :refer [Card CardFavorite Collection Dashboard DashboardFavorite Database Metric PermissionsGroup
+ PermissionsGroupMembership Pulse Segment Table]]
+ [test :as mt]
+ [util :as u]]
+ [metabase.api.search :as api.search]
[metabase.models
- [card :refer [Card]]
- [card-favorite :refer [CardFavorite]]
- [collection :as coll :refer [Collection]]
- [dashboard :refer [Dashboard]]
- [dashboard-favorite :refer [DashboardFavorite]]
- [database :refer [Database]]
- [metric :refer [Metric]]
[permissions :as perms]
- [permissions-group :as group :refer [PermissionsGroup]]
- [permissions-group-membership :refer [PermissionsGroupMembership]]
- [pulse :refer [Pulse]]
- [segment :refer [Segment]]
- [table :refer [Table]]]
- [metabase.test
- [data :as data]
- [util :as tu]]
+ [permissions-group :as group]]
[metabase.test.data.users :as test-users]
- [metabase.util :as u]
- [toucan.db :as db]
- [toucan.util.test :as tt]))
+ [toucan.db :as db]))
(def ^:private default-search-row
{:id true
@@ -47,10 +37,10 @@
(merge
{:table_id true, :database_id true}
(db/select-one [Table [:name :table_name] [:schema :table_schema] [:description :table_description]]
- :id (data/id :checkins))))
+ :id (mt/id :checkins))))
(defn- sorted-results [results]
- (sort-by (juxt :model :name) results))
+ (sort-by (juxt (comp (var-get #'api.search/model->sort-position) :model) :name) results))
(defn- default-search-results []
(sorted-results
@@ -101,7 +91,7 @@
(merge (data-map instance-name)
(when-not in-root-collection?
{:collection_id (u/get-id collection)})))]
- (tt/with-temp* [Collection [coll (data-map "collection %s collection")]
+ (mt/with-temp* [Collection [coll (data-map "collection %s collection")]
Card [card (coll-data-map "card %s card" coll)]
Dashboard [dashboard (coll-data-map "dashboard %s dashboard" coll)]
Pulse [pulse (coll-data-map "pulse %s pulse" coll)]
@@ -126,176 +116,167 @@
(let [raw-results (apply (test-users/user->client user-kwd) :get 200 "search" params)]
(for [result raw-results
;; filter out any results not from the usual test data DB (e.g. results from other drivers)
- :when (contains? #{(data/id) nil} (:database_id result))]
+ :when (contains? #{(mt/id) nil} (:database_id result))]
(-> result
- tu/boolean-ids-and-timestamps
+ mt/boolean-ids-and-timestamps
(update :collection_name #(some-> % string?))))))))
-;; Basic search, should find 1 of each entity type, all items in the root collection
-(expect
- (default-search-results)
- (with-search-items-in-root-collection "test"
- (search-request :crowberto :q "test")))
-
-;; Search with no search string. Note this search everything in the DB, including any stale data left behind from
-;; previous tests. Instead of an = comparison here, just ensure our default results are included
-(expect
- (set/subset?
- (set (default-search-results))
- (set (with-search-items-in-root-collection "test"
- (search-request :crowberto)))))
-
-;; Ensure that users without perms for the root collection don't get results
-;; NOTE: Metrics and segments don't have collections, so they'll be returned
-(expect
- (default-metric-segment-results)
- (tu/with-non-admin-groups-no-root-collection-perms
+(deftest basic-test
+ (testing "Basic search, should find 1 of each entity type, all items in the root collection"
(with-search-items-in-root-collection "test"
- (search-request :rasta :q "test"))))
+ (is (= (default-search-results)
+ (search-request :crowberto :q "test")))))
-;; Users that have root collection permissions should get root collection search results
-(expect
- (remove (comp #{"collection"} :model) (default-search-results))
- (tu/with-non-admin-groups-no-root-collection-perms
+ (testing (str "Search with no search string. Note this search everything in the DB, including any stale data left "
+ "behind from previous tests. Instead of an = comparison here, just ensure our default results are "
+ "included")
(with-search-items-in-root-collection "test"
- (tt/with-temp* [PermissionsGroup [group]
- PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
- (perms/grant-permissions! group (perms/collection-read-path {:metabase.models.collection/is-root? true}))
- (search-request :rasta :q "test")))))
-
-;; Users without root collection permissions should still see other collections they have access to
-(expect
- (sorted-results
- (into
- (default-results-with-collection)
- (map #(merge default-search-row % (table-search-results))
- [{:name "metric test2 metric", :description "Lookin' for a blueberry", :model "metric"}
- {:name "segment test2 segment", :description "Lookin' for a blueberry", :model "segment"}])))
- (tu/with-non-admin-groups-no-root-collection-perms
- (with-search-items-in-collection {:keys [collection]} "test"
- (with-search-items-in-root-collection "test2"
- (tt/with-temp* [PermissionsGroup [group]
- PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
- (perms/grant-collection-read-permissions! group (u/get-id collection))
- (search-request :rasta :q "test"))))))
+ (is (set/subset?
+ (set (default-search-results))
+ (set (search-request :crowberto))))))
-;; Users with root collection permissions should be able to search root collection data long with collections they
-;; have access to
-(expect
- (sorted-results
- (into
- (default-results-with-collection)
- (for [row (default-search-results)
- :when (not= "collection" (:model row))]
- (update row :name #(str/replace % "test" "test2")))))
- (tu/with-non-admin-groups-no-root-collection-perms
- (with-search-items-in-collection {:keys [collection]} "test"
- (with-search-items-in-root-collection "test2"
- (tt/with-temp* [PermissionsGroup [group]
+ (testing "Basic search should only return substring matches"
+ (with-search-items-in-root-collection "test"
+ (with-search-items-in-root-collection "something different"
+ (is (= (default-search-results)
+ (search-request :crowberto :q "test")))))))
+
+(deftest permissions-test
+ (testing (str "Ensure that users without perms for the root collection don't get results NOTE: Metrics and segments "
+ "don't have collections, so they'll be returned")
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (with-search-items-in-root-collection "test"
+ (is (= (default-metric-segment-results)
+ (search-request :rasta :q "test"))))))
+
+ (testing "Users that have root collection permissions should get root collection search results"
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (with-search-items-in-root-collection "test"
+ (mt/with-temp* [PermissionsGroup [group]
PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
(perms/grant-permissions! group (perms/collection-read-path {:metabase.models.collection/is-root? true}))
- (perms/grant-collection-read-permissions! group collection)
- (search-request :rasta :q "test"))))))
-
-;; Users with access to multiple collections should see results from all collections they have access to
-(expect
- (sorted-results
- (into
- (default-results-with-collection)
- (map (fn [row] (update row :name #(str/replace % "test" "test2")))
- (default-results-with-collection))))
- (with-search-items-in-collection {coll-1 :collection} "test"
- (with-search-items-in-collection {coll-2 :collection} "test2"
- (tt/with-temp* [PermissionsGroup [group]
- PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
- (perms/grant-collection-read-permissions! group (u/get-id coll-1))
- (perms/grant-collection-read-permissions! group (u/get-id coll-2))
- (search-request :rasta :q "test")))))
-
-;; User should only see results in the collection they have access to
-(expect
- (sorted-results
- (into
- (default-results-with-collection)
- (map #(merge default-search-row % (table-search-results))
- [{:name "metric test2 metric", :description "Lookin' for a blueberry", :model "metric"}
- {:name "segment test2 segment", :description "Lookin' for a blueberry", :model "segment"}])))
- (tu/with-non-admin-groups-no-root-collection-perms
+ (is (= (remove (comp #{"collection"} :model) (default-search-results))
+ (search-request :rasta :q "test")))))))
+
+ (testing "Users without root collection permissions should still see other collections they have access to"
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (with-search-items-in-collection {:keys [collection]} "test"
+ (with-search-items-in-root-collection "test2"
+ (mt/with-temp* [PermissionsGroup [group]
+ PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
+ (perms/grant-collection-read-permissions! group (u/get-id collection))
+ (is (= (sorted-results
+ (into
+ (default-results-with-collection)
+ (map #(merge default-search-row % (table-search-results))
+ [{:name "metric test2 metric", :description "Lookin' for a blueberry", :model "metric"}
+ {:name "segment test2 segment", :description "Lookin' for a blueberry", :model "segment"}])))
+ (search-request :rasta :q "test"))))))))
+
+ (testing (str "Users with root collection permissions should be able to search root collection data long with "
+ "collections they have access to")
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (with-search-items-in-collection {:keys [collection]} "test"
+ (with-search-items-in-root-collection "test2"
+ (mt/with-temp* [PermissionsGroup [group]
+ PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
+ (perms/grant-permissions! group (perms/collection-read-path {:metabase.models.collection/is-root? true}))
+ (perms/grant-collection-read-permissions! group collection)
+ (is (= (sorted-results
+ (into
+ (default-results-with-collection)
+ (for [row (default-search-results)
+ :when (not= "collection" (:model row))]
+ (update row :name #(str/replace % "test" "test2")))))
+ (search-request :rasta :q "test"))))))))
+
+ (testing "Users with access to multiple collections should see results from all collections they have access to"
(with-search-items-in-collection {coll-1 :collection} "test"
(with-search-items-in-collection {coll-2 :collection} "test2"
- (tt/with-temp* [PermissionsGroup [group]
+ (mt/with-temp* [PermissionsGroup [group]
PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
(perms/grant-collection-read-permissions! group (u/get-id coll-1))
- (search-request :rasta :q "test"))))))
-
-;; Favorites are per user, so other user's favorites don't cause search results to be favorited
-(expect
- (default-results-with-collection)
- (with-search-items-in-collection {:keys [card dashboard]} "test"
- (tt/with-temp* [CardFavorite [_ {:card_id (u/get-id card)
- :owner_id (test-users/user->id :rasta)}]
- DashboardFavorite [_ {:dashboard_id (u/get-id dashboard)
- :user_id (test-users/user->id :rasta)}]]
- (search-request :crowberto :q "test"))))
-
-;; Basic search, should find 1 of each entity type and include favorites when available
-(expect
- (on-search-types #{"dashboard" "card"}
- #(assoc % :favorite true)
- (default-results-with-collection))
- (with-search-items-in-collection {:keys [card dashboard]} "test"
- (tt/with-temp* [CardFavorite [_ {:card_id (u/get-id card)
- :owner_id (test-users/user->id :crowberto)}]
- DashboardFavorite [_ {:dashboard_id (u/get-id dashboard)
- :user_id (test-users/user->id :crowberto)}]]
- (search-request :crowberto :q "test"))))
-
-;; Basic search should only return substring matches
-(expect
- (default-search-results)
- (with-search-items-in-root-collection "test"
- (with-search-items-in-root-collection "something different"
- (search-request :crowberto :q "test"))))
+ (perms/grant-collection-read-permissions! group (u/get-id coll-2))
+ (is (= (sorted-results
+ (into
+ (default-results-with-collection)
+ (map (fn [row] (update row :name #(str/replace % "test" "test2")))
+ (default-results-with-collection))))
+ (search-request :rasta :q "test")))))))
+
+ (testing "User should only see results in the collection they have access to"
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (with-search-items-in-collection {coll-1 :collection} "test"
+ (with-search-items-in-collection {coll-2 :collection} "test2"
+ (mt/with-temp* [PermissionsGroup [group]
+ PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
+ (perms/grant-collection-read-permissions! group (u/get-id coll-1))
+ (is (= (sorted-results
+ (into
+ (default-results-with-collection)
+ (map #(merge default-search-row % (table-search-results))
+ [{:name "metric test2 metric", :description "Lookin' for a blueberry", :model "metric"}
+ {:name "segment test2 segment", :description "Lookin' for a blueberry", :model "segment"}])))
+ (search-request :rasta :q "test")))))))))
+
+(deftest favorites-test
+ (testing "Favorites are per user, so other user's favorites don't cause search results to be favorited"
+ (with-search-items-in-collection {:keys [card dashboard]} "test"
+ (mt/with-temp* [CardFavorite [_ {:card_id (u/get-id card)
+ :owner_id (test-users/user->id :rasta)}]
+ DashboardFavorite [_ {:dashboard_id (u/get-id dashboard)
+ :user_id (test-users/user->id :rasta)}]]
+ (is (= (default-results-with-collection)
+ (search-request :crowberto :q "test"))))))
+
+ (testing "Basic search, should find 1 of each entity type and include favorites when available"
+ (with-search-items-in-collection {:keys [card dashboard]} "test"
+ (mt/with-temp* [CardFavorite [_ {:card_id (u/get-id card)
+ :owner_id (test-users/user->id :crowberto)}]
+ DashboardFavorite [_ {:dashboard_id (u/get-id dashboard)
+ :user_id (test-users/user->id :crowberto)}]]
+ (is (= (on-search-types #{"dashboard" "card"}
+ #(assoc % :favorite true)
+ (default-results-with-collection))
+ (search-request :crowberto :q "test")))))))
(defn- archived [m]
(assoc m :archived true))
-;; Should return unarchived results by default
-(expect
- (default-search-results)
- (with-search-items-in-root-collection "test"
- (tt/with-temp* [Card [_ (archived {:name "card test card 2"})]
- Dashboard [_ (archived {:name "dashboard test dashboard 2"})]
- Collection [_ (archived {:name "collection test collection 2"})]
- Metric [_ (archived {:name "metric test metric 2"})]
- Segment [_ (archived {:name "segment test segment 2"})]]
- (search-request :crowberto :q "test"))))
-
-;; Should return archived results when specified
-(expect
- (default-archived-results)
- (with-search-items-in-root-collection "test2"
- (tt/with-temp* [Card [_ (archived {:name "card test card"})]
- Dashboard [_ (archived {:name "dashboard test dashboard"})]
- Collection [_ (archived {:name "collection test collection"})]
- Metric [_ (archived {:name "metric test metric"})]
- Segment [_ (archived {:name "segment test segment"})]]
- (search-request :crowberto :q "test", :archived "true"))))
-
-;; Search should not return alerts
-(expect
- []
- (with-search-items-in-root-collection "test"
- (tt/with-temp* [Pulse [pulse {:alert_condition "rows"
- :alert_first_only false
- :alert_above_goal nil
- :name nil}]]
- (filter (fn [{:keys [model id]}]
- (and (= id (u/get-id pulse))
- (= "pulse" model)))
- ((test-users/user->client :crowberto) :get 200 "search")))))
+(deftest archived-results-test
+ (testing "Should return unarchived results by default"
+ (with-search-items-in-root-collection "test"
+ (mt/with-temp* [Card [_ (archived {:name "card test card 2"})]
+ Dashboard [_ (archived {:name "dashboard test dashboard 2"})]
+ Collection [_ (archived {:name "collection test collection 2"})]
+ Metric [_ (archived {:name "metric test metric 2"})]
+ Segment [_ (archived {:name "segment test segment 2"})]]
+ (is (= (default-search-results)
+ (search-request :crowberto :q "test"))))))
+
+ (testing "Should return archived results when specified"
+ (with-search-items-in-root-collection "test2"
+ (mt/with-temp* [Card [_ (archived {:name "card test card"})]
+ Dashboard [_ (archived {:name "dashboard test dashboard"})]
+ Collection [_ (archived {:name "collection test collection"})]
+ Metric [_ (archived {:name "metric test metric"})]
+ Segment [_ (archived {:name "segment test segment"})]]
+ (is (= (default-archived-results)
+ (search-request :crowberto :q "test", :archived "true")))))))
+
+(deftest alerts-test
+ (testing "Search should not return alerts"
+ (with-search-items-in-root-collection "test"
+ (mt/with-temp* [Pulse [pulse {:alert_condition "rows"
+ :alert_first_only false
+ :alert_above_goal nil
+ :name nil}]]
+ (is (= []
+ (filter (fn [{:keys [model id]}]
+ (and (= id (u/get-id pulse))
+ (= "pulse" model)))
+ ((test-users/user->client :crowberto) :get 200 "search"))))))))
-;; You should see TABLES in the search results!
(defn- default-table-search-row [table-name]
(merge
default-search-row
@@ -307,33 +288,27 @@
:model "table"
:database_id true}))
-(expect
- [(default-table-search-row "Round Table")]
- (tt/with-temp Table [table {:name "Round Table"}]
- (search-request :crowberto :q "Round Table")))
-
-(expect
- [(default-table-search-row "Kitchen Table")]
- (tt/with-temp Table [table {:name "Kitchen Table"}]
- (search-request :rasta :q "Kitchen Table")))
-
-;; But *archived* tables should not appear in search results
-(let [table-name (tu/random-name)]
- (expect
- []
- (tt/with-temp Table [table {:name table-name}]
- (search-request :crowberto :q table-name :archived true))))
-
-(let [table-name (tu/random-name)]
- (expect
- []
- (tt/with-temp Table [table {:name table-name}]
- (search-request :rasta :q table-name :archived true))))
-
-;; you should not be able to see a Table if the current user doesn't have permissions for that Table
-(expect
- []
- (tt/with-temp* [Database [{db-id :id}]
- Table [table {:db_id db-id}]]
- (perms/revoke-permissions! (group/all-users) db-id)
- (search-request :rasta :q (:name table))))
+(deftest table-test
+ (testing "You should see Tables in the search results!"
+ (mt/with-temp Table [table {:name "Round Table"}]
+ (doseq [user [:crowberto :rasta]]
+ (is (= [(default-table-search-row "Round Table")]
+ (search-request user :q "Round Table")))))
+ (testing "When searching with ?archived=true, normal Tables should not show up in the results"
+ (let [table-name (mt/random-name)]
+ (mt/with-temp Table [table {:name table-name}]
+ (doseq [user [:crowberto :rasta]]
+ (is (= []
+ (search-request user :q table-name :archived true)))))))
+ (testing "*archived* tables should not appear in search results"
+ (let [table-name (mt/random-name)]
+ (mt/with-temp Table [table {:name table-name, :active false}]
+ (doseq [user [:crowberto :rasta]]
+ (is (= []
+ (search-request user :q table-name)))))))
+ (testing "you should not be able to see a Table if the current user doesn't have permissions for that Table"
+ (mt/with-temp* [Database [{db-id :id}]
+ Table [table {:db_id db-id}]]
+ (perms/revoke-permissions! (group/all-users) db-id)
+ (is (= []
+ (search-request :rasta :q (:name table))))))))
diff --git a/test/metabase/test/data/mysql.clj b/test/metabase/test/data/mysql.clj
index 63c4f3a5ab1..87d7f53fd20 100644
--- a/test/metabase/test/data/mysql.clj
+++ b/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")
diff --git a/test/metabase/test/initialize.clj b/test/metabase/test/initialize.clj
index 8bd75ffcf84..5a27d00df7d 100644
--- a/test/metabase/test/initialize.clj
+++ b/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)))))
diff --git a/test/metabase/test/initialize/db.clj b/test/metabase/test/initialize/db.clj
index e562a55ce7d..03b6735cc28 100644
--- a/test/metabase/test/initialize/db.clj
+++ b/test/metabase/test/initialize/db.clj
@@ -1,9 +1,14 @@
(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)))))
--
GitLab