[ARVADOS] updated: dfcbdd5823bf85a462305a58df79a0112d3020d8
Git user
git at public.curoverse.com
Thu Aug 31 14:53:59 EDT 2017
Summary of changes:
.../arvados/v1/collections_controller.rb | 2 +-
services/api/app/models/arvados_model.rb | 22 +++++++++++++++-------
.../arvados/v1/collections_controller_test.rb | 17 +++++++++++++++--
.../arvados/v1/groups_controller_test.rb | 16 ++++++++++++++++
4 files changed, 47 insertions(+), 10 deletions(-)
via dfcbdd5823bf85a462305a58df79a0112d3020d8 (commit)
from 1b077e159d6e0b4b299a98fe4ac29f8ab1ed5efd (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 dfcbdd5823bf85a462305a58df79a0112d3020d8
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Aug 31 14:50:29 2017 -0400
12032: Bug fix so include_trash still respects permissions.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 0312d95..3803d37 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -28,7 +28,7 @@ class Arvados::V1::CollectionsController < ApplicationController
def find_objects_for_index
if params[:include_trash] || ['destroy', 'trash', 'untrash'].include?(action_name)
- @objects = Collection.unscoped.readable_by(*@read_users)
+ @objects = Collection.readable_by(*@read_users, {include_trash: true, query_on: Collection.unscoped})
end
super
end
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 4a4f7ec..d656970 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -282,29 +282,37 @@ class ArvadosModel < ActiveRecord::Base
# Can read object (evidently a group or user) whose UUID is listed
# explicitly in user_uuids.
sql_conds += ["#{sql_table}.uuid IN (:user_uuids)"]
- trashed_check = if include_trash then "EXISTS" else "0 IN " end
+ if include_trash
+ not_trashed = ""
+ not_null = "IS NOT NULL"
+ else
+ not_trashed = "0 IN"
+ not_null = ""
+ end
+
perm_check = "perm_level >= 1"
if self.column_names.include? "owner_uuid"
# to be readable:
# row(s) exists granting readable permission to uuid or owner, and none are trashed
- sql_conds += ["#{trashed_check}(SELECT MAX(trashed) FROM permission_view "+
+ sql_conds += ["#{not_trashed} (SELECT MAX(trashed) FROM permission_view "+
"WHERE user_uuid IN (:user_uuids) AND #{perm_check} AND "+
- "(#{sql_table}.uuid = target_uuid OR (#{sql_table}.owner_uuid = target_uuid AND target_owner_uuid is NOT NULL)))"]
+ "(#{sql_table}.uuid = target_uuid OR (#{sql_table}.owner_uuid = target_uuid "+
+ "AND target_owner_uuid IS NOT NULL))) #{not_null}"]
# reader is owner, and item is not trashed
- not_trashed_check = if include_trash then
+ not_trashed_clause = if include_trash then
""
else
"AND 1 NOT IN (SELECT MAX(trashed) FROM permission_view "+
"WHERE #{sql_table}.uuid = target_uuid OR #{sql_table}.owner_uuid = target_uuid)"
end
- sql_conds += ["(#{sql_table}.owner_uuid IN (:user_uuids) #{not_trashed_check})"]
+ sql_conds += ["(#{sql_table}.owner_uuid IN (:user_uuids) #{not_trashed_clause})"]
else
# to be readable:
# * a non-trash row exists with readable permission uuid
- sql_conds += ["#{trashed_check}(SELECT MAX(trashed) FROM permission_view "+
+ sql_conds += ["#{not_trashed} (SELECT MAX(trashed) FROM permission_view "+
"WHERE user_uuid IN (:user_uuids) AND #{perm_check} AND "+
- "#{sql_table}.uuid = target_uuid) "]
+ "#{sql_table}.uuid = target_uuid) #{not_null}"]
end
if sql_table == "links"
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index 4cf8778..47f0887 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1112,7 +1112,7 @@ EOS
test 'cannot index collection in trashed subproject' do
authorize_with :active
- get :index
+ get :index, { limit: 1000 }
assert_response :success
item_uuids = json_response['items'].map do |item|
item['uuid']
@@ -1123,7 +1123,20 @@ EOS
test 'can index collection in untrashed subproject' do
authorize_with :active
Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
- get :index
+ get :index, { limit: 1000 }
+ assert_response :success
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ assert_includes(item_uuids, collections(:collection_in_trashed_subproject).uuid)
+ end
+
+ test 'can index trashed subproject collection with include_trash' do
+ authorize_with :active
+ get :index, {
+ include_trash: true,
+ limit: 1000
+ }
assert_response :success
item_uuids = json_response['items'].map do |item|
item['uuid']
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 5db8475..3687819 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -689,5 +689,21 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_response :success
assert_match /^trashed subproject 3 \(\d{4}-\d\d-\d\d.*?Z\)$/, json_response['name']
end
+
+ test "move trashed subproject to new owner #{auth}" do
+ authorize_with auth
+ put :update, {
+ id: groups(:trashed_subproject).uuid,
+ group: {
+ owner_uuid: users(:active).uuid
+ },
+ include_trashed: true,
+ format: :json,
+ }
+ # Currently fails but might want to change it in the future
+ # so leave the test here.
+ assert_response 404
+ end
+
end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list