From 8cbcec285db069e66b1558dd8e785ffa30e83e8d Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Mon, 27 Apr 2020 22:29:42 -0500
Subject: [PATCH] search: fix multiple metatag searches not working in some
 cases.

Bug: in some cases searching for multiple metatags would cause one
metatag to be ignored. For example, a search for {{user:1 pool:2}} would
be treated as a search for {{pool:2}}.

Cause: we used `ActiveRecord::Relation#merge` to combine two relations,
which was wrong because `merge` doesn't combine `column IN (?)` clauses
correctly. If there are two `column IN (?)` clauses on the same column,
then `#merge` takes only the second clause and ignores the first.

Fix: write our own half-baked `#and` method to work around Rails'
broken-by-design `#merge` method.

ref: https://github.com/rails/rails/issues/33501.
---
 app/logical/concerns/searchable.rb   |  9 +++++++++
 app/logical/post_query_builder.rb    |  2 +-
 test/unit/post_query_builder_test.rb | 18 +++++++++++-------
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/app/logical/concerns/searchable.rb b/app/logical/concerns/searchable.rb
index 59e92b4cb..6cd151723 100644
--- a/app/logical/concerns/searchable.rb
+++ b/app/logical/concerns/searchable.rb
@@ -5,6 +5,15 @@ module Searchable
     unscoped.where(all.where_clause.invert(kind).ast)
   end
 
+  # XXX hacky method to AND two relations together.
+  def and(relation)
+    q = all
+    q = q.where(relation.where_clause.ast) if relation.where_clause.present?
+    q = q.joins(relation.joins_values + q.joins_values) if relation.joins_values.present?
+    q = q.order(relation.order_values) if relation.order_values.present?
+    q
+  end
+
   # `operator` is an Arel::Predications method: :eq, :gt, :lt, :between, :in, etc.
   # https://github.com/rails/rails/blob/master/activerecord/lib/arel/predications.rb
   def where_operator(field, operator, *args)
diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb
index 8d1c2504b..1a2e34ab3 100644
--- a/app/logical/post_query_builder.rb
+++ b/app/logical/post_query_builder.rb
@@ -117,7 +117,7 @@ class PostQueryBuilder
 
   def metatags_match(metatags, relation)
     metatags.each do |metatag|
-      relation = relation.merge(metatag_matches(metatag.name, metatag.value, quoted: metatag.quoted))
+      relation = relation.and(metatag_matches(metatag.name, metatag.value, quoted: metatag.quoted))
     end
 
     relation
diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb
index 316e244f0..8922c8706 100644
--- a/test/unit/post_query_builder_test.rb
+++ b/test/unit/post_query_builder_test.rb
@@ -595,6 +595,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
       assert_tag_match([post1], "md5:abcd")
       assert_tag_match([post1], "md5:ABCD")
       assert_tag_match([post1], "md5:123,abcd")
+      assert_tag_match([], "md5:abcd md5:xyz")
     end
 
     should "return posts for a source:<text> search" do
@@ -613,6 +614,8 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
       assert_tag_match([post2, post1], "-source:none")
 
       assert_tag_match([], "source:'none'")
+      assert_tag_match([], "source:none source:abcde")
+      assert_tag_match([], "source:abcde source:xzy")
     end
 
     should "return posts for a pixiv source search" do
@@ -693,6 +696,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
       assert_tag_match([s], "rating:s")
       assert_tag_match([q], "rating:q")
       assert_tag_match([e], "rating:e")
+      assert_tag_match([], "rating:s rating:q")
 
       assert_tag_match(all - [s], "-rating:s")
       assert_tag_match(all - [q], "-rating:q")
@@ -726,8 +730,10 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
         upvoted   = create(:post, tag_string: "upvote:self")
         downvoted = create(:post, tag_string: "downvote:self")
 
-        assert_tag_match([upvoted],   "upvote:#{CurrentUser.name}")
+        assert_tag_match([upvoted], "upvote:#{CurrentUser.name}")
         assert_tag_match([downvoted], "downvote:#{CurrentUser.name}")
+        assert_tag_match([], "upvote:nobody upvote:#{CurrentUser.name}")
+        assert_tag_match([], "downvote:nobody downvote:#{CurrentUser.name}")
       end
     end
 
@@ -742,6 +748,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
         assert_tag_match([disapproved], "disapproved:disinterest")
         assert_tag_match([disapproved], "disapproved:DISINTEREST")
         assert_tag_match([], "disapproved:breaks_rules")
+        assert_tag_match([], "disapproved:breaks_rules disapproved:disinterest")
 
         assert_tag_match([pending], "-disapproved:#{CurrentUser.name}")
         assert_tag_match([pending], "-disapproved:disinterest")
@@ -887,16 +894,13 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
 
     should "succeed for exclusive tag searches with no other tag" do
       post1 = create(:post, rating: "s", tag_string: "aaa")
-      assert_nothing_raised do
-        relation = Post.tag_match("-aaa")
-      end
+      assert_tag_match([], "-aaa")
     end
 
     should "succeed for exclusive tag searches combined with a metatag" do
       post1 = create(:post, rating: "s", tag_string: "aaa")
-      assert_nothing_raised do
-        relation = Post.tag_match("-aaa id:>0")
-      end
+      assert_tag_match([], "-aaa id:>0")
+      assert_tag_match([], "-a* rating:s")
     end
   end