refactor(backend): UserBlockingService <-> UserFollowingService 双方向循環を解消

packages/backend/src/core/CoreModule.ts

@@ -57,6 +57,7 @@ import { S3Service } from './S3Service.js';
 import { SignupService } from './SignupService.js';
 import { WebAuthnService } from './WebAuthnService.js';
 import { UserBlockingService } from './UserBlockingService.js';
+import { BlockingDataAccessService } from './data-access/BlockingDataAccessService.js';
 import { CacheService } from './CacheService.js';
 import { UserService } from './UserService.js';
 import { UserFollowingService } from './UserFollowingService.js';
@@ -201,6 +202,7 @@ const $S3Service: Provider = { provide: 'S3Service', useExisting: S3Service };
 const $SignupService: Provider = { provide: 'SignupService', useExisting: SignupService };
 const $WebAuthnService: Provider = { provide: 'WebAuthnService', useExisting: WebAuthnService };
 const $UserBlockingService: Provider = { provide: 'UserBlockingService', useExisting: UserBlockingService };
+const $BlockingDataAccessService: Provider = { provide: 'BlockingDataAccessService', useExisting: BlockingDataAccessService };
 const $CacheService: Provider = { provide: 'CacheService', useExisting: CacheService };
 const $UserService: Provider = { provide: 'UserService', useExisting: UserService };
 const $UserFollowingService: Provider = { provide: 'UserFollowingService', useExisting: UserFollowingService };
@@ -354,6 +356,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		SignupService,
 		WebAuthnService,
 		UserBlockingService,
+		BlockingDataAccessService,
 		CacheService,
 		UserService,
 		UserFollowingService,
@@ -504,6 +507,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		$SignupService,
 		$WebAuthnService,
 		$UserBlockingService,
+		$BlockingDataAccessService,
 		$CacheService,
 		$UserService,
 		$UserFollowingService,
@@ -654,6 +658,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		SignupService,
 		WebAuthnService,
 		UserBlockingService,
+		BlockingDataAccessService,
 		CacheService,
 		UserService,
 		UserFollowingService,
@@ -803,6 +808,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		$SignupService,
 		$WebAuthnService,
 		$UserBlockingService,
+		$BlockingDataAccessService,
 		$CacheService,
 		$UserService,
 		$UserFollowingService,

packages/backend/src/core/UserBlockingService.ts

@@ -3,60 +3,47 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
-import { Inject, Injectable, OnModuleInit } from '@nestjs/common';
-import { ModuleRef } from '@nestjs/core';
-import { IdService } from '@/core/IdService.js';
+import { Inject, Injectable } from '@nestjs/common';
 import type { MiUser } from '@/models/User.js';
-import type { MiBlocking } from '@/models/Blocking.js';
 import { QueueService } from '@/core/QueueService.js';
 import { GlobalEventService } from '@/core/GlobalEventService.js';
 import { DI } from '@/di-symbols.js';
-import type { FollowRequestsRepository, BlockingsRepository, UserListsRepository, UserListMembershipsRepository } from '@/models/_.js';
+import type { FollowRequestsRepository, UserListsRepository, UserListMembershipsRepository } from '@/models/_.js';
 import Logger from '@/logger.js';
 import { UserEntityService } from '@/core/entities/UserEntityService.js';
 import { ApRendererService } from '@/core/activitypub/ApRendererService.js';
 import { LoggerService } from '@/core/LoggerService.js';
 import { UserWebhookService } from '@/core/UserWebhookService.js';
 import { bindThis } from '@/decorators.js';
-import { CacheService } from '@/core/CacheService.js';
+import { BlockingDataAccessService } from '@/core/data-access/BlockingDataAccessService.js';
 import { UserFollowingService } from '@/core/UserFollowingService.js';
 
 @Injectable()
