From 3d422ef3256e9eeef79d0c78743e19d8435dc352 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 5 Jun 2025 16:38:40 -0700 Subject: [PATCH] Reachability refactor The result of Oban jobs determine the reachability status. Publisher jobs will cancel themselves at execution time if the target server is now unreachable. Receiving activities does not immediately mark a server as reachable, but creates a ReachabilityWorker job to validate. A Cron will execute daily to test all unreachable servers. --- changelog.d/reachabililty.change | 1 + config/config.exs | 4 +- lib/pleroma/instances.ex | 20 +--- lib/pleroma/instances/instance.ex | 18 +-- lib/pleroma/object/fetcher.ex | 5 - .../activity_pub/activity_pub_controller.ex | 10 -- lib/pleroma/web/activity_pub/publisher.ex | 22 +--- .../controllers/instances_controller.ex | 2 +- .../cron/schedule_reachability_worker.ex | 33 ++++++ lib/pleroma/workers/publisher_worker.ex | 26 +++- lib/pleroma/workers/reachability_worker.ex | 31 +++++ lib/pleroma/workers/receiver_worker.ex | 11 ++ lib/pleroma/workers/remote_fetcher_worker.ex | 10 ++ test/pleroma/instances/instance_test.exs | 9 +- test/pleroma/instances_test.exs | 78 ++---------- test/pleroma/object/fetcher_test.exs | 12 -- .../activity_pub_controller_test.exs | 36 ------ .../web/activity_pub/publisher_test.exs | 111 ------------------ test/pleroma/web/federator_test.exs | 9 +- .../controllers/instances_controller_test.exs | 7 +- .../schedule_reachability_worker_test.exs | 52 ++++++++ .../pleroma/workers/publisher_worker_test.exs | 83 +++++++++++++ test/pleroma/workers/receiver_worker_test.exs | 61 +++++++++- .../workers/remote_fetcher_worker_test.exs | 2 +- 24 files changed, 341 insertions(+), 312 deletions(-) create mode 100644 changelog.d/reachabililty.change create mode 100644 lib/pleroma/workers/cron/schedule_reachability_worker.ex create mode 100644 lib/pleroma/workers/reachability_worker.ex create mode 100644 test/pleroma/workers/cron/schedule_reachability_worker_test.exs diff --git a/changelog.d/reachabililty.change b/changelog.d/reachabililty.change new file mode 100644 index 000000000..71b9514be --- /dev/null +++ b/changelog.d/reachabililty.change @@ -0,0 +1 @@ +Improved the logic of how we determine if a server is unreachable. diff --git a/config/config.exs b/config/config.exs index a231c5ba0..d164f8389 100644 --- a/config/config.exs +++ b/config/config.exs @@ -194,7 +194,6 @@ config :pleroma, :instance, account_approval_required: false, federating: true, federation_incoming_replies_max_depth: 100, - federation_reachability_timeout_days: 7, allow_relay: true, public: true, quarantined_instances: [], @@ -603,7 +602,8 @@ config :pleroma, Oban, crontab: [ {"0 0 * * 0", Pleroma.Workers.Cron.DigestEmailsWorker}, {"0 0 * * *", Pleroma.Workers.Cron.NewUsersDigestWorker}, - {"*/10 * * * *", Pleroma.Workers.Cron.AppCleanupWorker} + {"*/10 * * * *", Pleroma.Workers.Cron.AppCleanupWorker}, + {"0 0 * * *", Pleroma.Workers.Cron.ScheduleReachabilityWorker} ] config :pleroma, Pleroma.Formatter, diff --git a/lib/pleroma/instances.ex b/lib/pleroma/instances.ex index b6d83f591..9237e0944 100644 --- a/lib/pleroma/instances.ex +++ b/lib/pleroma/instances.ex @@ -15,25 +15,7 @@ defmodule Pleroma.Instances do defdelegate set_unreachable(url_or_host, unreachable_since \\ nil), to: Instance - defdelegate get_consistently_unreachable, to: Instance - - def set_consistently_unreachable(url_or_host), - do: set_unreachable(url_or_host, reachability_datetime_threshold()) - - def reachability_datetime_threshold do - federation_reachability_timeout_days = - Pleroma.Config.get([:instance, :federation_reachability_timeout_days], 0) - - if federation_reachability_timeout_days > 0 do - NaiveDateTime.add( - NaiveDateTime.utc_now(), - -federation_reachability_timeout_days * 24 * 3600, - :second - ) - else - ~N[0000-01-01 00:00:00] - end - end + defdelegate get_unreachable, to: Instance def host(url_or_host) when is_binary(url_or_host) do if url_or_host =~ ~r/^http/i do diff --git a/lib/pleroma/instances/instance.ex b/lib/pleroma/instances/instance.ex index 33f1229d0..baccc314c 100644 --- a/lib/pleroma/instances/instance.ex +++ b/lib/pleroma/instances/instance.ex @@ -51,7 +51,7 @@ defmodule Pleroma.Instances.Instance do |> cast(params, [:software_name, :software_version, :software_repository]) end - def filter_reachable([]), do: %{} + def filter_reachable([]), do: [] def filter_reachable(urls_or_hosts) when is_list(urls_or_hosts) do hosts = @@ -68,19 +68,15 @@ defmodule Pleroma.Instances.Instance do ) |> Map.new(& &1) - reachability_datetime_threshold = Instances.reachability_datetime_threshold() - for entry <- Enum.filter(urls_or_hosts, &is_binary/1) do host = host(entry) unreachable_since = unreachable_since_by_host[host] - if !unreachable_since || - NaiveDateTime.compare(unreachable_since, reachability_datetime_threshold) == :gt do - {entry, unreachable_since} + if is_nil(unreachable_since) do + entry end end |> Enum.filter(& &1) - |> Map.new(& &1) end def reachable?(url_or_host) when is_binary(url_or_host) do @@ -88,7 +84,7 @@ defmodule Pleroma.Instances.Instance do from(i in Instance, where: i.host == ^host(url_or_host) and - i.unreachable_since <= ^Instances.reachability_datetime_threshold(), + i.unreachable_since <= ^NaiveDateTime.utc_now(), select: true ) ) @@ -132,11 +128,9 @@ defmodule Pleroma.Instances.Instance do def set_unreachable(_, _), do: {:error, nil} - def get_consistently_unreachable do - reachability_datetime_threshold = Instances.reachability_datetime_threshold() - + def get_unreachable do from(i in Instance, - where: ^reachability_datetime_threshold > i.unreachable_since, + where: not is_nil(i.unreachable_since), order_by: i.unreachable_since, select: {i.host, i.unreachable_since} ) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index b54ef9ce5..ea5480a41 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -4,7 +4,6 @@ defmodule Pleroma.Object.Fetcher do alias Pleroma.HTTP - alias Pleroma.Instances alias Pleroma.Maps alias Pleroma.Object alias Pleroma.Object.Containment @@ -152,10 +151,6 @@ defmodule Pleroma.Object.Fetcher do {:ok, body} <- get_object(id), {:ok, data} <- safe_json_decode(body), :ok <- Containment.contain_origin_from_id(id, data) do - if not Instances.reachable?(id) do - Instances.set_reachable(id) - end - {:ok, data} else {:scheme, _} -> diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 7ac0bbab4..2ee72c49a 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -53,7 +53,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do ) plug(:log_inbox_metadata when action in [:inbox]) - plug(:set_requester_reachable when action in [:inbox]) plug(:relay_active? when action in [:relay]) defp relay_active?(conn, _) do @@ -520,15 +519,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do |> json(dgettext("errors", "error")) end - defp set_requester_reachable(%Plug.Conn{} = conn, _) do - with actor <- conn.params["actor"], - true <- is_binary(actor) do - Pleroma.Instances.set_reachable(actor) - end - - conn - end - defp log_inbox_metadata(%{params: %{"actor" => actor, "type" => type}} = conn, _) do Logger.metadata(actor: actor, type: type) conn diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index 0de3a0d43..78312b771 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -148,17 +148,9 @@ defmodule Pleroma.Web.ActivityPub.Publisher do {"digest", p.digest} ] ) do - if not is_nil(p.unreachable_since) do - Instances.set_reachable(p.inbox) - end - result else {_post_result, %{status: code} = response} = e -> - if is_nil(p.unreachable_since) do - Instances.set_unreachable(p.inbox) - end - Logger.metadata(activity: p.activity_id, inbox: p.inbox, status: code) Logger.error("Publisher failed to inbox #{p.inbox} with status #{code}") @@ -179,10 +171,6 @@ defmodule Pleroma.Web.ActivityPub.Publisher do connection_pool_snooze() e -> - if is_nil(p.unreachable_since) do - Instances.set_unreachable(p.inbox) - end - Logger.metadata(activity: p.activity_id, inbox: p.inbox) Logger.error("Publisher failed to inbox #{p.inbox} #{inspect(e)}") {:error, e} @@ -308,7 +296,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do Repo.checkout(fn -> Enum.each(inboxes, fn inboxes -> - Enum.each(inboxes, fn {inbox, unreachable_since} -> + Enum.each(inboxes, fn inbox -> %User{ap_id: ap_id} = Enum.find(recipients, fn actor -> actor.inbox == inbox end) # Get all the recipients on the same host and add them to cc. Otherwise, a remote @@ -318,8 +306,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do __MODULE__.enqueue_one(%{ inbox: inbox, cc: cc, - activity_id: activity.id, - unreachable_since: unreachable_since + activity_id: activity.id }) end) end) @@ -352,12 +339,11 @@ defmodule Pleroma.Web.ActivityPub.Publisher do |> Enum.each(fn {inboxes, priority} -> inboxes |> Instances.filter_reachable() - |> Enum.each(fn {inbox, unreachable_since} -> + |> Enum.each(fn inbox -> __MODULE__.enqueue_one( %{ inbox: inbox, - activity_id: activity.id, - unreachable_since: unreachable_since + activity_id: activity.id }, priority: priority ) diff --git a/lib/pleroma/web/pleroma_api/controllers/instances_controller.ex b/lib/pleroma/web/pleroma_api/controllers/instances_controller.ex index 6257e3153..85cfe9f00 100644 --- a/lib/pleroma/web/pleroma_api/controllers/instances_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/instances_controller.ex @@ -13,7 +13,7 @@ defmodule Pleroma.Web.PleromaAPI.InstancesController do def show(conn, _params) do unreachable = - Instances.get_consistently_unreachable() + Instances.get_unreachable() |> Map.new(fn {host, date} -> {host, to_string(date)} end) json(conn, %{"unreachable" => unreachable}) diff --git a/lib/pleroma/workers/cron/schedule_reachability_worker.ex b/lib/pleroma/workers/cron/schedule_reachability_worker.ex new file mode 100644 index 000000000..a0b8e261c --- /dev/null +++ b/lib/pleroma/workers/cron/schedule_reachability_worker.ex @@ -0,0 +1,33 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2022 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.Cron.ScheduleReachabilityWorker do + use Oban.Worker, + queue: :background, + max_attempts: 2 + + alias Pleroma.Instances + alias Pleroma.Repo + + @impl true + def perform(_job) do + unreachable_servers = Instances.get_unreachable() + + jobs = + unreachable_servers + |> Enum.map(fn {domain, _} -> + Pleroma.Workers.ReachabilityWorker.new(%{"domain" => domain}) + end) + + case Repo.transaction(fn -> + Enum.each(jobs, &Oban.insert/1) + end) do + {:ok, _} -> + :ok + + {:error, reason} -> + {:error, reason} + end + end +end diff --git a/lib/pleroma/workers/publisher_worker.ex b/lib/pleroma/workers/publisher_worker.ex index 7d9b022de..10736bef5 100644 --- a/lib/pleroma/workers/publisher_worker.ex +++ b/lib/pleroma/workers/publisher_worker.ex @@ -4,6 +4,7 @@ defmodule Pleroma.Workers.PublisherWorker do alias Pleroma.Activity + alias Pleroma.Instances alias Pleroma.Web.Federator use Oban.Worker, queue: :federator_outgoing, max_attempts: 5 @@ -14,9 +15,30 @@ defmodule Pleroma.Workers.PublisherWorker do Federator.perform(:publish, activity) end - def perform(%Job{args: %{"op" => "publish_one", "params" => params}}) do + def perform(%Job{args: %{"op" => "publish_one", "params" => params}} = job) do params = Map.new(params, fn {k, v} -> {String.to_atom(k), v} end) - Federator.perform(:publish_one, params) + + # Cancel / skip the job if this server believed to be unreachable now + if not Instances.reachable?(params.inbox) do + {:cancel, :unreachable} + else + case Federator.perform(:publish_one, params) do + {:ok, _} -> + :ok + + {:error, _} = error -> + # Only mark as unreachable on final failure + if job.attempt == job.max_attempts do + Instances.set_unreachable(params.inbox) + end + + error + + error -> + # Unexpected error, may have been client side + error + end + end end @impl true diff --git a/lib/pleroma/workers/reachability_worker.ex b/lib/pleroma/workers/reachability_worker.ex new file mode 100644 index 000000000..3a11dfe2a --- /dev/null +++ b/lib/pleroma/workers/reachability_worker.ex @@ -0,0 +1,31 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2022 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.ReachabilityWorker do + use Oban.Worker, + queue: :background, + max_attempts: 3, + unique: [period: :infinity, states: [:available, :scheduled]] + + alias Pleroma.HTTP + alias Pleroma.Instances + + @impl true + def perform(%Oban.Job{args: %{"domain" => domain}}) do + case HTTP.get("https://#{domain}/.well-known/nodeinfo") do + {:ok, %{status: status}} when status in 200..299 -> + Instances.set_reachable("https://#{domain}") + :ok + + {:ok, %{status: _status}} -> + {:error, :unreachable} + + {:error, _} = error -> + error + end + end + + @impl true + def timeout(_job), do: :timer.seconds(5) +end diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 11b672bef..e2c950967 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -3,6 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Workers.ReceiverWorker do + alias Pleroma.Instances alias Pleroma.Signature alias Pleroma.User alias Pleroma.Web.Federator @@ -37,6 +38,11 @@ defmodule Pleroma.Workers.ReceiverWorker do {:ok, _public_key} <- Signature.refetch_public_key(conn_data), {:signature, true} <- {:signature, Signature.validate_signature(conn_data)}, {:ok, res} <- Federator.perform(:incoming_ap_doc, params) do + unless Instances.reachable?(params["actor"]) do + domain = URI.parse(params["actor"]).host + Oban.insert(Pleroma.Workers.ReachabilityWorker.new(%{"domain" => domain})) + end + {:ok, res} else e -> process_errors(e) @@ -45,6 +51,11 @@ defmodule Pleroma.Workers.ReceiverWorker do def perform(%Job{args: %{"op" => "incoming_ap_doc", "params" => params}}) do with {:ok, res} <- Federator.perform(:incoming_ap_doc, params) do + unless Instances.reachable?(params["actor"]) do + domain = URI.parse(params["actor"]).host + Oban.insert(Pleroma.Workers.ReachabilityWorker.new(%{"domain" => domain})) + end + {:ok, res} else e -> process_errors(e) diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index aa09362f5..5f57ec2d7 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -3,6 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Workers.RemoteFetcherWorker do + alias Pleroma.Instances alias Pleroma.Object.Fetcher use Oban.Worker, queue: :background, unique: [period: :infinity] @@ -11,6 +12,15 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do def perform(%Job{args: %{"op" => "fetch_remote", "id" => id} = args}) do case Fetcher.fetch_object_from_id(id, depth: args["depth"]) do {:ok, _object} -> + # Mark the server as reachable since we successfully fetched an object + case URI.parse(id) do + %URI{host: host} when not is_nil(host) -> + Instances.set_reachable("https://#{host}") + + _ -> + :ok + end + :ok {:allowed_depth, false} -> diff --git a/test/pleroma/instances/instance_test.exs b/test/pleroma/instances/instance_test.exs index 6a718be21..f195d9bd6 100644 --- a/test/pleroma/instances/instance_test.exs +++ b/test/pleroma/instances/instance_test.exs @@ -3,7 +3,6 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Instances.InstanceTest do - alias Pleroma.Instances alias Pleroma.Instances.Instance alias Pleroma.Repo alias Pleroma.Tests.ObanHelpers @@ -14,8 +13,6 @@ defmodule Pleroma.Instances.InstanceTest do import ExUnit.CaptureLog import Pleroma.Factory - setup_all do: clear_config([:instance, :federation_reachability_timeout_days], 1) - describe "set_reachable/1" do test "clears `unreachable_since` of existing matching Instance record having non-nil `unreachable_since`" do unreachable_since = NaiveDateTime.to_iso8601(NaiveDateTime.utc_now()) @@ -145,7 +142,11 @@ defmodule Pleroma.Instances.InstanceTest do end test "Doesn't scrapes unreachable instances" do - instance = insert(:instance, unreachable_since: Instances.reachability_datetime_threshold()) + instance = + insert(:instance, + unreachable_since: NaiveDateTime.utc_now() |> NaiveDateTime.add(-:timer.hours(24)) + ) + url = "https://" <> instance.host assert capture_log(fn -> assert nil == Instance.get_or_update_favicon(URI.parse(url)) end) =~ diff --git a/test/pleroma/instances_test.exs b/test/pleroma/instances_test.exs index 96fa9cffe..cbafbfa44 100644 --- a/test/pleroma/instances_test.exs +++ b/test/pleroma/instances_test.exs @@ -7,73 +7,40 @@ defmodule Pleroma.InstancesTest do use Pleroma.DataCase - setup_all do: clear_config([:instance, :federation_reachability_timeout_days], 1) - describe "reachable?/1" do test "returns `true` for host / url with unknown reachability status" do assert Instances.reachable?("unknown.site") assert Instances.reachable?("http://unknown.site") end - - test "returns `false` for host / url marked unreachable for at least `reachability_datetime_threshold()`" do - host = "consistently-unreachable.name" - Instances.set_consistently_unreachable(host) - - refute Instances.reachable?(host) - refute Instances.reachable?("http://#{host}/path") - end - - test "returns `true` for host / url marked unreachable for less than `reachability_datetime_threshold()`" do - url = "http://eventually-unreachable.name/path" - - Instances.set_unreachable(url) - - assert Instances.reachable?(url) - assert Instances.reachable?(URI.parse(url).host) - end - - test "raises FunctionClauseError exception on non-binary input" do - assert_raise FunctionClauseError, fn -> Instances.reachable?(nil) end - assert_raise FunctionClauseError, fn -> Instances.reachable?(1) end - end end describe "filter_reachable/1" do setup do - host = "consistently-unreachable.name" - url1 = "http://eventually-unreachable.com/path" - url2 = "http://domain.com/path" + unreachable_host = "consistently-unreachable.name" + reachable_host = "http://domain.com/path" - Instances.set_consistently_unreachable(host) - Instances.set_unreachable(url1) + Instances.set_unreachable(unreachable_host) - result = Instances.filter_reachable([host, url1, url2, nil]) - %{result: result, url1: url1, url2: url2} + result = Instances.filter_reachable([unreachable_host, reachable_host, nil]) + %{result: result, reachable_host: reachable_host, unreachable_host: unreachable_host} end - test "returns a map with keys containing 'not marked consistently unreachable' elements of supplied list", - %{result: result, url1: url1, url2: url2} do - assert is_map(result) - assert Enum.sort([url1, url2]) == result |> Map.keys() |> Enum.sort() + test "returns a list of only reachable elements", + %{result: result, reachable_host: reachable_host} do + assert is_list(result) + assert [reachable_host] == result end - test "returns a map with `unreachable_since` values for keys", - %{result: result, url1: url1, url2: url2} do - assert is_map(result) - assert %NaiveDateTime{} = result[url1] - assert is_nil(result[url2]) - end - - test "returns an empty map for empty list or list containing no hosts / url" do - assert %{} == Instances.filter_reachable([]) - assert %{} == Instances.filter_reachable([nil]) + test "returns an empty list when provided no data" do + assert [] == Instances.filter_reachable([]) + assert [] == Instances.filter_reachable([nil]) end end describe "set_reachable/1" do test "sets unreachable url or host reachable" do host = "domain.com" - Instances.set_consistently_unreachable(host) + Instances.set_unreachable(host) refute Instances.reachable?(host) Instances.set_reachable(host) @@ -102,23 +69,4 @@ defmodule Pleroma.InstancesTest do assert {:error, _} = Instances.set_unreachable(1) end end - - describe "set_consistently_unreachable/1" do - test "sets reachable url or host unreachable" do - url = "http://domain.com?q=" - assert Instances.reachable?(url) - - Instances.set_consistently_unreachable(url) - refute Instances.reachable?(url) - end - - test "keeps unreachable url or host unreachable" do - host = "site.name" - Instances.set_consistently_unreachable(host) - refute Instances.reachable?(host) - - Instances.set_consistently_unreachable(host) - refute Instances.reachable?(host) - end - end end diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 7ba5090e1..9afa34fa2 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -6,7 +6,6 @@ defmodule Pleroma.Object.FetcherTest do use Pleroma.DataCase alias Pleroma.Activity - alias Pleroma.Instances alias Pleroma.Object alias Pleroma.Object.Fetcher alias Pleroma.Web.ActivityPub.ObjectValidator @@ -250,17 +249,6 @@ defmodule Pleroma.Object.FetcherTest do result = Fetcher.fetch_object_from_id("https://example.com/objects/no-content-type") assert {:fetch, {:error, nil}} = result end - - test "it resets instance reachability on successful fetch" do - id = "http://mastodon.example.org/@admin/99541947525187367" - Instances.set_consistently_unreachable(id) - refute Instances.reachable?(id) - - {:ok, _object} = - Fetcher.fetch_object_from_id("http://mastodon.example.org/@admin/99541947525187367") - - assert Instances.reachable?(id) - end end describe "implementation quirks" do diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 46b3d5f0d..d9be82e64 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -8,7 +8,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do alias Pleroma.Activity alias Pleroma.Delivery - alias Pleroma.Instances alias Pleroma.Object alias Pleroma.Tests.ObanHelpers alias Pleroma.User @@ -601,23 +600,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do assert Activity.get_by_ap_id(data["id"]) end - test "it clears `unreachable` federation status of the sender", %{conn: conn} do - data = File.read!("test/fixtures/mastodon-post-activity.json") |> Jason.decode!() - - sender_url = data["actor"] - Instances.set_consistently_unreachable(sender_url) - refute Instances.reachable?(sender_url) - - conn = - conn - |> assign(:valid_signature, true) - |> put_req_header("content-type", "application/activity+json") - |> post("/inbox", data) - - assert "ok" == json_response(conn, 200) - assert Instances.reachable?(sender_url) - end - test "accept follow activity", %{conn: conn} do clear_config([:instance, :federating], true) relay = Relay.get_actor() @@ -1108,24 +1090,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do assert response(conn, 200) =~ note_object.data["content"] end - test "it clears `unreachable` federation status of the sender", %{conn: conn, data: data} do - user = insert(:user) - data = Map.put(data, "bcc", [user.ap_id]) - - sender_host = URI.parse(data["actor"]).host - Instances.set_consistently_unreachable(sender_host) - refute Instances.reachable?(sender_host) - - conn = - conn - |> assign(:valid_signature, true) - |> put_req_header("content-type", "application/activity+json") - |> post("/users/#{user.nickname}/inbox", data) - - assert "ok" == json_response(conn, 200) - assert Instances.reachable?(sender_host) - end - test "it removes all follower collections but actor's", %{conn: conn} do [actor, recipient] = insert_pair(:user) diff --git a/test/pleroma/web/activity_pub/publisher_test.exs b/test/pleroma/web/activity_pub/publisher_test.exs index 99ed42877..a6f25c9a7 100644 --- a/test/pleroma/web/activity_pub/publisher_test.exs +++ b/test/pleroma/web/activity_pub/publisher_test.exs @@ -6,13 +6,11 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do use Oban.Testing, repo: Pleroma.Repo use Pleroma.Web.ConnCase - import ExUnit.CaptureLog import Pleroma.Factory import Tesla.Mock import Mock alias Pleroma.Activity - alias Pleroma.Instances alias Pleroma.Object alias Pleroma.Tests.ObanHelpers alias Pleroma.Web.ActivityPub.Publisher @@ -167,115 +165,6 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do }) |> Publisher.publish_one() end - - test_with_mock "calls `Instances.set_reachable` on successful federation if `unreachable_since` is set", - Instances, - [:passthrough], - [] do - _actor = insert(:user) - inbox = "http://200.site/users/nick1/inbox" - activity = insert(:note_activity) - - assert {:ok, _} = - Publisher.prepare_one(%{ - inbox: inbox, - activity_id: activity.id, - unreachable_since: NaiveDateTime.utc_now() |> NaiveDateTime.to_string() - }) - |> Publisher.publish_one() - - assert called(Instances.set_reachable(inbox)) - end - - test_with_mock "does NOT call `Instances.set_reachable` on successful federation if `unreachable_since` is nil", - Instances, - [:passthrough], - [] do - _actor = insert(:user) - inbox = "http://200.site/users/nick1/inbox" - activity = insert(:note_activity) - - assert {:ok, _} = - Publisher.prepare_one(%{ - inbox: inbox, - activity_id: activity.id, - unreachable_since: nil - }) - |> Publisher.publish_one() - - refute called(Instances.set_reachable(inbox)) - end - - test_with_mock "calls `Instances.set_unreachable` on target inbox on non-2xx HTTP response code", - Instances, - [:passthrough], - [] do - _actor = insert(:user) - inbox = "http://404.site/users/nick1/inbox" - activity = insert(:note_activity) - - assert {:cancel, _} = - Publisher.prepare_one(%{inbox: inbox, activity_id: activity.id}) - |> Publisher.publish_one() - - assert called(Instances.set_unreachable(inbox)) - end - - test_with_mock "it calls `Instances.set_unreachable` on target inbox on request error of any kind", - Instances, - [:passthrough], - [] do - _actor = insert(:user) - inbox = "http://connrefused.site/users/nick1/inbox" - activity = insert(:note_activity) - - assert capture_log(fn -> - assert {:error, _} = - Publisher.prepare_one(%{ - inbox: inbox, - activity_id: activity.id - }) - |> Publisher.publish_one() - end) =~ "connrefused" - - assert called(Instances.set_unreachable(inbox)) - end - - test_with_mock "does NOT call `Instances.set_unreachable` if target is reachable", - Instances, - [:passthrough], - [] do - _actor = insert(:user) - inbox = "http://200.site/users/nick1/inbox" - activity = insert(:note_activity) - - assert {:ok, _} = - Publisher.prepare_one(%{inbox: inbox, activity_id: activity.id}) - |> Publisher.publish_one() - - refute called(Instances.set_unreachable(inbox)) - end - - test_with_mock "does NOT call `Instances.set_unreachable` if target instance has non-nil `unreachable_since`", - Instances, - [:passthrough], - [] do - _actor = insert(:user) - inbox = "http://connrefused.site/users/nick1/inbox" - activity = insert(:note_activity) - - assert capture_log(fn -> - assert {:error, _} = - Publisher.prepare_one(%{ - inbox: inbox, - activity_id: activity.id, - unreachable_since: NaiveDateTime.utc_now() |> NaiveDateTime.to_string() - }) - |> Publisher.publish_one() - end) =~ "connrefused" - - refute called(Instances.set_unreachable(inbox)) - end end describe "publish/2" do diff --git a/test/pleroma/web/federator_test.exs b/test/pleroma/web/federator_test.exs index 4a398f239..16fe1066a 100644 --- a/test/pleroma/web/federator_test.exs +++ b/test/pleroma/web/federator_test.exs @@ -126,22 +126,17 @@ defmodule Pleroma.Web.FederatorTest do inbox: inbox2 }) - dt = NaiveDateTime.utc_now() - Instances.set_unreachable(inbox1, dt) - - Instances.set_consistently_unreachable(URI.parse(inbox2).host) + Instances.set_unreachable(URI.parse(inbox2).host) {:ok, _activity} = CommonAPI.post(user, %{status: "HI @nick1@domain.com, @nick2@domain2.com!"}) - expected_dt = NaiveDateTime.to_iso8601(dt) - ObanHelpers.perform(all_enqueued(worker: PublisherWorker)) assert ObanHelpers.member?( %{ "op" => "publish_one", - "params" => %{"inbox" => inbox1, "unreachable_since" => expected_dt} + "params" => %{"inbox" => inbox1} }, all_enqueued(worker: PublisherWorker) ) diff --git a/test/pleroma/web/pleroma_api/controllers/instances_controller_test.exs b/test/pleroma/web/pleroma_api/controllers/instances_controller_test.exs index 0d4951a73..702c05504 100644 --- a/test/pleroma/web/pleroma_api/controllers/instances_controller_test.exs +++ b/test/pleroma/web/pleroma_api/controllers/instances_controller_test.exs @@ -7,16 +7,11 @@ defmodule Pleroma.Web.PleromaApi.InstancesControllerTest do alias Pleroma.Instances - setup_all do: clear_config([:instance, :federation_reachability_timeout_days], 1) - setup do constant = "http://consistently-unreachable.name/" - eventual = "http://eventually-unreachable.com/path" {:ok, %Pleroma.Instances.Instance{unreachable_since: constant_unreachable}} = - Instances.set_consistently_unreachable(constant) - - _eventual_unreachable = Instances.set_unreachable(eventual) + Instances.set_unreachable(constant) %{constant_unreachable: constant_unreachable, constant: constant} end diff --git a/test/pleroma/workers/cron/schedule_reachability_worker_test.exs b/test/pleroma/workers/cron/schedule_reachability_worker_test.exs new file mode 100644 index 000000000..310c2e61a --- /dev/null +++ b/test/pleroma/workers/cron/schedule_reachability_worker_test.exs @@ -0,0 +1,52 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2022 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.Cron.ScheduleReachabilityWorkerTest do + use Pleroma.DataCase, async: true + use Oban.Testing, repo: Pleroma.Repo + + alias Pleroma.Instances + alias Pleroma.Workers.Cron.ScheduleReachabilityWorker + + describe "perform/1" do + test "schedules reachability checks for unreachable servers" do + # Mark some servers as unreachable + Instances.set_unreachable("https://example.com") + Instances.set_unreachable("https://test.com") + Instances.set_unreachable("https://another.com") + + # Verify they are marked as unreachable + refute Instances.reachable?("https://example.com") + refute Instances.reachable?("https://test.com") + refute Instances.reachable?("https://another.com") + + # Run the worker + assert :ok = ScheduleReachabilityWorker.perform(%Oban.Job{}) + + # Verify ReachabilityWorker jobs were scheduled for each server + # Note: domains in get_unreachable/0 are without the https:// prefix + assert_enqueued( + worker: Pleroma.Workers.ReachabilityWorker, + args: %{"domain" => "example.com"} + ) + + assert_enqueued( + worker: Pleroma.Workers.ReachabilityWorker, + args: %{"domain" => "test.com"} + ) + + assert_enqueued( + worker: Pleroma.Workers.ReachabilityWorker, + args: %{"domain" => "another.com"} + ) + end + + test "handles empty list of unreachable servers" do + # Ensure no servers are marked as unreachable + assert [] = Instances.get_unreachable() + assert :ok = ScheduleReachabilityWorker.perform(%Oban.Job{}) + refute_enqueued(worker: Pleroma.Workers.ReachabilityWorker) + end + end +end diff --git a/test/pleroma/workers/publisher_worker_test.exs b/test/pleroma/workers/publisher_worker_test.exs index 13372bf49..ca432d9bf 100644 --- a/test/pleroma/workers/publisher_worker_test.exs +++ b/test/pleroma/workers/publisher_worker_test.exs @@ -7,7 +7,9 @@ defmodule Pleroma.Workers.PublisherWorkerTest do use Oban.Testing, repo: Pleroma.Repo import Pleroma.Factory + import Mock + alias Pleroma.Instances alias Pleroma.Object alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Builder @@ -37,4 +39,85 @@ defmodule Pleroma.Workers.PublisherWorkerTest do assert {:ok, %Oban.Job{priority: 0}} = Federator.publish(post) end end + + describe "Server reachability:" do + setup do + user = insert(:user) + remote_user = insert(:user, local: false, inbox: "https://example.com/inbox") + {:ok, _, _} = Pleroma.User.follow(remote_user, user) + {:ok, activity} = CommonAPI.post(user, %{status: "Test post"}) + + %{ + user: user, + remote_user: remote_user, + activity: activity + } + end + + test "marks server as unreachable only on final failure", %{activity: activity} do + with_mock Pleroma.Web.Federator, + perform: fn :publish_one, _params -> {:error, :connection_error} end do + # First attempt + job = %Oban.Job{ + args: %{ + "op" => "publish_one", + "params" => %{ + "inbox" => "https://example.com/inbox", + "activity_id" => activity.id + } + }, + attempt: 1, + max_attempts: 5 + } + + assert {:error, :connection_error} = Pleroma.Workers.PublisherWorker.perform(job) + assert Instances.reachable?("https://example.com/inbox") + + # Final attempt + job = %{job | attempt: 5} + assert {:error, :connection_error} = Pleroma.Workers.PublisherWorker.perform(job) + refute Instances.reachable?("https://example.com/inbox") + end + end + + test "does not mark server as unreachable on successful publish", %{activity: activity} do + with_mock Pleroma.Web.Federator, + perform: fn :publish_one, _params -> {:ok, %{status: 200}} end do + job = %Oban.Job{ + args: %{ + "op" => "publish_one", + "params" => %{ + "inbox" => "https://example.com/inbox", + "activity_id" => activity.id + } + }, + attempt: 1, + max_attempts: 5 + } + + assert :ok = Pleroma.Workers.PublisherWorker.perform(job) + assert Instances.reachable?("https://example.com/inbox") + end + end + + test "cancels job if server is unreachable", %{activity: activity} do + # First mark the server as unreachable + Instances.set_unreachable("https://example.com/inbox") + refute Instances.reachable?("https://example.com/inbox") + + job = %Oban.Job{ + args: %{ + "op" => "publish_one", + "params" => %{ + "inbox" => "https://example.com/inbox", + "activity_id" => activity.id + } + }, + attempt: 1, + max_attempts: 5 + } + + assert {:cancel, :unreachable} = Pleroma.Workers.PublisherWorker.perform(job) + end + end end diff --git a/test/pleroma/workers/receiver_worker_test.exs b/test/pleroma/workers/receiver_worker_test.exs index 4d53c44ed..7f4789f91 100644 --- a/test/pleroma/workers/receiver_worker_test.exs +++ b/test/pleroma/workers/receiver_worker_test.exs @@ -3,13 +3,14 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Workers.ReceiverWorkerTest do - use Pleroma.DataCase + use Pleroma.DataCase, async: true use Oban.Testing, repo: Pleroma.Repo import Mock import Pleroma.Factory alias Pleroma.User + alias Pleroma.Web.CommonAPI alias Pleroma.Web.Federator alias Pleroma.Workers.ReceiverWorker @@ -243,4 +244,62 @@ defmodule Pleroma.Workers.ReceiverWorkerTest do assert {:cancel, _} = ReceiverWorker.perform(oban_job) end + + describe "Server reachability:" do + setup do + user = insert(:user) + remote_user = insert(:user, local: false, ap_id: "https://example.com/users/remote") + {:ok, _, _} = Pleroma.User.follow(user, remote_user) + {:ok, activity} = CommonAPI.post(remote_user, %{status: "Test post"}) + + %{ + user: user, + remote_user: remote_user, + activity: activity + } + end + + test "schedules ReachabilityWorker if host is unreachable", %{activity: activity} do + with_mocks [ + {Pleroma.Web.ActivityPub.Transmogrifier, [], + [handle_incoming: fn _ -> {:ok, activity} end]}, + {Pleroma.Instances, [], [reachable?: fn _ -> false end]}, + {Pleroma.Web.Federator, [], [perform: fn :incoming_ap_doc, _params -> {:ok, nil} end]} + ] do + job = %Oban.Job{ + args: %{ + "op" => "incoming_ap_doc", + "params" => activity.data + } + } + + Pleroma.Workers.ReceiverWorker.perform(job) + + assert_enqueued( + worker: Pleroma.Workers.ReachabilityWorker, + args: %{"domain" => "example.com"} + ) + end + end + + test "does not schedule ReachabilityWorker if host is reachable", %{activity: activity} do + with_mocks [ + {Pleroma.Web.ActivityPub.Transmogrifier, [], + [handle_incoming: fn _ -> {:ok, activity} end]}, + {Pleroma.Instances, [], [reachable?: fn _ -> true end]}, + {Pleroma.Web.Federator, [], [perform: fn :incoming_ap_doc, _params -> {:ok, nil} end]} + ] do + job = %Oban.Job{ + args: %{ + "op" => "incoming_ap_doc", + "params" => activity.data + } + } + + Pleroma.Workers.ReceiverWorker.perform(job) + + refute_enqueued(worker: Pleroma.Workers.ReachabilityWorker) + end + end + end end diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index 9caddb600..6eb6932cb 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -3,7 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Workers.RemoteFetcherWorkerTest do - use Pleroma.DataCase + use Pleroma.DataCase, async: true use Oban.Testing, repo: Pleroma.Repo alias Pleroma.Workers.RemoteFetcherWorker