diff --git a/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/permissions/application_permissions.clj b/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/permissions/application_permissions.clj
index a160d129425d38d0962da5b7c239ca04bbf94131..e4149eaf3f8cd0a57fb7660cf7897bbb268fff3a 100644
--- a/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/permissions/application_permissions.clj
+++ b/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/permissions/application_permissions.clj
@@ -7,19 +7,21 @@
[metabase.models.application-permissions-revision :as a-perm-revision]
[metabase.models.permissions :as perms]
[metabase.util.honey-sql-2 :as h2x]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2]))
;;; ---------------------------------------------------- Schemas -----------------------------------------------------
(def ^:private GroupPermissionsGraph
- {(s/enum :setting :monitoring :subscription) (s/enum :yes :no)})
+ [:map-of
+ [:enum :setting :monitoring :subscription]
+ [:enum :yes :no]])
(def ^:private ApplicationPermissionsGraph
- {:revision s/Int
- :groups {su/IntGreaterThanZero GroupPermissionsGraph}})
+ [:map {:closed true}
+ [:revision :int]
+ [:groups [:map-of ms/PositiveInt GroupPermissionsGraph]]])
;; -------------------------------------------------- Fetch Graph ---------------------------------------------------
@@ -40,14 +42,14 @@
:yes
:no))
-(s/defn permissions-set->application-perms :- GroupPermissionsGraph
+(mu/defn permissions-set->application-perms :- GroupPermissionsGraph
"Get a map of all application permissions for a group."
[permission-set]
{:setting (permission-for-type permission-set :setting)
:monitoring (permission-for-type permission-set :monitoring)
:subscription (permission-for-type permission-set :subscription)})
-(s/defn graph :- ApplicationPermissionsGraph
+(mu/defn graph :- ApplicationPermissionsGraph
"Fetch a graph representing the application permissions status for groups that has at least one application permission enabled.
This works just like the function of the same name in `metabase.models.permissions`;
see also the documentation for that function."
@@ -69,7 +71,7 @@
:no
(perms/revoke-application-permissions! group-id perm-type))))
-(s/defn update-graph!
+(mu/defn update-graph!
"Update the application Permissions graph.
This works just like [[metabase.models.permission/update-data-perms-graph!]], but for Application permissions;
refer to that function's extensive documentation to get a sense for how this works."
diff --git a/enterprise/backend/src/metabase_enterprise/enhancements/integrations/ldap.clj b/enterprise/backend/src/metabase_enterprise/enhancements/integrations/ldap.clj
index 841f9d65f206225eceea3ad402ab33bf1416055b..20830eacc63632dd14a3cd0b1ee044a56b6ce6a6 100644
--- a/enterprise/backend/src/metabase_enterprise/enhancements/integrations/ldap.clj
+++ b/enterprise/backend/src/metabase_enterprise/enhancements/integrations/ldap.clj
@@ -3,7 +3,6 @@
(:require
[metabase.integrations.common :as integrations.common]
[metabase.integrations.ldap.default-implementation :as default-impl]
- [metabase.models.interface :as mi]
[metabase.models.setting :as setting :refer [defsetting]]
[metabase.models.user :as user :refer [User]]
[metabase.public-settings.premium-features
@@ -11,15 +10,14 @@
:refer [defenterprise-schema]]
[metabase.util :as u]
[metabase.util.i18n :refer [deferred-tru]]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2])
(:import
(com.unboundid.ldap.sdk LDAPConnectionPool)))
(def ^:private EEUserInfo
- (assoc default-impl/UserInfo :attributes (s/maybe {s/Keyword s/Any})))
+ [:merge default-impl/UserInfo
+ [:map [:attributes [:maybe [:map-of :keyword :any]]]]])
(defsetting ldap-sync-user-attributes
(deferred-tru "Should we sync user attributes when someone logs in via LDAP?")
@@ -60,11 +58,11 @@
(t2/select-one [User :id :last_login :is_active] :id (:id user))) ; Reload updated user
user))))
-(defenterprise-schema find-user :- (s/maybe EEUserInfo)
+(defenterprise-schema find-user :- [:maybe EEUserInfo]
"Get user information for the supplied username."
:feature :sso-ldap
- [ldap-connection :- LDAPConnectionPool
- username :- su/NonBlankString
+ [ldap-connection :- (ms/InstanceOfClass LDAPConnectionPool)
+ username :- ms/NonBlankString
settings :- default-impl/LDAPSettings]
(when-let [result (default-impl/search ldap-connection username settings)]
(when-let [user-info (default-impl/ldap-search-result->user-info
@@ -76,7 +74,7 @@
;;; for some reason the `:clj-kondo/ignore` doesn't work inside of [[defenterprise-schema]]
#_{:clj-kondo/ignore [:deprecated-var]}
-(defenterprise-schema fetch-or-create-user! :- (mi/InstanceOf:Schema User)
+(defenterprise-schema fetch-or-create-user! :- (ms/InstanceOf User)
"Using the `user-info` (from `find-user`) get the corresponding Metabase user, creating it if necessary."
:feature :sso-ldap
[{:keys [first-name last-name email groups attributes], :as user-info} :- EEUserInfo
diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj b/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj
index d9fee0b34e5d2c52ca101ae4317bc623e5a85fc0..7f378f6ff63f23de78b7beb8e5ecda904370c5b7 100644
--- a/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj
+++ b/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj
@@ -31,9 +31,6 @@
[metabase.util.log :as log]
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
#_{:clj-kondo/ignore [:discouraged-namespace]}
[toucan2.core :as t2]))
@@ -145,8 +142,8 @@
{:source-query source-query}
e)))))
-(s/defn ^:private card-gtap->source
- [{card-id :card_id, :as gtap}]
+(defn- card-gtap->source
+ [{card-id :card_id :as gtap}]
(update-in (fetch-source-query/card-id->source-query-and-metadata card-id)
[:source-query :parameters]
concat
@@ -155,7 +152,7 @@
(defn- table-gtap->source [{table-id :table_id, :as gtap}]
{:source-query {:source-table table-id, :parameters (gtap->parameters gtap)}})
-(s/defn ^:private mbql-query-metadata :- (su/non-empty [su/Map])
+(mu/defn ^:private mbql-query-metadata :- [:+ :map]
[inner-query]
(binding [*current-user-id* nil]
((requiring-resolve 'metabase.query-processor/query->expected-cols)
@@ -172,9 +169,9 @@
(mbql-query-metadata {:source-table table-id}))
:ttl/threshold (u/minutes->ms 1)))
-(s/defn ^:private reconcile-metadata :- (su/non-empty [su/Map])
+(mu/defn ^:private reconcile-metadata :- [:+ :map]
"Combine the metadata in `source-query-metadata` with the `table-metadata` from the Table being sandboxed."
- [source-query-metadata :- (su/non-empty [su/Map]) table-metadata]
+ [source-query-metadata :- [:+ :map] table-metadata]
(let [col-name->table-metadata (m/index-by :name table-metadata)]
(vec
(for [col source-query-metadata
@@ -184,8 +181,8 @@
(gtap/check-column-types-match col table-col)
table-col)))))
-(s/defn ^:private native-query-metadata :- (su/non-empty [su/Map])
- [source-query :- {:source-query s/Any, s/Keyword s/Any}]
+(mu/defn ^:private native-query-metadata :- [:+ :map]
+ [source-query :- [:map [:source-query :any]]]
(let [result (binding [*current-user-id* nil]
((requiring-resolve 'metabase.query-processor/process-query)
{:database (u/the-id (lib.metadata/database (qp.store/metadata-provider)))
@@ -197,30 +194,31 @@
{:source-query source-query
:result result})))))
-(s/defn ^:private source-query-form-ensure-metadata :- {:source-query s/Any
- :source-metadata (su/non-empty [su/Map])
- s/Keyword s/Any}
+(mu/defn ^:private source-query-form-ensure-metadata :- [:and [:map-of :keyword :any]
+ [:map
+ [:source-query :any]
+ [:source-metadata [:+ :map]]]]
"Add `:source-metadata` to a `source-query` if needed. If the source metadata had to be resolved (because Card with
`card-id`) didn't already have it, save it so we don't have to resolve it again next time around."
- [{:keys [source-metadata], :as source-query} :- {:source-query s/Any, s/Keyword s/Any}
- table-id :- su/IntGreaterThanZero
- card-id :- (s/maybe su/IntGreaterThanZero)]
+ [{:keys [source-metadata], :as source-query} :- [:and [:map-of :keyword :any] [:map [:source-query :any]]]
+ table-id :- ms/PositiveInt
+ card-id :- [:maybe ms/PositiveInt]]
(let [table-metadata (original-table-metadata table-id)
;; make sure source query has `:source-metadata`; add it if needed
[metadata save?] (cond
- ;; if it already has `:source-metadata`, we're good to go.
- (seq source-metadata)
- [source-metadata false]
-
- ;; if it doesn't have source metadata, but it's an MBQL query, we can preprocess the query to
- ;; get the expected metadata.
- (not (get-in source-query [:source-query :native]))
- [(mbql-query-metadata source-query) true]
-
- ;; otherwise if it's a native query we'll have to run the query really quickly to get the
- ;; expected metadata.
- :else
- [(native-query-metadata source-query) true])
+ ;; if it already has `:source-metadata`, we're good to go.
+ (seq source-metadata)
+ [source-metadata false]
+
+ ;; if it doesn't have source metadata, but it's an MBQL query, we can preprocess the query to
+ ;; get the expected metadata.
+ (not (get-in source-query [:source-query :native]))
+ [(mbql-query-metadata source-query) true]
+
+ ;; otherwise if it's a native query we'll have to run the query really quickly to get the
+ ;; expected metadata.
+ :else
+ [(native-query-metadata source-query) true])
metadata (reconcile-metadata metadata table-metadata)]
(assert (seq metadata))
;; save the result metadata so we don't have to do it again next time if applicable
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj b/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
index 19130a62a0331025bc5f85bd5b3ef8151f7d727e..d41cb0dcba72615535d0644e0a1f876b525e6439 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/cmd.clj
@@ -27,31 +27,29 @@
[metabase.util.i18n :refer [deferred-trs trs]]
[metabase.util.log :as log]
[metabase.util.malli :as mu]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
[toucan2.core :as t2]))
(set! *warn-on-reflection* true)
(def ^:private Mode
- (su/with-api-error-message (s/enum :skip :update)
+ (mu/with-api-error-message [:enum :skip :update]
(deferred-trs "invalid --mode value")))
(def ^:private OnError
- (su/with-api-error-message (s/enum :continue :abort)
+ (mu/with-api-error-message [:enum :continue :abort]
(deferred-trs "invalid --on-error value")))
(def ^:private Context
- (su/with-api-error-message
- {(s/optional-key :on-error) OnError
- (s/optional-key :mode) Mode}
+ (mu/with-api-error-message
+ [:map {:closed true}
+ [:on-error {:optional true} OnError]
+ [:mode {:optional true} Mode]]
(deferred-trs "invalid context seed value")))
(defn- check-premium-token! []
(premium-features/assert-has-feature :serialization (trs "Serialization")))
-(s/defn v1-load
+(mu/defn v1-load
"Load serialized metabase instance as created by [[dump]] command from directory `path`."
[path context :- Context]
(plugins/load-plugins!)
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/names.clj b/enterprise/backend/src/metabase_enterprise/serialization/names.clj
index 83c3b6bdadd93c93cc018ed4f93654a2b60cac06..82c3916722c95daa05441ff1638cf0b85b8e5fec 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/names.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/names.clj
@@ -2,6 +2,7 @@
"Consistent instance-independent naming scheme that replaces IDs with human-readable paths."
(:require
[clojure.string :as str]
+ [malli.core :as mc]
[metabase.db.connection :as mdb.connection]
[metabase.lib.schema.id :as lib.schema.id]
[metabase.models.card :refer [Card]]
@@ -18,10 +19,8 @@
[metabase.models.user :refer [User]]
[metabase.util.i18n :as i18n :refer [trs]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[ring.util.codec :as codec]
- [schema.core :as s]
[toucan2.core :as t2]
[toucan2.protocols :as t2.protocols]))
@@ -133,18 +132,19 @@
;; All the references in the dumps should resolved to entities already loaded.
(def ^:private Context
- {(s/optional-key :database) su/IntGreaterThanZero
- (s/optional-key :table) su/IntGreaterThanZero
- (s/optional-key :schema) (s/maybe s/Str)
- (s/optional-key :field) su/IntGreaterThanZero
- (s/optional-key :metric) su/IntGreaterThanZero
- (s/optional-key :segment) su/IntGreaterThanZero
- (s/optional-key :card) su/IntGreaterThanZero
- (s/optional-key :dashboard) su/IntGreaterThanZero
- (s/optional-key :collection) (s/maybe su/IntGreaterThanZero) ; root collection
- (s/optional-key :pulse) su/IntGreaterThanZero
- (s/optional-key :user) su/IntGreaterThanZero
- (s/optional-key :snippet) (s/maybe su/IntGreaterThanZero)})
+ [:map {:closed true}
+ [:database {:optional true} ms/PositiveInt]
+ [:table {:optional true} ms/PositiveInt]
+ [:schema {:optional true} [:maybe :string]]
+ [:field {:optional true} ms/PositiveInt]
+ [:metric {:optional true} ms/PositiveInt]
+ [:segment {:optional true} ms/PositiveInt]
+ [:card {:optional true} ms/PositiveInt]
+ [:dashboard {:optional true} ms/PositiveInt]
+ [:collection {:optional true} [:maybe ms/PositiveInt]] ; root collection
+ [:pulse {:optional true} ms/PositiveInt]
+ [:user {:optional true} ms/PositiveInt]
+ [:snippet {:optional true} [:maybe ms/PositiveInt]]])
(defmulti ^:private path->context* (fn [_ model _ _]
model))
@@ -313,11 +313,11 @@
partition-name-components
(map (fn [[model-name & entity-parts]]
(cond-> {::model-name model-name ::entity-name (last entity-parts)}
- (and (= "collections" model-name) (> (count entity-parts) 1))
- (assoc :namespace (->> entity-parts
- first ; ns is first/only item after "collections"
- rest ; strip the starting :
- (apply str)))))))
+ (and (= "collections" model-name) (> (count entity-parts) 1))
+ (assoc :namespace (->> entity-parts
+ first ; ns is first/only item after "collections"
+ rest ; strip the starting :
+ (apply str)))))))
context (loop [acc-context {}
[{::keys [model-name entity-name] :as model-map} & more] components]
(let [model-attrs (dissoc model-map ::model-name ::entity-name)
@@ -325,18 +325,17 @@
(if (empty? more)
new-context
(recur new-context more))))]
- (try
- (s/validate (s/maybe Context) context)
- (catch Exception e
- (when-not *suppress-log-name-lookup-exception*
- (log/warn
- (ex-info (trs "Can''t resolve {0} in fully qualified name {1}"
- (str/join ", " (map name (keys (:value (ex-data e)))))
- fully-qualified-name)
- {:fully-qualified-name fully-qualified-name
- :resolve-name-failed? true
- :context context}
- e))))))))
+ (if (and
+ (not (mc/validate [:maybe Context] context))
+ (not *suppress-log-name-lookup-exception*))
+ (log/warn
+ (ex-info (trs "Can''t resolve {0} in fully qualified name {1}"
+ (str/join ", " (map name (keys context)))
+ fully-qualified-name)
+ {:fully-qualified-name fully-qualified-name
+ :resolve-name-failed? true
+ :context context}))
+ context))))
(defn name-for-logging
"Return a string representation of entity suitable for logs"
diff --git a/enterprise/backend/src/metabase_enterprise/snippet_collections/models/native_query_snippet/permissions.clj b/enterprise/backend/src/metabase_enterprise/snippet_collections/models/native_query_snippet/permissions.clj
index 07411ab2e9ba2ed5aecf5802fd1d3edbe29a349a..1ef045921a0e165ea50b730183085640d18c8b42 100644
--- a/enterprise/backend/src/metabase_enterprise/snippet_collections/models/native_query_snippet/permissions.clj
+++ b/enterprise/backend/src/metabase_enterprise/snippet_collections/models/native_query_snippet/permissions.clj
@@ -7,14 +7,13 @@
[metabase.public-settings.premium-features
:as premium-features
:refer [defenterprise]]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2]))
-(s/defn ^:private has-parent-collection-perms?
- [snippet :- {:collection_id (s/maybe su/IntGreaterThanZero), s/Keyword s/Any}
- read-or-write :- (s/enum :read :write)]
+(mu/defn ^:private has-parent-collection-perms?
+ [snippet :- [:map [:collection_id [:maybe ms/PositiveInt]]]
+ read-or-write :- [:enum :read :write]]
(mi/current-user-has-full-permissions? (perms/perms-objects-set-for-parent-collection "snippets" snippet read-or-write)))
(defenterprise can-read?
diff --git a/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj b/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj
index 265e0b27c363fb2e120af7cf0ea7c7a290242f50..8bd28f013e5fc8dcf3eb773e8cd237e0cc1f658b 100644
--- a/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj
+++ b/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj
@@ -3,24 +3,25 @@
the SSO backends and the generic routing code used to determine which SSO backend to use need this
information. Separating out this information creates a better dependency graph and avoids circular dependencies."
(:require
+ [malli.core :as mc]
[metabase.integrations.common :as integrations.common]
[metabase.models.setting :as setting :refer [defsetting]]
[metabase.models.setting.multi-setting :refer [define-multi-setting-impl]]
[metabase.public-settings :as public-settings]
[metabase.util.i18n :refer [deferred-tru trs tru]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[saml20-clj.core :as saml]
[schema.core :as s]))
(set! *warn-on-reflection* true)
(def ^:private GroupMappings
- (s/maybe {su/KeywordOrString [su/IntGreaterThanZero]}))
+ [:maybe [:map-of ms/KeywordOrString [:sequential ms/PositiveInt]]])
(def ^:private ^{:arglists '([group-mappings])} validate-group-mappings
- (s/validator GroupMappings))
+ (mc/validator GroupMappings))
(defsetting saml-identity-provider-uri
(deferred-tru "This is the URL where your users go to log in to your identity provider. Depending on which IdP you''re
@@ -107,7 +108,8 @@ on your IdP, this usually looks something like http://www.example.com/141xkex604
:cache? false
:default {}
:feature :sso-saml
- :setter (comp (partial setting/set-value-of-type! :json :saml-group-mappings) validate-group-mappings))
+ :setter (comp (partial setting/set-value-of-type! :json :saml-group-mappings)
+ (partial mu/validate-throw validate-group-mappings)))
(defsetting saml-configured
(deferred-tru "Are the mandatory SAML settings configured?")
@@ -174,7 +176,8 @@ on your IdP, this usually looks something like http://www.example.com/141xkex604
:cache? false
:default {}
:feature :sso-jwt
- :setter (comp (partial setting/set-value-of-type! :json :jwt-group-mappings) validate-group-mappings))
+ :setter (comp (partial setting/set-value-of-type! :json :jwt-group-mappings)
+ (partial mu/validate-throw validate-group-mappings)))
(defsetting jwt-configured
(deferred-tru "Are the mandatory JWT settings configured?")
diff --git a/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj b/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj
index 102b1fa57781c8218c03652ad046347baace7cd0..c4190d14f156a6e8814439250d999ca52d395d94 100644
--- a/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj
+++ b/enterprise/backend/src/metabase_enterprise/sso/integrations/sso_utils.clj
@@ -9,9 +9,8 @@
[metabase.util :as u]
[metabase.util.i18n :refer [trs tru]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2])
(:import
(java.net URI)))
@@ -19,15 +18,16 @@
(set! *warn-on-reflection* true)
(def ^:private UserAttributes
- {:first_name (s/maybe su/NonBlankString)
- :last_name (s/maybe su/NonBlankString)
- :email su/Email
+ [:map {:closed true}
+ [:first_name [:maybe ms/NonBlankString]]
+ [:last_name [:maybe ms/NonBlankString]]
+ [:email ms/Email]
;; TODO - we should avoid hardcoding this to make it easier to add new integrations. Maybe look at something like
;; the keys of `(methods sso/sso-get)`
- :sso_source (s/enum :saml :jwt)
- :login_attributes (s/maybe {s/Any s/Any})})
+ [:sso_source [:enum :saml :jwt]]
+ [:login_attributes [:maybe :map]]])
-(s/defn create-new-sso-user!
+(mu/defn create-new-sso-user!
"This function is basically the same thing as the `create-new-google-auth-user` from `metabase.models.user`. We need
to refactor the `core_user` table structure and the function used to populate it so that the enterprise product can
reuse it"
diff --git a/enterprise/backend/test/metabase_enterprise/enhancements/integrations/ldap_test.clj b/enterprise/backend/test/metabase_enterprise/enhancements/integrations/ldap_test.clj
index 0fc3b244d1d1d4f64d7b0fa90b77194e893b4949..d4686ab2d063d073044ecbd0d4db7b00cdc9555a 100644
--- a/enterprise/backend/test/metabase_enterprise/enhancements/integrations/ldap_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/enhancements/integrations/ldap_test.clj
@@ -8,8 +8,7 @@
:as premium-features-test]
[metabase.test :as mt]
[metabase.test.integrations.ldap :as ldap.test]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[schema.core :as s]
[toucan2.core :as t2]))
@@ -177,8 +176,9 @@
s/Keyword s/Any}
(ldap/fetch-or-create-user! user-info))))
(testing "Call fetch-or-create-user! again to trigger update"
- (is (schema= {:id su/IntGreaterThanZero s/Keyword s/Any}
- (ldap/fetch-or-create-user! (assoc-in user-info [:attributes :unladenspeed] 100)))))
+ (is (malli= [:and [:map-of :keyword :any]
+ [:map [:id ms/PositiveInt]]]
+ (ldap/fetch-or-create-user! (assoc-in user-info [:attributes :unladenspeed] 100)))))
(is (= {:first_name "John"
:last_name "Smith"
:common_name "John Smith"
@@ -199,12 +199,13 @@
(mt/with-temporary-setting-values [ldap-sync-user-attributes false]
(let [user-info (ldap/find-user "jsmith1")]
(testing "First let a user get created for John Smith"
- (is (schema= {:email (s/eq "john.smith@metabase.com")
- s/Keyword s/Any}
- (ldap/fetch-or-create-user! user-info))))
+ (is (malli= [:and [:map-of :keyword :any]
+ [:map [:email [:= "john.smith@metabase.com"]]]]
+ (ldap/fetch-or-create-user! user-info))))
(testing "Call fetch-or-create-user! again to trigger update"
- (is (schema= {:id su/IntGreaterThanZero s/Keyword s/Any}
- (ldap/fetch-or-create-user! (assoc-in user-info [:attributes :unladenspeed] 100)))))
+ (is (malli= [:and [:map-of :keyword :any]
+ [:map [:id ms/PositiveInt]]]
+ (ldap/fetch-or-create-user! (assoc-in user-info [:attributes :unladenspeed] 100)))))
(is (= {:first_name "John"
:last_name "Smith"
:common_name "John Smith"
diff --git a/enterprise/backend/test/metabase_enterprise/sandbox/api/permissions_test.clj b/enterprise/backend/test/metabase_enterprise/sandbox/api/permissions_test.clj
index 3abe4a9c00b8c6be5d995536172ee8d5ce87fdd3..4baab154c1ab644bb60d1ff1fc5f4f925e164197 100644
--- a/enterprise/backend/test/metabase_enterprise/sandbox/api/permissions_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/sandbox/api/permissions_test.clj
@@ -13,9 +13,7 @@
[metabase.query-processor :as qp]
[metabase.test :as mt]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -73,14 +71,14 @@
(get-in (mt/user-http-request :crowberto :get 200 "permissions/graph")
(venues-perms-graph-keypath &group)))))
(testing "GTAP should exist in application DB"
- (is (schema= [(s/one {:id su/IntGreaterThanZero
- :group_id (s/eq (u/the-id &group))
- :table_id (s/eq (mt/id :venues))
- :card_id (s/eq nil)
- :attribute_remappings (s/eq nil)
- s/Keyword s/Any}
- "GTAP")]
- (t2/select GroupTableAccessPolicy :group_id (u/the-id &group))))))
+ (is (malli= [:tuple
+ [:map
+ [:id ms/PositiveInt]
+ [:group_id [:= (u/the-id &group)]]
+ [:table_id [:= (mt/id :venues)]]
+ [:card_id nil?]
+ [:attribute_remappings nil?]]]
+ (t2/select GroupTableAccessPolicy :group_id (u/the-id &group))))))
(let [graph (mt/user-http-request :crowberto :get 200 "permissions/graph")
graph' (assoc-in graph (db-graph-keypath &group) (updated-db-perms))
response (mt/user-http-request :crowberto :put 200 "permissions/graph" graph')]
@@ -105,25 +103,27 @@
:group_id (u/the-id &group)
:table_id (mt/id :venues)))))
(testing "GTAP for same group, other database should not be affected"
- (is (schema= [(s/one {:id su/IntGreaterThanZero
- :group_id (s/eq (u/the-id &group))
- :table_id (s/eq (u/the-id db-2-table))
- :card_id (s/eq nil)
- :permission_id (s/eq nil)
- :attribute_remappings (s/eq nil)}
- "GTAP")]
- (t2/select GroupTableAccessPolicy
- :group_id (u/the-id &group)
- :table_id (u/the-id db-2-table)))))
+ (is (malli= [:tuple
+ [:map
+ [:id ms/PositiveInt]
+ [:group_id [:= (u/the-id &group)]]
+ [:table_id [:= (u/the-id db-2-table)]]
+ [:card_id nil?]
+ [:permission_id nil?]
+ [:attribute_remappings nil?]]]
+ (t2/select GroupTableAccessPolicy
+ :group_id (u/the-id &group)
+ :table_id (u/the-id db-2-table)))))
(testing "GTAP for same table, other group should not be affected"
- (is (schema= [(s/one {:id su/IntGreaterThanZero
- :group_id (s/eq (u/the-id other-group))
- :table_id (s/eq (mt/id :venues))
- :card_id (s/eq nil)
- :permission_id (s/eq nil)
- :attribute_remappings (s/eq nil)}
- "GTAP")]
- (t2/select GroupTableAccessPolicy :group_id (u/the-id other-group)))))))))))))
+ (is (malli= [:tuple
+ [:map
+ [:id ms/PositiveInt]
+ [:group_id [:= (u/the-id other-group)]]
+ [:table_id [:= (mt/id :venues)]]
+ [:card_id nil?]
+ [:permission_id nil?]
+ [:attribute_remappings nil?]]]
+ (t2/select GroupTableAccessPolicy :group_id (u/the-id other-group)))))))))))))
(deftest grant-sandbox-perms-dont-delete-gtaps-test
(testing "PUT /api/permissions/graph"
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/names_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/names_test.clj
index 0dfc0b7018ae24f19ca0b0a39d658d38bafd5e1c..53d7a7dcc644f80040255982f12c63476871c9cd 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/names_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/names_test.clj
@@ -77,13 +77,13 @@
:name "Price > 2"
:description "Price more than 2"
:collection_id collection-id}]
- (let [fully-qualified-name (str "/collections/root/collections/:snippets/Base Collection/collections"
- "/:snippets/Nested Collection/snippets/Price %3E 2")]
- (is (= fully-qualified-name
- (names/fully-qualified-name snippet)))
- (is (= {:collection collection-id
- :snippet (u/the-id snippet)}
- (names/fully-qualified-name->context fully-qualified-name))))))
+ (let [fully-qualified-name (str "/collections/root/collections/:snippets/Base Collection/collections"
+ "/:snippets/Nested Collection/snippets/Price %3E 2")]
+ (is (= fully-qualified-name
+ (names/fully-qualified-name snippet)))
+ (is (= {:collection collection-id
+ :snippet (u/the-id snippet)}
+ (names/fully-qualified-name->context fully-qualified-name))))))
(testing " with path elements matching one of our entity names"
; these drivers keep table names lowercased, causing "users" table to clash with our entity name "users"
(mt/test-drivers #{:postgres :mysql}
diff --git a/enterprise/backend/test/metabase_enterprise/util_test.clj b/enterprise/backend/test/metabase_enterprise/util_test.clj
index 88736371fe14a28c5d9c83c209ad9c6fa056bde2..76a35cc0fb5ea1b4e815c5d59d8c9287b3651061 100644
--- a/enterprise/backend/test/metabase_enterprise/util_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/util_test.clj
@@ -1,7 +1,6 @@
(ns metabase-enterprise.util-test
(:require
- [metabase.public-settings.premium-features :refer [defenterprise defenterprise-schema]]
- [schema.core :as s]))
+ [metabase.public-settings.premium-features :refer [defenterprise defenterprise-schema]]))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Defenterprise Macro |
@@ -35,20 +34,20 @@
[username]
(format "Hi %s, you're an extra special EE customer!" (name username)))
-(defenterprise-schema greeting-with-schema :- s/Str
+(defenterprise-schema greeting-with-schema :- :string
"Returns a greeting for a user, with schemas for the argument and return value."
:feature :none
- [username :- s/Keyword]
+ [username :- :keyword]
(format "Hi %s, the schema was valid, and you're running the Enterprise Edition of Metabase!" (name username)))
-(defenterprise-schema greeting-with-invalid-oss-return-schema :- s/Str
+(defenterprise-schema greeting-with-invalid-oss-return-schema :- :string
"Returns a greeting for a user."
:feature :none
- [username :- s/Keyword]
+ [username :- :keyword]
(format "Hi %s, the schema was valid, and you're running the Enterprise Edition of Metabase!" (name username)))
-(defenterprise-schema greeting-with-invalid-ee-return-schema :- s/Keyword
+(defenterprise-schema greeting-with-invalid-ee-return-schema :- :keyword
"Returns a greeting for a user."
:feature :custom-feature
- [username :- s/Keyword]
+ [username :- :keyword]
(format "Hi %s, the schema was valid, and you're running the Enterprise Edition of Metabase!" (name username)))
diff --git a/src/metabase/api/common.clj b/src/metabase/api/common.clj
index 392a3c92ec3f9dbaea638202040c8737d97bcc0f..769862011bd81c21f72921f8a900fe5ca166db56 100644
--- a/src/metabase/api/common.clj
+++ b/src/metabase/api/common.clj
@@ -18,10 +18,9 @@
[metabase.util :as u]
[metabase.util.i18n :as i18n :refer [deferred-tru tru]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[ring.middleware.multipart-params :as mp]
- [schema.core :as schema]
[toucan2.core :as t2]))
(declare check-403 check-404)
@@ -450,7 +449,7 @@
(check (not (and limit (not offset))) [400 (tru "When including a limit, an offset must also be included.")])
(check (not (and offset (not limit))) [400 (tru "When including an offset, a limit must also be included.")]))
-(schema/defn column-will-change? :- schema/Bool
+(mu/defn column-will-change? :- :boolean
"Helper for PATCH-style operations to see if a column is set to change when `object-updates` (i.e., the input to the
endpoint) is applied.
@@ -460,7 +459,7 @@
(api/column-will-change? :archived (t2/select-one Collection :id 10) {:archived false}) ; -> false, because value did not change
(api/column-will-change? :archived (t2/select-one Collection :id 10) {}) ; -> false; value not specified in updates (request body)"
- [k :- schema/Keyword, object-before-updates :- su/Map, object-updates :- su/Map]
+ [k :- :keyword object-before-updates :- :map object-updates :- :map]
(boolean
(and (contains? object-updates k)
(not= (get object-before-updates k)
@@ -468,12 +467,12 @@
;;; ------------------------------------------ COLLECTION POSITION HELPER FNS ----------------------------------------
-(schema/defn reconcile-position-for-collection!
+(mu/defn reconcile-position-for-collection!
"Compare `old-position` and `new-position` to determine what needs to be updated based on the position change. Used
for fixing card/dashboard/pulse changes that impact other instances in the collection"
- [collection-id :- (schema/maybe su/IntGreaterThanZero)
- old-position :- (schema/maybe su/IntGreaterThanZero)
- new-position :- (schema/maybe su/IntGreaterThanZero)]
+ [collection-id :- [:maybe ms/PositiveInt]
+ old-position :- [:maybe ms/PositiveInt]
+ new-position :- [:maybe ms/PositiveInt]]
(let [update-fn! (fn [plus-or-minus position-update-clause]
(doseq [model '[Card Dashboard Pulse]]
(t2/update! model {:collection_id collection-id
@@ -496,25 +495,25 @@
(def ^:private ModelWithPosition
"Intended to cover Cards/Dashboards/Pulses, it only asserts collection id and position, allowing extra keys"
- {:collection_id (schema/maybe su/IntGreaterThanZero)
- :collection_position (schema/maybe su/IntGreaterThanZero)
- schema/Any schema/Any})
+ [:map
+ [:collection_id [:maybe ms/PositiveInt]]
+ [:collection_position [:maybe ms/PositiveInt]]])
(def ^:private ModelWithOptionalPosition
"Intended to cover Cards/Dashboards/Pulses updates. Collection id and position are optional, if they are not
present, they didn't change. If they are present, they might have changed and we need to compare."
- {(schema/optional-key :collection_id) (schema/maybe su/IntGreaterThanZero)
- (schema/optional-key :collection_position) (schema/maybe su/IntGreaterThanZero)
- schema/Any schema/Any})
+ [:map
+ [:collection_id {:optional true} [:maybe ms/PositiveInt]]
+ [:collection_position {:optional true} [:maybe ms/PositiveInt]]])
-(schema/defn maybe-reconcile-collection-position!
+(mu/defn maybe-reconcile-collection-position!
"Generic function for working on cards/dashboards/pulses. Checks the before and after changes to see if there is any
impact to the collection position of that model instance. If so, executes updates to fix the collection position
that goes with the change. The 2-arg version of this function is used for a new card/dashboard/pulse (i.e. not
updating an existing instance, but creating a new one)."
([new-model-data :- ModelWithPosition]
(maybe-reconcile-collection-position! nil new-model-data))
- ([{old-collection-id :collection_id, old-position :collection_position, :as _before-update} :- (schema/maybe ModelWithPosition)
+ ([{old-collection-id :collection_id, old-position :collection_position, :as _before-update} :- [:maybe ModelWithPosition]
{new-collection-id :collection_id, new-position :collection_position, :as model-updates} :- ModelWithOptionalPosition]
(let [updated-collection? (and (contains? model-updates :collection_id)
(not= old-collection-id new-collection-id))
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index 7b9a4c2c99178a7189618ae7091aa0f3e72bbfd2..56efa6d1de434eba2b6321579bc20582faee820e 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -42,9 +42,6 @@
[metabase.util.log :as log]
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
[toucan2.core :as t2]))
(set! *warn-on-reflection* true)
@@ -468,10 +465,10 @@
(mbql.u/match-one field-clause [:field (id :guard integer?) _] id)))
;; TODO -- should we only check *new* or *modified* mappings?
-(s/defn ^:private check-parameter-mapping-permissions
+(mu/defn ^:private check-parameter-mapping-permissions
"Starting in 0.41.0, you must have *data* permissions in order to add or modify a DashboardCard parameter mapping."
{:added "0.41.0"}
- [parameter-mappings :- [dashboard-card/ParamMapping]]
+ [parameter-mappings :- [:sequential dashboard-card/ParamMapping]]
(when (seq parameter-mappings)
;; calculate a set of all Field IDs referenced by parameter mappings; then from those Field IDs calculate a set of
;; all Table IDs to which those Fields belong. This is done in a batched fashion so we can avoid N+1 query issues
@@ -786,8 +783,8 @@
"How many results to return when chain filtering"
1000)
-(s/defn ^:private mappings->field-ids :- (s/maybe #{su/IntGreaterThanZero})
- [parameter-mappings :- (s/maybe (s/cond-pre #{dashboard-card/ParamMapping} [dashboard-card/ParamMapping]))]
+(mu/defn ^:private mappings->field-ids :- [:maybe [:set ms/PositiveInt]]
+ [parameter-mappings :- [:maybe [:or [:set dashboard-card/ParamMapping] [:sequential dashboard-card/ParamMapping]]]]
(set (for [{{:keys [card]} :dashcard :keys [target]} parameter-mappings
:let [field-clause (params/param-target->field-clause target card)]
:when field-clause
@@ -857,7 +854,7 @@
(api/throw-403 e)
(throw e)))))))
-(s/defn param-values
+(mu/defn param-values
"Fetch values for a parameter.
The source of values could be:
@@ -867,10 +864,10 @@
([dashboard param-key query-params]
(param-values dashboard param-key query-params nil))
- ([dashboard :- su/Map
- param-key :- su/NonBlankString
- constraint-param-key->value :- su/Map
- query :- (s/maybe su/NonBlankString)]
+ ([dashboard :- :map
+ param-key :- ms/NonBlankString
+ constraint-param-key->value :- :map
+ query :- [:maybe ms/NonBlankString]]
(let [dashboard (t2/hydrate dashboard :resolved-params)
param (get (:resolved-params dashboard) param-key)]
(when-not param
diff --git a/src/metabase/api/geojson.clj b/src/metabase/api/geojson.clj
index 1657d14de8b4f884297ee91a6903fdf17b1855ac..e7b4c51cf04cf94673020183c4a714264f93750b 100644
--- a/src/metabase/api/geojson.clj
+++ b/src/metabase/api/geojson.clj
@@ -3,16 +3,14 @@
[clj-http.client :as http]
[clojure.java.io :as io]
[compojure.core :refer [GET]]
+ [malli.core :as mc]
[metabase.api.common :as api]
[metabase.api.common.validation :as validation]
[metabase.models.setting :as setting :refer [defsetting]]
[metabase.util.i18n :refer [deferred-tru tru]]
[metabase.util.malli.schema :as ms]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
[ring.util.codec :as codec]
- [ring.util.response :as response]
- [schema.core :as s])
+ [ring.util.response :as response])
(:import
(java.io BufferedReader)
(java.net InetAddress URL)
@@ -28,11 +26,14 @@
:default true)
(def ^:private CustomGeoJSON
- {s/Keyword {:name su/NonBlankString
- :url su/NonBlankString
- :region_key (s/maybe s/Str)
- :region_name (s/maybe s/Str)
- (s/optional-key :builtin) s/Bool}})
+ [:map-of :keyword [:map {:closed true}
+ [:name ms/NonBlankString]
+ [:url ms/NonBlankString]
+ [:region_key [:maybe :string]]
+ [:region_name [:maybe :string]]
+ [:builtin {:optional true} :boolean]]])
+
+(def ^:private CustomGeoJSONValidator (mc/validator CustomGeoJSON))
(def ^:private builtin-geojson
{:us_states {:name "United States"
@@ -46,7 +47,7 @@
:region_name "NAME"
:builtin true}})
-(defn- invalid-location-msg []
+(defn- invalid-location-msg []
(str (tru "Invalid GeoJSON file location: must either start with http:// or https:// or be a relative path to a file on the classpath.")
" "
(tru "URLs referring to hosts that supply internal hosting metadata are prohibited.")))
@@ -89,10 +90,8 @@
(defn- validate-geojson
"Throws a 400 if the supplied `geojson` is poorly structured or has an illegal URL/path"
[geojson]
- (try
- (s/validate CustomGeoJSON geojson)
- (catch Throwable e
- (throw (ex-info (tru "Invalid custom GeoJSON") {:status-code 400} e))))
+ (when-not (CustomGeoJSONValidator geojson)
+ (throw (ex-info (tru "Invalid custom GeoJSON") {:status-code 400})))
(or (valid-geojson-urls? geojson)
(throw (ex-info (invalid-location-msg) {:status-code 400}))))
diff --git a/src/metabase/api/session.clj b/src/metabase/api/session.clj
index de855bd1ddb4950d15847905a404c53a401f4976..51b659b7c0b3a764ffe94b72cd21a28b8fcb75df 100644
--- a/src/metabase/api/session.clj
+++ b/src/metabase/api/session.clj
@@ -23,11 +23,9 @@
[metabase.util :as u]
[metabase.util.i18n :refer [deferred-tru trs tru]]
[metabase.util.log :as log]
+ [metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
[metabase.util.password :as u.password]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
[throttle.core :as throttle]
[toucan2.core :as t2])
(:import
@@ -36,8 +34,10 @@
(set! *warn-on-reflection* true)
-(s/defn ^:private record-login-history!
- [session-id :- UUID user-id :- su/IntGreaterThanZero device-info :- request.u/DeviceInfo]
+(mu/defn ^:private record-login-history!
+ [session-id :- (ms/InstanceOfClass UUID)
+ user-id :- ms/PositiveInt
+ device-info :- request.u/DeviceInfo]
(t2/insert! LoginHistory (merge {:user_id user-id
:session_id (str session-id)}
device-info)))
@@ -50,11 +50,18 @@
session-type))
(def ^:private CreateSessionUserInfo
- {:id su/IntGreaterThanZero
- :last_login s/Any
- s/Keyword s/Any})
-
-(s/defmethod create-session! :sso :- {:id UUID, :type (s/enum :normal :full-app-embed) s/Keyword s/Any}
+ [:map
+ [:id ms/PositiveInt]
+ [:last_login :any]])
+
+(def ^:private SessionSchema
+ [:and
+ [:map-of :keyword :any]
+ [:map
+ [:id (ms/InstanceOfClass UUID)]
+ [:type [:enum :normal :full-app-embed]]]])
+
+(mu/defmethod create-session! :sso :- SessionSchema
[_ user :- CreateSessionUserInfo device-info :- request.u/DeviceInfo]
(let [session-uuid (random-uuid)
session (first (t2/insert-returning-instances! Session
@@ -70,8 +77,10 @@
(snowplow/track-event! ::snowplow/new-user-created (u/the-id user)))
(assoc session :id session-uuid)))
-(s/defmethod create-session! :password :- {:id UUID, :type (s/enum :normal :full-app-embed), s/Keyword s/Any}
- [session-type user :- CreateSessionUserInfo device-info :- request.u/DeviceInfo]
+(mu/defmethod create-session! :password :- SessionSchema
+ [session-type
+ user :- CreateSessionUserInfo
+ device-info :- request.u/DeviceInfo]
;; this is actually the same as `create-session!` for `:sso` but we check whether password login is enabled.
(when-not (public-settings/enable-password-login)
(throw (ex-info (str (tru "Password login is disabled for this instance.")) {:status-code 400})))
@@ -95,7 +104,7 @@
(def ^:private fake-salt "ee169694-5eb6-4010-a145-3557252d7807")
(def ^:private fake-hashed-password "$2a$10$owKjTym0ZGEEZOpxM0UyjekSvt66y1VvmOJddkAaMB37e0VAIVOX2")
-(s/defn ^:private ldap-login :- (s/maybe {:id UUID, s/Keyword s/Any})
+(mu/defn ^:private ldap-login :- [:maybe [:map [:id (ms/InstanceOfClass UUID)]]]
"If LDAP is enabled and a matching user exists return a new Session for them, or `nil` if they couldn't be
authenticated."
[username password device-info :- request.u/DeviceInfo]
@@ -118,9 +127,11 @@
(catch LDAPSDKException e
(log/error e (trs "Problem connecting to LDAP server, will fall back to local authentication"))))))
-(s/defn ^:private email-login :- (s/maybe {:id UUID, s/Keyword s/Any})
+(mu/defn ^:private email-login :- [:maybe [:map [:id (ms/InstanceOfClass UUID)]]]
"Find a matching `User` if one exists and return a new Session for them, or `nil` if they couldn't be authenticated."
- [username password device-info :- request.u/DeviceInfo]
+ [username :- ms/NonBlankString
+ password :- [:maybe ms/NonBlankString]
+ device-info :- request.u/DeviceInfo]
(if-let [user (t2/select-one [User :id :password_salt :password :last_login :is_active], :%lower.email (u/lower-case-en username))]
(when (u.password/verify-password password (:password_salt user) (:password user))
(if (:is_active user)
@@ -141,10 +152,12 @@
(when-not throttling-disabled?
(throttle/check throttler throttle-key)))
-(s/defn ^:private login :- {:id UUID, :type (s/enum :normal :full-app-embed), s/Keyword s/Any}
+(mu/defn ^:private login :- SessionSchema
"Attempt to login with different avaialable methods with `username` and `password`, returning new Session ID or
throwing an Exception if login could not be completed."
- [username :- su/NonBlankString password :- su/NonBlankString device-info :- request.u/DeviceInfo]
+ [username :- ms/NonBlankString
+ password :- ms/NonBlankString
+ device-info :- request.u/DeviceInfo]
;; Primitive "strategy implementation", should be reworked for modular providers in #3210
(or (ldap-login username password device-info) ; First try LDAP if it's enabled
(email-login username password device-info) ; Then try local authentication
diff --git a/src/metabase/automagic_dashboards/core.clj b/src/metabase/automagic_dashboards/core.clj
index 6560d22a666b67cc88ba160216d91a44a0b02280..f356f7203f79816e0e08ab721d3a375ba88a75e2 100644
--- a/src/metabase/automagic_dashboards/core.clj
+++ b/src/metabase/automagic_dashboards/core.clj
@@ -120,8 +120,7 @@
[metabase.util.i18n :as i18n :refer [deferred-tru trs tru trun]]
[metabase.util.log :as log]
[metabase.util.malli :as mu]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[potemkin :as p]
[ring.util.codec :as codec]
[schema.core :as s]
@@ -132,10 +131,10 @@
(def ^:private ^{:arglists '([field])} id-or-name
(some-fn :id :name))
-(s/defn ->field :- (s/maybe #_{:clj-kondo/ignore [:deprecated-var]} (mi/InstanceOf:Schema Field))
+(mu/defn ->field :- [:maybe (ms/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 (s/pred mbql.preds/Field? ":field or :expression"))]
+ field-id-or-name-or-clause :- [:or ms/PositiveInt ms/NonBlankString [:fn mbql.preds/Field?]]]
(let [id-or-name (if (sequential? field-id-or-name-or-clause)
(filters/field-reference->id field-id-or-name-or-clause)
field-id-or-name-or-clause)]
@@ -1037,8 +1036,8 @@
dashboard-templates/get-dashboard-template
dash-template->affinities)]
(match-affinities affinities
- #{"SourceSmall" "Timestamp" "SourceMedium"}))
- )
+ #{"SourceSmall" "Timestamp" "SourceMedium"})))
+
(s/defn ^:private make-base-context
"Create the underlying context to which we will add metrics, dimensions, and filters.
@@ -1182,8 +1181,8 @@
(update-vals
(all-satisfied-bindings distinct-affinity-sets available-dimensions)
(fn [v]
- (mapv (fn [combo] (update-vals combo #(select-keys % [:name]))) v))))
- )
+ (mapv (fn [combo] (update-vals combo #(select-keys % [:name]))) v)))))
+
(s/defn ^:private apply-dashboard-template
"Apply a 'dashboard template' (a card template) to the root entity to produce a dashboard
diff --git a/src/metabase/db/util.clj b/src/metabase/db/util.clj
index 1003e4684414dfc204ecb124cd60e09dcb388460..6a2e8e17a723a77b4630cab4af4d27c77e618e88 100644
--- a/src/metabase/db/util.clj
+++ b/src/metabase/db/util.clj
@@ -2,9 +2,8 @@
"Utility functions for querying the application database."
(:require
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2]
[toucan2.model :as t2.model]))
@@ -32,9 +31,9 @@
[:= (qualify source-entity fk) (qualify dest-entity pk)]]})
(def ^:private NamespacedKeyword
- (s/constrained s/Keyword (comp seq namespace) "namespaced keyword"))
+ [:and :keyword [:fn (comp seq namespace)]])
-(s/defn ^:private type-keyword->descendants :- (su/non-empty #{su/NonBlankString})
+(mu/defn ^:private type-keyword->descendants :- [:set {:min 1} ms/NonBlankString]
"Return a set of descendents of Metabase `type-keyword`. This includes `type-keyword` itself, so the set will always
have at least one element.
diff --git a/src/metabase/driver/sql.clj b/src/metabase/driver/sql.clj
index 97e544d925e7b078810b78dd8f231b752d829f91..59c0dd4e7a19d592df359ce3b92503cb896f4163 100644
--- a/src/metabase/driver/sql.clj
+++ b/src/metabase/driver/sql.clj
@@ -9,10 +9,9 @@
:as sql.params.substitution]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.driver.sql.util.unprepare :as unprepare]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [potemkin :as p]
- [schema.core :as s]))
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
+ [potemkin :as p]))
(comment sql.params.substitution/keep-me) ; this is so `cljr-clean-ns` and the linter don't remove the `:require`
@@ -48,8 +47,8 @@
[driver query]
(sql.qp/mbql->native driver query))
-(s/defmethod driver/substitute-native-parameters :sql
- [_driver {:keys [query] :as inner-query} :- {:query su/NonBlankString, s/Keyword s/Any}]
+(mu/defmethod driver/substitute-native-parameters :sql
+ [_driver {:keys [query] :as inner-query} :- [:and [:map-of :keyword :any] [:map {:query ms/NonBlankString}]]]
(let [[query params] (-> query
params.parse/parse
(sql.params.substitute/substitute (params.values/query->params-map inner-query)))]
diff --git a/src/metabase/driver/sql_jdbc/connection.clj b/src/metabase/driver/sql_jdbc/connection.clj
index 15946b39234936ac0b8ff82d106ae625471de42b..f3c0d2742cab0056c981952ab3cbcb4bedbb62bd 100644
--- a/src/metabase/driver/sql_jdbc/connection.clj
+++ b/src/metabase/driver/sql_jdbc/connection.clj
@@ -16,10 +16,8 @@
[metabase.util :as u]
[metabase.util.i18n :refer [trs tru]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli :as mu]
[metabase.util.ssh :as ssh]
- [schema.core :as s]
#_{:clj-kondo/ignore [:discouraged-namespace]}
[toucan2.core :as t2])
(:import
@@ -187,10 +185,10 @@
database-id->jdbc-spec-hash
(atom {}))
-(s/defn ^:private jdbc-spec-hash
+(mu/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} :- (s/maybe su/Map)]
+ [{driver :engine, :keys [details], :as database} :- [:maybe :map]]
(when (some? database)
(hash (connection-details->spec driver details))))
diff --git a/src/metabase/email.clj b/src/metabase/email.clj
index e2708bb34dabee2affd415e5e9dcbdb7ed7c266b..128ff239b010d55f9547c5a7c9fc68d8e94f8084 100644
--- a/src/metabase/email.clj
+++ b/src/metabase/email.clj
@@ -1,15 +1,14 @@
(ns metabase.email
(:require
+ [malli.core :as mc]
[metabase.analytics.prometheus :as prometheus]
[metabase.models.setting :as setting :refer [defsetting]]
- [metabase.util :as u]
[metabase.util.i18n :refer [deferred-tru trs tru]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[postal.core :as postal]
- [postal.support :refer [make-props]]
- [schema.core :as s])
+ [postal.support :refer [make-props]])
(:import
(javax.mail Session)))
@@ -31,19 +30,19 @@
:visibility :settings-manager)
(def ^:private ReplyToAddresses
- (s/maybe [su/Email]))
+ [:maybe [:sequential ms/Email]])
(def ^:private ^{:arglists '([reply-to-addresses])} validate-reply-to-addresses
- (s/validator ReplyToAddresses))
+ (mc/validator ReplyToAddresses))
(defsetting email-reply-to
(deferred-tru "The email address you want the replies to go to, if different from the from address.")
:type :json
:visibility :settings-manager
:setter (fn [new-value]
- (->> new-value
- validate-reply-to-addresses
- (setting/set-value-of-type! :json :email-reply-to))))
+ (if (validate-reply-to-addresses new-value)
+ (setting/set-value-of-type! :json :email-reply-to new-value)
+ (throw (ex-info "Invalid reply-to address" {:value new-value})))))
(defsetting email-smtp-host
(deferred-tru "The address of the SMTP server that handles your emails.")
@@ -106,54 +105,56 @@
(add-ssl-settings (email-smtp-security))))
(def ^:private EmailMessage
- (s/constrained
- {:subject s/Str
- :recipients [(s/pred u/email?)]
- :message-type (s/enum :text :html :attachments)
- :message (s/cond-pre s/Str [su/Map]) ; TODO - what should this be a sequence of?
- (s/optional-key :bcc?) (s/maybe s/Bool)}
- (fn [{:keys [message-type message]}]
- (if (= message-type :attachments)
- (and (sequential? message) (every? map? message))
- (string? message)))
- (str "Bad message-type/message combo: message-type `:attachments` should have a sequence of maps as its message; "
- "other types should have a String message.")))
-
-(s/defn send-message-or-throw!
+ [:and
+ [:map {:closed true}
+ [:subject :string]
+ [:recipients [:sequential ms/Email]]
+ [:message-type [:enum :text :html :attachments]]
+ [:message [:or :string [:sequential :map]]]
+ [:bcc? {:optional true} [:maybe :boolean]]]
+ [:fn {:error/message (str "Bad message-type/message combo: message-type `:attachments` should have a sequence of maps as its message; "
+ "other types should have a String message.")}
+ (fn [{:keys [message-type message]}]
+ (if (= message-type :attachments)
+ (and (sequential? message) (every? map? message))
+ (string? message)))]])
+
+(mu/defn send-message-or-throw!
"Send an email to one or more `recipients`. Upon success, this returns the `message` that was just sent. This function
does not catch and swallow thrown exceptions, it will bubble up."
{:style/indent 0}
- [{:keys [subject recipients message-type message], :as email} :- EmailMessage]
+ [{:keys [subject recipients message-type message] :as email} :- EmailMessage]
(try
- (when-not (email-smtp-host)
- (throw (ex-info (tru "SMTP host is not set.") {:cause :smtp-host-not-set})))
- ;; Now send the email
- (let [to-type (if (:bcc? email) :bcc :to)]
- (send-email! (smtp-settings)
- (merge
- {:from (if-let [from-name (email-from-name)]
- (str from-name " <" (email-from-address) ">")
- (email-from-address))
- to-type recipients
- :subject subject
- :body (case message-type
- :attachments message
- :text message
- :html [{:type "text/html; charset=utf-8"
- :content message}])}
- (when-let [reply-to (email-reply-to)]
- {:reply-to reply-to}))))
- (catch Throwable e
- (prometheus/inc :metabase-email/message-errors)
- (throw e))
- (finally
- (prometheus/inc :metabase-email/messages))))
+ (when-not (email-smtp-host)
+ (throw (ex-info (tru "SMTP host is not set.") {:cause :smtp-host-not-set})))
+ ;; Now send the email
+ (let [to-type (if (:bcc? email) :bcc :to)]
+ (send-email! (smtp-settings)
+ (merge
+ {:from (if-let [from-name (email-from-name)]
+ (str from-name " <" (email-from-address) ">")
+ (email-from-address))
+ to-type recipients
+ :subject subject
+ :body (case message-type
+ :attachments message
+ :text message
+ :html [{:type "text/html; charset=utf-8"
+ :content message}])}
+ (when-let [reply-to (email-reply-to)]
+ {:reply-to reply-to}))))
+ (catch Throwable e
+ (prometheus/inc :metabase-email/message-errors)
+ (throw e))
+ (finally
+ (prometheus/inc :metabase-email/messages))))
(def ^:private SMTPStatus
"Schema for the response returned by various functions in [[metabase.email]]. Response will be a map with the key
`:metabase.email/error`, which will either be `nil` (indicating no error) or an instance of [[java.lang.Throwable]]
with the error."
- {::error (s/maybe Throwable)})
+ [:map {:closed true}
+ [::error [:maybe [:fn #(instance? Throwable %)]]]])
(defn send-message!
"Send an email to one or more `:recipients`. `:recipients` is a sequence of email addresses; `:message-type` must be
@@ -175,17 +176,18 @@
{::error e})))
(def ^:private SMTPSettings
- {:host su/NonBlankString
- :port su/IntGreaterThanZero
- ;; TODO -- not sure which of these other ones are actually required or not, and which are optional.
- (s/optional-key :user) (s/maybe s/Str)
- (s/optional-key :security) (s/maybe (s/enum :tls :ssl :none :starttls))
- (s/optional-key :pass) (s/maybe s/Str)
- (s/optional-key :sender) (s/maybe s/Str)
- (s/optional-key :sender-name) (s/maybe s/Str)
- (s/optional-key :reply-to) (s/maybe [s/Str])})
-
-(s/defn ^:private test-smtp-settings :- SMTPStatus
+ [:map {:closed true}
+ [:host ms/NonBlankString]
+ [:port ms/PositiveInt]
+ ;; TODO -- not sure which of these other ones are actually required or not, and which are optional.
+ [:user {:optional true} [:maybe :string]]
+ [:security {:optional true} [:maybe [:enum :tls :ssl :none :starttls]]]
+ [:pass {:optional true} [:maybe :string]]
+ [:sender {:optional true} [:maybe :string]]
+ [:sender-name {:optional true} [:maybe :string]]
+ [:reply-to {:optional true} [:maybe [:sequential ms/Email]]]])
+
+(mu/defn ^:private test-smtp-settings :- SMTPStatus
"Tests an SMTP configuration by attempting to connect and authenticate if an authenticated method is passed
in `:security`."
[{:keys [host port user pass sender security], :as details} :- SMTPSettings]
@@ -215,7 +217,7 @@
us from getting banned on Outlook.com."
500)
-(s/defn ^:private guess-smtp-security :- (s/maybe (s/enum :tls :starttls :ssl))
+(mu/defn ^:private guess-smtp-security :- [:maybe [:enum :tls :starttls :ssl]]
"Attempts to use each of the security methods in security order with the same set of credentials. This is used only
when the initial connection attempt fails, so it won't overwrite a functioning configuration. If this uses something
other than the provided method, a warning gets printed on the config page.
@@ -233,9 +235,7 @@
nil)))
email-security-order))
-(s/defn test-smtp-connection :- (s/conditional
- ::error SMTPStatus
- :else SMTPSettings)
+(mu/defn test-smtp-connection :- [:or SMTPStatus SMTPSettings]
"Test the connection to an SMTP server to determine if we can send emails. Takes in a dictionary of properties such
as:
diff --git a/src/metabase/integrations/ldap.clj b/src/metabase/integrations/ldap.clj
index 0c0abefd1fbb73a11c50675c30e923f8cb998ec5..f677b5b258c656154490ddd5853a16801cddbd25 100644
--- a/src/metabase/integrations/ldap.clj
+++ b/src/metabase/integrations/ldap.clj
@@ -4,15 +4,13 @@
[clj-ldap.client :as ldap]
[metabase.config :as config]
[metabase.integrations.ldap.default-implementation :as default-impl]
- [metabase.models.interface :as mi]
[metabase.models.setting :as setting :refer [defsetting]]
[metabase.models.user :refer [User]]
[metabase.plugins.classloader :as classloader]
[metabase.util :as u]
[metabase.util.i18n :refer [deferred-tru tru]]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s])
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms])
(:import
(com.unboundid.ldap.sdk DN LDAPConnectionPool LDAPException)))
@@ -214,7 +212,7 @@
(let [dn (if (string? user-info) user-info (:dn user-info))]
(ldap/bind? conn dn password))))
-(s/defn ldap-settings
+(defn ldap-settings
"A map of all ldap settings"
[]
{:first-name-attribute (ldap-attribute-firstname)
@@ -226,16 +224,17 @@
:group-base (ldap-group-base)
:group-mappings (ldap-group-mappings)})
-(s/defn find-user :- (s/maybe default-impl/UserInfo)
+(mu/defn find-user :- [:maybe default-impl/UserInfo]
"Get user information for the supplied username."
- ([username :- su/NonBlankString]
+ ([username :- ms/NonBlankString]
(with-ldap-connection [conn]
(find-user conn username)))
- ([ldap-connection :- LDAPConnectionPool, username :- su/NonBlankString]
+ ([ldap-connection :- (ms/InstanceOfClass LDAPConnectionPool)
+ username :- ms/NonBlankString]
(default-impl/find-user ldap-connection username (ldap-settings))))
-(s/defn fetch-or-create-user! :- #_{:clj-kondo/ignore [:deprecated-var]} (mi/InstanceOf:Schema User)
+(mu/defn fetch-or-create-user! :- (ms/InstanceOf User)
"Using the `user-info` (from [[find-user]]) get the corresponding Metabase user, creating it if necessary."
[user-info :- default-impl/UserInfo]
(default-impl/fetch-or-create-user! user-info (ldap-settings)))
diff --git a/src/metabase/integrations/ldap/default_implementation.clj b/src/metabase/integrations/ldap/default_implementation.clj
index 288f8617542bb4c127446e47415f06e4a131554f..91405ac70ef0bc005e2b02cbf5984bdbb3328dc8 100644
--- a/src/metabase/integrations/ldap/default_implementation.clj
+++ b/src/metabase/integrations/ldap/default_implementation.clj
@@ -4,14 +4,11 @@
[clj-ldap.client :as ldap]
[clojure.string :as str]
[metabase.integrations.common :as integrations.common]
- [metabase.models.interface :as mi]
[metabase.models.user :as user :refer [User]]
- [metabase.public-settings.premium-features
- :refer [defenterprise-schema]]
+ [metabase.public-settings.premium-features :refer [defenterprise-schema]]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2])
(:import
(com.unboundid.ldap.sdk DN Filter LDAPConnectionPool)))
@@ -20,27 +17,26 @@
(def UserInfo
"Schema for LDAP User info as returned by `user-info` and used as input to `fetch-or-create-user!`."
- {:dn su/NonBlankString
- :first-name (s/maybe su/NonBlankString)
- :last-name (s/maybe su/NonBlankString)
- :email su/Email
- :groups [su/NonBlankString]
- s/Keyword s/Any})
+ [:map
+ [:dn ms/NonBlankString]
+ [:first-name [:maybe ms/NonBlankString]]
+ [:last-name [:maybe ms/NonBlankString]]
+ [:email ms/Email]
+ [:groups [:maybe [:sequential ms/NonBlankString]]]])
(def LDAPSettings
"Options passed to LDAP integration implementations. These are just the various LDAP Settings from
`metabase.integrations.ldap`, packaged up as a single map so implementations don't need to fetch Setting values
directly."
- {:first-name-attribute su/NonBlankString
- :last-name-attribute su/NonBlankString
- :email-attribute su/NonBlankString
- :sync-groups? s/Bool
- :user-base su/NonBlankString
- :user-filter su/NonBlankString
- :group-base (s/maybe su/NonBlankString)
- :group-mappings (s/maybe {DN [su/IntGreaterThanZero]})
- s/Keyword s/Any})
-
+ [:map
+ [:first-name-attribute ms/NonBlankString]
+ [:last-name-attribute ms/NonBlankString]
+ [:email-attribute ms/NonBlankString]
+ [:sync-groups? :boolean]
+ [:user-base ms/NonBlankString]
+ [:user-filter ms/NonBlankString]
+ [:group-base [:maybe ms/NonBlankString]]
+ [:group-mappings [:maybe [:map-of (ms/InstanceOfClass DN) [:sequential ms/PositiveInt]]]]])
;;; --------------------------------------------------- find-user ----------------------------------------------------
@@ -50,10 +46,10 @@
(def ^:private group-membership-filter
"(member={dn})")
-(s/defn search :- (s/maybe su/Map)
+(mu/defn search :- [:maybe :map]
"Search for a LDAP user with `username`."
- [ldap-connection :- LDAPConnectionPool
- username :- su/NonBlankString
+ [ldap-connection :- (ms/InstanceOfClass LDAPConnectionPool)
+ username :- ms/NonBlankString
{:keys [user-base user-filter]} :- LDAPSettings]
(some-> (first
(ldap/search
@@ -64,23 +60,23 @@
:size-limit 1}))
u/lower-case-map-keys))
-(s/defn ^:private process-group-membership-filter :- su/NonBlankString
+(mu/defn ^:private process-group-membership-filter :- ms/NonBlankString
"Replace DN and UID placeholders with values returned by the LDAP server."
- [group-membership-filter :- su/NonBlankString
- dn :- su/NonBlankString
- uid :- (s/maybe su/NonBlankString)]
+ [group-membership-filter :- ms/NonBlankString
+ dn :- ms/NonBlankString
+ uid :- [:maybe ms/NonBlankString]]
(let [uid-string (or uid "")]
(-> group-membership-filter
(str/replace "{dn}" (Filter/encodeValue ^String dn))
(str/replace "{uid}" (Filter/encodeValue ^String uid-string)))))
-(s/defn ^:private user-groups :- (s/maybe [su/NonBlankString])
+(mu/defn ^:private user-groups :- [:maybe [:sequential ms/NonBlankString]]
"Retrieve groups for a supplied DN."
- [ldap-connection :- LDAPConnectionPool
- dn :- su/NonBlankString
- uid :- (s/maybe su/NonBlankString)
+ [ldap-connection :- (ms/InstanceOfClass LDAPConnectionPool)
+ dn :- ms/NonBlankString
+ uid :- [:maybe ms/NonBlankString]
{:keys [group-base]} :- LDAPSettings
- group-membership-filter :- su/NonBlankString]
+ group-membership-filter :- ms/NonBlankString]
(when group-base
(let [results (ldap/search
ldap-connection
@@ -89,16 +85,16 @@
:filter (process-group-membership-filter group-membership-filter dn uid)})]
(map :dn results))))
-(s/defn ldap-search-result->user-info :- (s/maybe UserInfo)
+(mu/defn ldap-search-result->user-info :- [:maybe UserInfo]
"Convert the result "
- [ldap-connection :- LDAPConnectionPool
- {:keys [dn uid], :as result} :- su/Map
+ [ldap-connection :- (ms/InstanceOfClass LDAPConnectionPool)
+ {:keys [dn uid], :as result} :- :map
{:keys [first-name-attribute
last-name-attribute
email-attribute
sync-groups?]
:as settings} :- LDAPSettings
- group-membership-filter :- su/NonBlankString]
+ group-membership-filter :- ms/NonBlankString]
(let [{first-name (keyword first-name-attribute)
last-name (keyword last-name-attribute)
email (keyword email-attribute)} result]
@@ -113,39 +109,36 @@
(user-groups ldap-connection dn uid settings group-membership-filter)
[]))}))
-(defenterprise-schema find-user :- (s/maybe UserInfo)
+(defenterprise-schema find-user :- [:maybe UserInfo]
"Get user information for the supplied username."
metabase-enterprise.enhancements.integrations.ldap
- [ldap-connection :- LDAPConnectionPool
- username :- su/NonBlankString
+ [ldap-connection :- (ms/InstanceOfClass LDAPConnectionPool)
+ username :- ms/NonBlankString
settings :- LDAPSettings]
(when-let [result (search ldap-connection username settings)]
(ldap-search-result->user-info ldap-connection result settings group-membership-filter)))
-
;;; --------------------------------------------- fetch-or-create-user! ----------------------------------------------
-(s/defn ldap-groups->mb-group-ids :- #{su/IntGreaterThanZero}
+(mu/defn ldap-groups->mb-group-ids :- [:set ms/PositiveInt]
"Translate a set of a user's group DNs to a set of MB group IDs using the configured mappings."
- [ldap-groups :- (s/maybe [su/NonBlankString])
- {:keys [group-mappings]} :- (select-keys LDAPSettings [:group-mappings s/Keyword])]
+ [ldap-groups :- [:maybe [:sequential ms/NonBlankString]]
+ {:keys [group-mappings]} :- [:select-keys LDAPSettings [:group-mappings]]]
(-> group-mappings
(select-keys (map #(DN. (str %)) ldap-groups))
vals
flatten
set))
-(s/defn all-mapped-group-ids :- #{su/IntGreaterThanZero}
+(mu/defn all-mapped-group-ids :- [:set ms/PositiveInt]
"Returns the set of all MB group IDs that have configured mappings."
- [{:keys [group-mappings]} :- (select-keys LDAPSettings [:group-mappings s/Keyword])]
+ [{:keys [group-mappings]} :- [:select-keys LDAPSettings [:group-mappings]]]
(-> group-mappings
vals
flatten
set))
-;;; for some reason the `:clj-kondo/ignore` doesn't work inside of [[defenterprise-schema]]
-#_{:clj-kondo/ignore [:deprecated-var]}
-(defenterprise-schema fetch-or-create-user! :- (mi/InstanceOf:Schema User)
+(defenterprise-schema fetch-or-create-user! :- (ms/InstanceOf User)
"Using the `user-info` (from `find-user`) get the corresponding Metabase user, creating it if necessary."
metabase-enterprise.enhancements.integrations.ldap
[{:keys [first-name last-name email groups]} :- UserInfo
diff --git a/src/metabase/integrations/slack.clj b/src/metabase/integrations/slack.clj
index 1ba24c691e06072416f1544188db285ded9b8562..01060b7b3bf7560647af29456a8e454fbcff4b0a 100644
--- a/src/metabase/integrations/slack.clj
+++ b/src/metabase/integrations/slack.clj
@@ -12,9 +12,8 @@
[metabase.util.date-2 :as u.date]
[metabase.util.i18n :refer [deferred-tru trs tru]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]))
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]))
(set! *warn-on-reflection* true)
@@ -201,9 +200,9 @@
(:channels (slack-cached-channels-and-usernames)))]
(and channel-name (contains? channel-names channel-name)))))
-(s/defn valid-token?
+(mu/defn valid-token?
"Check whether a Slack token is valid by checking if the `conversations.list` Slack api accepts it."
- [token :- su/NonBlankString]
+ [token :- ms/NonBlankString]
(try
(binding [*send-token-error-emails?* false]
(boolean (take 1 (:channels (GET "conversations.list" :limit 1, :token token)))))
@@ -282,15 +281,14 @@
(throw (ex-info message {:status-code 400}))))))
(def ^:private NonEmptyByteArray
- (s/constrained
- (Class/forName "[B")
- not-empty
- "Non-empty byte array"))
+ [:and
+ (ms/InstanceOfClass (Class/forName "[B"))
+ [:fn not-empty]])
-(s/defn join-channel!
+(mu/defn join-channel!
"Given a channel ID, calls Slack API `conversations.join` endpoint to join the channel as the Metabase Slack app.
This must be done before uploading a file to the channel, if using a Slack app integration."
- [channel-id :- su/NonBlankString]
+ [channel-id :- ms/NonBlankString]
(POST "conversations.join" {:form-params {:channel channel-id}}))
(defn- maybe-lookup-id
@@ -305,9 +303,11 @@
channel-id' (get name->id channel-id channel-id)]
channel-id'))
-(s/defn upload-file!
+(mu/defn upload-file!
"Calls Slack API `files.upload` endpoint and returns the URL of the uploaded file."
- [file :- NonEmptyByteArray, filename :- su/NonBlankString, channel-id :- su/NonBlankString]
+ [file :- NonEmptyByteArray
+ filename :- ms/NonBlankString
+ channel-id :- ms/NonBlankString]
{:pre [(slack-configured?)]}
(let [request {:multipart [{:name "file", :content file}
{:name "filename", :content filename}
@@ -326,10 +326,12 @@
(u/prog1 (get-in response [:file :url_private])
(log/debug (trs "Uploaded image") <>))))
-(s/defn post-chat-message!
+(mu/defn post-chat-message!
"Calls Slack API `chat.postMessage` endpoint and posts a message to a channel. `attachments` should be serialized
JSON."
- [channel-id :- su/NonBlankString, text-or-nil :- (s/maybe s/Str) & [attachments]]
+ [channel-id :- ms/NonBlankString
+ text-or-nil :- [:maybe :string]
+ & [attachments]]
;; TODO: it would be nice to have an emoji or icon image to use here
(POST "chat.postMessage"
{:form-params
diff --git a/src/metabase/models/bookmark.clj b/src/metabase/models/bookmark.clj
index a42d94ff12c1fef2d724438a377a5aa139276a8f..f2289934867ab66e15d873271689fa327037a2ad 100644
--- a/src/metabase/models/bookmark.clj
+++ b/src/metabase/models/bookmark.clj
@@ -8,10 +8,9 @@
[metabase.models.collection :refer [Collection]]
[metabase.models.dashboard :refer [Dashboard]]
[metabase.util.honey-sql-2 :as h2x]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[methodical.core :as methodical]
- [schema.core :as s]
[toucan2.core :as t2]))
;; Used to be the toucan1 model name defined using [[toucan.models/defmodel]], now it's a reference to the toucan2 model name.
@@ -39,16 +38,17 @@
"Shape of a bookmark returned for user. Id is a string because it is a concatenation of the model and the model's
id. This is required for the frontend entity loading system and does not refer to any particular bookmark id,
although the compound key can be inferred from it."
- {:id s/Str
- :type (s/enum "card" "collection" "dashboard")
- :item_id su/IntGreaterThanZero
- :name su/NonBlankString
- (s/optional-key :dataset) (s/maybe s/Bool)
- (s/optional-key :display) (s/maybe s/Str)
- (s/optional-key :authority_level) (s/maybe s/Str)
- (s/optional-key :description) (s/maybe s/Str)})
+ [:map {:closed true}
+ [:id :string]
+ [:type [:enum "card" "collection" "dashboard"]]
+ [:item_id ms/PositiveInt]
+ [:name ms/NonBlankString]
+ [:authority_level {:optional true} [:maybe :string]]
+ [:dataset {:optional true} [:maybe :boolean]]
+ [:description {:optional true} [:maybe :string]]
+ [:display {:optional true} [:maybe :string]]])
-(s/defn ^:private normalize-bookmark-result :- BookmarkResult
+(mu/defn ^:private normalize-bookmark-result :- BookmarkResult
"Normalizes bookmark results. Bookmarks are left joined against the card, collection, and dashboard tables, but only
points to one of them. Normalizes it so it has just the desired fields."
[result]
@@ -92,7 +92,7 @@
:from [:collection_bookmark]
:where [:= :user_id user-id]}]}))
-(s/defn bookmarks-for-user :- [BookmarkResult]
+(mu/defn bookmarks-for-user :- [:sequential BookmarkResult]
"Get all bookmarks for a user. Each bookmark will have a string id made of the model and model-id, a type, and
item_id, name, and description from the underlying bookmarked item."
[user-id]
diff --git a/src/metabase/models/collection/graph.clj b/src/metabase/models/collection/graph.clj
index fd56fa8d3be351b5a7241359a586bed5c0745e25..14c337d12ec0f2821ace8489324d160884d897d9 100644
--- a/src/metabase/models/collection/graph.clj
+++ b/src/metabase/models/collection/graph.clj
@@ -12,9 +12,8 @@
[metabase.models.permissions-group :refer [PermissionsGroup]]
[metabase.util :as u]
[metabase.util.honey-sql-2 :as h2x]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2]))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -24,17 +23,18 @@
;;; ---------------------------------------------------- Schemas -----------------------------------------------------
(def ^:private CollectionPermissions
- (s/enum :write :read :none))
+ [:enum :write :read :none])
(def ^:private GroupPermissionsGraph
"collection-id -> status"
- {(s/optional-key :root) CollectionPermissions ; when doing a delta between old graph and new graph root won't always
- su/IntGreaterThanZero CollectionPermissions}) ; be present, which is why it's *optional*
+ ; when doing a delta between old graph and new graph root won't always
+ ; be present, which is why it's *optional*
+ [:map-of [:or [:= :root] ms/PositiveInt] CollectionPermissions])
(def ^:private PermissionsGraph
- {:revision s/Int
- :groups {su/IntGreaterThanZero GroupPermissionsGraph}})
-
+ [:map {:closed true}
+ [:revision :int]
+ [:groups [:map-of ms/PositiveInt GroupPermissionsGraph]]])
;;; -------------------------------------------------- Fetch Graph ---------------------------------------------------
@@ -42,14 +42,14 @@
(into {} (for [[group-id perms] (group-by :group_id (t2/select Permissions))]
{group-id (set (map :object perms))})))
-(s/defn ^:private perms-type-for-collection :- CollectionPermissions
+(mu/defn ^:private perms-type-for-collection :- CollectionPermissions
[permissions-set collection-or-id]
(cond
(perms/set-has-full-permissions? permissions-set (perms/collection-readwrite-path collection-or-id)) :write
(perms/set-has-full-permissions? permissions-set (perms/collection-read-path collection-or-id)) :read
:else :none))
-(s/defn ^:private group-permissions-graph :- GroupPermissionsGraph
+(mu/defn ^:private group-permissions-graph :- GroupPermissionsGraph
"Return the permissions graph for a single group having `permissions-set`."
[collection-namespace permissions-set collection-ids]
(into
@@ -57,10 +57,10 @@
(for [collection-id collection-ids]
{collection-id (perms-type-for-collection permissions-set collection-id)})))
-(s/defn ^:private non-personal-collection-ids :- #{su/IntGreaterThanZero}
+(mu/defn ^:private non-personal-collection-ids :- [:set ms/PositiveInt]
"Return a set of IDs of all Collections that are neither Personal Collections nor descendants of Personal
Collections (i.e., things that you can set Permissions for, and that should go in the graph.)"
- [collection-namespace :- (s/maybe su/KeywordOrString)]
+ [collection-namespace :- [:maybe ms/KeywordOrString]]
(let [personal-collection-ids (t2/select-pks-set Collection :personal_owner_id [:not= nil])
honeysql-form {:select [[:id :id]]
:from [:collection]
@@ -82,7 +82,7 @@
(group-id->perms group-id)
collection-ids)}))})))
-(s/defn graph :- PermissionsGraph
+(mu/defn graph :- PermissionsGraph
"Fetch a graph representing the current permissions status for every group and all permissioned collections. This
works just like the function of the same name in `metabase.models.permissions`; see also the documentation for that
function.
@@ -98,7 +98,7 @@
([]
(graph nil))
- ([collection-namespace :- (s/maybe su/KeywordOrString)]
+ ([collection-namespace :- [:maybe ms/KeywordOrString]]
(t2/with-transaction [_conn]
(-> collection-namespace
non-personal-collection-ids
@@ -107,12 +107,12 @@
;;; -------------------------------------------------- Update Graph --------------------------------------------------
-(s/defn ^:private update-collection-permissions!
+(mu/defn ^:private update-collection-permissions!
"Update the permissions for group ID with `group-id` on collection with ID
`collection-id` in the optional `collection-namespace` to `new-collection-perms`."
- [collection-namespace :- (s/maybe su/KeywordOrString)
- group-id :- su/IntGreaterThanZero
- collection-id :- (s/cond-pre (s/eq :root) su/IntGreaterThanZero)
+ [collection-namespace :- [:maybe ms/KeywordOrString]
+ group-id :- ms/PositiveInt
+ collection-id :- [:or [:= :root] ms/PositiveInt]
new-collection-perms :- CollectionPermissions]
(let [collection-id (if (= collection-id :root)
(assoc collection/root-collection :namespace collection-namespace)
@@ -124,21 +124,21 @@
:read (perms/grant-collection-read-permissions! group-id collection-id)
:none nil)))
-(s/defn ^:private update-group-permissions!
- [collection-namespace :- (s/maybe su/KeywordOrString)
- group-id :- su/IntGreaterThanZero
+(mu/defn ^:private update-group-permissions!
+ [collection-namespace :- [:maybe ms/KeywordOrString]
+ group-id :- ms/PositiveInt
new-group-perms :- GroupPermissionsGraph]
(doseq [[collection-id new-perms] new-group-perms]
(update-collection-permissions! collection-namespace group-id collection-id new-perms)))
-(s/defn update-graph!
+(mu/defn update-graph!
"Update the Collections permissions graph for Collections of `collection-namespace` (default `nil`, the 'default'
namespace). This works just like [[metabase.models.permission/update-data-perms-graph!]], but for Collections;
refer to that function's extensive documentation to get a sense for how this works."
([new-graph]
(update-graph! nil new-graph))
- ([collection-namespace :- (s/maybe su/KeywordOrString), new-graph :- PermissionsGraph]
+ ([collection-namespace :- [:maybe ms/KeywordOrString], new-graph :- PermissionsGraph]
(let [old-graph (graph collection-namespace)
old-perms (:groups old-graph)
new-perms (:groups new-graph)
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 3c7367042481dc051bd6507bbda17ddb10a29675..737da5956cdc600d71b57e0ef75c44e384327fe3 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -31,10 +31,7 @@
[metabase.util.log :as log]
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
[methodical.core :as methodical]
- [schema.core :as s]
[toucan2.core :as t2]
[toucan2.realize :as t2.realize]))
@@ -473,15 +470,13 @@
dashboard))
(def ^:private ParamWithMapping
- {:name su/NonBlankString
- :id su/NonBlankString
- :mappings (s/maybe #{dashboard-card/ParamMapping})
- s/Keyword s/Any})
-
-(s/defn ^:private dashboard->resolved-params* :- (let [param-id su/NonBlankString]
- {param-id ParamWithMapping})
- [dashboard :- {(s/optional-key :parameters) (s/maybe [su/Map])
- s/Keyword s/Any}]
+ [:map
+ [:id ms/NonBlankString]
+ [:name ms/NonBlankString]
+ [:mappings [:maybe [:set dashboard-card/ParamMapping]]]])
+
+(mu/defn ^:private dashboard->resolved-params* :- [:map-of ms/NonBlankString ParamWithMapping]
+ [dashboard :- [:map [:parameters [:maybe [:sequential :map]]]]]
(let [dashboard (t2/hydrate dashboard [:ordered_cards :card])
param-key->mappings (apply
merge-with set/union
diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj
index 8a2ccfab750b68a119111e10f2519610bbb410bb..26c341254528f1954438bf56c126113d4143835a 100644
--- a/src/metabase/models/dashboard_card.clj
+++ b/src/metabase/models/dashboard_card.clj
@@ -16,10 +16,7 @@
[metabase.util.honey-sql-2 :as h2x]
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
[methodical.core :as methodical]
- [schema.core :as s]
[toucan2.core :as t2]))
(def DashboardCard
@@ -111,9 +108,9 @@
(some->> (t2/select-one-fn :action_id :model/DashboardCard :id (u/the-id dashcard-or-dashcard-id))
(action/select-action :id)))
-(s/defn retrieve-dashboard-card
+(mu/defn retrieve-dashboard-card
"Fetch a single DashboardCard by its ID value."
- [id :- su/IntGreaterThanZero]
+ [id :- ms/PositiveInt]
(-> (t2/select-one :model/DashboardCard :id id)
(t2/hydrate :series)))
@@ -159,13 +156,13 @@
(t2/insert! DashboardCardSeries card-series))))
(def ^:private DashboardCardUpdates
- {:id su/IntGreaterThanZero
- (s/optional-key :action_id) (s/maybe su/IntGreaterThanZero)
- (s/optional-key :parameter_mappings) (s/maybe [su/Map])
- (s/optional-key :visualization_settings) (s/maybe su/Map)
+ [:map
+ [:id ms/PositiveInt]
+ [:action_id {:optional true} [:maybe ms/PositiveInt]]
+ [:parameter_mappings {:optional true} [:maybe [:sequential :map]]]
+ [:visualization_settings {:optional true} [:maybe :map]]
;; series is a sequence of IDs of additional cards after the first to include as "additional serieses"
- (s/optional-key :series) (s/maybe [su/IntGreaterThanZero])
- s/Keyword s/Any})
+ [:series {:optional true} [:maybe [:sequential ms/PositiveInt]]]])
(defn- shallow-updates
"Returns the keys in `new` that have different values than the corresponding keys in `old`"
@@ -175,7 +172,7 @@
(not= v (get old k)))
new)))
-(s/defn update-dashboard-card!
+(mu/defn update-dashboard-card!
"Updates an existing DashboardCard including all DashboardCardSeries.
`old-dashboard-card` is provided to avoid an extra DB call if there are no changes.
Returns nil."
@@ -199,10 +196,12 @@
(def ParamMapping
"Schema for a parameter mapping as it would appear in the DashboardCard `:parameter_mappings` column."
- {:parameter_id su/NonBlankString
- ;; TODO -- validate `:target` as well... breaks a few tests tho so those will have to be fixed
- #_:target #_s/Any
- s/Keyword s/Any})
+ [:and
+ [:map-of :keyword :any]
+ [:map
+ ;; TODO -- validate `:target` as well... breaks a few tests tho so those will have to be fixed
+ [:parameter_id ms/NonBlankString]
+ #_[:target :any]]])
(def ^:private NewDashboardCard
;; TODO - make the rest of the options explicit instead of just allowing whatever for other keys
diff --git a/src/metabase/models/field_values.clj b/src/metabase/models/field_values.clj
index aada25458d4bdc465f268533b3a71aeb36d52e26..307436518927acf989014c455236a42071315ced 100644
--- a/src/metabase/models/field_values.clj
+++ b/src/metabase/models/field_values.clj
@@ -20,6 +20,7 @@
But they will also be automatically deleted when the Full FieldValues of the same Field got updated."
(:require
[java-time :as t]
+ [malli.core :as mc]
[medley.core :as m]
[metabase.models.interface :as mi]
[metabase.models.serialization :as serdes]
@@ -29,10 +30,8 @@
[metabase.util.date-2 :as u.date]
[metabase.util.i18n :refer [trs tru]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[methodical.core :as methodical]
- [schema.core :as s]
[toucan2.core :as t2]))
(def ^Long category-cardinality-threshold
@@ -96,7 +95,7 @@
:type mi/transform-keyword})
(defn- assert-valid-human-readable-values [{human-readable-values :human_readable_values}]
- (when (s/check (s/maybe [(s/maybe su/NonBlankString)]) human-readable-values)
+ (when-not (mc/validate [:maybe [:sequential [:maybe ms/NonBlankString]]] human-readable-values)
(throw (ex-info (tru "Invalid human-readable-values: values must be a sequence; each item must be nil or a string")
{:human-readable-values human-readable-values
:status-code 400}))))
@@ -206,17 +205,12 @@
{:field-id field-id, :status-code 404})))))
(let [{base-type :base_type
visibility-type :visibility_type
- has-field-values :has_field_values
- :as field} field-or-field-id]
- (s/check {:visibility_type su/KeywordOrString
- :base_type (s/maybe su/KeywordOrString)
- :has_field_values (s/maybe su/KeywordOrString)
- s/Keyword s/Any}
- field)
+ has-field-values :has_field_values} field-or-field-id]
(boolean
- (and (not (contains? #{:retired :sensitive :hidden :details-only} (keyword visibility-type)))
- (not (isa? (keyword base-type) :type/Temporal))
- (#{:list :auto-list} (keyword has-field-values)))))))
+ (and
+ (not (contains? #{:retired :sensitive :hidden :details-only} (keyword visibility-type)))
+ (not (isa? (keyword base-type) :type/Temporal))
+ (#{:list :auto-list} (keyword has-field-values)))))))
(defn take-by-length
"Like `take` but condition by the total length of elements.
diff --git a/src/metabase/models/params.clj b/src/metabase/models/params.clj
index 191431cca65b6c8d285bd52a53c6a92fdb53859d..ad8e48e08ed7a0fa00f2f37564e078596bad281b 100644
--- a/src/metabase/models/params.clj
+++ b/src/metabase/models/params.clj
@@ -11,6 +11,7 @@
- custom-values: see [metabase.models.params.custom-values]"
(:require
[clojure.set :as set]
+ [malli.core :as mc]
[medley.core :as m]
[metabase.db.util :as mdb.u]
[metabase.mbql.normalize :as mbql.normalize]
@@ -23,9 +24,7 @@
[metabase.util.i18n :refer [tru]]
[metabase.util.log :as log]
[metabase.util.malli :as mu]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2]))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -35,14 +34,14 @@
(defn assert-valid-parameters
"Receive a Paremeterized Object and check if its parameters is valid."
[{:keys [parameters]}]
- (when (s/check (s/maybe [su/Parameter]) parameters)
+ (when-not (mc/validate [:maybe [:sequential ms/Parameter]] parameters)
(throw (ex-info (tru ":parameters must be a sequence of maps with :id and :type keys")
{:parameters parameters}))))
(defn assert-valid-parameter-mappings
"Receive a Paremeterized Object and check if its parameters is valid."
[{:keys [parameter_mappings]}]
- (when (s/check (s/maybe [su/ParameterMapping]) parameter_mappings)
+ (when-not (mc/validate [:maybe [:sequential ms/ParameterMapping]] parameter_mappings)
(throw (ex-info (tru ":parameter_mappings must be a sequence of maps with :parameter_id and :type keys")
{:parameter_mappings parameter_mappings}))))
@@ -182,11 +181,11 @@
(update field :dimensions (partial map remove-dimension-nonpublic-columns))))
-(s/defn ^:private param-field-ids->fields
+(mu/defn ^:private param-field-ids->fields
"Get the Fields (as a map of Field ID -> Field) that shoudl be returned for hydrated `:param_fields` for a Card or
Dashboard. These only contain the minimal amount of information necessary needed to power public or embedded
parameter widgets."
- [field-ids :- (s/maybe #{su/IntGreaterThanZero})]
+ [field-ids :- [:maybe [:set ms/PositiveInt]]]
(when (seq field-ids)
(m/index-by :id (-> (t2/select Field:params-columns-only :id [:in field-ids])
(t2/hydrate :has_field_values :name_field [:dimensions :human_readable_field])
@@ -241,7 +240,7 @@
[cards]
(reduce set/union #{} (map card->template-tag-field-ids cards)))
-(s/defn dashcards->param-field-ids :- #{su/IntGreaterThanZero}
+(mu/defn dashcards->param-field-ids :- [:set ms/PositiveInt]
"Return a set of Field IDs referenced by parameters in Cards in the given `dashcards`, or `nil` if none are referenced. This
also includes IDs of Fields that are to be found in the 'implicit' parameters for SQL template tag Field filters.
`dashcards` must be hydrated with :card."
@@ -281,7 +280,7 @@
:when field]
field)))
-(s/defn card->template-tag-field-ids :- #{su/IntGreaterThanZero}
+(mu/defn card->template-tag-field-ids :- [:set ms/PositiveInt]
"Return a set of Field IDs referenced in template tag parameters in `card`. This is mostly used for determining
Fields referenced by Cards for purposes other than processing queries. Filters out `:field` clauses using names."
[card]
diff --git a/src/metabase/models/pulse_card.clj b/src/metabase/models/pulse_card.clj
index cd35a5b53bc434c49a1a511d0b55e8b547e66314..ed0ae801abaf2776427a36177171bf71cb16ae61 100644
--- a/src/metabase/models/pulse_card.clj
+++ b/src/metabase/models/pulse_card.clj
@@ -2,10 +2,9 @@
(:require
[metabase.models.serialization :as serdes]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[methodical.core :as methodical]
- [schema.core :as s]
[toucan2.core :as t2]))
(def PulseCard
@@ -35,25 +34,26 @@
(or 0)))
(def ^:private NewPulseCard
- {:card_id su/IntGreaterThanZero
- :pulse_id su/IntGreaterThanZero
- :dashboard_card_id su/IntGreaterThanZero
- (s/optional-key :position) (s/maybe su/IntGreaterThanOrEqualToZero)
- (s/optional-key :include_csv) (s/maybe s/Bool)
- (s/optional-key :include_xls) (s/maybe s/Bool)})
+ [:map {:closed true}
+ [:card_id ms/PositiveInt]
+ [:pulse_id ms/PositiveInt]
+ [:dashboard_card_id ms/PositiveInt]
+ [:position {:optional true} [:maybe ms/IntGreaterThanOrEqualToZero]]
+ [:include_csv {:optional true} [:maybe :boolean]]
+ [:include_xls {:optional true} [:maybe :boolean]]])
-(s/defn bulk-create!
+(mu/defn bulk-create!
"Creates new PulseCards, joining the given card, pulse, and dashboard card and setting appropriate defaults for other
values if they're not provided."
- [new-pulse-cards :- [NewPulseCard]]
+ [new-pulse-cards :- [:sequential NewPulseCard]]
(t2/insert! PulseCard
(for [{:keys [card_id pulse_id dashboard_card_id position include_csv include_xls]} new-pulse-cards]
{:card_id card_id
:pulse_id pulse_id
:dashboard_card_id dashboard_card_id
:position (u/or-with some? position (next-position-for pulse_id))
- :include_csv (u/or-with some? include_csv false)
- :include_xls (u/or-with some? include_xls false)})))
+ :include_csv (boolean include_csv)
+ :include_xls (boolean include_xls)})))
; ----------------------------------------------------- Serialization -------------------------------------------------
diff --git a/src/metabase/models/task_history.clj b/src/metabase/models/task_history.clj
index 26a69e1bf7a8182ab30602e02d7ae6583b9cd769..5636a3bf55e856ec7b2c2eb4c421fb3496ea0ec7 100644
--- a/src/metabase/models/task_history.clj
+++ b/src/metabase/models/task_history.clj
@@ -8,10 +8,9 @@
[metabase.util :as u]
[metabase.util.i18n :refer [trs]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[methodical.core :as methodical]
- [schema.core :as s]
[toucan2.core :as t2]))
(set! *warn-on-reflection* true)
@@ -55,10 +54,10 @@
(t2/deftransforms :model/TaskHistory
{:task_details mi/transform-json})
-(s/defn all
+(mu/defn all
"Return all TaskHistory entries, applying `limit` and `offset` if not nil"
- [limit :- (s/maybe su/IntGreaterThanZero)
- offset :- (s/maybe su/IntGreaterThanOrEqualToZero)]
+ [limit :- [:maybe ms/PositiveInt]
+ offset :- [:maybe ms/IntGreaterThanOrEqualToZero]]
(t2/select TaskHistory (merge {:order-by [[:ended_at :desc]]}
(when limit
{:limit limit})
@@ -72,9 +71,10 @@
(def ^:private TaskHistoryInfo
"Schema for `info` passed to the `with-task-history` macro."
- {:task su/NonBlankString ; task name, i.e. `send-pulses`. Conventionally lisp-cased
- (s/optional-key :db_id) (s/maybe s/Int) ; DB involved, for sync operations or other tasks where this is applicable.
- (s/optional-key :task_details) (s/maybe su/Map)}) ; additional map of details to include in the recorded row
+ [:map {:closed true}
+ [:task ms/NonBlankString] ; task name, i.e. `send-pulses`. Conventionally lisp-cased
+ [:db_id {:optional true} [:maybe :int]] ; DB involved, for sync operations or other tasks where this is applicable.
+ [:task_details {:optional true} [:maybe :map]]]) ; additional map of details to include in the recorded row
(defn- save-task-history! [start-time-ms info]
(let [end-time-ms (System/currentTimeMillis)
@@ -88,9 +88,9 @@
(catch Throwable e
(log/warn e (trs "Error saving task history"))))))
-(s/defn do-with-task-history
+(mu/defn do-with-task-history
"Impl for `with-task-history` macro; see documentation below."
- [info :- TaskHistoryInfo, f]
+ [info :- TaskHistoryInfo f]
(let [start-time-ms (System/currentTimeMillis)]
(try
(u/prog1 (f)
diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj
index 7cfc9007ba83c8e722ec1b27f9be25d62bf8964b..d127c689765be4d86c26f5503e89ab461af95962 100644
--- a/src/metabase/models/user.clj
+++ b/src/metabase/models/user.clj
@@ -25,10 +25,7 @@
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
[metabase.util.password :as u.password]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
[methodical.core :as methodical]
- [schema.core :as schema]
[toucan2.core :as t2]
[toucan2.tools.default-fields :as t2.default-fields]))
@@ -322,18 +319,6 @@
[:login_attributes {:optional true} [:maybe LoginAttributes]]
[:sso_source {:optional true} [:maybe ms/NonBlankString]]])
-(def DefaultUser
- "Standard form of a user (for consumption by the frontend and such)"
- {:id su/IntGreaterThanOrEqualToZero
- :email su/NonBlankString
- :first_name su/NonBlankString
- :last_name su/NonBlankString
- :common_name su/NonBlankString
- :last_login schema/Any
- :date_joined schema/Any
- :is_qbnewb schema/Bool
- :is_superuser schema/Bool})
-
(def ^:private Invitor
"Map with info about the admin creating the user, used in the new user notification code"
[:map
diff --git a/src/metabase/public_settings/premium_features.clj b/src/metabase/public_settings/premium_features.clj
index 2e78d9bc78f709345c1e0fd53e23421ea564690e..05723b4b18c12b3904fea807467a33328f26c510 100644
--- a/src/metabase/public_settings/premium_features.clj
+++ b/src/metabase/public_settings/premium_features.clj
@@ -4,10 +4,10 @@
[cheshire.core :as json]
[clj-http.client :as http]
[clojure.core.memoize :as memoize]
- #_:clj-kondo/ignore
- [clojure.spec.alpha :as spec]
+ [clojure.spec.alpha :as s]
[clojure.string :as str]
[environ.core :refer [env]]
+ [malli.core :as mc]
[metabase.api.common :as api]
[metabase.config :as config]
[metabase.models.setting :as setting :refer [defsetting]]
@@ -16,9 +16,7 @@
[metabase.util.i18n :refer [deferred-tru trs tru]]
[metabase.util.log :as log]
[metabase.util.malli :as mu]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as schema]
+ [metabase.util.malli.schema :as ms]
[toucan2.connection :as t2.conn]
[toucan2.core :as t2]))
@@ -88,14 +86,13 @@
(def ^:private ^:const fetch-token-status-timeout-ms (u/seconds->ms 10))
(def ^:private TokenStatus
- {:valid schema/Bool
- :status su/NonBlankString
- (schema/optional-key :error-details) (schema/maybe su/NonBlankString)
- (schema/optional-key :features) [su/NonBlankString]
- (schema/optional-key :trial) schema/Bool
- (schema/optional-key :valid_thru) su/NonBlankString ; ISO 8601 timestamp
- ;; don't explode in the future if we add more to the response! lol
- schema/Any schema/Any})
+ [:map
+ [:valid :boolean]
+ [:status ms/NonBlankString]
+ [:error-details {:optional true} [:maybe ms/NonBlankString]]
+ [:features {:optional true} [:sequential ms/NonBlankString]]
+ [:trial {:optional true} :boolean]
+ [:valid_thru {:optional true} ms/NonBlankString]]) ; ISO 8601 timestamp
(defn- fetch-token-and-parse-body
[token base-url]
@@ -105,7 +102,7 @@
:body
(json/parse-string keyword)))
-(schema/defn ^:private fetch-token-status* :- TokenStatus
+(mu/defn ^:private fetch-token-status* :- TokenStatus
"Fetch info about the validity of `token` from the MetaStore."
[token :- ValidToken]
;; attempt to query the metastore API about the status of this token. If the request doesn't complete in a
@@ -164,7 +161,7 @@
(locking lock
(f token)))))
-(schema/defn ^:private valid-token->features* :- #{su/NonBlankString}
+(mu/defn ^:private valid-token->features* :- [:set ms/NonBlankString]
[token :- ValidToken]
(let [{:keys [valid status features error-details]} (fetch-token-status token)]
;; if token isn't valid throw an Exception with the `:status` message
@@ -204,7 +201,7 @@
;; validate the new value if we're not unsetting it
(try
(when (seq new-value)
- (when (schema/check ValidToken new-value)
+ (when-not (mc/validate ValidToken new-value)
(throw (ex-info (tru "Token format is invalid.")
{:status-code 400, :error-details "Token should be 64 hexadecimal characters."})))
(valid-token->features new-value)
@@ -223,7 +220,7 @@
(log/debug e (trs "Error validating token")))
;; log every five minutes
:ttl/threshold (* 1000 60 5))]
- (schema/defn token-features :- #{su/NonBlankString}
+ (mu/defn token-features :- [:set ms/NonBlankString]
"Get the features associated with the system's premium features token."
[]
(try
@@ -492,7 +489,7 @@
`(let [ee-ns# '~(or ee-ns (ns-name *ns*))
ee-fn-name# (symbol (str ee-ns# "/" '~fn-name))
oss-or-ee-fn# ~(if schema?
- `(schema/fn ~(symbol (str fn-name)) :- ~return-schema ~@fn-tail)
+ `(mu/fn ~(symbol (str fn-name)) :- ~return-schema ~@fn-tail)
`(fn ~(symbol (str fn-name)) ~@fn-tail))]
(register-mapping! ee-fn-name# (merge ~options {~oss-or-ee oss-or-ee-fn#}))
(def
@@ -504,24 +501,24 @@
[conformed-options]
(into {} (map (comp (juxt :k :v) second) conformed-options)))
-(spec/def ::defenterprise-options
- (spec/&
- (spec/*
- (spec/alt
- :feature (spec/cat :k #{:feature} :v keyword?)
- :fallback (spec/cat :k #{:fallback} :v #(or (#{:oss} %) (symbol? %)))))
- (spec/conformer options-conformer)))
-
-(spec/def ::defenterprise-args
- (spec/cat :docstr (spec/? string?)
- :ee-ns (spec/? symbol?)
- :options (spec/? ::defenterprise-options)
- :fn-tail (spec/* any?)))
-
-(spec/def ::defenterprise-schema-args
- (spec/cat :return-schema (spec/? (spec/cat :- #{:-}
- :schema any?))
- :defenterprise-args (spec/? ::defenterprise-args)))
+(s/def ::defenterprise-options
+ (s/&
+ (s/*
+ (s/alt
+ :feature (s/cat :k #{:feature} :v keyword?)
+ :fallback (s/cat :k #{:fallback} :v #(or (#{:oss} %) (symbol? %)))))
+ (s/conformer options-conformer)))
+
+(s/def ::defenterprise-args
+ (s/cat :docstr (s/? string?)
+ :ee-ns (s/? symbol?)
+ :options (s/? ::defenterprise-options)
+ :fn-tail (s/* any?)))
+
+(s/def ::defenterprise-schema-args
+ (s/cat :return-schema (s/? (s/cat :- #{:-}
+ :schema any?))
+ :defenterprise-args (s/? ::defenterprise-args)))
(defmacro defenterprise
"Defines a function that has separate implementations between the Metabase Community Edition (aka OSS) and
@@ -552,23 +549,23 @@
(Default: `:oss`)"
[fn-name & defenterprise-args]
{:pre [(symbol? fn-name)]}
- (let [parsed-args (spec/conform ::defenterprise-args defenterprise-args)
- _ (when (spec/invalid? parsed-args)
+ (let [parsed-args (s/conform ::defenterprise-args defenterprise-args)
+ _ (when (s/invalid? parsed-args)
(throw (ex-info "Failed to parse defenterprise args"
- (spec/explain-data ::defenterprise-args parsed-args))))
+ (s/explain-data ::defenterprise-args parsed-args))))
args (assoc parsed-args :fn-name fn-name)]
`(defenterprise-impl ~args)))
(defmacro defenterprise-schema
"A version of defenterprise which allows for schemas to be defined for the args and return value. Schema syntax is
- the same as when using `schema/defn`. Otherwise identical to `defenterprise`; see the docstring of that macro for
+ the same as when using `mu/defn`. Otherwise identical to `defenterprise`; see the docstring of that macro for
usage details."
[fn-name & defenterprise-args]
{:pre [(symbol? fn-name)]}
- (let [parsed-args (spec/conform ::defenterprise-schema-args defenterprise-args)
- _ (when (spec/invalid? parsed-args)
+ (let [parsed-args (s/conform ::defenterprise-schema-args defenterprise-args)
+ _ (when (s/invalid? parsed-args)
(throw (ex-info "Failed to parse defenterprise-schema args"
- (spec/explain-data ::defenterprise-schema-args parsed-args))))
+ (s/explain-data ::defenterprise-schema-args parsed-args))))
args (-> (:defenterprise-args parsed-args)
(assoc :schema? true)
(assoc :return-schema (-> parsed-args :return-schema :schema))
diff --git a/src/metabase/query_processor/dashboard.clj b/src/metabase/query_processor/dashboard.clj
index 471101e96353a72da871dc908e90d3c4be2e2406..a41cafa40da63275771d32cc21b52eafbbea687d 100644
--- a/src/metabase/query_processor/dashboard.clj
+++ b/src/metabase/query_processor/dashboard.clj
@@ -15,9 +15,8 @@
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
#_{:clj-kondo/ignore [:discouraged-namespace]}
[toucan2.core :as t2]))
@@ -114,14 +113,14 @@
target)))
dashboard-param-id->param))
-(s/defn ^:private resolve-params-for-query :- (s/maybe [su/Map])
+(mu/defn ^:private resolve-params-for-query :- [:maybe [:sequential :map]]
"Given a sequence of parameters included in a query-processing request to run the query for a Dashboard/Card, validate
that those parameters exist and have allowed types, and merge in default values and other info from the parameter
mappings."
- [dashboard-id :- su/IntGreaterThanZero
- card-id :- su/IntGreaterThanZero
- dashcard-id :- su/IntGreaterThanZero
- request-params :- (s/maybe [su/Map])]
+ [dashboard-id :- ms/PositiveInt
+ card-id :- ms/PositiveInt
+ dashcard-id :- ms/PositiveInt
+ request-params :- [:maybe [:sequential :map]]]
(log/tracef "Resolving Dashboard %d Card %d query request parameters" dashboard-id card-id)
(let [request-params (mbql.normalize/normalize-fragment [:parameters] request-params)
;; ignore default values in request params as well. (#20516)
diff --git a/src/metabase/server/middleware/browser_cookie.clj b/src/metabase/server/middleware/browser_cookie.clj
index fd913db33fdbc725e13943b4a72b3ec00e290b59..799484a4d7d3bdd42948b423f5174515d62bc9e9 100644
--- a/src/metabase/server/middleware/browser_cookie.clj
+++ b/src/metabase/server/middleware/browser_cookie.clj
@@ -6,10 +6,9 @@
(:require
[java-time :as t]
[metabase.server.request.util :as request.u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [ring.util.response :as response]
- [schema.core :as s]))
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
+ [ring.util.response :as response]))
(set! *warn-on-reflection* true)
@@ -30,7 +29,7 @@
{:same-site :none, :secure true}
{:same-site :lax})))
-(s/defn ^:private add-browser-id-cookie [request response browser-id :- su/NonBlankString]
+(mu/defn ^:private add-browser-id-cookie [request response browser-id :- ms/NonBlankString]
(response/set-cookie response browser-id-cookie-name browser-id (cookie-options request)))
(defn ensure-browser-id-cookie
diff --git a/src/metabase/server/request/util.clj b/src/metabase/server/request/util.clj
index 7d9329238e3bee24b75bd8247fda0b7b070ba06f..2963c5efd694eeb0c90f59f5f9b033b1c583caaa 100644
--- a/src/metabase/server/request/util.clj
+++ b/src/metabase/server/request/util.clj
@@ -10,10 +10,11 @@
[metabase.util :as u]
[metabase.util.i18n :refer [trs tru]]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
- [user-agent :as user-agent]))
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
+ [user-agent :as user-agent])
+ (:import
+ (java.time ZoneId)))
(set! *warn-on-reflection* true)
@@ -91,11 +92,12 @@
(def DeviceInfo
"Schema for the device info returned by `device-info`."
- {:device_id su/NonBlankString
- :device_description su/NonBlankString
- :ip_address su/NonBlankString})
+ [:map {:closed true}
+ [:device_id ms/NonBlankString]
+ [:device_description ms/NonBlankString]
+ [:ip_address ms/NonBlankString]])
-(s/defn device-info :- DeviceInfo
+(mu/defn device-info :- DeviceInfo
"Information about the device that made this request, as recorded by the `LoginHistory` table."
[{{:strs [user-agent]} :headers, :keys [browser-id], :as request}]
(let [id (or browser-id
@@ -138,28 +140,32 @@
reason or another."
5000)
-(def ^:private IPAddress
- (s/constrained su/NonBlankString u/ip-address? "valid IP address string"))
+(def ^:private IPAddress->Info
+ [:map-of
+ [:and {:error/message "valid IP address string"}
+ ms/NonBlankString [:fn u/ip-address?]]
+ [:map {:closed true}
+ [:description ms/NonBlankString]
+ [:timezone [:maybe (ms/InstanceOfClass ZoneId)]]]])
;; TODO -- replace with something better, like built-in database once we find one that's GPL compatible
-(s/defn geocode-ip-addresses :- (s/maybe {IPAddress {:description su/NonBlankString
- :timezone (s/maybe java.time.ZoneId)}})
+(mu/defn geocode-ip-addresses :- [:maybe IPAddress->Info]
"Geocode multiple IP addresses, returning a map of IP address -> info, with each info map containing human-friendly
`:description` of the location and a `java.time.ZoneId` `:timezone`, if that information is available."
- [ip-addresses :- [s/Str]]
+ [ip-addresses :- [:maybe [:sequential :string]]]
(let [ip-addresses (set (filter u/ip-address? ip-addresses))]
(when (seq ip-addresses)
(let [url (str "https://get.geojs.io/v1/ip/geo.json?ip=" (str/join "," ip-addresses))]
(try
- (let [response (-> (http/get url {:headers {"User-Agent" config/mb-app-id-string}
- :socket-timeout gecode-ip-address-timeout-ms
- :connection-timeout gecode-ip-address-timeout-ms})
- :body
- (json/parse-string true))]
- (into {} (for [info response]
- [(:ip info) {:description (or (describe-location info)
- "Unknown location")
- :timezone (u/ignore-exceptions (some-> (:timezone info) t/zone-id))}])))
- (catch Throwable e
- (log/error e (trs "Error geocoding IP addresses") {:url url})
- nil))))))
+ (let [response (-> (http/get url {:headers {"User-Agent" config/mb-app-id-string}
+ :socket-timeout gecode-ip-address-timeout-ms
+ :connection-timeout gecode-ip-address-timeout-ms})
+ :body
+ (json/parse-string true))]
+ (into {} (for [info response]
+ [(:ip info) {:description (or (describe-location info)
+ "Unknown location")
+ :timezone (u/ignore-exceptions (some-> (:timezone info) t/zone-id))}])))
+ (catch Throwable e
+ (log/error e (trs "Error geocoding IP addresses") {:url url})
+ nil))))))
diff --git a/src/metabase/task/sync_databases.clj b/src/metabase/task/sync_databases.clj
index 87582cccfb0b06b38c553dedd5d76972597c5489..73269d6e4c85551397d314377cafe082de2c1169 100644
--- a/src/metabase/task/sync_databases.clj
+++ b/src/metabase/task/sync_databases.clj
@@ -26,7 +26,6 @@
[metabase.util.malli :as mu]
[metabase.util.malli.registry :as mr]
[metabase.util.malli.schema :as ms]
- [schema.core :as s]
[toucan2.core :as t2])
(:import
(org.quartz CronTrigger JobDetail JobKey TriggerKey)))
@@ -145,13 +144,13 @@
(mu/defn ^:private trigger-key :- (ms/InstanceOfClass TriggerKey)
"Return an appropriate string key for the trigger for `task-info` and `database-or-id`."
- ^TriggerKey [database :- (mi/InstanceOf Database)
+ ^TriggerKey [database :- (ms/InstanceOf Database)
task-info :- TaskInfo]
(triggers/key (format "metabase.task.%s.trigger.%d" (name (:key task-info)) (u/the-id database))))
(mu/defn ^:private cron-schedule :- u.cron/CronScheduleString
"Fetch the appropriate cron schedule string for `database` and `task-info`."
- [database :- (mi/InstanceOf Database)
+ [database :- (ms/InstanceOf Database)
task-info :- TaskInfo]
(get database (:db-schedule-column task-info)))
@@ -162,7 +161,7 @@
(mu/defn ^:private trigger-description :- :string
"Return an appropriate description string for a job/trigger for Database described by `task-info`."
- [database :- (mi/InstanceOf Database)
+ [database :- (ms/InstanceOf Database)
task-info :- TaskInfo]
(format "%s Database %d" (name (:key task-info)) (u/the-id database)))
@@ -178,7 +177,7 @@
(mu/defn ^:private delete-task!
"Cancel a single sync task for `database-or-id` and `task-info`."
- [database :- (mi/InstanceOf Database)
+ [database :- (ms/InstanceOf Database)
task-info :- TaskInfo]
(let [trigger-key (trigger-key database task-info)]
(log/debug (u/format-color 'red
@@ -187,7 +186,7 @@
(mu/defn unschedule-tasks-for-db!
"Cancel *all* scheduled sync and FieldValues caching tasks for `database-or-id`."
- [database :- (mi/InstanceOf Database)]
+ [database :- (ms/InstanceOf Database)]
(doseq [task [sync-analyze-task-info field-values-task-info]]
(delete-task! database task)))
@@ -206,12 +205,12 @@
(jobs/with-identity (job-key task-info))
(jobs/store-durably)))
-(s/def ^:private sync-analyze-job (job sync-analyze-task-info))
-(s/def ^:private field-values-job (job field-values-task-info))
+(def ^:private sync-analyze-job (job sync-analyze-task-info))
+(def ^:private field-values-job (job field-values-task-info))
(mu/defn ^:private trigger :- (ms/InstanceOfClass CronTrigger)
"Build a Quartz Trigger for `database` and `task-info`."
- ^CronTrigger [database :- (mi/InstanceOf Database)
+ ^CronTrigger [database :- (ms/InstanceOf Database)
task-info :- TaskInfo]
(triggers/build
(triggers/with-description (trigger-description database task-info))
@@ -231,7 +230,7 @@
;; called [[from metabase.models.database/schedule-tasks!]] from the post-insert and the pre-update
(mu/defn check-and-schedule-tasks-for-db!
"Schedule a new Quartz job for `database` and `task-info` if it doesn't already exist or is incorrect."
- [database :- (mi/InstanceOf Database)]
+ [database :- (ms/InstanceOf Database)]
(let [sync-job (task/job-info (job-key sync-analyze-task-info))
fv-job (task/job-info (job-key field-values-task-info))
diff --git a/src/metabase/util/honey_sql_1.clj b/src/metabase/util/honey_sql_1.clj
index e496f0333c0e1a1759451713380af647f707e944..c9b3264a79058457973dfb904b5eff9b3a7e3f04 100644
--- a/src/metabase/util/honey_sql_1.clj
+++ b/src/metabase/util/honey_sql_1.clj
@@ -11,8 +11,8 @@
[honeysql.format :as hformat]
[honeysql.types]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[potemkin.types :as p.types]
[pretty.core :as pretty]
[schema.core :as s])
@@ -216,20 +216,15 @@
(hformat/to-sql (literal zone)))))
(def ^{:deprecated "0.46.0"} ^:private NormalizedTypeInfo
- {(s/optional-key :metabase.util.honeysql-extensions/database-type)
- (s/constrained
- su/NonBlankString
- (fn [s]
- (= s (u/lower-case-en s)))
- "lowercased string")})
+ [:map {:closed true}
+ [:metabase.util.honeysql-extensions/database-type {:optional true} [:and ms/NonBlankString [:fn #(= % (u/lower-case-en %))]]]])
#_{:clj-kondo/ignore [:deprecated-var]}
-(s/defn ^:private normalize-type-info :- NormalizedTypeInfo
+(mu/defn ^:private normalize-type-info :- NormalizedTypeInfo
"Normalize the values in the `type-info` for a `TypedHoneySQLForm` for easy comparisons (e.g., normalize
`:metabase.util.honeysql-extensions/database-type` to a lower-case string)."
{:deprecated "0.46.0"}
[type-info]
- #_{:clj-kondo/ignore [:deprecated-var]}
(cond-> type-info
(:metabase.util.honeysql-extensions/database-type type-info)
(update :metabase.util.honeysql-extensions/database-type (comp u/lower-case-en name))))
@@ -292,14 +287,14 @@
(= form-type
(some-> db-type name u/lower-case-en)))))
-(s/defn with-database-type-info
+(mu/defn with-database-type-info
"Convenience for adding only database type information to a `honeysql-form`. Wraps `honeysql-form` and returns a
`TypedHoneySQLForm`. Passing `nil` as `database-type` will remove any existing type info.
(with-database-type-info :field \"text\")
;; -> #TypedHoneySQLForm{:form :field, :info {::hx/database-type \"text\"}}"
{:deprecated "0.46.0", :style/indent [:form]}
- [honeysql-form db-type :- (s/maybe su/KeywordOrString)]
+ [honeysql-form db-type :- [:maybe ms/KeywordOrString]]
#_{:clj-kondo/ignore [:deprecated-var]}
(if (some? db-type)
(with-type-info honeysql-form {:metabase.util.honeysql-extensions/database-type db-type})
diff --git a/src/metabase/util/malli.cljc b/src/metabase/util/malli.cljc
index 1d4f6efdfad5d3034c91896fd10e9bd69e1ede0b..d33efcaa1aea0ff6aa2c9db9b5ca9ff014214a6d 100644
--- a/src/metabase/util/malli.cljc
+++ b/src/metabase/util/malli.cljc
@@ -100,3 +100,14 @@
(macros/case
:clj `(-defmethod-clj ~multifn ~dispatch-value ~@fn-tail)
:cljs `(-defmethod-cljs ~multifn ~dispatch-value ~@fn-tail))))
+
+#?(:clj
+ (defn validate-throw
+ "Returns the value if it matches the schema, else throw an exception."
+ [schema-or-validator value]
+ (if-not ((if (fn? schema-or-validator)
+ schema-or-validator
+ (mc/validator schema-or-validator))
+ value)
+ (throw (ex-info "Value does not match schema" {:value value :schema schema-or-validator}))
+ value)))
diff --git a/src/metabase/util/malli/schema.clj b/src/metabase/util/malli/schema.clj
index 820a29bc201829885d0a13d971d81c2a6d113dfb..0238f4865f4982d7c813611fa7eeec90681d6bcb 100644
--- a/src/metabase/util/malli/schema.clj
+++ b/src/metabase/util/malli/schema.clj
@@ -1,4 +1,8 @@
(ns metabase.util.malli.schema
+ "TODO: Consider refacor this namespace by defining custom schema with [[mr/def]] instead.
+
+ For example the PositiveInt can be defined as (mr/def ::positive-int pos-int?)
+ "
(:require
[cheshire.core :as json]
[malli.core :as mc]
@@ -85,7 +89,7 @@
(def PositiveNum
"Schema representing a numeric value greater than zero. This allows floating point numbers and integers."
(mu/with-api-error-message
- pos?
+ [:and number? pos?]
(deferred-tru "value must be a number greater than zero.")))
(def KeywordOrString
diff --git a/src/metabase/util/schema.clj b/src/metabase/util/schema.clj
index f410c305aa6ae5b60d13c5ef862425c701d0a04c..cefd303ef85c6bfcd647c5a49fd0d91925fc1b6f 100644
--- a/src/metabase/util/schema.clj
+++ b/src/metabase/util/schema.clj
@@ -4,20 +4,10 @@
Schemas defined are deprecated and should be replaced with Malli schema defined in [[metabase.util.malli.schema]].
If you update schemas in this ns, please make sure you update the malli schema too. It'll help us makes the transition easier."
- (:refer-clojure :exclude [distinct])
(:require
- [cheshire.core :as json]
[clojure.string :as str]
- [clojure.walk :as walk]
- [malli.core :as mc]
- [medley.core :as m]
- [metabase.mbql.normalize :as mbql.normalize]
- [metabase.mbql.schema :as mbql.s]
[metabase.types :as types]
- [metabase.util :as u]
- [metabase.util.date-2 :as u.date]
[metabase.util.i18n :as i18n :refer [deferred-tru]]
- [metabase.util.password :as u.password]
[schema.core :as s]
[schema.macros :as s.macros]
[schema.utils :as s.utils]))
@@ -58,148 +48,12 @@
(recur (s/named schema api-error-message) api-error-message)
(assoc schema :api-error-message api-error-message)))
-(defn api-param
- "Return `schema` with an additional `api-param-name` key that will be used in the auto-generate documentation and in
- error messages. This is important for situations where you want to bind a parameter coming in to the API to
- something other than the `snake_case` key it normally comes in as:
-
- ;; BAD -- Documentation/errors will tell you `dimension-type` is wrong
- [:is {{dimension-type :type} :body}]
- {dimension-type DimensionType}
-
- ;; GOOD - Documentation/errors will mention correct param name, `type`
- [:is {{dimension-type :type} :body}]
- {dimension-type (su/api-param \"type\" DimensionType)}"
- {:style/indent 1}
- [api-param-name schema]
- {:pre [(record? schema)]}
- (assoc schema :api-param-name (name api-param-name)))
-
-(defn- existing-schema->api-error-message
- "Error messages for various schemas already defined in `schema.core`. These are used as a fallback by API param
- validation if no value for `:api-error-message` is present."
- [existing-schema]
- (cond
- (= existing-schema s/Int) (deferred-tru "value must be an integer.")
- (= existing-schema s/Str) (deferred-tru "value must be a string.")
- (= existing-schema s/Bool) (deferred-tru "value must be a boolean.")
- (instance? java.util.regex.Pattern existing-schema) (deferred-tru
- "value must be a string that matches the regex `{0}`."
- existing-schema)))
-
-(declare api-error-message)
-
-(defn- create-cond-schema-message [child-schemas]
- (str (deferred-tru "value must satisfy one of the following requirements: ")
- (str/join " " (for [[i child-schema] (m/indexed child-schemas)]
- (format "%d) %s" (inc i) (api-error-message child-schema))))))
-
-(defn api-error-message
- "Extract the API error messages attached to a schema, if any. This functionality is fairly sophisticated:
-
- (api-error-message (s/maybe (non-empty [NonBlankString])))
- ;; -> \"value may be nil, or if non-nil, value must be an array. Each value must be a non-blank string.
- The array cannot be empty.\""
-
- ([schema] (api-error-message schema 0))
- ([schema indent-depth]
- (or (:api-error-message schema)
- (existing-schema->api-error-message schema)
- ;; for schemas wrapped by an `s/maybe` we can generate a nice error message like
- ;; "value may be nil, or if non-nil, value must be ..."
- (when (instance? schema.core.Maybe schema)
- (when-let [message (api-error-message (:schema schema))]
- (deferred-tru "value may be nil, or if non-nil, {0}" message)))
-
- ;; we can do something similar for enum schemas which are also likely to be defined inline
- (when (instance? schema.core.EnumSchema schema)
- (deferred-tru "value must be one of: {0}." (str/join ", " (for [v (sort (map str (:vs schema)))]
- (str "`" v "`")))))
- ;; For cond-pre schemas we'll generate something like
- ;; value must satisfy one of the following requirements:
- ;; 1) value must be a boolean.
- ;; 2) value must be a valid boolean string ('true' or 'false').
- (when (instance? schema.core.CondPre schema)
- (create-cond-schema-message (:schemas schema)))
-
- ;; For conditional schemas we'll generate a string similar to `cond-pre` above
- (when (instance? schema.core.ConditionalSchema schema)
- (create-cond-schema-message (map second (:preds-and-schemas schema))))
-
- ;; do the same for sequences of a schema
- (when (vector? schema)
- (str (deferred-tru "value must be an array.")
- (when (= (count schema) 1)
- (when-let [message (api-error-message (first schema))]
- (str " " (deferred-tru "Each {0}" message))))))
-
- ;; Optional map keys
- (when (instance? schema.core.OptionalKey schema)
- (deferred-tru "{0} (optional)" (api-error-message (:k schema))))
-
- ;; schema map keys
- (when (instance? clojure.lang.Keyword schema)
- (name schema))
-
- ;; for maps of a schema, write out what keys and values
- ;; this keeps track of indentation because the message is very difficult to read without it.
- (when (map? schema)
- (let [spaces (str/join (repeat indent-depth " "))]
- (str (deferred-tru "value must be a map with schema: (\n{0}{1}{2}{3}{4}{5}"
- spaces
- " "
- (str/join
- (str "\n" spaces " ")
- (for [k (sort-by pr-str (keys schema))] ;; keep order of keys deterministic
- (str
- (api-error-message k (inc indent-depth))
- " : "
- (api-error-message (get schema k) (inc indent-depth)))))
- "\n"
- spaces
- ")")))))))
-
(defn non-empty
"Add an addditonal constraint to `schema` (presumably an array) that requires it to be non-empty
(i.e., it must satisfy `seq`)."
[schema]
(with-api-error-message (s/constrained schema seq "Non-empty")
- (str (api-error-message schema) " " (deferred-tru "The array cannot be empty."))))
-
-(defn empty-or-distinct?
- "True if `coll` is either empty or distinct."
- [coll]
- (if (seq coll)
- (apply distinct? coll)
- true))
-
-(defn distinct
- "Add an additional constraint to `schema` (presumably an array) that requires all elements to be distinct."
- [schema]
- (with-api-error-message (s/constrained schema empty-or-distinct? "distinct")
- (str (api-error-message schema) " " (deferred-tru "All elements must be distinct."))))
-
-(defn open-schema
- "Allow for extra keys (recursively) in a schema.
- For instance:
-
- {(s/optional-key :thing) s/Int
- (s/optional-key :sub) {(s/optional-key :key) s/Int}}
-
- can validate a map with extra keys:
-
- {:thing 3
- :extra-key 5
- :sub {:key 3 :another-extra 5}}
-
- https://github.com/plumatic/schema/issues/120"
- [m]
- (walk/prewalk (fn [x]
- (if (and (map? x) (not (record? x)))
- (assoc (dissoc x (s/find-extra-keys-schema x)) s/Any s/Any)
- x))
- m))
-
+ (deferred-tru "The array cannot be empty.")))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | USEFUL SCHEMAS |
@@ -210,12 +64,6 @@
(with-api-error-message (s/constrained s/Str (complement str/blank?) "Non-blank string")
(deferred-tru "value must be a non-blank string.")))
-(def IntGreaterThanOrEqualToZero
- "Schema representing an integer than must also be greater than or equal to zero."
- (with-api-error-message
- (s/constrained s/Int (partial <= 0) (deferred-tru "Integer greater than or equal to zero"))
- (deferred-tru "value must be an integer greater than or equal to zero.")))
-
;; TODO - rename this to `PositiveInt`?
(def IntGreaterThanZero
"Schema representing an integer than must also be greater than zero."
@@ -223,196 +71,7 @@
(s/constrained s/Int (partial < 0) (deferred-tru "Integer greater than zero"))
(deferred-tru "value must be an integer greater than zero.")))
-(def PositiveNum
- "Schema representing a numeric value greater than zero. This allows floating point numbers and integers."
- (with-api-error-message
- (s/constrained s/Num (partial < 0) (deferred-tru "Number greater than zero"))
- (deferred-tru "value must be a number greater than zero.")))
-
-(def KeywordOrString
- "Schema for something that can be either a `Keyword` or a `String`."
- (with-api-error-message (s/named (s/cond-pre s/Keyword s/Str) (deferred-tru "Keyword or string"))
- (deferred-tru "value must be a keyword or string.")))
-
-(def FieldType
- "Schema for a valid Field base or effective (data) type (does it derive from `:type/*`)?"
- (with-api-error-message (s/pred #(isa? % :type/*) (deferred-tru "Valid field type"))
- (deferred-tru "value must be a valid field type.")))
-
-(def FieldSemanticType
- "Schema for a valid Field semantic type deriving from `:Semantic/*`."
- (with-api-error-message (s/pred #(isa? % :Semantic/*)
- (deferred-tru "Valid field semantic type"))
- (deferred-tru "value must be a valid field semantic type.")))
-
-(def FieldRelationType
- "Schema for a valid Field relation type deriving from `:Relation/*`"
- (with-api-error-message (s/pred #(isa? % :Relation/*)
- (deferred-tru "Valid field relation type"))
- (deferred-tru "value must be a valid field relation type.")))
-
-(def FieldSemanticOrRelationType
- "Schema for a valid Field semantic *or* Relation type. This is currently needed because the `semantic_column` is used
- to store either the semantic type or relation type info. When this is changed in the future we can get rid of this
- schema. See #15486."
- (with-api-error-message (s/pred (fn [k]
- (or (isa? k :Semantic/*)
- (isa? k :Relation/*)))
- (deferred-tru "Valid field semantic or relation type"))
- (deferred-tru "value must be a valid field semantic or relation type.")))
-
-(def CoercionStrategy
- "Schema for a valid Field coercion strategy (does it derive from `:Coercion/*`)?"
- (with-api-error-message (s/pred #(isa? % :Coercion/*) (deferred-tru "Valid coercion strategy"))
- (deferred-tru "value must be a valid coercion strategy.")))
-
-(def FieldTypeKeywordOrString
- "Like `FieldType` (e.g. a valid derivative of `:type/*`) but allows either a keyword or a string.
- This is useful especially for validating API input or objects coming out of the DB as it is unlikely
- those values will be encoded as keywords at that point."
- (with-api-error-message (s/pred #(isa? (keyword %) :type/*) (deferred-tru "Valid field data type (keyword or string)"))
- (deferred-tru "value must be a valid field data type (keyword or string).")))
-
-(def FieldSemanticTypeKeywordOrString
- "Like `FieldSemanticType` but accepts either a keyword or string."
- (with-api-error-message (s/pred #(isa? (keyword %) :Semantic/*) (deferred-tru "Valid field semantic type (keyword or string)"))
- (deferred-tru "value must be a valid field semantic type (keyword or string).")))
-
-(def FieldSemanticOrRelationTypeKeywordOrString
- "Like `FieldSemanticOrRelationType` but accepts either a keyword or string."
- (with-api-error-message (s/pred (fn [k]
- (let [k (keyword k)]
- (or (isa? k :Semantic/*)
- (isa? k :Relation/*))))
- (deferred-tru "Valid field semantic or relation type (keyword or string)"))
- (deferred-tru "value must be a valid field semantic or relation type (keyword or string).")))
-
-(def Field
- "Schema for a valid Field for API usage."
- (with-api-error-message (s/pred
- (comp (mc/validator mbql.s/Field)
- mbql.normalize/normalize-tokens))
- (deferred-tru "value must an array with :field id-or-name and an options map")))
-
-(def CoercionStrategyKeywordOrString
- "Like `CoercionStrategy` but accepts either a keyword or string."
- (with-api-error-message (s/pred #(isa? (keyword %) :Coercion/*) (deferred-tru "Valid coercion strategy"))
- (deferred-tru "value must be a valid coercion strategy (keyword or string).")))
-
-(def EntityTypeKeywordOrString
- "Validates entity type derivatives of `:entity/*`. Allows strings or keywords"
- (with-api-error-message (s/pred #(isa? (keyword %) :entity/*) (deferred-tru "Valid entity type (keyword or string)"))
- (deferred-tru "value must be a valid entity type (keyword or string).")))
-
(def Map
"Schema for a valid map."
(with-api-error-message (s/named clojure.lang.IPersistentMap (deferred-tru "Valid map"))
(deferred-tru "value must be a map.")))
-
-(def Email
- "Schema for a valid email string."
- (with-api-error-message (s/constrained s/Str u/email? (deferred-tru "Valid email address"))
- (deferred-tru "value must be a valid email address.")))
-
-(def ValidPassword
- "Schema for a valid password of sufficient complexity which is not found on a common password list."
- (with-api-error-message (s/constrained s/Str u.password/is-valid?)
- (deferred-tru "password is too common.")))
-
-(def IntString
- "Schema for a string that can be parsed as an integer.
- Something that adheres to this schema is guaranteed to to work with `Integer/parseInt`."
- (with-api-error-message (s/constrained s/Str #(u/ignore-exceptions (Integer/parseInt %)))
- (deferred-tru "value must be a valid integer.")))
-
-(def IntStringGreaterThanZero
- "Schema for a string that can be parsed as an integer, and is greater than zero.
- Something that adheres to this schema is guaranteed to to work with `Integer/parseInt`."
- (with-api-error-message (s/constrained s/Str #(u/ignore-exceptions (< 0 (Integer/parseInt %))))
- (deferred-tru "value must be a valid integer greater than zero.")))
-
-(defn- boolean-string? ^Boolean [s]
- (boolean (when (string? s)
- (let [s (u/lower-case-en s)]
- (contains? #{"true" "false"} s)))))
-
-(def BooleanString
- "Schema for a string that is a valid representation of a boolean (either `true` or `false`).
- Something that adheres to this schema is guaranteed to work with `Boolean/parseBoolean`."
- (with-api-error-message (s/constrained s/Str boolean-string?)
- (deferred-tru "value must be a valid boolean string (''true'' or ''false'').")))
-
-(def TemporalString
- "Schema for a string that can be parsed by date2/parse."
- (with-api-error-message (s/constrained s/Str #(u/ignore-exceptions (boolean (u.date/parse %))))
- (deferred-tru "value must be a valid date string")))
-
-(def JSONString
- "Schema for a string that is valid serialized JSON."
- (with-api-error-message (s/constrained s/Str #(try
- (json/parse-string %)
- true
- (catch Throwable _
- false)))
- (deferred-tru "value must be a valid JSON string.")))
-
-(def ^:private keyword-or-non-blank-str
- (s/conditional
- string? NonBlankString
- keyword? s/Keyword))
-
-(def ValuesSourceConfig
- "Schema for valid source_options within a Parameter"
- ;; TODO: This should be tighter
- {;; for source_type = 'static-list'
- (s/optional-key :values) (s/cond-pre [s/Any])
-
- ;; for source_type = 'card'
- (s/optional-key :card_id) IntGreaterThanZero
- (s/optional-key :value_field) Field
- ;; label_field is optional
- (s/optional-key :label_field) Field})
-
-(def Parameter
- "Schema for a valid Parameter.
- We're not using [metabase.mbql.schema/Parameter] here because this Parameter is meant to be used for
- Parameters we store on dashboard/card, and it has some difference with Parameter in MBQL."
- (with-api-error-message {:id NonBlankString
- :type keyword-or-non-blank-str
- (s/optional-key :values_source_type) (s/enum "static-list" "card" nil)
- (s/optional-key :values_source_config) ValuesSourceConfig
- ;; Allow blank name and slug #15279
- (s/optional-key :slug) s/Str
- (s/optional-key :name) s/Str
- (s/optional-key :default) s/Any
- (s/optional-key :sectionId) NonBlankString
- s/Keyword s/Any}
- (deferred-tru "parameter must be a map with :id and :type keys")))
-
-(def ParameterMapping
- "Schema for a valid Parameter Mapping"
- (with-api-error-message {:parameter_id NonBlankString
- :target s/Any
- (s/optional-key :card_id) IntGreaterThanZero
- s/Keyword s/Any}
- (deferred-tru "parameter_mapping must be a map with :parameter_id and :target keys")))
-
-(def EmbeddingParams
- "Schema for a valid map of embedding params."
- (with-api-error-message (s/maybe {s/Keyword (s/enum "disabled" "enabled" "locked")})
- (deferred-tru "value must be a valid embedding params map.")))
-
-(def ValidLocale
- "Schema for a valid ISO Locale code e.g. `en` or `en-US`. Case-insensitive and allows dashes or underscores."
- (with-api-error-message (s/constrained NonBlankString i18n/available-locale?)
- (deferred-tru "String must be a valid two-letter ISO language or language-country code e.g. 'en' or 'en_US'.")))
-
-(def NanoIdString
- "Schema for a 21-character NanoID string, like \"FReCLx5hSWTBU7kjCWfuu\"."
- (with-api-error-message #"^[A-Za-z0-9_\-]{21}$"
- (deferred-tru "String must be a valid 21-character NanoID string.")))
-
-(def UUIDString
- "Schema for a UUID string"
- (with-api-error-message u/uuid-regex
- (deferred-tru "String must be a valid 21-character NanoID string.")))
diff --git a/test/metabase/actions_test.clj b/test/metabase/actions_test.clj
index 8ad4ea3b078ba98a4e7c688aae5f6838dac477d6..c2fbcf635d6a269b867f94ea8a31d110e9cf138b 100644
--- a/test/metabase/actions_test.clj
+++ b/test/metabase/actions_test.clj
@@ -10,9 +10,7 @@
[metabase.query-processor :as qp]
[metabase.test :as mt]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[schema.core :as s]
[toucan2.core :as t2])
(:import
@@ -119,9 +117,11 @@
:request-body (assoc (mt/mbql-query categories) :create-row {(format-field-name :name) "created_row"})
:expect-fn (fn [result]
;; check that we return the entire row
- (is (schema= {:created-row {(format-field-name :id) su/IntGreaterThanZero
- (format-field-name :name) su/NonBlankString}}
- result)))}
+ (is (malli= [:map {:closed true}
+ [:created-row [:map {:closed true}
+ [(format-field-name :id) ms/PositiveInt]
+ [(format-field-name :name) ms/NonBlankString]]]]
+ result)))}
{:action :row/update
:request-body (assoc (mt/mbql-query categories {:filter [:= $id 1]})
:update_row {(format-field-name :name) "updated_row"})
diff --git a/test/metabase/api/action_test.clj b/test/metabase/api/action_test.clj
index 2ad22dc941a9e544ea79df23ef62d6e471fdf6ca..f5521584e122ab1ffce869b8cf2b043007393f2f 100644
--- a/test/metabase/api/action_test.clj
+++ b/test/metabase/api/action_test.clj
@@ -7,13 +7,10 @@
[metabase.api.action :as api.action]
[metabase.models :refer [Action Card Database]]
[metabase.models.collection :as collection]
- [metabase.models.user :as user]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -25,25 +22,37 @@
(comment api.action/keep-me)
+(def ^:private DefaultUser
+ [:map {:closed true}
+ [:id ms/PositiveInt]
+ [:email ms/NonBlankString]
+ [:first_name ms/NonBlankString]
+ [:last_name ms/NonBlankString]
+ [:common_name ms/NonBlankString]
+ [:last_login :any]
+ [:date_joined :any]
+ [:is_qbnewb :boolean]
+ [:is_superuser :boolean]])
+
(def ^:private ExpectedGetQueryActionAPIResponse
"Expected schema for a query action as it should appear in the response for an API request to one of the GET endpoints."
- {:id su/IntGreaterThanOrEqualToZero
- :type (s/eq "query")
- :model_id su/IntGreaterThanOrEqualToZero
- :database_id su/IntGreaterThanOrEqualToZero
- :dataset_query {:database su/IntGreaterThanOrEqualToZero
- :type (s/eq "native")
- :native {:query s/Str
- s/Keyword s/Any}
- s/Keyword s/Any}
- :parameters s/Any
- :parameter_mappings s/Any
- :visualization_settings su/Map
- :public_uuid (s/maybe su/UUIDString)
- :made_public_by_id (s/maybe su/IntGreaterThanOrEqualToZero)
- :creator_id su/IntGreaterThanZero
- :creator user/DefaultUser
- s/Keyword s/Any})
+ [:map
+ [:id ms/PositiveInt]
+ [:type [:= "query"]]
+ [:model_id ms/PositiveInt]
+ [:database_id ms/PositiveInt]
+ [:dataset_query [:map
+ [:database ms/PositiveInt]
+ [:type [:= "native"]]
+ [:native [:map
+ [:query :string]]]]]
+ [:parameters :any]
+ [:parameter_mappings :any]
+ [:visualization_settings :map]
+ [:public_uuid [:maybe ms/UUIDString]]
+ [:made_public_by_id [:maybe ms/PositiveInt]]
+ [:creator_id ms/PositiveInt]
+ [:creator DefaultUser]])
(defn all-actions-default
[card-id]
@@ -113,7 +122,7 @@
(doseq [action response
:when (= (:type action) "query")]
(testing "Should return a query action deserialized (#23201)"
- (is (schema= ExpectedGetQueryActionAPIResponse
+ (is (malli= ExpectedGetQueryActionAPIResponse
action)))))
(testing "Should not be allowed to list actions without permission on the model"
(is (= "You don't have permissions to do that."
@@ -127,7 +136,7 @@
(doseq [action response
:when (= (:type action) "query")]
(testing "Should return a query action deserialized (#23201)"
- (is (schema= ExpectedGetQueryActionAPIResponse
+ (is (malli= ExpectedGetQueryActionAPIResponse
action))))
(testing "Does not have archived actions"
(is (not (contains? action-ids (:id archived)))))
@@ -143,7 +152,7 @@
(mt/with-actions [{:keys [action-id]} {}]
(let [action (mt/user-http-request :crowberto :get 200 (format "action/%d" action-id))]
(testing "Should return a query action deserialized (#23201)"
- (is (schema= ExpectedGetQueryActionAPIResponse
+ (is (malli= ExpectedGetQueryActionAPIResponse
action))))
(testing "Should not be allowed to get the action without permission on the model"
(is (= "You don't have permissions to do that."
diff --git a/test/metabase/api/database_test.clj b/test/metabase/api/database_test.clj
index fd6c896b0fa2d0016bd7cef656d1f68c7facb792..06eaa496dff95d9c5d7758db0a846db1f7a3286f 100644
--- a/test/metabase/api/database_test.clj
+++ b/test/metabase/api/database_test.clj
@@ -26,8 +26,7 @@
[metabase.util :as u]
[metabase.util.cron :as u.cron]
[metabase.util.i18n :refer [deferred-tru]]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[ring.util.codec :as codec]
[schema.core :as s]
[toucan2.core :as t2]
@@ -242,19 +241,20 @@
(deftest create-db-test
(testing "POST /api/database"
(testing "Check that we can create a Database"
- (is (schema= (merge
- (m/map-vals s/eq (mt/object-defaults Database))
- {:metadata_sync_schedule #"0 \d{1,2} \* \* \* \? \*"
- :cache_field_values_schedule #"0 \d{1,2} \d{1,2} \* \* \? \*"}
- {:created_at java.time.temporal.Temporal
- :engine (s/eq ::test-driver)
- :id su/IntGreaterThanZero
- :details (s/eq {:db "my_db"})
- :updated_at java.time.temporal.Temporal
- :name su/NonBlankString
- :features (s/eq (driver.u/features ::test-driver (mt/db)))
- :creator_id (s/eq (mt/user->id :crowberto))})
- (create-db-via-api!))))))
+ (is (malli= [:merge
+ (into [:map] (m/map-vals (fn [v] [:fn #(= % v)]) (mt/object-defaults Database)))
+ [:map
+ [:metadata_sync_schedule #"0 \d{1,2} \* \* \* \? \*"]
+ [:cache_field_values_schedule #"0 \d{1,2} \d{1,2} \* \* \? \*"]
+ [:created_at (ms/InstanceOfClass java.time.temporal.Temporal)]
+ [:engine [:= ::test-driver]]
+ [:id ms/PositiveInt]
+ [:details [:fn #(= % {:db "my_db"})]]
+ [:updated_at (ms/InstanceOfClass java.time.temporal.Temporal)]
+ [:name ms/NonBlankString]
+ [:features [:= (driver.u/features ::test-driver (mt/db))]]
+ [:creator_id [:= (mt/user->id :crowberto)]]]]
+ (create-db-via-api!))))))
(deftest create-db-test-2
(testing "POST /api/database"
@@ -755,16 +755,18 @@
(def ^:private SavedQuestionsDB
"Schema for the expected shape of info about the 'saved questions' virtual DB from API responses."
- {:name (s/eq "Saved Questions")
- :id (s/eq -1337)
- :features (s/eq ["basic-aggregations"])
- :is_saved_questions (s/eq true)
- :tables [{:id #"^card__\d+$"
- :db_id s/Int
- :display_name s/Str
- :moderated_status (s/enum nil "verified")
- :schema s/Str ; collection name
- :description (s/maybe s/Str)}]})
+ [:map
+ [:name [:= "Saved Questions"]]
+ [:id [:= -1337]]
+ [:is_saved_questions [:= true]]
+ [:features [:= ["basic-aggregations"]]]
+ [:tables [:sequential [:map
+ [:id #"^card__\d+$"]
+ [:db_id :int]
+ [:display_name :string]
+ [:moderated_status [:or nil? [:= "verified"]]]
+ [:schema :string] ; collection name
+ [:description [:maybe :string]]]]]])
(defn- check-tables-included [response & tables]
(let [response-tables (set (:tables response))]
@@ -792,8 +794,8 @@
(mt/user-http-request :crowberto :post 202 (format "card/%d/query" (u/the-id card)))
;; Now fetch the database list. The 'Saved Questions' DB should be last on the list
(let [response (last (:data (mt/user-http-request :crowberto :get 200 "database?saved=true&include=tables")))]
- (is (schema= SavedQuestionsDB
- response))
+ (is (malli= SavedQuestionsDB
+ response))
(check-tables-included response (virtual-table-for-card card))))))))
(deftest databases-list-include-saved-questions-tables-test-2
@@ -821,8 +823,8 @@
;; Now fetch the database list. The 'Saved Questions' DB should be last on the list. Cards should have their
;; Collection name as their Schema
(let [response (last (:data (mt/user-http-request :crowberto :get 200 "database?saved=true&include=tables")))]
- (is (schema= SavedQuestionsDB
- response))
+ (is (malli= SavedQuestionsDB
+ response))
(check-tables-included
response
(virtual-table-for-card coin-card :schema "Coins")
@@ -834,8 +836,8 @@
(mt/with-temp [Card ok-card (assoc (card-with-native-query "OK Card") :result_metadata [{:name "cam"}])
Card cambiguous-card (assoc (card-with-native-query "Cambiguous Card") :result_metadata [{:name "cam"} {:name "cam_2"}])]
(let [response (fetch-virtual-database)]
- (is (schema= SavedQuestionsDB
- response))
+ (is (malli= SavedQuestionsDB
+ response))
(check-tables-included response (virtual-table-for-card ok-card))
(check-tables-not-included response (virtual-table-for-card cambiguous-card)))))))
@@ -852,8 +854,8 @@
Card ok-card (assoc (card-with-native-query "OK Card")
:result_metadata [{:name "finches"}])]
(let [response (fetch-virtual-database)]
- (is (schema= SavedQuestionsDB
- response))
+ (is (malli= SavedQuestionsDB
+ response))
(check-tables-included response (virtual-table-for-card ok-card))
(check-tables-not-included response (virtual-table-for-card bad-card)))))))
@@ -875,8 +877,8 @@
:breakout [!month.date]))
{:result_metadata [{:name "num_toucans"}]})]
(let [response (fetch-virtual-database)]
- (is (schema= SavedQuestionsDB
- response))
+ (is (malli= SavedQuestionsDB
+ response))
(check-tables-included response (virtual-table-for-card ok-card))
(check-tables-not-included response (virtual-table-for-card bad-card)))))))
@@ -886,18 +888,8 @@
:result_metadata [{:name "age_in_bird_years"}])]
(let [response (mt/user-http-request :crowberto :get 200
(format "database/%d/metadata" lib.schema.id/saved-questions-virtual-database-id))]
- (is (schema= {:name (s/eq "Saved Questions")
- :id (s/eq -1337)
- :is_saved_questions (s/eq true)
- :features (s/eq ["basic-aggregations"])
- :tables [{:id #"^card__\d+$"
- :db_id s/Int
- :display_name s/Str
- :moderated_status (s/enum nil "verified")
- :schema s/Str ; collection name
- :description (s/maybe s/Str)
- :fields [su/Map]}]}
- response))
+ (is (malli= SavedQuestionsDB
+ response))
(check-tables-included
response
(assoc (virtual-table-for-card card)
diff --git a/test/metabase/api/dataset_test.clj b/test/metabase/api/dataset_test.clj
index 64eb8f6432fab6441f38dc05d3dab50903d2ca4c..78165dac00ffaa8ee7bc3da90b1094e289c55a60 100644
--- a/test/metabase/api/dataset_test.clj
+++ b/test/metabase/api/dataset_test.clj
@@ -25,9 +25,7 @@
[metabase.test.data.users :as test.users]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli.schema :as ms]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -127,38 +125,38 @@
:native {:query "foobar"}}
result (mt/user-http-request :rasta :post 202 "dataset" query)]
(testing "\nAPI Response"
- (is (schema= {:data (s/eq {:rows []
- :cols []})
- :row_count (s/eq 0)
- :status (s/eq "failed")
- :context (s/eq "ad-hoc")
- :error #"Syntax error in SQL statement"
- :json_query (s/eq (merge
- query-defaults
- {:database (mt/id)
- :type "native"
- :native {:query "foobar"}}))
- :database_id (s/eq (mt/id))
- :state (s/eq "42000")
- :class (s/eq "class org.h2.jdbc.JdbcSQLSyntaxErrorException")
- s/Keyword s/Any}
- result)))
+ (is (malli= [:map
+ [:data [:fn #(= % {:rows [] :cols []})]]
+ [:row_count [:= 0]]
+ [:status [:= "failed"]]
+ [:context [:= "ad-hoc"]]
+ [:error #"Syntax error in SQL statement"]
+ [:json_query [:fn #(= % (merge
+ query-defaults
+ {:database (mt/id)
+ :type "native"
+ :native {:query "foobar"}}))]]
+ [:database_id [:= (mt/id)]]
+ [:state [:= "42000"]]
+ [:class [:= "class org.h2.jdbc.JdbcSQLSyntaxErrorException"]]]
+ result)))
(testing "\nSaved QueryExecution"
- (is (schema= {:hash (Class/forName "[B")
- :id su/IntGreaterThanZero
- :result_rows (s/eq 0)
- :row_count (s/eq 0)
- :context (s/eq :ad-hoc)
- :error #"Syntax error in SQL statement"
- :database_id (s/eq (mt/id))
- :executor_id (s/eq (mt/user->id :rasta))
- :native (s/eq true)
- :pulse_id (s/eq nil)
- :card_id (s/eq nil)
- :dashboard_id (s/eq nil)
- s/Keyword s/Any}
- (most-recent-query-execution-for-query query))))))))
+ (is (malli=
+ [:map
+ [:hash (ms/InstanceOfClass (Class/forName "[B"))]
+ [:id ms/PositiveInt]
+ [:result_rows [:= 0]]
+ [:row_count [:= 0]]
+ [:context [:= :ad-hoc]]
+ [:error #"Syntax error in SQL statement"]
+ [:database_id [:= (mt/id)]]
+ [:executor_id [:= (mt/user->id :rasta)]]
+ [:native [:= true]]
+ [:pulse_id nil?]
+ [:card_id nil?]
+ [:dashboard_id nil?]]
+ (most-recent-query-execution-for-query query))))))))
(defn- test-download-response-headers
[url]
@@ -268,11 +266,11 @@
;; the Database
(perms/revoke-data-perms! (perms-group/all-users) (mt/id))
(perms/grant-permissions! (perms-group/all-users) (mt/id) "schema_that_does_not_exist")
- (is (schema= {:status (s/eq "failed")
- :error (s/eq "You do not have permissions to run this query.")
- s/Keyword s/Any}
- (mt/user-http-request :rasta :post "dataset"
- (mt/mbql-query venues {:limit 1})))))))
+ (is (malli= [:map
+ [:status [:= "failed"]]
+ [:error [:= "You do not have permissions to run this query."]]]
+ (mt/user-http-request :rasta :post "dataset"
+ (mt/mbql-query venues {:limit 1})))))))
(deftest compile-test
(testing "POST /api/dataset/native"
@@ -302,12 +300,12 @@
;; Give All Users permissions to see the `venues` Table, but not ad-hoc native perms
(perms/revoke-data-perms! (perms-group/all-users) (mt/id))
(perms/grant-permissions! (perms-group/all-users) (mt/id) "PUBLIC" (mt/id :venues))
- (is (schema= {:permissions-error? (s/eq true)
- :message (s/eq "You do not have permissions to run this query.")
- s/Any s/Any}
- (mt/user-http-request :rasta :post "dataset/native"
- (mt/mbql-query venues
- {:fields [$id $name]})))))))
+ (is (malli= [:map
+ [:permissions-error? [:= true]]
+ [:message [:= "You do not have permissions to run this query."]]]
+ (mt/user-http-request :rasta :post "dataset/native"
+ (mt/mbql-query venues
+ {:fields [$id $name]})))))))
(testing "We should be able to format the resulting SQL query if desired"
;; Note that the following was tested against all driver branches of format-sql and all results were identical.
(is (= {:query (str "SELECT\n"
diff --git a/test/metabase/api/geojson_test.clj b/test/metabase/api/geojson_test.clj
index 42b092328f9a43954b8bfa549a0fb3f2c491e4ad..877049555f4e1c8b418a5a88ba2c0c835556e401 100644
--- a/test/metabase/api/geojson_test.clj
+++ b/test/metabase/api/geojson_test.clj
@@ -7,8 +7,7 @@
[metabase.models.setting :as setting]
[metabase.test :as mt]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[ring.adapter.jetty9 :as ring-jetty]
[schema.core :as s])
(:import
@@ -39,7 +38,7 @@
:region_name nil}})
(deftest ^:parallel geojson-schema-test
- (is (s/validate @#'api.geojson/CustomGeoJSON test-custom-geojson)))
+ (is (@#'api.geojson/CustomGeoJSONValidator test-custom-geojson)))
(deftest ^:parallel validate-geojson-test
(testing "It validates URLs and files appropriately"
@@ -225,18 +224,18 @@
(is (= expected-value
(api.geojson/custom-geojson))))
(testing "Env var value SHOULD NOT come back with [[setting/writable-settings]] -- should NOT be WRITABLE"
- (is (schema= {:key (s/eq :custom-geojson)
- :value (s/eq nil)
- :is_env_setting (s/eq true)
- :env_name (s/eq "MB_CUSTOM_GEOJSON")
- :description su/NonBlankString
- :default (s/eq "Using value of env var $MB_CUSTOM_GEOJSON")
- s/Keyword s/Any}
- (some
- (fn [{setting-name :key, :as setting}]
- (when (= setting-name :custom-geojson)
- setting))
- (setting/writable-settings)))))
+ (is (malli= [:map
+ [:key [:= :custom-geojson]]
+ [:value nil?]
+ [:is_env_setting [:= true]]
+ [:env_name [:= "MB_CUSTOM_GEOJSON"]]
+ [:description ms/NonBlankString]
+ [:default [:= "Using value of env var $MB_CUSTOM_GEOJSON"]]]
+ (some
+ (fn [{setting-name :key, :as setting}]
+ (when (= setting-name :custom-geojson)
+ setting))
+ (setting/writable-settings)))))
(testing "Env var value SHOULD come back with [[setting/user-readable-values-map]] -- should be READABLE."
(is (= expected-value
(get (setting/user-readable-values-map #{:public}) :custom-geojson)))))))))))
diff --git a/test/metabase/api/native_query_snippet_test.clj b/test/metabase/api/native_query_snippet_test.clj
index eb009c4b2c4fef589db10195ecc3c1a9849c9ded..4b5a1604d621d55a02ce989e8a6c16f0cc04af58 100644
--- a/test/metabase/api/native_query_snippet_test.clj
+++ b/test/metabase/api/native_query_snippet_test.clj
@@ -9,8 +9,7 @@
[metabase.models.permissions-group :as perms-group]
[metabase.test :as mt]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[schema.core :as s]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -87,16 +86,17 @@
(try
(let [snippet-input {:name "test-snippet", :description "Just null", :content "NULL"}
snippet-from-api (mt/user-http-request user :post 200 (snippet-url) snippet-input)]
- (is (schema= {:id su/IntGreaterThanZero
- :name (s/eq "test-snippet")
- :description (s/eq "Just null")
- :content (s/eq "NULL")
- :creator_id (s/eq (mt/user->id user))
- :archived (s/eq false)
- :created_at java.time.OffsetDateTime
- :updated_at java.time.OffsetDateTime
- s/Keyword s/Any}
- snippet-from-api)))
+ (is (malli=
+ [:map
+ [:id ms/PositiveInt]
+ [:name [:= "test-snippet"]]
+ [:description [:= "Just null"]]
+ [:content [:= "NULL"]]
+ [:creator_id [:= (mt/user->id user)]]
+ [:archived [:= false]]
+ [:created_at (ms/InstanceOfClass java.time.OffsetDateTime)]
+ [:updated_at (ms/InstanceOfClass java.time.OffsetDateTime)]]
+ snippet-from-api)))
(finally
(t2/delete! NativeQuerySnippet :name "test-snippet"))))))
diff --git a/test/metabase/api/session_test.clj b/test/metabase/api/session_test.clj
index baa50501fc308d19da2e18f5994f5754a3577e78..738d197428a8ac7863caa70262192b7a33e248a8 100644
--- a/test/metabase/api/session_test.clj
+++ b/test/metabase/api/session_test.clj
@@ -20,8 +20,7 @@
[metabase.test.fixtures :as fixtures]
[metabase.test.integrations.ldap :as ldap.test]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[schema.core :as s]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -55,15 +54,15 @@
(is (schema= SessionResponse
response))
(testing "Login should record a LoginHistory item"
- (is (schema= {:id su/IntGreaterThanZero
- :timestamp java.time.OffsetDateTime
- :user_id (s/eq (mt/user->id :rasta))
- :device_id su/UUIDString
- :device_description su/NonBlankString
- :ip_address su/NonBlankString
- :active (s/eq true)
- s/Keyword s/Any}
- (t2/select-one LoginHistory :user_id (mt/user->id :rasta), :session_id (:id response)))))))
+ (is (malli= [:map
+ [:id ms/PositiveInt]
+ [:timestamp (ms/InstanceOfClass java.time.OffsetDateTime)]
+ [:user_id [:= (mt/user->id :rasta)]]
+ [:device_id ms/UUIDString]
+ [:device_description ms/NonBlankString]
+ [:ip_address ms/NonBlankString]
+ [:active [:= true]]]
+ (t2/select-one LoginHistory :user_id (mt/user->id :rasta), :session_id (:id response)))))))
(testing "Test that 'remember me' checkbox sets Max-Age attribute on session cookie"
(let [body (assoc (mt/user->credentials :rasta) :remember true)
response (mt/client-real-response :post 200 "session" body)]
@@ -212,15 +211,15 @@
(is (= nil
(t2/select-one Session :id session-id)))
(testing "LoginHistory item should still exist, but session_id should be set to nil (active = false)"
- (is (schema= {:id (s/eq login-history-id)
- :timestamp java.time.OffsetDateTime
- :user_id (s/eq (mt/user->id :rasta))
- :device_id su/UUIDString
- :device_description su/NonBlankString
- :ip_address su/NonBlankString
- :active (s/eq false)
- s/Keyword s/Any}
- (t2/select-one LoginHistory :id login-history-id))))))))
+ (is (malli= [:map
+ [:id ms/PositiveInt]
+ [:timestamp (ms/InstanceOfClass java.time.OffsetDateTime)]
+ [:user_id [:= (mt/user->id :rasta)]]
+ [:device_id ms/UUIDString]
+ [:device_description ms/NonBlankString]
+ [:ip_address ms/NonBlankString]
+ [:active [:= false]]]
+ (t2/select-one LoginHistory :id login-history-id))))))))
(deftest forgot-password-test
(reset-throttlers!)
diff --git a/test/metabase/api/setup_test.clj b/test/metabase/api/setup_test.clj
index 984e8584e39a6850d1a4aa3ee85ff6380bb98e31..35c6c4d645bee0d1e3ef96b5398c0fea3da376f5 100644
--- a/test/metabase/api/setup_test.clj
+++ b/test/metabase/api/setup_test.clj
@@ -17,8 +17,7 @@
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[methodical.core :as methodical]
[schema.core :as schema]
[toucan2.core :as t2]))
@@ -346,7 +345,7 @@
(with-redefs [setup/has-user-setup (fn [] @has-user-setup)]
(is (not (setup/has-user-setup)))
(mt/discard-setting-changes [site-name site-locale anon-tracking-enabled admin-email]
- (is (schema= {:id su/UUIDString}
+ (is (malli= [:map {:closed true} [:id ms/NonBlankString]]
(client/client :post 200 "setup" body))))
;; In the non-test context, this is 'set' iff there is one or more users, and doesn't have to be toggled
(reset! has-user-setup true)
diff --git a/test/metabase/dashboard_subscription_test.clj b/test/metabase/dashboard_subscription_test.clj
index 7421cae15f670cee8f99c5845c2c1779712d9b08..e58bf14c746e1a4afe74156c32a9276b31fbc819 100644
--- a/test/metabase/dashboard_subscription_test.clj
+++ b/test/metabase/dashboard_subscription_test.clj
@@ -144,7 +144,7 @@
:body [{"Aviary KPIs" true}
pulse.test-util/png-attachment]
:bcc? true}
- email)))
+ email)))
(defn do-with-dashboard-fixture-for-dashboard
"Impl for [[with-link-card-fixture-for-dashboard]]."
diff --git a/test/metabase/driver/sql_jdbc/native_test.clj b/test/metabase/driver/sql_jdbc/native_test.clj
index 6e92f0cbe3bb3589d6ceaf942699032f60692cd5..67f591a8d382d5eaed1c0192d7f64dc230b6d9c3 100644
--- a/test/metabase/driver/sql_jdbc/native_test.clj
+++ b/test/metabase/driver/sql_jdbc/native_test.clj
@@ -5,9 +5,7 @@
[medley.core :as m]
[metabase.query-processor :as qp]
[metabase.test.data :as data]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]))
+ [metabase.util.malli.schema :as ms]))
(deftest basic-query-test
(testing "Check that a basic query works"
@@ -65,15 +63,15 @@
(deftest malformed-sql-response-test
(testing "Check that we get proper error responses for malformed SQL"
- (is (schema= {:status (s/eq :failed)
- :class (s/eq org.h2.jdbc.JdbcSQLSyntaxErrorException)
- :error #"^Column \"ZID\" not found"
- :stacktrace [su/NonBlankString]
- :json_query {:native {:query (s/eq "SELECT ZID FROM CHECKINS LIMIT 2")}
- :type (s/eq :native)
- s/Keyword s/Any}
- s/Keyword s/Any}
- (qp/process-userland-query
- {:native {:query "SELECT ZID FROM CHECKINS LIMIT 2"}
- :type :native
- :database (data/id)})))))
+ (is (malli= [:map
+ [:status [:= :failed]]
+ [:class [:= org.h2.jdbc.JdbcSQLSyntaxErrorException]]
+ [:error #"^Column \"ZID\" not found"]
+ [:stacktrace [:sequential ms/NonBlankString]]
+ [:json_query [:map
+ [:native [:map [:query [:= "SELECT ZID FROM CHECKINS LIMIT 2"]]]]
+ [:type [:= :native]]]]]
+ (qp/process-userland-query
+ {:native {:query "SELECT ZID FROM CHECKINS LIMIT 2"}
+ :type :native
+ :database (data/id)})))))
diff --git a/test/metabase/models/collection/graph_test.clj b/test/metabase/models/collection/graph_test.clj
index 57128d37ee5ec8ac384dae7112bcb2fb7f2836ea..19ca27dfcf82e6177f7c26e4c282e65fa50fe803 100644
--- a/test/metabase/models/collection/graph_test.clj
+++ b/test/metabase/models/collection/graph_test.clj
@@ -16,8 +16,7 @@
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli.schema :as ms]
[schema.core :as s]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -339,13 +338,13 @@
(nice-graph (graph/graph :currency)))))
(testing "A CollectionPermissionGraphRevision recording the *changes* to the perms graph should be saved."
- (is (schema= {:id su/IntGreaterThanZero
- :before (s/eq (mt/obj->json->obj (assoc before :namespace nil)))
- :after (s/eq {(keyword (str group-id)) {(keyword (str default-ab)) "write"}})
- :user_id (s/eq (mt/user->id :crowberto))
- :created_at java.time.temporal.Temporal
- s/Keyword s/Any}
- (t2/select-one CollectionPermissionGraphRevision {:order-by [[:id :desc]]}))))))
+ (is (malli= [:map
+ [:id ms/PositiveInt]
+ [:before [:fn #(= % (mt/obj->json->obj (assoc before :namespace nil)))]]
+ [:after [:fn #(= % {(keyword (str group-id)) {(keyword (str default-ab)) "write"}})]]
+ [:user_id [:= (mt/user->id :crowberto)]]
+ [:created_at (ms/InstanceOfClass java.time.temporal.Temporal)]]
+ (t2/select-one CollectionPermissionGraphRevision {:order-by [[:id :desc]]}))))))
(testing "Should be able to update the graph for a non-default namespace.\n"
(let [before (graph/graph :currency)]
@@ -358,13 +357,13 @@
(nice-graph (graph/graph)))))
(testing "A CollectionPermissionGraphRevision recording the *changes* to the perms graph should be saved."
- (is (schema= {:id su/IntGreaterThanZero
- :before (s/eq (mt/obj->json->obj (assoc before :namespace "currency")))
- :after (s/eq {(keyword (str group-id)) {(keyword (str currency-a)) "write"}})
- :user_id (s/eq (mt/user->id :crowberto))
- :created_at java.time.temporal.Temporal
- s/Keyword s/Any}
- (t2/select-one CollectionPermissionGraphRevision {:order-by [[:id :desc]]}))))))
+ (is (malli= [:map
+ [:id ms/PositiveInt]
+ [:before [:fn #(= % (mt/obj->json->obj (assoc before :namespace "currency")))]]
+ [:after [:fn #(= % {(keyword (str group-id)) {(keyword (str currency-a)) "write"}})]]
+ [:user_id [:= (mt/user->id :crowberto)]]
+ [:created_at (ms/InstanceOfClass java.time.temporal.Temporal)]]
+ (t2/select-one CollectionPermissionGraphRevision {:order-by [[:id :desc]]}))))))
(testing "should be able to update permissions for the Root Collection in the default namespace via the graph"
(graph/update-graph! (assoc (graph/graph) :groups {group-id {:root :read}}))
diff --git a/test/metabase/models/login_history_test.clj b/test/metabase/models/login_history_test.clj
index 42b613cc682d17b35cea6fa7813a15b2b7f5318f..50698fcc944a3dc7c81a1af86f90a79f905efc43 100644
--- a/test/metabase/models/login_history_test.clj
+++ b/test/metabase/models/login_history_test.clj
@@ -10,9 +10,7 @@
[metabase.test :as mt]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli.schema :as ms]
[toucan2.tools.with-temp :as t2.with-temp]))
(set! *warn-on-reflection* true)
@@ -65,14 +63,16 @@
:device_id device
:timestamp #t "2021-04-02T15:52:00-07:00[US/Pacific]"}]
- (is (schema= {(s/eq email)
- [{:from su/Email
- :to (s/eq [email])
- :subject (s/eq (format "We've Noticed a New Metabase Login, %s" first-name))
- :body [(s/one {:type (s/eq "text/html; charset=utf-8")
- :content s/Str}
- "HTML body")]}]}
- @mt/inbox))
+ (is (malli= [:map-of [:= email]
+ [:sequential
+ [:map {:closed true}
+ [:from ms/Email]
+ [:to [:= [email]]]
+ [:subject [:= (format "We've Noticed a New Metabase Login, %s" first-name)]]
+ [:body [:sequential [:map
+ [:type [:= "text/html; charset=utf-8"]]
+ [:content :string]]]]]]]
+ @mt/inbox))
(let [message (-> @mt/inbox (get email) first :body first :content)
site-url (public-settings/site-url)]
(testing (format "\nMessage = %s\nsite-url = %s" (pr-str message) (pr-str site-url))
diff --git a/test/metabase/public_settings/premium_features_test.clj b/test/metabase/public_settings/premium_features_test.clj
index 8967444bc6afa13b3d5557d7983aba1e154dc4ba..b39fd6fb2e73f4cdced677ae3b49c8d21d421ddd 100644
--- a/test/metabase/public_settings/premium_features_test.clj
+++ b/test/metabase/public_settings/premium_features_test.clj
@@ -13,7 +13,6 @@
:as premium-features
:refer [defenterprise defenterprise-schema]]
[metabase.test :as mt]
- [schema.core :as s]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -183,22 +182,22 @@
(is (= "Hi rasta, you're an EE customer but not extra special."
(special-greeting-or-custom :rasta))))))))
-(defenterprise-schema greeting-with-schema :- s/Str
+(defenterprise-schema greeting-with-schema :- :string
"Returns a greeting for a user."
metabase-enterprise.util-test
- [username :- s/Keyword]
+ [username :- :keyword]
(format "Hi %s, the argument was valid" (name username)))
-(defenterprise-schema greeting-with-invalid-oss-return-schema :- s/Keyword
+(defenterprise-schema greeting-with-invalid-oss-return-schema :- :keyword
"Returns a greeting for a user. The OSS implementation has an invalid return schema"
metabase-enterprise.util-test
- [username :- s/Keyword]
+ [username :- :keyword]
(format "Hi %s, the return value was valid" (name username)))
-(defenterprise-schema greeting-with-invalid-ee-return-schema :- s/Str
+(defenterprise-schema greeting-with-invalid-ee-return-schema :- :string
"Returns a greeting for a user."
metabase-enterprise.util-test
- [username :- s/Keyword]
+ [username :- :keyword]
(format "Hi %s, the return value was valid" (name username)))
(defenterprise greeting-with-only-ee-schema
@@ -213,12 +212,12 @@
(is (= "Hi rasta, the argument was valid" (greeting-with-schema :rasta)))
(is (thrown-with-msg? clojure.lang.ExceptionInfo
- #"Input to greeting-with-schema does not match schema"
+ #"Invalid input: \[\"should be a keyword\"\]"
(greeting-with-schema "rasta"))))
(testing "Return schemas are validated for OSS implementations"
(is (thrown-with-msg? clojure.lang.ExceptionInfo
- #"Output of greeting-with-invalid-oss-return-schema does not match schema"
+ #"Invalid output: \[\"should be a keyword\"\]"
(greeting-with-invalid-oss-return-schema :rasta)))))
(when config/ee-available?
@@ -227,7 +226,7 @@
(greeting-with-schema :rasta)))
(is (thrown-with-msg? clojure.lang.ExceptionInfo
- #"Input to greeting-with-schema does not match schema"
+ #"Invalid input: \[\"should be a keyword\"\]"
(greeting-with-schema "rasta"))))
(testing "Only EE schema is validated if EE implementation is called"
@@ -236,7 +235,7 @@
(with-premium-features #{:custom-feature}
(is (thrown-with-msg? clojure.lang.ExceptionInfo
- #"Output of greeting-with-invalid-ee-return-schema does not match schema"
+ #"Invalid output: \[\"should be a keyword\"\]"
(greeting-with-invalid-ee-return-schema :rasta)))))
(testing "EE schema is not validated if OSS fallback is called"
diff --git a/test/metabase/query_processor/middleware/parameters/native_test.clj b/test/metabase/query_processor/middleware/parameters/native_test.clj
index 148cf66c44b63eda0a5d6ee7242d9544e18685fd..8a0277878941be8f0e85a593db315a4b41bf9949 100644
--- a/test/metabase/query_processor/middleware/parameters/native_test.clj
+++ b/test/metabase/query_processor/middleware/parameters/native_test.clj
@@ -5,9 +5,7 @@
[metabase.query-processor.middleware.parameters.native :as qp.native]
[metabase.test :as mt]
[metabase.util :as u]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]
+ [metabase.util.malli.schema :as ms]
[toucan2.tools.with-temp :as t2.with-temp]))
(deftest include-card-parameters-test
@@ -29,7 +27,7 @@
:card-id (u/the-id card)}}}]
(mt/with-driver :h2
(mt/with-metadata-provider (mt/id)
- (is (schema= {:native su/NonBlankString
- :params (s/eq ["G%"])
- s/Keyword s/Any}
- (qp.native/expand-inner query))))))))))
+ (is (malli= [:map
+ [:native ms/NonBlankString]
+ [:params [:= ["G%"]]]]
+ (qp.native/expand-inner query))))))))))
diff --git a/test/metabase/task_test.clj b/test/metabase/task_test.clj
index d906bd69a3aaddd952b98fd09a9bf065a393a2a4..d7a0c75a448a786b6342bc6903b9dba46ab13344 100644
--- a/test/metabase/task_test.clj
+++ b/test/metabase/task_test.clj
@@ -9,9 +9,7 @@
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.test.util :as tu]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s])
+ [metabase.util.malli.schema :as ms])
(:import
(org.quartz CronTrigger JobDetail)))
@@ -91,16 +89,23 @@
(deftest scheduler-info-test
(testing "Make sure scheduler-info doesn't explode and returns info in the general shape we expect"
(mt/with-temp-scheduler
- (is (schema= {:scheduler (su/non-empty [s/Str])
- :jobs [{:key su/NonBlankString
- :description su/NonBlankString
- :triggers [{:key su/NonBlankString
- :description su/NonBlankString
- :misfire-instruction su/NonBlankString
- :state su/NonBlankString
- s/Keyword s/Any}]
- s/Keyword s/Any}]}
- (task/scheduler-info))))))
+ (is (malli= [:map {:closed true}
+ [:scheduler [:+ :string]]
+ [:jobs [:sequential
+ [:and
+ [:map-of :keyword :any]
+ [:map
+ [:key ms/NonBlankString]
+ [:description ms/NonBlankString]
+ [:triggers [:sequential
+ [:and
+ [:map-of :keyword :any]
+ [:map
+ [:key ms/NonBlankString]
+ [:description ms/NonBlankString]
+ [:misfire-instruction ms/NonBlankString]
+ [:state ms/NonBlankString]]]]]]]]]]
+ (task/scheduler-info))))))
(deftest start-scheduler-no-op-with-env-var-test
(tu/do-with-unstarted-temp-scheduler
diff --git a/test/metabase/test/data/interface.clj b/test/metabase/test/data/interface.clj
index bc015af8dd64173a394f930825f5fa8d73f89b90..b4d32f824c95e6aeb0017dbe6e746a4f504c4adb 100644
--- a/test/metabase/test/data/interface.clj
+++ b/test/metabase/test/data/interface.clj
@@ -24,11 +24,10 @@
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
[metabase.util.log :as log]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[potemkin.types :as p.types]
[pretty.core :as pretty]
- [schema.core :as s]
[toucan2.core :as t2]))
(set! *warn-on-reflection* true)
@@ -44,44 +43,46 @@
(p.types/defrecord+ DatabaseDefinition [database-name table-definitions])
(def ^:private FieldDefinitionSchema
- {:field-name su/NonBlankString
- :base-type (s/conditional
- #(and (map? %) (contains? % :natives))
- {:natives {s/Keyword su/NonBlankString}}
- #(and (map? %) (contains? % :native))
- {:native su/NonBlankString}
- :else
- su/FieldType)
- ;; this was added pretty recently (in the 44 cycle) so it might not be supported everywhere. It should work for
- ;; drivers using `:sql/test-extensions` and [[metabase.test.data.sql/field-definition-sql]] but you might need to add
- ;; support for it elsewhere if you want to use it. It only really matters for testing things that modify test
- ;; datasets e.g. [[mt/with-actions-test-data]]
- ;; default is nullable
- (s/optional-key :not-null?) (s/maybe s/Bool)
- (s/optional-key :unique?) (s/maybe s/Bool)
- (s/optional-key :semantic-type) (s/maybe su/FieldSemanticOrRelationType)
- (s/optional-key :effective-type) (s/maybe su/FieldType)
- (s/optional-key :coercion-strategy) (s/maybe su/CoercionStrategy)
- (s/optional-key :visibility-type) (s/maybe (apply s/enum field/visibility-types))
- (s/optional-key :fk) (s/maybe su/KeywordOrString)
- (s/optional-key :field-comment) (s/maybe su/NonBlankString)})
+ [:map {:closed true}
+ [:field-name ms/NonBlankString]
+ [:base-type [:or
+ [:map {:closed true}
+ [:natives [:map-of :keyword ms/NonBlankString]]]
+ [:map {:closed true}
+ [:native ms/NonBlankString]]
+ ms/FieldType]]
+ ;; this was added pretty recently (in the 44 cycle) so it might not be supported everywhere. It should work for
+ ;; drivers using `:sql/test-extensions` and [[metabase.test.data.sql/field-definition-sql]] but you might need to add
+ ;; support for it elsewhere if you want to use it. It only really matters for testing things that modify test
+ ;; datasets e.g. [[mt/with-actions-test-data]]
+ ;; default is nullable
+ [:not-null? {:optional true} [:maybe :boolean]]
+ [:unique? {:optional true} [:maybe :boolean]]
+ [:semantic-type {:optional true} [:maybe ms/FieldSemanticOrRelationType]]
+ [:effective-type {:optional true} [:maybe ms/FieldType]]
+ [:coercion-strategy {:optional true} [:maybe ms/CoercionStrategy]]
+ [:visibility-type {:optional true} [:maybe (into [:enum] field/visibility-types)]]
+ [:fk {:optional true} [:maybe ms/KeywordOrString]]
+ [:field-comment {:optional true} [:maybe ms/NonBlankString]]])
(def ^:private ValidFieldDefinition
- (s/constrained FieldDefinitionSchema (partial instance? FieldDefinition)))
+ [:and FieldDefinitionSchema (ms/InstanceOfClass FieldDefinition)])
(def ^:private ValidTableDefinition
- (s/constrained
- {:table-name su/NonBlankString
- :field-definitions [ValidFieldDefinition]
- :rows [[s/Any]]
- (s/optional-key :table-comment) (s/maybe su/NonBlankString)}
- (partial instance? TableDefinition)))
+ [:and
+ [:map {:closed true}
+ [:table-name ms/NonBlankString]
+ [:field-definitions [:sequential ValidFieldDefinition]]
+ [:rows [:sequential [:sequential :any]]]
+ [:table-comment {:optional true} [:maybe ms/NonBlankString]]]
+ (ms/InstanceOfClass TableDefinition)])
(def ^:private ValidDatabaseDefinition
- (s/constrained
- {:database-name su/NonBlankString ; this must be unique
- :table-definitions [ValidTableDefinition]}
- (partial instance? DatabaseDefinition)))
+ [:and
+ [:map {:closed true}
+ [:database-name ms/NonBlankString] ; this must be unique
+ [:table-definitions [:sequential ValidTableDefinition]]]
+ (ms/InstanceOfClass DatabaseDefinition)])
;; TODO - this should probably be a protocol instead
(defmulti ^DatabaseDefinition get-dataset-definition
@@ -427,37 +428,40 @@
(def ^:private DatasetTableDefinition
"Schema for a Table in a test dataset defined by a `defdataset` form or in a dataset defnition EDN file."
- [(s/one su/NonBlankString "table name")
- (s/one [DatasetFieldDefinition] "fields")
- (s/one [[s/Any]] "rows")])
+ [:tuple
+ ms/NonBlankString
+ [:sequential DatasetFieldDefinition]
+ [:sequential [:sequential :any]]])
;; TODO - not sure everything below belongs in this namespace
-(s/defn ^:private dataset-field-definition :- ValidFieldDefinition
+(mu/defn ^:private dataset-field-definition :- ValidFieldDefinition
"Parse a Field definition (from a `defdatset` form or EDN file) and return a FieldDefinition instance for
comsumption by various test-data-loading methods."
[field-definition-map :- DatasetFieldDefinition]
;; if definition uses a coercion strategy they need to provide the effective-type
(map->FieldDefinition field-definition-map))
-(s/defn ^:private dataset-table-definition :- ValidTableDefinition
+(mu/defn ^:private dataset-table-definition :- ValidTableDefinition
"Parse a Table definition (from a `defdatset` form or EDN file) and return a TableDefinition instance for
comsumption by various test-data-loading methods."
([tabledef :- DatasetTableDefinition]
(apply dataset-table-definition tabledef))
- ([table-name :- su/NonBlankString, field-definition-maps, rows]
+ ([table-name :- ms/NonBlankString
+ field-definition-maps
+ rows]
(map->TableDefinition
{:table-name table-name
:rows rows
:field-definitions (mapv dataset-field-definition field-definition-maps)})))
-(s/defn dataset-definition :- ValidDatabaseDefinition
+(mu/defn dataset-definition :- ValidDatabaseDefinition
"Parse a dataset definition (from a `defdatset` form or EDN file) and return a DatabaseDefinition instance for
comsumption by various test-data-loading methods."
- [database-name :- su/NonBlankString & table-definitions]
- (s/validate
- DatabaseDefinition
+ [database-name :- ms/NonBlankString & table-definitions]
+ (mu/validate-throw
+ (ms/InstanceOfClass DatabaseDefinition)
(map->DatabaseDefinition
{:database-name database-name
:table-definitions (for [table table-definitions]
@@ -504,10 +508,10 @@
[^EDNDatasetDefinition this]
@(.def this))
-(s/defn edn-dataset-definition
+(mu/defn edn-dataset-definition
"Define a new test dataset using the definition in an EDN file in the `test/metabase/test/data/dataset_definitions/`
directory. (Filename should be `dataset-name` + `.edn`.)"
- [dataset-name :- su/NonBlankString]
+ [dataset-name :- ms/NonBlankString]
(let [get-def (delay
(let [file-contents (edn/read-string
{:eof nil, :readers {'t #'u.date/parse}}
@@ -532,10 +536,10 @@
(pretty [_]
(list `transformed-dataset-definition new-name (pretty/pretty wrapped-definition))))
-(s/defn transformed-dataset-definition
+(mu/defn transformed-dataset-definition
"Create a dataset definition that is a transformation of an some other one, seqentially applying `transform-fns` to
it. The results of `transform-fns` are cached."
- [new-name :- su/NonBlankString wrapped-definition & transform-fns :- [(s/pred fn?)]]
+ [new-name :- ms/NonBlankString wrapped-definition & transform-fns]
(let [transform-fn (apply comp (reverse transform-fns))
get-def (delay
(transform-fn
@@ -551,7 +555,7 @@
(fn [dbdef]
(apply update dbdef :table-definitions f args)))
-(s/defn transform-dataset-only-tables :- (s/pred fn?)
+(mu/defn transform-dataset-only-tables :- fn?
"Create a function for `transformed-dataset-definition` to only keep some subset of Tables from the original dataset
definition."
[& table-names]
@@ -582,34 +586,37 @@
;; TODO - maybe this should go in a different namespace
-(s/defn ^:private tabledef-with-name :- ValidTableDefinition
+(mu/defn ^:private tabledef-with-name :- ValidTableDefinition
"Return `TableDefinition` with `table-name` in `dbdef`."
- [{:keys [table-definitions]} :- DatabaseDefinition, table-name :- su/NonBlankString]
+ [{:keys [table-definitions]} :- (ms/InstanceOfClass DatabaseDefinition)
+ table-name :- ms/NonBlankString]
(some
(fn [{this-name :table-name, :as tabledef}]
(when (= table-name this-name)
tabledef))
table-definitions))
-(s/defn ^:private fielddefs-for-table-with-name :- [ValidFieldDefinition]
+(mu/defn ^:private fielddefs-for-table-with-name :- [:sequential ValidFieldDefinition]
"Return the `FieldDefinitions` associated with table with `table-name` in `dbdef`."
- [dbdef :- DatabaseDefinition, table-name :- su/NonBlankString]
+ [dbdef :- (ms/InstanceOfClass DatabaseDefinition)
+ table-name :- ms/NonBlankString]
(:field-definitions (tabledef-with-name dbdef table-name)))
-(s/defn ^:private tabledef->id->row :- {su/IntGreaterThanZero {su/NonBlankString s/Any}}
- [{:keys [field-definitions rows]} :- TableDefinition]
+(mu/defn ^:private tabledef->id->row :- [:map-of ms/PositiveInt [:map-of ms/NonBlankString :any]]
+ [{:keys [field-definitions rows]} :- (ms/InstanceOfClass TableDefinition)]
(let [field-names (map :field-name field-definitions)]
(into {} (for [[i values] (m/indexed rows)]
[(inc i) (zipmap field-names values)]))))
-(s/defn ^:private dbdef->table->id->row :- {su/NonBlankString {su/IntGreaterThanZero {su/NonBlankString s/Any}}}
+(mu/defn ^:private dbdef->table->id->row :- [:map-of ms/NonBlankString [:map-of ms/PositiveInt [:map-of ms/NonBlankString :any]]]
"Return a map of table name -> map of row ID -> map of column key -> value."
- [{:keys [table-definitions]} :- DatabaseDefinition]
+ [{:keys [table-definitions]} :- (ms/InstanceOfClass DatabaseDefinition)]
(into {} (for [{:keys [table-name] :as tabledef} table-definitions]
[table-name (tabledef->id->row tabledef)])))
-(s/defn ^:private nest-fielddefs
- [dbdef :- DatabaseDefinition, table-name :- su/NonBlankString]
+(mu/defn ^:private nest-fielddefs
+ [dbdef :- (ms/InstanceOfClass DatabaseDefinition)
+ table-name :- ms/NonBlankString]
(let [nest-fielddef (fn nest-fielddef [{:keys [fk field-name], :as fielddef}]
(if-not fk
[fielddef]
@@ -618,7 +625,9 @@
(update nested-fielddef :field-name (partial vector field-name fk))))))]
(mapcat nest-fielddef (fielddefs-for-table-with-name dbdef table-name))))
-(s/defn ^:private flatten-rows [dbdef :- DatabaseDefinition, table-name :- su/NonBlankString]
+(mu/defn ^:private flatten-rows
+ [dbdef :- (ms/InstanceOfClass DatabaseDefinition)
+ table-name :- ms/NonBlankString]
(let [nested-fielddefs (nest-fielddefs dbdef table-name)
table->id->k->v (dbdef->table->id->row dbdef)
resolve-field (fn resolve-field [table id field-name]
@@ -640,19 +649,20 @@
(str/replace #"s$" "")
(str \_ (flatten-field-name fk-dest-name))))))
-(s/defn flattened-dataset-definition
+(mu/defn flattened-dataset-definition
"Create a flattened version of `dbdef` by following resolving all FKs and flattening all rows into the table with
`table-name`. For use with timeseries databases like Druid."
- [dataset-definition, table-name :- su/NonBlankString]
+ [dataset-definition
+ table-name :- ms/NonBlankString]
(transformed-dataset-definition table-name dataset-definition
(fn [dbdef]
(assoc dbdef
- :table-definitions
- [(map->TableDefinition
- {:table-name table-name
- :field-definitions (for [fielddef (nest-fielddefs dbdef table-name)]
- (update fielddef :field-name flatten-field-name))
- :rows (flatten-rows dbdef table-name)})]))))
+ :table-definitions
+ [(map->TableDefinition
+ {:table-name table-name
+ :field-definitions (for [fielddef (nest-fielddefs dbdef table-name)]
+ (update fielddef :field-name flatten-field-name))
+ :rows (flatten-rows dbdef table-name)})]))))
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/test/metabase/util/malli/schema_test.clj b/test/metabase/util/malli/schema_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..4fba0bd9b9ca42df48fb9f8109161b5dd71748ad
--- /dev/null
+++ b/test/metabase/util/malli/schema_test.clj
@@ -0,0 +1,119 @@
+(ns metabase.util.malli.schema-test
+ (:require
+ [clojure.test :refer :all]
+ [malli.core :as mc]
+ [metabase.util.malli.schema :as ms]))
+
+(deftest ^:parallel schema-test
+ (doseq [{:keys [schema failed-cases success-cases]}
+ [{:schema ms/NonBlankString
+ :failed-cases ["" 1]
+ :success-cases ["a thing"]}
+ {:schema ms/IntGreaterThanOrEqualToZero
+ :failed-cases ["1" -1 1.5]
+ :success-cases [0 1]}
+ {:schema ms/PositiveInt
+ :failed-cases ["1" 0 1.5]
+ :success-cases [1 2]}
+ {:schema ms/PositiveNum
+ :failed-cases [0 "1"]
+ :success-cases [1.5 2]}
+ {:schema ms/KeywordOrString
+ :failed-cases [1 [1] {:a 1}]
+ :success-cases [:a "a"]}
+ {:schema ms/FieldType
+ :failed-cases [:type/invalid :Semantic/*]
+ :success-cases [:type/Float]}
+ {:schema ms/FieldSemanticType
+ :failed-cases [:Semantic/invalid :type/Float]
+ :success-cases [:type/Category]}
+ {:schema ms/FieldRelationType
+ :failed-cases [:Relation/invalid :type/Category :type/Float]
+ :success-cases [:type/FK]}
+ {:schema ms/FieldSemanticOrRelationType
+ :failed-cases [:Relation/invalid :type/Float]
+ :success-cases [:type/FK :type/Category]}
+ {:schema ms/CoercionStrategy
+ :failed-cases [:type/Category :type/Float]
+ :success-cases [:Coercion/ISO8601->Date]}
+ {:schema ms/FieldTypeKeywordOrString
+ :failed-cases [1 :type/FK]
+ :success-cases [:type/Float "type/Float"]}
+ {:schema ms/FieldSemanticTypeKeywordOrString
+ :failed-cases [1 :type/FK]
+ :success-cases [:type/Category "type/Category"]}
+ {:schema ms/Field
+ :failed-cases [[:aggregation 0] [:field "name" {}]]
+ :success-cases [[:field 3 nil] ["field" "name" {:base-type :type/Float}]]}
+ {:schema ms/Map
+ :failed-cases [[] 1 "a"]
+ :success-cases [{} {:a :b}]}
+ {:schema ms/Email
+ :failed-cases ["abc.com" 1]
+ :success-cases ["ngoc@metabase.com"]}
+ {:schema ms/ValidPassword
+ :failed-cases ["abc.com" 1 "PASSW0RD"]
+ :success-cases ["unc0mmonpw"]}
+ {:schema ms/IntString
+ :failed-cases [:a "a" "1.5"]
+ :success-cases ["1"]}
+ {:schema ms/BooleanString
+ :failed-cases [:false :true true "f" "t"]
+ :success-cases ["true" "false"]}
+ {:schema ms/TemporalString
+ :failed-cases ["random string"]
+ :success-cases ["2019-10-28T13:14:15" "2019-10-28"]}
+ {:schema ms/JSONString
+ :failed-cases ["string"]
+ :success-cases ["{\"a\": 1}"]}
+ {:schema ms/EmbeddingParams
+ :failed-cases [{:key "value"}]
+ :success-cases [{:key "disabled"}]}
+ {:schema ms/ValidLocale
+ :failed-cases ["locale"]
+ :success-cases ["en" "es"]}
+ {:schema ms/NanoIdString
+ :failed-cases ["random"]
+ :success-cases ["FReCLx5hSWTBU7kjCWfuu"]}
+ {:schema ms/UUIDString
+ :failed-cases ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]
+ :success-cases ["84a51d43-2d29-4c2c-8484-e51eb5af2ca4"]}
+ {:schema ms/Parameter
+ :failed-cases [{:id "param-id"
+ :name "param-name"}
+ {:id "param-id"
+ :type "number"
+ :values_source_type "invalid-type"
+ :values_source_config {:values [[1 2 3]]}}
+ {:id "param-id"
+ :type "number"
+ :values_source_type "card"
+ :values_source_config {:card_id 3
+ :value_field [:aggregation 0]}}]
+ :success-cases [{:id "param-id"
+ :type "number"
+ :values_source_type "card"
+ :values_source_config {:card_id 3
+ :value_field [:field 3 nil]
+ :label_field [:field "name" {:base-type :type/Float}]}}
+ {:id "param-id"
+ :type "number"
+ :values_source_type "static-list"
+ :values_source_config {:values [[1 2 3]]}}]}
+ {:schema ms/ParameterMapping
+ :failed-cases [{:parameter_id "param-id"}
+ {:parameter_id "param-id"
+ :target [:field 3 nil]
+ :card_id "a"}]
+ :success-cases [{:parameter_id "param-id"
+ :target [:field 3 nil]
+ :card_id 3}]}]]
+
+ (testing (format "schema %s" (pr-str schema))
+ (doseq [case failed-cases]
+ (testing (format "case: %s should fail" (pr-str case))
+ (is (false? (mc/validate schema case)))))
+
+ (doseq [case success-cases]
+ (testing (format "case: %s should success" (pr-str case))
+ (is (true? (mc/validate schema case))))))))
diff --git a/test/metabase/util/malli_test.clj b/test/metabase/util/malli_test.clj
index 11547bae0c5388fea1491f24e029806583d95064..2be87ea2e4240cd10dcb3fa8f342d1dca0c63554 100644
--- a/test/metabase/util/malli_test.clj
+++ b/test/metabase/util/malli_test.clj
@@ -53,3 +53,12 @@
(is (= "map where {:ltf-key -> <Special Number that has to be less than four>}"
(umd/describe special-lt-4-schema)))))))
+
+(deftest validate-throw-test
+ (testing "with a schema"
+ (is (= {:a 1 :b "b"} (mu/validate-throw [:map [:a :int] [:b :string]] {:a 1 :b "b"})))
+ (is (thrown-with-msg? Exception #"Value does not match schema" (mu/validate-throw [:map [:a :int] [:b :string]] "1"))))
+ (let [map-validator (mc/validator [:map [:a :int] [:b :string]])]
+ (testing "with a schema"
+ (is (= {:a 1 :b "b"} (mu/validate-throw map-validator {:a 1 :b "b"}))
+ (is (thrown-with-msg? Exception #"Value does not match schema" (mu/validate-throw map-validator "1")))))))
diff --git a/test/metabase/util/schema_test.clj b/test/metabase/util/schema_test.clj
deleted file mode 100644
index 046f7dd8b42b9ceeeb949d926d3be9a85362b69d..0000000000000000000000000000000000000000
--- a/test/metabase/util/schema_test.clj
+++ /dev/null
@@ -1,257 +0,0 @@
-(ns ^:mb/once metabase.util.schema-test
- "Tests for utility schemas and various API helper functions."
- (:require
- [clojure.string :as str]
- [clojure.test :refer :all]
- [malli.core :as mc]
- [metabase.test :as mt]
- [metabase.util :as u]
- [metabase.util.malli.schema :as ms]
- #_{:clj-kondo/ignore [:deprecated-namespace]}
- [metabase.util.schema :as su]
- [schema.core :as s]))
-
-(set! *warn-on-reflection* true)
-
-(deftest ^:parallel generate-api-error-message-test
- (testing "check that the API error message generation is working as intended"
- (is (= (str "value may be nil, or if non-nil, value must satisfy one of the following requirements: "
- "1) value must be a boolean. "
- "2) value must be a valid boolean string ('true' or 'false').")
- (str (su/api-error-message (s/maybe (s/cond-pre s/Bool su/BooleanString))))))
- (is (= (str/join "\n"
- ["value must be a map with schema: ("
- " a : value must be a map with schema: ("
- " b : value must be a map with schema: ("
- " c : value must be a map with schema: ("
- " d : value must be a map with schema: ("
- " optional-key (optional) : value must be an integer."
- " key : value may be nil, or if non-nil, value must be a boolean."
- " )"
- " )"
- " )"
- " )"
- ")"])
- (str (su/api-error-message
- {:a {:b {:c {:d {:key (s/maybe s/Bool)
- (s/optional-key :optional-key) s/Int}}}}}))))))
-
-(defn- ex-info-msg [f]
- (try
- (f)
- (catch clojure.lang.ExceptionInfo e
- (.getMessage e))))
-
-(deftest translate-exception-message-test
- (mt/with-mock-i18n-bundles {"zz" {:messages {"Integer greater than zero" "INTEGER GREATER THAN ZERO"}}}
- (is (re= #".*Integer greater than zero.*"
- (ex-info-msg #(s/validate su/IntGreaterThanZero -1))))
- (mt/with-user-locale "zz"
- (is (re= #".*INTEGER GREATER THAN ZERO.*"
- (ex-info-msg #(s/validate su/IntGreaterThanZero -1)))))))
-
-(deftest ^:parallel distinct-test
- (is (= nil
- (s/check (su/distinct [s/Int]) [])))
- (is (= nil
- (s/check (su/distinct [s/Int]) [1])))
- (is (= nil
- (s/check (su/distinct [s/Int]) [1 2])))
- (is (some? (s/check (su/distinct [s/Int]) [1 2 1]))))
-
-(deftest ^:parallel open-schema-test
- (let [value {:thing 3
- :extra-key 5
- :sub {:key 3 :another-extra 5}}
- schema {(s/optional-key :thing) s/Int
- (s/optional-key :sub) {(s/optional-key :key) s/Int}}]
- (is (= {:sub {:another-extra 'disallowed-key}, :extra-key 'disallowed-key}
- (s/check schema value)))
- (is (nil? (s/check (su/open-schema schema) value))))
- (testing "handles if there are already s/Any's"
- (let [value {:thing 3
- :extra-key 5
- :sub {:key 3 :another-extra 5}}
- schema {(s/optional-key :thing) s/Int
- (s/optional-key :sub) {(s/optional-key :key) s/Int
- s/Any s/Any}
- s/Any s/Any}]
- (is (nil? (s/check (su/open-schema schema) value)))))
- (testing "handles if there are already s/Any's or s/Keyword's"
- (let [value {:thing 3
- :extra-key 5
- :sub {:key 3 :another-extra 5}}
- schema {(s/optional-key :thing) s/Int
- (s/optional-key :sub) {(s/optional-key :key) s/Int
- s/Keyword s/Any}
- s/Any s/Any}]
- (is (nil? (s/check (su/open-schema schema) value)))))
- (testing "still validates the spec-ed entries"
- (let [value {:thing 3.0
- :extra-key 5
- :sub {:key 3 :another-extra 5}}
- schema {(s/optional-key :thing) s/Int
- (s/optional-key :sub) {(s/optional-key :key) s/Int}}]
- (is (contains? (s/check (su/open-schema schema) value)
- :thing))))
- (testing "if it contains a generic open entry, it is replaced with an s/Any"
- (doseq [validator [s/Keyword s/Symbol s/Str s/Int]]
- (let [value {:thing 3}
- schema {(s/optional-key :thing) s/Int
- validator s/Any}]
- (is (nil? (s/check (su/open-schema schema) (assoc value :random-thing :whatever))))))))
-
-(defn- plumatic-validate
- [schema x]
- (boolean (u/ignore-exceptions
- (s/validate schema x))))
-
-(defn- malli-validate
- [schema x]
- (boolean (u/ignore-exceptions
- (mc/validate schema x))))
-
-(deftest ^:parallel malli-and-plumatic-compatibility
- (doseq [{:keys [plumatic malli failed-cases success-cases]}
- [{:plumatic su/NonBlankString
- :malli ms/NonBlankString
- :failed-cases ["" 1]
- :success-cases ["a thing"]}
- {:plumatic su/IntGreaterThanOrEqualToZero
- :malli ms/IntGreaterThanOrEqualToZero
- :failed-cases ["1" -1 1.5]
- :success-cases [0 1]}
- {:plumatic su/IntGreaterThanZero
- :malli ms/PositiveInt
- :failed-cases ["1" 0 1.5]
- :success-cases [1 2]}
- {:plumatic su/PositiveNum
- :malli ms/PositiveNum
- :failed-cases [0 "1"]
- :success-cases [1.5 2]}
- {:plumatic su/KeywordOrString
- :malli ms/KeywordOrString
- :failed-cases [1 [1] {:a 1}]
- :success-cases [:a "a"]}
- {:plumatic su/FieldType
- :malli ms/FieldType
- :failed-cases [:type/invalid :Semantic/*]
- :success-cases [:type/Float]}
- {:plumatic su/FieldSemanticType
- :malli ms/FieldSemanticType
- :failed-cases [:Semantic/invalid :type/Float]
- :success-cases [:type/Category]}
- {:plumatic su/FieldRelationType
- :malli ms/FieldRelationType
- :failed-cases [:Relation/invalid :type/Category :type/Float]
- :success-cases [:type/FK]}
- {:plumatic su/FieldSemanticOrRelationType
- :malli ms/FieldSemanticOrRelationType
- :failed-cases [:Relation/invalid :type/Float]
- :success-cases [:type/FK :type/Category]}
- {:plumatic su/CoercionStrategy
- :malli ms/CoercionStrategy
- :failed-cases [:type/Category :type/Float]
- :success-cases [:Coercion/ISO8601->Date]}
- {:plumatic su/FieldTypeKeywordOrString
- :malli ms/FieldTypeKeywordOrString
- :failed-cases [1 :type/FK]
- :success-cases [:type/Float "type/Float"]}
- {:plumatic su/FieldSemanticTypeKeywordOrString
- :malli ms/FieldSemanticTypeKeywordOrString
- :failed-cases [1 :type/FK]
- :success-cases [:type/Category "type/Category"]}
- {:plumatic su/Field
- :malli ms/Field
- :failed-cases [[:aggregation 0] [:field "name" {}]]
- :success-cases [[:field 3 nil] ["field" "name" {:base-type :type/Float}]]}
- {:plumatic su/Map
- :malli ms/Map
- :failed-cases [[] 1 "a"]
- :success-cases [{} {:a :b}]}
- {:plumatic su/Email
- :malli ms/Email
- :failed-cases ["abc.com" 1]
- :success-cases ["ngoc@metabase.com"]}
- {:plumatic su/ValidPassword
- :malli ms/ValidPassword
- :failed-cases ["abc.com" 1 "PASSW0RD"]
- :success-cases ["unc0mmonpw"]}
- {:plumatic su/IntString
- :malli ms/IntString
- :failed-cases [:a "a" "1.5"]
- :success-cases ["1"]}
- {:plumatic su/BooleanString
- :malli ms/BooleanString
- :failed-cases [:false :true true "f" "t"]
- :success-cases ["true" "false"]}
- {:plumatic su/TemporalString
- :malli ms/TemporalString
- :failed-cases ["random string"]
- :success-cases ["2019-10-28T13:14:15" "2019-10-28"]}
- {:plumatic su/JSONString
- :malli ms/JSONString
- :failed-cases ["string"]
- :success-cases ["{\"a\": 1}"]}
- {:plumatic su/EmbeddingParams
- :malli ms/EmbeddingParams
- :failed-cases [{:key "value"}]
- :success-cases [{:key "disabled"}]}
- {:plumatic su/ValidLocale
- :malli ms/ValidLocale
- :failed-cases ["locale"]
- :success-cases ["en" "es"]}
- {:plumatic su/NanoIdString
- :malli ms/NanoIdString
- :failed-cases ["random"]
- :success-cases ["FReCLx5hSWTBU7kjCWfuu"]}
- {:plumatic su/UUIDString
- :malli ms/UUIDString
- :failed-cases ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]
- :success-cases ["84a51d43-2d29-4c2c-8484-e51eb5af2ca4"]}
- {:plumatic su/Parameter
- :malli ms/Parameter
- :failed-cases [{:id "param-id"
- :name "param-name"}
- {:id "param-id"
- :type "number"
- :values_source_type "invalid-type"
- :values_source_config {:values [[1 2 3]]}}
- {:id "param-id"
- :type "number"
- :values_source_type "card"
- :values_source_config {:card_id 3
- :value_field [:aggregation 0]}}]
- :success-cases [{:id "param-id"
- :type "number"
- :values_source_type "card"
- :values_source_config {:card_id 3
- :value_field [:field 3 nil]
- :label_field [:field "name" {:base-type :type/Float}]}}
- {:id "param-id"
- :type "number"
- :values_source_type "static-list"
- :values_source_config {:values [[1 2 3]]}}]}
- {:plumatic su/ParameterMapping
- :malli ms/ParameterMapping
- :failed-cases [{:parameter_id "param-id"}
- {:parameter_id "param-id"
- :target [:field 3 nil]
- :card_id "a"}]
- :success-cases [{:parameter_id "param-id"
- :target [:field 3 nil]
- :card_id 3}]}]]
-
- (doseq [case failed-cases]
- (testing (format "case: %s should fail" (pr-str case))
- (testing (format "with malli Schema: %s" (pr-str malli))
- (is (false? (malli-validate malli case))))
- (testing (format "with Plumatic Schema: %s" (pr-str plumatic))
- (is (false? (plumatic-validate plumatic case))))))
-
- (doseq [case success-cases]
- (testing (format "case: %s should success" (pr-str case))
- (testing (format "with malli Schema: %s" (pr-str malli))
- (is (true? (malli-validate malli case))))
- (testing (format "with Plumatic Schema: %s" (pr-str plumatic))
- (is (true? (plumatic-validate plumatic case))))))))