From 5f915857cbb3620587f321514a065a73fd6ecc46 Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Mon, 4 May 2020 16:22:59 -0400 Subject: [PATCH] 16007: Separating 'trashed' from 'permissions' WIP Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- services/api/app/models/arvados_model.rb | 8 +-- services/api/app/models/group.rb | 15 ++++++ .../20200501150153_permission_table.rb | 50 +++++++++++++++++++ services/api/lib/refresh_permission_view.rb | 3 ++ .../arvados/v1/groups_controller_test.rb | 14 +++++- 5 files changed, 85 insertions(+), 5 deletions(-) diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index 816dbf4758..12f729813b 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 1f2b0d8b77..dbe2d4ed57 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 00ec5b19b2..87c8a4cb86 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 2a6eb3f655..9190796678 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 30ab89c7e2..2b5e8d5a9d 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], -- 2.30.2