- Actually use preloaded bookmarks in views

- Preload bookmarks in bookmark timeline
- Rework bookmark preload tests

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
 

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

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})

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)

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
+
+    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
+    end
 
-    queried_activity =
-      Ecto.Query.from(a in Activity, where: a.id == ^activity.id)
-      |> Activity.with_preloaded_object()
-      |> Repo.one()
+    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()
 
-    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 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
 
-    queried_activity = Activity.get_by_id_with_object(activity.id)
-    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
 
-    queried_activity =
-      Activity.get_create_by_object_ap_id_with_object(Object.normalize(activity).data["id"])
+    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 Enum.sort(queried_activity.bookmarks) == bookmarks
+    end
   end
 end

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