diff --git a/src/metabase/api/session.clj b/src/metabase/api/session.clj
index eeb578f5373dbf6dd45f341d76898ec30725beb7..22e80070ba80718aa45933a0aee013b716db1678 100644
--- a/src/metabase/api/session.clj
+++ b/src/metabase/api/session.clj
@@ -33,7 +33,8 @@
(db/insert! Session
:id <>
:user_id (:id user))
- (events/publish-event! :user-login {:user_id (:id user), :session_id <>, :first_login (not (boolean (:last_login user)))})))
+ (events/publish-event! :user-login
+ {:user_id (:id user), :session_id <>, :first_login (not (boolean (:last_login user)))})))
;;; ## API Endpoints
@@ -53,7 +54,8 @@
(try
(when-let [user-info (ldap/find-user username)]
(when-not (ldap/verify-password user-info password)
- ;; Since LDAP knows about the user, fail here to prevent the local strategy to be tried with a possibly outdated password
+ ;; Since LDAP knows about the user, fail here to prevent the local strategy to be tried with a possibly
+ ;; outdated password
(throw (ex-info password-fail-message
{:status-code 400
:errors {:password password-fail-snippet}})))
@@ -114,7 +116,8 @@
(throttle/check (forgot-password-throttlers :ip-address) remote-address)
(throttle/check (forgot-password-throttlers :email) email)
;; Don't leak whether the account doesn't exist, just pretend everything is ok
- (when-let [{user-id :id, google-auth? :google_auth} (db/select-one ['User :id :google_auth] :email email, :is_active true)]
+ (when-let [{user-id :id, google-auth? :google_auth} (db/select-one ['User :id :google_auth]
+ :email email, :is_active true)]
(let [reset-token (user/set-password-reset-token! user-id)
password-reset-url (str (public-settings/site-url) "/auth/reset_password/" reset-token)]
(email/send-password-reset-email! email google-auth? server-name password-reset-url)
@@ -130,7 +133,8 @@
[^String token]
(when-let [[_ user-id] (re-matches #"(^\d+)_.+$" token)]
(let [user-id (Integer/parseInt user-id)]
- (when-let [{:keys [reset_token reset_triggered], :as user} (db/select-one [User :id :last_login :reset_triggered :reset_token]
+ (when-let [{:keys [reset_token reset_triggered], :as user} (db/select-one [User :id :last_login :reset_triggered
+ :reset_token]
:id user-id, :is_active true)]
;; Make sure the plaintext token matches up with the hashed one for this user
(when (u/ignore-exceptions
@@ -206,8 +210,9 @@
(when-not (autocreate-user-allowed-for-email? email)
;; Use some wacky status code (428 - Precondition Required) so we will know when to so the error screen specific
;; to this situation
- (throw (ex-info (tru "You''ll need an administrator to create a Metabase account before you can use Google to log in.")
- {:status-code 428}))))
+ (throw
+ (ex-info (tru "You''ll need an administrator to create a Metabase account before you can use Google to log in.")
+ {:status-code 428}))))
(s/defn ^:private google-auth-create-new-user!
[{:keys [email] :as new-user} :- user/NewUser]
diff --git a/src/metabase/db.clj b/src/metabase/db.clj
index 78110be4483e775282c74a985b801aba254ebaeb..0d6349c45b56040849de8a017e8906952d85c2c7 100644
--- a/src/metabase/db.clj
+++ b/src/metabase/db.clj
@@ -1,5 +1,5 @@
(ns metabase.db
- "Database definition and helper functions for interacting with the database."
+ "Application database definition, and setup logic, and helper functions for interacting with it."
(:require [clojure
[string :as s]
[walk :as walk]]
@@ -11,6 +11,7 @@
[config :as config]
[util :as u]]
[metabase.db.spec :as dbspec]
+ [puppetlabs.i18n.core :refer [trs]]
[ring.util.codec :as codec]
[toucan.db :as db])
(:import com.mchange.v2.c3p0.ComboPooledDataSource
@@ -45,8 +46,9 @@
[(System/getProperty "user.dir") "/" db-file-name options]))))))
(defn- parse-connection-string
- "Parse a DB connection URI like `postgres://cam@localhost.com:5432/cams_cool_db?ssl=true&sslfactory=org.postgresql.ssl.NonValidatingFactory`
- and return a broken-out map."
+ "Parse a DB connection URI like
+ `postgres://cam@localhost.com:5432/cams_cool_db?ssl=true&sslfactory=org.postgresql.ssl.NonValidatingFactory` and
+ return a broken-out map."
[uri]
(when-let [[_ protocol user pass host port db query] (re-matches #"^([^:/@]+)://(?:([^:/@]+)(?::([^:@]+))?@)?([^:@]+)(?::(\d+))?/([^/?]+)(?:\?(.*))?$" uri)]
(merge {:type (case (keyword protocol)
@@ -73,8 +75,8 @@
(config/config-kw :mb-db-type)))
(def db-connection-details
- "Connection details that can be used when pretending the Metabase DB is itself a `Database`
- (e.g., to use the Generic SQL driver functions on the Metabase DB itself)."
+ "Connection details that can be used when pretending the Metabase DB is itself a `Database` (e.g., to use the Generic
+ SQL driver functions on the Metabase DB itself)."
(delay (or @connection-string-details
(case (db-type)
:h2 {:type :h2 ; TODO - we probably don't need to specifc `:type` here since we can just call (db-type)
@@ -112,19 +114,19 @@
(def ^:private ^:const ^String changelog-file "liquibase.yaml")
(defn- migrations-sql
- "Return a string of SQL containing the DDL statements needed to perform unrun LIQUIBASE migrations."
+ "Return a string of SQL containing the DDL statements needed to perform unrun `liquibase` migrations."
^String [^Liquibase liquibase]
(let [writer (StringWriter.)]
(.update liquibase "" writer)
(.toString writer)))
(defn- migrations-lines
- "Return a sequnce of DDL statements that should be used to perform migrations for LIQUIBASE.
+ "Return a sequnce of DDL statements that should be used to perform migrations for `liquibase`.
- MySQL gets snippy if we try to run the entire DB migration as one single string; it seems to only like it if we run
- one statement at a time; Liquibase puts each DDL statement on its own line automatically so just split by lines and
- filter out blank / comment lines. Even though this is not neccesary for H2 or Postgres go ahead and do it anyway
- because it keeps the code simple and doesn't make a significant performance difference."
+ MySQL gets snippy if we try to run the entire DB migration as one single string; it seems to only like it if we run
+ one statement at a time; Liquibase puts each DDL statement on its own line automatically so just split by lines and
+ filter out blank / comment lines. Even though this is not neccesary for H2 or Postgres go ahead and do it anyway
+ because it keeps the code simple and doesn't make a significant performance difference."
[^Liquibase liquibase]
(for [line (s/split-lines (migrations-sql liquibase))
:when (not (or (s/blank? line)
@@ -132,57 +134,61 @@
line))
(defn- has-unrun-migrations?
- "Does LIQUIBASE have migration change sets that haven't been run yet?
+ "Does `liquibase` have migration change sets that haven't been run yet?
- It's a good idea to Check to make sure there's actually something to do before running `(migrate :up)` because
- `migrations-sql` will always contain SQL to create and release migration locks, which is both slightly dangerous
- and a waste of time when we won't be using them."
+ It's a good idea to Check to make sure there's actually something to do before running `(migrate :up)` because
+ `migrations-sql` will always contain SQL to create and release migration locks, which is both slightly dangerous and
+ a waste of time when we won't be using them."
^Boolean [^Liquibase liquibase]
(boolean (seq (.listUnrunChangeSets liquibase nil))))
(defn- has-migration-lock?
- "Is a migration lock in place for LIQUIBASE?"
+ "Is a migration lock in place for `liquibase`?"
^Boolean [^Liquibase liquibase]
(boolean (seq (.listLocks liquibase))))
(defn- wait-for-migration-lock-to-be-cleared
- "Check and make sure the database isn't locked. If it is, sleep for 2 seconds and then retry several times.
- There's a chance the lock will end up clearing up so we can run migrations normally."
+ "Check and make sure the database isn't locked. If it is, sleep for 2 seconds and then retry several times. There's a
+ chance the lock will end up clearing up so we can run migrations normally."
[^Liquibase liquibase]
(u/auto-retry 5
(when (has-migration-lock? liquibase)
(Thread/sleep 2000)
- (throw (Exception. (str "Database has migration lock; cannot run migrations. You can force-release these locks "
- "by running `java -jar metabase.jar migrate release-locks`."))))))
+ (throw
+ (Exception.
+ (str
+ (trs "Database has migration lock; cannot run migrations.")
+ " "
+ (trs "You can force-release these locks by running `java -jar metabase.jar migrate release-locks`.")))))))
(defn- migrate-up-if-needed!
- "Run any unrun LIQUIBASE migrations, if needed.
+ "Run any unrun `liquibase` migrations, if needed.
- This creates SQL for the migrations to be performed, then executes each DDL statement.
- Running `.update` directly doesn't seem to work as we'd expect; it ends up commiting the changes made and they
- can't be rolled back at the end of the transaction block. Converting the migration to SQL string and running that
- via `jdbc/execute!` seems to do the trick."
+ This creates SQL for the migrations to be performed, then executes each DDL statement. Running `.update` directly
+ doesn't seem to work as we'd expect; it ends up commiting the changes made and they can't be rolled back at the end
+ of the transaction block. Converting the migration to SQL string and running that via `jdbc/execute!` seems to do
+ the trick."
[conn, ^Liquibase liquibase]
- (log/info "Checking if Database has unrun migrations...")
+ (log/info (trs "Checking if Database has unrun migrations..."))
(when (has-unrun-migrations? liquibase)
- (log/info "Database has unrun migrations. Waiting for migration lock to be cleared...")
+ (log/info (trs "Database has unrun migrations. Waiting for migration lock to be cleared..."))
(wait-for-migration-lock-to-be-cleared liquibase)
- (log/info "Migration lock is cleared. Running migrations...")
+ (log/info (trs "Migration lock is cleared. Running migrations..."))
(doseq [line (migrations-lines liquibase)]
(jdbc/execute! conn [line]))))
(defn- force-migrate-up-if-needed!
"Force migrating up. This does two things differently from `migrate-up-if-needed!`:
- 1. This doesn't check to make sure the DB locks are cleared
- 2. Any DDL statements that fail are ignored
+ 1. This doesn't check to make sure the DB locks are cleared
+ 2. Any DDL statements that fail are ignored
- It can be used to fix situations where the database got into a weird state, as was common before the fixes made in
- #3295.
+ It can be used to fix situations where the database got into a weird state, as was common before the fixes made in
+ #3295.
- Each DDL statement is ran inside a nested transaction; that way if the nested transaction fails we can roll it back
- without rolling back the entirety of changes that were made. (If a single statement in a transaction fails you
- can't do anything futher until you clear the error state by doing something like calling `.rollback`.)"
+ Each DDL statement is ran inside a nested transaction; that way if the nested transaction fails we can roll it back
+ without rolling back the entirety of changes that were made. (If a single statement in a transaction fails you can't
+ do anything futher until you clear the error state by doing something like calling `.rollback`.)"
[conn, ^Liquibase liquibase]
(.clearCheckSums liquibase)
(when (has-unrun-migrations? liquibase)
@@ -197,7 +203,7 @@
(def ^{:arglists '([])} ^DatabaseFactory database-factory
"Return an instance of the Liquibase `DatabaseFactory`. This is done on a background thread at launch because
- otherwise it adds 2 seconds to startup time."
+ otherwise it adds 2 seconds to startup time."
(partial deref (future (DatabaseFactory/getInstance))))
(defn- conn->liquibase
@@ -222,7 +228,8 @@
"DATABASECHANGELOG"
"databasechangelog")
fresh-install? (jdbc/with-db-metadata [meta (jdbc-details)] ;; don't migrate on fresh install
- (empty? (jdbc/metadata-query (.getTables meta nil nil liquibases-table-name (into-array String ["TABLE"])))))
+ (empty? (jdbc/metadata-query
+ (.getTables meta nil nil liquibases-table-name (into-array String ["TABLE"])))))
query (format "UPDATE %s SET FILENAME = ?" liquibases-table-name)]
(when-not fresh-install?
(jdbc/execute! conn [query "migrations/000_migrations.yaml"]))))
@@ -252,11 +259,11 @@
;; Disable auto-commit. This should already be off but set it just to be safe
(.setAutoCommit (jdbc/get-connection conn) false)
;; Set up liquibase and let it do its thing
- (log/info "Setting up Liquibase...")
+ (log/info (trs "Setting up Liquibase..."))
(try
(let [liquibase (conn->liquibase conn)]
(consolidate-liquibase-changesets conn)
- (log/info "Liquibase is ready.")
+ (log/info (trs "Liquibase is ready."))
(case direction
:up (migrate-up-if-needed! conn liquibase)
:force (force-migrate-up-if-needed! conn liquibase)
@@ -279,7 +286,7 @@
;;; +----------------------------------------------------------------------------------------------------------------+
(defn connection-pool
- "Create a C3P0 connection pool for the given database SPEC."
+ "Create a C3P0 connection pool for the given database `spec`."
[{:keys [subprotocol subname classname minimum-pool-size idle-connection-test-period excess-timeout]
:or {minimum-pool-size 3
idle-connection-test-period 0
@@ -348,12 +355,12 @@
(verify-db-connection (:type db-details) db-details))
([engine details]
{:pre [(keyword? engine) (map? details)]}
- (log/info (u/format-color 'cyan "Verifying %s Database Connection ..." (name engine)))
+ (log/info (u/format-color 'cyan (trs "Verifying {0} Database Connection ..." (name engine))))
(assert (binding [*allow-potentailly-unsafe-connections* true]
(require 'metabase.driver)
((resolve 'metabase.driver/can-connect-with-details?) engine details))
(format "Unable to connect to Metabase %s DB." (name engine)))
- (log/info "Verify Database Connection ... " (u/emoji "✅"))))
+ (log/info (trs "Verify Database Connection ... ") (u/emoji "✅"))))
(def ^:dynamic ^Boolean *disable-data-migrations*
@@ -379,11 +386,11 @@
(defn- run-schema-migrations!
"Run through our DB migration process and make sure DB is fully prepared"
[auto-migrate? db-details]
- (log/info "Running Database Migrations...")
+ (log/info (trs "Running Database Migrations..."))
(if auto-migrate?
(migrate! db-details :up)
(print-migrations-and-quit! db-details))
- (log/info "Database Migrations Current ... " (u/emoji "✅")))
+ (log/info (trs "Database Migrations Current ... ") (u/emoji "✅")))
(defn- run-data-migrations!
"Do any custom code-based migrations now that the db structure is up to date."
diff --git a/src/metabase/events.clj b/src/metabase/events.clj
index 28e245f5b2f51a7ec29fa2fa4ef552f88184e2e1..11613ae0531e4acf49ef59b94100d8843c0b6219 100644
--- a/src/metabase/events.clj
+++ b/src/metabase/events.clj
@@ -55,6 +55,7 @@
(defn publish-event!
"Publish an item into the events stream. Returns the published item to allow for chaining."
+ {:style/indent 1}
[topic event-item]
{:pre [(keyword topic)]}
(async/go (async/>! events-channel {:topic (keyword topic), :item event-item}))
diff --git a/src/metabase/metabot.clj b/src/metabase/metabot.clj
index 6b034ba180fadd26adee745cc7062b8800d8baf8..22ece0d117f2d1bbfafbdf55f3c6eb1ecbefdce1 100644
--- a/src/metabase/metabot.clj
+++ b/src/metabase/metabot.clj
@@ -7,6 +7,7 @@
[string :as str]]
[clojure.java.io :as io]
[clojure.tools.logging :as log]
+ [honeysql.core :as hsql]
[manifold
[deferred :as d]
[stream :as s]]
@@ -21,8 +22,10 @@
[permissions :refer [Permissions]]
[permissions-group :as perms-group]
[setting :as setting :refer [defsetting]]]
- [metabase.util.urls :as urls]
- [puppetlabs.i18n.core :refer [tru trs]]
+ [metabase.util
+ [date :as du]
+ [urls :as urls]]
+ [puppetlabs.i18n.core :refer [trs tru]]
[throttle.core :as throttle]
[toucan.db :as db]))
@@ -32,7 +35,106 @@
:default false)
-;;; ------------------------------------------------------------ Perms Checking ------------------------------------------------------------
+;;; ------------------------------------- Deciding which instance is the MetaBot -------------------------------------
+
+;; Close your eyes, and imagine a scenario: someone is running multiple Metabase instances in a horizontal cluster.
+;; Good for them, but how do we make sure one, and only one, of those instances, replies to incoming MetaBot commands?
+;; It would certainly be too much if someone ran, say, 4 instances, and typing `metabot kanye` into Slack gave them 4
+;; Kanye West quotes, wouldn't it?
+;;
+;; Luckily, we have an "elegant" solution: we'll use the Settings framework to keep track of which instance is
+;; currently serving as the MetaBot. We'll have that instance periodically check in; if it doesn't check in for some
+;; timeout interval, we'll consider the job of MetaBot up for grabs. Each instance will periodically check if the
+;; MetaBot job is open, and, if so, whoever discovers it first will take it.
+
+
+;; How do we uniquiely identify each instance?
+;;
+;; `local-process-uuid` is randomly-generated upon launch and used to identify this specific Metabase instance during
+;; this specifc run. Restarting the server will change this UUID, and each server in a hortizontal cluster will have
+;; its own ID, making this different from the `site-uuid` Setting. The local process UUID is used to differentiate
+;; different horizontally clustered MB instances so we can determine which of them will handle MetaBot duties.
+;;
+;; TODO - if we ever want to use this elsewhere, we need to move it to `metabase.config` or somewhere else central
+;; like that.
+(defonce ^:private local-process-uuid
+ (str (java.util.UUID/randomUUID)))
+
+(defsetting ^:private metabot-instance-uuid
+ "UUID of the active MetaBot instance (the Metabase process currently handling MetaBot duties.)"
+ ;; This should be cached because we'll be checking it fairly often, basically every 2 seconds as part of the
+ ;; websocket monitor thread to see whether we're MetaBot (the thread won't open the WebSocket unless that instance
+ ;; is handling MetaBot duties)
+ :internal? true)
+
+(defsetting ^:private metabot-instance-last-checkin
+ "Timestamp of the last time the active MetaBot instance checked in."
+ :internal? true
+ ;; caching is disabled for this, since it is intended to be updated frequently (once a minute or so) If we use the
+ ;; cache, it will trigger cache invalidation for all the other instances (wasteful), and possibly at any rate be
+ ;; incorrect (for example, if another instance checked in a minute ago, our local cache might not get updated right
+ ;; away, causing us to falsely assume the MetaBot role is up for grabs.)
+ :cache? false
+ :type :timestamp)
+
+(defn- current-timestamp-from-db
+ "Fetch the current timestamp from the DB. Why do this from the DB? It's not safe to assume multiple instances have
+ clocks exactly in sync; but since each instance is using the same application DB, we can use it as a cannonical
+ source of truth."
+ ^java.sql.Timestamp []
+ (-> (db/query {:select [[(hsql/raw "current_timestamp") :current_timestamp]]})
+ first
+ :current_timestamp))
+
+(defn- update-last-checkin!
+ "Update the last checkin timestamp recorded in the DB."
+ []
+ (metabot-instance-last-checkin (current-timestamp-from-db)))
+
+(defn- seconds-since-last-checkin
+ "Return the number of seconds since the active MetaBot instance last checked in (updated the
+ `metabot-instance-last-checkin` Setting). If a MetaBot instance has *never* checked in, this returns `nil`. (Since
+ `last-checkin` is one of the few Settings that isn't cached, this always requires a DB call.)"
+ []
+ (when-let [last-checkin (metabot-instance-last-checkin)]
+ (u/prog1 (-> (- (.getTime (current-timestamp-from-db))
+ (.getTime last-checkin))
+ (/ 1000))
+ (log/debug (u/format-color 'magenta (trs "Last MetaBot checkin was {0} ago." (du/format-seconds <>)))))))
+
+(def ^:private ^Integer recent-checkin-timeout-interval-seconds
+ "Number of seconds since the last MetaBot checkin that we will consider the MetaBot job to be 'up for grabs',
+ currently 3 minutes. (i.e. if the current MetaBot job holder doesn't check in for more than 3 minutes, it's up for
+ grabs.)"
+ (int (* 60 3)))
+
+(defn- last-checkin-was-not-recent?
+ "`true` if the last checkin of the active MetaBot instance was more than 3 minutes ago, or if there has never been a
+ checkin. (This requires DB calls, so it should not be called too often -- once a minute [at the time of this
+ writing] should be sufficient.)"
+ []
+ (if-let [seconds-since-last-checkin (seconds-since-last-checkin)]
+ (> seconds-since-last-checkin
+ recent-checkin-timeout-interval-seconds)
+ true))
+
+(defn- am-i-the-metabot?
+ "Does this instance currently have the MetaBot job? (Does not require any DB calls, so may safely be called
+ often (i.e. in the websocket monitor thread loop.)"
+ []
+ (= (metabot-instance-uuid)
+ local-process-uuid))
+
+(defn- become-metabot!
+ "Direct this instance to assume the duties of acting as MetaBot, and update the Settings we use to track assignment
+ accordingly."
+ []
+ (log/info (u/format-color 'green (trs "This instance will now handle MetaBot duties.")))
+ (metabot-instance-uuid local-process-uuid)
+ (update-last-checkin!))
+
+
+;;; ------------------------------------------------- Perms Checking -------------------------------------------------
(defn- metabot-permissions
"Return the set of permissions granted to the MetaBot."
@@ -50,7 +152,7 @@
`(do-with-metabot-permissions (fn [] ~@body)))
-;;; # ------------------------------------------------------------ Metabot Command Handlers ------------------------------------------------------------
+;;; -------------------------------------------- Metabot Command Handlers --------------------------------------------
(def ^:private ^:dynamic *channel-id* nil)
@@ -110,15 +212,18 @@
(defn- card-with-name [card-name]
(first (u/prog1 (db/select [Card :id :name], :%lower.name [:like (str \% (str/lower-case card-name) \%)])
(when (> (count <>) 1)
- (throw (Exception. (str (tru "Could you be a little more specific? I found these cards with names that matched:\n{0}"
- (format-cards <>)))))))))
+ (throw (Exception.
+ (str (tru "Could you be a little more specific? I found these cards with names that matched:\n{0}"
+ (format-cards <>)))))))))
(defn- id-or-name->card [card-id-or-name]
(cond
(integer? card-id-or-name) (db/select-one [Card :id :name], :id card-id-or-name)
(or (string? card-id-or-name)
(symbol? card-id-or-name)) (card-with-name card-id-or-name)
- :else (throw (Exception. (str (tru "I don''t know what Card `{0}` is. Give me a Card ID or name." card-id-or-name))))))
+ :else (throw (Exception.
+ (str (tru "I don''t know what Card `{0}` is. Give me a Card ID or name."
+ card-id-or-name))))))
(defn ^:metabot show
@@ -130,13 +235,16 @@
(do
(with-metabot-permissions
(read-check Card card-id))
- (do-async (let [attachments (pulse/create-and-upload-slack-attachments! (pulse/create-slack-attachment-data [(pulse/execute-card card-id, :context :metabot)]))]
+ (do-async (let [attachments (pulse/create-and-upload-slack-attachments!
+ (pulse/create-slack-attachment-data
+ [(pulse/execute-card card-id, :context :metabot)]))]
(slack/post-chat-message! *channel-id*
nil
attachments)))
(tru "Ok, just a second..."))
(throw (Exception. (str (tru "Not Found"))))))
- ;; If the card name comes without spaces, e.g. (show 'my 'wacky 'card) turn it into a string an recur: (show "my wacky card")
+ ;; If the card name comes without spaces, e.g. (show 'my 'wacky 'card) turn it into a string an recur: (show "my
+ ;; wacky card")
([word & more]
(show (str/join " " (cons word more)))))
@@ -171,7 +279,7 @@
(str ":kanye:\n> " (rand-nth @kanye-quotes)))
-;;; # ------------------------------------------------------------ Metabot Command Dispatch ------------------------------------------------------------
+;;; -------------------------------------------- Metabot Command Dispatch --------------------------------------------
(def ^:private apply-metabot-fn
(dispatch-fn "understand" :metabot))
@@ -189,7 +297,7 @@
(apply apply-metabot-fn tokens)))))
-;;; # ------------------------------------------------------------ Metabot Input Handling ------------------------------------------------------------
+;;; --------------------------------------------- Metabot Input Handling ---------------------------------------------
(defn- message->command-str
"Get the command portion of a message *event* directed at Metabot.
@@ -241,7 +349,7 @@
nil)))))
-;;; # ------------------------------------------------------------ Websocket Connection Stuff ------------------------------------------------------------
+;;; ------------------------------------------- Websocket Connection Stuff -------------------------------------------
(defn- connect-websocket! []
(when-let [websocket-url (slack/websocket-url)]
@@ -264,9 +372,10 @@
;; and if it is no longer equal to theirs they should die
(defonce ^:private websocket-monitor-thread-id (atom nil))
-;; we'll use a THROTTLER to implement exponential backoff for recconenction attempts, since THROTTLERS are designed with for this sort of thing
-;; e.g. after the first failed connection we'll wait 2 seconds, then each that amount increases by the `:delay-exponent` of 1.3
-;; so our reconnection schedule will look something like:
+;; we'll use a THROTTLER to implement exponential backoff for recconenction attempts, since THROTTLERS are designed
+;; with for this sort of thing e.g. after the first failed connection we'll wait 2 seconds, then each that amount
+;; increases by the `:delay-exponent` of 1.3. So our reconnection schedule will look something like:
+;;
;; number of consecutive failed attempts | seconds before next try (rounded up to nearest multiple of 2 seconds)
;; --------------------------------------+----------------------------------------------------------------------
;; 0 | 2
@@ -276,47 +385,91 @@
;; 4 | 8
;; 5 | 14
;; 6 | 30
-;; we'll throttle this based on values of the `slack-token` setting; that way if someone changes its value they won't have to wait
-;; whatever the exponential delay is before the connection is retried
+;;
+;; we'll throttle this based on values of the `slack-token` setting; that way if someone changes its value they won't
+;; have to wait whatever the exponential delay is before the connection is retried
(def ^:private reconnection-attempt-throttler
(throttle/make-throttler nil :attempts-threshold 1, :initial-delay-ms 2000, :delay-exponent 1.3))
(defn- should-attempt-to-reconnect? ^Boolean []
- (boolean (u/ignore-exceptions
- (throttle/check reconnection-attempt-throttler (slack/slack-token))
- true)))
+ (boolean
+ (u/ignore-exceptions
+ (throttle/check reconnection-attempt-throttler (slack/slack-token))
+ true)))
+
+(defn- reopen-websocket-connection-if-needed!
+ "Check to see if websocket connection is [still] open, [re-]open it if not."
+ []
+ ;; Only open the Websocket connection if this instance is the MetaBot
+ (when (am-i-the-metabot?)
+ (when (= (.getId (Thread/currentThread)) @websocket-monitor-thread-id)
+ (try
+ (when (or (not @websocket)
+ (s/closed? @websocket))
+ (log/debug (trs "MetaBot WebSocket is closed. Reconnecting now."))
+ (connect-websocket!))
+ (catch Throwable e
+ (log/error (trs "Error connecting websocket:") (.getMessage e)))))))
(defn- start-websocket-monitor! []
(future
(reset! websocket-monitor-thread-id (.getId (Thread/currentThread)))
- ;; Every 2 seconds check to see if websocket connection is [still] open, [re-]open it if not
(loop []
+ ;; Every 2 seconds...
(while (not (should-attempt-to-reconnect?))
(Thread/sleep 2000))
- (when (= (.getId (Thread/currentThread)) @websocket-monitor-thread-id)
- (try
- (when (or (not @websocket)
- (s/closed? @websocket))
- (log/debug (trs "MetaBot WebSocket is closed. Reconnecting now."))
- (connect-websocket!))
- (catch Throwable e
- (log/error (trs "Error connecting websocket:") (.getMessage e))))
- (recur)))))
+ (reopen-websocket-connection-if-needed!)
+ (recur))))
+
+
+(defn- check-and-update-instance-status!
+ "Check whether the current instance is serving as the MetaBot; if so, update the last checkin timestamp; if not, check
+ whether we should become the MetaBot (and do so if we should)."
+ []
+ (cond
+ ;; if we're already the MetaBot instance, update the last checkin timestamp
+ (am-i-the-metabot?)
+ (do
+ (log/debug (trs "This instance is performing MetaBot duties."))
+ (update-last-checkin!))
+ ;; otherwise if the last checkin was too long ago, it's time for us to assume the mantle of MetaBot
+ (last-checkin-was-not-recent?)
+ (become-metabot!)
+ ;; otherwise someone else is the MetaBot and we're done here! woo
+ :else
+ (log/debug (u/format-color 'blue (trs "Another instance is already handling MetaBot duties.")))))
+
+(defn- start-instance-monitor! []
+ (future
+ (loop []
+ (check-and-update-instance-status!)
+ (Thread/sleep (* 60 1000))
+ (recur))))
+
+(defn- seconds-to-wait-before-starting
+ "Return a random number of seconds to wait before starting MetaBot processess, between 0 and 59. This is done to
+ introduce a bit of jitter that should prevent a rush of multiple instances all racing to become the MetaBot at the
+ same time."
+ []
+ (mod (.nextInt (java.security.SecureRandom.)) 60))
(defn start-metabot!
"Start the MetaBot! :robot_face:
- This will spin up a background thread that opens and maintains a Slack WebSocket connection."
+ This will spin up a background thread that opens and maintains a Slack WebSocket connection."
[]
- (when (and (slack/slack-token)
- (metabot-enabled))
- (log/info "Starting MetaBot WebSocket monitor thread...")
- (start-websocket-monitor!)))
+ (future
+ (Thread/sleep (* 1000 (seconds-to-wait-before-starting)))
+ (when (and (slack/slack-token)
+ (metabot-enabled))
+ (log/info (trs "Starting MetaBot threads..."))
+ (start-websocket-monitor!)
+ (start-instance-monitor!))))
(defn stop-metabot!
"Stop the MetaBot! :robot_face:
- This will stop the background thread that responsible for the Slack WebSocket connection."
+ This will stop the background thread that responsible for the Slack WebSocket connection."
[]
(log/info (trs "Stopping MetaBot... 🤖"))
(reset! websocket-monitor-thread-id nil)
diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj
index 29a595b9e26a68ac848ae7b55fad76045e5fbce5..2c28b92fc1a773b6255b1eb5dd4334c18d46bc49 100644
--- a/src/metabase/models/setting.clj
+++ b/src/metabase/models/setting.clj
@@ -42,7 +42,9 @@
[db :as mdb]
[events :as events]
[util :as u]]
- [metabase.util.honeysql-extensions :as hx]
+ [metabase.util
+ [date :as du]
+ [honeysql-extensions :as hx]]
[puppetlabs.i18n.core :refer [trs tru]]
[schema.core :as s]
[toucan
@@ -60,7 +62,16 @@
(def ^:private Type
- (s/enum :string :boolean :json :integer :double))
+ (s/enum :string :boolean :json :integer :double :timestamp))
+
+(def ^:private default-tag-for-type
+ "Type tag that will be included in the Setting's metadata, so that the getter function will not cause reflection
+ warnings."
+ {:string String
+ :boolean Boolean
+ :integer Long
+ :double Double
+ :timestamp java.sql.Timestamp})
(def ^:private SettingDefinition
{:name s/Keyword
@@ -69,7 +80,9 @@
:type Type ; all values are stored in DB as Strings,
:getter clojure.lang.IFn ; different getters/setters take care of parsing/unparsing
:setter clojure.lang.IFn
- :internal? s/Bool}) ; should the API never return this setting? (default: false)
+ :tag (s/maybe Class) ; type annotation, e.g. ^String, to be applied. Defaults to tag based on :type
+ :internal? s/Bool ; should the API never return this setting? (default: false)
+ :cache? s/Bool}) ; should the getter always fetch this value "fresh" from the DB? (default: false)
(defonce ^:private registered-settings
@@ -157,10 +170,13 @@
(when-let [last-known-update (core/get @cache settings-last-updated-key)]
;; compare it to the value in the DB. This is done be seeing whether a row exists
;; WHERE value > <local-value>
- (db/select-one Setting
- {:where [:and
- [:= :key settings-last-updated-key]
- [:> :value last-known-update]]})))))
+ (u/prog1 (db/select-one Setting
+ {:where [:and
+ [:= :key settings-last-updated-key]
+ [:> :value last-known-update]]})
+ (when <>
+ (log/info (u/format-color 'red
+ (trs "Settings have been changed on another instance, and will be reloaded here.")))))))))
(def ^:private cache-update-check-interval-ms
"How often we should check whether the Settings cache is out of date (which requires a DB call)?"
@@ -227,11 +243,16 @@
(when (seq v)
v)))
+(def ^:private ^:dynamic *disable-cache* false)
+
(defn- db-value
"Get the value, if any, of `setting-or-name` from the DB (using / restoring the cache as needed)."
^String [setting-or-name]
- (restore-cache-if-needed!)
- (clojure.core/get @cache (setting-name setting-or-name)))
+ (if *disable-cache*
+ (db/select-one-field :value Setting :key (setting-name setting-or-name))
+ (do
+ (restore-cache-if-needed!)
+ (clojure.core/get @cache (setting-name setting-or-name)))))
(defn get-string
@@ -285,18 +306,26 @@
[setting-or-name]
(json/parse-string (get-string setting-or-name) keyword))
+(defn get-timestamp
+ "Get the string value of `setting-or-name` and parse it as an ISO-8601-formatted string, returning a Timestamp."
+ [setting-or-name]
+ (du/->Timestamp (get-string setting-or-name) :no-timezone))
+
(def ^:private default-getter-for-type
- {:string get-string
- :boolean get-boolean
- :integer get-integer
- :json get-json
- :double get-double})
+ {:string get-string
+ :boolean get-boolean
+ :integer get-integer
+ :json get-json
+ :timestamp get-timestamp
+ :double get-double})
(defn get
"Fetch the value of `setting-or-name`. What this means depends on the Setting's `:getter`; by default, this looks for
first for a corresponding env var, then checks the cache, then returns the default value of the Setting, if any."
[setting-or-name]
- ((:getter (resolve-setting setting-or-name))))
+ (let [{:keys [cache? getter]} (resolve-setting setting-or-name)]
+ (binding [*disable-cache* (not cache?)]
+ (getter))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -353,7 +382,10 @@
(swap! cache assoc setting-name new-value)
(swap! cache dissoc setting-name))
;; Record the fact that a Setting has been updated so eventaully other instances (if applicable) find out about it
- (update-settings-last-updated!)
+ ;; (For Settings that don't use the Cache, don't update the `last-updated` value, because it will cause other
+ ;; instances to do needless reloading of the cache from the DB)
+ (when-not *disable-cache*
+ (update-settings-last-updated!))
;; Now return the `new-value`.
new-value))
@@ -389,15 +421,20 @@
(defn set-json!
"Serialize `new-value` for `setting-or-name` as a JSON string and save it."
[setting-or-name new-value]
- (set-string! setting-or-name (when new-value
- (json/generate-string new-value))))
+ (set-string! setting-or-name (some-> new-value json/generate-string)))
+
+(defn set-timestamp!
+ "Serialize `new-value` for `setting-or-name` as a ISO 8601-encoded timestamp strign and save it."
+ [setting-or-name new-value]
+ (set-string! setting-or-name (some-> new-value du/date->iso-8601)))
(def ^:private default-setter-for-type
- {:string set-string!
- :boolean set-boolean!
- :integer set-integer!
- :json set-json!
- :double set-double!})
+ {:string set-string!
+ :boolean set-boolean!
+ :integer set-integer!
+ :json set-json!
+ :timestamp set-timestamp!
+ :double set-double!})
(defn set!
"Set the value of `setting-or-name`. What this means depends on the Setting's `:setter`; by default, this just updates
@@ -409,7 +446,9 @@
(mandrill-api-key \"xyz123\")"
[setting-or-name new-value]
- ((:setter (resolve-setting setting-or-name)) new-value))
+ (let [{:keys [setter cache?]} (resolve-setting setting-or-name)]
+ (binding [*disable-cache* (not cache?)]
+ (setter new-value))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -427,7 +466,9 @@
:default default
:getter (partial (default-getter-for-type setting-type) setting-name)
:setter (partial (default-setter-for-type setting-type) setting-name)
- :internal? false}
+ :tag (default-tag-for-type setting-type)
+ :internal? false
+ :cache? true}
(dissoc setting :name :type :default)))
(s/validate SettingDefinition <>)
(swap! registered-settings assoc setting-name <>)))
@@ -440,10 +481,11 @@
(defn metadata-for-setting-fn
"Create metadata for the function automatically generated by `defsetting`."
- [{:keys [default description], setting-type :type, :as setting}]
+ [{:keys [default description tag], setting-type :type, :as setting}]
{:arglists '([] [new-value])
;; indentation below is intentional to make it clearer what shape the generated documentation is going to take.
;; Turn on auto-complete-mode in Emacs and see for yourself!
+ :tag tag
:doc (str/join "\n" [ description
""
(format "`%s` is a %s Setting. You can get its value by calling:" (setting-name setting) (name setting-type))
@@ -501,7 +543,9 @@
* `:setter` - A custom setter fn, which takes a single argument. Overrides the default implementation. (This
can in turn call functions in this namespace like `set-string!` or `set-boolean!` to invoke the
default setter behavior. Keep in mind that the custom setter may be passed `nil`, which should
- clear the values of the Setting.)"
+ clear the values of the Setting.)
+ * `:cache?` - Should this Setting be cached? (default `true`)? Be careful when disabling this, because it could
+ have a very negative performance impact."
{:style/indent 1}
[setting-symb description & {:as options}]
{:pre [(symbol? setting-symb)]}
diff --git a/src/metabase/util/date.clj b/src/metabase/util/date.clj
index 2ad043a84d29ea060b1314144319add26eca8644..03a07b3fdb73e443a668b66843bd2497a5d41be2 100644
--- a/src/metabase/util/date.clj
+++ b/src/metabase/util/date.clj
@@ -24,7 +24,7 @@
:tag TimeZone}
*data-timezone*)
-(defprotocol ITimeZoneCoercible
+(defprotocol ^:private ITimeZoneCoercible
"Coerce object to `java.util.TimeZone`"
(coerce-to-timezone ^TimeZone [this]
"Coerce `this` to `java.util.TimeZone`"))
@@ -85,7 +85,7 @@
[db & body]
`(call-with-effective-timezone ~db (fn [] ~@body)))
-(defprotocol ITimestampCoercible
+(defprotocol ^:private ITimestampCoercible
"Coerce object to a `java.sql.Timestamp`."
(coerce-to-timestamp ^java.sql.Timestamp [this] [this timezone-coercible]
"Coerce this object to a `java.sql.Timestamp`. Strings are parsed as ISO-8601."))
@@ -110,7 +110,12 @@
(defn ^Timestamp ->Timestamp
"Converts `coercible-to-ts` to a `java.util.Timestamp`. Requires a `coercible-to-tz` if converting a string. Leans
- on clj-time to ensure correct conversions between the various types"
+ on clj-time to ensure correct conversions between the various types
+
+ NOTE: This function requires you to pass in a timezone or bind `*report-timezone*`, probably to make sure you're not
+ doing something dumb by forgetting it. For cases where you'd just like to parse an ISO-8601-encoded String in peace
+ without specifying a timezone, pass in `:no-timezone` as the second param to explicitly have things parsed without
+ one. (Keep in mind that if your string does not specify a timezone, it will be parsed as UTC by default.)"
([coercible-to-ts]
{:pre [(or (not (string? coercible-to-ts))
(and (string? coercible-to-ts) (bound? #'*report-timezone*)))]}
@@ -119,10 +124,11 @@
{:pre [(or (not (string? coercible-to-ts))
(and (string? coercible-to-ts) timezone))]}
(if (string? coercible-to-ts)
- (coerce-to-timestamp (str->date-time coercible-to-ts (coerce-to-timezone timezone)))
+ (coerce-to-timestamp (str->date-time coercible-to-ts (when-not (= timezone :no-timezone)
+ (coerce-to-timezone timezone))))
(coerce-to-timestamp coercible-to-ts))))
-(defprotocol IDateTimeFormatterCoercible
+(defprotocol ^:private IDateTimeFormatterCoercible
"Protocol for converting objects to `DateTimeFormatters`."
(->DateTimeFormatter ^org.joda.time.format.DateTimeFormatter [this]
"Coerce object to a `DateTimeFormatter`."))
@@ -139,15 +145,15 @@
(defn parse-date
- "Parse a datetime string S with a custom DATE-FORMAT, which can be a format string, clj-time formatter keyword, or
+ "Parse a datetime string `s` with a custom `date-format`, which can be a format string, clj-time formatter keyword, or
anything else that can be coerced to a `DateTimeFormatter`.
- (parse-date \"yyyyMMdd\" \"20160201\") -> #inst \"2016-02-01\"
- (parse-date :date-time \"2016-02-01T00:00:00.000Z\") -> #inst \"2016-02-01\""
+ (parse-date \"yyyyMMdd\" \"20160201\") -> #inst \"2016-02-01\"
+ (parse-date :date-time \"2016-02-01T00:00:00.000Z\") -> #inst \"2016-02-01\""
^java.sql.Timestamp [date-format, ^String s]
(->Timestamp (time/parse (->DateTimeFormatter date-format) s)))
-(defprotocol ISO8601
+(defprotocol ^:private ISO8601
"Protocol for converting objects to ISO8601 formatted strings."
(->iso-8601-datetime ^String [this timezone-id]
"Coerce object to an ISO8601 date-time string such as \"2015-11-18T23:55:03.841Z\" with a given TIMEZONE."))
@@ -174,12 +180,12 @@
(time/formatters :time)))))
(defn format-time
- "Returns a string representation of the time found in `T`"
+ "Returns a string representation of the time found in `t`"
[t time-zone-id]
(time/unparse (time-formatter time-zone-id) (coerce/to-date-time t)))
(defn is-time?
- "Returns true if `V` is a Time object"
+ "Returns true if `v` is a Time object"
[v]
(and v (instance? Time v)))
@@ -198,13 +204,13 @@
(->Timestamp (System/currentTimeMillis)))
(defn format-date
- "Format DATE using a given DATE-FORMAT. NOTE: This will create a date string in the JVM's timezone, not the report
+ "Format `date` using a given `date-format`. NOTE: This will create a date string in the JVM's timezone, not the report
timezone.
- DATE is anything that can coerced to a `Timestamp` via `->Timestamp`, such as a `Date`, `Timestamp`,
- `Long` (ms since the epoch), or an ISO-8601 `String`. DATE defaults to the current moment in time.
+ `date` is anything that can coerced to a `Timestamp` via `->Timestamp`, such as a `Date`, `Timestamp`,
+ `Long` (ms since the epoch), or an ISO-8601 `String`. `date` defaults to the current moment in time.
- DATE-FORMAT is anything that can be passed to `->DateTimeFormatter`, such as `String`
+ `date-format` is anything that can be passed to `->DateTimeFormatter`, such as `String`
(using [the usual date format args](http://docs.oracle.com/javase/7/docs/api/java/text/SimpleDateFormat.html)),
`Keyword`, or `DateTimeFormatter`.
@@ -218,7 +224,7 @@
(time/unparse (->DateTimeFormatter date-format) (coerce/from-sql-time (->Timestamp date)))))
(def ^{:arglists '([] [date])} date->iso-8601
- "Format DATE a an ISO-8601 string."
+ "Format `date` a an ISO-8601 string."
(partial format-date :date-time))
(defn date-string?
@@ -231,14 +237,14 @@
(->Timestamp s utc)))))
(defn ->Date
- "Coerece DATE to a `java.util.Date`."
+ "Coerece `date` to a `java.util.Date`."
(^java.util.Date []
(java.util.Date.))
(^java.util.Date [date]
(java.util.Date. (.getTime (->Timestamp date)))))
(defn ->Calendar
- "Coerce DATE to a `java.util.Calendar`."
+ "Coerce `date` to a `java.util.Calendar`."
(^java.util.Calendar []
(doto (Calendar/getInstance)
(.setTimeZone (TimeZone/getTimeZone "UTC"))))
@@ -250,7 +256,7 @@
(.setTimeZone (TimeZone/getTimeZone timezone-id)))))
(defn relative-date
- "Return a new `Timestamp` relative to the current time using a relative date UNIT.
+ "Return a new Timestamp relative to the current time using a relative date `unit`.
(relative-date :year -1) -> #inst 2014-11-12 ..."
(^java.sql.Timestamp [unit amount]
@@ -275,7 +281,7 @@
:year})
(defn date-extract
- "Extract UNIT from DATE. DATE defaults to now.
+ "Extract `unit` from `date`. `date` defaults to now.
(date-extract :year) -> 2015"
([unit]
@@ -324,7 +330,7 @@
(format "%d-%02d-01'T'ZZ" year month)))
(defn date-trunc
- "Truncate DATE to UNIT. DATE defaults to now.
+ "Truncate `date` to `unit`. `date` defaults to now.
(date-trunc :month).
;; -> #inst \"2015-11-01T00:00:00\""
@@ -344,7 +350,7 @@
:year (trunc-with-format "yyyy-01-01'T'ZZ" date timezone-id))))
(defn date-trunc-or-extract
- "Apply date bucketing with UNIT to DATE. DATE defaults to now."
+ "Apply date bucketing with `unit` to `date`. `date` defaults to now."
([unit]
(date-trunc-or-extract unit (System/currentTimeMillis) "UTC"))
([unit date]
@@ -369,9 +375,25 @@
(recur (/ n divisor) more)
(format "%.0f %s" (double n) (name unit)))))
+(defn format-microseconds
+ "Format a time interval in microseconds into something more readable."
+ ^String [microseconds]
+ (format-nanoseconds (* 1000.0 microseconds)))
+
+(defn format-milliseconds
+ "Format a time interval in milliseconds into something more readable."
+ ^String [milliseconds]
+ (format-microseconds (* 1000.0 milliseconds)))
+
+(defn format-seconds
+ "Format a time interval in seconds into something more readable."
+ ^String [seconds]
+ (format-milliseconds (* 1000.0 seconds)))
+
+;; TODO - Not sure this belongs in the datetime util namespace
(defmacro profile
- "Like `clojure.core/time`, but lets you specify a MESSAGE that gets printed with the total time,
- and formats the time nicely using `format-nanoseconds`."
+ "Like `clojure.core/time`, but lets you specify a `message` that gets printed with the total time, and formats the
+ time nicely using `format-nanoseconds`."
{:style/indent 1}
([form]
`(profile ~(str form) ~form))
@@ -383,8 +405,8 @@
(format-nanoseconds (- (System/nanoTime) start-time#))))))))
(defn- str->date-time-with-formatters
- "Attempt to parse `DATE-STR` using `FORMATTERS`. First successful
- parse is returned, or nil"
+ "Attempt to parse `date-str` using `formatters`. First successful parse is returned, or `nil` if it cannot be
+ successfully parsed."
([formatters date-str]
(str->date-time-with-formatters formatters date-str nil))
([formatters ^String date-str ^TimeZone tz]
@@ -401,9 +423,9 @@
(->DateTimeFormatter "yyyy-MM-dd HH:mm:ss.SSS"))
(def ^:private ordered-date-parsers
- "When using clj-time.format/parse without a formatter, it tries all default formatters, but not ordered by how
- likely the date formatters will succeed. This leads to very slow parsing as many attempts fail before the right one
- is found. Using this retains that flexibility but improves performance by trying the most likely ones first"
+ "When using clj-time.format/parse without a formatter, it tries all default formatters, but not ordered by how likely
+ the date formatters will succeed. This leads to very slow parsing as many attempts fail before the right one is
+ found. Using this retains that flexibility but improves performance by trying the most likely ones first"
(let [most-likely-default-formatters [:mysql :date-hour-minute-second :date-time :date
:basic-date-time :basic-date-time-no-ms
:date-time :date-time-no-ms]]
@@ -412,7 +434,7 @@
(vals (apply dissoc time/formatters most-likely-default-formatters)))))
(defn str->date-time
- "Like clj-time.format/parse but uses an ordered list of parsers to be faster. Returns the parsed date or nil if it
+ "Like clj-time.format/parse but uses an ordered list of parsers to be faster. Returns the parsed date, or `nil` if it
was unable to be parsed."
(^org.joda.time.DateTime [^String date-str]
(str->date-time date-str nil))
@@ -425,8 +447,7 @@
[(time/formatter "HH:mmZ") (time/formatter "HH:mm:SSZ") (time/formatter "HH:mm:SS.SSSZ")])))
(defn str->time
- "Parse `TIME-STR` and return a `java.sql.Time` instance. Returns nil
- if `TIME-STR` can't be parsed."
+ "Parse `time-str` and return a `java.sql.Time` instance. Returns `nil` if `time-str` can't be parsed."
([^String date-str]
(str->time date-str nil))
([^String date-str ^TimeZone tz]
diff --git a/test/metabase/metabot_test.clj b/test/metabase/metabot_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..be6bee6467c2e4576f38c97d0605df0c5fd22317
--- /dev/null
+++ b/test/metabase/metabot_test.clj
@@ -0,0 +1,37 @@
+(ns metabase.metabot-test
+ (:require [expectations :refer :all]
+ [metabase.metabot :as metabot]
+ [metabase.util.date :as du]))
+
+;; test that if we're not the MetaBot based on Settings, our function to check is working correctly
+(expect
+ false
+ (do
+ (#'metabot/metabot-instance-uuid nil)
+ (#'metabot/metabot-instance-last-checkin nil)
+ (#'metabot/am-i-the-metabot?)))
+
+;; test that if nobody is currently the MetaBot, we will become the MetaBot
+(expect
+ (do
+ (#'metabot/metabot-instance-uuid nil)
+ (#'metabot/metabot-instance-last-checkin nil)
+ (#'metabot/check-and-update-instance-status!)
+ (#'metabot/am-i-the-metabot?)))
+
+;; test that if nobody has checked in as MetaBot for a while, we will become the MetaBot
+(expect
+ (do
+ (#'metabot/metabot-instance-uuid (str (java.util.UUID/randomUUID)))
+ (#'metabot/metabot-instance-last-checkin (du/relative-date :minute -10 (#'metabot/current-timestamp-from-db)))
+ (#'metabot/check-and-update-instance-status!)
+ (#'metabot/am-i-the-metabot?)))
+
+;; check that if another instance has checked in recently, we will *not* become the MetaBot
+(expect
+ false
+ (do
+ (#'metabot/metabot-instance-uuid (str (java.util.UUID/randomUUID)))
+ (#'metabot/metabot-instance-last-checkin (#'metabot/current-timestamp-from-db))
+ (#'metabot/check-and-update-instance-status!)
+ (#'metabot/am-i-the-metabot?)))
diff --git a/test/metabase/models/setting_test.clj b/test/metabase/models/setting_test.clj
index c7c55ef06ac6b7eb920b6d8b3b4acf98c50f3def..6474e20c67236e392d84e3050f9a7a292cd7bd8c 100644
--- a/test/metabase/models/setting_test.clj
+++ b/test/metabase/models/setting_test.clj
@@ -49,6 +49,10 @@
(test-setting-2 setting-2-value))
+(expect
+ String
+ (:tag (meta #'test-setting-1)))
+
;; ## GETTERS
;; Test defsetting getter fn. Should return the value from env var MB_TEST_SETTING_1
(expect "ABCDEFG"
@@ -219,6 +223,10 @@
;;; ------------------------------------------------ BOOLEAN SETTINGS ------------------------------------------------
+(expect
+ Boolean
+ (:tag (meta #'test-boolean-setting)))
+
(expect
{:value nil, :is_env_setting false, :env_name "MB_TEST_BOOLEAN_SETTING", :default nil}
(user-facing-info-with-db-and-env-var-values :test-boolean-setting nil nil))
@@ -293,6 +301,10 @@
;; ok, make sure the setting was set
(toucan-name)))
+(expect
+ String
+ (:tag (meta #'toucan-name)))
+
;;; ----------------------------------------------- Encrypted Settings -----------------------------------------------
@@ -323,6 +335,23 @@
(actual-value-in-db :toucan-name)))
+;;; ----------------------------------------------- TIMESTAMP SETTINGS -----------------------------------------------
+
+(defsetting ^:private test-timestamp-setting
+ "Test timestamp setting"
+ :type :timestamp)
+
+(expect
+ java.sql.Timestamp
+ (:tag (meta #'test-timestamp-setting)))
+
+;; make sure we can set & fetch the value and that it gets serialized/deserialized correctly
+(expect
+ #inst "2018-07-11T09:32:00.000Z"
+ (do (test-timestamp-setting #inst "2018-07-11T09:32:00.000Z")
+ (test-timestamp-setting)))
+
+
;;; --------------------------------------------- Cache Synchronization ----------------------------------------------
(def ^:private settings-last-updated-key @(resolve 'metabase.models.setting/settings-last-updated-key))
@@ -444,3 +473,34 @@
;; detect a cache out-of-date situation and flush the cache as appropriate, giving us the updated value when we
;; call! :wow:
(toucan-name)))
+
+
+;;; ----------------------------------------------- Uncached Settings ------------------------------------------------
+
+(defsetting ^:private uncached-setting
+ "A test setting that should *not* be cached."
+ :cache? false)
+
+;; make sure uncached setting still saves to the DB
+(expect
+ "ABCDEF"
+ (encryption-test/with-secret-key nil
+ (uncached-setting "ABCDEF")
+ (actual-value-in-db "uncached-setting")))
+
+;; make sure that fetching the Setting always fetches the latest value from the DB
+(expect
+ "123456"
+ (encryption-test/with-secret-key nil
+ (uncached-setting "ABCDEF")
+ (db/update-where! Setting {:key "uncached-setting"}
+ :value "123456")
+ (uncached-setting)))
+
+;; make sure that updating the setting doesn't update the last-updated timestamp in the cache $$
+(expect
+ nil
+ (encryption-test/with-secret-key nil
+ (clear-settings-last-updated-value-in-db!)
+ (uncached-setting "abcdef")
+ (settings-last-updated-value-in-db)))
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index 006e5cf54a798830e5f89b0e9df7472d8509d03b..304763012af27fa8462cea0706ca89344c01a66d 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -37,7 +37,8 @@
[dataset-definitions :as defs]]
[toucan.db :as db]
[toucan.util.test :as test])
- (:import java.util.TimeZone
+ (:import com.mchange.v2.c3p0.PooledDataSource
+ java.util.TimeZone
org.joda.time.DateTimeZone
[org.quartz CronTrigger JobDetail JobKey Scheduler Trigger]))
@@ -451,7 +452,7 @@
timezone. That causes problems for tests that we can determine the database's timezone. This function will reset the
connections in the connection pool for `db` to ensure that we get fresh session with no timezone specified"
[db]
- (when-let [conn-pool (:datasource (sql/db->pooled-connection-spec db))]
+ (when-let [^PooledDataSource conn-pool (:datasource (sql/db->pooled-connection-spec db))]
(.softResetAllUsers conn-pool)))
(defn db-timezone-id