From 1d2f41642cfec5710055bcf8409778bb362beecb Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Mon, 28 Jan 2019 15:25:06 +0300 Subject: [PATCH] [#534] Various tweaks. Tests for Instances and Instance. --- config/config.exs | 2 +- lib/pleroma/instances.ex | 13 ++- lib/pleroma/instances/instance.ex | 28 ++--- lib/pleroma/web/activity_pub/activity_pub.ex | 4 +- lib/pleroma/web/salmon/salmon.ex | 4 - lib/pleroma/web/websub/websub.ex | 10 +- test/support/factory.ex | 7 ++ test/support/http_request_mock.ex | 28 +++++ .../activity_pub_controller_test.exs | 4 +- test/web/activity_pub/activity_pub_test.exs | 43 ++++++- test/web/federator_test.exs | 4 +- test/web/instances/instance_test.exs | 107 ++++++++++++++++++ test/web/instances/instances_test.exs | 94 +++++++++++++++ test/web/ostatus/ostatus_controller_test.exs | 2 +- test/web/websub/websub_controller_test.exs | 2 +- 15 files changed, 312 insertions(+), 40 deletions(-) create mode 100644 test/web/instances/instance_test.exs create mode 100644 test/web/instances/instances_test.exs diff --git a/config/config.exs b/config/config.exs index 7a1a875c9..c0a936b17 100644 --- a/config/config.exs +++ b/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, diff --git a/lib/pleroma/instances.ex b/lib/pleroma/instances.ex index 0b08f0eb8..5e107f4c9 100644 --- a/lib/pleroma/instances.ex +++ b/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( diff --git a/lib/pleroma/instances/instance.ex b/lib/pleroma/instances/instance.ex index e3af4a8a7..a87590d8b 100644 --- a/lib/pleroma/instances/instance.ex +++ b/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 diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 4232d6c0a..6cad02da6 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/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 diff --git a/lib/pleroma/web/salmon/salmon.ex b/lib/pleroma/web/salmon/salmon.ex index 80023127c..e96455423 100644 --- a/lib/pleroma/web/salmon/salmon.ex +++ b/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) diff --git a/lib/pleroma/web/websub/websub.ex b/lib/pleroma/web/websub/websub.ex index 64eba7221..abe148270 100644 --- a/lib/pleroma/web/websub/websub.ex +++ b/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 diff --git a/test/support/factory.ex b/test/support/factory.ex index 964b2b61c..0c21093ce 100644 --- a/test/support/factory.ex +++ b/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 diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index e4279e14d..3d6efd52c 100644 --- a/test/support/http_request_mock.ex +++ b/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 diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs index 1b704330f..eca5c134d 100644 --- a/test/web/activity_pub/activity_pub_controller_test.exs +++ b/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) diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 18f094379..d517c7aa7 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/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 diff --git a/test/web/federator_test.exs b/test/web/federator_test.exs index f4234aea8..c6d10ef78 100644 --- a/test/web/federator_test.exs +++ b/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!"}) diff --git a/test/web/instances/instance_test.exs b/test/web/instances/instance_test.exs new file mode 100644 index 000000000..a158c0a42 --- /dev/null +++ b/test/web/instances/instance_test.exs @@ -0,0 +1,107 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2018 Pleroma Authors +# 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 diff --git a/test/web/instances/instances_test.exs b/test/web/instances/instances_test.exs new file mode 100644 index 000000000..a2fdf1019 --- /dev/null +++ b/test/web/instances/instances_test.exs @@ -0,0 +1,94 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2018 Pleroma Authors +# 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 diff --git a/test/web/ostatus/ostatus_controller_test.exs b/test/web/ostatus/ostatus_controller_test.exs index ca447aa5d..ad9bc418a 100644 --- a/test/web/ostatus/ostatus_controller_test.exs +++ b/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) diff --git a/test/web/websub/websub_controller_test.exs b/test/web/websub/websub_controller_test.exs index c445ed676..cb19d6fe6 100644 --- a/test/web/websub/websub_controller_test.exs +++ b/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)