Merge branch 'gitlab-git-find-commits' into 'master'

Clean up Gitlab::Git::Commit.find_all

See merge request !12476

5 files changed, 98 insertions, 130 deletions

diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb
index 59737bb6085..2bc00a082df 100644
--- a/app/models/network/graph.rb
+++ b/app/models/network/graph.rb
@@ -113,7 +113,7 @@ module Network
 
       opts[:ref] = @commit.id if @filter_ref
 
-      @repo.find_commits(opts)
+      Gitlab::Git::Commit.find_all(@repo.raw_repository, opts)
     end
 
     def commits_sort_by_ref
diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb
index b68378f5c0b..9c0606d780a 100644
--- a/lib/gitlab/git/commit.rb
+++ b/lib/gitlab/git/commit.rb
@@ -104,9 +104,63 @@ module Gitlab
           []
         end
 
-        # Delegate Repository#find_commits
+        # Returns commits collection
+        #
+        # Ex.
+        #   Commit.find_all(
+        #     repo,
+        #     ref: 'master',
+        #     max_count: 10,
+        #     skip: 5,
+        #     order: :date
+        #   )
+        #
+        #   +options+ is a Hash of optional arguments to git
+        #     :ref is the ref from which to begin (SHA1 or name)
+        #     :max_count is the maximum number of commits to fetch
+        #     :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)
+        #
         def find_all(repo, options = {})
-          repo.find_commits(options)
+          actual_options = options.dup
+
+          allowed_options = [:ref, :max_count, :skip, :order]
+
+          actual_options.keep_if do |key|
+            allowed_options.include?(key)
+          end
+
+          default_options = { skip: 0 }
+          actual_options = default_options.merge(actual_options)
+
+          rugged = repo.rugged
+          walker = Rugged::Walker.new(rugged)
+
+          if actual_options[:ref]
+            walker.push(rugged.rev_parse_oid(actual_options[:ref]))
+          else
+            rugged.references.each("refs/heads/*") do |ref|
+              walker.push(ref.target_id)
+            end
+          end
+
+          walker.sorting(rugged_sort_type(actual_options[:order]))
+
+          commits = []
+          offset = actual_options[:skip]
+          limit = actual_options[:max_count]
+          walker.each(offset: offset, limit: limit) do |commit|
+            commits.push(decorate(commit))
+          end
+
+          walker.reset
+
+          commits
+        rescue Rugged::OdbError
+          []
         end
 
         def decorate(commit, ref = nil)
@@ -131,6 +185,20 @@ module Gitlab
           diff.find_similar!(break_rewrites: break_rewrites)
           diff
         end
+
+        # Returns the `Rugged` sorting type constant for one or more given
+        # sort types. Valid keys are `:none`, `:topo`, and `:date`, or an array
+        # containing more than one of them. `:date` uses a combination of date and
+        # topological sorting to closer mimic git's native ordering.
+        def rugged_sort_type(sort_type)
+          @rugged_sort_types ||= {
+            none: Rugged::SORT_NONE,
+            topo: Rugged::SORT_TOPO,
+            date: Rugged::SORT_DATE | Rugged::SORT_TOPO
+          }
+
+          @rugged_sort_types.fetch(sort_type, Rugged::SORT_NONE)
+        end
       end
 
       def initialize(raw_commit, head = nil)
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index c1f942f931a..0a0c6f76cd3 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -494,70 +494,6 @@ module Gitlab
         end
       end
 
-      # Returns commits collection
-      #
-      # Ex.
-      #   repo.find_commits(
-      #     ref: 'master',
-      #     max_count: 10,
-      #     skip: 5,
-      #     order: :date
-      #   )
-      #
-      #   +options+ is a Hash of optional arguments to git
-      #     :ref is the ref from which to begin (SHA1 or name)
-      #     :contains is the commit contained by the refs from which to begin (SHA1 or name)
-      #     :max_count is the maximum number of commits to fetch
-      #     :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)
-      #
-      def find_commits(options = {})
-        actual_options = options.dup
-
-        allowed_options = [:ref, :max_count, :skip, :contains, :order]
-
-        actual_options.keep_if do |key|
-          allowed_options.include?(key)
-        end
-
-        default_options = { skip: 0 }
-        actual_options = default_options.merge(actual_options)
-
-        walker = Rugged::Walker.new(rugged)
-
-        if actual_options[:ref]
-          walker.push(rugged.rev_parse_oid(actual_options[:ref]))
-        elsif actual_options[:contains]
-          branches_contains(actual_options[:contains]).each do |branch|
-            walker.push(branch.target_id)
-          end
-        else
-          rugged.references.each("refs/heads/*") do |ref|
-            walker.push(ref.target_id)
-          end
-        end
-
-        sort_type = rugged_sort_type(actual_options[:order])
-        walker.sorting(sort_type)
-
-        commits = []
-        offset = actual_options[:skip]
-        limit = actual_options[:max_count]
-        walker.each(offset: offset, limit: limit) do |commit|
-          gitlab_commit = Gitlab::Git::Commit.decorate(commit)
-          commits.push(gitlab_commit)
-        end
-
-        walker.reset
-
-        commits
-      rescue Rugged::OdbError
-        []
-      end
-
       # Returns branch names collection that contains the special commit(SHA1
       # or name)
       #
