Skip to content

Commit

Permalink
MG-2287 - Improve search for channels and groups (absmach#2315)
Browse files Browse the repository at this point in the history
Signed-off-by: nyagamunene <[email protected]>
  • Loading branch information
nyagamunene authored Jul 5, 2024
1 parent b8a1fe1 commit ccb4827
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 31 deletions.
11 changes: 8 additions & 3 deletions internal/groups/api/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func DecodeListGroupsRequest(_ context.Context, r *http.Request) (interface{}, e
memberID: chi.URLParam(r, "memberID"),
Page: mggroups.Page{
Level: level,
ID: parentID,
ParentID: parentID,
Permission: permission,
PageMeta: pm,
Direction: dir,
Expand Down Expand Up @@ -102,7 +102,7 @@ func DecodeListParentsRequest(_ context.Context, r *http.Request) (interface{},
tree: tree,
Page: mggroups.Page{
Level: level,
ID: chi.URLParam(r, "groupID"),
ParentID: chi.URLParam(r, "groupID"),
Permission: permission,
PageMeta: pm,
Direction: +1,
Expand Down Expand Up @@ -141,7 +141,7 @@ func DecodeListChildrenRequest(_ context.Context, r *http.Request) (interface{},
tree: tree,
Page: mggroups.Page{
Level: level,
ID: chi.URLParam(r, "groupID"),
ParentID: chi.URLParam(r, "groupID"),
Permission: permission,
PageMeta: pm,
Direction: -1,
Expand Down Expand Up @@ -272,6 +272,10 @@ func decodePageMeta(r *http.Request) (mggroups.PageMeta, error) {
if err != nil {
return mggroups.PageMeta{}, errors.Wrap(apiutil.ErrValidation, err)
}
id, err := apiutil.ReadStringQuery(r, api.IDOrder, "")
if err != nil {
return mggroups.PageMeta{}, errors.Wrap(apiutil.ErrValidation, err)
}
meta, err := apiutil.ReadMetadataQuery(r, api.MetadataKey, nil)
if err != nil {
return mggroups.PageMeta{}, errors.Wrap(apiutil.ErrValidation, err)
Expand All @@ -281,6 +285,7 @@ func decodePageMeta(r *http.Request) (mggroups.PageMeta, error) {
Offset: offset,
Limit: limit,
Name: name,
ID: id,
Metadata: meta,
Status: st,
}
Expand Down
2 changes: 1 addition & 1 deletion internal/groups/api/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestDecodeListGroupsRequest(t *testing.T) {
},
},
Level: 2,
ID: "random",
ParentID: "random",
Permission: "random",
Direction: -1,
ListPerms: true,
Expand Down
4 changes: 2 additions & 2 deletions internal/groups/api/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ func TestListGroupsEndpoint(t *testing.T) {
PageMeta: groups.PageMeta{
Limit: 10,
},
ID: validGroupResp.ID,
ParentID: validGroupResp.ID,
Direction: -1,
},
tree: false,
Expand Down Expand Up @@ -627,7 +627,7 @@ func TestListGroupsEndpoint(t *testing.T) {
PageMeta: groups.PageMeta{
Limit: 10,
},
ID: validGroupResp.ID,
ParentID: validGroupResp.ID,
Direction: 1,
},
tree: false,
Expand Down
2 changes: 1 addition & 1 deletion internal/groups/api/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func ListGroupsEndpoint(svc groups.Service, groupType, memberKind string) endpoi
if req.tree {
return buildGroupsResponseTree(page), nil
}
filterByID := req.Page.ID != ""
filterByID := req.Page.ParentID != ""

if groupType == groupTypeChannels {
return buildChannelsResponse(page, filterByID), nil
Expand Down
19 changes: 11 additions & 8 deletions internal/groups/postgres/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ func (repo groupRepository) RetrieveAll(ctx context.Context, gm mggroups.Page) (
var q string
query := buildQuery(gm)

if gm.ID != "" {
if gm.ParentID != "" {
q = buildHierachy(gm)
}
if gm.ID == "" {
if gm.ParentID == "" {
q = `SELECT DISTINCT g.id, g.domain_id, COALESCE(g.parent_id, '') AS parent_id, g.name, g.description,
g.metadata, g.created_at, g.updated_at, g.updated_by, g.status FROM groups g`
}
Expand Down Expand Up @@ -197,10 +197,10 @@ func (repo groupRepository) RetrieveByIDs(ctx context.Context, gm mggroups.Page,
}
query := buildQuery(gm, ids...)

if gm.ID != "" {
if gm.ParentID != "" {
q = buildHierachy(gm)
}
if gm.ID == "" {
if gm.ParentID == "" {
q = `SELECT DISTINCT g.id, g.domain_id, COALESCE(g.parent_id, '') AS parent_id, g.name, g.description,
g.metadata, g.created_at, g.updated_at, g.updated_by, g.status FROM groups g`
}
Expand Down Expand Up @@ -310,14 +310,14 @@ func buildHierachy(gm mggroups.Page) string {
switch {
case gm.Direction >= 0: // ancestors
query = `WITH RECURSIVE groups_cte as (
SELECT id, COALESCE(parent_id, '') AS parent_id, domain_id, name, description, metadata, created_at, updated_at, updated_by, status, 0 as level from groups WHERE id = :id
SELECT id, COALESCE(parent_id, '') AS parent_id, domain_id, name, description, metadata, created_at, updated_at, updated_by, status, 0 as level from groups WHERE id = :parent_id
UNION SELECT x.id, COALESCE(x.parent_id, '') AS parent_id, x.domain_id, x.name, x.description, x.metadata, x.created_at, x.updated_at, x.updated_by, x.status, level - 1 from groups x
INNER JOIN groups_cte a ON a.parent_id = x.id
) SELECT * FROM groups_cte g`

case gm.Direction < 0: // descendants
query = `WITH RECURSIVE groups_cte as (
SELECT id, COALESCE(parent_id, '') AS parent_id, domain_id, name, description, metadata, created_at, updated_at, updated_by, status, 0 as level, CONCAT('', '', id) as path from groups WHERE id = :id
SELECT id, COALESCE(parent_id, '') AS parent_id, domain_id, name, description, metadata, created_at, updated_at, updated_by, status, 0 as level, CONCAT('', '', id) as path from groups WHERE id = :parent_id
UNION SELECT x.id, COALESCE(x.parent_id, '') AS parent_id, x.domain_id, x.name, x.description, x.metadata, x.created_at, x.updated_at, x.updated_by, x.status, level + 1, CONCAT(path, '.', x.id) as path from groups x
INNER JOIN groups_cte d ON d.id = x.parent_id
) SELECT * FROM groups_cte g`
Expand All @@ -332,7 +332,10 @@ func buildQuery(gm mggroups.Page, ids ...string) string {
queries = append(queries, fmt.Sprintf(" id in ('%s') ", strings.Join(ids, "', '")))
}
if gm.Name != "" {
queries = append(queries, "g.name = :name")
queries = append(queries, "g.name ILIKE '%' || :name || '%'")
}
if gm.PageMeta.ID != "" {
queries = append(queries, "g.id ILIKE '%' || :id || '%'")
}
if gm.Status != mgclients.AllStatus {
queries = append(queries, "g.status = :status")
Expand Down Expand Up @@ -459,7 +462,7 @@ func toDBGroupPage(pm mggroups.Page) (dbGroupPage, error) {
Total: pm.Total,
Offset: pm.Offset,
Limit: pm.Limit,
ParentID: pm.ID,
ParentID: pm.ParentID,
DomainID: pm.DomainID,
Status: pm.Status,
}, nil
Expand Down
8 changes: 4 additions & 4 deletions internal/groups/postgres/groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ func TestRetrieveAll(t *testing.T) {
Offset: 0,
Limit: uint64(num),
},
ID: items[5].ID,
ParentID: items[5].ID,
Direction: 1,
},
response: mggroups.Page{
Expand All @@ -658,7 +658,7 @@ func TestRetrieveAll(t *testing.T) {
Offset: 0,
Limit: uint64(num),
},
ID: items[150].ID,
ParentID: items[150].ID,
Direction: -1,
},
response: mggroups.Page{
Expand Down Expand Up @@ -949,7 +949,7 @@ func TestRetrieveByIDs(t *testing.T) {
Offset: 0,
Limit: uint64(num),
},
ID: items[5].ID,
ParentID: items[5].ID,
Direction: 1,
},
ids: getIDs(items[0:20]),
Expand All @@ -970,7 +970,7 @@ func TestRetrieveByIDs(t *testing.T) {
Offset: 0,
Limit: uint64(num),
},
ID: items[15].ID,
ParentID: items[15].ID,
Direction: -1,
},
ids: getIDs(items[0:20]),
Expand Down
1 change: 0 additions & 1 deletion internal/groups/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ func (svc service) ListGroups(ctx context.Context, token, memberKind, memberID s
default:
return groups.Page{}, errMemberKind
}

gp, err := svc.groups.RetrieveByIDs(ctx, gm, ids...)
if err != nil {
return groups.Page{}, errors.Wrap(svcerr.ErrViewEntity, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/groups/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type Page struct {
PageMeta
Path string
Level uint64
ID string
ParentID string
Permission string
ListPerms bool
Direction int64 // ancestors (+1) or descendants (-1)
Expand Down
1 change: 1 addition & 0 deletions pkg/groups/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ type PageMeta struct {
Offset uint64 `json:"offset"`
Limit uint64 `json:"limit"`
Name string `json:"name,omitempty"`
ID string `json:"id,omitempty"`
DomainID string `json:"domain_id,omitempty"`
Tag string `json:"tag,omitempty"`
Metadata clients.Metadata `json:"metadata,omitempty"`
Expand Down
20 changes: 10 additions & 10 deletions pkg/sdk/go/groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ func TestListParentGroups(t *testing.T) {
Offset: offset,
Limit: limit,
},
ID: parentID,
ParentID: parentID,
Permission: auth.ViewPermission,
Direction: 1,
Level: sdk.MaxLevel,
Expand Down Expand Up @@ -586,7 +586,7 @@ func TestListParentGroups(t *testing.T) {
Offset: offset,
Limit: limit,
},
ID: parentID,
ParentID: parentID,
Permission: auth.ViewPermission,
Direction: 1,
Level: sdk.MaxLevel,
Expand Down Expand Up @@ -623,7 +623,7 @@ func TestListParentGroups(t *testing.T) {
Offset: offset,
Limit: 10,
},
ID: parentID,
ParentID: parentID,
Permission: auth.ViewPermission,
Direction: 1,
Level: sdk.MaxLevel,
Expand Down Expand Up @@ -675,7 +675,7 @@ func TestListParentGroups(t *testing.T) {
"name": "user_89",
},
},
ID: parentID,
ParentID: parentID,
Permission: auth.ViewPermission,
Direction: 1,
Level: sdk.MaxLevel,
Expand Down Expand Up @@ -724,7 +724,7 @@ func TestListParentGroups(t *testing.T) {
Offset: offset,
Limit: limit,
},
ID: parentID,
ParentID: parentID,
Permission: auth.ViewPermission,
Direction: 1,
Level: sdk.MaxLevel,
Expand Down Expand Up @@ -811,7 +811,7 @@ func TestListChildrenGroups(t *testing.T) {
Offset: offset,
Limit: limit,
},
ID: childID,
ParentID: childID,
Permission: auth.ViewPermission,
Direction: -1,
Level: sdk.MaxLevel,
Expand Down Expand Up @@ -843,7 +843,7 @@ func TestListChildrenGroups(t *testing.T) {
Offset: offset,
Limit: limit,
},
ID: childID,
ParentID: childID,
Permission: auth.ViewPermission,
Direction: -1,
Level: sdk.MaxLevel,
Expand Down Expand Up @@ -880,7 +880,7 @@ func TestListChildrenGroups(t *testing.T) {
Offset: offset,
Limit: 10,
},
ID: childID,
ParentID: childID,
Permission: auth.ViewPermission,
Direction: -1,
Level: sdk.MaxLevel,
Expand Down Expand Up @@ -931,7 +931,7 @@ func TestListChildrenGroups(t *testing.T) {
"name": "user_89",
},
},
ID: childID,
ParentID: childID,
Permission: auth.ViewPermission,
Direction: -1,
Level: sdk.MaxLevel,
Expand Down Expand Up @@ -980,7 +980,7 @@ func TestListChildrenGroups(t *testing.T) {
Offset: offset,
Limit: limit,
},
ID: childID,
ParentID: childID,
Permission: auth.ViewPermission,
Direction: -1,
Level: sdk.MaxLevel,
Expand Down

0 comments on commit ccb4827

Please sign in to comment.