feat: enable upsert behaviour (#163)

* feat: enable upsert behaviour

* feat: add metadata table - create & getbyid

* feat: add metadata table - updatebyemail, updatebyid, getbyemail

* feat: add metadata table - list

* fix: getbyids

* test: fixed existing tests

* test- fix updatebyemail & updatebyid

* test: fix existing tests

* fix: nil error handling

* test: remove no longer valid tests

* feat: add createmetadatakey api

* fix: transform metadata value to json string

* test: add createmetadatakey api test

* chore: remove unused errors

* chore: add newline in sql files

* refactor: remove id attribute from metadata keys table

* refactor: change struct name

* fix: unnecessary backslash being added

* chore: lint issues

core/user/errors.go

@@ -3,10 +3,12 @@ package user
 import "errors"
 
 var (
-	ErrNotExist     = errors.New("user doesn't exist")
-	ErrInvalidID    = errors.New("user id is invalid")
-	ErrInvalidEmail = errors.New("user email is invalid")
-	ErrMissingEmail = errors.New("user email is missing")
-	ErrInvalidUUID  = errors.New("invalid syntax of uuid")
-	ErrConflict     = errors.New("user already exist")
+	ErrNotExist         = errors.New("user doesn't exist")
+	ErrInvalidID        = errors.New("user id is invalid")
+	ErrInvalidEmail     = errors.New("user email is invalid")
+	ErrConflict         = errors.New("user already exist")
+	ErrEmptyKey         = errors.New("empty key")
+	ErrKeyAlreadyExists = errors.New("key already exist")
+	ErrMissingEmail     = errors.New("user email is missing")
+	ErrInvalidUUID      = errors.New("invalid syntax of uuid")
 )

core/user/service.go

@@ -40,6 +40,18 @@ func (s Service) Create(ctx context.Context, user User) (User, error) {
 	return newUser, nil
 }
 
+func (s Service) CreateMetadataKey(ctx context.Context, key UserMetadataKey) (UserMetadataKey, error) {
+	newUserMetadataKey, err := s.repository.CreateMetadataKey(ctx, UserMetadataKey{
+		Key:         key.Key,
+		Description: key.Description,
+	})
+	if err != nil {
+		return UserMetadataKey{}, err
+	}
+
+	return newUserMetadataKey, nil
+}
+
 func (s Service) List(ctx context.Context, flt Filter) (PagedUsers, error) {
 	users, err := s.repository.List(ctx, flt)
 	if err != nil {

core/user/user.go

@@ -15,6 +15,7 @@ type Repository interface {
 	List(ctx context.Context, flt Filter) ([]User, error)
 	UpdateByID(ctx context.Context, toUpdate User) (User, error)
 	UpdateByEmail(ctx context.Context, toUpdate User) (User, error)
+	CreateMetadataKey(ctx context.Context, key UserMetadataKey) (UserMetadataKey, error)
 }
 
 type User struct {
@@ -26,6 +27,13 @@ type User struct {
 	UpdatedAt time.Time
 }
 
+type UserMetadataKey struct {
+	Key         string
+	Description string
+	CreatedAt   time.Time
+	UpdatedAt   time.Time
+}
+
 type PagedUsers struct {
 	Count int32
 	Users []User

internal/api/v1beta1/errors.go

@@ -13,12 +13,11 @@ var (
 	ErrBadRequest            = errors.New("invalid syntax in body")
 	ErrConflictRequest       = errors.New("already exist")
 	ErrRequestBodyValidation = errors.New("invalid format for field(s)")
+	ErrEmptyEmailID          = errors.New("email id is empty")
 
 	grpcInternalServerError = status.Errorf(codes.Internal, ErrInternalServer.Error())
 	grpcConflictError       = status.Errorf(codes.AlreadyExists, ErrConflictRequest.Error())
 	grpcBadBodyError        = status.Error(codes.InvalidArgument, ErrBadRequest.Error())
 	grpcPermissionDenied    = status.Error(codes.PermissionDenied, errors.ErrForbidden.Error())
 	grpcUnauthenticated     = status.Error(codes.Unauthenticated, errors.ErrUnauthenticated.Error())
-
-	ErrEmptyEmailID = errors.New("email id is empty")
 )

internal/api/v1beta1/user.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/odpf/shield/core/user"
 	"github.com/odpf/shield/pkg/metadata"
+	"github.com/odpf/shield/pkg/uuid"
 	shieldv1beta1 "github.com/odpf/shield/proto/v1beta1"
 )
 
@@ -27,6 +28,7 @@ type UserService interface {
 	UpdateByID(ctx context.Context, toUpdate user.User) (user.User, error)
 	UpdateByEmail(ctx context.Context, toUpdate user.User) (user.User, error)
 	FetchCurrentUser(ctx context.Context) (user.User, error)
+	CreateMetadataKey(ctx context.Context, key user.UserMetadataKey) (user.UserMetadataKey, error)
 }
 
 func (h Handler) ListUsers(ctx context.Context, request *shieldv1beta1.ListUsersRequest) (*shieldv1beta1.ListUsersResponse, error) {
@@ -121,6 +123,33 @@ func (h Handler) CreateUser(ctx context.Context, request *shieldv1beta1.CreateUs
 	}}, nil
 }
 
+func (h Handler) CreateMetadataKey(ctx context.Context, request *shieldv1beta1.CreateMetadataKeyRequest) (*shieldv1beta1.CreateMetadataKeyResponse, error) {
+	logger := grpczap.Extract(ctx)
+
+	if request.GetBody() == nil {
+		return nil, grpcBadBodyError
+	}
+
+	newKey, err := h.userService.CreateMetadataKey(ctx, user.UserMetadataKey{
+		Key:         request.GetBody().GetKey(),
+		Description: request.GetBody().GetDescription(),
+	})
+	if err != nil {
+		logger.Error(err.Error())
+		switch {
+		case errors.Is(err, user.ErrConflict):
+			return nil, grpcConflictError
+		default:
+			return nil, grpcInternalServerError
+		}
+	}
+
+	return &shieldv1beta1.CreateMetadataKeyResponse{Metadatakey: &shieldv1beta1.MetadataKey{
+		Key:         newKey.Key,
+		Description: newKey.Description,
+	}}, nil
+}
+
 func (h Handler) GetUser(ctx context.Context, request *shieldv1beta1.GetUserRequest) (*shieldv1beta1.GetUserResponse, error) {
 	logger := grpczap.Extract(ctx)
 
@@ -184,6 +213,7 @@ func (h Handler) GetCurrentUser(ctx context.Context, request *shieldv1beta1.GetC
 
 func (h Handler) UpdateUser(ctx context.Context, request *shieldv1beta1.UpdateUserRequest) (*shieldv1beta1.UpdateUserResponse, error) {
 	logger := grpczap.Extract(ctx)
+	var updatedUser user.User
 
 	if strings.TrimSpace(request.GetId()) == "" {
 		return nil, grpcUserNotFoundError
@@ -203,22 +233,52 @@ func (h Handler) UpdateUser(ctx context.Context, request *shieldv1beta1.UpdateUs
 		return nil, grpcBadBodyError
 	}
 
-	updatedUser, err := h.userService.UpdateByID(ctx, user.User{
-		ID:       request.GetId(),
-		Name:     request.GetBody().GetName(),
-		Email:    email,
-		Metadata: metaDataMap,
-	})
-	if err != nil {
-		logger.Error(err.Error())
-		switch {
-		case errors.Is(err, user.ErrNotExist), errors.Is(err, user.ErrInvalidID), errors.Is(err, user.ErrInvalidUUID):
-			return nil, grpcUserNotFoundError
-		case errors.Is(err, user.ErrInvalidEmail):
-			return nil, grpcBadBodyError
-		case errors.Is(err, user.ErrConflict):
-			return nil, grpcConflictError
-		default:
+	id := request.GetId()
+	if uuid.IsValid(id) {
+		updatedUser, err = h.userService.UpdateByID(ctx, user.User{
+			ID:       request.GetId(),
+			Name:     request.GetBody().GetName(),
+			Email:    request.GetBody().GetEmail(),
+			Metadata: metaDataMap,
+		})
+		if err != nil {
+			logger.Error(err.Error())
+			switch {
+			case errors.Is(err, user.ErrNotExist), errors.Is(err, user.ErrInvalidID), errors.Is(err, user.ErrInvalidUUID):
+				return nil, grpcUserNotFoundError
+			case errors.Is(err, user.ErrInvalidEmail):
+				return nil, grpcBadBodyError
+			case errors.Is(err, user.ErrConflict):
+				return nil, grpcConflictError
+			default:
+				return nil, grpcInternalServerError
+			}
+		}
+	} else {
+		fetchedUser, err := h.userService.GetByEmail(ctx, id)
+		if err != nil {
+			if err == user.ErrNotExist {
+				createUserResponse, err := h.CreateUser(ctx, &shieldv1beta1.CreateUserRequest{Body: request.GetBody()})
+				if err != nil {
+					return nil, grpcInternalServerError
+				}
+				return &shieldv1beta1.UpdateUserResponse{User: createUserResponse.User}, nil
+			} else {
+				return nil, grpcInternalServerError
+			}
+		}
+
+		for key, value := range metaDataMap {
+			fetchedUser.Metadata[key] = value
+		}
+
+		updatedUser, err = h.userService.UpdateByEmail(ctx, user.User{
+			Name:     request.GetBody().GetName(),
+			Email:    request.GetBody().GetEmail(),
+			Metadata: fetchedUser.Metadata,
+		})
+		if err != nil {
+			logger.Error(err.Error())
 			return nil, grpcInternalServerError
 		}
 	}

internal/api/v1beta1/user_test.go

@@ -493,32 +493,6 @@ func TestUpdateUser(t *testing.T) {
 			want: nil,
 			err:  grpcInternalServerError,
 		},
-		{
-			title: "should return not found error if id is not uuid",
-			setup: func(us *mocks.UserService) {
-				us.EXPECT().UpdateByID(mock.AnythingOfType("*context.emptyCtx"), user.User{
-					ID:    "some-id",
-					Name:  "abc user",
-					Email: "[email protected]",
-					Metadata: metadata.Metadata{
-						"foo": "bar",
-					},
-				}).Return(user.User{}, user.ErrInvalidUUID)
-			},
-			req: &shieldv1beta1.UpdateUserRequest{
-				Id: "some-id",
-				Body: &shieldv1beta1.UserRequestBody{
-					Name:  "abc user",
-					Email: "[email protected]",
-					Metadata: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							"foo": structpb.NewStringValue("bar"),
-						},
-					},
-				}},
-			want: nil,
-			err:  grpcUserNotFoundError,
-		},
 		{
 			title: "should return not found error if id is invalid",
 			setup: func(us *mocks.UserService) {
@@ -543,32 +517,6 @@ func TestUpdateUser(t *testing.T) {
 			want: nil,
 			err:  grpcUserNotFoundError,
 		},
-		{
-			title: "should return not found error if user not exist",
-			setup: func(us *mocks.UserService) {
-				us.EXPECT().UpdateByID(mock.AnythingOfType("*context.emptyCtx"), user.User{
-					ID:    "some-id",
-					Name:  "abc user",
-					Email: "[email protected]",
-					Metadata: metadata.Metadata{
-						"foo": "bar",
-					},
-				}).Return(user.User{}, user.ErrNotExist)
-			},
-			req: &shieldv1beta1.UpdateUserRequest{
-				Id: "some-id",
-				Body: &shieldv1beta1.UserRequestBody{
-					Name:  "abc user",
-					Email: "[email protected]",
-					Metadata: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							"foo": structpb.NewStringValue("bar"),
-						},
-					},
-				}},
-			want: nil,
-			err:  grpcUserNotFoundError,
-		},
 		{
 			title: "should return already exist error if user service return error conflict",
 			setup: func(us *mocks.UserService) {
@@ -728,7 +676,7 @@ func TestUpdateUser(t *testing.T) {
 			mockDep := Handler{userService: mockUserSrv}
 			resp, err := mockDep.UpdateUser(ctx, tt.req)
 			assert.EqualValues(t, resp, tt.want)
-			assert.EqualValues(t, err, tt.err)
+			assert.EqualValues(t, tt.err, err)
 		})
 	}
 }
@@ -971,3 +919,58 @@ func TestHandler_ListUserGroups(t *testing.T) {
 		})
 	}
 }
+
+func TestCreateMetadataKey(t *testing.T) {
+	email := "[email protected]"
+	table := []struct {
+		title string
+		setup func(ctx context.Context, us *mocks.UserService) context.Context
+		req   *shieldv1beta1.CreateMetadataKeyRequest
+		want  *shieldv1beta1.CreateMetadataKeyResponse
+		err   error
+	}{
+		{
+			title: "should return error if body is empty",
+			setup: func(ctx context.Context, us *mocks.UserService) context.Context {
+				us.EXPECT().CreateMetadataKey(mock.AnythingOfType("*context.valueCtx"), shieldv1beta1.CreateMetadataKeyRequest{Body: nil}).Return(user.UserMetadataKey{}, grpcBadBodyError)
+				return user.SetContextWithEmail(ctx, email)
+			},
+			req:  &shieldv1beta1.CreateMetadataKeyRequest{Body: nil},
+			want: nil,
+			err:  grpcBadBodyError,
+		},
+		{
+			title: "should return error conflict if key already exists",
+			setup: func(ctx context.Context, us *mocks.UserService) context.Context {
+				us.EXPECT().CreateMetadataKey(mock.AnythingOfType("*context.valueCtx"), user.UserMetadataKey{
+					Key:         "k1",
+					Description: "key one",
+				}).Return(user.UserMetadataKey{}, user.ErrConflict)
+				return user.SetContextWithEmail(ctx, email)
+			},
+			req: &shieldv1beta1.CreateMetadataKeyRequest{Body: &shieldv1beta1.MetadataKeyRequestBody{
+				Key:         "k1",
+				Description: "key one",
+			}},
+			want: nil,
+			err:  grpcConflictError,
+		},
+	}
+
+	for _, tt := range table {
+		t.Run(tt.title, func(t *testing.T) {
+			var resp *shieldv1beta1.CreateMetadataKeyResponse
+			var err error
+
+			ctx := context.Background()
+			mockUserSrv := new(mocks.UserService)
+			if tt.setup != nil {
+				ctx = tt.setup(ctx, mockUserSrv)
+			}
+			mockDep := Handler{userService: mockUserSrv}
+			resp, err = mockDep.CreateMetadataKey(ctx, tt.req)
+			assert.EqualValues(t, tt.want, resp)
+			assert.EqualValues(t, tt.err, err)
+		})
+	}
+}

internal/store/postgres/group_repository.go

@@ -294,7 +294,6 @@ func (r GroupRepository) buildListUsersByGroupIDQuery(groupID, roleID string) (s
 		goqu.I("u.id").As("id"),
 		goqu.I("u.name").As("name"),
 		goqu.I("u.email").As("email"),
-		goqu.I("u.metadata").As("metadata"),
 		goqu.I("u.created_at").As("created_at"),
 		goqu.I("u.updated_at").As("updated_at"),
 	).