Fix avatar/banner proxy (#8346)

* Fix avatar/banner proxy Co-authored-by: mei23 <m@m544.net> * use getAvatarUrl * fix * join avatar and banner to improve performance * join * Update hybrid-timeline.ts * fix Co-authored-by: mei23 <m@m544.net> Co-authored-by: syuilo <Syuilotan@yahoo.co.jp>
misskey-dev · Feb 27, 2022 · e314be5 · e314be5
1 parent d071d18
commit e314be5
Show file tree

Hide file tree

Showing 34 changed files with 180 additions and 99 deletions.
diff --git a/.config/example.yml b/.config/example.yml
@@ -155,6 +155,9 @@ id: 'aid'
 # Media Proxy
 #mediaProxy: https://example.com/proxy
 
+# Proxy remote files (default: false)
+#proxyRemoteFiles: true
+
 # Sign to ActivityPub GET request (default: false)
 #signToActivityPubGet: true
 

diff --git a/locales/ja-JP.yml b/locales/ja-JP.yml
@@ -325,8 +325,6 @@ disablingTimelinesInfo: "これらのタイムラインを無効化しても、
 registration: "登録"
 enableRegistration: "誰でも新規登録できるようにする"
 invite: "招待"
-proxyRemoteFiles: "リモートのファイルをプロキシする"
-proxyRemoteFilesDescription: "この設定を有効にすると、未保存または保存容量超過で削除されたリモートファイルをローカルでプロキシし、サムネイルも生成するようになります。サーバーのストレージには影響しません、"
 driveCapacityPerLocalAccount: "ローカルユーザーひとりあたりのドライブ容量"
 driveCapacityPerRemoteAccount: "リモートユーザーひとりあたりのドライブ容量"
 inMb: "メガバイト単位"

diff --git a/packages/backend/migration/1626509500668-fix-remote-file-proxy.js b/packages/backend/migration/1626509500668-fix-remote-file-proxy.js
@@ -0,0 +1,23 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+class fixRemoteFileProxy1626509500668 {
+ constructor() {
+ this.name = 'fixRemoteFileProxy1626509500668';
+ }
+ async up(queryRunner) {
+ await queryRunner.query(`ALTER TABLE "user" DROP COLUMN "avatarUrl"`);
+ await queryRunner.query(`ALTER TABLE "user" DROP COLUMN "bannerUrl"`);
+ await queryRunner.query(`ALTER TABLE "user" DROP COLUMN "avatarBlurhash"`);
+ await queryRunner.query(`ALTER TABLE "user" DROP COLUMN "bannerBlurhash"`);
+ await queryRunner.query(`ALTER TABLE "meta" DROP COLUMN "proxyRemoteFiles"`);
+ }
+ async down(queryRunner) {
+ await queryRunner.query(`ALTER TABLE "meta" ADD "proxyRemoteFiles" boolean NOT NULL DEFAULT false`);
+ await queryRunner.query(`ALTER TABLE "user" ADD "bannerBlurhash" character varying(128)`);
+ await queryRunner.query(`ALTER TABLE "user" ADD "avatarBlurhash" character varying(128)`);
+ await queryRunner.query(`ALTER TABLE "user" ADD "bannerUrl" character varying(512)`);
+ await queryRunner.query(`ALTER TABLE "user" ADD "avatarUrl" character varying(512)`);
+ }
+}
+
+exports.fixRemoteFileProxy1626509500668 = fixRemoteFileProxy1626509500668;
diff --git a/packages/backend/src/config/types.ts b/packages/backend/src/config/types.ts
@@ -62,6 +62,7 @@ export type Source = {
  };
 
  mediaProxy?: string;
+ proxyRemoteFiles?: boolean;
 
  signToActivityPubGet?: boolean;
 };

diff --git a/packages/backend/src/models/entities/meta.ts b/packages/backend/src/models/entities/meta.ts
@@ -137,11 +137,6 @@ export class Meta {
  })
  public cacheRemoteFiles: boolean;
 
