[#534] Various tweaks. Tests for Instances and Instance.

config/config.exs

@@ -125,7 +125,7 @@
   banner_upload_limit: 4_000_000,
   registrations_open: true,
   federating: true,
-  federation_reachability_timeout_days: 90,
+  federation_reachability_timeout_days: 7,
   allow_relay: true,
   rewrite_policy: Pleroma.Web.ActivityPub.MRF.NoOpPolicy,
   public: true,

lib/pleroma/instances.ex

@@ -3,14 +3,17 @@ defmodule Pleroma.Instances do
 
   @adapter Pleroma.Instances.Instance
 
-  defdelegate filter_reachable(urls), to: @adapter
-  defdelegate reachable?(url), to: @adapter
-  defdelegate set_reachable(url), to: @adapter
-  defdelegate set_unreachable(url, unreachable_since \\ nil), to: @adapter
+  defdelegate filter_reachable(urls_or_hosts), to: @adapter
+  defdelegate reachable?(url_or_host), to: @adapter
+  defdelegate set_reachable(url_or_host), to: @adapter
+  defdelegate set_unreachable(url_or_host, unreachable_since \\ nil), to: @adapter
+
+  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] || 90
+      Pleroma.Config.get(:instance)[:federation_reachability_timeout_days] || 0
 
     if federation_reachability_timeout_days > 0 do
       NaiveDateTime.add(

lib/pleroma/instances/instance.ex

@@ -17,7 +17,7 @@ defmodule Pleroma.Instances.Instance do
     timestamps()
   end
 
-  defdelegate host(url), to: Instances
+  defdelegate host(url_or_host), to: Instances
 
   def changeset(struct, params \\ %{}) do
     struct
@@ -28,9 +28,9 @@ def changeset(struct, params \\ %{}) do
 
   def filter_reachable([]), do: []
 
-  def filter_reachable(urls) when is_list(urls) do
+  def filter_reachable(urls_or_hosts) when is_list(urls_or_hosts) do
     hosts =
-      urls
+      urls_or_hosts
       |> Enum.map(&(&1 && host(&1)))
       |> Enum.filter(&(to_string(&1) != ""))
 
@@ -44,14 +44,14 @@ def filter_reachable(urls) when is_list(urls) do
         )
       )
 
-    Enum.filter(urls, &(&1 && host(&1) not in unreachable_hosts))
+    Enum.filter(urls_or_hosts, &(&1 && host(&1) not in unreachable_hosts))
   end
 
