From 568e34858893f2bab713a210ae82834e3f6c030e Mon Sep 17 00:00:00 2001 From: eugenijm Date: Mon, 25 Mar 2019 20:21:48 +0300 Subject: [PATCH] Increment replies_count on replies (MastoAPI) --- lib/pleroma/activity.ex | 46 +++++++++++++ lib/pleroma/object.ex | 46 +++++++++++++ lib/pleroma/web/activity_pub/activity_pub.ex | 26 +++++++ .../web/mastodon_api/views/status_view.ex | 2 +- ...190325215156_update_status_reply_count.exs | 48 +++++++++++++ test/web/activity_pub/activity_pub_test.exs | 68 +++++++++++++++++++ 6 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 priv/repo/migrations/20190325215156_update_status_reply_count.exs diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 3dfabe9f3..bc3f8caba 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -245,4 +245,50 @@ def all_by_actor_and_id(actor, status_ids) do |> where([s], s.actor == ^actor) |> Repo.all() end + + def increase_replies_count(id) do + Activity + |> where(id: ^id) + |> update([a], + set: [ + data: + fragment( + """ + jsonb_set(?, '{object, repliesCount}', + (coalesce((?->'object'->>'repliesCount')::int, 0) + 1)::varchar::jsonb, true) + """, + a.data, + a.data + ) + ] + ) + |> Repo.update_all([]) + |> case do + {1, [activity]} -> activity + _ -> {:error, "Not found"} + end + end + + def decrease_replies_count(id) do + Activity + |> where(id: ^id) + |> update([a], + set: [ + data: + fragment( + """ + jsonb_set(?, '{object, repliesCount}', + (greatest(0, (?->'object'->>'repliesCount')::int - 1))::varchar::jsonb, true) + """, + a.data, + a.data + ) + ] + ) + |> Repo.update_all([]) + |> case do + {1, [activity]} -> activity + _ -> {:error, "Not found"} + end + end end diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 193ae3fa8..8a670645d 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -133,4 +133,50 @@ def update_and_set_cache(changeset) do e -> e end end + + def increase_replies_count(ap_id) do + Object + |> where([o], fragment("?->>'id' = ?::text", o.data, ^to_string(ap_id))) + |> update([o], + set: [ + data: + fragment( + """ + jsonb_set(?, '{repliesCount}', + (coalesce((?->>'repliesCount')::int, 0) + 1)::varchar::jsonb, true) + """, + o.data, + o.data + ) + ] + ) + |> Repo.update_all([]) + |> case do + {1, [object]} -> set_cache(object) + _ -> {:error, "Not found"} + end + end + + def decrease_replies_count(ap_id) do + Object + |> where([o], fragment("?->>'id' = ?::text", o.data, ^to_string(ap_id))) + |> update([o], + set: [ + data: + fragment( + """ + jsonb_set(?, '{repliesCount}', + (greatest(0, (?->>'repliesCount')::int - 1))::varchar::jsonb, true) + """, + o.data, + o.data + ) + ] + ) + |> Repo.update_all([]) + |> case do + {1, [object]} -> set_cache(object) + _ -> {:error, "Not found"} + end + end end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 80c64ae04..0d9a89d0b 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -89,6 +89,30 @@ def decrease_note_count_if_public(actor, object) do if is_public?(object), do: User.decrease_note_count(actor), else: {:ok, actor} end + def increase_replies_count_if_reply(%{ + "object" => + %{"inReplyTo" => reply_ap_id, "inReplyToStatusId" => reply_status_id} = object, + "type" => "Create" + }) do + if is_public?(object) do + Activity.increase_replies_count(reply_status_id) + Object.increase_replies_count(reply_ap_id) + end + end + + def increase_replies_count_if_reply(_create_data), do: :noop + + def decrease_replies_count_if_reply(%Object{ + data: %{"inReplyTo" => reply_ap_id, "inReplyToStatusId" => reply_status_id} = object + }) do + if is_public?(object) do + Activity.decrease_replies_count(reply_status_id) + Object.decrease_replies_count(reply_ap_id) + end + end + + def decrease_replies_count_if_reply(_object), do: :noop + def insert(map, local \\ true) when is_map(map) do with nil <- Activity.normalize(map), map <- lazy_put_activity_defaults(map), @@ -178,6 +202,7 @@ def create(%{to: to, actor: actor, context: context, object: object} = params) d additional ), {:ok, activity} <- insert(create_data, local), + _ <- increase_replies_count_if_reply(create_data), # Changing note count prior to enqueuing federation task in order to avoid # race conditions on updating user.info {:ok, _actor} <- increase_note_count_if_public(actor, activity), @@ -329,6 +354,7 @@ def delete(%Object{data: %{"id" => id, "actor" => actor}} = object, local \\ tru "deleted_activity_id" => activity && activity.id }, {:ok, activity} <- insert(data, local), + _ <- decrease_replies_count_if_reply(object), # Changing note count prior to enqueuing federation task in order to avoid # race conditions on updating user.info {:ok, _actor} <- decrease_note_count_if_public(user, object), diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 1ca8338cc..200bb453d 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -174,7 +174,7 @@ def render("status.json", %{activity: %{data: %{"object" => object}} = activity} content: content, created_at: created_at, reblogs_count: announcement_count, - replies_count: 0, + replies_count: object["repliesCount"] || 0, favourites_count: like_count, reblogged: present?(repeated), favourited: present?(favorited), diff --git a/priv/repo/migrations/20190325215156_update_status_reply_count.exs b/priv/repo/migrations/20190325215156_update_status_reply_count.exs new file mode 100644 index 000000000..50f1fe10b --- /dev/null +++ b/priv/repo/migrations/20190325215156_update_status_reply_count.exs @@ -0,0 +1,48 @@ +defmodule Pleroma.Repo.Migrations.UpdateStatusReplyCount do + use Ecto.Migration + + @public "https://www.w3.org/ns/activitystreams#Public" + + def up do + execute(""" + WITH reply_count AS ( + SELECT count(*) AS count, data->>'inReplyTo' AS ap_id + FROM objects + WHERE + data->>'inReplyTo' IS NOT NULL AND + data->>'type' = 'Note' AND ( + data->'cc' ? '#{@public}' OR + data->'to' ? '#{@public}') + GROUP BY data->>'inReplyTo' + ) + UPDATE objects AS o + SET "data" = jsonb_set(o.data, '{repliesCount}', reply_count.count::varchar::jsonb, true) + FROM reply_count + WHERE reply_count.ap_id = o.data->>'id'; + """) + + execute(""" + WITH reply_count AS (SELECT + count(*) as count, + data->'object'->>'inReplyTo' AS ap_id + FROM + activities + WHERE + data->'object'->>'inReplyTo' IS NOT NULL AND + data->'object'->>'type' = 'Note' AND ( + data->'object'->'cc' ? '#{@public}' OR + data->'object'->'to' ? '#{@public}') + GROUP BY + data->'object'->>'inReplyTo' + ) + UPDATE activities AS a + SET "data" = jsonb_set(a.data, '{object, repliesCount}', reply_count.count::varchar::jsonb, true) + FROM reply_count + WHERE reply_count.ap_id = a.data->'object'->>'id'; + """) + end + + def down do + :noop + end +end diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 96ad64e62..40bcced33 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -232,6 +232,39 @@ test "increases user note count only for public activities" do user = Repo.get(User, user.id) assert user.info.note_count == 2 end + + test "increases replies count" do + user = insert(:user) + user2 = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "1", "visibility" => "public"}) + ap_id = activity.data["id"] + reply_data = %{"status" => "1", "in_reply_to_status_id" => activity.id} + + # public + {:ok, _} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "public")) + assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) + assert data["object"]["repliesCount"] == 1 + assert object.data["repliesCount"] == 1 + + # unlisted + {:ok, _} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "unlisted")) + assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) + assert data["object"]["repliesCount"] == 2 + assert object.data["repliesCount"] == 2 + + # private + {:ok, _} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "private")) + assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) + assert data["object"]["repliesCount"] == 2 + assert object.data["repliesCount"] == 2 + + # direct + {:ok, _} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "direct")) + assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) + assert data["object"]["repliesCount"] == 2 + assert object.data["repliesCount"] == 2 + end end describe "fetch activities for recipients" do @@ -751,6 +784,40 @@ test "it creates a delete activity and checks that it is also sent to users ment assert user.ap_id in delete.data["to"] end + + test "decreases reply count" do + user = insert(:user) + user2 = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "1", "visibility" => "public"}) + reply_data = %{"status" => "1", "in_reply_to_status_id" => activity.id} + ap_id = activity.data["id"] + + {:ok, public_reply} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "public")) + {:ok, unlisted_reply} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "unlisted")) + {:ok, private_reply} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "private")) + {:ok, direct_reply} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "direct")) + + _ = CommonAPI.delete(direct_reply.id, user2) + assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) + assert data["object"]["repliesCount"] == 2 + assert object.data["repliesCount"] == 2 + + _ = CommonAPI.delete(private_reply.id, user2) + assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) + assert data["object"]["repliesCount"] == 2 + assert object.data["repliesCount"] == 2 + + _ = CommonAPI.delete(public_reply.id, user2) + assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) + assert data["object"]["repliesCount"] == 1 + assert object.data["repliesCount"] == 1 + + _ = CommonAPI.delete(unlisted_reply.id, user2) + assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) + assert data["object"]["repliesCount"] == 0 + assert object.data["repliesCount"] == 0 + end end describe "timeline post-processing" do @@ -789,6 +856,7 @@ test "it filters broken threads" do activities = ActivityPub.fetch_activities([user1.ap_id | user1.following]) + private_activity_1 = Activity.get_by_ap_id_with_object(private_activity_1.data["id"]) assert [public_activity, private_activity_1, private_activity_3] == activities assert length(activities) == 3