From d3bcf8ac2ae7e89d0ec6eddcd6374bc1e1c8b5fb Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Wed, 28 Jun 2017 14:20:29 +0200
Subject: [PATCH] Fix gitaly ref encoding bugs
---
lib/gitlab/git.rb | 4 +++-
lib/gitlab/git/repository.rb | 4 +---
lib/gitlab/gitaly_client/ref.rb | 23 ++++++++++++++++-------
spec/lib/gitlab/git/repository_spec.rb | 24 ++++++++++++++++++++++++
4 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb
index 936606152e9..4175746be39 100644
--- a/lib/gitlab/git.rb
+++ b/lib/gitlab/git.rb
@@ -7,8 +7,10 @@ module Gitlab
CommandError = Class.new(StandardError)
class << self
+ include Gitlab::EncodingHelper
+
def ref_name(ref)
- ref.sub(/\Arefs\/(tags|heads)\//, '')
+ encode! ref.sub(/\Arefs\/(tags|heads)\//, '')
end
def branch_name(ref)
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index dd296983491..23d0c8a9bdb 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -113,9 +113,7 @@ module Gitlab
def local_branches(sort_by: nil)
gitaly_migrate(:local_branches) do |is_enabled|
if is_enabled
- gitaly_ref_client.local_branches(sort_by: sort_by).map do |gitaly_branch|
- Gitlab::Git::Branch.new(self, gitaly_branch.name, gitaly_branch)
- end
+ gitaly_ref_client.local_branches(sort_by: sort_by)
else
branches(filter: :local, sort_by: sort_by)
end
diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb
index 6d5f54dd959..f4786f28a3a 100644
--- a/lib/gitlab/gitaly_client/ref.rb
+++ b/lib/gitlab/gitaly_client/ref.rb
@@ -1,8 +1,11 @@
module Gitlab
module GitalyClient
class Ref
+ include Gitlab::EncodingHelper
+
# 'repository' is a Gitlab::Git::Repository
def initialize(repository)
+ @repository = repository
@gitaly_repo = repository.gitaly_repository
@storage = repository.storage
end
@@ -16,13 +19,13 @@ module Gitlab
def branch_names
request = Gitaly::FindAllBranchNamesRequest.new(repository: @gitaly_repo)
response = GitalyClient.call(@storage, :ref, :find_all_branch_names, request)
- consume_refs_response(response, prefix: 'refs/heads/')
+ consume_refs_response(response) { |name| Gitlab::Git.branch_name(name) }
end
def tag_names
request = Gitaly::FindAllTagNamesRequest.new(repository: @gitaly_repo)
response = GitalyClient.call(@storage, :ref, :find_all_tag_names, request)
- consume_refs_response(response, prefix: 'refs/tags/')
+ consume_refs_response(response) { |name| Gitlab::Git.tag_name(name) }
end
def find_ref_name(commit_id, ref_prefix)
@@ -51,10 +54,8 @@ module Gitlab
private
- def consume_refs_response(response, prefix:)
- response.flat_map do |r|
- r.names.map { |name| name.sub(/\A#{Regexp.escape(prefix)}/, '') }
- end
+ def consume_refs_response(response)
+ response.flat_map { |message| message.names.map { |name| yield(name) } }
end
def sort_by_param(sort_by)
@@ -64,7 +65,15 @@ module Gitlab
end
def consume_branches_response(response)
- response.flat_map { |r| r.branches }
+ response.flat_map do |message|
+ message.branches.map do |gitaly_branch|
+ Gitlab::Git::Branch.new(
+ @repository,
+ encode!(gitaly_branch.name.dup),
+ gitaly_branch.commit_id
+ )
+ end
+ end
end
end
end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index ee25aeefa95..0cd458bf933 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -26,6 +26,10 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
end
+ it 'returns UTF-8' do
+ expect(repository.root_ref.encoding).to eq(Encoding.find('UTF-8'))
+ end
+
context 'with gitaly enabled' do
before do
stub_gitaly
@@ -123,6 +127,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
it 'has SeedRepo::Repo::BRANCHES.size elements' do
expect(subject.size).to eq(SeedRepo::Repo::BRANCHES.size)
end
+
+ it 'returns UTF-8' do
+ expect(subject.first.encoding).to eq(Encoding.find('UTF-8'))
+ end
+
it { is_expected.to include("master") }
it { is_expected.not_to include("branch-from-space") }
@@ -158,10 +167,15 @@ describe Gitlab::Git::Repository, seed_helper: true do
subject { repository.tag_names }
it { is_expected.to be_kind_of Array }
+
it 'has SeedRepo::Repo::TAGS.size elements' do
expect(subject.size).to eq(SeedRepo::Repo::TAGS.size)
end
+ it 'returns UTF-8' do
+ expect(subject.first.encoding).to eq(Encoding.find('UTF-8'))
+ end
+
describe '#last' do
subject { super().last }
it { is_expected.to eq("v1.2.1") }
@@ -1276,6 +1290,16 @@ describe Gitlab::Git::Repository, seed_helper: true do
Gitlab::GitalyClient.clear_stubs!
end
+ it 'returns a Branch with UTF-8 fields' do
+ branches = @repo.local_branches.to_a
+ expect(branches.size).to be > 0
+ utf_8 = Encoding.find('utf-8')
+ branches.each do |branch|
+ expect(branch.name.encoding).to eq(utf_8)
+ expect(branch.target.encoding).to eq(utf_8) unless branch.target.nil?
+ end
+ end
+
it 'gets the branches from GitalyClient' do
expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
.and_return([])
--
GitLab