JUJU-7073: add filter on group name/id for listGroups (#1426)

* add filter on group name/id for listGroups

* improvo godoc

* move fuzzy search to db layer

* refactor foreachgroup to list groups

* update godoc

* update godoc

* rename func arg from filter to pagination

* update godoc

* Trigger Ci

* add uuid fuzzy match

internal/db/group.go

@@ -82,41 +82,30 @@ func (d *Database) GetGroup(ctx context.Context, group *dbmodel.GroupEntry) (err
 	return nil
 }
 
-// ForEachGroup iterates through every group calling the given function
-// for each one. If the given function returns an error the iteration
-// will stop immediately and the error will be returned unmodified.
-func (d *Database) ForEachGroup(ctx context.Context, limit, offset int, f func(*dbmodel.GroupEntry) error) (err error) {
-	const op = errors.Op("db.ForEachGroup")
+// ListGroups returns a paginated list of groups defined by limit and offset.
+// match is used to fuzzy find based on entries' name or uuid using the LIKE operator (ex. LIKE %<match>%).
+func (d *Database) ListGroups(ctx context.Context, limit, offset int, match string) (_ []dbmodel.GroupEntry, err error) {
+	const op = errors.Op("db.ListGroups")
 	if err := d.ready(); err != nil {
-		return errors.E(op, err)
+		return nil, errors.E(op, err)
 	}
 
 	durationObserver := servermon.DurationObserver(servermon.DBQueryDurationHistogram, string(op))
 	defer durationObserver()
 	defer servermon.ErrorCounter(servermon.DBQueryErrorCount, &err, string(op))
 
 	db := d.DB.WithContext(ctx)
+	if match != "" {
+		db = db.Where("name LIKE ? OR uuid LIKE ?", "%"+match+"%", "%"+match+"%")
+	}
 	db = db.Order("name asc")
 	db = db.Limit(limit)
 	db = db.Offset(offset)
-	rows, err := db.Model(&dbmodel.GroupEntry{}).Rows()
-	if err != nil {
-		return errors.E(op, err)
-	}
-	defer rows.Close()
-	for rows.Next() {
-		var group dbmodel.GroupEntry
-		if err := db.ScanRows(rows, &group); err != nil {
-			return errors.E(op, err)
-		}
-		if err := f(&group); err != nil {
-			return err
-		}
-	}
-	if err := rows.Err(); err != nil {
-		return errors.E(op, dbError(err))
+	var groups []dbmodel.GroupEntry
+	if err := db.Find(&groups).Error; err != nil {
+		return nil, errors.E(op, dbError(err))
 	}
-	return nil
+	return groups, nil
 }
 
 // UpdateGroup updates the group identified by its ID.

internal/db/group_test.go

@@ -193,23 +193,37 @@ func (s *dbSuite) TestForEachGroup(c *qt.C) {
 		_, err := s.Database.AddGroup(context.Background(), fmt.Sprintf("test-group-%d", i))
 		c.Assert(err, qt.IsNil)
 	}
-	firstGroups := []*dbmodel.GroupEntry{}
 	ctx := context.Background()
-	err = s.Database.ForEachGroup(ctx, 5, 0, func(ge *dbmodel.GroupEntry) error {
-		firstGroups = append(firstGroups, ge)
-		return nil
-	})
+	firstGroups, err := s.Database.ListGroups(ctx, 5, 0, "")
 	c.Assert(err, qt.IsNil)
 	for i := 0; i < 5; i++ {
 		c.Assert(firstGroups[i].Name, qt.Equals, fmt.Sprintf("test-group-%d", i))
 	}
-	secondGroups := []*dbmodel.GroupEntry{}
-	err = s.Database.ForEachGroup(ctx, 5, 5, func(ge *dbmodel.GroupEntry) error {
-		secondGroups = append(secondGroups, ge)
-		return nil
-	})
+	secondGroups, err := s.Database.ListGroups(ctx, 5, 5, "")
 	c.Assert(err, qt.IsNil)
 	for i := 0; i < 5; i++ {
 		c.Assert(secondGroups[i].Name, qt.Equals, fmt.Sprintf("test-group-%d", i+5))
 	}
+
+	matchedGroups, err := s.Database.ListGroups(ctx, 5, 0, "group-1")
+	c.Assert(err, qt.IsNil)
+	c.Assert(matchedGroups, qt.HasLen, 1)
+	c.Assert(matchedGroups[0].Name, qt.Equals, "test-group-1")
+
+	matchedGroups, err = s.Database.ListGroups(ctx, 5, 0, "%not-existing%")
+	c.Assert(err, qt.IsNil)
+	c.Assert(matchedGroups, qt.HasLen, 0)
+
+	tg, err := s.Database.AddGroup(context.Background(), "\\%test-group")
+	c.Assert(err, qt.IsNil)
+
+	matchedGroups, err = s.Database.ListGroups(ctx, 5, 0, "\\%t")
+	c.Assert(err, qt.IsNil)
+	c.Assert(matchedGroups, qt.HasLen, 1)
+	c.Assert(matchedGroups[0].UUID, qt.Equals, tg.UUID)
+
+	matchedGroups, err = s.Database.ListGroups(ctx, 5, 0, tg.UUID)
+	c.Assert(err, qt.IsNil)
+	c.Assert(matchedGroups, qt.HasLen, 1)
+	c.Assert(matchedGroups[0].UUID, qt.Equals, tg.UUID)
 }

internal/jimm/access.go

@@ -790,18 +790,15 @@ func (j *JIMM) RemoveGroup(ctx context.Context, user *openfga.User, name string)
 }
 
 // ListGroups returns a list of groups known to JIMM.
-func (j *JIMM) ListGroups(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination) ([]dbmodel.GroupEntry, error) {
+// `match` will filter the list fuzzy matching group's name or uuid.
+func (j *JIMM) ListGroups(ctx context.Context, user *openfga.User, pagination pagination.LimitOffsetPagination, match string) ([]dbmodel.GroupEntry, error) {
 	const op = errors.Op("jimm.ListGroups")
 
 	if !user.JimmAdmin {
 		return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized")
 	}
 
-	var groups []dbmodel.GroupEntry
-	err := j.Database.ForEachGroup(ctx, filter.Limit(), filter.Offset(), func(ge *dbmodel.GroupEntry) error {
-		groups = append(groups, *ge)
-		return nil
-	})
+	groups, err := j.Database.ListGroups(ctx, pagination.Limit(), pagination.Offset(), match)
 	if err != nil {
 		return nil, errors.E(op, err)
 	}

internal/jimm/access_test.go

@@ -1079,8 +1079,8 @@ func TestListGroups(t *testing.T) {
 	u := openfga.NewUser(&user, ofgaClient)
 	u.JimmAdmin = true
 
-	filter := pagination.NewOffsetFilter(10, 0)
-	groups, err := j.ListGroups(ctx, u, filter)
+	pagination := pagination.NewOffsetFilter(10, 0)
+	groups, err := j.ListGroups(ctx, u, pagination, "")
 	c.Assert(err, qt.IsNil)
 	c.Assert(groups, qt.DeepEquals, []dbmodel.GroupEntry{group})
 
@@ -1095,7 +1095,7 @@ func TestListGroups(t *testing.T) {
 		_, err := j.AddGroup(ctx, u, name)
 		c.Assert(err, qt.IsNil)
 	}
-	groups, err = j.ListGroups(ctx, u, filter)
+	groups, err = j.ListGroups(ctx, u, pagination, "")
 	c.Assert(err, qt.IsNil)
 	sort.Slice(groups, func(i, j int) bool {
 		return groups[i].Name < groups[j].Name

internal/jimmhttp/rebac_admin/groups.go

@@ -41,7 +41,11 @@ func (s *groupsService) ListGroups(ctx context.Context, params *resources.GetGro
 		return nil, err
 	}
 	page, nextPage, pagination := pagination.CreatePagination(params.Size, params.Page, count)
-	groups, err := s.jimm.ListGroups(ctx, user, pagination)
+	match := ""
+	if params.Filter != nil && *params.Filter != "" {
+		match = *params.Filter
+	}
+	groups, err := s.jimm.ListGroups(ctx, user, pagination, match)
 	if err != nil {
 		return nil, err
 	}

internal/jimmhttp/rebac_admin/groups_integration_test.go

@@ -30,6 +30,37 @@ func (s *rebacAdminSuite) SetUpTest(c *gc.C) {
 
 var _ = gc.Suite(&rebacAdminSuite{})
 
+func (s rebacAdminSuite) TestListGroupsWithFilterIntegration(c *gc.C) {
+	ctx := context.Background()
+	for i := range 10 {
+		_, err := s.JIMM.AddGroup(ctx, s.AdminUser, fmt.Sprintf("test-group-filter-%d", i))
+		c.Assert(err, gc.IsNil)
+	}
+
+	ctx = rebac_handlers.ContextWithIdentity(ctx, s.AdminUser)
+	pageSize := 5
+	page := 0
+	params := &resources.GetGroupsParams{Size: &pageSize, Page: &page}
+	res, err := s.groupSvc.ListGroups(ctx, params)
+	c.Assert(err, gc.IsNil)
+	c.Assert(res, gc.Not(gc.IsNil))
+	c.Assert(res.Meta.Size, gc.Equals, 5)
+
+	match := "group-filter-1"
+	params.Filter = &match
+	res, err = s.groupSvc.ListGroups(ctx, params)
+	c.Assert(err, gc.IsNil)
+	c.Assert(res, gc.Not(gc.IsNil))
+	c.Assert(len(res.Data), gc.Equals, 1)
+
+	match = "group"
+	params.Filter = &match
+	res, err = s.groupSvc.ListGroups(ctx, params)
+	c.Assert(err, gc.IsNil)
+	c.Assert(res, gc.Not(gc.IsNil))
+	c.Assert(len(res.Data), gc.Equals, pageSize)
+}
+
 func (s rebacAdminSuite) TestGetGroupIdentitiesIntegration(c *gc.C) {
 	ctx := context.Background()
 	group, err := s.JIMM.AddGroup(ctx, s.AdminUser, "test-group")

internal/jimmhttp/rebac_admin/groups_test.go

@@ -86,7 +86,7 @@ func TestListGroups(t *testing.T) {
 	}
 	jimm := jimmtest.JIMM{
 		GroupService: mocks.GroupService{
-			ListGroups_: func(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination) ([]dbmodel.GroupEntry, error) {
+			ListGroups_: func(ctx context.Context, user *openfga.User, pagination pagination.LimitOffsetPagination, match string) ([]dbmodel.GroupEntry, error) {
 				return returnedGroups, listErr
 			},
 			CountGroups_: func(ctx context.Context, user *openfga.User) (int, error) {

internal/jujuapi/access_control.go

@@ -30,7 +30,7 @@ type GroupService interface {
 	CountGroups(ctx context.Context, user *openfga.User) (int, error)
 	GetGroupByUUID(ctx context.Context, user *openfga.User, uuid string) (*dbmodel.GroupEntry, error)
 	GetGroupByName(ctx context.Context, user *openfga.User, name string) (*dbmodel.GroupEntry, error)
-	ListGroups(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination) ([]dbmodel.GroupEntry, error)
+	ListGroups(ctx context.Context, user *openfga.User, pagination pagination.LimitOffsetPagination, match string) ([]dbmodel.GroupEntry, error)
 	RenameGroup(ctx context.Context, user *openfga.User, oldName, newName string) error
 	RemoveGroup(ctx context.Context, user *openfga.User, name string) error
 }
@@ -118,8 +118,8 @@ func (r *controllerRoot) RemoveGroup(ctx context.Context, req apiparams.RemoveGr
 func (r *controllerRoot) ListGroups(ctx context.Context, req apiparams.ListGroupsRequest) (apiparams.ListGroupResponse, error) {
 	const op = errors.Op("jujuapi.ListGroups")
 
-	filter := pagination.NewOffsetFilter(req.Limit, req.Offset)
-	groups, err := r.jimm.ListGroups(ctx, r.user, filter)
+	pagination := pagination.NewOffsetFilter(req.Limit, req.Offset)
+	groups, err := r.jimm.ListGroups(ctx, r.user, pagination, "")
 	if err != nil {
 		return apiparams.ListGroupResponse{}, errors.E(op, err)
 	}

internal/testutils/jimmtest/mocks/jimm_group_mock.go

@@ -22,7 +22,7 @@ type GroupService struct {
 	CountGroups_    func(ctx context.Context, user *openfga.User) (int, error)
 	GetGroupByUUID_ func(ctx context.Context, user *openfga.User, uuid string) (*dbmodel.GroupEntry, error)
 	GetGroupByName_ func(ctx context.Context, user *openfga.User, name string) (*dbmodel.GroupEntry, error)
-	ListGroups_     func(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination) ([]dbmodel.GroupEntry, error)
+	ListGroups_     func(ctx context.Context, user *openfga.User, pagination pagination.LimitOffsetPagination, match string) ([]dbmodel.GroupEntry, error)
 	RenameGroup_    func(ctx context.Context, user *openfga.User, oldName, newName string) error
 	RemoveGroup_    func(ctx context.Context, user *openfga.User, name string) error
 }
@@ -55,11 +55,11 @@ func (j *GroupService) GetGroupByName(ctx context.Context, user *openfga.User, n
 	return j.GetGroupByName_(ctx, user, name)
 }
 
-func (j *GroupService) ListGroups(ctx context.Context, user *openfga.User, filters pagination.LimitOffsetPagination) ([]dbmodel.GroupEntry, error) {
+func (j *GroupService) ListGroups(ctx context.Context, user *openfga.User, pagination pagination.LimitOffsetPagination, match string) ([]dbmodel.GroupEntry, error) {
 	if j.ListGroups_ == nil {
 		return nil, errors.E(errors.CodeNotImplemented)
 	}
-	return j.ListGroups_(ctx, user, filters)
+	return j.ListGroups_(ctx, user, pagination, match)
 }
 
 func (j *GroupService) RemoveGroup(ctx context.Context, user *openfga.User, name string) error {