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 Failing Users Property Based Tests #2134

Merged
merged 6 commits into from
Apr 3, 2024
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
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
Loading