diff options
| -rw-r--r-- | changelog.d/3883.fix | 1 | ||||
| -rw-r--r-- | lib/pleroma/object/fetcher.ex | 86 | ||||
| -rw-r--r-- | lib/pleroma/object/updater.ex | 48 | ||||
| -rw-r--r-- | lib/pleroma/web/activity_pub/side_effects.ex | 36 | ||||
| -rw-r--r-- | test/pleroma/object/fetcher_test.exs | 84 | 
5 files changed, 144 insertions, 111 deletions
diff --git a/changelog.d/3883.fix b/changelog.d/3883.fix new file mode 100644 index 000000000..6824f2013 --- /dev/null +++ b/changelog.d/3883.fix @@ -0,0 +1 @@ +Fix abnormal behaviour when refetching a poll diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index a9a9eeeed..cc3772563 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -8,77 +8,30 @@ defmodule Pleroma.Object.Fetcher do    alias Pleroma.Maps    alias Pleroma.Object    alias Pleroma.Object.Containment -  alias Pleroma.Repo    alias Pleroma.Signature    alias Pleroma.Web.ActivityPub.InternalFetchActor +  alias Pleroma.Web.ActivityPub.MRF    alias Pleroma.Web.ActivityPub.ObjectValidator +  alias Pleroma.Web.ActivityPub.Pipeline    alias Pleroma.Web.ActivityPub.Transmogrifier    alias Pleroma.Web.Federator    require Logger    require Pleroma.Constants -  defp touch_changeset(changeset) do -    updated_at = -      NaiveDateTime.utc_now() -      |> NaiveDateTime.truncate(:second) - -    Ecto.Changeset.put_change(changeset, :updated_at, updated_at) -  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) - -        %{updated_object: updated_object} = -          new_data -          |> Object.Updater.maybe_update_history(old_data, -            updated: changed?, -            use_history_in_new_object?: false -          ) - -        updated_object -      else -        new_data -      end - -    Map.merge(new_data, internal_fields) -  end - -  defp maybe_reinject_internal_fields(_, new_data), do: new_data -    @spec reinject_object(struct(), map()) :: {:ok, Object.t()} | {:error, any()} -  defp reinject_object(%Object{data: %{"type" => "Question"}} = object, new_data) do +  defp reinject_object(%Object{data: %{}} = object, new_data) do      Logger.debug("Reinjecting object #{new_data["id"]}") -    with data <- maybe_reinject_internal_fields(object, new_data), -         {:ok, data, _} <- ObjectValidator.validate(data, %{}), -         changeset <- Object.change(object, %{data: data}), -         changeset <- touch_changeset(changeset), -         {:ok, object} <- Repo.insert_or_update(changeset), -         {:ok, object} <- Object.set_cache(object) do -      {:ok, object} +    with {:ok, new_data, _} <- ObjectValidator.validate(new_data, %{}), +         {:ok, new_data} <- MRF.filter(new_data), +         {:ok, new_object, _} <- +           Object.Updater.do_update_and_invalidate_cache( +             object, +             new_data, +             _touch_changeset? = true +           ) do +      {:ok, new_object}      else        e ->          Logger.error("Error while processing object: #{inspect(e)}") @@ -86,20 +39,11 @@ defmodule Pleroma.Object.Fetcher do      end    end -  defp reinject_object(%Object{} = object, new_data) do -    Logger.debug("Reinjecting object #{new_data["id"]}") - -    with new_data <- Transmogrifier.fix_object(new_data), -         data <- maybe_reinject_internal_fields(object, new_data), -         changeset <- Object.change(object, %{data: data}), -         changeset <- touch_changeset(changeset), -         {:ok, object} <- Repo.insert_or_update(changeset), -         {:ok, object} <- Object.set_cache(object) do +  defp reinject_object(_, new_data) do +    with {:ok, object, _} <- Pipeline.common_pipeline(new_data, local: false) do        {:ok, object}      else -      e -> -        Logger.error("Error while processing object: #{inspect(e)}") -        {:error, e} +      e -> e      end    end diff --git a/lib/pleroma/object/updater.ex b/lib/pleroma/object/updater.ex index ab38d3ed2..bad811965 100644 --- a/lib/pleroma/object/updater.ex +++ b/lib/pleroma/object/updater.ex @@ -5,6 +5,9 @@  defmodule Pleroma.Object.Updater do    require Pleroma.Constants +  alias Pleroma.Object +  alias Pleroma.Repo +    def update_content_fields(orig_object_data, updated_object) do      Pleroma.Constants.status_updatable_fields()      |> Enum.reduce( @@ -237,4 +240,49 @@ defmodule Pleroma.Object.Updater do        {:history_items, e} -> e      end    end + +  defp maybe_touch_changeset(changeset, true) do +    updated_at = +      NaiveDateTime.utc_now() +      |> NaiveDateTime.truncate(:second) + +    Ecto.Changeset.put_change(changeset, :updated_at, updated_at) +  end + +  defp maybe_touch_changeset(changeset, _), do: changeset + +  def do_update_and_invalidate_cache(orig_object, updated_object, touch_changeset? \\ false) do +    orig_object_ap_id = updated_object["id"] +    orig_object_data = orig_object.data + +    %{ +      updated_data: updated_object_data, +      updated: updated, +      used_history_in_new_object?: used_history_in_new_object? +    } = make_new_object_data_from_update_object(orig_object_data, updated_object) + +    changeset = +      orig_object +      |> Repo.preload(:hashtags) +      |> Object.change(%{data: updated_object_data}) +      |> maybe_touch_changeset(touch_changeset?) + +    with {:ok, new_object} <- Repo.update(changeset), +         {:ok, _} <- Object.invalid_object_cache(new_object), +         {:ok, _} <- Object.set_cache(new_object), +         # The metadata/utils.ex uses the object id for the cache. +         {:ok, _} <- Pleroma.Activity.HTML.invalidate_cache_for(new_object.id) do +      if used_history_in_new_object? do +        with create_activity when not is_nil(create_activity) <- +               Pleroma.Activity.get_create_by_object_ap_id(orig_object_ap_id), +             {:ok, _} <- Pleroma.Activity.HTML.invalidate_cache_for(create_activity.id) do +          nil +        else +          _ -> nil +        end +      end + +      {:ok, new_object, updated} +    end +  end  end diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index e19642d50..098c177c7 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -428,37 +428,13 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do        end      if orig_object_data["type"] in Pleroma.Constants.updatable_object_types() do -      %{ -        updated_data: updated_object_data, -        updated: updated, -        used_history_in_new_object?: used_history_in_new_object? -      } = Object.Updater.make_new_object_data_from_update_object(orig_object_data, updated_object) - -      changeset = -        orig_object -        |> Repo.preload(:hashtags) -        |> Object.change(%{data: updated_object_data}) - -      with {:ok, new_object} <- Repo.update(changeset), -           {:ok, _} <- Object.invalid_object_cache(new_object), -           {:ok, _} <- Object.set_cache(new_object), -           # The metadata/utils.ex uses the object id for the cache. -           {:ok, _} <- Pleroma.Activity.HTML.invalidate_cache_for(new_object.id) do -        if used_history_in_new_object? do -          with create_activity when not is_nil(create_activity) <- -                 Pleroma.Activity.get_create_by_object_ap_id(orig_object_ap_id), -               {:ok, _} <- Pleroma.Activity.HTML.invalidate_cache_for(create_activity.id) do -            nil -          else -            _ -> nil -          end -        end +      {:ok, _, updated} = +        Object.Updater.do_update_and_invalidate_cache(orig_object, updated_object) -        if updated do -          object -          |> Activity.normalize() -          |> ActivityPub.notify_and_stream() -        end +      if updated do +        object +        |> Activity.normalize() +        |> ActivityPub.notify_and_stream()        end      end diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index c8ad66ddb..53c9277d6 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -9,8 +9,12 @@ defmodule Pleroma.Object.FetcherTest do    alias Pleroma.Instances    alias Pleroma.Object    alias Pleroma.Object.Fetcher +  alias Pleroma.Web.ActivityPub.ObjectValidator + +  require Pleroma.Constants    import Mock +  import Pleroma.Factory    import Tesla.Mock    setup do @@ -284,6 +288,8 @@ defmodule Pleroma.Object.FetcherTest do    describe "refetching" do      setup do +      insert(:user, ap_id: "https://mastodon.social/users/emelie") +        object1 = %{          "id" => "https://mastodon.social/1",          "actor" => "https://mastodon.social/users/emelie", @@ -293,10 +299,14 @@ defmodule Pleroma.Object.FetcherTest do          "bcc" => [],          "bto" => [],          "cc" => [], -        "to" => [], -        "summary" => "" +        "to" => [Pleroma.Constants.as_public()], +        "summary" => "", +        "published" => "2023-05-08 23:43:20Z", +        "updated" => "2023-05-09 23:43:20Z"        } +      {:ok, local_object1, _} = ObjectValidator.validate(object1, []) +        object2 = %{          "id" => "https://mastodon.social/2",          "actor" => "https://mastodon.social/users/emelie", @@ -306,8 +316,10 @@ defmodule Pleroma.Object.FetcherTest do          "bcc" => [],          "bto" => [],          "cc" => [], -        "to" => [], +        "to" => [Pleroma.Constants.as_public()],          "summary" => "", +        "published" => "2023-05-08 23:43:20Z", +        "updated" => "2023-05-09 23:43:25Z",          "formerRepresentations" => %{            "type" => "OrderedCollection",            "orderedItems" => [ @@ -319,14 +331,18 @@ defmodule Pleroma.Object.FetcherTest do                "bcc" => [],                "bto" => [],                "cc" => [], -              "to" => [], -              "summary" => "" +              "to" => [Pleroma.Constants.as_public()], +              "summary" => "", +              "published" => "2023-05-08 23:43:20Z", +              "updated" => "2023-05-09 23:43:21Z"              }            ],            "totalItems" => 1          }        } +      {:ok, local_object2, _} = ObjectValidator.validate(object2, []) +        mock(fn          %{            method: :get, @@ -335,7 +351,7 @@ defmodule Pleroma.Object.FetcherTest do            %Tesla.Env{              status: 200,              headers: [{"content-type", "application/activity+json"}], -            body: Jason.encode!(object1) +            body: Jason.encode!(object1 |> Map.put("updated", "2023-05-09 23:44:20Z"))            }          %{ @@ -345,7 +361,7 @@ defmodule Pleroma.Object.FetcherTest do            %Tesla.Env{              status: 200,              headers: [{"content-type", "application/activity+json"}], -            body: Jason.encode!(object2) +            body: Jason.encode!(object2 |> Map.put("updated", "2023-05-09 23:44:20Z"))            }          %{ @@ -370,7 +386,7 @@ defmodule Pleroma.Object.FetcherTest do            apply(HttpRequestMock, :request, [env])        end) -      %{object1: object1, object2: object2} +      %{object1: local_object1, object2: local_object2}      end      test "it keeps formerRepresentations if remote does not have this attr", %{object1: object1} do @@ -388,8 +404,9 @@ defmodule Pleroma.Object.FetcherTest do                  "bcc" => [],                  "bto" => [],                  "cc" => [], -                "to" => [], -                "summary" => "" +                "to" => [Pleroma.Constants.as_public()], +                "summary" => "", +                "published" => "2023-05-08 23:43:20Z"                }              ],              "totalItems" => 1 @@ -467,6 +484,53 @@ defmodule Pleroma.Object.FetcherTest do                 }               } = refetched.data      end + +    test "it keeps the history intact if only updated time has changed", +         %{object1: object1} do +      full_object1 = +        object1 +        |> Map.merge(%{ +          "updated" => "2023-05-08 23:43:47Z", +          "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 1"} +                 ], +                 "totalItems" => 1 +               } +             } = refetched.data +    end + +    test "it goes through ObjectValidator and MRF", %{object2: object2} do +      with_mock Pleroma.Web.ActivityPub.MRF, [:passthrough], +        filter: fn +          %{"type" => "Note"} = object -> +            {:ok, Map.put(object, "content", "MRFd content")} + +          arg -> +            passthrough([arg]) +        end do +        {:ok, o} = Object.create(object2) + +        assert {:ok, refetched} = Fetcher.refetch_object(o) + +        assert %{"content" => "MRFd content"} = refetched.data +      end +    end    end    describe "fetch with history" do  | 
