From 2bbdc5d143c66c9e5dfdf9fa74ef899022cebbb0 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Fri, 16 Aug 2019 20:49:35 -0500
Subject: [PATCH] jobs: migrate saved searches to ActiveJob.

* Fix tests to run the searches for real instead of mocking everything out.

* Fix SavedSearch.populate to only use the read only database in
  production because in breaks things in tests. Specifically:
  the posts get created in one db connection but searched for in
  another, but the second transaction doesn't see the uncommitted posts
  in the first transaction, so the search doesn't work.
---
 app/jobs/populate_saved_search_job.rb |  7 +++
 app/models/saved_search.rb            |  4 +-
 test/unit/saved_search_test.rb        | 84 +++++++++++++++------------
 3 files changed, 55 insertions(+), 40 deletions(-)
 create mode 100644 app/jobs/populate_saved_search_job.rb

diff --git a/app/jobs/populate_saved_search_job.rb b/app/jobs/populate_saved_search_job.rb
new file mode 100644
index 000000000..81b34c8d8
--- /dev/null
+++ b/app/jobs/populate_saved_search_job.rb
@@ -0,0 +1,7 @@
+class PopulateSavedSearchJob < ApplicationJob
+  queue_as :default
+
+  def perform(query)
+    SavedSearch.populate(query)
+  end
+end
diff --git a/app/models/saved_search.rb b/app/models/saved_search.rb
index 3f5882ae0..c57df5754 100644
--- a/app/models/saved_search.rb
+++ b/app/models/saved_search.rb
@@ -32,7 +32,7 @@ class SavedSearch < ApplicationRecord
             post_ids.merge(sub_ids)
             update_count += 1
           else
-            SavedSearch.delay(queue: "default").populate(query)
+            PopulateSavedSearchJob.perform_later(query)
           end
         end
         post_ids.to_a.sort.last(QUERY_LIMIT)
@@ -97,7 +97,7 @@ class SavedSearch < ApplicationRecord
         CurrentUser.as_system do
           redis_key = "search:#{query}"
           return if redis.exists(redis_key)
-          post_ids = Post.tag_match(query, read_only: true).limit(QUERY_LIMIT).pluck(:id)
+          post_ids = Post.tag_match(query, read_only: Rails.env.production?).limit(QUERY_LIMIT).pluck(:id)
           redis.sadd(redis_key, post_ids)
           redis.expire(redis_key, REDIS_EXPIRY)
         end
diff --git a/test/unit/saved_search_test.rb b/test/unit/saved_search_test.rb
index 7d2513186..9ba44497a 100644
--- a/test/unit/saved_search_test.rb
+++ b/test/unit/saved_search_test.rb
@@ -60,56 +60,64 @@ class SavedSearchTest < ActiveSupport::TestCase
   context ".post_ids_for" do
     context "with a label" do
       setup do
-        SavedSearch.expects(:queries_for).with(1, label: "blah").returns(%w(a b c))
+        create(:saved_search, query: "a", labels: ["blah"], user: @user)
+        create(:saved_search, query: "b", labels: ["blah"], user: @user)
+        create(:saved_search, query: "c", labels: ["blah"], user: @user)
+
+        create(:post, tag_string: "a")
+        create(:post, tag_string: "b")
+        create(:post, tag_string: "c")
       end
 
       context "without a primed cache" do
-        should "delay processing three times" do
-          SavedSearch.expects(:populate).times(3)
-          post_ids = SavedSearch.post_ids_for(1, label: "blah")
-          assert_equal([], post_ids)
-        end
-      end
-
-      context "with a primed cached" do
-        setup do
-          @mock_redis.sadd("search:a", 1)
-          @mock_redis.sadd("search:b", 2)
-          @mock_redis.sadd("search:c", 3)
-        end
-
-        should "fetch the post ids" do
-          SavedSearch.expects(:delay).never
-          post_ids = SavedSearch.post_ids_for(1, label: "blah")
-          assert_equal([1,2,3], post_ids)
-        end
-      end
-    end
-
-    context "without a label" do
-      setup do
-        SavedSearch.expects(:queries_for).with(1, label: nil).returns(%w(a b c))
-      end
-
-      context "without a primed cache" do
-        should "delay processing three times" do
-          SavedSearch.expects(:populate).times(3)
-          post_ids = SavedSearch.post_ids_for(1)
+        should "return nothing" do
+          post_ids = SavedSearch.post_ids_for(@user.id, label: "blah")
           assert_equal([], post_ids)
         end
       end
 
       context "with a primed cache" do
         setup do
-          @mock_redis.sadd("search:a", 1)
-          @mock_redis.sadd("search:b", 2)
-          @mock_redis.sadd("search:c", 3)
+          perform_enqueued_jobs do
+            SavedSearch.post_ids_for(@user.id, label: "blah")
+          end
         end
 
         should "fetch the post ids" do
-          SavedSearch.expects(:delay).never
-          post_ids = SavedSearch.post_ids_for(1)
-          assert_equal([1,2,3], post_ids)
+          post_ids = SavedSearch.post_ids_for(@user.id, label: "blah")
+          assert_equal(Post.pluck(:id).sort, post_ids.sort)
+        end
+      end
+    end
+
+    context "without a label" do
+      setup do
+        create(:saved_search, query: "a", user: @user)
+        create(:saved_search, query: "b", user: @user)
+        create(:saved_search, query: "c", user: @user)
+
+        create(:post, tag_string: "a")
+        create(:post, tag_string: "b")
+        create(:post, tag_string: "c")
+      end
+
+      context "without a primed cache" do
+        should "return nothing" do
+          post_ids = SavedSearch.post_ids_for(@user.id)
+          assert_equal([], post_ids)
+        end
+      end
+
+      context "with a primed cache" do
+        setup do
+          perform_enqueued_jobs do
+            SavedSearch.post_ids_for(@user.id)
+          end
+        end
+
+        should "fetch the post ids" do
+          post_ids = SavedSearch.post_ids_for(@user.id)
+          assert_equal(Post.pluck(:id).sort, post_ids.sort)
         end
       end
     end