Skip to content

Commit 723f6c7

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 9786720 commit 723f6c7

File tree

6 files changed

+93
-170
lines changed

6 files changed

+93
-170
lines changed

‎models/repo/pushmirror.go‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@ import (
1616
"xorm.io/builder"
1717
)
1818

19-
// ErrPushMirrorNotExist mirror does not exist error
20-
var ErrPushMirrorNotExist = util.NewNotExistErrorf("PushMirror does not exist")
21-
2219
// PushMirror represents mirror information of a repository.
2320
type PushMirror struct {
2421
ID int64 `xorm:"pk autoincr"`
@@ -122,25 +119,13 @@ func GetPushMirrorsByRepoID(ctx context.Context, repoID int64, listOptions db.Li
122119
})
123120
}
124121

125-
func GetPushMirrorByIDAndRepoID(ctx context.Context, id, repoID int64) (*PushMirror, error) {
122+
func GetPushMirrorByIDAndRepoID(ctx context.Context, id, repoID int64) (*PushMirror, bool, error) {
126123
var pushMirror PushMirror
127124
has, err := db.GetEngine(ctx).Where("id = ?", id).And("repo_id = ?", repoID).Get(&pushMirror)
128-
if err != nil {
129-
return nil, err
130-
} else if !has {
131-
return nil, ErrPushMirrorNotExist
132-
}
133-
return &pushMirror, nil
134-
}
135-
136-
func GetPushMirrorByID(ctx context.Context, id int64) (*PushMirror, error) {
137-
mirror, has, err := db.GetByID[PushMirror](ctx, id)
138-
if err != nil {
139-
return nil, err
140-
} else if !has {
141-
return nil, ErrPushMirrorNotExist
125+
if !has || err != nil {
126+
return nil, has, err
142127
}
143-
return mirror, nil
128+
return &pushMirror, true, nil
144129
}
145130

146131
// GetPushMirrorsSyncedOnCommit returns push-mirrors for this repo that should be updated by new commits

‎routers/web/repo/setting/setting.go‎

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"errors"
99
"fmt"
1010
"net/http"
11-
"strconv"
1211
"strings"
1312
"time"
1413

@@ -290,8 +289,8 @@ func SettingsPost(ctx *context.Context) {
290289
return
291290
}
292291

293-
m, err := selectPushMirrorByForm(ctx, form, repo)
294-
if err != nil {
292+
m, _, _ := repo_model.GetPushMirrorByIDAndRepoID(ctx, form.PushMirrorID, repo.ID)
293+
if m == nil {
295294
ctx.NotFound("", nil)
296295
return
297296
}
@@ -317,8 +316,8 @@ func SettingsPost(ctx *context.Context) {
317316
return
318317
}
319318

320-
m, err := selectPushMirrorByForm(ctx, form, repo)
321-
if err != nil {
319+
m, _, _ := repo_model.GetPushMirrorByIDAndRepoID(ctx, form.PushMirrorID, repo.ID)
320+
if m == nil {
322321
ctx.NotFound("", nil)
323322
return
324323
}
@@ -332,7 +331,10 @@ func SettingsPost(ctx *context.Context) {
332331
// If we observed its implementation in the context of `push-mirror-sync` where it
333332
// is evident that pushing to the queue is necessary for updates.
334333
// So, there are updates within the given interval, it is necessary to update the queue accordingly.
335-
mirror_service.AddPushMirrorToQueue(m.ID)
334+
if !ctx.FormBool("push_mirror_defer_sync") {
335+
// push_mirror_defer_sync is mainly for testing purpose, we do not really want to sync the push mirror immediately
336+
mirror_service.AddPushMirrorToQueue(m.ID)
337+
}
336338
ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success"))
337339
ctx.Redirect(repo.Link() + "/settings")
338340

@@ -346,18 +348,18 @@ func SettingsPost(ctx *context.Context) {
346348
// as an error on the UI for this action
347349
ctx.Data["Err_RepoName"] = nil
348350

349-
m, err := selectPushMirrorByForm(ctx, form, repo)
350-
if err != nil {
351+
m, _, _ := repo_model.GetPushMirrorByIDAndRepoID(ctx, form.PushMirrorID, repo.ID)
352+
if m == nil {
351353
ctx.NotFound("", nil)
352354
return
353355
}
354356

355-
if err = mirror_service.RemovePushMirrorRemote(ctx, m); err != nil {
357+
if err := mirror_service.RemovePushMirrorRemote(ctx, m); err != nil {
356358
ctx.ServerError("RemovePushMirrorRemote", err)
357359
return
358360
}
359361

360-
if err = repo_model.DeletePushMirrors(ctx, repo_model.PushMirrorOptions{ID: m.ID, RepoID: m.RepoID}); err != nil {
362+
if err := repo_model.DeletePushMirrors(ctx, repo_model.PushMirrorOptions{ID: m.ID, RepoID: m.RepoID}); err != nil {
361363
ctx.ServerError("DeletePushMirrorByID", err)
362364
return
363365
}
@@ -993,21 +995,3 @@ func handleSettingRemoteAddrError(ctx *context.Context, err error, form *forms.R
993995
}
994996
ctx.RenderWithErr(ctx.Tr("repo.mirror_address_url_invalid"), tplSettingsOptions, form)
995997
}
996-
997-
func selectPushMirrorByForm(ctx *context.Context, form *forms.RepoSettingForm, repo *repo_model.Repository) (*repo_model.PushMirror, error) {
998-
id, err := strconv.ParseInt(form.PushMirrorID, 10, 64)
999-
if err != nil {
1000-
return nil, err
1001-
}
1002-
1003-
pushMirror, err := repo_model.GetPushMirrorByIDAndRepoID(ctx, id, repo.ID)
1004-
if err != nil {
1005-
if err == repo_model.ErrPushMirrorNotExist {
1006-
return nil, fmt.Errorf("PushMirror[%v] not associated to repository %v", id, repo)
1007-
}
1008-
return nil, err
1009-
}
1010-
1011-
pushMirror.Repo = repo
1012-
return pushMirror, nil
1013-
}

‎services/forms/repo_form.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ type RepoSettingForm struct {
122122
MirrorPassword string
123123
LFS bool `form:"mirror_lfs"`
124124
LFSEndpoint string `form:"mirror_lfs_endpoint"`
125-
PushMirrorID string
125+
PushMirrorID int64
126126
PushMirrorAddress string
127127
PushMirrorUsername string
128128
PushMirrorPassword string

‎services/mirror/mirror.go‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"fmt"
99

1010
repo_model "code.gitea.io/gitea/models/repo"
11-
"code.gitea.io/gitea/modules/graceful"
1211
"code.gitea.io/gitea/modules/log"
1312
"code.gitea.io/gitea/modules/queue"
1413
"code.gitea.io/gitea/modules/setting"
@@ -119,14 +118,7 @@ func Update(ctx context.Context, pullLimit, pushLimit int) error {
119118
return nil
120119
}
121120

122-
func queueHandler(items ...*SyncRequest) []*SyncRequest {
123-
for _, req := range items {
124-
doMirrorSync(graceful.GetManager().ShutdownContext(), req)
125-
}
126-
return nil
127-
}
128-
129121
// InitSyncMirrors initializes a go routine to sync the mirrors
130122
func InitSyncMirrors() {
131-
StartSyncMirrors(queueHandler)
123+
StartSyncMirrors()
132124
}

‎services/mirror/queue.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,19 @@ type SyncRequest struct {
2828
ReferenceID int64 // RepoID for pull mirror, MirrorID for push mirror
2929
}
3030

31+
func queueHandler(items ...*SyncRequest) []*SyncRequest {
32+
for _, req := range items {
33+
doMirrorSync(graceful.GetManager().ShutdownContext(), req)
34+
}
35+
return nil
36+
}
37+
3138
// StartSyncMirrors starts a go routine to sync the mirrors
32-
func StartSyncMirrors(queueHandle func(data ...*SyncRequest) []*SyncRequest) {
39+
func StartSyncMirrors() {
3340
if !setting.Mirror.Enabled {
3441
return
3542
}
36-
mirrorQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "mirror", queueHandle)
43+
mirrorQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "mirror", queueHandler)
3744
if mirrorQueue == nil {
3845
log.Fatal("Unable to create mirror queue")
3946
}

‎tests/integration/mirror_push_test.go‎

Lines changed: 66 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/http"
1010
"net/url"
1111
"strconv"
12+
"strings"
1213
"testing"
1314
"time"
1415

@@ -19,7 +20,6 @@ import (
1920
"code.gitea.io/gitea/modules/git"
2021
"code.gitea.io/gitea/modules/gitrepo"
2122
"code.gitea.io/gitea/modules/setting"
22-
"code.gitea.io/gitea/modules/test"
2323
gitea_context "code.gitea.io/gitea/services/context"
2424
"code.gitea.io/gitea/services/migrations"
2525
mirror_service "code.gitea.io/gitea/services/mirror"
@@ -49,7 +49,8 @@ func testMirrorPush(t *testing.T, u *url.URL) {
4949

5050
ctx := NewAPITestContext(t, user.LowerName, srcRepo.Name)
5151

52-
doCreatePushMirror(ctx, fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(ctx.Username), url.PathEscape(mirrorRepo.Name)), user.LowerName, userPassword)(t)
52+
pushMirrorURL := fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(ctx.Username), url.PathEscape(mirrorRepo.Name))
53+
testCreatePushMirror(t, ctx.Session, user.LowerName, srcRepo.Name, pushMirrorURL, user.LowerName, userPassword, "0")
5354

5455
mirrors, _, err := repo_model.GetPushMirrorsByRepoID(db.DefaultContext, srcRepo.ID, db.ListOptions{})
5556
assert.NoError(t, err)
@@ -75,127 +76,81 @@ func testMirrorPush(t *testing.T, u *url.URL) {
7576
assert.Equal(t, srcCommit.ID, mirrorCommit.ID)
7677

7778
// Cleanup
78-
doRemovePushMirror(ctx, fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(ctx.Username), url.PathEscape(mirrorRepo.Name)), user.LowerName, userPassword, int(mirrors[0].ID))(t)
79+
assert.True(t, doRemovePushMirror(t, ctx.Session, user.LowerName, srcRepo.Name, int(mirrors[0].ID)))
7980
mirrors, _, err = repo_model.GetPushMirrorsByRepoID(db.DefaultContext, srcRepo.ID, db.ListOptions{})
8081
assert.NoError(t, err)
8182
assert.Len(t, mirrors, 0)
8283
}
8384

84-
func doCreatePushMirror(ctx APITestContext, address, username, password string) func(t *testing.T) {
85-
return func(t *testing.T) {
86-
csrf := GetUserCSRFToken(t, ctx.Session)
87-
88-
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), map[string]string{
89-
"_csrf": csrf,
90-
"action": "push-mirror-add",
91-
"push_mirror_address": address,
92-
"push_mirror_username": username,
93-
"push_mirror_password": password,
94-
"push_mirror_interval": "0",
95-
})
96-
ctx.Session.MakeRequest(t, req, http.StatusSeeOther)
97-
98-
flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash)
99-
assert.NotNil(t, flashCookie)
100-
assert.Contains(t, flashCookie.Value, "success")
101-
}
85+
func testCreatePushMirror(t *testing.T, session *TestSession, owner, repo, address, username, password, interval string) {
86+
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(owner), url.PathEscape(repo)), map[string]string{
87+
"_csrf": GetUserCSRFToken(t, session),
88+
"action": "push-mirror-add",
89+
"push_mirror_address": address,
90+
"push_mirror_username": username,
91+
"push_mirror_password": password,
92+
"push_mirror_interval": interval,
93+
})
94+
session.MakeRequest(t, req, http.StatusSeeOther)
95+
96+
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
97+
assert.NotNil(t, flashCookie)
98+
assert.Contains(t, flashCookie.Value, "success")
10299
}
103100

104-
func doRemovePushMirror(ctx APITestContext, address, username, password string, pushMirrorID int) func(t *testing.T) {
105-
return func(t *testing.T) {
106-
csrf := GetUserCSRFToken(t, ctx.Session)
107-
108-
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), map[string]string{
109-
"_csrf": csrf,
110-
"action": "push-mirror-remove",
111-
"push_mirror_id": strconv.Itoa(pushMirrorID),
112-
"push_mirror_address": address,
113-
"push_mirror_username": username,
114-
"push_mirror_password": password,
115-
"push_mirror_interval": "0",
116-
})
117-
ctx.Session.MakeRequest(t, req, http.StatusSeeOther)
118-
119-
flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash)
120-
assert.NotNil(t, flashCookie)
121-
assert.Contains(t, flashCookie.Value, "success")
122-
}
101+
func doRemovePushMirror(t *testing.T, session *TestSession, owner, repo string, pushMirrorID int) bool {
102+
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(owner), url.PathEscape(repo)), map[string]string{
103+
"_csrf": GetUserCSRFToken(t, session),
104+
"action": "push-mirror-remove",
105+
"push_mirror_id": strconv.Itoa(pushMirrorID),
106+
})
107+
resp := session.MakeRequest(t, req, NoExpectedStatus)
108+
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
109+
return resp.Code == http.StatusSeeOther && flashCookie != nil && strings.Contains(flashCookie.Value, "success")
123110
}
124111

