fix: サジェストされるユーザのリストアップ方法を見直し (#14180)

* fix: サジェストされるユーザのリストアップ方法を見直し

* fix comment

* fix CHANGELOG.md

* ノートの無いユーザ(updatedAtが無いユーザ)は含めないらしい

* fix test
This commit is contained in:
おさむのひと 2024-07-12 21:14:09 +09:00 committed by GitHub
parent 76b1c74a37
commit 6cd15275bb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 492 additions and 90 deletions

View File

@ -40,6 +40,11 @@
- Fix: リノートにリアクションできないように - Fix: リノートにリアクションできないように
- Fix: ユーザー名の前後に空白文字列がある場合は省略するように - Fix: ユーザー名の前後に空白文字列がある場合は省略するように
- Fix: プロフィール編集時に名前を空白文字列のみにできる問題を修正 - Fix: プロフィール編集時に名前を空白文字列のみにできる問題を修正
- Fix: ユーザ名のサジェスト時に表示される内容と順番を調整(以下の順番になります) #14149
1. フォロー中かつアクティブなユーザ
2. フォロー中かつ非アクティブなユーザ
3. フォローしていないアクティブなユーザ
4. フォローしていない非アクティブなユーザ
### Misskey.js ### Misskey.js
- Feat: `/drive/files/create` のリクエストに対応(`multipart/form-data`に対応) - Feat: `/drive/files/create` のリクエストに対応(`multipart/form-data`に対応)

View File

@ -12,6 +12,7 @@ import {
} from '@/core/entities/AbuseReportNotificationRecipientEntityService.js'; } from '@/core/entities/AbuseReportNotificationRecipientEntityService.js';
import { AbuseReportNotificationService } from '@/core/AbuseReportNotificationService.js'; import { AbuseReportNotificationService } from '@/core/AbuseReportNotificationService.js';
import { SystemWebhookService } from '@/core/SystemWebhookService.js'; import { SystemWebhookService } from '@/core/SystemWebhookService.js';
import { UserSearchService } from '@/core/UserSearchService.js';
import { AccountMoveService } from './AccountMoveService.js'; import { AccountMoveService } from './AccountMoveService.js';
import { AccountUpdateService } from './AccountUpdateService.js'; import { AccountUpdateService } from './AccountUpdateService.js';
import { AiService } from './AiService.js'; import { AiService } from './AiService.js';
@ -202,6 +203,7 @@ const $UserFollowingService: Provider = { provide: 'UserFollowingService', useEx
const $UserKeypairService: Provider = { provide: 'UserKeypairService', useExisting: UserKeypairService }; const $UserKeypairService: Provider = { provide: 'UserKeypairService', useExisting: UserKeypairService };
const $UserListService: Provider = { provide: 'UserListService', useExisting: UserListService }; const $UserListService: Provider = { provide: 'UserListService', useExisting: UserListService };
const $UserMutingService: Provider = { provide: 'UserMutingService', useExisting: UserMutingService }; const $UserMutingService: Provider = { provide: 'UserMutingService', useExisting: UserMutingService };
const $UserSearchService: Provider = { provide: 'UserSearchService', useExisting: UserSearchService };
const $UserSuspendService: Provider = { provide: 'UserSuspendService', useExisting: UserSuspendService }; const $UserSuspendService: Provider = { provide: 'UserSuspendService', useExisting: UserSuspendService };
const $UserAuthService: Provider = { provide: 'UserAuthService', useExisting: UserAuthService }; const $UserAuthService: Provider = { provide: 'UserAuthService', useExisting: UserAuthService };
const $VideoProcessingService: Provider = { provide: 'VideoProcessingService', useExisting: VideoProcessingService }; const $VideoProcessingService: Provider = { provide: 'VideoProcessingService', useExisting: VideoProcessingService };
@ -348,6 +350,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
UserKeypairService, UserKeypairService,
UserListService, UserListService,
UserMutingService, UserMutingService,
UserSearchService,
UserSuspendService, UserSuspendService,
UserAuthService, UserAuthService,
VideoProcessingService, VideoProcessingService,
@ -490,6 +493,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
$UserKeypairService, $UserKeypairService,
$UserListService, $UserListService,
$UserMutingService, $UserMutingService,
$UserSearchService,
$UserSuspendService, $UserSuspendService,
$UserAuthService, $UserAuthService,
$VideoProcessingService, $VideoProcessingService,
@ -633,6 +637,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
UserKeypairService, UserKeypairService,
UserListService, UserListService,
UserMutingService, UserMutingService,
UserSearchService,
UserSuspendService, UserSuspendService,
UserAuthService, UserAuthService,
VideoProcessingService, VideoProcessingService,
@ -774,6 +779,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
$UserKeypairService, $UserKeypairService,
$UserListService, $UserListService,
$UserMutingService, $UserMutingService,
$UserSearchService,
$UserSuspendService, $UserSuspendService,
$UserAuthService, $UserAuthService,
$VideoProcessingService, $VideoProcessingService,

View File

@ -0,0 +1,205 @@
/*
* SPDX-FileCopyrightText: syuilo and misskey-project
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { Inject, Injectable } from '@nestjs/common';
import { Brackets, SelectQueryBuilder } from 'typeorm';
import { DI } from '@/di-symbols.js';
import { type FollowingsRepository, MiUser, type UsersRepository } from '@/models/_.js';
import { bindThis } from '@/decorators.js';
import { sqlLikeEscape } from '@/misc/sql-like-escape.js';
import type { Config } from '@/config.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
import { Packed } from '@/misc/json-schema.js';
function defaultActiveThreshold() {
return new Date(Date.now() - 1000 * 60 * 60 * 24 * 30);
}
@Injectable()
export class UserSearchService {
constructor(
@Inject(DI.config)
private config: Config,
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
@Inject(DI.followingsRepository)
private followingsRepository: FollowingsRepository,
private userEntityService: UserEntityService,
) {
}
/**
* .
*
* - .
* 1.
* 2.
* 3.
* 4.
* - .
* 1.
* 2.
* - .
* - (IDが重複することはないが).
* 12, 3, 4
* - .
* - .
* - .
* - {@link opts.limit} .
*
* {@link params.activeThreshold} .
*
* @param params .
* @param opts .
* @param me . .
* @see {@link UserSearchService#buildSearchUserQueries}
* @see {@link UserSearchService#buildSearchUserNoLoginQueries}
*/
@bindThis
public async search(
params: {
username?: string | null,
host?: string | null,
activeThreshold?: Date,
},
opts?: {
limit?: number,
detail?: boolean,
},
me?: MiUser | null,
): Promise<Packed<'User'>[]> {
const queries = me ? this.buildSearchUserQueries(me, params) : this.buildSearchUserNoLoginQueries(params);
let resultSet = new Set<MiUser['id']>();
const limit = opts?.limit ?? 10;
for (const query of queries) {
const ids = await query
.select('user.id')
.limit(limit - resultSet.size)
.orderBy('user.usernameLower', 'ASC')
.getRawMany<{ user_id: MiUser['id'] }>()
.then(res => res.map(x => x.user_id));
resultSet = new Set([...resultSet, ...ids]);
if (resultSet.size >= limit) {
break;
}
}
return this.userEntityService.packMany<'UserLite' | 'UserDetailed'>(
[...resultSet].slice(0, limit),
me,
{ schema: opts?.detail ? 'UserDetailed' : 'UserLite' },
);
}
/**
* .
* @param me
* @param params
* @private
*/
@bindThis
private buildSearchUserQueries(
me: MiUser,
params: {
username?: string | null,
host?: string | null,
activeThreshold?: Date,
},
) {
// デフォルト30日以内に更新されたユーザーをアクティブユーザーとする
const activeThreshold = params.activeThreshold ?? defaultActiveThreshold();
const followingUserQuery = this.followingsRepository.createQueryBuilder('following')
.select('following.followeeId')
.where('following.followerId = :followerId', { followerId: me.id });
const activeFollowingUsersQuery = this.generateUserQueryBuilder(params)
.andWhere(`user.id IN (${followingUserQuery.getQuery()})`)
.andWhere('user.updatedAt > :activeThreshold', { activeThreshold });
activeFollowingUsersQuery.setParameters(followingUserQuery.getParameters());
const inactiveFollowingUsersQuery = this.generateUserQueryBuilder(params)
.andWhere(`user.id IN (${followingUserQuery.getQuery()})`)
.andWhere(new Brackets(qb => {
qb
.where('user.updatedAt IS NULL')
.orWhere('user.updatedAt <= :activeThreshold', { activeThreshold });
}));
inactiveFollowingUsersQuery.setParameters(followingUserQuery.getParameters());
// 自分自身がヒットするとしたらここ
const activeUserQuery = this.generateUserQueryBuilder(params)
.andWhere(`user.id NOT IN (${followingUserQuery.getQuery()})`)
.andWhere('user.updatedAt > :activeThreshold', { activeThreshold });
activeUserQuery.setParameters(followingUserQuery.getParameters());
const inactiveUserQuery = this.generateUserQueryBuilder(params)
.andWhere(`user.id NOT IN (${followingUserQuery.getQuery()})`)
.andWhere('user.updatedAt <= :activeThreshold', { activeThreshold });
inactiveUserQuery.setParameters(followingUserQuery.getParameters());
return [activeFollowingUsersQuery, inactiveFollowingUsersQuery, activeUserQuery, inactiveUserQuery];
}
/**
* .
* @param params
* @private
*/
@bindThis
private buildSearchUserNoLoginQueries(params: {
username?: string | null,
host?: string | null,
activeThreshold?: Date,
}) {
// デフォルト30日以内に更新されたユーザーをアクティブユーザーとする
const activeThreshold = params.activeThreshold ?? defaultActiveThreshold();
const activeUserQuery = this.generateUserQueryBuilder(params)
.andWhere(new Brackets(qb => {
qb
.where('user.updatedAt IS NULL')
.orWhere('user.updatedAt > :activeThreshold', { activeThreshold });
}));
const inactiveUserQuery = this.generateUserQueryBuilder(params)
.andWhere('user.updatedAt <= :activeThreshold', { activeThreshold });
return [activeUserQuery, inactiveUserQuery];
}
/**
* .
* @param params
* @private
*/
@bindThis
private generateUserQueryBuilder(params: {
username?: string | null,
host?: string | null,
}): SelectQueryBuilder<MiUser> {
const userQuery = this.usersRepository.createQueryBuilder('user');
if (params.username) {
userQuery.andWhere('user.usernameLower LIKE :username', { username: sqlLikeEscape(params.username.toLowerCase()) + '%' });
}
if (params.host) {
if (params.host === this.config.hostname || params.host === '.') {
userQuery.andWhere('user.host IS NULL');
} else {
userQuery.andWhere('user.host LIKE :host', {
host: sqlLikeEscape(params.host.toLowerCase()) + '%',
});
}
}
userQuery.andWhere('user.isSuspended = FALSE');
return userQuery;
}
}

