Commit 473cab81 authored by Jordan Ryan Reuter's avatar Jordan Ryan Reuter Committed by Oswaldo Ferreira

Manually set total_count when paginating commits

`Kaminari.paginate_array` takes some options, most relevant of which is
a `total_count` parameter. Using the `commit_count` for `total_count`
lets us correctly treat the return of `Repository#commits` as a subset
of all the commits we may wish to list.

Addition of a new `Repository#commit_count_for_ref` method was
necessarry to allow the user to start from an arbitrary ref.

Ref #1381
parent 2995f48e
...@@ -312,11 +312,13 @@ class Repository ...@@ -312,11 +312,13 @@ class Repository
if !branch_name || branch_name == root_ref if !branch_name || branch_name == root_ref
branches.each do |branch| branches.each do |branch|
cache.expire(:"diverging_commit_counts_#{branch.name}") cache.expire(:"diverging_commit_counts_#{branch.name}")
cache.expire(:"commit_count_#{branch.name}")
end end
# In case a commit is pushed to a non-root branch we only have to flush the # In case a commit is pushed to a non-root branch we only have to flush the
# cache for said branch. # cache for said branch.
else else
cache.expire(:"diverging_commit_counts_#{branch_name}") cache.expire(:"diverging_commit_counts_#{branch_name}")
cache.expire(:"commit_count_#{branch_name}")
end end
end end
...@@ -496,6 +498,14 @@ class Repository ...@@ -496,6 +498,14 @@ class Repository
end end
cache_method :commit_count, fallback: 0 cache_method :commit_count, fallback: 0
def commit_count_for_ref(ref)
return 0 if empty?
cache.fetch(:"commit_count_#{ref}") do
raw_repository.commit_count(ref)
end
end
def branch_names def branch_names
branches.map(&:name) branches.map(&:name)
end end
......
---
title: Add pagination to project commits API endpoint
merge_request: 5298
author: Jordan Ryan Reuter
...@@ -33,7 +33,10 @@ module API ...@@ -33,7 +33,10 @@ module API
after: params[:since], after: params[:since],
before: params[:until]) before: params[:until])
present commits, with: Entities::RepoCommit commit_count = user_project.repository.commit_count_for_ref(ref)
paginated_commits = Kaminari.paginate_array(commits, total_count: commit_count)
present paginate(paginated_commits), with: Entities::RepoCommit
end end
desc 'Commit multiple file changes as one commit' do desc 'Commit multiple file changes as one commit' do
......
...@@ -1042,7 +1042,7 @@ describe Repository, models: true do ...@@ -1042,7 +1042,7 @@ describe Repository, models: true do
it 'expires the cache for all branches' do it 'expires the cache for all branches' do
expect(cache).to receive(:expire). expect(cache).to receive(:expire).
at_least(repository.branches.length). at_least(repository.branches.length * 2).
times times
repository.expire_branch_cache repository.expire_branch_cache
...@@ -1050,14 +1050,14 @@ describe Repository, models: true do ...@@ -1050,14 +1050,14 @@ describe Repository, models: true do
it 'expires the cache for all branches when the root branch is given' do it 'expires the cache for all branches when the root branch is given' do
expect(cache).to receive(:expire). expect(cache).to receive(:expire).
at_least(repository.branches.length). at_least(repository.branches.length * 2).
times times
repository.expire_branch_cache(repository.root_ref) repository.expire_branch_cache(repository.root_ref)
end end
it 'expires the cache for a specific branch' do it 'expires the cache for a specific branch' do
expect(cache).to receive(:expire).once expect(cache).to receive(:expire).twice
repository.expire_branch_cache('foo') repository.expire_branch_cache('foo')
end end
...@@ -1742,6 +1742,21 @@ describe Repository, models: true do ...@@ -1742,6 +1742,21 @@ describe Repository, models: true do
end end
end end
describe '#commit_count_for_ref' do
context 'with a non-existing repository' do
it 'returns 0' do
expect(repository).to receive(:raw_repository).and_return(nil)
expect(repository.commit_count).to eq(0)
end
end
context 'when searching for the root ref' do
it 'returns the same count as #commit_count' do
expect(repository.commit_count_for_ref(repository.root_ref)).to eq(repository.commit_count)
end
end
end
describe '#cache_method_output', caching: true do describe '#cache_method_output', caching: true do
context 'with a non-existing repository' do context 'with a non-existing repository' do
let(:value) do let(:value) do
......
...@@ -86,6 +86,43 @@ describe API::Commits, api: true do ...@@ -86,6 +86,43 @@ describe API::Commits, api: true do
expect(json_response.first["id"]).to eq("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") expect(json_response.first["id"]).to eq("570e7b2abdd848b95f2f578043fc23bd6f6fd24d")
end end
end end
context 'pagination' do
it_behaves_like 'a paginated resources'
let(:page) { 0 }
let(:per_page) { 5 }
let(:ref_name) { 'master' }
let!(:request) do
get api("/projects/#{project.id}/repository/commits?page=#{page}&per_page=#{per_page}&ref_name=#{ref_name}", user)
end
it 'returns the commit count in the correct header' do
commit_count = project.repository.commit_count_for_ref(ref_name).to_s
expect(response.headers['X-Total']).to eq(commit_count)
end
context 'viewing the first page' do
it 'returns the first 5 commits' do
commit = project.repository.commit
expect(json_response.size).to eq(per_page)
expect(json_response.first['id']).to eq(commit.id)
end
end
context 'viewing the second page' do
let(:page) { 1 }
it 'returns the second 5 commits' do
commit = project.repository.commits('HEAD', offset: per_page * page).first
expect(json_response.size).to eq(per_page)
expect(json_response.first['id']).to eq(commit.id)
end
end
end
end end
describe "Create a commit with multiple files and actions" do describe "Create a commit with multiple files and actions" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment