Improve searching for private toots from URL (#14856)

* Improve searching for private toots from URL

Most of the time, when sharing toots, people use the toot URL rather than
the toot URI, which makes sense since it is the user-facing URL.

In Mastodon's case, the URL and URI are different, and Mastodon does not
have an index on URL, which means searching a private toot by URL is done
with a slow query that will only succeed for very recent toots.

This change gets rid of the slow query, and attempts to guess the URI from
URL instead, as Mastodon's are predictable.

* Add tests

* Only return status with guessed uri if url matches

Co-authored-by: Claire <claire.github-309c@sitedethib.com>
This commit is contained in:
ThibG 2020-12-17 06:51:49 +01:00 committed by GitHub
parent 2032748050
commit b1feb47055
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 108 additions and 1 deletions

View File

@ -34,7 +34,17 @@ class ResolveURLService < BaseService
# It may happen that the resource is a private toot, and thus not fetchable,
# but we can return the toot if we already know about it.
status = Status.find_by(uri: @url) || Status.find_by(url: @url)
scope = Status.where(uri: @url)
# We don't have an index on `url`, so try guessing the `uri` from `url`
parsed_url = Addressable::URI.parse(@url)
parsed_url.path.match(%r{/@(?<username>#{Account::USERNAME_RE})/(?<status_id>[0-9]+)\Z}) do |matched|
parsed_url.path = "/users/#{matched[:username]}/statuses/#{matched[:status_id]}"
scope = scope.or(Status.where(uri: parsed_url.to_s, url: @url))
end
status = scope.first
authorize_with @on_behalf_of, status, :show? unless status.nil?
status
rescue Mastodon::NotPermittedError

View File

@ -15,5 +15,102 @@ describe ResolveURLService, type: :service do
expect(subject.call(url)).to be_nil
end
context 'searching for a remote private status' do
let(:account) { Fabricate(:account) }
let(:poster) { Fabricate(:account, domain: 'example.com') }
let(:url) { 'https://example.com/@foo/42' }
let(:uri) { 'https://example.com/users/foo/statuses/42' }
let!(:status) { Fabricate(:status, url: url, uri: uri, account: poster, visibility: :private) }
before do
stub_request(:get, url).to_return(status: 404) if url.present?
stub_request(:get, uri).to_return(status: 404)
end
context 'when the account follows the poster' do
before do
account.follow!(poster)
end
context 'when the status uses Mastodon-style URLs' do
let(:url) { 'https://example.com/@foo/42' }
let(:uri) { 'https://example.com/users/foo/statuses/42' }
it 'returns status by url' do
expect(subject.call(url, on_behalf_of: account)).to eq(status)
end
it 'returns status by uri' do
expect(subject.call(uri, on_behalf_of: account)).to eq(status)
end
end
context 'when the status uses pleroma-style URLs' do
let(:url) { nil }
let(:uri) { 'https://example.com/objects/0123-456-789-abc-def' }
it 'returns status by uri' do
expect(subject.call(uri, on_behalf_of: account)).to eq(status)
end
end
end
context 'when the account does not follow the poster' do
context 'when the status uses Mastodon-style URLs' do
let(:url) { 'https://example.com/@foo/42' }
let(:uri) { 'https://example.com/users/foo/statuses/42' }
it 'does not return the status by url' do
expect(subject.call(url, on_behalf_of: account)).to be_nil
end
it 'does not return the status by uri' do
expect(subject.call(uri, on_behalf_of: account)).to be_nil
end
end
context 'when the status uses pleroma-style URLs' do
let(:url) { nil }
let(:uri) { 'https://example.com/objects/0123-456-789-abc-def' }
it 'returns status by uri' do
expect(subject.call(uri, on_behalf_of: account)).to be_nil
end
end
end
end
context 'searching for a local private status' do
let(:account) { Fabricate(:account) }
let(:poster) { Fabricate(:account) }
let!(:status) { Fabricate(:status, account: poster, visibility: :private) }
let(:url) { ActivityPub::TagManager.instance.url_for(status) }
let(:uri) { ActivityPub::TagManager.instance.uri_for(status) }
context 'when the account follows the poster' do
before do
account.follow!(poster)
end
it 'returns status by url' do
expect(subject.call(url, on_behalf_of: account)).to eq(status)
end
it 'returns status by uri' do
expect(subject.call(uri, on_behalf_of: account)).to eq(status)
end
end
context 'when the account does not follow the poster' do
it 'does not return the status by url' do
expect(subject.call(url, on_behalf_of: account)).to be_nil
end
it 'does not return the status by uri' do
expect(subject.call(uri, on_behalf_of: account)).to be_nil
end
end
end
end
end