diff --git a/Gemfile.lock b/Gemfile.lock index 094c9b1d6ee..4c232743bf1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -362,7 +362,7 @@ GEM rack (>= 2.2, < 4) rdf (~> 3.3) rexml (~> 3.2) - json-ld-preloaded (3.3.1) + json-ld-preloaded (3.3.2) json-ld (~> 3.3) rdf (~> 3.3) json-schema (5.2.1) @@ -627,7 +627,7 @@ GEM prism (1.4.0) prometheus_exporter (2.2.0) webrick - propshaft (1.2.0) + propshaft (1.2.1) actionpack (>= 7.0.0) activesupport (>= 7.0.0) rack @@ -759,7 +759,7 @@ GEM rspec-expectations (~> 3.13) rspec-mocks (~> 3.13) rspec-support (~> 3.13) - rspec-sidekiq (5.1.0) + rspec-sidekiq (5.2.0) rspec-core (~> 3.0) rspec-expectations (~> 3.0) rspec-mocks (~> 3.0) diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb index 10391aa3e21..e1406930147 100644 --- a/app/controllers/admin/accounts_controller.rb +++ b/app/controllers/admin/accounts_controller.rb @@ -16,11 +16,14 @@ module Admin def batch authorize :account, :index? - @form = Form::AccountBatch.new(form_account_batch_params) - @form.current_account = current_account - @form.action = action_from_button - @form.select_all_matching = params[:select_all_matching] - @form.query = filtered_accounts + @form = Form::AccountBatch.new( + form_account_batch_params.merge( + action: action_from_button, + current_account:, + query: filtered_accounts, + select_all_matching: params[:select_all_matching] + ) + ) @form.save rescue ActionController::ParameterMissing flash[:alert] = I18n.t('admin.accounts.no_account_selected') diff --git a/app/controllers/admin/tags_controller.rb b/app/controllers/admin/tags_controller.rb index a7bfd647944..f2c28328f86 100644 --- a/app/controllers/admin/tags_controller.rb +++ b/app/controllers/admin/tags_controller.rb @@ -5,6 +5,7 @@ module Admin before_action :set_tag, except: [:index] PER_PAGE = 20 + PERIOD_DAYS = 6.days def index authorize :tag, :index? @@ -15,7 +16,7 @@ module Admin def show authorize @tag, :show? - @time_period = (6.days.ago.to_date...Time.now.utc.to_date) + @time_period = report_range end def update @@ -24,7 +25,7 @@ module Admin if @tag.update(tag_params.merge(reviewed_at: Time.now.utc)) redirect_to admin_tag_path(@tag.id), notice: I18n.t('admin.tags.updated_msg') else - @time_period = (6.days.ago.to_date...Time.now.utc.to_date) + @time_period = report_range render :show end @@ -36,6 +37,10 @@ module Admin @tag = Tag.find(params[:id]) end + def report_range + (PERIOD_DAYS.ago.to_date...Time.now.utc.to_date) + end + def tag_params params .expect(tag: [:name, :display_name, :trendable, :usable, :listable]) diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index d3b0e89e97b..e25b161afd8 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -2,6 +2,7 @@ class Api::V1::StatusesController < Api::BaseController include Authorization + include AsyncRefreshesConcern before_action -> { authorize_if_got_token! :read, :'read:statuses' }, except: [:create, :update, :destroy] before_action -> { doorkeeper_authorize! :write, :'write:statuses' }, only: [:create, :update, :destroy] @@ -57,9 +58,17 @@ class Api::V1::StatusesController < Api::BaseController @context = Context.new(ancestors: loaded_ancestors, descendants: loaded_descendants) statuses = [@status] + @context.ancestors + @context.descendants - render json: @context, serializer: REST::ContextSerializer, relationships: StatusRelationshipsPresenter.new(statuses, current_user&.account_id) + refresh_key = "context:#{@status.id}:refresh" + async_refresh = AsyncRefresh.new(refresh_key) - ActivityPub::FetchAllRepliesWorker.perform_async(@status.id) if !current_account.nil? && @status.should_fetch_replies? + if async_refresh.running? + add_async_refresh_header(async_refresh) + elsif !current_account.nil? && @status.should_fetch_replies? + add_async_refresh_header(AsyncRefresh.create(refresh_key)) + ActivityPub::FetchAllRepliesWorker.perform_async(@status.id) + end + + render json: @context, serializer: REST::ContextSerializer, relationships: StatusRelationshipsPresenter.new(statuses, current_user&.account_id) end def create diff --git a/app/controllers/concerns/auth/captcha_concern.rb b/app/controllers/concerns/auth/captcha_concern.rb index c01da212499..a6232db943b 100644 --- a/app/controllers/concerns/auth/captcha_concern.rb +++ b/app/controllers/concerns/auth/captcha_concern.rb @@ -5,6 +5,18 @@ module Auth::CaptchaConcern include Hcaptcha::Adapters::ViewMethods + CAPTCHA_DIRECTIVES = %w( + connect_src + frame_src + script_src + style_src + ).freeze + + CAPTCHA_SOURCES = %w( + https://*.hcaptcha.com + https://hcaptcha.com + ).freeze + included do helper_method :render_captcha end @@ -42,20 +54,9 @@ module Auth::CaptchaConcern end def extend_csp_for_captcha! - policy = request.content_security_policy&.clone + return unless captcha_required? && request.content_security_policy.present? - return unless captcha_required? && policy.present? - - %w(script_src frame_src style_src connect_src).each do |directive| - values = policy.send(directive) - - values << 'https://hcaptcha.com' unless values.include?('https://hcaptcha.com') || values.include?('https:') - values << 'https://*.hcaptcha.com' unless values.include?('https://*.hcaptcha.com') || values.include?('https:') - - policy.send(directive, *values) - end - - request.content_security_policy = policy + request.content_security_policy = captcha_adjusted_policy end def render_captcha @@ -63,4 +64,24 @@ module Auth::CaptchaConcern hcaptcha_tags end + + private + + def captcha_adjusted_policy + request.content_security_policy.clone.tap do |policy| + populate_captcha_policy(policy) + end + end + + def populate_captcha_policy(policy) + CAPTCHA_DIRECTIVES.each do |directive| + values = policy.send(directive) + + CAPTCHA_SOURCES.each do |source| + values << source unless values.include?(source) || values.include?('https:') + end + + policy.send(directive, *values) + end + end end diff --git a/app/javascript/mastodon/actions/statuses_typed.ts b/app/javascript/mastodon/actions/statuses_typed.ts index b98abbe122e..cc9c389cdab 100644 --- a/app/javascript/mastodon/actions/statuses_typed.ts +++ b/app/javascript/mastodon/actions/statuses_typed.ts @@ -1,3 +1,5 @@ +import { createAction } from '@reduxjs/toolkit'; + import { apiGetContext } from 'mastodon/api/statuses'; import { createDataLoadingThunk } from 'mastodon/store/typed_functions'; @@ -6,13 +8,18 @@ import { importFetchedStatuses } from './importer'; export const fetchContext = createDataLoadingThunk( 'status/context', ({ statusId }: { statusId: string }) => apiGetContext(statusId), - (context, { dispatch }) => { + ({ context, refresh }, { dispatch }) => { const statuses = context.ancestors.concat(context.descendants); dispatch(importFetchedStatuses(statuses)); return { context, + refresh, }; }, ); + +export const completeContextRefresh = createAction<{ statusId: string }>( + 'status/context/complete', +); diff --git a/app/javascript/mastodon/api.ts b/app/javascript/mastodon/api.ts index dc9c20b5085..1820e00a537 100644 --- a/app/javascript/mastodon/api.ts +++ b/app/javascript/mastodon/api.ts @@ -20,6 +20,50 @@ export const getLinks = (response: AxiosResponse) => { return LinkHeader.parse(value); }; +export interface AsyncRefreshHeader { + id: string; + retry: number; +} + +const isAsyncRefreshHeader = (obj: object): obj is AsyncRefreshHeader => + 'id' in obj && 'retry' in obj; + +export const getAsyncRefreshHeader = ( + response: AxiosResponse, +): AsyncRefreshHeader | null => { + const value = response.headers['mastodon-async-refresh'] as + | string + | undefined; + + if (!value) { + return null; + } + + const asyncRefreshHeader: Record = {}; + + value.split(/,\s*/).forEach((pair) => { + const [key, val] = pair.split('=', 2); + + let typedValue: string | number; + + if (key && ['id', 'retry'].includes(key) && val) { + if (val.startsWith('"')) { + typedValue = val.slice(1, -1); + } else { + typedValue = parseInt(val); + } + + asyncRefreshHeader[key] = typedValue; + } + }); + + if (isAsyncRefreshHeader(asyncRefreshHeader)) { + return asyncRefreshHeader; + } + + return null; +}; + const csrfHeader: RawAxiosRequestHeaders = {}; const setCSRFHeader = () => { @@ -83,7 +127,7 @@ export default function api(withAuthorization = true) { return instance; } -type ApiUrl = `v${1 | 2}/${string}`; +type ApiUrl = `v${1 | '1_alpha' | 2}/${string}`; type RequestParamsOrData = Record; export async function apiRequest( diff --git a/app/javascript/mastodon/api/async_refreshes.ts b/app/javascript/mastodon/api/async_refreshes.ts new file mode 100644 index 00000000000..953300a4a86 --- /dev/null +++ b/app/javascript/mastodon/api/async_refreshes.ts @@ -0,0 +1,5 @@ +import { apiRequestGet } from 'mastodon/api'; +import type { ApiAsyncRefreshJSON } from 'mastodon/api_types/async_refreshes'; + +export const apiGetAsyncRefresh = (id: string) => + apiRequestGet(`v1_alpha/async_refreshes/${id}`); diff --git a/app/javascript/mastodon/api/statuses.ts b/app/javascript/mastodon/api/statuses.ts index 921a7bfe636..48eff2a692f 100644 --- a/app/javascript/mastodon/api/statuses.ts +++ b/app/javascript/mastodon/api/statuses.ts @@ -1,5 +1,14 @@ -import { apiRequestGet } from 'mastodon/api'; +import api, { getAsyncRefreshHeader } from 'mastodon/api'; import type { ApiContextJSON } from 'mastodon/api_types/statuses'; -export const apiGetContext = (statusId: string) => - apiRequestGet(`v1/statuses/${statusId}/context`); +export const apiGetContext = async (statusId: string) => { + const response = await api().request({ + method: 'GET', + url: `/api/v1/statuses/${statusId}/context`, + }); + + return { + context: response.data, + refresh: getAsyncRefreshHeader(response), + }; +}; diff --git a/app/javascript/mastodon/api_types/async_refreshes.ts b/app/javascript/mastodon/api_types/async_refreshes.ts new file mode 100644 index 00000000000..2d2fed24127 --- /dev/null +++ b/app/javascript/mastodon/api_types/async_refreshes.ts @@ -0,0 +1,7 @@ +export interface ApiAsyncRefreshJSON { + async_refresh: { + id: string; + status: 'running' | 'finished'; + result_count: number; + }; +} diff --git a/app/javascript/mastodon/components/status_list.jsx b/app/javascript/mastodon/components/status_list.jsx index cca449b0ca8..70b7968fba1 100644 --- a/app/javascript/mastodon/components/status_list.jsx +++ b/app/javascript/mastodon/components/status_list.jsx @@ -40,6 +40,12 @@ export default class StatusList extends ImmutablePureComponent { trackScroll: true, }; + componentDidMount() { + this.columnHeaderHeight = parseFloat( + getComputedStyle(this.node.node).getPropertyValue('--column-header-height') + ) || 0; + } + getFeaturedStatusCount = () => { return this.props.featuredStatusIds ? this.props.featuredStatusIds.size : 0; }; @@ -53,35 +59,68 @@ export default class StatusList extends ImmutablePureComponent { }; handleMoveUp = (id, featured) => { - const elementIndex = this.getCurrentStatusIndex(id, featured) - 1; - this._selectChild(elementIndex, true); + const index = this.getCurrentStatusIndex(id, featured); + this._selectChild(id, index, -1); }; handleMoveDown = (id, featured) => { - const elementIndex = this.getCurrentStatusIndex(id, featured) + 1; - this._selectChild(elementIndex, false); + const index = this.getCurrentStatusIndex(id, featured); + this._selectChild(id, index, 1); }; + _selectChild = (id, index, direction) => { + const listContainer = this.node.node; + let listItem = listContainer.querySelector( + // :nth-child uses 1-based indexing + `.item-list > :nth-child(${index + 1 + direction})` + ); + + if (!listItem) { + return; + } + + // If selected container element is empty, we skip it + if (listItem.matches(':empty')) { + this._selectChild(id, index + direction, direction); + return; + } + + // Check if the list item is a post + let targetElement = listItem.querySelector('.focusable'); + + // Otherwise, check if the item contains follow suggestions or + // is a 'load more' button. + if ( + !targetElement && ( + listItem.querySelector('.inline-follow-suggestions') || + listItem.matches('.load-more') + ) + ) { + targetElement = listItem; + } + + if (targetElement) { + const elementRect = targetElement.getBoundingClientRect(); + + const isFullyVisible = + elementRect.top >= this.columnHeaderHeight && + elementRect.bottom <= window.innerHeight; + + if (!isFullyVisible) { + targetElement.scrollIntoView({ + block: direction === 1 ? 'start' : 'center', + }); + } + + targetElement.focus(); + } + } + handleLoadOlder = debounce(() => { const { statusIds, lastId, onLoadMore } = this.props; onLoadMore(lastId || (statusIds.size > 0 ? statusIds.last() : undefined)); }, 300, { leading: true }); - _selectChild (index, align_top) { - const container = this.node.node; - // TODO: This breaks at the inline-follow-suggestions container - const element = container.querySelector(`article:nth-of-type(${index + 1}) .focusable`); - - if (element) { - if (align_top && container.scrollTop > element.offsetTop) { - element.scrollIntoView(true); - } else if (!align_top && container.scrollTop + container.clientHeight < element.offsetTop + element.offsetHeight) { - element.scrollIntoView(false); - } - element.focus(); - } - } - setRef = c => { this.node = c; }; diff --git a/app/javascript/mastodon/features/emoji/index.ts b/app/javascript/mastodon/features/emoji/index.ts index ef6cd67aeb5..4f23dc5395e 100644 --- a/app/javascript/mastodon/features/emoji/index.ts +++ b/app/javascript/mastodon/features/emoji/index.ts @@ -4,19 +4,27 @@ import { toSupportedLocale } from './locale'; const userLocale = toSupportedLocale(initialState?.meta.locale ?? 'en'); -const worker = - 'Worker' in window - ? new Worker(new URL('./worker', import.meta.url), { - type: 'module', - }) - : null; +let worker: Worker | null = null; export async function initializeEmoji() { + if (!worker && 'Worker' in window) { + try { + worker = new Worker(new URL('./worker', import.meta.url), { + type: 'module', + credentials: 'omit', + }); + } catch (err) { + console.warn('Error creating web worker:', err); + } + } + if (worker) { - worker.addEventListener('message', (event: MessageEvent) => { + // Assign worker to const to make TS happy inside the event listener. + const thisWorker = worker; + thisWorker.addEventListener('message', (event: MessageEvent) => { const { data: message } = event; if (message === 'ready') { - worker.postMessage('custom'); + thisWorker.postMessage('custom'); void loadEmojiLocale(userLocale); // Load English locale as well, because people are still used to // using it from before we supported other locales. diff --git a/app/javascript/mastodon/features/emoji/render.ts b/app/javascript/mastodon/features/emoji/render.ts index 8f0c1ee15fe..6ef9492147c 100644 --- a/app/javascript/mastodon/features/emoji/render.ts +++ b/app/javascript/mastodon/features/emoji/render.ts @@ -4,7 +4,6 @@ import EMOJI_REGEX from 'emojibase-regex/emoji-loose'; import { autoPlayGif } from '@/mastodon/initial_state'; import { assetHost } from '@/mastodon/utils/config'; -import { loadEmojiLocale } from '.'; import { EMOJI_MODE_NATIVE, EMOJI_MODE_NATIVE_WITH_FLAGS, @@ -17,6 +16,7 @@ import { searchCustomEmojisByShortcodes, searchEmojisByHexcodes, } from './database'; +import { loadEmojiLocale } from './index'; import { emojiToUnicodeHex, twemojiHasBorder, diff --git a/app/javascript/mastodon/features/status/components/refresh_controller.tsx b/app/javascript/mastodon/features/status/components/refresh_controller.tsx new file mode 100644 index 00000000000..04046302b62 --- /dev/null +++ b/app/javascript/mastodon/features/status/components/refresh_controller.tsx @@ -0,0 +1,111 @@ +import { useEffect, useState, useCallback } from 'react'; + +import { useIntl, defineMessages, FormattedMessage } from 'react-intl'; + +import classNames from 'classnames'; + +import { + fetchContext, + completeContextRefresh, +} from 'mastodon/actions/statuses'; +import type { AsyncRefreshHeader } from 'mastodon/api'; +import { apiGetAsyncRefresh } from 'mastodon/api/async_refreshes'; +import { LoadingIndicator } from 'mastodon/components/loading_indicator'; +import { useAppSelector, useAppDispatch } from 'mastodon/store'; + +const messages = defineMessages({ + loading: { + id: 'status.context.loading', + defaultMessage: 'Checking for more replies', + }, +}); + +export const RefreshController: React.FC<{ + statusId: string; + withBorder?: boolean; +}> = ({ statusId, withBorder }) => { + const refresh = useAppSelector( + (state) => state.contexts.refreshing[statusId], + ); + const dispatch = useAppDispatch(); + const intl = useIntl(); + const [ready, setReady] = useState(false); + const [loading, setLoading] = useState(false); + + useEffect(() => { + let timeoutId: ReturnType; + + const scheduleRefresh = (refresh: AsyncRefreshHeader) => { + timeoutId = setTimeout(() => { + void apiGetAsyncRefresh(refresh.id).then((result) => { + if (result.async_refresh.status === 'finished') { + dispatch(completeContextRefresh({ statusId })); + + if (result.async_refresh.result_count > 0) { + setReady(true); + } + } else { + scheduleRefresh(refresh); + } + + return ''; + }); + }, refresh.retry * 1000); + }; + + if (refresh) { + scheduleRefresh(refresh); + } + + return () => { + clearTimeout(timeoutId); + }; + }, [dispatch, setReady, statusId, refresh]); + + const handleClick = useCallback(() => { + setLoading(true); + setReady(false); + + dispatch(fetchContext({ statusId })) + .then(() => { + setLoading(false); + return ''; + }) + .catch(() => { + setLoading(false); + }); + }, [dispatch, setReady, statusId]); + + if (ready && !loading) { + return ( + + ); + } + + if (!refresh && !loading) { + return null; + } + + return ( +
+ +
+ ); +}; diff --git a/app/javascript/mastodon/features/status/index.jsx b/app/javascript/mastodon/features/status/index.jsx index 64cd0c4f825..77d23f55f6d 100644 --- a/app/javascript/mastodon/features/status/index.jsx +++ b/app/javascript/mastodon/features/status/index.jsx @@ -68,7 +68,7 @@ import { attachFullscreenListener, detachFullscreenListener, isFullscreen } from import ActionBar from './components/action_bar'; import { DetailedStatus } from './components/detailed_status'; - +import { RefreshController } from './components/refresh_controller'; const messages = defineMessages({ revealAll: { id: 'status.show_more_all', defaultMessage: 'Show more for all' }, @@ -548,7 +548,7 @@ class Status extends ImmutablePureComponent { render () { let ancestors, descendants, remoteHint; - const { isLoading, status, ancestorsIds, descendantsIds, intl, domain, multiColumn, pictureInPicture } = this.props; + const { isLoading, status, ancestorsIds, descendantsIds, refresh, intl, domain, multiColumn, pictureInPicture } = this.props; const { fullscreen } = this.state; if (isLoading) { @@ -578,11 +578,9 @@ class Status extends ImmutablePureComponent { if (!isLocal) { remoteHint = ( - } - label={{status.getIn(['account', 'acct']).split('@')[1]} }} />} + ); } diff --git a/app/javascript/mastodon/locales/en.json b/app/javascript/mastodon/locales/en.json index 59d39a15361..13b7aa42121 100644 --- a/app/javascript/mastodon/locales/en.json +++ b/app/javascript/mastodon/locales/en.json @@ -424,8 +424,6 @@ "hints.profiles.see_more_followers": "See more followers on {domain}", "hints.profiles.see_more_follows": "See more follows on {domain}", "hints.profiles.see_more_posts": "See more posts on {domain}", - "hints.threads.replies_may_be_missing": "Replies from other servers may be missing.", - "hints.threads.see_more": "See more replies on {domain}", "home.column_settings.show_quotes": "Show quotes", "home.column_settings.show_reblogs": "Show boosts", "home.column_settings.show_replies": "Show replies", @@ -847,6 +845,8 @@ "status.bookmark": "Bookmark", "status.cancel_reblog_private": "Unboost", "status.cannot_reblog": "This post cannot be boosted", + "status.context.load_new_replies": "New replies available", + "status.context.loading": "Checking for more replies", "status.continued_thread": "Continued thread", "status.copy": "Copy link to post", "status.delete": "Delete", diff --git a/app/javascript/mastodon/reducers/contexts.ts b/app/javascript/mastodon/reducers/contexts.ts index 7ecc6e3b29f..cf378b4c048 100644 --- a/app/javascript/mastodon/reducers/contexts.ts +++ b/app/javascript/mastodon/reducers/contexts.ts @@ -4,6 +4,7 @@ import type { Draft, UnknownAction } from '@reduxjs/toolkit'; import type { List as ImmutableList } from 'immutable'; import { timelineDelete } from 'mastodon/actions/timelines_typed'; +import type { AsyncRefreshHeader } from 'mastodon/api'; import type { ApiRelationshipJSON } from 'mastodon/api_types/relationships'; import type { ApiStatusJSON, @@ -12,7 +13,7 @@ import type { import type { Status } from 'mastodon/models/status'; import { blockAccountSuccess, muteAccountSuccess } from '../actions/accounts'; -import { fetchContext } from '../actions/statuses'; +import { fetchContext, completeContextRefresh } from '../actions/statuses'; import { TIMELINE_UPDATE } from '../actions/timelines'; import { compareId } from '../compare_id'; @@ -25,11 +26,13 @@ interface TimelineUpdateAction extends UnknownAction { interface State { inReplyTos: Record; replies: Record; + refreshing: Record; } const initialState: State = { inReplyTos: {}, replies: {}, + refreshing: {}, }; const normalizeContext = ( @@ -127,6 +130,13 @@ export const contextsReducer = createReducer(initialState, (builder) => { builder .addCase(fetchContext.fulfilled, (state, action) => { normalizeContext(state, action.meta.arg.statusId, action.payload.context); + + if (action.payload.refresh) { + state.refreshing[action.meta.arg.statusId] = action.payload.refresh; + } + }) + .addCase(completeContextRefresh, (state, action) => { + delete state.refreshing[action.payload.statusId]; }) .addCase(blockAccountSuccess, (state, action) => { filterContexts( diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 40b073f68b9..0fd97fb7129 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -2868,6 +2868,8 @@ a.account__display-name { } &__main { + --column-header-height: 62px; + box-sizing: border-box; width: 100%; flex: 0 1 auto; @@ -8815,6 +8817,10 @@ noscript { .conversation { position: relative; + // When scrolling these elements into view, take into account + // the column header height + scroll-margin-top: var(--column-header-height, 0); + &.unread { &::before { content: ''; diff --git a/app/models/concerns/status/fetch_replies_concern.rb b/app/models/concerns/status/fetch_replies_concern.rb index fd9929aba49..cc117cb5ac6 100644 --- a/app/models/concerns/status/fetch_replies_concern.rb +++ b/app/models/concerns/status/fetch_replies_concern.rb @@ -3,9 +3,6 @@ module Status::FetchRepliesConcern extend ActiveSupport::Concern - # enable/disable fetching all replies - FETCH_REPLIES_ENABLED = ENV['FETCH_REPLIES_ENABLED'] == 'true' - # debounce fetching all replies to minimize DoS FETCH_REPLIES_COOLDOWN_MINUTES = (ENV['FETCH_REPLIES_COOLDOWN_MINUTES'] || 15).to_i.minutes FETCH_REPLIES_INITIAL_WAIT_MINUTES = (ENV['FETCH_REPLIES_INITIAL_WAIT_MINUTES'] || 5).to_i.minutes @@ -36,7 +33,7 @@ module Status::FetchRepliesConcern def should_fetch_replies? # we aren't brand new, and we haven't fetched replies since the debounce window - FETCH_REPLIES_ENABLED && !local? && created_at <= FETCH_REPLIES_INITIAL_WAIT_MINUTES.ago && ( + !local? && created_at <= FETCH_REPLIES_INITIAL_WAIT_MINUTES.ago && ( fetched_replies_at.nil? || fetched_replies_at <= FETCH_REPLIES_COOLDOWN_MINUTES.ago ) end diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb index 98e3be1a0c9..f3109ad62a7 100644 --- a/app/models/form/account_batch.rb +++ b/app/models/form/account_batch.rb @@ -1,13 +1,11 @@ # frozen_string_literal: true -class Form::AccountBatch - include ActiveModel::Model - include Authorization - include AccountableConcern +class Form::AccountBatch < Form::BaseBatch include Payloadable - attr_accessor :account_ids, :action, :current_account, - :select_all_matching, :query + attr_accessor :account_ids, + :query, + :select_all_matching def save case action diff --git a/app/models/form/base_batch.rb b/app/models/form/base_batch.rb new file mode 100644 index 00000000000..d3af923784a --- /dev/null +++ b/app/models/form/base_batch.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class Form::BaseBatch + include ActiveModel::Model + include Authorization + include AccountableConcern + + attr_accessor :action, + :current_account + + def save + raise 'Override in subclass' + end +end diff --git a/app/models/form/custom_emoji_batch.rb b/app/models/form/custom_emoji_batch.rb index c63996e0695..b8cfb363999 100644 --- a/app/models/form/custom_emoji_batch.rb +++ b/app/models/form/custom_emoji_batch.rb @@ -1,12 +1,10 @@ # frozen_string_literal: true -class Form::CustomEmojiBatch - include ActiveModel::Model - include Authorization - include AccountableConcern - - attr_accessor :custom_emoji_ids, :action, :current_account, - :category_id, :category_name, :visible_in_picker +class Form::CustomEmojiBatch < Form::BaseBatch + attr_accessor :category_id, + :category_name, + :visible_in_picker, + :custom_emoji_ids def save case action diff --git a/app/models/form/domain_block_batch.rb b/app/models/form/domain_block_batch.rb index 39012df5173..af792fd41f6 100644 --- a/app/models/form/domain_block_batch.rb +++ b/app/models/form/domain_block_batch.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true -class Form::DomainBlockBatch - include ActiveModel::Model - include Authorization - include AccountableConcern - - attr_accessor :domain_blocks_attributes, :action, :current_account +class Form::DomainBlockBatch < Form::BaseBatch + attr_accessor :domain_blocks_attributes def save case action diff --git a/app/models/form/email_domain_block_batch.rb b/app/models/form/email_domain_block_batch.rb index df120182bc2..6292f2b1e12 100644 --- a/app/models/form/email_domain_block_batch.rb +++ b/app/models/form/email_domain_block_batch.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true -class Form::EmailDomainBlockBatch - include ActiveModel::Model - include Authorization - include AccountableConcern - - attr_accessor :email_domain_block_ids, :action, :current_account +class Form::EmailDomainBlockBatch < Form::BaseBatch + attr_accessor :email_domain_block_ids def save case action diff --git a/app/models/form/ip_block_batch.rb b/app/models/form/ip_block_batch.rb index bdfeb91c8a8..b6a189750e9 100644 --- a/app/models/form/ip_block_batch.rb +++ b/app/models/form/ip_block_batch.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true -class Form::IpBlockBatch - include ActiveModel::Model - include Authorization - include AccountableConcern - - attr_accessor :ip_block_ids, :action, :current_account +class Form::IpBlockBatch < Form::BaseBatch + attr_accessor :ip_block_ids def save case action diff --git a/app/models/worker_batch.rb b/app/models/worker_batch.rb new file mode 100644 index 00000000000..f741071ba95 --- /dev/null +++ b/app/models/worker_batch.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +class WorkerBatch + include Redisable + + TTL = 3600 + + def initialize(id = nil) + @id = id || SecureRandom.hex(12) + end + + attr_reader :id + + # Connect the batch with an async refresh. When the number of processed jobs + # passes the given threshold, the async refresh will be marked as finished. + # @param [String] async_refresh_key + # @param [Float] threshold + def connect(async_refresh_key, threshold: 1.0) + redis.hset(key, { 'async_refresh_key' => async_refresh_key, 'threshold' => threshold }) + end + + # Add jobs to the batch. Usually when the batch is created. + # @param [Array] jids + def add_jobs(jids) + if jids.blank? + async_refresh_key = redis.hget(key, 'async_refresh_key') + + if async_refresh_key.present? + async_refresh = AsyncRefresh.new(async_refresh_key) + async_refresh.finish! + end + + return + end + + redis.multi do |pipeline| + pipeline.sadd(key('jobs'), jids) + pipeline.expire(key('jobs'), TTL) + pipeline.hincrby(key, 'pending', jids.size) + pipeline.expire(key, TTL) + end + end + + # Remove a job from the batch, such as when it's been processed or it has failed. + # @param [String] jid + def remove_job(jid) + _, pending, processed, async_refresh_key, threshold = redis.multi do |pipeline| + pipeline.srem(key('jobs'), jid) + pipeline.hincrby(key, 'pending', -1) + pipeline.hincrby(key, 'processed', 1) + pipeline.hget(key, 'async_refresh_key') + pipeline.hget(key, 'threshold') + end + + if async_refresh_key.present? + async_refresh = AsyncRefresh.new(async_refresh_key) + async_refresh.increment_result_count(by: 1) + async_refresh.finish! if pending.zero? || processed >= threshold.to_f * (processed + pending) + end + end + + # Get pending jobs. + # @returns [Array] + def jobs + redis.smembers(key('jobs')) + end + + # Inspect the batch. + # @returns [Hash] + def info + redis.hgetall(key) + end + + private + + def key(suffix = nil) + "worker_batch:#{@id}#{":#{suffix}" if suffix}" + end +end diff --git a/app/services/activitypub/fetch_all_replies_service.rb b/app/services/activitypub/fetch_all_replies_service.rb index 765e5c8ae82..e9c1712ed66 100644 --- a/app/services/activitypub/fetch_all_replies_service.rb +++ b/app/services/activitypub/fetch_all_replies_service.rb @@ -6,7 +6,7 @@ class ActivityPub::FetchAllRepliesService < ActivityPub::FetchRepliesService # Limit of replies to fetch per status MAX_REPLIES = (ENV['FETCH_REPLIES_MAX_SINGLE'] || 500).to_i - def call(status_uri, collection_or_uri, max_pages: 1, request_id: nil) + def call(status_uri, collection_or_uri, max_pages: 1, async_refresh_key: nil, request_id: nil) @status_uri = status_uri super diff --git a/app/services/activitypub/fetch_replies_service.rb b/app/services/activitypub/fetch_replies_service.rb index 6a6d9e391a4..25eb275ca5c 100644 --- a/app/services/activitypub/fetch_replies_service.rb +++ b/app/services/activitypub/fetch_replies_service.rb @@ -6,7 +6,7 @@ class ActivityPub::FetchRepliesService < BaseService # Limit of fetched replies MAX_REPLIES = 5 - def call(reference_uri, collection_or_uri, max_pages: 1, allow_synchronous_requests: true, request_id: nil) + def call(reference_uri, collection_or_uri, max_pages: 1, allow_synchronous_requests: true, async_refresh_key: nil, request_id: nil) @reference_uri = reference_uri @allow_synchronous_requests = allow_synchronous_requests @@ -14,7 +14,10 @@ class ActivityPub::FetchRepliesService < BaseService return if @items.nil? @items = filter_replies(@items) - FetchReplyWorker.push_bulk(@items) { |reply_uri| [reply_uri, { 'request_id' => request_id }] } + + batch = WorkerBatch.new + batch.connect(async_refresh_key) if async_refresh_key.present? + batch.add_jobs(FetchReplyWorker.push_bulk(@items) { |reply_uri| [reply_uri, { 'request_id' => request_id, 'batch_id' => batch.id }] }) [@items, n_pages] end diff --git a/app/workers/activitypub/fetch_all_replies_worker.rb b/app/workers/activitypub/fetch_all_replies_worker.rb index 40b251cf148..ab9eebc4ec7 100644 --- a/app/workers/activitypub/fetch_all_replies_worker.rb +++ b/app/workers/activitypub/fetch_all_replies_worker.rb @@ -55,7 +55,7 @@ class ActivityPub::FetchAllRepliesWorker replies_collection_or_uri = get_replies_uri(status) return if replies_collection_or_uri.nil? - ActivityPub::FetchAllRepliesService.new.call(value_or_id(status), replies_collection_or_uri, max_pages: max_pages, **options.deep_symbolize_keys) + ActivityPub::FetchAllRepliesService.new.call(value_or_id(status), replies_collection_or_uri, max_pages: max_pages, async_refresh_key: "context:#{@root_status.id}:refresh", **options.deep_symbolize_keys) end # Get the URI of the replies collection of a status diff --git a/app/workers/fetch_reply_worker.rb b/app/workers/fetch_reply_worker.rb index ecb232bbbb0..da3b9a8c131 100644 --- a/app/workers/fetch_reply_worker.rb +++ b/app/workers/fetch_reply_worker.rb @@ -7,6 +7,9 @@ class FetchReplyWorker sidekiq_options queue: 'pull', retry: 3 def perform(child_url, options = {}) + batch = WorkerBatch.new(options.delete('batch_id')) if options['batch_id'] FetchRemoteStatusService.new.call(child_url, **options.symbolize_keys) + ensure + batch&.remove_job(jid) end end diff --git a/spec/models/worker_batch_spec.rb b/spec/models/worker_batch_spec.rb new file mode 100644 index 00000000000..b58dc48618a --- /dev/null +++ b/spec/models/worker_batch_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe WorkerBatch do + subject { described_class.new } + + let(:async_refresh_key) { 'test_refresh' } + let(:async_refresh) { nil } + + describe '#id' do + it 'returns a string' do + expect(subject.id).to be_a String + end + end + + describe '#connect' do + before do + subject.connect(async_refresh_key, threshold: 0.75) + end + + it 'persists the async refresh key' do + expect(subject.info['async_refresh_key']).to eq async_refresh_key + end + + it 'persists the threshold' do + expect(subject.info['threshold']).to eq '0.75' + end + end + + describe '#add_jobs' do + before do + subject.connect(async_refresh_key, threshold: 0.5) if async_refresh.present? + subject.add_jobs([]) + end + + context 'when called with empty array' do + it 'does not persist the number of pending jobs' do + expect(subject.info).to be_empty + end + + it 'does not persist the job IDs' do + expect(subject.jobs).to eq [] + end + + context 'when async refresh is connected' do + let(:async_refresh) { AsyncRefresh.new(async_refresh_key) } + + it 'immediately marks the async refresh as finished' do + expect(async_refresh.reload.finished?).to be true + end + end + end + + context 'when called with an array of job IDs' do + before do + subject.add_jobs(%w(foo bar)) + end + + it 'persists the number of pending jobs' do + expect(subject.info['pending']).to eq '2' + end + + it 'persists the job IDs' do + expect(subject.jobs).to eq %w(foo bar) + end + end + end + + describe '#remove_job' do + before do + subject.connect(async_refresh_key, threshold: 0.5) if async_refresh.present? + subject.add_jobs(%w(foo bar baz)) + subject.remove_job('foo') + end + + it 'removes the job from pending jobs' do + expect(subject.jobs).to eq %w(bar baz) + end + + it 'decrements the number of pending jobs' do + expect(subject.info['pending']).to eq '2' + end + + context 'when async refresh is connected' do + let(:async_refresh) { AsyncRefresh.new(async_refresh_key) } + + it 'increments async refresh progress' do + expect(async_refresh.reload.result_count).to eq 1 + end + + it 'marks the async refresh as finished when the threshold is reached' do + subject.remove_job('bar') + expect(async_refresh.reload.finished?).to be true + end + end + end + + describe '#info' do + it 'returns a hash' do + expect(subject.info).to be_a Hash + end + end +end diff --git a/spec/workers/activitypub/fetch_all_replies_worker_spec.rb b/spec/workers/activitypub/fetch_all_replies_worker_spec.rb index 9a8bdac0307..9795c4619a1 100644 --- a/spec/workers/activitypub/fetch_all_replies_worker_spec.rb +++ b/spec/workers/activitypub/fetch_all_replies_worker_spec.rb @@ -123,7 +123,6 @@ RSpec.describe ActivityPub::FetchAllRepliesWorker do end before do - stub_const('Status::FetchRepliesConcern::FETCH_REPLIES_ENABLED', true) all_items.each do |item| next if [top_note_uri, reply_note_uri].include? item diff --git a/yarn.lock b/yarn.lock index 147da72ad12..67739e74d4c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3294,23 +3294,7 @@ __metadata: languageName: node linkType: hard -"@rollup/pluginutils@npm:^5.0.1, @rollup/pluginutils@npm:^5.0.2, @rollup/pluginutils@npm:^5.1.0": - version: 5.1.4 - resolution: "@rollup/pluginutils@npm:5.1.4" - dependencies: - "@types/estree": "npm:^1.0.0" - estree-walker: "npm:^2.0.2" - picomatch: "npm:^4.0.2" - peerDependencies: - rollup: ^1.20.0||^2.0.0||^3.0.0||^4.0.0 - peerDependenciesMeta: - rollup: - optional: true - checksum: 10c0/6d58fbc6f1024eb4b087bc9bf59a1d655a8056a60c0b4021d3beaeec3f0743503f52467fd89d2cf0e7eccf2831feb40a05ad541a17637ea21ba10b21c2004deb - languageName: node - linkType: hard - -"@rollup/pluginutils@npm:^5.1.3": +"@rollup/pluginutils@npm:^5.0.1, @rollup/pluginutils@npm:^5.0.2, @rollup/pluginutils@npm:^5.1.0, @rollup/pluginutils@npm:^5.1.3": version: 5.2.0 resolution: "@rollup/pluginutils@npm:5.2.0" dependencies: @@ -5389,13 +5373,13 @@ __metadata: linkType: hard "axios@npm:^1.4.0": - version: 1.10.0 - resolution: "axios@npm:1.10.0" + version: 1.11.0 + resolution: "axios@npm:1.11.0" dependencies: follow-redirects: "npm:^1.15.6" - form-data: "npm:^4.0.0" + form-data: "npm:^4.0.4" proxy-from-env: "npm:^1.1.0" - checksum: 10c0/2239cb269cc789eac22f5d1aabd58e1a83f8f364c92c2caa97b6f5cbb4ab2903d2e557d9dc670b5813e9bcdebfb149e783fb8ab3e45098635cd2f559b06bd5d8 + checksum: 10c0/5de273d33d43058610e4d252f0963cc4f10714da0bfe872e8ef2cbc23c2c999acc300fd357b6bce0fc84a2ca9bd45740fa6bb28199ce2c1266c8b1a393f2b36e languageName: node linkType: hard @@ -7601,14 +7585,16 @@ __metadata: languageName: node linkType: hard -"form-data@npm:^4.0.0": - version: 4.0.1 - resolution: "form-data@npm:4.0.1" +"form-data@npm:^4.0.4": + version: 4.0.4 + resolution: "form-data@npm:4.0.4" dependencies: asynckit: "npm:^0.4.0" combined-stream: "npm:^1.0.8" + es-set-tostringtag: "npm:^2.1.0" + hasown: "npm:^2.0.2" mime-types: "npm:^2.1.12" - checksum: 10c0/bb102d570be8592c23f4ea72d7df9daa50c7792eb0cf1c5d7e506c1706e7426a4e4ae48a35b109e91c85f1c0ec63774a21ae252b66f4eb981cb8efef7d0463c8 + checksum: 10c0/373525a9a034b9d57073e55eab79e501a714ffac02e7a9b01be1c820780652b16e4101819785e1e18f8d98f0aee866cc654d660a435c378e16a72f2e7cac9695 languageName: node linkType: hard