This commit is contained in:
Claire 2024-11-26 11:02:49 +00:00 committed by GitHub
commit 563824074c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 89 additions and 24 deletions

View File

@ -30,7 +30,12 @@ class ActivityPub::RepliesController < ActivityPub::BaseController
end
def set_replies
@replies = only_other_accounts? ? Status.where.not(account_id: @account.id).joins(:account).merge(Account.without_suspended) : @account.statuses
if only_other_accounts?
@replies = Status.where.not(account_id: @account.id).joins(:account).merge(Account.without_suspended)
@replies = @replies.joins("LEFT JOIN blocks ON blocks.account_id = #{@account.id} AND blocks.target_account_id = statuses.account_id").where(blocks: { id: nil })
else
@replies = @account.statuses
end
@replies = @replies.distributable_visibility.where(in_reply_to_id: @status.id)
@replies = @replies.paginate_by_min_id(DESCENDANTS_LIMIT, params[:min_id])
end

View File

@ -390,6 +390,10 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
!replied_to_status.nil? && replied_to_status.account.local?
end
def reply_blocked?
!replied_to_status.account.blocking?(@account) && !replied_to_status.account.domain_blocking?(@account.domain)
end
def related_to_local_activity?
fetch? || followed_by_local_accounts? || requested_through_relay? ||
responds_to_followed_account? || addresses_local_accounts?
@ -406,7 +410,9 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
return false if local_usernames.empty?
Account.local.exists?(username: local_usernames)
scope = Account.local.where(Account.arel_table[:username].lower.in(local_usernames.map(&:downcase)))
scope = scope.where.not('EXISTS (SELECT 1 FROM blocks WHERE account_id = accounts.id AND target_account_id = ?)', @account.id)
scope.exists?
end
def tombstone_exists?
@ -414,7 +420,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
end
def forward_for_reply
return unless @status.distributable? && @json['signature'].present? && reply_to_local?
return unless @status.distributable? && @json['signature'].present? && reply_to_local? && !reply_blocked?
ActivityPub::RawDistributionWorker.perform_async(Oj.dump(@json), replied_to_status.account_id, [@account.preferred_inbox_url])
end

View File

