Merge remote-tracking branch 'parent/main' into upstream-20241021

spec/lib/account_reach_finder_spec.rb

@@ -38,16 +38,23 @@ RSpec.describe AccountReachFinder do
   end
 
   describe '#inboxes' do
-    it 'includes the preferred inbox URL of followers' do
-      expect(described_class.new(account).inboxes).to include(*[ap_follower_example_com, ap_follower_example_org, ap_follower_with_shared].map(&:preferred_inbox_url))
+    subject { described_class.new(account).inboxes }
+
+    it 'includes the preferred inbox URL of followers and recently mentioned accounts but not unrelated users' do
+      expect(subject)
+        .to include(*follower_inbox_urls)
+        .and include(*mentioned_account_inbox_urls)
+        .and not_include(unrelated_account.preferred_inbox_url)
     end
 
-    it 'includes the preferred inbox URL of recently-mentioned accounts' do
-      expect(described_class.new(account).inboxes).to include(*[ap_mentioned_with_shared, ap_mentioned_example_com, ap_mentioned_example_org].map(&:preferred_inbox_url))
+    def follower_inbox_urls
+      [ap_follower_example_com, ap_follower_example_org, ap_follower_with_shared]
+        .map(&:preferred_inbox_url)
     end
 
-    it 'does not include the inbox of unrelated users' do
-      expect(described_class.new(account).inboxes).to_not include(unrelated_account.preferred_inbox_url)
+    def mentioned_account_inbox_urls
+      [ap_mentioned_with_shared, ap_mentioned_example_com, ap_mentioned_example_org]
+        .map(&:preferred_inbox_url)
     end
   end
 end

spec/lib/activitypub/activity/block_spec.rb

@@ -3,6 +3,8 @@
 require 'rails_helper'
 
 RSpec.describe ActivityPub::Activity::Block do
+  subject { described_class.new(json, sender) }
+
   let(:sender)    { Fabricate(:account) }
   let(:recipient) { Fabricate(:account) }
 
@@ -16,93 +18,65 @@ RSpec.describe ActivityPub::Activity::Block do
     }.with_indifferent_access
   end
 
-  context 'when the recipient does not follow the sender' do
-    describe '#perform' do
-      subject { described_class.new(json, sender) }
-
-      before do
-        subject.perform
-      end
-
+  describe '#perform' do
+    context 'when the recipient does not follow the sender' do
       it 'creates a block from sender to recipient' do
-        expect(sender.blocking?(recipient)).to be true
+        subject.perform
+
+        expect(sender)
+          .to be_blocking(recipient)
       end
     end
-  end
 
-  context 'when the recipient is already blocked' do
-    before do
-      sender.block!(recipient, uri: 'old')
+    context 'when the recipient is already blocked' do
+      before { sender.block!(recipient, uri: 'old') }
+
+      it 'creates a block from sender to recipient and sets uri to last received block activity' do
+        subject.perform
+
+        expect(sender)
+          .to be_blocking(recipient)
+        expect(sender.block_relationships.find_by(target_account: recipient).uri)
+          .to eq 'foo'
+      end
     end
 
-    describe '#perform' do
-      subject { described_class.new(json, sender) }
+    context 'when the recipient follows the sender' do
+      before { recipient.follow!(sender) }
+
+      it 'creates a block from sender to recipient and ensures recipient not following sender' do
+        subject.perform
+
+        expect(sender)
+          .to be_blocking(recipient)
+        expect(recipient)
+          .to_not be_following(sender)
+      end
+    end
+
+    context 'when a matching undo has been received first' do
+      let(:undo_json) do
+        {
+          '@context': 'https://www.w3.org/ns/activitystreams',
+          id: 'bar',
+          type: 'Undo',
+          actor: ActivityPub::TagManager.instance.uri_for(sender),
+          object: json,
+        }.with_indifferent_access
+      end
 
       before do
+        recipient.follow!(sender)
+        ActivityPub::Activity::Undo.new(undo_json, sender).perform
+      end
+
+      it 'does not create a block from sender to recipient and ensures recipient not following sender' do
         subject.perform
-      end
 
