Inject history when object is refetched

lib/pleroma/object.ex

@@ -445,4 +445,26 @@ defp history_skeleton 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

lib/pleroma/object/fetcher.ex

@@ -26,8 +26,35 @@ defp touch_changeset(changeset) 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
 

lib/pleroma/web/activity_pub/side_effects.ex

@@ -436,28 +436,6 @@ defp update_content_fields(orig_object_data, updated_object) 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"
@@ -492,7 +470,7 @@ defp handle_update_object(
 
       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

test/pleroma/object/fetcher_test.exs

@@ -269,4 +269,191 @@ test "it can refetch pruned objects" 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