From f3e061c9645571997a01b1091d0e8a3f68c6bb21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Sat, 6 Aug 2022 03:24:31 +0200 Subject: [PATCH 01/18] Object: remove context_id field 30 to 70% of the objects in the object table are simple JSON objects containing a single field, 'id', being the context's ID. The reason for the creation of an object per context seems to be an old relic from the StatusNet era, and has only been used nowadays as an helper for threads in Pleroma-FE via the `pleroma.conversation_id` field in status views. An object per context was created, and its numerical ID (table column) was used and stored as 'context_id' in the object and activity along with the full 'context' URI/string. This commit removes this field and stops creation of objects for each context, which will also allow incoming activities to use activity IDs as contexts, something which was not possible before, or would have been very broken under most circumstances. The `pleroma.conversation_id` field has been reimplemented in a way to maintain backwards-compatibility by calculating a CRC32 of the full context URI/string in the object, instead of relying on the row ID for the created context object. --- lib/pleroma/object.ex | 4 --- .../article_note_page_validator.ex | 2 +- .../object_validators/common_fixes.ex | 4 +-- .../object_validators/event_validator.ex | 2 +- .../object_validators/question_validator.ex | 2 +- lib/pleroma/web/activity_pub/utils.ex | 23 ++------------- lib/pleroma/web/common_api/utils.ex | 29 ------------------- .../web/mastodon_api/views/status_view.ex | 2 +- .../web/activity_pub/activity_pub_test.exs | 5 +--- .../transmogrifier/question_handling_test.exs | 3 -- .../web/activity_pub/transmogrifier_test.exs | 2 -- test/pleroma/web/activity_pub/utils_test.exs | 4 --- test/pleroma/web/common_api/utils_test.exs | 27 ----------------- .../mastodon_api/views/status_view_test.exs | 2 +- 14 files changed, 9 insertions(+), 102 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index fe264b5e0b..c214a79c59 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -208,10 +208,6 @@ def get_cached_by_ap_id(ap_id) do end end - def context_mapping(context) do - Object.change(%Object{}, %{data: %{"id" => context}}) - end - def make_tombstone(%Object{data: %{"id" => id, "type" => type}}, deleted \\ DateTime.utc_now()) do %ObjectTombstone{ id: id, diff --git a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex index 57c8d1dc00..c5fb940341 100644 --- a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex @@ -94,7 +94,7 @@ def changeset(struct, data) do defp validate_data(data_cng) do data_cng |> validate_inclusion(:type, ["Article", "Note", "Page"]) - |> validate_required([:id, :actor, :attributedTo, :type, :context, :context_id]) + |> validate_required([:id, :actor, :attributedTo, :type, :context]) |> CommonValidations.validate_any_presence([:cc, :to]) |> CommonValidations.validate_fields_match([:actor, :attributedTo]) |> CommonValidations.validate_actor_presence() diff --git a/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex b/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex index 4f8c083ebc..c7e292bec3 100644 --- a/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex +++ b/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex @@ -22,14 +22,12 @@ def cast_and_filter_recipients(message, field, follower_collection, field_fallba end def fix_object_defaults(data) do - %{data: %{"id" => context}, id: context_id} = - Utils.create_context(data["context"] || data["conversation"]) + context = Utils.maybe_create_context(data["context"] || data["conversation"]) %User{follower_address: follower_collection} = User.get_cached_by_ap_id(data["attributedTo"]) data |> Map.put("context", context) - |> Map.put("context_id", context_id) |> cast_and_filter_recipients("to", follower_collection) |> cast_and_filter_recipients("cc", follower_collection) |> cast_and_filter_recipients("bto", follower_collection) diff --git a/lib/pleroma/web/activity_pub/object_validators/event_validator.ex b/lib/pleroma/web/activity_pub/object_validators/event_validator.ex index 0e99f20375..ab204f69a0 100644 --- a/lib/pleroma/web/activity_pub/object_validators/event_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/event_validator.ex @@ -62,7 +62,7 @@ def changeset(struct, data) do defp validate_data(data_cng) do data_cng |> validate_inclusion(:type, ["Event"]) - |> validate_required([:id, :actor, :attributedTo, :type, :context, :context_id]) + |> validate_required([:id, :actor, :attributedTo, :type, :context]) |> CommonValidations.validate_any_presence([:cc, :to]) |> CommonValidations.validate_fields_match([:actor, :attributedTo]) |> CommonValidations.validate_actor_presence() diff --git a/lib/pleroma/web/activity_pub/object_validators/question_validator.ex b/lib/pleroma/web/activity_pub/object_validators/question_validator.ex index 9412be4bc4..ce33051428 100644 --- a/lib/pleroma/web/activity_pub/object_validators/question_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/question_validator.ex @@ -80,7 +80,7 @@ def changeset(struct, data) do defp validate_data(data_cng) do data_cng |> validate_inclusion(:type, ["Question"]) - |> validate_required([:id, :actor, :attributedTo, :type, :context, :context_id]) + |> validate_required([:id, :actor, :attributedTo, :type, :context]) |> CommonValidations.validate_any_presence([:cc, :to]) |> CommonValidations.validate_fields_match([:actor, :attributedTo]) |> CommonValidations.validate_actor_presence() diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 9cde7805cd..d3b7d804fd 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -154,22 +154,7 @@ def get_notified_from_object(object) do Notification.get_notified_from_activity(%Activity{data: object}, false) end - def create_context(context) do - context = context || generate_id("contexts") - - # Ecto has problems accessing the constraint inside the jsonb, - # so we explicitly check for the existed object before insert - object = Object.get_cached_by_ap_id(context) - - with true <- is_nil(object), - changeset <- Object.context_mapping(context), - {:ok, inserted_object} <- Repo.insert(changeset) do - inserted_object - else - _ -> - object - end - end + def maybe_create_context(context), do: context || generate_id("contexts") @doc """ Enqueues an activity for federation if it's local @@ -201,18 +186,16 @@ def lazy_put_activity_defaults(map, true) do |> Map.put_new("id", "pleroma:fakeid") |> Map.put_new_lazy("published", &make_date/0) |> Map.put_new("context", "pleroma:fakecontext") - |> Map.put_new("context_id", -1) |> lazy_put_object_defaults(true) end def lazy_put_activity_defaults(map, _fake?) do - %{data: %{"id" => context}, id: context_id} = create_context(map["context"]) + context = maybe_create_context(map["context"]) map |> Map.put_new_lazy("id", &generate_activity_id/0) |> Map.put_new_lazy("published", &make_date/0) |> Map.put_new("context", context) - |> Map.put_new("context_id", context_id) |> lazy_put_object_defaults(false) end @@ -226,7 +209,6 @@ defp lazy_put_object_defaults(%{"object" => map} = activity, true) |> Map.put_new("id", "pleroma:fake_object_id") |> Map.put_new_lazy("published", &make_date/0) |> Map.put_new("context", activity["context"]) - |> Map.put_new("context_id", activity["context_id"]) |> Map.put_new("fake", true) %{activity | "object" => object} @@ -239,7 +221,6 @@ defp lazy_put_object_defaults(%{"object" => map} = activity, _) |> Map.put_new_lazy("id", &generate_object_id/0) |> Map.put_new_lazy("published", &make_date/0) |> Map.put_new("context", activity["context"]) - |> Map.put_new("context_id", activity["context_id"]) %{activity | "object" => object} end diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index ce850b0380..052bd77704 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -449,35 +449,6 @@ def get_report_statuses(%User{ap_id: actor}, %{status_ids: status_ids}) def get_report_statuses(_, _), do: {:ok, nil} - # DEPRECATED mostly, context objects are now created at insertion time. - def context_to_conversation_id(context) do - with %Object{id: id} <- Object.get_cached_by_ap_id(context) do - id - else - _e -> - changeset = Object.context_mapping(context) - - case Repo.insert(changeset) do - {:ok, %{id: id}} -> - id - - # This should be solved by an upsert, but it seems ecto - # has problems accessing the constraint inside the jsonb. - {:error, _} -> - Object.get_cached_by_ap_id(context).id - end - end - end - - def conversation_id_to_context(id) do - with %Object{data: %{"id" => context}} <- Repo.get(Object, id) do - context - else - _e -> - {:error, dgettext("errors", "No such conversation")} - end - end - def validate_character_limit("" = _full_payload, [] = _attachments) do {:error, dgettext("errors", "Cannot post an empty status without attachments")} end diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 1ebfd67403..31a0c420fd 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -61,7 +61,7 @@ defp get_context_id(%{data: %{"context_id" => context_id}}) when not is_nil(cont do: context_id defp get_context_id(%{data: %{"context" => context}}) when is_binary(context), - do: Utils.context_to_conversation_id(context) + do: :erlang.crc32(context) defp get_context_id(_), do: nil diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index 4d7e76266f..b8d73ea104 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -554,7 +554,6 @@ test "inserts a given map into the activity database, giving it an id if it has assert activity.data["ok"] == data["ok"] assert activity.data["id"] == given_id assert activity.data["context"] == "blabla" - assert activity.data["context_id"] end test "adds a context when none is there" do @@ -576,8 +575,6 @@ test "adds a context when none is there" do assert is_binary(activity.data["context"]) assert is_binary(object.data["context"]) - assert activity.data["context_id"] - assert object.data["context_id"] end test "adds an id to a given object if it lacks one and is a note and inserts it to the object database" do @@ -1612,7 +1609,7 @@ test "it can create a Flag activity", }) assert Repo.aggregate(Activity, :count, :id) == 1 - assert Repo.aggregate(Object, :count, :id) == 2 + assert Repo.aggregate(Object, :count, :id) == 1 assert Repo.aggregate(Notification, :count, :id) == 0 end end diff --git a/test/pleroma/web/activity_pub/transmogrifier/question_handling_test.exs b/test/pleroma/web/activity_pub/transmogrifier/question_handling_test.exs index d22ec400dc..d310705465 100644 --- a/test/pleroma/web/activity_pub/transmogrifier/question_handling_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier/question_handling_test.exs @@ -33,8 +33,6 @@ test "Mastodon Question activity" do assert object.data["context"] == "tag:mastodon.sdf.org,2019-05-10:objectId=15095122:objectType=Conversation" - assert object.data["context_id"] - assert object.data["anyOf"] == [] assert Enum.sort(object.data["oneOf"]) == @@ -68,7 +66,6 @@ test "Mastodon Question activity" do reply_object = Object.normalize(reply_activity, fetch: false) assert reply_object.data["context"] == object.data["context"] - assert reply_object.data["context_id"] == object.data["context_id"] end test "Mastodon Question activity with HTML tags in plaintext" do diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index 335fe1a30f..b254e5591a 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier_test.exs @@ -227,7 +227,6 @@ test "it strips internal fields" do assert is_nil(modified["object"]["like_count"]) assert is_nil(modified["object"]["announcements"]) assert is_nil(modified["object"]["announcement_count"]) - assert is_nil(modified["object"]["context_id"]) assert is_nil(modified["object"]["generator"]) end @@ -242,7 +241,6 @@ test "it strips internal fields of article" do assert is_nil(modified["object"]["like_count"]) assert is_nil(modified["object"]["announcements"]) assert is_nil(modified["object"]["announcement_count"]) - assert is_nil(modified["object"]["context_id"]) assert is_nil(modified["object"]["likes"]) end diff --git a/test/pleroma/web/activity_pub/utils_test.exs b/test/pleroma/web/activity_pub/utils_test.exs index 447621718e..e428938495 100644 --- a/test/pleroma/web/activity_pub/utils_test.exs +++ b/test/pleroma/web/activity_pub/utils_test.exs @@ -429,7 +429,6 @@ test "returns map with id and published data" do object = Object.normalize(note_activity, fetch: false) res = Utils.lazy_put_activity_defaults(%{"context" => object.data["id"]}) assert res["context"] == object.data["id"] - assert res["context_id"] == object.id assert res["id"] assert res["published"] end @@ -437,7 +436,6 @@ test "returns map with id and published data" do test "returns map with fake id and published data" do assert %{ "context" => "pleroma:fakecontext", - "context_id" => -1, "id" => "pleroma:fakeid", "published" => _ } = Utils.lazy_put_activity_defaults(%{}, true) @@ -454,13 +452,11 @@ test "returns activity data with object" do }) assert res["context"] == object.data["id"] - assert res["context_id"] == object.id assert res["id"] assert res["published"] assert res["object"]["id"] assert res["object"]["published"] assert res["object"]["context"] == object.data["id"] - assert res["object"]["context_id"] == object.id end end diff --git a/test/pleroma/web/common_api/utils_test.exs b/test/pleroma/web/common_api/utils_test.exs index 5b20199693..33ede0f04b 100644 --- a/test/pleroma/web/common_api/utils_test.exs +++ b/test/pleroma/web/common_api/utils_test.exs @@ -273,22 +273,6 @@ test "delegated renderers" do end end - describe "context_to_conversation_id" do - test "creates a mapping object" do - conversation_id = Utils.context_to_conversation_id("random context") - object = Object.get_by_ap_id("random context") - - assert conversation_id == object.id - end - - test "returns an existing mapping for an existing object" do - {:ok, object} = Object.context_mapping("random context") |> Repo.insert() - conversation_id = Utils.context_to_conversation_id("random context") - - assert conversation_id == object.id - end - end - describe "formats date to asctime" do test "when date is in ISO 8601 format" do date = DateTime.utc_now() |> DateTime.to_iso8601() @@ -517,17 +501,6 @@ test "returns empty string when date invalid" do end end - describe "conversation_id_to_context/1" do - test "returns id" do - object = insert(:note) - assert Utils.conversation_id_to_context(object.id) == object.data["id"] - end - - test "returns error if object not found" do - assert Utils.conversation_id_to_context("123") == {:error, "No such conversation"} - end - end - describe "maybe_notify_mentioned_recipients/2" do test "returns recipients when activity is not `Create`" do activity = insert(:like_activity) diff --git a/test/pleroma/web/mastodon_api/views/status_view_test.exs b/test/pleroma/web/mastodon_api/views/status_view_test.exs index 5d81c92b98..fa71f86f29 100644 --- a/test/pleroma/web/mastodon_api/views/status_view_test.exs +++ b/test/pleroma/web/mastodon_api/views/status_view_test.exs @@ -226,7 +226,7 @@ test "a note activity" do object_data = Object.normalize(note, fetch: false).data user = User.get_cached_by_ap_id(note.data["actor"]) - convo_id = Utils.context_to_conversation_id(object_data["context"]) + convo_id = :erlang.crc32(object_data["context"]) status = StatusView.render("show.json", %{activity: note}) From 7f71e3d0fe347920834be8c8e28d9c7f5b169e9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Sat, 6 Aug 2022 03:32:58 +0200 Subject: [PATCH 02/18] CommonFields: remove context_id --- .../web/activity_pub/object_validators/common_fields.ex | 2 -- lib/pleroma/web/mastodon_api/views/status_view.ex | 3 --- 2 files changed, 5 deletions(-) diff --git a/lib/pleroma/web/activity_pub/object_validators/common_fields.ex b/lib/pleroma/web/activity_pub/object_validators/common_fields.ex index 8e768ffbfa..095bd0da2e 100644 --- a/lib/pleroma/web/activity_pub/object_validators/common_fields.ex +++ b/lib/pleroma/web/activity_pub/object_validators/common_fields.ex @@ -51,8 +51,6 @@ defmacro status_object_fields do field(:summary, :string) field(:context, :string) - # short identifier for PleromaFE to group statuses by context - field(:context_id, :integer) field(:sensitive, :boolean, default: false) field(:replies_count, :integer, default: 0) diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 31a0c420fd..3ffe55c5d4 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -57,9 +57,6 @@ defp get_replied_to_activities(activities) do end) end - defp get_context_id(%{data: %{"context_id" => context_id}}) when not is_nil(context_id), - do: context_id - defp get_context_id(%{data: %{"context" => context}}) when is_binary(context), do: :erlang.crc32(context) From a9111bcaf2ba2371a1021dc171dab50615a4c040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Sun, 7 Aug 2022 20:37:17 +0200 Subject: [PATCH 03/18] StatusView: clear MSB on calculated conversation_id This field seems to be a left-over from the StatusNet era. If your application uses `pleroma.conversation_id`: this field is deprecated. It is currently stubbed instead by doing a CRC32 of the context, and clearing the MSB to avoid overflow exceptions with signed integers on the different clients using this field (Java/Kotlin code, mostly; see Husky and probably other mobile clients.) This should be removed in a future version of Pleroma. Pleroma-FE currently depends on this field, as well. --- lib/pleroma/web/mastodon_api/views/status_view.ex | 15 +++++++++++++-- test/pleroma/web/common_api/utils_test.exs | 1 - .../web/mastodon_api/views/status_view_test.exs | 3 +-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 3ffe55c5d4..5cb524f56f 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -57,8 +57,19 @@ defp get_replied_to_activities(activities) do end) end - defp get_context_id(%{data: %{"context" => context}}) when is_binary(context), - do: :erlang.crc32(context) + # DEPRECATED This field seems to be a left-over from the StatusNet era. + # If your application uses `pleroma.conversation_id`: this field is deprecated. + # It is currently stubbed instead by doing a CRC32 of the context, and + # clearing the MSB to avoid overflow exceptions with signed integers on the + # different clients using this field (Java/Kotlin code, mostly; see Husky.) + # This should be removed in a future version of Pleroma. Pleroma-FE currently + # depends on this field, as well. + defp get_context_id(%{data: %{"context" => context}}) when is_binary(context) do + use Bitwise + + :erlang.crc32(context) + |> band(bnot(0x8000_0000)) + end defp get_context_id(_), do: nil diff --git a/test/pleroma/web/common_api/utils_test.exs b/test/pleroma/web/common_api/utils_test.exs index 33ede0f04b..b538c59794 100644 --- a/test/pleroma/web/common_api/utils_test.exs +++ b/test/pleroma/web/common_api/utils_test.exs @@ -4,7 +4,6 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do alias Pleroma.Builders.UserBuilder - alias Pleroma.Object alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI.ActivityDraft alias Pleroma.Web.CommonAPI.Utils diff --git a/test/pleroma/web/mastodon_api/views/status_view_test.exs b/test/pleroma/web/mastodon_api/views/status_view_test.exs index fa71f86f29..4429f73c44 100644 --- a/test/pleroma/web/mastodon_api/views/status_view_test.exs +++ b/test/pleroma/web/mastodon_api/views/status_view_test.exs @@ -14,7 +14,6 @@ defmodule Pleroma.Web.MastodonAPI.StatusViewTest do alias Pleroma.User alias Pleroma.UserRelationship alias Pleroma.Web.CommonAPI - alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.MastodonAPI.AccountView alias Pleroma.Web.MastodonAPI.StatusView @@ -226,7 +225,7 @@ test "a note activity" do object_data = Object.normalize(note, fetch: false).data user = User.get_cached_by_ap_id(note.data["actor"]) - convo_id = :erlang.crc32(object_data["context"]) + convo_id = :erlang.crc32(object_data["context"]) |> Bitwise.band(Bitwise.bnot(0x8000_0000)) status = StatusView.render("show.json", %{activity: note}) From def0f5dc2e76b7c4ac22b393abf7f5de5e197659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Sun, 7 Aug 2022 20:39:35 +0200 Subject: [PATCH 04/18] StatusView: implement pleroma.context field This field replaces the now deprecated conversation_id field, and now exposes the ActivityPub object `context` directly via the MastoAPI instead of relying on StatusNet-era data concepts. --- lib/pleroma/web/api_spec/schemas/status.ex | 9 ++++++++- lib/pleroma/web/mastodon_api/views/status_view.ex | 1 + .../mastodon_api/controllers/status_controller_test.exs | 2 ++ test/pleroma/web/mastodon_api/views/status_view_test.exs | 3 +++ 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/api_spec/schemas/status.ex b/lib/pleroma/web/api_spec/schemas/status.ex index 6e6e30315a..8c19a9d9ff 100644 --- a/lib/pleroma/web/api_spec/schemas/status.ex +++ b/lib/pleroma/web/api_spec/schemas/status.ex @@ -142,9 +142,15 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Status do description: "A map consisting of alternate representations of the `content` property with the key being it's mimetype. Currently the only alternate representation supported is `text/plain`" }, + context: %Schema{ + type: :string, + description: "The thread identifier the status is associated with" + }, conversation_id: %Schema{ type: :integer, - description: "The ID of the AP context the status is associated with (if any)" + deprecated: true, + description: + "The ID of the AP context the status is associated with (if any); deprecated, please use `context` instead" }, direct_conversation_id: %Schema{ type: :integer, @@ -319,6 +325,7 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Status do "pinned" => false, "pleroma" => %{ "content" => %{"text/plain" => "foobar"}, + "context" => "http://localhost:4001/objects/8b4c0c80-6a37-4d2a-b1b9-05a19e3875aa", "conversation_id" => 345_972, "direct_conversation_id" => nil, "emoji_reactions" => [], diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 5cb524f56f..a4d6cd8077 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -375,6 +375,7 @@ def render("show.json", %{activity: %{data: %{"object" => _object}} = activity} pleroma: %{ local: activity.local, conversation_id: get_context_id(activity), + context: object.data["context"], in_reply_to_account_acct: reply_to_user && reply_to_user.nickname, content: %{"text/plain" => content_plaintext}, spoiler_text: %{"text/plain" => summary}, diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index dc6912b7bb..b85b7da9a9 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -262,6 +262,7 @@ test "posting a fake status", %{conn: conn} do |> Map.put("url", nil) |> Map.put("uri", nil) |> Map.put("created_at", nil) + |> Kernel.put_in(["pleroma", "context"], nil) |> Kernel.put_in(["pleroma", "conversation_id"], nil) fake_conn = @@ -285,6 +286,7 @@ test "posting a fake status", %{conn: conn} do |> Map.put("url", nil) |> Map.put("uri", nil) |> Map.put("created_at", nil) + |> Kernel.put_in(["pleroma", "context"], nil) |> Kernel.put_in(["pleroma", "conversation_id"], nil) assert real_status == fake_status diff --git a/test/pleroma/web/mastodon_api/views/status_view_test.exs b/test/pleroma/web/mastodon_api/views/status_view_test.exs index 4429f73c44..f9cdfee4e1 100644 --- a/test/pleroma/web/mastodon_api/views/status_view_test.exs +++ b/test/pleroma/web/mastodon_api/views/status_view_test.exs @@ -17,6 +17,8 @@ defmodule Pleroma.Web.MastodonAPI.StatusViewTest do alias Pleroma.Web.MastodonAPI.AccountView alias Pleroma.Web.MastodonAPI.StatusView + require Bitwise + import Pleroma.Factory import Tesla.Mock import OpenApiSpex.TestAssertions @@ -278,6 +280,7 @@ test "a note activity" do pleroma: %{ local: true, conversation_id: convo_id, + context: object_data["context"], in_reply_to_account_acct: nil, content: %{"text/plain" => HTML.strip_tags(object_data["content"])}, spoiler_text: %{"text/plain" => HTML.strip_tags(object_data["summary"])}, From c559c240d1a56f05fc70f69ae6b8c0809026fa2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Sun, 7 Aug 2022 20:41:24 +0200 Subject: [PATCH 05/18] Migrations: delete context objects These objects represent from 30 to 70% of the rows on the objects table, based on numbers from a few live instances (single-user, small, large.) As those pseudo-objects prevent creating objects with those actual IDs, deleting them is a better solution. This could have happened if an object used another object's ID as its context. --- ...5023_data_migration_delete_context_objects.exs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 priv/repo/migrations/20220807125023_data_migration_delete_context_objects.exs diff --git a/priv/repo/migrations/20220807125023_data_migration_delete_context_objects.exs b/priv/repo/migrations/20220807125023_data_migration_delete_context_objects.exs new file mode 100644 index 0000000000..debb474b2b --- /dev/null +++ b/priv/repo/migrations/20220807125023_data_migration_delete_context_objects.exs @@ -0,0 +1,15 @@ +defmodule Pleroma.Repo.Migrations.DataMigrationDeleteContextObjects do + use Ecto.Migration + + require Logger + + @doc "This migration removes objects created exclusively for contexts, containing only an `id` field." + + def change do + Logger.warn( + "This migration can take a very long time to execute, depending on your database size. Please be patient, Pleroma-tan is doing her best!\n" + ) + + execute("DELETE FROM objects WHERE (data->>'type') IS NULL;") + end +end From 3b6784b1de8454ab8c009ac688f6c62039117742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Tue, 2 Aug 2022 17:30:36 +0200 Subject: [PATCH 06/18] CreateGenericValidator: fix reply context fixing Incoming Pleroma replies to a Misskey thread were rejected due to a broken context fix, which caused them to not be visible until a non-Pleroma user interacted with the replies. This fix properly sets the post-fix object context to its parent Create activity as well, if it was changed. --- .../create_generic_validator.ex | 2 +- ...reate-pleroma-reply-to-misskey-thread.json | 61 +++++++++++++++++ .../tesla_mock/helene@p.helene.moe.json | 50 ++++++++++++++ .../mametsuko@mk.absturztau.be.json | 65 +++++++++++++++++++ .../mk.absturztau.be-93e7nm8wqg.json | 44 +++++++++++++ .../p.helene.moe-AM7S6vZQmL6pI9TgPY.json | 36 ++++++++++ .../create_generic_validator_test.exs | 9 ++- .../web/activity_pub/transmogrifier_test.exs | 15 +++-- test/support/http_request_mock.ex | 36 ++++++++++ 9 files changed, 309 insertions(+), 9 deletions(-) create mode 100644 test/fixtures/create-pleroma-reply-to-misskey-thread.json create mode 100644 test/fixtures/tesla_mock/helene@p.helene.moe.json create mode 100644 test/fixtures/tesla_mock/mametsuko@mk.absturztau.be.json create mode 100644 test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg.json create mode 100644 test/fixtures/tesla_mock/p.helene.moe-AM7S6vZQmL6pI9TgPY.json diff --git a/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex b/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex index c9a621cb12..2395abfd4a 100644 --- a/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/create_generic_validator.ex @@ -75,7 +75,7 @@ def fix(data, meta) do data |> CommonFixes.fix_actor() - |> Map.put_new("context", object["context"]) + |> Map.put("context", object["context"]) |> fix_addressing(object) end diff --git a/test/fixtures/create-pleroma-reply-to-misskey-thread.json b/test/fixtures/create-pleroma-reply-to-misskey-thread.json new file mode 100644 index 0000000000..0c31efa76e --- /dev/null +++ b/test/fixtures/create-pleroma-reply-to-misskey-thread.json @@ -0,0 +1,61 @@ +{ + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://p.helene.moe/schemas/litepub-0.1.jsonld", + { + "@language": "und" + } + ], + "actor": "https://p.helene.moe/users/helene", + "attachment": [], + "attributedTo": "https://p.helene.moe/users/helene", + "cc": [ + "https://p.helene.moe/users/helene/followers" + ], + "context": "https://p.helene.moe/contexts/cc324643-5583-4c3f-91d2-c6ed37db159d", + "conversation": "https://p.helene.moe/contexts/cc324643-5583-4c3f-91d2-c6ed37db159d", + "directMessage": false, + "id": "https://p.helene.moe/activities/5f80db86-a9bb-4883-9845-fbdbd1478f3a", + "object": { + "actor": "https://p.helene.moe/users/helene", + "attachment": [], + "attributedTo": "https://p.helene.moe/users/helene", + "cc": [ + "https://p.helene.moe/users/helene/followers" + ], + "content": "@mametsuko meow", + "context": "https://p.helene.moe/contexts/cc324643-5583-4c3f-91d2-c6ed37db159d", + "conversation": "https://p.helene.moe/contexts/cc324643-5583-4c3f-91d2-c6ed37db159d", + "id": "https://p.helene.moe/objects/fd5910ac-d9dc-412e-8d1d-914b203296c4", + "inReplyTo": "https://mk.absturztau.be/notes/93e7nm8wqg", + "published": "2022-08-02T13:46:58.403996Z", + "sensitive": null, + "source": "@mametsuko@mk.absturztau.be meow", + "summary": "", + "tag": [ + { + "href": "https://mk.absturztau.be/users/8ozbzjs3o8", + "name": "@mametsuko@mk.absturztau.be", + "type": "Mention" + } + ], + "to": [ + "https://mk.absturztau.be/users/8ozbzjs3o8", + "https://www.w3.org/ns/activitystreams#Public" + ], + "type": "Note" + }, + "published": "2022-08-02T13:46:58.403883Z", + "tag": [ + { + "href": "https://mk.absturztau.be/users/8ozbzjs3o8", + "name": "@mametsuko@mk.absturztau.be", + "type": "Mention" + } + ], + "to": [ + "https://mk.absturztau.be/users/8ozbzjs3o8", + "https://www.w3.org/ns/activitystreams#Public" + ], + "type": "Create" +} \ No newline at end of file diff --git a/test/fixtures/tesla_mock/helene@p.helene.moe.json b/test/fixtures/tesla_mock/helene@p.helene.moe.json new file mode 100644 index 0000000000..d7444817fd --- /dev/null +++ b/test/fixtures/tesla_mock/helene@p.helene.moe.json @@ -0,0 +1,50 @@ +{ + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://p.helene.moe/schemas/litepub-0.1.jsonld", + { + "@language": "und" + } + ], + "alsoKnownAs": [], + "attachment": [ + { + "name": "Timezone", + "type": "PropertyValue", + "value": "UTC+2 (Paris/Berlin)" + } + ], + "capabilities": { + "acceptsChatMessages": true + }, + "discoverable": true, + "endpoints": { + "oauthAuthorizationEndpoint": "https://p.helene.moe/oauth/authorize", + "oauthRegistrationEndpoint": "https://p.helene.moe/api/v1/apps", + "oauthTokenEndpoint": "https://p.helene.moe/oauth/token", + "sharedInbox": "https://p.helene.moe/inbox", + "uploadMedia": "https://p.helene.moe/api/ap/upload_media" + }, + "featured": "https://p.helene.moe/users/helene/collections/featured", + "followers": "https://p.helene.moe/users/helene/followers", + "following": "https://p.helene.moe/users/helene/following", + "icon": { + "type": "Image", + "url": "https://p.helene.moe/media/9a39209daa5a66b7ebb0547b08bf8360aa9d8d65a4ffba2603c6ffbe6aecb432.jpg" + }, + "id": "https://p.helene.moe/users/helene", + "inbox": "https://p.helene.moe/users/helene/inbox", + "manuallyApprovesFollowers": false, + "name": "Hélène", + "outbox": "https://p.helene.moe/users/helene/outbox", + "preferredUsername": "helene", + "publicKey": { + "id": "https://p.helene.moe/users/helene#main-key", + "owner": "https://p.helene.moe/users/helene", + "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtoSBPU/VS2Kx3f6ap3zv\nZVacJsgUfaoFb3c2ii/FRh9RmRVlarq8sJXcjsQt1e0oxWaWJaIDDwyKZPt6hXae\nrY/AiGGeNu+NA+BtY7l7+9Yu67HUyT62+1qAwYHKBXX3fLOPs/YmQI0Tt0c4wKAG\nKEkiYsRizghgpzUC6jqdKV71DJkUZ8yhckCGb2fLko1ajbWEssdaP51aLsyRMyC2\nuzeWrxtD4O/HG0ea4S6y5X6hnsAHIK4Y3nnyIQ6pn4tOsl3HgqkjXE9MmZSvMCFx\nBq89TfZrVXNa2gSZdZLdbbJstzEScQWNt1p6tA6rM+e4JXYGr+rMdF3G+jV7afI2\nFQIDAQAB\n-----END PUBLIC KEY-----\n\n" + }, + "summary": "I can speak: Français, English, Deutsch (nicht sehr gut), 日本語 (not very well)", + "tag": [], + "type": "Person", + "url": "https://p.helene.moe/users/helene" +} \ No newline at end of file diff --git a/test/fixtures/tesla_mock/mametsuko@mk.absturztau.be.json b/test/fixtures/tesla_mock/mametsuko@mk.absturztau.be.json new file mode 100644 index 0000000000..d8c13f7752 --- /dev/null +++ b/test/fixtures/tesla_mock/mametsuko@mk.absturztau.be.json @@ -0,0 +1,65 @@ +{ + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + { + "manuallyApprovesFollowers": "as:manuallyApprovesFollowers", + "sensitive": "as:sensitive", + "Hashtag": "as:Hashtag", + "quoteUrl": "as:quoteUrl", + "toot": "http://joinmastodon.org/ns#", + "Emoji": "toot:Emoji", + "featured": "toot:featured", + "discoverable": "toot:discoverable", + "schema": "http://schema.org#", + "PropertyValue": "schema:PropertyValue", + "value": "schema:value", + "misskey": "https://misskey-hub.net/ns#", + "_misskey_content": "misskey:_misskey_content", + "_misskey_quote": "misskey:_misskey_quote", + "_misskey_reaction": "misskey:_misskey_reaction", + "_misskey_votes": "misskey:_misskey_votes", + "_misskey_talk": "misskey:_misskey_talk", + "isCat": "misskey:isCat", + "vcard": "http://www.w3.org/2006/vcard/ns#" + } + ], + "type": "Person", + "id": "https://mk.absturztau.be/users/8ozbzjs3o8", + "inbox": "https://mk.absturztau.be/users/8ozbzjs3o8/inbox", + "outbox": "https://mk.absturztau.be/users/8ozbzjs3o8/outbox", + "followers": "https://mk.absturztau.be/users/8ozbzjs3o8/followers", + "following": "https://mk.absturztau.be/users/8ozbzjs3o8/following", + "featured": "https://mk.absturztau.be/users/8ozbzjs3o8/collections/featured", + "sharedInbox": "https://mk.absturztau.be/inbox", + "endpoints": { + "sharedInbox": "https://mk.absturztau.be/inbox" + }, + "url": "https://mk.absturztau.be/@mametsuko", + "preferredUsername": "mametsuko", + "name": "mametschko", + "summary": "

