spec(backend/notes/create): ネットワーク不安定・高負荷時ノートが重複して投稿される問題を修正 (MisskeyIO#432)

This commit is contained in:
まっちゃとーにゅ 2024-02-11 04:02:34 +09:00 committed by GitHub
parent f07a701418
commit 43ef9ca4e2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 51 additions and 6 deletions

View File

@ -3,9 +3,11 @@
* SPDX-License-Identifier: AGPL-3.0-only * SPDX-License-Identifier: AGPL-3.0-only
*/ */
import { createHash } from 'crypto';
import ms from 'ms'; import ms from 'ms';
import { In } from 'typeorm'; import { In } from 'typeorm';
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import * as Redis from 'ioredis';
import type { MiUser } from '@/models/User.js'; import type { MiUser } from '@/models/User.js';
import type { UsersRepository, NotesRepository, BlockingsRepository, DriveFilesRepository, ChannelsRepository } from '@/models/_.js'; import type { UsersRepository, NotesRepository, BlockingsRepository, DriveFilesRepository, ChannelsRepository } from '@/models/_.js';
import type { MiDriveFile } from '@/models/DriveFile.js'; import type { MiDriveFile } from '@/models/DriveFile.js';
@ -48,6 +50,13 @@ export const meta = {
}, },
errors: { errors: {
processing: {
message: 'We are processing your request. Please wait a moment.',
code: 'PROCESSING',
id: '3247052c-005d-440e-b3d8-2a64274483b0',
httpStatusCode: 202,
},
noSuchRenoteTarget: { noSuchRenoteTarget: {
message: 'No such renote target.', message: 'No such renote target.',
code: 'NO_SUCH_RENOTE_TARGET', code: 'NO_SUCH_RENOTE_TARGET',
@ -212,6 +221,9 @@ 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.redisForTimelines)
private redisForTimelines: Redis.Redis,
@Inject(DI.usersRepository) @Inject(DI.usersRepository)
private usersRepository: UsersRepository, private usersRepository: UsersRepository,
@ -231,6 +243,20 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
private noteCreateService: NoteCreateService, private noteCreateService: NoteCreateService,
) { ) {
super(meta, paramDef, async (ps, me) => { super(meta, paramDef, async (ps, me) => {
const hash = createHash('sha256').update(JSON.stringify(ps)).digest('base64');
const idempotent = process.env.FORCE_IGNORE_IDEMPOTENCY_FOR_TESTING !== 'true' ? await this.redisForTimelines.get(`note:idempotent:${me.id}:${hash}`) : null;
if (idempotent === '_') throw new ApiError(meta.errors.processing); // 他のサーバーで処理中
// すでに同じリクエストが処理されている場合、そのノートを返す
// ただし、記録されているノート見つからない場合は、新規として処理を続行
if (idempotent) {
const note = await this.notesRepository.findOneBy({ id: idempotent });
if (note) return { createdNote: await this.noteEntityService.pack(note, me) };
}
// 30秒の間、リクエストを処理中として記録
await this.redisForTimelines.set(`note:idempotent:${me.id}:${hash}`, '_', 'EX', 30);
let visibleUsers: MiUser[] = []; let visibleUsers: MiUser[] = [];
if (ps.visibleUserIds) { if (ps.visibleUserIds) {
visibleUsers = await this.usersRepository.findBy({ visibleUsers = await this.usersRepository.findBy({
@ -371,10 +397,16 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
apEmojis: ps.noExtractEmojis ? [] : undefined, apEmojis: ps.noExtractEmojis ? [] : undefined,
}); });
// 1分間、リクエストの処理結果を記録
await this.redisForTimelines.set(`note:idempotent:${me.id}:${hash}`, note.id, 'EX', 60);
return { return {
createdNote: await this.noteEntityService.pack(note, me), createdNote: await this.noteEntityService.pack(note, me),
}; };
} catch (err) { } catch (err) {
// エラーが発生した場合、リクエストの処理結果を削除
await this.redisForTimelines.unlink(`note:idempotent:${me.id}:${hash}`);
if (err instanceof IdentifiableError) { if (err instanceof IdentifiableError) {
if (err.id === '057d8d3e-b7ca-4f8b-b38c-dcdcbf34dc30') throw new ApiError(meta.errors.containsProhibitedWords); if (err.id === '057d8d3e-b7ca-4f8b-b38c-dcdcbf34dc30') throw new ApiError(meta.errors.containsProhibitedWords);
} }

View File

@ -13,6 +13,7 @@ import {
failedApiCall, failedApiCall,
post, post,
role, role,
sendEnvUpdateRequest,
signup, signup,
successfulApiCall, successfulApiCall,
testPaginationConsistency, testPaginationConsistency,
@ -74,6 +75,8 @@ describe('アンテナ', () => {
let userMutedByAlice: User; let userMutedByAlice: User;
beforeAll(async () => { beforeAll(async () => {
await sendEnvUpdateRequest({ key: 'FORCE_IGNORE_IDEMPOTENCY_FOR_TESTING', value: 'true' });
root = await signup({ username: 'root' }); root = await signup({ username: 'root' });
alice = await signup({ username: 'alice' }); alice = await signup({ username: 'alice' });
alicePost = await post(alice, { text: 'test' }); alicePost = await post(alice, { text: 'test' });

View File

@ -8,7 +8,7 @@ process.env.NODE_ENV = 'test';
import * as assert from 'assert'; import * as assert from 'assert';
import { MiNote } from '@/models/Note.js'; import { MiNote } from '@/models/Note.js';
import { MAX_NOTE_TEXT_LENGTH } from '@/const.js'; import { MAX_NOTE_TEXT_LENGTH } from '@/const.js';
import { api, initTestDb, post, signup, uploadFile, uploadUrl } from '../utils.js'; import { api, initTestDb, post, sendEnvUpdateRequest, signup, uploadFile, uploadUrl } from '../utils.js';
import type * as misskey from 'misskey-js'; import type * as misskey from 'misskey-js';
describe('Note', () => { describe('Note', () => {
@ -19,6 +19,8 @@ describe('Note', () => {
let tom: misskey.entities.SignupResponse; let tom: misskey.entities.SignupResponse;
beforeAll(async () => { beforeAll(async () => {
await sendEnvUpdateRequest({ key: 'FORCE_IGNORE_IDEMPOTENCY_FOR_TESTING', value: 'true' });
const connection = await initTestDb(true); const connection = await initTestDb(true);
Notes = connection.getRepository(MiNote); Notes = connection.getRepository(MiNote);
alice = await signup({ username: 'alice' }); alice = await signup({ username: 'alice' });

View File

@ -6,7 +6,7 @@
process.env.NODE_ENV = 'test'; process.env.NODE_ENV = 'test';
import * as assert from 'assert'; import * as assert from 'assert';
import { api, post, signup, sleep, waitFire } from '../utils.js'; import { api, post, sendEnvUpdateRequest, signup, sleep, waitFire } from '../utils.js';
import type * as misskey from 'misskey-js'; import type * as misskey from 'misskey-js';
describe('Renote Mute', () => { describe('Renote Mute', () => {
@ -16,6 +16,8 @@ describe('Renote Mute', () => {
let carol: misskey.entities.SignupResponse; let carol: misskey.entities.SignupResponse;
beforeAll(async () => { beforeAll(async () => {
await sendEnvUpdateRequest({ key: 'FORCE_IGNORE_IDEMPOTENCY_FOR_TESTING', value: 'true' });
alice = await signup({ username: 'alice' }); alice = await signup({ username: 'alice' });
bob = await signup({ username: 'bob' }); bob = await signup({ username: 'bob' });
carol = await signup({ username: 'carol' }); carol = await signup({ username: 'carol' });

View File

@ -8,7 +8,7 @@ process.env.NODE_ENV = 'test';
import * as assert from 'assert'; import * as assert from 'assert';
import { WebSocket } from 'ws'; import { WebSocket } from 'ws';
import { MiFollowing } from '@/models/Following.js'; import { MiFollowing } from '@/models/Following.js';
import { api, createAppToken, initTestDb, port, post, signup, waitFire } from '../utils.js'; import { api, createAppToken, initTestDb, port, post, sendEnvUpdateRequest, signup, waitFire } from '../utils.js';
import type * as misskey from 'misskey-js'; import type * as misskey from 'misskey-js';
describe('Streaming', () => { describe('Streaming', () => {
@ -46,6 +46,8 @@ describe('Streaming', () => {
let list: any; let list: any;
beforeAll(async () => { beforeAll(async () => {
await sendEnvUpdateRequest({ key: 'FORCE_IGNORE_IDEMPOTENCY_FOR_TESTING', value: 'true' });
const connection = await initTestDb(true); const connection = await initTestDb(true);
Followings = connection.getRepository(MiFollowing); Followings = connection.getRepository(MiFollowing);

View File

@ -12,11 +12,12 @@ import WebSocket, { ClientOptions } from 'ws';
import fetch, { File, RequestInit } from 'node-fetch'; import fetch, { File, RequestInit } from 'node-fetch';
import { DataSource } from 'typeorm'; import { DataSource } from 'typeorm';
import { JSDOM } from 'jsdom'; import { JSDOM } from 'jsdom';
import * as Redis from 'ioredis';
import { DEFAULT_POLICIES } from '@/core/RoleService.js'; import { DEFAULT_POLICIES } from '@/core/RoleService.js';
import { entities } from '../src/postgres.js';
import { loadConfig } from '../src/config.js';
import type * as misskey from 'misskey-js';
import { Packed } from '@/misc/json-schema.js'; import { Packed } from '@/misc/json-schema.js';
import { entities } from '@/postgres.js';
import { loadConfig } from '@/config.js';
import type * as misskey from 'misskey-js';
export { server as startServer, jobQueue as startJobQueue } from '@/boot/common.js'; export { server as startServer, jobQueue as startJobQueue } from '@/boot/common.js';
@ -584,6 +585,9 @@ export async function testPaginationConsistency<Entity extends { id: string, cre
export async function initTestDb(justBorrow = false, initEntities?: any[]) { export async function initTestDb(justBorrow = false, initEntities?: any[]) {
if (process.env.NODE_ENV !== 'test') throw new Error('NODE_ENV is not a test'); if (process.env.NODE_ENV !== 'test') throw new Error('NODE_ENV is not a test');
const redis = new Redis.Redis(config.redis);
await redis.flushdb();
const db = new DataSource({ const db = new DataSource({
type: 'postgres', type: 'postgres',
host: config.db.host, host: config.db.host,