added tests

lib/pleroma/object/fetcher.ex

@@ -13,6 +13,7 @@ defmodule Pleroma.Object.Fetcher do
 
   require Logger
 
+  @spec reinject_object(map()) :: {:ok, Object.t()} | {:error, any()}
   defp reinject_object(data) do
     Logger.debug("Reinjecting object #{data["id"]}")
 
@@ -29,50 +30,54 @@ defp reinject_object(data) do
   # TODO:
   # This will create a Create activity, which we need internally at the moment.
   def fetch_object_from_id(id, options \\ []) do
-    if object = Object.get_cached_by_ap_id(id) do
+    with {:fetch_object, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)},
+         {:fetch, {:ok, data}} <- {:fetch, fetch_and_contain_remote_object_from_id(id)},
+         {:normalize, nil} <- {:normalize, Object.normalize(data, false)},
+         params <- prepare_activity_params(data),
+         {:containment, :ok} <- {:containment, Containment.contain_origin(id, params)},
+         {:ok, activity} <- Transmogrifier.handle_incoming(params, options),
+         {:object, _data, %Object{} = object} <-
+           {:object, data, Object.normalize(activity, false)} do
       {:ok, object}
     else
-      Logger.info("Fetching #{id} via AP")
+      {:containment, _} ->
+        {:error, "Object containment failed."}
 
-      with {:fetch, {:ok, data}} <- {:fetch, fetch_and_contain_remote_object_from_id(id)},
+      {:error, {:reject, nil}} ->
-           {:normalize, nil} <- {:normalize, Object.normalize(data, false)},
+        {:reject, nil}
-           params <- %{
+
-             "type" => "Create",
+      {:object, data, nil} ->
-             "to" => data["to"],
+        reinject_object(data)
-             "cc" => data["cc"],
+
-             # Should we seriously keep this attributedTo thing?
+      {:normalize, object = %Object{}} ->
-             "actor" => data["actor"] || data["attributedTo"],
-             "object" => data
-           },
-           {:containment, :ok} <- {:containment, Containment.contain_origin(id, params)},
-           {:ok, activity} <- Transmogrifier.handle_incoming(params, options),
-           {:object, _data, %Object{} = object} <-
-             {:object, data, Object.normalize(activity, false)} do
         {:ok, object}
-      else
-        {:containment, _} ->
-          {:error, "Object containment failed."}
 
-        {:error, {:reject, nil}} ->
+      {:fetch_object, %Object{} = object} ->
-          {:reject, nil}
+        {:ok, object}
 
-        {:object, data, nil} ->
+      _e ->
-          reinject_object(data)
+        # Only fallback when receiving a fetch/normalization error with ActivityPub
+        Logger.info("Couldn't get object via AP, trying out OStatus fetching...")
 
-        {:normalize, object = %Object{}} ->
+        # FIXME: OStatus Object Containment?
-          {:ok, object}
+        case OStatus.fetch_activity_from_url(id) do
-
+          {:ok, [activity | _]} -> {:ok, Object.normalize(activity, false)}
-        _e ->
+          e -> e
-          # Only fallback when receiving a fetch/normalization error with ActivityPub
+        end
-          Logger.info("Couldn't get object via AP, trying out OStatus fetching...")
-
-          # FIXME: OStatus Object Containment?
-          case OStatus.fetch_activity_from_url(id) do
-            {:ok, [activity | _]} -> {:ok, Object.normalize(activity, false)}
-            e -> e
-          end
-      end
     end
+
+    # end
+  end
+
+  defp prepare_activity_params(data) do
+    %{
+      "type" => "Create",
+      "to" => data["to"],
+      "cc" => data["cc"],
+      # Should we seriously keep this attributedTo thing?
+      "actor" => data["actor"] || data["attributedTo"],
+      "object" => data
+    }
   end
 
   def fetch_object_from_id!(id, options \\ []) do

lib/pleroma/web/activity_pub/transmogrifier.ex

