rewrite mongo to use new dataset loader interface

src/metabase/driver/mongo/util.clj

@@ -33,7 +33,8 @@
   [f database]
   (let [connection-string (cond
                             (string? database)              database
-                            (:dbname (:details database))   (details-map->connection-string (:details database)) ; new-style
+                            (:dbname (:details database))   (details-map->connection-string (:details database)) ; new-style -- entire Database obj
+                            (:dbname database)              (details-map->connection-string database)            ; new-style -- connection details map only
                             (:conn_str (:details database)) (:conn_str (:details database))                      ; legacy
                             :else                           (throw (Exception. (str "with-mongo-connection failed: bad connection details:" (:details database)))))
         {conn :conn mongo-connection :db} (mg/connect-via-uri connection-string)]
@@ -55,7 +56,9 @@
 
      ;; You can use a string instead of a Database
      (with-mongo-connection [^com.mongodb.DBApiLayer conn \"mongodb://127.0.0.1:27017/test\"]
-        ...)"
+        ...)
+
+   DATABASE-OR-CONNECTION-STRING can also optionally be the connection details map on its own."
   [[binding database] & body]
   `(let [f# (fn [~binding]
               ~@body)]

test/metabase/api/meta/db_test.clj

@@ -123,7 +123,7 @@
                   :id $
                   :details $
                   :updated_at $
-                  :name "Mongo Test"
+                  :name "Test Database"
                   :organization_id nil
                   :description nil}))
              (match-$ @test-db

test/metabase/api/meta/table_test.clj

@@ -9,9 +9,9 @@
                              [foreign-key :refer [ForeignKey]]
                              [table :refer [Table]])
             [metabase.test.data :refer :all]
-            (metabase.test.data [datasets :as datasets, :refer [*dataset* with-dataset-when-testing]]
+            (metabase.test.data [data :as data]
+                                [datasets :as datasets, :refer [*dataset* with-dataset-when-testing]]
                                 [users :refer :all])
-            [metabase.test-data.data :as data]
             [metabase.test.util :refer [match-$ expect-eval-actual-first]]))
 
 
@@ -154,7 +154,8 @@
                               (+ (.getMonth inst) 1)
                               (.getDate inst)))]
     (->> data/test-data
-         :users
+         :table-definitions
+         first
          :rows
          (map second)
          (map format-inst)

test/metabase/driver/mongo/test_data.clj

 (ns metabase.driver.mongo.test-data
   "Functionality related to creating / loading a test database for the Mongo driver."
-  (:require [clojure.tools.logging :as log]
-            [colorize.core :as color]
-            [medley.core :as m]
-            (monger [collection :as mc]
-                    [core :as mg])
+  (:require [medley.core :as m]
             [metabase.db :refer :all]
-            [metabase.driver :as driver]
-            [metabase.driver.mongo.util :refer [with-mongo-connection]]
             (metabase.models [database :refer [Database]]
                              [field :refer [Field]]
                              [table :refer [Table]])
-            [metabase.test-data.data :as data]))
-
-(declare load-data
-         set-field-types!)
-
-;; ## CONSTANTS
-
-(def ^:private ^:const mongo-test-db-conn-str
-  "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
-  "Name of the Mongo test database."
-  "Mongo Test")
-
+            [metabase.test.data :refer [get-or-create-database!]]
+            (metabase.test.data [data :as data]
+                                [mongo :as loader])))
 
 ;; ## MONGO-TEST-DB + OTHER DELAYS
 
-(defn destroy!
-  "Remove `Database`, `Tables`, and `Fields` for the Mongo test DB."
-  []
-  #_(cascade-delete Database :name mongo-test-db-name))
-
 (defonce
   ^{:doc "A delay that fetches or creates the Mongo test `Database`.
           If DB is created, `load-data` and `sync-database!` are called to get the DB in a state that we can use for testing."}
   mongo-test-db
-  (delay
-   ;; Resolve metabase.test.data.datasets at runtime to avoid circular dependency
-   (require 'metabase.test.data.datasets)
-   (assert (contains? @@(ns-resolve 'metabase.test.data.datasets 'test-dataset-names) :mongo)
-           "Why are we attempting to use the Mongo test Database when we're not testing against mongo?")
-   (let [db (or (sel :one Database :name mongo-test-db-name)
-                (let [db (ins Database
-                           :name mongo-test-db-name
-                           :engine :mongo
-                           :details {:conn_str mongo-test-db-conn-str})]
-                  (log/info (color/cyan "Loading Mongo test data..."))
-                  (load-data)
-                  (driver/sync-database! db)
-                  (set-field-types!)
-                  (log/info (color/cyan "Done."))
-                  db))]
-     (assert (and (map? db)
-                  (integer? (:id db))
-                  (exists? Database :id (:id db))))
-     db)))
+  (delay (get-or-create-database! (loader/dataset-loader) data/test-data)))
 
 (defonce
   ^{:doc "A Delay that returns the ID of `mongo-test-db`, forcing creation of it if needed."}
@@ -68,6 +26,7 @@
 
 
 ;; ## FNS FOR GETTING RELEVANT TABLES / FIELDS
+;; TODO - This seems like it's duplicated a bit with the functions in metabase.test.data
 
 (defn table-name->table
   "Fetch `Table` for Mongo test database.
@@ -115,35 +74,3 @@
             (keyword? field-name)]
       :post [(integer? %)]}
      (:id (field-name->field table-name field-name)))))
-
-
-;; ## LOADING STUFF
-
-(defn- load-data
-  "Load the data for the Mongo test database. This can safely be called multiple times; duplicate documents will *not* be inserted."
-  []
-  (with-mongo-connection [mongo-db @mongo-test-db]
-    (doseq [[collection {fields :fields rows :rows}] data/test-data]
-      (let [fields (map :name fields)
-            rows (map-indexed (partial vector) rows)]
-        (doseq [[i row] rows]
-          (try
-            (mc/insert mongo-db (name collection) (assoc (zipmap fields row)
-                                                         :_id (inc i)))
-            (catch com.mongodb.MongoException$DuplicateKey _)))
-        (log/info (color/cyan (format "Loaded data for collection '%s'." (name collection))))))))
-
-(defn- set-field-types! []
-  (doseq [[collection-name {fields :fields}] data/test-data]
-    (doseq [{:keys [special-type field-type] :as field} fields]
-      (when (or field-type special-type)
-        (let [table-id (sel :one :id Table :name (name collection-name))
-              _        (assert (integer? table-id))
-              field-id (sel :one :id Field :table_id table-id :name (name (:name field)))
-              _        (assert (integer? table-id))]
-          (when special-type
-            (log/info (format "SET SPECIAL TYPE %s.%s -> %s..." collection-name (:name field) special-type))
-            (upd Field field-id :special_type special-type))
-          (when field-type
-            (log/info (format "SET FIELD TYPE %s.%s -> %s..." collection-name (:name field) field-type))
-            (upd Field field-id :field_type field-type)))))))

test/metabase/driver/mongo_test.clj

@@ -9,8 +9,8 @@
             [metabase.driver.mongo.test-data :refer :all]
             (metabase.models [field :refer [Field]]
                              [table :refer [Table]])
-            [metabase.test-data.data :refer [test-data]]
-            [metabase.test.data.datasets :as datasets]
+            (metabase.test.data [data :refer [test-data]]
+                                [datasets :as datasets])
             [metabase.test.util :refer [expect-eval-actual-first resolve-private-fns]]))
 
 ;; ## Logic for selectively running mongo

test/metabase/test/data.clj

@@ -2,6 +2,7 @@
   "Code related to creating and deleting test databases + datasets."
   (:require [clojure.string :as s]
             [clojure.tools.logging :as log]
+            [colorize.core :as color]
             [metabase.db :refer :all]
             [metabase.driver :as driver]
             (metabase.models [database :refer [Database]]
@@ -25,28 +26,28 @@
     (or (metabase-instance database-definition engine)
         (do
           ;; Create the database
-          (log/info (format "Creating %s Database %s..." (name engine) database-name))
+          (log/info (color/blue (format "Creating %s database %s..." (name engine) database-name)))
           (create-physical-db! dataset-loader database-definition)
 
           ;; Load data
-          (log/info "Loading data...")
+          (log/info (color/blue "Loading data..."))
           (doseq [^TableDefinition table-definition (:table-definitions database-definition)]
-            (log/info (format "Loading data for Table %s..." (:table-name table-definition)))
+            (log/info (color/blue (format "Loading data for table '%s'..." (:table-name table-definition))))
             (load-table-data! dataset-loader database-definition table-definition))
 
           ;; Add DB object to Metabase DB
-          (log/info "Adding DB to Metabase...")
+          (log/info (color/blue "Adding DB to Metabase..."))
           (let [db (ins Database
                      :name    database-name
                      :engine  (name engine)
                      :details (database->connection-details dataset-loader database-definition))]
 
             ;; Sync the database
-            (log/info "Syncing DB...")
+            (log/info (color/blue "Syncing DB..."))
             (driver/sync-database! db)
 
             ;; Add extra metadata like Field field-type, base-type, etc.
-            (log/info "Adding schema metadata...")
+            (log/info (color/blue "Adding schema metadata..."))
             (doseq [^TableDefinition table-definition (:table-definitions database-definition)]
               (let [table-name (:table-name table-definition)
                     table      (delay (let [table (metabase-instance table-definition db)]
@@ -63,7 +64,7 @@
                       (log/info (format "SET SPECIAL TYPE %s.%s -> %s" table-name field-name special-type))
                       (upd Field (:id @field) :special_type (name special-type)))))))
 
-            (log/info "Finished.")
+            (log/info (color/blue "Finished."))
             db)))))
 
 (defn remove-database!

test/metabase/test/data/datasets.clj

@@ -40,7 +40,6 @@
 (deftype MongoDriverData []
   IDataset
   (load-data! [_]
-    (mongo-data/destroy!)
     @mongo-data/mongo-test-db
     (assert (integer? @mongo-data/mongo-test-db-id)))
   (db [_]

test/metabase/test/data/interface.clj

 (ns metabase.test.data.interface
+  "`Definition` types for databases, tables, fields; related protocols, helper functions.
+
+   Objects that implement `IDatasetLoader` know how to load a `DatabaseDefinition` into an
+   actual physical RDMS database. This functionality allows us to easily test with multiple datasets."
   (:require [clojure.string :as s]
             [metabase.db :refer :all]
             (metabase.models [database :refer [Database]]
@@ -6,10 +10,6 @@
                              [table :refer [Table]]))
   (:import clojure.lang.Keyword))
 
-(defprotocol IEscapedName
-  (^String escaped-name [this]
-    "Return escaped version of DATABASE-NAME suitable for use as a filename / database name / etc."))
-
 (defrecord FieldDefinition [^String  field-name
                             ^Keyword base-type
                             ^Keyword field-type
@@ -21,10 +21,17 @@
                             rows])
 
 (defrecord DatabaseDefinition [^String database-name
-                               table-definitions]
-  IEscapedName
-  (escaped-name [_]
-    (s/replace database-name #"\s+" "_")))
+                               table-definitions])
+
+
+(defprotocol IEscapedName
+  (^String escaped-name [this]
+    "Return escaped version of DATABASE-NAME suitable for use as a filename / database name / etc."))
+
+(extend-protocol IEscapedName
+  DatabaseDefinition
+  (escaped-name [this]
+    (s/replace (:database-name this) #"\s+" "_")))
 
 
 (defprotocol IMetabaseInstance

test/metabase/test/data/mongo.clj

+(ns metabase.test.data.mongo
+  "MongoDB Dataset Loader."
+  (:require (monger [collection :as mc]
+                    [core :as mg])
+            [metabase.driver.mongo.util :refer [with-mongo-connection]]
+            [metabase.test.data.interface :refer :all])
+  (:import (metabase.test.data.interface DatabaseDefinition
+                                         FieldDefinition
+                                         TableDefinition)))
+
+(defrecord MongoDatasetLoader [])
+(extend-protocol IDatasetLoader
+  MongoDatasetLoader
+  (engine [_]
+    :mongo)
+
+  (database->connection-details [_ database-definition]
+    {:dbname (escaped-name database-definition)
+     :host   "localhost"})
+
+  ;; Nothing to do here ! DB created when we connect to it
+  (create-physical-db! [_ _])
+
+  (drop-physical-db! [this database-definition]
+    (mg/drop-db (mg/connect (database->connection-details this database-definition))
+                (escaped-name database-definition)))
+
+  ;; Nothing to do here, collection is created when we add documents to it
+  (create-physical-table! [_ _ _])
+
+  (drop-physical-table! [this database-definition {:keys [table-name]}]
+    (with-mongo-connection [^com.mongodb.DBApiLayer mongo-db (database->connection-details this database-definition)]
+      (mc/drop mongo-db (name table-name))))
+
+  (load-table-data! [this database-definition {:keys [field-definitions table-name rows]}]
+    (with-mongo-connection [^com.mongodb.DBApiLayer mongo-db (database->connection-details this database-definition)]
+      (let [field-names (->> field-definitions
+                             (map :field-name)
+                             (map keyword))]
+        ;; Use map-indexed so we can get an ID for each row (index + 1)
+        (doseq [[i row] (map-indexed (partial vector) rows)]
+          (try
+            ;; Insert each row
+            (mc/insert mongo-db (name table-name) (assoc (zipmap field-names row)
+                                                         :_id (inc i)))
+            ;; If row already exists then nothing to do
+            (catch com.mongodb.MongoException$DuplicateKey _)))))))
+
+(defn ^MongoDatasetLoader dataset-loader []
+  (->MongoDatasetLoader))