From ce7c887a27ad5af59f540650637a50da7f91fa52 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Thu, 23 Jan 2020 11:05:08 +0300 Subject: [PATCH] removed try/rescue --- lib/pleroma/scheduled_activity.ex | 2 +- .../cron/purge_expired_activities_worker.ex | 21 +++++--- .../workers/scheduled_activity_worker.ex | 12 ++--- test/scheduled_activity_test.exs | 29 ----------- .../purge_expired_activities_worker_test.exs | 22 ++++++++ .../scheduled_activity_worker_test.exs | 52 +++++++++++++++++++ 6 files changed, 95 insertions(+), 43 deletions(-) create mode 100644 test/workers/scheduled_activity_worker_test.exs diff --git a/lib/pleroma/scheduled_activity.ex b/lib/pleroma/scheduled_activity.ex index d01100702..68da0550c 100644 --- a/lib/pleroma/scheduled_activity.ex +++ b/lib/pleroma/scheduled_activity.ex @@ -156,7 +156,7 @@ def update(%ScheduledActivity{id: id} = scheduled_activity, attrs) do Multi.new() |> Multi.update(:scheduled_activity, changeset) |> Multi.update_all(:scheduled_job, job_query(id), - set: [scheduled_at: changeset.changes[:scheduled_at]] + set: [scheduled_at: get_field(changeset, :scheduled_at)] ) |> Repo.transaction() |> case do diff --git a/lib/pleroma/workers/cron/purge_expired_activities_worker.ex b/lib/pleroma/workers/cron/purge_expired_activities_worker.ex index 2a7274207..7a52860a9 100644 --- a/lib/pleroma/workers/cron/purge_expired_activities_worker.ex +++ b/lib/pleroma/workers/cron/purge_expired_activities_worker.ex @@ -26,14 +26,21 @@ def perform(_opts, _job) do end end - def delete_activity(expiration) do - try do - activity = Activity.get_by_id_with_object(expiration.activity_id) - user = User.get_by_ap_id(activity.object.data["actor"]) + def delete_activity(%ActivityExpiration{activity_id: activity_id}) do + with {:activity, %Activity{} = activity} <- + {:activity, Activity.get_by_id_with_object(activity_id)}, + {:user, %User{} = user} <- {:user, User.get_by_ap_id(activity.object.data["actor"])} do CommonAPI.delete(activity.id, user) - rescue - error -> - Logger.error("#{__MODULE__} Couldn't delete expired activity: #{inspect(error)}") + else + {:activity, _} -> + Logger.error( + "#{__MODULE__} Couldn't delete expired activity: not found activity ##{activity_id}" + ) + + {:user, _} -> + Logger.error( + "#{__MODULE__} Couldn't delete expired activity: not found actorof ##{activity_id}" + ) end end end diff --git a/lib/pleroma/workers/scheduled_activity_worker.ex b/lib/pleroma/workers/scheduled_activity_worker.ex index 5109d7f75..bd41ab4ce 100644 --- a/lib/pleroma/workers/scheduled_activity_worker.ex +++ b/lib/pleroma/workers/scheduled_activity_worker.ex @@ -29,12 +29,12 @@ def perform(%{"activity_id" => activity_id}, _job) do end end - defp post_activity(%ScheduledActivity{} = scheduled_activity) do - try do - {:ok, scheduled_activity} = ScheduledActivity.delete(scheduled_activity) - %User{} = user = User.get_cached_by_id(scheduled_activity.user_id) - {:ok, _result} = CommonAPI.post(user, scheduled_activity.params) - rescue + defp post_activity(%ScheduledActivity{user_id: user_id, params: params} = scheduled_activity) do + with {:delete, {:ok, _}} <- {:delete, ScheduledActivity.delete(scheduled_activity)}, + {:user, %User{} = user} <- {:user, User.get_cached_by_id(user_id)}, + {:post, {:ok, _}} <- {:post, CommonAPI.post(user, params)} do + :ok + else error -> Logger.error( "#{__MODULE__} Couldn't create a status from the scheduled activity: #{inspect(error)}" diff --git a/test/scheduled_activity_test.exs b/test/scheduled_activity_test.exs index d2c5f5aa2..6c13d300a 100644 --- a/test/scheduled_activity_test.exs +++ b/test/scheduled_activity_test.exs @@ -102,33 +102,4 @@ test "when scheduled_at is earlier than 5 minute from now" do assert changeset.errors == [scheduled_at: {"must be at least 5 minutes from now", []}] end end - - test "creates a status from the scheduled activity" do - Pleroma.Config.put([ScheduledActivity, :enabled], true) - user = insert(:user) - - naive_datetime = - NaiveDateTime.add( - NaiveDateTime.utc_now(), - -:timer.minutes(2), - :millisecond - ) - - scheduled_activity = - insert( - :scheduled_activity, - scheduled_at: naive_datetime, - user: user, - params: %{status: "hi"} - ) - - Pleroma.Workers.ScheduledActivityWorker.perform( - %{"activity_id" => scheduled_activity.id}, - :pid - ) - - refute Repo.get(ScheduledActivity, scheduled_activity.id) - activity = Repo.all(Pleroma.Activity) |> Enum.find(&(&1.actor == user.ap_id)) - assert Pleroma.Object.normalize(activity).data["content"] == "hi" - end end diff --git a/test/workers/cron/purge_expired_activities_worker_test.exs b/test/workers/cron/purge_expired_activities_worker_test.exs index 07980bcd0..c2561683e 100644 --- a/test/workers/cron/purge_expired_activities_worker_test.exs +++ b/test/workers/cron/purge_expired_activities_worker_test.exs @@ -4,8 +4,12 @@ defmodule Pleroma.Workers.Cron.PurgeExpiredActivitiesWorkerTest do use Pleroma.DataCase + alias Pleroma.ActivityExpiration + alias Pleroma.Workers.Cron.PurgeExpiredActivitiesWorker + import Pleroma.Factory + import ExUnit.CaptureLog clear_config([ActivityExpiration, :enabled]) @@ -31,4 +35,22 @@ test "deletes an expiration activity" do refute Pleroma.Repo.get(Pleroma.Activity, activity.id) refute Pleroma.Repo.get(Pleroma.ActivityExpiration, expiration.id) end + + describe "delete_activity/1" do + test "adds log message if activity isn't find" do + assert capture_log([level: :error], fn -> + PurgeExpiredActivitiesWorker.delete_activity(%ActivityExpiration{ + activity_id: "test-activity" + }) + end) =~ "Couldn't delete expired activity: not found activity" + end + + test "adds log message if actor isn't find" do + assert capture_log([level: :error], fn -> + PurgeExpiredActivitiesWorker.delete_activity(%ActivityExpiration{ + activity_id: "test-activity" + }) + end) =~ "Couldn't delete expired activity: not found activity" + end + end end diff --git a/test/workers/scheduled_activity_worker_test.exs b/test/workers/scheduled_activity_worker_test.exs new file mode 100644 index 000000000..1405d7142 --- /dev/null +++ b/test/workers/scheduled_activity_worker_test.exs @@ -0,0 +1,52 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.ScheduledActivityWorkerTest do + use Pleroma.DataCase + + alias Pleroma.ScheduledActivity + alias Pleroma.Workers.ScheduledActivityWorker + + import Pleroma.Factory + import ExUnit.CaptureLog + + clear_config([ScheduledActivity, :enabled]) + + test "creates a status from the scheduled activity" do + Pleroma.Config.put([ScheduledActivity, :enabled], true) + user = insert(:user) + + naive_datetime = + NaiveDateTime.add( + NaiveDateTime.utc_now(), + -:timer.minutes(2), + :millisecond + ) + + scheduled_activity = + insert( + :scheduled_activity, + scheduled_at: naive_datetime, + user: user, + params: %{status: "hi"} + ) + + ScheduledActivityWorker.perform( + %{"activity_id" => scheduled_activity.id}, + :pid + ) + + refute Repo.get(ScheduledActivity, scheduled_activity.id) + activity = Repo.all(Pleroma.Activity) |> Enum.find(&(&1.actor == user.ap_id)) + assert Pleroma.Object.normalize(activity).data["content"] == "hi" + end + + test "adds log message if ScheduledActivity isn't find" do + Pleroma.Config.put([ScheduledActivity, :enabled], true) + + assert capture_log([level: :error], fn -> + ScheduledActivityWorker.perform(%{"activity_id" => 42}, :pid) + end) =~ "Couldn't find scheduled activity" + end +end