Merge pull request #5465 from metabase/backported-improvements-from-sync-refactor

Backported improvements from sync refactor branch

src/metabase/models/database.clj

@@ -78,7 +78,7 @@
 (defn ^:hydrate tables
   "Return the `Tables` associated with this `Database`."
   [{:keys [id]}]
-  (db/select 'Table, :db_id id, :active true, {:order-by [[:%lower.display_name :asc]]}))
+  (db/select 'Table, :db_id id, :active true, {:order-by [[:%lower.display_name :asc]]})) ; TODO - do we want to include tables that should be `:hidden`?
 
 (defn schema-names
   "Return a *sorted set* of schema names (as strings) associated with this `Database`."

src/metabase/models/permissions.clj

@@ -339,7 +339,6 @@
    (grant-permissions! group-or-id (apply object-path db-id schema more)))
   ([group-or-id path]
    (try
-     (log/debug (u/format-color 'green "Granting permissions for group %d: %s" (u/get-id group-or-id) path))
      (db/insert! Permissions
        :group_id (u/get-id group-or-id)
        :object   path)

src/metabase/models/raw_column.clj

-(ns metabase.models.raw-column
+(ns ^:deprecated metabase.models.raw-column
   (:require [metabase.util :as u]
             [toucan
              [db :as db]
              [models :as models]]))
 
-(models/defmodel RawColumn :raw_column)
+(models/defmodel ^:deprecated RawColumn :raw_column)
 
 (defn- pre-insert [table]
   (let [defaults {:active  true

src/metabase/models/raw_table.clj

-(ns metabase.models.raw-table
+(ns ^:deprecated metabase.models.raw-table
   (:require [metabase.models.raw-column :refer [RawColumn]]
             [metabase.util :as u]
             [toucan
              [db :as db]
              [models :as models]]))
 
-(models/defmodel RawTable :raw_table)
+(models/defmodel ^:deprecated RawTable :raw_table)
 
 (defn- pre-insert [table]
   (let [defaults {:details {}}]

src/metabase/task.clj

@@ -52,3 +52,10 @@
   [job trigger]
   (when @quartz-scheduler
     (qs/schedule @quartz-scheduler job trigger)))
+
+(defn delete-task!
+  "delete a task from the scheduler"
+  [job-key trigger-key]
+  (when @quartz-scheduler
+    (qs/delete-trigger @quartz-scheduler trigger-key)
+    (qs/delete-job @quartz-scheduler job-key)))

src/metabase/util.clj

@@ -570,15 +570,29 @@
              (s/join (repeat blanks "·"))
              (format "] %s  %3.0f%%" (emoji (percent-done->emoji percent-done)) (* percent-done 100.0)))))))
 
-(defn filtered-stacktrace
-  "Get the stack trace associated with E and return it as a vector with non-metabase frames filtered out."
-  [^Throwable e]
-  (when e
-    (when-let [stacktrace (.getStackTrace e)]
-      (vec (for [frame stacktrace
-                 :let  [s (str frame)]
-                 :when (re-find #"metabase" s)]
-             (s/replace s #"^metabase\." ""))))))
+
+(defprotocol ^:private IFilteredStacktrace
+  (filtered-stacktrace [this]
+    "Get the stack trace associated with E and return it as a vector with non-metabase frames filtered out."))
+
+(extend nil
+  IFilteredStacktrace {:filtered-stacktrace (constantly nil)})
+
+(extend Throwable
+  IFilteredStacktrace {:filtered-stacktrace (fn [^Throwable this]
+                                              (filtered-stacktrace (.getStackTrace this)))})
+
+(extend Thread
+  IFilteredStacktrace {:filtered-stacktrace (fn [^Thread this]
+                                              (filtered-stacktrace (.getStackTrace this)))})
+
+;; StackTraceElement[] is what the `.getStackTrace` method for Thread and Throwable returns
+(extend (Class/forName "[Ljava.lang.StackTraceElement;")
+  IFilteredStacktrace {:filtered-stacktrace (fn [this]
+                                              (vec (for [frame this
+                                                         :let  [s (str frame)]
+                                                         :when (re-find #"metabase" s)]
+                                                     (s/replace s #"^metabase\." ""))))})
 
 (defn wrap-try-catch
   "Returns a new function that wraps F in a `try-catch`. When an exception is caught, it is logged

src/metabase/util/infer_spaces.clj

@@ -59,7 +59,7 @@
 ;;
 ;;     return " ".join(reversed(out))
 (defn infer-spaces
-  "Splits a string with no spaces into words using magic"
+  "Splits a string with no spaces into words using magic" ; what a great explanation. TODO - make this code readable
   [input]
   (let [s (s/lower-case input)
         cost (build-cost-array s)]

test/metabase/http_client.clj

@@ -98,12 +98,11 @@
         (log/error (u/pprint-to-str 'red body))
         (throw (ex-info message {:status-code actual-status-code}))))))
 
-(defn- method->request-fn [method]
-  (case method
-    :get    client/get
-    :post   client/post
-    :put    client/put
-    :delete client/delete))
+(def ^:private method->request-fn
+  {:get    client/get
+   :post   client/post
+   :put    client/put
+   :delete client/delete})
 
 (defn- -client [credentials method expected-status url http-body url-param-kwargs request-options]
   ;; Since the params for this function can get a little complicated make sure we validate them

test/metabase/test/data/interface.clj

@@ -239,11 +239,11 @@
   If no `:default` param is specified and the var isn't found, throw.
 
      (db-test-env-var :mysql :user) ; Look up `MB_MYSQL_TEST_USER`"
-  ([database-name env-var]
-   (db-test-env-var database-name env-var nil))
-  ([database-name env-var default]
+  ([engine env-var]
+   (db-test-env-var engine env-var nil))
+  ([engine env-var default]
    (get env
-        (keyword (format "mb-%s-test-%s" (name database-name) (name env-var)))
+        (keyword (format "mb-%s-test-%s" (name engine) (name env-var)))
         default)))
 
 (defn- to-system-env-var-str
@@ -259,10 +259,11 @@
 
 (defn db-test-env-var-or-throw
   "Same as `db-test-env-var` but will throw an exception if the variable is nil"
-  ([database-name env-var]
-   (db-test-env-var-or-throw database-name env-var nil))
-  ([database-name env-var default]
-   (or (db-test-env-var database-name env-var default)
+  ([engine env-var]
+   (db-test-env-var-or-throw engine env-var nil))
+  ([engine env-var default]
+   (or (db-test-env-var engine env-var default)
        (throw (Exception. (format "In order to test %s, you must specify the env var MB_%s_TEST_%s."
-                                  (name database-name)
+                                  (name engine)
+                                  (str/upper-case (name engine))
                                   (to-system-env-var-str env-var)))))))

test/metabase/test/data/presto.clj

 (ns metabase.test.data.presto
   (:require [clojure.string :as s]
-            [environ.core :refer [env]]
             [honeysql
              [core :as hsql]
              [helpers :as h]]

test/metabase/test/util.clj

@@ -217,7 +217,7 @@
   (or (symbol? x)
       (instance? clojure.lang.Namespace x)))
 
-(defn resolve-private-vars* [source-namespace target-namespace symbols]
+(defn ^:deprecated resolve-private-vars* [source-namespace target-namespace symbols]
   {:pre [(namespace-or-symbol? source-namespace)
          (namespace-or-symbol? target-namespace)
          (every? symbol? symbols)]}
@@ -227,11 +227,15 @@
                          (throw (Exception. (str source-namespace "/" symb " doesn't exist!"))))]]
     (intern target-namespace symb varr)))
 
-(defmacro resolve-private-vars
+(defmacro ^:deprecated resolve-private-vars
   "Have your cake and eat it too. This Macro adds private functions from another namespace to the current namespace so we can test them.
 
     (resolve-private-vars metabase.driver.generic-sql.sync
-      field-avg-length field-percent-urls)"
+      field-avg-length field-percent-urls)
+
+   DEPRECATED: Just refer to vars directly using `#'` syntax instead of using this macro.
+
+     (#'some-ns/field-avg-length ...)"
   [namespc & symbols]
   `(resolve-private-vars* (quote ~namespc) *ns* (quote ~symbols)))