[backend] Use a recursion limiter for user profile mentions instead of disabling recursion altogether

This commit is contained in:
Laura Hausmann 2023-10-25 13:16:54 +02:00
parent 9b13ec9c0c
commit 2575588fa3
No known key found for this signature in database
GPG Key ID: D044E84C5BE01605
5 changed files with 47 additions and 24 deletions

View File

@ -6,13 +6,13 @@ import { resolveMentionToUserAndProfile } from "@/remote/resolve-user.js";
import { IMentionedRemoteUsers } from "@/models/entities/note.js"; import { IMentionedRemoteUsers } from "@/models/entities/note.js";
import { unique } from "@/prelude/array.js"; import { unique } from "@/prelude/array.js";
import config from "@/config/index.js"; import config from "@/config/index.js";
import { Semaphore } from "async-mutex"; import { Mutex, Semaphore } from "async-mutex";
const queue = new Semaphore(5); const queue = new Semaphore(5);
export const UserProfileRepository = db.getRepository(UserProfile).extend({ export const UserProfileRepository = db.getRepository(UserProfile).extend({
// We must never await this without promiseEarlyReturn, otherwise giant webring-style profile mention trees will cause the queue to stop working // We must never await this without promiseEarlyReturn, otherwise giant webring-style profile mention trees will cause the queue to stop working
async updateMentions(id: UserProfile["userId"]){ async updateMentions(id: UserProfile["userId"], limiter: RecursionLimiter = new RecursionLimiter(20)){
const profile = await this.findOneBy({ userId: id }); const profile = await this.findOneBy({ userId: id });
if (!profile) return; if (!profile) return;
const tokens: mfm.MfmNode[] = []; const tokens: mfm.MfmNode[] = [];
@ -24,16 +24,16 @@ export const UserProfileRepository = db.getRepository(UserProfile).extend({
return queue.runExclusive(async () => { return queue.runExclusive(async () => {
const partial = { const partial = {
mentions: await populateMentions(tokens, profile.userHost) mentions: await populateMentions(tokens, profile.userHost, limiter)
}; };
return UserProfileRepository.update(profile.userId, partial); return UserProfileRepository.update(profile.userId, partial);
}); });
}, },
}); });
async function populateMentions(tokens: mfm.MfmNode[], objectHost: string | null): Promise<IMentionedRemoteUsers> { async function populateMentions(tokens: mfm.MfmNode[], objectHost: string | null, limiter: RecursionLimiter): Promise<IMentionedRemoteUsers> {
const mentions = extractMentions(tokens); const mentions = extractMentions(tokens);
const resolved = await Promise.all(mentions.map(m => resolveMentionToUserAndProfile(m.username, m.host, objectHost))); const resolved = await Promise.all(mentions.map(m => resolveMentionToUserAndProfile(m.username, m.host, objectHost, limiter)));
const remote = resolved.filter(p => p && p.data.host !== config.domain && (p.data.host !== null || objectHost !== null)) const remote = resolved.filter(p => p && p.data.host !== config.domain && (p.data.host !== null || objectHost !== null))
.map(p => p!); .map(p => p!);
const res = remote.map(m => { const res = remote.map(m => {
@ -47,3 +47,17 @@ async function populateMentions(tokens: mfm.MfmNode[], objectHost: string | null
return unique(res); return unique(res);
} }
export class RecursionLimiter {
private counter;
private mutex = new Mutex();
constructor(count: number = 20) {
this.counter = count;
}
public shouldContinue(): Promise<boolean> {
return this.mutex.runExclusive(() => {
return this.counter-- > 0;
});
}
}

View File

@ -6,9 +6,11 @@ import type { IObject, IApMention } from "../type.js";
import { isMention } from "../type.js"; import { isMention } from "../type.js";
import Resolver from "../resolver.js"; import Resolver from "../resolver.js";
import { resolvePerson } from "./person.js"; import { resolvePerson } from "./person.js";
import { RecursionLimiter } from "@/models/repositories/user-profile.js";
export async function extractApMentions( export async function extractApMentions(
tags: IObject | IObject[] | null | undefined, tags: IObject | IObject[] | null | undefined,
limiter: RecursionLimiter = new RecursionLimiter(20)
) { ) {
const hrefs = unique( const hrefs = unique(
extractApMentionObjects(tags).map((x) => x.href as string), extractApMentionObjects(tags).map((x) => x.href as string),
@ -20,7 +22,7 @@ export async function extractApMentions(
const mentionedUsers = ( const mentionedUsers = (
await Promise.all( await Promise.all(
hrefs.map((x) => hrefs.map((x) =>
limit(() => resolvePerson(x, resolver).catch(() => null)), limit(() => resolvePerson(x, resolver, limiter).catch(() => null)),
), ),
) )
).filter((x): x is CacheableUser => x != null); ).filter((x): x is CacheableUser => x != null);

View File

@ -53,6 +53,7 @@ import { DB_MAX_IMAGE_COMMENT_LENGTH } from "@/misc/hard-limits.js";
import { truncate } from "@/misc/truncate.js"; import { truncate } from "@/misc/truncate.js";
import { type Size, getEmojiSize } from "@/misc/emoji-meta.js"; import { type Size, getEmojiSize } from "@/misc/emoji-meta.js";
import { fetchMeta } from "@/misc/fetch-meta.js"; import { fetchMeta } from "@/misc/fetch-meta.js";
import { RecursionLimiter } from "@/models/repositories/user-profile.js";
const logger = apLogger; const logger = apLogger;
@ -108,6 +109,7 @@ export async function createNote(
value: string | IObject, value: string | IObject,
resolver?: Resolver, resolver?: Resolver,
silent = false, silent = false,
limiter: RecursionLimiter = new RecursionLimiter(20)
): Promise<Note | null> { ): Promise<Note | null> {
if (resolver == null) resolver = new Resolver(); if (resolver == null) resolver = new Resolver();
@ -163,6 +165,7 @@ export async function createNote(
const actor = (await resolvePerson( const actor = (await resolvePerson(
getOneApId(note.attributedTo), getOneApId(note.attributedTo),
resolver, resolver,
limiter
)) as CacheableRemoteUser; )) as CacheableRemoteUser;
// Skip if author is suspended. // Skip if author is suspended.
@ -188,8 +191,8 @@ export async function createNote(
let isTalk = note._misskey_talk && visibility === "specified"; let isTalk = note._misskey_talk && visibility === "specified";
const apMentions = await extractApMentions(note.tag); const apMentions = await extractApMentions(note.tag, limiter);
const apHashtags = await extractApHashtags(note.tag); const apHashtags = extractApHashtags(note.tag);
// Attachments // Attachments
// TODO: attachmentは必ずしもImageではない // TODO: attachmentは必ずしもImageではない
@ -216,7 +219,7 @@ export async function createNote(
// Reply // Reply
const reply: Note | null = note.inReplyTo const reply: Note | null = note.inReplyTo
? await resolveNote(note.inReplyTo, resolver) ? await resolveNote(note.inReplyTo, resolver, limiter)
.then((x) => { .then((x) => {
if (x == null) { if (x == null) {
logger.warn("Specified inReplyTo, but nout found"); logger.warn("Specified inReplyTo, but nout found");
@ -262,7 +265,7 @@ export async function createNote(
if (typeof uri !== "string" || !uri.match(/^https?:/)) if (typeof uri !== "string" || !uri.match(/^https?:/))
return { status: "permerror" }; return { status: "permerror" };
try { try {
const res = await resolveNote(uri); const res = await resolveNote(uri, undefined, limiter);
if (res) { if (res) {
return { return {
status: "ok", status: "ok",
@ -403,6 +406,7 @@ export async function createNote(
export async function resolveNote( export async function resolveNote(
value: string | IObject, value: string | IObject,
resolver?: Resolver, resolver?: Resolver,
limiter: RecursionLimiter = new RecursionLimiter(20)
): Promise<Note | null> { ): Promise<Note | null> {
const uri = typeof value === "string" ? value : value.id; const uri = typeof value === "string" ? value : value.id;
if (uri == null) throw new Error("missing uri"); if (uri == null) throw new Error("missing uri");
@ -437,7 +441,7 @@ export async function resolveNote(
// Fetch from remote server and register // Fetch from remote server and register
// If the attached `Note` Object is specified here instead of the uri, the note will be generated without going through the server fetch. // If the attached `Note` Object is specified here instead of the uri, the note will be generated without going through the server fetch.
// Since the attached Note Object may be disguised, always specify the uri and fetch it from the server. // Since the attached Note Object may be disguised, always specify the uri and fetch it from the server.
return await createNote(uri, resolver, true); return await createNote(uri, resolver, true, limiter);
} finally { } finally {
unlock(); unlock();
} }

View File

@ -53,6 +53,7 @@ import {
getSubjectHostFromRemoteUser, getSubjectHostFromRemoteUser,
getSubjectHostFromAcctParts getSubjectHostFromAcctParts
} from "@/remote/resolve-user.js" } from "@/remote/resolve-user.js"
import { RecursionLimiter } from "@/models/repositories/user-profile.js";
const logger = apLogger; const logger = apLogger;
@ -170,7 +171,7 @@ export async function createPerson(
uri: string, uri: string,
resolver?: Resolver, resolver?: Resolver,
subjectHost?: string, subjectHost?: string,
skipMentions: boolean = false limiter: RecursionLimiter = new RecursionLimiter(20)
): Promise<User> { ): Promise<User> {
if (typeof uri !== "string") throw new Error("uri is not string"); if (typeof uri !== "string") throw new Error("uri is not string");
@ -400,7 +401,7 @@ export async function createPerson(
updateUsertags(user!, tags); updateUsertags(user!, tags);
// Mentions update // Mentions update
if (!skipMentions) UserProfiles.updateMentions(user!.id); if (await limiter.shouldContinue()) UserProfiles.updateMentions(user!.id, limiter);
//#region Fetch avatar and header image //#region Fetch avatar and header image
const [avatar, banner] = await Promise.all( const [avatar, banner] = await Promise.all(
@ -436,7 +437,7 @@ export async function createPerson(
}); });
//#endregion //#endregion
await updateFeatured(user!.id, resolver).catch((err) => logger.error(err)); await updateFeatured(user!.id, resolver, limiter).catch((err) => logger.error(err));
return user!; return user!;
} }
@ -646,6 +647,7 @@ export async function updatePerson(
export async function resolvePerson( export async function resolvePerson(
uri: string, uri: string,
resolver?: Resolver, resolver?: Resolver,
limiter: RecursionLimiter = new RecursionLimiter(20)
): Promise<CacheableUser> { ): Promise<CacheableUser> {
if (typeof uri !== "string") throw new Error("uri is not string"); if (typeof uri !== "string") throw new Error("uri is not string");
@ -659,7 +661,7 @@ export async function resolvePerson(
// Fetched from remote server and registered // Fetched from remote server and registered
if (resolver == null) resolver = new Resolver(); if (resolver == null) resolver = new Resolver();
return await createPerson(uri, resolver); return await createPerson(uri, resolver, undefined, limiter);
} }
const services: { const services: {
@ -720,7 +722,7 @@ export async function analyzeAttachments(
return { fields, services }; return { fields, services };
} }
export async function updateFeatured(userId: User["id"], resolver?: Resolver) { export async function updateFeatured(userId: User["id"], resolver?: Resolver, limiter: RecursionLimiter = new RecursionLimiter(20)) {
const user = await Users.findOneByOrFail({ id: userId }); const user = await Users.findOneByOrFail({ id: userId });
if (!Users.isRemoteUser(user)) return; if (!Users.isRemoteUser(user)) return;
if (!user.featured) return; if (!user.featured) return;
@ -742,14 +744,14 @@ export async function updateFeatured(userId: User["id"], resolver?: Resolver) {
toArray(unresolvedItems).map((x) => resolver.resolve(x)), toArray(unresolvedItems).map((x) => resolver.resolve(x)),
); );
// Resolve and regist Notes // Resolve and register Notes
resolver.reset(); resolver.reset();
const limit = promiseLimit<Note | null>(2); const limit = promiseLimit<Note | null>(2);
const featuredNotes = await Promise.all( const featuredNotes = await Promise.all(
items items
.filter((item) => getApType(item) === "Note") // TODO: Maybe it doesn't have to be a Note. .filter((item) => getApType(item) === "Note") // TODO: Maybe it doesn't have to be a Note.
.slice(0, 5) .slice(0, 5)
.map((item) => limit(() => resolveNote(item, resolver))), .map((item) => limit(() => resolveNote(item, resolver, limiter))),
); );
await db.transaction(async (transactionalEntityManager) => { await db.transaction(async (transactionalEntityManager) => {

View File

@ -11,6 +11,7 @@ import { remoteLogger } from "./logger.js";
import { Cache } from "@/misc/cache.js"; import { Cache } from "@/misc/cache.js";
import { IMentionedRemoteUsers } from "@/models/entities/note.js"; import { IMentionedRemoteUsers } from "@/models/entities/note.js";
import { UserProfile } from "@/models/entities/user-profile.js"; import { UserProfile } from "@/models/entities/user-profile.js";
import { RecursionLimiter } from "@/models/repositories/user-profile.js";
const logger = remoteLogger.createSubLogger("resolve-user"); const logger = remoteLogger.createSubLogger("resolve-user");
const uriHostCache = new Cache<string>("resolveUserUriHost", 60 * 60 * 24); const uriHostCache = new Cache<string>("resolveUserUriHost", 60 * 60 * 24);
@ -31,7 +32,7 @@ export async function resolveUser(
host: string | null, host: string | null,
refresh: boolean = true, refresh: boolean = true,
awaitRefresh: boolean = true, awaitRefresh: boolean = true,
skipMentionsOnCreate: boolean = false limiter: RecursionLimiter = new RecursionLimiter(20)
): Promise<User> { ): Promise<User> {
const usernameLower = username.toLowerCase(); const usernameLower = username.toLowerCase();
@ -105,14 +106,14 @@ export async function resolveUser(
// Otherwise create and return new user // Otherwise create and return new user
else { else {
logger.succ(`return new remote user: ${chalk.magenta(finalAcctLower)}`); logger.succ(`return new remote user: ${chalk.magenta(finalAcctLower)}`);
return await createPerson(fingerRes.self.href, undefined, subjectHost, skipMentionsOnCreate); return await createPerson(fingerRes.self.href, undefined, subjectHost, limiter);
} }
} }
} }
// Not a split domain setup, so we can simply create and return the new user // Not a split domain setup, so we can simply create and return the new user
logger.succ(`return new remote user: ${chalk.magenta(finalAcctLower)}`); logger.succ(`return new remote user: ${chalk.magenta(finalAcctLower)}`);
return await createPerson(fingerRes.self.href, undefined, subjectHost, skipMentionsOnCreate); return await createPerson(fingerRes.self.href, undefined, subjectHost, limiter);
} }
// If user information is out of date, return it by starting over from WebFinger // If user information is out of date, return it by starting over from WebFinger
@ -188,17 +189,17 @@ export async function resolveUser(
} else if (refresh && !awaitRefresh && (user.lastFetchedAt == null || Date.now() - user.lastFetchedAt.getTime() > 1000 * 60 * 60 * 24)) { } else if (refresh && !awaitRefresh && (user.lastFetchedAt == null || Date.now() - user.lastFetchedAt.getTime() > 1000 * 60 * 60 * 24)) {
// Run the refresh in the background // Run the refresh in the background
// noinspection ES6MissingAwait // noinspection ES6MissingAwait
resolveUser(username, host, true, true, skipMentionsOnCreate); resolveUser(username, host, true, true, limiter);
} }
logger.info(`return existing remote user: ${acctLower}`); logger.info(`return existing remote user: ${acctLower}`);
return user; return user;
} }
export async function resolveMentionToUserAndProfile(username: string, host: string | null, objectHost: string | null) { export async function resolveMentionToUserAndProfile(username: string, host: string | null, objectHost: string | null, limiter: RecursionLimiter) {
return profileMentionCache.fetch(`${username}@${host ?? objectHost}`, async () => { return profileMentionCache.fetch(`${username}@${host ?? objectHost}`, async () => {
try { try {
const user = await resolveUser(username, host ?? objectHost, false, false, true); const user = await resolveUser(username, host ?? objectHost, false, false, limiter);
const profile = await UserProfiles.findOneBy({ userId: user.id }); const profile = await UserProfiles.findOneBy({ userId: user.id });
const data = { username, host: host ?? objectHost }; const data = { username, host: host ?? objectHost };