Commit 154253ca authored by Yorick Peterse's avatar Yorick Peterse

Refactor TrendingProjectsFinder to support caching

== Public Projects

This finder class now _only_ returns public projects. Previously this
finder would also return private and internal projects. Including these
projects makes caching data much harder and less efficient. Meanwhile
including this data isn't very useful as very few users would be
interested in seeing projects they have access to as trending. That is,
the feature is more useful when you want to see what _other_ popular
projects there are.

== Caching

The data returned by TrendingProjectsFinder is now cached for a day
based on the number of months the data should be restricted to. The
cache is not flushed explicitly, instead it's rebuilt whenever it
expires.

== Timings

To measure the impact I changed the finder code to use the last 24
months instead of the last month. I then executed and measured 10
requests to the explore page. On the current "master" branch (commit
88fa5916) this would take an average of
2.43 seconds. Using the changes of this commit this was reduced to
around 1.7 seconds.

Fixes gitlab-org/gitlab-ce#22164
parent 706737a0
...@@ -31,6 +31,7 @@ v 8.13.0 (unreleased) ...@@ -31,6 +31,7 @@ v 8.13.0 (unreleased)
- Take filters in account in issuable counters. !6496 - Take filters in account in issuable counters. !6496
- Use custom Ruby images to test builds (registry.dev.gitlab.org/gitlab/gitlab-build-images:*) - Use custom Ruby images to test builds (registry.dev.gitlab.org/gitlab/gitlab-build-images:*)
- Append issue template to existing description !6149 (Joseph Frazier) - 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. - Revoke button in Applications Settings underlines on hover.
- Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska) - Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska)
- Fix Long commit messages overflow viewport in file tree - Fix Long commit messages overflow viewport in file tree
......
...@@ -21,7 +21,7 @@ class Explore::ProjectsController < Explore::ApplicationController ...@@ -21,7 +21,7 @@ class Explore::ProjectsController < Explore::ApplicationController
end end
def trending def trending
@projects = TrendingProjectsFinder.new.execute(current_user) @projects = TrendingProjectsFinder.new.execute
@projects = filter_projects(@projects) @projects = filter_projects(@projects)
@projects = @projects.page(params[:page]) @projects = @projects.page(params[:page])
......
# Finder for retrieving public trending projects in a given time range.
class TrendingProjectsFinder class TrendingProjectsFinder
def execute(current_user, start_date = 1.month.ago) # current_user - The currently logged in User, if any.
projects_for(current_user).trending(start_date) # 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 end
private private
def projects_for(current_user) def cache_key_for(months)
ProjectsFinder.new.execute(current_user) "trending_projects/#{months}"
end end
end end
require 'spec_helper' require 'spec_helper'
describe TrendingProjectsFinder do 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 2.times do
describe 'without an explicit start date' do create(:note_on_commit, project: public_project2, created_at: 5.weeks.ago)
subject { described_class.new } end
it 'returns the trending projects' do create(:note_on_commit, project: private_project)
relation = double(:ar_relation) create(:note_on_commit, project: internal_project)
end
allow(subject).to receive(:projects_for) describe '#execute', caching: true do
.with(user) context 'without an explicit time range' do
.and_return(relation) it 'returns public trending projects' do
projects = described_class.new.execute
allow(relation).to receive(:trending) expect(projects).to eq([public_project1])
.with(an_instance_of(ActiveSupport::TimeWithZone))
end end
end end
describe 'with an explicit start date' do context 'with an explicit time range' do
let(:date) { 2.months.ago } 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 it 'caches the list of projects' do
relation = double(:ar_relation) projects = described_class.new
allow(subject).to receive(:projects_for) expect(Project).to receive(:trending).once
.with(user)
.and_return(relation)
allow(relation).to receive(:trending) 2.times { projects.execute }
.with(date)
end
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment