Merge branch 'trending-caching' into 'master'

Refactor TrendingProjectsFinder to support caching

## What does this MR do?

This refactors `TrendingProjectsFinder` so it can support caching of the data. See cb7d398972d786ba7133418266fa34ae641b2497 for more details.

## Why was this MR needed?

Trending projects is quite slow, easily taking seconds to load the entire page.

https://gitlab.com/gitlab-org/gitlab-ce/issues/22164

https://gitlab.com/gitlab-com/infrastructure/milestones/4, in particular the section "Trending page under 2s"

See merge request !6672

CHANGELOG

@@ -34,6 +34,7 @@ v 8.13.0 (unreleased)
   - Take filters in account in issuable counters. !6496
   - Use custom Ruby images to test builds (registry.dev.gitlab.org/gitlab/gitlab-build-images:*)
   - Append issue template to existing description !6149 (Joseph Frazier)
+  - Trending projects now only show public projects and the list of projects is cached for a day
   - Revoke button in Applications Settings underlines on hover.
   - Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska)
   - Fix Long commit messages overflow viewport in file tree

app/controllers/explore/projects_controller.rb

@@ -21,7 +21,7 @@ class Explore::ProjectsController < Explore::ApplicationController
   end
 
   def trending
-    @projects = TrendingProjectsFinder.new.execute(current_user)
+    @projects = TrendingProjectsFinder.new.execute
     @projects = filter_projects(@projects)
     @projects = @projects.page(params[:page])
 

app/finders/trending_projects_finder.rb

+# Finder for retrieving public trending projects in a given time range.
 class TrendingProjectsFinder
-  def execute(current_user, start_date = 1.month.ago)
-    projects_for(current_user).trending(start_date)
+  # current_user - The currently logged in User, if any.
+  # last_months - The number of months to limit the trending data to.
+  def execute(months_limit = 1)
+    Rails.cache.fetch(cache_key_for(months_limit), expires_in: 1.day) do
+      Project.public_only.trending(months_limit.months.ago)
+    end
   end
 
   private
 
-  def projects_for(current_user)
-    ProjectsFinder.new.execute(current_user)
+  def cache_key_for(months)
+    "trending_projects/#{months}"
   end
 end

app/models/project.rb

@@ -380,6 +380,7 @@ class Project < ActiveRecord::Base
         SELECT project_id, COUNT(*) AS amount
         FROM notes
         WHERE created_at >= #{sanitize(since)}
+        AND system IS FALSE
         GROUP BY project_id
       ) join_note_counts ON projects.id = join_note_counts.project_id"
 

spec/finders/trending_projects_finder_spec.rb

 require 'spec_helper'
 
 describe TrendingProjectsFinder do
-  let(:user) { build(:user) }
+  let(:user) { create(:user) }
+  let(:public_project1) { create(:empty_project, :public) }
+  let(:public_project2) { create(:empty_project, :public) }
+  let(:private_project) { create(:empty_project, :private) }
+  let(:internal_project) { create(:empty_project, :internal) }
+
+  before do
+    3.times do
+      create(:note_on_commit, project: public_project1)
+    end
 
-  describe '#execute' do
-    describe 'without an explicit start date' do
-      subject { described_class.new }
+    2.times do
+      create(:note_on_commit, project: public_project2, created_at: 5.weeks.ago)
+    end
 
-      it 'returns the trending projects' do
-        relation = double(:ar_relation)
+    create(:note_on_commit, project: private_project)
+    create(:note_on_commit, project: internal_project)
+  end
 
-        allow(subject).to receive(:projects_for)
-          .with(user)
-          .and_return(relation)
+  describe '#execute', caching: true do
+    context 'without an explicit time range' do
+      it 'returns public trending projects' do
+        projects = described_class.new.execute
 
-        allow(relation).to receive(:trending)
-          .with(an_instance_of(ActiveSupport::TimeWithZone))
+        expect(projects).to eq([public_project1])
       end
     end
 
-    describe 'with an explicit start date' do
-      let(:date) { 2.months.ago }
+    context 'with an explicit time range' do
+      it 'returns public trending projects' do
+        projects = described_class.new.execute(2)
 
-      subject { described_class.new }
+        expect(projects).to eq([public_project1, public_project2])
+      end
+    end
 
-      it 'returns the trending projects' do
-        relation = double(:ar_relation)
+    it 'caches the list of projects' do
+      projects = described_class.new
 
-        allow(subject).to receive(:projects_for)
-          .with(user)
-          .and_return(relation)
+      expect(Project).to receive(:trending).once
 
-        allow(relation).to receive(:trending)
-          .with(date)
-      end
+      2.times { projects.execute }
     end
   end
 end

spec/models/project_spec.rb

@@ -824,6 +824,14 @@ describe Project, models: true do
         expect(subject).to eq([project2, project1])
       end
     end
+
+    it 'does not take system notes into account' do
+      10.times do
+        create(:note_on_commit, project: project2, system: true)
+      end
+
+      expect(described_class.trending.to_a).to eq([project1, project2])
+    end
   end
 
   describe '.visible_to_user' do