[ARVADOS] updated: 1.3.0-2670-g98ec1f009
Git user
git at public.arvados.org
Tue Jun 16 19:53:01 UTC 2020
Summary of changes:
.../db/migrate/20200501150153_permission_table.rb | 34 +++++++++++++---------
services/api/db/structure.sql | 8 +++++
services/api/lib/update_permissions.rb | 5 ++++
3 files changed, 34 insertions(+), 13 deletions(-)
via 98ec1f0093bb097f9ccb78ac43a9858f20084ad6 (commit)
from b612ef0640ea45f03ad43ed4b124be1034d21071 (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 98ec1f0093bb097f9ccb78ac43a9858f20084ad6
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Jun 16 15:52:43 2020 -0400
16007: Update comments to discuss edge_id
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 0031bd766..4f9ea156d 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -18,7 +18,6 @@ class PermissionTable < ActiveRecord::Migration[5.0]
# permissions system. Updating trashed items follows a similar
# (but less complicated) strategy to updating permissions, so it
# may be helpful to look at that first.
- #
ActiveRecord::Base.connection.execute "DROP MATERIALIZED VIEW IF EXISTS materialized_permission_view;"
drop_table :permission_refresh_lock
@@ -99,12 +98,13 @@ $$;
}
# Merge all permission relationships into a single view. This
- # consists of: groups (projects) owning things, users owning
- # things, users owning themselves, and explicit permission links.
+ # consists of: groups owned by users and projects, users owned
+ # by other users, users have permission on themselves,
+ # and explicit permission links.
#
# A SQL view gets inlined into the query where it is used as a
# subquery. This enables the query planner to inject constraints,
- # so we only look up edges we plan to traverse and avoid a brute
+ # so it only has to look up edges it plans to traverse and avoid a brute
# force query of all edges.
ActiveRecord::Base.connection.execute %{
create view permission_graph_edges as
@@ -131,9 +131,9 @@ union all
where links.link_class='permission'
}
- # Code fragment that is used below. This is used to ensure that
- # the permission edge passed into compute_permission_subgraph
- # takes precedence over an existing edge in the "edges" view.
+ # This is used to ensure that the permission edge passed into
+ # compute_permission_subgraph takes replaces the existing edge in
+ # the "edges" view that is about to be removed.
edge_perm = %{
case (edges.edge_id = perm_edge_id)
when true then starting_perm
@@ -141,13 +141,13 @@ case (edges.edge_id = perm_edge_id)
end
}
- #
# The primary function to compute permissions for a subgraph.
- # This originally was organized somewhat more cleanly, but this
- # ran into performance issues due to the query optimizer not
- # working across function and "with" expression boundaries. So I
- # had to fall back on using string templates for the repeated
- # code. I'm sorry.
+ # Comments on how it works are inline.
+ #
+ # Due to performance issues due to the query optimizer not
+ # working across function and "with" expression boundaries, I
+ # had to fall back on using string templates for repeated code
+ # in order to inline it.
ActiveRecord::Base.connection.execute %{
create or replace function compute_permission_subgraph (perm_origin_uuid varchar(27),
@@ -172,6 +172,14 @@ as $$
starting_uuid One of 1, 2, 3 for can_read,
can_write, can_manage respectively, or 0 to revoke
permissions.
+
+ perm_edge_id: Identifies the permission edge that is being updated.
+ Changes of ownership, this is starting_uuid.
+ For links, this is the uuid of the link object.
+ This is used to override the edge value in the database
+ with starting_perm. This is necessary when revoking
+ permissions because the update happens before edge is
+ actually removed.
*/
with
/* Starting from starting_uuid, determine the set of objects that
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index affdf7e40..469475ad9 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -59,6 +59,14 @@ CREATE FUNCTION public.compute_permission_subgraph(perm_origin_uuid character va
starting_uuid One of 1, 2, 3 for can_read,
can_write, can_manage respectively, or 0 to revoke
permissions.
+
+ perm_edge_id: Identifies the permission edge that is being updated.
+ Changes of ownership, this is starting_uuid.
+ For links, this is the uuid of the link object.
+ This is used to override the edge value in the database
+ with starting_perm. This is necessary when revoking
+ permissions because the update happens before edge is
+ actually removed.
*/
with
/* Starting from starting_uuid, determine the set of objects that
diff --git a/services/api/lib/update_permissions.rb b/services/api/lib/update_permissions.rb
index d6e653772..4c2e72d95 100644
--- a/services/api/lib/update_permissions.rb
+++ b/services/api/lib/update_permissions.rb
@@ -50,6 +50,11 @@ def update_permissions perm_origin_uuid, starting_uuid, perm_level, edge_id=nil
# how the permissions are computed.
if edge_id.nil?
+ # For changes of ownership, edge_id is starting_uuid. In turns
+ # out most invocations of update_permissions are for changes of
+ # ownership, so make this parameter optional to reduce
+ # clutter.
+ # For permission links, the uuid of the link object will be passed in for edge_id.
edge_id = starting_uuid
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list