diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index a893f2c1a..670ab8743 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -445,4 +445,26 @@ defmodule Pleroma.Object do "orderedItems" => [] } end + + def maybe_update_history(updated_object, orig_object_data, updated) do + if not updated do + updated_object + else + # Put edit history + # Note that we may have got the edit history by first fetching the object + history = Object.history_for(orig_object_data) + + latest_history_item = + orig_object_data + |> Map.drop(["id", "formerRepresentations"]) + + new_history = + history + |> Map.put("orderedItems", [latest_history_item | history["orderedItems"]]) + |> Map.put("totalItems", history["totalItems"] + 1) + + updated_object + |> Map.put("formerRepresentations", new_history) + end + end end diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index deb3dc711..ce816c1fc 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -26,8 +26,35 @@ defmodule Pleroma.Object.Fetcher do end defp maybe_reinject_internal_fields(%{data: %{} = old_data}, new_data) do + has_history? = fn + %{"formerRepresentations" => %{"orderedItems" => list}} when is_list(list) -> true + _ -> false + end + internal_fields = Map.take(old_data, Pleroma.Constants.object_internal_fields()) + remote_history_exists? = has_history?.(new_data) + + # If the remote history exists, we treat that as the only source of truth. + new_data = + if has_history?.(old_data) and not remote_history_exists? do + Map.put(new_data, "formerRepresentations", old_data["formerRepresentations"]) + else + new_data + end + + # If the remote does not have history information, we need to manage it ourselves + new_data = + if not remote_history_exists? do + changed? = + Pleroma.Constants.status_updatable_fields() + |> Enum.any?(fn field -> Map.get(old_data, field) != Map.get(new_data, field) end) + + new_data |> Object.maybe_update_history(old_data, changed?) + else + new_data + end + Map.merge(new_data, internal_fields) end diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 49054c320..52a343de7 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -431,28 +431,6 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do ) end - defp maybe_update_history(updated_object, orig_object_data, updated) do - if not updated do - updated_object - else - # Put edit history - # Note that we may have got the edit history by first fetching the object - history = Object.history_for(orig_object_data) - - latest_history_item = - orig_object_data - |> Map.drop(["id", "formerRepresentations"]) - - new_history = - history - |> Map.put("orderedItems", [latest_history_item | history["orderedItems"]]) - |> Map.put("totalItems", history["totalItems"] + 1) - - updated_object - |> Map.put("formerRepresentations", new_history) - end - end - defp maybe_update_poll(to_be_updated, updated_object) do choice_key = fn data -> if Map.has_key?(data, "anyOf"), do: "anyOf", else: "oneOf" @@ -487,7 +465,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do updated_object_data = updated_object_data - |> maybe_update_history(orig_object_data, updated) + |> Object.maybe_update_history(orig_object_data, updated) |> maybe_update_poll(updated_object) orig_object diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 98130f434..5a79e064f 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -269,4 +269,191 @@ defmodule Pleroma.Object.FetcherTest do refute called(Pleroma.Signature.sign(:_, :_)) end end + + describe "refetching" do + setup do + object1 = %{ + "id" => "https://mastodon.social/1", + "actor" => "https://mastodon.social/users/emelie", + "attributedTo" => "https://mastodon.social/users/emelie", + "type" => "Note", + "content" => "test 1", + "bcc" => [], + "bto" => [], + "cc" => [], + "to" => [], + "summary" => "" + } + + object2 = %{ + "id" => "https://mastodon.social/2", + "actor" => "https://mastodon.social/users/emelie", + "attributedTo" => "https://mastodon.social/users/emelie", + "type" => "Note", + "content" => "test 2", + "bcc" => [], + "bto" => [], + "cc" => [], + "to" => [], + "summary" => "", + "formerRepresentations" => %{ + "type" => "OrderedCollection", + "orderedItems" => [ + %{ + "type" => "Note", + "content" => "orig 2", + "actor" => "https://mastodon.social/users/emelie", + "attributedTo" => "https://mastodon.social/users/emelie", + "bcc" => [], + "bto" => [], + "cc" => [], + "to" => [], + "summary" => "" + } + ], + "totalItems" => 1 + } + } + + mock(fn + %{ + method: :get, + url: "https://mastodon.social/1" + } -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: Jason.encode!(object1) + } + + %{ + method: :get, + url: "https://mastodon.social/2" + } -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: Jason.encode!(object2) + } + + %{ + method: :get, + url: "https://mastodon.social/users/emelie/collections/featured" + } -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: + Jason.encode!(%{ + "id" => "https://mastodon.social/users/emelie/collections/featured", + "type" => "OrderedCollection", + "actor" => "https://mastodon.social/users/emelie", + "attributedTo" => "https://mastodon.social/users/emelie", + "orderedItems" => [], + "totalItems" => 0 + }) + } + + env -> + apply(HttpRequestMock, :request, [env]) + end) + + %{object1: object1, object2: object2} + end + + test "it keeps formerRepresentations if remote does not have this attr", %{object1: object1} do + full_object1 = + object1 + |> Map.merge(%{ + "formerRepresentations" => %{ + "type" => "OrderedCollection", + "orderedItems" => [ + %{ + "type" => "Note", + "content" => "orig 2", + "actor" => "https://mastodon.social/users/emelie", + "attributedTo" => "https://mastodon.social/users/emelie", + "bcc" => [], + "bto" => [], + "cc" => [], + "to" => [], + "summary" => "" + } + ], + "totalItems" => 1 + } + }) + + {:ok, o} = Object.create(full_object1) + + assert {:ok, refetched} = Fetcher.refetch_object(o) + + assert %{"formerRepresentations" => %{"orderedItems" => [%{"content" => "orig 2"}]}} = + refetched.data + end + + test "it uses formerRepresentations from remote if possible", %{object2: object2} do + {:ok, o} = Object.create(object2) + + assert {:ok, refetched} = Fetcher.refetch_object(o) + + assert %{"formerRepresentations" => %{"orderedItems" => [%{"content" => "orig 2"}]}} = + refetched.data + end + + test "it replaces formerRepresentations with the one from remote", %{object2: object2} do + full_object2 = + object2 + |> Map.merge(%{ + "content" => "mew mew #def", + "formerRepresentations" => %{ + "type" => "OrderedCollection", + "orderedItems" => [ + %{"type" => "Note", "content" => "mew mew 2"} + ], + "totalItems" => 1 + } + }) + + {:ok, o} = Object.create(full_object2) + + assert {:ok, refetched} = Fetcher.refetch_object(o) + + assert %{ + "content" => "test 2", + "formerRepresentations" => %{"orderedItems" => [%{"content" => "orig 2"}]} + } = refetched.data + end + + test "it adds to formerRepresentations if the remote does not have one and the object has changed", + %{object1: object1} do + full_object1 = + object1 + |> Map.merge(%{ + "content" => "mew mew #def", + "formerRepresentations" => %{ + "type" => "OrderedCollection", + "orderedItems" => [ + %{"type" => "Note", "content" => "mew mew 1"} + ], + "totalItems" => 1 + } + }) + + {:ok, o} = Object.create(full_object1) + + assert {:ok, refetched} = Fetcher.refetch_object(o) + + assert %{ + "content" => "test 1", + "formerRepresentations" => %{ + "orderedItems" => [ + %{"content" => "mew mew #def"}, + %{"content" => "mew mew 1"} + ], + "totalItems" => 2 + } + } = refetched.data + end + end end