Change emoji api responses to JSON

lib/pleroma/web/emoji_api/emoji_api_controller.ex

@@ -6,7 +6,7 @@ defmodule Pleroma.Web.EmojiAPI.EmojiAPIController do
   def reload(conn, _params) do
     Pleroma.Emoji.reload()
 
-    conn |> text("ok")
+    conn |> json("ok")
   end
 
   @emoji_dir_path Path.join(
@@ -133,13 +133,15 @@ def download_shared(conn, %{"name" => name}) do
       {:can_download?, _} ->
         conn
         |> put_status(:forbidden)
-        |> text("Pack #{name} cannot be downloaded from this instance, either pack sharing\
-           was disabled for this pack or some files are missing")
+        |> json(%{
+          error: "Pack #{name} cannot be downloaded from this instance, either pack sharing\
+           was disabled for this pack or some files are missing"
+        })
 
       {:exists?, _} ->
         conn
         |> put_status(:not_found)
-        |> text("Pack #{name} does not exist")
+        |> json(%{error: "Pack #{name} does not exist"})
     end
   end
 
@@ -200,15 +202,15 @@ def download_from(conn, %{"instance_address" => address, "pack_name" => name} =
         File.write!(pack_file_path, Jason.encode!(full_pack, pretty: true))
       end
 
-      text(conn, "ok")
+      json(conn, "ok")
     else
       {:error, e} ->
-        conn |> put_status(:internal_server_error) |> text(e)
+        conn |> put_status(:internal_server_error) |> json(%{error: e})
 
       {:sha, _} ->
         conn
         |> put_status(:internal_server_error)
-        |> text("SHA256 for the pack doesn't match the one sent by the server")
+        |> json(%{error: "SHA256 for the pack doesn't match the one sent by the server"})
     end
   end
 
@@ -228,11 +230,11 @@ def create(conn, %{"name" => name}) do
         Jason.encode!(%{pack: %{}, files: %{}})
       )
 
-      conn |> text("ok")
+      conn |> json("ok")
     else
       conn
       |> put_status(:conflict)
-      |> text("A pack named \"#{name}\" already exists")
+      |> json(%{error: "A pack named \"#{name}\" already exists"})
     end
   end
 
@@ -244,10 +246,12 @@ def delete(conn, %{"name" => name}) do
 
     case File.rm_rf(pack_dir) do
       {:ok, _} ->
-        conn |> text("ok")
+        conn |> json("ok")
 
       {:error, _} ->
-        conn |> put_status(:internal_server_error) |> text("Couldn't delete the pack #{name}")
+        conn
+        |> put_status(:internal_server_error)
+        |> json(%{error: "Couldn't delete the pack #{name}"})
     end
   end
 
@@ -281,7 +285,7 @@ def update_metadata(conn, %{"pack_name" => name, "new_data" => new_data}) do
       {:has_all_files?, _} ->
         conn
         |> put_status(:bad_request)
-        |> text("The fallback archive does not have all files specified in pack.json")
+        |> json(%{error: "The fallback archive does not have all files specified in pack.json"})
     end
   end
 
@@ -302,6 +306,25 @@ defp update_metadata_and_send(conn, full_pack, new_data, pack_file_p) do
     json(conn, new_data)
   end
 
+  defp get_filename(%{"filename" => filename}), do: filename
+
+  defp get_filename(%{"file" => file}) do
+    case file do
+      %Plug.Upload{filename: filename} -> filename
+      url when is_binary(url) -> Path.basename(url)
+    end
+  end
+
+  defp empty?(str), do: String.trim(str) == ""
+
+  defp update_file_and_send(conn, updated_full_pack, pack_file_p) do
+    # Write the emoji pack file
+    File.write!(pack_file_p, Jason.encode!(updated_full_pack, pretty: true))
+
+    # Return the modified file list
+    json(conn, updated_full_pack["files"])
+  end
+
   @doc """
   Updates a file in a pack.
 
@@ -316,31 +339,21 @@ defp update_metadata_and_send(conn, full_pack, new_data, pack_file_p) do
     (from the current filename to `new_filename`)
   - `remove` removes the emoji named `shortcode` and it's associated file
   """
+
+  # Add
   def update_file(
         conn,
-        %{"pack_name" => pack_name, "action" => action, "shortcode" => shortcode} = params
+        %{"pack_name" => pack_name, "action" => "add", "shortcode" => shortcode} = params
       ) do
     pack_dir = Path.join(@emoji_dir_path, pack_name)
     pack_file_p = Path.join(pack_dir, "pack.json")
 
     full_pack = Jason.decode!(File.read!(pack_file_p))
 
-    res =
-      case action do
-        "add" ->
-          unless Map.has_key?(full_pack["files"], shortcode) do
-            filename =
-              if Map.has_key?(params, "filename") do
-                params["filename"]
-              else
-                case params["file"] do
-                  %Plug.Upload{filename: filename} -> filename
-                  url when is_binary(url) -> Path.basename(url)
-                end
-              end
-
-            unless String.trim(shortcode) |> String.length() == 0 or
-                     String.trim(filename) |> String.length() == 0 do
+    with {_, false} <- {:has_shortcode, Map.has_key?(full_pack["files"], shortcode)},
+         filename <- get_filename(params),
+         false <- empty?(shortcode),
+         false <- empty?(filename) do
       file_path = Path.join(pack_dir, filename)
 
       # If the name contains directories, create them
@@ -360,22 +373,31 @@ def update_file(
       end
 
       updated_full_pack = put_in(full_pack, ["files", shortcode], filename)
-
-              {:ok, updated_full_pack}
+      update_file_and_send(conn, updated_full_pack, pack_file_p)
     else
-              {:error,
-               conn
-               |> put_status(:bad_request)
-               |> text("shortcode or filename cannot be empty")}
-            end
-          else
-            {:error,
+      {:has_shortcode, _} ->
         conn
         |> put_status(:conflict)
-             |> text("An emoji with the \"#{shortcode}\" shortcode already exists")}
+        |> json(%{error: "An emoji with the \"#{shortcode}\" shortcode already exists"})
+
+      true ->
+        conn
+        |> put_status(:bad_request)
+        |> json(%{error: "shortcode or filename cannot be empty"})
+    end
   end
 
-        "remove" ->
+  # Remove
+  def update_file(conn, %{
+        "pack_name" => pack_name,
+        "action" => "remove",
+        "shortcode" => shortcode
+      }) do
+    pack_dir = Path.join(@emoji_dir_path, pack_name)
+    pack_file_p = Path.join(pack_dir, "pack.json")
+
+    full_pack = Jason.decode!(File.read!(pack_file_p))
+
     if Map.has_key?(full_pack["files"], shortcode) do
       {emoji_file_path, updated_full_pack} = pop_in(full_pack, ["files", shortcode])
 
@@ -393,17 +415,28 @@ def update_file(
         end
       end
 
-            {:ok, updated_full_pack}
+      update_file_and_send(conn, updated_full_pack, pack_file_p)
     else
-            {:error,
-             conn |> put_status(:bad_request) |> text("Emoji \"#{shortcode}\" does not exist")}
+      conn
+      |> put_status(:bad_request)
+      |> json(%{error: "Emoji \"#{shortcode}\" does not exist"})
+    end
   end
 
-        "update" ->
-          if Map.has_key?(full_pack["files"], shortcode) do
-            with %{"new_shortcode" => new_shortcode, "new_filename" => new_filename} <- params do
-              unless String.trim(new_shortcode) |> String.length() == 0 or
-                       String.trim(new_filename) |> String.length() == 0 do
+  # Update
+  def update_file(
+        conn,
+        %{"pack_name" => pack_name, "action" => "update", "shortcode" => shortcode} = params
+      ) do
+    pack_dir = Path.join(@emoji_dir_path, pack_name)
+    pack_file_p = Path.join(pack_dir, "pack.json")
+
+    full_pack = Jason.decode!(File.read!(pack_file_p))
+
+    with {_, true} <- {:has_shortcode, Map.has_key?(full_pack["files"], shortcode)},
+         %{"new_shortcode" => new_shortcode, "new_filename" => new_filename} <- params,
+         false <- empty?(new_shortcode),
+         false <- empty?(new_filename) do
       # First, remove the old shortcode, saving the old path
       {old_emoji_file_path, updated_full_pack} = pop_in(full_pack, ["files", shortcode])
       old_emoji_file_path = Path.join(pack_dir, old_emoji_file_path)
@@ -428,43 +461,30 @@ def update_file(
       end
 
       # Then, put in the new shortcode with the new path
-                updated_full_pack =
-                  put_in(updated_full_pack, ["files", new_shortcode], new_filename)
-
-                {:ok, updated_full_pack}
+      updated_full_pack = put_in(updated_full_pack, ["files", new_shortcode], new_filename)
+      update_file_and_send(conn, updated_full_pack, pack_file_p)
     else
-                {:error,
+      {:has_shortcode, _} ->
         conn
         |> put_status(:bad_request)
-                 |> text("new_shortcode or new_filename cannot be empty")}
-              end
-            else
-              _ ->
-                {:error,
+        |> json(%{error: "Emoji \"#{shortcode}\" does not exist"})
+
+      true ->
         conn
         |> put_status(:bad_request)
-                 |> text("new_shortcode or new_file were not specified")}
-            end
-          else
-            {:error,
-             conn |> put_status(:bad_request) |> text("Emoji \"#{shortcode}\" does not exist")}
-          end
+        |> json(%{error: "new_shortcode or new_filename cannot be empty"})
 
       _ ->
-          {:error, conn |> put_status(:bad_request) |> text("Unknown action: #{action}")}
+        conn
+        |> put_status(:bad_request)
+        |> json(%{error: "new_shortcode or new_file were not specified"})
+    end
   end
 
-    case res do
-      {:ok, updated_full_pack} ->
-        # Write the emoji pack file
-        File.write!(pack_file_p, Jason.encode!(updated_full_pack, pretty: true))
-
-        # Return the modified file list
-        conn |> json(updated_full_pack["files"])
-
-      {:error, e} ->
-        e
-    end
+  def update_file(conn, %{"action" => action}) do
+    conn
+    |> put_status(:bad_request)
+    |> json(%{error: "Unknown action: #{action}"})
   end
 
   @doc """
@@ -493,7 +513,7 @@ def import_from_fs(conn, _params) do
       {:error, _} ->
         conn
         |> put_status(:internal_server_error)
-        |> text("Error accessing emoji pack directory")
+        |> json(%{error: "Error accessing emoji pack directory"})
     end
   end
 

test/web/emoji_api_controller_test.exs

@@ -101,14 +101,14 @@ test "downloading shared & unshared packs from another instance via download_fro
              }
              |> Jason.encode!()
            )
-           |> text_response(200) == "ok"
+           |> json_response(200) == "ok"
 
     assert File.exists?("#{@emoji_dir_path}/test_pack2/pack.json")
     assert File.exists?("#{@emoji_dir_path}/test_pack2/blank.png")
 
     assert conn
            |> delete(emoji_api_path(conn, :delete, "test_pack2"))
-           |> response(200) == "ok"
+           |> json_response(200) == "ok"
 
     refute File.exists?("#{@emoji_dir_path}/test_pack2")
 
@@ -130,14 +130,14 @@ test "downloading shared & unshared packs from another instance via download_fro
              }
              |> Jason.encode!()
            )
-           |> text_response(200) == "ok"
+           |> json_response(200) == "ok"
 
     assert File.exists?("#{@emoji_dir_path}/test_pack_nonshared2/pack.json")
     assert File.exists?("#{@emoji_dir_path}/test_pack_nonshared2/blank.png")
 
     assert conn
            |> delete(emoji_api_path(conn, :delete, "test_pack_nonshared2"))
-           |> response(200) == "ok"
+           |> json_response(200) == "ok"
 
     refute File.exists?("#{@emoji_dir_path}/test_pack_nonshared2")
   end
@@ -225,7 +225,7 @@ test "when the fallback source doesn't have all the files", ctx do
 
       conn = build_conn()
 
-      assert conn
+      assert (conn
               |> assign(:user, ctx[:admin])
               |> post(
                 emoji_api_path(conn, :update_metadata, "test_pack"),
@@ -233,7 +233,7 @@ test "when the fallback source doesn't have all the files", ctx do
                   "new_data" => new_data
                 }
               )
-             |> text_response(:bad_request) =~ "does not have all"
+              |> json_response(:bad_request))["error"] =~ "does not have all"
     end
   end
 
@@ -267,9 +267,9 @@ test "updating pack files" do
 
     conn = conn |> assign(:user, admin)
 
-    assert conn
+    assert (conn
             |> post(emoji_api_path(conn, :update_file, "test_pack"), same_name)
-           |> text_response(:conflict) =~ "already exists"
+            |> json_response(:conflict))["error"] =~ "already exists"
 
     assert conn
            |> post(emoji_api_path(conn, :update_file, "test_pack"), different_name)
@@ -350,7 +350,7 @@ test "creating and deleting a pack" do
                "test_created"
              )
            )
-           |> text_response(200) == "ok"
+           |> json_response(200) == "ok"
 
     assert File.exists?("#{@emoji_dir_path}/test_created/pack.json")
 
@@ -361,7 +361,7 @@ test "creating and deleting a pack" do
 
     assert conn
            |> delete(emoji_api_path(conn, :delete, "test_created"))
-           |> response(200) == "ok"
+           |> json_response(200) == "ok"
 
     refute File.exists?("#{@emoji_dir_path}/test_created/pack.json")
   end