-  def reachable?(url) when is_binary(url) do
+  def reachable?(url_or_host) when is_binary(url_or_host) do
     !Repo.one(
       from(i in Instance,
         where:
-          i.host == ^host(url) and
+          i.host == ^host(url_or_host) and
             i.unreachable_since <= ^Instances.reachability_datetime_threshold(),
         select: true
       )
@@ -60,8 +60,8 @@ def reachable?(url) when is_binary(url) do
 
   def reachable?(_), do: true
 
-  def set_reachable(url) when is_binary(url) do
-    with host <- host(url),
+  def set_reachable(url_or_host) when is_binary(url_or_host) do
+    with host <- host(url_or_host),
          %Instance{} = existing_record <- Repo.get_by(Instance, %{host: host}) do
       {:ok, _instance} =
         existing_record
@@ -70,13 +70,13 @@ def set_reachable(url) when is_binary(url) do
     end
   end
 
-  def set_reachable(_), do: {0, :noop}
+  def set_reachable(_), do: {:error, nil}
 
-  def set_unreachable(url, unreachable_since \\ nil)
+  def set_unreachable(url_or_host, unreachable_since \\ nil)
 
-  def set_unreachable(url, unreachable_since) when is_binary(url) do
+  def set_unreachable(url_or_host, unreachable_since) when is_binary(url_or_host) do
     unreachable_since = unreachable_since || DateTime.utc_now()
-    host = host(url)
+    host = host(url_or_host)
     existing_record = Repo.get_by(Instance, %{host: host})
 
     changes = %{unreachable_since: unreachable_since}
@@ -89,7 +89,7 @@ def set_unreachable(url, unreachable_since) when is_binary(url) do
 
       existing_record.unreachable_since &&
           NaiveDateTime.compare(existing_record.unreachable_since, unreachable_since) != :gt ->
-        {:noop, existing_record}
+        {:ok, existing_record}
 
       true ->
         existing_record
@@ -98,5 +98,5 @@ def set_unreachable(url, unreachable_since) when is_binary(url) do
     end
   end
 
-  def set_unreachable(_, _), do: {0, :noop}
+  def set_unreachable(_, _), do: {:error, nil}
 end

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -750,9 +750,9 @@ def publish_one(%{inbox: inbox, json: json, actor: actor, id: id}) do
       Instances.set_reachable(inbox)
       result
     else
-      e ->
+      {_post_result, response} ->
         Instances.set_unreachable(inbox)
-        e
+        {:error, response}
     end
   end
 

lib/pleroma/web/salmon/salmon.ex

@@ -181,10 +181,6 @@ def send_to_user(url, feed, poster) when is_binary(url) do
       Logger.debug(fn -> "Pushed to #{url}, code #{code}" end)
       :ok
     else
-      {:reachable, false} ->
-        Logger.debug(fn -> "Pushing Salmon to #{url} skipped as marked unreachable)" end)
-        :noop
-
       e ->
         Instances.set_unreachable(url)
         Logger.debug(fn -> "Pushing Salmon to #{url} failed, #{inspect(e)}" end)

lib/pleroma/web/websub/websub.ex

@@ -286,14 +286,10 @@ def publish_one(%{xml: xml, topic: topic, callback: callback, secret: secret}) d
       Logger.info(fn -> "Pushed to #{callback}, code #{code}" end)
       {:ok, code}
     else
-      {:reachable, false} ->
-        Logger.debug(fn -> "Pushing to #{callback} skipped as marked unreachable)" end)
-        {:error, :noop}
-
-      e ->
+      {_post_result, response} ->
         Instances.set_unreachable(callback)
-        Logger.debug(fn -> "Couldn't push to #{callback}, #{inspect(e)}" end)
-        {:error, e}
+        Logger.debug(fn -> "Couldn't push to #{callback}, #{inspect(response)}" end)
+        {:error, response}
     end
   end
 end

test/support/factory.ex

@@ -220,4 +220,11 @@ def oauth_app_factory do
       client_secret: "aaa;/&bbb"
     }
   end
+
+  def instance_factory do
+    %Pleroma.Instances.Instance{
+      host: "domain.com",
+      unreachable_since: nil
+    }
+  end
 end

test/support/http_request_mock.ex

@@ -653,6 +653,14 @@ def get("https://social.heldscal.la/user/23211", _, _, Accept: "application/acti
     {:ok, Tesla.Mock.json(%{"id" => "https://social.heldscal.la/user/23211"}, status: 200)}
   end
 
+  def get("http://404.site" <> _, _, _, _) do
+    {:ok,
+     %Tesla.Env{
+       status: 404,
+       body: ""
+     }}
+  end
+
   def get(url, query, body, headers) do
     {:error,
      "Not implemented the mock response for get #{inspect(url)}, #{query}, #{inspect(body)}, #{
@@ -673,6 +681,26 @@ def post("http://example.org/needs_refresh", _, _, _) do
      }}
   end
 
+  def post("http://200.site" <> _, _, _, _) do
+    {:ok,
+     %Tesla.Env{
+       status: 200,
+       body: ""
+     }}
+  end
+
+  def post("http://connrefused.site" <> _, _, _, _) do
+    {:error, :connrefused}
+  end
+
+  def post("http://404.site" <> _, _, _, _) do
+    {:ok,
+     %Tesla.Env{
+       status: 404,
+       body: ""
+     }}
+  end
+
   def post(url, _query, _body, _headers) do
     {:error, "Not implemented the mock response for post #{inspect(url)}"}
   end

test/web/activity_pub/activity_pub_controller_test.exs

@@ -146,7 +146,7 @@ test "it inserts an incoming activity into the database", %{conn: conn} do
 
     test "it clears `unreachable` federation status of the sender", %{conn: conn} do
       sender_url = "https://pleroma.soykaf.com"
-      Instances.set_unreachable(sender_url, Instances.reachability_datetime_threshold())
+      Instances.set_consistently_unreachable(sender_url)
       refute Instances.reachable?(sender_url)
 
       data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!()
@@ -211,7 +211,7 @@ test "it returns a note activity in a collection", %{conn: conn} do
 
     test "it clears `unreachable` federation status of the sender", %{conn: conn} do
       sender_host = "pleroma.soykaf.com"
