more unit tests (plus a couple fixes) for sync-dynamic functions.

src/metabase/models/table.clj

@@ -83,6 +83,20 @@
   (db/sel :one :field [Table :db_id] :id table-id))
 
 
+(defn retire-tables
+  "Retire all `Tables` in the list of TABLE-IDs along with all of each tables `Fields`."
+  [table-ids]
+  {:pre [(set? table-ids)
+         (every? integer? table-ids)]}
+  ;; retire the tables
+  (k/update Table
+    (k/where {:id [in table-ids]})
+    (k/set-fields {:active false}))
+  ;; retire the fields of retired tables
+  (k/update Field
+    (k/where {:table_id [in table-ids]})
+    (k/set-fields {:visibility_type "retired"})))
+
 (defn update-table
   "Update `Table` with the data from TABLE-DEF."
   [{:keys [id display_name], :as existing-table} {table-name :name}]

src/metabase/sync_database/sync.clj

@@ -111,14 +111,7 @@
                                          (k/where {:db_id database-id
                                                    :active true
                                                    :raw_table.active false}))))]
-    ;; retire the tables
-    (k/update table/Table
-      (k/where {:id [in tables-to-remove]})
-      (k/set-fields {:active false}))
-    ;; retire the fields of retired tables
-    (k/update field/Field
-      (k/where {:table_id [in tables-to-remove]})
-      (k/set-fields {:visibility_type "retired"}))))
+    (table/retire-tables tables-to-remove)))
 
 
 (defn update-data-models-for-table!

src/metabase/sync_database/sync_dynamic.clj

@@ -60,13 +60,17 @@
 
 (defn scan-table-and-update-data-model!
   "Update the working `Table` and `Field` metadata for the given `Table`."
-  [driver database {raw-table-id :raw_table_id, :as existing-table}]
+  [driver database {raw-table-id :raw_table_id, table-id :id, :as existing-table}]
   (when-let [raw-tbl (db/sel :one raw-table/RawTable :id raw-table-id)]
     (try
-      (let [table-def (u/prog1 (driver/describe-table driver database (select-keys existing-table [:name :schema]))
-                        (schema/validate i/DescribeTable <>))]
-        (-> (table/update-table existing-table raw-tbl)
-            (save-table-fields! (:fields table-def))))
+      (if-not (:active raw-tbl)
+        ;; looks like table was deactivated, so lets retire this Table
+        (table/retire-tables #{table-id})
+        ;; otherwise we ask the driver for an updated table description and save that info
+        (let [table-def (u/prog1 (driver/describe-table driver database (select-keys existing-table [:name :schema]))
+                          (schema/validate i/DescribeTable <>))]
+          (-> (table/update-table existing-table raw-tbl)
+              (save-table-fields! (:fields table-def)))))
       ;; NOTE: dynamic schemas don't have FKs
       (catch Throwable t
         (log/error (u/format-color 'red "Unexpected error scanning table") t)))))

test/metabase/mock/moviedb.clj

 (ns metabase.mock.moviedb
+  "A simple relational schema based mocked for testing.  4 tables w/ some FKs."
   (:require [metabase.driver :as driver]))
 
 

test/metabase/mock/toucanery.clj

+(ns metabase.mock.toucanery
+  "A document style database mocked for testing.
+   This is a `:dynamic-schema` db with `:nested-fields`.
+   Most notably meant to serve as a representation of a Mongo database."
+  (:require [metabase.driver :as driver]))
+
+
+(def ^:const toucanery-tables
+  "Docstring"
+  {"transactions"  {:name   "transactions"
+                    :schema nil
+                    :fields #{{:name          "id"
+                               :pk?           true
+                               :base-type     :IntegerField}
+                              {:name          "ts"
+                               :base-type     :BigIntegerField
+                               :special-type  :timestamp_milliseconds}
+                              {:name          "toucan"
+                               :base-type     :DictionaryField
+                               :nested-fields #{{:name          "name"
+                                                 :base-type     :TextField}
+                                                {:name          "details"
+                                                 :base-type     :DictionaryField
+                                                 :nested-fields #{{:name         "age"
+                                                                   :base-type    :IntegerField}
+                                                                  {:name         "weight"
+                                                                   :special-type :category
+                                                                   :base-type    :DecimalField}}}}}
+                              {:name          "buyer"
+                               :base-type     :DictionaryField
+                               :nested-fields #{{:name      "name"
+                                                 :base-type :TextField}
+                                                {:name      "cc"
+                                                 :base-type :TextField}}}}}
+   "employees"     {:name   "employees"
+                    :schema nil
+                    :fields #{{:name      "id"
+                               :base-type :IntegerField}
+                              {:name      "name"
+                               :base-type :TextField}}}})
+
+(defrecord ToucaneryDriver []
+  clojure.lang.Named
+  (getName [_] "ToucaneryDriver"))
+
+(extend ToucaneryDriver
+  driver/IDriver
+  (merge driver/IDriverDefaultsMixin
+         {:analyze-table       (constantly nil)
+          :describe-database   (fn [_ {:keys [exclude-tables]}]
+                                 (let [tables (for [table (vals toucanery-tables)
+                                                    :when (not (contains? exclude-tables (:name table)))]
+                                                (select-keys table [:schema :name]))]
+                                   {:tables (set tables)}))
+          :describe-table      (fn [_ _ table]
+                                 (get toucanery-tables (:name table)))
+          :features            (constantly #{:dynamic-schema :nested-fields})
+          :details-fields      (constantly [])
+          :table-rows-seq      (constantly [{:keypath "movies.filming.description", :value "If the movie is currently being filmed."}
+                                            {:keypath "movies.description", :value "A cinematic adventure."}])}))
+
+(driver/register-driver! :toucanery (ToucaneryDriver.))
+
+
+(def ^:const toucanery-raw-tables-and-columns
+  [{:schema nil
+    :database_id true
+    :columns []
+    :name "employees"
+    :updated_at true
+    :details {}
+    :active true
+    :id true
+    :created_at true}
+   {:schema nil
+    :database_id true
+    :columns []
+    :name "transactions"
+    :updated_at true
+    :details {}
+    :active true
+    :id true
+    :created_at true}])
+
+(def ^:const toucanery-tables-and-fields
+  [{:description nil
+    :entity_type nil
+    :schema nil
+    :raw_table_id true
+    :name "employees"
+    :fields [{:description nil
+              :table_id true
+              :special_type :id
+              :name "id"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id false
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Id"
+              :created_at true
+              :base_type :IntegerField}
+             {:description nil
+              :table_id true
+              :special_type :name
+              :name "name"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id false
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Name"
+              :created_at true
+              :base_type :TextField}]
+    :rows nil
+    :updated_at true
+    :entity_name nil
+    :active true
+    :id true
+    :db_id true
+    :visibility_type nil
+    :display_name "Employees"
+    :created_at true}
+   {:description nil
+    :entity_type nil
+    :schema nil
+    :raw_table_id true
+    :name "transactions"
+    :fields [{:description nil
+              :table_id true
+              :special_type nil
+              :name "age"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id true
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Age"
+              :created_at true
+              :base_type :IntegerField}
+             {:description nil
+              :table_id true
+              :special_type nil
+              :name "buyer"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id false
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Buyer"
+              :created_at true
+              :base_type :DictionaryField}
+             {:description nil
+              :table_id true
+              :special_type nil
+              :name "cc"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id true
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Cc"
+              :created_at true
+              :base_type :TextField}
+             {:description nil
+              :table_id true
+              :special_type nil
+              :name "details"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id true
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Details"
+              :created_at true
+              :base_type :DictionaryField}
+             {:description nil
+              :table_id true
+              :special_type :id
+              :name "id"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id false
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Id"
+              :created_at true
+              :base_type :IntegerField}
+             {:description nil
+              :table_id true
+              :special_type :name
+              :name "name"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id true
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Name"
+              :created_at true
+              :base_type :TextField}
+             {:description nil
+              :table_id true
+              :special_type :name
+              :name "name"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id true
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Name"
+              :created_at true
+              :base_type :TextField}
+             {:description nil
+              :table_id true
+              :special_type nil
+              :name "toucan"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id false
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Toucan"
+              :created_at true
+              :base_type :DictionaryField}
+             {:description nil
+              :table_id true
+              :special_type :timestamp_milliseconds
+              :name "ts"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id false
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Ts"
+              :created_at true
+              :base_type :BigIntegerField}
+             {:description nil
+              :table_id true
+              :special_type :category
+              :name "weight"
+              :fk_target_field_id false
+              :updated_at true
+              :active true
+              :parent_id true
+              :id true
+              :raw_column_id false
+              :last_analyzed false
+              :field_type :info
+              :position 0
+              :visibility_type :normal
+              :preview_display true
+              :display_name "Weight"
+              :created_at true
+              :base_type :DecimalField}]
+    :rows nil
+    :updated_at true
+    :entity_name nil
+    :active true
+    :id true
+    :db_id true
+    :visibility_type nil
+    :display_name "Transactions"
+    :created_at true}])

test/metabase/sync_database/sync_dynamic_test.clj

@@ -2,16 +2,24 @@
   (:require [expectations :refer :all]
             [korma.core :as k]
             [metabase.db :as db]
+            [metabase.mock.toucanery :as toucanery]
             [metabase.models.database :as database]
             [metabase.models.field :as field]
+            [metabase.models.hydrate :as hydrate]
             [metabase.models.raw-table :as raw-table]
             [metabase.models.table :as table]
+            [metabase.sync-database.introspect :as introspect]
             [metabase.sync-database.sync-dynamic :refer :all]
             [metabase.test.util :as tu]))
 
 (tu/resolve-private-fns metabase.sync-database.sync-dynamic
   save-table-fields!)
 
