From cf96c4005743c61d44e17c9d37c6427eaf69c152 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 22 Jan 2020 21:10:17 +0300 Subject: [PATCH 01/10] [#1505] Added Mastodon-compatible `replies` collection to Note federated representation. --- config/config.exs | 4 ++ config/description.exs | 14 ++++++ lib/pleroma/activity.ex | 19 ++++++++ lib/pleroma/activity/queries.ex | 18 ++++++- .../web/activity_pub/transmogrifier.ex | 44 +++++++++++++++++ test/web/activity_pub/transmogrifier_test.exs | 47 +++++++++++++++++++ 6 files changed, 145 insertions(+), 1 deletion(-) diff --git a/config/config.exs b/config/config.exs index 2c154eb45..60642c467 100644 --- a/config/config.exs +++ b/config/config.exs @@ -620,6 +620,10 @@ config :pleroma, :modules, runtime_dir: "instance/modules" config :pleroma, configurable_from_database: false +config :pleroma, :mastodon_compatibility, + # https://git.pleroma.social/pleroma/pleroma/issues/1505 + federated_note_replies_limit: 5 + config :swarm, node_blacklist: [~r/myhtml_.*$/] # Import environment specific config. This must remain at the bottom # of this file so it overrides the configuration defined above. diff --git a/config/description.exs b/config/description.exs index f941349d5..a0675ec30 100644 --- a/config/description.exs +++ b/config/description.exs @@ -3089,6 +3089,20 @@ config :pleroma, :config_description, [ } ] }, + %{ + group: :pleroma, + key: :mastodon_compatibility, + type: :group, + description: "Mastodon compatibility-related settings.", + children: [ + %{ + key: :federated_note_replies_limit, + type: :integer, + description: + "The number of Note self-reply URIs to be included with outgoing federation (`5` to mimic Mastodon hardcoded value, `0` to disable)." + } + ] + }, %{ group: :pleroma, type: :group, diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 896cbb3c5..b7be7a800 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -329,4 +329,23 @@ defmodule Pleroma.Activity do _ -> nil end end + + def replies(activity, opts \\ []) do + object = Object.normalize(activity) + + query = + Activity + |> Queries.by_type("Create") + |> Queries.by_object_in_reply_to_id(object.data["id"], skip_preloading: true) + |> order_by([activity], asc: activity.id) + + if opts[:self_only] do + where(query, [a], a.actor == ^activity.actor) + else + query + end + end + + def self_replies(activity, opts \\ []), + do: replies(activity, Keyword.put(opts, :self_only, true)) end diff --git a/lib/pleroma/activity/queries.ex b/lib/pleroma/activity/queries.ex index 79f305201..c17affec9 100644 --- a/lib/pleroma/activity/queries.ex +++ b/lib/pleroma/activity/queries.ex @@ -7,7 +7,7 @@ defmodule Pleroma.Activity.Queries do Contains queries for Activity. """ - import Ecto.Query, only: [from: 2] + import Ecto.Query, only: [from: 2, where: 3] @type query :: Ecto.Queryable.t() | Activity.t() @@ -63,6 +63,22 @@ defmodule Pleroma.Activity.Queries do ) end + @spec by_object_id(query, String.t()) :: query + def by_object_in_reply_to_id(query, in_reply_to_id, opts \\ []) do + query = + if opts[:skip_preloading] do + Activity.with_joined_object(query) + else + Activity.with_preloaded_object(query) + end + + where( + query, + [activity, object: o], + fragment("(?)->>'inReplyTo' = ?", o.data, ^to_string(in_reply_to_id)) + ) + end + @spec by_type(query, String.t()) :: query def by_type(query \\ Activity, activity_type) do from( diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 2b8bfc3bd..9e712ab75 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -903,6 +903,49 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do def set_reply_to_uri(obj), do: obj + @doc """ + Serialized Mastodon-compatible `replies` collection containing _self-replies_. + Based on Mastodon's ActivityPub::NoteSerializer#replies. + """ + def set_replies(obj) do + limit = Pleroma.Config.get([:mastodon_compatibility, :federated_note_replies_limit], 0) + + replies_uris = + with true <- limit > 0 || nil, + %Activity{} = activity <- Activity.get_create_by_object_ap_id(obj["id"]) do + activity + |> Activity.self_replies() + |> select([a], fragment("?->>'id'", a.data)) + |> limit(^limit) + |> Repo.all() + end + + set_replies(obj, replies_uris || []) + end + + defp set_replies(obj, replies_uris) when replies_uris in [nil, []] do + obj + end + + defp set_replies(obj, replies_uris) do + # Note: stubs (Mastodon doesn't make separate requests via those URIs in FetchRepliesService) + masto_replies_uri = nil + masto_replies_next_page_uri = nil + + replies_collection = %{ + "type" => "Collection", + "id" => masto_replies_uri, + "first" => %{ + "type" => "Collection", + "part_of" => masto_replies_uri, + "items" => replies_uris, + "next" => masto_replies_next_page_uri + } + } + + Map.merge(obj, %{"replies" => replies_collection}) + end + # Prepares the object of an outgoing create activity. def prepare_object(object) do object @@ -914,6 +957,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do |> prepare_attachments |> set_conversation |> set_reply_to_uri + |> set_replies |> strip_internal_fields |> strip_internal_tags |> set_type diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 5da358c43..418b8a1ca 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -2027,4 +2027,51 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do } end end + + describe "set_replies/1" do + clear_config([:mastodon_compatibility, :federated_note_replies_limit]) do + Pleroma.Config.put([:mastodon_compatibility, :federated_note_replies_limit], 2) + end + + test "returns unmodified object if activity doesn't have self-replies" do + data = Poison.decode!(File.read!("test/fixtures/mastodon-post-activity.json")) + assert Transmogrifier.set_replies(data) == data + end + + test "sets `replies` collection with a limited number of self-replies" do + [user, another_user] = insert_list(2, :user) + + {:ok, %{id: id1} = activity} = CommonAPI.post(user, %{"status" => "1"}) + + {:ok, %{id: id2} = self_reply1} = + CommonAPI.post(user, %{"status" => "self-reply 1", "in_reply_to_status_id" => id1}) + + {:ok, self_reply2} = + CommonAPI.post(user, %{"status" => "self-reply 2", "in_reply_to_status_id" => id1}) + + # Assuming to _not_ be present in `replies` due to :federated_note_replies_limit is set to 2 + {:ok, _} = + CommonAPI.post(user, %{"status" => "self-reply 3", "in_reply_to_status_id" => id1}) + + {:ok, _} = + CommonAPI.post(user, %{ + "status" => "self-reply to self-reply", + "in_reply_to_status_id" => id2 + }) + + {:ok, _} = + CommonAPI.post(another_user, %{ + "status" => "another user's reply", + "in_reply_to_status_id" => id1 + }) + + object = Object.normalize(activity) + replies_uris = Enum.map([self_reply1, self_reply2], fn a -> a.data["id"] end) + + assert %{ + "type" => "Collection", + "first" => %{"type" => "Collection", "items" => ^replies_uris} + } = Transmogrifier.set_replies(object.data)["replies"] + end + end end From 86e4d23acb640efea8cbc879ddbeadfa0e04f9c8 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 25 Jan 2020 10:47:30 +0300 Subject: [PATCH 02/10] [#1505] Background fetching of incoming activities' `replies` collections. --- config/config.exs | 1 + .../web/activity_pub/transmogrifier.ex | 15 +++++- lib/pleroma/workers/remote_fetcher_worker.ex | 20 ++++++++ test/support/oban_helpers.ex | 4 ++ test/web/activity_pub/transmogrifier_test.exs | 48 +++++++++++++++++++ 5 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 lib/pleroma/workers/remote_fetcher_worker.ex diff --git a/config/config.exs b/config/config.exs index 60642c467..5f72df8a0 100644 --- a/config/config.exs +++ b/config/config.exs @@ -501,6 +501,7 @@ config :pleroma, Oban, transmogrifier: 20, scheduled_activities: 10, background: 5, + remote_fetcher: 2, attachments_cleanup: 5 ] diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 9e712ab75..d129334c2 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -424,7 +424,13 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do ]) } - ActivityPub.create(params) + with {:ok, created_activity} <- ActivityPub.create(params) do + for reply_id <- replies(object) do + Pleroma.Workers.RemoteFetcherWorker.enqueue("fetch_remote", %{"id" => reply_id}) + end + + {:ok, created_activity} + end else %Activity{} = activity -> {:ok, activity} _e -> :error @@ -946,6 +952,13 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do Map.merge(obj, %{"replies" => replies_collection}) end + def replies(%{"replies" => replies = %{}}) do + replies = with %{} <- replies["first"], do: replies["first"], else: (_ -> replies) + replies["items"] || [] + end + + def replies(_), do: [] + # Prepares the object of an outgoing create activity. def prepare_object(object) do object diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex new file mode 100644 index 000000000..60eafe2c1 --- /dev/null +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -0,0 +1,20 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.RemoteFetcherWorker do + alias Pleroma.Object.Fetcher + + use Pleroma.Workers.WorkerHelper, queue: "remote_fetcher" + + @impl Oban.Worker + def perform( + %{ + "op" => "fetch_remote", + "id" => id + }, + _job + ) do + Fetcher.fetch_object_from_id!(id) + end +end diff --git a/test/support/oban_helpers.ex b/test/support/oban_helpers.ex index 72792c064..0e3b654df 100644 --- a/test/support/oban_helpers.ex +++ b/test/support/oban_helpers.ex @@ -9,6 +9,10 @@ defmodule Pleroma.Tests.ObanHelpers do alias Pleroma.Repo + def wipe_all do + Repo.delete_all(Oban.Job) + end + def perform_all do Oban.Job |> Repo.all() diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 418b8a1ca..0fefb60da 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -3,7 +3,9 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do + use Oban.Testing, repo: Pleroma.Repo use Pleroma.DataCase + alias Pleroma.Activity alias Pleroma.Object alias Pleroma.Object.Fetcher @@ -1329,6 +1331,52 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do end end + describe "handle_incoming:`replies` handling" do + setup do + data = + File.read!("test/fixtures/mastodon-post-activity.json") + |> Poison.decode!() + + items = ["https://shitposter.club/notice/2827873", "https://shitposter.club/notice/7387606"] + collection = %{"items" => items} + %{data: data, items: items, collection: collection} + end + + test "it schedules background fetching of wrapped `replies` collection items", %{ + data: data, + items: items, + collection: collection + } do + replies = %{"first" => collection} + + object = Map.put(data["object"], "replies", replies) + data = Map.put(data, "object", object) + {:ok, _activity} = Transmogrifier.handle_incoming(data) + + for id <- items do + job_args = %{"op" => "fetch_remote", "id" => id} + assert_enqueued(worker: Pleroma.Workers.RemoteFetcherWorker, args: job_args) + end + end + + test "it schedules background fetching of unwrapped `replies` collection items", %{ + data: data, + items: items, + collection: collection + } do + replies = collection + + object = Map.put(data["object"], "replies", replies) + data = Map.put(data, "object", object) + {:ok, _activity} = Transmogrifier.handle_incoming(data) + + for id <- items do + job_args = %{"op" => "fetch_remote", "id" => id} + assert_enqueued(worker: Pleroma.Workers.RemoteFetcherWorker, args: job_args) + end + end + end + describe "prepare outgoing" do test "it inlines private announced objects" do user = insert(:user) From d458f4fdcafe847a7db8b1c663cfd945019816b7 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 8 Feb 2020 19:58:02 +0300 Subject: [PATCH 03/10] [#1505] Added tests, changelog entry, tweaked config settings related to replies output on outgoing federation. --- CHANGELOG.md | 2 ++ config/config.exs | 5 +---- config/description.exs | 20 +++++------------ .../web/activity_pub/transmogrifier.ex | 10 +++++++-- lib/pleroma/workers/remote_fetcher_worker.ex | 2 +- test/web/activity_pub/transmogrifier_test.exs | 10 ++++----- .../activity_pub/views/object_view_test.exs | 22 +++++++++++++++++++ 7 files changed, 45 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 713ae4361..a1fbe152a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Support for custom Elixir modules (such as MRF policies) - User settings: Add _This account is a_ option. - OAuth: admin scopes support (relevant setting: `[:auth, :enforce_oauth_admin_scope_usage]`). +- ActivityPub: support for `replies` collection (output for outgoing federation & fetching on incoming federation).
API Changes @@ -107,6 +108,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Configuration: `feed.logo` option for tag feed. - Tag feed: `/tags/:tag.rss` - list public statuses by hashtag. - Mastodon API: Add `reacted` property to `emoji_reactions` +- ActivityPub: `[:activitypub, :note_replies_output_limit]` setting sets the number of note self-replies to output on outgoing federation.
### Fixed diff --git a/config/config.exs b/config/config.exs index 370828c1c..62a10ca41 100644 --- a/config/config.exs +++ b/config/config.exs @@ -340,6 +340,7 @@ config :pleroma, :activitypub, unfollow_blocked: true, outgoing_blocks: true, follow_handshake_timeout: 500, + note_replies_output_limit: 5, sign_object_fetches: true config :pleroma, :streamer, @@ -624,10 +625,6 @@ config :pleroma, :modules, runtime_dir: "instance/modules" config :pleroma, configurable_from_database: false -config :pleroma, :mastodon_compatibility, - # https://git.pleroma.social/pleroma/pleroma/issues/1505 - federated_note_replies_limit: 5 - config :swarm, node_blacklist: [~r/myhtml_.*$/] # Import environment specific config. This must remain at the bottom # of this file so it overrides the configuration defined above. diff --git a/config/description.exs b/config/description.exs index 909ae00d9..9fd52f50e 100644 --- a/config/description.exs +++ b/config/description.exs @@ -1790,6 +1790,12 @@ config :pleroma, :config_description, [ type: :boolean, description: "Sign object fetches with HTTP signatures" }, + %{ + key: :note_replies_output_limit, + type: :integer, + description: + "The number of Note replies' URIs to be included with outgoing federation (`5` to match Mastodon hardcoded value, `0` to disable the output)." + }, %{ key: :follow_handshake_timeout, type: :integer, @@ -3097,20 +3103,6 @@ config :pleroma, :config_description, [ } ] }, - %{ - group: :pleroma, - key: :mastodon_compatibility, - type: :group, - description: "Mastodon compatibility-related settings.", - children: [ - %{ - key: :federated_note_replies_limit, - type: :integer, - description: - "The number of Note self-reply URIs to be included with outgoing federation (`5` to mimic Mastodon hardcoded value, `0` to disable)." - } - ] - }, %{ group: :pleroma, type: :group, diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index d129334c2..623236720 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -914,7 +914,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do Based on Mastodon's ActivityPub::NoteSerializer#replies. """ def set_replies(obj) do - limit = Pleroma.Config.get([:mastodon_compatibility, :federated_note_replies_limit], 0) + limit = Pleroma.Config.get([:activitypub, :note_replies_output_limit], 0) replies_uris = with true <- limit > 0 || nil, @@ -953,7 +953,13 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end def replies(%{"replies" => replies = %{}}) do - replies = with %{} <- replies["first"], do: replies["first"], else: (_ -> replies) + replies = + if is_map(replies["first"]) do + replies["first"] + else + replies + end + replies["items"] || [] end diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index 60eafe2c1..52db6059b 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -15,6 +15,6 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do }, _job ) do - Fetcher.fetch_object_from_id!(id) + {:ok, _object} = Fetcher.fetch_object_from_id(id) end end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 194b314a3..729594ded 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -1350,7 +1350,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do end end - describe "handle_incoming:`replies` handling" do + describe "`replies` handling in handle_incoming/2" do setup do data = File.read!("test/fixtures/mastodon-post-activity.json") @@ -1361,7 +1361,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do %{data: data, items: items, collection: collection} end - test "it schedules background fetching of wrapped `replies` collection items", %{ + test "with wrapped `replies` collection, it schedules background fetching of items", %{ data: data, items: items, collection: collection @@ -2096,8 +2096,8 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do end describe "set_replies/1" do - clear_config([:mastodon_compatibility, :federated_note_replies_limit]) do - Pleroma.Config.put([:mastodon_compatibility, :federated_note_replies_limit], 2) + clear_config([:activitypub, :note_replies_output_limit]) do + Pleroma.Config.put([:activitypub, :note_replies_output_limit], 2) end test "returns unmodified object if activity doesn't have self-replies" do @@ -2116,7 +2116,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do {:ok, self_reply2} = CommonAPI.post(user, %{"status" => "self-reply 2", "in_reply_to_status_id" => id1}) - # Assuming to _not_ be present in `replies` due to :federated_note_replies_limit is set to 2 + # Assuming to _not_ be present in `replies` due to :note_replies_output_limit is set to 2 {:ok, _} = CommonAPI.post(user, %{"status" => "self-reply 3", "in_reply_to_status_id" => id1}) diff --git a/test/web/activity_pub/views/object_view_test.exs b/test/web/activity_pub/views/object_view_test.exs index 13447dc29..6784788cc 100644 --- a/test/web/activity_pub/views/object_view_test.exs +++ b/test/web/activity_pub/views/object_view_test.exs @@ -36,6 +36,28 @@ defmodule Pleroma.Web.ActivityPub.ObjectViewTest do assert result["@context"] end + describe "note activity's `replies` collection rendering" do + clear_config([:activitypub, :note_replies_output_limit]) do + Pleroma.Config.put([:activitypub, :note_replies_output_limit], 5) + end + + test "renders `replies` collection for a note activity" do + user = insert(:user) + activity = insert(:note_activity, user: user) + + {:ok, self_reply1} = + CommonAPI.post(user, %{"status" => "self-reply 1", "in_reply_to_status_id" => activity.id}) + + result = ObjectView.render("object.json", %{object: refresh_record(activity)}) + replies_uris = [self_reply1.data["id"]] + + assert %{ + "type" => "Collection", + "first" => %{"type" => "Collection", "items" => ^replies_uris} + } = get_in(result, ["object", "replies"]) + end + end + test "renders a like activity" do note = insert(:note_activity) object = Object.normalize(note) From 7c3991f59eccc47551257dfe41817e71d0eb6717 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 9 Feb 2020 10:17:21 +0300 Subject: [PATCH 04/10] [#1505] Fixed `replies` serialization (included objects' ids instead of activities' ids). --- lib/pleroma/activity.ex | 19 ---------------- lib/pleroma/object.ex | 22 +++++++++++++++++++ .../web/activity_pub/transmogrifier.ex | 16 +++++++------- test/web/activity_pub/transmogrifier_test.exs | 2 +- .../activity_pub/views/object_view_test.exs | 2 +- 5 files changed, 32 insertions(+), 29 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 10b6d7ebd..72e2256ea 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -330,23 +330,4 @@ defmodule Pleroma.Activity do _ -> nil end end - - def replies(activity, opts \\ []) do - object = Object.normalize(activity) - - query = - Activity - |> Queries.by_type("Create") - |> Queries.by_object_in_reply_to_id(object.data["id"], skip_preloading: true) - |> order_by([activity], asc: activity.id) - - if opts[:self_only] do - where(query, [a], a.actor == ^activity.actor) - else - query - end - end - - def self_replies(activity, opts \\ []), - do: replies(activity, Keyword.put(opts, :self_only, true)) end diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 52556bf31..f316f8b36 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -301,4 +301,26 @@ defmodule Pleroma.Object do def local?(%Object{data: %{"id" => id}}) do String.starts_with?(id, Pleroma.Web.base_url() <> "/") end + + def replies(object, opts \\ []) do + object = Object.normalize(object) + + query = + Object + |> where( + [o], + fragment("(?)->>'inReplyTo' = ?", o.data, ^object.data["id"]) + ) + |> order_by([o], asc: o.id) + + if opts[:self_only] do + actor = object.data["actor"] + where(query, [o], fragment("(?)->>'actor' = ?", o.data, ^actor)) + else + query + end + end + + def self_replies(object, opts \\ []), + do: replies(object, Keyword.put(opts, :self_only, true)) end diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 8266545d1..e89588f29 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -407,7 +407,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do with nil <- Activity.get_create_by_object_ap_id(object["id"]), {:ok, %User{} = user} <- User.get_or_fetch_by_ap_id(data["actor"]) do options = Keyword.put(options, :depth, (options[:depth] || 0) + 1) - object = fix_object(data["object"], options) + object = fix_object(object, options) params = %{ to: data["to"], @@ -913,20 +913,20 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do Serialized Mastodon-compatible `replies` collection containing _self-replies_. Based on Mastodon's ActivityPub::NoteSerializer#replies. """ - def set_replies(obj) do + def set_replies(obj_data) do limit = Pleroma.Config.get([:activitypub, :note_replies_output_limit], 0) replies_uris = with true <- limit > 0 || nil, - %Activity{} = activity <- Activity.get_create_by_object_ap_id(obj["id"]) do - activity - |> Activity.self_replies() - |> select([a], fragment("?->>'id'", a.data)) + %Object{} = object <- Object.get_cached_by_ap_id(obj_data["id"]) do + object + |> Object.self_replies() + |> select([o], fragment("?->>'id'", o.data)) |> limit(^limit) |> Repo.all() end - set_replies(obj, replies_uris || []) + set_replies(obj_data, replies_uris || []) end defp set_replies(obj, replies_uris) when replies_uris in [nil, []] do @@ -952,7 +952,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do Map.merge(obj, %{"replies" => replies_collection}) end - def replies(%{"replies" => replies = %{}}) do + def replies(%{"replies" => replies} = _object) when is_map(replies) do replies = if is_map(replies["first"]) do replies["first"] diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 3720dda2a..d373762ea 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -2133,7 +2133,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do }) object = Object.normalize(activity) - replies_uris = Enum.map([self_reply1, self_reply2], fn a -> a.data["id"] end) + replies_uris = Enum.map([self_reply1, self_reply2], fn a -> a.object.data["id"] end) assert %{ "type" => "Collection", diff --git a/test/web/activity_pub/views/object_view_test.exs b/test/web/activity_pub/views/object_view_test.exs index 6784788cc..a9197b0c5 100644 --- a/test/web/activity_pub/views/object_view_test.exs +++ b/test/web/activity_pub/views/object_view_test.exs @@ -48,8 +48,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectViewTest do {:ok, self_reply1} = CommonAPI.post(user, %{"status" => "self-reply 1", "in_reply_to_status_id" => activity.id}) + replies_uris = [self_reply1.object.data["id"]] result = ObjectView.render("object.json", %{object: refresh_record(activity)}) - replies_uris = [self_reply1.data["id"]] assert %{ "type" => "Collection", From 6ea3c06d8d42cd92b23c6de2d041db1cfea63b5a Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 9 Feb 2020 14:09:01 +0300 Subject: [PATCH 05/10] [#1505] Minor refactoring. --- .../web/activity_pub/transmogrifier.ex | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index e89588f29..343228b37 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -914,22 +914,23 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do Based on Mastodon's ActivityPub::NoteSerializer#replies. """ def set_replies(obj_data) do - limit = Pleroma.Config.get([:activitypub, :note_replies_output_limit], 0) - replies_uris = - with true <- limit > 0 || nil, + with limit when limit > 0 <- + Pleroma.Config.get([:activitypub, :note_replies_output_limit], 0), %Object{} = object <- Object.get_cached_by_ap_id(obj_data["id"]) do object |> Object.self_replies() |> select([o], fragment("?->>'id'", o.data)) |> limit(^limit) |> Repo.all() + else + _ -> [] end - set_replies(obj_data, replies_uris || []) + set_replies(obj_data, replies_uris) end - defp set_replies(obj, replies_uris) when replies_uris in [nil, []] do + defp set_replies(obj, []) do obj end @@ -952,15 +953,12 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do Map.merge(obj, %{"replies" => replies_collection}) end - def replies(%{"replies" => replies} = _object) when is_map(replies) do - replies = - if is_map(replies["first"]) do - replies["first"] - else - replies - end + def replies(%{"replies" => %{"first" => %{"items" => items}}}) when not is_nil(items) do + items + end - replies["items"] || [] + def replies(%{"replies" => %{"items" => items}}) when not is_nil(items) do + items end def replies(_), do: [] From 24e49d14f287b0daf8c2977f2228be09139e4bf3 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 9 Feb 2020 17:34:48 +0300 Subject: [PATCH 06/10] [#1505] Removed wrapping of reply URIs into `first` element, added comments to transmogrifier tests. --- lib/pleroma/web/activity_pub/transmogrifier.ex | 12 +----------- test/web/activity_pub/transmogrifier_test.exs | 8 ++++---- test/web/activity_pub/views/object_view_test.exs | 6 ++---- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 343228b37..6f09b4994 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -935,19 +935,9 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end defp set_replies(obj, replies_uris) do - # Note: stubs (Mastodon doesn't make separate requests via those URIs in FetchRepliesService) - masto_replies_uri = nil - masto_replies_next_page_uri = nil - replies_collection = %{ "type" => "Collection", - "id" => masto_replies_uri, - "first" => %{ - "type" => "Collection", - "part_of" => masto_replies_uri, - "items" => replies_uris, - "next" => masto_replies_next_page_uri - } + "items" => replies_uris } Map.merge(obj, %{"replies" => replies_collection}) diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index d373762ea..7d9828d38 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -1361,6 +1361,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do %{data: data, items: items, collection: collection} end + # Mastodon wraps reply URIs in `replies->first->items` test "with wrapped `replies` collection, it schedules background fetching of items", %{ data: data, items: items, @@ -1378,6 +1379,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do end end + # Pleroma outputs reply URIs as `replies->items` test "it schedules background fetching of unwrapped `replies` collection items", %{ data: data, items: items, @@ -2135,10 +2137,8 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do object = Object.normalize(activity) replies_uris = Enum.map([self_reply1, self_reply2], fn a -> a.object.data["id"] end) - assert %{ - "type" => "Collection", - "first" => %{"type" => "Collection", "items" => ^replies_uris} - } = Transmogrifier.set_replies(object.data)["replies"] + assert %{"type" => "Collection", "items" => ^replies_uris} = + Transmogrifier.set_replies(object.data)["replies"] end end end diff --git a/test/web/activity_pub/views/object_view_test.exs b/test/web/activity_pub/views/object_view_test.exs index a9197b0c5..acc855b98 100644 --- a/test/web/activity_pub/views/object_view_test.exs +++ b/test/web/activity_pub/views/object_view_test.exs @@ -51,10 +51,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectViewTest do replies_uris = [self_reply1.object.data["id"]] result = ObjectView.render("object.json", %{object: refresh_record(activity)}) - assert %{ - "type" => "Collection", - "first" => %{"type" => "Collection", "items" => ^replies_uris} - } = get_in(result, ["object", "replies"]) + assert %{"type" => "Collection", "items" => ^replies_uris} = + get_in(result, ["object", "replies"]) end end From b95dd5e217e7e1477b53deb9992b65f20b5649ac Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Mon, 10 Feb 2020 11:46:16 +0300 Subject: [PATCH 07/10] [#1505] Improved replies-handling tests: updated Mastodon message fixture, used exact Pleroma federation message. --- test/fixtures/mastodon-post-activity.json | 13 ++++ test/web/activity_pub/transmogrifier_test.exs | 59 ++++++++++--------- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/test/fixtures/mastodon-post-activity.json b/test/fixtures/mastodon-post-activity.json index b91263431..5c3d22722 100644 --- a/test/fixtures/mastodon-post-activity.json +++ b/test/fixtures/mastodon-post-activity.json @@ -35,6 +35,19 @@ "inReplyTo": null, "inReplyToAtomUri": null, "published": "2018-02-12T14:08:20Z", + "replies": { + "id": "http://mastodon.example.org/users/admin/statuses/99512778738411822/replies", + "type": "Collection", + "first": { + "type": "CollectionPage", + "next": "http://mastodon.example.org/users/admin/statuses/99512778738411822/replies?min_id=99512778738411824&page=true", + "partOf": "http://mastodon.example.org/users/admin/statuses/99512778738411822/replies", + "items": [ + "http://mastodon.example.org/users/admin/statuses/99512778738411823", + "http://mastodon.example.org/users/admin/statuses/99512778738411824" + ] + } + }, "sensitive": true, "summary": "cw", "tag": [ diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 7d9828d38..bb68809b0 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -1350,27 +1350,20 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do end end - describe "`replies` handling in handle_incoming/2" do - setup do - data = - File.read!("test/fixtures/mastodon-post-activity.json") - |> Poison.decode!() - - items = ["https://shitposter.club/notice/2827873", "https://shitposter.club/notice/7387606"] - collection = %{"items" => items} - %{data: data, items: items, collection: collection} + describe "handle_incoming/2: `replies` handling:" do + clear_config([:activitypub, :note_replies_output_limit]) do + Pleroma.Config.put([:activitypub, :note_replies_output_limit], 5) end - # Mastodon wraps reply URIs in `replies->first->items` - test "with wrapped `replies` collection, it schedules background fetching of items", %{ - data: data, - items: items, - collection: collection - } do - replies = %{"first" => collection} + test "with Mastodon-formatted `replies` collection, it schedules background fetching of items" do + data = + "test/fixtures/mastodon-post-activity.json" + |> File.read!() + |> Poison.decode!() + + items = get_in(data, ["object", "replies", "first", "items"]) + assert length(items) > 0 - object = Map.put(data["object"], "replies", replies) - data = Map.put(data, "object", object) {:ok, _activity} = Transmogrifier.handle_incoming(data) for id <- items do @@ -1379,19 +1372,27 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do end end - # Pleroma outputs reply URIs as `replies->items` - test "it schedules background fetching of unwrapped `replies` collection items", %{ - data: data, - items: items, - collection: collection - } do - replies = collection + test "with Pleroma-formatted `replies` collection, it schedules background fetching of items" do + user = insert(:user) - object = Map.put(data["object"], "replies", replies) - data = Map.put(data, "object", object) - {:ok, _activity} = Transmogrifier.handle_incoming(data) + {:ok, activity} = CommonAPI.post(user, %{"status" => "post1"}) - for id <- items do + {:ok, reply1} = + CommonAPI.post(user, %{"status" => "reply1", "in_reply_to_status_id" => activity.id}) + + {:ok, reply2} = + CommonAPI.post(user, %{"status" => "reply2", "in_reply_to_status_id" => activity.id}) + + replies_uris = Enum.map([reply1, reply2], fn a -> a.object.data["id"] end) + + {:ok, federation_output} = Transmogrifier.prepare_outgoing(activity.data) + + Repo.delete(activity.object) + Repo.delete(activity) + + {:ok, _activity} = Transmogrifier.handle_incoming(federation_output) + + for id <- replies_uris do job_args = %{"op" => "fetch_remote", "id" => id} assert_enqueued(worker: Pleroma.Workers.RemoteFetcherWorker, args: job_args) end From 269d592181bff8601f6545b85158ee1c222ff20d Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 15 Feb 2020 20:41:38 +0300 Subject: [PATCH 08/10] [#1505] Restricted max thread distance for fetching replies on incoming federation (in addition to reply-to depth restriction). --- config/description.exs | 2 +- docs/introduction.md | 2 +- lib/pleroma/object/fetcher.ex | 20 +++--- .../web/activity_pub/transmogrifier.ex | 20 ++++-- lib/pleroma/web/federator/federator.ex | 16 +++-- lib/pleroma/workers/remote_fetcher_worker.ex | 4 +- test/object/fetcher_test.exs | 25 ++++++++ test/web/activity_pub/transmogrifier_test.exs | 62 ++++++++++++++++--- 8 files changed, 120 insertions(+), 31 deletions(-) diff --git a/config/description.exs b/config/description.exs index 5f7b6656c..50d058763 100644 --- a/config/description.exs +++ b/config/description.exs @@ -659,7 +659,7 @@ config :pleroma, :config_description, [ label: "Fed. incoming replies max depth", type: :integer, description: - "Max. depth of reply-to activities fetching on incoming federation, to prevent out-of-memory situations while" <> + "Max. depth of reply-to and reply activities fetching on incoming federation, to prevent out-of-memory situations while" <> " fetching very long threads. If set to `nil`, threads of any depth will be fetched. Lower this value if you experience out-of-memory crashes.", suggestions: [ 100 diff --git a/docs/introduction.md b/docs/introduction.md index 823e14f53..a915c143c 100644 --- a/docs/introduction.md +++ b/docs/introduction.md @@ -41,7 +41,7 @@ On the top right you will also see a wrench icon. This opens your personal setti This is where the interesting stuff happens! Depending on the timeline you will see different statuses, but each status has a standard structure: -- Profile pic, name and link to profile. An optional left-arrow if it's a reply to another status (hovering will reveal the replied-to status). Clicking on the profile pic will uncollapse the user's profile. +- Profile pic, name and link to profile. An optional left-arrow if it's a reply to another status (hovering will reveal the reply-to status). Clicking on the profile pic will uncollapse the user's profile. - A `+` button on the right allows you to Expand/Collapse an entire discussion thread. It also updates in realtime! - An arrow icon allows you to open the status on the instance where it's originating from. - The text of the status, including mentions and attachements. If you click on a mention, it will automatically open the profile page of that person. diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 037c42339..23ecd9e15 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -10,6 +10,7 @@ defmodule Pleroma.Object.Fetcher do alias Pleroma.Signature alias Pleroma.Web.ActivityPub.InternalFetchActor alias Pleroma.Web.ActivityPub.Transmogrifier + alias Pleroma.Web.Federator require Logger require Pleroma.Constants @@ -59,20 +60,23 @@ defmodule Pleroma.Object.Fetcher do end end - # TODO: - # This will create a Create activity, which we need internally at the moment. + # Note: will create a Create activity, which we need internally at the moment. def fetch_object_from_id(id, options \\ []) do - with {:fetch_object, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)}, - {:fetch, {:ok, data}} <- {:fetch, fetch_and_contain_remote_object_from_id(id)}, - {:normalize, nil} <- {:normalize, Object.normalize(data, false)}, + with {_, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)}, + {_, true} <- {:allowed_depth, Federator.allowed_thread_distance?(options[:depth])}, + {_, {:ok, data}} <- {:fetch, fetch_and_contain_remote_object_from_id(id)}, + {_, nil} <- {:normalize, Object.normalize(data, false)}, params <- prepare_activity_params(data), - {:containment, :ok} <- {:containment, Containment.contain_origin(id, params)}, - {:transmogrifier, {:ok, activity}} <- + {_, :ok} <- {:containment, Containment.contain_origin(id, params)}, + {_, {:ok, activity}} <- {:transmogrifier, Transmogrifier.handle_incoming(params, options)}, - {:object, _data, %Object{} = object} <- + {_, _data, %Object{} = object} <- {:object, data, Object.normalize(activity, false)} do {:ok, object} else + {:allowed_depth, false} -> + {:error, "Max thread distance exceeded."} + {:containment, _} -> {:error, "Object containment failed."} diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 6f09b4994..5bd2baca4 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -156,8 +156,9 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do when not is_nil(in_reply_to) do in_reply_to_id = prepare_in_reply_to(in_reply_to) object = Map.put(object, "inReplyToAtomUri", in_reply_to_id) + depth = (options[:depth] || 0) + 1 - if Federator.allowed_incoming_reply_depth?(options[:depth]) do + if Federator.allowed_thread_distance?(depth) do with {:ok, replied_object} <- get_obj_helper(in_reply_to_id, options), %Activity{} = _ <- Activity.get_create_by_object_ap_id(replied_object.data["id"]) do object @@ -312,7 +313,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do def fix_type(%{"inReplyTo" => reply_id, "name" => _} = object, options) when is_binary(reply_id) do - with true <- Federator.allowed_incoming_reply_depth?(options[:depth]), + with true <- Federator.allowed_thread_distance?(options[:depth]), {:ok, %{data: %{"type" => "Question"} = _} = _} <- get_obj_helper(reply_id, options) do Map.put(object, "type", "Answer") else @@ -406,7 +407,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do with nil <- Activity.get_create_by_object_ap_id(object["id"]), {:ok, %User{} = user} <- User.get_or_fetch_by_ap_id(data["actor"]) do - options = Keyword.put(options, :depth, (options[:depth] || 0) + 1) object = fix_object(object, options) params = %{ @@ -425,8 +425,15 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do } with {:ok, created_activity} <- ActivityPub.create(params) do - for reply_id <- replies(object) do - Pleroma.Workers.RemoteFetcherWorker.enqueue("fetch_remote", %{"id" => reply_id}) + reply_depth = (options[:depth] || 0) + 1 + + if Federator.allowed_thread_distance?(reply_depth) do + for reply_id <- replies(object) do + Pleroma.Workers.RemoteFetcherWorker.enqueue("fetch_remote", %{ + "id" => reply_id, + "depth" => reply_depth + }) + end end {:ok, created_activity} @@ -448,7 +455,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do |> fix_addressing with {:ok, %User{} = user} <- User.get_or_fetch_by_ap_id(data["actor"]) do - options = Keyword.put(options, :depth, (options[:depth] || 0) + 1) + reply_depth = (options[:depth] || 0) + 1 + options = Keyword.put(options, :depth, reply_depth) object = fix_object(object, options) params = %{ diff --git a/lib/pleroma/web/federator/federator.ex b/lib/pleroma/web/federator/federator.ex index f506a7d24..013fb5b70 100644 --- a/lib/pleroma/web/federator/federator.ex +++ b/lib/pleroma/web/federator/federator.ex @@ -15,13 +15,19 @@ defmodule Pleroma.Web.Federator do require Logger - @doc "Addresses [memory leaks on recursive replies fetching](https://git.pleroma.social/pleroma/pleroma/issues/161)" + @doc """ + Returns `true` if the distance to target object does not exceed max configured value. + Serves to prevent fetching of very long threads, especially useful on smaller instances. + Addresses [memory leaks on recursive replies fetching](https://git.pleroma.social/pleroma/pleroma/issues/161). + Applies to fetching of both ancestor (reply-to) and child (reply) objects. + """ # credo:disable-for-previous-line Credo.Check.Readability.MaxLineLength - def allowed_incoming_reply_depth?(depth) do - max_replies_depth = Pleroma.Config.get([:instance, :federation_incoming_replies_max_depth]) + def allowed_thread_distance?(distance) do + max_distance = Pleroma.Config.get([:instance, :federation_incoming_replies_max_depth]) - if max_replies_depth do - (depth || 1) <= max_replies_depth + if max_distance && max_distance >= 0 do + # Default depth is 0 (an object has zero distance from itself in its thread) + (distance || 0) <= max_distance else true end diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index 52db6059b..e860ca869 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -12,9 +12,9 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do %{ "op" => "fetch_remote", "id" => id - }, + } = args, _job ) do - {:ok, _object} = Fetcher.fetch_object_from_id(id) + {:ok, _object} = Fetcher.fetch_object_from_id(id, depth: args["depth"]) end end diff --git a/test/object/fetcher_test.exs b/test/object/fetcher_test.exs index 2aad7a588..3afd35648 100644 --- a/test/object/fetcher_test.exs +++ b/test/object/fetcher_test.exs @@ -26,6 +26,31 @@ defmodule Pleroma.Object.FetcherTest do :ok end + describe "max thread distance restriction" do + @ap_id "http://mastodon.example.org/@admin/99541947525187367" + + clear_config([:instance, :federation_incoming_replies_max_depth]) + + test "it returns thread depth exceeded error if thread depth is exceeded" do + Pleroma.Config.put([:instance, :federation_incoming_replies_max_depth], 0) + + assert {:error, "Max thread distance exceeded."} = + Fetcher.fetch_object_from_id(@ap_id, depth: 1) + end + + test "it fetches object if max thread depth is restricted to 0 and depth is not specified" do + Pleroma.Config.put([:instance, :federation_incoming_replies_max_depth], 0) + + assert {:ok, _} = Fetcher.fetch_object_from_id(@ap_id) + end + + test "it fetches object if requested depth does not exceed max thread depth" do + Pleroma.Config.put([:instance, :federation_incoming_replies_max_depth], 10) + + assert {:ok, _} = Fetcher.fetch_object_from_id(@ap_id, depth: 10) + end + end + describe "actor origin containment" do test "it rejects objects with a bogus origin" do {:error, _} = Fetcher.fetch_object_from_id("https://info.pleroma.site/activity.json") diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index bb68809b0..937f78cbe 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -42,7 +42,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do end @tag capture_log: true - test "it fetches replied-to activities if we don't have them" do + test "it fetches reply-to activities if we don't have them" do data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!() @@ -63,7 +63,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert returned_object.data["inReplyToAtomUri"] == "https://shitposter.club/notice/2827873" end - test "it does not fetch replied-to activities beyond max_replies_depth" do + test "it does not fetch reply-to activities beyond max replies depth limit" do data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!() @@ -75,7 +75,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do data = Map.put(data, "object", object) with_mock Pleroma.Web.Federator, - allowed_incoming_reply_depth?: fn _ -> false end do + allowed_thread_distance?: fn _ -> false end do {:ok, returned_activity} = Transmogrifier.handle_incoming(data) returned_object = Object.normalize(returned_activity, false) @@ -1350,12 +1350,14 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do end end - describe "handle_incoming/2: `replies` handling:" do + describe "`handle_incoming/2`, Mastodon format `replies` handling" do clear_config([:activitypub, :note_replies_output_limit]) do Pleroma.Config.put([:activitypub, :note_replies_output_limit], 5) end - test "with Mastodon-formatted `replies` collection, it schedules background fetching of items" do + clear_config([:instance, :federation_incoming_replies_max_depth]) + + setup do data = "test/fixtures/mastodon-post-activity.json" |> File.read!() @@ -1364,15 +1366,41 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do items = get_in(data, ["object", "replies", "first", "items"]) assert length(items) > 0 + %{data: data, items: items} + end + + test "schedules background fetching of `replies` items if max thread depth limit allows", %{ + data: data, + items: items + } do + Pleroma.Config.put([:instance, :federation_incoming_replies_max_depth], 10) + {:ok, _activity} = Transmogrifier.handle_incoming(data) for id <- items do - job_args = %{"op" => "fetch_remote", "id" => id} + job_args = %{"op" => "fetch_remote", "id" => id, "depth" => 1} assert_enqueued(worker: Pleroma.Workers.RemoteFetcherWorker, args: job_args) end end - test "with Pleroma-formatted `replies` collection, it schedules background fetching of items" do + test "does NOT schedule background fetching of `replies` beyond max thread depth limit allows", + %{data: data} do + Pleroma.Config.put([:instance, :federation_incoming_replies_max_depth], 0) + + {:ok, _activity} = Transmogrifier.handle_incoming(data) + + assert all_enqueued(worker: Pleroma.Workers.RemoteFetcherWorker) == [] + end + end + + describe "`handle_incoming/2`, Pleroma format `replies` handling" do + clear_config([:activitypub, :note_replies_output_limit]) do + Pleroma.Config.put([:activitypub, :note_replies_output_limit], 5) + end + + clear_config([:instance, :federation_incoming_replies_max_depth]) + + setup do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => "post1"}) @@ -1390,13 +1418,31 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do Repo.delete(activity.object) Repo.delete(activity) + %{federation_output: federation_output, replies_uris: replies_uris} + end + + test "schedules background fetching of `replies` items if max thread depth limit allows", %{ + federation_output: federation_output, + replies_uris: replies_uris + } do + Pleroma.Config.put([:instance, :federation_incoming_replies_max_depth], 1) + {:ok, _activity} = Transmogrifier.handle_incoming(federation_output) for id <- replies_uris do - job_args = %{"op" => "fetch_remote", "id" => id} + job_args = %{"op" => "fetch_remote", "id" => id, "depth" => 1} assert_enqueued(worker: Pleroma.Workers.RemoteFetcherWorker, args: job_args) end end + + test "does NOT schedule background fetching of `replies` beyond max thread depth limit allows", + %{federation_output: federation_output} do + Pleroma.Config.put([:instance, :federation_incoming_replies_max_depth], 0) + + {:ok, _activity} = Transmogrifier.handle_incoming(federation_output) + + assert all_enqueued(worker: Pleroma.Workers.RemoteFetcherWorker) == [] + end end describe "prepare outgoing" do From 343229465753ebcc59002a2dd604a910a8ce462d Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 20 Feb 2020 14:48:46 +0300 Subject: [PATCH 09/10] [#1505] Fixed @spec for Queries.by_object_in_reply_to_id/3 --- lib/pleroma/activity/queries.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/activity/queries.ex b/lib/pleroma/activity/queries.ex index c17affec9..363727c80 100644 --- a/lib/pleroma/activity/queries.ex +++ b/lib/pleroma/activity/queries.ex @@ -63,7 +63,7 @@ defmodule Pleroma.Activity.Queries do ) end - @spec by_object_id(query, String.t()) :: query + @spec by_object_in_reply_to_id(query, String.t(), keyword()) :: query def by_object_in_reply_to_id(query, in_reply_to_id, opts \\ []) do query = if opts[:skip_preloading] do From 0d14c3f41053f97d23fa9295745a817c08010969 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 20 Feb 2020 15:18:28 +0300 Subject: [PATCH 10/10] [#1505] Typo fix. --- config/config.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.exs b/config/config.exs index 47b53d5af..c5a56b14d 100644 --- a/config/config.exs +++ b/config/config.exs @@ -482,7 +482,7 @@ config :pleroma, Oban, scheduled_activities: 10, background: 5, remote_fetcher: 2, - attachments_cleanup: 5 + attachments_cleanup: 5, new_users_digest: 1 ], crontab: [