From 9a73b634ab4220f68a8296ccb582a68293874489 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Fri, 9 Jun 2017 12:48:25 +0100
Subject: Add table for files in merge request diffs

This adds an ID-less table containing one row per file, per merge request
diff. It has a column for each attribute on Gitlab::Git::Diff that is serialised
currently, with the advantage that we can easily query the attributes of this
new table.

It does not migrate existing data, so we have fallback code when the legacy
st_diffs column is present instead. For a merge request diff to be valid, it
should have at most one of:

* Rows in this new table, with the correct merge_request_diff_id.
* A non-NULL st_diffs column.

It may have neither, if the diff is empty.
---
 app/models/merge_request_diff.rb      | 54 ++++++++++++++++++++++++-----------
 app/models/merge_request_diff_file.rb | 11 +++++++
 2 files changed, 48 insertions(+), 17 deletions(-)
 create mode 100644 app/models/merge_request_diff_file.rb

(limited to 'app')

diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 99dd2130188..f1ee4d3f7a9 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -10,6 +10,7 @@ class MergeRequestDiff < ActiveRecord::Base
   VALID_CLASSES = [Hash, Rugged::Patch, Rugged::Diff::Delta].freeze
 
   belongs_to :merge_request
+  has_many :merge_request_diff_files, -> { order(:merge_request_diff_id, :relative_order) }
 
   serialize :st_commits # rubocop:disable Cop/ActiverecordSerialize
   serialize :st_diffs # rubocop:disable Cop/ActiverecordSerialize
@@ -91,7 +92,7 @@ class MergeRequestDiff < ActiveRecord::Base
           head_commit_sha).diffs(options)
     else
       @raw_diffs ||= {}
-      @raw_diffs[options] ||= load_diffs(st_diffs, options)
+      @raw_diffs[options] ||= load_diffs(options)
     end
   end
 
@@ -253,24 +254,44 @@ class MergeRequestDiff < ActiveRecord::Base
     update_columns_serialized(new_attributes)
   end
 
-  def dump_diffs(diffs)
-    if diffs.respond_to?(:map)
-      diffs.map(&:to_hash)
+  def create_merge_request_diff_files(diffs)
+    rows = diffs.map.with_index do |diff, index|
+      diff.to_hash.merge(
+        merge_request_diff_id: self.id,
+        relative_order: index
+      )
     end
+
+    Gitlab::Database.bulk_insert('merge_request_diff_files', rows)
   end
 
-  def load_diffs(raw, options)
-    if valid_raw_diff?(raw)
-      if paths = options[:paths]
-        raw = raw.select do |diff|
-          paths.include?(diff[:old_path]) || paths.include?(diff[:new_path])
-        end
-      end
+  def load_diffs(options)
+    return Gitlab::Git::DiffCollection.new([]) unless diffs_from_database
 
-      Gitlab::Git::DiffCollection.new(raw, options)
-    else
-      Gitlab::Git::DiffCollection.new([])
+    raw = diffs_from_database
+
+    if paths = options[:paths]
+      raw = raw.select do |diff|
+        paths.include?(diff[:old_path]) || paths.include?(diff[:new_path])
+      end
     end
+
+    Gitlab::Git::DiffCollection.new(raw, options)
+  end
+
+  def diffs_from_database
+    return @diffs_from_database if defined?(@diffs_from_database)
+
+    @diffs_from_database =
+      if st_diffs.present?
+        if valid_raw_diff?(st_diffs)
+          st_diffs
+        end
+      elsif merge_request_diff_files.present?
+        merge_request_diff_files
+          .as_json(only: Gitlab::Git::Diff::SERIALIZE_KEYS)
+          .map(&:with_indifferent_access)
+      end
   end
 
   # Load diffs between branches related to current merge request diff from repo
@@ -285,11 +306,10 @@ class MergeRequestDiff < ActiveRecord::Base
       new_attributes[:real_size] = diff_collection.real_size
 
       if diff_collection.any?
-        new_diffs = dump_diffs(diff_collection)
         new_attributes[:state] = :collected
-      end
 
-      new_attributes[:st_diffs] = new_diffs || []
+        create_merge_request_diff_files(diff_collection)
+      end
 
       # Set our state to 'overflow' to make the #empty? and #collected?
       # methods (generated by StateMachine) return false.
diff --git a/app/models/merge_request_diff_file.rb b/app/models/merge_request_diff_file.rb
new file mode 100644
index 00000000000..598ebd4d829
--- /dev/null
+++ b/app/models/merge_request_diff_file.rb
@@ -0,0 +1,11 @@
+class MergeRequestDiffFile < ActiveRecord::Base
+  include Gitlab::EncodingHelper
+
+  belongs_to :merge_request_diff
+
+  def utf8_diff
+    return '' if diff.blank?
+
+    encode_utf8(diff) if diff.respond_to?(:encoding)
+  end
+end
-- 
cgit v1.2.1