Skip to content
This repository was archived by the owner on May 30, 2024. It is now read-only.

Fix to handle the error that the webhook is not found when an user deactivates #254

Merged
merged 3 commits into from
Dec 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,6 @@ require (
go.uber.org/zap v1.19.1
golang.org/x/net v0.0.0-20210525063256-abc453219eb5 // indirect
golang.org/x/oauth2 v0.0.0-20210514164344-f6687ab2804c
gopkg.in/h2non/gock.v1 v1.1.2 // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
)
5 changes: 5 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/ad
github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs=
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk=
github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 h1:2VTzZjLZBgl62/EtslCrtky5vbi9dd7HrQPQIx6wqiw=
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542/go.mod h1:Ow0tF8D4Kplbc8s8sSb3V2oUCygFHVp8gC3Dn6U4MNI=
github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
Expand Down Expand Up @@ -348,6 +350,7 @@ github.com/modern-go/reflect2 v1.0.1 h1:9f412s+6RmYXLWZSEzVVgPGK7C2PphHj5RJrvfx9
github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32/go.mod h1:9wM+0iRr9ahx58uYLpLIr5fm8diHn0JbqRycJi6w0Ms=
github.com/nleeper/goment v1.4.2 h1:r4c8KkCrsBJUnVi/IJ5HEqev5QY8aCWOXQtu+eYXtnI=
github.com/nleeper/goment v1.4.2/go.mod h1:zDl5bAyDhqxwQKAvkSXMRLOdCowrdZz53ofRJc4VhTo=
github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U=
Expand Down Expand Up @@ -778,6 +781,8 @@ gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8
gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8bDuhia5mkpMnE=
gopkg.in/go-playground/validator.v9 v9.29.1/go.mod h1:+c9/zcJMFNgbLvly1L1V+PpxWdVbfP1avr/N00E2vyQ=
gopkg.in/h2non/gock.v1 v1.1.2 h1:jBbHXgGBK/AoPVfJh5x4r/WxIrElvbLel8TCZkkZJoY=
gopkg.in/h2non/gock.v1 v1.1.2/go.mod h1:n7UGz/ckNChHiK05rDoiC4MYSunEC/lyaUm2WWaDva0=
gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec/go.mod h1:aPpfJ7XW+gOuirDoZ8gHhLh3kZ1B08FtV2bbmy7Jv3s=
gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo=
Expand Down
10 changes: 7 additions & 3 deletions internal/interactor/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import (
"fmt"

"github.com/gitploy-io/gitploy/ent"
"github.com/gitploy-io/gitploy/pkg/e"
"github.com/gitploy-io/gitploy/vo"
"go.uber.org/zap"
)

