diff --git a/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj b/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj index 6e7cdb23d8b83b7d07b2ca87eef05431706100d5..52975aadd937de26ae8b7438328326a796da11a2 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 0000000000000000000000000000000000000000..ea71b95544ef52d0b9a1dce8f6bdb88cada8f926 --- /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 1d6ff18533e44eeeb0e19661c50c3affa5e43b06..d4e0de1ad0b3d5676d3d5146869dff6b90decaa9 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 0000000000000000000000000000000000000000..6be17e90f7e04bc90d663ec8bde7f48b6f61b772 --- /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 b1975049e0c46d102a8a20b0554b3412ca652501..05142ff48ed1591b7080ee814bfc6bbf1516a7bb 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 7eb1242eecaaab7a810d8bc5707f218a00ab41f2..cafce7ad33dac5de7eaf853f7f2b94854310acad 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 3460147dcff72bb4ff0bccdf4784d210e4e12869..52bb6fa240859164660722cef63d8894ea4dd48e 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 a189e9493b45636487110ffda0ce8df92b7dbc76..b2d03e0f8073c8e3b9d547d6a58f8b16a3409bc7 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 a5013d3ca105e2572c21426a60d2809d159433da..304d14fb422eb74ed52336b12fb4b4b8b43f6d0e 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 4dd7b93d8fcfd83485cb597e6d4efc29fab868bb..d6d2536110eab3feee5816f2315bdb8d50972e3b 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 49f4007ee73f1c5ff77684db404dfabfcff5903c..11b592d5f9957789badaf9fd961d979791233a23 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 b632301dcc284f662e23fb18947408627cc19d8e..b74e7eaf839616288d4e02fd4495997fab6b1122 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 d8fdb9b44e00bc79929468b8584710f5f413aa7a..da95dd5858bb667984d6d49b12f659a1a334ef3f 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 0b8ad58a834b61299f39661702fc94b0d2ac4360..8b38ada9140bcfa9cb88c0fc57fdca25160ac52c 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 e06de64ac21ed5be3e2720f82cc57525f7eeb3f7..bc498b47d682138664a96f4e7dc14083f6511947 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 c1ebdd11bf6043c003dfbfa97e7c93ecf286cce3..653d3c049647b4f7db6f1ef8ace485e0af26b8e2 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 d19561d6cadde712d4754f36787754602d13adc8..6cad9734b4a0820b0a73a03743ad2eb3f5a53e1d 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 4a8494e4c2a1bb0b41a0e04c7695954faf220709..c60620fa1852fe6b2ab02377790c38f6582b5962 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 55b9db239080ff11622c7f1b488cfa4b59d44c92..4fc8f85021f2ebacb05a46b55886097acdf68b3e 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 5d2428fdb531f47c5ab9d770f7be3d15baea08b4..f0b4a0532b4a4fdc336251bedfea12e23a68f5c6 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 2cf54373ecdeb8e88ed9c2b8efcbd2ef3026d948..4e9bc4be7384e064ef4fb748446e8659414aacc6 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 c27798ba77e3a2e90444e8a75f263b9853b549fb..bde7afd820295c2f5751ecf80bfebc891f33a4d9 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 221602455c6e46e19c119c61f47ef2f0f3e6d886..c912c75f36042642f96bf56bf6db008212f6a612 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 afc3005688889ad1e94ccda4528526641eb778b5..bd4f3934cc3cc9b5667a2054c0a7b3cba228421f 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 63070c688318246ee6923d4a9292bc6bddd004f2..18d35a212271bdff579dedf494f0c4d204d2aed7 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 172b47a6440f7f90541f9d2b9c600bdbe9a32e16..97b9de7076941ff4497f5fa9b3be97b2e332c7f6 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 280f2d7c811318a6f77ee15516b24bae5d731c85..30ccb53197ee97e1431be7745033561c5646f97b 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 7abf5d7aee8af6202f55ac6e4908544cff9da8c9..c77a50b2d6dab1210d9f0f535e898ea1475e38da 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 a14aa6669cc5ffa5d61f0d3376685026a5e8af13..8651aa4996ef188e4bbe75ec709eba08dd46d73d 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 2c3d3290f376472be3b7a8016826fe2828872385..bc4712dd5b511031d00527ac8b871dad2c3cf0e2 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)