-      it 'creates a block from sender to recipient' do
-        expect(sender.blocking?(recipient)).to be true
-      end
-
-      it 'sets the uri to that of last received block activity' do
-        expect(sender.block_relationships.find_by(target_account: recipient).uri).to eq 'foo'
-      end
-    end
-  end
-
-  context 'when the recipient follows the sender' do
-    before do
-      recipient.follow!(sender)
-    end
-
-    describe '#perform' do
-      subject { described_class.new(json, sender) }
-
-      before do
-        subject.perform
-      end
-
-      it 'creates a block from sender to recipient' do
-        expect(sender.blocking?(recipient)).to be true
-      end
-
-      it 'ensures recipient is not following sender' do
-        expect(recipient.following?(sender)).to be false
-      end
-    end
-  end
-
-  context 'when a matching undo has been received first' do
-    let(:undo_json) do
-      {
-        '@context': 'https://www.w3.org/ns/activitystreams',
-        id: 'bar',
-        type: 'Undo',
-        actor: ActivityPub::TagManager.instance.uri_for(sender),
-        object: json,
-      }.with_indifferent_access
-    end
-
-    before do
-      recipient.follow!(sender)
-      ActivityPub::Activity::Undo.new(undo_json, sender).perform
-    end
-
-    describe '#perform' do
-      subject { described_class.new(json, sender) }
-
-      before do
-        subject.perform
-      end
-
-      it 'does not create a block from sender to recipient' do
-        expect(sender.blocking?(recipient)).to be false
-      end
-
-      it 'ensures recipient is not following sender' do
-        expect(recipient.following?(sender)).to be false
+        expect(sender)
+          .to_not be_blocking(recipient)
+        expect(recipient)
+          .to_not be_following(sender)
       end
     end
   end

spec/lib/plain_text_formatter_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe PlainTextFormatter do
     subject { described_class.new(status.text, status.local?).to_s }
 
     context 'when status is local' do
-      let(:status) { Fabricate(:status, text: '<p>a text by a nerd who uses an HTML tag in text</p>', uri: nil) }
+      let(:status) { Fabricate.build(:status, text: '<p>a text by a nerd who uses an HTML tag in text</p>', uri: nil) }
 
       it 'returns the raw text' do
         expect(subject).to eq '<p>a text by a nerd who uses an HTML tag in text</p>'
@@ -15,10 +15,10 @@ RSpec.describe PlainTextFormatter do
     end
 
     context 'when status is remote' do
-      let(:remote_account) { Fabricate(:account, domain: 'remote.test', username: 'bob', url: 'https://remote.test/') }
+      let(:remote_account) { Fabricate.build(:account, domain: 'remote.test', username: 'bob', url: 'https://remote.test/') }
 
       context 'when text contains inline HTML tags' do
-        let(:status) { Fabricate(:status, account: remote_account, text: '<b>Lorem</b> <em>ipsum</em>') }
+        let(:status) { Fabricate.build(:status, account: remote_account, text: '<b>Lorem</b> <em>ipsum</em>') }
 
         it 'strips the tags' do
           expect(subject).to eq 'Lorem ipsum'
@@ -26,7 +26,7 @@ RSpec.describe PlainTextFormatter do
       end
 
       context 'when text contains <p> tags' do
-        let(:status) { Fabricate(:status, account: remote_account, text: '<p>Lorem</p><p>ipsum</p>') }
+        let(:status) { Fabricate.build(:status, account: remote_account, text: '<p>Lorem</p><p>ipsum</p>') }
 
         it 'inserts a newline' do
           expect(subject).to eq "Lorem\nipsum"
@@ -34,7 +34,7 @@ RSpec.describe PlainTextFormatter do
       end
 
       context 'when text contains a single <br> tag' do
-        let(:status) { Fabricate(:status, account: remote_account, text: 'Lorem<br>ipsum') }
+        let(:status) { Fabricate.build(:status, account: remote_account, text: 'Lorem<br>ipsum') }
 
         it 'inserts a newline' do
           expect(subject).to eq "Lorem\nipsum"
@@ -42,7 +42,7 @@ RSpec.describe PlainTextFormatter do
       end
 
       context 'when text contains consecutive <br> tag' do
-        let(:status) { Fabricate(:status, account: remote_account, text: 'Lorem<br><br><br>ipsum') }
+        let(:status) { Fabricate.build(:status, account: remote_account, text: 'Lorem<br><br><br>ipsum') }
 
         it 'inserts a single newline' do
           expect(subject).to eq "Lorem\nipsum"
@@ -50,7 +50,7 @@ RSpec.describe PlainTextFormatter do
       end
 
       context 'when text contains HTML entity' do
