Fix lock POST API to handle the constraint error in the store (#229)

* Add a new method 'GetEnv'

* Fix lock POST API to simplify

Author: Noah Hanjun Lee

internal/interactor/config.go

@@ -0,0 +1,22 @@
+package interactor
+
+import (
+	"context"
+
+	"github.com/gitploy-io/gitploy/ent"
+	"github.com/gitploy-io/gitploy/pkg/e"
+	"github.com/gitploy-io/gitploy/vo"
+)
+
+func (i *Interactor) GetEnv(ctx context.Context, u *ent.User, r *ent.Repo, env string) (*vo.Env, error) {
+	config, err := i.SCM.GetConfig(ctx, u, r)
+	if err != nil {
+		return nil, err
+	}
+
+	if !config.HasEnv(env) {
+		return nil, e.NewError(e.ErrorCodeConfigUndefinedEnv, nil)
+	}
+
+	return config.GetEnv(env), nil
+}

internal/interactor/config_test.go

@@ -0,0 +1,57 @@
+package interactor
+
+import (
+	"context"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+
+	"github.com/gitploy-io/gitploy/ent"
+	"github.com/gitploy-io/gitploy/internal/interactor/mock"
+	"github.com/gitploy-io/gitploy/pkg/e"
+	"github.com/gitploy-io/gitploy/vo"
+)
+
+func TestInteractor_GetEnv(t *testing.T) {
+	t.Run("Return an error when the environment is not defined.", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		scm := mock.NewMockSCM(ctrl)
+
+		scm.
+			EXPECT().
+			GetConfig(gomock.Any(), gomock.AssignableToTypeOf(&ent.User{}), gomock.AssignableToTypeOf(&ent.Repo{})).
+			Return(&vo.Config{
+				Envs: []*vo.Env{},
+			}, nil)
+
+		i := &Interactor{SCM: scm}
+
+		_, err := i.GetEnv(context.Background(), &ent.User{}, &ent.Repo{}, "production")
+		if !e.HasErrorCode(err, e.ErrorCodeConfigUndefinedEnv) {
+			t.Fatalf("GetEnv error = %v, wanted ErrorCodeConfigUndefinedEnv", err)
+		}
+	})
+
+	t.Run("Return the environment.", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		scm := mock.NewMockSCM(ctrl)
+
+		scm.
+			EXPECT().
+			GetConfig(gomock.Any(), gomock.AssignableToTypeOf(&ent.User{}), gomock.AssignableToTypeOf(&ent.Repo{})).
+			Return(&vo.Config{
+				Envs: []*vo.Env{
+					{
+						Name: "production",
+					},
+				},
+			}, nil)
+
+		i := &Interactor{SCM: scm}
+
+		_, err := i.GetEnv(context.Background(), &ent.User{}, &ent.Repo{}, "production")
+		if err != nil {
+			t.Fatalf("GetEnv returns an error: %v", err)
+		}
+	})
+}

internal/pkg/store/lock.go

@@ -106,6 +106,8 @@ func (s *Store) CreateLock(ctx context.Context, l *ent.Lock) (*ent.Lock, error)
 			e.ErrorCodeEntityUnprocessable,
 			fmt.Sprintf("Failed to create a lock. The value of \"%s\" field is invalid.", err.(*ent.ValidationError).Name),
 			err)
+	} else if ent.IsConstraintError(err) {
+		return nil, e.NewError(e.ErrorCodeLockAlreadyExist, err)
 	} else if err != nil {
 		return nil, e.NewError(e.ErrorCodeInternalError, err)
 	}

internal/pkg/store/lock_test.go

@@ -5,8 +5,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gitploy-io/gitploy/ent"
 	"github.com/gitploy-io/gitploy/ent/enttest"
 	"github.com/gitploy-io/gitploy/ent/migrate"
+	"github.com/gitploy-io/gitploy/pkg/e"
 )
 
 func TestStore_ListExpiredLocksLessThanTime(t *testing.T) {
@@ -56,3 +58,53 @@ func TestStore_ListExpiredLocksLessThanTime(t *testing.T) {
 		}
 	})
 }
