From f49ad8351b59b478675858c0dcd1e28c24d16041 Mon Sep 17 00:00:00 2001
From: karthik nayak <knayak@gitlab.com>
Date: Thu, 8 Sep 2022 15:54:26 +0000
Subject: [PATCH] gitaly: Add flag for simplification of
'find_local_branches_response'
Currently 'find_local_branches_response' returns a specific response
which holds branch information. Gitaly now can return a more generic
response when the flag 'simplify_find_local_branches_response' is
enabled.
Let's add this flag to gitlab too, so we can switch to the new more
generic response type.
---
GITALY_SERVER_VERSION | 2 +-
..._simplify_find_local_branches_response.yml | 8 ++
lib/gitlab/gitaly_client/ref_service.rb | 25 +++--
.../gitlab/gitaly_client/ref_service_spec.rb | 91 ++++++++++++++-----
4 files changed, 96 insertions(+), 30 deletions(-)
create mode 100644 config/feature_flags/undefined/gitaly_simplify_find_local_branches_response.yml
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 10fd06d1b0d8b..392a6981cb72a 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-325ef97d3218f1680399bcc78eb86034d6209a29
+c3718f5f9a3285ad8d99b4e3383edc13b1546fda
diff --git a/config/feature_flags/undefined/gitaly_simplify_find_local_branches_response.yml b/config/feature_flags/undefined/gitaly_simplify_find_local_branches_response.yml
new file mode 100644
index 0000000000000..c82f8ee26b793
--- /dev/null
+++ b/config/feature_flags/undefined/gitaly_simplify_find_local_branches_response.yml
@@ -0,0 +1,8 @@
+---
+name: gitaly_simplify_find_local_branches_response
+introduced_by_url: https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4850
+rollout_issue_url: https://gitlab.com/gitlab-org/gitaly/-/issues/4452
+milestone: '15.4'
+type: undefined
+group: group::gitaly
+default_enabled: false
diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb
index 0a1a61363f146..7bc4be66a18a9 100644
--- a/lib/gitlab/gitaly_client/ref_service.rb
+++ b/lib/gitlab/gitaly_client/ref_service.rb
@@ -269,14 +269,23 @@ def sort_tags_by_param(sort_by)
end
def consume_find_local_branches_response(response)
- response.flat_map do |message|
- message.branches.map do |gitaly_branch|
- Gitlab::Git::Branch.new(
- @repository,
- gitaly_branch.name.dup,
- gitaly_branch.commit_id,
- commit_from_local_branches_response(gitaly_branch)
- )
+ if Feature.enabled?(:gitaly_simplify_find_local_branches_response, type: :undefined)
+ response.flat_map do |message|
+ message.local_branches.map do |branch|
+ target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit)
+ Gitlab::Git::Branch.new(@repository, branch.name, branch.target_commit.id, target_commit)
+ end
+ end
+ else
+ response.flat_map do |message|
+ message.branches.map do |gitaly_branch|
+ Gitlab::Git::Branch.new(
+ @repository,
+ gitaly_branch.name.dup,
+ gitaly_branch.commit_id,
+ commit_from_local_branches_response(gitaly_branch)
+ )
+ end
end
end
end
diff --git a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb
index 277276bb1d3b9..bca549b957456 100644
--- a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb
@@ -156,35 +156,84 @@
end
describe '#local_branches' do
- it 'sends a find_local_branches message' do
- expect_any_instance_of(Gitaly::RefService::Stub)
- .to receive(:find_local_branches)
- .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
- .and_return([])
+ let(:remote_name) { 'my_remote' }
- client.local_branches
- end
+ shared_examples 'common examples' do
+ it 'sends a find_local_branches message' do
+ target_commits = create_list(:gitaly_commit, 4)
+ branches = target_commits.each_with_index.map do |gitaly_commit, i|
+ Gitaly::FindLocalBranchResponse.new(
+ name: "#{remote_name}/#{i}",
+ commit: gitaly_commit,
+ commit_author: Gitaly::FindLocalBranchCommitAuthor.new(
+ name: gitaly_commit.author.name,
+ email: gitaly_commit.author.email,
+ date: gitaly_commit.author.date,
+ timezone: gitaly_commit.author.timezone
+ ),
+ commit_committer: Gitaly::FindLocalBranchCommitAuthor.new(
+ name: gitaly_commit.committer.name,
+ email: gitaly_commit.committer.email,
+ date: gitaly_commit.committer.date,
+ timezone: gitaly_commit.committer.timezone
+ )
+ )
+ end
+ local_branches = target_commits.each_with_index.map do |gitaly_commit, i|
+ Gitaly::Branch.new(name: "#{remote_name}/#{i}", target_commit: gitaly_commit)
+ end
+ response = [
+ Gitaly::FindLocalBranchesResponse.new(branches: branches[0, 2], local_branches: local_branches[0, 2]),
+ Gitaly::FindLocalBranchesResponse.new(branches: branches[2, 2], local_branches: local_branches[2, 2])
+ ]
- it 'parses and sends the sort parameter' do
- expect_any_instance_of(Gitaly::RefService::Stub)
- .to receive(:find_local_branches)
- .with(gitaly_request_with_params(sort_by: :UPDATED_DESC), kind_of(Hash))
- .and_return([])
+ expect_any_instance_of(Gitaly::RefService::Stub)
+ .to receive(:find_local_branches)
+ .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
+ .and_return(response)
+
+ subject = client.local_branches
- client.local_branches(sort_by: 'updated_desc')
+ expect(subject.length).to be(target_commits.length)
+ end
+
+ it 'parses and sends the sort parameter' do
+ expect_any_instance_of(Gitaly::RefService::Stub)
+ .to receive(:find_local_branches)
+ .with(gitaly_request_with_params(sort_by: :UPDATED_DESC), kind_of(Hash))
+ .and_return([])
+
+ client.local_branches(sort_by: 'updated_desc')
+ end
+
+ it 'translates known mismatches on sort param values' do
+ expect_any_instance_of(Gitaly::RefService::Stub)
+ .to receive(:find_local_branches)
+ .with(gitaly_request_with_params(sort_by: :NAME), kind_of(Hash))
+ .and_return([])
+
+ client.local_branches(sort_by: 'name_asc')
+ end
+
+ it 'raises an argument error if an invalid sort_by parameter is passed' do
+ expect { client.local_branches(sort_by: 'invalid_sort') }.to raise_error(ArgumentError)
+ end
end
- it 'translates known mismatches on sort param values' do
- expect_any_instance_of(Gitaly::RefService::Stub)
- .to receive(:find_local_branches)
- .with(gitaly_request_with_params(sort_by: :NAME), kind_of(Hash))
- .and_return([])
+ context 'when feature flag :gitaly_simplify_find_local_branches_response is enabled' do
+ before do
+ stub_feature_flags(gitaly_simplify_find_local_branches_response: true)
+ end
- client.local_branches(sort_by: 'name_asc')
+ it_behaves_like 'common examples'
end
- it 'raises an argument error if an invalid sort_by parameter is passed' do
- expect { client.local_branches(sort_by: 'invalid_sort') }.to raise_error(ArgumentError)
+ context 'when feature flag :gitaly_simplify_find_local_branches_response is disabled' do
+ before do
+ stub_feature_flags(gitaly_simplify_find_local_branches_response: false)
+ end
+
+ it_behaves_like 'common examples'
end
end
--
GitLab