From 460e41585c2cd3f137c0f80173da60167fb318bf Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 25 Mar 2020 20:33:34 +0300 Subject: [PATCH] Further preloading (more endpoints), refactoring, tests. --- lib/pleroma/following_relationship.ex | 6 + lib/pleroma/user.ex | 5 +- lib/pleroma/user_relationship.ex | 20 ++++ .../web/mastodon_api/views/account_view.ex | 36 +++--- .../mastodon_api/views/notification_view.ex | 42 ++++--- .../web/mastodon_api/views/status_view.ex | 29 ++--- .../mastodon_api/views/account_view_test.exs | 109 ++++++++++-------- .../views/notification_view_test.exs | 42 +++---- .../mastodon_api/views/status_view_test.exs | 15 ++- 9 files changed, 179 insertions(+), 125 deletions(-) diff --git a/lib/pleroma/following_relationship.ex b/lib/pleroma/following_relationship.ex index 624bddfe4..a9538ea4e 100644 --- a/lib/pleroma/following_relationship.ex +++ b/lib/pleroma/following_relationship.ex @@ -151,4 +151,10 @@ def all_between_user_sets( ) |> Repo.all() end + + def find(following_relationships, follower, following) do + Enum.find(following_relationships, fn + fr -> fr.follower_id == follower.id and fr.following_id == following.id + end) + end end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 699256a3b..8ccb9242d 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -218,7 +218,10 @@ def unquote(:"#{outgoing_relation_target}_ap_ids")(user, restrict_deactivated? \ end end - @doc "Dumps id to SQL-compatible format" + @doc """ + Dumps Flake Id to SQL-compatible format (16-byte UUID). + E.g. "9pQtDGXuq4p3VlcJEm" -> <<0, 0, 1, 110, 179, 218, 42, 92, 213, 41, 44, 227, 95, 213, 0, 0>> + """ def binary_id(source_id) when is_binary(source_id) do with {:ok, dumped_id} <- FlakeId.Ecto.CompatType.dump(source_id) do dumped_id diff --git a/lib/pleroma/user_relationship.ex b/lib/pleroma/user_relationship.ex index 519d2998d..011cf6822 100644 --- a/lib/pleroma/user_relationship.ex +++ b/lib/pleroma/user_relationship.ex @@ -8,6 +8,7 @@ defmodule Pleroma.UserRelationship do import Ecto.Changeset import Ecto.Query + alias Pleroma.FollowingRelationship alias Pleroma.Repo alias Pleroma.User alias Pleroma.UserRelationship @@ -124,6 +125,25 @@ def exists?(dictionary, rel_type, source, target, func) do end end + @doc ":relationships option for StatusView / AccountView / NotificationView" + def view_relationships_option(nil = _reading_user, _actors) do + %{user_relationships: [], following_relationships: []} + end + + def view_relationships_option(%User{} = reading_user, actors) do + user_relationships = + UserRelationship.dictionary( + [reading_user], + actors, + [:block, :mute, :notification_mute, :reblog_mute], + [:block, :inverse_subscription] + ) + + following_relationships = FollowingRelationship.all_between_user_sets([reading_user], actors) + + %{user_relationships: user_relationships, following_relationships: following_relationships} + end + defp validate_not_self_relationship(%Ecto.Changeset{} = changeset) do changeset |> validate_change(:target_id, fn _, target_id -> diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index 6b2eca1f3..2cdfac7af 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -5,20 +5,23 @@ defmodule Pleroma.Web.MastodonAPI.AccountView do use Pleroma.Web, :view + alias Pleroma.FollowingRelationship alias Pleroma.User alias Pleroma.UserRelationship alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.MastodonAPI.AccountView - alias Pleroma.Web.MastodonAPI.StatusView alias Pleroma.Web.MediaProxy - defp find_following_rel(following_relationships, follower, following) do - Enum.find(following_relationships, fn - fr -> fr.follower_id == follower.id and fr.following_id == following.id - end) - end - def render("index.json", %{users: users} = opts) do + relationships_opt = + if Map.has_key?(opts, :relationships) do + opts[:relationships] + else + UserRelationship.view_relationships_option(opts[:for], users) + end + + opts = Map.put(opts, :relationships, relationships_opt) + users |> render_many(AccountView, "show.json", opts) |> Enum.filter(&Enum.any?/1) @@ -53,7 +56,7 @@ def render( follow_state = if following_relationships do user_to_target_following_relation = - find_following_rel(following_relationships, reading_user, target) + FollowingRelationship.find(following_relationships, reading_user, target) User.get_follow_state(reading_user, target, user_to_target_following_relation) else @@ -62,7 +65,7 @@ def render( followed_by = if following_relationships do - case find_following_rel(following_relationships, target, reading_user) do + case FollowingRelationship.find(following_relationships, target, reading_user) do %{state: "accept"} -> true _ -> false end @@ -70,7 +73,7 @@ def render( User.following?(target, reading_user) end - # NOTE: adjust StatusView.relationships_opts/2 if adding new relation-related flags + # NOTE: adjust UserRelationship.view_relationships_option/2 on new relation-related flags %{ id: to_string(target.id), following: follow_state == "accept", @@ -129,11 +132,16 @@ def render( } end - def render("relationships.json", %{user: user, targets: targets}) do - relationships_opts = StatusView.relationships_opts(user, targets) - opts = %{as: :target, user: user, relationships: relationships_opts} + def render("relationships.json", %{user: user, targets: targets} = opts) do + relationships_opt = + if Map.has_key?(opts, :relationships) do + opts[:relationships] + else + UserRelationship.view_relationships_option(user, targets) + end - render_many(targets, AccountView, "relationship.json", opts) + render_opts = %{as: :target, user: user, relationships: relationships_opt} + render_many(targets, AccountView, "relationship.json", render_opts) end defp do_render("show.json", %{user: user} = opts) do diff --git a/lib/pleroma/web/mastodon_api/views/notification_view.ex b/lib/pleroma/web/mastodon_api/views/notification_view.ex index e9c618496..db434271c 100644 --- a/lib/pleroma/web/mastodon_api/views/notification_view.ex +++ b/lib/pleroma/web/mastodon_api/views/notification_view.ex @@ -8,12 +8,13 @@ defmodule Pleroma.Web.MastodonAPI.NotificationView do alias Pleroma.Activity alias Pleroma.Notification alias Pleroma.User + alias Pleroma.UserRelationship alias Pleroma.Web.CommonAPI alias Pleroma.Web.MastodonAPI.AccountView alias Pleroma.Web.MastodonAPI.NotificationView alias Pleroma.Web.MastodonAPI.StatusView - def render("index.json", %{notifications: notifications, for: reading_user}) do + def render("index.json", %{notifications: notifications, for: reading_user} = opts) do activities = Enum.map(notifications, & &1.activity) parent_activities = @@ -30,21 +31,28 @@ def render("index.json", %{notifications: notifications, for: reading_user}) do |> Activity.with_preloaded_object(:left) |> Pleroma.Repo.all() - move_activities_targets = - activities - |> Enum.filter(&(Activity.mastodon_notification_type(&1) == "move")) - |> Enum.map(&User.get_cached_by_ap_id(&1.data["target"])) + relationships_opt = + if Map.has_key?(opts, :relationships) do + opts[:relationships] + else + move_activities_targets = + activities + |> Enum.filter(&(Activity.mastodon_notification_type(&1) == "move")) + |> Enum.map(&User.get_cached_by_ap_id(&1.data["target"])) - actors = - activities - |> Enum.map(fn a -> User.get_cached_by_ap_id(a.data["actor"]) end) - |> Enum.filter(& &1) - |> Kernel.++(move_activities_targets) + actors = + activities + |> Enum.map(fn a -> User.get_cached_by_ap_id(a.data["actor"]) end) + |> Enum.filter(& &1) + |> Kernel.++(move_activities_targets) + + UserRelationship.view_relationships_option(reading_user, actors) + end opts = %{ for: reading_user, parent_activities: parent_activities, - relationships: StatusView.relationships_opts(reading_user, actors) + relationships: relationships_opt } safe_render_many(notifications, NotificationView, "show.json", opts) @@ -85,27 +93,27 @@ def render( } } - relationships_opts = %{relationships: opts[:relationships]} + relationships_opt = %{relationships: opts[:relationships]} case mastodon_type do "mention" -> - put_status(response, activity, reading_user, relationships_opts) + put_status(response, activity, reading_user, relationships_opt) "favourite" -> - put_status(response, parent_activity_fn.(), reading_user, relationships_opts) + put_status(response, parent_activity_fn.(), reading_user, relationships_opt) "reblog" -> - put_status(response, parent_activity_fn.(), reading_user, relationships_opts) + put_status(response, parent_activity_fn.(), reading_user, relationships_opt) "move" -> - put_target(response, activity, reading_user, relationships_opts) + put_target(response, activity, reading_user, relationships_opt) "follow" -> response "pleroma:emoji_reaction" -> response - |> put_status(parent_activity_fn.(), reading_user, relationships_opts) + |> put_status(parent_activity_fn.(), reading_user, relationships_opt) |> put_emoji(activity) _ -> diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 0ef65b352..7b1cb7bf8 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -9,7 +9,6 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do alias Pleroma.Activity alias Pleroma.ActivityExpiration - alias Pleroma.FollowingRelationship alias Pleroma.HTML alias Pleroma.Object alias Pleroma.Repo @@ -72,24 +71,6 @@ defp reblogged?(activity, user) do present?(user && user.ap_id in (object.data["announcements"] || [])) end - def relationships_opts(_reading_user = nil, _actors) do - %{user_relationships: [], following_relationships: []} - end - - def relationships_opts(reading_user, actors) do - user_relationships = - UserRelationship.dictionary( - [reading_user], - actors, - [:block, :mute, :notification_mute, :reblog_mute], - [:block, :inverse_subscription] - ) - - following_relationships = FollowingRelationship.all_between_user_sets([reading_user], actors) - - %{user_relationships: user_relationships, following_relationships: following_relationships} - end - def render("index.json", opts) do # To do: check AdminAPIControllerTest on the reasons behind nil activities in the list activities = Enum.filter(opts.activities, & &1) @@ -105,13 +86,19 @@ def render("index.json", opts) do |> Activity.with_set_thread_muted_field(opts[:for]) |> Repo.all() - actors = Enum.map(activities ++ parent_activities, &get_user(&1.data["actor"])) + relationships_opt = + if Map.has_key?(opts, :relationships) do + opts[:relationships] + else + actors = Enum.map(activities ++ parent_activities, &get_user(&1.data["actor"])) + UserRelationship.view_relationships_option(opts[:for], actors) + end opts = opts |> Map.put(:replied_to_activities, replied_to_activities) |> Map.put(:parent_activities, parent_activities) - |> Map.put(:relationships, relationships_opts(opts[:for], actors)) + |> Map.put(:relationships, relationships_opt) safe_render_many(activities, StatusView, "show.json", opts) end diff --git a/test/web/mastodon_api/views/account_view_test.exs b/test/web/mastodon_api/views/account_view_test.exs index 983886c6b..ede62903f 100644 --- a/test/web/mastodon_api/views/account_view_test.exs +++ b/test/web/mastodon_api/views/account_view_test.exs @@ -4,8 +4,11 @@ defmodule Pleroma.Web.MastodonAPI.AccountViewTest do use Pleroma.DataCase + import Pleroma.Factory + alias Pleroma.User + alias Pleroma.UserRelationship alias Pleroma.Web.CommonAPI alias Pleroma.Web.MastodonAPI.AccountView @@ -182,6 +185,29 @@ test "Represent a smaller mention" do end describe "relationship" do + defp test_relationship_rendering(user, other_user, expected_result) do + opts = %{user: user, target: other_user} + assert expected_result == AccountView.render("relationship.json", opts) + + relationships_opt = UserRelationship.view_relationships_option(user, [other_user]) + opts = Map.put(opts, :relationships, relationships_opt) + assert expected_result == AccountView.render("relationship.json", opts) + end + + @blank_response %{ + following: false, + followed_by: false, + blocking: false, + blocked_by: false, + muting: false, + muting_notifications: false, + subscribing: false, + requested: false, + domain_blocking: false, + showing_reblogs: true, + endorsed: false + } + test "represent a relationship for the following and followed user" do user = insert(:user) other_user = insert(:user) @@ -192,23 +218,21 @@ test "represent a relationship for the following and followed user" do {:ok, _user_relationships} = User.mute(user, other_user, true) {:ok, _reblog_mute} = CommonAPI.hide_reblogs(user, other_user) - expected = %{ - id: to_string(other_user.id), - following: true, - followed_by: true, - blocking: false, - blocked_by: false, - muting: true, - muting_notifications: true, - subscribing: true, - requested: false, - domain_blocking: false, - showing_reblogs: false, - endorsed: false - } + expected = + Map.merge( + @blank_response, + %{ + following: true, + followed_by: true, + muting: true, + muting_notifications: true, + subscribing: true, + showing_reblogs: false, + id: to_string(other_user.id) + } + ) - assert expected == - AccountView.render("relationship.json", %{user: user, target: other_user}) + test_relationship_rendering(user, other_user, expected) end test "represent a relationship for the blocking and blocked user" do @@ -220,23 +244,13 @@ test "represent a relationship for the blocking and blocked user" do {:ok, _user_relationship} = User.block(user, other_user) {:ok, _user_relationship} = User.block(other_user, user) - expected = %{ - id: to_string(other_user.id), - following: false, - followed_by: false, - blocking: true, - blocked_by: true, - muting: false, - muting_notifications: false, - subscribing: false, - requested: false, - domain_blocking: false, - showing_reblogs: true, - endorsed: false - } + expected = + Map.merge( + @blank_response, + %{following: false, blocking: true, blocked_by: true, id: to_string(other_user.id)} + ) - assert expected == - AccountView.render("relationship.json", %{user: user, target: other_user}) + test_relationship_rendering(user, other_user, expected) end test "represent a relationship for the user blocking a domain" do @@ -245,8 +259,13 @@ test "represent a relationship for the user blocking a domain" do {:ok, user} = User.block_domain(user, "bad.site") - assert %{domain_blocking: true, blocking: false} = - AccountView.render("relationship.json", %{user: user, target: other_user}) + expected = + Map.merge( + @blank_response, + %{domain_blocking: true, blocking: false, id: to_string(other_user.id)} + ) + + test_relationship_rendering(user, other_user, expected) end test "represent a relationship for the user with a pending follow request" do @@ -257,23 +276,13 @@ test "represent a relationship for the user with a pending follow request" do user = User.get_cached_by_id(user.id) other_user = User.get_cached_by_id(other_user.id) - expected = %{ - id: to_string(other_user.id), - following: false, - followed_by: false, - blocking: false, - blocked_by: false, - muting: false, - muting_notifications: false, - subscribing: false, - requested: true, - domain_blocking: false, - showing_reblogs: true, - endorsed: false - } + expected = + Map.merge( + @blank_response, + %{requested: true, following: false, id: to_string(other_user.id)} + ) - assert expected == - AccountView.render("relationship.json", %{user: user, target: other_user}) + test_relationship_rendering(user, other_user, expected) end end diff --git a/test/web/mastodon_api/views/notification_view_test.exs b/test/web/mastodon_api/views/notification_view_test.exs index d04c3022f..7965af00a 100644 --- a/test/web/mastodon_api/views/notification_view_test.exs +++ b/test/web/mastodon_api/views/notification_view_test.exs @@ -16,6 +16,21 @@ defmodule Pleroma.Web.MastodonAPI.NotificationViewTest do alias Pleroma.Web.MastodonAPI.StatusView import Pleroma.Factory + defp test_notifications_rendering(notifications, user, expected_result) do + result = NotificationView.render("index.json", %{notifications: notifications, for: user}) + + assert expected_result == result + + result = + NotificationView.render("index.json", %{ + notifications: notifications, + for: user, + relationships: nil + }) + + assert expected_result == result + end + test "Mention notification" do user = insert(:user) mentioned_user = insert(:user) @@ -32,10 +47,7 @@ test "Mention notification" do created_at: Utils.to_masto_date(notification.inserted_at) } - result = - NotificationView.render("index.json", %{notifications: [notification], for: mentioned_user}) - - assert [expected] == result + test_notifications_rendering([notification], mentioned_user, [expected]) end test "Favourite notification" do @@ -55,9 +67,7 @@ test "Favourite notification" do created_at: Utils.to_masto_date(notification.inserted_at) } - result = NotificationView.render("index.json", %{notifications: [notification], for: user}) - - assert [expected] == result + test_notifications_rendering([notification], user, [expected]) end test "Reblog notification" do @@ -77,9 +87,7 @@ test "Reblog notification" do created_at: Utils.to_masto_date(notification.inserted_at) } - result = NotificationView.render("index.json", %{notifications: [notification], for: user}) - - assert [expected] == result + test_notifications_rendering([notification], user, [expected]) end test "Follow notification" do @@ -96,16 +104,12 @@ test "Follow notification" do created_at: Utils.to_masto_date(notification.inserted_at) } - result = - NotificationView.render("index.json", %{notifications: [notification], for: followed}) - - assert [expected] == result + test_notifications_rendering([notification], followed, [expected]) User.perform(:delete, follower) notification = Notification |> Repo.one() |> Repo.preload(:activity) - assert [] == - NotificationView.render("index.json", %{notifications: [notification], for: followed}) + test_notifications_rendering([notification], followed, []) end test "Move notification" do @@ -131,8 +135,7 @@ test "Move notification" do created_at: Utils.to_masto_date(notification.inserted_at) } - assert [expected] == - NotificationView.render("index.json", %{notifications: [notification], for: follower}) + test_notifications_rendering([notification], follower, [expected]) end test "EmojiReact notification" do @@ -158,7 +161,6 @@ test "EmojiReact notification" do created_at: Utils.to_masto_date(notification.inserted_at) } - assert expected == - NotificationView.render("show.json", %{notification: notification, for: user}) + test_notifications_rendering([notification], user, [expected]) end end diff --git a/test/web/mastodon_api/views/status_view_test.exs b/test/web/mastodon_api/views/status_view_test.exs index 191895c6f..9191730cd 100644 --- a/test/web/mastodon_api/views/status_view_test.exs +++ b/test/web/mastodon_api/views/status_view_test.exs @@ -12,10 +12,12 @@ defmodule Pleroma.Web.MastodonAPI.StatusViewTest do alias Pleroma.Object alias Pleroma.Repo alias Pleroma.User + alias Pleroma.UserRelationship alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.MastodonAPI.AccountView alias Pleroma.Web.MastodonAPI.StatusView + import Pleroma.Factory import Tesla.Mock @@ -212,12 +214,21 @@ test "tells if the message is muted for some reason" do {:ok, _user_relationships} = User.mute(user, other_user) {:ok, activity} = CommonAPI.post(other_user, %{"status" => "test"}) - status = StatusView.render("show.json", %{activity: activity}) + relationships_opt = UserRelationship.view_relationships_option(user, [other_user]) + + opts = %{activity: activity} + status = StatusView.render("show.json", opts) assert status.muted == false - status = StatusView.render("show.json", %{activity: activity, for: user}) + status = StatusView.render("show.json", Map.put(opts, :relationships, relationships_opt)) + assert status.muted == false + for_opts = %{activity: activity, for: user} + status = StatusView.render("show.json", for_opts) + assert status.muted == true + + status = StatusView.render("show.json", Map.put(for_opts, :relationships, relationships_opt)) assert status.muted == true end