commit: c3593639adfdd6f9e086aaab18bda5c83bcfcc8b
parent aafd7a687dea7595ee9431451d8e170fc3ff909e
Author: Tusooa Zhu <tusooa@kazv.moe>
Date: Thu, 9 Jun 2022 11:39:51 -0400
Fix incorrectly cached content after editing
Diffstat:
6 files changed, 120 insertions(+), 17 deletions(-)
diff --git a/lib/pleroma/activity/html.ex b/lib/pleroma/activity/html.ex
@@ -8,6 +8,40 @@ defmodule Pleroma.Activity.HTML do
@cachex Pleroma.Config.get([:cachex, :provider], Cachex)
+ # We store a list of cache keys related to an activity in a
+ # separate cache, scrubber_management_cache. It has the same
+ # size as scrubber_cache (see application.ex). Every time we add
+ # a cache to scrubber_cache, we update scrubber_management_cache.
+ #
+ # The most recent write of a certain key in the management cache
+ # is the same as the most recent write of any record related to that
+ # key in the main cache.
+ # Assuming LRW ( https://hexdocs.pm/cachex/Cachex.Policy.LRW.html ),
+ # this means when the management cache is evicted by cachex, all
+ # related records in the main cache will also have been evicted.
+
+ defp get_cache_keys_for(activity_id) do
+ with {:ok, list} when is_list(list) <- @cachex.get(:scrubber_management_cache, activity_id) do
+ list
+ else
+ _ -> []
+ end
+ end
+
+ defp add_cache_key_for(activity_id, additional_key) do
+ current = get_cache_keys_for(activity_id)
+
+ unless additional_key in current do
+ @cachex.put(:scrubber_management_cache, activity_id, [additional_key | current])
+ end
+ end
+
+ def invalidate_cache_for(activity_id) do
+ keys = get_cache_keys_for(activity_id)
+ Enum.map(keys, &@cachex.del(:scrubber_cache, &1))
+ @cachex.del(:scrubber_management_cache, activity_id)
+ end
+
def get_cached_scrubbed_html_for_activity(
content,
scrubbers,
@@ -19,6 +53,8 @@ defmodule Pleroma.Activity.HTML do
@cachex.fetch!(:scrubber_cache, key, fn _key ->
object = Object.normalize(activity, fetch: false)
+
+ add_cache_key_for(activity.id, key)
HTML.ensure_scrubbed_html(content, scrubbers, object.data["fake"] || false, callback)
end)
end
diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex
@@ -189,6 +189,7 @@ defmodule Pleroma.Application do
build_cachex("object", default_ttl: 25_000, ttl_interval: 1000, limit: 2500),
build_cachex("rich_media", default_ttl: :timer.minutes(120), limit: 5000),
build_cachex("scrubber", limit: 2500),
+ build_cachex("scrubber_management", limit: 2500),
build_cachex("idempotency", expiration: idempotency_expiration(), limit: 2500),
build_cachex("web_resp", limit: 2500),
build_cachex("emoji_packs", expiration: emoji_packs_expiration(), limit: 10),
diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex
@@ -455,7 +455,8 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
%{data: %{"type" => "Update", "object" => updated_object}} = object,
meta
) do
- orig_object = Object.get_by_ap_id(updated_object["id"])
+ orig_object_ap_id = updated_object["id"]
+ orig_object = Object.get_by_ap_id(orig_object_ap_id)
orig_object_data = orig_object.data
if orig_object_data["type"] in @updatable_object_types do
@@ -468,15 +469,21 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
|> Object.maybe_update_history(orig_object_data, updated)
|> maybe_update_poll(updated_object)
- orig_object
- |> Repo.preload(:hashtags)
- |> Object.change(%{data: updated_object_data})
- |> Object.update_and_set_cache()
-
- if updated do
- object
- |> Activity.normalize()
- |> ActivityPub.notify_and_stream()
+ changeset =
+ orig_object
+ |> Repo.preload(:hashtags)
+ |> Object.change(%{data: updated_object_data})
+
+ with {:ok, new_object} <- Repo.update(changeset),
+ {:ok, _} <- Object.invalid_object_cache(new_object),
+ {:ok, _} <- Object.set_cache(new_object),
+ # The metadata/utils.ex uses the object id for the cache.
+ {:ok, _} <- Pleroma.Activity.HTML.invalidate_cache_for(new_object.id) do
+ if updated do
+ object
+ |> Activity.normalize()
+ |> ActivityPub.notify_and_stream()
+ end
end
end
diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex
@@ -272,6 +272,16 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do
reply_to_user = reply_to && CommonAPI.get_user(reply_to.data["actor"])
+ history_len =
+ 1 +
+ (Object.history_for(object.data)
+ |> Map.get("orderedItems")
+ |> length())
+
+ # See render("history.json", ...) for more details
+ # Here the implicit index of the current content is 0
+ chrono_order = history_len - 1
+
content =
object
|> render_content()
@@ -281,14 +291,14 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do
|> Activity.HTML.get_cached_scrubbed_html_for_activity(
User.html_filter_policy(opts[:for]),
activity,
- "mastoapi:content"
+ "mastoapi:content:#{chrono_order}"
)
content_plaintext =
content
|> Activity.HTML.get_cached_stripped_html_for_activity(
activity,
- "mastoapi:content"
+ "mastoapi:content:#{chrono_order}"
)
summary = object.data["summary"] || ""
@@ -410,9 +420,25 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do
history = [object | past_history]
+ history_len = length(history)
+
+ history =
+ Enum.with_index(
+ history,
+ fn object, index ->
+ %{
+ # The history is prepended every time there is a new edit.
+ # In chrono_order, the oldest item is always at 0, and so on.
+ # The chrono_order is an invariant kept between edits.
+ chrono_order: history_len - 1 - index,
+ object: object
+ }
+ end
+ )
+
individual_opts =
opts
- |> Map.put(:as, :object)
+ |> Map.put(:as, :item)
|> Map.put(:user, user)
|> Map.put(:hashtags, hashtags)
@@ -421,7 +447,12 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do
def render(
"history_item.json",
- %{activity: activity, user: user, object: object, hashtags: hashtags} = opts
+ %{
+ activity: activity,
+ user: user,
+ item: %{object: object, chrono_order: chrono_order},
+ hashtags: hashtags
+ } = opts
) do
sensitive = object.data["sensitive"] || Enum.member?(hashtags, "nsfw")
@@ -439,7 +470,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do
|> Activity.HTML.get_cached_scrubbed_html_for_activity(
User.html_filter_policy(opts[:for]),
activity,
- "mastoapi:content"
+ "mastoapi:content:#{chrono_order}"
)
summary = object.data["summary"] || ""
diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs
@@ -2061,9 +2061,15 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
oauth_access(["write:statuses"])
end
- test "it updates the status", %{conn: conn, user: user} do
+ test "it updates the status" do
+ %{conn: conn, user: user} = oauth_access(["write:statuses", "read:statuses"])
+
{:ok, activity} = CommonAPI.post(user, %{status: "mew mew #abc", spoiler_text: "#def"})
+ conn
+ |> get("/api/v1/statuses/#{activity.id}")
+ |> json_response_and_validate_schema(200)
+
response =
conn
|> put_req_header("content-type", "application/json")
@@ -2075,6 +2081,14 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
assert response["content"] == "edited"
assert response["spoiler_text"] == "lol"
+
+ response =
+ conn
+ |> get("/api/v1/statuses/#{activity.id}")
+ |> json_response_and_validate_schema(200)
+
+ assert response["content"] == "edited"
+ assert response["spoiler_text"] == "lol"
end
test "it updates the attachments", %{conn: conn, user: user} do
diff --git a/test/pleroma/web/metadata/utils_test.exs b/test/pleroma/web/metadata/utils_test.exs
@@ -3,7 +3,7 @@
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.Metadata.UtilsTest do
- use Pleroma.DataCase, async: true
+ use Pleroma.DataCase, async: false
import Pleroma.Factory
alias Pleroma.Web.Metadata.Utils
@@ -22,6 +22,20 @@ defmodule Pleroma.Web.Metadata.UtilsTest do
assert Utils.scrub_html_and_truncate(note) == "Pleroma's really cool!"
end
+
+ test "it does not return old content after editing" do
+ user = insert(:user)
+
+ {:ok, activity} = Pleroma.Web.CommonAPI.post(user, %{status: "mew mew #def"})
+
+ object = Pleroma.Object.normalize(activity)
+ assert Utils.scrub_html_and_truncate(object) == "mew mew #def"
+
+ {:ok, update} = Pleroma.Web.CommonAPI.update(user, activity, %{status: "mew mew #abc"})
+ update = Pleroma.Activity.normalize(update)
+ object = Pleroma.Object.normalize(update)
+ assert Utils.scrub_html_and_truncate(object) == "mew mew #abc"
+ end
end
describe "scrub_html_and_truncate/2" do
