From 74e0e009d1fa550c441cc1cce6e43dba26b23d45 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 2 Mar 2021 14:34:52 +0100 Subject: [PATCH] Add cron job to clean old activities Signed-off-by: Thomas Citharel --- config/config.exs | 5 +- lib/graphql/resolvers/activity.ex | 2 +- lib/mobilizon/activities/activities.ex | 25 +++++- lib/service/clean_old_activity.ex | 64 ++++++++++++++ .../workers/clean_old_activity_worker.ex | 13 +++ test/service/clean_old_activity_test.exs | 84 +++++++++++++++++++ test/service/clean_orphan_media_test.exs | 2 +- test/service/clean_unconfirmed_users_test.exs | 2 +- test/support/factory.ex | 2 +- 9 files changed, 192 insertions(+), 7 deletions(-) create mode 100644 lib/service/clean_old_activity.ex create mode 100644 lib/service/workers/clean_old_activity_worker.ex create mode 100644 test/service/clean_old_activity_test.exs diff --git a/config/config.exs b/config/config.exs index 0daf59ff7..499cf18aa 100644 --- a/config/config.exs +++ b/config/config.exs @@ -32,6 +32,8 @@ config :mobilizon, :instance, orphan_upload_grace_period_hours: 48, remove_unconfirmed_users: true, unconfirmed_user_grace_period_hours: 48, + activity_expire_days: 365, + activity_keep_number: 100, email_from: "noreply@localhost", email_reply_to: "noreply@localhost" @@ -269,7 +271,8 @@ config :mobilizon, Oban, {"17 * * * *", Mobilizon.Service.Workers.RefreshGroups, queue: :background}, # To be activated in Mobilizon 1.2 # {"@hourly", Mobilizon.Service.Workers.CleanOrphanMediaWorker, queue: :background}, - {"@hourly", Mobilizon.Service.Workers.CleanUnconfirmedUsersWorker, queue: :background} + {"@hourly", Mobilizon.Service.Workers.CleanUnconfirmedUsersWorker, queue: :background}, + {"@daily", Mobilizon.Service.Workers.CleanOldActivityWorker, queue: :background} ]}, {Oban.Plugins.Pruner, max_age: 300} ] diff --git a/lib/graphql/resolvers/activity.ex b/lib/graphql/resolvers/activity.ex index aa6f73644..e76de34f7 100644 --- a/lib/graphql/resolvers/activity.ex +++ b/lib/graphql/resolvers/activity.ex @@ -19,7 +19,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Activity do with {:actor, %Actor{id: actor_id} = _actor} <- {:actor, Users.get_actor_for_user(user)}, {:member, true} <- {:member, Actors.is_member?(actor_id, group_id) or is_moderator(role)} do %Page{total: total, elements: elements} = - Activities.list_activities_for_group(group_id, actor_id, [], page, limit) + Activities.list_group_activities_for_member(group_id, actor_id, [], page, limit) elements = Enum.map(elements, fn %Activity{} = activity -> diff --git a/lib/mobilizon/activities/activities.ex b/lib/mobilizon/activities/activities.ex index 84e8a7bd0..bb263c798 100644 --- a/lib/mobilizon/activities/activities.ex +++ b/lib/mobilizon/activities/activities.ex @@ -72,13 +72,14 @@ defmodule Mobilizon.Activities do Repo.all(Activity) end - @spec list_activities_for_group( + @spec list_group_activities_for_member( + integer() | String.t(), integer() | String.t(), Keyword.t(), integer() | nil, integer() | nil ) :: Page.t() - def list_activities_for_group( + def list_group_activities_for_member( group_id, actor_asking_id, filters \\ [], @@ -97,6 +98,26 @@ defmodule Mobilizon.Activities do |> Page.build_page(page, limit) end + @spec list_group_activities( + integer() | String.t(), + Keyword.t(), + integer() | nil, + integer() | nil + ) :: Page.t() + def list_group_activities( + group_id, + filters \\ [], + page \\ nil, + limit \\ nil + ) do + Activity + |> where([a], a.group_id == ^group_id) + |> filter_object_type(Keyword.get(filters, :type)) + |> order_by(desc: :inserted_at) + |> preload([:author, :group]) + |> Page.build_page(page, limit) + end + @doc """ Gets a single activity. diff --git a/lib/service/clean_old_activity.ex b/lib/service/clean_old_activity.ex new file mode 100644 index 000000000..8801eee5f --- /dev/null +++ b/lib/service/clean_old_activity.ex @@ -0,0 +1,64 @@ +defmodule Mobilizon.Service.CleanOldActivity do + @moduledoc """ + Service to clean old activities + """ + + alias Mobilizon.Activities.Activity + alias Mobilizon.Actors.Actor + alias Mobilizon.Config + alias Mobilizon.Storage.Repo + import Ecto.Query + + @doc """ + Clean old activities + + Remove activities that are older than a certain period + + Options: + * `grace_period` how old in hours can the media be before it's taken into account for deletion + * `dry_run` just return the media that would have been deleted, don't actually delete it + """ + @spec clean(Keyword.t()) :: {:ok, list(Media.t())} | {:error, String.t()} + def clean(opts \\ []) do + {query, nb_actors} = find_activities(opts) + + if Keyword.get(opts, :dry_run, false) do + nb_activities = Repo.aggregate(query, :count) + {:ok, actors: nb_actors, activities: nb_activities} + else + {nb_activities, _} = Repo.delete_all(query) + {:ok, actors: nb_actors, activities: nb_activities} + end + end + + @spec find_activities(Keyword.t()) :: {Ecto.Query.t(), list()} + defp find_activities(opts) do + grace_period = + Keyword.get(opts, :grace_period, Config.get([:instance, :activity_expire_days], 365)) + + expiration_date = DateTime.add(DateTime.utc_now(), grace_period * -3600) + + activities_to_keep = + Keyword.get( + opts, + :activity_keep_number, + Config.get([:instance, :activity_keep_number], 100) + ) + + actor_ids = + Actor + |> where(type: :Group) + |> join(:inner, [ac], a in Activity, on: a.group_id == ac.id) + |> group_by([ac], ac.id) + |> having([_ac, a], count(a.id) > ^activities_to_keep) + |> select([ac], ac.id) + |> Repo.all() + + query = + Activity + |> where([a], a.inserted_at < ^expiration_date) + |> where([a], a.group_id in ^actor_ids) + + {query, length(actor_ids)} + end +end diff --git a/lib/service/workers/clean_old_activity_worker.ex b/lib/service/workers/clean_old_activity_worker.ex new file mode 100644 index 000000000..038d705af --- /dev/null +++ b/lib/service/workers/clean_old_activity_worker.ex @@ -0,0 +1,13 @@ +defmodule Mobilizon.Service.Workers.CleanOldActivityWorker do + @moduledoc """ + Worker to clean old activity + """ + + use Oban.Worker, queue: "background" + alias Mobilizon.Service.CleanOldActivity + + @impl Oban.Worker + def perform(%Job{}) do + CleanOldActivity.clean() + end +end diff --git a/test/service/clean_old_activity_test.exs b/test/service/clean_old_activity_test.exs new file mode 100644 index 000000000..37c4c7206 --- /dev/null +++ b/test/service/clean_old_activity_test.exs @@ -0,0 +1,84 @@ +defmodule Mobilizon.Service.CleanOldActivityTest do + use Mobilizon.DataCase + + import Mobilizon.Factory + + alias Mobilizon.{Activities, Config} + alias Mobilizon.Service.CleanOldActivity + + @activity_inserted_at_1 DateTime.from_iso8601("2019-01-02T10:33:39.207493Z") |> elem(1) + @activity_inserted_at_2 DateTime.from_iso8601("2021-03-02T10:33:39.207493Z") |> elem(1) + + setup do + group1 = insert(:group) + group2 = insert(:group) + Config.clear_config_cache() + + {:ok, group1: group1, group2: group2} + end + + describe "clean old activities" do + test "with default settings", %{group1: group1, group2: group2} do + create_activities(group1, group2) + Config.put([:instance, :activity_expire_days], 100) + Config.put([:instance, :activity_keep_number], 5) + assert Activities.list_group_activities(group1.id).total == 10 + assert Activities.list_group_activities(group2.id).total == 5 + + assert {:ok, actors: 1, activities: 5} == CleanOldActivity.clean(dry_run: true) + + assert Activities.list_group_activities(group1.id).total == 10 + assert Activities.list_group_activities(group2.id).total == 5 + + assert {:ok, actors: 1, activities: 5} == CleanOldActivity.clean() + + assert Activities.list_group_activities(group1.id).total == 5 + assert Activities.list_group_activities(group2.id).total == 5 + + assert {:ok, actors: 0, activities: 0} == CleanOldActivity.clean() + + assert Activities.list_group_activities(group1.id).total == 5 + assert Activities.list_group_activities(group2.id).total == 5 + Config.put([:instance, :activity_expire_days], 365) + Config.put([:instance, :activity_keep_number], 100) + end + + test "with custom settings", %{group1: group1, group2: group2} do + create_activities(group1, group2) + assert Activities.list_group_activities(group1.id).total == 10 + assert Activities.list_group_activities(group2.id).total == 5 + + assert {:ok, actors: 1, activities: 5} == + CleanOldActivity.clean(grace_period: 100, activity_keep_number: 5, dry_run: true) + + assert Activities.list_group_activities(group1.id).total == 10 + assert Activities.list_group_activities(group2.id).total == 5 + + assert {:ok, actors: 1, activities: 5} == + CleanOldActivity.clean(grace_period: 100, activity_keep_number: 5) + + assert Activities.list_group_activities(group1.id).total == 5 + assert Activities.list_group_activities(group2.id).total == 5 + + assert {:ok, actors: 0, activities: 0} == + CleanOldActivity.clean(grace_period: 100, activity_keep_number: 5) + + assert Activities.list_group_activities(group1.id).total == 5 + assert Activities.list_group_activities(group2.id).total == 5 + end + end + + defp create_activities(group1, group2) do + Enum.each(1..5, fn _ -> + insert(:mobilizon_activity, group: group1, inserted_at: @activity_inserted_at_1) + end) + + Enum.each(1..5, fn _ -> + insert(:mobilizon_activity, group: group1, inserted_at: @activity_inserted_at_2) + end) + + Enum.each(1..5, fn _ -> + insert(:mobilizon_activity, group: group2, inserted_at: @activity_inserted_at_2) + end) + end +end diff --git a/test/service/clean_orphan_media_test.exs b/test/service/clean_orphan_media_test.exs index f16d3a5cc..21c1c2264 100644 --- a/test/service/clean_orphan_media_test.exs +++ b/test/service/clean_orphan_media_test.exs @@ -1,4 +1,4 @@ -defmodule Mix.Tasks.Mobilizon.Media.CleanOrphanTest do +defmodule Mobilizon.Service.CleanOrphanMediaTest do use Mobilizon.DataCase import Mobilizon.Factory diff --git a/test/service/clean_unconfirmed_users_test.exs b/test/service/clean_unconfirmed_users_test.exs index 51ced50a3..8cf0b740b 100644 --- a/test/service/clean_unconfirmed_users_test.exs +++ b/test/service/clean_unconfirmed_users_test.exs @@ -1,4 +1,4 @@ -defmodule Mix.Tasks.Mobilizon.User.CleanUnconfirmedUsersTest do +defmodule Mobilizon.Service.CleanUnconfirmedUsersTest do use Mobilizon.DataCase import Mobilizon.Factory diff --git a/test/support/factory.ex b/test/support/factory.ex index b38a8da4b..0f4d54ada 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -418,7 +418,7 @@ defmodule Mobilizon.Factory do %Mobilizon.Activities.Activity{ type: :event, subject: :event_created, - subject_params: %{event: event}, + subject_params: %{event_title: event.title}, author: actor, group: group, object_type: :event,