[ARVADOS] updated: 1.3.0-2532-ge77609d17
Git user
git at public.arvados.org
Mon May 4 21:32:48 UTC 2020
Summary of changes:
services/api/app/models/arvados_model.rb | 8 +-
services/api/app/models/group.rb | 17 +++++
services/api/app/models/user.rb | 8 +-
.../db/migrate/20200501150153_permission_table.rb | 85 +++++++++++++++++-----
services/api/lib/refresh_permission_view.rb | 1 +
.../arvados/v1/groups_controller_test.rb | 14 +++-
services/api/test/test_helper.rb | 5 ++
7 files changed, 109 insertions(+), 29 deletions(-)
via e77609d17994548cd5abed7a0d25517c10c73327 (commit)
via 64fc7012a9e0c28e441415759d780e1a78b26e67 (commit)
via 96cc61430cdaa88983365ebaf87cbca4e396272d (commit)
via 12e896874595d4b78ec61f931772c391e8c7973d (commit)
via 5f915857cbb3620587f321514a065a73fd6ecc46 (commit)
from dbd795d39343d2c91db1602603002b2a604f1947 (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 e77609d17994548cd5abed7a0d25517c10c73327
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon May 4 17:27:15 2020 -0400
16007: Remember to set trash_at in trashed_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 f64a5b259..551201773 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -42,10 +42,12 @@ class Group < ArvadosModel
if is_trashed_changed? or owner_uuid_changed?
if is_trashed == true
ActiveRecord::Base.connection.exec_query %{
-insert into trashed_groups (group_uuid) select * from project_subtree($1);
+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.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));
commit 64fc7012a9e0c28e441415759d780e1a78b26e67
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon May 4 17:13:24 2020 -0400
16007: Update trashed_groups incrementally
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 531a2bbb9..f64a5b259 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -39,7 +39,7 @@ class Group < ArvadosModel
end
def maybe_invalidate_permissions_cache
- if is_trashed_changed?
+ if is_trashed_changed? or owner_uuid_changed?
if is_trashed == true
ActiveRecord::Base.connection.exec_query %{
insert into trashed_groups (group_uuid) select * from project_subtree($1);
diff --git a/services/api/lib/refresh_permission_view.rb b/services/api/lib/refresh_permission_view.rb
index 919079667..5ee8a7b44 100644
--- a/services/api/lib/refresh_permission_view.rb
+++ b/services/api/lib/refresh_permission_view.rb
@@ -10,8 +10,6 @@ def do_refresh_permission_view
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()")
- ActiveRecord::Base.connection.execute("DELETE FROM #{TRASHED_GROUPS}")
- ActiveRecord::Base.connection.execute("INSERT INTO #{TRASHED_GROUPS} select * from compute_trashed()")
end
end
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 5747a85cf..a5bd142b9 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -60,6 +60,11 @@ class ActiveSupport::TestCase
include ArvadosTestSupport
include CurrentApiClient
+ setup do
+ ActiveRecord::Base.connection.execute("DELETE FROM #{TRASHED_GROUPS}")
+ ActiveRecord::Base.connection.execute("INSERT INTO #{TRASHED_GROUPS} select * from compute_trashed()")
+ end
+
teardown do
Thread.current[:api_client_ip_address] = nil
Thread.current[:api_client_authorization] = nil
commit 96cc61430cdaa88983365ebaf87cbca4e396272d
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon May 4 16:59:06 2020 -0400
16007: Check trash_at timestamp
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 12f729813..83b9a6e10 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -298,14 +298,14 @@ class ArvadosModel < ApplicationRecord
# Only include records where the owner is not trashed
#sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
# "WHERE trashed = 1) #{exclude_trashed_records}"
- sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT * FROM trashed_groups) #{exclude_trashed_records}"
+ sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT group_uuid FROM trashed_groups where trash_at < clock_timestamp()) #{exclude_trashed_records}"
end
end
else
trashed_check = ""
if !include_trash then
#trashed_check = "AND trashed = 0"
- trashed_check = "AND target_uuid NOT IN (SELECT * FROM trashed_groups)"
+ trashed_check = "AND target_uuid NOT IN (SELECT group_uuid FROM trashed_groups where trash_at < clock_timestamp())"
end
# Note: it is possible to combine the direct_check and
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index dbe2d4ed5..531a2bbb9 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -42,13 +42,13 @@ class Group < ArvadosModel
if is_trashed_changed?
if is_trashed == true
ActiveRecord::Base.connection.exec_query %{
-insert into trashed_groups (uuid) select * from project_subtree($1);
+insert into trashed_groups (group_uuid) select * from project_subtree($1);
},
'Group.trash_subtree',
[[nil, self.uuid]]
- elsif is_trashed == false && TrashedGroup.find_by_uuid(self.owner_uuid).nil?
+ elsif is_trashed == false && TrashedGroup.find_by_group_uuid(self.owner_uuid).nil?
ActiveRecord::Base.connection.exec_query %{
-delete from trashed_groups where uuid in (select * from project_subtree_notrash($1));
+delete from trashed_groups where group_uuid in (select * from project_subtree_notrash($1));
},
'Group.untrash_subtree',
[[nil, self.uuid]]
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index e1d0ab1b3..9bd11c2c1 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -97,16 +97,18 @@ $$;
}
create_table :trashed_groups, :id => false do |t|
- t.string :uuid
+ t.string :group_uuid
+ t.datetime :trash_at
end
+ add_index :trashed_groups, :group_uuid
ActiveRecord::Base.connection.execute %{
create or replace function compute_trashed ()
-returns table (uuid varchar(27))
+returns table (uuid varchar(27), trash_at timestamp)
STABLE
language SQL
as $$
-select ps.target_uuid from groups,
+select ps.target_uuid as group_uuid, groups.trash_at from groups,
lateral project_subtree(groups.uuid) ps
where groups.is_trashed = true
$$;
@@ -118,5 +120,10 @@ $$;
def down
drop_table :materialized_permissions
drop_table :trashed_groups
+
+ 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 compute_trashed ();"
end
end
commit 12e896874595d4b78ec61f931772c391e8c7973d
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon May 4 16:34:24 2020 -0400
16007: Remove 'trashed' from permission computation
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index dd447ca51..a5fc12d0a 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -155,12 +155,12 @@ class User < ArvadosModel
def self.all_group_permissions
all_perms = {}
ActiveRecord::Base.connection.
- exec_query("SELECT user_uuid, target_owner_uuid, perm_level, trashed
+ exec_query("SELECT user_uuid, target_owner_uuid, perm_level
FROM #{PERMISSION_VIEW}
WHERE target_owner_uuid IS NOT NULL",
# "name" arg is a query label that appears in logs:
"all_group_permissions",
- ).rows.each do |user_uuid, group_uuid, max_p_val, trashed|
+ ).rows.each do |user_uuid, group_uuid, max_p_val|
all_perms[user_uuid] ||= {}
all_perms[user_uuid][group_uuid] = PERMS_FOR_VAL[max_p_val.to_i]
end
@@ -173,7 +173,7 @@ 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, trashed
+ exec_query("SELECT target_owner_uuid, perm_level
FROM #{PERMISSION_VIEW}
WHERE user_uuid = $1
AND target_owner_uuid IS NOT NULL",
@@ -181,7 +181,7 @@ class User < ArvadosModel
"group_permissions for #{uuid}",
# "binds" arg is an array of [col_id, value] for '$1' vars:
[[nil, uuid]],
- ).rows.each do |group_uuid, max_p_val, trashed|
+ ).rows.each do |group_uuid, max_p_val|
group_perms[group_uuid] = PERMS_FOR_VAL[max_p_val.to_i]
end
group_perms
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 87c8a4cb8..e1d0ab1b3 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -5,7 +5,6 @@ class PermissionTable < ActiveRecord::Migration[5.0]
t.string :target_uuid
t.integer :perm_level
t.string :target_owner_uuid
- t.integer :trashed
end
ActiveRecord::Base.connection.execute %{
@@ -13,20 +12,17 @@ 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),
- trashed smallint)
+ target_owner_uuid character varying(27))
VOLATILE
language SQL
as $$
WITH RECURSIVE perm_value(name, val) AS (
VALUES ('can_read'::text,(1)::smallint), ('can_login'::text,1), ('can_write'::text,2), ('can_manage'::text,3)
- ), perm_edges(tail_uuid, head_uuid, val, follow, trashed) AS (
+ ), 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,
- (0)::smallint AS trashed,
- (0)::smallint AS followtrash
+ ((pv.val = 3) OR (groups.uuid IS NOT NULL)) AS follow
FROM ((public.links
LEFT JOIN perm_value pv ON ((pv.name = (links.name)::text)))
LEFT JOIN public.groups ON (((pv.val < 3) AND ((groups.uuid)::text = (links.head_uuid)::text))))
@@ -35,26 +31,19 @@ as $$
SELECT groups.owner_uuid,
groups.uuid,
3,
- true AS bool,
- CASE
- WHEN ((groups.trash_at IS NOT NULL) AND (groups.trash_at < clock_timestamp())) THEN 1
- ELSE 0
- END AS "case",
- 1
+ true AS bool
FROM public.groups
- ), perm(val, follow, user_uuid, target_uuid, trashed) AS (
+ ), perm(val, follow, user_uuid, target_uuid) AS (
SELECT (3)::smallint AS val,
true AS follow,
(users.uuid)::character varying(32) AS user_uuid,
- (users.uuid)::character varying(32) AS target_uuid,
- (0)::smallint AS trashed
+ (users.uuid)::character varying(32) AS target_uuid
FROM public.users
UNION
SELECT (LEAST((perm_1.val)::integer, edges.val))::smallint AS val,
edges.follow,
perm_1.user_uuid,
- (edges.head_uuid)::character varying(32) AS target_uuid,
- ((GREATEST((perm_1.trashed)::integer, edges.trashed) * edges.followtrash))::smallint AS trashed
+ (edges.head_uuid)::character varying(32) AS target_uuid
FROM (perm perm_1
JOIN perm_edges edges ON ((perm_1.follow AND ((edges.tail_uuid)::text = (perm_1.target_uuid)::text))))
)
@@ -64,8 +53,7 @@ as $$
CASE perm.follow
WHEN true THEN perm.target_uuid
ELSE NULL::character varying
- END AS target_owner_uuid,
- max(perm.trashed) AS trashed
+ END AS target_owner_uuid
FROM perm
GROUP BY perm.user_uuid, perm.target_uuid,
CASE perm.follow
commit 5f915857cbb3620587f321514a065a73fd6ecc46
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon May 4 16:22:59 2020 -0400
16007: Separating 'trashed' from 'permissions' WIP
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 816dbf475..12f729813 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -296,14 +296,16 @@ class ArvadosModel < ApplicationRecord
if !include_trash
if sql_table != "api_client_authorizations"
# Only include records where the owner is not trashed
- sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
- "WHERE trashed = 1) #{exclude_trashed_records}"
+ #sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
+ # "WHERE trashed = 1) #{exclude_trashed_records}"
+ sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT * FROM trashed_groups) #{exclude_trashed_records}"
end
end
else
trashed_check = ""
if !include_trash then
- trashed_check = "AND trashed = 0"
+ #trashed_check = "AND trashed = 0"
+ trashed_check = "AND target_uuid NOT IN (SELECT * FROM trashed_groups)"
end
# Note: it is possible to combine the direct_check and
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 1f2b0d8b7..dbe2d4ed5 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -39,6 +39,21 @@ class Group < ArvadosModel
end
def maybe_invalidate_permissions_cache
+ if is_trashed_changed?
+ if is_trashed == true
+ ActiveRecord::Base.connection.exec_query %{
+insert into trashed_groups (uuid) select * from project_subtree($1);
+},
+ 'Group.trash_subtree',
+ [[nil, self.uuid]]
+ elsif is_trashed == false && TrashedGroup.find_by_uuid(self.owner_uuid).nil?
+ ActiveRecord::Base.connection.exec_query %{
+delete from trashed_groups where uuid in (select * from project_subtree_notrash($1));
+},
+ 'Group.untrash_subtree',
+ [[nil, self.uuid]]
+ end
+ end
if uuid_changed? or owner_uuid_changed? or is_trashed_changed?
# This can change users' permissions on other groups as well as
# this one.
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 00ec5b19b..87c8a4cb8 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -73,6 +73,55 @@ as $$
ELSE NULL::character varying
END
$$;
+}
+
+ ActiveRecord::Base.connection.execute %{
+create or replace function project_subtree (starting_uuid varchar(27))
+returns table (target_uuid varchar(27))
+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;
+$$;
+}
+
+ ActiveRecord::Base.connection.execute %{
+create or replace function project_subtree_notrash (starting_uuid varchar(27))
+returns table (target_uuid varchar(27))
+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)
+ where groups.is_trashed=false
+ )
+ select uuid from project_subtree;
+$$;
+}
+
+ create_table :trashed_groups, :id => false do |t|
+ t.string :uuid
+ end
+
+ ActiveRecord::Base.connection.execute %{
+create or replace function compute_trashed ()
+returns table (uuid varchar(27))
+STABLE
+language SQL
+as $$
+select ps.target_uuid from groups,
+ lateral project_subtree(groups.uuid) ps
+ where groups.is_trashed = true
+$$;
}
ActiveRecord::Base.connection.execute "DROP MATERIALIZED VIEW IF EXISTS materialized_permission_view;"
@@ -80,5 +129,6 @@ $$;
end
def down
drop_table :materialized_permissions
+ drop_table :trashed_groups
end
end
diff --git a/services/api/lib/refresh_permission_view.rb b/services/api/lib/refresh_permission_view.rb
index 2a6eb3f65..919079667 100644
--- a/services/api/lib/refresh_permission_view.rb
+++ b/services/api/lib/refresh_permission_view.rb
@@ -3,12 +3,15 @@
# SPDX-License-Identifier: AGPL-3.0
PERMISSION_VIEW = "materialized_permissions"
+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()")
+ ActiveRecord::Base.connection.execute("DELETE FROM #{TRASHED_GROUPS}")
+ ActiveRecord::Base.connection.execute("INSERT INTO #{TRASHED_GROUPS} select * from compute_trashed()")
end
end
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 30ab89c7e..2b5e8d5a9 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -505,9 +505,19 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
### trashed project tests ###
- [:active, :admin].each do |auth|
+ #
+ # The structure is
+ #
+ # trashed_project (zzzzz-j7d0g-trashedproject1)
+ # trashed_subproject (zzzzz-j7d0g-trashedproject2)
+ # trashed_subproject3 (zzzzz-j7d0g-trashedproject3)
+ # zzzzz-xvhdp-cr5trashedcontr
+
+ [:active,
+ :admin].each do |auth|
# project: to query, to untrash, is visible, parent contents listing success
- [[:trashed_project, [], false, true],
+ [
+ [:trashed_project, [], false, true],
[:trashed_project, [:trashed_project], true, true],
[:trashed_subproject, [], false, false],
[:trashed_subproject, [:trashed_project], true, true],
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list