Skip to content
Snippets Groups Projects
Unverified Commit 2d22d3af authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

[Toucan 2 Prep] Add helper for extending `IModel`; namespace Toucan properties (#27474)

* [Toucan 2 Prep] Add helper for extending `IModel`; namespace Toucan properties

* Clean namespaces

* Test fixes :wrench:
parent ae59ec81
No related branches found
No related tags found
No related merge requests found
Showing
with 192 additions and 186 deletions
......@@ -23,6 +23,7 @@
Emitter
Field
FieldValues
GroupTableAccessPolicy
HTTPAction
LoginHistory
Metric
......
......@@ -48,6 +48,7 @@
(eval . (put-clojure-indent 'mbql.match/match-one 1))
(eval . (put-clojure-indent 'mbql.match/replace 1))
(eval . (put-clojure-indent 'mbql.match/replace-in 2))
(eval . (put-clojure-indent 'mi/define-methods '(:form)))
(eval . (put-clojure-indent 'mt/dataset 1))
(eval . (put-clojure-indent 'mt/format-rows-by 1))
(eval . (put-clojure-indent 'mt/query 1))
......@@ -57,7 +58,6 @@
(eval . (put-clojure-indent 'qp.streaming/streaming-response 1))
(eval . (put-clojure-indent 'u/prog1 1))
(eval . (put-clojure-indent 'u/select-keys-when 1))
(eval . (put-clojure-indent 'u/strict-extend 1))
(eval . (put-clojure-indent 'with-meta '(:form)))
;; these ones have to be done with `define-clojure-indent' for now because of upstream bug
;; https://github.com/clojure-emacs/clojure-mode/issues/600 once that's resolved we should use `put-clojure-indent'
......
......@@ -137,10 +137,8 @@
(when (:card_id updates)
(check-columns-match-table updated)))))
(u/strict-extend (class GroupTableAccessPolicy)
models/IModel
(merge
models/IModelDefaults
{:types (constantly {:attribute_remappings ::attribute-remappings})
:pre-insert pre-insert
:pre-update pre-update}))
(mi/define-methods
GroupTableAccessPolicy
{:types (constantly {:attribute_remappings ::attribute-remappings})
:pre-insert pre-insert
:pre-update pre-update})
......@@ -8,9 +8,10 @@
[clojure.test :refer :all]
[metabase.db.data-migrations]
[metabase.models]
[metabase.models.interface :as mi]
[metabase.models.revision-test]
[metabase.models.serialization.hash :as serdes.hash]
[toucan.models :refer [IModel]]))
[toucan.models :as models :refer [IModel]]))
(comment metabase.models/keep-me
metabase.db.data-migrations/keep-me
......@@ -73,11 +74,11 @@
(doseq [^Class model (->> (extenders IModel)
(remove entities-not-exported)
(remove entities-external-name))]
(testing (format "Model %s should either: have the :entity_id property, or be explicitly listed as having an external name, or explicitly listed as excluded from serialization"
(testing (format (str "Model %s should either: have the ::mi/entity-id property, or be explicitly listed as having "
"an external name, or explicitly listed as excluded from serialization")
(.getSimpleName model))
(is (= true (-> (.newInstance model)
toucan.models/properties
:entity_id))))))
(is (contains? (set (keys (models/properties (.newInstance model))))
::mi/entity-id)))))
(deftest comprehensive-identity-hash-test
(doseq [^Class model (->> (extenders IModel)
......
......@@ -37,12 +37,11 @@
(u/update-if-exists template :parameters (mi/catch-normalization-exceptions mi/normalize-parameters-list)))
mi/json-out-with-keywordization))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Action)
models/IModel
(merge models/IModelDefaults
{:pre-insert (fn [action] (check-data-apps-enabled) action)
:types (constantly {:type :keyword})
:properties (constantly {:timestamped? true})}))
(mi/define-methods
Action
{:pre-insert (fn [action] (check-data-apps-enabled) action)
:types (constantly {:type :keyword})
:properties (constantly {::mi/timestamped? true})})
(defn- pre-update
[action]
......@@ -57,27 +56,25 @@
(def ^:private Action-subtype-IModel-impl
"[[models/IModel]] impl for `HTTPAction` and `QueryAction`"
(merge models/IModelDefaults
{:primary-key (constantly :action_id) ; This is ok as long as we're 1:1
:pre-delete pre-delete
:pre-update pre-update}))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class QueryAction)
models/IModel
Action-subtype-IModel-impl)
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class HTTPAction)
models/IModel
(merge Action-subtype-IModel-impl
{:types (constantly {:template ::json-with-nested-parameters})}))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ModelAction)
models/IModel
(merge models/IModelDefaults
{:pre-insert (fn [model-action] (check-data-apps-enabled) model-action)
:properties (constantly {:entity_id true})
:types (constantly {:parameter_mappings :parameters-list
:visualization_settings :visualization-settings})}))
{:primary-key (constantly :action_id) ; This is ok as long as we're 1:1
:pre-delete pre-delete
:pre-update pre-update})
(mi/define-methods
QueryAction
Action-subtype-IModel-impl)
(mi/define-methods
HTTPAction
(merge Action-subtype-IModel-impl
{:types (constantly {:template ::json-with-nested-parameters})}))
(mi/define-methods
ModelAction
{:pre-insert (fn [model-action] (check-data-apps-enabled) model-action)
:properties (constantly {::mi/entity-id true})
:types (constantly {:parameter_mappings :parameters-list
:visualization_settings :visualization-settings})})
;;; TODO -- this doesn't seem right. [[serdes.hash/identity-hash-fields]] is used to calculate `entity_id`, so we
;;; shouldn't use it in the calculation. We can fix this later
......
......@@ -8,7 +8,6 @@
[metabase.models.metric :refer [Metric]]
[metabase.models.pulse :refer [Pulse]]
[metabase.models.segment :refer [Segment]]
[metabase.util :as u]
[toucan.db :as db]
[toucan.models :as models]))
......@@ -53,11 +52,10 @@
:details {}}]
(merge defaults activity)))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Activity)
models/IModel
(merge models/IModelDefaults
{:types (constantly {:details :json, :topic :keyword})
:pre-insert pre-insert}))
(mi/define-methods
Activity
{:types (constantly {:details :json, :topic :keyword})
:pre-insert pre-insert})
(defmethod mi/can-read? Activity
[& args]
......
(ns metabase.models.app
(:require
[metabase.models.action :as action]
[metabase.models.interface :as mi]
[metabase.models.permissions :as perms]
[metabase.models.query :as query]
[metabase.models.serialization.hash :as serdes.hash]
[metabase.util :as u]
[toucan.db :as db]
[toucan.models :as models]))
......@@ -13,14 +13,13 @@
;;; You can read/write an App if you can read/write its Collection
(derive App ::perms/use-parent-collection-perms)
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class App)
models/IModel
(merge models/IModelDefaults
{:pre-insert (fn [app] (action/check-data-apps-enabled) app)
:types (constantly {:options :json
:nav_items :json})
:properties (constantly {:timestamped? true
:entity_id true})}))
(mi/define-methods
App
{:pre-insert (fn [app] (action/check-data-apps-enabled) app)
:types (constantly {:options :json
:nav_items :json})
:properties (constantly {::mi/timestamped? true
::mi/entity-id true})})
;;; Should not be needed as every app should have an entity_id, but currently it's necessary to satisfy
;;; metabase-enterprise.models.entity-id-test/comprehensive-identity-hash-test.
......
(ns metabase.models.application-permissions-revision
(:require
[metabase.util :as u]
[metabase.models.interface :as mi]
[metabase.util.i18n :refer [tru]]
[toucan.db :as db]
[toucan.models :as models]))
(models/defmodel ApplicationPermissionsRevision :application_permissions_revision)
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ApplicationPermissionsRevision)
models/IModel
(merge models/IModelDefaults
{:types (constantly {:before :json
:after :json})
:properties (constantly {:created-at-timestamped? true})
:pre-update (fn [& _] (throw (Exception. (tru "You cannot update a ApplicationPermissionsRevision!"))))}))
(mi/define-methods
ApplicationPermissionsRevision
{:types (constantly {:before :json
:after :json})
:properties (constantly {::mi/created-at-timestamped? true})
:pre-update (fn [& _] (throw (Exception. (tru "You cannot update a ApplicationPermissionsRevision!"))))})
(defn latest-id
"Return the ID of the newest `ApplicationPermissionsRevision`, or zero if none have been made yet.
......
......@@ -305,27 +305,26 @@
:in mi/json-in
:out result-metadata-out)
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Card)
models/IModel
(merge models/IModelDefaults
{:hydration-keys (constantly [:card])
:types (constantly {:dataset_query :metabase-query
:display :keyword
:embedding_params :json
:query_type :keyword
:result_metadata ::result-metadata
:visualization_settings :visualization-settings
:parameters :parameters-list
:parameter_mappings :parameters-list})
:properties (constantly {:timestamped? true
:entity_id true})
;; Make sure we normalize the query before calling `pre-update` or `pre-insert` because some of the
;; functions those fns call assume normalized queries
:pre-update (comp populate-query-fields pre-update populate-result-metadata maybe-normalize-query)
:pre-insert (comp populate-query-fields pre-insert populate-result-metadata maybe-normalize-query)
:post-insert post-insert
:pre-delete pre-delete
:post-select public-settings/remove-public-uuid-if-public-sharing-is-disabled}))
(mi/define-methods
Card
{:hydration-keys (constantly [:card])
:types (constantly {:dataset_query :metabase-query
:display :keyword
:embedding_params :json
:query_type :keyword
:result_metadata ::result-metadata
:visualization_settings :visualization-settings
:parameters :parameters-list
:parameter_mappings :parameters-list})
:properties (constantly {::mi/timestamped? true
::mi/entity-id true})
;; Make sure we normalize the query before calling `pre-update` or `pre-insert` because some of the
;; functions those fns call assume normalized queries
:pre-update (comp populate-query-fields pre-update populate-result-metadata maybe-normalize-query)
:pre-insert (comp populate-query-fields pre-insert populate-result-metadata maybe-normalize-query)
:post-insert post-insert
:pre-delete pre-delete
:post-select public-settings/remove-public-uuid-if-public-sharing-is-disabled})
(defmethod serdes.hash/identity-hash-fields Card
[_card]
......
......@@ -899,17 +899,16 @@
[_collection]
[:name :namespace parent-identity-hash :created_at])
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Collection)
models/IModel
(merge models/IModelDefaults
{:hydration-keys (constantly [:collection])
:types (constantly {:namespace :keyword
:authority_level :keyword})
:properties (constantly {:entity_id true})
:pre-insert pre-insert
:post-insert post-insert
:pre-update pre-update
:pre-delete pre-delete}))
(mi/define-methods
Collection
{:hydration-keys (constantly [:collection])
:types (constantly {:namespace :keyword
:authority_level :keyword})
:properties (constantly {::mi/entity-id true})
:pre-insert pre-insert
:post-insert post-insert
:pre-update pre-update
:pre-delete pre-delete})
(defmethod serdes.base/extract-query "Collection" [_ {:keys [collection-set]}]
(if (seq collection-set)
......
......@@ -38,11 +38,11 @@
:read perms/collection-read-path
:write perms/collection-readwrite-path) collection)}))
(u/strict-extend RootCollection
(extend RootCollection
models/IModel
(merge
models/IModelDefaults
{:types {:type :keyword}}))
{:types {:type :keyword}}))
(def ^RootCollection root-collection
"Special placeholder object representing the Root Collection, which isn't really a real Collection."
......
(ns metabase.models.collection-permission-graph-revision
(:require
[metabase.util :as u]
[metabase.models.interface :as mi]
[metabase.util.i18n :refer [tru]]
[toucan.db :as db]
[toucan.models :as models]))
(models/defmodel CollectionPermissionGraphRevision :collection_permission_graph_revision)
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class CollectionPermissionGraphRevision)
models/IModel
(merge models/IModelDefaults
{:types (constantly {:before :json
:after :json})
:properties (constantly {:created-at-timestamped? true})
:pre-update (fn [& _] (throw (Exception. (tru "You cannot update a CollectionPermissionGraphRevision!"))))}))
(mi/define-methods
CollectionPermissionGraphRevision
{:types (constantly {:before :json
:after :json})
:properties (constantly {::mi/created-at-timestamped? true})
:pre-update (fn [& _] (throw (Exception. (tru "You cannot update a CollectionPermissionGraphRevision!"))))})
(defn latest-id
"Return the ID of the newest `CollectionPermissionGraphRevision`, or zero if none have been made yet.
......
......@@ -14,6 +14,7 @@
:as dashboard-card
:refer [DashboardCard]]
[metabase.models.field-values :as field-values]
[metabase.models.interface :as mi]
[metabase.models.parameter-card
:as parameter-card
:refer [ParameterCard]]
......@@ -163,18 +164,17 @@
(assoc-in param [:values_source_config :card_id] (get param-id->card-id id (:card_id values_source_config)))
param)))))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Dashboard)
models/IModel
(merge models/IModelDefaults
{:properties (constantly {:timestamped? true
:entity_id true})
:types (constantly {:parameters :parameters-list, :embedding_params :json})
:pre-delete pre-delete
:pre-insert pre-insert
:post-insert post-insert
:pre-update pre-update
:post-update post-update
:post-select (comp populate-card-id-for-parameters public-settings/remove-public-uuid-if-public-sharing-is-disabled)}))
(mi/define-methods
Dashboard
{:properties (constantly {::mi/timestamped? true
::mi/entity-id true})
:types (constantly {:parameters :parameters-list, :embedding_params :json})
:pre-delete pre-delete
:pre-insert pre-insert
:post-insert post-insert
:pre-update pre-update
:post-update post-update
:post-select (comp populate-card-id-for-parameters public-settings/remove-public-uuid-if-public-sharing-is-disabled)})
(defmethod serdes.hash/identity-hash-fields Dashboard
[_dashboard]
......
......@@ -43,14 +43,13 @@
:visualization_settings {}}]
(merge defaults dashcard)))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class DashboardCard)
models/IModel
(merge models/IModelDefaults
{:properties (constantly {:timestamped? true
:entity_id true})
:types (constantly {:parameter_mappings :parameters-list
:visualization_settings :visualization-settings})
:pre-insert pre-insert}))
(mi/define-methods
DashboardCard
{:properties (constantly {::mi/timestamped? true
::mi/entity-id true})
:types (constantly {:parameter_mappings :parameters-list
:visualization_settings :visualization-settings})
:pre-insert pre-insert})
(defmethod serdes.hash/identity-hash-fields DashboardCard
[_dashboard-card]
......
......@@ -197,23 +197,22 @@
:read (perms/data-perms-path db-id)
:write (perms/db-details-write-perms-path db-id))})
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Database)
models/IModel
(merge models/IModelDefaults
{:hydration-keys (constantly [:database :db])
:types (constantly {:details :encrypted-json
:options :json
:engine :keyword
:metadata_sync_schedule :cron-string
:cache_field_values_schedule :cron-string
:start_of_week :keyword
:settings :encrypted-json
:dbms_version :json})
:post-insert post-insert
:post-select post-select
:pre-insert pre-insert
:pre-update pre-update
:pre-delete pre-delete}))
(mi/define-methods
Database
{:hydration-keys (constantly [:database :db])
:types (constantly {:details :encrypted-json
:options :json
:engine :keyword
:metadata_sync_schedule :cron-string
:cache_field_values_schedule :cron-string
:start_of_week :keyword
:settings :encrypted-json
:dbms_version :json})
:post-insert post-insert
:post-select post-select
:pre-insert pre-insert
:pre-update pre-update
:pre-delete pre-delete})
(defmethod serdes.hash/identity-hash-fields Database
[_database]
......
......@@ -3,10 +3,10 @@
Query Processor. For a more detailed explanation, refer to the documentation in
`metabase.query-processor.middleware.add-dimension-projections`."
(:require
[metabase.models.interface :as mi]
[metabase.models.serialization.base :as serdes.base]
[metabase.models.serialization.hash :as serdes.hash]
[metabase.models.serialization.util :as serdes.util]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
[toucan.models :as models]))
......@@ -17,12 +17,11 @@
(models/defmodel Dimension :dimension)
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Dimension)
models/IModel
(merge models/IModelDefaults
{:types (constantly {:type :keyword})
:properties (constantly {:timestamped? true
:entity_id true})}))
(mi/define-methods
Dimension
{:types (constantly {:type :keyword})
:properties (constantly {::mi/timestamped? true
::mi/entity-id true})})
(defmethod serdes.hash/identity-hash-fields Dimension
[_dimension]
......
......@@ -181,21 +181,20 @@
:out (comp update-semantic-numeric-values mi/json-out-with-keywordization))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Field)
models/IModel
(merge models/IModelDefaults
{:hydration-keys (constantly [:destination :field :origin :human_readable_field])
:types (constantly {:base_type ::base-type
:effective_type ::effective-type
:coercion_strategy ::coercion-strategy
:semantic_type ::semantic-type
:visibility_type :keyword
:has_field_values :keyword
:fingerprint :json-for-fingerprints
:settings :json
:nfc_path :json})
:properties (constantly {:timestamped? true})
:pre-insert pre-insert}))
(mi/define-methods
Field
{:hydration-keys (constantly [:destination :field :origin :human_readable_field])
:types (constantly {:base_type ::base-type
:effective_type ::effective-type
:coercion_strategy ::coercion-strategy
:semantic_type ::semantic-type
:visibility_type :keyword
:has_field_values :keyword
:fingerprint :json-for-fingerprints
:settings :json
:nfc_path :json})
:properties (constantly {::mi/timestamped? true})
:pre-insert pre-insert})
(defmethod serdes.hash/identity-hash-fields Field
[_field]
......
......@@ -22,6 +22,7 @@
[clojure.string :as str]
[clojure.tools.logging :as log]
[java-time :as t]
[metabase.models.interface :as mi]
[metabase.models.serialization.base :as serdes.base]
[metabase.models.serialization.hash :as serdes.hash]
[metabase.models.serialization.util :as serdes.util]
......@@ -164,16 +165,15 @@
:else
[])))))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class FieldValues)
models/IModel
(merge models/IModelDefaults
{:properties (constantly {:timestamped? true})
:types (constantly {:human_readable_values :json-no-keywordization
:values :json
:type :keyword})
:pre-insert pre-insert
:pre-update pre-update
:post-select post-select}))
(mi/define-methods
FieldValues
{:properties (constantly {::mi/timestamped? true})
:types (constantly {:human_readable_values :json-no-keywordization
:values :json
:type :keyword})
:pre-insert pre-insert
:pre-update pre-update
:post-select post-select})
(defmethod serdes.hash/identity-hash-fields FieldValues
[_field-values]
......
......@@ -266,16 +266,16 @@
(cond-> obj
(not (:updated_at obj)) (assoc :updated_at (now))))
(models/add-property! :timestamped?
(models/add-property! ::timestamped?
:insert (comp add-created-at-timestamp add-updated-at-timestamp)
:update add-updated-at-timestamp)
;; like `timestamped?`, but for models that only have an `:created_at` column
(models/add-property! :created-at-timestamped?
(models/add-property! ::created-at-timestamped?
:insert add-created-at-timestamp)
;; like `timestamped?`, but for models that only have an `:updated_at` column
(models/add-property! :updated-at-timestamped?
(models/add-property! ::updated-at-timestamped?
:insert add-updated-at-timestamp
:update add-updated-at-timestamp)
......@@ -287,7 +287,7 @@
obj
(assoc obj :entity_id (u/generate-nano-id))))
(models/add-property! :entity_id
(models/add-property! ::entity-id
:insert add-entity-id)
;;; +----------------------------------------------------------------------------------------------------------------+
......@@ -467,6 +467,28 @@
[_model _m]
(superuser?))
;;;; [[define-methods]]
(defn- validate-properties [properties]
(doseq [k (keys properties)]
(assert (namespace k) "All :properties keys should be namespaced!")
(assert (contains? (set (keys @@#'models/property-fns)) k)
(str "Invalid property: " k))))
(defn define-methods
"Helper for defining [[toucan.models/IModel]] methods for a `model`. Prefer this over using `extend` directly, because
it's easier to swap a single function when we make the switch to Toucan 2 in the future than to update all the
various model namespaces."
{:style/indent [:form]}
[model method-map]
(when-let [properties-method (:properties method-map)]
(validate-properties (properties-method model)))
(extend (class model)
models/IModel
(merge
models/IModelDefaults
method-map)))
;;;; redefs
......
......@@ -35,14 +35,12 @@
(db/select-one ['Table :db_id :schema :id] :id (u/the-id (:table_id metric))))]
(mi/perms-objects-set table read-or-write)))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Metric)
models/IModel
(merge
models/IModelDefaults
{:types (constantly {:definition :metric-segment-definition})
:properties (constantly {:timestamped? true
:entity_id true})
:pre-update pre-update}))
(mi/define-methods
Metric
{:types (constantly {:definition :metric-segment-definition})
:properties (constantly {::mi/timestamped? true
::mi/entity-id true})
:pre-update pre-update})
(defmethod serdes.hash/identity-hash-fields Metric
[_metric]
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment