From a2a2af244cf64659870e0547f0b0123ce0571a16 Mon Sep 17 00:00:00 2001 From: alpaca-tc Date: Tue, 16 May 2017 09:54:17 +0900 Subject: [PATCH] Optimize Status#permitted_for 24x (#3069) * Build query with arel node * Add spec for current Status#permitted_for implementation * Refactor status.rb * Order by visibility to optimize query --- app/models/status.rb | 22 +++++++++------- spec/models/status_spec.rb | 53 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/app/models/status.rb b/app/models/status.rb index 626f008ad..8d827e980 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -55,7 +55,7 @@ class Status < ApplicationRecord validates_with StatusLengthValidator validates :reblog, uniqueness: { scope: :account }, if: 'reblog?' - default_scope { order('id desc') } + default_scope { order(id: :desc) } scope :remote, -> { where.not(uri: nil) } scope :local, -> { where(uri: nil) } @@ -202,18 +202,22 @@ class Status < ApplicationRecord end def permitted_for(target_account, account) - return where.not(visibility: [:private, :direct]) if account.nil? + visibility = [:public, :unlisted] - if target_account.blocking?(account) # get rid of blocked peeps + if account.nil? + where(visibility: visibility) + elsif target_account.blocking?(account) # get rid of blocked peeps none elsif account.id == target_account.id # author can see own stuff all - elsif account.following?(target_account) # followers can see followers-only stuff, but also things they are mentioned in - joins('LEFT OUTER JOIN mentions ON statuses.id = mentions.status_id AND mentions.account_id = ' + account.id.to_s) - .where('statuses.visibility != ? OR mentions.id IS NOT NULL', Status.visibilities[:direct]) - else # non-followers can see everything that isn't private/direct, but can see stuff they are mentioned in - joins('LEFT OUTER JOIN mentions ON statuses.id = mentions.status_id AND mentions.account_id = ' + account.id.to_s) - .where('statuses.visibility NOT IN (?) OR mentions.id IS NOT NULL', [Status.visibilities[:direct], Status.visibilities[:private]]) + else + # followers can see followers-only stuff, but also things they are mentioned in. + # non-followers can see everything that isn't private/direct, but can see stuff they are mentioned in. + visibility.push(:private) if account.following?(target_account) + + joins("LEFT OUTER JOIN mentions ON statuses.id = mentions.status_id AND mentions.account_id = #{account.id}") + .where(arel_table[:visibility].in(visibility).or(Mention.arel_table[:id].not_eq(nil))) + .order(visibility: :desc) end end diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index d280525fc..d4f85b725 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -439,6 +439,59 @@ RSpec.describe Status, type: :model do end end + describe '.permitted_for' do + subject { described_class.permitted_for(target_account, account).pluck(:visibility) } + + let(:target_account) { alice } + let(:account) { bob } + let!(:public_status) { Fabricate(:status, account: target_account, visibility: 'public') } + let!(:unlisted_status) { Fabricate(:status, account: target_account, visibility: 'unlisted') } + let!(:private_status) { Fabricate(:status, account: target_account, visibility: 'private') } + + let!(:direct_status) do + Fabricate(:status, account: target_account, visibility: 'direct').tap do |status| + Fabricate(:mention, status: status, account: account) + end + end + + let!(:other_direct_status) do + Fabricate(:status, account: target_account, visibility: 'direct').tap do |status| + Fabricate(:mention, status: status) + end + end + + context 'given nil' do + let(:account) { nil } + let(:direct_status) { nil } + it { is_expected.to eq(%w(unlisted public)) } + end + + context 'given blocked account' do + before do + target_account.block!(account) + end + + it { is_expected.to be_empty } + end + + context 'given same account' do + let(:account) { target_account } + it { is_expected.to eq(%w(direct direct private unlisted public)) } + end + + context 'given followed account' do + before do + account.follow!(target_account) + end + + it { is_expected.to eq(%w(direct private unlisted public)) } + end + + context 'given unfollowed account' do + it { is_expected.to eq(%w(direct unlisted public)) } + end + end + describe 'before_create' do it 'sets account being replied to correctly over intermediary nodes' do first_status = Fabricate(:status, account: bob)