From c4e43da63e03f66fd2feaa192c4d8192bbc3451c Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Thu, 1 Nov 2018 07:28:48 +0000 Subject: [PATCH 1/3] object: add safe object deletion function --- lib/pleroma/object.ex | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 1bcff5a7b..8f96fd8fb 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -1,6 +1,6 @@ defmodule Pleroma.Object do use Ecto.Schema - alias Pleroma.{Repo, Object} + alias Pleroma.{Repo, Object, Activity} import Ecto.{Query, Changeset} schema "objects" do @@ -52,4 +52,12 @@ def get_cached_by_ap_id(ap_id) do def context_mapping(context) do Object.change(%Object{}, %{data: %{"id" => context}}) end + + def delete(%Object{data: %{"id" => id}} = object) do + with Repo.delete(object), + Repo.delete_all(Activity.all_non_create_by_object_ap_id_q(id)), + {:ok, true} <- Cachex.del(:user_cache, "object:#{id}") do + :ok + end + end end From 2bf358d7b47f3c2dda91b0ac638b6a068fb40a4c Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Thu, 1 Nov 2018 07:29:12 +0000 Subject: [PATCH 2/3] activitypub: use Object.delete() instead of mutating the database and cache directly --- lib/pleroma/web/activity_pub/activity_pub.ex | 3 +-- lib/pleroma/web/common_api/common_api.ex | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 5a81f6fa2..48ae36ebd 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -273,8 +273,7 @@ def delete(%Object{data: %{"id" => id, "actor" => actor}} = object, local \\ tru "to" => [user.follower_address, "https://www.w3.org/ns/activitystreams#Public"] } - with Repo.delete(object), - Repo.delete_all(Activity.all_non_create_by_object_ap_id_q(id)), + with Object.delete(object), {:ok, activity} <- insert(data, local), :ok <- maybe_federate(activity), {:ok, _actor} <- User.decrease_note_count(user) do diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 8607cb6b3..8f47bb127 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -9,8 +9,7 @@ def delete(activity_id, user) do with %Activity{data: %{"object" => %{"id" => object_id}}} <- Repo.get(Activity, activity_id), %Object{} = object <- Object.normalize(object_id), true <- user.info["is_moderator"] || user.ap_id == object.data["actor"], - {:ok, delete} <- ActivityPub.delete(object), - {:ok, true} <- Cachex.del(:user_cache, "object:#{object_id}") do + {:ok, delete} <- ActivityPub.delete(object) do {:ok, delete} end end From f55fc68f766c71e81ce754408007e3b763f32e0f Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Thu, 1 Nov 2018 07:37:07 +0000 Subject: [PATCH 3/3] tests: add tests for object deletion --- test/object_test.exs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/object_test.exs b/test/object_test.exs index 5eb9b7530..3e398776c 100644 --- a/test/object_test.exs +++ b/test/object_test.exs @@ -19,4 +19,34 @@ test "it ensures uniqueness of the id" do {:error, _result} = Repo.insert(cs) end end + + describe "deletion function" do + test "deletes an object" do + object = insert(:note) + found_object = Object.get_by_ap_id(object.data["id"]) + + assert object == found_object + + Object.delete(found_object) + + found_object = Object.get_by_ap_id(object.data["id"]) + + refute object == found_object + end + + test "ensures cache is cleared for the object" do + object = insert(:note) + cached_object = Object.get_cached_by_ap_id(object.data["id"]) + + assert object == cached_object + + Object.delete(cached_object) + + {:ok, nil} = Cachex.get(:user_cache, "object:#{object.data["id"]}") + + cached_object = Object.get_cached_by_ap_id(object.data["id"]) + + refute object == cached_object + end + end end