Merge branch 'ee-39561-seed_fu-fails-to-load-gitlab-test' into 'master'

[EE] Handle large values on `MergeRequestDiffCommit` dates

See merge request gitlab-org/gitlab-ee!3229

app/models/merge_request_diff_commit.rb

@@ -17,7 +17,9 @@ class MergeRequestDiffCommit < ActiveRecord::Base
       commit_hash.merge(
         merge_request_diff_id: merge_request_diff_id,
         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
 

lib/gitlab/database.rb

@@ -4,6 +4,10 @@ module Gitlab
     # https://www.postgresql.org/docs/9.2/static/datatype-numeric.html
     # http://dev.mysql.com/doc/refman/5.7/en/integer-types.html
     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
       ActiveRecord::Base.configurations[Rails.env]
@@ -124,6 +128,10 @@ module Gitlab
       EOF
     end
 
+    def self.sanitize_timestamp(timestamp)
+      MAX_TIMESTAMP_VALUE > timestamp ? timestamp : MAX_TIMESTAMP_VALUE.dup
+    end
+
     # pool_size - The size of the DB pool.
     # host - An optional host name to use instead of the default one.
     def self.create_connection_pool(pool_size, host = nil)

spec/lib/gitlab/database_spec.rb

@@ -304,4 +304,26 @@ describe Gitlab::Database do
       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

spec/models/merge_request_diff_commit_spec.rb

@@ -2,14 +2,93 @@ require 'rails_helper'
 
 describe MergeRequestDiffCommit do
   let(:merge_request) { create(:merge_request) }
-  subject { merge_request.commits.first }
+  let(:project) { merge_request.project }
 
   describe '#to_hash' do
+    subject { merge_request.commits.first }
+
     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: [])
 
       expect(subject.to_hash).to eq(commit_from_repo_hash)
     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