nya, ich bin eine Brotperson

", + "icon": { + "type": "Image", + "url": "https://mk.absturztau.be/files/webpublic-3b5594f4-fa52-4548-b4e3-c379ae2143ed", + "sensitive": false, + "name": null + }, + "image": { + "type": "Image", + "url": "https://mk.absturztau.be/files/webpublic-0d03b03d-b14b-4916-ac3d-8a137118ec84", + "sensitive": false, + "name": null + }, + "tag": [], + "manuallyApprovesFollowers": true, + "discoverable": false, + "publicKey": { + "id": "https://mk.absturztau.be/users/8ozbzjs3o8#main-key", + "type": "Key", + "owner": "https://mk.absturztau.be/users/8ozbzjs3o8", + "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAuN/S1spBGmh8FXI1Bt16\nXB7Cc0QutBp7UPgmDNHjOfsq0zrF4g3L1UBxvrpU0XX77XPMCd9yPvGwAYURH2mv\ntIcYuE+R90VLDmBu5MTVthcG2D874eCZ2rD2YsEYmN5AjTX7QBIqCck+qDhVWkkM\nEZ6S5Ht6IJ5Of74eKffXElQI/C6QB+9uEDOmPk0jCzgI5gw7xvJqFj/DIF4kUUAu\nA89JqaFZzZlkrSrj4cr48bLN/YOmpdaHu0BKHaDSHct4+MqlixqovgdB6RboCEDw\ne4Aeav7+Q0Y9oGIvuggg0Q+nCubnVNnaPyzd817tpPVzyZmTts+DKyDuv90SX3nR\nsPaNa5Ty60eqplUk4b7X1gSvuzBJUFBxTVV84WnjwoeoydaS6rSyjCDPGLBjaByc\nFyWMMEb/zlQyhLZfBlvT7k96wRSsMszh2hDALWmgYIhq/jNwINvALJ1GKLNHHKZ4\nyz2LnxVpRm2rWrZzbvtcnSQOt3LaPSZn8Wgwv4buyHF02iuVuIamZVtKexsE1Ixl\nIi9qa3AKEc5gOzYXhRhvHaruzoCehUbb/UHC5c8Tto8L5G1xYzjLP3qj3PT9w/wM\n+k1Ra/4JhuAnVFROOoOmx9rIELLHH7juY2nhM7plGhyt1M5gysgqEloij8QzyQU2\nZK1YlAERG2XFO6br8omhcmECAwEAAQ==\n-----END PUBLIC KEY-----\n" + }, + "isCat": true, + "vcard:Address": "Vienna, Austria" +} \ No newline at end of file diff --git a/test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg.json b/test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg.json new file mode 100644 index 0000000000..1b931a9a49 --- /dev/null +++ b/test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg.json @@ -0,0 +1,44 @@ +{ + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + { + "manuallyApprovesFollowers": "as:manuallyApprovesFollowers", + "sensitive": "as:sensitive", + "Hashtag": "as:Hashtag", + "quoteUrl": "as:quoteUrl", + "toot": "http://joinmastodon.org/ns#", + "Emoji": "toot:Emoji", + "featured": "toot:featured", + "discoverable": "toot:discoverable", + "schema": "http://schema.org#", + "PropertyValue": "schema:PropertyValue", + "value": "schema:value", + "misskey": "https://misskey-hub.net/ns#", + "_misskey_content": "misskey:_misskey_content", + "_misskey_quote": "misskey:_misskey_quote", + "_misskey_reaction": "misskey:_misskey_reaction", + "_misskey_votes": "misskey:_misskey_votes", + "_misskey_talk": "misskey:_misskey_talk", + "isCat": "misskey:isCat", + "vcard": "http://www.w3.org/2006/vcard/ns#" + } + ], + "id": "https://mk.absturztau.be/notes/93e7nm8wqg", + "type": "Note", + "attributedTo": "https://mk.absturztau.be/users/8ozbzjs3o8", + "summary": null, + "content": "