-export class UserBlockingService implements OnModuleInit {
+export class UserBlockingService {
 	private logger: Logger;
-	private userFollowingService: UserFollowingService;
 
 	constructor(
-		private moduleRef: ModuleRef,
-
 		@Inject(DI.followRequestsRepository)
 		private followRequestsRepository: FollowRequestsRepository,
 
-		@Inject(DI.blockingsRepository)
-		private blockingsRepository: BlockingsRepository,
-
 		@Inject(DI.userListsRepository)
 		private userListsRepository: UserListsRepository,
 
 		@Inject(DI.userListMembershipsRepository)
 		private userListMembershipsRepository: UserListMembershipsRepository,
 
-		private cacheService: CacheService,
+		private blockingDataAccessService: BlockingDataAccessService,
 		private userEntityService: UserEntityService,
-		private idService: IdService,
 		private queueService: QueueService,
 		private globalEventService: GlobalEventService,
 		private webhookService: UserWebhookService,
 		private apRendererService: ApRendererService,
 		private loggerService: LoggerService,
+		private userFollowingService: UserFollowingService,
 	) {
 		this.logger = this.loggerService.getLogger('user-block');
 	}
 
-	onModuleInit() {
-		this.userFollowingService = this.moduleRef.get('UserFollowingService');
-	}
-
 	@bindThis
 	public async block(blocker: MiUser, blockee: MiUser, silent = false) {
 		await Promise.all([
@@ -67,24 +54,15 @@ export class UserBlockingService implements OnModuleInit {
 			this.removeFromList(blockee, blocker),
 		]);
 
-		const blocking = {
-			id: this.idService.gen(),
-			blocker,
-			blockerId: blocker.id,
-			blockee,
-			blockeeId: blockee.id,
-		} as MiBlocking;
-
-		await this.blockingsRepository.insert(blocking);
-
-		this.cacheService.userBlockingCache.refresh(blocker.id);
-		this.cacheService.userBlockedCache.refresh(blockee.id);
-
-		this.globalEventService.publishInternalEvent('blockingCreated', {
+		const blocking = await this.blockingDataAccessService.createBlocking({
 			blockerId: blocker.id,
 			blockeeId: blockee.id,
 		});
 
+		// AP renderer に渡すため relation を補完
+		blocking.blocker = blocker;
+		blocking.blockee = blockee;
+
 		if (this.userEntityService.isLocalUser(blocker) && this.userEntityService.isRemoteUser(blockee)) {
 			const content = this.apRendererService.addContext(this.apRendererService.renderBlock(blocking));
 			this.queueService.deliver(blocker, content, blockee.inbox, false);
@@ -151,10 +129,7 @@ export class UserBlockingService implements OnModuleInit {
 
 	@bindThis
 	public async unblock(blocker: MiUser, blockee: MiUser) {
-		const blocking = await this.blockingsRepository.findOneBy({
-			blockerId: blocker.id,
-			blockeeId: blockee.id,
-		});
+		const blocking = await this.blockingDataAccessService.findBlocking(blocker.id, blockee.id);
 
 		if (blocking == null) {
 			this.logger.warn('ブロック解除がリクエストされましたがブロックしていませんでした');
@@ -166,15 +141,7 @@ export class UserBlockingService implements OnModuleInit {
 		blocking.blocker = blocker;
 		blocking.blockee = blockee;
 
-		await this.blockingsRepository.delete(blocking.id);
-
-		this.cacheService.userBlockingCache.refresh(blocker.id);
-		this.cacheService.userBlockedCache.refresh(blockee.id);
-
-		this.globalEventService.publishInternalEvent('blockingDeleted', {
-			blockerId: blocker.id,
-			blockeeId: blockee.id,
-		});
+		await this.blockingDataAccessService.deleteBlocking(blocking.id, blocker.id, blockee.id);
 
 		// deliver if remote bloking
 		if (this.userEntityService.isLocalUser(blocker) && this.userEntityService.isRemoteUser(blockee)) {
@@ -183,8 +150,11 @@ export class UserBlockingService implements OnModuleInit {
 		}
 	}
 
+	// TODO: 呼び出し側 (PollService / ReactionService / notes/polls/vote.ts /
+	// ChatService / NoteCreateService 等) を BlockingDataAccessService.isBlocking に
+	// 直接置換し、このラッパを削除する
 	@bindThis
-	public async checkBlocked(blockerId: MiUser['id'], blockeeId: MiUser['id']): Promise<boolean> {
-		return (await this.cacheService.userBlockingCache.fetch(blockerId)).has(blockeeId);
+	public checkBlocked(blockerId: MiUser['id'], blockeeId: MiUser['id']): Promise<boolean> {
+		return this.blockingDataAccessService.isBlocking(blockerId, blockeeId);
 	}
 }

packages/backend/src/core/UserFollowingService.ts

@@ -3,8 +3,7 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
-import { Inject, Injectable, OnModuleInit } from '@nestjs/common';
-import { ModuleRef } from '@nestjs/core';
+import { Inject, Injectable } from '@nestjs/common';
 import { Brackets, IsNull } from 'typeorm';
 import type { MiLocalUser, MiPartialLocalUser, MiPartialRemoteUser, MiRemoteUser, MiUser } from '@/models/User.js';
 import { IdentifiableError } from '@/misc/identifiable-error.js';
@@ -22,7 +21,7 @@ import type { FollowingsRepository, FollowRequestsRepository, InstancesRepositor
 import { UserEntityService } from '@/core/entities/UserEntityService.js';
 import { ApRendererService } from '@/core/activitypub/ApRendererService.js';
 import { bindThis } from '@/decorators.js';
-import { UserBlockingService } from '@/core/UserBlockingService.js';
+import { BlockingDataAccessService } from '@/core/data-access/BlockingDataAccessService.js';
 import { CacheService } from '@/core/CacheService.js';
 import type { Config } from '@/config.js';
 import { AccountMoveService } from '@/core/AccountMoveService.js';
@@ -46,12 +45,8 @@ type Remote = MiRemoteUser | {
 type Both = Local | Remote;
 
 @Injectable()
-export class UserFollowingService implements OnModuleInit {
-	private userBlockingService: UserBlockingService;
-
+export class UserFollowingService {
 	constructor(
-		private moduleRef: ModuleRef,
-
 		@Inject(DI.config)
 		private config: Config,
 
@@ -86,13 +81,10 @@ export class UserFollowingService implements OnModuleInit {
 		private accountMoveService: AccountMoveService,
 		private perUserFollowingChart: PerUserFollowingChart,
 		private instanceChart: InstanceChart,
+		private blockingDataAccessService: BlockingDataAccessService,
 	) {
 	}
 
-	onModuleInit() {
-		this.userBlockingService = this.moduleRef.get('UserBlockingService');
-	}
-
 	@bindThis
 	public async deliverAccept(follower: MiRemoteUser, followee: MiPartialLocalUser, requestId?: string) {
 		const content = this.apRendererService.addContext(this.apRendererService.renderAccept(this.apRendererService.renderFollow(follower, followee, requestId), followee));
@@ -124,8 +116,8 @@ export class UserFollowingService implements OnModuleInit {
 
 		// check blocking
 		const [blocking, blocked] = await Promise.all([
-			this.userBlockingService.checkBlocked(follower.id, followee.id),
-			this.userBlockingService.checkBlocked(followee.id, follower.id),
+			this.blockingDataAccessService.isBlocking(follower.id, followee.id),
+			this.blockingDataAccessService.isBlocking(followee.id, follower.id),
 		]);
 
 		if (this.userEntityService.isRemoteUser(follower) && this.userEntityService.isLocalUser(followee) && blocked) {
@@ -135,7 +127,7 @@ export class UserFollowingService implements OnModuleInit {
 			return;
 		} else if (this.userEntityService.isRemoteUser(follower) && this.userEntityService.isLocalUser(followee) && blocking) {
 			// リモートフォローを受けてブロックされているはずの場合だったら、ブロック解除しておく。
-			await this.userBlockingService.unblock(follower, followee);
+			await this.blockingDataAccessService.deleteBlockingBetween(follower.id, followee.id);
 		} else {
 			// それ以外は単純に例外
 			if (blocking) throw new IdentifiableError('710e8fb0-b8c3-4922-be49-d5d93d8e6a6e', 'blocking');
@@ -493,8 +485,8 @@ export class UserFollowingService implements OnModuleInit {
 
 		// check blocking
 		const [blocking, blocked] = await Promise.all([
-			this.userBlockingService.checkBlocked(follower.id, followee.id),
-			this.userBlockingService.checkBlocked(followee.id, follower.id),
+			this.blockingDataAccessService.isBlocking(follower.id, followee.id),
+			this.blockingDataAccessService.isBlocking(followee.id, follower.id),
 		]);
 
 		if (blocking) throw new Error('blocking');

packages/backend/src/core/data-access/BlockingDataAccessService.ts

@@ -0,0 +1,95 @@
+/*
+ * SPDX-FileCopyrightText: syuilo and misskey-project
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import { Inject, Injectable } from '@nestjs/common';
+import { DI } from '@/di-symbols.js';
+import type { BlockingsRepository } from '@/models/_.js';
+import type { MiBlocking } from '@/models/Blocking.js';
+import type { MiUser } from '@/models/User.js';
+import { CacheService } from '@/core/CacheService.js';
+import { GlobalEventService } from '@/core/GlobalEventService.js';
+import { IdService } from '@/core/IdService.js';
+import { bindThis } from '@/decorators.js';
+
+/**
+ * Blocking ドメインに対する DB / キャッシュアクセスを束ねた薄いデータアクセス層。
+ * また、キャッシュの鮮度を保つ為の更新処理や、更新イベントの発行もデータアクセス層の責務に含める。
+ */
+@Injectable()
+export class BlockingDataAccessService {
+	constructor(
+		@Inject(DI.blockingsRepository)
+		private blockingsRepository: BlockingsRepository,
+
+		private idService: IdService,
+		private cacheService: CacheService,
+		private globalEventService: GlobalEventService,
+	) {
+	}
+
+	@bindThis
+	public async isBlocking(blockerId: MiUser['id'], blockeeId: MiUser['id']): Promise<boolean> {
+		return (await this.cacheService.userBlockingCache.fetch(blockerId)).has(blockeeId);
+	}
+
+	@bindThis
+	public findBlocking(blockerId: MiUser['id'], blockeeId: MiUser['id']): Promise<MiBlocking | null> {
+		return this.blockingsRepository.findOneBy({
+			blockerId,
+			blockeeId,
+		});
+	}
+
+	@bindThis
+	public async createBlocking(input: Pick<MiBlocking, 'blockerId' | 'blockeeId'>): Promise<MiBlocking> {
+		const blocking: MiBlocking = {
+			id: this.idService.gen(),
+			blocker: null,
+			blockee: null,
+			...input,
+		};
+
+		await this.blockingsRepository.insert(blocking);
+
+		this.cacheService.userBlockingCache.refresh(blocking.blockerId);
+		this.cacheService.userBlockedCache.refresh(blocking.blockeeId);
+
+		this.globalEventService.publishInternalEvent('blockingCreated', {
+			blockerId: blocking.blockerId,
+			blockeeId: blocking.blockeeId,
+		});
+
+		return blocking;
+	}
+
+	@bindThis
+	public async deleteBlocking(blockingId: MiBlocking['id'], blockerId: MiUser['id'], blockeeId: MiUser['id']): Promise<void> {
+		await this.blockingsRepository.delete(blockingId);
+
+		this.cacheService.userBlockingCache.refresh(blockerId);
+		this.cacheService.userBlockedCache.refresh(blockeeId);
+
+		this.globalEventService.publishInternalEvent('blockingDeleted', {
+			blockerId,
+			blockeeId,
+		});
+	}
+
+	/**
+	 * `(blockerId, blockeeId)` のペアで blocking レコードを探し、見つかれば削除する。
+	 *
+	 * 既存 `MiBlocking` の id を呼び出し元が把握していないケース
+	 * (例: フォロー受信時の古い block 解消) 向けの便利メソッド。
+	 */
+	@bindThis
+	public async deleteBlockingBetween(blockerId: MiUser['id'], blockeeId: MiUser['id']): Promise<void> {
+		const blocking = await this.findBlocking(blockerId, blockeeId);
+		if (blocking == null) {
+			return;
+		}
+
+		await this.deleteBlocking(blocking.id, blockerId, blockeeId);
+	}
+}

packages/backend/test/unit/BlockingDataAccessService.ts

@@ -0,0 +1,31 @@
+/*
+ * SPDX-FileCopyrightText: syuilo and misskey-project
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import * as assert from 'assert';
+import { beforeAll, describe, test } from 'vitest';
+import { Test } from '@nestjs/testing';
+
+import { CoreModule } from '@/core/CoreModule.js';
+import { BlockingDataAccessService } from '@/core/data-access/BlockingDataAccessService.js';
+import { GlobalModule } from '@/GlobalModule.js';
+
+describe('BlockingDataAccessService', () => {
+	let blockingDataAccessService: BlockingDataAccessService;
+
+	beforeAll(async () => {
+		const app = await Test.createTestingModule({
+			imports: [GlobalModule, CoreModule],
+		}).compile();
+		blockingDataAccessService = app.get<BlockingDataAccessService>(BlockingDataAccessService);
+	});
+
+	test('DI コンテナから解決できる', () => {
+		assert.ok(blockingDataAccessService);
+		assert.strictEqual(typeof blockingDataAccessService.isBlocking, 'function');
+		assert.strictEqual(typeof blockingDataAccessService.findBlocking, 'function');
+		assert.strictEqual(typeof blockingDataAccessService.createBlocking, 'function');
+		assert.strictEqual(typeof blockingDataAccessService.deleteBlocking, 'function');
+	});
+});