From c302a5c2d7696bc9dddeabe914b92ad2fdc0b0ba Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 17:44:00 -0400 Subject: reorder relay activities to avoid delivery race condition --- packages/backend/src/core/NotePiningService.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'packages/backend/src/core/NotePiningService.ts') diff --git a/packages/backend/src/core/NotePiningService.ts b/packages/backend/src/core/NotePiningService.ts index d38b48b65d..6ab7268254 100644 --- a/packages/backend/src/core/NotePiningService.ts +++ b/packages/backend/src/core/NotePiningService.ts @@ -49,7 +49,7 @@ export class NotePiningService { * @param noteId */ @bindThis - public async addPinned(user: { id: MiUser['id']; host: MiUser['host']; }, noteId: MiNote['id']) { + public async addPinned(user: MiUser, noteId: MiNote['id']) { // Fetch pinee const note = await this.notesRepository.findOneBy({ id: noteId, @@ -78,7 +78,7 @@ export class NotePiningService { // Deliver to remote followers if (this.userEntityService.isLocalUser(user) && !note.localOnly && ['public', 'home'].includes(note.visibility)) { - this.deliverPinnedChange(user.id, note.id, true); + this.deliverPinnedChange(user, note.id, true); } } @@ -88,7 +88,7 @@ export class NotePiningService { * @param noteId */ @bindThis - public async removePinned(user: { id: MiUser['id']; host: MiUser['host']; }, noteId: MiNote['id']) { + public async removePinned(user: MiUser, noteId: MiNote['id']) { // Fetch unpinee const note = await this.notesRepository.findOneBy({ id: noteId, @@ -106,22 +106,19 @@ export class NotePiningService { // Deliver to remote followers if (this.userEntityService.isLocalUser(user) && !note.localOnly && ['public', 'home'].includes(note.visibility)) { - this.deliverPinnedChange(user.id, noteId, false); + this.deliverPinnedChange(user, noteId, false); } } @bindThis - public async deliverPinnedChange(userId: MiUser['id'], noteId: MiNote['id'], isAddition: boolean) { - const user = await this.usersRepository.findOneBy({ id: userId }); - if (user == null) throw new Error('user not found'); - + public async deliverPinnedChange(user: MiUser, noteId: MiNote['id'], isAddition: boolean) { if (!this.userEntityService.isLocalUser(user)) return; const target = `${this.config.url}/users/${user.id}/collections/featured`; const item = `${this.config.url}/notes/${noteId}`; const content = this.apRendererService.addContext(isAddition ? this.apRendererService.renderAdd(user, target, item) : this.apRendererService.renderRemove(user, target, item)); - this.apDeliverManagerService.deliverToFollowers(user, content); - this.relayService.deliverToRelays(user, content); + await this.apDeliverManagerService.deliverToFollowers(user, content); + await this.relayService.deliverToRelays(user, content); } } -- cgit v1.2.3-freya From d3a9995d0a252af877f401dc92802fb841291fa1 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 11 May 2025 06:02:52 -0400 Subject: use transaction to avoid unique constraint error when processing duplicate Add/Remove pinned note activities --- packages/backend/src/core/NotePiningService.ts | 32 +++++++++++++++----------- 1 file changed, 19 insertions(+), 13 deletions(-) (limited to 'packages/backend/src/core/NotePiningService.ts') diff --git a/packages/backend/src/core/NotePiningService.ts b/packages/backend/src/core/NotePiningService.ts index 6ab7268254..86f1a62d4a 100644 --- a/packages/backend/src/core/NotePiningService.ts +++ b/packages/backend/src/core/NotePiningService.ts @@ -10,7 +10,7 @@ import { IdentifiableError } from '@/misc/identifiable-error.js'; import type { MiUser } from '@/models/User.js'; import type { MiNote } from '@/models/Note.js'; import { IdService } from '@/core/IdService.js'; -import type { MiUserNotePining } from '@/models/UserNotePining.js'; +import { MiUserNotePining } from '@/models/UserNotePining.js'; import { RelayService } from '@/core/RelayService.js'; import type { Config } from '@/config.js'; import { UserEntityService } from '@/core/entities/UserEntityService.js'; @@ -18,6 +18,7 @@ import { ApDeliverManagerService } from '@/core/activitypub/ApDeliverManagerServ import { ApRendererService } from '@/core/activitypub/ApRendererService.js'; import { bindThis } from '@/decorators.js'; import { RoleService } from '@/core/RoleService.js'; +import type { DataSource } from 'typeorm'; @Injectable() export class NotePiningService { @@ -34,6 +35,9 @@ export class NotePiningService { @Inject(DI.userNotePiningsRepository) private userNotePiningsRepository: UserNotePiningsRepository, + @Inject(DI.db) + private readonly db: DataSource, + private userEntityService: UserEntityService, private idService: IdService, private roleService: RoleService, @@ -60,21 +64,23 @@ export class NotePiningService { throw new IdentifiableError('70c4e51f-5bea-449c-a030-53bee3cce202', 'No such note.'); } - const pinings = await this.userNotePiningsRepository.findBy({ userId: user.id }); + await this.db.transaction(async tem => { + const pinings = await tem.findBy(MiUserNotePining, { userId: user.id }); - if (pinings.length >= (await this.roleService.getUserPolicies(user.id)).pinLimit) { - throw new IdentifiableError('15a018eb-58e5-4da1-93be-330fcc5e4e1a', 'You can not pin notes any more.'); - } + if (pinings.length >= (await this.roleService.getUserPolicies(user.id)).pinLimit) { + throw new IdentifiableError('15a018eb-58e5-4da1-93be-330fcc5e4e1a', 'You can not pin notes any more.'); + } - if (pinings.some(pining => pining.noteId === note.id)) { - throw new IdentifiableError('23f0cf4e-59a3-4276-a91d-61a5891c1514', 'That note has already been pinned.'); - } + if (pinings.some(pining => pining.noteId === note.id)) { + throw new IdentifiableError('23f0cf4e-59a3-4276-a91d-61a5891c1514', 'That note has already been pinned.'); + } - await this.userNotePiningsRepository.insert({ - id: this.idService.gen(), - userId: user.id, - noteId: note.id, - } as MiUserNotePining); + await tem.insert(MiUserNotePining, { + id: this.idService.gen(), + userId: user.id, + noteId: note.id, + }); + }); // Deliver to remote followers if (this.userEntityService.isLocalUser(user) && !note.localOnly && ['public', 'home'].includes(note.visibility)) { -- cgit v1.2.3-freya