From 5cce29495353813d86872578886a78f6f0c83d8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?KMY=EF=BC=88=E9=9B=AA=E3=81=82=E3=81=99=E3=81=8B=EF=BC=89?= Date: Tue, 12 Dec 2023 09:52:08 +0900 Subject: [PATCH 1/4] =?UTF-8?q?Fix:=20#284=20`FetchInstanceInfoWorker`?= =?UTF-8?q?=E3=81=8C=E5=8E=9F=E5=9B=A0=E3=81=A7Sidekiq=E3=81=AEJob?= =?UTF-8?q?=E3=81=8C=E8=A9=B0=E3=81=BE=E3=82=8B=E5=95=8F=E9=A1=8C=20(#342)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: #284 `FetchInstanceInfoWorker`が原因でSidekiqのJobが詰まる問題 * Fix: InstanceInfoを取得するタイミング * Fix test * Fix test * Fix: HTTPコード * 調整 --- .../activitypub/process_account_service.rb | 5 +-- .../activitypub/fetch_instance_info_worker.rb | 36 +++++++++---------- .../update_instance_info_scheduler.rb | 15 -------- config/sidekiq.yml | 4 --- spec/lib/activitypub/activity/update_spec.rb | 1 + .../process_account_service_spec.rb | 4 +++ .../fetch_instance_info_worker_spec.rb | 20 +++++++++++ .../update_instance_info_scheduler_spec.rb | 19 ---------- 8 files changed, 45 insertions(+), 59 deletions(-) delete mode 100644 app/workers/scheduler/update_instance_info_scheduler.rb delete mode 100644 spec/workers/scheduler/update_instance_info_scheduler_spec.rb diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 473b2cabd33be5..23a434ae90eb85 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -46,7 +46,6 @@ def call(username, domain, json, options = {}) end create_account - fetch_instance_info end update_account @@ -66,6 +65,8 @@ def call(username, domain, json, options = {}) check_links! if @account.fields.any?(&:requires_verification?) end + fetch_instance_info + @account rescue Oj::ParseError nil @@ -211,7 +212,7 @@ def process_duplicate_accounts! end def fetch_instance_info - ActivityPub::FetchInstanceInfoWorker.perform_async(@account.domain) unless InstanceInfo.exists?(domain: @account.domain) + ActivityPub::FetchInstanceInfoWorker.perform_async(@account.domain) unless Rails.cache.exist?("fetch_instance_info:#{@account.domain}", expires_in: 1.day) end def actor_type diff --git a/app/workers/activitypub/fetch_instance_info_worker.rb b/app/workers/activitypub/fetch_instance_info_worker.rb index 57cbd97d10cd72..bc9a1a48157e13 100644 --- a/app/workers/activitypub/fetch_instance_info_worker.rb +++ b/app/workers/activitypub/fetch_instance_info_worker.rb @@ -8,28 +8,32 @@ class ActivityPub::FetchInstanceInfoWorker sidekiq_options queue: 'push', retry: 2 - class Error < StandardError; end - class RequestError < Error; end - class DeadError < Error; end - SUPPORTED_NOTEINFO_RELS = ['http://nodeinfo.diaspora.software/ns/schema/2.0', 'http://nodeinfo.diaspora.software/ns/schema/2.1'].freeze def perform(domain) @instance = Instance.find_by(domain: domain) return if !@instance || @instance.unavailable_domain.present? - with_redis_lock("instance_info:#{domain}") do - link = nodeinfo_link - return if link.nil? - - update_info!(link) + Rails.cache.fetch("fetch_instance_info:#{@instance.domain}", expires_in: 1.day, race_condition_ttl: 1.hour) do + fetch! end - rescue ActivityPub::FetchInstanceInfoWorker::DeadError + true end private + def fetch! + link = nodeinfo_link + return if link.nil? + + update_info!(link) + + true + rescue Mastodon::UnexpectedResponseError + true + end + def nodeinfo_link nodeinfo = fetch_json("https://#{@instance.domain}/.well-known/nodeinfo") return nil if nodeinfo.nil? || !nodeinfo.key?('links') @@ -63,15 +67,9 @@ def update_info!(url) def fetch_json(url) build_request(url).perform do |response| - if [200, 203].include?(response.code) - raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) - - body_to_json(response.body_with_limit) - elsif [400, 401, 403, 404, 410].include?(response.code) - raise ActivityPub::FetchInstanceInfoWorker::DeadError, "Request for #{@instance.domain} returned HTTP #{response.code}" - else - raise ActivityPub::FetchInstanceInfoWorker::RequestError, "Request for #{@instance.domain} returned HTTP #{response.code}" - end + raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) + + body_to_json(response.body_with_limit) end end diff --git a/app/workers/scheduler/update_instance_info_scheduler.rb b/app/workers/scheduler/update_instance_info_scheduler.rb deleted file mode 100644 index f5b2852859bd74..00000000000000 --- a/app/workers/scheduler/update_instance_info_scheduler.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -class Scheduler::UpdateInstanceInfoScheduler - include Sidekiq::Worker - - sidekiq_options retry: 0, lock: :until_executed, lock_ttl: 1.day.to_i - - def perform - Instance.select(:domain).reorder(nil).find_in_batches do |instances| - ActivityPub::FetchInstanceInfoWorker.push_bulk(instances) do |instance| - [instance.domain] - end - end - end -end diff --git a/config/sidekiq.yml b/config/sidekiq.yml index f954c00aaa206c..b643a1ecfb30cf 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -62,10 +62,6 @@ interval: 30 seconds class: Scheduler::SidekiqHealthScheduler queue: scheduler - update_instance_info_scheduler: - cron: '0 0 * * *' - class: Scheduler::UpdateInstanceInfoScheduler - queue: scheduler software_update_check_scheduler: interval: 30 minutes class: Scheduler::SoftwareUpdateCheckScheduler diff --git a/spec/lib/activitypub/activity/update_spec.rb b/spec/lib/activitypub/activity/update_spec.rb index 87e96d2d1b12cb..6c84c5836ae3c1 100644 --- a/spec/lib/activitypub/activity/update_spec.rb +++ b/spec/lib/activitypub/activity/update_spec.rb @@ -55,6 +55,7 @@ stub_request(:get, actor_json[:following]).to_return(status: 404) stub_request(:get, actor_json[:featured]).to_return(status: 404) stub_request(:get, actor_json[:featuredTags]).to_return(status: 404) + stub_request(:get, 'https://example.com/.well-known/nodeinfo').to_return(status: 404) subject.perform end diff --git a/spec/services/activitypub/process_account_service_spec.rb b/spec/services/activitypub/process_account_service_spec.rb index 78107935593eca..c927b260f7b6df 100644 --- a/spec/services/activitypub/process_account_service_spec.rb +++ b/spec/services/activitypub/process_account_service_spec.rb @@ -5,6 +5,10 @@ RSpec.describe ActivityPub::ProcessAccountService, type: :service do subject { described_class.new } + before do + stub_request(:get, 'https://example.com/.well-known/nodeinfo').to_return(status: 404) + end + context 'with searchability' do subject { described_class.new.call('alice', 'example.com', payload) } diff --git a/spec/workers/activitypub/fetch_instance_info_worker_spec.rb b/spec/workers/activitypub/fetch_instance_info_worker_spec.rb index f6dacff5fc49fd..9dc9594041aca8 100644 --- a/spec/workers/activitypub/fetch_instance_info_worker_spec.rb +++ b/spec/workers/activitypub/fetch_instance_info_worker_spec.rb @@ -67,9 +67,22 @@ Instance.refresh end + it 'does not update immediately' do + stub_request(:get, 'https://example.com/nodeinfo/2.0').to_return(status: 200, body: nodeinfo_json) + subject.perform('example.com') + stub_request(:get, 'https://example.com/nodeinfo/2.0').to_return(status: 200, body: new_nodeinfo_json) + subject.perform('example.com') + + info = InstanceInfo.find_by(domain: 'example.com') + expect(info).to_not be_nil + expect(info.software).to eq 'mastodon' + expect(info.version).to eq '4.2.0-beta1' + end + it 'performs a mastodon instance' do stub_request(:get, 'https://example.com/nodeinfo/2.0').to_return(status: 200, body: nodeinfo_json) subject.perform('example.com') + Rails.cache.delete('fetch_instance_info:example.com') stub_request(:get, 'https://example.com/nodeinfo/2.0').to_return(status: 200, body: new_nodeinfo_json) subject.perform('example.com') @@ -93,5 +106,12 @@ info = InstanceInfo.find_by(domain: 'example.com') expect(info).to be_nil end + + it 'does not fetch again immediately' do + expect(subject.perform('example.com')).to be true + expect(subject.perform('example.com')).to be true + + expect(a_request(:get, 'https://example.com/.well-known/nodeinfo')).to have_been_made.once + end end end diff --git a/spec/workers/scheduler/update_instance_info_scheduler_spec.rb b/spec/workers/scheduler/update_instance_info_scheduler_spec.rb deleted file mode 100644 index f3a190417f204d..00000000000000 --- a/spec/workers/scheduler/update_instance_info_scheduler_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe Scheduler::UpdateInstanceInfoScheduler do - let(:worker) { described_class.new } - - before do - stub_request(:get, 'https://example.com/.well-known/nodeinfo').to_return(status: 200, body: '{}') - Fabricate(:account, domain: 'example.com') - Instance.refresh - end - - describe 'perform' do - it 'runs without error' do - expect { worker.perform }.to_not raise_error - end - end -end From e9b69478d139b53e9108b7be063e2a93af1ba7a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?KMY=EF=BC=88=E9=9B=AA=E3=81=82=E3=81=99=E3=81=8B=EF=BC=89?= Date: Wed, 13 Dec 2023 09:13:35 +0900 Subject: [PATCH 2/4] =?UTF-8?q?Fix:=20#355=20LTL=E3=81=AE=E3=82=A4?= =?UTF-8?q?=E3=83=B3=E3=83=87=E3=83=83=E3=82=AF=E3=82=B9=E3=81=8C=E9=81=A9?= =?UTF-8?q?=E5=88=87=E3=81=AB=E3=81=AF=E3=82=89=E3=82=8C=E3=81=A6=E3=81=84?= =?UTF-8?q?=E3=81=AA=E3=81=84=E5=95=8F=E9=A1=8C=20(#356)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .rubocop_todo.yml | 1 + ...improve_index_for_public_timeline_speed.rb | 19 +++++++++++++++++++ db/schema.rb | 6 +++--- 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20231212225737_improve_index_for_public_timeline_speed.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3e93f4432b8f51..eb3d6185518198 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -130,6 +130,7 @@ Naming/VariableNumber: - 'db/migrate/20190820003045_update_statuses_index.rb' - 'db/migrate/20190823221802_add_local_index_to_statuses.rb' - 'db/migrate/20200119112504_add_public_index_to_statuses.rb' + - 'db/migrate/20231212225737_improve_index_for_public_timeline_speed.rb' - 'spec/models/account_spec.rb' - 'spec/models/domain_block_spec.rb' - 'spec/models/user_spec.rb' diff --git a/db/migrate/20231212225737_improve_index_for_public_timeline_speed.rb b/db/migrate/20231212225737_improve_index_for_public_timeline_speed.rb new file mode 100644 index 00000000000000..bcfe83de032fa6 --- /dev/null +++ b/db/migrate/20231212225737_improve_index_for_public_timeline_speed.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class ImproveIndexForPublicTimelineSpeed < ActiveRecord::Migration[7.0] + disable_ddl_transaction! + + def up + add_index :statuses, [:id, :account_id], name: :index_statuses_local_20231213, algorithm: :concurrently, order: { id: :desc }, where: '(local OR (uri IS NULL)) AND deleted_at IS NULL AND visibility IN (0, 10, 11) AND reblog_of_id IS NULL AND ((NOT reply) OR (in_reply_to_account_id = account_id))' + add_index :statuses, [:id, :account_id], name: :index_statuses_public_20231213, algorithm: :concurrently, order: { id: :desc }, where: 'deleted_at IS NULL AND visibility IN (0, 10, 11) AND reblog_of_id IS NULL AND ((NOT reply) OR (in_reply_to_account_id = account_id))' + remove_index :statuses, name: :index_statuses_local_20190824 + remove_index :statuses, name: :index_statuses_public_20200119 + end + + def down + add_index :statuses, [:id, :account_id], name: :index_statuses_local_20190824, algorithm: :concurrently, order: { id: :desc }, where: '(local OR (uri IS NULL)) AND deleted_at IS NULL AND visibility = 0 AND reblog_of_id IS NULL AND ((NOT reply) OR (in_reply_to_account_id = account_id))' + add_index :statuses, [:id, :account_id], name: :index_statuses_public_20200119, algorithm: :concurrently, order: { id: :desc }, where: 'deleted_at IS NULL AND visibility = 0 AND reblog_of_id IS NULL AND ((NOT reply) OR (in_reply_to_account_id = account_id))' + remove_index :statuses, name: :index_statuses_local_20231213 + remove_index :statuses, name: :index_statuses_public_20231213 + end +end diff --git a/db/schema.rb b/db/schema.rb index 24dc5450e1a346..f8fd9283e09ce6 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[7.0].define(version: 2023_10_07_090808) do +ActiveRecord::Schema[7.0].define(version: 2023_12_12_225737) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1193,8 +1193,8 @@ t.index ["account_id", "reblog_of_id", "deleted_at", "searchability"], name: "index_statuses_for_get_following_accounts_to_search", where: "((deleted_at IS NULL) AND (reblog_of_id IS NULL) AND (searchability = ANY (ARRAY[0, 10, 1])))" t.index ["account_id"], name: "index_statuses_on_account_id" t.index ["deleted_at"], name: "index_statuses_on_deleted_at", where: "(deleted_at IS NOT NULL)" - t.index ["id", "account_id"], name: "index_statuses_local_20190824", order: { id: :desc }, where: "((local OR (uri IS NULL)) AND (deleted_at IS NULL) AND (visibility = 0) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" - t.index ["id", "account_id"], name: "index_statuses_public_20200119", order: { id: :desc }, where: "((deleted_at IS NULL) AND (visibility = 0) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" + t.index ["id", "account_id"], name: "index_statuses_local_20231213", order: { id: :desc }, where: "((local OR (uri IS NULL)) AND (deleted_at IS NULL) AND (visibility = ANY (ARRAY[0, 10, 11])) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" + t.index ["id", "account_id"], name: "index_statuses_public_20231213", order: { id: :desc }, where: "((deleted_at IS NULL) AND (visibility = ANY (ARRAY[0, 10, 11])) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" t.index ["in_reply_to_account_id"], name: "index_statuses_on_in_reply_to_account_id", where: "(in_reply_to_account_id IS NOT NULL)" t.index ["in_reply_to_id"], name: "index_statuses_on_in_reply_to_id", where: "(in_reply_to_id IS NOT NULL)" t.index ["reblog_of_id", "account_id"], name: "index_statuses_on_reblog_of_id_and_account_id" From e8b6c16b52f97c1f221b7eccb6ed11716e5ff4d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?KMY=EF=BC=88=E9=9B=AA=E3=81=82=E3=81=99=E3=81=8B=EF=BC=89?= Date: Fri, 15 Dec 2023 09:41:22 +0900 Subject: [PATCH 3/4] Merge pull request from GHSA-qg32-3vrh-w6mw --- app/controllers/concerns/cache_concern.rb | 10 ++++++++ app/controllers/statuses_controller.rb | 31 ++++++++++++++++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/app/controllers/concerns/cache_concern.rb b/app/controllers/concerns/cache_concern.rb index 55ebe1bd649cb9..088b8db56a5eef 100644 --- a/app/controllers/concerns/cache_concern.rb +++ b/app/controllers/concerns/cache_concern.rb @@ -180,6 +180,16 @@ def enforce_cache_control! def render_with_cache(**options) raise ArgumentError, 'Only JSON render calls are supported' unless options.key?(:json) || block_given? + if options.delete(:cancel_cache) + if block_given? + options[:json] = yield + elsif options[:json].is_a?(Symbol) + options[:json] = send(options[:json]) + end + + return render(options) + end + key = options.delete(:key) || [[params[:controller], params[:action]].join('/'), options[:json].respond_to?(:cache_key) ? options[:json].cache_key : nil, options[:fields].nil? ? nil : options[:fields].join(',')].compact.join(':') expires_in = options.delete(:expires_in) || 3.minutes body = Rails.cache.read(key, raw: true) diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 50a8763b7296fa..9ae15a6ed0fcad 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -30,15 +30,15 @@ def show end format.json do - expires_in 3.minutes, public: true if @status.distributable? && public_fetch_mode? - render_with_cache json: @status, content_type: 'application/activity+json', serializer: ActivityPub::NoteSerializer, adapter: ActivityPub::Adapter + expires_in 3.minutes, public: true if @status.distributable? && public_fetch_mode? && !misskey_software? + render_with_cache json: @status, content_type: 'application/activity+json', serializer: status_activity_serializer, adapter: ActivityPub::Adapter, cancel_cache: misskey_software? end end end def activity - expires_in 3.minutes, public: @status.distributable? && public_fetch_mode? - render_with_cache json: ActivityPub::ActivityPresenter.from_status(@status), content_type: 'application/activity+json', serializer: ActivityPub::ActivitySerializer, adapter: ActivityPub::Adapter + expires_in 3.minutes, public: @status.distributable? && public_fetch_mode? && !misskey_software? + render_with_cache json: ActivityPub::ActivityPresenter.from_status(@status, for_misskey: misskey_software?), content_type: 'application/activity+json', serializer: ActivityPub::ActivitySerializer, adapter: ActivityPub::Adapter, cancel_cache: misskey_software? end def embed @@ -76,6 +76,29 @@ def set_instance_presenter @instance_presenter = InstancePresenter.new end + def misskey_software? + return @misskey_software if defined?(@misskey_software) + + @misskey_software = false + + return false if !@status.local? || signed_request_account&.domain.blank? + + info = InstanceInfo.find_by(domain: signed_request_account.domain) + return false if info.nil? + + @misskey_software = %w(misskey calckey cherrypick sharkey).include?(info.software) && + ((@status.public_unlisted_visibility? && @status.account.user&.setting_reject_public_unlisted_subscription) || + (@status.unlisted_visibility? && @status.account.user&.setting_reject_unlisted_subscription)) + end + + def status_activity_serializer + if misskey_software? + ActivityPub::NoteForMisskeySerializer + else + ActivityPub::NoteSerializer + end + end + def redirect_to_original redirect_to(ActivityPub::TagManager.instance.url_for(@status.reblog), allow_other_host: true) if @status.reblog? end From 38105dfbaa12e16dcb848b440672a24c9c705cd7 Mon Sep 17 00:00:00 2001 From: KMY Date: Mon, 11 Dec 2023 14:17:00 +0900 Subject: [PATCH 4/4] Bump version to 5.12 LTS --- lib/mastodon/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index e23a4afa949d79..0e819ce52cc8ca 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -9,7 +9,7 @@ def kmyblue_major end def kmyblue_minor - 11 + 12 end def kmyblue_flag