-
-
Notifications
You must be signed in to change notification settings - Fork 7.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved remote thread fetching #10106
Improved remote thread fetching #10106
Conversation
6c21123
to
4122cfa
Compare
def replies | ||
ActivityPub::CollectionPresenter.new( | ||
type: :unordered, | ||
items: object.self_replies(5).map(&:uri) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's odd to hardcode limit this to 5. For example, there is this self-replies thread of every "good morning" since January 1st: https://mastodon.technology/@ashfurrow/101341748880807263 And it's a lot of items... But only listing 5 of them seems inaccurate, without a way to indicate that there's more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be better to add something making explicit that the collection is incomplete, indeed.
But for the purposes of this PR, it doesn't need to list all replies, especially not replies that are not immediate replies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find a better attribute than replies
for this, and I can't find an existing Collection
attribute to indicate it's incomplete. But I think it's fine this way, if not completely satisfying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, while I do understand your suggestion of using a paginated Collection
not restricted to a hardcoded limit of 5 self-replies, I think this would make the code much more complex for no real benefit.
Indeed, going with your logic, replies
should not be limited to self-replies either, so we would need to have up to 5 self-replies in the first page, then have the following page include both self-replies and other replies, excluding the 5 self-replies, etc. That would also require a new URL scheme, etc.
The replies
collection is never going to be truly exhaustive anyway, we might exclude muted users, private posts, etc.
This is used for resolving threads downwards. The originating server must add a “replies” attributes with such replies for it to be useful.
8141a7f
to
e5a61d0
Compare
The collection isn't actually paginable yet as it has no id nor a `next` field. This may come in another PR.
e5a61d0
to
b520ca5
Compare
cf16d72
to
ad08e1a
Compare
ad08e1a
to
e03d2de
Compare
* Fetch up to 5 replies when discovering a new remote status This is used for resolving threads downwards. The originating server must add a “replies” attributes with such replies for it to be useful. * Add some tests for ActivityPub::FetchRepliesWorker * Add specs for ActivityPub::FetchRepliesService * Serialize up to 5 public self-replies for ActivityPub notes * Add specs for ActivityPub::NoteSerializer * Move exponential backoff logic to a worker concern * Fetch first page of paginated collections when fetching thread replies * Add specs for paginated collections in replies * Move Note replies serialization to a first CollectionPage The collection isn't actually paginable yet as it has no id nor a `next` field. This may come in another PR. * Use pluck(:uri) instead of map(&:uri) to improve performances * Fix fetching replies when they are in a CollectionPage
* Fetch up to 5 replies when discovering a new remote status This is used for resolving threads downwards. The originating server must add a “replies” attributes with such replies for it to be useful. * Add some tests for ActivityPub::FetchRepliesWorker * Add specs for ActivityPub::FetchRepliesService * Serialize up to 5 public self-replies for ActivityPub notes * Add specs for ActivityPub::NoteSerializer * Move exponential backoff logic to a worker concern * Fetch first page of paginated collections when fetching thread replies * Add specs for paginated collections in replies * Move Note replies serialization to a first CollectionPage The collection isn't actually paginable yet as it has no id nor a `next` field. This may come in another PR. * Use pluck(:uri) instead of map(&:uri) to improve performances * Fix fetching replies when they are in a CollectionPage
So far, Mastodon only fetches ancestors of newly-discovered toots, which means in order to make sure a thread is known by all your followers, you would need to boost the last status of the thread, instead (or in addition to) the first one.
This PR aims to change that by fetching a limited number of same-server toots for every newly-discovered remote toot, and announcing a limited number of public self-replies on toots. This way, provided that a thread is already complete, boosting its first status will cause remote Mastodon servers to fetch its self-replies, and so on, fetching the whole thread.