From: Ward Vandewege Date: Wed, 3 Mar 2021 01:07:23 +0000 (-0500) Subject: 17119: add second test, simplify code by switching to the generic X-Git-Tag: 2.2.0~84^2~68 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/35d5c737656b1ab7c2bdda27de3f67332be831c0 17119: add second test, simplify code by switching to the generic /groups/content endpoint. Arvados-DCO-1.1-Signed-off-by: Ward Vandewege --- diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go index 6941e1487c..b61dbd1b32 100644 --- a/lib/controller/rpc/conn.go +++ b/lib/controller/rpc/conn.go @@ -483,6 +483,8 @@ func (conn *Conn) GroupContents(ctx context.Context, options arvados.GroupConten options.Filters = append(options.Filters, filter) } } + // Use the generic /groups/contents endpoint for filter groups + options.UUID = "" } ep := arvados.EndpointGroupContents diff --git a/sdk/go/arvados/fs_project_test.go b/sdk/go/arvados/fs_project_test.go index 56a2bb8264..9d77c31d85 100644 --- a/sdk/go/arvados/fs_project_test.go +++ b/sdk/go/arvados/fs_project_test.go @@ -39,10 +39,9 @@ func (sc *spyingClient) RequestAndDecode(dst interface{}, method, path string, b return sc.Client.RequestAndDecode(dst, method, path, body, params) } -// TestFilterGroup is a very basic filter group test. It makes sure that a -// collection and group that match the filter are present, and that a group -// that does not match the filter is not present. func (s *SiteFSSuite) TestFilterGroup(c *check.C) { + // Make sure that a collection and group that match the filter are present, + // and that a group that does not match the filter is not present. s.fs.MountProject("fg", fixtureThisFilterGroupUUID) _, err := s.fs.OpenFile("/fg/baz_file", 0, 0) @@ -53,6 +52,18 @@ func (s *SiteFSSuite) TestFilterGroup(c *check.C) { _, err = s.fs.OpenFile("/fg/A Project", 0, 0) c.Assert(err, check.Not(check.IsNil)) + + // An empty filter means everything that is visible should be returned. + s.fs.MountProject("fg2", fixtureAFilterGroupTwoUUID) + + _, err = s.fs.OpenFile("/fg2/baz_file", 0, 0) + c.Assert(err, check.IsNil) + + _, err = s.fs.OpenFile("/fg2/A Subproject", 0, 0) + c.Assert(err, check.IsNil) + + _, err = s.fs.OpenFile("/fg2/A Project", 0, 0) + c.Assert(err, check.IsNil) } func (s *SiteFSSuite) TestCurrentUserHome(c *check.C) { diff --git a/sdk/go/arvados/fs_site_test.go b/sdk/go/arvados/fs_site_test.go index 9245023bab..02f21ded56 100644 --- a/sdk/go/arvados/fs_site_test.go +++ b/sdk/go/arvados/fs_site_test.go @@ -19,6 +19,7 @@ const ( fixtureActiveToken = "3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi" fixtureAProjectUUID = "zzzzz-j7d0g-v955i6s2oi1cbso" fixtureThisFilterGroupUUID = "zzzzz-j7d0g-thisfiltergroup" + fixtureAFilterGroupTwoUUID = "zzzzz-j7d0g-afiltergrouptwo" fixtureFooAndBarFilesInDirUUID = "zzzzz-4zz18-foonbarfilesdir" fixtureFooCollectionName = "zzzzz-4zz18-fy296fx3hot09f7 added sometime" fixtureFooCollectionPDH = "1f4b0bc7583c2a7f9102c395f4ffc5e3+45" diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb index 79d7b2fb54..394b5603b7 100644 --- a/services/api/app/controllers/arvados/v1/groups_controller.rb +++ b/services/api/app/controllers/arvados/v1/groups_controller.rb @@ -227,30 +227,12 @@ class Arvados::V1::GroupsController < ApplicationController end end - # filter groups need to be limited to those classes mentioned in the filters - # @object can also be a User object (virtual home project) - if @object and @object.is_a?(Group) and @object.group_class == "filter" - if request_filters.length() == 0 - raise ArgumentError.new("Filter group needs to have filters defined") - end - request_filters.each do |col, op, val| - if col.index('.') - col = col.split('.')[0] - col = col.capitalize.sub(/s$/,'') - wanted_klasses << col - end - end - end - filter_by_owner = {} if @object - # filter groups should not have an owner_uuid filter applied - if ! @object.is_a?(Group) or (@object.is_a?(Group) and @object.group_class != "filter") - if params['recursive'] - filter_by_owner[:owner_uuid] = [@object.uuid] + @object.descendant_project_uuids - else - filter_by_owner[:owner_uuid] = @object.uuid - end + if params['recursive'] + filter_by_owner[:owner_uuid] = [@object.uuid] + @object.descendant_project_uuids + else + filter_by_owner[:owner_uuid] = @object.uuid end if params['exclude_home_project'] diff --git a/services/api/lib/fix_roles_projects.rb b/services/api/lib/fix_roles_projects.rb index 5bb013c9ad..79fea45901 100644 --- a/services/api/lib/fix_roles_projects.rb +++ b/services/api/lib/fix_roles_projects.rb @@ -13,8 +13,8 @@ def fix_roles_projects # shouldn't be anything to do at all. act_as_system_user do ActiveRecord::Base.transaction do - Group.where("group_class != 'project' or group_class is null").each do |g| - # 1) any group not group_class != project becomes a 'role' (both empty and invalid groups) + Group.where("(group_class != 'project' and group_class != 'filter') or group_class is null").each do |g| + # 1) any group not group_class != project and != filter becomes a 'role' (both empty and invalid groups) old_owner = g.owner_uuid g.owner_uuid = system_user_uuid g.group_class = 'role' diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml index bf0641891e..da20f8be9e 100644 --- a/services/api/test/fixtures/groups.yml +++ b/services/api/test/fixtures/groups.yml @@ -120,6 +120,19 @@ afiltergroup: properties: filters: [[ "collections.name", "like", "baz%" ], [ "groups.name", "=", "A Subproject" ]] +afiltergroup2: + uuid: zzzzz-j7d0g-afiltergrouptwo + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2014-04-21 15:37:48 -0400 + modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr + modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz + modified_at: 2014-04-21 15:37:48 -0400 + updated_at: 2014-04-21 15:37:48 -0400 + name: A filter group without filters + group_class: filter + properties: + filters: [] + future_project_viewing_group: uuid: zzzzz-j7d0g-futrprojviewgrp owner_uuid: zzzzz-tpzed-000000000000000