From 3913b0196e47c829df90aa835ade2efdb7c43850 Mon Sep 17 00:00:00 2001
From: Ivan Tashkinov <ivantashkinov@gmail.com>
Date: Sun, 3 Feb 2019 13:28:13 +0300
Subject: [PATCH] [#582] Made single-pub task call Instance.set_reachable/1 if
`set_reachable` is not specified. Added tests.
---
lib/pleroma/web/activity_pub/activity_pub.ex | 4 +-
lib/pleroma/web/salmon/salmon.ex | 4 +-
lib/pleroma/web/websub/websub.ex | 4 +-
test/web/activity_pub/activity_pub_test.exs | 73 +++++++++++++++++++++++++++-
test/web/federator_test.exs | 35 ++++++++++---
5 files changed, 108 insertions(+), 12 deletions(-)
diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
index 5f6c8e7d3..4635e7fcd 100644
--- a/lib/pleroma/web/activity_pub/activity_pub.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -792,7 +792,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
{"digest", digest}
]
) do
- if params[:unreachable_since], do: Instances.set_reachable(inbox)
+ if !Map.has_key?(params, :unreachable_since) || params[:unreachable_since],
+ do: Instances.set_reachable(inbox)
+
result
else
{_post_result, response} ->
diff --git a/lib/pleroma/web/salmon/salmon.ex b/lib/pleroma/web/salmon/salmon.ex
index 4d519ece4..b1c2dc7fa 100644
--- a/lib/pleroma/web/salmon/salmon.ex
+++ b/lib/pleroma/web/salmon/salmon.ex
@@ -173,7 +173,9 @@ defmodule Pleroma.Web.Salmon do
feed,
[{"Content-Type", "application/magic-envelope+xml"}]
) do
- if params[:unreachable_since], do: Instances.set_reachable(url)
+ if !Map.has_key?(params, :unreachable_since) || params[:unreachable_since],
+ do: Instances.set_reachable(url)
+
Logger.debug(fn -> "Pushed to #{url}, code #{code}" end)
:ok
else
diff --git a/lib/pleroma/web/websub/websub.ex b/lib/pleroma/web/websub/websub.ex
index cf51dce76..90ba79962 100644
--- a/lib/pleroma/web/websub/websub.ex
+++ b/lib/pleroma/web/websub/websub.ex
@@ -283,7 +283,9 @@ defmodule Pleroma.Web.Websub do
{"X-Hub-Signature", "sha1=#{signature}"}
]
) do
- if params[:unreachable_since], do: Instances.set_reachable(callback)
+ if !Map.has_key?(params, :unreachable_since) || params[:unreachable_since],
+ do: Instances.set_reachable(callback)
+
Logger.info(fn -> "Pushed to #{callback}, code #{code}" end)
{:ok, code}
else
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