From: lain Date: Wed, 2 Sep 2020 17:09:55 +0000 (+0000) Subject: Merge branch 'fix/1991-tzdata-update' into 'develop' X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=0ab03e856480c35ce9478d086f103b396a8e7453;hp=a11f23c130331d98db941c3edcc4d2dcf139bbc6;p=akkoma Merge branch 'fix/1991-tzdata-update' into 'develop' user agent if Endpoint is not started yet Closes #1991 See merge request pleroma/pleroma!2945 --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 07bc6d77c..54685cfb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## unreleased-patch - ??? +### Added +- Rich media failure tracking (along with `:failure_backoff` option) + ### Fixed - Mastodon API: Search parameter `following` now correctly returns the followings rather than the followers +- Mastodon API: Timelines hanging for (`number of posts with links * rich media timeout`) in the worst case. + Reduced to just rich media timeout. ## [2.1.0] - 2020-08-28 diff --git a/config/config.exs b/config/config.exs index ea8869d48..ed37b93c0 100644 --- a/config/config.exs +++ b/config/config.exs @@ -412,6 +412,7 @@ config :pleroma, :rich_media, Pleroma.Web.RichMedia.Parsers.TwitterCard, Pleroma.Web.RichMedia.Parsers.OEmbed ], + failure_backoff: 60_000, ttl_setters: [Pleroma.Web.RichMedia.Parser.TTL.AwsSignedUrl] config :pleroma, :media_proxy, diff --git a/config/description.exs b/config/description.exs index 29a657333..5e08ba109 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2385,6 +2385,13 @@ config :pleroma, :config_description, [ suggestions: [ Pleroma.Web.RichMedia.Parser.TTL.AwsSignedUrl ] + }, + %{ + key: :failure_backoff, + type: :integer, + description: + "Amount of milliseconds after request failure, during which the request will not be retried.", + suggestions: [60_000] } ] }, diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 2f440adf4..a9a650fab 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -361,6 +361,7 @@ config :pleroma, Pleroma.Web.MediaProxy.Invalidation.Http, * `ignore_hosts`: list of hosts which will be ignored by the metadata parser. For example `["accounts.google.com", "xss.website"]`, defaults to `[]`. * `ignore_tld`: list TLDs (top-level domains) which will ignore for parse metadata. default is ["local", "localdomain", "lan"]. * `parsers`: list of Rich Media parsers. +* `failure_backoff`: Amount of milliseconds after request failure, during which the request will not be retried. ## HTTP server diff --git a/lib/pleroma/html.ex b/lib/pleroma/html.ex index dc1b9b840..20b02f091 100644 --- a/lib/pleroma/html.ex +++ b/lib/pleroma/html.ex @@ -109,8 +109,9 @@ defmodule Pleroma.HTML do result = content |> Floki.parse_fragment!() - |> Floki.filter_out("a.mention,a.hashtag,a.attachment,a[rel~=\"tag\"]") - |> Floki.attribute("a", "href") + |> Floki.find("a:not(.mention,.hashtag,.attachment,[rel~=\"tag\"])") + |> Enum.take(1) + |> Floki.attribute("href") |> Enum.at(0) {:commit, {:ok, result}} diff --git a/lib/pleroma/web/api_spec/operations/list_operation.ex b/lib/pleroma/web/api_spec/operations/list_operation.ex index c88ed5dd0..15039052e 100644 --- a/lib/pleroma/web/api_spec/operations/list_operation.ex +++ b/lib/pleroma/web/api_spec/operations/list_operation.ex @@ -114,7 +114,7 @@ defmodule Pleroma.Web.ApiSpec.ListOperation do description: "Add accounts to the given list.", operationId: "ListController.add_to_list", parameters: [id_param()], - requestBody: add_remove_accounts_request(), + requestBody: add_remove_accounts_request(true), security: [%{"oAuth" => ["write:lists"]}], responses: %{ 200 => Operation.response("Empty object", "application/json", %Schema{type: :object}) @@ -127,8 +127,16 @@ defmodule Pleroma.Web.ApiSpec.ListOperation do tags: ["Lists"], summary: "Remove accounts from list", operationId: "ListController.remove_from_list", - parameters: [id_param()], - requestBody: add_remove_accounts_request(), + parameters: [ + id_param(), + Operation.parameter( + :account_ids, + :query, + %Schema{type: :array, items: %Schema{type: :string}}, + "Array of account IDs" + ) + ], + requestBody: add_remove_accounts_request(false), security: [%{"oAuth" => ["write:lists"]}], responses: %{ 200 => Operation.response("Empty object", "application/json", %Schema{type: :object}) @@ -171,7 +179,7 @@ defmodule Pleroma.Web.ApiSpec.ListOperation do ) end - defp add_remove_accounts_request do + defp add_remove_accounts_request(required) when is_boolean(required) do request_body( "Parameters", %Schema{ @@ -180,9 +188,9 @@ defmodule Pleroma.Web.ApiSpec.ListOperation do properties: %{ account_ids: %Schema{type: :array, description: "Array of account IDs", items: FlakeID} }, - required: [:account_ids] + required: required && [:account_ids] }, - required: true + required: required ) end end diff --git a/lib/pleroma/web/mastodon_api/controllers/list_controller.ex b/lib/pleroma/web/mastodon_api/controllers/list_controller.ex index acdc76fd2..5daeaa780 100644 --- a/lib/pleroma/web/mastodon_api/controllers/list_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/list_controller.ex @@ -74,7 +74,7 @@ defmodule Pleroma.Web.MastodonAPI.ListController do # DELETE /api/v1/lists/:id/accounts def remove_from_list( - %{assigns: %{list: list}, body_params: %{account_ids: account_ids}} = conn, + %{assigns: %{list: list}, params: %{account_ids: account_ids}} = conn, _ ) do Enum.each(account_ids, fn account_id -> @@ -86,6 +86,10 @@ defmodule Pleroma.Web.MastodonAPI.ListController do json(conn, %{}) end + def remove_from_list(%{body_params: params} = conn, _) do + remove_from_list(%{conn | params: params}, %{}) + end + defp list_by_id_and_user(%{assigns: %{user: user}, params: %{id: id}} = conn, _) do case Pleroma.List.get(id, user) do %Pleroma.List{} = list -> assign(conn, :list, list) diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 01b8bb6bb..3fe1967be 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -23,6 +23,17 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do import Pleroma.Web.ActivityPub.Visibility, only: [get_visibility: 1, visible_for_user?: 2] + # This is a naive way to do this, just spawning a process per activity + # to fetch the preview. However it should be fine considering + # pagination is restricted to 40 activities at a time + defp fetch_rich_media_for_activities(activities) do + Enum.each(activities, fn activity -> + spawn(fn -> + Pleroma.Web.RichMedia.Helpers.fetch_data_for_activity(activity) + end) + end) + end + # TODO: Add cached version. defp get_replied_to_activities([]), do: %{} @@ -80,6 +91,11 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do # To do: check AdminAPIControllerTest on the reasons behind nil activities in the list activities = Enum.filter(opts.activities, & &1) + + # Start fetching rich media before doing anything else, so that later calls to get the cards + # only block for timeout in the worst case, as opposed to + # length(activities_with_links) * timeout + fetch_rich_media_for_activities(activities) replied_to_activities = get_replied_to_activities(activities) parent_activities = diff --git a/lib/pleroma/web/rich_media/parser.ex b/lib/pleroma/web/rich_media/parser.ex index e9aa2dd03..e98c743ca 100644 --- a/lib/pleroma/web/rich_media/parser.ex +++ b/lib/pleroma/web/rich_media/parser.ex @@ -17,14 +17,25 @@ defmodule Pleroma.Web.RichMedia.Parser do else @spec parse(String.t()) :: {:ok, map()} | {:error, any()} def parse(url) do - Cachex.fetch!(:rich_media_cache, url, fn _ -> - with {:ok, data} <- parse_url(url) do - {:commit, {:ok, data}} - else - error -> {:ignore, error} - end - end) - |> set_ttl_based_on_image(url) + with {:ok, data} <- get_cached_or_parse(url), + {:ok, _} <- set_ttl_based_on_image(data, url) do + {:ok, data} + else + error -> + Logger.error(fn -> "Rich media error: #{inspect(error)}" end) + end + end + + defp get_cached_or_parse(url) do + case Cachex.fetch!(:rich_media_cache, url, fn _ -> {:commit, parse_url(url)} end) do + {:ok, _data} = res -> + res + + {:error, _} = e -> + ttl = Pleroma.Config.get([:rich_media, :failure_backoff], 60_000) + Cachex.expire(:rich_media_cache, url, ttl) + e + end end end @@ -50,24 +61,23 @@ defmodule Pleroma.Web.RichMedia.Parser do config :pleroma, :rich_media, ttl_setters: [MyModule] """ - @spec set_ttl_based_on_image({:ok, map()} | {:error, any()}, String.t()) :: - {:ok, map()} | {:error, any()} - def set_ttl_based_on_image({:ok, data}, url) do - with {:ok, nil} <- Cachex.ttl(:rich_media_cache, url), - {:ok, ttl} when is_number(ttl) <- get_ttl_from_image(data, url) do - Cachex.expire_at(:rich_media_cache, url, ttl * 1000) - {:ok, data} - else + @spec set_ttl_based_on_image(map(), String.t()) :: + {:ok, Integer.t() | :noop} | {:error, :no_key} + def set_ttl_based_on_image(data, url) do + case get_ttl_from_image(data, url) do + {:ok, ttl} when is_number(ttl) -> + ttl = ttl * 1000 + + case Cachex.expire_at(:rich_media_cache, url, ttl) do + {:ok, true} -> {:ok, ttl} + {:ok, false} -> {:error, :no_key} + end + _ -> - {:ok, data} + {:ok, :noop} end end - def set_ttl_based_on_image({:error, _} = error, _) do - Logger.error("parsing error: #{inspect(error)}") - error - end - defp get_ttl_from_image(data, url) do [:rich_media, :ttl_setters] |> Pleroma.Config.get() diff --git a/test/web/mastodon_api/controllers/list_controller_test.exs b/test/web/mastodon_api/controllers/list_controller_test.exs index 57a9ef4a4..091ec006c 100644 --- a/test/web/mastodon_api/controllers/list_controller_test.exs +++ b/test/web/mastodon_api/controllers/list_controller_test.exs @@ -67,7 +67,7 @@ defmodule Pleroma.Web.MastodonAPI.ListControllerTest do assert following == [other_user.follower_address] end - test "removing users from a list" do + test "removing users from a list, body params" do %{user: user, conn: conn} = oauth_access(["write:lists"]) other_user = insert(:user) third_user = insert(:user) @@ -85,6 +85,24 @@ defmodule Pleroma.Web.MastodonAPI.ListControllerTest do assert following == [third_user.follower_address] end + test "removing users from a list, query params" do + %{user: user, conn: conn} = oauth_access(["write:lists"]) + other_user = insert(:user) + third_user = insert(:user) + {:ok, list} = Pleroma.List.create("name", user) + {:ok, list} = Pleroma.List.follow(list, other_user) + {:ok, list} = Pleroma.List.follow(list, third_user) + + assert %{} == + conn + |> put_req_header("content-type", "application/json") + |> delete("/api/v1/lists/#{list.id}/accounts?account_ids[]=#{other_user.id}") + |> json_response_and_validate_schema(:ok) + + %Pleroma.List{following: following} = Pleroma.List.get(list.id, user) + assert following == [third_user.follower_address] + end + test "listing users in a list" do %{user: user, conn: conn} = oauth_access(["read:lists"]) other_user = insert(:user) diff --git a/test/web/rich_media/aws_signed_url_test.exs b/test/web/rich_media/aws_signed_url_test.exs index a21f3c935..1ceae1a31 100644 --- a/test/web/rich_media/aws_signed_url_test.exs +++ b/test/web/rich_media/aws_signed_url_test.exs @@ -55,7 +55,7 @@ defmodule Pleroma.Web.RichMedia.TTL.AwsSignedUrlTest do Cachex.put(:rich_media_cache, url, metadata) - Pleroma.Web.RichMedia.Parser.set_ttl_based_on_image({:ok, metadata}, url) + Pleroma.Web.RichMedia.Parser.set_ttl_based_on_image(metadata, url) {:ok, cache_ttl} = Cachex.ttl(:rich_media_cache, url)