From 7183d9a113f67bb6b0f21bd2b3ba2f0d94b6134c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 10 Apr 2021 11:51:02 +0200 Subject: Change multiple mentions with same username to render with domain (#15718) Fix #15506 --- spec/lib/tag_manager_spec.rb | 36 ------------------------------------ 1 file changed, 36 deletions(-) (limited to 'spec') diff --git a/spec/lib/tag_manager_spec.rb b/spec/lib/tag_manager_spec.rb index e9a7aa934..2230f9710 100644 --- a/spec/lib/tag_manager_spec.rb +++ b/spec/lib/tag_manager_spec.rb @@ -83,40 +83,4 @@ RSpec.describe TagManager do expect(TagManager.instance.local_url?('https://domainn.test/')).to eq false end end - - describe '#same_acct?' do - # The following comparisons MUST be case-insensitive. - - it 'returns true if the needle has a correct username and domain for remote user' do - expect(TagManager.instance.same_acct?('username@domain.test', 'UsErNaMe@DoMaIn.Test')).to eq true - end - - it 'returns false if the needle is missing a domain for remote user' do - expect(TagManager.instance.same_acct?('username@domain.test', 'UsErNaMe')).to eq false - end - - it 'returns false if the needle has an incorrect domain for remote user' do - expect(TagManager.instance.same_acct?('username@domain.test', 'UsErNaMe@incorrect.test')).to eq false - end - - it 'returns false if the needle has an incorrect username for remote user' do - expect(TagManager.instance.same_acct?('username@domain.test', 'incorrect@DoMaIn.test')).to eq false - end - - it 'returns true if the needle has a correct username and domain for local user' do - expect(TagManager.instance.same_acct?('username', 'UsErNaMe@Cb6E6126.nGrOk.Io')).to eq true - end - - it 'returns true if the needle is missing a domain for local user' do - expect(TagManager.instance.same_acct?('username', 'UsErNaMe')).to eq true - end - - it 'returns false if the needle has an incorrect username for local user' do - expect(TagManager.instance.same_acct?('username', 'UsErNaM@Cb6E6126.nGrOk.Io')).to eq false - end - - it 'returns false if the needle has an incorrect domain for local user' do - expect(TagManager.instance.same_acct?('username', 'incorrect@Cb6E6126.nGrOk.Io')).to eq false - end - end end -- cgit From 619fad6cf8078ea997554081febe850404bee73c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 11 Apr 2021 11:22:50 +0200 Subject: Remove spam check and dependency on nilsimsa gem (#16011) --- Gemfile | 1 - Gemfile.lock | 8 -- app/controllers/admin/dashboard_controller.rb | 1 - app/lib/activitypub/activity/create.rb | 5 - app/lib/spam_check.rb | 198 -------------------------- app/models/form/admin_settings.rb | 2 - app/services/process_mentions_service.rb | 5 - app/services/remove_status_service.rb | 5 - app/views/admin/dashboard/index.html.haml | 2 - app/views/admin/settings/edit.html.haml | 3 - config/locales/en.yml | 6 - config/settings.yml | 1 - spec/lib/spam_check_spec.rb | 192 ------------------------- 13 files changed, 429 deletions(-) delete mode 100644 app/lib/spam_check.rb delete mode 100644 spec/lib/spam_check_spec.rb (limited to 'spec') diff --git a/Gemfile b/Gemfile index cb24207ca..1190f2558 100644 --- a/Gemfile +++ b/Gemfile @@ -62,7 +62,6 @@ gem 'idn-ruby', require: 'idn' gem 'kaminari', '~> 1.2' gem 'link_header', '~> 0.0' gem 'mime-types', '~> 3.3.1', require: 'mime/types/columnar' -gem 'nilsimsa', git: 'https://github.com/witgo/nilsimsa', ref: 'fd184883048b922b176939f851338d0a4971a532' gem 'nokogiri', '~> 1.11' gem 'nsa', git: 'https://github.com/Gargron/nsa', ref: 'd1079e0cdafdfed7f9f35478d13b9bdaa65965c0' gem 'oj', '~> 3.11' diff --git a/Gemfile.lock b/Gemfile.lock index 11765a967..8bea31332 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -18,13 +18,6 @@ GIT activerecord (>= 6.1.0) activesupport (>= 6.1.0) -GIT - remote: https://github.com/witgo/nilsimsa - revision: fd184883048b922b176939f851338d0a4971a532 - ref: fd184883048b922b176939f851338d0a4971a532 - specs: - nilsimsa (1.1.2) - GEM remote: https://rubygems.org/ specs: @@ -762,7 +755,6 @@ DEPENDENCIES microformats (~> 4.2) mime-types (~> 3.3.1) net-ldap (~> 0.17) - nilsimsa! nokogiri (~> 1.11) nsa! oj (~> 3.11) diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 4422825ee..c829ed98f 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -35,7 +35,6 @@ module Admin @whitelist_enabled = whitelist_mode? @profile_directory = Setting.profile_directory @timeline_preview = Setting.timeline_preview - @spam_check_enabled = Setting.spam_check_enabled @trends_enabled = Setting.trends end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 0fa306cdd..9f6dd9ce0 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -88,7 +88,6 @@ class ActivityPub::Activity::Create < ActivityPub::Activity resolve_thread(@status) fetch_replies(@status) - check_for_spam distribute(@status) forward_for_reply end @@ -492,10 +491,6 @@ class ActivityPub::Activity::Create < ActivityPub::Activity Tombstone.exists?(uri: object_uri) end - def check_for_spam - SpamCheck.perform(@status) - end - def forward_for_reply return unless @status.distributable? && @json['signature'].present? && reply_to_local? diff --git a/app/lib/spam_check.rb b/app/lib/spam_check.rb deleted file mode 100644 index dcb2db9ca..000000000 --- a/app/lib/spam_check.rb +++ /dev/null @@ -1,198 +0,0 @@ -# frozen_string_literal: true - -class SpamCheck - include Redisable - include ActionView::Helpers::TextHelper - - # Threshold over which two Nilsimsa values are considered - # to refer to the same text - NILSIMSA_COMPARE_THRESHOLD = 95 - - # Nilsimsa doesn't work well on small inputs, so below - # this size, we check only for exact matches with MD5 - NILSIMSA_MIN_SIZE = 10 - - # How long to keep the trail of digests between updates, - # there is no reason to store it forever - EXPIRE_SET_AFTER = 1.week.seconds - - # How many digests to keep in an account's trail. If it's - # too small, spam could rotate around different message templates - MAX_TRAIL_SIZE = 10 - - # How many detected duplicates to allow through before - # considering the message as spam - THRESHOLD = 5 - - def initialize(status) - @account = status.account - @status = status - end - - def skip? - disabled? || already_flagged? || trusted? || no_unsolicited_mentions? || solicited_reply? - end - - def spam? - if insufficient_data? - false - elsif nilsimsa? - digests_over_threshold?('nilsimsa') { |_, other_digest| nilsimsa_compare_value(digest, other_digest) >= NILSIMSA_COMPARE_THRESHOLD } - else - digests_over_threshold?('md5') { |_, other_digest| other_digest == digest } - end - end - - def flag! - auto_report_status! - end - - def remember! - # The scores in sorted sets don't actually have enough bits to hold an exact - # value of our snowflake IDs, so we use it only for its ordering property. To - # get the correct status ID back, we have to save it in the string value - - redis.zadd(redis_key, @status.id, digest_with_algorithm) - redis.zremrangebyrank(redis_key, 0, -(MAX_TRAIL_SIZE + 1)) - redis.expire(redis_key, EXPIRE_SET_AFTER) - end - - def reset! - redis.del(redis_key) - end - - def hashable_text - return @hashable_text if defined?(@hashable_text) - - @hashable_text = @status.text - @hashable_text = remove_mentions(@hashable_text) - @hashable_text = strip_tags(@hashable_text) unless @status.local? - @hashable_text = normalize_unicode(@status.spoiler_text + ' ' + @hashable_text) - @hashable_text = remove_whitespace(@hashable_text) - end - - def insufficient_data? - hashable_text.blank? - end - - def digest - @digest ||= begin - if nilsimsa? - Nilsimsa.new(hashable_text).hexdigest - else - Digest::MD5.hexdigest(hashable_text) - end - end - end - - def digest_with_algorithm - if nilsimsa? - ['nilsimsa', digest, @status.id].join(':') - else - ['md5', digest, @status.id].join(':') - end - end - - class << self - def perform(status) - spam_check = new(status) - - return if spam_check.skip? - - if spam_check.spam? - spam_check.flag! - else - spam_check.remember! - end - end - end - - private - - def disabled? - !Setting.spam_check_enabled - end - - def remove_mentions(text) - return text.gsub(Account::MENTION_RE, '') if @status.local? - - Nokogiri::HTML.fragment(text).tap do |html| - mentions = @status.mentions.map { |mention| ActivityPub::TagManager.instance.url_for(mention.account) } - - html.traverse do |element| - element.unlink if element.name == 'a' && mentions.include?(element['href']) - end - end.to_s - end - - def normalize_unicode(text) - text.unicode_normalize(:nfkc).downcase - end - - def remove_whitespace(text) - text.gsub(/\s+/, ' ').strip - end - - def auto_report_status! - status_ids = Status.where(visibility: %i(public unlisted)).where(id: matching_status_ids).pluck(:id) + [@status.id] if @status.distributable? - ReportService.new.call(Account.representative, @account, status_ids: status_ids, comment: I18n.t('spam_check.spam_detected')) - end - - def already_flagged? - @account.silenced? || @account.targeted_reports.unresolved.where(account_id: -99).exists? - end - - def trusted? - @account.trust_level > Account::TRUST_LEVELS[:untrusted] || (@account.local? && @account.user_staff?) - end - - def no_unsolicited_mentions? - @status.mentions.all? { |mention| mention.silent? || (!@account.local? && !mention.account.local?) || mention.account.following?(@account) } - end - - def solicited_reply? - !@status.thread.nil? && @status.thread.mentions.where(account: @account).exists? - end - - def nilsimsa_compare_value(first, second) - first = [first].pack('H*') - second = [second].pack('H*') - bits = 0 - - 0.upto(31) do |i| - bits += Nilsimsa::POPC[255 & (first[i].ord ^ second[i].ord)].ord - end - - 128 - bits # -128 <= Nilsimsa Compare Value <= 128 - end - - def nilsimsa? - hashable_text.size > NILSIMSA_MIN_SIZE - end - - def other_digests - redis.zrange(redis_key, 0, -1) - end - - def digests_over_threshold?(filter_algorithm) - other_digests.select do |record| - algorithm, other_digest, status_id = record.split(':') - - next unless algorithm == filter_algorithm - - yield algorithm, other_digest, status_id - end.size >= THRESHOLD - end - - def matching_status_ids - if nilsimsa? - other_digests.filter_map { |record| record.split(':')[2] if record.start_with?('nilsimsa') && nilsimsa_compare_value(digest, record.split(':')[1]) >= NILSIMSA_COMPARE_THRESHOLD } - else - other_digests.filter_map { |record| record.split(':')[2] if record.start_with?('md5') && record.split(':')[1] == digest } - end - end - - def redis_key - @redis_key ||= "spam_check:#{@account.id}" - end -end diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index e9f78da21..b5c3dcdbe 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -29,7 +29,6 @@ class Form::AdminSettings thumbnail hero mascot - spam_check_enabled trends trendable_by_default show_domain_blocks @@ -48,7 +47,6 @@ class Form::AdminSettings show_known_fediverse_at_about_page preview_sensitive_media profile_directory - spam_check_enabled trends trendable_by_default noindex diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 4c02c7865..73dbb1834 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -43,7 +43,6 @@ class ProcessMentionsService < BaseService end status.save! - check_for_spam(status) mentions.each { |mention| create_notification(mention) } end @@ -72,8 +71,4 @@ class ProcessMentionsService < BaseService def resolve_account_service ResolveAccountService.new end - - def check_for_spam(status) - SpamCheck.perform(status) - end end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index d6043fb5d..d642beeaa 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -41,7 +41,6 @@ class RemoveStatusService < BaseService remove_from_hashtags remove_from_public remove_from_media if @status.media_attachments.any? - remove_from_spam_check remove_media end @@ -163,10 +162,6 @@ class RemoveStatusService < BaseService @status.media_attachments.destroy_all end - def remove_from_spam_check - redis.zremrangebyscore("spam_check:#{@status.account_id}", @status.id, @status.id) - end - def lock_options { redis: Redis.current, key: "distribute:#{@status.id}" } end diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 205538402..57a753e6b 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -77,8 +77,6 @@ = feature_hint(link_to(t('admin.dashboard.trends'), edit_admin_settings_path), @trends_enabled) %li = feature_hint(link_to(t('admin.dashboard.feature_relay'), admin_relays_path), @relay_enabled) - %li - = feature_hint(link_to(t('admin.dashboard.feature_spam_check'), edit_admin_settings_path), @spam_check_enabled) .dashboard__widgets__versions %div diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index 159bd4b0a..0e705f205 100644 --- a/app/views/admin/settings/edit.html.haml +++ b/app/views/admin/settings/edit.html.haml @@ -92,9 +92,6 @@ .fields-group = f.input :noindex, as: :boolean, wrapper: :with_label, label: t('admin.settings.default_noindex.title'), hint: t('admin.settings.default_noindex.desc_html') - .fields-group - = f.input :spam_check_enabled, as: :boolean, wrapper: :with_label, label: t('admin.settings.spam_check_enabled.title'), hint: t('admin.settings.spam_check_enabled.desc_html') - %hr.spacer/ .fields-group diff --git a/config/locales/en.yml b/config/locales/en.yml index 182a8e985..3387b4df6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -363,7 +363,6 @@ en: feature_profile_directory: Profile directory feature_registrations: Registrations feature_relay: Federation relay - feature_spam_check: Anti-spam feature_timeline_preview: Timeline preview features: Features hidden_service: Federation with hidden services @@ -627,9 +626,6 @@ en: desc_html: You can write your own privacy policy, terms of service or other legalese. You can use HTML tags title: Custom terms of service site_title: Server name - spam_check_enabled: - desc_html: Mastodon can auto-report accounts that send repeated unsolicited messages. There may be false positives. - title: Anti-spam automation thumbnail: desc_html: Used for previews via OpenGraph and API. 1200x630px recommended title: Server thumbnail @@ -1209,8 +1205,6 @@ en: relationships: Follows and followers two_factor_authentication: Two-factor Auth webauthn_authentication: Security keys - spam_check: - spam_detected: This is an automated report. Spam has been detected. statuses: attached: audio: diff --git a/config/settings.yml b/config/settings.yml index 9cf68a096..b79ea620c 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -67,7 +67,6 @@ defaults: &defaults activity_api_enabled: true peers_api_enabled: true show_known_fediverse_at_about_page: true - spam_check_enabled: true show_domain_blocks: 'disabled' show_domain_blocks_rationale: 'disabled' require_invite_text: false diff --git a/spec/lib/spam_check_spec.rb b/spec/lib/spam_check_spec.rb deleted file mode 100644 index 159d83257..000000000 --- a/spec/lib/spam_check_spec.rb +++ /dev/null @@ -1,192 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe SpamCheck do - let!(:sender) { Fabricate(:account) } - let!(:alice) { Fabricate(:account, username: 'alice') } - let!(:bob) { Fabricate(:account, username: 'bob') } - - def status_with_html(text, options = {}) - status = PostStatusService.new.call(sender, { text: text }.merge(options)) - status.update_columns(text: Formatter.instance.format(status), local: false) - status - end - - describe '#hashable_text' do - it 'removes mentions from HTML for remote statuses' do - status = status_with_html('@alice Hello') - expect(described_class.new(status).hashable_text).to eq 'hello' - end - - it 'removes mentions from text for local statuses' do - status = PostStatusService.new.call(alice, text: "Hey @#{sender.username}, how are you?") - expect(described_class.new(status).hashable_text).to eq 'hey , how are you?' - end - end - - describe '#insufficient_data?' do - it 'returns true when there is no text' do - status = status_with_html('@alice') - expect(described_class.new(status).insufficient_data?).to be true - end - - it 'returns false when there is text' do - status = status_with_html('@alice h') - expect(described_class.new(status).insufficient_data?).to be false - end - end - - describe '#digest' do - it 'returns a string' do - status = status_with_html('@alice Hello world') - expect(described_class.new(status).digest).to be_a String - end - end - - describe '#spam?' do - it 'returns false for a unique status' do - status = status_with_html('@alice Hello') - expect(described_class.new(status).spam?).to be false - end - - it 'returns false for different statuses to the same recipient' do - status1 = status_with_html('@alice Hello') - described_class.new(status1).remember! - status2 = status_with_html('@alice Are you available to talk?') - expect(described_class.new(status2).spam?).to be false - end - - it 'returns false for statuses with different content warnings' do - status1 = status_with_html('@alice Are you available to talk?') - described_class.new(status1).remember! - status2 = status_with_html('@alice Are you available to talk?', spoiler_text: 'This is a completely different matter than what I was talking about previously, I swear!') - expect(described_class.new(status2).spam?).to be false - end - - it 'returns false for different statuses to different recipients' do - status1 = status_with_html('@alice How is it going?') - described_class.new(status1).remember! - status2 = status_with_html('@bob Are you okay?') - expect(described_class.new(status2).spam?).to be false - end - - it 'returns false for very short different statuses to different recipients' do - status1 = status_with_html('@alice 🙄') - described_class.new(status1).remember! - status2 = status_with_html('@bob Huh?') - expect(described_class.new(status2).spam?).to be false - end - - it 'returns false for statuses with no text' do - status1 = status_with_html('@alice') - described_class.new(status1).remember! - status2 = status_with_html('@bob') - expect(described_class.new(status2).spam?).to be false - end - - it 'returns true for duplicate statuses to the same recipient' do - described_class::THRESHOLD.times do - status1 = status_with_html('@alice Hello') - described_class.new(status1).remember! - end - - status2 = status_with_html('@alice Hello') - expect(described_class.new(status2).spam?).to be true - end - - it 'returns true for duplicate statuses to different recipients' do - described_class::THRESHOLD.times do - status1 = status_with_html('@alice Hello') - described_class.new(status1).remember! - end - - status2 = status_with_html('@bob Hello') - expect(described_class.new(status2).spam?).to be true - end - - it 'returns true for nearly identical statuses with random numbers' do - source_text = 'Sodium, atomic number 11, was first isolated by Humphry Davy in 1807. A chemical component of salt, he named it Na in honor of the saltiest region on earth, North America.' - - described_class::THRESHOLD.times do - status1 = status_with_html('@alice ' + source_text + ' 1234') - described_class.new(status1).remember! - end - - status2 = status_with_html('@bob ' + source_text + ' 9568') - expect(described_class.new(status2).spam?).to be true - end - end - - describe '#skip?' do - it 'returns true when the sender is already silenced' do - status = status_with_html('@alice Hello') - sender.silence! - expect(described_class.new(status).skip?).to be true - end - - it 'returns true when the mentioned person follows the sender' do - status = status_with_html('@alice Hello') - alice.follow!(sender) - expect(described_class.new(status).skip?).to be true - end - - it 'returns false when even one mentioned person doesn\'t follow the sender' do - status = status_with_html('@alice @bob Hello') - alice.follow!(sender) - expect(described_class.new(status).skip?).to be false - end - - it 'returns true when the sender is replying to a status that mentions the sender' do - parent = PostStatusService.new.call(alice, text: "Hey @#{sender.username}, how are you?") - status = status_with_html('@alice @bob Hello', thread: parent) - expect(described_class.new(status).skip?).to be true - end - end - - describe '#remember!' do - let(:status) { status_with_html('@alice') } - let(:spam_check) { described_class.new(status) } - let(:redis_key) { spam_check.send(:redis_key) } - - it 'remembers' do - expect(Redis.current.exists?(redis_key)).to be true - spam_check.remember! - expect(Redis.current.exists?(redis_key)).to be true - end - end - - describe '#reset!' do - let(:status) { status_with_html('@alice') } - let(:spam_check) { described_class.new(status) } - let(:redis_key) { spam_check.send(:redis_key) } - - before do - spam_check.remember! - end - - it 'resets' do - expect(Redis.current.exists?(redis_key)).to be true - spam_check.reset! - expect(Redis.current.exists?(redis_key)).to be false - end - end - - describe '#flag!' do - let!(:status1) { status_with_html('@alice General Kenobi you are a bold one') } - let!(:status2) { status_with_html('@alice @bob General Kenobi, you are a bold one') } - - before do - described_class.new(status1).remember! - described_class.new(status2).flag! - end - - it 'creates a report about the account' do - expect(sender.targeted_reports.unresolved.count).to eq 1 - end - - it 'attaches both matching statuses to the report' do - expect(sender.targeted_reports.first.status_ids).to include(status1.id, status2.id) - end - end -end -- cgit From f7117646afddb2676e9275d8efe90c3a20c59021 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 12 Apr 2021 12:37:14 +0200 Subject: Add cold-start follow recommendations (#15945) --- .../admin/follow_recommendations_controller.rb | 53 ++++++++++++++++++ app/controllers/api/v1/suggestions_controller.rb | 2 +- app/controllers/api/v2/suggestions_controller.rb | 19 +++++++ app/javascript/mastodon/actions/suggestions.js | 8 +-- .../features/compose/components/search_results.js | 12 ++--- app/javascript/mastodon/reducers/suggestions.js | 8 +-- app/lib/potential_friendship_tracker.rb | 12 +++-- app/models/account.rb | 3 +- app/models/account_suggestions.rb | 17 ++++++ app/models/account_summary.rb | 25 +++++++++ app/models/concerns/account_associations.rb | 3 ++ app/models/follow_recommendation.rb | 39 ++++++++++++++ app/models/follow_recommendation_filter.rb | 26 +++++++++ app/models/follow_recommendation_suppression.rb | 28 ++++++++++ app/models/form/account_batch.rb | 18 +++++++ app/policies/follow_recommendation_policy.rb | 15 ++++++ app/serializers/rest/suggestion_serializer.rb | 7 +++ .../follow_recommendations/_account.html.haml | 20 +++++++ .../admin/follow_recommendations/show.html.haml | 42 +++++++++++++++ .../scheduler/follow_recommendations_scheduler.rb | 61 +++++++++++++++++++++ config/locales/en.yml | 8 +++ config/navigation.rb | 1 + config/routes.rb | 2 + config/sidekiq.yml | 4 ++ .../20210322164601_create_account_summaries.rb | 9 ++++ ...20210323114347_create_follow_recommendations.rb | 5 ++ ...13_create_follow_recommendation_suppressions.rb | 9 ++++ db/schema.rb | 63 +++++++++++++++++++--- db/views/account_summaries_v01.sql | 22 ++++++++ db/views/follow_recommendations_v01.sql | 38 +++++++++++++ ...follow_recommendation_suppression_fabricator.rb | 3 ++ .../follow_recommendation_suppression_spec.rb | 4 ++ 32 files changed, 560 insertions(+), 26 deletions(-) create mode 100644 app/controllers/admin/follow_recommendations_controller.rb create mode 100644 app/controllers/api/v2/suggestions_controller.rb create mode 100644 app/models/account_suggestions.rb create mode 100644 app/models/account_summary.rb create mode 100644 app/models/follow_recommendation.rb create mode 100644 app/models/follow_recommendation_filter.rb create mode 100644 app/models/follow_recommendation_suppression.rb create mode 100644 app/policies/follow_recommendation_policy.rb create mode 100644 app/serializers/rest/suggestion_serializer.rb create mode 100644 app/views/admin/follow_recommendations/_account.html.haml create mode 100644 app/views/admin/follow_recommendations/show.html.haml create mode 100644 app/workers/scheduler/follow_recommendations_scheduler.rb create mode 100644 db/migrate/20210322164601_create_account_summaries.rb create mode 100644 db/migrate/20210323114347_create_follow_recommendations.rb create mode 100644 db/migrate/20210324171613_create_follow_recommendation_suppressions.rb create mode 100644 db/views/account_summaries_v01.sql create mode 100644 db/views/follow_recommendations_v01.sql create mode 100644 spec/fabricators/follow_recommendation_suppression_fabricator.rb create mode 100644 spec/models/follow_recommendation_suppression_spec.rb (limited to 'spec') diff --git a/app/controllers/admin/follow_recommendations_controller.rb b/app/controllers/admin/follow_recommendations_controller.rb new file mode 100644 index 000000000..e3eac62b3 --- /dev/null +++ b/app/controllers/admin/follow_recommendations_controller.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Admin + class FollowRecommendationsController < BaseController + before_action :set_language + + def show + authorize :follow_recommendation, :show? + + @form = Form::AccountBatch.new + @accounts = filtered_follow_recommendations + end + + def update + @form = Form::AccountBatch.new(form_account_batch_params.merge(current_account: current_account, action: action_from_button)) + @form.save + rescue ActionController::ParameterMissing + # Do nothing + ensure + redirect_to admin_follow_recommendations_path(filter_params) + end + + private + + def set_language + @language = follow_recommendation_filter.language + end + + def filtered_follow_recommendations + follow_recommendation_filter.results + end + + def follow_recommendation_filter + @follow_recommendation_filter ||= FollowRecommendationFilter.new(filter_params) + end + + def form_account_batch_params + params.require(:form_account_batch).permit(:action, account_ids: []) + end + + def filter_params + params.slice(*FollowRecommendationFilter::KEYS).permit(*FollowRecommendationFilter::KEYS) + end + + def action_from_button + if params[:suppress] + 'suppress_follow_recommendation' + elsif params[:unsuppress] + 'unsuppress_follow_recommendation' + end + end + end +end diff --git a/app/controllers/api/v1/suggestions_controller.rb b/app/controllers/api/v1/suggestions_controller.rb index 52054160d..b2788cc76 100644 --- a/app/controllers/api/v1/suggestions_controller.rb +++ b/app/controllers/api/v1/suggestions_controller.rb @@ -19,6 +19,6 @@ class Api::V1::SuggestionsController < Api::BaseController private def set_accounts - @accounts = PotentialFriendshipTracker.get(current_account.id, limit: limit_param(DEFAULT_ACCOUNTS_LIMIT)) + @accounts = PotentialFriendshipTracker.get(current_account, limit_param(DEFAULT_ACCOUNTS_LIMIT)) end end diff --git a/app/controllers/api/v2/suggestions_controller.rb b/app/controllers/api/v2/suggestions_controller.rb new file mode 100644 index 000000000..35eb276c0 --- /dev/null +++ b/app/controllers/api/v2/suggestions_controller.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class Api::V2::SuggestionsController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :read } + before_action :require_user! + before_action :set_suggestions + + def index + render json: @suggestions, each_serializer: REST::SuggestionSerializer + end + + private + + def set_suggestions + @suggestions = AccountSuggestions.get(current_account, limit_param(DEFAULT_ACCOUNTS_LIMIT)) + end +end diff --git a/app/javascript/mastodon/actions/suggestions.js b/app/javascript/mastodon/actions/suggestions.js index b15bd916b..0bf959017 100644 --- a/app/javascript/mastodon/actions/suggestions.js +++ b/app/javascript/mastodon/actions/suggestions.js @@ -11,8 +11,8 @@ export function fetchSuggestions() { return (dispatch, getState) => { dispatch(fetchSuggestionsRequest()); - api(getState).get('/api/v1/suggestions').then(response => { - dispatch(importFetchedAccounts(response.data)); + api(getState).get('/api/v2/suggestions').then(response => { + dispatch(importFetchedAccounts(response.data.map(x => x.account))); dispatch(fetchSuggestionsSuccess(response.data)); }).catch(error => dispatch(fetchSuggestionsFail(error))); }; @@ -25,10 +25,10 @@ export function fetchSuggestionsRequest() { }; }; -export function fetchSuggestionsSuccess(accounts) { +export function fetchSuggestionsSuccess(suggestions) { return { type: SUGGESTIONS_FETCH_SUCCESS, - accounts, + suggestions, skipLoading: true, }; }; diff --git a/app/javascript/mastodon/features/compose/components/search_results.js b/app/javascript/mastodon/features/compose/components/search_results.js index 4b4cdff74..c4e160b8a 100644 --- a/app/javascript/mastodon/features/compose/components/search_results.js +++ b/app/javascript/mastodon/features/compose/components/search_results.js @@ -51,13 +51,13 @@ class SearchResults extends ImmutablePureComponent { - {suggestions && suggestions.map(accountId => ( + {suggestions && suggestions.map(suggestion => ( ))} diff --git a/app/javascript/mastodon/reducers/suggestions.js b/app/javascript/mastodon/reducers/suggestions.js index 834be728f..1a6e66ee7 100644 --- a/app/javascript/mastodon/reducers/suggestions.js +++ b/app/javascript/mastodon/reducers/suggestions.js @@ -19,18 +19,18 @@ export default function suggestionsReducer(state = initialState, action) { return state.set('isLoading', true); case SUGGESTIONS_FETCH_SUCCESS: return state.withMutations(map => { - map.set('items', fromJS(action.accounts.map(x => x.id))); + map.set('items', fromJS(action.suggestions.map(x => ({ ...x, account: x.account.id })))); map.set('isLoading', false); }); case SUGGESTIONS_FETCH_FAIL: return state.set('isLoading', false); case SUGGESTIONS_DISMISS: - return state.update('items', list => list.filterNot(id => id === action.id)); + return state.update('items', list => list.filterNot(x => x.account === action.id)); case ACCOUNT_BLOCK_SUCCESS: case ACCOUNT_MUTE_SUCCESS: - return state.update('items', list => list.filterNot(id => id === action.relationship.id)); + return state.update('items', list => list.filterNot(x => x.account === action.relationship.id)); case DOMAIN_BLOCK_SUCCESS: - return state.update('items', list => list.filterNot(id => action.accounts.includes(id))); + return state.update('items', list => list.filterNot(x => action.accounts.includes(x.account))); default: return state; } diff --git a/app/lib/potential_friendship_tracker.rb b/app/lib/potential_friendship_tracker.rb index 188aa4a27..e72d454b6 100644 --- a/app/lib/potential_friendship_tracker.rb +++ b/app/lib/potential_friendship_tracker.rb @@ -28,10 +28,14 @@ class PotentialFriendshipTracker redis.zrem("interactions:#{account_id}", target_account_id) end - def get(account_id, limit: 20, offset: 0) - account_ids = redis.zrevrange("interactions:#{account_id}", offset, limit) - return [] if account_ids.empty? - Account.searchable.where(id: account_ids) + def get(account, limit) + account_ids = redis.zrevrange("interactions:#{account.id}", 0, limit) + + return [] if account_ids.empty? || limit < 1 + + accounts = Account.searchable.where(id: account_ids).index_by(&:id) + + account_ids.map { |id| accounts[id.to_i] }.compact end end end diff --git a/app/models/account.rb b/app/models/account.rb index d85fd1f6e..80689d4aa 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -110,6 +110,7 @@ class Account < ApplicationRecord scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) } scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) } + scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) } scope :tagged_with, ->(tag) { joins(:accounts_tags).where(accounts_tags: { tag_id: tag }) } scope :by_recent_status, -> { order(Arel.sql('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc, accounts.id desc')) } scope :by_recent_sign_in, -> { order(Arel.sql('(case when users.current_sign_in_at is null then 1 else 0 end) asc, users.current_sign_in_at desc, accounts.id desc')) } @@ -363,7 +364,7 @@ class Account < ApplicationRecord end def excluded_from_timeline_account_ids - Rails.cache.fetch("exclude_account_ids_for:#{id}") { blocking.pluck(:target_account_id) + blocked_by.pluck(:account_id) + muting.pluck(:target_account_id) } + Rails.cache.fetch("exclude_account_ids_for:#{id}") { block_relationships.pluck(:target_account_id) + blocked_by_relationships.pluck(:account_id) + mute_relationships.pluck(:target_account_id) } end def excluded_from_timeline_domains diff --git a/app/models/account_suggestions.rb b/app/models/account_suggestions.rb new file mode 100644 index 000000000..7fe9d618e --- /dev/null +++ b/app/models/account_suggestions.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AccountSuggestions + class Suggestion < ActiveModelSerializers::Model + attributes :account, :source + end + + def self.get(account, limit) + suggestions = PotentialFriendshipTracker.get(account, limit).map { |target_account| Suggestion.new(account: target_account, source: :past_interaction) } + suggestions.concat(FollowRecommendation.get(account, limit - suggestions.size, suggestions.map { |suggestion| suggestion.account.id }).map { |target_account| Suggestion.new(account: target_account, source: :global) }) if suggestions.size < limit + suggestions + end + + def self.remove(account, target_account_id) + PotentialFriendshipTracker.remove(account.id, target_account_id) + end +end diff --git a/app/models/account_summary.rb b/app/models/account_summary.rb new file mode 100644 index 000000000..6a7e17c6c --- /dev/null +++ b/app/models/account_summary.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: account_summaries +# +# account_id :bigint(8) primary key +# language :string +# sensitive :boolean +# + +class AccountSummary < ApplicationRecord + self.primary_key = :account_id + + scope :safe, -> { where(sensitive: false) } + scope :localized, ->(locale) { where(language: locale) } + scope :filtered, -> { joins(arel_table.join(FollowRecommendationSuppression.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:account_id].eq(FollowRecommendationSuppression.arel_table[:account_id])).join_sources).where(FollowRecommendationSuppression.arel_table[:id].eq(nil)) } + + def self.refresh + Scenic.database.refresh_materialized_view(table_name, concurrently: true, cascade: false) + end + + def readonly? + true + end +end diff --git a/app/models/concerns/account_associations.rb b/app/models/concerns/account_associations.rb index 98849f8fc..aaf371ebd 100644 --- a/app/models/concerns/account_associations.rb +++ b/app/models/concerns/account_associations.rb @@ -63,5 +63,8 @@ module AccountAssociations # Account deletion requests has_one :deletion_request, class_name: 'AccountDeletionRequest', inverse_of: :account, dependent: :destroy + + # Follow recommendations + has_one :follow_recommendation_suppression, inverse_of: :account, dependent: :destroy end end diff --git a/app/models/follow_recommendation.rb b/app/models/follow_recommendation.rb new file mode 100644 index 000000000..c4355224d --- /dev/null +++ b/app/models/follow_recommendation.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: follow_recommendations +# +# account_id :bigint(8) primary key +# rank :decimal(, ) +# reason :text is an Array +# + +class FollowRecommendation < ApplicationRecord + self.primary_key = :account_id + + belongs_to :account_summary, foreign_key: :account_id + belongs_to :account, foreign_key: :account_id + + scope :safe, -> { joins(:account_summary).merge(AccountSummary.safe) } + scope :localized, ->(locale) { joins(:account_summary).merge(AccountSummary.localized(locale)) } + scope :filtered, -> { joins(:account_summary).merge(AccountSummary.filtered) } + + def readonly? + true + end + + def self.get(account, limit, exclude_account_ids = []) + account_ids = Redis.current.zrevrange("follow_recommendations:#{account.user_locale}", 0, -1).map(&:to_i) - exclude_account_ids - [account.id] + + return [] if account_ids.empty? || limit < 1 + + accounts = Account.followable_by(account) + .not_excluded_by_account(account) + .not_domain_blocked_by_account(account) + .where(id: account_ids) + .limit(limit) + .index_by(&:id) + + account_ids.map { |id| accounts[id] }.compact + end +end diff --git a/app/models/follow_recommendation_filter.rb b/app/models/follow_recommendation_filter.rb new file mode 100644 index 000000000..acf03cd84 --- /dev/null +++ b/app/models/follow_recommendation_filter.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class FollowRecommendationFilter + KEYS = %i( + language + status + ).freeze + + attr_reader :params, :language + + def initialize(params) + @language = params.delete('language') || I18n.locale + @params = params + end + + def results + if params['status'] == 'suppressed' + Account.joins(:follow_recommendation_suppression).order(FollowRecommendationSuppression.arel_table[:id].desc).to_a + else + account_ids = Redis.current.zrevrange("follow_recommendations:#{@language}", 0, -1).map(&:to_i) + accounts = Account.where(id: account_ids).index_by(&:id) + + account_ids.map { |id| accounts[id] }.compact + end + end +end diff --git a/app/models/follow_recommendation_suppression.rb b/app/models/follow_recommendation_suppression.rb new file mode 100644 index 000000000..170506b85 --- /dev/null +++ b/app/models/follow_recommendation_suppression.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: follow_recommendation_suppressions +# +# id :bigint(8) not null, primary key +# account_id :bigint(8) not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class FollowRecommendationSuppression < ApplicationRecord + include Redisable + + belongs_to :account + + after_commit :remove_follow_recommendations, on: :create + + private + + def remove_follow_recommendations + redis.pipelined do + I18n.available_locales.each do |locale| + redis.zrem("follow_recommendations:#{locale}", account_id) + end + end + end +end diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb index 26d6d3abf..698933c9f 100644 --- a/app/models/form/account_batch.rb +++ b/app/models/form/account_batch.rb @@ -21,6 +21,10 @@ class Form::AccountBatch approve! when 'reject' reject! + when 'suppress_follow_recommendation' + suppress_follow_recommendation! + when 'unsuppress_follow_recommendation' + unsuppress_follow_recommendation! end end @@ -79,4 +83,18 @@ class Form::AccountBatch records.each { |account| authorize(account.user, :reject?) } .each { |account| DeleteAccountService.new.call(account, reserve_email: false, reserve_username: false) } end + + def suppress_follow_recommendation! + authorize(:follow_recommendation, :suppress?) + + accounts.each do |account| + FollowRecommendationSuppression.create(account: account) + end + end + + def unsuppress_follow_recommendation! + authorize(:follow_recommendation, :unsuppress?) + + FollowRecommendationSuppression.where(account_id: account_ids).destroy_all + end end diff --git a/app/policies/follow_recommendation_policy.rb b/app/policies/follow_recommendation_policy.rb new file mode 100644 index 000000000..68cd0e547 --- /dev/null +++ b/app/policies/follow_recommendation_policy.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class FollowRecommendationPolicy < ApplicationPolicy + def show? + staff? + end + + def suppress? + staff? + end + + def unsuppress? + staff? + end +end diff --git a/app/serializers/rest/suggestion_serializer.rb b/app/serializers/rest/suggestion_serializer.rb new file mode 100644 index 000000000..3d697fd9f --- /dev/null +++ b/app/serializers/rest/suggestion_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class REST::SuggestionSerializer < ActiveModel::Serializer + attributes :source + + has_one :account, serializer: REST::AccountSerializer +end diff --git a/app/views/admin/follow_recommendations/_account.html.haml b/app/views/admin/follow_recommendations/_account.html.haml new file mode 100644 index 000000000..af5a4aaf7 --- /dev/null +++ b/app/views/admin/follow_recommendations/_account.html.haml @@ -0,0 +1,20 @@ +.batch-table__row + %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox + = f.check_box :account_ids, { multiple: true, include_hidden: false }, account.id + .batch-table__row__content.batch-table__row__content--unpadded + %table.accounts-table + %tbody + %tr + %td= account_link_to account + %td.accounts-table__count.optional + = number_to_human account.statuses_count, strip_insignificant_zeros: true + %small= t('accounts.posts', count: account.statuses_count).downcase + %td.accounts-table__count.optional + = number_to_human account.followers_count, strip_insignificant_zeros: true + %small= t('accounts.followers', count: account.followers_count).downcase + %td.accounts-table__count + - if account.last_status_at.present? + %time.time-ago{ datetime: account.last_status_at.to_date.iso8601, title: l(account.last_status_at.to_date) }= l account.last_status_at + - else + \- + %small= t('accounts.last_active') diff --git a/app/views/admin/follow_recommendations/show.html.haml b/app/views/admin/follow_recommendations/show.html.haml new file mode 100644 index 000000000..1f050329a --- /dev/null +++ b/app/views/admin/follow_recommendations/show.html.haml @@ -0,0 +1,42 @@ +- content_for :page_title do + = t('admin.follow_recommendations.title') + +- content_for :header_tags do + = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous' + +.simple_form + %p.hint= t('admin.follow_recommendations.description_html') + +%hr.spacer/ + += form_tag admin_follow_recommendations_path, method: 'GET', class: 'simple_form' do + .filters + .filter-subset.filter-subset--with-select + %strong= t('admin.follow_recommendations.language') + .input.select.optional + = select_tag :language, options_for_select(I18n.available_locales.map { |key| [human_locale(key), key]}, @language) + + .filter-subset + %strong= t('admin.follow_recommendations.status') + %ul + %li= filter_link_to t('admin.accounts.moderation.active'), status: nil + %li= filter_link_to t('admin.follow_recommendations.suppressed'), status: 'suppressed' + += form_for(@form, url: admin_follow_recommendations_path, method: :patch) do |f| + - RelationshipFilter::KEYS.each do |key| + = hidden_field_tag key, params[key] if params[key].present? + + .batch-table + .batch-table__toolbar + %label.batch-table__toolbar__select.batch-checkbox-all + = check_box_tag :batch_checkbox_all, nil, false + .batch-table__toolbar__actions + - if params[:status].blank? && can?(:suppress, :follow_recommendation) + = f.button safe_join([fa_icon('times'), t('admin.follow_recommendations.suppress')]), name: :suppress, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + - if params[:status] == 'suppressed' && can?(:unsuppress, :follow_recommendation) + = f.button safe_join([fa_icon('plus'), t('admin.follow_recommendations.unsuppress')]), name: :unsuppress, class: 'table-action-link', type: :submit + .batch-table__body + - if @accounts.empty? + = nothing_here 'nothing-here--under-tabs' + - else + = render partial: 'account', collection: @accounts, locals: { f: f } diff --git a/app/workers/scheduler/follow_recommendations_scheduler.rb b/app/workers/scheduler/follow_recommendations_scheduler.rb new file mode 100644 index 000000000..0a0286496 --- /dev/null +++ b/app/workers/scheduler/follow_recommendations_scheduler.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +class Scheduler::FollowRecommendationsScheduler + include Sidekiq::Worker + include Redisable + + sidekiq_options retry: 0 + + # The maximum number of accounts that can be requested in one page from the + # API is 80, and the suggestions API does not allow pagination. This number + # leaves some room for accounts being filtered during live access + SET_SIZE = 100 + + def perform + # Maintaining a materialized view speeds-up subsequent queries significantly + AccountSummary.refresh + + fallback_recommendations = FollowRecommendation.safe.filtered.limit(SET_SIZE).index_by(&:account_id) + + I18n.available_locales.each do |locale| + recommendations = begin + if AccountSummary.safe.filtered.localized(locale).exists? # We can skip the work if no accounts with that language exist + FollowRecommendation.safe.filtered.localized(locale).limit(SET_SIZE).index_by(&:account_id) + else + {} + end + end + + # Use language-agnostic results if there are not enough language-specific ones + missing = SET_SIZE - recommendations.keys.size + + if missing.positive? + added = 0 + + # Avoid duplicate results + fallback_recommendations.each_value do |recommendation| + next if recommendations.key?(recommendation.account_id) + + recommendations[recommendation.account_id] = recommendation + added += 1 + + break if added >= missing + end + end + + redis.pipelined do + redis.del(key(locale)) + + recommendations.each_value do |recommendation| + redis.zadd(key(locale), recommendation.rank, recommendation.account_id) + end + end + end + end + + private + + def key(locale) + "follow_recommendations:#{locale}" + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index 3387b4df6..afab6d9b5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -440,6 +440,14 @@ en: create: Add domain title: Block new e-mail domain title: Blocked e-mail domains + follow_recommendations: + description_html: "Follow recommendations help new users quickly find interesting content. When a user has not interacted with others enough to form personalized follow recommendations, these accounts are recommended instead. They are re-calculated on a daily basis from a mix of accounts with the highest recent engagements and highest local follower counts for a given language." + language: For language + status: Status + suppress: Suppress follow recommendation + suppressed: Suppressed + title: Follow recommendations + unsuppress: Restore follow recommendation instances: by_domain: Domain delivery_available: Delivery is available diff --git a/config/navigation.rb b/config/navigation.rb index 3a82c7971..b3462c48d 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -39,6 +39,7 @@ SimpleNavigation::Configuration.run do |navigation| s.item :accounts, safe_join([fa_icon('users fw'), t('admin.accounts.title')]), admin_accounts_url, highlights_on: %r{/admin/accounts|/admin/pending_accounts} s.item :invites, safe_join([fa_icon('user-plus fw'), t('admin.invites.title')]), admin_invites_path s.item :tags, safe_join([fa_icon('hashtag fw'), t('admin.tags.title')]), admin_tags_path, highlights_on: %r{/admin/tags} + s.item :follow_recommendations, safe_join([fa_icon('user-plus fw'), t('admin.follow_recommendations.title')]), admin_follow_recommendations_path, highlights_on: %r{/admin/follow_recommendations} s.item :instances, safe_join([fa_icon('cloud fw'), t('admin.instances.title')]), admin_instances_url(limited: whitelist_mode? ? nil : '1'), highlights_on: %r{/admin/instances|/admin/domain_blocks|/admin/domain_allows}, if: -> { current_user.admin? } s.item :email_domain_blocks, safe_join([fa_icon('envelope fw'), t('admin.email_domain_blocks.title')]), admin_email_domain_blocks_url, highlights_on: %r{/admin/email_domain_blocks}, if: -> { current_user.admin? } s.item :ip_blocks, safe_join([fa_icon('ban fw'), t('admin.ip_blocks.title')]), admin_ip_blocks_url, highlights_on: %r{/admin/ip_blocks}, if: -> { current_user.admin? } diff --git a/config/routes.rb b/config/routes.rb index eedd0de69..4661a7c11 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -292,6 +292,7 @@ Rails.application.routes.draw do end resources :account_moderation_notes, only: [:create, :destroy] + resource :follow_recommendations, only: [:show, :update] resources :tags, only: [:index, :show, :update] do collection do @@ -507,6 +508,7 @@ Rails.application.routes.draw do namespace :v2 do resources :media, only: [:create] get '/search', to: 'search#index', as: :search + resources :suggestions, only: [:index] end namespace :web do diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 010923717..a8e4c7feb 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -25,6 +25,10 @@ cron: '<%= Random.rand(0..59) %> <%= Random.rand(0..2) %> * * *' class: Scheduler::FeedCleanupScheduler queue: scheduler + follow_recommendations_scheduler: + cron: '<%= Random.rand(0..59) %> <%= Random.rand(6..9) %> * * *' + class: Scheduler::FollowRecommendationsScheduler + queue: scheduler doorkeeper_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(0..2) %> * * 0' class: Scheduler::DoorkeeperCleanupScheduler diff --git a/db/migrate/20210322164601_create_account_summaries.rb b/db/migrate/20210322164601_create_account_summaries.rb new file mode 100644 index 000000000..b9faf180d --- /dev/null +++ b/db/migrate/20210322164601_create_account_summaries.rb @@ -0,0 +1,9 @@ +class CreateAccountSummaries < ActiveRecord::Migration[5.2] + def change + create_view :account_summaries, materialized: true + + # To be able to refresh the view concurrently, + # at least one unique index is required + safety_assured { add_index :account_summaries, :account_id, unique: true } + end +end diff --git a/db/migrate/20210323114347_create_follow_recommendations.rb b/db/migrate/20210323114347_create_follow_recommendations.rb new file mode 100644 index 000000000..77e729032 --- /dev/null +++ b/db/migrate/20210323114347_create_follow_recommendations.rb @@ -0,0 +1,5 @@ +class CreateFollowRecommendations < ActiveRecord::Migration[5.2] + def change + create_view :follow_recommendations + end +end diff --git a/db/migrate/20210324171613_create_follow_recommendation_suppressions.rb b/db/migrate/20210324171613_create_follow_recommendation_suppressions.rb new file mode 100644 index 000000000..c17a0be63 --- /dev/null +++ b/db/migrate/20210324171613_create_follow_recommendation_suppressions.rb @@ -0,0 +1,9 @@ +class CreateFollowRecommendationSuppressions < ActiveRecord::Migration[6.1] + def change + create_table :follow_recommendation_suppressions do |t| + t.references :account, null: false, foreign_key: { on_delete: :cascade }, index: { unique: true } + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4edaf5651..28f36abb1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2,15 +2,15 @@ # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# Note that this schema.rb definition is the authoritative source for your -# database schema. If you need to create the application database on another -# system, you should be using db:schema:load, not running all the migrations -# from scratch. The latter is a flawed and unsustainable approach (the more migrations -# you'll amass, the slower it'll run and the greater likelihood for issues). +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_03_08_133107) do +ActiveRecord::Schema.define(version: 2021_03_24_171613) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -406,6 +406,13 @@ ActiveRecord::Schema.define(version: 2021_03_08_133107) do t.index ["tag_id"], name: "index_featured_tags_on_tag_id" end + create_table "follow_recommendation_suppressions", force: :cascade do |t| + t.bigint "account_id", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["account_id"], name: "index_follow_recommendation_suppressions_on_account_id", unique: true + end + create_table "follow_requests", force: :cascade do |t| t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -996,6 +1003,7 @@ ActiveRecord::Schema.define(version: 2021_03_08_133107) do add_foreign_key "favourites", "statuses", name: "fk_b0e856845e", on_delete: :cascade add_foreign_key "featured_tags", "accounts", on_delete: :cascade add_foreign_key "featured_tags", "tags", on_delete: :cascade + add_foreign_key "follow_recommendation_suppressions", "accounts", on_delete: :cascade add_foreign_key "follow_requests", "accounts", column: "target_account_id", name: "fk_9291ec025d", on_delete: :cascade add_foreign_key "follow_requests", "accounts", name: "fk_76d644b0e7", on_delete: :cascade add_foreign_key "follows", "accounts", column: "target_account_id", name: "fk_745ca29eac", on_delete: :cascade @@ -1079,4 +1087,47 @@ ActiveRecord::Schema.define(version: 2021_03_08_133107) do SQL add_index "instances", ["domain"], name: "index_instances_on_domain", unique: true + create_view "account_summaries", materialized: true, sql_definition: <<-SQL + SELECT accounts.id AS account_id, + mode() WITHIN GROUP (ORDER BY t0.language) AS language, + mode() WITHIN GROUP (ORDER BY t0.sensitive) AS sensitive + FROM (accounts + CROSS JOIN LATERAL ( SELECT statuses.account_id, + statuses.language, + statuses.sensitive + FROM statuses + WHERE ((statuses.account_id = accounts.id) AND (statuses.deleted_at IS NULL)) + ORDER BY statuses.id DESC + LIMIT 20) t0) + WHERE ((accounts.suspended_at IS NULL) AND (accounts.silenced_at IS NULL) AND (accounts.moved_to_account_id IS NULL) AND (accounts.discoverable = true) AND (accounts.locked = false)) + GROUP BY accounts.id; + SQL + add_index "account_summaries", ["account_id"], name: "index_account_summaries_on_account_id", unique: true + + create_view "follow_recommendations", sql_definition: <<-SQL + SELECT t0.account_id, + sum(t0.rank) AS rank, + array_agg(t0.reason) AS reason + FROM ( SELECT accounts.id AS account_id, + ((count(follows.id))::numeric / (1.0 + (count(follows.id))::numeric)) AS rank, + 'most_followed'::text AS reason + FROM ((follows + JOIN accounts ON ((accounts.id = follows.target_account_id))) + JOIN users ON ((users.account_id = follows.account_id))) + WHERE ((users.current_sign_in_at >= (now() - 'P30D'::interval)) AND (accounts.suspended_at IS NULL) AND (accounts.moved_to_account_id IS NULL) AND (accounts.silenced_at IS NULL) AND (accounts.locked = false) AND (accounts.discoverable = true)) + GROUP BY accounts.id + HAVING (count(follows.id) >= 5) + UNION ALL + SELECT accounts.id AS account_id, + (sum((status_stats.reblogs_count + status_stats.favourites_count)) / (1.0 + sum((status_stats.reblogs_count + status_stats.favourites_count)))) AS rank, + 'most_interactions'::text AS reason + FROM ((status_stats + JOIN statuses ON ((statuses.id = status_stats.status_id))) + JOIN accounts ON ((accounts.id = statuses.account_id))) + WHERE ((statuses.id >= (((date_part('epoch'::text, (now() - 'P30D'::interval)) * (1000)::double precision))::bigint << 16)) AND (accounts.suspended_at IS NULL) AND (accounts.moved_to_account_id IS NULL) AND (accounts.silenced_at IS NULL) AND (accounts.locked = false) AND (accounts.discoverable = true)) + GROUP BY accounts.id + HAVING (sum((status_stats.reblogs_count + status_stats.favourites_count)) >= (5)::numeric)) t0 + GROUP BY t0.account_id + ORDER BY (sum(t0.rank)) DESC; + SQL end diff --git a/db/views/account_summaries_v01.sql b/db/views/account_summaries_v01.sql new file mode 100644 index 000000000..5a632b622 --- /dev/null +++ b/db/views/account_summaries_v01.sql @@ -0,0 +1,22 @@ +SELECT + accounts.id AS account_id, + mode() WITHIN GROUP (ORDER BY language ASC) AS language, + mode() WITHIN GROUP (ORDER BY sensitive ASC) AS sensitive +FROM accounts +CROSS JOIN LATERAL ( + SELECT + statuses.account_id, + statuses.language, + statuses.sensitive + FROM statuses + WHERE statuses.account_id = accounts.id + AND statuses.deleted_at IS NULL + ORDER BY statuses.id DESC + LIMIT 20 +) t0 +WHERE accounts.suspended_at IS NULL + AND accounts.silenced_at IS NULL + AND accounts.moved_to_account_id IS NULL + AND accounts.discoverable = 't' + AND accounts.locked = 'f' +GROUP BY accounts.id diff --git a/db/views/follow_recommendations_v01.sql b/db/views/follow_recommendations_v01.sql new file mode 100644 index 000000000..799abeaee --- /dev/null +++ b/db/views/follow_recommendations_v01.sql @@ -0,0 +1,38 @@ +SELECT + account_id, + sum(rank) AS rank, + array_agg(reason) AS reason +FROM ( + SELECT + accounts.id AS account_id, + count(follows.id) / (1.0 + count(follows.id)) AS rank, + 'most_followed' AS reason + FROM follows + INNER JOIN accounts ON accounts.id = follows.target_account_id + INNER JOIN users ON users.account_id = follows.account_id + WHERE users.current_sign_in_at >= (now() - interval '30 days') + AND accounts.suspended_at IS NULL + AND accounts.moved_to_account_id IS NULL + AND accounts.silenced_at IS NULL + AND accounts.locked = 'f' + AND accounts.discoverable = 't' + GROUP BY accounts.id + HAVING count(follows.id) >= 5 + UNION ALL + SELECT accounts.id AS account_id, + sum(reblogs_count + favourites_count) / (1.0 + sum(reblogs_count + favourites_count)) AS rank, + 'most_interactions' AS reason + FROM status_stats + INNER JOIN statuses ON statuses.id = status_stats.status_id + INNER JOIN accounts ON accounts.id = statuses.account_id + WHERE statuses.id >= ((date_part('epoch', now() - interval '30 days') * 1000)::bigint << 16) + AND accounts.suspended_at IS NULL + AND accounts.moved_to_account_id IS NULL + AND accounts.silenced_at IS NULL + AND accounts.locked = 'f' + AND accounts.discoverable = 't' + GROUP BY accounts.id + HAVING sum(reblogs_count + favourites_count) >= 5 +) t0 +GROUP BY account_id +ORDER BY rank DESC diff --git a/spec/fabricators/follow_recommendation_suppression_fabricator.rb b/spec/fabricators/follow_recommendation_suppression_fabricator.rb new file mode 100644 index 000000000..4a6a07a66 --- /dev/null +++ b/spec/fabricators/follow_recommendation_suppression_fabricator.rb @@ -0,0 +1,3 @@ +Fabricator(:follow_recommendation_suppression) do + account +end diff --git a/spec/models/follow_recommendation_suppression_spec.rb b/spec/models/follow_recommendation_suppression_spec.rb new file mode 100644 index 000000000..39107a2b0 --- /dev/null +++ b/spec/models/follow_recommendation_suppression_spec.rb @@ -0,0 +1,4 @@ +require 'rails_helper' + +RSpec.describe FollowRecommendationSuppression, type: :model do +end -- cgit From 120965eb0b1b0da6906bb242da50a77367defd96 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 12 Apr 2021 14:25:34 +0200 Subject: Change Web Push API deliveries to use request pooling (#16014) --- Gemfile | 2 +- Gemfile.lock | 2 +- app/models/web/push_subscription.rb | 91 +++++++++++------------ app/workers/web/push_notification_worker.rb | 65 +++++++++++++--- spec/workers/web/push_notification_worker_spec.rb | 48 ++++++++++++ 5 files changed, 150 insertions(+), 58 deletions(-) create mode 100644 spec/workers/web/push_notification_worker_spec.rb (limited to 'spec') diff --git a/Gemfile b/Gemfile index d4385f014..cc77ee618 100644 --- a/Gemfile +++ b/Gemfile @@ -94,7 +94,7 @@ gem 'tty-prompt', '~> 0.23', require: false gem 'twitter-text', '~> 3.1.0' gem 'tzinfo-data', '~> 1.2021' gem 'webpacker', '~> 5.2' -gem 'webpush' +gem 'webpush', '~> 0.3' gem 'webauthn', '~> 3.0.0.alpha1' gem 'json-ld' diff --git a/Gemfile.lock b/Gemfile.lock index b1fb350d4..1d3c24595 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -808,5 +808,5 @@ DEPENDENCIES webauthn (~> 3.0.0.alpha1) webmock (~> 3.12) webpacker (~> 5.2) - webpush + webpush (~> 0.3) xorcist (~> 1.1) diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb index c407a7789..7609b1bfc 100644 --- a/app/models/web/push_subscription.rb +++ b/app/models/web/push_subscription.rb @@ -24,81 +24,80 @@ class Web::PushSubscription < ApplicationRecord validates :key_p256dh, presence: true validates :key_auth, presence: true - def push(notification) - I18n.with_locale(associated_user&.locale || I18n.default_locale) do - push_payload(payload_for_notification(notification), 48.hours.seconds) - end + delegate :locale, to: :associated_user + + def encrypt(payload) + Webpush::Encryption.encrypt(payload, key_p256dh, key_auth) + end + + def audience + @audience ||= Addressable::URI.parse(endpoint).normalized_site + end + + def crypto_key_header + p256ecdsa = vapid_key.public_key_for_push_header + + "p256ecdsa=#{p256ecdsa}" + end + + def authorization_header + jwt = JWT.encode({ aud: audience, exp: 24.hours.from_now.to_i, sub: "mailto:#{contact_email}" }, vapid_key.curve, 'ES256', typ: 'JWT') + + "WebPush #{jwt}" end def pushable?(notification) - data&.key?('alerts') && ActiveModel::Type::Boolean.new.cast(data['alerts'][notification.type.to_s]) + ActiveModel::Type::Boolean.new.cast(data&.dig('alerts', notification.type.to_s)) end def associated_user return @associated_user if defined?(@associated_user) - @associated_user = if user_id.nil? - session_activation.user - else - user - end + @associated_user = begin + if user_id.nil? + session_activation.user + else + user + end + end end def associated_access_token return @associated_access_token if defined?(@associated_access_token) - @associated_access_token = if access_token_id.nil? - find_or_create_access_token.token - else - access_token.token - end + @associated_access_token = begin + if access_token_id.nil? + find_or_create_access_token.token + else + access_token.token + end + end end class << self def unsubscribe_for(application_id, resource_owner) - access_token_ids = Doorkeeper::AccessToken.where(application_id: application_id, resource_owner_id: resource_owner.id, revoked_at: nil) - .pluck(:id) - + access_token_ids = Doorkeeper::AccessToken.where(application_id: application_id, resource_owner_id: resource_owner.id, revoked_at: nil).pluck(:id) where(access_token_id: access_token_ids).delete_all end end private - def push_payload(message, ttl = 5.minutes.seconds) - Webpush.payload_send( - message: Oj.dump(message), - endpoint: endpoint, - p256dh: key_p256dh, - auth: key_auth, - ttl: ttl, - ssl_timeout: 10, - open_timeout: 10, - read_timeout: 10, - vapid: { - subject: "mailto:#{::Setting.site_contact_email}", - private_key: Rails.configuration.x.vapid_private_key, - public_key: Rails.configuration.x.vapid_public_key, - } - ) - end - - def payload_for_notification(notification) - ActiveModelSerializers::SerializableResource.new( - notification, - serializer: Web::NotificationSerializer, - scope: self, - scope_name: :current_push_subscription - ).as_json - end - def find_or_create_access_token Doorkeeper::AccessToken.find_or_create_for( application: Doorkeeper::Application.find_by(superapp: true), - resource_owner: session_activation.user_id, + resource_owner: user_id || session_activation.user_id, scopes: Doorkeeper::OAuth::Scopes.from_string('read write follow push'), expires_in: Doorkeeper.configuration.access_token_expires_in, use_refresh_token: Doorkeeper.configuration.refresh_token_enabled? ) end + + def vapid_key + @vapid_key ||= Webpush::VapidKey.from_keys(Rails.configuration.x.vapid_public_key, Rails.configuration.x.vapid_private_key) + end + + def contact_email + @contact_email ||= ::Setting.site_contact_email + end end diff --git a/app/workers/web/push_notification_worker.rb b/app/workers/web/push_notification_worker.rb index 46aeaa30b..57f5b5c22 100644 --- a/app/workers/web/push_notification_worker.rb +++ b/app/workers/web/push_notification_worker.rb @@ -3,22 +3,67 @@ class Web::PushNotificationWorker include Sidekiq::Worker - sidekiq_options backtrace: true, retry: 5 + sidekiq_options queue: 'push', retry: 5 + + TTL = 48.hours.to_s + URGENCY = 'normal' def perform(subscription_id, notification_id) - subscription = ::Web::PushSubscription.find(subscription_id) - notification = Notification.find(notification_id) + @subscription = Web::PushSubscription.find(subscription_id) + @notification = Notification.find(notification_id) + + # Polymorphically associated activity could have been deleted + # in the meantime, so we have to double-check before proceeding + return unless @notification.activity.present? && @subscription.pushable?(@notification) + + payload = @subscription.encrypt(push_notification_json) - subscription.push(notification) unless notification.activity.nil? - rescue Webpush::ResponseError => e - code = e.response.code.to_i + request_pool.with(@subscription.audience) do |http_client| + request = Request.new(:post, @subscription.endpoint, body: payload.fetch(:ciphertext), http_client: http_client) - if (400..499).cover?(code) && ![408, 429].include?(code) - subscription.destroy! - else - raise e + request.add_headers( + 'Content-Type' => 'application/octet-stream', + 'Ttl' => TTL, + 'Urgency' => URGENCY, + 'Content-Encoding' => 'aesgcm', + 'Encryption' => "salt=#{Webpush.encode64(payload.fetch(:salt)).delete('=')}", + 'Crypto-Key' => "dh=#{Webpush.encode64(payload.fetch(:server_public_key)).delete('=')};#{@subscription.crypto_key_header}", + 'Authorization' => @subscription.authorization_header + ) + + request.perform do |response| + # If the server responds with an error in the 4xx range + # that isn't about rate-limiting or timeouts, we can + # assume that the subscription is invalid or expired + # and must be removed + + if (400..499).cover?(response.code) && ![408, 429].include?(response.code) + @subscription.destroy! + elsif !(200...300).cover?(response.code) + raise Mastodon::UnexpectedResponseError, response + end + end end rescue ActiveRecord::RecordNotFound true end + + private + + def push_notification_json + json = I18n.with_locale(@subscription.locale || I18n.default_locale) do + ActiveModelSerializers::SerializableResource.new( + @notification, + serializer: Web::NotificationSerializer, + scope: @subscription, + scope_name: :current_push_subscription + ).as_json + end + + Oj.dump(json) + end + + def request_pool + RequestPool.current + end end diff --git a/spec/workers/web/push_notification_worker_spec.rb b/spec/workers/web/push_notification_worker_spec.rb new file mode 100644 index 000000000..5bc24f888 --- /dev/null +++ b/spec/workers/web/push_notification_worker_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Web::PushNotificationWorker do + subject { described_class.new } + + let(:p256dh) { 'BN4GvZtEZiZuqFxSKVZfSfluwKBD7UxHNBmWkfiZfCtgDE8Bwh-_MtLXbBxTBAWH9r7IPKL0lhdcaqtL1dfxU5E=' } + let(:auth) { 'Q2BoAjC09xH3ywDLNJr-dA==' } + let(:endpoint) { 'https://updates.push.services.mozilla.com/push/v1/subscription-id' } + let(:user) { Fabricate(:user) } + let(:notification) { Fabricate(:notification) } + let(:subscription) { Fabricate(:web_push_subscription, user_id: user.id, key_p256dh: p256dh, key_auth: auth, endpoint: endpoint, data: { alerts: { notification.type => true } }) } + let(:vapid_public_key) { 'BB37UCyc8LLX4PNQSe-04vSFvpUWGrENubUaslVFM_l5TxcGVMY0C3RXPeUJAQHKYlcOM2P4vTYmkoo0VZGZTM4=' } + let(:vapid_private_key) { 'OPrw1Sum3gRoL4-DXfSCC266r-qfFSRZrnj8MgIhRHg=' } + let(:vapid_key) { Webpush::VapidKey.from_keys(vapid_public_key, vapid_private_key) } + let(:contact_email) { 'sender@example.com' } + let(:ciphertext) { "+\xB8\xDBT}\x13\xB6\xDD.\xF9\xB0\xA7\xC8\xD2\x80\xFD\x99#\xF7\xAC\x83\xA4\xDB,\x1F\xB5\xB9w\x85>\xF7\xADr" } + let(:salt) { "X\x97\x953\xE4X\xF8_w\xE7T\x95\xC51q\xFE" } + let(:server_public_key) { "\x04\b-RK9w\xDD$\x16lFz\xF9=\xB4~\xC6\x12k\xF3\xF40t\xA9\xC1\fR\xC3\x81\x80\xAC\f\x7F\xE4\xCC\x8E\xC2\x88 n\x8BB\xF1\x9C\x14\a\xFA\x8D\xC9\x80\xA1\xDDyU\\&c\x01\x88#\x118Ua" } + let(:shared_secret) { "\t\xA7&\x85\t\xC5m\b\xA8\xA7\xF8B{1\xADk\xE1y'm\xEDE\xEC\xDD\xEDj\xB3$s\xA9\xDA\xF0" } + let(:payload) { { ciphertext: ciphertext, salt: salt, server_public_key: server_public_key, shared_secret: shared_secret } } + + describe 'perform' do + before do + allow_any_instance_of(subscription.class).to receive(:contact_email).and_return(contact_email) + allow_any_instance_of(subscription.class).to receive(:vapid_key).and_return(vapid_key) + allow(Webpush::Encryption).to receive(:encrypt).and_return(payload) + allow(JWT).to receive(:encode).and_return('jwt.encoded.payload') + + stub_request(:post, endpoint).to_return(status: 201, body: '') + + subject.perform(subscription.id, notification.id) + end + + it 'calls the relevant service with the correct headers' do + expect(a_request(:post, endpoint).with(headers: { + 'Content-Encoding' => 'aesgcm', + 'Content-Type' => 'application/octet-stream', + 'Crypto-Key' => 'dh=BAgtUks5d90kFmxGevk9tH7GEmvz9DB0qcEMUsOBgKwMf-TMjsKIIG6LQvGcFAf6jcmAod15VVwmYwGIIxE4VWE;p256ecdsa=' + vapid_public_key.delete('='), + 'Encryption' => 'salt=WJeVM-RY-F9351SVxTFx_g', + 'Ttl' => '172800', + 'Urgency' => 'normal', + 'Authorization' => 'WebPush jwt.encoded.payload', + }, body: "+\xB8\xDBT}\u0013\xB6\xDD.\xF9\xB0\xA7\xC8Ҁ\xFD\x99#\xF7\xAC\x83\xA4\xDB,\u001F\xB5\xB9w\x85>\xF7\xADr")).to have_been_made + end + end +end -- cgit From ce2148c57111981be455a9953d3bb589cf53967f Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 15 Apr 2021 05:00:25 +0200 Subject: Add `policy` param to `POST /api/v1/push/subscriptions` (#16040) With possible values `all`, `followed`, `follower`, and `none`, control from whom notifications will generate a Web Push alert --- .../api/v1/push/subscriptions_controller.rb | 28 +++---- .../api/web/push_subscriptions_controller.rb | 25 +++--- app/models/web/push_subscription.rb | 23 +++++- .../api/v1/push/subscriptions_controller_spec.rb | 28 ++++--- .../api/web/push_subscriptions_controller_spec.rb | 23 ++++-- spec/models/web/push_subscription_spec.rb | 94 ++++++++++++++++++++-- 6 files changed, 170 insertions(+), 51 deletions(-) (limited to 'spec') diff --git a/app/controllers/api/v1/push/subscriptions_controller.rb b/app/controllers/api/v1/push/subscriptions_controller.rb index 0918c61e9..47f2e6440 100644 --- a/app/controllers/api/v1/push/subscriptions_controller.rb +++ b/app/controllers/api/v1/push/subscriptions_controller.rb @@ -3,13 +3,13 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController before_action -> { doorkeeper_authorize! :push } before_action :require_user! - before_action :set_web_push_subscription - before_action :check_web_push_subscription, only: [:show, :update] + before_action :set_push_subscription + before_action :check_push_subscription, only: [:show, :update] def create - @web_subscription&.destroy! + @push_subscription&.destroy! - @web_subscription = ::Web::PushSubscription.create!( + @push_subscription = Web::PushSubscription.create!( endpoint: subscription_params[:endpoint], key_p256dh: subscription_params[:keys][:p256dh], key_auth: subscription_params[:keys][:auth], @@ -18,31 +18,31 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController access_token_id: doorkeeper_token.id ) - render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end def show - render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end def update - @web_subscription.update!(data: data_params) - render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer + @push_subscription.update!(data: data_params) + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end def destroy - @web_subscription&.destroy! + @push_subscription&.destroy! render_empty end private - def set_web_push_subscription - @web_subscription = ::Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id) + def set_push_subscription + @push_subscription = Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id) end - def check_web_push_subscription - not_found if @web_subscription.nil? + def check_push_subscription + not_found if @push_subscription.nil? end def subscription_params @@ -52,6 +52,6 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController def data_params return {} if params[:data].blank? - params.require(:data).permit(alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) + params.require(:data).permit(:policy, alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) end end diff --git a/app/controllers/api/web/push_subscriptions_controller.rb b/app/controllers/api/web/push_subscriptions_controller.rb index 1dce3e70f..bed57fc54 100644 --- a/app/controllers/api/web/push_subscriptions_controller.rb +++ b/app/controllers/api/web/push_subscriptions_controller.rb @@ -2,6 +2,7 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController before_action :require_user! + before_action :set_push_subscription, only: :update def create active_session = current_session @@ -15,9 +16,11 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController alerts_enabled = active_session.detection.device.mobile? || active_session.detection.device.tablet? data = { + policy: 'all', + alerts: { follow: alerts_enabled, - follow_request: false, + follow_request: alerts_enabled, favourite: alerts_enabled, reblog: alerts_enabled, mention: alerts_enabled, @@ -28,7 +31,7 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController data.deep_merge!(data_params) if params[:data] - web_subscription = ::Web::PushSubscription.create!( + push_subscription = ::Web::PushSubscription.create!( endpoint: subscription_params[:endpoint], key_p256dh: subscription_params[:keys][:p256dh], key_auth: subscription_params[:keys][:auth], @@ -37,27 +40,27 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController access_token_id: active_session.access_token_id ) - active_session.update!(web_push_subscription: web_subscription) + active_session.update!(web_push_subscription: push_subscription) - render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer + render json: push_subscription, serializer: REST::WebPushSubscriptionSerializer end def update - params.require([:id]) - - web_subscription = ::Web::PushSubscription.find(params[:id]) - web_subscription.update!(data: data_params) - - render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer + @push_subscription.update!(data: data_params) + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer end private + def set_push_subscription + @push_subscription = ::Web::PushSubscription.find(params[:id]) + end + def subscription_params @subscription_params ||= params.require(:subscription).permit(:endpoint, keys: [:auth, :p256dh]) end def data_params - @data_params ||= params.require(:data).permit(alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) + @data_params ||= params.require(:data).permit(:policy, alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) end end diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb index 7609b1bfc..6e46573ae 100644 --- a/app/models/web/push_subscription.rb +++ b/app/models/web/push_subscription.rb @@ -47,7 +47,7 @@ class Web::PushSubscription < ApplicationRecord end def pushable?(notification) - ActiveModel::Type::Boolean.new.cast(data&.dig('alerts', notification.type.to_s)) + policy_allows_notification?(notification) && alert_enabled_for_notification_type?(notification) end def associated_user @@ -100,4 +100,25 @@ class Web::PushSubscription < ApplicationRecord def contact_email @contact_email ||= ::Setting.site_contact_email end + + def alert_enabled_for_notification_type?(notification) + truthy?(data&.dig('alerts', notification.type.to_s)) + end + + def policy_allows_notification?(notification) + case data&.dig('policy') + when nil, 'all' + true + when 'none' + false + when 'followed' + notification.account.following?(notification.from_account) + when 'follower' + notification.from_account.following?(notification.account) + end + end + + def truthy?(val) + ActiveModel::Type::Boolean.new.cast(val) + end end diff --git a/spec/controllers/api/v1/push/subscriptions_controller_spec.rb b/spec/controllers/api/v1/push/subscriptions_controller_spec.rb index 01146294f..534d02879 100644 --- a/spec/controllers/api/v1/push/subscriptions_controller_spec.rb +++ b/spec/controllers/api/v1/push/subscriptions_controller_spec.rb @@ -27,20 +27,27 @@ describe Api::V1::Push::SubscriptionsController do let(:alerts_payload) do { data: { + policy: 'all', + alerts: { follow: true, + follow_request: true, favourite: false, reblog: true, mention: false, + poll: true, + status: false, } } }.with_indifferent_access end describe 'POST #create' do - it 'saves push subscriptions' do + before do post :create, params: create_payload + end + it 'saves push subscriptions' do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) expect(push_subscription.endpoint).to eq(create_payload[:subscription][:endpoint]) @@ -52,31 +59,34 @@ describe Api::V1::Push::SubscriptionsController do it 'replaces old subscription on repeat calls' do post :create, params: create_payload - post :create, params: create_payload - expect(Web::PushSubscription.where(endpoint: create_payload[:subscription][:endpoint]).count).to eq 1 end end describe 'PUT #update' do - it 'changes alert settings' do + before do post :create, params: create_payload put :update, params: alerts_payload + end + it 'changes alert settings' do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) - expect(push_subscription.data.dig('alerts', 'follow')).to eq(alerts_payload[:data][:alerts][:follow].to_s) - expect(push_subscription.data.dig('alerts', 'favourite')).to eq(alerts_payload[:data][:alerts][:favourite].to_s) - expect(push_subscription.data.dig('alerts', 'reblog')).to eq(alerts_payload[:data][:alerts][:reblog].to_s) - expect(push_subscription.data.dig('alerts', 'mention')).to eq(alerts_payload[:data][:alerts][:mention].to_s) + expect(push_subscription.data['policy']).to eq(alerts_payload[:data][:policy]) + + %w(follow follow_request favourite reblog mention poll status).each do |type| + expect(push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) + end end end describe 'DELETE #destroy' do - it 'removes the subscription' do + before do post :create, params: create_payload delete :destroy + end + it 'removes the subscription' do expect(Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint])).to be_nil end end diff --git a/spec/controllers/api/web/push_subscriptions_controller_spec.rb b/spec/controllers/api/web/push_subscriptions_controller_spec.rb index 381cdeab9..bda4a7661 100644 --- a/spec/controllers/api/web/push_subscriptions_controller_spec.rb +++ b/spec/controllers/api/web/push_subscriptions_controller_spec.rb @@ -22,11 +22,16 @@ describe Api::Web::PushSubscriptionsController do let(:alerts_payload) do { data: { + policy: 'all', + alerts: { follow: true, + follow_request: false, favourite: false, reblog: true, mention: false, + poll: true, + status: false, } } } @@ -59,10 +64,11 @@ describe Api::Web::PushSubscriptionsController do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) - expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s) - expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s) - expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s) - expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s) + expect(push_subscription.data['policy']).to eq 'all' + + %w(follow follow_request favourite reblog mention poll status).each do |type| + expect(push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) + end end end end @@ -81,10 +87,11 @@ describe Api::Web::PushSubscriptionsController do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) - expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s) - expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s) - expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s) - expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s) + expect(push_subscription.data['policy']).to eq 'all' + + %w(follow follow_request favourite reblog mention poll status).each do |type| + expect(push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) + end end end end diff --git a/spec/models/web/push_subscription_spec.rb b/spec/models/web/push_subscription_spec.rb index c6665611c..b44904369 100644 --- a/spec/models/web/push_subscription_spec.rb +++ b/spec/models/web/push_subscription_spec.rb @@ -1,16 +1,94 @@ require 'rails_helper' RSpec.describe Web::PushSubscription, type: :model do - let(:alerts) { { mention: true, reblog: false, follow: true, follow_request: false, favourite: true } } - let(:push_subscription) { Web::PushSubscription.new(data: { alerts: alerts }) } + let(:account) { Fabricate(:account) } + + let(:policy) { 'all' } + + let(:data) do + { + policy: policy, + + alerts: { + mention: true, + reblog: false, + follow: true, + follow_request: false, + favourite: true, + }, + } + end + + subject { described_class.new(data: data) } describe '#pushable?' do - it 'obeys alert settings' do - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Mention'))).to eq true - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Status'))).to eq false - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Follow'))).to eq true - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'FollowRequest'))).to eq false - expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Favourite'))).to eq true + let(:notification_type) { :mention } + let(:notification) { Fabricate(:notification, account: account, type: notification_type) } + + %i(mention reblog follow follow_request favourite).each do |type| + context "when notification is a #{type}" do + let(:notification_type) { type } + + it "returns boolean corresonding to alert setting" do + expect(subject.pushable?(notification)).to eq data[:alerts][type] + end + end + end + + context 'when policy is all' do + let(:policy) { 'all' } + + it 'returns true' do + expect(subject.pushable?(notification)).to eq true + end + end + + context 'when policy is none' do + let(:policy) { 'none' } + + it 'returns false' do + expect(subject.pushable?(notification)).to eq false + end + end + + context 'when policy is followed' do + let(:policy) { 'followed' } + + context 'and notification is from someone you follow' do + before do + account.follow!(notification.from_account) + end + + it 'returns true' do + expect(subject.pushable?(notification)).to eq true + end + end + + context 'and notification is not from someone you follow' do + it 'returns false' do + expect(subject.pushable?(notification)).to eq false + end + end + end + + context 'when policy is follower' do + let(:policy) { 'follower' } + + context 'and notification is from someone who follows you' do + before do + notification.from_account.follow!(account) + end + + it 'returns true' do + expect(subject.pushable?(notification)).to eq true + end + end + + context 'and notification is not from someone who follows you' do + it 'returns false' do + expect(subject.pushable?(notification)).to eq false + end + end end end end -- cgit From 3b8d085436fa38aed4d5fa3650e433fc7215b104 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 15 Apr 2021 16:28:43 +0200 Subject: Fix app name, website and redirect URIs not having a maximum length (#16042) Fix app scopes not being validated --- app/lib/application_extension.rb | 4 +- config/initializers/doorkeeper.rb | 5 ++ spec/controllers/api/v1/apps_controller_spec.rb | 78 ++++++++++++++++++++++--- 3 files changed, 77 insertions(+), 10 deletions(-) (limited to 'spec') diff --git a/app/lib/application_extension.rb b/app/lib/application_extension.rb index 1d80b8c6d..e61cd0721 100644 --- a/app/lib/application_extension.rb +++ b/app/lib/application_extension.rb @@ -4,6 +4,8 @@ module ApplicationExtension extend ActiveSupport::Concern included do - validates :website, url: true, if: :website? + validates :name, length: { maximum: 60 } + validates :website, url: true, length: { maximum: 2_000 }, if: :website? + validates :redirect_uri, length: { maximum: 2_000 } end end diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 63cff7c59..f78db8653 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -52,6 +52,11 @@ Doorkeeper.configure do # Issue access tokens with refresh token (disabled by default) # use_refresh_token + # Forbids creating/updating applications with arbitrary scopes that are + # not in configuration, i.e. `default_scopes` or `optional_scopes`. + # (Disabled by default) + enforce_configured_scopes + # Provide support for an owner to be assigned to each registered application (disabled by default) # Optional parameter :confirmation => true (default false) if you want to enforce ownership of # a registered application diff --git a/spec/controllers/api/v1/apps_controller_spec.rb b/spec/controllers/api/v1/apps_controller_spec.rb index 60a4c3b41..70cd62d48 100644 --- a/spec/controllers/api/v1/apps_controller_spec.rb +++ b/spec/controllers/api/v1/apps_controller_spec.rb @@ -4,23 +4,83 @@ RSpec.describe Api::V1::AppsController, type: :controller do render_views describe 'POST #create' do + let(:client_name) { 'Test app' } + let(:scopes) { nil } + let(:redirect_uris) { 'urn:ietf:wg:oauth:2.0:oob' } + let(:website) { nil } + + let(:app_params) do + { + client_name: client_name, + redirect_uris: redirect_uris, + scopes: scopes, + website: website, + } + end + before do - post :create, params: { client_name: 'Test app', redirect_uris: 'urn:ietf:wg:oauth:2.0:oob' } + post :create, params: app_params end - it 'returns http success' do - expect(response).to have_http_status(200) + context 'with valid params' do + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'creates an OAuth app' do + expect(Doorkeeper::Application.find_by(name: client_name)).to_not be nil + end + + it 'returns client ID and client secret' do + json = body_as_json + + expect(json[:client_id]).to_not be_blank + expect(json[:client_secret]).to_not be_blank + end + end + + context 'with an unsupported scope' do + let(:scopes) { 'hoge' } + + it 'returns http unprocessable entity' do + expect(response).to have_http_status(422) + end end - it 'creates an OAuth app' do - expect(Doorkeeper::Application.find_by(name: 'Test app')).to_not be nil + context 'with many duplicate scopes' do + let(:scopes) { (%w(read) * 40).join(' ') } + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'only saves the scope once' do + expect(Doorkeeper::Application.find_by(name: client_name).scopes.to_s).to eq 'read' + end + end + + context 'with a too-long name' do + let(:client_name) { 'hoge' * 20 } + + it 'returns http unprocessable entity' do + expect(response).to have_http_status(422) + end + end + + context 'with a too-long website' do + let(:website) { 'https://foo.bar/' + ('hoge' * 2_000) } + + it 'returns http unprocessable entity' do + expect(response).to have_http_status(422) + end end - it 'returns client ID and client secret' do - json = body_as_json + context 'with a too-long redirect_uris' do + let(:redirect_uris) { 'https://foo.bar/' + ('hoge' * 2_000) } - expect(json[:client_id]).to_not be_blank - expect(json[:client_secret]).to_not be_blank + it 'returns http unprocessable entity' do + expect(response).to have_http_status(422) + end end end end -- cgit From b3ceb3dcc4df62803aa967d7aecee686973a8996 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 17 Apr 2021 03:14:25 +0200 Subject: Add canonical e-mail blocks for suspended accounts (#16049) Prevent new accounts from being created using the same underlying e-mail as a suspended account using extensions and period permutations. Stores e-mails as a SHA256 hash --- app/helpers/email_helper.rb | 18 +++++++++ app/models/account.rb | 14 +++++++ app/models/canonical_email_block.rb | 27 +++++++++++++ app/validators/blacklisted_email_validator.rb | 30 +++++++++----- ...20210416200740_create_canonical_email_blocks.rb | 10 +++++ db/schema.rb | 12 +++++- .../canonical_email_block_fabricator.rb | 4 ++ spec/models/canonical_email_block_spec.rb | 47 ++++++++++++++++++++++ .../validators/blacklisted_email_validator_spec.rb | 29 +++++++++---- 9 files changed, 171 insertions(+), 20 deletions(-) create mode 100644 app/helpers/email_helper.rb create mode 100644 app/models/canonical_email_block.rb create mode 100644 db/migrate/20210416200740_create_canonical_email_blocks.rb create mode 100644 spec/fabricators/canonical_email_block_fabricator.rb create mode 100644 spec/models/canonical_email_block_spec.rb (limited to 'spec') diff --git a/app/helpers/email_helper.rb b/app/helpers/email_helper.rb new file mode 100644 index 000000000..360783c62 --- /dev/null +++ b/app/helpers/email_helper.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module EmailHelper + def self.included(base) + base.extend(self) + end + + def email_to_canonical_email(str) + username, domain = str.downcase.split('@', 2) + username, = username.gsub('.', '').split('+', 2) + + "#{username}@#{domain}" + end + + def email_to_canonical_email_hash(str) + Digest::SHA2.new(256).hexdigest(email_to_canonical_email(str)) + end +end diff --git a/app/models/account.rb b/app/models/account.rb index 80689d4aa..a573365de 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -235,6 +235,7 @@ class Account < ApplicationRecord transaction do create_deletion_request! update!(suspended_at: date, suspension_origin: origin) + create_canonical_email_block! end end @@ -242,6 +243,7 @@ class Account < ApplicationRecord transaction do deletion_request&.destroy! update!(suspended_at: nil, suspension_origin: nil) + destroy_canonical_email_block! end end @@ -569,4 +571,16 @@ class Account < ApplicationRecord def clean_feed_manager FeedManager.instance.clean_feeds!(:home, [id]) end + + def create_canonical_email_block! + return unless local? && user_email.present? + + CanonicalEmailBlock.create(reference_account: self, email: user_email) + end + + def destroy_canonical_email_block! + return unless local? + + CanonicalEmailBlock.where(reference_account: self).delete_all + end end diff --git a/app/models/canonical_email_block.rb b/app/models/canonical_email_block.rb new file mode 100644 index 000000000..a8546d65a --- /dev/null +++ b/app/models/canonical_email_block.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: canonical_email_blocks +# +# id :bigint(8) not null, primary key +# canonical_email_hash :string default(""), not null +# reference_account_id :bigint(8) not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class CanonicalEmailBlock < ApplicationRecord + include EmailHelper + + belongs_to :reference_account, class_name: 'Account' + + validates :canonical_email_hash, presence: true + + def email=(email) + self.canonical_email_hash = email_to_canonical_email_hash(email) + end + + def self.block?(email) + where(canonical_email_hash: email_to_canonical_email_hash(email)).exists? + end +end diff --git a/app/validators/blacklisted_email_validator.rb b/app/validators/blacklisted_email_validator.rb index 1ca73fdcc..eb66ad93d 100644 --- a/app/validators/blacklisted_email_validator.rb +++ b/app/validators/blacklisted_email_validator.rb @@ -6,26 +6,25 @@ class BlacklistedEmailValidator < ActiveModel::Validator @email = user.email - user.errors.add(:email, :blocked) if blocked_email? + user.errors.add(:email, :blocked) if blocked_email_provider? + user.errors.add(:email, :taken) if blocked_canonical_email? end private - def blocked_email? - on_blacklist? || not_on_whitelist? + def blocked_email_provider? + disallowed_through_email_domain_block? || disallowed_through_configuration? || not_allowed_through_configuration? end - def on_blacklist? - return true if EmailDomainBlock.block?(@email) - return false if Rails.configuration.x.email_domains_blacklist.blank? - - domains = Rails.configuration.x.email_domains_blacklist.gsub('.', '\.') - regexp = Regexp.new("@(.+\\.)?(#{domains})", true) + def blocked_canonical_email? + CanonicalEmailBlock.block?(@email) + end - regexp.match?(@email) + def disallowed_through_email_domain_block? + EmailDomainBlock.block?(@email) end - def not_on_whitelist? + def not_allowed_through_configuration? return false if Rails.configuration.x.email_domains_whitelist.blank? domains = Rails.configuration.x.email_domains_whitelist.gsub('.', '\.') @@ -33,4 +32,13 @@ class BlacklistedEmailValidator < ActiveModel::Validator @email !~ regexp end + + def disallowed_through_configuration? + return false if Rails.configuration.x.email_domains_blacklist.blank? + + domains = Rails.configuration.x.email_domains_blacklist.gsub('.', '\.') + regexp = Regexp.new("@(.+\\.)?(#{domains})", true) + + regexp.match?(@email) + end end diff --git a/db/migrate/20210416200740_create_canonical_email_blocks.rb b/db/migrate/20210416200740_create_canonical_email_blocks.rb new file mode 100644 index 000000000..a1f1660bf --- /dev/null +++ b/db/migrate/20210416200740_create_canonical_email_blocks.rb @@ -0,0 +1,10 @@ +class CreateCanonicalEmailBlocks < ActiveRecord::Migration[6.1] + def change + create_table :canonical_email_blocks do |t| + t.string :canonical_email_hash, null: false, default: '', index: { unique: true } + t.belongs_to :reference_account, null: false, foreign_key: { on_cascade: :delete, to_table: 'accounts' } + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 599cd873b..dcbbf4aea 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_03_24_171613) do +ActiveRecord::Schema.define(version: 2021_04_16_200740) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -280,6 +280,15 @@ ActiveRecord::Schema.define(version: 2021_03_24_171613) do t.index ["status_id"], name: "index_bookmarks_on_status_id" end + create_table "canonical_email_blocks", force: :cascade do |t| + t.string "canonical_email_hash", default: "", null: false + t.bigint "reference_account_id", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["canonical_email_hash"], name: "index_canonical_email_blocks_on_canonical_email_hash", unique: true + t.index ["reference_account_id"], name: "index_canonical_email_blocks_on_reference_account_id" + end + create_table "conversation_mutes", force: :cascade do |t| t.bigint "conversation_id", null: false t.bigint "account_id", null: false @@ -991,6 +1000,7 @@ ActiveRecord::Schema.define(version: 2021_03_24_171613) do add_foreign_key "blocks", "accounts", name: "fk_4269e03e65", on_delete: :cascade add_foreign_key "bookmarks", "accounts", on_delete: :cascade add_foreign_key "bookmarks", "statuses", on_delete: :cascade + add_foreign_key "canonical_email_blocks", "accounts", column: "reference_account_id" add_foreign_key "conversation_mutes", "accounts", name: "fk_225b4212bb", on_delete: :cascade add_foreign_key "conversation_mutes", "conversations", on_delete: :cascade add_foreign_key "custom_filters", "accounts", on_delete: :cascade diff --git a/spec/fabricators/canonical_email_block_fabricator.rb b/spec/fabricators/canonical_email_block_fabricator.rb new file mode 100644 index 000000000..a0b6e0d22 --- /dev/null +++ b/spec/fabricators/canonical_email_block_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:canonical_email_block) do + email "test@example.com" + reference_account { Fabricate(:account) } +end diff --git a/spec/models/canonical_email_block_spec.rb b/spec/models/canonical_email_block_spec.rb new file mode 100644 index 000000000..8e0050d65 --- /dev/null +++ b/spec/models/canonical_email_block_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe CanonicalEmailBlock, type: :model do + describe '#email=' do + let(:target_hash) { '973dfe463ec85785f5f95af5ba3906eedb2d931c24e69824a89ea65dba4e813b' } + + it 'sets canonical_email_hash' do + subject.email = 'test@example.com' + expect(subject.canonical_email_hash).to eq target_hash + end + + it 'sets the same hash even with dot permutations' do + subject.email = 't.e.s.t@example.com' + expect(subject.canonical_email_hash).to eq target_hash + end + + it 'sets the same hash even with extensions' do + subject.email = 'test+mastodon1@example.com' + expect(subject.canonical_email_hash).to eq target_hash + end + + it 'sets the same hash with different casing' do + subject.email = 'Test@EXAMPLE.com' + expect(subject.canonical_email_hash).to eq target_hash + end + end + + describe '.block?' do + let!(:canonical_email_block) { Fabricate(:canonical_email_block, email: 'foo@bar.com') } + + it 'returns true for the same email' do + expect(described_class.block?('foo@bar.com')).to be true + end + + it 'returns true for the same email with dots' do + expect(described_class.block?('f.oo@bar.com')).to be true + end + + it 'returns true for the same email with extensions' do + expect(described_class.block?('foo+spam@bar.com')).to be true + end + + it 'returns false for different email' do + expect(described_class.block?('hoge@bar.com')).to be false + end + end +end diff --git a/spec/validators/blacklisted_email_validator_spec.rb b/spec/validators/blacklisted_email_validator_spec.rb index 53b355a57..f7d5e01bc 100644 --- a/spec/validators/blacklisted_email_validator_spec.rb +++ b/spec/validators/blacklisted_email_validator_spec.rb @@ -9,23 +9,36 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do before do allow(user).to receive(:valid_invitation?) { false } - allow_any_instance_of(described_class).to receive(:blocked_email?) { blocked_email } - described_class.new.validate(user) + allow_any_instance_of(described_class).to receive(:blocked_email_provider?) { blocked_email } end - context 'blocked_email?' do + subject { described_class.new.validate(user); errors } + + context 'when e-mail provider is blocked' do let(:blocked_email) { true } - it 'calls errors.add' do - expect(errors).to have_received(:add).with(:email, :blocked) + it 'adds error' do + expect(subject).to have_received(:add).with(:email, :blocked) end end - context '!blocked_email?' do + context 'when e-mail provider is not blocked' do let(:blocked_email) { false } - it 'not calls errors.add' do - expect(errors).not_to have_received(:add).with(:email, :blocked) + it 'does not add errors' do + expect(subject).not_to have_received(:add).with(:email, :blocked) + end + + context 'when canonical e-mail is blocked' do + let(:other_user) { Fabricate(:user, email: 'i.n.f.o@mail.com') } + + before do + other_user.account.suspend! + end + + it 'adds error' do + expect(subject).to have_received(:add).with(:email, :taken) + end end end end -- cgit From 0b36e3419d4c4ce175f9db266ef5b3a49a9b3974 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 21 Apr 2021 04:46:09 +0200 Subject: Fix processing of remote Delete activities (#16084) * Add tests * Ensure deleted statuses are marked as such * Save some redis memory by not storing URIs in delete_upon_arrival values * Avoid possible race condition when processing incoming Deletes * Avoid potential duplicate Delete forwards * Lower lock durations to reduce issues in case of hard crash of the Rails process * Check for `lock.aquired?` and improve comment * Refactor RedisLock usage in app/lib/activitypub * Fix using incorrect or non-existent sender for relaying Deletes --- app/lib/activitypub/activity.rb | 14 ++++++- app/lib/activitypub/activity/announce.rb | 36 +++++++--------- app/lib/activitypub/activity/create.rb | 36 +++++----------- app/lib/activitypub/activity/delete.rb | 62 +++++++++++++++------------- app/services/remove_status_service.rb | 2 + spec/lib/activitypub/activity/delete_spec.rb | 20 +++++++++ 6 files changed, 91 insertions(+), 79 deletions(-) (limited to 'spec') diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 2b5d3ffc2..3baee4ca4 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -144,7 +144,7 @@ class ActivityPub::Activity end def delete_later!(uri) - redis.setex("delete_upon_arrival:#{@account.id}:#{uri}", 6.hours.seconds, uri) + redis.setex("delete_upon_arrival:#{@account.id}:#{uri}", 6.hours.seconds, true) end def status_from_object @@ -210,12 +210,22 @@ class ActivityPub::Activity end end - def lock_or_return(key, expire_after = 7.days.seconds) + def lock_or_return(key, expire_after = 2.hours.seconds) yield if redis.set(key, true, nx: true, ex: expire_after) ensure redis.del(key) end + def lock_or_fail(key) + RedisLock.acquire({ redis: Redis.current, key: key }) do |lock| + if lock.acquired? + yield + else + raise Mastodon::RaceConditionError + end + end + end + def fetch? !@options[:delivery] end diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index ae8b2db75..a1081522e 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -4,29 +4,25 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity def perform return reject_payload! if delete_arrived_first?(@json['id']) || !related_to_local_activity? - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - original_status = status_from_object + lock_or_fail("announce:#{@object['id']}") do + original_status = status_from_object - return reject_payload! if original_status.nil? || !announceable?(original_status) + return reject_payload! if original_status.nil? || !announceable?(original_status) - @status = Status.find_by(account: @account, reblog: original_status) + @status = Status.find_by(account: @account, reblog: original_status) - return @status unless @status.nil? + return @status unless @status.nil? - @status = Status.create!( - account: @account, - reblog: original_status, - uri: @json['id'], - created_at: @json['published'], - override_timestamps: @options[:override_timestamps], - visibility: visibility_from_audience - ) + @status = Status.create!( + account: @account, + reblog: original_status, + uri: @json['id'], + created_at: @json['published'], + override_timestamps: @options[:override_timestamps], + visibility: visibility_from_audience + ) - distribute(@status) - else - raise Mastodon::RaceConditionError - end + distribute(@status) end @status @@ -69,8 +65,4 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity def reblog_of_local_status? status_from_uri(object_uri)&.account&.local? end - - def lock_options - { redis: Redis.current, key: "announce:#{@object['id']}" } - end end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 9f6dd9ce0..c7a655d9d 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -45,19 +45,15 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def create_status return reject_payload! if unsupported_object_type? || invalid_origin?(object_uri) || tombstone_exists? || !related_to_local_activity? - RedisLock.acquire(lock_options) do |lock| - if lock.acquired? - return if delete_arrived_first?(object_uri) || poll_vote? # rubocop:disable Lint/NonLocalExitFromIterator + lock_or_fail("create:#{object_uri}") do + return if delete_arrived_first?(object_uri) || poll_vote? # rubocop:disable Lint/NonLocalExitFromIterator - @status = find_existing_status + @status = find_existing_status - if @status.nil? - process_status - elsif @options[:delivered_to_account_id].present? - postprocess_audience_and_deliver - end - else - raise Mastodon::RaceConditionError + if @status.nil? + process_status + elsif @options[:delivered_to_account_id].present? + postprocess_audience_and_deliver end end @@ -313,13 +309,9 @@ class ActivityPub::Activity::Create < ActivityPub::Activity poll = replied_to_status.preloadable_poll already_voted = true - RedisLock.acquire(poll_lock_options) do |lock| - if lock.acquired? - already_voted = poll.votes.where(account: @account).exists? - poll.votes.create!(account: @account, choice: poll.options.index(@object['name']), uri: object_uri) - else - raise Mastodon::RaceConditionError - end + lock_or_fail("vote:#{replied_to_status.poll_id}:#{@account.id}") do + already_voted = poll.votes.where(account: @account).exists? + poll.votes.create!(account: @account, choice: poll.options.index(@object['name']), uri: object_uri) end increment_voters_count! unless already_voted @@ -508,12 +500,4 @@ class ActivityPub::Activity::Create < ActivityPub::Activity poll.reload retry end - - def lock_options - { redis: Redis.current, key: "create:#{object_uri}" } - end - - def poll_lock_options - { redis: Redis.current, key: "vote:#{replied_to_status.poll_id}:#{@account.id}" } - end end diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index 2e5293b83..801647cf7 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -20,33 +20,35 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity def delete_note return if object_uri.nil? - unless invalid_origin?(object_uri) - RedisLock.acquire(lock_options) { |_lock| delete_later!(object_uri) } - Tombstone.find_or_create_by(uri: object_uri, account: @account) - end + lock_or_return("delete_status_in_progress:#{object_uri}", 5.minutes.seconds) do + unless invalid_origin?(object_uri) + # This lock ensures a concurrent `ActivityPub::Activity::Create` either + # does not create a status at all, or has finished saving it to the + # database before we try to load it. + # Without the lock, `delete_later!` could be called after `delete_arrived_first?` + # and `Status.find` before `Status.create!` + lock_or_fail("create:#{object_uri}") { delete_later!(object_uri) } - @status = Status.find_by(uri: object_uri, account: @account) - @status ||= Status.find_by(uri: @object['atomUri'], account: @account) if @object.is_a?(Hash) && @object['atomUri'].present? + Tombstone.find_or_create_by(uri: object_uri, account: @account) + end - return if @status.nil? + @status = Status.find_by(uri: object_uri, account: @account) + @status ||= Status.find_by(uri: @object['atomUri'], account: @account) if @object.is_a?(Hash) && @object['atomUri'].present? - if @status.distributable? - forward_for_reply - forward_for_reblogs - end + return if @status.nil? - delete_now! + forward! if @json['signature'].present? && @status.distributable? + delete_now! + end end - def forward_for_reblogs - return if @json['signature'].blank? - - rebloggers_ids = @status.reblogs.includes(:account).references(:account).merge(Account.local).pluck(:account_id) - inboxes = Account.where(id: ::Follow.where(target_account_id: rebloggers_ids).select(:account_id)).inboxes - [@account.preferred_inbox_url] + def rebloggers_ids + return @rebloggers_ids if defined?(@rebloggers_ids) + @rebloggers_ids = @status.reblogs.includes(:account).references(:account).merge(Account.local).pluck(:account_id) + end - ActivityPub::LowPriorityDeliveryWorker.push_bulk(inboxes) do |inbox_url| - [payload, rebloggers_ids.first, inbox_url] - end + def inboxes_for_reblogs + Account.where(id: ::Follow.where(target_account_id: rebloggers_ids).select(:account_id)).inboxes end def replied_to_status @@ -58,13 +60,19 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity !replied_to_status.nil? && replied_to_status.account.local? end - def forward_for_reply - return unless @json['signature'].present? && reply_to_local? + def inboxes_for_reply + replied_to_status.account.followers.inboxes + end + + def forward! + inboxes = inboxes_for_reblogs + inboxes += inboxes_for_reply if reply_to_local? + inboxes -= [@account.preferred_inbox_url] - inboxes = replied_to_status.account.followers.inboxes - [@account.preferred_inbox_url] + sender_id = reply_to_local? ? replied_to_status.account_id : rebloggers_ids.first - ActivityPub::LowPriorityDeliveryWorker.push_bulk(inboxes) do |inbox_url| - [payload, replied_to_status.account_id, inbox_url] + ActivityPub::LowPriorityDeliveryWorker.push_bulk(inboxes.uniq) do |inbox_url| + [payload, sender_id, inbox_url] end end @@ -75,8 +83,4 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity def payload @payload ||= Oj.dump(@json) end - - def lock_options - { redis: Redis.current, key: "create:#{object_uri}" } - end end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 2c2a26641..6e4d6e72a 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -16,6 +16,8 @@ class RemoveStatusService < BaseService @account = status.account @options = options + @status.discard + RedisLock.acquire(lock_options) do |lock| if lock.acquired? remove_from_self if @account.local? diff --git a/spec/lib/activitypub/activity/delete_spec.rb b/spec/lib/activitypub/activity/delete_spec.rb index 37b93ecf7..9dfb8a61b 100644 --- a/spec/lib/activitypub/activity/delete_spec.rb +++ b/spec/lib/activitypub/activity/delete_spec.rb @@ -49,4 +49,24 @@ RSpec.describe ActivityPub::Activity::Delete do end end end + + context 'when the status has been reported' do + describe '#perform' do + subject { described_class.new(json, sender) } + let!(:reporter) { Fabricate(:account) } + + before do + reporter.reports.create!(target_account: status.account, status_ids: [status.id], forwarded: false) + subject.perform + end + + it 'marks the status as deleted' do + expect(Status.find_by(id: status.id)).to be_nil + end + + it 'actually keeps a copy for inspection' do + expect(Status.with_discarded.find_by(id: status.id)).to_not be_nil + end + end + end end -- cgit From 9cc283f0b48104f5761b1e4f96040c18428d25e9 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 21 Apr 2021 18:31:24 +0200 Subject: Change the nouns "toot" and "status" to "post" (#16080) --- app/javascript/mastodon/locales/en.json | 78 ++++++++++++++++---------------- config/locales/en.yml | 76 +++++++++++++++---------------- spec/mailers/notification_mailer_spec.rb | 8 ++-- 3 files changed, 81 insertions(+), 81 deletions(-) (limited to 'spec') diff --git a/app/javascript/mastodon/locales/en.json b/app/javascript/mastodon/locales/en.json index 3f8d37249..b438111cf 100644 --- a/app/javascript/mastodon/locales/en.json +++ b/app/javascript/mastodon/locales/en.json @@ -32,13 +32,13 @@ "account.mute_notifications": "Mute notifications from @{name}", "account.muted": "Muted", "account.never_active": "Never", - "account.posts": "Toots", - "account.posts_with_replies": "Toots and replies", + "account.posts": "Posts", + "account.posts_with_replies": "Posts and replies", "account.report": "Report @{name}", "account.requested": "Awaiting approval. Click to cancel follow request", "account.share": "Share @{name}'s profile", "account.show_reblogs": "Show boosts from @{name}", - "account.statuses_counter": "{count, plural, one {{counter} Toot} other {{counter} Toots}}", + "account.statuses_counter": "{count, plural, one {{counter} Post} other {{counter} Posts}}", "account.unblock": "Unblock @{name}", "account.unblock_domain": "Unblock domain {domain}", "account.unendorse": "Don't feature on profile", @@ -71,7 +71,7 @@ "column.lists": "Lists", "column.mutes": "Muted users", "column.notifications": "Notifications", - "column.pins": "Pinned toots", + "column.pins": "Pinned posts", "column.public": "Federated timeline", "column_back_button.label": "Back", "column_header.hide_settings": "Hide settings", @@ -84,9 +84,9 @@ "community.column_settings.local_only": "Local only", "community.column_settings.media_only": "Media Only", "community.column_settings.remote_only": "Remote only", - "compose_form.direct_message_warning": "This toot will only be sent to the mentioned users.", + "compose_form.direct_message_warning": "This post will only be sent to the mentioned users.", "compose_form.direct_message_warning_learn_more": "Learn more", - "compose_form.hashtag_warning": "This toot won't be listed under any hashtag as it is unlisted. Only public toots can be searched by hashtag.", + "compose_form.hashtag_warning": "This post won't be listed under any hashtag as it is unlisted. Only public posts can be searched by hashtag.", "compose_form.lock_disclaimer": "Your account is not {locked}. Anyone can follow you to view your follower-only posts.", "compose_form.lock_disclaimer.lock": "locked", "compose_form.placeholder": "What's on your mind?", @@ -101,15 +101,15 @@ "compose_form.sensitive.hide": "{count, plural, one {Mark media as sensitive} other {Mark media as sensitive}}", "compose_form.sensitive.marked": "{count, plural, one {Media is marked as sensitive} other {Media is marked as sensitive}}", "compose_form.sensitive.unmarked": "{count, plural, one {Media is not marked as sensitive} other {Media is not marked as sensitive}}", - "compose_form.spoiler.marked": "Text is hidden behind warning", - "compose_form.spoiler.unmarked": "Text is not hidden", + "compose_form.spoiler.marked": "Remove content warning", + "compose_form.spoiler.unmarked": "Add content warning", "compose_form.spoiler_placeholder": "Write your warning here", "confirmation_modal.cancel": "Cancel", "confirmations.block.block_and_report": "Block & Report", "confirmations.block.confirm": "Block", "confirmations.block.message": "Are you sure you want to block {name}?", "confirmations.delete.confirm": "Delete", - "confirmations.delete.message": "Are you sure you want to delete this toot?", + "confirmations.delete.message": "Are you sure you want to delete this post?", "confirmations.delete_list.confirm": "Delete", "confirmations.delete_list.message": "Are you sure you want to permanently delete this list?", "confirmations.domain_block.confirm": "Block entire domain", @@ -120,7 +120,7 @@ "confirmations.mute.explanation": "This will hide posts from them and posts mentioning them, but it will still allow them to see your posts and follow you.", "confirmations.mute.message": "Are you sure you want to mute {name}?", "confirmations.redraft.confirm": "Delete & redraft", - "confirmations.redraft.message": "Are you sure you want to delete this toot and re-draft it? Favourites and boosts will be lost, and replies to the original post will be orphaned.", + "confirmations.redraft.message": "Are you sure you want to delete this post and re-draft it? Favourites and boosts will be lost, and replies to the original post will be orphaned.", "confirmations.reply.confirm": "Reply", "confirmations.reply.message": "Replying now will overwrite the message you are currently composing. Are you sure you want to proceed?", "confirmations.unfollow.confirm": "Unfollow", @@ -133,7 +133,7 @@ "directory.local": "From {domain} only", "directory.new_arrivals": "New arrivals", "directory.recently_active": "Recently active", - "embed.instructions": "Embed this toot on your website by copying the code below.", + "embed.instructions": "Embed this post on your website by copying the code below.", "embed.preview": "Here is what it will look like:", "emoji_button.activity": "Activity", "emoji_button.custom": "Custom", @@ -150,20 +150,20 @@ "emoji_button.symbols": "Symbols", "emoji_button.travel": "Travel & Places", "empty_column.account_suspended": "Account suspended", - "empty_column.account_timeline": "No toots here!", + "empty_column.account_timeline": "No posts here!", "empty_column.account_unavailable": "Profile unavailable", "empty_column.blocks": "You haven't blocked any users yet.", - "empty_column.bookmarked_statuses": "You don't have any bookmarked toots yet. When you bookmark one, it will show up here.", + "empty_column.bookmarked_statuses": "You don't have any bookmarked posts yet. When you bookmark one, it will show up here.", "empty_column.community": "The local timeline is empty. Write something publicly to get the ball rolling!", "empty_column.direct": "You don't have any direct messages yet. When you send or receive one, it will show up here.", "empty_column.domain_blocks": "There are no blocked domains yet.", - "empty_column.favourited_statuses": "You don't have any favourite toots yet. When you favourite one, it will show up here.", - "empty_column.favourites": "No one has favourited this toot yet. When someone does, they will show up here.", + "empty_column.favourited_statuses": "You don't have any favourite posts yet. When you favourite one, it will show up here.", + "empty_column.favourites": "No one has favourited this post yet. When someone does, they will show up here.", "empty_column.follow_requests": "You don't have any follow requests yet. When you receive one, it will show up here.", "empty_column.hashtag": "There is nothing in this hashtag yet.", "empty_column.home": "Your home timeline is empty! Visit {public} or use search to get started and meet other users.", "empty_column.home.public_timeline": "the public timeline", - "empty_column.list": "There is nothing in this list yet. When members of this list post new toots, they will appear here.", + "empty_column.list": "There is nothing in this list yet. When members of this list publish new posts, they will appear here.", "empty_column.lists": "You don't have any lists yet. When you create one, it will show up here.", "empty_column.mutes": "You haven't muted any users yet.", "empty_column.notifications": "You don't have any notifications yet. Interact with others to start the conversation.", @@ -212,23 +212,23 @@ "introduction.federation.local.text": "Public posts from people on the same server as you will appear in the local timeline.", "introduction.interactions.action": "Finish tutorial!", "introduction.interactions.favourite.headline": "Favourite", - "introduction.interactions.favourite.text": "You can save a toot for later, and let the author know that you liked it, by favouriting it.", + "introduction.interactions.favourite.text": "You can save a post for later, and let the author know that you liked it, by favouriting it.", "introduction.interactions.reblog.headline": "Boost", - "introduction.interactions.reblog.text": "You can share other people's toots with your followers by boosting them.", + "introduction.interactions.reblog.text": "You can share other people's posts with your followers by boosting them.", "introduction.interactions.reply.headline": "Reply", - "introduction.interactions.reply.text": "You can reply to other people's and your own toots, which will chain them together in a conversation.", + "introduction.interactions.reply.text": "You can reply to other people's and your own posts, which will chain them together in a conversation.", "introduction.welcome.action": "Let's go!", "introduction.welcome.headline": "First steps", "introduction.welcome.text": "Welcome to the fediverse! In a few moments, you'll be able to broadcast messages and talk to your friends across a wide variety of servers. But this server, {domain}, is special—it hosts your profile, so remember its name.", "keyboard_shortcuts.back": "to navigate back", "keyboard_shortcuts.blocked": "to open blocked users list", "keyboard_shortcuts.boost": "to boost", - "keyboard_shortcuts.column": "to focus a toot in one of the columns", + "keyboard_shortcuts.column": "to focus a post in one of the columns", "keyboard_shortcuts.compose": "to focus the compose textarea", "keyboard_shortcuts.description": "Description", "keyboard_shortcuts.direct": "to open direct messages column", "keyboard_shortcuts.down": "to move down in the list", - "keyboard_shortcuts.enter": "to open toot", + "keyboard_shortcuts.enter": "to open post", "keyboard_shortcuts.favourite": "to favourite", "keyboard_shortcuts.favourites": "to open favourites list", "keyboard_shortcuts.federated": "to open federated timeline", @@ -242,7 +242,7 @@ "keyboard_shortcuts.my_profile": "to open your profile", "keyboard_shortcuts.notifications": "to open notifications column", "keyboard_shortcuts.open_media": "to open media", - "keyboard_shortcuts.pinned": "to open pinned toots list", + "keyboard_shortcuts.pinned": "to open pinned posts list", "keyboard_shortcuts.profile": "to open author's profile", "keyboard_shortcuts.reply": "to reply", "keyboard_shortcuts.requests": "to open follow requests list", @@ -251,7 +251,7 @@ "keyboard_shortcuts.start": "to open \"get started\" column", "keyboard_shortcuts.toggle_hidden": "to show/hide text behind CW", "keyboard_shortcuts.toggle_sensitivity": "to show/hide media", - "keyboard_shortcuts.toot": "to start a brand new toot", + "keyboard_shortcuts.toot": "to start a brand new post", "keyboard_shortcuts.unfocus": "to un-focus compose textarea/search", "keyboard_shortcuts.up": "to move up in the list", "lightbox.close": "Close", @@ -284,7 +284,7 @@ "navigation_bar.blocks": "Blocked users", "navigation_bar.bookmarks": "Bookmarks", "navigation_bar.community_timeline": "Local timeline", - "navigation_bar.compose": "Compose new toot", + "navigation_bar.compose": "Compose new post", "navigation_bar.direct": "Direct messages", "navigation_bar.discover": "Discover", "navigation_bar.domain_blocks": "Blocked domains", @@ -299,17 +299,17 @@ "navigation_bar.logout": "Logout", "navigation_bar.mutes": "Muted users", "navigation_bar.personal": "Personal", - "navigation_bar.pins": "Pinned toots", + "navigation_bar.pins": "Pinned posts", "navigation_bar.preferences": "Preferences", "navigation_bar.public_timeline": "Federated timeline", "navigation_bar.security": "Security", - "notification.favourite": "{name} favourited your toot", + "notification.favourite": "{name} favourited your post", "notification.follow": "{name} followed you", "notification.follow_request": "{name} has requested to follow you", "notification.mention": "{name} mentioned you", "notification.own_poll": "Your poll has ended", "notification.poll": "A poll you have voted in has ended", - "notification.reblog": "{name} boosted your toot", + "notification.reblog": "{name} boosted your post", "notification.status": "{name} just posted", "notifications.clear": "Clear notifications", "notifications.clear_confirmation": "Are you sure you want to permanently clear all your notifications?", @@ -326,7 +326,7 @@ "notifications.column_settings.reblog": "Boosts:", "notifications.column_settings.show": "Show in column", "notifications.column_settings.sound": "Play sound", - "notifications.column_settings.status": "New toots:", + "notifications.column_settings.status": "New posts:", "notifications.filter.all": "All", "notifications.filter.boosts": "Boosts", "notifications.filter.favourites": "Favourites", @@ -352,7 +352,7 @@ "poll.voted": "You voted for this answer", "poll_button.add_poll": "Add a poll", "poll_button.remove_poll": "Remove poll", - "privacy.change": "Adjust toot privacy", + "privacy.change": "Change post privacy", "privacy.direct.long": "Visible for mentioned users only", "privacy.direct.short": "Direct", "privacy.private.long": "Visible for followers only", @@ -379,23 +379,23 @@ "report.target": "Reporting {target}", "search.placeholder": "Search", "search_popout.search_format": "Advanced search format", - "search_popout.tips.full_text": "Simple text returns toots you have written, favourited, boosted, or have been mentioned in, as well as matching usernames, display names, and hashtags.", + "search_popout.tips.full_text": "Simple text returns posts you have written, favourited, boosted, or have been mentioned in, as well as matching usernames, display names, and hashtags.", "search_popout.tips.hashtag": "hashtag", - "search_popout.tips.status": "toot", + "search_popout.tips.status": "post", "search_popout.tips.text": "Simple text returns matching display names, usernames and hashtags", "search_popout.tips.user": "user", "search_results.accounts": "People", "search_results.hashtags": "Hashtags", - "search_results.statuses": "Toots", - "search_results.statuses_fts_disabled": "Searching toots by their content is not enabled on this Mastodon server.", + "search_results.statuses": "Posts", + "search_results.statuses_fts_disabled": "Searching posts by their content is not enabled on this Mastodon server.", "search_results.total": "{count, number} {count, plural, one {result} other {results}}", "status.admin_account": "Open moderation interface for @{name}", - "status.admin_status": "Open this toot in the moderation interface", + "status.admin_status": "Open this post in the moderation interface", "status.block": "Block @{name}", "status.bookmark": "Bookmark", "status.cancel_reblog_private": "Unboost", "status.cannot_reblog": "This post cannot be boosted", - "status.copy": "Copy link to toot", + "status.copy": "Copy link to post", "status.delete": "Delete", "status.detailed_status": "Detailed conversation view", "status.direct": "Direct message @{name}", @@ -408,14 +408,14 @@ "status.more": "More", "status.mute": "Mute @{name}", "status.mute_conversation": "Mute conversation", - "status.open": "Expand this toot", + "status.open": "Expand this post", "status.pin": "Pin on profile", - "status.pinned": "Pinned toot", + "status.pinned": "Pinned post", "status.read_more": "Read more", "status.reblog": "Boost", "status.reblog_private": "Boost with original visibility", "status.reblogged_by": "{name} boosted", - "status.reblogs.empty": "No one has boosted this toot yet. When someone does, they will show up here.", + "status.reblogs.empty": "No one has boosted this post yet. When someone does, they will show up here.", "status.redraft": "Delete & re-draft", "status.remove_bookmark": "Remove bookmark", "status.reply": "Reply", @@ -446,7 +446,7 @@ "timeline_hint.remote_resource_not_displayed": "{resource} from other servers are not displayed.", "timeline_hint.resources.followers": "Followers", "timeline_hint.resources.follows": "Follows", - "timeline_hint.resources.statuses": "Older toots", + "timeline_hint.resources.statuses": "Older posts", "trends.counter_by_accounts": "{count, plural, one {{counter} person} other {{counter} people}} talking", "trends.trending_now": "Trending now", "ui.beforeunload": "Your draft will be lost if you leave Mastodon.", diff --git a/config/locales/en.yml b/config/locales/en.yml index 7aebd0743..765f71250 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1,7 +1,7 @@ --- en: about: - about_hashtag_html: These are public toots tagged with #%{hashtag}. You can interact with them if you have an account anywhere in the fediverse. + about_hashtag_html: These are public posts tagged with #%{hashtag}. You can interact with them if you have an account anywhere in the fediverse. about_mastodon_html: 'The social network of the future: No ads, no corporate surveillance, ethical design, and decentralization! Own your data with Mastodon!' about_this: About active_count_after: active @@ -32,9 +32,9 @@ en: server_stats: 'Server stats:' source_code: Source code status_count_after: - one: status - other: statuses - status_count_before: Who authored + one: post + other: posts + status_count_before: Who published tagline: Follow friends and discover new ones terms: Terms of service unavailable_content: Moderated servers @@ -76,10 +76,10 @@ en: pin_errors: following: You must be already following the person you want to endorse posts: - one: Toot - other: Toots - posts_tab_heading: Toots - posts_with_replies: Toots and replies + one: Post + other: Posts + posts_tab_heading: Posts + posts_with_replies: Posts and replies roles: admin: Admin bot: Bot @@ -199,7 +199,7 @@ en: targeted_reports: Reported by others silence: Limit silenced: Limited - statuses: Statuses + statuses: Posts subscribe: Subscribe suspended: Suspended suspension_irreversible: The data of this account has been irreversibly deleted. You can unsuspend the account to make it usable but it will not recover any data it previously had. @@ -237,7 +237,7 @@ en: destroy_domain_block: Delete Domain Block destroy_email_domain_block: Delete e-mail domain block destroy_ip_block: Delete IP rule - destroy_status: Delete Status + destroy_status: Delete Post disable_2fa_user: Disable 2FA disable_custom_emoji: Disable Custom Emoji disable_user: Disable User @@ -259,7 +259,7 @@ en: update_announcement: Update Announcement update_custom_emoji: Update Custom Emoji update_domain_block: Update Domain Block - update_status: Update Status + update_status: Update Post actions: assigned_to_self_report_html: "%{name} assigned report %{target} to themselves" change_email_user_html: "%{name} changed the e-mail address of user %{target}" @@ -278,7 +278,7 @@ en: destroy_domain_block_html: "%{name} unblocked domain %{target}" destroy_email_domain_block_html: "%{name} unblocked e-mail domain %{target}" destroy_ip_block_html: "%{name} deleted rule for IP %{target}" - destroy_status_html: "%{name} removed status by %{target}" + destroy_status_html: "%{name} removed post by %{target}" disable_2fa_user_html: "%{name} disabled two factor requirement for user %{target}" disable_custom_emoji_html: "%{name} disabled emoji %{target}" disable_user_html: "%{name} disabled login for user %{target}" @@ -300,8 +300,8 @@ en: update_announcement_html: "%{name} updated announcement %{target}" update_custom_emoji_html: "%{name} updated emoji %{target}" update_domain_block_html: "%{name} updated domain block for %{target}" - update_status_html: "%{name} updated status by %{target}" - deleted_status: "(deleted status)" + update_status_html: "%{name} updated post by %{target}" + deleted_status: "(deleted post)" empty: No logs found. filter_by_action: Filter by action filter_by_user: Filter by user @@ -499,11 +499,11 @@ en: relays: add_new: Add new relay delete: Delete - description_html: A federation relay is an intermediary server that exchanges large volumes of public toots between servers that subscribe and publish to it. It can help small and medium servers discover content from the fediverse, which would otherwise require local users manually following other people on remote servers. + description_html: A federation relay is an intermediary server that exchanges large volumes of public posts between servers that subscribe and publish to it. It can help small and medium servers discover content from the fediverse, which would otherwise require local users manually following other people on remote servers. disable: Disable disabled: Disabled enable: Enable - enable_hint: Once enabled, your server will subscribe to all public toots from this relay, and will begin sending this server's public toots to it. + enable_hint: Once enabled, your server will subscribe to all public posts from this relay, and will begin sending this server's public posts to it. enabled: Enabled inbox_url: Relay URL pending: Waiting for relay's approval @@ -561,7 +561,7 @@ en: title: Server rules settings: activity_api_enabled: - desc_html: Counts of locally posted statuses, active users, and new registrations in weekly buckets + desc_html: Counts of locally published posts, active users, and new registrations in weekly buckets title: Publish aggregate statistics about user activity in the API bootstrap_timeline_accounts: desc_html: Separate multiple usernames by comma. Only local and unlocked accounts will work. Default when empty is all local admins. @@ -665,8 +665,8 @@ en: media: title: Media no_media: No media - no_status_selected: No statuses were changed as none were selected - title: Account statuses + no_status_selected: No posts were changed as none were selected + title: Account posts with_media: With media system_checks: database_schema_check: @@ -730,14 +730,14 @@ en: guide_link: https://crowdin.com/project/mastodon guide_link_text: Everyone can contribute. sensitive_content: Sensitive content - toot_layout: Toot layout + toot_layout: Post layout application_mailer: notification_preferences: Change e-mail preferences salutation: "%{name}," settings: 'Change e-mail preferences: %{link}' view: 'View:' view_profile: View profile - view_status: View status + view_status: View post applications: created: Application successfully created destroyed: Application successfully deleted @@ -874,7 +874,7 @@ en: archive_takeout: date: Date download: Download your archive - hint_html: You can request an archive of your toots and uploaded media. The exported data will be in the ActivityPub format, readable by any compliant software. You can request an archive every 7 days. + hint_html: You can request an archive of your posts and uploaded media. The exported data will be in the ActivityPub format, readable by any compliant software. You can request an archive every 7 days. in_progress: Compiling your archive... request: Request your archive size: Size @@ -991,7 +991,7 @@ en: limit: You have reached the maximum amount of lists media_attachments: validations: - images_and_video: Cannot attach a video to a status that already contains images + images_and_video: Cannot attach a video to a post that already contains images not_ready: Cannot attach files that have not finished processing. Try again in a moment! too_many: Cannot attach more than 4 files migrations: @@ -1044,8 +1044,8 @@ en: other: "%{count} new notifications since your last visit \U0001F418" title: In your absence... favourite: - body: 'Your status was favourited by %{name}:' - subject: "%{name} favourited your status" + body: 'Your post was favourited by %{name}:' + subject: "%{name} favourited your post" title: New favourite follow: body: "%{name} is now following you!" @@ -1064,8 +1064,8 @@ en: poll: subject: A poll by %{name} has ended reblog: - body: 'Your status was boosted by %{name}:' - subject: "%{name} boosted your status" + body: 'Your post was boosted by %{name}:' + subject: "%{name} boosted your post" title: New boost status: subject: "%{name} just posted" @@ -1144,16 +1144,16 @@ en: remote_interaction: favourite: proceed: Proceed to favourite - prompt: 'You want to favourite this toot:' + prompt: 'You want to favourite this post:' reblog: proceed: Proceed to boost - prompt: 'You want to boost this toot:' + prompt: 'You want to boost this post:' reply: proceed: Proceed to reply - prompt: 'You want to reply to this toot:' + prompt: 'You want to reply to this post:' scheduled_statuses: - over_daily_limit: You have exceeded the limit of %{limit} scheduled toots for that day - over_total_limit: You have exceeded the limit of %{limit} scheduled toots + over_daily_limit: You have exceeded the limit of %{limit} scheduled posts for that day + over_total_limit: You have exceeded the limit of %{limit} scheduled posts too_soon: The scheduled date must be in the future sessions: activity: Last activity @@ -1236,14 +1236,14 @@ en: one: 'contained a disallowed hashtag: %{tags}' other: 'contained the disallowed hashtags: %{tags}' errors: - in_reply_not_found: The status you are trying to reply to does not appear to exist. + in_reply_not_found: The post you are trying to reply to does not appear to exist. language_detection: Automatically detect language open_in_web: Open in web over_character_limit: character limit of %{max} exceeded pin_errors: - limit: You have already pinned the maximum number of toots - ownership: Someone else's toot cannot be pinned - private: Non-public toot cannot be pinned + limit: You have already pinned the maximum number of posts + ownership: Someone else's post cannot be pinned + private: Non-public posts cannot be pinned reblog: A boost cannot be pinned poll: total_people: @@ -1268,7 +1268,7 @@ en: unlisted: Unlisted unlisted_long: Everyone can see, but not listed on public timelines stream_entries: - pinned: Pinned toot + pinned: Pinned post reblogged: boosted sensitive_content: Sensitive content tags: @@ -1394,7 +1394,7 @@ en: explanation: disable: You can no longer login to your account or use it in any other way, but your profile and other data remains intact. sensitive: Your uploaded media files and linked media will be treated as sensitive. - silence: You can still use your account but only people who are already following you will see your toots on this server, and you may be excluded from various public listings. However, others may still manually follow you. + silence: You can still use your account but only people who are already following you will see your posts on this server, and you may be excluded from various public listings. However, others may still manually follow you. suspend: You can no longer use your account, and your profile and other data are no longer accessible. You can still login to request a backup of your data until the data is fully removed, but we will retain some data to prevent you from evading the suspension. get_in_touch: You can reply to this e-mail to get in touch with the staff of %{instance}. review_server_policies: Review server policies diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 38916b54f..3ae106218 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -59,12 +59,12 @@ RSpec.describe NotificationMailer, type: :mailer do include_examples 'localized subject', 'notification_mailer.favourite.subject', name: 'bob' it "renders the headers" do - expect(mail.subject).to eq("bob favourited your status") + expect(mail.subject).to eq("bob favourited your post") expect(mail.to).to eq([receiver.email]) end it "renders the body" do - expect(mail.body.encoded).to match("Your status was favourited by bob") + expect(mail.body.encoded).to match("Your post was favourited by bob") expect(mail.body.encoded).to include 'The body of the own status' end end @@ -76,12 +76,12 @@ RSpec.describe NotificationMailer, type: :mailer do include_examples 'localized subject', 'notification_mailer.reblog.subject', name: 'bob' it "renders the headers" do - expect(mail.subject).to eq("bob boosted your status") + expect(mail.subject).to eq("bob boosted your post") expect(mail.to).to eq([receiver.email]) end it "renders the body" do - expect(mail.body.encoded).to match("Your status was boosted by bob") + expect(mail.body.encoded).to match("Your post was boosted by bob") expect(mail.body.encoded).to include 'The body of the own status' end end -- cgit From a6564d56d6af0467eb8cfa96444b4503f437e0a6 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 23 Apr 2021 22:51:21 +0200 Subject: Fix edge case where accepted follow cannot be processed because of follow limit (#16098) --- app/models/follow_request.rb | 2 +- spec/models/follow_request_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index 59fefcdf6..0b6f7629a 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -29,7 +29,7 @@ class FollowRequest < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } def authorize! - account.follow!(target_account, reblogs: show_reblogs, notify: notify, uri: uri) + account.follow!(target_account, reblogs: show_reblogs, notify: notify, uri: uri, bypass_limit: true) MergeWorker.perform_async(target_account.id, account.id) if account.local? destroy! end diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index cc484a5b9..36ce8ee60 100644 --- a/spec/models/follow_request_spec.rb +++ b/spec/models/follow_request_spec.rb @@ -7,7 +7,7 @@ RSpec.describe FollowRequest, type: :model do let(:target_account) { Fabricate(:account) } it 'calls Account#follow!, MergeWorker.perform_async, and #destroy!' do - expect(account).to receive(:follow!).with(target_account, reblogs: true, notify: false, uri: follow_request.uri) + expect(account).to receive(:follow!).with(target_account, reblogs: true, notify: false, uri: follow_request.uri, bypass_limit: true) expect(MergeWorker).to receive(:perform_async).with(target_account.id, account.id) expect(follow_request).to receive(:destroy!) follow_request.authorize! -- cgit From daccc07dc170627b17564402296f6c8631d0cd97 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 24 Apr 2021 17:01:43 +0200 Subject: Change auto-following admin-selected accounts, show in recommendations (#16078) --- app/controllers/api/v1/suggestions_controller.rb | 10 ++-- app/lib/potential_friendship_tracker.rb | 10 ---- app/models/account_suggestions.rb | 25 +++++--- app/models/account_suggestions/global_source.rb | 37 ++++++++++++ .../past_interactions_source.rb | 36 ++++++++++++ app/models/account_suggestions/setting_source.rb | 68 ++++++++++++++++++++++ app/models/account_suggestions/source.rb | 34 +++++++++++ app/models/account_suggestions/suggestion.rb | 7 +++ app/models/follow_recommendation.rb | 15 ----- app/models/form/admin_settings.rb | 2 - app/services/bootstrap_timeline_service.rb | 37 +----------- app/validators/existing_username_validator.rb | 24 ++++++-- app/views/admin/settings/edit.html.haml | 5 +- config/locales/en.yml | 7 +-- config/settings.yml | 1 - spec/services/bootstrap_timeline_service_spec.rb | 38 ------------ 16 files changed, 228 insertions(+), 128 deletions(-) create mode 100644 app/models/account_suggestions/global_source.rb create mode 100644 app/models/account_suggestions/past_interactions_source.rb create mode 100644 app/models/account_suggestions/setting_source.rb create mode 100644 app/models/account_suggestions/source.rb create mode 100644 app/models/account_suggestions/suggestion.rb (limited to 'spec') diff --git a/app/controllers/api/v1/suggestions_controller.rb b/app/controllers/api/v1/suggestions_controller.rb index b2788cc76..9737ae5cb 100644 --- a/app/controllers/api/v1/suggestions_controller.rb +++ b/app/controllers/api/v1/suggestions_controller.rb @@ -5,20 +5,20 @@ class Api::V1::SuggestionsController < Api::BaseController before_action -> { doorkeeper_authorize! :read } before_action :require_user! - before_action :set_accounts def index - render json: @accounts, each_serializer: REST::AccountSerializer + suggestions = suggestions_source.get(current_account, limit: limit_param(DEFAULT_ACCOUNTS_LIMIT)) + render json: suggestions.map(&:account), each_serializer: REST::AccountSerializer end def destroy - PotentialFriendshipTracker.remove(current_account.id, params[:id]) + suggestions_source.remove(current_account, params[:id]) render_empty end private - def set_accounts - @accounts = PotentialFriendshipTracker.get(current_account, limit_param(DEFAULT_ACCOUNTS_LIMIT)) + def suggestions_source + AccountSuggestions::PastInteractionsSource.new end end diff --git a/app/lib/potential_friendship_tracker.rb b/app/lib/potential_friendship_tracker.rb index e72d454b6..f5bc20346 100644 --- a/app/lib/potential_friendship_tracker.rb +++ b/app/lib/potential_friendship_tracker.rb @@ -27,15 +27,5 @@ class PotentialFriendshipTracker def remove(account_id, target_account_id) redis.zrem("interactions:#{account_id}", target_account_id) end - - def get(account, limit) - account_ids = redis.zrevrange("interactions:#{account.id}", 0, limit) - - return [] if account_ids.empty? || limit < 1 - - accounts = Account.searchable.where(id: account_ids).index_by(&:id) - - account_ids.map { |id| accounts[id.to_i] }.compact - end end end diff --git a/app/models/account_suggestions.rb b/app/models/account_suggestions.rb index 7fe9d618e..d1774e62f 100644 --- a/app/models/account_suggestions.rb +++ b/app/models/account_suggestions.rb @@ -1,17 +1,28 @@ # frozen_string_literal: true class AccountSuggestions - class Suggestion < ActiveModelSerializers::Model - attributes :account, :source - end + SOURCES = [ + AccountSuggestions::SettingSource, + AccountSuggestions::PastInteractionsSource, + AccountSuggestions::GlobalSource, + ].freeze def self.get(account, limit) - suggestions = PotentialFriendshipTracker.get(account, limit).map { |target_account| Suggestion.new(account: target_account, source: :past_interaction) } - suggestions.concat(FollowRecommendation.get(account, limit - suggestions.size, suggestions.map { |suggestion| suggestion.account.id }).map { |target_account| Suggestion.new(account: target_account, source: :global) }) if suggestions.size < limit - suggestions + SOURCES.each_with_object([]) do |source_class, suggestions| + source_suggestions = source_class.new.get( + account, + skip_account_ids: suggestions.map(&:account_id), + limit: limit - suggestions.size + ) + + suggestions.concat(source_suggestions) + end end def self.remove(account, target_account_id) - PotentialFriendshipTracker.remove(account.id, target_account_id) + SOURCES.each do |source_class| + source = source_class.new + source.remove(account, target_account_id) + end end end diff --git a/app/models/account_suggestions/global_source.rb b/app/models/account_suggestions/global_source.rb new file mode 100644 index 000000000..ac764de50 --- /dev/null +++ b/app/models/account_suggestions/global_source.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class AccountSuggestions::GlobalSource < AccountSuggestions::Source + def key + :global + end + + def get(account, skip_account_ids: [], limit: 40) + account_ids = account_ids_for_locale(account.user_locale) - [account.id] - skip_account_ids + + as_ordered_suggestions( + scope(account).where(id: account_ids), + account_ids + ).take(limit) + end + + def remove(_account, _target_account_id) + nil + end + + private + + def scope(account) + Account.searchable + .followable_by(account) + .not_excluded_by_account(account) + .not_domain_blocked_by_account(account) + end + + def account_ids_for_locale(locale) + Redis.current.zrevrange("follow_recommendations:#{locale}", 0, -1).map(&:to_i) + end + + def to_ordered_list_key(account) + account.id + end +end diff --git a/app/models/account_suggestions/past_interactions_source.rb b/app/models/account_suggestions/past_interactions_source.rb new file mode 100644 index 000000000..d169394f1 --- /dev/null +++ b/app/models/account_suggestions/past_interactions_source.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class AccountSuggestions::PastInteractionsSource < AccountSuggestions::Source + include Redisable + + def key + :past_interactions + end + + def get(account, skip_account_ids: [], limit: 40) + account_ids = account_ids_for_account(account.id, limit + skip_account_ids.size) - skip_account_ids + + as_ordered_suggestions( + scope.where(id: account_ids), + account_ids + ).take(limit) + end + + def remove(account, target_account_id) + redis.zrem("interactions:#{account.id}", target_account_id) + end + + private + + def scope + Account.searchable + end + + def account_ids_for_account(account_id, limit) + redis.zrevrange("interactions:#{account_id}", 0, limit).map(&:to_i) + end + + def to_ordered_list_key(account) + account.id + end +end diff --git a/app/models/account_suggestions/setting_source.rb b/app/models/account_suggestions/setting_source.rb new file mode 100644 index 000000000..be9eff233 --- /dev/null +++ b/app/models/account_suggestions/setting_source.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +class AccountSuggestions::SettingSource < AccountSuggestions::Source + def key + :staff + end + + def get(account, skip_account_ids: [], limit: 40) + return [] unless setting_enabled? + + as_ordered_suggestions( + scope(account).where(setting_to_where_condition).where.not(id: skip_account_ids), + usernames_and_domains + ).take(limit) + end + + def remove(_account, _target_account_id) + nil + end + + private + + def scope(account) + Account.searchable + .followable_by(account) + .not_excluded_by_account(account) + .not_domain_blocked_by_account(account) + .where(locked: false) + .where.not(id: account.id) + end + + def usernames_and_domains + @usernames_and_domains ||= setting_to_usernames_and_domains + end + + def setting_enabled? + setting.present? + end + + def setting_to_where_condition + usernames_and_domains.map do |(username, domain)| + Arel::Nodes::Grouping.new( + Account.arel_table[:username].lower.eq(username.downcase).and( + Account.arel_table[:domain].lower.eq(domain&.downcase) + ) + ) + end.reduce(:or) + end + + def setting_to_usernames_and_domains + setting.split(',').map do |str| + username, domain = str.strip.gsub(/\A@/, '').split('@', 2) + domain = nil if TagManager.instance.local_domain?(domain) + + next if username.blank? + + [username, domain] + end.compact + end + + def setting + Setting.bootstrap_timeline_accounts + end + + def to_ordered_list_key(account) + [account.username, account.domain] + end +end diff --git a/app/models/account_suggestions/source.rb b/app/models/account_suggestions/source.rb new file mode 100644 index 000000000..bd1068d20 --- /dev/null +++ b/app/models/account_suggestions/source.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class AccountSuggestions::Source + def key + raise NotImplementedError + end + + def get(_account, **kwargs) + raise NotImplementedError + end + + def remove(_account, target_account_id) + raise NotImplementedError + end + + protected + + def as_ordered_suggestions(scope, ordered_list) + return [] if ordered_list.empty? + + map = scope.index_by(&method(:to_ordered_list_key)) + + ordered_list.map { |ordered_list_key| map[ordered_list_key] }.compact.map do |account| + AccountSuggestions::Suggestion.new( + account: account, + source: key + ) + end + end + + def to_ordered_list_key(_account) + raise NotImplementedError + end +end diff --git a/app/models/account_suggestions/suggestion.rb b/app/models/account_suggestions/suggestion.rb new file mode 100644 index 000000000..2c6f4d27f --- /dev/null +++ b/app/models/account_suggestions/suggestion.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AccountSuggestions::Suggestion < ActiveModelSerializers::Model + attributes :account, :source + + delegate :id, to: :account, prefix: true +end diff --git a/app/models/follow_recommendation.rb b/app/models/follow_recommendation.rb index c4355224d..6670b6560 100644 --- a/app/models/follow_recommendation.rb +++ b/app/models/follow_recommendation.rb @@ -21,19 +21,4 @@ class FollowRecommendation < ApplicationRecord def readonly? true end - - def self.get(account, limit, exclude_account_ids = []) - account_ids = Redis.current.zrevrange("follow_recommendations:#{account.user_locale}", 0, -1).map(&:to_i) - exclude_account_ids - [account.id] - - return [] if account_ids.empty? || limit < 1 - - accounts = Account.followable_by(account) - .not_excluded_by_account(account) - .not_domain_blocked_by_account(account) - .where(id: account_ids) - .limit(limit) - .index_by(&:id) - - account_ids.map { |id| accounts[id] }.compact - end end diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index b5c3dcdbe..6fc7c56fd 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -16,7 +16,6 @@ class Form::AdminSettings open_deletion timeline_preview show_staff_badge - enable_bootstrap_timeline_accounts bootstrap_timeline_accounts theme min_invite_role @@ -41,7 +40,6 @@ class Form::AdminSettings open_deletion timeline_preview show_staff_badge - enable_bootstrap_timeline_accounts activity_api_enabled peers_api_enabled show_known_fediverse_at_about_page diff --git a/app/services/bootstrap_timeline_service.rb b/app/services/bootstrap_timeline_service.rb index 8412aa7e7..e1a1b98c3 100644 --- a/app/services/bootstrap_timeline_service.rb +++ b/app/services/bootstrap_timeline_service.rb @@ -5,48 +5,13 @@ class BootstrapTimelineService < BaseService @source_account = source_account autofollow_inviter! - autofollow_bootstrap_timeline_accounts! if Setting.enable_bootstrap_timeline_accounts end private def autofollow_inviter! return unless @source_account&.user&.invite&.autofollow? - FollowService.new.call(@source_account, @source_account.user.invite.user.account) - end - - def autofollow_bootstrap_timeline_accounts! - bootstrap_timeline_accounts.each do |target_account| - begin - FollowService.new.call(@source_account, target_account) - rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError - nil - end - end - end - - def bootstrap_timeline_accounts - return @bootstrap_timeline_accounts if defined?(@bootstrap_timeline_accounts) - - @bootstrap_timeline_accounts = bootstrap_timeline_accounts_usernames.empty? ? admin_accounts : local_unlocked_accounts(bootstrap_timeline_accounts_usernames) - end - - def bootstrap_timeline_accounts_usernames - @bootstrap_timeline_accounts_usernames ||= (Setting.bootstrap_timeline_accounts || '').split(',').map { |str| str.strip.gsub(/\A@/, '') }.reject(&:blank?) - end - def admin_accounts - User.admins - .includes(:account) - .where(accounts: { locked: false }) - .map(&:account) - end - - def local_unlocked_accounts(usernames) - Account.local - .without_suspended - .where(username: usernames) - .where(locked: false) - .where(moved_to_account_id: nil) + FollowService.new.call(@source_account, @source_account.user.invite.user.account) end end diff --git a/app/validators/existing_username_validator.rb b/app/validators/existing_username_validator.rb index 723302ec9..afbe0c635 100644 --- a/app/validators/existing_username_validator.rb +++ b/app/validators/existing_username_validator.rb @@ -4,11 +4,25 @@ class ExistingUsernameValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) return if value.blank? - if options[:multiple] - missing_usernames = value.split(',').map { |username| username.strip.gsub(/\A@/, '') }.filter_map { |username| username unless Account.find_local(username) } - record.errors.add(attribute, I18n.t('existing_username_validator.not_found_multiple', usernames: missing_usernames.join(', '))) if missing_usernames.any? - else - record.errors.add(attribute, I18n.t('existing_username_validator.not_found')) unless Account.find_local(value.strip.gsub(/\A@/, '')) + usernames_and_domains = begin + value.split(',').map do |str| + username, domain = str.strip.gsub(/\A@/, '').split('@') + domain = nil if TagManager.instance.local_domain?(domain) + + next if username.blank? + + [str, username, domain] + end.compact + end + + usernames_with_no_accounts = usernames_and_domains.filter_map do |(str, username, domain)| + str unless Account.find_remote(username, domain) + end + + if usernames_with_no_accounts.any? && options[:multiple] + record.errors.add(attribute, I18n.t('existing_username_validator.not_found_multiple', usernames: usernames_with_no_accounts.join(', '))) + elsif usernames_with_no_accounts.any? || usernames_and_domains.size > 1 + record.errors.add(attribute, I18n.t('existing_username_validator.not_found')) end end end diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index 7783dbfeb..33bfc43d3 100644 --- a/app/views/admin/settings/edit.html.haml +++ b/app/views/admin/settings/edit.html.haml @@ -50,10 +50,7 @@ %hr.spacer/ .fields-group - = f.input :enable_bootstrap_timeline_accounts, as: :boolean, wrapper: :with_label, label: t('admin.settings.enable_bootstrap_timeline_accounts.title'), hint: t('admin.settings.enable_bootstrap_timeline_accounts.desc_html') - - .fields-group - = f.input :bootstrap_timeline_accounts, wrapper: :with_block_label, label: t('admin.settings.bootstrap_timeline_accounts.title'), hint: t('admin.settings.bootstrap_timeline_accounts.desc_html'), disabled: !Setting.enable_bootstrap_timeline_accounts + = f.input :bootstrap_timeline_accounts, wrapper: :with_block_label, label: t('admin.settings.bootstrap_timeline_accounts.title'), hint: t('admin.settings.bootstrap_timeline_accounts.desc_html') %hr.spacer/ diff --git a/config/locales/en.yml b/config/locales/en.yml index 765f71250..1b41ee063 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -564,8 +564,8 @@ en: desc_html: Counts of locally published posts, active users, and new registrations in weekly buckets title: Publish aggregate statistics about user activity in the API bootstrap_timeline_accounts: - desc_html: Separate multiple usernames by comma. Only local and unlocked accounts will work. Default when empty is all local admins. - title: Default follows for new users + desc_html: Separate multiple usernames by comma. These accounts will be guaranteed to be shown in follow recommendations + title: Recommend these accounts to new users contact_information: email: Business e-mail username: Contact username @@ -582,9 +582,6 @@ en: users: To logged-in local users domain_blocks_rationale: title: Show rationale - enable_bootstrap_timeline_accounts: - desc_html: Make new users automatically follow configured accounts so their home feed doesn't start out empty - title: Enable default follows for new users hero: desc_html: Displayed on the frontpage. At least 600x100px recommended. When not set, falls back to server thumbnail title: Hero image diff --git a/config/settings.yml b/config/settings.yml index b79ea620c..06cee2532 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -62,7 +62,6 @@ defaults: &defaults - mod - moderator disallowed_hashtags: # space separated string or list of hashtags without the hash - enable_bootstrap_timeline_accounts: true bootstrap_timeline_accounts: '' activity_api_enabled: true peers_api_enabled: true diff --git a/spec/services/bootstrap_timeline_service_spec.rb b/spec/services/bootstrap_timeline_service_spec.rb index a28d2407c..880ca4f0d 100644 --- a/spec/services/bootstrap_timeline_service_spec.rb +++ b/spec/services/bootstrap_timeline_service_spec.rb @@ -1,42 +1,4 @@ require 'rails_helper' RSpec.describe BootstrapTimelineService, type: :service do - subject { described_class.new } - - describe '#call' do - let(:source_account) { Fabricate(:account) } - - context 'when setting is empty' do - let!(:admin) { Fabricate(:user, admin: true) } - - before do - Setting.bootstrap_timeline_accounts = nil - subject.call(source_account) - end - - it 'follows admin accounts from account' do - expect(source_account.following?(admin.account)).to be true - end - end - - context 'when setting is set' do - let!(:alice) { Fabricate(:account, username: 'alice') } - let!(:bob) { Fabricate(:account, username: 'bob') } - let!(:eve) { Fabricate(:account, username: 'eve', suspended: true) } - - before do - Setting.bootstrap_timeline_accounts = 'alice, @bob, eve, unknown' - subject.call(source_account) - end - - it 'follows found accounts from account' do - expect(source_account.following?(alice)).to be true - expect(source_account.following?(bob)).to be true - end - - it 'does not follow suspended account' do - expect(source_account.following?(eve)).to be false - end - end - end end -- cgit From 7f0c49c58ab75bbd6b4dc44d47a8c4ab57db9d51 Mon Sep 17 00:00:00 2001 From: abcang Date: Sun, 25 Apr 2021 13:33:28 +0900 Subject: Improve tag search query (#16104) --- app/models/tag.rb | 14 ++++++-------- ...10421121431_add_case_insensitive_btree_index_to_tags.rb | 13 +++++++++++++ db/schema.rb | 4 ++-- spec/models/tag_spec.rb | 14 ++++++++++++++ 4 files changed, 35 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20210421121431_add_case_insensitive_btree_index_to_tags.rb (limited to 'spec') diff --git a/app/models/tag.rb b/app/models/tag.rb index bb93a52e2..efffc7eee 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -40,7 +40,8 @@ class Tag < ApplicationRecord scope :trendable, -> { Setting.trendable_by_default ? where(trendable: [true, nil]) : where(trendable: true) } scope :discoverable, -> { listable.joins(:account_tag_stat).where(AccountTagStat.arel_table[:accounts_count].gt(0)).order(Arel.sql('account_tag_stats.accounts_count desc')) } scope :recently_used, ->(account) { joins(:statuses).where(statuses: { id: account.statuses.select(:id).limit(1000) }).group(:id).order(Arel.sql('count(*) desc')) } - scope :matches_name, ->(value) { where(arel_table[:name].matches("#{value}%")) } + # Search with case-sensitive to use B-tree index. + scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } delegate :accounts_count, :accounts_count=, @@ -126,10 +127,9 @@ class Tag < ApplicationRecord end def search_for(term, limit = 5, offset = 0, options = {}) - normalized_term = normalize(term.strip) - pattern = sanitize_sql_like(normalized_term) + '%' - query = Tag.listable.where(arel_table[:name].lower.matches(pattern)) - query = query.where(arel_table[:name].lower.eq(normalized_term).or(arel_table[:reviewed_at].not_eq(nil))) if options[:exclude_unreviewed] + striped_term = term.strip + query = Tag.listable.matches_name(striped_term) + query = query.merge(matching_name(striped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed] query.order(Arel.sql('length(name) ASC, name ASC')) .limit(limit) @@ -145,7 +145,7 @@ class Tag < ApplicationRecord end def matching_name(name_or_names) - names = Array(name_or_names).map { |name| normalize(name).mb_chars.downcase.to_s } + names = Array(name_or_names).map { |name| arel_table.lower(normalize(name)) } if names.size == 1 where(arel_table[:name].lower.eq(names.first)) @@ -154,8 +154,6 @@ class Tag < ApplicationRecord end end - private - def normalize(str) str.gsub(/\A#/, '') end diff --git a/db/migrate/20210421121431_add_case_insensitive_btree_index_to_tags.rb b/db/migrate/20210421121431_add_case_insensitive_btree_index_to_tags.rb new file mode 100644 index 000000000..ed359e8cd --- /dev/null +++ b/db/migrate/20210421121431_add_case_insensitive_btree_index_to_tags.rb @@ -0,0 +1,13 @@ +class AddCaseInsensitiveBtreeIndexToTags < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + safety_assured { execute 'CREATE UNIQUE INDEX CONCURRENTLY index_tags_on_name_lower_btree ON tags (lower(name) text_pattern_ops)' } + remove_index :tags, name: 'index_tags_on_name_lower' + end + + def down + safety_assured { execute 'CREATE UNIQUE INDEX CONCURRENTLY index_tags_on_name_lower ON tags (lower(name))' } + remove_index :tags, name: 'index_tags_on_name_lower_btree' + end +end diff --git a/db/schema.rb b/db/schema.rb index dcbbf4aea..8dc0661fc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_04_16_200740) do +ActiveRecord::Schema.define(version: 2021_04_21_121431) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -862,7 +862,7 @@ ActiveRecord::Schema.define(version: 2021_04_16_200740) do t.datetime "last_status_at" t.float "max_score" t.datetime "max_score_at" - t.index "lower((name)::text)", name: "index_tags_on_name_lower", unique: true + t.index "lower((name)::text) text_pattern_ops", name: "index_tags_on_name_lower_btree", unique: true end create_table "tombstones", force: :cascade do |t| diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index df876593c..3949dbce5 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -96,6 +96,20 @@ RSpec.describe Tag, type: :model do end end + describe '.matches_name' do + it 'returns tags for multibyte case-insensitive names' do + upcase_string = 'abcABCａｂｃＡＢＣやゆよ' + downcase_string = 'abcabcａｂｃａｂｃやゆよ'; + + tag = Fabricate(:tag, name: downcase_string) + expect(Tag.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('100\\%abc%')] + end + end + describe '.matching_name' do it 'returns tags for multibyte case-insensitive names' do upcase_string = 'abcABCａｂｃＡＢＣやゆよ' -- cgit From 8c44b723bb7505eb5923019b454ce9abd7ea59c0 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 3 May 2021 15:45:19 +0200 Subject: Change confirmations controller to redirect to / for approved users (#16151) Clicking the confirmation link multiple times currently leads to entering account settings, which can be confusing. This commit changes that so that it redirects to the root path, so it behaves the same way as clicking only once in most cases. --- app/controllers/auth/confirmations_controller.rb | 4 +- .../auth/confirmations_controller_spec.rb | 46 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb index 898525269..1475bbcef 100644 --- a/app/controllers/auth/confirmations_controller.rb +++ b/app/controllers/auth/confirmations_controller.rb @@ -17,7 +17,9 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController private def require_unconfirmed! - redirect_to edit_user_registration_path if user_signed_in? && current_user.confirmed? && current_user.unconfirmed_email.blank? + if user_signed_in? && current_user.confirmed? && current_user.unconfirmed_email.blank? + redirect_to(current_user.approved? ? root_path : edit_user_registration_path) + end end def set_body_classes diff --git a/spec/controllers/auth/confirmations_controller_spec.rb b/spec/controllers/auth/confirmations_controller_spec.rb index 0b6b74ff9..8469119d2 100644 --- a/spec/controllers/auth/confirmations_controller_spec.rb +++ b/spec/controllers/auth/confirmations_controller_spec.rb @@ -32,6 +32,52 @@ describe Auth::ConfirmationsController, type: :controller do end end + context 'when user is unconfirmed and unapproved' do + let!(:user) { Fabricate(:user, confirmation_token: 'foobar', confirmed_at: nil, approved: false) } + + before do + allow(BootstrapTimelineWorker).to receive(:perform_async) + @request.env['devise.mapping'] = Devise.mappings[:user] + get :show, params: { confirmation_token: 'foobar' } + end + + it 'redirects to login' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'when user is already confirmed' do + let!(:user) { Fabricate(:user) } + + before do + allow(BootstrapTimelineWorker).to receive(:perform_async) + @request.env['devise.mapping'] = Devise.mappings[:user] + sign_in(user, scope: :user) + get :show, params: { confirmation_token: 'foobar' } + end + + it 'redirects to root path' do + expect(response).to redirect_to(root_path) + end + end + + context 'when user is already confirmed but unapproved' do + let!(:user) { Fabricate(:user, approved: false) } + + before do + allow(BootstrapTimelineWorker).to receive(:perform_async) + @request.env['devise.mapping'] = Devise.mappings[:user] + user.approved = false + user.save! + sign_in(user, scope: :user) + get :show, params: { confirmation_token: 'foobar' } + end + + it 'redirects to settings' do + expect(response).to redirect_to(edit_user_registration_path) + end + end + context 'when user is updating email' do let!(:user) { Fabricate(:user, confirmation_token: 'foobar', unconfirmed_email: 'new-email@example.com') } -- cgit From 566fc909134586d1746ad60ee455832dec6bc61a Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 6 May 2021 14:22:54 +0200 Subject: Add Ruby 3.0 support (#16046) * Fix issues with POSIX::Spawn, Terrapin and Ruby 3.0 Also improve the Terrapin monkey-patch for the stderr/stdout issue. * Fix keyword argument handling throughout the codebase * Monkey-patch Paperclip to fix keyword arguments handling in validators * Change validation_extensions to please CodeClimate * Bump microformats from 4.2.1 to 4.3.1 * Allow Ruby 3.0 * Add Ruby 3.0 test target to CircleCI * Add test for admin dashboard warnings * Fix admin dashboard warnings on Ruby 3.0 --- .circleci/config.yml | 27 +++++++ Gemfile | 2 +- Gemfile.lock | 6 +- app/controllers/activitypub/outboxes_controller.rb | 2 +- app/controllers/api/v1/accounts_controller.rb | 4 +- .../api/v1/follow_requests_controller.rb | 2 +- app/models/session_activation.rb | 2 +- app/models/user.rb | 19 +++-- app/views/admin/dashboard/index.html.haml | 2 +- app/workers/import/relationship_worker.rb | 6 +- config/application.rb | 1 + config/initializers/session_store.rb | 5 +- lib/paperclip/validation_extensions.rb | 58 +++++++++++++++ lib/terrapin/multi_pipe_extensions.rb | 87 +++++++++++----------- .../controllers/admin/dashboard_controller_spec.rb | 12 ++- spec/mailers/notification_mailer_spec.rb | 4 +- spec/mailers/user_mailer_spec.rb | 4 +- spec/models/session_activation_spec.rb | 6 +- .../account_relationships_presenter_spec.rb | 2 +- 19 files changed, 177 insertions(+), 74 deletions(-) create mode 100644 lib/paperclip/validation_extensions.rb (limited to 'spec') diff --git a/.circleci/config.yml b/.circleci/config.yml index 862fa126b..2f3860d7c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -129,6 +129,13 @@ jobs: environment: *ruby_environment <<: *install_ruby_dependencies + install-ruby3.0: + <<: *defaults + docker: + - image: circleci/ruby:3.0-buster-node + environment: *ruby_environment + <<: *install_ruby_dependencies + build: <<: *defaults steps: @@ -187,6 +194,18 @@ jobs: - image: circleci/redis:5-alpine <<: *test_steps + test-ruby3.0: + <<: *defaults + docker: + - image: circleci/ruby:3.0-buster-node + environment: *ruby_environment + - image: circleci/postgres:12.2 + environment: + POSTGRES_USER: root + POSTGRES_HOST_AUTH_METHOD: trust + - image: circleci/redis:5-alpine + <<: *test_steps + test-webui: <<: *defaults docker: @@ -227,6 +246,10 @@ workflows: requires: - install - install-ruby2.7 + - install-ruby3.0: + requires: + - install + - install-ruby2.7 - build: requires: - install-ruby2.7 @@ -241,6 +264,10 @@ workflows: requires: - install-ruby2.6 - build + - test-ruby3.0: + requires: + - install-ruby3.0 + - build - test-webui: requires: - install diff --git a/Gemfile b/Gemfile index 6ca0a81de..5a55d6b04 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,7 @@ # frozen_string_literal: true source 'https://rubygems.org' -ruby '>= 2.5.0', '< 3.0.0' +ruby '>= 2.5.0', '< 3.1.0' gem 'pkg-config', '~> 1.4' diff --git a/Gemfile.lock b/Gemfile.lock index b1ae4fd22..980750b63 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -292,7 +292,7 @@ GEM ipaddress (0.8.3) iso-639 (0.3.5) jmespath (1.4.0) - json (2.3.1) + json (2.5.1) json-canonicalization (0.2.1) json-ld (3.1.9) htmlentities (~> 4.3) @@ -344,7 +344,7 @@ GEM redis (>= 3.0.5) memory_profiler (1.0.0) method_source (1.0.0) - microformats (4.2.1) + microformats (4.3.1) json (~> 2.2) nokogiri (~> 1.10) mime-types (3.3.1) @@ -354,7 +354,7 @@ GEM nokogiri (~> 1) rake mini_mime (1.0.3) - mini_portile2 (2.5.0) + mini_portile2 (2.5.1) minitest (5.14.4) msgpack (1.4.2) multi_json (1.15.0) diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index 5fd735ad6..111285036 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -20,7 +20,7 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController def outbox_presenter if page_requested? ActivityPub::CollectionPresenter.new( - id: outbox_url(page_params), + id: outbox_url(**page_params), type: :ordered, part_of: outbox_url, prev: prev_page, diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 996f1b79b..95869f554 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -35,7 +35,7 @@ class Api::V1::AccountsController < Api::BaseController follow = FollowService.new.call(current_user.account, @account, reblogs: params.key?(:reblogs) ? truthy_param?(:reblogs) : nil, notify: params.key?(:notify) ? truthy_param?(:notify) : nil, with_rate_limit: true) options = @account.locked? || current_user.account.silenced? ? {} : { following_map: { @account.id => { reblogs: follow.show_reblogs?, notify: follow.notify? } }, requested_map: { @account.id => false } } - render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options) + render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(**options) end def block @@ -70,7 +70,7 @@ class Api::V1::AccountsController < Api::BaseController end def relationships(**options) - AccountRelationshipsPresenter.new([@account.id], current_user.account_id, options) + AccountRelationshipsPresenter.new([@account.id], current_user.account_id, **options) end def account_params diff --git a/app/controllers/api/v1/follow_requests_controller.rb b/app/controllers/api/v1/follow_requests_controller.rb index b34c76f29..f4b2a74d0 100644 --- a/app/controllers/api/v1/follow_requests_controller.rb +++ b/app/controllers/api/v1/follow_requests_controller.rb @@ -29,7 +29,7 @@ class Api::V1::FollowRequestsController < Api::BaseController end def relationships(**options) - AccountRelationshipsPresenter.new([params[:id]], current_user.account_id, options) + AccountRelationshipsPresenter.new([params[:id]], current_user.account_id, **options) end def load_accounts diff --git a/app/models/session_activation.rb b/app/models/session_activation.rb index b0ce9d112..3a59bad93 100644 --- a/app/models/session_activation.rb +++ b/app/models/session_activation.rb @@ -44,7 +44,7 @@ class SessionActivation < ApplicationRecord end def activate(**options) - activation = create!(options) + activation = create!(**options) purge_old activation end diff --git a/app/models/user.rb b/app/models/user.rb index 0440627c5..4973c68b6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -370,15 +370,20 @@ class User < ApplicationRecord protected - def send_devise_notification(notification, *args) + def send_devise_notification(notification, *args, **kwargs) # This method can be called in `after_update` and `after_commit` hooks, # but we must make sure the mailer is actually called *after* commit, # otherwise it may work on stale data. To do this, figure out if we are # within a transaction. + + # It seems like devise sends keyword arguments as a hash in the last + # positional argument + kwargs = args.pop if args.last.is_a?(Hash) && kwargs.empty? + if ActiveRecord::Base.connection.current_transaction.try(:records)&.include?(self) - pending_devise_notifications << [notification, args] + pending_devise_notifications << [notification, args, kwargs] else - render_and_send_devise_message(notification, *args) + render_and_send_devise_message(notification, *args, **kwargs) end end @@ -389,8 +394,8 @@ class User < ApplicationRecord end def send_pending_devise_notifications - pending_devise_notifications.each do |notification, args| - render_and_send_devise_message(notification, *args) + pending_devise_notifications.each do |notification, args, kwargs| + render_and_send_devise_message(notification, *args, **kwargs) end # Empty the pending notifications array because the @@ -403,8 +408,8 @@ class User < ApplicationRecord @pending_devise_notifications ||= [] end - def render_and_send_devise_message(notification, *args) - devise_mailer.send(notification, self, *args).deliver_later + def render_and_send_devise_message(notification, *args, **kwargs) + devise_mailer.send(notification, self, *args, **kwargs).deliver_later end def set_approved diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 57a753e6b..e8a2b46fd 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -5,7 +5,7 @@ .flash-message-stack - @system_checks.each do |message| .flash-message.warning - = t("admin.system_checks.#{message.key}.message_html", message.value ? { value: content_tag(:strong, message.value) } : {}) + = t("admin.system_checks.#{message.key}.message_html", value: message.value ? content_tag(:strong, message.value) : nil) - if message.action = link_to t("admin.system_checks.#{message.key}.action"), message.action diff --git a/app/workers/import/relationship_worker.rb b/app/workers/import/relationship_worker.rb index 4a7100435..6791b15c3 100644 --- a/app/workers/import/relationship_worker.rb +++ b/app/workers/import/relationship_worker.rb @@ -5,7 +5,7 @@ class Import::RelationshipWorker sidekiq_options queue: 'pull', retry: 8, dead: false - def perform(account_id, target_account_uri, relationship, options = {}) + def perform(account_id, target_account_uri, relationship, options) from_account = Account.find(account_id) target_domain = domain(target_account_uri) target_account = stoplight_wrap_request(target_domain) { ResolveAccountService.new.call(target_account_uri, { check_delivery_availability: true }) } @@ -16,7 +16,7 @@ class Import::RelationshipWorker case relationship when 'follow' begin - FollowService.new.call(from_account, target_account, options) + FollowService.new.call(from_account, target_account, **options) rescue ActiveRecord::RecordInvalid raise if FollowLimitValidator.limit_for_account(from_account) < from_account.following_count end @@ -27,7 +27,7 @@ class Import::RelationshipWorker when 'unblock' UnblockService.new.call(from_account, target_account) when 'mute' - MuteService.new.call(from_account, target_account, options) + MuteService.new.call(from_account, target_account, **options) when 'unmute' UnmuteService.new.call(from_account, target_account) end diff --git a/config/application.rb b/config/application.rb index 37a996224..08a4e4c97 100644 --- a/config/application.rb +++ b/config/application.rb @@ -10,6 +10,7 @@ require_relative '../lib/exceptions' require_relative '../lib/enumerable' require_relative '../lib/sanitize_ext/sanitize_config' require_relative '../lib/redis/namespace_extensions' +require_relative '../lib/paperclip/validation_extensions' require_relative '../lib/paperclip/url_generator_extensions' require_relative '../lib/paperclip/attachment_extensions' require_relative '../lib/paperclip/media_type_spoof_detector_extensions' diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index e5d1be4c6..3d9bf96fd 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -1,7 +1,6 @@ # Be sure to restart your server when you modify this file. -Rails.application.config.session_store :cookie_store, { +Rails.application.config.session_store :cookie_store, key: '_mastodon_session', secure: (Rails.env.production? || ENV['LOCAL_HTTPS'] == 'true'), - same_site: :lax, -} + same_site: :lax diff --git a/lib/paperclip/validation_extensions.rb b/lib/paperclip/validation_extensions.rb new file mode 100644 index 000000000..0df0434f6 --- /dev/null +++ b/lib/paperclip/validation_extensions.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +# Monkey-patch various Paperclip validators for Ruby 3.0 compatibility + +module Paperclip + module Validators + module AttachmentSizeValidatorExtensions + def validate_each(record, attr_name, _value) + base_attr_name = attr_name + attr_name = "#{attr_name}_file_size".to_sym + value = record.send(:read_attribute_for_validation, attr_name) + + if value.present? + options.slice(*Paperclip::Validators::AttachmentSizeValidator::AVAILABLE_CHECKS).each do |option, option_value| + option_value = option_value.call(record) if option_value.is_a?(Proc) + option_value = extract_option_value(option, option_value) + + next if value.send(Paperclip::Validators::AttachmentSizeValidator::CHECKS[option], option_value) + + error_message_key = options[:in] ? :in_between : option + [attr_name, base_attr_name].each do |error_attr_name| + record.errors.add(error_attr_name, error_message_key, **filtered_options(value).merge( + min: min_value_in_human_size(record), + max: max_value_in_human_size(record), + count: human_size(option_value) + )) + end + end + end + end + end + + module AttachmentContentTypeValidatorExtensions + def mark_invalid(record, attribute, types) + record.errors.add attribute, :invalid, **options.merge({ types: types.join(', ') }) + end + end + + module AttachmentPresenceValidatorExtensions + def validate_each(record, attribute, _value) + if record.send("#{attribute}_file_name").blank? + record.errors.add(attribute, :blank, **options) + end + end + end + + module AttachmentFileNameValidatorExtensions + def mark_invalid(record, attribute, patterns) + record.errors.add attribute, :invalid, options.merge({ names: patterns.join(', ') }) + end + end + end +end + +Paperclip::Validators::AttachmentSizeValidator.prepend(Paperclip::Validators::AttachmentSizeValidatorExtensions) +Paperclip::Validators::AttachmentContentTypeValidator.prepend(Paperclip::Validators::AttachmentContentTypeValidatorExtensions) +Paperclip::Validators::AttachmentPresenceValidator.prepend(Paperclip::Validators::AttachmentPresenceValidatorExtensions) +Paperclip::Validators::AttachmentFileNameValidator.prepend(Paperclip::Validators::AttachmentFileNameValidatorExtensions) diff --git a/lib/terrapin/multi_pipe_extensions.rb b/lib/terrapin/multi_pipe_extensions.rb index 51d7de37c..209f4ad6c 100644 --- a/lib/terrapin/multi_pipe_extensions.rb +++ b/lib/terrapin/multi_pipe_extensions.rb @@ -1,61 +1,64 @@ # frozen_string_literal: false -# Fix adapted from https://github.com/thoughtbot/terrapin/pull/5 + +require 'fcntl' module Terrapin module MultiPipeExtensions - def read - read_streams(@stdout_in, @stderr_in) - end + def initialize + @stdout_in, @stdout_out = IO.pipe + @stderr_in, @stderr_out = IO.pipe - def close_read - begin - @stdout_in.close - rescue IOError - # Do nothing - end - - begin - @stderr_in.close - rescue IOError - # Do nothing - end + clear_nonblocking_flags! end - def read_streams(output, error) - @stdout_output = '' - @stderr_output = '' + def pipe_options + # Add some flags to explicitly close the other end of the pipes + { out: @stdout_out, err: @stderr_out, @stdout_in => :close, @stderr_in => :close } + end - read_fds = [output, error] + def read + # While we are patching Terrapin, fix child process potentially getting stuck on writing + # to stderr. - until read_fds.empty? - to_read, = IO.select(read_fds) + @stdout_output = +'' + @stderr_output = +'' - if to_read.include?(output) - @stdout_output << read_stream(output) - read_fds.delete(output) if output.closed? - end + fds_to_read = [@stdout_in, @stderr_in] + until fds_to_read.empty? + rs, = IO.select(fds_to_read) - if to_read.include?(error) - @stderr_output << read_stream(error) - read_fds.delete(error) if error.closed? - end + read_nonblocking!(@stdout_in, @stdout_output, fds_to_read) if rs.include?(@stdout_in) + read_nonblocking!(@stderr_in, @stderr_output, fds_to_read) if rs.include?(@stderr_in) end end - def read_stream(io) - result = '' - - begin - while (partial_result = io.read_nonblock(8192)) - result << partial_result - end - rescue EOFError, Errno::EPIPE - io.close - rescue Errno::EINTR, Errno::EWOULDBLOCK, Errno::EAGAIN - # Do nothing + private + + # @param [IO] io IO Stream to read until there is nothing to read + # @param [String] result Mutable string to which read values will be appended to + # @param [Array] fds_to_read Mutable array from which `io` should be removed on EOF + def read_nonblocking!(io, result, fds_to_read) + while (partial_result = io.read_nonblock(8192)) + result << partial_result end + rescue IO::WaitReadable + # Do nothing + rescue EOFError + fds_to_read.delete(io) + end + + def clear_nonblocking_flags! + # Ruby 3.0 sets pipes to non-blocking mode, and resets the flags as + # needed when calling fork/exec-related syscalls, but posix-spawn does + # not currently do that, so we need to do it manually for the time being + # so that the child process do not error out when the buffers are full. + stdout_flags = @stdout_out.fcntl(Fcntl::F_GETFL) + @stdout_out.fcntl(Fcntl::F_SETFL, stdout_flags & ~Fcntl::O_NONBLOCK) if stdout_flags & Fcntl::O_NONBLOCK - result + stderr_flags = @stderr_out.fcntl(Fcntl::F_GETFL) + @stderr_out.fcntl(Fcntl::F_SETFL, stderr_flags & ~Fcntl::O_NONBLOCK) if stderr_flags & Fcntl::O_NONBLOCK + rescue NameError, NotImplementedError, Errno::EINVAL + # Probably on windows, where pipes are blocking by default end end end diff --git a/spec/controllers/admin/dashboard_controller_spec.rb b/spec/controllers/admin/dashboard_controller_spec.rb index 73b50e721..7824854f9 100644 --- a/spec/controllers/admin/dashboard_controller_spec.rb +++ b/spec/controllers/admin/dashboard_controller_spec.rb @@ -3,9 +3,19 @@ require 'rails_helper' describe Admin::DashboardController, type: :controller do + render_views + describe 'GET #index' do - it 'returns 200' do + before do + allow(Admin::SystemCheck).to receive(:perform).and_return([ + Admin::SystemCheck::Message.new(:database_schema_check), + Admin::SystemCheck::Message.new(:rules_check, nil, admin_rules_path), + Admin::SystemCheck::Message.new(:sidekiq_process_check, 'foo, bar'), + ]) sign_in Fabricate(:user, admin: true) + end + + it 'returns 200' do get :index expect(response).to have_http_status(200) diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 3ae106218..9b645bad8 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -10,12 +10,12 @@ RSpec.describe NotificationMailer, type: :mailer do it 'renders subject localized for the locale of the receiver' do locale = %i(de en).sample receiver.update!(locale: locale) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: locale)) end it 'renders subject localized for the default locale if the locale of the receiver is unavailable' do receiver.update!(locale: nil) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: I18n.default_locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: I18n.default_locale)) end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 6b430b505..9c866788f 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -9,12 +9,12 @@ describe UserMailer, type: :mailer do it 'renders subject localized for the locale of the receiver' do locale = I18n.available_locales.sample receiver.update!(locale: locale) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: locale)) end it 'renders subject localized for the default locale if the locale of the receiver is unavailable' do receiver.update!(locale: nil) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: I18n.default_locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: I18n.default_locale)) end end diff --git a/spec/models/session_activation_spec.rb b/spec/models/session_activation_spec.rb index 2aa695037..450dc1399 100644 --- a/spec/models/session_activation_spec.rb +++ b/spec/models/session_activation_spec.rb @@ -74,13 +74,13 @@ RSpec.describe SessionActivation, type: :model do let(:options) { { user: Fabricate(:user), session_id: '1' } } it 'calls create! and purge_old' do - expect(described_class).to receive(:create!).with(options) + expect(described_class).to receive(:create!).with(**options) expect(described_class).to receive(:purge_old) - described_class.activate(options) + described_class.activate(**options) end it 'returns an instance of SessionActivation' do - expect(described_class.activate(options)).to be_kind_of SessionActivation + expect(described_class.activate(**options)).to be_kind_of SessionActivation end end diff --git a/spec/presenters/account_relationships_presenter_spec.rb b/spec/presenters/account_relationships_presenter_spec.rb index f8b048d38..edfbbb354 100644 --- a/spec/presenters/account_relationships_presenter_spec.rb +++ b/spec/presenters/account_relationships_presenter_spec.rb @@ -13,7 +13,7 @@ RSpec.describe AccountRelationshipsPresenter do allow(Account).to receive(:domain_blocking_map).with(account_ids, current_account_id).and_return(default_map) end - let(:presenter) { AccountRelationshipsPresenter.new(account_ids, current_account_id, options) } + let(:presenter) { AccountRelationshipsPresenter.new(account_ids, current_account_id, **options) } let(:current_account_id) { Fabricate(:account).id } let(:account_ids) { [Fabricate(:account).id] } let(:default_map) { { 1 => true } } -- cgit From 2c77d97e0d59e045a9b04fccc83f0f24d190d8d8 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 7 May 2021 14:33:19 +0200 Subject: Add joined date to profiles in web UI (#16169) --- .../mastodon/features/account/components/header.js | 2 ++ app/javascript/mastodon/locales/defaultMessages.json | 14 +++++++++----- app/javascript/styles/mastodon/components.scss | 11 +++++++++++ app/serializers/activitypub/actor_serializer.rb | 6 +++++- app/serializers/rest/account_serializer.rb | 4 ++++ app/services/activitypub/process_account_service.rb | 3 ++- spec/lib/activitypub/activity/update_spec.rb | 2 +- 7 files changed, 34 insertions(+), 8 deletions(-) (limited to 'spec') diff --git a/app/javascript/mastodon/features/account/components/header.js b/app/javascript/mastodon/features/account/components/header.js index 8e49486bd..20641121f 100644 --- a/app/javascript/mastodon/features/account/components/header.js +++ b/app/javascript/mastodon/features/account/components/header.js @@ -326,6 +326,8 @@ class Header extends ImmutablePureComponent { {account.get('id') !== me && !suspended && } {account.get('note').length > 0 && account.get('note') !== '

' &&
} + +
{!suspended && ( diff --git a/app/javascript/mastodon/locales/defaultMessages.json b/app/javascript/mastodon/locales/defaultMessages.json index 86cf83403..172fae81f 100644 --- a/app/javascript/mastodon/locales/defaultMessages.json +++ b/app/javascript/mastodon/locales/defaultMessages.json @@ -909,6 +909,10 @@ { "defaultMessage": "Group", "id": "account.badges.group" + }, + { + "defaultMessage": "Joined {date}", + "id": "account.joined" } ], "path": "app/javascript/mastodon/features/account/components/header.json" @@ -1919,12 +1923,12 @@ "id": "home.hide_announcements" }, { - "defaultMessage": "Your home timeline is empty! Visit {public} or use search to get started and meet other users.", + "defaultMessage": "Your home timeline is empty! Follow more people to fill it up. {suggestions}", "id": "empty_column.home" }, { - "defaultMessage": "the public timeline", - "id": "empty_column.home.public_timeline" + "defaultMessage": "See some suggestions", + "id": "empty_column.home.suggestions" } ], "path": "app/javascript/mastodon/features/home_timeline/index.json" @@ -2417,7 +2421,7 @@ "id": "notifications.mark_as_read" }, { - "defaultMessage": "You don't have any notifications yet. Interact with others to start the conversation.", + "defaultMessage": "You don't have any notifications yet. When other people interact with you, you will see it here.", "id": "empty_column.notifications" } ], @@ -3249,4 +3253,4 @@ ], "path": "app/javascript/mastodon/features/video/index.json" } -] \ No newline at end of file +] diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 74a181603..d3dd1af60 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -6769,6 +6769,17 @@ noscript { } } + .account__header__joined { + font-size: 14px; + padding: 5px 15px; + color: $darker-text-color; + + .columns-area--mobile & { + padding-left: 20px; + padding-right: 20px; + } + } + .account__header__fields { margin: 0; border-top: 1px solid lighten($ui-base-color, 12%); diff --git a/app/serializers/activitypub/actor_serializer.rb b/app/serializers/activitypub/actor_serializer.rb index 759ef30f9..d92aae7b3 100644 --- a/app/serializers/activitypub/actor_serializer.rb +++ b/app/serializers/activitypub/actor_serializer.rb @@ -13,7 +13,7 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer :inbox, :outbox, :featured, :featured_tags, :preferred_username, :name, :summary, :url, :manually_approves_followers, - :discoverable + :discoverable, :published has_one :public_key, serializer: ActivityPub::PublicKeySerializer @@ -158,6 +158,10 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer !object.suspended? && !object.also_known_as.empty? end + def published + object.created_at.midnight.iso8601 + end + class CustomEmojiSerializer < ActivityPub::EmojiSerializer end diff --git a/app/serializers/rest/account_serializer.rb b/app/serializers/rest/account_serializer.rb index 189a62d0e..219f8075a 100644 --- a/app/serializers/rest/account_serializer.rb +++ b/app/serializers/rest/account_serializer.rb @@ -55,6 +55,10 @@ class REST::AccountSerializer < ActiveModel::Serializer full_asset_url(object.suspended? ? object.header.default_url : object.header_static_url) end + def created_at + object.created_at.midnight.iso8601 + end + def last_status_at object.last_status_at&.to_date&.iso8601 end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 6afeb92d6..bb2e8f665 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -87,6 +87,7 @@ class ActivityPub::ProcessAccountService < BaseService @account.url = url || @uri @account.uri = @uri @account.actor_type = actor_type + @account.created_at = @json['published'] if @json['published'].present? end def set_immediate_attributes! @@ -101,7 +102,7 @@ class ActivityPub::ProcessAccountService < BaseService end def set_fetchable_key! - @account.public_key = public_key || '' + @account.public_key = public_key || '' end def set_fetchable_attributes! diff --git a/spec/lib/activitypub/activity/update_spec.rb b/spec/lib/activitypub/activity/update_spec.rb index 42da29860..1c9bcf43b 100644 --- a/spec/lib/activitypub/activity/update_spec.rb +++ b/spec/lib/activitypub/activity/update_spec.rb @@ -13,7 +13,7 @@ RSpec.describe ActivityPub::Activity::Update do end let(:modified_sender) do - sender.dup.tap do |modified_sender| + sender.tap do |modified_sender| modified_sender.display_name = 'Totally modified now' end end -- cgit From 74081433d0078784b7c2139f6caaa812740632b2 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 7 May 2021 14:33:43 +0200 Subject: Change trending hashtags to be affected be reblogs (#16164) If a status with a hashtag becomes very popular, it stands to reason that the hashtag should have a chance at trending Fix no stats being recorded for hashtags that are not allowed to trend, and stop ignoring bots Remove references to hashtags in profile directory from the code and the admin UI --- app/controllers/directories_controller.rb | 10 ------ app/lib/activitypub/activity/announce.rb | 4 +++ app/lib/activitypub/activity/create.rb | 2 +- app/models/account.rb | 11 ++----- app/models/account_tag_stat.rb | 24 -------------- app/models/tag.rb | 38 +++++----------------- app/models/tag_filter.rb | 2 -- app/models/trending_tags.rb | 12 ++++--- app/services/process_hashtags_service.rb | 3 +- app/services/reblog_service.rb | 11 +++++++ app/views/admin/tags/_tag.html.haml | 2 -- app/views/admin/tags/index.html.haml | 9 ++--- app/views/admin/tags/show.html.haml | 13 ++------ config/locales/en.yml | 7 ++-- config/locales/simple_form.en.yml | 2 +- config/routes.rb | 2 -- .../20210502233513_drop_account_tag_stats.rb | 13 ++++++++ db/schema.rb | 10 ------ spec/models/account_tag_stat_spec.rb | 38 ---------------------- spec/models/trending_tags_spec.rb | 6 ++-- 20 files changed, 59 insertions(+), 160 deletions(-) delete mode 100644 app/models/account_tag_stat.rb create mode 100644 db/post_migrate/20210502233513_drop_account_tag_stats.rb delete mode 100644 spec/models/account_tag_stat_spec.rb (limited to 'spec') diff --git a/app/controllers/directories_controller.rb b/app/controllers/directories_controller.rb index f198ad5ba..f28c5b2af 100644 --- a/app/controllers/directories_controller.rb +++ b/app/controllers/directories_controller.rb @@ -6,7 +6,6 @@ class DirectoriesController < ApplicationController before_action :authenticate_user!, if: :whitelist_mode? before_action :require_enabled! before_action :set_instance_presenter - before_action :set_tag, only: :show before_action :set_accounts skip_before_action :require_functional!, unless: :whitelist_mode? @@ -15,23 +14,14 @@ class DirectoriesController < ApplicationController render :index end - def show - render :index - end - private def require_enabled! return not_found unless Setting.profile_directory end - def set_tag - @tag = Tag.discoverable.find_normalized!(params[:id]) - end - def set_accounts @accounts = Account.local.discoverable.by_recent_status.page(params[:page]).per(20).tap do |query| - query.merge!(Account.tagged_with(@tag.id)) if @tag query.merge!(Account.not_excluded_by_account(current_account)) if current_account end end diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index a1081522e..9f778ffb9 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -22,6 +22,10 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity visibility: visibility_from_audience ) + original_status.tags.each do |tag| + tag.use!(@account) + end + distribute(@status) end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index c7a655d9d..e46361c14 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -164,7 +164,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def attach_tags(status) @tags.each do |tag| status.tags << tag - TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility? + tag.use!(@account, status: status, at_time: status.created_at) if status.public_visibility? end @mentions.each do |mention| diff --git a/app/models/account.rb b/app/models/account.rb index a573365de..994459338 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -111,7 +111,6 @@ class Account < ApplicationRecord scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) } scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) } scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) } - scope :tagged_with, ->(tag) { joins(:accounts_tags).where(accounts_tags: { tag_id: tag }) } scope :by_recent_status, -> { order(Arel.sql('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc, accounts.id desc')) } scope :by_recent_sign_in, -> { order(Arel.sql('(case when users.current_sign_in_at is null then 1 else 0 end) asc, users.current_sign_in_at desc, accounts.id desc')) } scope :popular, -> { order('account_stats.followers_count desc') } @@ -279,19 +278,13 @@ class Account < ApplicationRecord if hashtags_map.key?(tag.name) hashtags_map.delete(tag.name) else - transaction do - tags.delete(tag) - tag.decrement_count!(:accounts_count) - end + tags.delete(tag) end end # Add hashtags that were so far missing hashtags_map.each_value do |tag| - transaction do - tags << tag - tag.increment_count!(:accounts_count) - end + tags << tag end end diff --git a/app/models/account_tag_stat.rb b/app/models/account_tag_stat.rb deleted file mode 100644 index 3c36c155a..000000000 --- a/app/models/account_tag_stat.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true -# == Schema Information -# -# Table name: account_tag_stats -# -# id :bigint(8) not null, primary key -# tag_id :bigint(8) not null -# accounts_count :bigint(8) default(0), not null -# hidden :boolean default(FALSE), not null -# created_at :datetime not null -# updated_at :datetime not null -# - -class AccountTagStat < ApplicationRecord - belongs_to :tag, inverse_of: :account_tag_stat - - def increment_count!(key) - update(key => public_send(key) + 1) - end - - def decrement_count!(key) - update(key => [public_send(key) - 1, 0].max) - end -end diff --git a/app/models/tag.rb b/app/models/tag.rb index efffc7eee..735c30608 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -20,10 +20,8 @@ class Tag < ApplicationRecord has_and_belongs_to_many :statuses has_and_belongs_to_many :accounts - has_and_belongs_to_many :sample_accounts, -> { local.discoverable.popular.limit(3) }, class_name: 'Account' has_many :featured_tags, dependent: :destroy, inverse_of: :tag - has_one :account_tag_stat, dependent: :destroy HASHTAG_SEPARATORS = "_\u00B7\u200c" HASHTAG_NAME_RE = "([[:word:]_][[:word:]#{HASHTAG_SEPARATORS}]*[[:alpha:]#{HASHTAG_SEPARATORS}][[:word:]#{HASHTAG_SEPARATORS}]*[[:word:]_])|([[:word:]_]*[[:alpha:]][[:word:]_]*)" @@ -38,29 +36,11 @@ class Tag < ApplicationRecord scope :usable, -> { where(usable: [true, nil]) } scope :listable, -> { where(listable: [true, nil]) } scope :trendable, -> { Setting.trendable_by_default ? where(trendable: [true, nil]) : where(trendable: true) } - scope :discoverable, -> { listable.joins(:account_tag_stat).where(AccountTagStat.arel_table[:accounts_count].gt(0)).order(Arel.sql('account_tag_stats.accounts_count desc')) } scope :recently_used, ->(account) { joins(:statuses).where(statuses: { id: account.statuses.select(:id).limit(1000) }).group(:id).order(Arel.sql('count(*) desc')) } - # Search with case-sensitive to use B-tree index. - scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } - - delegate :accounts_count, - :accounts_count=, - :increment_count!, - :decrement_count!, - to: :account_tag_stat - - after_save :save_account_tag_stat + scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } # Search with case-sensitive to use B-tree index update_index('tags#tag', :self) - def account_tag_stat - super || build_account_tag_stat - end - - def cached_sample_accounts - Rails.cache.fetch("#{cache_key}/sample_accounts", expires_in: 12.hours) { sample_accounts } - end - def to_param name end @@ -95,6 +75,10 @@ class Tag < ApplicationRecord requested_review_at.present? end + def use!(account, status: nil, at_time: Time.now.utc) + TrendingTags.record_use!(self, account, status: status, at_time: at_time) + end + def trending? TrendingTags.trending?(self) end @@ -127,9 +111,10 @@ class Tag < ApplicationRecord end def search_for(term, limit = 5, offset = 0, options = {}) - striped_term = term.strip - query = Tag.listable.matches_name(striped_term) - query = query.merge(matching_name(striped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed] + stripped_term = term.strip + + query = Tag.listable.matches_name(stripped_term) + query = query.merge(matching_name(stripped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed] query.order(Arel.sql('length(name) ASC, name ASC')) .limit(limit) @@ -161,11 +146,6 @@ class Tag < ApplicationRecord private - def save_account_tag_stat - return unless account_tag_stat&.changed? - account_tag_stat.save - end - def validate_name_change errors.add(:name, I18n.t('tags.does_not_match_previous_name')) unless name_was.mb_chars.casecmp(name.mb_chars).zero? end diff --git a/app/models/tag_filter.rb b/app/models/tag_filter.rb index a9ff5b703..85bfcbea5 100644 --- a/app/models/tag_filter.rb +++ b/app/models/tag_filter.rb @@ -33,8 +33,6 @@ class TagFilter def scope_for(key, value) case key.to_s - when 'directory' - Tag.discoverable when 'reviewed' Tag.reviewed.order(reviewed_at: :desc) when 'unreviewed' diff --git a/app/models/trending_tags.rb b/app/models/trending_tags.rb index 9c2aa0ee8..31890b082 100644 --- a/app/models/trending_tags.rb +++ b/app/models/trending_tags.rb @@ -13,19 +13,23 @@ class TrendingTags class << self include Redisable - def record_use!(tag, account, at_time = Time.now.utc) - return if account.silenced? || account.bot? || !tag.usable? || !(tag.trendable? || tag.requires_review?) + def record_use!(tag, account, status: nil, at_time: Time.now.utc) + return unless tag.usable? && !account.silenced? + # Even if a tag is not allowed to trend, we still need to + # record the stats since they can be displayed in other places increment_historical_use!(tag.id, at_time) increment_unique_use!(tag.id, account.id, at_time) increment_use!(tag.id, at_time) - tag.update(last_status_at: Time.now.utc) if tag.last_status_at.nil? || tag.last_status_at < 12.hours.ago + # Only update when the tag was last used once every 12 hours + # and only if a status is given (lets use ignore reblogs) + tag.update(last_status_at: at_time) if status.present? && (tag.last_status_at.nil? || (tag.last_status_at < at_time && tag.last_status_at < 12.hours.ago)) end def update!(at_time = Time.now.utc) tag_ids = redis.smembers("#{KEY}:used:#{at_time.beginning_of_day.to_i}") + redis.zrange(KEY, 0, -1) - tags = Tag.where(id: tag_ids.uniq) + tags = Tag.trendable.where(id: tag_ids.uniq) # First pass to calculate scores and update the set diff --git a/app/services/process_hashtags_service.rb b/app/services/process_hashtags_service.rb index e8e139b05..c42b79db8 100644 --- a/app/services/process_hashtags_service.rb +++ b/app/services/process_hashtags_service.rb @@ -8,8 +8,7 @@ class ProcessHashtagsService < BaseService Tag.find_or_create_by_names(tags) do |tag| status.tags << tag records << tag - - TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility? + tag.use!(status.account, status: status, at_time: status.created_at) if status.public_visibility? end return unless status.distributable? diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb index 5032397b3..744bdf567 100644 --- a/app/services/reblog_service.rb +++ b/app/services/reblog_service.rb @@ -35,6 +35,7 @@ class ReblogService < BaseService create_notification(reblog) bump_potential_friendship(account, reblog) + record_use(account, reblog) reblog end @@ -59,6 +60,16 @@ class ReblogService < BaseService PotentialFriendshipTracker.record(account.id, reblog.reblog.account_id, :reblog) end + def record_use(account, reblog) + return unless reblog.public_visibility? + + original_status = reblog.reblog + + original_status.tags.each do |tag| + tag.use!(account) + end + end + def build_json(reblog) Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(reblog), ActivityPub::ActivitySerializer, signer: reblog.account)) end diff --git a/app/views/admin/tags/_tag.html.haml b/app/views/admin/tags/_tag.html.haml index 287d28e53..adf4ca7b2 100644 --- a/app/views/admin/tags/_tag.html.haml +++ b/app/views/admin/tags/_tag.html.haml @@ -10,8 +10,6 @@ = tag.name %small - = t('admin.tags.in_directory', count: tag.accounts_count) - • = t('admin.tags.unique_uses_today', count: tag.history.first[:accounts]) - if tag.trending? diff --git a/app/views/admin/tags/index.html.haml b/app/views/admin/tags/index.html.haml index d7719d45d..d78f3c6d1 100644 --- a/app/views/admin/tags/index.html.haml +++ b/app/views/admin/tags/index.html.haml @@ -5,12 +5,6 @@ = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous' .filters - .filter-subset - %strong= t('admin.tags.context') - %ul - %li= filter_link_to t('generic.all'), directory: nil - %li= filter_link_to t('admin.tags.directory'), directory: '1' - .filter-subset %strong= t('admin.tags.review') %ul @@ -23,8 +17,9 @@ %strong= t('generic.order_by') %ul %li= filter_link_to t('admin.tags.most_recent'), popular: nil, active: nil - %li= filter_link_to t('admin.tags.most_popular'), popular: '1', active: nil %li= filter_link_to t('admin.tags.last_active'), active: '1', popular: nil + %li= filter_link_to t('admin.tags.most_popular'), popular: '1', active: nil + = form_tag admin_tags_url, method: 'GET', class: 'simple_form' do .fields-group diff --git a/app/views/admin/tags/show.html.haml b/app/views/admin/tags/show.html.haml index c9a147587..c4caffda1 100644 --- a/app/views/admin/tags/show.html.haml +++ b/app/views/admin/tags/show.html.haml @@ -10,15 +10,6 @@ %div .dashboard__counters__num= number_with_delimiter @accounts_week .dashboard__counters__label= t 'admin.tags.accounts_week' - %div - - if @tag.accounts_count > 0 - = link_to explore_hashtag_path(@tag) do - .dashboard__counters__num= number_with_delimiter @tag.accounts_count - .dashboard__counters__label= t 'admin.tags.directory' - - else - %div - .dashboard__counters__num= number_with_delimiter @tag.accounts_count - .dashboard__counters__label= t 'admin.tags.directory' %hr.spacer/ @@ -30,8 +21,8 @@ .fields-group = f.input :usable, as: :boolean, wrapper: :with_label - = f.input :trendable, as: :boolean, wrapper: :with_label, disabled: !Setting.trends - = f.input :listable, as: :boolean, wrapper: :with_label, disabled: !Setting.profile_directory + = f.input :trendable, as: :boolean, wrapper: :with_label + = f.input :listable, as: :boolean, wrapper: :with_label .actions = f.button :button, t('generic.save_changes'), type: :submit diff --git a/config/locales/en.yml b/config/locales/en.yml index bfa489817..d8ad5bd84 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -698,12 +698,9 @@ en: accounts_today: Unique uses today accounts_week: Unique uses this week breakdown: Breakdown of today's usage by source - context: Context - directory: In directory - in_directory: "%{count} in directory" - last_active: Last active + last_active: Recently used most_popular: Most popular - most_recent: Most recent + most_recent: Recently created name: Hashtag review: Review status reviewed: Reviewed diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 8ff880ebc..c4388ffc5 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -208,7 +208,7 @@ en: rule: text: Rule tag: - listable: Allow this hashtag to appear in searches and on the profile directory + listable: Allow this hashtag to appear in searches and suggestions name: Hashtag trendable: Allow this hashtag to appear under trends usable: Allow posts to use this hashtag diff --git a/config/routes.rb b/config/routes.rb index 8ca7fccdd..2373d8a51 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -97,8 +97,6 @@ Rails.application.routes.draw do post '/interact/:id', to: 'remote_interaction#create' get '/explore', to: 'directories#index', as: :explore - get '/explore/:id', to: 'directories#show', as: :explore_hashtag - get '/settings', to: redirect('/settings/profile') namespace :settings do diff --git a/db/post_migrate/20210502233513_drop_account_tag_stats.rb b/db/post_migrate/20210502233513_drop_account_tag_stats.rb new file mode 100644 index 000000000..80adadcab --- /dev/null +++ b/db/post_migrate/20210502233513_drop_account_tag_stats.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class DropAccountTagStats < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + drop_table :account_tag_stats + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/schema.rb b/db/schema.rb index 88e906079..19b1afe00 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -115,15 +115,6 @@ ActiveRecord::Schema.define(version: 2021_05_05_174616) do t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true end - create_table "account_tag_stats", force: :cascade do |t| - t.bigint "tag_id", null: false - t.bigint "accounts_count", default: 0, null: false - t.boolean "hidden", default: false, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["tag_id"], name: "index_account_tag_stats_on_tag_id", unique: true - end - create_table "account_warning_presets", force: :cascade do |t| t.text "text", default: "", null: false t.datetime "created_at", null: false @@ -985,7 +976,6 @@ ActiveRecord::Schema.define(version: 2021_05_05_174616) do add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "account_pins", "accounts", on_delete: :cascade add_foreign_key "account_stats", "accounts", on_delete: :cascade - add_foreign_key "account_tag_stats", "tags", on_delete: :cascade add_foreign_key "account_warnings", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "account_warnings", "accounts", on_delete: :nullify add_foreign_key "accounts", "accounts", column: "moved_to_account_id", on_delete: :nullify diff --git a/spec/models/account_tag_stat_spec.rb b/spec/models/account_tag_stat_spec.rb deleted file mode 100644 index 6d3057f35..000000000 --- a/spec/models/account_tag_stat_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe AccountTagStat, type: :model do - key = 'accounts_count' - let(:account_tag_stat) { Fabricate(:tag).account_tag_stat } - - describe '#increment_count!' do - it 'calls #update' do - args = { key => account_tag_stat.public_send(key) + 1 } - expect(account_tag_stat).to receive(:update).with(args) - account_tag_stat.increment_count!(key) - end - - it 'increments value by 1' do - expect do - account_tag_stat.increment_count!(key) - end.to change { account_tag_stat.accounts_count }.by(1) - end - end - - describe '#decrement_count!' do - it 'calls #update' do - args = { key => [account_tag_stat.public_send(key) - 1, 0].max } - expect(account_tag_stat).to receive(:update).with(args) - account_tag_stat.decrement_count!(key) - end - - it 'decrements value by 1' do - account_tag_stat.update(key => 1) - - expect do - account_tag_stat.decrement_count!(key) - end.to change { account_tag_stat.accounts_count }.by(-1) - end - end -end diff --git a/spec/models/trending_tags_spec.rb b/spec/models/trending_tags_spec.rb index b6122c994..dfbc7d6f8 100644 --- a/spec/models/trending_tags_spec.rb +++ b/spec/models/trending_tags_spec.rb @@ -7,9 +7,9 @@ RSpec.describe TrendingTags do describe '.update!' do let!(:at_time) { Time.now.utc } - let!(:tag1) { Fabricate(:tag, name: 'Catstodon') } - let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon') } - let!(:tag3) { Fabricate(:tag, name: 'OCs') } + let!(:tag1) { Fabricate(:tag, name: 'Catstodon', trendable: true) } + let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon', trendable: true) } + let!(:tag3) { Fabricate(:tag, name: 'OCs', trendable: true) } before do allow(Redis.current).to receive(:pfcount) do |key| -- cgit From 1294f9ee4fab176bdc3989d667eed43f57baad5a Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 7 May 2021 19:32:58 +0200 Subject: Remove PubSubHubbub-related columns from accounts table (#16170) --- app/models/account.rb | 14 +++--- .../20210507001928_remove_hub_url_from_accounts.rb | 12 +++++ db/schema.rb | 6 +-- spec/controllers/relationships_controller_spec.rb | 6 +-- spec/services/authorize_follow_service_spec.rb | 18 -------- .../services/batched_remove_status_service_spec.rb | 2 +- spec/services/block_service_spec.rb | 13 ------ spec/services/favourite_service_spec.rb | 14 ------ spec/services/fetch_remote_status_service_spec.rb | 52 ---------------------- spec/services/process_mentions_service_spec.rb | 31 ------------- spec/services/reblog_service_spec.rb | 16 ------- spec/services/reject_follow_service_spec.rb | 18 -------- spec/services/remove_status_service_spec.rb | 2 +- spec/services/unblock_service_spec.rb | 14 ------ spec/services/unfollow_service_spec.rb | 14 ------ 15 files changed, 24 insertions(+), 208 deletions(-) create mode 100644 db/post_migrate/20210507001928_remove_hub_url_from_accounts.rb (limited to 'spec') diff --git a/app/models/account.rb b/app/models/account.rb index 994459338..2c5455d8e 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -6,12 +6,8 @@ # id :bigint(8) not null, primary key # username :string default(""), not null # domain :string -# secret :string default(""), not null # private_key :text # public_key :text default(""), not null -# remote_url :string default(""), not null -# salmon_url :string default(""), not null -# hub_url :string default(""), not null # created_at :datetime not null # updated_at :datetime not null # note :text default(""), not null @@ -49,12 +45,18 @@ # avatar_storage_schema_version :integer # header_storage_schema_version :integer # devices_url :string -# sensitized_at :datetime # suspension_origin :integer +# sensitized_at :datetime # class Account < ApplicationRecord - self.ignored_columns = %w(subscription_expires_at) + self.ignored_columns = %w( + subscription_expires_at + secret + remote_url + salmon_url + hub_url + ) USERNAME_RE = /[a-z0-9_]+([a-z0-9_\.-]+[a-z0-9_]+)?/i MENTION_RE = /(?<=^|[^\/[:word:]])@((#{USERNAME_RE})(?:@[[:word:]\.\-]+[a-z0-9]+)?)/i diff --git a/db/post_migrate/20210507001928_remove_hub_url_from_accounts.rb b/db/post_migrate/20210507001928_remove_hub_url_from_accounts.rb new file mode 100644 index 000000000..83a1f5fcf --- /dev/null +++ b/db/post_migrate/20210507001928_remove_hub_url_from_accounts.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class RemoveHubURLFromAccounts < ActiveRecord::Migration[5.2] + def change + safety_assured do + remove_column :accounts, :secret, :string, null: false, default: '' + remove_column :accounts, :remote_url, :string, null: false, default: '' + remove_column :accounts, :salmon_url, :string, null: false, default: '' + remove_column :accounts, :hub_url, :string, null: false, default: '' + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 19b1afe00..583bdf317 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_05_05_174616) do +ActiveRecord::Schema.define(version: 2021_05_07_001928) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -136,12 +136,8 @@ ActiveRecord::Schema.define(version: 2021_05_05_174616) do create_table "accounts", id: :bigint, default: -> { "timestamp_id('accounts'::text)" }, force: :cascade do |t| t.string "username", default: "", null: false t.string "domain" - t.string "secret", default: "", null: false t.text "private_key" t.text "public_key", default: "", null: false - t.string "remote_url", default: "", null: false - t.string "salmon_url", default: "", null: false - t.string "hub_url", default: "", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.text "note", default: "", null: false diff --git a/spec/controllers/relationships_controller_spec.rb b/spec/controllers/relationships_controller_spec.rb index 16e255afe..2056a2ac2 100644 --- a/spec/controllers/relationships_controller_spec.rb +++ b/spec/controllers/relationships_controller_spec.rb @@ -36,11 +36,7 @@ describe RelationshipsController do end describe 'PATCH #update' do - let(:poopfeast) { Fabricate(:account, username: 'poopfeast', domain: 'example.com', salmon_url: 'http://example.com/salmon') } - - before do - stub_request(:post, 'http://example.com/salmon').to_return(status: 200) - end + let(:poopfeast) { Fabricate(:account, username: 'poopfeast', domain: 'example.com') } shared_examples 'redirects back to followers page' do it 'redirects back to followers page' do diff --git a/spec/services/authorize_follow_service_spec.rb b/spec/services/authorize_follow_service_spec.rb index ce56d57a6..8e5d8fb03 100644 --- a/spec/services/authorize_follow_service_spec.rb +++ b/spec/services/authorize_follow_service_spec.rb @@ -22,24 +22,6 @@ RSpec.describe AuthorizeFollowService, type: :service do end end - describe 'remote OStatus' do - let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', domain: 'example.com', salmon_url: 'http://salmon.example.com')).account } - - before do - FollowRequest.create(account: bob, target_account: sender) - stub_request(:post, "http://salmon.example.com/").to_return(:status => 200, :body => "", :headers => {}) - subject.call(bob, sender) - end - - it 'removes follow request' do - expect(bob.requested?(sender)).to be false - end - - it 'creates follow relation' do - expect(bob.following?(sender)).to be true - end - end - describe 'remote ActivityPub' do let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', domain: 'example.com', protocol: :activitypub, inbox_url: 'http://example.com/inbox')).account } diff --git a/spec/services/batched_remove_status_service_spec.rb b/spec/services/batched_remove_status_service_spec.rb index c1f54a6fd..4203952c6 100644 --- a/spec/services/batched_remove_status_service_spec.rb +++ b/spec/services/batched_remove_status_service_spec.rb @@ -4,7 +4,7 @@ RSpec.describe BatchedRemoveStatusService, type: :service do subject { BatchedRemoveStatusService.new } let!(:alice) { Fabricate(:account) } - let!(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com', salmon_url: 'http://example.com/salmon') } + let!(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com') } let!(:jeff) { Fabricate(:user).account } let!(:hank) { Fabricate(:account, username: 'hank', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } diff --git a/spec/services/block_service_spec.rb b/spec/services/block_service_spec.rb index de20dd026..3714f09e9 100644 --- a/spec/services/block_service_spec.rb +++ b/spec/services/block_service_spec.rb @@ -17,19 +17,6 @@ RSpec.describe BlockService, type: :service do end end - describe 'remote OStatus' do - let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', domain: 'example.com', salmon_url: 'http://salmon.example.com')).account } - - before do - stub_request(:post, "http://salmon.example.com/").to_return(:status => 200, :body => "", :headers => {}) - subject.call(sender, bob) - end - - it 'creates a blocking relation' do - expect(sender.blocking?(bob)).to be true - end - end - describe 'remote ActivityPub' do let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox')).account } diff --git a/spec/services/favourite_service_spec.rb b/spec/services/favourite_service_spec.rb index 4c29ea77b..fc7f58eb4 100644 --- a/spec/services/favourite_service_spec.rb +++ b/spec/services/favourite_service_spec.rb @@ -18,20 +18,6 @@ RSpec.describe FavouriteService, type: :service do end end - describe 'remote OStatus' do - let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', protocol: :ostatus, domain: 'example.com', salmon_url: 'http://salmon.example.com')).account } - let(:status) { Fabricate(:status, account: bob, uri: 'tag:example.com:blahblah') } - - before do - stub_request(:post, "http://salmon.example.com/").to_return(:status => 200, :body => "", :headers => {}) - subject.call(sender, status) - end - - it 'creates a favourite' do - expect(status.favourites.first).to_not be_nil - end - end - describe 'remote ActivityPub' do let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, protocol: :activitypub, username: 'bob', domain: 'example.com', inbox_url: 'http://example.com/inbox')).account } let(:status) { Fabricate(:status, account: bob) } diff --git a/spec/services/fetch_remote_status_service_spec.rb b/spec/services/fetch_remote_status_service_spec.rb index 1c4b4fee2..0e63cc9eb 100644 --- a/spec/services/fetch_remote_status_service_spec.rb +++ b/spec/services/fetch_remote_status_service_spec.rb @@ -31,56 +31,4 @@ RSpec.describe FetchRemoteStatusService, type: :service do expect(status.text).to eq 'Lorem ipsum' end end - - context 'protocol is :ostatus' do - subject { described_class.new } - - before do - Fabricate(:account, username: 'tracer', domain: 'real.domain', remote_url: 'https://real.domain/users/tracer') - end - - it 'does not create status with author at different domain' do - status_body = <<-XML.squish - - - tag:real.domain,2017-04-27:objectId=4487555:objectType=Status - 2017-04-27T13:49:25Z - 2017-04-27T13:49:25Z - http://activitystrea.ms/schema/1.0/note - http://activitystrea.ms/schema/1.0/post - - https://real.domain/users/tracer - http://activitystrea.ms/schema/1.0/person - https://real.domain/users/tracer - tracer - - Overwatch rocks - - XML - - expect(subject.call('https://fake.domain/foo', status_body)).to be_nil - end - - it 'does not create status with wrong id when id uses http format' do - status_body = <<-XML.squish - - - https://other-real.domain/statuses/123 - 2017-04-27T13:49:25Z - 2017-04-27T13:49:25Z - http://activitystrea.ms/schema/1.0/note - http://activitystrea.ms/schema/1.0/post - - https://real.domain/users/tracer - http://activitystrea.ms/schema/1.0/person - https://real.domain/users/tracer - tracer - - Overwatch rocks - - XML - - expect(subject.call('https://real.domain/statuses/456', status_body)).to be_nil - end - end end diff --git a/spec/services/process_mentions_service_spec.rb b/spec/services/process_mentions_service_spec.rb index c30de8eeb..3b2f9d698 100644 --- a/spec/services/process_mentions_service_spec.rb +++ b/spec/services/process_mentions_service_spec.rb @@ -7,37 +7,6 @@ RSpec.describe ProcessMentionsService, type: :service do subject { ProcessMentionsService.new } - context 'OStatus with public toot' do - let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :ostatus, domain: 'example.com', salmon_url: 'http://salmon.example.com') } - - before do - stub_request(:post, remote_user.salmon_url) - subject.call(status) - end - - it 'does not create a mention' do - expect(remote_user.mentions.where(status: status).count).to eq 0 - end - end - - context 'OStatus with private toot' do - let(:visibility) { :private } - let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :ostatus, domain: 'example.com', salmon_url: 'http://salmon.example.com') } - - before do - stub_request(:post, remote_user.salmon_url) - subject.call(status) - end - - it 'does not create a mention' do - expect(remote_user.mentions.where(status: status).count).to eq 0 - end - - it 'does not post to remote user\'s Salmon end point' do - expect(a_request(:post, remote_user.salmon_url)).to_not have_been_made - end - end - context 'ActivityPub' do context do let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } diff --git a/spec/services/reblog_service_spec.rb b/spec/services/reblog_service_spec.rb index 58fb46f0f..e2077f282 100644 --- a/spec/services/reblog_service_spec.rb +++ b/spec/services/reblog_service_spec.rb @@ -32,22 +32,6 @@ RSpec.describe ReblogService, type: :service do end end - context 'OStatus' do - let(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com', salmon_url: 'http://salmon.example.com') } - let(:status) { Fabricate(:status, account: bob, uri: 'tag:example.com;something:something') } - - subject { ReblogService.new } - - before do - stub_request(:post, 'http://salmon.example.com') - subject.call(alice, status) - end - - it 'creates a reblog' do - expect(status.reblogs.count).to eq 1 - end - end - context 'ActivityPub' do let(:bob) { Fabricate(:account, username: 'bob', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } let(:status) { Fabricate(:status, account: bob) } diff --git a/spec/services/reject_follow_service_spec.rb b/spec/services/reject_follow_service_spec.rb index 1aec060db..732cb07f7 100644 --- a/spec/services/reject_follow_service_spec.rb +++ b/spec/services/reject_follow_service_spec.rb @@ -22,24 +22,6 @@ RSpec.describe RejectFollowService, type: :service do end end - describe 'remote OStatus' do - let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', domain: 'example.com', salmon_url: 'http://salmon.example.com')).account } - - before do - FollowRequest.create(account: bob, target_account: sender) - stub_request(:post, "http://salmon.example.com/").to_return(:status => 200, :body => "", :headers => {}) - subject.call(bob, sender) - end - - it 'removes follow request' do - expect(bob.requested?(sender)).to be false - end - - it 'does not create follow relation' do - expect(bob.following?(sender)).to be false - end - end - describe 'remote ActivityPub' do let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', domain: 'example.com', protocol: :activitypub, inbox_url: 'http://example.com/inbox')).account } diff --git a/spec/services/remove_status_service_spec.rb b/spec/services/remove_status_service_spec.rb index 7ce75b2c7..21fb0cd35 100644 --- a/spec/services/remove_status_service_spec.rb +++ b/spec/services/remove_status_service_spec.rb @@ -4,7 +4,7 @@ RSpec.describe RemoveStatusService, type: :service do subject { RemoveStatusService.new } let!(:alice) { Fabricate(:account, user: Fabricate(:user)) } - let!(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com', salmon_url: 'http://example.com/salmon') } + let!(:bob) { Fabricate(:account, username: 'bob', domain: 'example.com') } let!(:jeff) { Fabricate(:account) } let!(:hank) { Fabricate(:account, username: 'hank', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } let!(:bill) { Fabricate(:account, username: 'bill', protocol: :activitypub, domain: 'example2.com', inbox_url: 'http://example2.com/inbox') } diff --git a/spec/services/unblock_service_spec.rb b/spec/services/unblock_service_spec.rb index 6350c6834..c43ab24b0 100644 --- a/spec/services/unblock_service_spec.rb +++ b/spec/services/unblock_service_spec.rb @@ -18,20 +18,6 @@ RSpec.describe UnblockService, type: :service do end end - describe 'remote OStatus' do - let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', domain: 'example.com', salmon_url: 'http://salmon.example.com')).account } - - before do - sender.block!(bob) - stub_request(:post, "http://salmon.example.com/").to_return(:status => 200, :body => "", :headers => {}) - subject.call(sender, bob) - end - - it 'destroys the blocking relation' do - expect(sender.blocking?(bob)).to be false - end - end - describe 'remote ActivityPub' do let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox')).account } diff --git a/spec/services/unfollow_service_spec.rb b/spec/services/unfollow_service_spec.rb index 84b5dafbc..7f0b575e4 100644 --- a/spec/services/unfollow_service_spec.rb +++ b/spec/services/unfollow_service_spec.rb @@ -18,20 +18,6 @@ RSpec.describe UnfollowService, type: :service do end end - describe 'remote OStatus' do - let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', protocol: :ostatus, domain: 'example.com', salmon_url: 'http://salmon.example.com')).account } - - before do - sender.follow!(bob) - stub_request(:post, "http://salmon.example.com/").to_return(:status => 200, :body => "", :headers => {}) - subject.call(sender, bob) - end - - it 'destroys the following relation' do - expect(sender.following?(bob)).to be false - end - end - describe 'remote ActivityPub' do let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox')).account } -- cgit From afb788218913061a36fad9b14e2e3e34025cc25b Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 10 May 2021 17:31:55 +0200 Subject: Fix blocking someone not clearing up list feeds (#16205) --- app/lib/feed_manager.rb | 30 +++++++++++++++++++++++++++++ app/services/after_block_service.rb | 5 +++++ spec/services/after_block_service_spec.rb | 32 ++++++++++++++++++++++++++----- 3 files changed, 62 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 43aeecb35..d5e435216 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -194,6 +194,36 @@ class FeedManager end end + # Clear all statuses from or mentioning target_account from a list feed + # @param [List] list + # @param [Account] target_account + # @return [void] + def clear_from_list(list, target_account) + timeline_key = key(:list, list.id) + timeline_status_ids = redis.zrange(timeline_key, 0, -1) + statuses = Status.where(id: timeline_status_ids).select(:id, :reblog_of_id, :account_id).to_a + reblogged_ids = Status.where(id: statuses.map(&:reblog_of_id).compact, account: target_account).pluck(:id) + with_mentions_ids = Mention.active.where(status_id: statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact, account: target_account).pluck(:status_id) + + target_statuses = statuses.select do |status| + status.account_id == target_account.id || reblogged_ids.include?(status.reblog_of_id) || with_mentions_ids.include?(status.id) || with_mentions_ids.include?(status.reblog_of_id) + end + + target_statuses.each do |status| + unpush_from_list(list, status) + end + end + + # Clear all statuses from or mentioning target_account from an account's lists + # @param [Account] account + # @param [Account] target_account + # @return [void] + def clear_from_lists(account, target_account) + List.where(account: account).each do |list| + clear_from_list(list, target_account) + end + end + # Populate home feed of account from scratch # @param [Account] account # @return [void] diff --git a/app/services/after_block_service.rb b/app/services/after_block_service.rb index 314919df8..899e84be4 100644 --- a/app/services/after_block_service.rb +++ b/app/services/after_block_service.rb @@ -6,6 +6,7 @@ class AfterBlockService < BaseService @target_account = target_account clear_home_feed! + clear_list_feeds! clear_notifications! clear_conversations! end @@ -16,6 +17,10 @@ class AfterBlockService < BaseService FeedManager.instance.clear_from_home(@account, @target_account) end + def clear_list_feeds! + FeedManager.instance.clear_from_lists(@account, @target_account) + end + def clear_conversations! AccountConversation.where(account: @account).where('? = ANY(participant_account_ids)', @target_account.id).in_batches.destroy_all end diff --git a/spec/services/after_block_service_spec.rb b/spec/services/after_block_service_spec.rb index f63b2045a..fe5b26b2b 100644 --- a/spec/services/after_block_service_spec.rb +++ b/spec/services/after_block_service_spec.rb @@ -5,12 +5,14 @@ RSpec.describe AfterBlockService, type: :service do -> { described_class.new.call(account, target_account) } end - let(:account) { Fabricate(:account) } - let(:target_account) { Fabricate(:account) } + let(:account) { Fabricate(:account) } + let(:target_account) { Fabricate(:account) } + let(:status) { Fabricate(:status, account: target_account) } + let(:other_status) { Fabricate(:status, account: target_account) } + let(:other_account_status) { Fabricate(:status) } + let(:other_account_reblog) { Fabricate(:status, reblog_of_id: other_status.id) } describe 'home timeline' do - let(:status) { Fabricate(:status, account: target_account) } - let(:other_account_status) { Fabricate(:status) } let(:home_timeline_key) { FeedManager.instance.key(:home, account.id) } before do @@ -20,10 +22,30 @@ RSpec.describe AfterBlockService, type: :service do it "clears account's statuses" do FeedManager.instance.push_to_home(account, status) FeedManager.instance.push_to_home(account, other_account_status) + FeedManager.instance.push_to_home(account, other_account_reblog) is_expected.to change { Redis.current.zrange(home_timeline_key, 0, -1) - }.from([status.id.to_s, other_account_status.id.to_s]).to([other_account_status.id.to_s]) + }.from([status.id.to_s, other_account_status.id.to_s, other_account_reblog.id.to_s]).to([other_account_status.id.to_s]) + end + end + + describe 'lists' do + let(:list) { Fabricate(:list, account: account) } + let(:list_timeline_key) { FeedManager.instance.key(:list, list.id) } + + before do + Redis.current.del(list_timeline_key) + end + + it "clears account's statuses" do + FeedManager.instance.push_to_list(list, status) + FeedManager.instance.push_to_list(list, other_account_status) + FeedManager.instance.push_to_list(list, other_account_reblog) + + is_expected.to change { + Redis.current.zrange(list_timeline_key, 0, -1) + }.from([status.id.to_s, other_account_status.id.to_s, other_account_reblog.id.to_s]).to([other_account_status.id.to_s]) end end end -- cgit