From 3687788cf2ab91f6f40f76f9a82c448c477b1fec Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 27 May 2020 13:45:14 +0000 Subject: [PATCH] Merge branch 'notification-fixes' into 'develop' Notification performance fixes See merge request pleroma/pleroma!2595 --- lib/pleroma/notification.ex | 17 ++++------------- ...527104138_change_notification_user_index.exs | 8 ++++++++ test/notification_test.exs | 11 +++++++---- 3 files changed, 19 insertions(+), 17 deletions(-) create mode 100644 priv/repo/migrations/20200527104138_change_notification_user_index.exs diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 556075fba..8c6887a6b 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -70,8 +70,9 @@ def for_user_query(user, opts \\ %{}) 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 @@ def for_user_since(user, date) 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 @@ def set_read_up_to(%{id: user_id} = _user, id) 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 index 000000000..4dcfe6de9 --- /dev/null +++ b/priv/repo/migrations/20200527104138_change_notification_user_index.exs @@ -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 diff --git a/test/notification_test.exs b/test/notification_test.exs index d04754a9d..80fa52312 100644 --- a/test/notification_test.exs +++ b/test/notification_test.exs @@ -446,8 +446,7 @@ test "it sets all notifications as read up to a specified notification ID" 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 @@ test "it sets all notifications as read up to a specified notification ID" 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 @@ test "it returns notifications for muted user without notifications" 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