]> source.dussan.org Git - gitea.git/commitdiff
Performance optimization for git push (#30104)
authorLunny Xiao <xiaolunwen@gmail.com>
Tue, 9 Apr 2024 03:43:17 +0000 (11:43 +0800)
committerGitHub <noreply@github.com>
Tue, 9 Apr 2024 03:43:17 +0000 (03:43 +0000)
Agit returned result should be from `ProcReceive` hook but not
`PostReceive` hook. Then for all non-agit pull requests, it will not
check the pull requests for every pushing `refs/pull/%d/head`.

cmd/hook.go
modules/private/hook.go
routers/private/hook_post_receive.go
services/agit/agit.go
tests/integration/git_push_test.go

index 6a3358853ddc9b4ac5035ecb849f6cd0dd9e0a00..c04591d79ec2876bc0d071aefa47d389b106809d 100644 (file)
@@ -448,21 +448,24 @@ Gitea or set your environment appropriately.`, "")
 
 func hookPrintResults(results []private.HookPostReceiveBranchResult) {
        for _, res := range results {
-               if !res.Message {
-                       continue
-               }
+               hookPrintResult(res.Message, res.Create, res.Branch, res.URL)
+       }
+}
 
-               fmt.Fprintln(os.Stderr, "")
-               if res.Create {
-                       fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", res.Branch)
-                       fmt.Fprintf(os.Stderr, "  %s\n", res.URL)
-               } else {
-                       fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
-                       fmt.Fprintf(os.Stderr, "  %s\n", res.URL)
-               }
-               fmt.Fprintln(os.Stderr, "")
-               os.Stderr.Sync()
+func hookPrintResult(output, isCreate bool, branch, url string) {
+       if !output {
+               return
+       }
+       fmt.Fprintln(os.Stderr, "")
+       if isCreate {
+               fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", branch)
+               fmt.Fprintf(os.Stderr, "  %s\n", url)
+       } else {
+               fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
+               fmt.Fprintf(os.Stderr, "  %s\n", url)
        }
+       fmt.Fprintln(os.Stderr, "")
+       os.Stderr.Sync()
 }
 
 func pushOptions() map[string]string {
@@ -691,6 +694,12 @@ Gitea or set your environment appropriately.`, "")
        }
        err = writeFlushPktLine(ctx, os.Stdout)
 
+       if err == nil {
+               for _, res := range resp.Results {
+                       hookPrintResult(res.ShouldShowMessage, res.IsCreatePR, res.HeadBranch, res.URL)
+               }
+       }
+
        return err
 }
 
index cab8c812248f3c68c5e0f8a1587f020d958b9d92..79c3d482298ccf9235d77500decef0799afd7cc2 100644 (file)
@@ -11,6 +11,7 @@ import (
        "time"
 
        "code.gitea.io/gitea/modules/git"
+       "code.gitea.io/gitea/modules/optional"
        "code.gitea.io/gitea/modules/setting"
 )
 
@@ -32,13 +33,13 @@ const (
 )
 
 // Bool checks for a key in the map and parses as a boolean
