From 869e198627c1c5ddd016f4cc9bfaf809f1e0a436 Mon Sep 17 00:00:00 2001 From: noah Date: Mon, 14 Nov 2022 23:05:57 +0900 Subject: [PATCH 1/3] Migrate the logic of deployment change --- internal/interactor/deployment.go | 35 +++++++++++++++++++ internal/interactor/interface.go | 2 ++ .../server/api/v1/repos/deployment_change.go | 34 ++---------------- internal/server/api/v1/repos/interface.go | 1 + .../server/api/v1/repos/mock/interactor.go | 15 ++++++++ pkg/e/code.go | 18 +++++++++- 6 files changed, 72 insertions(+), 33 deletions(-) diff --git a/internal/interactor/deployment.go b/internal/interactor/deployment.go index 3e23e004..e534a98f 100644 --- a/internal/interactor/deployment.go +++ b/internal/interactor/deployment.go @@ -273,6 +273,41 @@ func (i *DeploymentInteractor) CreateDeploymentStatus(ctx context.Context, ds *e return ds, nil } +// CompareCommitsFromLastestDeployment returns the changed commit from the last deployment. +// If there is no last deployment, an empty slice is returned. +func (i *DeploymentInteractor) CompareCommitsFromLastestDeployment(ctx context.Context, r *ent.Repo, number int, options *ListOptions) ([]*extent.Commit, error) { + d, err := i.store.FindDeploymentOfRepoByNumber(ctx, r, number) + if err != nil { + i.log.Sugar().Errorf("Failed to find the deployment by ID(%d).", number) + return nil, err + } + + if d.Status == deployment.StatusWaiting { + i.log.Info("The deployment is waiting, it doesn't have an SHA to compare.") + return []*extent.Commit{}, nil + } + + last, err := i.store.FindPrevSuccessDeployment(ctx, d) + if e.HasErrorCode(err, e.ErrorCodeEntityNotFound) { + i.log.Info("The lastes deployment is not found. It returns an empty commit slice.") + return []*extent.Commit{}, nil + } else if err != nil { + i.log.Error("Failed to find the last success deployment.") + return nil, err + } + + // TODO: Handling the not-found error. + u := ctx.Value(KeyUser).(*ent.User) + + commits, _, err := i.scm.CompareCommits(ctx, u, r, last.Sha, d.Sha, options) + if err != nil { + i.log.Sugar().Errorf("Failed to compare %s ... %s.", last.Sha, d.Sha) + return nil, err + } + + return commits, nil +} + func (i *DeploymentInteractor) runClosingInactiveDeployment(stop <-chan struct{}) { ctx := context.Background() diff --git a/internal/interactor/interface.go b/internal/interactor/interface.go index 4b187f12..f2699abe 100644 --- a/internal/interactor/interface.go +++ b/internal/interactor/interface.go @@ -9,6 +9,8 @@ import ( "github.com/gitploy-io/gitploy/model/extent" ) +const KeyUser = "gitploy.user" + type ( Store interface { UserStore diff --git a/internal/server/api/v1/repos/deployment_change.go b/internal/server/api/v1/repos/deployment_change.go index f34d995e..8a625134 100644 --- a/internal/server/api/v1/repos/deployment_change.go +++ b/internal/server/api/v1/repos/deployment_change.go @@ -13,13 +13,10 @@ import ( gb "github.com/gitploy-io/gitploy/internal/server/global" "github.com/gitploy-io/gitploy/model/ent" "github.com/gitploy-io/gitploy/model/ent/deployment" - "github.com/gitploy-io/gitploy/model/extent" "github.com/gitploy-io/gitploy/pkg/e" ) func (s *DeploymentAPI) ListChanges(c *gin.Context) { - ctx := c.Request.Context() - var ( number int page int @@ -51,36 +48,9 @@ func (s *DeploymentAPI) ListChanges(c *gin.Context) { vr, _ := c.Get(KeyRepo) re := vr.(*ent.Repo) - d, err := s.i.FindDeploymentOfRepoByNumber(ctx, re, number) - if err != nil { - s.log.Check(gb.GetZapLogLevel(err), "Failed to find the deployments.").Write(zap.Error(err)) - gb.ResponseWithError(c, err) - return - } - - ld, err := s.i.FindPrevSuccessDeployment(ctx, d) - if e.HasErrorCode(err, e.ErrorCodeEntityNotFound) { - s.log.Debug("The previous deployment is not found.") - gb.Response(c, http.StatusOK, []*extent.Commit{}) - return - } else if err != nil { - s.log.Check(gb.GetZapLogLevel(err), "Failed to find the deployments.").Write(zap.Error(err)) - gb.ResponseWithError(c, err) - return - } - - // Get SHA when the status of deployment is waiting. - sha := d.Sha - if sha == "" { - sha, err = s.getCommitSha(ctx, u, re, d.Type, d.Ref) - if err != nil { - s.log.Check(gb.GetZapLogLevel(err), "It has failed to get the commit SHA.").Write(zap.Error(err)) - gb.ResponseWithError(c, err) - return - } - } + ctx := context.WithValue(c.Request.Context(), gb.KeyUser, u) - commits, _, err := s.i.CompareCommits(ctx, u, re, ld.Sha, sha, &i.ListOptions{ + commits, err := s.i.CompareCommitsFromLastestDeployment(ctx, re, number, &i.ListOptions{ Page: page, PerPage: perPage, }) diff --git a/internal/server/api/v1/repos/interface.go b/internal/server/api/v1/repos/interface.go index fbf86191..ae1cda57 100644 --- a/internal/server/api/v1/repos/interface.go +++ b/internal/server/api/v1/repos/interface.go @@ -32,6 +32,7 @@ type ( DeployToRemote(ctx context.Context, u *ent.User, r *ent.Repo, d *ent.Deployment, env *extent.Env) (*ent.Deployment, error) GetConfig(ctx context.Context, u *ent.User, r *ent.Repo) (*extent.Config, error) GetEvaluatedConfig(ctx context.Context, u *ent.User, r *ent.Repo, v *extent.EvalValues) (*extent.Config, error) + CompareCommitsFromLastestDeployment(ctx context.Context, r *ent.Repo, number int, options *i.ListOptions) ([]*extent.Commit, error) ListDeploymentStatuses(ctx context.Context, d *ent.Deployment) ([]*ent.DeploymentStatus, error) CreateRemoteDeploymentStatus(ctx context.Context, u *ent.User, r *ent.Repo, d *ent.Deployment, ds *extent.RemoteDeploymentStatus) (*extent.RemoteDeploymentStatus, error) diff --git a/internal/server/api/v1/repos/mock/interactor.go b/internal/server/api/v1/repos/mock/interactor.go index 142e711d..adbaaffe 100644 --- a/internal/server/api/v1/repos/mock/interactor.go +++ b/internal/server/api/v1/repos/mock/interactor.go @@ -68,6 +68,21 @@ func (mr *MockInteractorMockRecorder) CompareCommits(ctx, u, r, base, head, opt return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CompareCommits", reflect.TypeOf((*MockInteractor)(nil).CompareCommits), ctx, u, r, base, head, opt) } +// CompareCommitsFromLastestDeployment mocks base method. +func (m *MockInteractor) CompareCommitsFromLastestDeployment(ctx context.Context, r *ent.Repo, number int, options *interactor.ListOptions) ([]*extent.Commit, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CompareCommitsFromLastestDeployment", ctx, r, number, options) + ret0, _ := ret[0].([]*extent.Commit) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CompareCommitsFromLastestDeployment indicates an expected call of CompareCommitsFromLastestDeployment. +func (mr *MockInteractorMockRecorder) CompareCommitsFromLastestDeployment(ctx, r, number, options interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CompareCommitsFromLastestDeployment", reflect.TypeOf((*MockInteractor)(nil).CompareCommitsFromLastestDeployment), ctx, r, number, options) +} + // CreateLock mocks base method. func (m *MockInteractor) CreateLock(ctx context.Context, l *ent.Lock) (*ent.Lock, error) { m.ctrl.T.Helper() diff --git a/pkg/e/code.go b/pkg/e/code.go index 4ef65a54..7f4fe53e 100644 --- a/pkg/e/code.go +++ b/pkg/e/code.go @@ -77,6 +77,16 @@ func NewError(code ErrorCode, wrap error) *Error { } } +// TODO: Migrate the other functions which +// constructs an error into this function. +func NError(code ErrorCode, message string) *Error { + return &Error{ + Code: code, + Message: GetMessage(code), + httpCode: mapHTTPCode(code), + } +} + func NewErrorWithMessage(code ErrorCode, message string, wrap error) *Error { return &Error{ Code: code, @@ -105,14 +115,20 @@ func (e *Error) GetHTTPCode() int { } // SetHTTPCode sets the HTTP code manually. -func (e *Error) SetHTTPCode(code int) { +func (e *Error) SetHTTPCode(code int) *Error { e.httpCode = code + return e } func (e *Error) Unwrap() error { return e.Wrap } +func (e *Error) SetWrap(wrap error) *Error { + e.Wrap = wrap + return e +} + func mapHTTPCode(code ErrorCode) int { httpCode, ok := httpCodes[code] if !ok { From 6798008db414f8092c98435496d76735c0836701 Mon Sep 17 00:00:00 2001 From: noah Date: Sun, 4 Dec 2022 17:33:32 +0900 Subject: [PATCH 2/3] Change the interface of `CompareCommits...` --- internal/interactor/deployment.go | 8 +------- internal/server/api/v1/repos/deployment_change.go | 9 ++++++++- internal/server/api/v1/repos/interface.go | 2 +- internal/server/api/v1/repos/mock/interactor.go | 8 ++++---- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/internal/interactor/deployment.go b/internal/interactor/deployment.go index e534a98f..99fb2568 100644 --- a/internal/interactor/deployment.go +++ b/internal/interactor/deployment.go @@ -275,13 +275,7 @@ func (i *DeploymentInteractor) CreateDeploymentStatus(ctx context.Context, ds *e // CompareCommitsFromLastestDeployment returns the changed commit from the last deployment. // If there is no last deployment, an empty slice is returned. -func (i *DeploymentInteractor) CompareCommitsFromLastestDeployment(ctx context.Context, r *ent.Repo, number int, options *ListOptions) ([]*extent.Commit, error) { - d, err := i.store.FindDeploymentOfRepoByNumber(ctx, r, number) - if err != nil { - i.log.Sugar().Errorf("Failed to find the deployment by ID(%d).", number) - return nil, err - } - +func (i *DeploymentInteractor) CompareCommitsFromLastestDeployment(ctx context.Context, r *ent.Repo, d *ent.Deployment, options *ListOptions) ([]*extent.Commit, error) { if d.Status == deployment.StatusWaiting { i.log.Info("The deployment is waiting, it doesn't have an SHA to compare.") return []*extent.Commit{}, nil diff --git a/internal/server/api/v1/repos/deployment_change.go b/internal/server/api/v1/repos/deployment_change.go index 8a625134..fe7688ff 100644 --- a/internal/server/api/v1/repos/deployment_change.go +++ b/internal/server/api/v1/repos/deployment_change.go @@ -50,7 +50,14 @@ func (s *DeploymentAPI) ListChanges(c *gin.Context) { ctx := context.WithValue(c.Request.Context(), gb.KeyUser, u) - commits, err := s.i.CompareCommitsFromLastestDeployment(ctx, re, number, &i.ListOptions{ + d, err := s.i.FindDeploymentOfRepoByNumber(ctx, re, number) + if err != nil { + s.log.Warn("Failed to find the deployment.", zap.Error(err)) + gb.ResponseWithError(c, err) + return + } + + commits, err := s.i.CompareCommitsFromLastestDeployment(ctx, re, d, &i.ListOptions{ Page: page, PerPage: perPage, }) diff --git a/internal/server/api/v1/repos/interface.go b/internal/server/api/v1/repos/interface.go index ae1cda57..f5d85482 100644 --- a/internal/server/api/v1/repos/interface.go +++ b/internal/server/api/v1/repos/interface.go @@ -32,7 +32,7 @@ type ( DeployToRemote(ctx context.Context, u *ent.User, r *ent.Repo, d *ent.Deployment, env *extent.Env) (*ent.Deployment, error) GetConfig(ctx context.Context, u *ent.User, r *ent.Repo) (*extent.Config, error) GetEvaluatedConfig(ctx context.Context, u *ent.User, r *ent.Repo, v *extent.EvalValues) (*extent.Config, error) - CompareCommitsFromLastestDeployment(ctx context.Context, r *ent.Repo, number int, options *i.ListOptions) ([]*extent.Commit, error) + CompareCommitsFromLastestDeployment(ctx context.Context, r *ent.Repo, d *ent.Deployment, options *i.ListOptions) ([]*extent.Commit, error) ListDeploymentStatuses(ctx context.Context, d *ent.Deployment) ([]*ent.DeploymentStatus, error) CreateRemoteDeploymentStatus(ctx context.Context, u *ent.User, r *ent.Repo, d *ent.Deployment, ds *extent.RemoteDeploymentStatus) (*extent.RemoteDeploymentStatus, error) diff --git a/internal/server/api/v1/repos/mock/interactor.go b/internal/server/api/v1/repos/mock/interactor.go index adbaaffe..da81979b 100644 --- a/internal/server/api/v1/repos/mock/interactor.go +++ b/internal/server/api/v1/repos/mock/interactor.go @@ -69,18 +69,18 @@ func (mr *MockInteractorMockRecorder) CompareCommits(ctx, u, r, base, head, opt } // CompareCommitsFromLastestDeployment mocks base method. -func (m *MockInteractor) CompareCommitsFromLastestDeployment(ctx context.Context, r *ent.Repo, number int, options *interactor.ListOptions) ([]*extent.Commit, error) { +func (m *MockInteractor) CompareCommitsFromLastestDeployment(ctx context.Context, r *ent.Repo, d *ent.Deployment, options *interactor.ListOptions) ([]*extent.Commit, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CompareCommitsFromLastestDeployment", ctx, r, number, options) + ret := m.ctrl.Call(m, "CompareCommitsFromLastestDeployment", ctx, r, d, options) ret0, _ := ret[0].([]*extent.Commit) ret1, _ := ret[1].(error) return ret0, ret1 } // CompareCommitsFromLastestDeployment indicates an expected call of CompareCommitsFromLastestDeployment. -func (mr *MockInteractorMockRecorder) CompareCommitsFromLastestDeployment(ctx, r, number, options interface{}) *gomock.Call { +func (mr *MockInteractorMockRecorder) CompareCommitsFromLastestDeployment(ctx, r, d, options interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CompareCommitsFromLastestDeployment", reflect.TypeOf((*MockInteractor)(nil).CompareCommitsFromLastestDeployment), ctx, r, number, options) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CompareCommitsFromLastestDeployment", reflect.TypeOf((*MockInteractor)(nil).CompareCommitsFromLastestDeployment), ctx, r, d, options) } // CreateLock mocks base method. From 9c03040dea566b2a93cc205b0df22806d4bf6650 Mon Sep 17 00:00:00 2001 From: noah Date: Sun, 4 Dec 2022 17:53:36 +0900 Subject: [PATCH 3/3] Fix lint & test --- internal/interactor/global.go | 8 +++++ internal/interactor/interface.go | 2 -- .../server/api/v1/repos/deployment_change.go | 33 ++----------------- .../api/v1/repos/deployment_change_test.go | 13 ++------ 4 files changed, 12 insertions(+), 44 deletions(-) create mode 100644 internal/interactor/global.go diff --git a/internal/interactor/global.go b/internal/interactor/global.go new file mode 100644 index 00000000..bd6ea9c3 --- /dev/null +++ b/internal/interactor/global.go @@ -0,0 +1,8 @@ +package interactor + +// ContextKey is used for context access. +type ContextKey int + +const ( + KeyUser ContextKey = iota +) diff --git a/internal/interactor/interface.go b/internal/interactor/interface.go index f2699abe..4b187f12 100644 --- a/internal/interactor/interface.go +++ b/internal/interactor/interface.go @@ -9,8 +9,6 @@ import ( "github.com/gitploy-io/gitploy/model/extent" ) -const KeyUser = "gitploy.user" - type ( Store interface { UserStore diff --git a/internal/server/api/v1/repos/deployment_change.go b/internal/server/api/v1/repos/deployment_change.go index fe7688ff..5b644791 100644 --- a/internal/server/api/v1/repos/deployment_change.go +++ b/internal/server/api/v1/repos/deployment_change.go @@ -2,7 +2,6 @@ package repos import ( "context" - "fmt" "net/http" "strconv" @@ -12,7 +11,6 @@ import ( i "github.com/gitploy-io/gitploy/internal/interactor" gb "github.com/gitploy-io/gitploy/internal/server/global" "github.com/gitploy-io/gitploy/model/ent" - "github.com/gitploy-io/gitploy/model/ent/deployment" "github.com/gitploy-io/gitploy/pkg/e" ) @@ -48,7 +46,7 @@ func (s *DeploymentAPI) ListChanges(c *gin.Context) { vr, _ := c.Get(KeyRepo) re := vr.(*ent.Repo) - ctx := context.WithValue(c.Request.Context(), gb.KeyUser, u) + ctx := context.WithValue(c.Request.Context(), i.KeyUser, u) d, err := s.i.FindDeploymentOfRepoByNumber(ctx, re, number) if err != nil { @@ -67,33 +65,6 @@ func (s *DeploymentAPI) ListChanges(c *gin.Context) { return } + s.log.Info("Get the changed commits for the deployment.", zap.Int("number", number)) gb.Response(c, http.StatusOK, commits) } - -func (s *DeploymentAPI) getCommitSha(ctx context.Context, u *ent.User, re *ent.Repo, typ deployment.Type, ref string) (string, error) { - switch typ { - case deployment.TypeCommit: - c, err := s.i.GetCommit(ctx, u, re, ref) - if err != nil { - return "", err - } - - return c.SHA, nil - case deployment.TypeBranch: - b, err := s.i.GetBranch(ctx, u, re, ref) - if err != nil { - return "", err - } - - return b.CommitSHA, nil - case deployment.TypeTag: - t, err := s.i.GetTag(ctx, u, re, ref) - if err != nil { - return "", err - } - - return t.CommitSHA, nil - default: - return "", fmt.Errorf("Type must be one of commit, branch, tag.") - } -} diff --git a/internal/server/api/v1/repos/deployment_change_test.go b/internal/server/api/v1/repos/deployment_change_test.go index c59083f1..0e2fb022 100644 --- a/internal/server/api/v1/repos/deployment_change_test.go +++ b/internal/server/api/v1/repos/deployment_change_test.go @@ -55,21 +55,12 @@ func TestDeploymentAPI_ListChanges(t *testing.T) { m. EXPECT(). - FindPrevSuccessDeployment(ctx, any). - Return(&ent.Deployment{ - ID: 5, - Sha: base, - Status: deployment.StatusSuccess, - }, nil) - - m. - EXPECT(). - CompareCommits(ctx, any, any, base, head, gomock.Any()). + CompareCommitsFromLastestDeployment(ctx, any, gomock.AssignableToTypeOf(&ent.Deployment{}), any). Return([]*extent.Commit{ { SHA: head, }, - }, []*extent.CommitFile{}, nil) + }, nil) // Ready the router to handle it. gin.SetMode(gin.ReleaseMode)