Skip to content

Commit

Permalink
NOISSUE - Fix Failing Users Property Based Tests (#2134)
Browse files Browse the repository at this point in the history
Signed-off-by: Rodney Osodo <[email protected]>
  • Loading branch information
rodneyosodo authored Apr 3, 2024
1 parent a39d462 commit eb6b201
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 90 deletions.
18 changes: 13 additions & 5 deletions api/openapi/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -383,14 +383,16 @@ paths:
tags:
- Users
parameters:
- $ref: "#/components/parameters/Referrer"
- $ref: "#/components/parameters/Referer"
requestBody:
$ref: "#/components/requestBodies/RequestPasswordReset"
responses:
"201":
description: Users link for resetting password.
"400":
description: Failed due to malformed JSON.
"404":
description: A non-existent entity request.
"415":
description: Missing or invalid content type.
"422":
Expand Down Expand Up @@ -883,8 +885,8 @@ paths:
security:
- bearerAuth: []
responses:
"200":
description: Member assigned.
"204":
description: Member unassigned.
"400":
description: Failed due to malformed group's ID.
"401":
Expand Down Expand Up @@ -1211,6 +1213,7 @@ components:
secret:
type: string
example: password
minimum: 8
description: User secret password.
metadata:
type: object
Expand Down Expand Up @@ -1352,10 +1355,12 @@ components:
old_secret:
type: string
example: oldpassword
minimum: 8
description: Old user secret password.
new_secret:
type: string
example: newpassword
minimum: 8
description: New user secret password.
required:
- old_secret
Expand Down Expand Up @@ -1451,6 +1456,7 @@ components:
secret:
type: string
example: password
minimum: 8
description: User secret password.
required:
- identity
Expand Down Expand Up @@ -1490,8 +1496,8 @@ components:
example: 1970-01-01_00:00:00

parameters:
Referrer:
name: Referrer
Referer:
name: Referer
description: Host being sent by browser.
in: header
schema:
Expand Down Expand Up @@ -1815,9 +1821,11 @@ components:
password:
type: string
format: password
minimum: 8
description: New password.
old_password:
type: string
minimum: 8
format: password
description: Old password.

Expand Down
3 changes: 1 addition & 2 deletions auth/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"time"

"github.com/absmach/magistrala"
"github.com/absmach/magistrala/internal/postgres"
"github.com/absmach/magistrala/pkg/errors"
svcerr "github.com/absmach/magistrala/pkg/errors/service"
)
Expand Down Expand Up @@ -670,7 +669,7 @@ func (svc service) ListDomains(ctx context.Context, token string, p Page) (Domai
}
dp, err := svc.domains.ListDomains(ctx, p)
if err != nil {
return DomainsPage{}, postgres.HandleError(svcerr.ErrViewEntity, err)
return DomainsPage{}, errors.Wrap(svcerr.ErrViewEntity, err)
}
if p.SubjectID == "" {
for i := range dp.Domains {
Expand Down
35 changes: 17 additions & 18 deletions internal/groups/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ import (
)

var (
errParentUnAuthz = errors.New("failed to authorize parent group")
errMemberKind = errors.New("invalid member kind")
errRetrieveGroups = errors.New("failed to retrieve groups")
errGroupIDs = errors.New("invalid group ids")
errParentUnAuthz = errors.New("failed to authorize parent group")
errMemberKind = errors.New("invalid member kind")
errGroupIDs = errors.New("invalid group ids")
)

type service struct {
Expand Down Expand Up @@ -70,7 +69,7 @@ func (svc service) CreateGroup(ctx context.Context, token, kind string, g groups

g, err = svc.groups.Save(ctx, g)
if err != nil {
return groups.Group{}, err
return groups.Group{}, errors.Wrap(svcerr.ErrCreateEntity, err)
}
// IMPROVEMENT NOTE: Add defer function , if return err is not nil, then delete group

Expand Down Expand Up @@ -104,7 +103,7 @@ func (svc service) CreateGroup(ctx context.Context, token, kind string, g groups
})
}
if _, err := svc.auth.AddPolicies(ctx, &policies); err != nil {
return g, err
return g, errors.Wrap(svcerr.ErrAddPolicies, err)
}

return g, nil
Expand Down Expand Up @@ -228,7 +227,7 @@ func (svc service) ListGroups(ctx context.Context, token, memberKind, memberID s

gp, err := svc.groups.RetrieveByIDs(ctx, gm, ids...)
if err != nil {
return groups.Page{}, err
return groups.Page{}, errors.Wrap(svcerr.ErrViewEntity, err)
}

if gm.ListPerms && len(gp.Groups) > 0 {
Expand Down Expand Up @@ -454,7 +453,7 @@ func (svc service) Assign(ctx context.Context, token, groupID, relation, memberK
func (svc service) assignParentGroup(ctx context.Context, domain, parentGroupID string, groupIDs []string) (err error) {
groupsPage, err := svc.groups.RetrieveByIDs(ctx, groups.Page{PageMeta: groups.PageMeta{Limit: 1<<63 - 1}}, groupIDs...)
if err != nil {
return errors.Wrap(errRetrieveGroups, err)
return errors.Wrap(svcerr.ErrViewEntity, err)
}
if len(groupsPage.Groups) == 0 {
return errGroupIDs
Expand Down Expand Up @@ -484,7 +483,7 @@ func (svc service) assignParentGroup(ctx context.Context, domain, parentGroupID
}

if _, err := svc.auth.AddPolicies(ctx, &addPolicies); err != nil {
return err
return errors.Wrap(svcerr.ErrAddPolicies, err)
}
defer func() {
if err != nil {
Expand All @@ -500,7 +499,7 @@ func (svc service) assignParentGroup(ctx context.Context, domain, parentGroupID
func (svc service) unassignParentGroup(ctx context.Context, domain, parentGroupID string, groupIDs []string) (err error) {
groupsPage, err := svc.groups.RetrieveByIDs(ctx, groups.Page{PageMeta: groups.PageMeta{Limit: 1<<63 - 1}}, groupIDs...)
if err != nil {
return errors.Wrap(errRetrieveGroups, err)
return errors.Wrap(svcerr.ErrViewEntity, err)
}
if len(groupsPage.Groups) == 0 {
return errGroupIDs
Expand Down Expand Up @@ -530,7 +529,7 @@ func (svc service) unassignParentGroup(ctx context.Context, domain, parentGroupI
}

if _, err := svc.auth.DeletePolicies(ctx, &deletePolicies); err != nil {
return err
return errors.Wrap(svcerr.ErrDeletePolicies, err)
}
defer func() {
if err != nil {
Expand Down Expand Up @@ -616,7 +615,7 @@ func (svc service) DeleteGroup(ctx context.Context, token, groupID string) error
Subject: groupID,
ObjectType: auth.GroupType,
}); err != nil {
return err
return errors.Wrap(svcerr.ErrDeletePolicies, err)
}

// Remove policy of things
Expand All @@ -625,7 +624,7 @@ func (svc service) DeleteGroup(ctx context.Context, token, groupID string) error
Subject: groupID,
ObjectType: auth.ThingType,
}); err != nil {
return err
return errors.Wrap(svcerr.ErrDeletePolicies, err)
}

// Remove policy from domain
Expand All @@ -634,12 +633,12 @@ func (svc service) DeleteGroup(ctx context.Context, token, groupID string) error
Object: groupID,
ObjectType: auth.GroupType,
}); err != nil {
return err
return errors.Wrap(svcerr.ErrDeletePolicies, err)
}

// Remove group from database
if err := svc.groups.Delete(ctx, groupID); err != nil {
return err
return errors.Wrap(svcerr.ErrRemoveEntity, err)
}

// Remove policy of users
Expand All @@ -648,7 +647,7 @@ func (svc service) DeleteGroup(ctx context.Context, token, groupID string) error
Object: groupID,
ObjectType: auth.GroupType,
}); err != nil {
return err
return errors.Wrap(svcerr.ErrDeletePolicies, err)
}

return nil
Expand Down Expand Up @@ -691,7 +690,7 @@ func (svc service) changeGroupStatus(ctx context.Context, token string, group gr
}
dbGroup, err := svc.groups.RetrieveByID(ctx, group.ID)
if err != nil {
return groups.Group{}, err
return groups.Group{}, errors.Wrap(svcerr.ErrViewEntity, err)
}
if dbGroup.Status == group.Status {
return groups.Group{}, errors.ErrStatusAlreadyAssigned
Expand All @@ -704,7 +703,7 @@ func (svc service) changeGroupStatus(ctx context.Context, token string, group gr
func (svc service) identify(ctx context.Context, token string) (*magistrala.IdentityRes, error) {
res, err := svc.auth.Identify(ctx, &magistrala.IdentityReq{Token: token})
if err != nil {
return nil, err
return nil, errors.Wrap(svcerr.ErrAuthentication, err)
}
if res.GetId() == "" || res.GetDomainId() == "" {
return nil, svcerr.ErrDomainAuthorization
Expand Down
3 changes: 3 additions & 0 deletions pkg/errors/service/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ var (
// ErrDeletePolicies indicates failed to delete policies.
ErrDeletePolicies = errors.New("failed to delete policies")

// ErrIssueToken indicates a failure to issue token.
ErrIssueToken = errors.New("failed to issue token")

// ErrPasswordFormat indicates weak password.
ErrPasswordFormat = errors.New("password does not meet the requirements")

Expand Down
10 changes: 5 additions & 5 deletions pkg/sdk/go/channels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func TestCreateChannel(t *testing.T) {
Status: mgclients.EnabledStatus.String(),
},
token: token,
err: errors.NewSDKErrorWithStatus(repoerr.ErrCreateEntity, http.StatusUnprocessableEntity),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrCreateEntity, svcerr.ErrCreateEntity), http.StatusUnprocessableEntity),
},
{
desc: "create channel with missing name",
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestListChannels(t *testing.T) {
token: invalidToken,
offset: offset,
limit: limit,
err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized),
response: nil,
},
{
Expand Down Expand Up @@ -616,7 +616,7 @@ func TestListChannelsByThing(t *testing.T) {
clientID: testsutil.GenerateUUID(t),
page: sdk.PageMetadata{},
response: []sdk.Channel(nil),
err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized),
},
}

Expand Down Expand Up @@ -659,7 +659,7 @@ func TestEnableChannel(t *testing.T) {
repoCall1 := grepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(mggroups.Group{}, repoerr.ErrNotFound)
repoCall2 := grepo.On("ChangeStatus", mock.Anything, mock.Anything).Return(nil)
_, err := mgsdk.EnableChannel("wrongID", validToken)
assert.Equal(t, errors.NewSDKErrorWithStatus(svcerr.ErrNotFound, http.StatusNotFound), err, fmt.Sprintf("Enable channel with wrong id: expected %v got %v", svcerr.ErrNotFound, err))
assert.Equal(t, errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrNotFound), http.StatusNotFound), err, fmt.Sprintf("Enable channel with wrong id: expected %v got %v", svcerr.ErrNotFound, err))
ok := repoCall1.Parent.AssertCalled(t, "RetrieveByID", mock.Anything, "wrongID")
assert.True(t, ok, "RetrieveByID was not called on enabling channel")
repoCall.Unset()
Expand Down Expand Up @@ -711,7 +711,7 @@ func TestDisableChannel(t *testing.T) {
repoCall1 := grepo.On("ChangeStatus", mock.Anything, mock.Anything).Return(nil)
repoCall2 := grepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(mggroups.Group{}, repoerr.ErrNotFound)
_, err := mgsdk.DisableChannel("wrongID", validToken)
assert.Equal(t, err, errors.NewSDKErrorWithStatus(svcerr.ErrNotFound, http.StatusNotFound), fmt.Sprintf("Disable channel with wrong id: expected %v got %v", svcerr.ErrNotFound, err))
assert.Equal(t, err, errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrNotFound), http.StatusNotFound), fmt.Sprintf("Disable channel with wrong id: expected %v got %v", svcerr.ErrNotFound, err))
ok := repoCall1.Parent.AssertCalled(t, "RetrieveByID", mock.Anything, "wrongID")
assert.True(t, ok, "Memberships was not called on disabling channel with wrong id")
repoCall.Unset()
Expand Down
12 changes: 6 additions & 6 deletions pkg/sdk/go/groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func TestCreateGroup(t *testing.T) {
ParentID: wrongID,
Status: clients.EnabledStatus.String(),
},
err: errors.NewSDKErrorWithStatus(svcerr.ErrCreateEntity, http.StatusUnprocessableEntity),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrCreateEntity, svcerr.ErrCreateEntity), http.StatusUnprocessableEntity),
},
{
desc: "create group with missing name",
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestListGroups(t *testing.T) {
token: invalidToken,
offset: offset,
limit: limit,
err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized),
response: nil,
},
{
Expand Down Expand Up @@ -333,7 +333,7 @@ func TestListParentGroups(t *testing.T) {
token: invalidToken,
offset: offset,
limit: limit,
err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized),
response: nil,
},
{
Expand Down Expand Up @@ -464,7 +464,7 @@ func TestListChildrenGroups(t *testing.T) {
token: invalidToken,
offset: offset,
limit: limit,
err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthentication, http.StatusUnauthorized),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthentication, svcerr.ErrAuthentication), http.StatusUnauthorized),
response: nil,
},
{
Expand Down Expand Up @@ -796,7 +796,7 @@ func TestEnableGroup(t *testing.T) {
repoCall1 := grepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(mggroups.Group{}, repoerr.ErrNotFound)
repoCall2 := grepo.On("ChangeStatus", mock.Anything, mock.Anything).Return(nil)
_, err := mgsdk.EnableGroup("wrongID", validToken)
assert.Equal(t, err, errors.NewSDKErrorWithStatus(svcerr.ErrNotFound, http.StatusNotFound), fmt.Sprintf("Enable group with wrong id: expected %v got %v", svcerr.ErrNotFound, err))
assert.Equal(t, err, errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrNotFound), http.StatusNotFound), fmt.Sprintf("Enable group with wrong id: expected %v got %v", svcerr.ErrNotFound, err))
ok := repoCall1.Parent.AssertCalled(t, "RetrieveByID", mock.Anything, "wrongID")
assert.True(t, ok, "RetrieveByID was not called on enabling group")
repoCall.Unset()
Expand Down Expand Up @@ -849,7 +849,7 @@ func TestDisableGroup(t *testing.T) {
repoCall1 := grepo.On("ChangeStatus", mock.Anything, mock.Anything).Return(nil)
repoCall2 := grepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(mggroups.Group{}, repoerr.ErrNotFound)
_, err := mgsdk.DisableGroup("wrongID", validToken)
assert.Equal(t, err, errors.NewSDKErrorWithStatus(svcerr.ErrNotFound, http.StatusNotFound), fmt.Sprintf("Disable group with wrong id: expected %v got %v", svcerr.ErrNotFound, err))
assert.Equal(t, err, errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrNotFound), http.StatusNotFound), fmt.Sprintf("Disable group with wrong id: expected %v got %v", svcerr.ErrNotFound, err))
ok := repoCall1.Parent.AssertCalled(t, "RetrieveByID", mock.Anything, "wrongID")
assert.True(t, ok, "Memberships was not called on disabling group with wrong id")
repoCall.Unset()
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdk/go/things_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ func TestListThingsByChannel(t *testing.T) {
channelID: wrongID,
page: sdk.PageMetadata{},
response: []sdk.Thing(nil),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrNotFound, svcerr.ErrNotFound), http.StatusNotFound),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity),
},
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/sdk/go/tokens_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/absmach/magistrala"
"github.com/absmach/magistrala/internal/apiutil"
"github.com/absmach/magistrala/pkg/errors"
repoerr "github.com/absmach/magistrala/pkg/errors/repository"
svcerr "github.com/absmach/magistrala/pkg/errors/service"
sdk "github.com/absmach/magistrala/pkg/sdk/go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -69,7 +69,7 @@ func TestIssueToken(t *testing.T) {
login: sdk.Login{Identity: "invalid", Secret: "secret"},
token: &magistrala.Token{},
dbClient: wrongClient,
err: errors.NewSDKErrorWithStatus(errors.Wrap(repoerr.ErrNotFound, repoerr.ErrNotFound), http.StatusNotFound),
err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrViewEntity, svcerr.ErrViewEntity), http.StatusUnprocessableEntity),
},
}
for _, tc := range cases {
Expand Down
Loading

0 comments on commit eb6b201

Please sign in to comment.