Commit 082034c0 authored by Douwe Maan's avatar Douwe Maan
Browse files

Merge branch '42434-allow-commits-endpoint-to-work-over-all-commits' into 'master'

Resolve "Allow API method /projects/:id/repository/commits  to work over all commits"

Closes #42434

See merge request gitlab-org/gitlab-ce!17182
......@@ -139,7 +139,7 @@ def commits_by(oids:)
end
end
def commits(ref, path: nil, limit: nil, offset: nil, skip_merges: false, after: nil, before: nil)
def commits(ref = nil, path: nil, limit: nil, offset: nil, skip_merges: false, after: nil, before: nil, all: nil)
options = {
repo: raw_repository,
ref: ref,
......@@ -149,7 +149,8 @@ def commits(ref, path: nil, limit: nil, offset: nil, skip_merges: false, after:
after: after,
before: before,
follow: Array(path).length == 1,
skip_merges: skip_merges
skip_merges: skip_merges,
all: all
}
commits = Gitlab::Git::Commit.where(options)
......
---
title: Allow commits endpoint to work over all commits of a repository
merge_request: 17182
author:
type: added
......@@ -14,6 +14,9 @@ GET /projects/:id/repository/commits
| `ref_name` | string | no | The name of a repository branch or tag or if not given the default branch |
| `since` | string | no | Only commits after or on this date will be returned in ISO 8601 format YYYY-MM-DDTHH:MM:SSZ |
| `until` | string | no | Only commits before or on this date will be returned in ISO 8601 format YYYY-MM-DDTHH:MM:SSZ |
| `path` | string | no | The file path |
| `all` | boolean | no | Retrieve every commit from the repository |
```bash
curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/5/repository/commits"
......
......@@ -18,25 +18,28 @@ class Commits < Grape::API
optional :since, type: DateTime, desc: 'Only commits after or on this date will be returned'
optional :until, type: DateTime, desc: 'Only commits before or on this date will be returned'
optional :path, type: String, desc: 'The file path'
optional :all, type: Boolean, desc: 'Every commit will be returned'
use :pagination
end
get ':id/repository/commits' do
path = params[:path]
before = params[:until]
after = params[:since]
ref = params[:ref_name] || user_project.try(:default_branch) || 'master'
ref = params[:ref_name] || user_project.try(:default_branch) || 'master' unless params[:all]
offset = (params[:page] - 1) * params[:per_page]
all = params[:all]
commits = user_project.repository.commits(ref,
path: path,
limit: params[:per_page],
offset: offset,
before: before,
after: after)
after: after,
all: all)
commit_count =
if path || before || after
user_project.repository.count_commits(ref: ref, path: path, before: before, after: after)
if all || path || before || after
user_project.repository.count_commits(ref: ref, path: path, before: before, after: after, all: all)
else
# Cacheable commit count.
user_project.repository.commit_count_for_ref(ref)
......
......@@ -467,7 +467,8 @@ def log(options)
follow: false,
skip_merges: false,
after: nil,
before: nil
before: nil,
all: false
}
options = default_options.merge(options)
......@@ -478,8 +479,9 @@ def log(options)
raise ArgumentError.new("invalid Repository#log limit: #{limit.inspect}")
end
# TODO support options[:all] in Gitaly https://gitlab.com/gitlab-org/gitaly/issues/1049
gitaly_migrate(:find_commits) do |is_enabled|
if is_enabled
if is_enabled && !options[:all]
gitaly_commit_client.find_commits(options)
else
raw_log(options).map { |c| Commit.decorate(self, c) }
......@@ -489,13 +491,16 @@ def log(options)
# Used in gitaly-ruby
def raw_log(options)
actual_ref = options[:ref] || root_ref
begin
sha = sha_from_ref(actual_ref)
rescue Rugged::OdbError, Rugged::InvalidError, Rugged::ReferenceError
# Return an empty array if the ref wasn't found
return []
end
sha =
unless options[:all]
actual_ref = options[:ref] || root_ref
begin
sha_from_ref(actual_ref)
rescue Rugged::OdbError, Rugged::InvalidError, Rugged::ReferenceError
# Return an empty array if the ref wasn't found
return []
end
end
log_by_shell(sha, options)
end
......@@ -503,8 +508,9 @@ def raw_log(options)
def count_commits(options)
count_commits_options = process_count_commits_options(options)
# TODO add support for options[:all] in Gitaly https://gitlab.com/gitlab-org/gitaly/issues/1050
gitaly_migrate(:count_commits) do |is_enabled|
if is_enabled
if is_enabled && !options[:all]
count_commits_by_gitaly(count_commits_options)
else
count_commits_by_shelling_out(count_commits_options)
......@@ -1701,7 +1707,12 @@ def log_by_shell(sha, options)
cmd << '--no-merges' if options[:skip_merges]
cmd << "--after=#{options[:after].iso8601}" if options[:after]
cmd << "--before=#{options[:before].iso8601}" if options[:before]
cmd << sha
if options[:all]
cmd += %w[--all --reverse]
else
cmd << sha
end
# :path can be a string or an array of strings
if options[:path].present?
......@@ -1918,7 +1929,16 @@ def count_commits_shelling_command(options)
cmd << "--before=#{options[:before].iso8601}" if options[:before]
cmd << "--max-count=#{options[:max_count]}" if options[:max_count]
cmd << "--left-right" if options[:left_right]
cmd += %W[--count #{options[:ref]}]
cmd << '--count'
cmd << if options[:all]
'--all'
elsif options[:ref]
options[:ref]
else
raise ArgumentError, "Please specify a valid ref or set the 'all' attribute to true"
end
cmd += %W[-- #{options[:path]}] if options[:path].present?
cmd
end
......
......@@ -895,7 +895,7 @@ def submodule_url(path)
repository.log(options.merge(path: "encoding"))
end
it "should not follow renames" do
it "does not follow renames" do
expect(log_commits).to include(commit_with_new_name)
expect(log_commits).to include(rename_commit)
expect(log_commits).not_to include(commit_with_old_name)
......@@ -907,7 +907,7 @@ def submodule_url(path)
repository.log(options.merge(path: "encoding/CHANGELOG"))
end
it "should not follow renames" do
it "does not follow renames" do
expect(log_commits).to include(commit_with_new_name)
expect(log_commits).to include(rename_commit)
expect(log_commits).not_to include(commit_with_old_name)
......@@ -919,7 +919,7 @@ def submodule_url(path)
repository.log(options.merge(path: "CHANGELOG"))
end
it "should not follow renames" do
it "does not follow renames" do
expect(log_commits).to include(commit_with_old_name)
expect(log_commits).to include(rename_commit)
expect(log_commits).not_to include(commit_with_new_name)
......@@ -931,7 +931,7 @@ def submodule_url(path)
repository.log(options.merge(ref: "refs/heads/fix-blob-path", path: "files/testdir/file.txt"))
end
it "should return a list of commits" do
it "returns a list of commits" do
expect(log_commits.size).to eq(1)
end
end
......@@ -991,6 +991,16 @@ def commit_files(commit)
it { expect { repository.log(limit: limit) }.to raise_error(ArgumentError) }
end
end
context 'with all' do
let(:options) { { all: true, limit: 50 } }
it 'returns a list of commits' do
commits = repository.log(options)
expect(commits.size).to eq(37)
end
end
end
describe "#rugged_commits_between" do
......@@ -1134,6 +1144,20 @@ def commit_files(commit)
context 'when Gitaly count_commits feature is disabled', :skip_gitaly_mock do
it_behaves_like 'extended commit counting'
context "with all" do
it "returns the number of commits in the whole repository" do
options = { all: true }
expect(repository.count_commits(options)).to eq(34)
end
end
context 'without all or ref being specified' do
it "raises an ArgumentError" do
expect { repository.count_commits({}) }.to raise_error(ArgumentError, "Please specify a valid ref or set the 'all' attribute to true")
end
end
end
end
......
......@@ -242,23 +242,51 @@ def expect_to_raise_storage_error
end
describe '#commits' do
it 'sets follow when path is a single path' do
expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: true)).and_call_original.twice
repository.commits('master', limit: 1, path: 'README.md')
repository.commits('master', limit: 1, path: ['README.md'])
context 'when neither the all flag nor a ref are specified' do
it 'returns every commit from default branch' do
expect(repository.commits(limit: 60).size).to eq(37)
end
end
it 'does not set follow when path is multiple paths' do
expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: false)).and_call_original
context 'when ref is passed' do
it 'returns every commit from the specified ref' do
expect(repository.commits('master', limit: 60).size).to eq(37)
end
repository.commits('master', limit: 1, path: ['README.md', 'CHANGELOG'])
end
context 'when all' do
it 'returns every commit from the repository' do
expect(repository.commits('master', limit: 60, all: true).size).to eq(60)
end
end
context 'with path' do
it 'sets follow when it is a single path' do
expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: true)).and_call_original.twice
repository.commits('master', limit: 1, path: 'README.md')
repository.commits('master', limit: 1, path: ['README.md'])
end
it 'does not set follow when there are no paths' do
expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: false)).and_call_original
it 'does not set follow when it is multiple paths' do
expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: false)).and_call_original
repository.commits('master', limit: 1)
repository.commits('master', limit: 1, path: ['README.md', 'CHANGELOG'])
end
end
context 'without path' do
it 'does not set follow' do
expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: false)).and_call_original
repository.commits('master', limit: 1)
end
end
end
context "when 'all' flag is set" do
it 'returns every commit from the repository' do
expect(repository.commits(all: true, limit: 60).size).to eq(60)
end
end
end
......
......@@ -149,6 +149,18 @@
end
end
context 'all optional parameter' do
it 'returns all project commits' do
commit_count = project.repository.count_commits(all: true)
get api("/projects/#{project_id}/repository/commits?all=true", user)
expect(response).to include_pagination_headers
expect(response.headers['X-Total']).to eq(commit_count.to_s)
expect(response.headers['X-Page']).to eql('1')
end
end
context 'with pagination params' do
let(:page) { 1 }
let(:per_page) { 5 }
......
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