Undoing: Move undoing likes to the pipeline everywhere.

This commit is contained in:
lain 2020-05-05 15:08:41 +02:00
parent f1da8882f9
commit a3071f0231
10 changed files with 75 additions and 105 deletions

View File

@ -29,7 +29,9 @@ defmodule Pleroma.User do
alias Pleroma.UserRelationship alias Pleroma.UserRelationship
alias Pleroma.Web alias Pleroma.Web
alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.ActivityPub
alias Pleroma.Web.ActivityPub.Builder
alias Pleroma.Web.ActivityPub.ObjectValidators.Types alias Pleroma.Web.ActivityPub.ObjectValidators.Types
alias Pleroma.Web.ActivityPub.Pipeline
alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Utils
alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI
alias Pleroma.Web.CommonAPI.Utils, as: CommonUtils alias Pleroma.Web.CommonAPI.Utils, as: CommonUtils
@ -1553,11 +1555,13 @@ defp delete_activity(%{data: %{"type" => "Create"}} = activity) do
end end
defp delete_activity(%{data: %{"type" => "Like"}} = activity) do defp delete_activity(%{data: %{"type" => "Like"}} = activity) do
object = Object.normalize(activity) actor =
activity.actor activity.actor
|> get_cached_by_ap_id() |> get_cached_by_ap_id()
|> ActivityPub.unlike(object)
{:ok, undo, _} = Builder.undo(actor, activity)
Pipeline.common_pipeline(undo, local: true)
end end
defp delete_activity(%{data: %{"type" => "Announce"}} = activity) do defp delete_activity(%{data: %{"type" => "Announce"}} = activity) do

View File

