From 0f70e83e8c745645703be8084001b1ef92c03823 Mon Sep 17 00:00:00 2001 From: lain Date: Sun, 3 Jun 2018 19:11:22 +0200 Subject: [PATCH] Better error handling in TwitterApiController. --- lib/pleroma/activity.ex | 4 +++- lib/pleroma/web/common_api/utils.ex | 11 +++++---- lib/pleroma/web/twitter_api/twitter_api.ex | 6 ++--- .../web/twitter_api/twitter_api_controller.ex | 23 +++++++++++++++--- .../twitter_api_controller_test.exs | 24 +++++++++++++++++-- 5 files changed, 54 insertions(+), 14 deletions(-) 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 @@ def create_activity_by_object_id_query(ap_ids) 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 @@ def unblock(%User{} = blocker, params) 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 @@ defp unrepeat(%User{} = user, ap_id_or_id) 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 @@ def get_by_id_or_ap_id(id) 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 @@ defp forbidden_json_reply(conn, error_message) 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 @@ test "without valid credentials", %{conn: conn} 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 @@ test "with credentials", %{conn: conn, user: current_user} 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 @@ test "it returns the tags timeline", %{conn: conn} do test "Convert newlines to
in bio", %{conn: conn} do user = insert(:user) - conn = + _conn = conn |> assign(:user, user) |> post("/api/account/update_profile.json", %{ @@ -904,6 +922,8 @@ test "with credentials and valid password", %{conn: conn, user: current_user} 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