Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(backend): ダイレクト投稿がタイムライン上に正常に表示されない問題を修正 #11993

Merged
merged 10 commits into from
Oct 9, 2023
Merged

fix(backend): ダイレクト投稿がタイムライン上に正常に表示されない問題を修正 #11993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b780208
DMをredisにpushするように
slofp Oct 8, 2023
515044a
add test
slofp Oct 8, 2023
2223afd
Merge remote-tracking branch 'upstream/develop' into dm-push-tl
slofp Oct 8, 2023
5efd05b
Merge remote-tracking branch 'upstream/develop' into dm-push-tl
slofp Oct 8, 2023
a3d6525
add CHANGELOG
slofp Oct 8, 2023
8239328
Merge branch 'develop' into pr/11993
syuilo Oct 9, 2023
1788cbe
Update NoteCreateService.ts
syuilo Oct 9, 2023
8bdba1d
lint
syuilo Oct 9, 2023
400e8d7
:v:
syuilo Oct 9, 2023
beafb27
前のバージョンから発生した問題ではないため不要
syuilo Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 33 additions & 27 deletions packages/backend/src/core/NoteCreateService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,11 +494,7 @@ export class NoteCreateService implements OnApplicationShutdown {
// Increment notes count (user)
this.incNotesCountOfUser(user);

if (data.visibility === 'specified') {
// TODO?
} else {
this.pushToTl(note, user);
}
this.pushToTl(note, user);

this.antennaService.addNoteToAntennas(note, user);

Expand Down Expand Up @@ -861,24 +857,34 @@ export class NoteCreateService implements OnApplicationShutdown {
}
} else {
// TODO: キャッシュ？
const followings = await this.followingsRepository.find({
where: {
followeeId: user.id,
followerHost: IsNull(),
isFollowerHibernated: false,
},
select: ['followerId', 'withReplies'],
});

const userListMemberships = await this.userListMembershipsRepository.find({
where: {
userId: user.id,
},
select: ['userListId', 'withReplies'],
});
// eslint-disable-next-line prefer-const
let [followings, userListMemberships] = await Promise.all([
this.followingsRepository.find({
where: {
followeeId: user.id,
followerHost: IsNull(),
isFollowerHibernated: false,
},
select: ['followerId', 'withReplies'],
}),
this.userListMembershipsRepository.find({
where: {
userId: user.id,
},
select: ['userListId', 'userListUserId', 'withReplies'],
}),
]);

if (note.visibility === 'followers') {
// TODO: 重そうだから何とかしたい Set 使う？
userListMemberships = userListMemberships.filter(x => followings.some(f => f.followerId === x.userListUserId));
}

// TODO: あまりにも数が多いと redisPipeline.exec に失敗する(理由は不明)ため、3万件程度を目安に分割して実行するようにする
for (const following of followings) {
// 基本的にvisibleUserIdsには自身のidが含まれている前提であること
if (note.visibility === 'specified' && !note.visibleUserIds.some(v => v === following.followerId)) continue;

// 自分自身以外への返信
if (note.replyId && note.replyUserId !== note.userId) {
if (!following.withReplies) continue;
Expand All @@ -899,13 +905,13 @@ export class NoteCreateService implements OnApplicationShutdown {
}
}

// TODO
//if (note.visibility === 'followers') {
// // TODO: 重そうだから何とかしたい Set 使う？
// userLists = userLists.filter(x => followings.some(f => f.followerId === x.userListUserId));
//}

for (const userListMembership of userListMemberships) {
// ダイレクトのとき、そのリストが対象外のユーザーの場合
if (
note.visibility === 'specified' &&
!note.visibleUserIds.some(v => v === userListMembership.userListUserId)
) continue;

// 自分自身以外への返信
if (note.replyId && note.replyUserId !== note.userId) {
if (!userListMembership.withReplies) continue;
Expand All @@ -926,7 +932,7 @@ export class NoteCreateService implements OnApplicationShutdown {
}
}

{ // 自分自身のHTL
if (note.visibility !== 'specified' || !note.visibleUserIds.some(v => v === user.id)) { // 自分自身のHTL
redisPipeline.xadd(
`homeTimeline:${user.id}`,
'MAXLEN', '~', meta.perUserHomeTimelineCacheMax.toString(),
Expand Down
1 change: 1 addition & 0 deletions packages/backend/src/server/api/endpoints/users/notes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
}
}

if (note.visibility === 'specified' && (!me || (me.id !== note.userId && !note.visibleUserIds.some(v => v === me.id)))) return false;
if (note.visibility === 'followers' && !isFollowing) return false;

return true;
Expand Down
175 changes: 157 additions & 18 deletions packages/backend/test/e2e/timelines.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/

// How to run:
// pnpm jest -- e2e/timelines.ts

process.env.NODE_ENV = 'test';
process.env.FORCE_FOLLOW_REMOTE_USER_FOR_TESTING = 'true';

Expand Down Expand Up @@ -378,6 +381,104 @@ describe('Timelines', () => {

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), false);
});

test.concurrent('自分の visibility: specified なノートが含まれる', async () => {
const [alice] = await Promise.all([signup()]);

const aliceNote = await post(alice, { text: 'hi', visibility: 'specified' });

await waitForPushToTl();

const res = await api('/notes/timeline', {}, alice);

assert.strictEqual(res.body.some((note: any) => note.id === aliceNote.id), true);
assert.strictEqual(res.body.find((note: any) => note.id === aliceNote.id).text, 'hi');
});

test.concurrent('フォローしているユーザーの自身を visibleUserIds に指定した visibility: specified なノートが含まれる', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

await api('/following/create', { userId: bob.id }, alice);
await sleep(1000);
const bobNote = await post(bob, { text: 'hi', visibility: 'specified', visibleUserIds: [alice.id] });

await waitForPushToTl();

const res = await api('/notes/timeline', {}, alice);

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), true);
assert.strictEqual(res.body.find((note: any) => note.id === bobNote.id).text, 'hi');
});

