From 473cab818aff034d072f0f6c8537a584bc5aa41c Mon Sep 17 00:00:00 2001
From: Jordan Ryan Reuter <jordan.reuter@scimedsolutions.com>
Date: Fri, 20 Jan 2017 10:04:16 -0500
Subject: [PATCH] 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
---
app/models/repository.rb | 10 +++++
.../1381-use-commit-count-for-pagination.yml | 4 ++
lib/api/commits.rb | 5 ++-
spec/models/repository_spec.rb | 21 +++++++++--
spec/requests/api/commits_spec.rb | 37 +++++++++++++++++++
5 files changed, 73 insertions(+), 4 deletions(-)
create mode 100644 changelogs/unreleased/1381-use-commit-count-for-pagination.yml
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 2a12b36a84d..c0c5816d151 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -312,11 +312,13 @@ class Repository
if !branch_name || branch_name == root_ref
branches.each do |branch|
cache.expire(:"diverging_commit_counts_#{branch.name}")
+ cache.expire(:"commit_count_#{branch.name}")
end
# In case a commit is pushed to a non-root branch we only have to flush the
# cache for said branch.
else
cache.expire(:"diverging_commit_counts_#{branch_name}")
+ cache.expire(:"commit_count_#{branch_name}")
end
end
@@ -496,6 +498,14 @@ class Repository
end
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
branches.map(&:name)
end
diff --git a/changelogs/unreleased/1381-use-commit-count-for-pagination.yml b/changelogs/unreleased/1381-use-commit-count-for-pagination.yml
new file mode 100644
index 00000000000..ae84f64ed03
--- /dev/null
+++ b/changelogs/unreleased/1381-use-commit-count-for-pagination.yml
@@ -0,0 +1,4 @@
+---
+title: Add pagination to project commits API endpoint
+merge_request: 5298
+author: Jordan Ryan Reuter
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index b0aa10f8bf2..6205bff3bc0 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -33,7 +33,10 @@ module API
after: params[:since],
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
desc 'Commit multiple file changes as one commit' do
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index eb992e1354e..45acdc52d08 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1042,7 +1042,7 @@ describe Repository, models: true do
it 'expires the cache for all branches' do
expect(cache).to receive(:expire).
- at_least(repository.branches.length).
+ at_least(repository.branches.length * 2).
times
repository.expire_branch_cache
@@ -1050,14 +1050,14 @@ describe Repository, models: true do
it 'expires the cache for all branches when the root branch is given' do
expect(cache).to receive(:expire).
- at_least(repository.branches.length).
+ at_least(repository.branches.length * 2).
times
repository.expire_branch_cache(repository.root_ref)
end
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')
end
@@ -1742,6 +1742,21 @@ describe Repository, models: true do
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
context 'with a non-existing repository' do
let(:value) do
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 5190fcca2d1..1d298988f17 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -86,6 +86,43 @@ describe API::Commits, api: true do
expect(json_response.first["id"]).to eq("570e7b2abdd848b95f2f578043fc23bd6f6fd24d")
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
describe "Create a commit with multiple files and actions" do
--
GitLab