Don't return garbage on failed collection fetches
And for now treat partial fetches as a success, since for all current users partial collection data is better than no data at all. If an error occurred while fetching a page, this previously returned a bogus {:ok, {:error, _}} success, causing the error to be attached to the object as an reply list subsequently leading to the whole post getting rejected during validation. Also the pinned collection caller did not actually handle the preexisting error case resulting in process crashes.
This commit is contained in:
parent
4701aa2a38
commit
46148c0825
2 changed files with 29 additions and 18 deletions
|
@ -14,7 +14,7 @@ defmodule Akkoma.Collections.Fetcher do
|
||||||
@spec fetch_collection(String.t() | map()) :: {:ok, [Pleroma.Object.t()]} | {:error, any()}
|
@spec fetch_collection(String.t() | map()) :: {:ok, [Pleroma.Object.t()]} | {:error, any()}
|
||||||
def fetch_collection(ap_id) when is_binary(ap_id) do
|
def fetch_collection(ap_id) when is_binary(ap_id) do
|
||||||
with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(ap_id) do
|
with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(ap_id) do
|
||||||
{:ok, objects_from_collection(page)}
|
partial_as_success(objects_from_collection(page))
|
||||||
else
|
else
|
||||||
e ->
|
e ->
|
||||||
Logger.error("Could not fetch collection #{ap_id} - #{inspect(e)}")
|
Logger.error("Could not fetch collection #{ap_id} - #{inspect(e)}")
|
||||||
|
@ -24,9 +24,12 @@ def fetch_collection(ap_id) when is_binary(ap_id) do
|
||||||
|
|
||||||
def fetch_collection(%{"type" => type} = page)
|
def fetch_collection(%{"type" => type} = page)
|
||||||
when type in ["Collection", "OrderedCollection", "CollectionPage", "OrderedCollectionPage"] do
|
when type in ["Collection", "OrderedCollection", "CollectionPage", "OrderedCollectionPage"] do
|
||||||
{:ok, objects_from_collection(page)}
|
partial_as_success(objects_from_collection(page))
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp partial_as_success({:partial, items}), do: {:ok, items}
|
||||||
|
defp partial_as_success(res), do: res
|
||||||
|
|
||||||
defp items_in_page(%{"type" => type, "orderedItems" => items})
|
defp items_in_page(%{"type" => type, "orderedItems" => items})
|
||||||
when is_list(items) and type in ["OrderedCollection", "OrderedCollectionPage"],
|
when is_list(items) and type in ["OrderedCollection", "OrderedCollectionPage"],
|
||||||
do: items
|
do: items
|
||||||
|
@ -53,11 +56,11 @@ defp objects_from_collection(%{"type" => type, "first" => %{"id" => id}})
|
||||||
fetch_page_items(id)
|
fetch_page_items(id)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp objects_from_collection(_page), do: []
|
defp objects_from_collection(_page), do: {:ok, []}
|
||||||
|
|
||||||
defp fetch_page_items(id, items \\ []) do
|
defp fetch_page_items(id, items \\ []) do
|
||||||
if Enum.count(items) >= Config.get([:activitypub, :max_collection_objects]) do
|
if Enum.count(items) >= Config.get([:activitypub, :max_collection_objects]) do
|
||||||
items
|
{:ok, items}
|
||||||
else
|
else
|
||||||
with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(id) do
|
with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(id) do
|
||||||
objects = items_in_page(page)
|
objects = items_in_page(page)
|
||||||
|
@ -65,18 +68,22 @@ defp fetch_page_items(id, items \\ []) do
|
||||||
if Enum.count(objects) > 0 do
|
if Enum.count(objects) > 0 do
|
||||||
maybe_next_page(page, items ++ objects)
|
maybe_next_page(page, items ++ objects)
|
||||||
else
|
else
|
||||||
items
|
{:ok, items}
|
||||||
end
|
end
|
||||||
else
|
else
|
||||||
{:error, :not_found} ->
|
{:error, :not_found} ->
|
||||||
items
|
{:ok, items}
|
||||||
|
|
||||||
{:error, :forbidden} ->
|
{:error, :forbidden} ->
|
||||||
items
|
{:ok, items}
|
||||||
|
|
||||||
{:error, error} ->
|
{:error, error} ->
|
||||||
Logger.error("Could not fetch page #{id} - #{inspect(error)}")
|
Logger.error("Could not fetch page #{id} - #{inspect(error)}")
|
||||||
{:error, error}
|
|
||||||
|
case items do
|
||||||
|
[] -> {:error, error}
|
||||||
|
_ -> {:partial, items}
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -85,5 +92,5 @@ defp maybe_next_page(%{"next" => id}, items) when is_binary(id) do
|
||||||
fetch_page_items(id, items)
|
fetch_page_items(id, items)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp maybe_next_page(_, items), do: items
|
defp maybe_next_page(_, items), do: {:ok, items}
|
||||||
end
|
end
|
||||||
|
|
|
@ -1797,7 +1797,7 @@ def pin_data_from_featured_collection(%{
|
||||||
else
|
else
|
||||||
e ->
|
e ->
|
||||||
Logger.error("Could not decode featured collection at fetch #{first}, #{inspect(e)}")
|
Logger.error("Could not decode featured collection at fetch #{first}, #{inspect(e)}")
|
||||||
{:ok, %{}}
|
%{}
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -1807,14 +1807,18 @@ def pin_data_from_featured_collection(
|
||||||
} = collection
|
} = collection
|
||||||
)
|
)
|
||||||
when type in ["OrderedCollection", "Collection"] do
|
when type in ["OrderedCollection", "Collection"] do
|
||||||
{:ok, objects} = Collections.Fetcher.fetch_collection(collection)
|
with {:ok, objects} <- Collections.Fetcher.fetch_collection(collection) do
|
||||||
|
|
||||||
# Items can either be a map _or_ a string
|
# Items can either be a map _or_ a string
|
||||||
objects
|
objects
|
||||||
|> Map.new(fn
|
|> Map.new(fn
|
||||||
ap_id when is_binary(ap_id) -> {ap_id, NaiveDateTime.utc_now()}
|
ap_id when is_binary(ap_id) -> {ap_id, NaiveDateTime.utc_now()}
|
||||||
%{"id" => object_ap_id} -> {object_ap_id, NaiveDateTime.utc_now()}
|
%{"id" => object_ap_id} -> {object_ap_id, NaiveDateTime.utc_now()}
|
||||||
end)
|
end)
|
||||||
|
else
|
||||||
|
e ->
|
||||||
|
Logger.warning("Failed to fetch featured collection #{collection}, #{inspect(e)}")
|
||||||
|
%{}
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def pin_data_from_featured_collection(obj) do
|
def pin_data_from_featured_collection(obj) do
|
||||||
|
|
Loading…
Reference in a new issue