@ -73,15 +73,30 @@ module Status::ThreadingConcern
limit += 1 if limit.present?
descendants_with_self = Status.find_by_sql([<<-SQL.squish, id: id, limit: limit, depth: depth])
WITH RECURSIVE search_tree(id, path) AS (
SELECT id, ARRAY[id]
FROM statuses
WHERE id = :id
WITH RECURSIVE search_tree(id, path, authors) AS (
SELECT
id, ARRAY[id], ARRAY[account_id]
FROM
statuses
WHERE
id = :id
UNION ALL
SELECT statuses.id, path || statuses.id
FROM search_tree
JOIN statuses ON statuses.in_reply_to_id = search_tree.id
WHERE COALESCE(array_length(path, 1) < :depth, TRUE) AND NOT statuses.id = ANY(path)
SELECT
statuses.id, path || statuses.id, (CASE
WHEN array_length(authors, 1) >= 30 THEN authors
WHEN statuses.account_id = ANY(authors) THEN authors
ELSE authors || statuses.account_id
END)
FROM
search_tree
JOIN
statuses ON statuses.in_reply_to_id = search_tree.id
WHERE
COALESCE(array_length(path, 1) < :depth, TRUE) AND NOT statuses.id = ANY(path)
AND NOT EXISTS (SELECT 1 FROM blocks WHERE target_account_id = statuses.account_id AND account_id = any(authors))
AND NOT EXISTS (SELECT 1 FROM account_domain_blocks b JOIN accounts a ON a.domain = b.domain WHERE a.id = statuses.account_id AND b.account_id = any(authors))
)
SELECT id
FROM search_tree

View File

@ -8,6 +8,13 @@ RSpec.describe ActivityPub::RepliesController do
let(:remote_reply_id) { 'https://foobar.com/statuses/1234' }
let(:remote_querier) { nil }
let(:bob) { Fabricate(:account) }
let!(:local_bob_reply) { Fabricate(:status, account: bob, thread: status, visibility: :public) }
let!(:local_public_reply) { Fabricate(:status, thread: status, visibility: :public) }
let!(:public_self_reply) { Fabricate(:status, account: status.account, thread: status, visibility: :public) }
let!(:remote_public_reply) { Fabricate(:status, account: remote_account, thread: status, visibility: :public, uri: remote_reply_id) }
shared_examples 'common behavior' do
context 'when status is private' do
let(:parent_visibility) { :private }
@ -82,7 +89,11 @@ RSpec.describe ActivityPub::RepliesController do
first: be_a(Hash).and(
include(
items: be_an(Array)
.and(have_attributes(size: 1))
.and(contain_exactly(
a_hash_including(
id: ActivityPub::TagManager.instance.uri_for(public_self_reply)
)
))
.and(all(satisfy { |item| targets_public_collection?(item) }))
)
)
@ -123,11 +134,35 @@ RSpec.describe ActivityPub::RepliesController do
context 'with only_other_accounts' do
let(:only_other_accounts) { 'true' }
context 'when blocking some of the repliers' do
before do
status.account.block!(bob)
status.account.block!(remote_public_reply.account)
end
it "does not list the blocked user's replies" do
expect(response.parsed_body)
.to include(
first: be_a(Hash).and(
include(items:
contain_exactly(
a_hash_including(id: ActivityPub::TagManager.instance.uri_for(local_public_reply))
))
)
)
end
end
it 'returns items with other public or unlisted replies' do
expect(response.parsed_body)
.to include(
first: be_a(Hash).and(
include(items: be_an(Array).and(have_attributes(size: 3)))
include(items:
contain_exactly(
a_hash_including(id: ActivityPub::TagManager.instance.uri_for(local_bob_reply)),
a_hash_including(id: ActivityPub::TagManager.instance.uri_for(local_public_reply)),
ActivityPub::TagManager.instance.uri_for(remote_public_reply)
))
)
)
end
@ -178,13 +213,8 @@ RSpec.describe ActivityPub::RepliesController do
stub_const 'ActivityPub::RepliesController::DESCENDANTS_LIMIT', 5
allow(controller).to receive(:signed_request_actor).and_return(remote_querier)
Fabricate(:status, thread: status, visibility: :public)
Fabricate(:status, thread: status, visibility: :public)
Fabricate(:status, thread: status, visibility: :private)
Fabricate(:status, account: status.account, thread: status, visibility: :public)
Fabricate(:status, account: status.account, thread: status, visibility: :private)
Fabricate(:status, account: remote_account, thread: status, visibility: :public, uri: remote_reply_id)
end
describe 'GET #index' do

View File

@ -24,12 +24,12 @@ RSpec.describe Status::ThreadingConcern do
expect(reply_to_second_reply.ancestors(4, viewer)).to_not include(reply_to_status, status)
end
it 'does not return conversation history from blocked users' do
it 'does not return conversation history from users blocked by the viewer' do
viewer.block!(jeff)
expect(reply_to_second_reply.ancestors(4, viewer)).to_not include(reply_to_status)
end
it 'does not return conversation history from muted users' do
it 'does not return conversation history from users muted by the viewer' do
viewer.mute!(jeff)
expect(reply_to_second_reply.ancestors(4, viewer)).to_not include(reply_to_status)
end
@ -39,7 +39,7 @@ RSpec.describe Status::ThreadingConcern do
expect(reply_to_second_reply.ancestors(4, viewer)).to_not include(reply_to_status)
end
it 'does not return conversation history from blocked domains' do
it 'does not return conversation history from domains blocked by the viewer' do
viewer.block_domain!('example.com')
expect(reply_to_second_reply.ancestors(4, viewer)).to_not include(reply_to_first_reply)
end
@ -82,10 +82,13 @@ RSpec.describe Status::ThreadingConcern do
let!(:alice) { Fabricate(:account, username: 'alice') }
let!(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com') }
let!(:jeff) { Fabricate(:account, username: 'jeff') }
let!(:jack) { Fabricate(:account, username: 'jack') }
let!(:status) { Fabricate(:status, account: alice) }
let!(:reply_to_status_from_alice) { Fabricate(:status, thread: status, account: alice) }
let!(:reply_to_status_from_bob) { Fabricate(:status, thread: status, account: bob) }
let!(:reply_to_alice_reply_from_jeff) { Fabricate(:status, thread: reply_to_status_from_alice, account: jeff) }
let!(:reply_to_alice_from_jack) { Fabricate(:status, thread: status, account: jack) }
let!(:reply_to_jack_from_bob) { Fabricate(:status, thread: reply_to_alice_from_jack, account: bob) }
let!(:viewer) { Fabricate(:account, username: 'viewer') }
it 'returns replies' do
@ -99,12 +102,18 @@ RSpec.describe Status::ThreadingConcern do
expect(status.descendants(4, viewer)).to_not include(reply_to_status_from_alice, reply_to_alice_reply_from_jeff)
end
it 'does not return replies from blocked users' do
it 'does not return replies from users blocked by the viewer' do
viewer.block!(jeff)
expect(status.descendants(4, viewer)).to_not include(reply_to_alice_reply_from_jeff)
end
it 'does not return replies from muted users' do
it 'does not return subthreads from users blocked by the author' do
alice.block!(jack)
expect(status.descendants(50, viewer)).to_not include(reply_to_alice_from_jack)
expect(status.descendants(50, viewer)).to_not include(reply_to_jack_from_bob)
end
it 'does not return replies from users muted by the viewer' do
viewer.mute!(jeff)
expect(status.descendants(4, viewer)).to_not include(reply_to_alice_reply_from_jeff)
end
@ -114,7 +123,7 @@ RSpec.describe Status::ThreadingConcern do
expect(status.descendants(4, viewer)).to_not include(reply_to_alice_reply_from_jeff)
end
it 'does not return replies from blocked domains' do
it 'does not return replies from domains blocked by the viewer' do
viewer.block_domain!('example.com')
expect(status.descendants(4, viewer)).to_not include(reply_to_status_from_bob)
end