summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZeger-Jan van de Weg <git@zjvandeweg.nl>2018-01-30 09:59:45 +0100
committerZeger-Jan van de Weg <git@zjvandeweg.nl>2018-02-01 13:00:33 +0100
commit0a47d1924d6b283a174672f33cf7a0de6b281fef (patch)
treec11fce108b9fed69fd1c6749eff795e83af19afe
parent498d32363aa61d679ff749be727a6591257afb6d (diff)
downloadgitlab-ce-0a47d1924d6b283a174672f33cf7a0de6b281fef.tar.gz
Client changes for Tag,BranchNamesContainingCommit
As part of gitlab-org/gitaly#884, this commit contains the client implementation for both TagNamesContaintingCommit and BranchNamesContainingCommit. The interface in the Repository model stays the same, but the implementation on the serverside, e.g. Gitaly, uses `for-each-ref`, as opposed to `branch` or `tag` which both aren't plumbing command. The result stays the same. On the serverside, we have the opportunity to limit the number of names to return. However, this is not supported on the front end yet. My proposal to use this ability: gitlab-org/gitlab-ce#42581. For now, this ability is not used as that would change more behaviours on a feature flag which might lead to unexpected changes on page refresh for example.
-rw-r--r--app/models/repository.rb4
-rw-r--r--lib/gitlab/git/branch.rb18
-rw-r--r--lib/gitlab/git/repository.rb42
-rw-r--r--lib/gitlab/git/tag.rb8
-rw-r--r--lib/gitlab/gitaly_client/ref_service.rb26
-rw-r--r--spec/models/repository_spec.rb45
6 files changed, 109 insertions, 34 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb
index edfb236a91a..7f443846a82 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -721,11 +721,11 @@ class Repository
end
def branch_names_contains(sha)
- refs_contains_sha('branch', sha)
+ raw_repository.branch_names_contains_sha(sha)
end
def tag_names_contains(sha)
- refs_contains_sha('tag', sha)
+ raw_repository.tag_names_contains_sha(sha)
end
def local_branches
diff --git a/lib/gitlab/git/branch.rb b/lib/gitlab/git/branch.rb
index 3487e099381..2c051bd33b9 100644
--- a/lib/gitlab/git/branch.rb
+++ b/lib/gitlab/git/branch.rb
@@ -1,13 +1,17 @@
-# Gitaly note: JV: no RPC's here.
-
module Gitlab
module Git
class Branch < Ref
- def self.find(repo, branch_name)
- if branch_name.is_a?(Gitlab::Git::Branch)
- branch_name
- else
- repo.find_branch(branch_name)
+ class << self
+ def find(repo, branch_name)
+ if branch_name.is_a?(Gitlab::Git::Branch)
+ branch_name
+ else
+ repo.find_branch(branch_name)
+ end
+ end
+
+ def names_contains_sha(repo, sha, limit: 0)
+ GitalyClient::RefService.new(repo).branch_names_contains_sha(sha)
end
end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 68b54d28876..76b94bf9030 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -1355,20 +1355,23 @@ module Gitlab
raise CommandError.new(e)
end
- def refs_contains_sha(ref_type, sha)
- args = %W(#{ref_type} --contains #{sha})
- names = run_git(args).first
-
- if names.respond_to?(:split)
- names = names.split("\n").map(&:strip)
-
- names.each do |name|
- name.slice! '* '
+ def branch_names_contains_sha(sha)
+ gitaly_migrate(:branch_names_contains_sha) do |is_enabled|
+ if is_enabled
+ Gitlab::Git::Branch.names_contains_sha(self, sha)
+ else
+ refs_contains_sha(:branch, sha)
end
+ end
+ end
- names
- else
- []
+ def tag_names_contains_sha(sha)
+ gitaly_migrate(:tag_names_contains_sha) do |is_enabled|
+ if is_enabled
+ Gitlab::Git::Tag.names_contains_sha(self, sha)
+ else
+ refs_contains_sha(:tag, sha)
+ end
end
end
@@ -1446,6 +1449,21 @@ module Gitlab
end
end
+ def refs_contains_sha(ref_type, sha)
+ args = %W(#{ref_type} --contains #{sha})
+ names = run_git(args).first
+
+ return [] unless names.respond_to?(:split)
+
+ names = names.split("\n").map(&:strip)
+
+ names.each do |name|
+ name.slice! '* '
+ end
+
+ names
+ end
+
def shell_write_ref(ref_path, ref, old_ref)
raise ArgumentError, "invalid ref_path #{ref_path.inspect}" if ref_path.include?(' ')
raise ArgumentError, "invalid ref #{ref.inspect}" if ref.include?("\x00")
diff --git a/lib/gitlab/git/tag.rb b/lib/gitlab/git/tag.rb
index bc4e160dce9..c163d383eb0 100644
--- a/lib/gitlab/git/tag.rb
+++ b/lib/gitlab/git/tag.rb
@@ -1,8 +1,12 @@
-# Gitaly note: JV: no RPC's here.
-#
module Gitlab
module Git
class Tag < Ref
+ class << self
+ def names_contains_sha(repo, sha)
+ GitalyClient::RefService.new(repo).branch_names_contains_sha(sha)
+ end
+ end
+
attr_reader :object_sha
def initialize(repository, name, target, target_commit, message = nil)
diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb
index 8b9a224b700..07122da4c2c 100644
--- a/lib/gitlab/gitaly_client/ref_service.rb
+++ b/lib/gitlab/gitaly_client/ref_service.rb
@@ -145,6 +145,32 @@ module Gitlab
raise Gitlab::Git::Repository::GitError, response.git_error if response.git_error.present?
end
+ # Limit: 0 implies no limit, thus all tag names will be returned
+ def tag_names_containing(sha, limit: 0)
+ request = Gitaly::ListTagNamesContainingCommitRequest.new(
+ repository: @gitaly_repo,
+ commit_id: sha,
+ limit: limit
+ )
+
+ stream = GitalyClient.call(@repository.storage, :ref_service, :list_tag_names_containing_commit, request)
+
+ stream.each_with_object([]) { |response, array| array.concat(response.tag_names) }
+ end
+
+ # Limit: 0 implies no limit, thus all tag names will be returned
+ def branch_names_contains_sha(sha, limit: 0)
+ request = Gitaly::ListBranchNamesContainingCommitRequest.new(
+ repository: @gitaly_repo,
+ commit_id: sha,
+ limit: limit
+ )
+
+ stream = GitalyClient.call(@repository.storage, :ref_service, :list_branch_names_containing_commit, request)
+
+ stream.each_with_object([]) { |response, array| array.concat(response.branch_names) }
+ end
+
private
def consume_refs_response(response)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 1102b1c9006..07d43acd338 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -36,26 +36,49 @@ describe Repository do
end
describe '#branch_names_contains' do
- subject { repository.branch_names_contains(sample_commit.id) }
+ shared_examples '#branch_names_contains' do
+ set(:project) { create(:project, :repository) }
+ let(:repository) { project.repository }
- it { is_expected.to include('master') }
- it { is_expected.not_to include('feature') }
- it { is_expected.not_to include('fix') }
+ subject { repository.branch_names_contains(sample_commit.id) }
- describe 'when storage is broken', :broken_storage do
- it 'should raise a storage error' do
- expect_to_raise_storage_error do
- broken_repository.branch_names_contains(sample_commit.id)
+ it { is_expected.to include('master') }
+ it { is_expected.not_to include('feature') }
+ it { is_expected.not_to include('fix') }
+
+ describe 'when storage is broken', :broken_storage do
+ it 'should raise a storage error' do
+ expect_to_raise_storage_error do
+ broken_repository.branch_names_contains(sample_commit.id)
+ end
end
end
end
+
+ context 'when gitaly is enabled' do
+ it_behaves_like '#branch_names_contains'
+ end
+
+ context 'when gitaly is disabled', :skip_gitaly_mock do
+ it_behaves_like '#branch_names_contains'
+ end
end
describe '#tag_names_contains' do
- subject { repository.tag_names_contains(sample_commit.id) }
+ shared_examples '#tag_names_contains' do
+ subject { repository.tag_names_contains(sample_commit.id) }
+
+ it { is_expected.to include('v1.1.0') }
+ it { is_expected.not_to include('v1.0.0') }
+ end
+
+ context 'when gitaly is enabled' do
+ it_behaves_like '#tag_names_contains'
+ end
- it { is_expected.to include('v1.1.0') }
- it { is_expected.not_to include('v1.0.0') }
+ context 'when gitaly is enabled', :skip_gitaly_mock do
+ it_behaves_like '#tag_names_contains'
+ end
end
describe 'tags_sorted_by' do