Add specs

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

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"

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.

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 << if options[:all]
-          cmd += %w[--count --all]
+                 '--all'
-        elsif options[:ref].present?
+               elsif options[:ref]
-          cmd += %W[--count #{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
 

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
 

spec/models/repository_spec.rb

@@ -242,25 +242,53 @@ describe Repository do
   end
 
   describe '#commits' do
-    it 'sets follow when path is a single path' do
+    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
+
+    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
+
+      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 path is multiple paths' do
+        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, path: ['README.md', 'CHANGELOG'])
         end
+      end
 
-    it 'does not set follow when there are no paths' do
+      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
 
   describe '#new_commits' do
     let(:new_refs) do

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 }