commit: 5ff8f07ca906d77a6ec1d5ba912a787f855364f9
parent: 1fe092e03cab11a08e5a15d6bbaf0fd603ef4a00
Author: lain <lain@soykaf.club>
Date: Fri, 2 Aug 2019 11:23:07 +0000
Merge branch 'feature/hide-follows-remote' into 'develop'
Refactor Follows/Followers counter syncronization and set hide_followers/hide_follows for remote users
See merge request pleroma/pleroma!1411
Diffstat:
12 files changed, 326 insertions(+), 89 deletions(-)
diff --git a/config/test.exs b/config/test.exs
@@ -29,7 +29,8 @@ config :pleroma, :instance,
email: "admin@example.com",
notify_email: "noreply@example.com",
skip_thread_containment: false,
- federating: false
+ federating: false,
+ external_user_synchronization: false
config :pleroma, :activitypub, sign_object_fetches: false
diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex
@@ -114,7 +114,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")
date =
@@ -141,4 +141,9 @@ defmodule Pleroma.Object.Fetcher do
{:error, e}
end
end
+
+ 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"}
end
diff --git 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
@@ -406,6 +408,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 +429,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)
@@ -709,32 +715,73 @@ 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)})
+ 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)})
+
+ 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
@@ -16,6 +16,8 @@ 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)
field(:confirmation_token, :string, default: nil)
@@ -223,7 +225,11 @@ defmodule Pleroma.User.Info do
:uri,
:hub,
:topic,
- :salmon
+ :salmon,
+ :hide_followers,
+ :hide_follows,
+ :follower_count,
+ :following_count
])
end
@@ -234,7 +240,11 @@ defmodule Pleroma.User.Info do
:source_data,
:banner,
:locked,
- :magic_key
+ :magic_key,
+ :follower_count,
+ :following_count,
+ :hide_follows,
+ :hide_followers
])
end
@@ -348,4 +358,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
@@ -1009,10 +1009,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"],
@@ -1036,6 +1036,71 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
{:ok, user_data}
end
+ 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(user.follower_address),
+ followers_count when is_integer(followers_count) <- followers_data["totalItems"],
+ {:ok, hide_followers} <- collection_private(followers_data) do
+ {:ok,
+ %{
+ hide_follows: hide_follows,
+ follower_count: followers_count,
+ following_count: following_count,
+ hide_followers: hide_followers
+ }}
+ 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, info} <- fetch_follow_information_for_user(data) do
+ 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
+ {:ok, false}
+ else
+ 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] ->
+ {:ok, true}
+
+ {:error, _} = e ->
+ e
+
+ e ->
+ {:error, e}
+ 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
@@ -1047,7 +1112,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
@@ -608,13 +608,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)
@@ -1076,10 +1076,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}
@@ -1112,27 +1108,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
diff --git a/test/fixtures/users_mock/masto_closed_followers_page.json b/test/fixtures/users_mock/masto_closed_followers_page.json
@@ -0,0 +1 @@
+{"@context":"https://www.w3.org/ns/activitystreams","id":"http://localhost:4001/users/masto_closed/followers?page=1","type":"OrderedCollectionPage","totalItems":437,"next":"http://localhost:4001/users/masto_closed/followers?page=2","partOf":"http://localhost:4001/users/masto_closed/followers","orderedItems":["https://testing.uguu.ltd/users/rin","https://patch.cx/users/rin","https://letsalllovela.in/users/xoxo","https://pleroma.site/users/crushv","https://aria.company/users/boris","https://kawen.space/users/crushv","https://freespeech.host/users/cvcvcv","https://pleroma.site/users/picpub","https://pixelfed.social/users/nosleep","https://boopsnoot.gq/users/5c1896d162f7d337f90492a3","https://pikachu.rocks/users/waifu","https://royal.crablettesare.life/users/crablettes"]}
diff --git a/test/fixtures/users_mock/masto_closed_following_page.json b/test/fixtures/users_mock/masto_closed_following_page.json
@@ -0,0 +1 @@
+{"@context":"https://www.w3.org/ns/activitystreams","id":"http://localhost:4001/users/masto_closed/following?page=1","type":"OrderedCollectionPage","totalItems":152,"next":"http://localhost:4001/users/masto_closed/following?page=2","partOf":"http://localhost:4001/users/masto_closed/following","orderedItems":["https://testing.uguu.ltd/users/rin","https://patch.cx/users/rin","https://letsalllovela.in/users/xoxo","https://pleroma.site/users/crushv","https://aria.company/users/boris","https://kawen.space/users/crushv","https://freespeech.host/users/cvcvcv","https://pleroma.site/users/picpub","https://pixelfed.social/users/nosleep","https://boopsnoot.gq/users/5c1896d162f7d337f90492a3","https://pikachu.rocks/users/waifu","https://royal.crablettesare.life/users/crablettes"]}
diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex
@@ -796,6 +796,14 @@ defmodule HttpRequestMock do
}}
end
+ def get("http://localhost:4001/users/masto_closed/followers?page=1", _, _, _) do
+ {:ok,
+ %Tesla.Env{
+ status: 200,
+ body: File.read!("test/fixtures/users_mock/masto_closed_followers_page.json")
+ }}
+ end
+
def get("http://localhost:4001/users/masto_closed/following", _, _, _) do
{:ok,
%Tesla.Env{
@@ -804,6 +812,14 @@ defmodule HttpRequestMock do
}}
end
+ def get("http://localhost:4001/users/masto_closed/following?page=1", _, _, _) do
+ {:ok,
+ %Tesla.Env{
+ status: 200,
+ body: File.read!("test/fixtures/users_mock/masto_closed_following_page.json")
+ }}
+ end
+
def get("http://localhost:4001/users/fuser2/followers", _, _, _) do
{:ok,
%Tesla.Env{
diff --git a/test/user_test.exs b/test/user_test.exs
@@ -1393,4 +1393,78 @@ defmodule Pleroma.UserTest do
assert %User{bio: "test-bio"} = User.get_cached_by_ap_id(user.ap_id)
end
end
+
+ describe "following/followers synchronization" do
+ setup do
+ sync = Pleroma.Config.get([:instance, :external_user_synchronization])
+ on_exit(fn -> Pleroma.Config.put([:instance, :external_user_synchronization], sync) end)
+ end
+
+ test "updates the counters normally on following/getting a follow when disabled" do
+ Pleroma.Config.put([:instance, :external_user_synchronization], false)
+ user = insert(:user)
+
+ other_user =
+ insert(:user,
+ local: false,
+ follower_address: "http://localhost:4001/users/masto_closed/followers",
+ following_address: "http://localhost:4001/users/masto_closed/following",
+ info: %{ap_enabled: true}
+ )
+
+ assert User.user_info(other_user).following_count == 0
+ assert User.user_info(other_user).follower_count == 0
+
+ {:ok, user} = Pleroma.User.follow(user, other_user)
+ other_user = Pleroma.User.get_by_id(other_user.id)
+
+ assert User.user_info(user).following_count == 1
+ assert User.user_info(other_user).follower_count == 1
+ end
+
+ test "syncronizes the counters with the remote instance for the followed when enabled" do
+ Pleroma.Config.put([:instance, :external_user_synchronization], false)
+
+ user = insert(:user)
+
+ other_user =
+ insert(:user,
+ local: false,
+ follower_address: "http://localhost:4001/users/masto_closed/followers",
+ following_address: "http://localhost:4001/users/masto_closed/following",
+ info: %{ap_enabled: true}
+ )
+
+ assert User.user_info(other_user).following_count == 0
+ assert User.user_info(other_user).follower_count == 0
+
+ Pleroma.Config.put([:instance, :external_user_synchronization], true)
+ {:ok, _user} = User.follow(user, other_user)
+ other_user = User.get_by_id(other_user.id)
+
+ assert User.user_info(other_user).follower_count == 437
+ end
+
+ test "syncronizes the counters with the remote instance for the follower when enabled" do
+ Pleroma.Config.put([:instance, :external_user_synchronization], false)
+
+ user = insert(:user)
+
+ other_user =
+ insert(:user,
+ local: false,
+ follower_address: "http://localhost:4001/users/masto_closed/followers",
+ following_address: "http://localhost:4001/users/masto_closed/following",
+ info: %{ap_enabled: true}
+ )
+
+ assert User.user_info(other_user).following_count == 0
+ assert User.user_info(other_user).follower_count == 0
+
+ Pleroma.Config.put([:instance, :external_user_synchronization], true)
+ {:ok, other_user} = User.follow(other_user, user)
+
+ assert User.user_info(other_user).following_count == 152
+ end
+ end
end
diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs
@@ -1128,4 +1128,65 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
assert result.id == activity.id
end
end
+
+ describe "fetch_follow_information_for_user" do
+ test "syncronizes following/followers counters" do
+ user =
+ insert(:user,
+ local: false,
+ follower_address: "http://localhost:4001/users/fuser2/followers",
+ following_address: "http://localhost:4001/users/fuser2/following"
+ )
+
+ {:ok, info} = ActivityPub.fetch_follow_information_for_user(user)
+ assert info.follower_count == 527
+ assert info.following_count == 267
+ end
+
+ test "detects hidden followers" do
+ mock(fn env ->
+ case env.url do
+ "http://localhost:4001/users/masto_closed/followers?page=1" ->
+ %Tesla.Env{status: 403, body: ""}
+
+ _ ->
+ apply(HttpRequestMock, :request, [env])
+ end
+ end)
+
+ user =
+ insert(:user,
+ local: false,
+ follower_address: "http://localhost:4001/users/masto_closed/followers",
+ following_address: "http://localhost:4001/users/masto_closed/following"
+ )
+
+ {:ok, info} = ActivityPub.fetch_follow_information_for_user(user)
+ assert info.hide_followers == true
+ assert info.hide_follows == false
+ end
+
+ test "detects hidden follows" do
+ mock(fn env ->
+ case env.url do
+ "http://localhost:4001/users/masto_closed/following?page=1" ->
+ %Tesla.Env{status: 403, body: ""}
+
+ _ ->
+ apply(HttpRequestMock, :request, [env])
+ end
+ end)
+
+ user =
+ insert(:user,
+ local: false,
+ follower_address: "http://localhost:4001/users/masto_closed/followers",
+ following_address: "http://localhost:4001/users/masto_closed/following"
+ )
+
+ {:ok, info} = ActivityPub.fetch_follow_information_for_user(user)
+ assert info.hide_followers == false
+ assert info.hide_follows == true
+ end
+ end
end
diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs
@@ -1373,32 +1373,4 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest 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
