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

Resolve remote accounts when mentioned even if they are already known #5539

Merged
merged 1 commit into from
Nov 7, 2017

Conversation

ClearlyClaire
Copy link
Contributor

If there is one place where the remote account information needs to be up-to-date (at least for the public key and protocol information), it's when delivering toots (especially direct messages).

Without this change, Mastodon mentioning an user never triggers account resolving (unless the remote user is not known at all), which is particularly annoying when dealing with OStatus → ActivityPub migration, as Direct Messages are disabled for OStatus.

This change triggers resolving (still capped to one resolving per account per day in ResolveRemoteAccountService) when mentioning remote users, which may increase traffic a bit.

@ClearlyClaire
Copy link
Contributor Author

Let's not merge this right now, I have something to figure out…

@ClearlyClaire
Copy link
Contributor Author

Fixed a blocking issue in #5543, so this pull request is effectively based on that last one.

This commit reduces the risk of not having up-to-date public key or protocol
information for a remote account, which is required to deliver toots
(especially direct messages).
@nightpool
Copy link
Member

Adding this traffic to all mentions is an incredibly large performance hit. I think it's more critical to fix #5535 so we don't silently and unexpectedly break direct messages with ostatus users.

if as a stop-gap we want to scope this pull request to only re-fetch for direct/private messages mentioning ostatus users, that would be fine. I think an equally useful stopgap is to always deliver to mentioned users regardless of visibility.

Copy link
Member

@nightpool nightpool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either scope this so it's only for ostatus users and only on private/DM messages, or open a new pull request to always deliver to explicitly mentioned accounts (modulo gargron's opinion)

@ClearlyClaire
Copy link
Contributor Author

It is going to be a performance hit, indeed, but it is still capped at max one resolve per remote account per day, so I do not expect it to have such a huge performance impact.

However, you're right, for the intended purpose (OStatus → ActivityPub switch), it can be restricted to private/direct messages to OStatus users. I'm going to change it accordingly.

@nightpool
Copy link
Member

Oh, I thought this would have been on the other side of the webfinger_update_due check. In that case I guess it's probably fine?

@ClearlyClaire
Copy link
Contributor Author

Thanks! As for #5535 I have a very crude idea in mind: not processing the mention at all if we know we are not going to deliver it. It's not great design, but it's much better than the current situation: at least a user would know that something's up is the mention is not processed (no link, full handle).

@Gargron Gargron merged commit 7bea153 into mastodon:master Nov 7, 2017
@ClearlyClaire ClearlyClaire deleted the update-account-on-mention branch November 11, 2017 15:38
cobodo pushed a commit to cobodo/mastodon that referenced this pull request Dec 6, 2017
…mastodon#5539)

This commit reduces the risk of not having up-to-date public key or protocol
information for a remote account, which is required to deliver toots
(especially direct messages).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants