From 79b25be4e1e9e97277a831c98ccea86a038914de Mon Sep 17 00:00:00 2001
From: Egor Kislitsyn <egor@kislitsyn.com>
Date: Tue, 24 Sep 2019 14:16:52 +0700
Subject: [PATCH 1/4] Do not return tuple when unneeded

---
 lib/mix/tasks/pleroma/user.ex                 |  8 +--
 lib/pleroma/user.ex                           | 72 ++++++++-----------
 lib/pleroma/web/activity_pub/publisher.ex     |  4 +-
 .../controllers/mastodon_api_controller.ex    | 10 +--
 test/user_test.exs                            | 22 +++---
 5 files changed, 53 insertions(+), 63 deletions(-)

diff --git a/lib/mix/tasks/pleroma/user.ex b/lib/mix/tasks/pleroma/user.ex
index eb0052144..84c923901 100644
--- a/lib/mix/tasks/pleroma/user.ex
+++ b/lib/mix/tasks/pleroma/user.ex
@@ -228,9 +228,9 @@ def run(["unsubscribe", nickname]) do
       shell_info("Deactivating #{user.nickname}")
       User.deactivate(user)
 
-      {:ok, friends} = User.get_friends(user)
-
-      Enum.each(friends, fn friend ->
+      user
+      |> User.get_friends()
+      |> Enum.each(fn friend ->
         user = User.get_cached_by_id(user.id)
 
         shell_info("Unsubscribing #{friend.nickname} from #{user.nickname}")
@@ -405,7 +405,7 @@ def run(["delete_activities", nickname]) do
     start_pleroma()
 
     with %User{local: true} = user <- User.get_cached_by_nickname(nickname) do
-      {:ok, _} = User.delete_user_activities(user)
+      User.delete_user_activities(user)
       shell_info("User #{nickname} statuses deleted.")
     else
       _ ->
diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index ab253a274..8d126933b 100644
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -685,9 +685,9 @@ def get_followers_query(user), do: get_followers_query(user, nil)
 
   @spec get_followers(User.t(), pos_integer()) :: {:ok, list(User.t())}
   def get_followers(user, page \\ nil) do
-    q = get_followers_query(user, page)
-
-    {:ok, Repo.all(q)}
+    user
+    |> get_followers_query(page)
+    |> Repo.all()
   end
 
   @spec get_external_followers(User.t(), pos_integer()) :: {:ok, list(User.t())}
@@ -720,9 +720,9 @@ def get_friends_query(user, page) do
   def get_friends_query(user), do: get_friends_query(user, nil)
 
   def get_friends(user, page \\ nil) do
-    q = get_friends_query(user, page)
-
-    {:ok, Repo.all(q)}
+    user
+    |> get_friends_query(page)
+    |> Repo.all()
   end
 
   def get_friends_ids(user, page \\ nil) do
@@ -733,15 +733,13 @@ def get_friends_ids(user, page \\ nil) do
 
   @spec get_follow_requests(User.t()) :: {:ok, [User.t()]}
   def get_follow_requests(%User{} = user) do
-    users =
-      Activity.follow_requests_for_actor(user)
-      |> join(:inner, [a], u in User, on: a.actor == u.ap_id)
-      |> where([a, u], not fragment("? @> ?", u.following, ^[user.follower_address]))
-      |> group_by([a, u], u.id)
-      |> select([a, u], u)
-      |> Repo.all()
-
-    {:ok, users}
+    user
+    |> Activity.follow_requests_for_actor()
+    |> join(:inner, [a], u in User, on: a.actor == u.ap_id)
+    |> where([a, u], not fragment("? @> ?", u.following, ^[user.follower_address]))
+    |> group_by([a, u], u.id)
+    |> select([a, u], u)
+    |> Repo.all()
   end
 
   def increase_note_count(%User{} = user) do
@@ -1104,15 +1102,13 @@ def deactivate_async(user, status \\ true) do
   def deactivate(%User{} = user, status \\ true) do
     info_cng = User.Info.set_activation_status(user.info, status)
 
-    with {:ok, friends} <- User.get_friends(user),
-         {:ok, followers} <- User.get_followers(user),
-         {:ok, user} <-
+    with {:ok, user} <-
            user
            |> change()
            |> put_embed(:info, info_cng)
            |> update_and_set_cache() do
-      Enum.each(followers, &invalidate_cache(&1))
-      Enum.each(friends, &update_follower_count(&1))
+      Enum.each(get_followers(user), &invalidate_cache/1)
+      Enum.each(get_friends(user), &update_follower_count/1)
 
       {:ok, user}
     end
@@ -1137,18 +1133,18 @@ def perform(:delete, %User{} = user) do
     {:ok, _user} = ActivityPub.delete(user)
 
     # Remove all relationships
-    {:ok, followers} = User.get_followers(user)
-
-    Enum.each(followers, fn follower ->
+    user
+    |> get_followers()
+    |> Enum.each(fn follower ->
       ActivityPub.unfollow(follower, user)
-      User.unfollow(follower, user)
+      unfollow(follower, user)
     end)
 
-    {:ok, friends} = User.get_friends(user)
-
-    Enum.each(friends, fn followed ->
+    user
+    |> get_friends()
+    |> Enum.each(fn followed ->
       ActivityPub.unfollow(user, followed)
-      User.unfollow(user, followed)
+      unfollow(user, followed)
     end)
 
     delete_user_activities(user)
@@ -1160,13 +1156,11 @@ def perform(:delete, %User{} = user) do
   def perform(:fetch_initial_posts, %User{} = user) do
     pages = Pleroma.Config.get!([:fetch_initial_posts, :pages])
 
-    Enum.each(
-      # Insert all the posts in reverse order, so they're in the right order on the timeline
-      Enum.reverse(Utils.fetch_ordered_collection(user.info.source_data["outbox"], pages)),
-      &Pleroma.Web.Federator.incoming_ap_doc/1
-    )
-
-    {:ok, user}
+    # Insert all the posts in reverse order, so they're in the right order on the timeline
+    user.info.source_data["outbox"]
+    |> Utils.fetch_ordered_collection(pages)
+    |> Enum.reverse()
+    |> Enum.each(&Pleroma.Web.Federator.incoming_ap_doc/1)
   end
 
   def perform(:deactivate_async, user, status), do: deactivate(user, status)
@@ -1252,16 +1246,12 @@ def follow_import(%User{} = follower, followed_identifiers)
     })
   end
 
-  def delete_user_activities(%User{ap_id: ap_id} = user) do
+  def delete_user_activities(%User{ap_id: ap_id}) do
     ap_id
     |> Activity.Queries.by_actor()
     |> RepoStreamer.chunk_stream(50)
-    |> Stream.each(fn activities ->
-      Enum.each(activities, &delete_activity(&1))
-    end)
+    |> Stream.each(fn activities -> Enum.each(activities, &delete_activity/1) end)
     |> Stream.run()
-
-    {:ok, user}
   end
 
   defp delete_activity(%{data: %{"type" => "Create"}} = activity) do
diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex
index 114251b24..3866dacee 100644
--- a/lib/pleroma/web/activity_pub/publisher.ex
+++ b/lib/pleroma/web/activity_pub/publisher.ex
@@ -111,11 +111,11 @@ defp should_federate?(inbox, public) do
 
   @spec recipients(User.t(), Activity.t()) :: list(User.t()) | []
   defp recipients(actor, activity) do
-    {:ok, followers} =
+    followers =
       if actor.follower_address in activity.recipients do
         User.get_external_followers(actor)
       else
-        {:ok, []}
+        []
       end
 
     fetchers =
diff --git a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
index 6421c2c53..270c74089 100644
--- a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
@@ -958,11 +958,11 @@ def following(%{assigns: %{user: for_user}} = conn, %{"id" => id} = params) do
   end
 
   def follow_requests(%{assigns: %{user: followed}} = conn, _params) do
-    with {:ok, follow_requests} <- User.get_follow_requests(followed) do
-      conn
-      |> put_view(AccountView)
-      |> render("accounts.json", %{for: followed, users: follow_requests, as: :user})
-    end
+    follow_requests = User.get_follow_requests(followed)
+
+    conn
+    |> put_view(AccountView)
+    |> render("accounts.json", %{for: followed, users: follow_requests, as: :user})
   end
 
   def authorize_follow_request(%{assigns: %{user: followed}} = conn, %{"id" => id}) do
diff --git a/test/user_test.exs b/test/user_test.exs
index aebe7aa06..21ea1d28e 100644
--- a/test/user_test.exs
+++ b/test/user_test.exs
@@ -74,8 +74,8 @@ test "returns all pending follow requests" do
     CommonAPI.follow(follower, unlocked)
     CommonAPI.follow(follower, locked)
 
-    assert {:ok, []} = User.get_follow_requests(unlocked)
-    assert {:ok, [activity]} = User.get_follow_requests(locked)
+    assert [] = User.get_follow_requests(unlocked)
+    assert [activity] = User.get_follow_requests(locked)
 
     assert activity
   end
@@ -90,7 +90,7 @@ test "doesn't return already accepted or duplicate follow requests" do
     CommonAPI.follow(accepted_follower, locked)
     User.follow(accepted_follower, locked)
 
-    assert {:ok, [activity]} = User.get_follow_requests(locked)
+    assert [activity] = User.get_follow_requests(locked)
     assert activity
   end
 
@@ -99,10 +99,10 @@ test "clears follow requests when requester is blocked" do
     follower = insert(:user)
 
     CommonAPI.follow(follower, followed)
-    assert {:ok, [_activity]} = User.get_follow_requests(followed)
+    assert [_activity] = User.get_follow_requests(followed)
 
     {:ok, _follower} = User.block(followed, follower)
-    assert {:ok, []} = User.get_follow_requests(followed)
+    assert [] = User.get_follow_requests(followed)
   end
 
   test "follow_all follows mutliple users" do
@@ -560,7 +560,7 @@ test "it sets the follower_adress" do
 
     test "it enforces the fqn format for nicknames" do
       cs = User.remote_user_creation(%{@valid_remote | nickname: "bla"})
-      assert cs.changes.local == false
+      assert Ecto.Changeset.get_field(cs, :local) == false
       assert cs.changes.avatar
       refute cs.valid?
     end
@@ -584,7 +584,7 @@ test "gets all followers for a given user" do
       {:ok, follower_one} = User.follow(follower_one, user)
       {:ok, follower_two} = User.follow(follower_two, user)
 
-      {:ok, res} = User.get_followers(user)
+      res = User.get_followers(user)
 
       assert Enum.member?(res, follower_one)
       assert Enum.member?(res, follower_two)
@@ -600,7 +600,7 @@ test "gets all friends (followed users) for a given user" do
       {:ok, user} = User.follow(user, followed_one)
       {:ok, user} = User.follow(user, followed_two)
 
-      {:ok, res} = User.get_friends(user)
+      res = User.get_friends(user)
 
       followed_one = User.get_cached_by_ap_id(followed_one.ap_id)
       followed_two = User.get_cached_by_ap_id(followed_two.ap_id)
@@ -975,7 +975,7 @@ test "hide a user from followers " do
       info = User.get_cached_user_info(user2)
 
       assert info.follower_count == 0
-      assert {:ok, []} = User.get_followers(user2)
+      assert [] = User.get_followers(user2)
     end
 
     test "hide a user from friends" do
@@ -991,7 +991,7 @@ test "hide a user from friends" do
 
       assert info.following_count == 0
       assert User.following_count(user2) == 0
-      assert {:ok, []} = User.get_friends(user2)
+      assert [] = User.get_friends(user2)
     end
 
     test "hide a user's statuses from timelines and notifications" do
@@ -1034,7 +1034,7 @@ test "hide a user's statuses from timelines and notifications" do
     test ".delete_user_activities deletes all create activities", %{user: user} do
       {:ok, activity} = CommonAPI.post(user, %{"status" => "2hu"})
 
-      {:ok, _} = User.delete_user_activities(user)
+      User.delete_user_activities(user)
 
       # TODO: Remove favorites, repeats, delete activities.
       refute Activity.get_by_id(activity.id)

From 209395c7e60afe7115f22afd6936d9c6bdd7bb72 Mon Sep 17 00:00:00 2001
From: Egor Kislitsyn <egor@kislitsyn.com>
Date: Tue, 24 Sep 2019 19:50:07 +0700
Subject: [PATCH 2/4] Add User.change_info/2 and User.update_info/2

---
 lib/mix/tasks/pleroma/user.ex                 |  25 +--
 lib/pleroma/user.ex                           | 173 +++++-------------
 lib/pleroma/user/info.ex                      |  24 +--
 .../web/admin_api/admin_api_controller.ex     |  59 +++---
 lib/pleroma/web/common_api/common_api.ex      |  28 +--
 .../controllers/mastodon_api_controller.ex    |  68 +++----
 .../web/twitter_api/twitter_api_controller.ex |   8 +-
 test/tasks/database_test.exs                  |   4 +-
 test/user_test.exs                            |  18 ++
 .../mastodon_api_controller_test.exs          |  17 +-
 test/web/oauth/oauth_controller_test.exs      |   9 +-
 test/web/ostatus/ostatus_controller_test.exs  |  20 +-
 12 files changed, 146 insertions(+), 307 deletions(-)

diff --git a/lib/mix/tasks/pleroma/user.ex b/lib/mix/tasks/pleroma/user.ex
index 84c923901..d93ba8dee 100644
--- a/lib/mix/tasks/pleroma/user.ex
+++ b/lib/mix/tasks/pleroma/user.ex
@@ -4,7 +4,6 @@
 
 defmodule Mix.Tasks.Pleroma.User do
   use Mix.Task
-  import Ecto.Changeset
   import Mix.Pleroma
   alias Pleroma.User
   alias Pleroma.UserInviteToken
@@ -443,39 +442,21 @@ def run(["sign_out", nickname]) do
   end
 
   defp set_moderator(user, value) do
-    info_cng = User.Info.admin_api_update(user.info, %{is_moderator: value})
-
-    user_cng =
-      Ecto.Changeset.change(user)
-      |> put_embed(:info, info_cng)
-
-    {:ok, user} = User.update_and_set_cache(user_cng)
+    {:ok, user} = User.update_info(user, &User.Info.admin_api_update(&1, %{is_moderator: value}))
 
     shell_info("Moderator status of #{user.nickname}: #{user.info.is_moderator}")
     user
   end
 
   defp set_admin(user, value) do
-    info_cng = User.Info.admin_api_update(user.info, %{is_admin: value})
-
-    user_cng =
-      Ecto.Changeset.change(user)
-      |> put_embed(:info, info_cng)
-
-    {:ok, user} = User.update_and_set_cache(user_cng)
+    {:ok, user} = User.update_info(user, &User.Info.admin_api_update(&1, %{is_admin: value}))
 
     shell_info("Admin status of #{user.nickname}: #{user.info.is_admin}")
     user
   end
 
   defp set_locked(user, value) do
-    info_cng = User.Info.user_upgrade(user.info, %{locked: value})
-
-    user_cng =
-      Ecto.Changeset.change(user)
-      |> put_embed(:info, info_cng)
-
-    {:ok, user} = User.update_and_set_cache(user_cng)
+    {:ok, user} = User.update_info(user, &User.Info.user_upgrade(&1, %{locked: value}))
 
     shell_info("Locked status of #{user.nickname}: #{user.info.locked}")
     user
diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index 8d126933b..422bc6fa6 100644
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -197,8 +197,6 @@ def remote_user_creation(params) do
       |> truncate_if_exists(:name, name_limit)
       |> truncate_if_exists(:bio, bio_limit)
 
-    info_cng = User.Info.remote_user_creation(%User.Info{}, params[:info])
-
     changes =
       %User{}
       |> cast(params, [:bio, :name, :ap_id, :nickname, :avatar])
@@ -208,7 +206,7 @@ def remote_user_creation(params) do
       |> validate_length(:bio, max: bio_limit)
       |> validate_length(:name, max: name_limit)
       |> put_change(:local, false)
-      |> put_embed(:info, info_cng)
+      |> change_info(&User.Info.remote_user_creation(&1, params[:info]))
 
     if changes.valid? do
       case info_cng.changes[:source_data] do
@@ -245,7 +243,6 @@ def upgrade_changeset(struct, params \\ %{}, remote? \\ false) do
     name_limit = Pleroma.Config.get([:instance, :user_name_length], 100)
 
     params = Map.put(params, :last_refreshed_at, NaiveDateTime.utc_now())
-    info_cng = User.Info.user_upgrade(struct.info, params[:info], remote?)
 
     struct
     |> cast(params, [
@@ -260,7 +257,7 @@ def upgrade_changeset(struct, params \\ %{}, remote? \\ false) do
     |> validate_format(:nickname, local_nickname_regex())
     |> validate_length(:bio, max: bio_limit)
     |> validate_length(:name, max: name_limit)
-    |> put_embed(:info, info_cng)
+    |> change_info(&User.Info.user_upgrade(&1, params[:info], remote?))
   end
 
   def password_update_changeset(struct, params) do
@@ -785,21 +782,15 @@ def decrease_note_count(%User{} = user) do
   end
 
   def update_note_count(%User{} = user) do
-    note_count_query =
+    note_count =
       from(
         a in Object,
         where: fragment("?->>'actor' = ? and ?->>'type' = 'Note'", a.data, ^user.ap_id, a.data),
         select: count(a.id)
       )
+      |> Repo.one()
 
-    note_count = Repo.one(note_count_query)
-
-    info_cng = User.Info.set_note_count(user.info, note_count)
-
-    user
-    |> change()
-    |> put_embed(:info, info_cng)
-    |> update_and_set_cache()
+    update_info(user, &User.Info.set_note_count(&1, note_count))
   end
 
   @spec maybe_fetch_follow_information(User.t()) :: User.t()
@@ -816,17 +807,7 @@ def maybe_fetch_follow_information(user) do
 
   def fetch_follow_information(user) do
     with {:ok, info} <- ActivityPub.fetch_follow_information_for_user(user) do
-      info_cng = User.Info.follow_information_update(user.info, info)
-
-      changeset =
-        user
-        |> change()
-        |> put_embed(:info, info_cng)
-
-      update_and_set_cache(changeset)
-    else
-      {:error, _} = e -> e
-      e -> {:error, e}
+      update_info(user, &User.Info.follow_information_update(&1, info))
     end
   end
 
@@ -900,31 +881,11 @@ def get_recipients_from_activity(%Activity{recipients: to}) do
 
   @spec mute(User.t(), User.t(), boolean()) :: {:ok, User.t()} | {:error, String.t()}
   def mute(muter, %User{ap_id: ap_id}, notifications? \\ true) do
-    info = muter.info
-
-    info_cng =
-      User.Info.add_to_mutes(info, ap_id)
-      |> User.Info.add_to_muted_notifications(info, ap_id, notifications?)
-
-    cng =
-      change(muter)
-      |> put_embed(:info, info_cng)
-
-    update_and_set_cache(cng)
+    update_info(muter, &User.Info.add_to_mutes(&1, ap_id, notifications?))
   end
 
   def unmute(muter, %{ap_id: ap_id}) do
-    info = muter.info
-
-    info_cng =
-      User.Info.remove_from_mutes(info, ap_id)
-      |> User.Info.remove_from_muted_notifications(info, ap_id)
-
-    cng =
-      change(muter)
-      |> put_embed(:info, info_cng)
-
-    update_and_set_cache(cng)
+    update_info(muter, &User.Info.remove_from_mutes(&1, ap_id))
   end
 
   def subscribe(subscriber, %{ap_id: ap_id}) do
@@ -936,26 +897,14 @@ def subscribe(subscriber, %{ap_id: ap_id}) do
       if blocked do
         {:error, "Could not subscribe: #{subscribed.nickname} is blocking you"}
       else
-        info_cng =
-          subscribed.info
-          |> User.Info.add_to_subscribers(subscriber.ap_id)
-
-        change(subscribed)
-        |> put_embed(:info, info_cng)
-        |> update_and_set_cache()
+        update_info(subscribed, &User.Info.add_to_subscribers(&1, subscriber.ap_id))
       end
     end
   end
 
   def unsubscribe(unsubscriber, %{ap_id: ap_id}) do
     with %User{} = user <- get_cached_by_ap_id(ap_id) do
-      info_cng =
-        user.info
-        |> User.Info.remove_from_subscribers(unsubscriber.ap_id)
-
-      change(user)
-      |> put_embed(:info, info_cng)
-      |> update_and_set_cache()
+      update_info(user, &User.Info.remove_from_subscribers(&1, unsubscriber.ap_id))
     end
   end
 
@@ -990,15 +939,7 @@ def block(blocker, %User{ap_id: ap_id} = blocked) do
 
     {:ok, blocker} = update_follower_count(blocker)
 
-    info_cng =
-      blocker.info
-      |> User.Info.add_to_block(ap_id)
-
-    cng =
-      change(blocker)
-      |> put_embed(:info, info_cng)
-
-    update_and_set_cache(cng)
+    update_info(blocker, &User.Info.add_to_block(&1, ap_id))
   end
 
   # helper to handle the block given only an actor's AP id
@@ -1007,15 +948,7 @@ def block(blocker, %{ap_id: ap_id}) do
   end
 
   def unblock(blocker, %{ap_id: ap_id}) do
-    info_cng =
-      blocker.info
-      |> User.Info.remove_from_block(ap_id)
-
-    cng =
-      change(blocker)
-      |> put_embed(:info, info_cng)
-
-    update_and_set_cache(cng)
+    update_info(blocker, &User.Info.remove_from_block(&1, ap_id))
   end
 
   def mutes?(nil, _), do: false
@@ -1072,27 +1005,11 @@ def subscribers(user) do
   end
 
   def block_domain(user, domain) do
-    info_cng =
-      user.info
-      |> User.Info.add_to_domain_block(domain)
-
-    cng =
-      change(user)
-      |> put_embed(:info, info_cng)
-
-    update_and_set_cache(cng)
+    update_info(user, &User.Info.add_to_domain_block(&1, domain))
   end
 
   def unblock_domain(user, domain) do
-    info_cng =
-      user.info
-      |> User.Info.remove_from_domain_block(domain)
-
-    cng =
-      change(user)
-      |> put_embed(:info, info_cng)
-
-    update_and_set_cache(cng)
+    update_info(user, &User.Info.remove_from_domain_block(&1, domain))
   end
 
   def deactivate_async(user, status \\ true) do
@@ -1100,13 +1017,7 @@ def deactivate_async(user, status \\ true) do
   end
 
   def deactivate(%User{} = user, status \\ true) do
-    info_cng = User.Info.set_activation_status(user.info, status)
-
-    with {:ok, user} <-
-           user
-           |> change()
-           |> put_embed(:info, info_cng)
-           |> update_and_set_cache() do
+    with {:ok, user} <- update_info(user, &User.Info.set_activation_status(&1, status)) do
       Enum.each(get_followers(user), &invalidate_cache/1)
       Enum.each(get_friends(user), &update_follower_count/1)
 
@@ -1115,11 +1026,7 @@ def deactivate(%User{} = user, status \\ true) do
   end
 
   def update_notification_settings(%User{} = user, settings \\ %{}) do
-    info_changeset = User.Info.update_notification_settings(user.info, settings)
-
-    change(user)
-    |> put_embed(:info, info_changeset)
-    |> update_and_set_cache()
+    update_info(user, &User.Info.update_notification_settings(&1, settings))
   end
 
   def delete(%User{} = user) do
@@ -1560,11 +1467,7 @@ def list_inactive_users_query(inactivity_threshold \\ 7) do
   @spec switch_email_notifications(t(), String.t(), boolean()) ::
           {:ok, t()} | {:error, Ecto.Changeset.t()}
   def switch_email_notifications(user, type, status) do
-    info = Pleroma.User.Info.update_email_notifications(user.info, %{type => status})
-
-    change(user)
-    |> put_embed(:info, info)
-    |> update_and_set_cache()
+    update_info(user, &User.Info.update_email_notifications(&1, %{type => status}))
   end
 
   @doc """
@@ -1586,13 +1489,8 @@ def touch_last_digest_emailed_at(user) do
   def toggle_confirmation(%User{} = user) do
     need_confirmation? = !user.info.confirmation_pending
 
-    info_changeset =
-      User.Info.confirmation_changeset(user.info, need_confirmation: need_confirmation?)
-
     user
-    |> change()
-    |> put_embed(:info, info_changeset)
-    |> update_and_set_cache()
+    |> update_info(&User.Info.confirmation_changeset(&1, need_confirmation: need_confirmation?))
   end
 
   def get_mascot(%{info: %{mascot: %{} = mascot}}) when not is_nil(mascot) do
@@ -1615,16 +1513,11 @@ def get_mascot(%{info: %{mascot: mascot}}) when is_nil(mascot) do
     }
   end
 
-  def ensure_keys_present(%User{info: info} = user) do
-    if info.keys do
-      {:ok, user}
-    else
-      {:ok, pem} = Keys.generate_rsa_pem()
+  def ensure_keys_present(%{info: %{keys: keys}} = user) when not is_nil(keys), do: {:ok, user}
 
-      user
-      |> Ecto.Changeset.change()
-      |> Ecto.Changeset.put_embed(:info, User.Info.set_keys(info, pem))
-      |> update_and_set_cache()
+  def ensure_keys_present(%User{} = user) do
+    with {:ok, pem} <- Keys.generate_rsa_pem() do
+      update_info(user, &User.Info.set_keys(&1, pem))
     end
   end
 
@@ -1670,4 +1563,26 @@ def change_email(user, email) do
     |> validate_format(:email, @email_regex)
     |> update_and_set_cache()
   end
+
+  @doc """
+  Changes `user.info` and returns the user changeset.
+
+  `fun` is called with the `user.info`.
+  """
+  def change_info(user, fun) do
+    changeset = change(user)
+    info = get_field(changeset, :info) || %User.Info{}
+    put_embed(changeset, :info, fun.(info))
+  end
+
+  @doc """
+  Updates `user.info` and sets cache.
+
+  `fun` is called with the `user.info`.
+  """
+  def update_info(user, fun) do
+    user
+    |> change_info(fun)
+    |> update_and_set_cache()
+  end
 end
diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex
index 99745f496..92e3944f7 100644
--- a/lib/pleroma/user/info.ex
+++ b/lib/pleroma/user/info.ex
@@ -187,16 +187,11 @@ def set_subscribers(info, subscribers) do
     |> validate_required([:subscribers])
   end
 
-  @spec add_to_mutes(Info.t(), String.t()) :: Changeset.t()
-  def add_to_mutes(info, muted) do
-    set_mutes(info, Enum.uniq([muted | info.mutes]))
-  end
-
-  @spec add_to_muted_notifications(Changeset.t(), Info.t(), String.t(), boolean()) ::
-          Changeset.t()
-  def add_to_muted_notifications(changeset, info, muted, notifications?) do
-    set_notification_mutes(
-      changeset,
+  @spec add_to_mutes(Info.t(), String.t(), boolean()) :: Changeset.t()
+  def add_to_mutes(info, muted, notifications?) do
+    info
+    |> set_mutes(Enum.uniq([muted | info.mutes]))
+    |> set_notification_mutes(
       Enum.uniq([muted | info.muted_notifications]),
       notifications?
     )
@@ -204,12 +199,9 @@ def add_to_muted_notifications(changeset, info, muted, notifications?) do
 
   @spec remove_from_mutes(Info.t(), String.t()) :: Changeset.t()
   def remove_from_mutes(info, muted) do
-    set_mutes(info, List.delete(info.mutes, muted))
-  end
-
-  @spec remove_from_muted_notifications(Changeset.t(), Info.t(), String.t()) :: Changeset.t()
-  def remove_from_muted_notifications(changeset, info, muted) do
-    set_notification_mutes(changeset, List.delete(info.muted_notifications, muted), true)
+    info
+    |> set_mutes(List.delete(info.mutes, muted))
+    |> set_notification_mutes(List.delete(info.muted_notifications, muted), true)
   end
 
   def add_to_block(info, blocked) do
diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex
index 0d1db8fa0..6e703d169 100644
--- a/lib/pleroma/web/admin_api/admin_api_controller.ex
+++ b/lib/pleroma/web/admin_api/admin_api_controller.ex
@@ -254,18 +254,12 @@ def right_add(%{assigns: %{user: admin}} = conn, %{
         "nickname" => nickname
       })
       when permission_group in ["moderator", "admin"] do
-    user = User.get_cached_by_nickname(nickname)
+    info = Map.put(%{}, "is_" <> permission_group, true)
 
-    info =
-      %{}
-      |> Map.put("is_" <> permission_group, true)
-
-    info_cng = User.Info.admin_api_update(user.info, info)
-
-    cng =
-      user
-      |> Ecto.Changeset.change()
-      |> Ecto.Changeset.put_embed(:info, info_cng)
+    {:ok, user} =
+      nickname
+      |> User.get_cached_by_nickname()
+      |> User.update_info(&User.Info.admin_api_update(&1, info))
 
     ModerationLog.insert_log(%{
       action: "grant",
@@ -274,8 +268,6 @@ def right_add(%{assigns: %{user: admin}} = conn, %{
       permission: permission_group
     })
 
-    {:ok, _user} = User.update_and_set_cache(cng)
-
     json(conn, info)
   end
 
@@ -293,40 +285,33 @@ def right_get(conn, %{"nickname" => nickname}) do
     })
   end
 
+  def right_delete(%{assigns: %{user: %{nickname: nickname}}} = conn, %{"nickname" => nickname}) do
+    render_error(conn, :forbidden, "You can't revoke your own admin status.")
+  end
+
   def right_delete(
-        %{assigns: %{user: %User{:nickname => admin_nickname} = admin}} = conn,
+        %{assigns: %{user: admin}} = conn,
         %{
           "permission_group" => permission_group,
           "nickname" => nickname
         }
       )
       when permission_group in ["moderator", "admin"] do
-    if admin_nickname == nickname do
-      render_error(conn, :forbidden, "You can't revoke your own admin status.")
-    else
-      user = User.get_cached_by_nickname(nickname)
+    info = Map.put(%{}, "is_" <> permission_group, false)
 
-      info =
-        %{}
-        |> Map.put("is_" <> permission_group, false)
+    {:ok, user} =
+      nickname
+      |> User.get_cached_by_nickname()
+      |> User.update_info(&User.Info.admin_api_update(&1, info))
 
-      info_cng = User.Info.admin_api_update(user.info, info)
+    ModerationLog.insert_log(%{
+      action: "revoke",
+      actor: admin,
+      subject: user,
+      permission: permission_group
+    })
 
-      cng =
-        Ecto.Changeset.change(user)
-        |> Ecto.Changeset.put_embed(:info, info_cng)
-
-      {:ok, _user} = User.update_and_set_cache(cng)
-
-      ModerationLog.insert_log(%{
-        action: "revoke",
-        actor: admin,
-        subject: user,
-        permission: permission_group
-      })
-
-      json(conn, info)
-    end
+    json(conn, info)
   end
 
   def right_delete(conn, _) do
diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex
index 5faddc9f4..2f12ad43a 100644
--- a/lib/pleroma/web/common_api/common_api.ex
+++ b/lib/pleroma/web/common_api/common_api.ex
@@ -302,12 +302,11 @@ def post(user, %{"status" => status} = data) do
 
   # Updates the emojis for a user based on their profile
   def update(user) do
+    emoji = emoji_from_profile(user)
+    source_data = user.info |> Map.get(:source_data, {}) |> Map.put("tag", emoji)
+
     user =
-      with emoji <- emoji_from_profile(user),
-           source_data <- (user.info.source_data || %{}) |> Map.put("tag", emoji),
-           info_cng <- User.Info.set_source_data(user.info, source_data),
-           change <- Ecto.Changeset.change(user) |> Ecto.Changeset.put_embed(:info, info_cng),
-           {:ok, user} <- User.update_and_set_cache(change) do
+      with {:ok, user} <- User.update_info(user, &User.Info.set_source_data(&1, source_data)) do
         user
       else
         _e ->
@@ -336,10 +335,7 @@ def pin(id_or_ap_id, %{ap_id: user_ap_id} = user) do
            }
          } = activity <- get_by_id_or_ap_id(id_or_ap_id),
          true <- Visibility.is_public?(activity),
-         %{valid?: true} = info_changeset <- User.Info.add_pinnned_activity(user.info, activity),
-         changeset <-
-           Ecto.Changeset.change(user) |> Ecto.Changeset.put_embed(:info, info_changeset),
-         {:ok, _user} <- User.update_and_set_cache(changeset) do
+         {:ok, _user} <- User.update_info(user, &User.Info.add_pinnned_activity(&1, activity)) do
       {:ok, activity}
     else
       %{errors: [pinned_activities: {err, _}]} ->
@@ -352,11 +348,7 @@ def pin(id_or_ap_id, %{ap_id: user_ap_id} = user) do
 
   def unpin(id_or_ap_id, user) do
     with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id),
-         %{valid?: true} = info_changeset <-
-           User.Info.remove_pinnned_activity(user.info, activity),
-         changeset <-
-           Ecto.Changeset.change(user) |> Ecto.Changeset.put_embed(:info, info_changeset),
-         {:ok, _user} <- User.update_and_set_cache(changeset) do
+         {:ok, _user} <- User.update_info(user, &User.Info.remove_pinnned_activity(&1, activity)) do
       {:ok, activity}
     else
       %{errors: [pinned_activities: {err, _}]} ->
@@ -462,9 +454,7 @@ def hide_reblogs(user, muted) do
     ap_id = muted.ap_id
 
     if ap_id not in user.info.muted_reblogs do
-      info_changeset = User.Info.add_reblog_mute(user.info, ap_id)
-      changeset = Ecto.Changeset.change(user) |> Ecto.Changeset.put_embed(:info, info_changeset)
-      User.update_and_set_cache(changeset)
+      User.update_info(user, &User.Info.add_reblog_mute(&1, ap_id))
     end
   end
 
@@ -472,9 +462,7 @@ def show_reblogs(user, muted) do
     ap_id = muted.ap_id
 
     if ap_id in user.info.muted_reblogs do
-      info_changeset = User.Info.remove_reblog_mute(user.info, ap_id)
-      changeset = Ecto.Changeset.change(user) |> Ecto.Changeset.put_embed(:info, info_changeset)
-      User.update_and_set_cache(changeset)
+      User.update_info(user, &User.Info.remove_reblog_mute(&1, ap_id))
     end
   end
 end
diff --git a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
index 270c74089..8a5287079 100644
--- a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
@@ -188,14 +188,13 @@ def update_credentials(%{assigns: %{user: user}} = conn, params) do
       end)
       |> Map.put(:emoji, user_info_emojis)
 
-    info_cng = User.Info.profile_update(user.info, info_params)
+    changeset =
+      user
+      |> User.update_changeset(user_params)
+      |> User.change_info(&User.Info.profile_update(&1, info_params))
 
-    with changeset <- User.update_changeset(user, user_params),
-         changeset <- Changeset.put_embed(changeset, :info, info_cng),
-         {:ok, user} <- User.update_and_set_cache(changeset) do
-      if original_user != user do
-        CommonAPI.update(user)
-      end
+    with {:ok, user} <- User.update_and_set_cache(changeset) do
+      if original_user != user, do: CommonAPI.update(user)
 
       json(
         conn,
@@ -225,12 +224,10 @@ def update_avatar(%{assigns: %{user: user}} = conn, params) do
   end
 
   def update_banner(%{assigns: %{user: user}} = conn, %{"banner" => ""}) do
-    with new_info <- %{"banner" => %{}},
-         info_cng <- User.Info.profile_update(user.info, new_info),
-         changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_cng),
-         {:ok, user} <- User.update_and_set_cache(changeset) do
-      CommonAPI.update(user)
+    new_info = %{"banner" => %{}}
 
+    with {:ok, user} <- User.update_info(user, &User.Info.profile_update(&1, new_info)) do
+      CommonAPI.update(user)
       json(conn, %{url: nil})
     end
   end
@@ -238,9 +235,7 @@ def update_banner(%{assigns: %{user: user}} = conn, %{"banner" => ""}) do
   def update_banner(%{assigns: %{user: user}} = conn, params) do
     with {:ok, object} <- ActivityPub.upload(%{"img" => params["banner"]}, type: :banner),
          new_info <- %{"banner" => object.data},
-         info_cng <- User.Info.profile_update(user.info, new_info),
-         changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_cng),
-         {:ok, user} <- User.update_and_set_cache(changeset) do
+         {:ok, user} <- User.update_info(user, &User.Info.profile_update(&1, new_info)) do
       CommonAPI.update(user)
       %{"url" => [%{"href" => href} | _]} = object.data
 
@@ -249,10 +244,9 @@ def update_banner(%{assigns: %{user: user}} = conn, params) do
   end
 
   def update_background(%{assigns: %{user: user}} = conn, %{"img" => ""}) do
-    with new_info <- %{"background" => %{}},
-         info_cng <- User.Info.profile_update(user.info, new_info),
-         changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_cng),
-         {:ok, _user} <- User.update_and_set_cache(changeset) do
+    new_info = %{"background" => %{}}
+
+    with {:ok, _user} <- User.update_info(user, &User.Info.profile_update(&1, new_info)) do
       json(conn, %{url: nil})
     end
   end
@@ -260,9 +254,7 @@ def update_background(%{assigns: %{user: user}} = conn, %{"img" => ""}) do
   def update_background(%{assigns: %{user: user}} = conn, params) do
     with {:ok, object} <- ActivityPub.upload(params, type: :background),
          new_info <- %{"background" => object.data},
-         info_cng <- User.Info.profile_update(user.info, new_info),
-         changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_cng),
-         {:ok, _user} <- User.update_and_set_cache(changeset) do
+         {:ok, _user} <- User.update_info(user, &User.Info.profile_update(&1, new_info)) do
       %{"url" => [%{"href" => href} | _]} = object.data
 
       json(conn, %{url: href})
@@ -816,26 +808,16 @@ def upload(%{assigns: %{user: user}} = conn, %{"file" => file} = data) do
   def set_mascot(%{assigns: %{user: user}} = conn, %{"file" => file}) do
     with {:ok, object} <- ActivityPub.upload(file, actor: User.ap_id(user)),
          %{} = attachment_data <- Map.put(object.data, "id", object.id),
-         %{type: type} = rendered <-
+         # Reject if not an image
+         %{type: "image"} = rendered <-
            StatusView.render("attachment.json", %{attachment: attachment_data}) do
-      # Reject if not an image
-      if type == "image" do
-        # Sure!
-        # Save to the user's info
-        info_changeset = User.Info.mascot_update(user.info, rendered)
+      # Sure!
+      # Save to the user's info
+      {:ok, _user} = User.update_info(user, &User.Info.mascot_update(&1, rendered))
 
-        user_changeset =
-          user
-          |> Changeset.change()
-          |> Changeset.put_embed(:info, info_changeset)
-
-        {:ok, _user} = User.update_and_set_cache(user_changeset)
-
-        conn
-        |> json(rendered)
-      else
-        render_error(conn, :unsupported_media_type, "mascots can only be images")
-      end
+      json(conn, rendered)
+    else
+      %{type: _} -> render_error(conn, :unsupported_media_type, "mascots can only be images")
     end
   end
 
@@ -1366,11 +1348,7 @@ def index(%{assigns: %{user: user}} = conn, _params) do
   end
 
   def put_settings(%{assigns: %{user: user}} = conn, %{"data" => settings} = _params) do
-    info_cng = User.Info.mastodon_settings_update(user.info, settings)
-
-    with changeset <- Changeset.change(user),
-         changeset <- Changeset.put_embed(changeset, :info, info_cng),
-         {:ok, _user} <- User.update_and_set_cache(changeset) do
+    with {:ok, _user} <- User.update_info(user, &User.Info.mastodon_settings_update(&1, settings)) do
       json(conn, %{})
     else
       e ->
diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex
index 42234ae09..27f3664e0 100644
--- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex
+++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex
@@ -5,7 +5,6 @@
 defmodule Pleroma.Web.TwitterAPI.Controller do
   use Pleroma.Web, :controller
 
-  alias Ecto.Changeset
   alias Pleroma.Notification
   alias Pleroma.User
   alias Pleroma.Web.OAuth.Token
@@ -16,15 +15,14 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
   action_fallback(:errors)
 
   def confirm_email(conn, %{"user_id" => uid, "token" => token}) do
-    with %User{} = user <- User.get_cached_by_id(uid),
          true <- user.local,
          true <- user.info.confirmation_pending,
          true <- user.info.confirmation_token == token,
-         info_change <- User.Info.confirmation_changeset(user.info, need_confirmation: false),
-         changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_change),
-         {:ok, _} <- User.update_and_set_cache(changeset) do
       conn
       |> redirect(to: "/")
+    with %User{info: info} = user <- User.get_cached_by_id(uid),
+         {:ok, _} <-
+           User.update_info(user, &User.Info.confirmation_changeset(&1, need_confirmation: false)) do
     end
   end
 
diff --git a/test/tasks/database_test.exs b/test/tasks/database_test.exs
index a9925c361..b63dcac00 100644
--- a/test/tasks/database_test.exs
+++ b/test/tasks/database_test.exs
@@ -77,12 +77,10 @@ test "following and followers count are updated" do
       assert length(following) == 2
       assert info.follower_count == 0
 
-      info_cng = Ecto.Changeset.change(info, %{follower_count: 3})
-
       {:ok, user} =
         user
         |> Ecto.Changeset.change(%{following: following ++ following})
-        |> Ecto.Changeset.put_embed(:info, info_cng)
+        |> User.change_info(&Ecto.Changeset.change(&1, %{follower_count: 3}))
         |> Repo.update()
 
       assert length(user.following) == 4
diff --git a/test/user_test.exs b/test/user_test.exs
index 21ea1d28e..126bd69e8 100644
--- a/test/user_test.exs
+++ b/test/user_test.exs
@@ -1707,4 +1707,22 @@ test "sets password_reset_pending to true", %{user: user} do
       assert password_reset_pending
     end
   end
+
+  test "change_info/2" do
+    user = insert(:user)
+    assert user.info.hide_follows == false
+
+    changeset = User.change_info(user, &User.Info.profile_update(&1, %{hide_follows: true}))
+    assert changeset.changes.info.changes.hide_follows == true
+  end
+
+  test "update_info/2" do
+    user = insert(:user)
+    assert user.info.hide_follows == false
+
+    assert {:ok, _} = User.update_info(user, &User.Info.profile_update(&1, %{hide_follows: true}))
+
+    assert %{info: %{hide_follows: true}} = Repo.get(User, user.id)
+    assert {:ok, %{info: %{hide_follows: true}}} = Cachex.get(:user_cache, "ap_id:#{user.ap_id}")
+  end
 end
diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs
index 73a3bf135..e32468f85 100644
--- a/test/web/mastodon_api/mastodon_api_controller_test.exs
+++ b/test/web/mastodon_api/mastodon_api_controller_test.exs
@@ -2613,14 +2613,11 @@ test "get instance stats", %{conn: conn} do
     {:ok, _} = CommonAPI.post(user, %{"status" => "cofe"})
 
     # Stats should count users with missing or nil `info.deactivated` value
-    user = User.get_cached_by_id(user.id)
-    info_change = Changeset.change(user.info, %{deactivated: nil})
 
     {:ok, _user} =
-      user
-      |> Changeset.change()
-      |> Changeset.put_embed(:info, info_change)
-      |> User.update_and_set_cache()
+      user.id
+      |> User.get_cached_by_id()
+      |> User.update_info(&Changeset.change(&1, %{deactivated: nil}))
 
     Pleroma.Stats.force_update()
 
@@ -3953,13 +3950,9 @@ test "it returns 400 when user is not local", %{conn: conn, user: user} do
 
   describe "POST /api/v1/pleroma/accounts/confirmation_resend" do
     setup do
-      user = insert(:user)
-      info_change = User.Info.confirmation_changeset(user.info, need_confirmation: true)
-
       {:ok, user} =
-        user
-        |> Changeset.change()
-        |> Changeset.put_embed(:info, info_change)
+        insert(:user)
+        |> User.change_info(&User.Info.confirmation_changeset(&1, need_confirmation: true))
         |> Repo.update()
 
       assert user.info.confirmation_pending
diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs
index 8b88fd784..0cf755806 100644
--- a/test/web/oauth/oauth_controller_test.exs
+++ b/test/web/oauth/oauth_controller_test.exs
@@ -7,6 +7,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
   import Pleroma.Factory
 
   alias Pleroma.Repo
+  alias Pleroma.User
   alias Pleroma.Web.OAuth.Authorization
   alias Pleroma.Web.OAuth.OAuthController
   alias Pleroma.Web.OAuth.Token
@@ -775,15 +776,11 @@ test "rejects token exchange with invalid client credentials" do
 
     test "rejects token exchange for valid credentials belonging to unconfirmed user and confirmation is required" do
       Pleroma.Config.put([:instance, :account_activation_required], true)
-
       password = "testpassword"
-      user = insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt(password))
-      info_change = Pleroma.User.Info.confirmation_changeset(user.info, need_confirmation: true)
 
       {:ok, user} =
-        user
-        |> Ecto.Changeset.change()
-        |> Ecto.Changeset.put_embed(:info, info_change)
+        insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt(password))
+        |> User.change_info(&User.Info.confirmation_changeset(&1, need_confirmation: true))
         |> Repo.update()
 
       refute Pleroma.User.auth_active?(user)
diff --git a/test/web/ostatus/ostatus_controller_test.exs b/test/web/ostatus/ostatus_controller_test.exs
index ec96f0012..2b40fb47e 100644
--- a/test/web/ostatus/ostatus_controller_test.exs
+++ b/test/web/ostatus/ostatus_controller_test.exs
@@ -50,20 +50,16 @@ test "decodes a salmon with a changed magic key", %{conn: conn} do
                assert response(conn, 200)
              end) =~ "[error]"
 
-      # Set a wrong magic-key for a user so it has to refetch
-      salmon_user = User.get_cached_by_ap_id("http://gs.example.org:4040/index.php/user/1")
-
       # Wrong key
-      info_cng =
-        User.Info.remote_user_creation(salmon_user.info, %{
-          magic_key:
-            "RSA.pu0s-halox4tu7wmES1FVSx6u-4wc0YrUFXcqWXZG4-27UmbCOpMQftRCldNRfyA-qLbz-eqiwrong1EwUvjsD4cYbAHNGHwTvDOyx5AKthQUP44ykPv7kjKGh3DWKySJvcs9tlUG87hlo7AvnMo9pwRS_Zz2CacQ-MKaXyDepk=.AQAB"
-        })
+      info = %{
+        magic_key:
+          "RSA.pu0s-halox4tu7wmES1FVSx6u-4wc0YrUFXcqWXZG4-27UmbCOpMQftRCldNRfyA-qLbz-eqiwrong1EwUvjsD4cYbAHNGHwTvDOyx5AKthQUP44ykPv7kjKGh3DWKySJvcs9tlUG87hlo7AvnMo9pwRS_Zz2CacQ-MKaXyDepk=.AQAB"
+      }
 
-      salmon_user
-      |> Ecto.Changeset.change()
-      |> Ecto.Changeset.put_embed(:info, info_cng)
-      |> User.update_and_set_cache()
+      # Set a wrong magic-key for a user so it has to refetch
+      "http://gs.example.org:4040/index.php/user/1"
+      |> User.get_cached_by_ap_id()
+      |> User.update_info(&User.Info.remote_user_creation(&1, info))
 
       assert capture_log(fn ->
                conn =

From 1bea67cb5e70ae28209a193c33b9da2d3c41cfb7 Mon Sep 17 00:00:00 2001
From: Egor Kislitsyn <egor@kislitsyn.com>
Date: Tue, 24 Sep 2019 14:14:34 +0700
Subject: [PATCH 3/4] Cleanup Pleroma.User

---
 lib/pleroma/user.ex                           | 231 ++++++++----------
 lib/pleroma/web/common_api/common_api.ex      |  25 +-
 .../web/twitter_api/twitter_api_controller.ex |   7 +-
 3 files changed, 108 insertions(+), 155 deletions(-)

diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index 422bc6fa6..640ef05c4 100644
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -106,9 +106,7 @@ def profile_url(%User{info: %{source_data: %{"url" => url}}}), do: url
   def profile_url(%User{ap_id: ap_id}), do: ap_id
   def profile_url(_), do: nil
 
-  def ap_id(%User{nickname: nickname}) do
-    "#{Web.base_url()}/users/#{nickname}"
-  end
+  def ap_id(%User{nickname: nickname}), do: "#{Web.base_url()}/users/#{nickname}"
 
   def ap_followers(%User{follower_address: fa}) when is_binary(fa), do: fa
   def ap_followers(%User{} = user), do: "#{ap_id(user)}/followers"
@@ -119,12 +117,9 @@ def ap_following(%User{} = user), do: "#{ap_id(user)}/following"
 
   def user_info(%User{} = user, args \\ %{}) do
     following_count =
-      if args[:following_count],
-        do: args[:following_count],
-        else: user.info.following_count || following_count(user)
+      Map.get(args, :following_count, user.info.following_count || following_count(user))
 
-    follower_count =
-      if args[:follower_count], do: args[:follower_count], else: user.info.follower_count
+    follower_count = Map.get(args, :follower_count, user.info.follower_count)
 
     %{
       note_count: user.info.note_count,
@@ -137,12 +132,11 @@ def user_info(%User{} = user, args \\ %{}) do
   end
 
   def follow_state(%User{} = user, %User{} = target) do
-    follow_activity = Utils.fetch_latest_follow(user, target)
-
-    if follow_activity,
-      do: follow_activity.data["state"],
+    case Utils.fetch_latest_follow(user, target) do
+      %{data: %{"state" => state}} -> state
       # Ideally this would be nil, but then Cachex does not commit the value
-      else: false
+      _ -> false
+    end
   end
 
   def get_cached_follow_state(user, target) do
@@ -152,11 +146,7 @@ def get_cached_follow_state(user, target) do
 
   @spec set_follow_state_cache(String.t(), String.t(), String.t()) :: {:ok | :error, boolean()}
   def set_follow_state_cache(user_ap_id, target_ap_id, state) do
-    Cachex.put(
-      :user_cache,
-      "follow_state:#{user_ap_id}|#{target_ap_id}",
-      state
-    )
+    Cachex.put(:user_cache, "follow_state:#{user_ap_id}|#{target_ap_id}", state)
   end
 
   def set_info_cache(user, args) do
@@ -197,32 +187,25 @@ def remote_user_creation(params) do
       |> truncate_if_exists(:name, name_limit)
       |> truncate_if_exists(:bio, bio_limit)
 
-    changes =
-      %User{}
+    changeset =
+      %User{local: false}
       |> cast(params, [:bio, :name, :ap_id, :nickname, :avatar])
       |> validate_required([:name, :ap_id])
       |> unique_constraint(:nickname)
       |> validate_format(:nickname, @email_regex)
       |> validate_length(:bio, max: bio_limit)
       |> validate_length(:name, max: name_limit)
-      |> put_change(:local, false)
       |> change_info(&User.Info.remote_user_creation(&1, params[:info]))
 
-    if changes.valid? do
-      case info_cng.changes[:source_data] do
-        %{"followers" => followers, "following" => following} ->
-          changes
-          |> put_change(:follower_address, followers)
-          |> put_change(:following_address, following)
+    case params[:info][:source_data] do
+      %{"followers" => followers, "following" => following} ->
+        changeset
+        |> put_change(:follower_address, followers)
+        |> put_change(:following_address, following)
 
-        _ ->
-          followers = User.ap_followers(%User{nickname: changes.changes[:nickname]})
-
-          changes
-          |> put_change(:follower_address, followers)
-      end
-    else
-      changes
+      _ ->
+        followers = ap_followers(%User{nickname: get_field(changeset, :nickname)})
+        put_change(changeset, :follower_address, followers)
     end
   end
 
@@ -308,43 +291,39 @@ def register_changeset(struct, params \\ %{}, opts \\ []) do
         opts[:need_confirmation]
       end
 
-    info_change =
-      User.Info.confirmation_changeset(%User.Info{}, need_confirmation: need_confirmation?)
+    struct
+    |> cast(params, [:bio, :email, :name, :nickname, :password, :password_confirmation])
+    |> validate_required([:name, :nickname, :password, :password_confirmation])
+    |> validate_confirmation(:password)
+    |> unique_constraint(:email)
+    |> unique_constraint(:nickname)
+    |> validate_exclusion(:nickname, Pleroma.Config.get([User, :restricted_nicknames]))
+    |> validate_format(:nickname, local_nickname_regex())
+    |> validate_format(:email, @email_regex)
+    |> validate_length(:bio, max: bio_limit)
+    |> validate_length(:name, min: 1, max: name_limit)
+    |> change_info(&User.Info.confirmation_changeset(&1, need_confirmation: need_confirmation?))
+    |> maybe_validate_required_email(opts[:external])
+    |> put_password_hash
+    |> put_ap_id()
+    |> unique_constraint(:ap_id)
+    |> put_following_and_follower_address()
+  end
 
-    changeset =
-      struct
-      |> cast(params, [:bio, :email, :name, :nickname, :password, :password_confirmation])
-      |> validate_required([:name, :nickname, :password, :password_confirmation])
-      |> validate_confirmation(:password)
-      |> unique_constraint(:email)
-      |> unique_constraint(:nickname)
-      |> validate_exclusion(:nickname, Pleroma.Config.get([User, :restricted_nicknames]))
-      |> validate_format(:nickname, local_nickname_regex())
-      |> validate_format(:email, @email_regex)
-      |> validate_length(:bio, max: bio_limit)
-      |> validate_length(:name, min: 1, max: name_limit)
-      |> put_change(:info, info_change)
+  def maybe_validate_required_email(changeset, true), do: changeset
+  def maybe_validate_required_email(changeset, _), do: validate_required(changeset, [:email])
 
-    changeset =
-      if opts[:external] do
-        changeset
-      else
-        validate_required(changeset, [:email])
-      end
+  defp put_ap_id(changeset) do
+    ap_id = ap_id(%User{nickname: get_field(changeset, :nickname)})
+    put_change(changeset, :ap_id, ap_id)
+  end
 
-    if changeset.valid? do
-      ap_id = User.ap_id(%User{nickname: changeset.changes[:nickname]})
-      followers = User.ap_followers(%User{nickname: changeset.changes[:nickname]})
+  defp put_following_and_follower_address(changeset) do
+    followers = ap_followers(%User{nickname: get_field(changeset, :nickname)})
 
-      changeset
-      |> put_password_hash
-      |> put_change(:ap_id, ap_id)
-      |> unique_constraint(:ap_id)
-      |> put_change(:following, [followers])
-      |> put_change(:follower_address, followers)
-    else
-      changeset
-    end
+    changeset
+    |> put_change(:following, [followers])
+    |> put_change(:follower_address, followers)
   end
 
   defp autofollow_users(user) do
@@ -359,9 +338,8 @@ defp autofollow_users(user) do
 
   @doc "Inserts provided changeset, performs post-registration actions (confirmation email sending etc.)"
   def register(%Ecto.Changeset{} = changeset) do
-    with {:ok, user} <- Repo.insert(changeset),
-         {:ok, user} <- post_register_action(user) do
-      {:ok, user}
+    with {:ok, user} <- Repo.insert(changeset) do
+      post_register_action(user)
     end
   end
 
@@ -407,7 +385,7 @@ def maybe_direct_follow(%User{} = follower, %User{local: true} = followed) do
   end
 
   def maybe_direct_follow(%User{} = follower, %User{} = followed) do
-    if not User.ap_enabled?(followed) do
+    if not ap_enabled?(followed) do
       follow(follower, followed)
     else
       {:ok, follower}
@@ -440,9 +418,7 @@ def follow_all(follower, followeds) do
 
     {1, [follower]} = Repo.update_all(q, [])
 
-    Enum.each(followeds, fn followed ->
-      update_follower_count(followed)
-    end)
+    Enum.each(followeds, &update_follower_count/1)
 
     set_cache(follower)
   end
@@ -552,8 +528,6 @@ def set_cache(%User{} = user) do
   def update_and_set_cache(changeset) do
     with {:ok, user} <- Repo.update(changeset, stale_error_field: :id) do
       set_cache(user)
-    else
-      e -> e
     end
   end
 
@@ -590,9 +564,7 @@ def get_cached_by_nickname(nickname) do
     key = "nickname:#{nickname}"
 
     Cachex.fetch!(:user_cache, key, fn ->
-      user_result = get_or_fetch_by_nickname(nickname)
-
-      case user_result do
+      case get_or_fetch_by_nickname(nickname) do
         {:ok, user} -> {:commit, user}
         {:error, _error} -> {:ignore, nil}
       end
@@ -632,13 +604,11 @@ def get_by_nickname_or_email(nickname_or_email) do
 
   def get_cached_user_info(user) do
     key = "user_info:#{user.id}"
-    Cachex.fetch!(:user_cache, key, fn _ -> user_info(user) end)
+    Cachex.fetch!(:user_cache, key, fn -> user_info(user) end)
   end
 
   def fetch_by_nickname(nickname) do
-    ap_try = ActivityPub.make_user_from_nickname(nickname)
-
-    case ap_try do
+    case ActivityPub.make_user_from_nickname(nickname) do
       {:ok, user} -> {:ok, user}
       _ -> OStatus.make_user(nickname)
     end
@@ -673,7 +643,8 @@ def get_followers_query(%User{} = user, nil) do
   end
 
   def get_followers_query(user, page) do
-    from(u in get_followers_query(user, nil))
+    user
+    |> get_followers_query(nil)
     |> User.Query.paginate(page, 20)
   end
 
@@ -689,18 +660,17 @@ def get_followers(user, page \\ nil) do
 
   @spec get_external_followers(User.t(), pos_integer()) :: {:ok, list(User.t())}
   def get_external_followers(user, page \\ nil) do
-    q =
-      user
-      |> get_followers_query(page)
-      |> User.Query.build(%{external: true})
-
-    {:ok, Repo.all(q)}
+    user
+    |> get_followers_query(page)
+    |> User.Query.build(%{external: true})
+    |> Repo.all()
   end
 
   def get_followers_ids(user, page \\ nil) do
-    q = get_followers_query(user, page)
-
-    Repo.all(from(u in q, select: u.id))
+    user
+    |> get_followers_query(page)
+    |> select([u], u.id)
+    |> Repo.all()
   end
 
   @spec get_friends_query(User.t(), pos_integer() | nil) :: Ecto.Query.t()
@@ -709,7 +679,8 @@ def get_friends_query(%User{} = user, nil) do
   end
 
   def get_friends_query(user, page) do
-    from(u in get_friends_query(user, nil))
+    user
+    |> get_friends_query(nil)
     |> User.Query.paginate(page, 20)
   end
 
@@ -723,9 +694,10 @@ def get_friends(user, page \\ nil) do
   end
 
   def get_friends_ids(user, page \\ nil) do
-    q = get_friends_query(user, page)
-
-    Repo.all(from(u in q, select: u.id))
+    user
+    |> get_friends_query(page)
+    |> select([u], u.id)
+    |> Repo.all()
   end
 
   @spec get_follow_requests(User.t()) :: {:ok, [User.t()]}
@@ -889,12 +861,10 @@ def unmute(muter, %{ap_id: ap_id}) do
   end
 
   def subscribe(subscriber, %{ap_id: ap_id}) do
-    deny_follow_blocked = Pleroma.Config.get([:user, :deny_follow_blocked])
-
     with %User{} = subscribed <- get_cached_by_ap_id(ap_id) do
-      blocked = blocks?(subscribed, subscriber) and deny_follow_blocked
+      deny_follow_blocked = Pleroma.Config.get([:user, :deny_follow_blocked])
 
-      if blocked do
+      if blocks?(subscribed, subscriber) and deny_follow_blocked do
         {:error, "Could not subscribe: #{subscribed.nickname} is blocking you"}
       else
         update_info(subscribed, &User.Info.add_to_subscribers(&1, subscriber.ap_id))
@@ -933,9 +903,7 @@ def block(blocker, %User{ap_id: ap_id} = blocked) do
         blocker
       end
 
-    if following?(blocked, blocker) do
-      unfollow(blocked, blocker)
-    end
+    if following?(blocked, blocker), do: unfollow(blocked, blocker)
 
     {:ok, blocker} = update_follower_count(blocker)
 
@@ -1168,17 +1136,19 @@ defp delete_activity(%{data: %{"type" => "Create"}} = activity) do
   end
 
   defp delete_activity(%{data: %{"type" => "Like"}} = activity) do
-    user = get_cached_by_ap_id(activity.actor)
     object = Object.normalize(activity)
 
-    ActivityPub.unlike(user, object)
+    activity.actor
+    |> get_cached_by_ap_id()
+    |> ActivityPub.unlike(object)
   end
 
   defp delete_activity(%{data: %{"type" => "Announce"}} = activity) do
-    user = get_cached_by_ap_id(activity.actor)
     object = Object.normalize(activity)
 
-    ActivityPub.unannounce(user, object)
+    activity.actor
+    |> get_cached_by_ap_id()
+    |> ActivityPub.unannounce(object)
   end
 
   defp delete_activity(_activity), do: "Doing nothing"
@@ -1190,9 +1160,7 @@ def html_filter_policy(%User{info: %{no_rich_text: true}}) do
   def html_filter_policy(_), do: Pleroma.Config.get([:markup, :scrub_policy])
 
   def fetch_by_ap_id(ap_id) do
-    ap_try = ActivityPub.make_user_from_ap_id(ap_id)
-
-    case ap_try do
+    case ActivityPub.make_user_from_ap_id(ap_id) do
       {:ok, user} ->
         {:ok, user}
 
@@ -1207,7 +1175,7 @@ def fetch_by_ap_id(ap_id) do
   def get_or_fetch_by_ap_id(ap_id) do
     user = get_cached_by_ap_id(ap_id)
 
-    if !is_nil(user) and !User.needs_update?(user) do
+    if !is_nil(user) and !needs_update?(user) do
       {:ok, user}
     else
       # Whether to fetch initial posts for the user (if it's a new user & the fetching is enabled)
@@ -1227,19 +1195,20 @@ def get_or_fetch_by_ap_id(ap_id) do
 
   @doc "Creates an internal service actor by URI if missing.  Optionally takes nickname for addressing."
   def get_or_create_service_actor_by_ap_id(uri, nickname \\ nil) do
-    if user = get_cached_by_ap_id(uri) do
+    with %User{} = user <- get_cached_by_ap_id(uri) do
       user
     else
-      changes =
-        %User{info: %User.Info{}}
-        |> cast(%{}, [:ap_id, :nickname, :local])
-        |> put_change(:ap_id, uri)
-        |> put_change(:nickname, nickname)
-        |> put_change(:local, true)
-        |> put_change(:follower_address, uri <> "/followers")
+      _ ->
+        {:ok, user} =
+          %User{info: %User.Info{}}
+          |> cast(%{}, [:ap_id, :nickname, :local])
+          |> put_change(:ap_id, uri)
+          |> put_change(:nickname, nickname)
+          |> put_change(:local, true)
+          |> put_change(:follower_address, uri <> "/followers")
+          |> Repo.insert()
 
-      {:ok, user} = Repo.insert(changes)
-      user
+        user
     end
   end
 
@@ -1296,23 +1265,21 @@ def get_or_fetch(nickname), do: get_or_fetch_by_nickname(nickname)
   # this is because we have synchronous follow APIs and need to simulate them
   # with an async handshake
   def wait_and_refresh(_, %User{local: true} = a, %User{local: true} = b) do
-    with %User{} = a <- User.get_cached_by_id(a.id),
-         %User{} = b <- User.get_cached_by_id(b.id) do
+    with %User{} = a <- get_cached_by_id(a.id),
+         %User{} = b <- get_cached_by_id(b.id) do
       {:ok, a, b}
     else
-      _e ->
-        :error
+      nil -> :error
     end
   end
 
   def wait_and_refresh(timeout, %User{} = a, %User{} = b) do
     with :ok <- :timer.sleep(timeout),
-         %User{} = a <- User.get_cached_by_id(a.id),
-         %User{} = b <- User.get_cached_by_id(b.id) do
+         %User{} = a <- get_cached_by_id(a.id),
+         %User{} = b <- get_cached_by_id(b.id) do
       {:ok, a, b}
     else
-      _e ->
-        :error
+      nil -> :error
     end
   end
 
@@ -1374,7 +1341,7 @@ defp update_tags(%User{} = user, new_tags) do
   defp normalize_tags(tags) do
     [tags]
     |> List.flatten()
-    |> Enum.map(&String.downcase(&1))
+    |> Enum.map(&String.downcase/1)
   end
 
   defp local_nickname_regex do
diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex
index 2f12ad43a..40eebe2aa 100644
--- a/lib/pleroma/web/common_api/common_api.ex
+++ b/lib/pleroma/web/common_api/common_api.ex
@@ -309,8 +309,7 @@ def update(user) do
       with {:ok, user} <- User.update_info(user, &User.Info.set_source_data(&1, source_data)) do
         user
       else
-        _e ->
-          user
+        _e -> user
       end
 
     ActivityPub.update(%{
@@ -338,11 +337,8 @@ def pin(id_or_ap_id, %{ap_id: user_ap_id} = user) do
          {:ok, _user} <- User.update_info(user, &User.Info.add_pinnned_activity(&1, activity)) do
       {:ok, activity}
     else
-      %{errors: [pinned_activities: {err, _}]} ->
-        {:error, err}
-
-      _ ->
-        {:error, dgettext("errors", "Could not pin")}
+      {:error, %{changes: %{info: %{errors: [pinned_activities: {err, _}]}}}} -> {:error, err}
+      _ -> {:error, dgettext("errors", "Could not pin")}
     end
   end
 
@@ -351,11 +347,8 @@ def unpin(id_or_ap_id, user) do
          {:ok, _user} <- User.update_info(user, &User.Info.remove_pinnned_activity(&1, activity)) do
       {:ok, activity}
     else
-      %{errors: [pinned_activities: {err, _}]} ->
-        {:error, err}
-
-      _ ->
-        {:error, dgettext("errors", "Could not unpin")}
+      %{errors: [pinned_activities: {err, _}]} -> {:error, err}
+      _ -> {:error, dgettext("errors", "Could not unpin")}
     end
   end
 
@@ -450,17 +443,13 @@ defp set_visibility(activity, %{"visibility" => visibility}) do
 
   defp set_visibility(activity, _), do: {:ok, activity}
 
-  def hide_reblogs(user, muted) do
-    ap_id = muted.ap_id
-
+  def hide_reblogs(user, %{ap_id: ap_id} = _muted) do
     if ap_id not in user.info.muted_reblogs do
       User.update_info(user, &User.Info.add_reblog_mute(&1, ap_id))
     end
   end
 
-  def show_reblogs(user, muted) do
-    ap_id = muted.ap_id
-
+  def show_reblogs(user, %{ap_id: ap_id} = _muted) do
     if ap_id in user.info.muted_reblogs do
       User.update_info(user, &User.Info.remove_reblog_mute(&1, ap_id))
     end
diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex
index 27f3664e0..aa06e2630 100644
--- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex
+++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex
@@ -15,14 +15,11 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
   action_fallback(:errors)
 
   def confirm_email(conn, %{"user_id" => uid, "token" => token}) do
-         true <- user.local,
-         true <- user.info.confirmation_pending,
-         true <- user.info.confirmation_token == token,
-      conn
-      |> redirect(to: "/")
     with %User{info: info} = user <- User.get_cached_by_id(uid),
+         true <- user.local and info.confirmation_pending and info.confirmation_token == token,
          {:ok, _} <-
            User.update_info(user, &User.Info.confirmation_changeset(&1, need_confirmation: false)) do
+      redirect(conn, to: "/")
     end
   end
 

From 035f22f7849815c5f77a734c56f409c0f08ac853 Mon Sep 17 00:00:00 2001
From: Egor Kislitsyn <egor@kislitsyn.com>
Date: Tue, 24 Sep 2019 14:49:02 +0700
Subject: [PATCH 4/4] Fix Credo warnings

---
 .../web/mastodon_api/controllers/mastodon_api_controller.ex  | 2 +-
 lib/pleroma/web/twitter_api/twitter_api_controller.ex        | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
index 8a5287079..873ef20bc 100644
--- a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
+++ b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
@@ -1348,7 +1348,7 @@ def index(%{assigns: %{user: user}} = conn, _params) do
   end
 
   def put_settings(%{assigns: %{user: user}} = conn, %{"data" => settings} = _params) do
-    with {:ok, _user} <- User.update_info(user, &User.Info.mastodon_settings_update(&1, settings)) do
+    with {:ok, _} <- User.update_info(user, &User.Info.mastodon_settings_update(&1, settings)) do
       json(conn, %{})
     else
       e ->
diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex
index aa06e2630..5024ac70d 100644
--- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex
+++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex
@@ -15,10 +15,11 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
   action_fallback(:errors)
 
   def confirm_email(conn, %{"user_id" => uid, "token" => token}) do
+    new_info = [need_confirmation: false]
+
     with %User{info: info} = user <- User.get_cached_by_id(uid),
          true <- user.local and info.confirmation_pending and info.confirmation_token == token,
-         {:ok, _} <-
-           User.update_info(user, &User.Info.confirmation_changeset(&1, need_confirmation: false)) do
+         {:ok, _} <- User.update_info(user, &User.Info.confirmation_changeset(&1, new_info)) do
       redirect(conn, to: "/")
     end
   end