From 06947c91471fda6e774c5b6fc04720b87db2b1e6 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 3 May 2019 22:15:47 +0300 Subject: [PATCH 1/7] Remove bookmarks assoc --- lib/pleroma/user.ex | 2 -- .../mastodon_api/mastodon_api_controller.ex | 24 +------------------ 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index fd2ce81ad..b1adaad2f 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -10,7 +10,6 @@ defmodule Pleroma.User do alias Comeonin.Pbkdf2 alias Pleroma.Activity - alias Pleroma.Bookmark alias Pleroma.Notification alias Pleroma.Object alias Pleroma.Registration @@ -54,7 +53,6 @@ defmodule Pleroma.User do field(:search_type, :integer, virtual: true) field(:tags, {:array, :string}, default: []) field(:last_refreshed_at, :naive_datetime_usec) - has_many(:bookmarks, Bookmark) has_many(:notifications, Notification) has_many(:registrations, Registration) embeds_one(:info, Pleroma.User.Info) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index be60e5e3c..2a3d58592 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -295,8 +295,6 @@ def home_timeline(%{assigns: %{user: user}} = conn, params) do |> ActivityPub.contain_timeline(user) |> Enum.reverse() - user = Repo.preload(user, bookmarks: :activity) - conn |> add_link_headers(:home_timeline, activities) |> put_view(StatusView) @@ -315,8 +313,6 @@ def public_timeline(%{assigns: %{user: user}} = conn, params) do |> ActivityPub.fetch_public_activities() |> Enum.reverse() - user = Repo.preload(user, bookmarks: :activity) - conn |> add_link_headers(:public_timeline, activities, false, %{"local" => local_only}) |> put_view(StatusView) @@ -324,8 +320,7 @@ def public_timeline(%{assigns: %{user: user}} = conn, params) do end def user_statuses(%{assigns: %{user: reading_user}} = conn, params) do - with %User{} = user <- User.get_cached_by_id(params["id"]), - reading_user <- Repo.preload(reading_user, :bookmarks) do + with %User{} = user <- User.get_cached_by_id(params["id"]) do activities = ActivityPub.fetch_user_activities(user, reading_user, params) conn @@ -352,8 +347,6 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do |> ActivityPub.fetch_activities_query(params) |> Pagination.fetch_paginated(params) - user = Repo.preload(user, bookmarks: :activity) - conn |> add_link_headers(:dm_timeline, activities) |> put_view(StatusView) @@ -363,8 +356,6 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do def get_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do with %Activity{} = activity <- Activity.get_by_id_with_object(id), true <- Visibility.visible_for_user?(activity, user) do - user = Repo.preload(user, bookmarks: :activity) - conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user}) @@ -514,8 +505,6 @@ def delete_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do def reblog_status(%{assigns: %{user: user}} = conn, %{"id" => ap_id_or_id}) do with {:ok, announce, _activity} <- CommonAPI.repeat(ap_id_or_id, user), %Activity{} = announce <- Activity.normalize(announce.data) do - user = Repo.preload(user, bookmarks: :activity) - conn |> put_view(StatusView) |> try_render("status.json", %{activity: announce, for: user, as: :activity}) @@ -525,8 +514,6 @@ def reblog_status(%{assigns: %{user: user}} = conn, %{"id" => ap_id_or_id}) do def unreblog_status(%{assigns: %{user: user}} = conn, %{"id" => ap_id_or_id}) do with {:ok, _unannounce, %{data: %{"id" => id}}} <- CommonAPI.unrepeat(ap_id_or_id, user), %Activity{} = activity <- Activity.get_create_by_object_ap_id_with_object(id) do - user = Repo.preload(user, bookmarks: :activity) - conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) @@ -577,8 +564,6 @@ def bookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), {:ok, _bookmark} <- Bookmark.create(user.id, activity.id) do - user = Repo.preload(user, bookmarks: :activity) - conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) @@ -590,8 +575,6 @@ def unbookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), {:ok, _bookmark} <- Bookmark.destroy(user.id, activity.id) do - user = Repo.preload(user, bookmarks: :activity) - conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) @@ -1112,8 +1095,6 @@ def favourites(%{assigns: %{user: user}} = conn, params) do ActivityPub.fetch_activities([], params) |> Enum.reverse() - user = Repo.preload(user, bookmarks: :activity) - conn |> add_link_headers(:favourites, activities) |> put_view(StatusView) @@ -1159,7 +1140,6 @@ def user_favourites(%{assigns: %{user: for_user}} = conn, %{"id" => id} = params def bookmarks(%{assigns: %{user: user}} = conn, params) do user = User.get_cached_by_id(user.id) - user = Repo.preload(user, bookmarks: :activity) bookmarks = Bookmark.for_user_query(user.id) @@ -1276,8 +1256,6 @@ def list_timeline(%{assigns: %{user: user}} = conn, %{"list_id" => id} = params) |> ActivityPub.fetch_activities_bounded(following, params) |> Enum.reverse() - user = Repo.preload(user, bookmarks: :activity) - conn |> put_view(StatusView) |> render("index.json", %{activities: activities, for: user, as: :activity}) From f841eb7cdb83afc444dfe260581b6be6e690a717 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 4 May 2019 12:46:42 +0300 Subject: [PATCH 2/7] Preload bookmarks wherever the object is preloaded --- lib/pleroma/activity.ex | 12 ++++++-- lib/pleroma/web/activity_pub/activity_pub.ex | 7 +++++ test/activity_test.exs | 29 ++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 73e63bb14..7845c264a 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -6,6 +6,7 @@ defmodule Pleroma.Activity do use Ecto.Schema alias Pleroma.Activity + alias Pleroma.Bookmark alias Pleroma.Notification alias Pleroma.Object alias Pleroma.Repo @@ -36,6 +37,7 @@ defmodule Pleroma.Activity do field(:actor, :string) field(:recipients, {:array, :string}, default: []) has_many(:notifications, Notification, on_delete: :delete_all) + has_many(:bookmarks, Bookmark, on_delete: :delete_all) # Attention: this is a fake relation, don't try to preload it blindly and expect it to work! # The foreign key is embedded in a jsonb field. @@ -71,6 +73,7 @@ def with_preloaded_object(query) do ) ) |> preload([activity, object], object: object) + |> preload(:bookmarks) end def get_by_ap_id(ap_id) do @@ -102,7 +105,8 @@ def get_by_ap_id_with_object(ap_id) do activity.data, activity.data ), - preload: [object: o] + preload: [object: o], + preload: :bookmarks ) ) end @@ -122,7 +126,8 @@ def get_by_id_with_object(id) do activity.data, activity.data ), - preload: [object: o] + preload: [object: o], + preload: :bookmarks ) |> Repo.one() end @@ -200,7 +205,8 @@ def create_by_object_ap_id_with_object(ap_id) when is_binary(ap_id) do activity.data, activity.data ), - preload: [object: o] + preload: [object: o], + preload: :bookmarks ) end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 6c737d0a4..bd2544470 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -137,6 +137,13 @@ def insert(map, local \\ true, fake \\ false) when is_map(map) do activity end + activity = + if activity.data["type"] in ["Create", "Announce"] do + Repo.preload(activity, :bookmarks) + else + activity + end + Task.start(fn -> Pleroma.Web.RichMedia.Helpers.fetch_data_for_activity(activity) end) diff --git a/test/activity_test.exs b/test/activity_test.exs index ad889f544..e2a8baada 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -5,6 +5,8 @@ defmodule Pleroma.ActivityTest do use Pleroma.DataCase alias Pleroma.Activity + alias Pleroma.Bookmark + alias Pleroma.Object import Pleroma.Factory test "returns an activity by it's AP id" do @@ -28,4 +30,31 @@ test "returns the activity that created an object" do assert activity == found_activity end + + test "preloading object preloads bookmarks" do + user1 = insert(:user) + user2 = insert(:user) + activity = insert(:note_activity) + {:ok, bookmark1} = Bookmark.create(user1.id, activity.id) + {:ok, bookmark2} = Bookmark.create(user2.id, activity.id) + bookmarks = Enum.sort([bookmark1, bookmark2]) + + queried_activity = + Ecto.Query.from(a in Activity, where: a.id == ^activity.id) + |> Activity.with_preloaded_object() + |> Repo.one() + + assert Enum.sort(queried_activity.bookmarks) == bookmarks + + queried_activity = Activity.get_by_ap_id_with_object(activity.data["id"]) + assert Enum.sort(queried_activity.bookmarks) == bookmarks + + queried_activity = Activity.get_by_id_with_object(activity.id) + assert Enum.sort(queried_activity.bookmarks) == bookmarks + + queried_activity = + Activity.get_create_by_object_ap_id_with_object(Object.normalize(activity).data["id"]) + + assert Enum.sort(queried_activity.bookmarks) == bookmarks + end end From 3a7c14645ed726bd6b7deb6489ec0578c4d8cd79 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 4 May 2019 13:42:54 +0300 Subject: [PATCH 3/7] - Actually use preloaded bookmarks in views - Preload bookmarks in bookmark timeline - Rework bookmark preload tests --- lib/pleroma/activity.ex | 13 ++-- lib/pleroma/bookmark.ex | 3 +- .../mastodon_api/mastodon_api_controller.ex | 9 ++- .../web/mastodon_api/views/status_view.ex | 18 +++++- test/activity_test.exs | 60 +++++++++++++------ test/web/mastodon_api/status_view_test.exs | 2 + 6 files changed, 77 insertions(+), 28 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 7845c264a..e432fcb07 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -73,6 +73,11 @@ def with_preloaded_object(query) do ) ) |> preload([activity, object], object: object) + |> with_preloaded_bookmarks() + end + + def with_preloaded_bookmarks(query) do + query |> preload(:bookmarks) end @@ -105,9 +110,9 @@ def get_by_ap_id_with_object(ap_id) do activity.data, activity.data ), - preload: [object: o], - preload: :bookmarks + preload: [object: o] ) + |> with_preloaded_bookmarks() ) end @@ -126,9 +131,9 @@ def get_by_id_with_object(id) do activity.data, activity.data ), - preload: [object: o], - preload: :bookmarks + preload: [object: o] ) + |> with_preloaded_bookmarks() |> Repo.one() end diff --git a/lib/pleroma/bookmark.ex b/lib/pleroma/bookmark.ex index 7f8fd43b6..bbb51c22c 100644 --- a/lib/pleroma/bookmark.ex +++ b/lib/pleroma/bookmark.ex @@ -38,7 +38,8 @@ def for_user_query(user_id) do Bookmark |> where(user_id: ^user_id) |> join(:inner, [b], activity in assoc(b, :activity)) - |> preload([b, a], activity: a) + |> join(:inner, [_b, a], bookmarks in assoc(a, :bookmarks)) + |> preload([b, a, b2], activity: {a, bookmarks: b2}) end def get(user_id, activity_id) do diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 2a3d58592..451fc85ce 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -563,7 +563,9 @@ def bookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do with %Activity{} = activity <- Activity.get_by_id_with_object(id), %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), - {:ok, _bookmark} <- Bookmark.create(user.id, activity.id) do + {:ok, bookmark} <- Bookmark.create(user.id, activity.id) do + activity = %{activity | bookmarks: [bookmark | activity.bookmarks]} + conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) @@ -575,6 +577,11 @@ def unbookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), {:ok, _bookmark} <- Bookmark.destroy(user.id, activity.id) do + bookmarks = + Enum.filter(activity.bookmarks, fn %{user_id: user_id} -> user_id != user.id end) + + activity = %{activity | bookmarks: bookmarks} + conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 62d064d71..c5a7bcbab 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -80,13 +80,21 @@ def render( user = get_user(activity.data["actor"]) created_at = Utils.to_masto_date(activity.data["published"]) - reblogged_activity = Activity.get_create_by_object_ap_id(object) + reblogged_activity = + Activity.create_by_object_ap_id(object) + |> Activity.with_preloaded_bookmarks() + |> Repo.one() + reblogged = render("status.json", Map.put(opts, :activity, reblogged_activity)) activity_object = Object.normalize(activity) favorited = opts[:for] && opts[:for].ap_id in (activity_object.data["likes"] || []) - bookmarked = opts[:for] && CommonAPI.bookmarked?(opts[:for], reblogged_activity) + bookmarked = + opts[:for] && Ecto.assoc_loaded?(reblogged_activity.bookmarks) && + Enum.any?(reblogged_activity.bookmarks, fn %{user_id: user_id} -> + user_id == opts[:for].id + end) mentions = activity.recipients @@ -149,7 +157,11 @@ def render("status.json", %{activity: %{data: %{"object" => _object}} = activity favorited = opts[:for] && opts[:for].ap_id in (object.data["likes"] || []) - bookmarked = opts[:for] && CommonAPI.bookmarked?(opts[:for], activity) + bookmarked = + opts[:for] && Ecto.assoc_loaded?(activity.bookmarks) && + Enum.any?(activity.bookmarks, fn %{user_id: user_id} -> + user_id == opts[:for].id + end) attachment_data = object.data["attachment"] || [] attachments = render_many(attachment_data, StatusView, "attachment.json", as: :attachment) diff --git a/test/activity_test.exs b/test/activity_test.exs index e2a8baada..dd149543c 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -31,30 +31,52 @@ test "returns the activity that created an object" do assert activity == found_activity end - test "preloading object preloads bookmarks" do - user1 = insert(:user) - user2 = insert(:user) - activity = insert(:note_activity) - {:ok, bookmark1} = Bookmark.create(user1.id, activity.id) - {:ok, bookmark2} = Bookmark.create(user2.id, activity.id) - bookmarks = Enum.sort([bookmark1, bookmark2]) + describe "preloading bookmarks" do + setup do + user1 = insert(:user) + user2 = insert(:user) + activity = insert(:note_activity) + {:ok, bookmark1} = Bookmark.create(user1.id, activity.id) + {:ok, bookmark2} = Bookmark.create(user2.id, activity.id) + [activity: activity, bookmarks: Enum.sort([bookmark1, bookmark2])] + end - queried_activity = - Ecto.Query.from(a in Activity, where: a.id == ^activity.id) - |> Activity.with_preloaded_object() - |> Repo.one() + test "using with_preloaded_bookmarks", %{activity: activity, bookmarks: bookmarks} do + queried_activity = + Ecto.Query.from(a in Activity, where: a.id == ^activity.id) + |> Activity.with_preloaded_bookmarks() + |> Repo.one() - assert Enum.sort(queried_activity.bookmarks) == bookmarks + assert Enum.sort(queried_activity.bookmarks) == bookmarks + end - queried_activity = Activity.get_by_ap_id_with_object(activity.data["id"]) - assert Enum.sort(queried_activity.bookmarks) == bookmarks + test "using with_preloaded_object", %{activity: activity, bookmarks: bookmarks} do + queried_activity = + Ecto.Query.from(a in Activity, where: a.id == ^activity.id) + |> Activity.with_preloaded_object() + |> Repo.one() - queried_activity = Activity.get_by_id_with_object(activity.id) - assert Enum.sort(queried_activity.bookmarks) == bookmarks + assert Enum.sort(queried_activity.bookmarks) == bookmarks + end - queried_activity = - Activity.get_create_by_object_ap_id_with_object(Object.normalize(activity).data["id"]) + test "using get_by_ap_id_with_object", %{activity: activity, bookmarks: bookmarks} do + queried_activity = Activity.get_by_ap_id_with_object(activity.data["id"]) + assert Enum.sort(queried_activity.bookmarks) == bookmarks + end - assert Enum.sort(queried_activity.bookmarks) == bookmarks + test "using get_by_id_with_object", %{activity: activity, bookmarks: bookmarks} do + queried_activity = Activity.get_by_id_with_object(activity.id) + assert Enum.sort(queried_activity.bookmarks) == bookmarks + end + + test "using get_create_by_object_ap_id_with_object", %{ + activity: activity, + bookmarks: bookmarks + } do + queried_activity = + Activity.get_create_by_object_ap_id_with_object(Object.normalize(activity).data["id"]) + + assert Enum.sort(queried_activity.bookmarks) == bookmarks + end end end diff --git a/test/web/mastodon_api/status_view_test.exs b/test/web/mastodon_api/status_view_test.exs index 5fddc6c58..d7c800e83 100644 --- a/test/web/mastodon_api/status_view_test.exs +++ b/test/web/mastodon_api/status_view_test.exs @@ -168,6 +168,8 @@ test "tells if the status is bookmarked" do {:ok, _bookmark} = Bookmark.create(user.id, activity.id) + activity = Activity.get_by_id_with_object(activity.id) + status = StatusView.render("status.json", %{activity: activity, for: user}) assert status.bookmarked == true From be067ec2ab7d3ebcbad7b1306ab19b266dcda664 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 4 May 2019 13:50:39 +0300 Subject: [PATCH 4/7] Use with_preloaded_bookmark in create_by_object_ap_id_with_object --- lib/pleroma/activity.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index e432fcb07..ab41364dc 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -210,9 +210,9 @@ def create_by_object_ap_id_with_object(ap_id) when is_binary(ap_id) do activity.data, activity.data ), - preload: [object: o], - preload: :bookmarks + preload: [object: o] ) + |> with_preloaded_bookmarks() end def create_by_object_ap_id_with_object(_), do: nil From 4c5125dedc429ac021c861c11eb2e41c856ae4e8 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 7 May 2019 18:00:50 +0300 Subject: [PATCH 5/7] Remove `bookmarks` assoc and add a fake `bookmark` assoc instead --- lib/pleroma/activity.ex | 29 +++++--- lib/pleroma/bookmark.ex | 3 +- lib/pleroma/web/activity_pub/activity_pub.ex | 15 ++-- .../mastodon_api/mastodon_api_controller.ex | 11 +-- .../web/mastodon_api/views/status_view.ex | 24 +++---- test/activity_test.exs | 68 +++++++++---------- 6 files changed, 71 insertions(+), 79 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index ab41364dc..2b661edc1 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -10,6 +10,7 @@ defmodule Pleroma.Activity do alias Pleroma.Notification alias Pleroma.Object alias Pleroma.Repo + alias Pleroma.User import Ecto.Changeset import Ecto.Query @@ -36,8 +37,9 @@ defmodule Pleroma.Activity do field(:local, :boolean, default: true) field(:actor, :string) field(:recipients, {:array, :string}, default: []) + # This is a fake relation, do not use outside of with_preloaded_bookmark/get_bookmark + has_one(:bookmark, Bookmark) has_many(:notifications, Notification, on_delete: :delete_all) - has_many(:bookmarks, Bookmark, on_delete: :delete_all) # Attention: this is a fake relation, don't try to preload it blindly and expect it to work! # The foreign key is embedded in a jsonb field. @@ -73,14 +75,18 @@ def with_preloaded_object(query) do ) ) |> preload([activity, object], object: object) - |> with_preloaded_bookmarks() end - def with_preloaded_bookmarks(query) do - query - |> preload(:bookmarks) + def with_preloaded_bookmark(query, %User{} = user) do + from([a] in query, + left_join: b in Bookmark, + on: b.user_id == ^user.id and b.activity_id == a.id, + preload: [bookmark: b] + ) end + def with_preloaded_bookmark(query, _), do: query + def get_by_ap_id(ap_id) do Repo.one( from( @@ -90,6 +96,16 @@ def get_by_ap_id(ap_id) do ) end + def get_bookmark(%Activity{} = activity, %User{} = user) do + if Ecto.assoc_loaded?(activity.bookmark) do + activity.bookmark + else + Bookmark.get(user.id, activity.id) + end + end + + def get_bookmark(_, _), do: nil + def change(struct, params \\ %{}) do struct |> cast(params, [:data]) @@ -112,7 +128,6 @@ def get_by_ap_id_with_object(ap_id) do ), preload: [object: o] ) - |> with_preloaded_bookmarks() ) end @@ -133,7 +148,6 @@ def get_by_id_with_object(id) do ), preload: [object: o] ) - |> with_preloaded_bookmarks() |> Repo.one() end @@ -212,7 +226,6 @@ def create_by_object_ap_id_with_object(ap_id) when is_binary(ap_id) do ), preload: [object: o] ) - |> with_preloaded_bookmarks() end def create_by_object_ap_id_with_object(_), do: nil diff --git a/lib/pleroma/bookmark.ex b/lib/pleroma/bookmark.ex index bbb51c22c..7f8fd43b6 100644 --- a/lib/pleroma/bookmark.ex +++ b/lib/pleroma/bookmark.ex @@ -38,8 +38,7 @@ def for_user_query(user_id) do Bookmark |> where(user_id: ^user_id) |> join(:inner, [b], activity in assoc(b, :activity)) - |> join(:inner, [_b, a], bookmarks in assoc(a, :bookmarks)) - |> preload([b, a, b2], activity: {a, bookmarks: b2}) + |> preload([b, a], activity: a) end def get(user_id, activity_id) do diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index bd2544470..a4053986f 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -137,13 +137,6 @@ def insert(map, local \\ true, fake \\ false) when is_map(map) do activity end - activity = - if activity.data["type"] in ["Create", "Announce"] do - Repo.preload(activity, :bookmarks) - else - activity - end - Task.start(fn -> Pleroma.Web.RichMedia.Helpers.fetch_data_for_activity(activity) end) @@ -822,11 +815,19 @@ defp maybe_preload_objects(query, _) do |> Activity.with_preloaded_object() end + defp maybe_preload_bookmarks(query, %{"skip_preload" => true}), do: query + + defp maybe_preload_bookmarks(query, opts) do + query + |> Activity.with_preloaded_bookmark(opts["user"]) + end + def fetch_activities_query(recipients, opts \\ %{}) do base_query = from(activity in Activity) base_query |> maybe_preload_objects(opts) + |> maybe_preload_bookmarks(opts) |> restrict_recipients(recipients, opts["user"]) |> restrict_tag(opts) |> restrict_tag_reject(opts) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 451fc85ce..83ad90989 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -563,9 +563,7 @@ def bookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do with %Activity{} = activity <- Activity.get_by_id_with_object(id), %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), - {:ok, bookmark} <- Bookmark.create(user.id, activity.id) do - activity = %{activity | bookmarks: [bookmark | activity.bookmarks]} - + {:ok, _bookmark} <- Bookmark.create(user.id, activity.id) do conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) @@ -577,11 +575,6 @@ def unbookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), {:ok, _bookmark} <- Bookmark.destroy(user.id, activity.id) do - bookmarks = - Enum.filter(activity.bookmarks, fn %{user_id: user_id} -> user_id != user.id end) - - activity = %{activity | bookmarks: bookmarks} - conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) @@ -1154,7 +1147,7 @@ def bookmarks(%{assigns: %{user: user}} = conn, params) do activities = bookmarks - |> Enum.map(fn b -> b.activity end) + |> Enum.map(fn b -> Map.put(b.activity, :bookmark, Map.delete(b, :activity)) end) conn |> add_link_headers(:bookmarks, bookmarks) diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index c5a7bcbab..bd2372944 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -75,26 +75,22 @@ def render("index.json", opts) do def render( "status.json", - %{activity: %{data: %{"type" => "Announce", "object" => object}} = activity} = opts + %{activity: %{data: %{"type" => "Announce", "object" => _object}} = activity} = opts ) do user = get_user(activity.data["actor"]) created_at = Utils.to_masto_date(activity.data["published"]) + activity_object = Object.normalize(activity) reblogged_activity = - Activity.create_by_object_ap_id(object) - |> Activity.with_preloaded_bookmarks() + Activity.create_by_object_ap_id(activity_object.data["id"]) + |> Activity.with_preloaded_bookmark(opts[:for]) |> Repo.one() reblogged = render("status.json", Map.put(opts, :activity, reblogged_activity)) - activity_object = Object.normalize(activity) favorited = opts[:for] && opts[:for].ap_id in (activity_object.data["likes"] || []) - bookmarked = - opts[:for] && Ecto.assoc_loaded?(reblogged_activity.bookmarks) && - Enum.any?(reblogged_activity.bookmarks, fn %{user_id: user_id} -> - user_id == opts[:for].id - end) + bookmarked = Activity.get_bookmark(reblogged_activity, opts[:for]) != nil mentions = activity.recipients @@ -104,8 +100,8 @@ def render( %{ id: to_string(activity.id), - uri: object, - url: object, + uri: activity_object.data["id"], + url: activity_object.data["id"], account: AccountView.render("account.json", %{user: user}), in_reply_to_id: nil, in_reply_to_account_id: nil, @@ -157,11 +153,7 @@ def render("status.json", %{activity: %{data: %{"object" => _object}} = activity favorited = opts[:for] && opts[:for].ap_id in (object.data["likes"] || []) - bookmarked = - opts[:for] && Ecto.assoc_loaded?(activity.bookmarks) && - Enum.any?(activity.bookmarks, fn %{user_id: user_id} -> - user_id == opts[:for].id - end) + bookmarked = Activity.get_bookmark(activity, opts[:for]) != nil attachment_data = object.data["attachment"] || [] attachments = render_many(attachment_data, StatusView, "attachment.json", as: :attachment) diff --git a/test/activity_test.exs b/test/activity_test.exs index dd149543c..7e91d534b 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -6,7 +6,6 @@ defmodule Pleroma.ActivityTest do use Pleroma.DataCase alias Pleroma.Activity alias Pleroma.Bookmark - alias Pleroma.Object import Pleroma.Factory test "returns an activity by it's AP id" do @@ -31,52 +30,47 @@ test "returns the activity that created an object" do assert activity == found_activity end - describe "preloading bookmarks" do - setup do - user1 = insert(:user) - user2 = insert(:user) + test "preloading a bookmark" do + user = insert(:user) + user2 = insert(:user) + user3 = insert(:user) + activity = insert(:note_activity) + {:ok, _bookmark} = Bookmark.create(user.id, activity.id) + {:ok, _bookmark2} = Bookmark.create(user2.id, activity.id) + {:ok, bookmark3} = Bookmark.create(user3.id, activity.id) + + queried_activity = + Ecto.Query.from(Pleroma.Activity) + |> Activity.with_preloaded_bookmark(user3) + |> Repo.one() + + assert queried_activity.bookmark == bookmark3 + end + + describe "getting a bookmark" do + test "when association is loaded" do + user = insert(:user) activity = insert(:note_activity) - {:ok, bookmark1} = Bookmark.create(user1.id, activity.id) - {:ok, bookmark2} = Bookmark.create(user2.id, activity.id) - [activity: activity, bookmarks: Enum.sort([bookmark1, bookmark2])] - end + {:ok, bookmark} = Bookmark.create(user.id, activity.id) - test "using with_preloaded_bookmarks", %{activity: activity, bookmarks: bookmarks} do queried_activity = - Ecto.Query.from(a in Activity, where: a.id == ^activity.id) - |> Activity.with_preloaded_bookmarks() + Ecto.Query.from(Pleroma.Activity) + |> Activity.with_preloaded_bookmark(user) |> Repo.one() - assert Enum.sort(queried_activity.bookmarks) == bookmarks + assert Activity.get_bookmark(queried_activity, user) == bookmark end - test "using with_preloaded_object", %{activity: activity, bookmarks: bookmarks} do + test "when association is not loaded" do + user = insert(:user) + activity = insert(:note_activity) + {:ok, bookmark} = Bookmark.create(user.id, activity.id) + queried_activity = - Ecto.Query.from(a in Activity, where: a.id == ^activity.id) - |> Activity.with_preloaded_object() + Ecto.Query.from(Pleroma.Activity) |> Repo.one() - assert Enum.sort(queried_activity.bookmarks) == bookmarks - end - - test "using get_by_ap_id_with_object", %{activity: activity, bookmarks: bookmarks} do - queried_activity = Activity.get_by_ap_id_with_object(activity.data["id"]) - assert Enum.sort(queried_activity.bookmarks) == bookmarks - end - - test "using get_by_id_with_object", %{activity: activity, bookmarks: bookmarks} do - queried_activity = Activity.get_by_id_with_object(activity.id) - assert Enum.sort(queried_activity.bookmarks) == bookmarks - end - - test "using get_create_by_object_ap_id_with_object", %{ - activity: activity, - bookmarks: bookmarks - } do - queried_activity = - Activity.get_create_by_object_ap_id_with_object(Object.normalize(activity).data["id"]) - - assert Enum.sort(queried_activity.bookmarks) == bookmarks + assert Activity.get_bookmark(queried_activity, user) == bookmark end end end From 6020ff3fb696c6b46e948d69a8687182c701a743 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Tue, 7 May 2019 19:30:27 +0000 Subject: [PATCH 6/7] activitypub: add optional order constraint to timeline query builder --- lib/pleroma/web/activity_pub/activity_pub.ex | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index a4053986f..8f8c23a9b 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -822,12 +822,25 @@ defp maybe_preload_bookmarks(query, opts) do |> Activity.with_preloaded_bookmark(opts["user"]) end + defp maybe_order(query, %{order: :desc}) do + query + |> order_by(desc: :id) + end + + defp maybe_order(query, %{order: :asc}) do + query + |> order_by(asc: :id) + end + + defp maybe_order(query, _), do: query + def fetch_activities_query(recipients, opts \\ %{}) do base_query = from(activity in Activity) base_query |> maybe_preload_objects(opts) |> maybe_preload_bookmarks(opts) + |> maybe_order(opts) |> restrict_recipients(recipients, opts["user"]) |> restrict_tag(opts) |> restrict_tag_reject(opts) From d64c3b604e4eb5e8dd3f51aff7d1f15024d5fa33 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Tue, 7 May 2019 19:30:51 +0000 Subject: [PATCH 7/7] twitterapi: use order constraint to force descending order --- lib/pleroma/web/twitter_api/twitter_api_controller.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index ef7b6fe65..21e6c555a 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -182,6 +182,7 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do |> Map.put("blocking_user", user) |> Map.put("user", user) |> Map.put(:visibility, "direct") + |> Map.put(:order, :desc) activities = ActivityPub.fetch_activities_query([user.ap_id], params)