diff --git a/project.clj b/project.clj
index 9374b0a266f78d47d0437783029287aa8dea36fe..ccb9a195b8b847b3c42f1968e034ead3942eb1e2 100644
--- a/project.clj
+++ b/project.clj
@@ -140,7 +140,8 @@
:docstring-checker {:include [#"^metabase"]
:exclude [#"test"
#"^metabase\.http-client$"]}
- :profiles {:dev {:dependencies [[expectations "2.2.0-beta2"] ; unit tests
+ :profiles {:dev {:dependencies [[clj-http-fake "1.0.3"] ; Library to mock clj-http responses
+ [expectations "2.2.0-beta2"] ; unit tests
[ring/ring-mock "0.3.0"]] ; Library to create mock Ring requests for unit tests
:plugins [[docstring-checker "1.0.2"] ; Check that all public vars have docstrings. Run with 'lein docstring-checker'
[jonase/eastwood "0.3.1"
diff --git a/src/metabase/integrations/slack.clj b/src/metabase/integrations/slack.clj
index 97df78f092a442ec7b71a0ec5bdb8d8b0459303c..e0164cdf71e4fdbf7a070a8da6bb592a3f6a6e65 100644
--- a/src/metabase/integrations/slack.clj
+++ b/src/metabase/integrations/slack.clj
@@ -1,6 +1,7 @@
(ns metabase.integrations.slack
(:require [cheshire.core :as json]
[clj-http.client :as http]
+ [clojure.java.io :as io]
[clojure.tools.logging :as log]
[metabase.models.setting :as setting :refer [defsetting]]
[metabase.util.i18n :refer [tru]]
@@ -19,7 +20,7 @@
(defn- handle-response [{:keys [status body]}]
- (let [body (json/parse-string body keyword)]
+ (let [body (-> body io/reader (json/parse-stream keyword))]
(if (and (= 200 status) (:ok body))
body
(let [error (if (= (:error body) "invalid_auth")
@@ -31,15 +32,16 @@
(defn- do-slack-request [request-fn params-key endpoint & {:keys [token], :as params, :or {token (slack-token)}}]
(when token
(handle-response (request-fn (str slack-api-base-url "/" (name endpoint)) {params-key (assoc params :token token)
- :conn-timeout 1000
- :socket-timeout 1000}))))
+ :as :stream
+ :conn-timeout 1000
+ :socket-timeout 1000}))))
(def ^{:arglists '([endpoint & {:as params}]), :style/indent 1} GET "Make a GET request to the Slack API." (partial do-slack-request http/get :query-params))
(def ^{:arglists '([endpoint & {:as params}]), :style/indent 1} POST "Make a POST request to the Slack API." (partial do-slack-request http/post :form-params))
(def ^{:arglists '([& {:as args}])} channels-list
"Calls Slack api `channels.list` function and returns the list of available channels."
- (comp :channels (partial GET :channels.list, :exclude_archived 1)))
+ (comp :channels (partial GET :channels.list, :exclude_archived true, :exclude_members true)))
(def ^{:arglists '([& {:as args}])} users-list
"Calls Slack api `users.list` function and returns the list of available users."
@@ -55,7 +57,7 @@
[]
(some (fn [channel] (when (= (:name channel) files-channel-name)
channel))
- (channels-list :exclude_archived 0)))
+ (channels-list :exclude_archived false)))
(defn files-channel
"Calls Slack api `channels.info` to check whether a channel named #metabase_files exists. If it doesn't,
diff --git a/test/metabase/integrations/slack_test.clj b/test/metabase/integrations/slack_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..21f80a3265fdab441b43aba18c6bb396703928f5
--- /dev/null
+++ b/test/metabase/integrations/slack_test.clj
@@ -0,0 +1,122 @@
+(ns metabase.integrations.slack-test
+ (:require [cheshire.core :as json]
+ [clj-http.fake :as http-fake]
+ [clojure.java.io :as io]
+ [expectations :refer :all]
+ [metabase.integrations.slack :as slack-integ :refer :all]
+ [metabase.test.util :as tu]))
+
+(def ^:private default-channels-response
+ (delay (slurp (io/resource "slack_channels_response.json"))))
+
+(def ^:private default-channels
+ (delay (:channels (json/parse-string @default-channels-response keyword))))
+
+(def ^:private channels-request
+ {:address "https://slack.com/api/channels.list"
+ :query-params {:token "test-token"
+ :exclude_archived "true"
+ :exclude_members "true"}})
+
+(defn- expected-200-response [body]
+ (fn [_]
+ {:status 200
+ :body (if (string? body)
+ body
+ (json/generate-string body))}))
+
+(def ^:private invalid-token-response
+ (expected-200-response
+ {:ok false
+ :error "invalid_auth"}))
+
+(defn- exception-if-called [_]
+ (throw (Exception. "Failure, route should not have been invoked")))
+
+;; Channels should return nil if no Slack token has been configured
+(expect
+ nil
+ (http-fake/with-fake-routes {channels-request exception-if-called}
+ (tu/with-temporary-setting-values [slack-token nil]
+ (channels-list))))
+
+;; Test the channels call and expected response
+(expect
+ @default-channels
+ (http-fake/with-fake-routes {channels-request (expected-200-response @default-channels-response)}
+ (tu/with-temporary-setting-values [slack-token "test-token"]
+ (channels-list))))
+
+;; Test the invalid token auth flow
+(expect
+ {:ex-class clojure.lang.ExceptionInfo
+ :msg nil
+ :data {:errors {:slack-token "Invalid token"}}}
+ (http-fake/with-fake-routes {channels-request invalid-token-response}
+ (tu/with-temporary-setting-values [slack-token "test-token"]
+ (tu/exception-and-message
+ (channels-list)))))
+
+(def ^:private default-users-response
+ (delay (slurp (io/resource "slack_users_response.json"))))
+
+(def ^:private default-users
+ (delay (:members (json/parse-string @default-users-response keyword))))
+
+(def ^:private users-request
+ {:address "https://slack.com/api/users.list"
+ :query-params {:token "test-token"}})
+
+;; Users should return nil if no Slack token has been configured
+(expect
+ nil
+ (http-fake/with-fake-routes {users-request exception-if-called}
+ (tu/with-temporary-setting-values [slack-token nil]
+ (users-list))))
+
+;; Test the users call and the expected response
+(expect
+ @default-users
+ (http-fake/with-fake-routes {users-request (expected-200-response @default-users-response)}
+ (tu/with-temporary-setting-values [slack-token "test-token"]
+ (users-list))))
+
+;; Test the invalid token auth flow for users
+(expect
+ {:ex-class clojure.lang.ExceptionInfo
+ :msg nil
+ :data {:errors {:slack-token "Invalid token"}}}
+ (http-fake/with-fake-routes {users-request invalid-token-response}
+ (tu/with-temporary-setting-values [slack-token "test-token"]
+ (tu/exception-and-message
+ (users-list)))))
+
+(def ^:private files-request
+ (assoc-in channels-request [:query-params :exclude_archived] "false"))
+
+;; Asking for the files channel when slack is not configured throws an exception
+(expect
+ {:ex-class clojure.lang.ExceptionInfo
+ :msg (var-get #'slack-integ/channel-missing-msg)
+ :data {:status-code 400}}
+ (http-fake/with-fake-routes {files-request exception-if-called}
+ (tu/exception-and-message
+ (files-channel))))
+
+(defn- create-files-channel []
+ (let [channel-name (var-get #'slack-integ/files-channel-name)]
+ (-> @default-channels
+ first
+ (assoc
+ :name channel-name, :name_normalized channel-name,
+ :purpose {:value "Metabase file upload location", :creator "", :last_set 0}))))
+
+;; Testing the call that finds the metabase files channel
+(expect
+ (create-files-channel)
+ (http-fake/with-fake-routes {files-request (-> @default-channels-response
+ json/parse-string
+ (update :channels conj (create-files-channel))
+ expected-200-response)}
+ (tu/with-temporary-setting-values [slack-token "test-token"]
+ (files-channel))))
diff --git a/test/metabase/query_processor/middleware/add_query_throttle_test.clj b/test/metabase/query_processor/middleware/add_query_throttle_test.clj
index 9abe9798be2591bbad8a292c7502ba0d210a4969..9387c696684a55b9da2377a7a79bc485c81c7418 100644
--- a/test/metabase/query_processor/middleware/add_query_throttle_test.clj
+++ b/test/metabase/query_processor/middleware/add_query_throttle_test.clj
@@ -10,14 +10,6 @@
[metabase.util :as u])
(:import java.util.concurrent.Semaphore))
-(defmacro ^:private exception-and-message [& body]
- `(try
- ~@body
- (catch Exception e#
- {:ex-class (class e#)
- :msg (.getMessage e#)
- :data (ex-data e#)})))
-
(defmacro ^:private with-query-wait-time-in-seconds [time-in-seconds & body]
`(with-redefs [throttle/max-query-wait-time-in-millis ~(* 1000 time-in-seconds)]
~@body))
@@ -30,7 +22,7 @@
:data {:status-code 503
:type ::throttle/concurrent-query-limit-reached}}
(with-query-wait-time-in-seconds 1
- (exception-and-message
+ (tu/exception-and-message
(let [semaphore (Semaphore. 5)]
(.acquire semaphore 5)
((#'throttle/throttle-queries semaphore (constantly "Should never be returned")) {})))))
@@ -43,7 +35,7 @@
:data {:status-code 503
:type ::throttle/concurrent-query-limit-reached}}
(with-query-wait-time-in-seconds 1
- (exception-and-message
+ (tu/exception-and-message
(let [semaphore (Semaphore. 5)
my-qp (->> identity
(#'throttle/throttle-queries semaphore)
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index 705fe1a975a5e206d0be6abab7124e327077a22d..ad4dc0e724d7d86094a623e2961e1186c6bc4eda 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -675,3 +675,13 @@
:else
x))
+
+(defmacro exception-and-message
+ "Invokes `body`, catches the exception and returns a map with the exception class, message and data"
+ [& body]
+ `(try
+ ~@body
+ (catch Exception e#
+ {:ex-class (class e#)
+ :msg (.getMessage e#)
+ :data (ex-data e#)})))
diff --git a/test_resources/slack_channels_response.json b/test_resources/slack_channels_response.json
new file mode 100644
index 0000000000000000000000000000000000000000..b38aa245adecb5410be5fc0b30c708401411825b
--- /dev/null
+++ b/test_resources/slack_channels_response.json
@@ -0,0 +1,70 @@
+{
+ "ok": true,
+ "channels": [
+ {
+ "id": "C0G9QF9GW",
+ "name": "random",
+ "is_channel": true,
+ "created": 1449709280,
+ "creator": "U0G9QF9C6",
+ "is_archived": false,
+ "is_general": false,
+ "name_normalized": "random",
+ "is_shared": false,
+ "is_org_shared": false,
+ "is_member": true,
+ "is_private": false,
+ "is_mpim": false,
+ "members": [
+ "U0G9QF9C6",
+ "U0G9WFXNZ"
+ ],
+ "topic": {
+ "value": "Other stuff",
+ "creator": "U0G9QF9C6",
+ "last_set": 1449709352
+ },
+ "purpose": {
+ "value": "A place for non-work-related flimflam, faffing, hodge-podge or jibber-jabber you'd prefer to keep out of more focused work-related channels.",
+ "creator": "",
+ "last_set": 0
+ },
+ "previous_names": [],
+ "num_members": 2
+ },
+ {
+ "id": "C0G9QKBBL",
+ "name": "general",
+ "is_channel": true,
+ "created": 1449709280,
+ "creator": "U0G9QF9C6",
+ "is_archived": false,
+ "is_general": true,
+ "name_normalized": "general",
+ "is_shared": false,
+ "is_org_shared": false,
+ "is_member": true,
+ "is_private": false,
+ "is_mpim": false,
+ "members": [
+ "U0G9QF9C6",
+ "U0G9WFXNZ"
+ ],
+ "topic": {
+ "value": "Talk about anything!",
+ "creator": "U0G9QF9C6",
+ "last_set": 1449709364
+ },
+ "purpose": {
+ "value": "To talk about anything!",
+ "creator": "U0G9QF9C6",
+ "last_set": 1449709334
+ },
+ "previous_names": [],
+ "num_members": 2
+ }
+ ],
+ "response_metadata": {
+ "next_cursor": "dGVhbTpDMUg5UkVTR0w="
+ }
+}
diff --git a/test_resources/slack_users_response.json b/test_resources/slack_users_response.json
new file mode 100644
index 0000000000000000000000000000000000000000..b91a795278dd46cad98294c79248eed426056724
--- /dev/null
+++ b/test_resources/slack_users_response.json
@@ -0,0 +1,86 @@
+{
+ "ok": true,
+ "members": [
+ {
+ "id": "W012A3CDE",
+ "team_id": "T012AB3C4",
+ "name": "spengler",
+ "deleted": false,
+ "color": "9f69e7",
+ "real_name": "spengler",
+ "tz": "America/Los_Angeles",
+ "tz_label": "Pacific Daylight Time",
+ "tz_offset": -25200,
+ "profile": {
+ "avatar_hash": "ge3b51ca72de",
+ "status_text": "Print is dead",
+ "status_emoji": ":books:",
+ "real_name": "Egon Spengler",
+ "display_name": "spengler",
+ "real_name_normalized": "Egon Spengler",
+ "display_name_normalized": "spengler",
+ "email": "spengler@ghostbusters.example.com",
+ "image_24": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
+ "image_32": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
+ "image_48": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
+ "image_72": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
+ "image_192": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
+ "image_512": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
+ "team": "T012AB3C4"
+ },
+ "is_admin": true,
+ "is_owner": false,
+ "is_primary_owner": false,
+ "is_restricted": false,
+ "is_ultra_restricted": false,
+ "is_bot": false,
+ "updated": 1502138686,
+ "is_app_user": false,
+ "has_2fa": false
+ },
+ {
+ "id": "W07QCRPA4",
+ "team_id": "T0G9PQBBK",
+ "name": "glinda",
+ "deleted": false,
+ "color": "9f69e7",
+ "real_name": "Glinda Southgood",
+ "tz": "America/Los_Angeles",
+ "tz_label": "Pacific Daylight Time",
+ "tz_offset": -25200,
+ "profile": {
+ "avatar_hash": "8fbdd10b41c6",
+ "image_24": "https://a.slack-edge.com...png",
+ "image_32": "https://a.slack-edge.com...png",
+ "image_48": "https://a.slack-edge.com...png",
+ "image_72": "https://a.slack-edge.com...png",
+ "image_192": "https://a.slack-edge.com...png",
+ "image_512": "https://a.slack-edge.com...png",
+ "image_1024": "https://a.slack-edge.com...png",
+ "image_original": "https://a.slack-edge.com...png",
+ "first_name": "Glinda",
+ "last_name": "Southgood",
+ "title": "Glinda the Good",
+ "phone": "",
+ "skype": "",
+ "real_name": "Glinda Southgood",
+ "real_name_normalized": "Glinda Southgood",
+ "display_name": "Glinda the Fairly Good",
+ "display_name_normalized": "Glinda the Fairly Good",
+ "email": "glenda@south.oz.coven"
+ },
+ "is_admin": true,
+ "is_owner": false,
+ "is_primary_owner": false,
+ "is_restricted": false,
+ "is_ultra_restricted": false,
+ "is_bot": false,
+ "updated": 1480527098,
+ "has_2fa": false
+ }
+ ],
+ "cache_ts": 1498777272,
+ "response_metadata": {
+ "next_cursor": "dXNlcjpVMEc5V0ZYTlo="
+ }
+}