+(defn- get-tables [database-id]
+  (->> (hydrate/hydrate (db/sel :many table/Table :db_id database-id (k/order :id)) :fields)
+       (mapv tu/boolean-ids-and-timestamps)))
+
+
 ;; save-table-fields!  (also covers save-nested-fields!)
 (expect
   [[]
@@ -273,7 +281,70 @@
          (get-fields))])))
 
 
-;; TODO: update-data-models-for-table!
+;; scan-table-and-update-data-model!
+(expect
+  [[(last toucanery/toucanery-tables-and-fields)]
+   [(last toucanery/toucanery-tables-and-fields)]
+   [(-> (last toucanery/toucanery-tables-and-fields)
+       (assoc :active false
+              :fields []))]]
+  (tu/with-temp* [database/Database [{database-id :id, :as db} {:engine :toucanery}]]
+    (let [driver (toucanery/->ToucaneryDriver)]
+      ;; do a quick introspection to add the RawTables to the db
+      (introspect/introspect-database-and-update-raw-tables! driver db)
+      ;; stub out the Table we are going to sync for real below
+      (let [raw-table-id (db/sel :one :field [raw-table/RawTable :id] :database_id database-id, :name "transactions")
+            tbl          (db/ins table/Table
+                           :db_id        database-id
+                           :raw_table_id raw-table-id
+                           :name         "transactions"
+                           :active       true)]
+        [;; now lets run a sync and check what we got
+         (do
+           (scan-table-and-update-data-model! driver db tbl)
+           (get-tables database-id))
+         ;; run the sync a second time to see how we respond to repeat syncing (should be same since nothing changed)
+         (do
+           (scan-table-and-update-data-model! driver db tbl)
+           (get-tables database-id))
+         ;; one more time, but lets disable the table this time and ensure that's handled properly
+         (do
+           (k/update raw-table/RawTable
+             (k/set-fields {:active false})
+             (k/where {:database_id database-id, :name "transactions"}))
+           (scan-table-and-update-data-model! driver db tbl)
+           (get-tables database-id))]))))
 
