From 5edff32733eff2cbffbf614b31eea85da8dc3aaf Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 15 Apr 2020 20:33:24 +0200 Subject: Change delivery failure tracking to work with hostnames instead of URLs (#13437) --- app/controllers/activitypub/inboxes_controller.rb | 2 +- app/controllers/admin/instances_controller.rb | 2 +- app/lib/delivery_failure_tracker.rb | 34 +++++++++++++---------- app/models/account.rb | 2 +- app/models/announcement.rb | 2 +- app/models/relay.rb | 4 +-- app/models/unavailable_domain.rb | 22 +++++++++++++++ app/views/admin/accounts/show.html.haml | 4 +-- app/workers/activitypub/delivery_worker.rb | 2 +- 9 files changed, 51 insertions(+), 23 deletions(-) create mode 100644 app/models/unavailable_domain.rb (limited to 'app') diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb index 291eec19a..0a561e7f0 100644 --- a/app/controllers/activitypub/inboxes_controller.rb +++ b/app/controllers/activitypub/inboxes_controller.rb @@ -49,7 +49,7 @@ class ActivityPub::InboxesController < ActivityPub::BaseController ResolveAccountWorker.perform_async(signed_request_account.acct) end - DeliveryFailureTracker.track_inverse_success!(signed_request_account) + DeliveryFailureTracker.reset!(signed_request_account.inbox_url) end def process_payload diff --git a/app/controllers/admin/instances_controller.rb b/app/controllers/admin/instances_controller.rb index 2fc041207..1790becbf 100644 --- a/app/controllers/admin/instances_controller.rb +++ b/app/controllers/admin/instances_controller.rb @@ -19,7 +19,7 @@ module Admin @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?(Account.select(:shared_inbox_url).where(domain: params[:id]).first&.shared_inbox_url) + @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 diff --git a/app/lib/delivery_failure_tracker.rb b/app/lib/delivery_failure_tracker.rb index 8d3be35de..25fa694d2 100644 --- a/app/lib/delivery_failure_tracker.rb +++ b/app/lib/delivery_failure_tracker.rb @@ -3,47 +3,53 @@ class DeliveryFailureTracker FAILURE_DAYS_THRESHOLD = 7 - def initialize(inbox_url) - @inbox_url = inbox_url + def initialize(url_or_host) + @host = url_or_host.start_with?('https://') || url_or_host.start_with?('http://') ? Addressable::URI.parse(url_or_host).normalized_host : url_or_host end def track_failure! Redis.current.sadd(exhausted_deliveries_key, today) - Redis.current.sadd('unavailable_inboxes', @inbox_url) if reached_failure_threshold? + UnavailableDomain.create(domain: @host) if reached_failure_threshold? end def track_success! Redis.current.del(exhausted_deliveries_key) - Redis.current.srem('unavailable_inboxes', @inbox_url) + UnavailableDomain.find_by(domain: @host)&.destroy end def days Redis.current.scard(exhausted_deliveries_key) || 0 end + def available? + !UnavailableDomain.where(domain: @host).exists? + end + + alias reset! track_success! + class << self - def filter(arr) - arr.reject(&method(:unavailable?)) - end + def without_unavailable(urls) + unavailable_domains_map = Rails.cache.fetch('unavailable_domains') { UnavailableDomain.pluck(:domain).each_with_object({}) { |domain, hash| hash[domain] = true } } - def unavailable?(url) - Redis.current.sismember('unavailable_inboxes', url) + urls.reject do |url| + host = Addressable::URI.parse(url).normalized_host + unavailable_domains_map[host] + end end def available?(url) - !unavailable?(url) + new(url).available? end - def track_inverse_success!(from_account) - new(from_account.inbox_url).track_success! if from_account.inbox_url.present? - new(from_account.shared_inbox_url).track_success! if from_account.shared_inbox_url.present? + def reset!(url) + new(url).reset! end end private def exhausted_deliveries_key - "exhausted_deliveries:#{@inbox_url}" + "exhausted_deliveries:#{@host}" end def today diff --git a/app/models/account.rb b/app/models/account.rb index 6aceb809c..dc14e8538 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -413,7 +413,7 @@ class Account < ApplicationRecord def inboxes urls = reorder(nil).where(protocol: :activitypub).pluck(Arel.sql("distinct coalesce(nullif(accounts.shared_inbox_url, ''), accounts.inbox_url)")) - DeliveryFailureTracker.filter(urls) + DeliveryFailureTracker.without_unavailable(urls) end def search_for(terms, limit = 10, offset = 0) diff --git a/app/models/announcement.rb b/app/models/announcement.rb index a4e427b49..c493604c2 100644 --- a/app/models/announcement.rb +++ b/app/models/announcement.rb @@ -14,7 +14,7 @@ # created_at :datetime not null # updated_at :datetime not null # published_at :datetime -# status_ids :bigint is an Array +# status_ids :bigint(8) is an Array # class Announcement < ApplicationRecord diff --git a/app/models/relay.rb b/app/models/relay.rb index 8c8a97db3..870f31979 100644 --- a/app/models/relay.rb +++ b/app/models/relay.rb @@ -27,7 +27,7 @@ class Relay < ApplicationRecord payload = Oj.dump(follow_activity(activity_id)) update!(state: :pending, follow_activity_id: activity_id) - DeliveryFailureTracker.new(inbox_url).track_success! + DeliveryFailureTracker.track_success!(inbox_url) ActivityPub::DeliveryWorker.perform_async(payload, some_local_account.id, inbox_url) end @@ -36,7 +36,7 @@ class Relay < ApplicationRecord payload = Oj.dump(unfollow_activity(activity_id)) update!(state: :idle, follow_activity_id: nil) - DeliveryFailureTracker.new(inbox_url).track_success! + DeliveryFailureTracker.track_success!(inbox_url) ActivityPub::DeliveryWorker.perform_async(payload, some_local_account.id, inbox_url) end diff --git a/app/models/unavailable_domain.rb b/app/models/unavailable_domain.rb new file mode 100644 index 000000000..e2918b586 --- /dev/null +++ b/app/models/unavailable_domain.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: unavailable_domains +# +# id :bigint(8) not null, primary key +# domain :string default(""), not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class UnavailableDomain < ApplicationRecord + include DomainNormalizable + + after_commit :reset_cache! + + private + + def reset_cache! + Rails.cache.delete('unavailable_domains') + end +end diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index 965fd6fb6..408f94eed 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -171,12 +171,12 @@ %th= t('admin.accounts.inbox_url') %td = @account.inbox_url - = fa_icon DeliveryFailureTracker.unavailable?(@account.inbox_url) ? 'times' : 'check' + = fa_icon DeliveryFailureTracker.available?(@account.inbox_url) ? 'check' : 'times' %tr %th= t('admin.accounts.shared_inbox_url') %td = @account.shared_inbox_url - = fa_icon DeliveryFailureTracker.unavailable?(@account.shared_inbox_url) ? 'times' : 'check' + = fa_icon DeliveryFailureTracker.available?(@account.shared_inbox_url) ? 'check': 'times' %div{ style: 'overflow: hidden' } %div{ style: 'float: right' } diff --git a/app/workers/activitypub/delivery_worker.rb b/app/workers/activitypub/delivery_worker.rb index 196af4af1..0f925658f 100644 --- a/app/workers/activitypub/delivery_worker.rb +++ b/app/workers/activitypub/delivery_worker.rb @@ -12,7 +12,7 @@ class ActivityPub::DeliveryWorker HEADERS = { 'Content-Type' => 'application/activity+json' }.freeze def perform(json, source_account_id, inbox_url, options = {}) - return if DeliveryFailureTracker.unavailable?(inbox_url) + return unless DeliveryFailureTracker.available?(inbox_url) @options = options.with_indifferent_access @json = json -- cgit