Skip to content

Commit

Permalink
自分のフォロワー限定投稿に対するリプライがホームタイムラインで見えないことが有る問題を修正 (#197)
Browse files Browse the repository at this point in the history
* fix: reply to my follower notes are not shown on the home timeline

(cherry picked from commit 28a0511)

* fix: reply to follower note by non-following is on social timeline

(cherry picked from commit 5b6076e)

* docs: changelog

(cherry picked from commit ef5472a)

* test: add endpoint test for changes

* test(e2e): 自分のfollowers投稿に対するリプライが流れる

* test(e2e/streaming): 自分のfollowers投稿に対するリプライが流れる

* test(e2e/streaming): フォローしていないユーザによるフォロワー限定投稿に対するリプライがソーシャルタイムラインで表示されることがある問題

* fix: reply to follower note by non-following is on vmimi social timeline
  • Loading branch information
anatawa12 authored Jun 3, 2024
1 parent b55dd9e commit c075c27
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 7 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
- Fix: フォロー中のユーザーに関する"TLに他の人への返信を含める"の設定が分かりづらい問題を修正
- Fix: ランダム文字列でアップロードする際に拡張子内にドットが含まれるものを正常にアップロードできるように

### Server
- Fix: 自分のフォロワー限定投稿に対するリプライがホームタイムラインで見えないことが有る問題を修正
- Fix: フォローしていないユーザによるフォロワー限定投稿に対するリプライがソーシャルタイムラインで表示されることがある問題を修正

## 2024.5.0 (merged to 2024.5.0-kinel.1)

### Note
Expand Down
13 changes: 13 additions & 0 deletions packages/backend/src/server/api/endpoints/notes/hybrid-timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,12 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
];
}

const [
followings,
] = await Promise.all([
this.cacheService.userFollowingsCache.fetch(me.id),
]);

const redisTimeline = await this.fanoutTimelineEndpointService.timeline({
untilId,
sinceId,
Expand All @@ -152,6 +158,13 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
useDbFallback: serverSettings.enableFanoutTimelineDbFallback,
alwaysIncludeMyNotes: true,
excludePureRenotes: !ps.withRenotes,
noteFilter: note => {
if (note.reply && note.reply.visibility === 'followers') {
if (!Object.hasOwn(followings, note.reply.userId) && note.reply.userId !== me.id) return false;
}

return true;
},
dbFallback: async (untilId, sinceId, limit) => await this.getFromDb({
untilId,
sinceId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
excludePureRenotes: !ps.withRenotes,
noteFilter: note => {
if (note.reply && note.reply.visibility === 'followers') {
if (!Object.hasOwn(followings, note.reply.userId)) return false;
if (!Object.hasOwn(followings, note.reply.userId) && note.reply.userId !== me.id) return false;
}

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { FanoutTimelineEndpointService } from '@/core/FanoutTimelineEndpointServ
import { MiLocalUser } from '@/models/User.js';
import { MetaService } from '@/core/MetaService.js';
import { IdService } from '@/core/IdService.js';
import { CacheService } from '@/core/CacheService.js';
import { UserFollowingService } from '@/core/UserFollowingService.js';
import { FanoutTimelineName } from '@/core/FanoutTimelineService.js';
import { ApiError } from '../../error.js';
Expand Down Expand Up @@ -81,6 +82,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
private roleService: RoleService,
private activeUsersChart: ActiveUsersChart,
private idService: IdService,
private cacheService: CacheService,
private vmimiRelayTimelineService: VmimiRelayTimelineService,
private userFollowingService: UserFollowingService,
private fanoutTimelineEndpointService: FanoutTimelineEndpointService,
Expand Down Expand Up @@ -135,6 +137,12 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
];
}

const [
followings,
] = await Promise.all([
this.cacheService.userFollowingsCache.fetch(me.id),
]);

const redisTimeline = await this.fanoutTimelineEndpointService.timeline({
untilId,
sinceId,
Expand All @@ -145,6 +153,13 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
useDbFallback: serverSettings.enableFanoutTimelineDbFallback,
alwaysIncludeMyNotes: true,
excludePureRenotes: !ps.withRenotes,
noteFilter: note => {
if (note.reply && note.reply.visibility === 'followers') {
if (!Object.hasOwn(followings, note.reply.userId) && note.reply.userId !== me.id) return false;
}

return true;
},
dbFallback: async (untilId, sinceId, limit) => await this.getFromDb({
untilId,
sinceId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class HomeTimelineChannel extends Channel {
const reply = note.reply;
if (this.following[note.userId]?.withReplies) {
// 自分のフォローしていないユーザーの visibility: followers な投稿への返信は弾く
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId)) return;
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId) && reply.userId !== this.user!.id) return;
} else {
// 「チャンネル接続主への返信」でもなければ、「チャンネル接続主が行った返信」でもなければ、「投稿者の投稿者自身への返信」でもない場合
if (reply.userId !== this.user!.id && !isMe && reply.userId !== note.userId) return;
Expand All @@ -72,7 +72,7 @@ class HomeTimelineChannel extends Channel {
if (note.renote.reply) {
const reply = note.renote.reply;
// 自分のフォローしていないユーザーの visibility: followers な投稿への返信のリノートは弾く
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId)) return;
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId) && reply.userId !== this.user!.id) return;
}
}

Expand Down
12 changes: 10 additions & 2 deletions packages/backend/src/server/api/stream/channels/hybrid-timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,22 @@ class HybridTimelineChannel extends Channel {
const reply = note.reply;
if ((this.following[note.userId]?.withReplies ?? false) || this.withReplies) {
// 自分のフォローしていないユーザーの visibility: followers な投稿への返信は弾く
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId)) return;
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId) && reply.userId !== this.user!.id) return;
} else {
// 「チャンネル接続主への返信」でもなければ、「チャンネル接続主が行った返信」でもなければ、「投稿者の投稿者自身への返信」でもない場合
if (reply.userId !== this.user!.id && !isMe && reply.userId !== note.userId) return;
}
}

