From ba37b9e5779eb0c7834b9bc76ac213e7d931011d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 28 May 2024 17:31:59 +0800 Subject: [PATCH 1/3] Add missed return after `ctx.ServerError` (#31130) (partial) Only routers/api/v1/repo/mirror.go (cherry picked from commit b6f15c7948ac3d09977350de83ec91d5789ea083) --- routers/api/v1/repo/mirror.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/api/v1/repo/mirror.go b/routers/api/v1/repo/mirror.go index 2a896de4fe..eddd449206 100644 --- a/routers/api/v1/repo/mirror.go +++ b/routers/api/v1/repo/mirror.go @@ -383,6 +383,7 @@ func CreatePushMirror(ctx *context.APIContext, mirrorOption *api.CreatePushMirro if err = mirror_service.AddPushMirrorRemote(ctx, pushMirror, address); err != nil { if err := repo_model.DeletePushMirrors(ctx, repo_model.PushMirrorOptions{ID: pushMirror.ID, RepoID: pushMirror.RepoID}); err != nil { ctx.ServerError("DeletePushMirrors", err) + return } ctx.ServerError("AddPushMirrorRemote", err) return From 5747951cc7c3bce05b593447168cd6337965054b Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 29 May 2024 19:52:26 +0200 Subject: [PATCH 2/3] test(mock): DeletePushMirrors & AddPushMirrorRemote make them into variables that can be mocked --- models/repo/pushmirror.go | 4 +++- services/mirror/mirror_push.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/models/repo/pushmirror.go b/models/repo/pushmirror.go index e08333511c..3cf54facae 100644 --- a/models/repo/pushmirror.go +++ b/models/repo/pushmirror.go @@ -94,7 +94,9 @@ func UpdatePushMirrorInterval(ctx context.Context, m *PushMirror) error { return err } -func DeletePushMirrors(ctx context.Context, opts PushMirrorOptions) error { +var DeletePushMirrors = deletePushMirrors + +func deletePushMirrors(ctx context.Context, opts PushMirrorOptions) error { if opts.RepoID > 0 { _, err := db.Delete[PushMirror](ctx, opts) return err diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index 21ba0afeff..8303c9fb0c 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -28,7 +28,9 @@ import ( var stripExitStatus = regexp.MustCompile(`exit status \d+ - `) // AddPushMirrorRemote registers the push mirror remote. -func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr string) error { +var AddPushMirrorRemote = addPushMirrorRemote + +func addPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr string) error { addRemoteAndConfig := func(addr, path string) error { cmd := git.NewCommand(ctx, "remote", "add", "--mirror=push").AddDynamicArguments(m.RemoteName, addr) if strings.Contains(addr, "://") && strings.Contains(addr, "@") { From 166bb2861f91d2ac5d83906dd178d0cef3f1adbc Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 29 May 2024 17:17:58 +0200 Subject: [PATCH 3/3] tests(api): POST /repos/{owner}/{repo}/push_mirrors coverage --- models/fixtures/push_mirror.yml | 1 + tests/integration/api_push_mirror_test.go | 130 ++++++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 models/fixtures/push_mirror.yml create mode 100644 tests/integration/api_push_mirror_test.go diff --git a/models/fixtures/push_mirror.yml b/models/fixtures/push_mirror.yml new file mode 100644 index 0000000000..ca780a73aa --- /dev/null +++ b/models/fixtures/push_mirror.yml @@ -0,0 +1 @@ +[] # empty diff --git a/tests/integration/api_push_mirror_test.go b/tests/integration/api_push_mirror_test.go new file mode 100644 index 0000000000..9b0497aa3b --- /dev/null +++ b/tests/integration/api_push_mirror_test.go @@ -0,0 +1,130 @@ +// Copyright The Forgejo Authors +// SPDX-License-Identifier: MIT + +package integration + +import ( + "context" + "fmt" + "net/http" + "net/url" + "testing" + + auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/services/migrations" + mirror_service "code.gitea.io/gitea/services/mirror" + repo_service "code.gitea.io/gitea/services/repository" + + "github.com/stretchr/testify/assert" +) + +func TestAPIPushMirror(t *testing.T) { + onGiteaRun(t, testAPIPushMirror) +} + +func testAPIPushMirror(t *testing.T, u *url.URL) { + defer test.MockVariableValue(&setting.Migrations.AllowLocalNetworks, true)() + defer test.MockVariableValue(&setting.Mirror.Enabled, true)() + defer test.MockProtect(&mirror_service.AddPushMirrorRemote)() + defer test.MockProtect(&repo_model.DeletePushMirrors)() + + assert.NoError(t, migrations.Init()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + srcRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: srcRepo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeAll) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/push_mirrors", owner.Name, srcRepo.Name) + + mirrorRepo, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user, user, repo_service.CreateRepoOptions{ + Name: "test-push-mirror", + }) + assert.NoError(t, err) + remoteAddress := fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(user.Name), url.PathEscape(mirrorRepo.Name)) + + deletePushMirrors := repo_model.DeletePushMirrors + deletePushMirrorsError := "deletePushMirrorsError" + deletePushMirrorsFail := func(ctx context.Context, opts repo_model.PushMirrorOptions) error { + return fmt.Errorf(deletePushMirrorsError) + } + + addPushMirrorRemote := mirror_service.AddPushMirrorRemote + addPushMirrorRemoteError := "addPushMirrorRemoteError" + addPushMirrorRemoteFail := func(ctx context.Context, m *repo_model.PushMirror, addr string) error { + return fmt.Errorf(addPushMirrorRemoteError) + } + + for _, testCase := range []struct { + name string + message string + status int + mirrorCount int + setup func() + }{ + { + name: "success", + status: http.StatusOK, + mirrorCount: 1, + setup: func() { + mirror_service.AddPushMirrorRemote = addPushMirrorRemote + repo_model.DeletePushMirrors = deletePushMirrors + }, + }, + { + name: "fail to add and delete", + message: deletePushMirrorsError, + status: http.StatusInternalServerError, + mirrorCount: 1, + setup: func() { + mirror_service.AddPushMirrorRemote = addPushMirrorRemoteFail + repo_model.DeletePushMirrors = deletePushMirrorsFail + }, + }, + { + name: "fail to add", + message: addPushMirrorRemoteError, + status: http.StatusInternalServerError, + mirrorCount: 0, + setup: func() { + mirror_service.AddPushMirrorRemote = addPushMirrorRemoteFail + repo_model.DeletePushMirrors = deletePushMirrors + }, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + testCase.setup() + req := NewRequestWithJSON(t, "POST", urlStr, &api.CreatePushMirrorOption{ + RemoteAddress: remoteAddress, + Interval: "8h", + }).AddTokenAuth(token) + + resp := MakeRequest(t, req, testCase.status) + if testCase.message != "" { + err := api.APIError{} + DecodeJSON(t, resp, &err) + assert.EqualValues(t, testCase.message, err.Message) + } + + req = NewRequest(t, "GET", urlStr).AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusOK) + var pushMirrors []*api.PushMirror + DecodeJSON(t, resp, &pushMirrors) + if assert.Len(t, pushMirrors, testCase.mirrorCount) && testCase.mirrorCount > 0 { + pushMirror := pushMirrors[0] + assert.EqualValues(t, remoteAddress, pushMirror.RemoteAddress) + + repo_model.DeletePushMirrors = deletePushMirrors + req = NewRequest(t, "DELETE", fmt.Sprintf("%s/%s", urlStr, pushMirror.RemoteName)).AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) + } + }) + } +}