Skip to content

Commit 9c9dc1f

Browse files
camsaulcamsaul
authored
Fix Kondo linter for with-temp and add linters for Malli explain/validate (metabase#51747)
* DEFENDPOINT 2.0 * Remove blank file * Fix Kondo linter for with-temp and add linters for Malli explain/validate * Fix Kondo errors and reformat * Revert unneeded change
1 parent f1b714a commit 9c9dc1f

260 files changed

Lines changed: 3269 additions & 3427 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.clj-kondo/config.edn‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
;;
5757
;; globally-enabled custom linters
5858
;;
59+
:metabase/check-me-humanize {:level :warning}
5960
:metabase/defsetting-must-specify-export {:level :warning}
6061
:metabase/mbql-query-first-arg {:level :warning}
6162
:metabase/missing-test-expr-requires-in-cljs {:level :warning}
@@ -269,13 +270,16 @@
269270
honeysql.core/raw {:message "Use hx/raw instead because it is Honey SQL 2 friendly"}
270271
java-time/with-clock {:message "Use mt/with-clock"}
271272
java.util.UUID/randomUUID {:message "Use clojure.core/random-uuid instead of java.util.UUID/randomUUID"}
272-
malli.core/explainer {:message "Use metabase.util.malli.registry/explainer instead of malli.core/explainer"}
273+
malli.core/explain {:message "Use metabase.util.malli.registry/explain instead of malli.core/explain because it has built-in caching"}
274+
malli.core/explainer {:message "Use metabase.util.malli.registry/explainer instead of malli.core/explainer because it has built-in caching"}
275+
malli.core/validate {:message "Use metabase.util.malli.registry/validate instead of malli.core/validate because it has built-in caching"}
276+
malli.core/validator {:message "Use metabase.util.malli.registry/validator instead of malli.core/validator because it has built-in caching"}
273277
me.flowthing.pp/pprint {:message "Use metabase.util.log instead of me.flowthing.pp/pprint"}
274278
medley.core/random-uuid {:message "Use clojure.core/random-uuid instead of medley.core/random-uuid"}
275279
metabase.driver/database-supports? {:message "Use metabase.driver.util/supports? instead of metabase.driver/database-supports?"}
276280
metabase.lib.equality/find-column-indexes-for-refs {:message "Use lib.equality/closest-matches-in-metadata or lib.equality/find-closest-matches-for-refs instead of lib.equality/find-column-indexes-for-refs"}
277281
metabase.test/with-temp* {:message "Use mt/with-temp instead of mt/with-temp*"}
278-
toucan2.tools.with-temp {:message "Use mt/with-temp instead of t2.with-temp/with-temp"}}
282+
toucan2.tools.with-temp/with-temp {:message "Use mt/with-temp instead of toucan2.tools.with-temp/with-temp"}}
279283

280284
:discouraged-namespace
281285
{camel-snake-kebab.core {:message "CSK is not Turkish-safe, use the versions in metabase.util instead."}
@@ -861,6 +865,7 @@
861865
clojure.test/deftest hooks.clojure.test/deftest
862866
clojure.test/is hooks.clojure.test/is
863867
clojure.test/use-fixtures hooks.clojure.test/use-fixtures
868+
malli.error/humanize hooks.malli.error/humanize
864869
metabase-enterprise.advanced-permissions.models.permissions.application-permissions-test/with-new-group-and-current-graph hooks.common/with-two-top-level-bindings
865870
metabase-enterprise.audit-app.pages-test/with-temp-objects hooks.common/with-one-binding
866871
metabase-enterprise.cache.config-test/with-temp-persist-models hooks.common/with-seven-bindings

‎.clj-kondo/src/hooks/malli/error.clj‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
(ns hooks.malli.error
2+
(:require
3+
[clj-kondo.hooks-api :as hooks]
4+
[hooks.common]))
5+
6+
(defn humanize [x]
7+
(letfn [(check-node [node]
8+
(when (hooks/list-node? node)
9+
(let [[_ child] (:children node)]
10+
(when (hooks/list-node? child)
11+
(let [[symb] (:children child)]
12+
(when-let [qualified-symb (hooks.common/node->qualified-symbol symb)]
13+
(when ('#{malli.core/validate metabase.util.malli.registry/validate}
14+
qualified-symb)
15+
(hooks/reg-finding! (assoc (meta symb)
16+
:message "Use malli.error/humanize with explain, NOT with validate. [:metabase/check-me-humanize]"
17+
:type :metabase/check-me-humanize)))))))))]
18+
(check-node (:node x))
19+
x))
20+
21+
(comment
22+
(humanize {:node (hooks/parse-string
23+
(pr-str '(malli.error/humanize
24+
(malli.core/validate
25+
metabase.sync.sync-metadata.dbms-version/DBMSVersion
26+
metabase.driver.druid.sync-test/dbms-version))))}))

