Page MenuHomePhorge

No OneTemporary

Authored By
Unknown
Size
23 KB
Referenced Files
None
Subscribers
None
diff --git a/changelog.d/user-imports.fix b/changelog.d/user-imports.fix
new file mode 100644
index 000000000..0076c73d7
--- /dev/null
+++ b/changelog.d/user-imports.fix
@@ -0,0 +1 @@
+Imports of blocks, mutes, and follows would retry repeatedly due to incorrect error handling and all work executed in a single job
diff --git a/lib/pleroma/user/import.ex b/lib/pleroma/user/import.ex
index 11905237c..b79fa88eb 100644
--- a/lib/pleroma/user/import.ex
+++ b/lib/pleroma/user/import.ex
@@ -1,91 +1,111 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.User.Import do
use Ecto.Schema
+ alias Pleroma.Repo
alias Pleroma.User
alias Pleroma.Web.CommonAPI
alias Pleroma.Workers.BackgroundWorker
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(: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)},
+ {:ok, _} <- User.mute(user, muted_user) do
+ {:ok, muted_user}
+ else
+ {:existing_mute, true} -> :ok
+ error -> handle_error(:mutes_import, actor, 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(blocked, blocker) do
- blocked
- else
- error -> handle_error(:blocks_import, identifier, error)
- end
- end
- )
+ def perform(:block_import, %User{} = user, actor) do
+ with {:ok, %User{} = blocked} <- User.get_or_fetch(actor),
+ {_, false} <- {:existing_block, User.blocks_user?(user, blocked)},
+ {:ok, _block} <- CommonAPI.block(blocked, user) do
+ {:ok, blocked}
+ else
+ {:existing_block, true} -> :ok
+ error -> handle_error(:blocks_import, actor, 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(followed, follower) do
- followed
- else
- error -> handle_error(:follow_import, identifier, error)
- end
- end
- )
+ def perform(:follow_import, %User{} = user, actor) do
+ with {:ok, %User{} = followed} <- User.get_or_fetch(actor),
+ {_, false} <- {:existing_follow, User.following?(user, followed)},
+ {:ok, user, followed} <- User.maybe_direct_follow(user, followed),
+ {:ok, _, _, _} <- CommonAPI.follow(followed, user) do
+ {:ok, followed}
+ else
+ {:existing_follow, true} -> :ok
+ error -> handle_error(:follow_import, actor, error)
+ end
end
- def perform(_, _, _), do: :ok
-
defp handle_error(op, user_id, error) do
Logger.debug("#{op} failed for #{user_id} with: #{inspect(error)}")
error
end
- def blocks_import(%User{} = blocker, [_ | _] = identifiers) do
- BackgroundWorker.new(%{
- "op" => "blocks_import",
- "user_id" => blocker.id,
- "identifiers" => identifiers
- })
- |> Oban.insert()
+ def blocks_import(%User{} = user, [_ | _] = actors) do
+ jobs =
+ Repo.checkout(fn ->
+ Enum.reduce(actors, [], fn actor, acc ->
+ {:ok, job} =
+ BackgroundWorker.new(%{
+ "op" => "block_import",
+ "user_id" => user.id,
+ "actor" => actor
+ })
+ |> Oban.insert()
+
+ acc ++ [job]
+ end)
+ end)
+
+ {:ok, jobs}
end
- def follow_import(%User{} = follower, [_ | _] = identifiers) do
- BackgroundWorker.new(%{
- "op" => "follow_import",
- "user_id" => follower.id,
- "identifiers" => identifiers
- })
- |> Oban.insert()
+ def follows_import(%User{} = user, [_ | _] = actors) do
+ jobs =
+ Repo.checkout(fn ->
+ Enum.reduce(actors, [], fn actor, acc ->
+ {:ok, job} =
+ BackgroundWorker.new(%{
+ "op" => "follow_import",
+ "user_id" => user.id,
+ "actor" => actor
+ })
+ |> Oban.insert()
+
+ acc ++ [job]
+ end)
+ end)
+
+ {:ok, jobs}
end
- def mutes_import(%User{} = user, [_ | _] = identifiers) do
- BackgroundWorker.new(%{
- "op" => "mutes_import",
- "user_id" => user.id,
- "identifiers" => identifiers
- })
- |> Oban.insert()
+ def mutes_import(%User{} = user, [_ | _] = actors) do
+ jobs =
+ Repo.checkout(fn ->
+ Enum.reduce(actors, [], fn actor, acc ->
+ {:ok, job} =
+ BackgroundWorker.new(%{
+ "op" => "mute_import",
+ "user_id" => user.id,
+ "actor" => actor
+ })
+ |> Oban.insert()
+
+ acc ++ [job]
+ end)
+ end)
+
+ {:ok, jobs}
end
end
diff --git a/lib/pleroma/web/pleroma_api/controllers/user_import_controller.ex b/lib/pleroma/web/pleroma_api/controllers/user_import_controller.ex
index 96466f192..d65c30dab 100644
--- a/lib/pleroma/web/pleroma_api/controllers/user_import_controller.ex
+++ b/lib/pleroma/web/pleroma_api/controllers/user_import_controller.ex
@@ -1,82 +1,82 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.PleromaAPI.UserImportController do
use Pleroma.Web, :controller
require Logger
alias Pleroma.User
alias Pleroma.Web.ApiSpec
alias Pleroma.Web.Plugs.OAuthScopesPlug
plug(OAuthScopesPlug, %{scopes: ["follow", "write:follows"]} when action == :follow)
plug(OAuthScopesPlug, %{scopes: ["follow", "write:blocks"]} when action == :blocks)
plug(OAuthScopesPlug, %{scopes: ["follow", "write:mutes"]} when action == :mutes)
plug(Pleroma.Web.ApiSpec.CastAndValidate, replace_params: false)
defdelegate open_api_operation(action), to: ApiSpec.UserImportOperation
def follow(
%{private: %{open_api_spex: %{body_params: %{list: %Plug.Upload{path: path}}}}} = conn,
_
) do
list = File.read!(path)
do_follow(conn, list)
end
def follow(%{private: %{open_api_spex: %{body_params: %{list: list}}}} = conn, _),
do: do_follow(conn, list)
def do_follow(%{assigns: %{user: follower}} = conn, list) do
identifiers =
list
|> String.split("\n")
|> Enum.map(&(&1 |> String.split(",") |> List.first()))
|> List.delete("Account address")
|> Enum.map(&(&1 |> String.trim() |> String.trim_leading("@")))
|> Enum.reject(&(&1 == ""))
- User.Import.follow_import(follower, identifiers)
- json(conn, "job started")
+ User.Import.follows_import(follower, identifiers)
+ json(conn, "jobs started")
end
def blocks(
%{private: %{open_api_spex: %{body_params: %{list: %Plug.Upload{path: path}}}}} = conn,
_
) do
list = File.read!(path)
do_block(conn, list)
end
def blocks(%{private: %{open_api_spex: %{body_params: %{list: list}}}} = conn, _),
do: do_block(conn, list)
defp do_block(%{assigns: %{user: blocker}} = conn, list) do
User.Import.blocks_import(blocker, prepare_user_identifiers(list))
- json(conn, "job started")
+ json(conn, "jobs started")
end
def mutes(
%{private: %{open_api_spex: %{body_params: %{list: %Plug.Upload{path: path}}}}} = conn,
_
) do
list = File.read!(path)
do_mute(conn, list)
end
def mutes(%{private: %{open_api_spex: %{body_params: %{list: list}}}} = conn, _),
do: do_mute(conn, list)
defp do_mute(%{assigns: %{user: user}} = conn, list) do
User.Import.mutes_import(user, prepare_user_identifiers(list))
- json(conn, "job started")
+ json(conn, "jobs started")
end
defp prepare_user_identifiers(list) do
list
|> String.split()
|> Enum.map(&String.trim_leading(&1, "@"))
end
end
diff --git a/lib/pleroma/workers/background_worker.ex b/lib/pleroma/workers/background_worker.ex
index 60da2d5ca..4737c6ea2 100644
--- a/lib/pleroma/workers/background_worker.ex
+++ b/lib/pleroma/workers/background_worker.ex
@@ -1,44 +1,44 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Workers.BackgroundWorker do
alias Pleroma.User
use Oban.Worker, queue: :background
@impl true
def perform(%Job{args: %{"op" => "user_activation", "user_id" => user_id, "status" => status}}) do
user = User.get_cached_by_id(user_id)
User.perform(:set_activation_async, user, status)
end
def perform(%Job{args: %{"op" => "force_password_reset", "user_id" => user_id}}) do
user = User.get_cached_by_id(user_id)
User.perform(:force_password_reset, user)
end
- def perform(%Job{args: %{"op" => op, "user_id" => user_id, "identifiers" => identifiers}})
- when op in ["blocks_import", "follow_import", "mutes_import"] do
+ def perform(%Job{args: %{"op" => op, "user_id" => user_id, "actor" => actor}})
+ when op in ["block_import", "follow_import", "mute_import"] do
user = User.get_cached_by_id(user_id)
- {:ok, User.Import.perform(String.to_existing_atom(op), user, identifiers)}
+ User.Import.perform(String.to_existing_atom(op), user, actor)
end
def perform(%Job{
args: %{"op" => "move_following", "origin_id" => origin_id, "target_id" => target_id}
}) do
origin = User.get_cached_by_id(origin_id)
target = User.get_cached_by_id(target_id)
Pleroma.FollowingRelationship.move_following(origin, target)
end
def perform(%Job{args: %{"op" => "verify_fields_links", "user_id" => user_id}}) do
user = User.get_by_id(user_id)
User.perform(:verify_fields_links, user)
end
@impl true
def timeout(_job), do: :timer.seconds(900)
end
diff --git a/test/pleroma/user/import_test.exs b/test/pleroma/user/import_test.exs
index f75305e0e..1d6469a4f 100644
--- a/test/pleroma/user/import_test.exs
+++ b/test/pleroma/user/import_test.exs
@@ -1,75 +1,78 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.User.ImportTest do
alias Pleroma.Tests.ObanHelpers
alias Pleroma.User
use Pleroma.DataCase
use Oban.Testing, repo: Pleroma.Repo
import Pleroma.Factory
setup_all do
Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end)
:ok
end
describe "follow_import" do
test "it imports user followings from list" do
[user1, user2, user3] = insert_list(3, :user)
identifiers = [
user2.ap_id,
user3.nickname
]
- {:ok, job} = User.Import.follow_import(user1, identifiers)
+ {:ok, jobs} = User.Import.follows_import(user1, identifiers)
+
+ for job <- jobs do
+ assert {:ok, %User{}} = ObanHelpers.perform(job)
+ end
- 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
end
describe "blocks_import" do
test "it imports user blocks from list" do
[user1, user2, user3] = insert_list(3, :user)
identifiers = [
user2.ap_id,
user3.nickname
]
- {:ok, job} = User.Import.blocks_import(user1, identifiers)
+ {:ok, jobs} = User.Import.blocks_import(user1, identifiers)
+
+ for job <- jobs do
+ assert {:ok, %User{}} = ObanHelpers.perform(job)
+ end
- 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
end
describe "mutes_import" do
test "it imports user mutes from list" do
[user1, user2, user3] = insert_list(3, :user)
identifiers = [
user2.ap_id,
user3.nickname
]
- {:ok, job} = User.Import.mutes_import(user1, identifiers)
+ {:ok, jobs} = User.Import.mutes_import(user1, identifiers)
+
+ for job <- jobs do
+ assert {:ok, %User{}} = ObanHelpers.perform(job)
+ end
- 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
end
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 52a62e416..efdc743e3 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
@@ -1,235 +1,269 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.PleromaAPI.UserImportControllerTest do
use Pleroma.Web.ConnCase
use Oban.Testing, repo: Pleroma.Repo
alias Pleroma.Tests.ObanHelpers
import Pleroma.Factory
import Mock
setup do
Tesla.Mock.mock(fn env -> apply(HttpRequestMock, :request, [env]) end)
:ok
end
describe "POST /api/pleroma/follow_import" do
setup do: oauth_access(["follow"])
test "it returns HTTP 200", %{conn: conn} do
user2 = insert(:user)
- assert "job started" ==
+ assert "jobs started" ==
conn
|> put_req_header("content-type", "application/json")
|> post("/api/pleroma/follow_import", %{"list" => "#{user2.ap_id}"})
|> json_response_and_validate_schema(200)
end
test "it imports follow lists from file", %{conn: conn} do
user2 = insert(:user)
with_mocks([
{File, [],
read!: fn "follow_list.txt" ->
"Account address,Show boosts\n#{user2.ap_id},true"
end}
]) do
- assert "job started" ==
+ assert "jobs started" ==
conn
|> put_req_header("content-type", "application/json")
|> post("/api/pleroma/follow_import", %{
"list" => %Plug.Upload{path: "follow_list.txt"}
})
|> 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 [{:ok, updated_user}] = ObanHelpers.perform_all()
+ assert updated_user.id == user2.id
+ assert updated_user.follower_count == 1
end
end
test "it imports new-style mastodon follow lists", %{conn: conn} do
user2 = insert(:user)
response =
conn
|> put_req_header("content-type", "application/json")
|> post("/api/pleroma/follow_import", %{
"list" => "Account address,Show boosts\n#{user2.ap_id},true"
})
|> json_response_and_validate_schema(200)
- assert response == "job started"
+ assert response == "jobs started"
end
test "requires 'follow' or 'write:follows' permissions" do
token1 = insert(:oauth_token, scopes: ["read", "write"])
token2 = insert(:oauth_token, scopes: ["follow"])
token3 = insert(:oauth_token, scopes: ["something"])
another_user = insert(:user)
for token <- [token1, token2, token3] do
conn =
build_conn()
|> put_req_header("authorization", "Bearer #{token.token}")
|> put_req_header("content-type", "application/json")
|> post("/api/pleroma/follow_import", %{"list" => "#{another_user.ap_id}"})
if token == token3 do
assert %{"error" => "Insufficient permissions: follow | write:follows."} ==
json_response(conn, 403)
else
assert json_response_and_validate_schema(conn, 200)
end
end
end
test "it imports follows with different nickname variations", %{conn: conn} do
users = [user2, user3, user4, user5, user6] = insert_list(5, :user)
identifiers =
[
user2.ap_id,
user3.nickname,
" ",
"@" <> user4.nickname,
user5.nickname <> "@localhost",
"@" <> user6.nickname <> "@localhost"
]
|> Enum.join("\n")
- assert "job started" ==
+ assert "jobs started" ==
conn
|> put_req_header("content-type", "application/json")
|> 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)
+ results = ObanHelpers.perform_all()
+
+ returned_users =
+ for {_, returned_user} <- results do
+ returned_user
+ end
+
+ assert returned_users == Enum.map(users, &refresh_record/1)
end
end
describe "POST /api/pleroma/blocks_import" do
# Note: "follow" or "write:blocks" permission is required
setup do: oauth_access(["write:blocks"])
test "it returns HTTP 200", %{conn: conn} do
user2 = insert(:user)
- assert "job started" ==
+ assert "jobs started" ==
conn
|> put_req_header("content-type", "application/json")
|> post("/api/pleroma/blocks_import", %{"list" => "#{user2.ap_id}"})
|> json_response_and_validate_schema(200)
end
test "it imports blocks users from file", %{conn: conn} do
users = [user2, user3] = insert_list(2, :user)
with_mocks([
{File, [], read!: fn "blocks_list.txt" -> "#{user2.ap_id} #{user3.ap_id}" end}
]) do
- assert "job started" ==
+ assert "jobs started" ==
conn
|> put_req_header("content-type", "application/json")
|> post("/api/pleroma/blocks_import", %{
"list" => %Plug.Upload{path: "blocks_list.txt"}
})
|> json_response_and_validate_schema(200)
- assert [{:ok, job_result}] = ObanHelpers.perform_all()
- assert job_result == users
+ results = ObanHelpers.perform_all()
+
+ returned_users =
+ for {_, returned_user} <- results do
+ returned_user
+ end
+
+ assert returned_users == users
end
end
test "it imports blocks with different nickname variations", %{conn: conn} do
users = [user2, user3, user4, user5, user6] = insert_list(5, :user)
identifiers =
[
user2.ap_id,
user3.nickname,
"@" <> user4.nickname,
user5.nickname <> "@localhost",
"@" <> user6.nickname <> "@localhost"
]
|> Enum.join(" ")
- assert "job started" ==
+ assert "jobs started" ==
conn
|> put_req_header("content-type", "application/json")
|> post("/api/pleroma/blocks_import", %{"list" => identifiers})
|> json_response_and_validate_schema(200)
- assert [{:ok, job_result}] = ObanHelpers.perform_all()
- assert job_result == users
+ results = ObanHelpers.perform_all()
+
+ returned_user_ids =
+ for {_, user} <- results do
+ user.id
+ end
+
+ original_user_ids =
+ for user <- users do
+ user.id
+ end
+
+ assert match?(^original_user_ids, returned_user_ids)
end
end
describe "POST /api/pleroma/mutes_import" do
# Note: "follow" or "write:mutes" permission is required
setup do: oauth_access(["write:mutes"])
test "it returns HTTP 200", %{user: user, conn: conn} do
user2 = insert(:user)
- assert "job started" ==
+ assert "jobs started" ==
conn
|> put_req_header("content-type", "application/json")
|> 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]
+ [{:ok, result_user}] = ObanHelpers.perform_all()
+
+ assert result_user == refresh_record(user2)
assert Pleroma.User.mutes?(user, user2)
end
test "it imports mutes users from file", %{user: user, conn: conn} do
- users = [user2, user3] = insert_list(2, :user)
+ [user2, user3] = insert_list(2, :user)
with_mocks([
{File, [], read!: fn "mutes_list.txt" -> "#{user2.ap_id} #{user3.ap_id}" end}
]) do
- assert "job started" ==
+ assert "jobs started" ==
conn
|> put_req_header("content-type", "application/json")
|> post("/api/pleroma/mutes_import", %{
"list" => %Plug.Upload{path: "mutes_list.txt"}
})
|> json_response_and_validate_schema(200)
- assert [{:ok, job_result}] = ObanHelpers.perform_all()
- assert job_result == users
- assert Enum.all?(users, &Pleroma.User.mutes?(user, &1))
+ results = ObanHelpers.perform_all()
+
+ returned_users =
+ for {_, returned_user} <- results do
+ returned_user
+ end
+
+ assert Enum.all?(returned_users, &Pleroma.User.mutes?(user, &1))
end
end
test "it imports mutes with different nickname variations", %{user: user, conn: conn} do
- users = [user2, user3, user4, user5, user6] = insert_list(5, :user)
+ [user2, user3, user4, user5, user6] = insert_list(5, :user)
identifiers =
[
user2.ap_id,
user3.nickname,
"@" <> user4.nickname,
user5.nickname <> "@localhost",
"@" <> user6.nickname <> "@localhost"
]
|> Enum.join(" ")
- assert "job started" ==
+ assert "jobs started" ==
conn
|> put_req_header("content-type", "application/json")
|> post("/api/pleroma/mutes_import", %{"list" => identifiers})
|> json_response_and_validate_schema(200)
- assert [{:ok, job_result}] = ObanHelpers.perform_all()
- assert job_result == users
- assert Enum.all?(users, &Pleroma.User.mutes?(user, &1))
+ results = ObanHelpers.perform_all()
+
+ returned_users =
+ for {_, returned_user} <- results do
+ returned_user
+ end
+
+ assert Enum.all?(returned_users, &Pleroma.User.mutes?(user, &1))
end
end
end

File Metadata

Mime Type
text/x-diff
Expires
Fri, Oct 30, 11:02 PM (17 h, 44 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1800695
Default Alt Text
(23 KB)

Event Timeline