From ad103807bedb1774202686d85020c8d6427a19a0 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Wed, 12 Oct 2022 19:21:58 -0700
Subject: [PATCH] Add CLI SerDes command for seeding entity IDs (#25655)
(#25870)
* Add CLI SerDes command for seeding entity IDs (#25655)
* Remove stray comment
* Fix Kondo error
* Fix Eastwood error
* Update identity-hash-fields to multimethod for ModelAction
* Fix identity-hash-fields for Dimension and DashboardCardSeries
---
.../metabase_enterprise/serialization/cmd.clj | 8 ++
.../serialization/v2/seed_entity_ids.clj | 114 ++++++++++++++++++
.../models/entity_id_test.clj | 17 +--
.../serialization/v2/seed_entity_ids_test.clj | 37 ++++++
src/metabase/cmd.clj | 9 ++
src/metabase/models/action.clj | 9 +-
src/metabase/models/app.clj | 13 +-
src/metabase/models/card.clj | 7 +-
src/metabase/models/collection.clj | 21 ++--
src/metabase/models/dashboard.clj | 7 +-
src/metabase/models/dashboard_card.clj | 17 +--
src/metabase/models/dashboard_card_series.clj | 16 +--
src/metabase/models/database.clj | 7 +-
src/metabase/models/dimension.clj | 9 +-
src/metabase/models/field.clj | 7 +-
src/metabase/models/field_values.clj | 7 +-
src/metabase/models/interface.clj | 2 +
src/metabase/models/metric.clj | 7 +-
src/metabase/models/native_query_snippet.clj | 7 +-
src/metabase/models/pulse.clj | 7 +-
src/metabase/models/pulse_card.clj | 7 +-
src/metabase/models/pulse_channel.clj | 7 +-
src/metabase/models/segment.clj | 7 +-
src/metabase/models/serialization/hash.clj | 24 ++--
src/metabase/models/setting.clj | 9 +-
src/metabase/models/table.clj | 7 +-
src/metabase/models/timeline.clj | 7 +-
src/metabase/models/timeline_event.clj | 7 +-
src/metabase/models/user.clj | 8 +-
test/metabase/models/dispatch_test.clj | 7 +-
30 files changed, 311 insertions(+), 107 deletions(-)
create mode 100644 enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj
create mode 100644 enterprise/backend/test/metabase_enterprise/serialization/v2/seed_entity_ids_test.clj
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj b/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
index 6e7cdb23d8b..52975aadd93 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
@@ -7,6 +7,7 @@
[metabase-enterprise.serialization.v2.extract :as v2.extract]
[metabase-enterprise.serialization.v2.ingest.yaml :as v2.ingest]
[metabase-enterprise.serialization.v2.load :as v2.load]
+ [metabase-enterprise.serialization.v2.seed-entity-ids :as v2.seed-entity-ids]
[metabase-enterprise.serialization.v2.storage.yaml :as v2.storage]
[metabase.db :as mdb]
[metabase.models.card :refer [Card]]
@@ -202,3 +203,10 @@
(if v2
(v2-dump path opts)
(v1-dump path state user opts)))
+
+(defn seed-entity-ids
+ "Add entity IDs for instances of serializable models that don't already have them.
+
+ Returns truthy if all entity IDs were added successfully, or falsey if any errors were encountered."
+ [options]
+ (v2.seed-entity-ids/seed-entity-ids! options))
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj b/enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj
new file mode 100644
index 00000000000..ea71b95544e
--- /dev/null
+++ b/enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj
@@ -0,0 +1,114 @@
+(ns metabase-enterprise.serialization.v2.seed-entity-ids
+ (:require
+ [clojure.string :as str]
+ [clojure.tools.logging :as log]
+ [metabase.db :as mdb]
+ [metabase.db.connection :as mdb.connection]
+ [metabase.models]
+ [metabase.models.dispatch :as models.dispatch]
+ [metabase.models.serialization.hash :as serdes.hash]
+ [metabase.util :as u]
+ [metabase.util.i18n :refer [trs]]
+ [toucan.db :as db]
+ [toucan.models :as models]))
+
+(set! *warn-on-reflection* true)
+
+;;; make sure all the models get loaded up so we can resolve them based on their table names.
+;;;
+;;; TODO -- what about enterprise models that have `entity_id`? Don't know of any yet. We'll have to cross that bridge
+;;; when we get there.
+(comment metabase.models/keep-me)
+
+(defn- entity-id-table-names
+ "Return a set of lower-cased names of all application database tables that have an `entity_id` column."
+ []
+ (with-open [conn (.getConnection mdb.connection/*application-db*)]
+ (let [dbmeta (.getMetaData conn)]
+ (with-open [rset (.getColumns dbmeta nil nil nil (case (mdb.connection/db-type)
+ :h2 "ENTITY_ID"
+ (:mysql :postgres) "entity_id"))]
+ (into #{} (map (comp u/lower-case-en :table_name)) (resultset-seq rset))))))
+
+(defn- make-table-name->model
+ "Create a map of (lower-cased) application DB table name -> corresponding Toucan model."
+ []
+ (into {} (for [^Class klass (extenders toucan.models/IModel)
+ :let [model (models.dispatch/model (.newInstance klass))]
+ :when (models/model? model)
+ :let [table-name (some-> model :table 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."
+ []
+ (let [entity-id-table-names (entity-id-table-names)
+ table-name->model (make-table-name->model)
+ entity-id-table-name->model (into {}
+ (map (fn [table-name]
+ [table-name (table-name->model 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.
+ (when-not (= (count entity-id-table-names)
+ (count entity-id-models))
+ (throw (ex-info (trs "{0} tables have entity_id; expected to resolve the same number of models, but only got {1}"
+ (count entity-id-table-names)
+ (count entity-id-models))
+ {:tables entity-id-table-names
+ :resolved entity-id-table-name->model})))
+ (set entity-id-models)))
+
+(defn- seed-entity-id-for-instance! [model instance]
+ (try
+ (let [primary-key (models/primary-key model)
+ pk-value (get instance primary-key)]
+ (when-not (some? pk-value)
+ (throw (ex-info (format "Missing value for primary key column %s" (pr-str primary-key))
+ {:model (name model)
+ :instance instance
+ :primary-key primary-key})))
+ (let [new-hash (serdes.hash/identity-hash instance)]
+ (log/infof "Update %s %s entity ID => %s" (name model) (pr-str pk-value) (pr-str new-hash))
+ (db/update! model pk-value :entity_id new-hash))
+ {:update-count 1})
+ (catch Throwable e
+ (log/errorf e "Error updating entity ID: %s" (ex-message e))
+ {:error-count 1})))
+
+(defn- seed-entity-ids-for-model! [model]
+ (log/infof "Seeding Entity IDs for model %s" (name model))
+ (let [reducible-instances (db/select-reducible model :entity_id nil)]
+ (transduce
+ (map (fn [instance]
+ (seed-entity-id-for-instance! model instance)))
+ (completing
+ (partial merge-with +)
+ (fn [{:keys [update-count error-count], :as results}]
+ (when (pos? update-count)
+ (log/infof "Updated %d %s instance(s) successfully." update-count (name model)))
+ (when (pos? error-count)
+ (log/infof "Failed to update %d %s instance(s) because of errors." error-count (name model)))
+ results))
+ {:update-count 0, :error-count 0}
+ reducible-instances)))
+
+(defn seed-entity-ids!
+ "Create entity IDs for any instances of models that support them but do not have them, i.e. find instances of models
+ that have an `entity_id` column whose `entity_id` is `nil` and populate that column.
+
+ `options` are currently ignored but this may change in the future.
+
+ Returns truthy if all missing entity IDs were created successfully, and falsey if there were any errors."
+ [options]
+ (log/infof "Seeding Entity IDs with options %s" (pr-str options))
+ (mdb/setup-db!)
+ (let [{:keys [error-count]} (transduce
+ (map seed-entity-ids-for-model!)
+ (completing (partial merge-with +))
+ {:update-count 0, :error-count 0}
+ (entity-id-models))]
+ (zero? error-count)))
diff --git a/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj b/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj
index 1d6ff18533e..d4e0de1ad0b 100644
--- a/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj
@@ -72,9 +72,9 @@
metabase_enterprise.sandbox.models.group_table_access_policy.GroupTableAccessPolicyInstance})
(deftest comprehensive-entity-id-test
- (doseq [model (->> (extenders IModel)
- (remove entities-not-exported)
- (remove entities-external-name))]
+ (doseq [^Class model (->> (extenders IModel)
+ (remove entities-not-exported)
+ (remove entities-external-name))]
(testing (format "Model %s should either: have the :entity_id property, or be explicitly listed as having an external name, or explicitly listed as excluded from serialization"
(.getSimpleName model))
(is (= true (-> (.newInstance model)
@@ -82,7 +82,10 @@
:entity_id))))))
(deftest comprehensive-identity-hash-test
- (doseq [model (->> (extenders IModel)
- (remove entities-not-exported))]
- (testing (format "Model %s should implement IdentityHashable" (.getSimpleName model))
- (is (extends? serdes.hash/IdentityHashable model)))))
+ (doseq [^Class model (->> (extenders IModel)
+ (remove entities-not-exported))]
+ (testing (format "Model %s should implement identity-hash-fields" (.getSimpleName model))
+ (is (some? (try
+ (serdes.hash/identity-hash-fields (.newInstance model))
+ (catch java.lang.IllegalArgumentException _
+ nil)))))))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/seed_entity_ids_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/seed_entity_ids_test.clj
new file mode 100644
index 00000000000..6be17e90f7e
--- /dev/null
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/seed_entity_ids_test.clj
@@ -0,0 +1,37 @@
+(ns metabase-enterprise.serialization.v2.seed-entity-ids-test
+ (:require
+ [clojure.string :as str]
+ [clojure.test :refer :all]
+ [metabase-enterprise.serialization.v2.seed-entity-ids
+ :as v2.seed-entity-ids]
+ [metabase.models :refer [Collection]]
+ [metabase.test :as mt]
+ [toucan.db :as db]))
+
+(deftest seed-entity-ids-test
+ (testing "Sanity check: should succeed before we go around testing specific situations"
+ (is (true? (v2.seed-entity-ids/seed-entity-ids! nil))))
+ (testing "With a temp Collection with no entity ID"
+ (mt/with-temp Collection [c {:name "No Entity ID Collection", :slug "no_entity_id_collection", :color "#FF0000"}]
+ (db/update! Collection (:id c) :entity_id nil)
+ (letfn [(entity-id []
+ (some-> (db/select-one-field :entity_id Collection :id (:id c)) str/trim))]
+ (is (= nil
+ (entity-id)))
+ (testing "Should return truthy on success"
+ (is (= true
+ (v2.seed-entity-ids/seed-entity-ids! nil))))
+ (is (= "c03d4632"
+ (entity-id))))
+ (testing "Error: duplicate entity IDs"
+ (mt/with-temp Collection [c2 {:name "No Entity ID Collection", :slug "no_entity_id_collection", :color "#FF0000"}]
+ (db/update! Collection (:id c2) :entity_id nil)
+ (letfn [(entity-id []
+ (some-> (db/select-one-field :entity_id Collection :id (:id c2)) str/trim))]
+ (is (= nil
+ (entity-id)))
+ (testing "Should return falsey on error"
+ (is (= false
+ (v2.seed-entity-ids/seed-entity-ids! nil))))
+ (is (= nil
+ (entity-id)))))))))
diff --git a/src/metabase/cmd.clj b/src/metabase/cmd.clj
index b1975049e0c..05142ff48ed 100644
--- a/src/metabase/cmd.clj
+++ b/src/metabase/cmd.clj
@@ -154,6 +154,15 @@
(let [cmd (resolve-enterprise-command 'metabase-enterprise.serialization.cmd/dump)]
(cmd path (cmd-args->map args)))))
+(defn ^:command seed-entity-ids
+ "Add entity IDs for instances of serializable models that don't already have them."
+ [& options]
+ (let [cmd (resolve-enterprise-command 'metabase-enterprise.serialization.cmd/seed-entity-ids)
+ options (cmd-args->map options)]
+ (system-exit! (if (cmd options)
+ 0
+ 1))))
+
(defn ^:command rotate-encryption-key
"Rotate the encryption key of a metabase database. The MB_ENCRYPTION_SECRET_KEY environment variable has to be set to
the current key, and the parameter `new-key` has to be the new key. `new-key` has to be at least 16 chars."
diff --git a/src/metabase/models/action.clj b/src/metabase/models/action.clj
index 7eb1242eeca..cafce7ad33d 100644
--- a/src/metabase/models/action.clj
+++ b/src/metabase/models/action.clj
@@ -62,10 +62,13 @@
(merge models/IModelDefaults
{:properties (constantly {:entity_id true})
:types (constantly {:parameter_mappings :parameters-list
- :visualization_settings :visualization-settings})})
+ :visualization_settings :visualization-settings})}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields [:entity_id]})
+;;; TODO -- this doesn't seem right. [[serdes.hash/identity-hash-fields]] is used to calculate `entity_id`, so we
+;;; shouldn't use it in the calculation. We can fix this later
+(defmethod serdes.hash/identity-hash-fields ModelAction
+ [_model-action]
+ [:entity_id])
(defn insert!
"Inserts an Action and related HTTPAction or QueryAction. Returns the action id."
diff --git a/src/metabase/models/app.clj b/src/metabase/models/app.clj
index 3460147dcff..52bb6fa2408 100644
--- a/src/metabase/models/app.clj
+++ b/src/metabase/models/app.clj
@@ -17,12 +17,13 @@
{:types (constantly {:options :json
:nav_items :json})
:properties (constantly {:timestamped? true
- :entity_id true})})
+ :entity_id true})}))
- ;; Should not be needed as every app should have an entity_id, but currently it's
- ;; necessary to satisfy metabase-enterprise.models.entity-id-test/comprehensive-identity-hash-test.
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:entity_id])})
+;;; Should not be needed as every app should have an entity_id, but currently it's necessary to satisfy
+;;; metabase-enterprise.models.entity-id-test/comprehensive-identity-hash-test.
+(defmethod serdes.hash/identity-hash-fields App
+ [_app]
+ [:entity_id])
(defn add-app-id
"Add `app_id` to Collections that are linked with an App."
@@ -36,7 +37,7 @@
(let [coll-id->app-id (into {}
(map (juxt :collection_id :id))
(db/select [App :collection_id :id]
- :collection_id [:in coll-ids]))]
+ :collection_id [:in coll-ids]))]
(for [coll collections]
(let [app-id (coll-id->app-id (:id coll))]
(cond-> coll
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index a189e9493b4..b2d03e0f807 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -324,10 +324,11 @@
:pre-insert (comp populate-query-fields pre-insert populate-result-metadata maybe-normalize-query)
:post-insert post-insert
:pre-delete pre-delete
- :post-select public-settings/remove-public-uuid-if-public-sharing-is-disabled})
+ :post-select public-settings/remove-public-uuid-if-public-sharing-is-disabled}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name (serdes.hash/hydrated-hash :collection)])})
+(defmethod serdes.hash/identity-hash-fields Card
+ [_card]
+ [:name (serdes.hash/hydrated-hash :collection)])
;;; ------------------------------------------------- Serialization --------------------------------------------------
(defmethod serdes.base/extract-query "Card" [_ {:keys [user]}]
diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj
index a5013d3ca10..304d14fb422 100644
--- a/src/metabase/models/collection.clj
+++ b/src/metabase/models/collection.clj
@@ -891,6 +891,10 @@
(serdes.hash/identity-hash (db/select-one Collection :id parent-id))
"ROOT")))
+(defmethod serdes.hash/identity-hash-fields Collection
+ [_collection]
+ [:name :namespace parent-identity-hash])
+
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Collection)
models/IModel
(merge models/IModelDefaults
@@ -901,19 +905,16 @@
:pre-insert pre-insert
:post-insert post-insert
:pre-update pre-update
- :pre-delete pre-delete})
-
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name :namespace parent-identity-hash])})
+ :pre-delete pre-delete}))
(defn- collection-query [maybe-user]
(serdes.base/raw-reducible-query
- "Collection"
- {:where [:and
- [:= :archived false]
- (if (nil? maybe-user)
- [:is :personal_owner_id nil]
- [:= :personal_owner_id maybe-user])]}))
+ "Collection"
+ {:where [:and
+ [:= :archived false]
+ (if (nil? maybe-user)
+ [:is :personal_owner_id nil]
+ [:= :personal_owner_id maybe-user])]}))
(defmethod serdes.base/extract-query "Collection" [_ {:keys [user]}]
(let [unowned (collection-query nil)]
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 4dd7b93d8fc..d6d2536110e 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -141,10 +141,11 @@
:pre-insert pre-insert
:pre-update pre-update
:post-update post-update
- :post-select public-settings/remove-public-uuid-if-public-sharing-is-disabled})
+ :post-select public-settings/remove-public-uuid-if-public-sharing-is-disabled}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name (serdes.hash/hydrated-hash :collection)])})
+(defmethod serdes.hash/identity-hash-fields Dashboard
+ [_dashboard]
+ [:name (serdes.hash/hydrated-hash :collection)])
;;; --------------------------------------------------- Revisions ----------------------------------------------------
diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj
index 49f4007ee73..11b592d5f99 100644
--- a/src/metabase/models/dashboard_card.clj
+++ b/src/metabase/models/dashboard_card.clj
@@ -48,14 +48,15 @@
:entity_id true})
:types (constantly {:parameter_mappings :parameters-list
:visualization_settings :visualization-settings})
- :pre-insert pre-insert})
-
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [(serdes.hash/hydrated-hash :card)
- (comp serdes.hash/identity-hash
- #(db/select-one 'Dashboard :id %)
- :dashboard_id)
- :visualization_settings])})
+ :pre-insert pre-insert}))
+
+(defmethod serdes.hash/identity-hash-fields DashboardCard
+ [_dashboard-card]
+ [(serdes.hash/hydrated-hash :card)
+ (comp serdes.hash/identity-hash
+ #(db/select-one 'Dashboard :id %)
+ :dashboard_id)
+ :visualization_settings])
;;; --------------------------------------------------- HYDRATION ----------------------------------------------------
diff --git a/src/metabase/models/dashboard_card_series.clj b/src/metabase/models/dashboard_card_series.clj
index b632301dcc2..b74e7eaf839 100644
--- a/src/metabase/models/dashboard_card_series.clj
+++ b/src/metabase/models/dashboard_card_series.clj
@@ -1,15 +1,15 @@
(ns metabase.models.dashboard-card-series
- (:require [metabase.models.serialization.hash :as serdes.hash]
- [metabase.util :as u]
- [toucan.db :as db]
- [toucan.models :as models]))
+ (:require
+ [metabase.models.serialization.hash :as serdes.hash]
+ [toucan.db :as db]
+ [toucan.models :as models]))
(models/defmodel DashboardCardSeries :dashboardcard_series)
(defn- dashboard-card [{:keys [dashboardcard_id]}]
(db/select-one 'DashboardCard :id dashboardcard_id))
-(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class DashboardCardSeries)
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [(comp serdes.hash/identity-hash dashboard-card)
- (serdes.hash/hydrated-hash :card)])})
+(defmethod serdes.hash/identity-hash-fields DashboardCardSeries
+ [_dashboard-card-series]
+ [(comp serdes.hash/identity-hash dashboard-card)
+ (serdes.hash/hydrated-hash :card)])
diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj
index d8fdb9b44e0..da95dd5858b 100644
--- a/src/metabase/models/database.clj
+++ b/src/metabase/models/database.clj
@@ -211,10 +211,11 @@
:post-select post-select
:pre-insert pre-insert
:pre-update pre-update
- :pre-delete pre-delete})
+ :pre-delete pre-delete}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name :engine])})
+(defmethod serdes.hash/identity-hash-fields Database
+ [_database]
+ [:name :engine])
;;; ---------------------------------------------- Hydration / Util Fns ----------------------------------------------
diff --git a/src/metabase/models/dimension.clj b/src/metabase/models/dimension.clj
index 0b8ad58a834..8b38ada9140 100644
--- a/src/metabase/models/dimension.clj
+++ b/src/metabase/models/dimension.clj
@@ -20,11 +20,12 @@
(merge models/IModelDefaults
{:types (constantly {:type :keyword})
:properties (constantly {:timestamped? true
- :entity_id true})})
+ :entity_id true})}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [(serdes.hash/hydrated-hash :field)
- (serdes.hash/hydrated-hash :human_readable_field)])})
+(defmethod serdes.hash/identity-hash-fields Dimension
+ [_dimension]
+ [(serdes.hash/hydrated-hash :field)
+ (serdes.hash/hydrated-hash :human_readable_field)])
;;; ------------------------------------------------- Serialization --------------------------------------------------
(defmethod serdes.base/extract-one "Dimension"
diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj
index e06de64ac21..bc498b47d68 100644
--- a/src/metabase/models/field.clj
+++ b/src/metabase/models/field.clj
@@ -193,10 +193,11 @@
:settings :json
:nfc_path :json})
:properties (constantly {:timestamped? true})
- :pre-insert pre-insert})
+ :pre-insert pre-insert}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name (serdes.hash/hydrated-hash :table)])})
+(defmethod serdes.hash/identity-hash-fields Field
+ [_field]
+ [:name (serdes.hash/hydrated-hash :table)])
;;; ---------------------------------------------- Hydration / Util Fns ----------------------------------------------
diff --git a/src/metabase/models/field_values.clj b/src/metabase/models/field_values.clj
index c1ebdd11bf6..653d3c04964 100644
--- a/src/metabase/models/field_values.clj
+++ b/src/metabase/models/field_values.clj
@@ -164,10 +164,11 @@
:type :keyword})
:pre-insert pre-insert
:pre-update pre-update
- :post-select post-select})
+ :post-select post-select}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [(serdes.hash/hydrated-hash :field)])})
+(defmethod serdes.hash/identity-hash-fields FieldValues
+ [_field-values]
+ [(serdes.hash/hydrated-hash :field)])
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Utils fns |
diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj
index d19561d6cad..6cad9734b4a 100644
--- a/src/metabase/models/interface.clj
+++ b/src/metabase/models/interface.clj
@@ -454,3 +454,5 @@
(alter-meta! (var ~model) assoc ::defmodel-hash ~(hash &form))))))
(alter-var-root #'models/defmodel (constantly @#'defmodel))
+(alter-meta! #'models/defmodel (fn [mta]
+ (merge mta (select-keys (meta #'defmodel) [:file :line :column :ns]))))
diff --git a/src/metabase/models/metric.clj b/src/metabase/models/metric.clj
index 4a8494e4c2a..c60620fa185 100644
--- a/src/metabase/models/metric.clj
+++ b/src/metabase/models/metric.clj
@@ -44,10 +44,11 @@
{:types (constantly {:definition :metric-segment-definition})
:properties (constantly {:timestamped? true
:entity_id true})
- :pre-update pre-update})
+ :pre-update pre-update}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name (serdes.hash/hydrated-hash :table)])})
+(defmethod serdes.hash/identity-hash-fields Metric
+ [_metric]
+ [:name (serdes.hash/hydrated-hash :table)])
;;; --------------------------------------------------- REVISIONS ----------------------------------------------------
diff --git a/src/metabase/models/native_query_snippet.clj b/src/metabase/models/native_query_snippet.clj
index 55b9db23908..4fc8f85021f 100644
--- a/src/metabase/models/native_query_snippet.clj
+++ b/src/metabase/models/native_query_snippet.clj
@@ -39,10 +39,11 @@
{:properties (constantly {:timestamped? true
:entity_id true})
:pre-insert pre-insert
- :pre-update pre-update})
+ :pre-update pre-update}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name (serdes.hash/hydrated-hash :collection)])})
+(defmethod serdes.hash/identity-hash-fields NativeQuerySnippet
+ [_snippet]
+ [:name (serdes.hash/hydrated-hash :collection)])
(defmethod mi/can-read? NativeQuerySnippet
[& args]
diff --git a/src/metabase/models/pulse.clj b/src/metabase/models/pulse.clj
index 5d2428fdb53..f0b4a0532b4 100644
--- a/src/metabase/models/pulse.clj
+++ b/src/metabase/models/pulse.clj
@@ -133,10 +133,11 @@
:entity_id true})
:pre-insert pre-insert
:pre-update pre-update
- :types (constantly {:parameters :json})})
+ :types (constantly {:parameters :json})}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name (serdes.hash/hydrated-hash :collection)])})
+(defmethod serdes.hash/identity-hash-fields Pulse
+ [_pulse]
+ [:name (serdes.hash/hydrated-hash :collection)])
(def ^:private ^:dynamic *automatically-archive-when-last-channel-is-deleted*
"Should we automatically archive a Pulse when its last `PulseChannel` is deleted? Normally we do, but this is disabled
diff --git a/src/metabase/models/pulse_card.clj b/src/metabase/models/pulse_card.clj
index 2cf54373ecd..4e9bc4be738 100644
--- a/src/metabase/models/pulse_card.clj
+++ b/src/metabase/models/pulse_card.clj
@@ -13,10 +13,11 @@
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PulseCard)
models/IModel
(merge models/IModelDefaults
- {:properties (constantly {:entity_id true})})
+ {:properties (constantly {:entity_id true})}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [(serdes.hash/hydrated-hash :pulse) (serdes.hash/hydrated-hash :card)])})
+(defmethod serdes.hash/identity-hash-fields PulseCard
+ [_pulse-card]
+ [(serdes.hash/hydrated-hash :pulse) (serdes.hash/hydrated-hash :card)])
(defn next-position-for
"Return the next available `pulse_card.position` for the given `pulse`"
diff --git a/src/metabase/models/pulse_channel.clj b/src/metabase/models/pulse_channel.clj
index c27798ba77e..bde7afd8202 100644
--- a/src/metabase/models/pulse_channel.clj
+++ b/src/metabase/models/pulse_channel.clj
@@ -194,10 +194,11 @@
:entity_id true})
:pre-delete pre-delete
:pre-insert validate-email-domains
- :pre-update validate-email-domains})
+ :pre-update validate-email-domains}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [(serdes.hash/hydrated-hash :pulse) :channel_type :details])})
+(defmethod serdes.hash/identity-hash-fields PulseChannel
+ [_pulse-channel]
+ [(serdes.hash/hydrated-hash :pulse) :channel_type :details])
(defn will-delete-recipient
"This function is called by [[metabase.models.pulse-channel-recipient/pre-delete]] when a `PulseChannelRecipient` is
diff --git a/src/metabase/models/segment.clj b/src/metabase/models/segment.clj
index 221602455c6..c912c75f360 100644
--- a/src/metabase/models/segment.clj
+++ b/src/metabase/models/segment.clj
@@ -44,10 +44,11 @@
:properties (constantly {:timestamped? true
:entity_id true})
:hydration-keys (constantly [:segment])
- :pre-update pre-update})
+ :pre-update pre-update}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name (serdes.hash/hydrated-hash :table)])})
+(defmethod serdes.hash/identity-hash-fields Segment
+ [_segment]
+ [:name (serdes.hash/hydrated-hash :table)])
;;; --------------------------------------------------- Revisions ----------------------------------------------------
diff --git a/src/metabase/models/serialization/hash.clj b/src/metabase/models/serialization/hash.clj
index afc30056888..bd4f3934cc3 100644
--- a/src/metabase/models/serialization/hash.clj
+++ b/src/metabase/models/serialization/hash.clj
@@ -19,28 +19,30 @@
- Any foreign keys should be hydrated and the identity-hash of the foreign entity used as part of the hash.
- There's a [[hydrated-hash]] helper for this with several example uses."
(:require
+ [metabase.models.interface :as mi]
[metabase.util.i18n :refer [tru]]
- [potemkin.types :as p.types]
[toucan.hydrate :refer [hydrate]]))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Identity Hashes |
;;; +----------------------------------------------------------------------------------------------------------------+
+
;;; Generated entity_id values have lately been added to most exported models, but they only get populated on newly
;;; created entities. Since we can't rely on entity_id being present, we need a content-based definition of identity for
;;; all exported models.
-(p.types/defprotocol+ IdentityHashable
- (identity-hash-fields
- [entity]
- "You probably want to call [[metabase.models.serialization.hash/identity-hash]] instead of calling this directly.
- Content-based identity for the entities we export in serialization. This should return a sequence of functions to
- apply to an entity and then hash. For example, Metric's [[identity-hash-fields]] is `[:name :table]`. These
- functions are mapped over each entity and [[clojure.core/hash]] called on the result. This gives a portable hash
- value across JVMs, Clojure versions, and platforms.
+(defmulti identity-hash-fields
+ "You probably want to call [[metabase.models.serialization.hash/identity-hash]] instead of calling this directly.
+
+ Content-based identity for the entities we export in serialization. This should return a sequence of functions to
+ apply to an entity and then hash. For example, Metric's [[identity-hash-fields]] is `[:name :table]`. These
+ functions are mapped over each entity and [[clojure.core/hash]] called on the result. This gives a portable hash
+ value across JVMs, Clojure versions, and platforms.
- NOTE: No numeric database IDs! For any foreign key, use [[hydrated-hash]] to hydrate the foreign entity and include
- its [[identity-hash]] as part of this hash. This is a portable way of capturing the foreign relationship."))
+ NOTE: No numeric database IDs! For any foreign key, use [[hydrated-hash]] to hydrate the foreign entity and include
+ its [[identity-hash]] as part of this hash. This is a portable way of capturing the foreign relationship."
+ {:arglists '([model-or-instance])}
+ mi/dispatch-on-model)
(defn raw-hash
"Hashes a Clojure value into an 8-character hex string, which is used as the identity hash.
diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj
index 63070c68831..18d35a21227 100644
--- a/src/metabase/models/setting.clj
+++ b/src/metabase/models/setting.clj
@@ -143,14 +143,15 @@
models/IModel
(merge models/IModelDefaults
{:types (constantly {:value :encrypted-text})
- :primary-key (constantly :key)})
+ :primary-key (constantly :key)}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:key])})
+(defmethod serdes.hash/identity-hash-fields Setting
+ [_setting]
+ [:key])
(defmethod serdes.base/extract-all "Setting" [_model _opts]
(for [{:keys [key value]} (admin-writable-site-wide-settings
- :getter (partial get-value-of-type :string))]
+ :getter (partial get-value-of-type :string))]
{:serdes/meta [{:model "Setting" :id (name key)}]
:key key
:value value}))
diff --git a/src/metabase/models/table.clj b/src/metabase/models/table.clj
index 172b47a6440..97b9de70769 100644
--- a/src/metabase/models/table.clj
+++ b/src/metabase/models/table.clj
@@ -77,10 +77,11 @@
:field_order :keyword})
:properties (constantly {:timestamped? true})
:pre-insert pre-insert
- :pre-delete pre-delete})
+ :pre-delete pre-delete}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:schema :name (serdes.hash/hydrated-hash :db)])})
+(defmethod serdes.hash/identity-hash-fields Table
+ [_table]
+ [:schema :name (serdes.hash/hydrated-hash :db)])
;;; ------------------------------------------------ Field ordering -------------------------------------------------
diff --git a/src/metabase/models/timeline.clj b/src/metabase/models/timeline.clj
index 280f2d7c811..30ccb53197e 100644
--- a/src/metabase/models/timeline.clj
+++ b/src/metabase/models/timeline.clj
@@ -56,10 +56,11 @@
(merge
models/IModelDefaults
{:properties (constantly {:timestamped? true
- :entity_id true})})
+ :entity_id true})}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name (serdes.hash/hydrated-hash :collection)])})
+(defmethod serdes.hash/identity-hash-fields Timeline
+ [_timeline]
+ [:name (serdes.hash/hydrated-hash :collection)])
;;;; serialization
(defmethod serdes.base/extract-one "Timeline"
diff --git a/src/metabase/models/timeline_event.clj b/src/metabase/models/timeline_event.clj
index 7abf5d7aee8..c77a50b2d6d 100644
--- a/src/metabase/models/timeline_event.clj
+++ b/src/metabase/models/timeline_event.clj
@@ -90,10 +90,11 @@
(merge
models/IModelDefaults
;; todo: add hydration keys??
- {:properties (constantly {:timestamped? true})})
+ {:properties (constantly {:timestamped? true})}))
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:name :timestamp (serdes.hash/hydrated-hash :timeline)])})
+(defmethod serdes.hash/identity-hash-fields TimelineEvent
+ [_timeline-event]
+ [:name :timestamp (serdes.hash/hydrated-hash :timeline)])
;;;; serialization
(defmethod serdes.base/serdes-entity-id "TimelineEvent" [_model-name {:keys [timestamp]}]
diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj
index a14aa6669cc..8651aa4996e 100644
--- a/src/metabase/models/user.clj
+++ b/src/metabase/models/user.clj
@@ -144,9 +144,11 @@
:pre-update pre-update
:post-select post-select
:types (constantly {:login_attributes :json-no-keywordization
- :settings :encrypted-json})})
- serdes.hash/IdentityHashable
- {:identity-hash-fields (constantly [:email])})
+ :settings :encrypted-json})}))
+
+(defmethod serdes.hash/identity-hash-fields User
+ [_user]
+ [:email])
(defn group-ids
"Fetch set of IDs of PermissionsGroup a User belongs to."
diff --git a/test/metabase/models/dispatch_test.clj b/test/metabase/models/dispatch_test.clj
index 2c3d3290f37..bc4712dd5b5 100644
--- a/test/metabase/models/dispatch_test.clj
+++ b/test/metabase/models/dispatch_test.clj
@@ -21,7 +21,12 @@
(models.dispatch/model User))))
(testing "instance"
(is (identical? User
- (models.dispatch/model (a-user))))))
+ (models.dispatch/model (a-user)))))
+ (testing ".newInstance"
+ (is (identical? User
+ (models.dispatch/model (.newInstance
+ #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]}
+ (class User)))))))
(deftest dispatch-by-clause-name-or-class-test
(testing (str `mbql.u/dispatch-by-clause-name-or-class " should use " `models.dispatch/dispatch-value)
--
GitLab