diff --git a/modules/gitrepo/commit_test.go b/modules/gitrepo/commit_test.go index 638795a9855..a97a2bee265 100644 --- a/modules/gitrepo/commit_test.go +++ b/modules/gitrepo/commit_test.go @@ -21,19 +21,6 @@ func TestCommitsCount(t *testing.T) { assert.Equal(t, int64(3), commitsCount) } -func TestCommitsCountWithoutBase(t *testing.T) { - bareRepo1 := &mockRepository{path: "repo1_bare"} - - commitsCount, err := CommitsCount(t.Context(), bareRepo1, - CommitsCountOptions{ - Not: "master", - Revision: []string{"branch1"}, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(2), commitsCount) -} - func TestCommitsCountWithSinceUntil(t *testing.T) { bareRepo1 := &mockRepository{path: "repo1_bare"} revision := []string{"8006ff9adbf0cb94da7dad9e537e53817f9fa5c0"} @@ -65,6 +52,19 @@ func TestCommitsCountWithSinceUntil(t *testing.T) { } } +func TestCommitsCountWithoutBase(t *testing.T) { + bareRepo1 := &mockRepository{path: "repo1_bare"} + + commitsCount, err := CommitsCount(t.Context(), bareRepo1, + CommitsCountOptions{ + Not: "master", + Revision: []string{"branch1"}, + }) + + assert.NoError(t, err) + assert.Equal(t, int64(2), commitsCount) +} + func TestGetLatestCommitTime(t *testing.T) { bareRepo1 := &mockRepository{path: "repo1_bare"} lct, err := GetLatestCommitTime(t.Context(), bareRepo1) diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index 6f8aaefb6d9..fb7d2986bb6 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -258,8 +258,27 @@ func GetAllCommits(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } else if commitsCountTotal == 0 { - ctx.APIErrorNotFound() - return + // when date filters are active, a zero count may just mean no + // commits in the requested range — not that the path is invalid + if since == "" && until == "" { + ctx.APIErrorNotFound() + return + } + // verify the path actually exists in the revision history + totalWithoutDate, err := gitrepo.CommitsCount(ctx, ctx.Repo.Repository, + gitrepo.CommitsCountOptions{ + Not: not, + Revision: []string{sha}, + RelPath: []string{path}, + }) + if err != nil { + ctx.APIErrorInternal(err) + return + } + if totalWithoutDate == 0 { + ctx.APIErrorNotFound() + return + } } commits, _, err = ctx.Repo.GitRepo.CommitsByFileAndRange( diff --git a/tests/integration/api_repo_git_commits_test.go b/tests/integration/api_repo_git_commits_test.go index 9d460a20c08..d7542b38f50 100644 --- a/tests/integration/api_repo_git_commits_test.go +++ b/tests/integration/api_repo_git_commits_test.go @@ -241,3 +241,27 @@ func TestGetFileHistoryNotOnMaster(t *testing.T) { assert.Equal(t, "1", resp.Header().Get("X-Total")) } + +func TestGetFileHistoryEmptyDateRange(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.AccessTokenScopeReadRepository) + + // readme.md exists in repo16 but no commits fall before 1970, so the date + // filter yields an empty range: this must return 200 with an empty list, + // not 404 (regression: a valid path with an empty date range was a 404). + req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo16/commits?path=readme.md&sha=good-sign&until=1970-01-01T00:00:00Z", user.Name). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + + apiData := DecodeJSON(t, resp, []api.Commit{}) + assert.Empty(t, apiData) + assert.Equal(t, "0", resp.Header().Get("X-Total")) + + // a path that does not exist must still return 404 even with a date filter + req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo16/commits?path=does-not-exist.md&sha=good-sign&until=1970-01-01T00:00:00Z", user.Name). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) +}