From a31af93e1d10d9db8796d86ccda35873697b5a4c Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Tue, 10 Sep 2019 16:43:10 +0300 Subject: [PATCH 1/3] added tests /activity_pub/transmogrifier.ex --- .../web/activity_pub/transmogrifier.ex | 264 +++++++----------- test/web/activity_pub/transmogrifier_test.exs | 162 +++++++++++ 2 files changed, 270 insertions(+), 156 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 468961bd0..93b3a1f97 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -41,8 +41,7 @@ def fix_object(object, options \\ []) do end def fix_summary(%{"summary" => nil} = object) do - object - |> Map.put("summary", "") + Map.put(object, "summary", "") end def fix_summary(%{"summary" => _} = object) do @@ -50,10 +49,7 @@ def fix_summary(%{"summary" => _} = object) do object end - def fix_summary(object) do - object - |> Map.put("summary", "") - end + def fix_summary(object), do: Map.put(object, "summary", "") def fix_addressing_list(map, field) do cond do @@ -73,13 +69,9 @@ def fix_explicit_addressing( explicit_mentions, follower_collection ) do - explicit_to = - to - |> Enum.filter(fn x -> x in explicit_mentions end) + explicit_to = Enum.filter(to, fn x -> x in explicit_mentions end) - explicit_cc = - to - |> Enum.filter(fn x -> x not in explicit_mentions end) + explicit_cc = Enum.filter(to, fn x -> x not in explicit_mentions end) final_cc = (cc ++ explicit_cc) @@ -97,13 +89,19 @@ def fix_explicit_addressing(object, _explicit_mentions, _followers_collection), def fix_explicit_addressing(%{"directMessage" => true} = object), do: object def fix_explicit_addressing(object) do - explicit_mentions = + explicit_mentions = Utils.determine_explicit_mentions(object) + + %User{follower_address: follower_collection} = object - |> Utils.determine_explicit_mentions() + |> Containment.get_actor() + |> User.get_cached_by_ap_id() - follower_collection = User.get_cached_by_ap_id(Containment.get_actor(object)).follower_address - - explicit_mentions = explicit_mentions ++ [Pleroma.Constants.as_public(), follower_collection] + explicit_mentions = + explicit_mentions ++ + [ + Pleroma.Constants.as_public(), + follower_collection + ] fix_explicit_addressing(object, explicit_mentions, follower_collection) end @@ -147,48 +145,25 @@ def fix_addressing(object) do end def fix_actor(%{"attributedTo" => actor} = object) do - object - |> Map.put("actor", Containment.get_actor(%{"actor" => actor})) + Map.put(object, "actor", Containment.get_actor(%{"actor" => actor})) end def fix_in_reply_to(object, options \\ []) def fix_in_reply_to(%{"inReplyTo" => in_reply_to} = object, options) when not is_nil(in_reply_to) do - in_reply_to_id = - cond do - is_bitstring(in_reply_to) -> - in_reply_to - - is_map(in_reply_to) && is_bitstring(in_reply_to["id"]) -> - in_reply_to["id"] - - is_list(in_reply_to) && is_bitstring(Enum.at(in_reply_to, 0)) -> - Enum.at(in_reply_to, 0) - - # Maybe I should output an error too? - true -> - "" - end - + in_reply_to_id = prepare_in_reply_to(in_reply_to) object = Map.put(object, "inReplyToAtomUri", in_reply_to_id) if Federator.allowed_incoming_reply_depth?(options[:depth]) do - case get_obj_helper(in_reply_to_id, options) do - {:ok, replied_object} -> - with %Activity{} = _activity <- - Activity.get_create_by_object_ap_id(replied_object.data["id"]) do - object - |> Map.put("inReplyTo", replied_object.data["id"]) - |> Map.put("inReplyToAtomUri", object["inReplyToAtomUri"] || in_reply_to_id) - |> Map.put("conversation", replied_object.data["context"] || object["conversation"]) - |> Map.put("context", replied_object.data["context"] || object["conversation"]) - else - e -> - Logger.error("Couldn't fetch \"#{inspect(in_reply_to_id)}\", error: #{inspect(e)}") - object - end - + with {:ok, replied_object} <- get_obj_helper(in_reply_to_id, options), + %Activity{} = _ <- Activity.get_create_by_object_ap_id(replied_object.data["id"]) do + object + |> Map.put("inReplyTo", replied_object.data["id"]) + |> Map.put("inReplyToAtomUri", object["inReplyToAtomUri"] || in_reply_to_id) + |> Map.put("conversation", replied_object.data["context"] || object["conversation"]) + |> Map.put("context", replied_object.data["context"] || object["conversation"]) + else e -> Logger.error("Couldn't fetch \"#{inspect(in_reply_to_id)}\", error: #{inspect(e)}") object @@ -200,6 +175,22 @@ def fix_in_reply_to(%{"inReplyTo" => in_reply_to} = object, options) def fix_in_reply_to(object, _options), do: object + defp prepare_in_reply_to(in_reply_to) do + cond do + is_bitstring(in_reply_to) -> + in_reply_to + + is_map(in_reply_to) && is_bitstring(in_reply_to["id"]) -> + in_reply_to["id"] + + is_list(in_reply_to) && is_bitstring(Enum.at(in_reply_to, 0)) -> + Enum.at(in_reply_to, 0) + + true -> + "" + end + end + def fix_context(object) do context = object["context"] || object["conversation"] || Utils.generate_context_id() @@ -210,8 +201,7 @@ def fix_context(object) do def fix_attachments(%{"attachment" => attachment} = object) when is_list(attachment) do attachments = - attachment - |> Enum.map(fn data -> + Enum.map(attachment, fn data -> media_type = data["mediaType"] || data["mimeType"] href = data["url"] || data["href"] @@ -222,30 +212,23 @@ def fix_attachments(%{"attachment" => attachment} = object) when is_list(attachm |> Map.put("url", url) end) - object - |> Map.put("attachment", attachments) + Map.put(object, "attachment", attachments) end def fix_attachments(%{"attachment" => attachment} = object) when is_map(attachment) do - Map.put(object, "attachment", [attachment]) - |> fix_attachments() + fix_attachments(Map.put(object, "attachment", [attachment])) end def fix_attachments(object), do: object def fix_url(%{"url" => url} = object) when is_map(url) do - object - |> Map.put("url", url["href"]) + Map.put(object, "url", url["href"]) end def fix_url(%{"type" => "Video", "url" => url} = object) when is_list(url) do first_element = Enum.at(url, 0) - link_element = - url - |> Enum.filter(fn x -> is_map(x) end) - |> Enum.filter(fn x -> x["mimeType"] == "text/html" end) - |> Enum.at(0) + link_element = Enum.find(url, fn x -> is_map(x) and x["mimeType"] == "text/html" end) object |> Map.put("attachment", [first_element]) @@ -263,36 +246,32 @@ def fix_url(%{"type" => object_type, "url" => url} = object) true -> "" end - object - |> Map.put("url", url_string) + Map.put(object, "url", url_string) end def fix_url(object), do: object def fix_emoji(%{"tag" => tags} = object) when is_list(tags) do - emoji = tags |> Enum.filter(fn data -> data["type"] == "Emoji" and data["icon"] end) - emoji = - emoji + tags + |> Enum.filter(fn data -> data["type"] == "Emoji" and data["icon"] end) |> Enum.reduce(%{}, fn data, mapping -> name = String.trim(data["name"], ":") - mapping |> Map.put(name, data["icon"]["url"]) + Map.put(mapping, name, data["icon"]["url"]) end) # we merge mastodon and pleroma emoji into a single mapping, to allow for both wire formats emoji = Map.merge(object["emoji"] || %{}, emoji) - object - |> Map.put("emoji", emoji) + Map.put(object, "emoji", emoji) end def fix_emoji(%{"tag" => %{"type" => "Emoji"} = tag} = object) do name = String.trim(tag["name"], ":") emoji = %{name => tag["icon"]["url"]} - object - |> Map.put("emoji", emoji) + Map.put(object, "emoji", emoji) end def fix_emoji(object), do: object @@ -303,17 +282,13 @@ def fix_tag(%{"tag" => tag} = object) when is_list(tag) do |> Enum.filter(fn data -> data["type"] == "Hashtag" and data["name"] end) |> Enum.map(fn data -> String.slice(data["name"], 1..-1) end) - combined = tag ++ tags - - object - |> Map.put("tag", combined) + Map.put(object, "tag", tag ++ tags) end def fix_tag(%{"tag" => %{"type" => "Hashtag", "name" => hashtag} = tag} = object) do combined = [tag, String.slice(hashtag, 1..-1)] - object - |> Map.put("tag", combined) + Map.put(object, "tag", combined) end def fix_tag(%{"tag" => %{} = tag} = object), do: Map.put(object, "tag", [tag]) @@ -325,8 +300,7 @@ def fix_content_map(%{"contentMap" => content_map} = object) do content_groups = Map.to_list(content_map) {_, content} = Enum.at(content_groups, 0) - object - |> Map.put("content", content) + Map.put(object, "content", content) end def fix_content_map(object), do: object @@ -335,16 +309,11 @@ def fix_type(object, options \\ []) def fix_type(%{"inReplyTo" => reply_id, "name" => _} = object, options) when is_binary(reply_id) do - reply = - with true <- Federator.allowed_incoming_reply_depth?(options[:depth]), - {:ok, object} <- get_obj_helper(reply_id, options) do - object - end - - if reply && reply.data["type"] == "Question" do + with true <- Federator.allowed_incoming_reply_depth?(options[:depth]), + {:ok, %{data: %{"type" => "Question"} = _} = _} <- get_obj_helper(reply_id, options) do Map.put(object, "type", "Answer") else - object + _ -> object end end @@ -376,6 +345,17 @@ defp get_follow_activity(follow_object, followed) do end end + # Reduce the object list to find the reported user. + defp get_reported(objects) do + Enum.reduce_while(objects, nil, fn ap_id, _ -> + with %User{} = user <- User.get_cached_by_ap_id(ap_id) do + {:halt, user} + else + _ -> {:cont, nil} + end + end) + end + def handle_incoming(data, options \\ []) # Flag objects are placed ahead of the ID check because Mastodon 2.8 and earlier send them @@ -384,31 +364,19 @@ def handle_incoming(%{"type" => "Flag", "object" => objects, "actor" => actor} = with context <- data["context"] || Utils.generate_context_id(), content <- data["content"] || "", %User{} = actor <- User.get_cached_by_ap_id(actor), - # Reduce the object list to find the reported user. - %User{} = account <- - Enum.reduce_while(objects, nil, fn ap_id, _ -> - with %User{} = user <- User.get_cached_by_ap_id(ap_id) do - {:halt, user} - else - _ -> {:cont, nil} - end - end), - + %User{} = account <- get_reported(objects), # Remove the reported user from the object list. statuses <- Enum.filter(objects, fn ap_id -> ap_id != account.ap_id end) do - params = %{ + %{ actor: actor, context: context, account: account, statuses: statuses, content: content, - additional: %{ - "cc" => [account.ap_id] - } + additional: %{"cc" => [account.ap_id]} } - - ActivityPub.flag(params) + |> ActivityPub.flag() end end @@ -755,8 +723,13 @@ def handle_incoming( 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: {:ok, object}, else: nil + if object = Object.normalize(id, true, options) do + {:ok, object} + else + nil + end end def set_reply_to_uri(%{"inReplyTo" => in_reply_to} = object) when is_binary(in_reply_to) do @@ -855,27 +828,24 @@ def prepare_outgoing(%{"type" => _type} = data) do {:ok, data} end - def maybe_fix_object_url(data) do - if is_binary(data["object"]) and not String.starts_with?(data["object"], "http") do - case get_obj_helper(data["object"]) do - {:ok, relative_object} -> - if relative_object.data["external_url"] do - _data = - data - |> Map.put("object", relative_object.data["external_url"]) - else - data - end - - e -> - Logger.error("Couldn't fetch #{data["object"]} #{inspect(e)}") - data - end + def maybe_fix_object_url(%{"object" => object} = data) when is_binary(object) do + with false <- String.starts_with?(object, "http"), + {:fetch, {:ok, relative_object}} <- {:fetch, get_obj_helper(object)}, + %{data: %{"external_url" => external_url}} when not is_nil(external_url) <- + relative_object do + Map.put(data, "object", external_url) else - data + {:fetch, e} -> + Logger.error("Couldn't fetch #{object} #{inspect(e)}") + data + + _ -> + data end end + def maybe_fix_object_url(data), do: data + def add_hashtags(object) do tags = (object["tag"] || []) @@ -893,8 +863,7 @@ def add_hashtags(object) do tag end) - object - |> Map.put("tag", tags) + Map.put(object, "tag", tags) end def add_mention_tags(object) do @@ -907,15 +876,13 @@ def add_mention_tags(object) do tags = object["tag"] || [] - object - |> Map.put("tag", tags ++ mentions) + Map.put(object, "tag", tags ++ mentions) end def add_emoji_tags(%User{info: %{"emoji" => _emoji} = user_info} = object) do user_info = add_emoji_tags(user_info) - object - |> Map.put(:info, user_info) + Map.put(object, :info, user_info) end # TODO: we should probably send mtime instead of unix epoch time for updated @@ -923,8 +890,7 @@ def add_emoji_tags(%{"emoji" => emoji} = object) do tags = object["tag"] || [] out = - emoji - |> Enum.map(fn {name, url} -> + Enum.map(emoji, fn {name, url} -> %{ "icon" => %{"url" => url, "type" => "Image"}, "name" => ":" <> name <> ":", @@ -934,13 +900,10 @@ def add_emoji_tags(%{"emoji" => emoji} = object) do } end) - object - |> Map.put("tag", tags ++ out) + Map.put(object, "tag", tags ++ out) end - def add_emoji_tags(object) do - object - end + def add_emoji_tags(object), do: object def set_conversation(object) do Map.put(object, "conversation", object["context"]) @@ -959,9 +922,7 @@ def set_type(object), do: object def add_attributed_to(object) do attributed_to = object["attributedTo"] || object["actor"] - - object - |> Map.put("attributedTo", attributed_to) + Map.put(object, "attributedTo", attributed_to) end def prepare_attachments(object) do @@ -972,8 +933,7 @@ def prepare_attachments(object) do %{"url" => href, "mediaType" => media_type, "name" => data["name"], "type" => "Document"} end) - object - |> Map.put("attachment", attachments) + Map.put(object, "attachment", attachments) end defp strip_internal_fields(object) do @@ -990,12 +950,9 @@ defp strip_internal_fields(object) do end defp strip_internal_tags(%{"tag" => tags} = object) do - tags = - tags - |> Enum.filter(fn x -> is_map(x) end) + tags = Enum.filter(tags, fn x -> is_map(x) end) - object - |> Map.put("tag", tags) + Map.put(object, "tag", tags) end defp strip_internal_tags(object), do: object @@ -1074,16 +1031,11 @@ def maybe_retire_websub(ap_id) do end end - def maybe_fix_user_url(data) do - if is_map(data["url"]) do - Map.put(data, "url", data["url"]["href"]) - else - data - end + def maybe_fix_user_url(%{"url" => url} = data) when is_map(url) do + Map.put(data, "url", url["href"]) end - def maybe_fix_user_object(data) do - data - |> maybe_fix_user_url - end + def maybe_fix_user_url(data), do: data + + def maybe_fix_user_object(data), do: maybe_fix_user_url(data) end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 0661d5d7c..63c869d35 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -1451,4 +1451,166 @@ test "removes recipient's follower collection from cc", %{user: user} do refute recipient.follower_address in fixed_object["to"] end end + + describe "fix_summary/1" do + test "returns fixed object" do + assert Transmogrifier.fix_summary(%{"summary" => nil}) == %{"summary" => ""} + assert Transmogrifier.fix_summary(%{"summary" => "ok"}) == %{"summary" => "ok"} + assert Transmogrifier.fix_summary(%{}) == %{"summary" => ""} + end + end + + describe "fix_in_reply_to/2" do + clear_config([:instance, :federation_incoming_replies_max_depth]) + + setup do + data = Poison.decode!(File.read!("test/fixtures/mastodon-post-activity.json")) + [data: data] + end + + test "returns not modified object when hasn't containts inReplyTo field", %{data: data} do + assert Transmogrifier.fix_in_reply_to(data) == data + end + + test "returns object with inReplyToAtomUri when denied incoming reply", %{data: data} do + Pleroma.Config.put([:instance, :federation_incoming_replies_max_depth], 0) + + object_with_reply = + Map.put(data["object"], "inReplyTo", "https://shitposter.club/notice/2827873") + + modified_object = Transmogrifier.fix_in_reply_to(object_with_reply) + assert modified_object["inReplyTo"] == "https://shitposter.club/notice/2827873" + assert modified_object["inReplyToAtomUri"] == "https://shitposter.club/notice/2827873" + + object_with_reply = + Map.put(data["object"], "inReplyTo", %{"id" => "https://shitposter.club/notice/2827873"}) + + modified_object = Transmogrifier.fix_in_reply_to(object_with_reply) + assert modified_object["inReplyTo"] == %{"id" => "https://shitposter.club/notice/2827873"} + assert modified_object["inReplyToAtomUri"] == "https://shitposter.club/notice/2827873" + + object_with_reply = + Map.put(data["object"], "inReplyTo", ["https://shitposter.club/notice/2827873"]) + + modified_object = Transmogrifier.fix_in_reply_to(object_with_reply) + assert modified_object["inReplyTo"] == ["https://shitposter.club/notice/2827873"] + assert modified_object["inReplyToAtomUri"] == "https://shitposter.club/notice/2827873" + + object_with_reply = Map.put(data["object"], "inReplyTo", []) + modified_object = Transmogrifier.fix_in_reply_to(object_with_reply) + assert modified_object["inReplyTo"] == [] + assert modified_object["inReplyToAtomUri"] == "" + end + + test "returns modified object when allowed incoming reply", %{data: data} do + object_with_reply = + Map.put( + data["object"], + "inReplyTo", + "https://shitposter.club/notice/2827873" + ) + + Pleroma.Config.put([:instance, :federation_incoming_replies_max_depth], 5) + modified_object = Transmogrifier.fix_in_reply_to(object_with_reply) + + assert modified_object["inReplyTo"] == + "tag:shitposter.club,2017-05-05:noticeId=2827873:objectType=comment" + + assert modified_object["inReplyToAtomUri"] == "https://shitposter.club/notice/2827873" + + assert modified_object["conversation"] == + "tag:shitposter.club,2017-05-05:objectType=thread:nonce=3c16e9c2681f6d26" + + assert modified_object["context"] == + "tag:shitposter.club,2017-05-05:objectType=thread:nonce=3c16e9c2681f6d26" + end + end + + describe "fix_url/1" do + test "fixes data for object when url is map" do + object = %{ + "url" => %{ + "type" => "Link", + "mimeType" => "video/mp4", + "href" => "https://peede8d-46fb-ad81-2d4c2d1630e3-480.mp4" + } + } + + assert Transmogrifier.fix_url(object) == %{ + "url" => "https://peede8d-46fb-ad81-2d4c2d1630e3-480.mp4" + } + end + + test "fixes data for video object" do + object = %{ + "type" => "Video", + "url" => [ + %{ + "type" => "Link", + "mimeType" => "video/mp4", + "href" => "https://peede8d-46fb-ad81-2d4c2d1630e3-480.mp4" + }, + %{ + "type" => "Link", + "mimeType" => "video/mp4", + "href" => "https://peertube46fb-ad81-2d4c2d1630e3-240.mp4" + }, + %{ + "type" => "Link", + "mimeType" => "text/html", + "href" => "https://peertube.-2d4c2d1630e3" + }, + %{ + "type" => "Link", + "mimeType" => "text/html", + "href" => "https://peertube.-2d4c2d16377-42" + } + ] + } + + assert Transmogrifier.fix_url(object) == %{ + "attachment" => [ + %{ + "href" => "https://peede8d-46fb-ad81-2d4c2d1630e3-480.mp4", + "mimeType" => "video/mp4", + "type" => "Link" + } + ], + "type" => "Video", + "url" => "https://peertube.-2d4c2d1630e3" + } + end + + test "fixes url for not Video object" do + object = %{ + "type" => "Text", + "url" => [ + %{ + "type" => "Link", + "mimeType" => "text/html", + "href" => "https://peertube.-2d4c2d1630e3" + }, + %{ + "type" => "Link", + "mimeType" => "text/html", + "href" => "https://peertube.-2d4c2d16377-42" + } + ] + } + + assert Transmogrifier.fix_url(object) == %{ + "type" => "Text", + "url" => "https://peertube.-2d4c2d1630e3" + } + + assert Transmogrifier.fix_url(%{"type" => "Text", "url" => []}) == %{ + "type" => "Text", + "url" => "" + } + end + + test "retunrs not modified object" do + assert Transmogrifier.fix_url(%{"type" => "Text"}) == %{"type" => "Text"} + end + end end From fcf604fa43031be747b33c05866a192d9651322c Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Wed, 11 Sep 2019 07:23:33 +0300 Subject: [PATCH 2/3] added tests --- lib/pleroma/object/fetcher.ex | 77 ++++++++++--------- .../web/activity_pub/transmogrifier.ex | 12 +-- test/web/activity_pub/transmogrifier_test.exs | 74 ++++++++++++++++++ 3 files changed, 121 insertions(+), 42 deletions(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index c1795ae0f..2217d1eb3 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/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)}, - {:normalize, nil} <- {:normalize, Object.normalize(data, false)}, - params <- %{ - "type" => "Create", - "to" => data["to"], - "cc" => data["cc"], - # Should we seriously keep this attributedTo thing? - "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 + {:error, {:reject, nil}} -> + {:reject, nil} + + {:object, data, nil} -> + reinject_object(data) + + {:normalize, object = %Object{}} -> {:ok, object} - else - {:containment, _} -> - {:error, "Object containment failed."} - {:error, {:reject, nil}} -> - {:reject, nil} + {:fetch_object, %Object{} = object} -> + {:ok, object} - {:object, data, nil} -> - reinject_object(data) + _e -> + # 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{}} -> - {:ok, object} - - _e -> - # Only fallback when receiving a fetch/normalization error with ActivityPub - 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 + # 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 + + 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 diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 93b3a1f97..18a3c3f39 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/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 - {:ok, object} - else - nil + case Object.normalize(id, true, options) do + %Object{} = object -> {:ok, object} + _ -> nil end end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 63c869d35..ab6e76056 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/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 From 007e0c1ce158bdfc11738a194944534837ae0258 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Wed, 11 Sep 2019 23:19:06 +0300 Subject: [PATCH 3/3] added tests --- .../web/activity_pub/transmogrifier.ex | 35 ++++++++++--------- .../web/activity_pub/views/user_view.ex | 7 ++-- test/web/activity_pub/transmogrifier_test.exs | 31 ++++++++++++++++ .../web/activity_pub/views/user_view_test.exs | 16 +++++++++ 4 files changed, 68 insertions(+), 21 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 18a3c3f39..9f699de9e 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -870,41 +870,44 @@ def add_mention_tags(object) do mentions = object |> Utils.get_notified_from_object() - |> Enum.map(fn user -> - %{"type" => "Mention", "href" => user.ap_id, "name" => "@#{user.nickname}"} - end) + |> Enum.map(&build_mention_tag/1) tags = object["tag"] || [] Map.put(object, "tag", tags ++ mentions) end - def add_emoji_tags(%User{info: %{"emoji" => _emoji} = user_info} = object) do - user_info = add_emoji_tags(user_info) + defp build_mention_tag(%{ap_id: ap_id, nickname: nickname} = _) do + %{"type" => "Mention", "href" => ap_id, "name" => "@#{nickname}"} + end - Map.put(object, :info, user_info) + def take_emoji_tags(%User{info: %{emoji: emoji} = _user_info} = _user) do + emoji + |> Enum.flat_map(&Map.to_list/1) + |> Enum.map(&build_emoji_tag/1) end # TODO: we should probably send mtime instead of unix epoch time for updated def add_emoji_tags(%{"emoji" => emoji} = object) do tags = object["tag"] || [] - out = - Enum.map(emoji, fn {name, url} -> - %{ - "icon" => %{"url" => url, "type" => "Image"}, - "name" => ":" <> name <> ":", - "type" => "Emoji", - "updated" => "1970-01-01T00:00:00Z", - "id" => url - } - end) + out = Enum.map(emoji, &build_emoji_tag/1) Map.put(object, "tag", tags ++ out) end def add_emoji_tags(object), do: object + defp build_emoji_tag({name, url}) do + %{ + "icon" => %{"url" => url, "type" => "Image"}, + "name" => ":" <> name <> ":", + "type" => "Emoji", + "updated" => "1970-01-01T00:00:00Z", + "id" => url + } + end + def set_conversation(object) do Map.put(object, "conversation", object["context"]) end diff --git a/lib/pleroma/web/activity_pub/views/user_view.ex b/lib/pleroma/web/activity_pub/views/user_view.ex index 7be734b26..8abfa1fcd 100644 --- a/lib/pleroma/web/activity_pub/views/user_view.ex +++ b/lib/pleroma/web/activity_pub/views/user_view.ex @@ -75,10 +75,7 @@ def render("user.json", %{user: user}) do endpoints = render("endpoints.json", %{user: user}) - user_tags = - user - |> Transmogrifier.add_emoji_tags() - |> Map.get("tag", []) + emoji_tags = Transmogrifier.take_emoji_tags(user) fields = user.info @@ -110,7 +107,7 @@ def render("user.json", %{user: user}) do }, "endpoints" => endpoints, "attachment" => fields, - "tag" => (user.info.source_data["tag"] || []) ++ user_tags + "tag" => (user.info.source_data["tag"] || []) ++ emoji_tags } |> Map.merge(maybe_make_image(&User.avatar_url/2, "icon", user)) |> Map.merge(maybe_make_image(&User.banner_url/2, "image", user)) diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index ab6e76056..87ef843c6 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -1687,4 +1687,35 @@ test "returns modified object when attachment is list" do } end end + + describe "fix_emoji/1" do + test "returns not modified object when object not contains tags" do + data = Poison.decode!(File.read!("test/fixtures/mastodon-post-activity.json")) + assert Transmogrifier.fix_emoji(data) == data + end + + test "returns object with emoji when object contains list tags" do + assert Transmogrifier.fix_emoji(%{ + "tag" => [ + %{"type" => "Emoji", "name" => ":bib:", "icon" => %{"url" => "/test"}}, + %{"type" => "Hashtag"} + ] + }) == %{ + "emoji" => %{"bib" => "/test"}, + "tag" => [ + %{"icon" => %{"url" => "/test"}, "name" => ":bib:", "type" => "Emoji"}, + %{"type" => "Hashtag"} + ] + } + end + + test "returns object with emoji when object contains map tag" do + assert Transmogrifier.fix_emoji(%{ + "tag" => %{"type" => "Emoji", "name" => ":bib:", "icon" => %{"url" => "/test"}} + }) == %{ + "emoji" => %{"bib" => "/test"}, + "tag" => %{"icon" => %{"url" => "/test"}, "name" => ":bib:", "type" => "Emoji"} + } + end + end end diff --git a/test/web/activity_pub/views/user_view_test.exs b/test/web/activity_pub/views/user_view_test.exs index fb7fd9e79..4390f9272 100644 --- a/test/web/activity_pub/views/user_view_test.exs +++ b/test/web/activity_pub/views/user_view_test.exs @@ -37,6 +37,22 @@ test "Renders profile fields" do } = UserView.render("user.json", %{user: user}) end + test "Renders with emoji tags" do + user = insert(:user, %{info: %{emoji: [%{"bib" => "/test"}]}}) + + assert %{ + "tag" => [ + %{ + "icon" => %{"type" => "Image", "url" => "/test"}, + "id" => "/test", + "name" => ":bib:", + "type" => "Emoji", + "updated" => "1970-01-01T00:00:00Z" + } + ] + } = UserView.render("user.json", %{user: user}) + end + test "Does not add an avatar image if the user hasn't set one" do user = insert(:user) {:ok, user} = User.ensure_keys_present(user)