From 5839e67eb86d6d14b21222247ce8e113c3b26637 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Thu, 6 Feb 2020 18:01:12 +0300 Subject: [PATCH] return data only for updated emoji --- CHANGELOG.md | 7 +- docs/API/pleroma_api.md | 2 +- .../controllers/emoji_api_controller.ex | 93 +++++----- test/instance_static/add/shortcode.png | Bin 0 -> 95 bytes .../controllers/emoji_api_controller_test.exs | 162 ++++++++++-------- 5 files changed, 149 insertions(+), 115 deletions(-) create mode 100644 test/instance_static/add/shortcode.png diff --git a/CHANGELOG.md b/CHANGELOG.md index c0f1bcf57..a220c14f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -125,13 +125,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
API Changes -- **Breaking** EmojiReactions: Change endpoints and responses to align with Mastodon -- **Breaking** Admin API: `PATCH /api/pleroma/admin/users/:nickname/force_password_reset` is now `PATCH /api/pleroma/admin/users/force_password_reset` (accepts `nicknames` array in the request body) +- **Breaking:** EmojiReactions: Change endpoints and responses to align with Mastodon +- **Breaking:** Admin API: `PATCH /api/pleroma/admin/users/:nickname/force_password_reset` is now `PATCH /api/pleroma/admin/users/force_password_reset` (accepts `nicknames` array in the request body) - **Breaking:** Admin API: Return link alongside with token on password reset - **Breaking:** Admin API: `PUT /api/pleroma/admin/reports/:id` is now `PATCH /api/pleroma/admin/reports`, see admin_api.md for details - **Breaking:** `/api/pleroma/admin/users/invite_token` now uses `POST`, changed accepted params and returns full invite in json instead of only token string. -- **Breaking** replying to reports is now "report notes", enpoint changed from `POST /api/pleroma/admin/reports/:id/respond` to `POST /api/pleroma/admin/reports/:id/notes` +- **Breaking:** replying to reports is now "report notes", endpoint changed from `POST /api/pleroma/admin/reports/:id/respond` to `POST /api/pleroma/admin/reports/:id/notes` - Mastodon API: stopped sanitizing display names, field names and subject fields since they are supposed to be treated as plaintext +- **Breaking:** Pleroma API: `/api/pleroma/emoji/packs/:name/update_file` endpoint returns only updated emoji data. - Admin API: Return `total` when querying for reports - Mastodon API: Return `pleroma.direct_conversation_id` when creating a direct message (`POST /api/v1/statuses`) - Admin API: Return link alongside with token on password reset diff --git a/docs/API/pleroma_api.md b/docs/API/pleroma_api.md index 90c43c356..a7c7731ce 100644 --- a/docs/API/pleroma_api.md +++ b/docs/API/pleroma_api.md @@ -357,7 +357,7 @@ The status posting endpoint takes an additional parameter, `in_reply_to_conversa * if the `action` is `update`, changes emoji shortcode (from `shortcode` to `new_shortcode` or moves the file (from the current filename to `new_filename`) * if the `action` is `remove`, removes the emoji named `shortcode` and it's associated file -* Response: JSON, updated "files" section of the pack and 200 status, 409 if the trying to use a shortcode +* Response: JSON, emoji shortcode with filename which was added/updated/deleted and 200 status, 409 if the trying to use a shortcode that is already taken, 400 if there was an error with the shortcode, filename or file (additional info in the "error" part of the response JSON) diff --git a/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex b/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex index e01825b48..981bac4fa 100644 --- a/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex @@ -385,23 +385,35 @@ keeping it in cache for #{div(cache_ms, 1000)}s") 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 get_filename(%Plug.Upload{filename: filename}), do: filename + defp get_filename(url) when is_binary(url), do: Path.basename(url) 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)) + defp update_pack_file(updated_full_pack, pack_file_p) do + content = Jason.encode!(updated_full_pack, pretty: true) - # Return the modified file list - json(conn, updated_full_pack["files"]) + File.write!(pack_file_p, content) + end + + defp create_subdirs(file_path) do + if String.contains?(file_path, "/") do + file_path + |> Path.dirname() + |> File.mkdir_p!() + end + end + + defp pack_info(pack_name) do + dir = Path.join(emoji_dir_path(), pack_name) + json_path = Path.join(dir, "pack.json") + + json = + json_path + |> File.read!() + |> Jason.decode!() + + {dir, json_path, json} end @doc """ @@ -422,23 +434,25 @@ keeping it in cache for #{div(cache_ms, 1000)}s") # Add def update_file( conn, - %{"pack_name" => pack_name, "action" => "add", "shortcode" => shortcode} = params + %{"pack_name" => pack_name, "action" => "add"} = params ) do - pack_dir = Path.join(emoji_dir_path(), pack_name) - pack_file_p = Path.join(pack_dir, "pack.json") + shortcode = + if params["shortcode"] do + params["shortcode"] + else + filename = get_filename(params["file"]) + Path.basename(filename, Path.extname(filename)) + end - full_pack = Jason.decode!(File.read!(pack_file_p)) + {pack_dir, pack_file_p, full_pack} = pack_info(pack_name) with {_, false} <- {:has_shortcode, Map.has_key?(full_pack["files"], shortcode)}, - filename <- get_filename(params), + filename <- params["filename"] || get_filename(params["file"]), false <- empty?(shortcode), - false <- empty?(filename) do - file_path = Path.join(pack_dir, filename) - + false <- empty?(filename), + file_path <- Path.join(pack_dir, filename) do # If the name contains directories, create them - if String.contains?(file_path, "/") do - File.mkdir_p!(Path.dirname(file_path)) - end + create_subdirs(file_path) case params["file"] do %Plug.Upload{path: upload_path} -> @@ -451,8 +465,11 @@ keeping it in cache for #{div(cache_ms, 1000)}s") File.write!(file_path, file_contents) end - updated_full_pack = put_in(full_pack, ["files", shortcode], filename) - update_file_and_send(conn, updated_full_pack, pack_file_p) + full_pack + |> put_in(["files", shortcode], filename) + |> update_pack_file(pack_file_p) + + json(conn, %{shortcode => filename}) else {:has_shortcode, _} -> conn @@ -472,10 +489,7 @@ keeping it in cache for #{div(cache_ms, 1000)}s") "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)) + {pack_dir, pack_file_p, full_pack} = pack_info(pack_name) if Map.has_key?(full_pack["files"], shortcode) do {emoji_file_path, updated_full_pack} = pop_in(full_pack, ["files", shortcode]) @@ -494,7 +508,8 @@ keeping it in cache for #{div(cache_ms, 1000)}s") end end - update_file_and_send(conn, updated_full_pack, pack_file_p) + update_pack_file(updated_full_pack, pack_file_p) + json(conn, %{shortcode => full_pack["files"][shortcode]}) else conn |> put_status(:bad_request) @@ -507,10 +522,7 @@ keeping it in cache for #{div(cache_ms, 1000)}s") 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)) + {pack_dir, pack_file_p, full_pack} = pack_info(pack_name) with {_, true} <- {:has_shortcode, Map.has_key?(full_pack["files"], shortcode)}, %{"new_shortcode" => new_shortcode, "new_filename" => new_filename} <- params, @@ -522,9 +534,7 @@ keeping it in cache for #{div(cache_ms, 1000)}s") new_emoji_file_path = Path.join(pack_dir, new_filename) # If the name contains directories, create them - if String.contains?(new_emoji_file_path, "/") do - File.mkdir_p!(Path.dirname(new_emoji_file_path)) - end + create_subdirs(new_emoji_file_path) # Move/Rename the old filename to a new filename # These are probably on the same filesystem, so just rename should work @@ -540,8 +550,11 @@ keeping it in cache for #{div(cache_ms, 1000)}s") end # Then, put in the new shortcode with the new path - updated_full_pack = put_in(updated_full_pack, ["files", new_shortcode], new_filename) - update_file_and_send(conn, updated_full_pack, pack_file_p) + updated_full_pack + |> put_in(["files", new_shortcode], new_filename) + |> update_pack_file(pack_file_p) + + json(conn, %{new_shortcode => new_filename}) else {:has_shortcode, _} -> conn diff --git a/test/instance_static/add/shortcode.png b/test/instance_static/add/shortcode.png new file mode 100644 index 0000000000000000000000000000000000000000..8f50fa02340e7e09e562f86e00b6e4bd6ad1d565 GIT binary patch literal 95 zcmeAS@N?(olHy`uVBq!ia0vp^4Is=2Bp6=1#-sr$rjj7PU - File.write!(pack_file, original_content) + on_exit(fn -> + File.write!(pack_file, original_content) + end) - File.rm_rf!("#{@emoji_dir_path}/test_pack/blank_url.png") - File.rm_rf!("#{@emoji_dir_path}/test_pack/dir") - File.rm_rf!("#{@emoji_dir_path}/test_pack/dir_2") - end) + admin = insert(:user, is_admin: true) + %{conn: conn} = oauth_access(["admin:write"], user: admin) + {:ok, conn: conn} + end - admin = insert(:user, is_admin: true) - %{conn: conn} = oauth_access(["admin:write"], user: admin) + test "update file without shortcode", %{conn: conn} do + on_exit(fn -> File.rm_rf!("#{@emoji_dir_path}/test_pack/shortcode.png") end) - same_name = %{ - "action" => "add", - "shortcode" => "blank", - "filename" => "dir/blank.png", - "file" => %Plug.Upload{ - filename: "blank.png", - path: "#{@emoji_dir_path}/test_pack/blank.png" + assert conn + |> post("/api/pleroma/emoji/packs/test_pack/update_file", %{ + "action" => "add", + "file" => %Plug.Upload{ + filename: "shortcode.png", + path: "#{Pleroma.Config.get([:instance, :static_dir])}/add/shortcode.png" + } + }) + |> json_response(200) == %{"shortcode" => "shortcode.png"} + end + + test "updating pack files", %{conn: conn} do + on_exit(fn -> + File.rm_rf!("#{@emoji_dir_path}/test_pack/blank_url.png") + File.rm_rf!("#{@emoji_dir_path}/test_pack/dir") + File.rm_rf!("#{@emoji_dir_path}/test_pack/dir_2") + end) + + same_name = %{ + "action" => "add", + "shortcode" => "blank", + "filename" => "dir/blank.png", + "file" => %Plug.Upload{ + filename: "blank.png", + path: "#{@emoji_dir_path}/test_pack/blank.png" + } } - } - different_name = %{same_name | "shortcode" => "blank_2"} + different_name = %{same_name | "shortcode" => "blank_2"} - assert (conn - |> post(emoji_api_path(conn, :update_file, "test_pack"), same_name) - |> json_response(:conflict))["error"] =~ "already exists" + assert (conn + |> post(emoji_api_path(conn, :update_file, "test_pack"), same_name) + |> json_response(:conflict))["error"] =~ "already exists" - assert conn - |> post(emoji_api_path(conn, :update_file, "test_pack"), different_name) - |> json_response(200) == %{"blank" => "blank.png", "blank_2" => "dir/blank.png"} + assert conn + |> post(emoji_api_path(conn, :update_file, "test_pack"), different_name) + |> json_response(200) == %{"blank_2" => "dir/blank.png"} - assert File.exists?("#{@emoji_dir_path}/test_pack/dir/blank.png") + assert File.exists?("#{@emoji_dir_path}/test_pack/dir/blank.png") - assert conn - |> post(emoji_api_path(conn, :update_file, "test_pack"), %{ - "action" => "update", - "shortcode" => "blank_2", - "new_shortcode" => "blank_3", - "new_filename" => "dir_2/blank_3.png" - }) - |> json_response(200) == %{"blank" => "blank.png", "blank_3" => "dir_2/blank_3.png"} + assert conn + |> post(emoji_api_path(conn, :update_file, "test_pack"), %{ + "action" => "update", + "shortcode" => "blank_2", + "new_shortcode" => "blank_3", + "new_filename" => "dir_2/blank_3.png" + }) + |> json_response(200) == %{"blank_3" => "dir_2/blank_3.png"} - refute File.exists?("#{@emoji_dir_path}/test_pack/dir/") - assert File.exists?("#{@emoji_dir_path}/test_pack/dir_2/blank_3.png") + refute File.exists?("#{@emoji_dir_path}/test_pack/dir/") + assert File.exists?("#{@emoji_dir_path}/test_pack/dir_2/blank_3.png") - assert conn - |> post(emoji_api_path(conn, :update_file, "test_pack"), %{ - "action" => "remove", - "shortcode" => "blank_3" - }) - |> json_response(200) == %{"blank" => "blank.png"} + assert conn + |> post(emoji_api_path(conn, :update_file, "test_pack"), %{ + "action" => "remove", + "shortcode" => "blank_3" + }) + |> json_response(200) == %{"blank_3" => "dir_2/blank_3.png"} - refute File.exists?("#{@emoji_dir_path}/test_pack/dir_2/") + refute File.exists?("#{@emoji_dir_path}/test_pack/dir_2/") - mock(fn - %{ - method: :get, - url: "https://test-blank/blank_url.png" - } -> - text(File.read!("#{@emoji_dir_path}/test_pack/blank.png")) - end) + mock(fn + %{ + method: :get, + url: "https://test-blank/blank_url.png" + } -> + text(File.read!("#{@emoji_dir_path}/test_pack/blank.png")) + end) - # The name should be inferred from the URL ending - from_url = %{ - "action" => "add", - "shortcode" => "blank_url", - "file" => "https://test-blank/blank_url.png" - } + # The name should be inferred from the URL ending + from_url = %{ + "action" => "add", + "shortcode" => "blank_url", + "file" => "https://test-blank/blank_url.png" + } - assert conn - |> post(emoji_api_path(conn, :update_file, "test_pack"), from_url) - |> json_response(200) == %{ - "blank" => "blank.png", - "blank_url" => "blank_url.png" - } + assert conn + |> post(emoji_api_path(conn, :update_file, "test_pack"), from_url) + |> json_response(200) == %{ + "blank_url" => "blank_url.png" + } - assert File.exists?("#{@emoji_dir_path}/test_pack/blank_url.png") + assert File.exists?("#{@emoji_dir_path}/test_pack/blank_url.png") - assert conn - |> post(emoji_api_path(conn, :update_file, "test_pack"), %{ - "action" => "remove", - "shortcode" => "blank_url" - }) - |> json_response(200) == %{"blank" => "blank.png"} + assert conn + |> post(emoji_api_path(conn, :update_file, "test_pack"), %{ + "action" => "remove", + "shortcode" => "blank_url" + }) + |> json_response(200) == %{"blank_url" => "blank_url.png"} - refute File.exists?("#{@emoji_dir_path}/test_pack/blank_url.png") + refute File.exists?("#{@emoji_dir_path}/test_pack/blank_url.png") + end end test "creating and deleting a pack" do