diff --git a/users/api/endpoint.go b/users/api/endpoint.go index 0b44f4d867..966388f115 100644 --- a/users/api/endpoint.go +++ b/users/api/endpoint.go @@ -259,23 +259,21 @@ func updateGroupEndpoint(svc users.Service) endpoint.Endpoint { return func(ctx context.Context, request interface{}) (interface{}, error) { req := request.(updateGroupReq) if err := req.validate(); err != nil { - return createGroupRes{}, err + return updateGroupRes{}, err } + group := users.Group{ + ID: req.id, Name: req.Name, Description: req.Description, Metadata: req.Metadata, } + if err := svc.UpdateGroup(ctx, req.token, group); err != nil { - return createGroupRes{}, err + return updateGroupRes{}, err } - res := createGroupRes{ - Name: group.Name, - Description: group.Description, - Metadata: group.Metadata, - created: false, - } - return res, nil + + return updateGroupRes{}, nil } } @@ -290,7 +288,10 @@ func viewGroupEndpoint(svc users.Service) endpoint.Endpoint { return viewGroupRes{}, err } res := viewGroupRes{ + ID: group.ID, Name: group.Name, + ParentID: group.ParentID, + OwnerID: group.OwnerID, Description: group.Description, Metadata: group.Metadata, } diff --git a/users/api/endpoint_test.go b/users/api/endpoint_test.go index c16be7a45e..d342960df6 100644 --- a/users/api/endpoint_test.go +++ b/users/api/endpoint_test.go @@ -475,7 +475,7 @@ func TestGroupCreate(t *testing.T) { {"group create with existing name", createValidTokenRequest, contentType, http.StatusConflict, groupExists, token}, {"group create with invalid token", createInvalidTokenRequest, contentType, http.StatusForbidden, unauthRes, ""}, {"group create with empty JSON request", "{}", contentType, http.StatusBadRequest, malformedRes, token}, - {"group create empty request", "", contentType, http.StatusBadRequest, failDecodeRes, token}, + {"group create empty request", "", contentType, http.StatusBadRequest, malformedRes, token}, {"group create missing content type", createValidTokenRequest, "", http.StatusUnsupportedMediaType, unsupportedRes, token}, } diff --git a/users/api/requests.go b/users/api/requests.go index b1ecf72d27..280972f18a 100644 --- a/users/api/requests.go +++ b/users/api/requests.go @@ -129,7 +129,9 @@ func (req createGroupReq) validate() error { type updateGroupReq struct { token string + id string Name string `json:"name,omitempty"` + ParentID string `json:"parent_id,omitempty"` Description string `json:"description,omitempty"` Metadata map[string]interface{} `json:"metadata,omitempty"` } @@ -138,12 +140,13 @@ func (req updateGroupReq) validate() error { if req.token == "" { return users.ErrUnauthorizedAccess } - if req.Name == "" { + if req.id == "" { return users.ErrMalformedEntity } - if len(req.Name) > maxNameSize { + if req.Name == "" || len(req.Name) > maxNameSize { return users.ErrMalformedEntity } + return nil } diff --git a/users/api/responses.go b/users/api/responses.go index 9c6f90c919..37ee99e130 100644 --- a/users/api/responses.go +++ b/users/api/responses.go @@ -6,9 +6,9 @@ package api import ( "fmt" "net/http" - "os/user" "github.com/mainflux/mainflux" + "github.com/mainflux/mainflux/users" ) var ( @@ -93,7 +93,7 @@ func (res updateUserRes) Empty() bool { type viewUserRes struct { ID string `json:"id"` Email string `json:"email"` - Groups []user.Group `json:"groups"` + Groups []users.Group `json:"groups"` Metadata map[string]interface{} `json:"metadata,omitempty"` } @@ -213,7 +213,7 @@ func (res passwChangeRes) Empty() bool { type groupPageRes struct { pageRes - Groups []viewGroupRes `json:"groups"` + Groups []viewGroupRes } func (res groupPageRes) Code() int { diff --git a/users/api/transport.go b/users/api/transport.go index 016fd56533..086ee55453 100644 --- a/users/api/transport.go +++ b/users/api/transport.go @@ -160,7 +160,7 @@ func MakeHandler(svc users.Service, tracer opentracing.Tracer) http.Handler { mux.Patch("/groups/:groupID", kithttp.NewServer( kitot.TraceServer(tracer, "update_group")(updateGroupEndpoint(svc)), - decodeGroupCreate, + decodeGroupUpdate, encodeResponse, opts..., )) @@ -312,7 +312,7 @@ func decodeGroupCreate(_ context.Context, r *http.Request) (interface{}, error) var req createGroupReq if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - return nil, errors.Wrap(ErrFailedDecode, err) + return nil, errors.Wrap(users.ErrMalformedEntity, err) } req.token = r.Header.Get("Authorization") @@ -320,11 +320,23 @@ func decodeGroupCreate(_ context.Context, r *http.Request) (interface{}, error) return req, nil } -func decodeGroupRequest(_ context.Context, r *http.Request) (interface{}, error) { +func decodeGroupUpdate(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { return nil, ErrUnsupportedContentType } + req := updateGroupReq{ + token: r.Header.Get("Authorization"), + id: bone.GetValue(r, "groupID"), + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + return nil, errors.Wrap(users.ErrMalformedEntity, err) + } + + return req, nil +} + +func decodeGroupRequest(_ context.Context, r *http.Request) (interface{}, error) { req := groupReq{ token: r.Header.Get("Authorization"), groupID: bone.GetValue(r, "groupID"), @@ -371,10 +383,6 @@ func decodeListUserGroupsRequest(_ context.Context, r *http.Request) (interface{ } func decodeUserGroupRequest(_ context.Context, r *http.Request) (interface{}, error) { - if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, ErrUnsupportedContentType - } - req := userGroupReq{ token: r.Header.Get("Authorization"), groupID: bone.GetValue(r, "groupID"), diff --git a/users/postgres/groups.go b/users/postgres/groups.go index 8f03ede319..55980787b7 100644 --- a/users/postgres/groups.go +++ b/users/postgres/groups.go @@ -75,15 +75,14 @@ func (gr groupRepository) Save(ctx context.Context, group users.Group) (users.Gr } func (gr groupRepository) Update(ctx context.Context, group users.Group) error { - q := `UPDATE groups SET(name, description, metadata) VALUES (:name, :description, :metadata) WHERE id = :id` - + q := `UPDATE groups SET name = :name, metadata = :metadata, description = :description WHERE id = :id;` dbu, err := toDBGroup(group) if err != nil { - return errors.Wrap(errUpdateDB, err) + return errors.Wrap(users.ErrUpdateGroup, err) } if _, err := gr.db.NamedExecContext(ctx, q, dbu); err != nil { - return errors.Wrap(errUpdateDB, err) + return errors.Wrap(users.ErrUpdateGroup, err) } return nil diff --git a/users/postgres/groups_test.go b/users/postgres/groups_test.go index 7741befcd1..656c93cc02 100644 --- a/users/postgres/groups_test.go +++ b/users/postgres/groups_test.go @@ -6,6 +6,7 @@ package postgres_test import ( "context" "fmt" + "strings" "testing" "github.com/mainflux/mainflux/pkg/errors" @@ -18,10 +19,15 @@ import ( ) const ( - groupName = "Mainflux" - password = "12345678" + maxNameSize = 254 + maxDescSize = 1024 + groupName = "Mainflux" + password = "12345678" ) +var invalidName = strings.Repeat("m", maxNameSize+1) +var invalidDesc = strings.Repeat("m", maxDescSize+1) + func TestGroupSave(t *testing.T) { dbMiddleware := postgres.NewDatabase(db) repo := postgres.NewGroupRepo(dbMiddleware) @@ -142,6 +148,65 @@ func TestGroupRetrieveByID(t *testing.T) { } } +func TestGroupUpdate(t *testing.T) { + dbMiddleware := postgres.NewDatabase(db) + groupRepo := postgres.NewGroupRepo(dbMiddleware) + + gid, err := uuid.New().ID() + require.Nil(t, err, fmt.Sprintf("group id unexpected error: %s", err)) + group := users.Group{ + ID: gid, + Name: groupName, + } + + _, err = groupRepo.Save(context.Background(), group) + require.Nil(t, err, fmt.Sprintf("group save got unexpected error: %s", err)) + + cases := []struct { + desc string + group users.Group + err error + }{ + { + desc: "update group for existing id", + group: users.Group{ + ID: gid, + Name: groupName + "-1", + }, + err: nil, + }, + { + desc: "update group for non-existing id", + group: users.Group{ + ID: "wrong", + Name: groupName + "-2", + }, + err: users.ErrUpdateGroup, + }, + { + desc: "update group for invalid name", + group: users.Group{ + ID: gid, + Name: invalidName, + }, + err: users.ErrUpdateGroup, + }, + { + desc: "update group for invalid description", + group: users.Group{ + ID: gid, + Description: invalidDesc, + }, + err: users.ErrUpdateGroup, + }, + } + + for _, tc := range cases { + err := groupRepo.Update(context.Background(), tc.group) + assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err)) + } +} + func TestGroupDelete(t *testing.T) { dbMiddleware := postgres.NewDatabase(db) repo := postgres.NewGroupRepo(dbMiddleware) diff --git a/users/service.go b/users/service.go index 73ca522073..2dbb1557ea 100644 --- a/users/service.go +++ b/users/service.go @@ -59,6 +59,9 @@ var ( // ErrCreateGroup indicates error in creating group. ErrCreateGroup = errors.New("failed to create group") + // ErrUpdateGroup indicates error in updating group. + ErrUpdateGroup = errors.New("failed to update group") + // ErrDeleteGroupMissing indicates in delete operation that group doesnt exist. ErrDeleteGroupMissing = errors.New("group is not existing, already deleted")