Pipeline: Always run common_pipeline in a transaction for now.

This commit is contained in:
lain 2020-04-29 13:45:50 +02:00
parent 67659afe48
commit e055b8d203
4 changed files with 36 additions and 40 deletions

View file

@ -5,6 +5,7 @@
defmodule Pleroma.Web.ActivityPub.Pipeline do defmodule Pleroma.Web.ActivityPub.Pipeline do
alias Pleroma.Activity alias Pleroma.Activity
alias Pleroma.Object alias Pleroma.Object
alias Pleroma.Repo
alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.ActivityPub
alias Pleroma.Web.ActivityPub.MRF alias Pleroma.Web.ActivityPub.MRF
alias Pleroma.Web.ActivityPub.ObjectValidator alias Pleroma.Web.ActivityPub.ObjectValidator
@ -14,6 +15,16 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do
@spec common_pipeline(map(), keyword()) :: @spec common_pipeline(map(), keyword()) ::
{:ok, Activity.t() | Object.t(), keyword()} | {:error, any()} {:ok, Activity.t() | Object.t(), keyword()} | {:error, any()}
def common_pipeline(object, meta) do def common_pipeline(object, meta) do
case Repo.transaction(fn -> do_common_pipeline(object, meta) end) do
{:ok, value} ->
value
{:error, e} ->
{:error, e}
end
end
def do_common_pipeline(object, meta) do
with {_, {:ok, validated_object, meta}} <- with {_, {:ok, validated_object, meta}} <-
{:validate_object, ObjectValidator.validate(object, meta)}, {:validate_object, ObjectValidator.validate(object, meta)},
{_, {:ok, mrfd_object}} <- {:mrf_object, MRF.filter(validated_object)}, {_, {:ok, mrfd_object}} <- {:mrf_object, MRF.filter(validated_object)},

View file

@ -3,24 +3,18 @@
# SPDX-License-Identifier: AGPL-3.0-only # SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.ActivityPub.Transmogrifier.ChatMessageHandling do defmodule Pleroma.Web.ActivityPub.Transmogrifier.ChatMessageHandling do
alias Pleroma.Repo
alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Pipeline
def handle_incoming( def handle_incoming(
%{"type" => "Create", "object" => %{"type" => "ChatMessage"}} = data, %{"type" => "Create", "object" => %{"type" => "ChatMessage"}} = data,
_options _options
) do ) do
# Create has to be run inside a transaction because the object is created as a side effect. case Pipeline.common_pipeline(data, local: false) do
# If this does not work, we need to roll back creating the activity. {:ok, activity, _} ->
case Repo.transaction(fn -> Pipeline.common_pipeline(data, local: false) end) do
{:ok, {:ok, activity, _}} ->
{:ok, activity} {:ok, activity}
{:ok, e} -> e ->
e e
{:error, e} ->
{:error, e}
end end
end end
end end

View file

@ -9,7 +9,6 @@ defmodule Pleroma.Web.CommonAPI do
alias Pleroma.FollowingRelationship alias Pleroma.FollowingRelationship
alias Pleroma.Formatter alias Pleroma.Formatter
alias Pleroma.Object alias Pleroma.Object
alias Pleroma.Repo
alias Pleroma.ThreadMute alias Pleroma.ThreadMute
alias Pleroma.User alias Pleroma.User
alias Pleroma.UserRelationship alias Pleroma.UserRelationship
@ -26,8 +25,6 @@ defmodule Pleroma.Web.CommonAPI do
require Logger require Logger
def post_chat_message(%User{} = user, %User{} = recipient, content) do def post_chat_message(%User{} = user, %User{} = recipient, content) do
transaction =
Repo.transaction(fn ->
with {_, true} <- with {_, true} <-
{:content_length, {:content_length,
String.length(content) <= Pleroma.Config.get([:instance, :chat_limit])}, String.length(content) <= Pleroma.Config.get([:instance, :chat_limit])},
@ -39,8 +36,7 @@ def post_chat_message(%User{} = user, %User{} = recipient, content) do
content |> Formatter.html_escape("text/plain") content |> Formatter.html_escape("text/plain")
)}, )},
{_, {:ok, create_activity_data, _meta}} <- {_, {:ok, create_activity_data, _meta}} <-
{:build_create_activity, {:build_create_activity, Builder.create(user, chat_message_data, [recipient.ap_id])},
Builder.create(user, chat_message_data, [recipient.ap_id])},
{_, {:ok, %Activity{} = activity, _meta}} <- {_, {:ok, %Activity{} = activity, _meta}} <-
{:common_pipeline, {:common_pipeline,
Pipeline.common_pipeline(create_activity_data, Pipeline.common_pipeline(create_activity_data,
@ -51,12 +47,6 @@ def post_chat_message(%User{} = user, %User{} = recipient, content) do
{:content_length, false} -> {:error, :content_too_long} {:content_length, false} -> {:error, :content_too_long}
e -> e e -> e
end end
end)
case transaction do
{:ok, value} -> value
error -> error
end
end end
def follow(follower, followed) do def follow(follower, followed) do

View file

@ -68,6 +68,7 @@ test "it inserts it and creates a chat" do
recipient = insert(:user, ap_id: List.first(data["to"]), local: true) recipient = insert(:user, ap_id: List.first(data["to"]), local: true)
{:ok, %Activity{} = activity} = Transmogrifier.handle_incoming(data) {:ok, %Activity{} = activity} = Transmogrifier.handle_incoming(data)
assert activity.local == false
assert activity.actor == author.ap_id assert activity.actor == author.ap_id
assert activity.recipients == [recipient.ap_id, author.ap_id] assert activity.recipients == [recipient.ap_id, author.ap_id]