17119: implement review feedback.
authorWard Vandewege <ward@curii.com>
Tue, 23 Mar 2021 20:22:56 +0000 (16:22 -0400)
committerWard Vandewege <ward@curii.com>
Wed, 24 Mar 2021 01:13:30 +0000 (21:13 -0400)
Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward@curii.com>

doc/_config.yml
doc/api/methods/groups.html.textile.liquid
doc/api/permission-model.html.textile.liquid
doc/api/projects.html.textile.liquid [moved from doc/user/topics/projects.html.textile.liquid with 98% similarity]
lib/controller/localdb/conn.go
lib/controller/rpc/conn.go
sdk/go/arvadostest/api.go
services/api/app/models/group.rb

index f519d229e379110ce351d6f259f51f8ee2c3648a..d2b6d62e479c7ab72f73f36087dc92d3b9b21daa 100644 (file)
@@ -44,7 +44,6 @@ navbar:
       - user/tutorials/tutorial-keep-mount-os-x.html.textile.liquid
       - user/tutorials/tutorial-keep-mount-windows.html.textile.liquid
       - user/tutorials/tutorial-keep-collection-lifecycle.html.textile.liquid
-      - user/topics/projects.html.textile.liquid
       - user/topics/arv-copy.html.textile.liquid
       - user/topics/collection-versioning.html.textile.liquid
       - user/topics/storage-classes.html.textile.liquid
@@ -129,6 +128,7 @@ navbar:
       - api/keep-webdav.html.textile.liquid
       - api/keep-s3.html.textile.liquid
       - api/keep-web-urls.html.textile.liquid
+      - api/projects.html.textile.liquid
       - api/methods/collections.html.textile.liquid
       - api/methods/repositories.html.textile.liquid
     - Container engine:
index 4b63de5de18a4f87ed22ed8175b6a6d8fcc7e9e0..e4b8594dd195b7e7c4240d9628cc83065572f0f5 100644 (file)
@@ -25,7 +25,7 @@ Each Group has, in addition to the "Common resource fields":{{site.baseurl}}/api
 table(table table-bordered table-condensed).
 |_. Attribute|_. Type|_. Description|_. Example|
 |name|string|||
-|group_class|string|Type of group. @project@ and @filter@ indicate that the group should be displayed by Workbench and arv-mount as a project for organizing and naming objects. @role@ means FIXME. |@"filter"@
+|group_class|string|Type of group. @project@ and @filter@ indicate that the group should be displayed by Workbench and arv-mount as a project for organizing and naming objects. @role@ is used as part of the "permission system":{{site.baseurl}}/api/permission-model.html. |@"filter"@
 @"project"@
 @"role"@|
 |description|text|||
index f54dc8bf2be13587dedbf09d13f11670ad6b7fca..d9e16e047d125976bf5eb8bf852c6fca6d618d6c 100644 (file)
@@ -26,7 +26,7 @@ There are four levels of permission: *none*, *can_read*, *can_write*, and *can_m
 
 h2. Ownership
 
-All Arvados objects have an @owner_uuid@ field. Valid uuid types for @owner_uuid@ are "User" and "Group".  For Group, the @group_class@ must be "filter", "project" or "role".
+All Arvados objects have an @owner_uuid@ field. Valid uuid types for @owner_uuid@ are "User" and "Group".  In the case of a Group, the @group_class@ must be "project".
 
 The User or Group specified by @owner_uuid@ has *can_manage* permission on the object.  This permission is one way: an object that is owned does not get any special permissions on the User or Group that owns it.
 
@@ -67,8 +67,8 @@ A "project" is a subtype of Group that is displayed as a "Project" in Workbench,
 * Projects can be targets (@head_uuid@) of permission links, but not origins (@tail_uuid@).  Putting a project in a @tail_uuid@ field is an error.
 
 A "filter" is a subtype of Group that is displayed as a "Project" in Workbench, and as a directory by @arv-mount@. See "the groups API documentation":/api/methods/groups.html for more information.
