From db4119f9711e6c2ac640325640f6e61b1e177ed3 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Fri, 19 May 2017 10:21:52 -0400
Subject: [PATCH] Specs for precompute feed service (#3142)

* Add spec for precompute feed service

* Refactor PrecomputeFeedService

* spec wip
---
 app/services/precompute_feed_service.rb       | 46 +++++++++++++++----
 app/workers/regeneration_worker.rb            |  4 +-
 spec/services/precompute_feed_service_spec.rb | 17 +++++++
 3 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/app/services/precompute_feed_service.rb b/app/services/precompute_feed_service.rb
index 46f0057cb..acda5108a 100644
--- a/app/services/precompute_feed_service.rb
+++ b/app/services/precompute_feed_service.rb
@@ -1,20 +1,48 @@
 # frozen_string_literal: true
 
 class PrecomputeFeedService < BaseService
-  # Fill up a user's home/mentions feed from DB and return a subset
-  # @param [Symbol] type :home or :mentions
-  # @param [Account] account
-  def call(_, account)
+  LIMIT = FeedManager::MAX_ITEMS / 4
+
+  def call(account)
+    @account = account
+    populate_feed
+  end
+
+  private
+
+  attr_reader :account
+
+  def populate_feed
     redis.pipelined do
-      # NOTE: Added `id desc, account_id desc` to `ORDER BY` section to optimize query.
-      Status.as_home_timeline(account).order(account_id: :desc).limit(FeedManager::MAX_ITEMS / 4).each do |status|
-        next if status.direct_visibility? || FeedManager.instance.filter?(:home, status, account.id)
-        redis.zadd(FeedManager.instance.key(:home, account.id), status.id, status.reblog? ? status.reblog_of_id : status.id)
+      statuses.each do |status|
+        process_status(status)
       end
     end
   end
 
-  private
+  def process_status(status)
+    add_status_to_feed(status) unless skip_status?(status)
+  end
+
+  def skip_status?(status)
+    status.direct_visibility? || status_filtered?(status)
+  end
+
+  def add_status_to_feed(status)
+    redis.zadd(account_home_key, status.id, status.reblog? ? status.reblog_of_id : status.id)
+  end
+
+  def status_filtered?(status)
+    FeedManager.instance.filter?(:home, status, account.id)
+  end
+
+  def account_home_key
+    FeedManager.instance.key(:home, account.id)
+  end
+
+  def statuses
+    Status.as_home_timeline(account).order(account_id: :desc).limit(LIMIT)
+  end
 
   def redis
     Redis.current
diff --git a/app/workers/regeneration_worker.rb b/app/workers/regeneration_worker.rb
index da8b845f6..0cc6b668c 100644
--- a/app/workers/regeneration_worker.rb
+++ b/app/workers/regeneration_worker.rb
@@ -6,6 +6,8 @@ class RegenerationWorker
   sidekiq_options queue: 'pull', backtrace: true, unique: :until_executed
 
   def perform(account_id, _ = :home)
-    PrecomputeFeedService.new.call(:home, Account.find(account_id))
+    account = Account.find(account_id)
+
+    PrecomputeFeedService.new.call(account)
   end
 end
diff --git a/spec/services/precompute_feed_service_spec.rb b/spec/services/precompute_feed_service_spec.rb
index 392cb0e4d..9f56b0256 100644
--- a/spec/services/precompute_feed_service_spec.rb
+++ b/spec/services/precompute_feed_service_spec.rb
@@ -1,5 +1,22 @@
+# frozen_string_literal: true
+
 require 'rails_helper'
 
 RSpec.describe PrecomputeFeedService do
   subject { PrecomputeFeedService.new }
+
+  describe 'call' do
+    let(:account) { Fabricate(:account) }
+    it 'fills a user timeline with statuses' do
+      account = Fabricate(:account)
+      followed_account = Fabricate(:account)
+      Fabricate(:follow, account: account, target_account: followed_account)
+      status = Fabricate(:status, account: followed_account)
+
+      expected_redis_args = FeedManager.instance.key(:home, account.id), status.id, status.id
+      expect_any_instance_of(Redis).to receive(:zadd).with(*expected_redis_args)
+
+      subject.call(account)
+    end
+  end
 end