From cd9daf644e2b3844b9382768a3add335f942b76c Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Mon, 19 Feb 2018 14:42:00 +0000 Subject: Add specs --- ...w-commits-endpoint-to-work-over-all-commits.yml | 5 +++ doc/api/commits.md | 3 ++ lib/api/commits.rb | 2 +- lib/gitlab/git/repository.rb | 25 ++++++----- spec/lib/gitlab/git/repository_spec.rb | 32 +++++++++++-- spec/models/repository_spec.rb | 52 +++++++++++++++++----- spec/requests/api/commits_spec.rb | 12 +++++ 7 files changed, 104 insertions(+), 27 deletions(-) create mode 100644 changelogs/unreleased/42434-allow-commits-endpoint-to-work-over-all-commits.yml diff --git a/changelogs/unreleased/42434-allow-commits-endpoint-to-work-over-all-commits.yml b/changelogs/unreleased/42434-allow-commits-endpoint-to-work-over-all-commits.yml new file mode 100644 index 00000000000..c596a88ba0b --- /dev/null +++ b/changelogs/unreleased/42434-allow-commits-endpoint-to-work-over-all-commits.yml @@ -0,0 +1,5 @@ +--- +title: Allow commits endpoint to work over all commits of a repository +merge_request: 17182 +author: +type: added diff --git a/doc/api/commits.md b/doc/api/commits.md index 2c745d00887..55c673fd06a 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -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" diff --git a/lib/api/commits.rb b/lib/api/commits.rb index c4a180d3b0e..982f45425a3 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -38,7 +38,7 @@ module API all: all) commit_count = - if path || before || 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. diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index e51cdac4a04..0d749b7f5c6 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -467,7 +467,8 @@ module Gitlab follow: false, skip_merges: false, after: nil, - before: nil + before: nil, + all: false } options = default_options.merge(options) @@ -478,8 +479,9 @@ module Gitlab 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) } @@ -506,8 +508,9 @@ module Gitlab 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) @@ -1707,7 +1710,7 @@ module Gitlab if options[:all] cmd += %w[--all --reverse] - elsif sha + else cmd << sha end @@ -1926,15 +1929,17 @@ module Gitlab 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 << '--count' - if options[:all] - cmd += %w[--count --all] - elsif options[:ref].present? - cmd += %W[--count #{options[:ref]}] - end + 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 diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index d601a383a98..4b910cc42ad 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -895,7 +895,7 @@ describe Gitlab::Git::Repository, seed_helper: true do 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 @@ describe Gitlab::Git::Repository, seed_helper: true do 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 @@ describe Gitlab::Git::Repository, seed_helper: true do 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 @@ describe Gitlab::Git::Repository, seed_helper: true do 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 @@ describe Gitlab::Git::Repository, seed_helper: true do 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 @@ describe Gitlab::Git::Repository, seed_helper: true do 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 diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 0bc07dc7a85..38653e18306 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -242,23 +242,51 @@ describe Repository do 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 diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index ad3eec88952..852f67db958 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -149,6 +149,18 @@ describe API::Commits do 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 } -- cgit v1.2.1