From 59804abc3d59070e56e0c541884a54fb88242eb2 Mon Sep 17 00:00:00 2001 From: alpaca-tc Date: Sat, 6 May 2017 23:31:07 +0900 Subject: [PATCH] Optimize MuteService and AfterBlockService (#2836) --- app/lib/feed_manager.rb | 8 ++++++ app/services/after_block_service.rb | 16 +----------- app/services/mute_service.rb | 16 +----------- spec/services/after_block_service_spec.rb | 29 ++++++++++++++++++++ spec/services/mute_service_spec.rb | 32 ++++++++++++++++++++++- 5 files changed, 70 insertions(+), 31 deletions(-) create mode 100644 spec/services/after_block_service_spec.rb diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 339a5c78bde..aaff9acd3b1 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -76,6 +76,14 @@ class FeedManager end end + def clear_from_timeline(account, target_account) + timeline_key = key(:home, account.id) + timeline_status_ids = redis.zrange(timeline_key, 0, -1) + target_status_ids = Status.where(id: timeline_status_ids, account: target_account).ids + + redis.zrem(timeline_key, target_status_ids) if target_status_ids.present? + end + private def redis diff --git a/app/services/after_block_service.rb b/app/services/after_block_service.rb index d1a846f21c7..a7dce08c758 100644 --- a/app/services/after_block_service.rb +++ b/app/services/after_block_service.rb @@ -2,30 +2,16 @@ class AfterBlockService < BaseService def call(account, target_account) - clear_timelines(account, target_account) + FeedManager.instance.clear_from_timeline(account, target_account) clear_notifications(account, target_account) end private - def clear_timelines(account, target_account) - home_key = FeedManager.instance.key(:home, account.id) - - redis.pipelined do - target_account.statuses.select('id').reorder(nil).find_each do |status| - redis.zrem(home_key, status.id) - end - end - end - def clear_notifications(account, target_account) Notification.where(account: account).joins(:follow).where(activity_type: 'Follow', follows: { account_id: target_account.id }).delete_all Notification.where(account: account).joins(mention: :status).where(activity_type: 'Mention', statuses: { account_id: target_account.id }).delete_all Notification.where(account: account).joins(:favourite).where(activity_type: 'Favourite', favourites: { account_id: target_account.id }).delete_all Notification.where(account: account).joins(:status).where(activity_type: 'Status', statuses: { account_id: target_account.id }).delete_all end - - def redis - Redis.current - end end diff --git a/app/services/mute_service.rb b/app/services/mute_service.rb index 1a650ed2a0c..92f92cc7d3d 100644 --- a/app/services/mute_service.rb +++ b/app/services/mute_service.rb @@ -3,21 +3,7 @@ class MuteService < BaseService def call(account, target_account) return if account.id == target_account.id - clear_home_timeline(account, target_account) + FeedManager.instance.clear_from_timeline(account, target_account) account.mute!(target_account) end - - private - - def clear_home_timeline(account, target_account) - home_key = FeedManager.instance.key(:home, account.id) - - target_account.statuses.select('id').reorder(nil).find_each do |status| - redis.zrem(home_key, status.id) - end - end - - def redis - Redis.current - end end diff --git a/spec/services/after_block_service_spec.rb b/spec/services/after_block_service_spec.rb new file mode 100644 index 00000000000..65f36c7d135 --- /dev/null +++ b/spec/services/after_block_service_spec.rb @@ -0,0 +1,29 @@ +require 'rails_helper' + +RSpec.describe AfterBlockService do + subject do + -> { described_class.new.call(account, target_account) } + end + + let(:account) { Fabricate(:account) } + let(:target_account) { Fabricate(:account) } + + describe 'home timeline' do + let(:status) { Fabricate(:status, account: target_account) } + let(:other_account_status) { Fabricate(:status) } + let(:home_timeline_key) { FeedManager.instance.key(:home, account.id) } + + before do + Redis.current.del(home_timeline_key) + end + + it "clears account's statuses" do + FeedManager.instance.push(:home, account, status) + FeedManager.instance.push(:home, account, other_account_status) + + is_expected.to change { + Redis.current.zrange(home_timeline_key, 0, -1) + }.from([status.id.to_s, other_account_status.id.to_s]).to([other_account_status.id.to_s]) + end + end +end diff --git a/spec/services/mute_service_spec.rb b/spec/services/mute_service_spec.rb index 39736841692..8097cb250e6 100644 --- a/spec/services/mute_service_spec.rb +++ b/spec/services/mute_service_spec.rb @@ -1,5 +1,35 @@ require 'rails_helper' RSpec.describe MuteService do - subject { MuteService.new } + subject do + -> { described_class.new.call(account, target_account) } + end + + let(:account) { Fabricate(:account) } + let(:target_account) { Fabricate(:account) } + + describe 'home timeline' do + let(:status) { Fabricate(:status, account: target_account) } + let(:other_account_status) { Fabricate(:status) } + let(:home_timeline_key) { FeedManager.instance.key(:home, account.id) } + + before do + Redis.current.del(home_timeline_key) + end + + it "clears account's statuses" do + FeedManager.instance.push(:home, account, status) + FeedManager.instance.push(:home, account, other_account_status) + + is_expected.to change { + Redis.current.zrange(home_timeline_key, 0, -1) + }.from([status.id.to_s, other_account_status.id.to_s]).to([other_account_status.id.to_s]) + end + end + + it 'mutes account' do + is_expected.to change { + account.muting?(target_account) + }.from(false).to(true) + end end