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

app/models/repository.rb

@@ -139,7 +139,7 @@ class Repository
     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 @@ class Repository
       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)

changelogs/unreleased/42434-allow-commits-endpoint-to-work-over-all-commits.yml

+---
+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

@@ -18,25 +18,28 @@ module 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)

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) }
@@ -489,13 +491,16 @@ module Gitlab
 
       # 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 @@ 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)
@@ -1701,7 +1707,12 @@ module Gitlab
         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 @@ 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 += %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

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,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
 

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 }