From 4ae17c62944eab89acfa96a0912819093c872436 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 30 Aug 2024 15:25:21 -0400 Subject: [PATCH 01/14] NodeInfo: Accept application/activity+json requests --- changelog.d/well-known.change | 1 + lib/pleroma/web/router.ex | 2 +- test/pleroma/web/node_info_test.exs | 13 +++++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 changelog.d/well-known.change diff --git a/changelog.d/well-known.change b/changelog.d/well-known.change new file mode 100644 index 0000000000..e928124fbc --- /dev/null +++ b/changelog.d/well-known.change @@ -0,0 +1 @@ +Accept application/activity+json for requests to .well-known/nodeinfo diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 6492e38619..9e4b403e05 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -189,7 +189,7 @@ defmodule Pleroma.Web.Router do end pipeline :well_known do - plug(:accepts, ["json", "jrd", "jrd+json", "xml", "xrd+xml"]) + plug(:accepts, ["activity+json", "json", "jrd", "jrd+json", "xml", "xrd+xml"]) end pipeline :config do diff --git a/test/pleroma/web/node_info_test.exs b/test/pleroma/web/node_info_test.exs index f474220be4..afe4ebb36d 100644 --- a/test/pleroma/web/node_info_test.exs +++ b/test/pleroma/web/node_info_test.exs @@ -24,6 +24,19 @@ test "GET /.well-known/nodeinfo", %{conn: conn} do |> get(href) |> json_response(200) end) + + accept_types = [ + "application/activity+json", + "application/json", + "application/jrd+json" + ] + + for type <- accept_types do + conn + |> put_req_header("accept", type) + |> get("/.well-known/nodeinfo") + |> json_response(200) + end end test "nodeinfo shows staff accounts", %{conn: conn} do From fb376ce0056cf977d3673c99bca4e77c564b4c47 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 5 Sep 2024 15:27:43 -0400 Subject: [PATCH 02/14] Test Account View does not indicate following if a FollowingRelationship is missing --- .../mastodon_api/views/account_view_test.exs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/pleroma/web/mastodon_api/views/account_view_test.exs b/test/pleroma/web/mastodon_api/views/account_view_test.exs index dca64853d1..5d2f55c6d6 100644 --- a/test/pleroma/web/mastodon_api/views/account_view_test.exs +++ b/test/pleroma/web/mastodon_api/views/account_view_test.exs @@ -456,6 +456,44 @@ test "represent a relationship for the following and followed user" do test_relationship_rendering(user, other_user, expected) end + test "relationship does not indicate following if a FollowingRelationship is missing" do + user = insert(:user) + other_user = insert(:user, local: false) + + # Create a follow relationship with the real Follow Activity and Accept it + assert {:ok, _, _, _} = CommonAPI.follow(other_user, user) + assert {:ok, _} = CommonAPI.accept_follow_request(user, other_user) + + assert %{data: %{"state" => "accept"}} = + Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(user, other_user) + + # Fetch the relationship and forcibly delete it to simulate a Follow Accept that did not complete processing + %{following_relationships: [relationship]} = + Pleroma.UserRelationship.view_relationships_option(user, [other_user]) + + assert {:ok, _} = Pleroma.Repo.delete(relationship) + + assert %{following_relationships: [], user_relationships: []} == + Pleroma.UserRelationship.view_relationships_option(user, [other_user]) + + expected = + Map.merge( + @blank_response, + %{ + following: false, + followed_by: false, + muting: false, + muting_notifications: false, + subscribing: false, + notifying: false, + showing_reblogs: true, + id: to_string(other_user.id) + } + ) + + test_relationship_rendering(user, other_user, expected) + end + test "represent a relationship for the blocking and blocked user" do user = insert(:user) other_user = insert(:user) From 4d76692db36d6779fddacee3a7690739064eae0c Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 5 Sep 2024 11:43:48 -0400 Subject: [PATCH 03/14] Fix Following status bug --- changelog.d/following-state.fix | 1 + .../web/mastodon_api/views/account_view.ex | 19 +++++++++---------- 2 files changed, 10 insertions(+), 10 deletions(-) create mode 100644 changelog.d/following-state.fix diff --git a/changelog.d/following-state.fix b/changelog.d/following-state.fix new file mode 100644 index 0000000000..314ea62102 --- /dev/null +++ b/changelog.d/following-state.fix @@ -0,0 +1 @@ +Resolved edge case where the API can report you are following a user but the relationship is not fully established. diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index 6976ca6e5e..298c739867 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -92,14 +92,13 @@ def render( User.get_follow_state(reading_user, target) end - followed_by = - if following_relationships do - case FollowingRelationship.find(following_relationships, target, reading_user) do - %{state: :follow_accept} -> true - _ -> false - end - else - User.following?(target, reading_user) + followed_by = FollowingRelationship.following?(target, reading_user) + following = FollowingRelationship.following?(reading_user, target) + + requested = + cond do + following -> false + true -> match?(:follow_pending, follow_state) end subscribing = @@ -114,7 +113,7 @@ def render( # NOTE: adjust UserRelationship.view_relationships_option/2 on new relation-related flags %{ id: to_string(target.id), - following: follow_state == :follow_accept, + following: following, followed_by: followed_by, blocking: UserRelationship.exists?( @@ -150,7 +149,7 @@ def render( ), subscribing: subscribing, notifying: subscribing, - requested: follow_state == :follow_pending, + requested: requested, domain_blocking: User.blocks_domain?(reading_user, target), showing_reblogs: not UserRelationship.exists?( From e51cd31a576715d8e7d991e4fce850edcf4c8a1e Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 5 Sep 2024 17:06:53 -0400 Subject: [PATCH 04/14] Bump credo to prevent it from crashing --- mix.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.lock b/mix.lock index 07c1122aaf..865e09a4c7 100644 --- a/mix.lock +++ b/mix.lock @@ -22,7 +22,7 @@ "cowboy": {:hex, :cowboy, "2.12.0", "f276d521a1ff88b2b9b4c54d0e753da6c66dd7be6c9fca3d9418b561828a3731", [:make, :rebar3], [{:cowlib, "2.13.0", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "1.8.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "8a7abe6d183372ceb21caa2709bec928ab2b72e18a3911aa1771639bef82651e"}, "cowboy_telemetry": {:hex, :cowboy_telemetry, "0.4.0", "f239f68b588efa7707abce16a84d0d2acf3a0f50571f8bb7f56a15865aae820c", [:rebar3], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7d98bac1ee4565d31b62d59f8823dfd8356a169e7fcbb83831b8a5397404c9de"}, "cowlib": {:hex, :cowlib, "2.13.0", "db8f7505d8332d98ef50a3ef34b34c1afddec7506e4ee4dd4a3a266285d282ca", [:make, :rebar3], [], "hexpm", "e1e1284dc3fc030a64b1ad0d8382ae7e99da46c3246b815318a4b848873800a4"}, - "credo": {:hex, :credo, "1.7.3", "05bb11eaf2f2b8db370ecaa6a6bda2ec49b2acd5e0418bc106b73b07128c0436", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "35ea675a094c934c22fb1dca3696f3c31f2728ae6ef5a53b5d648c11180a4535"}, + "credo": {:hex, :credo, "1.7.7", "771445037228f763f9b2afd612b6aa2fd8e28432a95dbbc60d8e03ce71ba4446", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "8bc87496c9aaacdc3f90f01b7b0582467b69b4bd2441fe8aae3109d843cc2f2e"}, "crontab": {:hex, :crontab, "1.1.8", "2ce0e74777dfcadb28a1debbea707e58b879e6aa0ffbf9c9bb540887bce43617", [:mix], [{:ecto, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :ecto, repo: "hexpm", optional: true]}], "hexpm"}, "custom_base": {:hex, :custom_base, "0.2.1", "4a832a42ea0552299d81652aa0b1f775d462175293e99dfbe4d7dbaab785a706", [:mix], [], "hexpm", "8df019facc5ec9603e94f7270f1ac73ddf339f56ade76a721eaa57c1493ba463"}, "db_connection": {:hex, :db_connection, "2.7.0", "b99faa9291bb09892c7da373bb82cba59aefa9b36300f6145c5f201c7adf48ec", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "dcf08f31b2701f857dfc787fbad78223d61a32204f217f15e881dd93e4bdd3ff"}, From 5f573b4095ade584a590b756c83f13f89336cd04 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 5 Sep 2024 17:11:02 -0400 Subject: [PATCH 05/14] Credo: comment line length --- test/pleroma/web/mastodon_api/views/account_view_test.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/pleroma/web/mastodon_api/views/account_view_test.exs b/test/pleroma/web/mastodon_api/views/account_view_test.exs index 5d2f55c6d6..f88b90955f 100644 --- a/test/pleroma/web/mastodon_api/views/account_view_test.exs +++ b/test/pleroma/web/mastodon_api/views/account_view_test.exs @@ -467,7 +467,8 @@ test "relationship does not indicate following if a FollowingRelationship is mis assert %{data: %{"state" => "accept"}} = Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(user, other_user) - # Fetch the relationship and forcibly delete it to simulate a Follow Accept that did not complete processing + # Fetch the relationship and forcibly delete it to simulate + # a Follow Accept that did not complete processing %{following_relationships: [relationship]} = Pleroma.UserRelationship.view_relationships_option(user, [other_user]) From a887188890a6b8c9e97c6cafe1776bb151e63843 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 09:42:53 -0400 Subject: [PATCH 06/14] Oban: more unique job constraints --- changelog.d/oban-uniques.change | 1 + lib/pleroma/workers/receiver_worker.ex | 2 +- lib/pleroma/workers/remote_fetcher_worker.ex | 2 +- lib/pleroma/workers/rich_media_worker.ex | 2 +- lib/pleroma/workers/user_refresh_worker.ex | 2 +- lib/pleroma/workers/web_pusher_worker.ex | 2 +- 6 files changed, 6 insertions(+), 5 deletions(-) create mode 100644 changelog.d/oban-uniques.change diff --git a/changelog.d/oban-uniques.change b/changelog.d/oban-uniques.change new file mode 100644 index 0000000000..d9deb46961 --- /dev/null +++ b/changelog.d/oban-uniques.change @@ -0,0 +1 @@ +Adjust more Oban workers to enforce unique job constraints. diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 0373ec15f8..11b672befd 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -7,7 +7,7 @@ defmodule Pleroma.Workers.ReceiverWorker do alias Pleroma.User alias Pleroma.Web.Federator - use Oban.Worker, queue: :federator_incoming, max_attempts: 5 + use Oban.Worker, queue: :federator_incoming, max_attempts: 5, unique: [period: :infinity] @impl true diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index 9d3f1ec539..aa09362f51 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -5,7 +5,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do alias Pleroma.Object.Fetcher - use Oban.Worker, queue: :background + use Oban.Worker, queue: :background, unique: [period: :infinity] @impl true def perform(%Job{args: %{"op" => "fetch_remote", "id" => id} = args}) do diff --git a/lib/pleroma/workers/rich_media_worker.ex b/lib/pleroma/workers/rich_media_worker.ex index d5ba7b63ec..e351ecd6eb 100644 --- a/lib/pleroma/workers/rich_media_worker.ex +++ b/lib/pleroma/workers/rich_media_worker.ex @@ -7,7 +7,7 @@ defmodule Pleroma.Workers.RichMediaWorker do alias Pleroma.Web.RichMedia.Backfill alias Pleroma.Web.RichMedia.Card - use Oban.Worker, queue: :background, max_attempts: 3, unique: [period: 300] + use Oban.Worker, queue: :background, max_attempts: 3, unique: [period: :infinity] @impl true def perform(%Job{args: %{"op" => "expire", "url" => url} = _args}) do diff --git a/lib/pleroma/workers/user_refresh_worker.ex b/lib/pleroma/workers/user_refresh_worker.ex index 222a4a8f7e..ee276774bb 100644 --- a/lib/pleroma/workers/user_refresh_worker.ex +++ b/lib/pleroma/workers/user_refresh_worker.ex @@ -3,7 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Workers.UserRefreshWorker do - use Oban.Worker, queue: :background, max_attempts: 1, unique: [period: 300] + use Oban.Worker, queue: :background, max_attempts: 1, unique: [period: :infinity] alias Pleroma.User diff --git a/lib/pleroma/workers/web_pusher_worker.ex b/lib/pleroma/workers/web_pusher_worker.ex index f4232d02af..879b26cc3f 100644 --- a/lib/pleroma/workers/web_pusher_worker.ex +++ b/lib/pleroma/workers/web_pusher_worker.ex @@ -7,7 +7,7 @@ defmodule Pleroma.Workers.WebPusherWorker do alias Pleroma.Repo alias Pleroma.Web.Push.Impl - use Oban.Worker, queue: :web_push + use Oban.Worker, queue: :web_push, unique: [period: :infinity] @impl true def perform(%Job{args: %{"op" => "web_push", "notification_id" => notification_id}}) do From fc3ea94a1c17e84033fa593c0ea987fbfa545447 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 09:58:03 -0400 Subject: [PATCH 07/14] Dialyzer: the pattern can never match the type --- lib/pleroma/object/fetcher.ex | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 9d9a201caf..ff7aa539fc 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -58,8 +58,11 @@ def refetch_object(%Object{data: %{"id" => id}} = object) do end end + @typep fetcher_errors :: + :error | :reject | :allowed_depth | :fetch | :containment | :transmogrifier + # Note: will create a Create activity, which we need internally at the moment. - @spec fetch_object_from_id(String.t(), list()) :: {:ok, Object.t()} | {:error | :reject, any()} + @spec fetch_object_from_id(String.t(), list()) :: {:ok, Object.t()} | {fetcher_errors(), any()} def fetch_object_from_id(id, options \\ []) do with {_, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)}, {_, true} <- {:allowed_depth, Federator.allowed_thread_distance?(options[:depth])}, From bc16f09d7b9c1a15c8c9be6d99092b9a8d867d18 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:12:13 -0400 Subject: [PATCH 08/14] Dialyzer: the pattern can never match the type The original error was for the chat controller: lib/pleroma/web/pleroma_api/controllers/chat_controller.ex:104:pattern_match The pattern can never match the type {:error, :content_too_long | :forbidden | :no_content | :not_found} | {:user, nil}. Improve typespecs for the Pipeline and apply them where it could be encountered --- lib/pleroma/object/fetcher.ex | 3 +- lib/pleroma/web/activity_pub/pipeline.ex | 19 ++++++---- lib/pleroma/web/common_api.ex | 44 ++++++++++++++---------- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index ff7aa539fc..69a5f32685 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -62,7 +62,8 @@ def refetch_object(%Object{data: %{"id" => id}} = object) do :error | :reject | :allowed_depth | :fetch | :containment | :transmogrifier # Note: will create a Create activity, which we need internally at the moment. - @spec fetch_object_from_id(String.t(), list()) :: {:ok, Object.t()} | {fetcher_errors(), any()} + @spec fetch_object_from_id(String.t(), list()) :: + {:ok, Object.t()} | {fetcher_errors(), any()} | Pipeline.errors() def fetch_object_from_id(id, options \\ []) do with {_, nil} <- {:fetch_object, Object.get_cached_by_ap_id(id)}, {_, true} <- {:allowed_depth, Federator.allowed_thread_distance?(options[:depth])}, diff --git a/lib/pleroma/web/activity_pub/pipeline.ex b/lib/pleroma/web/activity_pub/pipeline.ex index 7f11a4d67e..fc36935d57 100644 --- a/lib/pleroma/web/activity_pub/pipeline.ex +++ b/lib/pleroma/web/activity_pub/pipeline.ex @@ -22,22 +22,27 @@ defp mrf, do: Config.get([:pipeline, :mrf], MRF) defp activity_pub, do: Config.get([:pipeline, :activity_pub], ActivityPub) defp config, do: Config.get([:pipeline, :config], Config) - @spec common_pipeline(map(), keyword()) :: - {:ok, Activity.t() | Object.t(), keyword()} | {:error | :reject, any()} + @type results :: {:ok, Activity.t() | Object.t(), keyword()} + @type errors :: {:error | :reject, any()} + + # The Repo.transaction will wrap the result in an {:ok, _} + # and only returns an {:error, _} if the error encountered was related + # to the SQL transaction + @spec common_pipeline(map(), keyword()) :: results() | errors() def common_pipeline(object, meta) do case Repo.transaction(fn -> do_common_pipeline(object, meta) end, Utils.query_timeout()) do {:ok, {:ok, activity, meta}} -> side_effects().handle_after_transaction(meta) {:ok, activity, meta} - {:ok, value} -> - value + {:ok, {:error, _} = error} -> + error + + {:ok, {:reject, _} = error} -> + error {:error, e} -> {:error, e} - - {:reject, e} -> - {:reject, e} end end diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index 921e414c34..412424dae8 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -26,7 +26,7 @@ defmodule Pleroma.Web.CommonAPI do require Pleroma.Constants require Logger - @spec block(User.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec block(User.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def block(blocked, blocker) do with {:ok, block_data, _} <- Builder.block(blocker, blocked), {:ok, block, _} <- Pipeline.common_pipeline(block_data, local: true) do @@ -35,7 +35,7 @@ def block(blocked, blocker) do end @spec post_chat_message(User.t(), User.t(), String.t(), list()) :: - {:ok, Activity.t()} | {:error, any()} + {:ok, Activity.t()} | Pipeline.errors() def post_chat_message(%User{} = user, %User{} = recipient, content, opts \\ []) do with maybe_attachment <- opts[:media_id] && Object.get_by_id(opts[:media_id]), :ok <- validate_chat_attachment_attribution(maybe_attachment, user), @@ -58,7 +58,7 @@ def post_chat_message(%User{} = user, %User{} = recipient, content, opts \\ []) )} do {:ok, activity} else - {:common_pipeline, {:reject, _} = e} -> e + {:common_pipeline, e} -> e e -> e end end @@ -99,7 +99,8 @@ defp validate_chat_content_length(content, _) do end end - @spec unblock(User.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec unblock(User.t(), User.t()) :: + {:ok, Activity.t()} | {:ok, :no_activity} | Pipeline.errors() | {:error, :not_blocking} def unblock(blocked, blocker) do with {_, %Activity{} = block} <- {:fetch_block, Utils.fetch_latest_block(blocker, blocked)}, {:ok, unblock_data, _} <- Builder.undo(blocker, block), @@ -120,7 +121,9 @@ def unblock(blocked, blocker) do end @spec follow(User.t(), User.t()) :: - {:ok, User.t(), User.t(), Activity.t() | Object.t()} | {:error, :rejected} + {:ok, User.t(), User.t(), Activity.t() | Object.t()} + | {:error, :rejected} + | Pipeline.errors() def follow(followed, follower) do timeout = Pleroma.Config.get([:activitypub, :follow_handshake_timeout]) @@ -145,7 +148,7 @@ def unfollow(unfollowed, follower) do end end - @spec accept_follow_request(User.t(), User.t()) :: {:ok, User.t()} | {:error, any()} + @spec accept_follow_request(User.t(), User.t()) :: {:ok, User.t()} | Pipeline.errors() def accept_follow_request(follower, followed) do with %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed), {:ok, accept_data, _} <- Builder.accept(followed, follow_activity), @@ -154,7 +157,7 @@ def accept_follow_request(follower, followed) do end end - @spec reject_follow_request(User.t(), User.t()) :: {:ok, User.t()} | {:error, any()} | nil + @spec reject_follow_request(User.t(), User.t()) :: {:ok, User.t()} | Pipeline.errors() | nil def reject_follow_request(follower, followed) do with %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed), {:ok, reject_data, _} <- Builder.reject(followed, follow_activity), @@ -163,7 +166,8 @@ def reject_follow_request(follower, followed) do end end - @spec delete(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec delete(String.t(), User.t()) :: + {:ok, Activity.t()} | Pipeline.errors() | {:error, :not_found | String.t()} def delete(activity_id, user) do with {_, %Activity{data: %{"object" => _, "type" => "Create"}} = activity} <- {:find_activity, Activity.get_by_id(activity_id, filter: [])}, @@ -213,7 +217,7 @@ def delete(activity_id, user) do end end - @spec repeat(String.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, any()} + @spec repeat(String.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, :not_found} def repeat(id, user, params \\ %{}) do with %Activity{data: %{"type" => "Create"}} = activity <- Activity.get_by_id(id), object = %Object{} <- Object.normalize(activity, fetch: false), @@ -231,7 +235,7 @@ def repeat(id, user, params \\ %{}) do end end - @spec unrepeat(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec unrepeat(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, :not_found | String.t()} def unrepeat(id, user) do with {_, %Activity{data: %{"type" => "Create"}} = activity} <- {:find_activity, Activity.get_by_id(id)}, @@ -247,7 +251,8 @@ def unrepeat(id, user) do end end - @spec favorite(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec favorite(String.t(), User.t()) :: + {:ok, Activity.t()} | {:ok, :already_liked} | {:error, :not_found | String.t()} def favorite(id, %User{} = user) do case favorite_helper(user, id) do {:ok, _} = res -> @@ -285,7 +290,8 @@ defp favorite_helper(user, id) do end end - @spec unfavorite(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, any()} + @spec unfavorite(String.t(), User.t()) :: + {:ok, Activity.t()} | {:error, :not_found | String.t()} def unfavorite(id, user) do with {_, %Activity{data: %{"type" => "Create"}} = activity} <- {:find_activity, Activity.get_by_id(id)}, @@ -302,7 +308,7 @@ def unfavorite(id, user) do end @spec react_with_emoji(String.t(), User.t(), String.t()) :: - {:ok, Activity.t()} | {:error, any()} + {:ok, Activity.t()} | {:error, String.t()} def react_with_emoji(id, user, emoji) do with %Activity{} = activity <- Activity.get_by_id(id), object <- Object.normalize(activity, fetch: false), @@ -316,7 +322,7 @@ def react_with_emoji(id, user, emoji) do end @spec unreact_with_emoji(String.t(), User.t(), String.t()) :: - {:ok, Activity.t()} | {:error, any()} + {:ok, Activity.t()} | {:error, String.t()} def unreact_with_emoji(id, user, emoji) do with %Activity{} = reaction_activity <- Utils.get_latest_reaction(id, user, emoji), {_, {:ok, _}} <- {:cancel_jobs, maybe_cancel_jobs(reaction_activity)}, @@ -329,7 +335,7 @@ def unreact_with_emoji(id, user, emoji) do end end - @spec vote(Object.t(), User.t(), list()) :: {:ok, list(), Object.t()} | {:error, any()} + @spec vote(Object.t(), User.t(), list()) :: {:ok, list(), Object.t()} | Pipeline.errors() def vote(%Object{data: %{"type" => "Question"}} = object, %User{} = user, choices) do with :ok <- validate_not_author(object, user), :ok <- validate_existing_votes(user, object), @@ -461,7 +467,7 @@ def post(user, %{status: _} = data) do end end - @spec update(Activity.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, any()} + @spec update(Activity.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, nil} def update(orig_activity, %User{} = user, changes) do with orig_object <- Object.normalize(orig_activity), {:ok, new_object} <- make_update_data(user, orig_object, changes), @@ -497,7 +503,7 @@ defp make_update_data(user, orig_object, changes) do end end - @spec pin(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, term()} + @spec pin(String.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def pin(id, %User{} = user) do with %Activity{} = activity <- create_activity_by_id(id), true <- activity_belongs_to_actor(activity, user.ap_id), @@ -537,7 +543,7 @@ defp activity_is_public(activity) do end end - @spec unpin(String.t(), User.t()) :: {:ok, Activity.t()} | {:error, term()} + @spec unpin(String.t(), User.t()) :: {:ok, Activity.t()} | Pipeline.errors() def unpin(id, user) do with %Activity{} = activity <- create_activity_by_id(id), {:ok, unpin_data, _} <- Builder.unpin(user, activity.object), @@ -552,7 +558,7 @@ def unpin(id, user) do end end - @spec add_mute(Activity.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, any()} + @spec add_mute(Activity.t(), User.t(), map()) :: {:ok, Activity.t()} | {:error, String.t()} def add_mute(activity, user, params \\ %{}) do expires_in = Map.get(params, :expires_in, 0) From 7eb579c1911f2eac175c2030f6bb80685b4ab4f8 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:18:12 -0400 Subject: [PATCH 09/14] Dialyzer: invalid contract --- lib/pleroma/user/import.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/user/import.ex b/lib/pleroma/user/import.ex index b79fa88eb2..ab6bdb8d41 100644 --- a/lib/pleroma/user/import.ex +++ b/lib/pleroma/user/import.ex @@ -12,7 +12,7 @@ defmodule Pleroma.User.Import do require Logger - @spec perform(atom(), User.t(), list()) :: :ok | list() | {:error, any()} + @spec perform(atom(), User.t(), String.t()) :: :ok | {:error, any()} def perform(:mute_import, %User{} = user, actor) do with {:ok, %User{} = muted_user} <- User.get_or_fetch(actor), {_, false} <- {:existing_mute, User.mutes_user?(user, muted_user)}, @@ -49,7 +49,7 @@ def perform(:follow_import, %User{} = user, actor) do defp handle_error(op, user_id, error) do Logger.debug("#{op} failed for #{user_id} with: #{inspect(error)}") - error + {:error, error} end def blocks_import(%User{} = user, [_ | _] = actors) do From 06d6febff960e5aafd44709d0a61311b45892a81 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:19:24 -0400 Subject: [PATCH 10/14] Dialyzer: The pattern variable _e@1 can never match the type, because it is covered by previous clauses. --- lib/pleroma/user/backup.ex | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/pleroma/user/backup.ex b/lib/pleroma/user/backup.ex index 7feaa22bf5..70cf5b2a1c 100644 --- a/lib/pleroma/user/backup.ex +++ b/lib/pleroma/user/backup.ex @@ -92,9 +92,6 @@ def schedule_backup(backup) do else true -> {:error, "Backup is missing id. Please insert it into the Repo first."} - - e -> - {:error, e} end end From 1d0e3b1355c5a5883be1522f0f925c398c0e87a4 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:24:37 -0400 Subject: [PATCH 11/14] Dialyzer: The pattern variable _ can never match the type, because it is covered by previous clauses. --- lib/pleroma/user/backup.ex | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/pleroma/user/backup.ex b/lib/pleroma/user/backup.ex index 70cf5b2a1c..c5038c8f46 100644 --- a/lib/pleroma/user/backup.ex +++ b/lib/pleroma/user/backup.ex @@ -294,9 +294,6 @@ defp write(query, dir, name, fun) do ) acc - - _ -> - acc end end) From 06ce5e3b43b4a6809397bdb0eb192a82e7243e93 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:27:07 -0400 Subject: [PATCH 12/14] Dialyzer: pattern_match The pattern can never match the type {:diff, false}. --- lib/pleroma/user/backup.ex | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/user/backup.ex b/lib/pleroma/user/backup.ex index c5038c8f46..d77d49890f 100644 --- a/lib/pleroma/user/backup.ex +++ b/lib/pleroma/user/backup.ex @@ -118,14 +118,13 @@ def schedule_delete(backup) do end defp permitted?(user) do - with {_, %__MODULE__{inserted_at: inserted_at}} <- {:last, get_last(user)}, - days = Config.get([__MODULE__, :limit_days]), - diff = Timex.diff(NaiveDateTime.utc_now(), inserted_at, :days), - {_, true} <- {:diff, diff > days} do - true + with {_, %__MODULE__{inserted_at: inserted_at}} <- {:last, get_last(user)} do + days = Config.get([__MODULE__, :limit_days]) + diff = Timex.diff(NaiveDateTime.utc_now(), inserted_at, :days) + + diff > days else {:last, nil} -> true - {:diff, false} -> false end end From 5b26c56624ad281987a18091a6ae245833d2fde1 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:34:06 -0400 Subject: [PATCH 13/14] Changelog --- changelog.d/dialyzer.skip | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 changelog.d/dialyzer.skip diff --git a/changelog.d/dialyzer.skip b/changelog.d/dialyzer.skip new file mode 100644 index 0000000000..e69de29bb2 From 1afcfd4845fb71e111b7cbcf18858bb100863f8a Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 6 Sep 2024 11:51:16 -0400 Subject: [PATCH 14/14] Add tests for Mastodon mention hashtag class --- test/pleroma/html_test.exs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/pleroma/html_test.exs b/test/pleroma/html_test.exs index 1be1619711..d17b075403 100644 --- a/test/pleroma/html_test.exs +++ b/test/pleroma/html_test.exs @@ -41,6 +41,10 @@ defmodule Pleroma.HTMLTest do @foo """ + @mention_hashtags_sample """ + + """ + describe "StripTags scrubber" do test "works as expected" do expected = """ @@ -126,6 +130,15 @@ test "filters invalid microformats markup" do Pleroma.HTML.Scrubber.TwitterText ) end + + test "does allow mention hashtags" do + expected = """ + + """ + + assert expected == + HTML.filter_tags(@mention_hashtags_sample, Pleroma.HTML.Scrubber.Default) + end end describe "default scrubber" do @@ -189,6 +202,15 @@ test "filters invalid microformats markup" do Pleroma.HTML.Scrubber.Default ) end + + test "does allow mention hashtags" do + expected = """ + + """ + + assert expected == + HTML.filter_tags(@mention_hashtags_sample, Pleroma.HTML.Scrubber.Default) + end end describe "extract_first_external_url_from_object" do