From 13ab8defc0c5f9728c00ce49044dd60c02c7e81f Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 4 May 2020 14:34:19 +0200 Subject: [PATCH 1/5] Pipeline: Move transctioning to common pipeline. --- lib/pleroma/web/activity_pub/pipeline.ex | 23 ++++++++++++++++---- lib/pleroma/web/activity_pub/side_effects.ex | 13 ++++------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/web/activity_pub/pipeline.ex b/lib/pleroma/web/activity_pub/pipeline.ex index 7ccee54c9..d5abb7567 100644 --- a/lib/pleroma/web/activity_pub/pipeline.ex +++ b/lib/pleroma/web/activity_pub/pipeline.ex @@ -4,20 +4,33 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do alias Pleroma.Activity + alias Pleroma.Object + alias Pleroma.Repo alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.MRF alias Pleroma.Web.ActivityPub.ObjectValidator alias Pleroma.Web.ActivityPub.SideEffects alias Pleroma.Web.Federator - @spec common_pipeline(map(), keyword()) :: {:ok, Activity.t(), keyword()} | {:error, any()} + @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, value} -> + value + + {:error, e} -> + {:error, e} + end + end + + def do_common_pipeline(object, meta) do with {_, {:ok, validated_object, meta}} <- {:validate_object, ObjectValidator.validate(object, meta)}, {_, {:ok, mrfd_object}} <- {:mrf_object, MRF.filter(validated_object)}, - {_, {:ok, %Activity{} = activity, meta}} <- + {_, {:ok, activity, meta}} <- {:persist_object, ActivityPub.persist(mrfd_object, meta)}, - {_, {:ok, %Activity{} = activity, meta}} <- + {_, {:ok, activity, meta}} <- {:execute_side_effects, SideEffects.handle(activity, meta)}, {_, {:ok, _}} <- {:federation, maybe_federate(activity, meta)} do {:ok, activity, meta} @@ -27,7 +40,9 @@ def common_pipeline(object, meta) do end end - defp maybe_federate(activity, meta) do + defp maybe_federate(%Object{}, _), do: {:ok, :not_federated} + + defp maybe_federate(%Activity{} = activity, meta) do with {:ok, local} <- Keyword.fetch(meta, :local) do if local do Federator.publish(activity) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 5981e7545..6a8f1af96 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -15,17 +15,12 @@ def handle(object, meta \\ []) # - Add like to object # - Set up notification def handle(%{data: %{"type" => "Like"}} = object, meta) do - {:ok, result} = - Pleroma.Repo.transaction(fn -> - liked_object = Object.get_by_ap_id(object.data["object"]) - Utils.add_like_to_object(object, liked_object) + liked_object = Object.get_by_ap_id(object.data["object"]) + Utils.add_like_to_object(object, liked_object) - Notification.create_notifications(object) + Notification.create_notifications(object) - {:ok, object, meta} - end) - - result + {:ok, object, meta} end # Nothing to do From 335aabc39c6384216d2eaa3036cea541141d8025 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 4 May 2020 14:45:28 +0200 Subject: [PATCH 2/5] Transmogrifier tests: Extract like tests. --- test/support/factory.ex | 1 + .../transmogrifier/like_handling_test.exs | 78 +++++++++++++++++++ test/web/activity_pub/transmogrifier_test.exs | 56 ------------- 3 files changed, 79 insertions(+), 56 deletions(-) create mode 100644 test/web/activity_pub/transmogrifier/like_handling_test.exs diff --git a/test/support/factory.ex b/test/support/factory.ex index f0b797fd4..495764782 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -32,6 +32,7 @@ def user_factory do password_hash: Comeonin.Pbkdf2.hashpwsalt("test"), bio: sequence(:bio, &"Tester Number #{&1}"), last_digest_emailed_at: NaiveDateTime.utc_now(), + last_refreshed_at: NaiveDateTime.utc_now(), notification_settings: %Pleroma.User.NotificationSetting{} } diff --git a/test/web/activity_pub/transmogrifier/like_handling_test.exs b/test/web/activity_pub/transmogrifier/like_handling_test.exs new file mode 100644 index 000000000..54a5c1dbc --- /dev/null +++ b/test/web/activity_pub/transmogrifier/like_handling_test.exs @@ -0,0 +1,78 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ActivityPub.Transmogrifier.LikeHandlingTest do + use Pleroma.DataCase + + alias Pleroma.Activity + alias Pleroma.Web.ActivityPub.Transmogrifier + alias Pleroma.Web.CommonAPI + + import Pleroma.Factory + + test "it works for incoming likes" do + user = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hello"}) + + data = + File.read!("test/fixtures/mastodon-like.json") + |> Poison.decode!() + |> Map.put("object", activity.data["object"]) + + _actor = insert(:user, ap_id: data["actor"], local: false) + + {:ok, %Activity{data: data, local: false} = activity} = Transmogrifier.handle_incoming(data) + + refute Enum.empty?(activity.recipients) + + assert data["actor"] == "http://mastodon.example.org/users/admin" + assert data["type"] == "Like" + assert data["id"] == "http://mastodon.example.org/users/admin#likes/2" + assert data["object"] == activity.data["object"] + end + + test "it works for incoming misskey likes, turning them into EmojiReacts" do + user = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hello"}) + + data = + File.read!("test/fixtures/misskey-like.json") + |> Poison.decode!() + |> Map.put("object", activity.data["object"]) + + _actor = insert(:user, ap_id: data["actor"], local: false) + + {:ok, %Activity{data: activity_data, local: false}} = Transmogrifier.handle_incoming(data) + + assert activity_data["actor"] == data["actor"] + assert activity_data["type"] == "EmojiReact" + assert activity_data["id"] == data["id"] + assert activity_data["object"] == activity.data["object"] + assert activity_data["content"] == "🍮" + end + + test "it works for incoming misskey likes that contain unicode emojis, turning them into EmojiReacts" do + user = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hello"}) + + data = + File.read!("test/fixtures/misskey-like.json") + |> Poison.decode!() + |> Map.put("object", activity.data["object"]) + |> Map.put("_misskey_reaction", "⭐") + + _actor = insert(:user, ap_id: data["actor"], local: false) + + {:ok, %Activity{data: activity_data, local: false}} = Transmogrifier.handle_incoming(data) + + assert activity_data["actor"] == data["actor"] + assert activity_data["type"] == "EmojiReact" + assert activity_data["id"] == data["id"] + assert activity_data["object"] == activity.data["object"] + assert activity_data["content"] == "⭐" + end +end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 36e1e7bd1..23efa4be6 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -325,62 +325,6 @@ test "it cleans up incoming notices which are not really DMs" do assert object_data["cc"] == to end - test "it works for incoming likes" do - user = insert(:user) - {:ok, activity} = CommonAPI.post(user, %{"status" => "hello"}) - - data = - File.read!("test/fixtures/mastodon-like.json") - |> Poison.decode!() - |> Map.put("object", activity.data["object"]) - - {:ok, %Activity{data: data, local: false} = activity} = Transmogrifier.handle_incoming(data) - - refute Enum.empty?(activity.recipients) - - assert data["actor"] == "http://mastodon.example.org/users/admin" - assert data["type"] == "Like" - assert data["id"] == "http://mastodon.example.org/users/admin#likes/2" - assert data["object"] == activity.data["object"] - end - - test "it works for incoming misskey likes, turning them into EmojiReacts" do - user = insert(:user) - {:ok, activity} = CommonAPI.post(user, %{"status" => "hello"}) - - data = - File.read!("test/fixtures/misskey-like.json") - |> Poison.decode!() - |> Map.put("object", activity.data["object"]) - - {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) - - assert data["actor"] == data["actor"] - assert data["type"] == "EmojiReact" - assert data["id"] == data["id"] - assert data["object"] == activity.data["object"] - assert data["content"] == "🍮" - end - - test "it works for incoming misskey likes that contain unicode emojis, turning them into EmojiReacts" do - user = insert(:user) - {:ok, activity} = CommonAPI.post(user, %{"status" => "hello"}) - - data = - File.read!("test/fixtures/misskey-like.json") - |> Poison.decode!() - |> Map.put("object", activity.data["object"]) - |> Map.put("_misskey_reaction", "⭐") - - {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) - - assert data["actor"] == data["actor"] - assert data["type"] == "EmojiReact" - assert data["id"] == data["id"] - assert data["object"] == activity.data["object"] - assert data["content"] == "⭐" - end - test "it works for incoming emoji reactions" do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => "hello"}) From e03c301ebeea6687ee7f19d447232864b182d581 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 4 May 2020 17:08:31 +0200 Subject: [PATCH 3/5] LikeValidator: Fix up missing recipients. --- .../object_validators/like_validator.ex | 34 +++++++++++++++++-- .../activity_pub/object_validator_test.exs | 13 +++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/activity_pub/object_validators/like_validator.ex b/lib/pleroma/web/activity_pub/object_validators/like_validator.ex index 49546ceaa..d9ee07995 100644 --- a/lib/pleroma/web/activity_pub/object_validators/like_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/like_validator.ex @@ -5,6 +5,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator do use Ecto.Schema + alias Pleroma.Object alias Pleroma.Web.ActivityPub.ObjectValidators.Types alias Pleroma.Web.ActivityPub.Utils @@ -19,8 +20,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator do field(:object, Types.ObjectID) field(:actor, Types.ObjectID) field(:context, :string) - field(:to, {:array, :string}) - field(:cc, {:array, :string}) + field(:to, {:array, :string}, default: []) + field(:cc, {:array, :string}, default: []) end def cast_and_validate(data) do @@ -31,7 +32,34 @@ def cast_and_validate(data) do def cast_data(data) do %__MODULE__{} - |> cast(data, [:id, :type, :object, :actor, :context, :to, :cc]) + |> changeset(data) + end + + def changeset(struct, data) do + struct + |> cast(data, __schema__(:fields)) + |> fix_after_cast() + end + + def fix_after_cast(cng) do + cng + |> fix_recipients() + end + + def fix_recipients(cng) do + to = get_field(cng, :to) || [] + cc = get_field(cng, :cc) || [] + object = get_field(cng, :object) + + with {[], []} <- {to, cc}, + %Object{data: %{"actor" => actor}} <- Object.get_cached_by_ap_id(object), + {:ok, actor} <- Types.ObjectID.cast(actor) do + cng + |> put_change(:to, [actor]) + else + _ -> + cng + end end def validate_data(data_cng) do diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index 3c5c3696e..9e9e41c6b 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -36,6 +36,19 @@ test "is valid for a valid object", %{valid_like: valid_like} do assert LikeValidator.cast_and_validate(valid_like).valid? end + test "sets the 'to' field to the object actor if no recipients are given", %{ + valid_like: valid_like, + user: user + } do + without_recipients = + valid_like + |> Map.delete("to") + + {:ok, object, _meta} = ObjectValidator.validate(without_recipients, []) + + assert object["to"] == [user.ap_id] + end + test "it errors when the actor is missing or not known", %{valid_like: valid_like} do without_actor = Map.delete(valid_like, "actor") From 0f9bed022fab80f59353597dde82896ef954a678 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 4 May 2020 17:18:17 +0200 Subject: [PATCH 4/5] LikeValidator: Fix up context. --- .../object_validators/like_validator.ex | 18 ++++++++++++++++-- .../web/activity_pub/object_validator_test.exs | 13 +++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/object_validators/like_validator.ex b/lib/pleroma/web/activity_pub/object_validators/like_validator.ex index d9ee07995..1bce739bd 100644 --- a/lib/pleroma/web/activity_pub/object_validators/like_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/like_validator.ex @@ -44,11 +44,25 @@ def changeset(struct, data) do def fix_after_cast(cng) do cng |> fix_recipients() + |> fix_context() + end + + def fix_context(cng) do + object = get_field(cng, :object) + + with nil <- get_field(cng, :context), + %Object{data: %{"context" => context}} <- Object.get_cached_by_ap_id(object) do + cng + |> put_change(:context, context) + else + _ -> + cng + end end def fix_recipients(cng) do - to = get_field(cng, :to) || [] - cc = get_field(cng, :cc) || [] + to = get_field(cng, :to) + cc = get_field(cng, :cc) object = get_field(cng, :object) with {[], []} <- {to, cc}, diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index 9e9e41c6b..93989e28a 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -49,6 +49,19 @@ test "sets the 'to' field to the object actor if no recipients are given", %{ assert object["to"] == [user.ap_id] end + test "sets the context field to the context of the object if no context is given", %{ + valid_like: valid_like, + post_activity: post_activity + } do + without_context = + valid_like + |> Map.delete("context") + + {:ok, object, _meta} = ObjectValidator.validate(without_context, []) + + assert object["context"] == post_activity.data["context"] + end + test "it errors when the actor is missing or not known", %{valid_like: valid_like} do without_actor = Map.delete(valid_like, "actor") From 3559dd108518dfa740878cecdc86c0357b265857 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 4 May 2020 17:18:38 +0200 Subject: [PATCH 5/5] Transmogrifier: Rely on LikeValidator. --- .../web/activity_pub/transmogrifier.ex | 55 +------------------ 1 file changed, 3 insertions(+), 52 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index c966ec960..581e7040b 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -15,7 +15,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.ObjectValidator - alias Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Visibility @@ -658,16 +657,9 @@ def handle_incoming( end def handle_incoming(%{"type" => "Like"} = data, _options) do - with {_, {:ok, cast_data_sym}} <- - {:casting_data, - data |> LikeValidator.cast_data() |> Ecto.Changeset.apply_action(:insert)}, - cast_data = ObjectValidator.stringify_keys(Map.from_struct(cast_data_sym)), - :ok <- ObjectValidator.fetch_actor_and_object(cast_data), - {_, {:ok, cast_data}} <- {:ensure_context_presence, ensure_context_presence(cast_data)}, - {_, {:ok, cast_data}} <- - {:ensure_recipients_presence, ensure_recipients_presence(cast_data)}, - {_, {:ok, activity, _meta}} <- - {:common_pipeline, Pipeline.common_pipeline(cast_data, local: false)} do + with :ok <- ObjectValidator.fetch_actor_and_object(data), + {:ok, activity, _meta} <- + Pipeline.common_pipeline(data, local: false) do {:ok, activity} else e -> {:error, e} @@ -1296,45 +1288,4 @@ def maybe_fix_user_url(%{"url" => url} = data) when is_map(url) do def maybe_fix_user_url(data), do: data def maybe_fix_user_object(data), do: maybe_fix_user_url(data) - - defp ensure_context_presence(%{"context" => context} = data) when is_binary(context), - do: {:ok, data} - - defp ensure_context_presence(%{"object" => object} = data) when is_binary(object) do - with %{data: %{"context" => context}} when is_binary(context) <- Object.normalize(object) do - {:ok, Map.put(data, "context", context)} - else - _ -> - {:error, :no_context} - end - end - - defp ensure_context_presence(_) do - {:error, :no_context} - end - - defp ensure_recipients_presence(%{"to" => [_ | _], "cc" => [_ | _]} = data), - do: {:ok, data} - - defp ensure_recipients_presence(%{"object" => object} = data) do - case Object.normalize(object) do - %{data: %{"actor" => actor}} -> - data = - data - |> Map.put("to", [actor]) - |> Map.put("cc", data["cc"] || []) - - {:ok, data} - - nil -> - {:error, :no_object} - - _ -> - {:error, :no_actor} - end - end - - defp ensure_recipients_presence(_) do - {:error, :no_object} - end end