From f75f707f6cf07c66a23ddbbe80a9b782a1ecb6f8 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Tue, 25 Dec 2018 03:00:06 +0300 Subject: [PATCH] Revert Activity tombstones, add ObjectTombstone struct --- lib/pleroma/activity.ex | 23 +----------------- lib/pleroma/object.ex | 21 ++++++++-------- lib/pleroma/object_tombstone.ex | 4 ++++ test/activity_test.exs | 24 ------------------- test/object_test.exs | 4 ++++ test/user_test.exs | 2 +- test/web/activity_pub/transmogrifier_test.exs | 2 +- .../mastodon_api_controller_test.exs | 21 +++++++++++++++- .../delete_handling_test.exs | 4 ++-- 9 files changed, 43 insertions(+), 62 deletions(-) create mode 100644 lib/pleroma/object_tombstone.ex diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index be04363aa..200addd6e 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -1,7 +1,7 @@ defmodule Pleroma.Activity do use Ecto.Schema alias Pleroma.{Repo, Activity, Notification} - import Ecto.{Query, Changeset} + import Ecto.Query # https://github.com/tootsuite/mastodon/blob/master/app/models/notification.rb#L19 @mastodon_notification_types %{ @@ -103,25 +103,4 @@ defmodule Pleroma.Activity do end def mastodon_notification_type(%Activity{}), do: nil - - def get_tombstone(%Activity{data: data}, deleted \\ DateTime.utc_now()) do - %{ - id: data["id"], - context: data["context"], - type: "Tombstone", - published: data["published"], - deleted: deleted - } - end - - def swap_data_with_tombstone(activity) do - with tombstone = get_tombstone(activity), - Notification.clear(activity), - {:ok, changed_activity} = - activity - |> change(%{data: tombstone}) - |> Repo.update() do - {:ok, changed_activity} - end - end end diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 31f206c39..5b1347b37 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, User, Activity} + alias Pleroma.{Repo, Object, User, Activity, ObjectTombstone} import Ecto.{Query, Changeset} schema "objects" do @@ -62,16 +62,17 @@ defmodule Pleroma.Object do Object.change(%Object{}, %{data: %{"id" => context}}) end - def get_tombstone(%Object{data: data}, deleted \\ DateTime.utc_now()) do - %{ - id: data["id"], - type: "Tombstone", + def make_tombstone(%Object{data: %{"id" => id, "type" => type}}, deleted \\ DateTime.utc_now()) do + %ObjectTombstone{ + id: id, + formerType: type, deleted: deleted } + |> Map.from_struct() end - def swap_data_with_tombstone(object) do - tombstone = get_tombstone(object) + def swap_object_with_tombstone(object) do + tombstone = make_tombstone(object) object |> Object.change(%{data: tombstone}) @@ -79,10 +80,8 @@ defmodule Pleroma.Object do end def delete(%Object{data: %{"id" => id}} = object) do - with swap_data_with_tombstone(object), - Activity.all_non_create_by_object_ap_id_q(id) - |> Repo.all() - |> Enum.each(&Activity.swap_data_with_tombstone/1), + with {:ok, _obj} = swap_object_with_tombstone(object), + Repo.delete_all(Activity.all_non_create_by_object_ap_id_q(id)), {:ok, true} <- Cachex.del(:object_cache, "object:#{id}") do {:ok, object} end diff --git a/lib/pleroma/object_tombstone.ex b/lib/pleroma/object_tombstone.ex new file mode 100644 index 000000000..64d836d3e --- /dev/null +++ b/lib/pleroma/object_tombstone.ex @@ -0,0 +1,4 @@ +defmodule Pleroma.ObjectTombstone do + @enforce_keys [:id, :formerType, :deleted] + defstruct [:id, :formerType, :deleted, type: "Tombstone"] +end diff --git a/test/activity_test.exs b/test/activity_test.exs index dd11323b5..b949d0e2e 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -25,28 +25,4 @@ defmodule Pleroma.ActivityTest do assert activity == found_activity end - - test "returns tombstone" do - activity = insert(:note_activity) - deleted = DateTime.utc_now() - - assert Pleroma.Activity.get_tombstone(activity, deleted) == %{ - id: activity.data["id"], - context: activity.data["context"], - type: "Tombstone", - published: activity.data["published"], - deleted: deleted - } - end - - test "swaps data with tombstone" do - activity = insert(:note_activity) - - {:ok, deleted} = Pleroma.Activity.swap_data_with_tombstone(activity) - assert deleted.data.type == "Tombstone" - - found_activity = Repo.get(Activity, activity.id) - - assert deleted.data.type == found_activity.data["type"] - end end diff --git a/test/object_test.exs b/test/object_test.exs index 909605560..c0a3de2d9 100644 --- a/test/object_test.exs +++ b/test/object_test.exs @@ -32,6 +32,8 @@ defmodule Pleroma.ObjectTest do found_object = Object.get_by_ap_id(object.data["id"]) refute object == found_object + + assert found_object.data["type"] == "Tombstone" end test "ensures cache is cleared for the object" do @@ -47,6 +49,8 @@ defmodule Pleroma.ObjectTest do cached_object = Object.get_cached_by_ap_id(object.data["id"]) refute object == cached_object + + assert cached_object.data["type"] == "Tombstone" end end end diff --git a/test/user_test.exs b/test/user_test.exs index f7a003c28..b4d8174c6 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -625,7 +625,7 @@ defmodule Pleroma.UserTest do # TODO: Remove favorites, repeats, delete activities. - assert Repo.get(Activity, activity.id).data["type"] == "Tombstone" + refute Repo.get(Activity, activity.id) end test "get_public_key_for_ap_id fetches a user that's not in the db" do diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 8ab240dff..0428e052d 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -363,7 +363,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do {:ok, %Activity{local: false}} = Transmogrifier.handle_incoming(data) - assert Repo.get(Activity, activity.id).data["type"] == "Tombstone" + refute Repo.get(Activity, activity.id) end test "it fails for incoming deletes with spoofed origin" do diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index f1baa9953..23f63372c 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -292,7 +292,7 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIControllerTest do assert %{} = json_response(conn, 200) - assert Repo.get(Activity, activity.id).data["type"] == "Tombstone" + refute Repo.get(Activity, activity.id) end test "when you didn't create it", %{conn: conn} do @@ -308,6 +308,25 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIControllerTest do assert Repo.get(Activity, activity.id) == activity end + + # test "404 when making an attempt to get it" do + # activity = insert(:note_activity) + # author = User.get_by_ap_id(activity.data["actor"]) + + # conn = + # conn + # |> assign(:user, author) + # |> delete("/api/v1/statuses/#{activity.id}") + + # assert %{} = json_response(conn, 200) + + # conn = + # build_conn() + # |> assign(:user, author) + # |> get("/api/v1/statuses/#{activity.id}") + + # assert %{} = json_response(conn, 200) + # end end describe "filters" do diff --git a/test/web/ostatus/incoming_documents/delete_handling_test.exs b/test/web/ostatus/incoming_documents/delete_handling_test.exs index 4e9c0f90f..c8fbff6cc 100644 --- a/test/web/ostatus/incoming_documents/delete_handling_test.exs +++ b/test/web/ostatus/incoming_documents/delete_handling_test.exs @@ -23,8 +23,8 @@ defmodule Pleroma.Web.OStatus.DeleteHandlingTest do {:ok, [delete]} = OStatus.handle_incoming(incoming) - assert Repo.get(Activity, note.id).data["type"] == "Tombstone" - assert Repo.get(Activity, like.id).data["type"] == "Tombstone" + refute Repo.get(Activity, note.id) + refute Repo.get(Activity, like.id) assert Object.get_by_ap_id(note.data["object"]["id"]).data["type"] == "Tombstone" assert Repo.get(Activity, second_note.id) assert Object.get_by_ap_id(second_note.data["object"]["id"])