From 863ec33ba2a90708d199f18683ffe0c4658c710a Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Wed, 11 Mar 2020 12:21:44 +0100 Subject: [PATCH 01/23] Add support for funkwhale Audio activity reel2bits fixture not included as it lacks the Actor fixture for it. Closes: https://git.pleroma.social/pleroma/pleroma/issues/1624 Closes: https://git.pleroma.social/pleroma/pleroma/issues/764 --- .../web/activity_pub/transmogrifier.ex | 5 ++- .../web/mastodon_api/views/status_view.ex | 2 +- test/fixtures/tesla_mock/funkwhale_audio.json | 44 +++++++++++++++++++ .../tesla_mock/funkwhale_channel.json | 44 +++++++++++++++++++ test/support/http_request_mock.ex | 15 +++++++ .../mastodon_api/views/status_view_test.exs | 16 +++++++ test/web/oauth/oauth_controller_test.exs | 2 +- 7 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/tesla_mock/funkwhale_audio.json create mode 100644 test/fixtures/tesla_mock/funkwhale_channel.json diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 9cd3de705..f52b065f6 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -229,7 +229,8 @@ def fix_url(%{"url" => url} = object) when is_map(url) do Map.put(object, "url", url["href"]) end - def fix_url(%{"type" => "Video", "url" => url} = object) when is_list(url) do + def fix_url(%{"type" => object_type, "url" => url} = object) + when object_type in ["Video", "Audio"] and is_list(url) do first_element = Enum.at(url, 0) link_element = Enum.find(url, fn x -> is_map(x) and x["mimeType"] == "text/html" end) @@ -398,7 +399,7 @@ def handle_incoming( %{"type" => "Create", "object" => %{"type" => objtype} = object} = data, options ) - when objtype in ["Article", "Event", "Note", "Video", "Page", "Question", "Answer"] do + when objtype in ["Article", "Event", "Note", "Video", "Page", "Question", "Answer", "Audio"] do actor = Containment.get_actor(data) data = diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index f7469cdff..a042075f5 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -421,7 +421,7 @@ def get_reply_to(%{data: %{"object" => _object}} = activity, _) do end def render_content(%{data: %{"type" => object_type}} = object) - when object_type in ["Video", "Event"] do + when object_type in ["Video", "Event", "Audio"] do with name when not is_nil(name) and name != "" <- object.data["name"] do "

#{name}

#{object.data["content"]}" else diff --git a/test/fixtures/tesla_mock/funkwhale_audio.json b/test/fixtures/tesla_mock/funkwhale_audio.json new file mode 100644 index 000000000..15736b1f8 --- /dev/null +++ b/test/fixtures/tesla_mock/funkwhale_audio.json @@ -0,0 +1,44 @@ +{ + "id": "https://channels.tests.funkwhale.audio/federation/music/uploads/42342395-0208-4fee-a38d-259a6dae0871", + "type": "Audio", + "name": "Compositions - Test Audio for Pleroma", + "attributedTo": "https://channels.tests.funkwhale.audio/federation/actors/compositions", + "published": "2020-03-11T10:01:52.714918+00:00", + "to": "https://www.w3.org/ns/activitystreams#Public", + "url": [ + { + "type": "Link", + "mimeType": "audio/ogg", + "href": "https://channels.tests.funkwhale.audio/api/v1/listen/3901e5d8-0445-49d5-9711-e096cf32e515/?upload=42342395-0208-4fee-a38d-259a6dae0871&download=false" + }, + { + "type": "Link", + "mimeType": "text/html", + "href": "https://channels.tests.funkwhale.audio/library/tracks/74" + } + ], + "content": "

This is a test Audio for Pleroma.

", + "mediaType": "text/html", + "tag": [ + { + "type": "Hashtag", + "name": "#funkwhale" + }, + { + "type": "Hashtag", + "name": "#test" + }, + { + "type": "Hashtag", + "name": "#tests" + } + ], + "summary": "#funkwhale #test #tests", + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + { + "manuallyApprovesFollowers": "as:manuallyApprovesFollowers" + } + ] +} diff --git a/test/fixtures/tesla_mock/funkwhale_channel.json b/test/fixtures/tesla_mock/funkwhale_channel.json new file mode 100644 index 000000000..cf9ee8151 --- /dev/null +++ b/test/fixtures/tesla_mock/funkwhale_channel.json @@ -0,0 +1,44 @@ +{ + "id": "https://channels.tests.funkwhale.audio/federation/actors/compositions", + "outbox": "https://channels.tests.funkwhale.audio/federation/actors/compositions/outbox", + "inbox": "https://channels.tests.funkwhale.audio/federation/actors/compositions/inbox", + "preferredUsername": "compositions", + "type": "Person", + "name": "Compositions", + "followers": "https://channels.tests.funkwhale.audio/federation/actors/compositions/followers", + "following": "https://channels.tests.funkwhale.audio/federation/actors/compositions/following", + "manuallyApprovesFollowers": false, + "url": [ + { + "type": "Link", + "href": "https://channels.tests.funkwhale.audio/channels/compositions", + "mediaType": "text/html" + }, + { + "type": "Link", + "href": "https://channels.tests.funkwhale.audio/api/v1/channels/compositions/rss", + "mediaType": "application/rss+xml" + } + ], + "icon": { + "type": "Image", + "url": "https://channels.tests.funkwhale.audio/media/attachments/75/b4/f1/nosmile.jpeg", + "mediaType": "image/jpeg" + }, + "summary": "

I'm testing federation with the fediverse :)

", + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + { + "manuallyApprovesFollowers": "as:manuallyApprovesFollowers" + } + ], + "publicKey": { + "owner": "https://channels.tests.funkwhale.audio/federation/actors/compositions", + "publicKeyPem": "-----BEGIN RSA PUBLIC KEY-----\nMIIBCgKCAQEAv25u57oZfVLV3KltS+HcsdSx9Op4MmzIes1J8Wu8s0KbdXf2zEwS\nsVqyHgs/XCbnzsR3FqyJTo46D2BVnvZcuU5srNcR2I2HMaqQ0oVdnATE4K6KdcgV\nN+98pMWo56B8LTgE1VpvqbsrXLi9jCTzjrkebVMOP+ZVu+64v1qdgddseblYMnBZ\nct0s7ONbHnqrWlTGf5wES1uIZTVdn5r4MduZG+Uenfi1opBS0lUUxfWdW9r0oF2b\nyneZUyaUCbEroeKbqsweXCWVgnMarUOsgqC42KM4cf95lySSwTSaUtZYIbTw7s9W\n2jveU/rVg8BYZu5JK5obgBoxtlUeUoSswwIDAQAB\n-----END RSA PUBLIC KEY-----\n", + "id": "https://channels.tests.funkwhale.audio/federation/actors/compositions#main-key" + }, + "endpoints": { + "sharedInbox": "https://channels.tests.funkwhale.audio/federation/shared/inbox" + } +} diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index d46887865..0079d8c44 100644 --- a/test/support/http_request_mock.ex +++ b/test/support/http_request_mock.ex @@ -1273,6 +1273,21 @@ def get("https://patch.cx/users/rin", _, _, _) do {:ok, %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/rin.json")}} end + def get( + "https://channels.tests.funkwhale.audio/federation/music/uploads/42342395-0208-4fee-a38d-259a6dae0871", + _, + _, + _ + ) do + {:ok, + %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/funkwhale_audio.json")}} + end + + def get("https://channels.tests.funkwhale.audio/federation/actors/compositions", _, _, _) do + {:ok, + %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/funkwhale_channel.json")}} + end + def get("http://example.com/rel_me/error", _, _, _) do {:ok, %Tesla.Env{status: 404, body: ""}} 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..3e1812a1f 100644 --- a/test/web/mastodon_api/views/status_view_test.exs +++ b/test/web/mastodon_api/views/status_view_test.exs @@ -420,6 +420,22 @@ test "a peertube video" do assert length(represented[:media_attachments]) == 1 end + test "funkwhale audio" do + user = insert(:user) + + {:ok, object} = + Pleroma.Object.Fetcher.fetch_object_from_id( + "https://channels.tests.funkwhale.audio/federation/music/uploads/42342395-0208-4fee-a38d-259a6dae0871" + ) + + %Activity{} = activity = Activity.get_create_by_object_ap_id(object.data["id"]) + + represented = StatusView.render("show.json", %{for: user, activity: activity}) + + assert represented[:id] == to_string(activity.id) + assert length(represented[:media_attachments]) == 1 + end + test "a Mobilizon event" do user = insert(:user) diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs index cff469c28..5f86d999c 100644 --- a/test/web/oauth/oauth_controller_test.exs +++ b/test/web/oauth/oauth_controller_test.exs @@ -581,7 +581,7 @@ test "redirects with oauth authorization, " <> # In case scope param is missing, expecting _all_ app-supported scopes to be granted for user <- [non_admin, admin], {requested_scopes, expected_scopes} <- - %{scopes_subset => scopes_subset, nil => app_scopes} do + %{scopes_subset => scopes_subset, nil: app_scopes} do conn = post( build_conn(), From 7c8003c3fcdcab075b9722ab236bf2d1d0e0e8cd Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 15 Mar 2020 21:00:12 +0300 Subject: [PATCH 02/23] [#1364] Improved control over generation / sending of notifications. Fixed blocking / muting users notifications issue. Added tests. --- lib/pleroma/activity.ex | 10 ++ lib/pleroma/notification.ex | 129 +++++++++++++----- lib/pleroma/thread_mute.ex | 37 ++++- lib/pleroma/user.ex | 50 +++++-- lib/pleroma/user_relationship.ex | 9 +- .../web/activity_pub/transmogrifier.ex | 8 +- test/notification_test.exs | 112 ++++++++++++++- 7 files changed, 292 insertions(+), 63 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 6ca05f74e..bbaa561a7 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -95,6 +95,16 @@ def with_preloaded_object(query, join_type \\ :inner) do |> preload([activity, object: object], object: object) end + def user_actor(%Activity{actor: nil}), do: nil + + def user_actor(%Activity{} = activity) do + with %User{} <- activity.user_actor do + activity.user_actor + else + _ -> User.get_cached_by_ap_id(activity.actor) + end + end + def with_joined_user_actor(query, join_type \\ :inner) do join(query, join_type, [activity], u in User, on: u.ap_id == activity.actor, diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 60dba3434..0d7a6610a 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -10,6 +10,7 @@ defmodule Pleroma.Notification do alias Pleroma.Object alias Pleroma.Pagination alias Pleroma.Repo + alias Pleroma.ThreadMute alias Pleroma.User alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.Push @@ -17,6 +18,7 @@ defmodule Pleroma.Notification do import Ecto.Query import Ecto.Changeset + require Logger @type t :: %__MODULE__{} @@ -101,7 +103,7 @@ defp exclude_notification_muted(query, user, opts) do query |> where([n, a], a.actor not in ^notification_muted_ap_ids) - |> join(:left, [n, a], tm in Pleroma.ThreadMute, + |> join(:left, [n, a], tm in ThreadMute, on: tm.user_id == ^user.id and tm.context == fragment("?->>'context'", a.data) ) |> where([n, a, o, tm], is_nil(tm.user_id)) @@ -284,58 +286,108 @@ def dismiss(%{id: user_id} = _user, id) do def create_notifications(%Activity{data: %{"to" => _, "type" => "Create"}} = activity) do object = Object.normalize(activity) - unless object && object.data["type"] == "Answer" do - users = get_notified_from_activity(activity) - notifications = Enum.map(users, fn user -> create_notification(activity, user) end) - {:ok, notifications} - else + if object && object.data["type"] == "Answer" do {:ok, []} + else + do_create_notifications(activity) end end def create_notifications(%Activity{data: %{"type" => type}} = activity) when type in ["Like", "Announce", "Follow", "Move", "EmojiReact"] do - notifications = - activity - |> get_notified_from_activity() - |> Enum.map(&create_notification(activity, &1)) - - {:ok, notifications} + do_create_notifications(activity) end def create_notifications(_), do: {:ok, []} + defp do_create_notifications(%Activity{} = activity) do + {enabled_receivers, disabled_receivers} = get_notified_from_activity(activity) + potential_receivers = enabled_receivers ++ disabled_receivers + + notifications = + Enum.map(potential_receivers, fn user -> + do_send = user in enabled_receivers + create_notification(activity, user, do_send) + end) + + {:ok, notifications} + end + # TODO move to sql, too. - def create_notification(%Activity{} = activity, %User{} = user) do + def create_notification(%Activity{} = activity, %User{} = user, do_send \\ true) do unless skip?(activity, user) do notification = %Notification{user_id: user.id, activity: activity} {:ok, notification} = Repo.insert(notification) - ["user", "user:notification"] - |> Streamer.stream(notification) + if do_send do + Streamer.stream(["user", "user:notification"], notification) + Push.send(notification) + end - Push.send(notification) notification end end + @doc """ + Returns a tuple with 2 elements: + {enabled notification receivers, currently disabled receivers (blocking / [thread] muting)} + """ def get_notified_from_activity(activity, local_only \\ true) def get_notified_from_activity(%Activity{data: %{"type" => type}} = activity, local_only) when type in ["Create", "Like", "Announce", "Follow", "Move", "EmojiReact"] do - [] - |> Utils.maybe_notify_to_recipients(activity) - |> Utils.maybe_notify_mentioned_recipients(activity) - |> Utils.maybe_notify_subscribers(activity) - |> Utils.maybe_notify_followers(activity) - |> Enum.uniq() - |> User.get_users_from_set(local_only) + potential_receiver_ap_ids = + [] + |> Utils.maybe_notify_to_recipients(activity) + |> Utils.maybe_notify_mentioned_recipients(activity) + |> Utils.maybe_notify_subscribers(activity) + |> Utils.maybe_notify_followers(activity) + |> Enum.uniq() + + notification_enabled_ap_ids = + potential_receiver_ap_ids + |> exclude_relation_restricting_ap_ids(activity) + |> exclude_thread_muter_ap_ids(activity) + + potential_receivers = + potential_receiver_ap_ids + |> Enum.uniq() + |> User.get_users_from_set(local_only) + + notification_enabled_users = + Enum.filter(potential_receivers, fn u -> u.ap_id in notification_enabled_ap_ids end) + + {notification_enabled_users, potential_receivers -- notification_enabled_users} end - def get_notified_from_activity(_, _local_only), do: [] + def get_notified_from_activity(_, _local_only), do: {[], []} + + @doc "Filters out AP IDs of users basing on their relationships with activity actor user" + def exclude_relation_restricting_ap_ids([], _activity), do: [] + + def exclude_relation_restricting_ap_ids(ap_ids, %Activity{} = activity) do + relation_restricted_ap_ids = + activity + |> Activity.user_actor() + |> User.incoming_relations_ungrouped_ap_ids([ + :block, + :notification_mute + ]) + + Enum.uniq(ap_ids) -- relation_restricted_ap_ids + end + + @doc "Filters out AP IDs of users who mute activity thread" + def exclude_thread_muter_ap_ids([], _activity), do: [] + + def exclude_thread_muter_ap_ids(ap_ids, %Activity{} = activity) do + thread_muter_ap_ids = ThreadMute.muter_ap_ids(activity.data["context"]) + + Enum.uniq(ap_ids) -- thread_muter_ap_ids + end @spec skip?(Activity.t(), User.t()) :: boolean() - def skip?(activity, user) do + def skip?(%Activity{} = activity, %User{} = user) do [ :self, :followers, @@ -344,18 +396,20 @@ def skip?(activity, user) do :non_follows, :recently_followed ] - |> Enum.any?(&skip?(&1, activity, user)) + |> Enum.find(&skip?(&1, activity, user)) end + def skip?(_, _), do: false + @spec skip?(atom(), Activity.t(), User.t()) :: boolean() - def skip?(:self, activity, user) do + def skip?(:self, %Activity{} = activity, %User{} = user) do activity.data["actor"] == user.ap_id end def skip?( :followers, - activity, - %{notification_settings: %{followers: false}} = user + %Activity{} = activity, + %User{notification_settings: %{followers: false}} = user ) do actor = activity.data["actor"] follower = User.get_cached_by_ap_id(actor) @@ -364,15 +418,19 @@ def skip?( def skip?( :non_followers, - activity, - %{notification_settings: %{non_followers: false}} = user + %Activity{} = activity, + %User{notification_settings: %{non_followers: false}} = user ) do actor = activity.data["actor"] follower = User.get_cached_by_ap_id(actor) !User.following?(follower, user) end - def skip?(:follows, activity, %{notification_settings: %{follows: false}} = user) do + def skip?( + :follows, + %Activity{} = activity, + %User{notification_settings: %{follows: false}} = user + ) do actor = activity.data["actor"] followed = User.get_cached_by_ap_id(actor) User.following?(user, followed) @@ -380,15 +438,16 @@ def skip?(:follows, activity, %{notification_settings: %{follows: false}} = user def skip?( :non_follows, - activity, - %{notification_settings: %{non_follows: false}} = user + %Activity{} = activity, + %User{notification_settings: %{non_follows: false}} = user ) do actor = activity.data["actor"] followed = User.get_cached_by_ap_id(actor) !User.following?(user, followed) end - def skip?(:recently_followed, %{data: %{"type" => "Follow"}} = activity, user) do + # To do: consider defining recency in hours and checking FollowingRelationship with a single SQL + def skip?(:recently_followed, %Activity{data: %{"type" => "Follow"}} = activity, %User{} = user) do actor = activity.data["actor"] Notification.for_user(user) diff --git a/lib/pleroma/thread_mute.ex b/lib/pleroma/thread_mute.ex index cc815430a..2b4cf02cf 100644 --- a/lib/pleroma/thread_mute.ex +++ b/lib/pleroma/thread_mute.ex @@ -9,7 +9,8 @@ defmodule Pleroma.ThreadMute do alias Pleroma.ThreadMute alias Pleroma.User - require Ecto.Query + import Ecto.Changeset + import Ecto.Query schema "thread_mutes" do belongs_to(:user, User, type: FlakeId.Ecto.CompatType) @@ -18,19 +19,43 @@ defmodule Pleroma.ThreadMute do def changeset(mute, params \\ %{}) do mute - |> Ecto.Changeset.cast(params, [:user_id, :context]) - |> Ecto.Changeset.foreign_key_constraint(:user_id) - |> Ecto.Changeset.unique_constraint(:user_id, name: :unique_index) + |> cast(params, [:user_id, :context]) + |> foreign_key_constraint(:user_id) + |> unique_constraint(:user_id, name: :unique_index) end def query(user_id, context) do {:ok, user_id} = FlakeId.Ecto.CompatType.dump(user_id) ThreadMute - |> Ecto.Query.where(user_id: ^user_id) - |> Ecto.Query.where(context: ^context) + |> where(user_id: ^user_id) + |> where(context: ^context) end + def muters_query(context) do + ThreadMute + |> join(:inner, [tm], u in assoc(tm, :user)) + |> where([tm], tm.context == ^context) + |> select([tm, u], u.ap_id) + end + + def muter_ap_ids(context, ap_ids \\ nil) + + def muter_ap_ids(context, ap_ids) when context not in [nil, ""] do + context + |> muters_query() + |> maybe_filter_on_ap_id(ap_ids) + |> Repo.all() + end + + def muter_ap_ids(_context, _ap_ids), do: [] + + defp maybe_filter_on_ap_id(query, ap_ids) when is_list(ap_ids) do + where(query, [tm, u], u.ap_id in ^ap_ids) + end + + defp maybe_filter_on_ap_id(query, _ap_ids), do: query + def add_mute(user_id, context) do %ThreadMute{} |> changeset(%{user_id: user_id, context: context}) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index db510d957..8c8ecfe35 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -149,22 +149,26 @@ defmodule Pleroma.User do {outgoing_relation, outgoing_relation_target}, {incoming_relation, incoming_relation_source} ]} <- @user_relationships_config do - # Definitions of `has_many :blocker_blocks`, `has_many :muter_mutes` etc. + # Definitions of `has_many` relations: :blocker_blocks, :muter_mutes, :reblog_muter_mutes, + # :notification_muter_mutes, :subscribee_subscriptions has_many(outgoing_relation, UserRelationship, foreign_key: :source_id, where: [relationship_type: relationship_type] ) - # Definitions of `has_many :blockee_blocks`, `has_many :mutee_mutes` etc. + # Definitions of `has_many` relations: :blockee_blocks, :mutee_mutes, :reblog_mutee_mutes, + # :notification_mutee_mutes, :subscriber_subscriptions has_many(incoming_relation, UserRelationship, foreign_key: :target_id, where: [relationship_type: relationship_type] ) - # Definitions of `has_many :blocked_users`, `has_many :muted_users` etc. + # Definitions of `has_many` relations: :blocked_users, :muted_users, :reblog_muted_users, + # :notification_muted_users, :subscriber_users has_many(outgoing_relation_target, through: [outgoing_relation, :target]) - # Definitions of `has_many :blocker_users`, `has_many :muter_users` etc. + # Definitions of `has_many` relations: :blocker_users, :muter_users, :reblog_muter_users, + # :notification_muter_users, :subscribee_users has_many(incoming_relation_source, through: [incoming_relation, :source]) end @@ -184,7 +188,9 @@ defmodule Pleroma.User do for {_relationship_type, [{_outgoing_relation, outgoing_relation_target}, _]} <- @user_relationships_config do - # Definitions of `blocked_users_relation/1`, `muted_users_relation/1`, etc. + # `def blocked_users_relation/2`, `def muted_users_relation/2`, + # `def reblog_muted_users_relation/2`, `def notification_muted_users/2`, + # `def subscriber_users/2` def unquote(:"#{outgoing_relation_target}_relation")(user, restrict_deactivated? \\ false) do target_users_query = assoc(user, unquote(outgoing_relation_target)) @@ -195,7 +201,8 @@ def unquote(:"#{outgoing_relation_target}_relation")(user, restrict_deactivated? end end - # Definitions of `blocked_users/1`, `muted_users/1`, etc. + # `def blocked_users/2`, `def muted_users/2`, `def reblog_muted_users/2`, + # `def notification_muted_users/2`, `def subscriber_users/2` def unquote(outgoing_relation_target)(user, restrict_deactivated? \\ false) do __MODULE__ |> apply(unquote(:"#{outgoing_relation_target}_relation"), [ @@ -205,7 +212,8 @@ def unquote(outgoing_relation_target)(user, restrict_deactivated? \\ false) do |> Repo.all() end - # Definitions of `blocked_users_ap_ids/1`, `muted_users_ap_ids/1`, etc. + # `def blocked_users_ap_ids/2`, `def muted_users_ap_ids/2`, `def reblog_muted_users_ap_ids/2`, + # `def notification_muted_users_ap_ids/2`, `def subscriber_users_ap_ids/2` def unquote(:"#{outgoing_relation_target}_ap_ids")(user, restrict_deactivated? \\ false) do __MODULE__ |> apply(unquote(:"#{outgoing_relation_target}_relation"), [ @@ -1217,7 +1225,9 @@ def subscribed_to?(%User{} = user, %{ap_id: ap_id}) do E.g. `outgoing_relations_ap_ids(user, [:block])` -> `%{block: ["https://some.site/users/userapid"]}` """ @spec outgoing_relations_ap_ids(User.t(), list(atom())) :: %{atom() => list(String.t())} - def outgoing_relations_ap_ids(_, []), do: %{} + def outgoing_relations_ap_ids(_user, []), do: %{} + + def outgoing_relations_ap_ids(nil, _relationship_types), do: %{} def outgoing_relations_ap_ids(%User{} = user, relationship_types) when is_list(relationship_types) do @@ -1238,6 +1248,30 @@ def outgoing_relations_ap_ids(%User{} = user, relationship_types) ) end + def incoming_relations_ungrouped_ap_ids(user, relationship_types, ap_ids \\ nil) + + def incoming_relations_ungrouped_ap_ids(_user, [], _ap_ids), do: [] + + def incoming_relations_ungrouped_ap_ids(nil, _relationship_types, _ap_ids), do: [] + + def incoming_relations_ungrouped_ap_ids(%User{} = user, relationship_types, ap_ids) + when is_list(relationship_types) do + user + |> assoc(:incoming_relationships) + |> join(:inner, [user_rel], u in assoc(user_rel, :source)) + |> where([user_rel, u], user_rel.relationship_type in ^relationship_types) + |> maybe_filter_on_ap_id(ap_ids) + |> select([user_rel, u], u.ap_id) + |> distinct(true) + |> Repo.all() + end + + defp maybe_filter_on_ap_id(query, ap_ids) when is_list(ap_ids) do + where(query, [user_rel, u], u.ap_id in ^ap_ids) + end + + defp maybe_filter_on_ap_id(query, _ap_ids), do: query + def deactivate_async(user, status \\ true) do BackgroundWorker.enqueue("deactivate_user", %{"user_id" => user.id, "status" => status}) end diff --git a/lib/pleroma/user_relationship.ex b/lib/pleroma/user_relationship.ex index 393947942..01b6ace9d 100644 --- a/lib/pleroma/user_relationship.ex +++ b/lib/pleroma/user_relationship.ex @@ -21,15 +21,18 @@ defmodule Pleroma.UserRelationship do end for relationship_type <- Keyword.keys(UserRelationshipTypeEnum.__enum_map__()) do - # Definitions of `create_block/2`, `create_mute/2` etc. + # `def create_block/2`, `def create_mute/2`, `def create_reblog_mute/2`, + # `def create_notification_mute/2`, `def create_inverse_subscription/2` def unquote(:"create_#{relationship_type}")(source, target), do: create(unquote(relationship_type), source, target) - # Definitions of `delete_block/2`, `delete_mute/2` etc. + # `def delete_block/2`, `def delete_mute/2`, `def delete_reblog_mute/2`, + # `def delete_notification_mute/2`, `def delete_inverse_subscription/2` def unquote(:"delete_#{relationship_type}")(source, target), do: delete(unquote(relationship_type), source, target) - # Definitions of `block_exists?/2`, `mute_exists?/2` etc. + # `def block_exists?/2`, `def mute_exists?/2`, `def reblog_mute_exists?/2`, + # `def notification_mute_exists?/2`, `def inverse_subscription_exists?/2` def unquote(:"#{relationship_type}_exists?")(source, target), do: exists?(unquote(relationship_type), source, target) end diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 9cd3de705..d6549a932 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -1108,13 +1108,11 @@ def add_hashtags(object) do end def add_mention_tags(object) do - mentions = - object - |> Utils.get_notified_from_object() - |> Enum.map(&build_mention_tag/1) + {enabled_receivers, disabled_receivers} = Utils.get_notified_from_object(object) + potential_receivers = enabled_receivers ++ disabled_receivers + mentions = Enum.map(potential_receivers, &build_mention_tag/1) tags = object["tag"] || [] - Map.put(object, "tag", tags ++ mentions) end diff --git a/test/notification_test.exs b/test/notification_test.exs index 56a581810..bc2d80f05 100644 --- a/test/notification_test.exs +++ b/test/notification_test.exs @@ -6,12 +6,14 @@ defmodule Pleroma.NotificationTest do use Pleroma.DataCase import Pleroma.Factory + import Mock alias Pleroma.Notification alias Pleroma.Tests.ObanHelpers alias Pleroma.User alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.CommonAPI + alias Pleroma.Web.Push alias Pleroma.Web.Streamer describe "create_notifications" do @@ -382,7 +384,7 @@ test "Returns recent notifications" do end end - describe "notification target determination" do + describe "notification target determination / get_notified_from_activity/2" do test "it sends notifications to addressed users in new messages" do user = insert(:user) other_user = insert(:user) @@ -392,7 +394,9 @@ test "it sends notifications to addressed users in new messages" do "status" => "hey @#{other_user.nickname}!" }) - assert other_user in Notification.get_notified_from_activity(activity) + {enabled_receivers, _disabled_receivers} = Notification.get_notified_from_activity(activity) + + assert other_user in enabled_receivers end test "it sends notifications to mentioned users in new messages" do @@ -420,7 +424,9 @@ test "it sends notifications to mentioned users in new messages" do {:ok, activity} = Transmogrifier.handle_incoming(create_activity) - assert other_user in Notification.get_notified_from_activity(activity) + {enabled_receivers, _disabled_receivers} = Notification.get_notified_from_activity(activity) + + assert other_user in enabled_receivers end test "it does not send notifications to users who are only cc in new messages" do @@ -442,7 +448,9 @@ test "it does not send notifications to users who are only cc in new messages" d {:ok, activity} = Transmogrifier.handle_incoming(create_activity) - assert other_user not in Notification.get_notified_from_activity(activity) + {enabled_receivers, _disabled_receivers} = Notification.get_notified_from_activity(activity) + + assert other_user not in enabled_receivers end test "it does not send notification to mentioned users in likes" do @@ -457,7 +465,10 @@ test "it does not send notification to mentioned users in likes" do {:ok, activity_two, _} = CommonAPI.favorite(activity_one.id, third_user) - assert other_user not in Notification.get_notified_from_activity(activity_two) + {enabled_receivers, _disabled_receivers} = + Notification.get_notified_from_activity(activity_two) + + assert other_user not in enabled_receivers end test "it does not send notification to mentioned users in announces" do @@ -472,7 +483,96 @@ test "it does not send notification to mentioned users in announces" do {:ok, activity_two, _} = CommonAPI.repeat(activity_one.id, third_user) - assert other_user not in Notification.get_notified_from_activity(activity_two) + {enabled_receivers, _disabled_receivers} = + Notification.get_notified_from_activity(activity_two) + + assert other_user not in enabled_receivers + end + + test_with_mock "it returns blocking recipient in disabled recipients list", + Push, + [:passthrough], + [] do + user = insert(:user) + other_user = insert(:user) + {:ok, _user_relationship} = User.block(other_user, user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hey @#{other_user.nickname}!"}) + + {enabled_receivers, disabled_receivers} = Notification.get_notified_from_activity(activity) + + assert [] == enabled_receivers + assert [other_user] == disabled_receivers + + assert 1 == length(Repo.all(Notification)) + refute called(Push.send(:_)) + end + + test_with_mock "it returns notification-muting recipient in disabled recipients list", + Push, + [:passthrough], + [] do + user = insert(:user) + other_user = insert(:user) + {:ok, _user_relationships} = User.mute(other_user, user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hey @#{other_user.nickname}!"}) + + {enabled_receivers, disabled_receivers} = Notification.get_notified_from_activity(activity) + + assert [] == enabled_receivers + assert [other_user] == disabled_receivers + + assert 1 == length(Repo.all(Notification)) + refute called(Push.send(:_)) + end + + test_with_mock "it returns thread-muting recipient in disabled recipients list", + Push, + [:passthrough], + [] do + user = insert(:user) + other_user = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hey @#{other_user.nickname}!"}) + + {:ok, _} = CommonAPI.add_mute(other_user, activity) + + {:ok, same_context_activity} = + CommonAPI.post(user, %{ + "status" => "hey-hey-hey @#{other_user.nickname}!", + "in_reply_to_status_id" => activity.id + }) + + {enabled_receivers, disabled_receivers} = + Notification.get_notified_from_activity(same_context_activity) + + assert [other_user] == disabled_receivers + refute other_user in enabled_receivers + + [pre_mute_notification, post_mute_notification] = + Repo.all(from(n in Notification, where: n.user_id == ^other_user.id, order_by: n.id)) + + pre_mute_notification_id = pre_mute_notification.id + post_mute_notification_id = post_mute_notification.id + + assert called( + Push.send( + :meck.is(fn + %Notification{id: ^pre_mute_notification_id} -> true + _ -> false + end) + ) + ) + + refute called( + Push.send( + :meck.is(fn + %Notification{id: ^post_mute_notification_id} -> true + _ -> false + end) + ) + ) end end From d198e7fa2a0c92be4e99c5a765de85096d318bfe Mon Sep 17 00:00:00 2001 From: eugenijm Date: Tue, 28 Jan 2020 09:47:59 +0300 Subject: [PATCH 03/23] Admin API: `PATCH /api/pleroma/admin/users/:nickname/change_password` --- CHANGELOG.md | 1 + docs/API/admin_api.md | 8 +++++ lib/pleroma/moderation_log.ex | 11 +++++++ .../web/admin_api/admin_api_controller.ex | 33 +++++++++++++++++++ lib/pleroma/web/router.ex | 1 + .../admin_api/admin_api_controller_test.exs | 26 +++++++++++++++ 6 files changed, 80 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4168086e2..0f8091c8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: User timelines will now respect blocks, unless you are getting the user timeline of somebody you blocked (which would be empty otherwise). - Mastodon API: Favoriting / Repeating a post multiple times will now return the identical response every time. Before, executing that action twice would return an error ("already favorited") on the second try. - Mastodon API: Limit timeline requests to 3 per timeline per 500ms per user/ip by default. +- Admin API: `PATCH /api/pleroma/admin/users/:nickname/change_password` ### Added diff --git a/docs/API/admin_api.md b/docs/API/admin_api.md index 47afdfba5..cb8201f11 100644 --- a/docs/API/admin_api.md +++ b/docs/API/admin_api.md @@ -414,6 +414,14 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret - `nicknames` - Response: none (code `204`) +## `PATCH /api/pleroma/admin/users/:nickname/change_password` + +### Change the user password + +- Params: + - `new_password` +- Response: none (code `200`) + ## `GET /api/pleroma/admin/reports` ### Get a list of reports diff --git a/lib/pleroma/moderation_log.ex b/lib/pleroma/moderation_log.ex index e32895f70..b5435a553 100644 --- a/lib/pleroma/moderation_log.ex +++ b/lib/pleroma/moderation_log.ex @@ -605,6 +605,17 @@ def get_log_entry_message(%ModerationLog{ }" end + @spec get_log_entry_message(ModerationLog) :: String.t() + def get_log_entry_message(%ModerationLog{ + data: %{ + "actor" => %{"nickname" => actor_nickname}, + "action" => "change_password", + "subject" => subjects + } + }) do + "@#{actor_nickname} changed password for users: #{users_to_nicknames_string(subjects)}" + end + defp nicknames_to_string(nicknames) do nicknames |> Enum.map(&"@#{&1}") diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 175260bc2..2aa2c6ac2 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -658,6 +658,39 @@ def force_password_reset(%{assigns: %{user: admin}} = conn, %{"nicknames" => nic json_response(conn, :no_content, "") end + @doc "Changes password for a given user" + def change_password(%{assigns: %{user: admin}} = conn, %{"nickname" => nickname} = params) do + with {_, user} <- {:user, User.get_cached_by_nickname(nickname)}, + {:ok, _user} <- + User.reset_password(user, %{ + password: params["new_password"], + password_confirmation: params["new_password"] + }) do + ModerationLog.insert_log(%{ + actor: admin, + subject: [user], + action: "change_password" + }) + + User.force_password_reset_async(user) + + ModerationLog.insert_log(%{ + actor: admin, + subject: [user], + action: "force_password_reset" + }) + + json(conn, %{status: "success"}) + else + {:error, changeset} -> + {_, {error, _}} = Enum.at(changeset.errors, 0) + json(conn, %{error: "New password #{error}."}) + + _ -> + json(conn, %{error: "Unable to change password."}) + end + end + def list_reports(conn, params) do {page, page_size} = page_params(params) diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index e4e3ee704..c03ad101e 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -173,6 +173,7 @@ defmodule Pleroma.Web.Router do get("/users/:nickname/password_reset", AdminAPIController, :get_password_reset) patch("/users/force_password_reset", AdminAPIController, :force_password_reset) + patch("/users/:nickname/change_password", AdminAPIController, :change_password) get("/users", AdminAPIController, :list_users) get("/users/:nickname", AdminAPIController, :user_show) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index e4c152fb7..0c1214f05 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -3389,6 +3389,32 @@ test "returns log filtered by search", %{conn: conn, moderator: moderator} do end end + describe "PATCH /users/:nickname/change_password" do + test "changes password", %{conn: conn, admin: admin} do + user = insert(:user) + assert user.password_reset_pending == false + + conn = + patch(conn, "/api/pleroma/admin/users/#{user.nickname}/change_password", %{ + "new_password" => "password" + }) + + assert json_response(conn, 200) == %{"status" => "success"} + + ObanHelpers.perform_all() + + assert User.get_by_id(user.id).password_reset_pending == true + + [log_entry1, log_entry2] = ModerationLog |> Repo.all() |> Enum.sort() + + assert ModerationLog.get_log_entry_message(log_entry1) == + "@#{admin.nickname} changed password for users: @#{user.nickname}" + + assert ModerationLog.get_log_entry_message(log_entry2) == + "@#{admin.nickname} forced password reset for users: @#{user.nickname}" + end + end + describe "PATCH /users/:nickname/force_password_reset" do test "sets password_reset_pending to true", %{conn: conn} do user = insert(:user) From 13cce9c0debbf9a80ed5da26cb34ca563e5e1417 Mon Sep 17 00:00:00 2001 From: eugenijm Date: Fri, 31 Jan 2020 21:07:46 +0300 Subject: [PATCH 04/23] Admin API: `PATCH /api/pleroma/admin/users/:nickname/credentials`, `GET /api/pleroma/admin/users/:nickname/credentials`. --- CHANGELOG.md | 2 +- docs/API/admin_api.md | 75 +++++++++++++++- lib/pleroma/moderation_log.ex | 4 +- lib/pleroma/user.ex | 86 ++++++++++++++++++- .../web/admin_api/admin_api_controller.ex | 34 +++++--- .../web/admin_api/views/account_view.ex | 40 +++++++++ .../controllers/account_controller.ex | 60 +++---------- lib/pleroma/web/router.ex | 3 +- .../admin_api/admin_api_controller_test.exs | 57 ++++++++++-- 9 files changed, 286 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f8091c8c..ec04c26e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,7 +67,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: User timelines will now respect blocks, unless you are getting the user timeline of somebody you blocked (which would be empty otherwise). - Mastodon API: Favoriting / Repeating a post multiple times will now return the identical response every time. Before, executing that action twice would return an error ("already favorited") on the second try. - Mastodon API: Limit timeline requests to 3 per timeline per 500ms per user/ip by default. -- Admin API: `PATCH /api/pleroma/admin/users/:nickname/change_password` +- Admin API: `PATCH /api/pleroma/admin/users/:nickname/credentials` and `GET /api/pleroma/admin/users/:nickname/credentials` ### Added diff --git a/docs/API/admin_api.md b/docs/API/admin_api.md index cb8201f11..edcf73e14 100644 --- a/docs/API/admin_api.md +++ b/docs/API/admin_api.md @@ -414,12 +414,81 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret - `nicknames` - Response: none (code `204`) -## `PATCH /api/pleroma/admin/users/:nickname/change_password` +## `GET /api/pleroma/admin/users/:nickname/credentials` -### Change the user password +### Get the user's email, password, display and settings-related fields - Params: - - `new_password` + - `nickname` + +- Response: + +```json +{ + "actor_type": "Person", + "allow_following_move": true, + "avatar": "https://pleroma.social/media/7e8e7508fd545ef580549b6881d80ec0ff2c81ed9ad37b9bdbbdf0e0d030159d.jpg", + "background": "https://pleroma.social/media/4de34c0bd10970d02cbdef8972bef0ebbf55f43cadc449554d4396156162fe9a.jpg", + "banner": "https://pleroma.social/media/8d92ba2bd244b613520abf557dd448adcd30f5587022813ee9dd068945986946.jpg", + "bio": "bio", + "default_scope": "public", + "discoverable": false, + "email": "user@example.com", + "fields": [ + { + "name": "example", + "value": "https://example.com" + } + ], + "hide_favorites": false, + "hide_followers": false, + "hide_followers_count": false, + "hide_follows": false, + "hide_follows_count": false, + "id": "9oouHaEEUR54hls968", + "locked": true, + "name": "user", + "no_rich_text": true, + "pleroma_settings_store": {}, + "raw_fields": [ + { + "id": 1, + "name": "example", + "value": "https://example.com" + }, + ], + "show_role": true, + "skip_thread_containment": false +} +``` + +## `PATCH /api/pleroma/admin/users/:nickname/credentials` + +### Change the user's email, password, display and settings-related fields + +- Params: + - `email` + - `password` + - `name` + - `bio` + - `avatar` + - `locked` + - `no_rich_text` + - `default_scope` + - `banner` + - `hide_follows` + - `hide_followers` + - `hide_followers_count` + - `hide_follows_count` + - `hide_favorites` + - `allow_following_move` + - `background` + - `show_role` + - `skip_thread_containment` + - `fields` + - `discoverable` + - `actor_type` + - Response: none (code `200`) ## `GET /api/pleroma/admin/reports` diff --git a/lib/pleroma/moderation_log.ex b/lib/pleroma/moderation_log.ex index b5435a553..7aacd9d80 100644 --- a/lib/pleroma/moderation_log.ex +++ b/lib/pleroma/moderation_log.ex @@ -609,11 +609,11 @@ def get_log_entry_message(%ModerationLog{ def get_log_entry_message(%ModerationLog{ data: %{ "actor" => %{"nickname" => actor_nickname}, - "action" => "change_password", + "action" => "updated_users", "subject" => subjects } }) do - "@#{actor_nickname} changed password for users: #{users_to_nicknames_string(subjects)}" + "@#{actor_nickname} updated users: #{users_to_nicknames_string(subjects)}" end defp nicknames_to_string(nicknames) do diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 911dde6e2..44de64345 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -417,9 +417,55 @@ def update_changeset(struct, params \\ %{}) do |> validate_format(:nickname, local_nickname_regex()) |> validate_length(:bio, max: bio_limit) |> validate_length(:name, min: 1, max: name_limit) + |> put_fields() + |> put_change_if_present(:bio, &{:ok, parse_bio(&1, struct)}) + |> put_change_if_present(:avatar, &put_upload(&1, :avatar)) + |> put_change_if_present(:banner, &put_upload(&1, :banner)) + |> put_change_if_present(:background, &put_upload(&1, :background)) + |> put_change_if_present( + :pleroma_settings_store, + &{:ok, Map.merge(struct.pleroma_settings_store, &1)} + ) |> validate_fields(false) end + defp put_fields(changeset) do + if raw_fields = get_change(changeset, :raw_fields) do + raw_fields = + raw_fields + |> Enum.filter(fn %{"name" => n} -> n != "" end) + + fields = + raw_fields + |> Enum.map(fn f -> Map.update!(f, "value", &AutoLinker.link(&1)) end) + + changeset + |> put_change(:raw_fields, raw_fields) + |> put_change(:fields, fields) + else + changeset + end + end + + defp put_change_if_present(changeset, map_field, value_function) do + if value = get_change(changeset, map_field) do + with {:ok, new_value} <- value_function.(value) do + put_change(changeset, map_field, new_value) + else + _ -> changeset + end + else + changeset + end + end + + defp put_upload(value, type) do + with %Plug.Upload{} <- value, + {:ok, object} <- ActivityPub.upload(value, type: type) do + {:ok, object.data} + end + end + def upgrade_changeset(struct, params \\ %{}, remote? \\ false) do bio_limit = Pleroma.Config.get([:instance, :user_bio_length], 5000) name_limit = Pleroma.Config.get([:instance, :user_name_length], 100) @@ -463,6 +509,27 @@ def upgrade_changeset(struct, params \\ %{}, remote? \\ false) do |> validate_fields(remote?) end + def update_as_admin_changeset(struct, params) do + struct + |> update_changeset(params) + |> cast(params, [:email]) + |> delete_change(:also_known_as) + |> unique_constraint(:email) + |> validate_format(:email, @email_regex) + end + + @spec update_as_admin(%User{}, map) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} + def update_as_admin(user, params) do + params = Map.put(params, "password_confirmation", params["password"]) + changeset = update_as_admin_changeset(user, params) + + if params["password"] do + reset_password(user, changeset, params) + else + User.update_and_set_cache(changeset) + end + end + def password_update_changeset(struct, params) do struct |> cast(params, [:password, :password_confirmation]) @@ -473,10 +540,14 @@ def password_update_changeset(struct, params) do end @spec reset_password(User.t(), map) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} - def reset_password(%User{id: user_id} = user, data) do + def reset_password(%User{} = user, params) do + reset_password(user, user, params) + end + + def reset_password(%User{id: user_id} = user, struct, params) do multi = Multi.new() - |> Multi.update(:user, password_update_changeset(user, data)) + |> Multi.update(:user, password_update_changeset(struct, params)) |> Multi.delete_all(:tokens, OAuth.Token.Query.get_by_user(user_id)) |> Multi.delete_all(:auth, OAuth.Authorization.delete_by_user_query(user)) @@ -1856,6 +1927,17 @@ def fields(%{fields: nil}), do: [] def fields(%{fields: fields}), do: fields + def sanitized_fields(%User{} = user) do + user + |> User.fields() + |> Enum.map(fn %{"name" => name, "value" => value} -> + %{ + "name" => name, + "value" => Pleroma.HTML.filter_tags(value, Pleroma.HTML.Scrubber.LinksOnly) + } + end) + end + def validate_fields(changeset, remote? \\ false) do limit_name = if remote?, do: :max_remote_account_fields, else: :max_account_fields limit = Pleroma.Config.get([:instance, limit_name], 0) diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 2aa2c6ac2..0368df1e9 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -38,7 +38,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do plug( OAuthScopesPlug, %{scopes: ["read:accounts"], admin: true} - when action in [:list_users, :user_show, :right_get] + when action in [:list_users, :user_show, :right_get, :show_user_credentials] ) plug( @@ -54,7 +54,8 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do :tag_users, :untag_users, :right_add, - :right_delete + :right_delete, + :update_user_credentials ] ) @@ -658,21 +659,34 @@ def force_password_reset(%{assigns: %{user: admin}} = conn, %{"nicknames" => nic json_response(conn, :no_content, "") end - @doc "Changes password for a given user" - def change_password(%{assigns: %{user: admin}} = conn, %{"nickname" => nickname} = params) do + @doc "Show a given user's credentials" + def show_user_credentials(%{assigns: %{user: admin}} = conn, %{"nickname" => nickname}) do + with %User{} = user <- User.get_cached_by_nickname_or_id(nickname) do + conn + |> put_view(AccountView) + |> render("credentials.json", %{user: user, for: admin}) + else + _ -> {:error, :not_found} + end + end + + @doc "Updates a given user" + def update_user_credentials( + %{assigns: %{user: admin}} = conn, + %{"nickname" => nickname} = params + ) do with {_, user} <- {:user, User.get_cached_by_nickname(nickname)}, {:ok, _user} <- - User.reset_password(user, %{ - password: params["new_password"], - password_confirmation: params["new_password"] - }) do + User.update_as_admin(user, params) do ModerationLog.insert_log(%{ actor: admin, subject: [user], - action: "change_password" + action: "updated_users" }) - User.force_password_reset_async(user) + if params["password"] do + User.force_password_reset_async(user) + end ModerationLog.insert_log(%{ actor: admin, diff --git a/lib/pleroma/web/admin_api/views/account_view.ex b/lib/pleroma/web/admin_api/views/account_view.ex index 1e03849de..a16a3ebf0 100644 --- a/lib/pleroma/web/admin_api/views/account_view.ex +++ b/lib/pleroma/web/admin_api/views/account_view.ex @@ -23,6 +23,43 @@ def render("index.json", %{users: users}) do } end + def render("credentials.json", %{user: user, for: for_user}) do + user = User.sanitize_html(user, User.html_filter_policy(for_user)) + avatar = User.avatar_url(user) |> MediaProxy.url() + banner = User.banner_url(user) |> MediaProxy.url() + background = image_url(user.background) |> MediaProxy.url() + + user + |> Map.take([ + :id, + :bio, + :email, + :fields, + :name, + :nickname, + :locked, + :no_rich_text, + :default_scope, + :hide_follows, + :hide_followers_count, + :hide_follows_count, + :hide_followers, + :hide_favorites, + :allow_following_move, + :show_role, + :skip_thread_containment, + :pleroma_settings_store, + :raw_fields, + :discoverable, + :actor_type + ]) + |> Map.merge(%{ + "avatar" => avatar, + "banner" => banner, + "background" => background + }) + end + def render("show.json", %{user: user}) do avatar = User.avatar_url(user) |> MediaProxy.url() display_name = Pleroma.HTML.strip_tags(user.name || user.nickname) @@ -104,4 +141,7 @@ defp parse_error(errors) do "" end end + + defp image_url(%{"url" => [%{"href" => href} | _]}), do: href + defp image_url(_), do: nil end diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex index 88c997b9f..56e6214c5 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -8,7 +8,6 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do import Pleroma.Web.ControllerHelper, only: [add_link_headers: 2, truthy_param?: 1, assign_account_by_id: 2, json_response: 3] - alias Pleroma.Emoji alias Pleroma.Plugs.OAuthScopesPlug alias Pleroma.Plugs.RateLimiter alias Pleroma.User @@ -140,17 +139,6 @@ def verify_credentials(%{assigns: %{user: user}} = conn, _) do def update_credentials(%{assigns: %{user: original_user}} = conn, params) do user = original_user - params = - if Map.has_key?(params, "fields_attributes") do - Map.update!(params, "fields_attributes", fn fields -> - fields - |> normalize_fields_attributes() - |> Enum.filter(fn %{"name" => n} -> n != "" end) - end) - else - params - end - user_params = [ :no_rich_text, @@ -169,46 +157,20 @@ def update_credentials(%{assigns: %{user: original_user}} = conn, params) do add_if_present(acc, params, to_string(key), key, &{:ok, truthy_param?(&1)}) end) |> add_if_present(params, "display_name", :name) - |> add_if_present(params, "note", :bio, fn value -> {:ok, User.parse_bio(value, user)} end) - |> add_if_present(params, "avatar", :avatar, fn value -> - with %Plug.Upload{} <- value, - {:ok, object} <- ActivityPub.upload(value, type: :avatar) do - {:ok, object.data} - end - end) - |> add_if_present(params, "header", :banner, fn value -> - with %Plug.Upload{} <- value, - {:ok, object} <- ActivityPub.upload(value, type: :banner) do - {:ok, object.data} - end - end) - |> add_if_present(params, "pleroma_background_image", :background, fn value -> - with %Plug.Upload{} <- value, - {:ok, object} <- ActivityPub.upload(value, type: :background) do - {:ok, object.data} - end - end) - |> add_if_present(params, "fields_attributes", :fields, fn fields -> - fields = Enum.map(fields, fn f -> Map.update!(f, "value", &AutoLinker.link(&1)) end) - - {:ok, fields} - end) - |> add_if_present(params, "fields_attributes", :raw_fields) - |> add_if_present(params, "pleroma_settings_store", :pleroma_settings_store, fn value -> - {:ok, Map.merge(user.pleroma_settings_store, value)} - end) + |> add_if_present(params, "note", :bio) + |> add_if_present(params, "avatar", :avatar) + |> add_if_present(params, "header", :banner) + |> add_if_present(params, "pleroma_background_image", :background) + |> add_if_present( + params, + "fields_attributes", + :raw_fields, + &{:ok, normalize_fields_attributes(&1)} + ) + |> add_if_present(params, "pleroma_settings_store", :pleroma_settings_store) |> add_if_present(params, "default_scope", :default_scope) |> add_if_present(params, "actor_type", :actor_type) - emojis_text = (user_params["display_name"] || "") <> (user_params["note"] || "") - - user_emojis = - user - |> Map.get(:emoji, []) - |> Enum.concat(Emoji.Formatter.get_emoji_map(emojis_text)) - |> Enum.dedup() - - user_params = Map.put(user_params, :emoji, user_emojis) changeset = User.update_changeset(user, user_params) with {:ok, user} <- User.update_and_set_cache(changeset) do diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index c03ad101e..2927775eb 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -173,7 +173,8 @@ defmodule Pleroma.Web.Router do get("/users/:nickname/password_reset", AdminAPIController, :get_password_reset) patch("/users/force_password_reset", AdminAPIController, :force_password_reset) - patch("/users/:nickname/change_password", AdminAPIController, :change_password) + get("/users/:nickname/credentials", AdminAPIController, :show_user_credentials) + patch("/users/:nickname/credentials", AdminAPIController, :update_user_credentials) get("/users", AdminAPIController, :list_users) get("/users/:nickname", AdminAPIController, :user_show) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 0c1214f05..0a317cf88 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -3389,30 +3389,73 @@ test "returns log filtered by search", %{conn: conn, moderator: moderator} do end end - describe "PATCH /users/:nickname/change_password" do - test "changes password", %{conn: conn, admin: admin} do + describe "GET /users/:nickname/credentials" do + test "gets the user credentials", %{conn: conn} do + user = insert(:user) + conn = get(conn, "/api/pleroma/admin/users/#{user.nickname}/credentials") + + response = assert json_response(conn, 200) + assert response["email"] == user.email + end + + test "returns 403 if requested by a non-admin" do + user = insert(:user) + + conn = + build_conn() + |> assign(:user, user) + |> get("/api/pleroma/admin/users/#{user.nickname}/credentials") + + assert json_response(conn, :forbidden) + end + end + + describe "PATCH /users/:nickname/credentials" do + test "changes password and email", %{conn: conn, admin: admin} do user = insert(:user) assert user.password_reset_pending == false conn = - patch(conn, "/api/pleroma/admin/users/#{user.nickname}/change_password", %{ - "new_password" => "password" + patch(conn, "/api/pleroma/admin/users/#{user.nickname}/credentials", %{ + "password" => "new_password", + "email" => "new_email@example.com", + "name" => "new_name" }) assert json_response(conn, 200) == %{"status" => "success"} ObanHelpers.perform_all() - assert User.get_by_id(user.id).password_reset_pending == true + updated_user = User.get_by_id(user.id) - [log_entry1, log_entry2] = ModerationLog |> Repo.all() |> Enum.sort() + assert updated_user.email == "new_email@example.com" + assert updated_user.name == "new_name" + assert updated_user.password_hash != user.password_hash + assert updated_user.password_reset_pending == true + + [log_entry2, log_entry1] = ModerationLog |> Repo.all() |> Enum.sort() assert ModerationLog.get_log_entry_message(log_entry1) == - "@#{admin.nickname} changed password for users: @#{user.nickname}" + "@#{admin.nickname} updated users: @#{user.nickname}" assert ModerationLog.get_log_entry_message(log_entry2) == "@#{admin.nickname} forced password reset for users: @#{user.nickname}" end + + test "returns 403 if requested by a non-admin" do + user = insert(:user) + + conn = + build_conn() + |> assign(:user, user) + |> patch("/api/pleroma/admin/users/#{user.nickname}/credentials", %{ + "password" => "new_password", + "email" => "new_email@example.com", + "name" => "new_name" + }) + + assert json_response(conn, :forbidden) + end end describe "PATCH /users/:nickname/force_password_reset" do From 74388336852b18d5d5f108a8305f1a038301f7a1 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Mon, 16 Mar 2020 21:58:10 +0300 Subject: [PATCH 05/23] [#1364] Improved notification-related tests. --- lib/pleroma/notification.ex | 1 + test/notification_test.exs | 121 +++++++++++++++++++++++------------- 2 files changed, 79 insertions(+), 43 deletions(-) diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 0d7a6610a..104368fd1 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -344,6 +344,7 @@ def get_notified_from_activity(%Activity{data: %{"type" => type}} = activity, lo |> Utils.maybe_notify_followers(activity) |> Enum.uniq() + # Since even subscribers and followers can mute / thread-mute, filtering all above AP IDs notification_enabled_ap_ids = potential_receiver_ap_ids |> exclude_relation_restricting_ap_ids(activity) diff --git a/test/notification_test.exs b/test/notification_test.exs index bc2d80f05..a7282c929 100644 --- a/test/notification_test.exs +++ b/test/notification_test.exs @@ -82,6 +82,80 @@ test "does not create a notification for subscribed users if status is a reply" end end + describe "CommonApi.post/2 notification-related functionality" do + test_with_mock "creates but does NOT send notification to blocker user", + Push, + [:passthrough], + [] do + user = insert(:user) + blocker = insert(:user) + {:ok, _user_relationship} = User.block(blocker, user) + + {:ok, _activity} = CommonAPI.post(user, %{"status" => "hey @#{blocker.nickname}!"}) + + blocker_id = blocker.id + assert [%Notification{user_id: ^blocker_id}] = Repo.all(Notification) + refute called(Push.send(:_)) + end + + test_with_mock "creates but does NOT send notification to notification-muter user", + Push, + [:passthrough], + [] do + user = insert(:user) + muter = insert(:user) + {:ok, _user_relationships} = User.mute(muter, user) + + {:ok, _activity} = CommonAPI.post(user, %{"status" => "hey @#{muter.nickname}!"}) + + muter_id = muter.id + assert [%Notification{user_id: ^muter_id}] = Repo.all(Notification) + refute called(Push.send(:_)) + end + + test_with_mock "creates but does NOT send notification to thread-muter user", + Push, + [:passthrough], + [] do + user = insert(:user) + thread_muter = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hey @#{thread_muter.nickname}!"}) + + {:ok, _} = CommonAPI.add_mute(thread_muter, activity) + + {:ok, _same_context_activity} = + CommonAPI.post(user, %{ + "status" => "hey-hey-hey @#{thread_muter.nickname}!", + "in_reply_to_status_id" => activity.id + }) + + [pre_mute_notification, post_mute_notification] = + Repo.all(from(n in Notification, where: n.user_id == ^thread_muter.id, order_by: n.id)) + + pre_mute_notification_id = pre_mute_notification.id + post_mute_notification_id = post_mute_notification.id + + assert called( + Push.send( + :meck.is(fn + %Notification{id: ^pre_mute_notification_id} -> true + _ -> false + end) + ) + ) + + refute called( + Push.send( + :meck.is(fn + %Notification{id: ^post_mute_notification_id} -> true + _ -> false + end) + ) + ) + end + end + describe "create_notification" do @tag needs_streamer: true test "it creates a notification for user and send to the 'user' and the 'user:notification' stream" do @@ -489,10 +563,7 @@ test "it does not send notification to mentioned users in announces" do assert other_user not in enabled_receivers end - test_with_mock "it returns blocking recipient in disabled recipients list", - Push, - [:passthrough], - [] do + test "it returns blocking recipient in disabled recipients list" do user = insert(:user) other_user = insert(:user) {:ok, _user_relationship} = User.block(other_user, user) @@ -503,15 +574,9 @@ test "it does not send notification to mentioned users in announces" do assert [] == enabled_receivers assert [other_user] == disabled_receivers - - assert 1 == length(Repo.all(Notification)) - refute called(Push.send(:_)) end - test_with_mock "it returns notification-muting recipient in disabled recipients list", - Push, - [:passthrough], - [] do + test "it returns notification-muting recipient in disabled recipients list" do user = insert(:user) other_user = insert(:user) {:ok, _user_relationships} = User.mute(other_user, user) @@ -522,15 +587,9 @@ test "it does not send notification to mentioned users in announces" do assert [] == enabled_receivers assert [other_user] == disabled_receivers - - assert 1 == length(Repo.all(Notification)) - refute called(Push.send(:_)) end - test_with_mock "it returns thread-muting recipient in disabled recipients list", - Push, - [:passthrough], - [] do + test "it returns thread-muting recipient in disabled recipients list" do user = insert(:user) other_user = insert(:user) @@ -549,30 +608,6 @@ test "it does not send notification to mentioned users in announces" do assert [other_user] == disabled_receivers refute other_user in enabled_receivers - - [pre_mute_notification, post_mute_notification] = - Repo.all(from(n in Notification, where: n.user_id == ^other_user.id, order_by: n.id)) - - pre_mute_notification_id = pre_mute_notification.id - post_mute_notification_id = post_mute_notification.id - - assert called( - Push.send( - :meck.is(fn - %Notification{id: ^pre_mute_notification_id} -> true - _ -> false - end) - ) - ) - - refute called( - Push.send( - :meck.is(fn - %Notification{id: ^post_mute_notification_id} -> true - _ -> false - end) - ) - ) end end @@ -820,7 +855,7 @@ test "it doesn't return notifications for blocked user" do assert Notification.for_user(user) == [] end - test "it doesn't return notificatitons for blocked domain" do + test "it doesn't return notifications for blocked domain" do user = insert(:user) blocked = insert(:user, ap_id: "http://some-domain.com") {:ok, user} = User.block_domain(user, "some-domain.com") From 4705590f76f6a875aa99f32c8b08c20d793470a8 Mon Sep 17 00:00:00 2001 From: Cevado Date: Mon, 16 Mar 2020 22:02:01 -0300 Subject: [PATCH 06/23] Fix ssl option on Ecto config breaking release To use `:ssl` option on Ecto config it's required to include Erlang ssl application, this prevents releases to start when `:ssl` option is set to true. --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index dd598345c..890979f8b 100644 --- a/mix.exs +++ b/mix.exs @@ -63,7 +63,7 @@ def copy_nginx_config(%{path: target_path} = release) do def application do [ mod: {Pleroma.Application, []}, - extra_applications: [:logger, :runtime_tools, :comeonin, :quack, :fast_sanitize], + extra_applications: [:logger, :runtime_tools, :comeonin, :quack, :fast_sanitize, :ssl], included_applications: [:ex_syslogger] ] end From 981e015f1b68c7cf807b0ddbf3948809f11b7fff Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sun, 22 Mar 2020 17:10:37 +0300 Subject: [PATCH 07/23] Mastodon API Account view: Remove an outdated hack The hack with caching the follow relationship was introduced when we still were storing it inside the follow activity, resulting in slow queries. Now we store follow state in `FollowRelationship` table, so this is no longer necessary. --- lib/pleroma/user.ex | 18 ------------------ lib/pleroma/web/activity_pub/activity_pub.ex | 3 +-- lib/pleroma/web/activity_pub/utils.ex | 5 +---- .../web/mastodon_api/views/account_view.ex | 13 +++---------- 4 files changed, 5 insertions(+), 34 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 8693c0b80..12c2ad815 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -292,24 +292,6 @@ def ap_followers(%User{} = user), do: "#{ap_id(user)}/followers" def ap_following(%User{following_address: fa}) when is_binary(fa), do: fa def ap_following(%User{} = user), do: "#{ap_id(user)}/following" - def follow_state(%User{} = user, %User{} = target) do - case Utils.fetch_latest_follow(user, target) do - %{data: %{"state" => state}} -> state - # Ideally this would be nil, but then Cachex does not commit the value - _ -> false - end - end - - def get_cached_follow_state(user, target) do - key = "follow_state:#{user.ap_id}|#{target.ap_id}" - Cachex.fetch!(:user_cache, key, fn _ -> {:commit, follow_state(user, target)} end) - end - - @spec set_follow_state_cache(String.t(), String.t(), String.t()) :: {:ok | :error, boolean()} - def set_follow_state_cache(user_ap_id, target_ap_id, state) do - Cachex.put(:user_cache, "follow_state:#{user_ap_id}|#{target_ap_id}", state) - end - @spec restrict_deactivated(Ecto.Query.t()) :: Ecto.Query.t() def restrict_deactivated(query) do from(u in query, where: u.deactivated != ^true) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index d9f74b6a4..30e282840 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -503,8 +503,7 @@ def follow(follower, followed, activity_id \\ nil, local \\ true) do defp do_follow(follower, followed, activity_id, local) do with data <- make_follow_data(follower, followed, activity_id), {:ok, activity} <- insert(data, local), - :ok <- maybe_federate(activity), - _ <- User.set_follow_state_cache(follower.ap_id, followed.ap_id, activity.data["state"]) do + :ok <- maybe_federate(activity) do {:ok, activity} else {:error, error} -> Repo.rollback(error) diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 15dd2ed45..c65bbed67 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -440,22 +440,19 @@ def update_follow_state_for_all( |> update(set: [data: fragment("jsonb_set(data, '{state}', ?)", ^state)]) |> Repo.update_all([]) - User.set_follow_state_cache(actor, object, state) - activity = Activity.get_by_id(activity.id) {:ok, activity} end def update_follow_state( - %Activity{data: %{"actor" => actor, "object" => object}} = activity, + %Activity{} = activity, state ) do new_data = Map.put(activity.data, "state", state) changeset = Changeset.change(activity, data: new_data) with {:ok, activity} <- Repo.update(changeset) do - User.set_follow_state_cache(actor, object, state) {:ok, activity} end end diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index 341dc2c91..4ebce73b4 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -36,25 +36,18 @@ def render("relationship.json", %{user: nil, target: _target}) do end def render("relationship.json", %{user: %User{} = user, target: %User{} = target}) do - follow_state = User.get_cached_follow_state(user, target) - - requested = - if follow_state && !User.following?(user, target) do - follow_state == "pending" - else - false - end + follow_state = User.get_follow_state(user, target) %{ id: to_string(target.id), - following: User.following?(user, target), + following: follow_state == "accept", followed_by: User.following?(target, user), blocking: User.blocks_user?(user, target), blocked_by: User.blocks_user?(target, user), muting: User.mutes?(user, target), muting_notifications: User.muted_notifications?(user, target), subscribing: User.subscribed_to?(user, target), - requested: requested, + requested: follow_state == "pending", domain_blocking: User.blocks_domain?(user, target), showing_reblogs: User.showing_reblogs?(user, target), endorsed: false From 15be6ba9c200b2a4ae153d26876be1b5cbb6357e Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Sun, 22 Mar 2020 16:38:12 +0100 Subject: [PATCH 08/23] AccountView: fix for other forms of
in bio Closes: https://git.pleroma.social/pleroma/pleroma/issues/1643 --- lib/pleroma/web/mastodon_api/views/account_view.ex | 2 +- .../account_controller/update_credentials_test.exs | 4 ++-- test/web/mastodon_api/views/account_view_test.exs | 7 ++++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index 4ebce73b4..2bf711386 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -115,7 +115,7 @@ defp do_render("show.json", %{user: user} = opts) do fields: user.fields, bot: bot, source: %{ - note: Pleroma.HTML.strip_tags((user.bio || "") |> String.replace("
", "\n")), + note: (user.bio || "") |> String.replace(~r(
), "\n") |> Pleroma.HTML.strip_tags(), sensitive: false, fields: user.raw_fields, pleroma: %{ diff --git a/test/web/mastodon_api/controllers/account_controller/update_credentials_test.exs b/test/web/mastodon_api/controllers/account_controller/update_credentials_test.exs index 43538cb17..51cebe567 100644 --- a/test/web/mastodon_api/controllers/account_controller/update_credentials_test.exs +++ b/test/web/mastodon_api/controllers/account_controller/update_credentials_test.exs @@ -76,7 +76,7 @@ test "updates the user's bio", %{conn: conn} do conn = patch(conn, "/api/v1/accounts/update_credentials", %{ - "note" => "I drink #cofe with @#{user2.nickname}" + "note" => "I drink #cofe with @#{user2.nickname}\n\nsuya.." }) assert user_data = json_response(conn, 200) @@ -84,7 +84,7 @@ test "updates the user's bio", %{conn: conn} do assert user_data["note"] == ~s(I drink #cofe with @#{user2.nickname}) + }" class="u-url mention" href="#{user2.ap_id}" rel="ugc">@#{user2.nickname}

suya..) end test "updates the user's locking status", %{conn: conn} do diff --git a/test/web/mastodon_api/views/account_view_test.exs b/test/web/mastodon_api/views/account_view_test.exs index d60ed7b64..983886c6b 100644 --- a/test/web/mastodon_api/views/account_view_test.exs +++ b/test/web/mastodon_api/views/account_view_test.exs @@ -32,7 +32,8 @@ test "Represent a user account" do background: background_image, nickname: "shp@shitposter.club", name: ":karjalanpiirakka: shp", - bio: "valid html", + bio: + "valid html. a
b
c
d
f", inserted_at: ~N[2017-08-15 15:47:06.597036] }) @@ -46,7 +47,7 @@ test "Represent a user account" do followers_count: 3, following_count: 0, statuses_count: 5, - note: "valid html", + note: "valid html. a
b
c
d
f", url: user.ap_id, avatar: "http://localhost:4001/images/avi.png", avatar_static: "http://localhost:4001/images/avi.png", @@ -63,7 +64,7 @@ test "Represent a user account" do fields: [], bot: false, source: %{ - note: "valid html", + note: "valid html. a\nb\nc\nd\nf", sensitive: false, pleroma: %{ actor_type: "Person", From c2e415143b1dfe5d89eff06fbce6840c445aa5fa Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 22 Mar 2020 21:51:44 +0300 Subject: [PATCH 09/23] WIP: preloading of user relations for timeline/statuses rendering (performance improvement). --- lib/pleroma/user.ex | 6 +- lib/pleroma/user_relationship.ex | 44 ++++++++++++ .../web/mastodon_api/views/account_view.ex | 69 ++++++++++++++++--- .../web/mastodon_api/views/status_view.ex | 60 ++++++++++++++-- 4 files changed, 160 insertions(+), 19 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 12c2ad815..daaa6d86b 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -1642,8 +1642,12 @@ def all_superusers do |> Repo.all() end + def muting_reblogs?(%User{} = user, %User{} = target) do + UserRelationship.reblog_mute_exists?(user, target) + end + def showing_reblogs?(%User{} = user, %User{} = target) do - not UserRelationship.reblog_mute_exists?(user, target) + not muting_reblogs?(user, target) end @doc """ diff --git a/lib/pleroma/user_relationship.ex b/lib/pleroma/user_relationship.ex index 393947942..167a3919c 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 FlakeId.Ecto.CompatType alias Pleroma.Repo alias Pleroma.User alias Pleroma.UserRelationship @@ -34,6 +35,10 @@ def unquote(:"#{relationship_type}_exists?")(source, target), do: exists?(unquote(relationship_type), source, target) end + def user_relationship_types, do: Keyword.keys(user_relationship_mappings()) + + def user_relationship_mappings, do: UserRelationshipTypeEnum.__enum_map__() + def changeset(%UserRelationship{} = user_relationship, params \\ %{}) do user_relationship |> cast(params, [:relationship_type, :source_id, :target_id]) @@ -72,6 +77,45 @@ def delete(relationship_type, %User{} = source, %User{} = target) do end end + def dictionary( + source_users, + target_users, + source_to_target_rel_types \\ nil, + target_to_source_rel_types \\ nil + ) + when is_list(source_users) and is_list(target_users) do + get_bin_ids = fn user -> + with {:ok, bin_id} <- CompatType.dump(user.id), do: bin_id + end + + source_user_ids = Enum.map(source_users, &get_bin_ids.(&1)) + target_user_ids = Enum.map(target_users, &get_bin_ids.(&1)) + + get_rel_type_codes = fn rel_type -> user_relationship_mappings()[rel_type] end + + source_to_target_rel_types = + Enum.map(source_to_target_rel_types || user_relationship_types(), &get_rel_type_codes.(&1)) + + target_to_source_rel_types = + Enum.map(target_to_source_rel_types || user_relationship_types(), &get_rel_type_codes.(&1)) + + __MODULE__ + |> where( + fragment( + "(source_id = ANY(?) AND target_id = ANY(?) AND relationship_type = ANY(?)) OR \ + (source_id = ANY(?) AND target_id = ANY(?) AND relationship_type = ANY(?))", + ^source_user_ids, + ^target_user_ids, + ^source_to_target_rel_types, + ^target_user_ids, + ^source_user_ids, + ^target_to_source_rel_types + ) + ) + |> select([ur], [ur.relationship_type, ur.source_id, ur.target_id]) + |> Repo.all() + 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 4ebce73b4..15a579278 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -10,6 +10,19 @@ defmodule Pleroma.Web.MastodonAPI.AccountView do alias Pleroma.Web.MastodonAPI.AccountView alias Pleroma.Web.MediaProxy + def test_rel(user_relationships, rel_type, source, target, func) do + cond do + is_nil(source) or is_nil(target) -> + false + + user_relationships -> + [rel_type, source.id, target.id] in user_relationships + + true -> + func.(source, target) + end + end + def render("index.json", %{users: users} = opts) do users |> render_many(AccountView, "show.json", opts) @@ -35,21 +48,50 @@ def render("relationship.json", %{user: nil, target: _target}) do %{} end - def render("relationship.json", %{user: %User{} = user, target: %User{} = target}) do - follow_state = User.get_follow_state(user, target) + def render( + "relationship.json", + %{user: %User{} = reading_user, target: %User{} = target} = opts + ) do + user_relationships = Map.get(opts, :user_relationships) + follow_state = User.get_follow_state(reading_user, target) + + # TODO: add a note on adjusting StatusView.user_relationships_opt/1 re: preloading of user relations %{ id: to_string(target.id), following: follow_state == "accept", - followed_by: User.following?(target, user), - blocking: User.blocks_user?(user, target), - blocked_by: User.blocks_user?(target, user), - muting: User.mutes?(user, target), - muting_notifications: User.muted_notifications?(user, target), - subscribing: User.subscribed_to?(user, target), + followed_by: User.following?(target, reading_user), + blocking: + test_rel(user_relationships, :block, reading_user, target, &User.blocks_user?(&1, &2)), + blocked_by: + test_rel(user_relationships, :block, target, reading_user, &User.blocks_user?(&1, &2)), + muting: test_rel(user_relationships, :mute, reading_user, target, &User.mutes?(&1, &2)), + muting_notifications: + test_rel( + user_relationships, + :notification_mute, + reading_user, + target, + &User.muted_notifications?(&1, &2) + ), + subscribing: + test_rel( + user_relationships, + :inverse_subscription, + target, + reading_user, + &User.subscribed_to?(&2, &1) + ), requested: follow_state == "pending", - domain_blocking: User.blocks_domain?(user, target), - showing_reblogs: User.showing_reblogs?(user, target), + domain_blocking: User.blocks_domain?(reading_user, target), + showing_reblogs: + not test_rel( + user_relationships, + :reblog_mute, + reading_user, + target, + &User.muting_reblogs?(&1, &2) + ), endorsed: false } end @@ -93,7 +135,12 @@ defp do_render("show.json", %{user: user} = opts) do } end) - relationship = render("relationship.json", %{user: opts[:for], target: user}) + relationship = + render("relationship.json", %{ + user: opts[:for], + target: user, + user_relationships: opts[:user_relationships] + }) %{ id: to_string(user.id), diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index f7469cdff..e0c368ec9 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -13,6 +13,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusView 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 @@ -70,11 +71,34 @@ defp reblogged?(activity, user) do present?(user && user.ap_id in (object.data["announcements"] || [])) end - def render("index.json", opts) do - replied_to_activities = get_replied_to_activities(opts.activities) - opts = Map.put(opts, :replied_to_activities, replied_to_activities) + defp user_relationships_opt(opts) do + reading_user = opts[:for] - safe_render_many(opts.activities, StatusView, "show.json", opts) + if reading_user do + activities = opts[:activities] + actors = Enum.map(activities, fn a -> get_user(a.data["actor"]) end) + + UserRelationship.dictionary( + [reading_user], + actors, + [:block, :mute, :notification_mute, :reblog_mute], + [:block, :inverse_subscription] + ) + else + [] + end + end + + def render("index.json", opts) do + activities = opts.activities + replied_to_activities = get_replied_to_activities(activities) + + opts = + opts + |> Map.put(:replied_to_activities, replied_to_activities) + |> Map.put(:user_relationships, user_relationships_opt(opts)) + + safe_render_many(activities, StatusView, "show.json", opts) end def render( @@ -107,7 +131,12 @@ def render( id: to_string(activity.id), uri: activity_object.data["id"], url: activity_object.data["id"], - account: AccountView.render("show.json", %{user: user, for: opts[:for]}), + account: + AccountView.render("show.json", %{ + user: user, + for: opts[:for], + user_relationships: opts[:user_relationships] + }), in_reply_to_id: nil, in_reply_to_account_id: nil, reblog: reblogged, @@ -253,11 +282,28 @@ def render("show.json", %{activity: %{data: %{"object" => _object}} = activity} _ -> [] end + user_relationships_opt = opts[:user_relationships] + + muted = + thread_muted? || + Pleroma.Web.MastodonAPI.AccountView.test_rel( + user_relationships_opt, + :mute, + opts[:for], + user, + fn for_user, user -> User.mutes?(for_user, user) end + ) + %{ id: to_string(activity.id), uri: object.data["id"], url: url, - account: AccountView.render("show.json", %{user: user, for: opts[:for]}), + account: + AccountView.render("show.json", %{ + user: user, + for: opts[:for], + user_relationships: user_relationships_opt + }), in_reply_to_id: reply_to && to_string(reply_to.id), in_reply_to_account_id: reply_to_user && to_string(reply_to_user.id), reblog: nil, @@ -270,7 +316,7 @@ def render("show.json", %{activity: %{data: %{"object" => _object}} = activity} reblogged: reblogged?(activity, opts[:for]), favourited: present?(favorited), bookmarked: present?(bookmarked), - muted: thread_muted? || User.mutes?(opts[:for], user), + muted: muted, pinned: pinned?(activity, user), sensitive: sensitive, spoiler_text: summary, From 3c78e5f3275494b3dc4546e65f19eb3a3c97033a Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Mon, 23 Mar 2020 12:01:11 +0300 Subject: [PATCH 10/23] Preloading of follow relations for timeline/statuses rendering (performance improvement). Refactoring. --- lib/pleroma/following_relationship.ex | 26 +++++++ lib/pleroma/user.ex | 7 ++ lib/pleroma/user_relationship.ex | 13 ++++ .../web/mastodon_api/views/account_view.ex | 75 ++++++++++++++----- .../web/mastodon_api/views/status_view.ex | 46 +++++++----- 5 files changed, 130 insertions(+), 37 deletions(-) diff --git a/lib/pleroma/following_relationship.ex b/lib/pleroma/following_relationship.ex index a6d281151..dd1696136 100644 --- a/lib/pleroma/following_relationship.ex +++ b/lib/pleroma/following_relationship.ex @@ -129,4 +129,30 @@ def move_following(origin, target) do move_following(origin, target) end end + + def all_between_user_sets( + source_users, + target_users + ) + when is_list(source_users) and is_list(target_users) do + get_bin_ids = fn user -> + with {:ok, bin_id} <- CompatType.dump(user.id), do: bin_id + end + + source_user_ids = Enum.map(source_users, &get_bin_ids.(&1)) + target_user_ids = Enum.map(target_users, &get_bin_ids.(&1)) + + __MODULE__ + |> where( + fragment( + "(follower_id = ANY(?) AND following_id = ANY(?)) OR \ + (follower_id = ANY(?) AND following_id = ANY(?))", + ^source_user_ids, + ^target_user_ids, + ^target_user_ids, + ^source_user_ids + ) + ) + |> Repo.all() + end end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index daaa6d86b..eb72755a0 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -674,7 +674,14 @@ def unfollow(%User{} = follower, %User{} = followed) do def get_follow_state(%User{} = follower, %User{} = following) do following_relationship = FollowingRelationship.get(follower, following) + get_follow_state(follower, following, following_relationship) + end + def get_follow_state( + %User{} = follower, + %User{} = following, + following_relationship + ) do case {following_relationship, following.local} do {nil, false} -> case Utils.fetch_latest_follow(follower, following) do diff --git a/lib/pleroma/user_relationship.ex b/lib/pleroma/user_relationship.ex index 167a3919c..9423e3a42 100644 --- a/lib/pleroma/user_relationship.ex +++ b/lib/pleroma/user_relationship.ex @@ -116,6 +116,19 @@ def dictionary( |> Repo.all() end + def exists?(dictionary, rel_type, source, target, func) do + cond do + is_nil(source) or is_nil(target) -> + false + + dictionary -> + [rel_type, source.id, target.id] in dictionary + + true -> + func.(source, target) + end + 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 15a579278..2fe46158b 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -6,21 +6,15 @@ defmodule Pleroma.Web.MastodonAPI.AccountView do use Pleroma.Web, :view alias Pleroma.User + alias Pleroma.UserRelationship alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.MastodonAPI.AccountView alias Pleroma.Web.MediaProxy - def test_rel(user_relationships, rel_type, source, target, func) do - cond do - is_nil(source) or is_nil(target) -> - false - - user_relationships -> - [rel_type, source.id, target.id] in user_relationships - - true -> - func.(source, target) - end + 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 @@ -53,21 +47,61 @@ def render( %{user: %User{} = reading_user, target: %User{} = target} = opts ) do user_relationships = Map.get(opts, :user_relationships) + following_relationships = opts[:following_relationships] - follow_state = User.get_follow_state(reading_user, target) + follow_state = + if following_relationships do + user_to_target_following_relation = + find_following_rel(following_relationships, reading_user, target) + + User.get_follow_state(reading_user, target, user_to_target_following_relation) + else + User.get_follow_state(reading_user, target) + end + + followed_by = + if following_relationships do + with %{state: "accept"} <- + find_following_rel(following_relationships, target, reading_user) do + true + else + _ -> false + end + else + User.following?(target, reading_user) + end # TODO: add a note on adjusting StatusView.user_relationships_opt/1 re: preloading of user relations %{ id: to_string(target.id), following: follow_state == "accept", - followed_by: User.following?(target, reading_user), + followed_by: followed_by, blocking: - test_rel(user_relationships, :block, reading_user, target, &User.blocks_user?(&1, &2)), + UserRelationship.exists?( + user_relationships, + :block, + reading_user, + target, + &User.blocks_user?(&1, &2) + ), blocked_by: - test_rel(user_relationships, :block, target, reading_user, &User.blocks_user?(&1, &2)), - muting: test_rel(user_relationships, :mute, reading_user, target, &User.mutes?(&1, &2)), + UserRelationship.exists?( + user_relationships, + :block, + target, + reading_user, + &User.blocks_user?(&1, &2) + ), + muting: + UserRelationship.exists?( + user_relationships, + :mute, + reading_user, + target, + &User.mutes?(&1, &2) + ), muting_notifications: - test_rel( + UserRelationship.exists?( user_relationships, :notification_mute, reading_user, @@ -75,7 +109,7 @@ def render( &User.muted_notifications?(&1, &2) ), subscribing: - test_rel( + UserRelationship.exists?( user_relationships, :inverse_subscription, target, @@ -85,7 +119,7 @@ def render( requested: follow_state == "pending", domain_blocking: User.blocks_domain?(reading_user, target), showing_reblogs: - not test_rel( + not UserRelationship.exists?( user_relationships, :reblog_mute, reading_user, @@ -139,7 +173,8 @@ defp do_render("show.json", %{user: user} = opts) do render("relationship.json", %{ user: opts[:for], target: user, - user_relationships: opts[:user_relationships] + user_relationships: opts[:user_relationships], + following_relationships: opts[:following_relationships] }) %{ diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index e0c368ec9..55a5513f9 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -9,6 +9,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do alias Pleroma.Activity alias Pleroma.ActivityExpiration + alias Pleroma.FollowingRelationship alias Pleroma.HTML alias Pleroma.Object alias Pleroma.Repo @@ -71,22 +72,31 @@ defp reblogged?(activity, user) do present?(user && user.ap_id in (object.data["announcements"] || [])) end - defp user_relationships_opt(opts) do + defp relationships_opts(opts) do reading_user = opts[:for] - if reading_user do - activities = opts[:activities] - actors = Enum.map(activities, fn a -> get_user(a.data["actor"]) end) + {user_relationships, following_relationships} = + if reading_user do + activities = opts[:activities] + actors = Enum.map(activities, fn a -> get_user(a.data["actor"]) end) - UserRelationship.dictionary( - [reading_user], - actors, - [:block, :mute, :notification_mute, :reblog_mute], - [:block, :inverse_subscription] - ) - else - [] - end + 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, following_relationships} + else + {[], []} + end + + %{user_relationships: user_relationships, following_relationships: following_relationships} end def render("index.json", opts) do @@ -96,7 +106,7 @@ def render("index.json", opts) do opts = opts |> Map.put(:replied_to_activities, replied_to_activities) - |> Map.put(:user_relationships, user_relationships_opt(opts)) + |> Map.merge(relationships_opts(opts)) safe_render_many(activities, StatusView, "show.json", opts) end @@ -135,7 +145,8 @@ def render( AccountView.render("show.json", %{ user: user, for: opts[:for], - user_relationships: opts[:user_relationships] + user_relationships: opts[:user_relationships], + following_relationships: opts[:following_relationships] }), in_reply_to_id: nil, in_reply_to_account_id: nil, @@ -286,7 +297,7 @@ def render("show.json", %{activity: %{data: %{"object" => _object}} = activity} muted = thread_muted? || - Pleroma.Web.MastodonAPI.AccountView.test_rel( + UserRelationship.exists?( user_relationships_opt, :mute, opts[:for], @@ -302,7 +313,8 @@ def render("show.json", %{activity: %{data: %{"object" => _object}} = activity} AccountView.render("show.json", %{ user: user, for: opts[:for], - user_relationships: user_relationships_opt + user_relationships: user_relationships_opt, + following_relationships: opts[:following_relationships] }), in_reply_to_id: reply_to && to_string(reply_to.id), in_reply_to_account_id: reply_to_user && to_string(reply_to_user.id), From 5a34dca8eda46479a3459b60c623d6fa94fc662b Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 23 Mar 2020 14:03:31 +0400 Subject: [PATCH 11/23] Add emoji support in statuses in staticfe --- lib/pleroma/web/static_fe/static_fe_controller.ex | 4 +++- priv/static/static/static-fe.css | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/static_fe/static_fe_controller.ex b/lib/pleroma/web/static_fe/static_fe_controller.ex index 7f9464268..7a35238d7 100644 --- a/lib/pleroma/web/static_fe/static_fe_controller.ex +++ b/lib/pleroma/web/static_fe/static_fe_controller.ex @@ -60,7 +60,9 @@ defp represent(%Activity{object: %Object{data: data}} = activity, selected) do content = if data["content"] do - Pleroma.HTML.filter_tags(data["content"]) + data["content"] + |> Pleroma.HTML.filter_tags() + |> Pleroma.Emoji.Formatter.emojify(Map.get(data, "emoji", %{})) else nil end diff --git a/priv/static/static/static-fe.css b/priv/static/static/static-fe.css index 19c56387b..db61ff266 100644 --- a/priv/static/static/static-fe.css +++ b/priv/static/static/static-fe.css @@ -174,3 +174,10 @@ .alert-info { font-weight: 500; font-size: 16px; } + +img.emoji { + width: 32px; + height: 32px; + padding: 0; + vertical-align: middle; +} From eec1fcaf55bdcbc2d3aed4eaf044bb8ef6c4effa Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 23 Mar 2020 15:58:55 +0100 Subject: [PATCH 12/23] Home timeline tests: Add failing test for relationships --- .../controllers/timeline_controller_test.exs | 57 +++++++++++++++++-- 1 file changed, 53 insertions(+), 4 deletions(-) diff --git a/test/web/mastodon_api/controllers/timeline_controller_test.exs b/test/web/mastodon_api/controllers/timeline_controller_test.exs index 6fedb4223..47849fc48 100644 --- a/test/web/mastodon_api/controllers/timeline_controller_test.exs +++ b/test/web/mastodon_api/controllers/timeline_controller_test.exs @@ -21,9 +21,12 @@ defmodule Pleroma.Web.MastodonAPI.TimelineControllerTest do setup do: oauth_access(["read:statuses"]) test "the home timeline", %{user: user, conn: conn} do - following = insert(:user) + following = insert(:user, nickname: "followed") + third_user = insert(:user, nickname: "repeated") - {:ok, _activity} = CommonAPI.post(following, %{"status" => "test"}) + {:ok, _activity} = CommonAPI.post(following, %{"status" => "post"}) + {:ok, activity} = CommonAPI.post(third_user, %{"status" => "repeated post"}) + {:ok, _, _} = CommonAPI.repeat(activity.id, following) ret_conn = get(conn, "/api/v1/timelines/home") @@ -31,9 +34,55 @@ test "the home timeline", %{user: user, conn: conn} do {:ok, _user} = User.follow(user, following) - conn = get(conn, "/api/v1/timelines/home") + ret_conn = get(conn, "/api/v1/timelines/home") - assert [%{"content" => "test"}] = json_response(conn, :ok) + assert [ + %{ + "reblog" => %{ + "content" => "repeated post", + "account" => %{ + "pleroma" => %{ + "relationship" => %{"following" => false, "followed_by" => false} + } + } + }, + "account" => %{"pleroma" => %{"relationship" => %{"following" => true}}} + }, + %{ + "content" => "post", + "account" => %{ + "acct" => "followed", + "pleroma" => %{"relationship" => %{"following" => true}} + } + } + ] = json_response(ret_conn, :ok) + + {:ok, _user} = User.follow(third_user, user) + + ret_conn = get(conn, "/api/v1/timelines/home") + + assert [ + %{ + "reblog" => %{ + "content" => "repeated post", + "account" => %{ + "acct" => "repeated", + "pleroma" => %{ + # This part does not match correctly + "relationship" => %{"following" => false, "followed_by" => true} + } + } + }, + "account" => %{"pleroma" => %{"relationship" => %{"following" => true}}} + }, + %{ + "content" => "post", + "account" => %{ + "acct" => "followed", + "pleroma" => %{"relationship" => %{"following" => true}} + } + } + ] = json_response(ret_conn, :ok) end test "the home timeline when the direct messages are excluded", %{user: user, conn: conn} do From 3bd2829e5c125f961b7508bf40ef534a21070562 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 23 Mar 2020 18:56:01 +0100 Subject: [PATCH 13/23] Benchmarks: Add timeline benchmark --- benchmarks/load_testing/generator.ex | 3 +- .../mix/tasks/pleroma/benchmarks/timelines.ex | 76 +++++++++++++++++++ lib/pleroma/web/controller_helper.ex | 7 +- 3 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 benchmarks/mix/tasks/pleroma/benchmarks/timelines.ex diff --git a/benchmarks/load_testing/generator.ex b/benchmarks/load_testing/generator.ex index 3f88fefd7..17e89c13c 100644 --- a/benchmarks/load_testing/generator.ex +++ b/benchmarks/load_testing/generator.ex @@ -22,9 +22,10 @@ def generate_like_activities(user, posts) do def generate_users(opts) do IO.puts("Starting generating #{opts[:users_max]} users...") - {time, _} = :timer.tc(fn -> do_generate_users(opts) end) + {time, users} = :timer.tc(fn -> do_generate_users(opts) end) IO.puts("Inserting users take #{to_sec(time)} sec.\n") + users end defp do_generate_users(opts) do diff --git a/benchmarks/mix/tasks/pleroma/benchmarks/timelines.ex b/benchmarks/mix/tasks/pleroma/benchmarks/timelines.ex new file mode 100644 index 000000000..dc6f3d3fc --- /dev/null +++ b/benchmarks/mix/tasks/pleroma/benchmarks/timelines.ex @@ -0,0 +1,76 @@ +defmodule Mix.Tasks.Pleroma.Benchmarks.Timelines do + use Mix.Task + alias Pleroma.Repo + alias Pleroma.LoadTesting.Generator + + alias Pleroma.Web.CommonAPI + + def run(_args) do + Mix.Pleroma.start_pleroma() + + # Cleaning tables + clean_tables() + + [{:ok, user} | users] = Generator.generate_users(users_max: 1000) + + # Let the user make 100 posts + + 1..100 + |> Enum.each(fn i -> CommonAPI.post(user, %{"status" => to_string(i)}) end) + + # Let 10 random users post + posts = + users + |> Enum.take_random(10) + |> Enum.map(fn {:ok, random_user} -> + {:ok, activity} = CommonAPI.post(random_user, %{"status" => "."}) + activity + end) + + # let our user repeat them + posts + |> Enum.each(fn activity -> + CommonAPI.repeat(activity.id, user) + end) + + Benchee.run( + %{ + "user timeline, no followers" => fn reading_user -> + conn = + Phoenix.ConnTest.build_conn() + |> Plug.Conn.assign(:user, reading_user) + |> Plug.Conn.assign(:skip_link_headers, true) + + Pleroma.Web.MastodonAPI.AccountController.statuses(conn, %{"id" => user.id}) + end + }, + inputs: %{"user" => user, "no user" => nil}, + time: 60 + ) + + users + |> Enum.each(fn {:ok, follower} -> Pleroma.User.follow(follower, user) end) + + Benchee.run( + %{ + "user timeline, all following" => fn reading_user -> + conn = + Phoenix.ConnTest.build_conn() + |> Plug.Conn.assign(:user, reading_user) + |> Plug.Conn.assign(:skip_link_headers, true) + + Pleroma.Web.MastodonAPI.AccountController.statuses(conn, %{"id" => user.id}) + end + }, + inputs: %{"user" => user, "no user" => nil}, + time: 60 + ) + end + + defp clean_tables do + IO.puts("Deleting old data...\n") + Ecto.Adapters.SQL.query!(Repo, "TRUNCATE users CASCADE;") + Ecto.Adapters.SQL.query!(Repo, "TRUNCATE activities CASCADE;") + Ecto.Adapters.SQL.query!(Repo, "TRUNCATE objects CASCADE;") + end +end diff --git a/lib/pleroma/web/controller_helper.ex b/lib/pleroma/web/controller_helper.ex index ad293cda9..b49523ec3 100644 --- a/lib/pleroma/web/controller_helper.ex +++ b/lib/pleroma/web/controller_helper.ex @@ -34,7 +34,12 @@ defp param_to_integer(val, default) when is_binary(val) do defp param_to_integer(_, default), do: default - def add_link_headers(conn, activities, extra_params \\ %{}) do + def add_link_headers(conn, activities, extra_params \\ %{}) + + def add_link_headers(%{assigns: %{skip_link_headers: true}} = conn, _activities, _extra_params), + do: conn + + def add_link_headers(conn, activities, extra_params) do case List.last(activities) do %{id: max_id} -> params = From d1a9716a988fe9f670033ad46cc9637038fbd1e8 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Tue, 24 Mar 2020 17:38:18 +0400 Subject: [PATCH 14/23] Fix activity deletion --- lib/pleroma/web/activity_pub/activity_pub.ex | 10 ++++++++++ test/web/activity_pub/activity_pub_test.exs | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 30e282840..974231925 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -583,6 +583,16 @@ defp do_delete(%Object{data: %{"id" => id, "actor" => actor}} = object, options) end end + defp do_delete(%Object{data: %{"type" => "Tombstone", "id" => ap_id}}, _) do + activity = + ap_id + |> Activity.Queries.by_object_id() + |> Activity.Queries.by_type("Delete") + |> Repo.one() + + {:ok, activity} + end + @spec block(User.t(), User.t(), String.t() | nil, boolean()) :: {:ok, Activity.t()} | {:error, any()} def block(blocker, blocked, activity_id \\ nil, local \\ true) do diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index a43dd34f0..049b14498 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -1425,6 +1425,12 @@ test "it creates a delete activity and deletes the original object" do assert Repo.get(Object, object.id).data["type"] == "Tombstone" end + test "it doesn't fail when an activity was already deleted" do + {:ok, delete} = insert(:note_activity) |> Object.normalize() |> ActivityPub.delete() + + assert {:ok, ^delete} = delete |> Object.normalize() |> ActivityPub.delete() + end + test "decrements user note count only for public activities" do user = insert(:user, note_count: 10) From 13cbb9f6ada8dcb15bb7ed12be4d88a18c5db7f7 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 24 Mar 2020 22:14:26 +0300 Subject: [PATCH 15/23] Implemented preloading of relationships with parent activities' actors for statuses/timeline rendering. Applied preloading for notifications rendering. Fixed announces rendering issue (preloading-related). --- lib/pleroma/activity/queries.ex | 7 ++ .../web/mastodon_api/views/account_view.ex | 15 ++- .../mastodon_api/views/notification_view.ex | 98 +++++++++++++++---- .../web/mastodon_api/views/status_view.ex | 85 +++++++++------- .../controllers/timeline_controller_test.exs | 1 - 5 files changed, 138 insertions(+), 68 deletions(-) diff --git a/lib/pleroma/activity/queries.ex b/lib/pleroma/activity/queries.ex index 04593b9fb..a34c20343 100644 --- a/lib/pleroma/activity/queries.ex +++ b/lib/pleroma/activity/queries.ex @@ -35,6 +35,13 @@ def by_author(query \\ Activity, %User{ap_id: ap_id}) do from(a in query, where: a.actor == ^ap_id) end + def find_by_object_ap_id(activities, object_ap_id) do + Enum.find( + activities, + &(object_ap_id in [is_map(&1.data["object"]) && &1.data["object"]["id"], &1.data["object"]]) + ) + end + @spec by_object_id(query, String.t() | [String.t()]) :: query def by_object_id(query \\ Activity, object_id) diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index 2fe46158b..89bea9957 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -46,8 +46,8 @@ def render( "relationship.json", %{user: %User{} = reading_user, target: %User{} = target} = opts ) do - user_relationships = Map.get(opts, :user_relationships) - following_relationships = opts[:following_relationships] + user_relationships = get_in(opts, [:relationships, :user_relationships]) + following_relationships = get_in(opts, [:relationships, :following_relationships]) follow_state = if following_relationships do @@ -61,17 +61,15 @@ def render( followed_by = if following_relationships do - with %{state: "accept"} <- - find_following_rel(following_relationships, target, reading_user) do - true - else + case find_following_rel(following_relationships, target, reading_user) do + %{state: "accept"} -> true _ -> false end else User.following?(target, reading_user) end - # TODO: add a note on adjusting StatusView.user_relationships_opt/1 re: preloading of user relations + # NOTE: adjust StatusView.relationships_opts/2 if adding new relation-related flags %{ id: to_string(target.id), following: follow_state == "accept", @@ -173,8 +171,7 @@ defp do_render("show.json", %{user: user} = opts) do render("relationship.json", %{ user: opts[:for], target: user, - user_relationships: opts[:user_relationships], - following_relationships: opts[:following_relationships] + relationships: opts[:relationships] }) %{ diff --git a/lib/pleroma/web/mastodon_api/views/notification_view.ex b/lib/pleroma/web/mastodon_api/views/notification_view.ex index 33145c484..e9c618496 100644 --- a/lib/pleroma/web/mastodon_api/views/notification_view.ex +++ b/lib/pleroma/web/mastodon_api/views/notification_view.ex @@ -13,19 +13,68 @@ defmodule Pleroma.Web.MastodonAPI.NotificationView do alias Pleroma.Web.MastodonAPI.NotificationView alias Pleroma.Web.MastodonAPI.StatusView - def render("index.json", %{notifications: notifications, for: user}) do - safe_render_many(notifications, NotificationView, "show.json", %{for: user}) + def render("index.json", %{notifications: notifications, for: reading_user}) do + activities = Enum.map(notifications, & &1.activity) + + parent_activities = + activities + |> Enum.filter( + &(Activity.mastodon_notification_type(&1) in [ + "favourite", + "reblog", + "pleroma:emoji_reaction" + ]) + ) + |> Enum.map(& &1.data["object"]) + |> Activity.create_by_object_ap_id() + |> 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"])) + + actors = + activities + |> Enum.map(fn a -> User.get_cached_by_ap_id(a.data["actor"]) end) + |> Enum.filter(& &1) + |> Kernel.++(move_activities_targets) + + opts = %{ + for: reading_user, + parent_activities: parent_activities, + relationships: StatusView.relationships_opts(reading_user, actors) + } + + safe_render_many(notifications, NotificationView, "show.json", opts) end - def render("show.json", %{ - notification: %Notification{activity: activity} = notification, - for: user - }) do + def render( + "show.json", + %{ + notification: %Notification{activity: activity} = notification, + for: reading_user + } = opts + ) do actor = User.get_cached_by_ap_id(activity.data["actor"]) - parent_activity = Activity.get_create_by_object_ap_id(activity.data["object"]) + + parent_activity_fn = fn -> + if opts[:parent_activities] do + Activity.Queries.find_by_object_ap_id(opts[:parent_activities], activity.data["object"]) + else + Activity.get_create_by_object_ap_id(activity.data["object"]) + end + end + mastodon_type = Activity.mastodon_notification_type(activity) - with %{id: _} = account <- AccountView.render("show.json", %{user: actor, for: user}) do + with %{id: _} = account <- + AccountView.render("show.json", %{ + user: actor, + for: reading_user, + relationships: opts[:relationships] + }) do response = %{ id: to_string(notification.id), type: mastodon_type, @@ -36,24 +85,28 @@ def render("show.json", %{ } } + relationships_opts = %{relationships: opts[:relationships]} + case mastodon_type do "mention" -> - put_status(response, activity, user) + put_status(response, activity, reading_user, relationships_opts) "favourite" -> - put_status(response, parent_activity, user) + put_status(response, parent_activity_fn.(), reading_user, relationships_opts) "reblog" -> - put_status(response, parent_activity, user) + put_status(response, parent_activity_fn.(), reading_user, relationships_opts) "move" -> - put_target(response, activity, user) + put_target(response, activity, reading_user, relationships_opts) "follow" -> response "pleroma:emoji_reaction" -> - put_status(response, parent_activity, user) |> put_emoji(activity) + response + |> put_status(parent_activity_fn.(), reading_user, relationships_opts) + |> put_emoji(activity) _ -> nil @@ -64,16 +117,21 @@ def render("show.json", %{ end defp put_emoji(response, activity) do - response - |> Map.put(:emoji, activity.data["content"]) + Map.put(response, :emoji, activity.data["content"]) end - defp put_status(response, activity, user) do - Map.put(response, :status, StatusView.render("show.json", %{activity: activity, for: user})) + defp put_status(response, activity, reading_user, opts) do + status_render_opts = Map.merge(opts, %{activity: activity, for: reading_user}) + status_render = StatusView.render("show.json", status_render_opts) + + Map.put(response, :status, status_render) end - defp put_target(response, activity, user) do - target = User.get_cached_by_ap_id(activity.data["target"]) - Map.put(response, :target, AccountView.render("show.json", %{user: target, for: user})) + defp put_target(response, activity, reading_user, opts) do + target_user = User.get_cached_by_ap_id(activity.data["target"]) + target_render_opts = Map.merge(opts, %{user: target_user, for: reading_user}) + target_render = AccountView.render("show.json", target_render_opts) + + Map.put(response, :target, target_render) end end diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 55a5513f9..0ef65b352 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -72,41 +72,46 @@ defp reblogged?(activity, user) do present?(user && user.ap_id in (object.data["announcements"] || [])) end - defp relationships_opts(opts) do - reading_user = opts[:for] + def relationships_opts(_reading_user = nil, _actors) do + %{user_relationships: [], following_relationships: []} + end - {user_relationships, following_relationships} = - if reading_user do - activities = opts[:activities] - actors = Enum.map(activities, fn a -> get_user(a.data["actor"]) end) + def relationships_opts(reading_user, actors) do + user_relationships = + UserRelationship.dictionary( + [reading_user], + actors, + [:block, :mute, :notification_mute, :reblog_mute], + [:block, :inverse_subscription] + ) - 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, following_relationships} - else - {[], []} - end + 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 - activities = opts.activities + # To do: check AdminAPIControllerTest on the reasons behind nil activities in the list + activities = Enum.filter(opts.activities, & &1) replied_to_activities = get_replied_to_activities(activities) + parent_activities = + activities + |> Enum.filter(&(&1.data["type"] == "Announce" && &1.data["object"])) + |> Enum.map(&Object.normalize(&1).data["id"]) + |> Activity.create_by_object_ap_id() + |> Activity.with_preloaded_object(:left) + |> Activity.with_preloaded_bookmark(opts[:for]) + |> Activity.with_set_thread_muted_field(opts[:for]) + |> Repo.all() + + actors = Enum.map(activities ++ parent_activities, &get_user(&1.data["actor"])) + opts = opts |> Map.put(:replied_to_activities, replied_to_activities) - |> Map.merge(relationships_opts(opts)) + |> Map.put(:parent_activities, parent_activities) + |> Map.put(:relationships, relationships_opts(opts[:for], actors)) safe_render_many(activities, StatusView, "show.json", opts) end @@ -119,17 +124,25 @@ def render( created_at = Utils.to_masto_date(activity.data["published"]) activity_object = Object.normalize(activity) - reblogged_activity = - Activity.create_by_object_ap_id(activity_object.data["id"]) - |> Activity.with_preloaded_bookmark(opts[:for]) - |> Activity.with_set_thread_muted_field(opts[:for]) - |> Repo.one() + reblogged_parent_activity = + if opts[:parent_activities] do + Activity.Queries.find_by_object_ap_id( + opts[:parent_activities], + activity_object.data["id"] + ) + else + Activity.create_by_object_ap_id(activity_object.data["id"]) + |> Activity.with_preloaded_bookmark(opts[:for]) + |> Activity.with_set_thread_muted_field(opts[:for]) + |> Repo.one() + end - reblogged = render("show.json", Map.put(opts, :activity, reblogged_activity)) + reblog_rendering_opts = Map.put(opts, :activity, reblogged_parent_activity) + reblogged = render("show.json", reblog_rendering_opts) favorited = opts[:for] && opts[:for].ap_id in (activity_object.data["likes"] || []) - bookmarked = Activity.get_bookmark(reblogged_activity, opts[:for]) != nil + bookmarked = Activity.get_bookmark(reblogged_parent_activity, opts[:for]) != nil mentions = activity.recipients @@ -145,8 +158,7 @@ def render( AccountView.render("show.json", %{ user: user, for: opts[:for], - user_relationships: opts[:user_relationships], - following_relationships: opts[:following_relationships] + relationships: opts[:relationships] }), in_reply_to_id: nil, in_reply_to_account_id: nil, @@ -156,7 +168,7 @@ def render( reblogs_count: 0, replies_count: 0, favourites_count: 0, - reblogged: reblogged?(reblogged_activity, opts[:for]), + reblogged: reblogged?(reblogged_parent_activity, opts[:for]), favourited: present?(favorited), bookmarked: present?(bookmarked), muted: false, @@ -293,12 +305,10 @@ def render("show.json", %{activity: %{data: %{"object" => _object}} = activity} _ -> [] end - user_relationships_opt = opts[:user_relationships] - muted = thread_muted? || UserRelationship.exists?( - user_relationships_opt, + get_in(opts, [:relationships, :user_relationships]), :mute, opts[:for], user, @@ -313,8 +323,7 @@ def render("show.json", %{activity: %{data: %{"object" => _object}} = activity} AccountView.render("show.json", %{ user: user, for: opts[:for], - user_relationships: user_relationships_opt, - following_relationships: opts[:following_relationships] + relationships: opts[:relationships] }), in_reply_to_id: reply_to && to_string(reply_to.id), in_reply_to_account_id: reply_to_user && to_string(reply_to_user.id), diff --git a/test/web/mastodon_api/controllers/timeline_controller_test.exs b/test/web/mastodon_api/controllers/timeline_controller_test.exs index 47849fc48..97b1c3e66 100644 --- a/test/web/mastodon_api/controllers/timeline_controller_test.exs +++ b/test/web/mastodon_api/controllers/timeline_controller_test.exs @@ -68,7 +68,6 @@ test "the home timeline", %{user: user, conn: conn} do "account" => %{ "acct" => "repeated", "pleroma" => %{ - # This part does not match correctly "relationship" => %{"following" => false, "followed_by" => true} } } From e743c2232970e321c833604b232520587ad8e402 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 25 Mar 2020 09:04:00 +0300 Subject: [PATCH 16/23] Fixed incorrect usage of "relations" as a short form of "relationships". --- config/description.exs | 2 +- lib/pleroma/notification.ex | 6 +++--- lib/pleroma/user.ex | 20 +++++++++---------- lib/pleroma/web/activity_pub/activity_pub.ex | 8 ++++---- .../controllers/account_controller.ex | 10 +++++++--- lib/pleroma/web/streamer/worker.ex | 2 +- test/user_test.exs | 6 +++--- 7 files changed, 29 insertions(+), 25 deletions(-) diff --git a/config/description.exs b/config/description.exs index 732c76734..68fa8b03b 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2442,7 +2442,7 @@ %{ key: :relations_actions, type: [:tuple, {:list, :tuple}], - description: "For actions on relations with all users (follow, unfollow)", + description: "For actions on relationships with all users (follow, unfollow)", suggestions: [{1000, 10}, [{10_000, 10}, {10_000, 50}]] }, %{ diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 104368fd1..bc691dce3 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -39,11 +39,11 @@ def changeset(%Notification{} = notification, attrs) do end defp for_user_query_ap_id_opts(user, opts) do - ap_id_relations = + ap_id_relationships = [:block] ++ if opts[@include_muted_option], do: [], else: [:notification_mute] - preloaded_ap_ids = User.outgoing_relations_ap_ids(user, ap_id_relations) + preloaded_ap_ids = User.outgoing_relationships_ap_ids(user, ap_id_relationships) exclude_blocked_opts = Map.merge(%{blocked_users_ap_ids: preloaded_ap_ids[:block]}, opts) @@ -370,7 +370,7 @@ def exclude_relation_restricting_ap_ids(ap_ids, %Activity{} = activity) do relation_restricted_ap_ids = activity |> Activity.user_actor() - |> User.incoming_relations_ungrouped_ap_ids([ + |> User.incoming_relationships_ungrouped_ap_ids([ :block, :notification_mute ]) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 05efc74d4..4919c8e58 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -1222,15 +1222,15 @@ def subscribed_to?(%User{} = user, %{ap_id: ap_id}) do end @doc """ - Returns map of outgoing (blocked, muted etc.) relations' user AP IDs by relation type. - E.g. `outgoing_relations_ap_ids(user, [:block])` -> `%{block: ["https://some.site/users/userapid"]}` + Returns map of outgoing (blocked, muted etc.) relationships' user AP IDs by relation type. + E.g. `outgoing_relationships_ap_ids(user, [:block])` -> `%{block: ["https://some.site/users/userapid"]}` """ - @spec outgoing_relations_ap_ids(User.t(), list(atom())) :: %{atom() => list(String.t())} - def outgoing_relations_ap_ids(_user, []), do: %{} + @spec outgoing_relationships_ap_ids(User.t(), list(atom())) :: %{atom() => list(String.t())} + def outgoing_relationships_ap_ids(_user, []), do: %{} - def outgoing_relations_ap_ids(nil, _relationship_types), do: %{} + def outgoing_relationships_ap_ids(nil, _relationship_types), do: %{} - def outgoing_relations_ap_ids(%User{} = user, relationship_types) + def outgoing_relationships_ap_ids(%User{} = user, relationship_types) when is_list(relationship_types) do db_result = user @@ -1249,13 +1249,13 @@ def outgoing_relations_ap_ids(%User{} = user, relationship_types) ) end - def incoming_relations_ungrouped_ap_ids(user, relationship_types, ap_ids \\ nil) + def incoming_relationships_ungrouped_ap_ids(user, relationship_types, ap_ids \\ nil) - def incoming_relations_ungrouped_ap_ids(_user, [], _ap_ids), do: [] + def incoming_relationships_ungrouped_ap_ids(_user, [], _ap_ids), do: [] - def incoming_relations_ungrouped_ap_ids(nil, _relationship_types, _ap_ids), do: [] + def incoming_relationships_ungrouped_ap_ids(nil, _relationship_types, _ap_ids), do: [] - def incoming_relations_ungrouped_ap_ids(%User{} = user, relationship_types, ap_ids) + def incoming_relationships_ungrouped_ap_ids(%User{} = user, relationship_types, ap_ids) when is_list(relationship_types) do user |> assoc(:incoming_relationships) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index d9f74b6a4..60e74758f 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1230,17 +1230,17 @@ defp maybe_order(query, _), do: query defp fetch_activities_query_ap_ids_ops(opts) do source_user = opts["muting_user"] - ap_id_relations = if source_user, do: [:mute, :reblog_mute], else: [] + ap_id_relationships = if source_user, do: [:mute, :reblog_mute], else: [] - ap_id_relations = - ap_id_relations ++ + ap_id_relationships = + ap_id_relationships ++ if opts["blocking_user"] && opts["blocking_user"] == source_user do [:block] else [] end - preloaded_ap_ids = User.outgoing_relations_ap_ids(source_user, ap_id_relations) + preloaded_ap_ids = User.outgoing_relationships_ap_ids(source_user, ap_id_relationships) restrict_blocked_opts = Map.merge(%{"blocked_users_ap_ids" => preloaded_ap_ids[:block]}, opts) restrict_muted_opts = Map.merge(%{"muted_users_ap_ids" => preloaded_ap_ids[:mute]}, opts) diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex index 88c997b9f..9d83a9fc1 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -63,11 +63,15 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do when action != :create ) - @relations [:follow, :unfollow] + @relationship_actions [:follow, :unfollow] @needs_account ~W(followers following lists follow unfollow mute unmute block unblock)a - plug(RateLimiter, [name: :relations_id_action, params: ["id", "uri"]] when action in @relations) - plug(RateLimiter, [name: :relations_actions] when action in @relations) + plug( + RateLimiter, + [name: :relation_id_action, params: ["id", "uri"]] when action in @relationship_actions + ) + + plug(RateLimiter, [name: :relations_actions] when action in @relationship_actions) plug(RateLimiter, [name: :app_account_creation] when action == :create) plug(:assign_account_by_id when action in @needs_account) diff --git a/lib/pleroma/web/streamer/worker.ex b/lib/pleroma/web/streamer/worker.ex index 29f992a67..abfed21c8 100644 --- a/lib/pleroma/web/streamer/worker.ex +++ b/lib/pleroma/web/streamer/worker.ex @@ -130,7 +130,7 @@ defp do_stream(%{topic: topic, item: item}) do defp should_send?(%User{} = user, %Activity{} = item) do %{block: blocked_ap_ids, mute: muted_ap_ids, reblog_mute: reblog_muted_ap_ids} = - User.outgoing_relations_ap_ids(user, [:block, :mute, :reblog_mute]) + User.outgoing_relationships_ap_ids(user, [:block, :mute, :reblog_mute]) recipient_blocks = MapSet.new(blocked_ap_ids ++ muted_ap_ids) recipients = MapSet.new(item.recipients) diff --git a/test/user_test.exs b/test/user_test.exs index b07fed42b..f3d044a80 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -86,7 +86,7 @@ test "returns invisible actor" do {:ok, user: insert(:user)} end - test "outgoing_relations_ap_ids/1", %{user: user} do + test "outgoing_relationships_ap_ids/1", %{user: user} do rel_types = [:block, :mute, :notification_mute, :reblog_mute, :inverse_subscription] ap_ids_by_rel = @@ -124,10 +124,10 @@ test "outgoing_relations_ap_ids/1", %{user: user} do assert ap_ids_by_rel[:inverse_subscription] == Enum.sort(Enum.map(User.subscriber_users(user), & &1.ap_id)) - outgoing_relations_ap_ids = User.outgoing_relations_ap_ids(user, rel_types) + outgoing_relationships_ap_ids = User.outgoing_relationships_ap_ids(user, rel_types) assert ap_ids_by_rel == - Enum.into(outgoing_relations_ap_ids, %{}, fn {k, v} -> {k, Enum.sort(v)} end) + Enum.into(outgoing_relationships_ap_ids, %{}, fn {k, v} -> {k, Enum.sort(v)} end) end end From 3fa3d45dbecafb06fb7eb4f0260f610d4225e0a7 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 25 Mar 2020 13:05:00 +0300 Subject: [PATCH 17/23] [#1364] Minor improvements / comments. Further fixes of incorrect usage of "relations" as a short form of "relationships". --- lib/pleroma/activity.ex | 1 + lib/pleroma/notification.ex | 12 +++++++----- lib/pleroma/thread_mute.ex | 7 ++++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index bbaa561a7..5a8329e69 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -95,6 +95,7 @@ def with_preloaded_object(query, join_type \\ :inner) do |> preload([activity, object: object], object: object) end + # Note: applies to fake activities (ActivityPub.Utils.get_notified_from_object/1 etc.) def user_actor(%Activity{actor: nil}), do: nil def user_actor(%Activity{} = activity) do diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 63e3e9be9..04ee510b9 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -322,6 +322,8 @@ def create_notification(%Activity{} = activity, %User{} = user, do_send \\ true) @doc """ Returns a tuple with 2 elements: {enabled notification receivers, currently disabled receivers (blocking / [thread] muting)} + + NOTE: might be called for FAKE Activities, see ActivityPub.Utils.get_notified_from_object/1 """ def get_notified_from_activity(activity, local_only \\ true) @@ -338,7 +340,7 @@ def get_notified_from_activity(%Activity{data: %{"type" => type}} = activity, lo # Since even subscribers and followers can mute / thread-mute, filtering all above AP IDs notification_enabled_ap_ids = potential_receiver_ap_ids - |> exclude_relation_restricting_ap_ids(activity) + |> exclude_relationship_restricted_ap_ids(activity) |> exclude_thread_muter_ap_ids(activity) potential_receivers = @@ -355,10 +357,10 @@ def get_notified_from_activity(%Activity{data: %{"type" => type}} = activity, lo def get_notified_from_activity(_, _local_only), do: {[], []} @doc "Filters out AP IDs of users basing on their relationships with activity actor user" - def exclude_relation_restricting_ap_ids([], _activity), do: [] + def exclude_relationship_restricted_ap_ids([], _activity), do: [] - def exclude_relation_restricting_ap_ids(ap_ids, %Activity{} = activity) do - relation_restricted_ap_ids = + def exclude_relationship_restricted_ap_ids(ap_ids, %Activity{} = activity) do + relationship_restricted_ap_ids = activity |> Activity.user_actor() |> User.incoming_relationships_ungrouped_ap_ids([ @@ -366,7 +368,7 @@ def exclude_relation_restricting_ap_ids(ap_ids, %Activity{} = activity) do :notification_mute ]) - Enum.uniq(ap_ids) -- relation_restricted_ap_ids + Enum.uniq(ap_ids) -- relationship_restricted_ap_ids end @doc "Filters out AP IDs of users who mute activity thread" diff --git a/lib/pleroma/thread_mute.ex b/lib/pleroma/thread_mute.ex index 2b4cf02cf..a7ea13891 100644 --- a/lib/pleroma/thread_mute.ex +++ b/lib/pleroma/thread_mute.ex @@ -41,15 +41,16 @@ def muters_query(context) do def muter_ap_ids(context, ap_ids \\ nil) - def muter_ap_ids(context, ap_ids) when context not in [nil, ""] do + # Note: applies to fake activities (ActivityPub.Utils.get_notified_from_object/1 etc.) + def muter_ap_ids(context, _ap_ids) when is_nil(context), do: [] + + def muter_ap_ids(context, ap_ids) do context |> muters_query() |> maybe_filter_on_ap_id(ap_ids) |> Repo.all() end - def muter_ap_ids(_context, _ap_ids), do: [] - defp maybe_filter_on_ap_id(query, ap_ids) when is_list(ap_ids) do where(query, [tm, u], u.ap_id in ^ap_ids) end From be5e2c4dbba63831ea6a0617556e686969b5080f Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 25 Mar 2020 17:01:45 +0300 Subject: [PATCH 18/23] Applied relationships preloading to GET /api/v1/accounts/relationships. Refactoring (User.binary_id/1). --- lib/pleroma/conversation/participation.ex | 11 ++++------- lib/pleroma/following_relationship.ex | 8 ++------ lib/pleroma/thread_mute.ex | 4 ++-- lib/pleroma/user.ex | 15 +++++++++++++++ lib/pleroma/user_relationship.ex | 9 ++------- .../web/mastodon_api/views/account_view.ex | 6 +++++- 6 files changed, 30 insertions(+), 23 deletions(-) diff --git a/lib/pleroma/conversation/participation.ex b/lib/pleroma/conversation/participation.ex index 693825cf5..215265fc9 100644 --- a/lib/pleroma/conversation/participation.ex +++ b/lib/pleroma/conversation/participation.ex @@ -129,21 +129,18 @@ def for_user(user, params \\ %{}) do end def restrict_recipients(query, user, %{"recipients" => user_ids}) do - user_ids = + user_binary_ids = [user.id | user_ids] |> Enum.uniq() - |> Enum.reduce([], fn user_id, acc -> - {:ok, user_id} = FlakeId.Ecto.CompatType.dump(user_id) - [user_id | acc] - end) + |> User.binary_id() conversation_subquery = __MODULE__ |> group_by([p], p.conversation_id) |> having( [p], - count(p.user_id) == ^length(user_ids) and - fragment("array_agg(?) @> ?", p.user_id, ^user_ids) + count(p.user_id) == ^length(user_binary_ids) and + fragment("array_agg(?) @> ?", p.user_id, ^user_binary_ids) ) |> select([p], %{id: p.conversation_id}) diff --git a/lib/pleroma/following_relationship.ex b/lib/pleroma/following_relationship.ex index dd1696136..624bddfe4 100644 --- a/lib/pleroma/following_relationship.ex +++ b/lib/pleroma/following_relationship.ex @@ -135,12 +135,8 @@ def all_between_user_sets( target_users ) when is_list(source_users) and is_list(target_users) do - get_bin_ids = fn user -> - with {:ok, bin_id} <- CompatType.dump(user.id), do: bin_id - end - - source_user_ids = Enum.map(source_users, &get_bin_ids.(&1)) - target_user_ids = Enum.map(target_users, &get_bin_ids.(&1)) + source_user_ids = User.binary_id(source_users) + target_user_ids = User.binary_id(target_users) __MODULE__ |> where( diff --git a/lib/pleroma/thread_mute.ex b/lib/pleroma/thread_mute.ex index cc815430a..f657758aa 100644 --- a/lib/pleroma/thread_mute.ex +++ b/lib/pleroma/thread_mute.ex @@ -24,10 +24,10 @@ def changeset(mute, params \\ %{}) do end def query(user_id, context) do - {:ok, user_id} = FlakeId.Ecto.CompatType.dump(user_id) + user_binary_id = User.binary_id(user_id) ThreadMute - |> Ecto.Query.where(user_id: ^user_id) + |> Ecto.Query.where(user_id: ^user_binary_id) |> Ecto.Query.where(context: ^context) end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index f74e43cce..699256a3b 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -218,6 +218,21 @@ def unquote(:"#{outgoing_relation_target}_ap_ids")(user, restrict_deactivated? \ end end + @doc "Dumps id to SQL-compatible format" + def binary_id(source_id) when is_binary(source_id) do + with {:ok, dumped_id} <- FlakeId.Ecto.CompatType.dump(source_id) do + dumped_id + else + _ -> source_id + end + end + + def binary_id(source_ids) when is_list(source_ids) do + Enum.map(source_ids, &binary_id/1) + end + + def binary_id(%User{} = user), do: binary_id(user.id) + @doc "Returns status account" @spec account_status(User.t()) :: account_status() def account_status(%User{deactivated: true}), do: :deactivated diff --git a/lib/pleroma/user_relationship.ex b/lib/pleroma/user_relationship.ex index 9423e3a42..519d2998d 100644 --- a/lib/pleroma/user_relationship.ex +++ b/lib/pleroma/user_relationship.ex @@ -8,7 +8,6 @@ defmodule Pleroma.UserRelationship do import Ecto.Changeset import Ecto.Query - alias FlakeId.Ecto.CompatType alias Pleroma.Repo alias Pleroma.User alias Pleroma.UserRelationship @@ -84,12 +83,8 @@ def dictionary( target_to_source_rel_types \\ nil ) when is_list(source_users) and is_list(target_users) do - get_bin_ids = fn user -> - with {:ok, bin_id} <- CompatType.dump(user.id), do: bin_id - end - - source_user_ids = Enum.map(source_users, &get_bin_ids.(&1)) - target_user_ids = Enum.map(target_users, &get_bin_ids.(&1)) + source_user_ids = User.binary_id(source_users) + target_user_ids = User.binary_id(target_users) get_rel_type_codes = fn rel_type -> user_relationship_mappings()[rel_type] end diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index 702d9e658..6b2eca1f3 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -9,6 +9,7 @@ defmodule Pleroma.Web.MastodonAPI.AccountView do 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 @@ -129,7 +130,10 @@ def render( end def render("relationships.json", %{user: user, targets: targets}) do - render_many(targets, AccountView, "relationship.json", user: user, as: :target) + relationships_opts = StatusView.relationships_opts(user, targets) + opts = %{as: :target, user: user, relationships: relationships_opts} + + render_many(targets, AccountView, "relationship.json", opts) end defp do_render("show.json", %{user: user} = opts) do From 9081a071eecd0eeb4b67008754555e9c9d73eae7 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Wed, 25 Mar 2020 18:46:17 +0400 Subject: [PATCH 19/23] Add a test for accounts/update_credentials --- .../account_controller/update_credentials_test.exs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/web/mastodon_api/controllers/account_controller/update_credentials_test.exs b/test/web/mastodon_api/controllers/account_controller/update_credentials_test.exs index 51cebe567..b693c1a47 100644 --- a/test/web/mastodon_api/controllers/account_controller/update_credentials_test.exs +++ b/test/web/mastodon_api/controllers/account_controller/update_credentials_test.exs @@ -118,6 +118,18 @@ test "updates the user's hide_followers status", %{conn: conn} do assert user_data["pleroma"]["hide_followers"] == true end + test "updates the user's discoverable status", %{conn: conn} do + assert %{"source" => %{"pleroma" => %{"discoverable" => true}}} = + conn + |> patch("/api/v1/accounts/update_credentials", %{discoverable: "true"}) + |> json_response(:ok) + + assert %{"source" => %{"pleroma" => %{"discoverable" => false}}} = + conn + |> patch("/api/v1/accounts/update_credentials", %{discoverable: "false"}) + |> json_response(:ok) + end + test "updates the user's hide_followers_count and hide_follows_count", %{conn: conn} do conn = patch(conn, "/api/v1/accounts/update_credentials", %{ From c8475cd5c63af18471864fe57504999ddd09e496 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 25 Mar 2020 15:48:15 +0000 Subject: [PATCH 20/23] Apply suggestion to benchmarks/load_testing/generator.ex --- benchmarks/load_testing/generator.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/load_testing/generator.ex b/benchmarks/load_testing/generator.ex index 17e89c13c..e4673757c 100644 --- a/benchmarks/load_testing/generator.ex +++ b/benchmarks/load_testing/generator.ex @@ -24,7 +24,7 @@ def generate_users(opts) do IO.puts("Starting generating #{opts[:users_max]} users...") {time, users} = :timer.tc(fn -> do_generate_users(opts) end) - IO.puts("Inserting users take #{to_sec(time)} sec.\n") + IO.puts("Inserting users took #{to_sec(time)} sec.\n") users end From 460e41585c2cd3f137c0f80173da60167fb318bf Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 25 Mar 2020 20:33:34 +0300 Subject: [PATCH 21/23] 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 From 6b793d3f8336fcba5cac596f9e76d0274633f98d Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 26 Mar 2020 21:54:01 +0300 Subject: [PATCH 22/23] Ensured no auxiliary computations (actors list preparation etc.) related to relationships preloading if no user is present (for statuses / accounts / relationships rendering). --- .../web/mastodon_api/views/account_view.ex | 26 +++++++++++----- .../mastodon_api/views/notification_view.ex | 31 +++++++++++-------- .../web/mastodon_api/views/status_view.ex | 16 +++++++--- 3 files changed, 47 insertions(+), 26 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index 2cdfac7af..0efcabc01 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -14,10 +14,15 @@ defmodule Pleroma.Web.MastodonAPI.AccountView do 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) + cond do + Map.has_key?(opts, :relationships) -> + opts[:relationships] + + is_nil(opts[:for]) -> + UserRelationship.view_relationships_option(nil, []) + + true -> + UserRelationship.view_relationships_option(opts[:for], users) end opts = Map.put(opts, :relationships, relationships_opt) @@ -134,10 +139,15 @@ def render( 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) + cond do + Map.has_key?(opts, :relationships) -> + opts[:relationships] + + is_nil(opts[:for]) -> + UserRelationship.view_relationships_option(nil, []) + + true -> + UserRelationship.view_relationships_option(user, targets) end render_opts = %{as: :target, user: user, relationships: relationships_opt} diff --git a/lib/pleroma/web/mastodon_api/views/notification_view.ex b/lib/pleroma/web/mastodon_api/views/notification_view.ex index db434271c..a809080fd 100644 --- a/lib/pleroma/web/mastodon_api/views/notification_view.ex +++ b/lib/pleroma/web/mastodon_api/views/notification_view.ex @@ -32,21 +32,26 @@ def render("index.json", %{notifications: notifications, for: reading_user} = op |> Pleroma.Repo.all() 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"])) + cond do + Map.has_key?(opts, :relationships) -> + opts[:relationships] - actors = - activities - |> Enum.map(fn a -> User.get_cached_by_ap_id(a.data["actor"]) end) - |> Enum.filter(& &1) - |> Kernel.++(move_activities_targets) + is_nil(opts[:for]) -> + UserRelationship.view_relationships_option(nil, []) - UserRelationship.view_relationships_option(reading_user, actors) + true -> + 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) + + UserRelationship.view_relationships_option(reading_user, actors) end opts = %{ diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 7b1cb7bf8..d36b9ee5c 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -87,11 +87,17 @@ def render("index.json", opts) do |> Repo.all() 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) + cond do + Map.has_key?(opts, :relationships) -> + opts[:relationships] + + is_nil(opts[:for]) -> + UserRelationship.view_relationships_option(nil, []) + + true -> + actors = Enum.map(activities ++ parent_activities, &get_user(&1.data["actor"])) + + UserRelationship.view_relationships_option(opts[:for], actors) end opts = From dfbc05d4965a04a82d4c4c5b8842f4117757f30e Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Fri, 27 Mar 2020 08:01:03 +0300 Subject: [PATCH 23/23] Misc refactoring / tweaks (`ThreadMute.exists?/2`). --- lib/pleroma/thread_mute.ex | 4 ++-- lib/pleroma/web/common_api/common_api.ex | 2 +- .../web/mastodon_api/views/notification_view.ex | 12 ++++++------ lib/pleroma/web/mastodon_api/views/status_view.ex | 7 ++++--- test/web/mastodon_api/views/account_view_test.exs | 2 +- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/thread_mute.ex b/lib/pleroma/thread_mute.ex index 5768e7711..be01d541d 100644 --- a/lib/pleroma/thread_mute.ex +++ b/lib/pleroma/thread_mute.ex @@ -68,8 +68,8 @@ def remove_mute(user_id, context) do |> Repo.delete_all() end - def check_muted(user_id, context) do + def exists?(user_id, context) do query(user_id, context) - |> Repo.all() + |> Repo.exists?() end end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 091011c6b..2646b9f7b 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -358,7 +358,7 @@ def remove_mute(user, activity) do def thread_muted?(%{id: nil} = _user, _activity), do: false def thread_muted?(user, activity) do - ThreadMute.check_muted(user.id, activity.data["context"]) != [] + ThreadMute.exists?(user.id, activity.data["context"]) end def report(user, %{"account_id" => account_id} = data) do diff --git a/lib/pleroma/web/mastodon_api/views/notification_view.ex b/lib/pleroma/web/mastodon_api/views/notification_view.ex index a809080fd..89f5734ff 100644 --- a/lib/pleroma/web/mastodon_api/views/notification_view.ex +++ b/lib/pleroma/web/mastodon_api/views/notification_view.ex @@ -98,27 +98,27 @@ def render( } } - relationships_opt = %{relationships: opts[:relationships]} + render_opts = %{relationships: opts[:relationships]} case mastodon_type do "mention" -> - put_status(response, activity, reading_user, relationships_opt) + put_status(response, activity, reading_user, render_opts) "favourite" -> - put_status(response, parent_activity_fn.(), reading_user, relationships_opt) + put_status(response, parent_activity_fn.(), reading_user, render_opts) "reblog" -> - put_status(response, parent_activity_fn.(), reading_user, relationships_opt) + put_status(response, parent_activity_fn.(), reading_user, render_opts) "move" -> - put_target(response, activity, reading_user, relationships_opt) + put_target(response, activity, reading_user, render_opts) "follow" -> response "pleroma:emoji_reaction" -> response - |> put_status(parent_activity_fn.(), reading_user, relationships_opt) + |> put_status(parent_activity_fn.(), reading_user, render_opts) |> 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 d36b9ee5c..440eef4ba 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -228,9 +228,10 @@ def render("show.json", %{activity: %{data: %{"object" => _object}} = activity} end thread_muted? = - case activity.thread_muted? do - thread_muted? when is_boolean(thread_muted?) -> thread_muted? - nil -> (opts[:for] && CommonAPI.thread_muted?(opts[:for], activity)) || false + cond do + is_nil(opts[:for]) -> false + is_boolean(activity.thread_muted?) -> activity.thread_muted? + true -> CommonAPI.thread_muted?(opts[:for], activity) end attachment_data = object.data["attachment"] || [] diff --git a/test/web/mastodon_api/views/account_view_test.exs b/test/web/mastodon_api/views/account_view_test.exs index ede62903f..0d1c3ecb3 100644 --- a/test/web/mastodon_api/views/account_view_test.exs +++ b/test/web/mastodon_api/views/account_view_test.exs @@ -186,7 +186,7 @@ test "Represent a smaller mention" do describe "relationship" do defp test_relationship_rendering(user, other_user, expected_result) do - opts = %{user: user, target: other_user} + opts = %{user: user, target: other_user, relationships: nil} assert expected_result == AccountView.render("relationship.json", opts) relationships_opt = UserRelationship.view_relationships_option(user, [other_user])