[ARVADOS] created: 302d9232af7fd02a4fa57af5fd89fd0cec39fdbe

Git user git at public.curoverse.com
Sat Jan 7 03:25:13 EST 2017


        at  302d9232af7fd02a4fa57af5fd89fd0cec39fdbe (commit)


commit 302d9232af7fd02a4fa57af5fd89fd0cec39fdbe
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Jan 7 03:21:54 2017 -0500

    10816: Use a recursive postgres query instead of building the permission graph in Ruby.

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 964de38..1c26f5c 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -135,60 +135,40 @@ class User < ArvadosModel
   # Return a hash of {group_uuid: perm_hash} where perm_hash[:read]
   # and perm_hash[:write] are true if this user can read and write
   # objects owned by group_uuid.
-  #
-  # The permission graph is built by repeatedly enumerating all
-  # permission links reachable from self.uuid, and then calling
-  # search_permissions
   def calculate_group_permissions
-      permissions_from = {}
-      todo = {self.uuid => true}
-      done = {}
-      # Build the equivalence class of permissions starting with
-      # self.uuid. On each iteration of this loop, todo contains
-      # the next set of uuids in the permission equivalence class
-      # to evaluate.
-      while !todo.empty?
-        lookup_uuids = todo.keys
-        lookup_uuids.each do |uuid| done[uuid] = true end
-        todo = {}
-        newgroups = []
-        # include all groups owned by the current set of uuids.
-        Group.where('owner_uuid in (?)', lookup_uuids).each do |group|
-          newgroups << [group.owner_uuid, group.uuid, 'can_manage']
-        end
-        # add any permission links from the current lookup_uuids to a Group.
-        Link.where('link_class = ? and tail_uuid in (?) and ' \
-                   '(head_uuid like ? or (name = ? and head_uuid like ?))',
-                   'permission',
-                   lookup_uuids,
-                   Group.uuid_like_pattern,
-                   'can_manage',
-                   User.uuid_like_pattern).each do |link|
-          newgroups << [link.tail_uuid, link.head_uuid, link.name]
-        end
-        newgroups.each do |tail_uuid, head_uuid, perm_name|
-          unless done.has_key? head_uuid
-            todo[head_uuid] = true
-          end
-          link_permissions = {}
-          case perm_name
-          when 'can_read'
-            link_permissions = {read:true}
-          when 'can_write'
-            link_permissions = {read:true,write:true}
-          when 'can_manage'
-            link_permissions = ALL_PERMISSIONS
-          end
-          permissions_from[tail_uuid] ||= {}
-          permissions_from[tail_uuid][head_uuid] ||= {}
-          link_permissions.each do |k,v|
-            permissions_from[tail_uuid][head_uuid][k] ||= v
-          end
-        end
+    conn = ActiveRecord::Base.connection
+    self.class.transaction do
+      # Check whether the temporary view has already been created
+      # during this connection. If not, create it.
+      conn.execute 'SAVEPOINT check_permission_view'
+      begin
+        conn.execute('SELECT 1 FROM permission_view LIMIT 0')
+      rescue
+        conn.execute 'ROLLBACK TO SAVEPOINT check_permission_view'
+        sql = File.read(Rails.root.join('lib', 'create_permission_view.sql'))
+        conn.exec_query(sql)
+      else
+        conn.execute 'RELEASE SAVEPOINT check_permission_view'
       end
-      perms = search_permissions(self.uuid, permissions_from)
-      Rails.cache.write "groups_for_user_#{self.uuid}", perms
-      perms
+    end
+
+    group_perms = {}
+    perms_for_val =
+      [{},
+       {read: true},
+       {read: true, write: true},
+       {read: true, write: true, manage: true}]
+    conn.exec_query('SELECT target_owner_uuid, max(perm_level)
+                  FROM permission_view
+                  WHERE user_uuid = $1
+                  AND target_owner_uuid IS NOT NULL
+                  GROUP BY target_owner_uuid',
+                  "group_permissions for #{uuid}",
+                  [[nil, uuid]]).rows.each do |group_uuid, max_p_val|
+      group_perms[group_uuid] = perms_for_val[max_p_val.to_i]
+    end
+    Rails.cache.write "groups_for_user_#{self.uuid}", group_perms
+    group_perms
   end
 
   # Return a hash of {group_uuid: perm_hash} where perm_hash[:read]
diff --git a/services/api/lib/create_permission_view.sql b/services/api/lib/create_permission_view.sql
new file mode 100644
index 0000000..2a9e55b
--- /dev/null
+++ b/services/api/lib/create_permission_view.sql
@@ -0,0 +1,41 @@
+CREATE TEMPORARY VIEW permission_view AS
+WITH RECURSIVE
+perm_value (name, val) AS (
+     VALUES
+     ('can_read',   1::smallint),
+     ('can_login',  1),
+     ('can_write',  2),
+     ('can_manage', 3)
+     ),
+perm_edges (tail_uuid, head_uuid, val, follow) AS (
+       SELECT links.tail_uuid,
+              links.head_uuid,
+              pv.val,
+              (pv.val = 3 OR groups.uuid IS NOT NULL) AS follow
+              FROM links
+              LEFT JOIN perm_value pv ON pv.name = links.name
+              LEFT JOIN groups ON pv.val<3 AND groups.uuid = links.head_uuid
+              WHERE links.link_class = 'permission'
+       UNION ALL
+       SELECT owner_uuid, uuid, 3, true FROM groups
+       ),
+perm (val, follow, user_uuid, target_uuid) AS (
+     SELECT 3::smallint             AS val,
+            true                    AS follow,
+            users.uuid::varchar(32) AS user_uuid,
+            users.uuid::varchar(32) AS target_uuid
+            FROM users
+     UNION
+     SELECT LEAST(perm.val, edges.val)::smallint AS val,
+            edges.follow                         AS follow,
+            perm.user_uuid::varchar(32)          AS user_uuid,
+            edges.head_uuid::varchar(32)         AS target_uuid
+            FROM perm
+            INNER JOIN perm_edges edges
+            ON perm.follow AND edges.tail_uuid = perm.target_uuid
+)
+SELECT user_uuid,
+       target_uuid,
+       val AS perm_level,
+       CASE follow WHEN true THEN target_uuid ELSE NULL END AS target_owner_uuid
+       FROM perm;
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index 0d66a4b..3bd6ed4 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -276,10 +276,12 @@ class UserTest < ActiveSupport::TestCase
     assert @uninvited_user.can? :write=>"#{@uninvited_user.uuid}"
     assert @uninvited_user.can? :manage=>"#{@uninvited_user.uuid}"
 
-    assert @uninvited_user.groups_i_can(:read).size == 1, "inactive and uninvited user can only read anonymous user group"
-    assert @uninvited_user.groups_i_can(:read).first.ends_with? 'anonymouspublic' , "inactive and uninvited user can only read anonymous user group"
-    assert @uninvited_user.groups_i_can(:write).size == 0, "inactive and uninvited user should not be able write to any groups"
-    assert @uninvited_user.groups_i_can(:manage).size == 0, "inactive and uninvited user should not be able manage any groups"
+    assert_equal(@uninvited_user.groups_i_can(:read).sort,
+                 [@uninvited_user.uuid, groups(:anonymous_group).uuid].sort)
+    assert_equal(@uninvited_user.groups_i_can(:write),
+                 [@uninvited_user.uuid])
+    assert_equal(@uninvited_user.groups_i_can(:manage),
+                 [@uninvited_user.uuid])
   end
 
   test "find user method checks" do

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list