From 0fc361c4dcf9b714fef8dc8a59e6856fd58f2425 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Tue, 28 Mar 2017 19:14:48 +0200 Subject: [PATCH 1/4] Use Gitaly for Environment#first_deployment_for --- app/models/repository.rb | 10 ++-------- lib/gitlab/git/repository.rb | 15 +++++++++++++++ lib/gitlab/gitaly_client/ref.rb | 10 ++++++++++ spec/models/environment_spec.rb | 23 +++++++++++++++++++---- 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index dc1c1fab880..1293cb1d486 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -6,6 +6,8 @@ class Repository attr_accessor :path_with_namespace, :project + delegate :ref_name_for_sha, to: :raw_repository + CommitError = Class.new(StandardError) CreateTreeError = Class.new(StandardError) @@ -700,14 +702,6 @@ class Repository end end - def ref_name_for_sha(ref_path, sha) - args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{ref_path} --contains #{sha}) - - # Not found -> ["", 0] - # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0] - Gitlab::Popen.popen(args, path_to_repo).first.split.last - end - def refs_contains_sha(ref_type, sha) args = %W(#{Gitlab.config.git.bin_path} #{ref_type} --contains #{sha}) names = Gitlab::Popen.popen(args, path_to_repo).first diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 2e4314932c8..4fe6f340ee9 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -452,6 +452,21 @@ module Gitlab Gitlab::Git::DiffCollection.new(diff_patches(from, to, options, *paths), options) end + # Returns a RefName for a given SHA + def ref_name_for_sha(ref_path, sha) + Gitlab::GitalyClient.migrate(:find_ref_name) do |is_enabled| + if is_enabled + Gitlab::GitalyClient::Ref.find_ref_name(self, sha, ref_path) + else + args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{ref_path} --contains #{sha}) + + # Not found -> ["", 0] + # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0] + Gitlab::Popen.popen(args, @path).first.split.last + end + end + end + # Returns commits collection # # Ex. diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb index bfc5fa573c7..4958d00c542 100644 --- a/lib/gitlab/gitaly_client/ref.rb +++ b/lib/gitlab/gitaly_client/ref.rb @@ -23,6 +23,16 @@ module Gitlab consume_refs_response(stub.find_all_tag_names(request), prefix: 'refs/tags/') end + def find_ref_name(commit_id, ref_prefix) + request = Gitaly::FindRefNameRequest.new( + repository: @repository, + commit_id: commit_id, + prefix: ref_prefix + ) + + stub.find_ref_name(request).name + end + private def consume_refs_response(response, prefix:) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 9f0e7fbbe26..3ad3a25d873 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -100,12 +100,27 @@ describe Environment, models: true do let(:head_commit) { project.commit } let(:commit) { project.commit.parent } - it 'returns deployment id for the environment' do - expect(environment.first_deployment_for(commit)).to eq deployment1 + context 'Gitaly find_ref_name feature disables' do + it 'returns deployment id for the environment' do + expect(environment.first_deployment_for(commit)).to eq deployment1 + end + + it 'return nil when no deployment is found' do + expect(environment.first_deployment_for(head_commit)).to eq nil + end end - it 'return nil when no deployment is found' do - expect(environment.first_deployment_for(head_commit)).to eq nil + context 'Gitaly find_ref_name feature enabled' do + before do + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:find_ref_name).and_return(true) + end + + it 'checks that GitalyClient is called' do + expect(Gitlab::GitalyClient::Ref).to receive(:find_ref_name).with(project.repository.raw_repository, commit.id, environment.ref_path) + + environment.first_deployment_for(commit) + end + end end From ae7ec00492775c75b56725169bcc754c015b51b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Tue, 4 Apr 2017 21:43:55 +0200 Subject: [PATCH 2/4] rubocop --- spec/models/environment_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 3ad3a25d873..6d99ebcd68f 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -120,7 +120,6 @@ describe Environment, models: true do environment.first_deployment_for(commit) end - end end From 47907a417214e3d5c1ce350307795241f0cada38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Wed, 5 Apr 2017 22:56:40 +0200 Subject: [PATCH 3/4] Cleanup --- spec/models/environment_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 6d99ebcd68f..c8f4e9ff50e 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -100,7 +100,7 @@ describe Environment, models: true do let(:head_commit) { project.commit } let(:commit) { project.commit.parent } - context 'Gitaly find_ref_name feature disables' do + context 'Gitaly find_ref_name feature disabled' do it 'returns deployment id for the environment' do expect(environment.first_deployment_for(commit)).to eq deployment1 end @@ -115,7 +115,7 @@ describe Environment, models: true do allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:find_ref_name).and_return(true) end - it 'checks that GitalyClient is called' do + it 'calls GitalyClient' do expect(Gitlab::GitalyClient::Ref).to receive(:find_ref_name).with(project.repository.raw_repository, commit.id, environment.ref_path) environment.first_deployment_for(commit) From 70982bb420ad69e354db9b6999feed4e4fab9dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Thu, 6 Apr 2017 12:10:03 +0200 Subject: [PATCH 4/4] Hopefully this works --- lib/gitlab/git/repository.rb | 2 +- spec/models/environment_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 4fe6f340ee9..9e338282e96 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -456,7 +456,7 @@ module Gitlab def ref_name_for_sha(ref_path, sha) Gitlab::GitalyClient.migrate(:find_ref_name) do |is_enabled| if is_enabled - Gitlab::GitalyClient::Ref.find_ref_name(self, sha, ref_path) + gitaly_ref_client.find_ref_name(sha, ref_path) else args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{ref_path} --contains #{sha}) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index c8f4e9ff50e..af7753caba6 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -116,7 +116,7 @@ describe Environment, models: true do end it 'calls GitalyClient' do - expect(Gitlab::GitalyClient::Ref).to receive(:find_ref_name).with(project.repository.raw_repository, commit.id, environment.ref_path) + expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:find_ref_name) environment.first_deployment_for(commit) end