‎enterprise/backend/src/metabase_enterprise/serialization/names.clj‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22
"Consistent instance-independent naming scheme that replaces IDs with human-readable paths."
33
(:require
44
[clojure.string :as str]
5-
[malli.core :as mc]
65
[metabase.db :as mdb]
76
[metabase.lib.schema.id :as lib.schema.id]
87
[metabase.models.interface :as mi]
98
[metabase.util.log :as log]
9+
[metabase.util.malli.registry :as mr]
1010
[metabase.util.malli.schema :as ms]
1111
[ring.util.codec :as codec]
1212
[toucan2.core :as t2]
@@ -303,7 +303,7 @@
303303
new-context
304304
(recur new-context more))))]
305305
(if (and
306-
(not (mc/validate [:maybe Context] context))
306+
(not (mr/validate [:maybe Context] context))
307307
(not *suppress-log-name-lookup-exception*))
308308
(log/warn
309309
(ex-info (format "Can't resolve %s in fully qualified name %s"

‎enterprise/backend/src/metabase_enterprise/sso/integrations/sso_settings.clj‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
the SSO backends and the generic routing code used to determine which SSO backend to use need this
44
information. Separating out this information creates a better dependency graph and avoids circular dependencies."
55
(:require
6-
[malli.core :as mc]
76
[metabase-enterprise.scim.api :as scim]
87
[metabase.integrations.common :as integrations.common]
98
[metabase.models.setting :as setting :refer [defsetting]]
@@ -12,6 +11,7 @@
1211
[metabase.util.i18n :refer [deferred-tru tru]]
1312
[metabase.util.log :as log]
1413
[metabase.util.malli :as mu]
14+
[metabase.util.malli.registry :as mr]
1515
[metabase.util.malli.schema :as ms]
1616
[saml20-clj.core :as saml]))
1717

@@ -21,7 +21,7 @@
2121
[:maybe [:map-of ms/KeywordOrString [:sequential ms/PositiveInt]]])
2222

2323
(def ^:private ^{:arglists '([group-mappings])} validate-group-mappings
24-
(mc/validator GroupMappings))
24+
(mr/validator GroupMappings))
2525

2626
(defsetting saml-user-provisioning-enabled?
2727
(deferred-tru "When we enable SAML user provisioning, we automatically create a Metabase account on SAML signin for users who

‎enterprise/backend/test/metabase_enterprise/advanced_config/api/pulse_test.clj‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@
33
[clojure.test :refer :all]
44
[metabase.test :as mt]
55
[metabase.test.fixtures :as fixtures]
6-
[metabase.util :as u]
7-
[toucan2.tools.with-temp :as t2.with-temp]))
6+
[metabase.util :as u]))
87

98
(use-fixtures :once (fixtures/initialize :plugins))
109

