From: lain <lain@soykaf.club>
Date: Sun, 3 Jun 2018 17:11:22 +0000 (+0200)
Subject: Better error handling in TwitterApiController.
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=0f70e83e8c745645703be8084001b1ef92c03823;p=akkoma

Better error handling in TwitterApiController.
---

diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex
index c7502981e..dd6805125 100644
--- a/lib/pleroma/activity.ex
+++ b/lib/pleroma/activity.ex
@@ -72,8 +72,10 @@ defmodule Pleroma.Activity do
     )
   end
 
-  def get_create_activity_by_object_ap_id(ap_id) do
+  def get_create_activity_by_object_ap_id(ap_id) when is_binary(ap_id) do
     create_activity_by_object_id_query([ap_id])
     |> Repo.one()
   end
+
+  def get_create_activity_by_object_ap_id(_), do: nil
 end
diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex
index 9c9951371..30089f553 100644
--- a/lib/pleroma/web/common_api/utils.ex
+++ b/lib/pleroma/web/common_api/utils.ex
@@ -9,11 +9,12 @@ defmodule Pleroma.Web.CommonAPI.Utils do
   def get_by_id_or_ap_id(id) do
     activity = Repo.get(Activity, id) || Activity.get_create_activity_by_object_ap_id(id)
 
-    if activity.data["type"] == "Create" do
-      activity
-    else
-      Activity.get_create_activity_by_object_ap_id(activity.data["object"])
-    end
+    activity &&
+      if activity.data["type"] == "Create" do
+        activity
+      else
+        Activity.get_create_activity_by_object_ap_id(activity.data["object"])
+      end
   end
 
   def get_replied_to_activity(id) when not is_nil(id) do
diff --git a/lib/pleroma/web/twitter_api/twitter_api.ex b/lib/pleroma/web/twitter_api/twitter_api.ex
index 331efa90b..ccc6fe8e7 100644
--- a/lib/pleroma/web/twitter_api/twitter_api.ex
+++ b/lib/pleroma/web/twitter_api/twitter_api.ex
@@ -64,7 +64,7 @@ defmodule Pleroma.Web.TwitterAPI.TwitterAPI do
   end
 
   def repeat(%User{} = user, ap_id_or_id) do
-    with {:ok, _announce, %{data: %{"id" => id}}} = CommonAPI.repeat(ap_id_or_id, user),
+    with {:ok, _announce, %{data: %{"id" => id}}} <- CommonAPI.repeat(ap_id_or_id, user),
          %Activity{} = activity <- Activity.get_create_activity_by_object_ap_id(id) do
       {:ok, activity}
     end
@@ -77,14 +77,14 @@ defmodule Pleroma.Web.TwitterAPI.TwitterAPI do
   end
 
   def fav(%User{} = user, ap_id_or_id) do
-    with {:ok, _fav, %{data: %{"id" => id}}} = CommonAPI.favorite(ap_id_or_id, user),
+    with {:ok, _fav, %{data: %{"id" => id}}} <- CommonAPI.favorite(ap_id_or_id, user),
          %Activity{} = activity <- Activity.get_create_activity_by_object_ap_id(id) do
       {:ok, activity}
     end
   end
 
   def unfav(%User{} = user, ap_id_or_id) do
-    with {:ok, _unfav, _fav, %{data: %{"id" => id}}} = CommonAPI.unfavorite(ap_id_or_id, user),
+    with {:ok, _unfav, _fav, %{data: %{"id" => id}}} <- CommonAPI.unfavorite(ap_id_or_id, user),
          %Activity{} = activity <- Activity.get_create_activity_by_object_ap_id(id) do
       {:ok, activity}
     end
diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex
index 320f2fcf4..d53dd0c44 100644
--- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex
+++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex
@@ -8,6 +8,8 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
 
   require Logger
 
+  action_fallback(:errors)
+
   def verify_credentials(%{assigns: %{user: user}} = conn, _params) do
     token = Phoenix.Token.sign(conn, "user socket", user.id)
     render(conn, UserView, "show.json", %{user: user, token: token})