@@ -1228,20 +1164,6 @@ module Gitlab
       rescue GRPC::BadStatus => e
         raise CommandError.new(e)
       end
-
-      # Returns the `Rugged` sorting type constant for one or more given
-      # sort types. Valid keys are `:none`, `:topo`, and `:date`, or an array
-      # containing more than one of them. `:date` uses a combination of date and
-      # topological sorting to closer mimic git's native ordering.
-      def rugged_sort_type(sort_type)
-        @rugged_sort_types ||= {
-          none: Rugged::SORT_NONE,
-          topo: Rugged::SORT_TOPO,
-          date: Rugged::SORT_DATE | Rugged::SORT_TOPO
-        }
-
-        @rugged_sort_types.fetch(sort_type, Rugged::SORT_NONE)
-      end
     end
   end
 end
diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb
index 3e44c577643..f20a14155dc 100644
--- a/spec/lib/gitlab/git/commit_spec.rb
+++ b/spec/lib/gitlab/git/commit_spec.rb
@@ -244,6 +244,33 @@ describe Gitlab::Git::Commit, seed_helper: true do
     end
 
     describe '.find_all' do
+      it 'should return a return a collection of commits' do
+        commits = described_class.find_all(repository)
+
+        expect(commits).not_to be_empty
+        expect(commits).to all( be_a_kind_of(Gitlab::Git::Commit) )
+      end
+
+      context 'while applying a sort order based on the `order` option' do
+        it "allows ordering topologically (no parents shown before their children)" do
+          expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_TOPO)
+
+          described_class.find_all(repository, order: :topo)
+        end
+
+        it "allows ordering by date" do
+          expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_DATE | Rugged::SORT_TOPO)
+
+          described_class.find_all(repository, order: :date)
+        end
+
+        it "applies no sorting by default" do
+          expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_NONE)
+
+          described_class.find_all(repository)
+        end
+      end
+
       context 'max_count' do
         subject do
           commits = Gitlab::Git::Commit.find_all(
@@ -281,26 +308,6 @@ describe Gitlab::Git::Commit, seed_helper: true do
         it { is_expected.to include(SeedRepo::FirstCommit::ID) }
         it { is_expected.not_to include(SeedRepo::LastCommit::ID) }
       end
-
-      context 'contains feature + max_count' do
-        subject do
-          commits = Gitlab::Git::Commit.find_all(
-            repository,
-            contains: 'feature',
-            max_count: 7
-          )
-
-          commits.map { |c| c.id }
-        end
-
-        it 'has 7 elements' do
-          expect(subject.size).to eq(7)
-        end
-
-        it { is_expected.not_to include(SeedRepo::Commit::PARENT_ID) }
-        it { is_expected.not_to include(SeedRepo::Commit::ID) }
-        it { is_expected.to include(SeedRepo::BigCommit::ID) }
-      end
     end
   end
 
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 703b0c2c202..4894b558e03 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1101,35 +1101,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe '#find_commits' do
-    it 'should return a return a collection of commits' do
-      commits = repository.find_commits
-
-      expect(commits).not_to be_empty
-      expect(commits).to all( be_a_kind_of(Gitlab::Git::Commit) )
-    end
-
-    context 'while applying a sort order based on the `order` option' do
-      it "allows ordering topologically (no parents shown before their children)" do
-        expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_TOPO)
-
-        repository.find_commits(order: :topo)
-      end
-
-      it "allows ordering by date" do
-        expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_DATE | Rugged::SORT_TOPO)
-
-        repository.find_commits(order: :date)
-      end
-
-      it "applies no sorting by default" do
-        expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_NONE)
-
-        repository.find_commits
-      end
-    end
-  end
-
   describe '#branches with deleted branch' do
     before(:each) do
       ref = double()