From f2722bebdaabdbb15a333566082e746f78702bc7 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Sun, 23 Jun 2024 23:02:22 +0300 Subject: [PATCH 01/31] Added search things api Signed-off-by: nyagamunene --- internal/api/common.go | 1 - pkg/apiutil/errors.go | 7 +- things/api/http/clients.go | 55 +++++++++ things/api/http/endpoints.go | 39 +++++++ things/api/http/endpoints_test.go | 182 ++++++++++++++++-------------- things/api/http/requests.go | 17 +++ things/api/http/requests_test.go | 41 +++++++ things/api/logging.go | 20 ++++ things/api/metrics.go | 8 ++ things/events/events.go | 26 +++++ things/events/streams.go | 15 +++ things/mocks/service.go | 28 +++++ things/postgres/clients.go | 94 +++++++++++++++ things/service.go | 28 +++++ things/things.go | 3 + things/tracing/tracing.go | 6 + 16 files changed, 483 insertions(+), 87 deletions(-) diff --git a/internal/api/common.go b/internal/api/common.go index 68388e0254..e7872a9968 100644 --- a/internal/api/common.go +++ b/internal/api/common.go @@ -124,7 +124,6 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { errors.Contains(err, errors.ErrMalformedEntity), errors.Contains(err, apiutil.ErrMissingID), errors.Contains(err, apiutil.ErrMissingName), - errors.Contains(err, apiutil.ErrMissingAlias), errors.Contains(err, apiutil.ErrMissingEmail), errors.Contains(err, apiutil.ErrMissingHost), errors.Contains(err, apiutil.ErrInvalidResetPass), diff --git a/pkg/apiutil/errors.go b/pkg/apiutil/errors.go index 833f8ecbd8..3d36dd7dec 100644 --- a/pkg/apiutil/errors.go +++ b/pkg/apiutil/errors.go @@ -135,9 +135,6 @@ var ( // ErrMissingName indicates missing identity name. ErrMissingName = errors.New("missing identity name") - // ErrMissingName indicates missing alias. - ErrMissingAlias = errors.New("missing alias") - // ErrInvalidLevel indicates an invalid group level. ErrInvalidLevel = errors.New("invalid group level (should be between 0 and 5)") @@ -179,4 +176,8 @@ var ( // ErrInvalidTimeFormat indicates invalid time format i.e not unix time. ErrInvalidTimeFormat = errors.New("invalid time format use unix time") + + ErrEmptySearchQuery = errors.New("search query must not be empty") + + ErrLenSearchQuery = errors.New("search query must be at least 3 characters") ) diff --git a/things/api/http/clients.go b/things/api/http/clients.go index 759e2f169c..df86c46fd7 100644 --- a/things/api/http/clients.go +++ b/things/api/http/clients.go @@ -60,6 +60,13 @@ func clientsHandler(svc things.Service, r *chi.Mux, logger *slog.Logger) http.Ha opts..., ), "view_thing_permissions").ServeHTTP) + r.Get("/search", otelhttp.NewHandler(kithttp.NewServer( + searchClientsEndpoint(svc), + decodeSearchClients, + api.EncodeResponse, + opts..., + ), "search_things").ServeHTTP) + r.Patch("/{thingID}", otelhttp.NewHandler(kithttp.NewServer( updateClientEndpoint(svc), decodeUpdateClient, @@ -209,6 +216,54 @@ func decodeListClients(_ context.Context, r *http.Request) (interface{}, error) return req, nil } +func decodeSearchClients(_ context.Context, r *http.Request) (interface{}, error) { + s, err := apiutil.ReadStringQuery(r, api.StatusKey, api.DefClientStatus) + if err != nil { + return nil, errors.Wrap(apiutil.ErrValidation, err) + } + o, err := apiutil.ReadNumQuery[uint64](r, api.OffsetKey, api.DefOffset) + if err != nil { + return nil, errors.Wrap(apiutil.ErrValidation, err) + } + l, err := apiutil.ReadNumQuery[uint64](r, api.LimitKey, api.DefLimit) + if err != nil { + return nil, errors.Wrap(apiutil.ErrValidation, err) + } + n, err := apiutil.ReadStringQuery(r, api.NameKey, "") + if err != nil { + return nil, errors.Wrap(apiutil.ErrValidation, err) + } + t, err := apiutil.ReadStringQuery(r, api.TagKey, "") + if err != nil { + return nil, errors.Wrap(apiutil.ErrValidation, err) + } + st, err := mgclients.ToStatus(s) + if err != nil { + return nil, errors.Wrap(apiutil.ErrValidation, err) + } + id, err := apiutil.ReadStringQuery(r, api.IDOrder, "") + if err != nil { + return nil, errors.Wrap(apiutil.ErrValidation, err) + } + + req := searchThingsReq{ + apiutil.ExtractBearerToken(r), + mgclients.Page{Status: st, Offset: o, Limit: l, Name: n, Tag: t, Identity: id}, + } + + for _, field := range []string{req.Name, req.Identity, req.Tag} { + if field != "" && len(field) < 3 { + req = searchThingsReq{ + token: apiutil.ExtractBearerToken(r), + Page: mgclients.Page{}, + } + return req, errors.Wrap(apiutil.ErrLenSearchQuery, apiutil.ErrValidation) + } + } + + return req, nil +} + func decodeUpdateClient(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), api.ContentType) { return nil, errors.Wrap(apiutil.ErrValidation, apiutil.ErrUnsupportedContentType) diff --git a/things/api/http/endpoints.go b/things/api/http/endpoints.go index 8e6d8d28b1..b7cf0951e3 100644 --- a/things/api/http/endpoints.go +++ b/things/api/http/endpoints.go @@ -147,6 +147,45 @@ func listMembersEndpoint(svc things.Service) endpoint.Endpoint { } } +func searchClientsEndpoint(svc things.Service) endpoint.Endpoint { + return func(ctx context.Context, request interface{}) (interface{}, error) { + req := request.(searchThingsReq) + if err := req.validate(); err != nil { + return nil, errors.Wrap(apiutil.ErrValidation, err) + } + + pm := mgclients.Page{ + Status: req.Status, + Offset: req.Offset, + Limit: req.Limit, + Name: req.Name, + Tag: req.Tag, + Metadata: req.Metadata, + Identity: req.Identity, + Order: req.Order, + Dir: req.Dir, + } + page, err := svc.SearchThings(ctx, req.token, pm) + if err != nil { + return nil, err + } + + res := clientsPageRes{ + pageRes: pageRes{ + Total: page.Total, + Offset: page.Offset, + Limit: page.Limit, + }, + Clients: []viewClientRes{}, + } + for _, client := range page.Clients { + res.Clients = append(res.Clients, viewClientRes{Client: client}) + } + + return res, nil + } +} + func updateClientEndpoint(svc things.Service) endpoint.Endpoint { return func(ctx context.Context, request interface{}) (interface{}, error) { req := request.(updateClientReq) diff --git a/things/api/http/endpoints_test.go b/things/api/http/endpoints_test.go index a0346ac436..89bf846663 100644 --- a/things/api/http/endpoints_test.go +++ b/things/api/http/endpoints_test.go @@ -380,6 +380,7 @@ func TestListThings(t *testing.T) { desc: "list things with valid token", token: validToken, status: http.StatusOK, + query: "name=clientname", listThingsResponse: mgclients.ClientsPage{ Page: mgclients.Page{ Total: 1, @@ -391,12 +392,14 @@ func TestListThings(t *testing.T) { { desc: "list things with empty token", token: "", + query: "name=clientname", status: http.StatusUnauthorized, err: apiutil.ErrBearerToken, }, { desc: "list things with invalid token", token: inValidToken, + query: "name=clientname", status: http.StatusUnauthorized, err: svcerr.ErrAuthentication, }, @@ -410,14 +413,14 @@ func TestListThings(t *testing.T) { }, Clients: []mgclients.Client{client}, }, - query: "offset=1", + query: "name=clientname&offset=1", status: http.StatusOK, err: nil, }, { desc: "list things with invalid offset", token: validToken, - query: "offset=invalid", + query: "name=clientname&offset=invalid", status: http.StatusBadRequest, err: apiutil.ErrValidation, }, @@ -431,17 +434,31 @@ func TestListThings(t *testing.T) { }, Clients: []mgclients.Client{client}, }, - query: "limit=1", + query: "name=clientname&limit=1", status: http.StatusOK, err: nil, }, { desc: "list things with invalid limit", token: validToken, - query: "limit=invalid", + query: "name=clientname&limit=invalid", status: http.StatusBadRequest, err: apiutil.ErrValidation, }, + { + desc: "search things with empty query", + token: validToken, + query: "", + status: http.StatusBadRequest, + err: apiutil.ErrEmptySearchQuery, + }, + { + desc: "search users with invalid length of query", + token: validToken, + query: "name=a", + status: http.StatusBadRequest, + err: apiutil.ErrLenSearchQuery, + }, { desc: "list things with limit greater than max", token: validToken, @@ -449,19 +466,6 @@ func TestListThings(t *testing.T) { status: http.StatusBadRequest, err: apiutil.ErrValidation, }, - { - desc: "list things with name", - token: validToken, - listThingsResponse: mgclients.ClientsPage{ - Page: mgclients.Page{ - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - query: "name=clientname", - status: http.StatusOK, - err: nil, - }, { desc: "list things with invalid name", token: validToken, @@ -476,19 +480,6 @@ func TestListThings(t *testing.T) { status: http.StatusBadRequest, err: apiutil.ErrInvalidQueryParams, }, - { - desc: "list things with status", - token: validToken, - listThingsResponse: mgclients.ClientsPage{ - Page: mgclients.Page{ - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - query: "status=enabled", - status: http.StatusOK, - err: nil, - }, { desc: "list things with invalid status", token: validToken, @@ -496,13 +487,6 @@ func TestListThings(t *testing.T) { status: http.StatusBadRequest, err: apiutil.ErrValidation, }, - { - desc: "list things with duplicate status", - token: validToken, - query: "status=enabled&status=disabled", - status: http.StatusBadRequest, - err: apiutil.ErrInvalidQueryParams, - }, { desc: "list things with tags", token: validToken, @@ -516,13 +500,6 @@ func TestListThings(t *testing.T) { status: http.StatusOK, err: nil, }, - { - desc: "list things with invalid tags", - token: validToken, - query: "tag=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, { desc: "list things with duplicate tags", token: validToken, @@ -531,98 +508,137 @@ func TestListThings(t *testing.T) { err: apiutil.ErrInvalidQueryParams, }, { - desc: "list things with metadata", + desc: "list things with id", token: validToken, + query: fmt.Sprintf("id=%s", client.ID), listThingsResponse: mgclients.ClientsPage{ Page: mgclients.Page{ Total: 1, }, Clients: []mgclients.Client{client}, }, - query: "metadata=%7B%22domain%22%3A%20%22example.com%22%7D&", status: http.StatusOK, err: nil, }, + } + + for _, tc := range cases { + req := testRequest{ + client: ts.Client(), + method: http.MethodGet, + url: ts.URL + "/things?" + tc.query, + contentType: contentType, + token: tc.token, + } + + svcCall := svc.On("ListClients", mock.Anything, tc.token, "", mock.Anything).Return(tc.listThingsResponse, tc.err) + res, err := req.make() + assert.Nil(t, err, fmt.Sprintf("%s: unexpected error %s", tc.desc, err)) + + var bodyRes respBody + err = json.NewDecoder(res.Body).Decode(&bodyRes) + assert.Nil(t, err, fmt.Sprintf("%s: unexpected error while decoding response body: %s", tc.desc, err)) + if bodyRes.Err != "" || bodyRes.Message != "" { + err = errors.Wrap(errors.New(bodyRes.Err), errors.New(bodyRes.Message)) + } + assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) + assert.Equal(t, tc.status, res.StatusCode, fmt.Sprintf("%s: expected status code %d got %d", tc.desc, tc.status, res.StatusCode)) + svcCall.Unset() + } +} + +func TestSearchThings(t *testing.T) { + ts, svc, _ := newThingsServer() + defer ts.Close() + + cases := []struct { + desc string + query string + token string + searchThingsRes mgclients.ClientsPage + status int + err error + }{ { - desc: "list things with invalid metadata", + desc: "search things with valid token", token: validToken, - query: "metadata=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, + status: http.StatusOK, + query: "name=clientname", + searchThingsRes: mgclients.ClientsPage{ + Page: mgclients.Page{ + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + err: nil, }, { - desc: "list things with duplicate metadata", - token: validToken, - query: "metadata=%7B%22domain%22%3A%20%22example.com%22%7D&metadata=%7B%22domain%22%3A%20%22example.com%22%7D", - status: http.StatusBadRequest, - err: apiutil.ErrInvalidQueryParams, + desc: "search things with empty token", + token: "", + query: "name=clientname", + status: http.StatusUnauthorized, + err: apiutil.ErrBearerToken, }, { - desc: "list things with permissions", + desc: "search things with invalid token", + token: inValidToken, + query: "name=clientname", + status: http.StatusUnauthorized, + err: svcerr.ErrAuthentication, + }, + { + desc: "search things with offset", token: validToken, - listThingsResponse: mgclients.ClientsPage{ + searchThingsRes: mgclients.ClientsPage{ Page: mgclients.Page{ - Total: 1, + Offset: 1, + Total: 1, }, Clients: []mgclients.Client{client}, }, - query: "permission=view", + query: "name=client&offset=1", status: http.StatusOK, err: nil, }, { - desc: "list things with invalid permissions", + desc: "search things with invalid offset", token: validToken, - query: "permission=invalid", + query: "name=clientname&offset=invalid", status: http.StatusBadRequest, err: apiutil.ErrValidation, }, { - desc: "list things with duplicate permissions", - token: validToken, - query: "permission=view&permission=view", - status: http.StatusBadRequest, - err: apiutil.ErrInvalidQueryParams, - }, - { - desc: "list things with list perms", + desc: "search things with limit", token: validToken, - listThingsResponse: mgclients.ClientsPage{ + searchThingsRes: mgclients.ClientsPage{ Page: mgclients.Page{ + Limit: 1, Total: 1, }, Clients: []mgclients.Client{client}, }, - query: "list_perms=true", + query: "name=clientname&limit=1", status: http.StatusOK, err: nil, }, { - desc: "list things with invalid list perms", + desc: "search things with invalid limit", token: validToken, - query: "list_perms=invalid", + query: "name=clientname&limit=invalid", status: http.StatusBadRequest, err: apiutil.ErrValidation, }, - { - desc: "list things with duplicate list perms", - token: validToken, - query: "list_perms=true&listPerms=true", - status: http.StatusBadRequest, - err: apiutil.ErrInvalidQueryParams, - }, } - for _, tc := range cases { req := testRequest{ client: ts.Client(), method: http.MethodGet, - url: ts.URL + "/things?" + tc.query, + url: fmt.Sprintf("%s/things/search?%s", ts.URL, tc.query), contentType: contentType, token: tc.token, } - svcCall := svc.On("ListClients", mock.Anything, tc.token, "", mock.Anything).Return(tc.listThingsResponse, tc.err) + svcCall := svc.On("SearchThings", mock.Anything, tc.token, mock.Anything).Return(tc.searchThingsRes, tc.err) res, err := req.make() assert.Nil(t, err, fmt.Sprintf("%s: unexpected error %s", tc.desc, err)) diff --git a/things/api/http/requests.go b/things/api/http/requests.go index 42ef86424b..c8b39391b4 100644 --- a/things/api/http/requests.go +++ b/things/api/http/requests.go @@ -134,6 +134,23 @@ func (req listMembersReq) validate() error { return nil } +type searchThingsReq struct { + token string + mgclients.Page +} + +func (req searchThingsReq) validate() error { + if req.token == "" { + return apiutil.ErrBearerToken + } + + if req.Name == "" && req.Tag == "" && req.Identity == "" { + return apiutil.ErrEmptySearchQuery + } + + return nil +} + type updateClientReq struct { token string id string diff --git a/things/api/http/requests_test.go b/things/api/http/requests_test.go index e3d9ee5010..f3bc22b88e 100644 --- a/things/api/http/requests_test.go +++ b/things/api/http/requests_test.go @@ -19,6 +19,7 @@ import ( const ( valid = "valid" invalid = "invalid" + name = "client" ) var validID = testsutil.GenerateUUID(&testing.T{}) @@ -319,6 +320,46 @@ func TestListMembersReqValidate(t *testing.T) { } } +func TestSearchThingsReqValidate(t *testing.T) { + cases := []struct { + desc string + req searchThingsReq + err error + }{ + { + desc: "valid request", + req: searchThingsReq{ + token: valid, + Page: mgclients.Page{ + Name: name, + }, + }, + err: nil, + }, + { + desc: "empty token", + req: searchThingsReq{ + token: "", + Page: mgclients.Page{ + Name: name, + }, + }, + err: apiutil.ErrBearerToken, + }, + { + desc: "empty query", + req: searchThingsReq{ + token: valid, + }, + err: apiutil.ErrEmptySearchQuery, + }, + } + for _, c := range cases { + err := c.req.validate() + assert.Equal(t, c.err, err) + } +} + func TestUpdateClientReqValidate(t *testing.T) { cases := []struct { desc string diff --git a/things/api/logging.go b/things/api/logging.go index 164b0ecdfb..7f8b279b33 100644 --- a/things/api/logging.go +++ b/things/api/logging.go @@ -214,6 +214,26 @@ func (lm *loggingMiddleware) ListClientsByGroup(ctx context.Context, token, chan return lm.svc.ListClientsByGroup(ctx, token, channelID, cp) } +func (lm *loggingMiddleware) SearchThings(ctx context.Context, token string, pm mgclients.Page) (cp mgclients.ClientsPage, err error) { + defer func(begin time.Time) { + args := []any{ + slog.String("duration", time.Since(begin).String()), + slog.Group("page", + slog.Uint64("limit", pm.Limit), + slog.Uint64("offset", pm.Offset), + slog.Uint64("total", cp.Total), + ), + } + if err != nil { + args = append(args, slog.Any("error", err)) + lm.logger.Warn("Search things failed to complete successfully", args...) + return + } + lm.logger.Info("Search things completed successfully", args...) + }(time.Now()) + return lm.svc.SearchThings(ctx, token, pm) +} + func (lm *loggingMiddleware) Identify(ctx context.Context, key string) (id string, err error) { defer func(begin time.Time) { args := []any{ diff --git a/things/api/metrics.go b/things/api/metrics.go index 24976d39a8..a58ccda472 100644 --- a/things/api/metrics.go +++ b/things/api/metrics.go @@ -110,6 +110,14 @@ func (ms *metricsMiddleware) ListClientsByGroup(ctx context.Context, token, grou return ms.svc.ListClientsByGroup(ctx, token, groupID, pm) } +func (ms *metricsMiddleware) SearchThings(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { + defer func(begin time.Time) { + ms.counter.With("method", "search_things").Add(1) + ms.latency.With("method", "search_things").Observe(time.Since(begin).Seconds()) + }(time.Now()) + return ms.svc.SearchThings(ctx, token, pm) +} + func (ms *metricsMiddleware) Identify(ctx context.Context, key string) (string, error) { defer func(begin time.Time) { ms.counter.With("method", "identify_thing").Add(1) diff --git a/things/events/events.go b/things/events/events.go index 56b68b6ddc..a5ace9c1ad 100644 --- a/things/events/events.go +++ b/things/events/events.go @@ -20,6 +20,7 @@ const ( clientViewPerms = clientPrefix + "view_perms" clientList = clientPrefix + "list" clientListByGroup = clientPrefix + "list_by_channel" + clientSearch = clientPrefix + "search" clientIdentify = clientPrefix + "identify" clientAuthorize = clientPrefix + "authorize" ) @@ -275,6 +276,31 @@ func (lcge listClientByGroupEvent) Encode() (map[string]interface{}, error) { return val, nil } +type searchThingsEvent struct { + mgclients.Page +} + +func (ste searchThingsEvent) Encode() (map[string]interface{}, error) { + val := map[string]interface{}{ + "operation": clientSearch, + "total": ste.Total, + "offset": ste.Offset, + "limit": ste.Limit, + } + if ste.Name != "" { + val["name"] = ste.Name + } + if ste.Tag != "" { + val["tag"] = ste.Tag + } + if ste.IDs != nil { + ids := fmt.Sprintf("[%s]", strings.Join(ste.IDs, ",")) + val["ids"] = ids + } + + return val, nil +} + type identifyClientEvent struct { thingID string } diff --git a/things/events/streams.go b/things/events/streams.go index ca6a392fb2..825c05ee2b 100644 --- a/things/events/streams.go +++ b/things/events/streams.go @@ -156,6 +156,21 @@ func (es *eventStore) ListClientsByGroup(ctx context.Context, token, chID string return mp, nil } +func (es *eventStore) SearchThings(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { + cp, err := es.svc.SearchThings(ctx, token, pm) + if err != nil { + return cp, err + } + event := searchThingsEvent{ + pm, + } + if err := es.Publish(ctx, event); err != nil { + return cp, err + } + + return cp, nil +} + func (es *eventStore) EnableClient(ctx context.Context, token, id string) (mgclients.Client, error) { cli, err := es.svc.EnableClient(ctx, token, id) if err != nil { diff --git a/things/mocks/service.go b/things/mocks/service.go index efae25fdc6..bfaee35f4b 100644 --- a/things/mocks/service.go +++ b/things/mocks/service.go @@ -242,6 +242,34 @@ func (_m *Service) ListClientsByGroup(ctx context.Context, token string, groupID return r0, r1 } +// SearchThings provides a mock function with given fields: ctx, token, pm +func (_m *Service) SearchThings(ctx context.Context, token string, pm clients.Page) (clients.ClientsPage, error) { + ret := _m.Called(ctx, token, pm) + + if len(ret) == 0 { + panic("no return value specified for SearchThings") + } + + var r0 clients.ClientsPage + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string, clients.Page) (clients.ClientsPage, error)); ok { + return rf(ctx, token, pm) + } + if rf, ok := ret.Get(0).(func(context.Context, string, clients.Page) clients.ClientsPage); ok { + r0 = rf(ctx, token, pm) + } else { + r0 = ret.Get(0).(clients.ClientsPage) + } + + if rf, ok := ret.Get(1).(func(context.Context, string, clients.Page) error); ok { + r1 = rf(ctx, token, pm) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // Share provides a mock function with given fields: ctx, token, id, relation, userids func (_m *Service) Share(ctx context.Context, token string, id string, relation string, userids ...string) error { _va := make([]interface{}, len(userids)) diff --git a/things/postgres/clients.go b/things/postgres/clients.go index e07e88373a..c8387b38c0 100644 --- a/things/postgres/clients.go +++ b/things/postgres/clients.go @@ -6,6 +6,7 @@ package postgres import ( "context" "fmt" + "strings" mgclients "github.com/absmach/magistrala/pkg/clients" pgclients "github.com/absmach/magistrala/pkg/clients/postgres" @@ -121,3 +122,96 @@ func (repo clientRepo) RetrieveBySecret(ctx context.Context, key string) (mgclie return mgclients.Client{}, repoerr.ErrNotFound } + +func (repo clientRepo) Delete(ctx context.Context, id string) error { + q := "DELETE FROM clients AS c WHERE c.id = $1 ;" + + result, err := repo.DB.ExecContext(ctx, q, id) + if err != nil { + return postgres.HandleError(repoerr.ErrRemoveEntity, err) + } + if rows, _ := result.RowsAffected(); rows == 0 { + return repoerr.ErrNotFound + } + + return nil +} + +func (repo clientRepo) SearchBasicInfo(ctx context.Context, pm mgclients.Page) (mgclients.ClientsPage, error) { + sq, tq := constructSearchQuery(pm) + + q := fmt.Sprintf(`SELECT c.id, c.name, c.created_at, c.updated_at FROM clients c %s LIMIT :limit OFFSET :offset;`, sq) + dbPage, err := pgclients.ToDBClientsPage(pm) + if err != nil { + return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) + } + + rows, err := repo.DB.NamedQueryContext(ctx, q, dbPage) + if err != nil { + return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) + } + defer rows.Close() + + var items []mgclients.Client + for rows.Next() { + dbc := pgclients.DBClient{} + if err := rows.StructScan(&dbc); err != nil { + return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrViewEntity, err) + } + + c, err := pgclients.ToClient(dbc) + if err != nil { + return mgclients.ClientsPage{}, err + } + + items = append(items, c) + } + + cq := fmt.Sprintf(`SELECT COUNT(*) FROM clients c %s;`, tq) + total, err := postgres.Total(ctx, repo.DB, cq, dbPage) + if err != nil { + return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrViewEntity, err) + } + + page := mgclients.ClientsPage{ + Clients: items, + Page: mgclients.Page{ + Total: total, + Offset: pm.Offset, + Limit: pm.Limit, + }, + } + + return page, nil +} + +func constructSearchQuery(pm mgclients.Page) (string, string) { + var query []string + var emq string + var tq string + + if pm.Name != "" { + query = append(query, "name ~ :name") + } + if pm.Identity != "" { + query = append(query, "id ~ :identity") + } + if pm.Tag != "" { + query = append(query, ":tag ~ ANY(tags)") + } + + if len(query) > 0 { + emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) + } + + tq = emq + + switch pm.Order { + case "name", "tag", "created_at", "updated_at": + emq = fmt.Sprintf("%s ORDER BY %s", emq, pm.Order) + if pm.Dir == api.AscDir || pm.Dir == api.DescDir { + emq = fmt.Sprintf("%s %s", emq, pm.Dir) + } + } + return emq, tq +} diff --git a/things/service.go b/things/service.go index 8e11233cdb..5646761164 100644 --- a/things/service.go +++ b/things/service.go @@ -530,6 +530,34 @@ func (svc service) ListClientsByGroup(ctx context.Context, token, groupID string }, nil } +func (svc service) SearchThings(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { + res, err := svc.identify(ctx, token) + if err != nil { + return mgclients.ClientsPage{}, err + } + + if _, err := svc.authorize(ctx, "", auth.UserType, auth.UsersKind, res.GetId(), auth.MembershipPermission, auth.DomainType, res.GetDomainId()); err != nil { + return mgclients.ClientsPage{}, err + } + + cp, err := svc.clients.SearchBasicInfo(ctx, pm) + if err != nil { + return mgclients.ClientsPage{}, err + } + + things := mgclients.ClientsPage{} + for _, val := range cp.Clients { + if _, err := svc.authorize(ctx, res.GetDomainId(), auth.UserType, auth.UsersKind, res.GetId(), auth.AdminPermission, auth.ThingType, val.ID); err == nil { + things.Clients = append(things.Clients, val) + things.Page.Total += 1 + } + } + things.Page.Offset = cp.Offset + things.Page.Limit = cp.Limit + + return things, nil +} + func (svc service) Identify(ctx context.Context, key string) (string, error) { id, err := svc.clientCache.ID(ctx, key) if err == nil { diff --git a/things/things.go b/things/things.go index 270d59e131..e8e85b0fc5 100644 --- a/things/things.go +++ b/things/things.go @@ -33,6 +33,9 @@ type Service interface { // the provided key. ListClientsByGroup(ctx context.Context, token, groupID string, pm clients.Page) (clients.MembersPage, error) + // SearchThings searches for things with provided filters for a valid auth token. + SearchThings(ctx context.Context, token string, pm clients.Page) (clients.ClientsPage, error) + // UpdateClient updates the client's name and metadata. UpdateClient(ctx context.Context, token string, client clients.Client) (clients.Client, error) diff --git a/things/tracing/tracing.go b/things/tracing/tracing.go index f097163279..c3f7959bfe 100644 --- a/things/tracing/tracing.go +++ b/things/tracing/tracing.go @@ -54,6 +54,12 @@ func (tm *tracingMiddleware) ListClients(ctx context.Context, token, reqUserID s return tm.svc.ListClients(ctx, token, reqUserID, pm) } +func (tm *tracingMiddleware) SearchThings(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { + ctx, span := tm.tracer.Start(ctx, "svc_search_things") + defer span.End() + return tm.svc.SearchThings(ctx, token, pm) +} + // UpdateClient traces the "UpdateClient" operation of the wrapped policies.Service. func (tm *tracingMiddleware) UpdateClient(ctx context.Context, token string, cli mgclients.Client) (mgclients.Client, error) { ctx, span := tm.tracer.Start(ctx, "svc_update_client_name_and_metadata", trace.WithAttributes(attribute.String("id", cli.ID))) From 4026de0987206164fa37f5a7ddde33d55e48d8f5 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Mon, 24 Jun 2024 01:14:16 +0300 Subject: [PATCH 02/31] Added sdk and cli support Signed-off-by: nyagamunene --- cli/things.go | 43 +++++++++++++ internal/api/common.go | 1 + pkg/apiutil/errors.go | 3 + pkg/sdk/go/sdk.go | 11 ++++ pkg/sdk/go/things.go | 19 ++++++ pkg/sdk/go/things_test.go | 130 ++++++++++++++++++++++++++++++++++++++ pkg/sdk/mocks/sdk.go | 30 +++++++++ things/service_test.go | 51 +++++++++++++++ 8 files changed, 288 insertions(+) diff --git a/cli/things.go b/cli/things.go index f7c1f4b4bc..57c1d44568 100644 --- a/cli/things.go +++ b/cli/things.go @@ -5,6 +5,9 @@ package cli import ( "encoding/json" + "fmt" + "net/url" + "strconv" mgclients "github.com/absmach/magistrala/pkg/clients" mgxsdk "github.com/absmach/magistrala/pkg/sdk/go" @@ -362,6 +365,46 @@ var cmdThings = []cobra.Command{ logJSON(ul) }, }, + { + Use: "search ", + Short: "Search things", + Long: "Search things by name, id or tags\n" + + "Usage:\n" + + "\tmagistrala-cli things search \n", + Run: func(cmd *cobra.Command, args []string) { + if len(args) != 2 { + logUsage(cmd.Use) + return + } + + values, err := url.ParseQuery(args[0]) + if err != nil { + logError(fmt.Errorf("Failed to parse query: %s", err)) + } + + pm := mgxsdk.PageMetadata{ + Name: values.Get("name"), + Tag: values.Get("tags"), + Identity: values.Get("id"), + } + + if off, err := strconv.Atoi(values.Get("offset")); err == nil { + pm.Offset = uint64(off) + } + + if lim, err := strconv.Atoi(values.Get("limit")); err == nil { + pm.Limit = uint64(lim) + } + + things, err := sdk.SearchThings(pm, args[1]) + if err != nil { + logError(err) + return + } + + logJSON(things) + }, + }, } // NewThingsCmd returns things command. diff --git a/internal/api/common.go b/internal/api/common.go index e7872a9968..68388e0254 100644 --- a/internal/api/common.go +++ b/internal/api/common.go @@ -124,6 +124,7 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { errors.Contains(err, errors.ErrMalformedEntity), errors.Contains(err, apiutil.ErrMissingID), errors.Contains(err, apiutil.ErrMissingName), + errors.Contains(err, apiutil.ErrMissingAlias), errors.Contains(err, apiutil.ErrMissingEmail), errors.Contains(err, apiutil.ErrMissingHost), errors.Contains(err, apiutil.ErrInvalidResetPass), diff --git a/pkg/apiutil/errors.go b/pkg/apiutil/errors.go index 3d36dd7dec..8a5fffde05 100644 --- a/pkg/apiutil/errors.go +++ b/pkg/apiutil/errors.go @@ -135,6 +135,9 @@ var ( // ErrMissingName indicates missing identity name. ErrMissingName = errors.New("missing identity name") + // ErrMissingName indicates missing alias. + ErrMissingAlias = errors.New("missing alias") + // ErrInvalidLevel indicates an invalid group level. ErrInvalidLevel = errors.New("invalid group level (should be between 0 and 5)") diff --git a/pkg/sdk/go/sdk.go b/pkg/sdk/go/sdk.go index 4b63c86273..84902347e8 100644 --- a/pkg/sdk/go/sdk.go +++ b/pkg/sdk/go/sdk.go @@ -484,6 +484,17 @@ type SDK interface { // fmt.Println(users) ListThingUsers(thingID string, pm PageMetadata, token string) (UsersPage, errors.SDKError) + // SearchThings returns page of things based on the search criteria. + // + // example: + // pm := sdk.PageMetadata{ + // Offset: 0, + // Limit: 10, + // Name: "My Thing", + // } + // things, _ := sdk.SearchThings(pm, "token") + // fmt.Println(things) + SearchThings(pm PageMetadata, token string) (ThingsPage, errors.SDKError) // DeleteThing deletes a thing with the given id. // // example: diff --git a/pkg/sdk/go/things.go b/pkg/sdk/go/things.go index 16f629416d..d6bc647dd4 100644 --- a/pkg/sdk/go/things.go +++ b/pkg/sdk/go/things.go @@ -295,6 +295,25 @@ func (sdk mgSDK) ListThingUsers(thingID string, pm PageMetadata, token string) ( return up, nil } +func (sdk mgSDK) SearchThings(pm PageMetadata, token string) (ThingsPage, errors.SDKError) { + url, err := sdk.withQueryParams(sdk.thingsURL, fmt.Sprintf("%s/search", thingsEndpoint), pm) + if err != nil { + return ThingsPage{}, errors.NewSDKError(err) + } + + _, body, sdkerr := sdk.processRequest(http.MethodGet, url, token, nil, nil, http.StatusOK) + if sdkerr != nil { + return ThingsPage{}, sdkerr + } + + var tp ThingsPage + if err := json.Unmarshal(body, &tp); err != nil { + return ThingsPage{}, errors.NewSDKError(err) + } + + return tp, nil +} + func (sdk mgSDK) DeleteThing(id, token string) errors.SDKError { url := fmt.Sprintf("%s/%s/%s", sdk.thingsURL, thingsEndpoint, id) _, _, sdkerr := sdk.processRequest(http.MethodDelete, url, token, nil, nil, http.StatusNoContent) diff --git a/pkg/sdk/go/things_test.go b/pkg/sdk/go/things_test.go index 2c56c988d9..65bbda1e85 100644 --- a/pkg/sdk/go/things_test.go +++ b/pkg/sdk/go/things_test.go @@ -607,6 +607,136 @@ func TestListThingsByChannel(t *testing.T) { } } +func TestSearchThings(t *testing.T) { + ts, cRepo, _, auth, _ := setupThings() + defer ts.Close() + + var ths []sdk.Thing + conf := sdk.Config{ + ThingsURL: ts.URL, + } + mgsdk := sdk.NewSDK(conf) + + for i := 10; i < 100; i++ { + th := sdk.Thing{ + ID: generateUUID(t), + Name: fmt.Sprintf("thing_%d", i), + Credentials: sdk.Credentials{ + Identity: fmt.Sprintf("identity_%d", i), + Secret: generateUUID(t), + }, + Metadata: sdk.Metadata{"name": fmt.Sprintf("thing_%d", i)}, + Status: mgclients.EnabledStatus.String(), + } + if i == 50 { + th.Status = mgclients.DisabledStatus.String() + th.Tags = []string{"tag1", "tag2"} + } + ths = append(ths, th) + } + + cases := []struct { + desc string + token string + page sdk.PageMetadata + response []sdk.Thing + authorizeRes *magistrala.AuthorizeRes + searchRes mgclients.ClientsPage + identifyRes *magistrala.IdentityRes + err errors.SDKError + identifyErr error + authorizeErr error + }{ + { + desc: "search things with authorized token", + token: validToken, + identifyRes: &magistrala.IdentityRes{Id: validID, DomainId: testsutil.GenerateUUID(t)}, + authorizeRes: &magistrala.AuthorizeRes{Authorized: true}, + page: sdk.PageMetadata{ + Offset: offset, + Limit: limit, + Name: "thing_10", + }, + response: []sdk.Thing{ths[10]}, + searchRes: mgclients.ClientsPage{ + Page: mgclients.Page{ + Total: 1, + Offset: offset, + Limit: limit, + }, + Clients: convertThings(ths[10]), + }, + err: nil, + }, + { + desc: "search things with invalid token", + token: invalidToken, + page: sdk.PageMetadata{ + Offset: offset, + Limit: limit, + Name: "thing_10", + }, + err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), + response: nil, + identifyErr: svcerr.ErrAuthentication, + }, + { + desc: "search things with empty token", + token: "", + page: sdk.PageMetadata{ + Offset: offset, + Limit: limit, + Name: "thing_10", + }, + err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrBearerToken), http.StatusUnauthorized), + response: nil, + }, + { + desc: "search things with empty query", + token: validToken, + page: sdk.PageMetadata{ + Offset: offset, + Limit: limit, + Name: "", + }, + err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrEmptySearchQuery), http.StatusBadRequest), + }, + { + desc: "search for things with invalid length of query", + token: validToken, + page: sdk.PageMetadata{ + Offset: offset, + Limit: limit, + Name: "a", + }, + err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrLenSearchQuery, apiutil.ErrValidation), http.StatusBadRequest), + }, + { + desc: "search for things with no domain id", + token: validToken, + page: sdk.PageMetadata{ + Offset: offset, + Limit: limit, + Name: "thing_10", + }, + identifyRes: &magistrala.IdentityRes{Id: validID}, + authorizeRes: &magistrala.AuthorizeRes{Authorized: false}, + err: errors.NewSDKErrorWithStatus(svcerr.ErrDomainAuthorization, http.StatusForbidden), + }, + } + for _, tc := range cases { + repoCall := auth.On("Identify", mock.Anything, &magistrala.IdentityReq{Token: tc.token}).Return(tc.identifyRes, tc.identifyErr) + repoCall1 := auth.On("Authorize", mock.Anything, mock.Anything).Return(tc.authorizeRes, tc.authorizeErr) + repoCall2 := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(tc.searchRes, tc.err) + page, err := mgsdk.SearchThings(tc.page, tc.token) + assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err)) + assert.Equal(t, tc.response, page.Things, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, page)) + repoCall.Unset() + repoCall1.Unset() + repoCall2.Unset() + } +} + func TestThing(t *testing.T) { ts, cRepo, _, auth, _ := setupThings() defer ts.Close() diff --git a/pkg/sdk/mocks/sdk.go b/pkg/sdk/mocks/sdk.go index 5fbc8033cf..208d184932 100644 --- a/pkg/sdk/mocks/sdk.go +++ b/pkg/sdk/mocks/sdk.go @@ -2050,6 +2050,36 @@ func (_m *SDK) RevokeCert(thingID string, token string) (time.Time, errors.SDKEr return r0, r1 } +// SearchThings provides a mock function with given fields: pm, token +func (_m *SDK) SearchThings(pm sdk.PageMetadata, token string) (sdk.ThingsPage, errors.SDKError) { + ret := _m.Called(pm, token) + + if len(ret) == 0 { + panic("no return value specified for SearchThings") + } + + var r0 sdk.ThingsPage + var r1 errors.SDKError + if rf, ok := ret.Get(0).(func(sdk.PageMetadata, string) (sdk.ThingsPage, errors.SDKError)); ok { + return rf(pm, token) + } + if rf, ok := ret.Get(0).(func(sdk.PageMetadata, string) sdk.ThingsPage); ok { + r0 = rf(pm, token) + } else { + r0 = ret.Get(0).(sdk.ThingsPage) + } + + if rf, ok := ret.Get(1).(func(sdk.PageMetadata, string) errors.SDKError); ok { + r1 = rf(pm, token) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(errors.SDKError) + } + } + + return r0, r1 +} + // SendInvitation provides a mock function with given fields: invitation, token func (_m *SDK) SendInvitation(invitation sdk.Invitation, token string) error { ret := _m.Called(invitation, token) diff --git a/things/service_test.go b/things/service_test.go index 810bcda99d..fb99e4b745 100644 --- a/things/service_test.go +++ b/things/service_test.go @@ -1593,6 +1593,57 @@ func TestListMembers(t *testing.T) { } } +func TestSearchThings(t *testing.T) { + svc, cRepo, auth, _ := newService() + + cases := []struct { + desc string + token string + page mgclients.Page + identifyResponse *magistrala.IdentityRes + authorizeResponse *magistrala.AuthorizeRes + searchResponse mgclients.ClientsPage + responseErr error + authorizeErr error + identifyErr error + err error + }{ + { + desc: "search clients with valid token", + token: validToken, + page: mgclients.Page{Offset: 0, Limit: 100, Name: "clientname"}, + searchResponse: mgclients.ClientsPage{ + Page: mgclients.Page{Total: 1, Offset: 0, Limit: 100}, + Clients: []mgclients.Client{client}, + }, + identifyResponse: &magistrala.IdentityRes{Id: client.ID, DomainId: testsutil.GenerateUUID(t)}, + authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, + identifyErr: nil, + err: nil, + }, + { + desc: "search clients with invalid token", + token: inValidToken, + page: mgclients.Page{Offset: 0, Limit: 100, Name: "clientname"}, + searchResponse: mgclients.ClientsPage{}, + identifyErr: svcerr.ErrAuthentication, + err: svcerr.ErrAuthentication, + }, + } + + for _, tc := range cases { + authCall := auth.On("Identify", context.Background(), &magistrala.IdentityReq{Token: tc.token}).Return(tc.identifyResponse, tc.identifyErr) + repoCall := auth.On("Authorize", mock.Anything, mock.Anything).Return(tc.authorizeResponse, tc.authorizeErr) + repoCall1 := cRepo.On("SearchBasicInfo", context.Background(), tc.page).Return(tc.searchResponse, tc.responseErr) + page, err := svc.SearchThings(context.Background(), tc.token, tc.page) + assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) + assert.Equal(t, tc.searchResponse, page, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.searchResponse, page)) + authCall.Unset() + repoCall.Unset() + repoCall1.Unset() + } +} + func TestDeleteClient(t *testing.T) { svc, cRepo, auth, cache := newService() From 1c02d5a83b53a46793b9644c266050a9f49f01c6 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Mon, 24 Jun 2024 01:49:52 +0300 Subject: [PATCH 03/31] Fixed decodeseachclients Signed-off-by: nyagamunene --- things/api/http/clients.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/things/api/http/clients.go b/things/api/http/clients.go index df86c46fd7..4a6538b038 100644 --- a/things/api/http/clients.go +++ b/things/api/http/clients.go @@ -217,10 +217,6 @@ func decodeListClients(_ context.Context, r *http.Request) (interface{}, error) } func decodeSearchClients(_ context.Context, r *http.Request) (interface{}, error) { - s, err := apiutil.ReadStringQuery(r, api.StatusKey, api.DefClientStatus) - if err != nil { - return nil, errors.Wrap(apiutil.ErrValidation, err) - } o, err := apiutil.ReadNumQuery[uint64](r, api.OffsetKey, api.DefOffset) if err != nil { return nil, errors.Wrap(apiutil.ErrValidation, err) @@ -237,10 +233,7 @@ func decodeSearchClients(_ context.Context, r *http.Request) (interface{}, error if err != nil { return nil, errors.Wrap(apiutil.ErrValidation, err) } - st, err := mgclients.ToStatus(s) - if err != nil { - return nil, errors.Wrap(apiutil.ErrValidation, err) - } + id, err := apiutil.ReadStringQuery(r, api.IDOrder, "") if err != nil { return nil, errors.Wrap(apiutil.ErrValidation, err) @@ -248,7 +241,7 @@ func decodeSearchClients(_ context.Context, r *http.Request) (interface{}, error req := searchThingsReq{ apiutil.ExtractBearerToken(r), - mgclients.Page{Status: st, Offset: o, Limit: l, Name: n, Tag: t, Identity: id}, + mgclients.Page{Offset: o, Limit: l, Name: n, Tag: t, Identity: id}, } for _, field := range []string{req.Name, req.Identity, req.Tag} { From 3dd49ef7c793d4cf3f1754d0fdcc96089f8dc46f Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Thu, 27 Jun 2024 16:59:59 +0300 Subject: [PATCH 04/31] Added errors description Signed-off-by: nyagamunene --- pkg/apiutil/errors.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/pkg/apiutil/errors.go b/pkg/apiutil/errors.go index 8a5fffde05..4e8fd9c5ce 100644 --- a/pkg/apiutil/errors.go +++ b/pkg/apiutil/errors.go @@ -171,16 +171,8 @@ var ( // ErrEmptyMessage indicates empty message. ErrEmptyMessage = errors.New("empty message") - // ErrMissingEntityType indicates missing entity type. - ErrMissingEntityType = errors.New("missing entity type") - - // ErrInvalidEntityType indicates invalid entity type. - ErrInvalidEntityType = errors.New("invalid entity type") - - // ErrInvalidTimeFormat indicates invalid time format i.e not unix time. - ErrInvalidTimeFormat = errors.New("invalid time format use unix time") - ErrEmptySearchQuery = errors.New("search query must not be empty") + // ErrLenSearchQuery indicates search query length. ErrLenSearchQuery = errors.New("search query must be at least 3 characters") ) From f5f3821b1f5dd3dda2e838d760ef93da27ffcab1 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Thu, 27 Jun 2024 19:55:08 +0300 Subject: [PATCH 05/31] Linted code Signed-off-by: nyagamunene --- things/events/events.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/things/events/events.go b/things/events/events.go index a5ace9c1ad..52e1d17ade 100644 --- a/things/events/events.go +++ b/things/events/events.go @@ -4,6 +4,8 @@ package events import ( + "fmt" + "strings" "time" mgclients "github.com/absmach/magistrala/pkg/clients" From ef6bf8cefc34f111b269c401851b54462b05c7ab Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Sat, 29 Jun 2024 02:19:13 +0300 Subject: [PATCH 06/31] Fixed search with tags Signed-off-by: nyagamunene --- things/postgres/clients.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/things/postgres/clients.go b/things/postgres/clients.go index c8387b38c0..03c91d6cc2 100644 --- a/things/postgres/clients.go +++ b/things/postgres/clients.go @@ -8,6 +8,7 @@ import ( "fmt" "strings" + "github.com/absmach/magistrala/internal/api" mgclients "github.com/absmach/magistrala/pkg/clients" pgclients "github.com/absmach/magistrala/pkg/clients/postgres" "github.com/absmach/magistrala/pkg/errors" @@ -191,13 +192,13 @@ func constructSearchQuery(pm mgclients.Page) (string, string) { var tq string if pm.Name != "" { - query = append(query, "name ~ :name") + query = append(query, "name ~* :name") } if pm.Identity != "" { - query = append(query, "id ~ :identity") + query = append(query, "id ~* :identity") } if pm.Tag != "" { - query = append(query, ":tag ~ ANY(tags)") + query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") } if len(query) > 0 { From 48cd4551496ff2647a7e497b6d16b57c2cc9132c Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Mon, 1 Jul 2024 00:07:43 +0300 Subject: [PATCH 07/31] Fixed search with ids Signed-off-by: nyagamunene --- cli/things.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/things.go b/cli/things.go index 57c1d44568..f02e534487 100644 --- a/cli/things.go +++ b/cli/things.go @@ -383,9 +383,9 @@ var cmdThings = []cobra.Command{ } pm := mgxsdk.PageMetadata{ - Name: values.Get("name"), - Tag: values.Get("tags"), - Identity: values.Get("id"), + Name: values.Get("name"), + Tag: values.Get("tags"), + ID: values.Get("id"), } if off, err := strconv.Atoi(values.Get("offset")); err == nil { From 439a6e0549b78986b14026b5018de05c300bfdc6 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Mon, 1 Jul 2024 01:27:07 +0300 Subject: [PATCH 08/31] changed database operation Signed-off-by: nyagamunene --- cli/things.go | 2 +- pkg/clients/postgres/clients.go | 31 +++++++++++++++++++++++++++ things/api/http/clients.go | 4 ++-- things/api/http/requests.go | 2 +- things/postgres/clients.go | 37 ++------------------------------- 5 files changed, 37 insertions(+), 39 deletions(-) diff --git a/cli/things.go b/cli/things.go index f02e534487..a38ab41d2c 100644 --- a/cli/things.go +++ b/cli/things.go @@ -384,8 +384,8 @@ var cmdThings = []cobra.Command{ pm := mgxsdk.PageMetadata{ Name: values.Get("name"), - Tag: values.Get("tags"), ID: values.Get("id"), + Tag: values.Get("tag"), } if off, err := strconv.Atoi(values.Get("offset")); err == nil { diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index d9cf9e337c..60d9aff071 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -554,3 +554,34 @@ func ConstructSearchQuery(pm clients.Page) (string, string) { return emq, tq } + +func ConstructThingSearchQuery(pm clients.Page) (string, string) { + var query []string + var emq string + var tq string + + if pm.Name != "" { + query = append(query, "name ILIKE :name") + } + if pm.ID != "" { + query = append(query, "id ILIKE :id") + } + if pm.Tag != "" { + query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") + } + + if len(query) > 0 { + emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) + } + + tq = emq + + switch pm.Order { + case "name", "tag", "created_at", "updated_at": + emq = fmt.Sprintf("%s ORDER BY %s", emq, pm.Order) + if pm.Dir == api.AscDir || pm.Dir == api.DescDir { + emq = fmt.Sprintf("%s %s", emq, pm.Dir) + } + } + return emq, tq +} diff --git a/things/api/http/clients.go b/things/api/http/clients.go index 4a6538b038..488852d9d2 100644 --- a/things/api/http/clients.go +++ b/things/api/http/clients.go @@ -241,10 +241,10 @@ func decodeSearchClients(_ context.Context, r *http.Request) (interface{}, error req := searchThingsReq{ apiutil.ExtractBearerToken(r), - mgclients.Page{Offset: o, Limit: l, Name: n, Tag: t, Identity: id}, + mgclients.Page{Offset: o, Limit: l, Name: n, Tag: t, ID: id}, } - for _, field := range []string{req.Name, req.Identity, req.Tag} { + for _, field := range []string{req.Name, req.ID, req.Tag} { if field != "" && len(field) < 3 { req = searchThingsReq{ token: apiutil.ExtractBearerToken(r), diff --git a/things/api/http/requests.go b/things/api/http/requests.go index c8b39391b4..412d59e5a1 100644 --- a/things/api/http/requests.go +++ b/things/api/http/requests.go @@ -144,7 +144,7 @@ func (req searchThingsReq) validate() error { return apiutil.ErrBearerToken } - if req.Name == "" && req.Tag == "" && req.Identity == "" { + if req.Name == "" && req.Tag == "" && req.ID == "" { return apiutil.ErrEmptySearchQuery } diff --git a/things/postgres/clients.go b/things/postgres/clients.go index 03c91d6cc2..15505ecb42 100644 --- a/things/postgres/clients.go +++ b/things/postgres/clients.go @@ -6,9 +6,7 @@ package postgres import ( "context" "fmt" - "strings" - "github.com/absmach/magistrala/internal/api" mgclients "github.com/absmach/magistrala/pkg/clients" pgclients "github.com/absmach/magistrala/pkg/clients/postgres" "github.com/absmach/magistrala/pkg/errors" @@ -139,14 +137,14 @@ func (repo clientRepo) Delete(ctx context.Context, id string) error { } func (repo clientRepo) SearchBasicInfo(ctx context.Context, pm mgclients.Page) (mgclients.ClientsPage, error) { - sq, tq := constructSearchQuery(pm) + sq, tq := pgclients.ConstructThingSearchQuery(pm) q := fmt.Sprintf(`SELECT c.id, c.name, c.created_at, c.updated_at FROM clients c %s LIMIT :limit OFFSET :offset;`, sq) dbPage, err := pgclients.ToDBClientsPage(pm) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } - + fmt.Println("query: ", q) rows, err := repo.DB.NamedQueryContext(ctx, q, dbPage) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) @@ -185,34 +183,3 @@ func (repo clientRepo) SearchBasicInfo(ctx context.Context, pm mgclients.Page) ( return page, nil } - -func constructSearchQuery(pm mgclients.Page) (string, string) { - var query []string - var emq string - var tq string - - if pm.Name != "" { - query = append(query, "name ~* :name") - } - if pm.Identity != "" { - query = append(query, "id ~* :identity") - } - if pm.Tag != "" { - query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") - } - - if len(query) > 0 { - emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) - } - - tq = emq - - switch pm.Order { - case "name", "tag", "created_at", "updated_at": - emq = fmt.Sprintf("%s ORDER BY %s", emq, pm.Order) - if pm.Dir == api.AscDir || pm.Dir == api.DescDir { - emq = fmt.Sprintf("%s %s", emq, pm.Dir) - } - } - return emq, tq -} From 654ea9f866413fad0dc4a7ac8970c4b76700a413 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Mon, 1 Jul 2024 01:45:57 +0300 Subject: [PATCH 09/31] Refactor: Fixed search with ids Signed-off-by: nyagamunene --- pkg/clients/postgres/clients.go | 2 +- things/api/http/clients.go | 10 +++++----- things/api/http/endpoints.go | 19 +++++++++---------- things/api/http/requests.go | 2 +- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index 60d9aff071..9db3fd96d2 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -563,7 +563,7 @@ func ConstructThingSearchQuery(pm clients.Page) (string, string) { if pm.Name != "" { query = append(query, "name ILIKE :name") } - if pm.ID != "" { + if pm.Id != "" { query = append(query, "id ILIKE :id") } if pm.Tag != "" { diff --git a/things/api/http/clients.go b/things/api/http/clients.go index 488852d9d2..3fd2a93638 100644 --- a/things/api/http/clients.go +++ b/things/api/http/clients.go @@ -61,8 +61,8 @@ func clientsHandler(svc things.Service, r *chi.Mux, logger *slog.Logger) http.Ha ), "view_thing_permissions").ServeHTTP) r.Get("/search", otelhttp.NewHandler(kithttp.NewServer( - searchClientsEndpoint(svc), - decodeSearchClients, + searchThingsEndpoint(svc), + decodeSearchThings, api.EncodeResponse, opts..., ), "search_things").ServeHTTP) @@ -216,7 +216,7 @@ func decodeListClients(_ context.Context, r *http.Request) (interface{}, error) return req, nil } -func decodeSearchClients(_ context.Context, r *http.Request) (interface{}, error) { +func decodeSearchThings(_ context.Context, r *http.Request) (interface{}, error) { o, err := apiutil.ReadNumQuery[uint64](r, api.OffsetKey, api.DefOffset) if err != nil { return nil, errors.Wrap(apiutil.ErrValidation, err) @@ -241,10 +241,10 @@ func decodeSearchClients(_ context.Context, r *http.Request) (interface{}, error req := searchThingsReq{ apiutil.ExtractBearerToken(r), - mgclients.Page{Offset: o, Limit: l, Name: n, Tag: t, ID: id}, + mgclients.Page{Offset: o, Limit: l, Name: n, Tag: t, Id: id}, } - for _, field := range []string{req.Name, req.ID, req.Tag} { + for _, field := range []string{req.Name, req.Id, req.Tag} { if field != "" && len(field) < 3 { req = searchThingsReq{ token: apiutil.ExtractBearerToken(r), diff --git a/things/api/http/endpoints.go b/things/api/http/endpoints.go index b7cf0951e3..4db67b9a90 100644 --- a/things/api/http/endpoints.go +++ b/things/api/http/endpoints.go @@ -147,7 +147,7 @@ func listMembersEndpoint(svc things.Service) endpoint.Endpoint { } } -func searchClientsEndpoint(svc things.Service) endpoint.Endpoint { +func searchThingsEndpoint(svc things.Service) endpoint.Endpoint { return func(ctx context.Context, request interface{}) (interface{}, error) { req := request.(searchThingsReq) if err := req.validate(); err != nil { @@ -155,15 +155,14 @@ func searchClientsEndpoint(svc things.Service) endpoint.Endpoint { } pm := mgclients.Page{ - Status: req.Status, - Offset: req.Offset, - Limit: req.Limit, - Name: req.Name, - Tag: req.Tag, - Metadata: req.Metadata, - Identity: req.Identity, - Order: req.Order, - Dir: req.Dir, + Status: req.Status, + Offset: req.Offset, + Limit: req.Limit, + Name: req.Name, + Id: req.Id, + Tag: req.Tag, + Order: req.Order, + Dir: req.Dir, } page, err := svc.SearchThings(ctx, req.token, pm) if err != nil { diff --git a/things/api/http/requests.go b/things/api/http/requests.go index 412d59e5a1..14826c433e 100644 --- a/things/api/http/requests.go +++ b/things/api/http/requests.go @@ -144,7 +144,7 @@ func (req searchThingsReq) validate() error { return apiutil.ErrBearerToken } - if req.Name == "" && req.Tag == "" && req.ID == "" { + if req.Name == "" && req.Tag == "" && req.Id == "" { return apiutil.ErrEmptySearchQuery } From 4c29013463eacb2fc2e617d4e6e91e1123a8e3a4 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Mon, 1 Jul 2024 12:13:16 +0300 Subject: [PATCH 10/31] Add id field in clients and database page Signed-off-by: nyagamunene --- pkg/clients/postgres/clients.go | 4 ++-- things/api/http/endpoints.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index 9db3fd96d2..d3c635fd9a 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -561,10 +561,10 @@ func ConstructThingSearchQuery(pm clients.Page) (string, string) { var tq string if pm.Name != "" { - query = append(query, "name ILIKE :name") + query = append(query, "name ILIKE '%' || :name || '%'") } if pm.Id != "" { - query = append(query, "id ILIKE :id") + query = append(query, "id ILIKE '%' || :id || '%'") } if pm.Tag != "" { query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") diff --git a/things/api/http/endpoints.go b/things/api/http/endpoints.go index 4db67b9a90..50f201fe26 100644 --- a/things/api/http/endpoints.go +++ b/things/api/http/endpoints.go @@ -155,7 +155,6 @@ func searchThingsEndpoint(svc things.Service) endpoint.Endpoint { } pm := mgclients.Page{ - Status: req.Status, Offset: req.Offset, Limit: req.Limit, Name: req.Name, From 744f8c1e5c814efc0775619a8ce79d0b6354102a Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Mon, 1 Jul 2024 12:39:13 +0300 Subject: [PATCH 11/31] Remove delete in things postgres Signed-off-by: nyagamunene --- pkg/clients/clients.go | 4 ++-- things/postgres/clients.go | 17 +++-------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/pkg/clients/clients.go b/pkg/clients/clients.go index 68a8416052..aa7202d9c8 100644 --- a/pkg/clients/clients.go +++ b/pkg/clients/clients.go @@ -78,8 +78,8 @@ type Repository interface { // RetrieveAll retrieves all clients. RetrieveAll(ctx context.Context, pm Page) (ClientsPage, error) - // SearchBasicInfo list all clients only with basic information. - SearchBasicInfo(ctx context.Context, pm Page) (ClientsPage, error) + // RetrieveAllBasicInfo list all clients only with basic information. + RetrieveAllBasicInfo(ctx context.Context, pm Page) (ClientsPage, error) // RetrieveAllByIDs retrieves for given client IDs . RetrieveAllByIDs(ctx context.Context, pm Page) (ClientsPage, error) diff --git a/things/postgres/clients.go b/things/postgres/clients.go index 15505ecb42..c4bdc1a93b 100644 --- a/things/postgres/clients.go +++ b/things/postgres/clients.go @@ -33,6 +33,9 @@ type Repository interface { // RetrieveBySecret retrieves a client based on the secret (key). RetrieveBySecret(ctx context.Context, key string) (mgclients.Client, error) + + // SearchBasicInfo retrieves basic information about clients. + SearchBasicInfo(ctx context.Context, pm mgclients.Page) (mgclients.ClientsPage, error) } // NewRepository instantiates a PostgreSQL @@ -122,20 +125,6 @@ func (repo clientRepo) RetrieveBySecret(ctx context.Context, key string) (mgclie return mgclients.Client{}, repoerr.ErrNotFound } -func (repo clientRepo) Delete(ctx context.Context, id string) error { - q := "DELETE FROM clients AS c WHERE c.id = $1 ;" - - result, err := repo.DB.ExecContext(ctx, q, id) - if err != nil { - return postgres.HandleError(repoerr.ErrRemoveEntity, err) - } - if rows, _ := result.RowsAffected(); rows == 0 { - return repoerr.ErrNotFound - } - - return nil -} - func (repo clientRepo) SearchBasicInfo(ctx context.Context, pm mgclients.Page) (mgclients.ClientsPage, error) { sq, tq := pgclients.ConstructThingSearchQuery(pm) From 2b3ac8baaf70a5a568ab4be18e51b625b738eddb Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Mon, 1 Jul 2024 13:00:18 +0300 Subject: [PATCH 12/31] Add things search api docs Signed-off-by: nyagamunene --- api/openapi/things.yml | 28 ++++++++++++++ pkg/apiutil/errors.go | 10 +++++ pkg/clients/postgres/clients_test.go | 4 +- pkg/sdk/go/sdk.go | 1 + things/mocks/repository.go | 28 ++++++++++++++ users/mocks/repository.go | 56 ++++++++++++++-------------- 6 files changed, 97 insertions(+), 30 deletions(-) diff --git a/api/openapi/things.yml b/api/openapi/things.yml index 8deae43c9b..62b6df7a5b 100644 --- a/api/openapi/things.yml +++ b/api/openapi/things.yml @@ -401,6 +401,34 @@ paths: "500": $ref: "#/components/responses/ServiceError" + /things/search: + get: + operationId: searchThings + summary: Searches for things + description: | + Searches for things based on the provided query parameters i.e name, id and tags. + tags: + - Things + parameters: + - $ref: "#/components/parameters/ThingName" + - $ref: "#/components/parameters/ThingID" + - $ref: "#/components/parameters/Tags" + - $ref: "#/components/parameters/Offset" + - $ref: "#/components/parameters/Limit" + responses: + "200": + $ref: "#/components/responses/ThingPageRes" + "400": + description: Failed due to malformed query parameters. + "401": + description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. + "422": + description: Database can't process request. + "500": + $ref: "#/components/responses/ServiceError" + /channels/{chanID}/things: get: operationId: listThingsInaChannel diff --git a/pkg/apiutil/errors.go b/pkg/apiutil/errors.go index 4e8fd9c5ce..f09b94fad7 100644 --- a/pkg/apiutil/errors.go +++ b/pkg/apiutil/errors.go @@ -171,6 +171,16 @@ var ( // ErrEmptyMessage indicates empty message. ErrEmptyMessage = errors.New("empty message") + // ErrMissingEntityType indicates missing entity type. + ErrMissingEntityType = errors.New("missing entity type") + + // ErrInvalidEntityType indicates invalid entity type. + ErrInvalidEntityType = errors.New("invalid entity type") + + // ErrInvalidTimeFormat indicates invalid time format i.e not unix time. + ErrInvalidTimeFormat = errors.New("invalid time format use unix time") + + // ErrEmptySearchQuery indicates empty search query. ErrEmptySearchQuery = errors.New("search query must not be empty") // ErrLenSearchQuery indicates search query length. diff --git a/pkg/clients/postgres/clients_test.go b/pkg/clients/postgres/clients_test.go index a5c0c01bbd..178fcdf4ab 100644 --- a/pkg/clients/postgres/clients_test.go +++ b/pkg/clients/postgres/clients_test.go @@ -927,7 +927,7 @@ func TestRetrieveByIDs(t *testing.T) { } } -func TestSearchBasicInfo(t *testing.T) { +func TestRetrieveAllBasicInfo(t *testing.T) { t.Cleanup(func() { _, err := db.Exec("DELETE FROM clients") require.Nil(t, err, fmt.Sprintf("clean clients unexpected error: %s", err)) @@ -1289,7 +1289,7 @@ func TestSearchBasicInfo(t *testing.T) { } for _, c := range cases { t.Run(c.desc, func(t *testing.T) { - switch response, err := repo.SearchBasicInfo(context.Background(), c.page); { + switch response, err := repo.RetrieveAllBasicInfo(context.Background(), c.page); { case err == nil: if c.page.Order != "" && c.page.Dir != "" { c.response = response diff --git a/pkg/sdk/go/sdk.go b/pkg/sdk/go/sdk.go index 84902347e8..a91f7adf3e 100644 --- a/pkg/sdk/go/sdk.go +++ b/pkg/sdk/go/sdk.go @@ -495,6 +495,7 @@ type SDK interface { // things, _ := sdk.SearchThings(pm, "token") // fmt.Println(things) SearchThings(pm PageMetadata, token string) (ThingsPage, errors.SDKError) + // DeleteThing deletes a thing with the given id. // // example: diff --git a/things/mocks/repository.go b/things/mocks/repository.go index 0d5429af14..e3c2d98459 100644 --- a/things/mocks/repository.go +++ b/things/mocks/repository.go @@ -91,6 +91,34 @@ func (_m *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clients return r0, r1 } +// RetrieveAllBasicInfo provides a mock function with given fields: ctx, pm +func (_m *Repository) RetrieveAllBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { + ret := _m.Called(ctx, pm) + + if len(ret) == 0 { + panic("no return value specified for RetrieveAllBasicInfo") + } + + var r0 clients.ClientsPage + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, clients.Page) (clients.ClientsPage, error)); ok { + return rf(ctx, pm) + } + if rf, ok := ret.Get(0).(func(context.Context, clients.Page) clients.ClientsPage); ok { + r0 = rf(ctx, pm) + } else { + r0 = ret.Get(0).(clients.ClientsPage) + } + + if rf, ok := ret.Get(1).(func(context.Context, clients.Page) error); ok { + r1 = rf(ctx, pm) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // RetrieveAllByIDs provides a mock function with given fields: ctx, pm func (_m *Repository) RetrieveAllByIDs(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { ret := _m.Called(ctx, pm) diff --git a/users/mocks/repository.go b/users/mocks/repository.go index f543e44c2e..12baf1ac2b 100644 --- a/users/mocks/repository.go +++ b/users/mocks/repository.go @@ -109,6 +109,34 @@ func (_m *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clients return r0, r1 } +// RetrieveAllBasicInfo provides a mock function with given fields: ctx, pm +func (_m *Repository) RetrieveAllBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { + ret := _m.Called(ctx, pm) + + if len(ret) == 0 { + panic("no return value specified for RetrieveAllBasicInfo") + } + + var r0 clients.ClientsPage + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, clients.Page) (clients.ClientsPage, error)); ok { + return rf(ctx, pm) + } + if rf, ok := ret.Get(0).(func(context.Context, clients.Page) clients.ClientsPage); ok { + r0 = rf(ctx, pm) + } else { + r0 = ret.Get(0).(clients.ClientsPage) + } + + if rf, ok := ret.Get(1).(func(context.Context, clients.Page) error); ok { + r1 = rf(ctx, pm) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // RetrieveAllByIDs provides a mock function with given fields: ctx, pm func (_m *Repository) RetrieveAllByIDs(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { ret := _m.Called(ctx, pm) @@ -221,34 +249,6 @@ func (_m *Repository) Save(ctx context.Context, client clients.Client) (clients. return r0, r1 } -// SearchBasicInfo provides a mock function with given fields: ctx, pm -func (_m *Repository) SearchBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { - ret := _m.Called(ctx, pm) - - if len(ret) == 0 { - panic("no return value specified for SearchBasicInfo") - } - - var r0 clients.ClientsPage - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, clients.Page) (clients.ClientsPage, error)); ok { - return rf(ctx, pm) - } - if rf, ok := ret.Get(0).(func(context.Context, clients.Page) clients.ClientsPage); ok { - r0 = rf(ctx, pm) - } else { - r0 = ret.Get(0).(clients.ClientsPage) - } - - if rf, ok := ret.Get(1).(func(context.Context, clients.Page) error); ok { - r1 = rf(ctx, pm) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // Update provides a mock function with given fields: ctx, client func (_m *Repository) Update(ctx context.Context, client clients.Client) (clients.Client, error) { ret := _m.Called(ctx, client) From 2aca171142cc9f83f4da6d47e94991b6f4b7bda9 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Mon, 1 Jul 2024 13:04:10 +0300 Subject: [PATCH 13/31] fix failing ci Signed-off-by: nyagamunene --- pkg/sdk/go/sdk.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sdk/go/sdk.go b/pkg/sdk/go/sdk.go index a91f7adf3e..72e3e0003c 100644 --- a/pkg/sdk/go/sdk.go +++ b/pkg/sdk/go/sdk.go @@ -495,7 +495,7 @@ type SDK interface { // things, _ := sdk.SearchThings(pm, "token") // fmt.Println(things) SearchThings(pm PageMetadata, token string) (ThingsPage, errors.SDKError) - + // DeleteThing deletes a thing with the given id. // // example: From 14d316c3a0df428653595944cee65f3c248b6d42 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 13:03:54 +0300 Subject: [PATCH 14/31] Refactor list things Signed-off-by: nyagamunene --- pkg/clients/postgres/clients.go | 5 ++++- things/api/http/clients.go | 5 +++++ things/api/http/endpoints.go | 1 + things/api/http/requests.go | 1 + things/service.go | 2 +- 5 files changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index d3c635fd9a..2aafc8ba00 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -505,7 +505,7 @@ func PageQuery(pm clients.Page) (string, error) { query = append(query, "id ILIKE '%' || :id || '%'") } if pm.Tag != "" { - query = append(query, ":tag = ANY(c.tags)") + query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") } if pm.Status != clients.AllStatus { query = append(query, "c.status = :status") @@ -569,6 +569,9 @@ func ConstructThingSearchQuery(pm clients.Page) (string, string) { if pm.Tag != "" { query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") } + if len(pm.IDs) != 0 { + query = append(query, fmt.Sprintf("id IN ('%s')", strings.Join(pm.IDs, "','"))) + } if len(query) > 0 { emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) diff --git a/things/api/http/clients.go b/things/api/http/clients.go index 3fd2a93638..2c5040ad67 100644 --- a/things/api/http/clients.go +++ b/things/api/http/clients.go @@ -188,6 +188,10 @@ func decodeListClients(_ context.Context, r *http.Request) (interface{}, error) if err != nil { return nil, errors.Wrap(apiutil.ErrValidation, err) } + id, err := apiutil.ReadStringQuery(r, api.IDOrder, "") + if err != nil { + return nil, errors.Wrap(apiutil.ErrValidation, err) + } p, err := apiutil.ReadStringQuery(r, api.PermissionKey, api.DefPermission) if err != nil { return nil, errors.Wrap(apiutil.ErrValidation, err) @@ -212,6 +216,7 @@ func decodeListClients(_ context.Context, r *http.Request) (interface{}, error) permission: p, listPerms: lp, userID: chi.URLParam(r, "userID"), + id: id, } return req, nil } diff --git a/things/api/http/endpoints.go b/things/api/http/endpoints.go index 50f201fe26..162ab1b104 100644 --- a/things/api/http/endpoints.go +++ b/things/api/http/endpoints.go @@ -109,6 +109,7 @@ func listClientsEndpoint(svc things.Service) endpoint.Endpoint { Metadata: req.metadata, ListPerms: req.listPerms, Role: mgclients.AllRole, // retrieve all things since things don't have roles + Id: req.id, } page, err := svc.ListClients(ctx, req.token, req.userID, pm) if err != nil { diff --git a/things/api/http/requests.go b/things/api/http/requests.go index 14826c433e..d8019c53bd 100644 --- a/things/api/http/requests.go +++ b/things/api/http/requests.go @@ -98,6 +98,7 @@ type listClientsReq struct { userID string listPerms bool metadata mgclients.Metadata + id string } func (req listClientsReq) validate() error { diff --git a/things/service.go b/things/service.go index 5646761164..b4ece8c748 100644 --- a/things/service.go +++ b/things/service.go @@ -181,7 +181,7 @@ func (svc service) ListClients(ctx context.Context, token, reqUserID string, pm pm.IDs = ids - tp, err := svc.clients.RetrieveAllByIDs(ctx, pm) + tp, err := svc.clients.SearchBasicInfo(ctx, pm) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrViewEntity, err) } From 41d32b99a096a43d2d7674ce33f79bf03e112f7e Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 15:13:18 +0300 Subject: [PATCH 15/31] Refactor listclients in things service Signed-off-by: nyagamunene --- api/openapi/things.yml | 28 ---- cli/things.go | 43 ------- pkg/apiutil/errors.go | 6 - pkg/clients/clients.go | 4 +- pkg/clients/postgres/clients.go | 56 +------- pkg/clients/postgres/clients_test.go | 4 +- pkg/sdk/go/sdk.go | 12 -- pkg/sdk/go/things.go | 19 --- pkg/sdk/go/things_test.go | 130 ------------------- pkg/sdk/mocks/sdk.go | 30 ----- things/api/http/clients.go | 48 ------- things/api/http/endpoints.go | 37 ------ things/api/http/endpoints_test.go | 183 --------------------------- things/api/http/requests.go | 17 --- things/api/http/requests_test.go | 40 ------ things/mocks/repository.go | 28 ---- things/postgres/clients.go | 51 -------- things/service.go | 4 +- users/mocks/repository.go | 56 ++++---- 19 files changed, 39 insertions(+), 757 deletions(-) diff --git a/api/openapi/things.yml b/api/openapi/things.yml index 62b6df7a5b..8deae43c9b 100644 --- a/api/openapi/things.yml +++ b/api/openapi/things.yml @@ -401,34 +401,6 @@ paths: "500": $ref: "#/components/responses/ServiceError" - /things/search: - get: - operationId: searchThings - summary: Searches for things - description: | - Searches for things based on the provided query parameters i.e name, id and tags. - tags: - - Things - parameters: - - $ref: "#/components/parameters/ThingName" - - $ref: "#/components/parameters/ThingID" - - $ref: "#/components/parameters/Tags" - - $ref: "#/components/parameters/Offset" - - $ref: "#/components/parameters/Limit" - responses: - "200": - $ref: "#/components/responses/ThingPageRes" - "400": - description: Failed due to malformed query parameters. - "401": - description: Missing or invalid access token provided. - "403": - description: Failed to perform authorization over the entity. - "422": - description: Database can't process request. - "500": - $ref: "#/components/responses/ServiceError" - /channels/{chanID}/things: get: operationId: listThingsInaChannel diff --git a/cli/things.go b/cli/things.go index a38ab41d2c..f7c1f4b4bc 100644 --- a/cli/things.go +++ b/cli/things.go @@ -5,9 +5,6 @@ package cli import ( "encoding/json" - "fmt" - "net/url" - "strconv" mgclients "github.com/absmach/magistrala/pkg/clients" mgxsdk "github.com/absmach/magistrala/pkg/sdk/go" @@ -365,46 +362,6 @@ var cmdThings = []cobra.Command{ logJSON(ul) }, }, - { - Use: "search ", - Short: "Search things", - Long: "Search things by name, id or tags\n" + - "Usage:\n" + - "\tmagistrala-cli things search \n", - Run: func(cmd *cobra.Command, args []string) { - if len(args) != 2 { - logUsage(cmd.Use) - return - } - - values, err := url.ParseQuery(args[0]) - if err != nil { - logError(fmt.Errorf("Failed to parse query: %s", err)) - } - - pm := mgxsdk.PageMetadata{ - Name: values.Get("name"), - ID: values.Get("id"), - Tag: values.Get("tag"), - } - - if off, err := strconv.Atoi(values.Get("offset")); err == nil { - pm.Offset = uint64(off) - } - - if lim, err := strconv.Atoi(values.Get("limit")); err == nil { - pm.Limit = uint64(lim) - } - - things, err := sdk.SearchThings(pm, args[1]) - if err != nil { - logError(err) - return - } - - logJSON(things) - }, - }, } // NewThingsCmd returns things command. diff --git a/pkg/apiutil/errors.go b/pkg/apiutil/errors.go index f09b94fad7..833f8ecbd8 100644 --- a/pkg/apiutil/errors.go +++ b/pkg/apiutil/errors.go @@ -179,10 +179,4 @@ var ( // ErrInvalidTimeFormat indicates invalid time format i.e not unix time. ErrInvalidTimeFormat = errors.New("invalid time format use unix time") - - // ErrEmptySearchQuery indicates empty search query. - ErrEmptySearchQuery = errors.New("search query must not be empty") - - // ErrLenSearchQuery indicates search query length. - ErrLenSearchQuery = errors.New("search query must be at least 3 characters") ) diff --git a/pkg/clients/clients.go b/pkg/clients/clients.go index aa7202d9c8..c627a120d6 100644 --- a/pkg/clients/clients.go +++ b/pkg/clients/clients.go @@ -78,8 +78,8 @@ type Repository interface { // RetrieveAll retrieves all clients. RetrieveAll(ctx context.Context, pm Page) (ClientsPage, error) - // RetrieveAllBasicInfo list all clients only with basic information. - RetrieveAllBasicInfo(ctx context.Context, pm Page) (ClientsPage, error) + // SearchBasicInfo retrieves clients based on search criteria. + SearchBasicInfo(ctx context.Context, pm Page) (ClientsPage, error) // RetrieveAllByIDs retrieves for given client IDs . RetrieveAllByIDs(ctx context.Context, pm Page) (ClientsPage, error) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index 2aafc8ba00..07bebedda7 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -191,7 +191,7 @@ func (repo *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clien } func (repo *Repository) SearchBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { - sq, tq := ConstructSearchQuery(pm) + sq, tq := constructSearchQuery(pm) q := fmt.Sprintf(`SELECT c.id, c.name, c.created_at, c.updated_at FROM clients c %s LIMIT :limit OFFSET :offset;`, sq) @@ -334,24 +334,6 @@ func (repo *Repository) Delete(ctx context.Context, id string) error { return nil } -func (repo *Repository) CheckSuperAdmin(ctx context.Context, adminID string) error { - q := "SELECT 1 FROM clients WHERE id = $1 AND role = $2" - rows, err := repo.DB.QueryContext(ctx, q, adminID, clients.AdminRole) - if err != nil { - return postgres.HandleError(repoerr.ErrViewEntity, err) - } - defer rows.Close() - - if rows.Next() { - if err := rows.Err(); err != nil { - return postgres.HandleError(repoerr.ErrViewEntity, err) - } - return nil - } - - return repoerr.ErrNotFound -} - type DBClient struct { ID string `db:"id"` Name string `db:"name,omitempty"` @@ -523,11 +505,11 @@ func PageQuery(pm clients.Page) (string, error) { return emq, nil } -func ConstructSearchQuery(pm clients.Page) (string, string) { +func constructSearchQuery(pm clients.Page) (string, string) { var query []string var emq string var tq string - + fmt.Printf("PM: %+v\n", pm) if pm.Name != "" { query = append(query, "name ILIKE '%' || :name || '%'") } @@ -537,35 +519,6 @@ func ConstructSearchQuery(pm clients.Page) (string, string) { if pm.Id != "" { query = append(query, "id ILIKE '%' || :id || '%'") } - - if len(query) > 0 { - emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) - } - - tq = emq - - switch pm.Order { - case "name", "identity", "created_at", "updated_at": - emq = fmt.Sprintf("%s ORDER BY %s", emq, pm.Order) - if pm.Dir == api.AscDir || pm.Dir == api.DescDir { - emq = fmt.Sprintf("%s %s", emq, pm.Dir) - } - } - - return emq, tq -} - -func ConstructThingSearchQuery(pm clients.Page) (string, string) { - var query []string - var emq string - var tq string - - if pm.Name != "" { - query = append(query, "name ILIKE '%' || :name || '%'") - } - if pm.Id != "" { - query = append(query, "id ILIKE '%' || :id || '%'") - } if pm.Tag != "" { query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") } @@ -580,11 +533,12 @@ func ConstructThingSearchQuery(pm clients.Page) (string, string) { tq = emq switch pm.Order { - case "name", "tag", "created_at", "updated_at": + case "name", "identity", "created_at", "updated_at": emq = fmt.Sprintf("%s ORDER BY %s", emq, pm.Order) if pm.Dir == api.AscDir || pm.Dir == api.DescDir { emq = fmt.Sprintf("%s %s", emq, pm.Dir) } } + return emq, tq } diff --git a/pkg/clients/postgres/clients_test.go b/pkg/clients/postgres/clients_test.go index 178fcdf4ab..a5c0c01bbd 100644 --- a/pkg/clients/postgres/clients_test.go +++ b/pkg/clients/postgres/clients_test.go @@ -927,7 +927,7 @@ func TestRetrieveByIDs(t *testing.T) { } } -func TestRetrieveAllBasicInfo(t *testing.T) { +func TestSearchBasicInfo(t *testing.T) { t.Cleanup(func() { _, err := db.Exec("DELETE FROM clients") require.Nil(t, err, fmt.Sprintf("clean clients unexpected error: %s", err)) @@ -1289,7 +1289,7 @@ func TestRetrieveAllBasicInfo(t *testing.T) { } for _, c := range cases { t.Run(c.desc, func(t *testing.T) { - switch response, err := repo.RetrieveAllBasicInfo(context.Background(), c.page); { + switch response, err := repo.SearchBasicInfo(context.Background(), c.page); { case err == nil: if c.page.Order != "" && c.page.Dir != "" { c.response = response diff --git a/pkg/sdk/go/sdk.go b/pkg/sdk/go/sdk.go index 72e3e0003c..4b63c86273 100644 --- a/pkg/sdk/go/sdk.go +++ b/pkg/sdk/go/sdk.go @@ -484,18 +484,6 @@ type SDK interface { // fmt.Println(users) ListThingUsers(thingID string, pm PageMetadata, token string) (UsersPage, errors.SDKError) - // SearchThings returns page of things based on the search criteria. - // - // example: - // pm := sdk.PageMetadata{ - // Offset: 0, - // Limit: 10, - // Name: "My Thing", - // } - // things, _ := sdk.SearchThings(pm, "token") - // fmt.Println(things) - SearchThings(pm PageMetadata, token string) (ThingsPage, errors.SDKError) - // DeleteThing deletes a thing with the given id. // // example: diff --git a/pkg/sdk/go/things.go b/pkg/sdk/go/things.go index d6bc647dd4..16f629416d 100644 --- a/pkg/sdk/go/things.go +++ b/pkg/sdk/go/things.go @@ -295,25 +295,6 @@ func (sdk mgSDK) ListThingUsers(thingID string, pm PageMetadata, token string) ( return up, nil } -func (sdk mgSDK) SearchThings(pm PageMetadata, token string) (ThingsPage, errors.SDKError) { - url, err := sdk.withQueryParams(sdk.thingsURL, fmt.Sprintf("%s/search", thingsEndpoint), pm) - if err != nil { - return ThingsPage{}, errors.NewSDKError(err) - } - - _, body, sdkerr := sdk.processRequest(http.MethodGet, url, token, nil, nil, http.StatusOK) - if sdkerr != nil { - return ThingsPage{}, sdkerr - } - - var tp ThingsPage - if err := json.Unmarshal(body, &tp); err != nil { - return ThingsPage{}, errors.NewSDKError(err) - } - - return tp, nil -} - func (sdk mgSDK) DeleteThing(id, token string) errors.SDKError { url := fmt.Sprintf("%s/%s/%s", sdk.thingsURL, thingsEndpoint, id) _, _, sdkerr := sdk.processRequest(http.MethodDelete, url, token, nil, nil, http.StatusNoContent) diff --git a/pkg/sdk/go/things_test.go b/pkg/sdk/go/things_test.go index 65bbda1e85..2c56c988d9 100644 --- a/pkg/sdk/go/things_test.go +++ b/pkg/sdk/go/things_test.go @@ -607,136 +607,6 @@ func TestListThingsByChannel(t *testing.T) { } } -func TestSearchThings(t *testing.T) { - ts, cRepo, _, auth, _ := setupThings() - defer ts.Close() - - var ths []sdk.Thing - conf := sdk.Config{ - ThingsURL: ts.URL, - } - mgsdk := sdk.NewSDK(conf) - - for i := 10; i < 100; i++ { - th := sdk.Thing{ - ID: generateUUID(t), - Name: fmt.Sprintf("thing_%d", i), - Credentials: sdk.Credentials{ - Identity: fmt.Sprintf("identity_%d", i), - Secret: generateUUID(t), - }, - Metadata: sdk.Metadata{"name": fmt.Sprintf("thing_%d", i)}, - Status: mgclients.EnabledStatus.String(), - } - if i == 50 { - th.Status = mgclients.DisabledStatus.String() - th.Tags = []string{"tag1", "tag2"} - } - ths = append(ths, th) - } - - cases := []struct { - desc string - token string - page sdk.PageMetadata - response []sdk.Thing - authorizeRes *magistrala.AuthorizeRes - searchRes mgclients.ClientsPage - identifyRes *magistrala.IdentityRes - err errors.SDKError - identifyErr error - authorizeErr error - }{ - { - desc: "search things with authorized token", - token: validToken, - identifyRes: &magistrala.IdentityRes{Id: validID, DomainId: testsutil.GenerateUUID(t)}, - authorizeRes: &magistrala.AuthorizeRes{Authorized: true}, - page: sdk.PageMetadata{ - Offset: offset, - Limit: limit, - Name: "thing_10", - }, - response: []sdk.Thing{ths[10]}, - searchRes: mgclients.ClientsPage{ - Page: mgclients.Page{ - Total: 1, - Offset: offset, - Limit: limit, - }, - Clients: convertThings(ths[10]), - }, - err: nil, - }, - { - desc: "search things with invalid token", - token: invalidToken, - page: sdk.PageMetadata{ - Offset: offset, - Limit: limit, - Name: "thing_10", - }, - err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized), - response: nil, - identifyErr: svcerr.ErrAuthentication, - }, - { - desc: "search things with empty token", - token: "", - page: sdk.PageMetadata{ - Offset: offset, - Limit: limit, - Name: "thing_10", - }, - err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrBearerToken), http.StatusUnauthorized), - response: nil, - }, - { - desc: "search things with empty query", - token: validToken, - page: sdk.PageMetadata{ - Offset: offset, - Limit: limit, - Name: "", - }, - err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrEmptySearchQuery), http.StatusBadRequest), - }, - { - desc: "search for things with invalid length of query", - token: validToken, - page: sdk.PageMetadata{ - Offset: offset, - Limit: limit, - Name: "a", - }, - err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrLenSearchQuery, apiutil.ErrValidation), http.StatusBadRequest), - }, - { - desc: "search for things with no domain id", - token: validToken, - page: sdk.PageMetadata{ - Offset: offset, - Limit: limit, - Name: "thing_10", - }, - identifyRes: &magistrala.IdentityRes{Id: validID}, - authorizeRes: &magistrala.AuthorizeRes{Authorized: false}, - err: errors.NewSDKErrorWithStatus(svcerr.ErrDomainAuthorization, http.StatusForbidden), - }, - } - for _, tc := range cases { - repoCall := auth.On("Identify", mock.Anything, &magistrala.IdentityReq{Token: tc.token}).Return(tc.identifyRes, tc.identifyErr) - repoCall1 := auth.On("Authorize", mock.Anything, mock.Anything).Return(tc.authorizeRes, tc.authorizeErr) - repoCall2 := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(tc.searchRes, tc.err) - page, err := mgsdk.SearchThings(tc.page, tc.token) - assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err)) - assert.Equal(t, tc.response, page.Things, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, page)) - repoCall.Unset() - repoCall1.Unset() - repoCall2.Unset() - } -} - func TestThing(t *testing.T) { ts, cRepo, _, auth, _ := setupThings() defer ts.Close() diff --git a/pkg/sdk/mocks/sdk.go b/pkg/sdk/mocks/sdk.go index 208d184932..5fbc8033cf 100644 --- a/pkg/sdk/mocks/sdk.go +++ b/pkg/sdk/mocks/sdk.go @@ -2050,36 +2050,6 @@ func (_m *SDK) RevokeCert(thingID string, token string) (time.Time, errors.SDKEr return r0, r1 } -// SearchThings provides a mock function with given fields: pm, token -func (_m *SDK) SearchThings(pm sdk.PageMetadata, token string) (sdk.ThingsPage, errors.SDKError) { - ret := _m.Called(pm, token) - - if len(ret) == 0 { - panic("no return value specified for SearchThings") - } - - var r0 sdk.ThingsPage - var r1 errors.SDKError - if rf, ok := ret.Get(0).(func(sdk.PageMetadata, string) (sdk.ThingsPage, errors.SDKError)); ok { - return rf(pm, token) - } - if rf, ok := ret.Get(0).(func(sdk.PageMetadata, string) sdk.ThingsPage); ok { - r0 = rf(pm, token) - } else { - r0 = ret.Get(0).(sdk.ThingsPage) - } - - if rf, ok := ret.Get(1).(func(sdk.PageMetadata, string) errors.SDKError); ok { - r1 = rf(pm, token) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(errors.SDKError) - } - } - - return r0, r1 -} - // SendInvitation provides a mock function with given fields: invitation, token func (_m *SDK) SendInvitation(invitation sdk.Invitation, token string) error { ret := _m.Called(invitation, token) diff --git a/things/api/http/clients.go b/things/api/http/clients.go index 2c5040ad67..24652c4680 100644 --- a/things/api/http/clients.go +++ b/things/api/http/clients.go @@ -60,13 +60,6 @@ func clientsHandler(svc things.Service, r *chi.Mux, logger *slog.Logger) http.Ha opts..., ), "view_thing_permissions").ServeHTTP) - r.Get("/search", otelhttp.NewHandler(kithttp.NewServer( - searchThingsEndpoint(svc), - decodeSearchThings, - api.EncodeResponse, - opts..., - ), "search_things").ServeHTTP) - r.Patch("/{thingID}", otelhttp.NewHandler(kithttp.NewServer( updateClientEndpoint(svc), decodeUpdateClient, @@ -221,47 +214,6 @@ func decodeListClients(_ context.Context, r *http.Request) (interface{}, error) return req, nil } -func decodeSearchThings(_ context.Context, r *http.Request) (interface{}, error) { - o, err := apiutil.ReadNumQuery[uint64](r, api.OffsetKey, api.DefOffset) - if err != nil { - return nil, errors.Wrap(apiutil.ErrValidation, err) - } - l, err := apiutil.ReadNumQuery[uint64](r, api.LimitKey, api.DefLimit) - if err != nil { - return nil, errors.Wrap(apiutil.ErrValidation, err) - } - n, err := apiutil.ReadStringQuery(r, api.NameKey, "") - if err != nil { - return nil, errors.Wrap(apiutil.ErrValidation, err) - } - t, err := apiutil.ReadStringQuery(r, api.TagKey, "") - if err != nil { - return nil, errors.Wrap(apiutil.ErrValidation, err) - } - - id, err := apiutil.ReadStringQuery(r, api.IDOrder, "") - if err != nil { - return nil, errors.Wrap(apiutil.ErrValidation, err) - } - - req := searchThingsReq{ - apiutil.ExtractBearerToken(r), - mgclients.Page{Offset: o, Limit: l, Name: n, Tag: t, Id: id}, - } - - for _, field := range []string{req.Name, req.Id, req.Tag} { - if field != "" && len(field) < 3 { - req = searchThingsReq{ - token: apiutil.ExtractBearerToken(r), - Page: mgclients.Page{}, - } - return req, errors.Wrap(apiutil.ErrLenSearchQuery, apiutil.ErrValidation) - } - } - - return req, nil -} - func decodeUpdateClient(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), api.ContentType) { return nil, errors.Wrap(apiutil.ErrValidation, apiutil.ErrUnsupportedContentType) diff --git a/things/api/http/endpoints.go b/things/api/http/endpoints.go index 162ab1b104..ff70689a3a 100644 --- a/things/api/http/endpoints.go +++ b/things/api/http/endpoints.go @@ -148,43 +148,6 @@ func listMembersEndpoint(svc things.Service) endpoint.Endpoint { } } -func searchThingsEndpoint(svc things.Service) endpoint.Endpoint { - return func(ctx context.Context, request interface{}) (interface{}, error) { - req := request.(searchThingsReq) - if err := req.validate(); err != nil { - return nil, errors.Wrap(apiutil.ErrValidation, err) - } - - pm := mgclients.Page{ - Offset: req.Offset, - Limit: req.Limit, - Name: req.Name, - Id: req.Id, - Tag: req.Tag, - Order: req.Order, - Dir: req.Dir, - } - page, err := svc.SearchThings(ctx, req.token, pm) - if err != nil { - return nil, err - } - - res := clientsPageRes{ - pageRes: pageRes{ - Total: page.Total, - Offset: page.Offset, - Limit: page.Limit, - }, - Clients: []viewClientRes{}, - } - for _, client := range page.Clients { - res.Clients = append(res.Clients, viewClientRes{Client: client}) - } - - return res, nil - } -} - func updateClientEndpoint(svc things.Service) endpoint.Endpoint { return func(ctx context.Context, request interface{}) (interface{}, error) { req := request.(updateClientReq) diff --git a/things/api/http/endpoints_test.go b/things/api/http/endpoints_test.go index 89bf846663..381cdcdfd7 100644 --- a/things/api/http/endpoints_test.go +++ b/things/api/http/endpoints_test.go @@ -364,189 +364,6 @@ func TestCreateThings(t *testing.T) { } } -func TestListThings(t *testing.T) { - ts, svc, _ := newThingsServer() - defer ts.Close() - - cases := []struct { - desc string - query string - token string - listThingsResponse mgclients.ClientsPage - status int - err error - }{ - { - desc: "list things with valid token", - token: validToken, - status: http.StatusOK, - query: "name=clientname", - listThingsResponse: mgclients.ClientsPage{ - Page: mgclients.Page{ - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - err: nil, - }, - { - desc: "list things with empty token", - token: "", - query: "name=clientname", - status: http.StatusUnauthorized, - err: apiutil.ErrBearerToken, - }, - { - desc: "list things with invalid token", - token: inValidToken, - query: "name=clientname", - status: http.StatusUnauthorized, - err: svcerr.ErrAuthentication, - }, - { - desc: "list things with offset", - token: validToken, - listThingsResponse: mgclients.ClientsPage{ - Page: mgclients.Page{ - Offset: 1, - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - query: "name=clientname&offset=1", - status: http.StatusOK, - err: nil, - }, - { - desc: "list things with invalid offset", - token: validToken, - query: "name=clientname&offset=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, - { - desc: "list things with limit", - token: validToken, - listThingsResponse: mgclients.ClientsPage{ - Page: mgclients.Page{ - Limit: 1, - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - query: "name=clientname&limit=1", - status: http.StatusOK, - err: nil, - }, - { - desc: "list things with invalid limit", - token: validToken, - query: "name=clientname&limit=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, - { - desc: "search things with empty query", - token: validToken, - query: "", - status: http.StatusBadRequest, - err: apiutil.ErrEmptySearchQuery, - }, - { - desc: "search users with invalid length of query", - token: validToken, - query: "name=a", - status: http.StatusBadRequest, - err: apiutil.ErrLenSearchQuery, - }, - { - desc: "list things with limit greater than max", - token: validToken, - query: fmt.Sprintf("limit=%d", api.MaxLimitSize+1), - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, - { - desc: "list things with invalid name", - token: validToken, - query: "name=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, - { - desc: "list things with duplicate name", - token: validToken, - query: "name=1&name=2", - status: http.StatusBadRequest, - err: apiutil.ErrInvalidQueryParams, - }, - { - desc: "list things with invalid status", - token: validToken, - query: "status=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, - { - desc: "list things with tags", - token: validToken, - listThingsResponse: mgclients.ClientsPage{ - Page: mgclients.Page{ - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - query: "tag=tag1,tag2", - status: http.StatusOK, - err: nil, - }, - { - desc: "list things with duplicate tags", - token: validToken, - query: "tag=tag1&tag=tag2", - status: http.StatusBadRequest, - err: apiutil.ErrInvalidQueryParams, - }, - { - desc: "list things with id", - token: validToken, - query: fmt.Sprintf("id=%s", client.ID), - listThingsResponse: mgclients.ClientsPage{ - Page: mgclients.Page{ - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - status: http.StatusOK, - err: nil, - }, - } - - for _, tc := range cases { - req := testRequest{ - client: ts.Client(), - method: http.MethodGet, - url: ts.URL + "/things?" + tc.query, - contentType: contentType, - token: tc.token, - } - - svcCall := svc.On("ListClients", mock.Anything, tc.token, "", mock.Anything).Return(tc.listThingsResponse, tc.err) - res, err := req.make() - assert.Nil(t, err, fmt.Sprintf("%s: unexpected error %s", tc.desc, err)) - - var bodyRes respBody - err = json.NewDecoder(res.Body).Decode(&bodyRes) - assert.Nil(t, err, fmt.Sprintf("%s: unexpected error while decoding response body: %s", tc.desc, err)) - if bodyRes.Err != "" || bodyRes.Message != "" { - err = errors.Wrap(errors.New(bodyRes.Err), errors.New(bodyRes.Message)) - } - assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) - assert.Equal(t, tc.status, res.StatusCode, fmt.Sprintf("%s: expected status code %d got %d", tc.desc, tc.status, res.StatusCode)) - svcCall.Unset() - } -} - func TestSearchThings(t *testing.T) { ts, svc, _ := newThingsServer() defer ts.Close() diff --git a/things/api/http/requests.go b/things/api/http/requests.go index d8019c53bd..41746442cb 100644 --- a/things/api/http/requests.go +++ b/things/api/http/requests.go @@ -135,23 +135,6 @@ func (req listMembersReq) validate() error { return nil } -type searchThingsReq struct { - token string - mgclients.Page -} - -func (req searchThingsReq) validate() error { - if req.token == "" { - return apiutil.ErrBearerToken - } - - if req.Name == "" && req.Tag == "" && req.Id == "" { - return apiutil.ErrEmptySearchQuery - } - - return nil -} - type updateClientReq struct { token string id string diff --git a/things/api/http/requests_test.go b/things/api/http/requests_test.go index f3bc22b88e..74fccf2c8e 100644 --- a/things/api/http/requests_test.go +++ b/things/api/http/requests_test.go @@ -320,46 +320,6 @@ func TestListMembersReqValidate(t *testing.T) { } } -func TestSearchThingsReqValidate(t *testing.T) { - cases := []struct { - desc string - req searchThingsReq - err error - }{ - { - desc: "valid request", - req: searchThingsReq{ - token: valid, - Page: mgclients.Page{ - Name: name, - }, - }, - err: nil, - }, - { - desc: "empty token", - req: searchThingsReq{ - token: "", - Page: mgclients.Page{ - Name: name, - }, - }, - err: apiutil.ErrBearerToken, - }, - { - desc: "empty query", - req: searchThingsReq{ - token: valid, - }, - err: apiutil.ErrEmptySearchQuery, - }, - } - for _, c := range cases { - err := c.req.validate() - assert.Equal(t, c.err, err) - } -} - func TestUpdateClientReqValidate(t *testing.T) { cases := []struct { desc string diff --git a/things/mocks/repository.go b/things/mocks/repository.go index e3c2d98459..0d5429af14 100644 --- a/things/mocks/repository.go +++ b/things/mocks/repository.go @@ -91,34 +91,6 @@ func (_m *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clients return r0, r1 } -// RetrieveAllBasicInfo provides a mock function with given fields: ctx, pm -func (_m *Repository) RetrieveAllBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { - ret := _m.Called(ctx, pm) - - if len(ret) == 0 { - panic("no return value specified for RetrieveAllBasicInfo") - } - - var r0 clients.ClientsPage - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, clients.Page) (clients.ClientsPage, error)); ok { - return rf(ctx, pm) - } - if rf, ok := ret.Get(0).(func(context.Context, clients.Page) clients.ClientsPage); ok { - r0 = rf(ctx, pm) - } else { - r0 = ret.Get(0).(clients.ClientsPage) - } - - if rf, ok := ret.Get(1).(func(context.Context, clients.Page) error); ok { - r1 = rf(ctx, pm) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // RetrieveAllByIDs provides a mock function with given fields: ctx, pm func (_m *Repository) RetrieveAllByIDs(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { ret := _m.Called(ctx, pm) diff --git a/things/postgres/clients.go b/things/postgres/clients.go index c4bdc1a93b..e07e88373a 100644 --- a/things/postgres/clients.go +++ b/things/postgres/clients.go @@ -33,9 +33,6 @@ type Repository interface { // RetrieveBySecret retrieves a client based on the secret (key). RetrieveBySecret(ctx context.Context, key string) (mgclients.Client, error) - - // SearchBasicInfo retrieves basic information about clients. - SearchBasicInfo(ctx context.Context, pm mgclients.Page) (mgclients.ClientsPage, error) } // NewRepository instantiates a PostgreSQL @@ -124,51 +121,3 @@ func (repo clientRepo) RetrieveBySecret(ctx context.Context, key string) (mgclie return mgclients.Client{}, repoerr.ErrNotFound } - -func (repo clientRepo) SearchBasicInfo(ctx context.Context, pm mgclients.Page) (mgclients.ClientsPage, error) { - sq, tq := pgclients.ConstructThingSearchQuery(pm) - - q := fmt.Sprintf(`SELECT c.id, c.name, c.created_at, c.updated_at FROM clients c %s LIMIT :limit OFFSET :offset;`, sq) - dbPage, err := pgclients.ToDBClientsPage(pm) - if err != nil { - return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) - } - fmt.Println("query: ", q) - rows, err := repo.DB.NamedQueryContext(ctx, q, dbPage) - if err != nil { - return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) - } - defer rows.Close() - - var items []mgclients.Client - for rows.Next() { - dbc := pgclients.DBClient{} - if err := rows.StructScan(&dbc); err != nil { - return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrViewEntity, err) - } - - c, err := pgclients.ToClient(dbc) - if err != nil { - return mgclients.ClientsPage{}, err - } - - items = append(items, c) - } - - cq := fmt.Sprintf(`SELECT COUNT(*) FROM clients c %s;`, tq) - total, err := postgres.Total(ctx, repo.DB, cq, dbPage) - if err != nil { - return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrViewEntity, err) - } - - page := mgclients.ClientsPage{ - Clients: items, - Page: mgclients.Page{ - Total: total, - Offset: pm.Offset, - Limit: pm.Limit, - }, - } - - return page, nil -} diff --git a/things/service.go b/things/service.go index b4ece8c748..04eb0f960c 100644 --- a/things/service.go +++ b/things/service.go @@ -181,7 +181,7 @@ func (svc service) ListClients(ctx context.Context, token, reqUserID string, pm pm.IDs = ids - tp, err := svc.clients.SearchBasicInfo(ctx, pm) + tp, err := svc.SearchThings(ctx, token, pm) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrViewEntity, err) } @@ -547,7 +547,7 @@ func (svc service) SearchThings(ctx context.Context, token string, pm mgclients. things := mgclients.ClientsPage{} for _, val := range cp.Clients { - if _, err := svc.authorize(ctx, res.GetDomainId(), auth.UserType, auth.UsersKind, res.GetId(), auth.AdminPermission, auth.ThingType, val.ID); err == nil { + if _, err := svc.authorize(ctx, res.GetDomainId(), auth.UserType, auth.UsersKind, res.GetId(), auth.ViewPermission, auth.ThingType, val.ID); err == nil { things.Clients = append(things.Clients, val) things.Page.Total += 1 } diff --git a/users/mocks/repository.go b/users/mocks/repository.go index 12baf1ac2b..f543e44c2e 100644 --- a/users/mocks/repository.go +++ b/users/mocks/repository.go @@ -109,34 +109,6 @@ func (_m *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clients return r0, r1 } -// RetrieveAllBasicInfo provides a mock function with given fields: ctx, pm -func (_m *Repository) RetrieveAllBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { - ret := _m.Called(ctx, pm) - - if len(ret) == 0 { - panic("no return value specified for RetrieveAllBasicInfo") - } - - var r0 clients.ClientsPage - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, clients.Page) (clients.ClientsPage, error)); ok { - return rf(ctx, pm) - } - if rf, ok := ret.Get(0).(func(context.Context, clients.Page) clients.ClientsPage); ok { - r0 = rf(ctx, pm) - } else { - r0 = ret.Get(0).(clients.ClientsPage) - } - - if rf, ok := ret.Get(1).(func(context.Context, clients.Page) error); ok { - r1 = rf(ctx, pm) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // RetrieveAllByIDs provides a mock function with given fields: ctx, pm func (_m *Repository) RetrieveAllByIDs(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { ret := _m.Called(ctx, pm) @@ -249,6 +221,34 @@ func (_m *Repository) Save(ctx context.Context, client clients.Client) (clients. return r0, r1 } +// SearchBasicInfo provides a mock function with given fields: ctx, pm +func (_m *Repository) SearchBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { + ret := _m.Called(ctx, pm) + + if len(ret) == 0 { + panic("no return value specified for SearchBasicInfo") + } + + var r0 clients.ClientsPage + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, clients.Page) (clients.ClientsPage, error)); ok { + return rf(ctx, pm) + } + if rf, ok := ret.Get(0).(func(context.Context, clients.Page) clients.ClientsPage); ok { + r0 = rf(ctx, pm) + } else { + r0 = ret.Get(0).(clients.ClientsPage) + } + + if rf, ok := ret.Get(1).(func(context.Context, clients.Page) error); ok { + r1 = rf(ctx, pm) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // Update provides a mock function with given fields: ctx, client func (_m *Repository) Update(ctx context.Context, client clients.Client) (clients.Client, error) { ret := _m.Called(ctx, client) From 9d2a6e2c0954accf9e028f305e48666ecd275964 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 15:32:05 +0300 Subject: [PATCH 16/31] Fix failing ci tests Signed-off-by: nyagamunene --- pkg/sdk/go/things_test.go | 2 +- things/service_test.go | 40 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/pkg/sdk/go/things_test.go b/pkg/sdk/go/things_test.go index 2c56c988d9..49016bfa73 100644 --- a/pkg/sdk/go/things_test.go +++ b/pkg/sdk/go/things_test.go @@ -471,7 +471,7 @@ func TestListThings(t *testing.T) { repoCall1 = auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: false}, svcerr.ErrAuthorization) repoCall2 = auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{}, svcerr.ErrAuthorization) } - repoCall3 := cRepo.On("RetrieveAllByIDs", mock.Anything, mock.Anything).Return(mgclients.ClientsPage{Page: convertClientPage(pm), Clients: convertThings(tc.response...)}, tc.err) + repoCall3 := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(mgclients.ClientsPage{Page: convertClientPage(pm), Clients: convertThings(tc.response...)}, tc.err) page, err := mgsdk.Things(pm, validToken) assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err)) assert.Equal(t, tc.response, page.Things, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, page)) diff --git a/things/service_test.go b/things/service_test.go index fb99e4b745..52e144a1e4 100644 --- a/things/service_test.go +++ b/things/service_test.go @@ -28,7 +28,7 @@ import ( var ( secret = "strongsecret" validCMetadata = mgclients.Metadata{"role": "client"} - ID = testsutil.GenerateUUID(&testing.T{}) + ID = "6e5e10b3-d4df-4758-b426-4929d55ad740" client = mgclients.Client{ ID: ID, Name: "clientname", @@ -1629,6 +1629,44 @@ func TestSearchThings(t *testing.T) { identifyErr: svcerr.ErrAuthentication, err: svcerr.ErrAuthentication, }, + { + desc: "search clients with id", + token: validToken, + page: mgclients.Page{Offset: 0, Id: "6e5e10b3-d4df-4758-b426-4929d55ad740", Limit: 100}, + searchResponse: mgclients.ClientsPage{ + Page: mgclients.Page{Total: 1, Offset: 0, Limit: 100}, + Clients: []mgclients.Client{client}, + }, + identifyResponse: &magistrala.IdentityRes{Id: client.ID, DomainId: testsutil.GenerateUUID(t)}, + authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, + identifyErr: nil, + err: nil, + }, + { + desc: "search clients with tag", + token: validToken, + page: mgclients.Page{Offset: 0, Tag: "tag1", Limit: 100}, + searchResponse: mgclients.ClientsPage{ + Page: mgclients.Page{Total: 1, Offset: 0, Limit: 100}, + Clients: []mgclients.Client{client}, + }, + identifyResponse: &magistrala.IdentityRes{Id: client.ID, DomainId: testsutil.GenerateUUID(t)}, + authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, + identifyErr: nil, + err: nil, + }, + { + desc: "search clients with random name", + token: validToken, + page: mgclients.Page{Offset: 0, Name: "randomname", Limit: 100}, + searchResponse: mgclients.ClientsPage{ + Page: mgclients.Page{Total: 0, Offset: 0, Limit: 100}, + }, + identifyResponse: &magistrala.IdentityRes{Id: client.ID, DomainId: testsutil.GenerateUUID(t)}, + authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, + identifyErr: nil, + err: nil, + }, } for _, tc := range cases { From 0e0d26b35225cadb723ad2f57b9c2a5848127866 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 15:40:10 +0300 Subject: [PATCH 17/31] Fix failing ci tests Signed-off-by: nyagamunene --- pkg/sdk/go/things_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sdk/go/things_test.go b/pkg/sdk/go/things_test.go index 49016bfa73..42b2eb148e 100644 --- a/pkg/sdk/go/things_test.go +++ b/pkg/sdk/go/things_test.go @@ -1042,7 +1042,7 @@ func TestEnableThing(t *testing.T) { repoCall1 = auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: false}, svcerr.ErrAuthorization) } repoCall2 := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{Policies: toIDs(tc.response.Things)}, nil) - repoCall3 := cRepo.On("RetrieveAllByIDs", mock.Anything, mock.Anything).Return(convertThingsPage(tc.response), nil) + repoCall3 := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(convertThingsPage(tc.response), nil) clientsPage, err := mgsdk.Things(pm, validToken) assert.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) size := uint64(len(clientsPage.Things)) From 374c732cfccbe73a83ef0292d85c3a136722d7c8 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 15:46:46 +0300 Subject: [PATCH 18/31] Fix failing ci tests Signed-off-by: nyagamunene --- pkg/sdk/go/things_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sdk/go/things_test.go b/pkg/sdk/go/things_test.go index 42b2eb148e..d5b41bfbc3 100644 --- a/pkg/sdk/go/things_test.go +++ b/pkg/sdk/go/things_test.go @@ -1179,7 +1179,7 @@ func TestDisableThing(t *testing.T) { repoCall1 = auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: false}, svcerr.ErrAuthorization) } repoCall2 := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{Policies: toIDs(tc.response.Things)}, nil) - repoCall3 := cRepo.On("RetrieveAllByIDs", mock.Anything, mock.Anything).Return(convertThingsPage(tc.response), nil) + repoCall3 := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(convertThingsPage(tc.response), nil) page, err := mgsdk.Things(pm, validToken) assert.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) size := uint64(len(page.Things)) From 0c842a935b934fd30d8651289a7093f1ceb434cd Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 18:05:33 +0300 Subject: [PATCH 19/31] Fix Things tests Signed-off-by: nyagamunene --- .vscode/launch.json | 7 +++++++ things/service.go | 4 ---- things/service_test.go | 26 ++++++++++++++++++++------ 3 files changed, 27 insertions(+), 10 deletions(-) create mode 100644 .vscode/launch.json diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 0000000000..5c7247b40a --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,7 @@ +{ + // Use IntelliSense to learn about possible attributes. + // Hover to view descriptions of existing attributes. + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 + "version": "0.2.0", + "configurations": [] +} \ No newline at end of file diff --git a/things/service.go b/things/service.go index 04eb0f960c..3eea7b5601 100644 --- a/things/service.go +++ b/things/service.go @@ -536,10 +536,6 @@ func (svc service) SearchThings(ctx context.Context, token string, pm mgclients. return mgclients.ClientsPage{}, err } - if _, err := svc.authorize(ctx, "", auth.UserType, auth.UsersKind, res.GetId(), auth.MembershipPermission, auth.DomainType, res.GetDomainId()); err != nil { - return mgclients.ClientsPage{}, err - } - cp, err := svc.clients.SearchBasicInfo(ctx, pm) if err != nil { return mgclients.ClientsPage{}, err diff --git a/things/service_test.go b/things/service_test.go index 52e144a1e4..db51602904 100644 --- a/things/service_test.go +++ b/things/service_test.go @@ -428,6 +428,7 @@ func TestListClients(t *testing.T) { identifyResponse *magistrala.IdentityRes authorizeResponse *magistrala.AuthorizeRes authorizeResponse1 *magistrala.AuthorizeRes + authorizeResponse2 *magistrala.AuthorizeRes listObjectsResponse *magistrala.ListObjectsRes listObjectsResponse1 *magistrala.ListObjectsRes retrieveAllResponse mgclients.ClientsPage @@ -438,6 +439,7 @@ func TestListClients(t *testing.T) { identifyErr error authorizeErr error authorizeErr1 error + authorizeErr2 error listObjectsErr error retrieveAllErr error listPermissionsErr error @@ -455,6 +457,7 @@ func TestListClients(t *testing.T) { }, identifyResponse: &magistrala.IdentityRes{Id: nonAdminID, UserId: nonAdminID, DomainId: domainID}, authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, + authorizeResponse2: &magistrala.AuthorizeRes{Authorized: true}, listObjectsResponse: &magistrala.ListObjectsRes{}, retrieveAllResponse: mgclients.ClientsPage{ Page: mgclients.Page{ @@ -531,8 +534,9 @@ func TestListClients(t *testing.T) { Limit: 100, ListPerms: true, }, - identifyResponse: &magistrala.IdentityRes{Id: nonAdminID, UserId: nonAdminID, DomainId: domainID}, - authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, + identifyResponse: &magistrala.IdentityRes{Id: nonAdminID, UserId: nonAdminID, DomainId: domainID}, + authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, + authorizeResponse2: &magistrala.AuthorizeRes{Authorized: true}, retrieveAllResponse: mgclients.ClientsPage{ Page: mgclients.Page{ Total: 2, @@ -618,8 +622,17 @@ func TestListClients(t *testing.T) { ObjectType: "domain", Object: tc.identifyResponse.DomainId, }).Return(tc.authorizeResponse1, tc.authorizeErr1) + authorizeCall3 := auth.On("Authorize", context.Background(), &magistrala.AuthorizeReq{ + Domain: domainID, + SubjectType: authsvc.UserType, + SubjectKind: authsvc.UsersKind, + Subject: tc.identifyResponse.UserId, + Permission: "view", + ObjectType: "thing", + Object: client.ID, + }).Return(tc.authorizeResponse2, tc.authorizeErr2) listAllObjectsCall := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(tc.listObjectsResponse, tc.listObjectsErr) - retrieveAllCall := cRepo.On("RetrieveAllByIDs", mock.Anything, mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) + retrieveAllCall := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) listPermissionsCall := auth.On("ListPermissions", mock.Anything, mock.Anything).Return(tc.listPermissionsResponse, tc.listPermissionsErr) page, err := svc.ListClients(context.Background(), tc.token, tc.id, tc.page) @@ -631,6 +644,7 @@ func TestListClients(t *testing.T) { listAllObjectsCall.Unset() retrieveAllCall.Unset() listPermissionsCall.Unset() + authorizeCall3.Unset() } cases2 := []struct { @@ -799,7 +813,7 @@ func TestListClients(t *testing.T) { Permission: "", ObjectType: authsvc.ThingType, }).Return(tc.listObjectsResponse1, tc.listObjectsErr1) - retrieveAllCall := cRepo.On("RetrieveAllByIDs", mock.Anything, mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) + retrieveAllCall := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) listPermissionsCall := auth.On("ListPermissions", mock.Anything, mock.Anything).Return(tc.listPermissionsResponse, tc.listPermissionsErr) page, err := svc.ListClients(context.Background(), tc.token, tc.id, tc.page) @@ -1193,7 +1207,7 @@ func TestEnableClient(t *testing.T) { repoCall := auth.On("Identify", mock.Anything, &magistrala.IdentityReq{Token: validToken}).Return(&magistrala.IdentityRes{Id: validID, DomainId: testsutil.GenerateUUID(t)}, nil) repoCall1 := auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: true}, nil) repoCall2 := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{Policies: getIDs(tc.response.Clients)}, nil) - repoCall3 := cRepo.On("RetrieveAllByIDs", context.Background(), mock.Anything).Return(tc.response, nil) + repoCall3 := cRepo.On("SearchBasicInfo", context.Background(), mock.Anything).Return(tc.response, nil) page, err := svc.ListClients(context.Background(), validToken, "", pm) require.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) size := uint64(len(page.Clients)) @@ -1363,7 +1377,7 @@ func TestDisableClient(t *testing.T) { repoCall := auth.On("Identify", mock.Anything, &magistrala.IdentityReq{Token: validToken}).Return(&magistrala.IdentityRes{Id: validID, DomainId: testsutil.GenerateUUID(t)}, nil) repoCall1 := auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: true}, nil) repoCall2 := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{Policies: getIDs(tc.response.Clients)}, nil) - repoCall3 := cRepo.On("RetrieveAllByIDs", context.Background(), mock.Anything).Return(tc.response, nil) + repoCall3 := cRepo.On("SearchBasicInfo", context.Background(), mock.Anything).Return(tc.response, nil) page, err := svc.ListClients(context.Background(), validToken, "", pm) require.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) size := uint64(len(page.Clients)) From 86a48cb147a58fa4cd810d8545172300d408b30c Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 18:07:25 +0300 Subject: [PATCH 20/31] Remove debug folder Signed-off-by: nyagamunene --- .vscode/launch.json | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 .vscode/launch.json diff --git a/.vscode/launch.json b/.vscode/launch.json deleted file mode 100644 index 5c7247b40a..0000000000 --- a/.vscode/launch.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - // Use IntelliSense to learn about possible attributes. - // Hover to view descriptions of existing attributes. - // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 - "version": "0.2.0", - "configurations": [] -} \ No newline at end of file From 4f010ef402ef2559a10f2de2a36f8f0357479317 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 18:17:18 +0300 Subject: [PATCH 21/31] Remove thing search endpoint test Signed-off-by: nyagamunene --- things/api/http/endpoints_test.go | 107 ------------------------------ 1 file changed, 107 deletions(-) diff --git a/things/api/http/endpoints_test.go b/things/api/http/endpoints_test.go index 381cdcdfd7..0f1cc3732d 100644 --- a/things/api/http/endpoints_test.go +++ b/things/api/http/endpoints_test.go @@ -364,113 +364,6 @@ func TestCreateThings(t *testing.T) { } } -func TestSearchThings(t *testing.T) { - ts, svc, _ := newThingsServer() - defer ts.Close() - - cases := []struct { - desc string - query string - token string - searchThingsRes mgclients.ClientsPage - status int - err error - }{ - { - desc: "search things with valid token", - token: validToken, - status: http.StatusOK, - query: "name=clientname", - searchThingsRes: mgclients.ClientsPage{ - Page: mgclients.Page{ - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - err: nil, - }, - { - desc: "search things with empty token", - token: "", - query: "name=clientname", - status: http.StatusUnauthorized, - err: apiutil.ErrBearerToken, - }, - { - desc: "search things with invalid token", - token: inValidToken, - query: "name=clientname", - status: http.StatusUnauthorized, - err: svcerr.ErrAuthentication, - }, - { - desc: "search things with offset", - token: validToken, - searchThingsRes: mgclients.ClientsPage{ - Page: mgclients.Page{ - Offset: 1, - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - query: "name=client&offset=1", - status: http.StatusOK, - err: nil, - }, - { - desc: "search things with invalid offset", - token: validToken, - query: "name=clientname&offset=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, - { - desc: "search things with limit", - token: validToken, - searchThingsRes: mgclients.ClientsPage{ - Page: mgclients.Page{ - Limit: 1, - Total: 1, - }, - Clients: []mgclients.Client{client}, - }, - query: "name=clientname&limit=1", - status: http.StatusOK, - err: nil, - }, - { - desc: "search things with invalid limit", - token: validToken, - query: "name=clientname&limit=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, - } - for _, tc := range cases { - req := testRequest{ - client: ts.Client(), - method: http.MethodGet, - url: fmt.Sprintf("%s/things/search?%s", ts.URL, tc.query), - contentType: contentType, - token: tc.token, - } - - svcCall := svc.On("SearchThings", mock.Anything, tc.token, mock.Anything).Return(tc.searchThingsRes, tc.err) - res, err := req.make() - assert.Nil(t, err, fmt.Sprintf("%s: unexpected error %s", tc.desc, err)) - - var bodyRes respBody - err = json.NewDecoder(res.Body).Decode(&bodyRes) - assert.Nil(t, err, fmt.Sprintf("%s: unexpected error while decoding response body: %s", tc.desc, err)) - if bodyRes.Err != "" || bodyRes.Message != "" { - err = errors.Wrap(errors.New(bodyRes.Err), errors.New(bodyRes.Message)) - } - assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) - assert.Equal(t, tc.status, res.StatusCode, fmt.Sprintf("%s: expected status code %d got %d", tc.desc, tc.status, res.StatusCode)) - svcCall.Unset() - } -} - func TestViewThing(t *testing.T) { ts, svc, _ := newThingsServer() defer ts.Close() From 45f13ed6760baf01b663b677b9734b99faebb8db Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 22:59:20 +0300 Subject: [PATCH 22/31] Refactor: Search Thing service Signed-off-by: nyagamunene --- pkg/clients/postgres/clients.go | 53 ++++++++------------ things/api/logging.go | 20 -------- things/api/metrics.go | 8 --- things/events/events.go | 28 ----------- things/events/streams.go | 15 ------ things/mocks/service.go | 28 ----------- things/service.go | 26 +--------- things/service_test.go | 89 --------------------------------- things/things.go | 3 -- things/tracing/tracing.go | 6 --- 10 files changed, 23 insertions(+), 253 deletions(-) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index 07bebedda7..42b273f277 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -469,36 +469,19 @@ func PageQuery(pm clients.Page) (string, error) { if err != nil { return "", errors.Wrap(errors.ErrMalformedEntity, err) } - var query []string - var emq string + + query := buildQueryConditions(pm) if mq != "" { query = append(query, mq) } - if len(pm.IDs) != 0 { - query = append(query, fmt.Sprintf("id IN ('%s')", strings.Join(pm.IDs, "','"))) - } - if pm.Name != "" { - query = append(query, "name ILIKE '%' || :name || '%'") - } - if pm.Identity != "" { - query = append(query, "identity ILIKE '%' || :identity || '%'") - } - if pm.Id != "" { - query = append(query, "id ILIKE '%' || :id || '%'") - } - if pm.Tag != "" { - query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") - } + if pm.Status != clients.AllStatus { query = append(query, "c.status = :status") } if pm.Domain != "" { query = append(query, "c.domain_id = :domain_id") } - - if pm.Role != clients.AllRole { - query = append(query, "c.role = :role") - } + var emq string if len(query) > 0 { emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) } @@ -506,10 +489,18 @@ func PageQuery(pm clients.Page) (string, error) { } func constructSearchQuery(pm clients.Page) (string, string) { - var query []string + query := buildQueryConditions(pm) var emq string - var tq string - fmt.Printf("PM: %+v\n", pm) + if len(query) > 0 { + emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) + } + tq := emq + emq = applyOrdering(emq, pm) + return emq, tq +} + +func buildQueryConditions(pm clients.Page) []string { + var query []string if pm.Name != "" { query = append(query, "name ILIKE '%' || :name || '%'") } @@ -522,16 +513,17 @@ func constructSearchQuery(pm clients.Page) (string, string) { if pm.Tag != "" { query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") } - if len(pm.IDs) != 0 { + if len(query) == 0 && len(pm.IDs) != 0 { query = append(query, fmt.Sprintf("id IN ('%s')", strings.Join(pm.IDs, "','"))) } - - if len(query) > 0 { - emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) + if pm.Role != clients.AllRole { + query = append(query, "c.role = :role") } - tq = emq + return query +} +func applyOrdering(emq string, pm clients.Page) string { switch pm.Order { case "name", "identity", "created_at", "updated_at": emq = fmt.Sprintf("%s ORDER BY %s", emq, pm.Order) @@ -539,6 +531,5 @@ func constructSearchQuery(pm clients.Page) (string, string) { emq = fmt.Sprintf("%s %s", emq, pm.Dir) } } - - return emq, tq + return emq } diff --git a/things/api/logging.go b/things/api/logging.go index 7f8b279b33..164b0ecdfb 100644 --- a/things/api/logging.go +++ b/things/api/logging.go @@ -214,26 +214,6 @@ func (lm *loggingMiddleware) ListClientsByGroup(ctx context.Context, token, chan return lm.svc.ListClientsByGroup(ctx, token, channelID, cp) } -func (lm *loggingMiddleware) SearchThings(ctx context.Context, token string, pm mgclients.Page) (cp mgclients.ClientsPage, err error) { - defer func(begin time.Time) { - args := []any{ - slog.String("duration", time.Since(begin).String()), - slog.Group("page", - slog.Uint64("limit", pm.Limit), - slog.Uint64("offset", pm.Offset), - slog.Uint64("total", cp.Total), - ), - } - if err != nil { - args = append(args, slog.Any("error", err)) - lm.logger.Warn("Search things failed to complete successfully", args...) - return - } - lm.logger.Info("Search things completed successfully", args...) - }(time.Now()) - return lm.svc.SearchThings(ctx, token, pm) -} - func (lm *loggingMiddleware) Identify(ctx context.Context, key string) (id string, err error) { defer func(begin time.Time) { args := []any{ diff --git a/things/api/metrics.go b/things/api/metrics.go index a58ccda472..24976d39a8 100644 --- a/things/api/metrics.go +++ b/things/api/metrics.go @@ -110,14 +110,6 @@ func (ms *metricsMiddleware) ListClientsByGroup(ctx context.Context, token, grou return ms.svc.ListClientsByGroup(ctx, token, groupID, pm) } -func (ms *metricsMiddleware) SearchThings(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { - defer func(begin time.Time) { - ms.counter.With("method", "search_things").Add(1) - ms.latency.With("method", "search_things").Observe(time.Since(begin).Seconds()) - }(time.Now()) - return ms.svc.SearchThings(ctx, token, pm) -} - func (ms *metricsMiddleware) Identify(ctx context.Context, key string) (string, error) { defer func(begin time.Time) { ms.counter.With("method", "identify_thing").Add(1) diff --git a/things/events/events.go b/things/events/events.go index 52e1d17ade..56b68b6ddc 100644 --- a/things/events/events.go +++ b/things/events/events.go @@ -4,8 +4,6 @@ package events import ( - "fmt" - "strings" "time" mgclients "github.com/absmach/magistrala/pkg/clients" @@ -22,7 +20,6 @@ const ( clientViewPerms = clientPrefix + "view_perms" clientList = clientPrefix + "list" clientListByGroup = clientPrefix + "list_by_channel" - clientSearch = clientPrefix + "search" clientIdentify = clientPrefix + "identify" clientAuthorize = clientPrefix + "authorize" ) @@ -278,31 +275,6 @@ func (lcge listClientByGroupEvent) Encode() (map[string]interface{}, error) { return val, nil } -type searchThingsEvent struct { - mgclients.Page -} - -func (ste searchThingsEvent) Encode() (map[string]interface{}, error) { - val := map[string]interface{}{ - "operation": clientSearch, - "total": ste.Total, - "offset": ste.Offset, - "limit": ste.Limit, - } - if ste.Name != "" { - val["name"] = ste.Name - } - if ste.Tag != "" { - val["tag"] = ste.Tag - } - if ste.IDs != nil { - ids := fmt.Sprintf("[%s]", strings.Join(ste.IDs, ",")) - val["ids"] = ids - } - - return val, nil -} - type identifyClientEvent struct { thingID string } diff --git a/things/events/streams.go b/things/events/streams.go index 825c05ee2b..ca6a392fb2 100644 --- a/things/events/streams.go +++ b/things/events/streams.go @@ -156,21 +156,6 @@ func (es *eventStore) ListClientsByGroup(ctx context.Context, token, chID string return mp, nil } -func (es *eventStore) SearchThings(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { - cp, err := es.svc.SearchThings(ctx, token, pm) - if err != nil { - return cp, err - } - event := searchThingsEvent{ - pm, - } - if err := es.Publish(ctx, event); err != nil { - return cp, err - } - - return cp, nil -} - func (es *eventStore) EnableClient(ctx context.Context, token, id string) (mgclients.Client, error) { cli, err := es.svc.EnableClient(ctx, token, id) if err != nil { diff --git a/things/mocks/service.go b/things/mocks/service.go index bfaee35f4b..efae25fdc6 100644 --- a/things/mocks/service.go +++ b/things/mocks/service.go @@ -242,34 +242,6 @@ func (_m *Service) ListClientsByGroup(ctx context.Context, token string, groupID return r0, r1 } -// SearchThings provides a mock function with given fields: ctx, token, pm -func (_m *Service) SearchThings(ctx context.Context, token string, pm clients.Page) (clients.ClientsPage, error) { - ret := _m.Called(ctx, token, pm) - - if len(ret) == 0 { - panic("no return value specified for SearchThings") - } - - var r0 clients.ClientsPage - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, clients.Page) (clients.ClientsPage, error)); ok { - return rf(ctx, token, pm) - } - if rf, ok := ret.Get(0).(func(context.Context, string, clients.Page) clients.ClientsPage); ok { - r0 = rf(ctx, token, pm) - } else { - r0 = ret.Get(0).(clients.ClientsPage) - } - - if rf, ok := ret.Get(1).(func(context.Context, string, clients.Page) error); ok { - r1 = rf(ctx, token, pm) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // Share provides a mock function with given fields: ctx, token, id, relation, userids func (_m *Service) Share(ctx context.Context, token string, id string, relation string, userids ...string) error { _va := make([]interface{}, len(userids)) diff --git a/things/service.go b/things/service.go index 3eea7b5601..8cb248b997 100644 --- a/things/service.go +++ b/things/service.go @@ -181,7 +181,7 @@ func (svc service) ListClients(ctx context.Context, token, reqUserID string, pm pm.IDs = ids - tp, err := svc.SearchThings(ctx, token, pm) + tp, err := svc.clients.SearchBasicInfo(ctx, pm) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrViewEntity, err) } @@ -530,30 +530,6 @@ func (svc service) ListClientsByGroup(ctx context.Context, token, groupID string }, nil } -func (svc service) SearchThings(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { - res, err := svc.identify(ctx, token) - if err != nil { - return mgclients.ClientsPage{}, err - } - - cp, err := svc.clients.SearchBasicInfo(ctx, pm) - if err != nil { - return mgclients.ClientsPage{}, err - } - - things := mgclients.ClientsPage{} - for _, val := range cp.Clients { - if _, err := svc.authorize(ctx, res.GetDomainId(), auth.UserType, auth.UsersKind, res.GetId(), auth.ViewPermission, auth.ThingType, val.ID); err == nil { - things.Clients = append(things.Clients, val) - things.Page.Total += 1 - } - } - things.Page.Offset = cp.Offset - things.Page.Limit = cp.Limit - - return things, nil -} - func (svc service) Identify(ctx context.Context, key string) (string, error) { id, err := svc.clientCache.ID(ctx, key) if err == nil { diff --git a/things/service_test.go b/things/service_test.go index db51602904..ee90b97cba 100644 --- a/things/service_test.go +++ b/things/service_test.go @@ -1607,95 +1607,6 @@ func TestListMembers(t *testing.T) { } } -func TestSearchThings(t *testing.T) { - svc, cRepo, auth, _ := newService() - - cases := []struct { - desc string - token string - page mgclients.Page - identifyResponse *magistrala.IdentityRes - authorizeResponse *magistrala.AuthorizeRes - searchResponse mgclients.ClientsPage - responseErr error - authorizeErr error - identifyErr error - err error - }{ - { - desc: "search clients with valid token", - token: validToken, - page: mgclients.Page{Offset: 0, Limit: 100, Name: "clientname"}, - searchResponse: mgclients.ClientsPage{ - Page: mgclients.Page{Total: 1, Offset: 0, Limit: 100}, - Clients: []mgclients.Client{client}, - }, - identifyResponse: &magistrala.IdentityRes{Id: client.ID, DomainId: testsutil.GenerateUUID(t)}, - authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, - identifyErr: nil, - err: nil, - }, - { - desc: "search clients with invalid token", - token: inValidToken, - page: mgclients.Page{Offset: 0, Limit: 100, Name: "clientname"}, - searchResponse: mgclients.ClientsPage{}, - identifyErr: svcerr.ErrAuthentication, - err: svcerr.ErrAuthentication, - }, - { - desc: "search clients with id", - token: validToken, - page: mgclients.Page{Offset: 0, Id: "6e5e10b3-d4df-4758-b426-4929d55ad740", Limit: 100}, - searchResponse: mgclients.ClientsPage{ - Page: mgclients.Page{Total: 1, Offset: 0, Limit: 100}, - Clients: []mgclients.Client{client}, - }, - identifyResponse: &magistrala.IdentityRes{Id: client.ID, DomainId: testsutil.GenerateUUID(t)}, - authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, - identifyErr: nil, - err: nil, - }, - { - desc: "search clients with tag", - token: validToken, - page: mgclients.Page{Offset: 0, Tag: "tag1", Limit: 100}, - searchResponse: mgclients.ClientsPage{ - Page: mgclients.Page{Total: 1, Offset: 0, Limit: 100}, - Clients: []mgclients.Client{client}, - }, - identifyResponse: &magistrala.IdentityRes{Id: client.ID, DomainId: testsutil.GenerateUUID(t)}, - authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, - identifyErr: nil, - err: nil, - }, - { - desc: "search clients with random name", - token: validToken, - page: mgclients.Page{Offset: 0, Name: "randomname", Limit: 100}, - searchResponse: mgclients.ClientsPage{ - Page: mgclients.Page{Total: 0, Offset: 0, Limit: 100}, - }, - identifyResponse: &magistrala.IdentityRes{Id: client.ID, DomainId: testsutil.GenerateUUID(t)}, - authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, - identifyErr: nil, - err: nil, - }, - } - - for _, tc := range cases { - authCall := auth.On("Identify", context.Background(), &magistrala.IdentityReq{Token: tc.token}).Return(tc.identifyResponse, tc.identifyErr) - repoCall := auth.On("Authorize", mock.Anything, mock.Anything).Return(tc.authorizeResponse, tc.authorizeErr) - repoCall1 := cRepo.On("SearchBasicInfo", context.Background(), tc.page).Return(tc.searchResponse, tc.responseErr) - page, err := svc.SearchThings(context.Background(), tc.token, tc.page) - assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) - assert.Equal(t, tc.searchResponse, page, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.searchResponse, page)) - authCall.Unset() - repoCall.Unset() - repoCall1.Unset() - } -} - func TestDeleteClient(t *testing.T) { svc, cRepo, auth, cache := newService() diff --git a/things/things.go b/things/things.go index e8e85b0fc5..270d59e131 100644 --- a/things/things.go +++ b/things/things.go @@ -33,9 +33,6 @@ type Service interface { // the provided key. ListClientsByGroup(ctx context.Context, token, groupID string, pm clients.Page) (clients.MembersPage, error) - // SearchThings searches for things with provided filters for a valid auth token. - SearchThings(ctx context.Context, token string, pm clients.Page) (clients.ClientsPage, error) - // UpdateClient updates the client's name and metadata. UpdateClient(ctx context.Context, token string, client clients.Client) (clients.Client, error) diff --git a/things/tracing/tracing.go b/things/tracing/tracing.go index c3f7959bfe..f097163279 100644 --- a/things/tracing/tracing.go +++ b/things/tracing/tracing.go @@ -54,12 +54,6 @@ func (tm *tracingMiddleware) ListClients(ctx context.Context, token, reqUserID s return tm.svc.ListClients(ctx, token, reqUserID, pm) } -func (tm *tracingMiddleware) SearchThings(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { - ctx, span := tm.tracer.Start(ctx, "svc_search_things") - defer span.End() - return tm.svc.SearchThings(ctx, token, pm) -} - // UpdateClient traces the "UpdateClient" operation of the wrapped policies.Service. func (tm *tracingMiddleware) UpdateClient(ctx context.Context, token string, cli mgclients.Client) (mgclients.Client, error) { ctx, span := tm.tracer.Start(ctx, "svc_update_client_name_and_metadata", trace.WithAttributes(attribute.String("id", cli.ID))) From 42e760d61e422fabe5ca3ce37341b7ebb42162a0 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 23:03:43 +0300 Subject: [PATCH 23/31] Rename SearchBasicInfo to SearchClients Signed-off-by: nyagamunene --- pkg/clients/clients.go | 4 ++-- pkg/clients/postgres/clients.go | 2 +- pkg/clients/postgres/clients_test.go | 4 ++-- pkg/sdk/go/things_test.go | 6 +++--- things/mocks/repository.go | 6 +++--- things/service.go | 2 +- things/service_test.go | 8 ++++---- users/mocks/repository.go | 6 +++--- users/service.go | 4 ++-- users/service_test.go | 4 ++-- 10 files changed, 23 insertions(+), 23 deletions(-) diff --git a/pkg/clients/clients.go b/pkg/clients/clients.go index c627a120d6..1278675533 100644 --- a/pkg/clients/clients.go +++ b/pkg/clients/clients.go @@ -78,8 +78,8 @@ type Repository interface { // RetrieveAll retrieves all clients. RetrieveAll(ctx context.Context, pm Page) (ClientsPage, error) - // SearchBasicInfo retrieves clients based on search criteria. - SearchBasicInfo(ctx context.Context, pm Page) (ClientsPage, error) + // SearchClients retrieves clients based on search criteria. + SearchClients(ctx context.Context, pm Page) (ClientsPage, error) // RetrieveAllByIDs retrieves for given client IDs . RetrieveAllByIDs(ctx context.Context, pm Page) (ClientsPage, error) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index 42b273f277..1a19fb4077 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -190,7 +190,7 @@ func (repo *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clien return page, nil } -func (repo *Repository) SearchBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { +func (repo *Repository) SearchClients(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { sq, tq := constructSearchQuery(pm) q := fmt.Sprintf(`SELECT c.id, c.name, c.created_at, c.updated_at FROM clients c %s LIMIT :limit OFFSET :offset;`, sq) diff --git a/pkg/clients/postgres/clients_test.go b/pkg/clients/postgres/clients_test.go index a5c0c01bbd..e381b48eae 100644 --- a/pkg/clients/postgres/clients_test.go +++ b/pkg/clients/postgres/clients_test.go @@ -927,7 +927,7 @@ func TestRetrieveByIDs(t *testing.T) { } } -func TestSearchBasicInfo(t *testing.T) { +func TestSearchClients(t *testing.T) { t.Cleanup(func() { _, err := db.Exec("DELETE FROM clients") require.Nil(t, err, fmt.Sprintf("clean clients unexpected error: %s", err)) @@ -1289,7 +1289,7 @@ func TestSearchBasicInfo(t *testing.T) { } for _, c := range cases { t.Run(c.desc, func(t *testing.T) { - switch response, err := repo.SearchBasicInfo(context.Background(), c.page); { + switch response, err := repo.SearchClients(context.Background(), c.page); { case err == nil: if c.page.Order != "" && c.page.Dir != "" { c.response = response diff --git a/pkg/sdk/go/things_test.go b/pkg/sdk/go/things_test.go index d5b41bfbc3..cd51f3b8bb 100644 --- a/pkg/sdk/go/things_test.go +++ b/pkg/sdk/go/things_test.go @@ -471,7 +471,7 @@ func TestListThings(t *testing.T) { repoCall1 = auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: false}, svcerr.ErrAuthorization) repoCall2 = auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{}, svcerr.ErrAuthorization) } - repoCall3 := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(mgclients.ClientsPage{Page: convertClientPage(pm), Clients: convertThings(tc.response...)}, tc.err) + repoCall3 := cRepo.On("SearchClients", mock.Anything, mock.Anything).Return(mgclients.ClientsPage{Page: convertClientPage(pm), Clients: convertThings(tc.response...)}, tc.err) page, err := mgsdk.Things(pm, validToken) assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err)) assert.Equal(t, tc.response, page.Things, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, page)) @@ -1042,7 +1042,7 @@ func TestEnableThing(t *testing.T) { repoCall1 = auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: false}, svcerr.ErrAuthorization) } repoCall2 := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{Policies: toIDs(tc.response.Things)}, nil) - repoCall3 := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(convertThingsPage(tc.response), nil) + repoCall3 := cRepo.On("SearchClients", mock.Anything, mock.Anything).Return(convertThingsPage(tc.response), nil) clientsPage, err := mgsdk.Things(pm, validToken) assert.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) size := uint64(len(clientsPage.Things)) @@ -1179,7 +1179,7 @@ func TestDisableThing(t *testing.T) { repoCall1 = auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: false}, svcerr.ErrAuthorization) } repoCall2 := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{Policies: toIDs(tc.response.Things)}, nil) - repoCall3 := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(convertThingsPage(tc.response), nil) + repoCall3 := cRepo.On("SearchClients", mock.Anything, mock.Anything).Return(convertThingsPage(tc.response), nil) page, err := mgsdk.Things(pm, validToken) assert.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) size := uint64(len(page.Things)) diff --git a/things/mocks/repository.go b/things/mocks/repository.go index 0d5429af14..ab254b064b 100644 --- a/things/mocks/repository.go +++ b/things/mocks/repository.go @@ -240,12 +240,12 @@ func (_m *Repository) Save(ctx context.Context, client ...clients.Client) ([]cli return r0, r1 } -// SearchBasicInfo provides a mock function with given fields: ctx, pm -func (_m *Repository) SearchBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { +// SearchClients provides a mock function with given fields: ctx, pm +func (_m *Repository) SearchClients(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { ret := _m.Called(ctx, pm) if len(ret) == 0 { - panic("no return value specified for SearchBasicInfo") + panic("no return value specified for SearchClients") } var r0 clients.ClientsPage diff --git a/things/service.go b/things/service.go index 8cb248b997..8ece3b59b5 100644 --- a/things/service.go +++ b/things/service.go @@ -181,7 +181,7 @@ func (svc service) ListClients(ctx context.Context, token, reqUserID string, pm pm.IDs = ids - tp, err := svc.clients.SearchBasicInfo(ctx, pm) + tp, err := svc.clients.SearchClients(ctx, pm) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrViewEntity, err) } diff --git a/things/service_test.go b/things/service_test.go index ee90b97cba..07ae189b32 100644 --- a/things/service_test.go +++ b/things/service_test.go @@ -632,7 +632,7 @@ func TestListClients(t *testing.T) { Object: client.ID, }).Return(tc.authorizeResponse2, tc.authorizeErr2) listAllObjectsCall := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(tc.listObjectsResponse, tc.listObjectsErr) - retrieveAllCall := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) + retrieveAllCall := cRepo.On("SearchClients", mock.Anything, mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) listPermissionsCall := auth.On("ListPermissions", mock.Anything, mock.Anything).Return(tc.listPermissionsResponse, tc.listPermissionsErr) page, err := svc.ListClients(context.Background(), tc.token, tc.id, tc.page) @@ -813,7 +813,7 @@ func TestListClients(t *testing.T) { Permission: "", ObjectType: authsvc.ThingType, }).Return(tc.listObjectsResponse1, tc.listObjectsErr1) - retrieveAllCall := cRepo.On("SearchBasicInfo", mock.Anything, mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) + retrieveAllCall := cRepo.On("SearchClients", mock.Anything, mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) listPermissionsCall := auth.On("ListPermissions", mock.Anything, mock.Anything).Return(tc.listPermissionsResponse, tc.listPermissionsErr) page, err := svc.ListClients(context.Background(), tc.token, tc.id, tc.page) @@ -1207,7 +1207,7 @@ func TestEnableClient(t *testing.T) { repoCall := auth.On("Identify", mock.Anything, &magistrala.IdentityReq{Token: validToken}).Return(&magistrala.IdentityRes{Id: validID, DomainId: testsutil.GenerateUUID(t)}, nil) repoCall1 := auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: true}, nil) repoCall2 := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{Policies: getIDs(tc.response.Clients)}, nil) - repoCall3 := cRepo.On("SearchBasicInfo", context.Background(), mock.Anything).Return(tc.response, nil) + repoCall3 := cRepo.On("SearchClients", context.Background(), mock.Anything).Return(tc.response, nil) page, err := svc.ListClients(context.Background(), validToken, "", pm) require.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) size := uint64(len(page.Clients)) @@ -1377,7 +1377,7 @@ func TestDisableClient(t *testing.T) { repoCall := auth.On("Identify", mock.Anything, &magistrala.IdentityReq{Token: validToken}).Return(&magistrala.IdentityRes{Id: validID, DomainId: testsutil.GenerateUUID(t)}, nil) repoCall1 := auth.On("Authorize", mock.Anything, mock.Anything).Return(&magistrala.AuthorizeRes{Authorized: true}, nil) repoCall2 := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(&magistrala.ListObjectsRes{Policies: getIDs(tc.response.Clients)}, nil) - repoCall3 := cRepo.On("SearchBasicInfo", context.Background(), mock.Anything).Return(tc.response, nil) + repoCall3 := cRepo.On("SearchClients", context.Background(), mock.Anything).Return(tc.response, nil) page, err := svc.ListClients(context.Background(), validToken, "", pm) require.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) size := uint64(len(page.Clients)) diff --git a/users/mocks/repository.go b/users/mocks/repository.go index f543e44c2e..8470e6e9a8 100644 --- a/users/mocks/repository.go +++ b/users/mocks/repository.go @@ -221,12 +221,12 @@ func (_m *Repository) Save(ctx context.Context, client clients.Client) (clients. return r0, r1 } -// SearchBasicInfo provides a mock function with given fields: ctx, pm -func (_m *Repository) SearchBasicInfo(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { +// SearchClients provides a mock function with given fields: ctx, pm +func (_m *Repository) SearchClients(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { ret := _m.Called(ctx, pm) if len(ret) == 0 { - panic("no return value specified for SearchBasicInfo") + panic("no return value specified for SearchClients") } var r0 clients.ClientsPage diff --git a/users/service.go b/users/service.go index 6195a6b2c8..5922b39de4 100644 --- a/users/service.go +++ b/users/service.go @@ -196,7 +196,7 @@ func (svc service) ListClients(ctx context.Context, token string, pm mgclients.P Identity: pm.Identity, Role: mgclients.UserRole, } - pg, err := svc.clients.SearchBasicInfo(ctx, p) + pg, err := svc.clients.SearchClients(ctx, p) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrViewEntity, err) } @@ -559,7 +559,7 @@ func (svc service) SearchUsers(ctx context.Context, token string, pm mgclients.P return mgclients.ClientsPage{}, err } - cp, err := svc.clients.SearchBasicInfo(ctx, pm) + cp, err := svc.clients.SearchClients(ctx, pm) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrSearch, err) } diff --git a/users/service_test.go b/users/service_test.go index abb18a7799..224c080111 100644 --- a/users/service_test.go +++ b/users/service_test.go @@ -597,7 +597,7 @@ func TestListClients(t *testing.T) { authCall1 := auth.On("Authorize", context.Background(), mock.Anything).Return(tc.authorizeResponse, tc.authorizeErr) repoCall := cRepo.On("CheckSuperAdmin", context.Background(), mock.Anything).Return(tc.superAdminErr) repoCall1 := cRepo.On("RetrieveAll", context.Background(), mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) - repoCall2 := cRepo.On("SearchBasicInfo", context.Background(), mock.Anything).Return(tc.response, tc.err) + repoCall2 := cRepo.On("SearchClients", context.Background(), mock.Anything).Return(tc.response, tc.err) page, err := svc.ListClients(context.Background(), tc.token, tc.page) assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) assert.Equal(t, tc.response, page, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, page)) @@ -2233,7 +2233,7 @@ func TestSearchUsers(t *testing.T) { for _, tc := range cases { authCall := auth.On("Identify", context.Background(), &magistrala.IdentityReq{Token: tc.token}).Return(tc.identifyResp, tc.identifyErr) - repoCall := cRepo.On("SearchBasicInfo", context.Background(), tc.page).Return(tc.response, tc.responseErr) + repoCall := cRepo.On("SearchClients", context.Background(), tc.page).Return(tc.response, tc.responseErr) page, err := svc.SearchUsers(context.Background(), tc.token, tc.page) assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) assert.Equal(t, tc.response, page, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, page)) From d9fb0def23513cf5fb36fe313fb3a280a4587cb5 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Tue, 2 Jul 2024 23:06:22 +0300 Subject: [PATCH 24/31] fix TestListClientsTest Signed-off-by: nyagamunene --- things/service_test.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/things/service_test.go b/things/service_test.go index 07ae189b32..721c147e0a 100644 --- a/things/service_test.go +++ b/things/service_test.go @@ -622,15 +622,6 @@ func TestListClients(t *testing.T) { ObjectType: "domain", Object: tc.identifyResponse.DomainId, }).Return(tc.authorizeResponse1, tc.authorizeErr1) - authorizeCall3 := auth.On("Authorize", context.Background(), &magistrala.AuthorizeReq{ - Domain: domainID, - SubjectType: authsvc.UserType, - SubjectKind: authsvc.UsersKind, - Subject: tc.identifyResponse.UserId, - Permission: "view", - ObjectType: "thing", - Object: client.ID, - }).Return(tc.authorizeResponse2, tc.authorizeErr2) listAllObjectsCall := auth.On("ListAllObjects", mock.Anything, mock.Anything).Return(tc.listObjectsResponse, tc.listObjectsErr) retrieveAllCall := cRepo.On("SearchClients", mock.Anything, mock.Anything).Return(tc.retrieveAllResponse, tc.retrieveAllErr) listPermissionsCall := auth.On("ListPermissions", mock.Anything, mock.Anything).Return(tc.listPermissionsResponse, tc.listPermissionsErr) @@ -644,7 +635,6 @@ func TestListClients(t *testing.T) { listAllObjectsCall.Unset() retrieveAllCall.Unset() listPermissionsCall.Unset() - authorizeCall3.Unset() } cases2 := []struct { From 1ac2597e97c3315bb4543c97dc476e0c74554266 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Wed, 3 Jul 2024 12:14:20 +0300 Subject: [PATCH 25/31] Undo delete of TestListThings Signed-off-by: nyagamunene --- things/api/http/endpoints_test.go | 274 ++++++++++++++++++++++++++++++ 1 file changed, 274 insertions(+) diff --git a/things/api/http/endpoints_test.go b/things/api/http/endpoints_test.go index 0f1cc3732d..a0346ac436 100644 --- a/things/api/http/endpoints_test.go +++ b/things/api/http/endpoints_test.go @@ -364,6 +364,280 @@ func TestCreateThings(t *testing.T) { } } +func TestListThings(t *testing.T) { + ts, svc, _ := newThingsServer() + defer ts.Close() + + cases := []struct { + desc string + query string + token string + listThingsResponse mgclients.ClientsPage + status int + err error + }{ + { + desc: "list things with valid token", + token: validToken, + status: http.StatusOK, + listThingsResponse: mgclients.ClientsPage{ + Page: mgclients.Page{ + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + err: nil, + }, + { + desc: "list things with empty token", + token: "", + status: http.StatusUnauthorized, + err: apiutil.ErrBearerToken, + }, + { + desc: "list things with invalid token", + token: inValidToken, + status: http.StatusUnauthorized, + err: svcerr.ErrAuthentication, + }, + { + desc: "list things with offset", + token: validToken, + listThingsResponse: mgclients.ClientsPage{ + Page: mgclients.Page{ + Offset: 1, + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + query: "offset=1", + status: http.StatusOK, + err: nil, + }, + { + desc: "list things with invalid offset", + token: validToken, + query: "offset=invalid", + status: http.StatusBadRequest, + err: apiutil.ErrValidation, + }, + { + desc: "list things with limit", + token: validToken, + listThingsResponse: mgclients.ClientsPage{ + Page: mgclients.Page{ + Limit: 1, + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + query: "limit=1", + status: http.StatusOK, + err: nil, + }, + { + desc: "list things with invalid limit", + token: validToken, + query: "limit=invalid", + status: http.StatusBadRequest, + err: apiutil.ErrValidation, + }, + { + desc: "list things with limit greater than max", + token: validToken, + query: fmt.Sprintf("limit=%d", api.MaxLimitSize+1), + status: http.StatusBadRequest, + err: apiutil.ErrValidation, + }, + { + desc: "list things with name", + token: validToken, + listThingsResponse: mgclients.ClientsPage{ + Page: mgclients.Page{ + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + query: "name=clientname", + status: http.StatusOK, + err: nil, + }, + { + desc: "list things with invalid name", + token: validToken, + query: "name=invalid", + status: http.StatusBadRequest, + err: apiutil.ErrValidation, + }, + { + desc: "list things with duplicate name", + token: validToken, + query: "name=1&name=2", + status: http.StatusBadRequest, + err: apiutil.ErrInvalidQueryParams, + }, + { + desc: "list things with status", + token: validToken, + listThingsResponse: mgclients.ClientsPage{ + Page: mgclients.Page{ + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + query: "status=enabled", + status: http.StatusOK, + err: nil, + }, + { + desc: "list things with invalid status", + token: validToken, + query: "status=invalid", + status: http.StatusBadRequest, + err: apiutil.ErrValidation, + }, + { + desc: "list things with duplicate status", + token: validToken, + query: "status=enabled&status=disabled", + status: http.StatusBadRequest, + err: apiutil.ErrInvalidQueryParams, + }, + { + desc: "list things with tags", + token: validToken, + listThingsResponse: mgclients.ClientsPage{ + Page: mgclients.Page{ + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + query: "tag=tag1,tag2", + status: http.StatusOK, + err: nil, + }, + { + desc: "list things with invalid tags", + token: validToken, + query: "tag=invalid", + status: http.StatusBadRequest, + err: apiutil.ErrValidation, + }, + { + desc: "list things with duplicate tags", + token: validToken, + query: "tag=tag1&tag=tag2", + status: http.StatusBadRequest, + err: apiutil.ErrInvalidQueryParams, + }, + { + desc: "list things with metadata", + token: validToken, + listThingsResponse: mgclients.ClientsPage{ + Page: mgclients.Page{ + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + query: "metadata=%7B%22domain%22%3A%20%22example.com%22%7D&", + status: http.StatusOK, + err: nil, + }, + { + desc: "list things with invalid metadata", + token: validToken, + query: "metadata=invalid", + status: http.StatusBadRequest, + err: apiutil.ErrValidation, + }, + { + desc: "list things with duplicate metadata", + token: validToken, + query: "metadata=%7B%22domain%22%3A%20%22example.com%22%7D&metadata=%7B%22domain%22%3A%20%22example.com%22%7D", + status: http.StatusBadRequest, + err: apiutil.ErrInvalidQueryParams, + }, + { + desc: "list things with permissions", + token: validToken, + listThingsResponse: mgclients.ClientsPage{ + Page: mgclients.Page{ + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + query: "permission=view", + status: http.StatusOK, + err: nil, + }, + { + desc: "list things with invalid permissions", + token: validToken, + query: "permission=invalid", + status: http.StatusBadRequest, + err: apiutil.ErrValidation, + }, + { + desc: "list things with duplicate permissions", + token: validToken, + query: "permission=view&permission=view", + status: http.StatusBadRequest, + err: apiutil.ErrInvalidQueryParams, + }, + { + desc: "list things with list perms", + token: validToken, + listThingsResponse: mgclients.ClientsPage{ + Page: mgclients.Page{ + Total: 1, + }, + Clients: []mgclients.Client{client}, + }, + query: "list_perms=true", + status: http.StatusOK, + err: nil, + }, + { + desc: "list things with invalid list perms", + token: validToken, + query: "list_perms=invalid", + status: http.StatusBadRequest, + err: apiutil.ErrValidation, + }, + { + desc: "list things with duplicate list perms", + token: validToken, + query: "list_perms=true&listPerms=true", + status: http.StatusBadRequest, + err: apiutil.ErrInvalidQueryParams, + }, + } + + for _, tc := range cases { + req := testRequest{ + client: ts.Client(), + method: http.MethodGet, + url: ts.URL + "/things?" + tc.query, + contentType: contentType, + token: tc.token, + } + + svcCall := svc.On("ListClients", mock.Anything, tc.token, "", mock.Anything).Return(tc.listThingsResponse, tc.err) + res, err := req.make() + assert.Nil(t, err, fmt.Sprintf("%s: unexpected error %s", tc.desc, err)) + + var bodyRes respBody + err = json.NewDecoder(res.Body).Decode(&bodyRes) + assert.Nil(t, err, fmt.Sprintf("%s: unexpected error while decoding response body: %s", tc.desc, err)) + if bodyRes.Err != "" || bodyRes.Message != "" { + err = errors.Wrap(errors.New(bodyRes.Err), errors.New(bodyRes.Message)) + } + assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) + assert.Equal(t, tc.status, res.StatusCode, fmt.Sprintf("%s: expected status code %d got %d", tc.desc, tc.status, res.StatusCode)) + svcCall.Unset() + } +} + func TestViewThing(t *testing.T) { ts, svc, _ := newThingsServer() defer ts.Close() From 4bfaf4e310b14fcdda5b76596c1987570e09d439 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Wed, 3 Jul 2024 12:57:26 +0300 Subject: [PATCH 26/31] Removed constructquery function Signed-off-by: nyagamunene --- pkg/clients/postgres/clients.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index 1a19fb4077..fd558871d7 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -191,9 +191,15 @@ func (repo *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clien } func (repo *Repository) SearchClients(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { - sq, tq := constructSearchQuery(pm) + query := buildQueryConditions(pm) + var emq string + if len(query) > 0 { + emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) + } + tq := emq + emq = applyOrdering(emq, pm) - q := fmt.Sprintf(`SELECT c.id, c.name, c.created_at, c.updated_at FROM clients c %s LIMIT :limit OFFSET :offset;`, sq) + q := fmt.Sprintf(`SELECT c.id, c.name, c.created_at, c.updated_at FROM clients c %s LIMIT :limit OFFSET :offset;`, emq) dbPage, err := ToDBClientsPage(pm) if err != nil { @@ -488,17 +494,6 @@ func PageQuery(pm clients.Page) (string, error) { return emq, nil } -func constructSearchQuery(pm clients.Page) (string, string) { - query := buildQueryConditions(pm) - var emq string - if len(query) > 0 { - emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) - } - tq := emq - emq = applyOrdering(emq, pm) - return emq, tq -} - func buildQueryConditions(pm clients.Page) []string { var query []string if pm.Name != "" { From 5d4ab2567c6dbaa81d937df7fa017320da66a252 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Wed, 3 Jul 2024 19:50:39 +0300 Subject: [PATCH 27/31] Refactor clients postgres Signed-off-by: nyagamunene --- pkg/clients/postgres/clients.go | 58 +++++++++++++++++++-------------- users/service.go | 2 ++ 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index fd558871d7..d397989f41 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -143,6 +143,7 @@ func (repo *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clien if err != nil { return clients.ClientsPage{}, errors.Wrap(repoerr.ErrViewEntity, err) } + query = applyOrdering(query, pm) q := fmt.Sprintf(`SELECT c.id, c.name, c.tags, c.identity, c.metadata, COALESCE(c.domain_id, '') AS domain_id, c.status, c.created_at, c.updated_at, COALESCE(c.updated_by, '') AS updated_by FROM clients c %s ORDER BY c.created_at LIMIT :limit OFFSET :offset;`, query) @@ -191,15 +192,15 @@ func (repo *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clien } func (repo *Repository) SearchClients(ctx context.Context, pm clients.Page) (clients.ClientsPage, error) { - query := buildQueryConditions(pm) - var emq string - if len(query) > 0 { - emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) + query, err := PageQuery(pm) + if err != nil { + return clients.ClientsPage{}, errors.Wrap(repoerr.ErrViewEntity, err) } - tq := emq - emq = applyOrdering(emq, pm) - q := fmt.Sprintf(`SELECT c.id, c.name, c.created_at, c.updated_at FROM clients c %s LIMIT :limit OFFSET :offset;`, emq) + tq := query + query = applyOrdering(query, pm) + + q := fmt.Sprintf(`SELECT c.id, c.name, c.created_at, c.updated_at FROM clients c %s LIMIT :limit OFFSET :offset;`, query) dbPage, err := ToDBClientsPage(pm) if err != nil { @@ -255,6 +256,7 @@ func (repo *Repository) RetrieveAllByIDs(ctx context.Context, pm clients.Page) ( if err != nil { return clients.ClientsPage{}, errors.Wrap(repoerr.ErrViewEntity, err) } + query = applyOrdering(query, pm) q := fmt.Sprintf(`SELECT c.id, c.name, c.tags, c.identity, c.metadata, COALESCE(c.domain_id, '') AS domain_id, c.status, c.created_at, c.updated_at, COALESCE(c.updated_by, '') AS updated_by FROM clients c %s ORDER BY c.created_at LIMIT :limit OFFSET :offset;`, query) @@ -476,17 +478,26 @@ func PageQuery(pm clients.Page) (string, error) { return "", errors.Wrap(errors.ErrMalformedEntity, err) } - query := buildQueryConditions(pm) + var query []string + if pm.Name != "" || pm.Identity != "" || pm.Id != "" || pm.Tag != "" { + return buildSearch(query, pm), nil + } if mq != "" { query = append(query, mq) } + if len(pm.IDs) != 0 { + query = append(query, fmt.Sprintf("id IN ('%s')", strings.Join(pm.IDs, "','"))) + } if pm.Status != clients.AllStatus { query = append(query, "c.status = :status") } if pm.Domain != "" { query = append(query, "c.domain_id = :domain_id") } + if pm.Role != clients.AllRole { + query = append(query, "c.role = :role") + } var emq string if len(query) > 0 { emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) @@ -494,8 +505,18 @@ func PageQuery(pm clients.Page) (string, error) { return emq, nil } -func buildQueryConditions(pm clients.Page) []string { - var query []string +func applyOrdering(emq string, pm clients.Page) string { + switch pm.Order { + case "name", "identity", "created_at", "updated_at": + emq = fmt.Sprintf("%s ORDER BY %s", emq, pm.Order) + if pm.Dir == api.AscDir || pm.Dir == api.DescDir { + emq = fmt.Sprintf("%s %s", emq, pm.Dir) + } + } + return emq +} + +func buildSearch(query []string, pm clients.Page) string { if pm.Name != "" { query = append(query, "name ILIKE '%' || :name || '%'") } @@ -508,23 +529,12 @@ func buildQueryConditions(pm clients.Page) []string { if pm.Tag != "" { query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") } - if len(query) == 0 && len(pm.IDs) != 0 { - query = append(query, fmt.Sprintf("id IN ('%s')", strings.Join(pm.IDs, "','"))) - } if pm.Role != clients.AllRole { query = append(query, "c.role = :role") } - - return query -} - -func applyOrdering(emq string, pm clients.Page) string { - switch pm.Order { - case "name", "identity", "created_at", "updated_at": - emq = fmt.Sprintf("%s ORDER BY %s", emq, pm.Order) - if pm.Dir == api.AscDir || pm.Dir == api.DescDir { - emq = fmt.Sprintf("%s %s", emq, pm.Dir) - } + var emq string + if len(query) > 0 { + emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) } return emq } diff --git a/users/service.go b/users/service.go index 5922b39de4..d7e66e75bb 100644 --- a/users/service.go +++ b/users/service.go @@ -181,6 +181,7 @@ func (svc service) ListClients(ctx context.Context, token string, pm mgclients.P return mgclients.ClientsPage{}, err } if err := svc.checkSuperAdmin(ctx, userID); err == nil { + pm.Role = mgclients.AllRole pg, err := svc.clients.RetrieveAll(ctx, pm) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrViewEntity, err) @@ -193,6 +194,7 @@ func (svc service) ListClients(ctx context.Context, token string, pm mgclients.P Offset: pm.Offset, Limit: pm.Limit, Name: pm.Name, + Id: pm.Id, Identity: pm.Identity, Role: mgclients.UserRole, } From 8346accc1b3799c5c54e1888f81b2d641a5381d8 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Thu, 4 Jul 2024 15:30:17 +0300 Subject: [PATCH 28/31] Remove buildquery method Signed-off-by: nyagamunene --- pkg/clients/postgres/clients.go | 45 +++++++++++++++------------------ users/service.go | 11 +------- 2 files changed, 21 insertions(+), 35 deletions(-) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index d397989f41..a9f3b8bc3d 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -479,9 +479,27 @@ func PageQuery(pm clients.Page) (string, error) { } var query []string - if pm.Name != "" || pm.Identity != "" || pm.Id != "" || pm.Tag != "" { - return buildSearch(query, pm), nil + if pm.Name != "" { + query = append(query, "name ILIKE '%' || :name || '%'") + } + if pm.Identity != "" { + query = append(query, "identity ILIKE '%' || :identity || '%'") } + if pm.Id != "" { + query = append(query, "id ILIKE '%' || :id || '%'") + } + if pm.Tag != "" { + query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") + } + if pm.Role != clients.AllRole { + query = append(query, "c.role = :role") + } + // If there are search params presents, use search and ignore other options. + // Always combine role with search params, so len(query) > 1. + if len(query) > 1 { + return fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")), nil + } + if mq != "" { query = append(query, mq) } @@ -515,26 +533,3 @@ func applyOrdering(emq string, pm clients.Page) string { } return emq } - -func buildSearch(query []string, pm clients.Page) string { - if pm.Name != "" { - query = append(query, "name ILIKE '%' || :name || '%'") - } - if pm.Identity != "" { - query = append(query, "identity ILIKE '%' || :identity || '%'") - } - if pm.Id != "" { - query = append(query, "id ILIKE '%' || :id || '%'") - } - if pm.Tag != "" { - query = append(query, "EXISTS (SELECT 1 FROM unnest(tags) AS tag WHERE tag ILIKE '%' || :tag || '%')") - } - if pm.Role != clients.AllRole { - query = append(query, "c.role = :role") - } - var emq string - if len(query) > 0 { - emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) - } - return emq -} diff --git a/users/service.go b/users/service.go index d7e66e75bb..3fd810743e 100644 --- a/users/service.go +++ b/users/service.go @@ -189,16 +189,7 @@ func (svc service) ListClients(ctx context.Context, token string, pm mgclients.P return pg, err } - p := mgclients.Page{ - Status: mgclients.EnabledStatus, - Offset: pm.Offset, - Limit: pm.Limit, - Name: pm.Name, - Id: pm.Id, - Identity: pm.Identity, - Role: mgclients.UserRole, - } - pg, err := svc.clients.SearchClients(ctx, p) + pg, err := svc.clients.SearchClients(ctx, pm) if err != nil { return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrViewEntity, err) } From 2b022e2e146b5a729fe65e0001d43d7fb6bd1c12 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Thu, 4 Jul 2024 15:37:01 +0300 Subject: [PATCH 29/31] Remove double role commands Signed-off-by: nyagamunene --- pkg/clients/postgres/clients.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index a9f3b8bc3d..ad1aeddd4b 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -513,9 +513,6 @@ func PageQuery(pm clients.Page) (string, error) { if pm.Domain != "" { query = append(query, "c.domain_id = :domain_id") } - if pm.Role != clients.AllRole { - query = append(query, "c.role = :role") - } var emq string if len(query) > 0 { emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND ")) From 192ed489deaf190f6321bedf22c763b22f87757c Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Thu, 4 Jul 2024 15:54:40 +0300 Subject: [PATCH 30/31] Remove SearchUser in service Signed-off-by: nyagamunene --- users/api/logging.go | 21 ------------ users/api/metrics.go | 9 ----- users/clients.go | 3 -- users/events/events.go | 22 ------------ users/events/streams.go | 16 --------- users/service.go | 14 -------- users/service_test.go | 73 ---------------------------------------- users/tracing/tracing.go | 8 ----- 8 files changed, 166 deletions(-) diff --git a/users/api/logging.go b/users/api/logging.go index 0e0af94ba5..7ed01bcbe7 100644 --- a/users/api/logging.go +++ b/users/api/logging.go @@ -381,27 +381,6 @@ func (lm *loggingMiddleware) ListMembers(ctx context.Context, token, objectKind, return lm.svc.ListMembers(ctx, token, objectKind, objectID, cp) } -// SearchClients logs the search_clients request. It logs the page metadata and the time it took to complete the request. -func (lm *loggingMiddleware) SearchUsers(ctx context.Context, token string, cp mgclients.Page) (mp mgclients.ClientsPage, err error) { - defer func(begin time.Time) { - args := []any{ - slog.String("duration", time.Since(begin).String()), - slog.Group("page", - slog.Uint64("limit", cp.Limit), - slog.Uint64("offset", cp.Offset), - slog.Uint64("total", mp.Total), - ), - } - if err != nil { - args = append(args, slog.Any("error", err)) - lm.logger.Warn("Search clients failed to complete successfully", args...) - return - } - lm.logger.Info("Search clients completed successfully", args...) - }(time.Now()) - return lm.svc.SearchUsers(ctx, token, cp) -} - // Identify logs the identify request. It logs the time it took to complete the request. func (lm *loggingMiddleware) Identify(ctx context.Context, token string) (id string, err error) { defer func(begin time.Time) { diff --git a/users/api/metrics.go b/users/api/metrics.go index 667826180f..ccf231adda 100644 --- a/users/api/metrics.go +++ b/users/api/metrics.go @@ -183,15 +183,6 @@ func (ms *metricsMiddleware) ListMembers(ctx context.Context, token, objectKind, return ms.svc.ListMembers(ctx, token, objectKind, objectID, pm) } -// SearchClients instruments SearchClients method with metrics. -func (ms *metricsMiddleware) SearchUsers(ctx context.Context, token string, pm mgclients.Page) (mp mgclients.ClientsPage, err error) { - defer func(begin time.Time) { - ms.counter.With("method", "search_clients").Add(1) - ms.latency.With("method", "search_clients").Observe(time.Since(begin).Seconds()) - }(time.Now()) - return ms.svc.SearchUsers(ctx, token, pm) -} - // Identify instruments Identify method with metrics. func (ms *metricsMiddleware) Identify(ctx context.Context, token string) (string, error) { defer func(begin time.Time) { diff --git a/users/clients.go b/users/clients.go index 078f8ce4fb..e40c4f226d 100644 --- a/users/clients.go +++ b/users/clients.go @@ -31,9 +31,6 @@ type Service interface { // ListMembers retrieves everything that is assigned to a group/thing identified by objectID. ListMembers(ctx context.Context, token, objectKind, objectID string, pm clients.Page) (clients.MembersPage, error) - // SearchClients searches for users with provided filters for a valid auth token. - SearchUsers(ctx context.Context, token string, pm clients.Page) (clients.ClientsPage, error) - // UpdateClient updates the client's name and metadata. UpdateClient(ctx context.Context, token string, client clients.Client) (clients.Client, error) diff --git a/users/events/events.go b/users/events/events.go index 5fe80b455d..dda7a669a6 100644 --- a/users/events/events.go +++ b/users/events/events.go @@ -19,7 +19,6 @@ const ( profileView = clientPrefix + "view_profile" clientList = clientPrefix + "list" clientListByGroup = clientPrefix + "list_by_group" - clientSearch = clientPrefix + "search" clientIdentify = clientPrefix + "identify" generateResetToken = clientPrefix + "generate_reset_token" issueToken = clientPrefix + "issue_token" @@ -308,27 +307,6 @@ func (lcge listClientByGroupEvent) Encode() (map[string]interface{}, error) { return val, nil } -type searchClientEvent struct { - mgclients.Page -} - -func (sce searchClientEvent) Encode() (map[string]interface{}, error) { - val := map[string]interface{}{ - "operation": clientSearch, - "total": sce.Total, - "offset": sce.Offset, - "limit": sce.Limit, - } - if sce.Name != "" { - val["name"] = sce.Name - } - if sce.Identity != "" { - val["identity"] = sce.Identity - } - - return val, nil -} - type identifyClientEvent struct { userID string } diff --git a/users/events/streams.go b/users/events/streams.go index df479505f3..5563d89478 100644 --- a/users/events/streams.go +++ b/users/events/streams.go @@ -176,22 +176,6 @@ func (es *eventStore) ListMembers(ctx context.Context, token, objectKind, object return mp, nil } -func (es *eventStore) SearchUsers(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { - cp, err := es.svc.SearchUsers(ctx, token, pm) - if err != nil { - return cp, err - } - event := searchClientEvent{ - pm, - } - - if err := es.Publish(ctx, event); err != nil { - return cp, err - } - - return cp, nil -} - func (es *eventStore) EnableClient(ctx context.Context, token, id string) (mgclients.Client, error) { user, err := es.svc.EnableClient(ctx, token, id) if err != nil { diff --git a/users/service.go b/users/service.go index 3fd810743e..07ec3b972f 100644 --- a/users/service.go +++ b/users/service.go @@ -546,20 +546,6 @@ func (svc service) ListMembers(ctx context.Context, token, objectKind, objectID }, nil } -func (svc service) SearchUsers(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { - _, err := svc.identify(ctx, token) - if err != nil { - return mgclients.ClientsPage{}, err - } - - cp, err := svc.clients.SearchClients(ctx, pm) - if err != nil { - return mgclients.ClientsPage{}, errors.Wrap(svcerr.ErrSearch, err) - } - - return cp, nil -} - func (svc service) retrieveObjectUsersPermissions(ctx context.Context, domainID, objectType, objectID string, client *mgclients.Client) error { userID := auth.EncodeDomainUserID(domainID, client.ID) permissions, err := svc.listObjectUserPermission(ctx, userID, objectType, objectID) diff --git a/users/service_test.go b/users/service_test.go index 224c080111..404e8e8b00 100644 --- a/users/service_test.go +++ b/users/service_test.go @@ -2169,79 +2169,6 @@ func TestListMembers(t *testing.T) { } } -func TestSearchUsers(t *testing.T) { - svc, cRepo, auth, _ := newService(true) - cases := []struct { - desc string - token string - page mgclients.Page - identifyResp *magistrala.IdentityRes - response mgclients.ClientsPage - responseErr error - identifyErr error - err error - }{ - { - desc: "search clients with valid token", - token: validToken, - page: mgclients.Page{Offset: 0, Name: "clientname", Limit: 100}, - response: mgclients.ClientsPage{ - Page: mgclients.Page{Total: 1, Offset: 0, Limit: 100}, - Clients: []mgclients.Client{client}, - }, - identifyResp: &magistrala.IdentityRes{UserId: client.ID}, - }, - { - desc: "search clients with invalid token", - token: inValidToken, - page: mgclients.Page{Offset: 0, Name: "clientname", Limit: 100}, - response: mgclients.ClientsPage{}, - responseErr: svcerr.ErrAuthentication, - err: svcerr.ErrAuthentication, - }, - { - desc: "search clients with id", - token: validToken, - page: mgclients.Page{Offset: 0, Id: "d8dd12ef-aa2a-43fe-8ef2-2e4fe514360f", Limit: 100}, - response: mgclients.ClientsPage{ - Page: mgclients.Page{Total: 1, Offset: 0, Limit: 100}, - Clients: []mgclients.Client{client}, - }, - identifyResp: &magistrala.IdentityRes{UserId: client.ID}, - }, - { - desc: "search clients with username", - token: validToken, - page: mgclients.Page{Offset: 0, Identity: "clientidentity", Limit: 100}, - response: mgclients.ClientsPage{ - Page: mgclients.Page{Total: 1, Offset: 0, Limit: 100}, - Clients: []mgclients.Client{client}, - }, - identifyResp: &magistrala.IdentityRes{UserId: client.ID}, - }, - { - desc: "search clients with random name", - token: validToken, - page: mgclients.Page{Offset: 0, Name: "randomname", Limit: 100}, - response: mgclients.ClientsPage{ - Page: mgclients.Page{Total: 0, Offset: 0, Limit: 100}, - Clients: []mgclients.Client{}, - }, - identifyResp: &magistrala.IdentityRes{UserId: client.ID}, - }, - } - - for _, tc := range cases { - authCall := auth.On("Identify", context.Background(), &magistrala.IdentityReq{Token: tc.token}).Return(tc.identifyResp, tc.identifyErr) - repoCall := cRepo.On("SearchClients", context.Background(), tc.page).Return(tc.response, tc.responseErr) - page, err := svc.SearchUsers(context.Background(), tc.token, tc.page) - assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) - assert.Equal(t, tc.response, page, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, page)) - authCall.Unset() - repoCall.Unset() - } -} - func TestIssueToken(t *testing.T) { svc, cRepo, auth, _ := newService(true) diff --git a/users/tracing/tracing.go b/users/tracing/tracing.go index 7d4674c219..ff93c915b6 100644 --- a/users/tracing/tracing.go +++ b/users/tracing/tracing.go @@ -185,14 +185,6 @@ func (tm *tracingMiddleware) ListMembers(ctx context.Context, token, objectKind, return tm.svc.ListMembers(ctx, token, objectKind, objectID, pm) } -// SearchClients traces the "SearchClients" operation of the wrapped clients.Service. -func (tm *tracingMiddleware) SearchUsers(ctx context.Context, token string, pm mgclients.Page) (mgclients.ClientsPage, error) { - ctx, span := tm.tracer.Start(ctx, "svc_search_clients", trace.WithAttributes(attribute.String("token", token))) - defer span.End() - - return tm.svc.SearchUsers(ctx, token, pm) -} - // Identify traces the "Identify" operation of the wrapped clients.Service. func (tm *tracingMiddleware) Identify(ctx context.Context, token string) (string, error) { ctx, span := tm.tracer.Start(ctx, "svc_identify", trace.WithAttributes(attribute.String("token", token))) From a466a117cc5a23b9b82cc69ad72287dcce5f8346 Mon Sep 17 00:00:00 2001 From: nyagamunene Date: Thu, 4 Jul 2024 15:57:11 +0300 Subject: [PATCH 31/31] Update mocks Signed-off-by: nyagamunene --- users/mocks/service.go | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/users/mocks/service.go b/users/mocks/service.go index f143cef06c..cd769feeca 100644 --- a/users/mocks/service.go +++ b/users/mocks/service.go @@ -331,34 +331,6 @@ func (_m *Service) ResetSecret(ctx context.Context, resetToken string, secret st return r0 } -// SearchUsers provides a mock function with given fields: ctx, token, pm -func (_m *Service) SearchUsers(ctx context.Context, token string, pm clients.Page) (clients.ClientsPage, error) { - ret := _m.Called(ctx, token, pm) - - if len(ret) == 0 { - panic("no return value specified for SearchUsers") - } - - var r0 clients.ClientsPage - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, clients.Page) (clients.ClientsPage, error)); ok { - return rf(ctx, token, pm) - } - if rf, ok := ret.Get(0).(func(context.Context, string, clients.Page) clients.ClientsPage); ok { - r0 = rf(ctx, token, pm) - } else { - r0 = ret.Get(0).(clients.ClientsPage) - } - - if rf, ok := ret.Get(1).(func(context.Context, string, clients.Page) error); ok { - r1 = rf(ctx, token, pm) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // SendPasswordReset provides a mock function with given fields: ctx, host, email, user, token func (_m *Service) SendPasswordReset(ctx context.Context, host string, email string, user string, token string) error { ret := _m.Called(ctx, host, email, user, token)