Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NOISSUE - Fix ViewGroup and UpdateGroup #1269

Merged
merged 5 commits into from
Oct 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions users/api/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mising ID

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
}
}

Expand All @@ -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,
}
Expand Down
2 changes: 1 addition & 1 deletion users/api/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
}

Expand Down
7 changes: 5 additions & 2 deletions users/api/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need parent id here also

Metadata map[string]interface{} `json:"metadata,omitempty"`
}
Expand All @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions users/api/responses.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ package api
import (
"fmt"
"net/http"
"os/user"

"github.com/mainflux/mainflux"
"github.com/mainflux/mainflux/users"
)

var (
Expand Down Expand Up @@ -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"`
}

Expand Down Expand Up @@ -213,7 +213,7 @@ func (res passwChangeRes) Empty() bool {

type groupPageRes struct {
pageRes
Groups []viewGroupRes `json:"groups"`
Groups []viewGroupRes
}

func (res groupPageRes) Code() int {
Expand Down
22 changes: 15 additions & 7 deletions users/api/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...,
))
Expand Down Expand Up @@ -312,19 +312,31 @@ 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")

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"),
Expand Down Expand Up @@ -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"),
Expand Down
7 changes: 3 additions & 4 deletions users/postgres/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
69 changes: 67 additions & 2 deletions users/postgres/groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package postgres_test
import (
"context"
"fmt"
"strings"
"testing"

"github.com/mainflux/mainflux/pkg/errors"
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions users/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down