optimize direct timeline (#7614)

* optimize direct timeline

* fix typo in class name

* change filter condition for direct timeline

* fix codestyle issue

* revoke index_accounts_not_silenced because direct timeline does not use it.

* revoke index_accounts_not_silenced because direct timeline does not use it.

* fix rspec test condition.

* fix rspec test condition.

* fix rspec test condition.

* revoke adding column and partial index

* (direct timeline) move merging logic to model

* fix pagination parameter

* add method arguments that switches return array of status or cache_ids

* fix order by

* returns ActiveRecord.Relation in default behavor

* fix codestyle issue
This commit is contained in:
tateisu 2018-05-28 18:04:06 +09:00 committed by Eugen Rochko
parent ab36e0ef72
commit b87a1229c7
3 changed files with 59 additions and 22 deletions

View File

@ -23,15 +23,18 @@ class Api::V1::Timelines::DirectController < Api::BaseController
end end
def direct_statuses def direct_statuses
direct_timeline_statuses.paginate_by_max_id( direct_timeline_statuses
limit_param(DEFAULT_STATUSES_LIMIT),
params[:max_id],
params[:since_id]
)
end end
def direct_timeline_statuses def direct_timeline_statuses
Status.as_direct_timeline(current_account) # this query requires built in pagination.
Status.as_direct_timeline(
current_account,
limit_param(DEFAULT_STATUSES_LIMIT),
params[:max_id],
params[:since_id],
true # returns array of cache_ids object
)
end end
def insert_pagination_headers def insert_pagination_headers

View File

@ -188,12 +188,45 @@ class Status < ApplicationRecord
where(account: [account] + account.following).where(visibility: [:public, :unlisted, :private]) where(account: [account] + account.following).where(visibility: [:public, :unlisted, :private])
end end
def as_direct_timeline(account) def as_direct_timeline(account, limit = 20, max_id = nil, since_id = nil, cache_ids = false)
query = joins("LEFT OUTER JOIN mentions ON statuses.id = mentions.status_id AND mentions.account_id = #{account.id}") # direct timeline is mix of direct message from_me and to_me.
.where("mentions.account_id = #{account.id} OR statuses.account_id = #{account.id}") # 2 querys are executed with pagination.
.where(visibility: [:direct]) # constant expression using arel_table is required for partial index
apply_timeline_filters(query, account, false) # _from_me part does not require any timeline filters
query_from_me = where(account_id: account.id)
.where(Status.arel_table[:visibility].eq(3))
.limit(limit)
.order('statuses.id DESC')
# _to_me part requires mute and block filter.
# FIXME: may we check mutes.hide_notifications?
query_to_me = Status
.joins(:mentions)
.merge(Mention.where(account_id: account.id))
.where(Status.arel_table[:visibility].eq(3))
.limit(limit)
.order('mentions.status_id DESC')
.not_excluded_by_account(account)
if max_id.present?
query_from_me = query_from_me.where('statuses.id < ?', max_id)
query_to_me = query_to_me.where('mentions.status_id < ?', max_id)
end
if since_id.present?
query_from_me = query_from_me.where('statuses.id > ?', since_id)
query_to_me = query_to_me.where('mentions.status_id > ?', since_id)
end
if cache_ids
# returns array of cache_ids object that have id and updated_at
(query_from_me.cache_ids.to_a + query_to_me.cache_ids.to_a).uniq(&:id).sort_by(&:id).reverse.take(limit)
else
# returns ActiveRecord.Relation
items = (query_from_me.select(:id).to_a + query_to_me.select(:id).to_a).uniq(&:id).sort_by(&:id).reverse.take(limit)
Status.where(id: items.map(&:id))
end
end end
def as_public_timeline(account = nil, local_only = false) def as_public_timeline(account = nil, local_only = false)

View File

@ -154,7 +154,7 @@ RSpec.describe Status, type: :model do
describe '#target' do describe '#target' do
it 'returns nil if the status is self-contained' do it 'returns nil if the status is self-contained' do
expect(subject.target).to be_nil expect(subject.target).to be_nil
end end
it 'returns nil if the status is a reply' do it 'returns nil if the status is a reply' do
@ -333,24 +333,25 @@ RSpec.describe Status, type: :model do
expect(@results).to_not include(@followed_public_status) expect(@results).to_not include(@followed_public_status)
end end
it 'includes direct statuses mentioning recipient from followed' do
Fabricate(:mention, account: account, status: @followed_direct_status)
expect(@results).to include(@followed_direct_status)
end
it 'does not include direct statuses not mentioning recipient from followed' do it 'does not include direct statuses not mentioning recipient from followed' do
expect(@results).to_not include(@followed_direct_status) expect(@results).to_not include(@followed_direct_status)
end end
it 'includes direct statuses mentioning recipient from non-followed' do
Fabricate(:mention, account: account, status: @not_followed_direct_status)
expect(@results).to include(@not_followed_direct_status)
end
it 'does not include direct statuses not mentioning recipient from non-followed' do it 'does not include direct statuses not mentioning recipient from non-followed' do
expect(@results).to_not include(@not_followed_direct_status) expect(@results).to_not include(@not_followed_direct_status)
end end
it 'includes direct statuses mentioning recipient from followed' do
Fabricate(:mention, account: account, status: @followed_direct_status)
results2 = Status.as_direct_timeline(account)
expect(results2).to include(@followed_direct_status)
end
it 'includes direct statuses mentioning recipient from non-followed' do
Fabricate(:mention, account: account, status: @not_followed_direct_status)
results2 = Status.as_direct_timeline(account)
expect(results2).to include(@not_followed_direct_status)
end
end end
describe '.as_public_timeline' do describe '.as_public_timeline' do