From 115ab2869b2742f0cc68116a8c03359d220fd608 Mon Sep 17 00:00:00 2001 From: Partho Ghosh Date: Tue, 3 Jan 2023 17:12:48 -0800 Subject: Fix ・ detection in hashtag regex to construct hashtag correctly (#22888) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix ・ detection in hashtag regex to construct hashtag correctly * Fixed rubocop liniting issues * More rubocop linting fix --- spec/models/tag_spec.rb | 57 +++++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 25 deletions(-) (limited to 'spec') diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index b16f99a79..102d2f625 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -1,21 +1,22 @@ +# frozen_string_literal: true require 'rails_helper' -RSpec.describe Tag, type: :model do +RSpec.describe Tag do describe 'validations' do it 'invalid with #' do - expect(Tag.new(name: '#hello_world')).to_not be_valid + expect(described_class.new(name: '#hello_world')).not_to be_valid end it 'invalid with .' do - expect(Tag.new(name: '.abcdef123')).to_not be_valid + expect(described_class.new(name: '.abcdef123')).not_to be_valid end it 'invalid with spaces' do - expect(Tag.new(name: 'hello world')).to_not be_valid + expect(described_class.new(name: 'hello world')).not_to be_valid end it 'valid with ａｅｓｔｈｅｔｉｃ' do - expect(Tag.new(name: 'ａｅｓｔｈｅｔｉｃ')).to be_valid + expect(described_class.new(name: 'ａｅｓｔｈｅｔｉｃ')).to be_valid end end @@ -62,6 +63,10 @@ RSpec.describe Tag, type: :model do expect(subject.match('hello #one·two·three').to_s).to eq ' #one·two·three' end + it 'matches ・unicode in ぼっち・ざ・ろっく correctly' do + expect(subject.match('testing #ぼっち・ざ・ろっく').to_s).to eq ' #ぼっち・ざ・ろっく' + end + it 'matches ZWNJ' do expect(subject.match('just add #نرم‌افزار and').to_s).to eq ' #نرم‌افزار' end @@ -89,44 +94,46 @@ RSpec.describe Tag, type: :model do describe '.find_normalized' do it 'returns tag for a multibyte case-insensitive name' do upcase_string = 'abcABCａｂｃＡＢＣやゆよ' - downcase_string = 'abcabcａｂｃａｂｃやゆよ'; + downcase_string = 'abcabcａｂｃａｂｃやゆよ' tag = Fabricate(:tag, name: HashtagNormalizer.new.normalize(downcase_string)) - expect(Tag.find_normalized(upcase_string)).to eq tag + expect(described_class.find_normalized(upcase_string)).to eq tag end end describe '.matches_name' do it 'returns tags for multibyte case-insensitive names' do upcase_string = 'abcABCａｂｃＡＢＣやゆよ' - downcase_string = 'abcabcａｂｃａｂｃやゆよ'; + downcase_string = 'abcabcａｂｃａｂｃやゆよ' tag = Fabricate(:tag, name: HashtagNormalizer.new.normalize(downcase_string)) - expect(Tag.matches_name(upcase_string)).to eq [tag] + expect(described_class.matches_name(upcase_string)).to eq [tag] end it 'uses the LIKE operator' do - expect(Tag.matches_name('100%abc').to_sql).to eq %q[SELECT "tags".* FROM "tags" WHERE LOWER("tags"."name") LIKE LOWER('100abc%')] + result = %q[SELECT "tags".* FROM "tags" WHERE LOWER("tags"."name") LIKE LOWER('100abc%')] + expect(described_class.matches_name('100%abc').to_sql).to eq result end end describe '.matching_name' do it 'returns tags for multibyte case-insensitive names' do upcase_string = 'abcABCａｂｃＡＢＣやゆよ' - downcase_string = 'abcabcａｂｃａｂｃやゆよ'; + downcase_string = 'abcabcａｂｃａｂｃやゆよ' tag = Fabricate(:tag, name: HashtagNormalizer.new.normalize(downcase_string)) - expect(Tag.matching_name(upcase_string)).to eq [tag] + expect(described_class.matching_name(upcase_string)).to eq [tag] end end describe '.find_or_create_by_names' do + let(:upcase_string) { 'abcABCａｂｃＡＢＣやゆよ' } + let(:downcase_string) { 'abcabcａｂｃａｂｃやゆよ' } + it 'runs a passed block once per tag regardless of duplicates' do - upcase_string = 'abcABCａｂｃＡＢＣやゆよ' - downcase_string = 'abcabcａｂｃａｂｃやゆよ'; - count = 0 + count = 0 - Tag.find_or_create_by_names([upcase_string, downcase_string]) do |tag| + described_class.find_or_create_by_names([upcase_string, downcase_string]) do |_tag| count += 1 end @@ -136,28 +143,28 @@ RSpec.describe Tag, type: :model do describe '.search_for' do it 'finds tag records with matching names' do - tag = Fabricate(:tag, name: "match") - _miss_tag = Fabricate(:tag, name: "miss") + tag = Fabricate(:tag, name: 'match') + _miss_tag = Fabricate(:tag, name: 'miss') - results = Tag.search_for("match") + results = described_class.search_for('match') expect(results).to eq [tag] end it 'finds tag records in case insensitive' do - tag = Fabricate(:tag, name: "MATCH") - _miss_tag = Fabricate(:tag, name: "miss") + tag = Fabricate(:tag, name: 'MATCH') + _miss_tag = Fabricate(:tag, name: 'miss') - results = Tag.search_for("match") + results = described_class.search_for('match') expect(results).to eq [tag] end it 'finds the exact matching tag as the first item' do - similar_tag = Fabricate(:tag, name: "matchlater", reviewed_at: Time.now.utc) - tag = Fabricate(:tag, name: "match", reviewed_at: Time.now.utc) + similar_tag = Fabricate(:tag, name: 'matchlater', reviewed_at: Time.now.utc) + tag = Fabricate(:tag, name: 'match', reviewed_at: Time.now.utc) - results = Tag.search_for("match") + results = described_class.search_for('match') expect(results).to eq [tag, similar_tag] end -- cgit From fdd1facba16db75e425c02807323eb2666688652 Mon Sep 17 00:00:00 2001 From: Jeong Arm Date: Thu, 5 Jan 2023 21:30:38 +0900 Subject: Fix home TL could contain post from who blocked me (#22849) * Fix home tl contains post from who blocked me * Add test * Fix feed_manager's build_crutches blocked_by was not includes status' owner * Add test for status from I blocked * Fix typo --- app/lib/feed_manager.rb | 3 ++- spec/lib/feed_manager_spec.rb | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 510667558..b9c5bc2cd 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -365,6 +365,7 @@ class FeedManager end return true if check_for_blocks.any? { |target_account_id| crutches[:blocking][target_account_id] || crutches[:muting][target_account_id] } + return true if crutches[:blocked_by][status.account_id] if status.reply? && !status.in_reply_to_account_id.nil? # Filter out if it's a reply should_filter = !crutches[:following][status.in_reply_to_account_id] # and I'm not following the person it's a reply to @@ -548,7 +549,7 @@ class FeedManager crutches[:blocking] = Block.where(account_id: receiver_id, target_account_id: check_for_blocks).pluck(:target_account_id).index_with(true) crutches[:muting] = Mute.where(account_id: receiver_id, target_account_id: check_for_blocks).pluck(:target_account_id).index_with(true) crutches[:domain_blocking] = AccountDomainBlock.where(account_id: receiver_id, domain: statuses.flat_map { |s| [s.account.domain, s.reblog&.account&.domain] }.compact).pluck(:domain).index_with(true) - crutches[:blocked_by] = Block.where(target_account_id: receiver_id, account_id: statuses.map { |s| s.reblog&.account_id }.compact).pluck(:account_id).index_with(true) + crutches[:blocked_by] = Block.where(target_account_id: receiver_id, account_id: statuses.map { |s| [s.account_id, s.reblog&.account_id] }.flatten.compact).pluck(:account_id).index_with(true) crutches end diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index 0f3b05e5a..eb55c3983 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -39,6 +39,18 @@ RSpec.describe FeedManager do expect(FeedManager.instance.filter?(:home, reblog, bob)).to be false end + it 'returns true for post from account who blocked me' do + status = Fabricate(:status, text: 'Hello, World', account: alice) + alice.block!(bob) + expect(FeedManager.instance.filter?(:home, status, bob)).to be true + end + + it 'returns true for post from blocked account' do + status = Fabricate(:status, text: 'Hello, World', account: alice) + bob.block!(alice) + expect(FeedManager.instance.filter?(:home, status, bob)).to be true + end + it 'returns true for reblog by followee of blocked account' do status = Fabricate(:status, text: 'Hello world', account: jeff) reblog = Fabricate(:status, reblog: status, account: alice) -- cgit From 18fb01ef7ca3829b07bb8ea101bdd073da72cfbc Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 5 Jan 2023 13:47:21 +0100 Subject: Fix possible race conditions when suspending/unsuspending accounts (#22363) * Fix possible race conditions when suspending/unsuspending accounts * Fix tests Tests were assuming SuspensionWorker and UnsuspensionWorker would do the suspending/unsuspending themselves, but this has changed. --- app/services/suspend_account_service.rb | 9 ++++----- app/services/unsuspend_account_service.rb | 8 +++----- spec/services/suspend_account_service_spec.rb | 6 ++++-- spec/services/unsuspend_account_service_spec.rb | 14 +++++++------- 4 files changed, 18 insertions(+), 19 deletions(-) (limited to 'spec') diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 6856c2c51..211544fea 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -3,10 +3,13 @@ class SuspendAccountService < BaseService include Payloadable + # Carry out the suspension of a recently-suspended account + # @param [Account] account Account to suspend def call(account) + return unless account.suspended? + @account = account - suspend! reject_remote_follows! distribute_update_actor! unmerge_from_home_timelines! @@ -16,10 +19,6 @@ class SuspendAccountService < BaseService private - def suspend! - @account.suspend! unless @account.suspended? - end - def reject_remote_follows! return if @account.local? || !@account.activitypub? diff --git a/app/services/unsuspend_account_service.rb b/app/services/unsuspend_account_service.rb index 534203dce..70667308e 100644 --- a/app/services/unsuspend_account_service.rb +++ b/app/services/unsuspend_account_service.rb @@ -2,10 +2,12 @@ class UnsuspendAccountService < BaseService include Payloadable + + # Restores a recently-unsuspended account + # @param [Account] account Account to restore def call(account) @account = account - unsuspend! refresh_remote_account! return if @account.nil? || @account.suspended? @@ -18,10 +20,6 @@ class UnsuspendAccountService < BaseService private - def unsuspend! - @account.unsuspend! if @account.suspended? - end - def refresh_remote_account! return if @account.local? diff --git a/spec/services/suspend_account_service_spec.rb b/spec/services/suspend_account_service_spec.rb index 5d45e4ffd..126b13986 100644 --- a/spec/services/suspend_account_service_spec.rb +++ b/spec/services/suspend_account_service_spec.rb @@ -13,6 +13,8 @@ RSpec.describe SuspendAccountService, type: :service do local_follower.follow!(account) list.accounts << account + + account.suspend! end it "unmerges from local followers' feeds" do @@ -21,8 +23,8 @@ RSpec.describe SuspendAccountService, type: :service do expect(FeedManager.instance).to have_received(:unmerge_from_list).with(account, list) end - it 'marks account as suspended' do - expect { subject }.to change { account.suspended? }.from(false).to(true) + it 'does not change the “suspended” flag' do + expect { subject }.to_not change { account.suspended? } end end diff --git a/spec/services/unsuspend_account_service_spec.rb b/spec/services/unsuspend_account_service_spec.rb index 3ac4cc085..987eb09e2 100644 --- a/spec/services/unsuspend_account_service_spec.rb +++ b/spec/services/unsuspend_account_service_spec.rb @@ -14,7 +14,7 @@ RSpec.describe UnsuspendAccountService, type: :service do local_follower.follow!(account) list.accounts << account - account.suspend!(origin: :local) + account.unsuspend! end end @@ -30,8 +30,8 @@ RSpec.describe UnsuspendAccountService, type: :service do stub_request(:post, 'https://bob.com/inbox').to_return(status: 201) end - it 'marks account as unsuspended' do - expect { subject }.to change { account.suspended? }.from(true).to(false) + it 'does not change the “suspended” flag' do + expect { subject }.to_not change { account.suspended? } end include_examples 'common behavior' do @@ -83,8 +83,8 @@ RSpec.describe UnsuspendAccountService, type: :service do expect(FeedManager.instance).to have_received(:merge_into_list).with(account, list) end - it 'marks account as unsuspended' do - expect { subject }.to change { account.suspended? }.from(true).to(false) + it 'does not change the “suspended” flag' do + expect { subject }.to_not change { account.suspended? } end end @@ -107,8 +107,8 @@ RSpec.describe UnsuspendAccountService, type: :service do expect(FeedManager.instance).to_not have_received(:merge_into_list).with(account, list) end - it 'does not mark the account as unsuspended' do - expect { subject }.not_to change { account.suspended? } + it 'marks account as suspended' do + expect { subject }.to change { account.suspended? }.from(false).to(true) end end -- cgit