Skip to content

Commit 7b8c413

Browse files
heiytorgustavosbarreto
heiytor
authored and
gustavosbarreto
committed
refactor: replace specific user getter methods with unified UserResolve
Replace `UserGetByID`, `UserGetByEmail`, and `UserGetByUsername` with a single `UserResolve` method that uses resolver patterns for better consistency and maintainability. - Add `UserResolver` enum with ID, Email, and Username resolvers - Implement `UserResolve` method in store with resolver-based querying - Update all service layers to use `UserResolve` instead of specific getters - Remove deprecated `UserGetByID`, `UserGetByEmail`, `UserGetByUsername` methods - Update all tests to use new `UserResolve` method calls - Fix test mocks to return correct number of values (remove extra int parameter) - Add comprehensive test coverage for UserResolve functionality
1 parent 2800ec7 commit 7b8c413

File tree

17 files changed

+380
-567
lines changed

17 files changed

+380
-567
lines changed

‎api/services/auth.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,15 +195,12 @@ func (s *service) AuthLocalUser(ctx context.Context, req *requests.AuthLocalUser
195195
return nil, 0, "", NewErrAuthMethodNotAllowed(models.UserAuthMethodLocal.String())
196196
}
197197

198-
var err error
199-
var user *models.User
200-
198+
resolver := store.UserUsernameResolver
201199
if req.Identifier.IsEmail() {
202-
user, err = s.store.UserGetByEmail(ctx, strings.ToLower(string(req.Identifier)))
203-
} else {
204-
user, err = s.store.UserGetByUsername(ctx, strings.ToLower(string(req.Identifier)))
200+
resolver = store.UserEmailResolver
205201
}
206202

203+
user, err := s.store.UserResolve(ctx, resolver, strings.ToLower(string(req.Identifier)))
207204
if err != nil {
208205
return nil, 0, "", NewErrAuthUnathorized(nil)
209206
}
@@ -330,7 +327,7 @@ func (s *service) AuthLocalUser(ctx context.Context, req *requests.AuthLocalUser
330327
}
331328

332329
func (s *service) CreateUserToken(ctx context.Context, req *requests.CreateUserToken) (*models.UserAuthResponse, error) {
333-
user, _, err := s.store.UserGetByID(ctx, req.UserID, false)
330+
user, err := s.store.UserResolve(ctx, store.UserIDResolver, req.UserID)
334331
if err != nil {
335332
return nil, NewErrUserNotFound(req.UserID, err)
336333
}

‎api/services/auth_test.go‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ func TestService_AuthLocalUser(t *testing.T) {
858858
).
859859
Once()
860860
mock.
861-
On("UserGetByUsername", ctx, "john_doe").
861+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
862862
Return(nil, store.ErrNoDocuments).
863863
Once()
864864
},
@@ -891,7 +891,7 @@ func TestService_AuthLocalUser(t *testing.T) {
891891
).
892892
Once()
893893
mock.
894-
On("UserGetByEmail", ctx, "[email protected]").
894+
On("UserResolve", ctx, store.UserEmailResolver, "[email protected]").
895895
Return(nil, store.ErrNoDocuments).
896896
Once()
897897
},
@@ -944,7 +944,10 @@ func TestService_AuthLocalUser(t *testing.T) {
944944
},
945945
}
946946

947-
mock.On("UserGetByUsername", ctx, "john_doe").Return(user, nil).Once()
947+
mock.
948+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
949+
Return(user, nil).
950+
Once()
948951
},
949952
expected: Expected{
950953
res: nil,
@@ -995,7 +998,10 @@ func TestService_AuthLocalUser(t *testing.T) {
995998
},
996999
}
9971000