-;; TODO: update-data-models-from-raw-tables!
-;; make sure to test case where FK relationship tables are out of order
+
+;; scan-database-and-update-data-model!
+(expect
+  [toucanery/toucanery-raw-tables-and-columns
+   toucanery/toucanery-tables-and-fields
+   toucanery/toucanery-tables-and-fields
+   (conj (vec (drop-last toucanery/toucanery-tables-and-fields))
+         (-> (last toucanery/toucanery-tables-and-fields)
+             (assoc :active false
+                    :fields [])))]
+  (tu/with-temp* [database/Database [{database-id :id, :as db} {:engine :toucanery}]]
+    (let [driver (toucanery/->ToucaneryDriver)]
+      ;; do a quick introspection to add the RawTables to the db
+      (introspect/introspect-database-and-update-raw-tables! driver db)
+
+      [;; first check that the raw tables stack up as expected, especially that fields were skipped because this is a :dynamic-schema db
+       (->> (hydrate/hydrate (db/sel :many raw-table/RawTable :database_id database-id (k/order :id)) :columns)
+            (mapv tu/boolean-ids-and-timestamps))
+       ;; now lets run a sync and check what we got
+       (do
+         (scan-database-and-update-data-model! driver db)
+         (get-tables database-id))
+       ;; run the sync a second time to see how we respond to repeat syncing (should be same since nothing changed)
+       (do
+         (scan-database-and-update-data-model! driver db)
+         (get-tables database-id))
+       ;; one more time, but lets disable a table this time and ensure that's handled properly
+       (do
+         (k/update raw-table/RawTable
+           (k/set-fields {:active false})
+           (k/where {:database_id database-id, :name "transactions"}))
+         (scan-database-and-update-data-model! driver db)
+         (get-tables database-id))])))