diff --git a/app/models/concerns/cached_commit.rb b/app/models/concerns/cached_commit.rb
index 8a53fec061261dee9bb058203f1a6488e49dd3f0..74120f49d01677f030a9816ce4066f3e14f56fc7 100644
--- a/app/models/concerns/cached_commit.rb
+++ b/app/models/concerns/cached_commit.rb
@@ -19,4 +19,8 @@ def parent_ids
def referenced_by
[]
end
+
+ def extended_trailers
+ {}
+ end
end
diff --git a/app/models/merge_request_context_commit.rb b/app/models/merge_request_context_commit.rb
index 281e11c7c136d2591e042bf336ad121f7c534587..921ad7e1f0afdd5cc8d1435c8f5ef70c91c4069f 100644
--- a/app/models/merge_request_context_commit.rb
+++ b/app/models/merge_request_context_commit.rb
@@ -26,6 +26,13 @@ def self.delete_bulk(merge_request, commits)
# create MergeRequestContextCommit by given commit sha and it's diff file record
def self.bulk_insert(rows, **args)
+ # Remove the new extended_trailers attribute as this shouldn't be
+ # inserted into the database. This will be removed once the old
+ # format of the trailers attribute is deprecated.
+ rows = rows.map do |row|
+ row.except(:extended_trailers).to_hash
+ end
+
ApplicationRecord.legacy_bulk_insert('merge_request_context_commits', rows, **args) # rubocop:disable Gitlab/BulkInsert
end
diff --git a/app/models/merge_request_diff_commit.rb b/app/models/merge_request_diff_commit.rb
index 790520c412356935cb7e15b02af461a8543760d8..d0d9f1733463aec33f0da274fdb540f088a49b8a 100644
--- a/app/models/merge_request_diff_commit.rb
+++ b/app/models/merge_request_diff_commit.rb
@@ -53,8 +53,13 @@ def self.create_bulk(merge_request_diff_id, commits)
# These fields are only used to determine the author/committer IDs, we
# don't store them in the DB.
+ #
+ # Trailers are stored in the DB here in order to allow changelog parsing.
+ # Rather than add an additional column for :extended_trailers, we're instead
+ # ignoring it for now until we deprecate the :trailers field and replace it with
+ # the new functionality.
commit_hash = commit_hash
- .except(:author_name, :author_email, :committer_name, :committer_email)
+ .except(:author_name, :author_email, :committer_name, :committer_email, :extended_trailers)
commit_hash.merge(
commit_author_id: author.id,
diff --git a/lib/api/entities/commit.rb b/lib/api/entities/commit.rb
index ab1f51289d7835f2c952e362c98b71e1db9dc94a..99ae4b66f674e63b3d36d07c04e2f0c0e29c240e 100644
--- a/lib/api/entities/commit.rb
+++ b/lib/api/entities/commit.rb
@@ -17,6 +17,10 @@ class Commit < Grape::Entity
expose :committer_email, documentation: { type: 'string', example: 'jack@example.com' }
expose :committed_date, documentation: { type: 'dateTime', example: '2012-05-28T04:42:42-07:00' }
expose :trailers, documentation: { type: 'object', example: '{ "Merged-By": "Jane Doe janedoe@gitlab.com" }' }
+ expose :extended_trailers, documentation: {
+ type: 'object',
+ example: '{ "Signed-off-by": ["John Doe <johndoe@gitlab.com>", "Jane Doe <janedoe@gitlab.com>"] }'
+ }
expose :web_url,
documentation: {
diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb
index 1086ea45a7a0f3f8102e0bebf60ee35b995499ca..d899ed3ba25f4ecd45ed118d8cff029cbcc4ce1d 100644
--- a/lib/gitlab/git/commit.rb
+++ b/lib/gitlab/git/commit.rb
@@ -28,7 +28,8 @@ class Commit
SERIALIZE_KEYS = [
:id, :message, :parent_ids,
:authored_date, :author_name, :author_email,
- :committed_date, :committer_name, :committer_email, :trailers, :referenced_by
+ :committed_date, :committer_name, :committer_email,
+ :trailers, :extended_trailers, :referenced_by
].freeze
attr_accessor(*SERIALIZE_KEYS)
@@ -432,9 +433,17 @@ def init_from_gitaly(commit)
@committer_email = commit.committer.email.dup
@parent_ids = Array(commit.parent_ids)
@trailers = commit.trailers.to_h { |t| [t.key, t.value] }
+ @extended_trailers = parse_commit_trailers(commit.trailers)
@referenced_by = Array(commit.referenced_by)
end
+ # Turn the commit trailers into a hash of key: [value, value] arrays
+ def parse_commit_trailers(trailers)
+ trailers.each_with_object({}) do |trailer, hash|
+ (hash[trailer.key] ||= []) << trailer.value
+ end
+ end
+
# Gitaly provides a UNIX timestamp in author.date.seconds, and a timezone
# offset in author.timezone. If the latter isn't present, assume UTC.
def init_date_from_gitaly(author)
diff --git a/spec/factories/gitaly/commit.rb b/spec/factories/gitaly/commit.rb
index 4e8220e449a58df604cca5524a6d65e6cc6c5148..ecf3e4e065e5e9bc0bf74c494a29b0ba78458150 100644
--- a/spec/factories/gitaly/commit.rb
+++ b/spec/factories/gitaly/commit.rb
@@ -16,5 +16,15 @@
body { subject + "\nMy body" }
author { association(:gitaly_commit_author) }
committer { association(:gitaly_commit_author) }
+
+ trailers do
+ trailers = body.lines.keep_if { |l| l =~ /.*: / }.map do |l|
+ key, value = *l.split(":").map(&:strip)
+
+ Gitaly::CommitTrailer.new(key: key, value: value)
+ end
+
+ Google::Protobuf::RepeatedField.new(:message, Gitaly::CommitTrailer, trailers)
+ end
end
end
diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb
index d8d62ac9670c8572fef54f21a63d1c01ab40b99f..6c8634281ae223c58ed641496c3accf9708de207 100644
--- a/spec/lib/gitlab/git/commit_spec.rb
+++ b/spec/lib/gitlab/git/commit_spec.rb
@@ -8,7 +8,6 @@
describe "Commit info from gitaly commit" do
let(:subject) { (+"My commit").force_encoding('ASCII-8BIT') }
- let(:body) { subject + (+"My body").force_encoding('ASCII-8BIT') }
let(:body_size) { body.length }
let(:gitaly_commit) { build(:gitaly_commit, subject: subject, body: body, body_size: body_size, tree_id: tree_id) }
let(:id) { gitaly_commit.id }
@@ -17,6 +16,17 @@
let(:author) { gitaly_commit.author }
let(:commit) { described_class.new(repository, gitaly_commit) }
+ let(:body) do
+ body = +<<~BODY
+ Bleep bloop.
+
+ Cc: John Doe <johndoe@gitlab.com>
+ Cc: Jane Doe <janedoe@gitlab.com>
+ BODY
+
+ [subject, "\n", body].join.force_encoding("ASCII-8BIT")
+ end
+
it { expect(commit.short_id).to eq(id[0..10]) }
it { expect(commit.id).to eq(id) }
it { expect(commit.sha).to eq(id) }
@@ -29,6 +39,18 @@
it { expect(commit.parent_ids).to eq(gitaly_commit.parent_ids) }
it { expect(commit.tree_id).to eq(tree_id) }
+ it "parses the commit trailers" do
+ expect(commit.trailers).to eq(
+ { "Cc" => "Jane Doe <janedoe@gitlab.com>" }
+ )
+ end
+
+ it "parses the extended commit trailers" do
+ expect(commit.extended_trailers).to eq(
+ { "Cc" => ["John Doe <johndoe@gitlab.com>", "Jane Doe <janedoe@gitlab.com>"] }
+ )
+ end
+
context 'non-UTC dates' do
let(:seconds) { Time.now.to_i }
@@ -773,6 +795,7 @@ def sample_commit_hash
message: "tree css fixes",
parent_ids: ["874797c3a73b60d2187ed6e2fcabd289ff75171e"],
trailers: {},
+ extended_trailers: {},
referenced_by: []
}
end
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 6a11291828822a448466476136c72ab1cb3a71a5..4ec5d195ff8b2ceea9a0a402b8c9bdf46b16a024 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -44,6 +44,30 @@
expect(response).to include_limited_pagination_headers
end
+
+ describe "commit trailers" do
+ it "doesn't include the commit trailers by default" do
+ get api(route, current_user), params: { page: 2 }
+
+ commit_with_trailers = json_response.find { |c| c["trailers"].any? }
+
+ expect(commit_with_trailers).to be_nil
+ expect(json_response.first["trailers"]).to eq({})
+ end
+
+ it "does include the commit trailers when specified in the params" do
+ # Test repo commits with trailers are further down the list, so use a
+ # higher page number.
+ get api(route, current_user), params: { page: 2, trailers: true }
+
+ commit_with_trailers = json_response.find { |c| c["trailers"].any? }
+
+ expect(commit_with_trailers["trailers"]).to be_a(Hash)
+ expect(commit_with_trailers["extended_trailers"]).to be_a(Hash)
+ expect(commit_with_trailers["trailers"].size).to be > 0
+ expect(commit_with_trailers["extended_trailers"].size).to be > 0
+ end
+ end
end
context 'when unauthenticated', 'and project is public' do
@@ -426,6 +450,10 @@
expect(commit['trailers']).to eq(
'Signed-off-by' => 'Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>'
)
+
+ expect(commit['extended_trailers']).to eq(
+ 'Signed-off-by' => ['Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>']
+ )
end
end
end