follow-up fixes to NoteVisibilityService changes

This commit is contained in:
Hazelnoot 2025-09-18 19:27:36 -04:00
parent 4b57d7d6dd
commit 4bab56609f
3 changed files with 107 additions and 163 deletions

View file

@ -19,7 +19,7 @@ import { isQuote, isRenote } from '@/misc/is-renote.js';
import { CacheService } from '@/core/CacheService.js';
import { isReply } from '@/misc/is-reply.js';
import { isInstanceMuted } from '@/misc/is-instance-muted.js';
import { NoteVisibilityService, PopulatedNote } from '@/core/NoteVisibilityService.js';
import { NotePopulationData, NoteVisibilityService, PopulatedNote } from '@/core/NoteVisibilityService.js';
import { FederatedInstanceService } from '@/core/FederatedInstanceService.js';
type TimelineOptions = {
@ -85,29 +85,29 @@ export class FanoutTimelineEndpointService {
const shouldFallbackToDb = noteIds.length === 0 || ps.sinceId != null && ps.sinceId < oldestNoteId;
if (!shouldFallbackToDb) {
let filter = ps.noteFilter ?? (_note => true);
let filter: (note: MiNote, populated: PopulatedNote) => boolean = ps.noteFilter ?? (() => true);
if (ps.excludeNoFiles) {
const parentFilter = filter;
filter = (note) => note.fileIds.length !== 0 && parentFilter(note);
filter = (note, populated) => note.fileIds.length !== 0 && parentFilter(note, populated);
}
if (ps.excludeReplies) {
const parentFilter = filter;
filter = (note) => {
filter = (note, populated) => {
if (note.userId !== ps.me?.id && isReply(note, ps.me?.id)) return false;
return parentFilter(note);
return parentFilter(note, populated);
};
}
if (ps.excludeBots) {
const parentFilter = filter;
filter = (note) => !note.user?.isBot && parentFilter(note);
filter = (note, populated) => !note.user?.isBot && parentFilter(note, populated);
}
if (ps.excludePureRenotes) {
const parentFilter = filter;
filter = (note) => (!isRenote(note) || isQuote(note)) && parentFilter(note);
filter = (note, populated) => (!isRenote(note) || isQuote(note)) && parentFilter(note, populated);
}
{
@ -115,37 +115,37 @@ export class FanoutTimelineEndpointService {
const data = await this.noteVisibilityService.populateData(me);
const parentFilter = filter;
filter = (note) => {
const { accessible, silence } = this.noteVisibilityService.checkNoteVisibility(note as PopulatedNote, me, { data, filters: { includeSilencedAuthor: ps.ignoreAuthorFromUserSilence } });
filter = (note, populated) => {
const { accessible, silence } = this.noteVisibilityService.checkNoteVisibility(populated, me, { data, filters: { includeSilencedAuthor: ps.ignoreAuthorFromUserSilence } });
if (!accessible || silence) return false;
return parentFilter(note);
return parentFilter(note, populated);
};
}
{
const parentFilter = filter;
filter = (note) => {
filter = (note, populated) => {
if (!ps.ignoreAuthorFromInstanceBlock) {
if (note.user?.instance?.isBlocked) return false;
}
if (note.userId !== note.renoteUserId && note.renote?.user?.instance?.isBlocked) return false;
if (note.userId !== note.replyUserId && note.reply?.user?.instance?.isBlocked) return false;
return parentFilter(note);
return parentFilter(note, populated);
};
}
{
const parentFilter = filter;
filter = (note) => {
filter = (note, populated) => {
if (!ps.ignoreAuthorFromUserSuspension) {
if (note.user?.isSuspended) return false;
}
if (note.userId !== note.renoteUserId && note.renote?.user?.isSuspended) return false;
if (note.userId !== note.replyUserId && note.reply?.user?.isSuspended) return false;
return parentFilter(note);
return parentFilter(note, populated);
};
}
@ -190,7 +190,7 @@ export class FanoutTimelineEndpointService {
return await ps.dbFallback(ps.untilId, ps.sinceId, ps.limit);
}
private async getAndFilterFromDb(noteIds: string[], noteFilter: (note: MiNote) => boolean, idCompare: (a: string, b: string) => number): Promise<MiNote[]> {
private async getAndFilterFromDb(noteIds: string[], noteFilter: (note: MiNote, populated: PopulatedNote) => boolean, idCompare: (a: string, b: string) => number): Promise<MiNote[]> {
const query = this.notesRepository.createQueryBuilder('note')
.where('note.id IN (:...noteIds)', { noteIds: noteIds })
.leftJoinAndSelect('note.reply', 'reply')
@ -198,52 +198,78 @@ export class FanoutTimelineEndpointService {
.leftJoinAndSelect('note.channel', 'channel')
// Needed for populated note
.leftJoinAndSelect('renote.renote', 'renoteRenote')
.leftJoinAndSelect('renote.reply', 'renoteReply')
;
const notes = await query.getMany();
// Manually populate user/instance since it's cacheable and avoids many joins.
// These fields *must* be populated or NoteVisibilityService won't work right!
await this.populateUsers(notes);
return notes
.filter(noteFilter)
.sort((a, b) => idCompare(a.id, b.id));
const populatedNotes = await this.populateNotes(notes);
return populatedNotes
.filter(({ note, populated }) => noteFilter(note, populated))
.sort((a, b) => idCompare(a.id, b.id))
.map(({ note }) => note);
}
private async populateUsers(notes: MiNote[]): Promise<void> {
// Enumerate users and instances
/**
* Given a sample of notes to return, populates the relations from cache and generates a NotePopulationData hint object.
* This is messy and kinda gross, but it allows us to use the synchronous checkNoteVisibility from within the filter callbacks.
*/
private async populateNotes(notes: MiNote[]): Promise<{ id: string, note: MiNote, populated: PopulatedNote }[]> {
// Manually populate user/instance since it's cacheable and avoids many joins.
// These fields *must* be populated or NoteVisibilityService won't work right!
const populationData = await this.populateUsers(notes);
// This is async, but it should never await because we populate above.
return await Promise.all(notes.map(async note => ({
id: note.id,
note: note,
populated: await this.noteVisibilityService.populateNote(note, populationData),
})));
}
/**
* This does two things:
* 1. Populates the user/instance relations of every note in the object graph.
* 2. Returns fetched note/user/instance maps for use as hint data for NoteVisibilityService.
*/
private async populateUsers(notes: MiNote[]): Promise<NotePopulationData> {
// Enumerate all related data
const allNotes = new Map<string, MiNote>();
const usersToFetch = new Set<string>();
const instancesToFetch = new Set<string>();
for (const note of notes) {
// note
allNotes.set(note.id, note);
usersToFetch.add(note.userId);
if (note.userHost) {
instancesToFetch.add(note.userHost);
}
// note.reply
if (note.reply) {
allNotes.set(note.reply.id, note.reply);
usersToFetch.add(note.reply.userId);
if (note.reply.userHost) {
instancesToFetch.add(note.reply.userHost);
}
}
// note.renote
if (note.renote) {
allNotes.set(note.renote.id, note.renote);
usersToFetch.add(note.renote.userId);
if (note.renote.userHost) {
instancesToFetch.add(note.renote.userHost);
}
if (note.renote.reply) {
usersToFetch.add(note.renote.reply.userId);
if (note.renote.reply.userHost) {
instancesToFetch.add(note.renote.reply.userHost);
}
}
if (note.renote.renote) {
usersToFetch.add(note.renote.renote.userId);
if (note.renote.renote.userHost) {
instancesToFetch.add(note.renote.renote.userHost);
}
}
// note.renote.reply
if (note.renote?.reply) {
allNotes.set(note.renote.reply.id, note.renote.reply);
usersToFetch.add(note.renote.reply.userId);
if (note.renote.reply.userHost) {
instancesToFetch.add(note.renote.reply.userHost);
}
}
}
@ -271,10 +297,10 @@ export class FanoutTimelineEndpointService {
if (note.renote.reply) {
note.renote.reply.user = users.get(note.renote.reply.userId) ?? null;
}
if (note.renote.renote) {
note.renote.renote.user = users.get(note.renote.renote.userId) ?? null;
}
}
}
// Optimization: return our accumulated data to avoid duplicate lookups later
return { users, instances, notes: allNotes };
}
}

View file

@ -8,11 +8,11 @@ import { CacheService } from '@/core/CacheService.js';
import type { MiNote } from '@/models/Note.js';
import type { MiUser } from '@/models/User.js';
import { bindThis } from '@/decorators.js';
import { Packed } from '@/misc/json-schema.js';
import type { Packed } from '@/misc/json-schema.js';
import { IdService } from '@/core/IdService.js';
import { awaitAll } from '@/misc/prelude/await-all.js';
import { FederatedInstanceService } from '@/core/FederatedInstanceService.js';
import type { MiFollowing, NotesRepository } from '@/models/_.js';
import type { MiFollowing, MiInstance, NotesRepository } from '@/models/_.js';
import { DI } from '@/di-symbols.js';
/**
@ -69,15 +69,15 @@ export class NoteVisibilityService {
user = await this.cacheService.findUserById(user);
}
const populatedNote = await this.populateNote(note);
const populatedNote = await this.populateNote(note, opts?.hint);
const populatedData = await this.populateData(user, opts?.hint ?? {});
return this.checkNoteVisibility(populatedNote, user, { filters: opts?.filters, data: populatedData });
}
// TODO pass in notes hint
private async populateNote(note: MiNote | Packed<'Note'>, diveReply = true, diveRenote = true): Promise<PopulatedNote> {
const userPromise = this.getNoteUser(note);
@bindThis
public async populateNote(note: MiNote | Packed<'Note'>, hint?: NotePopulationData, diveReply = true, diveRenote = true): Promise<PopulatedNote> {
const userPromise = this.getNoteUser(note, hint);
// noinspection ES6MissingAwait
return await awaitAll({
@ -90,9 +90,9 @@ export class NoteVisibilityService {
userHost: userPromise.then(u => u.host),
user: userPromise,
renoteId: note.renoteId ?? null,
renote: diveRenote ? this.getNoteRenote(note) : null,
renote: diveRenote ? this.getNoteRenote(note, hint) : null,
replyId: note.replyId ?? null,
reply: diveReply ? this.getNoteReply(note) : null,
reply: diveReply ? this.getNoteReply(note, hint) : null,
hasPoll: 'hasPoll' in note ? note.hasPoll : (note.poll != null),
mentions: note.mentions ?? [],
visibleUserIds: note.visibleUserIds ?? [],
@ -103,9 +103,18 @@ export class NoteVisibilityService {
});
}
private async getNoteUser(note: MiNote | Packed<'Note'>): Promise<PopulatedUser> {
const user = note.user ?? await this.cacheService.findUserById(note.userId);
const instance = user.instance ?? (user.host ? await this.federatedInstanceService.fetchOrRegister(user.host) : null);
private async getNoteUser(note: MiNote | Packed<'Note'>, hint?: NotePopulationData): Promise<PopulatedUser> {
const user = note.user
?? hint?.users?.get(note.userId)
?? await this.cacheService.findUserById(note.userId);
const instance = user.host
? (
user.instance
?? hint?.instances?.get(user.host)
?? await this.federatedInstanceService.fetchOrRegister(user.host)
) : null;
return {
...user,
makeNotesHiddenBefore: user.makeNotesHiddenBefore ?? null,
@ -118,23 +127,27 @@ export class NoteVisibilityService {
};
}
private async getNoteRenote(note: MiNote | Packed<'Note'>): Promise<PopulatedNote | null> {
private async getNoteRenote(note: MiNote | Packed<'Note'>, hint?: NotePopulationData): Promise<PopulatedNote | null> {
if (!note.renoteId) return null;
const renote = note.renote ?? await this.notesRepository.findOneByOrFail({ id: note.renoteId });
const renote = note.renote
?? hint?.notes?.get(note.renoteId)
?? await this.notesRepository.findOneByOrFail({ id: note.renoteId });
// Renote needs to include the reply!
// This will dive one more time before landing in getNoteReply, which terminates recursion.
// Based on the logic in NoteEntityService.pack()
return await this.populateNote(renote, true, false);
return await this.populateNote(renote, hint, true, false);
}
private async getNoteReply(note: MiNote | Packed<'Note'>): Promise<PopulatedNote | null> {
private async getNoteReply(note: MiNote | Packed<'Note'>, hint?: NotePopulationData): Promise<PopulatedNote | null> {
if (!note.replyId) return null;
const reply = note.reply ?? await this.notesRepository.findOneByOrFail({ id: note.replyId });
const reply = note.reply
?? hint?.notes?.get(note.replyId)
?? await this.notesRepository.findOneByOrFail({ id: note.replyId });
return await this.populateNote(reply, false, false);
return await this.populateNote(reply, hint, false, false);
}
@bindThis
@ -261,11 +274,11 @@ export class NoteVisibilityService {
// Based on NoteEntityService.treatVisibility
@bindThis
public syncVisibility(note: PopulatedNote): void {
public syncVisibility(note: PopulatedNote | Packed<'Note'>): void {
// Make followers-only
if (note.user.makeNotesFollowersOnlyBefore && note.visibility !== 'specified' && note.visibility !== 'followers') {
const followersOnlyBefore = note.user.makeNotesFollowersOnlyBefore * 1000;
const createdAt = note.createdAt.valueOf();
const createdAt = new Date(note.createdAt).valueOf();
// I don't understand this logic, but I tried to break it out for readability
const followersOnlyOpt1 = followersOnlyBefore <= 0 && (Date.now() - createdAt > 0 - followersOnlyBefore);
@ -388,7 +401,7 @@ export class NoteVisibilityService {
}
}
export interface NoteVisibilityData {
export interface NoteVisibilityData extends NotePopulationData {
userBlockers: Set<string> | null;
userFollowings: Map<string, Omit<MiFollowing, 'isFollowerHibernated'>> | null;
userMutedThreads: Set<string> | null;
@ -398,21 +411,16 @@ export interface NoteVisibilityData {
userMutedInstances: Set<string> | null;
}
export interface NotePopulationData {
notes?: Map<string, MiNote>;
users?: Map<string, MiUser>;
instances?: Map<string, MiInstance>;
}
// This represents the *requesting* user!
export type PopulatedMe = Pick<MiUser, 'id' | 'host'> | null | undefined;
// type PopulatedNote = Flatten<PopulatedObjectNote>;
// type PopulatedNote = Flatten<Packed<'Note'>, MiNote> & {
// user: PopulatedUser,
// renote?: PopulatedNote | null,
// reply?: PopulatedNote | null,
// };
// type PopulatedUser = Flatten<Packed<'UserLite'>, MiUser> & {
// instance?: PopulatedInstance | null,
// };
// type PopulatedInstance = Flatten<Packed<'UserLite'>['instance'], MiInstance>;
interface PopulatedNote {
export interface PopulatedNote {
id: string;
threadId: string;
userId: string;

View file

@ -19,6 +19,7 @@ import { ReactionsBufferingService } from '@/core/ReactionsBufferingService.js';
import { QueryService } from '@/core/QueryService.js';
import type { Config } from '@/config.js';
import { NoteVisibilityService } from '@/core/NoteVisibilityService.js';
import type { PopulatedNote } from '@/core/NoteVisibilityService.js';
import type { NoteVisibilityData } from '@/core/NoteVisibilityService.js';
import type { OnModuleInit } from '@nestjs/common';
import type { CacheService } from '../CacheService.js';
@ -150,7 +151,7 @@ export class NoteEntityService implements OnModuleInit {
const data = await this.noteVisibilityService.populateData(me, hint);
for (const note of notes) {
this.hideNote(note, me, data);
await this.hideNoteAsync(note, me, data);
}
}
@ -163,97 +164,6 @@ export class NoteEntityService implements OnModuleInit {
}
}
@bindThis
public hideNote(packedNote: Packed<'Note'>, me: Pick<MiUser, 'id' | 'host'> | null, data: NoteVisibilityData): void {
// Implementation moved to NoteVisibilityService
/*
if (meId === packedNote.userId) return;
// TODO: isVisibleForMe を使うようにしても良さそう(型違うけど)
let hide = false;
if (packedNote.user.requireSigninToViewContents && meId == null) {
hide = true;
}
if (!hide) {
const hiddenBefore = packedNote.user.makeNotesHiddenBefore;
if ((hiddenBefore != null)
&& (
(hiddenBefore <= 0 && (Date.now() - new Date(packedNote.createdAt).getTime() > 0 - (hiddenBefore * 1000)))
|| (hiddenBefore > 0 && (new Date(packedNote.createdAt).getTime() < hiddenBefore * 1000))
)
) {
hide = true;
}
}
// visibility が specified かつ自分が指定されていなかったら非表示
if (!hide) {
if (packedNote.visibility === 'specified') {
if (meId == null) {
hide = true;
} else if (meId === packedNote.userId) {
hide = false;
} else {
// 指定されているかどうか
const specified = packedNote.visibleUserIds!.some(id => meId === id);
if (!specified) {
hide = true;
}
}
}
}
// visibility が followers かつ自分が投稿者のフォロワーでなかったら非表示
if (!hide) {
if (packedNote.visibility === 'followers') {
if (meId == null) {
hide = true;
} else if (meId === packedNote.userId) {
hide = false;
} else if (packedNote.reply && (meId === packedNote.reply.userId)) {
// 自分の投稿に対するリプライ
hide = false;
} else if (packedNote.mentions && packedNote.mentions.some(id => meId === id)) {
// 自分へのメンション
hide = false;
} else if (packedNote.renote && (meId === packedNote.renote.userId)) {
hide = false;
} else {
const isFollowing = hint?.myFollowing
? hint.myFollowing.has(packedNote.userId)
: (await this.cacheService.userFollowingsCache.fetch(meId)).has(packedNote.userId);
hide = !isFollowing;
}
}
}
// If this is a pure renote (boost), then we should *also* check the boosted note's visibility.
// Otherwise we can have empty notes on the timeline, which is not good.
// Notes are packed in depth-first order, so we can safely grab the "isHidden" property to avoid duplicated checks.
// This is pulled out to ensure that we check both the renote *and* the boosted note.
if (packedNote.renote?.isHidden && isPackedPureRenote(packedNote)) {
hide = true;
}
if (!hide && meId && packedNote.userId !== meId) {
const blockers = hint?.myBlockers ?? await this.cacheService.userBlockedCache.fetch(meId);
const isBlocked = blockers.has(packedNote.userId);
if (isBlocked) hide = true;
}
*/
const hide = this.noteVisibilityService.checkNoteVisibility(packedNote, me, { data }).redact;
if (hide) {
this.redactNoteContents(packedNote);
}
}
private redactNoteContents(packedNote: Packed<'Note'>) {
{
packedNote.visibleUserIds = undefined;