@@ -218,19 +220,22 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
   end
 
   def favorite(%{assigns: %{user: user}} = conn, %{"id" => id}) do
-    with {:ok, activity} <- TwitterAPI.fav(user, id) do
+    with {_, {:ok, id}} <- {:param_cast, Ecto.Type.cast(:integer, id)},
+         {:ok, activity} <- TwitterAPI.fav(user, id) do
       render(conn, ActivityView, "activity.json", %{activity: activity, for: user})
     end
   end
 
   def unfavorite(%{assigns: %{user: user}} = conn, %{"id" => id}) do
-    with {:ok, activity} <- TwitterAPI.unfav(user, id) do
+    with {_, {:ok, id}} <- {:param_cast, Ecto.Type.cast(:integer, id)},
+         {:ok, activity} <- TwitterAPI.unfav(user, id) do
       render(conn, ActivityView, "activity.json", %{activity: activity, for: user})
     end
   end
 
   def retweet(%{assigns: %{user: user}} = conn, %{"id" => id}) do
-    with {:ok, activity} <- TwitterAPI.repeat(user, id) do
+    with {_, {:ok, id}} <- {:param_cast, Ecto.Type.cast(:integer, id)},
+         {:ok, activity} <- TwitterAPI.repeat(user, id) do
       render(conn, ActivityView, "activity.json", %{activity: activity, for: user})
     end
   end
@@ -389,4 +394,16 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
   defp error_json(conn, error_message) do
     %{"error" => error_message, "request" => conn.request_path} |> Jason.encode!()
   end
+
+  def errors(conn, {:param_cast, _}) do
+    conn
+    |> put_status(400)
+    |> json("Invalid parameters")
+  end
+
+  def errors(conn, _) do
+    conn
+    |> put_status(500)
+    |> json("Something went wrong")
+  end
 end
diff --git a/test/web/twitter_api/twitter_api_controller_test.exs b/test/web/twitter_api/twitter_api_controller_test.exs
index 03e5824a9..68f4331df 100644
--- a/test/web/twitter_api/twitter_api_controller_test.exs
+++ b/test/web/twitter_api/twitter_api_controller_test.exs
@@ -260,7 +260,7 @@ defmodule Pleroma.Web.TwitterAPI.ControllerTest do
     test "with credentials", %{conn: conn, user: current_user} do
       other_user = insert(:user)
 
-      {:ok, activity} =
+      {:ok, _activity} =
         ActivityBuilder.insert(%{"to" => [current_user.ap_id]}, %{user: other_user})
 
       conn =
@@ -510,6 +510,24 @@ defmodule Pleroma.Web.TwitterAPI.ControllerTest do
 
       assert json_response(conn, 200)
     end
+
+    test "with credentials, invalid param", %{conn: conn, user: current_user} do
+      conn =
+        conn
+        |> with_credentials(current_user.nickname, "test")
+        |> post("/api/favorites/create/wrong.json")
+
+      assert json_response(conn, 400)
+    end
+
+    test "with credentials, invalid activity", %{conn: conn, user: current_user} do
+      conn =
+        conn
+        |> with_credentials(current_user.nickname, "test")
+        |> post("/api/favorites/create/1.json")
+
+      assert json_response(conn, 500)
+    end
   end
 
   describe "POST /api/favorites/destroy/:id" do
@@ -793,7 +811,7 @@ defmodule Pleroma.Web.TwitterAPI.ControllerTest do
   test "Convert newlines to <br> in bio", %{conn: conn} do
     user = insert(:user)
 
-    conn =
+    _conn =
       conn
       |> assign(:user, user)
       |> post("/api/account/update_profile.json", %{
@@ -904,6 +922,8 @@ defmodule Pleroma.Web.TwitterAPI.ControllerTest do
         |> post("/api/pleroma/delete_account", %{"password" => "test"})
 
       assert json_response(conn, 200) == %{"status" => "success"}
+      # Wait a second for the started task to end
+      :timer.sleep(1000)
     end
   end
 end