-* A filter cannot own things (cannot appear in @owner_uuid@).  Putting a role in an @owner_uuid@ field is an error.
-* A filter can be owned by a user or another project.
+* A filter group cannot own things (cannot appear in @owner_uuid@).  Putting a role in an @owner_uuid@ field is an error.
+* A filter group can be owned by a user or a project.
 * The name of a filter is unique only among projects and filters with the same owner_uuid.
 * Filters can be targets (@head_uuid@) of permission links, but not origins (@tail_uuid@).  Putting a filter in a @tail_uuid@ field is an error.
 
similarity index 98%
rename from doc/user/topics/projects.html.textile.liquid
rename to doc/api/projects.html.textile.liquid
index a23ff528ae7d6335b13025dfb66fb7a61a4d401c..b1c74fe0d729794f26c2b535989f9dedf21c462b 100644 (file)
@@ -1,7 +1,7 @@
 ---
 layout: default
-navsection: userguide
-title: "Projects"
+navsection: api
+title: "Projects and filter groups"
 ...
 {% comment %}
 Copyright (C) The Arvados Authors. All rights reserved.
index d197675f8dc6e774d10427b11121a8f27e2c4823..c6b70ed2e98c7ec224a5aeea66d44611427ee62f 100644 (file)
@@ -6,6 +6,8 @@ package localdb
 
 import (
        "context"
+       "fmt"
+       "strings"
 
        "git.arvados.org/arvados.git/lib/controller/railsproxy"
        "git.arvados.org/arvados.git/lib/controller/rpc"
@@ -45,3 +47,49 @@ func (conn *Conn) Login(ctx context.Context, opts arvados.LoginOptions) (arvados
 func (conn *Conn) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) {
        return conn.loginController.UserAuthenticate(ctx, opts)
 }
+
+func (conn *Conn) GroupContents(ctx context.Context, options arvados.GroupContentsOptions) (arvados.ObjectList, error) {
+       // The requested UUID can be a user (virtual home project), which we just pass on to
+       // the API server.
+       if strings.Index(options.UUID, "j7d0g") != 6 {
+               return conn.railsProxy.GroupContents(ctx, options)
+       }
+
+       var resp arvados.ObjectList
+
+       // Get the group object
+       respGroup, err := conn.GroupGet(ctx, arvados.GetOptions{UUID: options.UUID})
+       if err != nil {
+               return resp, err
+       }
+
+       // If the group has groupClass 'filter', apply the filters before getting the contents.
+       if respGroup.GroupClass == "filter" {
+               if filters, ok := respGroup.Properties["filters"].([]interface{}); ok {
+                       for _, f := range filters {
+                               // f is supposed to be a []string
+                               tmp, ok2 := f.([]interface{})
+                               if !ok2 || len(tmp) < 3 {
+                                       return resp, fmt.Errorf("filter unparsable: %T, %+v, original field: %T, %+v\n", tmp, tmp, f, f)
+                               }
+                               var filter arvados.Filter
+                               if attr, ok2 := tmp[0].(string); ok2 {
+                                       filter.Attr = attr
+                               } else {
+                                       return resp, fmt.Errorf("filter unparsable: attribute must be string: %T, %+v, filter: %T, %+v\n", tmp[0], tmp[0], f, f)
+                               }
+                               if operator, ok2 := tmp[1].(string); ok2 {
+                                       filter.Operator = operator
+                               } else {
+                                       return resp, fmt.Errorf("filter unparsable: operator must be string: %T, %+v, filter: %T, %+v\n", tmp[1], tmp[1], f, f)
+                               }
+                               filter.Operand = tmp[2]
+                               options.Filters = append(options.Filters, filter)
+                       }
+               }
+               // Use the generic /groups/contents endpoint for filter groups
+               options.UUID = ""
+       }
+
+       return conn.railsProxy.GroupContents(ctx, options)
+}
index 9d64dc539af9a0697460284de8183a05e469f84b..61d20de78a824e869677e15f8c9937b69e9e4121 100644 (file)
@@ -23,7 +23,6 @@ import (
 
        "git.arvados.org/arvados.git/sdk/go/arvados"
        "git.arvados.org/arvados.git/sdk/go/auth"
-       "git.arvados.org/arvados.git/sdk/go/ctxlog"
        "git.arvados.org/arvados.git/sdk/go/httpserver"
 )
 
@@ -446,49 +445,9 @@ func (conn *Conn) GroupList(ctx context.Context, options arvados.ListOptions) (a
 }
 
 func (conn *Conn) GroupContents(ctx context.Context, options arvados.GroupContentsOptions) (arvados.ObjectList, error) {
-       // The requested UUID can be a user (virtual home project), which we just pass on to
-       // the API server.
-       if strings.Index(options.UUID, "j7d0g") != 6 {
-               ep := arvados.EndpointGroupContents
-               var resp arvados.ObjectList
-               err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
-               return resp, err
-       }
-
-       log := ctxlog.FromContext(ctx)
-       var resp arvados.ObjectList
-
-       // Get the group object
-       epGet := arvados.EndpointGroupGet
-       var respGroup arvados.Group
-       err := conn.requestAndDecode(ctx, &respGroup, epGet, nil, options)
-       if err != nil {
-               return resp, err
-       }
-
-       // If the group has groupClass 'filter', apply the filters before getting the contents.
-       if respGroup.GroupClass == "filter" {
-               if filters, ok := respGroup.Properties["filters"]; ok {
-                       for _, f := range filters.([]interface{}) {
-                               // f is supposed to be a []string
-                               tmp, ok2 := f.([]interface{})
-                               if !ok2 || len(tmp) < 3 {
-                                       log.Warnf("filter unparsable: %T, %+v, original field: %T, %+v\n", tmp, tmp, f, f)
-                                       continue
-                               }
-                               var filter arvados.Filter
-                               filter.Attr = tmp[0].(string)
-                               filter.Operator = tmp[1].(string)
-                               filter.Operand = tmp[2]
-                               options.Filters = append(options.Filters, filter)
-                       }
-               }
-               // Use the generic /groups/contents endpoint for filter groups
-               options.UUID = ""
-       }
-
        ep := arvados.EndpointGroupContents
-       err = conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       var resp arvados.ObjectList
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
        return resp, err
 }
 
index d9708e3b1aacd307185deee5644023d5791a8ff1..f255aeb2d085a9dbb4bc2fd2494daccf3f9d2605 100644 (file)
@@ -157,6 +157,10 @@ func (as *APIStub) GroupDelete(ctx context.Context, options arvados.DeleteOption
        as.appendCall(ctx, as.GroupDelete, options)
        return arvados.Group{}, as.Error
 }
+func (as *APIStub) GroupTrash(ctx context.Context, options arvados.DeleteOptions) (arvados.Group, error) {
+       as.appendCall(ctx, as.GroupTrash, options)
+       return arvados.Group{}, as.Error
+}
 func (as *APIStub) GroupUntrash(ctx context.Context, options arvados.UntrashOptions) (arvados.Group, error) {
        as.appendCall(ctx, as.GroupUntrash, options)
        return arvados.Group{}, as.Error
index 2243ae54dbdc9dbcce0a5c08a8a0dad4f196f033..fd2f5f18c2ac8018b152307042e82d587f8290a7 100644 (file)
@@ -60,6 +60,7 @@ class Group < ArvadosModel
   def check_filter_group_filters
     if group_class == 'filter'
       if !self.properties.key?("filters")
+        errors.add :properties, "filters property missing, it must be an array of arrays, each with 3 elements"
         return
       end
       if !self.properties["filters"].is_a?(Array)