Serdes prep for toucan2 (#29785)

* makes sure serdes can find and handle toucan2 models

enterprise/backend/src/metabase_enterprise/serialization/v2/backfill_ids.clj

@@ -12,8 +12,8 @@
    [metabase.util :as u]
    [metabase.util.i18n :refer [trs]]
    [metabase.util.log :as log]
-   [toucan.models :as models]
-   [toucan2.core :as t2]))
+   [toucan2.core :as t2]
+   [toucan2.model :as t2.model]))
 
 (defn backfill-ids-for
   "Updates all rows of a particular model to have `:entity_id` set, based on the [[serdes/identity-hash]]."
@@ -27,8 +27,14 @@
                     eid    (u/generate-nano-id hashed)]]
         (t2/update! model (get entity pk) {:entity_id eid})))))
 
-(defn- has-entity-id? [model]
-  (::mi/entity-id (models/properties model)))
+(defn has-entity-id?
+  "Returns true if the model has an `:entity_id` column."
+  [model]
+  (or
+    ;; toucan1 models
+    (isa? model ::mi/entity-id)
+    ;; toucan2 models
+    (isa? model :hook/entity-id)))
 
 (defn backfill-ids
   "Updates all rows of all models that are (a) serialized and (b) have `entity_id` columns to have the
@@ -36,6 +42,6 @@
   row."
   []
   (doseq [model-name (concat serdes.models/exported-models serdes.models/inlined-models)
-          :let [model (mdb.u/resolve-model (symbol model-name))]
+          :let [model (t2.model/resolve-model (symbol model-name))]
           :when (has-entity-id? model)]
     (backfill-ids-for model)))

enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj

@@ -10,7 +10,6 @@
    [metabase.util.i18n :refer [trs]]
    [metabase.util.log :as log]
    [toucan.db :as db]
-   [toucan.models :as models]
    [toucan2.core :as t2]))
 
 (set! *warn-on-reflection* true)
@@ -31,16 +30,22 @@
                                                          (:mysql :postgres) "entity_id"))]
         (into #{} (map (comp u/lower-case-en :table_name)) (resultset-seq rset))))))
 
+(defn toucan-models
+  "Return a list of all toucan models."
+  []
+  (concat (descendants :toucan1/model) (descendants :metabase/model)))
+
 (defn- make-table-name->model
   "Create a map of (lower-cased) application DB table name -> corresponding Toucan model."
   []
-  (into {} (for [model (descendants :toucan1/model)
-                 :when (models/model? model)
-                 :let  [table-name (some-> model t2/table-name name)]
-                 :when table-name
-                 ;; ignore any models defined in test namespaces.
-                 :when (not (str/includes? (namespace model) "test"))]
-             [table-name model])))
+  (into {}
+        (for [model (toucan-models)
+              :when (mdb.u/toucan-model? model)
+              :let  [table-name (some-> model t2/table-name name)]
+              :when table-name
+              ;; ignore any models defined in test namespaces.
+              :when (not (str/includes? (namespace model) "test"))]
+         [table-name model])))
 
 (defn- entity-id-models
   "Return a set of all Toucan models that have an `entity_id` column."
@@ -49,7 +54,10 @@
         table-name->model           (make-table-name->model)
         entity-id-table-name->model (into {}
                                           (map (fn [table-name]
-                                                 [table-name (table-name->model table-name)]))
+                                                 (if-let [model (table-name->model table-name)]
+                                                  [table-name model]
+                                                  (throw (ex-info (trs "Model not found for table {0}" table-name)
+                                                                  {:table-name table-name})))))
                                           entity-id-table-names)
         entity-id-models            (set (vals entity-id-table-name->model))]
     ;; make sure we've resolved all of the tables that have entity_id to their corresponding models.

enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj

@@ -6,12 +6,12 @@
   explicitly excluded, or has the :entity_id property."
   (:require
    [clojure.test :refer :all]
+   [metabase-enterprise.serialization.v2.backfill-ids :as serdes.backfill]
+   [metabase-enterprise.serialization.v2.seed-entity-ids :as v2.seed-entity-ids]
    [metabase.db.data-migrations]
    [metabase.models]
-   [metabase.models.interface :as mi]
    [metabase.models.revision-test]
-   [metabase.models.serialization :as serdes]
-   [toucan.models :as models]))
+   [metabase.models.serialization :as serdes]))
 
 (set! *warn-on-reflection* true)
 
