Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

metabase.util.schema to Malli #27471

Merged
merged 19 commits into from
Jan 5, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .clj-kondo/config.edn
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@
{:exclude
{metabase.cmd/dump {:namespaces ["metabase\\.cmd-test" "metabase-enterprise\\.serialization\\.cmd-test"]}
metabase.cmd/load {:namespaces ["metabase\\.cmd-test" "metabase-enterprise\\.serialization\\.cmd-test"]}
metabase.db.data-migrations/data-migrations {:namespaces ["metabase.db.data-migrations"]}
metabase.db.data-migrations/defmigration {:namespaces ["metabase.db.data-migrations"]}
metabase.db.data-migrations/run-migration-if-needed! {:namespaces ["metabase.db.data-migrations"]}
metabase.db.data-migrations/data-migrations {:namespaces ["metabase\\.db\\.data-migrations"]}
metabase.db.data-migrations/defmigration {:namespaces ["metabase\\.db\\.data-migrations"]}
metabase.db.data-migrations/run-migration-if-needed! {:namespaces ["metabase\\.db\\.data-migrations"]}
metabase.driver.common/ThreadSafeSimpleDateFormat {:namespaces ["metabase\\.driver.*"]}
metabase.driver.common/create-db-time-formatters {:namespaces ["metabase\\.driver.*"]}
metabase.driver.common/current-db-time {:namespaces ["metabase.*"]}
Expand Down Expand Up @@ -434,6 +434,7 @@
metabase.api.common/defendpoint-schema hooks.metabase.api.common/defendpoint
metabase.api.common/defendpoint hooks.metabase.api.common/defendpoint
metabase.api.common/defendpoint-async hooks.metabase.api.common/defendpoint
metabase.api.common/defendpoint-async-schema hooks.metabase.api.common/defendpoint
metabase.api.dashboard-test/with-chain-filter-fixtures hooks.common/let-one-with-optional-value
metabase.api.embed-test/do-response-formats hooks.common/with-two-bindings
metabase.api.embed-test/with-chain-filter-fixtures hooks.common/let-one-with-optional-value
Expand Down
4 changes: 2 additions & 2 deletions .lsp/config.edn
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
:clean {:ns-inner-blocks-indentation :keep}
:linters {:clojure-lsp/unused-public-var {:level :warning
:exclude-when-defined-by
#{metabase.api.common/defendpoint
metabase.api.common/malli-defendpoint}}}}
#{metabase.api.common/defendpoint-schema
metabase.api.common/defendpoint}}}}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
[metabase-enterprise.advanced-permissions.models.permissions.application-permissions :as a-perms]
[metabase.api.common :as api]))

#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema GET "/graph"
"Fetch a graph of Application Permissions."
[]
Expand All @@ -30,6 +31,7 @@
[graph]
(update graph :groups dejsonify-groups))

#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema PUT "/graph"
"Do a batch update of Application Permissions by passing a modified graph."
[:as {:keys [body]}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
- Permissions have three levels: full, limited, and none.
- Native query download permissions are fully inferred from the non-native download permissions. For more details,
see the docstring for [[metabase.models.permissions/update-native-download-permissions!]]."
[group-id :- su/IntGreaterThanZero db-id :- su/IntGreaterThanZero new-download-perms :- perms/DownloadPermissionsGraph]
[group-id :- su/IntGreaterThanZeroPlumatic db-id :- su/IntGreaterThanZeroPlumatic new-download-perms :- perms/DownloadPermissionsGraph]
(when-not (premium-features/enable-advanced-permissions?)
(throw (perms/ee-permissions-exception :download)))
(when-let [schemas (:schemas new-download-perms)]
Expand Down Expand Up @@ -157,7 +157,7 @@

(s/defn update-db-data-model-permissions!
"Update the data model permissions graph for a database."
[group-id :- su/IntGreaterThanZero db-id :- su/IntGreaterThanZero new-data-model-perms :- perms/DataModelPermissionsGraph]
[group-id :- su/IntGreaterThanZeroPlumatic db-id :- su/IntGreaterThanZeroPlumatic new-data-model-perms :- perms/DataModelPermissionsGraph]
(when-not (premium-features/enable-advanced-permissions?)
(throw (perms/ee-permissions-exception :data-model)))
(when-let [schemas (:schemas new-data-model-perms)]
Expand Down Expand Up @@ -188,7 +188,7 @@

(s/defn update-db-details-permissions!
"Update the DB details permissions for a database."
[group-id :- su/IntGreaterThanZero db-id :- su/IntGreaterThanZero new-perms :- perms/DetailsPermissions]
[group-id :- su/IntGreaterThanZeroPlumatic db-id :- su/IntGreaterThanZeroPlumatic new-perms :- perms/DetailsPermissions]
(when-not (premium-features/enable-advanced-permissions?)
(throw (perms/ee-permissions-exception :details)))
(case new-perms
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

(def ^:private ApplicationPermissionsGraph
{:revision s/Int
:groups {su/IntGreaterThanZero GroupPermissionsGraph}})
:groups {su/IntGreaterThanZeroPlumatic GroupPermissionsGraph}})

;; -------------------------------------------------- Fetch Graph ---------------------------------------------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
[metabase.models.pulse-channel-recipient :refer [PulseChannelRecipient]]
[toucan.db :as db]))

#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema DELETE "/:id/subscriptions"
"Delete all Alert and DashboardSubscription subscriptions for a User (i.e., so they will no longer receive them).
Archive all Alerts and DashboardSubscriptions created by the User. Only allowed for admins or for the current user."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
(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}
[[(s/one su/KeywordOrStringPlumatic "field name")
(s/one {:base_type su/FieldTypePlumatic, :display_name su/NonBlankStringPlumatic, s/Keyword s/Any}
"field metadata")]]))

