From 216b85b053d091306e3311a21f5b050f70a75130 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 14 Dec 2020 09:06:34 +0100 Subject: Fix performance on instances list in admin UI (#15282) - Reduce duplicate queries - Remove n+1 queries - Add accounts count to detailed view - Add separate action log entry for updating existing domain blocks --- app/controllers/admin/domain_blocks_controller.rb | 5 ++- app/controllers/admin/instances_controller.rb | 44 +++------------------- .../api/v1/instances/peers_controller.rb | 2 +- 3 files changed, 9 insertions(+), 42 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index 74a36b79c..6a5b41a74 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -29,6 +29,7 @@ module Admin @domain_block = existing_domain_block @domain_block.update(resource_params) end + if @domain_block.save DomainBlockWorker.perform_async(@domain_block.id) log_action :create, @domain_block @@ -40,7 +41,7 @@ module Admin end def update - authorize :domain_block, :create? + authorize :domain_block, :update? @domain_block.update(update_params) @@ -48,7 +49,7 @@ module Admin if @domain_block.save DomainBlockWorker.perform_async(@domain_block.id, severity_changed) - log_action :create, @domain_block + log_action :update, @domain_block redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg') else render :edit diff --git a/app/controllers/admin/instances_controller.rb b/app/controllers/admin/instances_controller.rb index 1790becbf..b5918d231 100644 --- a/app/controllers/admin/instances_controller.rb +++ b/app/controllers/admin/instances_controller.rb @@ -2,65 +2,31 @@ module Admin class InstancesController < BaseController - before_action :set_domain_block, only: :show - before_action :set_domain_allow, only: :show + before_action :set_instances, only: :index before_action :set_instance, only: :show def index authorize :instance, :index? - - @instances = ordered_instances end def show authorize :instance, :show? - - @following_count = Follow.where(account: Account.where(domain: params[:id])).count - @followers_count = Follow.where(target_account: Account.where(domain: params[:id])).count - @reports_count = Report.where(target_account: Account.where(domain: params[:id])).count - @blocks_count = Block.where(target_account: Account.where(domain: params[:id])).count - @available = DeliveryFailureTracker.available?(params[:id]) - @media_storage = MediaAttachment.where(account: Account.where(domain: params[:id])).sum(:file_file_size) - @private_comment = @domain_block&.private_comment - @public_comment = @domain_block&.public_comment end private - def set_domain_block - @domain_block = DomainBlock.rule_for(params[:id]) - end - - def set_domain_allow - @domain_allow = DomainAllow.rule_for(params[:id]) - end - def set_instance - resource = Account.by_domain_accounts.find_by(domain: params[:id]) - resource ||= @domain_block - resource ||= @domain_allow + @instance = Instance.find(params[:id]) + end - if resource - @instance = Instance.new(resource) - else - not_found - end + def set_instances + @instances = filtered_instances.page(params[:page]) end def filtered_instances InstanceFilter.new(whitelist_mode? ? { allowed: true } : filter_params).results end - def paginated_instances - filtered_instances.page(params[:page]) - end - - helper_method :paginated_instances - - def ordered_instances - paginated_instances.map { |resource| Instance.new(resource) } - end - def filter_params params.slice(*InstanceFilter::KEYS).permit(*InstanceFilter::KEYS) end diff --git a/app/controllers/api/v1/instances/peers_controller.rb b/app/controllers/api/v1/instances/peers_controller.rb index 9fa440935..2877fec52 100644 --- a/app/controllers/api/v1/instances/peers_controller.rb +++ b/app/controllers/api/v1/instances/peers_controller.rb @@ -8,7 +8,7 @@ class Api::V1::Instances::PeersController < Api::BaseController def index expires_in 1.day, public: true - render_with_cache(expires_in: 1.day) { Account.remote.domains } + render_with_cache(expires_in: 1.day) { Instance.where.not(domain: DomainBlock.select(:domain)).pluck(:domain) } end private -- cgit From 1045549f85041b13002801808b30a332c3a68c61 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 15 Dec 2020 12:55:29 +0100 Subject: Add stoplight for object storage failures, return HTTP 503 (#13043) --- app/controllers/api/base_controller.rb | 2 +- app/controllers/application_controller.rb | 2 +- app/lib/activitypub/activity/create.rb | 4 ++++ config/initializers/paperclip.rb | 11 +++++++++++ lib/paperclip/attachment_extensions.rb | 17 +++++++++++++++++ 5 files changed, 34 insertions(+), 2 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index fe199e689..85f4cc768 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -40,7 +40,7 @@ class Api::BaseController < ApplicationController render json: { error: 'This action is not allowed' }, status: 403 end - rescue_from Mastodon::RaceConditionError do + rescue_from Mastodon::RaceConditionError, Seahorse::Client::NetworkingError, Stoplight::Error::RedLight do render json: { error: 'There was a temporary problem serving your request, please try again' }, status: 503 end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2201e463e..44616d6e5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -28,7 +28,7 @@ class ApplicationController < ActionController::Base rescue_from ActiveRecord::RecordNotFound, with: :not_found rescue_from Mastodon::NotPermittedError, with: :forbidden rescue_from HTTP::Error, OpenSSL::SSL::SSLError, with: :internal_server_error - rescue_from Mastodon::RaceConditionError, with: :service_unavailable + rescue_from Mastodon::RaceConditionError, Seahorse::Client::NetworkingError, Stoplight::Error::RedLight, with: :service_unavailable rescue_from Mastodon::RateLimitExceededError, with: :too_many_requests before_action :store_current_location, except: :raise_not_found, unless: :devise_controller? diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index c77f237f9..612744676 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -228,6 +228,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity emoji ||= CustomEmoji.new(domain: @account.domain, shortcode: shortcode, uri: uri) emoji.image_remote_url = image_url emoji.save + rescue Seahorse::Client::NetworkingError + nil end def process_attachments @@ -250,6 +252,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity media_attachment.save rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError RedownloadMediaWorker.perform_in(rand(30..600).seconds, media_attachment.id) + rescue Seahorse::Client::NetworkingError + nil end end diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index 25adcd8d6..9ad7fd814 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -113,3 +113,14 @@ else end Paperclip.options[:content_type_mappings] = { csv: Import::FILE_TYPES } + +# In some places in the code, we rescue this exception, but we don't always +# load the S3 library, so it may be an undefined constant: + +unless defined?(Seahorse) + module Seahorse + module Client + class NetworkingError < StandardError; end + end + end +end diff --git a/lib/paperclip/attachment_extensions.rb b/lib/paperclip/attachment_extensions.rb index 752e79e65..94f7769b6 100644 --- a/lib/paperclip/attachment_extensions.rb +++ b/lib/paperclip/attachment_extensions.rb @@ -39,6 +39,23 @@ module Paperclip def default_url(style_name = default_style) @url_generator.for_as_default(style_name) end + + STOPLIGHT_THRESHOLD = 10 + STOPLIGHT_COOLDOWN = 30 + + # We overwrite this method to put a circuit breaker around + # calls to object storage, to stop hitting APIs that are slow + # to respond or don't respond at all and as such minimize the + # impact of object storage outages on application throughput + def save + Stoplight('object-storage') { super }.with_threshold(STOPLIGHT_THRESHOLD).with_cool_off_time(STOPLIGHT_COOLDOWN).with_error_handler do |error, handle| + if error.is_a?(Seahorse::Client::NetworkingError) + handle.call(error) + else + raise error + end + end.run + end end end -- cgit From 8a95867693dfe072241d5ddcd0ba7ed8546eab1e Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 18 Dec 2020 08:30:41 +0100 Subject: Add option to obfuscate domain name in public list of domain blocks (#15355) - Replace the middle of the domain with * characters (except for periods) - Add SHA-256 digest of the domain name in tooltip --- app/controllers/admin/domain_blocks_controller.rb | 4 ++-- app/models/domain_block.rb | 20 ++++++++++++++++++++ app/views/about/_domain_blocks.html.haml | 2 +- app/views/admin/domain_blocks/edit.html.haml | 3 +++ app/views/admin/domain_blocks/new.html.haml | 3 +++ config/locales/en.yml | 2 ++ .../20201218054746_add_obfuscate_to_domain_blocks.rb | 15 +++++++++++++++ db/schema.rb | 3 ++- 8 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20201218054746_add_obfuscate_to_domain_blocks.rb (limited to 'app/controllers') diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index 6a5b41a74..ba927b04a 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -74,11 +74,11 @@ module Admin end def update_params - params.require(:domain_block).permit(:severity, :reject_media, :reject_reports, :private_comment, :public_comment) + params.require(:domain_block).permit(:severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate) end def resource_params - params.require(:domain_block).permit(:domain, :severity, :reject_media, :reject_reports, :private_comment, :public_comment) + params.require(:domain_block).permit(:domain, :severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate) end end end diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb index 829d7583b..bba04c603 100644 --- a/app/models/domain_block.rb +++ b/app/models/domain_block.rb @@ -12,6 +12,7 @@ # reject_reports :boolean default(FALSE), not null # private_comment :text # public_comment :text +# obfuscate :boolean default(FALSE), not null # class DomainBlock < ApplicationRecord @@ -73,4 +74,23 @@ class DomainBlock < ApplicationRecord scope = suspend? ? accounts.where(suspended_at: created_at) : accounts.where(silenced_at: created_at) scope.count end + + def public_domain + return domain unless obfuscate? + + length = domain.size + visible_ratio = length / 4 + + domain.chars.map.with_index do |chr, i| + if i > visible_ratio && i < length - visible_ratio && chr != '.' + '*' + else + chr + end + end.join + end + + def domain_digest + Digest::SHA256.hexdigest(domain) + end end diff --git a/app/views/about/_domain_blocks.html.haml b/app/views/about/_domain_blocks.html.haml index e0c5df41d..35a30f16e 100644 --- a/app/views/about/_domain_blocks.html.haml +++ b/app/views/about/_domain_blocks.html.haml @@ -7,6 +7,6 @@ - domain_blocks.each do |domain_block| %tr %td.nowrap - %span{ title: domain_block.domain }= domain_block.domain + %span{ title: "SHA-256: #{domain_block.domain_digest}" }= domain_block.public_domain %td = domain_block.public_comment if display_blocks_rationale? diff --git a/app/views/admin/domain_blocks/edit.html.haml b/app/views/admin/domain_blocks/edit.html.haml index d5868070a..6fe2edc82 100644 --- a/app/views/admin/domain_blocks/edit.html.haml +++ b/app/views/admin/domain_blocks/edit.html.haml @@ -20,6 +20,9 @@ .fields-group = f.input :reject_reports, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_blocks.reject_reports'), hint: I18n.t('admin.domain_blocks.reject_reports_hint') + .fields-group + = f.input :obfuscate, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_blocks.obfuscate'), hint: I18n.t('admin.domain_blocks.obfuscate_hint') + .field-group = f.input :private_comment, wrapper: :with_label, label: I18n.t('admin.domain_blocks.private_comment'), hint: t('admin.domain_blocks.private_comment_hint'), rows: 6 diff --git a/app/views/admin/domain_blocks/new.html.haml b/app/views/admin/domain_blocks/new.html.haml index f503f9b77..8b78f71f2 100644 --- a/app/views/admin/domain_blocks/new.html.haml +++ b/app/views/admin/domain_blocks/new.html.haml @@ -20,6 +20,9 @@ .fields-group = f.input :reject_reports, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_blocks.reject_reports'), hint: I18n.t('admin.domain_blocks.reject_reports_hint') + .fields-group + = f.input :obfuscate, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_blocks.obfuscate'), hint: I18n.t('admin.domain_blocks.obfuscate_hint') + .field-group = f.input :private_comment, wrapper: :with_label, label: I18n.t('admin.domain_blocks.private_comment'), hint: t('admin.domain_blocks.private_comment_hint'), rows: 6 diff --git a/config/locales/en.yml b/config/locales/en.yml index 83f75b28f..e91f4344f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -402,6 +402,8 @@ en: silence: Silence suspend: Suspend title: New domain block + obfuscate: Obfuscate domain name + obfuscate_hint: Partially obfuscate the domain name in the list if advertising the list of domain limitations is enabled private_comment: Private comment private_comment_hint: Comment about this domain limitation for internal use by the moderators. public_comment: Public comment diff --git a/db/migrate/20201218054746_add_obfuscate_to_domain_blocks.rb b/db/migrate/20201218054746_add_obfuscate_to_domain_blocks.rb new file mode 100644 index 000000000..26f4ddb85 --- /dev/null +++ b/db/migrate/20201218054746_add_obfuscate_to_domain_blocks.rb @@ -0,0 +1,15 @@ +require Rails.root.join('lib', 'mastodon', 'migration_helpers') + +class AddObfuscateToDomainBlocks < ActiveRecord::Migration[5.2] + include Mastodon::MigrationHelpers + + disable_ddl_transaction! + + def up + safety_assured { add_column_with_default :domain_blocks, :obfuscate, :boolean, default: false, allow_null: false } + end + + def down + remove_column :domain_blocks, :obfuscate + end +end diff --git a/db/schema.rb b/db/schema.rb index 55822a4b3..18bf1d4ca 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: 2020_12_06_004238) do +ActiveRecord::Schema.define(version: 2020_12_18_054746) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -360,6 +360,7 @@ ActiveRecord::Schema.define(version: 2020_12_06_004238) do t.boolean "reject_reports", default: false, null: false t.text "private_comment" t.text "public_comment" + t.boolean "obfuscate", default: false, null: false t.index ["domain"], name: "index_domain_blocks_on_domain", unique: true end -- cgit