From d624da9c1aac731bd49a7bbb949744ebf4986479 Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Fri, 1 Aug 2025 11:49:12 +0900 Subject: feat: remote notes cleaning (#16292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Create CleanRemoteNotesProcessorService.ts * Update CleanRemoteNotesProcessorService.ts * Update CleanRemoteNotesProcessorService.ts * wip * Update CleanRemoteNotesProcessorService.ts * Update CleanRemoteNotesProcessorService.ts * Update CleanRemoteNotesProcessorService.ts * Update CleanRemoteNotesProcessorService.ts * Update CleanRemoteNotesProcessorService.ts * Update CleanRemoteNotesProcessorService.ts * Update CleanRemoteNotesProcessorService.ts * Update CleanRemoteNotesProcessorService.ts * Update job-queue.job.vue * wip * Update CleanRemoteNotesProcessorService.ts * wip * wip * wip * Update CleanRemoteNotesProcessorService.ts * wip * Update CHANGELOG.md * Revert "wip" This reverts commit 89d455d302c1106c421bcec309fd7bf02509465e. * wip * woip * Update QueueService.ts * Update QueueService.ts * ピン留め考慮 * Update CleanRemoteNotesProcessorService.ts * Update QueueService.ts * Update CleanRemoteNotesProcessorService.ts * add log * Update CHANGELOG.md * wip * Update MkServerSetupWizard.vue --- packages/backend/src/queue/QueueProcessorModule.ts | 4 +- .../backend/src/queue/QueueProcessorService.ts | 3 + .../processors/CleanRemoteNotesProcessorService.ts | 174 +++++++++++++++++++++ 3 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts (limited to 'packages/backend/src/queue') diff --git a/packages/backend/src/queue/QueueProcessorModule.ts b/packages/backend/src/queue/QueueProcessorModule.ts index 9044285bf6..e01414cd53 100644 --- a/packages/backend/src/queue/QueueProcessorModule.ts +++ b/packages/backend/src/queue/QueueProcessorModule.ts @@ -6,7 +6,6 @@ import { Module } from '@nestjs/common'; import { CoreModule } from '@/core/CoreModule.js'; import { GlobalModule } from '@/GlobalModule.js'; -import { CheckModeratorsActivityProcessorService } from '@/queue/processors/CheckModeratorsActivityProcessorService.js'; import { QueueLoggerService } from './QueueLoggerService.js'; import { QueueProcessorService } from './QueueProcessorService.js'; import { DeliverProcessorService } from './processors/DeliverProcessorService.js'; @@ -18,6 +17,8 @@ import { CheckExpiredMutingsProcessorService } from './processors/CheckExpiredMu import { BakeBufferedReactionsProcessorService } from './processors/BakeBufferedReactionsProcessorService.js'; import { CleanChartsProcessorService } from './processors/CleanChartsProcessorService.js'; import { CleanProcessorService } from './processors/CleanProcessorService.js'; +import { CheckModeratorsActivityProcessorService } from './processors/CheckModeratorsActivityProcessorService.js'; +import { CleanRemoteNotesProcessorService } from './processors/CleanRemoteNotesProcessorService.js'; import { CleanRemoteFilesProcessorService } from './processors/CleanRemoteFilesProcessorService.js'; import { DeleteAccountProcessorService } from './processors/DeleteAccountProcessorService.js'; import { DeleteDriveFilesProcessorService } from './processors/DeleteDriveFilesProcessorService.js'; @@ -83,6 +84,7 @@ import { RelationshipProcessorService } from './processors/RelationshipProcessor AggregateRetentionProcessorService, CheckExpiredMutingsProcessorService, CheckModeratorsActivityProcessorService, + CleanRemoteNotesProcessorService, QueueProcessorService, ], exports: [ diff --git a/packages/backend/src/queue/QueueProcessorService.ts b/packages/backend/src/queue/QueueProcessorService.ts index c98ebcdcd9..7b64182754 100644 --- a/packages/backend/src/queue/QueueProcessorService.ts +++ b/packages/backend/src/queue/QueueProcessorService.ts @@ -43,6 +43,7 @@ import { CheckExpiredMutingsProcessorService } from './processors/CheckExpiredMu import { BakeBufferedReactionsProcessorService } from './processors/BakeBufferedReactionsProcessorService.js'; import { CleanProcessorService } from './processors/CleanProcessorService.js'; import { AggregateRetentionProcessorService } from './processors/AggregateRetentionProcessorService.js'; +import { CleanRemoteNotesProcessorService } from './processors/CleanRemoteNotesProcessorService.js'; import { QueueLoggerService } from './QueueLoggerService.js'; import { QUEUE, baseWorkerOptions } from './const.js'; @@ -123,6 +124,7 @@ export class QueueProcessorService implements OnApplicationShutdown { private bakeBufferedReactionsProcessorService: BakeBufferedReactionsProcessorService, private checkModeratorsActivityProcessorService: CheckModeratorsActivityProcessorService, private cleanProcessorService: CleanProcessorService, + private cleanRemoteNotesProcessorService: CleanRemoteNotesProcessorService, ) { this.logger = this.queueLoggerService.logger; @@ -164,6 +166,7 @@ export class QueueProcessorService implements OnApplicationShutdown { case 'bakeBufferedReactions': return this.bakeBufferedReactionsProcessorService.process(); case 'checkModeratorsActivity': return this.checkModeratorsActivityProcessorService.process(); case 'clean': return this.cleanProcessorService.process(); + case 'cleanRemoteNotes': return this.cleanRemoteNotesProcessorService.process(job); default: throw new Error(`unrecognized job type ${job.name} for system`); } }; diff --git a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts new file mode 100644 index 0000000000..5b682e20b8 --- /dev/null +++ b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts @@ -0,0 +1,174 @@ +/* + * SPDX-FileCopyrightText: syuilo and misskey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { setTimeout } from 'node:timers/promises'; +import { Inject, Injectable } from '@nestjs/common'; +import { And, In, IsNull, LessThan, MoreThan, Not } from 'typeorm'; +import { DI } from '@/di-symbols.js'; +import type { MiMeta, MiNote, NoteFavoritesRepository, NotesRepository, UserNotePiningsRepository } from '@/models/_.js'; +import type Logger from '@/logger.js'; +import { bindThis } from '@/decorators.js'; +import { IdService } from '@/core/IdService.js'; +import { QueueLoggerService } from '../QueueLoggerService.js'; +import type * as Bull from 'bullmq'; + +@Injectable() +export class CleanRemoteNotesProcessorService { + private logger: Logger; + + constructor( + @Inject(DI.meta) + private meta: MiMeta, + + @Inject(DI.notesRepository) + private notesRepository: NotesRepository, + + @Inject(DI.noteFavoritesRepository) + private noteFavoritesRepository: NoteFavoritesRepository, + + @Inject(DI.userNotePiningsRepository) + private userNotePiningsRepository: UserNotePiningsRepository, + + private idService: IdService, + private queueLoggerService: QueueLoggerService, + ) { + this.logger = this.queueLoggerService.logger.createSubLogger('clean-remote-notes'); + } + + @bindThis + public async process(job: Bull.Job>): Promise<{ + deletedCount: number; + oldest: number | null; + newest: number | null; + skipped?: boolean; + }> { + if (!this.meta.enableRemoteNotesCleaning) { + this.logger.info('Remote notes cleaning is disabled, skipping...'); + return { + deletedCount: 0, + oldest: null, + newest: null, + skipped: true, + }; + } + + this.logger.info('cleaning remote notes...'); + + const maxDuration = this.meta.remoteNotesCleaningMaxProcessingDurationInMinutes * 60 * 1000; // Convert minutes to milliseconds + const startAt = Date.now(); + + const MAX_NOTE_COUNT_PER_QUERY = 50; + + const stats = { + deletedCount: 0, + oldest: null as number | null, + newest: null as number | null, + }; + + let cursor: MiNote['id'] = this.idService.gen(Date.now() - (1000 * 60 * 60 * 24 * this.meta.remoteNotesCleaningExpiryDaysForEachNotes)); + + while (true) { + const batchBeginAt = Date.now(); + + let notes: Pick[] = await this.notesRepository.find({ + where: { + id: LessThan(cursor), + userHost: Not(IsNull()), + clippedCount: 0, + renoteCount: 0, + }, + take: MAX_NOTE_COUNT_PER_QUERY, + order: { + // 新しい順 + // https://github.com/misskey-dev/misskey/pull/16292#issuecomment-3139376314 + id: -1, + }, + select: ['id'], + }); + + const fetchedCount = notes.length; + + for (const note of notes) { + if (note.id < cursor) { + cursor = note.id; + } + } + + const pinings = notes.length === 0 ? [] : await this.userNotePiningsRepository.find({ + where: { + noteId: In(notes.map(note => note.id)), + }, + select: ['noteId'], + }); + + notes = notes.filter(note => { + return !pinings.some(pining => pining.noteId === note.id); + }); + + const favorites = notes.length === 0 ? [] : await this.noteFavoritesRepository.find({ + where: { + noteId: In(notes.map(note => note.id)), + }, + select: ['noteId'], + }); + + notes = notes.filter(note => { + return !favorites.some(favorite => favorite.noteId === note.id); + }); + + const replies = notes.length === 0 ? [] : await this.notesRepository.find({ + where: { + replyId: In(notes.map(note => note.id)), + userHost: IsNull(), + }, + select: ['replyId'], + }); + + notes = notes.filter(note => { + return !replies.some(reply => reply.replyId === note.id); + }); + + if (notes.length > 0) { + await this.notesRepository.delete(notes.map(note => note.id)); + + for (const note of notes) { + const t = this.idService.parse(note.id).date.getTime(); + if (stats.oldest === null || t < stats.oldest) { + stats.oldest = t; + } + if (stats.newest === null || t > stats.newest) { + stats.newest = t; + } + } + + stats.deletedCount += notes.length; + } + + job.log(`Deleted ${notes.length} of ${fetchedCount}; ${Date.now() - batchBeginAt}ms`); + + const elapsed = Date.now() - startAt; + + if (elapsed >= maxDuration) { + this.logger.info(`Reached maximum duration of ${maxDuration}ms, stopping...`); + job.log('Reached maximum duration, stopping cleaning.'); + job.updateProgress(100); + break; + } + + job.updateProgress((elapsed / maxDuration) * 100); + + await setTimeout(1000 * 5); // Wait a moment to avoid overwhelming the db + } + + this.logger.succ('cleaning of remote notes completed.'); + + return { + deletedCount: stats.deletedCount, + oldest: stats.oldest, + newest: stats.newest, + skipped: false, + }; + } +} -- cgit v1.2.3-freya From 2f13f923a83c836fe08257f239e4fa34dba9c5e3 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Mon, 4 Aug 2025 18:39:08 +0900 Subject: chore: リモートノートの削除条件をデータベース上で確認するように (#16351) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../processors/CleanRemoteNotesProcessorService.ts | 116 +++++++++++---------- 1 file changed, 63 insertions(+), 53 deletions(-) (limited to 'packages/backend/src/queue') diff --git a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts index 5b682e20b8..6c64d6aa39 100644 --- a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts +++ b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts @@ -5,7 +5,7 @@ import { setTimeout } from 'node:timers/promises'; import { Inject, Injectable } from '@nestjs/common'; -import { And, In, IsNull, LessThan, MoreThan, Not } from 'typeorm'; +import { And, Brackets, In, IsNull, LessThan, MoreThan, Not } from 'typeorm'; import { DI } from '@/di-symbols.js'; import type { MiMeta, MiNote, NoteFavoritesRepository, NotesRepository, UserNotePiningsRepository } from '@/models/_.js'; import type Logger from '@/logger.js'; @@ -67,69 +67,79 @@ export class CleanRemoteNotesProcessorService { newest: null as number | null, }; - let cursor: MiNote['id'] = this.idService.gen(Date.now() - (1000 * 60 * 60 * 24 * this.meta.remoteNotesCleaningExpiryDaysForEachNotes)); + // The date limit for the newest note to be considered for deletion. + // All notes newer than this limit will always be retained. + const newestLimit = this.idService.gen(Date.now() - (1000 * 60 * 60 * 24 * this.meta.remoteNotesCleaningExpiryDaysForEachNotes)); + + let cursor = '0'; // oldest note ID to start from while (true) { const batchBeginAt = Date.now(); - let notes: Pick[] = await this.notesRepository.find({ - where: { - id: LessThan(cursor), - userHost: Not(IsNull()), - clippedCount: 0, - renoteCount: 0, - }, - take: MAX_NOTE_COUNT_PER_QUERY, - order: { - // 新しい順 - // https://github.com/misskey-dev/misskey/pull/16292#issuecomment-3139376314 - id: -1, - }, - select: ['id'], - }); + // We use string literals instead of query builder for several reasons: + // - for removeCondition, we need to use it in having clause, which is not supported by Brackets. + // - for recursive part, we need to preserve the order of columns, but typeorm query builder does not guarantee the order of columns in the result query + + // The condition for removing the notes. + // The note must be: + // - old enough (older than the newestLimit) + // - a remote note (userHost is not null). + // - not have clipped + // - not have pinned on the user profile + // - not has been favorite by any user + const removeCondition = 'note.id < :newestLimit' + + ' AND note."clippedCount" = 0' + + ' AND note."userHost" IS NOT NULL' + // using both userId and noteId instead of just noteId to use index on user_note_pining table. + // This is safe because notes are only pinned by the user who created them. + + ' AND NOT EXISTS(SELECT 1 FROM "user_note_pining" WHERE "noteId" = note."id" AND "userId" = note."userId")' + // We cannot use userId trick because users can favorite notes from other users. + + ' AND NOT EXISTS(SELECT 1 FROM "note_favorite" WHERE "noteId" = note."id")' + ; + + // The initiator query contains the oldest ${MAX_NOTE_COUNT_PER_QUERY} remote non-clipped notes + const initiatorQuery = ` + SELECT "note"."id" AS "id", "note"."replyId" AS "replyId", "note"."renoteId" AS "renoteId", "note"."id" AS "initiatorId" + FROM "note" "note" WHERE ${removeCondition} AND "note"."id" > :cursor ORDER BY "note"."id" ASC LIMIT ${MAX_NOTE_COUNT_PER_QUERY}`; + + // The union query queries the related notes and replies related to the initiator query + const unionQuery = ` + SELECT "note"."id", "note"."replyId", "note"."renoteId", rn."initiatorId" + FROM "note" "note" + INNER JOIN "related_notes" "rn" + ON "note"."replyId" = rn.id + OR "note"."renoteId" = rn.id + OR "note"."id" = rn."replyId" + OR "note"."id" = rn."renoteId" + `; + const recursiveQuery = `(${initiatorQuery}) UNION (${unionQuery})`; + + const removableInitiatorNotesQuery = this.notesRepository.createQueryBuilder('note') + .select('rn."initiatorId"') + .innerJoin('related_notes', 'rn', 'note.id = rn.id') + .groupBy('rn."initiatorId"') + .having(`bool_and(${removeCondition})`); + + const notesQuery = this.notesRepository.createQueryBuilder('note') + .addCommonTableExpression(recursiveQuery, 'related_notes', { recursive: true }) + .select('note.id', 'id') + .addSelect('rn."initiatorId"') + .innerJoin('related_notes', 'rn', 'note.id = rn.id') + .where(`rn."initiatorId" IN (${ removableInitiatorNotesQuery.getQuery() })`) + .setParameters({ cursor, newestLimit }); + + const notes: { id: MiNote['id'], initiatorId: MiNote['id'] }[] = await notesQuery.getRawMany(); const fetchedCount = notes.length; + // update the cursor to the newest initiatorId found in the fetched notes. + // We don't use 'id' since the note can be newer than the initiator note. for (const note of notes) { - if (note.id < cursor) { - cursor = note.id; + if (cursor < note.initiatorId) { + cursor = note.initiatorId; } } - const pinings = notes.length === 0 ? [] : await this.userNotePiningsRepository.find({ - where: { - noteId: In(notes.map(note => note.id)), - }, - select: ['noteId'], - }); - - notes = notes.filter(note => { - return !pinings.some(pining => pining.noteId === note.id); - }); - - const favorites = notes.length === 0 ? [] : await this.noteFavoritesRepository.find({ - where: { - noteId: In(notes.map(note => note.id)), - }, - select: ['noteId'], - }); - - notes = notes.filter(note => { - return !favorites.some(favorite => favorite.noteId === note.id); - }); - - const replies = notes.length === 0 ? [] : await this.notesRepository.find({ - where: { - replyId: In(notes.map(note => note.id)), - userHost: IsNull(), - }, - select: ['replyId'], - }); - - notes = notes.filter(note => { - return !replies.some(reply => reply.replyId === note.id); - }); - if (notes.length > 0) { await this.notesRepository.delete(notes.map(note => note.id)); -- cgit v1.2.3-freya From 85e3e496880316c4b55199ea651c210a25420a6e Mon Sep 17 00:00:00 2001 From: tamaina Date: Fri, 8 Aug 2025 21:31:31 +0900 Subject: fix(backend): Fix and create unit test of CleanRemoteNotesProcessorService (#16368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * wip * test(backend): CleanRemoteNotesProcessorService (basic) * test(backend): CleanRemoteNotesProcessorService (advanced) * :v: * a * split initiator query * no order by * ??? * old → older --- .../processors/CleanRemoteNotesProcessorService.ts | 169 +++--- .../processors/CleanRemoteNotesProcessorService.ts | 631 +++++++++++++++++++++ 2 files changed, 725 insertions(+), 75 deletions(-) create mode 100644 packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts (limited to 'packages/backend/src/queue') diff --git a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts index 6c64d6aa39..da3bb804c2 100644 --- a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts +++ b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts @@ -5,9 +5,8 @@ import { setTimeout } from 'node:timers/promises'; import { Inject, Injectable } from '@nestjs/common'; -import { And, Brackets, In, IsNull, LessThan, MoreThan, Not } from 'typeorm'; import { DI } from '@/di-symbols.js'; -import type { MiMeta, MiNote, NoteFavoritesRepository, NotesRepository, UserNotePiningsRepository } from '@/models/_.js'; +import type { MiMeta, MiNote, NotesRepository } from '@/models/_.js'; import type Logger from '@/logger.js'; import { bindThis } from '@/decorators.js'; import { IdService } from '@/core/IdService.js'; @@ -25,12 +24,6 @@ export class CleanRemoteNotesProcessorService { @Inject(DI.notesRepository) private notesRepository: NotesRepository, - @Inject(DI.noteFavoritesRepository) - private noteFavoritesRepository: NoteFavoritesRepository, - - @Inject(DI.userNotePiningsRepository) - private userNotePiningsRepository: UserNotePiningsRepository, - private idService: IdService, private queueLoggerService: QueueLoggerService, ) { @@ -61,6 +54,69 @@ export class CleanRemoteNotesProcessorService { const MAX_NOTE_COUNT_PER_QUERY = 50; + //#retion queries + // We use string literals instead of query builder for several reasons: + // - for removeCondition, we need to use it in having clause, which is not supported by Brackets. + // - for recursive part, we need to preserve the order of columns, but typeorm query builder does not guarantee the order of columns in the result query + + // The condition for removing the notes. + // The note must be: + // - old enough (older than the newestLimit) + // - a remote note (userHost is not null). + // - not have clipped + // - not have pinned on the user profile + // - not has been favorite by any user + const removeCondition = 'note.id < :newestLimit' + + ' AND note."clippedCount" = 0' + + ' AND note."userHost" IS NOT NULL' + // using both userId and noteId instead of just noteId to use index on user_note_pining table. + // This is safe because notes are only pinned by the user who created them. + + ' AND NOT EXISTS(SELECT 1 FROM "user_note_pining" WHERE "noteId" = note."id" AND "userId" = note."userId")' + // We cannot use userId trick because users can favorite notes from other users. + + ' AND NOT EXISTS(SELECT 1 FROM "note_favorite" WHERE "noteId" = note."id")' + ; + + // The initiator query contains the oldest ${MAX_NOTE_COUNT_PER_QUERY} remote non-clipped notes + const initiatorQuery = this.notesRepository.createQueryBuilder('note') + .select('note.id', 'id') + .where(removeCondition) + .andWhere('note.id > :cursor') + .orderBy('note.id', 'ASC') + .limit(MAX_NOTE_COUNT_PER_QUERY); + + // The union query queries the related notes and replies related to the initiator query + const unionQuery = ` + SELECT "note"."id", "note"."replyId", "note"."renoteId", rn."initiatorId" + FROM "note" "note" + INNER JOIN "related_notes" "rn" + ON "note"."replyId" = rn.id + OR "note"."renoteId" = rn.id + OR "note"."id" = rn."replyId" + OR "note"."id" = rn."renoteId" + `; + + const selectRelatedNotesFromInitiatorIdsQuery = ` + SELECT "note"."id" AS "id", "note"."replyId" AS "replyId", "note"."renoteId" AS "renoteId", "note"."id" AS "initiatorId" + FROM "note" "note" WHERE "note"."id" IN (:...initiatorIds) + `; + + const recursiveQuery = `(${selectRelatedNotesFromInitiatorIdsQuery}) UNION (${unionQuery})`; + + const removableInitiatorNotesQuery = this.notesRepository.createQueryBuilder('note') + .select('rn."initiatorId"') + .innerJoin('related_notes', 'rn', 'note.id = rn.id') + .groupBy('rn."initiatorId"') + .having(`bool_and(${removeCondition})`); + + const notesQuery = this.notesRepository.createQueryBuilder('note') + .addCommonTableExpression(recursiveQuery, 'related_notes', { recursive: true }) + .select('note.id', 'id') + .addSelect('rn."initiatorId"') + .innerJoin('related_notes', 'rn', 'note.id = rn.id') + .where(`rn."initiatorId" IN (${removableInitiatorNotesQuery.getQuery()})`) + .distinctOn(['note.id']); + //#endregion + const stats = { deletedCount: 0, oldest: null as number | null, @@ -74,77 +130,45 @@ export class CleanRemoteNotesProcessorService { let cursor = '0'; // oldest note ID to start from while (true) { + //#region check time const batchBeginAt = Date.now(); - // We use string literals instead of query builder for several reasons: - // - for removeCondition, we need to use it in having clause, which is not supported by Brackets. - // - for recursive part, we need to preserve the order of columns, but typeorm query builder does not guarantee the order of columns in the result query - - // The condition for removing the notes. - // The note must be: - // - old enough (older than the newestLimit) - // - a remote note (userHost is not null). - // - not have clipped - // - not have pinned on the user profile - // - not has been favorite by any user - const removeCondition = 'note.id < :newestLimit' - + ' AND note."clippedCount" = 0' - + ' AND note."userHost" IS NOT NULL' - // using both userId and noteId instead of just noteId to use index on user_note_pining table. - // This is safe because notes are only pinned by the user who created them. - + ' AND NOT EXISTS(SELECT 1 FROM "user_note_pining" WHERE "noteId" = note."id" AND "userId" = note."userId")' - // We cannot use userId trick because users can favorite notes from other users. - + ' AND NOT EXISTS(SELECT 1 FROM "note_favorite" WHERE "noteId" = note."id")' - ; + const elapsed = batchBeginAt - startAt; - // The initiator query contains the oldest ${MAX_NOTE_COUNT_PER_QUERY} remote non-clipped notes - const initiatorQuery = ` - SELECT "note"."id" AS "id", "note"."replyId" AS "replyId", "note"."renoteId" AS "renoteId", "note"."id" AS "initiatorId" - FROM "note" "note" WHERE ${removeCondition} AND "note"."id" > :cursor ORDER BY "note"."id" ASC LIMIT ${MAX_NOTE_COUNT_PER_QUERY}`; + if (elapsed >= maxDuration) { + this.logger.info(`Reached maximum duration of ${maxDuration}ms, stopping...`); + job.log('Reached maximum duration, stopping cleaning.'); + job.updateProgress(100); + break; + } - // The union query queries the related notes and replies related to the initiator query - const unionQuery = ` - SELECT "note"."id", "note"."replyId", "note"."renoteId", rn."initiatorId" - FROM "note" "note" - INNER JOIN "related_notes" "rn" - ON "note"."replyId" = rn.id - OR "note"."renoteId" = rn.id - OR "note"."id" = rn."replyId" - OR "note"."id" = rn."renoteId" - `; - const recursiveQuery = `(${initiatorQuery}) UNION (${unionQuery})`; + job.updateProgress((elapsed / maxDuration) * 100); + //#endregion - const removableInitiatorNotesQuery = this.notesRepository.createQueryBuilder('note') - .select('rn."initiatorId"') - .innerJoin('related_notes', 'rn', 'note.id = rn.id') - .groupBy('rn."initiatorId"') - .having(`bool_and(${removeCondition})`); + // First, we fetch the initiator notes that are older than the newestLimit. + const initiatorNotes: { id: MiNote['id'] }[] = await initiatorQuery.setParameters({ cursor, newestLimit }).getRawMany(); - const notesQuery = this.notesRepository.createQueryBuilder('note') - .addCommonTableExpression(recursiveQuery, 'related_notes', { recursive: true }) - .select('note.id', 'id') - .addSelect('rn."initiatorId"') - .innerJoin('related_notes', 'rn', 'note.id = rn.id') - .where(`rn."initiatorId" IN (${ removableInitiatorNotesQuery.getQuery() })`) - .setParameters({ cursor, newestLimit }); + // update the cursor to the newest initiatorId found in the fetched notes. + const newCursor = initiatorNotes.reduce((max, note) => note.id > max ? note.id : max, cursor); - const notes: { id: MiNote['id'], initiatorId: MiNote['id'] }[] = await notesQuery.getRawMany(); + if (initiatorNotes.length === 0 || cursor === newCursor || newCursor >= newestLimit) { + // If no notes were found or the cursor did not change, we can stop. + job.log('No more notes to clean. (no initiator notes found or cursor did not change.)'); + break; + } - const fetchedCount = notes.length; + const notes: { id: MiNote['id'], initiatorId: MiNote['id'] }[] = await notesQuery.setParameters({ + initiatorIds: initiatorNotes.map(note => note.id), + newestLimit, + }).getRawMany(); - // update the cursor to the newest initiatorId found in the fetched notes. - // We don't use 'id' since the note can be newer than the initiator note. - for (const note of notes) { - if (cursor < note.initiatorId) { - cursor = note.initiatorId; - } - } + cursor = newCursor; if (notes.length > 0) { await this.notesRepository.delete(notes.map(note => note.id)); - for (const note of notes) { - const t = this.idService.parse(note.id).date.getTime(); + for (const { id } of notes) { + const t = this.idService.parse(id).date.getTime(); if (stats.oldest === null || t < stats.oldest) { stats.oldest = t; } @@ -156,19 +180,14 @@ export class CleanRemoteNotesProcessorService { stats.deletedCount += notes.length; } - job.log(`Deleted ${notes.length} of ${fetchedCount}; ${Date.now() - batchBeginAt}ms`); - - const elapsed = Date.now() - startAt; + job.log(`Deleted ${notes.length} from ${initiatorNotes.length} initiators; ${Date.now() - batchBeginAt}ms`); - if (elapsed >= maxDuration) { - this.logger.info(`Reached maximum duration of ${maxDuration}ms, stopping...`); - job.log('Reached maximum duration, stopping cleaning.'); - job.updateProgress(100); + if (initiatorNotes.length < MAX_NOTE_COUNT_PER_QUERY) { + // If we fetched less than the maximum, it means there are no more notes to process. + job.log(`No more notes to clean. (fewer than MAX_NOTE_COUNT_PER_QUERY =${MAX_NOTE_COUNT_PER_QUERY}.)`); break; } - job.updateProgress((elapsed / maxDuration) * 100); - await setTimeout(1000 * 5); // Wait a moment to avoid overwhelming the db } diff --git a/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts b/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts new file mode 100644 index 0000000000..15f8eda865 --- /dev/null +++ b/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts @@ -0,0 +1,631 @@ +/* + * SPDX-FileCopyrightText: syuilo and misskey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { jest } from '@jest/globals'; +import { Test, TestingModule } from '@nestjs/testing'; +import ms from 'ms'; +import { + type MiNote, + type MiUser, + type NotesRepository, + type NoteFavoritesRepository, + type UserNotePiningsRepository, + type UsersRepository, + type UserProfilesRepository, + MiMeta, +} from '@/models/_.js'; +import { CleanRemoteNotesProcessorService } from '@/queue/processors/CleanRemoteNotesProcessorService.js'; +import { DI } from '@/di-symbols.js'; +import { IdService } from '@/core/IdService.js'; +import { QueueLoggerService } from '@/queue/QueueLoggerService.js'; +import { GlobalModule } from '@/GlobalModule.js'; +import { secureRndstr } from '@/misc/secure-rndstr.js'; + +describe('CleanRemoteNotesProcessorService', () => { + let app: TestingModule; + let service: CleanRemoteNotesProcessorService; + let idService: IdService; + let notesRepository: NotesRepository; + let noteFavoritesRepository: NoteFavoritesRepository; + let userNotePiningsRepository: UserNotePiningsRepository; + let usersRepository: UsersRepository; + let userProfilesRepository: UserProfilesRepository; + + // Local user + let alice: MiUser; + // Remote user 1 + let bob: MiUser; + // Remote user 2 + let carol: MiUser; + + const meta = new MiMeta(); + + // Mock job object + const createMockJob = () => ({ + log: jest.fn(), + updateProgress: jest.fn(), + }); + + async function createUser(data: Partial = {}) { + const id = idService.gen(); + const un = data.username || secureRndstr(16); + const user = await usersRepository + .insert({ + id, + username: un, + usernameLower: un.toLowerCase(), + ...data, + }) + .then(x => usersRepository.findOneByOrFail(x.identifiers[0])); + + await userProfilesRepository.save({ + userId: id, + }); + + return user; + } + + async function createNote(data: Partial, user: MiUser, time?: number): Promise { + const id = idService.gen(time); + const note = await notesRepository + .insert({ + id: id, + text: `note_${id}`, + userId: user.id, + userHost: user.host, + visibility: 'public', + ...data, + }) + .then(x => notesRepository.findOneByOrFail(x.identifiers[0])); + return note; + } + + beforeAll(async () => { + app = await Test + .createTestingModule({ + imports: [ + GlobalModule, + ], + providers: [ + CleanRemoteNotesProcessorService, + IdService, + { + provide: QueueLoggerService, + useFactory: () => ({ + logger: { + createSubLogger: () => ({ + info: jest.fn(), + warn: jest.fn(), + succ: jest.fn(), + }), + }, + }), + }, + ], + }) + .overrideProvider(DI.meta).useFactory({ factory: () => meta }) + .compile(); + + service = app.get(CleanRemoteNotesProcessorService); + idService = app.get(IdService); + notesRepository = app.get(DI.notesRepository); + noteFavoritesRepository = app.get(DI.noteFavoritesRepository); + userNotePiningsRepository = app.get(DI.userNotePiningsRepository); + usersRepository = app.get(DI.usersRepository); + userProfilesRepository = app.get(DI.userProfilesRepository); + + alice = await createUser({ username: 'alice', host: null }); + bob = await createUser({ username: 'bob', host: 'remote1.example.com' }); + carol = await createUser({ username: 'carol', host: 'remote2.example.com' }); + + app.enableShutdownHooks(); + }); + + beforeEach(() => { + // Reset mocks + jest.clearAllMocks(); + + // Set default meta values + meta.enableRemoteNotesCleaning = true; + meta.remoteNotesCleaningMaxProcessingDurationInMinutes = 0.3; + meta.remoteNotesCleaningExpiryDaysForEachNotes = 30; + }, 60 * 1000); + + afterEach(async () => { + // Clean up test data + await Promise.all([ + notesRepository.createQueryBuilder().delete().execute(), + userNotePiningsRepository.createQueryBuilder().delete().execute(), + noteFavoritesRepository.createQueryBuilder().delete().execute(), + ]); + }, 60 * 1000); + + afterAll(async () => { + await app.close(); + }); + + describe('basic', () => { + test('should skip cleaning when enableRemoteNotesCleaning is false', async () => { + meta.enableRemoteNotesCleaning = false; + const job = createMockJob(); + + const result = await service.process(job as any); + + expect(result).toEqual({ + deletedCount: 0, + oldest: null, + newest: null, + skipped: true, + }); + }); + + test('should return success result when enableRemoteNotesCleaning is true and no notes to clean', async () => { + const job = createMockJob(); + + await createNote({}, alice); + const result = await service.process(job as any); + + expect(result).toEqual({ + deletedCount: 0, + oldest: null, + newest: null, + skipped: false, + }); + }, 3000); + + test('should clean remote notes and return stats', async () => { + // Remote notes + const remoteNotes = await Promise.all([ + createNote({}, bob), + createNote({}, carol), + createNote({}, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000), + createNote({}, carol, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 2000), // Note older than expiry + ]); + + // Local notes + const localNotes = await Promise.all([ + createNote({}, alice), + createNote({}, alice, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000), + ]); + + const job = createMockJob(); + + const result = await service.process(job as any); + + expect(result).toEqual({ + deletedCount: 2, + oldest: expect.any(Number), + newest: expect.any(Number), + skipped: false, + }); + + // Check side-by-side from all notes + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.length).toBe(4); + expect(remainingNotes.some(n => n.id === remoteNotes[0].id)).toBe(true); + expect(remainingNotes.some(n => n.id === remoteNotes[1].id)).toBe(true); + expect(remainingNotes.some(n => n.id === remoteNotes[2].id)).toBe(false); + expect(remainingNotes.some(n => n.id === remoteNotes[3].id)).toBe(false); + expect(remainingNotes.some(n => n.id === localNotes[0].id)).toBe(true); + expect(remainingNotes.some(n => n.id === localNotes[1].id)).toBe(true); + }); + }); + + describe('advanced', () => { + // お気に入り + test('should not delete note that is favorited by any user', async () => { + const job = createMockJob(); + + // Create old remote note that should be deleted + const olderRemoteNote = await createNote({}, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + + // Favorite the note + await noteFavoritesRepository.save({ + id: idService.gen(), + userId: alice.id, + noteId: olderRemoteNote.id, + }); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); + expect(result.skipped).toBe(false); + + const remainingNote = await notesRepository.findOneBy({ id: olderRemoteNote.id }); + expect(remainingNote).not.toBeNull(); + }); + + // ピン留め + test('should not delete note that is pinned by the user', async () => { + const job = createMockJob(); + + // Create old remote note that should be deleted + const olderRemoteNote = await createNote({}, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + + // Pin the note by the user who created it + await userNotePiningsRepository.save({ + id: idService.gen(), + userId: bob.id, // Same user as the note creator + noteId: olderRemoteNote.id, + }); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); + expect(result.skipped).toBe(false); + + const remainingNote = await notesRepository.findOneBy({ id: olderRemoteNote.id }); + expect(remainingNote).not.toBeNull(); + }); + + // クリップ + test('should not delete note that is clipped', async () => { + const job = createMockJob(); + + // Create old remote note that is clipped + const clippedNote = await createNote({ + clippedCount: 1, // Clipped + }, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); + expect(result.skipped).toBe(false); + + const remainingNote = await notesRepository.findOneBy({ id: clippedNote.id }); + expect(remainingNote).not.toBeNull(); + }); + + // 古いreply, renoteが含まれている時の挙動 + test('should handle reply/renote relationships correctly', async () => { + const job = createMockJob(); + + // Create old remote notes with reply/renote relationships + const originalNote = await createNote({}, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + const replyNote = await createNote({ + replyId: originalNote.id, + }, carol, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 2000); + const renoteNote = await createNote({ + renoteId: originalNote.id, + }, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 3000); + + const result = await service.process(job as any); + + // Should delete all three notes as they are all old and remote + expect(result.deletedCount).toBe(3); + expect(result.skipped).toBe(false); + + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.some(n => n.id === originalNote.id)).toBe(false); + expect(remainingNotes.some(n => n.id === replyNote.id)).toBe(false); + expect(remainingNotes.some(n => n.id === renoteNote.id)).toBe(false); + }); + + // 古いリモートノートに新しいリプライがある時、どちらも削除されない + test('should not delete both old remote note with new reply', async () => { + const job = createMockJob(); + + // Create old remote note that should be deleted + const oldNote = await createNote({}, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + + // Create a reply note that is newer than the expiry period + const recentReplyNote = await createNote({ + replyId: oldNote.id, + }, carol, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) + 1000); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); // Only the old note should be deleted + expect(result.skipped).toBe(false); + + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.some(n => n.id === oldNote.id)).toBe(true); + expect(remainingNotes.some(n => n.id === recentReplyNote.id)).toBe(true); // Recent reply note should remain + }); + + // 古いリモートノートに新しいリプライと古いリプライがある時、全て残る + test('should not delete old remote note with new reply and old reply', async () => { + const job = createMockJob(); + + // Create old remote note that should be deleted + const oldNote = await createNote({}, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + + // Create a reply note that is newer than the expiry period + const recentReplyNote = await createNote({ + replyId: oldNote.id, + }, carol, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) + 1000); + + // Create an old reply note that should be deleted + const oldReplyNote = await createNote({ + replyId: oldNote.id, + }, carol, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 2000); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); + expect(result.skipped).toBe(false); + + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.some(n => n.id === oldNote.id)).toBe(true); + expect(remainingNotes.some(n => n.id === recentReplyNote.id)).toBe(true); // Recent reply note should remain + expect(remainingNotes.some(n => n.id === oldReplyNote.id)).toBe(true); // Old reply note should be deleted + }); + + // リプライがお気に入りされているとき、どちらも削除されない + test('should not delete reply note that is favorited', async () => { + const job = createMockJob(); + + // Create old remote note that should be deleted + const olderRemoteNote = await createNote({}, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + + // Create a reply note that is newer than the expiry period + const replyNote = await createNote({ + replyId: olderRemoteNote.id, + }, carol, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 2000); + + // Favorite the reply note + await noteFavoritesRepository.save({ + id: idService.gen(), + userId: alice.id, + noteId: replyNote.id, + }); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); // Only the old note should be deleted + expect(result.skipped).toBe(false); + + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.some(n => n.id === olderRemoteNote.id)).toBe(true); + expect(remainingNotes.some(n => n.id === replyNote.id)).toBe(true); // Recent reply note should remain + }); + + // リプライがピン留めされているとき、どちらも削除されない + test('should not delete reply note that is pinned', async () => { + const job = createMockJob(); + + // Create old remote note that should be deleted + const olderRemoteNote = await createNote({}, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + + // Create a reply note that is newer than the expiry period + const replyNote = await createNote({ + replyId: olderRemoteNote.id, + }, carol, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 2000); + + // Pin the reply note + await userNotePiningsRepository.save({ + id: idService.gen(), + userId: carol.id, + noteId: replyNote.id, + }); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); // Only the old note should be deleted + expect(result.skipped).toBe(false); + + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.some(n => n.id === olderRemoteNote.id)).toBe(true); + expect(remainingNotes.some(n => n.id === replyNote.id)).toBe(true); // Reply note should remain + }); + + // リプライがクリップされているとき、どちらも削除されない + test('should not delete reply note that is clipped', async () => { + const job = createMockJob(); + + // Create old remote note that should be deleted + const olderRemoteNote = await createNote({}, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + + // Create a reply note that is old but clipped + const replyNote = await createNote({ + replyId: olderRemoteNote.id, + clippedCount: 1, // Clipped + }, carol, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 2000); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); // Both notes should be kept because reply is clipped + expect(result.skipped).toBe(false); + + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.some(n => n.id === olderRemoteNote.id)).toBe(true); + expect(remainingNotes.some(n => n.id === replyNote.id)).toBe(true); + }); + + test('should handle mixed scenarios with multiple conditions', async () => { + const job = createMockJob(); + + // Create various types of notes + const oldTime = Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000; + + // Should be deleted: old remote note with no special conditions + const deletableNote = await createNote({}, bob, oldTime); + + // Should NOT be deleted: old remote note but favorited + const favoritedNote = await createNote({}, carol, oldTime); + await noteFavoritesRepository.save({ + id: idService.gen(), + userId: alice.id, + noteId: favoritedNote.id, + }); + + // Should NOT be deleted: old remote note but pinned + const pinnedNote = await createNote({}, bob, oldTime); + await userNotePiningsRepository.save({ + id: idService.gen(), + userId: bob.id, + noteId: pinnedNote.id, + }); + + // Should NOT be deleted: old remote note but clipped + const clippedNote = await createNote({ + clippedCount: 2, + }, carol, oldTime); + + // Should NOT be deleted: old local note + const localNote = await createNote({}, alice, oldTime); + + // Should NOT be deleted: new remote note + const newerRemoteNote = await createNote({}, bob); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(1); // Only deletableNote should be deleted + expect(result.skipped).toBe(false); + + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.length).toBe(5); + expect(remainingNotes.some(n => n.id === deletableNote.id)).toBe(false); // Deleted + expect(remainingNotes.some(n => n.id === favoritedNote.id)).toBe(true); // Kept + expect(remainingNotes.some(n => n.id === pinnedNote.id)).toBe(true); // Kept + expect(remainingNotes.some(n => n.id === clippedNote.id)).toBe(true); // Kept + expect(remainingNotes.some(n => n.id === localNote.id)).toBe(true); // Kept + expect(remainingNotes.some(n => n.id === newerRemoteNote.id)).toBe(true); // Kept + }); + + // 大量のノート + test('should handle large number of notes correctly', async () => { + const AMOUNT = 130; + const job = createMockJob(); + + const oldTime = Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000; + const noteIds = []; + for (let i = 0; i < AMOUNT; i++) { + const note = await createNote({}, bob, oldTime - i); + noteIds.push(note.id); + } + + const result = await service.process(job as any); + + // Should delete all notes, but may require multiple batches + expect(result.deletedCount).toBe(AMOUNT); + expect(result.skipped).toBe(false); + + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.length).toBe(0); + }); + + // 大量のノート + リプライ or リノート + test('should handle large number of notes with replies correctly', async () => { + const AMOUNT = 130; + const job = createMockJob(); + + const oldTime = Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000; + const noteIds = []; + for (let i = 0; i < AMOUNT; i++) { + const note = await createNote({}, bob, oldTime - i - AMOUNT); + noteIds.push(note.id); + if (i % 2 === 0) { + // Create a reply for every second note + await createNote({ replyId: note.id }, carol, oldTime - i); + } else { + // Create a renote for every second note + await createNote({ renoteId: note.id }, bob, oldTime - i); + } + } + + const result = await service.process(job as any); + // Should delete all notes, but may require multiple batches + expect(result.deletedCount).toBe(AMOUNT * 2); + expect(result.skipped).toBe(false); + }); + + // 大量の古いノート + 新しいリプライ or リノート + test('should handle large number of old notes with new replies correctly', async () => { + const AMOUNT = 130; + const job = createMockJob(); + + const oldTime = Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000; + const newTime = Date.now(); + const noteIds = []; + for (let i = 0; i < AMOUNT; i++) { + const note = await createNote({}, bob, oldTime - i); + noteIds.push(note.id); + if (i % 2 === 0) { + // Create a reply for every second note + await createNote({ replyId: note.id }, carol, newTime + i); + } else { + // Create a renote for every second note + await createNote({ renoteId: note.id }, bob, newTime + i); + } + } + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); + expect(result.skipped).toBe(false); + }); + + // 大量の残す対象(clippedCount: 1)と大量の削除対象 + test('should handle large number of notes, mixed conditions with clippedCount', async () => { + const AMOUNT_BASE = 70; + const job = createMockJob(); + + const oldTime = Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000; + const noteIds = []; + for (let i = 0; i < AMOUNT_BASE; i++) { + const note = await createNote({ clippedCount: 1 }, bob, oldTime - i - AMOUNT_BASE); + noteIds.push(note.id); + } + for (let i = 0; i < AMOUNT_BASE; i++) { + const note = await createNote({}, carol, oldTime - i); + noteIds.push(note.id); + } + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(AMOUNT_BASE); // Assuming half are deletable + expect(result.skipped).toBe(false); + }); + + // 大量の残す対象(リプライ)と大量の削除対象 + test('should handle large number of notes, mixed conditions with replies', async () => { + const AMOUNT_BASE = 70; + const job = createMockJob(); + const oldTime = Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000; + const newTime = Date.now(); + for (let i = 0; i < AMOUNT_BASE; i++) { + // should remain + const note = await createNote({}, carol, oldTime - AMOUNT_BASE - i); + // should remain + await createNote({ replyId: note.id }, bob, newTime + i); + } + + const noteIdsExpectedToBeDeleted = []; + for (let i = 0; i < AMOUNT_BASE; i++) { + // should be deleted + const note = await createNote({}, bob, oldTime - i); + noteIdsExpectedToBeDeleted.push(note.id); + } + + const result = await service.process(job as any); + expect(result.deletedCount).toBe(AMOUNT_BASE); // Assuming all replies are deletable + expect(result.skipped).toBe(false); + + const remainingNotes = await notesRepository.find(); + expect(remainingNotes.length).toBe(AMOUNT_BASE * 2); // Only replies should remain + noteIdsExpectedToBeDeleted.forEach(id => { + expect(remainingNotes.some(n => n.id === id)).toBe(false); // All original notes should be deleted + }); + }); + + test('should update cursor correctly during batch processing', async () => { + const job = createMockJob(); + + // Create notes with specific timing to test cursor behavior + const baseTime = Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 10000; + + const note1 = await createNote({}, bob, baseTime); + const note2 = await createNote({}, carol, baseTime - 1000); + const note3 = await createNote({}, bob, baseTime - 2000); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(3); + expect(result.newest).toBe(idService.parse(note1.id).date.getTime()); + expect(result.oldest).toBe(idService.parse(note3.id).date.getTime()); + expect(result.skipped).toBe(false); + }); + }); +}); -- cgit v1.2.3-freya From 90b96093411250df81bded348074ea16d32d32c1 Mon Sep 17 00:00:00 2001 From: "饺子w (Yumechi)" <35571479+eternal-flame-AD@users.noreply.github.com> Date: Thu, 14 Aug 2025 07:54:28 +0000 Subject: enhance: performance for CleanRemoteNotesProcessorService (#16404) * enhance: performance for CleanRemoteNotesProcessorService Signed-off-by: eternal-flame-AD * suggestions Signed-off-by: eternal-flame-AD * docs Signed-off-by: eternal-flame-AD * change initial limit to 100 Signed-off-by: eternal-flame-AD * robustness for transient race conditions Signed-off-by: eternal-flame-AD * handle cursors in postgres Signed-off-by: eternal-flame-AD * robustness: transient errors and timeout handling Signed-off-by: eternal-flame-AD * use '0' as initial cursor Signed-off-by: eternal-flame-AD --------- Signed-off-by: eternal-flame-AD --- .../processors/CleanRemoteNotesProcessorService.ts | 282 +++++++++++++-------- .../processors/CleanRemoteNotesProcessorService.ts | 3 + 2 files changed, 186 insertions(+), 99 deletions(-) (limited to 'packages/backend/src/queue') diff --git a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts index da3bb804c2..77a9dc5557 100644 --- a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts +++ b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts @@ -5,6 +5,7 @@ import { setTimeout } from 'node:timers/promises'; import { Inject, Injectable } from '@nestjs/common'; +import { DataSource, IsNull, LessThan, QueryFailedError, Not } from 'typeorm'; import { DI } from '@/di-symbols.js'; import type { MiMeta, MiNote, NotesRepository } from '@/models/_.js'; import type Logger from '@/logger.js'; @@ -24,18 +25,31 @@ export class CleanRemoteNotesProcessorService { @Inject(DI.notesRepository) private notesRepository: NotesRepository, + @Inject(DI.db) + private db: DataSource, + private idService: IdService, private queueLoggerService: QueueLoggerService, ) { this.logger = this.queueLoggerService.logger.createSubLogger('clean-remote-notes'); } + @bindThis + private computeProgress(minId: string, maxId: string, cursorLeft: string) { + const minTs = this.idService.parse(minId).date.getTime(); + const maxTs = this.idService.parse(maxId).date.getTime(); + const cursorTs = this.idService.parse(cursorLeft).date.getTime(); + + return ((cursorTs - minTs) / (maxTs - minTs)) * 100; + } + @bindThis public async process(job: Bull.Job>): Promise<{ deletedCount: number; oldest: number | null; newest: number | null; - skipped?: boolean; + skipped: boolean; + transientErrors: number; }> { if (!this.meta.enableRemoteNotesCleaning) { this.logger.info('Remote notes cleaning is disabled, skipping...'); @@ -44,6 +58,7 @@ export class CleanRemoteNotesProcessorService { oldest: null, newest: null, skipped: true, + transientErrors: 0, }; } @@ -52,12 +67,10 @@ export class CleanRemoteNotesProcessorService { const maxDuration = this.meta.remoteNotesCleaningMaxProcessingDurationInMinutes * 60 * 1000; // Convert minutes to milliseconds const startAt = Date.now(); - const MAX_NOTE_COUNT_PER_QUERY = 50; - - //#retion queries - // We use string literals instead of query builder for several reasons: - // - for removeCondition, we need to use it in having clause, which is not supported by Brackets. - // - for recursive part, we need to preserve the order of columns, but typeorm query builder does not guarantee the order of columns in the result query + //#region queries + // The date limit for the newest note to be considered for deletion. + // All notes newer than this limit will always be retained. + const newestLimit = this.idService.gen(Date.now() - (1000 * 60 * 60 * 24 * this.meta.remoteNotesCleaningExpiryDaysForEachNotes)); // The condition for removing the notes. // The note must be: @@ -66,56 +79,93 @@ export class CleanRemoteNotesProcessorService { // - not have clipped // - not have pinned on the user profile // - not has been favorite by any user - const removeCondition = 'note.id < :newestLimit' - + ' AND note."clippedCount" = 0' - + ' AND note."userHost" IS NOT NULL' - // using both userId and noteId instead of just noteId to use index on user_note_pining table. - // This is safe because notes are only pinned by the user who created them. - + ' AND NOT EXISTS(SELECT 1 FROM "user_note_pining" WHERE "noteId" = note."id" AND "userId" = note."userId")' - // We cannot use userId trick because users can favorite notes from other users. - + ' AND NOT EXISTS(SELECT 1 FROM "note_favorite" WHERE "noteId" = note."id")' - ; - - // The initiator query contains the oldest ${MAX_NOTE_COUNT_PER_QUERY} remote non-clipped notes - const initiatorQuery = this.notesRepository.createQueryBuilder('note') - .select('note.id', 'id') - .where(removeCondition) - .andWhere('note.id > :cursor') - .orderBy('note.id', 'ASC') - .limit(MAX_NOTE_COUNT_PER_QUERY); - - // The union query queries the related notes and replies related to the initiator query - const unionQuery = ` - SELECT "note"."id", "note"."replyId", "note"."renoteId", rn."initiatorId" - FROM "note" "note" - INNER JOIN "related_notes" "rn" - ON "note"."replyId" = rn.id - OR "note"."renoteId" = rn.id - OR "note"."id" = rn."replyId" - OR "note"."id" = rn."renoteId" - `; - - const selectRelatedNotesFromInitiatorIdsQuery = ` - SELECT "note"."id" AS "id", "note"."replyId" AS "replyId", "note"."renoteId" AS "renoteId", "note"."id" AS "initiatorId" - FROM "note" "note" WHERE "note"."id" IN (:...initiatorIds) - `; - - const recursiveQuery = `(${selectRelatedNotesFromInitiatorIdsQuery}) UNION (${unionQuery})`; - - const removableInitiatorNotesQuery = this.notesRepository.createQueryBuilder('note') - .select('rn."initiatorId"') - .innerJoin('related_notes', 'rn', 'note.id = rn.id') - .groupBy('rn."initiatorId"') - .having(`bool_and(${removeCondition})`); - - const notesQuery = this.notesRepository.createQueryBuilder('note') - .addCommonTableExpression(recursiveQuery, 'related_notes', { recursive: true }) + const removalCriteria = [ + 'note."id" < :newestLimit', + 'note."clippedCount" = 0', + 'note."userHost" IS NOT NULL', + 'NOT EXISTS (SELECT 1 FROM user_note_pining WHERE "noteId" = note."id")', + 'NOT EXISTS (SELECT 1 FROM note_favorite WHERE "noteId" = note."id")', + ].join(' AND '); + + const minId = (await this.notesRepository.createQueryBuilder('note') + .select('MIN(note.id)', 'minId') + .where({ + id: LessThan(newestLimit), + userHost: Not(IsNull()), + replyId: IsNull(), + renoteId: IsNull(), + }) + .getRawOne<{ minId?: MiNote['id'] }>())?.minId; + + if (!minId) { + this.logger.info('No notes can possibly be deleted, skipping...'); + return { + deletedCount: 0, + oldest: null, + newest: null, + skipped: false, + transientErrors: 0, + }; + } + + // start with a conservative limit and adjust it based on the query duration + const minimumLimit = 10; + let currentLimit = 100; + let cursorLeft = '0'; + + const candidateNotesCteName = 'candidate_notes'; + + // tree walk down all root notes, short-circuit when the first unremovable note is found + const candidateNotesQueryBase = this.notesRepository.createQueryBuilder('note') + .select('note."id"', 'id') + .addSelect('note."replyId"', 'replyId') + .addSelect('note."renoteId"', 'renoteId') + .addSelect('note."id"', 'rootId') + .addSelect('TRUE', 'isRemovable') + .addSelect('TRUE', 'isBase') + .where('note."id" > :cursorLeft') + .andWhere(removalCriteria) + .andWhere({ replyId: IsNull(), renoteId: IsNull() }); + + const candidateNotesQueryInductive = this.notesRepository.createQueryBuilder('note') .select('note.id', 'id') - .addSelect('rn."initiatorId"') - .innerJoin('related_notes', 'rn', 'note.id = rn.id') - .where(`rn."initiatorId" IN (${removableInitiatorNotesQuery.getQuery()})`) - .distinctOn(['note.id']); - //#endregion + .addSelect('note."replyId"', 'replyId') + .addSelect('note."renoteId"', 'renoteId') + .addSelect('parent."rootId"', 'rootId') + .addSelect(removalCriteria, 'isRemovable') + .addSelect('FALSE', 'isBase') + .innerJoin(candidateNotesCteName, 'parent', 'parent."id" = note."replyId" OR parent."id" = note."renoteId"') + .where('parent."isRemovable" = TRUE'); + + // A note tree can be deleted if there are no unremovable rows with the same rootId. + // + // `candidate_notes` will have the following structure after recursive query (some columns omitted): + // After performing a LEFT JOIN with `candidate_notes` as `unremovable`, + // the note tree containing unremovable notes will be anti-joined. + // For removable rows, the `unremovable` columns will have `NULL` values. + // | id | rootId | isRemovable | + // |-----|--------|-------------| + // | aaa | aaa | TRUE | + // | bbb | aaa | FALSE | + // | ccc | aaa | FALSE | + // | ddd | ddd | TRUE | + // | eee | ddd | TRUE | + // | fff | fff | TRUE | + // | ggg | ggg | FALSE | + // + const candidateNotesQuery = this.db.createQueryBuilder() + .select(`"${candidateNotesCteName}"."id"`, 'id') + .addSelect('unremovable."id" IS NULL', 'isRemovable') + .addSelect(`BOOL_OR("${candidateNotesCteName}"."isBase")`, 'isBase') + .addCommonTableExpression( + `((SELECT "base".* FROM (${candidateNotesQueryBase.orderBy('note.id', 'ASC').limit(currentLimit).getQuery()}) AS "base") UNION ${candidateNotesQueryInductive.getQuery()})`, + candidateNotesCteName, + { recursive: true }, + ) + .from(candidateNotesCteName, candidateNotesCteName) + .leftJoin(candidateNotesCteName, 'unremovable', `unremovable."rootId" = "${candidateNotesCteName}"."rootId" AND unremovable."isRemovable" = FALSE`) + .groupBy(`"${candidateNotesCteName}"."id"`) + .addGroupBy('unremovable."id" IS NULL'); const stats = { deletedCount: 0, @@ -123,74 +173,107 @@ export class CleanRemoteNotesProcessorService { newest: null as number | null, }; - // The date limit for the newest note to be considered for deletion. - // All notes newer than this limit will always be retained. - const newestLimit = this.idService.gen(Date.now() - (1000 * 60 * 60 * 24 * this.meta.remoteNotesCleaningExpiryDaysForEachNotes)); - - let cursor = '0'; // oldest note ID to start from - - while (true) { + let lowThroughputWarned = false; + let transientErrors = 0; + for (;;) { //#region check time const batchBeginAt = Date.now(); const elapsed = batchBeginAt - startAt; + const progress = this.computeProgress(minId, newestLimit, cursorLeft > minId ? cursorLeft : minId); + if (elapsed >= maxDuration) { - this.logger.info(`Reached maximum duration of ${maxDuration}ms, stopping...`); - job.log('Reached maximum duration, stopping cleaning.'); + job.log(`Reached maximum duration of ${maxDuration}ms, stopping... (last cursor: ${cursorLeft}, final progress ${progress}%)`); job.updateProgress(100); break; } - job.updateProgress((elapsed / maxDuration) * 100); + const wallClockUsage = elapsed / maxDuration; + if (wallClockUsage > 0.5 && progress < 50 && !lowThroughputWarned) { + const msg = `Not projected to finish in time! (wall clock usage ${wallClockUsage * 100}% at ${progress}%, current limit ${currentLimit})`; + this.logger.warn(msg); + job.log(msg); + lowThroughputWarned = true; + } + job.updateProgress(progress); //#endregion - // First, we fetch the initiator notes that are older than the newestLimit. - const initiatorNotes: { id: MiNote['id'] }[] = await initiatorQuery.setParameters({ cursor, newestLimit }).getRawMany(); - - // update the cursor to the newest initiatorId found in the fetched notes. - const newCursor = initiatorNotes.reduce((max, note) => note.id > max ? note.id : max, cursor); + const queryBegin = performance.now(); + let noteIds = null; + + try { + noteIds = await candidateNotesQuery.setParameters( + { newestLimit, cursorLeft }, + ).getRawMany<{ id: MiNote['id'], isRemovable: boolean, isBase: boolean }>(); + } catch (e) { + if (currentLimit > minimumLimit && e instanceof QueryFailedError && e.driverError?.code === '57014') { + // Statement timeout (maybe suddenly hit a large note tree), reduce the limit and try again + // continuous failures will eventually converge to currentLimit == minimumLimit and then throw + currentLimit = Math.max(minimumLimit, Math.floor(currentLimit * 0.25)); + continue; + } + throw e; + } - if (initiatorNotes.length === 0 || cursor === newCursor || newCursor >= newestLimit) { - // If no notes were found or the cursor did not change, we can stop. - job.log('No more notes to clean. (no initiator notes found or cursor did not change.)'); + if (noteIds.length === 0) { + job.log('No more notes to clean.'); break; } - const notes: { id: MiNote['id'], initiatorId: MiNote['id'] }[] = await notesQuery.setParameters({ - initiatorIds: initiatorNotes.map(note => note.id), - newestLimit, - }).getRawMany(); - - cursor = newCursor; - - if (notes.length > 0) { - await this.notesRepository.delete(notes.map(note => note.id)); - - for (const { id } of notes) { - const t = this.idService.parse(id).date.getTime(); - if (stats.oldest === null || t < stats.oldest) { - stats.oldest = t; + const queryDuration = performance.now() - queryBegin; + // try to adjust such that each query takes about 1~5 seconds and reasonable NodeJS heap so the task stays responsive + // this should not oscillate.. + if (queryDuration > 5000 || noteIds.length > 5000) { + currentLimit = Math.floor(currentLimit * 0.5); + } else if (queryDuration < 1000 && noteIds.length < 1000) { + currentLimit = Math.floor(currentLimit * 1.5); + } + // clamp to a sane range + currentLimit = Math.min(Math.max(currentLimit, minimumLimit), 5000); + + const deletableNoteIds = noteIds.filter(result => result.isRemovable).map(result => result.id); + if (deletableNoteIds.length > 0) { + try { + await this.notesRepository.delete(deletableNoteIds); + + for (const id of deletableNoteIds) { + const t = this.idService.parse(id).date.getTime(); + if (stats.oldest === null || t < stats.oldest) { + stats.oldest = t; + } + if (stats.newest === null || t > stats.newest) { + stats.newest = t; + } } - if (stats.newest === null || t > stats.newest) { - stats.newest = t; + + stats.deletedCount += deletableNoteIds.length; + } catch (e) { + // check for integrity violation errors (class 23) that might have occurred between the check and the delete + // we can safely continue to the next batch + if (e instanceof QueryFailedError && e.driverError?.code?.startsWith('23')) { + transientErrors++; + job.log(`Error deleting notes: ${e} (transient race condition?)`); + } else { + throw e; } } - - stats.deletedCount += notes.length; } - job.log(`Deleted ${notes.length} from ${initiatorNotes.length} initiators; ${Date.now() - batchBeginAt}ms`); + cursorLeft = noteIds.filter(result => result.isBase).reduce((max, { id }) => id > max ? id : max, cursorLeft); - if (initiatorNotes.length < MAX_NOTE_COUNT_PER_QUERY) { - // If we fetched less than the maximum, it means there are no more notes to process. - job.log(`No more notes to clean. (fewer than MAX_NOTE_COUNT_PER_QUERY =${MAX_NOTE_COUNT_PER_QUERY}.)`); - break; + job.log(`Deleted ${noteIds.length} notes; ${Date.now() - batchBeginAt}ms`); + + if (process.env.NODE_ENV !== 'test') { + await setTimeout(Math.min(1000 * 5, queryDuration)); // Wait a moment to avoid overwhelming the db } + }; - await setTimeout(1000 * 5); // Wait a moment to avoid overwhelming the db + if (transientErrors > 0) { + const msg = `${transientErrors} transient errors occurred while cleaning remote notes. You may need a second pass to complete the cleaning.`; + this.logger.warn(msg); + job.log(msg); } - this.logger.succ('cleaning of remote notes completed.'); return { @@ -198,6 +281,7 @@ export class CleanRemoteNotesProcessorService { oldest: stats.oldest, newest: stats.newest, skipped: false, + transientErrors, }; } } diff --git a/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts b/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts index 15f8eda865..597d6b90cd 100644 --- a/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts +++ b/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts @@ -158,6 +158,7 @@ describe('CleanRemoteNotesProcessorService', () => { oldest: null, newest: null, skipped: true, + transientErrors: 0, }); }); @@ -172,6 +173,7 @@ describe('CleanRemoteNotesProcessorService', () => { oldest: null, newest: null, skipped: false, + transientErrors: 0, }); }, 3000); @@ -199,6 +201,7 @@ describe('CleanRemoteNotesProcessorService', () => { oldest: expect.any(Number), newest: expect.any(Number), skipped: false, + transientErrors: 0, }); // Check side-by-side from all notes -- cgit v1.2.3-freya From 60f7278aff27b9a0e03c1f1a2a77663cfb0e0ddb Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Fri, 15 Aug 2025 22:39:55 +0900 Subject: fix: Remote Note Cleaning will delete notes embedded in a page (#16408) * feat: preserve number of pages referencing the note * chore: delete pages on account delete * fix: notes on the pages are removed by CleanRemoteNotes * test: add the simplest test for page embedded notes * fix: section block is not considered * fix: section block is not considered in migration * chore: remove comments from columns * revert unnecessary change * add pageCount to webhook test * fix type error on backend --- .../migration/1755168347001-PageCountInNote.js | 58 ++++++ packages/backend/src/core/CoreModule.ts | 6 + packages/backend/src/core/PageService.ts | 223 +++++++++++++++++++++ packages/backend/src/core/WebhookTestService.ts | 1 + packages/backend/src/models/Note.ts | 7 + .../processors/CleanRemoteNotesProcessorService.ts | 1 + .../processors/DeleteAccountProcessorService.ts | 29 ++- .../src/server/api/endpoints/pages/create.ts | 41 ++-- .../src/server/api/endpoints/pages/delete.ts | 41 ++-- .../src/server/api/endpoints/pages/update.ts | 69 +++---- packages/backend/test/unit/NoteCreateService.ts | 1 + packages/backend/test/unit/misc/is-renote.ts | 1 + .../processors/CleanRemoteNotesProcessorService.ts | 18 ++ 13 files changed, 400 insertions(+), 96 deletions(-) create mode 100644 packages/backend/migration/1755168347001-PageCountInNote.js create mode 100644 packages/backend/src/core/PageService.ts (limited to 'packages/backend/src/queue') diff --git a/packages/backend/migration/1755168347001-PageCountInNote.js b/packages/backend/migration/1755168347001-PageCountInNote.js new file mode 100644 index 0000000000..9f1894ab2f --- /dev/null +++ b/packages/backend/migration/1755168347001-PageCountInNote.js @@ -0,0 +1,58 @@ +/* + * SPDX-FileCopyrightText: syuilo and misskey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +export class PageCountInNote1755168347001 { + name = 'PageCountInNote1755168347001' + + async up(queryRunner) { + await queryRunner.query(`ALTER TABLE "note" ADD "pageCount" smallint NOT NULL DEFAULT '0'`); + + // Update existing notes + // block_list CTE collects all page blocks on the pages including child blocks in the section blocks. + // The clipped_notes CTE counts how many distinct pages each note block is referenced in. + // Finally, we update the note table with the count of pages for each referenced note. + await queryRunner.query(` + WITH RECURSIVE block_list AS ( + ( + SELECT + page.id as page_id, + block as block + FROM page + CROSS JOIN LATERAL jsonb_array_elements(page.content) block + WHERE block->>'type' = 'note' OR block->>'type' = 'section' + ) + UNION ALL + ( + SELECT + block_list.page_id, + child_block AS block + FROM LATERAL ( + SELECT page_id, block + FROM block_list + WHERE block_list.block->>'type' = 'section' + ) block_list + CROSS JOIN LATERAL jsonb_array_elements(block_list.block->'children') child_block + WHERE child_block->>'type' = 'note' OR child_block->>'type' = 'section' + ) + ), + clipped_notes AS ( + SELECT + (block->>'note') AS note_id, + COUNT(distinct block_list.page_id) AS count + FROM block_list + WHERE block_list.block->>'type' = 'note' + GROUP BY block->>'note' + ) + UPDATE note + SET "pageCount" = clipped_notes.count + FROM clipped_notes + WHERE note.id = clipped_notes.note_id; + `); + } + + async down(queryRunner) { + await queryRunner.query(`ALTER TABLE "note" DROP COLUMN "pageCount"`); + } +} diff --git a/packages/backend/src/core/CoreModule.ts b/packages/backend/src/core/CoreModule.ts index 0c0c5d3a39..a30bff0fe4 100644 --- a/packages/backend/src/core/CoreModule.ts +++ b/packages/backend/src/core/CoreModule.ts @@ -78,6 +78,7 @@ import { ChannelFollowingService } from './ChannelFollowingService.js'; import { ChatService } from './ChatService.js'; import { RegistryApiService } from './RegistryApiService.js'; import { ReversiService } from './ReversiService.js'; +import { PageService } from './PageService.js'; import { ChartLoggerService } from './chart/ChartLoggerService.js'; import FederationChart from './chart/charts/federation.js'; @@ -227,6 +228,7 @@ const $ChannelFollowingService: Provider = { provide: 'ChannelFollowingService', const $ChatService: Provider = { provide: 'ChatService', useExisting: ChatService }; const $RegistryApiService: Provider = { provide: 'RegistryApiService', useExisting: RegistryApiService }; const $ReversiService: Provider = { provide: 'ReversiService', useExisting: ReversiService }; +const $PageService: Provider = { provide: 'PageService', useExisting: PageService }; const $ChartLoggerService: Provider = { provide: 'ChartLoggerService', useExisting: ChartLoggerService }; const $FederationChart: Provider = { provide: 'FederationChart', useExisting: FederationChart }; @@ -379,6 +381,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting ChatService, RegistryApiService, ReversiService, + PageService, ChartLoggerService, FederationChart, @@ -527,6 +530,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting $ChatService, $RegistryApiService, $ReversiService, + $PageService, $ChartLoggerService, $FederationChart, @@ -676,6 +680,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting ChatService, RegistryApiService, ReversiService, + PageService, FederationChart, NotesChart, @@ -822,6 +827,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting $ChatService, $RegistryApiService, $ReversiService, + $PageService, $FederationChart, $NotesChart, diff --git a/packages/backend/src/core/PageService.ts b/packages/backend/src/core/PageService.ts new file mode 100644 index 0000000000..7f0e5c7ccc --- /dev/null +++ b/packages/backend/src/core/PageService.ts @@ -0,0 +1,223 @@ +/* + * SPDX-FileCopyrightText: syuilo and misskey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { Inject, Injectable } from '@nestjs/common'; +import { DataSource, In, Not } from 'typeorm'; +import { DI } from '@/di-symbols.js'; +import { + type NotesRepository, + MiPage, + type PagesRepository, + MiDriveFile, + type UsersRepository, + MiNote, +} from '@/models/_.js'; +import { bindThis } from '@/decorators.js'; +import { RoleService } from '@/core/RoleService.js'; +import { IdService } from '@/core/IdService.js'; +import type { MiUser } from '@/models/User.js'; +import { IdentifiableError } from '@/misc/identifiable-error.js'; +import { ModerationLogService } from '@/core/ModerationLogService.js'; + +export interface PageBody { + title: string; + name: string; + summary: string | null; + content: Array>; + variables: Array>; + script: string; + eyeCatchingImage?: MiDriveFile | null; + font: string; + alignCenter: boolean; + hideTitleWhenPinned: boolean; +} + +@Injectable() +export class PageService { + constructor( + @Inject(DI.db) + private db: DataSource, + + @Inject(DI.pagesRepository) + private pagesRepository: PagesRepository, + + @Inject(DI.notesRepository) + private notesRepository: NotesRepository, + + @Inject(DI.usersRepository) + private usersRepository: UsersRepository, + + private roleService: RoleService, + private moderationLogService: ModerationLogService, + private idService: IdService, + ) { + } + + @bindThis + public async create( + me: MiUser, + body: PageBody, + ): Promise { + await this.pagesRepository.findBy({ + userId: me.id, + name: body.name, + }).then(result => { + if (result.length > 0) { + throw new IdentifiableError('1a79e38e-3d83-4423-845b-a9d83ff93b61'); + } + }); + + const page = await this.pagesRepository.insertOne(new MiPage({ + id: this.idService.gen(), + updatedAt: new Date(), + title: body.title, + name: body.name, + summary: body.summary, + content: body.content, + variables: body.variables, + script: body.script, + eyeCatchingImageId: body.eyeCatchingImage ? body.eyeCatchingImage.id : null, + userId: me.id, + visibility: 'public', + alignCenter: body.alignCenter, + hideTitleWhenPinned: body.hideTitleWhenPinned, + font: body.font, + })); + + const referencedNotes = this.collectReferencedNotes(page.content); + if (referencedNotes.length > 0) { + await this.notesRepository.increment({ id: In(referencedNotes) }, 'pageCount', 1); + } + + return page; + } + + @bindThis + public async update( + me: MiUser, + pageId: MiPage['id'], + body: Partial, + ): Promise { + await this.db.transaction(async (transaction) => { + const page = await transaction.findOne(MiPage, { + where: { + id: pageId, + }, + lock: { mode: 'for_no_key_update' }, + }); + + if (page == null) { + throw new IdentifiableError('66aefd3c-fdb2-4a71-85ae-cc18bea85d3f'); + } + if (page.userId !== me.id) { + throw new IdentifiableError('d0017699-8256-46f1-aed4-bc03bed73616'); + } + + if (body.name != null) { + await transaction.findBy(MiPage, { + id: Not(pageId), + userId: me.id, + name: body.name, + }).then(result => { + if (result.length > 0) { + throw new IdentifiableError('d05bfe24-24b6-4ea2-a3ec-87cc9bf4daa4'); + } + }); + } + + await transaction.update(MiPage, page.id, { + updatedAt: new Date(), + title: body.title, + name: body.name, + summary: body.summary === undefined ? page.summary : body.summary, + content: body.content, + variables: body.variables, + script: body.script, + alignCenter: body.alignCenter, + hideTitleWhenPinned: body.hideTitleWhenPinned, + font: body.font, + eyeCatchingImageId: body.eyeCatchingImage === undefined ? undefined : (body.eyeCatchingImage?.id ?? null), + }); + + console.log("page.content", page.content); + + if (body.content != null) { + const beforeReferencedNotes = this.collectReferencedNotes(page.content); + const afterReferencedNotes = this.collectReferencedNotes(body.content); + + const removedNotes = beforeReferencedNotes.filter(noteId => !afterReferencedNotes.includes(noteId)); + const addedNotes = afterReferencedNotes.filter(noteId => !beforeReferencedNotes.includes(noteId)); + + if (removedNotes.length > 0) { + await transaction.decrement(MiNote, { id: In(removedNotes) }, 'pageCount', 1); + } + if (addedNotes.length > 0) { + await transaction.increment(MiNote, { id: In(addedNotes) }, 'pageCount', 1); + } + } + }); + } + + @bindThis + public async delete(me: MiUser, pageId: MiPage['id']): Promise { + await this.db.transaction(async (transaction) => { + const page = await transaction.findOne(MiPage, { + where: { + id: pageId, + }, + lock: { mode: 'pessimistic_write' }, // same lock level as DELETE + }); + + if (page == null) { + throw new IdentifiableError('66aefd3c-fdb2-4a71-85ae-cc18bea85d3f'); + } + + if (!await this.roleService.isModerator(me) && page.userId !== me.id) { + throw new IdentifiableError('d0017699-8256-46f1-aed4-bc03bed73616'); + } + + await transaction.delete(MiPage, page.id); + + if (page.userId !== me.id) { + const user = await this.usersRepository.findOneByOrFail({ id: page.userId }); + this.moderationLogService.log(me, 'deletePage', { + pageId: page.id, + pageUserId: page.userId, + pageUserUsername: user.username, + page, + }); + } + + const referencedNotes = this.collectReferencedNotes(page.content); + if (referencedNotes.length > 0) { + await transaction.decrement(MiNote, { id: In(referencedNotes) }, 'pageCount', 1); + } + }); + } + + collectReferencedNotes(content: MiPage['content']): string[] { + const referencingNotes = new Set(); + const recursiveCollect = (content: unknown[]) => { + for (const contentElement of content) { + if (typeof contentElement === 'object' + && contentElement !== null + && 'type' in contentElement) { + if (contentElement.type === 'note' + && 'note' in contentElement + && typeof contentElement.note === 'string') { + referencingNotes.add(contentElement.note); + } + if (contentElement.type === 'section' + && 'children' in contentElement + && Array.isArray(contentElement.children)) { + recursiveCollect(contentElement.children); + } + } + } + }; + recursiveCollect(content); + return [...referencingNotes]; + } +} diff --git a/packages/backend/src/core/WebhookTestService.ts b/packages/backend/src/core/WebhookTestService.ts index 9cf985b688..907b5ea6be 100644 --- a/packages/backend/src/core/WebhookTestService.ts +++ b/packages/backend/src/core/WebhookTestService.ts @@ -85,6 +85,7 @@ function generateDummyNote(override?: Partial): MiNote { renoteCount: 10, repliesCount: 5, clippedCount: 0, + pageCount: 0, reactions: {}, visibility: 'public', uri: null, diff --git a/packages/backend/src/models/Note.ts b/packages/backend/src/models/Note.ts index ff46615729..26d5c1d535 100644 --- a/packages/backend/src/models/Note.ts +++ b/packages/backend/src/models/Note.ts @@ -114,6 +114,13 @@ export class MiNote { }) public clippedCount: number; + // The number of note page blocks referencing this note. + // This column is used by Remote Note Cleaning and manually updated rather than automatically with triggers. + @Column('smallint', { + default: 0, + }) + public pageCount: number; + @Column('jsonb', { default: {}, }) diff --git a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts index 77a9dc5557..f53d403280 100644 --- a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts +++ b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts @@ -82,6 +82,7 @@ export class CleanRemoteNotesProcessorService { const removalCriteria = [ 'note."id" < :newestLimit', 'note."clippedCount" = 0', + 'note."pageCount" = 0', 'note."userHost" IS NOT NULL', 'NOT EXISTS (SELECT 1 FROM user_note_pining WHERE "noteId" = note."id")', 'NOT EXISTS (SELECT 1 FROM note_favorite WHERE "noteId" = note."id")', diff --git a/packages/backend/src/queue/processors/DeleteAccountProcessorService.ts b/packages/backend/src/queue/processors/DeleteAccountProcessorService.ts index 14a53e0c42..b643c2a6d0 100644 --- a/packages/backend/src/queue/processors/DeleteAccountProcessorService.ts +++ b/packages/backend/src/queue/processors/DeleteAccountProcessorService.ts @@ -6,7 +6,7 @@ import { Inject, Injectable } from '@nestjs/common'; import { MoreThan } from 'typeorm'; import { DI } from '@/di-symbols.js'; -import type { DriveFilesRepository, NotesRepository, UserProfilesRepository, UsersRepository } from '@/models/_.js'; +import type { DriveFilesRepository, NotesRepository, PagesRepository, UserProfilesRepository, UsersRepository } from '@/models/_.js'; import type Logger from '@/logger.js'; import { DriveService } from '@/core/DriveService.js'; import type { MiDriveFile } from '@/models/DriveFile.js'; @@ -14,6 +14,7 @@ import type { MiNote } from '@/models/Note.js'; import { EmailService } from '@/core/EmailService.js'; import { bindThis } from '@/decorators.js'; import { SearchService } from '@/core/SearchService.js'; +import { PageService } from '@/core/PageService.js'; import { QueueLoggerService } from '../QueueLoggerService.js'; import type * as Bull from 'bullmq'; import type { DbUserDeleteJobData } from '../types.js'; @@ -35,7 +36,11 @@ export class DeleteAccountProcessorService { @Inject(DI.driveFilesRepository) private driveFilesRepository: DriveFilesRepository, + @Inject(DI.pagesRepository) + private pagesRepository: PagesRepository, + private driveService: DriveService, + private pageService: PageService, private emailService: EmailService, private queueLoggerService: QueueLoggerService, private searchService: SearchService, @@ -112,6 +117,28 @@ export class DeleteAccountProcessorService { this.logger.succ('All of files deleted'); } + { + // delete pages. Necessary for decrementing pageCount of notes. + while (true) { + const pages = await this.pagesRepository.find({ + where: { + userId: user.id, + }, + take: 100, + order: { + id: 1, + }, + }); + + if (pages.length === 0) { + break; + } + for (const page of pages) { + await this.pageService.delete(user, page.id); + } + } + } + { // Send email notification const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id }); if (profile.email && profile.emailVerified) { diff --git a/packages/backend/src/server/api/endpoints/pages/create.ts b/packages/backend/src/server/api/endpoints/pages/create.ts index 6de5fe3d44..96bc2a953a 100644 --- a/packages/backend/src/server/api/endpoints/pages/create.ts +++ b/packages/backend/src/server/api/endpoints/pages/create.ts @@ -5,12 +5,13 @@ import ms from 'ms'; import { Inject, Injectable } from '@nestjs/common'; -import type { DriveFilesRepository, PagesRepository } from '@/models/_.js'; -import { IdService } from '@/core/IdService.js'; -import { MiPage, pageNameSchema } from '@/models/Page.js'; +import type { DriveFilesRepository, MiDriveFile, PagesRepository } from '@/models/_.js'; +import { pageNameSchema } from '@/models/Page.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { PageEntityService } from '@/core/entities/PageEntityService.js'; import { DI } from '@/di-symbols.js'; +import { PageService } from '@/core/PageService.js'; +import { IdentifiableError } from '@/misc/identifiable-error.js'; import { ApiError } from '../../error.js'; export const meta = { @@ -77,11 +78,11 @@ export default class extends Endpoint { // eslint- @Inject(DI.driveFilesRepository) private driveFilesRepository: DriveFilesRepository, + private pageService: PageService, private pageEntityService: PageEntityService, - private idService: IdService, ) { super(meta, paramDef, async (ps, me) => { - let eyeCatchingImage = null; + let eyeCatchingImage: MiDriveFile | null = null; if (ps.eyeCatchingImageId != null) { eyeCatchingImage = await this.driveFilesRepository.findOneBy({ id: ps.eyeCatchingImageId, @@ -102,24 +103,20 @@ export default class extends Endpoint { // eslint- } }); - const page = await this.pagesRepository.insertOne(new MiPage({ - id: this.idService.gen(), - updatedAt: new Date(), - title: ps.title, - name: ps.name, - summary: ps.summary, - content: ps.content, - variables: ps.variables, - script: ps.script, - eyeCatchingImageId: eyeCatchingImage ? eyeCatchingImage.id : null, - userId: me.id, - visibility: 'public', - alignCenter: ps.alignCenter, - hideTitleWhenPinned: ps.hideTitleWhenPinned, - font: ps.font, - })); + try { + const page = await this.pageService.create(me, { + ...ps, + eyeCatchingImage, + summary: ps.summary ?? null, + }); - return await this.pageEntityService.pack(page); + return await this.pageEntityService.pack(page); + } catch (err) { + if (err instanceof IdentifiableError && err.id === '1a79e38e-3d83-4423-845b-a9d83ff93b61') { + throw new ApiError(meta.errors.nameAlreadyExists); + } + throw err; + } }); } } diff --git a/packages/backend/src/server/api/endpoints/pages/delete.ts b/packages/backend/src/server/api/endpoints/pages/delete.ts index f2bc946788..a33868552d 100644 --- a/packages/backend/src/server/api/endpoints/pages/delete.ts +++ b/packages/backend/src/server/api/endpoints/pages/delete.ts @@ -4,12 +4,14 @@ */ import { Inject, Injectable } from '@nestjs/common'; -import type { PagesRepository, UsersRepository } from '@/models/_.js'; +import type { MiDriveFile, PagesRepository, UsersRepository } from '@/models/_.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { DI } from '@/di-symbols.js'; import { ModerationLogService } from '@/core/ModerationLogService.js'; import { RoleService } from '@/core/RoleService.js'; import { ApiError } from '../../error.js'; +import { IdentifiableError } from '@/misc/identifiable-error.js'; +import { PageService } from '@/core/PageService.js'; export const meta = { tags: ['pages'], @@ -44,36 +46,17 @@ export const paramDef = { @Injectable() export default class extends Endpoint { // eslint-disable-line import/no-default-export constructor( - @Inject(DI.pagesRepository) - private pagesRepository: PagesRepository, - - @Inject(DI.usersRepository) - private usersRepository: UsersRepository, - - private moderationLogService: ModerationLogService, - private roleService: RoleService, + private pageService: PageService, ) { super(meta, paramDef, async (ps, me) => { - const page = await this.pagesRepository.findOneBy({ id: ps.pageId }); - - if (page == null) { - throw new ApiError(meta.errors.noSuchPage); - } - - if (!await this.roleService.isModerator(me) && page.userId !== me.id) { - throw new ApiError(meta.errors.accessDenied); - } - - await this.pagesRepository.delete(page.id); - - if (page.userId !== me.id) { - const user = await this.usersRepository.findOneByOrFail({ id: page.userId }); - this.moderationLogService.log(me, 'deletePage', { - pageId: page.id, - pageUserId: page.userId, - pageUserUsername: user.username, - page, - }); + try { + await this.pageService.delete(me, ps.pageId); + } catch (err) { + if (err instanceof IdentifiableError) { + if (err.id === '66aefd3c-fdb2-4a71-85ae-cc18bea85d3f') throw new ApiError(meta.errors.noSuchPage); + if (err.id === 'd0017699-8256-46f1-aed4-bc03bed73616') throw new ApiError(meta.errors.accessDenied); + } + throw err; } }); } diff --git a/packages/backend/src/server/api/endpoints/pages/update.ts b/packages/backend/src/server/api/endpoints/pages/update.ts index a6aeb6002e..6fa5c1d75c 100644 --- a/packages/backend/src/server/api/endpoints/pages/update.ts +++ b/packages/backend/src/server/api/endpoints/pages/update.ts @@ -4,13 +4,14 @@ */ import ms from 'ms'; -import { Not } from 'typeorm'; import { Inject, Injectable } from '@nestjs/common'; -import type { PagesRepository, DriveFilesRepository } from '@/models/_.js'; +import type { DriveFilesRepository, MiDriveFile } from '@/models/_.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { DI } from '@/di-symbols.js'; import { ApiError } from '../../error.js'; import { pageNameSchema } from '@/models/Page.js'; +import { IdentifiableError } from '@/misc/identifiable-error.js'; +import { PageService } from '@/core/PageService.js'; export const meta = { tags: ['pages'], @@ -75,57 +76,37 @@ export const paramDef = { @Injectable() export default class extends Endpoint { // eslint-disable-line import/no-default-export constructor( - @Inject(DI.pagesRepository) - private pagesRepository: PagesRepository, - @Inject(DI.driveFilesRepository) private driveFilesRepository: DriveFilesRepository, + + private pageService: PageService, ) { super(meta, paramDef, async (ps, me) => { - const page = await this.pagesRepository.findOneBy({ id: ps.pageId }); - if (page == null) { - throw new ApiError(meta.errors.noSuchPage); - } - if (page.userId !== me.id) { - throw new ApiError(meta.errors.accessDenied); - } - - if (ps.eyeCatchingImageId != null) { - const eyeCatchingImage = await this.driveFilesRepository.findOneBy({ - id: ps.eyeCatchingImageId, - userId: me.id, - }); + try { + let eyeCatchingImage: MiDriveFile | null | undefined | string = ps.eyeCatchingImageId; + if (eyeCatchingImage != null) { + eyeCatchingImage = await this.driveFilesRepository.findOneBy({ + id: eyeCatchingImage, + userId: me.id, + }); - if (eyeCatchingImage == null) { - throw new ApiError(meta.errors.noSuchFile); + if (eyeCatchingImage == null) { + throw new ApiError(meta.errors.noSuchFile); + } } - } - if (ps.name != null) { - await this.pagesRepository.findBy({ - id: Not(ps.pageId), - userId: me.id, - name: ps.name, - }).then(result => { - if (result.length > 0) { - throw new ApiError(meta.errors.nameAlreadyExists); - } + await this.pageService.update(me, ps.pageId, { + ...ps, + eyeCatchingImage, }); + } catch (err) { + if (err instanceof IdentifiableError) { + if (err.id === '66aefd3c-fdb2-4a71-85ae-cc18bea85d3f') throw new ApiError(meta.errors.noSuchPage); + if (err.id === 'd0017699-8256-46f1-aed4-bc03bed73616') throw new ApiError(meta.errors.accessDenied); + if (err.id === 'd05bfe24-24b6-4ea2-a3ec-87cc9bf4daa4') throw new ApiError(meta.errors.nameAlreadyExists); + } + throw err; } - - await this.pagesRepository.update(page.id, { - updatedAt: new Date(), - title: ps.title, - name: ps.name, - summary: ps.summary === undefined ? page.summary : ps.summary, - content: ps.content, - variables: ps.variables, - script: ps.script, - alignCenter: ps.alignCenter, - hideTitleWhenPinned: ps.hideTitleWhenPinned, - font: ps.font, - eyeCatchingImageId: ps.eyeCatchingImageId, - }); }); } } diff --git a/packages/backend/test/unit/NoteCreateService.ts b/packages/backend/test/unit/NoteCreateService.ts index f2d4c8ffbb..23f409420e 100644 --- a/packages/backend/test/unit/NoteCreateService.ts +++ b/packages/backend/test/unit/NoteCreateService.ts @@ -40,6 +40,7 @@ describe('NoteCreateService', () => { renoteCount: 0, repliesCount: 0, clippedCount: 0, + pageCount: 0, reactions: {}, visibility: 'public', uri: null, diff --git a/packages/backend/test/unit/misc/is-renote.ts b/packages/backend/test/unit/misc/is-renote.ts index 0b713e8bf6..74d17abcb6 100644 --- a/packages/backend/test/unit/misc/is-renote.ts +++ b/packages/backend/test/unit/misc/is-renote.ts @@ -23,6 +23,7 @@ const base: MiNote = { renoteCount: 0, repliesCount: 0, clippedCount: 0, + pageCount: 0, reactions: {}, visibility: 'public', uri: null, diff --git a/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts b/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts index 597d6b90cd..631e160afc 100644 --- a/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts +++ b/packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts @@ -281,6 +281,24 @@ describe('CleanRemoteNotesProcessorService', () => { expect(remainingNote).not.toBeNull(); }); + // ページ + test('should not delete note that is embedded in a page', async () => { + const job = createMockJob(); + + // Create old remote note that is embedded in a page + const clippedNote = await createNote({ + pageCount: 1, // Embedded in a page + }, bob, Date.now() - ms(`${meta.remoteNotesCleaningExpiryDaysForEachNotes} days`) - 1000); + + const result = await service.process(job as any); + + expect(result.deletedCount).toBe(0); + expect(result.skipped).toBe(false); + + const remainingNote = await notesRepository.findOneBy({ id: clippedNote.id }); + expect(remainingNote).not.toBeNull(); + }); + // 古いreply, renoteが含まれている時の挙動 test('should handle reply/renote relationships correctly', async () => { const job = createMockJob(); -- cgit v1.2.3-freya From fea9f27fd6e9d04e0746e4dfc6f971aacc001eb9 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Sun, 17 Aug 2025 17:08:38 +0900 Subject: chore: preserve notes with local reactions (#16412) --- .../backend/src/queue/processors/CleanRemoteNotesProcessorService.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/backend/src/queue') diff --git a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts index f53d403280..36c34c753c 100644 --- a/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts +++ b/packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts @@ -86,6 +86,7 @@ export class CleanRemoteNotesProcessorService { 'note."userHost" IS NOT NULL', 'NOT EXISTS (SELECT 1 FROM user_note_pining WHERE "noteId" = note."id")', 'NOT EXISTS (SELECT 1 FROM note_favorite WHERE "noteId" = note."id")', + 'NOT EXISTS (SELECT 1 FROM note_reaction INNER JOIN "user" ON note_reaction."userId" = "user".id WHERE note_reaction."noteId" = note."id" AND "user"."host" IS NULL)', ].join(' AND '); const minId = (await this.notesRepository.createQueryBuilder('note') -- cgit v1.2.3-freya