From 5fc9c1c8cdebec868440cd8ed4b3ad84f214e117 Mon Sep 17 00:00:00 2001
From: piuvas <piuvas@proton.me>
Date: Wed, 8 Jan 2025 12:51:46 -0300
Subject: [PATCH 1/5] shallow clone
---
.../backend/src/server/api/stream/channel.ts | 37 ++++++++++++++++---
.../api/stream/channels/bubble-timeline.ts | 23 +++---------
.../api/stream/channels/global-timeline.ts | 23 +++---------
.../api/stream/channels/home-timeline.ts | 19 ++--------
.../api/stream/channels/hybrid-timeline.ts | 19 ++--------
.../api/stream/channels/local-timeline.ts | 19 ++--------
6 files changed, 51 insertions(+), 89 deletions(-)
diff --git a/packages/backend/src/server/api/stream/channel.ts b/packages/backend/src/server/api/stream/channel.ts
index 93d5046902..ed6e41c4bb 100644
--- a/packages/backend/src/server/api/stream/channel.ts
+++ b/packages/backend/src/server/api/stream/channel.ts
@@ -9,8 +9,8 @@ import { isUserRelated } from '@/misc/is-user-related.js';
import { isRenotePacked, isQuotePacked } from '@/misc/is-renote.js';
import type { Packed } from '@/misc/json-schema.js';
import type { JsonObject, JsonValue } from '@/misc/json-value.js';
-import type Connection from './Connection.js';
import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
+import type Connection from './Connection.js';
/**
* Stream channel
@@ -103,14 +103,41 @@ export default abstract class Channel {
public onMessage?(type: string, body: JsonValue): void;
- public async assignMyReaction(note: Packed<'Note'>, noteEntityService: NoteEntityService) {
- if (this.user && Object.keys(note.reactions).length > 0) {
- const myReaction = await noteEntityService.populateMyReaction(note, this.user.id);
- note.myReaction = myReaction;
+ public async assignMyReaction(note: Packed<'Note'>, noteEntityService: NoteEntityService): Promise<Packed<'Note'>> {
+ let changed = false;
+ 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 (this.user && 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;
}
}
+
export type MiChannelService<T extends boolean> = {
shouldShare: boolean;
requireCredential: T;
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 b2745db92d..98ecf16a83 100644
--- a/packages/backend/src/server/api/stream/channels/bubble-timeline.ts
+++ b/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 = [];
- 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(note);
-
- this.send('note', note);
+ const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
+
+ this.connection.cacheNote(clonedNote);
+
+ this.send('note', clonedNote);
}
@bindThis
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 8df59d906d..4443b20bed 100644
--- a/packages/backend/src/server/api/stream/channels/global-timeline.ts
+++ b/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 = [];
- 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(note);
-
- this.send('note', note);
+ const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
+
+ this.connection.cacheNote(clonedNote);
+
+ this.send('note', clonedNote);
}
@bindThis
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 f48eff85c9..af1b17b533 100644
--- a/packages/backend/src/server/api/stream/channels/home-timeline.ts
+++ b/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 = [];
- 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);
+ const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
- this.connection.cacheNote(note);
+ this.connection.cacheNote(clonedNote);
- this.send('note', note);
+ this.send('note', clonedNote);
}
@bindThis
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 8c58b2518e..7c604c0b58 100644
--- a/packages/backend/src/server/api/stream/channels/hybrid-timeline.ts
+++ b/packages/backend/src/server/api/stream/channels/hybrid-timeline.ts
@@ -98,24 +98,11 @@ class HybridTimelineChannel extends Channel {
}
}
- const reactionsToFetch = [];
- 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);
+ const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
- this.connection.cacheNote(note);
+ this.connection.cacheNote(clonedNote);
- this.send('note', note);
+ this.send('note', clonedNote);
}
@bindThis
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 cb832bd3c2..2d48b6ecfb 100644
--- a/packages/backend/src/server/api/stream/channels/local-timeline.ts
+++ b/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 = [];
- 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);
+ const clonedNote = await this.assignMyReaction(note, this.noteEntityService);
- this.connection.cacheNote(note);
+ this.connection.cacheNote(clonedNote);
- this.send('note', note);
+ this.send('note', clonedNote);
}
@bindThis
--
GitLab
From e76e6cd08f69ddd7f8c62039b9d3e64a7a6189ca Mon Sep 17 00:00:00 2001
From: piuvas <piuvas@proton.me>
Date: Wed, 8 Jan 2025 12:58:57 -0300
Subject: [PATCH 2/5] small refactor
---
.../backend/src/server/api/stream/channel.ts | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/packages/backend/src/server/api/stream/channel.ts b/packages/backend/src/server/api/stream/channel.ts
index ed6e41c4bb..6589a7af8d 100644
--- a/packages/backend/src/server/api/stream/channel.ts
+++ b/packages/backend/src/server/api/stream/channel.ts
@@ -115,14 +115,14 @@ export default abstract class Channel {
clonedNote.renote.myReaction = myReaction;
}
}
- }
- if (this.user && 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 (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) {
--
GitLab
From a3fc9a1085c7ad1fedf64fd6417c04cdcc936887 Mon Sep 17 00:00:00 2001
From: piuvas <piuvas@proton.me>
Date: Wed, 8 Jan 2025 13:10:20 -0300
Subject: [PATCH 3/5] comment code
---
packages/backend/src/server/api/stream/channel.ts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/packages/backend/src/server/api/stream/channel.ts b/packages/backend/src/server/api/stream/channel.ts
index 6589a7af8d..7a044296fe 100644
--- a/packages/backend/src/server/api/stream/channel.ts
+++ b/packages/backend/src/server/api/stream/channel.ts
@@ -105,6 +105,8 @@ export default abstract class Channel {
public async assignMyReaction(note: Packed<'Note'>, noteEntityService: NoteEntityService): Promise<Packed<'Note'>> {
let changed = false;
+ // cloning here seems like the best solution for a race condition
+ // where multiple users shared the same myReaction. (Sharkey #877)
const clonedNote = { ...note };
if (this.user && isRenotePacked(note) && !isQuotePacked(note)) {
if (note.renote && Object.keys(note.renote.reactions).length > 0) {
--
GitLab
From f1d9bb2cf1d9045ec13cdeb231656054b9e5bfde Mon Sep 17 00:00:00 2001
From: piuvas <piuvas@proton.me>
Date: Fri, 10 Jan 2025 22:10:18 -0300
Subject: [PATCH 4/5] requested changes
---
packages/backend/src/server/api/stream/channel.ts | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/packages/backend/src/server/api/stream/channel.ts b/packages/backend/src/server/api/stream/channel.ts
index 7a044296fe..cfac4b30ce 100644
--- a/packages/backend/src/server/api/stream/channel.ts
+++ b/packages/backend/src/server/api/stream/channel.ts
@@ -9,8 +9,8 @@ import { isUserRelated } from '@/misc/is-user-related.js';
import { isRenotePacked, isQuotePacked } from '@/misc/is-renote.js';
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 { NoteEntityService } from '@/core/entities/NoteEntityService.js';
/**
* Stream channel
@@ -105,7 +105,7 @@ export default abstract class Channel {
public async assignMyReaction(note: Packed<'Note'>, noteEntityService: NoteEntityService): Promise<Packed<'Note'>> {
let changed = false;
- // cloning here seems like the best solution for a race condition
+ // cloning here seems like the best solution for not sharing changes with other users.
// where multiple users shared the same myReaction. (Sharkey #877)
const clonedNote = { ...note };
if (this.user && isRenotePacked(note) && !isQuotePacked(note)) {
@@ -139,7 +139,6 @@ export default abstract class Channel {
}
}
-
export type MiChannelService<T extends boolean> = {
shouldShare: boolean;
requireCredential: T;
--
GitLab
From 3fc377839c6155d1eb3c55ae7eb24e85e4ad90c7 Mon Sep 17 00:00:00 2001
From: piuvas <mail@piuvas.net>
Date: Sun, 12 Jan 2025 21:11:36 -0300
Subject: [PATCH 5/5] comment :3
---
packages/backend/src/server/api/stream/channel.ts | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/packages/backend/src/server/api/stream/channel.ts b/packages/backend/src/server/api/stream/channel.ts
index cfac4b30ce..047dedd5ce 100644
--- a/packages/backend/src/server/api/stream/channel.ts
+++ b/packages/backend/src/server/api/stream/channel.ts
@@ -105,8 +105,11 @@ export default abstract class Channel {
public async assignMyReaction(note: Packed<'Note'>, noteEntityService: NoteEntityService): Promise<Packed<'Note'>> {
let changed = false;
- // cloning here seems like the best solution for not sharing changes with other users.
- // where multiple users shared the same myReaction. (Sharkey #877)
+ // 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) {
--
GitLab