Merge branch 'add-first-parent-to-find-commits' into 'master'

Add first_parent to find_commits

See merge request gitlab-org/gitlab!16822

Gemfile

@@ -446,7 +446,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 1.58.0'
+gem 'gitaly', '~> 1.65.0'
 
 gem 'grpc', '~> 1.19.0'
 

Gemfile.lock

@@ -358,7 +358,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     git (1.5.0)
-    gitaly (1.58.0)
+    gitaly (1.65.0)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab-labkit (0.5.2)
@@ -1168,7 +1168,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 1.58.0)
+  gitaly (~> 1.65.0)
   github-markup (~> 1.7.0)
   gitlab-labkit (~> 0.5)
   gitlab-license (~> 1.0)

app/models/repository.rb

@@ -133,18 +133,28 @@ class Repository
     end
   end
 
-  def commits(ref = nil, path: nil, limit: nil, offset: nil, skip_merges: false, after: nil, before: nil, all: nil)
+  # the opts are:
+  #  - :path
+  #  - :limit
+  #  - :offset
+  #  - :skip_merges
+  #  - :after
+  #  - :before
+  #  - :all
+  #  - :first_parent
+  def commits(ref = nil, opts = {})
     options = {
       repo: raw_repository,
       ref: ref,
-      path: path,
-      limit: limit,
-      offset: offset,
-      after: after,
-      before: before,
-      follow: Array(path).length == 1,
-      skip_merges: skip_merges,
-      all: all
+      path: opts[:path],
+      follow: Array(opts[:path]).length == 1,
+      limit: opts[:limit],
+      offset: opts[:offset],
+      skip_merges: !!opts[:skip_merges],
+      after: opts[:after],
+      before: opts[:before],
+      all: !!opts[:all],
+      first_parent: !!opts[:first_parent]
     }
 
     commits = Gitlab::Git::Commit.where(options)

changelogs/unreleased/add-first-parent-to-find-commits.yml

+---
+title: Add first_parent option to list commits api
+merge_request: 32410
+author: jhenkens
+type: added

doc/api/commits.md

@@ -11,12 +11,13 @@ GET /projects/:id/repository/commits
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
 | `id`      | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
-| `ref_name` | string | no | The name of a repository branch or tag or if not given the default branch |
+| `ref_name` | string | no | The name of a repository branch, tag or revision range, 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 |
 | `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 |
 
 ```bash
 curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/repository/commits"

lib/api/commits.rb

@@ -37,6 +37,7 @@ module API
         optional :path, type: String, desc: 'The file path'
         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'
         use :pagination
       end
       get ':id/repository/commits' do
@@ -47,6 +48,7 @@ module API
         offset = (params[:page] - 1) * params[:per_page]
         all = params[:all]
         with_stats = params[:with_stats]
+        first_parent = params[:first_parent]
 
         commits = user_project.repository.commits(ref,
                                                   path: path,
@@ -54,11 +56,12 @@ module API
                                                   offset: offset,
                                                   before: before,
                                                   after: after,
-                                                  all: all)
+                                                  all: all,
+                                                  first_parent: first_parent)
 
         commit_count =
-          if all || path || before || after
-            user_project.repository.count_commits(ref: ref, path: path, before: before, after: after, all: all)
+          if all || path || before || after || first_parent
+            user_project.repository.count_commits(ref: ref, path: path, before: before, after: after, all: all, first_parent: first_parent)
           else
             # Cacheable commit count.
             user_project.repository.commit_count_for_ref(ref)

lib/gitlab/gitaly_client/commit_service.rb

@@ -140,7 +140,8 @@ module Gitlab
         request = Gitaly::CountCommitsRequest.new(
           repository: @gitaly_repo,
           revision: encode_binary(ref),
-          all: !!options[:all]
+          all: !!options[:all],
+          first_parent: !!options[:first_parent]
         )
         request.after = Google::Protobuf::Timestamp.new(seconds: options[:after].to_i) if options[:after].present?
         request.before = Google::Protobuf::Timestamp.new(seconds: options[:before].to_i) if options[:before].present?
@@ -325,6 +326,7 @@ module Gitlab
           follow:       options[:follow],
           skip_merges:  options[:skip_merges],
           all:          !!options[:all],
+          first_parent: !!options[:first_parent],
           disable_walk: true # This option is deprecated. The 'walk' implementation is being removed.
         )
         request.after    = GitalyClient.timestamp(options[:after]) if options[:after]

spec/models/repository_spec.rb

@@ -279,7 +279,7 @@ describe Repository do
   describe '#commits' 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)
+        expect(repository.commits(nil, limit: 60).size).to eq(37)
       end
     end
 
@@ -320,7 +320,7 @@ describe Repository do
 
     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)
+        expect(repository.commits(nil, all: true, limit: 60).size).to eq(60)
       end
     end
   end

spec/requests/api/commits_spec.rb

@@ -169,6 +169,18 @@ describe API::Commits do
         end
       end
 
+      context 'first_parent optional parameter' do
+        it 'returns all first_parent commits' do
+          commit_count = project.repository.count_commits(ref: SeedRepo::Commit::ID, first_parent: true)
+
+          get api("/projects/#{project_id}/repository/commits", user), params: { ref_name: SeedRepo::Commit::ID, first_parent: 'true' }
+
+          expect(response).to include_pagination_headers
+          expect(commit_count).to eq(12)
+          expect(response.headers['X-Total']).to eq(commit_count.to_s)
+        end
+      end
+
       context 'with_stats optional parameter' do
         let(:project) { create(:project, :public, :repository) }