[ARVADOS] updated: f301a70fdcfda9872965835b26d1400a53d584a1

Git user git at public.curoverse.com
Mon Jan 9 11:49:54 EST 2017


Summary of changes:
 build/run-library.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

  discards  309e099c9d264ca93a09ff13e4df642ee7fff00e (commit)
  discards  302d9232af7fd02a4fa57af5fd89fd0cec39fdbe (commit)
       via  f301a70fdcfda9872965835b26d1400a53d584a1 (commit)
       via  2ecdf8f2362585a838199ad97b5c0497432c4bd7 (commit)
       via  85f8ab2ef898f8d0bb7187db6db59a93ac528ec5 (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (309e099c9d264ca93a09ff13e4df642ee7fff00e)
            \
             N -- N -- N (f301a70fdcfda9872965835b26d1400a53d584a1)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit f301a70fdcfda9872965835b26d1400a53d584a1
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