Merge branch 'idempotency-key-optimistic-posting' into 'develop'

Add `idempotency_key` to the chat message entity

Closes #2126

See merge request pleroma/pleroma!3015
This commit is contained in:
feld 2020-10-31 17:01:29 +00:00
commit 37e8e8bf8e
10 changed files with 50 additions and 6 deletions

View file

@ -38,6 +38,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Pleroma API: Pagination for remote/local packs and emoji. - Pleroma API: Pagination for remote/local packs and emoji.
- Admin API: (`GET /api/pleroma/admin/users`) added filters user by `unconfirmed` status - Admin API: (`GET /api/pleroma/admin/users`) added filters user by `unconfirmed` status
- Admin API: (`GET /api/pleroma/admin/users`) added filters user by `actor_type` - Admin API: (`GET /api/pleroma/admin/users`) added filters user by `actor_type`
- Pleroma API: Add `idempotency_key` to the chat message entity that can be used for optimistic message sending.
</details> </details>

View file

@ -173,11 +173,14 @@ Returned data:
"created_at": "2020-04-21T15:06:45.000Z", "created_at": "2020-04-21T15:06:45.000Z",
"emojis": [], "emojis": [],
"id": "12", "id": "12",
"unread": false "unread": false,
"idempotency_key": "75442486-0874-440c-9db1-a7006c25a31f"
} }
] ]
``` ```
- idempotency_key: The copy of the `idempotency-key` HTTP request header that can be used for optimistic message sending. Included only during the first few minutes after the message creation.
### Posting a chat message ### Posting a chat message
Posting a chat message for given Chat id works like this: Posting a chat message for given Chat id works like this:

View file

@ -168,7 +168,11 @@ defp cachex_children do
build_cachex("web_resp", limit: 2500), build_cachex("web_resp", limit: 2500),
build_cachex("emoji_packs", expiration: emoji_packs_expiration(), limit: 10), build_cachex("emoji_packs", expiration: emoji_packs_expiration(), limit: 10),
build_cachex("failed_proxy_url", limit: 2500), build_cachex("failed_proxy_url", limit: 2500),
build_cachex("banned_urls", default_ttl: :timer.hours(24 * 30), limit: 5_000) build_cachex("banned_urls", default_ttl: :timer.hours(24 * 30), limit: 5_000),
build_cachex("chat_message_id_idempotency_key",
expiration: chat_message_id_idempotency_key_expiration(),
limit: 500_000
)
] ]
end end
@ -178,6 +182,9 @@ defp emoji_packs_expiration,
defp idempotency_expiration, defp idempotency_expiration,
do: expiration(default: :timer.seconds(6 * 60 * 60), interval: :timer.seconds(60)) do: expiration(default: :timer.seconds(6 * 60 * 60), interval: :timer.seconds(60))
defp chat_message_id_idempotency_key_expiration,
do: expiration(default: :timer.minutes(2), interval: :timer.seconds(60))
defp seconds_valid_interval, defp seconds_valid_interval,
do: :timer.seconds(Config.get!([Pleroma.Captcha, :seconds_valid])) do: :timer.seconds(Config.get!([Pleroma.Captcha, :seconds_valid]))

View file

