17119: First take: add groups endpoints to controller.

Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward@curii.com>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index b86266d67e6f02c170deb631d32c777ecb072781..b43e79c3ff56c761ac749be1ad134e6ffade7ef9 100644 (file)
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -402,6 +402,38 @@ func (conn *Conn) ContainerRequestDelete(ctx context.Context, options arvados.De
        return conn.chooseBackend(options.UUID).ContainerRequestDelete(ctx, options)
 }
 
+func (conn *Conn) GroupCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Group, error) {
+       return conn.chooseBackend(options.ClusterID).GroupCreate(ctx, options)
+}
+
+func (conn *Conn) GroupUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.Group, error) {
+       return conn.chooseBackend(options.UUID).GroupUpdate(ctx, options)
+}
+
+func (conn *Conn) GroupGet(ctx context.Context, options arvados.GetOptions) (arvados.Group, error) {
+       return conn.chooseBackend(options.UUID).GroupGet(ctx, options)
+}
+
+func (conn *Conn) GroupList(ctx context.Context, options arvados.ListOptions) (arvados.GroupList, error) {
+       return conn.generated_GroupList(ctx, options)
+}
+
+func (conn *Conn) GroupContents(ctx context.Context, options arvados.ContentsOptions) (arvados.GroupList, error) {
+       return conn.chooseBackend(options.UUID).GroupContents(ctx, options)
+}
+
+func (conn *Conn) GroupShared(ctx context.Context, options arvados.SharedOptions) (arvados.GroupList, error) {
+       return conn.chooseBackend(options.UUID).GroupShared(ctx, options)
+}
+
+func (conn *Conn) GroupDelete(ctx context.Context, options arvados.DeleteOptions) (arvados.Group, error) {
+       return conn.chooseBackend(options.UUID).GroupDelete(ctx, options)
+}
+
+func (conn *Conn) GroupUntrash(ctx context.Context, options arvados.UntrashOptions) (arvados.Group, error) {
+       return conn.chooseBackend(options.UUID).GroupUntrash(ctx, options)
+}
+
 func (conn *Conn) SpecimenList(ctx context.Context, options arvados.ListOptions) (arvados.SpecimenList, error) {
        return conn.generated_SpecimenList(ctx, options)
 }

