Merge pull request 'add missed return in POST /repos/{owner}/{repo}/push_mirrors' (#3945) from earl-warren/forgejo:wip-missed-return into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3945
Reviewed-by: Victoria <efertone@noreply.codeberg.org>

models/fixtures/push_mirror.yml

@@ -0,0 +1 @@
+[] # empty

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

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

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, "@") {

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)
+			}
+		})
+	}
+}