mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-12-27 22:23:50 +03:00
fix: keep commit count limit in file history pagination static and not increase with every page
This fixes a regression introduced by58a4407acb
from 2022 which reintroduced passing `--skip` to `git rev-list` in favor of the custom skipping reader based on `io.CopyN` from59d1cc49f1
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 commitcd2c1361c5
)
This commit is contained in:
parent
a66f8c1aff
commit
7db46f9ecc
2 changed files with 62 additions and 1 deletions
|
@ -228,7 +228,7 @@ func (repo *Repository) CommitsByFileAndRange(opts CommitsByFileAndRangeOptions)
|
||||||
go func() {
|
go func() {
|
||||||
stderr := strings.Builder{}
|
stderr := strings.Builder{}
|
||||||
gitCmd := NewCommand(repo.Ctx, "rev-list").
|
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)
|
AddOptionFormat("--skip=%d", skip)
|
||||||
gitCmd.AddDynamicArguments(opts.Revision)
|
gitCmd.AddDynamicArguments(opts.Revision)
|
||||||
|
|
||||||
|
|
|
@ -7,6 +7,9 @@ import (
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"code.gitea.io/gitea/modules/setting"
|
||||||
|
"code.gitea.io/gitea/modules/test"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
@ -101,3 +104,61 @@ func TestRepository_CommitsBetweenIDs(t *testing.T) {
|
||||||
assert.Len(t, commits, c.ExpectedCommits, "case %d", i)
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue