From 0f412cf6e68fcebda3e94b71b7f182af689748bf Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Mon, 24 Dec 2018 02:25:36 +0300 Subject: [PATCH 1/9] Create tombstone instead of object deletion --- lib/pleroma/activity.ex | 20 ++++++++++++++- lib/pleroma/object.ex | 22 ++++++++++++++-- lib/pleroma/web/common_api/utils.ex | 7 +++++- test/activity_test.exs | 25 +++++++++++++++++++ test/user_test.exs | 2 +- test/web/activity_pub/activity_pub_test.exs | 2 +- test/web/activity_pub/transmogrifier_test.exs | 2 +- .../mastodon_api_controller_test.exs | 2 +- 8 files changed, 74 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 200addd6e..0845233ee 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 + import Ecto.{Query, Changeset} # https://github.com/tootsuite/mastodon/blob/master/app/models/notification.rb#L19 @mastodon_notification_types %{ @@ -103,4 +103,22 @@ def mastodon_notification_type(%Activity{data: %{"type" => unquote(ap_type)}}), 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 + tombstone = get_tombstone(activity) + + activity + |> change(%{data: tombstone}) + |> Repo.update() + end end diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 31c8dd5bd..436cf6d5d 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -62,9 +62,27 @@ def context_mapping(context) do Object.change(%Object{}, %{data: %{"id" => context}}) end + def get_tombstone(%Object{data: data}, deleted \\ DateTime.utc_now()) do + %{ + id: data["id"], + type: "tombstone", + deleted: deleted + } + end + + def swap_data_with_tombstone(object) do + tombstone = get_tombstone(object) + + object + |> Object.change(%{data: tombstone}) + |> Repo.update() + 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)), + 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), {:ok, true} <- Cachex.del(:object_cache, "object:#{id}") do {:ok, object} end diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index 142283684..d25fef6bc 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -69,7 +69,12 @@ def to_for_user_and_mentions(_user, mentions, inReplyTo, "direct") do mentioned_users = Enum.map(mentions, fn {_, %{ap_id: ap_id}} -> ap_id end) if inReplyTo do - {Enum.uniq([inReplyTo.data["actor"] | mentioned_users]), []} + to = + [inReplyTo.data["actor"] | mentioned_users] + |> Enum.uniq() + |> Enum.reject(&is_nil/1) + + {to, []} else {mentioned_users, []} end diff --git a/test/activity_test.exs b/test/activity_test.exs index 55849c522..87c9ddc50 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -1,5 +1,6 @@ defmodule Pleroma.ActivityTest do use Pleroma.DataCase + alias Pleroma.Activity import Pleroma.Factory test "returns an activity by it's AP id" do @@ -24,4 +25,28 @@ test "returns the activity that created an object" 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["object"]["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/user_test.exs b/test/user_test.exs index b4d8174c6..43a3687ec 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -625,7 +625,7 @@ test ".delete deactivates a user, all follow relationships and all create activi # TODO: Remove favorites, repeats, delete activities. - refute Repo.get(Activity, activity.id) + assert Repo.get(Activity, activity.id).data["type"] == "tombstone" 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/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 470ed08b2..95731a868 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -478,7 +478,7 @@ test "it creates a delete activity and deletes the original object" do assert Repo.get(Activity, delete.id) != nil - assert Repo.get(Object, object.id) == nil + assert Repo.get(Object, object.id).data["type"] == "tombstone" end end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 0428e052d..18a5ad3d0 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -363,7 +363,7 @@ test "it works for incoming deletes" do {:ok, %Activity{local: false}} = Transmogrifier.handle_incoming(data) - refute Repo.get(Activity, activity.id) + assert Repo.get(Activity, activity.id).data["type"] == "tombstone" 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 aec0f851c..6c6cc2a00 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 @@ test "when you created it", %{conn: conn} do assert %{} = json_response(conn, 200) - assert Repo.get(Activity, activity.id) == nil + assert Repo.get(Activity, activity.id).data["type"] == "tombstone" end test "when you didn't create it", %{conn: conn} do From 18a4cbb244dbc188f5a391626fb98e3a53571318 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Mon, 24 Dec 2018 20:09:18 +0300 Subject: [PATCH 2/9] Capitalize "tombstone" --- lib/pleroma/object.ex | 2 +- test/activity_test.exs | 4 ++-- test/user_test.exs | 2 +- test/web/activity_pub/activity_pub_test.exs | 2 +- test/web/activity_pub/transmogrifier_test.exs | 2 +- test/web/mastodon_api/mastodon_api_controller_test.exs | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 436cf6d5d..31f206c39 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -65,7 +65,7 @@ def context_mapping(context) do def get_tombstone(%Object{data: data}, deleted \\ DateTime.utc_now()) do %{ id: data["id"], - type: "tombstone", + type: "Tombstone", deleted: deleted } end diff --git a/test/activity_test.exs b/test/activity_test.exs index 87c9ddc50..c47fe39da 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -33,7 +33,7 @@ test "returns tombstone" do assert Pleroma.Activity.get_tombstone(activity, deleted) == %{ id: activity.data["object"]["id"], context: activity.data["context"], - type: "tombstone", + type: "Tombstone", published: activity.data["published"], deleted: deleted } @@ -43,7 +43,7 @@ test "swaps data with tombstone" do activity = insert(:note_activity) {:ok, deleted} = Pleroma.Activity.swap_data_with_tombstone(activity) - assert deleted.data.type == "tombstone" + assert deleted.data.type == "Tombstone" found_activity = Repo.get(Activity, activity.id) diff --git a/test/user_test.exs b/test/user_test.exs index 43a3687ec..f7a003c28 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -625,7 +625,7 @@ test ".delete deactivates a user, all follow relationships and all create activi # TODO: Remove favorites, repeats, delete activities. - assert Repo.get(Activity, activity.id).data["type"] == "tombstone" + assert Repo.get(Activity, activity.id).data["type"] == "Tombstone" 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/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 95731a868..4d16a87e2 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -478,7 +478,7 @@ test "it creates a delete activity and deletes the original object" do assert Repo.get(Activity, delete.id) != nil - assert Repo.get(Object, object.id).data["type"] == "tombstone" + assert Repo.get(Object, object.id).data["type"] == "Tombstone" end end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 18a5ad3d0..8ab240dff 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -363,7 +363,7 @@ test "it works for incoming deletes" do {:ok, %Activity{local: false}} = Transmogrifier.handle_incoming(data) - assert Repo.get(Activity, activity.id).data["type"] == "tombstone" + assert Repo.get(Activity, activity.id).data["type"] == "Tombstone" 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 6c6cc2a00..f1baa9953 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 @@ test "when you created it", %{conn: conn} do assert %{} = json_response(conn, 200) - assert Repo.get(Activity, activity.id).data["type"] == "tombstone" + assert Repo.get(Activity, activity.id).data["type"] == "Tombstone" end test "when you didn't create it", %{conn: conn} do From 2bbec33c7112ede3f93a7d35e9d5f3ac5a31ce05 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Tue, 25 Dec 2018 00:29:13 +0300 Subject: [PATCH 3/9] Fix failing tests --- lib/pleroma/activity.ex | 15 +++++++++------ lib/pleroma/notification.ex | 10 +++++++--- lib/pleroma/web/activity_pub/transmogrifier.ex | 3 ++- test/activity_test.exs | 2 +- .../incoming_documents/delete_handling_test.exs | 6 +++--- 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 0845233ee..be04363aa 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -108,17 +108,20 @@ def get_tombstone(%Activity{data: data}, deleted \\ DateTime.utc_now()) do %{ id: data["id"], context: data["context"], - type: "tombstone", + type: "Tombstone", published: data["published"], deleted: deleted } end def swap_data_with_tombstone(activity) do - tombstone = get_tombstone(activity) - - activity - |> change(%{data: tombstone}) - |> Repo.update() + 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/notification.ex b/lib/pleroma/notification.ex index 47f6b6ee7..457cba935 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -75,10 +75,14 @@ def get(%{id: user_id} = _user, id) do end end - def clear(user) do - query = from(n in Notification, where: n.user_id == ^user.id) + def clear(%User{} = user) do + from(n in Notification, where: n.user_id == ^user.id) + |> Repo.delete_all() + end - Repo.delete_all(query) + def clear(%Activity{} = activity) do + from(n in Notification, where: n.activity_id == ^activity.id) + |> Repo.delete_all() end def dismiss(%{id: user_id} = _user, id) do diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index e6af4b211..87514870d 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -586,7 +586,8 @@ def get_obj_helper(id) do end def set_reply_to_uri(%{"inReplyTo" => inReplyTo} = object) do - with false <- String.starts_with?(inReplyTo, "http"), + with false <- is_nil(inReplyTo), + false <- String.starts_with?(inReplyTo, "http"), {:ok, %{data: replied_to_object}} <- get_obj_helper(inReplyTo) do Map.put(object, "inReplyTo", replied_to_object["external_url"] || inReplyTo) else diff --git a/test/activity_test.exs b/test/activity_test.exs index c47fe39da..dd11323b5 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -31,7 +31,7 @@ test "returns tombstone" do deleted = DateTime.utc_now() assert Pleroma.Activity.get_tombstone(activity, deleted) == %{ - id: activity.data["object"]["id"], + id: activity.data["id"], context: activity.data["context"], type: "Tombstone", published: activity.data["published"], diff --git a/test/web/ostatus/incoming_documents/delete_handling_test.exs b/test/web/ostatus/incoming_documents/delete_handling_test.exs index 1e041e5b0..4e9c0f90f 100644 --- a/test/web/ostatus/incoming_documents/delete_handling_test.exs +++ b/test/web/ostatus/incoming_documents/delete_handling_test.exs @@ -23,9 +23,9 @@ test "it removes the mentioned activity" do {:ok, [delete]} = OStatus.handle_incoming(incoming) - refute Repo.get(Activity, note.id) - refute Repo.get(Activity, like.id) - refute Object.get_by_ap_id(note.data["object"]["id"]) + assert Repo.get(Activity, note.id).data["type"] == "Tombstone" + assert Repo.get(Activity, like.id).data["type"] == "Tombstone" + 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"]) From f75f707f6cf07c66a23ddbbe80a9b782a1ecb6f8 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Tue, 25 Dec 2018 03:00:06 +0300 Subject: [PATCH 4/9] 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 @@ def mastodon_notification_type(%Activity{data: %{"type" => unquote(ap_type)}}), 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 @@ def context_mapping(context) 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 @@ def swap_data_with_tombstone(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 @@ test "returns the activity that created an object" 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 @@ test "deletes an object" 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 @@ test "ensures cache is cleared for the object" 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 @@ test ".delete deactivates a user, all follow relationships and all create activi # 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 @@ test "it works for incoming deletes" 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 @@ test "when you created it", %{conn: conn} 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 @@ test "when you didn't create it", %{conn: conn} 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 @@ test "it removes the mentioned activity" 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"]) From aeb89bece60846d8311afd69d0ccfd1df80cbe65 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Tue, 25 Dec 2018 03:38:02 +0300 Subject: [PATCH 5/9] Remove unused test --- .../mastodon_api_controller_test.exs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index 23f63372c..00cf52d69 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -308,25 +308,6 @@ test "when you didn't create it", %{conn: conn} 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 From ab2ee436342c821b7def04a2b9f1f195c0437065 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Tue, 25 Dec 2018 03:41:14 +0300 Subject: [PATCH 6/9] Fix Activity test --- test/activity_test.exs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/activity_test.exs b/test/activity_test.exs index b949d0e2e..a4c13c5e4 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -5,14 +5,14 @@ defmodule Pleroma.ActivityTest do test "returns an activity by it's AP id" do activity = insert(:note_activity) - found_activity = Pleroma.Activity.get_by_ap_id(activity.data["id"]) + found_activity = Activity.get_by_ap_id(activity.data["id"]) assert activity == found_activity end test "returns activities by it's objects AP ids" do activity = insert(:note_activity) - [found_activity] = Pleroma.Activity.all_by_object_ap_id(activity.data["object"]["id"]) + [found_activity] = Activity.all_by_object_ap_id(activity.data["object"]["id"]) assert activity == found_activity end @@ -21,7 +21,7 @@ test "returns the activity that created an object" do activity = insert(:note_activity) found_activity = - Pleroma.Activity.get_create_activity_by_object_ap_id(activity.data["object"]["id"]) + Activity.get_create_activity_by_object_ap_id(activity.data["object"]["id"]) assert activity == found_activity end From ca2e9ce9cc98f046ea2be0a9051cdf06d253d7f6 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Tue, 25 Dec 2018 03:44:48 +0300 Subject: [PATCH 7/9] Revert unneeded changes --- lib/pleroma/notification.ex | 7 +------ lib/pleroma/web/activity_pub/transmogrifier.ex | 3 +-- lib/pleroma/web/common_api/utils.ex | 7 +------ 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 457cba935..47578d60e 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -75,16 +75,11 @@ def get(%{id: user_id} = _user, id) do end end - def clear(%User{} = user) do + def clear(user) do from(n in Notification, where: n.user_id == ^user.id) |> Repo.delete_all() end - def clear(%Activity{} = activity) do - from(n in Notification, where: n.activity_id == ^activity.id) - |> Repo.delete_all() - end - def dismiss(%{id: user_id} = _user, id) do notification = Repo.get(Notification, id) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 87514870d..e6af4b211 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -586,8 +586,7 @@ def get_obj_helper(id) do end def set_reply_to_uri(%{"inReplyTo" => inReplyTo} = object) do - with false <- is_nil(inReplyTo), - false <- String.starts_with?(inReplyTo, "http"), + with false <- String.starts_with?(inReplyTo, "http"), {:ok, %{data: replied_to_object}} <- get_obj_helper(inReplyTo) do Map.put(object, "inReplyTo", replied_to_object["external_url"] || inReplyTo) else diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index d25fef6bc..142283684 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -69,12 +69,7 @@ def to_for_user_and_mentions(_user, mentions, inReplyTo, "direct") do mentioned_users = Enum.map(mentions, fn {_, %{ap_id: ap_id}} -> ap_id end) if inReplyTo do - to = - [inReplyTo.data["actor"] | mentioned_users] - |> Enum.uniq() - |> Enum.reject(&is_nil/1) - - {to, []} + {Enum.uniq([inReplyTo.data["actor"] | mentioned_users]), []} else {mentioned_users, []} end From 340dd7a75e308cdf6936864e05d80a3bcdfdd6eb Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Tue, 25 Dec 2018 03:47:20 +0300 Subject: [PATCH 8/9] Format --- test/activity_test.exs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/activity_test.exs b/test/activity_test.exs index a4c13c5e4..ddcf54803 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -20,8 +20,7 @@ test "returns activities by it's objects AP ids" do test "returns the activity that created an object" do activity = insert(:note_activity) - found_activity = - Activity.get_create_activity_by_object_ap_id(activity.data["object"]["id"]) + found_activity = Activity.get_create_activity_by_object_ap_id(activity.data["object"]["id"]) assert activity == found_activity end From 012b7ab5e64eae468c630730f392ea8289a5d874 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Tue, 25 Dec 2018 23:40:57 +0300 Subject: [PATCH 9/9] Add test to check /object/:id does not leak the tombstone itself --- test/web/ostatus/ostatus_controller_test.exs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/test/web/ostatus/ostatus_controller_test.exs b/test/web/ostatus/ostatus_controller_test.exs index 560305c15..c503cadae 100644 --- a/test/web/ostatus/ostatus_controller_test.exs +++ b/test/web/ostatus/ostatus_controller_test.exs @@ -1,7 +1,7 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do use Pleroma.Web.ConnCase import Pleroma.Factory - alias Pleroma.{User, Repo} + alias Pleroma.{User, Repo, Object} alias Pleroma.Web.CommonAPI alias Pleroma.Web.OStatus.ActivityRepresenter @@ -110,6 +110,22 @@ test "404s on nonexisting objects", %{conn: conn} do |> response(404) end + test "404s on deleted objects", %{conn: conn} do + note_activity = insert(:note_activity) + [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["object"]["id"])) + object = Object.get_by_ap_id(note_activity.data["object"]["id"]) + + conn + |> get("/objects/#{uuid}") + |> response(200) + + Object.delete(object) + + conn + |> get("/objects/#{uuid}") + |> response(404) + end + test "gets an activity", %{conn: conn} do note_activity = insert(:note_activity) [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["id"]))