From e8fa477793e1395664f79d572800f11994cdd38d Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 13 Jul 2019 19:17:57 +0300 Subject: 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 --- 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 +++++++++++++++++++++++++- lib/pleroma/web/activity_pub/transmogrifier.ex | 27 ------------- 5 files changed, 71 insertions(+), 32 deletions(-) (limited to 'lib') 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 @@ defmodule Pleroma.Object.Fetcher 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 @@ defmodule Pleroma.Object.Fetcher 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 @@ defmodule Pleroma.User do 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 @@ defmodule Pleroma.User.Info do :uri, :hub, :topic, - :salmon + :salmon, + :hide_followers, + :hide_follows, + :follower_count, + :following_count ]) end @@ -206,7 +211,11 @@ defmodule Pleroma.User.Info 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 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub 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 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub 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 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier 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 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier 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 -- cgit v1.2.3 From e5b850a99115859ceb028c3891f59d5e6ffd5d56 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 13 Jul 2019 23:56:10 +0300 Subject: Refactor fetching follow information to a separate function --- lib/pleroma/user/info.ex | 1 + lib/pleroma/web/activity_pub/activity_pub.ex | 51 ++++++++++++++++++---------- 2 files changed, 34 insertions(+), 18 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index 2d8395b73..67e8801ea 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) + # Should be filled in only for remote users field(:following_count, :integer, default: nil) field(:locked, :boolean, default: false) field(:confirmation_pending, :boolean, default: false) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 0a22fe223..eadd335ca 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1013,17 +1013,15 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub 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), + def fetch_follow_information_for_user(user) do + with {:ok, following_data} <- + Fetcher.fetch_and_contain_remote_object_from_id(user.following_address), + following_count when is_integer(following_count) <- following_data["totalItems"], + {:ok, 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 + Fetcher.fetch_and_contain_remote_object_from_id(user.follower_address), + followers_count when is_integer(followers_count) <- followers_data["totalItems"], + {:ok, hide_followers} <- collection_private(followers_data) do info = %{ "hide_follows" => hide_follows, "follower_count" => followers_count, @@ -1031,8 +1029,22 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do "hide_followers" => hide_followers } - info = Map.merge(data.info, info) - Map.put(data, :info, info) + info = Map.merge(user.info, info) + {:ok, Map.put(user, :info, info)} + else + {:error, _} = e -> + e + + e -> + {:error, e} + end + end + + defp maybe_update_follow_information(data) do + with {:enabled, true} <- + {:enabled, Pleroma.Config.get([:instance, :external_user_synchronization])}, + {:ok, data} <- fetch_follow_information_for_user(data) do + data else {:enabled, false} -> data @@ -1046,19 +1058,22 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do end end - defp collection_private?(data) do + defp collection_private(data) do if is_map(data["first"]) and data["first"]["type"] in ["CollectionPage", "OrderedCollectionPage"] do - false + {:ok, false} else with {:ok, _data} <- Fetcher.fetch_and_contain_remote_object_from_id(data["first"]) do - false + {:ok, false} else {:error, {:ok, %{status: code}}} when code in [401, 403] -> - true + {:ok, true} + + {:error, _} = e -> + e - _e -> - false + e -> + {:error, e} end end end -- cgit v1.2.3 From d06d1b751d44802c5c3701f916ae2ce7d3c3be56 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sun, 14 Jul 2019 00:21:35 +0300 Subject: Use atoms when updating user info --- lib/pleroma/web/activity_pub/activity_pub.ex | 16 ++++++++-------- lib/pleroma/web/activity_pub/transmogrifier.ex | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index eadd335ca..df4155d21 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -986,10 +986,10 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do user_data = %{ ap_id: data["id"], info: %{ - "ap_enabled" => true, - "source_data" => data, - "banner" => banner, - "locked" => locked + ap_enabled: true, + source_data: data, + banner: banner, + locked: locked }, avatar: avatar, name: data["name"], @@ -1023,10 +1023,10 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do followers_count when is_integer(followers_count) <- followers_data["totalItems"], {:ok, hide_followers} <- collection_private(followers_data) do info = %{ - "hide_follows" => hide_follows, - "follower_count" => followers_count, - "following_count" => following_count, - "hide_followers" => hide_followers + hide_follows: hide_follows, + follower_count: followers_count, + following_count: following_count, + hide_followers: hide_followers } info = Map.merge(user.info, info) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index e34fe6611..10b362908 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -609,13 +609,13 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do with %User{ap_id: ^actor_id} = actor <- User.get_cached_by_ap_id(object["id"]) do {:ok, new_user_data} = ActivityPub.user_data_from_user_object(object) - banner = new_user_data[:info]["banner"] - locked = new_user_data[:info]["locked"] || false + banner = new_user_data[:info][:banner] + locked = new_user_data[:info][:locked] || false update_data = new_user_data |> Map.take([:name, :bio, :avatar]) - |> Map.put(:info, %{"banner" => banner, "locked" => locked}) + |> Map.put(:info, %{banner: banner, locked: locked}) actor |> User.upgrade_changeset(update_data) -- cgit v1.2.3 From 183da33e005c8a8e8472350a3b6b36ff6f82d67d Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sun, 14 Jul 2019 00:56:02 +0300 Subject: Add tests for fetch_follow_information_for_user and check object type when fetching the page --- lib/pleroma/web/activity_pub/activity_pub.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index df4155d21..c821ba45f 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1063,7 +1063,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do data["first"]["type"] in ["CollectionPage", "OrderedCollectionPage"] do {:ok, false} else - with {:ok, _data} <- Fetcher.fetch_and_contain_remote_object_from_id(data["first"]) do + with {:ok, %{"type" => type}} when type in ["CollectionPage", "OrderedCollectionPage"] <- + Fetcher.fetch_and_contain_remote_object_from_id(data["first"]) do {:ok, false} else {:error, {:ok, %{status: code}}} when code in [401, 403] -> -- cgit v1.2.3 From 0c2dcb4c69ed340d02a4b20a4f341f1d9aaaba38 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sun, 14 Jul 2019 01:58:39 +0300 Subject: Add follow information refetching after following/unfollowing --- lib/pleroma/user.ex | 89 +++++++++++++++++++++------- lib/pleroma/user/info.ex | 10 ++++ lib/pleroma/web/activity_pub/activity_pub.ex | 21 ++++--- 3 files changed, 88 insertions(+), 32 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index c252e8bff..2e9b01205 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -406,6 +406,8 @@ defmodule Pleroma.User do {1, [follower]} = Repo.update_all(q, []) + follower = maybe_update_following_count(follower) + {:ok, _} = update_follower_count(followed) set_cache(follower) @@ -425,6 +427,8 @@ defmodule Pleroma.User do {1, [follower]} = Repo.update_all(q, []) + follower = maybe_update_following_count(follower) + {:ok, followed} = update_follower_count(followed) set_cache(follower) @@ -698,32 +702,75 @@ defmodule Pleroma.User do |> update_and_set_cache() end + def maybe_fetch_follow_information(user) do + with {:ok, user} <- fetch_follow_information(user) do + user + else + e -> + Logger.error( + "Follower/Following counter update for #{user.ap_id} failed.\n" <> inspect(e) + ) + + user + end + end + + 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} + end + end + def update_follower_count(%User{} = user) do - follower_count_query = - User.Query.build(%{followers: user, deactivated: false}) - |> select([u], %{count: count(u.id)}) + unless user.local == false and Pleroma.Config.get([:instance, :external_user_synchronization]) do + follower_count_query = + User.Query.build(%{followers: user, deactivated: false}) + |> select([u], %{count: count(u.id)}) + + User + |> where(id: ^user.id) + |> join(:inner, [u], s in subquery(follower_count_query)) + |> update([u, s], + set: [ + info: + fragment( + "jsonb_set(?, '{follower_count}', ?::varchar::jsonb, true)", + u.info, + s.count + ) + ] + ) + |> select([u], u) + |> Repo.update_all([]) + |> case do + {1, [user]} -> set_cache(user) + _ -> {:error, user} + end + else + {:ok, maybe_fetch_follow_information(user)} + end + end - User - |> where(id: ^user.id) - |> join(:inner, [u], s in subquery(follower_count_query)) - |> update([u, s], - set: [ - info: - fragment( - "jsonb_set(?, '{follower_count}', ?::varchar::jsonb, true)", - u.info, - s.count - ) - ] - ) - |> select([u], u) - |> Repo.update_all([]) - |> case do - {1, [user]} -> set_cache(user) - _ -> {:error, user} + def maybe_update_following_count(%User{local: false} = user) do + if Pleroma.Config.get([:instance, :external_user_synchronization]) do + {:ok, maybe_fetch_follow_information(user)} + else + user end end + def maybe_update_following_count(user), do: user + def remove_duplicated_following(%User{following: following} = user) do uniq_following = Enum.uniq(following) diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index 67e8801ea..4cc3f2f2c 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -330,4 +330,14 @@ defmodule Pleroma.User.Info do cast(info, params, [:muted_reblogs]) end + + def follow_information_update(info, params) do + info + |> cast(params, [ + :hide_followers, + :hide_follows, + :follower_count, + :following_count + ]) + end end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index c821ba45f..2dd9dbf7f 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1022,15 +1022,13 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do Fetcher.fetch_and_contain_remote_object_from_id(user.follower_address), followers_count when is_integer(followers_count) <- followers_data["totalItems"], {:ok, 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(user.info, info) - {:ok, Map.put(user, :info, info)} + {:ok, + %{ + hide_follows: hide_follows, + follower_count: followers_count, + following_count: following_count, + hide_followers: hide_followers + }} else {:error, _} = e -> e @@ -1043,8 +1041,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do defp maybe_update_follow_information(data) do with {:enabled, true} <- {:enabled, Pleroma.Config.get([:instance, :external_user_synchronization])}, - {:ok, data} <- fetch_follow_information_for_user(data) do - data + {:ok, info} <- fetch_follow_information_for_user(data) do + info = Map.merge(data.info, info) + Map.put(data, :info, info) else {:enabled, false} -> data -- cgit v1.2.3 From d4ee76ab6355db0bed59b5126fe04d3399561798 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 20 Jul 2019 18:52:41 +0000 Subject: Apply suggestion to lib/pleroma/user.ex --- lib/pleroma/user.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 2e9b01205..956ec6240 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -708,7 +708,7 @@ defmodule Pleroma.User do else e -> Logger.error( - "Follower/Following counter update for #{user.ap_id} failed.\n" <> inspect(e) + "Follower/Following counter update for #{user.ap_id} failed.\n#{inspect(e)}" ) user -- cgit v1.2.3 From c3ecaea64dd377b586e3b2a5316e90884ec78fe6 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Sat, 20 Jul 2019 18:53:00 +0000 Subject: Apply suggestion to lib/pleroma/object/fetcher.ex --- lib/pleroma/object/fetcher.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index bc3e7e5bc..1e60d0082 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -97,7 +97,8 @@ defmodule Pleroma.Object.Fetcher do end end - def fetch_and_contain_remote_object_from_id(_id) do + def fetch_and_contain_remote_object_from_id(%{"id" => id), do: fetch_and_contain_remote_object_from_id(id) + def fetch_and_contain_remote_object_from_id(_id), do: {:error, "id must be a string"} {:error, "id must be a string"} end end -- cgit v1.2.3 From f42719506c539a4058c52d3a6e4a828948ac74ce Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 31 Jul 2019 14:20:34 +0300 Subject: Fix credo issues --- lib/pleroma/user.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index fd1c0a544..7acf1e53c 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -741,7 +741,7 @@ defmodule Pleroma.User do end def update_follower_count(%User{} = user) do - unless user.local == false and Pleroma.Config.get([:instance, :external_user_synchronization]) do + unless !user.local and Pleroma.Config.get([:instance, :external_user_synchronization]) do follower_count_query = User.Query.build(%{followers: user, deactivated: false}) |> select([u], %{count: count(u.id)}) -- cgit v1.2.3 From 301ea0dc0466371032f44f3e936d1b951ed9784c Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 31 Jul 2019 19:37:55 +0300 Subject: Add tests for counters being updated on follow --- lib/pleroma/user.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 7acf1e53c..69835f3dd 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -741,7 +741,7 @@ defmodule Pleroma.User do end def update_follower_count(%User{} = user) do - unless !user.local and Pleroma.Config.get([:instance, :external_user_synchronization]) do + if user.local or !Pleroma.Config.get([:instance, :external_user_synchronization]) do follower_count_query = User.Query.build(%{followers: user, deactivated: false}) |> select([u], %{count: count(u.id)}) -- cgit v1.2.3