From 39ba777fb9154a5f1beece6ff9540daca90a4700 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 28 Mar 2025 11:35:33 +0100 Subject: [PATCH] Fix follower synchronization mechanism erroneously removing followers from multi-page collections (#34272) --- .../synchronize_followers_service.rb | 3 + .../synchronize_followers_service_spec.rb | 69 +++++++++++++------ 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/app/services/activitypub/synchronize_followers_service.rb b/app/services/activitypub/synchronize_followers_service.rb index a9aab653c..33ecded62 100644 --- a/app/services/activitypub/synchronize_followers_service.rb +++ b/app/services/activitypub/synchronize_followers_service.rb @@ -57,6 +57,9 @@ class ActivityPub::SynchronizeFollowersService < BaseService collection = fetch_collection(collection['first']) if collection['first'].present? return unless collection.is_a?(Hash) + # Abort if we'd have to paginate through more than one page of followers + return if collection['next'].present? + case collection['type'] when 'Collection', 'CollectionPage' as_array(collection['items']) diff --git a/spec/services/activitypub/synchronize_followers_service_spec.rb b/spec/services/activitypub/synchronize_followers_service_spec.rb index f62376ab9..dfccae3e0 100644 --- a/spec/services/activitypub/synchronize_followers_service_spec.rb +++ b/spec/services/activitypub/synchronize_followers_service_spec.rb @@ -29,31 +29,33 @@ RSpec.describe ActivityPub::SynchronizeFollowersService, type: :service do }.with_indifferent_access end + around do |example| + Sidekiq::Testing.fake! do + example.run + Sidekiq::Worker.clear_all + end + end + + before do + alice.follow!(actor) + bob.follow!(actor) + mallory.request_follow!(actor) + end + shared_examples 'synchronizes followers' do before do - alice.follow!(actor) - bob.follow!(actor) - mallory.request_follow!(actor) - - allow(ActivityPub::DeliveryWorker).to receive(:perform_async) - subject.call(actor, collection_uri) end - it 'keeps expected followers' do - expect(alice.following?(actor)).to be true - end - - it 'removes local followers not in the remote list' do - expect(bob.following?(actor)).to be false - end - - it 'converts follow requests to follow relationships when they have been accepted' do - expect(mallory.following?(actor)).to be true - end - - it 'sends an Undo Follow to the actor' do - expect(ActivityPub::DeliveryWorker).to have_received(:perform_async).with(anything, eve.id, actor.inbox_url) + it 'maintains following records and sends Undo Follow to actor' do + expect(alice) + .to be_following(actor) # Keep expected followers + expect(bob) + .to_not be_following(actor) # Remove local followers not in remote list + expect(mallory) + .to be_following(actor) # Convert follow request to follow when accepted + expect(ActivityPub::DeliveryWorker) + .to have_enqueued_sidekiq_job(anything, eve.id, actor.inbox_url) # Send Undo Follow to actor end end @@ -83,7 +85,7 @@ RSpec.describe ActivityPub::SynchronizeFollowersService, type: :service do it_behaves_like 'synchronizes followers' end - context 'when the endpoint is a paginated Collection of actor URIs' do + context 'when the endpoint is a single-page paginated Collection of actor URIs' do let(:payload) do { '@context': 'https://www.w3.org/ns/activitystreams', @@ -103,5 +105,30 @@ RSpec.describe ActivityPub::SynchronizeFollowersService, type: :service do it_behaves_like 'synchronizes followers' end + + context 'when the endpoint is a paginated Collection of actor URIs with a next page' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: collection_uri, + first: { + type: 'CollectionPage', + partOf: collection_uri, + items: items, + next: "#{collection_uri}/page2", + }, + }.with_indifferent_access + end + + before do + stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' }) + end + + it 'does not change followers' do + expect { subject.call(actor, collection_uri) } + .to_not(change { actor.followers.reload.reorder(id: :asc).pluck(:id) }) + end + end end end