+
+func TestStore_CreateLock(t *testing.T) {
+	t.Run("Return an error when the lock is exist.", func(t *testing.T) {
+		ctx := context.Background()
+
+		client := enttest.Open(t, "sqlite3", "file:ent?mode=memory&cache=shared&_fk=1",
+			enttest.WithMigrateOptions(migrate.WithForeignKeys(false)),
+		)
+		defer client.Close()
+
+		t.Log("Lock the production environment.")
+		client.Lock.
+			Create().
+			SetEnv("production").
+			SetRepoID(1).
+			SetUserID(1).
+			SaveX(ctx)
+
+		s := NewStore(client)
+
+		t.Log("Lock the production environment again.")
+		if _, err := s.CreateLock(ctx, &ent.Lock{
+			Env:    "production",
+			RepoID: 1,
+			UserID: 1,
+		}); !e.HasErrorCode(err, e.ErrorCodeLockAlreadyExist) {
+			t.Fatalf("Error = %v, wanted ErrorCodeEntityUnprocessable", err)
+		}
+	})
+
+	t.Run("Lock the environment.", func(t *testing.T) {
+		ctx := context.Background()
+
+		client := enttest.Open(t, "sqlite3", "file:ent?mode=memory&cache=shared&_fk=1",
+			enttest.WithMigrateOptions(migrate.WithForeignKeys(false)),
+		)
+		defer client.Close()
+
+		s := NewStore(client)
+
+		_, err := s.CreateLock(ctx, &ent.Lock{
+			Env:    "production",
+			RepoID: 1,
+			UserID: 1,
+		})
+		if err != nil {
+			t.Fatalf("CreateLock retruns an error: %v", err)
+		}
+	})
+}

internal/server/api/v1/repos/interface.go

