merge: Clone note when adding myReaction to streamed notes. (!854)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/854

Approved-by: dakkar <dakkar@thenautilus.net>
Approved-by: Marie <github@yuugi.dev>

packages/backend/src/server/api/stream/channel.ts

@@ -103,11 +103,42 @@ export default abstract class Channel {
 
 	public onMessage?(type: string, body: JsonValue): void;
 
-	public async assignMyReaction(note: Packed<'Note'>, noteEntityService: NoteEntityService) {
+	public async assignMyReaction(note: Packed<'Note'>, noteEntityService: NoteEntityService): Promise<Packed<'Note'>> {
-		if (this.user && Object.keys(note.reactions).length > 0) {
+		let changed = false;
-			const myReaction = await noteEntityService.populateMyReaction(note, this.user.id);
+		// StreamingApiServerService creates a single EventEmitter per server process,
-			note.myReaction = myReaction;
+		// 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 noteEntityService.populateMyReaction(note.renote, this.user.id);
+				if (myReaction) {
+					changed = true;
+					clonedNote.renote = { ...note.renote };
+					clonedNote.renote.myReaction = myReaction;
+				}
+			}
+			if (note.renote?.reply && Object.keys(note.renote.reply.reactions).length > 0) {
+				const myReaction = await noteEntityService.populateMyReaction(note.renote.reply, this.user.id);
+				if (myReaction) {
+					changed = true;
+					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 noteEntityService.populateMyReaction(note.reply, this.user.id);
+			if (myReaction) {
+				changed = true;
+				clonedNote.reply = { ...note.reply };
+				clonedNote.reply.myReaction = myReaction;
+			}
+		}
+		return changed ? clonedNote : note;
 	}
 }
 

packages/backend/src/server/api/stream/channels/bubble-timeline.ts

@@ -65,24 +65,11 @@ class BubbleTimelineChannel extends Channel {
 
 		if (this.isNoteMutedOrBlocked(note)) return;
 
-		const reactionsToFetch = [];
+		const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
-		if (this.user && isRenotePacked(note) && !isQuotePacked(note)) {
-			if (note.renote) {
-				reactionsToFetch.push(this.assignMyReaction(note.renote, this.noteEntityService));
-				if (note.renote.reply) {
-					reactionsToFetch.push(this.assignMyReaction(note.renote.reply, this.noteEntityService));
-				}
-			}
-		}
-		if (this.user && note.reply) {
-			reactionsToFetch.push(this.assignMyReaction(note.reply, this.noteEntityService));
-		}
 
-		await Promise.all(reactionsToFetch);
+		this.connection.cacheNote(clonedNote);
 
-		this.connection.cacheNote(note);
+		this.send('note', clonedNote);
-
-		this.send('note', note);
 	}
 
 	@bindThis

packages/backend/src/server/api/stream/channels/global-timeline.ts

@@ -60,24 +60,11 @@ class GlobalTimelineChannel extends Channel {
 
 		if (this.isNoteMutedOrBlocked(note)) return;
 
-		const reactionsToFetch = [];
+		const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
-		if (this.user && isRenotePacked(note) && !isQuotePacked(note)) {
-			if (note.renote) {
-				reactionsToFetch.push(this.assignMyReaction(note.renote, this.noteEntityService));
-				if (note.renote.reply) {
-					reactionsToFetch.push(this.assignMyReaction(note.renote.reply, this.noteEntityService));
-				}
-			}
-		}
-		if (this.user && note.reply) {
-			reactionsToFetch.push(this.assignMyReaction(note.reply, this.noteEntityService));
-		}
 
-		await Promise.all(reactionsToFetch);
+		this.connection.cacheNote(clonedNote);
 
-		this.connection.cacheNote(note);
+		this.send('note', clonedNote);
-
-		this.send('note', note);
 	}
 
 	@bindThis

packages/backend/src/server/api/stream/channels/home-timeline.ts

@@ -81,24 +81,11 @@ class HomeTimelineChannel extends Channel {
 
 		if (this.isNoteMutedOrBlocked(note)) return;
 
-		const reactionsToFetch = [];
+		const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
-		if (this.user && isRenotePacked(note) && !isQuotePacked(note)) {
-			if (note.renote) {
-				reactionsToFetch.push(this.assignMyReaction(note.renote, this.noteEntityService));
-				if (note.renote.reply) {
-					reactionsToFetch.push(this.assignMyReaction(note.renote.reply, this.noteEntityService));
-				}
-			}
-		}
-		if (this.user && note.reply) {
-			reactionsToFetch.push(this.assignMyReaction(note.reply, this.noteEntityService));
-		}
 
-		await Promise.all(reactionsToFetch);
+		this.connection.cacheNote(clonedNote);
 
-		this.connection.cacheNote(note);
+		this.send('note', clonedNote);
-
-		this.send('note', note);
 	}
 
 	@bindThis

packages/backend/src/server/api/stream/channels/hybrid-timeline.ts

@@ -98,24 +98,11 @@ class HybridTimelineChannel extends Channel {
 			}
 		}
 
-		const reactionsToFetch = [];
+		const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
-		if (this.user && isRenotePacked(note) && !isQuotePacked(note)) {
-			if (note.renote) {
-				reactionsToFetch.push(this.assignMyReaction(note.renote, this.noteEntityService));
-				if (note.renote.reply) {
-					reactionsToFetch.push(this.assignMyReaction(note.renote.reply, this.noteEntityService));
-				}
-			}
-		}
-		if (this.user && note.reply) {
-			reactionsToFetch.push(this.assignMyReaction(note.reply, this.noteEntityService));
-		}
 
-		await Promise.all(reactionsToFetch);
+		this.connection.cacheNote(clonedNote);
 
-		this.connection.cacheNote(note);
+		this.send('note', clonedNote);
-
-		this.send('note', note);
 	}
 
 	@bindThis

packages/backend/src/server/api/stream/channels/local-timeline.ts

@@ -70,24 +70,11 @@ class LocalTimelineChannel extends Channel {
 
 		if (this.isNoteMutedOrBlocked(note)) return;
 
-		const reactionsToFetch = [];
+		const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
-		if (this.user && isRenotePacked(note) && !isQuotePacked(note)) {
-			if (note.renote) {
-				reactionsToFetch.push(this.assignMyReaction(note.renote, this.noteEntityService));
-				if (note.renote.reply) {
-					reactionsToFetch.push(this.assignMyReaction(note.renote.reply, this.noteEntityService));
-				}
-			}
-		}
-		if (this.user && note.reply) {
-			reactionsToFetch.push(this.assignMyReaction(note.reply, this.noteEntityService));
-		}
 
-		await Promise.all(reactionsToFetch);
+		this.connection.cacheNote(clonedNote);
 
-		this.connection.cacheNote(note);
+		this.send('note', clonedNote);
-
-		this.send('note', note);
 	}
 
 	@bindThis