From 6119ad8975623503d498401b798189c010acc50b Mon Sep 17 00:00:00 2001 From: luthfifahlevi <77453015+luthfifahlevi@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:22:23 +0700 Subject: [PATCH] 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 --- cli/root.go | 4 +- compass.yaml.example | 5 +- core/asset/patch.go | 1 - core/asset/service_test.go | 4 +- core/user/context_test.go | 2 +- core/user/errors.go | 12 +- core/user/errors_test.go | 12 +- core/user/mocks/user_repository.go | 62 ++++---- core/user/service.go | 28 +--- core/user/service_test.go | 60 ++------ core/user/user.go | 11 +- core/user/user_test.go | 4 +- docs/docs/configuration.md | 6 +- docs/docs/reference/configuration.md | 4 - internal/client/client.go | 8 +- internal/server/server.go | 7 +- internal/server/v1beta1/asset.go | 8 - internal/server/v1beta1/asset_test.go | 124 ++++++++-------- internal/server/v1beta1/comment_test.go | 30 ++-- internal/server/v1beta1/discussion_test.go | 28 ++-- internal/server/v1beta1/lineage_test.go | 8 +- internal/server/v1beta1/mocks/user_service.go | 29 ++-- internal/server/v1beta1/search_test.go | 24 +-- internal/server/v1beta1/server.go | 4 +- internal/server/v1beta1/server_test.go | 16 +- internal/server/v1beta1/tag_template_test.go | 32 ++-- internal/server/v1beta1/tag_test.go | 34 ++--- internal/server/v1beta1/type_test.go | 8 +- internal/server/v1beta1/user.go | 6 +- internal/server/v1beta1/user_test.go | 82 +++++------ internal/store/postgres/asset_repository.go | 10 +- .../store/postgres/asset_repository_test.go | 14 +- .../postgres/discussion_comment_repository.go | 3 +- .../discussion_comment_repository_test.go | 2 +- .../store/postgres/discussion_repository.go | 1 - ...add_constraint_email_users_unique.down.sql | 11 ++ ...0_add_constraint_email_users_unique.up.sql | 11 ++ internal/store/postgres/postgres_test.go | 4 +- internal/store/postgres/star_repository.go | 3 - internal/store/postgres/user_model.go | 5 - internal/store/postgres/user_model_test.go | 8 +- internal/store/postgres/user_repository.go | 67 ++++----- .../store/postgres/user_repository_test.go | 138 ++++-------------- pkg/grpc_interceptor/interceptor_test.go | 4 +- pkg/grpc_interceptor/user.go | 19 +-- pkg/grpc_interceptor/user_test.go | 11 +- test/e2e_test/helper_test.go | 2 - 47 files changed, 405 insertions(+), 571 deletions(-) create mode 100644 internal/store/postgres/migrations/000020_add_constraint_email_users_unique.down.sql create mode 100644 internal/store/postgres/migrations/000020_add_constraint_email_users_unique.up.sql diff --git a/cli/root.go b/cli/root.go index 56e0f366..b6342ac1 100644 --- a/cli/root.go +++ b/cli/root.go @@ -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 { diff --git a/compass.yaml.example b/compass.yaml.example index bcd99f44..d12c1e2d 100644 --- a/compass.yaml.example +++ b/compass.yaml.example @@ -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: @@ -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: gotocompany@email.com + serverheaderkey_email: Compass-User-Email // if ommited, will use value on service.identity.headerkey_email + serverheadervalue_email: gotocompany@email.com asset: additional_types: diff --git a/core/asset/patch.go b/core/asset/patch.go index f7516605..5ec3fe37 100644 --- a/core/asset/patch.go +++ b/core/asset/patch.go @@ -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), } diff --git a/core/asset/service_test.go b/core/asset/service_test.go index 7145bb7f..67d8880d 100644 --- a/core/asset/service_test.go +++ b/core/asset/service_test.go @@ -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) @@ -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) diff --git a/core/user/context_test.go b/core/user/context_test.go index 07332376..fc42fd6b 100644 --- a/core/user/context_test.go +++ b/core/user/context_test.go @@ -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: "test@test.com"} userCtx := user.NewContext(context.Background(), passedUser) actual := user.FromContext(userCtx) if !cmp.Equal(passedUser, actual) { diff --git a/core/user/errors.go b/core/user/errors.go index ff56731e..aaa6464b 100644 --- a/core/user/errors.go +++ b/core/user/errors.go @@ -8,15 +8,11 @@ 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) } @@ -24,15 +20,11 @@ func (e NotFoundError) Error() string { } 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) } @@ -40,9 +32,9 @@ func (e DuplicateRecordError) Error() string { } 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) } diff --git a/core/user/errors_test.go b/core/user/errors_test.go index 63fd992b..45ff6b5c 100644 --- a/core/user/errors_test.go +++ b/core/user/errors_test.go @@ -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: "test@test.com"}, + ExpectedString: "could not find user with email \"test@test.com\"", }, { 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: "test@test.com"}, + ExpectedString: "duplicate user with email \"test@test.com\"", }, { Description: "invalid error return correct error string", - Err: user.InvalidError{UUID: "uuid"}, - ExpectedString: "empty field with uuid \"uuid\"", + Err: user.InvalidError{Email: "test@test.com"}, + ExpectedString: "empty field with email \"test@test.com\"", }, } diff --git a/core/user/mocks/user_repository.go b/core/user/mocks/user_repository.go index e0d78746..cb8072e4 100644 --- a/core/user/mocks/user_repository.go +++ b/core/user/mocks/user_repository.go @@ -128,23 +128,23 @@ func (_c *UserRepository_GetByEmail_Call) RunAndReturn(run func(context.Context, return _c } -// GetByUUID provides a mock function with given fields: ctx, uuid -func (_m *UserRepository) GetByUUID(ctx context.Context, uuid string) (user.User, error) { - ret := _m.Called(ctx, uuid) +// GetOrInsertByEmail provides a mock function with given fields: ctx, u +func (_m *UserRepository) GetOrInsertByEmail(ctx context.Context, u *user.User) (string, error) { + ret := _m.Called(ctx, u) - var r0 user.User + var r0 string var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (user.User, error)); ok { - return rf(ctx, uuid) + if rf, ok := ret.Get(0).(func(context.Context, *user.User) (string, error)); ok { + return rf(ctx, u) } - if rf, ok := ret.Get(0).(func(context.Context, string) user.User); ok { - r0 = rf(ctx, uuid) + if rf, ok := ret.Get(0).(func(context.Context, *user.User) string); ok { + r0 = rf(ctx, u) } else { - r0 = ret.Get(0).(user.User) + r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = rf(ctx, uuid) + if rf, ok := ret.Get(1).(func(context.Context, *user.User) error); ok { + r1 = rf(ctx, u) } else { r1 = ret.Error(1) } @@ -152,37 +152,37 @@ func (_m *UserRepository) GetByUUID(ctx context.Context, uuid string) (user.User return r0, r1 } -// UserRepository_GetByUUID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetByUUID' -type UserRepository_GetByUUID_Call struct { +// UserRepository_GetOrInsertByEmail_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetOrInsertByEmail' +type UserRepository_GetOrInsertByEmail_Call struct { *mock.Call } -// GetByUUID is a helper method to define mock.On call +// GetOrInsertByEmail is a helper method to define mock.On call // - ctx context.Context -// - uuid string -func (_e *UserRepository_Expecter) GetByUUID(ctx interface{}, uuid interface{}) *UserRepository_GetByUUID_Call { - return &UserRepository_GetByUUID_Call{Call: _e.mock.On("GetByUUID", ctx, uuid)} +// - u *user.User +func (_e *UserRepository_Expecter) GetOrInsertByEmail(ctx interface{}, u interface{}) *UserRepository_GetOrInsertByEmail_Call { + return &UserRepository_GetOrInsertByEmail_Call{Call: _e.mock.On("GetOrInsertByEmail", ctx, u)} } -func (_c *UserRepository_GetByUUID_Call) Run(run func(ctx context.Context, uuid string)) *UserRepository_GetByUUID_Call { +func (_c *UserRepository_GetOrInsertByEmail_Call) Run(run func(ctx context.Context, u *user.User)) *UserRepository_GetOrInsertByEmail_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) + run(args[0].(context.Context), args[1].(*user.User)) }) return _c } -func (_c *UserRepository_GetByUUID_Call) Return(_a0 user.User, _a1 error) *UserRepository_GetByUUID_Call { +func (_c *UserRepository_GetOrInsertByEmail_Call) Return(_a0 string, _a1 error) *UserRepository_GetOrInsertByEmail_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *UserRepository_GetByUUID_Call) RunAndReturn(run func(context.Context, string) (user.User, error)) *UserRepository_GetByUUID_Call { +func (_c *UserRepository_GetOrInsertByEmail_Call) RunAndReturn(run func(context.Context, *user.User) (string, error)) *UserRepository_GetOrInsertByEmail_Call { _c.Call.Return(run) return _c } -// UpsertByEmail provides a mock function with given fields: ctx, u -func (_m *UserRepository) UpsertByEmail(ctx context.Context, u *user.User) (string, error) { +// InsertByEmail provides a mock function with given fields: ctx, u +func (_m *UserRepository) InsertByEmail(ctx context.Context, u *user.User) (string, error) { ret := _m.Called(ctx, u) var r0 string @@ -205,31 +205,31 @@ func (_m *UserRepository) UpsertByEmail(ctx context.Context, u *user.User) (stri return r0, r1 } -// UserRepository_UpsertByEmail_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'UpsertByEmail' -type UserRepository_UpsertByEmail_Call struct { +// UserRepository_InsertByEmail_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'InsertByEmail' +type UserRepository_InsertByEmail_Call struct { *mock.Call } -// UpsertByEmail is a helper method to define mock.On call +// InsertByEmail is a helper method to define mock.On call // - ctx context.Context // - u *user.User -func (_e *UserRepository_Expecter) UpsertByEmail(ctx interface{}, u interface{}) *UserRepository_UpsertByEmail_Call { - return &UserRepository_UpsertByEmail_Call{Call: _e.mock.On("UpsertByEmail", ctx, u)} +func (_e *UserRepository_Expecter) InsertByEmail(ctx interface{}, u interface{}) *UserRepository_InsertByEmail_Call { + return &UserRepository_InsertByEmail_Call{Call: _e.mock.On("InsertByEmail", ctx, u)} } -func (_c *UserRepository_UpsertByEmail_Call) Run(run func(ctx context.Context, u *user.User)) *UserRepository_UpsertByEmail_Call { +func (_c *UserRepository_InsertByEmail_Call) Run(run func(ctx context.Context, u *user.User)) *UserRepository_InsertByEmail_Call { _c.Call.Run(func(args mock.Arguments) { run(args[0].(context.Context), args[1].(*user.User)) }) return _c } -func (_c *UserRepository_UpsertByEmail_Call) Return(_a0 string, _a1 error) *UserRepository_UpsertByEmail_Call { +func (_c *UserRepository_InsertByEmail_Call) Return(_a0 string, _a1 error) *UserRepository_InsertByEmail_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *UserRepository_UpsertByEmail_Call) RunAndReturn(run func(context.Context, *user.User) (string, error)) *UserRepository_UpsertByEmail_Call { +func (_c *UserRepository_InsertByEmail_Call) RunAndReturn(run func(context.Context, *user.User) (string, error)) *UserRepository_InsertByEmail_Call { _c.Call.Return(run) return _c } diff --git a/core/user/service.go b/core/user/service.go index a8f3b281..8d98f3f1 100644 --- a/core/user/service.go +++ b/core/user/service.go @@ -2,8 +2,6 @@ package user import ( "context" - "errors" - "github.com/goto/salt/log" ) @@ -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 diff --git a/core/user/service_test.go b/core/user/service_test.go index 47aee970..1a74326d 100644 --- a/core/user/service_test.go +++ b/core/user/service_test.go @@ -12,65 +12,35 @@ import ( ) func TestValidateUser(t *testing.T) { + userID := "user-id" type testCase struct { Description string - UUID string Email string - Setup func(ctx context.Context, inputUUID, inputEmail string, userRepo *mocks.UserRepository) + Setup func(ctx context.Context, inputEmail string, userRepo *mocks.UserRepository) ExpectErr error } testCases := []testCase{ { - Description: "should return no user error when uuid is empty and email is optional", - UUID: "", + Description: "should return no user error when email is empty", ExpectErr: user.ErrNoUserInformation, }, { - Description: "should return user UUID when successfully found user UUID from DB", - UUID: "a-uuid", - Setup: func(ctx context.Context, inputUUID, inputEmail string, userRepo *mocks.UserRepository) { - userRepo.EXPECT().GetByUUID(ctx, inputUUID).Return(user.User{ID: "user-id", UUID: inputUUID}, nil) + Description: "should return user ID when successfully found user Email from DB, or not found but can create the new one", + Email: "test@test.com", + Setup: func(ctx context.Context, inputEmail string, userRepo *mocks.UserRepository) { + userRepo.EXPECT().GetOrInsertByEmail(ctx, &user.User{Email: inputEmail}).Return(userID, nil) }, ExpectErr: nil, }, { - Description: "should return user error if GetByUUID return nil error and empty ID", - UUID: "a-uuid", - Setup: func(ctx context.Context, inputUUID, inputEmail string, userRepo *mocks.UserRepository) { - userRepo.EXPECT().GetByUUID(ctx, inputUUID).Return(user.User{}, nil) + Description: "should return user error if InsertByEmail return empty ID and error", + Email: "test@test.com", + Setup: func(ctx context.Context, inputEmail string, userRepo *mocks.UserRepository) { + mockErr := errors.New("error get or insert user") + userRepo.EXPECT().GetOrInsertByEmail(ctx, &user.User{Email: inputEmail}).Return("", mockErr) }, - ExpectErr: errors.New("fetched user uuid from DB is empty"), - }, - { - Description: "should return user UUID when not found user UUID from DB but can create the new one without email", - UUID: "an-email-success-create", - Setup: func(ctx context.Context, inputUUID, inputEmail string, userRepo *mocks.UserRepository) { - userRepo.EXPECT().GetByUUID(ctx, inputUUID).Return(user.User{}, user.NotFoundError{UUID: inputUUID}) - userRepo.EXPECT().UpsertByEmail(ctx, &user.User{UUID: inputUUID}).Return("some-id", nil) - }, - ExpectErr: nil, - }, - { - Description: "should return user UUID when not found user UUID from DB but can create the new one wit email", - UUID: "an-uuid-error", - Email: "an-email-success-create", - Setup: func(ctx context.Context, inputUUID, inputEmail string, userRepo *mocks.UserRepository) { - userRepo.EXPECT().GetByUUID(ctx, inputUUID).Return(user.User{}, user.NotFoundError{UUID: inputUUID}) - userRepo.EXPECT().UpsertByEmail(ctx, &user.User{UUID: inputUUID, Email: inputEmail}).Return("some-id", nil) - }, - ExpectErr: nil, - }, - { - Description: "should return error when not found user ID from DB but can't create the new one", - UUID: "an-uuid-error", - Email: "an-email", - Setup: func(ctx context.Context, inputUUID, inputEmail string, userRepo *mocks.UserRepository) { - mockErr := errors.New("error upserting user") - userRepo.EXPECT().GetByUUID(ctx, inputUUID).Return(user.User{}, mockErr) - userRepo.EXPECT().UpsertByEmail(ctx, &user.User{UUID: inputUUID, Email: inputEmail}).Return("", mockErr) - }, - ExpectErr: errors.New("error upserting user"), + ExpectErr: errors.New("error get or insert user"), }, } @@ -81,12 +51,12 @@ func TestValidateUser(t *testing.T) { mockUserRepo := new(mocks.UserRepository) if tc.Setup != nil { - tc.Setup(ctx, tc.UUID, tc.Email, mockUserRepo) + tc.Setup(ctx, tc.Email, mockUserRepo) } userSvc := user.NewService(logger, mockUserRepo) - _, err := userSvc.ValidateUser(ctx, tc.UUID, tc.Email) + _, err := userSvc.ValidateUser(ctx, tc.Email) assert.Equal(t, tc.ExpectErr, err) }) diff --git a/core/user/user.go b/core/user/user.go index 8430665f..45866fc6 100644 --- a/core/user/user.go +++ b/core/user/user.go @@ -8,8 +8,7 @@ import ( // User is a basic entity of a user type User struct { - ID string `json:"-" diff:"-" db:"id"` - UUID string `json:"uuid,omitempty" diff:"-" db:"uuid"` + ID string `json:"id" diff:"-" db:"id"` Email string `json:"email" diff:"email" db:"email"` Provider string `json:"provider" diff:"-" db:"provider"` CreatedAt time.Time `json:"-" diff:"-" db:"created_at"` @@ -22,8 +21,8 @@ func (u *User) Validate() error { return ErrNoUserInformation } - if u.UUID == "" { - return InvalidError{UUID: u.UUID} + if u.Email == "" { + return InvalidError{Email: u.Email} } return nil @@ -33,6 +32,6 @@ func (u *User) Validate() error { type Repository interface { Create(ctx context.Context, u *User) (string, error) GetByEmail(ctx context.Context, email string) (User, error) - GetByUUID(ctx context.Context, uuid string) (User, error) - UpsertByEmail(ctx context.Context, u *User) (string, error) + InsertByEmail(ctx context.Context, u *User) (string, error) + GetOrInsertByEmail(ctx context.Context, u *User) (string, error) } diff --git a/core/user/user_test.go b/core/user/user_test.go index 45c84c29..adcf9d12 100644 --- a/core/user/user_test.go +++ b/core/user/user_test.go @@ -20,13 +20,13 @@ func TestValidate(t *testing.T) { ExpectError: ErrNoUserInformation, }, { - Title: "should return error invalid if uuid is empty", + Title: "should return error invalid if email is empty", User: &User{Provider: "provider"}, ExpectError: InvalidError{}, }, { Title: "should return nil if user is valid", - User: &User{UUID: "some-uuid", Provider: "provider"}, + User: &User{Email: "test@test.com", Provider: "provider"}, ExpectError: nil, }, } diff --git a/docs/docs/configuration.md b/docs/docs/configuration.md index 09edbdd1..e550aad4 100644 --- a/docs/docs/configuration.md +++ b/docs/docs/configuration.md @@ -51,7 +51,6 @@ service: host: localhost #required port: 8080 #required identity: - headerkey_uuid: Compass-User-UUID #required headerkey_email: Compass-User-Email #optional provider_default_name: shield #optional grpc: @@ -76,7 +75,6 @@ DB_PASSWORD=compass_password DB_SSLMODE=disable SERVICE_HOST=localhost SERVICE_PORT=8080 -SERVICE_IDENTITY_HEADERKEY_UUID=Compass-User-UUID SERVICE_IDENTITY_HEADERKEY_EMAIL=Compass-User-Email SERVICE_IDENTITY_PROVIDER_DEFAULT_NAME=shield SERVICE_GRPC_PORT=8081 @@ -262,8 +260,8 @@ Add client configurations in the same `~/compass.yaml` file in root of current d ```yml client: host: localhost:8081 - serverheaderkey_uuid: Compass-User-UUID - serverheadervalue_uuid: john.doe@example.com + serverheaderkey_email: Compass-User-Email + serverheadervalue_email: john.doe@example.com ``` #### Required Header/Metadata in API diff --git a/docs/docs/reference/configuration.md b/docs/docs/reference/configuration.md index a7e2d9f3..a3db96cb 100644 --- a/docs/docs/reference/configuration.md +++ b/docs/docs/reference/configuration.md @@ -57,10 +57,6 @@ Compass's required variables to start using it. * Example value: `disable` * Type: `optional` * PostgreSQL DB SSL mode to connect. -### `IDENTITY_UUID_HEADER` -* Example value: `Compass-User-UUID` -* Type: `required` -* Header key to accept Compass User UUID. See [User](../concepts/user.md) for more information about the usage. ### `IDENTITY_EMAIL_HEADER` * Example value: `Compass-User-Email` * Type: `optional` diff --git a/internal/client/client.go b/internal/client/client.go index debe6cba..2a2d15fb 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -12,9 +12,9 @@ import ( ) type Config struct { - Host string `mapstructure:"host" default:"localhost:8081"` - ServerHeaderKeyUUID string `yaml:"serverheaderkey_uuid" mapstructure:"serverheaderkey_uuid" default:"Compass-User-UUID"` - ServerHeaderValueUUID string `yaml:"serverheadervalue_uuid" mapstructure:"serverheadervalue_uuid" default:"compass@gotocompany.com"` + Host string `mapstructure:"host" default:"localhost:8081"` + ServerHeaderKeyEmail string `yaml:"serverheaderkey_email" mapstructure:"serverheaderkey_email" default:"Compass-User-Email"` + ServerHeaderValueEmail string `yaml:"serverheadervalue_email" mapstructure:"serverheadervalue_email" default:"compass@gotocompany.com"` } func Create(ctx context.Context, cfg Config) (compassv1beta1.CompassServiceClient, func(), error) { @@ -35,7 +35,7 @@ func Create(ctx context.Context, cfg Config) (compassv1beta1.CompassServiceClien } func SetMetadata(ctx context.Context, cfg Config) context.Context { - md := metadata.New(map[string]string{cfg.ServerHeaderKeyUUID: cfg.ServerHeaderValueUUID}) + md := metadata.New(map[string]string{cfg.ServerHeaderKeyEmail: cfg.ServerHeaderValueEmail}) ctx = metadata.NewOutgoingContext(ctx, md) return ctx diff --git a/internal/server/server.go b/internal/server/server.go index 380faecc..b27b77c5 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -47,9 +47,8 @@ func (cfg Config) grpcAddr() string { return fmt.Sprintf("%s:%d", cfg.Host, cfg. type IdentityConfig struct { // User Identity - HeaderKeyUUID string `yaml:"headerkey_uuid" mapstructure:"headerkey_uuid" default:"Compass-User-UUID"` - HeaderValueUUID string `yaml:"headervalue_uuid" mapstructure:"headervalue_uuid" default:"gotocompany@email.com"` HeaderKeyEmail string `yaml:"headerkey_email" mapstructure:"headerkey_email" default:"Compass-User-Email"` + HeaderValueEmail string `yaml:"headervalue_email" mapstructure:"headervalue_email" default:"gotocompany@email.com"` ProviderDefaultName string `yaml:"provider_default_name" mapstructure:"provider_default_name" default:""` } @@ -91,7 +90,7 @@ func Serve( grpclogrus.UnaryServerInterceptor(logger.Entry()), otelgrpc.UnaryServerInterceptor(), nrgrpc.UnaryServerInterceptor(nrApp), - grpc_interceptor.UserHeaderCtx(config.Identity.HeaderKeyUUID, config.Identity.HeaderKeyEmail), + grpc_interceptor.UserHeaderCtx(config.Identity.HeaderKeyEmail), grpcctxtags.UnaryServerInterceptor(), grpcrecovery.UnaryServerInterceptor(), )), @@ -175,8 +174,6 @@ func Serve( func makeHeaderMatcher(c Config) func(key string) (string, bool) { return func(key string) (string, bool) { switch strings.ToLower(key) { - case strings.ToLower(c.Identity.HeaderKeyUUID): - return key, true case strings.ToLower(c.Identity.HeaderKeyEmail): return key, true default: diff --git a/internal/server/v1beta1/asset.go b/internal/server/v1beta1/asset.go index 5be91565..d5808822 100644 --- a/internal/server/v1beta1/asset.go +++ b/internal/server/v1beta1/asset.go @@ -466,7 +466,6 @@ func (server *APIServer) buildAsset(baseAsset *compassv1beta1.UpsertAssetRequest for _, owner := range dedupe(baseAsset.GetOwners()) { owners = append(owners, user.User{ ID: owner.Id, - UUID: owner.Uuid, Email: owner.Email, Provider: owner.Provider, }) @@ -572,24 +571,17 @@ func dedupe(owners []*compassv1beta1.User) []*compassv1beta1.User { n := len(owners) uniq := make([]*compassv1beta1.User, 0, n) ids := make(map[string]struct{}, n) - uuids := make(map[string]struct{}, n) emails := make(map[string]struct{}, n) for _, o := range owners { if _, ok := ids[o.Id]; ok { continue } - if _, ok := uuids[o.Uuid]; ok { - continue - } if _, ok := emails[o.Email]; ok { continue } if o.Id != "" { ids[o.Id] = struct{}{} } - if o.Uuid != "" { - uuids[o.Uuid] = struct{}{} - } if o.Email != "" { emails[o.Email] = struct{}{} } diff --git a/internal/server/v1beta1/asset_test.go b/internal/server/v1beta1/asset_test.go index 516e3a91..6bda3606 100644 --- a/internal/server/v1beta1/asset_test.go +++ b/internal/server/v1beta1/asset_test.go @@ -30,8 +30,8 @@ import ( func TestGetAllAssets(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = uuid.NewString() ) type testCase struct { Description string @@ -47,7 +47,7 @@ func TestGetAllAssets(t *testing.T) { ExpectStatus: codes.Internal, Request: &compassv1beta1.GetAllAssetsRequest{}, Setup: func(ctx context.Context, as *mocks.AssetService, us *mocks.UserService) { - us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("", errors.New("some-error")) + us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string")).Return("", errors.New("some-error")) }, }, { @@ -187,7 +187,7 @@ func TestGetAllAssets(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -198,7 +198,7 @@ func TestGetAllAssets(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockAssetSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -220,11 +220,11 @@ func TestGetAllAssets(t *testing.T) { func TestGetAssetByID(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() - assetID = uuid.NewString() - now = time.Now() - ast = asset.Asset{ + userID = uuid.NewString() + userEmail = "test@test.com" + assetID = uuid.NewString() + now = time.Now() + ast = asset.Asset{ ID: assetID, Owners: []user.User{{Email: "dummy@trash.com"}}, Probes: []asset.Probe{ @@ -263,7 +263,7 @@ func TestGetAssetByID(t *testing.T) { Description: `should return error if user validation in ctx fails`, ExpectStatus: codes.Internal, Setup: func(ctx context.Context, as *mocks.AssetService, us *mocks.UserService) { - us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("", errors.New("some-error")) + us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string")).Return("", errors.New("some-error")) }, }, { @@ -328,7 +328,7 @@ func TestGetAssetByID(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := mocks.NewUserService(t) @@ -337,7 +337,7 @@ func TestGetAssetByID(t *testing.T) { tc.Setup(ctx, mockAssetSvc, mockUserSvc) } - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -360,7 +360,7 @@ func TestGetAssetByID(t *testing.T) { func TestUpsertAsset(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = uuid.NewString() assetID = uuid.NewString() validPayload = &compassv1beta1.UpsertAssetRequest{ Asset: &compassv1beta1.UpsertAssetRequest_Asset{ @@ -371,10 +371,9 @@ func TestUpsertAsset(t *testing.T) { Data: &structpb.Struct{}, Url: "https://sample-url.com", Owners: []*compassv1beta1.User{ - {Id: "id", Uuid: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}, + {Id: "id", Email: "email@email.com", Provider: "provider"}, // the following users should get de-duplicated. {Id: "id"}, - {Uuid: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8"}, {Email: "email@email.com"}, }, }, @@ -413,7 +412,7 @@ func TestUpsertAsset(t *testing.T) { ExpectStatus: codes.Internal, Request: &compassv1beta1.UpsertAssetRequest{}, Setup: func(ctx context.Context, _ *mocks.AssetService, us *mocks.UserService) { - us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("", errors.New("some-error")) + us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string")).Return("", errors.New("some-error")) }, }, { @@ -503,7 +502,7 @@ func TestUpsertAsset(t *testing.T) { UpdatedBy: user.User{ID: userID}, Data: map[string]interface{}{}, URL: "https://sample-url.com", - Owners: []user.User{{ID: "id", UUID: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}}, + Owners: []user.User{{ID: "id", Email: "email@email.com", Provider: "provider"}}, } upstreams := []string{"upstream-1"} downstreams := []string{"downstream-1", "downstream-2"} @@ -530,7 +529,7 @@ func TestUpsertAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -541,7 +540,7 @@ func TestUpsertAsset(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockAssetSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -564,7 +563,7 @@ func TestUpsertAsset(t *testing.T) { func TestUpsertPatchAsset(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = uuid.NewString() assetID = uuid.NewString() validPayload = &compassv1beta1.UpsertPatchAssetRequest{ Asset: &compassv1beta1.UpsertPatchAssetRequest_Asset{ @@ -575,10 +574,9 @@ func TestUpsertPatchAsset(t *testing.T) { Data: &structpb.Struct{}, Url: "https://sample-url.com", Owners: []*compassv1beta1.User{ - {Id: "id", Uuid: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}, + {Id: "id", Email: "email@email.com", Provider: "provider"}, // the following users should get de-duplicated. {Id: "id"}, - {Uuid: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8"}, {Email: "email@email.com"}, }, }, @@ -620,7 +618,7 @@ func TestUpsertPatchAsset(t *testing.T) { UpdatedBy: user.User{ID: userID}, Data: map[string]interface{}{}, URL: "https://sample-url-old.com", - Owners: []user.User{{ID: "id", UUID: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}}, + Owners: []user.User{{ID: "id", Email: "email@email.com", Provider: "provider"}}, } currentAssetNew = asset.Asset{ URN: "test dagger", @@ -630,7 +628,7 @@ func TestUpsertPatchAsset(t *testing.T) { UpdatedBy: user.User{ID: userID}, Data: map[string]interface{}{}, URL: "https://sample-url.com", - Owners: []user.User{{ID: "id", UUID: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}}, + Owners: []user.User{{ID: "id", Email: "email@email.com", Provider: "provider"}}, } ) type testCase struct { @@ -647,7 +645,7 @@ func TestUpsertPatchAsset(t *testing.T) { ExpectStatus: codes.Internal, Request: &compassv1beta1.UpsertPatchAssetRequest{}, Setup: func(ctx context.Context, _ *mocks.AssetService, us *mocks.UserService) { - us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("", errors.New("some-error")) + us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string")).Return("", errors.New("some-error")) }, }, { @@ -760,7 +758,7 @@ func TestUpsertPatchAsset(t *testing.T) { UpdatedBy: user.User{ID: userID}, Data: map[string]interface{}{}, URL: "https://sample-url.com", - Owners: []user.User{{ID: "id", UUID: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}}, + Owners: []user.User{{ID: "id", Email: "email@email.com", Provider: "provider"}}, } upstreams := []string{"upstream-1"} downstreams := []string{"downstream-1", "downstream-2"} @@ -796,7 +794,7 @@ func TestUpsertPatchAsset(t *testing.T) { UpdatedBy: user.User{ID: userID}, Data: map[string]interface{}{}, URL: "https://sample-url-old.com", - Owners: []user.User{{ID: "id", UUID: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}}, + Owners: []user.User{{ID: "id", Email: "email@email.com", Provider: "provider"}}, } assetWithID := patchedAsset @@ -816,7 +814,7 @@ func TestUpsertPatchAsset(t *testing.T) { Name: wrapperspb.String("new-name"), Service: "kafka", Data: &structpb.Struct{}, - Owners: []*compassv1beta1.User{{Id: "id", Uuid: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}}, + Owners: []*compassv1beta1.User{{Id: "id", Email: "email@email.com", Provider: "provider"}}, }, }, ExpectStatus: codes.OK, @@ -841,7 +839,7 @@ func TestUpsertPatchAsset(t *testing.T) { UpdatedBy: user.User{ID: userID}, Data: map[string]interface{}{}, URL: "https://sample-url-old.com", - Owners: []user.User{{ID: "id", UUID: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}}, + Owners: []user.User{{ID: "id", Email: "email@email.com", Provider: "provider"}}, } assetWithID := patchedAsset @@ -861,7 +859,7 @@ func TestUpsertPatchAsset(t *testing.T) { Name: wrapperspb.String("new-name"), Service: "kafka", Data: &structpb.Struct{}, - Owners: []*compassv1beta1.User{{Id: "id", Uuid: "1aecb8b3-23a9-4456-8ebd-3aafc746fff8", Email: "email@email.com", Provider: "provider"}}, + Owners: []*compassv1beta1.User{{Id: "id", Email: "email@email.com", Provider: "provider"}}, }, OverwriteLineage: true, }, @@ -879,7 +877,7 @@ func TestUpsertPatchAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -890,7 +888,7 @@ func TestUpsertPatchAsset(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockAssetSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -912,8 +910,8 @@ func TestUpsertPatchAsset(t *testing.T) { func TestDeleteAsset(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = uuid.NewString() ) type TestCase struct { Description string @@ -958,7 +956,7 @@ func TestDeleteAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -969,7 +967,7 @@ func TestDeleteAsset(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockAssetSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -986,7 +984,7 @@ func TestDeleteAsset(t *testing.T) { func TestDeleteAssets(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = uuid.NewString() dummyQuery = "testing < now()" dummyRequest = asset.DeleteAssetsRequest{ QueryExpr: dummyQuery, @@ -1026,7 +1024,7 @@ func TestDeleteAssets(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -1037,7 +1035,7 @@ func TestDeleteAssets(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockAssetSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -1058,7 +1056,7 @@ func TestGetAssetStargazers(t *testing.T) { size = 20 defaultStarCfg = star.Filter{Offset: offset, Size: size} userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = uuid.NewString() ) type TestCase struct { @@ -1075,7 +1073,7 @@ func TestGetAssetStargazers(t *testing.T) { ExpectStatus: codes.Internal, Request: &compassv1beta1.GetAssetStargazersRequest{}, Setup: func(ctx context.Context, as *mocks.StarService, us *mocks.UserService) { - us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("", errors.New("some-error")) + us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string")).Return("", errors.New("some-error")) }, }, { @@ -1129,7 +1127,7 @@ func TestGetAssetStargazers(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -1139,7 +1137,7 @@ func TestGetAssetStargazers(t *testing.T) { } defer mockStarSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{StarSvc: mockStarSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -1161,8 +1159,8 @@ func TestGetAssetStargazers(t *testing.T) { func TestGetAssetVersionHistory(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = uuid.NewString() ) type TestCase struct { @@ -1179,7 +1177,7 @@ func TestGetAssetVersionHistory(t *testing.T) { ExpectStatus: codes.Internal, Request: &compassv1beta1.GetAssetVersionHistoryRequest{}, Setup: func(ctx context.Context, _ *mocks.AssetService, us *mocks.UserService) { - us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("", errors.New("some-error")) + us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string")).Return("", errors.New("some-error")) }, }, { @@ -1259,7 +1257,7 @@ func TestGetAssetVersionHistory(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -1270,7 +1268,7 @@ func TestGetAssetVersionHistory(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockAssetSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -1292,10 +1290,10 @@ func TestGetAssetVersionHistory(t *testing.T) { func TestGetAssetByVersion(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() - version = "0.2" - ast = asset.Asset{ + userID = uuid.NewString() + userEmail = uuid.NewString() + version = "0.2" + ast = asset.Asset{ ID: assetID, Version: version, Owners: []user.User{{Email: "dummy@trash.com"}}, @@ -1316,7 +1314,7 @@ func TestGetAssetByVersion(t *testing.T) { ExpectStatus: codes.Internal, Request: &compassv1beta1.GetAssetByVersionRequest{}, Setup: func(ctx context.Context, _ *mocks.AssetService, us *mocks.UserService) { - us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("", errors.New("some-error")) + us.EXPECT().ValidateUser(ctx, mock.AnythingOfType("string")).Return("", errors.New("some-error")) }, }, { @@ -1379,7 +1377,7 @@ func TestGetAssetByVersion(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -1390,7 +1388,7 @@ func TestGetAssetByVersion(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockAssetSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -1412,11 +1410,11 @@ func TestGetAssetByVersion(t *testing.T) { func TestCreateAssetProbe(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() - assetURN = "test-urn" - now = time.Now().UTC() - probeID = uuid.NewString() + userID = uuid.NewString() + userEmail = uuid.NewString() + assetURN = "test-urn" + now = time.Now().UTC() + probeID = uuid.NewString() ) type testCase struct { @@ -1554,7 +1552,7 @@ func TestCreateAssetProbe(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := mocks.NewUserService(t) @@ -1563,7 +1561,7 @@ func TestCreateAssetProbe(t *testing.T) { tc.Setup(ctx, mockAssetSvc) } - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockAssetSvc, UserSvc: mockUserSvc, Logger: logger}) diff --git a/internal/server/v1beta1/comment_test.go b/internal/server/v1beta1/comment_test.go index f8ad0ce7..d9ef0036 100644 --- a/internal/server/v1beta1/comment_test.go +++ b/internal/server/v1beta1/comment_test.go @@ -25,7 +25,7 @@ import ( func TestCreateComment(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" discussionID = "11111" validRequest = &compassv1beta1.CreateCommentRequest{ DiscussionId: discussionID, @@ -92,7 +92,7 @@ func TestCreateComment(t *testing.T) { for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -103,7 +103,7 @@ func TestCreateComment(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -124,7 +124,7 @@ func TestCreateComment(t *testing.T) { func TestGetAllComments(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" discussionID = "11111" ) type testCase struct { @@ -219,7 +219,7 @@ func TestGetAllComments(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -230,7 +230,7 @@ func TestGetAllComments(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -253,7 +253,7 @@ func TestGetAllComments(t *testing.T) { func TestGetComment(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" discussionID = "123" commentID = "11" ) @@ -346,7 +346,7 @@ func TestGetComment(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -357,7 +357,7 @@ func TestGetComment(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -380,7 +380,7 @@ func TestGetComment(t *testing.T) { func TestUpdateComment(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" discussionID = "123" commentID = "11" ) @@ -492,7 +492,7 @@ func TestUpdateComment(t *testing.T) { for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -503,7 +503,7 @@ func TestUpdateComment(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) _, err := handler.UpdateComment(ctx, tc.Request) @@ -519,7 +519,7 @@ func TestUpdateComment(t *testing.T) { func TestDeleteComment(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" discussionID = "123" commentID = "11" ) @@ -601,7 +601,7 @@ func TestDeleteComment(t *testing.T) { for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -612,7 +612,7 @@ func TestDeleteComment(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) diff --git a/internal/server/v1beta1/discussion_test.go b/internal/server/v1beta1/discussion_test.go index e2f32e4c..fddfcc26 100644 --- a/internal/server/v1beta1/discussion_test.go +++ b/internal/server/v1beta1/discussion_test.go @@ -25,8 +25,8 @@ import ( func TestGetAllDiscussions(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -111,7 +111,7 @@ func TestGetAllDiscussions(t *testing.T) { for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -122,7 +122,7 @@ func TestGetAllDiscussions(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -144,8 +144,8 @@ func TestGetAllDiscussions(t *testing.T) { func TestCreateDiscussion(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) validRequest := &compassv1beta1.CreateDiscussionRequest{ Title: "Lorem Ipsum", @@ -240,7 +240,7 @@ func TestCreateDiscussion(t *testing.T) { for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -251,7 +251,7 @@ func TestCreateDiscussion(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -268,7 +268,7 @@ func TestCreateDiscussion(t *testing.T) { func TestGetDiscussion(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" discussionID = "123" ) @@ -341,7 +341,7 @@ func TestGetDiscussion(t *testing.T) { for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -352,7 +352,7 @@ func TestGetDiscussion(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) got, err := handler.GetDiscussion(ctx, tc.Request) @@ -374,7 +374,7 @@ func TestGetDiscussion(t *testing.T) { func TestPatchDiscussion(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" discussionID = "123" ) @@ -479,7 +479,7 @@ func TestPatchDiscussion(t *testing.T) { for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -490,7 +490,7 @@ func TestPatchDiscussion(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) diff --git a/internal/server/v1beta1/lineage_test.go b/internal/server/v1beta1/lineage_test.go index ef01be6b..35365e43 100644 --- a/internal/server/v1beta1/lineage_test.go +++ b/internal/server/v1beta1/lineage_test.go @@ -21,12 +21,12 @@ import ( func TestGetLineageGraph(t *testing.T) { // TODO[2022-10-13|@sudo-suhas]: Add comprehensive tests var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) t.Run("get Lineage", func(t *testing.T) { t.Run("should return a graph containing the requested resource, along with it's related resources", func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() nodeURN := "job-1" level := 8 @@ -59,7 +59,7 @@ func TestGetLineageGraph(t *testing.T) { defer mockSvc.AssertExpectations(t) mockSvc.EXPECT().GetLineage(ctx, nodeURN, asset.LineageQuery{Level: level, Direction: direction, WithAttributes: true}).Return(lineage, nil) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) diff --git a/internal/server/v1beta1/mocks/user_service.go b/internal/server/v1beta1/mocks/user_service.go index 91c41229..a1d9556a 100644 --- a/internal/server/v1beta1/mocks/user_service.go +++ b/internal/server/v1beta1/mocks/user_service.go @@ -21,23 +21,23 @@ func (_m *UserService) EXPECT() *UserService_Expecter { return &UserService_Expecter{mock: &_m.Mock} } -// ValidateUser provides a mock function with given fields: ctx, uuid, email -func (_m *UserService) ValidateUser(ctx context.Context, uuid string, email string) (string, error) { - ret := _m.Called(ctx, uuid, email) +// ValidateUser provides a mock function with given fields: ctx, email +func (_m *UserService) ValidateUser(ctx context.Context, email string) (string, error) { + ret := _m.Called(ctx, email) var r0 string var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) (string, error)); ok { - return rf(ctx, uuid, email) + if rf, ok := ret.Get(0).(func(context.Context, string) (string, error)); ok { + return rf(ctx, email) } - if rf, ok := ret.Get(0).(func(context.Context, string, string) string); ok { - r0 = rf(ctx, uuid, email) + if rf, ok := ret.Get(0).(func(context.Context, string) string); ok { + r0 = rf(ctx, email) } else { r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { - r1 = rf(ctx, uuid, email) + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, email) } else { r1 = ret.Error(1) } @@ -52,15 +52,14 @@ type UserService_ValidateUser_Call struct { // ValidateUser is a helper method to define mock.On call // - ctx context.Context -// - uuid string // - email string -func (_e *UserService_Expecter) ValidateUser(ctx interface{}, uuid interface{}, email interface{}) *UserService_ValidateUser_Call { - return &UserService_ValidateUser_Call{Call: _e.mock.On("ValidateUser", ctx, uuid, email)} +func (_e *UserService_Expecter) ValidateUser(ctx interface{}, email interface{}) *UserService_ValidateUser_Call { + return &UserService_ValidateUser_Call{Call: _e.mock.On("ValidateUser", ctx, email)} } -func (_c *UserService_ValidateUser_Call) Run(run func(ctx context.Context, uuid string, email string)) *UserService_ValidateUser_Call { +func (_c *UserService_ValidateUser_Call) Run(run func(ctx context.Context, email string)) *UserService_ValidateUser_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(string)) + run(args[0].(context.Context), args[1].(string)) }) return _c } @@ -70,7 +69,7 @@ func (_c *UserService_ValidateUser_Call) Return(_a0 string, _a1 error) *UserServ return _c } -func (_c *UserService_ValidateUser_Call) RunAndReturn(run func(context.Context, string, string) (string, error)) *UserService_ValidateUser_Call { +func (_c *UserService_ValidateUser_Call) RunAndReturn(run func(context.Context, string) (string, error)) *UserService_ValidateUser_Call { _c.Call.Return(run) return _c } diff --git a/internal/server/v1beta1/search_test.go b/internal/server/v1beta1/search_test.go index 414de612..8c283198 100644 --- a/internal/server/v1beta1/search_test.go +++ b/internal/server/v1beta1/search_test.go @@ -22,8 +22,8 @@ import ( func TestSearch(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -210,7 +210,7 @@ func TestSearch(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := mocks.NewUserService(t) @@ -219,7 +219,7 @@ func TestSearch(t *testing.T) { tc.Setup(ctx, mockSvc) } - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -241,8 +241,8 @@ func TestSearch(t *testing.T) { func TestSuggest(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -307,7 +307,7 @@ func TestSuggest(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := mocks.NewUserService(t) @@ -316,7 +316,7 @@ func TestSuggest(t *testing.T) { tc.Setup(ctx, mockSvc) } - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -338,8 +338,8 @@ func TestSuggest(t *testing.T) { func TestGroupAssets(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -479,7 +479,7 @@ func TestGroupAssets(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := mocks.NewUserService(t) @@ -488,7 +488,7 @@ func TestGroupAssets(t *testing.T) { tc.Setup(ctx, mockSvc) } - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) diff --git a/internal/server/v1beta1/server.go b/internal/server/v1beta1/server.go index fec75d72..6eaaeb22 100644 --- a/internal/server/v1beta1/server.go +++ b/internal/server/v1beta1/server.go @@ -63,12 +63,12 @@ func NewAPIServer(d APIServerDeps) *APIServer { func (server *APIServer) ValidateUserInCtx(ctx context.Context) (string, error) { usr := user.FromContext(ctx) - userID, err := server.userService.ValidateUser(ctx, usr.UUID, usr.Email) + userID, err := server.userService.ValidateUser(ctx, usr.Email) if err != nil { if errors.Is(err, user.ErrNoUserInformation) { return "", status.Errorf(codes.InvalidArgument, err.Error()) } - if errors.As(err, &user.DuplicateRecordError{UUID: usr.UUID, Email: usr.Email}) { + if errors.As(err, &user.DuplicateRecordError{Email: usr.Email}) { return "", status.Errorf(codes.AlreadyExists, err.Error()) } return "", status.Errorf(codes.Internal, codes.Internal.String()) diff --git a/internal/server/v1beta1/server_test.go b/internal/server/v1beta1/server_test.go index 041fbd11..59c2828c 100644 --- a/internal/server/v1beta1/server_test.go +++ b/internal/server/v1beta1/server_test.go @@ -17,13 +17,13 @@ import ( func TestValidateUserInCtx(t *testing.T) { var ( - userUUID = uuid.NewString() - userID = uuid.NewString() + userEmail = "test@test.com" + userID = uuid.NewString() ) type testCase struct { Description string UserID string - UserUUID string + UserEmail string ExpectStatus codes.Code Setup func(context.Context, *mocks.UserService) PostCheck func(resp *compassv1beta1.GetUserStarredAssetsResponse) error @@ -33,25 +33,25 @@ func TestValidateUserInCtx(t *testing.T) { { Description: "should return invalid argument error if ValidateUser empty uuid is passed", UserID: "", - UserUUID: "", + UserEmail: "", ExpectStatus: codes.InvalidArgument, Setup: func(ctx context.Context, us *mocks.UserService) { - us.EXPECT().ValidateUser(ctx, "", "").Return("", user.ErrNoUserInformation) + us.EXPECT().ValidateUser(ctx, "").Return("", user.ErrNoUserInformation) }, }, { Description: "should return internal error if ValidateUser returns some error", UserID: userID, - UserUUID: userUUID, + UserEmail: userEmail, ExpectStatus: codes.Internal, Setup: func(ctx context.Context, us *mocks.UserService) { - us.EXPECT().ValidateUser(ctx, userUUID, "").Return("", errors.New("internal error")) + us.EXPECT().ValidateUser(ctx, userEmail).Return("", errors.New("internal error")) }, }, } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: tc.UserUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: tc.UserEmail}) logger := log.NewNoop() diff --git a/internal/server/v1beta1/tag_template_test.go b/internal/server/v1beta1/tag_template_test.go index 9bbf9ae9..51511f1a 100644 --- a/internal/server/v1beta1/tag_template_test.go +++ b/internal/server/v1beta1/tag_template_test.go @@ -73,8 +73,8 @@ var sampleTemplatePB = &compassv1beta1.TagTemplate{ func TestGetAllTagTemplates(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = uuid.NewString() ) type testCase struct { Description string @@ -140,7 +140,7 @@ func TestGetAllTagTemplates(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -153,7 +153,7 @@ func TestGetAllTagTemplates(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, @@ -181,7 +181,7 @@ func TestGetAllTagTemplates(t *testing.T) { func TestCreateTagTemplate(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = uuid.NewString() validRequest = &compassv1beta1.CreateTagTemplateRequest{ Urn: sampleTemplatePB.GetUrn(), DisplayName: sampleTemplatePB.GetDisplayName(), @@ -275,7 +275,7 @@ func TestCreateTagTemplate(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -288,7 +288,7 @@ func TestCreateTagTemplate(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, @@ -316,7 +316,7 @@ func TestCreateTagTemplate(t *testing.T) { func TestGetTagTemplate(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = uuid.NewString() validRequest = &compassv1beta1.GetTagTemplateRequest{ TemplateUrn: sampleTemplatePB.GetUrn(), } @@ -367,7 +367,7 @@ func TestGetTagTemplate(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -380,7 +380,7 @@ func TestGetTagTemplate(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, @@ -408,7 +408,7 @@ func TestGetTagTemplate(t *testing.T) { func TestUpdateTagTemplate(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" validRequest = &compassv1beta1.UpdateTagTemplateRequest{ TemplateUrn: sampleTemplatePB.GetUrn(), DisplayName: sampleTemplatePB.GetDisplayName(), @@ -504,7 +504,7 @@ func TestUpdateTagTemplate(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -517,7 +517,7 @@ func TestUpdateTagTemplate(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, @@ -545,7 +545,7 @@ func TestUpdateTagTemplate(t *testing.T) { func TestDeleteTagTemplate(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = uuid.NewString() validRequest = &compassv1beta1.DeleteTagTemplateRequest{ TemplateUrn: sampleTemplatePB.GetUrn(), } @@ -585,7 +585,7 @@ func TestDeleteTagTemplate(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) @@ -598,7 +598,7 @@ func TestDeleteTagTemplate(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, diff --git a/internal/server/v1beta1/tag_test.go b/internal/server/v1beta1/tag_test.go index 1e08df4d..6e3ec480 100644 --- a/internal/server/v1beta1/tag_test.go +++ b/internal/server/v1beta1/tag_test.go @@ -83,8 +83,8 @@ var sampleTagPB = &compassv1beta1.Tag{ func TestGetTagByAssetAndTemplate(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -186,7 +186,7 @@ func TestGetTagByAssetAndTemplate(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) mockTagSvc := new(mocks.TagService) @@ -198,7 +198,7 @@ func TestGetTagByAssetAndTemplate(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, @@ -226,7 +226,7 @@ func TestGetTagByAssetAndTemplate(t *testing.T) { func TestCreateTagAsset(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" validRequest = &compassv1beta1.CreateTagAssetRequest{ AssetId: sampleTagPB.GetAssetId(), TemplateUrn: sampleTagPB.GetTemplateUrn(), @@ -323,7 +323,7 @@ func TestCreateTagAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) mockTagSvc := new(mocks.TagService) @@ -335,7 +335,7 @@ func TestCreateTagAsset(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, @@ -363,7 +363,7 @@ func TestCreateTagAsset(t *testing.T) { func TestUpdateTagAsset(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" validRequest = &compassv1beta1.UpdateTagAssetRequest{ AssetId: sampleTagPB.GetAssetId(), TemplateUrn: sampleTagPB.GetTemplateUrn(), @@ -452,7 +452,7 @@ func TestUpdateTagAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) mockTagSvc := new(mocks.TagService) @@ -464,7 +464,7 @@ func TestUpdateTagAsset(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, @@ -491,8 +491,8 @@ func TestUpdateTagAsset(t *testing.T) { func TestDeleteTagAsset(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -554,7 +554,7 @@ func TestDeleteTagAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) mockTagSvc := new(mocks.TagService) @@ -566,7 +566,7 @@ func TestDeleteTagAsset(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, @@ -588,7 +588,7 @@ func TestDeleteTagAsset(t *testing.T) { func TestGetAllTagsByAsset(t *testing.T) { var ( userID = uuid.NewString() - userUUID = uuid.NewString() + userEmail = "test@test.com" validRequest = &compassv1beta1.GetAllTagsByAssetRequest{ AssetId: assetID, } @@ -638,7 +638,7 @@ func TestGetAllTagsByAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() mockUserSvc := new(mocks.UserService) mockTagSvc := new(mocks.TagService) @@ -650,7 +650,7 @@ func TestGetAllTagsByAsset(t *testing.T) { defer mockTagSvc.AssertExpectations(t) defer mockTemplateSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{ TagSvc: mockTagSvc, diff --git a/internal/server/v1beta1/type_test.go b/internal/server/v1beta1/type_test.go index 56d6a6ef..f51b168d 100644 --- a/internal/server/v1beta1/type_test.go +++ b/internal/server/v1beta1/type_test.go @@ -19,8 +19,8 @@ import ( func TestGetTypes(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -115,7 +115,7 @@ func TestGetTypes(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) mockUserSvc := new(mocks.UserService) mockSvc := new(mocks.AssetService) @@ -126,7 +126,7 @@ func TestGetTypes(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{AssetSvc: mockSvc, UserSvc: mockUserSvc, Logger: logger}) diff --git a/internal/server/v1beta1/user.go b/internal/server/v1beta1/user.go index dbfe01f6..64a8e206 100644 --- a/internal/server/v1beta1/user.go +++ b/internal/server/v1beta1/user.go @@ -18,7 +18,7 @@ import ( ) type UserService interface { - ValidateUser(ctx context.Context, uuid, email string) (string, error) + ValidateUser(ctx context.Context, email string) (string, error) } func (server *APIServer) GetUserStarredAssets(ctx context.Context, req *compassv1beta1.GetUserStarredAssetsRequest) (*compassv1beta1.GetUserStarredAssetsResponse, error) { @@ -252,7 +252,7 @@ func userToProto(u user.User) *compassv1beta1.User { return nil } return &compassv1beta1.User{ - Uuid: u.UUID, + Id: u.ID, Email: u.Email, } } @@ -274,7 +274,6 @@ func userToFullProto(u user.User) *compassv1beta1.User { return &compassv1beta1.User{ Id: u.ID, - Uuid: u.UUID, Email: u.Email, Provider: u.Provider, CreatedAt: createdAtPB, @@ -296,7 +295,6 @@ func userFromProto(proto *compassv1beta1.User) user.User { return user.User{ ID: proto.GetId(), - UUID: proto.GetUuid(), Email: proto.GetEmail(), Provider: proto.GetProvider(), CreatedAt: createdAt, diff --git a/internal/server/v1beta1/user_test.go b/internal/server/v1beta1/user_test.go index 3a1e423b..c9443681 100644 --- a/internal/server/v1beta1/user_test.go +++ b/internal/server/v1beta1/user_test.go @@ -25,10 +25,10 @@ import ( func TestGetUserStarredAssets(t *testing.T) { var ( - userUUID = uuid.NewString() - userID = uuid.NewString() - offset = 2 - size = 10 + userEmail = "test@test.com" + userID = uuid.NewString() + offset = 2 + size = 10 ) type testCase struct { Description string @@ -99,7 +99,7 @@ func TestGetUserStarredAssets(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() @@ -111,7 +111,7 @@ func TestGetUserStarredAssets(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockStarSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{StarSvc: mockStarSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -137,10 +137,10 @@ func TestGetUserStarredAssets(t *testing.T) { func TestGetMyStarredAssets(t *testing.T) { var ( - userUUID = uuid.NewString() - userID = uuid.NewString() - offset = 2 - size = 10 + userEmail = "test@test.com" + userID = uuid.NewString() + offset = 2 + size = 10 ) type testCase struct { Description string @@ -211,7 +211,7 @@ func TestGetMyStarredAssets(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() @@ -223,7 +223,7 @@ func TestGetMyStarredAssets(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockStarSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{StarSvc: mockStarSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -248,7 +248,7 @@ func TestGetMyStarredAssets(t *testing.T) { func TestGetMyStarredAsset(t *testing.T) { var ( - userUUID = uuid.NewString() + userEmail = "test@test.com" userID = uuid.NewString() assetID = uuid.NewString() assetType = "an-asset-type" @@ -313,7 +313,7 @@ func TestGetMyStarredAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() @@ -325,7 +325,7 @@ func TestGetMyStarredAsset(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockStarSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{StarSvc: mockStarSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -349,9 +349,9 @@ func TestGetMyStarredAsset(t *testing.T) { func TestStarAsset(t *testing.T) { var ( - userID = uuid.NewString() - assetID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + assetID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -405,7 +405,7 @@ func TestStarAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() @@ -417,7 +417,7 @@ func TestStarAsset(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockStarSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{StarSvc: mockStarSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -435,9 +435,9 @@ func TestStarAsset(t *testing.T) { func TestUnstarAsset(t *testing.T) { var ( - userID = uuid.NewString() - assetID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + assetID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -484,7 +484,7 @@ func TestUnstarAsset(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() @@ -496,7 +496,7 @@ func TestUnstarAsset(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockStarSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{StarSvc: mockStarSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -514,8 +514,8 @@ func TestUnstarAsset(t *testing.T) { func TestGetMyDiscussions(t *testing.T) { var ( - userID = uuid.NewString() - userUUID = uuid.NewString() + userID = uuid.NewString() + userEmail = "test@test.com" ) type testCase struct { Description string @@ -638,7 +638,7 @@ func TestGetMyDiscussions(t *testing.T) { } for _, tc := range testCases { t.Run(tc.Description, func(t *testing.T) { - ctx := user.NewContext(context.Background(), user.User{UUID: userUUID}) + ctx := user.NewContext(context.Background(), user.User{Email: userEmail}) logger := log.NewNoop() @@ -650,7 +650,7 @@ func TestGetMyDiscussions(t *testing.T) { defer mockUserSvc.AssertExpectations(t) defer mockDiscussionSvc.AssertExpectations(t) - mockUserSvc.EXPECT().ValidateUser(ctx, userUUID, "").Return(userID, nil) + mockUserSvc.EXPECT().ValidateUser(ctx, userEmail).Return(userID, nil) handler := NewAPIServer(APIServerDeps{DiscussionSvc: mockDiscussionSvc, UserSvc: mockUserSvc, Logger: logger}) @@ -680,14 +680,14 @@ func TestUserToProto(t *testing.T) { testCases := []testCase{ { - Title: "should return nil if UUID is empty", + Title: "should return nil if email is empty", User: user.User{}, ExpectProto: nil, }, { Title: "should return fields without timestamp", - User: user.User{UUID: "uuid1", Email: "email@email.com", Provider: "provider", CreatedAt: timeDummy, UpdatedAt: timeDummy}, - ExpectProto: &compassv1beta1.User{Uuid: "uuid1", Email: "email@email.com"}, + User: user.User{ID: "86a7987c-2f4d-4a0b-a8da-08b17e81a047", Email: "email@email.com", Provider: "provider", CreatedAt: timeDummy, UpdatedAt: timeDummy}, + ExpectProto: &compassv1beta1.User{Id: "86a7987c-2f4d-4a0b-a8da-08b17e81a047", Email: "email@email.com"}, }, } for _, tc := range testCases { @@ -710,19 +710,19 @@ func TestUserToFullProto(t *testing.T) { testCases := []testCase{ { - Title: "should return nil if UUID is empty", + Title: "should return nil if email is empty", User: user.User{}, ExpectProto: nil, }, { Title: "should return without timestamp pb if timestamp is zero", - User: user.User{UUID: "uuid1", Provider: "provider"}, - ExpectProto: &compassv1beta1.User{Uuid: "uuid1", Provider: "provider"}, + User: user.User{Email: "test@test.com", Provider: "provider"}, + ExpectProto: &compassv1beta1.User{Email: "test@test.com", Provider: "provider"}, }, { Title: "should return with timestamp pb if timestamp is not zero", - User: user.User{UUID: "uuid1", Provider: "provider", CreatedAt: timeDummy, UpdatedAt: timeDummy}, - ExpectProto: &compassv1beta1.User{Uuid: "uuid1", Provider: "provider", CreatedAt: timestamppb.New(timeDummy), UpdatedAt: timestamppb.New(timeDummy)}, + User: user.User{Email: "test@test.com", Provider: "provider", CreatedAt: timeDummy, UpdatedAt: timeDummy}, + ExpectProto: &compassv1beta1.User{Email: "test@test.com", Provider: "provider", CreatedAt: timestamppb.New(timeDummy), UpdatedAt: timestamppb.New(timeDummy)}, }, } for _, tc := range testCases { @@ -746,13 +746,13 @@ func TestUserFromProto(t *testing.T) { testCases := []testCase{ { Title: "should return non empty time.Time if timestamp pb is not zero", - UserPB: &compassv1beta1.User{Uuid: "uuid1", Provider: "provider", CreatedAt: timestamppb.New(timeDummy), UpdatedAt: timestamppb.New(timeDummy)}, - ExpectUser: user.User{UUID: "uuid1", Provider: "provider", CreatedAt: timeDummy, UpdatedAt: timeDummy}, + UserPB: &compassv1beta1.User{Email: "test@test.com", Provider: "provider", CreatedAt: timestamppb.New(timeDummy), UpdatedAt: timestamppb.New(timeDummy)}, + ExpectUser: user.User{Email: "test@test.com", Provider: "provider", CreatedAt: timeDummy, UpdatedAt: timeDummy}, }, { Title: "should return empty time.Time if timestamp pb is zero", - UserPB: &compassv1beta1.User{Uuid: "uuid1", Provider: "provider"}, - ExpectUser: user.User{UUID: "uuid1", Provider: "provider"}, + UserPB: &compassv1beta1.User{Email: "test@test.com", Provider: "provider"}, + ExpectUser: user.User{Email: "test@test.com", Provider: "provider"}, }, } for _, tc := range testCases { diff --git a/internal/store/postgres/asset_repository.go b/internal/store/postgres/asset_repository.go index 28d58bf8..bb955809 100644 --- a/internal/store/postgres/asset_repository.go +++ b/internal/store/postgres/asset_repository.go @@ -724,7 +724,6 @@ func (r *AssetRepository) getOwners(ctx context.Context, assetID string) ([]user query := ` SELECT u.id as "id", - u.uuid as "uuid", u.email as "email", u.provider as "provider" FROM asset_owners ao @@ -841,11 +840,8 @@ func (r *AssetRepository) createOrFetchUsers(ctx context.Context, tx *sqlx.Tx, u fetchedUser user.User err error ) - if u.UUID != "" { - fetchedUser, err = r.userRepo.GetByUUIDWithTx(ctx, tx, u.UUID) - } else { - fetchedUser, err = r.userRepo.GetByEmailWithTx(ctx, tx, u.Email) - } + fetchedUser, err = r.userRepo.GetByEmailWithTx(ctx, tx, u.Email) + switch { case errors.As(err, &user.NotFoundError{}): u.Provider = r.defaultUserProvider @@ -911,7 +907,6 @@ func (r *AssetRepository) getAssetSQL() sq.SelectBuilder { a.updated_at as updated_at, a.refreshed_at as refreshed_at, u.id as "updated_by.id", - u.uuid as "updated_by.uuid", u.email as "updated_by.email", u.provider as "updated_by.provider", u.created_at as "updated_by.created_at", @@ -937,7 +932,6 @@ func (r *AssetRepository) getAssetVersionSQL() sq.SelectBuilder { a.changelog as changelog, a.owners as owners, u.id as "updated_by.id", - u.uuid as "updated_by.uuid", u.email as "updated_by.email", u.provider as "updated_by.provider", u.created_at as "updated_by.created_at", diff --git a/internal/store/postgres/asset_repository_test.go b/internal/store/postgres/asset_repository_test.go index d219cd2c..47b5316b 100644 --- a/internal/store/postgres/asset_repository_test.go +++ b/internal/store/postgres/asset_repository_test.go @@ -62,22 +62,22 @@ func (r *AssetRepositoryTestSuite) createUsers(userRepo user.Repository) []user. var err error users := []user.User{} - user1 := user.User{UUID: uuid.NewString(), Email: "user-test-1@gotocompany.com", Provider: defaultProviderName} + user1 := user.User{Email: "user-test-1@gotocompany.com", Provider: defaultProviderName} user1.ID, err = userRepo.Create(r.ctx, &user1) r.Require().NoError(err) users = append(users, user1) - user2 := user.User{UUID: uuid.NewString(), Email: "user-test-2@gotocompany.com", Provider: defaultProviderName} + user2 := user.User{Email: "user-test-2@gotocompany.com", Provider: defaultProviderName} user2.ID, err = userRepo.Create(r.ctx, &user2) r.Require().NoError(err) users = append(users, user2) - user3 := user.User{UUID: uuid.NewString(), Email: "user-test-3@gotocompany.com", Provider: defaultProviderName} + user3 := user.User{Email: "user-test-3@gotocompany.com", Provider: defaultProviderName} user3.ID, err = userRepo.Create(r.ctx, &user3) r.Require().NoError(err) users = append(users, user3) - user4 := user.User{UUID: uuid.NewString(), Email: "user-test-4@gotocompany.com", Provider: defaultProviderName} + user4 := user.User{Email: "user-test-4@gotocompany.com", Provider: defaultProviderName} user4.ID, err = userRepo.Create(r.ctx, &user4) r.Require().NoError(err) users = append(users, user4) @@ -1030,8 +1030,7 @@ func (r *AssetRepositoryTestSuite) TestUpsert() { Owners: []user.User{ r.users[1], {Email: r.users[2].Email}, - {UUID: r.users[2].UUID}, // should get deduplicated by ID on fetch by UUID - {ID: r.users[1].ID}, // should get deduplicated by ID + {ID: r.users[1].ID}, // should get deduplicated by ID }, UpdatedBy: r.users[0], } @@ -1056,7 +1055,7 @@ func (r *AssetRepositoryTestSuite) TestUpsert() { Service: "bigquery", Owners: []user.User{ {Email: "newuser@example.com"}, - {UUID: "795151e5-4c9f-4951-a8e1-6966b5aa2bb6"}, + {Email: "newuser2@example.com"}, {Email: "newuser@example.com"}, // should get deduplicated by ID on fetch user by email }, UpdatedBy: r.users[0], @@ -1071,7 +1070,6 @@ func (r *AssetRepositoryTestSuite) TestUpsert() { r.Len(actual.Owners, 2) r.Equal(ast.Owners[0].Email, actual.Owners[0].Email) - r.Equal(ast.Owners[1].UUID, actual.Owners[1].UUID) }) }) diff --git a/internal/store/postgres/discussion_comment_repository.go b/internal/store/postgres/discussion_comment_repository.go index 5875a792..d0b13033 100644 --- a/internal/store/postgres/discussion_comment_repository.go +++ b/internal/store/postgres/discussion_comment_repository.go @@ -154,12 +154,11 @@ func (r *DiscussionRepository) selectCommentsSQL() sq.SelectBuilder { c.created_at as created_at, c.updated_at as updated_at, uo.id as "owner.id", - uo.uuid as "owner.uuid", uo.email as "owner.email", uo.provider as "owner.provider", uo.created_at as "owner.created_at", uo.updated_at as "owner.updated_at", - uu.uuid as "updated_by.uuid", + uu.id as "updated_by.id", uu.email as "updated_by.email", uu.provider as "updated_by.provider", uu.created_at as "updated_by.created_at", diff --git a/internal/store/postgres/discussion_comment_repository_test.go b/internal/store/postgres/discussion_comment_repository_test.go index bdbcd020..895965e9 100644 --- a/internal/store/postgres/discussion_comment_repository_test.go +++ b/internal/store/postgres/discussion_comment_repository_test.go @@ -165,7 +165,7 @@ func (r *DiscussionRepositoryTestSuite) TestUpdateComment() { r.NoError(err) r.Equal(newCmt.Body, cmt.Body) r.NotEqual(newCmt.UpdatedAt, cmt.UpdatedAt) - r.Equal(newCmt.UpdatedBy.UUID, cmt.UpdatedBy.UUID) + r.Equal(newCmt.UpdatedBy.ID, cmt.UpdatedBy.ID) }) r.Run("should return error when updating a comment that does not exist", func() { diff --git a/internal/store/postgres/discussion_repository.go b/internal/store/postgres/discussion_repository.go index 1f50b73d..d3d0c49e 100644 --- a/internal/store/postgres/discussion_repository.go +++ b/internal/store/postgres/discussion_repository.go @@ -185,7 +185,6 @@ func (r *DiscussionRepository) selectSQL() sq.SelectBuilder { d.created_at as created_at, d.updated_at as updated_at, u.id as "owner.id", - u.uuid as "owner.uuid", u.email as "owner.email", u.provider as "owner.provider", u.created_at as "owner.created_at", diff --git a/internal/store/postgres/migrations/000020_add_constraint_email_users_unique.down.sql b/internal/store/postgres/migrations/000020_add_constraint_email_users_unique.down.sql new file mode 100644 index 00000000..9dd4901b --- /dev/null +++ b/internal/store/postgres/migrations/000020_add_constraint_email_users_unique.down.sql @@ -0,0 +1,11 @@ +DO $$ + BEGIN + IF EXISTS ( + SELECT 1 + FROM pg_constraint + WHERE conname = 'users_email_unique' + ) THEN + ALTER TABLE users + DROP CONSTRAINT users_email_unique; + END IF; +END $$; diff --git a/internal/store/postgres/migrations/000020_add_constraint_email_users_unique.up.sql b/internal/store/postgres/migrations/000020_add_constraint_email_users_unique.up.sql new file mode 100644 index 00000000..a18268bb --- /dev/null +++ b/internal/store/postgres/migrations/000020_add_constraint_email_users_unique.up.sql @@ -0,0 +1,11 @@ +DO $$ + BEGIN + IF NOT EXISTS ( + SELECT 1 + FROM pg_constraint + WHERE conname = 'users_email_unique' + ) THEN + ALTER TABLE users + ADD CONSTRAINT users_email_unique UNIQUE (email); + END IF; +END $$; diff --git a/internal/store/postgres/postgres_test.go b/internal/store/postgres/postgres_test.go index 02d5401f..1c714336 100644 --- a/internal/store/postgres/postgres_test.go +++ b/internal/store/postgres/postgres_test.go @@ -6,7 +6,6 @@ import ( "testing" "time" - "github.com/google/uuid" "github.com/goto/compass/core/asset" "github.com/goto/compass/core/user" "github.com/goto/compass/internal/store/postgres" @@ -93,7 +92,6 @@ func getAsset(ownerEmail, assetURN, assetType string) *asset.Asset { func getUser(email string) *user.User { timestamp := time.Now().UTC() return &user.User{ - UUID: uuid.NewString(), Email: email, Provider: defaultProviderName, CreatedAt: timestamp, @@ -105,7 +103,7 @@ func createUsers(userRepo user.Repository, num int) ([]user.User, error) { var users []user.User for i := 0; i < num; i++ { email := fmt.Sprintf("user-test-%d@gotocompany.com", i+1) - user1 := user.User{UUID: uuid.NewString(), Email: email, Provider: defaultProviderName} + user1 := user.User{Email: email, Provider: defaultProviderName} var err error user1.ID, err = userRepo.Create(context.Background(), &user1) if err != nil { diff --git a/internal/store/postgres/star_repository.go b/internal/store/postgres/star_repository.go index 9906bf78..2090c25a 100644 --- a/internal/store/postgres/star_repository.go +++ b/internal/store/postgres/star_repository.go @@ -79,7 +79,6 @@ func (r *StarRepository) GetStargazers(ctx context.Context, flt star.Filter, ass if err := r.client.db.SelectContext(ctx, &userModels, ` SELECT DISTINCT ON (u.id) u.id, - u.uuid, u.email, u.provider, u.created_at, @@ -130,7 +129,6 @@ func (r *StarRepository) GetAllAssetsByUserID(ctx context.Context, flt star.Filt a.created_at as created_at, a.updated_at as updated_at, u.id as "updated_by.id", - u.uuid as "updated_by.uuid", u.email as "updated_by.email", u.provider as "updated_by.provider", u.created_at as "updated_by.created_at", @@ -195,7 +193,6 @@ func (r *StarRepository) GetAssetByUserID(ctx context.Context, userID, assetID s a.created_at, a.updated_at, u.id as "updated_by.id", - u.uuid as "updated_by.uuid", u.email as "updated_by.email", u.provider as "updated_by.provider", u.created_at as "updated_by.created_at", diff --git a/internal/store/postgres/user_model.go b/internal/store/postgres/user_model.go index 9e873f4f..1eb2b3fd 100644 --- a/internal/store/postgres/user_model.go +++ b/internal/store/postgres/user_model.go @@ -8,7 +8,6 @@ import ( type UserModel struct { ID sql.NullString `db:"id"` - UUID sql.NullString `db:"uuid"` Email sql.NullString `db:"email"` Provider sql.NullString `db:"provider"` CreatedAt sql.NullTime `db:"created_at"` @@ -18,7 +17,6 @@ type UserModel struct { func (u *UserModel) toUser() user.User { return user.User{ ID: u.ID.String, - UUID: u.UUID.String, Email: u.Email.String, Provider: u.Provider.String, CreatedAt: u.CreatedAt.Time, @@ -31,9 +29,6 @@ func newUserModel(u *user.User) UserModel { if u.ID != "" { um.ID = sql.NullString{String: u.ID, Valid: true} } - if u.UUID != "" { - um.UUID = sql.NullString{String: u.UUID, Valid: true} - } if u.Email != "" { um.Email = sql.NullString{String: u.Email, Valid: true} } diff --git a/internal/store/postgres/user_model_test.go b/internal/store/postgres/user_model_test.go index 174f71ae..9692f080 100644 --- a/internal/store/postgres/user_model_test.go +++ b/internal/store/postgres/user_model_test.go @@ -15,8 +15,7 @@ func TestUserModel(t *testing.T) { someUUID := uuid.NewString() timestamp := time.Now().UTC() um := UserModel{ - ID: sql.NullString{String: "12", Valid: true}, - UUID: sql.NullString{String: someUUID, Valid: true}, + ID: sql.NullString{String: someUUID, Valid: true}, Email: sql.NullString{String: "user@gotocompany.com", Valid: true}, Provider: sql.NullString{String: "compass", Valid: true}, CreatedAt: sql.NullTime{Time: timestamp, Valid: true}, @@ -26,7 +25,6 @@ func TestUserModel(t *testing.T) { ud := um.toUser() assert.Equal(t, um.ID.String, ud.ID) - assert.Equal(t, um.UUID.String, ud.UUID) assert.Equal(t, um.Email.String, ud.Email) assert.Equal(t, um.Provider.String, ud.Provider) assert.True(t, um.CreatedAt.Time.Equal(ud.CreatedAt)) @@ -38,8 +36,7 @@ func TestUserModel(t *testing.T) { timestamp := time.Now().UTC() ud := &user.User{ - ID: "12", - UUID: someUUID, + ID: someUUID, Email: "user@gotocompany.com", Provider: "compass", CreatedAt: timestamp, @@ -49,7 +46,6 @@ func TestUserModel(t *testing.T) { um := newUserModel(ud) assert.Equal(t, um.ID.String, ud.ID) - assert.Equal(t, um.UUID.String, ud.UUID) assert.Equal(t, um.Email.String, ud.Email) assert.Equal(t, um.Provider.String, ud.Provider) assert.True(t, um.CreatedAt.Time.Equal(ud.CreatedAt)) diff --git a/internal/store/postgres/user_repository.go b/internal/store/postgres/user_repository.go index f18c08d8..c4c94791 100644 --- a/internal/store/postgres/user_repository.go +++ b/internal/store/postgres/user_repository.go @@ -5,7 +5,6 @@ import ( "database/sql" "errors" "fmt" - sq "github.com/Masterminds/squirrel" "github.com/goto/compass/core/user" "github.com/jmoiron/sqlx" @@ -16,9 +15,8 @@ type UserRepository struct { client *Client } -// UpsertByEmail updates a row if email match and uuid is empty -// if email not found, insert a new row -func (r *UserRepository) UpsertByEmail(ctx context.Context, ud *user.User) (string, error) { +// InsertByEmail insert a new row if email not found +func (r *UserRepository) InsertByEmail(ctx context.Context, ud *user.User) (string, error) { var userID string if err := ud.Validate(); err != nil { @@ -28,25 +26,23 @@ func (r *UserRepository) UpsertByEmail(ctx context.Context, ud *user.User) (stri um := newUserModel(ud) if err := r.client.db.QueryRowxContext(ctx, ` - INSERT INTO users (uuid, email, provider) VALUES ($1, $2, $3) ON CONFLICT (email) - DO UPDATE SET uuid = $1, email = $2 WHERE users.uuid IS NULL + INSERT INTO users (email, provider) VALUES ($1, $2) RETURNING id - `, um.UUID, um.Email, um.Provider).Scan(&userID); err != nil { + `, um.Email, um.Provider).Scan(&userID); err != nil { err := checkPostgresError(err) - if errors.Is(err, sql.ErrNoRows) { - return "", user.DuplicateRecordError{UUID: ud.UUID, Email: ud.Email} + if errors.Is(err, errDuplicateKey) { + return "", user.DuplicateRecordError{Email: ud.Email} } return "", err } if userID == "" { - return "", fmt.Errorf("error User UUID is empty from DB") + return "", fmt.Errorf("error User ID is empty from DB") } return userID, nil } // Create insert a user to the database -// a new data is still inserted if either uuid or email is empty -// but returns error if both uuid and email are empty +// returns error if email is empty func (r *UserRepository) Create(ctx context.Context, ud *user.User) (string, error) { return r.create(ctx, r.client.db, ud) } @@ -63,7 +59,7 @@ func (r *UserRepository) create(ctx context.Context, querier sqlx.QueryerContext return "", user.ErrNoUserInformation } - if ud.UUID == "" && ud.Email == "" { + if ud.Email == "" { return "", user.ErrNoUserInformation } @@ -72,24 +68,24 @@ func (r *UserRepository) create(ctx context.Context, querier sqlx.QueryerContext if err := querier.QueryRowxContext(ctx, ` INSERT INTO users - (uuid, email, provider) + (email, provider) VALUES - ($1, $2, $3) + ($1, $2) RETURNING id - `, um.UUID, um.Email, um.Provider).Scan(&userID); err != nil { + `, um.Email, um.Provider).Scan(&userID); err != nil { err := checkPostgresError(err) if errors.Is(err, errDuplicateKey) { - return "", user.DuplicateRecordError{UUID: ud.UUID, Email: ud.Email} + return "", user.DuplicateRecordError{Email: ud.Email} } return "", err } if userID == "" { - return "", fmt.Errorf("error User UUID is empty from DB") + return "", fmt.Errorf("error User ID is empty from DB") } return userID, nil } -// GetUUID retrieves user UUID given the email +// GetByEmail retrieves user by given the email func (r *UserRepository) GetByEmail(ctx context.Context, email string) (user.User, error) { return r.GetByEmailWithTx(ctx, r.client.db, email) } @@ -105,24 +101,29 @@ func (r *UserRepository) GetByEmailWithTx(ctx context.Context, querier sqlx.Quer return u, nil } -// GetbyUUID retrieves user given the uuid -func (r *UserRepository) GetByUUID(ctx context.Context, uuid string) (user.User, error) { - return r.GetByUUIDWithTx(ctx, r.client.db, uuid) -} - -func (r *UserRepository) GetByUUIDWithTx(ctx context.Context, querier sqlx.QueryerContext, uuid string) (user.User, error) { - u, err := getUserByPredicate(ctx, querier, sq.Eq{"uuid": uuid}) - if err != nil && errors.Is(err, sql.ErrNoRows) { - return user.User{}, user.NotFoundError{UUID: uuid} - } - if err != nil { - return user.User{}, err +func (r *UserRepository) GetOrInsertByEmail(ctx context.Context, ud *user.User) (string, error) { + var userID string + if err := ud.Validate(); err != nil { + return "", err } - return u, nil + um := newUserModel(ud) + + email := um.Email.String + err := r.client.RunWithinTx(ctx, func(*sqlx.Tx) error { + usr, err := r.GetByEmail(ctx, email) + if err == nil { + userID = usr.ID + return nil + } + + userID, err = r.InsertByEmail(ctx, &user.User{Email: email}) + return err + }) + return userID, err } func getUserByPredicate(ctx context.Context, querier sqlx.QueryerContext, pred sq.Eq) (user.User, error) { - qry, args, err := sq.Select("id", "uuid", "email", "provider", "created_at", "updated_at"). + qry, args, err := sq.Select("id", "email", "provider", "created_at", "updated_at"). From("users"). Where(pred). PlaceholderFormat(sq.Dollar). diff --git a/internal/store/postgres/user_repository_test.go b/internal/store/postgres/user_repository_test.go index 4e25bbf3..eafb3c12 100644 --- a/internal/store/postgres/user_repository_test.go +++ b/internal/store/postgres/user_repository_test.go @@ -5,7 +5,6 @@ import ( "fmt" "testing" - "github.com/google/uuid" "github.com/goto/compass/core/user" "github.com/goto/compass/internal/store/postgres" "github.com/goto/compass/internal/testutils" @@ -77,30 +76,15 @@ func (r *UserRepositoryTestSuite) TestCreate() { } func (r *UserRepositoryTestSuite) TestCreateWithTx() { - validUserWithoutUUID := &user.User{ + validUser := &user.User{ Email: "userWithTx@gotocompany.com", Provider: "compass", } - validUserWithoutEmail := &user.User{ - UUID: "a-uuid", - Provider: "compass", - } - r.Run("return no error if succesfully create user without uuid", func() { + r.Run("return no error if successfully create user with email", func() { var id string err := r.client.RunWithinTx(r.ctx, func(tx *sqlx.Tx) error { var err error - id, err = r.repository.CreateWithTx(r.ctx, tx, validUserWithoutUUID) - return err - }) - r.NotEmpty(id) - r.NoError(err) - }) - - r.Run("return no error if succesfully create user without email", func() { - var id string - err := r.client.RunWithinTx(r.ctx, func(tx *sqlx.Tx) error { - var err error - id, err = r.repository.CreateWithTx(r.ctx, tx, validUserWithoutEmail) + id, err = r.repository.CreateWithTx(r.ctx, tx, validUser) return err }) r.NotEmpty(id) @@ -122,13 +106,13 @@ func (r *UserRepositoryTestSuite) TestCreateWithTx() { err := testutils.RunMigrationsWithClient(r.T(), r.client) r.NoError(err) - id, err := r.repository.Create(r.ctx, validUserWithoutUUID) + id, err := r.repository.Create(r.ctx, validUser) r.NoError(err) r.NotEmpty(id) err = r.client.RunWithinTx(r.ctx, func(tx *sqlx.Tx) error { var err error - id, err = r.repository.CreateWithTx(r.ctx, tx, validUserWithoutUUID) + id, err = r.repository.CreateWithTx(r.ctx, tx, validUser) return err }) r.ErrorAs(err, new(user.DuplicateRecordError)) @@ -179,125 +163,65 @@ func (r *UserRepositoryTestSuite) TestGetBy() { }) r.NoError(err) }) - - r.Run("by uuid", func() { - r.Run("return empty string and ErrNotFound if uuid not found in DB", func() { - usr, err := r.repository.GetByUUID(r.ctx, "random") - r.ErrorAs(err, new(user.NotFoundError)) - r.Empty(usr) - }) - - r.Run("return non empty user if email found in DB", func() { - err := testutils.RunMigrationsWithClient(r.T(), r.client) - r.NoError(err) - - user := getUser("use-getbyuuid@gotocompany.com") - id, err := r.repository.Create(r.ctx, user) - r.NoError(err) - r.NotEmpty(id) - - usr, err := r.repository.GetByUUID(r.ctx, user.UUID) - r.NoError(err) - r.NotEmpty(usr) - }) - }) - - r.Run("by UUID with tx, return the user created in the tx", func() { - err := testutils.RunMigrationsWithClient(r.T(), r.client) - r.NoError(err) - - u := getUser("use-getbyuuid@gotocompany.com") - err = r.client.RunWithinTx(r.ctx, func(tx *sqlx.Tx) error { - id, err := r.repository.CreateWithTx(r.ctx, tx, u) - r.NoError(err) - - _, err = r.repository.GetByUUID(r.ctx, u.UUID) - r.ErrorAs(err, new(user.NotFoundError)) - - u, err := r.repository.GetByUUIDWithTx(r.ctx, tx, u.UUID) - r.NoError(err) - r.Equal(id, u.ID) - - return nil - }) - r.NoError(err) - }) } -func (r *UserRepositoryTestSuite) TestUpsertByEmail() { +func (r *UserRepositoryTestSuite) TestGetOrInsertByEmail() { r.Run("return ErrNoUserInformation if user is nil", func() { - id, err := r.repository.UpsertByEmail(r.ctx, nil) + id, err := r.repository.GetOrInsertByEmail(r.ctx, nil) r.ErrorIs(err, user.ErrNoUserInformation) r.Empty(id) }) - r.Run("return ErrDuplicateRecord if record already exist", func() { - usr := &user.User{UUID: uuid.NewString(), Email: "dummy@gotocompany.com"} + r.Run("return user ID if record already exist", func() { + usr := &user.User{Email: "dummy@gotocompany.com"} err := r.insertEmail(usr.Email) r.NoError(err) - usr.UUID = uuid.NewString() - id, err := r.repository.UpsertByEmail(r.ctx, usr) + id, err := r.repository.GetOrInsertByEmail(r.ctx, usr) r.NoError(err) r.NotEmpty(id) - - id, err = r.repository.UpsertByEmail(r.ctx, usr) - r.ErrorIs(err, user.DuplicateRecordError{UUID: usr.UUID, Email: usr.Email}) - r.Empty(id) }) - r.Run("new row is inserted with uuid and email if user not exist", func() { - usr := &user.User{UUID: uuid.NewString(), Email: "user-upsert-1@gotocompany.com"} - id, err := r.repository.UpsertByEmail(r.ctx, usr) + r.Run("new row is inserted with email if user not exist", func() { + usr := &user.User{Email: "user-upsert-1@gotocompany.com"} + id, err := r.repository.GetOrInsertByEmail(r.ctx, usr) r.NoError(err) r.NotEmpty(id) - gotUser, err := r.repository.GetByUUID(r.ctx, usr.UUID) + gotUser, err := r.repository.GetByEmail(r.ctx, usr.Email) r.NoError(err) - r.Equal(gotUser.UUID, usr.UUID) r.Equal(gotUser.Email, usr.Email) }) +} - r.Run("new row is inserted with uuid only if user not exist", func() { - usr := &user.User{UUID: uuid.NewString()} - id, err := r.repository.UpsertByEmail(r.ctx, usr) - r.NoError(err) - r.NotEmpty(id) - - gotUser, err := r.repository.GetByUUID(r.ctx, usr.UUID) - r.NoError(err) - r.Equal(gotUser.UUID, usr.UUID) - r.Equal(gotUser.Email, usr.Email) +func (r *UserRepositoryTestSuite) TestInsertByEmail() { + r.Run("return ErrNoUserInformation if user is nil", func() { + id, err := r.repository.InsertByEmail(r.ctx, nil) + r.ErrorIs(err, user.ErrNoUserInformation) + r.Empty(id) }) - r.Run("upserting existing row with empty uuid is upserted with uuid and email", func() { - usr := &user.User{Email: "user-upsert-2@gotocompany.com"} + r.Run("return ErrDuplicateRecord if record already exist", func() { + usr := &user.User{Email: "dummy-insert@gotocompany.com"} err := r.insertEmail(usr.Email) r.NoError(err) - usr.UUID = uuid.NewString() - id, err := r.repository.UpsertByEmail(r.ctx, usr) - r.NoError(err) - r.NotEmpty(id) - - gotUser, err := r.repository.GetByUUID(r.ctx, usr.UUID) - r.NoError(err) - r.Equal(gotUser.UUID, usr.UUID) - r.Equal(gotUser.Email, usr.Email) + id, err := r.repository.InsertByEmail(r.ctx, usr) + r.ErrorIs(err, user.DuplicateRecordError{Email: usr.Email}) + r.Empty(id) }) - r.Run("upserting existing row with non empty uuid would return error", func() { - usr := &user.User{UUID: uuid.NewString(), Email: "user-upsert-3@gotocompany.com"} - - id, err := r.repository.Create(r.ctx, usr) + r.Run("new row is inserted with email if user not exist", func() { + usr := &user.User{Email: "user-insert-1@gotocompany.com"} + id, err := r.repository.InsertByEmail(r.ctx, usr) r.NoError(err) r.NotEmpty(id) - id, err = r.repository.UpsertByEmail(r.ctx, usr) - r.Error(err) - r.Empty(id) + gotUser, err := r.repository.GetByEmail(r.ctx, usr.Email) + r.NoError(err) + r.Equal(gotUser.Email, usr.Email) }) } diff --git a/pkg/grpc_interceptor/interceptor_test.go b/pkg/grpc_interceptor/interceptor_test.go index 833791e9..048a6f7d 100644 --- a/pkg/grpc_interceptor/interceptor_test.go +++ b/pkg/grpc_interceptor/interceptor_test.go @@ -16,8 +16,8 @@ type dummyService struct { func (s *dummyService) Ping(ctx context.Context, ping *pb_testproto.PingRequest) (*pb_testproto.PingResponse, error) { if ping.Value == "testuser" { usr := user.FromContext(ctx) - if usr.UUID == "" { - return nil, status.Error(codes.InvalidArgument, "uuid not found") + if usr.Email == "" { + return nil, status.Error(codes.InvalidArgument, "email not found") } } return s.TestServiceServer.Ping(ctx, ping) diff --git a/pkg/grpc_interceptor/user.go b/pkg/grpc_interceptor/user.go index 2edf16e0..9b46ede6 100644 --- a/pkg/grpc_interceptor/user.go +++ b/pkg/grpc_interceptor/user.go @@ -9,31 +9,22 @@ import ( "google.golang.org/grpc/metadata" ) -// UserHeaderCtx middleware will propagate a valid user ID as string -// within request context +// UserHeaderCtx middleware will propagate a valid user ID as string within request context // use `user.FromContext` function to get the user ID string -func UserHeaderCtx(IdentityHeaderKeyUUID, IdentityHeaderKeyEmail string) grpc.UnaryServerInterceptor { +func UserHeaderCtx(identityHeaderKeyEmail string) grpc.UnaryServerInterceptor { return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) { - var ( - userUUID = "" - userEmail = "" - ) + userEmail := "" md, ok := metadata.FromIncomingContext(ctx) if !ok { return "", fmt.Errorf("metadata in grpc doesn't exist") } - metadataValues := md.Get(IdentityHeaderKeyUUID) - if len(metadataValues) > 0 { - userUUID = metadataValues[0] - } - - metadataValues = md.Get(IdentityHeaderKeyEmail) + metadataValues := md.Get(identityHeaderKeyEmail) if len(metadataValues) > 0 { userEmail = metadataValues[0] } - newCtx := user.NewContext(ctx, user.User{UUID: userUUID, Email: userEmail}) + newCtx := user.NewContext(ctx, user.User{Email: userEmail}) return handler(newCtx, req) } } diff --git a/pkg/grpc_interceptor/user_test.go b/pkg/grpc_interceptor/user_test.go index 2ab266ba..41426817 100644 --- a/pkg/grpc_interceptor/user_test.go +++ b/pkg/grpc_interceptor/user_test.go @@ -30,7 +30,7 @@ func TestUserSuite(t *testing.T) { TestService: &dummyService{TestServiceServer: &grpc_testing.TestPingService{T: t}}, ServerOpts: []grpc.ServerOption{ grpc_middleware.WithUnaryServerChain( - UserHeaderCtx(IdentityHeaderKeyUUID, IdentityHeaderKeyEmail)), + UserHeaderCtx(IdentityHeaderKeyEmail)), }, }, } @@ -41,22 +41,21 @@ func (s *UserTestSuite) TestUnary_IdentityHeaderNotPresent() { _, err := s.Client.Ping(s.SimpleCtx(), &pb_testproto.PingRequest{Value: "testuser", SleepTimeMs: 9999}) code := status.Code(err) require.Equal(s.T(), codes.InvalidArgument, code) - require.EqualError(s.T(), err, "rpc error: code = InvalidArgument desc = uuid not found") + require.EqualError(s.T(), err, "rpc error: code = InvalidArgument desc = email not found") } func (s *UserTestSuite) TestUnary_HeaderPresentAndEmpty() { - ctx := metadata.AppendToOutgoingContext(context.Background(), IdentityHeaderKeyUUID, "", IdentityHeaderKeyEmail, "") + ctx := metadata.AppendToOutgoingContext(context.Background(), IdentityHeaderKeyEmail, "") _, err := s.Client.Ping(ctx, &pb_testproto.PingRequest{Value: "testuser", SleepTimeMs: 9999}) code := status.Code(err) require.Equal(s.T(), codes.InvalidArgument, code) - require.EqualError(s.T(), err, "rpc error: code = InvalidArgument desc = uuid not found") + require.EqualError(s.T(), err, "rpc error: code = InvalidArgument desc = email not found") } func (s *UserTestSuite) TestUnary_HeaderPresentAndPassed() { userEmail := "user-email" - userUUID := "user-uuid" - ctx := metadata.AppendToOutgoingContext(s.SimpleCtx(), IdentityHeaderKeyUUID, userUUID, IdentityHeaderKeyEmail, userEmail) + ctx := metadata.AppendToOutgoingContext(s.SimpleCtx(), IdentityHeaderKeyEmail, userEmail) _, err := s.Client.Ping(ctx, &pb_testproto.PingRequest{Value: "testuser", SleepTimeMs: 9999}) code := status.Code(err) require.Equal(s.T(), codes.OK, code) diff --git a/test/e2e_test/helper_test.go b/test/e2e_test/helper_test.go index e9ff5c74..6f351474 100644 --- a/test/e2e_test/helper_test.go +++ b/test/e2e_test/helper_test.go @@ -15,7 +15,6 @@ import ( var ( SERVER_HOST = "http://localhost:8080" - IDENTITY_HEADER_KEY_UUID = "Compass-User-UUID" IDENTITY_HEADER_KEY_EMAIL = "Compass-User-Email" ) @@ -129,7 +128,6 @@ func (c *Client) makeRequest(method, url string, payload, data interface{}) (err } req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json") - req.Header.Set(IDENTITY_HEADER_KEY_UUID, "compassendtoendtest@gotocompany.com") req.Header.Set(IDENTITY_HEADER_KEY_EMAIL, "compassendtoendtest@gotocompany.com") for key, value := range c.headers {