Re-use issue/MR counts for the pagination systemmr-index-page-performance

This changes the issue and MR index pages so the pagination system
re-uses the output of the COUNT(*) query used to calculate the number of
rows per state (opened, closed, etc). This removes the need for an
additional COUNT(*) on both pages.

4 files changed, 149 insertions, 2 deletions

diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb
new file mode 100644
index 00000000000..c9687af4dd2
--- /dev/null
+++ b/spec/controllers/concerns/issuable_collections_spec.rb
@@ -0,0 +1,82 @@
+require 'spec_helper'
+
+describe IssuableCollections do
+  let(:user) { create(:user) }
+
+  let(:controller) do
+    klass = Class.new do
+      def self.helper_method(name); end
+
+      include IssuableCollections
+    end
+
+    controller = klass.new
+
+    allow(controller).to receive(:params).and_return(state: 'opened')
+
+    controller
+  end
+
+  describe '#redirect_out_of_range' do
+    before do
+      allow(controller).to receive(:url_for)
+    end
+
+    it 'returns true and redirects if the offset is out of range' do
+      relation = double(:relation, current_page: 10)
+
+      expect(controller).to receive(:redirect_to)
+      expect(controller.send(:redirect_out_of_range, relation, 2)).to eq(true)
+    end
+
+    it 'returns false if the offset is not out of range' do
+      relation = double(:relation, current_page: 1)
+
+      expect(controller).not_to receive(:redirect_to)
+      expect(controller.send(:redirect_out_of_range, relation, 2)).to eq(false)
+    end
+  end
+
+  describe '#issues_page_count' do
+    it 'returns the number of issue pages' do
+      project = create(:project, :public)
+
+      create(:issue, project: project)
+
+      finder = IssuesFinder.new(user)
+      issues = finder.execute
+
+      allow(controller).to receive(:issues_finder)
+        .and_return(finder)
+
+      expect(controller.send(:issues_page_count, issues)).to eq(1)
+    end
+  end
+
+  describe '#merge_requests_page_count' do
+    it 'returns the number of merge request pages' do
+      project = create(:project, :public)
+
+      create(:merge_request, source_project: project, target_project: project)
+
+      finder = MergeRequestsFinder.new(user)
+      merge_requests = finder.execute
+
+      allow(controller).to receive(:merge_requests_finder)
+        .and_return(finder)
+
+      pages = controller.send(:merge_requests_page_count, merge_requests)
+
+      expect(pages).to eq(1)
+    end
+  end
+
+  describe '#page_count_for_relation' do
+    it 'returns the number of pages' do
+      relation = double(:relation, limit_value: 20)
+      pages = controller.send(:page_count_for_relation, relation, 28)
+
+      expect(pages).to eq(2)
+    end
+  end
+end
diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb
index 0e80df94e18..47b173dea0a 100644
--- a/spec/finders/issues_finder_spec.rb
+++ b/spec/finders/issues_finder_spec.rb
@@ -15,8 +15,8 @@ describe IssuesFinder do
   set(:award_emoji3) { create(:award_emoji, name: 'thumbsdown', user: user, awardable: issue3) }
 
   describe '#execute' do
-    set(:closed_issue) { create(:issue, author: user2, assignees: [user2], project: project2, state: 'closed') }
-    set(:label_link) { create(:label_link, label: label, target: issue2) }
+    let!(:closed_issue) { create(:issue, author: user2, assignees: [user2], project: project2, state: 'closed') }
+    let!(:label_link) { create(:label_link, label: label, target: issue2) }
     let(:search_user) { user }
     let(:params) { {} }
     let(:issues) { described_class.new(search_user, params.reverse_merge(scope: scope, state: 'opened')).execute }
@@ -347,6 +347,20 @@ describe IssuesFinder do
     end
   end
 
+  describe '#row_count', :request_store do
+    it 'returns the number of rows for the default state' do
+      finder = described_class.new(user)
+
+      expect(finder.row_count).to eq(3)
+    end
+
+    it 'returns the number of rows for a given state' do
+      finder = described_class.new(user, state: 'closed')
+
+      expect(finder.row_count).to be_zero
+    end
+  end
+
   describe '#with_confidentiality_access_check' do
     let(:guest) { create(:user) }
     set(:authorized_user) { create(:user) }
diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb
index b54155a6704..95f445e7905 100644
--- a/spec/finders/merge_requests_finder_spec.rb
+++ b/spec/finders/merge_requests_finder_spec.rb
@@ -108,4 +108,18 @@ describe MergeRequestsFinder do
       end
     end
   end
+
+  describe '#row_count', :request_store do
+    it 'returns the number of rows for the default state' do
+      finder = described_class.new(user)
+
+      expect(finder.row_count).to eq(3)
+    end
+
+    it 'returns the number of rows for a given state' do
+      finder = described_class.new(user, state: 'closed')
+
+      expect(finder.row_count).to eq(1)
+    end
+  end
 end
diff --git a/spec/lib/gitlab/issuables_count_for_state_spec.rb b/spec/lib/gitlab/issuables_count_for_state_spec.rb
new file mode 100644
index 00000000000..c262fdfcb61
--- /dev/null
+++ b/spec/lib/gitlab/issuables_count_for_state_spec.rb
@@ -0,0 +1,37 @@
+require 'spec_helper'
+
+describe Gitlab::IssuablesCountForState do
+  let(:finder) do
+    double(:finder, count_by_state: { opened: 2, closed: 1 })
+  end
+
+  let(:counter) { described_class.new(finder) }
+
+  describe '#for_state_or_opened' do
+    it 'returns the number of issuables for the given state' do
+      expect(counter.for_state_or_opened(:closed)).to eq(1)
+    end
+
+    it 'returns the number of open issuables when no state is given' do
+      expect(counter.for_state_or_opened).to eq(2)
+    end
+
+    it 'returns the number of open issuables when a nil value is given' do
+      expect(counter.for_state_or_opened(nil)).to eq(2)
+    end
+  end
+
+  describe '#[]' do
+    it 'returns the number of issuables for the given state' do
+      expect(counter[:closed]).to eq(1)
+    end
+
+    it 'casts valid states from Strings to Symbols' do
+      expect(counter['closed']).to eq(1)
+    end
+
+    it 'returns 0 when using an invalid state name as a String' do
+      expect(counter['kittens']).to be_zero
+    end
+  end
+end