Skip to content

Commit 430d9df

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent b8a4432 commit 430d9df

File tree

7 files changed

+86
-33
lines changed

7 files changed

+86
-33
lines changed

‎cmd/hook.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ Gitea or set your environment appropriately.`, "")
352352
GitQuarantinePath: os.Getenv(private.GitQuarantinePath),
353353
GitPushOptions: pushOptions(),
354354
PullRequestID: prID,
355-
PushTrigger: os.Getenv(repo_module.EnvPushTrigger),
355+
PushTrigger: repo_module.PushTrigger(os.Getenv(repo_module.EnvPushTrigger)),
356356
}
357357
oldCommitIDs := make([]string, hookBatchSize)
358358
newCommitIDs := make([]string, hookBatchSize)

‎modules/private/hook.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"code.gitea.io/gitea/modules/git"
1414
"code.gitea.io/gitea/modules/optional"
15+
"code.gitea.io/gitea/modules/repository"
1516
"code.gitea.io/gitea/modules/setting"
1617
)
1718

@@ -54,7 +55,7 @@ type HookOptions struct {
5455
GitQuarantinePath string
5556
GitPushOptions GitPushOptions
5657
PullRequestID int64
57-
PushTrigger string
58+
PushTrigger repository.PushTrigger
5859
DeployKeyID int64 // if the pusher is a DeployKey, then UserID is the repo's org user.
5960
IsUncyclo bool
6061
ActionPerm int

‎modules/repository/env.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ const (
3434
type PushTrigger string
3535

3636
const (
37-
PushTriggerPRMergeToBase PushTrigger = "pr-merge-to-base"
37+
PushTriggerPRMergeToBase PushTrigger = "pr-merge-to-base"
38+
PushTriggerPRUpdateWithBase PushTrigger = "pr-update-with-base"
3839
)
3940

4041
// InternalPushingEnvironment returns an os environment to switch off hooks on push

‎routers/private/hook_post_receive.go

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,11 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
164164
}
165165

166166
// handle pull request merging, a pull request action should push at least 1 commit
167-
handlePullRequestMerging(ctx, opts, ownerName, repoName, updates)
168-
if ctx.Written() {
169-
return
167+
if opts.PushTrigger == repo_module.PushTriggerPRMergeToBase {
168+
handlePullRequestMerging(ctx, opts, ownerName, repoName, updates)
169+
if ctx.Written() {
170+
return
171+
}
170172
}
171173

172174
isPrivate := opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate)
@@ -183,9 +185,7 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
183185
wasEmpty = repo.IsEmpty
184186
}
185187

186-
pusher, err := cache.GetWithContextCache(ctx, contextCachePusherKey, opts.UserID, func() (*user_model.User, error) {
187-
return user_model.GetUserByID(ctx, opts.UserID)
188-
})
188+
pusher, err := loadContextCacheUser(ctx, opts.UserID)
189189
if err != nil {
190190
log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
191191
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
@@ -321,55 +321,55 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
321321
})
322322
}
323323

324-
const contextCachePusherKey = "hook_post_receive_pusher"
324+
func loadContextCacheUser(ctx context.Context, id int64) (*user_model.User, error) {
325+
return cache.GetWithContextCache(ctx, "hook_post_receive_user", id, func() (*user_model.User, error) {
326+
return user_model.GetUserByID(ctx, id)
327+
})
328+
}
325329

326330
// handlePullRequestMerging handle pull request merging, a pull request action should only push 1 commit
327331
func handlePullRequestMerging(ctx *gitea_context.PrivateContext, opts *private.HookOptions, ownerName, repoName string, updates []*repo_module.PushUpdateOptions) {
328-
if opts.PushTrigger != string(repo_module.PushTriggerPRMergeToBase) || len(updates) < 1 {
332+
if len(updates) == 0 {
333+
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
334+
Err: fmt.Sprintf("Pushing a merged PR (pr:%d) no commits pushed ", opts.PullRequestID),
335+
})
329336
return
330337
}
338+
if len(updates) != 1 && !setting.IsProd {
339+
// it shouldn't happen
340+
panic(fmt.Sprintf("Pushing a merged PR (pr:%d) should only push 1 commit, but got %d", opts.PullRequestID, len(updates)))
341+
}
331342

332-
// Get the pull request
333343
pr, err := issues_model.GetPullRequestByID(ctx, opts.PullRequestID)
334344
if err != nil {
335345
log.Error("GetPullRequestByID[%d]: %v", opts.PullRequestID, err)
336-
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
337-
Err: fmt.Sprintf("GetPullRequestByID[%d]: %v", opts.PullRequestID, err),
338-
})
346+
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "GetPullRequestByID failed"})
339347
return
340348
}
341349

342-
pusher, err := cache.GetWithContextCache(ctx, contextCachePusherKey, opts.UserID, func() (*user_model.User, error) {
343-
return user_model.GetUserByID(ctx, opts.UserID)
344-
})
350+
pusher, err := loadContextCacheUser(ctx, opts.UserID)
345351
if err != nil {
346352
log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
347-
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
348-
Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
349-
})
353+
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Load pusher user failed"})
350354
return
351355
}
352356

353357
pr.MergedCommitID = updates[len(updates)-1].NewCommitID
354358
pr.MergedUnix = timeutil.TimeStampNow()
355359
pr.Merger = pusher
356-
pr.MergerID = opts.UserID
357-
358-
if err := db.WithTx(ctx, func(ctx context.Context) error {
360+
pr.MergerID = pusher.ID
361+
err = db.WithTx(ctx, func(ctx context.Context) error {
359362
// Removing an auto merge pull and ignore if not exist
360363
if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) {
361364
return fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", opts.PullRequestID, err)
362365
}
363-
364366
if _, err := pr.SetMerged(ctx); err != nil {
365-
return fmt.Errorf("Failed to SetMerged: %s/%s Error: %v", ownerName, repoName, err)
367+
return fmt.Errorf("SetMerged failed: %s/%s Error: %v", ownerName, repoName, err)
366368
}
367369
return nil
368-
}); err != nil {
369-
log.Error("%v", err)
370-
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
371-
Err: err.Error(),
372-
})
373-
return
370+
})
371+
if err != nil {
372+
log.Error("Failed to update PR to merged: %v", err)
373+
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Failed to update PR to merged"})
374374
}
375375
}

‎routers/private/hook_post_receive_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package private
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/models/db"
10+
issues_model "code.gitea.io/gitea/models/issues"
11+
"code.gitea.io/gitea/models/unittest"
12+
"code.gitea.io/gitea/modules/private"
13+
repo_module "code.gitea.io/gitea/modules/repository"
14+
"code.gitea.io/gitea/services/contexttest"
15+
16+
"github.com/stretchr/testify/assert"
17+
)
18+
19+
func TestHandlePullRequestMerging(t *testing.T) {
20+
assert.NoError(t, unittest.PrepareTestDatabase())
21+
pr, err := issues_model.GetUnmergedPullRequest(db.DefaultContext, 1, 1, "branch2", "master", issues_model.PullRequestFlowGithub)
22+
assert.NoError(t, err)
23+
assert.NoError(t, pr.LoadBaseRepo(db.DefaultContext))
24+
ctx, resp := contexttest.MockPrivateContext(t, "/")
25+
handlePullRequestMerging(ctx, &private.HookOptions{
26+
PullRequestID: pr.ID,
27+
UserID: 2,
28+
}, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, []*repo_module.PushUpdateOptions{
29+
{NewCommitID: "01234567"},
30+
})
31+
assert.Equal(t, 0, len(resp.Body.String()))
32+
pr, err = issues_model.GetPullRequestByID(db.DefaultContext, pr.ID)
33+
assert.NoError(t, err)
34+
assert.True(t, pr.HasMerged)
35+
assert.EqualValues(t, "01234567", pr.MergedCommitID)
36+
// TODO: test the removal of auto merge
37+
}

‎services/contexttest/context_tests.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,19 @@ func MockAPIContext(t *testing.T, reqPath string) (*context.APIContext, *httptes
9494
return ctx, resp
9595
}
9696

97+
func MockPrivateContext(t *testing.T, reqPath string) (*context.PrivateContext, *httptest.ResponseRecorder) {
98+
resp := httptest.NewRecorder()
99+
req := mockRequest(t, reqPath)
100+
base, baseCleanUp := context.NewBaseContext(resp, req)
101+
base.Data = middleware.GetContextData(req.Context())
102+
base.Locale = &translation.MockLocale{}
103+
ctx := &context.PrivateContext{Base: base}
104+
_ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later
105+
chiCtx := chi.NewRouteContext()
106+
ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx)
107+
return ctx, resp
108+
}
109+
97110
// LoadRepo load a repo into a test context.
98111
func LoadRepo(t *testing.T, ctx gocontext.Context, repoID int64) {
99112
var doer *user_model.User

‎services/pull/update.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
user_model "code.gitea.io/gitea/models/user"
1616
"code.gitea.io/gitea/modules/git"
1717
"code.gitea.io/gitea/modules/log"
18+
"code.gitea.io/gitea/modules/repository"
1819
)
1920

2021
// Update updates pull request with base branch.
@@ -72,7 +73,7 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
7273
BaseBranch: pr.HeadBranch,
7374
}
7475

75-
_, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, "")
76+
_, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, repository.PushTriggerPRUpdateWithBase)
7677

7778
defer func() {
7879
go AddTestPullRequestTask(doer, reversePR.HeadRepo.ID, reversePR.HeadBranch, false, "", "")

0 commit comments

Comments
 (0)