From: lain Date: Wed, 16 Dec 2020 16:51:48 +0000 (+0100) Subject: Pipeline test: Switch from Mock to Mox. X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=5db1e6c8d37ea114433afe0a9247314ab92cc52f;p=akkoma Pipeline test: Switch from Mock to Mox. Speeds up the test and makes it possible to run async. --- diff --git a/config/test.exs b/config/test.exs index c4fd5c52f..a85881592 100644 --- a/config/test.exs +++ b/config/test.exs @@ -124,6 +124,14 @@ config :tzdata, :autoupdate, :disabled config :pleroma, :mrf, policies: [] +config :pleroma, :pipeline, + object_validator: Pleroma.Web.ActivityPub.ObjectValidatorMock, + mrf: Pleroma.Web.ActivityPub.MRFMock, + activity_pub: Pleroma.Web.ActivityPub.ActivityPubMock, + side_effects: Pleroma.Web.ActivityPub.SideEffectsMock, + federator: Pleroma.Web.FederatorMock, + config: Pleroma.ConfigMock + config :pleroma, :cachex, provider: Pleroma.CachexMock if File.exists?("./config/test.secret.exs") do diff --git a/lib/pleroma/config.ex b/lib/pleroma/config.ex index 97f877595..1ee4777f6 100644 --- a/lib/pleroma/config.ex +++ b/lib/pleroma/config.ex @@ -2,15 +2,24 @@ # Copyright © 2017-2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only +defmodule Pleroma.Config.Getting do + @callback get(any()) :: any() + @callback get(any(), any()) :: any() +end + defmodule Pleroma.Config do + @behaviour Pleroma.Config.Getting defmodule Error do defexception [:message] end + @impl true def get(key), do: get(key, nil) + @impl true def get([key], default), do: get(key, default) + @impl true def get([_ | _] = path, default) do case fetch(path) do {:ok, value} -> value @@ -18,6 +27,7 @@ defmodule Pleroma.Config do end end + @impl true def get(key, default) do Application.get_env(:pleroma, key, default) end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 1c91bc074..0f839af10 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -2,6 +2,10 @@ # Copyright © 2017-2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only +defmodule Pleroma.Web.ActivityPub.ActivityPub.Persisting do + @callback persist(map(), keyword()) :: {:ok, Activity.t() | Object.t()} +end + defmodule Pleroma.Web.ActivityPub.ActivityPub do alias Pleroma.Activity alias Pleroma.Activity.Ir.Topics @@ -32,6 +36,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do require Logger require Pleroma.Constants + @behaviour Pleroma.Web.ActivityPub.ActivityPub.Persisting + defp get_recipients(%{"type" => "Create"} = data) do to = Map.get(data, "to", []) cc = Map.get(data, "cc", []) @@ -85,13 +91,14 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do defp increase_replies_count_if_reply(_create_data), do: :noop @object_types ~w[ChatMessage Question Answer Audio Video Event Article] - @spec persist(map(), keyword()) :: {:ok, Activity.t() | Object.t()} + @impl true def persist(%{"type" => type} = object, meta) when type in @object_types do with {:ok, object} <- Object.create(object) do {:ok, object, meta} end end + @impl true def persist(object, meta) do with local <- Keyword.fetch!(meta, :local), {recipients, _, _} <- get_recipients(object), diff --git a/lib/pleroma/web/activity_pub/mrf.ex b/lib/pleroma/web/activity_pub/mrf.ex index 6e73b2f22..3de21b219 100644 --- a/lib/pleroma/web/activity_pub/mrf.ex +++ b/lib/pleroma/web/activity_pub/mrf.ex @@ -2,9 +2,15 @@ # Copyright © 2017-2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only +defmodule Pleroma.Web.ActivityPub.MRF.PipelineFiltering do + @callback pipeline_filter(map(), keyword()) :: {:ok, map(), keyword()} | {:error, any()} +end + defmodule Pleroma.Web.ActivityPub.MRF do require Logger + @behaviour Pleroma.Web.ActivityPub.MRF.PipelineFiltering + @mrf_config_descriptions [ %{ group: :pleroma, @@ -70,6 +76,7 @@ defmodule Pleroma.Web.ActivityPub.MRF do def filter(%{} = object), do: get_policies() |> filter(object) + @impl true def pipeline_filter(%{} = message, meta) do object = meta[:object_data] ap_id = message["object"] diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex index bd0a2a8dc..a731c5a1c 100644 --- a/lib/pleroma/web/activity_pub/object_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validator.ex @@ -2,6 +2,10 @@ # Copyright © 2017-2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only +defmodule Pleroma.Web.ActivityPub.ObjectValidator.Validating do + @callback validate(map(), keyword()) :: {:ok, map(), keyword()} | {:error, any()} +end + defmodule Pleroma.Web.ActivityPub.ObjectValidator do @moduledoc """ This module is responsible for validating an object (which can be an activity) @@ -9,6 +13,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do the system. """ + @behaviour Pleroma.Web.ActivityPub.ObjectValidator.Validating + alias Pleroma.Activity alias Pleroma.EctoType.ActivityPub.ObjectValidators alias Pleroma.Object @@ -32,7 +38,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do alias Pleroma.Web.ActivityPub.ObjectValidators.UndoValidator alias Pleroma.Web.ActivityPub.ObjectValidators.UpdateValidator - @spec validate(map(), keyword()) :: {:ok, map(), keyword()} | {:error, any()} + @impl true def validate(object, meta) def validate(%{"type" => type} = object, meta) diff --git a/lib/pleroma/web/activity_pub/pipeline.ex b/lib/pleroma/web/activity_pub/pipeline.ex index 98c32a42b..2715b94d4 100644 --- a/lib/pleroma/web/activity_pub/pipeline.ex +++ b/lib/pleroma/web/activity_pub/pipeline.ex @@ -14,12 +14,19 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.Federator + @side_effects Config.get([:pipeline, :side_effects], SideEffects) + @federator Config.get([:pipeline, :federator], Federator) + @object_validator Config.get([:pipeline, :object_validator], ObjectValidator) + @mrf Config.get([:pipeline, :mrf], MRF) + @activity_pub Config.get([:pipeline, :activity_pub], ActivityPub) + @config Config.get([:pipeline, :config], Config) + @spec common_pipeline(map(), keyword()) :: {:ok, Activity.t() | Object.t(), keyword()} | {:error, any()} def common_pipeline(object, meta) do case Repo.transaction(fn -> do_common_pipeline(object, meta) end) do {:ok, {:ok, activity, meta}} -> - SideEffects.handle_after_transaction(meta) + @side_effects.handle_after_transaction(meta) {:ok, activity, meta} {:ok, value} -> @@ -35,13 +42,13 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do def do_common_pipeline(object, meta) do with {_, {:ok, validated_object, meta}} <- - {:validate_object, ObjectValidator.validate(object, meta)}, + {:validate_object, @object_validator.validate(object, meta)}, {_, {:ok, mrfd_object, meta}} <- - {:mrf_object, MRF.pipeline_filter(validated_object, meta)}, + {:mrf_object, @mrf.pipeline_filter(validated_object, meta)}, {_, {:ok, activity, meta}} <- - {:persist_object, ActivityPub.persist(mrfd_object, meta)}, + {:persist_object, @activity_pub.persist(mrfd_object, meta)}, {_, {:ok, activity, meta}} <- - {:execute_side_effects, SideEffects.handle(activity, meta)}, + {:execute_side_effects, @side_effects.handle(activity, meta)}, {_, {:ok, _}} <- {:federation, maybe_federate(activity, meta)} do {:ok, activity, meta} else @@ -54,7 +61,7 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do defp maybe_federate(%Activity{} = activity, meta) do with {:ok, local} <- Keyword.fetch(meta, :local) do - do_not_federate = meta[:do_not_federate] || !Config.get([:instance, :federating]) + do_not_federate = meta[:do_not_federate] || !@config.get([:instance, :federating]) if !do_not_federate and local and not Visibility.is_local_public?(activity) do activity = @@ -64,7 +71,7 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do activity end - Federator.publish(activity) + @federator.publish(activity) {:ok, :federated} else {:ok, :not_federated} diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index c947e2c24..cb54eb89a 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -2,6 +2,11 @@ # Copyright © 2017-2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only +defmodule Pleroma.Web.ActivityPub.SideEffects.Handling do + @callback handle(map(), keyword()) :: {:ok, map(), keyword()} | {:error, any()} + @callback handle_after_transaction(map()) :: map() +end + defmodule Pleroma.Web.ActivityPub.SideEffects do @moduledoc """ This module looks at an inserted object and executes the side effects that it @@ -29,11 +34,15 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do @cachex Pleroma.Config.get([:cachex, :provider], Cachex) + @behaviour Pleroma.Web.ActivityPub.SideEffects.Handling + + @impl true def handle(object, meta \\ []) # Task this handles # - Follows # - Sends a notification + @impl true def handle( %{ data: %{ @@ -61,6 +70,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do # - Rejects all existing follow activities for this person # - Updates the follow state # - Dismisses notification + @impl true def handle( %{ data: %{ @@ -87,6 +97,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do # - Follows if possible # - Sends a notification # - Generates accept or reject if appropriate + @impl true def handle( %{ data: %{ @@ -128,6 +139,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do # Tasks this handles: # - Unfollow and block + @impl true def handle( %{data: %{"type" => "Block", "object" => blocked_user, "actor" => blocking_user}} = object, @@ -146,6 +158,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do # # For a local user, we also get a changeset with the full information, so we # can update non-federating, non-activitypub settings as well. + @impl true def handle(%{data: %{"type" => "Update", "object" => updated_object}} = object, meta) do if changeset = Keyword.get(meta, :user_update_changeset) do changeset @@ -164,6 +177,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do # Tasks this handles: # - Add like to object # - Set up notification + @impl true def handle(%{data: %{"type" => "Like"}} = object, meta) do liked_object = Object.get_by_ap_id(object.data["object"]) Utils.add_like_to_object(object, liked_object) @@ -181,6 +195,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do # - Increase replies count # - Set up ActivityExpiration # - Set up notifications + @impl true def handle(%{data: %{"type" => "Create"}} = activity, meta) do with {:ok, object, meta} <- handle_object_creation(meta[:object_data], meta), %User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do @@ -209,6 +224,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do # - Add announce to object # - Set up notification # - Stream out the announce + @impl true def handle(%{data: %{"type" => "Announce"}} = object, meta) do announced_object = Object.get_by_ap_id(object.data["object"]) user = User.get_cached_by_ap_id(object.data["actor"]) @@ -226,6 +242,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do {:ok, object, meta} end + @impl true def handle(%{data: %{"type" => "Undo", "object" => undone_object}} = object, meta) do with undone_object <- Activity.get_by_ap_id(undone_object), :ok <- handle_undoing(undone_object) do @@ -236,6 +253,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do # Tasks this handles: # - Add reaction to object # - Set up notification + @impl true def handle(%{data: %{"type" => "EmojiReact"}} = object, meta) do reacted_object = Object.get_by_ap_id(object.data["object"]) Utils.add_emoji_reaction_to_object(object, reacted_object) @@ -252,6 +270,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do # - Reduce the user note count # - Reduce the reply count # - Stream out the activity + @impl true def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, meta) do deleted_object = Object.normalize(deleted_object, false) || @@ -297,6 +316,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do end # Nothing to do + @impl true def handle(object, meta) do {:ok, object, meta} end @@ -441,6 +461,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do |> Keyword.put(:notifications, notifications ++ existing) end + @impl true def handle_after_transaction(meta) do meta |> send_notifications() diff --git a/lib/pleroma/web/federator.ex b/lib/pleroma/web/federator.ex index 130654145..186861fd9 100644 --- a/lib/pleroma/web/federator.ex +++ b/lib/pleroma/web/federator.ex @@ -2,6 +2,10 @@ # Copyright © 2017-2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only +defmodule Pleroma.Web.Federator.Publishing do + @callback publish(map()) :: any() +end + defmodule Pleroma.Web.Federator do alias Pleroma.Activity alias Pleroma.Object.Containment @@ -15,6 +19,8 @@ defmodule Pleroma.Web.Federator do require Logger + @behaviour Pleroma.Web.Federator.Publishing + @doc """ Returns `true` if the distance to target object does not exceed max configured value. Serves to prevent fetching of very long threads, especially useful on smaller instances. @@ -39,10 +45,12 @@ defmodule Pleroma.Web.Federator do ReceiverWorker.enqueue("incoming_ap_doc", %{"params" => params}) end + @impl true def publish(%{id: "pleroma:fakeid"} = activity) do perform(:publish, activity) end + @impl true def publish(activity) do PublisherWorker.enqueue("publish", %{"activity_id" => activity.id}) end diff --git a/test/pleroma/web/activity_pub/pipeline_test.exs b/test/pleroma/web/activity_pub/pipeline_test.exs index 210a06563..d0e3fb347 100644 --- a/test/pleroma/web/activity_pub/pipeline_test.exs +++ b/test/pleroma/web/activity_pub/pipeline_test.exs @@ -3,14 +3,35 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.ActivityPub.PipelineTest do - use Pleroma.DataCase + use Pleroma.DataCase, async: true - import Mock + import Mox import Pleroma.Factory + alias Pleroma.Web.ActivityPub.ActivityPubMock + alias Pleroma.Web.ActivityPub.MRFMock + alias Pleroma.Web.ActivityPub.ObjectValidatorMock + alias Pleroma.Web.ActivityPub.SideEffectsMock + alias Pleroma.Web.FederatorMock + alias Pleroma.ConfigMock + + setup :verify_on_exit! + describe "common_pipeline/2" do setup do - clear_config([:instance, :federating], true) + ObjectValidatorMock + |> expect(:validate, fn o, m -> {:ok, o, m} end) + + MRFMock + |> expect(:pipeline_filter, fn o, m -> {:ok, o, m} end) + + ActivityPubMock + |> expect(:persist, fn o, m -> {:ok, o, m} end) + + SideEffectsMock + |> expect(:handle, fn o, m -> {:ok, o, m} end) + |> expect(:handle_after_transaction, fn m -> m end) + :ok end @@ -21,159 +42,53 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do activity_with_object = %{activity | data: Map.put(activity.data, "object", object)} - with_mocks([ - {Pleroma.Web.ActivityPub.ObjectValidator, [], [validate: fn o, m -> {:ok, o, m} end]}, - { - Pleroma.Web.ActivityPub.MRF, - [], - [pipeline_filter: fn o, m -> {:ok, o, m} end] - }, - { - Pleroma.Web.ActivityPub.ActivityPub, - [], - [persist: fn o, m -> {:ok, o, m} end] - }, - { - Pleroma.Web.ActivityPub.SideEffects, - [], - [ - handle: fn o, m -> {:ok, o, m} end, - handle_after_transaction: fn m -> m end - ] - }, - { - Pleroma.Web.Federator, - [], - [publish: fn _o -> :ok end] - } - ]) do - assert {:ok, ^activity, ^meta} = - Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta) - - assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta)) - refute called(Pleroma.Web.Federator.publish(activity)) - assert_called(Pleroma.Web.Federator.publish(activity_with_object)) - end + FederatorMock + |> expect(:publish, fn ^activity_with_object -> :ok end) + + ConfigMock + |> expect(:get, fn [:instance, :federating] -> true end) + + assert {:ok, ^activity, ^meta} = + Pleroma.Web.ActivityPub.Pipeline.common_pipeline( + activity, + meta + ) end test "it goes through validation, filtering, persisting, side effects and federation for local activities" do activity = insert(:note_activity) meta = [local: true] - with_mocks([ - {Pleroma.Web.ActivityPub.ObjectValidator, [], [validate: fn o, m -> {:ok, o, m} end]}, - { - Pleroma.Web.ActivityPub.MRF, - [], - [pipeline_filter: fn o, m -> {:ok, o, m} end] - }, - { - Pleroma.Web.ActivityPub.ActivityPub, - [], - [persist: fn o, m -> {:ok, o, m} end] - }, - { - Pleroma.Web.ActivityPub.SideEffects, - [], - [ - handle: fn o, m -> {:ok, o, m} end, - handle_after_transaction: fn m -> m end - ] - }, - { - Pleroma.Web.Federator, - [], - [publish: fn _o -> :ok end] - } - ]) do - assert {:ok, ^activity, ^meta} = - Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta) - - assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta)) - assert_called(Pleroma.Web.Federator.publish(activity)) - end + FederatorMock + |> expect(:publish, fn ^activity -> :ok end) + + ConfigMock + |> expect(:get, fn [:instance, :federating] -> true end) + + assert {:ok, ^activity, ^meta} = + Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta) end test "it goes through validation, filtering, persisting, side effects without federation for remote activities" do activity = insert(:note_activity) meta = [local: false] - with_mocks([ - {Pleroma.Web.ActivityPub.ObjectValidator, [], [validate: fn o, m -> {:ok, o, m} end]}, - { - Pleroma.Web.ActivityPub.MRF, - [], - [pipeline_filter: fn o, m -> {:ok, o, m} end] - }, - { - Pleroma.Web.ActivityPub.ActivityPub, - [], - [persist: fn o, m -> {:ok, o, m} end] - }, - { - Pleroma.Web.ActivityPub.SideEffects, - [], - [handle: fn o, m -> {:ok, o, m} end, handle_after_transaction: fn m -> m end] - }, - { - Pleroma.Web.Federator, - [], - [] - } - ]) do - assert {:ok, ^activity, ^meta} = - Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta) - - assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta)) - end + ConfigMock + |> expect(:get, fn [:instance, :federating] -> true end) + + assert {:ok, ^activity, ^meta} = + Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta) end test "it goes through validation, filtering, persisting, side effects without federation for local activities if federation is deactivated" do - clear_config([:instance, :federating], false) - activity = insert(:note_activity) meta = [local: true] - with_mocks([ - {Pleroma.Web.ActivityPub.ObjectValidator, [], [validate: fn o, m -> {:ok, o, m} end]}, - { - Pleroma.Web.ActivityPub.MRF, - [], - [pipeline_filter: fn o, m -> {:ok, o, m} end] - }, - { - Pleroma.Web.ActivityPub.ActivityPub, - [], - [persist: fn o, m -> {:ok, o, m} end] - }, - { - Pleroma.Web.ActivityPub.SideEffects, - [], - [handle: fn o, m -> {:ok, o, m} end, handle_after_transaction: fn m -> m end] - }, - { - Pleroma.Web.Federator, - [], - [] - } - ]) do - assert {:ok, ^activity, ^meta} = - Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta) - - assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta)) - assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta)) - end + ConfigMock + |> expect(:get, fn [:instance, :federating] -> false end) + + assert {:ok, ^activity, ^meta} = + Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta) end end end diff --git a/test/support/mocks.ex b/test/support/mocks.ex index d790553cd..a600a6458 100644 --- a/test/support/mocks.ex +++ b/test/support/mocks.ex @@ -3,3 +3,23 @@ # SPDX-License-Identifier: AGPL-3.0-only Mox.defmock(Pleroma.CachexMock, for: Pleroma.Caching) + +Mox.defmock(Pleroma.Web.ActivityPub.ObjectValidatorMock, + for: Pleroma.Web.ActivityPub.ObjectValidator.Validating +) + +Mox.defmock(Pleroma.Web.ActivityPub.MRFMock, + for: Pleroma.Web.ActivityPub.MRF.PipelineFiltering +) + +Mox.defmock(Pleroma.Web.ActivityPub.ActivityPubMock, + for: Pleroma.Web.ActivityPub.ActivityPub.Persisting +) + +Mox.defmock(Pleroma.Web.ActivityPub.SideEffectsMock, + for: Pleroma.Web.ActivityPub.SideEffects.Handling +) + +Mox.defmock(Pleroma.Web.FederatorMock, for: Pleroma.Web.Federator.Publishing) + +Mox.defmock(Pleroma.ConfigMock, for: Pleroma.Config.Getting)