From 90f8feae46ca49299dbe138a229a51c5294f88be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Fri, 23 Jun 2017 17:33:16 -0400 Subject: Adapt to new Gitaly commit message format --- lib/gitlab/git/commit.rb | 20 +++++++++++++++++++ lib/gitlab/gitaly_client/ref_service.rb | 8 ++++---- spec/lib/gitlab/git/commit_spec.rb | 35 +++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index d0f04d25db2..1b9f3c57957 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -210,6 +210,8 @@ module Gitlab init_from_hash(raw_commit) elsif raw_commit.is_a?(Rugged::Commit) init_from_rugged(raw_commit) + elsif raw_commit.is_a?(Gitaly::GitCommit) + init_from_gitaly(raw_commit) else raise "Invalid raw commit type: #{raw_commit.class}" end @@ -371,6 +373,24 @@ module Gitlab @parent_ids = commit.parents.map(&:oid) end + def init_from_gitaly(commit) + @raw_commit = commit + @id = commit.id + # NOTE: For ease of parsing in Gitaly, we have only the subject of + # the commit and not the full message. + # TODO: Once gitaly "takes over" Rugged consider separating the + # subject from the message to make it clearer when there's one + # available but not the other. + @message = commit.subject.dup + @authored_date = Time.at(commit.author.date.seconds) + @author_name = commit.author.name.dup + @author_email = commit.author.email.dup + @committed_date = Time.at(commit.committer.date.seconds) + @committer_name = commit.committer.name.dup + @committer_email = commit.committer.email.dup + @parent_ids = commit.parent_ids + end + def serialize_keys SERIALIZE_KEYS end diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index f541887843d..2c3d53410ac 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -96,11 +96,11 @@ module Gitlab id: response.commit_id, message: message, authored_date: Time.at(response.commit_author.date.seconds), - author_name: response.commit_author.name, - author_email: response.commit_author.email, + author_name: response.commit_author.name.dup, + author_email: response.commit_author.email.dup, committed_date: Time.at(response.commit_committer.date.seconds), - committer_name: response.commit_committer.name, - committer_email: response.commit_committer.email + committer_name: response.commit_committer.name.dup, + committer_email: response.commit_committer.email.dup } Gitlab::Git::Commit.decorate(hash) diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index f20a14155dc..62b353890d6 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -64,6 +64,41 @@ describe Gitlab::Git::Commit, seed_helper: true do end end + describe "Commit info from gitaly commit" do + let(:id) { 'f00' } + let(:subject) { "My commit".force_encoding('ASCII-8BIT') } + let(:committer) do + Gitaly::CommitAuthor.new( + name: generate(:name), + email: generate(:email), + date: Google::Protobuf::Timestamp.new(seconds: 123) + ) + end + let(:author) do + Gitaly::CommitAuthor.new( + name: generate(:name), + email: generate(:email), + date: Google::Protobuf::Timestamp.new(seconds: 456) + ) + end + let(:gitaly_commit) do + Gitaly::GitCommit.new( + id: id, subject: subject, author: author, committer: committer + ) + end + let(:commit) { described_class.new(gitaly_commit) } + + it { expect(commit.short_id).to eq(id[0..10]) } + it { expect(commit.id).to eq(id) } + it { expect(commit.sha).to eq(id) } + it { expect(commit.safe_message).to eq(subject) } + it { expect(commit.created_at).to eq(Time.at(committer.date.seconds)) } + it { expect(commit.author_email).to eq(author.email) } + it { expect(commit.author_name).to eq(author.name) } + it { expect(commit.committer_name).to eq(committer.name) } + it { expect(commit.committer_email).to eq(committer.email) } + end + context 'Class methods' do describe '.find' do it "should return first head commit if without params" do -- cgit v1.2.1 From 25b01b4c8594e8260dd1c86523bb9989aefd1fda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Fri, 23 Jun 2017 17:52:51 -0400 Subject: Incorporate Gitaly's Commits#between RPC --- lib/gitlab/git/commit.rb | 14 ++++++++++---- lib/gitlab/git/repository.rb | 16 ++++++++-------- lib/gitlab/gitaly_client/commit_service.rb | 15 +++++++++++++++ spec/lib/gitlab/git/commit_spec.rb | 15 +++++++++++++-- spec/lib/gitlab/gitaly_client/commit_service_spec.rb | 16 +++++++++++++++- spec/services/git_push_service_spec.rb | 4 ++-- 6 files changed, 63 insertions(+), 17 deletions(-) diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 1b9f3c57957..76a562f356e 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -98,7 +98,15 @@ module Gitlab # Commit.between(repo, '29eda46b', 'master') # def between(repo, base, head) - repo.commits_between(base, head).map do |commit| + commits = Gitlab::GitalyClient.migrate(:commits_between) do |is_enabled| + if is_enabled + repo.gitaly_commit_client.between(base, head) + else + repo.commits_between(base, head) + end + end + + commits.map do |commit| decorate(commit) end rescue Rugged::ReferenceError @@ -376,12 +384,10 @@ module Gitlab def init_from_gitaly(commit) @raw_commit = commit @id = commit.id - # NOTE: For ease of parsing in Gitaly, we have only the subject of - # the commit and not the full message. # TODO: Once gitaly "takes over" Rugged consider separating the # subject from the message to make it clearer when there's one # available but not the other. - @message = commit.subject.dup + @message = (commit.body.presence || commit.subject).dup @authored_date = Time.at(commit.author.date.seconds) @author_name = commit.author.name.dup @author_email = commit.author.email.dup diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index c091bb9dcfe..63eebadff2e 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -807,6 +807,14 @@ module Gitlab Gitlab::GitalyClient::Util.repository(@storage, @relative_path) end + def gitaly_ref_client + @gitaly_ref_client ||= Gitlab::GitalyClient::RefService.new(self) + end + + def gitaly_commit_client + @gitaly_commit_client ||= Gitlab::GitalyClient::CommitService.new(self) + end + private # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'. @@ -1105,14 +1113,6 @@ module Gitlab end end - def gitaly_ref_client - @gitaly_ref_client ||= Gitlab::GitalyClient::RefService.new(self) - end - - def gitaly_commit_client - @gitaly_commit_client ||= Gitlab::GitalyClient::CommitService.new(self) - end - def gitaly_migrate(method, &block) Gitlab::GitalyClient.migrate(method, &block) rescue GRPC::NotFound => e diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 470e3ac8779..8f5738fed06 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -65,6 +65,17 @@ module Gitlab GitalyClient.call(@repository.storage, :commit_service, :count_commits, request).count end + def between(from, to) + request = Gitaly::CommitsBetweenRequest.new( + repository: @gitaly_repo, + from: from, + to: to + ) + + response = GitalyClient.call(@repository.storage, :commit_service, :commits_between, request) + consume_commits_response(response) + end + private def commit_diff_request_params(commit, options = {}) @@ -77,6 +88,10 @@ module Gitlab paths: options.fetch(:paths, []) } end + + def consume_commits_response(response) + response.flat_map { |r| r.commits } + end end end end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 62b353890d6..60de91324f0 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -67,6 +67,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe "Commit info from gitaly commit" do let(:id) { 'f00' } let(:subject) { "My commit".force_encoding('ASCII-8BIT') } + let(:body) { subject + "My body".force_encoding('ASCII-8BIT') } let(:committer) do Gitaly::CommitAuthor.new( name: generate(:name), @@ -83,7 +84,11 @@ describe Gitlab::Git::Commit, seed_helper: true do end let(:gitaly_commit) do Gitaly::GitCommit.new( - id: id, subject: subject, author: author, committer: committer + id: id, + subject: subject, + body: body, + author: author, + committer: committer ) end let(:commit) { described_class.new(gitaly_commit) } @@ -91,12 +96,18 @@ describe Gitlab::Git::Commit, seed_helper: true do it { expect(commit.short_id).to eq(id[0..10]) } it { expect(commit.id).to eq(id) } it { expect(commit.sha).to eq(id) } - it { expect(commit.safe_message).to eq(subject) } + it { expect(commit.safe_message).to eq(body) } it { expect(commit.created_at).to eq(Time.at(committer.date.seconds)) } it { expect(commit.author_email).to eq(author.email) } it { expect(commit.author_name).to eq(author.name) } it { expect(commit.committer_name).to eq(committer.name) } it { expect(commit.committer_email).to eq(committer.email) } + + context 'no body' do + let(:body) { "".force_encoding('ASCII-8BIT') } + + it { expect(commit.safe_message).to eq(subject) } + end end context 'Class methods' do diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index fee5bb45fe5..93affb12f2b 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -1,7 +1,6 @@ require 'spec_helper' describe Gitlab::GitalyClient::CommitService do - let(:diff_stub) { double('Gitaly::DiffService::Stub') } let(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:repository_message) { repository.gitaly_repository } @@ -82,4 +81,19 @@ describe Gitlab::GitalyClient::CommitService do end end end + + describe '#between' do + let(:from) { 'master' } + let(:to) { '4b825dc642cb6eb9a060e54bf8d69288fbee4904' } + it 'sends an RPC request' do + request = Gitaly::CommitsBetweenRequest.new( + repository: repository_message, from: from, to: to + ) + + expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:commits_between) + .with(request, kind_of(Hash)).and_return([]) + + described_class.new(repository).between(from, to) + end + end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 3f77ed10069..c493c08a7ae 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -108,7 +108,7 @@ describe GitPushService, services: true do it { is_expected.to include(id: @commit.id) } it { is_expected.to include(message: @commit.safe_message) } - it { is_expected.to include(timestamp: @commit.date.xmlschema) } + it { expect(subject[:timestamp].in_time_zone).to eq(@commit.date.in_time_zone) } it do is_expected.to include( url: [ @@ -163,7 +163,7 @@ describe GitPushService, services: true do execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) end end - + context "Sends System Push data" do it "when pushing on a branch" do expect(SystemHookPushWorker).to receive(:perform_async).with(@push_data, :push_hooks) -- cgit v1.2.1