@@ -204,7 +204,6 @@ def fix_attachments(%{"attachment" => attachment} = object) when is_list(attachm
       Enum.map(attachment, fn data ->
         media_type = data["mediaType"] || data["mimeType"]
         href = data["url"] || data["href"]
-
         url = [%{"type" => "Link", "mediaType" => media_type, "href" => href}]
 
         data
@@ -216,7 +215,9 @@ def fix_attachments(%{"attachment" => attachment} = object) when is_list(attachm
   end
 
   def fix_attachments(%{"attachment" => attachment} = object) when is_map(attachment) do
-    fix_attachments(Map.put(object, "attachment", [attachment]))
+    object
+    |> Map.put("attachment", [attachment])
+    |> fix_attachments()
   end
 
   def fix_attachments(object), do: object
@@ -725,10 +726,9 @@ def handle_incoming(_, _), do: :error
 
   @spec get_obj_helper(String.t(), Keyword.t()) :: {:ok, Object.t()} | nil
   def get_obj_helper(id, options \\ []) do
-    if object = Object.normalize(id, true, options) do
+    case Object.normalize(id, true, options) do
-      {:ok, object}
+      %Object{} = object -> {:ok, object}
-    else
+      _ -> nil
-      nil
     end
   end
 

test/web/activity_pub/transmogrifier_test.exs

@@ -1613,4 +1613,78 @@ test "retunrs not modified object" do
       assert Transmogrifier.fix_url(%{"type" => "Text"}) == %{"type" => "Text"}
     end
   end
+
+  describe "get_obj_helper/2" do
+    test "returns nil when cannot normalize object" do
+      refute Transmogrifier.get_obj_helper("test-obj-id")
+    end
+
+    test "returns {:ok, %Object{}} for success case" do
+      assert {:ok, %Object{}} =
+               Transmogrifier.get_obj_helper("https://shitposter.club/notice/2827873")
+    end
+  end
+
+  describe "fix_attachments/1" do
+    test "returns not modified object" do
+      data = Poison.decode!(File.read!("test/fixtures/mastodon-post-activity.json"))
+      assert Transmogrifier.fix_attachments(data) == data
+    end
+
+    test "returns modified object when attachment is map" do
+      assert Transmogrifier.fix_attachments(%{
+               "attachment" => %{
+                 "mediaType" => "video/mp4",
+                 "url" => "https://peertube.moe/stat-480.mp4"
+               }
+             }) == %{
+               "attachment" => [
+                 %{
+                   "mediaType" => "video/mp4",
+                   "url" => [
+                     %{
+                       "href" => "https://peertube.moe/stat-480.mp4",
+                       "mediaType" => "video/mp4",
+                       "type" => "Link"
+                     }
+                   ]
+                 }
+               ]
+             }
+    end
+
+    test "returns modified object when attachment is list" do
+      assert Transmogrifier.fix_attachments(%{
+               "attachment" => [
+                 %{"mediaType" => "video/mp4", "url" => "https://pe.er/stat-480.mp4"},
+                 %{"mimeType" => "video/mp4", "href" => "https://pe.er/stat-480.mp4"}
+               ]
+             }) == %{
+               "attachment" => [
+                 %{
+                   "mediaType" => "video/mp4",
+                   "url" => [
+                     %{
+                       "href" => "https://pe.er/stat-480.mp4",
+                       "mediaType" => "video/mp4",
+                       "type" => "Link"
+                     }
+                   ]
+                 },
+                 %{
+                   "href" => "https://pe.er/stat-480.mp4",
+                   "mediaType" => "video/mp4",
+                   "mimeType" => "video/mp4",
+                   "url" => [
+                     %{
+                       "href" => "https://pe.er/stat-480.mp4",
+                       "mediaType" => "video/mp4",
+                       "type" => "Link"
+                     }
+                   ]
+                 }
+               ]
+             }
+    end
+  end
 end