@ -312,6 +312,12 @@ def handle_object_creation(%{"type" => "ChatMessage"} = object, meta) do
{:ok, chat} = Chat.bump_or_create(user.id, other_user.ap_id) {:ok, chat} = Chat.bump_or_create(user.id, other_user.ap_id)
{:ok, cm_ref} = MessageReference.create(chat, object, user.ap_id != actor.ap_id) {:ok, cm_ref} = MessageReference.create(chat, object, user.ap_id != actor.ap_id)
Cachex.put(
:chat_message_id_idempotency_key_cache,
cm_ref.id,
meta[:idempotency_key]
)
{ {
["user", "user:pleroma_chat"], ["user", "user:pleroma_chat"],
{user, %{cm_ref | chat: chat, object: object}} {user, %{cm_ref | chat: chat, object: object}}

View file

@ -45,7 +45,8 @@ def post_chat_message(%User{} = user, %User{} = recipient, content, opts \\ [])
{_, {:ok, %Activity{} = activity, _meta}} <- {_, {:ok, %Activity{} = activity, _meta}} <-
{:common_pipeline, {:common_pipeline,
Pipeline.common_pipeline(create_activity_data, Pipeline.common_pipeline(create_activity_data,
local: true local: true,
idempotency_key: opts[:idempotency_key]
)} do )} do
{:ok, activity} {:ok, activity}
else else

View file

@ -80,7 +80,8 @@ def post_chat_message(
%User{} = recipient <- User.get_cached_by_ap_id(chat.recipient), %User{} = recipient <- User.get_cached_by_ap_id(chat.recipient),
{:ok, activity} <- {:ok, activity} <-
CommonAPI.post_chat_message(user, recipient, params[:content], CommonAPI.post_chat_message(user, recipient, params[:content],
media_id: params[:media_id] media_id: params[:media_id],
idempotency_key: idempotency_key(conn)
), ),
message <- Object.normalize(activity, false), message <- Object.normalize(activity, false),
cm_ref <- MessageReference.for_chat_and_object(chat, message) do cm_ref <- MessageReference.for_chat_and_object(chat, message) do
@ -169,4 +170,11 @@ def show(%{assigns: %{user: user}} = conn, %{id: id}) do
|> render("show.json", chat: chat) |> render("show.json", chat: chat)
end end
end end
defp idempotency_key(conn) do
case get_req_header(conn, "idempotency-key") do
[key] -> key
_ -> nil
end
end
end end

View file

@ -5,6 +5,7 @@
defmodule Pleroma.Web.PleromaAPI.Chat.MessageReferenceView do defmodule Pleroma.Web.PleromaAPI.Chat.MessageReferenceView do
use Pleroma.Web, :view use Pleroma.Web, :view
alias Pleroma.Maps
alias Pleroma.User alias Pleroma.User
alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.CommonAPI.Utils
alias Pleroma.Web.MastodonAPI.StatusView alias Pleroma.Web.MastodonAPI.StatusView
@ -37,6 +38,7 @@ def render(
Pleroma.Web.RichMedia.Helpers.fetch_data_for_object(object) Pleroma.Web.RichMedia.Helpers.fetch_data_for_object(object)
) )
} }
|> put_idempotency_key()
end end
def render("index.json", opts) do def render("index.json", opts) do
@ -47,4 +49,13 @@ def render("index.json", opts) do
Map.put(opts, :as, :chat_message_reference) Map.put(opts, :as, :chat_message_reference)
) )
end end
defp put_idempotency_key(data) do
with {:ok, idempotency_key} <- Cachex.get(:chat_message_id_idempotency_key_cache, data.id) do
data
|> Maps.put_if_present(:idempotency_key, idempotency_key)
else
_ -> data
end
end
end end

View file

@ -82,11 +82,13 @@ test "it posts a message to the chat", %{conn: conn, user: user} do
result = result =
conn conn
|> put_req_header("content-type", "application/json") |> put_req_header("content-type", "application/json")
|> put_req_header("idempotency-key", "123")
|> post("/api/v1/pleroma/chats/#{chat.id}/messages", %{"content" => "Hallo!!"}) |> post("/api/v1/pleroma/chats/#{chat.id}/messages", %{"content" => "Hallo!!"})
|> json_response_and_validate_schema(200) |> json_response_and_validate_schema(200)
assert result["content"] == "Hallo!!" assert result["content"] == "Hallo!!"
assert result["chat_id"] == chat.id |> to_string() assert result["chat_id"] == chat.id |> to_string()
assert result["idempotency_key"] == "123"
end end
test "it fails if there is no content", %{conn: conn, user: user} do test "it fails if there is no content", %{conn: conn, user: user} do

View file

@ -25,7 +25,9 @@ test "it displays a chat message" do
} }
{:ok, upload} = ActivityPub.upload(file, actor: user.ap_id) {:ok, upload} = ActivityPub.upload(file, actor: user.ap_id)
{:ok, activity} = CommonAPI.post_chat_message(user, recipient, "kippis :firefox:")
{:ok, activity} =
CommonAPI.post_chat_message(user, recipient, "kippis :firefox:", idempotency_key: "123")
chat = Chat.get(user.id, recipient.ap_id) chat = Chat.get(user.id, recipient.ap_id)
@ -42,6 +44,7 @@ test "it displays a chat message" do
assert chat_message[:created_at] assert chat_message[:created_at]
assert chat_message[:unread] == false assert chat_message[:unread] == false
assert match?([%{shortcode: "firefox"}], chat_message[:emojis]) assert match?([%{shortcode: "firefox"}], chat_message[:emojis])
assert chat_message[:idempotency_key] == "123"
clear_config([:rich_media, :enabled], true) clear_config([:rich_media, :enabled], true)

View file

@ -255,7 +255,9 @@ test "it sends chat messages to the 'user:pleroma_chat' stream", %{
} do } do
other_user = insert(:user) other_user = insert(:user)
{:ok, create_activity} = CommonAPI.post_chat_message(other_user, user, "hey cirno") {:ok, create_activity} =
CommonAPI.post_chat_message(other_user, user, "hey cirno", idempotency_key: "123")
object = Object.normalize(create_activity, false) object = Object.normalize(create_activity, false)
chat = Chat.get(user.id, other_user.ap_id) chat = Chat.get(user.id, other_user.ap_id)
cm_ref = MessageReference.for_chat_and_object(chat, object) cm_ref = MessageReference.for_chat_and_object(chat, object)