-        let(:status) { Fabricate(:status, account: remote_account, text: 'Lorem &amp; ipsum &#x2764;') }
+        let(:status) { Fabricate.build(:status, account: remote_account, text: 'Lorem &amp; ipsum &#x2764;') }
 
         it 'unescapes the entity' do
           expect(subject).to eq 'Lorem & ipsum ❤'
@@ -58,7 +58,7 @@ RSpec.describe PlainTextFormatter do
       end
 
       context 'when text contains <script> tag' do
-        let(:status) { Fabricate(:status, account: remote_account, text: 'Lorem <script> alert("Booh!") </script>ipsum') }
+        let(:status) { Fabricate.build(:status, account: remote_account, text: 'Lorem <script> alert("Booh!") </script>ipsum') }
 
         it 'strips the tag and its contents' do
           expect(subject).to eq 'Lorem ipsum'
@@ -66,7 +66,7 @@ RSpec.describe PlainTextFormatter do
       end
 
       context 'when text contains an HTML comment tags' do
-        let(:status) { Fabricate(:status, account: remote_account, text: 'Lorem <!-- Booh! -->ipsum') }
+        let(:status) { Fabricate.build(:status, account: remote_account, text: 'Lorem <!-- Booh! -->ipsum') }
 
         it 'strips the comment' do
           expect(subject).to eq 'Lorem ipsum'
@@ -74,7 +74,7 @@ RSpec.describe PlainTextFormatter do
       end
 
       context 'when text contains HTML ruby tags' do
-        let(:status) { Fabricate(:status, account: remote_account, text: '<p>Lorem <ruby>明日 <rp>(</rp><rt>Ashita</rt><rp>)</rp></ruby> ipsum</p>') }
+        let(:status) { Fabricate.build(:status, account: remote_account, text: '<p>Lorem <ruby>明日 <rp>(</rp><rt>Ashita</rt><rp>)</rp></ruby> ipsum</p>') }
 
         it 'strips the comment' do
           expect(subject).to eq 'Lorem 明日 (Ashita) ipsum'

spec/lib/vacuum/access_tokens_vacuum_spec.rb

@@ -14,32 +14,24 @@ RSpec.describe Vacuum::AccessTokensVacuum do
     let!(:expired_access_grant) { Fabricate(:access_grant, expires_in: 59.minutes.to_i, created_at: 1.hour.ago) }
     let!(:active_access_grant) { Fabricate(:access_grant) }
 
-    before do
+    it 'deletes revoked/expired access tokens and revoked/expired grants, but preserves active tokens/grants' do
       subject.perform
-    end
 
-    it 'deletes revoked access tokens' do
-      expect { revoked_access_token.reload }.to raise_error ActiveRecord::RecordNotFound
-    end
+      expect { revoked_access_token.reload }
+        .to raise_error ActiveRecord::RecordNotFound
+      expect { expired_access_token.reload }
+        .to raise_error ActiveRecord::RecordNotFound
 
-    it 'deletes expired access tokens' do
-      expect { expired_access_token.reload }.to raise_error ActiveRecord::RecordNotFound
-    end
+      expect { revoked_access_grant.reload }
+        .to raise_error ActiveRecord::RecordNotFound
+      expect { expired_access_grant.reload }
+        .to raise_error ActiveRecord::RecordNotFound
 
-    it 'deletes revoked access grants' do
-      expect { revoked_access_grant.reload }.to raise_error ActiveRecord::RecordNotFound
-    end
+      expect { active_access_token.reload }
+        .to_not raise_error
 
-    it 'deletes expired access grants' do
-      expect { expired_access_grant.reload }.to raise_error ActiveRecord::RecordNotFound
-    end
-
-    it 'does not delete active access tokens' do
-      expect { active_access_token.reload }.to_not raise_error
-    end
-
-    it 'does not delete active access grants' do
-      expect { active_access_grant.reload }.to_not raise_error
+      expect { active_access_grant.reload }
+        .to_not raise_error
     end
   end
 end

spec/lib/vacuum/backups_vacuum_spec.rb

@@ -11,16 +11,13 @@ RSpec.describe Vacuum::BackupsVacuum do
     let!(:expired_backup) { Fabricate(:backup, created_at: (retention_period + 1.day).ago) }
     let!(:current_backup) { Fabricate(:backup) }
 
-    before do
+    it 'deletes backups past the retention period but preserves those within the period' do
       subject.perform
-    end
 
