From 3ced541f50e81aeaa1624476976bcdac12085241 Mon Sep 17 00:00:00 2001
From: Oleksandr Yakushev <alex@bytopia.org>
Date: Thu, 26 Sep 2024 12:19:03 +0300
Subject: [PATCH] perf: Use vectors to iterate over all user permissions
(#48025)
---
src/metabase/models/permissions.clj | 15 +++++++++------
src/metabase/models/user.clj | 23 +++++++++++++----------
src/metabase/util/performance.clj | 7 ++++++-
3 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/src/metabase/models/permissions.clj b/src/metabase/models/permissions.clj
index 80e755f7b12..e65681ed0df 100644
--- a/src/metabase/models/permissions.clj
+++ b/src/metabase/models/permissions.clj
@@ -182,6 +182,7 @@
[metabase.util.log :as log]
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
+ [metabase.util.performance :as perf]
[methodical.core :as methodical]
[toucan2.core :as t2]))
@@ -277,25 +278,27 @@
(defn set-has-full-permissions?
"Does `permissions-set` grant *full* access to object with `path`?"
^Boolean [permissions-set path]
- (boolean (some #(is-permissions-for-object? % path) permissions-set)))
+ (boolean (perf/some #(is-permissions-for-object? % path) permissions-set)))
(defn set-has-partial-permissions?
"Does `permissions-set` grant access full access to object with `path` *or* to a descendant of it?"
^Boolean [permissions-set path]
- (boolean (some #(is-partial-permissions-for-object? % path) permissions-set)))
+ (boolean (perf/some #(is-partial-permissions-for-object? % path) permissions-set)))
(mu/defn set-has-full-permissions-for-set? :- :boolean
"Do the permissions paths in `permissions-set` grant *full* access to all the object paths in `paths-set`?"
[permissions-set paths-set]
- (every? (partial set-has-full-permissions? permissions-set)
- paths-set))
+ (let [permissions (or (:as-vec (meta permissions-set))
+ permissions-set)]
+ (every? (partial set-has-full-permissions? permissions) paths-set)))
(mu/defn set-has-partial-permissions-for-set? :- :boolean
"Do the permissions paths in `permissions-set` grant *partial* access to all the object paths in `paths-set`?
(`permissions-set` must grant partial access to *every* object in `paths-set` set)."
[permissions-set paths-set]
- (every? (partial set-has-partial-permissions? permissions-set)
- paths-set))
+ (let [permissions (or (:as-vec (meta permissions-set))
+ permissions-set)]
+ (every? (partial set-has-partial-permissions? permissions) paths-set)))
(mu/defn set-has-application-permission-of-type? :- :boolean
"Does `permissions-set` grant *full* access to a application permission of type `perm-type`?"
diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj
index b2fcb921c70..13989ed56f8 100644
--- a/src/metabase/models/user.clj
+++ b/src/metabase/models/user.clj
@@ -250,16 +250,19 @@
(defn permissions-set
"Return a set of all permissions object paths that `user-or-id` has been granted access to. (2 DB Calls)"
[user-or-id]
- (set (when-let [user-id (u/the-id user-or-id)]
- (concat
- ;; Current User always gets readwrite perms for their Personal Collection and for its descendants! (1 DB Call)
- (map perms/collection-readwrite-path (collection/user->personal-collection-and-descendant-ids user-or-id))
- ;; include the other Perms entries for any Group this User is in (1 DB Call)
- (map :object (mdb.query/query {:select [:p.object]
- :from [[:permissions_group_membership :pgm]]
- :join [[:permissions_group :pg] [:= :pgm.group_id :pg.id]
- [:permissions :p] [:= :p.group_id :pg.id]]
- :where [:= :pgm.user_id user-id]}))))))
+ (let [s
+ (set (when-let [user-id (u/the-id user-or-id)]
+ (concat
+ ;; Current User always gets readwrite perms for their Personal Collection and for its descendants! (1 DB Call)
+ (map perms/collection-readwrite-path (collection/user->personal-collection-and-descendant-ids user-or-id))
+ ;; include the other Perms entries for any Group this User is in (1 DB Call)
+ (map :object (mdb.query/query {:select [:p.object]
+ :from [[:permissions_group_membership :pgm]]
+ :join [[:permissions_group :pg] [:= :pgm.group_id :pg.id]
+ [:permissions :p] [:= :p.group_id :pg.id]]
+ :where [:= :pgm.user_id user-id]})))))]
+ ;; Append permissions as a vector for more efficient iteration in checks that go over each permission linearly.
+ (with-meta s {:as-vec (vec s)})))
;;; --------------------------------------------------- Hydration ----------------------------------------------------
diff --git a/src/metabase/util/performance.clj b/src/metabase/util/performance.clj
index f64d28308b8..202e7932c57 100644
--- a/src/metabase/util/performance.clj
+++ b/src/metabase/util/performance.clj
@@ -1,6 +1,6 @@
(ns metabase.util.performance
"Functions and utilities for faster processing."
- (:refer-clojure :exclude [reduce mapv])
+ (:refer-clojure :exclude [reduce mapv some])
(:import (clojure.lang LazilyPersistentVector RT)))
(set! *warn-on-reflection* true)
@@ -126,3 +126,8 @@
([x y] (mapv #(% x y) fns))
([x y z] (mapv #(% x y z) fns))
([x y z & args] (mapv #(apply % x y z args) fns)))))
+
+(defn some
+ "Like `clojure.core/some` but uses our custom `reduce` which in turn uses iterators."
+ [f coll]
+ (unreduced (reduce #(when-let [match (f %2)] (reduced match)) nil coll)))
--
GitLab