Merge pull request #3350 from CSCfi/low-level-optimization-2

Low level optimizations 2

src/clj/rems/db/applications.clj

@@ -200,22 +200,37 @@
             :bar #{"user-1"}}
            (group-users-by-role apps)))))
 
+(defn- user-roles-frequencies [applications]
+  (if (seq applications)
+    (let [all-user-roles (->> applications
+                              (eduction (map val)
+                                        (mapcat :application/user-roles)))
+          roles-by-user (reduce (fn [m [user roles]]
+                                  (assoc! m
+                                          ::users (conj! (::users m) user)
+                                          user (reduce conj!
+                                                       (or (m user) (transient []))
+                                                       roles)))
+                                (transient {::users (transient #{})})
+                                all-user-roles)
+          users (persistent! (::users roles-by-user))]
+      (-> (reduce (fn [m user]
+                    (assoc! m user (frequencies (persistent! (m user)))))
+                  roles-by-user
+                  users)
+          (dissoc! ::users)
+          persistent!))
+    {}))
+
 (defn- update-user-roles [updated-users old-roles-by-user old-updated-enriched-apps new-updated-enriched-apps]
-  (let [all-old-app-roles (->> old-updated-enriched-apps
-                               vals
-                               (mapv :application/user-roles))
-        all-new-app-roles (->> new-updated-enriched-apps
-                               vals
-                               (mapv :application/user-roles))]
+  (let [all-old-app-roles (user-roles-frequencies old-updated-enriched-apps)
+        all-new-app-roles (user-roles-frequencies new-updated-enriched-apps)]
+
     (into old-roles-by-user
           (for [userid updated-users
-                :let [old-user-roles (get old-roles-by-user userid {})
-                      old-app-roles (->> all-old-app-roles
-                                         (mapcat #(get % userid))
-                                         frequencies)
-                      new-app-roles (->> all-new-app-roles
-                                         (mapcat #(get % userid))
-                                         frequencies)]]
+                :let [old-user-roles (or (old-roles-by-user userid) {})
+                      old-app-roles (or (all-old-app-roles userid) {})
+                      new-app-roles (or (all-new-app-roles userid) {})]]
 
             [userid (as-> old-user-roles roles
                       (merge-with - roles old-app-roles)

src/clj/rems/permissions.clj

@@ -1,8 +1,8 @@
 (ns rems.permissions
-  (:require [clojure.set :as set]
+  (:require [clojure.core.memoize :refer [memo]]
+            [clojure.set :as set]
             [clojure.test :refer [deftest is testing]]
-            [medley.core :refer [map-vals map-kv-vals]]
-            [rems.common.util :refer [conj-set]]))
+            [medley.core :refer [map-vals]]))
 
 (defn give-role-to-users [application role users]
   (assert (keyword? role) {:role role})
@@ -63,6 +63,13 @@
       (is (= #{:role-2} (user-roles app "user-2")))
       (is (= #{:everyone-else} (user-roles app "user-3"))))))
 
+(defn- set-role-permissions [app-role-permissions permission-map]
+  (reduce-kv (fn [m role permissions]
+               (assert (keyword? role) {:role role})
+               (assoc m role (set permissions)))
+             app-role-permissions
+             permission-map))
+
 (defn update-role-permissions
   "Sets role specific permissions for the application.
 
@@ -72,11 +79,7 @@
    or they may be used to specify whether the user can see all events and
    comments from the reviewers (e.g. `:see-everything`)."
   [application permission-map]
-  (reduce (fn [application [role permissions]]
-            (assert (keyword? role) {:role role})
-            (assoc-in application [:application/role-permissions role] (set permissions)))
-          application
-          permission-map))
+  (update application :application/role-permissions set-role-permissions permission-map))
 
 (deftest test-update-role-permissions
   (testing "adding"
@@ -119,31 +122,42 @@
   (assert (keyword? permission)
           {:message "invalid permission" :rule rule}))
 
-(defn compile-rules
-  "Compiles rules of the format `[{:role keyword :permission keyword}]`
-  to the format expected by the `whitelist` and `blacklist` functions.
-  If `:role` is missing or nil, it means every role."
-  [rules]
+(defn- validate-and-compile [rules]
   (doseq [rule rules]
     (validate-rule rule))
   (->> rules
        (group-by :role)
        (map-vals (fn [rules]
                    (set (map :permission rules))))))
 
-(defn- map-permissions [application f]
-  (update application :application/role-permissions #(map-kv-vals f %)))
+(def compile-rules
+  "Compiles rules of the format `[{:role keyword :permission keyword}]`
+  to the format expected by the `whitelist` and `blacklist` functions.
+  If `:role` is missing or nil, it means every role."
+  (memo validate-and-compile))
 
 (defn- permissions-for-role [rules role]
-  (set/union (get rules role #{})
-             (get rules nil #{})))
+  (let [role-permissions (or (rules role) #{})]
+    (if-some [every-permissions (rules nil)]
+      (set/union role-permissions
+                 every-permissions)
+      role-permissions)))
+
+(defn- blacklist-permissions [role-permissions rules]
+  (reduce-kv (fn [m role permissions]
+               (assoc m role (if-some [role-permissions (permissions-for-role rules role)]
+                               (set/difference permissions role-permissions)
+                               permissions)))
+             {}
+             role-permissions))
+
+(def ^:private memoized-blacklist-permissions (memo blacklist-permissions))
 
 (defn blacklist
   "Applies rules for restricting the possible permissions.
   `rules` is what `compile-rules` returns."
   [application rules]
-  (map-permissions application (fn [role permissions]
-                                 (set/difference permissions (permissions-for-role rules role)))))
+  (update application :application/role-permissions memoized-blacklist-permissions rules))
 
 (deftest test-blacklist
   (let [app (-> {}
@@ -163,12 +177,21 @@
              (blacklist app (compile-rules [{:role :role-1 :permission :foo}
                                             {:role :role-2 :permission :bar}])))))))
 
+(defn- whitelist-permissions [role-permissions rules]
+  (reduce-kv (fn [m role permissions]
+               (assoc m role (if-some [role-permissions (permissions-for-role rules role)]
+                               (set/intersection permissions role-permissions)
+                               permissions)))
+             {}
+             role-permissions))
+
+(def ^:private memoized-whitelist-permissions (memo whitelist-permissions))
+
 (defn whitelist
   "Applies rules for restricting the possible permissions.
   `rules` is what `compile-rules` returns."
   [application rules]
-  (map-permissions application (fn [role permissions]
-                                 (set/intersection permissions (permissions-for-role rules role)))))
+  (update application :application/role-permissions memoized-whitelist-permissions rules))
 
 (deftest test-whitelist
   (let [app (-> {}

test/clj/rems/application/test_model.clj

@@ -1302,7 +1302,7 @@
                                 :event/actor-attributes {:userid "handler" :email "[email protected]" :name "Handler" :secret "secret"}
                                 :event/visibility :visibility/public}]
           :application/user-roles {"handler" #{:handler}, "reporter1" #{:reporter}}
-          :application/role-permissions nil
+          :application/role-permissions {}
           :application/description "foo"
           :application/forms [{:form/id 40
                                :form/title "form name" ; deprecated