Skip to content

Commit b95d781

Browse files
heiytorotavio
heiytor
authored and
otavio
committed
refactor(api): extract member operations to separate store interface
- Create new MemberStore interface for namespace membership operations - Add api/store/member.go with MemberStore interface definition - Rename NamespaceAddMember to NamespaceCreateMembership - Rename NamespaceUpdateMember to NamespaceUpdateMembership - Rename NamespaceRemoveMember to NamespaceDeleteMembership - Change NamespaceUpdateMembership to accept *models.Member instead of memberID and *models.MemberChanges - Change NamespaceDeleteMembership to accept *models.Member instead of memberID string
1 parent 9c27cdb commit b95d781

File tree

15 files changed

+485
-408
lines changed

15 files changed

+485
-408
lines changed

‎api/services/member.go‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (s *service) AddNamespaceMember(ctx context.Context, req *requests.Namespac
9797
return nil, NewErrNamespaceMemberDuplicated(passiveUser.ID, nil)
9898
}
9999

100-
if err := s.store.WithTransaction(ctx, s.resendMemberInvite(m.ID, req)); err != nil {
100+
if err := s.store.WithTransaction(ctx, s.resendMemberInvite(m, req)); err != nil {
101101
return nil, err
102102
}
103103
} else {
@@ -127,7 +127,7 @@ func (s *service) addMember(memberID string, req *requests.NamespaceAddMember) s
127127
member.ExpiresAt = time.Time{}
128128
}
129129