meow

", + "_misskey_content": "meow", + "published": "2022-08-01T11:06:49.568Z", + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://mk.absturztau.be/users/8ozbzjs3o8/followers" + ], + "inReplyTo": null, + "attachment": [], + "sensitive": false, + "tag": [] +} \ No newline at end of file diff --git a/test/fixtures/tesla_mock/p.helene.moe-AM7S6vZQmL6pI9TgPY.json b/test/fixtures/tesla_mock/p.helene.moe-AM7S6vZQmL6pI9TgPY.json new file mode 100644 index 0000000000..a1ef5e20b3 --- /dev/null +++ b/test/fixtures/tesla_mock/p.helene.moe-AM7S6vZQmL6pI9TgPY.json @@ -0,0 +1,36 @@ +{ + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://p.helene.moe/schemas/litepub-0.1.jsonld", + { + "@language": "und" + } + ], + "actor": "https://p.helene.moe/users/helene", + "attachment": [], + "attributedTo": "https://p.helene.moe/users/helene", + "cc": [ + "https://p.helene.moe/users/helene/followers" + ], + "content": "@mametsuko meow", + "context": "https://p.helene.moe/contexts/cc324643-5583-4c3f-91d2-c6ed37db159d", + "conversation": "https://p.helene.moe/contexts/cc324643-5583-4c3f-91d2-c6ed37db159d", + "id": "https://p.helene.moe/objects/fd5910ac-d9dc-412e-8d1d-914b203296c4", + "inReplyTo": "https://mk.absturztau.be/notes/93e7nm8wqg", + "published": "2022-08-02T13:46:58.403996Z", + "sensitive": null, + "source": "@mametsuko@mk.absturztau.be meow", + "summary": "", + "tag": [ + { + "href": "https://mk.absturztau.be/users/8ozbzjs3o8", + "name": "@mametsuko@mk.absturztau.be", + "type": "Mention" + } + ], + "to": [ + "https://mk.absturztau.be/users/8ozbzjs3o8", + "https://www.w3.org/ns/activitystreams#Public" + ], + "type": "Note" +} \ No newline at end of file diff --git a/test/pleroma/web/activity_pub/object_validators/create_generic_validator_test.exs b/test/pleroma/web/activity_pub/object_validators/create_generic_validator_test.exs index 0a5b44beb0..e771260c94 100644 --- a/test/pleroma/web/activity_pub/object_validators/create_generic_validator_test.exs +++ b/test/pleroma/web/activity_pub/object_validators/create_generic_validator_test.exs @@ -23,10 +23,10 @@ test "a Create/Note from Roadhouse validates" do {:ok, object_data} = ObjectValidator.cast_and_apply(note_activity["object"]) meta = [object_data: ObjectValidator.stringify_keys(object_data)] - %{valid?: true} = CreateGenericValidator.cast_and_validate(note_activity, meta) + assert %{valid?: true} = CreateGenericValidator.cast_and_validate(note_activity, meta) end - test "a Create/Note with mismatched context is invalid" do + test "a Create/Note with mismatched context uses the Note's context" do user = insert(:user) note = %{ @@ -54,6 +54,9 @@ test "a Create/Note with mismatched context is invalid" do {:ok, object_data} = ObjectValidator.cast_and_apply(note_activity["object"]) meta = [object_data: ObjectValidator.stringify_keys(object_data)] - %{valid?: false} = CreateGenericValidator.cast_and_validate(note_activity, meta) + validated = CreateGenericValidator.cast_and_validate(note_activity, meta) + + assert validated.valid? + assert {:context, note["context"]} in validated.changes end end diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index b254e5591a..a079728953 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier_test.exs @@ -108,15 +108,20 @@ test "it accepts Move activities" do assert activity.data["type"] == "Move" end - test "a reply with mismatched context is rejected" do - insert(:user, ap_id: "https://macgirvin.com/channel/mike") + test "it fixes both the Create and object contexts in a reply" do + insert(:user, ap_id: "https://mk.absturztau.be/users/8ozbzjs3o8") + insert(:user, ap_id: "https://p.helene.moe/users/helene") - note_activity = - "test/fixtures/roadhouse-create-activity.json" + create_activity = + "test/fixtures/create-pleroma-reply-to-misskey-thread.json" |> File.read!() |> Jason.decode!() - assert {:error, _} = Transmogrifier.handle_incoming(note_activity) + assert {:ok, %Activity{} = activity} = Transmogrifier.handle_incoming(create_activity) + + object = Object.normalize(activity, fetch: false) + + assert activity.data["context"] == object.data["context"] end end diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index eb844e469e..c1f0f7af1d 100644 --- a/test/support/http_request_mock.ex +++ b/test/support/http_request_mock.ex @@ -1401,6 +1401,42 @@ def get("https://gleasonator.com/users/macgirvin/collections/featured", _, _, _) }} end + def get("https://mk.absturztau.be/users/8ozbzjs3o8", _, _, _) do + {:ok, + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/tesla_mock/mametsuko@mk.absturztau.be.json"), + headers: activitypub_object_headers() + }} + end + + def get("https://p.helene.moe/users/helene", _, _, _) do + {:ok, + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/tesla_mock/helene@p.helene.moe.json"), + headers: activitypub_object_headers() + }} + end + + def get("https://mk.absturztau.be/notes/93e7nm8wqg", _, _, _) do + {:ok, + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg.json"), + headers: activitypub_object_headers() + }} + end + + def get("https://p.helene.moe/objects/fd5910ac-d9dc-412e-8d1d-914b203296c4", _, _, _) do + {:ok, + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/tesla_mock/p.helene.moe-AM7S6vZQmL6pI9TgPY.json"), + headers: activitypub_object_headers() + }} + end + def get(url, query, body, headers) do {:error, "Mock response not implemented for GET #{inspect(url)}, #{query}, #{inspect(body)}, #{inspect(headers)}"} From bb02ee99f58e378e33162211f41fe5979d5da8ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Wed, 10 Aug 2022 04:21:28 +0200 Subject: [PATCH 07/18] CommonFixes: more predictable context generation `context` fields for objects and activities can now be generated based on the object/activity `inReplyTo` field or its ActivityPub ID, as a fallback method in cases where `context` fields are missing for incoming activities and objects. --- .../object_validators/common_fixes.ex | 5 ++- .../mk.absturztau.be-93e7nm8wqg-activity.json | 1 + .../transmogrifier/note_handling_test.exs | 38 +++++++++++++++++++ test/support/http_request_mock.ex | 17 +++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg-activity.json diff --git a/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex b/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex index c7e292bec3..add46d561d 100644 --- a/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex +++ b/lib/pleroma/web/activity_pub/object_validators/common_fixes.ex @@ -22,7 +22,10 @@ def cast_and_filter_recipients(message, field, follower_collection, field_fallba end def fix_object_defaults(data) do - context = Utils.maybe_create_context(data["context"] || data["conversation"]) + context = + Utils.maybe_create_context( + data["context"] || data["conversation"] || data["inReplyTo"] || data["id"] + ) %User{follower_address: follower_collection} = User.get_cached_by_ap_id(data["attributedTo"]) diff --git a/test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg-activity.json b/test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg-activity.json new file mode 100644 index 0000000000..b45ab78e4a --- /dev/null +++ b/test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg-activity.json @@ -0,0 +1 @@ +{"@context":["https://www.w3.org/ns/activitystreams","https://w3id.org/security/v1",{"manuallyApprovesFollowers":"as:manuallyApprovesFollowers","sensitive":"as:sensitive","Hashtag":"as:Hashtag","quoteUrl":"as:quoteUrl","toot":"http://joinmastodon.org/ns#","Emoji":"toot:Emoji","featured":"toot:featured","discoverable":"toot:discoverable","schema":"http://schema.org#","PropertyValue":"schema:PropertyValue","value":"schema:value","misskey":"https://misskey-hub.net/ns#","_misskey_content":"misskey:_misskey_content","_misskey_quote":"misskey:_misskey_quote","_misskey_reaction":"misskey:_misskey_reaction","_misskey_votes":"misskey:_misskey_votes","_misskey_talk":"misskey:_misskey_talk","isCat":"misskey:isCat","vcard":"http://www.w3.org/2006/vcard/ns#"}],"id":"https://mk.absturztau.be/notes/93e7nm8wqg/activity","actor":"https://mk.absturztau.be/users/8ozbzjs3o8","type":"Create","published":"2022-08-01T11:06:49.568Z","object":{"id":"https://mk.absturztau.be/notes/93e7nm8wqg","type":"Note","attributedTo":"https://mk.absturztau.be/users/8ozbzjs3o8","summary":null,"content":"

meow

","_misskey_content":"meow","published":"2022-08-01T11:06:49.568Z","to":["https://www.w3.org/ns/activitystreams#Public"],"cc":["https://mk.absturztau.be/users/8ozbzjs3o8/followers"],"inReplyTo":null,"attachment":[],"sensitive":false,"tag":[]},"to":["https://www.w3.org/ns/activitystreams#Public"],"cc":["https://mk.absturztau.be/users/8ozbzjs3o8/followers"]} \ No newline at end of file diff --git a/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs b/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs index b00fd919b6..7c406fbd05 100644 --- a/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs @@ -707,4 +707,42 @@ test "take_emoji_tags/1" do } ] end + + test "the standalone note uses its own ID when context is missing" do + insert(:user, ap_id: "https://mk.absturztau.be/users/8ozbzjs3o8") + + activity = + "test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg-activity.json" + |> File.read!() + |> Jason.decode!() + + {:ok, %Activity{} = modified} = Transmogrifier.handle_incoming(activity) + object = Object.normalize(modified, fetch: false) + + assert object.data["context"] == object.data["id"] + assert modified.data["context"] == object.data["id"] + end + + test "the reply note uses its parent's ID when context is missing and reply is unreachable" do + insert(:user, ap_id: "https://mk.absturztau.be/users/8ozbzjs3o8") + + activity = + "test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg-activity.json" + |> File.read!() + |> Jason.decode!() + + object = + activity["object"] + |> Map.put("inReplyTo", "https://404.site/object/went-to-buy-milk") + + activity = + activity + |> Map.put("object", object) + + {:ok, %Activity{} = modified} = Transmogrifier.handle_incoming(activity) + object = Object.normalize(modified, fetch: false) + + assert object.data["context"] == object.data["inReplyTo"] + assert modified.data["context"] == object.data["inReplyTo"] + end end diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index c1f0f7af1d..7f6065579f 100644 --- a/test/support/http_request_mock.ex +++ b/test/support/http_request_mock.ex @@ -1084,6 +1084,14 @@ def get("http://404.site" <> _, _, _, _) do }} end + def get("https://404.site" <> _, _, _, _) do + {:ok, + %Tesla.Env{ + status: 404, + body: "" + }} + end + def get( "https://zetsubou.xn--q9jyb4c/.well-known/webfinger?resource=acct:lain@zetsubou.xn--q9jyb4c", _, @@ -1428,6 +1436,15 @@ def get("https://mk.absturztau.be/notes/93e7nm8wqg", _, _, _) do }} end + def get("https://mk.absturztau.be/notes/93e7nm8wqg/activity", _, _, _) do + {:ok, + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/tesla_mock/mk.absturztau.be-93e7nm8wqg-activity.json"), + headers: activitypub_object_headers() + }} + end + def get("https://p.helene.moe/objects/fd5910ac-d9dc-412e-8d1d-914b203296c4", _, _, _) do {:ok, %Tesla.Env{ From 88c1c76d3eca3412d1e02008f1b8d96fe8fe0b96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Mon, 15 Aug 2022 01:15:23 +0200 Subject: [PATCH 08/18] Migrations: delete contexts with BaseMigrator Due to the lengthiness of this task, the migration has been adapted into a BaseMigrator migration, running in the background instead. --- config/config.exs | 2 + config/description.exs | 21 +++ lib/pleroma/application.ex | 3 +- lib/pleroma/data_migration.ex | 1 + .../context_objects_deletion_migrator.ex | 139 ++++++++++++++++++ ..._data_migration_delete_context_objects.exs | 13 +- 6 files changed, 173 insertions(+), 6 deletions(-) create mode 100644 lib/pleroma/migrators/context_objects_deletion_migrator.ex diff --git a/config/config.exs b/config/config.exs index 0fc9598077..eadc255ccd 100644 --- a/config/config.exs +++ b/config/config.exs @@ -673,6 +673,8 @@ config :pleroma, :populate_hashtags_table, fault_rate_allowance: 0.01 +config :pleroma, :delete_context_objects, fault_rate_allowance: 0.01 + config :pleroma, :env, Mix.env() config :http_signatures, diff --git a/config/description.exs b/config/description.exs index c6c6b1b5d2..c28447b371 100644 --- a/config/description.exs +++ b/config/description.exs @@ -495,6 +495,27 @@ } ] }, + %{ + group: :pleroma, + key: :delete_context_objects, + type: :group, + description: "`delete_context_objects` background migration settings", + children: [ + %{ + key: :fault_rate_allowance, + type: :float, + description: + "Max accepted rate of objects that failed in the migration. Any value from 0.0 which tolerates no errors to 1.0 which will enable the feature even if context object deletion failed for all records.", + suggestions: [0.01] + }, + %{ + key: :sleep_interval_ms, + type: :integer, + description: + "Sleep interval between each chunk of processed records in order to decrease the load on the system (defaults to 0 and should be keep default on most instances)." + } + ] + }, %{ group: :pleroma, key: :instance, diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index d808bc732b..c546713caf 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -238,7 +238,8 @@ defp dont_run_in_test(_) do defp background_migrators do [ - Pleroma.Migrators.HashtagsTableMigrator + Pleroma.Migrators.HashtagsTableMigrator, + Pleroma.Migrators.ContextObjectsDeletionMigrator ] end diff --git a/lib/pleroma/data_migration.ex b/lib/pleroma/data_migration.ex index 59d891d8db..8451678fc4 100644 --- a/lib/pleroma/data_migration.ex +++ b/lib/pleroma/data_migration.ex @@ -42,4 +42,5 @@ def get_by_name(name) do end def populate_hashtags_table, do: get_by_name("populate_hashtags_table") + def delete_context_objects, do: get_by_name("delete_context_objects") end diff --git a/lib/pleroma/migrators/context_objects_deletion_migrator.ex b/lib/pleroma/migrators/context_objects_deletion_migrator.ex new file mode 100644 index 0000000000..fb224795a4 --- /dev/null +++ b/lib/pleroma/migrators/context_objects_deletion_migrator.ex @@ -0,0 +1,139 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2022 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Migrators.ContextObjectsDeletionMigrator do + defmodule State do + use Pleroma.Migrators.Support.BaseMigratorState + + @impl Pleroma.Migrators.Support.BaseMigratorState + defdelegate data_migration(), to: Pleroma.DataMigration, as: :delete_context_objects + end + + use Pleroma.Migrators.Support.BaseMigrator + + alias Pleroma.Migrators.Support.BaseMigrator + alias Pleroma.Object + + @doc "This migration removes objects created exclusively for contexts, containing only an `id` field." + + @impl BaseMigrator + def feature_config_path, do: [:features, :delete_context_objects] + + @impl BaseMigrator + def fault_rate_allowance, do: Config.get([:delete_context_objects, :fault_rate_allowance], 0) + + @impl BaseMigrator + def perform do + data_migration_id = data_migration_id() + max_processed_id = get_stat(:max_processed_id, 0) + + Logger.info("Deleting context objects from `objects` (from oid: #{max_processed_id})...") + + query() + |> where([object], object.id > ^max_processed_id) + |> Repo.chunk_stream(100, :batches, timeout: :infinity) + |> Stream.each(fn objects -> + object_ids = Enum.map(objects, & &1.id) + + results = Enum.map(object_ids, &delete_context_object(&1)) + + failed_ids = + results + |> Enum.filter(&(elem(&1, 0) == :error)) + |> Enum.map(&elem(&1, 1)) + + chunk_affected_count = + results + |> Enum.filter(&(elem(&1, 0) == :ok)) + |> length() + + for failed_id <- failed_ids do + _ = + Repo.query( + "INSERT INTO data_migration_failed_ids(data_migration_id, record_id) " <> + "VALUES ($1, $2) ON CONFLICT DO NOTHING;", + [data_migration_id, failed_id] + ) + end + + _ = + Repo.query( + "DELETE FROM data_migration_failed_ids " <> + "WHERE data_migration_id = $1 AND record_id = ANY($2)", + [data_migration_id, object_ids -- failed_ids] + ) + + max_object_id = Enum.at(object_ids, -1) + + put_stat(:max_processed_id, max_object_id) + increment_stat(:iteration_processed_count, length(object_ids)) + increment_stat(:processed_count, length(object_ids)) + increment_stat(:failed_count, length(failed_ids)) + increment_stat(:affected_count, chunk_affected_count) + put_stat(:records_per_second, records_per_second()) + persist_state() + + # A quick and dirty approach to controlling the load this background migration imposes + sleep_interval = Config.get([:delete_context_objects, :sleep_interval_ms], 0) + Process.sleep(sleep_interval) + end) + |> Stream.run() + end + + @impl BaseMigrator + def query do + # Context objects have no activity type, and only one field, `id`. + # Only those context objects are without types. + from( + object in Object, + where: fragment("(?)->'type' IS NULL", object.data), + select: %{ + id: object.id + } + ) + end + + @spec delete_context_object(integer()) :: {:ok | :error, integer()} + defp delete_context_object(id) do + result = + %Object{id: id} + |> Repo.delete() + |> elem(0) + + {result, id} + end + + @impl BaseMigrator + def retry_failed do + data_migration_id = data_migration_id() + + failed_objects_query() + |> Repo.chunk_stream(100, :one) + |> Stream.each(fn object -> + with {res, _} when res != :error <- delete_context_object(object.id) do + _ = + Repo.query( + "DELETE FROM data_migration_failed_ids " <> + "WHERE data_migration_id = $1 AND record_id = $2", + [data_migration_id, object.id] + ) + end + end) + |> Stream.run() + + put_stat(:failed_count, failures_count()) + persist_state() + + force_continue() + end + + defp failed_objects_query do + from(o in Object) + |> join(:inner, [o], dmf in fragment("SELECT * FROM data_migration_failed_ids"), + on: dmf.record_id == o.id + ) + |> where([_o, dmf], dmf.data_migration_id == ^data_migration_id()) + |> order_by([o], asc: o.id) + end +end diff --git a/priv/repo/migrations/20220807125023_data_migration_delete_context_objects.exs b/priv/repo/migrations/20220807125023_data_migration_delete_context_objects.exs index debb474b2b..84365dbe33 100644 --- a/priv/repo/migrations/20220807125023_data_migration_delete_context_objects.exs +++ b/priv/repo/migrations/20220807125023_data_migration_delete_context_objects.exs @@ -3,13 +3,16 @@ defmodule Pleroma.Repo.Migrations.DataMigrationDeleteContextObjects do require Logger - @doc "This migration removes objects created exclusively for contexts, containing only an `id` field." + def up do + dt = NaiveDateTime.utc_now() - def change do - Logger.warn( - "This migration can take a very long time to execute, depending on your database size. Please be patient, Pleroma-tan is doing her best!\n" + execute( + "INSERT INTO data_migrations(name, inserted_at, updated_at) " <> + "VALUES ('delete_context_objects', '#{dt}', '#{dt}') ON CONFLICT DO NOTHING;" ) + end - execute("DELETE FROM objects WHERE (data->>'type') IS NULL;") + def down do + execute("DELETE FROM data_migrations WHERE name = 'delete_context_objects';") end end From f41d970a592568956aa97959f28cb89cadf5f2bc Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Mon, 18 Jul 2022 15:21:27 +0100 Subject: [PATCH 09/18] fix resolution of GTS user keys --- lib/pleroma/signature.ex | 23 +++++++++++++++-------- test/pleroma/signature_test.exs | 5 +++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/signature.ex b/lib/pleroma/signature.ex index dbe6fd209f..ff0c568564 100644 --- a/lib/pleroma/signature.ex +++ b/lib/pleroma/signature.ex @@ -10,17 +10,14 @@ defmodule Pleroma.Signature do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub + @known_suffixes ["/publickey", "/main-key"] + def key_id_to_actor_id(key_id) do uri = - URI.parse(key_id) + key_id + |> URI.parse() |> Map.put(:fragment, nil) - - uri = - if not is_nil(uri.path) and String.ends_with?(uri.path, "/publickey") do - Map.put(uri, :path, String.replace(uri.path, "/publickey", "")) - else - uri - end + |> remove_suffix(@known_suffixes) maybe_ap_id = URI.to_string(uri) @@ -36,6 +33,16 @@ def key_id_to_actor_id(key_id) do end end + defp remove_suffix(uri, [test | rest]) do + if not is_nil(uri.path) and String.ends_with?(uri.path, test) do + Map.put(uri, :path, String.replace(uri.path, test, "")) + else + remove_suffix(uri, rest) + end + end + + defp remove_suffix(uri, []), do: uri + def fetch_public_key(conn) do with %{"keyId" => kid} <- HTTPSignatures.signature_for_conn(conn), {:ok, actor_id} <- key_id_to_actor_id(kid), diff --git a/test/pleroma/signature_test.exs b/test/pleroma/signature_test.exs index 92d05f26cd..b849cbee7f 100644 --- a/test/pleroma/signature_test.exs +++ b/test/pleroma/signature_test.exs @@ -109,6 +109,11 @@ test "it properly deduces the actor id for mastodon and pleroma" do {:ok, "https://example.com/users/1234"} end + test "it deduces the actor id for gotoSocial" do + assert Signature.key_id_to_actor_id("https://example.com/users/1234/main-key") == + {:ok, "https://example.com/users/1234"} + end + test "it calls webfinger for 'acct:' accounts" do with_mock(Pleroma.Web.WebFinger, finger: fn _ -> %{"ap_id" => "https://gensokyo.2hu/users/raymoo"} end From 61254111e59f02118cad15de49d1e0704c07030e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Wed, 17 Aug 2022 03:30:02 +0200 Subject: [PATCH 10/18] HttpSignaturePlug: accept standard (request-target) The (request-target) used by Pleroma is non-standard, but many HTTP signature implementations do it this way due to a misinterpretation of the draft 06 of HTTP signatures: "path" was interpreted as not having the query, though later examples show that it must be the absolute path with the query part of the URL as well. This behavior is kept to make sure most software (Pleroma itself, Mastodon, and probably others) do not break, but Pleroma now accepts signatures for a (request-target) containing the query, as expected by many HTTP signature libraries, and clarified in the draft 11 of HTTP signatures. Additionally, the new draft renamed (request-target) to @request-target. We now support both for incoming requests' signatures. --- lib/pleroma/web/plugs/http_signature_plug.ex | 53 +++++++++++++++++--- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex index d023754a65..4bf3252180 100644 --- a/lib/pleroma/web/plugs/http_signature_plug.ex +++ b/lib/pleroma/web/plugs/http_signature_plug.ex @@ -25,21 +25,58 @@ def call(conn, _opts) do end end + defp validate_signature(conn, request_target) do + # Newer drafts for HTTP signatures now use @request-target instead of the + # old (request-target). We'll now support both for incoming signatures. + conn = + conn + |> put_req_header("(request-target)", request_target) + |> put_req_header("@request-target", request_target) + + HTTPSignatures.validate_conn(conn) + end + + defp validate_signature(conn) do + # This (request-target) is non-standard, but many implementations do it + # this way due to a misinterpretation of + # https://datatracker.ietf.org/doc/html/draft-cavage-http-signatures-06 + # "path" was interpreted as not having the query, though later examples + # show that it must be the absolute path + query. This behavior is kept to + # make sure most software (Pleroma itself, Mastodon, and probably others) + # do not break. + request_target = String.downcase("#{conn.method}") <> " #{conn.request_path}" + + # This is the proper way to build the @request-target, as expected by + # many HTTP signature libraries, clarified in the following draft: + # https://www.ietf.org/archive/id/draft-ietf-httpbis-message-signatures-11.html#section-2.2.6 + # It is the same as before, but containing the query part as well. + proper_target = request_target <> "?#{conn.query_string}" + + cond do + # Normal, non-standard behavior but expected by Pleroma and more. + validate_signature(conn, request_target) -> + true + + # Has query string and the previous one failed: let's try the standard. + conn.query_string != "" -> + validate_signature(conn, proper_target) + + # If there's no query string and signature fails, it's rotten. + true -> + false + end + end + defp maybe_assign_valid_signature(conn) do if has_signature_header?(conn) do - # set (request-target) header to the appropriate value - # we also replace the digest header with the one we computed - request_target = String.downcase("#{conn.method}") <> " #{conn.request_path}" - + # we replace the digest header with the one we computed in DigestPlug conn = - conn - |> put_req_header("(request-target)", request_target) - |> case do + case conn do %{assigns: %{digest: digest}} = conn -> put_req_header(conn, "digest", digest) conn -> conn end - assign(conn, :valid_signature, HTTPSignatures.validate_conn(conn)) + assign(conn, :valid_signature, validate_signature(conn)) else Logger.debug("No signature header!") conn From 4661b56720b4f70eb6996bf975c4d88db9828006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Fri, 19 Aug 2022 02:45:49 +0200 Subject: [PATCH 11/18] ArticleNotePageValidator: fix replies fixing Some software, like GoToSocial, expose replies as ActivityPub Collections, but do not expose any item array directly in the object, causing validation to fail via the ObjectID validator. Now, Pleroma will drop that field in this situation too. --- .../article_note_page_validator.ex | 5 ++++- .../article_note_page_validator_test.exs | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex index 57c8d1dc00..4243e0fbf6 100644 --- a/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex @@ -60,7 +60,10 @@ defp fix_replies(%{"replies" => %{"first" => %{"items" => replies}}} = data) defp fix_replies(%{"replies" => %{"items" => replies}} = data) when is_list(replies), do: Map.put(data, "replies", replies) - defp fix_replies(%{"replies" => replies} = data) when is_bitstring(replies), + # TODO: Pleroma does not have any support for Collections at the moment. + # If the `replies` field is not something the ObjectID validator can handle, + # the activity/object would be rejected, which is bad behavior. + defp fix_replies(%{"replies" => replies} = data) when not is_list(replies), do: Map.drop(data, ["replies"]) defp fix_replies(data), do: data diff --git a/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs index e59bf67875..2dd1361eaf 100644 --- a/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs +++ b/test/pleroma/web/activity_pub/object_validators/article_note_page_validator_test.exs @@ -54,4 +54,17 @@ test "a note with an attachment should work", _ do %{valid?: true} = ArticleNotePageValidator.cast_and_validate(note) end + + test "a Note without replies/first/items validates" do + insert(:user, ap_id: "https://mastodon.social/users/emelie") + + note = + "test/fixtures/tesla_mock/status.emelie.json" + |> File.read!() + |> Jason.decode!() + |> pop_in(["replies", "first", "items"]) + |> elem(1) + + %{valid?: true} = ArticleNotePageValidator.cast_and_validate(note) + end end From 3afa1903ee202cc0acb4170bc06c491c15875145 Mon Sep 17 00:00:00 2001 From: Tusooa Zhu Date: Sat, 27 Aug 2022 17:51:41 -0400 Subject: [PATCH 12/18] Do not stream out Create of ChatMessage --- lib/pleroma/activity/ir/topics.ex | 8 ++++++++ test/pleroma/activity/ir/topics_test.exs | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/lib/pleroma/activity/ir/topics.ex b/lib/pleroma/activity/ir/topics.ex index 56c52e9d17..f058cc0c96 100644 --- a/lib/pleroma/activity/ir/topics.ex +++ b/lib/pleroma/activity/ir/topics.ex @@ -13,6 +13,14 @@ def get_activity_topics(activity) do |> List.flatten() end + defp generate_topics(%{data: %{"type" => "ChatMessage"}}, %{data: %{"type" => "Delete"}}) do + ["user", "user:pleroma_chat"] + end + + defp generate_topics(%{data: %{"type" => "ChatMessage"}}, %{data: %{"type" => "Create"}}) do + [] + end + defp generate_topics(%{data: %{"type" => "Answer"}}, _) do [] end diff --git a/test/pleroma/activity/ir/topics_test.exs b/test/pleroma/activity/ir/topics_test.exs index 311f85dea4..0abda60b08 100644 --- a/test/pleroma/activity/ir/topics_test.exs +++ b/test/pleroma/activity/ir/topics_test.exs @@ -13,6 +13,29 @@ defmodule Pleroma.Activity.Ir.TopicsTest do import Mock + describe "chat message" do + test "Create produces no topics" do + activity = %Activity{ + object: %Object{data: %{"type" => "ChatMessage"}}, + data: %{"type" => "Create"} + } + + assert [] == Topics.get_activity_topics(activity) + end + + test "Delete produces user and user:pleroma_chat" do + activity = %Activity{ + object: %Object{data: %{"type" => "ChatMessage"}}, + data: %{"type" => "Delete"} + } + + topics = Topics.get_activity_topics(activity) + assert [_, _] = topics + assert "user" in topics + assert "user:pleroma_chat" in topics + end + end + describe "poll answer" do test "produce no topics" do activity = %Activity{object: %Object{data: %{"type" => "Answer"}}} From f9b86c3c22c10d54a721cfe632f9c7455a8b2f9c Mon Sep 17 00:00:00 2001 From: Tusooa Zhu Date: Sat, 27 Aug 2022 19:34:56 -0400 Subject: [PATCH 13/18] Make local-only posts stream in local timeline --- lib/pleroma/activity/ir/topics.ex | 17 +++++++- test/pleroma/activity/ir/topics_test.exs | 53 ++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/activity/ir/topics.ex b/lib/pleroma/activity/ir/topics.ex index f058cc0c96..fa4350797d 100644 --- a/lib/pleroma/activity/ir/topics.ex +++ b/lib/pleroma/activity/ir/topics.ex @@ -39,6 +39,10 @@ defp visibility_tags(object, activity) do end |> item_creation_tags(object, activity) + "local" -> + ["public:local"] + |> item_creation_tags(object, activity) + "direct" -> ["direct"] @@ -71,7 +75,18 @@ defp remote_topics(_), do: [] defp attachment_topics(%{data: %{"attachment" => []}}, _act), do: [] - defp attachment_topics(_object, %{local: true}), do: ["public:media", "public:local:media"] + defp attachment_topics(_object, %{local: true} = activity) do + case Visibility.get_visibility(activity) do + "public" -> + ["public:media", "public:local:media"] + + "local" -> + ["public:local:media"] + + _ -> + [] + end + end defp attachment_topics(_object, %{actor: actor}) when is_binary(actor), do: ["public:media", "public:remote:media:" <> URI.parse(actor).host] diff --git a/test/pleroma/activity/ir/topics_test.exs b/test/pleroma/activity/ir/topics_test.exs index 0abda60b08..8317868a5d 100644 --- a/test/pleroma/activity/ir/topics_test.exs +++ b/test/pleroma/activity/ir/topics_test.exs @@ -137,6 +137,36 @@ test "local action doesn't produce public:remote topic", %{activity: activity} d end end + describe "local-public visibility create events" do + setup do + activity = %Activity{ + object: %Object{data: %{"attachment" => []}}, + data: %{"type" => "Create", "to" => [Pleroma.Web.ActivityPub.Utils.as_local_public()]} + } + + {:ok, activity: activity} + end + + test "doesn't produce public topics", %{activity: activity} do + topics = Topics.get_activity_topics(activity) + + refute Enum.member?(topics, "public") + end + + test "produces public:local topics", %{activity: activity} do + topics = Topics.get_activity_topics(activity) + + assert Enum.member?(topics, "public:local") + end + + test "with no attachments doesn't produce public:media topics", %{activity: activity} do + topics = Topics.get_activity_topics(activity) + + refute Enum.member?(topics, "public:media") + refute Enum.member?(topics, "public:local:media") + end + end + describe "public visibility create events with attachments" do setup do activity = %Activity{ @@ -175,6 +205,29 @@ test "non-local action produces public:remote:media topic", %{activity: activity end end + describe "local-public visibility create events with attachments" do + setup do + activity = %Activity{ + object: %Object{data: %{"attachment" => ["foo"]}}, + data: %{"type" => "Create", "to" => [Pleroma.Web.ActivityPub.Utils.as_local_public()]} + } + + {:ok, activity: activity} + end + + test "do not produce public:media topics", %{activity: activity} do + topics = Topics.get_activity_topics(activity) + + refute Enum.member?(topics, "public:media") + end + + test "produces public:local:media topics", %{activity: activity} do + topics = Topics.get_activity_topics(activity) + + assert Enum.member?(topics, "public:local:media") + end + end + describe "non-public visibility" do test "produces direct topic" do activity = %Activity{object: %Object{data: %{"type" => "Note"}}, data: %{"to" => []}} From ffd379456bb9e4f125ce5e2480be4d2819b88147 Mon Sep 17 00:00:00 2001 From: Tusooa Zhu Date: Wed, 31 Aug 2022 15:57:06 -0400 Subject: [PATCH 14/18] Do not stream out Announces to public timelines --- lib/pleroma/activity/ir/topics.ex | 6 +++++- test/pleroma/activity/ir/topics_test.exs | 27 ++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/activity/ir/topics.ex b/lib/pleroma/activity/ir/topics.ex index fa4350797d..b9fcd6693a 100644 --- a/lib/pleroma/activity/ir/topics.ex +++ b/lib/pleroma/activity/ir/topics.ex @@ -29,7 +29,7 @@ defp generate_topics(object, activity) do ["user", "list"] ++ visibility_tags(object, activity) end - defp visibility_tags(object, activity) do + defp visibility_tags(object, %{data: %{"type" => "Create"}} = activity) do case Visibility.get_visibility(activity) do "public" -> if activity.local do @@ -51,6 +51,10 @@ defp visibility_tags(object, activity) do end end + defp visibility_tags(_object, _activity) do + [] + end + defp item_creation_tags(tags, object, %{data: %{"type" => "Create"}} = activity) do tags ++ remote_topics(activity) ++ hashtags_to_topics(object) ++ attachment_topics(object, activity) diff --git a/test/pleroma/activity/ir/topics_test.exs b/test/pleroma/activity/ir/topics_test.exs index 8317868a5d..d299fea633 100644 --- a/test/pleroma/activity/ir/topics_test.exs +++ b/test/pleroma/activity/ir/topics_test.exs @@ -58,7 +58,7 @@ test "always add user and list topics" do setup do activity = %Activity{ object: %Object{data: %{"type" => "Note"}}, - data: %{"to" => [Pleroma.Constants.as_public()]} + data: %{"to" => [Pleroma.Constants.as_public()], "type" => "Create"} } {:ok, activity: activity} @@ -137,6 +137,25 @@ test "local action doesn't produce public:remote topic", %{activity: activity} d end end + describe "public visibility Announces" do + setup do + activity = %Activity{ + object: %Object{data: %{"attachment" => []}}, + data: %{"type" => "Announce", "to" => [Pleroma.Constants.as_public()]} + } + + {:ok, activity: activity} + end + + test "does not generate public topics", %{activity: activity} do + topics = Topics.get_activity_topics(activity) + + refute "public" in topics + refute "public:remote" in topics + refute "public:local" in topics + end + end + describe "local-public visibility create events" do setup do activity = %Activity{ @@ -230,7 +249,11 @@ test "produces public:local:media topics", %{activity: activity} do describe "non-public visibility" do test "produces direct topic" do - activity = %Activity{object: %Object{data: %{"type" => "Note"}}, data: %{"to" => []}} + activity = %Activity{ + object: %Object{data: %{"type" => "Note"}}, + data: %{"to" => [], "type" => "Create"} + } + topics = Topics.get_activity_topics(activity) assert Enum.member?(topics, "direct") From 20a0dd6516e453cdedb5a7a2b7356c529eeacf84 Mon Sep 17 00:00:00 2001 From: Tusooa Zhu Date: Wed, 31 Aug 2022 22:14:54 -0400 Subject: [PATCH 15/18] Exclude Announce instead of restricting to Create in visibility_tags --- lib/pleroma/activity/ir/topics.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/activity/ir/topics.ex b/lib/pleroma/activity/ir/topics.ex index b9fcd6693a..8249cbe271 100644 --- a/lib/pleroma/activity/ir/topics.ex +++ b/lib/pleroma/activity/ir/topics.ex @@ -29,7 +29,7 @@ defp generate_topics(object, activity) do ["user", "list"] ++ visibility_tags(object, activity) end - defp visibility_tags(object, %{data: %{"type" => "Create"}} = activity) do + defp visibility_tags(object, %{data: %{"type" => type}} = activity) when type != "Announce" do case Visibility.get_visibility(activity) do "public" -> if activity.local do From c32e28e1b09021647d6229236a689852410dffe5 Mon Sep 17 00:00:00 2001 From: Tusooa Zhu Date: Thu, 1 Sep 2022 07:33:58 -0400 Subject: [PATCH 16/18] Fix SideEffectsTest --- test/pleroma/web/activity_pub/side_effects_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pleroma/web/activity_pub/side_effects_test.exs b/test/pleroma/web/activity_pub/side_effects_test.exs index 64c4a8c140..2cbb64e8f1 100644 --- a/test/pleroma/web/activity_pub/side_effects_test.exs +++ b/test/pleroma/web/activity_pub/side_effects_test.exs @@ -545,7 +545,7 @@ test "it streams out the announce", %{announce: announce} do {:ok, announce, _} = SideEffects.handle(announce) assert called( - Pleroma.Web.Streamer.stream(["user", "list", "public", "public:local"], announce) + Pleroma.Web.Streamer.stream(["user", "list"], announce) ) assert called(Pleroma.Web.Push.send(:_)) From d19696cf60c6eb98ec751fb5d67f994cc39cd4b6 Mon Sep 17 00:00:00 2001 From: Tusooa Zhu Date: Fri, 2 Sep 2022 22:58:35 -0400 Subject: [PATCH 17/18] Lint --- test/pleroma/web/activity_pub/side_effects_test.exs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/pleroma/web/activity_pub/side_effects_test.exs b/test/pleroma/web/activity_pub/side_effects_test.exs index 2cbb64e8f1..6d6f9b2b6f 100644 --- a/test/pleroma/web/activity_pub/side_effects_test.exs +++ b/test/pleroma/web/activity_pub/side_effects_test.exs @@ -544,9 +544,7 @@ test "it streams out the announce", %{announce: announce} do ]) do {:ok, announce, _} = SideEffects.handle(announce) - assert called( - Pleroma.Web.Streamer.stream(["user", "list"], announce) - ) + assert called(Pleroma.Web.Streamer.stream(["user", "list"], announce)) assert called(Pleroma.Web.Push.send(:_)) end From 4477c6baff6ea3c17ceca5d9113960b5b78d5ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne?= Date: Tue, 23 Aug 2022 14:49:05 +0200 Subject: [PATCH 18/18] Metadata/Utils: use summary as description if set When generating OpenGraph and TwitterCard metadata for a post, the summary field will be used first if it is set to generate the post description. --- lib/pleroma/web/metadata/utils.ex | 15 ++++++-- .../metadata/providers/twitter_card_test.exs | 33 +++++++++++++++++ test/pleroma/web/metadata/utils_test.exs | 36 ++++++++++++++++++- 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/metadata/utils.ex b/lib/pleroma/web/metadata/utils.ex index 8052eaa443..15414a988d 100644 --- a/lib/pleroma/web/metadata/utils.ex +++ b/lib/pleroma/web/metadata/utils.ex @@ -8,8 +8,8 @@ defmodule Pleroma.Web.Metadata.Utils do alias Pleroma.Formatter alias Pleroma.HTML - def scrub_html_and_truncate(%{data: %{"content" => content}} = object) do - content + defp scrub_html_and_truncate_object_field(field, object) do + field # html content comes from DB already encoded, decode first and scrub after |> HtmlEntities.decode() |> String.replace(~r//, " ") @@ -19,6 +19,17 @@ def scrub_html_and_truncate(%{data: %{"content" => content}} = object) do |> Formatter.truncate() end + def scrub_html_and_truncate(%{data: %{"summary" => summary}} = object) + when is_binary(summary) and summary != "" do + summary + |> scrub_html_and_truncate_object_field(object) + end + + def scrub_html_and_truncate(%{data: %{"content" => content}} = object) do + content + |> scrub_html_and_truncate_object_field(object) + end + def scrub_html_and_truncate(content, max_length \\ 200) when is_binary(content) do content |> scrub_html diff --git a/test/pleroma/web/metadata/providers/twitter_card_test.exs b/test/pleroma/web/metadata/providers/twitter_card_test.exs index 392496993d..1a0cea9cec 100644 --- a/test/pleroma/web/metadata/providers/twitter_card_test.exs +++ b/test/pleroma/web/metadata/providers/twitter_card_test.exs @@ -39,6 +39,7 @@ test "it uses summary twittercard if post has no attachment" do "actor" => user.ap_id, "tag" => [], "id" => "https://pleroma.gov/objects/whatever", + "summary" => "", "content" => "pleroma in a nutshell" } }) @@ -54,6 +55,36 @@ test "it uses summary twittercard if post has no attachment" do ] == result end + test "it uses summary as description if post has one" do + user = insert(:user, name: "Jimmy Hendriks", bio: "born 19 March 1994") + {:ok, activity} = CommonAPI.post(user, %{status: "HI"}) + + note = + insert(:note, %{ + data: %{ + "actor" => user.ap_id, + "tag" => [], + "id" => "https://pleroma.gov/objects/whatever", + "summary" => "Public service announcement on caffeine consumption", + "content" => "cofe" + } + }) + + result = TwitterCard.build_tags(%{object: note, user: user, activity_id: activity.id}) + + assert [ + {:meta, [property: "twitter:title", content: Utils.user_name_string(user)], []}, + {:meta, + [ + property: "twitter:description", + content: "Public service announcement on caffeine consumption" + ], []}, + {:meta, [property: "twitter:image", content: "http://localhost:4001/images/avi.png"], + []}, + {:meta, [property: "twitter:card", content: "summary"], []} + ] == result + end + test "it renders avatar not attachment if post is nsfw and unfurl_nsfw is disabled" do clear_config([Pleroma.Web.Metadata, :unfurl_nsfw], false) user = insert(:user, name: "Jimmy Hendriks", bio: "born 19 March 1994") @@ -65,6 +96,7 @@ test "it renders avatar not attachment if post is nsfw and unfurl_nsfw is disabl "actor" => user.ap_id, "tag" => [], "id" => "https://pleroma.gov/objects/whatever", + "summary" => "", "content" => "pleroma in a nutshell", "sensitive" => true, "attachment" => [ @@ -109,6 +141,7 @@ test "it renders supported types of attachments and skips unknown types" do "actor" => user.ap_id, "tag" => [], "id" => "https://pleroma.gov/objects/whatever", + "summary" => "", "content" => "pleroma in a nutshell", "attachment" => [ %{ diff --git a/test/pleroma/web/metadata/utils_test.exs b/test/pleroma/web/metadata/utils_test.exs index 5f2f4a056f..85ef6033a7 100644 --- a/test/pleroma/web/metadata/utils_test.exs +++ b/test/pleroma/web/metadata/utils_test.exs @@ -8,7 +8,7 @@ defmodule Pleroma.Web.Metadata.UtilsTest do alias Pleroma.Web.Metadata.Utils describe "scrub_html_and_truncate/1" do - test "it returns text without encode HTML" do + test "it returns content text without encode HTML if summary is nil" do user = insert(:user) note = @@ -16,6 +16,7 @@ test "it returns text without encode HTML" do data: %{ "actor" => user.ap_id, "id" => "https://pleroma.gov/objects/whatever", + "summary" => nil, "content" => "Pleroma's really cool!" } }) @@ -23,6 +24,39 @@ test "it returns text without encode HTML" do assert Utils.scrub_html_and_truncate(note) == "Pleroma's really cool!" end + test "it returns context text without encode HTML if summary is empty" do + user = insert(:user) + + note = + insert(:note, %{ + data: %{ + "actor" => user.ap_id, + "id" => "https://pleroma.gov/objects/whatever", + "summary" => "", + "content" => "Pleroma's really cool!" + } + }) + + assert Utils.scrub_html_and_truncate(note) == "Pleroma's really cool!" + end + + test "it returns summary text without encode HTML if summary is filled" do + user = insert(:user) + + note = + insert(:note, %{ + data: %{ + "actor" => user.ap_id, + "id" => "https://pleroma.gov/objects/whatever", + "summary" => "Public service announcement on caffeine consumption", + "content" => "cofe" + } + }) + + assert Utils.scrub_html_and_truncate(note) == + "Public service announcement on caffeine consumption" + end + test "it does not return old content after editing" do user = insert(:user)