998-
mock.On("UserGetByUsername", ctx, "john_doe").Return(user, nil).Once()
1001+
mock.
1002+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
1003+
Return(user, nil).
1004+
Once()
9991005
},
10001006
expected: Expected{
10011007
res: nil,
@@ -1026,7 +1032,7 @@ func TestService_AuthLocalUser(t *testing.T) {
10261032
).
10271033
Once()
10281034
mock.
1029-
On("UserGetByEmail", ctx, "[email protected]").
1035+
On("UserResolve", ctx, store.UserEmailResolver, "[email protected]").
10301036
Return(
10311037
&models.User{
10321038
ID: "65fdd16b5f62f93184ec8a39",
@@ -1102,7 +1108,7 @@ func TestService_AuthLocalUser(t *testing.T) {
11021108
}
11031109

11041110
mock.
1105-
On("UserGetByUsername", ctx, "john_doe").
1111+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
11061112
Return(user, nil).
11071113
Once()
11081114
cacheMock.
@@ -1160,7 +1166,7 @@ func TestService_AuthLocalUser(t *testing.T) {
11601166
}
11611167

11621168
mock.
1163-
On("UserGetByUsername", ctx, "john_doe").
1169+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
11641170
Return(user, nil).
11651171
Once()
11661172
cacheMock.
@@ -1226,7 +1232,7 @@ func TestService_AuthLocalUser(t *testing.T) {
12261232
}
12271233

12281234
mock.
1229-
On("UserGetByUsername", ctx, "john_doe").
1235+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
12301236
Return(user, nil).
12311237
Once()
12321238
cacheMock.
@@ -1301,7 +1307,7 @@ func TestService_AuthLocalUser(t *testing.T) {
13011307
}
13021308

13031309
mock.
1304-
On("UserGetByUsername", ctx, "john_doe").
1310+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
13051311
Return(user, nil).
13061312
Once()
13071313
cacheMock.
@@ -1405,7 +1411,7 @@ func TestService_AuthLocalUser(t *testing.T) {
14051411
}
14061412

14071413
mock.
1408-
On("UserGetByUsername", ctx, "john_doe").
1414+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
14091415
Return(user, nil).
14101416
Once()
14111417
cacheMock.
@@ -1500,7 +1506,7 @@ func TestService_AuthLocalUser(t *testing.T) {
15001506
}
15011507

15021508
mock.
1503-
On("UserGetByUsername", ctx, "john_doe").
1509+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
15041510
Return(user, nil).
15051511
Once()
15061512
cacheMock.
@@ -1607,7 +1613,7 @@ func TestService_AuthLocalUser(t *testing.T) {
16071613
}
16081614

16091615
mock.
1610-
On("UserGetByUsername", ctx, "john_doe").
1616+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
16111617
Return(user, nil).
16121618
Once()
16131619
cacheMock.
@@ -1715,7 +1721,7 @@ func TestService_AuthLocalUser(t *testing.T) {
17151721
}
17161722

17171723
mock.
1718-
On("UserGetByUsername", ctx, "john_doe").
1724+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
17191725
Return(user, nil).
17201726
Once()
17211727
cacheMock.
@@ -1822,7 +1828,7 @@ func TestService_AuthLocalUser(t *testing.T) {
18221828
}
18231829

18241830
mock.
1825-
On("UserGetByUsername", ctx, "john_doe").
1831+
On("UserResolve", ctx, store.UserUsernameResolver, "john_doe").
18261832
Return(user, nil).
18271833
Once()
18281834
cacheMock.
@@ -1927,8 +1933,8 @@ func TestCreateUserToken(t *testing.T) {
19271933
req: &requests.CreateUserToken{UserID: "000000000000000000000000", TenantID: "00000000-0000-4000-0000-000000000000"},
19281934
requiredMocks: func(ctx context.Context) {
19291935
storeMock.
1930-
On("UserGetByID", ctx, "000000000000000000000000", false).
1931-
Return(nil, 0, store.ErrNoDocuments).
1936+
On("UserResolve", ctx, store.UserIDResolver, "000000000000000000000000").
1937+
Return(nil, store.ErrNoDocuments).
19321938
Once()
19331939
},
19341940
expected: Expected{
@@ -1941,7 +1947,7 @@ func TestCreateUserToken(t *testing.T) {
19411947
req: &requests.CreateUserToken{UserID: "000000000000000000000000", TenantID: "00000000-0000-4000-0000-000000000000"},
19421948
requiredMocks: func(ctx context.Context) {
19431949
storeMock.
1944-
On("UserGetByID", ctx, "000000000000000000000000", false).
1950+
On("UserResolve", ctx, store.UserIDResolver, "000000000000000000000000").
19451951
Return(
19461952
&models.User{
19471953
ID: "000000000000000000000000",
@@ -1962,7 +1968,6 @@ func TestCreateUserToken(t *testing.T) {
19621968
PreferredNamespace: "",
19631969
},
19641970
},
1965-
0,
19661971
nil,
19671972
).
19681973
Once()
@@ -1981,7 +1986,7 @@ func TestCreateUserToken(t *testing.T) {
19811986
req: &requests.CreateUserToken{UserID: "000000000000000000000000", TenantID: "00000000-0000-4000-0000-000000000000"},
19821987
requiredMocks: func(ctx context.Context) {
19831988
storeMock.
1984-
On("UserGetByID", ctx, "000000000000000000000000", false).
1989+
On("UserResolve", ctx, store.UserIDResolver, "000000000000000000000000").
19851990
Return(
19861991
&models.User{
19871992
ID: "000000000000000000000000",
@@ -2002,7 +2007,6 @@ func TestCreateUserToken(t *testing.T) {
20022007
PreferredNamespace: "",
20032008
},
20042009
},
2005-
0,
20062010
nil,
20072011
).
20082012
Once()
@@ -2027,7 +2031,7 @@ func TestCreateUserToken(t *testing.T) {
20272031
req: &requests.CreateUserToken{UserID: "000000000000000000000000", TenantID: "00000000-0000-4000-0000-000000000000"},
20282032
requiredMocks: func(ctx context.Context) {
20292033
storeMock.
2030-
On("UserGetByID", ctx, "000000000000000000000000", false).
2034+
On("UserResolve", ctx, store.UserIDResolver, "000000000000000000000000").
20312035
Return(
20322036
&models.User{
20332037
ID: "000000000000000000000000",
@@ -2048,7 +2052,6 @@ func TestCreateUserToken(t *testing.T) {
20482052
PreferredNamespace: "",
20492053
},
20502054
},
2051-
0,
20522055
nil,
20532056
).
20542057
Once()
@@ -2079,7 +2082,7 @@ func TestCreateUserToken(t *testing.T) {
20792082
req: &requests.CreateUserToken{UserID: "000000000000000000000000", TenantID: "00000000-0000-4000-0000-000000000000"},
20802083
requiredMocks: func(ctx context.Context) {
20812084
storeMock.
2082-
On("UserGetByID", ctx, "000000000000000000000000", false).
2085+
On("UserResolve", ctx, store.UserIDResolver, "000000000000000000000000").
20832086
Return(
20842087
&models.User{
20852088
ID: "000000000000000000000000",
@@ -2100,7 +2103,6 @@ func TestCreateUserToken(t *testing.T) {
21002103
PreferredNamespace: "",
21012104
},
21022105
},
2103-
0,
21042106
nil,
21052107
).
21062108
Once()
@@ -2151,7 +2153,7 @@ func TestCreateUserToken(t *testing.T) {
21512153
req: &requests.CreateUserToken{UserID: "000000000000000000000000", TenantID: ""},
21522154
requiredMocks: func(ctx context.Context) {
21532155
storeMock.
2154-
On("UserGetByID", ctx, "000000000000000000000000", false).
2156+
On("UserResolve", ctx, store.UserIDResolver, "000000000000000000000000").
21552157
Return(
21562158
&models.User{
21572159
ID: "000000000000000000000000",
@@ -2172,7 +2174,6 @@ func TestCreateUserToken(t *testing.T) {
21722174
PreferredNamespace: "00000000-0000-4000-0000-000000000000",
21732175
},
21742176
},
2175-
0,
21762177
nil,
21772178
).
21782179
Once()
@@ -2218,7 +2219,7 @@ func TestCreateUserToken(t *testing.T) {
22182219
req: &requests.CreateUserToken{UserID: "000000000000000000000000", TenantID: ""},
22192220
requiredMocks: func(ctx context.Context) {
22202221
storeMock.
2221-
On("UserGetByID", ctx, "000000000000000000000000", false).
2222+
On("UserResolve", ctx, store.UserIDResolver, "000000000000000000000000").
22222223
Return(
22232224
&models.User{
22242225
ID: "000000000000000000000000",
@@ -2239,7 +2240,6 @@ func TestCreateUserToken(t *testing.T) {
22392240
PreferredNamespace: "",
22402241
},
22412242
},
2242-
0,
22432243
nil,
22442244
).
22452245
Once()

‎api/services/member.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (s *service) AddNamespaceMember(ctx context.Context, req *requests.Namespac
5454
return nil, NewErrNamespaceNotFound(req.TenantID, err)
5555
}
5656

57-
user, _, err := s.store.UserGetByID(ctx, req.UserID, false)
57+
user, err := s.store.UserResolve(ctx, store.UserIDResolver, req.UserID)
5858
if err != nil || user == nil {
5959
return nil, NewErrUserNotFound(req.UserID, err)
6060
}
@@ -72,7 +72,7 @@ func (s *service) AddNamespaceMember(ctx context.Context, req *requests.Namespac
7272
// In cloud instances, if the target user does not exist, we need to create a new user
7373
// with the specified email. We use the inserted ID to identify the user once they complete
7474
// the registration and accepts the invitation.
75-
passiveUser, err := s.store.UserGetByEmail(ctx, strings.ToLower(req.MemberEmail))
75+
passiveUser, err := s.store.UserResolve(ctx, store.UserEmailResolver, strings.ToLower(req.MemberEmail))
7676
if err != nil {
7777
if !envs.IsCloud() || !errors.Is(err, store.ErrNoDocuments) {
7878
return nil, NewErrUserNotFound(req.MemberEmail, err)
@@ -162,7 +162,7 @@ func (s *service) UpdateNamespaceMember(ctx context.Context, req *requests.Names
162162
return NewErrNamespaceNotFound(req.TenantID, err)
163163
}
164164

165-
user, _, err := s.store.UserGetByID(ctx, req.UserID, false)
165+
user, err := s.store.UserResolve(ctx, store.UserIDResolver, req.UserID)
166166
if err != nil {
167167
return NewErrUserNotFound(req.UserID, err)
168168
}
@@ -199,7 +199,7 @@ func (s *service) RemoveNamespaceMember(ctx context.Context, req *requests.Names
199199
return nil, NewErrNamespaceNotFound(req.TenantID, err)
200200
}
201201

202-
user, _, err := s.store.UserGetByID(ctx, req.UserID, false)
202+
user, err := s.store.UserResolve(ctx, store.UserIDResolver, req.UserID)
203203
if err != nil {
204204
return nil, NewErrUserNotFound(req.UserID, err)
205205
}

0 commit comments

Comments
 (0)