130-
if err := s.store.NamespaceAddMember(ctx, req.TenantID, member); err != nil {
130+
if err := s.store.NamespaceCreateMembership(ctx, req.TenantID, member); err != nil {
131131
return err
132132
}
133133

@@ -143,16 +143,16 @@ func (s *service) addMember(memberID string, req *requests.NamespaceAddMember) s
143143

144144
// resendMemberInvite returns a transaction callback that resends an invitation to the member with the
145145
// specified ID.
146-
func (s *service) resendMemberInvite(memberID string, req *requests.NamespaceAddMember) store.TransactionCb {
146+
func (s *service) resendMemberInvite(member *models.Member, req *requests.NamespaceAddMember) store.TransactionCb {
147147
return func(ctx context.Context) error {
148-
expiresAt := clock.Now().Add(7 * (24 * time.Hour))
149-
changes := &models.MemberChanges{ExpiresAt: &expiresAt, Role: req.MemberRole}
148+
member.ExpiresAt = clock.Now().Add(7 * (24 * time.Hour))
149+
member.Role = req.MemberRole
150150

151-
if err := s.store.NamespaceUpdateMember(ctx, req.TenantID, memberID, changes); err != nil {
151+
if err := s.store.NamespaceUpdateMembership(ctx, req.TenantID, member); err != nil {
152152
return err
153153
}
154154

155-
return s.client.InviteMember(ctx, req.TenantID, memberID, req.FowardedHost)
155+
return s.client.InviteMember(ctx, req.TenantID, member.ID, req.FowardedHost)
156156
}
157157
}
158158

@@ -172,19 +172,20 @@ func (s *service) UpdateNamespaceMember(ctx context.Context, req *requests.Names
172172
return NewErrNamespaceMemberNotFound(user.ID, err)
173173
}
174174

175-
if _, ok := namespace.FindMember(req.MemberID); !ok {
175+
member, ok := namespace.FindMember(req.MemberID)
176+
if !ok {
176177
return NewErrNamespaceMemberNotFound(req.MemberID, err)
177178
}
178179

179-
changes := &models.MemberChanges{Role: req.MemberRole}
180-
181-
if changes.Role != authorizer.RoleInvalid {
180+
if req.MemberRole != authorizer.RoleInvalid {
182181
if !active.Role.HasAuthority(req.MemberRole) {
183182
return NewErrRoleInvalid()
184183
}
184+
185+
member.Role = req.MemberRole
185186
}
186187

187-
if err := s.store.NamespaceUpdateMember(ctx, req.TenantID, req.MemberID, changes); err != nil {
188+
if err := s.store.NamespaceUpdateMembership(ctx, req.TenantID, member); err != nil {
188189
return err
189190
}
190191

@@ -218,7 +219,7 @@ func (s *service) RemoveNamespaceMember(ctx context.Context, req *requests.Names
218219
return nil, NewErrRoleInvalid()
219220
}
220221

221-
if err := s.removeMember(ctx, namespace, req.MemberID); err != nil { //nolint:revive
222+
if err := s.removeMember(ctx, namespace, passive); err != nil { //nolint:revive
222223
return nil, err
223224
}
224225

@@ -238,11 +239,12 @@ func (s *service) LeaveNamespace(ctx context.Context, req *requests.LeaveNamespa
238239
return nil, NewErrNamespaceNotFound(req.TenantID, err)
239240
}
240241

241-
if m, ok := ns.FindMember(req.UserID); !ok || m.Role == authorizer.RoleOwner {
242+
member, ok := ns.FindMember(req.UserID)
243+
if !ok || member.Role == authorizer.RoleOwner {
242244
return nil, NewErrAuthForbidden()
243245
}
244246

245-
if err := s.removeMember(ctx, ns, req.UserID); err != nil { //nolint:revive
247+
if err := s.removeMember(ctx, ns, member); err != nil { //nolint:revive
246248
return nil, err
247249
}
248250

@@ -276,13 +278,13 @@ func (s *service) LeaveNamespace(ctx context.Context, req *requests.LeaveNamespa
276278
return s.CreateUserToken(ctx, &requests.CreateUserToken{UserID: req.UserID})
277279
}
278280

279-
func (s *service) removeMember(ctx context.Context, ns *models.Namespace, userID string) error {
280-
if err := s.store.NamespaceRemoveMember(ctx, ns.TenantID, userID); err != nil {
281+
func (s *service) removeMember(ctx context.Context, ns *models.Namespace, member *models.Member) error {
282+
if err := s.store.NamespaceDeleteMembership(ctx, ns.TenantID, member); err != nil {
281283
switch {
282284
case errors.Is(err, store.ErrNoDocuments):
283285
return NewErrNamespaceNotFound(ns.TenantID, err)
284286
case errors.Is(err, mongo.ErrUserNotFound):
285-
return NewErrNamespaceMemberNotFound(userID, err)
287+
return NewErrNamespaceMemberNotFound(member.ID, err)
286288
default:
287289
return err
288290
}

‎api/services/member_test.go‎

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,7 @@ func TestService_addMember(t *testing.T) {
792792
Return("false").
793793
Once()
794794
storeMock.
795-
On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusAccepted, AddedAt: now, ExpiresAt: time.Time{}}).
795+
On("NamespaceCreateMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusAccepted, AddedAt: now, ExpiresAt: time.Time{}}).
796796
Return(errors.New("error")).
797797
Once()
798798
},
@@ -814,7 +814,7 @@ func TestService_addMember(t *testing.T) {
814814
Return("false").
815815
Once()
816816
storeMock.
817-
On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusAccepted, AddedAt: now, ExpiresAt: time.Time{}}).
817+
On("NamespaceCreateMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusAccepted, AddedAt: now, ExpiresAt: time.Time{}}).
818818
Return(nil).
819819
Once()
820820
envMock.
@@ -840,7 +840,7 @@ func TestService_addMember(t *testing.T) {
840840
Return("true").
841841
Once()
842842
storeMock.
843-
On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}).
843+
On("NamespaceCreateMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}).
844844
Return(errors.New("error")).
845845
Once()
846846
},
@@ -862,7 +862,7 @@ func TestService_addMember(t *testing.T) {
862862
Return("true").
863863
Once()
864864
storeMock.
865-
On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}).
865+
On("NamespaceCreateMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}).
866866
Return(nil).
867867
Once()
868868
envMock.
@@ -892,7 +892,7 @@ func TestService_addMember(t *testing.T) {
892892
Return("true").
893893
Once()
894894
storeMock.
895-
On("NamespaceAddMember", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}).
895+
On("NamespaceCreateMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleObserver, Status: models.MemberStatusPending, AddedAt: now, ExpiresAt: now.Add(7 * (24 * time.Hour))}).
896896
Return(nil).
897897
Once()
898898
envMock.
@@ -937,40 +937,62 @@ func TestService_resendMemberInvite(t *testing.T) {
937937

938938
cases := []struct {
939939
description string
940-
memberID string
940+
member *models.Member
941941
req *requests.NamespaceAddMember
942942
requiredMocks func(context.Context)
943943
expected error
944944
}{
945945
{
946946
description: "fails cannot update the member",
947-
memberID: "000000000000000000000000",
947+
member: &models.Member{
948+
ID: "000000000000000000000000",
949+
AddedAt: now.Add(-7 * (24 * time.Hour)),
950+
ExpiresAt: now.Add(-1 * (24 * time.Hour)),
951+
Role: authorizer.RoleAdministrator,
952+
Status: models.MemberStatusPending,
953+
},
948954
req: &requests.NamespaceAddMember{
949955
FowardedHost: "localhost",
950956
TenantID: "00000000-0000-4000-0000-000000000000",
951957
MemberRole: authorizer.RoleObserver,
952958
},
953959
requiredMocks: func(ctx context.Context) {
954-
expiresAt := now.Add(7 * (24 * time.Hour))
955960
storeMock.
956-
On("NamespaceUpdateMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000000", &models.MemberChanges{Role: authorizer.RoleObserver, ExpiresAt: &expiresAt}).
961+
On("NamespaceUpdateMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{
962+
ID: "000000000000000000000000",
963+
AddedAt: now.Add(-7 * (24 * time.Hour)),
964+
ExpiresAt: now.Add(7 * (24 * time.Hour)),
965+
Role: authorizer.RoleObserver,
966+
Status: models.MemberStatusPending,
967+
}).
957968
Return(errors.New("error")).
958969
Once()
959970
},
960971
expected: errors.New("error"),
961972
},
962973
{
963974
description: "fails when cannot send the invite",
964-
memberID: "000000000000000000000000",
975+
member: &models.Member{
976+
ID: "000000000000000000000000",
977+
AddedAt: now.Add(-7 * (24 * time.Hour)),
978+
ExpiresAt: now.Add(-1 * (24 * time.Hour)),
979+
Role: authorizer.RoleAdministrator,
980+
Status: models.MemberStatusPending,
981+
},
965982
req: &requests.NamespaceAddMember{
966983
FowardedHost: "localhost",
967984
TenantID: "00000000-0000-4000-0000-000000000000",
968985
MemberRole: authorizer.RoleObserver,
969986
},
970987
requiredMocks: func(ctx context.Context) {
971-
expiresAt := now.Add(7 * (24 * time.Hour))
972988
storeMock.
973-
On("NamespaceUpdateMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000000", &models.MemberChanges{Role: authorizer.RoleObserver, ExpiresAt: &expiresAt}).
989+
On("NamespaceUpdateMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{
990+
ID: "000000000000000000000000",
991+
AddedAt: now.Add(-7 * (24 * time.Hour)),
992+
ExpiresAt: now.Add(7 * (24 * time.Hour)),
993+
Role: authorizer.RoleObserver,
994+
Status: models.MemberStatusPending,
995+
}).
974996
Return(nil).
975997
Once()
976998
clientMock.
@@ -982,16 +1004,27 @@ func TestService_resendMemberInvite(t *testing.T) {
9821004
},
9831005
{
9841006
description: "succeeds",
985-
memberID: "000000000000000000000000",
1007+
member: &models.Member{
1008+
ID: "000000000000000000000000",
1009+
AddedAt: now.Add(-7 * (24 * time.Hour)),
1010+
ExpiresAt: now.Add(-1 * (24 * time.Hour)),
1011+
Role: authorizer.RoleAdministrator,
1012+
Status: models.MemberStatusPending,
1013+
},
9861014
req: &requests.NamespaceAddMember{
9871015
FowardedHost: "localhost",
9881016
TenantID: "00000000-0000-4000-0000-000000000000",
9891017
MemberRole: authorizer.RoleObserver,
9901018
},
9911019
requiredMocks: func(ctx context.Context) {
992-
expiresAt := now.Add(7 * (24 * time.Hour))
9931020
storeMock.
994-
On("NamespaceUpdateMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000000", &models.MemberChanges{Role: authorizer.RoleObserver, ExpiresAt: &expiresAt}).
1021+
On("NamespaceUpdateMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{
1022+
ID: "000000000000000000000000",
1023+
AddedAt: now.Add(-7 * (24 * time.Hour)),
1024+
ExpiresAt: now.Add(7 * (24 * time.Hour)),
1025+
Role: authorizer.RoleObserver,
1026+
Status: models.MemberStatusPending,
1027+
}).
9951028
Return(nil).
9961029
Once()
9971030
clientMock.
@@ -1010,12 +1043,11 @@ func TestService_resendMemberInvite(t *testing.T) {
10101043
ctx := context.Background()
10111044
tc.requiredMocks(ctx)
10121045

1013-
cb := s.resendMemberInvite(tc.memberID, tc.req)
1046+
cb := s.resendMemberInvite(tc.member, tc.req)
10141047
assert.Equal(tt, tc.expected, cb(ctx))
10151048

1016-
envMock.AssertExpectations(tt)
10171049
storeMock.AssertExpectations(tt)
1018-
clockMock.AssertExpectations(tt)
1050+
envMock.AssertExpectations(tt)
10191051
})
10201052
}
10211053
}
@@ -1240,7 +1272,7 @@ func TestUpdateNamespaceMember(t *testing.T) {
12401272
}, nil).
12411273
Once()
12421274
storeMock.
1243-
On("NamespaceUpdateMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000001", &models.MemberChanges{Role: authorizer.RoleAdministrator}).
1275+
On("NamespaceUpdateMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000001", Role: authorizer.RoleAdministrator}).
12441276
Return(nil).
12451277
Once()
12461278
},
@@ -1458,7 +1490,7 @@ func TestRemoveNamespaceMember(t *testing.T) {
14581490
}, nil).
14591491
Once()
14601492
storeMock.
1461-
On("NamespaceRemoveMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000001").
1493+
On("NamespaceDeleteMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000001", Role: authorizer.RoleAdministrator}).
14621494
Return(errors.New("error")).
14631495
Once()
14641496
},
@@ -1501,7 +1533,7 @@ func TestRemoveNamespaceMember(t *testing.T) {
15011533
}, nil).
15021534
Once()
15031535
storeMock.
1504-
On("NamespaceRemoveMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000001").
1536+
On("NamespaceDeleteMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000001", Role: authorizer.RoleAdministrator}).
15051537
Return(nil).
15061538
Once()
15071539
storeMock.
@@ -1657,7 +1689,7 @@ func TestService_LeaveNamespace(t *testing.T) {
16571689
}, nil).
16581690
Once()
16591691
storeMock.
1660-
On("NamespaceRemoveMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000000").
1692+
On("NamespaceDeleteMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleAdministrator}).
16611693
Return(errors.New("error")).
16621694
Once()
16631695
},
@@ -1689,7 +1721,7 @@ func TestService_LeaveNamespace(t *testing.T) {
16891721
}, nil).
16901722
Once()
16911723
storeMock.
1692-
On("NamespaceRemoveMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000000").
1724+
On("NamespaceDeleteMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleAdministrator}).
16931725
Return(nil).
16941726
Once()
16951727
},
@@ -1764,7 +1796,7 @@ func TestService_LeaveNamespace(t *testing.T) {
17641796
}, nil).
17651797
Once()
17661798
storeMock.
1767-
On("NamespaceRemoveMember", ctx, "00000000-0000-4000-0000-000000000000", "000000000000000000000000").
1799+
On("NamespaceDeleteMembership", ctx, "00000000-0000-4000-0000-000000000000", &models.Member{ID: "000000000000000000000000", Role: authorizer.RoleAdministrator}).
17681800
Return(nil).
17691801
Once()
17701802
storeMock.

‎api/store/member.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package store
2+
3+
import (
4+
"context"
5+
6+
"github.com/shellhub-io/shellhub/pkg/models"
7+
)
8+
9+
type MemberStore interface {
10+
NamespaceCreateMembership(ctx context.Context, tenantID string, member *models.Member) error
11+
NamespaceUpdateMembership(ctx context.Context, tenantID string, member *models.Member) error
12+
NamespaceDeleteMembership(ctx context.Context, tenantID string, member *models.Member) error
13+
}

0 commit comments

Comments
 (0)