From 2d22d3af60fb2c225b60a950a96e21eb7376865d Mon Sep 17 00:00:00 2001 From: Cam Saul <1455846+camsaul@users.noreply.github.com> Date: Tue, 3 Jan 2023 08:20:35 -0800 Subject: [PATCH] [Toucan 2 Prep] Add helper for extending `IModel`; namespace Toucan properties (#27474) * [Toucan 2 Prep] Add helper for extending `IModel`; namespace Toucan properties * Clean namespaces * Test fixes :wrench: --- .../models/disallow_type_or_class.clj | 1 + .dir-locals.el | 2 +- .../models/group_table_access_policy.clj | 12 ++--- .../models/entity_id_test.clj | 11 ++-- src/metabase/models/action.clj | 51 +++++++++---------- src/metabase/models/activity.clj | 10 ++-- src/metabase/models/app.clj | 17 +++---- .../application_permissions_revision.clj | 15 +++--- src/metabase/models/card.clj | 41 ++++++++------- src/metabase/models/collection.clj | 21 ++++---- src/metabase/models/collection/root.clj | 4 +- .../collection_permission_graph_revision.clj | 15 +++--- src/metabase/models/dashboard.clj | 24 ++++----- src/metabase/models/dashboard_card.clj | 15 +++--- src/metabase/models/database.clj | 33 ++++++------ src/metabase/models/dimension.clj | 13 +++-- src/metabase/models/field.clj | 29 +++++------ src/metabase/models/field_values.clj | 20 ++++---- src/metabase/models/interface.clj | 30 +++++++++-- src/metabase/models/metric.clj | 14 +++-- .../models/metric_important_field.clj | 8 ++- src/metabase/models/moderation_review.clj | 11 ++-- src/metabase/models/native_query_snippet.clj | 14 +++-- src/metabase/models/parameter_card.clj | 14 ++--- src/metabase/models/permissions.clj | 10 ++-- src/metabase/models/permissions_group.clj | 11 ++-- .../models/permissions_group_membership.clj | 12 ++--- src/metabase/models/permissions_revision.clj | 15 +++--- src/metabase/models/persisted_info.clj | 7 ++- src/metabase/models/pulse.clj | 18 +++---- src/metabase/models/pulse_card.clj | 8 +-- src/metabase/models/pulse_channel.clj | 23 +++++---- .../models/pulse_channel_recipient.clj | 10 ++-- src/metabase/models/query.clj | 11 ++-- src/metabase/models/query_cache.clj | 9 ++-- src/metabase/models/query_execution.clj | 14 ++--- src/metabase/models/revision.clj | 13 +++-- src/metabase/models/secret.clj | 15 +++--- src/metabase/models/segment.clj | 16 +++--- src/metabase/models/session.clj | 16 +++--- src/metabase/models/setting.clj | 10 ++-- src/metabase/models/table.clj | 19 ++++--- src/metabase/models/task_history.clj | 9 ++-- src/metabase/models/timeline.clj | 12 ++--- src/metabase/models/timeline_event.clj | 10 ++-- src/metabase/models/user.clj | 24 ++++----- src/metabase/models/view_log.clj | 10 ++-- src/metabase/util.clj | 25 --------- 48 files changed, 351 insertions(+), 401 deletions(-) diff --git a/.clj-kondo/hooks/metabase/models/disallow_type_or_class.clj b/.clj-kondo/hooks/metabase/models/disallow_type_or_class.clj index 2289fb91c4b..521c2100f10 100644 --- a/.clj-kondo/hooks/metabase/models/disallow_type_or_class.clj +++ b/.clj-kondo/hooks/metabase/models/disallow_type_or_class.clj @@ -23,6 +23,7 @@ Emitter Field FieldValues + GroupTableAccessPolicy HTTPAction LoginHistory Metric diff --git a/.dir-locals.el b/.dir-locals.el index ec6fab4945c..cd94e400041 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -48,6 +48,7 @@ (eval . (put-clojure-indent 'mbql.match/match-one 1)) (eval . (put-clojure-indent 'mbql.match/replace 1)) (eval . (put-clojure-indent 'mbql.match/replace-in 2)) + (eval . (put-clojure-indent 'mi/define-methods '(:form))) (eval . (put-clojure-indent 'mt/dataset 1)) (eval . (put-clojure-indent 'mt/format-rows-by 1)) (eval . (put-clojure-indent 'mt/query 1)) @@ -57,7 +58,6 @@ (eval . (put-clojure-indent 'qp.streaming/streaming-response 1)) (eval . (put-clojure-indent 'u/prog1 1)) (eval . (put-clojure-indent 'u/select-keys-when 1)) - (eval . (put-clojure-indent 'u/strict-extend 1)) (eval . (put-clojure-indent 'with-meta '(:form))) ;; these ones have to be done with `define-clojure-indent' for now because of upstream bug ;; https://github.com/clojure-emacs/clojure-mode/issues/600 once that's resolved we should use `put-clojure-indent' diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/models/group_table_access_policy.clj b/enterprise/backend/src/metabase_enterprise/sandbox/models/group_table_access_policy.clj index 50e76c33895..d9612b2f34c 100644 --- a/enterprise/backend/src/metabase_enterprise/sandbox/models/group_table_access_policy.clj +++ b/enterprise/backend/src/metabase_enterprise/sandbox/models/group_table_access_policy.clj @@ -137,10 +137,8 @@ (when (:card_id updates) (check-columns-match-table updated))))) -(u/strict-extend (class GroupTableAccessPolicy) - models/IModel - (merge - models/IModelDefaults - {:types (constantly {:attribute_remappings ::attribute-remappings}) - :pre-insert pre-insert - :pre-update pre-update})) +(mi/define-methods + GroupTableAccessPolicy + {:types (constantly {:attribute_remappings ::attribute-remappings}) + :pre-insert pre-insert + :pre-update pre-update}) 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 5b770a5795c..9a1f2684172 100644 --- a/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj +++ b/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj @@ -8,9 +8,10 @@ [clojure.test :refer :all] [metabase.db.data-migrations] [metabase.models] + [metabase.models.interface :as mi] [metabase.models.revision-test] [metabase.models.serialization.hash :as serdes.hash] - [toucan.models :refer [IModel]])) + [toucan.models :as models :refer [IModel]])) (comment metabase.models/keep-me metabase.db.data-migrations/keep-me @@ -73,11 +74,11 @@ (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" + (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") (.getSimpleName model)) - (is (= true (-> (.newInstance model) - toucan.models/properties - :entity_id)))))) + (is (contains? (set (keys (models/properties (.newInstance model)))) + ::mi/entity-id))))) (deftest comprehensive-identity-hash-test (doseq [^Class model (->> (extenders IModel) diff --git a/src/metabase/models/action.clj b/src/metabase/models/action.clj index 2f9b93b5b10..7d5f108f846 100644 --- a/src/metabase/models/action.clj +++ b/src/metabase/models/action.clj @@ -37,12 +37,11 @@ (u/update-if-exists template :parameters (mi/catch-normalization-exceptions mi/normalize-parameters-list))) mi/json-out-with-keywordization)) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Action) - models/IModel - (merge models/IModelDefaults - {:pre-insert (fn [action] (check-data-apps-enabled) action) - :types (constantly {:type :keyword}) - :properties (constantly {:timestamped? true})})) +(mi/define-methods + Action + {:pre-insert (fn [action] (check-data-apps-enabled) action) + :types (constantly {:type :keyword}) + :properties (constantly {::mi/timestamped? true})}) (defn- pre-update [action] @@ -57,27 +56,25 @@ (def ^:private Action-subtype-IModel-impl "[[models/IModel]] impl for `HTTPAction` and `QueryAction`" - (merge models/IModelDefaults - {:primary-key (constantly :action_id) ; This is ok as long as we're 1:1 - :pre-delete pre-delete - :pre-update pre-update})) - -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class QueryAction) - models/IModel - Action-subtype-IModel-impl) - -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class HTTPAction) - models/IModel - (merge Action-subtype-IModel-impl - {:types (constantly {:template ::json-with-nested-parameters})})) - -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ModelAction) - models/IModel - (merge models/IModelDefaults - {:pre-insert (fn [model-action] (check-data-apps-enabled) model-action) - :properties (constantly {:entity_id true}) - :types (constantly {:parameter_mappings :parameters-list - :visualization_settings :visualization-settings})})) + {:primary-key (constantly :action_id) ; This is ok as long as we're 1:1 + :pre-delete pre-delete + :pre-update pre-update}) + +(mi/define-methods + QueryAction + Action-subtype-IModel-impl) + +(mi/define-methods + HTTPAction + (merge Action-subtype-IModel-impl + {:types (constantly {:template ::json-with-nested-parameters})})) + +(mi/define-methods + ModelAction + {:pre-insert (fn [model-action] (check-data-apps-enabled) model-action) + :properties (constantly {::mi/entity-id true}) + :types (constantly {:parameter_mappings :parameters-list + :visualization_settings :visualization-settings})}) ;;; 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 diff --git a/src/metabase/models/activity.clj b/src/metabase/models/activity.clj index 41d3ac04cf8..87b10148b2e 100644 --- a/src/metabase/models/activity.clj +++ b/src/metabase/models/activity.clj @@ -8,7 +8,6 @@ [metabase.models.metric :refer [Metric]] [metabase.models.pulse :refer [Pulse]] [metabase.models.segment :refer [Segment]] - [metabase.util :as u] [toucan.db :as db] [toucan.models :as models])) @@ -53,11 +52,10 @@ :details {}}] (merge defaults activity))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Activity) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:details :json, :topic :keyword}) - :pre-insert pre-insert})) +(mi/define-methods + Activity + {:types (constantly {:details :json, :topic :keyword}) + :pre-insert pre-insert}) (defmethod mi/can-read? Activity [& args] diff --git a/src/metabase/models/app.clj b/src/metabase/models/app.clj index 984d34f8eeb..ae1b460f708 100644 --- a/src/metabase/models/app.clj +++ b/src/metabase/models/app.clj @@ -1,10 +1,10 @@ (ns metabase.models.app (:require [metabase.models.action :as action] + [metabase.models.interface :as mi] [metabase.models.permissions :as perms] [metabase.models.query :as query] [metabase.models.serialization.hash :as serdes.hash] - [metabase.util :as u] [toucan.db :as db] [toucan.models :as models])) @@ -13,14 +13,13 @@ ;;; You can read/write an App if you can read/write its Collection (derive App ::perms/use-parent-collection-perms) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class App) - models/IModel - (merge models/IModelDefaults - {:pre-insert (fn [app] (action/check-data-apps-enabled) app) - :types (constantly {:options :json - :nav_items :json}) - :properties (constantly {:timestamped? true - :entity_id true})})) +(mi/define-methods + App + {:pre-insert (fn [app] (action/check-data-apps-enabled) app) + :types (constantly {:options :json + :nav_items :json}) + :properties (constantly {::mi/timestamped? true + ::mi/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. diff --git a/src/metabase/models/application_permissions_revision.clj b/src/metabase/models/application_permissions_revision.clj index 40f4ebdcf1c..f0c28c69700 100644 --- a/src/metabase/models/application_permissions_revision.clj +++ b/src/metabase/models/application_permissions_revision.clj @@ -1,19 +1,18 @@ (ns metabase.models.application-permissions-revision (:require - [metabase.util :as u] + [metabase.models.interface :as mi] [metabase.util.i18n :refer [tru]] [toucan.db :as db] [toucan.models :as models])) (models/defmodel ApplicationPermissionsRevision :application_permissions_revision) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ApplicationPermissionsRevision) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:before :json - :after :json}) - :properties (constantly {:created-at-timestamped? true}) - :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a ApplicationPermissionsRevision!"))))})) +(mi/define-methods + ApplicationPermissionsRevision + {:types (constantly {:before :json + :after :json}) + :properties (constantly {::mi/created-at-timestamped? true}) + :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a ApplicationPermissionsRevision!"))))}) (defn latest-id "Return the ID of the newest `ApplicationPermissionsRevision`, or zero if none have been made yet. diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index 07a82cea58d..8e012c0ff5e 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -305,27 +305,26 @@ :in mi/json-in :out result-metadata-out) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Card) - models/IModel - (merge models/IModelDefaults - {:hydration-keys (constantly [:card]) - :types (constantly {:dataset_query :metabase-query - :display :keyword - :embedding_params :json - :query_type :keyword - :result_metadata ::result-metadata - :visualization_settings :visualization-settings - :parameters :parameters-list - :parameter_mappings :parameters-list}) - :properties (constantly {:timestamped? true - :entity_id true}) - ;; Make sure we normalize the query before calling `pre-update` or `pre-insert` because some of the - ;; functions those fns call assume normalized queries - :pre-update (comp populate-query-fields pre-update populate-result-metadata maybe-normalize-query) - :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})) +(mi/define-methods + Card + {:hydration-keys (constantly [:card]) + :types (constantly {:dataset_query :metabase-query + :display :keyword + :embedding_params :json + :query_type :keyword + :result_metadata ::result-metadata + :visualization_settings :visualization-settings + :parameters :parameters-list + :parameter_mappings :parameters-list}) + :properties (constantly {::mi/timestamped? true + ::mi/entity-id true}) + ;; Make sure we normalize the query before calling `pre-update` or `pre-insert` because some of the + ;; functions those fns call assume normalized queries + :pre-update (comp populate-query-fields pre-update populate-result-metadata maybe-normalize-query) + :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}) (defmethod serdes.hash/identity-hash-fields Card [_card] diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj index ac861c477d1..a321115fbec 100644 --- a/src/metabase/models/collection.clj +++ b/src/metabase/models/collection.clj @@ -899,17 +899,16 @@ [_collection] [:name :namespace parent-identity-hash :created_at]) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Collection) - models/IModel - (merge models/IModelDefaults - {:hydration-keys (constantly [:collection]) - :types (constantly {:namespace :keyword - :authority_level :keyword}) - :properties (constantly {:entity_id true}) - :pre-insert pre-insert - :post-insert post-insert - :pre-update pre-update - :pre-delete pre-delete})) +(mi/define-methods + Collection + {:hydration-keys (constantly [:collection]) + :types (constantly {:namespace :keyword + :authority_level :keyword}) + :properties (constantly {::mi/entity-id true}) + :pre-insert pre-insert + :post-insert post-insert + :pre-update pre-update + :pre-delete pre-delete}) (defmethod serdes.base/extract-query "Collection" [_ {:keys [collection-set]}] (if (seq collection-set) diff --git a/src/metabase/models/collection/root.clj b/src/metabase/models/collection/root.clj index bb22bea35ec..bb2b22d0965 100644 --- a/src/metabase/models/collection/root.clj +++ b/src/metabase/models/collection/root.clj @@ -38,11 +38,11 @@ :read perms/collection-read-path :write perms/collection-readwrite-path) collection)})) -(u/strict-extend RootCollection +(extend RootCollection models/IModel (merge models/IModelDefaults - {:types {:type :keyword}})) + {:types {:type :keyword}})) (def ^RootCollection root-collection "Special placeholder object representing the Root Collection, which isn't really a real Collection." diff --git a/src/metabase/models/collection_permission_graph_revision.clj b/src/metabase/models/collection_permission_graph_revision.clj index 47b8b671e38..1b740fe586b 100644 --- a/src/metabase/models/collection_permission_graph_revision.clj +++ b/src/metabase/models/collection_permission_graph_revision.clj @@ -1,19 +1,18 @@ (ns metabase.models.collection-permission-graph-revision (:require - [metabase.util :as u] + [metabase.models.interface :as mi] [metabase.util.i18n :refer [tru]] [toucan.db :as db] [toucan.models :as models])) (models/defmodel CollectionPermissionGraphRevision :collection_permission_graph_revision) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class CollectionPermissionGraphRevision) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:before :json - :after :json}) - :properties (constantly {:created-at-timestamped? true}) - :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a CollectionPermissionGraphRevision!"))))})) +(mi/define-methods + CollectionPermissionGraphRevision + {:types (constantly {:before :json + :after :json}) + :properties (constantly {::mi/created-at-timestamped? true}) + :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a CollectionPermissionGraphRevision!"))))}) (defn latest-id "Return the ID of the newest `CollectionPermissionGraphRevision`, or zero if none have been made yet. diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj index d58f31a2817..2a159ceac34 100644 --- a/src/metabase/models/dashboard.clj +++ b/src/metabase/models/dashboard.clj @@ -14,6 +14,7 @@ :as dashboard-card :refer [DashboardCard]] [metabase.models.field-values :as field-values] + [metabase.models.interface :as mi] [metabase.models.parameter-card :as parameter-card :refer [ParameterCard]] @@ -163,18 +164,17 @@ (assoc-in param [:values_source_config :card_id] (get param-id->card-id id (:card_id values_source_config))) param))))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Dashboard) - models/IModel - (merge models/IModelDefaults - {:properties (constantly {:timestamped? true - :entity_id true}) - :types (constantly {:parameters :parameters-list, :embedding_params :json}) - :pre-delete pre-delete - :pre-insert pre-insert - :post-insert post-insert - :pre-update pre-update - :post-update post-update - :post-select (comp populate-card-id-for-parameters public-settings/remove-public-uuid-if-public-sharing-is-disabled)})) +(mi/define-methods + Dashboard + {:properties (constantly {::mi/timestamped? true + ::mi/entity-id true}) + :types (constantly {:parameters :parameters-list, :embedding_params :json}) + :pre-delete pre-delete + :pre-insert pre-insert + :post-insert post-insert + :pre-update pre-update + :post-update post-update + :post-select (comp populate-card-id-for-parameters public-settings/remove-public-uuid-if-public-sharing-is-disabled)}) (defmethod serdes.hash/identity-hash-fields Dashboard [_dashboard] diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj index 7cc72c73418..1f324998e29 100644 --- a/src/metabase/models/dashboard_card.clj +++ b/src/metabase/models/dashboard_card.clj @@ -43,14 +43,13 @@ :visualization_settings {}}] (merge defaults dashcard))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class DashboardCard) - models/IModel - (merge models/IModelDefaults - {:properties (constantly {:timestamped? true - :entity_id true}) - :types (constantly {:parameter_mappings :parameters-list - :visualization_settings :visualization-settings}) - :pre-insert pre-insert})) +(mi/define-methods + DashboardCard + {:properties (constantly {::mi/timestamped? true + ::mi/entity-id true}) + :types (constantly {:parameter_mappings :parameters-list + :visualization_settings :visualization-settings}) + :pre-insert pre-insert}) (defmethod serdes.hash/identity-hash-fields DashboardCard [_dashboard-card] diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj index 6f790ef7d9f..62ecebab51c 100644 --- a/src/metabase/models/database.clj +++ b/src/metabase/models/database.clj @@ -197,23 +197,22 @@ :read (perms/data-perms-path db-id) :write (perms/db-details-write-perms-path db-id))}) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Database) - models/IModel - (merge models/IModelDefaults - {:hydration-keys (constantly [:database :db]) - :types (constantly {:details :encrypted-json - :options :json - :engine :keyword - :metadata_sync_schedule :cron-string - :cache_field_values_schedule :cron-string - :start_of_week :keyword - :settings :encrypted-json - :dbms_version :json}) - :post-insert post-insert - :post-select post-select - :pre-insert pre-insert - :pre-update pre-update - :pre-delete pre-delete})) +(mi/define-methods + Database + {:hydration-keys (constantly [:database :db]) + :types (constantly {:details :encrypted-json + :options :json + :engine :keyword + :metadata_sync_schedule :cron-string + :cache_field_values_schedule :cron-string + :start_of_week :keyword + :settings :encrypted-json + :dbms_version :json}) + :post-insert post-insert + :post-select post-select + :pre-insert pre-insert + :pre-update pre-update + :pre-delete pre-delete}) (defmethod serdes.hash/identity-hash-fields Database [_database] diff --git a/src/metabase/models/dimension.clj b/src/metabase/models/dimension.clj index f54f4140fdd..ec3e7854677 100644 --- a/src/metabase/models/dimension.clj +++ b/src/metabase/models/dimension.clj @@ -3,10 +3,10 @@ Query Processor. For a more detailed explanation, refer to the documentation in `metabase.query-processor.middleware.add-dimension-projections`." (:require + [metabase.models.interface :as mi] [metabase.models.serialization.base :as serdes.base] [metabase.models.serialization.hash :as serdes.hash] [metabase.models.serialization.util :as serdes.util] - [metabase.util :as u] [metabase.util.date-2 :as u.date] [toucan.models :as models])) @@ -17,12 +17,11 @@ (models/defmodel Dimension :dimension) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Dimension) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:type :keyword}) - :properties (constantly {:timestamped? true - :entity_id true})})) +(mi/define-methods + Dimension + {:types (constantly {:type :keyword}) + :properties (constantly {::mi/timestamped? true + ::mi/entity-id true})}) (defmethod serdes.hash/identity-hash-fields Dimension [_dimension] diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj index 62f45c429f7..d0cd88a7044 100644 --- a/src/metabase/models/field.clj +++ b/src/metabase/models/field.clj @@ -181,21 +181,20 @@ :out (comp update-semantic-numeric-values mi/json-out-with-keywordization)) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Field) - models/IModel - (merge models/IModelDefaults - {:hydration-keys (constantly [:destination :field :origin :human_readable_field]) - :types (constantly {:base_type ::base-type - :effective_type ::effective-type - :coercion_strategy ::coercion-strategy - :semantic_type ::semantic-type - :visibility_type :keyword - :has_field_values :keyword - :fingerprint :json-for-fingerprints - :settings :json - :nfc_path :json}) - :properties (constantly {:timestamped? true}) - :pre-insert pre-insert})) +(mi/define-methods + Field + {:hydration-keys (constantly [:destination :field :origin :human_readable_field]) + :types (constantly {:base_type ::base-type + :effective_type ::effective-type + :coercion_strategy ::coercion-strategy + :semantic_type ::semantic-type + :visibility_type :keyword + :has_field_values :keyword + :fingerprint :json-for-fingerprints + :settings :json + :nfc_path :json}) + :properties (constantly {::mi/timestamped? true}) + :pre-insert pre-insert}) (defmethod serdes.hash/identity-hash-fields Field [_field] diff --git a/src/metabase/models/field_values.clj b/src/metabase/models/field_values.clj index 37ebbeeefdf..06a0f5b5d27 100644 --- a/src/metabase/models/field_values.clj +++ b/src/metabase/models/field_values.clj @@ -22,6 +22,7 @@ [clojure.string :as str] [clojure.tools.logging :as log] [java-time :as t] + [metabase.models.interface :as mi] [metabase.models.serialization.base :as serdes.base] [metabase.models.serialization.hash :as serdes.hash] [metabase.models.serialization.util :as serdes.util] @@ -164,16 +165,15 @@ :else []))))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class FieldValues) - models/IModel - (merge models/IModelDefaults - {:properties (constantly {:timestamped? true}) - :types (constantly {:human_readable_values :json-no-keywordization - :values :json - :type :keyword}) - :pre-insert pre-insert - :pre-update pre-update - :post-select post-select})) +(mi/define-methods + FieldValues + {:properties (constantly {::mi/timestamped? true}) + :types (constantly {:human_readable_values :json-no-keywordization + :values :json + :type :keyword}) + :pre-insert pre-insert + :pre-update pre-update + :post-select post-select}) (defmethod serdes.hash/identity-hash-fields FieldValues [_field-values] diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj index 709cacab6b5..f816aa7883a 100644 --- a/src/metabase/models/interface.clj +++ b/src/metabase/models/interface.clj @@ -266,16 +266,16 @@ (cond-> obj (not (:updated_at obj)) (assoc :updated_at (now)))) -(models/add-property! :timestamped? +(models/add-property! ::timestamped? :insert (comp add-created-at-timestamp add-updated-at-timestamp) :update add-updated-at-timestamp) ;; like `timestamped?`, but for models that only have an `:created_at` column -(models/add-property! :created-at-timestamped? +(models/add-property! ::created-at-timestamped? :insert add-created-at-timestamp) ;; like `timestamped?`, but for models that only have an `:updated_at` column -(models/add-property! :updated-at-timestamped? +(models/add-property! ::updated-at-timestamped? :insert add-updated-at-timestamp :update add-updated-at-timestamp) @@ -287,7 +287,7 @@ obj (assoc obj :entity_id (u/generate-nano-id)))) -(models/add-property! :entity_id +(models/add-property! ::entity-id :insert add-entity-id) ;;; +----------------------------------------------------------------------------------------------------------------+ @@ -467,6 +467,28 @@ [_model _m] (superuser?)) +;;;; [[define-methods]] + +(defn- validate-properties [properties] + (doseq [k (keys properties)] + (assert (namespace k) "All :properties keys should be namespaced!") + (assert (contains? (set (keys @@#'models/property-fns)) k) + (str "Invalid property: " k)))) + +(defn define-methods + "Helper for defining [[toucan.models/IModel]] methods for a `model`. Prefer this over using `extend` directly, because + it's easier to swap a single function when we make the switch to Toucan 2 in the future than to update all the + various model namespaces." + {:style/indent [:form]} + [model method-map] + (when-let [properties-method (:properties method-map)] + (validate-properties (properties-method model))) + (extend (class model) + models/IModel + (merge + models/IModelDefaults + method-map))) + ;;;; redefs diff --git a/src/metabase/models/metric.clj b/src/metabase/models/metric.clj index d3094d0863f..a46e267e71e 100644 --- a/src/metabase/models/metric.clj +++ b/src/metabase/models/metric.clj @@ -35,14 +35,12 @@ (db/select-one ['Table :db_id :schema :id] :id (u/the-id (:table_id metric))))] (mi/perms-objects-set table read-or-write))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Metric) - models/IModel - (merge - models/IModelDefaults - {:types (constantly {:definition :metric-segment-definition}) - :properties (constantly {:timestamped? true - :entity_id true}) - :pre-update pre-update})) +(mi/define-methods + Metric + {:types (constantly {:definition :metric-segment-definition}) + :properties (constantly {::mi/timestamped? true + ::mi/entity-id true}) + :pre-update pre-update}) (defmethod serdes.hash/identity-hash-fields Metric [_metric] diff --git a/src/metabase/models/metric_important_field.clj b/src/metabase/models/metric_important_field.clj index 63e6ab9d935..bf88ba2ce75 100644 --- a/src/metabase/models/metric_important_field.clj +++ b/src/metabase/models/metric_important_field.clj @@ -2,7 +2,6 @@ "Intersection table for `Metric` and `Field`; this is used to keep track of the top 0-3 important fields for a metric as shown in the Getting Started guide." (:require [metabase.models.interface :as mi] - [metabase.util :as u] [toucan.models :as models])) (models/defmodel MetricImportantField :metric_important_field) @@ -11,7 +10,6 @@ (derive ::mi/read-policy.always-allow) (derive ::mi/write-policy.superuser)) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class MetricImportantField) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:definition :json})})) +(mi/define-methods + MetricImportantField + {:types (constantly {:definition :json})}) diff --git a/src/metabase/models/moderation_review.clj b/src/metabase/models/moderation_review.clj index 047a571d984..17b21f4a41b 100644 --- a/src/metabase/models/moderation_review.clj +++ b/src/metabase/models/moderation_review.clj @@ -2,9 +2,9 @@ "TODO -- this should be moved to `metabase-enterprise.content-management.models.moderation-review` since it's a premium-only model." (:require + [metabase.models.interface :as mi] [metabase.models.permissions :as perms] [metabase.moderation :as moderation] - [metabase.util :as u] [metabase.util.schema :as su] [schema.core :as s] [toucan.db :as db] @@ -33,11 +33,10 @@ ;;; TODO: this is wrong, but what should it be? (derive ModerationReview ::perms/use-parent-collection-perms) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ModerationReview) - models/IModel - (merge models/IModelDefaults - {:properties (constantly {:timestamped? true}) - :types (constantly {:moderated_item_type :keyword})})) +(mi/define-methods + ModerationReview + {:properties (constantly {::mi/timestamped? true}) + :types (constantly {:moderated_item_type :keyword})}) (def max-moderation-reviews "The amount of moderation reviews we will keep on hand." diff --git a/src/metabase/models/native_query_snippet.clj b/src/metabase/models/native_query_snippet.clj index aefed42cf73..e1d6328b806 100644 --- a/src/metabase/models/native_query_snippet.clj +++ b/src/metabase/models/native_query_snippet.clj @@ -33,14 +33,12 @@ (throw (UnsupportedOperationException. (tru "You cannot update the creator_id of a NativeQuerySnippet."))))) (collection/check-collection-namespace NativeQuerySnippet (:collection_id updates)))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class NativeQuerySnippet) - models/IModel - (merge - models/IModelDefaults - {:properties (constantly {:timestamped? true - :entity_id true}) - :pre-insert pre-insert - :pre-update pre-update})) +(mi/define-methods + NativeQuerySnippet + {:properties (constantly {::mi/timestamped? true + ::mi/entity-id true}) + :pre-insert pre-insert + :pre-update pre-update}) (defmethod serdes.hash/identity-hash-fields NativeQuerySnippet [_snippet] diff --git a/src/metabase/models/parameter_card.clj b/src/metabase/models/parameter_card.clj index 8938fc681d0..166a297b518 100644 --- a/src/metabase/models/parameter_card.clj +++ b/src/metabase/models/parameter_card.clj @@ -1,5 +1,6 @@ (ns metabase.models.parameter-card (:require + [metabase.models.interface :as mi] [metabase.util :as u] [metabase.util.i18n :refer [tru]] [toucan.db :as db] @@ -28,13 +29,12 @@ (u/prog1 pc (validate-parameterized-object-type pc))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ParameterCard) - models/IModel - (merge models/IModelDefaults - {:properties (constantly {:timestamped? true}) - :types (constantly {:parameterized_object_type :keyword}) - :pre-insert pre-insert - :pre-update pre-update})) +(mi/define-methods + ParameterCard + {:properties (constantly {::mi/timestamped? true}) + :types (constantly {:parameterized_object_type :keyword}) + :pre-insert pre-insert + :pre-update pre-update}) (defn delete-all-for-parameterized-object! "Delete all ParameterCard for a give Parameterized Object and NOT listed in the optional diff --git a/src/metabase/models/permissions.clj b/src/metabase/models/permissions.clj index 724850802eb..4ac06d81b1c 100644 --- a/src/metabase/models/permissions.clj +++ b/src/metabase/models/permissions.clj @@ -610,11 +610,11 @@ (:object permissions)))) (assert-not-admin-group permissions)) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Permissions) - models/IModel (merge models/IModelDefaults - {:pre-insert pre-insert - :pre-update pre-update - :pre-delete pre-delete})) +(mi/define-methods + Permissions + {:pre-insert pre-insert + :pre-update pre-update + :pre-delete pre-delete}) ;;; +----------------------------------------------------------------------------------------------------------------+ diff --git a/src/metabase/models/permissions_group.clj b/src/metabase/models/permissions_group.clj index 06d9fd40490..20198f3d9c1 100644 --- a/src/metabase/models/permissions_group.clj +++ b/src/metabase/models/permissions_group.clj @@ -11,6 +11,7 @@ [clojure.string :as str] [honeysql.helpers :as hh] [metabase.db.connection :as mdb.connection] + [metabase.models.interface :as mi] [metabase.models.setting :as setting] [metabase.plugins.classloader :as classloader] [metabase.public-settings.premium-features :as premium-features] @@ -100,11 +101,11 @@ (when group-name (check-name-not-already-taken group-name)))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PermissionsGroup) - models/IModel (merge models/IModelDefaults - {:pre-delete pre-delete - :pre-insert pre-insert - :pre-update pre-update})) +(mi/define-methods + PermissionsGroup + {:pre-delete pre-delete + :pre-insert pre-insert + :pre-update pre-update}) ;;; ---------------------------------------------------- Util Fns ---------------------------------------------------- diff --git a/src/metabase/models/permissions_group_membership.clj b/src/metabase/models/permissions_group_membership.clj index 9b5f2c5c421..ad15e72c255 100644 --- a/src/metabase/models/permissions_group_membership.clj +++ b/src/metabase/models/permissions_group_membership.clj @@ -1,5 +1,6 @@ (ns metabase.models.permissions-group-membership (:require + [metabase.models.interface :as mi] [metabase.models.permissions-group :as perms-group] [metabase.util :as u] [metabase.util.i18n :refer [deferred-tru tru]] @@ -54,9 +55,8 @@ (db/update! 'User user_id :is_superuser true)))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PermissionsGroupMembership) - models/IModel - (merge models/IModelDefaults - {:pre-delete pre-delete - :pre-insert pre-insert - :post-insert post-insert})) +(mi/define-methods + PermissionsGroupMembership + {:pre-delete pre-delete + :pre-insert pre-insert + :post-insert post-insert}) diff --git a/src/metabase/models/permissions_revision.clj b/src/metabase/models/permissions_revision.clj index 7f7bece3854..feaa7172ee9 100644 --- a/src/metabase/models/permissions_revision.clj +++ b/src/metabase/models/permissions_revision.clj @@ -1,19 +1,18 @@ (ns metabase.models.permissions-revision (:require - [metabase.util :as u] + [metabase.models.interface :as mi] [metabase.util.i18n :refer [tru]] [toucan.db :as db] [toucan.models :as models])) (models/defmodel PermissionsRevision :permissions_revision) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PermissionsRevision) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:before :json - :after :json}) - :properties (constantly {:created-at-timestamped? true}) - :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a PermissionsRevision!"))))})) +(mi/define-methods + PermissionsRevision + {:types (constantly {:before :json + :after :json}) + :properties (constantly {::mi/created-at-timestamped? true}) + :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a PermissionsRevision!"))))}) (defn latest-id "Return the ID of the newest `PermissionsRevision`, or zero if none have been made yet. diff --git a/src/metabase/models/persisted_info.clj b/src/metabase/models/persisted_info.clj index 700dbbe9993..92ec91fccb2 100644 --- a/src/metabase/models/persisted_info.clj +++ b/src/metabase/models/persisted_info.clj @@ -65,10 +65,9 @@ (models/defmodel PersistedInfo :persisted_info) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PersistedInfo) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:definition ::definition})})) +(mi/define-methods + PersistedInfo + {:types (constantly {:definition ::definition})}) (defn persisted? "Hydrate a card :is_persisted for the frontend." diff --git a/src/metabase/models/pulse.clj b/src/metabase/models/pulse.clj index 5690e9af483..6376eb74aa8 100644 --- a/src/metabase/models/pulse.clj +++ b/src/metabase/models/pulse.clj @@ -127,16 +127,14 @@ (= api/*current-user-id* (:creator_id notification)) (mi/current-user-has-full-permissions? :write notification))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Pulse) - models/IModel - (merge - models/IModelDefaults - {:hydration-keys (constantly [:pulse]) - :properties (constantly {:timestamped? true - :entity_id true}) - :pre-insert pre-insert - :pre-update pre-update - :types (constantly {:parameters :json})})) +(mi/define-methods + Pulse + {:hydration-keys (constantly [:pulse]) + :properties (constantly {::mi/timestamped? true + ::mi/entity-id true}) + :pre-insert pre-insert + :pre-update pre-update + :types (constantly {:parameters :json})}) (defmethod serdes.hash/identity-hash-fields Pulse [_pulse] diff --git a/src/metabase/models/pulse_card.clj b/src/metabase/models/pulse_card.clj index 39edf169180..ad1bb844f05 100644 --- a/src/metabase/models/pulse_card.clj +++ b/src/metabase/models/pulse_card.clj @@ -1,5 +1,6 @@ (ns metabase.models.pulse-card (:require + [metabase.models.interface :as mi] [metabase.models.serialization.base :as serdes.base] [metabase.models.serialization.hash :as serdes.hash] [metabase.models.serialization.util :as serdes.util] @@ -11,10 +12,9 @@ (models/defmodel PulseCard :pulse_card) -(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})})) +(mi/define-methods + PulseCard + {:properties (constantly {::mi/entity-id true})}) (defmethod serdes.hash/identity-hash-fields PulseCard [_pulse-card] diff --git a/src/metabase/models/pulse_channel.clj b/src/metabase/models/pulse_channel.clj index 1497ec2ea1f..0872ce957b6 100644 --- a/src/metabase/models/pulse_channel.clj +++ b/src/metabase/models/pulse_channel.clj @@ -185,17 +185,18 @@ (throw (ex-info (tru "Wrong email address for User {0}." id) {:status-code 403}))))))))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PulseChannel) - models/IModel - (merge - models/IModelDefaults - {:hydration-keys (constantly [:pulse_channel]) - :types (constantly {:details :json, :channel_type :keyword, :schedule_type :keyword, :schedule_frame :keyword}) - :properties (constantly {:timestamped? true - :entity_id true}) - :pre-delete pre-delete - :pre-insert validate-email-domains - :pre-update validate-email-domains})) +(mi/define-methods + PulseChannel + {:hydration-keys (constantly [:pulse_channel]) + :types (constantly {:details :json + :channel_type :keyword + :schedule_type :keyword + :schedule_frame :keyword}) + :properties (constantly {::mi/timestamped? true + ::mi/entity-id true}) + :pre-delete pre-delete + :pre-insert validate-email-domains + :pre-update validate-email-domains}) (defmethod serdes.hash/identity-hash-fields PulseChannel [_pulse-channel] diff --git a/src/metabase/models/pulse_channel_recipient.clj b/src/metabase/models/pulse_channel_recipient.clj index b9f3d4176bf..4480a19d984 100644 --- a/src/metabase/models/pulse_channel_recipient.clj +++ b/src/metabase/models/pulse_channel_recipient.clj @@ -1,7 +1,7 @@ (ns metabase.models.pulse-channel-recipient (:require + [metabase.models.interface :as mi] [metabase.plugins.classloader :as classloader] - [metabase.util :as u] [toucan.models :as models])) (models/defmodel PulseChannelRecipient :pulse_channel_recipient) @@ -12,8 +12,6 @@ (classloader/require 'metabase.models.pulse-channel) ((resolve 'metabase.models.pulse-channel/will-delete-recipient) pcr)) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PulseChannelRecipient) - models/IModel - (merge - models/IModelDefaults - {:pre-delete pre-delete})) +(mi/define-methods + PulseChannelRecipient + {:pre-delete pre-delete}) diff --git a/src/metabase/models/query.clj b/src/metabase/models/query.clj index ed277cd95e0..1ded80fff59 100644 --- a/src/metabase/models/query.clj +++ b/src/metabase/models/query.clj @@ -6,19 +6,16 @@ [metabase.db :as mdb] [metabase.mbql.normalize :as mbql.normalize] [metabase.models.interface :as mi] - [metabase.util :as u] [metabase.util.honeysql-extensions :as hx] [toucan.db :as db] [toucan.models :as models])) (models/defmodel Query :query) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Query) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:query :json}) - :primary-key (constantly :query_hash)})) - +(mi/define-methods + Query + {:types (constantly {:query :json}) + :primary-key (constantly :query_hash)}) ;;; Helper Fns diff --git a/src/metabase/models/query_cache.clj b/src/metabase/models/query_cache.clj index c3fb18732bb..964a4771a1d 100644 --- a/src/metabase/models/query_cache.clj +++ b/src/metabase/models/query_cache.clj @@ -1,12 +1,11 @@ (ns metabase.models.query-cache "A model used to cache query results in the database." (:require - [metabase.util :as u] + [metabase.models.interface :as mi] [toucan.models :as models])) (models/defmodel QueryCache :query_cache) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class QueryCache) - models/IModel - (merge models/IModelDefaults - {:properties (constantly {:updated-at-timestamped? true})})) +(mi/define-methods + QueryCache + {:properties (constantly {::mi/updated-at-timestamped? true})}) diff --git a/src/metabase/models/query_execution.clj b/src/metabase/models/query_execution.clj index b4c6816c3a0..f90785ce667 100644 --- a/src/metabase/models/query_execution.clj +++ b/src/metabase/models/query_execution.clj @@ -3,6 +3,7 @@ time, context it was executed in, etc." (:require [metabase.mbql.schema :as mbql.s] + [metabase.models.interface :as mi] [metabase.util :as u] [metabase.util.i18n :refer [tru]] [schema.core :as s] @@ -21,10 +22,9 @@ ;; sadly we have 2 ways to reference the row count :( (assoc query-execution :row_count (or result_rows 0))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class QueryExecution) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:json_query :json, :status :keyword, :context :keyword}) - :pre-insert pre-insert - :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a QueryExecution!")))) - :post-select post-select})) +(mi/define-methods + QueryExecution + {:types (constantly {:json_query :json, :status :keyword, :context :keyword}) + :pre-insert pre-insert + :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a QueryExecution!")))) + :post-select post-select}) diff --git a/src/metabase/models/revision.clj b/src/metabase/models/revision.clj index cf3d1795106..507a0d25d2c 100644 --- a/src/metabase/models/revision.clj +++ b/src/metabase/models/revision.clj @@ -71,13 +71,12 @@ (cond-> revision model (update :object (partial models/do-post-select model))))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Revision) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:object :json}) - :pre-insert pre-insert - :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a Revision!")))) - :post-select do-post-select-for-object})) +(mi/define-methods + Revision + {:types (constantly {:object :json}) + :pre-insert pre-insert + :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a Revision!")))) + :post-select do-post-select-for-object}) ;;; # Functions diff --git a/src/metabase/models/secret.clj b/src/metabase/models/secret.clj index eae60cebdd6..460e830d09a 100644 --- a/src/metabase/models/secret.clj +++ b/src/metabase/models/secret.clj @@ -27,15 +27,12 @@ (derive ::mi/read-policy.superuser) (derive ::mi/write-policy.superuser)) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Secret) - models/IModel - (merge models/IModelDefaults - { ;:hydration-keys (constantly [:database :db]) ; don't think there's any hydration going on since other models - ; won't have a direct secret-id column - :types (constantly {:value :secret-value - :kind :keyword - :source :keyword}) - :properties (constantly {:timestamped? true})})) +(mi/define-methods + Secret + {:types (constantly {:value :secret-value + :kind :keyword + :source :keyword}) + :properties (constantly {::mi/timestamped? true})}) ;;; ---------------------------------------------- Hydration / Util Fns ---------------------------------------------- diff --git a/src/metabase/models/segment.clj b/src/metabase/models/segment.clj index 9c106b251b8..ade79f34dc0 100644 --- a/src/metabase/models/segment.clj +++ b/src/metabase/models/segment.clj @@ -34,15 +34,13 @@ (db/select-one ['Table :db_id :schema :id] :id (u/the-id (:table_id segment))))] (mi/perms-objects-set table read-or-write))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Segment) - models/IModel - (merge - models/IModelDefaults - {:types (constantly {:definition :metric-segment-definition}) - :properties (constantly {:timestamped? true - :entity_id true}) - :hydration-keys (constantly [:segment]) - :pre-update pre-update})) +(mi/define-methods + Segment + {:types (constantly {:definition :metric-segment-definition}) + :properties (constantly {::mi/timestamped? true + ::mi/entity-id true}) + :hydration-keys (constantly [:segment]) + :pre-update pre-update}) (defmethod serdes.hash/identity-hash-fields Segment [_segment] diff --git a/src/metabase/models/session.clj b/src/metabase/models/session.clj index d5c861296ba..bb182890fe0 100644 --- a/src/metabase/models/session.clj +++ b/src/metabase/models/session.clj @@ -2,9 +2,9 @@ (:require [buddy.core.codecs :as codecs] [buddy.core.nonce :as nonce] + [metabase.models.interface :as mi] [metabase.server.middleware.misc :as mw.misc] [metabase.server.request.util :as request.u] - [metabase.util :as u] [schema.core :as s] [toucan.models :as models])) @@ -25,11 +25,9 @@ (let [session-type (if anti-csrf-token :full-app-embed :normal)] (assoc session :type session-type))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Session) - models/IModel - (merge - models/IModelDefaults - {:pre-insert pre-insert - :post-insert post-insert - :pre-update pre-update - :properties (constantly {:created-at-timestamped? true})})) +(mi/define-methods + Session + {:pre-insert pre-insert + :post-insert post-insert + :pre-update pre-update + :properties (constantly {::mi/created-at-timestamped? true})}) diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj index 47e37668cd5..94295e0e8c3 100644 --- a/src/metabase/models/setting.clj +++ b/src/metabase/models/setting.clj @@ -82,6 +82,7 @@ [environ.core :as env] [medley.core :as m] [metabase.api.common :as api] + [metabase.models.interface :as mi] [metabase.models.serialization.base :as serdes.base] [metabase.models.serialization.hash :as serdes.hash] [metabase.models.setting.cache :as setting.cache] @@ -141,11 +142,10 @@ "The model that underlies [[defsetting]]." :setting) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Setting) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:value :encrypted-text}) - :primary-key (constantly :key)})) +(mi/define-methods + Setting + {:types (constantly {:value :encrypted-text}) + :primary-key (constantly :key)}) (defmethod serdes.hash/identity-hash-fields Setting [_setting] diff --git a/src/metabase/models/table.clj b/src/metabase/models/table.clj index c06f79d352d..5ae21102980 100644 --- a/src/metabase/models/table.clj +++ b/src/metabase/models/table.clj @@ -70,16 +70,15 @@ :read (perms/table-read-path table) :write (perms/data-model-write-perms-path db-id schema table-id))}) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Table) - models/IModel - (merge models/IModelDefaults - {:hydration-keys (constantly [:table]) - :types (constantly {:entity_type :keyword - :visibility_type :keyword - :field_order :keyword}) - :properties (constantly {:timestamped? true}) - :pre-insert pre-insert - :pre-delete pre-delete})) +(mi/define-methods + Table + {:hydration-keys (constantly [:table]) + :types (constantly {:entity_type :keyword + :visibility_type :keyword + :field_order :keyword}) + :properties (constantly {::mi/timestamped? true}) + :pre-insert pre-insert + :pre-delete pre-delete}) (defmethod serdes.hash/identity-hash-fields Table [_table] diff --git a/src/metabase/models/task_history.clj b/src/metabase/models/task_history.clj index 5585d4f7f81..4422801683d 100644 --- a/src/metabase/models/task_history.clj +++ b/src/metabase/models/task_history.clj @@ -64,11 +64,10 @@ (u/prog1 task (snowplow/track-event! ::snowplow/new-task-history *current-user-id* (task->snowplow-event <>)))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class TaskHistory) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:task_details :json}) - :post-insert post-insert})) +(mi/define-methods + TaskHistory + {:types (constantly {:task_details :json}) + :post-insert post-insert}) (s/defn all "Return all TaskHistory entries, applying `limit` and `offset` if not nil" diff --git a/src/metabase/models/timeline.clj b/src/metabase/models/timeline.clj index 0bc9fec15c3..43670075c38 100644 --- a/src/metabase/models/timeline.clj +++ b/src/metabase/models/timeline.clj @@ -2,12 +2,12 @@ (:require [java-time :as t] [metabase.models.collection :as collection] + [metabase.models.interface :as mi] [metabase.models.permissions :as perms] [metabase.models.serialization.base :as serdes.base] [metabase.models.serialization.hash :as serdes.hash] [metabase.models.serialization.util :as serdes.util] [metabase.models.timeline-event :as timeline-event] - [metabase.util :as u] [metabase.util.date-2 :as u.date] [schema.core :as s] [toucan.db :as db] @@ -54,12 +54,10 @@ (nil? collection-id) (->> (map hydrate-root-collection)) events? (timeline-event/include-events options))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Timeline) - models/IModel - (merge - models/IModelDefaults - {:properties (constantly {:timestamped? true - :entity_id true})})) +(mi/define-methods + Timeline + {:properties (constantly {::mi/timestamped? true + ::mi/entity-id true})}) (defmethod serdes.hash/identity-hash-fields Timeline [_timeline] diff --git a/src/metabase/models/timeline_event.clj b/src/metabase/models/timeline_event.clj index a942f09a0e6..ef7a4f04771 100644 --- a/src/metabase/models/timeline_event.clj +++ b/src/metabase/models/timeline_event.clj @@ -4,7 +4,6 @@ [metabase.models.serialization.base :as serdes.base] [metabase.models.serialization.hash :as serdes.hash] [metabase.models.serialization.util :as serdes.util] - [metabase.util :as u] [metabase.util.date-2 :as u.date] [metabase.util.honeysql-extensions :as hx] [schema.core :as s] @@ -93,12 +92,9 @@ ;;;; model -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class TimelineEvent) - models/IModel - (merge - models/IModelDefaults - ;; todo: add hydration keys?? - {:properties (constantly {:timestamped? true})})) +(mi/define-methods + TimelineEvent + {:properties (constantly {::mi/timestamped? true})}) (defmethod serdes.hash/identity-hash-fields TimelineEvent [_timeline-event] diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj index 47aedf2161a..850cac22860 100644 --- a/src/metabase/models/user.clj +++ b/src/metabase/models/user.clj @@ -4,6 +4,7 @@ [clojure.string :as str] [clojure.tools.logging :as log] [metabase.models.collection :as collection] + [metabase.models.interface :as mi] [metabase.models.permissions :as perms] [metabase.models.permissions-group :as perms-group] [metabase.models.permissions-group-membership @@ -144,18 +145,17 @@ "Sequence of columns Group Managers can see when fetching a list of Users.." (into non-admin-or-self-visible-columns [:is_superuser :last_login])) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class User) - models/IModel - (merge models/IModelDefaults - {:default-fields (constantly default-user-columns) - :hydration-keys (constantly [:author :creator :user]) - :properties (constantly {:updated-at-timestamped? true}) - :pre-insert pre-insert - :post-insert post-insert - :pre-update pre-update - :post-select post-select - :types (constantly {:login_attributes :json-no-keywordization - :settings :encrypted-json})})) +(mi/define-methods + User + {:default-fields (constantly default-user-columns) + :hydration-keys (constantly [:author :creator :user]) + :properties (constantly {::mi/updated-at-timestamped? true}) + :pre-insert pre-insert + :post-insert post-insert + :pre-update pre-update + :post-select post-select + :types (constantly {:login_attributes :json-no-keywordization + :settings :encrypted-json})}) (defmethod serdes.hash/identity-hash-fields User [_user] diff --git a/src/metabase/models/view_log.clj b/src/metabase/models/view_log.clj index fb485a085b4..8d5ad1ceff0 100644 --- a/src/metabase/models/view_log.clj +++ b/src/metabase/models/view_log.clj @@ -2,7 +2,6 @@ "The ViewLog is used to log an event where a given User views a given object such as a Table or Card (Question)." (:require [metabase.models.interface :as mi] - [metabase.util :as u] [toucan.models :as models])) (models/defmodel ViewLog :view_log) @@ -15,8 +14,7 @@ (let [defaults {:timestamp :%now}] (merge defaults log-entry))) -(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ViewLog) - models/IModel - (merge models/IModelDefaults - {:pre-insert pre-insert - :types (constantly {:metadata :json})})) +(mi/define-methods + ViewLog + {:pre-insert pre-insert + :types (constantly {:metadata :json})}) diff --git a/src/metabase/util.clj b/src/metabase/util.clj index 5c9d5a44422..df6cd94bae2 100644 --- a/src/metabase/util.clj +++ b/src/metabase/util.clj @@ -1,7 +1,6 @@ (ns metabase.util "Common utility functions useful throughout the codebase." (:require - [clojure.data :as data] [clojure.java.classpath :as classpath] [clojure.math.numeric-tower :as math] [clojure.pprint :refer [pprint]] @@ -349,30 +348,6 @@ (not (Double/isNaN x)) (not (Double/isInfinite x)))) -(defn- check-protocol-impl-method-map - "Check that the methods expected for `protocol` are all implemented by `method-map`, and that no extra methods are - provided. Used internally by `strict-extend`." - [protocol method-map] - (let [[missing-methods extra-methods] (data/diff (set (keys (:method-map protocol))) (set (keys method-map)))] - (when missing-methods - (throw (Exception. (format "Missing implementations for methods in %s: %s" (:var protocol) missing-methods)))) - (when extra-methods - (throw (Exception. (format "Methods implemented that are not in %s: %s " (:var protocol) extra-methods)))))) - -(defn strict-extend - "A strict version of `extend` that throws an exception if any methods declared in the protocol are missing or any - methods not declared in the protocol are provided. - - Since this has better compile-time error-checking, prefer `strict-extend` to regular `extend` in all situations, and - to `extend-protocol`/ `extend-type` going forward." - ;; TODO - maybe implement strict-extend-protocol and strict-extend-type ? - {:style/indent :defn} - [atype protocol method-map & more] - (check-protocol-impl-method-map protocol method-map) - (extend atype protocol method-map) - (when (seq more) - (apply strict-extend atype more))) - (defn remove-diacritical-marks "Return a version of `s` with diacritical marks removed." ^String [^String s] -- GitLab