Skip to content

Commit

Permalink
feat: deprecate UUID users (#86)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
luthfifahlevi and Muhammad Luthfi Fahlevi authored Nov 26, 2024
1 parent f2972f1 commit 6119ad8
Show file tree
Hide file tree
Showing 47 changed files with 405 additions and 571 deletions.
4 changes: 2 additions & 2 deletions cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ var rootCmd = &cobra.Command{
}

func New(cliConfig *Config) *cobra.Command {
if cliConfig.Client.ServerHeaderKeyUUID == "" {
cliConfig.Client.ServerHeaderKeyUUID = cliConfig.Service.Identity.HeaderKeyUUID
if cliConfig.Client.ServerHeaderKeyEmail == "" {
cliConfig.Client.ServerHeaderKeyEmail = cliConfig.Service.Identity.HeaderValueEmail
}

rootCmd.PersistentPreRunE = func(subCmd *cobra.Command, args []string) error {
Expand Down
5 changes: 2 additions & 3 deletions compass.yaml.example
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ service:
port: 8080
request_timeout: 10s
identity:
headerkey_uuid: Compass-User-UUID
headerkey_email: Compass-User-Email
provider_default_name: shield
grpc:
Expand Down Expand Up @@ -72,8 +71,8 @@ worker:

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

asset:
additional_types:
Expand Down
1 change: 0 additions & 1 deletion core/asset/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ func buildOwners(data interface{}) []user.User {
buildOwner := func(data map[string]interface{}) user.User {
return user.User{
ID: getString("id", data),
UUID: getString("uuid", data),
Email: getString("email", data),
Provider: getString("provider", data),
}
Expand Down
4 changes: 2 additions & 2 deletions core/asset/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ func TestService_DeleteAsset(t *testing.T) {
Err: errors.New("unknown error"),
},
{
Description: `should call DeleteByURN on repositories by fetching URN when given a UUID`,
Description: `should call DeleteByURN on repositories by fetching URN when given an ID`,
ID: assetID,
Setup: func(ctx context.Context, ar *mocks.AssetRepository, dr *mocks.DiscoveryRepository, lr *mocks.LineageRepository) {
ar.EXPECT().GetByID(ctx, assetID).Return(asset.Asset{ID: assetID, URN: urn}, nil)
Expand All @@ -419,7 +419,7 @@ func TestService_DeleteAsset(t *testing.T) {
Err: nil,
},
{
Description: `should call DeleteByURN on repositories when not given a UUID`,
Description: `should call DeleteByURN on repositories when not given an ID`,
ID: urn,
Setup: func(ctx context.Context, ar *mocks.AssetRepository, dr *mocks.DiscoveryRepository, lr *mocks.LineageRepository) {
ar.EXPECT().DeleteByURN(ctx, urn).Return(nil)
Expand Down
2 changes: 1 addition & 1 deletion core/user/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

func TestContext(t *testing.T) {
t.Run("should return passed user if exist in context", func(t *testing.T) {
passedUser := user.User{UUID: "uuid", Email: "email"}
passedUser := user.User{Email: "[email protected]"}
userCtx := user.NewContext(context.Background(), passedUser)
actual := user.FromContext(userCtx)
if !cmp.Equal(passedUser, actual) {
Expand Down
12 changes: 2 additions & 10 deletions core/user/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,33 @@ import (
var ErrNoUserInformation = errors.New("no user information")

type NotFoundError struct {
UUID string
Email string
}

func (e NotFoundError) Error() string {
cause := "could not find user"
if e.UUID != "" {
cause += fmt.Sprintf(" with uuid \"%s\"", e.UUID)
}
if e.Email != "" {
cause += fmt.Sprintf(" with email \"%s\"", e.Email)
}
return cause
}

type DuplicateRecordError struct {
UUID string
Email string
}

func (e DuplicateRecordError) Error() string {
cause := "duplicate user"
if e.UUID != "" {
cause += fmt.Sprintf(" with uuid \"%s\"", e.UUID)
}
if e.Email != "" {
cause += fmt.Sprintf(" with email \"%s\"", e.Email)
}
return cause
}

type InvalidError struct {
UUID string
Email string
}

func (e InvalidError) Error() string {
return fmt.Sprintf("empty field with uuid \"%s\"", e.UUID)
return fmt.Sprintf("empty field with email %q", e.Email)
}
12 changes: 6 additions & 6 deletions core/user/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ func TestErrors(t *testing.T) {
testCases := []testCase{
{
Description: "not found error return correct error string",
Err: user.NotFoundError{UUID: "uuid", Email: "email"},
ExpectedString: "could not find user with uuid \"uuid\" with email \"email\"",
Err: user.NotFoundError{Email: "[email protected]"},
ExpectedString: "could not find user with email \"[email protected]\"",
},
{
Description: "duplicate error return correct error string",
Err: user.DuplicateRecordError{UUID: "uuid", Email: "email"},
ExpectedString: "duplicate user with uuid \"uuid\" with email \"email\"",
Err: user.DuplicateRecordError{Email: "[email protected]"},
ExpectedString: "duplicate user with email \"[email protected]\"",
},
{
Description: "invalid error return correct error string",
Err: user.InvalidError{UUID: "uuid"},
ExpectedString: "empty field with uuid \"uuid\"",
Err: user.InvalidError{Email: "[email protected]"},
ExpectedString: "empty field with email \"[email protected]\"",
},
}

Expand Down
62 changes: 31 additions & 31 deletions core/user/mocks/user_repository.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 7 additions & 21 deletions core/user/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package user

import (
"context"
"errors"

"github.com/goto/salt/log"
)

Expand All @@ -13,32 +11,20 @@ type Service struct {
logger log.Logger
}

// ValidateUser checks if user uuid is already in DB
// ValidateUser checks if user email is already in DB
// if exist in DB, return user ID, if not exist in DB, create a new one
func (s *Service) ValidateUser(ctx context.Context, uuid, email string) (string, error) {
if uuid == "" {
func (s *Service) ValidateUser(ctx context.Context, email string) (string, error) {
if email == "" {
return "", ErrNoUserInformation
}

usr, err := s.repository.GetByUUID(ctx, uuid)
if err == nil {
if usr.ID != "" {
return usr.ID, nil
}
err := errors.New("fetched user uuid from DB is empty")
s.logger.Error(err.Error())
return "", err
}

uid, err := s.repository.UpsertByEmail(ctx, &User{
UUID: uuid,
Email: email,
})
userID, err := s.repository.GetOrInsertByEmail(ctx, &User{Email: email})
if err != nil {
s.logger.Error("error when UpsertByEmail in ValidateUser service", "err", err.Error())
s.logger.Error("error when GetOrInsertByEmail in ValidateUser service", "err", err.Error())
return "", err
}
return uid, nil

return userID, nil
}

// NewService initializes user service
Expand Down
Loading

0 comments on commit 6119ad8

Please sign in to comment.