(defmulti internal-query
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
;; ORDER BY {{group-fn(timestamp}} ASC
(s/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 :- su/IntGreaterThanZeroPlumatic, 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)]
Expand All @@ -40,7 +40,7 @@
(s/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 :- su/IntGreaterThanZeroPlumatic, unit :- common/DateTimeUnitStr]
{:metadata [[:date {:display_name "Date",
:base_type (common/datetime-unit-str->base-type unit)}]
[:cached-views {:display_name "Cached Views",
Expand All @@ -64,7 +64,7 @@
(s/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 :- su/IntGreaterThanZeroPlumatic, 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)]
Expand All @@ -80,7 +80,7 @@
(s/defn revision-history
"Get a revision history table for a Card or Dashboard."
[model-entity :- (s/cond-pre (mi/InstanceOf Card) (mi/InstanceOf Dashboard))
model-id :- su/IntGreaterThanZero]
model-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand All @@ -95,7 +95,7 @@

(s/defn audit-log
"Get a view log for a Card or Dashboard."
[model :- ModelName, model-id :- su/IntGreaterThanZero]
[model :- ModelName, model-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,22 @@

;; 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]
[_ dashboard-id :- su/IntGreaterThanZeroPlumatic 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]
[_ dashboard-id :- su/IntGreaterThanZeroPlumatic]
(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]
[_ dashboard-id :- su/IntGreaterThanZeroPlumatic]
(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]
[_ dashboard-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

;; Query execution history for queries against this Database.
(s/defmethod audit.i/internal-query ::audit-log
[_ database-id :- su/IntGreaterThanZero]
[_ database-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@

;; Details about a specific query (currently just average execution time).
(s/defmethod audit.i/internal-query ::details
[_ query-hash :- su/NonBlankString]
[_ query-hash :- su/NonBlankStringPlumatic]
{: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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,25 @@

;; 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]
[_ card-id :- su/IntGreaterThanZeroPlumatic 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]
[_ card-id :- su/IntGreaterThanZeroPlumatic, 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]
[_ card-id :- su/IntGreaterThanZeroPlumatic]
(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]
[_ card-id :- su/IntGreaterThanZeroPlumatic]
(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]
[_ card-id :- su/IntGreaterThanZeroPlumatic datetime-unit :- common/DateTimeUnitStr]
(card-and-dash-detail/avg-execution-time-by-time card-id datetime-unit))
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

;; View log for a specific Table.
(s/defmethod audit.i/internal-query ::audit-log
[_ table-id :- su/IntGreaterThanZero]
[_ table-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
;;
;; (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]
[_ user-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand Down Expand Up @@ -79,7 +79,7 @@

;; 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]
[_ user-id :- su/IntGreaterThanZeroPlumatic]
{: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}]]
Expand All @@ -98,7 +98,7 @@

;; 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]
[_ user-id :- su/IntGreaterThanZeroPlumatic]
{: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}]]
Expand All @@ -117,7 +117,7 @@

;; Query views by a specific User.
(s/defmethod audit.i/internal-query ::query-views
[_ user-id :- su/IntGreaterThanZero]
[_ user-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand Down Expand Up @@ -157,7 +157,7 @@

;; Dashboard views by a specific User.
(s/defmethod audit.i/internal-query ::dashboard-views
[_ user-id :- su/IntGreaterThanZero]
[_ user-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand All @@ -180,7 +180,7 @@
;; 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
user-id :- su/IntGreaterThanZeroPlumatic
model :- (s/enum "card" "dashboard")
datetime-unit :- common/DateTimeUnitStr]
{:metadata [[:date {:display_name "Date", :base_type (common/datetime-unit-str->base-type datetime-unit)}]
Expand All @@ -199,12 +199,12 @@
(s/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)]
([_ user-id :- su/IntGreaterThanZeroPlumatic query-string :- (s/maybe s/Str)]
(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]
[_ user-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand Down Expand Up @@ -249,7 +249,7 @@
;; 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]
[_ user-id :- su/IntGreaterThanZeroPlumatic]
{: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}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
(for [[k v] metadata]
(assoc v :name (name k))))

(s/defn ^:private format-results [{:keys [results metadata]} :- {:results [su/Map]
(s/defn ^:private format-results [{:keys [results metadata]} :- {:results [su/MapPlumatic]
:metadata audit.i/ResultsMetadata}]
(check-results-and-metadata-keys-match results metadata)
{:cols (metadata->cols metadata)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
[metabase.util.schema :as su]
[schema.core :as s]))

#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema POST "/"
"Create a new `ModerationReview`."
[:as {{:keys [text moderated_item_id moderated_item_type status]} :body}]
{text (s/maybe s/Str)
moderated_item_id su/IntGreaterThanZero
moderated_item_id su/IntGreaterThanZeroPlumatic
moderated_item_type moderation/moderated-item-types
status moderation-review/Statuses}
(api/check-superuser)
Expand Down
Loading