From 0f0acc740d30c47d093f27875d4decf0693b2845 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 13 May 2020 15:31:28 +0200 Subject: [PATCH] Chat: Allow posting without content if an attachment is present. --- docs/API/chats.md | 5 ++- .../chat_message_validator.ex | 14 +++++++- .../web/api_spec/operations/chat_operation.ex | 12 ++++--- .../web/api_spec/schemas/chat_message.ex | 2 +- lib/pleroma/web/common_api/common_api.ex | 17 +++++++--- .../controllers/chat_controller.ex | 7 ++-- .../activity_pub/object_validator_test.exs | 32 +++++++++++++++++++ test/web/common_api/common_api_test.exs | 23 +++++++++++++ .../controllers/chat_controller_test.exs | 18 +++++++++-- 9 files changed, 111 insertions(+), 19 deletions(-) diff --git a/docs/API/chats.md b/docs/API/chats.md index ad36961ae..1ea18ff5f 100644 --- a/docs/API/chats.md +++ b/docs/API/chats.md @@ -166,11 +166,10 @@ Posting a chat message for given Chat id works like this: `POST /api/v1/pleroma/chats/{id}/messages` Parameters: -- content: The text content of the message +- content: The text content of the message. Optional if media is attached. - media_id: The id of an upload that will be attached to the message. -Currently, no formatting beyond basic escaping and emoji is implemented, as well as no -attachments. This will most probably change. +Currently, no formatting beyond basic escaping and emoji is implemented. Returned data: diff --git a/lib/pleroma/web/activity_pub/object_validators/chat_message_validator.ex b/lib/pleroma/web/activity_pub/object_validators/chat_message_validator.ex index e40c80ab4..9c20c188a 100644 --- a/lib/pleroma/web/activity_pub/object_validators/chat_message_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/chat_message_validator.ex @@ -61,12 +61,24 @@ def changeset(struct, data) do def validate_data(data_cng) do data_cng |> validate_inclusion(:type, ["ChatMessage"]) - |> validate_required([:id, :actor, :to, :type, :content, :published]) + |> validate_required([:id, :actor, :to, :type, :published]) + |> validate_content_or_attachment() |> validate_length(:to, is: 1) |> validate_length(:content, max: Pleroma.Config.get([:instance, :remote_limit])) |> validate_local_concern() end + def validate_content_or_attachment(cng) do + attachment = get_field(cng, :attachment) + + if attachment do + cng + else + cng + |> validate_required([:content]) + end + end + @doc """ Validates the following - If both users are in our system diff --git a/lib/pleroma/web/api_spec/operations/chat_operation.ex b/lib/pleroma/web/api_spec/operations/chat_operation.ex index 8ba10c603..a1c5db5dc 100644 --- a/lib/pleroma/web/api_spec/operations/chat_operation.ex +++ b/lib/pleroma/web/api_spec/operations/chat_operation.ex @@ -5,6 +5,7 @@ defmodule Pleroma.Web.ApiSpec.ChatOperation do alias OpenApiSpex.Operation alias OpenApiSpex.Schema + alias Pleroma.Web.ApiSpec.Schemas.ApiError alias Pleroma.Web.ApiSpec.Schemas.Chat alias Pleroma.Web.ApiSpec.Schemas.ChatMessage @@ -149,14 +150,15 @@ def post_chat_message_operation do parameters: [ Operation.parameter(:id, :path, :string, "The ID of the Chat") ], - requestBody: request_body("Parameters", chat_message_create(), required: true), + requestBody: request_body("Parameters", chat_message_create()), responses: %{ 200 => Operation.response( "The newly created ChatMessage", "application/json", ChatMessage - ) + ), + 400 => Operation.response("Bad Request", "application/json", ApiError) }, security: [ %{ @@ -292,10 +294,12 @@ def chat_message_create do description: "POST body for creating an chat message", type: :object, properties: %{ - content: %Schema{type: :string, description: "The content of your message"}, + content: %Schema{ + type: :string, + description: "The content of your message. Optional if media_id is present" + }, media_id: %Schema{type: :string, description: "The id of an upload"} }, - required: [:content], example: %{ "content" => "Hey wanna buy feet pics?", "media_id" => "134234" diff --git a/lib/pleroma/web/api_spec/schemas/chat_message.ex b/lib/pleroma/web/api_spec/schemas/chat_message.ex index 6e8f1a10a..3ee85aa76 100644 --- a/lib/pleroma/web/api_spec/schemas/chat_message.ex +++ b/lib/pleroma/web/api_spec/schemas/chat_message.ex @@ -16,7 +16,7 @@ defmodule Pleroma.Web.ApiSpec.Schemas.ChatMessage do id: %Schema{type: :string}, account_id: %Schema{type: :string, description: "The Mastodon API id of the actor"}, chat_id: %Schema{type: :string}, - content: %Schema{type: :string}, + content: %Schema{type: :string, nullable: true}, created_at: %Schema{type: :string, format: :"date-time"}, emojis: %Schema{type: :array}, attachment: %Schema{type: :object, nullable: true} diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 664175a4f..7008cea44 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -26,14 +26,14 @@ defmodule Pleroma.Web.CommonAPI do require Logger def post_chat_message(%User{} = user, %User{} = recipient, content, opts \\ []) do - with :ok <- validate_chat_content_length(content), - maybe_attachment <- opts[:media_id] && Object.get_by_id(opts[:media_id]), + with maybe_attachment <- opts[:media_id] && Object.get_by_id(opts[:media_id]), + :ok <- validate_chat_content_length(content, !!maybe_attachment), {_, {:ok, chat_message_data, _meta}} <- {:build_object, Builder.chat_message( user, recipient.ap_id, - content |> Formatter.html_escape("text/plain"), + content |> format_chat_content, attachment: maybe_attachment )}, {_, {:ok, create_activity_data, _meta}} <- @@ -47,7 +47,16 @@ def post_chat_message(%User{} = user, %User{} = recipient, content, opts \\ []) end end - defp validate_chat_content_length(content) do + defp format_chat_content(nil), do: nil + + defp format_chat_content(content) do + content |> Formatter.html_escape("text/plain") + end + + defp validate_chat_content_length(_, true), do: :ok + defp validate_chat_content_length(nil, false), do: {:error, :no_content} + + defp validate_chat_content_length(content, _) do if String.length(content) <= Pleroma.Config.get([:instance, :chat_limit]) do :ok else diff --git a/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex b/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex index 496cb8e87..210c8ec4a 100644 --- a/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex @@ -58,8 +58,7 @@ def delete_message(%{assigns: %{user: %{ap_id: actor} = user}} = conn, %{ end def post_chat_message( - %{body_params: %{content: content} = params, assigns: %{user: %{id: user_id} = user}} = - conn, + %{body_params: params, assigns: %{user: %{id: user_id} = user}} = conn, %{ id: id } @@ -67,7 +66,9 @@ def post_chat_message( with %Chat{} = chat <- Repo.get_by(Chat, id: id, user_id: user_id), %User{} = recipient <- User.get_cached_by_ap_id(chat.recipient), {:ok, activity} <- - CommonAPI.post_chat_message(user, recipient, content, media_id: params[:media_id]), + CommonAPI.post_chat_message(user, recipient, params[:content], + media_id: params[:media_id] + ), message <- Object.normalize(activity) do conn |> put_view(ChatMessageView) diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index d9f5a8fac..da33d3dbc 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -103,6 +103,38 @@ test "validates for a basic object with an attachment", %{ assert object["attachment"] end + test "validates for a basic object with an attachment but without content", %{ + valid_chat_message: valid_chat_message, + user: user + } do + file = %Plug.Upload{ + content_type: "image/jpg", + path: Path.absname("test/fixtures/image.jpg"), + filename: "an_image.jpg" + } + + {:ok, attachment} = ActivityPub.upload(file, actor: user.ap_id) + + valid_chat_message = + valid_chat_message + |> Map.put("attachment", attachment.data) + |> Map.delete("content") + + assert {:ok, object, _meta} = ObjectValidator.validate(valid_chat_message, []) + + assert object["attachment"] + end + + test "does not validate if the message has no content", %{ + valid_chat_message: valid_chat_message + } do + contentless = + valid_chat_message + |> Map.delete("content") + + refute match?({:ok, _object, _meta}, ObjectValidator.validate(contentless, [])) + end + test "does not validate if the message is longer than the remote_limit", %{ valid_chat_message: valid_chat_message } do diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index fd2c486a1..46ffd2888 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -27,6 +27,29 @@ defmodule Pleroma.Web.CommonAPITest do describe "posting chat messages" do setup do: clear_config([:instance, :chat_limit]) + test "it posts a chat message without content but with an attachment" do + author = insert(:user) + recipient = insert(:user) + + file = %Plug.Upload{ + content_type: "image/jpg", + path: Path.absname("test/fixtures/image.jpg"), + filename: "an_image.jpg" + } + + {:ok, upload} = ActivityPub.upload(file, actor: author.ap_id) + + {:ok, activity} = + CommonAPI.post_chat_message( + author, + recipient, + nil, + media_id: upload.id + ) + + assert activity + end + test "it posts a chat message" do author = insert(:user) recipient = insert(:user) diff --git a/test/web/pleroma_api/controllers/chat_controller_test.exs b/test/web/pleroma_api/controllers/chat_controller_test.exs index 037dd2297..d79aa3148 100644 --- a/test/web/pleroma_api/controllers/chat_controller_test.exs +++ b/test/web/pleroma_api/controllers/chat_controller_test.exs @@ -53,6 +53,20 @@ test "it posts a message to the chat", %{conn: conn, user: user} do assert result["chat_id"] == chat.id |> to_string() end + test "it fails if there is no content", %{conn: conn, user: user} do + other_user = insert(:user) + + {:ok, chat} = Chat.get_or_create(user.id, other_user.ap_id) + + result = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/pleroma/chats/#{chat.id}/messages") + |> json_response_and_validate_schema(400) + + assert result + end + test "it works with an attachment", %{conn: conn, user: user} do file = %Plug.Upload{ content_type: "image/jpg", @@ -70,13 +84,11 @@ test "it works with an attachment", %{conn: conn, user: user} do conn |> put_req_header("content-type", "application/json") |> post("/api/v1/pleroma/chats/#{chat.id}/messages", %{ - "content" => "Hallo!!", "media_id" => to_string(upload.id) }) |> json_response_and_validate_schema(200) - assert result["content"] == "Hallo!!" - assert result["chat_id"] == chat.id |> to_string() + assert result["attachment"] end end