Merge branch '35671-api-repository-commits-with-order' into 'master'

API: Ability to list commits in order (--topo-order)

Closes #35671

See merge request gitlab-org/gitlab!24702

Gemfile

@@ -455,7 +455,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 1.85.0'
+gem 'gitaly', '~> 1.86.0'
 
 gem 'grpc', '~> 1.24.0'
 

Gemfile.lock

@@ -375,7 +375,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     git (1.5.0)
-    gitaly (1.85.0)
+    gitaly (1.86.0)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab-chronic (0.10.5)
@@ -1230,7 +1230,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 1.85.0)
+  gitaly (~> 1.86.0)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
   gitlab-labkit (= 0.9.1)

app/models/repository.rb

@@ -134,15 +134,6 @@ class Repository
     end
   end
 
-  # the opts are:
-  #  - :path
-  #  - :limit
-  #  - :offset
-  #  - :skip_merges
-  #  - :after
-  #  - :before
-  #  - :all
-  #  - :first_parent
   def commits(ref = nil, opts = {})
     options = {
       repo: raw_repository,
@@ -155,7 +146,8 @@ class Repository
       after: opts[:after],
       before: opts[:before],
       all: !!opts[:all],
-      first_parent: !!opts[:first_parent]
+      first_parent: !!opts[:first_parent],
+      order: opts[:order]
     }
 
     commits = Gitlab::Git::Commit.where(options)

changelogs/unreleased/35671-api-repository-commits-with-order.yml

+---
+title: 'API: Ability to list commits in order (--topo-order)'
+merge_request: 24702
+author:
+type: added

doc/api/commits.md

@@ -18,6 +18,7 @@ GET /projects/:id/repository/commits
 | `all` | boolean | no | Retrieve every commit from the repository |
 | `with_stats` | boolean | no | Stats about each commit will be added to the response |
 | `first_parent` | boolean | no | Follow only the first parent commit upon seeing a merge commit |
+| `order` | string | no | List commits in order. Possible value: [`topo`](https://git-scm.com/docs/git-log#Documentation/git-log.txt---topo-order). |
 
 ```shell
 curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/repository/commits"

lib/api/commits.rb

@@ -38,6 +38,7 @@ module API
         optional :all, type: Boolean, desc: 'Every commit will be returned'
         optional :with_stats, type: Boolean, desc: 'Stats about each commit will be added to the response'
         optional :first_parent, type: Boolean, desc: 'Only include the first parent of merges'
+        optional :order, type: String, desc: 'List commits in order', values: %w[topo]
         use :pagination
       end
       get ':id/repository/commits' do
@@ -49,6 +50,7 @@ module API
         all = params[:all]
         with_stats = params[:with_stats]
         first_parent = params[:first_parent]
+        order = params[:order]
 
         commits = user_project.repository.commits(ref,
                                                   path: path,
@@ -57,7 +59,8 @@ module API
                                                   before: before,
                                                   after: after,
                                                   all: all,
-                                                  first_parent: first_parent)
+                                                  first_parent: first_parent,
+                                                  order: order)
 
         commit_count =
           if all || path || before || after || first_parent

lib/gitlab/git/commit.rb

@@ -130,8 +130,7 @@ module Gitlab
         #     :skip is the number of commits to skip
         #     :order is the commits order and allowed value is :none (default), :date,
         #        :topo, or any combination of them (in an array). Commit ordering types
-        #        are documented here:
-        #        http://www.rubydoc.info/github/libgit2/rugged/Rugged#SORT_NONE-constant)
+        #        are documented here: https://git-scm.com/docs/git-log#_commit_ordering
         def find_all(repo, options = {})
           wrapped_gitaly_errors do
             Gitlab::GitalyClient::CommitService.new(repo).find_all_commits(options)

lib/gitlab/gitaly_client/commit_service.rb

@@ -324,6 +324,7 @@ module Gitlab
         request.after    = GitalyClient.timestamp(options[:after]) if options[:after]
         request.before   = GitalyClient.timestamp(options[:before]) if options[:before]
         request.revision = encode_binary(options[:ref]) if options[:ref]
+        request.order = options[:order].upcase if options[:order].present?
 
         request.paths = encode_repeated(Array(options[:path])) if options[:path].present?
 

spec/lib/gitlab/gitaly_client/commit_service_spec.rb

@@ -279,4 +279,19 @@ describe Gitlab::GitalyClient::CommitService do
       expect(subject.deletions).to eq(15)
     end
   end
+
+  describe '#find_commits' do
+    it 'sends an RPC request' do
+      request = Gitaly::FindCommitsRequest.new(
+        repository: repository_message,
+        disable_walk: true,
+        order: 'TOPO'
+      )
+
+      expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:find_commits)
+        .with(request, kind_of(Hash)).and_return([])
+
+      client.find_commits(order: 'topo')
+    end
+  end
 end

spec/models/repository_spec.rb

@@ -325,6 +325,14 @@ describe Repository do
         expect(repository.commits(nil, all: true, limit: 60).size).to eq(60)
       end
     end
+
+    context "when 'order' flag is set" do
+      it 'passes order option to perform the query' do
+        expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(order: 'topo')).and_call_original
+
+        repository.commits('master', limit: 1, order: 'topo')
+      end
+    end
   end
 
   describe '#new_commits' do

spec/requests/api/commits_spec.rb

@@ -12,7 +12,6 @@ describe API::Commits do
   let(:project) { create(:project, :repository, creator: user, path: 'my.project') }
   let(:branch_with_dot) { project.repository.find_branch('ends-with.json') }
   let(:branch_with_slash) { project.repository.find_branch('improve/awesome') }
-
   let(:project_id) { project.id }
   let(:current_user) { nil }
 
@@ -241,6 +240,40 @@ describe API::Commits do
           end
         end
       end
+
+      context 'with order parameter' do
+        let(:route) { "/projects/#{project_id}/repository/commits?ref_name=0031876&per_page=6&order=#{order}" }
+
+        context 'set to topo' do
+          let(:order) { 'topo' }
+
+          # git log --graph -n 6 --pretty=format:"%h" --topo-order 0031876
+          # *   0031876
+          # |\
+          # | * 48ca272
+          # | * 335bc94
+          # * | bf6e164
+          # * | 9d526f8
+          # |/
+          # * 1039376
+          it 'returns project commits ordered by topo order' do
+            commits = project.repository.commits("0031876", limit: 6, order: 'topo')
+
+            get api(route, current_user)
+
+            expect(json_response.size).to eq(6)
+            expect(json_response.map { |entry| entry["id"] }).to eq(commits.map(&:id))
+          end
+        end
+
+        context 'set to blank' do
+          let(:order) { '' }
+
+          it_behaves_like '400 response' do
+            let(:request) { get api(route, current_user) }
+          end
+        end
+      end
     end
   end