diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index c5d1026aa191511636315f5357f9649655bb0aa9..3fb9419217429d5df608da572e8b782f37e1de23 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -517,6 +517,7 @@ metabase.test/with-temp-file clojure.core/let metabase.test/with-user-in-groups clojure.core/let metabase.util.files/with-open-path-to-resource clojure.core/let + metabase.util.malli/defmethod schema.core/defmethod metabase.util.malli/defn schema.core/defn metabase.util.ssh/with-ssh-tunnel clojure.core/let monger.operators/defoperator clojure.core/def diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/interface.clj b/enterprise/backend/src/metabase_enterprise/audit_app/interface.clj index e4e6ae53d97e24a00a73c3df2fce377b9a00986c..8fc2deb5f38080c02f88ede0488c04e7614ee318 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/interface.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/interface.clj @@ -2,15 +2,17 @@ (:require [metabase.plugins.classloader :as classloader] [metabase.util.i18n :refer [tru]] - [metabase.util.schema :as su] - [schema.core :as s])) + [metabase.util.malli.schema :as ms])) (def ResultsMetadata "Schema for the expected format for `:metadata` returned by an internal query function." - (su/non-empty - [[(s/one su/KeywordOrString "field name") - (s/one {:base_type su/FieldType, :display_name su/NonBlankString, s/Keyword s/Any} - "field metadata")]])) + [:sequential + {:min 1} + [:tuple + ms/KeywordOrString + [:map + [:base_type ms/FieldType] + [:display_name ms/NonBlankString]]]]) (defmulti internal-query "Define a new internal query type. Conventionally `query-type` should be a namespaced keyword with the namespace in diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/common.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/common.clj index e00d4b3e567f3abf709b6b9d0b7cac9fc6a5e697..9eb5b4131b3a57eba76e68f39336a826ab417afc 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/common.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/common.clj @@ -20,11 +20,10 @@ [metabase.query-processor.timezone :as qp.timezone] [metabase.util :as u] [metabase.util.honey-sql-2 :as h2x] - #_{:clj-kondo/ignore [:discouraged-namespace]} + #_{:clj-kondo/ignore [:deprecated-namespace :discouraged-namespace]} [metabase.util.honeysql-extensions :as hx] [metabase.util.i18n :refer [tru]] - [metabase.util.urls :as urls] - [schema.core :as s])) + [metabase.util.urls :as urls])) (set! *warn-on-reflection* true) @@ -207,7 +206,7 @@ (def DateTimeUnitStr "Scheme for a valid QP DateTime unit as a string (the format they will come into the audit QP). E.g. something like `day` or `day-of-week`." - (apply s/enum (keys datetime-unit-str->base-type))) + (into [:enum] (keys datetime-unit-str->base-type))) (defn grouped-datetime "Group a datetime expression by `unit` using the appropriate SQL QP `date` implementation for our application 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 cc49a337baaab371f1f34f753a90de9f87e7d0d6..2247a84f96aa458e82b460d17422d4fac35a317c 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 @@ -6,11 +6,11 @@ [metabase.models.dashboard :refer [Dashboard]] [metabase.models.revision :as revision] [metabase.util.honey-sql-2 :as h2x] - [metabase.util.schema :as su] - [schema.core :as s])) + [metabase.util.malli :as mu] + [metabase.util.malli.schema :as ms])) (def ^:private ModelName - (s/enum "card" "dashboard")) + [:enum "card" "dashboard"]) ;; SELECT {{group-fn(timestamp}} AS "date", count(*) AS views ;; FROM view_log @@ -18,9 +18,11 @@ ;; AND model_id = {{model-id}} ;; GROUP BY {{group-fn(timestamp}} ;; ORDER BY {{group-fn(timestamp}} ASC -(s/defn views-by-time +(mu/defn views-by-time "Get views of a Card or Dashboard broken out by a time `unit`, e.g. `day` or `day-of-week`." - [model :- ModelName, model-id :- su/IntGreaterThanZero, unit :- common/DateTimeUnitStr] + [model :- ModelName + model-id :- ms/PositiveInt + unit :- common/DateTimeUnitStr] {:metadata [[:date {:display_name "Date", :base_type (common/datetime-unit-str->base-type unit)}] [:views {:display_name "Views", :base_type :type/Integer}]] :results (let [grouped-timestamp (common/grouped-datetime unit :timestamp)] @@ -35,10 +37,11 @@ :order-by [[grouped-timestamp :asc]]} (common/add-45-days-clause :timestamp))))}) -(s/defn cached-views-by-time +(mu/defn cached-views-by-time "Get number of views of a Card broken out by a time `unit`, e.g. `day` or `day-of-week` and by cache status. Still here instead of in cards because of similarity to views-by-time" - [card-id :- su/IntGreaterThanZero, unit :- common/DateTimeUnitStr] + [card-id :- ms/PositiveInt + unit :- common/DateTimeUnitStr] {:metadata [[:date {:display_name "Date", :base_type (common/datetime-unit-str->base-type unit)}] [:cached-views {:display_name "Cached Views", @@ -59,10 +62,11 @@ :order-by [[grouped-timestamp :asc]]} (common/add-45-days-clause :started_at))))}) -(s/defn avg-execution-time-by-time +(mu/defn avg-execution-time-by-time "Get average execution time of a Card broken out by a time `unit`, e.g. `day` or `day-of-week`. Still here instead of in cards because of similarity to views-by-time" - [card-id :- su/IntGreaterThanZero, unit :- common/DateTimeUnitStr] + [card-id :- ms/PositiveInt + unit :- common/DateTimeUnitStr] {:metadata [[:date {:display_name "Date", :base_type (common/datetime-unit-str->base-type unit)}] [:avg_runtime {:display_name "Average Runtime", :base_type :type/Number}]] :results (let [grouped-timestamp (common/grouped-datetime unit :started_at)] @@ -75,10 +79,10 @@ :order-by [[grouped-timestamp :asc]]} (common/add-45-days-clause :started_at))))}) -(s/defn revision-history +(mu/defn revision-history "Get a revision history table for a Card or Dashboard." - [model :- (s/enum Card Dashboard) - model-id :- su/IntGreaterThanZero] + [model :- [:enum Card Dashboard] + model-id :- ms/PositiveInt] {: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}] @@ -91,9 +95,10 @@ :change_made (-> revision :description) :revision_id (-> revision :id)})}) -(s/defn audit-log +(mu/defn audit-log "Get a view log for a Card or Dashboard." - [model :- ModelName, model-id :- su/IntGreaterThanZero] + [model :- ModelName + model-id :- ms/PositiveInt] {:metadata [[:when {:display_name "When", :base_type :type/DateTime}] [:user_id {:display_name "User ID", :base_type :type/Integer, :remapped_to :who}] [:who {:display_name "Who", :base_type :type/Name, :remapped_from :user_id}] diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/dashboard_detail.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/dashboard_detail.clj index bbd7afa7e796637ec583820ffc84d4c4917efc30..aa817c06963edea1d426f8b01f57b423b7d30266 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/dashboard_detail.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/dashboard_detail.clj @@ -3,30 +3,33 @@ (:require [metabase-enterprise.audit-app.interface :as audit.i] [metabase-enterprise.audit-app.pages.common :as common] - [metabase-enterprise.audit-app.pages.common.card-and-dashboard-detail :as card-and-dash-detail] + [metabase-enterprise.audit-app.pages.common.card-and-dashboard-detail + :as card-and-dash-detail] [metabase-enterprise.audit-app.pages.common.cards :as cards] [metabase.models.dashboard :refer [Dashboard]] - [metabase.util.schema :as su] - [schema.core :as s])) + [metabase.util.malli :as mu] + [metabase.util.malli.schema :as ms])) ;; Get views of a Dashboard broken out by a time `unit`, e.g. `day` or `day-of-week`. -(s/defmethod audit.i/internal-query ::views-by-time - [_ dashboard-id :- su/IntGreaterThanZero datetime-unit :- common/DateTimeUnitStr] +(mu/defmethod audit.i/internal-query ::views-by-time + [_query-type + dashboard-id :- ms/PositiveInt + datetime-unit :- common/DateTimeUnitStr] (card-and-dash-detail/views-by-time "dashboard" dashboard-id datetime-unit)) ;; Revision history for a specific Dashboard. -(s/defmethod audit.i/internal-query ::revision-history - [_ dashboard-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::revision-history + [_query-type dashboard-id :- ms/PositiveInt] (card-and-dash-detail/revision-history Dashboard dashboard-id)) ;; View log for a specific Dashboard. -(s/defmethod audit.i/internal-query ::audit-log - [_ dashboard-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::audit-log + [_query-type dashboard-id :- ms/PositiveInt] (card-and-dash-detail/audit-log "dashboard" dashboard-id)) ;; Information about the Saved Questions (Cards) in this instance. -(s/defmethod audit.i/internal-query ::cards - [_ dashboard-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::cards + [_query-type dashboard-id :- ms/PositiveInt] {:metadata [[:card_id {:display_name "Card ID", :base_type :type/Integer, :remapped_to :card_name}] [:card_name {:display_name "Title", :base_type :type/Name, :remapped_from :card_id}] [:collection_id {:display_name "Collection ID", :base_type :type/Integer, :remapped_to :collection_name}] diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/dashboards.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/dashboards.clj index 81aa59bc9604138ca762d3df9205a118c173651d..e9dfda022e6f7773d7ae8b996658791e5a26cc10 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/dashboards.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/dashboards.clj @@ -5,11 +5,11 @@ [metabase-enterprise.audit-app.pages.common :as common] [metabase-enterprise.audit-app.pages.common.dashboards :as dashboards] [metabase.util.honey-sql-2 :as h2x] - [schema.core :as s])) + [metabase.util.malli :as mu])) ;; Two-series timeseries that includes total number of Dashboard views and saves broken out by a `datetime-unit`. -(s/defmethod audit.i/internal-query ::views-and-saves-by-time - [_ datetime-unit :- common/DateTimeUnitStr] +(mu/defmethod audit.i/internal-query ::views-and-saves-by-time + [_query-type datetime-unit :- common/DateTimeUnitStr] {:metadata [[:date {:display_name "Date", :base_type (common/datetime-unit-str->base-type datetime-unit)}] [:views {:display_name "Views", :base_type :type/Integer}] [:saves {:display_name "Saves", :base_type :type/Integer}]] @@ -130,8 +130,8 @@ :limit 10})}) ;; Internal audit app query powering a table of different Dashboards with lots of extra info about them. -(s/defmethod audit.i/internal-query ::table +(mu/defmethod audit.i/internal-query ::table ([query-type] (audit.i/internal-query query-type nil)) - ([_ query-string :- (s/maybe s/Str)] + ([_query-type query-string :- [:maybe :string]] (dashboards/table query-string))) diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/database_detail.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/database_detail.clj index 3016e75fc8814f8c358fd1e6ebf8ae8a968f058b..413e88be3deaf420257ae53db31d3d8cb470c9c8 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/database_detail.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/database_detail.clj @@ -2,13 +2,13 @@ (:require [metabase-enterprise.audit-app.interface :as audit.i] [metabase-enterprise.audit-app.pages.common :as common] - [metabase.util.schema :as su] - [ring.util.codec :as codec] - [schema.core :as s])) + [metabase.util.malli :as mu] + [metabase.util.malli.schema :as ms] + [ring.util.codec :as codec])) ;; Query execution history for queries against this Database. -(s/defmethod audit.i/internal-query ::audit-log - [_ database-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::audit-log + [_query-type database-id :- ms/PositiveInt] {:metadata [[:started_at {:display_name "Viewed on", :base_type :type/DateTime}] [:card_id {:display_name "Card ID", :base_type :type/Integer, :remapped_to :query}] [:query_hash {:display_name "Query Hash", :base_type :type/Text}] diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/databases.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/databases.clj index 0622f37506663482ce29a40d7cacc0a2578dc435..b13576fd2341d51076c0273eb697464a6740fa0e 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/databases.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/databases.clj @@ -4,7 +4,7 @@ [metabase-enterprise.audit-app.pages.common :as common] [metabase.util.cron :as u.cron] [metabase.util.honey-sql-2 :as h2x] - [schema.core :as s])) + [metabase.util.malli :as mu])) ;; SELECT ;; db.id AS database_id, @@ -39,8 +39,8 @@ :order-by [[[:lower :db.name] :asc]]})}) ;; Query that returns count of query executions grouped by Database and a `datetime-unit`. -(s/defmethod audit.i/internal-query ::query-executions-by-time - [_ datetime-unit :- common/DateTimeUnitStr] +(mu/defmethod audit.i/internal-query ::query-executions-by-time + [_query-type datetime-unit :- common/DateTimeUnitStr] {:metadata [[:date {:display_name "Date", :base_type (common/datetime-unit-str->base-type datetime-unit)}] [:database_id {:display_name "Database ID", :base_type :type/Integer, :remapped_to :database_name}] [:database_name {:display_name "Database Name", :base_type :type/Name, :remapped_from :database_id}] @@ -74,10 +74,10 @@ (audit.i/internal-query ::query-executions-by-time "day")) ;; Table with information and statistics about all the data warehouse Databases in this Metabase instance. -(s/defmethod audit.i/internal-query ::table +(mu/defmethod audit.i/internal-query ::table ([query-type] (audit.i/internal-query query-type nil)) - ([_ query-string :- (s/maybe s/Str)] + ([_query-type query-string :- [:maybe :string]] ;; TODO - Should we convert sync_schedule from a cron string into English? Not sure that's going to be feasible for ;; really complicated schedules {:metadata [[:database_id {:display_name "Database ID", :base_type :type/Integer, :remapped_to :title}] diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/query_detail.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/query_detail.clj index 5933d71f867ca30a570578840c4423e7f1bb9cd1..96fe1b347e110b91deb8818ca3bfc44a326562a0 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/query_detail.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/query_detail.clj @@ -5,12 +5,12 @@ [metabase-enterprise.audit-app.interface :as audit.i] [metabase-enterprise.audit-app.pages.common :as common] [metabase-enterprise.audit-app.pages.common.cards :as cards] - [metabase.util.schema :as su] - [ring.util.codec :as codec] - [schema.core :as s])) + [metabase.util.malli :as mu] + [metabase.util.malli.schema :as ms] + [ring.util.codec :as codec])) -(defmethod audit.i/internal-query ::bad-card - [_ card-id] +(mu/defmethod audit.i/internal-query ::bad-card + [_query-type card-id :- ms/PositiveInt] {:metadata [[:card_id {:display_name "Question ID", :base_type :type/Integer :remapped_from :card_name}] [:card_name {:display_name "Question", :base_type :type/Text :remapped_from :card_id}] [:error_str {:display_name "Error", :base_type :type/Text :code true}] @@ -59,8 +59,8 @@ :where [:= :card.id card-id]})}) ;; Details about a specific query (currently just average execution time). -(s/defmethod audit.i/internal-query ::details - [_ query-hash :- su/NonBlankString] +(mu/defmethod audit.i/internal-query ::details + [_query-type query-hash :- ms/NonBlankString] {:metadata [[:query {:display_name "Query", :base_type :type/Dictionary}] [:average_execution_time {:display_name "Avg. Exec. Time (ms)", :base_type :type/Number}]] :results (common/reducible-query diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/question_detail.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/question_detail.clj index 713d17f9dc8f73b70a6dd72526046d63e9e804f1..9cba4e1d4cebe259a06252b6030b98f7880dea4f 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/question_detail.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/question_detail.clj @@ -3,32 +3,39 @@ (:require [metabase-enterprise.audit-app.interface :as audit.i] [metabase-enterprise.audit-app.pages.common :as common] - [metabase-enterprise.audit-app.pages.common.card-and-dashboard-detail :as card-and-dash-detail] + [metabase-enterprise.audit-app.pages.common.card-and-dashboard-detail + :as card-and-dash-detail] [metabase.models.card :refer [Card]] - [metabase.util.schema :as su] - [schema.core :as s])) + [metabase.util.malli :as mu] + [metabase.util.malli.schema :as ms])) ;; Get views of a Card broken out by a time `unit`, e.g. `day` or `day-of-week`. -(s/defmethod audit.i/internal-query ::views-by-time - [_ card-id :- su/IntGreaterThanZero datetime-unit :- common/DateTimeUnitStr] +(mu/defmethod audit.i/internal-query ::views-by-time + [_query-type + card-id :- ms/PositiveInt + datetime-unit :- common/DateTimeUnitStr] (card-and-dash-detail/views-by-time "card" card-id datetime-unit)) ;; Get cached views of a Card broken out by a time `unit`, e.g. `day` or `day-of-week`. -(s/defmethod audit.i/internal-query ::cached-views-by-time - [_ card-id :- su/IntGreaterThanZero, datetime-unit :- common/DateTimeUnitStr] +(mu/defmethod audit.i/internal-query ::cached-views-by-time + [_query-type + card-id :- ms/PositiveInt + datetime-unit :- common/DateTimeUnitStr] (card-and-dash-detail/cached-views-by-time card-id datetime-unit)) ;; Get the revision history for a Card. -(s/defmethod audit.i/internal-query ::revision-history - [_ card-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::revision-history + [_query-type card-id :- ms/PositiveInt] (card-and-dash-detail/revision-history Card card-id)) ;; Get a view log for a Card. -(s/defmethod audit.i/internal-query ::audit-log - [_ card-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::audit-log + [_query-type card-id :- ms/PositiveInt] (card-and-dash-detail/audit-log "card" card-id)) ;; Average execution time broken out by period -(s/defmethod audit.i/internal-query ::avg-execution-time-by-time - [_ card-id :- su/IntGreaterThanZero datetime-unit :- common/DateTimeUnitStr] +(mu/defmethod audit.i/internal-query ::avg-execution-time-by-time + [_query-type + card-id :- ms/PositiveInt + datetime-unit :- common/DateTimeUnitStr] (card-and-dash-detail/avg-execution-time-by-time card-id datetime-unit)) diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/schemas.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/schemas.clj index 0d784d91ac4fd19b1b825ccfca4ace9224c14d1f..d8e380a736b8898848895b4de35e587ee82a5c48 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/schemas.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/schemas.clj @@ -3,7 +3,7 @@ [metabase-enterprise.audit-app.interface :as audit.i] [metabase-enterprise.audit-app.pages.common :as common] [metabase.util.honey-sql-2 :as h2x] - [schema.core :as s])) + [metabase.util.malli :as mu])) ;; WITH counts AS ( ;; SELECT db."name" AS db_name, t."schema" AS db_schema @@ -118,10 +118,10 @@ ;; ;; DEPRECATED Query that returns a data for a table full of fascinating information about the different schemas in use ;; in our application. -(s/defmethod audit.i/internal-query ::table +(mu/defmethod audit.i/internal-query ::table ([query-type] (audit.i/internal-query query-type nil)) - ([_ query-string :- (s/maybe s/Str)] + ([_query-type query-string :- [:maybe :string]] {:metadata [[:database_id {:display_name "Database ID", :base_type :type/Integer, :remapped_to :database}] [:database {:display_name "Database", :base_type :type/Title, :remapped_from :database_id}] [:schema_id {:display_name "Schema ID", :base_type :type/Text, :remapped_to :schema}] diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/table_detail.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/table_detail.clj index 6139bb7210eee4025d3ab8c1a0d5c589c748dc68..69faade94fd460e831d2e3f149368f841f688bf9 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/table_detail.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/table_detail.clj @@ -2,13 +2,13 @@ (:require [metabase-enterprise.audit-app.interface :as audit.i] [metabase-enterprise.audit-app.pages.common :as common] - [metabase.util.schema :as su] - [ring.util.codec :as codec] - [schema.core :as s])) + [metabase.util.malli :as mu] + [metabase.util.malli.schema :as ms] + [ring.util.codec :as codec])) ;; View log for a specific Table. -(s/defmethod audit.i/internal-query ::audit-log - [_ table-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::audit-log + [_query-type table-id :- ms/PositiveInt] {:metadata [[:started_at {:display_name "Viewed on", :base_type :type/DateTime}] [:card_id {:display_name "Card ID", :base_type :type/Integer, :remapped_to :query}] [:query {:display_name "Query", :base_type :type/Text, :remapped_from :card_id}] diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/tables.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/tables.clj index 5e03a07632a61ce3b7da60cbadaff047095638f5..08fe15259efcd92200a215759090858ad5dff355 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/tables.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/tables.clj @@ -3,7 +3,7 @@ [metabase-enterprise.audit-app.interface :as audit.i] [metabase-enterprise.audit-app.pages.common :as common] [metabase.util.honey-sql-2 :as h2x] - [schema.core :as s])) + [metabase.util.malli :as mu])) ;; WITH table_executions AS ( ;; SELECT t.id AS table_id, count(*) AS executions @@ -52,10 +52,10 @@ (query-counts :asc)) ;; A table of Tables. -(s/defmethod audit.i/internal-query ::table +(mu/defmethod audit.i/internal-query ::table ([query-type] (audit.i/internal-query query-type nil)) - ([_ query-string :- (s/maybe s/Str)] + ([_query-type query-string :- [:maybe :string]] {:metadata [[:database_id {:display_name "Database ID", :base_type :type/Integer, :remapped_to :database_name}] [:database_name {:display_name "Database", :base_type :type/Text, :remapped_from :database_id}] [:schema_id {:display_name "Schema ID", :base_type :type/Text, :remapped_to :schema_name}] diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/user_detail.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/user_detail.clj index 783e0807fbc805c160d45962cfeef743c94558bb..3300b938793b48d1f044d76479cda6ed0d40ab6c 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/user_detail.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/user_detail.clj @@ -5,17 +5,17 @@ [metabase-enterprise.audit-app.pages.common.cards :as cards] [metabase-enterprise.audit-app.pages.common.dashboards :as dashboards] [metabase.util.honey-sql-2 :as h2x] - [metabase.util.schema :as su] + [metabase.util.malli :as mu] + [metabase.util.malli.schema :as ms] [metabase.util.urls :as urls] - [ring.util.codec :as codec] - [schema.core :as s])) + [ring.util.codec :as codec])) ;; Query that probides a single row of information about a given User, similar to the `users/table` query but ;; restricted to a single result. ;; ;; (TODO - in the designs, this is pivoted; should we do that here in Clojure-land?) -(s/defmethod audit.i/internal-query ::table - [_ user-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::table + [_query-type user-id :- ms/PositiveInt] {:metadata [[:name {:display_name "Name", :base_type :type/Name}] [:role {:display_name "Role", :base_type :type/Text}] [:groups {:display_name "Groups", :base_type :type/Text}] @@ -77,8 +77,8 @@ :pulses_saved]})}) ;; Return the 10 most-viewed Dashboards for a given User, in descending order. -(s/defmethod audit.i/internal-query ::most-viewed-dashboards - [_ user-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::most-viewed-dashboards + [_query-type user-id :- ms/PositiveInt] {:metadata [[:dashboard_id {:display_name "Dashboard ID", :base_type :type/Integer, :remapped_to :dashboard_name}] [:dashboard_name {:display_name "Dashboard", :base_type :type/Name, :remapped_from :dashboard_id}] [:count {:display_name "Views", :base_type :type/Integer}]] @@ -96,8 +96,8 @@ :limit 10})}) ;; Return the 10 most-viewed Questions for a given User, in descending order. -(s/defmethod audit.i/internal-query ::most-viewed-questions - [_ user-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::most-viewed-questions + [_query-type user-id :- ms/PositiveInt] {:metadata [[:card_id {:display_name "Card ID", :base_type :type/Integer, :remapped_to :card_name}] [:card_name {:display_name "Query", :base_type :type/Name, :remapped_from :card_id}] [:count {:display_name "Views", :base_type :type/Integer}]] @@ -115,8 +115,8 @@ :limit 10})}) ;; Query views by a specific User. -(s/defmethod audit.i/internal-query ::query-views - [_ user-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::query-views + [_query-type user-id :- ms/PositiveInt] {:metadata [[:viewed_on {:display_name "Viewed On", :base_type :type/DateTime}] [:card_id {:display_name "Card ID" :base_type :type/Integer, :remapped_to :card_name}] [:card_name {:display_name "Query", :base_type :type/Text, :remapped_from :card_id}] @@ -155,8 +155,8 @@ :xform (map #(update (vec %) 3 codec/base64-encode))}) ;; Dashboard views by a specific User. -(s/defmethod audit.i/internal-query ::dashboard-views - [_ user-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::dashboard-views + [_query-type user-id :- ms/PositiveInt] {:metadata [[:timestamp {:display_name "Viewed on", :base_type :type/DateTime}] [:dashboard_id {:display_name "Dashboard ID", :base_type :type/Integer, :remapped_to :dashboard_name}] [:dashboard_name {:display_name "Dashboard", :base_type :type/Text, :remapped_from :dashboard_id}] @@ -177,10 +177,10 @@ :order-by [[:vl.timestamp :desc]]})}) ;; Timeseries chart that shows the number of Question or Dashboard views for a User, broken out by `datetime-unit`. -(s/defmethod audit.i/internal-query ::object-views-by-time - [_ - user-id :- su/IntGreaterThanZero - model :- (s/enum "card" "dashboard") +(mu/defmethod audit.i/internal-query ::object-views-by-time + [_query-type + user-id :- ms/PositiveInt + model :- [:enum "card" "dashboard"] datetime-unit :- common/DateTimeUnitStr] {:metadata [[:date {:display_name "Date", :base_type (common/datetime-unit-str->base-type datetime-unit)}] [:views {:display_name "Views", :base_type :type/Integer}]] @@ -195,15 +195,15 @@ :order-by [[(common/grouped-datetime datetime-unit :timestamp) :asc]]})}) ;; Dashboards created by a specific User. -(s/defmethod audit.i/internal-query ::created-dashboards +(mu/defmethod audit.i/internal-query ::created-dashboards ([query-type user-id] (audit.i/internal-query query-type user-id nil)) - ([_ user-id :- su/IntGreaterThanZero query-string :- (s/maybe s/Str)] + ([_query-type user-id :- ms/PositiveInt query-string :- [:maybe :string]] (dashboards/table query-string [:= :u.id user-id]))) ;; Questions created by a specific User. -(s/defmethod audit.i/internal-query ::created-questions - [_ user-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::created-questions + [_query-type user-id :- ms/PositiveInt] {:metadata [[:card_id {:display_name "Card ID", :base_type :type/Integer, :remapped_to :card_name}] [:card_name {:display_name "Title", :base_type :type/Name, :remapped_from :card_id}] [:collection_id {:display_name "Collection ID", :base_type :type/Integer, :remapped_to :collection_name}] @@ -247,8 +247,8 @@ ;; Table of query downloads (i.e., queries whose results are returned as CSV/JSON/XLS) done by this user, ordered by ;; most recent. -(s/defmethod audit.i/internal-query ::downloads - [_ user-id :- su/IntGreaterThanZero] +(mu/defmethod audit.i/internal-query ::downloads + [_query-type user-id :- ms/PositiveInt] {:metadata [[:downloaded_at {:display_name "Downloaded At", :base_type :type/DateTime}] [:rows_downloaded {:display_name "Rows Downloaded", :base_type :type/Integer}] [:card_id {:display_name "Card ID", :base_type :type/Integer, :remapped_to :card_name}] diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/pages/users.clj b/enterprise/backend/src/metabase_enterprise/audit_app/pages/users.clj index e897c649968ba583ffb775bde664e16fc960a2f9..a7f8d4da6d0c7fed22c9f313724216f8943cfbfc 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/pages/users.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/pages/users.clj @@ -3,8 +3,8 @@ [metabase-enterprise.audit-app.interface :as audit.i] [metabase-enterprise.audit-app.pages.common :as common] [metabase.util.honey-sql-2 :as h2x] - [ring.util.codec :as codec] - [schema.core :as s])) + [metabase.util.malli :as mu] + [ring.util.codec :as codec])) ;; DEPRECATED Query that returns data for a two-series timeseries: the number of DAU (a User is considered active for ;; purposes of this query if they ran at least one query that day), and total number of queries ran. Broken out by @@ -29,8 +29,8 @@ ;; Two-series timeseries that returns number of active Users (Users who ran at least one query) and number of new ;; Users, broken out by `datetime-unit`. -(s/defmethod audit.i/internal-query ::active-and-new-by-time - [_ datetime-unit :- common/DateTimeUnitStr] +(mu/defmethod audit.i/internal-query ::active-and-new-by-time + [_query-type datetime-unit :- common/DateTimeUnitStr] {:metadata [[:date {:display_name "Date", :base_type (common/datetime-unit-str->base-type datetime-unit)}] [:active_users {:display_name "Active Users", :base_type :type/Integer}] [:new_users {:display_name "New Users", :base_type :type/Integer}]] @@ -143,11 +143,11 @@ :limit 10})}) ;; A table of all the Users for this instance, and various statistics about them (see metadata below). -(s/defmethod audit.i/internal-query ::table +(mu/defmethod audit.i/internal-query ::table ([query-type] (audit.i/internal-query query-type nil)) - ([_ query-string :- (s/maybe s/Str)] + ([_query-type query-string :- [:maybe :string]] {:metadata [[:user_id {:display_name "User ID", :base_type :type/Integer, :remapped_to :name}] [:name {:display_name "Member", :base_type :type/Name, :remapped_from :user_id}] [:role {:display_name "Role", :base_type :type/Text}] diff --git a/enterprise/backend/src/metabase_enterprise/audit_app/query_processor/middleware/handle_audit_queries.clj b/enterprise/backend/src/metabase_enterprise/audit_app/query_processor/middleware/handle_audit_queries.clj index 7bbe2181fff2911245b75b819e61e572f5eea0ef..152c239c0f5d8366db2a8ee30fd5ac45d212353c 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_app/query_processor/middleware/handle_audit_queries.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_app/query_processor/middleware/handle_audit_queries.clj @@ -45,8 +45,7 @@ [metabase.query-processor.context :as qp.context] [metabase.query-processor.error-type :as qp.error-type] [metabase.util.i18n :refer [tru]] - [metabase.util.schema :as su] - [schema.core :as s])) + [metabase.util.malli :as mu])) (defn- check-results-and-metadata-keys-match "Primarily for dev and debugging purposes. We can probably take this out when shipping the finished product." @@ -68,8 +67,9 @@ (for [[k v] metadata] (assoc v :name (name k)))) -(s/defn ^:private format-results [{:keys [results metadata]} :- {:results [su/Map] - :metadata audit.i/ResultsMetadata}] +(mu/defn ^:private format-results [{:keys [results metadata]} :- [:map + [:results [:sequential :map]] + [:metadata audit.i/ResultsMetadata]]] (check-results-and-metadata-keys-match results metadata) {:cols (metadata->cols metadata) :rows (for [row results] @@ -78,10 +78,18 @@ (def InternalQuery "Schema for a valid `internal` type query." - {:type (s/enum :internal "internal") - :fn #"^([\w\d-]+\.)*[\w\d-]+/[\w\d-]+$" ; namespace-qualified symbol - (s/optional-key :args) [s/Any] - s/Any s/Any}) + [:map + [:type [:enum :internal "internal"]] + [:fn [:and + :string + [:fn + {:error/message "namespace-qualified symbol serialized as a string"} + (fn [s] + (try + (when-let [symb (symbol s)] + (qualified-symbol? symb)) + (catch Throwable _)))]]] + [:args {:optional true} [:sequential :any]]]) (def ^:dynamic *additional-query-params* "Additional `internal` query params beyond `type`, `fn`, and `args`. These are bound to this dynamic var which is a @@ -109,7 +117,7 @@ reduce-reducible-results reduce-legacy-results) rff context results)) -(s/defn ^:private process-internal-query +(mu/defn ^:private process-internal-query [{qualified-fn-str :fn, args :args, :as query} :- InternalQuery rff context] ;; Make sure current user is a superuser or has monitoring permissions (validation/check-has-application-permission :monitoring) diff --git a/enterprise/backend/test/metabase_enterprise/audit_app/pages_test.clj b/enterprise/backend/test/metabase_enterprise/audit_app/pages_test.clj index bb96b1ba7a79d76779da7a98d3b9a53c01c38ed3..7cff8a4082dce7e334f77b8ac4b18c36dc3a4c9f 100644 --- a/enterprise/backend/test/metabase_enterprise/audit_app/pages_test.clj +++ b/enterprise/backend/test/metabase_enterprise/audit_app/pages_test.clj @@ -1,4 +1,4 @@ -(ns metabase-enterprise.audit-app.pages-test +(ns ^:mb/once metabase-enterprise.audit-app.pages-test (:require [clojure.java.classpath :as classpath] [clojure.java.io :as io] @@ -17,7 +17,8 @@ [metabase.test.fixtures :as fixtures] [metabase.util :as u] [ring.util.codec :as codec] - [schema.core :as s])) + [schema.core :as s] + [toucan2.tools.with-temp :as t2.with-temp])) (use-fixtures :once (fixtures/initialize :db :test-users)) @@ -96,7 +97,7 @@ {:namespace ns-symb, :file file})) (and (seq? form) - (#{'defmethod 's/defmethod} (first form)) + (#{'defmethod 'mu/defmethod} (first form)) (= (second form) 'audit.i/internal-query) (= (nth form 2) query-type)) form @@ -172,11 +173,11 @@ (qp/process-query query)))))) (defn- do-with-temp-objects [f] - (mt/with-temp* [Database [database] - Table [table {:db_id (u/the-id database)}] - Card [card {:table_id (u/the-id table), :database_id (u/the-id database)}] - Dashboard [dash] - DashboardCard [_ {:card_id (u/the-id card), :dashboard_id (u/the-id dash)}]] + (t2.with-temp/with-temp [Database database {} + Table table {:db_id (u/the-id database)} + Card card {:table_id (u/the-id table), :database_id (u/the-id database)} + Dashboard dash {} + DashboardCard _ {:card_id (u/the-id card), :dashboard_id (u/the-id dash)}] (f {:database database, :table table, :card card, :dash dash}))) (defmacro ^:private with-temp-objects [[objects-binding] & body] @@ -194,9 +195,9 @@ (testing "User name fallback to email, implemented in `metabase-enterprise.audit-app.pages.common/user-full-name` works in audit queries." (mt/with-test-user :crowberto (premium-features-test/with-premium-features #{:audit-app} - (mt/with-temp* [User [a {:first_name "a" :last_name nil :email "a@metabase.com"}] - User [b {:first_name nil :last_name "b" :email "b@metabase.com"}] - User [c {:first_name nil :last_name nil :email "c@metabase.com"}]] + (t2.with-temp/with-temp [User a {:first_name "a" :last_name nil :email "a@metabase.com"} + User b {:first_name nil :last_name "b" :email "b@metabase.com"} + User c {:first_name nil :last_name nil :email "c@metabase.com"}] (is (= #{"a" "b" "c@metabase.com"} (->> (get-in (mt/user-http-request :crowberto :post 202 "dataset" {:type :internal @@ -210,11 +211,11 @@ (testing "User login method takes into account both the google_auth and sso_source columns" (mt/with-test-user :crowberto (premium-features-test/with-premium-features #{:audit-app} - (mt/with-temp* [User [a {:email "a@metabase.com" :sso_source nil}] - User [b {:email "b@metabase.com" :sso_source :google}] - User [c {:email "c@metabase.com" :sso_source :saml}] - User [d {:email "d@metabase.com" :sso_source :jwt}] - User [e {:email "e@metabase.com" :sso_source :ldap}]] + (t2.with-temp/with-temp [User a {:email "a@metabase.com" :sso_source nil} + User b {:email "b@metabase.com" :sso_source :google} + User c {:email "c@metabase.com" :sso_source :saml} + User d {:email "d@metabase.com" :sso_source :jwt} + User e {:email "e@metabase.com" :sso_source :ldap}] (is (= ["Email" "Google Sign-In" "SAML" "JWT" "LDAP"] (->> (get-in (mt/user-http-request :crowberto :post 202 "dataset" {:type :internal diff --git a/src/metabase/util/malli.cljc b/src/metabase/util/malli.cljc index b20bb10928dccb25a5fc2a679d65d9cd68251d67..ada5cc501474018a5f2fe3e6268abe3dd25218ef 100644 --- a/src/metabase/util/malli.cljc +++ b/src/metabase/util/malli.cljc @@ -1,5 +1,5 @@ (ns metabase.util.malli - (:refer-clojure :exclude [defn]) + (:refer-clojure :exclude [defn defmethod]) (:require [clojure.core :as core] [malli.core :as mc] @@ -13,6 +13,7 @@ [malli.experimental :as mx] [malli.instrument :as minst] [metabase.util.i18n :as i18n] + [metabase.util.malli.defmethod :as mu.defmethod] [net.cgrand.macrovich :as macros] [ring.util.codec :as codec]))) #?(:cljs (:require-macros [metabase.util.malli]))) @@ -160,3 +161,11 @@ :description description-message ;; override generic description in :specific-errors key in API's response :error/fn (fn [_ _] specific-error-message)))) + +#?(:clj + (defmacro defmethod + "Like [[schema.core/defmethod]], but for Malli." + [multifn dispatch-value & fn-tail] + `(.addMethod ~(vary-meta multifn assoc :tag 'clojure.lang.MultiFn) + ~dispatch-value + ~(mu.defmethod/instrumented-fn-form fn-tail)))) diff --git a/src/metabase/util/malli/defmethod.clj b/src/metabase/util/malli/defmethod.clj new file mode 100644 index 0000000000000000000000000000000000000000..0e1de24d8cf33f49a6ae9142f1331b45dbe5add0 --- /dev/null +++ b/src/metabase/util/malli/defmethod.clj @@ -0,0 +1,55 @@ +(ns metabase.util.malli.defmethod + "Impl for [[metabase.util.malli/defmethod]]." + (:require + [clojure.core :as core] + [malli.core :as mc] + [malli.destructure] + [malli.experimental :as mx])) + +(defn- arity-schema [arity return-schema] + [:=> + (:schema (malli.destructure/parse (:args arity))) + return-schema]) + +(defn- parameterized-fn-tail->schema [fn-tail] + (let [{:keys [return arities]} (mc/parse mx/SchematizedParams (if (symbol? (first fn-tail)) + fn-tail + (cons 'f fn-tail))) + return-schema (:schema return :any) + [arities-type arities-value] arities] + (case arities-type + :single (arity-schema arities-value return-schema) + :multiple (into [:function] + (for [arity (:arities arities-value)] + (arity-schema arity return-schema)))))) + +(defn- deparameterized-arity [{:keys [body args prepost], :as _arity}] + (concat + [(:arglist (malli.destructure/parse args))] + (when prepost + [prepost]) + body)) + +(defn- deparameterized-fn-tail [fn-tail] + (let [{:keys [arities]} (mc/parse mx/SchematizedParams (if (symbol? (first fn-tail)) + fn-tail + (cons 'f fn-tail))) + [arities-type arities-value] arities] + (case arities-type + :single (deparameterized-arity arities-value) + :multiple (for [arity (:arities arities-value)] + (deparameterized-arity arity))))) + +;;; TODO -- this could also be used to power a Malli version of [[fn]] +(defn instrumented-fn-form + "Given a `fn-tail` like + + ([x :- :int y] (+ 1 2)) + + return an unevaluated instrumented [[fn]] form like + + (mc/-instrument {:schema [:=> [:cat :int :any] :any]} + (fn [x y] (+ 1 2)))" + [fn-tail] + `(mc/-instrument {:schema ~(parameterized-fn-tail->schema fn-tail)} + (fn ~@(deparameterized-fn-tail fn-tail)))) diff --git a/test/metabase/util/malli/defmethod_test.clj b/test/metabase/util/malli/defmethod_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..8767938ea5052d43338bcf79c86c3cfac1a24f9a --- /dev/null +++ b/test/metabase/util/malli/defmethod_test.clj @@ -0,0 +1,39 @@ +(ns metabase.util.malli.defmethod-test + (:require + [clojure.test :refer :all] + [metabase.util.malli.defmethod :as mu.defmethod])) + +(deftest ^:parallel instrumented-fn-form-test + (are [form expected] (= expected + (mu.defmethod/instrumented-fn-form form)) + '([x :- :int y]) + '(malli.core/-instrument + {:schema [:=> [:cat :int :any] :any]} + (clojure.core/fn [x y])) + + '(:- :int [x :- :int y]) + '(malli.core/-instrument + {:schema [:=> [:cat :int :any] :int]} + (clojure.core/fn [x y])) + + '(:- :int [x :- :int y] (+ x y)) + '(malli.core/-instrument + {:schema [:=> [:cat :int :any] :int]} + (clojure.core/fn [x y] + (+ x y))) + + '([x :- :int y] {:pre [(int? x)]}) + '(malli.core/-instrument + {:schema [:=> [:cat :int :any] :any]} + (clojure.core/fn [x y] {:pre [(int? x)]})) + + '(:- :int + ([x] (inc x)) + ([x :- :int y] (+ x y))) + '(malli.core/-instrument + {:schema [:function + [:=> [:cat :any] :int] + [:=> [:cat :int :any] :int]]} + (clojure.core/fn + ([x] (inc x)) + ([x y] (+ x y))))))