From eae9a6d34d88698ba19ba2461374cb60b1c4a33f Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 18 Sep 2019 18:13:21 +0300 Subject: [PATCH 1/9] Initial poll refresh support Implement refreshing the object with an interval and call the function when getting the poll. --- lib/pleroma/object.ex | 18 ++++ lib/pleroma/object/fetcher.ex | 17 +++- .../controllers/mastodon_api_controller.ex | 2 +- test/fixtures/tesla_mock/poll_modified.json | 1 + test/fixtures/tesla_mock/poll_original.json | 1 + test/fixtures/tesla_mock/rin.json | 1 + test/object_test.exs | 86 +++++++++++++++++++ test/support/http_request_mock.ex | 4 + 8 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/tesla_mock/poll_modified.json create mode 100644 test/fixtures/tesla_mock/poll_original.json create mode 100644 test/fixtures/tesla_mock/rin.json diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 5033798ae..640e068e5 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -38,6 +38,24 @@ defmodule Pleroma.Object do def get_by_id(nil), do: nil def get_by_id(id), do: Repo.get(Object, id) + def get_by_id_and_maybe_refetch(id, opts \\ []) do + %{updated_at: updated_at} = object = get_by_id(id) + + if opts[:interval] && + NaiveDateTime.diff(updated_at, NaiveDateTime.utc_now()) > opts[:interval] do + case Fetcher.refetch_object(object) do + {:ok, %Object{} = object} -> + object + + e -> + Logger.error("Couldn't refresh #{object.data["id"]}:\n#{inspect(e)}") + object + end + else + object + end + end + def get_by_ap_id(nil), do: nil def get_by_ap_id(ap_id) do diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index c1795ae0f..da1ebd8b3 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -7,17 +7,19 @@ defmodule Pleroma.Object.Fetcher do alias Pleroma.Object alias Pleroma.Object.Containment alias Pleroma.Signature + alias Pleroma.Repo alias Pleroma.Web.ActivityPub.InternalFetchActor alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.OStatus require Logger - defp reinject_object(data) do + defp reinject_object(struct, data) do Logger.debug("Reinjecting object #{data["id"]}") with data <- Transmogrifier.fix_object(data), - {:ok, object} <- Object.create(data) do + changeset <- Object.change(struct, %{data: data}), + {:ok, object} <- Repo.insert_or_update(changeset) do {:ok, object} else e -> @@ -26,6 +28,15 @@ defmodule Pleroma.Object.Fetcher do end end + def refetch_object(%Object{data: %{"id" => id}} = object) do + with {:ok, data} <- fetch_and_contain_remote_object_from_id(id), + {:ok, object} <- reinject_object(object, data) do + {:ok, object} + else + e -> {:error, e} + end + end + # TODO: # This will create a Create activity, which we need internally at the moment. def fetch_object_from_id(id, options \\ []) do @@ -57,7 +68,7 @@ defmodule Pleroma.Object.Fetcher do {:reject, nil} {:object, data, nil} -> - reinject_object(data) + reinject_object(%Object{}, data) {:normalize, object = %Object{}} -> {:ok, object} diff --git a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex index 93ca44d31..edc5e7214 100644 --- a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex @@ -485,7 +485,7 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do end def get_poll(%{assigns: %{user: user}} = conn, %{"id" => id}) do - with %Object{} = object <- Object.get_by_id(id), + with %Object{} = object <- Object.get_by_id_and_maybe_refetch(id, interval: 60), %Activity{} = activity <- Activity.get_create_by_object_ap_id(object.data["id"]), true <- Visibility.visible_for_user?(activity, user) do conn diff --git a/test/fixtures/tesla_mock/poll_modified.json b/test/fixtures/tesla_mock/poll_modified.json new file mode 100644 index 000000000..1d026b592 --- /dev/null +++ b/test/fixtures/tesla_mock/poll_modified.json @@ -0,0 +1 @@ +{"@context":["https://www.w3.org/ns/activitystreams","https://patch.cx/schemas/litepub-0.1.jsonld",{"@language":"und"}],"actor":"https://patch.cx/users/rin","attachment":[],"attributedTo":"https://patch.cx/users/rin","cc":["https://patch.cx/users/rin/followers"],"closed":"2019-09-19T00:32:36.785333","content":"can you vote on this poll?","context":"https://patch.cx/contexts/626ecafd-3377-46c4-b908-3721a4d4373c","conversation":"https://patch.cx/contexts/626ecafd-3377-46c4-b908-3721a4d4373c","id":"https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d","oneOf":[{"name":"yes","replies":{"totalItems":8,"type":"Collection"},"type":"Note"},{"name":"no","replies":{"totalItems":3,"type":"Collection"},"type":"Note"}],"published":"2019-09-18T14:32:36.802152Z","sensitive":false,"summary":"","tag":[],"to":["https://www.w3.org/ns/activitystreams#Public"],"type":"Question"} \ No newline at end of file diff --git a/test/fixtures/tesla_mock/poll_original.json b/test/fixtures/tesla_mock/poll_original.json new file mode 100644 index 000000000..267876b3c --- /dev/null +++ b/test/fixtures/tesla_mock/poll_original.json @@ -0,0 +1 @@ +{"@context":["https://www.w3.org/ns/activitystreams","https://patch.cx/schemas/litepub-0.1.jsonld",{"@language":"und"}],"actor":"https://patch.cx/users/rin","attachment":[],"attributedTo":"https://patch.cx/users/rin","cc":["https://patch.cx/users/rin/followers"],"closed":"2019-09-19T00:32:36.785333","content":"can you vote on this poll?","context":"https://patch.cx/contexts/626ecafd-3377-46c4-b908-3721a4d4373c","conversation":"https://patch.cx/contexts/626ecafd-3377-46c4-b908-3721a4d4373c","id":"https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d","oneOf":[{"name":"yes","replies":{"totalItems":4,"type":"Collection"},"type":"Note"},{"name":"no","replies":{"totalItems":0,"type":"Collection"},"type":"Note"}],"published":"2019-09-18T14:32:36.802152Z","sensitive":false,"summary":"","tag":[],"to":["https://www.w3.org/ns/activitystreams#Public"],"type":"Question"} \ No newline at end of file diff --git a/test/fixtures/tesla_mock/rin.json b/test/fixtures/tesla_mock/rin.json new file mode 100644 index 000000000..2cf623764 --- /dev/null +++ b/test/fixtures/tesla_mock/rin.json @@ -0,0 +1 @@ +{"@context":["https://www.w3.org/ns/activitystreams","https://patch.cx/schemas/litepub-0.1.jsonld",{"@language":"und"}],"attachment":[],"endpoints":{"oauthAuthorizationEndpoint":"https://patch.cx/oauth/authorize","oauthRegistrationEndpoint":"https://patch.cx/api/v1/apps","oauthTokenEndpoint":"https://patch.cx/oauth/token","sharedInbox":"https://patch.cx/inbox"},"followers":"https://patch.cx/users/rin/followers","following":"https://patch.cx/users/rin/following","icon":{"type":"Image","url":"https://patch.cx/media/4e914f5b84e4a259a3f6c2d2edc9ab642f2ab05f3e3d9c52c81fc2d984b3d51e.jpg"},"id":"https://patch.cx/users/rin","image":{"type":"Image","url":"https://patch.cx/media/f739efddefeee49c6e67e947c4811fdc911785c16ae43da4c3684051fbf8da6a.jpg?name=f739efddefeee49c6e67e947c4811fdc911785c16ae43da4c3684051fbf8da6a.jpg"},"inbox":"https://patch.cx/users/rin/inbox","manuallyApprovesFollowers":false,"name":"rinpatch","outbox":"https://patch.cx/users/rin/outbox","preferredUsername":"rin","publicKey":{"id":"https://patch.cx/users/rin#main-key","owner":"https://patch.cx/users/rin","publicKeyPem":"-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA5DLtwGXNZElJyxFGfcVc\nXANhaMadj/iYYQwZjOJTV9QsbtiNBeIK54PJrYuU0/0YIdrvS1iqheX5IwXRhcwa\nhm3ZyLz7XeN9st7FBni4BmZMBtMpxAuYuu5p/jbWy13qAiYOhPreCx0wrWgm/lBD\n9mkgaxIxPooBE0S4ZWEJIDIV1Vft3AWcRUyWW1vIBK0uZzs6GYshbQZB952S0yo4\nFzI1hABGHncH8UvuFauh4EZ8tY7/X5I0pGRnDOcRN1dAht5w5yTA+6r5kebiFQjP\nIzN/eCO/a9Flrj9YGW7HDNtjSOH0A31PLRGlJtJO3yK57dnf5ppyCZGfL4emShQo\ncQIDAQAB\n-----END PUBLIC KEY-----\n\n"},"summary":"your friendly neighborhood pleroma developer
I like cute things and distributed systems, and really hate delete and redrafts","tag":[],"type":"Person","url":"https://patch.cx/users/rin"} \ No newline at end of file diff --git a/test/object_test.exs b/test/object_test.exs index ba96aeea4..72e36316c 100644 --- a/test/object_test.exs +++ b/test/object_test.exs @@ -89,4 +89,90 @@ defmodule Pleroma.ObjectTest do ) end end + + describe "get_by_id_and_maybe_refetch" do + test "refetches if the time since the last refetch is greater than the interval" do + mock(fn + %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> + %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_original.json")} + + env -> + apply(HttpRequestMock, :request, [env]) + end) + + %Object{} = + object = Object.normalize("https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d") + + assert Enum.at(object.data["oneOf"], 0)["replies"]["totalItems"] == 4 + assert Enum.at(object.data["oneOf"], 1)["replies"]["totalItems"] == 0 + + mock(fn + %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> + %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_modified.json")} + + env -> + apply(HttpRequestMock, :request, [env]) + end) + + updated_object = Object.get_by_id_and_maybe_refetch(object.id, interval: -1) + assert Enum.at(updated_object.data["oneOf"], 0)["replies"]["totalItems"] == 8 + assert Enum.at(updated_object.data["oneOf"], 1)["replies"]["totalItems"] == 3 + end + + test "returns the old object if refetch fails" do + mock(fn + %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> + %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_original.json")} + + env -> + apply(HttpRequestMock, :request, [env]) + end) + + %Object{} = + object = Object.normalize("https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d") + + assert Enum.at(object.data["oneOf"], 0)["replies"]["totalItems"] == 4 + assert Enum.at(object.data["oneOf"], 1)["replies"]["totalItems"] == 0 + + mock(fn + %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> + %Tesla.Env{status: 404, body: ""} + + env -> + apply(HttpRequestMock, :request, [env]) + end) + + updated_object = Object.get_by_id_and_maybe_refetch(object.id, interval: -1) + assert Enum.at(updated_object.data["oneOf"], 0)["replies"]["totalItems"] == 4 + assert Enum.at(updated_object.data["oneOf"], 1)["replies"]["totalItems"] == 0 + end + + test "does not refetch if the time since the last refetch is greater than the interval" do + mock(fn + %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> + %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_original.json")} + + env -> + apply(HttpRequestMock, :request, [env]) + end) + + %Object{} = + object = Object.normalize("https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d") + + assert Enum.at(object.data["oneOf"], 0)["replies"]["totalItems"] == 4 + assert Enum.at(object.data["oneOf"], 1)["replies"]["totalItems"] == 0 + + mock(fn + %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> + %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_modified.json")} + + env -> + apply(HttpRequestMock, :request, [env]) + end) + + updated_object = Object.get_by_id_and_maybe_refetch(object.id, interval: 100) + assert Enum.at(updated_object.data["oneOf"], 0)["replies"]["totalItems"] == 4 + assert Enum.at(updated_object.data["oneOf"], 1)["replies"]["totalItems"] == 0 + end + end end diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index 231e7c498..833162a61 100644 --- a/test/support/http_request_mock.ex +++ b/test/support/http_request_mock.ex @@ -1004,6 +1004,10 @@ defmodule HttpRequestMock do {:ok, %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/sjw.json")}} end + def get("https://patch.cx/users/rin", _, _, _) do + {:ok, %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/rin.json")}} + end + def get(url, query, body, headers) do {:error, "Mock response not implemented for GET #{inspect(url)}, #{query}, #{inspect(body)}, #{ From f95ae0e50adc0b7f9bf90f61a2c884ffa431521c Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 18 Sep 2019 18:31:24 +0300 Subject: [PATCH 2/9] Capture log in object refetch tests --- test/object_test.exs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/test/object_test.exs b/test/object_test.exs index 72e36316c..25e8d45d4 100644 --- a/test/object_test.exs +++ b/test/object_test.exs @@ -4,6 +4,7 @@ defmodule Pleroma.ObjectTest do use Pleroma.DataCase + import ExUnit.CaptureLog import Pleroma.Factory import Tesla.Mock alias Pleroma.Object @@ -134,17 +135,23 @@ defmodule Pleroma.ObjectTest do assert Enum.at(object.data["oneOf"], 0)["replies"]["totalItems"] == 4 assert Enum.at(object.data["oneOf"], 1)["replies"]["totalItems"] == 0 - mock(fn - %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> - %Tesla.Env{status: 404, body: ""} + assert capture_log(fn -> + mock(fn + %{ + method: :get, + url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d" + } -> + %Tesla.Env{status: 404, body: ""} - env -> - apply(HttpRequestMock, :request, [env]) - end) + env -> + apply(HttpRequestMock, :request, [env]) + end) - updated_object = Object.get_by_id_and_maybe_refetch(object.id, interval: -1) - assert Enum.at(updated_object.data["oneOf"], 0)["replies"]["totalItems"] == 4 - assert Enum.at(updated_object.data["oneOf"], 1)["replies"]["totalItems"] == 0 + updated_object = Object.get_by_id_and_maybe_refetch(object.id, interval: -1) + assert Enum.at(updated_object.data["oneOf"], 0)["replies"]["totalItems"] == 4 + assert Enum.at(updated_object.data["oneOf"], 1)["replies"]["totalItems"] == 0 + end) =~ + "[error] Couldn't refresh https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d" end test "does not refetch if the time since the last refetch is greater than the interval" do From 8be9a9b5bbc3f722c708272dfe744d35db66dc66 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 18 Sep 2019 18:52:33 +0300 Subject: [PATCH 3/9] Fix wrong argument order when calling NaiveDateTime.diff --- lib/pleroma/object.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 640e068e5..3fa407931 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -42,7 +42,7 @@ defmodule Pleroma.Object do %{updated_at: updated_at} = object = get_by_id(id) if opts[:interval] && - NaiveDateTime.diff(updated_at, NaiveDateTime.utc_now()) > opts[:interval] do + NaiveDateTime.diff(NaiveDateTime.utc_now(), updated_at) > opts[:interval] do case Fetcher.refetch_object(object) do {:ok, %Object{} = object} -> object From b9cdead0765bde6bc17ead31992529ea44037aa6 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 18 Sep 2019 19:07:25 +0300 Subject: [PATCH 4/9] Set updated_at even if the object stayed the same --- lib/pleroma/object/fetcher.ex | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index da1ebd8b3..786e31cce 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -14,11 +14,20 @@ defmodule Pleroma.Object.Fetcher do require Logger + defp touch_changeset(changeset) do + updated_at = + NaiveDateTime.utc_now() + |> NaiveDateTime.truncate(:second) + + Ecto.Changeset.put_change(changeset, :updated_at, updated_at) + end + defp reinject_object(struct, data) do Logger.debug("Reinjecting object #{data["id"]}") with data <- Transmogrifier.fix_object(data), changeset <- Object.change(struct, %{data: data}), + changeset <- touch_changeset(changeset), {:ok, object} <- Repo.insert_or_update(changeset) do {:ok, object} else From 845e8ad5f62d92666403c4e7c12d590b514c4bc5 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 18 Sep 2019 19:24:20 +0300 Subject: [PATCH 5/9] Move object internal fields to a constant --- lib/pleroma/constants.ex | 12 ++++++++++++ lib/pleroma/web/activity_pub/transmogrifier.ex | 10 +--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/constants.ex b/lib/pleroma/constants.ex index ef1418543..0bf20cdd0 100644 --- a/lib/pleroma/constants.ex +++ b/lib/pleroma/constants.ex @@ -6,4 +6,16 @@ defmodule Pleroma.Constants do use Const const(as_public, do: "https://www.w3.org/ns/activitystreams#Public") + + const(object_internal_fields, + do: [ + "likes", + "like_count", + "announcements", + "announcement_count", + "emoji", + "context_id", + "deleted_activity_id" + ] + ) end diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index c3d071393..1c67fee2e 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -997,15 +997,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do defp strip_internal_fields(object) do object - |> Map.drop([ - "likes", - "like_count", - "announcements", - "announcement_count", - "emoji", - "context_id", - "deleted_activity_id" - ]) + |> Map.drop(Pleroma.Constants.object_internal_fields()) end defp strip_internal_tags(%{"tag" => tags} = object) do From 644bc1e64a81408be6ffc82b063e19ffb9a9a1a6 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 18 Sep 2019 19:53:51 +0300 Subject: [PATCH 6/9] Preserve internal fields when reinjecting --- lib/pleroma/object/fetcher.ex | 10 ++++ test/object_test.exs | 102 ++++++++++++++++++++-------------- 2 files changed, 69 insertions(+), 43 deletions(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 786e31cce..fecc97c5e 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -13,6 +13,7 @@ defmodule Pleroma.Object.Fetcher do alias Pleroma.Web.OStatus require Logger + require Pleroma.Constants defp touch_changeset(changeset) do updated_at = @@ -22,10 +23,19 @@ defmodule Pleroma.Object.Fetcher do Ecto.Changeset.put_change(changeset, :updated_at, updated_at) end + defp maybe_reinject_internal_fields(data, %{data: %{} = old_data}) do + internal_fields = Map.take(old_data, Pleroma.Constants.object_internal_fields()) + + Map.merge(data, internal_fields) + end + + defp maybe_reinject_internal_fields(data, _), do: data + defp reinject_object(struct, data) do Logger.debug("Reinjecting object #{data["id"]}") with data <- Transmogrifier.fix_object(data), + data <- maybe_reinject_internal_fields(data, struct), changeset <- Object.change(struct, %{data: data}), changeset <- touch_changeset(changeset), {:ok, object} <- Repo.insert_or_update(changeset) do diff --git a/test/object_test.exs b/test/object_test.exs index 25e8d45d4..3d64fdb49 100644 --- a/test/object_test.exs +++ b/test/object_test.exs @@ -7,8 +7,10 @@ defmodule Pleroma.ObjectTest do import ExUnit.CaptureLog import Pleroma.Factory import Tesla.Mock + alias Pleroma.Activity alias Pleroma.Object alias Pleroma.Repo + alias Pleroma.Web.CommonAPI setup do mock(fn env -> apply(HttpRequestMock, :request, [env]) end) @@ -92,7 +94,7 @@ defmodule Pleroma.ObjectTest do end describe "get_by_id_and_maybe_refetch" do - test "refetches if the time since the last refetch is greater than the interval" do + setup do mock(fn %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_original.json")} @@ -101,34 +103,41 @@ defmodule Pleroma.ObjectTest do apply(HttpRequestMock, :request, [env]) end) + mock_modified = fn resp -> + mock(fn + %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> + resp + + env -> + apply(HttpRequestMock, :request, [env]) + end) + end + + on_exit(fn -> mock(fn env -> apply(HttpRequestMock, :request, [env]) end) end) + + [mock_modified: mock_modified] + end + + test "refetches if the time since the last refetch is greater than the interval", %{ + mock_modified: mock_modified + } do %Object{} = object = Object.normalize("https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d") assert Enum.at(object.data["oneOf"], 0)["replies"]["totalItems"] == 4 assert Enum.at(object.data["oneOf"], 1)["replies"]["totalItems"] == 0 - mock(fn - %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> - %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_modified.json")} - - env -> - apply(HttpRequestMock, :request, [env]) - end) + mock_modified.(%Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/tesla_mock/poll_modified.json") + }) updated_object = Object.get_by_id_and_maybe_refetch(object.id, interval: -1) assert Enum.at(updated_object.data["oneOf"], 0)["replies"]["totalItems"] == 8 assert Enum.at(updated_object.data["oneOf"], 1)["replies"]["totalItems"] == 3 end - test "returns the old object if refetch fails" do - mock(fn - %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> - %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_original.json")} - - env -> - apply(HttpRequestMock, :request, [env]) - end) - + test "returns the old object if refetch fails", %{mock_modified: mock_modified} do %Object{} = object = Object.normalize("https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d") @@ -136,16 +145,7 @@ defmodule Pleroma.ObjectTest do assert Enum.at(object.data["oneOf"], 1)["replies"]["totalItems"] == 0 assert capture_log(fn -> - mock(fn - %{ - method: :get, - url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d" - } -> - %Tesla.Env{status: 404, body: ""} - - env -> - apply(HttpRequestMock, :request, [env]) - end) + mock_modified.(%Tesla.Env{status: 404, body: ""}) updated_object = Object.get_by_id_and_maybe_refetch(object.id, interval: -1) assert Enum.at(updated_object.data["oneOf"], 0)["replies"]["totalItems"] == 4 @@ -154,32 +154,48 @@ defmodule Pleroma.ObjectTest do "[error] Couldn't refresh https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d" end - test "does not refetch if the time since the last refetch is greater than the interval" do - mock(fn - %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> - %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_original.json")} - - env -> - apply(HttpRequestMock, :request, [env]) - end) - + test "does not refetch if the time since the last refetch is greater than the interval", %{ + mock_modified: mock_modified + } do %Object{} = object = Object.normalize("https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d") assert Enum.at(object.data["oneOf"], 0)["replies"]["totalItems"] == 4 assert Enum.at(object.data["oneOf"], 1)["replies"]["totalItems"] == 0 - mock(fn - %{method: :get, url: "https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d"} -> - %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/poll_modified.json")} - - env -> - apply(HttpRequestMock, :request, [env]) - end) + mock_modified.(%Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/tesla_mock/poll_modified.json") + }) updated_object = Object.get_by_id_and_maybe_refetch(object.id, interval: 100) assert Enum.at(updated_object.data["oneOf"], 0)["replies"]["totalItems"] == 4 assert Enum.at(updated_object.data["oneOf"], 1)["replies"]["totalItems"] == 0 end + + test "preserves internal fields on refetch", %{mock_modified: mock_modified} do + %Object{} = + object = Object.normalize("https://patch.cx/objects/9a172665-2bc5-452d-8428-2361d4c33b1d") + + assert Enum.at(object.data["oneOf"], 0)["replies"]["totalItems"] == 4 + assert Enum.at(object.data["oneOf"], 1)["replies"]["totalItems"] == 0 + + user = insert(:user) + activity = Activity.get_create_by_object_ap_id(object.data["id"]) + {:ok, _activity, object} = CommonAPI.favorite(activity.id, user) + + assert object.data["like_count"] == 1 + + mock_modified.(%Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/tesla_mock/poll_modified.json") + }) + + updated_object = Object.get_by_id_and_maybe_refetch(object.id, interval: -1) + assert Enum.at(updated_object.data["oneOf"], 0)["replies"]["totalItems"] == 8 + assert Enum.at(updated_object.data["oneOf"], 1)["replies"]["totalItems"] == 3 + + assert updated_object.data["like_count"] == 1 + end end end From 40c7d6fa173b4c32e6cec9a85f67c050ce20bd25 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 18 Sep 2019 19:59:23 +0300 Subject: [PATCH 7/9] Do not refetch local objects --- lib/pleroma/object/fetcher.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index fecc97c5e..91e6b6dca 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -48,10 +48,12 @@ defmodule Pleroma.Object.Fetcher do end def refetch_object(%Object{data: %{"id" => id}} = object) do - with {:ok, data} <- fetch_and_contain_remote_object_from_id(id), + with {:local, false} <- {:local, String.starts_with?(id, Pleroma.Web.base_url() <> "/")}, + {:ok, data} <- fetch_and_contain_remote_object_from_id(id), {:ok, object} <- reinject_object(object, data) do {:ok, object} else + {:local, true} -> object e -> {:error, e} end end From c7313c8dfbdc4022db56a77c5e8de6529d52e765 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 18 Sep 2019 22:09:03 +0300 Subject: [PATCH 8/9] Fix credo issues --- lib/pleroma/object/fetcher.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 91e6b6dca..cea33b5af 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -6,8 +6,8 @@ defmodule Pleroma.Object.Fetcher do alias Pleroma.HTTP alias Pleroma.Object alias Pleroma.Object.Containment - alias Pleroma.Signature alias Pleroma.Repo + alias Pleroma.Signature alias Pleroma.Web.ActivityPub.InternalFetchActor alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.OStatus From bab901bfd7285e4f2ae76a686cf4edb8a3dc57c1 Mon Sep 17 00:00:00 2001 From: Ariadne Conill Date: Sun, 6 Oct 2019 14:54:44 +0000 Subject: [PATCH 9/9] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79f867e62..b65706b29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: Blocks are now treated consistently between the Streaming API and the Timeline APIs - ActivityPub: Correct addressing of Undo. - ActivityPub: Correct addressing of profile update activities. +- ActivityPub: Polls are now refreshed when necessary. - Mastodon API: Ensure the `account` field is not empty when rendering Notification entities. ### Removed