Skip to content
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

Merged
merged 11 commits into from Feb 28, 2019

Conversation

@ThibG
Copy link
Collaborator

commented Feb 24, 2019

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.

@ThibG ThibG force-pushed the ThibG:features/thread-fetching branch 25 times, most recently from 6c21123 to 4122cfa Feb 24, 2019
@ThibG ThibG changed the title [WiP] Improved remote thread fetching Improved remote thread fetching Feb 25, 2019
def replies
ActivityPub::CollectionPresenter.new(
type: :unordered,
items: object.self_replies(5).map(&:uri)

This comment has been minimized.

Copy link
@Gargron

Gargron Feb 25, 2019

Member

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.

This comment has been minimized.

Copy link
@ThibG

ThibG Feb 25, 2019

Author Collaborator

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.

This comment has been minimized.

Copy link
@ThibG

ThibG Feb 26, 2019

Author Collaborator

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.

This comment has been minimized.

Copy link
@ThibG

ThibG Feb 28, 2019

Author Collaborator

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.
@ThibG ThibG force-pushed the ThibG:features/thread-fetching branch 4 times, most recently from 8141a7f to e5a61d0 Feb 27, 2019
@ThibG ThibG force-pushed the ThibG:features/thread-fetching branch from e5a61d0 to b520ca5 Feb 28, 2019
@ThibG ThibG force-pushed the ThibG:features/thread-fetching branch from cf16d72 to ad08e1a Feb 28, 2019
@ThibG ThibG force-pushed the ThibG:features/thread-fetching branch from ad08e1a to e03d2de Feb 28, 2019
@Gargron Gargron merged commit 9d3c6f1 into tootsuite:master Feb 28, 2019
11 checks passed
11 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: check-i18n Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-webui Your tests passed on CircleCI!
Details
codeclimate All good!
Details
hiyuki2578 added a commit to ProjectMyosotis/mastodon that referenced this pull request Oct 2, 2019
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.