1110
(deftest test-pulse-endpoint-should-respect-email-domain-allow-list-test
1211
(testing "POST /api/pulse/test"
13-
(t2.with-temp/with-temp [:model/Card card {:name "Test card"
14-
:dataset_query (mt/mbql-query venues)}]
12+
(mt/with-temp [:model/Card card {:name "Test card"
13+
:dataset_query (mt/mbql-query venues)}]
1514
;; make sure we validate raw emails whether they're part of `:details` or part of `:recipients` -- we
1615
;; technically allow either right now
1716
(doseq [channel [{:details {:emails ["test@metabase.com"]}}

‎enterprise/backend/test/metabase_enterprise/advanced_config/models/pulse_channel_test.clj‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,10 @@
55
[metabase-enterprise.advanced-config.models.pulse-channel :as advanced-config.models.pulse-channel]
66
[metabase.test :as mt]
77
[metabase.util :as u]
8-
[toucan2.core :as t2]
9-
[toucan2.tools.with-temp :as t2.with-temp]))
8+
[toucan2.core :as t2]))
109

1110
(deftest validate-email-domains-test
12-
(t2.with-temp/with-temp [:model/Pulse {pulse-id :id}]
11+
(mt/with-temp [:model/Pulse {pulse-id :id}]
1312
(doseq [operation [:create :update]
1413
allowed-domains [nil
1514
#{"metabase.com"}
@@ -31,11 +30,11 @@
3130
(let [thunk (case operation
3231
:create
3332
#(first (t2/insert-returning-instances! :model/PulseChannel
34-
(merge (t2.with-temp/with-temp-defaults :model/PulseChannel)
33+
(merge (mt/with-temp-defaults :model/PulseChannel)
3534
{:pulse_id pulse-id, :details {:emails emails}})))
3635

3736
:update
38-
#(t2.with-temp/with-temp [:model/PulseChannel {pulse-channel-id :id} {:pulse_id pulse-id}]
37+
#(mt/with-temp [:model/PulseChannel {pulse-channel-id :id} {:pulse_id pulse-id}]
3938
(t2/update! :model/PulseChannel pulse-channel-id {:details {:emails emails}})))]
4039
(if fail?
4140
(testing "should fail"

‎enterprise/backend/test/metabase_enterprise/advanced_permissions/api/application_test.clj‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,10 @@
33
[clojure.test :refer :all]
44
[metabase-enterprise.advanced-permissions.models.permissions.application-permissions :as a-perms]
55
[metabase.models.permissions-group :as perms-group]
6-
[metabase.test :as mt]
7-
[toucan2.tools.with-temp :as t2.with-temp]))
6+
[metabase.test :as mt]))
87

98
(deftest application-permissions-test
10-
(t2.with-temp/with-temp [:model/PermissionsGroup _]
9+
(mt/with-temp [:model/PermissionsGroup _]
1110
(testing "GET /api/ee/advanced-permissions/application/graph"
1211
(mt/with-premium-features #{}
1312
(testing "Should require a token with `:advanced-permissions`"
@@ -33,7 +32,7 @@
3332
groups))))))))
3433

3534
(deftest application-permissions-test-2
36-
(t2.with-temp/with-temp [:model/PermissionsGroup {group-id :id}]
35+
(mt/with-temp [:model/PermissionsGroup {group-id :id}]
3736
(testing "PUT /api/ee/advanced-permissions/application/graph"
3837
(let [current-graph (mt/with-premium-features #{:advanced-permissions}
3938
(mt/user-http-request :crowberto :get 200 "ee/advanced-permissions/application/graph"))

‎enterprise/backend/test/metabase_enterprise/advanced_permissions/api/group_manager_test.clj‎

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@
88
[metabase.models.user :as user]
99
[metabase.test :as mt]
1010
[metabase.util :as u]
11-
[toucan2.core :as t2]
12-
[toucan2.tools.with-temp :as t2.with-temp]))
11+
[toucan2.core :as t2]))
1312

1413
(deftest permissions-group-apis-test
1514
(testing "/api/permissions/group"
@@ -88,35 +87,35 @@
8887

8988
(defn- add-membership! [user status group-info is-group-manager]
9089
(testing (format ", add membership with %s user" (mt/user-descriptor user))
91-
(t2.with-temp/with-temp [:model/User user-info]
90+
(mt/with-temp [:model/User user-info]
9291
(mt/user-http-request user :post status "permissions/membership"
9392
{:group_id (:id group-info)
9493
:user_id (:id user-info)
9594
:is_group_manager is-group-manager}))))
9695

9796
(defn- update-membership! [user status group-info is-group-manager]
9897
(testing (format ", update membership with %s user" (mt/user-descriptor user))
99-
(t2.with-temp/with-temp [:model/User user-info {}
100-
:model/PermissionsGroupMembership {:keys [id]} {:user_id (:id user-info)
101-
:group_id (:id group-info)}]
98+
(mt/with-temp [:model/User user-info {}
99+
:model/PermissionsGroupMembership {:keys [id]} {:user_id (:id user-info)
100+
:group_id (:id group-info)}]
102101
(mt/user-http-request user :put status (format "permissions/membership/%d" id)
103102
{:is_group_manager is-group-manager}))))
104103

105104
(defn- delete-membership! [user status group-info]
106105
(testing (format ", delete membership with %s user" (mt/user-descriptor user))
107-
(t2.with-temp/with-temp [:model/User user-info {}
108-
:model/PermissionsGroupMembership {pgm-id :id} {:user_id (:id user-info)
109-
:group_id (:id group-info)}]
106+
(mt/with-temp [:model/User user-info {}
107+
:model/PermissionsGroupMembership {pgm-id :id} {:user_id (:id user-info)
108+
:group_id (:id group-info)}]
110109
(mt/user-http-request user :delete status (format "permissions/membership/%d" pgm-id)))))
111110

112111
(defn- clear-memberships! [user status group-info]
113112
(testing (format ", clearing group memberships with %s user" (mt/user-descriptor :crowberto))
114-
(t2.with-temp/with-temp [:model/User user-info-1 {}
115-
:model/User user-info-2 {}
116-
:model/PermissionsGroupMembership _ {:user_id (:id user-info-1)
117-
:group_id (:id group-info)}
118-
:model/PermissionsGroupMembership _ {:user_id (:id user-info-2)
119-
:group_id (:id group-info)}]
113+
(mt/with-temp [:model/User user-info-1 {}
114+
:model/User user-info-2 {}
115+
:model/PermissionsGroupMembership _ {:user_id (:id user-info-1)
116+
:group_id (:id group-info)}
117+
:model/PermissionsGroupMembership _ {:user_id (:id user-info-2)
118+
:group_id (:id group-info)}]
120119
(mt/user-http-request user :put status (format "permissions/membership/%d/clear" (:id group-info))))))
121120

122121
(defn- membership->groups-ids [membership]
@@ -200,10 +199,10 @@
200199
(is (= #{(:id group)} (membership->groups-ids (get-membership user 200))))))
201200

202201
(testing "admin cant be group manager"
203-
(t2.with-temp/with-temp [:model/User new-user {:is_superuser true}
204-
:model/PermissionsGroupMembership _ {:user_id (:id new-user)
205-
:group_id (:id group)
206-
:is_group_manager false}]
202+
(mt/with-temp [:model/User new-user {:is_superuser true}
203+
:model/PermissionsGroupMembership _ {:user_id (:id new-user)
204+
:group_id (:id group)
205+
:is_group_manager false}]
207206
(is (= "Admin cant be a group manager."
208207
(mt/user-http-request user :post 400 "permissions/membership"
209208
{:group_id (:id group)
@@ -245,23 +244,23 @@
245244
(deftest get-users-api-group-id-test
246245
(testing "GET /api/user?group_id=:group_id"
247246
(testing "should sort by admins -> group managers -> normal users when filter by group_id"
248-
(t2.with-temp/with-temp [:model/User user-a {:first_name "A"
249-
:last_name "A"}
250-
:model/User user-b {:first_name "B"
251-
:last_name "B"}
252-
:model/User user-c {:first_name "C"
253-
:last_name "C"
254-
:is_superuser true}
255-
:model/PermissionsGroup group {}
256-
:model/PermissionsGroupMembership _ {:user_id (:id user-a)
257-
:group_id (:id group)
258-
:is_group_manager false}
259-
:model/PermissionsGroupMembership _ {:user_id (:id user-b)
260-
:group_id (:id group)
261-
:is_group_manager true}
262-
:model/PermissionsGroupMembership _ {:user_id (:id user-c)
263-
:group_id (:id group)
264-
:is_group_manager false}]
247+
(mt/with-temp [:model/User user-a {:first_name "A"
248+
:last_name "A"}
249+
:model/User user-b {:first_name "B"
250+
:last_name "B"}
251+
:model/User user-c {:first_name "C"
252+
:last_name "C"
253+
:is_superuser true}
254+
:model/PermissionsGroup group {}
255+
:model/PermissionsGroupMembership _ {:user_id (:id user-a)
256+
:group_id (:id group)
257+
:is_group_manager false}
258+
:model/PermissionsGroupMembership _ {:user_id (:id user-b)
259+
:group_id (:id group)
260+
:is_group_manager true}
261+
:model/PermissionsGroupMembership _ {:user_id (:id user-c)
262+
:group_id (:id group)
263+
:is_group_manager false}]
265264
(is (=? {:data [{:first_name "C"}
266265
{:first_name "B"}
267266
{:first_name "A"}]}
@@ -274,7 +273,7 @@
274273
user [group]]
275274
(letfn [(get-user [req-user status]
276275
(testing (format "- get user with %s user" (mt/user-descriptor user))
277-
(t2.with-temp/with-temp [:model/User new-user]
276+
(mt/with-temp [:model/User new-user]
278277
(mt/user-http-request req-user :get status (format "user/%d" (:id new-user))))))]
279278

280279
(testing "if `advanced-permissions` is disabled, require admins"
@@ -366,6 +365,6 @@
366365
(set (:user_group_memberships (remove-user-from-group! user 200 group))))))
367366

368367
(testing "Can't remove users from group they're not manager of"
369-
(t2.with-temp/with-temp [:model/PermissionsGroup random-group]
368+
(mt/with-temp [:model/PermissionsGroup random-group]
370369
(add-user-to-group! user 403 random-group)
371370
(remove-user-from-group! user 403 random-group))))))))))))

0 commit comments

Comments
 (0)