-      Instances.set_unreachable(sender_host, Instances.reachability_datetime_threshold())
+      Instances.set_consistently_unreachable(sender_host)
       refute Instances.reachable?(sender_host)
 
       user = insert(:user)

test/web/activity_pub/activity_pub_test.exs

@@ -7,11 +7,12 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
   alias Pleroma.Web.ActivityPub.ActivityPub
   alias Pleroma.Web.ActivityPub.Utils
   alias Pleroma.Web.CommonAPI
-  alias Pleroma.{Activity, Object, User}
+  alias Pleroma.{Activity, Object, User, Instances}
   alias Pleroma.Builders.ActivityBuilder
 
   import Pleroma.Factory
   import Tesla.Mock
+  import Mock
 
   setup do
     mock(fn env -> apply(HttpRequestMock, :request, [env]) end)
@@ -659,6 +660,46 @@ test "returned pinned statuses" do
     assert 3 = length(activities)
   end
 
+  describe "publish_one/1" do
+    test_with_mock "it 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"
+
+      assert {:error, _} =
+               ActivityPub.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1})
+
+      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"
+
+      assert {:error, _} =
+               ActivityPub.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1})
+
+      assert called(Instances.set_unreachable(inbox))
+    end
+
+    test_with_mock "it does NOT call `Instances.set_unreachable` if target is reachable",
+                   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})
+
+      refute called(Instances.set_unreachable(inbox))
+    end
+  end
+
   def data_uri do
     File.read!("test/fixtures/avatar_data_uri")
   end

test/web/federator_test.exs

@@ -128,7 +128,7 @@ test "with relays deactivated, it does not publish to the relay", %{
           callback: "https://pleroma2.soykaf.com/cb"
         })
 
-      Instances.set_unreachable(sub1.callback, Instances.reachability_datetime_threshold())
+      Instances.set_consistently_unreachable(sub1.callback)
 
       {:ok, _activity} = CommonAPI.post(user, %{"status" => "HI"})
 
@@ -158,7 +158,7 @@ test "with relays deactivated, it does not publish to the relay", %{
           info: %{salmon: "https://domain2.com/salmon"}
         })
 
-      Instances.set_unreachable("domain.com", Instances.reachability_datetime_threshold())
+      Instances.set_consistently_unreachable("domain.com")
 
       {:ok, _activity} =
         CommonAPI.post(user, %{"status" => "HI @nick1@domain.com, @nick2@domain2.com!"})

test/web/instances/instance_test.exs

@@ -0,0 +1,107 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2018 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Instances.InstanceTest do
+  alias Pleroma.Repo
+  alias Pleroma.Instances.Instance
+
+  use Pleroma.DataCase
+
+  import Pleroma.Factory
+
+  setup_all do
+    config_path = [:instance, :federation_reachability_timeout_days]
+    initial_setting = Pleroma.Config.get(config_path)
+
+    Pleroma.Config.put(config_path, 1)
+    on_exit(fn -> Pleroma.Config.put(config_path, initial_setting) end)
+
+    :ok
+  end
+
+  describe "set_reachable/1" do
+    test "clears `unreachable_since` of existing matching Instance record having non-nil `unreachable_since`" do
+      instance = insert(:instance, unreachable_since: NaiveDateTime.utc_now())
+
+      assert {:ok, instance} = Instance.set_reachable(instance.host)
+      refute instance.unreachable_since
+    end
+
+    test "keeps nil `unreachable_since` of existing matching Instance record having nil `unreachable_since`" do
+      instance = insert(:instance, unreachable_since: nil)
+
+      assert {:ok, instance} = Instance.set_reachable(instance.host)
+      refute instance.unreachable_since
+    end
+
+    test "does NOT create an Instance record in case of no existing matching record" do
+      host = "domain.org"
+      assert nil == Instance.set_reachable(host)
+
+      assert [] = Repo.all(Ecto.Query.from(i in Instance))
+      assert Instance.reachable?(host)
+    end
+  end
+
+  describe "set_unreachable/1" do
+    test "creates new record having `unreachable_since` to current time if record does not exist" do
+      assert {:ok, instance} = Instance.set_unreachable("https://domain.com/path")
+
+      instance = Repo.get(Instance, instance.id)
+      assert instance.unreachable_since
+      assert "domain.com" == instance.host
+    end
+
+    test "sets `unreachable_since` of existing record having nil `unreachable_since`" do
+      instance = insert(:instance, unreachable_since: nil)
+      refute instance.unreachable_since
+
+      assert {:ok, _} = Instance.set_unreachable(instance.host)
+
+      instance = Repo.get(Instance, instance.id)
+      assert instance.unreachable_since
+    end
+
+    test "does NOT modify `unreachable_since` value of existing record in case it's present" do
+      instance =
+        insert(:instance, unreachable_since: NaiveDateTime.add(NaiveDateTime.utc_now(), -10))
+
+      assert instance.unreachable_since
+      initial_value = instance.unreachable_since
+
+      assert {:ok, _} = Instance.set_unreachable(instance.host)
+
+      instance = Repo.get(Instance, instance.id)
+      assert initial_value == instance.unreachable_since
+    end
+  end
+
+  describe "set_unreachable/2" do
+    test "sets `unreachable_since` value of existing record in case it's newer than supplied value" do
+      instance =
+        insert(:instance, unreachable_since: NaiveDateTime.add(NaiveDateTime.utc_now(), -10))
+
+      assert instance.unreachable_since
+
+      past_value = NaiveDateTime.add(NaiveDateTime.utc_now(), -100)
+      assert {:ok, _} = Instance.set_unreachable(instance.host, past_value)
+
+      instance = Repo.get(Instance, instance.id)
+      assert past_value == instance.unreachable_since
+    end
+
+    test "does NOT modify `unreachable_since` value of existing record in case it's equal to or older than supplied value" do
+      instance =
+        insert(:instance, unreachable_since: NaiveDateTime.add(NaiveDateTime.utc_now(), -10))
+
+      assert instance.unreachable_since
+      initial_value = instance.unreachable_since
+
+      assert {:ok, _} = Instance.set_unreachable(instance.host, NaiveDateTime.utc_now())
+
+      instance = Repo.get(Instance, instance.id)
+      assert initial_value == instance.unreachable_since
+    end
+  end
+end

