Merge branch 'feature/favorite-refactor' into 'develop'

Like activities: Move fixes to validator.

See merge request pleroma/pleroma!2467

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(:to, {:array, :string}, default: [])
-    field(:cc, {:array, :string})
+    field(:cc, {:array, :string}, default: [])
   end
 
   def cast_and_validate(data) do
@@ -31,7 +32,48 @@ 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()
+    |> 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)
+    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

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)

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} =
+    liked_object = Object.get_by_ap_id(object.data["object"])
-      Pleroma.Repo.transaction(fn ->
+    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}
+    {:ok, object, meta}
-      end)
-
-    result
   end
 
   # Nothing to do

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}} <-
+    with :ok <- ObjectValidator.fetch_actor_and_object(data),
-           {:casting_data,
+         {:ok, activity, _meta} <-
-            data |> LikeValidator.cast_data() |> Ecto.Changeset.apply_action(:insert)},
+           Pipeline.common_pipeline(data, local: false) do
-         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
       {: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

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{}
     }
 

test/web/activity_pub/object_validator_test.exs

@@ -36,6 +36,32 @@ 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 "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")
 

test/web/activity_pub/transmogrifier/like_handling_test.exs

@@ -0,0 +1,78 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
+# 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

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"})