Fix handling of inlined featured collections in ActivityPub actor objects (#34789)

app/services/activitypub/fetch_featured_collection_service.rb

@@ -4,13 +4,11 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService
   include JsonLdHelper
 
   def call(account, **options)
-    return if account.featured_collection_url.blank? || account.suspended? || account.local?
+    return if (account.featured_collection_url.blank? && options[:collection].blank?) || account.suspended? || account.local?
 
     @account = account
     @options = options
-    @json    = fetch_resource(@account.featured_collection_url, true, local_follower)
+    @json    = fetch_collection(options[:collection].presence || @account.featured_collection_url)
-
-    return unless supported_context?(@json)
 
     process_items(collection_items(@json))
   end

app/services/activitypub/process_account_service.rb

@@ -57,7 +57,7 @@ class ActivityPub::ProcessAccountService < BaseService
     after_suspension_change! if suspension_changed?
 
     unless @options[:only_key] || @account.suspended?
-      check_featured_collection! if @account.featured_collection_url.present?
+      check_featured_collection! if @json['featured'].present?
       check_featured_tags_collection! if @json['featuredTags'].present?
       check_links! if @account.fields.any?(&:requires_verification?)
     end
@@ -121,7 +121,7 @@ class ActivityPub::ProcessAccountService < BaseService
   end
 
   def set_immediate_attributes!
-    @account.featured_collection_url = @json['featured'] || ''
+    @account.featured_collection_url = valid_collection_uri(@json['featured'])
     @account.display_name            = @json['name'] || ''
     @account.note                    = @json['summary'] || ''
     @account.locked                  = @json['manuallyApprovesFollowers'] || false
@@ -186,7 +186,7 @@ class ActivityPub::ProcessAccountService < BaseService
   end
 
   def check_featured_collection!
-    ActivityPub::SynchronizeFeaturedCollectionWorker.perform_async(@account.id, { 'hashtag' => @json['featuredTags'].blank?, 'request_id' => @options[:request_id] })
+    ActivityPub::SynchronizeFeaturedCollectionWorker.perform_async(@account.id, { 'hashtag' => @json['featuredTags'].blank?, 'collection' => @json['featured'], 'request_id' => @options[:request_id] })
   end
 
   def check_featured_tags_collection!

app/workers/activitypub/synchronize_featured_collection_worker.rb

@@ -6,7 +6,7 @@ class ActivityPub::SynchronizeFeaturedCollectionWorker
   sidekiq_options queue: 'pull', lock: :until_executed, lock_ttl: 1.day.to_i
 
   def perform(account_id, options = {})
-    options = { note: true, hashtag: false }.deep_merge(options.deep_symbolize_keys)
+    options = { note: true, hashtag: false }.deep_merge(options.symbolize_keys)
 
     ActivityPub::FetchFeaturedCollectionService.new.call(Account.find(account_id), **options)
   rescue ActiveRecord::RecordNotFound

spec/services/activitypub/fetch_featured_collection_service_spec.rb

@@ -67,9 +67,12 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService do
       type: 'Collection',
       id: actor.featured_collection_url,
       items: items,
-    }.with_indifferent_access
+    }.deep_stringify_keys
   end
 
+  describe '#call' do
+    subject { described_class.new.call(actor, note: true, hashtag: false) }
+
     shared_examples 'sets pinned posts' do
       before do
         stub_request(:get, 'https://example.com/account/pinned/known').to_return(status: 200, body: Oj.dump(status_json_pinned_known), headers: { 'Content-Type': 'application/activity+json' })
@@ -78,7 +81,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService do
         stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_reachable), headers: { 'Content-Type': 'application/activity+json' })
         stub_request(:get, 'https://example.com/account/collections/featured').to_return(status: 200, body: Oj.dump(featured_with_null), headers: { 'Content-Type': 'application/activity+json' })
 
-      subject.call(actor, note: true, hashtag: false)
+        subject
       end
 
       it 'sets expected posts as pinned posts' do
@@ -91,7 +94,32 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService do
       end
     end
 
-  describe '#call' do
+    context 'when passing the collection via an argument' do
+      subject { described_class.new.call(actor, note: true, hashtag: false, collection: collection_or_uri) }
+
+      context 'when the collection is an URL' do
+        let(:collection_or_uri) { actor.featured_collection_url }
+
+        before do
+          stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
+        end
+
+        it_behaves_like 'sets pinned posts'
+      end
+
+      context 'when the collection is inlined' do
+        let(:collection_or_uri) do
+          {
+            '@context': 'https://www.w3.org/ns/activitystreams',
+            type: 'Collection',
+            items: items,
+          }.deep_stringify_keys
+        end
+
+        it_behaves_like 'sets pinned posts'
+      end
+    end
+
     context 'when the endpoint is a Collection' do
       before do
         stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
@@ -121,7 +149,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService do
 
         before do
           stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_reachable), headers: { 'Content-Type': 'application/activity+json' })
-          subject.call(actor, note: true, hashtag: false)
+          subject
         end
 
         it 'sets expected posts as pinned posts' do
@@ -157,7 +185,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService do
 
         before do
           stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_reachable), headers: { 'Content-Type': 'application/activity+json' })
-          subject.call(actor, note: true, hashtag: false)
+          subject
         end
 
         it 'sets expected posts as pinned posts' do

spec/services/activitypub/process_account_service_spec.rb

@@ -83,6 +83,27 @@ RSpec.describe ActivityPub::ProcessAccountService do
     end
   end
 
+  context 'with inlined feature collection' do
+    let(:payload) do
+      {
+        id: 'https://foo.test',
+        type: 'Actor',
+        inbox: 'https://foo.test/inbox',
+        featured: {
+          type: 'OrderedCollection',
+          orderedItems: ['https://example.com/statuses/1'],
+        },
+      }.deep_stringify_keys
+    end
+
+    it 'queues featured collection synchronization', :aggregate_failures do
+      account = subject.call('alice', 'example.com', payload)
+
+      expect(account.featured_collection_url).to eq ''
+      expect(ActivityPub::SynchronizeFeaturedCollectionWorker).to have_enqueued_sidekiq_job(account.id, { 'hashtag' => true, 'request_id' => anything, 'collection' => payload['featured'] })
+    end
+  end
+
   context 'when account is not suspended' do
     subject { described_class.new.call(account.username, account.domain, payload) }