unit tests for Mongo driver syncing + connection

.dir-locals.el

@@ -19,6 +19,7 @@
                               (expect-eval-actual-first 1)
                               (expect-expansion 0)
                               (expect-let 1)
+                              (expect-with-data-loaded 1)
                               (ins 1)
                               (let-400 1)
                               (let-404 1)

src/metabase/driver.clj

@@ -41,6 +41,7 @@
    java.math.BigInteger         :BigIntegerField
    java.sql.Date                :DateField
    java.sql.Timestamp           :DateTimeField
+   java.util.Date               :DateField
    org.postgresql.util.PGobject :UnknownField}) ; this mapping included here since Native QP uses class->base-type directly. TODO - perhaps make *class-base->type* driver specific?
 
 ;; ## Driver Lookup
@@ -84,6 +85,7 @@
 (defn can-connect?
   "Check whether we can connect to DATABASE and perform a basic query (such as `SELECT 1`)."
   [database]
+  {:pre [(map? database)]}
   (try
     (i/can-connect? (engine->driver (:engine database)) database)
     (catch Throwable e
@@ -95,6 +97,9 @@
 
      (can-connect-with-details? :postgres {:host \"localhost\", :port 5432, ...})"
   [engine details-map]
+  {:pre [(keyword? engine)
+         (contains? (set (keys available-drivers)) engine)
+         (map? details-map)]}
   (try
     (i/can-connect-with-details? (engine->driver engine) details-map)
     (catch Throwable e
@@ -105,17 +110,20 @@
   "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]
+      {:pre [(map? database)]}
       (time (-sync-database! (engine->driver (:engine database)) database)))))
 
 (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]
+      {:pre [(map? table)]}
       (-sync-table! (database-id->driver (:db_id table)) table))))
 
 (defn process-query
   "Process a structured or native query, and return the result."
   [query]
+  {:pre [(map? query)]}
   (binding [qp/*query* query]
     (i/process-query (database-id->driver (:database query))
                      (qp/preprocess query))))

src/metabase/driver/mongo.clj

@@ -60,6 +60,8 @@
          1.0)))
 
   (can-connect-with-details? [this {:keys [user password host port dbname]}]
+    (assert (and host
+                 dbname))
     (can-connect? this (str "mongodb://"
                             user
                             (when password
@@ -86,7 +88,7 @@
       (-> (mdb/get-collection-names conn)
           (set/difference #{"system.indexes"}))))
 
-  (active-column-names->type [this table]
+  (active-column-names->type [_ table]
     (with-mongo-connection [_ @(:db table)]
       (->> (table->column-names table)
            (map (fn [column-name]
@@ -102,9 +104,10 @@
   (field-values-lazy-seq [_ field]
     (lazy-seq
      (let [table @(:table field)]
-       (with-mongo-connection [conn @(:db table)]
-         (mq/with-collection conn (:name table)
-           (mq/fields [(:name field)])))))))
+       (map (keyword (:name field))
+            (with-mongo-connection [^com.mongodb.DBApiLayer conn @(:db table)]
+              (mq/with-collection conn (:name table)
+                (mq/fields [(:name field)]))))))))
 
 (def ^:const driver
   "Concrete instance of the MongoDB driver."

src/metabase/driver/mongo/util.clj

@@ -29,10 +29,12 @@
    The DB connection is re-used by subsequent calls to `with-mongo-connection` within BODY.
    (We're smart about it: DATABASE isn't even evaluated if `*mongo-connection*` is already bound.)
 
-     (with-mongo-connection [conn @(:db (sel :one Table ...))] ; delay isn't derefed if *mongo-connection* is already bound
+     ;; delay isn't derefed if *mongo-connection* is already bound
+     (with-mongo-connection [^com.mongodb.DBApiLayer conn @(:db (sel :one Table ...))]
        ...)
 
-     (with-mongo-connection [conn \"mongodb://127.0.0.1:27017/test\"] ; use a string instead of a DB
+     ;; You can use a string instead of a Database
+     (with-mongo-connection [^com.mongodb.DBApiLayer conn \"mongodb://127.0.0.1:27017/test\"]
         ...)"
   [[binding database] & body]
   `(let [f# (fn [~binding]

test/metabase/driver/mongo/test_data.clj

@@ -18,7 +18,7 @@
 ;; ## CONSTANTS
 
 (def ^:private ^:const mongo-test-db-conn-str
-  "Connection string for the Metabase Mongo Test DB." ; TODO - this needs to come from env var or something so we can get tests working in CircleCI too
+  "Connection string for the Metabase Mongo Test DB." ; TODO - does this need to come from an env var so it works in CircleCI?
   "mongodb://localhost/metabase-test")
 
 (def ^:private ^:const mongo-test-db-name
@@ -48,26 +48,50 @@
 
 ;; ## FNS FOR GETTING RELEVANT TABLES / FIELDS
 
-(def ^{:arglists '([table-name])} table-name->table
-  "Return Mongo test database `Table` with given name.
-
-     (table-name->table :users)
-       -> {:id 10, :name \"users\", ...}"
-  (let [-table-name->table (delay (m/map-keys keyword (sel :many :field->obj [Table :name] :db_id @mongo-test-db-id)))]
-    (fn [table-name]
-      {:pre [(or (keyword? table-name)
-                 (string? table-name))]
-       :post [(map? %)]}
-      (@-table-name->table (keyword table-name)))))
-
-(defn table-name->id [table-name]
-  {:pre [(or (keyword? table-name)
-             (string? table-name))]
-   :post [(integer? %)]}
-  (:id (table-name->table (keyword table-name))))
-
-(defn table-name->field-name->field [table-name]
-  (let [table (table-name->table table-name)]))
+(defn table-name->table
+  "Fetch `Table` for Mongo test database.
+
+    (table-name->table :users) -> {:id 100, :name \"users\", ...}"
+  [table-name]
+  {:pre [(keyword? table-name)]
+   :post [(map? %)]}
+  (sel :one Table :db_id @mongo-test-db-id :name (name table-name)))
+
+(def ^{:arglists '([table-name])} table-name->id
+  "Return ID of `Table` for Mongo test database (memoized).
+
+    (table-name->id :users) -> 10"
+  (memoize
+   (fn [table-name]
+     {:pre [(keyword? table-name)]
+      :post [(integer? %)]}
+     (:id (table-name->table table-name)))))
+
+(defn table-name->field-name->field
+  "Return a map of `field-name -> field` for `Table` for Mongo test database."
+  [table-name]
+  (m/map-keys keyword (sel :many :field->obj [Field :name] :table_id (table-name->id table-name))))
+
+(defn field-name->field
+  "Fetch `Field` for Mongo test database.
+
+    (field-name->field :users :name) -> {:id 292, :name \"name\", ...}"
+  [table-name field-name]
+  {:pre [(keyword? table-name)
+         (keyword? field-name)]
+   :post [(map? %)]}
+  ((table-name->field-name->field table-name) field-name))
+
+(def ^{:arglists '([table-name field-name])} field-name->id
+  "Return ID of `Field` for Mongo test Database (memoized).
+
+    (field-name->id :users :name) -> 292"
+  (memoize
+   (fn [table-name field-name]
+     {:pre [(keyword? table-name)
+            (keyword? field-name)]
+      :post [(integer? %)]}
+     (:id (field-name->field table-name field-name)))))
 
 
 ;; ## LOADING STUFF

test/metabase/driver/mongo_test.clj

+(ns metabase.driver.mongo-test
+  "Tests for Mongo driver."
+  (:require [expectations :refer :all]
+            [korma.core :as k]
+            [metabase.db :refer :all]
+            [metabase.driver :as driver]
+            (metabase.driver [interface :as i]
+                             [mongo :as mongo])
+            [metabase.driver.mongo.test-data :refer :all]
+            (metabase.models [field :refer [Field]]
+                             [table :refer [Table]])
+            [metabase.test-data.data :refer [test-data]]
+            [metabase.test.util :refer [expect-eval-actual-first resolve-private-fns]]))
+
+;; ## Constants + Helper Fns/Macros
+;; TODO - move these to metabase.test-data ?
+(def ^:private ^:const table-names
+  "The names of the various test data `Tables`."
+  [:categories
+   :checkins
+   :users
+   :venues])
+
+(def ^:private ^:const field-names
+  "Names of various test data `Fields`, as `[table-name field-name]` pairs."
+  [[:categories :_id]
+   [:categories :name]
+   [:checkins :_id]
+   [:checkins :date]
+   [:checkins :user_id]
+   [:checkins :venue_id]
+   [:users :_id]
+   [:users :last_login]
+   [:users :name]
+   [:venues :_id]
+   [:venues :category_id]
+   [:venues :latitude]
+   [:venues :longitude]
+   [:venues :name]
+   [:venues :price]])
+
+(defmacro expect-with-data-loaded
+  "Like `expect`, but forces the test database to be loaded/synced/etc. before running the test."
+  [expected actual]
+  `(expect (do @mongo-test-db
+               ~expected)
+     ~actual))
+
+(defn- table-name->fake-table
+  "Return an object that can be passed like a `Table` to driver sync functions."
+  [table-name]
+  {:pre [(keyword? table-name)]}
+  {:db mongo-test-db
+   :name (name table-name)})
+
+(defn- field-name->fake-field
+  "Return an object that can be passed like a `Field` to driver sync functions."
+  [table-name field-name]
+  {:pre [(keyword? table-name)
+         (keyword? field-name)]}
+  {:name (name field-name)
+   :table (delay (table-name->fake-table table-name))})
+
+;; ## Tests for connection functions
+
+(expect true
+  (driver/can-connect? @mongo-test-db))
+
+(expect false
+  (driver/can-connect? {:engine :mongo
+                        :details {:conn_str "mongodb://123.4.5.6/bad-db-name?connectTimeoutMS=50"}})) ; timeout after 50ms instead of 10s so test's don't take forever
+
+(expect false
+  (driver/can-connect? {:engine :mongo
+                        :details {:conn_str "mongodb://localhost:3000/bad-db-name?connectTimeoutMS=50"}}))
+
+(expect false
+  (driver/can-connect-with-details? :mongo {}))
+
+(expect true
+  (driver/can-connect-with-details? :mongo {:host "localhost"
+                                            :port 27017
+                                            :dbname "metabase-test"}))
+
+;; should use default port 27017 if not specified
+(expect true
+  (driver/can-connect-with-details? :mongo {:host "localhost"
+                                            :dbname "metabase-test"}))
+
+(expect false
+  (driver/can-connect-with-details? :mongo {:host "123.4.5.6"
+                                            :dbname "bad-db-name?connectTimeoutMS=50"}))
+
+(expect false
+  (driver/can-connect-with-details? :mongo {:host "localhost"
+                                            :port 3000
+                                            :dbname "bad-db-name?connectTimeoutMS=50"}))
+
+
+;; ## Tests for individual syncing functions
+
+(resolve-private-fns metabase.driver.mongo field->base-type table->column-names)
+
+;; ### active-table-names
+(expect
+    #{"checkins" "categories" "users" "venues"}
+  (i/active-table-names mongo/driver @mongo-test-db))
+
+;; ### table->column-names
+(expect-with-data-loaded
+    [#{:_id :name}
+     #{:_id :date :venue_id :user_id}
+     #{:_id :name :last_login}
+     #{:_id :name :longitude :latitude :price :category_id}]
+  (->> table-names
+       (map table-name->fake-table)
+       (map table->column-names)))
+
+;; ### field->base-type
+(expect-with-data-loaded
+    [:IntegerField  ; categories._id
+     :TextField     ; categories.name
+     :IntegerField  ; checkins._id
+     :DateField     ; checkins.date
+     :IntegerField  ; checkins.user_id
+     :IntegerField  ; checkins.venue_id
+     :IntegerField  ; users._id
+     :DateField     ; users.last_login
+     :TextField     ; users.name
+     :IntegerField  ; venues._id
+     :IntegerField  ; venues.category_id
+     :FloatField    ; venues.latitude
+     :FloatField    ; venues.longitude
+     :TextField     ; venues.name
+     :IntegerField] ; venues.price
+  (->> field-names
+       (map (partial apply field-name->fake-field))
+       (mapv field->base-type)))
+
+;; ### active-column-names->type
+(expect
+    [{"_id" :IntegerField, "name" :TextField}
+     {"_id" :IntegerField, "date" :DateField, "venue_id" :IntegerField, "user_id" :IntegerField}
+     {"_id" :IntegerField, "name" :TextField, "last_login" :DateField}
+     {"_id" :IntegerField, "name" :TextField, "longitude" :FloatField, "latitude" :FloatField, "price" :IntegerField, "category_id" :IntegerField}]
+  (->> table-names
+       (map table-name->fake-table)
+       (mapv (partial i/active-column-names->type mongo/driver))))
+
+;; ### table-pks
+(expect
+    [#{"_id"} #{"_id"} #{"_id"} #{"_id"}] ; _id for every table
+  (->> table-names
+       (map table-name->fake-table)
+       (mapv (partial i/table-pks mongo/driver))))
+
+
+;; ## Big-picture tests for the way data should look post-sync
+
+;; Test that Tables got synced correctly, and row counts are correct
+(expect-with-data-loaded
+    [{:rows 75, :active true, :name "categories"}
+     {:rows 1000, :active true, :name "checkins"}
+     {:rows 15, :active true, :name "users"}
+     {:rows 100, :active true, :name "venues"}]
+  (sel :many :fields [Table :name :active :rows] :db_id @mongo-test-db-id (k/order :name)))
+
+;; Test that Fields got synced correctly, and types are correct
+(expect-with-data-loaded
+    [({:special_type :id, :base_type :IntegerField, :field_type :info, :active true, :name "_id"}
+      {:special_type :category, :base_type :DateField, :field_type :info, :active true, :name "last_login"}
+      {:special_type :category, :base_type :TextField, :field_type :info, :active true, :name "name"})
+     ({:special_type :id, :base_type :IntegerField, :field_type :info, :active true, :name "_id"}
+      {:special_type :category, :base_type :DateField, :field_type :info, :active true, :name "last_login"}
+      {:special_type :category, :base_type :TextField, :field_type :info, :active true, :name "name"})
+     ({:special_type :id, :base_type :IntegerField, :field_type :info, :active true, :name "_id"}
+      {:special_type :category, :base_type :DateField, :field_type :info, :active true, :name "last_login"}
+      {:special_type :category, :base_type :TextField, :field_type :info, :active true, :name "name"})
+     ({:special_type :id, :base_type :IntegerField, :field_type :info, :active true, :name "_id"}
+      {:special_type :category, :base_type :DateField, :field_type :info, :active true, :name "last_login"}
+      {:special_type :category, :base_type :TextField, :field_type :info, :active true, :name "name"})]
+  (let [table->fields (fn [table-name]
+                        (sel :many :fields [Field :name :active :field_type :base_type :special_type] :table_id (table-name->id :users) (k/order :name)))]
+    (mapv table->fields table-names)))