things are almost all working again

src/metabase/api/meta/db.clj

@@ -41,7 +41,7 @@
   (write-check Org org)
   (let-500 [new-db (ins Database :organization_id org :name name :engine engine :details details)]
     ;; kick off background job to gather schema metadata about our new db
-    (future (driver/sync-database new-db))
+    (future (driver/sync-database! new-db))
     ;; make sure we return the newly created db object
     new-db))
 
@@ -137,7 +137,7 @@
   [id]
   (let-404 [db (sel :one Database :id id)]
     (write-check db)
-    (future (driver/sync-database db))) ; run sync-tables asynchronously
+    (future (driver/sync-database! db))) ; run sync-tables asynchronously
   {:status :ok})
 
 

src/metabase/api/meta/table.clj

@@ -74,7 +74,7 @@
   (let-404 [table (sel :one Table :id id)]
     (write-check table)
     ;; run the task asynchronously
-    (future (driver/sync-table table)))
+    (future (driver/sync-table! table)))
   {:status :ok})
 
 (defendpoint POST "/:id/reorder"

src/metabase/api/notify.clj

@@ -15,10 +15,10 @@
   (let-404 [database (sel :one Database :id id)]
     (cond
       table_id (when-let [table (sel :one Table :db_id id :id (int table_id))]
-                 (future (driver/sync-table table)))
+                 (future (driver/sync-table! table)))
       table_name (when-let [table (sel :one Table :db_id id :name table_name)]
-                   (future (driver/sync-table table)))
-      :else (future (driver/sync-database database))))
+                   (future (driver/sync-table! table)))
+      :else (future (driver/sync-database! database))))
   {:success true})
 
 

src/metabase/db/metadata_queries.clj