- @Column('boolean', {
- default: false,
- })
- public proxyRemoteFiles: boolean;
-
  @Column({
  ...id(),
  nullable: true,

diff --git a/packages/backend/src/models/entities/user.ts b/packages/backend/src/models/entities/user.ts
@@ -106,26 +106,6 @@ export class User {
  })
  public tags: string[];
 
- @Column('varchar', {
- length: 512, nullable: true,
- })
- public avatarUrl: string | null;
-
- @Column('varchar', {
- length: 512, nullable: true,
- })
- public bannerUrl: string | null;
-
- @Column('varchar', {
- length: 128, nullable: true,
- })
- public avatarBlurhash: string | null;
-
- @Column('varchar', {
- length: 128, nullable: true,
- })
- public bannerBlurhash: string | null;
-
  @Column('boolean', {
  default: false,
  comment: 'Whether the User is suspended.',

diff --git a/packages/backend/src/models/repositories/drive-file.ts b/packages/backend/src/models/repositories/drive-file.ts
@@ -41,7 +41,7 @@ export class DriveFileRepository extends Repository<DriveFile> {
  return file.properties;
  }
 
- public getPublicUrl(file: DriveFile, thumbnail = false, meta?: Meta): string | null {
+ public getPublicUrl(file: DriveFile, thumbnail = false): string | null {
  // リモートかつメディアプロキシ
  if (file.uri != null && file.userHost != null && config.mediaProxy != null) {
  return appendQuery(config.mediaProxy, query({
@@ -51,7 +51,7 @@ export class DriveFileRepository extends Repository<DriveFile> {
  }
 
  // リモートかつ期限切れはローカルプロキシを試みる
- if (file.uri != null && file.isLink && meta && meta.proxyRemoteFiles) {
+ if (file.uri != null && file.isLink && config.proxyRemoteFiles) {
  const key = thumbnail ? file.thumbnailAccessKey : file.webpublicAccessKey;
 
  if (key && !key.match('/')) { // 古いものはここにオブジェクトストレージキーが入ってるので除外
@@ -136,8 +136,8 @@ export class DriveFileRepository extends Repository<DriveFile> {
  isSensitive: file.isSensitive,
  blurhash: file.blurhash,
  properties: opts.self ? file.properties : this.getPublicProperties(file),
- url: opts.self ? file.url : this.getPublicUrl(file, false, meta),
- thumbnailUrl: this.getPublicUrl(file, true, meta),
+ url: opts.self ? file.url : this.getPublicUrl(file, false),
+ thumbnailUrl: this.getPublicUrl(file, true),
  comment: file.comment,
  folderId: file.folderId,
  folder: opts.detail && file.folderId ? DriveFolders.pack(file.folderId, {

diff --git a/packages/backend/src/models/repositories/note-reaction.ts b/packages/backend/src/models/repositories/note-reaction.ts
@@ -23,10 +23,10 @@ export class NoteReactionRepository extends Repository<NoteReaction> {
  return {
  id: reaction.id,
  createdAt: reaction.createdAt.toISOString(),
- user: await Users.pack(reaction.userId, me),
+ user: await Users.pack(reaction.user ?? reaction.userId, me),
  type: convertLegacyReaction(reaction.reaction),
  ...(opts.withNote ? {
- note: await Notes.pack(reaction.noteId, me),
+ note: await Notes.pack(reaction.note ?? reaction.noteId, me),
  } : {}),
  };
  }

diff --git a/packages/backend/src/models/repositories/user.ts b/packages/backend/src/models/repositories/user.ts
@@ -1,7 +1,7 @@
 import { EntityRepository, Repository, In, Not } from 'typeorm';
 import Ajv from 'ajv';
 import { User, ILocalUser, IRemoteUser } from '@/models/entities/user.js';
-import { Notes, NoteUnreads, FollowRequests, Notifications, MessagingMessages, UserNotePinings, Followings, Blockings, Mutings, UserProfiles, UserSecurityKeys, UserGroupJoinings, Pages, Announcements, AnnouncementReads, Antennas, AntennaNotes, ChannelFollowings, Instances } from '../index.js';
+import { Notes, NoteUnreads, FollowRequests, Notifications, MessagingMessages, UserNotePinings, Followings, Blockings, Mutings, UserProfiles, UserSecurityKeys, UserGroupJoinings, Pages, Announcements, AnnouncementReads, Antennas, AntennaNotes, ChannelFollowings, Instances, DriveFiles } from '../index.js';
 import config from '@/config/index.js';
 import { Packed } from '@/misc/schema.js';
 import { awaitAll, Promiseable } from '@/prelude/await-all.js';
@@ -182,13 +182,18 @@ export class UserRepository extends Repository<User> {
  }
 
  public getAvatarUrl(user: User): string {
- if (user.avatarUrl) {
- return user.avatarUrl;
+ // TODO: avatarIdがあるがavatarがない(JOINされてない)場合のハンドリング
+ if (user.avatar) {
+ return DriveFiles.getPublicUrl(user.avatar, true) || this.getIdenticonUrl(user.id);
  } else {
- return `${config.url}/identicon/${user.id}`;
+ return this.getIdenticonUrl(user.id);
  }
  }
 
+ public getIdenticonUrl(userId: User['id']): string {
+ return `${config.url}/identicon/${userId}`;
+ }
+
  public async pack<ExpectsMe extends boolean | null = null, D extends boolean = false>(
  src: User['id'] | User,
  me?: { id: User['id'] } | null | undefined,
@@ -202,7 +207,18 @@ export class UserRepository extends Repository<User> {
  includeSecrets: false,
  }, options);
 
- const user = typeof src === 'object' ? src : await this.findOneOrFail(src);
+ let user: User;
+
+ if (typeof src === 'object') {
+ user = src;
+ if (src.avatar === undefined && src.avatarId) src.avatar = await DriveFiles.findOne(src.avatarId) ?? null;
+ if (src.banner === undefined && src.bannerId) src.banner = await DriveFiles.findOne(src.bannerId) ?? null;
+ } else {
+ user = await this.findOneOrFail(src, {
+ relations: ['avatar', 'banner'],
+ });
+ }
+
  const meId = me ? me.id : null;
  const isMe = meId === user.id;
 
@@ -232,7 +248,7 @@ export class UserRepository extends Repository<User> {
  username: user.username,
  host: user.host,
  avatarUrl: this.getAvatarUrl(user),
- avatarBlurhash: user.avatarBlurhash,
+ avatarBlurhash: user.avatar?.blurhash || null,
  avatarColor: null, // 後方互換性のため
  isAdmin: user.isAdmin || falsy,
  isModerator: user.isModerator || falsy,
@@ -256,8 +272,8 @@ export class UserRepository extends Repository<User> {
  createdAt: user.createdAt.toISOString(),
  updatedAt: user.updatedAt ? user.updatedAt.toISOString() : null,
  lastFetchedAt: user.lastFetchedAt ? user.lastFetchedAt.toISOString() : null,
- bannerUrl: user.bannerUrl,
- bannerBlurhash: user.bannerBlurhash,
+ bannerUrl: user.banner ? DriveFiles.getPublicUrl(user.banner, false) : null,
+ bannerBlurhash: user.banner?.blurhash || null,
  bannerColor: null, // 後方互換性のため
  isLocked: user.isLocked,
  isSilenced: user.isSilenced || falsy,

diff --git a/packages/backend/src/remote/activitypub/models/person.ts b/packages/backend/src/remote/activitypub/models/person.ts
@@ -228,26 +228,14 @@ export async function createPerson(uri: string, resolver?: Resolver): Promise<Us
 
  const avatarId = avatar ? avatar.id : null;
  const bannerId = banner ? banner.id : null;
- const avatarUrl = avatar ? DriveFiles.getPublicUrl(avatar, true) : null;
- const bannerUrl = banner ? DriveFiles.getPublicUrl(banner) : null;
- const avatarBlurhash = avatar ? avatar.blurhash : null;
- const bannerBlurhash = banner ? banner.blurhash : null;
 
  await Users.update(user!.id, {
  avatarId,
  bannerId,
- avatarUrl,
- bannerUrl,
- avatarBlurhash,
- bannerBlurhash,
  });
 
  user!.avatarId = avatarId;
  user!.bannerId = bannerId;
- user!.avatarUrl = avatarUrl;
- user!.bannerUrl = bannerUrl;
- user!.avatarBlurhash = avatarBlurhash;
- user!.bannerBlurhash = bannerBlurhash;
  //#endregion
 
  //#region カスタム絵文字取得
@@ -340,14 +328,10 @@ export async function updatePerson(uri: string, resolver?: Resolver | null, hint
 
  if (avatar) {
  updates.avatarId = avatar.id;
- updates.avatarUrl = DriveFiles.getPublicUrl(avatar, true);
- updates.avatarBlurhash = avatar.blurhash;
  }
 
  if (banner) {
  updates.bannerId = banner.id;
- updates.bannerUrl = DriveFiles.getPublicUrl(banner);
- updates.bannerBlurhash = banner.blurhash;
  }
 
  // Update user

diff --git a/packages/backend/src/server/api/endpoints/admin/update-meta.ts b/packages/backend/src/server/api/endpoints/admin/update-meta.ts
@@ -39,7 +39,6 @@ export const paramDef = {
  localDriveCapacityMb: { type: 'integer' },
  remoteDriveCapacityMb: { type: 'integer' },
  cacheRemoteFiles: { type: 'boolean' },
- proxyRemoteFiles: { type: 'boolean' },
  emailRequiredForSignup: { type: 'boolean' },
  enableHcaptcha: { type: 'boolean' },
  hcaptchaSiteKey: { type: 'string', nullable: true },
@@ -175,10 +174,6 @@ export default define(meta, paramDef, async (ps, me) => {
  set.cacheRemoteFiles = ps.cacheRemoteFiles;
  }
 
- if (ps.proxyRemoteFiles !== undefined) {
- set.proxyRemoteFiles = ps.proxyRemoteFiles;
- }
-
  if (ps.emailRequiredForSignup !== undefined) {
  set.emailRequiredForSignup = ps.emailRequiredForSignup;
  }

diff --git a/packages/backend/src/server/api/endpoints/antennas/notes.ts b/packages/backend/src/server/api/endpoints/antennas/notes.ts
@@ -65,10 +65,16 @@ export default define(meta, paramDef, async (ps, user) => {
  ps.sinceId, ps.untilId, ps.sinceDate, ps.untilDate)
  .andWhere(`note.id IN (${ antennaQuery.getQuery() })`)
  .innerJoinAndSelect('note.user', 'user')
+ .leftJoinAndSelect('user.avatar', 'avatar')
+ .leftJoinAndSelect('user.banner', 'banner')
  .leftJoinAndSelect('note.reply', 'reply')
  .leftJoinAndSelect('note.renote', 'renote')
  .leftJoinAndSelect('reply.user', 'replyUser')
+ .leftJoinAndSelect('replyUser.avatar', 'replyUserAvatar')
+ .leftJoinAndSelect('replyUser.banner', 'replyUserBanner')
  .leftJoinAndSelect('renote.user', 'renoteUser')
+ .leftJoinAndSelect('renoteUser.avatar', 'renoteUserAvatar')
+ .leftJoinAndSelect('renoteUser.banner', 'renoteUserBanner')
  .setParameters(antennaQuery.getParameters());
 
  generateVisibilityQuery(query, user);

diff --git a/packages/backend/src/server/api/endpoints/channels/timeline.ts b/packages/backend/src/server/api/endpoints/channels/timeline.ts
@@ -55,10 +55,16 @@ export default define(meta, paramDef, async (ps, user) => {
  const query = makePaginationQuery(Notes.createQueryBuilder('note'), ps.sinceId, ps.untilId, ps.sinceDate, ps.untilDate)
  .andWhere('note.channelId = :channelId', { channelId: channel.id })
  .innerJoinAndSelect('note.user', 'user')
+ .leftJoinAndSelect('user.avatar', 'avatar')
+ .leftJoinAndSelect('user.banner', 'banner')
  .leftJoinAndSelect('note.reply', 'reply')
  .leftJoinAndSelect('note.renote', 'renote')
  .leftJoinAndSelect('reply.user', 'replyUser')
+ .leftJoinAndSelect('replyUser.avatar', 'replyUserAvatar')
+ .leftJoinAndSelect('replyUser.banner', 'replyUserBanner')
  .leftJoinAndSelect('renote.user', 'renoteUser')
+ .leftJoinAndSelect('renoteUser.avatar', 'renoteUserAvatar')
+ .leftJoinAndSelect('renoteUser.banner', 'renoteUserBanner')
  .leftJoinAndSelect('note.channel', 'channel');
  //#endregion
 

diff --git a/packages/backend/src/server/api/endpoints/clips/notes.ts b/packages/backend/src/server/api/endpoints/clips/notes.ts
@@ -64,10 +64,16 @@ export default define(meta, paramDef, async (ps, user) => {
  const query = makePaginationQuery(Notes.createQueryBuilder('note'), ps.sinceId, ps.untilId)
  .andWhere(`note.id IN (${ clipQuery.getQuery() })`)
  .innerJoinAndSelect('note.user', 'user')
+ .leftJoinAndSelect('user.avatar', 'avatar')
+ .leftJoinAndSelect('user.banner', 'banner')
  .leftJoinAndSelect('note.reply', 'reply')
  .leftJoinAndSelect('note.renote', 'renote')
  .leftJoinAndSelect('reply.user', 'replyUser')
+ .leftJoinAndSelect('replyUser.avatar', 'replyUserAvatar')
+ .leftJoinAndSelect('replyUser.banner', 'replyUserBanner')
  .leftJoinAndSelect('renote.user', 'renoteUser')
+ .leftJoinAndSelect('renoteUser.avatar', 'renoteUserAvatar')
+ .leftJoinAndSelect('renoteUser.banner', 'renoteUserBanner')
  .setParameters(clipQuery.getParameters());
 
  if (user) {

diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts
@@ -71,10 +71,16 @@ export default define(meta, paramDef, async (ps, user) => {
  .leftJoinAndSelect('notification.notifier', 'notifier')
  .leftJoinAndSelect('notification.note', 'note')
  .leftJoinAndSelect('note.user', 'user')
+ .leftJoinAndSelect('user.avatar', 'avatar')
+ .leftJoinAndSelect('user.banner', 'banner')
  .leftJoinAndSelect('note.reply', 'reply')
  .leftJoinAndSelect('note.renote', 'renote')
  .leftJoinAndSelect('reply.user', 'replyUser')
- .leftJoinAndSelect('renote.user', 'renoteUser');
+ .leftJoinAndSelect('replyUser.avatar', 'replyUserAvatar')
+ .leftJoinAndSelect('replyUser.banner', 'replyUserBanner')
+ .leftJoinAndSelect('renote.user', 'renoteUser')
+ .leftJoinAndSelect('renoteUser.avatar', 'renoteUserAvatar')
+ .leftJoinAndSelect('renoteUser.banner', 'renoteUserBanner');
 
  query.andWhere(new Brackets(qb => { qb
  .where(`notification.notifierId NOT IN (${ mutingQuery.getQuery() })`)

diff --git a/packages/backend/src/server/api/endpoints/i/update.ts b/packages/backend/src/server/api/endpoints/i/update.ts
@@ -175,25 +175,13 @@ export default define(meta, paramDef, async (ps, _user, token) => {
 
  if (avatar == null || avatar.userId !== user.id) throw new ApiError(meta.errors.noSuchAvatar);
  if (!avatar.type.startsWith('image/')) throw new ApiError(meta.errors.avatarNotAnImage);
-
- updates.avatarUrl = DriveFiles.getPublicUrl(avatar, true);
-
- if (avatar.blurhash) {
- updates.avatarBlurhash = avatar.blurhash;
- }
  }
 
  if (ps.bannerId) {
  const banner = await DriveFiles.findOne(ps.bannerId);
 
  if (banner == null || banner.userId !== user.id) throw new ApiError(meta.errors.noSuchBanner);
  if (!banner.type.startsWith('image/')) throw new ApiError(meta.errors.bannerNotAnImage);
-
- updates.bannerUrl = DriveFiles.getPublicUrl(banner, false);
-
- if (banner.blurhash) {
- updates.bannerBlurhash = banner.blurhash;
- }
  }
 
  if (ps.pinnedPageId) {

diff --git a/packages/backend/src/server/api/endpoints/meta.ts b/packages/backend/src/server/api/endpoints/meta.ts
@@ -93,10 +93,6 @@ export const meta = {
  type: 'boolean',
  optional: false, nullable: false,
  },
- proxyRemoteFiles: {
- type: 'boolean',
- optional: false, nullable: false,
- },
  emailRequiredForSignup: {
  type: 'boolean',
  optional: false, nullable: false,
@@ -529,7 +525,6 @@ export default define(meta, paramDef, async (ps, me) => {
  pinnedPages: instance.pinnedPages,
  pinnedClipId: instance.pinnedClipId,
  cacheRemoteFiles: instance.cacheRemoteFiles,
- proxyRemoteFiles: instance.proxyRemoteFiles,
  requireSetup: (await Users.count({
  host: null,
  })) === 0,

diff --git a/packages/backend/src/server/api/endpoints/notes.ts b/packages/backend/src/server/api/endpoints/notes.ts
@@ -37,10 +37,16 @@ export default define(meta, paramDef, async (ps) => {
  .andWhere(`note.visibility = 'public'`)
  .andWhere(`note.localOnly = FALSE`)
  .innerJoinAndSelect('note.user', 'user')
+ .leftJoinAndSelect('user.avatar', 'avatar')
+ .leftJoinAndSelect('user.banner', 'banner')
  .leftJoinAndSelect('note.reply', 'reply')
  .leftJoinAndSelect('note.renote', 'renote')
  .leftJoinAndSelect('reply.user', 'replyUser')
- .leftJoinAndSelect('renote.user', 'renoteUser');
+ .leftJoinAndSelect('replyUser.avatar', 'replyUserAvatar')
+ .leftJoinAndSelect('replyUser.banner', 'replyUserBanner')
+ .leftJoinAndSelect('renote.user', 'renoteUser')
+ .leftJoinAndSelect('renoteUser.avatar', 'renoteUserAvatar')
+ .leftJoinAndSelect('renoteUser.banner', 'renoteUserBanner');
 
  if (ps.local) {
  query.andWhere('note.userHost IS NULL');