From 7db46f9ecc7ae94d3393adc57c54d223fa6aa5a8 Mon Sep 17 00:00:00 2001
From: emilylange <git@emilylange.de>
Date: Sat, 21 Dec 2024 16:03:52 +0100
Subject: [PATCH] fix: keep commit count limit in file history pagination
 static and not increase with every page

This fixes a regression introduced by 58a4407acb04b329e94e92977b1414feb309d5de
from 2022 which reintroduced passing `--skip` to `git rev-list` in favor
of the custom skipping reader based on `io.CopyN` from 59d1cc49f1d7c63d25e2a139befc8c02b830ba09
and then forgetting to also revert the
`--max-count=CommitsRangeSize*Page` math.

Before this commit:

~~~bash
# curl -s "http://localhost:3000/api/v1/repos/forgejo/forgejo/commits?path=templates&page=1" | jq length
50

# curl -s "http://localhost:3000/api/v1/repos/forgejo/forgejo/commits?path=templates&page=2" | jq length
100

# curl -s "http://localhost:3000/api/v1/repos/forgejo/forgejo/commits?path=templates&page=10" | jq length
500
~~~

With this commit applied:

~~~bash
# curl -s "http://localhost:3000/api/v1/repos/forgejo/forgejo/commits?path=templates&page=1" | jq length
50

# curl -s "http://localhost:3000/api/v1/repos/forgejo/forgejo/commits?path=templates&page=2" | jq length
50

# curl -s "http://localhost:3000/api/v1/repos/forgejo/forgejo/commits?path=templates&page=10" | jq length
50
~~~

(cherry picked from commit cd2c1361c56d9bba0527ea3308015aa301b9ca4a)
---
 modules/git/repo_commit.go      |  2 +-
 modules/git/repo_commit_test.go | 61 +++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
index 1f3d64fe03..5ea92ed4f3 100644
--- a/modules/git/repo_commit.go
+++ b/modules/git/repo_commit.go
@@ -228,7 +228,7 @@ func (repo *Repository) CommitsByFileAndRange(opts CommitsByFileAndRangeOptions)
 	go func() {
 		stderr := strings.Builder{}
 		gitCmd := NewCommand(repo.Ctx, "rev-list").
-			AddOptionFormat("--max-count=%d", setting.Git.CommitsRangeSize*opts.Page).
+			AddOptionFormat("--max-count=%d", setting.Git.CommitsRangeSize).
 			AddOptionFormat("--skip=%d", skip)
 		gitCmd.AddDynamicArguments(opts.Revision)
 
diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go
index e2a9f97fae..cdeab3e100 100644
--- a/modules/git/repo_commit_test.go
+++ b/modules/git/repo_commit_test.go
@@ -7,6 +7,9 @@ import (
 	"path/filepath"
 	"testing"
 
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -101,3 +104,61 @@ func TestRepository_CommitsBetweenIDs(t *testing.T) {
 		assert.Len(t, commits, c.ExpectedCommits, "case %d", i)
 	}
 }
+
+func TestCommitsByRange(t *testing.T) {
+	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
+	bareRepo1, err := openRepositoryWithDefaultContext(bareRepo1Path)
+	require.NoError(t, err)
+	defer bareRepo1.Close()
+
+	baseCommit, err := bareRepo1.GetBranchCommit("master")
+	require.NoError(t, err)
+
+	testCases := []struct {
+		Page                int
+		ExpectedCommitCount int
+	}{
+		{1, 3},
+		{2, 3},
+		{3, 1},
+		{4, 0},
+	}
+	for _, testCase := range testCases {
+		commits, err := baseCommit.CommitsByRange(testCase.Page, 3, "")
+		require.NoError(t, err)
+		assert.Len(t, commits, testCase.ExpectedCommitCount, "page: %d", testCase.Page)
+	}
+}
+
+func TestCommitsByFileAndRange(t *testing.T) {
+	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
+	bareRepo1, err := openRepositoryWithDefaultContext(bareRepo1Path)
+	require.NoError(t, err)
+	defer bareRepo1.Close()
+	defer test.MockVariableValue(&setting.Git.CommitsRangeSize, 2)()
+
+	testCases := []struct {
+		File                string
+		Page                int
+		ExpectedCommitCount int
+	}{
+		{"file1.txt", 1, 1},
+		{"file2.txt", 1, 1},
+		{"file*.txt", 1, 2},
+		{"foo", 1, 2},
+		{"foo", 2, 1},
+		{"foo", 3, 0},
+		{"f*", 1, 2},
+		{"f*", 2, 2},
+		{"f*", 3, 1},
+	}
+	for _, testCase := range testCases {
+		commits, err := bareRepo1.CommitsByFileAndRange(CommitsByFileAndRangeOptions{
+			Revision: "master",
+			File:     testCase.File,
+			Page:     testCase.Page,
+		})
+		require.NoError(t, err)
+		assert.Len(t, commits, testCase.ExpectedCommitCount, "file: '%s', page: %d", testCase.File, testCase.Page)
+	}
+}