From 648a75e687445f441e7233b51a505706e083b1e1 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 2 Aug 2025 13:06:04 +0200 Subject: [PATCH] fix: correctly get stats for API commits (#8756) - Instead of generating a patch and parsing its contents, use a faster and simple way to get it via `--shortstat`. - Resolves forgejo/forgejo#8725 - Regression of forgejo/forgejo#7682 - Adds unit test. - Adds integration test. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8756 Reviewed-by: Earl Warren Co-authored-by: Gusted Co-committed-by: Gusted --- modules/git/repo_compare.go | 11 +++ modules/git/repo_compare_test.go | 81 +++++++++++++++++++ services/convert/git_commit.go | 11 +-- .../integration/api_repo_git_commits_test.go | 19 +++++ 4 files changed, 115 insertions(+), 7 deletions(-) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 373b5befb5..94f1911c4a 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -183,6 +183,17 @@ func (repo *Repository) GetDiffShortStat(base, head string) (numFiles, totalAddi return numFiles, totalAdditions, totalDeletions, err } +// GetCommitStat returns the number of files, total additions and total deletions the commit has. +func (repo *Repository) GetCommitShortStat(commitID string) (numFiles, totalAdditions, totalDeletions int, err error) { + cmd := NewCommand(repo.Ctx, "diff-tree", "--shortstat", "--no-commit-id", "--root").AddDynamicArguments(commitID) + stdout, _, err := cmd.RunStdString(&RunOpts{Dir: repo.Path}) + if err != nil { + return 0, 0, 0, err + } + + return parseDiffStat(stdout) +} + // GetDiffShortStat counts number of changed files, number of additions and deletions func GetDiffShortStat(ctx context.Context, repoPath string, trustedArgs TrustedCmdArgs, dynamicArgs ...string) (numFiles, totalAdditions, totalDeletions int, err error) { // Now if we call: diff --git a/modules/git/repo_compare_test.go b/modules/git/repo_compare_test.go index 86bd6855a7..b1ebdf6177 100644 --- a/modules/git/repo_compare_test.go +++ b/modules/git/repo_compare_test.go @@ -1,4 +1,5 @@ // Copyright 2018 The Gitea Authors. All rights reserved. +// Copyright 2025 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package git @@ -162,3 +163,83 @@ func TestGetCommitFilesChanged(t *testing.T) { assert.ElementsMatch(t, tc.files, changedFiles) } } + +func TestGetCommitShortStat(t *testing.T) { + t.Run("repo1_bare", func(t *testing.T) { + repo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare")) + if err != nil { + require.NoError(t, err) + return + } + defer repo.Close() + + numFiles, totalAddition, totalDeletions, err := repo.GetCommitShortStat("ce064814f4a0d337b333e646ece456cd39fab612") + require.NoError(t, err) + assert.Equal(t, 0, numFiles) + assert.Equal(t, 0, totalAddition) + assert.Equal(t, 0, totalDeletions) + + numFiles, totalAddition, totalDeletions, err = repo.GetCommitShortStat("feaf4ba6bc635fec442f46ddd4512416ec43c2c2") + require.NoError(t, err) + assert.Equal(t, 0, numFiles) + assert.Equal(t, 0, totalAddition) + assert.Equal(t, 0, totalDeletions) + + numFiles, totalAddition, totalDeletions, err = repo.GetCommitShortStat("37991dec2c8e592043f47155ce4808d4580f9123") + require.NoError(t, err) + assert.Equal(t, 1, numFiles) + assert.Equal(t, 1, totalAddition) + assert.Equal(t, 0, totalDeletions) + + numFiles, totalAddition, totalDeletions, err = repo.GetCommitShortStat("6fbd69e9823458e6c4a2fc5c0f6bc022b2f2acd1") + require.NoError(t, err) + assert.Equal(t, 2, numFiles) + assert.Equal(t, 2, totalAddition) + assert.Equal(t, 0, totalDeletions) + + numFiles, totalAddition, totalDeletions, err = repo.GetCommitShortStat("8006ff9adbf0cb94da7dad9e537e53817f9fa5c0") + require.NoError(t, err) + assert.Equal(t, 2, numFiles) + assert.Equal(t, 2, totalAddition) + assert.Equal(t, 0, totalDeletions) + + numFiles, totalAddition, totalDeletions, err = repo.GetCommitShortStat("8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2") + require.NoError(t, err) + assert.Equal(t, 1, numFiles) + assert.Equal(t, 1, totalAddition) + assert.Equal(t, 0, totalDeletions) + + numFiles, totalAddition, totalDeletions, err = repo.GetCommitShortStat("95bb4d39648ee7e325106df01a621c530863a653") + require.NoError(t, err) + assert.Equal(t, 1, numFiles) + assert.Equal(t, 1, totalAddition) + assert.Equal(t, 0, totalDeletions) + }) + + t.Run("repo6_blame_sha256", func(t *testing.T) { + repo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo6_blame_sha256")) + if err != nil { + require.NoError(t, err) + return + } + defer repo.Close() + + numFiles, totalAddition, totalDeletions, err := repo.GetCommitShortStat("e2f5660e15159082902960af0ed74fc144921d2b0c80e069361853b3ece29ba3") + require.NoError(t, err) + assert.Equal(t, 1, numFiles) + assert.Equal(t, 1, totalAddition) + assert.Equal(t, 0, totalDeletions) + + numFiles, totalAddition, totalDeletions, err = repo.GetCommitShortStat("9347b0198cd1f25017579b79d0938fa89dba34ad2514f0dd92f6bc975ed1a2fe") + require.NoError(t, err) + assert.Equal(t, 1, numFiles) + assert.Equal(t, 1, totalAddition) + assert.Equal(t, 1, totalDeletions) + + numFiles, totalAddition, totalDeletions, err = repo.GetCommitShortStat("ab2b57a4fa476fb2edb74dafa577caf918561abbaa8fba0c8dc63c412e17a7cc") + require.NoError(t, err) + assert.Equal(t, 1, numFiles) + assert.Equal(t, 6, totalAddition) + assert.Equal(t, 0, totalDeletions) + }) +} diff --git a/services/convert/git_commit.go b/services/convert/git_commit.go index 4603cfac4d..6a691966b8 100644 --- a/services/convert/git_commit.go +++ b/services/convert/git_commit.go @@ -15,7 +15,6 @@ import ( api "forgejo.org/modules/structs" "forgejo.org/modules/util" ctx "forgejo.org/services/context" - "forgejo.org/services/gitdiff" ) // ToCommitUser convert a git.Signature to an api.CommitUser @@ -210,17 +209,15 @@ func ToCommit(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Rep // Get diff stats for commit if opts.Stat { - diff, _, err := gitdiff.GetDiffSimple(ctx, gitRepo, &gitdiff.DiffOptions{ - AfterCommitID: commit.ID.String(), - }) + _, totalAdditions, totalDeletions, err := gitRepo.GetCommitShortStat(commit.ID.String()) if err != nil { return nil, err } res.Stats = &api.CommitStats{ - Total: diff.TotalAddition + diff.TotalDeletion, - Additions: diff.TotalAddition, - Deletions: diff.TotalDeletion, + Total: totalAdditions + totalDeletions, + Additions: totalAdditions, + Deletions: totalDeletions, } } diff --git a/tests/integration/api_repo_git_commits_test.go b/tests/integration/api_repo_git_commits_test.go index 7a93029d4c..db73307653 100644 --- a/tests/integration/api_repo_git_commits_test.go +++ b/tests/integration/api_repo_git_commits_test.go @@ -231,3 +231,22 @@ func TestGetFileHistoryNotOnMaster(t *testing.T) { assert.Equal(t, "1", resp.Header().Get("X-Total")) } + +func TestAPIReposGitCommit(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo16/git/commits/f27c2b2b03dcab38beaf89b0ab4ff61f6de63441"). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + + var apiData api.Commit + DecodeJSON(t, resp, &apiData) + + assert.Equal(t, "f27c2b2b03dcab38beaf89b0ab4ff61f6de63441", apiData.CommitMeta.SHA) + assert.Equal(t, 1, apiData.Stats.Total) + assert.Equal(t, 1, apiData.Stats.Additions) + assert.Equal(t, 0, apiData.Stats.Deletions) +}