diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 204899fa39..09b064c9b2 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -505,7 +505,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity url = quote if url.present? - ResolveURLService.new.call(url, on_behalf_of: @account, local_only: true).present? + ActivityPub::TagManager.instance.uri_to_resource(url, Status)&.local? else false end diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index ef3bb8310d..2754299378 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -208,7 +208,7 @@ class ActivityPub::TagManager uri_to_resource(uri, Account) end - def uri_to_resource(uri, klass) + def uri_to_resource(uri, klass, url: false) return if uri.nil? if local_uri?(uri) @@ -221,7 +221,9 @@ class ActivityPub::TagManager elsif OStatus::TagManager.instance.local_id?(uri) klass.find_by(id: OStatus::TagManager.instance.unique_tag_to_local_id(uri, klass.to_s)) else - klass.find_by(uri: uri.split('#').first) + resource = klass.find_by(uri: uri.split('#').first) + resource ||= klass.where('uri != url').find_by(url: uri.split('#').first) if url + resource end rescue ActiveRecord::RecordNotFound nil diff --git a/app/services/process_references_service.rb b/app/services/process_references_service.rb index 40405c6ee7..f18a0dc76e 100644 --- a/app/services/process_references_service.rb +++ b/app/services/process_references_service.rb @@ -108,7 +108,16 @@ class ProcessReferencesService < BaseService end def url_to_status(url) - ResolveURLService.new.call(url, on_behalf_of: @status.account, fetch_remote: @fetch_remote && @no_fetch_urls.exclude?(url)) + status = ActivityPub::TagManager.instance.uri_to_resource(url, Status, url: true) + status ||= ResolveURLService.new.call(url, on_behalf_of: @status.account) if @fetch_remote && @no_fetch_urls.exclude?(url) + referrable?(status) ? status : nil + end + + def referrable?(target_status) + return false if target_status.nil? + return @referrable if defined?(@referrable) + + @referrable = StatusPolicy.new(@status.account, target_status).show? end def quote_status_ids @@ -116,7 +125,7 @@ class ProcessReferencesService < BaseService end def quotable?(target_status) - target_status.account.allow_quote? && (!@status.local? || StatusPolicy.new(@status.account, target_status).quote?) + target_status.account.allow_quote? && StatusPolicy.new(@status.account, target_status).quote? end def add_references diff --git a/app/services/resolve_url_service.rb b/app/services/resolve_url_service.rb index 1194afc368..19a94e77ad 100644 --- a/app/services/resolve_url_service.rb +++ b/app/services/resolve_url_service.rb @@ -6,16 +6,15 @@ class ResolveURLService < BaseService USERNAME_STATUS_RE = %r{/@(?#{Account::USERNAME_RE})/(?[0-9]+)\Z} - def call(url, on_behalf_of: nil, fetch_remote: true, local_only: false) + def call(url, on_behalf_of: nil) @url = url @on_behalf_of = on_behalf_of - @fetch_remote = fetch_remote if local_url? process_local_url - elsif !local_only && fetch_remote && !fetched_resource.nil? + elsif !fetched_resource.nil? process_url - elsif !local_only + else process_url_from_db end end @@ -38,7 +37,7 @@ class ResolveURLService < BaseService return account unless account.nil? end - return unless @on_behalf_of.present? && (!@fetch_remote || [401, 403, 404].include?(fetch_resource_service.response_code)) + return unless @on_behalf_of.present? && [401, 403, 404].include?(fetch_resource_service.response_code) # 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. diff --git a/db/migrate/20231214225249_index_to_statuses_url.rb b/db/migrate/20231214225249_index_to_statuses_url.rb new file mode 100644 index 0000000000..ab3eff41ab --- /dev/null +++ b/db/migrate/20231214225249_index_to_statuses_url.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class IndexToStatusesURL < ActiveRecord::Migration[7.1] + disable_ddl_transaction! + + def up + add_index :statuses, :url, name: :index_statuses_on_url, algorithm: :concurrently, opclass: :text_pattern_ops, where: 'url IS NOT NULL AND url <> uri' + end + + def down + remove_index :statuses, name: :index_statuses_on_url + end +end diff --git a/db/schema.rb b/db/schema.rb index 62e7ed98fb..3fae126a34 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2023_12_12_225737) do +ActiveRecord::Schema[7.1].define(version: 2023_12_14_225249) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1242,6 +1242,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_12_12_225737) do t.index ["quote_of_id", "account_id"], name: "index_statuses_on_quote_of_id_and_account_id" t.index ["reblog_of_id", "account_id"], name: "index_statuses_on_reblog_of_id_and_account_id" t.index ["uri"], name: "index_statuses_on_uri", unique: true, opclass: :text_pattern_ops, where: "(uri IS NOT NULL)" + t.index ["url"], name: "index_statuses_on_url", opclass: :text_pattern_ops, where: "((url IS NOT NULL) AND ((url)::text <> (uri)::text))" end create_table "statuses_tags", primary_key: ["tag_id", "status_id"], force: :cascade do |t| diff --git a/spec/services/process_references_service_spec.rb b/spec/services/process_references_service_spec.rb index 37818b2d45..b4c063788a 100644 --- a/spec/services/process_references_service_spec.rb +++ b/spec/services/process_references_service_spec.rb @@ -241,6 +241,76 @@ RSpec.describe ProcessReferencesService, type: :service do end end end + + context 'when already fetched remote post' do + let(:account) { Fabricate(:account, followers_url: 'http://example.com/followers', domain: 'example.com', uri: 'https://example.com/actor') } + let!(:remote_status) { Fabricate(:status, account: account, uri: 'https://example.com/test_post', url: 'https://example.com/test_post', text: 'Lorem ipsum') } + let(:object_json) do + { + id: 'https://example.com/test_post', + to: ActivityPub::TagManager::COLLECTIONS[:public], + '@context': ActivityPub::TagManager::CONTEXT, + type: 'Note', + actor: account.uri, + attributedTo: account.uri, + content: 'Lorem ipsum', + } + end + let(:text) { 'BT:https://example.com/test_post' } + + shared_examples 'reference once' do |uri, url| + it 'reference it' do + expect(subject.size).to eq 1 + expect(subject[0][1]).to eq 'BT' + + status = Status.find_by(id: subject[0][0]) + expect(status).to_not be_nil + expect(status.id).to eq remote_status.id + expect(status.uri).to eq uri + expect(status.url).to eq url + end + end + + before do + stub_request(:get, 'https://example.com/test_post').to_return(status: 200, body: Oj.dump(object_json), headers: { 'Content-Type' => 'application/activity+json' }) + end + + it_behaves_like 'reference once', 'https://example.com/test_post', 'https://example.com/test_post' + + context 'when uri and url is difference and url is not accessable' do + let(:remote_status) { Fabricate(:status, account: account, uri: 'https://example.com/test_post', url: 'https://example.com/test_post_ohagi', text: 'Lorem ipsum') } + let(:text) { 'BT:https://example.com/test_post_ohagi' } + let(:object_json) do + { + id: 'https://example.com/test_post', + url: 'https://example.com/test_post_ohagi', + to: ActivityPub::TagManager::COLLECTIONS[:public], + '@context': ActivityPub::TagManager::CONTEXT, + type: 'Note', + actor: account.uri, + attributedTo: account.uri, + content: 'Lorem ipsum', + } + end + + before do + stub_request(:get, 'https://example.com/test_post_ohagi').to_return(status: 404) + end + + it_behaves_like 'reference once', 'https://example.com/test_post', 'https://example.com/test_post_ohagi' + + it 'do not request to uri' do + subject + expect(a_request(:get, 'https://example.com/test_post_ohagi')).to_not have_been_made + end + + context 'when url and uri is specified at the same time' do + let(:text) { 'BT:https://example.com/test_post_ohagi BT:https://example.com/test_post' } + + it_behaves_like 'reference once', 'https://example.com/test_post', 'https://example.com/test_post_ohagi' + end + end + end end describe 'editing new status' do