From 3bc27b9b6441fddc143750e369125ab14cd9ae05 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 5 May 2026 16:46:47 +0200 Subject: [PATCH] Resolve unknown tagged collections in remote posts (#38900) --- app/lib/activitypub/activity/create.rb | 11 +++++ .../process_status_update_service.rb | 18 +++++++- .../tagged_collection_resolve_worker.rb | 24 ++++++++++ spec/lib/activitypub/activity/create_spec.rb | 25 ++++++++--- .../process_status_update_service_spec.rb | 4 ++ .../tagged_collection_resolve_worker_spec.rb | 44 +++++++++++++++++++ 6 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 app/workers/tagged_collection_resolve_worker.rb create mode 100644 spec/workers/tagged_collection_resolve_worker_spec.rb diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 3b86d561c4..9d79a963ac 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -47,6 +47,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity @mentions = [] @tagged_objects = [] @unresolved_mentions = [] + @unresolved_collections = [] @silenced_account_ids = [] @params = {} @quote = nil @@ -68,6 +69,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity resolve_thread(@status) resolve_unresolved_mentions(@status) + resolve_unresolved_collections(@status) fetch_replies(@status) fetch_and_verify_quote distribute @@ -286,8 +288,11 @@ class ActivityPub::Activity::Create < ActivityPub::Activity # TODO: We probably want to resolve unknown objects and push them to an `@unresolved_tagged_objects` on failure collection = ActivityPub::TagManager.instance.uri_to_resource(tag['id'], Collection) + collection ||= ActivityPub::FetchRemoteFeaturedCollectionService.new.call(tag['id'], request_id: @options[:request_id]) @tagged_objects << TaggedObject.new(uri: ActivityPub::TagManager.instance.uri_for(collection), object: collection, ap_type: 'FeaturedCollection') if collection.present? + rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS + @unresolved_collections << tag['id'] end def process_attachments @@ -378,6 +383,12 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end end + def resolve_unresolved_collections(status) + @unresolved_collections.uniq.each do |uri| + TaggedCollectionResolveWorker.perform_in(rand(PROCESSING_DELAY), status.id, uri, { 'request_id' => @options[:request_id] }) + end + end + def fetch_replies(status) collection = @object['replies'] return if collection.blank? diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index a76c49ca08..954e5f0de0 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -237,11 +237,22 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end def update_tagged_objects! + unresolved_tagged_objects = [] + current_tagged_objects = @raw_tagged_objects.filter_map do |tagged_object| url = tagged_object['id'] # TODO: We probably want to resolve unknown objects at authoring time - ActivityPub::TagManager.instance.uri_to_resource(url, Collection) + collection = ActivityPub::TagManager.instance.uri_to_resource(url, Collection) + collection ||= ActivityPub::FetchRemoteFeaturedCollectionService.new.call(url, request_id: @request_id) + + collection + rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS + # Since previous tagged objects are about already-known collections, + # they don't try to resolve again and won't fall into this case. + # In other words, this failure case is only for new collections and can safely be retried later + unresolved_tagged_objects << url + nil end # Any previously-unresolved URI would be resolved here @@ -252,6 +263,11 @@ class ActivityPub::ProcessStatusUpdateService < BaseService # Remove unused links @status.tagged_objects.where.not(uri: current_tagged_objects.map { |object| ActivityPub::TagManager.instance.uri_for(object) }).delete_all + + # Queue unresolved collections for later + unresolved_tagged_objects.uniq.each do |uri| + TaggedCollectionResolveWorker.perform_in(rand(PROCESSING_DELAY), @status.id, uri, { 'request_id' => @request_id }) + end end def update_mentions! diff --git a/app/workers/tagged_collection_resolve_worker.rb b/app/workers/tagged_collection_resolve_worker.rb new file mode 100644 index 0000000000..5bbfe2f248 --- /dev/null +++ b/app/workers/tagged_collection_resolve_worker.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class TaggedCollectionResolveWorker + include Sidekiq::Worker + include ExponentialBackoff + include JsonLdHelper + + sidekiq_options queue: 'pull', retry: 7 + + def perform(status_id, uri, options = {}) + status = Status.find_by(id: status_id) + return if status.nil? + + collection = ActivityPub::TagManager.instance.uri_to_resource(uri, Collection) + collection ||= ActivityPub::FetchRemoteFeaturedCollectionService.new.call(uri, request_id: options['request_id']) + return if collection.nil? + + status.tagged_objects.upsert({ ap_type: 'FeaturedCollection', object_id: collection.id, object_type: 'Collection' }, unique_by: %w(status_id object_type object_id)) + rescue Mastodon::UnexpectedResponseError => e + response = e.response + + raise(e) unless response_error_unsalvageable?(response) + end +end diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index f21552caf5..8dc6d9d425 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -88,10 +88,16 @@ RSpec.describe ActivityPub::Activity::Create do content: '@bob lorem ipsum', published: 1.hour.ago.utc.iso8601, updated: 1.hour.ago.utc.iso8601, - tag: { - type: 'Mention', - href: 'http://notexisting.dontexistingtld/actor', - }, + tag: [ + { + type: 'Mention', + href: 'http://notexisting.dontexistingtld/actor', + }, + { + type: 'FeaturedCollection', + id: 'http://notexisting.notexistingtld/collection', + }, + ], } end @@ -150,8 +156,10 @@ RSpec.describe ActivityPub::Activity::Create do expect(Notification.count).to eq 2 end - it 'ignores unprocessable mention', :aggregate_failures do - stub_request(:get, invalid_mention_json[:tag][:href]).to_raise(HTTP::ConnectionError) + it 'ignores unprocessable mentions and tagged collections', :aggregate_failures do + stub_request(:get, invalid_mention_json[:tag][0][:href]).to_raise(HTTP::ConnectionError) + stub_request(:get, invalid_mention_json[:tag][1][:id]).to_raise(HTTP::ConnectionError) + # When receiving the post that contains an invalid mention… described_class.new(activity_for_object(invalid_mention_json), sender, delivery: true).perform @@ -166,7 +174,10 @@ RSpec.describe ActivityPub::Activity::Create do expect(status.nil?).to be false # It has queued a mention resolve job - expect(MentionResolveWorker).to have_enqueued_sidekiq_job(status.id, invalid_mention_json[:tag][:href], anything) + expect(MentionResolveWorker).to have_enqueued_sidekiq_job(status.id, invalid_mention_json[:tag][0][:href], anything) + + # It has queued a collection resolve job + expect(TaggedCollectionResolveWorker).to have_enqueued_sidekiq_job(status.id, invalid_mention_json[:tag][1][:id], anything) end end diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 5df86eed54..593471f1c7 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -7,6 +7,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do let!(:status) { Fabricate(:status, text: 'Hello world', uri: 'https://example.com/statuses/1234', account: Fabricate(:account, domain: 'example.com')) } let(:bogus_mention) { 'https://example.com/users/erroringuser' } + let(:bogus_collection) { 'https://example.com/collections/erroringcollection' } let(:payload) do { '@context': 'https://www.w3.org/ns/activitystreams', @@ -21,6 +22,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do { type: 'Mention', href: ActivityPub::TagManager.instance.uri_for(alice) }, { type: 'Mention', href: bogus_mention }, { type: 'FeaturedCollection', id: ActivityPub::TagManager.instance.uri_for(featured_collection) }, + { type: 'FeaturedCollection', id: bogus_collection }, ], } end @@ -39,6 +41,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do tags.each { |t| status.tags << t } media_attachments.each { |m| status.media_attachments << m } stub_request(:get, bogus_mention).to_raise(HTTP::ConnectionError) + stub_request(:get, bogus_collection).to_raise(HTTP::ConnectionError) end describe '#call' do @@ -50,6 +53,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do spoiler_text: eq('Show more') ) expect(MentionResolveWorker).to have_enqueued_sidekiq_job(status.id, bogus_mention, anything) + expect(TaggedCollectionResolveWorker).to have_enqueued_sidekiq_job(status.id, bogus_collection, anything) end context 'when the changes are only in sanitized-out HTML' do diff --git a/spec/workers/tagged_collection_resolve_worker_spec.rb b/spec/workers/tagged_collection_resolve_worker_spec.rb new file mode 100644 index 0000000000..ee5c193ec5 --- /dev/null +++ b/spec/workers/tagged_collection_resolve_worker_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe TaggedCollectionResolveWorker do + let(:status_id) { -42 } + let(:uri) { 'https://example.com/collections/unknown' } + + describe '#perform' do + subject { described_class.new.perform(status_id, uri, {}) } + + context 'with a non-existent status' do + it 'returns nil' do + expect(subject).to be_nil + end + end + + context 'with a valid user' do + let(:status) { Fabricate(:status) } + let(:status_id) { status.id } + + let(:service_double) { instance_double(ActivityPub::FetchRemoteFeaturedCollectionService) } + + before do + allow(ActivityPub::FetchRemoteFeaturedCollectionService).to receive(:new).and_return(service_double) + + allow(service_double) + .to receive(:call) + .with(uri, anything) do + Fabricate(:remote_collection, account: Fabricate.build(:account, domain: 'example.com'), uri: uri) + end + end + + it 'resolves the collection and adds a new tagged object', :aggregate_failures do + expect { subject } + .to change { status.reload.tagged_objects } + .from([]) + .to(a_collection_including(having_attributes(object: having_attributes(uri: uri)))) + + expect(service_double).to have_received(:call).once + end + end + end +end