View File

@ -3,15 +3,9 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
import { Brackets } from 'typeorm'; import { Injectable } from '@nestjs/common';
import { Inject, Injectable } from '@nestjs/common';
import type { UsersRepository, FollowingsRepository } from '@/models/_.js';
import type { Config } from '@/config.js';
import type { MiUser } from '@/models/User.js';
import { Endpoint } from '@/server/api/endpoint-base.js'; import { Endpoint } from '@/server/api/endpoint-base.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js'; import { UserSearchService } from '@/core/UserSearchService.js';
import { DI } from '@/di-symbols.js';
import { sqlLikeEscape } from '@/misc/sql-like-escape.js';
export const meta = { export const meta = {
tags: ['users'], tags: ['users'],
@ -49,89 +43,16 @@ export const paramDef = {
@Injectable() @Injectable()
export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-disable-line import/no-default-export export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-disable-line import/no-default-export
constructor( constructor(
@Inject(DI.config) private userSearchService: UserSearchService,
private config: Config,
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
@Inject(DI.followingsRepository)
private followingsRepository: FollowingsRepository,
private userEntityService: UserEntityService,
) { ) {
super(meta, paramDef, async (ps, me) => { super(meta, paramDef, (ps, me) => {
const setUsernameAndHostQuery = (query = this.usersRepository.createQueryBuilder('user')) => { return this.userSearchService.search({
if (ps.username) { username: ps.username,
query.andWhere('user.usernameLower LIKE :username', { username: sqlLikeEscape(ps.username.toLowerCase()) + '%' }); host: ps.host,
} }, {
limit: ps.limit,
if (ps.host) { detail: ps.detail,
if (ps.host === this.config.hostname || ps.host === '.') { }, me);
query.andWhere('user.host IS NULL');
} else {
query.andWhere('user.host LIKE :host', {
host: sqlLikeEscape(ps.host.toLowerCase()) + '%',
});
}
}
return query;
};
const activeThreshold = new Date(Date.now() - (1000 * 60 * 60 * 24 * 30)); // 30日
let users: MiUser[] = [];
if (me) {
const followingQuery = this.followingsRepository.createQueryBuilder('following')
.select('following.followeeId')
.where('following.followerId = :followerId', { followerId: me.id });
const query = setUsernameAndHostQuery()
.andWhere(`user.id IN (${ followingQuery.getQuery() })`)
.andWhere('user.id != :meId', { meId: me.id })
.andWhere('user.isSuspended = FALSE')
.andWhere(new Brackets(qb => {
qb
.where('user.updatedAt IS NULL')
.orWhere('user.updatedAt > :activeThreshold', { activeThreshold: activeThreshold });
}));
query.setParameters(followingQuery.getParameters());
users = await query
.orderBy('user.usernameLower', 'ASC')
.limit(ps.limit)
.getMany();
if (users.length < ps.limit) {
const otherQuery = setUsernameAndHostQuery()
.andWhere(`user.id NOT IN (${ followingQuery.getQuery() })`)
.andWhere('user.isSuspended = FALSE')
.andWhere('user.updatedAt IS NOT NULL');
otherQuery.setParameters(followingQuery.getParameters());
const otherUsers = await otherQuery
.orderBy('user.updatedAt', 'DESC')
.limit(ps.limit - users.length)
.getMany();
users = users.concat(otherUsers);
}
} else {
const query = setUsernameAndHostQuery()
.andWhere('user.isSuspended = FALSE')
.andWhere('user.updatedAt IS NOT NULL');
users = await query
.orderBy('user.updatedAt', 'DESC')
.limit(ps.limit - users.length)
.getMany();
}
return await this.userEntityService.packMany(users, me, { schema: ps.detail ? 'UserDetailed' : 'UserLite' });
}); });
} }
} }

View File

@ -0,0 +1,265 @@
/*
* SPDX-FileCopyrightText: syuilo and misskey-project
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { Test, TestingModule } from '@nestjs/testing';
import { describe, jest, test } from '@jest/globals';
import { In } from 'typeorm';
import { UserSearchService } from '@/core/UserSearchService.js';
import { FollowingsRepository, MiUser, UserProfilesRepository, UsersRepository } from '@/models/_.js';
import { IdService } from '@/core/IdService.js';
import { GlobalModule } from '@/GlobalModule.js';
import { DI } from '@/di-symbols.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
describe('UserSearchService', () => {
let app: TestingModule;
let service: UserSearchService;
let usersRepository: UsersRepository;
let followingsRepository: FollowingsRepository;
let idService: IdService;
let userProfilesRepository: UserProfilesRepository;
let root: MiUser;
let alice: MiUser;
let alyce: MiUser;
let alycia: MiUser;
let alysha: MiUser;
let alyson: MiUser;
let alyssa: MiUser;
let bob: MiUser;
let bobbi: MiUser;
let bobbie: MiUser;
let bobby: MiUser;
async function createUser(data: Partial<MiUser> = {}) {
const user = await usersRepository
.insert({
id: idService.gen(),
...data,
})
.then(x => usersRepository.findOneByOrFail(x.identifiers[0]));
await userProfilesRepository.insert({
userId: user.id,
});
return user;
}
async function createFollowings(follower: MiUser, followees: MiUser[]) {
for (const followee of followees) {
await followingsRepository.insert({
id: idService.gen(),
followerId: follower.id,
followeeId: followee.id,
});
}
}
async function setActive(users: MiUser[]) {
for (const user of users) {
await usersRepository.update(user.id, {
updatedAt: new Date(),
});
}
}
async function setInactive(users: MiUser[]) {
for (const user of users) {
await usersRepository.update(user.id, {
updatedAt: new Date(0),
});
}
}
async function setSuspended(users: MiUser[]) {
for (const user of users) {
await usersRepository.update(user.id, {
isSuspended: true,
});
}
}
beforeAll(async () => {
app = await Test
.createTestingModule({
imports: [
GlobalModule,
],
providers: [
UserSearchService,
{
provide: UserEntityService, useFactory: jest.fn(() => ({
// とりあえずIDが返れば確認が出来るので
packMany: (value: any) => value,
})),
},
IdService,
],
})
.compile();
await app.init();
usersRepository = app.get(DI.usersRepository);
userProfilesRepository = app.get(DI.userProfilesRepository);
followingsRepository = app.get(DI.followingsRepository);
service = app.get(UserSearchService);
idService = app.get(IdService);
});
beforeEach(async () => {
root = await createUser({ username: 'root', usernameLower: 'root', isRoot: true });
alice = await createUser({ username: 'Alice', usernameLower: 'alice' });
alyce = await createUser({ username: 'Alyce', usernameLower: 'alyce' });
alycia = await createUser({ username: 'Alycia', usernameLower: 'alycia' });
alysha = await createUser({ username: 'Alysha', usernameLower: 'alysha' });
alyson = await createUser({ username: 'Alyson', usernameLower: 'alyson', host: 'example.com' });
alyssa = await createUser({ username: 'Alyssa', usernameLower: 'alyssa', host: 'example.com' });
bob = await createUser({ username: 'Bob', usernameLower: 'bob' });
bobbi = await createUser({ username: 'Bobbi', usernameLower: 'bobbi' });
bobbie = await createUser({ username: 'Bobbie', usernameLower: 'bobbie', host: 'example.com' });
bobby = await createUser({ username: 'Bobby', usernameLower: 'bobby', host: 'example.com' });
});
afterEach(async () => {
await usersRepository.delete({});
});
afterAll(async () => {
await app.close();
});
describe('search', () => {
test('フォロー中のアクティブユーザのうち、"al"から始まる人が全員ヒットする', async () => {
await createFollowings(root, [alice, alyce, alycia, alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
await setActive([alice, alyce, alyssa, bob, bobbi, bobbie, bobby]);
await setInactive([alycia, alysha, alyson]);
const result = await service.search(
{ username: 'al' },
{ limit: 100 },
root,
);
// alycia, alysha, alysonは非アクティブなので後ろに行く
expect(result).toEqual([alice, alyce, alyssa, alycia, alysha, alyson].map(x => x.id));
});
test('フォロー中の非アクティブユーザのうち、"al"から始まる人が全員ヒットする', async () => {
await createFollowings(root, [alycia, alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
await setInactive([alice, alyce, alycia, alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
const result = await service.search(
{ username: 'al' },
{ limit: 100 },
root,
);
// alice, alyceはフォローしていないので後ろに行く
expect(result).toEqual([alycia, alysha, alyson, alyssa, alice, alyce].map(x => x.id));
});
test('フォローしていないアクティブユーザのうち、"al"から始まる人が全員ヒットする', async () => {
await setActive([alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
await setInactive([alice, alyce, alycia]);
const result = await service.search(
{ username: 'al' },
{ limit: 100 },
root,
);
// alice, alyce, alyciaは非アクティブなので後ろに行く
expect(result).toEqual([alysha, alyson, alyssa, alice, alyce, alycia].map(x => x.id));
});
test('フォローしていない非アクティブユーザのうち、"al"から始まる人が全員ヒットする', async () => {
await setInactive([alice, alyce, alycia, alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
const result = await service.search(
{ username: 'al' },
{ limit: 100 },
root,
);
expect(result).toEqual([alice, alyce, alycia, alysha, alyson, alyssa].map(x => x.id));
});
test('フォロー(アクティブ)、フォロー(非アクティブ)、非フォロー(アクティブ)、非フォロー(非アクティブ)混在時の優先順位度確認', async () => {
await createFollowings(root, [alyson, alyssa, bob, bobbi, bobbie]);
await setActive([root, alyssa, bob, bobbi, alyce, alycia]);
await setInactive([alyson, alice, alysha, bobbie, bobby]);
const result = await service.search(
{ },
{ limit: 100 },
root,
);
// 見る用
// const users = await usersRepository.findBy({ id: In(result) }).then(it => new Map(it.map(x => [x.id, x])));
// console.log(result.map(x => users.get(x as any)).map(it => it?.username));
// フォローしててアクティブなので先頭: alyssa, bob, bobbi
// フォローしてて非アクティブなので次: alyson, bobbie
// フォローしてないけどアクティブなので次: alyce, alycia, root(アルファベット順的にここになる)
// フォローしてないし非アクティブなので最後: alice, alysha, bobby
expect(result).toEqual([alyssa, bob, bobbi, alyson, bobbie, alyce, alycia, root, alice, alysha, bobby].map(x => x.id));
});
test('[非ログイン] アクティブユーザのうち、"al"から始まる人が全員ヒットする', async () => {
await setActive([alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
await setInactive([alice, alyce, alycia]);
const result = await service.search(
{ username: 'al' },
{ limit: 100 },
);
// alice, alyce, alyciaは非アクティブなので後ろに行く
expect(result).toEqual([alysha, alyson, alyssa, alice, alyce, alycia].map(x => x.id));
});
test('[非ログイン] 非アクティブユーザのうち、"al"から始まる人が全員ヒットする', async () => {
await setInactive([alice, alyce, alycia, alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
const result = await service.search(
{ username: 'al' },
{ limit: 100 },
);
expect(result).toEqual([alice, alyce, alycia, alysha, alyson, alyssa].map(x => x.id));
});
test('フォロー中のアクティブユーザのうち、"al"から始まり"example.com"にいる人が全員ヒットする', async () => {
await createFollowings(root, [alice, alyce, alycia, alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
await setActive([alice, alyce, alycia, alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
const result = await service.search(
{ username: 'al', host: 'exam' },
{ limit: 100 },
root,
);
expect(result).toEqual([alyson, alyssa].map(x => x.id));
});
test('サスペンド済みユーザは出ない', async () => {
await setActive([alice, alyce, alycia, alysha, alyson, alyssa, bob, bobbi, bobbie, bobby]);
await setSuspended([alice, alyce, alycia]);
const result = await service.search(
{ username: 'al' },
{ limit: 100 },
root,
);
expect(result).toEqual([alysha, alyson, alyssa].map(x => x.id));
});
});
});