-func (g GitPushOptions) Bool(key string, def bool) bool {
+func (g GitPushOptions) Bool(key string) optional.Option[bool] {
        if val, ok := g[key]; ok {
                if b, err := strconv.ParseBool(val); err == nil {
-                       return b
+                       return optional.Some(b)
                }
        }
-       return def
+       return optional.None[bool]()
 }
 
 // HookOptions represents the options for the Hook calls
@@ -87,13 +88,17 @@ type HookProcReceiveResult struct {
 
 // HookProcReceiveRefResult represents an individual result from ProcReceive
 type HookProcReceiveRefResult struct {
-       OldOID       string
-       NewOID       string
-       Ref          string
-       OriginalRef  git.RefName
-       IsForcePush  bool
-       IsNotMatched bool
-       Err          string
+       OldOID            string
+       NewOID            string
+       Ref               string
+       OriginalRef       git.RefName
+       IsForcePush       bool
+       IsNotMatched      bool
+       Err               string
+       IsCreatePR        bool
+       URL               string
+       ShouldShowMessage bool
+       HeadBranch        string
 }
 
 // HookPreReceive check whether the provided commits are allowed
index 101ae923024947bf294efc28db1630022513340c..769a68970d210f5e2c5a7f9512937d55076e4256 100644 (file)
@@ -6,11 +6,12 @@ package private
 import (
        "fmt"
        "net/http"
-       "strconv"
 
        git_model "code.gitea.io/gitea/models/git"
        issues_model "code.gitea.io/gitea/models/issues"
+       access_model "code.gitea.io/gitea/models/perm/access"
        repo_model "code.gitea.io/gitea/models/repo"
+       user_model "code.gitea.io/gitea/models/user"
        "code.gitea.io/gitea/modules/git"
        "code.gitea.io/gitea/modules/gitrepo"
        "code.gitea.io/gitea/modules/log"
@@ -159,8 +160,10 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
                }
        }
 
+       isPrivate := opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate)
+       isTemplate := opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate)
        // Handle Push Options
-       if len(opts.GitPushOptions) > 0 {
+       if isPrivate.Has() || isTemplate.Has() {
                // load the repository
                if repo == nil {
                        repo = loadRepository(ctx, ownerName, repoName)
@@ -171,13 +174,49 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
                        wasEmpty = repo.IsEmpty
                }
 
-               repo.IsPrivate = opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate, repo.IsPrivate)
-               repo.IsTemplate = opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate, repo.IsTemplate)
-               if err := repo_model.UpdateRepositoryCols(ctx, repo, "is_private", "is_template"); err != nil {
+               pusher, err := user_model.GetUserByID(ctx, opts.UserID)
+               if err != nil {
                        log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
                        ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
                                Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
                        })
+                       return
+               }
+               perm, err := access_model.GetUserRepoPermission(ctx, repo, pusher)
+               if err != nil {
+                       log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
+                       ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
+                               Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
+                       })
+                       return
+               }
+               if !perm.IsOwner() && !perm.IsAdmin() {
+                       ctx.JSON(http.StatusNotFound, private.HookPostReceiveResult{
+                               Err: "Permissions denied",
+                       })
+                       return
+               }
+
+               cols := make([]string, 0, len(opts.GitPushOptions))
+
+               if isPrivate.Has() {
+                       repo.IsPrivate = isPrivate.Value()
+                       cols = append(cols, "is_private")
+               }
+
+               if isTemplate.Has() {
+                       repo.IsTemplate = isTemplate.Value()
+                       cols = append(cols, "is_template")
+               }
+
+               if len(cols) > 0 {
+                       if err := repo_model.UpdateRepositoryCols(ctx, repo, cols...); err != nil {
+                               log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
+                               ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
+                                       Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
+                               })
+                               return
+                       }
                }
        }
 
@@ -192,42 +231,6 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
                refFullName := opts.RefFullNames[i]
                newCommitID := opts.NewCommitIDs[i]
 