test.concurrent('フォローしていないユーザーの自身を visibleUserIds に指定した visibility: specified なノートが含まれない', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

const bobNote = await post(bob, { text: 'hi', visibility: 'specified', visibleUserIds: [alice.id] });

await waitForPushToTl();

const res = await api('/notes/timeline', {}, alice);

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), false);
});

test.concurrent('フォローしているユーザーの自身を visibleUserIds に指定していない visibility: specified なノートが含まれない', async () => {
const [alice, bob, carol] = await Promise.all([signup(), signup(), signup()]);

await api('/following/create', { userId: bob.id }, alice);
await sleep(1000);
const bobNote = await post(bob, { text: 'hi', visibility: 'specified', visibleUserIds: [carol.id] });

await waitForPushToTl();

const res = await api('/notes/timeline', {}, alice);

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), false);
});

test.concurrent('フォローしていないユーザーからの visibility: specified なノートに返信したときの自身のノートが含まれる', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

const bobNote = await post(bob, { text: 'hi', visibility: 'specified', visibleUserIds: [alice.id] });
const aliceNote = await post(alice, { text: 'ok', visibility: 'specified', visibleUserIds: [bob.id], replyId: bobNote.id });

await waitForPushToTl();

const res = await api('/notes/timeline', {}, alice);

assert.strictEqual(res.body.some((note: any) => note.id === aliceNote.id), true);
assert.strictEqual(res.body.find((note: any) => note.id === aliceNote.id).text, 'ok');
});

/* TODO
test.concurrent('自身の visibility: specified なノートへのフォローしていないユーザーからの返信が含まれる', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

const aliceNote = await post(alice, { text: 'hi', visibility: 'specified', visibleUserIds: [bob.id] });
const bobNote = await post(bob, { text: 'ok', visibility: 'specified', visibleUserIds: [alice.id], replyId: aliceNote.id });

await waitForPushToTl();

const res = await api('/notes/timeline', {}, alice);

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), true);
assert.strictEqual(res.body.find((note: any) => note.id === bobNote.id).text, 'ok');
});
*/

// ↑の挙動が理想だけど実装が面倒かも
test.concurrent('自身の visibility: specified なノートへのフォローしていないユーザーからの返信が含まれない', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

const aliceNote = await post(alice, { text: 'hi', visibility: 'specified', visibleUserIds: [bob.id] });
const bobNote = await post(bob, { text: 'ok', visibility: 'specified', visibleUserIds: [alice.id], replyId: aliceNote.id });

await waitForPushToTl();

const res = await api('/notes/timeline', {}, alice);

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), false);
});
});

