[#3213] Fixed `hashtags.name` lookup (must use `citext` type to do index scan). Fixed...
authorIvan Tashkinov <ivantashkinov@gmail.com>
Mon, 15 Feb 2021 18:13:14 +0000 (21:13 +0300)
committerIvan Tashkinov <ivantashkinov@gmail.com>
Mon, 15 Feb 2021 18:13:14 +0000 (21:13 +0300)
lib/pleroma/hashtag.ex
lib/pleroma/web/activity_pub/activity_pub.ex
test/pleroma/web/activity_pub/activity_pub_test.exs

index 0d6a4d09e8caa5f8bd16cd5116ce7bd165b1854e..a6d033816191498af1a47085f0e063458cbf3ae4 100644 (file)
@@ -22,7 +22,9 @@ defmodule Pleroma.Hashtag do
   end
 
   def get_by_name(name) do
-    Repo.get_by(Hashtag, name: name)
+    from(h in Hashtag)
+    |> where([h], fragment("name = ?::citext", ^String.downcase(name)))
+    |> Repo.one()
   end
 
   def get_or_create_by_name(name) when is_bitstring(name) do
@@ -37,6 +39,7 @@ defmodule Pleroma.Hashtag do
   end
 
   def get_or_create_by_names(names) when is_list(names) do
+    names = Enum.map(names, &String.downcase/1)
     timestamp = NaiveDateTime.truncate(NaiveDateTime.utc_now(), :second)
 
     structs =
@@ -52,7 +55,8 @@ defmodule Pleroma.Hashtag do
              Multi.new()
              |> Multi.insert_all(:insert_all_op, Hashtag, structs, on_conflict: :nothing)
              |> Multi.run(:query_op, fn _repo, _changes ->
-               {:ok, Repo.all(from(ht in Hashtag, where: ht.name in ^names))}
+               {:ok,
+                Repo.all(from(ht in Hashtag, where: ht.name in fragment("?::citext[]", ^names)))}
              end)
              |> Repo.transaction() do
         {:ok, hashtags}
index 9623e635a7e32a98354b4189adbf48637bec10f9..e012f2779e9be2820d609f8276c6c004d0b86d2a 100644 (file)
@@ -698,6 +698,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
   end
 
   defp restrict_embedded_tag_all(query, %{tag_all: [_ | _] = tag_all}) do
+    tag_all = Enum.map(tag_all, &String.downcase/1)
+
     from(
       [_activity, object] in query,
       where: fragment("(?)->'tag' \\?& (?)", object.data, ^tag_all)
@@ -714,10 +716,12 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     raise_on_missing_preload()
   end
 
-  defp restrict_embedded_tag_any(query, %{tag: [_ | _] = tag}) do
+  defp restrict_embedded_tag_any(query, %{tag: [_ | _] = tag_any}) do
+    tag_any = Enum.map(tag_any, &String.downcase/1)
+
     from(
       [_activity, object] in query,
-      where: fragment("(?)->'tag' \\?| (?)", object.data, ^tag)
+      where: fragment("(?)->'tag' \\?| (?)", object.data, ^tag_any)
     )
   end
 
@@ -732,6 +736,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
   end
 
   defp restrict_embedded_tag_reject_any(query, %{tag_reject: [_ | _] = tag_reject}) do
+    tag_reject = Enum.map(tag_reject, &String.downcase/1)
+
     from(
       [_activity, object] in query,
       where: fragment("not (?)->'tag' \\?| (?)", object.data, ^tag_reject)
@@ -749,6 +755,10 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     raise_on_missing_preload()
   end
 
+  defp restrict_hashtag_all(query, %{tag_all: [single_tag]}) do
+    restrict_hashtag_any(query, %{tag: single_tag})
+  end
+
   defp restrict_hashtag_all(query, %{tag_all: [_ | _] = tags}) do
     from(
       [_activity, object] in query,
@@ -756,7 +766,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
         fragment(
           """
           (SELECT array_agg(hashtags.name) FROM hashtags JOIN hashtags_objects
-            ON hashtags_objects.hashtag_id = hashtags.id WHERE hashtags.name = ANY(?)
+            ON hashtags_objects.hashtag_id = hashtags.id WHERE hashtags.name = ANY(?::citext[])
               AND hashtags_objects.object_id = ?) @> ?
           """,
           ^tags,
@@ -767,7 +777,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
   end
 
   defp restrict_hashtag_all(query, %{tag_all: tag}) when is_binary(tag) do
-    restrict_hashtag_any(query, %{tag: tag})
+    restrict_hashtag_all(query, %{tag_all: [tag]})
   end
 
   defp restrict_hashtag_all(query, _), do: query
@@ -783,7 +793,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
         fragment(
           """
           EXISTS (SELECT 1 FROM hashtags JOIN hashtags_objects
-            ON hashtags_objects.hashtag_id = hashtags.id WHERE hashtags.name = ANY(?)
+            ON hashtags_objects.hashtag_id = hashtags.id WHERE hashtags.name = ANY(?::citext[])
               AND hashtags_objects.object_id = ? LIMIT 1)
           """,
           ^tags,
@@ -809,7 +819,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
         fragment(
           """
           NOT EXISTS (SELECT 1 FROM hashtags JOIN hashtags_objects
-            ON hashtags_objects.hashtag_id = hashtags.id WHERE hashtags.name = ANY(?)
+            ON hashtags_objects.hashtag_id = hashtags.id WHERE hashtags.name = ANY(?::citext[])
               AND hashtags_objects.object_id = ? LIMIT 1)
           """,
           ^tags_reject,
index bab5a199cb2927f988b9fb04c3ec206a5128ce70..c41c8a5dd910c0799cd1bdf0cbe090e02742d89e 100644 (file)
@@ -213,24 +213,24 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
   test "it fetches the appropriate tag-restricted posts" do
     user = insert(:user)
 
-    {:ok, status_one} = CommonAPI.post(user, %{status: ". #test"})
+    {:ok, status_one} = CommonAPI.post(user, %{status: ". #TEST"})
     {:ok, status_two} = CommonAPI.post(user, %{status: ". #essais"})
-    {:ok, status_three} = CommonAPI.post(user, %{status: ". #test #reject"})
+    {:ok, status_three} = CommonAPI.post(user, %{status: ". #test #Reject"})
 
-    {:ok, status_four} = CommonAPI.post(user, %{status: ". #any1 #any2"})
-    {:ok, status_five} = CommonAPI.post(user, %{status: ". #any2 #any1"})
+    {:ok, status_four} = CommonAPI.post(user, %{status: ". #Any1 #any2"})
+    {:ok, status_five} = CommonAPI.post(user, %{status: ". #Any2 #any1"})
 
     for hashtag_timeline_strategy <- [true, false] do
       clear_config([:database, :improved_hashtag_timeline], hashtag_timeline_strategy)
 
       fetch_one = ActivityPub.fetch_activities([], %{type: "Create", tag: "test"})
 
-      fetch_two = ActivityPub.fetch_activities([], %{type: "Create", tag: ["test", "essais"]})
+      fetch_two = ActivityPub.fetch_activities([], %{type: "Create", tag: ["TEST", "essais"]})
 
       fetch_three =
         ActivityPub.fetch_activities([], %{
           type: "Create",
-          tag: ["test", "essais"],
+          tag: ["test", "Essais"],
           tag_reject: ["reject"]
         })
 
@@ -238,21 +238,21 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
         ActivityPub.fetch_activities([], %{
           type: "Create",
           tag: ["test"],
-          tag_all: ["test", "reject"]
+          tag_all: ["test", "REJECT"]
         })
 
       # Testing that deduplication (if needed) is done on DB (not Ecto) level; :limit is important
       fetch_five =
         ActivityPub.fetch_activities([], %{
           type: "Create",
-          tag: ["any1", "any2"],
+          tag: ["ANY1", "any2"],
           limit: 2
         })
 
       fetch_six =
         ActivityPub.fetch_activities([], %{
           type: "Create",
-          tag: ["any1", "any2"],
+          tag: ["any1", "Any2"],
           tag_all: [],
           tag_reject: []
         })