-               // post update for agit pull request
-               // FIXME: use pr.Flow to test whether it's an Agit PR or a GH PR
-               if git.DefaultFeatures.SupportProcReceive && refFullName.IsPull() {
-                       if repo == nil {
-                               repo = loadRepository(ctx, ownerName, repoName)
-                               if ctx.Written() {
-                                       return
-                               }
-                       }
-
-                       pullIndex, _ := strconv.ParseInt(refFullName.PullName(), 10, 64)
-                       if pullIndex <= 0 {
-                               continue
-                       }
-
-                       pr, err := issues_model.GetPullRequestByIndex(ctx, repo.ID, pullIndex)
-                       if err != nil && !issues_model.IsErrPullRequestNotExist(err) {
-                               log.Error("Failed to get PR by index %v Error: %v", pullIndex, err)
-                               ctx.JSON(http.StatusInternalServerError, private.Response{
-                                       Err: fmt.Sprintf("Failed to get PR by index %v Error: %v", pullIndex, err),
-                               })
-                               return
-                       }
-                       if pr == nil {
-                               continue
-                       }
-
-                       results = append(results, private.HookPostReceiveBranchResult{
-                               Message: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
-                               Create:  false,
-                               Branch:  "",
-                               URL:     fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
-                       })
-                       continue
-               }
-
                // If we've pushed a branch (and not deleted it)
                if !git.IsEmptyCommitID(newCommitID) && refFullName.IsBranch() {
                        // First ensure we have the repository loaded, we're allowed pulls requests and we can get the base repo
index eb3bafa9065578d8bc9f90a900e3c072b70873ef..52a70469e0c0fcab935853e7f662d821dec2e40b 100644 (file)
@@ -16,6 +16,7 @@ import (
        "code.gitea.io/gitea/modules/git"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/private"
+       "code.gitea.io/gitea/modules/setting"
        notify_service "code.gitea.io/gitea/services/notify"
        pull_service "code.gitea.io/gitea/services/pull"
 )
@@ -145,10 +146,14 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
                        log.Trace("Pull request created: %d/%d", repo.ID, prIssue.ID)
 
                        results = append(results, private.HookProcReceiveRefResult{
-                               Ref:         pr.GetGitRefName(),
-                               OriginalRef: opts.RefFullNames[i],
-                               OldOID:      objectFormat.EmptyObjectID().String(),
-                               NewOID:      opts.NewCommitIDs[i],
+                               Ref:               pr.GetGitRefName(),
+                               OriginalRef:       opts.RefFullNames[i],
+                               OldOID:            objectFormat.EmptyObjectID().String(),
+                               NewOID:            opts.NewCommitIDs[i],
+                               IsCreatePR:        true,
+                               URL:               fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
+                               ShouldShowMessage: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
+                               HeadBranch:        headBranch,
                        })
                        continue
                }
@@ -208,11 +213,14 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
                isForcePush := comment != nil && comment.IsForcePush
 
                results = append(results, private.HookProcReceiveRefResult{
-                       OldOID:      oldCommitID,
-                       NewOID:      opts.NewCommitIDs[i],
-                       Ref:         pr.GetGitRefName(),
-                       OriginalRef: opts.RefFullNames[i],
-                       IsForcePush: isForcePush,
+                       OldOID:            oldCommitID,
+                       NewOID:            opts.NewCommitIDs[i],
+                       Ref:               pr.GetGitRefName(),
+                       OriginalRef:       opts.RefFullNames[i],
+                       IsForcePush:       isForcePush,
+                       IsCreatePR:        false,
+                       URL:               fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
+                       ShouldShowMessage: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
                })
        }
 
index 0a357248077eaf9d6321115b7ed43d5f57807c1c..b37fb02444e7265f16e35ddc5fbcca87c5f55400 100644 (file)
@@ -49,6 +49,17 @@ func testGitPush(t *testing.T, u *url.URL) {
                })
        })
 
+       t.Run("Push branch with options", func(t *testing.T) {
+               runTestGitPush(t, u, func(t *testing.T, gitPath string) (pushed, deleted []string) {
+                       branchName := "branch-with-options"
+                       doGitCreateBranch(gitPath, branchName)(t)
+                       doGitPushTestRepository(gitPath, "origin", branchName, "-o", "repo.private=true", "-o", "repo.template=true")(t)
+                       pushed = append(pushed, branchName)
+
+                       return pushed, deleted
+               })
+       })
+
        t.Run("Delete branches", func(t *testing.T) {
                runTestGitPush(t, u, func(t *testing.T, gitPath string) (pushed, deleted []string) {
                        doGitPushTestRepository(gitPath, "origin", "master")(t) // make sure master is the default branch instead of a branch we are going to delete