125-
func TestRepoSettingPushMirror(t *testing.T) {
112+
func doUpdatePushMirror(t *testing.T, session *TestSession, owner, repo string, pushMirrorID int64, interval string) bool {
113+
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", owner, repo), map[string]string{
114+
"_csrf": GetUserCSRFToken(t, session),
115+
"action": "push-mirror-update",
116+
"push_mirror_id": strconv.FormatInt(pushMirrorID, 10),
117+
"push_mirror_interval": interval,
118+
"push_mirror_defer_sync": "true",
119+
})
120+
resp := session.MakeRequest(t, req, NoExpectedStatus)
121+
return resp.Code == http.StatusSeeOther
122+
}
123+
124+
func TestRepoSettingPushMirrorUpdate(t *testing.T) {
126125
defer tests.PrepareTestEnv(t)()
126+
setting.Migrations.AllowLocalNetworks = true
127+
assert.NoError(t, migrations.Init())
127128

128129
session := loginUser(t, "user2")
129-
130-
repoPrefix := "/user2/repo2"
131130
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
131+
testCreatePushMirror(t, session, "user2", "repo2", "https://127.0.0.1/user1/repo1.git", "", "", "24h")
132132

133-
defer func() {
134-
migrations.Init()
135-
}()
136-
defer test.MockVariableValue(&setting.Migrations.AllowedDomains, "127.0.0.1")()
137-
assert.NoError(t, migrations.Init())
138-
139-
// visit repository setting page
140-
req := NewRequest(t, "GET", repoPrefix+"/settings")
141-
resp := session.MakeRequest(t, req, http.StatusOK)
142-
htmlDoc := NewHTMLParser(t, resp.Body)
143-
144-
onGiteaRun(t, func(t *testing.T, u *url.URL) {
145-
t.Run("Push Mirror Add", func(t *testing.T) {
146-
req = NewRequestWithValues(t, "POST", repoPrefix+"/settings", map[string]string{
147-
"_csrf": htmlDoc.GetCSRF(),
148-
"action": "push-mirror-add",
149-
"push_mirror_address": u.String() + "/user1/repo1.git",
150-
"push_mirror_interval": "0",
151-
})
152-
session.MakeRequest(t, req, http.StatusSeeOther)
153-
154-
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
155-
assert.NotNil(t, flashCookie)
156-
assert.Contains(t, flashCookie.Value, "success")
157-
158-
mirrors, cnt, err := repo_model.GetPushMirrorsByRepoID(db.DefaultContext, repo2.ID, db.ListOptions{})
159-
assert.NoError(t, err)
160-
assert.Len(t, mirrors, 1)
161-
assert.EqualValues(t, 1, cnt)
162-
assert.EqualValues(t, 0, mirrors[0].Interval)
163-
})
164-
165-
mirrors, _, _ := repo_model.GetPushMirrorsByRepoID(db.DefaultContext, repo2.ID, db.ListOptions{})
166-
167-
t.Run("Push Mirror Update", func(t *testing.T) {
168-
req := NewRequestWithValues(t, "POST", repoPrefix+"/settings", map[string]string{
169-
"_csrf": htmlDoc.GetCSRF(),
170-
"action": "push-mirror-update",
171-
"push_mirror_id": strconv.FormatInt(mirrors[0].ID, 10),
172-
"push_mirror_interval": "10m0s",
173-
})
174-
session.MakeRequest(t, req, http.StatusSeeOther)
175-
176-
mirror, err := repo_model.GetPushMirrorByID(db.DefaultContext, mirrors[0].ID)
177-
assert.NoError(t, err)
178-
assert.EqualValues(t, 10*time.Minute, mirror.Interval)
179-
180-
req = NewRequestWithValues(t, "POST", repoPrefix+"/settings", map[string]string{
181-
"_csrf": htmlDoc.GetCSRF(),
182-
"action": "push-mirror-update",
183-
"push_mirror_id": strconv.FormatInt(9999, 10), // 1 is an mirror ID which is not exist
184-
"push_mirror_interval": "10m0s",
185-
})
186-
session.MakeRequest(t, req, http.StatusNotFound)
187-
})
188-
189-
t.Run("Push Mirror Remove", func(t *testing.T) {
190-
req := NewRequestWithValues(t, "POST", repoPrefix+"/settings", map[string]string{
191-
"_csrf": htmlDoc.GetCSRF(),
192-
"action": "push-mirror-remove",
193-
"push_mirror_id": strconv.FormatInt(mirrors[0].ID, 10),
194-
})
195-
session.MakeRequest(t, req, http.StatusSeeOther)
196-
197-
_, err := repo_model.GetPushMirrorByID(db.DefaultContext, mirrors[0].ID)
198-
assert.Error(t, err)
199-
})
200-
})
133+
pushMirrors, cnt, err := repo_model.GetPushMirrorsByRepoID(db.DefaultContext, repo2.ID, db.ListOptions{})
134+
assert.NoError(t, err)
135+
assert.EqualValues(t, 1, cnt)
136+
assert.EqualValues(t, 24*time.Hour, pushMirrors[0].Interval)
137+
repo2PushMirrorID := pushMirrors[0].ID
138+
139+
// update repo2 push mirror
140+
assert.True(t, doUpdatePushMirror(t, session, "user2", "repo2", repo2PushMirrorID, "10m0s"))
141+
pushMirror := unittest.AssertExistsAndLoadBean(t, &repo_model.PushMirror{ID: repo2PushMirrorID})
142+
assert.EqualValues(t, 10*time.Minute, pushMirror.Interval)
143+
144+
// avoid updating repo1 push mirror
145+
assert.False(t, doUpdatePushMirror(t, session, "user2", "repo1", repo2PushMirrorID, "20m0s"))
146+
pushMirror = unittest.AssertExistsAndLoadBean(t, &repo_model.PushMirror{ID: repo2PushMirrorID})
147+
assert.EqualValues(t, 10*time.Minute, pushMirror.Interval) // not changed
148+
149+
// avoid deleting repo2 push mirror
150+
assert.False(t, doRemovePushMirror(t, session, "user2", "repo1", int(repo2PushMirrorID)))
151+
unittest.AssertExistsAndLoadBean(t, &repo_model.PushMirror{ID: repo2PushMirrorID})
152+
153+
// delete repo2 push mirror
154+
assert.True(t, doRemovePushMirror(t, session, "user2", "repo2", int(repo2PushMirrorID)))
155+
unittest.AssertNotExistsBean(t, &repo_model.PushMirror{ID: repo2PushMirrorID})
201156
}

0 commit comments

Comments
 (0)