Remove `bookmarks` assoc and add a fake `bookmark` assoc instead

This commit is contained in:
rinpatch 2019-05-07 18:00:50 +03:00 committed by William Pitcock
parent be067ec2ab
commit 4c5125dedc
6 changed files with 71 additions and 79 deletions

View File

@ -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

View File

@ -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

View File

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

View File

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

View File

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

View File

@ -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