-    it 'deletes backups past the retention period' do
-      expect { expired_backup.reload }.to raise_error ActiveRecord::RecordNotFound
-    end
-
-    it 'does not delete backups within the retention period' do
-      expect { current_backup.reload }.to_not raise_error
+      expect { expired_backup.reload }
+        .to raise_error ActiveRecord::RecordNotFound
+      expect { current_backup.reload }
+        .to_not raise_error
     end
   end
 end

spec/lib/vacuum/feeds_vacuum_spec.rb

@@ -18,11 +18,11 @@ RSpec.describe Vacuum::FeedsVacuum do
       redis.sadd(feed_key_for(inactive_user, 'reblogs:2'), 3)
       redis.zadd(list_key_for(list), 1, 1)
       redis.zadd(antenna_key_for(antenna), 1, 1)
-
-      subject.perform
     end
 
     it 'clears feeds of inactive users and lists' do
+      subject.perform
+
       expect(redis.zcard(feed_key_for(inactive_user))).to eq 0
       expect(redis.zcard(feed_key_for(active_user))).to eq 1
       expect(redis.exists?(feed_key_for(inactive_user, 'reblogs'))).to be false

spec/lib/vacuum/media_attachments_vacuum_spec.rb

@@ -17,9 +17,9 @@ RSpec.describe Vacuum::MediaAttachmentsVacuum do
     let!(:old_unattached_media) { Fabricate(:media_attachment, account_id: nil, created_at: 10.days.ago) }
     let!(:new_unattached_media) { Fabricate(:media_attachment, account_id: nil, created_at: 1.hour.ago) }
 
-    before { subject.perform }
-
     it 'handles attachments based on metadata details' do
+      subject.perform
+
       expect(old_remote_media.reload.file) # Remote and past retention period
         .to be_blank
       expect(old_local_media.reload.file) # Local and past retention

spec/lib/vacuum/preview_cards_vacuum_spec.rb

@@ -15,24 +15,22 @@ RSpec.describe Vacuum::PreviewCardsVacuum do
     before do
       old_preview_card.statuses << Fabricate(:status)
       new_preview_card.statuses << Fabricate(:status)
+    end
 
+    it 'handles preview card cleanup' do
       subject.perform
-    end
 
-    it 'deletes cache of preview cards last updated before the retention period' do
-      expect(old_preview_card.reload.image).to be_blank
-    end
+      expect(old_preview_card.reload.image) # last updated before retention period
+        .to be_blank
 
-    it 'does not delete cache of preview cards last updated within the retention period' do
-      expect(new_preview_card.reload.image).to_not be_blank
-    end
+      expect(new_preview_card.reload.image) # last updated within the retention period
+        .to_not be_blank
 
-    it 'does not delete attached preview cards' do
-      expect(new_preview_card.reload).to be_persisted
-    end
+      expect(new_preview_card.reload) # Keep attached preview cards
+        .to be_persisted
 
-    it 'does not delete orphaned preview cards in the retention period' do
-      expect(orphaned_preview_card.reload).to be_persisted
+      expect(orphaned_preview_card.reload) # keep orphaned cards in the retention period
+        .to be_persisted
     end
   end
 end

spec/lib/vacuum/statuses_vacuum_spec.rb

@@ -16,24 +16,20 @@ RSpec.describe Vacuum::StatusesVacuum do
     let!(:local_status_old) { Fabricate(:status, created_at: (retention_period + 2.days).ago) }
     let!(:local_status_recent) { Fabricate(:status, created_at: (retention_period - 2.days).ago) }
 
-    before do
+    it 'deletes remote statuses past the retention period and keeps others' do
       subject.perform
-    end
 
-    it 'deletes remote statuses past the retention period' do
-      expect { remote_status_old.reload }.to raise_error ActiveRecord::RecordNotFound
-    end
+      expect { remote_status_old.reload }
+        .to raise_error ActiveRecord::RecordNotFound
 
-    it 'does not delete local statuses past the retention period' do
-      expect { local_status_old.reload }.to_not raise_error
-    end
+      expect { local_status_old.reload }
+        .to_not raise_error
 
-    it 'does not delete remote statuses within the retention period' do
-      expect { remote_status_recent.reload }.to_not raise_error
-    end
+      expect { remote_status_recent.reload }
+        .to_not raise_error
 
-    it 'does not delete local statuses within the retention period' do
-      expect { local_status_recent.reload }.to_not raise_error
+      expect { local_status_recent.reload }
+        .to_not raise_error
     end
   end