removed try/rescue

diff --git a/lib/pleroma/scheduled_activity.ex b/lib/pleroma/scheduled_activity.ex
index d011007028857accf5eb1e4ad976c38f4ff6a97f..68da0550c3366bdec3b01e2bf7f73896f970042e 100644 (file)
--- a/lib/pleroma/scheduled_activity.ex
+++ b/lib/pleroma/scheduled_activity.ex
@@ -156,7 +156,7 @@ defmodule Pleroma.ScheduledActivity 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 2a72742078bd6d6123b0903f52362c2e4d3d579e..7a52860a9722c2efdda601f9949302ac1e5b7d06 100644 (file)
--- a/lib/pleroma/workers/cron/purge_expired_activities_worker.ex
+++ b/lib/pleroma/workers/cron/purge_expired_activities_worker.ex
@@ -26,14 +26,21 @@ defmodule Pleroma.Workers.Cron.PurgeExpiredActivitiesWorker 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 5109d7f759aa77bd33a61fcc2088ba6905678d8d..bd41ab4ce4c754aaf0e088fd771222842fe07491 100644 (file)
--- a/lib/pleroma/workers/scheduled_activity_worker.ex
+++ b/lib/pleroma/workers/scheduled_activity_worker.ex
@@ -29,12 +29,12 @@ defmodule Pleroma.Workers.ScheduledActivityWorker 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 d2c5f5aa2c98b744c645ad0b98fe553fc114181a..6c13d300a560f14eea6a43f8e7cf315600ca9352 100644 (file)
--- a/test/scheduled_activity_test.exs
+++ b/test/scheduled_activity_test.exs
@@ -102,33 +102,4 @@ defmodule Pleroma.ScheduledActivityTest 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 07980bcd0b889bafdb9064c21d02efd363624212..c2561683e6e0062ea3f50d092c9c6217350fc22d 100644 (file)
--- 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 @@ defmodule Pleroma.Workers.Cron.PurgeExpiredActivitiesWorkerTest 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 (file)
index 0000000..1405d71
--- /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 <https://pleroma.social/>
+# 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