From eec05a81d63a7e29067512f94e0030b8c30c120f Mon Sep 17 00:00:00 2001 From: Aung Kyaw Phyo Date: Sat, 13 Jun 2026 02:44:23 +0700 Subject: [PATCH] Harden settings auth latency and add settings composite index --- app/controllers/api/v1/settings_controller.rb | 25 +++++++++++++---- app/controllers/api_controller.rb | 10 +++++-- app/services/contributor_search_service.rb | 19 ++++++++++--- app/services/remote_account_verify_service.rb | 27 ++++++++++++------- ...chwork_settings_on_account_and_app_name.rb | 21 +++++++++++++++ 5 files changed, 81 insertions(+), 21 deletions(-) create mode 100644 db/migrate/20260613090000_add_unique_index_to_patchwork_settings_on_account_and_app_name.rb diff --git a/app/controllers/api/v1/settings_controller.rb b/app/controllers/api/v1/settings_controller.rb index 62e4b50e..fe408a0d 100644 --- a/app/controllers/api/v1/settings_controller.rb +++ b/app/controllers/api/v1/settings_controller.rb @@ -9,7 +9,9 @@ class SettingsController < ApiController before_action :set_setting, only: [:destroy] def index - @setting = Setting.find_by(account: @account, app_name: @app_name) + @setting = with_timing('settings.index.find_by') do + Setting.find_by(account: @account, app_name: @app_name) + end if @setting render_success(@setting) @@ -54,15 +56,20 @@ def set_setting end def authenticate_and_set_account + auth_started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + if request.headers['Authorization'].present? && params[:instance_domain].present? - validate_mastodon_account - @account = current_remote_account + with_timing('settings.index.validate_mastodon_account') { validate_mastodon_account } + @account = with_timing('settings.index.current_remote_account') { current_remote_account } else - authenticate_user_from_header - @account = current_account + with_timing('settings.index.authenticate_user_from_header') { authenticate_user_from_header } + @account = with_timing('settings.index.current_account') { current_account } end rescue AuthenticationError render_unauthorized('api.setting.errors.authentication_failed') + ensure + elapsed_ms = ((Process.clock_gettime(Process::CLOCK_MONOTONIC) - auth_started_at) * 1000).round(1) + Rails.logger.info("[settings#index] authenticate_and_set_account=#{elapsed_ms}ms") end def validate_or_set_app_name @@ -100,6 +107,14 @@ def default_setting } }.compact end + + def with_timing(label) + started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + result = yield + elapsed_ms = ((Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at) * 1000).round(1) + Rails.logger.info("[settings#index] #{label}=#{elapsed_ms}ms") + result + end end end end diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 16b42c7b..709057a6 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -1,6 +1,8 @@ class ApiController < ApplicationController respond_to :json + TOKEN_INFO_TIMEOUT_SECONDS = 3 + # Include internationalization support for API controllers include LocaleDetection # Include standardized API response helpers with I18n support @@ -116,9 +118,13 @@ def validate_token(token) else 'http://localhost:3001/oauth/token/info' end - response = HTTParty.get(url, headers: { 'Authorization' => "Bearer #{token}" }) + response = HTTParty.get( + url, + headers: { 'Authorization' => "Bearer #{token}" }, + timeout: TOKEN_INFO_TIMEOUT_SECONDS + ) JSON.parse(response.body) - rescue HTTParty::Error => e + rescue HTTParty::Error, Net::OpenTimeout, Net::ReadTimeout, Timeout::Error, SocketError, JSON::ParserError => e Rails.logger.error "Error fetching user info: #{e.message}" nil end diff --git a/app/services/contributor_search_service.rb b/app/services/contributor_search_service.rb index 9b2be7ce..431abf2c 100644 --- a/app/services/contributor_search_service.rb +++ b/app/services/contributor_search_service.rb @@ -3,6 +3,10 @@ class ContributorSearchService include ActionView::Helpers::AssetTagHelper include ApplicationHelper + SEARCH_REQUEST_TIMEOUT_SECONDS = 3 + SAVED_ACCOUNT_LOOKUP_ATTEMPTS = 3 + SAVED_ACCOUNT_LOOKUP_SLEEP_SECONDS = 1 + def initialize(query, options = {}) @query = query @api_base_url = options[:url] @@ -27,19 +31,26 @@ def search_mastodon }, headers: { 'Authorization' => "Bearer #{@token}" - } + }, + timeout: SEARCH_REQUEST_TIMEOUT_SECONDS ) + rescue HTTParty::Error, Net::OpenTimeout, Net::ReadTimeout, Timeout::Error, SocketError + nil end def find_saved_accounts_with_retry(accounts) return [] unless accounts.present? - saved_accounts = [] - while saved_accounts.empty? + saved_accounts = Account.none + SAVED_ACCOUNT_LOOKUP_ATTEMPTS.times do |attempt| saved_accounts = Account.where(username: accounts.map { |account| account['username'] }) - sleep(2) if saved_accounts.empty? + break if saved_accounts.exists? + + sleep(SAVED_ACCOUNT_LOOKUP_SLEEP_SECONDS) if attempt < SAVED_ACCOUNT_LOOKUP_ATTEMPTS - 1 end + return [] unless saved_accounts.exists? + saved_accounts.map do |account| { diff --git a/app/services/remote_account_verify_service.rb b/app/services/remote_account_verify_service.rb index c2b4134a..284a50af 100644 --- a/app/services/remote_account_verify_service.rb +++ b/app/services/remote_account_verify_service.rb @@ -1,6 +1,9 @@ require 'httparty' class RemoteAccountVerifyService + VERIFY_CREDENTIALS_TIMEOUT_SECONDS = 3 + SEARCH_RETRY_ATTEMPTS = 3 + def initialize(token, domain) @token = token @domain = domain @@ -15,15 +18,21 @@ def call def verify_account_credentials begin url = "https://#{@domain}/api/v1/accounts/verify_credentials" - response = HTTParty.get(url, headers: { 'Authorization' => "Bearer #{@token}" }) + response = HTTParty.get( + url, + headers: { 'Authorization' => "Bearer #{@token}" }, + timeout: VERIFY_CREDENTIALS_TIMEOUT_SECONDS + ) @remote_account = JSON.parse(response.body) - rescue HTTParty::Error => e + rescue HTTParty::Error, Net::OpenTimeout, Net::ReadTimeout, Timeout::Error, SocketError, JSON::ParserError => e Rails.logger.error "Error fetching #{@domain}'s account info: #{e.message}" nil end end def fetch_remote_account_id + return nil unless @remote_account.is_a?(Hash) && @remote_account['username'].present? + # Find account in local server domain = @domain if @domain == 'backend.newsmast.org' @@ -39,20 +48,18 @@ def fetch_remote_account_id end def search_target_account_id(query) - retries = 5 - result = nil - # Owner account's user id owner_user = User.find_by(role: UserRole.find_by(name: 'Owner')) + return nil unless owner_user + token = GenerateAdminAccessTokenService.new(owner_user.id).call + return nil if token.blank? - while retries >= 0 + SEARCH_RETRY_ATTEMPTS.times do result = ContributorSearchService.new(query, url: ENV['MASTODON_INSTANCE_URL'], token: token).call - if result.any? - return result.last['id'] - end - retries -= 1 + return result.last['id'] if result.any? end + nil end end diff --git a/db/migrate/20260613090000_add_unique_index_to_patchwork_settings_on_account_and_app_name.rb b/db/migrate/20260613090000_add_unique_index_to_patchwork_settings_on_account_and_app_name.rb new file mode 100644 index 00000000..89d61ef2 --- /dev/null +++ b/db/migrate/20260613090000_add_unique_index_to_patchwork_settings_on_account_and_app_name.rb @@ -0,0 +1,21 @@ +class AddUniqueIndexToPatchworkSettingsOnAccountAndAppName < ActiveRecord::Migration[7.1] + disable_ddl_transaction! + + INDEX_NAME = 'index_patchwork_settings_on_account_id_and_app_name' + + def up + add_index :patchwork_settings, + [:account_id, :app_name], + unique: true, + name: INDEX_NAME, + algorithm: :concurrently, + if_not_exists: true + end + + def down + remove_index :patchwork_settings, + name: INDEX_NAME, + algorithm: :concurrently, + if_exists: true + end +end