Resolve unknown tagged collections in remote posts (#38900)

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?

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!

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

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: {
+        tag: [
-          type: 'Mention',
+          {
-          href: 'http://notexisting.dontexistingtld/actor',
+            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
+    it 'ignores unprocessable mentions and tagged collections', :aggregate_failures do
-      stub_request(:get, invalid_mention_json[:tag][:href]).to_raise(HTTP::ConnectionError)
+      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
 

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

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