From e8fa477793e1395664f79d572800f11994cdd38d Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 13 Jul 2019 19:17:57 +0300 Subject: [PATCH] Refactor Follows/Followers counter syncronization - Actually sync counters in the database instead of info cache (which got overriden after user update was finished anyway) - Add following count field to user info - Set hide_followers/hide_follows for remote users based on http status codes for the first collection page --- config/test.exs | 3 +- lib/pleroma/object/fetcher.ex | 6 ++- lib/pleroma/user.ex | 4 +- lib/pleroma/user/info.ex | 13 ++++- lib/pleroma/web/activity_pub/activity_pub.ex | 53 ++++++++++++++++++- .../web/activity_pub/transmogrifier.ex | 27 ---------- test/web/activity_pub/transmogrifier_test.exs | 28 ---------- 7 files changed, 73 insertions(+), 61 deletions(-) diff --git a/config/test.exs b/config/test.exs index 96ecf3592..28eea3b00 100644 --- a/config/test.exs +++ b/config/test.exs @@ -29,7 +29,8 @@ email: "admin@example.com", notify_email: "noreply@example.com", skip_thread_containment: false, - federating: false + federating: false, + external_user_synchronization: false # Configure your database config :pleroma, Pleroma.Repo, diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 101c21f96..bc3e7e5bc 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -76,7 +76,7 @@ def fetch_object_from_id!(id, options \\ []) do end end - def fetch_and_contain_remote_object_from_id(id) do + def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do Logger.info("Fetching object #{id} via AP") with true <- String.starts_with?(id, "http"), @@ -96,4 +96,8 @@ def fetch_and_contain_remote_object_from_id(id) do {:error, e} end end + + def fetch_and_contain_remote_object_from_id(_id) do + {:error, "id must be a string"} + end end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index e5a6c2529..c252e8bff 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -114,7 +114,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: following_count(user) + if args[:following_count], + do: args[:following_count], + else: user.info.following_count || following_count(user) follower_count = if args[:follower_count], do: args[:follower_count], else: user.info.follower_count diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index 08e43ff0f..2d8395b73 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -16,6 +16,7 @@ defmodule Pleroma.User.Info do field(:source_data, :map, default: %{}) field(:note_count, :integer, default: 0) field(:follower_count, :integer, default: 0) + field(:following_count, :integer, default: nil) field(:locked, :boolean, default: false) field(:confirmation_pending, :boolean, default: false) field(:confirmation_token, :string, default: nil) @@ -195,7 +196,11 @@ def remote_user_creation(info, params) do :uri, :hub, :topic, - :salmon + :salmon, + :hide_followers, + :hide_follows, + :follower_count, + :following_count ]) end @@ -206,7 +211,11 @@ def user_upgrade(info, params) do :source_data, :banner, :locked, - :magic_key + :magic_key, + :follower_count, + :following_count, + :hide_follows, + :hide_followers ]) end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index a3174a787..0a22fe223 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1013,6 +1013,56 @@ defp object_to_user_data(data) do {:ok, user_data} end + defp maybe_update_follow_information(data) do + with {:enabled, true} <- + {:enabled, Pleroma.Config.get([:instance, :external_user_synchronization])}, + {:ok, following_data} <- + Fetcher.fetch_and_contain_remote_object_from_id(data.following_address), + following_count <- following_data["totalItems"], + hide_follows <- collection_private?(following_data), + {:ok, followers_data} <- + Fetcher.fetch_and_contain_remote_object_from_id(data.follower_address), + followers_count <- followers_data["totalItems"], + hide_followers <- collection_private?(followers_data) do + info = %{ + "hide_follows" => hide_follows, + "follower_count" => followers_count, + "following_count" => following_count, + "hide_followers" => hide_followers + } + + info = Map.merge(data.info, info) + Map.put(data, :info, info) + else + {:enabled, false} -> + data + + e -> + Logger.error( + "Follower/Following counter update for #{data.ap_id} failed.\n" <> inspect(e) + ) + + data + end + end + + defp collection_private?(data) do + if is_map(data["first"]) and + data["first"]["type"] in ["CollectionPage", "OrderedCollectionPage"] do + false + else + with {:ok, _data} <- Fetcher.fetch_and_contain_remote_object_from_id(data["first"]) do + false + else + {:error, {:ok, %{status: code}}} when code in [401, 403] -> + true + + _e -> + false + end + end + end + def user_data_from_user_object(data) do with {:ok, data} <- MRF.filter(data), {:ok, data} <- object_to_user_data(data) do @@ -1024,7 +1074,8 @@ def user_data_from_user_object(data) do def fetch_and_prepare_user_from_ap_id(ap_id) do with {:ok, data} <- Fetcher.fetch_and_contain_remote_object_from_id(ap_id), - {:ok, data} <- user_data_from_user_object(data) do + {:ok, data} <- user_data_from_user_object(data), + data <- maybe_update_follow_information(data) do {:ok, data} else e -> Logger.error("Could not decode user at fetch #{ap_id}, #{inspect(e)}") diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index d14490bb5..e34fe6611 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -1087,10 +1087,6 @@ def upgrade_user_from_ap_id(ap_id) do PleromaJobQueue.enqueue(:transmogrifier, __MODULE__, [:user_upgrade, user]) end - if Pleroma.Config.get([:instance, :external_user_synchronization]) do - update_following_followers_counters(user) - end - {:ok, user} else %User{} = user -> {:ok, user} @@ -1123,27 +1119,4 @@ def maybe_fix_user_object(data) do data |> maybe_fix_user_url end - - def update_following_followers_counters(user) do - info = %{} - - following = fetch_counter(user.following_address) - info = if following, do: Map.put(info, :following_count, following), else: info - - followers = fetch_counter(user.follower_address) - info = if followers, do: Map.put(info, :follower_count, followers), else: info - - User.set_info_cache(user, info) - end - - defp fetch_counter(url) do - with {:ok, %{body: body, status: code}} when code in 200..299 <- - Pleroma.HTTP.get( - url, - [{:Accept, "application/activity+json"}] - ), - {:ok, data} <- Jason.decode(body) do - data["totalItems"] - end - end end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index b896a532b..6d05138fb 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -1359,32 +1359,4 @@ test "removes recipient's follower collection from cc", %{user: user} do refute recipient.follower_address in fixed_object["to"] end end - - test "update_following_followers_counters/1" do - user1 = - insert(:user, - local: false, - follower_address: "http://localhost:4001/users/masto_closed/followers", - following_address: "http://localhost:4001/users/masto_closed/following" - ) - - user2 = - insert(:user, - local: false, - follower_address: "http://localhost:4001/users/fuser2/followers", - following_address: "http://localhost:4001/users/fuser2/following" - ) - - Transmogrifier.update_following_followers_counters(user1) - Transmogrifier.update_following_followers_counters(user2) - - %{follower_count: followers, following_count: following} = User.get_cached_user_info(user1) - assert followers == 437 - assert following == 152 - - %{follower_count: followers, following_count: following} = User.get_cached_user_info(user2) - - assert followers == 527 - assert following == 267 - end end