Revamp trending projects query

This changes the query to use a COUNT nested in an INNER JOIN, instead
of a COUNT plus a GROUP BY. There are two reasons for this:

1. Using a COUNT in an INNER JOIN can be quite a bit faster.
2. The use of a GROUP BY means that method calls such as "any?"
   (and everything else that calls "count") operate on a Hash that
   counts the amount of notes on a per project basis, instead of just
   counting the total amount of projects.

The query has been moved into Project.trending as its logic is simple
enough. As a result of this testing the TrendingProjectsFinder class
simply involves testing if the right methods are called, removing the
need for setting up database records.
This commit is contained in:
Yorick Peterse 2015-10-06 16:35:51 +02:00
parent d15eec6460
commit b7abba0ca0
4 changed files with 74 additions and 33 deletions

View file

@ -1,13 +1,6 @@
class TrendingProjectsFinder
def execute(current_user, start_date = nil)
start_date ||= Date.today - 1.month
projects = projects_for(current_user)
# Determine trending projects based on comments count
# for period of time - ex. month
projects.joins(:notes).where('notes.created_at >= ?', start_date).
group("projects.id").reorder("count(notes.id) DESC")
def execute(current_user, start_date = 1.month.ago)
projects_for(current_user).trending(start_date)
end
private

View file

@ -260,6 +260,20 @@ class Project < ActiveRecord::Base
name_pattern = Gitlab::Regex::NAMESPACE_REGEX_STR
%r{(?<project>#{name_pattern}/#{name_pattern})}
end
def trending(since = 1.month.ago)
# By counting in the JOIN we don't expose the GROUP BY to the outer query.
# This means that calls such as "any?" and "count" just return a number of
# the total count, instead of the counts grouped per project as a Hash.
join_body = "INNER JOIN (
SELECT project_id, COUNT(*) AS amount
FROM notes
WHERE created_at >= #{sanitize(since)}
GROUP BY project_id
) join_note_counts ON projects.id = join_note_counts.project_id"
joins(join_body).reorder('join_note_counts.amount DESC')
end
end
def team

View file

@ -1,42 +1,38 @@
require 'spec_helper'
describe TrendingProjectsFinder do
let(:user) { create(:user) }
let(:group) { create(:group) }
let(:project1) { create(:empty_project, :public, group: group) }
let(:project2) { create(:empty_project, :public, group: group) }
before do
2.times do
create(:note_on_commit, project: project1)
end
create(:note_on_commit, project: project2)
end
let(:user) { build(:user) }
describe '#execute' do
describe 'without an explicit start date' do
subject { described_class.new.execute(user).to_a }
subject { described_class.new }
it 'sorts Projects by the amount of notes in descending order' do
expect(subject).to eq([project1, project2])
it 'returns the trending projects' do
relation = double(:ar_relation)
allow(subject).to receive(:projects_for)
.with(user)
.and_return(relation)
allow(relation).to receive(:trending)
.with(an_instance_of(ActiveSupport::TimeWithZone))
end
end
describe 'with an explicit start date' do
let(:date) { 2.months.ago }
subject { described_class.new.execute(user, date).to_a }
subject { described_class.new }
before do
2.times do
create(:note_on_commit, project: project2, created_at: date)
end
end
it 'returns the trending projects' do
relation = double(:ar_relation)
it 'sorts Projects by the amount of notes in descending order' do
expect(subject).to eq([project2, project1])
allow(subject).to receive(:projects_for)
.with(user)
.and_return(relation)
allow(relation).to receive(:trending)
.with(date)
end
end
end

View file

@ -423,4 +423,42 @@ describe Project do
it { expect(project.gitlab_ci?).to be_truthy }
it { expect(project.gitlab_ci_project).to be_a(Ci::Project) }
end
describe '.trending' do
let(:group) { create(:group) }
let(:project1) { create(:empty_project, :public, group: group) }
let(:project2) { create(:empty_project, :public, group: group) }
before do
2.times do
create(:note_on_commit, project: project1)
end
create(:note_on_commit, project: project2)
end
describe 'without an explicit start date' do
subject { described_class.trending.to_a }
it 'sorts Projects by the amount of notes in descending order' do
expect(subject).to eq([project1, project2])
end
end
describe 'with an explicit start date' do
let(:date) { 2.months.ago }
subject { described_class.trending(date).to_a }
before do
2.times do
create(:note_on_commit, project: project2, created_at: date)
end
end
it 'sorts Projects by the amount of notes in descending order' do
expect(subject).to eq([project2, project1])
end
end
end
end