if (isRenotePacked(note) && !isQuotePacked(note) && !this.withRenotes) return;
// 純粋なリノート(引用リノートでないリノート)の場合
if (isRenotePacked(note) && !isQuotePacked(note) && note.renote) {
if (!this.withRenotes) return;
if (note.renote.reply) {
const reply = note.renote.reply;
// 自分のフォローしていないユーザーの visibility: followers な投稿への返信のリノートは弾く
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId) && reply.userId !== this.user!.id) return;
}
}

if (this.user && note.renoteId && !note.text) {
if (note.renote && Object.keys(note.renote.reactions).length > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,22 @@ class VmimiRelayHybridTimelineChannel extends Channel {
const reply = note.reply;
if ((this.following[note.userId]?.withReplies ?? false) || this.withReplies) {
// 自分のフォローしていないユーザーの visibility: followers な投稿への返信は弾く
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId)) return;
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId) && reply.userId !== this.user!.id) return;
} else {
// 「チャンネル接続主への返信」でもなければ、「チャンネル接続主が行った返信」でもなければ、「投稿者の投稿者自身への返信」でもない場合
if (reply.userId !== this.user!.id && !isMe && reply.userId !== note.userId) return;
}
}

if (isRenotePacked(note) && !isQuotePacked(note) && !this.withRenotes) return;
// 純粋なリノート(引用リノートでないリノート)の場合
if (isRenotePacked(note) && !isQuotePacked(note) && note.renote) {
if (!this.withRenotes) return;
if (note.renote.reply) {
const reply = note.renote.reply;
// 自分のフォローしていないユーザーの visibility: followers な投稿への返信のリノートは弾く
if (reply.visibility === 'followers' && !Object.hasOwn(this.following, reply.userId) && reply.userId !== this.user!.id) return;
}
}

