Skip to content

Commit 6119ad8

Browse files
luthfifahleviMuhammad Luthfi Fahlevi
and
Muhammad Luthfi Fahlevi
authored
feat: deprecate UUID users (#86)
* feat: deprecate uuid * test: fix and add unit tests * feat: add migration sql to drop uuid column in users table * refactor: remove unused method * feat: add id as response, replacement for uuid * test: update case * feat: update the compass yaml example * refactor: remove unused variable and documentation * feat: separate drop uuid users column to another MR --------- Co-authored-by: Muhammad Luthfi Fahlevi <[email protected]>
1 parent f2972f1 commit 6119ad8

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+405
-571
lines changed

cli/root.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ var rootCmd = &cobra.Command{
3333
}
3434

3535
func New(cliConfig *Config) *cobra.Command {
36-
if cliConfig.Client.ServerHeaderKeyUUID == "" {
37-
cliConfig.Client.ServerHeaderKeyUUID = cliConfig.Service.Identity.HeaderKeyUUID
36+
if cliConfig.Client.ServerHeaderKeyEmail == "" {
37+
cliConfig.Client.ServerHeaderKeyEmail = cliConfig.Service.Identity.HeaderValueEmail
3838
}
3939

4040
rootCmd.PersistentPreRunE = func(subCmd *cobra.Command, args []string) error {

compass.yaml.example

+2-3
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ service:
4444
port: 8080
4545
request_timeout: 10s
4646
identity:
47-
headerkey_uuid: Compass-User-UUID
4847
headerkey_email: Compass-User-Email
4948
provider_default_name: shield
5049
grpc:
@@ -72,8 +71,8 @@ worker:
7271

7372
client:
7473
host: localhost:8081
75-
serverheaderkey_uuid: Compass-User-UUID // if ommited, will use value on service.identity.headerkey_uuid
76-
serverheadervalue_uuid: [email protected]
74+
serverheaderkey_email: Compass-User-Email // if ommited, will use value on service.identity.headerkey_email
75+
serverheadervalue_email: [email protected]
7776

7877
asset:
7978
additional_types:

core/asset/patch.go

-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ func buildOwners(data interface{}) []user.User {
5454
buildOwner := func(data map[string]interface{}) user.User {
5555
return user.User{
5656
ID: getString("id", data),
57-
UUID: getString("uuid", data),
5857
Email: getString("email", data),
5958
Provider: getString("provider", data),
6059
}

core/asset/service_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ func TestService_DeleteAsset(t *testing.T) {
408408
Err: errors.New("unknown error"),
409409
},
410410
{
411-
Description: `should call DeleteByURN on repositories by fetching URN when given a UUID`,
411+
Description: `should call DeleteByURN on repositories by fetching URN when given an ID`,
412412
ID: assetID,
413413
Setup: func(ctx context.Context, ar *mocks.AssetRepository, dr *mocks.DiscoveryRepository, lr *mocks.LineageRepository) {
414414
ar.EXPECT().GetByID(ctx, assetID).Return(asset.Asset{ID: assetID, URN: urn}, nil)
@@ -419,7 +419,7 @@ func TestService_DeleteAsset(t *testing.T) {
419419
Err: nil,
420420
},
421421
{
422-
Description: `should call DeleteByURN on repositories when not given a UUID`,
422+
Description: `should call DeleteByURN on repositories when not given an ID`,
423423
ID: urn,
424424
Setup: func(ctx context.Context, ar *mocks.AssetRepository, dr *mocks.DiscoveryRepository, lr *mocks.LineageRepository) {
425425
ar.EXPECT().DeleteByURN(ctx, urn).Return(nil)

core/user/context_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010

1111
func TestContext(t *testing.T) {
1212
t.Run("should return passed user if exist in context", func(t *testing.T) {
13-
passedUser := user.User{UUID: "uuid", Email: "email"}
13+
passedUser := user.User{Email: "[email protected]"}
1414
userCtx := user.NewContext(context.Background(), passedUser)
1515
actual := user.FromContext(userCtx)
1616
if !cmp.Equal(passedUser, actual) {

core/user/errors.go

+2-10
Original file line numberDiff line numberDiff line change
@@ -8,41 +8,33 @@ import (
88
var ErrNoUserInformation = errors.New("no user information")
99

1010
type NotFoundError struct {
11-
UUID string
1211
Email string
1312
}
1413

1514
func (e NotFoundError) Error() string {
1615
cause := "could not find user"
17-
if e.UUID != "" {
18-
cause += fmt.Sprintf(" with uuid \"%s\"", e.UUID)
19-
}
2016
if e.Email != "" {
2117
cause += fmt.Sprintf(" with email \"%s\"", e.Email)
2218
}
2319
return cause
2420
}
2521

2622
type DuplicateRecordError struct {
27-
UUID string
2823
Email string
2924
}
3025

3126
func (e DuplicateRecordError) Error() string {
3227
cause := "duplicate user"
33-
if e.UUID != "" {
34-
cause += fmt.Sprintf(" with uuid \"%s\"", e.UUID)
35-
}
3628
if e.Email != "" {
3729
cause += fmt.Sprintf(" with email \"%s\"", e.Email)
3830
}
3931
return cause
4032
}
4133

4234
type InvalidError struct {
43-
UUID string
35+
Email string
4436
}
4537

4638
func (e InvalidError) Error() string {
47-
return fmt.Sprintf("empty field with uuid \"%s\"", e.UUID)
39+
return fmt.Sprintf("empty field with email %q", e.Email)
4840
}

core/user/errors_test.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,18 @@ func TestErrors(t *testing.T) {
1616
testCases := []testCase{
1717
{
1818
Description: "not found error return correct error string",
19-
Err: user.NotFoundError{UUID: "uuid", Email: "email"},
20-
ExpectedString: "could not find user with uuid \"uuid\" with email \"email\"",
19+
Err: user.NotFoundError{Email: "[email protected]"},
20+
ExpectedString: "could not find user with email \"[email protected]\"",
2121
},
2222
{
2323
Description: "duplicate error return correct error string",
24-
Err: user.DuplicateRecordError{UUID: "uuid", Email: "email"},
25-
ExpectedString: "duplicate user with uuid \"uuid\" with email \"email\"",
24+
Err: user.DuplicateRecordError{Email: "[email protected]"},
25+
ExpectedString: "duplicate user with email \"[email protected]\"",
2626
},
2727
{
2828
Description: "invalid error return correct error string",
29-
Err: user.InvalidError{UUID: "uuid"},
30-
ExpectedString: "empty field with uuid \"uuid\"",
29+
Err: user.InvalidError{Email: "[email protected]"},
30+
ExpectedString: "empty field with email \"[email protected]\"",
3131
},
3232
}
3333

core/user/mocks/user_repository.go

+31-31
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/user/service.go

+7-21
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package user
22

33
import (
44
"context"
5-
"errors"
6-
75
"github.com/goto/salt/log"
86
)
97

@@ -13,32 +11,20 @@ type Service struct {
1311
logger log.Logger
1412
}
1513

16-
// ValidateUser checks if user uuid is already in DB
14+
// ValidateUser checks if user email is already in DB
1715
// if exist in DB, return user ID, if not exist in DB, create a new one
18-
func (s *Service) ValidateUser(ctx context.Context, uuid, email string) (string, error) {
19-
if uuid == "" {
16+
func (s *Service) ValidateUser(ctx context.Context, email string) (string, error) {
17+
if email == "" {
2018
return "", ErrNoUserInformation
2119
}
2220

23-
usr, err := s.repository.GetByUUID(ctx, uuid)
24-
if err == nil {
25-
if usr.ID != "" {
26-
return usr.ID, nil
27-
}
28-
err := errors.New("fetched user uuid from DB is empty")
29-
s.logger.Error(err.Error())
30-
return "", err
31-
}
32-
33-
uid, err := s.repository.UpsertByEmail(ctx, &User{
34-
UUID: uuid,
35-
Email: email,
36-
})
21+
userID, err := s.repository.GetOrInsertByEmail(ctx, &User{Email: email})
3722
if err != nil {
38-
s.logger.Error("error when UpsertByEmail in ValidateUser service", "err", err.Error())
23+
s.logger.Error("error when GetOrInsertByEmail in ValidateUser service", "err", err.Error())
3924
return "", err
4025
}
41-
return uid, nil
26+
27+
return userID, nil
4228
}
4329

4430
// NewService initializes user service

0 commit comments

Comments
 (0)