Handle large values on MergeRequestDiffCommit dates

This commit is contained in:
Alejandro Rodríguez 2017-10-30 20:21:56 -03:00
parent bba020a56c
commit 1796936abc
4 changed files with 114 additions and 3 deletions

View file

@ -17,7 +17,9 @@ class MergeRequestDiffCommit < ActiveRecord::Base
commit_hash.merge( commit_hash.merge(
merge_request_diff_id: merge_request_diff_id, merge_request_diff_id: merge_request_diff_id,
relative_order: index, relative_order: index,
sha: sha_attribute.type_cast_for_database(sha) sha: sha_attribute.type_cast_for_database(sha),
authored_date: Gitlab::Database.sanitize_timestamp(commit_hash[:authored_date]),
committed_date: Gitlab::Database.sanitize_timestamp(commit_hash[:committed_date])
) )
end end

View file

@ -4,6 +4,10 @@ module Gitlab
# https://www.postgresql.org/docs/9.2/static/datatype-numeric.html # https://www.postgresql.org/docs/9.2/static/datatype-numeric.html
# http://dev.mysql.com/doc/refman/5.7/en/integer-types.html # http://dev.mysql.com/doc/refman/5.7/en/integer-types.html
MAX_INT_VALUE = 2147483647 MAX_INT_VALUE = 2147483647
# The max value between MySQL's TIMESTAMP and PostgreSQL's timestampz:
# https://www.postgresql.org/docs/9.1/static/datatype-datetime.html
# https://dev.mysql.com/doc/refman/5.7/en/datetime.html
MAX_TIMESTAMP_VALUE = Time.at((1 << 31) - 1).freeze
def self.config def self.config
ActiveRecord::Base.configurations[Rails.env] ActiveRecord::Base.configurations[Rails.env]
@ -120,6 +124,10 @@ module Gitlab
EOF EOF
end end
def self.sanitize_timestamp(timestamp)
MAX_TIMESTAMP_VALUE > timestamp ? timestamp : MAX_TIMESTAMP_VALUE.dup
end
# pool_size - The size of the DB pool. # pool_size - The size of the DB pool.
# host - An optional host name to use instead of the default one. # host - An optional host name to use instead of the default one.
def self.create_connection_pool(pool_size, host = nil) def self.create_connection_pool(pool_size, host = nil)

View file

@ -256,4 +256,26 @@ describe Gitlab::Database do
expect(described_class.false_value).to eq 0 expect(described_class.false_value).to eq 0
end end
end end
describe '#sanitize_timestamp' do
let(:max_timestamp) { Time.at((1 << 31) - 1) }
subject { described_class.sanitize_timestamp(timestamp) }
context 'with a timestamp smaller than MAX_TIMESTAMP_VALUE' do
let(:timestamp) { max_timestamp - 10.years }
it 'returns the given timestamp' do
expect(subject).to eq(timestamp)
end
end
context 'with a timestamp larger than MAX_TIMESTAMP_VALUE' do
let(:timestamp) { max_timestamp + 1.second }
it 'returns MAX_TIMESTAMP_VALUE' do
expect(subject).to eq(max_timestamp)
end
end
end
end end

View file

@ -2,14 +2,93 @@ require 'rails_helper'
describe MergeRequestDiffCommit do describe MergeRequestDiffCommit do
let(:merge_request) { create(:merge_request) } let(:merge_request) { create(:merge_request) }
subject { merge_request.commits.first } let(:project) { merge_request.project }
describe '#to_hash' do describe '#to_hash' do
subject { merge_request.commits.first }
it 'returns the same results as Commit#to_hash, except for parent_ids' do it 'returns the same results as Commit#to_hash, except for parent_ids' do
commit_from_repo = merge_request.project.repository.commit(subject.sha) commit_from_repo = project.repository.commit(subject.sha)
commit_from_repo_hash = commit_from_repo.to_hash.merge(parent_ids: []) commit_from_repo_hash = commit_from_repo.to_hash.merge(parent_ids: [])
expect(subject.to_hash).to eq(commit_from_repo_hash) expect(subject.to_hash).to eq(commit_from_repo_hash)
end end
end end
describe '.create_bulk' do
let(:sha_attribute) { Gitlab::Database::ShaAttribute.new }
let(:merge_request_diff_id) { merge_request.merge_request_diff.id }
let(:commits) do
[
project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e'),
project.commit('570e7b2abdd848b95f2f578043fc23bd6f6fd24d')
]
end
let(:rows) do
[
{
"message": "Add submodule from gitlab.com\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
"authored_date": "2014-02-27T10:01:38.000+01:00".to_time,
"author_name": "Dmitriy Zaporozhets",
"author_email": "dmitriy.zaporozhets@gmail.com",
"committed_date": "2014-02-27T10:01:38.000+01:00".to_time,
"committer_name": "Dmitriy Zaporozhets",
"committer_email": "dmitriy.zaporozhets@gmail.com",
"merge_request_diff_id": merge_request_diff_id,
"relative_order": 0,
"sha": sha_attribute.type_cast_for_database('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
},
{
"message": "Change some files\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
"authored_date": "2014-02-27T09:57:31.000+01:00".to_time,
"author_name": "Dmitriy Zaporozhets",
"author_email": "dmitriy.zaporozhets@gmail.com",
"committed_date": "2014-02-27T09:57:31.000+01:00".to_time,
"committer_name": "Dmitriy Zaporozhets",
"committer_email": "dmitriy.zaporozhets@gmail.com",
"merge_request_diff_id": merge_request_diff_id,
"relative_order": 1,
"sha": sha_attribute.type_cast_for_database('570e7b2abdd848b95f2f578043fc23bd6f6fd24d')
}
]
end
subject { described_class.create_bulk(merge_request_diff_id, commits) }
it 'inserts the commits into the database en masse' do
expect(Gitlab::Database).to receive(:bulk_insert)
.with(described_class.table_name, rows)
subject
end
context 'with dates larger than the DB limit' do
let(:commits) do
# This commit's date is "Sun Aug 17 07:12:55 292278994 +0000"
[project.commit('ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69')]
end
let(:timestamp) { Time.at((1 << 31) - 1) }
let(:rows) do
[{
"message": "Weird commit date\n",
"authored_date": timestamp,
"author_name": "Alejandro Rodríguez",
"author_email": "alejorro70@gmail.com",
"committed_date": timestamp,
"committer_name": "Alejandro Rodríguez",
"committer_email": "alejorro70@gmail.com",
"merge_request_diff_id": merge_request_diff_id,
"relative_order": 0,
"sha": sha_attribute.type_cast_for_database('ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69')
}]
end
it 'uses a sanitized date' do
expect(Gitlab::Database).to receive(:bulk_insert)
.with(described_class.table_name, rows)
subject
end
end
end
end end