@@ -73,17 +73,16 @@
     :metabase-enterprise.sandbox.models.group-table-access-policy/GroupTableAccessPolicy})
 
 (deftest ^:parallel comprehensive-entity-id-test
-  (doseq [model (->> (descendants :toucan1/model)
+  (doseq [model (->> (v2.seed-entity-ids/toucan-models)
                      (remove entities-not-exported)
                      (remove entities-external-name))]
     (testing (format (str "Model %s should either: have the ::mi/entity-id property, or be explicitly listed as having "
                           "an external name, or explicitly listed as excluded from serialization")
                      model)
-      (is (contains? (set (keys (models/properties model)))
-                     ::mi/entity-id)))))
+      (is (true? (serdes.backfill/has-entity-id? model))))))
 
 (deftest ^:parallel comprehensive-identity-hash-test
-  (doseq [model (->> (descendants :toucan1/model)
+  (doseq [model (->> (v2.seed-entity-ids/toucan-models)
                      (remove entities-not-exported))]
     (testing (format "Model %s should implement identity-hash-fields" model)
       (is (some? (try

src/metabase/db/util.clj

@@ -6,26 +6,27 @@
    [schema.core :as s]
    [toucan.db :as db]
    [toucan.models :as models]
-   [toucan2.core :as t2]))
+   [toucan2.core :as t2]
+   [toucan2.model :as t2.model]))
+
+(defn toucan-model?
+  "Check if `model` is a toucan model.
+  In toucan2 any keywords can be a model so it's always true for keyword."
+  [model]
+  (if (keyword? model)
+    true
+    #_{:clj-kondo/ignore [:discouraged-var]}
+    (models/model? model)))
 
 (defn primary-key
   "Replacement of [[mdb.u/primary-key]], this is used to make the transition to toucan 2 easier.
   In toucan2, every keyword can be a model so if `model` is a keyword, returns as is, otherwise calls [[mdb.u/primary-key]]."
   [model]
   (if (keyword? model)
-   (first (t2/primary-keys :m/card))
+   (first (t2/primary-keys model))
    #_{:clj-kondo/ignore [:discouraged-var]}
    (models/primary-key model)))
 
-(defn resolve-model
-  "Replacement of [[mb.models/resolve-model]], this is used to make the transition to toucan 2 easier.
-  In toucan2, every keyword can be a model so if `model` is a keyword, returns as is, otherwise calls [[toucan1.db/resolve-model]]."
-  [model]
-  (if (keyword? model)
-    model
-    #_{:clj-kondo/ignore [:discouraged-var]}
-    (db/resolve-model model)))
-
 (defn join
   "Convenience for generating a HoneySQL `JOIN` clause.
 
@@ -33,7 +34,7 @@
        (mdb/join [FieldValues :field_id] [Field :id])
        :active true)"
   [[source-entity fk] [dest-entity pk]]
-  {:left-join [(t2/table-name (resolve-model dest-entity))
+  {:left-join [(t2/table-name (t2.model/resolve-model dest-entity))
                [:= (db/qualify source-entity fk) (db/qualify dest-entity pk)]]})
 
 (def ^:private NamespacedKeyword

src/metabase/models/interface.clj

@@ -25,6 +25,7 @@
    [taoensso.nippy :as nippy]
    [toucan.models :as models]
    [toucan2.core :as t2]
+   [toucan2.model :as t2.model]
    [toucan2.tools.before-insert :as t2.before-insert]
    [toucan2.tools.hydrate :as t2.hydrate]
    [toucan2.util :as t2.u])
@@ -374,6 +375,16 @@
   :ret  any?)
 
 
+(methodical/defmethod t2.model/resolve-model :around clojure.lang.Symbol
+  "Handle models deriving from :metabase/model."
+  [symb]
+  (or
+    (when (simple-symbol? symb)
+      (let [metabase-models-keyword (keyword "model" (name symb))]
+        (when (isa? metabase-models-keyword :metabase/model)
+          metabase-models-keyword)))
+    (next-method symb)))
+
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                             New Permissions Stuff                                              |
 ;;; +----------------------------------------------------------------------------------------------------------------+
@@ -590,6 +601,6 @@
 (reset! t2.hydrate/global-error-on-unknown-key true)
 
 (methodical/defmethod t2.hydrate/fk-keys-for-automagic-hydration :default
-  "In Metabase the FK key used for automagic hydration should use underscores (work around unstream Toucan 2 issue)."
+  "In Metabase the FK key used for automagic hydration should use underscores (work around upstream Toucan 2 issue)."
   [_original-model dest-key _hydrated-key]
   [(csk/->snake_case (keyword (str (name dest-key) "_id")))])

src/metabase/models/revision.clj

 (ns metabase.models.revision
   (:require
    [clojure.data :as data]
-   [metabase.db.util :as mdb.u]
    [metabase.models.interface :as mi]
    [metabase.models.revision.diff :refer [diff-string]]
    [metabase.models.user :refer [User]]
@@ -9,7 +8,8 @@
    [metabase.util.i18n :refer [tru]]
    [toucan.hydrate :refer [hydrate]]
    [toucan.models :as models]
-   [toucan2.core :as t2]))
+   [toucan2.core :as t2]
+   [toucan2.model :as t2.model]))
 
 (def ^:const max-revisions
   "Maximum number of revisions to keep for each individual object. After this limit is surpassed, the oldest revisions
@@ -68,7 +68,7 @@
   [{:keys [model], :as revision}]
   ;; in some cases (such as tests) we have 'fake' models that cannot be resolved normally; don't fail entirely in
   ;; those cases
-  (let [model (u/ignore-exceptions (mdb.u/resolve-model (symbol model)))]
+  (let [model (u/ignore-exceptions (t2.model/resolve-model (symbol model)))]
     (cond-> revision
       model (update :object (partial models/do-post-select model)))))
 

src/metabase/models/serialization.clj

@@ -26,7 +26,8 @@
    [metabase.util.log :as log]
    [toucan.db :as db]
    [toucan.hydrate :refer [hydrate]]
-   [toucan2.core :as t2])
+   [toucan2.core :as t2]
+   [toucan2.model :as t2.model])
   (:refer-clojure :exclude [descendants]))
 
 (set! *warn-on-reflection* true)
@@ -92,7 +93,7 @@
 (defn infer-self-path
   "Returns `{:model \"ModelName\" :id \"id-string\"}`"
   [model-name entity]
-  (let [model (mdb.u/resolve-model (symbol model-name))
+  (let [model (t2.model/resolve-model (symbol model-name))
         pk    (mdb.u/primary-key model)]
     {:model model-name
      :id    (or (entity-id model-name entity)
@@ -197,7 +198,7 @@
 
   Returns the Clojure map."
   [model-name entity]
-  (let [model (mdb.u/resolve-model (symbol model-name))
+  (let [model (t2.model/resolve-model (symbol model-name))
         pk    (mdb.u/primary-key model)]
     (-> (into {} entity)
         (assoc :serdes/meta (generate-path model-name entity))
@@ -246,7 +247,7 @@
 
 (defmethod load-find-local :default [path]
   (let [{id :id model-name :model} (last path)
-        model                      (mdb.u/resolve-model (symbol model-name))]
+        model                      (t2.model/resolve-model (symbol model-name))]
     (when model
       (*lookup-by-id* model id))))
 
@@ -300,7 +301,7 @@
   (fn [model _ _] model))
 
 (defmethod load-update! :default [model-name ingested local]
-  (let [model    (mdb.u/resolve-model (symbol model-name))
+  (let [model    (t2.model/resolve-model (symbol model-name))
         pk       (mdb.u/primary-key model)
         id       (get local pk)]
     (log/tracef "Upserting %s %d: old %s new %s" model-name id (pr-str local) (pr-str ingested))
@@ -325,7 +326,7 @@
 
 (defmethod load-insert! :default [model-name ingested]
   (log/tracef "Inserting %s: %s" model-name (pr-str ingested))
-  (first (t2/insert-returning-instances! (symbol model-name) ingested)))
+  (first (t2/insert-returning-instances! (t2.model/resolve-model (symbol model-name)) ingested)))
 
 (defmulti load-one!
   "Black box for integrating a deserialized entity into this appdb.
@@ -445,7 +446,7 @@
   [id model]
   (when id
     (let [model-name (name model)
-          model      (mdb.u/resolve-model (symbol model-name))
+          model      (t2.model/resolve-model (symbol model-name))
           entity     (t2/select-one model (mdb.u/primary-key model) id)
           path       (mapv :id (generate-path model-name entity))]
       (if (= (count path) 1)
@@ -466,7 +467,7 @@
   [eid model]
   (when eid
     (let [model-name (name model)
-          model      (mdb.u/resolve-model (symbol model-name))
+          model      (t2.model/resolve-model (symbol model-name))
           eid        (if (vector? eid)
                        (last eid)
                        eid)

test/metabase/test.clj

@@ -49,7 +49,8 @@
    [pjstadig.humane-test-output :as humane-test-output]
    [potemkin :as p]
    [toucan.util.test :as tt]
-   [toucan2.core :as t2]))
+   [toucan2.core :as t2]
+   [toucan2.model :as t2.model]))
 
 (set! *warn-on-reflection* true)
 
@@ -419,4 +420,4 @@
    (fn [toucan-model]
      (hawk.init/assert-tests-are-not-initializing (list 'object-defaults (symbol (name toucan-model))))
      (initialize/initialize-if-needed! :db)
-     (mdb.u/resolve-model toucan-model))))
+     (t2.model/resolve-model toucan-model))))

test/metabase/test/util.clj

@@ -63,6 +63,7 @@
    [toucan.models :as models]
    [toucan.util.test :as tt]
    [toucan2.core :as t2]
+   [toucan2.model :as t2.model]
    [toucan2.tools.with-temp :as t2.with-temp])
   (:import
    (java.io File FileInputStream)
@@ -474,7 +475,7 @@
   (hawk.parallel/assert-test-is-not-parallel "with-temp-vals-in-db")
   ;; use low-level `query` and `execute` functions here, because Toucan `select` and `update` functions tend to do
   ;; things like add columns like `common_name` that don't actually exist, causing subsequent update to fail
-  (let [model                    (mdb.u/resolve-model model)
+  (let [model                    (t2.model/resolve-model model)
         [original-column->value] (mdb.query/query {:select (keys column->temp-value)
                                                    :from   [(t2/table-name model)]
                                                    :where  [:= :id (u/the-id object-or-id)]})]