test/web/instances/instances_test.exs

@@ -0,0 +1,94 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2018 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.InstancesTest do
+  alias Pleroma.Instances
+
+  use Pleroma.DataCase
+
+  setup_all do
+    config_path = [:instance, :federation_reachability_timeout_days]
+    initial_setting = Pleroma.Config.get(config_path)
+
+    Pleroma.Config.put(config_path, 1)
+    on_exit(fn -> Pleroma.Config.put(config_path, initial_setting) end)
+
+    :ok
+  end
+
+  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
+  end
+
+  describe "filter_reachable/1" do
+    test "keeps only reachable elements of supplied list" do
+      host = "consistently-unreachable.name"
+      url1 = "http://eventually-unreachable.com/path"
+      url2 = "http://domain.com/path"
+
+      Instances.set_consistently_unreachable(host)
+      Instances.set_unreachable(url1)
+
+      assert [url1, url2] == Instances.filter_reachable([host, url1, url2])
+    end
+  end
+
+  describe "set_reachable/1" do
+    test "sets unreachable url or host reachable" do
+      host = "domain.com"
+      Instances.set_consistently_unreachable(host)
+      refute Instances.reachable?(host)
+
+      Instances.set_reachable(host)
+      assert Instances.reachable?(host)
+    end
+
+    test "keeps reachable url or host reachable" do
+      url = "https://site.name?q="
+      assert Instances.reachable?(url)
+
+      Instances.set_reachable(url)
+      assert Instances.reachable?(url)
+    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

test/web/ostatus/ostatus_controller_test.exs

@@ -62,7 +62,7 @@ test "decodes a salmon with a changed magic key", %{conn: conn} do
 
     test "it clears `unreachable` federation status of the sender", %{conn: conn} do
       sender_url = "https://pleroma.soykaf.com"
-      Instances.set_unreachable(sender_url, Instances.reachability_datetime_threshold())
+      Instances.set_consistently_unreachable(sender_url)
       refute Instances.reachable?(sender_url)
 
       user = insert(:user)

test/web/websub/websub_controller_test.exs

@@ -85,7 +85,7 @@ test "rejects incoming feed updates with the wrong signature", %{conn: conn} do
 
     test "it clears `unreachable` federation status of the sender", %{conn: conn} do
       sender_url = "https://pleroma.soykaf.com"
-      Instances.set_unreachable(sender_url, Instances.reachability_datetime_threshold())
+      Instances.set_consistently_unreachable(sender_url)
       refute Instances.reachable?(sender_url)
 
       websub = insert(:websub_client_subscription)