Merge pull request #2924 from metabase/code-cleanup-1337

Code cleanup 

.dir-locals.el

@@ -16,10 +16,8 @@
                               (catch-api-exceptions 0)
                               (check 1)
                               (checkp 1)
-                              (cond-as-> 2)
                               (context 2)
                               (create-database-definition 1)
-                              (engine-case 0)
                               (execute-query 1)
                               (execute-sql! 2)
                               (expect 0)
@@ -28,25 +26,18 @@
                               (expect-with-all-engines 0)
                               (expect-with-engine 1)
                               (expect-with-engines 1)
-                              (ins 1)
                               (let-400 1)
                               (let-404 1)
                               (let-500 1)
-                              (macrolet 1)
                               (match 1)
                               (match-$ 1)
-                              (org-perms-case 1)
                               (post-select 1)
                               (pre-cascade-delete 1)
                               (pre-insert 1)
                               (pre-update 1)
                               (project 1)
                               (qp-expect-with-engines 1)
-                              (query-with-temp-db 1)
                               (resolve-private-fns 1)
                               (select 1)
-                              (subselect 1)
                               (sync-in-context 2)
-                              (upd 2)
-                              (when-testing-engine 1)
-                              (with-credentials 1)))))))
+                              (when-testing-engine 1)))))))

src/metabase/api/common/internal.clj

@@ -181,11 +181,12 @@
 
     (typify-route \"/:id/card\") -> [\"/:id/card\" :id #\"[0-9]+\"]"
   [route]
-  (if (vector? route) route
-      (let [arg-types (->> (route-arg-keywords route)
-                           typify-args)]
-        (if (empty? arg-types) route
-            (apply vector route arg-types)))))
+  (if (vector? route)
+    route
+    (let [arg-types (typify-args (route-arg-keywords route))]
+      (if (empty? arg-types)
+        route
+        (apply vector route arg-types)))))
 
 
 ;;; ## ROUTE ARG AUTO PARSING
