From b40b4bc4e5b49ac2b35746cee7b1db92428d3ee1 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 3 Feb 2019 12:41:27 +0300 Subject: [#582] Optimized federation retirement by reducing the number of SQL calls (calling `Instances.set_reachable/1` only if instance had `unreachable_since`, calling `Instances.set_unreachable/1` only if instance had nil `unreachable_since`). --- test/web/federator_test.exs | 4 ++-- test/web/instances/instances_test.exs | 23 +++++++++++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) (limited to 'test') diff --git a/test/web/federator_test.exs b/test/web/federator_test.exs index c6d10ef78..7bb249d74 100644 --- a/test/web/federator_test.exs +++ b/test/web/federator_test.exs @@ -163,8 +163,8 @@ defmodule Pleroma.Web.FederatorTest do {:ok, _activity} = CommonAPI.post(user, %{"status" => "HI @nick1@domain.com, @nick2@domain2.com!"}) - assert called(Federator.enqueue(:publish_single_salmon, {remote_user2, :_, :_})) - refute called(Federator.enqueue(:publish_single_websub, {remote_user1, :_, :_})) + assert called(Federator.enqueue(:publish_single_salmon, %{recipient: remote_user2})) + refute called(Federator.enqueue(:publish_single_websub, %{recipient: remote_user1})) end end diff --git a/test/web/instances/instances_test.exs b/test/web/instances/instances_test.exs index adb8560a7..2530c09fe 100644 --- a/test/web/instances/instances_test.exs +++ b/test/web/instances/instances_test.exs @@ -47,7 +47,7 @@ defmodule Pleroma.InstancesTest do end describe "filter_reachable/1" do - test "keeps only reachable elements of supplied list" do + setup do host = "consistently-unreachable.name" url1 = "http://eventually-unreachable.com/path" url2 = "http://domain.com/path" @@ -55,7 +55,26 @@ defmodule Pleroma.InstancesTest do Instances.set_consistently_unreachable(host) Instances.set_unreachable(url1) - assert [url1, url2] == Instances.filter_reachable([host, url1, url2]) + result = Instances.filter_reachable([host, url1, url2, nil]) + %{result: result, url1: url1, url2: url2} + 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() + 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]) end end -- cgit v1.2.3 From 3913b0196e47c829df90aa835ade2efdb7c43850 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 3 Feb 2019 13:28:13 +0300 Subject: [#582] Made single-pub task call Instance.set_reachable/1 if `set_reachable` is not specified. Added tests. --- test/web/activity_pub/activity_pub_test.exs | 73 ++++++++++++++++++++++++++++- test/web/federator_test.exs | 35 +++++++++++--- 2 files changed, 99 insertions(+), 9 deletions(-) (limited to 'test') diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 2ada4f2e5..a55961ac4 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -698,7 +698,57 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do end describe "publish_one/1" do - test_with_mock "it calls `Instances.set_unreachable` on target inbox on non-2xx HTTP response code", + test_with_mock "calls `Instances.set_reachable` on successful federation if `unreachable_since` is not specified", + Instances, + [:passthrough], + [] do + actor = insert(:user) + inbox = "http://200.site/users/nick1/inbox" + + assert {:ok, _} = ActivityPub.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) + + assert called(Instances.set_reachable(inbox)) + 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" + + assert {:ok, _} = + ActivityPub.publish_one(%{ + inbox: inbox, + json: "{}", + actor: actor, + id: 1, + unreachable_since: NaiveDateTime.utc_now() + }) + + 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" + + assert {:ok, _} = + ActivityPub.publish_one(%{ + inbox: inbox, + json: "{}", + actor: actor, + id: 1, + unreachable_since: nil + }) + + 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 @@ -724,7 +774,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do assert called(Instances.set_unreachable(inbox)) end - test_with_mock "it does NOT call `Instances.set_unreachable` if target is reachable", + test_with_mock "does NOT call `Instances.set_unreachable` if target is reachable", Instances, [:passthrough], [] do @@ -735,6 +785,25 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do 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" + + assert {:error, _} = + ActivityPub.publish_one(%{ + inbox: inbox, + json: "{}", + actor: actor, + id: 1, + unreachable_since: NaiveDateTime.utc_now() + }) + + refute called(Instances.set_unreachable(inbox)) + end end def data_uri do diff --git a/test/web/federator_test.exs b/test/web/federator_test.exs index 7bb249d74..05f813291 100644 --- a/test/web/federator_test.exs +++ b/test/web/federator_test.exs @@ -95,15 +95,18 @@ defmodule Pleroma.Web.FederatorTest do info: %{ap_enabled: true, source_data: %{"inbox" => inbox2}} }) - Instances.set_unreachable( - URI.parse(inbox2).host, - Instances.reachability_datetime_threshold() - ) + dt = NaiveDateTime.utc_now() + Instances.set_unreachable(inbox1, dt) + + Instances.set_consistently_unreachable(URI.parse(inbox2).host) {:ok, _activity} = CommonAPI.post(user, %{"status" => "HI @nick1@domain.com, @nick2@domain2.com!"}) - assert called(Federator.enqueue(:publish_single_ap, %{inbox: inbox1})) + assert called( + Federator.enqueue(:publish_single_ap, %{inbox: inbox1, unreachable_since: dt}) + ) + refute called(Federator.enqueue(:publish_single_ap, %{inbox: inbox2})) end @@ -128,11 +131,20 @@ defmodule Pleroma.Web.FederatorTest do callback: "https://pleroma2.soykaf.com/cb" }) + dt = NaiveDateTime.utc_now() + Instances.set_unreachable(sub2.callback, dt) + Instances.set_consistently_unreachable(sub1.callback) {:ok, _activity} = CommonAPI.post(user, %{"status" => "HI"}) - assert called(Federator.enqueue(:publish_single_websub, %{callback: sub2.callback})) + assert called( + Federator.enqueue(:publish_single_websub, %{ + callback: sub2.callback, + unreachable_since: dt + }) + ) + refute called(Federator.enqueue(:publish_single_websub, %{callback: sub1.callback})) end @@ -158,12 +170,21 @@ defmodule Pleroma.Web.FederatorTest do info: %{salmon: "https://domain2.com/salmon"} }) + dt = NaiveDateTime.utc_now() + Instances.set_unreachable(remote_user2.ap_id, dt) + Instances.set_consistently_unreachable("domain.com") {:ok, _activity} = CommonAPI.post(user, %{"status" => "HI @nick1@domain.com, @nick2@domain2.com!"}) - assert called(Federator.enqueue(:publish_single_salmon, %{recipient: remote_user2})) + assert called( + Federator.enqueue(:publish_single_salmon, %{ + recipient: remote_user2, + unreachable_since: dt + }) + ) + refute called(Federator.enqueue(:publish_single_websub, %{recipient: remote_user1})) end end -- cgit v1.2.3