func (i *Interactor) ActivateRepo(ctx context.Context, u *ent.User, r *ent.Repo, c *vo.WebhookConfig) (*ent.Repo, error) {
Expand All @@ -27,13 +29,15 @@ func (i *Interactor) ActivateRepo(ctx context.Context, u *ent.User, r *ent.Repo,

func (i *Interactor) DeactivateRepo(ctx context.Context, u *ent.User, r *ent.Repo) (*ent.Repo, error) {
err := i.SCM.DeleteWebhook(ctx, u, r, r.WebhookID)
if err != nil {
return nil, fmt.Errorf("failed to delete the webhook: %w", err)
if e.HasErrorCode(err, e.ErrorCodeEntityNotFound) {
i.log.Info("The webhook is not found, skip to delete the webhook.", zap.Int64("id", r.WebhookID))
} else if err != nil {
return nil, err
}

r, err = i.Store.Deactivate(ctx, r)
if err != nil {
return nil, fmt.Errorf("failed to deactivate the webhook: %w", err)
return nil, err
}

return r, nil
Expand Down
38 changes: 38 additions & 0 deletions internal/interactor/repo_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
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"
)

func TestInteractor_DeactivateRepo(t *testing.T) {
t.Run("Deactivate successfully even if the webhook is not found.", func(t *testing.T) {
ctrl := gomock.NewController(t)
store := mock.NewMockStore(ctrl)
scm := mock.NewMockSCM(ctrl)

t.Log("Mocking DeleteWebhook to return an EntityNotFound error.")
scm.
EXPECT().
DeleteWebhook(gomock.Any(), gomock.AssignableToTypeOf(&ent.User{}), gomock.AssignableToTypeOf(&ent.Repo{}), gomock.Any()).
Return(e.NewError(e.ErrorCodeEntityNotFound, nil))

store.
EXPECT().
Deactivate(gomock.Any(), gomock.AssignableToTypeOf(&ent.Repo{})).
Return(&ent.Repo{}, nil)

i := newMockInteractor(store, scm)

_, err := i.DeactivateRepo(context.Background(), &ent.User{}, &ent.Repo{})
if err != nil {
t.Fatalf("DeactivateRepo returns an error: %v", err)
}
})
}
7 changes: 6 additions & 1 deletion internal/pkg/github/repos.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,13 @@ func (g *Github) CreateWebhook(ctx context.Context, u *ent.User, r *ent.Repo, c
}

func (g *Github) DeleteWebhook(ctx context.Context, u *ent.User, r *ent.Repo, id int64) error {
_, err := g.Client(ctx, u.Token).
res, err := g.Client(ctx, u.Token).
Repositories.
DeleteHook(ctx, r.Namespace, r.Name, id)
// https://docs.github.com/en/rest/reference/repos#delete-a-repository-webhook
if res.StatusCode == http.StatusNotFound {
return e.NewErrorWithMessage(e.ErrorCodeEntityNotFound, "The webhook is not found.", err)
}

return err
}
62 changes: 62 additions & 0 deletions internal/pkg/github/repos_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package github

import (
"context"
"testing"

"github.com/gitploy-io/gitploy/ent"
"github.com/gitploy-io/gitploy/pkg/e"
"gopkg.in/h2non/gock.v1"
)

func TestGithub_DeleteWebhook(t *testing.T) {
t.Run("Return the ErrorCodeEntityNotFound error when the webhook is not found.", func(t *testing.T) {
t.Log("Mocking the delete webhook API")
gock.New("https://api.github.com").
Delete("/repos/gitploy-io/gitploy/hooks/1").
Reply(404)

g := NewGithub(&GithubConfig{})

const hookID = 1

err := g.DeleteWebhook(
context.Background(),
&ent.User{},
&ent.Repo{
Namespace: "gitploy-io",
Name: "gitploy",
},
hookID,
)

if !e.HasErrorCode(err, e.ErrorCodeEntityNotFound) {
t.Fatalf("DeleteWebhook doesn't returns an ErrorCodeEntityNotFound error: %v", err)
}
})

t.Run("Delete the webhook.", func(t *testing.T) {
t.Log("Mocking the delete webhook API")
gock.New("https://api.github.com").
Delete("/repos/gitploy-io/gitploy/hooks/1").
Reply(200)

g := NewGithub(&GithubConfig{})

const hookID = 1

err := g.DeleteWebhook(
context.Background(),
&ent.User{},
&ent.Repo{
Namespace: "gitploy-io",
Name: "gitploy",
},
hookID,
)

if err != nil {
t.Fatalf("DeleteWebhook returns an error: %v", err)
}
})
}
17 changes: 9 additions & 8 deletions pkg/e/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ const (
// ErrorCodeDeploymentStatusNotWaiting is the status must be 'waiting' to create a remote deployment.
ErrorCodeDeploymentStatusInvalid ErrorCode = "deployment_status_invalid"

// ErrorCodeEntityNotFound is the entity is not found.
// Entity is a resource of store or scm.
ErrorCodeEntityNotFound ErrorCode = "entity_not_found"
// ErrorCodeEntityUnprocessable is the entity is unprocessable.
ErrorCodeEntityUnprocessable ErrorCode = "entity_unprocessable"

// ErrorCodeInternalError is the internal error couldn't be handled.
ErrorCodeInternalError ErrorCode = "internal_error"

Comment on lines +27 to +35
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort alphabetically

// ErrorCodeLockAlreadyExist is that the environment is already locked.
ErrorCodeLockAlreadyExist ErrorCode = "lock_already_exist"

Expand All @@ -32,19 +41,11 @@ const (
// ErrorCodeLicenseRequired is that the license is required.
ErrorCodeLicenseRequired ErrorCode = "license_required"

// ErrorCodeEntityNotFound is the entity is not found.
ErrorCodeEntityNotFound ErrorCode = "entity_not_found"
// ErrorCodeEntityUnprocessable is the entity is unprocessable.
ErrorCodeEntityUnprocessable ErrorCode = "entity_unprocessable"

// ErrorCodeParameterInvalid is a parameter of a request is invalid.
ErrorCodeParameterInvalid ErrorCode = "parameter_invalid"

// ErrorPermissionRequired is the permission is required to access.
ErrorPermissionRequired ErrorCode = "permission_required"

// ErrorCodeInternalError is the internal error couldn't be handled.
ErrorCodeInternalError ErrorCode = "internal_error"
)

type (
Expand Down
12 changes: 6 additions & 6 deletions pkg/e/trans.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ var messages = map[ErrorCode]string{
ErrorCodeDeploymentLocked: "The environment is locked.",
ErrorCodeDeploymentNotApproved: "The deployment is not approved",
ErrorCodeDeploymentStatusInvalid: "The deployment status is invalid",
ErrorCodeEntityNotFound: "It is not found.",
ErrorCodeEntityUnprocessable: "Invalid request payload.",
ErrorCodeInternalError: "Server internal error.",
ErrorCodeLockAlreadyExist: "The environment is already locked",
ErrorCodeLicenseDecode: "Decoding the license is failed.",
ErrorCodeLicenseRequired: "The license is required.",
ErrorCodeEntityNotFound: "It is not found.",
ErrorCodeEntityUnprocessable: "Invalid request payload.",
ErrorCodeParameterInvalid: "Invalid request parameter.",
ErrorPermissionRequired: "The permission is required",
ErrorCodeInternalError: "Server internal error.",
}

func GetMessage(code ErrorCode) string {
Expand All @@ -39,14 +39,14 @@ var httpCodes = map[ErrorCode]int{
ErrorCodeDeploymentLocked: http.StatusUnprocessableEntity,
ErrorCodeDeploymentNotApproved: http.StatusUnprocessableEntity,
ErrorCodeDeploymentStatusInvalid: http.StatusUnprocessableEntity,
ErrorCodeEntityNotFound: http.StatusNotFound,
ErrorCodeEntityUnprocessable: http.StatusUnprocessableEntity,
ErrorCodeInternalError: http.StatusInternalServerError,
ErrorCodeLockAlreadyExist: http.StatusUnprocessableEntity,
ErrorCodeLicenseDecode: http.StatusUnprocessableEntity,
ErrorCodeLicenseRequired: http.StatusPaymentRequired,
ErrorCodeEntityNotFound: http.StatusNotFound,
ErrorCodeEntityUnprocessable: http.StatusUnprocessableEntity,
ErrorCodeParameterInvalid: http.StatusBadRequest,
ErrorPermissionRequired: http.StatusForbidden,
ErrorCodeInternalError: http.StatusInternalServerError,
}

func GetHttpCode(code ErrorCode) int {
Expand Down