[ARVADOS] updated: 1.3.0-2534-gce043d3f6
Git user
git at public.arvados.org
Thu May 7 03:02:09 UTC 2020
Summary of changes:
services/api/app/models/arvados_model.rb | 2 +-
services/api/app/models/group.rb | 85 +++++++++++++----
services/api/app/models/user.rb | 10 +-
.../db/migrate/20200501150153_permission_table.rb | 104 ++++++++++++++-------
services/api/lib/refresh_permission_view.rb | 18 +++-
services/api/test/test_helper.rb | 1 +
6 files changed, 165 insertions(+), 55 deletions(-)
via ce043d3f68376f6658d1bf401a6bf5cf00e4a221 (commit)
via 6431ee2e2cb4267de7b51cf4a52f23cb14cdc2a3 (commit)
from e77609d17994548cd5abed7a0d25517c10c73327 (commit)
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 ce043d3f68376f6658d1bf401a6bf5cf00e4a221
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed May 6 23:01:18 2020 -0400
16007: initialize materialized_permissions from search_permission_graph
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 83b9a6e10..150f46212 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -324,7 +324,7 @@ class ArvadosModel < ApplicationRecord
owner_check = ""
if sql_table != "api_client_authorizations" and sql_table != "groups" then
owner_check = "OR #{sql_table}.owner_uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
- "WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 #{trashed_check} AND target_owner_uuid IS NOT NULL) "
+ "WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 #{trashed_check} AND traverse_owned) "
end
links_cond = ""
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 3292642d4..cc81b3fab 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -70,6 +70,7 @@ insert into trashed_groups (group_uuid, trash_at)
on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
}
end
+
if uuid_changed? or owner_uuid_changed?
# This can change users' permissions on other groups as well as
# this one.
@@ -81,6 +82,44 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
# Ensure a new group can be accessed by the appropriate users
# immediately after being created.
User.invalidate_permissions_cache self.async_permissions_update
+
+ if new_record? or owner_uuid_changed?
+ # 1. Compute set (group, permission) implied by traversing
+ # graph starting at this group
+ # 2. Find links from outside the graph that point inside
+ # 3. We now have the full set of permissions for a subset of groups.
+ # 3. Upsert each permission in our subset (user, group, val)
+ # 4. Delete permissions from table not in our computed subset.
+
+ temptable_perm_from_start = "perm_from_start_#{rand(2**64).to_s(10)}"
+ ActiveRecord::Base.connection.exec_query %{
+create temporary table #{temptable_perm_from_start} on commit drop
+as select $1::varchar as starting_uuid, target_uuid, val
+from search_permission_graph($1::varchar, 3::smallint)
+},
+ 'Group.search_permissions',
+ [[nil, self.uuid]]
+
+ temptable_additional_perms = "additional_perms_#{rand(2**64).to_s(10)}"
+ ActiveRecord::Base.connection.exec_query %{
+create temporary table #{temptable_additional_perms} on commit drop
+as select links.tail_uuid as starting_uuid, ps.target_uuid, ps.val
+ from links, lateral search_permission_graph(links.head_uuid::varchar, CASE
+WHEN links.name = 'can_read' THEN 1::smallint
+WHEN links.name = 'can_login' THEN 1::smallint
+WHEN links.name = 'can_write' THEN 2::smallint
+WHEN links.name = 'can_manage' THEN 3::smallint
+END) as ps
+where links.link_class='permission' and
+ links.tail_uuid not in (select target_uuid from #{temptable_perm_from_start}) and
+ links.head_uuid in (select target_uuid from #{temptable_perm_from_start})
+}
+
+ q1 = ActiveRecord::Base.connection.exec_query "select * from #{temptable_perm_from_start}"
+ q1.each do |r|
+ #puts r
+ end
+ end
end
def assign_name
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index a5fc12d0a..a34491966 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -155,9 +155,9 @@ class User < ArvadosModel
def self.all_group_permissions
all_perms = {}
ActiveRecord::Base.connection.
- exec_query("SELECT user_uuid, target_owner_uuid, perm_level
+ exec_query("SELECT user_uuid, target_uuid, perm_level
FROM #{PERMISSION_VIEW}
- WHERE target_owner_uuid IS NOT NULL",
+ WHERE traverse_owned",
# "name" arg is a query label that appears in logs:
"all_group_permissions",
).rows.each do |user_uuid, group_uuid, max_p_val|
@@ -173,12 +173,12 @@ class User < ArvadosModel
def group_permissions
group_perms = {self.uuid => {:read => true, :write => true, :manage => true}}
ActiveRecord::Base.connection.
- exec_query("SELECT target_owner_uuid, perm_level
+ exec_query("SELECT target_uuid, perm_level
FROM #{PERMISSION_VIEW}
WHERE user_uuid = $1
- AND target_owner_uuid IS NOT NULL",
+ AND traverse_owned",
# "name" arg is a query label that appears in logs:
- "group_permissions for #{uuid}",
+ "group_permissions_for_user",
# "binds" arg is an array of [col_id, value] for '$1' vars:
[[nil, uuid]],
).rows.each do |group_uuid, max_p_val|
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 20b715df4..00f9bde91 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -4,7 +4,7 @@ class PermissionTable < ActiveRecord::Migration[5.0]
t.string :user_uuid
t.string :target_uuid
t.integer :perm_level
- t.string :target_owner_uuid
+ t.boolean :traverse_owned
end
ActiveRecord::Base.connection.execute %{
@@ -12,7 +12,7 @@ create or replace function compute_permission_table ()
returns table(user_uuid character varying (27),
target_uuid character varying (27),
perm_level smallint,
- target_owner_uuid character varying(27))
+ traverse_owned bool)
VOLATILE
language SQL
as $$
@@ -31,7 +31,7 @@ as $$
SELECT groups.owner_uuid,
groups.uuid,
3,
- true AS bool
+ true
FROM public.groups
), perm(val, follow, user_uuid, target_uuid) AS (
SELECT (3)::smallint AS val,
@@ -50,16 +50,9 @@ as $$
SELECT perm.user_uuid,
perm.target_uuid,
max(perm.val) AS perm_level,
- CASE perm.follow
- WHEN true THEN perm.target_uuid
- ELSE NULL::character varying
- END AS target_owner_uuid
+ bool_or(perm.follow) as traverse_owned
FROM perm
- GROUP BY perm.user_uuid, perm.target_uuid,
- CASE perm.follow
- WHEN true THEN perm.target_uuid
- ELSE NULL::character varying
- END
+ GROUP BY perm.user_uuid, perm.target_uuid
$$;
}
@@ -70,12 +63,12 @@ STABLE
language SQL
as $$
WITH RECURSIVE
- project_subtree(uuid) as (
- values (starting_uuid)
- union
- select groups.uuid from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid)
- )
- select uuid from project_subtree;
+ project_subtree(uuid) as (
+ values (starting_uuid)
+ union
+ select groups.uuid from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid)
+ )
+ select uuid from project_subtree;
$$;
}
@@ -86,13 +79,13 @@ STABLE
language SQL
as $$
WITH RECURSIVE
- project_subtree(uuid, trash_at) as (
- values (starting_uuid, starting_trash_at)
- union
- select groups.uuid, LEAST(project_subtree.trash_at, groups.trash_at)
+ project_subtree(uuid, trash_at) as (
+ values (starting_uuid, starting_trash_at)
+ union
+ select groups.uuid, LEAST(project_subtree.trash_at, groups.trash_at)
from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid)
- )
- select uuid, trash_at from project_subtree;
+ )
+ select uuid, trash_at from project_subtree;
$$;
}
@@ -112,6 +105,53 @@ select ps.target_uuid as group_uuid, ps.trash_at from groups,
lateral project_subtree_with_trash_at(groups.uuid, groups.trash_at) ps
where groups.owner_uuid like '_____-tpzed-_______________'
$$;
+}
+
+ ActiveRecord::Base.connection.execute("INSERT INTO trashed_groups select * from compute_trashed()")
+
+ # Get a set of permission by searching the graph and following
+ # ownership and permission links.
+ #
+ # edges() - a subselect with the union of ownership and permission links
+ #
+ # traverse_graph() - recursive query, from the starting node,
+ # self-join with edges to find outgoing permissions.
+ # Re-runs the query on new rows until there are no more results.
+ # This accomplishes a breadth-first search of the permission graph.
+ #
+ ActiveRecord::Base.connection.execute %{
+create or replace function search_permission_graph (starting_uuid varchar(27), starting_perm integer)
+returns table (target_uuid varchar(27), val integer, traverse_owned bool)
+STABLE
+language SQL
+as $$
+WITH RECURSIVE edges(tail_uuid, head_uuid, val) as (
+ select groups.owner_uuid, groups.uuid, (3) from groups
+ union
+ select links.tail_uuid,
+ links.head_uuid,
+ CASE
+ WHEN links.name = 'can_read' THEN 1
+ WHEN links.name = 'can_login' THEN 1
+ WHEN links.name = 'can_write' THEN 2
+ WHEN links.name = 'can_manage' THEN 3
+ END as val
+ from links
+ where links.link_class='permission'
+ ),
+ traverse_graph(target_uuid, val, traverse_owned) as (
+ values (starting_uuid, starting_perm, true)
+ union
+ (select edges.head_uuid,
+ least(edges.val, traverse_graph.val),
+ (edges.head_uuid like '_____-j7d0g-_______________' or
+ (edges.head_uuid like '_____-tpzed-_______________' and edges.val >= 3))
+ from edges
+ join traverse_graph on (traverse_graph.target_uuid = edges.tail_uuid)
+ where traverse_graph.traverse_owned))
+ select target_uuid, max(val), bool_or(traverse_owned) from traverse_graph
+ group by (target_uuid) ;
+$$;
}
ActiveRecord::Base.connection.execute "DROP MATERIALIZED VIEW IF EXISTS materialized_permission_view;"
diff --git a/services/api/lib/refresh_permission_view.rb b/services/api/lib/refresh_permission_view.rb
index 5ee8a7b44..f6642192a 100644
--- a/services/api/lib/refresh_permission_view.rb
+++ b/services/api/lib/refresh_permission_view.rb
@@ -8,8 +8,24 @@ TRASHED_GROUPS = "trashed_groups"
def do_refresh_permission_view
ActiveRecord::Base.transaction do
ActiveRecord::Base.connection.execute("LOCK TABLE permission_refresh_lock")
+ # ActiveRecord::Base.connection.execute("DELETE FROM #{PERMISSION_VIEW}")
+ # ActiveRecord::Base.connection.execute("INSERT INTO #{PERMISSION_VIEW} select * from compute_permission_table()")
+
+ # puts "do_refresh_permission_view1"
+ # ActiveRecord::Base.connection.exec_query("select * from materialized_permissions order by user_uuid, target_uuid, perm_level").each do |r|
+ # puts "d: #{r}"
+ # end
+
ActiveRecord::Base.connection.execute("DELETE FROM #{PERMISSION_VIEW}")
- ActiveRecord::Base.connection.execute("INSERT INTO #{PERMISSION_VIEW} select * from compute_permission_table()")
+ ActiveRecord::Base.connection.execute %{
+INSERT INTO #{PERMISSION_VIEW}
+select users.uuid, g.target_uuid, g.val, g.traverse_owned
+from users, lateral search_permission_graph(users.uuid, 3) as g
+}
+ # puts "do_refresh_permission_view2"
+ # ActiveRecord::Base.connection.exec_query("select * from materialized_permissions order by user_uuid, target_uuid, perm_level").each do |r|
+ # puts "d: #{r}"
+ # end
end
end
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index a5bd142b9..cee618557 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -61,6 +61,7 @@ class ActiveSupport::TestCase
include CurrentApiClient
setup do
+ do_refresh_permission_view
ActiveRecord::Base.connection.execute("DELETE FROM #{TRASHED_GROUPS}")
ActiveRecord::Base.connection.execute("INSERT INTO #{TRASHED_GROUPS} select * from compute_trashed()")
end
commit 6431ee2e2cb4267de7b51cf4a52f23cb14cdc2a3
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue May 5 16:58:44 2020 -0400
16007: New unified method for updating trash status of groups
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 551201773..3292642d4 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -39,24 +39,38 @@ class Group < ArvadosModel
end
def maybe_invalidate_permissions_cache
- if is_trashed_changed? or owner_uuid_changed?
- if is_trashed == true
- ActiveRecord::Base.connection.exec_query %{
-insert into trashed_groups (group_uuid, trash_at)
- select target_uuid as group_uuid, $2 as trash_at from project_subtree($1);
-},
- 'Group.trash_subtree',
- [[nil, self.uuid],
- [nil, self.trash_at]]
- elsif is_trashed == false && TrashedGroup.find_by_group_uuid(self.owner_uuid).nil?
- ActiveRecord::Base.connection.exec_query %{
-delete from trashed_groups where group_uuid in (select * from project_subtree_notrash($1));
+ if trash_at_changed? or owner_uuid_changed?
+ # The group was added or removed from the trash.
+ #
+ # Strategy:
+ # Determine the time this goes in the trash
+ # (or null, if it does not belong in the trash)
+ # Compute subtree to determine the time each subproject goes
+ # in the trash
+ # Remove groups that don't belong from trash
+ # Add/update groups that do belong in the trash
+
+ temptable = "group_subtree_#{rand(2**64).to_s(10)}"
+ ActiveRecord::Base.connection.exec_query %{
+create temporary table #{temptable} on commit drop
+as select * from project_subtree_with_trash_at($1, LEAST($2, $3)::timestamp)
},
- 'Group.untrash_subtree',
- [[nil, self.uuid]]
- end
+ 'Group.get_subtree',
+ [[nil, self.uuid],
+ [nil, TrashedGroup.find_by_group_uuid(self.owner_uuid).andand.trash_at],
+ [nil, self.trash_at]]
+
+ ActiveRecord::Base.connection.exec_query %{
+delete from trashed_groups where group_uuid in (select target_uuid from #{temptable} where trash_at is NULL);
+}
+
+ ActiveRecord::Base.connection.exec_query %{
+insert into trashed_groups (group_uuid, trash_at)
+ select target_uuid as group_uuid, trash_at from #{temptable} where trash_at is not NULL
+on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
+}
end
- if uuid_changed? or owner_uuid_changed? or is_trashed_changed?
+ if uuid_changed? or owner_uuid_changed?
# This can change users' permissions on other groups as well as
# this one.
invalidate_permissions_cache
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 9bd11c2c1..20b715df4 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -80,19 +80,19 @@ $$;
}
ActiveRecord::Base.connection.execute %{
-create or replace function project_subtree_notrash (starting_uuid varchar(27))
-returns table (target_uuid varchar(27))
+create or replace function project_subtree_with_trash_at (starting_uuid varchar(27), starting_trash_at timestamp)
+returns table (target_uuid varchar(27), trash_at timestamp)
STABLE
language SQL
as $$
WITH RECURSIVE
- project_subtree(uuid) as (
- values (starting_uuid)
+ project_subtree(uuid, trash_at) as (
+ values (starting_uuid, starting_trash_at)
union
- select groups.uuid from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid)
- where groups.is_trashed=false
+ select groups.uuid, LEAST(project_subtree.trash_at, groups.trash_at)
+ from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid)
)
- select uuid from project_subtree;
+ select uuid, trash_at from project_subtree;
$$;
}
@@ -100,7 +100,7 @@ $$;
t.string :group_uuid
t.datetime :trash_at
end
- add_index :trashed_groups, :group_uuid
+ add_index :trashed_groups, :group_uuid, :unique => true
ActiveRecord::Base.connection.execute %{
create or replace function compute_trashed ()
@@ -108,9 +108,9 @@ returns table (uuid varchar(27), trash_at timestamp)
STABLE
language SQL
as $$
-select ps.target_uuid as group_uuid, groups.trash_at from groups,
- lateral project_subtree(groups.uuid) ps
- where groups.is_trashed = true
+select ps.target_uuid as group_uuid, ps.trash_at from groups,
+ lateral project_subtree_with_trash_at(groups.uuid, groups.trash_at) ps
+ where groups.owner_uuid like '_____-tpzed-_______________'
$$;
}
@@ -123,7 +123,7 @@ $$;
ActiveRecord::Base.connection.execute "DROP function compute_permission_table ();"
ActiveRecord::Base.connection.execute "DROP function project_subtree (varchar(27));"
- ActiveRecord::Base.connection.execute "DROP function project_subtree_notrash (varchar(27));"
+ ActiveRecord::Base.connection.execute "DROP function project_subtree_with_trash_at (varchar(27), timestamp);"
ActiveRecord::Base.connection.execute "DROP function compute_trashed ();"
end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list