@@ -31,6 +31,7 @@ type (
 		Deploy(ctx context.Context, u *ent.User, re *ent.Repo, d *ent.Deployment, env *vo.Env) (*ent.Deployment, error)
 		DeployToRemote(ctx context.Context, u *ent.User, r *ent.Repo, d *ent.Deployment, env *vo.Env) (*ent.Deployment, error)
 		GetConfig(ctx context.Context, u *ent.User, r *ent.Repo) (*vo.Config, error)
+		GetEnv(ctx context.Context, u *ent.User, r *ent.Repo, env string) (*vo.Env, error)
 
 		ListReviews(ctx context.Context, d *ent.Deployment) ([]*ent.Review, error)
 		FindReviewOfUser(ctx context.Context, u *ent.User, d *ent.Deployment) (*ent.Review, error)

internal/server/api/v1/repos/lock.go

@@ -84,7 +84,7 @@ func (r *Repo) CreateLock(c *gin.Context) {
 	vu, _ := c.Get(global.KeyUser)
 	u := vu.(*ent.User)
 
-	cfg, err := r.i.GetConfig(ctx, u, re)
+	env, err := r.i.GetEnv(ctx, u, re, p.Env)
 	if e.HasErrorCode(err, e.ErrorCodeEntityNotFound) {
 		r.log.Check(gb.GetZapLogLevel(err), "The configuration file is not found.").Write(zap.Error(err))
 		// To override the HTTP status 422.
@@ -96,32 +96,8 @@ func (r *Repo) CreateLock(c *gin.Context) {
 		return
 	}
 
-	if !cfg.HasEnv(p.Env) {
-		r.log.Warn("The environment is not defined in the configuration.")
-		gb.ResponseWithError(
-			c,
-			e.NewErrorWithMessage(e.ErrorCodeConfigParseError, "The environment is not defiend in the configuration.", nil),
-		)
-		return
-	}
-
-	// TODO: migrate the business logic into the interactor.
-	if ok, err := r.i.HasLockOfRepoForEnv(ctx, re, p.Env); ok {
-		r.log.Warn("The lock already exist.", zap.String("env", p.Env))
-		gb.ResponseWithError(
-			c,
-			e.NewErrorWithMessage(e.ErrorCodeEntityUnprocessable, "The lock already exist.", err),
-		)
-		return
-	} else if err != nil {
-		r.log.Check(gb.GetZapLogLevel(err), "Failed to check the lock.").Write(zap.Error(err))
-		gb.ResponseWithError(c, err)
-		return
-	}
-
-	// Lock the environment.
 	l, err := r.i.CreateLock(ctx, &ent.Lock{
-		Env:       p.Env,
+		Env:       env.Name,
 		ExpiredAt: expiredAt,
 		UserID:    u.ID,
 		RepoID:    re.ID,
@@ -136,7 +112,7 @@ func (r *Repo) CreateLock(c *gin.Context) {
 		l = nl
 	}
 
-	r.log.Debug("Lock the env.", zap.String("env", p.Env))
+	r.log.Info("Lock the environment.", zap.String("repo", re.GetFullName()), zap.String("env", p.Env), zap.String("login", u.Login))
 	gb.Response(c, http.StatusCreated, l)
 }
 
@@ -201,6 +177,7 @@ func (r *Repo) UpdateLock(c *gin.Context) {
 		l = nl
 	}
 
+	r.log.Info("Patch the environment lock.", zap.Int("lock_id", l.ID))
 	gb.Response(c, http.StatusOK, l)
 }
 

internal/server/api/v1/repos/lock_test.go

@@ -17,11 +17,12 @@ import (
 	"github.com/gitploy-io/gitploy/ent"
 	"github.com/gitploy-io/gitploy/internal/server/api/v1/repos/mock"
 	"github.com/gitploy-io/gitploy/internal/server/global"
+	"github.com/gitploy-io/gitploy/pkg/e"
 	"github.com/gitploy-io/gitploy/vo"
 )
 
 func TestRepo_CreateLock(t *testing.T) {
-	t.Run("Return 422 when the env is not found", func(t *testing.T) {
+	t.Run("Return 422 when the environment is undefined.", func(t *testing.T) {
 		input := struct {
 			payload *lockPostPayload
 		}{
@@ -36,14 +37,8 @@ func TestRepo_CreateLock(t *testing.T) {
 		t.Log("Read deploy.yml and check the env.")
 		m.
 			EXPECT().
-			GetConfig(gomock.Any(), gomock.AssignableToTypeOf(&ent.User{}), gomock.AssignableToTypeOf(&ent.Repo{})).
-			Return(&vo.Config{
-				Envs: []*vo.Env{
-					{
-						Name: "dev",
-					},
-				},
-			}, nil)
+			GetEnv(gomock.Any(), gomock.AssignableToTypeOf(&ent.User{}), gomock.AssignableToTypeOf(&ent.Repo{}), gomock.Any()).
+			Return(nil, e.NewError(e.ErrorCodeConfigUndefinedEnv, nil))
 
 		r := NewRepo(RepoConfig{}, m)
 
@@ -61,7 +56,7 @@ func TestRepo_CreateLock(t *testing.T) {
 
 		router.ServeHTTP(w, req)
 		if w.Code != http.StatusUnprocessableEntity {
-			t.Fatalf("Code = %v, wanted %v. Body=%v", w.Code, http.StatusCreated, w.Body)
+			t.Fatalf("Code = %v, wanted %v. Body=%v", w.Code, http.StatusUnprocessableEntity, w.Body)
 		}
 	})
 
@@ -80,21 +75,11 @@ func TestRepo_CreateLock(t *testing.T) {
 		t.Log("Read deploy.yml and check the env.")
 		m.
 			EXPECT().
-			GetConfig(gomock.Any(), gomock.AssignableToTypeOf(&ent.User{}), gomock.AssignableToTypeOf(&ent.Repo{})).
-			Return(&vo.Config{
-				Envs: []*vo.Env{
-					{
-						Name: "production",
-					},
-				},
+			GetEnv(gomock.Any(), gomock.AssignableToTypeOf(&ent.User{}), gomock.AssignableToTypeOf(&ent.Repo{}), gomock.Any()).
+			Return(&vo.Env{
+				Name: "production",
 			}, nil)
 
-		t.Log("Check whether the env is locked or not.")
-		m.
-			EXPECT().
-			HasLockOfRepoForEnv(gomock.Any(), gomock.AssignableToTypeOf(&ent.Repo{}), input.payload.Env).
-			Return(false, nil)
-
 		t.Log("Lock the env.")
 		m.
 			EXPECT().

internal/server/api/v1/repos/mock/interactor.go

@@ -8,6 +8,8 @@ import (
 const (
 	// ErrorCodeConfigParseError is that an error occurs when it parse the file.
 	ErrorCodeConfigParseError ErrorCode = "config_parse_error"
+	// ErrorCodeConfigUndefinedEnv is that the environment is not defined in the configuration file.
+	ErrorCodeConfigUndefinedEnv ErrorCode = "config_undefined_env"
 	// ErrorCodeConfigRegexpError is the regexp(re2) is invalid.
 	ErrorCodeConfigRegexpError ErrorCode = "config_regexp_error"
 
@@ -22,6 +24,9 @@ const (
 	// ErrorCodeDeploymentStatusNotWaiting is the status must be 'waiting' to create a remote deployment.
 	ErrorCodeDeploymentStatusInvalid ErrorCode = "deployment_status_invalid"
 
+	// ErrorCodeLockAlreadyExist is that the environment is already locked.
+	ErrorCodeLockAlreadyExist ErrorCode = "lock_already_exist"
+
 	// ErrorCodeLicenseDecode is the error when the license is decoded.
 	ErrorCodeLicenseDecode ErrorCode = "license_decode"
 	// ErrorCodeLicenseRequired is that the license is required.

pkg/e/code.go

@@ -4,12 +4,14 @@ import "net/http"
 
 var messages = map[ErrorCode]string{
 	ErrorCodeConfigParseError:        "The configuration is invalid.",
+	ErrorCodeConfigUndefinedEnv:      "The environment is not defined in the configuration.",
 	ErrorCodeConfigRegexpError:       "The regexp is invalid.",
 	ErrorCodeDeploymentConflict:      "The conflict occurs, please retry.",
 	ErrorCodeDeploymentInvalid:       "The validation has failed.",
 	ErrorCodeDeploymentLocked:        "The environment is locked.",
 	ErrorCodeDeploymentNotApproved:   "The deployment is not approved",
 	ErrorCodeDeploymentStatusInvalid: "The deployment status is invalid",
+	ErrorCodeLockAlreadyExist:        "The environment is already locked",
 	ErrorCodeLicenseDecode:           "Decoding the license is failed.",
 	ErrorCodeLicenseRequired:         "The license is required.",
 	ErrorCodeEntityNotFound:          "It is not found.",
@@ -30,12 +32,14 @@ func GetMessage(code ErrorCode) string {
 
 var httpCodes = map[ErrorCode]int{
 	ErrorCodeConfigParseError:        http.StatusUnprocessableEntity,
+	ErrorCodeConfigUndefinedEnv:      http.StatusUnprocessableEntity,
 	ErrorCodeConfigRegexpError:       http.StatusUnprocessableEntity,
 	ErrorCodeDeploymentConflict:      http.StatusUnprocessableEntity,
 	ErrorCodeDeploymentInvalid:       http.StatusUnprocessableEntity,
 	ErrorCodeDeploymentLocked:        http.StatusUnprocessableEntity,
 	ErrorCodeDeploymentNotApproved:   http.StatusUnprocessableEntity,
 	ErrorCodeDeploymentStatusInvalid: http.StatusUnprocessableEntity,
+	ErrorCodeLockAlreadyExist:        http.StatusUnprocessableEntity,
 	ErrorCodeLicenseDecode:           http.StatusUnprocessableEntity,
 	ErrorCodeLicenseRequired:         http.StatusPaymentRequired,
 	ErrorCodeEntityNotFound:          http.StatusNotFound,