@ -398,29 +398,6 @@ defp do_unreact_with_emoji(user, reaction_id, options) do
end end
end end
@spec unlike(User.t(), Object.t(), String.t() | nil, boolean()) ::
{:ok, Activity.t(), Activity.t(), Object.t()} | {:ok, Object.t()} | {:error, any()}
def unlike(%User{} = actor, %Object{} = object, activity_id \\ nil, local \\ true) do
with {:ok, result} <-
Repo.transaction(fn -> do_unlike(actor, object, activity_id, local) end) do
result
end
end
defp do_unlike(actor, object, activity_id, local) do
with %Activity{} = like_activity <- get_existing_like(actor.ap_id, object),
unlike_data <- make_unlike_data(actor, like_activity, activity_id),
{:ok, unlike_activity} <- insert(unlike_data, local),
{:ok, _activity} <- Repo.delete(like_activity),
{:ok, object} <- remove_like_from_object(like_activity, object),
:ok <- maybe_federate(unlike_activity) do
{:ok, unlike_activity, like_activity, object}
else
nil -> {:ok, object}
{:error, error} -> Repo.rollback(error)
end
end
@spec announce(User.t(), Object.t(), String.t() | nil, boolean(), boolean()) :: @spec announce(User.t(), Object.t(), String.t() | nil, boolean(), boolean()) ::
{:ok, Activity.t(), Object.t()} | {:error, any()} {:ok, Activity.t(), Object.t()} | {:error, any()}
def announce( def announce(

View File

@ -5,8 +5,10 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
liked object, a `Follow` activity will add the user to the follower liked object, a `Follow` activity will add the user to the follower
collection, and so on. collection, and so on.
""" """
alias Pleroma.Activity
alias Pleroma.Notification alias Pleroma.Notification
alias Pleroma.Object alias Pleroma.Object
alias Pleroma.Repo
alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Utils
def handle(object, meta \\ []) def handle(object, meta \\ [])
@ -23,8 +25,25 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do
{:ok, object, meta} {:ok, object, meta}
end end
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
{:ok, object, meta}
end
end
# Nothing to do # Nothing to do
def handle(object, meta) do def handle(object, meta) do
{:ok, object, meta} {:ok, object, meta}
end end
def handle_undoing(%{data: %{"type" => "Like"}} = object) do
with %Object{} = liked_object <- Object.get_by_ap_id(object.data["object"]),
{:ok, _} <- Utils.remove_like_from_object(object, liked_object),
{:ok, _} <- Repo.delete(object) do
:ok
end
end
def handle_undoing(object), do: {:error, ["don't know how to handle", object]}
end end

View File

@ -865,19 +865,12 @@ def handle_incoming(
def handle_incoming( def handle_incoming(
%{ %{
"type" => "Undo", "type" => "Undo",
"object" => %{"type" => "Like", "object" => object_id}, "object" => %{"type" => "Like"}
"actor" => _actor,
"id" => id
} = data, } = data,
_options _options
) do ) do
with actor <- Containment.get_actor(data), with {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do
{:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(actor),
{:ok, object} <- get_obj_helper(object_id),
{:ok, activity, _, _} <- ActivityPub.unlike(actor, object, id, false) do
{:ok, activity} {:ok, activity}
else
_e -> :error
end end
end end

View File

@ -166,9 +166,12 @@ def favorite_helper(user, id) do
def unfavorite(id, user) do def unfavorite(id, user) do
with {_, %Activity{data: %{"type" => "Create"}} = activity} <- with {_, %Activity{data: %{"type" => "Create"}} = activity} <-
{:find_activity, Activity.get_by_id(id)} do {:find_activity, Activity.get_by_id(id)},
object = Object.normalize(activity) %Object{} = note <- Object.normalize(activity, false),
ActivityPub.unlike(user, object) %Activity{} = like <- Utils.get_existing_like(user.ap_id, note),
{:ok, undo, _} <- Builder.undo(user, like),
{:ok, activity, _} <- Pipeline.common_pipeline(undo, local: false) do
{:ok, activity}
else else
{:find_activity, _} -> {:error, :not_found} {:find_activity, _} -> {:error, :not_found}
_ -> {:error, dgettext("errors", "Could not unfavorite")} _ -> {:error, dgettext("errors", "Could not unfavorite")}

View File

@ -222,9 +222,9 @@ def favourite(%{assigns: %{user: user}} = conn, %{"id" => activity_id}) do
end end
@doc "POST /api/v1/statuses/:id/unfavourite" @doc "POST /api/v1/statuses/:id/unfavourite"
def unfavourite(%{assigns: %{user: user}} = conn, %{"id" => ap_id_or_id}) do def unfavourite(%{assigns: %{user: user}} = conn, %{"id" => activity_id}) do
with {:ok, _, _, %{data: %{"id" => id}}} <- CommonAPI.unfavorite(ap_id_or_id, user), with {:ok, _unfav} <- CommonAPI.unfavorite(activity_id, user),
%Activity{} = activity <- Activity.get_create_by_object_ap_id(id) do %Activity{} = activity <- Activity.get_by_id(activity_id) do
try_render(conn, "show.json", activity: activity, for: user, as: :activity) try_render(conn, "show.json", activity: activity, for: user, as: :activity)
end end
end end

View File

@ -724,7 +724,7 @@ test "liking an activity results in 1 notification, then 0 if the activity is un
assert length(Notification.for_user(user)) == 1 assert length(Notification.for_user(user)) == 1
{:ok, _, _, _} = CommonAPI.unfavorite(activity.id, other_user) {:ok, _} = CommonAPI.unfavorite(activity.id, other_user)
assert Enum.empty?(Notification.for_user(user)) assert Enum.empty?(Notification.for_user(user))
end end

View File

@ -995,66 +995,6 @@ test "reverts emoji unreact on error" do
end end
end end
describe "unliking" do
test_with_mock "sends an activity to federation", Federator, [:passthrough], [] do
Config.put([:instance, :federating], true)
note_activity = insert(:note_activity)
object = Object.normalize(note_activity)
user = insert(:user)
{:ok, object} = ActivityPub.unlike(user, object)
refute called(Federator.publish())
{:ok, _like_activity} = CommonAPI.favorite(user, note_activity.id)
object = Object.get_by_id(object.id)
assert object.data["like_count"] == 1
{:ok, unlike_activity, _, object} = ActivityPub.unlike(user, object)
assert object.data["like_count"] == 0
assert called(Federator.publish(unlike_activity))
end
test "reverts unliking on error" do
note_activity = insert(:note_activity)
user = insert(:user)
{:ok, like_activity} = CommonAPI.favorite(user, note_activity.id)
object = Object.normalize(note_activity)
assert object.data["like_count"] == 1
with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do
assert {:error, :reverted} = ActivityPub.unlike(user, object)
end
assert Object.get_by_ap_id(object.data["id"]) == object
assert object.data["like_count"] == 1
assert Activity.get_by_id(like_activity.id)
end
test "unliking a previously liked object" do
note_activity = insert(:note_activity)
object = Object.normalize(note_activity)
user = insert(:user)
# Unliking something that hasn't been liked does nothing
{:ok, object} = ActivityPub.unlike(user, object)
assert object.data["like_count"] == 0
{:ok, like_activity} = CommonAPI.favorite(user, note_activity.id)
object = Object.get_by_id(object.id)
assert object.data["like_count"] == 1
{:ok, unlike_activity, _, object} = ActivityPub.unlike(user, object)
assert object.data["like_count"] == 0
assert Activity.get_by_id(like_activity.id) == nil
assert note_activity.actor in unlike_activity.recipients
end
end
describe "announcing an object" do describe "announcing an object" do
test "adds an announce activity to the db" do test "adds an announce activity to the db" do
note_activity = insert(:note_activity) note_activity = insert(:note_activity)

View File

@ -5,6 +5,7 @@
defmodule Pleroma.Web.ActivityPub.SideEffectsTest do defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
use Pleroma.DataCase use Pleroma.DataCase
alias Pleroma.Activity
alias Pleroma.Notification alias Pleroma.Notification
alias Pleroma.Object alias Pleroma.Object
alias Pleroma.Repo alias Pleroma.Repo
@ -15,6 +16,34 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
import Pleroma.Factory import Pleroma.Factory
describe "Undo objects" do
setup do
poster = insert(:user)
user = insert(:user)
{:ok, post} = CommonAPI.post(poster, %{"status" => "hey"})
{:ok, like} = CommonAPI.favorite(user, post.id)
{:ok, undo_data, _meta} = Builder.undo(user, like)
{:ok, like_undo, _meta} = ActivityPub.persist(undo_data, local: true)
%{like_undo: like_undo, post: post, like: like}
end
test "a like undo removes the like from the object", %{like_undo: like_undo, post: post} do
{:ok, _like_undo, _} = SideEffects.handle(like_undo)
object = Object.get_by_ap_id(post.data["object"])
assert object.data["like_count"] == 0
assert object.data["likes"] == []
end
test "deletes the original like", %{like_undo: like_undo, like: like} do
{:ok, _like_undo, _} = SideEffects.handle(like_undo)
refute Activity.get_by_id(like.id)
end
end
describe "like objects" do describe "like objects" do
setup do setup do
poster = insert(:user) poster = insert(:user)

View File

@ -6,6 +6,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UndoHandlingTest do
use Pleroma.DataCase use Pleroma.DataCase
alias Pleroma.Activity alias Pleroma.Activity
alias Pleroma.Object
alias Pleroma.User alias Pleroma.User
alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.ActivityPub.Transmogrifier
alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI
@ -67,7 +68,11 @@ test "it works for incoming unlikes with an existing like activity" do
assert data["actor"] == "http://mastodon.example.org/users/admin" assert data["actor"] == "http://mastodon.example.org/users/admin"
assert data["type"] == "Undo" assert data["type"] == "Undo"
assert data["id"] == "http://mastodon.example.org/users/admin#likes/2/undo" assert data["id"] == "http://mastodon.example.org/users/admin#likes/2/undo"
assert data["object"]["id"] == "http://mastodon.example.org/users/admin#likes/2" assert data["object"] == "http://mastodon.example.org/users/admin#likes/2"
note = Object.get_by_ap_id(like_data["object"])
assert note.data["like_count"] == 0
assert note.data["likes"] == []
end end
test "it works for incoming unlikes with an existing like activity and a compact object" do test "it works for incoming unlikes with an existing like activity and a compact object" do
@ -94,7 +99,7 @@ test "it works for incoming unlikes with an existing like activity and a compact
assert data["actor"] == "http://mastodon.example.org/users/admin" assert data["actor"] == "http://mastodon.example.org/users/admin"
assert data["type"] == "Undo" assert data["type"] == "Undo"
assert data["id"] == "http://mastodon.example.org/users/admin#likes/2/undo" assert data["id"] == "http://mastodon.example.org/users/admin#likes/2/undo"
assert data["object"]["id"] == "http://mastodon.example.org/users/admin#likes/2" assert data["object"] == "http://mastodon.example.org/users/admin#likes/2"
end end
test "it works for incoming unannounces with an existing notice" do test "it works for incoming unannounces with an existing notice" do