Merge pull request '[v9.0/forgejo] fix: api repo compare with commit hashes' (go-gitea#5993) from bp-v9.0/forgejo-e434ecd-d2dc4fa-1b9d124-01c9c19-ca0cd42 into v9.0/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5993
Reviewed-by: Gusted <[email protected]>

Author: Gusted

routers/api/v1/repo/pull.go

@@ -1086,10 +1086,19 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
 
 	ctx.Repo.PullRequest.SameRepo = isSameRepo
 	log.Trace("Repo path: %q, base branch: %q, head branch: %q", ctx.Repo.GitRepo.Path, baseBranch, headBranch)
+
 	// Check if base branch is valid.
-	if !ctx.Repo.GitRepo.IsBranchExist(baseBranch) && !ctx.Repo.GitRepo.IsTagExist(baseBranch) {
-		ctx.NotFound("BaseNotExist")
-		return nil, nil, nil, "", ""
+	baseIsCommit := ctx.Repo.GitRepo.IsCommitExist(baseBranch)
+	baseIsBranch := ctx.Repo.GitRepo.IsBranchExist(baseBranch)
+	baseIsTag := ctx.Repo.GitRepo.IsTagExist(baseBranch)
+	if !baseIsCommit && !baseIsBranch && !baseIsTag {
+		// Check for short SHA usage
+		if baseCommit, _ := ctx.Repo.GitRepo.GetCommit(baseBranch); baseCommit != nil {
+			baseBranch = baseCommit.ID.String()
+		} else {
+			ctx.NotFound("BaseNotExist")
+			return nil, nil, nil, "", ""
+		}
 	}
 
 	// Check if current user has fork of repository or in the same repository.
@@ -1162,13 +1171,34 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
 	}
 
 	// Check if head branch is valid.
-	if !headGitRepo.IsBranchExist(headBranch) && !headGitRepo.IsTagExist(headBranch) {
-		headGitRepo.Close()
-		ctx.NotFound()
-		return nil, nil, nil, "", ""
+	headIsCommit := headGitRepo.IsBranchExist(headBranch)
+	headIsBranch := headGitRepo.IsTagExist(headBranch)
+	headIsTag := headGitRepo.IsCommitExist(baseBranch)
+	if !headIsCommit && !headIsBranch && !headIsTag {
+		// Check if headBranch is short sha commit hash
+		if headCommit, _ := headGitRepo.GetCommit(headBranch); headCommit != nil {
+			headBranch = headCommit.ID.String()
+		} else {
+			headGitRepo.Close()
+			ctx.NotFound("IsRefExist", nil)
+			return nil, nil, nil, "", ""
+		}
+	}
+
+	baseBranchRef := baseBranch
+	if baseIsBranch {
+		baseBranchRef = git.BranchPrefix + baseBranch
+	} else if baseIsTag {
+		baseBranchRef = git.TagPrefix + baseBranch
+	}
+	headBranchRef := headBranch
+	if headIsBranch {
+		headBranchRef = headBranch
+	} else if headIsTag {
+		headBranchRef = headBranch
 	}
 
-	compareInfo, err := headGitRepo.GetCompareInfo(repo_model.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch, false, false)
+	compareInfo, err := headGitRepo.GetCompareInfo(repo_model.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranchRef, headBranchRef, false, false)
 	if err != nil {
 		headGitRepo.Close()
 		ctx.Error(http.StatusInternalServerError, "GetCompareInfo", err)

tests/integration/api_repo_compare_test.go

@@ -36,3 +36,22 @@ func TestAPICompareBranches(t *testing.T) {
 	assert.Equal(t, 2, apiResp.TotalCommits)
 	assert.Len(t, apiResp.Commits, 2)
 }
+
+func TestAPICompareCommits(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	// Login as User2.
+	session := loginUser(t, user.Name)
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+
+	req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/c8e31bc...8babce9").
+		AddTokenAuth(token)
+	resp := MakeRequest(t, req, http.StatusOK)
+
+	var apiResp *api.Compare
+	DecodeJSON(t, resp, &apiResp)
+
+	assert.Equal(t, 2, apiResp.TotalCommits)
+	assert.Len(t, apiResp.Commits, 2)
+}