Merge branch 'notification-fixes' into 'develop'
authorrinpatch <rinpatch@sdf.org>
Wed, 27 May 2020 13:45:14 +0000 (13:45 +0000)
committerrinpatch <rinpatch@sdf.org>
Sun, 7 Jun 2020 21:58:31 +0000 (00:58 +0300)
Notification performance fixes

See merge request pleroma/pleroma!2595

lib/pleroma/notification.ex
priv/repo/migrations/20200527104138_change_notification_user_index.exs [new file with mode: 0644]
test/notification_test.exs

index 556075fbab791ee7a34541f2bbbb76c83d3e0d54..8c6887a6bba4df3d58ad462547d30d92ded52d37 100644 (file)
@@ -70,8 +70,9 @@ defmodule Pleroma.Notification do
     |> join(:left, [n, a], object in Object,
       on:
         fragment(
-          "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)",
+          "(?->>'id') = COALESCE(?->'object'->>'id', ?->>'object')",
           object.data,
+          a.data,
           a.data
         )
     )
@@ -195,7 +196,7 @@ defmodule Pleroma.Notification do
     |> Repo.all()
   end
 
-  def set_read_up_to(%{id: user_id} = _user, id) do
+  def set_read_up_to(%{id: user_id} = user, id) do
     query =
       from(
         n in Notification,
@@ -215,18 +216,8 @@ defmodule Pleroma.Notification do
 
     {_, notification_ids} = Repo.update_all(query, [])
 
-    Notification
+    for_user_query(user)
     |> where([n], n.id in ^notification_ids)
-    |> join(:inner, [n], activity in assoc(n, :activity))
-    |> join(:left, [n, a], object in Object,
-      on:
-        fragment(
-          "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)",
-          object.data,
-          a.data
-        )
-    )
-    |> preload([n, a, o], activity: {a, object: o})
     |> Repo.all()
   end
 
diff --git a/priv/repo/migrations/20200527104138_change_notification_user_index.exs b/priv/repo/migrations/20200527104138_change_notification_user_index.exs
new file mode 100644 (file)
index 0000000..4dcfe6d
--- /dev/null
@@ -0,0 +1,8 @@
+defmodule Pleroma.Repo.Migrations.ChangeNotificationUserIndex do
+  use Ecto.Migration
+
+  def change do
+    drop_if_exists(index(:notifications, [:user_id]))
+    create_if_not_exists(index(:notifications, [:user_id, "id desc nulls last"]))
+  end
+end
index d04754a9d007ce6ee9103e8ce46bae8027c79306..80fa5231214d903acbed66d0c1f4d8f15e497981 100644 (file)
@@ -446,8 +446,7 @@ defmodule Pleroma.NotificationTest do
           "status" => "hey again @#{other_user.nickname}!"
         })
 
-      [n2, n1] = notifs = Notification.for_user(other_user)
-      assert length(notifs) == 2
+      [n2, n1] = Notification.for_user(other_user)
 
       assert n2.id > n1.id
 
@@ -456,7 +455,9 @@ defmodule Pleroma.NotificationTest do
           "status" => "hey yet again @#{other_user.nickname}!"
         })
 
-      Notification.set_read_up_to(other_user, n2.id)
+      [_, read_notification] = Notification.set_read_up_to(other_user, n2.id)
+
+      assert read_notification.activity.object
 
       [n3, n2, n1] = Notification.for_user(other_user)
 
@@ -885,7 +886,9 @@ defmodule Pleroma.NotificationTest do
 
       {:ok, _activity} = CommonAPI.post(muted, %{"status" => "hey @#{user.nickname}"})
 
-      assert length(Notification.for_user(user)) == 1
+      [notification] = Notification.for_user(user)
+
+      assert notification.activity.object
     end
 
     test "it doesn't return notifications for muted user with notifications" do