Improve checking if projects would be returned

In various places we check if the same relation would return projects.
This is done using "any?" which will run a COUNT query with any
LIMIT/OFFSET values still applied.

To work around all this we introduce 2 helper methods that take care of
doing the right thing. This leads to the produced queries being simpler
and fewer queries being executed.

7 files changed, 72 insertions, 6 deletions

diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 34ff6107eab..a268413e84f 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -225,6 +225,26 @@ module ProjectsHelper
     end
   end
 
+  # Returns true if any projects are present.
+  #
+  # If the relation has a LIMIT applied we'll cast the relation to an Array
+  # since repeated any? checks would otherwise result in multiple COUNT queries
+  # being executed.
+  #
+  # If no limit is applied we'll just issue a COUNT since the result set could
+  # be too large to load into memory.
+  def any_projects?(projects)
+    if projects.limit_value
+      projects.to_a.any?
+    else
+      projects.except(:offset).any?
+    end
+  end
+
+  def has_projects_or_name?(projects, params)
+    !!(params[:name] || any_projects?(projects))
+  end
+
   private
 
   def repo_children_classes(field)
diff --git a/app/views/dashboard/projects/index.html.haml b/app/views/dashboard/projects/index.html.haml
index ec6cb1a9624..c546252455a 100644
--- a/app/views/dashboard/projects/index.html.haml
+++ b/app/views/dashboard/projects/index.html.haml
@@ -13,10 +13,8 @@
   - if show_callout?('user_callout_dismissed')
     = render 'shared/user_callout'
 
-  - if @projects.any? || params[:name]
+  - if has_projects_or_name?(@projects, params)
     = render 'dashboard/projects_head'
-
-  - if @projects.any? || params[:name]
     = render 'projects'
   - else
     = render "zero_authorized_projects"
diff --git a/app/views/dashboard/projects/starred.html.haml b/app/views/dashboard/projects/starred.html.haml
index ae1d733a516..14f9f8cd70a 100644
--- a/app/views/dashboard/projects/starred.html.haml
+++ b/app/views/dashboard/projects/starred.html.haml
@@ -9,7 +9,7 @@
 %div{ class: container_class }
   = render 'dashboard/projects_head'
 
-  - if @projects.any? || params[:filter_projects]
+  - if params[:filter_projects] || any_projects?(@projects)
     = render 'projects'
   - else
     %h3 You don't have starred projects yet
diff --git a/app/views/shared/_new_project_item_select.html.haml b/app/views/shared/_new_project_item_select.html.haml
index 5f3cdaefd54..b417e83cdb6 100644
--- a/app/views/shared/_new_project_item_select.html.haml
+++ b/app/views/shared/_new_project_item_select.html.haml
@@ -1,4 +1,4 @@
-- if @projects.any?
+- if any_projects?(@projects)
   .project-item-select-holder
     = project_select_tag :project_path, class: "project-item-select", data: { include_groups: local_assigns[:include_groups], order_by: 'last_activity_at', relative_path: local_assigns[:path] }, with_feature_enabled: local_assigns[:with_feature_enabled]
     %a.btn.btn-new.new-project-item-select-button
diff --git a/app/views/shared/projects/_list.html.haml b/app/views/shared/projects/_list.html.haml
index 7ed6c622558..66783fac6b2 100644
--- a/app/views/shared/projects/_list.html.haml
+++ b/app/views/shared/projects/_list.html.haml
@@ -10,7 +10,7 @@
 - load_pipeline_status(projects)
 
 .js-projects-list-holder
-  - if projects.any?
+  - if any_projects?(projects)
     %ul.projects-list
       - projects.each_with_index do |project, i|
         - css_class = (i >= projects_limit) || project.pending_delete? ? 'hide' : nil
diff --git a/changelogs/unreleased/dont-use-limit-offset-when-counting-projects.yml b/changelogs/unreleased/dont-use-limit-offset-when-counting-projects.yml
new file mode 100644
index 00000000000..8ecea635ce5
--- /dev/null
+++ b/changelogs/unreleased/dont-use-limit-offset-when-counting-projects.yml
@@ -0,0 +1,4 @@
+---
+title: "Improve performance of checking for projects on the projects dashboard"
+merge_request:
+author:
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 236a7c29634..37a5e6b474e 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -411,4 +411,48 @@ describe ProjectsHelper do
       end
     end
   end
+
+  describe '#has_projects_or_name?' do
+    let(:projects) do
+      create(:project)
+      Project.all
+    end
+
+    it 'returns true when there are projects' do
+      expect(helper.has_projects_or_name?(projects, {})).to eq(true)
+    end
+
+    it 'returns true when there are no projects but a name is given' do
+      expect(helper.has_projects_or_name?(Project.none, name: 'foo')).to eq(true)
+    end
+
+    it 'returns false when there are no projects and there is no name' do
+      expect(helper.has_projects_or_name?(Project.none, {})).to eq(false)
+    end
+  end
+
+  describe '#any_projects?' do
+    before do
+      create(:project)
+    end
+
+    it 'returns true when projects will be returned' do
+      expect(helper.any_projects?(Project.all)).to eq(true)
+    end
+
+    it 'returns false when no projects will be returned' do
+      expect(helper.any_projects?(Project.none)).to eq(false)
+    end
+
+    it 'only executes a single query when a LIMIT is applied' do
+      relation = Project.limit(1)
+      recorder = ActiveRecord::QueryRecorder.new do
+        2.times do
+          helper.any_projects?(relation)
+        end
+      end
+
+      expect(recorder.count).to eq(1)
+    end
+  end
 end