@@ -7,7 +7,7 @@
 ;; It would be more efficient if we could let the QP could macroexpand normally for predefined queries like these
 
 (defn- field-query [field query]
-  (->> (driver/driver-process-query
+  (->> (driver/process-query
         {:type :query
          :database ((u/deref-> field :table :db) :id)
          :query (assoc query

src/metabase/driver.clj

@@ -5,7 +5,8 @@
             [cheshire.core :as cheshire]
             [medley.core :refer :all]
             [metabase.db :refer [exists? ins sel upd]]
-            (metabase.driver [result :as result])
+            (metabase.driver [interface :as i]
+                             [result :as result])
             (metabase.models [database :refer [Database]]
                              [query-execution :refer [QueryExecution]])
             [metabase.util :as u]))
@@ -18,31 +19,6 @@
   [["h2" "H2"]
    ["postgres" "PostgreSQL"]])
 
-(defprotocol IDriver
-  ;; Connection
-  (can-connect? [this database]
-    "Check whether we can connect to DATABASE and perform a simple query.
-     (To check whether we can connect to a database given only its details, use `can-connect-with-details?` instead).
-
-       (can-connect? (sel :one Database :id 1))")
-
-  (can-connect-with-details? [this details-map]
-    "Check whether we can connect to a database and performa a simple query.
-     Returns true if we can, otherwise returns false or throws an Exception.
-
-       (can-connect-with-details? {:engine :postgres, :dbname \"book\", ...})")
-
-  ;; Syncing
-  (sync-database! [this database]
-    "Sync DATABASE and all its Tables and Fields.")
-
-  (sync-table! [this table]
-    "Sync TABLE and all its Fields.")
-
-  ;; Query Processing
-  (process-query [this query]
-    "Process a native or structured query."))
-
 
 ;; ## Driver Lookup
 
@@ -62,24 +38,26 @@
 
 ;; ## Implementation-Agnostic Driver API
 
-(defn driver-can-connect? [database]
-  (can-connect? ^IDriver (engine->driver (:engine database)) database))
-
-(defn driver-can-connect-with-details [details])
-
-(defn driver-sync-database! [database]
-  (sync-database! ^IDriver (engine->driver (:engine database)) database))
+(defn can-connect? [database]
+  (i/can-connect? ^i/IDriver (engine->driver (:engine database)) database))
 
-(defn driver-sync-table! [table]
-  (sync-table! ^IDriver (database-id->driver (:db_id table)) table))
+(defn can-connect-with-details? [engine details-map]
+  (i/can-connect-with-details? ^i/IDriver (engine->driver engine) details-map))
 
-(defn driver-process-query [query]
-  (process-query ^IDriver (database-id->driver (:database query)) query))
+(def ^{:arglists '([database])} sync-database!
+  "Sync a `Database`, its `Tables`, and `Fields`."
+  (let [-sync-database! (u/runtime-resolved-fn 'metabase.driver.sync 'sync-database!)] ; these need to be resolved at runtime to avoid circular deps
+    (fn [database]
+      (-sync-database! ^i/IDriver (engine->driver (:engine database)) database))))
 
-;; ## DEPRECATED API -- Pending Removal
+(def ^{:arglists '([table])} sync-table!
+  "Sync a `Table` and its `Fields`."
+  (let [-sync-table! (u/runtime-resolved-fn 'metabase.driver.sync 'sync-table!)]
+    (fn [table]
+      (-sync-table! ^i/IDriver (database-id->driver (:db_id table)) table))))
 
-(defn connection [database]
-  nil)
+(defn process-query [query]
+  (i/process-query ^i/IDriver (database-id->driver (:database query)) query))
 
 
 ;; ## Query Execution Stuff
@@ -88,8 +66,8 @@
   "Process and run a query and return results."
   [{:keys [type] :as query}]
   (case (keyword type)
-    :native (driver-process-query query)
-    :query (driver-process-query query)
+    :native (process-query query)
+    :query (process-query query)
     :result (result/process-and-run query)))
 
 (defn dataset-query

src/metabase/driver/generic_sql.clj

 (ns metabase.driver.generic-sql
-  (:require [clojure.tools.logging :as log]
+  (:require [clojure.java.jdbc :as jdbc]
+            [clojure.tools.logging :as log]
+            [korma.core :refer :all]
             [metabase.driver :as driver]
-            [metabase.driver.sync :as driver-sync]
+            (metabase.driver [interface :refer :all]
+                             [sync :as driver-sync])
             (metabase.driver.generic-sql [connection :as connection]
-                                         [sync :as sync]
                                          [query-processor :as qp]
                                          [util :refer :all])))
 
-(defmacro deftype+
-  "Same as `deftype` but define an extra constructor fn that takes params as kwargs."
-  [name fields convenience-fn-name & body]
-  `(do (deftype ~name ~fields
-         ~@body)
-       (defn ~convenience-fn-name [& {:keys ~fields}]
-         (new ~name ~@fields))))
-
-(deftype+ SqlDriver [column->base-type
-                     connection-details->korma-connection
+(defrecord SqlDriver [column->base-type
+                     connection-details->connection-spec
                      database->connection-details
                      sql-string-length-fn]
-  make-sql-driver
-
-  driver/IDriver
+  IDriver
   ;; Connection
   (can-connect? [_ database]
     (try (connection/test-connection (-> database
                                          database->connection-details
-                                         connection-details->korma-connection))
+                                         connection-details->connection-spec))
          (catch Throwable e
            (log/error "Failed to connect to database:" (.getMessage e))
            false)))
 
   (can-connect-with-details? [_ details]
-    (connection/test-connection (connection-details->korma-connection details)))
+    (connection/test-connection (connection-details->connection-spec details)))
+
+  ;; Query Processing
+  (process-query [_ query]
+    (qp/process-and-run query))
 
   ;; Syncing
-  (sync-database! [_ database]
-    (with-jdbc-metadata [md database]
-      (driver-sync/sync-database! (sync/->GenericSqlSyncDriverDatasource column->base-type sql-string-length-fn md) database)))
+  (sync-in-context [_ database do-sync-fn]
+      (with-jdbc-metadata [_ database]
+        (do-sync-fn)))
 
-  (sync-table! [_ table]
-    (let [database @(:db table)]
-      (with-jdbc-metadata [md database]
-        (driver-sync/sync-table! (sync/->GenericSqlSyncDriverDatasource column->base-type sql-string-length-fn md) table))))
+  (active-table-names [_ database]
+    (with-jdbc-metadata [^java.sql.DatabaseMetaData md database]
+      (->> (.getTables md nil nil nil (into-array String ["TABLE"]))
+           jdbc/result-set-seq
+           (map :table_name)
+           set)))
 
-  ;; Query Processing
-  (process-query [_ query]
-    (qp/process-and-run query)))
+  (active-column-names->type [_ table]
+    (with-jdbc-metadata [^java.sql.DatabaseMetaData md @(:db table)]
+      (->> (.getColumns md nil nil (:name table) nil)
+           jdbc/result-set-seq
+           (filter #(not= (:table_schem %) "INFORMATION_SCHEMA")) ; filter out internal tables
+           (map (fn [{:keys [column_name type_name]}]
+                  {column_name (column->base-type (or (keyword type_name)
+                                                      :UnknownField))}))
+           (into {}))))
+
+  (table-pks [_ table]
+    (with-jdbc-metadata [^java.sql.DatabaseMetaData md @(:db table)]
+      (->> (.getPrimaryKeys md nil nil (:name table))
+           jdbc/result-set-seq
+           (map :column_name)
+           set)))
+
+  ISyncDriverTableFKs
+  (table-fks [_ table]
+    (with-jdbc-metadata [^java.sql.DatabaseMetaData md @(:db table)]
+      (->> (.getImportedKeys md nil nil (:name table))
+           jdbc/result-set-seq
+           (map (fn [result]
+                  {:fk-column-name   (:fkcolumn_name result)
+                   :dest-table-name  (:pktable_name result)
+                   :dest-column-name (:pkcolumn_name result)}))
+           set)))
+
+  ISyncDriverFieldAvgLength
+  (field-avg-length [_ field]
+    (or (some-> (korma-entity @(:table field))
+                (select (aggregate (avg (sqlfn* sql-string-length-fn
+                                                (raw (format "CAST(\"%s\" AS TEXT)" (name (:name field))))))
+                                   :len))
+                first
+                :len
+                int)
+        0))
+
+  ISyncDriverFieldPercentUrls
+  (field-percent-urls [_ field]
+    (let [korma-table (korma-entity @(:table field))
+          total-non-null-count (-> (select korma-table
+                                           (aggregate (count :*) :count)
+                                           (where {(keyword (:name field)) [not= nil]})) first :count)]
+      (if (= total-non-null-count 0) 0
+          (let [url-count (-> (select korma-table
+                                      (aggregate (count :*) :count)
+                                      (where {(keyword (:name field)) [like "http%://_%.__%"]})) first :count)]
+            (float (/ url-count total-non-null-count)))))))

src/metabase/driver/generic_sql/native.clj

@@ -53,7 +53,7 @@
              (with-out-str (clojure.pprint/pprint query)))
   (try (let [database (sel :one Database :id database-id)
              db (-> database
-                    korma-db
+                    db->korma-db
                     korma.db/get-connection)
              [columns & [first-row :as rows]] (jdbc/with-db-transaction [conn db :read-only? true]
                                                 ;; If timezone is specified in the Query and the driver supports setting the timezone then execute SQL to set it
@@ -79,5 +79,5 @@
                      (.getMessage e))})))
 
 (def db (delay (-> (sel :one Database :id 1)
-                   korma-db
+                   db->korma-db
                    korma.db/get-connection)))

src/metabase/driver/generic_sql/sync.clj

-(ns metabase.driver.generic-sql.sync
-  "Generic implementations of `metabase.driver` `(sync-tables [db]) function that should work across any SQL database supported by Korma."
-  (:require [clojure.java.jdbc :as jdbc]
-            [clojure.math.numeric-tower :as math]
-            [clojure.tools.logging :as log]
-            [korma.core :refer :all]
-            [metabase.db :refer :all]
-            [metabase.driver.sync :as sync]
-            [metabase.driver.generic-sql.util :refer :all]
-            (metabase.models [database :refer [Database]]
-                             [field :refer [Field]]
-                             [foreign-key :refer [ForeignKey]]
-                             [table :refer [Table]])
-            [metabase.util :as u]))
-
-;; # NEW IMPL
-
-(deftype GenericSqlSyncDriverDatasource [column->base-type sql-string-length-fn ^java.sql.DatabaseMetaData metadata]
-  sync/ISyncDriverDataSource
-  (active-table-names [_ database]
-    (->> (.getTables metadata nil nil nil (into-array String ["TABLE"]))
-         jdbc/result-set-seq
-         (map :table_name)
-         set))
-
-  (active-column-names->type [_ table]
-    (->> (.getColumns metadata nil nil (:name table) nil)
-         jdbc/result-set-seq
-         (filter #(not= (:table_schem %) "INFORMATION_SCHEMA")) ; filter out internal tables
-         (map (fn [{:keys [column_name type_name]}]
-                {column_name (column->base-type (or (keyword type_name)
-                                                    :UnknownField))}))
-         (into {})))
-
-  (table-pks [_ table]
-    (->> (.getPrimaryKeys metadata nil nil (:name table))
-         jdbc/result-set-seq
-         (map :column_name)
-         set))
-
-  sync/ISyncDriverTableFKs
-  (table-fks [_ table]
-    (->> (.getImportedKeys metadata nil nil (:name table))
-         jdbc/result-set-seq
-         (map (fn [result]
-                {:fk-column-name   (:fkcolumn_name result)
-                 :dest-table-name  (:pktable_name result)
-                 :dest-column-name (:pkcolumn_name result)}))
-         set))
-
-  sync/ISyncDriverFieldValues
-  (field-values-lazy-seq [_ field]
-    (let [korma-table (korma-entity @(:table field))]
-      (->> (select korma-table (fields [(keyword (:name field)) :value]))
-           (map :value))))
-
-  sync/ISyncDriverFieldAvgLength
-  (field-avg-length [_ field]
-    (or (some-> (korma-entity @(:table field))
-                (select (aggregate (avg (sqlfn* sql-string-length-fn
-                                                (raw (format "CAST(\"%s\" AS TEXT)" (name (:name field))))))
-                                   :len))
-                first
-                :len
-                int)
-        0))
-
-  sync/ISyncDriverFieldPercentUrls
-  (field-percent-urls [_ field]
-    (let [korma-table (korma-entity @(:table field))
-          total-non-null-count (-> (select korma-table
-                                           (aggregate (count :*) :count)
-                                           (where {(keyword (:name field)) [not= nil]})) first :count)]
-      (if (= total-non-null-count 0) 0
-          (let [url-count (-> (select korma-table
-                                      (aggregate (count :*) :count)
-                                      (where {(keyword (:name field)) [like "http%://_%.__%"]})) first :count)]
-            (float (/ url-count total-non-null-count)))))))

src/metabase/driver/generic_sql/util.clj

@@ -13,19 +13,30 @@
                              [field :refer [Field]]
                              [table :refer [Table]])))
 
-
 ;; Cache the Korma DB connections for a given Database for 60 seconds instead of creating new ones every single time
-(def ^{:arglists '([database])} korma-db
+(defn- db->connection-spec [database]
+  (let [driver                              (driver/engine->driver (:engine database))
+        database->connection-details        (:database->connection-details driver)
+        connection-details->connection-spec (:connection-details->connection-spec driver)]
+    (-> database database->connection-details connection-details->connection-spec)))
+
+#_(def ^{:arglists '([database])} db->korma-db
   "Return a Korma database definition for DATABASE."
-  (let [db-id->korma-db (memo/ttl (fn [db]
-                                    (log/debug (color/red "Creating a new DB connection..."))
-                                    (kdb/create-db (assoc (driver/connection db)
-                                                          :make-pool? true)))
-                                  :ttl/threshold (* 60 1000))]
+  (let [-db->korma-db (memo/ttl (fn [database]
+                                  (log/debug (color/red "Creating a new DB connection..."))
+                                  (kdb/create-db (db->connection-spec database)))
+                                :ttl/threshold (* 60 1000))]
     ;; only :engine and :details are needed for driver/connection so just pass those so memoization works as expected
     (fn [database]
-      (db-id->korma-db (select-keys database [:engine :details])))))
+      (-db->korma-db (select-keys database [:engine :details])))))
 
+(defn db->korma-db [database]
+  (kdb/create-db (db->connection-spec database)))
+
+(def -db (sel :one Database :id 1))
+(defn x []
+  (-> (clojure.set/rename-keys (:details -db) {:conn_str :db})
+      korma.db/h2))
 
 (def ^:dynamic ^java.sql.DatabaseMetaData *jdbc-metadata*
   "JDBC metadata object for a database. This is set by `with-jdbc-metadata`."
@@ -35,7 +46,7 @@
   "Internal implementation. Don't use this directly; use `with-jdbc-metadata`."
   [database f]
   (if *jdbc-metadata* (f *jdbc-metadata*)
-                      (jdbc/with-db-metadata [md (driver/connection database)]
+                      (jdbc/with-db-metadata [md (db->connection-spec database)]
                         (binding [*jdbc-metadata* md]
                           (f *jdbc-metadata*)))))
 
@@ -70,8 +81,7 @@
   {:pre [(delay? db)]}
   {:table name
    :pk    :id
-   :db    (korma-db @db)})
-
+   :db    (db->korma-db @db)})
 
 (defn table-id->korma-entity
   "Lookup `Table` with TABLE-ID and return a korma entity that can be used in a korma form."

src/metabase/driver/h2.clj

@@ -6,11 +6,11 @@
 
 ;; ## CONNECTION
 
-(defn- connection-details->korma-connection [details-map]
-  (korma.db/h2 (set/rename-keys details-map {:conn_str :db})))
+(defn- connection-details->connection-spec [details-map]
+  (korma.db/h2 details-map))
 
 (defn- database->connection-details [database]
-  (:details database))
+  (set/rename-keys (:details database) {:conn_str :db}))
 
 
 ;; ## SYNCING
@@ -83,9 +83,9 @@
 ;; ## DRIVER
 
 (def ^:const driver
-  (generic-sql/make-sql-driver
-   :column->base-type                    column->base-type
-   :connection-details->korma-connection connection-details->korma-connection
-   :database->connection-details         database->connection-details
-   :sql-string-length-fn                 :LENGTH
-   :timezone->set-timezone-sql           nil))
+  (generic-sql/map->SqlDriver
+   {:column->base-type                   column->base-type
+    :connection-details->connection-spec connection-details->connection-spec
+    :database->connection-details        database->connection-details
+    :sql-string-length-fn                :LENGTH
+    :timezone->set-timezone-sql          nil}))

src/metabase/driver/interface.clj

+(ns metabase.driver.interface)
+
+;; ## IDriver Protocol
+
+(defprotocol IDriver
+  "Methods all drivers must implement."
+  ;; Connection
+  (can-connect? [this database]
+    "Check whether we can connect to DATABASE and perform a simple query.
+     (To check whether we can connect to a database given only its details, use `can-connect-with-details?` instead).
+
+       (can-connect? (sel :one Database :id 1))")
+  (can-connect-with-details? [this details-map]
+    "Check whether we can connect to a database and performa a simple query.
+     Returns true if we can, otherwise returns false or throws an Exception.
+
+       (can-connect-with-details? {:engine :postgres, :dbname \"book\", ...})")
+
+  ;; Syncing
+  (sync-in-context [this database do-sync-fn]
+    "This function is basically around-advice for `sync-database!` and `sync-table!` operations.
+     Implementers can setup any context necessary for syncing, then need to call DO-SYNC-FN,
+     which takes no args.
+
+       (sync-in-context [_ database do-sync-fn]
+         (with-jdbc-metadata [_ database]
+           (do-sync-fn)))")
+  (active-table-names [this database]
+    "Return a set of string names of tables, collections, or equivalent that currently exist in DATABASE.")
+  (active-column-names->type [this table]
+    "Return a map of string names of active columns (or equivalent) -> `Field` `base_type` for TABLE (or equivalent).")
+  (table-pks [this table]
+    "Return a set of string names of active Fields that are primary keys for TABLE (or equivalent).")
+
+  ;; Query Processing
+  (process-query [this query]
+    "Process a native or structured query."))
+
+
+;; ## ISyncDriverTableFKs Protocol (Optional)
+
+(defprotocol ISyncDriverTableFKs
+  "Optional protocol to provide FK information for a TABLE.
+   If a sync driver implements it, Table FKs will be synced; otherwise, the step will be skipped."
+  (table-fks [this table]
+    "Return a set of maps containing info about FK columns for TABLE.
+     Each map should contain the following keys:
+
+     *  fk-column-name
+     *  dest-table-name
+     *  dest-column-name"))
+
+
+;; ## ISyncDriverField Protocols
+
+;; Sync drivers need to implement either ISyncDriverFieldValues or ISyncDriverFieldAvgLength *and* ISyncDriverFieldPercentUrls.
+;;
+;; ISyncDriverFieldValues is used to provide a generic fallback implementation of the other two that calculate these values by
+;; iterating over *every* value of the Field in Clojure-land. Since that's slower, it's preferable to provide implementations
+;; of ISyncDriverFieldAvgLength/ISyncDriverFieldPercentUrls when possible. (You can also implement ISyncDriverFieldValues and
+;; *one* of the other two; the optimized implementation will be used for that and the fallback implementation for the other)
+
+(defprotocol ISyncDriverFieldValues
+  "Optional. Used to implement generic fallback implementations of `ISyncDriverFieldAvgLength` and `ISyncDriverFieldPercentUrls`.
+   If a sync driver doesn't implement *either* of those protocols, it must implement this one."
+  (field-values-lazy-seq [this field]
+    "Return a lazy sequence of all values of Field."))
+
+(defprotocol ISyncDriverFieldAvgLength
+  "Optional. If this isn't provided, a fallback implementation that calculates average length in Clojure-land will be used instead.
+   If a driver doesn't implement this protocol, it *must* implement `ISyncDriverFieldValues`."
+  (field-avg-length [this field]
+    "Return the average length of all non-nil values of textual FIELD."))
+
+(defprotocol ISyncDriverFieldPercentUrls
+  "Optional. If this isn't provided, a fallback implementation that calculates URL percentage in Clojure-land will be used instead.
+   If a driver doesn't implement this protocol, it *must* implement `ISyncDriverFieldValues`."
+  (field-percent-urls [this field]
+    "Return the percentage of non-nil values of textual FIELD that are valid URLs."))

src/metabase/driver/postgres.clj

@@ -73,7 +73,7 @@
 
 ;; ## CONNECTION
 
-(defn- connection-details->korma-connection [details-map]
+(defn- connection-details->connection-spec [details-map]
   (kdb/postgres (rename-keys details-map {:dbname :db})))
 
 (defn- database->connection-details [database]
@@ -102,9 +102,9 @@
 ;; ## DRIVER
 
 (def ^:const driver
-  (generic-sql/make-sql-driver
-   :column->base-type                    column->base-type
-   :connection-details->korma-connection connection-details->korma-connection
-   :database->connection-details         database->connection-details
-   :sql-string-length-fn                 :CHAR_LENGTH
-   :timezone->set-timezone-sql           timezone->set-timezone-sql))
+  (generic-sql/map->SqlDriver
+   {:column->base-type                   column->base-type
+    :connection-details->connection-spec connection-details->connection-spec
+    :database->connection-details        database->connection-details
+    :sql-string-length-fn                :CHAR_LENGTH
+    :timezone->set-timezone-sql          timezone->set-timezone-sql}))

src/metabase/driver/sync.clj

 (ns metabase.driver.sync
-  "Generalized DB / Table syncing functionality."
+  "The logic for doing DB and Table syncing itself."
   (:require [clojure.math.numeric-tower :as math]
             [clojure.tools.logging :as log]
             [colorize.core :as color]
             [korma.core :as k]
             [metabase.db :refer :all]
+            [metabase.driver.interface :refer :all]
             [metabase.driver.sync.queries :as queries]
             (metabase.models [field :refer [Field] :as field]
                              [foreign-key :refer [ForeignKey]]
@@ -23,107 +24,57 @@
          sync-table-fields-metadata!
          update-table-row-count!)
 
-
-;; ## ISyncDriverDataSource Protocol
-
-(defprotocol ISyncDriverDataSource
-  "Required -- all sync drivers must implement this protocol."
-  (active-table-names [this database]
-    "Return a set of string names of tables, collections, or equivalent that currently exist in DATABASE.")
-  (active-column-names->type [this table]
-    "Return a map of string names of active columns (or equivalent) -> `Field` `base_type` for TABLE (or equivalent).")
-  (table-pks [this table]
-    "Return a set of string names of active Fields that are primary keys for TABLE (or equivalent)."))
-
-
-;; ## ISyncDriverTableFKs Protocol
-
-(defprotocol ISyncDriverTableFKs
-  "Optional protocol to provide FK information for a TABLE.
-   If a sync driver implements it, Table FKs will be synced; otherwise, the step will be skipped."
-  (table-fks [this table]
-    "Return a set of maps containing info about FK columns for TABLE.
-     Each map should contain the following keys:
-
-     *  fk-column-name
-     *  dest-table-name
-     *  dest-column-name"))
-
-
-;; ## ISyncDriverField Protocols
-
-;; Sync drivers need to implement either ISyncDriverFieldValues or ISyncDriverFieldAvgLength *and* ISyncDriverFieldPercentUrls.
-;;
-;; ISyncDriverFieldValues is used to provide a generic fallback implementation of the other two that calculate these values by
-;; iterating over *every* value of the Field in Clojure-land. Since that's slower, it's preferable to provide implementations
-;; of ISyncDriverFieldAvgLength/ISyncDriverFieldPercentUrls when possible. (You can also implement ISyncDriverFieldValues and
-;; *one* of the other two; the optimized implementation will be used for that and the fallback implementation for the other)
-
-(defprotocol ISyncDriverFieldValues
-  "Optional. Used to implement generic fallback implementations of `ISyncDriverFieldAvgLength` and `ISyncDriverFieldPercentUrls`.
-   If a sync driver doesn't implement *either* of those protocols, it must implement this one."
-  (field-values-lazy-seq [this field]
-    "Return a lazy sequence of all values of Field."))
-
-(defprotocol ISyncDriverFieldAvgLength
-  "Optional. If this isn't provided, a fallback implementation that calculates average length in Clojure-land will be used instead.
-   If a driver doesn't implement this protocol, it *must* implement `ISyncDriverFieldValues`."
-  (field-avg-length [this field]
-    "Return the average length of all non-nil values of textual FIELD."))
-
-(defprotocol ISyncDriverFieldPercentUrls
-  "Optional. If this isn't provided, a fallback implementation that calculates URL percentage in Clojure-land will be used instead.
-   If a driver doesn't implement this protocol, it *must* implement `ISyncDriverFieldValues`."
-  (field-percent-urls [this field]
-    "Return the percentage of non-nil values of textual FIELD that are valid URLs."))
-
-
 ;; ## sync-database! and sync-table!
 
 (defn sync-database!
   "Sync DATABASE and all its Tables and Fields."
   [driver database]
-  (log/info (color/blue (format "Syncing database %s..." (:name database))))
-
-  (let [active-table-names (active-table-names driver database)
-        table-name->id (sel :many :field->id [Table :name] :db_id (:id database) :active true)]
-    (assert (set? active-table-names) "active-table-names should return a set.")
-    (assert (every? string? active-table-names) "active-table-names should return the names of Tables as *strings*.")
-
-    ;; First, let's mark any Tables that are no longer active as such.
-    ;; These are ones that exist in table-name->id but not in active-table-names.
-    (log/debug "Marking inactive tables...")
-    (doseq [[table-name table-id] table-name->id]
-      (when-not (contains? active-table-names table-name)
-        (upd Table table-id :active false)
-        (log/info (format "Marked table %s.%s as inactive." (:name database) table-name))
-
-        ;; We need to mark driver Table's Fields as inactive so we don't expose them in UI such as FK selector (etc.) This can happen in the background
-        (future (k/update Field
-                          (k/where {:table_id table-id})
-                          (k/set-fields {:active false})))))
-
-    ;; Next, we'll create new Tables (ones that came back in active-table-names but *not* in table-name->id)
-    (log/debug "Creating new tables...")
-    (let [existing-table-names (set (keys table-name->id))]
-      (doseq [active-table-name active-table-names]
-        (when-not (contains? existing-table-names active-table-name)
-          (ins Table :db_id (:id database), :active true, :name active-table-name)
-          (log/info (format "Found new table: %s.%s" (:name database) active-table-name))))))
-
-  ;; Now sync the active tables
-  (log/debug "Syncing active tables...")
-  (->> (sel :many Table :db_id (:id database) :active true)
-       (map #(assoc % :db (delay database))) ; replace default delays with ones that reuse database (and don't require a DB call)
-       (sync-database-active-tables! driver))
-
-  (log/info (color/blue (format "Finished syncing database %s." (:name database)))))
+  (sync-in-context driver database
+    (fn []
+      (log/info (color/blue (format "Syncing database %s..." (:name database))))
+
+      (let [active-table-names (active-table-names driver database)
+            table-name->id (sel :many :field->id [Table :name] :db_id (:id database) :active true)]
+        (assert (set? active-table-names) "active-table-names should return a set.")
+        (assert (every? string? active-table-names) "active-table-names should return the names of Tables as *strings*.")
+
+        ;; First, let's mark any Tables that are no longer active as such.
+        ;; These are ones that exist in table-name->id but not in active-table-names.
+        (log/debug "Marking inactive tables...")
+        (doseq [[table-name table-id] table-name->id]
+          (when-not (contains? active-table-names table-name)
+            (upd Table table-id :active false)
+            (log/info (format "Marked table %s.%s as inactive." (:name database) table-name))
+
+            ;; We need to mark driver Table's Fields as inactive so we don't expose them in UI such as FK selector (etc.) This can happen in the background
+            (future (k/update Field
+                              (k/where {:table_id table-id})
+                              (k/set-fields {:active false})))))
+
+        ;; Next, we'll create new Tables (ones that came back in active-table-names but *not* in table-name->id)
+        (log/debug "Creating new tables...")
+        (let [existing-table-names (set (keys table-name->id))]
+          (doseq [active-table-name active-table-names]
+            (when-not (contains? existing-table-names active-table-name)
+              (ins Table :db_id (:id database), :active true, :name active-table-name)
+              (log/info (format "Found new table: %s.%s" (:name database) active-table-name))))))
+
+      ;; Now sync the active tables
+      (log/debug "Syncing active tables...")
+      (->> (sel :many Table :db_id (:id database) :active true)
+           (map #(assoc % :db (delay database))) ; replace default delays with ones that reuse database (and don't require a DB call)
+           (sync-database-active-tables! driver))
+
+      (log/info (color/blue (format "Finished syncing database %s." (:name database)))))))
 
 (defn sync-table!
   "Sync a *single* TABLE by running all the sync steps for it.
    This is used *instead* of `sync-database!` when syncing just one Table is desirable."
   [driver table]
-  (sync-database-active-tables! driver [table]))
+  (let [database @(:db table)]
+    (sync-in-context driver database
+      (fn []
+        (sync-database-active-tables! driver [table])))))
 
 
 ;; ### sync-database-active-tables! -- runs the sync-table steps over sequence of Tables

src/metabase/driver/sync/queries.clj

@@ -6,10 +6,10 @@
 (defn- qp-query [table query-dict]
   (binding [context/*table* table
             context/*database* @(:db table)]
-    (driver/driver-process-query {:database (:db_id table)
-                                  :type "query"
-                                  :query (assoc query-dict
-                                                :source_table (:id table))})))
+    (driver/process-query {:database (:db_id table)
+                           :type "query"
+                           :query (assoc query-dict
+                                         :source_table (:id table))})))
 
 (defn table-row-count
   "Fetch the row count of TABLE via the query processor."

test/metabase/driver/generic_sql/connection_test.clj

 (ns metabase.driver.generic-sql.connection-test
   (:require [expectations :refer :all]
             [metabase.db :refer [sel]]
+            [metabase.driver :as driver]
             [metabase.driver.generic-sql.connection :refer :all]
             [metabase.models.database :refer [Database]]
             [metabase.test-data :refer :all]))
@@ -9,18 +10,18 @@
 
 ;; Check that we can connect to the Test DB
 (expect true
-  (can-connect? @test-db))
+  (driver/can-connect? @test-db))
 
 ;; Lie and say Test DB is Postgres. CAN-CONNECT? should fail
 (expect false
-  (can-connect? (assoc @test-db :engine :postgres)))
+  (driver/can-connect? (assoc @test-db :engine :postgres)))
 
 ;; Random made-up DBs should fail
 (expect false
-  (can-connect? {:engine :postgres
+  (driver/can-connect? {:engine :postgres
                  :details {:conn_str "host=localhost port=5432 dbname=ABCDEFGHIJKLMNOP user=rasta"}}))
 
 ;; Things that you can connect to, but are not DBs, should fail
 (expect false
-  (can-connect? {:engine :postgres
+  (driver/can-connect? {:engine :postgres
                  :details {:conn_str "host=google.com port=80"}}))

test/metabase/test_data/load.clj

@@ -54,7 +54,7 @@
             (log/info "Adding foreign key constraints...")
             (add-foreign-key-constraints!)
             (log/info "Syncing database...")
-            (driver/sync-database db)
+            (driver/sync-database! db)
             (log/info "Adding Schema Metadata...")
             (add-metadata!)
             (log/info "Finished. Enjoy your test data <3")