diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 4b181ec59..a4dbc3947 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -44,7 +44,15 @@ def get_by_ap_id(ap_id) do Repo.one(from(object in Object, where: fragment("(?)->>'id' = ?", object.data, ^ap_id))) end + defp warn_on_no_object_preloaded(ap_id) do + "Object.normalize() called without preloaded object (#{ap_id}). Consider preloading the object" + |> Logger.debug() + + Logger.debug("Backtrace: #{inspect(Process.info(:erlang.self(), :current_stacktrace))}") + end + def normalize(_, fetch_remote \\ true) + # If we pass an Activity to Object.normalize(), we can try to use the preloaded object. # Use this whenever possible, especially when walking graphs in an O(N) loop! def normalize(%Object{} = object, _), do: object @@ -55,25 +63,15 @@ def normalize(%Activity{data: %{"object" => %{"fake" => true} = data}}, _) do %Object{id: "pleroma:fake_object_id", data: data} end - # Catch and log Object.normalize() calls where the Activity's child object is not - # preloaded. + # No preloaded object def normalize(%Activity{data: %{"object" => %{"id" => ap_id}}}, fetch_remote) do - Logger.debug( - "Object.normalize() called without preloaded object (#{ap_id}). Consider preloading the object!" - ) - - Logger.debug("Backtrace: #{inspect(Process.info(:erlang.self(), :current_stacktrace))}") - + warn_on_no_object_preloaded(ap_id) normalize(ap_id, fetch_remote) end + # No preloaded object def normalize(%Activity{data: %{"object" => ap_id}}, fetch_remote) do - Logger.debug( - "Object.normalize() called without preloaded object (#{ap_id}). Consider preloading the object!" - ) - - Logger.debug("Backtrace: #{inspect(Process.info(:erlang.self(), :current_stacktrace))}") - + warn_on_no_object_preloaded(ap_id) normalize(ap_id, fetch_remote) end diff --git a/test/activity_test.exs b/test/activity_test.exs index 7ba4363c8..b27f6fd36 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -6,6 +6,7 @@ defmodule Pleroma.ActivityTest do use Pleroma.DataCase alias Pleroma.Activity alias Pleroma.Bookmark + alias Pleroma.Object alias Pleroma.ThreadMute import Pleroma.Factory @@ -18,15 +19,18 @@ test "returns an activity by it's AP id" do test "returns activities by it's objects AP ids" do activity = insert(:note_activity) - [found_activity] = Activity.get_all_create_by_object_ap_id(activity.data["object"]["id"]) + object_data = Object.normalize(activity).data + + [found_activity] = Activity.get_all_create_by_object_ap_id(object_data["id"]) assert activity == found_activity end test "returns the activity that created an object" do activity = insert(:note_activity) + object_data = Object.normalize(activity).data - found_activity = Activity.get_create_by_object_ap_id(activity.data["object"]["id"]) + found_activity = Activity.get_create_by_object_ap_id(object_data["id"]) assert activity == found_activity end diff --git a/test/bbs/handler_test.exs b/test/bbs/handler_test.exs index 7d5d68d11..6f6533e3d 100644 --- a/test/bbs/handler_test.exs +++ b/test/bbs/handler_test.exs @@ -59,6 +59,7 @@ test "replying" do another_user = insert(:user) {:ok, activity} = CommonAPI.post(another_user, %{"status" => "this is a test post"}) + activity_object = Object.normalize(activity) output = capture_io(fn -> @@ -76,8 +77,9 @@ test "replying" do ) assert reply.actor == user.ap_id - object = Object.normalize(reply) - assert object.data["content"] == "this is a reply" - assert object.data["inReplyTo"] == activity.data["object"] + + reply_object_data = Object.normalize(reply).data + assert reply_object_data["content"] == "this is a reply" + assert reply_object_data["inReplyTo"] == activity_object.data["id"] end end diff --git a/test/support/factory.ex b/test/support/factory.ex index c2812e8f7..b1023da38 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -5,6 +5,7 @@ defmodule Pleroma.Factory do use ExMachina.Ecto, repo: Pleroma.Repo alias Pleroma.User + alias Pleroma.Object def participation_factory do conversation = insert(:conversation) @@ -122,7 +123,7 @@ def note_activity_factory(attrs \\ %{}) do "type" => "Create", "actor" => note.data["actor"], "to" => note.data["to"], - "object" => note.data, + "object" => note.data["id"], "published" => DateTime.utc_now() |> DateTime.to_iso8601(), "context" => note.data["context"] } @@ -176,13 +177,14 @@ def announce_activity_factory(attrs \\ %{}) do def like_activity_factory do note_activity = insert(:note_activity) + object = Object.normalize(note_activity) user = insert(:user) data = %{ "id" => Pleroma.Web.ActivityPub.Utils.generate_activity_id(), "actor" => user.ap_id, "type" => "Like", - "object" => note_activity.data["object"]["id"], + "object" => object.data["id"], "published_at" => DateTime.utc_now() |> DateTime.to_iso8601() } diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs index 8b3233729..c99726180 100644 --- a/test/web/activity_pub/activity_pub_controller_test.exs +++ b/test/web/activity_pub/activity_pub_controller_test.exs @@ -163,7 +163,8 @@ test "it returns 404 for tombstone objects", %{conn: conn} do describe "/object/:uuid/likes" do test "it returns the like activities in a collection", %{conn: conn} do like = insert(:like_activity) - uuid = String.split(like.data["object"], "/") |> List.last() + like_object_ap_id = Object.normalize(like).data["id"] + uuid = String.split(like_object_ap_id, "/") |> List.last() result = conn @@ -302,6 +303,7 @@ test "it rejects reads from other users", %{conn: conn} do test "it returns a note activity in a collection", %{conn: conn} do note_activity = insert(:direct_note_activity) + note_object = Object.normalize(note_activity) user = User.get_cached_by_ap_id(hd(note_activity.data["to"])) conn = @@ -310,7 +312,7 @@ test "it returns a note activity in a collection", %{conn: conn} do |> put_req_header("accept", "application/activity+json") |> get("/users/#{user.nickname}/inbox") - assert response(conn, 200) =~ note_activity.data["object"]["content"] + assert response(conn, 200) =~ note_object.data["content"] end test "it clears `unreachable` federation status of the sender", %{conn: conn, data: data} do @@ -388,6 +390,7 @@ test "it will not bomb when there is no activity", %{conn: conn} do test "it returns a note activity in a collection", %{conn: conn} do note_activity = insert(:note_activity) + note_object = Object.normalize(note_activity) user = User.get_cached_by_ap_id(note_activity.data["actor"]) conn = @@ -395,7 +398,7 @@ test "it returns a note activity in a collection", %{conn: conn} do |> put_req_header("accept", "application/activity+json") |> get("/users/#{user.nickname}/outbox") - assert response(conn, 200) =~ note_activity.data["object"]["content"] + assert response(conn, 200) =~ note_object.data["content"] end test "it returns an announce activity in a collection", %{conn: conn} do @@ -457,12 +460,13 @@ test "it rejects an incoming activity with bogus type", %{conn: conn} do test "it erects a tombstone when receiving a delete activity", %{conn: conn} do note_activity = insert(:note_activity) + note_object = Object.normalize(note_activity) user = User.get_cached_by_ap_id(note_activity.data["actor"]) data = %{ type: "Delete", object: %{ - id: note_activity.data["object"]["id"] + id: note_object.data["id"] } } @@ -475,19 +479,19 @@ test "it erects a tombstone when receiving a delete activity", %{conn: conn} do result = json_response(conn, 201) assert Activity.get_by_ap_id(result["id"]) - object = Object.get_by_ap_id(note_activity.data["object"]["id"]) - assert object + assert object = Object.get_by_ap_id(note_object.data["id"]) assert object.data["type"] == "Tombstone" end test "it rejects delete activity of object from other actor", %{conn: conn} do note_activity = insert(:note_activity) + note_object = Object.normalize(note_activity) user = insert(:user) data = %{ type: "Delete", object: %{ - id: note_activity.data["object"]["id"] + id: note_object.data["id"] } } @@ -502,12 +506,13 @@ test "it rejects delete activity of object from other actor", %{conn: conn} do test "it increases like count when receiving a like action", %{conn: conn} do note_activity = insert(:note_activity) + note_object = Object.normalize(note_activity) user = User.get_cached_by_ap_id(note_activity.data["actor"]) data = %{ type: "Like", object: %{ - id: note_activity.data["object"]["id"] + id: note_object.data["id"] } } @@ -520,8 +525,7 @@ test "it increases like count when receiving a like action", %{conn: conn} do result = json_response(conn, 201) assert Activity.get_by_ap_id(result["id"]) - object = Object.get_by_ap_id(note_activity.data["object"]["id"]) - assert object + assert object = Object.get_by_ap_id(note_object.data["id"]) assert object.data["like_count"] == 1 end end diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 76586ee4a..2728fef25 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -254,10 +254,8 @@ test "adds an id to a given object if it lacks one and is a note and inserts it } {:ok, %Activity{} = activity} = ActivityPub.insert(data) - object = Object.normalize(activity.data["object"]) - + assert object = Object.normalize(activity) assert is_binary(object.data["id"]) - assert %Object{} = Object.get_by_ap_id(activity.data["object"]) end end @@ -659,7 +657,8 @@ test "returns reblogs for users for whom reblogs have not been muted" do describe "like an object" do test "adds a like activity to the db" do note_activity = insert(:note_activity) - object = Object.get_by_ap_id(note_activity.data["object"]["id"]) + assert object = Object.normalize(note_activity) + user = insert(:user) user_two = insert(:user) @@ -667,7 +666,7 @@ test "adds a like activity to the db" do assert like_activity.data["actor"] == user.ap_id assert like_activity.data["type"] == "Like" - assert like_activity.data["object"] == object.data["id"] + # assert like_activity.data["object"] == object.data["id"] assert like_activity.data["to"] == [User.ap_followers(user), note_activity.data["actor"]] assert like_activity.data["context"] == object.data["context"] assert object.data["like_count"] == 1 @@ -678,9 +677,7 @@ test "adds a like activity to the db" do assert like_activity == same_like_activity assert object.data["likes"] == [user.ap_id] - - [note_activity] = Activity.get_all_create_by_object_ap_id(object.data["id"]) - assert note_activity.data["object"]["like_count"] == 1 + assert object.data["like_count"] == 1 {:ok, _like_activity, object} = ActivityPub.like(user_two, object) assert object.data["like_count"] == 2 @@ -690,7 +687,7 @@ test "adds a like activity to the db" do describe "unliking" do test "unliking a previously liked object" do note_activity = insert(:note_activity) - object = Object.get_by_ap_id(note_activity.data["object"]["id"]) + object = Object.normalize(note_activity) user = insert(:user) # Unliking something that hasn't been liked does nothing @@ -710,7 +707,7 @@ test "unliking a previously liked object" do describe "announcing an object" do test "adds an announce activity to the db" do note_activity = insert(:note_activity) - object = Object.get_by_ap_id(note_activity.data["object"]["id"]) + object = Object.normalize(note_activity) user = insert(:user) {:ok, announce_activity, object} = ActivityPub.announce(user, object) @@ -731,7 +728,7 @@ test "adds an announce activity to the db" do describe "unannouncing an object" do test "unannouncing a previously announced object" do note_activity = insert(:note_activity) - object = Object.get_by_ap_id(note_activity.data["object"]["id"]) + object = Object.normalize(note_activity) user = insert(:user) # Unannouncing an object that is not announced does nothing @@ -810,10 +807,11 @@ test "creates an undo activity for the last follow" do assert activity.data["type"] == "Undo" assert activity.data["actor"] == follower.ap_id - assert is_map(activity.data["object"]) - assert activity.data["object"]["type"] == "Follow" - assert activity.data["object"]["object"] == followed.ap_id - assert activity.data["object"]["id"] == follow_activity.data["id"] + embedded_object = activity.data["object"] + assert is_map(embedded_object) + assert embedded_object["type"] == "Follow" + assert embedded_object["object"] == followed.ap_id + assert embedded_object["id"] == follow_activity.data["id"] end end @@ -839,22 +837,23 @@ test "creates an undo activity for the last block" do assert activity.data["type"] == "Undo" assert activity.data["actor"] == blocker.ap_id - assert is_map(activity.data["object"]) - assert activity.data["object"]["type"] == "Block" - assert activity.data["object"]["object"] == blocked.ap_id - assert activity.data["object"]["id"] == block_activity.data["id"] + embedded_object = activity.data["object"] + assert is_map(embedded_object) + assert embedded_object["type"] == "Block" + assert embedded_object["object"] == blocked.ap_id + assert embedded_object["id"] == block_activity.data["id"] end end describe "deletion" do test "it creates a delete activity and deletes the original object" do note = insert(:note_activity) - object = Object.get_by_ap_id(note.data["object"]["id"]) + object = Object.normalize(note) {:ok, delete} = ActivityPub.delete(object) assert delete.data["type"] == "Delete" assert delete.data["actor"] == note.data["actor"] - assert delete.data["object"] == note.data["object"]["id"] + assert delete.data["object"] == object.data["id"] assert Activity.get_by_id(delete.id) != nil @@ -900,13 +899,14 @@ test "decrements user note count only for public activities" do test "it creates a delete activity and checks that it is also sent to users mentioned by the deleted object" do user = insert(:user) note = insert(:note_activity) + object = Object.normalize(note) {:ok, object} = - Object.get_by_ap_id(note.data["object"]["id"]) + object |> Object.change(%{ data: %{ - "actor" => note.data["object"]["actor"], - "id" => note.data["object"]["id"], + "actor" => object.data["actor"], + "id" => object.data["id"], "to" => [user.ap_id], "type" => "Note" } @@ -1018,8 +1018,9 @@ test "it creates an update activity with the new user data" do assert update.data["actor"] == user.ap_id assert update.data["to"] == [user.follower_address] - assert update.data["object"]["id"] == user_data["id"] - assert update.data["object"]["type"] == user_data["type"] + assert embedded_object = update.data["object"] + assert embedded_object["id"] == user_data["id"] + assert embedded_object["type"] == user_data["type"] end end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 68ec03c33..e0ab7b4c6 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -30,7 +30,7 @@ test "it ignores an incoming notice if we already have it" do data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!() - |> Map.put("object", activity.data["object"]) + |> Map.put("object", Object.normalize(activity).data) {:ok, returned_activity} = Transmogrifier.handle_incoming(data) @@ -51,7 +51,7 @@ test "it fetches replied-to activities if we don't have them" do |> Map.put("object", object) {:ok, returned_activity} = Transmogrifier.handle_incoming(data) - returned_object = Object.normalize(returned_activity.data["object"]) + returned_object = Object.normalize(returned_activity) assert activity = Activity.get_create_by_object_ap_id( @@ -99,25 +99,27 @@ test "it works for incoming notices" do assert data["actor"] == "http://mastodon.example.org/users/admin" - object = Object.normalize(data["object"]).data - assert object["id"] == "http://mastodon.example.org/users/admin/statuses/99512778738411822" + object_data = Object.normalize(data["object"]).data - assert object["to"] == ["https://www.w3.org/ns/activitystreams#Public"] + assert object_data["id"] == + "http://mastodon.example.org/users/admin/statuses/99512778738411822" - assert object["cc"] == [ + assert object_data["to"] == ["https://www.w3.org/ns/activitystreams#Public"] + + assert object_data["cc"] == [ "http://mastodon.example.org/users/admin/followers", "http://localtesting.pleroma.lol/users/lain" ] - assert object["actor"] == "http://mastodon.example.org/users/admin" - assert object["attributedTo"] == "http://mastodon.example.org/users/admin" + assert object_data["actor"] == "http://mastodon.example.org/users/admin" + assert object_data["attributedTo"] == "http://mastodon.example.org/users/admin" - assert object["context"] == + assert object_data["context"] == "tag:mastodon.example.org,2018-02-12:objectId=20:objectType=Conversation" - assert object["sensitive"] == true + assert object_data["sensitive"] == true - user = User.get_cached_by_ap_id(object["actor"]) + user = User.get_cached_by_ap_id(object_data["actor"]) assert user.info.note_count == 1 end @@ -548,10 +550,11 @@ test "it works for incoming unannounces with an existing notice" do {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) assert data["type"] == "Undo" - assert data["object"]["type"] == "Announce" - assert data["object"]["object"] == activity.data["object"] + assert object_data = data["object"] + assert object_data["type"] == "Announce" + assert object_data["object"] == activity.data["object"] - assert data["object"]["id"] == + assert object_data["id"] == "http://mastodon.example.org/users/admin/statuses/99542391527669785/activity" end @@ -861,7 +864,7 @@ test "it accepts Flag activities" do other_user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => "test post"}) - object = Object.normalize(activity.data["object"]) + object = Object.normalize(activity) message = %{ "@context" => "https://www.w3.org/ns/activitystreams", diff --git a/test/web/activity_pub/views/object_view_test.exs b/test/web/activity_pub/views/object_view_test.exs index d939fc5a7..281f96e1e 100644 --- a/test/web/activity_pub/views/object_view_test.exs +++ b/test/web/activity_pub/views/object_view_test.exs @@ -2,6 +2,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectViewTest do use Pleroma.DataCase import Pleroma.Factory + alias Pleroma.Object alias Pleroma.Web.ActivityPub.ObjectView alias Pleroma.Web.CommonAPI @@ -19,19 +20,21 @@ test "renders a note object" do test "renders a note activity" do note = insert(:note_activity) + object = Pleroma.Object.normalize(note) result = ObjectView.render("object.json", %{object: note}) assert result["id"] == note.data["id"] assert result["to"] == note.data["to"] assert result["object"]["type"] == "Note" - assert result["object"]["content"] == note.data["object"]["content"] + assert result["object"]["content"] == object.data["content"] assert result["type"] == "Create" assert result["@context"] end test "renders a like activity" do note = insert(:note_activity) + object = Object.normalize(note) user = insert(:user) {:ok, like_activity, _} = CommonAPI.favorite(note.id, user) @@ -39,12 +42,13 @@ test "renders a like activity" do result = ObjectView.render("object.json", %{object: like_activity}) assert result["id"] == like_activity.data["id"] - assert result["object"] == note.data["object"]["id"] + assert result["object"] == object.data["id"] assert result["type"] == "Like" end test "renders an announce activity" do note = insert(:note_activity) + object = Object.normalize(note) user = insert(:user) {:ok, announce_activity, _} = CommonAPI.repeat(note.id, user) @@ -52,7 +56,7 @@ test "renders an announce activity" do result = ObjectView.render("object.json", %{object: announce_activity}) assert result["id"] == announce_activity.data["id"] - assert result["object"] == note.data["object"]["id"] + assert result["object"] == object.data["id"] assert result["type"] == "Announce" end end diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 6f57bbe1f..958c931c4 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -34,7 +34,7 @@ test "it de-duplicates tags" do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => "#2hu #2HU"}) - object = Object.normalize(activity.data["object"]) + object = Object.normalize(activity) assert object.data["tag"] == ["2hu"] end @@ -87,7 +87,7 @@ test "it filters out obviously bad tags when accepting a post as HTML" do "content_type" => "text/html" }) - object = Object.normalize(activity.data["object"]) + object = Object.normalize(activity) assert object.data["content"] == "
2hu
alert('xss')" end @@ -103,7 +103,7 @@ test "it filters out obviously bad tags when accepting a post as Markdown" do "content_type" => "text/markdown" }) - object = Object.normalize(activity.data["object"]) + object = Object.normalize(activity) assert object.data["content"] == "2hu
alert('xss')" end diff --git a/test/web/mastodon_api/status_view_test.exs b/test/web/mastodon_api/status_view_test.exs index ec75150ab..f637097b8 100644 --- a/test/web/mastodon_api/status_view_test.exs +++ b/test/web/mastodon_api/status_view_test.exs @@ -55,7 +55,7 @@ test "tries to get a user by nickname if fetching by ap_id doesn't work" do test "a note with null content" do note = insert(:note_activity) - note_object = Object.normalize(note.data["object"]) + note_object = Object.normalize(note) data = note_object.data @@ -73,26 +73,27 @@ test "a note with null content" do test "a note activity" do note = insert(:note_activity) + object_data = Object.normalize(note).data user = User.get_cached_by_ap_id(note.data["actor"]) - convo_id = Utils.context_to_conversation_id(note.data["object"]["context"]) + convo_id = Utils.context_to_conversation_id(object_data["context"]) status = StatusView.render("status.json", %{activity: note}) created_at = - (note.data["object"]["published"] || "") + (object_data["published"] || "") |> String.replace(~r/\.\d+Z/, ".000Z") expected = %{ id: to_string(note.id), - uri: note.data["object"]["id"], + uri: object_data["id"], url: Pleroma.Web.Router.Helpers.o_status_url(Pleroma.Web.Endpoint, :notice, note), account: AccountView.render("account.json", %{user: user}), in_reply_to_id: nil, in_reply_to_account_id: nil, card: nil, reblog: nil, - content: HtmlSanitizeEx.basic_html(note.data["object"]["content"]), + content: HtmlSanitizeEx.basic_html(object_data["content"]), created_at: created_at, reblogs_count: 0, replies_count: 0, @@ -104,14 +105,14 @@ test "a note activity" do pinned: false, sensitive: false, poll: nil, - spoiler_text: HtmlSanitizeEx.basic_html(note.data["object"]["summary"]), + spoiler_text: HtmlSanitizeEx.basic_html(object_data["summary"]), visibility: "public", media_attachments: [], mentions: [], tags: [ %{ - name: "#{note.data["object"]["tag"]}", - url: "/tag/#{note.data["object"]["tag"]}" + name: "#{object_data["tag"]}", + url: "/tag/#{object_data["tag"]}" } ], application: %{ @@ -131,8 +132,8 @@ test "a note activity" do local: true, conversation_id: convo_id, in_reply_to_account_acct: nil, - content: %{"text/plain" => HtmlSanitizeEx.strip_tags(note.data["object"]["content"])}, - spoiler_text: %{"text/plain" => HtmlSanitizeEx.strip_tags(note.data["object"]["summary"])} + content: %{"text/plain" => HtmlSanitizeEx.strip_tags(object_data["content"])}, + spoiler_text: %{"text/plain" => HtmlSanitizeEx.strip_tags(object_data["summary"])} } } diff --git a/test/web/ostatus/activity_representer_test.exs b/test/web/ostatus/activity_representer_test.exs index 16ee02abb..a3a92ce5b 100644 --- a/test/web/ostatus/activity_representer_test.exs +++ b/test/web/ostatus/activity_representer_test.exs @@ -38,22 +38,23 @@ test "an external note activity" do test "a note activity" do note_activity = insert(:note_activity) + object_data = Object.normalize(note_activity).data user = User.get_cached_by_ap_id(note_activity.data["actor"]) expected = """