commit: 252528a4b9ca3a5d92f1676c44989ad7d8777de1
parent: 2b57f73b776d0de3c91b3b0dd19e4458c205f690
Author: rinpatch <rinpatch@sdf.org>
Date: Thu, 16 Apr 2020 14:59:11 +0000
Merge branch 'refactor/remove-upgrade-changeset' into 'develop'
Remove User.upgrade_changeset in favor of remote_user_creation
See merge request pleroma/pleroma!2368
Diffstat:
5 files changed, 37 insertions(+), 127 deletions(-)
diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
@@ -339,18 +339,26 @@ defmodule Pleroma.User do
end
end
- def remote_user_creation(params) do
+ def remote_user_changeset(struct \\ %User{local: false}, params) do
bio_limit = Pleroma.Config.get([:instance, :user_bio_length], 5000)
name_limit = Pleroma.Config.get([:instance, :user_name_length], 100)
+ name =
+ case params[:name] do
+ name when is_binary(name) and byte_size(name) > 0 -> name
+ _ -> params[:nickname]
+ end
+
params =
params
+ |> Map.put(:name, name)
+ |> Map.put_new(:last_refreshed_at, NaiveDateTime.utc_now())
|> truncate_if_exists(:name, name_limit)
|> truncate_if_exists(:bio, bio_limit)
|> truncate_fields_param()
changeset =
- %User{local: false}
+ struct
|> cast(
params,
[
@@ -375,7 +383,8 @@ defmodule Pleroma.User do
:discoverable,
:invisible,
:actor_type,
- :also_known_as
+ :also_known_as,
+ :last_refreshed_at
]
)
|> validate_required([:name, :ap_id])
@@ -488,49 +497,6 @@ defmodule Pleroma.User do
end
end
- def upgrade_changeset(struct, params \\ %{}, remote? \\ false) do
- bio_limit = Pleroma.Config.get([:instance, :user_bio_length], 5000)
- name_limit = Pleroma.Config.get([:instance, :user_name_length], 100)
-
- params = Map.put(params, :last_refreshed_at, NaiveDateTime.utc_now())
-
- params = if remote?, do: truncate_fields_param(params), else: params
-
- struct
- |> cast(
- params,
- [
- :bio,
- :name,
- :follower_address,
- :following_address,
- :avatar,
- :last_refreshed_at,
- :ap_enabled,
- :source_data,
- :banner,
- :locked,
- :magic_key,
- :follower_count,
- :following_count,
- :hide_follows,
- :fields,
- :hide_followers,
- :allow_following_move,
- :discoverable,
- :hide_followers_count,
- :hide_follows_count,
- :actor_type,
- :also_known_as
- ]
- )
- |> unique_constraint(:nickname)
- |> validate_format(:nickname, local_nickname_regex())
- |> validate_length(:bio, max: bio_limit)
- |> validate_length(:name, max: name_limit)
- |> validate_fields(remote?)
- end
-
def update_as_admin_changeset(struct, params) do
struct
|> update_changeset(params)
@@ -1643,17 +1609,6 @@ defmodule Pleroma.User do
end
end
- defp blank?(""), do: nil
- defp blank?(n), do: n
-
- def insert_or_update_user(data) do
- data
- |> Map.put(:name, blank?(data[:name]) || data[:nickname])
- |> remote_user_creation()
- |> Repo.insert(on_conflict: {:replace_all_except, [:id]}, conflict_target: :nickname)
- |> set_cache()
- end
-
def ap_enabled?(%User{local: true}), do: true
def ap_enabled?(%User{ap_enabled: ap_enabled}), do: ap_enabled
def ap_enabled?(_), do: false
diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -1551,11 +1551,22 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
end
def make_user_from_ap_id(ap_id) do
- if _user = User.get_cached_by_ap_id(ap_id) do
+ user = User.get_cached_by_ap_id(ap_id)
+
+ if user && !User.ap_enabled?(user) do
Transmogrifier.upgrade_user_from_ap_id(ap_id)
else
with {:ok, data} <- fetch_and_prepare_user_from_ap_id(ap_id) do
- User.insert_or_update_user(data)
+ if user do
+ user
+ |> User.remote_user_changeset(data)
+ |> User.update_and_set_cache()
+ else
+ data
+ |> User.remote_user_changeset()
+ |> Repo.insert()
+ |> User.set_cache()
+ end
else
e -> {:error, e}
end
diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex
@@ -711,7 +711,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
{:ok, new_user_data} = ActivityPub.user_data_from_user_object(object)
actor
- |> User.upgrade_changeset(new_user_data, true)
+ |> User.remote_user_changeset(new_user_data)
|> User.update_and_set_cache()
ActivityPub.update(%{
@@ -1254,12 +1254,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
def upgrade_user_from_ap_id(ap_id) do
with %User{local: false} = user <- User.get_cached_by_ap_id(ap_id),
{:ok, data} <- ActivityPub.fetch_and_prepare_user_from_ap_id(ap_id),
- already_ap <- User.ap_enabled?(user),
- {:ok, user} <- upgrade_user(user, data) do
- if not already_ap do
- TransmogrifierWorker.enqueue("user_upgrade", %{"user_id" => user.id})
- end
-
+ {:ok, user} <- update_user(user, data) do
+ TransmogrifierWorker.enqueue("user_upgrade", %{"user_id" => user.id})
{:ok, user}
else
%User{} = user -> {:ok, user}
@@ -1267,9 +1263,9 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
end
end
- defp upgrade_user(user, data) do
+ defp update_user(user, data) do
user
- |> User.upgrade_changeset(data, true)
+ |> User.remote_user_changeset(data)
|> User.update_and_set_cache()
end
diff --git a/test/user_test.exs b/test/user_test.exs
@@ -610,7 +610,7 @@ defmodule Pleroma.UserTest do
) <> "/followers"
end
- describe "remote user creation changeset" do
+ describe "remote user changeset" do
@valid_remote %{
bio: "hello",
name: "Someone",
@@ -622,28 +622,28 @@ defmodule Pleroma.UserTest do
setup do: clear_config([:instance, :user_name_length])
test "it confirms validity" do
- cs = User.remote_user_creation(@valid_remote)
+ cs = User.remote_user_changeset(@valid_remote)
assert cs.valid?
end
test "it sets the follower_adress" do
- cs = User.remote_user_creation(@valid_remote)
+ cs = User.remote_user_changeset(@valid_remote)
# remote users get a fake local follower address
assert cs.changes.follower_address ==
User.ap_followers(%User{nickname: @valid_remote[:nickname]})
end
test "it enforces the fqn format for nicknames" do
- cs = User.remote_user_creation(%{@valid_remote | nickname: "bla"})
+ cs = User.remote_user_changeset(%{@valid_remote | nickname: "bla"})
assert Ecto.Changeset.get_field(cs, :local) == false
assert cs.changes.avatar
refute cs.valid?
end
test "it has required fields" do
- [:name, :ap_id]
+ [:ap_id]
|> Enum.each(fn field ->
- cs = User.remote_user_creation(Map.delete(@valid_remote, field))
+ cs = User.remote_user_changeset(Map.delete(@valid_remote, field))
refute cs.valid?
end)
end
@@ -1199,58 +1199,6 @@ defmodule Pleroma.UserTest do
assert {:ok, _key} = User.get_public_key_for_ap_id("http://mastodon.example.org/users/admin")
end
- describe "insert or update a user from given data" do
- test "with normal data" do
- user = insert(:user, %{nickname: "nick@name.de"})
- data = %{ap_id: user.ap_id <> "xxx", name: user.name, nickname: user.nickname}
-
- assert {:ok, %User{}} = User.insert_or_update_user(data)
- end
-
- test "with overly long fields" do
- current_max_length = Pleroma.Config.get([:instance, :account_field_value_length], 255)
- user = insert(:user, nickname: "nickname@supergood.domain")
-
- data = %{
- ap_id: user.ap_id,
- name: user.name,
- nickname: user.nickname,
- fields: [
- %{"name" => "myfield", "value" => String.duplicate("h", current_max_length + 1)}
- ]
- }
-
- assert {:ok, %User{}} = User.insert_or_update_user(data)
- end
-
- test "with an overly long bio" do
- current_max_length = Pleroma.Config.get([:instance, :user_bio_length], 5000)
- user = insert(:user, nickname: "nickname@supergood.domain")
-
- data = %{
- ap_id: user.ap_id,
- name: user.name,
- nickname: user.nickname,
- bio: String.duplicate("h", current_max_length + 1)
- }
-
- assert {:ok, %User{}} = User.insert_or_update_user(data)
- end
-
- test "with an overly long display name" do
- current_max_length = Pleroma.Config.get([:instance, :user_name_length], 100)
- user = insert(:user, nickname: "nickname@supergood.domain")
-
- data = %{
- ap_id: user.ap_id,
- name: String.duplicate("h", current_max_length + 1),
- nickname: user.nickname
- }
-
- assert {:ok, %User{}} = User.insert_or_update_user(data)
- end
- end
-
describe "per-user rich-text filtering" do
test "html_filter_policy returns default policies, when rich-text is enabled" do
user = insert(:user)
diff --git a/test/web/activity_pub/views/user_view_test.exs b/test/web/activity_pub/views/user_view_test.exs
@@ -29,7 +29,7 @@ defmodule Pleroma.Web.ActivityPub.UserViewTest do
{:ok, user} =
insert(:user)
- |> User.upgrade_changeset(%{fields: fields})
+ |> User.update_changeset(%{fields: fields})
|> User.update_and_set_cache()
assert %{
