sam/pleroma
sam
/
pleroma
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Object: remove context_id field 

30 to 70% of the objects in the object table are simple JSON objects
containing a single field, 'id', being the context's ID. The reason for
the creation of an object per context seems to be an old relic from the
StatusNet era, and has only been used nowadays as an helper for threads
in Pleroma-FE via the `pleroma.conversation_id` field in status views.
An object per context was created, and its numerical ID (table column)
was used and stored as 'context_id' in the object and activity along
with the full 'context' URI/string.

This commit removes this field and stops creation of objects for each
context, which will also allow incoming activities to use activity IDs
as contexts, something which was not possible before, or would have been
very broken under most circumstances.

The `pleroma.conversation_id` field has been reimplemented in a way to
maintain backwards-compatibility by calculating a CRC32 of the full
context URI/string in the object, instead of relying on the row ID for
the created context object.
This commit is contained in:
Hélène 2022-08-06 03:24:31 +02:00
parent 7de21ec991
commit f3e061c964
No known key found for this signature in database
GPG Key ID: A215F2E9F1589D62
14 changed files with 9 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
lib/pleroma/object.ex
View File

@ -208,10 +208,6 @@ def get_cached_by_ap_id(ap_id) do
end
end
def context_mapping(context) do
Object.change(%Object{}, %{data: %{"id" => context}})
end
def make_tombstone(%Object{data: %{"id" => id, "type" => type}}, deleted \\ DateTime.utc_now()) do
%ObjectTombstone{
id: id,

2
lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex
View File

@ -94,7 +94,7 @@ def changeset(struct, data) do
defp validate_data(data_cng) do
data_cng
|> validate_inclusion(:type, ["Article", "Note", "Page"])
|> validate_required([:id, :actor, :attributedTo, :type, :context, :context_id])
|> validate_required([:id, :actor, :attributedTo, :type, :context])
|> CommonValidations.validate_any_presence([:cc, :to])
|> CommonValidations.validate_fields_match([:actor, :attributedTo])
|> CommonValidations.validate_actor_presence()

4
lib/pleroma/web/activity_pub/object_validators/common_fixes.ex
View File

@ -22,14 +22,12 @@ def cast_and_filter_recipients(message, field, follower_collection, field_fallba
end
def fix_object_defaults(data) do
%{data: %{"id" => context}, id: context_id} =
Utils.create_context(data["context"] || data["conversation"])
context = Utils.maybe_create_context(data["context"] || data["conversation"])
%User{follower_address: follower_collection} = User.get_cached_by_ap_id(data["attributedTo"])
data
|> Map.put("context", context)
|> Map.put("context_id", context_id)
|> cast_and_filter_recipients("to", follower_collection)
|> cast_and_filter_recipients("cc", follower_collection)
|> cast_and_filter_recipients("bto", follower_collection)

2
lib/pleroma/web/activity_pub/object_validators/event_validator.ex
View File

@ -62,7 +62,7 @@ def changeset(struct, data) do
defp validate_data(data_cng) do
data_cng
|> validate_inclusion(:type, ["Event"])
|> validate_required([:id, :actor, :attributedTo, :type, :context, :context_id])
|> validate_required([:id, :actor, :attributedTo, :type, :context])
|> CommonValidations.validate_any_presence([:cc, :to])
|> CommonValidations.validate_fields_match([:actor, :attributedTo])
|> CommonValidations.validate_actor_presence()

2
lib/pleroma/web/activity_pub/object_validators/question_validator.ex
View File

@ -80,7 +80,7 @@ def changeset(struct, data) do
defp validate_data(data_cng) do
data_cng
|> validate_inclusion(:type, ["Question"])
|> validate_required([:id, :actor, :attributedTo, :type, :context, :context_id])
|> validate_required([:id, :actor, :attributedTo, :type, :context])
|> CommonValidations.validate_any_presence([:cc, :to])
|> CommonValidations.validate_fields_match([:actor, :attributedTo])
|> CommonValidations.validate_actor_presence()

23
lib/pleroma/web/activity_pub/utils.ex
View File

@ -154,22 +154,7 @@ def get_notified_from_object(object) do
Notification.get_notified_from_activity(%Activity{data: object}, false)
end
def create_context(context) do
context = context || generate_id("contexts")
# Ecto has problems accessing the constraint inside the jsonb,
# so we explicitly check for the existed object before insert
object = Object.get_cached_by_ap_id(context)
with true <- is_nil(object),
changeset <- Object.context_mapping(context),
{:ok, inserted_object} <- Repo.insert(changeset) do
inserted_object
else
_ ->
object
end
end
def maybe_create_context(context), do: context || generate_id("contexts")
@doc """
Enqueues an activity for federation if it's local
@ -201,18 +186,16 @@ def lazy_put_activity_defaults(map, true) do
|> Map.put_new("id", "pleroma:fakeid")
|> Map.put_new_lazy("published", &make_date/0)
|> Map.put_new("context", "pleroma:fakecontext")
|> Map.put_new("context_id", -1)
|> lazy_put_object_defaults(true)
end
def lazy_put_activity_defaults(map, _fake?) do
%{data: %{"id" => context}, id: context_id} = create_context(map["context"])
context = maybe_create_context(map["context"])
map
|> Map.put_new_lazy("id", &generate_activity_id/0)
|> Map.put_new_lazy("published", &make_date/0)
|> Map.put_new("context", context)
|> Map.put_new("context_id", context_id)
|> lazy_put_object_defaults(false)
end
@ -226,7 +209,6 @@ defp lazy_put_object_defaults(%{"object" => map} = activity, true)
|> Map.put_new("id", "pleroma:fake_object_id")
|> Map.put_new_lazy("published", &make_date/0)
|> Map.put_new("context", activity["context"])
|> Map.put_new("context_id", activity["context_id"])
|> Map.put_new("fake", true)
%{activity | "object" => object}
@ -239,7 +221,6 @@ defp lazy_put_object_defaults(%{"object" => map} = activity, _)
|> Map.put_new_lazy("id", &generate_object_id/0)
|> Map.put_new_lazy("published", &make_date/0)
|> Map.put_new("context", activity["context"])
|> Map.put_new("context_id", activity["context_id"])
%{activity | "object" => object}
end

29
lib/pleroma/web/common_api/utils.ex
View File

@ -449,35 +449,6 @@ def get_report_statuses(%User{ap_id: actor}, %{status_ids: status_ids})
def get_report_statuses(_, _), do: {:ok, nil}
# DEPRECATED mostly, context objects are now created at insertion time.
def context_to_conversation_id(context) do
with %Object{id: id} <- Object.get_cached_by_ap_id(context) do
id
else
_e ->
changeset = Object.context_mapping(context)
case Repo.insert(changeset) do
{:ok, %{id: id}} ->
id
# This should be solved by an upsert, but it seems ecto
# has problems accessing the constraint inside the jsonb.
{:error, _} ->
Object.get_cached_by_ap_id(context).id
end
end
end
def conversation_id_to_context(id) do
with %Object{data: %{"id" => context}} <- Repo.get(Object, id) do
context
else
_e ->
{:error, dgettext("errors", "No such conversation")}
end
end
def validate_character_limit("" = _full_payload, [] = _attachments) do
{:error, dgettext("errors", "Cannot post an empty status without attachments")}
end

2
lib/pleroma/web/mastodon_api/views/status_view.ex
View File

@ -61,7 +61,7 @@ defp get_context_id(%{data: %{"context_id" => context_id}}) when not is_nil(cont
do: context_id
defp get_context_id(%{data: %{"context" => context}}) when is_binary(context),
do: Utils.context_to_conversation_id(context)
do: :erlang.crc32(context)
defp get_context_id(_), do: nil

5
test/pleroma/web/activity_pub/activity_pub_test.exs
View File

@ -554,7 +554,6 @@ test "inserts a given map into the activity database, giving it an id if it has
assert activity.data["ok"] == data["ok"]
assert activity.data["id"] == given_id
assert activity.data["context"] == "blabla"
assert activity.data["context_id"]
end
test "adds a context when none is there" do
@ -576,8 +575,6 @@ test "adds a context when none is there" do
assert is_binary(activity.data["context"])
assert is_binary(object.data["context"])
assert activity.data["context_id"]
assert object.data["context_id"]
end
test "adds an id to a given object if it lacks one and is a note and inserts it to the object database" do
@ -1612,7 +1609,7 @@ test "it can create a Flag activity",
})
assert Repo.aggregate(Activity, :count, :id) == 1
assert Repo.aggregate(Object, :count, :id) == 2
assert Repo.aggregate(Object, :count, :id) == 1
assert Repo.aggregate(Notification, :count, :id) == 0
end
end

3
test/pleroma/web/activity_pub/transmogrifier/question_handling_test.exs
View File

@ -33,8 +33,6 @@ test "Mastodon Question activity" do
assert object.data["context"] ==
"tag:mastodon.sdf.org,2019-05-10:objectId=15095122:objectType=Conversation"
assert object.data["context_id"]
assert object.data["anyOf"] == []
assert Enum.sort(object.data["oneOf"]) ==
@ -68,7 +66,6 @@ test "Mastodon Question activity" do
reply_object = Object.normalize(reply_activity, fetch: false)
assert reply_object.data["context"] == object.data["context"]
assert reply_object.data["context_id"] == object.data["context_id"]
end
test "Mastodon Question activity with HTML tags in plaintext" do

2
test/pleroma/web/activity_pub/transmogrifier_test.exs
View File

@ -227,7 +227,6 @@ test "it strips internal fields" do
assert is_nil(modified["object"]["like_count"])
assert is_nil(modified["object"]["announcements"])
assert is_nil(modified["object"]["announcement_count"])
assert is_nil(modified["object"]["context_id"])
assert is_nil(modified["object"]["generator"])
end
@ -242,7 +241,6 @@ test "it strips internal fields of article" do
assert is_nil(modified["object"]["like_count"])
assert is_nil(modified["object"]["announcements"])
assert is_nil(modified["object"]["announcement_count"])
assert is_nil(modified["object"]["context_id"])
assert is_nil(modified["object"]["likes"])
end

4
test/pleroma/web/activity_pub/utils_test.exs
View File

@ -429,7 +429,6 @@ test "returns map with id and published data" do
object = Object.normalize(note_activity, fetch: false)
res = Utils.lazy_put_activity_defaults(%{"context" => object.data["id"]})
assert res["context"] == object.data["id"]
assert res["context_id"] == object.id
assert res["id"]
assert res["published"]
end
@ -437,7 +436,6 @@ test "returns map with id and published data" do
test "returns map with fake id and published data" do
assert %{
"context" => "pleroma:fakecontext",
"context_id" => -1,
"id" => "pleroma:fakeid",
"published" => _
} = Utils.lazy_put_activity_defaults(%{}, true)
@ -454,13 +452,11 @@ test "returns activity data with object" do
})
assert res["context"] == object.data["id"]
assert res["context_id"] == object.id
assert res["id"]
assert res["published"]
assert res["object"]["id"]
assert res["object"]["published"]
assert res["object"]["context"] == object.data["id"]
assert res["object"]["context_id"] == object.id
end
end

27
test/pleroma/web/common_api/utils_test.exs
View File

@ -273,22 +273,6 @@ test "delegated renderers" do
end
end
describe "context_to_conversation_id" do
test "creates a mapping object" do
conversation_id = Utils.context_to_conversation_id("random context")
object = Object.get_by_ap_id("random context")
assert conversation_id == object.id
end
test "returns an existing mapping for an existing object" do
{:ok, object} = Object.context_mapping("random context") |> Repo.insert()
conversation_id = Utils.context_to_conversation_id("random context")
assert conversation_id == object.id
end
end
describe "formats date to asctime" do
test "when date is in ISO 8601 format" do
date = DateTime.utc_now() |> DateTime.to_iso8601()
@ -517,17 +501,6 @@ test "returns empty string when date invalid" do
end
end
describe "conversation_id_to_context/1" do
test "returns id" do
object = insert(:note)
assert Utils.conversation_id_to_context(object.id) == object.data["id"]
end
test "returns error if object not found" do
assert Utils.conversation_id_to_context("123") == {:error, "No such conversation"}
end
end
describe "maybe_notify_mentioned_recipients/2" do
test "returns recipients when activity is not `Create`" do
activity = insert(:like_activity)

2
test/pleroma/web/mastodon_api/views/status_view_test.exs
View File

@ -226,7 +226,7 @@ test "a note activity" do
object_data = Object.normalize(note, fetch: false).data
user = User.get_cached_by_ap_id(note.data["actor"])
convo_id = Utils.context_to_conversation_id(object_data["context"])
convo_id = :erlang.crc32(object_data["context"])
status = StatusView.render("show.json", %{activity: note})