From 67c4aadd464ffed3466e9703d8fc8be41ffe9aa4 Mon Sep 17 00:00:00 2001 From: Cam Saul <1455846+camsaul@users.noreply.github.com> Date: Mon, 22 Aug 2022 14:19:22 -0700 Subject: [PATCH] [Toucan 2 prep] Dont call type on models (#24915) * [Toucan 2 prep] Don't invoke Toucan models as functions * Some fixes * Test fixes * Test fix * [Toucan 2 prep] Don't call `type` or `class` on Toucan models * Test fixes * More test fixes :wrench: --- .clj-kondo/config.edn | 11 +- .../models/disallow_type_or_class.clj | 8 +- .../common/card_and_dashboard_detail.clj | 4 +- .../metabase_enterprise/sandbox/api/table.clj | 4 +- .../serialization/names.clj | 25 +-- .../serialization/serialize.clj | 24 +-- .../serialization/load_test.clj | 18 +- .../serialization/upsert_test.clj | 12 +- .../bigquery_cloud_sdk/query_processor.clj | 2 +- .../metabase/driver/mongo/query_processor.clj | 4 +- shared/src/metabase/mbql/util.cljc | 13 +- src/metabase/api/emitter.clj | 4 +- src/metabase/api/native_query_snippet.clj | 2 +- .../automagic_dashboards/comparison.clj | 17 +- src/metabase/automagic_dashboards/core.clj | 156 +++++++++--------- src/metabase/domain_entities/core.clj | 6 +- src/metabase/driver/sql/query_processor.clj | 4 +- src/metabase/driver/sql_jdbc/connection.clj | 11 +- .../sql_jdbc/sync/describe_database.clj | 3 +- src/metabase/integrations/ldap.clj | 5 +- src/metabase/models/action.clj | 6 +- src/metabase/models/activity.clj | 2 +- .../application_permissions_revision.clj | 2 +- src/metabase/models/card.clj | 2 +- src/metabase/models/collection.clj | 2 +- .../collection_permission_graph_revision.clj | 2 +- src/metabase/models/dashboard.clj | 4 +- src/metabase/models/dashboard_card.clj | 2 +- src/metabase/models/dashboard_card_series.clj | 2 +- src/metabase/models/database.clj | 2 +- src/metabase/models/dimension.clj | 2 +- src/metabase/models/dispatch.clj | 56 +++++++ src/metabase/models/emitter.clj | 21 ++- src/metabase/models/field.clj | 2 +- src/metabase/models/field_values.clj | 2 +- src/metabase/models/interface.clj | 33 ++++ src/metabase/models/login_history.clj | 2 +- src/metabase/models/metric.clj | 4 +- .../models/metric_important_field.clj | 2 +- src/metabase/models/moderation_review.clj | 2 +- src/metabase/models/native_query_snippet.clj | 4 +- src/metabase/models/permissions.clj | 2 +- src/metabase/models/permissions_group.clj | 2 +- .../models/permissions_group_membership.clj | 2 +- src/metabase/models/permissions_revision.clj | 2 +- src/metabase/models/persisted_info.clj | 2 +- src/metabase/models/pulse.clj | 2 +- src/metabase/models/pulse_card.clj | 2 +- src/metabase/models/pulse_channel.clj | 2 +- .../models/pulse_channel_recipient.clj | 2 +- src/metabase/models/query.clj | 2 +- src/metabase/models/query_cache.clj | 2 +- src/metabase/models/query_execution.clj | 2 +- src/metabase/models/revision.clj | 2 +- src/metabase/models/secret.clj | 4 +- src/metabase/models/segment.clj | 4 +- src/metabase/models/session.clj | 2 +- src/metabase/models/setting.clj | 2 +- src/metabase/models/table.clj | 2 +- src/metabase/models/task_history.clj | 2 +- src/metabase/models/timeline.clj | 2 +- src/metabase/models/timeline_event.clj | 2 +- src/metabase/models/user.clj | 2 +- src/metabase/models/view_log.clj | 2 +- .../middleware/wrap_value_literals.clj | 5 +- src/metabase/query_processor/store.clj | 7 +- src/metabase/related.clj | 45 +++-- src/metabase/sync/analyze/classify.clj | 3 +- src/metabase/sync/interface.clj | 9 +- src/metabase/sync/util.clj | 38 ++--- src/metabase/transforms/core.clj | 5 +- src/metabase/util.clj | 4 +- test/metabase/actions/test_util.clj | 6 +- test/metabase/api/bookmark_test.clj | 7 +- test/metabase/api/embed_test.clj | 19 ++- test/metabase/api/emitter_test.clj | 5 +- test/metabase/api/public_test.clj | 19 ++- .../automagic_dashboards/core_test.clj | 4 +- .../driver/sql_jdbc/connection_test.clj | 21 +-- test/metabase/models/action_test.clj | 5 +- test/metabase/models/dispatch_test.clj | 38 +++++ test/metabase/models/emitter_test.clj | 2 +- test/metabase/test/redefs.clj | 11 +- test/metabase/test/util.clj | 5 +- 84 files changed, 493 insertions(+), 304 deletions(-) create mode 100644 src/metabase/models/dispatch.clj create mode 100644 test/metabase/models/dispatch_test.clj diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index a390f94d0b5..a8f1b1967ee 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -359,7 +359,7 @@ toucan.models models}} :metabase/disallow-invoking-model {:level :warning} - #_:metabase/disallow-class-or-type-on-model #_{:level :warning}} + :metabase/disallow-class-or-type-on-model {:level :warning}} :lint-as {clojure.core.logic/defne clj-kondo.lint-as/def-catch-all @@ -470,6 +470,7 @@ metabase.models/BookmarkOrdering hooks.metabase.models.disallow-invoking-model/hook metabase.models/Card hooks.metabase.models.disallow-invoking-model/hook metabase.models/CardBookmark hooks.metabase.models.disallow-invoking-model/hook + metabase.models/CardEmitter hooks.metabase.models.disallow-invoking-model/hook metabase.models/Collection hooks.metabase.models.disallow-invoking-model/hook metabase.models/CollectionBookmark hooks.metabase.models.disallow-invoking-model/hook metabase.models/CollectionPermissionGraphRevision hooks.metabase.models.disallow-invoking-model/hook @@ -477,10 +478,13 @@ metabase.models/DashboardBookmark hooks.metabase.models.disallow-invoking-model/hook metabase.models/DashboardCard hooks.metabase.models.disallow-invoking-model/hook metabase.models/DashboardCardSeries hooks.metabase.models.disallow-invoking-model/hook + metabase.models/DashboardEmitter hooks.metabase.models.disallow-invoking-model/hook metabase.models/Database hooks.metabase.models.disallow-invoking-model/hook metabase.models/Dimension hooks.metabase.models.disallow-invoking-model/hook + metabase.models/Emitter hooks.metabase.models.disallow-invoking-model/hook metabase.models/Field hooks.metabase.models.disallow-invoking-model/hook metabase.models/FieldValues hooks.metabase.models.disallow-invoking-model/hook + metabase.models/HTTPAction hooks.metabase.models.disallow-invoking-model/hook metabase.models/LoginHistory hooks.metabase.models.disallow-invoking-model/hook metabase.models/Metric hooks.metabase.models.disallow-invoking-model/hook metabase.models/MetricImportantField hooks.metabase.models.disallow-invoking-model/hook @@ -496,6 +500,7 @@ metabase.models/PulseChannel hooks.metabase.models.disallow-invoking-model/hook metabase.models/PulseChannelRecipient hooks.metabase.models.disallow-invoking-model/hook metabase.models/Query hooks.metabase.models.disallow-invoking-model/hook + metabase.models/QueryAction hooks.metabase.models.disallow-invoking-model/hook metabase.models/QueryCache hooks.metabase.models.disallow-invoking-model/hook metabase.models/QueryExecution hooks.metabase.models.disallow-invoking-model/hook metabase.models/Revision hooks.metabase.models.disallow-invoking-model/hook @@ -562,6 +567,6 @@ :hooks.metabase.test.data/mbql-query-first-arg {:level :error} ;; FIXME - :unresolved-symbol {:level :off} - :deprecated-var {:level :off} + :unresolved-symbol {:level :off} + :deprecated-var {:level :off} }}}} 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 494f07a6fd8..8ce8193910c 100644 --- a/.clj-kondo/hooks/metabase/models/disallow_type_or_class.clj +++ b/.clj-kondo/hooks/metabase/models/disallow_type_or_class.clj @@ -2,11 +2,13 @@ (:require [clj-kondo.hooks-api :as hooks])) (def known-models - '#{Activity + '#{Action + Activity ApplicationPermissionsRevision BookmarkOrdering Card CardBookmark + CardEmitter Collection CollectionBookmark CollectionPermissionGraphRevision @@ -14,10 +16,13 @@ DashboardBookmark DashboardCard DashboardCardSeries + DashboardEmitter Database Dimension + Emitter Field FieldValues + HTTPAction LoginHistory Metric MetricImportantField @@ -33,6 +38,7 @@ PulseChannel PulseChannelRecipient Query + QueryAction QueryCache QueryExecution Revision diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/common/card_and_dashboard_detail.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/common/card_and_dashboard_detail.clj index 894851fa9c9..9b8fba0661e 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/common/card_and_dashboard_detail.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/common/card_and_dashboard_detail.clj @@ -4,6 +4,7 @@ [metabase-enterprise.audit-app.pages.common :as common] [metabase.models.card :refer [Card]] [metabase.models.dashboard :refer [Dashboard]] + [metabase.models.interface :as mi] [metabase.models.revision :as revision] [metabase.util.honeysql-extensions :as hx] [metabase.util.schema :as su] @@ -77,7 +78,8 @@ (s/defn revision-history "Get a revision history table for a Card or Dashboard." - [model-entity :- (s/cond-pre (class Card) (class Dashboard)), model-id :- su/IntGreaterThanZero] + [model-entity :- (s/cond-pre (mi/InstanceOf Card) (mi/InstanceOf Dashboard)) + model-id :- su/IntGreaterThanZero] {:metadata [[:timestamp {:display_name "Edited on", :base_type :type/DateTime}] [:user_id {:display_name "User ID", :base_type :type/Integer, :remapped_to :user_name}] [:user_name {:display_name "Edited by", :base_type :type/Name, :remapped_from :user_id}] diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/api/table.clj b/enterprise/backend/src/metabase_enterprise/sandbox/api/table.clj index 0865739e5ad..6088aafb0a3 100644 --- a/enterprise/backend/src/metabase_enterprise/sandbox/api/table.clj +++ b/enterprise/backend/src/metabase_enterprise/sandbox/api/table.clj @@ -16,7 +16,7 @@ [toucan.db :as db] [toucan.models :as models])) -(s/defn ^:private find-gtap-question :- (s/maybe (type Card)) +(s/defn ^:private find-gtap-question :- (s/maybe (mi/InstanceOf Card)) "Find the associated GTAP question (if there is one) for the given `table-or-table-id` and `user-or-user-id`. Returns nil if no question was found." [table-or-table-id user-or-user-id] @@ -33,7 +33,7 @@ (s/defn only-segmented-perms? :- s/Bool "Returns true if the user has only segemented and not full table permissions. If the user has full table permissions we wouldn't want to apply this segment filtering." - [table :- (type Table)] + [table :- (mi/InstanceOf Table)] (and (not (perms/set-has-full-permissions? @api/*current-user-permissions-set* (perms/table-query-path table))) diff --git a/enterprise/backend/src/metabase_enterprise/serialization/names.clj b/enterprise/backend/src/metabase_enterprise/serialization/names.clj index 7bda6bcaabe..ec7aa2e5948 100644 --- a/enterprise/backend/src/metabase_enterprise/serialization/names.clj +++ b/enterprise/backend/src/metabase_enterprise/serialization/names.clj @@ -9,6 +9,7 @@ [metabase.models.dashboard :refer [Dashboard]] [metabase.models.database :as database :refer [Database]] [metabase.models.field :refer [Field]] + [metabase.models.interface :as mi] [metabase.models.metric :refer [Metric]] [metabase.models.native-query-snippet :refer [NativeQuerySnippet]] [metabase.models.pulse :refer [Pulse]] @@ -32,7 +33,7 @@ "Inverse of `safe-name`." codec/url-decode) -(defmulti ^:private fully-qualified-name* type) +(defmulti ^:private fully-qualified-name* mi/model) (def ^{:arglists '([entity] [model id])} fully-qualified-name "Get the logical path for entity `entity`." @@ -44,11 +45,11 @@ id (fully-qualified-name* (db/select-one model :id id))))))) -(defmethod fully-qualified-name* (type Database) +(defmethod fully-qualified-name* Database [db] (str "/databases/" (safe-name db))) -(defmethod fully-qualified-name* (type Table) +(defmethod fully-qualified-name* Table [table] (if (:schema table) (format "%s/schemas/%s/tables/%s" @@ -59,17 +60,17 @@ (->> table :db_id (fully-qualified-name Database)) (safe-name table)))) -(defmethod fully-qualified-name* (type Field) +(defmethod fully-qualified-name* Field [field] (if (:fk_target_field_id field) (str (->> field :table_id (fully-qualified-name Table)) "/fks/" (safe-name field)) (str (->> field :table_id (fully-qualified-name Table)) "/fields/" (safe-name field)))) -(defmethod fully-qualified-name* (type Metric) +(defmethod fully-qualified-name* Metric [metric] (str (->> metric :table_id (fully-qualified-name Table)) "/metrics/" (safe-name metric))) -(defmethod fully-qualified-name* (type Segment) +(defmethod fully-qualified-name* Segment [segment] (str (->> segment :table_id (fully-qualified-name Table)) "/segments/" (safe-name segment))) @@ -78,7 +79,7 @@ (str ":" (if (keyword? coll-ns) (name coll-ns) coll-ns) "/"))] (str "/collections/" ns-part (safe-name collection)))) -(defmethod fully-qualified-name* (type Collection) +(defmethod fully-qualified-name* Collection [collection] (let [parents (some->> (str/split (:location collection) #"/") rest @@ -87,21 +88,21 @@ (apply str))] (str root-collection-path parents (local-collection-name collection)))) -(defmethod fully-qualified-name* (type Dashboard) +(defmethod fully-qualified-name* Dashboard [dashboard] (format "%s/dashboards/%s" (or (some->> dashboard :collection_id (fully-qualified-name Collection)) root-collection-path) (safe-name dashboard))) -(defmethod fully-qualified-name* (type Pulse) +(defmethod fully-qualified-name* Pulse [pulse] (format "%s/pulses/%s" (or (some->> pulse :collection_id (fully-qualified-name Collection)) root-collection-path) (safe-name pulse))) -(defmethod fully-qualified-name* (type Card) +(defmethod fully-qualified-name* Card [card] (format "%s/cards/%s" (or (some->> card @@ -110,11 +111,11 @@ root-collection-path) (safe-name card))) -(defmethod fully-qualified-name* (type User) +(defmethod fully-qualified-name* User [user] (str "/users/" (:email user))) -(defmethod fully-qualified-name* (type NativeQuerySnippet) +(defmethod fully-qualified-name* NativeQuerySnippet [snippet] (format "%s/snippets/%s" (or (some->> snippet :collection_id (fully-qualified-name Collection)) diff --git a/enterprise/backend/src/metabase_enterprise/serialization/serialize.clj b/enterprise/backend/src/metabase_enterprise/serialization/serialize.clj index 578b61023b7..90335488ff8 100644 --- a/enterprise/backend/src/metabase_enterprise/serialization/serialize.clj +++ b/enterprise/backend/src/metabase_enterprise/serialization/serialize.clj @@ -13,6 +13,7 @@ [metabase.models.database :as database :refer [Database]] [metabase.models.dimension :refer [Dimension]] [metabase.models.field :as field :refer [Field]] + [metabase.models.interface :as mi] [metabase.models.metric :refer [Metric]] [metabase.models.native-query-snippet :refer [NativeQuerySnippet]] [metabase.models.pulse :refer [Pulse]] @@ -103,21 +104,22 @@ (some #(instance? % entity) (map type [Metric Field Segment])) (dissoc :table_id))) (defmulti ^:private serialize-one - type) + {:arglists '([instance])} + mi/model) (def ^{:arglists '([entity])} serialize "Serialize entity `entity`." (comp ids->fully-qualified-names strip-crud serialize-one)) (defmethod serialize-one :default - [entity] - entity) + [instance] + instance) -(defmethod serialize-one (type Database) +(defmethod serialize-one Database [db] (dissoc db :features)) -(defmethod serialize-one (type Field) +(defmethod serialize-one Field [field] (let [field (-> field (update :parent_id (partial fully-qualified-name Field)) @@ -199,18 +201,18 @@ (assoc :visualization_settings (convert-viz-settings (:visualization_settings dashboard-card))) strip-crud)))) -(defmethod serialize-one (type Dashboard) +(defmethod serialize-one Dashboard [dashboard] (assoc dashboard :dashboard_cards (dashboard-cards-for-dashboard dashboard))) -(defmethod serialize-one (type Card) +(defmethod serialize-one Card [card] (-> card (m/update-existing :table_id (partial fully-qualified-name Table)) (update :database_id (partial fully-qualified-name Database)) (m/update-existing :visualization_settings convert-viz-settings))) -(defmethod serialize-one (type Pulse) +(defmethod serialize-one Pulse [pulse] (assoc pulse :cards (for [card (db/select PulseCard :pulse_id (u/the-id pulse))] @@ -220,16 +222,16 @@ :channels (for [channel (db/select PulseChannel :pulse_id (u/the-id pulse))] (strip-crud channel)))) -(defmethod serialize-one (type User) +(defmethod serialize-one User [user] (select-keys user [:first_name :last_name :email :is_superuser])) -(defmethod serialize-one (type Dimension) +(defmethod serialize-one Dimension [dimension] (-> dimension (update :field_id (partial fully-qualified-name Field)) (update :human_readable_field_id (partial fully-qualified-name Field)))) -(defmethod serialize-one (type NativeQuerySnippet) +(defmethod serialize-one NativeQuerySnippet [snippet] (select-keys snippet [:name :description :content])) diff --git a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj index 403ff80f8ac..1c1d7a8755e 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj @@ -9,6 +9,7 @@ [metabase.models :refer [Card Collection Dashboard DashboardCard DashboardCardSeries Database Dimension Field FieldValues Metric NativeQuerySnippet Pulse PulseCard PulseChannel Segment Table User]] + [metabase.models.interface :as mi] [metabase.query-processor :as qp] [metabase.query-processor.middleware.permissions :as qp.perms] [metabase.query-processor.store :as qp.store] @@ -107,10 +108,11 @@ card-ids)) (defmulti ^:private assert-loaded-entity - (fn [entity _fingerprint] - (type entity))) + {:arglists '([instance fingerprint])} + (fn [instance _fingerprint] + (mi/model instance))) -(defmethod assert-loaded-entity (type Card) +(defmethod assert-loaded-entity Card [{card-name :name :as card} {:keys [query-results collections]}] (testing (format "Card: %s" card-name) (query-res-match query-results card) @@ -143,7 +145,7 @@ (let [[_ ^String parent-id] (re-matches #".*/(\d+)/$" (:location collection))] (db/select-one-field :name Collection :id (Integer. parent-id)))) -(defmethod assert-loaded-entity (type Collection) +(defmethod assert-loaded-entity Collection [collection _] (case (:name collection) "My Nested Collection" (is (= "My Collection" (collection-parent-name collection))) @@ -161,9 +163,9 @@ "Should not have loaded different user's PC")) collection) -(defmethod assert-loaded-entity (type NativeQuerySnippet) +(defmethod assert-loaded-entity NativeQuerySnippet [snippet {:keys [entities]}] - (when-let [orig-snippet (first (filter (every-pred #(= (type NativeQuerySnippet) (type %)) + (when-let [orig-snippet (first (filter (every-pred #(mi/instance-of? NativeQuerySnippet %) #(= (:name snippet) (:name %))) (map last entities)))] (is (some? orig-snippet)) (is (= (select-keys orig-snippet [:name :description :content]) @@ -193,7 +195,7 @@ (is (contains? #{"price" "PRICE"} (:name f2))) (is (= {:dimension [:field (u/the-id f2) {:source-field (u/the-id f1)}]} dimension))))) -(defmethod assert-loaded-entity (type Dashboard) +(defmethod assert-loaded-entity Dashboard [dashboard _] (testing "The dashboard card series were loaded correctly" (when (= "My Dashboard" (:name dashboard)) @@ -250,7 +252,7 @@ (is (= "Textbox Card" (get-in dashcard [:visualization_settings :text]))))))) dashboard) -(defmethod assert-loaded-entity (type Pulse) +(defmethod assert-loaded-entity Pulse [pulse _] (is (some? pulse)) (let [pulse-cards (db/select PulseCard :pulse_id (u/the-id pulse))] diff --git a/enterprise/backend/test/metabase_enterprise/serialization/upsert_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/upsert_test.clj index da8836d0fcc..8577209e165 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/upsert_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/upsert_test.clj @@ -4,9 +4,11 @@ [metabase-enterprise.serialization.upsert :as upsert] [metabase.models :refer [Card Collection Dashboard DashboardCard Database Field Metric NativeQuerySnippet Pulse Segment Table User]] + [metabase.models.interface :as mi] [metabase.test :as mt] [metabase.util :as u] - [toucan.db :as db])) + [toucan.db :as db] + [toucan.models :as models])) (def ^:private same? (comp nil? second data/diff)) @@ -69,9 +71,9 @@ (testing "Card 2" (is (same? (db/select-one Card :id id2) e2)))))) -(defn- dummy-entity [dummy-dashboard model entity instance-num] +(defn- dummy-entity [dummy-dashboard instance entity instance-num] (cond - (= (type DashboardCard) (type model)) + (mi/instance-of? DashboardCard instance) ;; hack to make sure that :visualization_settings are slightly different between the two dummy instances ;; this is necessary because DashboardCards have that as part of their identity-condition (assoc entity :dashboard_id (u/the-id dummy-dashboard) @@ -92,7 +94,7 @@ ;; create an additional entity so we're sure whe get the right one model [_ (dummy-entity dashboard model e1 1)] model [{id :id} (dummy-entity dashboard model e2 2)]] - (let [e (model id)] + (let [e (db/select-one model (models/primary-key model) id)] ;; make sure that all columns in identity-condition actually exist in the model (is (= (set id-cond) (-> e (select-keys id-cond) @@ -101,7 +103,7 @@ (is (= (#'upsert/select-identical model (cond-> e ;; engine is a keyword but has to be a string for ;; HoneySQL to not interpret it as a col name - (= (class e) (class Database)) (update :engine name))) + (mi/instance-of? Database e) (update :engine name))) e)))))))] (doseq [model [Collection Card diff --git a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj index a9c737cce79..eff072fb28d 100644 --- a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj +++ b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj @@ -173,7 +173,7 @@ "TIME" :time nil)) -(defmethod temporal-type (class Field) +(defmethod temporal-type Field [{base-type :base_type, effective-type :effective_type, database-type :database_type}] (or (database-type->temporal-type database-type) (base-type->temporal-type (or effective-type base-type)))) diff --git a/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj b/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj index b670973ae9f..f44c4f80bca 100644 --- a/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj +++ b/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj @@ -105,7 +105,7 @@ :in `(let [~field ~(keyword (str "$$" (name field)))] ~@body)}}) -(defmethod ->lvalue (class Field) +(defmethod ->lvalue Field [field] (field->name field \.)) @@ -121,7 +121,7 @@ [[_ expression-name]] (->rvalue (mbql.u/expression-with-name (:query *query*) expression-name))) -(defmethod ->rvalue (class Field) +(defmethod ->rvalue Field [{coercion :coercion_strategy, :as field}] (let [field-name (str \$ (field->name field "."))] (cond diff --git a/shared/src/metabase/mbql/util.cljc b/shared/src/metabase/mbql/util.cljc index 09873ccc72f..8ea82b2062a 100644 --- a/shared/src/metabase/mbql/util.cljc +++ b/shared/src/metabase/mbql/util.cljc @@ -8,6 +8,7 @@ [metabase.mbql.schema :as mbql.s] [metabase.mbql.schema.helpers :as schema.helpers] [metabase.mbql.util.match :as mbql.match] + [metabase.models.dispatch :as models.dispatch] [metabase.shared.util.i18n :as i18n] metabase.util.i18n [potemkin :as p] @@ -344,9 +345,15 @@ "Dispatch function perfect for use with multimethods that dispatch off elements of an MBQL query. If `x` is an MBQL clause, dispatches off the clause name; otherwise dispatches off `x`'s class." ([x] - (if (mbql-clause? x) - (first x) - (type x))) + #?(:clj + (if (mbql-clause? x) + (first x) + (or (metabase.models.dispatch/model x) + (type x))) + :cljs + (if (mbql-clause? x) + (first x) + (type x)))) ([x _] (dispatch-by-clause-name-or-class x))) diff --git a/src/metabase/api/emitter.clj b/src/metabase/api/emitter.clj index abdc5f670a5..bf25d969734 100644 --- a/src/metabase/api/emitter.clj +++ b/src/metabase/api/emitter.clj @@ -58,14 +58,14 @@ (api/defendpoint PUT "/:emitter-id" "Endpoint to update an emitter." [emitter-id :as {emitter :body}] - (api/check-404 (Emitter emitter-id)) + (api/check-404 (db/select-one Emitter :id emitter-id)) (db/update! Emitter emitter-id emitter) api/generic-204-no-content) (api/defendpoint DELETE "/:emitter-id" "Endpoint to delete an emitter." [emitter-id] - (api/check-404 (Emitter emitter-id)) + (api/check-404 (db/select-one Emitter :id emitter-id)) (db/delete! Emitter :id emitter-id) api/generic-204-no-content) diff --git a/src/metabase/api/native_query_snippet.clj b/src/metabase/api/native_query_snippet.clj index 6d81e9e5d82..53b65772b08 100644 --- a/src/metabase/api/native_query_snippet.clj +++ b/src/metabase/api/native_query_snippet.clj @@ -12,7 +12,7 @@ [toucan.db :as db] [toucan.hydrate :refer [hydrate]])) -(s/defn ^:private hydrated-native-query-snippet :- (s/maybe (class NativeQuerySnippet)) +(s/defn ^:private hydrated-native-query-snippet :- (s/maybe (mi/InstanceOf NativeQuerySnippet)) [id :- su/IntGreaterThanZero] (-> (api/read-check (db/select-one NativeQuerySnippet :id id)) (hydrate :creator))) diff --git a/src/metabase/automagic_dashboards/comparison.clj b/src/metabase/automagic_dashboards/comparison.clj index 78f6eb10f42..12aaf75b913 100644 --- a/src/metabase/automagic_dashboards/comparison.clj +++ b/src/metabase/automagic_dashboards/comparison.clj @@ -1,10 +1,19 @@ (ns metabase.automagic-dashboards.comparison (:require [medley.core :as m] [metabase.api.common :as api] - [metabase.automagic-dashboards.core :refer [->field ->related-entity ->root automagic-analysis capitalize-first cell-title encode-base64-json metric-name source-name]] + [metabase.automagic-dashboards.core :refer [->field + ->related-entity + ->root + automagic-analysis + capitalize-first + cell-title + encode-base64-json + metric-name + source-name]] [metabase.automagic-dashboards.filters :as filters] [metabase.automagic-dashboards.populate :as populate] [metabase.mbql.normalize :as mbql.normalize] + [metabase.models.interface :as mi] [metabase.models.table :refer [Table]] [metabase.query-processor.util :as qp.util] [metabase.related :as related] @@ -196,7 +205,7 @@ :description ""}) (when (and ((some-fn :query-filter :cell-query) left) (not= (:source left) (:entity right))) - [{:url (if (->> left :source (instance? (type Table))) + [{:url (if (->> left :source (mi/instance-of? Table)) (str (:url left) "/compare/table/" (-> left :source u/the-id)) (str (:url left) "/compare/adhoc/" @@ -237,8 +246,8 @@ (cell-title left)))) right (cond-> right (part-vs-whole-comparison? left right) - (assoc :comparison-name (condp instance? (:entity right) - (type Table) + (assoc :comparison-name (condp mi/instance-of? (:entity right) + Table (tru "All {0}" (:short-name right)) (tru "{0}, all {1}" diff --git a/src/metabase/automagic_dashboards/core.clj b/src/metabase/automagic_dashboards/core.clj index 92e95eec5cc..08740be9065 100644 --- a/src/metabase/automagic_dashboards/core.clj +++ b/src/metabase/automagic_dashboards/core.clj @@ -44,7 +44,7 @@ (def ^:private ^{:arglists '([field])} id-or-name (some-fn :id :name)) -(s/defn ->field :- (s/maybe (type Field)) +(s/defn ->field :- (s/maybe (mi/InstanceOf Field)) "Return `Field` instance for a given ID or name in the context of root." [{{result-metadata :result_metadata} :source, :as root} field-id-or-name-or-clause :- (s/cond-pre su/IntGreaterThanZero su/NonBlankString mbql.s/Field)] @@ -158,9 +158,9 @@ respect to that entity. It is called a root because the automated dashboard uses productions to recursively create a tree of dashboard cards to fill the dashboards. This multimethod is for turning a given entity into a root." {:arglists '([entity])} - type) + mi/model) -(defmethod ->root (type Table) +(defmethod ->root Table [table] {:entity table :full-name (if (ga-table? table) @@ -172,7 +172,7 @@ :url (format "%stable/%s" public-endpoint (u/the-id table)) :rules-prefix ["table"]}) -(defmethod ->root (type Segment) +(defmethod ->root Segment [segment] (let [table (->> segment :table_id (db/select-one Table :id))] {:entity segment @@ -185,7 +185,7 @@ :url (format "%ssegment/%s" public-endpoint (u/the-id segment)) :rules-prefix ["table"]})) -(defmethod ->root (type Metric) +(defmethod ->root Metric [metric] (let [table (->> metric :table_id (db/select-one Table :id))] {:entity metric @@ -200,7 +200,7 @@ :url (format "%smetric/%s" public-endpoint (:id metric)) :rules-prefix ["metric"]})) -(defmethod ->root (type Field) +(defmethod ->root Field [field] (let [table (field/table field)] {:entity field @@ -245,7 +245,7 @@ (native-query? card) (-> card (assoc :entity_type :entity/GenericTable)) :else (->> card table-id (db/select-one Table :id)))) -(defmethod ->root (type Card) +(defmethod ->root Card [card] (let [source (source card)] {:entity card @@ -259,7 +259,7 @@ "table" "question")]})) -(defmethod ->root (type Query) +(defmethod ->root Query [query] (let [source (source query)] {:entity query @@ -282,7 +282,7 @@ :arglists '([template-type model]) :private true} ->reference (fn [template-type model] - [template-type (type model)])) + [template-type (mi/model model)])) (defn- optimal-datetime-resolution [field] @@ -302,7 +302,7 @@ :year) :day))) -(defmethod ->reference [:mbql (type Field)] +(defmethod ->reference [:mbql Field] [_ {:keys [fk_target_field_id id link aggregation name base_type] :as field}] (let [reference (mbql.normalize/normalize (cond @@ -322,7 +322,7 @@ :else reference))) -(defmethod ->reference [:string (type Field)] +(defmethod ->reference [:string Field] [_ {:keys [display_name full-name link]}] (cond full-name full-name @@ -331,25 +331,25 @@ display_name) :else display_name)) -(defmethod ->reference [:string (type Table)] +(defmethod ->reference [:string Table] [_ {:keys [display_name full-name]}] (or full-name display_name)) -(defmethod ->reference [:string (type Metric)] +(defmethod ->reference [:string Metric] [_ {:keys [name full-name]}] (or full-name name)) -(defmethod ->reference [:mbql (type Metric)] +(defmethod ->reference [:mbql Metric] [_ {:keys [id definition]}] (if id [:metric id] (-> definition :aggregation first))) -(defmethod ->reference [:native (type Field)] +(defmethod ->reference [:native Field] [_ field] (field/qualified-name field)) -(defmethod ->reference [:native (type Table)] +(defmethod ->reference [:native Table] [_ {:keys [name]}] name) @@ -521,7 +521,7 @@ subform)) {:type :query :database (-> context :root :database) - :query (cond-> {:source-table (if (->> context :source (instance? (type Table))) + :query (cond-> {:source-table (if (->> context :source (mi/instance-of? Table)) (-> context :source u/the-id) (->> context :source u/the-id (str "card__")))} (seq filters) @@ -704,9 +704,9 @@ (defmulti ^{:private true :arglists '([context entity])} - inject-root (fn [_ entity] (type entity))) + inject-root (fn [_ instance] (mi/model instance))) -(defmethod inject-root (type Field) +(defmethod inject-root Field [context field] (let [field (assoc field :link (->> context @@ -728,7 +728,7 @@ :score rules/max-score}]]) (into {})))))) -(defmethod inject-root (type Metric) +(defmethod inject-root Metric [context metric] (update context :metrics assoc "this" {:metric (->reference :mbql metric) :name (:name metric) @@ -742,10 +742,10 @@ [root, rule :- rules/Rule] {:pre [(:source root)]} (let [source (:source root) - tables (concat [source] (when (instance? (type Table) source) + tables (concat [source] (when (mi/instance-of? Table source) (linked-tables source))) engine (source->engine source) - table->fields (if (instance? (type Table) source) + table->fields (if (mi/instance-of? Table source) (comp (->> (db/select Field :table_id [:in (map u/the-id tables)] :visibility_type "normal" @@ -853,7 +853,7 @@ (defn- drilldown-fields [context] - (when (and (->> context :root :source (instance? (type Table))) + (when (and (->> context :root :source (mi/instance-of? Table)) (-> context :root :entity ga-table? not)) (->> context :dimensions @@ -872,7 +872,7 @@ :title (tru "Compare with {0}" (:comparison-name segment)) :description ""}) (when ((some-fn :query-filter :cell-query) root) - [{:url (if (->> root :source (instance? (type Table))) + [{:url (if (->> root :source (mi/instance-of? Table)) (str (:url root) "/compare/table/" (-> root :source u/the-id)) (str (:url root) "/compare/adhoc/" (encode-base64-json @@ -926,50 +926,50 @@ {}))) (def ^:private related-selectors - {(type Table) (let [down [[:indepth] [:segments :metrics] [:drilldown-fields]] - sideways [[:linking-to :linked-from] [:tables]] - compare [[:compare]]] - {:zoom-in [down down down down] - :related [sideways sideways] - :compare [compare compare]}) - (type Segment) (let [down [[:indepth] [:segments :metrics] [:drilldown-fields]] - sideways [[:linking-to] [:tables]] - up [[:table]] - compare [[:compare]]] - {:zoom-in [down down down] - :zoom-out [up] - :related [sideways sideways] - :compare [compare compare]}) - (type Metric) (let [down [[:drilldown-fields]] - sideways [[:metrics :segments]] - up [[:table]] - compare [[:compare]]] - {:zoom-in [down down] - :zoom-out [up] - :related [sideways sideways sideways] - :compare [compare compare]}) - (type Field) (let [sideways [[:fields]] - up [[:table] [:metrics :segments]] - compare [[:compare]]] - {:zoom-out [up] - :related [sideways sideways] - :compare [compare]}) - (type Card) (let [down [[:drilldown-fields]] - sideways [[:metrics] [:similar-questions :dashboard-mates]] - up [[:table]] - compare [[:compare]]] - {:zoom-in [down down] - :zoom-out [up] - :related [sideways sideways sideways] - :compare [compare compare]}) - (type Query) (let [down [[:drilldown-fields]] - sideways [[:metrics] [:similar-questions]] - up [[:table]] - compare [[:compare]]] - {:zoom-in [down down] - :zoom-out [up] - :related [sideways sideways sideways] - :compare [compare compare]})}) + {Table (let [down [[:indepth] [:segments :metrics] [:drilldown-fields]] + sideways [[:linking-to :linked-from] [:tables]] + compare [[:compare]]] + {:zoom-in [down down down down] + :related [sideways sideways] + :compare [compare compare]}) + Segment (let [down [[:indepth] [:segments :metrics] [:drilldown-fields]] + sideways [[:linking-to] [:tables]] + up [[:table]] + compare [[:compare]]] + {:zoom-in [down down down] + :zoom-out [up] + :related [sideways sideways] + :compare [compare compare]}) + Metric (let [down [[:drilldown-fields]] + sideways [[:metrics :segments]] + up [[:table]] + compare [[:compare]]] + {:zoom-in [down down] + :zoom-out [up] + :related [sideways sideways sideways] + :compare [compare compare]}) + Field (let [sideways [[:fields]] + up [[:table] [:metrics :segments]] + compare [[:compare]]] + {:zoom-out [up] + :related [sideways sideways] + :compare [compare]}) + Card (let [down [[:drilldown-fields]] + sideways [[:metrics] [:similar-questions :dashboard-mates]] + up [[:table]] + compare [[:compare]]] + {:zoom-in [down down] + :zoom-out [up] + :related [sideways sideways sideways] + :compare [compare compare]}) + Query (let [down [[:drilldown-fields]] + sideways [[:metrics] [:similar-questions]] + up [[:table]] + compare [[:compare]]] + {:zoom-in [down down] + :zoom-out [up] + :related [sideways sideways sideways] + :compare [compare compare]})}) (s/defn ^:private related "Build a balanced list of related X-rays. General composition of the list is determined for each @@ -979,7 +979,7 @@ (drilldown-fields context) (related-entities root) (comparisons root)) - (fill-related max-related (related-selectors (-> root :entity type))))) + (fill-related max-related (get related-selectors (-> root :entity mi/model))))) (defn- filter-referenced-fields "Return a map of fields referenced in filter clause." @@ -1028,21 +1028,21 @@ "Create a transient dashboard analyzing given entity." {:arglists '([entity opts])} (fn [entity _] - (type entity))) + (mi/model entity))) -(defmethod automagic-analysis (type Table) +(defmethod automagic-analysis Table [table opts] (automagic-dashboard (merge (->root table) opts))) -(defmethod automagic-analysis (type Segment) +(defmethod automagic-analysis Segment [segment opts] (automagic-dashboard (merge (->root segment) opts))) -(defmethod automagic-analysis (type Metric) +(defmethod automagic-analysis Metric [metric opts] (automagic-dashboard (merge (->root metric) opts))) -(s/defn ^:private collect-metrics :- (s/maybe [(type Metric)]) +(s/defn ^:private collect-metrics :- (s/maybe [(mi/InstanceOf Metric)]) [root question] (map (fn [aggregation-clause] (if (-> aggregation-clause @@ -1057,7 +1057,7 @@ :table_id table-id})))) (get-in question [:dataset_query :query :aggregation]))) -(s/defn ^:private collect-breakout-fields :- (s/maybe [(type Field)]) +(s/defn ^:private collect-breakout-fields :- (s/maybe [(mi/InstanceOf Field)]) [root question] (for [breakout (get-in question [:dataset_query :query :breakout]) field-clause (take 1 (filters/collect-field-references breakout)) @@ -1207,7 +1207,7 @@ (update dashboard :ordered_cards #(map (partial splice-in join-statement) %)) dashboard)) -(defmethod automagic-analysis (type Card) +(defmethod automagic-analysis Card [card {:keys [cell-query] :as opts}] (let [root (->root card) cell-url (format "%squestion/%s/cell/%s" public-endpoint @@ -1232,7 +1232,7 @@ {:transient_name title :name title})))))))) -(defmethod automagic-analysis (type Query) +(defmethod automagic-analysis Query [query {:keys [cell-query] :as opts}] (let [root (->root query) cell-query (when cell-query (mbql.normalize/normalize-fragment [:query :filter] cell-query)) @@ -1260,7 +1260,7 @@ {:transient_name title :name title})))))))) -(defmethod automagic-analysis (type Field) +(defmethod automagic-analysis Field [field opts] (automagic-dashboard (merge (->root field) opts))) diff --git a/src/metabase/domain_entities/core.clj b/src/metabase/domain_entities/core.clj index 776670bed0c..9cd4304dca1 100644 --- a/src/metabase/domain_entities/core.clj +++ b/src/metabase/domain_entities/core.clj @@ -4,6 +4,7 @@ [metabase.domain-entities.specs :refer [domain-entity-specs MBQL]] [metabase.mbql.util :as mbql.u] [metabase.models.card :refer [Card]] + [metabase.models.interface :as mi] [metabase.models.table :as table :refer [Table]] [metabase.util :as u] [schema.core :as s])) @@ -24,10 +25,11 @@ (def SourceEntity "A source for a card. Can be either a table or another card." - (s/cond-pre (type Table) (type Card))) + (s/cond-pre (mi/InstanceOf Table) (mi/InstanceOf Card))) (def Bindings - "Top-level lexical context mapping source names to their corresponding entity and constituent dimensions. See also `DimensionBindings`." + "Top-level lexical context mapping source names to their corresponding entity and constituent dimensions. See also + `DimensionBindings`." {SourceName {(s/optional-key :entity) SourceEntity (s/required-key :dimensions) DimensionBindings}}) diff --git a/src/metabase/driver/sql/query_processor.clj b/src/metabase/driver/sql/query_processor.clj index 274995d5ccc..f740fa51b38 100644 --- a/src/metabase/driver/sql/query_processor.clj +++ b/src/metabase/driver/sql/query_processor.clj @@ -854,7 +854,7 @@ ;;; -------------------------------------------------- source-table -------------------------------------------------- -(defmethod ->honeysql [:sql (class Table)] +(defmethod ->honeysql [:sql Table] [driver table] (let [{table-name :name, schema :schema} table] (->honeysql driver (hx/identifier :table schema table-name)))) @@ -1003,7 +1003,7 @@ prefix-field-alias]) ;; deprecated, but we'll keep it here for now for backwards compatibility. -(defmethod ->honeysql [:sql (type Field)] +(defmethod ->honeysql [:sql Field] [driver field] (deprecated/log-deprecation-warning driver "->honeysql [:sql (class Field)]" "0.42.0") (->honeysql driver [:field (:id field) nil])) diff --git a/src/metabase/driver/sql_jdbc/connection.clj b/src/metabase/driver/sql_jdbc/connection.clj index ad9d68876d5..b1dc11cf585 100644 --- a/src/metabase/driver/sql_jdbc/connection.clj +++ b/src/metabase/driver/sql_jdbc/connection.clj @@ -7,10 +7,13 @@ [metabase.connection-pool :as connection-pool] [metabase.driver :as driver] [metabase.models.database :refer [Database]] + [metabase.models.interface :as mi] [metabase.query-processor.error-type :as qp.error-type] [metabase.util :as u] [metabase.util.i18n :refer [trs tru]] + [metabase.util.schema :as su] [metabase.util.ssh :as ssh] + [schema.core :as s] [toucan.db :as db]) (:import com.mchange.v2.c3p0.DataSources javax.sql.DataSource)) @@ -137,11 +140,10 @@ database-id->jdbc-spec-hash (atom {})) -(defn- jdbc-spec-hash +(s/defn ^:private jdbc-spec-hash "Computes a hash value for the JDBC connection spec based on `database`'s `:details` map, for the purpose of determining if details changed and therefore the existing connection pool needs to be invalidated." - [{driver :engine, :keys [details], :as database}] - {:pre [(or nil? (instance? (type Database) database))]} + [{driver :engine, :keys [details], :as database} :- (s/maybe su/Map)] (when (some? database) (hash (connection-details->spec driver details)))) @@ -194,7 +196,8 @@ (u/id db-or-id-or-spec) (let [database-id (u/the-id db-or-id-or-spec) ;; we need the Database instance no matter what (in order to compare details hash with cached value) - db (or (and (instance? (type Database) db-or-id-or-spec) db-or-id-or-spec) ; passed in + db (or (when (mi/instance-of? Database db-or-id-or-spec) + db-or-id-or-spec) ; passed in (db/select-one [Database :id :engine :details] :id database-id) ; look up by ID (throw (ex-info (tru "Database {0} does not exist." database-id) {:status-code 404 diff --git a/src/metabase/driver/sql_jdbc/sync/describe_database.clj b/src/metabase/driver/sql_jdbc/sync/describe_database.clj index 2b23ceb20a8..faf68e5adba 100644 --- a/src/metabase/driver/sql_jdbc/sync/describe_database.clj +++ b/src/metabase/driver/sql_jdbc/sync/describe_database.clj @@ -12,6 +12,7 @@ [metabase.driver.sync :as driver.s] [metabase.driver.util :as driver.u] [metabase.models :refer [Database]] + [metabase.models.interface :as mi] [metabase.util.honeysql-extensions :as hx] [toucan.db :as db]) (:import [java.sql Connection DatabaseMetaData ResultSet])) @@ -133,7 +134,7 @@ (db-tables driver (.getMetaData conn) nil db-name-or-nil))) (defn- db-or-id-or-spec->database [db-or-id-or-spec] - (cond (instance? (class Database) db-or-id-or-spec) + (cond (mi/instance-of? Database db-or-id-or-spec) db-or-id-or-spec (int? db-or-id-or-spec) diff --git a/src/metabase/integrations/ldap.clj b/src/metabase/integrations/ldap.clj index 793b9a3585e..98be17f8b4e 100644 --- a/src/metabase/integrations/ldap.clj +++ b/src/metabase/integrations/ldap.clj @@ -3,6 +3,7 @@ [clj-ldap.client :as ldap] [metabase.integrations.ldap.default-implementation :as default-impl] [metabase.integrations.ldap.interface :as i] + [metabase.models.interface :as mi] [metabase.models.setting :as setting :refer [defsetting]] [metabase.models.user :refer [User]] [metabase.plugins.classloader :as classloader] @@ -210,7 +211,7 @@ ([ldap-connection :- LDAPConnectionPool, username :- su/NonBlankString] (default-impl/find-user ldap-connection username (ldap-settings)))) -(s/defn fetch-or-create-user! :- (class User) - "Using the `user-info` (from `find-user`) get the corresponding Metabase user, creating it if necessary." +(s/defn fetch-or-create-user! :- (mi/InstanceOf User) + "Using the `user-info` (from [[find-user]]) get the corresponding Metabase user, creating it if necessary." [user-info :- i/UserInfo] (default-impl/fetch-or-create-user! user-info (ldap-settings))) diff --git a/src/metabase/models/action.clj b/src/metabase/models/action.clj index 6db464fb695..cf6968fd4ea 100644 --- a/src/metabase/models/action.clj +++ b/src/metabase/models/action.clj @@ -19,7 +19,7 @@ (u/update-if-exists template :parameters (mi/catch-normalization-exceptions mi/normalize-parameters-list))) mi/json-out-with-keywordization)) -(u/strict-extend (class Action) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Action) models/IModel (merge models/IModelDefaults {:types (constantly {:type :keyword}) @@ -43,11 +43,11 @@ :pre-delete pre-delete :pre-update pre-update})) -(u/strict-extend (class QueryAction) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class QueryAction) models/IModel Action-subtype-IModel-impl) -(u/strict-extend (class HTTPAction) +(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})})) diff --git a/src/metabase/models/activity.clj b/src/metabase/models/activity.clj index 69bd59a629e..e399427f9dc 100644 --- a/src/metabase/models/activity.clj +++ b/src/metabase/models/activity.clj @@ -52,7 +52,7 @@ :details {}}] (merge defaults activity))) -(u/strict-extend (class 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}) diff --git a/src/metabase/models/application_permissions_revision.clj b/src/metabase/models/application_permissions_revision.clj index cfa1256bd4c..958491dc41c 100644 --- a/src/metabase/models/application_permissions_revision.clj +++ b/src/metabase/models/application_permissions_revision.clj @@ -6,7 +6,7 @@ (models/defmodel ApplicationPermissionsRevision :application_permissions_revision) -(u/strict-extend (class ApplicationPermissionsRevision) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ApplicationPermissionsRevision) models/IModel (merge models/IModelDefaults {:types (constantly {:before :json diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index 539f7212a2f..00a0f96ba1f 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -286,7 +286,7 @@ :in mi/json-in :out result-metadata-out) -(u/strict-extend (class Card) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Card) models/IModel (merge models/IModelDefaults {:hydration-keys (constantly [:card]) diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj index 17824327e6a..745c2da3ada 100644 --- a/src/metabase/models/collection.clj +++ b/src/metabase/models/collection.clj @@ -886,7 +886,7 @@ (serdes.hash/identity-hash (db/select-one Collection :id parent-id)) "ROOT"))) -(u/strict-extend (class Collection) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Collection) models/IModel (merge models/IModelDefaults {:hydration-keys (constantly [:collection]) diff --git a/src/metabase/models/collection_permission_graph_revision.clj b/src/metabase/models/collection_permission_graph_revision.clj index ec33e4e4df6..5fdd3a08225 100644 --- a/src/metabase/models/collection_permission_graph_revision.clj +++ b/src/metabase/models/collection_permission_graph_revision.clj @@ -6,7 +6,7 @@ (models/defmodel CollectionPermissionGraphRevision :collection_permission_graph_revision) -(u/strict-extend (class CollectionPermissionGraphRevision) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class CollectionPermissionGraphRevision) models/IModel (merge models/IModelDefaults {:types (constantly {:before :json diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj index 6c05b604922..8d802c7d31a 100644 --- a/src/metabase/models/dashboard.clj +++ b/src/metabase/models/dashboard.clj @@ -129,7 +129,7 @@ [dashboard] (update-dashboard-subscription-pulses! dashboard)) -(u/strict-extend (class Dashboard) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Dashboard) models/IModel (merge models/IModelDefaults {:properties (constantly {:timestamped? true @@ -233,7 +233,7 @@ (->> (filter identity) build-sentence)))) -(u/strict-extend (class Dashboard) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Dashboard) revision/IRevisioned (merge revision/IRevisionedDefaults {:serialize-instance (fn [_ _ dashboard] (serialize-dashboard dashboard)) diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj index aac585a40df..b6094703e69 100644 --- a/src/metabase/models/dashboard_card.clj +++ b/src/metabase/models/dashboard_card.clj @@ -39,7 +39,7 @@ :visualization_settings {}}] (merge defaults dashcard))) -(u/strict-extend (class DashboardCard) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class DashboardCard) models/IModel (merge models/IModelDefaults {:properties (constantly {:timestamped? true diff --git a/src/metabase/models/dashboard_card_series.clj b/src/metabase/models/dashboard_card_series.clj index 3d5ff22e05f..b632301dcc2 100644 --- a/src/metabase/models/dashboard_card_series.clj +++ b/src/metabase/models/dashboard_card_series.clj @@ -9,7 +9,7 @@ (defn- dashboard-card [{:keys [dashboardcard_id]}] (db/select-one 'DashboardCard :id dashboardcard_id)) -(u/strict-extend (class DashboardCardSeries) +(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)])}) diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj index e6c764d2a0a..9a07ae7a374 100644 --- a/src/metabase/models/database.clj +++ b/src/metabase/models/database.clj @@ -189,7 +189,7 @@ :read (perms/data-perms-path db-id) :write (perms/db-details-write-perms-path db-id))}) -(u/strict-extend (class Database) +(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]) diff --git a/src/metabase/models/dimension.clj b/src/metabase/models/dimension.clj index 38699941c45..0b8ad58a834 100644 --- a/src/metabase/models/dimension.clj +++ b/src/metabase/models/dimension.clj @@ -15,7 +15,7 @@ (models/defmodel Dimension :dimension) -(u/strict-extend (class 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}) diff --git a/src/metabase/models/dispatch.clj b/src/metabase/models/dispatch.clj new file mode 100644 index 00000000000..94b80a859ae --- /dev/null +++ b/src/metabase/models/dispatch.clj @@ -0,0 +1,56 @@ +(ns metabase.models.dispatch + "Helpers to assist in the transition to Toucan 2. Once we switch to Toucan 2 this stuff shouldn't be needed, but we + can update this namespace instead of having to update code all over the place." + (:require + [schema.core :as s] + [toucan.db :as db] + [toucan.models :as models])) + +(defn toucan-instance? + "True if `x` is a Toucan instance, but not a Toucan model." + [x] + (and (record? x) + (extends? models/IModel (class x)) + (not (models/model? x)))) + +(defn instance-of? + "Is `x` an instance of a some Toucan `model`? Use this instead of of using the `<Model>Instance` or calling [[type]] + or [[class]] on a model yourself, since that won't work once we switch to Toucan 2." + [model x] + (let [model (db/resolve-model model)] + (instance? (class model) x))) + +(defn InstanceOf + "Helper for creating a schema to check whether something is an instance of `model`. Use this instead of of using the + `<Model>Instance` or calling [[type]] or [[class]] on a model yourself, since that won't work once we switch to + Toucan 2. + + (s/defn my-fn :- (mi/InstanceOf User) + [] + ...)" + [model] + (s/pred (fn [x] + (instance-of? model x)) + (format "instance of a %s" (name model)))) + +(defn model + "Given either a Toucan model or a Toucan instance, return the Toucan model. Otherwise return `nil`." + [x] + (cond + (models/model? x) + x + + (toucan-instance? x) + (let [model-symb (symbol (name x))] + (db/resolve-model model-symb)) + + :else + nil)) + +(defn instance + "Create a new instance of Toucan `model` with a map `m`. + + (instance User {:first_name \"Cam\"})" + [model m] + (let [model (db/resolve-model model)] + (into (empty model) m))) diff --git a/src/metabase/models/emitter.clj b/src/metabase/models/emitter.clj index 68cb7d7a3df..6c73e3482b4 100644 --- a/src/metabase/models/emitter.clj +++ b/src/metabase/models/emitter.clj @@ -25,13 +25,12 @@ :in (comp mi/json-in normalize-parameter-mappings) :out (comp (mi/catch-normalization-exceptions normalize-parameter-mappings) mi/json-out-with-keywordization)) -(u/strict-extend - (class Emitter) - models/IModel - (merge models/IModelDefaults - {:types (constantly {:parameter_mappings ::parameter-mappings - :options :json}) - :properties (constantly {:timestamped? true})})) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Emitter) + models/IModel + (merge models/IModelDefaults + {:types (constantly {:parameter_mappings ::parameter-mappings + :options :json}) + :properties (constantly {:timestamped? true})})) (def ^:private Emitter-subtype-IModel-impl "[[models/IModel]] impl for `DashboardEmitter` and `CardEmitter`" @@ -39,11 +38,11 @@ ; This is ok as long as we're 1:1 {:primary-key (constantly :emitter_id)})) -(u/strict-extend (class DashboardEmitter) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class DashboardEmitter) models/IModel Emitter-subtype-IModel-impl) -(u/strict-extend (class CardEmitter) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class CardEmitter) models/IModel Emitter-subtype-IModel-impl) @@ -52,10 +51,10 @@ {:batched-hydrate :emitters} [items] (if-let [[emitter-type join-column] (cond - (every? #(= (class Dashboard) (type %)) items) + (every? #(mi/instance-of? Dashboard %) items) [DashboardEmitter :dashboard_id] - (every? #(= (class Card) (type %)) items) + (every? #(mi/instance-of? Card %) items) [CardEmitter :card_id])] (let [qualified-join-column (keyword (str "typed_emitter." (name join-column))) emitters (->> {:select [qualified-join-column diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj index 15a78385dc9..ab63c363207 100644 --- a/src/metabase/models/field.clj +++ b/src/metabase/models/field.clj @@ -176,7 +176,7 @@ :out (comp update-semantic-numeric-values mi/json-out-with-keywordization)) -(u/strict-extend (class Field) +(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]) diff --git a/src/metabase/models/field_values.clj b/src/metabase/models/field_values.clj index 8001589b108..c1ebdd11bf6 100644 --- a/src/metabase/models/field_values.clj +++ b/src/metabase/models/field_values.clj @@ -155,7 +155,7 @@ :else []))))) -(u/strict-extend (class FieldValues) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class FieldValues) models/IModel (merge models/IModelDefaults {:properties (constantly {:timestamped? true}) diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj index 027e6060c2a..1b8360e6a8c 100644 --- a/src/metabase/models/interface.clj +++ b/src/metabase/models/interface.clj @@ -7,11 +7,13 @@ [metabase.db.connection :as mdb.connection] [metabase.mbql.normalize :as mbql.normalize] [metabase.mbql.schema :as mbql.s] + [metabase.models.dispatch :as models.dispatch] [metabase.plugins.classloader :as classloader] [metabase.util :as u] [metabase.util.cron :as u.cron] [metabase.util.encryption :as encryption] [metabase.util.i18n :refer [trs tru]] + [potemkin :as p] [potemkin.types :as p.types] [schema.core :as s] [taoensso.nippy :as nippy] @@ -20,6 +22,16 @@ java.sql.Blob java.util.zip.GZIPInputStream)) +(comment models.dispatch/keep-me) + +(p/import-vars + [models.dispatch + toucan-instance? + instance-of? + InstanceOf + model + instance]) + ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | Toucan Extensions | ;;; +----------------------------------------------------------------------------------------------------------------+ @@ -355,3 +367,24 @@ permissions for the paths returned by its implementation of `perms-objects-set`. (`read-or-write` is either `:read` or `:write` and passed to `perms-objects-set`; you'll usually want to partially bind it in the implementation map)." (partial check-perms-with-fn 'metabase.models.permissions/set-has-partial-permissions-for-set?)) + +;;;; redefs + +;;; swap out [[models/defmodel]] with a special magical version that avoids redefining stuff if the definition has not +;;; changed at all. This is important to make the stuff in [[models.dispatch]] work properly, since we're dispatching +;;; off of the model objects themselves e.g. [[metabase.models.user/User]] -- it is important that they do not change + +(defonce ^:private original-defmodel @(resolve `models/defmodel)) + +(defmacro ^:private defmodel [model & args] + (let [varr (ns-resolve *ns* model) + existing-hash (some-> varr meta ::defmodel-hash) + has-same-hash? (= existing-hash (hash &form))] + (when has-same-hash? + (println model "has not changed, skipping redefinition")) + (when-not has-same-hash? + `(do + ~(apply original-defmodel &form &env model args) + (alter-meta! (var ~model) assoc ::defmodel-hash ~(hash &form)))))) + +(alter-var-root #'models/defmodel (constantly @#'defmodel)) diff --git a/src/metabase/models/login_history.clj b/src/metabase/models/login_history.clj index 16886ee6e1d..7df7c38d68d 100644 --- a/src/metabase/models/login_history.clj +++ b/src/metabase/models/login_history.clj @@ -85,7 +85,7 @@ (defn- pre-update [_login-history] (throw (RuntimeException. (tru "You can''t update a LoginHistory after it has been created.")))) -(extend (class LoginHistory) +(extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class LoginHistory) models/IModel (merge models/IModelDefaults diff --git a/src/metabase/models/metric.clj b/src/metabase/models/metric.clj index 7c5a4d7cbe1..27272775114 100644 --- a/src/metabase/models/metric.clj +++ b/src/metabase/models/metric.clj @@ -31,7 +31,7 @@ (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 (class Metric) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Metric) models/IModel (merge models/IModelDefaults @@ -73,7 +73,7 @@ (get-in base-diff [:before :definition])) (assoc :definition {:before (get-in metric1 [:definition]) :after (get-in metric2 [:definition])}))))) -(u/strict-extend (class Metric) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Metric) revision/IRevisioned (merge revision/IRevisionedDefaults {:serialize-instance serialize-metric diff --git a/src/metabase/models/metric_important_field.clj b/src/metabase/models/metric_important_field.clj index 25455713965..a943ba00b4a 100644 --- a/src/metabase/models/metric_important_field.clj +++ b/src/metabase/models/metric_important_field.clj @@ -6,7 +6,7 @@ (models/defmodel MetricImportantField :metric_important_field) -(u/strict-extend (class MetricImportantField) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class MetricImportantField) models/IModel (merge models/IModelDefaults {:types (constantly {:definition :json})}) diff --git a/src/metabase/models/moderation_review.clj b/src/metabase/models/moderation_review.clj index 4e17c62c056..0cd18e8fe3d 100644 --- a/src/metabase/models/moderation_review.clj +++ b/src/metabase/models/moderation_review.clj @@ -29,7 +29,7 @@ (models/defmodel ModerationReview :moderation_review) -(u/strict-extend (class ModerationReview) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ModerationReview) models/IModel (merge models/IModelDefaults {:properties (constantly {:timestamped? true}) diff --git a/src/metabase/models/native_query_snippet.clj b/src/metabase/models/native_query_snippet.clj index 80d1c745a3a..94227de2a5e 100644 --- a/src/metabase/models/native_query_snippet.clj +++ b/src/metabase/models/native_query_snippet.clj @@ -16,7 +16,7 @@ (models/defmodel NativeQuerySnippet :native_query_snippet) -(defmethod collection/allowed-namespaces (class NativeQuerySnippet) +(defmethod collection/allowed-namespaces #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class NativeQuerySnippet) [_] #{:snippets}) @@ -32,7 +32,7 @@ (throw (UnsupportedOperationException. (tru "You cannot update the creator_id of a NativeQuerySnippet."))))) (collection/check-collection-namespace NativeQuerySnippet (:collection_id updates)))) -(u/strict-extend (class NativeQuerySnippet) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class NativeQuerySnippet) models/IModel (merge models/IModelDefaults diff --git a/src/metabase/models/permissions.clj b/src/metabase/models/permissions.clj index 8ef51eed639..a559125b080 100644 --- a/src/metabase/models/permissions.clj +++ b/src/metabase/models/permissions.clj @@ -622,7 +622,7 @@ (:object permissions)))) (assert-not-admin-group permissions)) -(u/strict-extend (class 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 diff --git a/src/metabase/models/permissions_group.clj b/src/metabase/models/permissions_group.clj index ab20f0cc1fe..24b83904168 100644 --- a/src/metabase/models/permissions_group.clj +++ b/src/metabase/models/permissions_group.clj @@ -99,7 +99,7 @@ (when group-name (check-name-not-already-taken group-name)))) -(u/strict-extend (class PermissionsGroup) +(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 diff --git a/src/metabase/models/permissions_group_membership.clj b/src/metabase/models/permissions_group_membership.clj index 4118e8da5b7..2be6d616992 100644 --- a/src/metabase/models/permissions_group_membership.clj +++ b/src/metabase/models/permissions_group_membership.clj @@ -53,7 +53,7 @@ (db/update! 'User user_id :is_superuser true)))) -(u/strict-extend (class PermissionsGroupMembership) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PermissionsGroupMembership) models/IModel (merge models/IModelDefaults {:pre-delete pre-delete diff --git a/src/metabase/models/permissions_revision.clj b/src/metabase/models/permissions_revision.clj index 3ec6a0e25e6..9bad8352399 100644 --- a/src/metabase/models/permissions_revision.clj +++ b/src/metabase/models/permissions_revision.clj @@ -6,7 +6,7 @@ (models/defmodel PermissionsRevision :permissions_revision) -(u/strict-extend (class PermissionsRevision) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PermissionsRevision) models/IModel (merge models/IModelDefaults {:types (constantly {:before :json diff --git a/src/metabase/models/persisted_info.clj b/src/metabase/models/persisted_info.clj index 9fd2c72ccfb..fa562207eab 100644 --- a/src/metabase/models/persisted_info.clj +++ b/src/metabase/models/persisted_info.clj @@ -64,7 +64,7 @@ (models/defmodel PersistedInfo :persisted_info) -(u/strict-extend (class PersistedInfo) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PersistedInfo) models/IModel (merge models/IModelDefaults {:types (constantly {:definition ::definition})})) diff --git a/src/metabase/models/pulse.clj b/src/metabase/models/pulse.clj index c8b0fb7ebc0..284610b9913 100644 --- a/src/metabase/models/pulse.clj +++ b/src/metabase/models/pulse.clj @@ -122,7 +122,7 @@ (= api/*current-user-id* (:creator_id notification)) (mi/current-user-has-full-permissions? :write notification))) -(u/strict-extend (class Pulse) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Pulse) models/IModel (merge models/IModelDefaults diff --git a/src/metabase/models/pulse_card.clj b/src/metabase/models/pulse_card.clj index 69e0077d776..2cf54373ecd 100644 --- a/src/metabase/models/pulse_card.clj +++ b/src/metabase/models/pulse_card.clj @@ -10,7 +10,7 @@ (models/defmodel PulseCard :pulse_card) -(u/strict-extend (class PulseCard) +(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})}) diff --git a/src/metabase/models/pulse_channel.clj b/src/metabase/models/pulse_channel.clj index e10e47abac1..84aca7b4cc4 100644 --- a/src/metabase/models/pulse_channel.clj +++ b/src/metabase/models/pulse_channel.clj @@ -180,7 +180,7 @@ (throw (ex-info (tru "Wrong email address for User {0}." id) {:status-code 403}))))))))) -(u/strict-extend (class PulseChannel) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PulseChannel) models/IModel (merge models/IModelDefaults diff --git a/src/metabase/models/pulse_channel_recipient.clj b/src/metabase/models/pulse_channel_recipient.clj index 104b2e352ba..20a26e37f76 100644 --- a/src/metabase/models/pulse_channel_recipient.clj +++ b/src/metabase/models/pulse_channel_recipient.clj @@ -11,7 +11,7 @@ (classloader/require 'metabase.models.pulse-channel) ((resolve 'metabase.models.pulse-channel/will-delete-recipient) pcr)) -(u/strict-extend (class PulseChannelRecipient) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class PulseChannelRecipient) models/IModel (merge models/IModelDefaults diff --git a/src/metabase/models/query.clj b/src/metabase/models/query.clj index 922fd10c719..98c517d8b71 100644 --- a/src/metabase/models/query.clj +++ b/src/metabase/models/query.clj @@ -10,7 +10,7 @@ (models/defmodel Query :query) -(u/strict-extend (class 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}) diff --git a/src/metabase/models/query_cache.clj b/src/metabase/models/query_cache.clj index eb4950709ee..ab6b5a6da45 100644 --- a/src/metabase/models/query_cache.clj +++ b/src/metabase/models/query_cache.clj @@ -5,7 +5,7 @@ (models/defmodel QueryCache :query_cache) -(u/strict-extend (class QueryCache) +(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})})) diff --git a/src/metabase/models/query_execution.clj b/src/metabase/models/query_execution.clj index 3de2a2e3f37..8186341effc 100644 --- a/src/metabase/models/query_execution.clj +++ b/src/metabase/models/query_execution.clj @@ -20,7 +20,7 @@ ;; sadly we have 2 ways to reference the row count :( (assoc query-execution :row_count (or result_rows 0))) -(u/strict-extend (class QueryExecution) +(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}) diff --git a/src/metabase/models/revision.clj b/src/metabase/models/revision.clj index b27e4c95d60..6903027a7f0 100644 --- a/src/metabase/models/revision.clj +++ b/src/metabase/models/revision.clj @@ -77,7 +77,7 @@ (cond-> revision model (update :object (partial models/do-post-select model))))) -(u/strict-extend (class Revision) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Revision) models/IModel (merge models/IModelDefaults {:types (constantly {:object :json}) diff --git a/src/metabase/models/secret.clj b/src/metabase/models/secret.clj index b0cc378dadc..3cee10e5581 100644 --- a/src/metabase/models/secret.clj +++ b/src/metabase/models/secret.clj @@ -21,7 +21,7 @@ (models/defmodel Secret :secret) -(u/strict-extend (class Secret) +(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 @@ -282,7 +282,7 @@ secret* (cond (int? secret-or-id) (db/select-one Secret :id secret-or-id) - (instance? (class Secret) secret-or-id) + (mi/instance-of? Secret secret-or-id) secret-or-id :else ; default; app DB look up from the ID in db-details diff --git a/src/metabase/models/segment.clj b/src/metabase/models/segment.clj index 570fa308967..0c0c3a5b37c 100644 --- a/src/metabase/models/segment.clj +++ b/src/metabase/models/segment.clj @@ -30,7 +30,7 @@ (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 (class Segment) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Segment) models/IModel (merge models/IModelDefaults @@ -74,7 +74,7 @@ :after (get-in segment2 [:definition])}))))) -(u/strict-extend (class Segment) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Segment) revision/IRevisioned (merge revision/IRevisionedDefaults diff --git a/src/metabase/models/session.clj b/src/metabase/models/session.clj index a2388cc075e..95051fe5370 100644 --- a/src/metabase/models/session.clj +++ b/src/metabase/models/session.clj @@ -24,7 +24,7 @@ (let [session-type (if anti-csrf-token :full-app-embed :normal)] (assoc session :type session-type))) -(u/strict-extend (class Session) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Session) models/IModel (merge models/IModelDefaults diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj index c1b4174c2e9..4782028b465 100644 --- a/src/metabase/models/setting.clj +++ b/src/metabase/models/setting.clj @@ -139,7 +139,7 @@ "The model that underlies [[defsetting]]." :setting) -(u/strict-extend (class 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}) diff --git a/src/metabase/models/table.clj b/src/metabase/models/table.clj index 3f74726f6ea..b470db2e327 100644 --- a/src/metabase/models/table.clj +++ b/src/metabase/models/table.clj @@ -63,7 +63,7 @@ :read (perms/table-read-path table) :write (perms/data-model-write-perms-path db-id schema table-id))}) -(u/strict-extend (class Table) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Table) models/IModel (merge models/IModelDefaults {:hydration-keys (constantly [:table]) diff --git a/src/metabase/models/task_history.clj b/src/metabase/models/task_history.clj index 360afa76feb..a55cd44e3d1 100644 --- a/src/metabase/models/task_history.clj +++ b/src/metabase/models/task_history.clj @@ -60,7 +60,7 @@ (u/prog1 task (snowplow/track-event! ::snowplow/new-task-history *current-user-id* (task->snowplow-event <>)))) -(u/strict-extend (class TaskHistory) +(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}) diff --git a/src/metabase/models/timeline.clj b/src/metabase/models/timeline.clj index 441f647d37b..778d5fb663c 100644 --- a/src/metabase/models/timeline.clj +++ b/src/metabase/models/timeline.clj @@ -50,7 +50,7 @@ (nil? collection-id) (->> (map hydrate-root-collection)) events? (timeline-event/include-events options))) -(u/strict-extend (class Timeline) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Timeline) models/IModel (merge models/IModelDefaults diff --git a/src/metabase/models/timeline_event.clj b/src/metabase/models/timeline_event.clj index 438ddfaca7f..9cd73d49ed6 100644 --- a/src/metabase/models/timeline_event.clj +++ b/src/metabase/models/timeline_event.clj @@ -81,7 +81,7 @@ ;;;; model -(u/strict-extend (class TimelineEvent) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class TimelineEvent) models/IModel (merge models/IModelDefaults diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj index 884d10b9e8b..f2f82f73a15 100644 --- a/src/metabase/models/user.clj +++ b/src/metabase/models/user.clj @@ -133,7 +133,7 @@ "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 (class User) +(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) diff --git a/src/metabase/models/view_log.clj b/src/metabase/models/view_log.clj index 428cfeeaced..6108a463f58 100644 --- a/src/metabase/models/view_log.clj +++ b/src/metabase/models/view_log.clj @@ -10,7 +10,7 @@ (let [defaults {:timestamp :%now}] (merge defaults log-entry))) -(u/strict-extend (class ViewLog) +(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ViewLog) models/IModel (merge models/IModelDefaults {:pre-insert pre-insert diff --git a/src/metabase/query_processor/middleware/wrap_value_literals.clj b/src/metabase/query_processor/middleware/wrap_value_literals.clj index 9e6701e57bb..3a2efa586fa 100644 --- a/src/metabase/query_processor/middleware/wrap_value_literals.clj +++ b/src/metabase/query_processor/middleware/wrap_value_literals.clj @@ -21,8 +21,9 @@ (defmethod type-info :default [_] nil) -(defmethod type-info (class Field) [this] - (let [field-info (select-keys this [:base_type :effective_type :coercion_strategy :semantic_type :database_type :name])] +(defmethod type-info Field + [field] + (let [field-info (select-keys field [:base_type :effective_type :coercion_strategy :semantic_type :database_type :name])] (merge field-info ;; add in a default unit for this Field so we know to wrap datetime strings in `absolute-datetime` below based on diff --git a/src/metabase/query_processor/store.clj b/src/metabase/query_processor/store.clj index 0b26742cd72..b8f2f220846 100644 --- a/src/metabase/query_processor/store.clj +++ b/src/metabase/query_processor/store.clj @@ -14,6 +14,7 @@ those Fields potentially dozens of times in a single query execution." (:require [metabase.models.database :refer [Database]] [metabase.models.field :refer [Field]] + [metabase.models.interface :as mi] [metabase.models.table :refer [Table]] [metabase.util :as u] [metabase.util.i18n :refer [tru]] @@ -60,7 +61,7 @@ (def ^:private DatabaseInstanceWithRequiredStoreKeys (s/both - (class Database) + (mi/InstanceOf Database) {:id su/IntGreaterThanZero :engine s/Keyword :name su/NonBlankString @@ -77,7 +78,7 @@ (def ^:private TableInstanceWithRequiredStoreKeys (s/both - (class Table) + (mi/InstanceOf Table) {:schema (s/maybe s/Str) :name su/NonBlankString s/Any s/Any})) @@ -105,7 +106,7 @@ (def ^:private FieldInstanceWithRequiredStorekeys (s/both - (class Field) + (mi/InstanceOf Field) {:name su/NonBlankString :display_name su/NonBlankString :description (s/maybe s/Str) diff --git a/src/metabase/related.clj b/src/metabase/related.clj index a364f9e0669..5e4bd7c144b 100644 --- a/src/metabase/related.clj +++ b/src/metabase/related.clj @@ -38,33 +38,32 @@ (map #(update % 0 qp.util/normalize-token))) (tree-seq sequential? identity form)))) -(defmulti - ^{:doc "Return the relevant parts of a given entity's definition. - Relevant parts are those that carry semantic meaning, and especially - context-bearing forms." - :arglists '([entity])} - definition type) +(defmulti definition + "Return the relevant parts of a given entity's definition. Relevant parts are those that carry semantic meaning, and + especially context-bearing forms." + {:arglists '([instance])} + mi/model) -(defmethod definition (type Card) +(defmethod definition Card [card] (-> card :dataset_query :query ((juxt :breakout :aggregation :expressions :fields)))) -(defmethod definition (type Metric) +(defmethod definition Metric [metric] (-> metric :definition ((juxt :aggregation :filter)))) -(defmethod definition (type Segment) +(defmethod definition Segment [segment] (-> segment :definition :filter)) -(defmethod definition (type Field) +(defmethod definition Field [field] [[:field-id (:id field)]]) -(defn similarity +(defn- similarity "How similar are entities `a` and `b` based on a structural comparison of their definition (MBQL). For the purposes of finding related entites we are only interested in @@ -206,12 +205,12 @@ (keep (comp Collection :collection_id)) filter-visible)) -(defmulti - ^{:doc "Return related entities." - :arglists '([entity])} - related type) +(defmulti related + "Return related entities." + {:arglists '([entity])} + mi/model) -(defmethod related (type Card) +(defmethod related Card [card] (let [table (db/select-one Table :id (:table_id card)) similar-questions (similar-questions card)] @@ -230,11 +229,11 @@ :dashboards (recommended-dashboards similar-questions) :collections (recommended-collections similar-questions)})) -(defmethod related (type Query) +(defmethod related Query [query] - (related (with-meta query {:type (type Card)}))) + (related (mi/instance Card query))) -(defmethod related (type Metric) +(defmethod related Metric [metric] (let [table (db/select-one Table :id (:table_id metric))] {:table table @@ -247,7 +246,7 @@ (rank-by-similarity metric) interesting-mix)})) -(defmethod related (type Segment) +(defmethod related Segment [segment] (let [table (db/select-one Table :id (:table_id segment))] {:table table @@ -261,7 +260,7 @@ interesting-mix) :linked-from (linked-from table)})) -(defmethod related (type Table) +(defmethod related Table [table] (let [linking-to (linking-to table) linked-from (linked-from table)] @@ -279,7 +278,7 @@ filter-visible interesting-mix)})) -(defmethod related (type Field) +(defmethod related Field [field] (let [table (db/select-one Table :id (:table_id field))] {:table table @@ -300,7 +299,7 @@ filter-visible interesting-mix)})) -(defmethod related (type Dashboard) +(defmethod related Dashboard [dashboard] (let [cards (map Card (db/select-field :card_id DashboardCard :dashboard_id (:id dashboard)))] diff --git a/src/metabase/sync/analyze/classify.clj b/src/metabase/sync/analyze/classify.clj index 12013bedc7f..f270d8ce64b 100644 --- a/src/metabase/sync/analyze/classify.clj +++ b/src/metabase/sync/analyze/classify.clj @@ -19,6 +19,7 @@ (:require [clojure.data :as data] [clojure.tools.logging :as log] [metabase.models.field :refer [Field]] + [metabase.models.interface :as mi] [metabase.models.table :refer [Table]] [metabase.sync.analyze.classifiers.category :as classifiers.category] [metabase.sync.analyze.classifiers.name :as classifiers.name] @@ -55,7 +56,7 @@ (throw (Exception. (format "Classifiers are not allowed to set the value of %s." k))))) ;; cool, now we should be ok to update the model (when values-to-set - (db/update! (if (instance? (type Field) original-model) + (db/update! (if (mi/instance-of? Field original-model) Field Table) (u/the-id original-model) diff --git a/src/metabase/sync/interface.clj b/src/metabase/sync/interface.clj index 79a100e3515..f51ef4bc7b2 100644 --- a/src/metabase/sync/interface.clj +++ b/src/metabase/sync/interface.clj @@ -3,6 +3,7 @@ (:require [clj-time.core :as time] [metabase.models.database :refer [Database]] [metabase.models.field :refer [Field]] + [metabase.models.interface :as mi] [metabase.models.table :refer [Table]] [metabase.util :as u] [metabase.util.schema :as su] @@ -71,10 +72,10 @@ ;; out from the ns declaration when running `cljr-clean-ns`. Plus as a bonus in the future we could add additional ;; validations to these, e.g. requiring that a Field have a base_type -(def DatabaseInstance "Schema for a valid instance of a Metabase Database." (class Database)) -(def TableInstance "Schema for a valid instance of a Metabase Table." (class Table)) -(def FieldInstance "Schema for a valid instance of a Metabase Field." (class Field)) -(def ResultColumnMetadataInstance "Schema for a valid instance of a Metabase Field." (class {})) +(def DatabaseInstance "Schema for a valid instance of a Metabase Database." (mi/InstanceOf Database)) +(def TableInstance "Schema for a valid instance of a Metabase Table." (mi/InstanceOf Table)) +(def FieldInstance "Schema for a valid instance of a Metabase Field." (mi/InstanceOf Field)) +(def ResultColumnMetadataInstance "Schema for result column metadata." su/Map) ;;; +----------------------------------------------------------------------------------------------------------------+ diff --git a/src/metabase/sync/util.clj b/src/metabase/sync/util.clj index 13d1bcf7604..68bddd08962 100644 --- a/src/metabase/sync/util.clj +++ b/src/metabase/sync/util.clj @@ -11,6 +11,8 @@ [metabase.driver.util :as driver.u] [metabase.events :as events] [metabase.models.database :refer [Database]] + [metabase.models.field :refer [Field]] + [metabase.models.interface :as mi] [metabase.models.table :refer [Table]] [metabase.models.task-history :refer [TaskHistory]] [metabase.query-processor.interface :as qp.i] @@ -288,31 +290,29 @@ [database-or-id] (db/select Table, :db_id (u/the-id database-or-id), :active true, :visibility_type nil)) +(defmulti name-for-logging + "Return an appropriate string for logging an object in sync logging messages. Should be something like -;; The `name-for-logging` function is used all over the sync code to make sure we have easy access to consistently -;; formatted descriptions of various objects. + \"postgres Database 'test-data'\" -(defprotocol ^:private NameForLogging - (name-for-logging [this] - "Return an appropriate string for logging an object in sync logging messages. Should be something like \"postgres - Database 'test-data'\"")) + This function is used all over the sync code to make sure we have easy access to consistently formatted descriptions + of various objects." + {:arglists '([instance])} + mi/model) -(extend-protocol NameForLogging - i/DatabaseInstance - (name-for-logging [{database-name :name, id :id, engine :engine,}] - (trs "{0} Database {1} ''{2}''" (name engine) (or id "") database-name)) +(defmethod name-for-logging Database + [{database-name :name, id :id, engine :engine,}] + (trs "{0} Database {1} ''{2}''" (name engine) (or id "") database-name)) - i/TableInstance - (name-for-logging [{schema :schema, id :id, table-name :name}] - (trs "Table {0} ''{1}''" (or id "") (str (when (seq schema) (str schema ".")) table-name))) +(defmethod name-for-logging Table [{schema :schema, id :id, table-name :name}] + (trs "Table {0} ''{1}''" (or id "") (str (when (seq schema) (str schema ".")) table-name))) - i/FieldInstance - (name-for-logging [{field-name :name, id :id}] - (trs "Field {0} ''{1}''" (or id "") field-name)) +(defmethod name-for-logging Field [{field-name :name, id :id}] + (trs "Field {0} ''{1}''" (or id "") field-name)) - i/ResultColumnMetadataInstance - (name-for-logging [{field-name :name}] - (trs "Field ''{0}''" field-name))) +;;; this is used for result metadata stuff. +(defmethod name-for-logging :default [{field-name :name}] + (trs "Field ''{0}''" field-name)) (defn calculate-hash "Calculate a cryptographic hash on `clj-data` and return that hash as a string" diff --git a/src/metabase/transforms/core.clj b/src/metabase/transforms/core.clj index 2c03761f229..981d4ba32d1 100644 --- a/src/metabase/transforms/core.clj +++ b/src/metabase/transforms/core.clj @@ -6,6 +6,7 @@ [metabase.mbql.schema :as mbql.s] [metabase.mbql.util :as mbql.u] [metabase.models.field :refer [Field]] + [metabase.models.interface :as mi] [metabase.models.table :as table :refer [Table]] [metabase.query-processor :as qp] [metabase.transforms.materialize :as tf.materialize] @@ -83,7 +84,7 @@ (s/defn ^:private ->source-table-reference "Serialize `entity` into a form suitable as `:source-table` value." [entity :- SourceEntity] - (if (instance? (type Table) entity) + (if (mi/instance-of? Table entity) (u/the-id entity) (str "card__" (u/the-id entity)))) @@ -126,7 +127,7 @@ (assoc bindings name {:entity (tf.materialize/make-card-for-step! step query) :dimensions (infer-resulting-dimensions local-bindings step query)}))) -(def ^:private Tableset [(type Table)]) +(def ^:private Tableset [(mi/InstanceOf Table)]) (s/defn ^:private find-tables-with-domain-entity :- Tableset [tableset :- Tableset, domain-entity-spec :- DomainEntitySpec] diff --git a/src/metabase/util.clj b/src/metabase/util.clj index 57c52feb851..6cca399f11c 100644 --- a/src/metabase/util.clj +++ b/src/metabase/util.clj @@ -374,7 +374,7 @@ 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 1} + {:style/indent :defn} [atype protocol method-map & more] (check-protocol-impl-method-map protocol method-map) (extend atype protocol method-map) @@ -382,7 +382,7 @@ (apply strict-extend atype more))) (defn remove-diacritical-marks - "Return a version of S with diacritical marks removed." + "Return a version of `s` with diacritical marks removed." ^String [^String s] (when (seq s) (str/replace diff --git a/test/metabase/actions/test_util.clj b/test/metabase/actions/test_util.clj index 96cc2b2611c..15b9d5047ea 100644 --- a/test/metabase/actions/test_util.clj +++ b/test/metabase/actions/test_util.clj @@ -178,11 +178,11 @@ (db/select-one-field :parameter_mappings Emitter :id emitter-id)))) ;; these are tied to the Card or Dashboad and Emitter above and will get cascade deleted. We can't use `with-temp*` for them ;; because it doesn't seem to work with tables with compound PKs - (condp = (type parent-model) - (type Card) + (condp = parent-model + Card (db/insert! CardEmitter {:card_id emitter-parent-id :emitter_id emitter-id}) - (type Dashboard) + Dashboard (db/insert! DashboardEmitter {:dashboard_id emitter-parent-id :emitter_id emitter-id})) (f (assoc context diff --git a/test/metabase/api/bookmark_test.clj b/test/metabase/api/bookmark_test.clj index c515c79fac4..431442b5f16 100644 --- a/test/metabase/api/bookmark_test.clj +++ b/test/metabase/api/bookmark_test.clj @@ -7,6 +7,7 @@ [metabase.models.card :refer [Card]] [metabase.models.collection :refer [Collection]] [metabase.models.dashboard :refer [Dashboard]] + [metabase.models.interface :as mi] [metabase.test :as mt] [metabase.util :as u] [toucan.db :as db])) @@ -56,17 +57,17 @@ (defn bookmark-models [user-id & models] (doseq [model models] (cond - (instance? (class Collection) model) + (mi/instance-of? Collection model) (db/insert! CollectionBookmark {:user_id user-id :collection_id (u/the-id model)}) - (instance? (class Card) model) + (mi/instance-of? Card model) (db/insert! CardBookmark {:user_id user-id :card_id (u/the-id model)}) - (instance? (class Dashboard) model) + (mi/instance-of? Dashboard model) (db/insert! DashboardBookmark {:user_id user-id :dashboard_id (u/the-id model)}) diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj index ef4f728b586..8c42bbcedc1 100644 --- a/test/metabase/api/embed_test.clj +++ b/test/metabase/api/embed_test.clj @@ -14,6 +14,7 @@ [metabase.api.public-test :as public-test] [metabase.http-client :as client] [metabase.models :refer [Card Dashboard DashboardCard DashboardCardSeries]] + [metabase.models.interface :as mi] [metabase.models.params.chain-filter-test :as chain-filer-test] [metabase.models.permissions :as perms] [metabase.models.permissions-group :as perms-group] @@ -577,9 +578,9 @@ (defn- field-values-url [card-or-dashboard field-or-id] (str "embed/" - (condp instance? card-or-dashboard - (class Card) (str "card/" (card-token card-or-dashboard)) - (class Dashboard) (str "dashboard/" (dash-token card-or-dashboard))) + (condp mi/instance-of? card-or-dashboard + Card (str "card/" (card-token card-or-dashboard)) + Dashboard (str "dashboard/" (dash-token card-or-dashboard))) "/field/" (u/the-id field-or-id) "/values")) @@ -690,9 +691,9 @@ (defn- field-search-url [card-or-dashboard field-or-id search-field-or-id] (str "embed/" - (condp instance? card-or-dashboard - (class Card) (str "card/" (card-token card-or-dashboard)) - (class Dashboard) (str "dashboard/" (dash-token card-or-dashboard))) + (condp mi/instance-of? card-or-dashboard + Card (str "card/" (card-token card-or-dashboard)) + Dashboard (str "dashboard/" (dash-token card-or-dashboard))) "/field/" (u/the-id field-or-id) "/search/" (u/the-id search-field-or-id))) @@ -733,9 +734,9 @@ (defn- field-remapping-url [card-or-dashboard field-or-id remapped-field-or-id] (str "embed/" - (condp instance? card-or-dashboard - (class Card) (str "card/" (card-token card-or-dashboard)) - (class Dashboard) (str "dashboard/" (dash-token card-or-dashboard))) + (condp mi/instance-of? card-or-dashboard + Card (str "card/" (card-token card-or-dashboard)) + Dashboard (str "dashboard/" (dash-token card-or-dashboard))) "/field/" (u/the-id field-or-id) "/remapping/" (u/the-id remapped-field-or-id))) diff --git a/test/metabase/api/emitter_test.clj b/test/metabase/api/emitter_test.clj index 49e2e96c030..2029e81d7fc 100644 --- a/test/metabase/api/emitter_test.clj +++ b/test/metabase/api/emitter_test.clj @@ -6,7 +6,8 @@ [metabase.models :refer [Card Dashboard Emitter]] [metabase.test :as mt] [metabase.util.schema :as su] - [schema.core :as s])) + [schema.core :as s] + [toucan.db :as db])) (deftest create-emitter-test (testing "POST /api/emitter" @@ -41,7 +42,7 @@ (testing "Should be able to update an emitter" (mt/user-http-request :crowberto :put 204 (format "emitter/%d" emitter-id) {:options {:a 1}}) - (is (partial= {:options {:a 1}} (Emitter emitter-id)))) + (is (partial= {:options {:a 1}} (db/select-one Emitter :id emitter-id)))) (testing "Should 404 if bad emitter-id" (is (= "Not found." (mt/user-http-request :crowberto :put 404 (format "emitter/%d" Integer/MAX_VALUE) diff --git a/test/metabase/api/public_test.clj b/test/metabase/api/public_test.clj index 3b8bee16e58..8b9356285e9 100644 --- a/test/metabase/api/public_test.clj +++ b/test/metabase/api/public_test.clj @@ -9,6 +9,7 @@ [metabase.api.public :as api.public] [metabase.http-client :as client] [metabase.models :refer [Card Collection Dashboard DashboardCard DashboardCardSeries Dimension Field FieldValues]] + [metabase.models.interface :as mi] [metabase.models.params.chain-filter-test :as chain-filter-test] [metabase.models.permissions :as perms] [metabase.models.permissions-group :as perms-group] @@ -755,9 +756,9 @@ (defn- field-values-url [card-or-dashboard field-or-id] (str "public/" - (condp instance? card-or-dashboard - (class Card) "card" - (class Dashboard) "dashboard") + (condp mi/instance-of? card-or-dashboard + Card "card" + Dashboard "dashboard") "/" (or (:public_uuid card-or-dashboard) (throw (Exception. (str "Missing public UUID: " card-or-dashboard)))) "/field/" (u/the-id field-or-id) @@ -868,9 +869,9 @@ (defn- field-search-url [card-or-dashboard field-or-id search-field-or-id] (str "public/" - (condp instance? card-or-dashboard - (class Card) "card" - (class Dashboard) "dashboard") + (condp mi/instance-of? card-or-dashboard + Card "card" + Dashboard "dashboard") "/" (:public_uuid card-or-dashboard) "/field/" (u/the-id field-or-id) "/search/" (u/the-id search-field-or-id))) @@ -934,9 +935,9 @@ (defn- field-remapping-url [card-or-dashboard field-or-id remapped-field-or-id] (str "public/" - (condp instance? card-or-dashboard - (class Card) "card" - (class Dashboard) "dashboard") + (condp mi/instance-of? card-or-dashboard + Card "card" + Dashboard "dashboard") "/" (:public_uuid card-or-dashboard) "/field/" (u/the-id field-or-id) "/remapping/" (u/the-id remapped-field-or-id))) diff --git a/test/metabase/automagic_dashboards/core_test.clj b/test/metabase/automagic_dashboards/core_test.clj index b41585c9180..c968925647e 100644 --- a/test/metabase/automagic_dashboards/core_test.clj +++ b/test/metabase/automagic_dashboards/core_test.clj @@ -9,6 +9,7 @@ [metabase.mbql.schema :as mbql.s] [metabase.models :refer [Card Collection Database Field Metric Table]] [metabase.models.field :as field] + [metabase.models.interface :as mi] [metabase.models.permissions :as perms] [metabase.models.permissions-group :as perms-group] [metabase.models.query :as query :refer [Query]] @@ -67,7 +68,8 @@ ;; that size limiting works. (testing (u/pprint-to-str (list 'automagic-analysis entity {:cell-query cell-query, :show :all})) (automagic-dashboards.test/test-dashboard-is-valid (magic/automagic-analysis entity {:cell-query cell-query, :show :all}) card-count)) - (when (or (nil? (#{(type Query) (type Card)} (type entity))) + (when (or (and (not (mi/instance-of? Query entity)) + (not (mi/instance-of? Card entity))) (#'magic/table-like? entity)) (testing (u/pprint-to-str (list 'automagic-analysis entity {:cell-query cell-query, :show 1})) ;; 1 for the actual card returned + 1 for the visual display card = 2 diff --git a/test/metabase/driver/sql_jdbc/connection_test.clj b/test/metabase/driver/sql_jdbc/connection_test.clj index 6572ebd03a3..a3e59006b1a 100644 --- a/test/metabase/driver/sql_jdbc/connection_test.clj +++ b/test/metabase/driver/sql_jdbc/connection_test.clj @@ -155,17 +155,18 @@ ;; restore the original test DB details, no matter what just happened (db/update! Database (mt/id) :details (:details db))))))) (testing "postgres secrets are stable (#23034)" - (mt/with-temp* [Secret [secret {:name "file based secret" - :kind :perm-cert - :source nil - :value (.getBytes "super secret")}]] - (let [db {:engine :postgres - :details {:ssl true - :ssl-mode "verify-ca" - :ssl-root-cert-options "uploaded" + (mt/with-temp* [Secret [secret {:name "file based secret" + :kind :perm-cert + :source nil + :value (.getBytes "super secret") + :creator_id (mt/user->id :crowberto)}]] + (let [db {:engine :postgres + :details {:ssl true + :ssl-mode "verify-ca" + :ssl-root-cert-options "uploaded" :ssl-root-cert-creator-id (mt/user->id :crowberto) - :ssl-root-cert-source nil - :ssl-root-cert-id (:id secret) + :ssl-root-cert-source nil + :ssl-root-cert-id (:id secret) :ssl-root-cert-created-at "2022-07-25T15:57:51.556-05:00"}}] (is (instance? java.io.File (:sslrootcert (#'sql-jdbc.conn/connection-details->spec :postgres diff --git a/test/metabase/models/action_test.clj b/test/metabase/models/action_test.clj index e1947756f19..849d592c97b 100644 --- a/test/metabase/models/action_test.clj +++ b/test/metabase/models/action_test.clj @@ -3,6 +3,7 @@ [metabase.actions.test-util :as actions.test-util] [metabase.models :refer [Emitter]] [metabase.test :as mt] + [toucan.db :as db] [toucan.hydrate :refer [hydrate]])) (deftest hydrate-query-action-test @@ -10,7 +11,7 @@ (actions.test-util/with-actions-test-data-and-actions-enabled (actions.test-util/with-action [{:keys [query-action-card-id action-id] :as context} {}] (actions.test-util/with-card-emitter [{:keys [emitter-id]} context] - (let [emitter (Emitter emitter-id) + (let [emitter (db/select-one Emitter :id emitter-id) hydrated-emitter (hydrate emitter :action)] (is (partial= {:id action-id @@ -24,7 +25,7 @@ (actions.test-util/with-actions-test-data-and-actions-enabled (actions.test-util/with-action [{:keys [action-id] :as context} {:type :http}] (actions.test-util/with-card-emitter [{:keys [emitter-id]} context] - (let [emitter (Emitter emitter-id) + (let [emitter (db/select-one Emitter :id emitter-id) hydrated-emitter (hydrate emitter :action)] (is (partial= {:id action-id diff --git a/test/metabase/models/dispatch_test.clj b/test/metabase/models/dispatch_test.clj new file mode 100644 index 00000000000..c71c216b0f0 --- /dev/null +++ b/test/metabase/models/dispatch_test.clj @@ -0,0 +1,38 @@ +(ns metabase.models.dispatch-test + (:require + [clojure.test :refer :all] + [metabase.mbql.util :as mbql.u] + [metabase.models.dispatch :as models.dispatch] + [metabase.models.user :as user :refer [User]] + [metabase.test :as mt] + [toucan.db :as db])) + +(defn- a-user [] + (db/select-one User :id (mt/user->id :rasta))) + +(deftest toucan-instance?-test + (is (models.dispatch/toucan-instance? (a-user))) + (is (not (models.dispatch/toucan-instance? User)))) + +(deftest model-test + (testing "model" + (is (identical? User + (models.dispatch/model User)))) + (testing "instance" + (is (identical? User + (models.dispatch/model (a-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) + (testing "model" + (is (identical? User + (mbql.u/dispatch-by-clause-name-or-class User)))) + (testing "instance" + (is (identical? User + (mbql.u/dispatch-by-clause-name-or-class (a-user))))))) + +(deftest instance-test + (is (= (user/map->UserInstance {:a 1}) + (models.dispatch/instance User {:a 1}))) + (is (identical? (class (user/map->UserInstance {:a 1})) + (class (models.dispatch/instance User {:a 1}))))) diff --git a/test/metabase/models/emitter_test.clj b/test/metabase/models/emitter_test.clj index b1328b5ed5e..cd33a795bd5 100644 --- a/test/metabase/models/emitter_test.clj +++ b/test/metabase/models/emitter_test.clj @@ -36,7 +36,7 @@ (actions.test-util/with-dashboard-emitter [{dashboard-id :emitter-parent-id} context] (is (= #{{:id card-id :type "card" :name (str "Card " action-id)} {:id dashboard-id :type "dashboard" :name (str "Dashboard " action-id)}} - (set (:emitter-usages (hydrate (Action action-id) :action/emitter-usages))))))))))) + (set (:emitter-usages (hydrate (db/select-one Action :id action-id) :action/emitter-usages))))))))))) (deftest card-emitter-usages-hydration-test (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom) diff --git a/test/metabase/test/redefs.clj b/test/metabase/test/redefs.clj index ab7cf78b30e..c6650c5c471 100644 --- a/test/metabase/test/redefs.clj +++ b/test/metabase/test/redefs.clj @@ -24,13 +24,20 @@ (let [attributes-with-defaults (merge (tt/with-temp-defaults model) attributes)] (try - (let [temp-object (let [object (db/insert! model attributes-with-defaults)] + (let [temp-object (let [object (or (db/insert! model attributes-with-defaults) + ;; if the object didn't come back from insert see if it's possible to + ;; manually fetch it (work around Toucan 1 bugs where things with + ;; non-integer values don't come back from H2, e.g. models like Session) + (let [pk-key (models/primary-key model) + pk-value (get attributes-with-defaults pk-key)] + (when pk-value + (db/select-one model pk-key pk-value))))] (assert (record? object) (str (format "db/insert! for %s did not return a valid row. Got: %s." (name model) (pr-str object)) \newline - "(Tip: db/insert! doesn't seem to work with H2 with tables with compound PKs.)")) + "(Tip: db/insert! doesn't seem to work with H2 with tables with compound PKs, or with non-integer IDs.)")) object) primary-key-column (models/primary-key model) primary-key-value (get temp-object primary-key-column)] diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj index 8c3da2a180c..8da203a7549 100644 --- a/test/metabase/test/util.clj +++ b/test/metabase/test/util.clj @@ -16,6 +16,7 @@ PersistedInfo Pulse PulseCard PulseChannel Revision Segment Setting Table TaskHistory Timeline TimelineEvent User]] [metabase.models.collection :as collection] + [metabase.models.interface :as mi] [metabase.models.permissions :as perms] [metabase.models.permissions-group :as perms-group] [metabase.models.setting :as setting] @@ -678,13 +679,13 @@ "Additional conditions that should be used to restrict which instances automatically get deleted by `with-model-cleanup`. Conditions should be a HoneySQL `:where` clause." {:arglists '([model])} - type) + mi/model) (defmethod with-model-cleanup-additional-conditions :default [_] nil) -(defmethod with-model-cleanup-additional-conditions (type Collection) +(defmethod with-model-cleanup-additional-conditions Collection [_] ;; NEVER delete personal collections for the test users. [:or -- GitLab