From da15d6127c8d430dfc069f9815ce783dd9ca35f7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 6 Mar 2024 13:52:48 +0800 Subject: [PATCH] A small refactor for agit implementation (#29614) This PR made the code simpler, reduced unnecessary database queries and fixed some warnning for the errors.New . --------- Co-authored-by: KN4CK3R --- services/agit/agit.go | 73 +++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/services/agit/agit.go b/services/agit/agit.go index 2233fe8547..eb3bafa906 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "os" + "strconv" "strings" issues_model "code.gitea.io/gitea/models/issues" @@ -21,26 +22,17 @@ import ( // ProcReceive handle proc receive work func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, opts *private.HookOptions) ([]private.HookProcReceiveRefResult, error) { - // TODO: Add more options? - var ( - topicBranch string - title string - description string - forcePush bool - ) - results := make([]private.HookProcReceiveRefResult, 0, len(opts.OldCommitIDs)) - - ownerName := repo.OwnerName - repoName := repo.Name - - topicBranch = opts.GitPushOptions["topic"] - _, forcePush = opts.GitPushOptions["force-push"] + topicBranch := opts.GitPushOptions["topic"] + forcePush, _ := strconv.ParseBool(opts.GitPushOptions["force-push"]) + title := strings.TrimSpace(opts.GitPushOptions["title"]) + description := strings.TrimSpace(opts.GitPushOptions["description"]) // TODO: Add more options? objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) + userName := strings.ToLower(opts.UserName) pusher, err := user_model.GetUserByID(ctx, opts.UserID) if err != nil { - return nil, fmt.Errorf("Failed to get user. Error: %w", err) + return nil, fmt.Errorf("failed to get user. Error: %w", err) } for i := range opts.OldCommitIDs { @@ -85,9 +77,6 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. continue } - var headBranch string - userName := strings.ToLower(opts.UserName) - if len(curentTopicBranch) == 0 { curentTopicBranch = topicBranch } @@ -95,6 +84,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. // because different user maybe want to use same topic, // So it's better to make sure the topic branch name // has user name prefix + var headBranch string if !strings.HasPrefix(curentTopicBranch, userName+"/") { headBranch = userName + "/" + curentTopicBranch } else { @@ -104,21 +94,26 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. pr, err := issues_model.GetUnmergedPullRequest(ctx, repo.ID, repo.ID, headBranch, baseBranchName, issues_model.PullRequestFlowAGit) if err != nil { if !issues_model.IsErrPullRequestNotExist(err) { - return nil, fmt.Errorf("Failed to get unmerged agit flow pull request in repository: %s/%s Error: %w", ownerName, repoName, err) + return nil, fmt.Errorf("failed to get unmerged agit flow pull request in repository: %s Error: %w", repo.FullName(), err) + } + + var commit *git.Commit + if title == "" || description == "" { + commit, err = gitRepo.GetCommit(opts.NewCommitIDs[i]) + if err != nil { + return nil, fmt.Errorf("failed to get commit %s in repository: %s Error: %w", opts.NewCommitIDs[i], repo.FullName(), err) + } } // create a new pull request - if len(title) == 0 { - var has bool - title, has = opts.GitPushOptions["title"] - if !has || len(title) == 0 { - commit, err := gitRepo.GetCommit(opts.NewCommitIDs[i]) - if err != nil { - return nil, fmt.Errorf("Failed to get commit %s in repository: %s/%s Error: %w", opts.NewCommitIDs[i], ownerName, repoName, err) - } - title = strings.Split(commit.CommitMessage, "\n")[0] - } - description = opts.GitPushOptions["description"] + if title == "" { + title = strings.Split(commit.CommitMessage, "\n")[0] + } + if description == "" { + _, description, _ = strings.Cut(commit.CommitMessage, "\n\n") + } + if description == "" { + description = title } prIssue := &issues_model.Issue{ @@ -160,12 +155,12 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. // update exist pull request if err := pr.LoadBaseRepo(ctx); err != nil { - return nil, fmt.Errorf("Unable to load base repository for PR[%d] Error: %w", pr.ID, err) + return nil, fmt.Errorf("unable to load base repository for PR[%d] Error: %w", pr.ID, err) } oldCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName()) if err != nil { - return nil, fmt.Errorf("Unable to get ref commit id in base repository for PR[%d] Error: %w", pr.ID, err) + return nil, fmt.Errorf("unable to get ref commit id in base repository for PR[%d] Error: %w", pr.ID, err) } if oldCommitID == opts.NewCommitIDs[i] { @@ -179,9 +174,11 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. } if !forcePush { - output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1").AddDynamicArguments(oldCommitID, "^"+opts.NewCommitIDs[i]).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: os.Environ()}) + output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1"). + AddDynamicArguments(oldCommitID, "^"+opts.NewCommitIDs[i]). + RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: os.Environ()}) if err != nil { - return nil, fmt.Errorf("Fail to detect force push: %w", err) + return nil, fmt.Errorf("failed to detect force push: %w", err) } else if len(output) > 0 { results = append(results, private.HookProcReceiveRefResult{ OriginalRef: opts.RefFullNames[i], @@ -195,17 +192,13 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. pr.HeadCommitID = opts.NewCommitIDs[i] if err = pull_service.UpdateRef(ctx, pr); err != nil { - return nil, fmt.Errorf("Failed to update pull ref. Error: %w", err) + return nil, fmt.Errorf("failed to update pull ref. Error: %w", err) } pull_service.AddToTaskQueue(ctx, pr) - pusher, err := user_model.GetUserByID(ctx, opts.UserID) - if err != nil { - return nil, fmt.Errorf("Failed to get user. Error: %w", err) - } err = pr.LoadIssue(ctx) if err != nil { - return nil, fmt.Errorf("Failed to load pull issue. Error: %w", err) + return nil, fmt.Errorf("failed to load pull issue. Error: %w", err) } comment, err := pull_service.CreatePushPullComment(ctx, pusher, pr, oldCommitID, opts.NewCommitIDs[i]) if err == nil && comment != nil {