diff --git a/lib/controller/federation/generate.go b/lib/controller/federation/generate.go
index 9ce7fdcb21f6aa19b264b57982facaeee39305f9..6a2a4858560b84d01e14c3e5d170eb595d4ba512 100644 (file)
--- a/lib/controller/federation/generate.go
+++ b/lib/controller/federation/generate.go
@@ -52,7 +52,7 @@ func main() {
                defer out.Close()
                out.Write(regexp.MustCompile(`(?ms)^.*package .*?import.*?\n\)\n`).Find(buf))
                io.WriteString(out, "//\n// -- this file is auto-generated -- do not edit -- edit list.go and run \"go generate\" instead --\n//\n\n")
-               for _, t := range []string{"Container", "ContainerRequest", "Specimen", "User"} {
+               for _, t := range []string{"Container", "ContainerRequest", "Group", "Specimen", "User"} {
                        _, err := out.Write(bytes.ReplaceAll(orig, []byte("Collection"), []byte(t)))
                        if err != nil {
                                panic(err)

diff --git a/lib/controller/federation/generated.go b/lib/controller/federation/generated.go
index ab9db93a4d105a17572beee6080cd5b13f2755c9..49a2e5b7513f537f0f129fac6c8ba81103e72a29 100755 (executable)
--- a/lib/controller/federation/generated.go
+++ b/lib/controller/federation/generated.go
@@ -99,6 +99,47 @@ func (conn *Conn) generated_ContainerRequestList(ctx context.Context, options ar
        return merged, err
 }
 
+func (conn *Conn) generated_GroupList(ctx context.Context, options arvados.ListOptions) (arvados.GroupList, error) {
+       var mtx sync.Mutex
+       var merged arvados.GroupList
+       var needSort atomic.Value
+       needSort.Store(false)
+       err := conn.splitListRequest(ctx, options, func(ctx context.Context, _ string, backend arvados.API, options arvados.ListOptions) ([]string, error) {
+               options.ForwardedFor = conn.cluster.ClusterID + "-" + options.ForwardedFor
+               cl, err := backend.GroupList(ctx, options)
+               if err != nil {
+                       return nil, err
+               }
+               mtx.Lock()
+               defer mtx.Unlock()
+               if len(merged.Items) == 0 {
+                       merged = cl
+               } else if len(cl.Items) > 0 {
+                       merged.Items = append(merged.Items, cl.Items...)
+                       needSort.Store(true)
+               }
+               uuids := make([]string, 0, len(cl.Items))
+               for _, item := range cl.Items {
+                       uuids = append(uuids, item.UUID)
+               }
+               return uuids, nil
+       })
+       if needSort.Load().(bool) {
+               // Apply the default/implied order, "modified_at desc"
+               sort.Slice(merged.Items, func(i, j int) bool {
+                       mi, mj := merged.Items[i].ModifiedAt, merged.Items[j].ModifiedAt
+                       return mj.Before(mi)
+               })
+       }
+       if merged.Items == nil {
+               // Return empty results as [], not null
+               // (https://github.com/golang/go/issues/27589 might be
+               // a better solution in the future)
+               merged.Items = []arvados.Group{}
+       }
+       return merged, err
+}
+
 func (conn *Conn) generated_SpecimenList(ctx context.Context, options arvados.ListOptions) (arvados.SpecimenList, error) {
        var mtx sync.Mutex
        var merged arvados.SpecimenList

diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 5f6fb192e1731a75b9052e9096c9a04dab6ddd99..40f335e952d6d40820b5ac52d6c2b5ea92d9946c 100644 (file)
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -104,6 +104,8 @@ func (h *Handler) setup() {
                mux.Handle("/arvados/v1/connect/", rtr)
                mux.Handle("/arvados/v1/container_requests", rtr)
                mux.Handle("/arvados/v1/container_requests/", rtr)
+               mux.Handle("/arvados/v1/groups", rtr)
+               mux.Handle("/arvados/v1/groups/", rtr)
                mux.Handle("/login", rtr)
                mux.Handle("/logout", rtr)
        }

diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go
index 83c89d322ab3d85aa31fff177b92115efa469ed8..f5db9426b82f0ca63b1e9b1dfd49e5dc49534e0c 100644 (file)
--- a/lib/controller/router/router.go
+++ b/lib/controller/router/router.go
@@ -228,6 +228,62 @@ func (rtr *router) addRoutes() {
                                return rtr.backend.ContainerSSH(ctx, *opts.(*arvados.ContainerSSHOptions))
                        },
                },
+               {
+                       arvados.EndpointGroupCreate,
+                       func() interface{} { return &arvados.CreateOptions{} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.GroupCreate(ctx, *opts.(*arvados.CreateOptions))
+                       },
+               },
+               {
+                       arvados.EndpointGroupUpdate,
+                       func() interface{} { return &arvados.UpdateOptions{} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.GroupUpdate(ctx, *opts.(*arvados.UpdateOptions))
+                       },
+               },
+               {
+                       arvados.EndpointGroupGet,
+                       func() interface{} { return &arvados.GetOptions{} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.GroupGet(ctx, *opts.(*arvados.GetOptions))
+                       },
+               },
+               {
+                       arvados.EndpointGroupList,
+                       func() interface{} { return &arvados.ListOptions{Limit: -1} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.GroupList(ctx, *opts.(*arvados.ListOptions))
+                       },
+               },
+               {
+                       arvados.EndpointGroupContents,
+                       func() interface{} { return &arvados.ListOptions{Limit: -1} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.GroupContents(ctx, *opts.(*arvados.ContentsOptions))
+                       },
+               },
+               {
+                       arvados.EndpointGroupShared,
+                       func() interface{} { return &arvados.ListOptions{Limit: -1} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.GroupShared(ctx, *opts.(*arvados.SharedOptions))
+                       },
+               },
+               {
+                       arvados.EndpointGroupDelete,
+                       func() interface{} { return &arvados.DeleteOptions{} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.GroupDelete(ctx, *opts.(*arvados.DeleteOptions))
+                       },
+               },
+               {
+                       arvados.EndpointGroupUntrash,
+                       func() interface{} { return &arvados.UntrashOptions{} },
+                       func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               return rtr.backend.GroupUntrash(ctx, *opts.(*arvados.UntrashOptions))
+                       },
+               },
                {
                        arvados.EndpointSpecimenCreate,
                        func() interface{} { return &arvados.CreateOptions{} },

diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 3a19f4ab5ad50d2ad5ceb5fbdf9981108bf1213f..69bb06c0fd83116a5cc4285d0c448a6ed4b16e3f 100644 (file)
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -416,6 +416,62 @@ func (conn *Conn) ContainerRequestDelete(ctx context.Context, options arvados.De
        return resp, err
 }
 
+func (conn *Conn) GroupCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Group, error) {
+       ep := arvados.EndpointGroupCreate
+       var resp arvados.Group
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) GroupUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.Group, error) {
+       ep := arvados.EndpointGroupUpdate
+       var resp arvados.Group
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) GroupGet(ctx context.Context, options arvados.GetOptions) (arvados.Group, error) {
+       ep := arvados.EndpointGroupGet
+       var resp arvados.Group
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) GroupList(ctx context.Context, options arvados.ListOptions) (arvados.GroupList, error) {
+       ep := arvados.EndpointGroupList
+       var resp arvados.GroupList
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) GroupContents(ctx context.Context, options arvados.ContentsOptions) (arvados.GroupList, error) {
+       ep := arvados.EndpointGroupContents
+       var resp arvados.GroupList
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) GroupShared(ctx context.Context, options arvados.SharedOptions) (arvados.GroupList, error) {
+       ep := arvados.EndpointGroupShared
+       var resp arvados.GroupList
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) GroupDelete(ctx context.Context, options arvados.DeleteOptions) (arvados.Group, error) {
+       ep := arvados.EndpointGroupDelete
+       var resp arvados.Group
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
+func (conn *Conn) GroupUntrash(ctx context.Context, options arvados.UntrashOptions) (arvados.Group, error) {
+       ep := arvados.EndpointGroupUntrash
+       var resp arvados.Group
+       err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+       return resp, err
+}
+
 func (conn *Conn) SpecimenCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Specimen, error) {
        ep := arvados.EndpointSpecimenCreate
        var resp arvados.Specimen

diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 37a3e007b16108cb6a2f9c6627c29106eee52bca..3331be58d5cd0f38e880a5435625e5cd85b26f4c 100644 (file)
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -51,6 +51,14 @@ var (
        EndpointContainerRequestGet           = APIEndpoint{"GET", "arvados/v1/container_requests/{uuid}", ""}
        EndpointContainerRequestList          = APIEndpoint{"GET", "arvados/v1/container_requests", ""}
        EndpointContainerRequestDelete        = APIEndpoint{"DELETE", "arvados/v1/container_requests/{uuid}", ""}
+       EndpointGroupCreate                   = APIEndpoint{"POST", "arvados/v1/groups", "group"}
+       EndpointGroupUpdate                   = APIEndpoint{"PATCH", "arvados/v1/groups/{uuid}", "group"}
+       EndpointGroupGet                      = APIEndpoint{"GET", "arvados/v1/groups/{uuid}", ""}
+       EndpointGroupList                     = APIEndpoint{"GET", "arvados/v1/groups", ""}
+       EndpointGroupContents                 = APIEndpoint{"GET", "arvados/v1/groups/{uuid}/contents", ""}
+       EndpointGroupShared                   = APIEndpoint{"GET", "arvados/v1/groups/{uuid}/shared", ""}
+       EndpointGroupDelete                   = APIEndpoint{"DELETE", "arvados/v1/groups/{uuid}", ""}
+       EndpointGroupUntrash                  = APIEndpoint{"POST", "arvados/v1/groups/{uuid}/untrash", ""}
        EndpointUserActivate                  = APIEndpoint{"POST", "arvados/v1/users/{uuid}/activate", ""}
        EndpointUserCreate                    = APIEndpoint{"POST", "arvados/v1/users", "user"}
        EndpointUserCurrent                   = APIEndpoint{"GET", "arvados/v1/users/current", ""}
@@ -124,6 +132,27 @@ type UpdateOptions struct {
        BypassFederation bool                   `json:"bypass_federation"`
 }
 
+type ContentsOptions struct {
+       UUID               string   `json:"uuid,omitempty"`
+       Select             []string `json:"select"`
+       Filters            []Filter `json:"filters"`
+       Limit              int64    `json:"limit"`
+       Offset             int64    `json:"offset"`
+       Order              []string `json:"order"`
+       Recursive          bool     `json:"recursive"`
+       ExcludeHomeProject bool     `json:"exclude_home_project"`
+}
+
+type SharedOptions struct {
+       UUID    string   `json:"uuid,omitempty"`
+       Select  []string `json:"select"`
+       Filters []Filter `json:"filters"`
+       Limit   int64    `json:"limit"`
+       Offset  int64    `json:"offset"`
+       Order   []string `json:"order"`
+       Include string   `json:"include"`
+}
+
 type UpdateUUIDOptions struct {
        UUID    string `json:"uuid"`
        NewUUID string `json:"new_uuid"`
@@ -203,6 +232,14 @@ type API interface {
        ContainerRequestGet(ctx context.Context, options GetOptions) (ContainerRequest, error)
        ContainerRequestList(ctx context.Context, options ListOptions) (ContainerRequestList, error)
        ContainerRequestDelete(ctx context.Context, options DeleteOptions) (ContainerRequest, error)
+       GroupCreate(ctx context.Context, options CreateOptions) (Group, error)
+       GroupUpdate(ctx context.Context, options UpdateOptions) (Group, error)
+       GroupGet(ctx context.Context, options GetOptions) (Group, error)
+       GroupList(ctx context.Context, options ListOptions) (GroupList, error)
+       GroupContents(ctx context.Context, options ContentsOptions) (GroupList, error)
+       GroupShared(ctx context.Context, options SharedOptions) (GroupList, error)
+       GroupDelete(ctx context.Context, options DeleteOptions) (Group, error)
+       GroupUntrash(ctx context.Context, options UntrashOptions) (Group, error)
        SpecimenCreate(ctx context.Context, options CreateOptions) (Specimen, error)
        SpecimenUpdate(ctx context.Context, options UpdateOptions) (Specimen, error)
        SpecimenGet(ctx context.Context, options GetOptions) (Specimen, error)

diff --git a/sdk/go/arvados/group.go b/sdk/go/arvados/group.go
index bf2fe72ff880b9a3885482bbd326aab4255c0691..26bd6e1874a6ac592c3bdfc5ef010cee199db1ac 100644 (file)
--- a/sdk/go/arvados/group.go
+++ b/sdk/go/arvados/group.go
@@ -4,12 +4,28 @@
 
 package arvados
 
+import (
+       "time"
+)
+
 // Group is an arvados#group record
 type Group struct {
-       UUID       string `json:"uuid"`
-       Name       string `json:"name"`
-       OwnerUUID  string `json:"owner_uuid"`
-       GroupClass string `json:"group_class"`
+       UUID                 string                 `json:"uuid"`
+       Name                 string                 `json:"name"`
+       OwnerUUID            string                 `json:"owner_uuid"`
+       GroupClass           string                 `json:"group_class"`
+       Etag                 string                 `json:"etag"`
+       Href                 string                 `json:"href"`
+       TrashAt              *time.Time             `json:"trash_at"`
+       CreatedAt            time.Time              `json:"created_at"`
+       ModifiedAt           time.Time              `json:"modified_at"`
+       ModifiedByClientUUID string                 `json:"modified_by_client_uuid"`
+       ModifiedByUserUUID   string                 `json:"modified_by_user_uuid"`
+       DeleteAt             *time.Time             `json:"delete_at"`
+       IsTrashed            bool                   `json:"is_trashed"`
+       Properties           map[string]interface{} `json:"properties"`
+       WritableBy           []string               `json:"writable_by,omitempty"`
+       Description          string                 `json:"description"`
 }
 
 // GroupList is an arvados#groupList resource.

diff --git a/sdk/go/arvadostest/api.go b/sdk/go/arvadostest/api.go
index 930eabf27ef997a2662d0a56c8c5a1494e59a98a..fe8c324660107960314f9f73a9843ca68ec37d61 100644 (file)
--- a/sdk/go/arvadostest/api.go
+++ b/sdk/go/arvadostest/api.go
@@ -129,6 +129,34 @@ func (as *APIStub) ContainerRequestDelete(ctx context.Context, options arvados.D
        as.appendCall(ctx, as.ContainerRequestDelete, options)
        return arvados.ContainerRequest{}, as.Error
 }
+func (as *APIStub) GroupCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Group, error) {
+       as.appendCall(ctx, as.GroupCreate, options)
+       return arvados.Group{}, as.Error
+}
+func (as *APIStub) GroupUpdate(ctx context.Context, options arvados.UpdateOptions) (arvados.Group, error) {
+       as.appendCall(ctx, as.GroupUpdate, options)
+       return arvados.Group{}, as.Error
+}
+func (as *APIStub) GroupGet(ctx context.Context, options arvados.GetOptions) (arvados.Group, error) {
+       as.appendCall(ctx, as.GroupGet, options)
+       return arvados.Group{}, as.Error
+}
+func (as *APIStub) GroupList(ctx context.Context, options arvados.ListOptions) (arvados.GroupList, error) {
+       as.appendCall(ctx, as.GroupList, options)
+       return arvados.GroupList{}, as.Error
+}
+func (as *APIStub) GroupContents(ctx context.Context, options arvados.GetOptions) (map[string]interface{}, error) {
+       as.appendCall(ctx, as.GroupContents, options)
+       return nil, as.Error
+}
+func (as *APIStub) GroupShared(ctx context.Context, options arvados.GetOptions) (map[string]interface{}, error) {
+       as.appendCall(ctx, as.GroupShared, options)
+       return nil, as.Error
+}
+func (as *APIStub) GroupDelete(ctx context.Context, options arvados.DeleteOptions) (arvados.Group, error) {
+       as.appendCall(ctx, as.GroupDelete, options)
+       return arvados.Group{}, as.Error
+}
 func (as *APIStub) SpecimenCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Specimen, error) {
        as.appendCall(ctx, as.SpecimenCreate, options)
        return arvados.Specimen{}, as.Error

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index f2bae3a4b5b2c79a9a3aacc882b7e570d713bc92..2c5dee91ca3aeb44ef5f27b37619abdb560bc7c6 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -889,6 +889,13 @@ class ArvadosModel < ApplicationRecord
     }.merge(attributes['scheduling_parameters'] || {})
   end
 
+  # The 'group_class' for a group should default to 'project'
+  def fill_group_defaults
+    if !attributes.key?("group_class") || attributes['group_class'].nil? || attributes['group_class'].empty?
+      self.group_class = 'project'
+    end
+  end
+
   # ArvadosModel.find_by_uuid needs extra magic to allow it to return
   # an object in any class.
   def self.find_by_uuid uuid

diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 7e015f3564e7475f6103e8f4a42c5beb5bf53c83..519dcd3072013c5c2280d2eab90f4a33beee6a10 100644 (file)
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -16,6 +16,7 @@ class Group < ArvadosModel
   # already know how to properly treat them.
   attribute :properties, :jsonbHash, default: {}
 
+  before_validation :fill_group_defaults
   validate :ensure_filesystem_compatible_name
   validate :check_group_class
   before_create :assign_name
@@ -42,14 +43,14 @@ class Group < ArvadosModel
   end
 
   def ensure_filesystem_compatible_name
-    # project groups need filesystem-compatible names, but others
+    # project and filter groups need filesystem-compatible names, but others
     # don't.
-    super if group_class == 'project'
+    super if group_class == 'project' || group_class == 'filter'
   end
 
   def check_group_class
-    if group_class != 'project' && group_class != 'role'
-      errors.add :group_class, "value must be one of 'project' or 'role', was '#{group_class}'"
+    if group_class != 'project' && group_class != 'role' && group_class != 'filter'
+      errors.add :group_class, "value must be one of 'project', 'role' or 'filter', was '#{group_class}'"
     end
     if group_class_changed? && !group_class_was.nil?
       errors.add :group_class, "cannot be modified after record is created"

diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index 30fddfa5b8be8f89c2f43651c1c316a6e59253fe..d7a33a4515d615a19928525198c3acc61c7d9d1a 100644 (file)
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -62,7 +62,7 @@ class GroupTest < ActiveSupport::TestCase
     assert g_foo.errors.messages[:owner_uuid].join(" ").match(/ownership cycle/)
   end
 
-  test "cannot create a group that is not a 'role' or 'project'" do
+  test "cannot create a group that is not a 'role' or 'project' or 'filter'" do
     set_user_from_auth :active_trustedclient
 
     assert_raises(ActiveRecord::RecordInvalid) do
@@ -278,6 +278,7 @@ class GroupTest < ActiveSupport::TestCase
       Rails.configuration.Collections.ForwardSlashNameSubstitution = subst
       proj = Group.create group_class: "project"
       role = Group.create group_class: "role"
+      filt = Group.create group_class: "filter"
       [[nil, true],
        ["", true],
        [".", false],
@@ -292,6 +293,8 @@ class GroupTest < ActiveSupport::TestCase
         assert_equal true, role.valid?
         proj.name = name
         assert_equal valid, proj.valid?, "#{name.inspect} should be #{valid ? "valid" : "invalid"}"
+        filt.name = name
+        assert_equal valid, filt.valid?, "#{name.inspect} should be #{valid ? "valid" : "invalid"}"
       end
     end
   end