Skip to content

Commit

Permalink
refactor(backend): core/activitypub/models (#11067)
Browse files Browse the repository at this point in the history 
* cleanup(`ApImageService.ts`)

* refactor(`ApImageService.ts`)

* cleanup(`check-https.ts`)

* cleanup(`ApMentionService.ts`)

* refactor(`ApMentionService.ts`)

* cleanup(`ApNoteService.ts`): unneeded `eslint-disable-next-line`

* cleanup(`ApNoteService.ts`)

* WIP(`ApImageService.ts`): `image.url`を`getApHrefNullable()`に通すかどうか悩んでいる

* refactor(`ApNoteService.ts`): function return type

* cleanup(`ApNoteService.ts`): deadcode

* refactor(`ApNoteService.ts`): `eslint-disable-next-line`

* refactor(`ApNoteService.ts`): non-null assertion

これまでは`getApId()`の方でエラーがスローされていた。

* cleanup(`ApNoteService.ts`): unneeded await

* refactor(`ApNoteService.ts`): note.attachment

- `toArray()`を使うように
- よくわからない条件式を整理
- `as`をなくすために`promiseLimit()`でジェネリクスを使うように

* cleanup(`ApNoteService.ts`)

* refactor(`ApNoteService.ts`): よりよい型定義

`res`が`null`でないことは確認されているようだったので`null`とのunionはなくした

* refactor(`ApNoteService.ts`): 不要な条件を削除

* cleanup(`ApNoteService.ts`)

* cleanup(`ApNoteService.ts`): 重要でない`as`を削除

* refactor(`ApNoteService.ts`): `eslint-disable-next-line`

* cleanup(`ApNoteService.ts`): deadcode

* cleanup(`ApNoteService.ts`): unneeded non-null assertion

* refactor(`ApNoteService.ts`): 不要な条件を削除

* WIP(`ApNoteService.ts`): `as`をなくす

エラーメッセージを考える

* cleanup(`ApNoteService.ts`): 不要な`as`を削除

* cleanup(`ApPersonService.ts`): `no-unused-vars`

* cleanup(`ApPersonService.ts`): deadcode

* refactor(`ApPersonService.ts`): function return type

* cleanup(`ApPersonService.ts`): deadcode

* cleanup(`ApPersonService.ts`): deadcode

* WIP(`ApPersonService.ts`): `as`を調整

`null`でないか確認する処理が続いていたので型アサーションは`null`とのunionにした。
より本質的な改善の余地があるように感じるのでひとまずWIPとしてコミット。

* refactor(`ApPersonService.ts`): `eslint-disable-next-line`

* WIP(`ApPersonService.ts`): `as any`をなくした

エラーをスローするようにせざるを得なかったのでエラーメッセージを考える必要がある。

* WIP(`ApNoteService.ts`): non-null assertion

non-nullアサーションを減らすために事前に存在確認をするようにした。
エラーをスローするようにしたのでメッセージを考えなければならない。

* refactor(`ApNoteService.ts`): non-null assertion -> optional chaining

* refactor(`ApPersonService.ts`): `eslint-disable-next-line`

* refactor(`ApPersonService.ts`): `eslint-disable-next-line`

* refactor(`ApPersonService.ts`): function return type

* refactor(`ApPersonService.ts`): type guardによるnon-null assertionの削除

* WIP(`ApPersonService.ts`): `analyzeAttachments`

- Field型を事前に定義しておくように

- `attachments`が`IObject`だった場合、返り値が`{ fields: [] }`になるようだが構わないのか？
- `toArray()`を通すべきでは？

* Revert "WIP(`ApImageService.ts`): `image.url`を`getApHrefNullable()`に通すかどうか悩んでいる"

This reverts commit aeefb84.

* cleanup(`ApImageService.ts`): `import`

* refactor(`ApImageService.ts`): 冗長だった部分を短く

* cleanup(`ApMentionService.ts`): `import`

* refactor(`ApImageService.ts`): `JSON.stringify()`でのindentationを追加

* cleanup(`ApNoteService.ts`): `import`

* cleanup(`ApNoteService.ts`)

* cleanup(`ApNoteService.ts`)

* cleanup(`ApNoteService.ts`)

* cleanup(`ApNoteService.ts`): `any`に対するnon-null assertion

* refactor(`ApNoteService.ts`): 添付ファイル

* cleanup(`ApPersonService.ts`): `import`

* refactor(`ApPersonService.ts`): より実情に即した`as`に

* cleanup(`ApPersonService.ts`)

* refactor(`ApPersonService.ts`): 冗長だった部分を修正

* cleanup(`ApPersonService.ts`): deadcode

* cleanup(`ApPersonService.ts`)

* cleanup(`ApQuestionService.ts`): `import`

* refactor(`ApQuestionService.ts`): `eslint-disable-next-line`

* refactor(`ApQuestionService.ts`): `eslint-disable-next-line`

* cleanup(`ApQuestionService.ts`)

* refactor(`ApQuestionService.ts`): non-null assertionを消した

* cleanup(`ApQuestionService.ts`)

* WIP(`ApQuestionService.ts`): non-null assertionを消す

エラーメッセージを考える必要がある。

* refactor(`ApQuestionService.ts`): `any`を消す

* refactor(`ApQuestionService.ts`): function return type

* WIP(`ApPersonService.ts`): 可読性の低い三項演算子を削除しつつnon-null assertionを回避

エラーメッセージを考える必要がある。

* cleanup(`ApPersonService.ts`): 不必要な三項演算子を削除

* cleanup(`ApPersonService.ts`): 不要な`as`

* cleanup(`ApPersonService.ts`)

* refactor(`ApPersonService.ts`)

* refactor(`ApPersonService.ts`): 可読性の低い三項演算子を削除

元の実装が悪いと判断し`null`かどうかの確認をより厳密に行うようにした。

* cleanup(`ApPersonService.ts`)

* cleanup(`ApPersonService.ts`)

* refactor(`ApPersonService.ts`): 返り値を`void`に統一

この返り値を参照しているコードは見当たらなかった。
また、普通に意味がない値であるように見受けられた。

* fixup! refactor(`ApPersonService.ts`): 返り値を`void`に統一

* refactor(`ApNoteService.ts`)

* refactor(`ApPersonService.ts`)

* cleanup(`ApPersonService.ts`)

* cleanup(`ApPersonService.ts`)

* refactor(`ApPersonService.ts`): 返り値の`void`統一と条件式の調整

この返り値を参照しているコードは見当たらなかった。
また、普通に意味がない値であるように見受けられた。

* cleanup(`ApQuestionService.ts`)

* refactor(`ApQuestionService.ts`)

* refactor(`ApQuestionService.ts`)

* refactor(`tag.ts`): function return type

* fixup! enhance: account migration (#10592)

* fixup! WIP(`ApPersonService.ts`): 可読性の低い三項演算子を削除しつつnon-null assertionを回避

* fixup! cleanup(`ApPersonService.ts`): 不要な`as`

* refactor: エラーメッセージを見繕った

* Revert "cleanup(`ApImageService.ts`): `import`"

This reverts commit 1454d04.

* Revert "cleanup(`ApMentionService.ts`): `import`"

This reverts commit 244f672.

* Revert "cleanup(`ApNoteService.ts`): `import`"

This reverts commit d8f0d76.

* Revert "cleanup(`ApPersonService.ts`): `import`"

This reverts commit 5190ef9.

# Conflicts:
#	packages/backend/src/core/activitypub/models/ApPersonService.ts

* Revert "cleanup(`ApQuestionService.ts`): `import`"

This reverts commit 778585e.

* processRemoteMoveはそのままにしてほしい

* Revert "fixup! refactor(`ApPersonService.ts`): 返り値を`void`に統一"

This reverts commit 083cd67.

* Revert "refactor(`ApPersonService.ts`): 返り値を`void`に統一"

This reverts commit bfa0fcd.

---------

Co-authored-by: tamaina <[email protected]>
  • Loading branch information
@tamaina
okayurisotto and tamaina authored Jul 7, 2023
1 parent 3c6175d commit 4f876c9
Show file tree
Hide file tree
Showing 7 changed files with 190 additions and 248 deletions.
35 changes: 15 additions & 20 deletions packages/backend/src/core/activitypub/models/ApImageService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import { DB_MAX_IMAGE_COMMENT_LENGTH } from '@/const.js';
import { DriveService } from '@/core/DriveService.js';
import type Logger from '@/logger.js';
import { bindThis } from '@/decorators.js';
import { checkHttps } from '@/misc/check-https.js';
import { ApResolverService } from '../ApResolverService.js';
import { ApLoggerService } from '../ApLoggerService.js';
import { checkHttps } from '@/misc/check-https.js';
import type { IObject } from '../type.js';

@Injectable()
export class ApImageService {
Expand All @@ -37,18 +38,22 @@ export class ApImageService {
* Imageを作成します。
*/
@bindThis
public async createImage(actor: RemoteUser, value: any): Promise<DriveFile> {
public async createImage(actor: RemoteUser, value: string | IObject): Promise<DriveFile> {
// 投稿者が凍結されていたらスキップ
if (actor.isSuspended) {
throw new Error('actor has been suspended');
}

const image = await this.apResolverService.createResolver().resolve(value) as any;
const image = await this.apResolverService.createResolver().resolve(value);

if (image.url == null) {
throw new Error('invalid image: url not privided');
}

if (typeof image.url !== 'string') {
throw new Error('invalid image: unexpected type of url: ' + JSON.stringify(image.url, null, 2));
}

if (!checkHttps(image.url)) {
throw new Error('invalid image: unexpected schema of url: ' + image.url);
}
Expand All @@ -57,29 +62,19 @@ export class ApImageService {

const instance = await this.metaService.fetch();

let file = await this.driveService.uploadFromUrl({
const file = await this.driveService.uploadFromUrl({
url: image.url,
user: actor,
uri: image.url,
sensitive: image.sensitive,
isLink: !instance.cacheRemoteFiles,
comment: truncate(image.name, DB_MAX_IMAGE_COMMENT_LENGTH),
comment: truncate(image.name ?? undefined, DB_MAX_IMAGE_COMMENT_LENGTH),
});
if (!file.isLink || file.url === image.url) return file;

if (file.isLink) {
// URLが異なっている場合、同じ画像が以前に異なるURLで登録されていたということなので、
// URLを更新する
if (file.url !== image.url) {
await this.driveFilesRepository.update({ id: file.id }, {
url: image.url,
uri: image.url,
});

file = await this.driveFilesRepository.findOneByOrFail({ id: file.id });
}
}

return file;
// URLが異なっている場合、同じ画像が以前に異なるURLで登録されていたということなので、URLを更新する
await this.driveFilesRepository.update({ id: file.id }, { url: image.url, uri: image.url });
return await this.driveFilesRepository.findOneByOrFail({ id: file.id });
}

/**
Expand All @@ -89,7 +84,7 @@ export class ApImageService {
* リモートサーバーからフェッチしてMisskeyに登録しそれを返します。
*/
@bindThis
public async resolveImage(actor: RemoteUser, value: any): Promise<DriveFile> {
public async resolveImage(actor: RemoteUser, value: string | IObject): Promise<DriveFile> {
// TODO

// リモートサーバーからフェッチしてきて登録
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/core/activitypub/models/ApMentionService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export class ApMentionService {
}

@bindThis
public async extractApMentions(tags: IObject | IObject[] | null | undefined, resolver: Resolver) {
const hrefs = unique(this.extractApMentionObjects(tags).map(x => x.href as string));
public async extractApMentions(tags: IObject | IObject[] | null | undefined, resolver: Resolver): Promise<User[]> {
const hrefs = unique(this.extractApMentionObjects(tags).map(x => x.href));

const limit = promiseLimit<User | null>(2);
const mentionedUsers = (await Promise.all(
Expand Down
137 changes: 61 additions & 76 deletions packages/backend/src/core/activitypub/models/ApNoteService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { UtilityService } from '@/core/UtilityService.js';
import { bindThis } from '@/decorators.js';
import { checkHttps } from '@/misc/check-https.js';
import { getOneApId, getApId, getOneApHrefNullable, validPost, isEmoji, getApType } from '../type.js';
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
import { ApLoggerService } from '../ApLoggerService.js';
import { ApMfmService } from '../ApMfmService.js';
import { ApDbResolverService } from '../ApDbResolverService.js';
Expand Down Expand Up @@ -72,13 +71,9 @@ export class ApNoteService {
}

@bindThis
public validateNote(object: IObject, uri: string) {
public validateNote(object: IObject, uri: string): Error | null {
const expectHost = this.utilityService.extractDbHost(uri);

if (object == null) {
return new Error('invalid Note: object is null');
}

if (!validPost.includes(getApType(object))) {
return new Error(`invalid Note: invalid object type ${getApType(object)}`);
}
Expand Down Expand Up @@ -110,19 +105,18 @@ export class ApNoteService {
*/
@bindThis
public async createNote(value: string | IObject, resolver?: Resolver, silent = false): Promise<Note | null> {
// eslint-disable-next-line no-param-reassign
if (resolver == null) resolver = this.apResolverService.createResolver();

const object = await resolver.resolve(value);

const entryUri = getApId(value);
const err = this.validateNote(object, entryUri);
if (err) {
this.logger.error(`${err.message}`, {
resolver: {
history: resolver.getHistory(),
},
value: value,
object: object,
this.logger.error(err.message, {
resolver: { history: resolver.getHistory() },
value,
object,
});
throw new Error('invalid note');
}
Expand All @@ -144,7 +138,11 @@ export class ApNoteService {
this.logger.info(`Creating the Note: ${note.id}`);

// 投稿者をフェッチ
const actor = await this.apPersonService.resolvePerson(getOneApId(note.attributedTo!), resolver) as RemoteUser;
if (note.attributedTo == null) {
throw new Error('invalid note.attributedTo: ' + note.attributedTo);
}

const actor = await this.apPersonService.resolvePerson(getOneApId(note.attributedTo), resolver) as RemoteUser;

// 投稿者が凍結されていたらスキップ
if (actor.isSuspended) {
Expand All @@ -164,59 +162,49 @@ export class ApNoteService {
}

const apMentions = await this.apMentionService.extractApMentions(note.tag, resolver);
const apHashtags = await extractApHashtags(note.tag);
const apHashtags = extractApHashtags(note.tag);

// 添付ファイル
// TODO: attachmentは必ずしもImageではない
// TODO: attachmentは必ずしも配列ではない
// Noteがsensitiveなら添付もsensitiveにする
const limit = promiseLimit(2);

note.attachment = Array.isArray(note.attachment) ? note.attachment : note.attachment ? [note.attachment] : [];
const files = note.attachment
.map(attach => attach.sensitive = note.sensitive)
? (await Promise.all(note.attachment.map(x => limit(() => this.apImageService.resolveImage(actor, x)) as Promise<DriveFile>)))
.filter(image => image != null)
: [];
const limit = promiseLimit<DriveFile>(2);
const files = (await Promise.all(toArray(note.attachment).map(attach => (
limit(() => this.apImageService.resolveImage(actor, {
...attach,
sensitive: note.sensitive, // Noteがsensitiveなら添付もsensitiveにする
}))
))));

// リプライ
const reply: Note | null = note.inReplyTo
? await this.resolveNote(note.inReplyTo, resolver).then(x => {
if (x == null) {
this.logger.warn('Specified inReplyTo, but not found');
throw new Error('inReplyTo not found');
} else {
? await this.resolveNote(note.inReplyTo, resolver)
.then(x => {
if (x == null) {
this.logger.warn('Specified inReplyTo, but not found');
throw new Error('inReplyTo not found');
}

return x;
}
}).catch(async err => {
this.logger.warn(`Error in inReplyTo ${note.inReplyTo} - ${err.statusCode ?? err}`);
throw err;
})
})
.catch(async err => {
this.logger.warn(`Error in inReplyTo ${note.inReplyTo} - ${err.statusCode ?? err}`);
throw err;
})
: null;

// 引用
let quote: Note | undefined | null;
let quote: Note | undefined | null = null;

if (note._misskey_quote || note.quoteUrl) {
const tryResolveNote = async (uri: string): Promise<{
status: 'ok';
res: Note | null;
} | {
status: 'permerror' | 'temperror';
}> => {
if (typeof uri !== 'string' || !uri.match(/^https?:/)) return { status: 'permerror' };
const tryResolveNote = async (uri: string): Promise<
| { status: 'ok'; res: Note }
| { status: 'permerror' | 'temperror' }
> => {
if (!uri.match(/^https?:/)) return { status: 'permerror' };
try {
const res = await this.resolveNote(uri);
if (res) {
return {
status: 'ok',
res,
};
} else {
return {
status: 'permerror',
};
}
if (res == null) return { status: 'permerror' };
return { status: 'ok', res };
} catch (e) {
return {
status: (e instanceof StatusError && e.isClientError) ? 'permerror' : 'temperror',
Expand All @@ -225,9 +213,9 @@ export class ApNoteService {
};

const uris = unique([note._misskey_quote, note.quoteUrl].filter((x): x is string => typeof x === 'string'));
const results = await Promise.all(uris.map(uri => tryResolveNote(uri)));
const results = await Promise.all(uris.map(tryResolveNote));

quote = results.filter((x): x is { status: 'ok', res: Note | null } => x.status === 'ok').map(x => x.res).find(x => x);
quote = results.filter((x): x is { status: 'ok', res: Note } => x.status === 'ok').map(x => x.res).at(0);
if (!quote) {
if (results.some(x => x.status === 'temperror')) {
throw new Error('quote resolve failed');
Expand Down Expand Up @@ -271,7 +259,7 @@ export class ApNoteService {

const emojis = await this.extractEmojis(note.tag ?? [], actor.host).catch(e => {
this.logger.info(`extractEmojis: ${e}`);
return [] as Emoji[];
return [];
});

const apEmojis = emojis.map(emoji => emoji.name);
Expand Down Expand Up @@ -309,19 +297,18 @@ export class ApNoteService {
const uri = typeof value === 'string' ? value : value.id;
if (uri == null) throw new Error('missing uri');

// ブロックしてたら中断
// ブロックしていたら中断
const meta = await this.metaService.fetch();
if (this.utilityService.isBlockedHost(meta.blockedHosts, this.utilityService.extractDbHost(uri))) throw new StatusError('blocked host', 451);
if (this.utilityService.isBlockedHost(meta.blockedHosts, this.utilityService.extractDbHost(uri))) {
throw new StatusError('blocked host', 451);
}

const unlock = await this.appLockService.getApLock(uri);

try {
//#region このサーバーに既に登録されていたらそれを返す
const exist = await this.fetchNote(uri);

if (exist) {
return exist;
}
if (exist) return exist;
//#endregion

if (uri.startsWith(this.config.url)) {
Expand All @@ -339,43 +326,41 @@ export class ApNoteService {

@bindThis
public async extractEmojis(tags: IObject | IObject[], host: string): Promise<Emoji[]> {
// eslint-disable-next-line no-param-reassign
host = this.utilityService.toPuny(host);

if (!tags) return [];

const eomjiTags = toArray(tags).filter(isEmoji);

const existingEmojis = await this.emojisRepository.findBy({
host,
name: In(eomjiTags.map(tag => tag.name!.replaceAll(':', ''))),
name: In(eomjiTags.map(tag => tag.name.replaceAll(':', ''))),
});

return await Promise.all(eomjiTags.map(async tag => {
const name = tag.name!.replaceAll(':', '');
const name = tag.name.replaceAll(':', '');
tag.icon = toSingle(tag.icon);

const exists = existingEmojis.find(x => x.name === name);

if (exists) {
if ((tag.updated != null && exists.updatedAt == null)
if ((exists.updatedAt == null)
|| (tag.id != null && exists.uri == null)
|| (tag.updated != null && exists.updatedAt != null && new Date(tag.updated) > exists.updatedAt)
|| (tag.icon!.url !== exists.originalUrl)
|| (new Date(tag.updated) > exists.updatedAt)
|| (tag.icon.url !== exists.originalUrl)
) {
await this.emojisRepository.update({
host,
name,
}, {
uri: tag.id,
originalUrl: tag.icon!.url,
publicUrl: tag.icon!.url,
originalUrl: tag.icon.url,
publicUrl: tag.icon.url,
updatedAt: new Date(),
});

return await this.emojisRepository.findOneBy({
host,
name,
}) as Emoji;
const emoji = await this.emojisRepository.findOneBy({ host, name });
if (emoji == null) throw new Error('emoji update failed');
return emoji;
}

return exists;
Expand All @@ -388,11 +373,11 @@ export class ApNoteService {
host,
name,
uri: tag.id,
originalUrl: tag.icon!.url,
publicUrl: tag.icon!.url,
originalUrl: tag.icon.url,
publicUrl: tag.icon.url,
updatedAt: new Date(),
aliases: [],
} as Partial<Emoji>).then(x => this.emojisRepository.findOneByOrFail(x.identifiers[0]));
}).then(x => this.emojisRepository.findOneByOrFail(x.identifiers[0]));
}));
}
}
Loading

0 comments on commit 4f876c9

Please sign in to comment.