[#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

@@ -792,7 +792,9 @@ def publish_one(%{inbox: inbox, json: json, actor: actor, id: id} = params) 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} ->

lib/pleroma/web/salmon/salmon.ex

@@ -173,7 +173,9 @@ def send_to_user(%{recipient: url, feed: feed, poster: poster} = params) when is
              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

lib/pleroma/web/websub/websub.ex

@@ -283,7 +283,9 @@ def publish_one(%{xml: xml, topic: topic, callback: callback, secret: secret} =
                {"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

test/web/activity_pub/activity_pub_test.exs

@@ -698,7 +698,57 @@ test "returned pinned statuses" 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 @@ test "returned pinned statuses" 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 @@ test "returned pinned statuses" 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

test/web/federator_test.exs

@@ -95,15 +95,18 @@ test "with relays deactivated, it does not publish to the relay", %{
         info: %{ap_enabled: true, source_data: %{"inbox" => inbox2}}
       })
 
-      Instances.set_unreachable(
+      dt = NaiveDateTime.utc_now()
-        URI.parse(inbox2).host,
+      Instances.set_unreachable(inbox1, dt)
-        Instances.reachability_datetime_threshold()
+
-      )
+      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 @@ test "with relays deactivated, it does not publish to the relay", %{
           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 @@ test "with relays deactivated, it does not publish to the relay", %{
           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