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 <admin@oldschoolhack.me>
go-gitea · Mar 6, 2024 · da15d61 · da15d61
1 parent 8d32f3c
commit da15d61
Showing 1 changed file with 33 additions and 40 deletions.
diff --git 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,16 +77,14 @@ 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
 		}
 
 		// 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)
 			}
 
-			// 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]
+			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)
 				}
-				description = opts.GitPushOptions["description"]
+			}
+
+			// create a new pull request
+			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 {