From 304c0417ed36693cf3f7b3609d1a05c897b1c113 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 23 May 2025 17:01:07 +0200 Subject: [PATCH] Fix handling of inlined `featured` collections in ActivityPub actor objects (#34789) --- .../fetch_featured_collection_service.rb | 6 +- .../activitypub/process_account_service.rb | 6 +- .../synchronize_featured_collection_worker.rb | 2 +- .../fetch_featured_collection_service_spec.rb | 76 +++++++++++++------ .../process_account_service_spec.rb | 21 +++++ 5 files changed, 79 insertions(+), 32 deletions(-) diff --git a/app/services/activitypub/fetch_featured_collection_service.rb b/app/services/activitypub/fetch_featured_collection_service.rb index 25c62f3be6..e1fa560a8a 100644 --- a/app/services/activitypub/fetch_featured_collection_service.rb +++ b/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) - - return unless supported_context?(@json) + @json = fetch_collection(options[:collection].presence || @account.featured_collection_url) process_items(collection_items(@json)) end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index e5c2319728..201f7513b9 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/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! diff --git a/app/workers/activitypub/synchronize_featured_collection_worker.rb b/app/workers/activitypub/synchronize_featured_collection_worker.rb index 7a187d7f53..f2643d6960 100644 --- a/app/workers/activitypub/synchronize_featured_collection_worker.rb +++ b/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 diff --git a/spec/services/activitypub/fetch_featured_collection_service_spec.rb b/spec/services/activitypub/fetch_featured_collection_service_spec.rb index 7ea87922ac..f0002bc388 100644 --- a/spec/services/activitypub/fetch_featured_collection_service_spec.rb +++ b/spec/services/activitypub/fetch_featured_collection_service_spec.rb @@ -67,31 +67,59 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService do type: 'Collection', id: actor.featured_collection_url, items: items, - }.with_indifferent_access - end - - 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' }) - stub_request(:get, 'https://example.com/account/pinned/unknown-inlined').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_inlined), headers: { 'Content-Type': 'application/activity+json' }) - stub_request(:get, 'https://example.com/account/pinned/unknown-unreachable').to_return(status: 404) - 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) - end - - it 'sets expected posts as pinned posts' do - expect(actor.pinned_statuses.pluck(:uri)).to contain_exactly( - 'https://example.com/account/pinned/known', - 'https://example.com/account/pinned/unknown-inlined', - 'https://example.com/account/pinned/unknown-reachable' - ) - expect(actor.pinned_statuses).to_not include(known_status) - end + }.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' }) + stub_request(:get, 'https://example.com/account/pinned/unknown-inlined').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_inlined), headers: { 'Content-Type': 'application/activity+json' }) + stub_request(:get, 'https://example.com/account/pinned/unknown-unreachable').to_return(status: 404) + 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 + end + + it 'sets expected posts as pinned posts' do + expect(actor.pinned_statuses.pluck(:uri)).to contain_exactly( + 'https://example.com/account/pinned/known', + 'https://example.com/account/pinned/unknown-inlined', + 'https://example.com/account/pinned/unknown-reachable' + ) + expect(actor.pinned_statuses).to_not include(known_status) + end + end + + 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 diff --git a/spec/services/activitypub/process_account_service_spec.rb b/spec/services/activitypub/process_account_service_spec.rb index e4a36cf182..eb0ba3524a 100644 --- a/spec/services/activitypub/process_account_service_spec.rb +++ b/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) }