@@ -195,11 +196,11 @@
   that can be used in a `let` form."
   [arg-symbol]
   (when (symbol? arg-symbol)
-    (some-> (arg-type arg-symbol)                                    ; :int
-            *auto-parse-types*                                       ; {:parser ... }
-            :parser                                                  ; Integer/parseInt
+    (some-> (arg-type arg-symbol)                                     ; :int
+            *auto-parse-types*                                        ; {:parser ... }
+            :parser                                                   ; Integer/parseInt
             ((fn [parser] `(when ~arg-symbol (~parser ~arg-symbol)))) ; (when id (Integer/parseInt id))
-            ((partial vector arg-symbol)))))                         ; [id (Integer/parseInt id)]
+            ((partial vector arg-symbol)))))                          ; [id (Integer/parseInt id)]
 
 (defmacro auto-parse
   "Create a `let` form that applies corresponding parse-fn for any symbols in ARGS that are present in `*auto-parse-types*`."

src/metabase/api/database.clj

@@ -115,8 +115,7 @@
                          details
                          (assoc details :password (get-in database [:details :password])))
           conn-error   (test-database-connection engine details)
-          is_full_sync (if (nil? is_full_sync)
-                         nil
+          is_full_sync (when-not (nil? is_full_sync)
                          (boolean is_full_sync))]
       (if-not conn-error
         ;; no error, proceed with update

src/metabase/api/tiles.clj

@@ -81,7 +81,7 @@
           (.setColor graphics color-white)
           (.fillRect graphics (tile-pixel :x) (tile-pixel :y) pin-size pin-size)
           (.setColor graphics color-blue)
-          (.fillRect graphics (+ 1 (tile-pixel :x)) (+ 1 (tile-pixel :y)) (- pin-size 2) (- pin-size 2))))
+          (.fillRect graphics (inc (tile-pixel :x)) (inc (tile-pixel :y)) (- pin-size 2) (- pin-size 2))))
       (catch Throwable e
         (.printStackTrace e))
       (finally

src/metabase/driver.clj

@@ -221,12 +221,13 @@
                                  (filter identity)
                                  (take max-sync-lazy-seq-results))
         field-values-count (count field-values)]
-    (if (= field-values-count 0) 0
-        (int (math/round (/ (->> field-values
-                                 (map str)
-                                 (map count)
-                                 (reduce +))
-                            field-values-count))))))
+    (if (zero? field-values-count)
+      0
+      (int (math/round (/ (->> field-values
+                               (map str)
+                               (map count)
+                               (reduce +))
+                          field-values-count))))))
 
 
 (def IDriverDefaultsMixin

src/metabase/driver/druid/query_processor.clj

@@ -132,7 +132,7 @@
 (defn- handle-aggregation [query-type {{ag-type :aggregation-type, ag-field :field} :aggregation} druid-query]
   (when (isa? query-type ::ag-query)
     (merge druid-query
-           (let [ag-type (if (= ag-type :rows) nil ag-type)]
+           (let [ag-type (when-not (= ag-type :rows) ag-type)]
              (match [ag-type ag-field]
                ;; For 'distinct values' queries (queries with a breakout by no aggregation) just aggregate by count, but name it :___count so it gets discarded automatically
                [nil     nil] {:aggregations [(ag:count :___count)]}

src/metabase/driver/generic_sql.clj

@@ -252,7 +252,7 @@
         0)))
 
 (defn- url-percentage [url-count total-count]
-  (if (and total-count (> total-count 0) url-count)
+  (if (and total-count (pos? total-count) url-count)
     (float (/ url-count total-count))
     0.0))
 
@@ -367,13 +367,11 @@
 
 (defn- describe-table-fks [driver database table]
   (with-metadata [metadata driver database]
-    (set (->> (.getImportedKeys metadata nil (:schema table) (:name table))
-              jdbc/result-set-seq
-              (mapv (fn [result]
-                      {:fk-column-name   (:fkcolumn_name result)
-                       :dest-table       {:name   (:pktable_name result)
-                                          :schema (:pktable_schem result)}
-                       :dest-column-name (:pkcolumn_name result)}))))))
+    (set (for [result (jdbc/result-set-seq (.getImportedKeys metadata nil (:schema table) (:name table)))]
+           {:fk-column-name   (:fkcolumn_name result)
+            :dest-table       {:name   (:pktable_name result)
+                               :schema (:pktable_schem result)}
+            :dest-column-name (:pkcolumn_name result)}))))
 
 
 (defn analyze-table

src/metabase/driver/mongo.clj

 (ns metabase.driver.mongo
   "MongoDB Driver."
-  (:require [clojure.set :as set]
+  (:require (clojure [set :as set]
+                     [string :as s])
             [clojure.tools.logging :as log]
             [cheshire.core :as json]
             (monger [collection :as mc]
@@ -74,40 +75,42 @@
       fields
       (recur more-keys (update fields k (partial update-field-attrs (k field-value)))))))
 
+(defn- safe-inc [n]
+  (inc (or n 0)))
+
 (defn- update-field-attrs [field-value field-def]
-  (let [safe-inc #(inc (or % 0))]
-    (-> field-def
-        (update :count safe-inc)
-        (update :len #(if (string? field-value)
-                       (+ (or % 0) (count field-value))
-                       %))
-        (update :types (fn [types]
-                         (update types (type field-value) safe-inc)))
-        (update :special-types (fn [special-types]
-                                 (if-let [st (val->special-type field-value)]
-                                   (update special-types st safe-inc)
-                                   special-types)))
-        (update :nested-fields (fn [nested-fields]
-                                 (if (isa? (type field-value) clojure.lang.IPersistentMap)
-                                   (find-nested-fields field-value nested-fields)
-                                   nested-fields))))))
+  (-> field-def
+      (update :count safe-inc)
+      (update :len #(if (string? field-value)
+                      (+ (or % 0) (count field-value))
+                      %))
+      (update :types (fn [types]
+                       (update types (type field-value) safe-inc)))
+      (update :special-types (fn [special-types]
+                               (if-let [st (val->special-type field-value)]
+                                 (update special-types st safe-inc)
+                                 special-types)))
+      (update :nested-fields (fn [nested-fields]
+                               (if (isa? (type field-value) clojure.lang.IPersistentMap)
+                                 (find-nested-fields field-value nested-fields)
+                                 nested-fields)))))
 
 (defn- describe-table-field [field-kw field-info]
   ;; TODO: indicate preview-display status based on :len
   (cond-> {:name      (name field-kw)
-           :base-type (->> (into [] (:types field-info))
+           :base-type (->> (vec (:types field-info))
                            (sort-by second)
                            last
                            first
                            driver/class->base-type)}
-          (= :_id field-kw) (assoc :pk? true)
-          (:special-types field-info) (assoc :special-type (->> (into [] (:special-types field-info))
-                                                               (filter #(not (nil? (first %))))
-                                                               (sort-by second)
-                                                               last
-                                                               first))
-          (:nested-fields field-info) (assoc :nested-fields (set (for [field (keys (:nested-fields field-info))]
-                                                                  (describe-table-field field (field (:nested-fields field-info))))))))
+    (= :_id field-kw)           (assoc :pk? true)
+    (:special-types field-info) (assoc :special-type (->> (vec (:special-types field-info))
+                                                          (filter #(not (nil? (first %))))
+                                                          (sort-by second)
+                                                          last
+                                                          first))
+    (:nested-fields field-info) (assoc :nested-fields (set (for [field (keys (:nested-fields field-info))]
+                                                             (describe-table-field field (field (:nested-fields field-info))))))))
 
 (defn- describe-database [database]
   (with-mongo-connection [^com.mongodb.DB conn database]
@@ -154,7 +157,7 @@
          name-components (rest (field/qualified-name-components field))]
      (assert (seq name-components))
      (for [row (mq/with-collection *mongo-connection* (:name table)
-                 (mq/fields [(apply str (interpose "." name-components))]))]
+                 (mq/fields [(s/join \. name-components)]))]
        (get-in row (map keyword name-components))))))
 
 

src/metabase/driver/mongo/query_processor.clj

@@ -66,7 +66,7 @@
 (defn- field->name
   "Return a single string name for FIELD. For nested fields, this creates a combined qualified name."
   ^String [^Field field, ^String separator]
-  (apply str (interpose separator (rest (qualified-name-components field)))))
+  (s/join separator (rest (qualified-name-components field))))
 
 (defmacro ^:private mongo-let
   {:style/indent 1}

src/metabase/driver/sqlite.clj

 (ns metabase.driver.sqlite
-  (:require [clojure.set :as set]
+  (:require (clojure [set :as set]
+                     [string :as s])
             (honeysql [core :as hsql]
                       [format :as hformat])
             [metabase.config :as config]
@@ -32,7 +33,7 @@
 ;; register the SQLite concatnation operator `||` with HoneySQL as `sqlite-concat`
 ;; (hsql/format (hsql/call :sqlite-concat :a :b)) -> "(a || b)"
 (defmethod hformat/fn-handler "sqlite-concat" [_ & args]
-  (str "(" (apply str (interpose " || " (map hformat/to-sql args))) ")"))
+  (str "(" (s/join " || " (map hformat/to-sql args)) ")"))
 
 (def ^:private ->date     (partial hsql/call :date))
 (def ^:private ->datetime (partial hsql/call :datetime))

src/metabase/events.clj

@@ -53,7 +53,7 @@
 (def ^:private events-publication
   "Publication for general events channel.
    Expects a map as input and the map must have a `:topic` key."
-  (async/pub events-channel #(:topic %)))
+  (async/pub events-channel :topic))
 
 (defn publish-event
   "Publish an item into the events stream.

src/metabase/events/metabot_lifecycle.clj

@@ -30,7 +30,7 @@
       (let [{:keys [slack-token metabot-enabled]} object]
         (cond
           (and (contains? object :metabot-enabled)
-               (not (= "true" metabot-enabled)))   (metabot/stop-metabot!)
+               (not= "true" metabot-enabled))      (metabot/stop-metabot!)
           (and (contains? object :slack-token)
                (seq slack-token))                  (metabot/start-metabot!)))
       (catch Throwable e

src/metabase/events/notifications.clj

@@ -41,24 +41,26 @@
     ;; if we have no dependencies on cards then do nothing
     deps-by-model
     ;; otherwise pull out dependent card ids and add dashboard/pulse dependencies
-    (let [card-ids (mapv :model_id (get deps-by-model "Card"))]
+    (let [card-ids (map :model_id (get deps-by-model "Card"))]
       (assoc deps-by-model
-        "Dashboard" (for [dashcard (db/select [DashboardCard :dashboard_id], :card_id [:in card-ids])]
-                      (set/rename-keys dashcard {:dashboard_id :model_id}))
-        "Pulse"     (for [pulsecard (db/select [PulseCard :pulse_id], :card_id [:in card-ids])]
-                      (set/rename-keys pulsecard {:pulse_id :model_id}))))))
+        "Dashboard" (when (seq card-ids)
+                      (for [dashcard (db/select [DashboardCard :dashboard_id], :card_id [:in card-ids])]
+                        (set/rename-keys dashcard {:dashboard_id :model_id})))
+        "Pulse"     (when (seq card-ids)
+                      (for [pulsecard (db/select [PulseCard :pulse_id], :card_id [:in card-ids])]
+                        (set/rename-keys pulsecard {:pulse_id :model_id})))))))
 
 (defn- pull-dependencies [model model-id]
   (when-let [deps (db/select [Dependency :model :model_id]
                     :dependent_on_model model
                     :dependent_on_id    model-id)]
-    (let [deps-by-model     (-> (group-by :model deps)
-                                add-objects-dependent-on-cards)
+    (let [deps-by-model     (add-objects-dependent-on-cards (group-by :model deps))
           deps-with-details (for [model (keys deps-by-model)
                                   :let  [ids (mapv :model_id (get deps-by-model model))]]
                               ;; TODO: this is slightly dangerous because we assume :name and :creator_id are available
-                              (for [object (db/select [(model->entity (keyword model)) :id :name :creator_id]
-                                             :id [:in ids])]
+                              (for [object (when (seq ids)
+                                             (db/select [(model->entity (keyword model)) :id :name :creator_id]
+                                               :id [:in ids]))]
                                 (assoc object :model model)))]
       ;; we end up with a list of lists, so flatten before returning
       (flatten deps-with-details))))

src/metabase/metabot.clj

@@ -26,9 +26,9 @@
   ([message m]
    (str message " " (keys-description m)))
   ([m]
-   (apply str (interpose ", " (sort (for [[k varr] m
-                                          :when    (not (:unlisted (meta varr)))]
-                                      (str \` (name k) \`)))))))
+   (str/join ", " (sort (for [[k varr] m
+                              :when    (not (:unlisted (meta varr)))]
+                          (str \` (name k) \`))))))
 
 (defn- dispatch-fn [verb tag]
   (let [fn-map (into {} (for [[symb varr] (ns-interns *ns*)
@@ -102,7 +102,7 @@
      (throw (Exception. "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")
   ([word & more]
-   (show (apply str (interpose " " (cons word more))))))
+   (show (str/join " " (cons word more)))))
 
 
 (defn meme:up-and-to-the-right

src/metabase/models/dashboard_card.clj

@@ -73,7 +73,7 @@
   ;; first off, just delete all series on the dashboard card (we add them again below)
   (db/cascade-delete! DashboardCardSeries :dashboardcard_id id)
   ;; now just insert all of the series that were given to us
-  (when-not (empty? card-ids)
+  (when (seq card-ids)
     (let [cards (map-indexed (fn [i card-id]
                                {:dashboardcard_id id, :card_id card-id, :position i})
                              card-ids)]

src/metabase/models/field.clj

 (ns metabase.models.field
-  (:require [clojure.data :as d]
-            [clojure.string :as s]
+  (:require (clojure [data :as d]
+                     [string :as s])
             [medley.core :as m]
             [metabase.db :as db]
             (metabase.models [common :as common]
@@ -112,7 +112,7 @@
 (defn qualified-name
   "Return a combined qualified name for FIELD, e.g. `table_name.parent_field_name.field_name`."
   [field]
-  (apply str (interpose \. (qualified-name-components field))))
+  (s/join \. (qualified-name-components field)))
 
 (defn table
   "Return the `Table` associated with this `Field`."

src/metabase/models/pulse.clj

@@ -62,7 +62,7 @@
   ;; first off, just delete any cards associated with this pulse (we add them again below)
   (db/cascade-delete! PulseCard :pulse_id id)
   ;; now just insert all of the cards that were given to us
-  (when-not (empty? card-ids)
+  (when (seq card-ids)
     (let [cards (map-indexed (fn [idx itm] {:pulse_id id :card_id itm :position idx}) card-ids)]
       (db/insert-many! PulseCard cards))))
 
@@ -103,9 +103,11 @@
   (let [new-channels   (group-by (comp keyword :channel_type) channels)
         old-channels   (group-by (comp keyword :channel_type) (db/select PulseChannel :pulse_id id))
         handle-channel #(create-update-delete-channel! id (first (get new-channels %)) (first (get old-channels %)))]
-    (assert (= 0 (count (get new-channels nil))) "Cannot have channels without a :channel_type attribute")
+    (assert (zero? (count (get new-channels nil)))
+      "Cannot have channels without a :channel_type attribute")
     ;; for each of our possible channel types call our handler function
-    (dorun (map handle-channel (vec (keys pulse-channel/channel-types))))))
+    (doseq [[channel-type _] pulse-channel/channel-types]
+      (handle-channel channel-type))))
 
 (defn retrieve-pulse
   "Fetch a single `Pulse` by its ID value."

src/metabase/models/user.clj

@@ -17,7 +17,7 @@
                (not (s/blank? password))))
   (assert (not (:password_salt user))
     "Don't try to pass an encrypted password to (ins User). Password encryption is handled by pre-insert.")
-  (let [salt     (.toString (java.util.UUID/randomUUID))
+  (let [salt     (str (java.util.UUID/randomUUID))
         defaults {:date_joined  (u/new-sql-timestamp)
                   :last_login   nil
                   :is_staff     true
@@ -81,7 +81,7 @@
                         :email      email-address
                         :first_name first-name
                         :last_name  last-name
-                        :password   (if (not (nil? password))
+                        :password   (if-not (nil? password)
                                       password
                                       (str (java.util.UUID/randomUUID))))]
     (when send-welcome
@@ -95,7 +95,7 @@
 (defn set-user-password!
   "Updates the stored password for a specified `User` by hashing the password with a random salt."
   [user-id password]
-  (let [salt     (.toString (java.util.UUID/randomUUID))
+  (let [salt     (str (java.util.UUID/randomUUID))
         password (creds/hash-bcrypt (str salt password))]
     ;; NOTE: any password change expires the password reset token
     (db/update! User user-id

src/metabase/pulse/render.clj

 (ns metabase.pulse.render
   (:require [clojure.java.io :as io]
             (clojure [pprint :refer [cl-format]]
-                     [string :refer [upper-case]])
+                     [string :as s])
             [clojure.tools.logging :as log]
             (clj-time [coerce :as c]
                       [core :as t]
@@ -80,9 +80,9 @@
 
      (style {:font-weight 400, :color \"white\"}) -> \"font-weight: 400; color: white;\""
   [& style-maps]
-  (apply str (interpose " " (for [[k v] (into {} style-maps)
-                                  :let  [v (if (keyword? v) (name v) v)]]
-                              (str (name k) ": " v ";")))))
+  (s/join " " (for [[k v] (into {} style-maps)
+                    :let  [v (if (keyword? v) (name v) v)]]
+                (str (name k) ": " v ";"))))
 
 
 (defn- datetime-field?
@@ -109,7 +109,11 @@
     :hour          (f/unparse (f/formatter "h a - MMM YYYY") (c/from-long timestamp))
     :week          (str "Week " (f/unparse (f/formatter "w - YYYY") (c/from-long timestamp)))
     :month         (f/unparse (f/formatter "MMMM YYYY") (c/from-long timestamp))
-    :quarter       (str "Q" (+ 1 (int (/ (t/month (c/from-long timestamp)) 3))) " - " (t/year (c/from-long timestamp)))
+    :quarter       (str "Q"
+                        (inc (int (/ (t/month (c/from-long timestamp))
+                                     3)))
+                        " - "
+                        (t/year (c/from-long timestamp)))
     :year          (str timestamp)
     :hour-of-day   (str timestamp) ; TODO: probably shouldn't even be showing sparkline for x-of-y groupings?
     :day-of-week   (str timestamp)
@@ -127,7 +131,8 @@
     (t/within? (t/interval (t/minus interval-start interval) interval-start)                   date) last-interval-name))
 
 (defn- start-of-this-week    [] (-> (org.joda.time.LocalDate.) .weekOfWeekyear .roundFloorCopy .toDateTimeAtStartOfDay))
-(defn- start-of-this-quarter [] (t/date-midnight (year) (+ 1 (* 3 (Math/floor (/ (dec (month)) 3))))))
+(defn- start-of-this-quarter [] (t/date-midnight (year) (inc (* 3 (Math/floor (/ (dec (month))
+                                                                                 3))))))
 
 (defn- format-timestamp-relative
   "Formats timestamps with relative names (today, yesterday, this *, last *) based on column :unit, if possible, otherwie returns nil"
@@ -223,15 +228,15 @@
     [:table {:style (style {:padding-bottom :8px, :border-bottom (str "4px solid " color-gray-1)})}
      [:thead
       [:tr
-       (for [col-idx col-indexes :let [col (-> cols (nth col-idx))]]
+       (for [col-idx col-indexes :let [col (nth cols col-idx)]]
          [:th {:style (style bar-td-style bar-th-style {:min-width :60px})}
-          (h (upper-case (name (or (:display_name col) (:name col)))))])
+          (h (s/upper-case (name (or (:display_name col) (:name col)))))])
        (when bar-column
          [:th {:style (style bar-td-style bar-th-style {:width "99%"})}])]]
      [:tbody
       (map-indexed (fn [row-idx row]
                      [:tr {:style (style {:color (if (odd? row-idx) color-gray-2 color-gray-3)})}
-                      (for [col-idx col-indexes :let [col (-> cols (nth col-idx))]]
+                      (for [col-idx col-indexes :let [col (nth cols col-idx)]]
                         [:td {:style (style bar-td-style (when (and bar-column (= col-idx 1)) {:font-weight 700}))}
                          (-> row (nth col-idx) (format-cell col) h)])
                       (when bar-column
@@ -319,7 +324,7 @@
   (let [ft-row (if (datetime-field? (first cols))
                  #(.getTime ^Date (u/->Timestamp %))
                  identity)
-        rows   (if (> (ft-row (first (first rows)))
+        rows   (if (> (ft-row (ffirst rows))
                       (ft-row (first (last rows))))
                  (reverse rows)
                  rows)

src/metabase/query_processor.clj

@@ -577,7 +577,7 @@
   {:arglists '([query options])}
   [query {:keys [executed_by]}]
   {:pre [(integer? executed_by)]}
-  (let [query-uuid      (.toString (java.util.UUID/randomUUID))
+  (let [query-uuid      (str (java.util.UUID/randomUUID))
         query-hash      (hash query)
         query-execution {:uuid              query-uuid
                          :executor_id       executed_by