Backport EE changes for productivity analytics12079-productivity-analytics-part-1

Improve diff lines count efficiency

5 files changed, 93 insertions, 4 deletions

diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 2402fa8e38f..4db2b7a74e5 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -197,7 +197,7 @@ class MergeRequestDiff < ApplicationRecord
 
   def lines_count
     strong_memoize(:lines_count) do
-      diffs.diff_files.sum(&:line_count)
+      raw_diffs(limits: false).line_count
     end
   end
 
@@ -222,6 +222,10 @@ class MergeRequestDiff < ApplicationRecord
     commits.last
   end
 
+  def last_commit
+    commits.first
+  end
+
   def base_commit
     return unless base_commit_sha
 
diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb
index 5c70cb6c66c..cb9154cb1e8 100644
--- a/lib/gitlab/git/diff_collection.rb
+++ b/lib/gitlab/git/diff_collection.rb
@@ -81,6 +81,12 @@ module Gitlab
         end
       end
 
+      def line_count
+        populate!
+
+        @line_count
+      end
+
       def decorate!
         collection = each_with_index do |element, i|
           @array[i] = yield(element)
diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb
index 81658874be7..be6ab0c1200 100644
--- a/spec/lib/gitlab/git/diff_collection_spec.rb
+++ b/spec/lib/gitlab/git/diff_collection_spec.rb
@@ -74,6 +74,11 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           end
         end
 
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq file_count * line_count }
+        end
+
         context 'when limiting is disabled' do
           let(:limits) { false }
 
@@ -100,6 +105,11 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
               expect(subject.size).to eq(3)
             end
           end
+
+          describe '#line_count' do
+            subject { super().line_count }
+            it { is_expected.to eq file_count * line_count }
+          end
         end
       end
 
@@ -120,6 +130,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('0+') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq 1000 }
+        end
+
         it { expect(subject.size).to eq(0) }
 
         context 'when limiting is disabled' do
@@ -139,6 +155,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
             subject { super().real_size }
             it { is_expected.to eq('3') }
           end
+
+          describe '#line_count' do
+            subject { super().line_count }
+            it { is_expected.to eq file_count * line_count }
+          end
+
           it { expect(subject.size).to eq(3) }
         end
       end
@@ -164,6 +186,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('10+') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq 10 }
+        end
+
         it { expect(subject.size).to eq(10) }
 
         context 'when limiting is disabled' do
@@ -183,6 +211,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
             subject { super().real_size }
             it { is_expected.to eq('11') }
           end
+
+          describe '#line_count' do
+            subject { super().line_count }
+            it { is_expected.to eq file_count * line_count }
+          end
+
           it { expect(subject.size).to eq(11) }
         end
       end
@@ -204,6 +238,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('3+') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq 120 }
+        end
+
         it { expect(subject.size).to eq(3) }
 
         context 'when limiting is disabled' do
@@ -223,6 +263,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
             subject { super().real_size }
             it { is_expected.to eq('11') }
           end
+
+          describe '#line_count' do
+            subject { super().line_count }
+            it { is_expected.to eq file_count * line_count }
+          end
+
           it { expect(subject.size).to eq(11) }
         end
       end
@@ -248,6 +294,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('10') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq file_count * line_count }
+        end
+
         it { expect(subject.size).to eq(10) }
       end
     end
@@ -270,6 +322,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
         subject { super().real_size }
         it { is_expected.to eq('9+') }
       end
+
+      describe '#line_count' do
+        subject { super().line_count }
+        it { is_expected.to eq file_count * line_count }
+      end
+
       it { expect(subject.size).to eq(9) }
 
       context 'when limiting is disabled' do
@@ -289,6 +347,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('10') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq file_count * line_count }
+        end
+
         it { expect(subject.size).to eq(10) }
       end
     end
@@ -316,6 +380,11 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
       subject { super().real_size }
       it { is_expected.to eq('0')}
     end
+
+    describe '#line_count' do
+      subject { super().line_count }
+      it { is_expected.to eq 0 }
+    end
   end
 
   describe '#each' do
diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb
index 49573af0fed..bd97cabc11e 100644
--- a/spec/models/merge_request/metrics_spec.rb
+++ b/spec/models/merge_request/metrics_spec.rb
@@ -3,8 +3,6 @@
 require 'spec_helper'
 
 describe MergeRequest::Metrics do
-  subject { described_class.new }
-
   describe 'associations' do
     it { is_expected.to belong_to(:merge_request) }
     it { is_expected.to belong_to(:latest_closed_by).class_name('User') }
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index e7dd7287a75..b86663fd7d9 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -400,6 +400,18 @@ describe MergeRequestDiff do
     end
   end
 
+  describe '#first_commit' do
+    it 'returns first commit' do
+      expect(diff_with_commits.first_commit.sha).to eq(diff_with_commits.merge_request_diff_commits.last.sha)
+    end
+  end
+
+  describe '#last_commit' do
+    it 'returns last commit' do
+      expect(diff_with_commits.last_commit.sha).to eq(diff_with_commits.merge_request_diff_commits.first.sha)
+    end
+  end
+
   describe '#commits_by_shas' do
     let(:commit_shas) { diff_with_commits.commit_shas }
 
@@ -489,7 +501,7 @@ describe MergeRequestDiff do
     subject { diff_with_commits }
 
     it 'returns sum of all changed lines count in diff files' do
-      expect(subject.lines_count).to eq 109
+      expect(subject.lines_count).to eq 189
     end
   end
 end