From 4c2a0fed63649c087b406a6814dbcb96085983e4 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 23 Jun 2025 15:45:47 -0400 Subject: [PATCH] fix streaming API notes missing reactions, not always being hidden, and having incorrect values for the isRenoted, isFavorited, isMutingThread, and isMutingNote properties --- .../src/core/entities/NoteEntityService.ts | 171 +++++++++++++++++- .../backend/src/server/api/stream/channel.ts | 95 +++++----- .../api/stream/channels/bubble-timeline.ts | 4 +- .../src/server/api/stream/channels/channel.ts | 4 +- .../api/stream/channels/global-timeline.ts | 4 +- .../src/server/api/stream/channels/hashtag.ts | 4 +- .../api/stream/channels/home-timeline.ts | 4 +- .../api/stream/channels/hybrid-timeline.ts | 4 +- .../api/stream/channels/local-timeline.ts | 4 +- .../api/stream/channels/role-timeline.ts | 4 +- .../server/api/stream/channels/user-list.ts | 4 +- .../src/server/web/ClientServerService.ts | 1 + 12 files changed, 227 insertions(+), 76 deletions(-) diff --git a/packages/backend/src/core/entities/NoteEntityService.ts b/packages/backend/src/core/entities/NoteEntityService.ts index c87c6d95dc..6720644197 100644 --- a/packages/backend/src/core/entities/NoteEntityService.ts +++ b/packages/backend/src/core/entities/NoteEntityService.ts @@ -137,9 +137,21 @@ export class NoteEntityService implements OnModuleInit { return packedNote.visibility; } + @bindThis + public async hideNotes(notes: Packed<'Note'>[], meId: string | null): Promise { + const myFollowing = meId ? new Map(await this.cacheService.userFollowingsCache.fetch(meId)) : new Map>(); + const myBlockers = meId ? new Set(await this.cacheService.userBlockedCache.fetch(meId)) : new Set(); + + // This shouldn't actually await, but we have to wrap it anyway because hideNote() is async + await Promise.all(notes.map(note => this.hideNote(note, meId, { + myFollowing, + myBlockers, + }))); + } + @bindThis public async hideNote(packedNote: Packed<'Note'>, meId: MiUser['id'] | null, hint?: { - myFollowing?: ReadonlyMap, + myFollowing?: ReadonlyMap> | ReadonlySet, myBlockers?: ReadonlySet, }): Promise { if (meId === packedNote.userId) return; @@ -281,6 +293,152 @@ export class NoteEntityService implements OnModuleInit { }; } + @bindThis + public async populateMyNoteMutings(notes: Packed<'Note'>[], meId: string): Promise> { + const mutedNotes = await this.cacheService.noteMutingsCache.fetch(meId); + + const mutedIds = notes + .filter(note => mutedNotes.has(note.id)) + .map(note => note.id); + return new Set(mutedIds); + } + + @bindThis + public async populateMyTheadMutings(notes: Packed<'Note'>[], meId: string): Promise> { + const mutedThreads = await this.cacheService.threadMutingsCache.fetch(meId); + + const mutedIds = notes + .filter(note => mutedThreads.has(note.threadId)) + .map(note => note.id); + return new Set(mutedIds); + } + + @bindThis + public async populateMyRenotes(notes: Packed<'Note'>[], meId: string, _hint_?: { + myRenotes: Map; + }): Promise> { + const fetchedRenotes = new Set(); + const toFetch = new Set(); + + if (_hint_) { + for (const note of notes) { + const fromHint = _hint_.myRenotes.get(note.id); + + // null means we know there's no renote, so just skip it. + if (fromHint === false) continue; + + if (fromHint) { + fetchedRenotes.add(note.id); + } else { + toFetch.add(note.id); + } + } + } + + if (toFetch.size > 0) { + const fetched = await this.queryService + .andIsRenote(this.notesRepository.createQueryBuilder('note'), 'note') + .andWhere({ + userId: meId, + renoteId: In(Array.from(toFetch)), + }) + .select('note.renoteId', 'renoteId') + .getRawMany<{ renoteId: string }>(); + + for (const { renoteId } of fetched) { + fetchedRenotes.add(renoteId); + } + } + + return fetchedRenotes; + } + + @bindThis + public async populateMyFavorites(notes: Packed<'Note'>[], meId: string, _hint_?: { + myFavorites: Map; + }): Promise> { + const fetchedFavorites = new Set(); + const toFetch = new Set(); + + if (_hint_) { + for (const note of notes) { + const fromHint = _hint_.myFavorites.get(note.id); + + // null means we know there's no favorite, so just skip it. + if (fromHint === false) continue; + + if (fromHint) { + fetchedFavorites.add(note.id); + } else { + toFetch.add(note.id); + } + } + } + + if (toFetch.size > 0) { + const fetched = await this.noteFavoritesRepository.find({ + where: { + userId: meId, + noteId: In(Array.from(toFetch)), + }, + select: { + noteId: true, + }, + }) as { noteId: string }[]; + + for (const { noteId } of fetched) { + fetchedFavorites.add(noteId); + } + } + + return fetchedFavorites; + } + + @bindThis + public async populateMyReactions(notes: Packed<'Note'>[], meId: string, _hint_?: { + myReactions: Map; + }): Promise> { + const fetchedReactions = new Map(); + const toFetch = new Set(); + + if (_hint_) { + for (const note of notes) { + const fromHint = _hint_.myReactions.get(note.id); + + // null means we know there's no reaction, so just skip it. + if (fromHint === null) continue; + + if (fromHint) { + const converted = this.reactionService.convertLegacyReaction(fromHint); + fetchedReactions.set(note.id, converted); + } else if (Object.values(note.reactions).some(count => count > 0)) { + // Note has at least one reaction, so we need to fetch + toFetch.add(note.id); + } + } + } + + if (toFetch.size > 0) { + const fetched = await this.noteReactionsRepository.find({ + where: { + userId: meId, + noteId: In(Array.from(toFetch)), + }, + select: { + noteId: true, + reaction: true, + }, + }); + + for (const { noteId, reaction } of fetched) { + const converted = this.reactionService.convertLegacyReaction(reaction); + fetchedReactions.set(noteId, converted); + } + } + + return fetchedReactions; + } + @bindThis public async populateMyReaction(note: { id: MiNote['id']; reactions: MiNote['reactions']; reactionAndUserPairCache?: MiNote['reactionAndUserPairCache']; }, meId: MiUser['id'], _hint_?: { myReactions: Map; @@ -312,9 +470,14 @@ export class NoteEntityService implements OnModuleInit { return undefined; } - const reaction = await this.noteReactionsRepository.findOneBy({ - userId: meId, - noteId: note.id, + const reaction = await this.noteReactionsRepository.findOne({ + where: { + userId: meId, + noteId: note.id, + }, + select: { + reaction: true, + }, }); if (reaction) { diff --git a/packages/backend/src/server/api/stream/channel.ts b/packages/backend/src/server/api/stream/channel.ts index 397f9e9367..4417d6841e 100644 --- a/packages/backend/src/server/api/stream/channel.ts +++ b/packages/backend/src/server/api/stream/channel.ts @@ -10,7 +10,8 @@ import { isRenotePacked, isQuotePacked, isPackedPureRenote } from '@/misc/is-ren import type { Packed } from '@/misc/json-schema.js'; import type { JsonObject, JsonValue } from '@/misc/json-value.js'; import { NoteEntityService } from '@/core/entities/NoteEntityService.js'; -import type Connection from './Connection.js'; +import { deepClone } from '@/misc/clone.js'; +import type Connection from '@/server/api/stream/Connection.js'; /** * Stream channel @@ -116,24 +117,6 @@ export default abstract class Channel { return false; } - /** - * This function modifies {@link note}, please make sure it has been shallow cloned. - * See Dakkar's comment of {@link assignMyReaction} for more - * @param note The note to change - */ - protected async hideNote(note: Packed<'Note'>): Promise { - if (note.renote) { - await this.hideNote(note.renote); - } - - if (note.reply) { - await this.hideNote(note.reply); - } - - const meId = this.user?.id ?? null; - await this.noteEntityService.hideNote(note, meId); - } - constructor(id: string, connection: Connection, noteEntityService: NoteEntityService) { this.id = id; this.connection = connection; @@ -160,37 +143,41 @@ export default abstract class Channel { public onMessage?(type: string, body: JsonValue): void; - public async assignMyReaction(note: Packed<'Note'>): Promise> { + public async rePackNote(note: Packed<'Note'>): Promise> { + // If there's no user, then packing won't change anything. + // We can just re-use the original note. + if (!this.user) { + return note; + } + // StreamingApiServerService creates a single EventEmitter per server process, // so a new note arriving from redis gets de-serialised once per server process, // and then that single object is passed to all active channels on each connection. // If we didn't clone the notes here, different connections would asynchronously write // different values to the same object, resulting in a random value being sent to each frontend. -- Dakkar - const clonedNote = { ...note }; - if (this.user && isRenotePacked(note) && !isQuotePacked(note)) { - if (note.renote && Object.keys(note.renote.reactions).length > 0) { - const myReaction = await this.noteEntityService.populateMyReaction(note.renote, this.user.id); - if (myReaction) { - clonedNote.renote = { ...note.renote }; - clonedNote.renote.myReaction = myReaction; - } - } - if (note.renote?.reply && Object.keys(note.renote.reply.reactions).length > 0) { - const myReaction = await this.noteEntityService.populateMyReaction(note.renote.reply, this.user.id); - if (myReaction) { - clonedNote.renote = { ...note.renote }; - clonedNote.renote.reply = { ...note.renote.reply }; - clonedNote.renote.reply.myReaction = myReaction; - } - } - } - if (this.user && note.reply && Object.keys(note.reply.reactions).length > 0) { - const myReaction = await this.noteEntityService.populateMyReaction(note.reply, this.user.id); - if (myReaction) { - clonedNote.reply = { ...note.reply }; - clonedNote.reply.myReaction = myReaction; - } - } + const clonedNote = deepClone(note); + const notes = crawl(clonedNote); + + // Hide notes before everything else, since this modifies fields that the other functions will check. + await this.noteEntityService.hideNotes(notes, this.user.id); + + // TODO cache reaction/renote/favorite hints in the connection. + // Those functions accept partial hints and will fetch anything else. + + const [myReactions, myRenotes, myFavorites, myThreadMutings, myNoteMutings] = await Promise.all([ + this.noteEntityService.populateMyReactions(notes, this.user.id), + this.noteEntityService.populateMyRenotes(notes, this.user.id), + this.noteEntityService.populateMyFavorites(notes, this.user.id), + this.noteEntityService.populateMyTheadMutings(notes, this.user.id), + this.noteEntityService.populateMyNoteMutings(notes, this.user.id), + ]); + + note.myReaction = myReactions.get(note.id) ?? null; + note.isRenoted = myRenotes.has(note.id); + note.isFavorited = myFavorites.has(note.id); + note.isMutingThread = myThreadMutings.has(note.id); + note.isMutingNote = myNoteMutings.has(note.id); + return clonedNote; } } @@ -201,3 +188,21 @@ export type MiChannelService = { kind: T extends true ? string : string | null | undefined; create: (id: string, connection: Connection) => Channel; }; + +function crawl(note: Packed<'Note'>, into?: Packed<'Note'>[]): Packed<'Note'>[] { + into ??= []; + + if (!into.includes(note)) { + into.push(note); + } + + if (note.reply) { + crawl(note.reply, into); + } + + if (note.renote) { + crawl(note.renote, into); + } + + return into; +} diff --git a/packages/backend/src/server/api/stream/channels/bubble-timeline.ts b/packages/backend/src/server/api/stream/channels/bubble-timeline.ts index 72f719b411..a7f538eeda 100644 --- a/packages/backend/src/server/api/stream/channels/bubble-timeline.ts +++ b/packages/backend/src/server/api/stream/channels/bubble-timeline.ts @@ -78,9 +78,7 @@ class BubbleTimelineChannel extends Channel { } } - const clonedNote = await this.assignMyReaction(note); - await this.hideNote(clonedNote); - + const clonedNote = await this.rePackNote(note); this.send('note', clonedNote); } diff --git a/packages/backend/src/server/api/stream/channels/channel.ts b/packages/backend/src/server/api/stream/channels/channel.ts index 65fb8d67cb..9eea423088 100644 --- a/packages/backend/src/server/api/stream/channels/channel.ts +++ b/packages/backend/src/server/api/stream/channels/channel.ts @@ -49,9 +49,7 @@ class ChannelChannel extends Channel { if (this.isNoteMutedOrBlocked(note)) return; - const clonedNote = await this.assignMyReaction(note); - await this.hideNote(clonedNote); - + const clonedNote = await this.rePackNote(note); this.send('note', clonedNote); } diff --git a/packages/backend/src/server/api/stream/channels/global-timeline.ts b/packages/backend/src/server/api/stream/channels/global-timeline.ts index 5c73f637c7..9bdd299ddb 100644 --- a/packages/backend/src/server/api/stream/channels/global-timeline.ts +++ b/packages/backend/src/server/api/stream/channels/global-timeline.ts @@ -79,9 +79,7 @@ class GlobalTimelineChannel extends Channel { } } - const clonedNote = await this.assignMyReaction(note); - await this.hideNote(clonedNote); - + const clonedNote = await this.rePackNote(note); this.send('note', clonedNote); } diff --git a/packages/backend/src/server/api/stream/channels/hashtag.ts b/packages/backend/src/server/api/stream/channels/hashtag.ts index f47a10f293..e0331828d2 100644 --- a/packages/backend/src/server/api/stream/channels/hashtag.ts +++ b/packages/backend/src/server/api/stream/channels/hashtag.ts @@ -45,9 +45,7 @@ class HashtagChannel extends Channel { if (this.isNoteMutedOrBlocked(note)) return; - const clonedNote = await this.assignMyReaction(note); - await this.hideNote(clonedNote); - + const clonedNote = await this.rePackNote(note); this.send('note', clonedNote); } diff --git a/packages/backend/src/server/api/stream/channels/home-timeline.ts b/packages/backend/src/server/api/stream/channels/home-timeline.ts index c7062c0394..bb28cbf81e 100644 --- a/packages/backend/src/server/api/stream/channels/home-timeline.ts +++ b/packages/backend/src/server/api/stream/channels/home-timeline.ts @@ -73,9 +73,7 @@ class HomeTimelineChannel extends Channel { } } - const clonedNote = await this.assignMyReaction(note); - await this.hideNote(clonedNote); - + const clonedNote = await this.rePackNote(note); this.send('note', clonedNote); } diff --git a/packages/backend/src/server/api/stream/channels/hybrid-timeline.ts b/packages/backend/src/server/api/stream/channels/hybrid-timeline.ts index 7cb64c9f89..82d96c6e7e 100644 --- a/packages/backend/src/server/api/stream/channels/hybrid-timeline.ts +++ b/packages/backend/src/server/api/stream/channels/hybrid-timeline.ts @@ -90,9 +90,7 @@ class HybridTimelineChannel extends Channel { } } - const clonedNote = await this.assignMyReaction(note); - await this.hideNote(clonedNote); - + const clonedNote = await this.rePackNote(note); this.send('note', clonedNote); } diff --git a/packages/backend/src/server/api/stream/channels/local-timeline.ts b/packages/backend/src/server/api/stream/channels/local-timeline.ts index 4869d871d6..c6c04d356f 100644 --- a/packages/backend/src/server/api/stream/channels/local-timeline.ts +++ b/packages/backend/src/server/api/stream/channels/local-timeline.ts @@ -82,9 +82,7 @@ class LocalTimelineChannel extends Channel { } } - const clonedNote = await this.assignMyReaction(note); - await this.hideNote(clonedNote); - + const clonedNote = await this.rePackNote(note); this.send('note', clonedNote); } diff --git a/packages/backend/src/server/api/stream/channels/role-timeline.ts b/packages/backend/src/server/api/stream/channels/role-timeline.ts index a3886618f1..2a8d4f7ffd 100644 --- a/packages/backend/src/server/api/stream/channels/role-timeline.ts +++ b/packages/backend/src/server/api/stream/channels/role-timeline.ts @@ -70,9 +70,7 @@ class RoleTimelineChannel extends Channel { } } - const clonedNote = await this.assignMyReaction(note); - await this.hideNote(clonedNote); - + const clonedNote = await this.rePackNote(note); this.send('note', clonedNote); } else { this.send(data.type, data.body); diff --git a/packages/backend/src/server/api/stream/channels/user-list.ts b/packages/backend/src/server/api/stream/channels/user-list.ts index 4dae24a696..98bcbd11ba 100644 --- a/packages/backend/src/server/api/stream/channels/user-list.ts +++ b/packages/backend/src/server/api/stream/channels/user-list.ts @@ -114,9 +114,7 @@ class UserListChannel extends Channel { } } - const clonedNote = await this.assignMyReaction(note); - await this.hideNote(clonedNote); - + const clonedNote = await this.rePackNote(note); this.send('note', clonedNote); } diff --git a/packages/backend/src/server/web/ClientServerService.ts b/packages/backend/src/server/web/ClientServerService.ts index c40d042fa4..654d477628 100644 --- a/packages/backend/src/server/web/ClientServerService.ts +++ b/packages/backend/src/server/web/ClientServerService.ts @@ -601,6 +601,7 @@ export class ClientServerService { relations: ['user'], }); + // TODO pack with current user, or the frontend can get bad data if (note && !note.user!.requireSigninToViewContents) { const _note = await this.noteEntityService.pack(note); const profile = await this.userProfilesRepository.findOneByOrFail({ userId: note.userId });