From ee7059c9cf128b5460af7ecf50f5738e2328827d Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Sun, 27 Nov 2022 21:45:41 +0000 Subject: [PATCH] Spin off imports into n oban jobs --- CHANGELOG.md | 1 + lib/pleroma/user/import.ex | 83 ++++++++----------- lib/pleroma/workers/background_worker.ex | 4 +- test/pleroma/emoji/formatter_test.exs | 2 +- test/pleroma/user/import_test.exs | 33 +++++--- .../user_import_controller_test.exs | 30 +++---- 6 files changed, 74 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b7b7e836..b6eb928c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - MastoAPI: Accept BooleanLike input on `/api/v1/accounts/:id/follow` (fixes follows with mastodon.py) - Relays from akkoma are now off by default - NormalizeMarkup MRF is now on by default +- Follow/Block/Mute imports now spin off into *n* tasks to avoid the oban timeout ## 2022.11 diff --git a/lib/pleroma/user/import.ex b/lib/pleroma/user/import.ex index 60cd18041..95c4ef34e 100644 --- a/lib/pleroma/user/import.ex +++ b/lib/pleroma/user/import.ex @@ -12,47 +12,32 @@ defmodule Pleroma.User.Import do require Logger @spec perform(atom(), User.t(), list()) :: :ok | list() | {:error, any()} - def perform(:mutes_import, %User{} = user, [_ | _] = identifiers) do - Enum.map( - identifiers, - fn identifier -> - with {:ok, %User{} = muted_user} <- User.get_or_fetch(identifier), - {:ok, _} <- User.mute(user, muted_user) do - muted_user - else - error -> handle_error(:mutes_import, identifier, error) - end - end - ) + def perform(:mutes_import, %User{} = user, identifier) do + with {:ok, %User{} = muted_user} <- User.get_or_fetch(identifier), + {:ok, _} <- User.mute(user, muted_user) do + muted_user + else + error -> handle_error(:mutes_import, identifier, error) + end end - def perform(:blocks_import, %User{} = blocker, [_ | _] = identifiers) do - Enum.map( - identifiers, - fn identifier -> - with {:ok, %User{} = blocked} <- User.get_or_fetch(identifier), - {:ok, _block} <- CommonAPI.block(blocker, blocked) do - blocked - else - error -> handle_error(:blocks_import, identifier, error) - end - end - ) + def perform(:blocks_import, %User{} = blocker, identifier) do + with {:ok, %User{} = blocked} <- User.get_or_fetch(identifier), + {:ok, _block} <- CommonAPI.block(blocker, blocked) do + blocked + else + error -> handle_error(:blocks_import, identifier, error) + end end - def perform(:follow_import, %User{} = follower, [_ | _] = identifiers) do - Enum.map( - identifiers, - fn identifier -> - with {:ok, %User{} = followed} <- User.get_or_fetch(identifier), - {:ok, follower, followed} <- User.maybe_direct_follow(follower, followed), - {:ok, _, _, _} <- CommonAPI.follow(follower, followed) do - followed - else - error -> handle_error(:follow_import, identifier, error) - end - end - ) + def perform(:follow_import, %User{} = follower, identifier) do + with {:ok, %User{} = followed} <- User.get_or_fetch(identifier), + {:ok, follower, followed} <- User.maybe_direct_follow(follower, followed), + {:ok, _, _, _} <- CommonAPI.follow(follower, followed) do + followed + else + error -> handle_error(:follow_import, identifier, error) + end end def perform(_, _, _), do: :ok @@ -62,24 +47,24 @@ defp handle_error(op, user_id, error) do error end - def blocks_import(%User{} = blocker, [_ | _] = identifiers) do - BackgroundWorker.enqueue( - "blocks_import", - %{"user_id" => blocker.id, "identifiers" => identifiers} + defp enqueue_many(op, user, identifiers) do + Enum.map( + identifiers, + fn identifier -> + BackgroundWorker.enqueue(op, %{"user_id" => user.id, "identifier" => identifier}) + end ) end + def blocks_import(%User{} = blocker, [_ | _] = identifiers) do + enqueue_many("blocks_import", blocker, identifiers) + end + def follow_import(%User{} = follower, [_ | _] = identifiers) do - BackgroundWorker.enqueue( - "follow_import", - %{"user_id" => follower.id, "identifiers" => identifiers} - ) + enqueue_many("follow_import", follower, identifiers) end def mutes_import(%User{} = user, [_ | _] = identifiers) do - BackgroundWorker.enqueue( - "mutes_import", - %{"user_id" => user.id, "identifiers" => identifiers} - ) + enqueue_many("mutes_import", user, identifiers) end end diff --git a/lib/pleroma/workers/background_worker.ex b/lib/pleroma/workers/background_worker.ex index 4db077232..9bd9899db 100644 --- a/lib/pleroma/workers/background_worker.ex +++ b/lib/pleroma/workers/background_worker.ex @@ -25,10 +25,10 @@ def perform(%Job{args: %{"op" => "force_password_reset", "user_id" => user_id}}) User.perform(:force_password_reset, user) end - def perform(%Job{args: %{"op" => op, "user_id" => user_id, "identifiers" => identifiers}}) + def perform(%Job{args: %{"op" => op, "user_id" => user_id, "identifier" => identifier}}) when op in ["blocks_import", "follow_import", "mutes_import"] do user = User.get_cached_by_id(user_id) - {:ok, User.Import.perform(String.to_atom(op), user, identifiers)} + {:ok, User.Import.perform(String.to_atom(op), user, identifier)} end def perform(%Job{ diff --git a/test/pleroma/emoji/formatter_test.exs b/test/pleroma/emoji/formatter_test.exs index 3942f609f..eafdd5a43 100644 --- a/test/pleroma/emoji/formatter_test.exs +++ b/test/pleroma/emoji/formatter_test.exs @@ -11,7 +11,7 @@ test "it adds cool emoji" do text = "I love :firefox:" expected_result = - "I love \"firefox\"" + "I love \"firefox\"" assert Formatter.emojify(text) == expected_result end diff --git a/test/pleroma/user/import_test.exs b/test/pleroma/user/import_test.exs index 2be2ee75b..19d022529 100644 --- a/test/pleroma/user/import_test.exs +++ b/test/pleroma/user/import_test.exs @@ -25,11 +25,14 @@ test "it imports user followings from list" do user3.nickname ] - {:ok, job} = User.Import.follow_import(user1, identifiers) + [{:ok, job1}, {:ok, job2}] = User.Import.follow_import(user1, identifiers) + + assert {:ok, result} = ObanHelpers.perform(job1) + assert result == refresh_record(user2) + + assert {:ok, result} = ObanHelpers.perform(job2) + assert result == refresh_record(user3) - assert {:ok, result} = ObanHelpers.perform(job) - assert is_list(result) - assert result == [refresh_record(user2), refresh_record(user3)] assert User.following?(user1, user2) assert User.following?(user1, user3) end @@ -44,11 +47,14 @@ test "it imports user blocks from list" do user3.nickname ] - {:ok, job} = User.Import.blocks_import(user1, identifiers) + [{:ok, job1}, {:ok, job2}] = User.Import.blocks_import(user1, identifiers) + + assert {:ok, result} = ObanHelpers.perform(job1) + assert result == user2 + + assert {:ok, result} = ObanHelpers.perform(job2) + assert result == user3 - assert {:ok, result} = ObanHelpers.perform(job) - assert is_list(result) - assert result == [user2, user3] assert User.blocks?(user1, user2) assert User.blocks?(user1, user3) end @@ -63,11 +69,14 @@ test "it imports user mutes from list" do user3.nickname ] - {:ok, job} = User.Import.mutes_import(user1, identifiers) + [{:ok, job1}, {:ok, job2}] = User.Import.mutes_import(user1, identifiers) + + assert {:ok, result} = ObanHelpers.perform(job1) + assert result == user2 + + assert {:ok, result} = ObanHelpers.perform(job2) + assert result == user3 - assert {:ok, result} = ObanHelpers.perform(job) - assert is_list(result) - assert result == [user2, user3] assert User.mutes?(user1, user2) assert User.mutes?(user1, user3) end diff --git a/test/pleroma/web/pleroma_api/controllers/user_import_controller_test.exs b/test/pleroma/web/pleroma_api/controllers/user_import_controller_test.exs index d977bc3a2..f9175058d 100644 --- a/test/pleroma/web/pleroma_api/controllers/user_import_controller_test.exs +++ b/test/pleroma/web/pleroma_api/controllers/user_import_controller_test.exs @@ -47,8 +47,8 @@ test "it imports follow lists from file", %{conn: conn} do |> json_response_and_validate_schema(200) assert [{:ok, job_result}] = ObanHelpers.perform_all() - assert job_result == [refresh_record(user2)] - assert [%Pleroma.User{follower_count: 1}] = job_result + assert job_result == refresh_record(user2) + assert %Pleroma.User{follower_count: 1} = job_result end end @@ -108,8 +108,8 @@ test "it imports follows with different nickname variations", %{conn: conn} do |> post("/api/pleroma/follow_import", %{"list" => identifiers}) |> json_response_and_validate_schema(200) - assert [{:ok, job_result}] = ObanHelpers.perform_all() - assert job_result == Enum.map(users, &refresh_record/1) + job_results = Enum.map(ObanHelpers.perform_all(), fn {:ok, result} -> result end) + assert job_results == Enum.map(users, &refresh_record/1) end end @@ -141,8 +141,8 @@ test "it imports blocks users from file", %{conn: conn} do }) |> json_response_and_validate_schema(200) - assert [{:ok, job_result}] = ObanHelpers.perform_all() - assert job_result == users + job_results = Enum.map(ObanHelpers.perform_all(), fn {:ok, result} -> result end) + assert job_results == users end end @@ -165,8 +165,8 @@ test "it imports blocks with different nickname variations", %{conn: conn} do |> post("/api/pleroma/blocks_import", %{"list" => identifiers}) |> json_response_and_validate_schema(200) - assert [{:ok, job_result}] = ObanHelpers.perform_all() - assert job_result == users + job_results = Enum.map(ObanHelpers.perform_all(), fn {:ok, result} -> result end) + assert job_results == users end end @@ -183,12 +183,12 @@ test "it returns HTTP 200", %{user: user, conn: conn} do |> post("/api/pleroma/mutes_import", %{"list" => "#{user2.ap_id}"}) |> json_response_and_validate_schema(200) - assert [{:ok, job_result}] = ObanHelpers.perform_all() - assert job_result == [user2] + job_results = Enum.map(ObanHelpers.perform_all(), fn {:ok, result} -> result end) + assert job_results == [user2] assert Pleroma.User.mutes?(user, user2) end - test "it imports mutes users from file", %{user: user, conn: conn} do + test "it imports muted users from file", %{user: user, conn: conn} do users = [user2, user3] = insert_list(2, :user) with_mocks([ @@ -202,8 +202,8 @@ test "it imports mutes users from file", %{user: user, conn: conn} do }) |> json_response_and_validate_schema(200) - assert [{:ok, job_result}] = ObanHelpers.perform_all() - assert job_result == users + job_results = Enum.map(ObanHelpers.perform_all(), fn {:ok, result} -> result end) + assert job_results == users assert Enum.all?(users, &Pleroma.User.mutes?(user, &1)) end end @@ -227,8 +227,8 @@ test "it imports mutes with different nickname variations", %{user: user, conn: |> post("/api/pleroma/mutes_import", %{"list" => identifiers}) |> json_response_and_validate_schema(200) - assert [{:ok, job_result}] = ObanHelpers.perform_all() - assert job_result == users + job_results = Enum.map(ObanHelpers.perform_all(), fn {:ok, result} -> result end) + assert job_results == users assert Enum.all?(users, &Pleroma.User.mutes?(user, &1)) end end