describe('Local TL', () => {
Expand Down Expand Up @@ -630,7 +731,6 @@ describe('Timelines', () => {
assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), true);
});

/* 未実装
test.concurrent('リスインしているフォローしていないユーザーの visibility: followers なノートが含まれない', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

Expand All @@ -645,23 +745,6 @@ describe('Timelines', () => {

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), false);
});
*/

test.concurrent('リスインしているフォローしていないユーザーの visibility: followers なノートが含まれるが隠される', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

const list = await api('/users/lists/create', { name: 'list' }, alice).then(res => res.body);
await api('/users/lists/push', { listId: list.id, userId: bob.id }, alice);
await sleep(1000);
const bobNote = await post(bob, { text: 'hi', visibility: 'followers' });

await waitForPushToTl();

const res = await api('/notes/user-list-timeline', { listId: list.id }, alice);

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), true);
assert.strictEqual(res.body.find((note: any) => note.id === bobNote.id).text, null);
});

test.concurrent('リスインしているフォローしていないユーザーの他人への返信が含まれない', async () => {
const [alice, bob, carol] = await Promise.all([signup(), signup(), signup()]);
Expand Down Expand Up @@ -778,6 +861,38 @@ describe('Timelines', () => {
assert.strictEqual(res.body.some((note: any) => note.id === bobNote1.id), false);
assert.strictEqual(res.body.some((note: any) => note.id === bobNote2.id), true);
}, 1000 * 10);

test.concurrent('リスインしているユーザーの自身宛ての visibility: specified なノートが含まれる', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

const list = await api('/users/lists/create', { name: 'list' }, alice).then(res => res.body);
await api('/users/lists/push', { listId: list.id, userId: bob.id }, alice);
await sleep(1000);
const bobNote = await post(bob, { text: 'hi', visibility: 'specified', visibleUserIds: [alice.id] });

await waitForPushToTl();

const res = await api('/notes/user-list-timeline', { listId: list.id }, alice);

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), true);
assert.strictEqual(res.body.find((note: any) => note.id === bobNote.id).text, 'hi');
});

test.concurrent('リスインしているユーザーの自身宛てではない visibility: specified なノートが含まれない', async () => {
const [alice, bob, carol] = await Promise.all([signup(), signup(), signup()]);

const list = await api('/users/lists/create', { name: 'list' }, alice).then(res => res.body);
await api('/users/lists/push', { listId: list.id, userId: bob.id }, alice);
await api('/users/lists/push', { listId: list.id, userId: carol.id }, alice);
await sleep(1000);
const bobNote = await post(bob, { text: 'hi', visibility: 'specified', visibleUserIds: [carol.id] });

await waitForPushToTl();

const res = await api('/notes/user-list-timeline', { listId: list.id }, alice);

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), false);
});
});

describe('User TL', () => {
Expand Down Expand Up @@ -951,6 +1066,30 @@ describe('Timelines', () => {
assert.strictEqual(res.body.some((note: any) => note.id === bobNote2.id), true);
assert.strictEqual(res.body.some((note: any) => note.id === bobNote3.id), true);
});

test.concurrent('自身の visibility: specified なノートが含まれる', async () => {
const [alice] = await Promise.all([signup()]);

const aliceNote = await post(alice, { text: 'hi', visibility: 'specified' });

await waitForPushToTl();

const res = await api('/users/notes', { userId: alice.id, withReplies: true }, alice);

assert.strictEqual(res.body.some((note: any) => note.id === aliceNote.id), true);
});

test.concurrent('visibleUserIds に指定されてない visibility: specified なノートが含まれない', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

const bobNote = await post(bob, { text: 'hi', visibility: 'specified' });

await waitForPushToTl();

const res = await api('/users/notes', { userId: bob.id, withReplies: true }, alice);

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), false);
});
});

// TODO: リノートミュート済みユーザーのテスト
Expand Down
3 changes: 3 additions & 0 deletions packages/shared/.eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ module.exports = {
'before': true,
'after': true,
}],
'brace-style': ['error', '1tbs', {
'allowSingleLine': true,
}],
'padded-blocks': ['error', 'never'],
/* TODO: path aliasを使わないとwarnする
'no-restricted-imports': ['warn', {
Expand Down
Loading