if (this.user && note.renoteId && !note.text) {
if (note.renote && Object.keys(note.renote.reactions).length > 0) {
Expand Down
62 changes: 62 additions & 0 deletions packages/backend/test/e2e/streaming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('Streaming', () => {
let kyoko: misskey.entities.SignupResponse;
let chitose: misskey.entities.SignupResponse;
let kanako: misskey.entities.SignupResponse;
let erin: misskey.entities.SignupResponse;

// Remote users
let akari: misskey.entities.SignupResponse;
Expand All @@ -53,6 +54,7 @@ describe('Streaming', () => {
kyoko = await signup({ username: 'kyoko' });
chitose = await signup({ username: 'chitose' });
kanako = await signup({ username: 'kanako' });
erin = await signup({ username: 'erin' }); // erin: A generic fifth participant

akari = await signup({ username: 'akari', host: 'example.com' });
chinatsu = await signup({ username: 'chinatsu', host: 'example.com' });
Expand All @@ -71,6 +73,12 @@ describe('Streaming', () => {
// Follow: kyoko => chitose
await api('following/create', { userId: chitose.id }, kyoko);

// Follow: erin <=> ayano each other.
// erin => ayano: withReplies: true
await api('following/create', { userId: ayano.id, withReplies: true }, erin);
// ayano => erin: withReplies: false
await api('following/create', { userId: erin.id, withReplies: false }, ayano);

// Mute: chitose => kanako
await api('mute/create', { userId: kanako.id }, chitose);

Expand Down Expand Up @@ -297,6 +305,28 @@ describe('Streaming', () => {

assert.strictEqual(fired, true);
});

test('withReplies: true のとき自分のfollowers投稿に対するリプライが流れる', async () => {
const erinNote = await post(erin, { text: 'hi', visibility: 'followers' });
const fired = await waitFire(
erin, 'homeTimeline', // erin:home
() => api('notes/create', { text: 'hello', replyId: erinNote.id }, ayano), // ayano reply to erin's followers post
msg => msg.type === 'note' && msg.body.userId === ayano.id, // wait ayano
);

assert.strictEqual(fired, true);
});

test('withReplies: false でも自分の投稿に対するリプライが流れる', async () => {
const ayanoNote = await post(ayano, { text: 'hi', visibility: 'followers' });
const fired = await waitFire(
ayano, 'homeTimeline', // ayano:home
() => api('notes/create', { text: 'hello', replyId: ayanoNote.id }, erin), // erin reply to ayano's followers post
msg => msg.type === 'note' && msg.body.userId === erin.id, // wait erin
);

assert.strictEqual(fired, true);
});
}); // Home

describe('Local Timeline', () => {
Expand Down Expand Up @@ -475,6 +505,38 @@ describe('Streaming', () => {

assert.strictEqual(fired, false);
});

test('withReplies: true のとき自分のfollowers投稿に対するリプライが流れる', async () => {
const erinNote = await post(erin, { text: 'hi', visibility: 'followers' });
const fired = await waitFire(
erin, 'homeTimeline', // erin:home
() => api('notes/create', { text: 'hello', replyId: erinNote.id }, ayano), // ayano reply to erin's followers post
msg => msg.type === 'note' && msg.body.userId === ayano.id, // wait ayano
);

assert.strictEqual(fired, true);
});

test('withReplies: false でも自分の投稿に対するリプライが流れる', async () => {
const ayanoNote = await post(ayano, { text: 'hi', visibility: 'followers' });
const fired = await waitFire(
ayano, 'homeTimeline', // ayano:home
() => api('notes/create', { text: 'hello', replyId: ayanoNote.id }, erin), // erin reply to ayano's followers post
msg => msg.type === 'note' && msg.body.userId === erin.id, // wait erin
);

assert.strictEqual(fired, true);
});

test('withReplies: true のフォローしていない人のfollowersノートに対するリプライが流れない', async () => {
const fired = await waitFire(
erin, 'homeTimeline', // erin:home
() => api('notes/create', { text: 'hello', replyId: chitose.id }, ayano), // ayano reply to chitose's post
msg => msg.type === 'note' && msg.body.userId === ayano.id, // wait ayano
);

assert.strictEqual(fired, false);
});
});

describe('Global Timeline', () => {
Expand Down
75 changes: 75 additions & 0 deletions packages/backend/test/e2e/timelines.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ describe('Timelines', () => {
test.concurrent('withReplies: true でフォローしているユーザーの他人の visibility: followers な投稿への返信が含まれない', async () => {
const [alice, bob, carol] = await Promise.all([signup(), signup(), signup()]);

await api('following/create', { userId: carol.id }, bob);
await api('following/create', { userId: bob.id }, alice);
await api('following/update', { userId: bob.id, withReplies: true }, alice);
await sleep(1000);
Expand Down Expand Up @@ -152,6 +153,24 @@ describe('Timelines', () => {
assert.strictEqual(res.body.find((note: any) => note.id === carolNote.id).text, 'hi');
});

test.concurrent('withReplies: true でフォローしているユーザーの自分の visibility: followers な投稿への返信が含まれる', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

await api('following/create', { userId: bob.id }, alice);
await api('following/create', { userId: alice.id }, bob);
await api('following/update', { userId: bob.id, withReplies: true }, alice);
await sleep(1000);
const aliceNote = await post(alice, { text: 'hi', visibility: 'followers' });
const bobNote = await post(bob, { text: 'hi', replyId: aliceNote.id });

await waitForPushToTl();

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

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

test.concurrent('withReplies: true でフォローしているユーザーの行った別のフォローしているユーザーの投稿への visibility: specified な返信が含まれない', async () => {
const [alice, bob, carol] = await Promise.all([signup(), signup(), signup()]);

Expand Down Expand Up @@ -721,6 +740,62 @@ describe('Timelines', () => {
assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), true);
});

test.concurrent('withReplies: true でフォローしているユーザーの他人の visibility: followers な投稿への返信が含まれない', async () => {
const [alice, bob, carol] = await Promise.all([signup(), signup(), signup()]);

await api('following/create', { userId: carol.id }, bob);
await api('following/create', { userId: bob.id }, alice);
await api('following/update', { userId: bob.id, withReplies: true }, alice);
await sleep(1000);
const carolNote = await post(carol, { text: 'hi', visibility: 'followers' });
const bobNote = await post(bob, { text: 'hi', replyId: carolNote.id });

await waitForPushToTl();

const res = await api('notes/hybrid-timeline', { limit: 100 }, alice);

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

test.concurrent('withReplies: true でフォローしているユーザーの行った別のフォローしているユーザーの visibility: followers な投稿への返信が含まれる', async () => {
const [alice, bob, carol] = await Promise.all([signup(), signup(), signup()]);

await api('following/create', { userId: bob.id }, alice);
await api('following/create', { userId: carol.id }, alice);
await api('following/create', { userId: carol.id }, bob);
await api('following/update', { userId: bob.id, withReplies: true }, alice);
await sleep(1000);
const carolNote = await post(carol, { text: 'hi', visibility: 'followers' });
const bobNote = await post(bob, { text: 'hi', replyId: carolNote.id });

await waitForPushToTl();

const res = await api('notes/hybrid-timeline', { limit: 100 }, alice);

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

test.concurrent('withReplies: true でフォローしているユーザーの自分の visibility: followers な投稿への返信が含まれる', async () => {
const [alice, bob] = await Promise.all([signup(), signup()]);

await api('following/create', { userId: bob.id }, alice);
await api('following/create', { userId: alice.id }, bob);
await api('following/update', { userId: bob.id, withReplies: true }, alice);
await sleep(1000);
const aliceNote = await post(alice, { text: 'hi', visibility: 'followers' });
const bobNote = await post(bob, { text: 'hi', replyId: aliceNote.id });

await waitForPushToTl();

const res = await api('notes/hybrid-timeline', { limit: 100 }, alice);

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

test.concurrent('他人の他人への返信が含まれない', async () => {
const [alice, bob, carol] = await Promise.all([signup(), signup(), signup()]);

Expand Down

0 comments on commit c075c27

Please sign in to comment.