usbharu
/
misskey
mirror of https://github.com/misskey-dev/misskey.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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
This commit is contained in:
anatawa12 2025-08-15 22:39:55 +09:00 committed by GitHub
parent bae92a944d
commit 60f7278aff
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 402 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
packages/backend/migration/1755168347001-PageCountInNote.js Normal file
View File

@ -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"`);
}
}

6
packages/backend/src/core/CoreModule.ts
View File

@ -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,

223
packages/backend/src/core/PageService.ts Normal file
View File

@ -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<Record<string, any>>;
variables: Array<Record<string, any>>;
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<MiPage> {
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<PageBody>,
): Promise<void> {
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<void> {
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<string>();
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];
}
}

1
packages/backend/src/core/WebhookTestService.ts
View File

@ -85,6 +85,7 @@ function generateDummyNote(override?: Partial<MiNote>): MiNote {
renoteCount: 10,
repliesCount: 5,
clippedCount: 0,
pageCount: 0,
reactions: {},
visibility: 'public',
uri: null,

7
packages/backend/src/models/Note.ts
View File

@ -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: {},
})

1
packages/backend/src/queue/processors/CleanRemoteNotesProcessorService.ts
View File

@ -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")',

29
packages/backend/src/queue/processors/DeleteAccountProcessorService.ts
View File

@ -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) {

39
packages/backend/src/server/api/endpoints/pages/create.ts
View File

@ -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<typeof meta, typeof paramDef> { // 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<typeof meta, typeof paramDef> { // 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);
} catch (err) {
if (err instanceof IdentifiableError && err.id === '1a79e38e-3d83-4423-845b-a9d83ff93b61') {
throw new ApiError(meta.errors.nameAlreadyExists);
}
throw err;
}
});
}
}

39
packages/backend/src/server/api/endpoints/pages/delete.ts
View File

@ -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<typeof meta, typeof paramDef> { // 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);
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);
}
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,
});
throw err;
}
});
}

59
packages/backend/src/server/api/endpoints/pages/update.ts
View File

@ -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,24 +76,17 @@ export const paramDef = {
@Injectable()
export default class extends Endpoint<typeof meta, typeof paramDef> { // 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,
try {
let eyeCatchingImage: MiDriveFile | null | undefined | string = ps.eyeCatchingImageId;
if (eyeCatchingImage != null) {
eyeCatchingImage = await this.driveFilesRepository.findOneBy({
id: eyeCatchingImage,
userId: me.id,
});
@ -101,31 +95,18 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
}
}
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,
});
});
}
}

1
packages/backend/test/unit/NoteCreateService.ts
View File

@ -40,6 +40,7 @@ describe('NoteCreateService', () => {
renoteCount: 0,
repliesCount: 0,
clippedCount: 0,
pageCount: 0,
reactions: {},
visibility: 'public',
uri: null,

1
packages/backend/test/unit/misc/is-renote.ts
View File

@ -23,6 +23,7 @@ const base: MiNote = {
renoteCount: 0,
repliesCount: 0,
clippedCount: 0,
pageCount: 0,
reactions: {},
visibility: 'public',
uri: null,

18
packages/backend/test/unit/queue/processors/CleanRemoteNotesProcessorService.ts
View File

@ -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();