From 6c7c27f15f02cac915023590b4dfde6c4fd0af0b Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Mon, 11 Dec 2023 10:22:30 -0500 Subject: [PATCH] 21214: Include filter groups in sitefs. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- sdk/go/arvados/fs_project.go | 4 +- sdk/go/arvados/fs_project_test.go | 83 ++++++++++++++++--------------- sdk/go/arvadostest/fixtures.go | 5 +- services/keep-web/handler_test.go | 37 ++++++++++++++ 4 files changed, 84 insertions(+), 45 deletions(-) diff --git a/sdk/go/arvados/fs_project.go b/sdk/go/arvados/fs_project.go index a68e83945e..ed9bb0c08d 100644 --- a/sdk/go/arvados/fs_project.go +++ b/sdk/go/arvados/fs_project.go @@ -38,7 +38,7 @@ func (fs *customFileSystem) projectsLoadOne(parent inode, uuid, name string) (in Filters: []Filter{ {"name", "=", strings.Replace(name, subst, "/", -1)}, {"uuid", "is_a", []string{"arvados#collection", "arvados#group"}}, - {"groups.group_class", "=", "project"}, + {"groups.group_class", "in", []string{"project", "filter"}}, }, Select: []string{"uuid", "name", "modified_at", "properties"}, }) @@ -104,7 +104,7 @@ func (fs *customFileSystem) projectsLoadAll(parent inode, uuid string) ([]inode, {"uuid", "is_a", class}, } if class == "arvados#group" { - filters = append(filters, Filter{"group_class", "=", "project"}) + filters = append(filters, Filter{"groups.group_class", "in", []string{"project", "filter"}}) } params := ResourceListParams{ diff --git a/sdk/go/arvados/fs_project_test.go b/sdk/go/arvados/fs_project_test.go index d3dac7a14f..5a675100f7 100644 --- a/sdk/go/arvados/fs_project_test.go +++ b/sdk/go/arvados/fs_project_test.go @@ -42,61 +42,62 @@ func (sc *spyingClient) RequestAndDecode(dst interface{}, method, path string, b 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) - c.Assert(err, check.IsNil) - _, err = s.fs.OpenFile("/fg/A Subproject", 0, 0) - c.Assert(err, check.IsNil) + checkOpen := func(path string, exists bool) { + f, err := s.fs.Open(path) + if exists { + if c.Check(err, check.IsNil) { + c.Check(f.Close(), check.IsNil) + } + } else { + c.Check(err, check.Equals, os.ErrNotExist) + } + } - _, err = s.fs.OpenFile("/fg/A Project", 0, 0) - c.Assert(err, check.Not(check.IsNil)) + checkOpen("/users/active/This filter group/baz_file", true) + checkOpen("/users/active/This filter group/A Subproject", true) + checkOpen("/users/active/This filter group/A Project", false) + s.fs.MountProject("fg", fixtureThisFilterGroupUUID) + checkOpen("/fg/baz_file", true) + checkOpen("/fg/A Subproject", true) + checkOpen("/fg/A Project", false) + s.fs.MountProject("home", "") + checkOpen("/home/A filter group with an is_a collection filter/baz_file", true) + checkOpen("/home/A filter group with an is_a collection filter/baz_file/baz", true) + checkOpen("/home/A filter group with an is_a collection filter/A Subproject", false) + checkOpen("/home/A filter group with an is_a collection filter/A Project", false) // An empty filter means everything that is visible should be returned. + checkOpen("/users/active/A filter group without filters/baz_file", true) + checkOpen("/users/active/A filter group without filters/A Subproject", true) + checkOpen("/users/active/A filter group without filters/A Project", true) 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) + checkOpen("/fg2/baz_file", true) + checkOpen("/fg2/A Subproject", true) + checkOpen("/fg2/A Project", true) // An 'is_a' 'arvados#collection' filter means only collections should be returned. + checkOpen("/users/active/A filter group with an is_a collection filter/baz_file", true) + checkOpen("/users/active/A filter group with an is_a collection filter/baz_file/baz", true) + checkOpen("/users/active/A filter group with an is_a collection filter/A Subproject", false) + checkOpen("/users/active/A filter group with an is_a collection filter/A Project", false) s.fs.MountProject("fg3", fixtureAFilterGroupThreeUUID) - - _, err = s.fs.OpenFile("/fg3/baz_file", 0, 0) - c.Assert(err, check.IsNil) - - _, err = s.fs.OpenFile("/fg3/A Subproject", 0, 0) - c.Assert(err, check.Not(check.IsNil)) + checkOpen("/fg3/baz_file", true) + checkOpen("/fg3/baz_file/baz", true) + checkOpen("/fg3/A Subproject", false) // An 'exists' 'arvados#collection' filter means only collections with certain properties should be returned. s.fs.MountProject("fg4", fixtureAFilterGroupFourUUID) - - _, err = s.fs.Stat("/fg4/collection with list property with odd values") - c.Assert(err, check.IsNil) - - _, err = s.fs.Stat("/fg4/collection with list property with even values") - c.Assert(err, check.IsNil) + checkOpen("/fg4/collection with list property with odd values", true) + checkOpen("/fg4/collection with list property with even values", true) + checkOpen("/fg4/baz_file", false) // A 'contains' 'arvados#collection' filter means only collections with certain properties should be returned. s.fs.MountProject("fg5", fixtureAFilterGroupFiveUUID) - - _, err = s.fs.Stat("/fg5/collection with list property with odd values") - c.Assert(err, check.IsNil) - - _, err = s.fs.Stat("/fg5/collection with list property with string value") - c.Assert(err, check.IsNil) - - _, err = s.fs.Stat("/fg5/collection with prop2 5") - c.Assert(err, check.Not(check.IsNil)) - - _, err = s.fs.Stat("/fg5/collection with list property with even values") - c.Assert(err, check.Not(check.IsNil)) + checkOpen("/fg5/collection with list property with odd values", true) + checkOpen("/fg5/collection with list property with string value", true) + checkOpen("/fg5/collection with prop2 5", false) + checkOpen("/fg5/collection with list property with even values", false) } func (s *SiteFSSuite) TestCurrentUserHome(c *check.C) { diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go index ac12f7ae13..3b8a618fea 100644 --- a/sdk/go/arvadostest/fixtures.go +++ b/sdk/go/arvadostest/fixtures.go @@ -37,8 +37,9 @@ const ( StorageClassesDesiredArchiveConfirmedDefault = "zzzzz-4zz18-3t236wr12769qqa" EmptyCollectionUUID = "zzzzz-4zz18-gs9ooj1h9sd5mde" - AProjectUUID = "zzzzz-j7d0g-v955i6s2oi1cbso" - ASubprojectUUID = "zzzzz-j7d0g-axqo7eu9pwvna1x" + AProjectUUID = "zzzzz-j7d0g-v955i6s2oi1cbso" + ASubprojectUUID = "zzzzz-j7d0g-axqo7eu9pwvna1x" + AFilterGroupUUID = "zzzzz-j7d0g-thisfiltergroup" FooAndBarFilesInDirUUID = "zzzzz-4zz18-foonbarfilesdir" FooAndBarFilesInDirPDH = "870369fc72738603c2fad16664e50e2d+58" diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go index c14789889d..42bfd80fa5 100644 --- a/services/keep-web/handler_test.go +++ b/services/keep-web/handler_test.go @@ -1105,6 +1105,17 @@ func (s *IntegrationSuite) TestDirectoryListingWithNoAnonymousToken(c *check.C) } func (s *IntegrationSuite) testDirectoryListing(c *check.C) { + // The "ownership cycle" test fixtures are reachable from the + // "filter group without filters" group, causing webdav's + // walkfs to recurse indefinitely. Avoid that by deleting one + // of the bogus fixtures. + arv := arvados.NewClientFromEnv() + err := arv.RequestAndDecode(nil, "DELETE", "arvados/v1/groups/zzzzz-j7d0g-cx2al9cqkmsf1hs", nil, nil) + if err != nil { + c.Assert(err, check.FitsTypeOf, &arvados.TransactionError{}) + c.Check(err.(*arvados.TransactionError).StatusCode, check.Equals, 404) + } + s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com" authHeader := http.Header{ "Authorization": {"OAuth2 " + arvadostest.ActiveToken}, @@ -1241,6 +1252,30 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) { expect: []string{"waz"}, cutDirs: 2, }, + { + uri: "download.example.com/users/active/This filter group/", + header: authHeader, + expect: []string{"A Subproject/"}, + cutDirs: 3, + }, + { + uri: "download.example.com/users/active/This filter group/A Subproject", + header: authHeader, + expect: []string{"baz_file/"}, + cutDirs: 4, + }, + { + uri: "download.example.com/by_id/" + arvadostest.AFilterGroupUUID, + header: authHeader, + expect: []string{"A Subproject/"}, + cutDirs: 2, + }, + { + uri: "download.example.com/by_id/" + arvadostest.AFilterGroupUUID + "/A Subproject", + header: authHeader, + expect: []string{"baz_file/"}, + cutDirs: 3, + }, } { comment := check.Commentf("HTML: %q => %q", trial.uri, trial.expect) resp := httptest.NewRecorder() @@ -1278,6 +1313,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) { } else { c.Check(resp.Code, check.Equals, http.StatusOK, comment) for _, e := range trial.expect { + e = strings.Replace(e, " ", "%20", -1) c.Check(resp.Body.String(), check.Matches, `(?ms).*href="./`+e+`".*`, comment) } c.Check(resp.Body.String(), check.Matches, `(?ms).*--cut-dirs=`+fmt.Sprintf("%d", trial.cutDirs)+` .*`, comment) @@ -1320,6 +1356,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) { } else { e = filepath.Join(u.Path, e) } + e = strings.Replace(e, " ", "%20", -1) c.Check(resp.Body.String(), check.Matches, `(?ms).*`+e+`.*`, comment) } } -- 2.30.2