From aacca10475fa7a8cf25016483458ac28a517ce93 Mon Sep 17 00:00:00 2001 From: Jesse Chan Date: Sun, 9 May 2021 03:34:42 +0800 Subject: [PATCH] server: simplify service manager, fixes dangling ref --- server/middleware/appendUserServices.ts | 8 +- server/middleware/clientActivityStream.ts | 4 +- server/models/Users.ts | 4 +- server/routes/api/auth.ts | 10 +- server/services/BaseService.ts | 6 +- server/services/index.ts | 193 ++++-------------- .../interfaces/clientGatewayService.ts | 14 +- 7 files changed, 72 insertions(+), 167 deletions(-) diff --git a/server/middleware/appendUserServices.ts b/server/middleware/appendUserServices.ts index 6f5b8684..6b874ba1 100644 --- a/server/middleware/appendUserServices.ts +++ b/server/middleware/appendUserServices.ts @@ -1,12 +1,14 @@ import type {Request, Response, NextFunction} from 'express'; -import services from '../services'; +import {getAllServices} from '../services'; + +import type {ServiceInstances} from '../services'; declare global { // eslint-disable-next-line @typescript-eslint/no-namespace namespace Express { interface Request { - services: ReturnType; + services: ServiceInstances; } } } @@ -20,7 +22,7 @@ export default (req: Request, res: Response, next: NextFunction) => { return failedInitializeResponse(res); } - req.services = services.getAllServices(req.user); + req.services = getAllServices(req.user); if (req.services?.clientGatewayService == null) { return failedInitializeResponse(res); } diff --git a/server/middleware/clientActivityStream.ts b/server/middleware/clientActivityStream.ts index 9b139b76..f0ad0e8d 100644 --- a/server/middleware/clientActivityStream.ts +++ b/server/middleware/clientActivityStream.ts @@ -5,8 +5,8 @@ import type TypedEmitter from 'typed-emitter'; import type {HistorySnapshot} from '@shared/constants/historySnapshotTypes'; import DiskUsage from '../models/DiskUsage'; +import {getAllServices} from '../services'; import ServerEvent from '../models/ServerEvent'; -import services from '../services'; import type {DiskUsageSummary} from '../models/DiskUsage'; import type {TransferHistory} from '../../shared/types/TransferData'; @@ -21,7 +21,7 @@ export default async (req: Request { return this.listUsers() - .then((users) => Promise.all(users.map((user) => services.bootstrapServicesForUser(user)))) + .then((users) => Promise.all(users.map((user) => bootstrapServicesForUser(user)))) .then(() => undefined); } diff --git a/server/routes/api/auth.ts b/server/routes/api/auth.ts index 99477e6f..238a4e79 100644 --- a/server/routes/api/auth.ts +++ b/server/routes/api/auth.ts @@ -10,10 +10,10 @@ import { authUpdateUserSchema, AuthVerificationPreloadConfigs, } from '../../../shared/schema/api/auth'; +import {bootstrapServicesForUser, destroyUserServices} from '../../services'; import config from '../../../config'; import {getAuthToken, getCookieOptions} from '../../util/authUtil'; import requireAdmin from '../../middleware/requireAdmin'; -import services from '../../services'; import Users from '../../models/Users'; import type { @@ -155,7 +155,7 @@ router.post( // Attempt to save the user return Users.createUser(credentials).then( (user) => { - services.bootstrapServicesForUser(user); + bootstrapServicesForUser(user); if (req.query.cookie === 'false') { return res.status(200).json({username: user.username}); @@ -306,7 +306,7 @@ router.delete( async (req, res): Promise => { return Users.removeUser(req.params.username) .then((id) => { - services.destroyUserServices(id); + destroyUserServices(id); return res.json({username: req.params.username}); }) .catch(({code, message}) => res.status(500).json({code, message})); @@ -341,8 +341,8 @@ router.patch<{username: Credentials['username']}, unknown, AuthUpdateUserOptions return Users.updateUser(username, patch) .then((newUsername) => { return Users.lookupUser(newUsername).then((user) => { - services.destroyUserServices(user._id); - services.bootstrapServicesForUser(user); + destroyUserServices(user._id); + bootstrapServicesForUser(user); return res.status(200).json({}); }); }) diff --git a/server/services/BaseService.ts b/server/services/BaseService.ts index fe7787a9..df3ce3fe 100644 --- a/server/services/BaseService.ts +++ b/server/services/BaseService.ts @@ -3,13 +3,13 @@ import type TypedEmitter from 'typed-emitter'; import type {UserInDatabase} from '@shared/schema/Auth'; -import type {UserServices} from '.'; +import type {ServiceInstances} from '.'; class BaseService extends (EventEmitter as { new (): TypedEmitter; }) { user: UserInDatabase; - services?: UserServices; + services?: ServiceInstances; constructor(user: UserInDatabase) { super(); @@ -28,7 +28,7 @@ class BaseService extends (EventEmitter as { this.user = user; } - updateServices(service?: UserServices) { + updateServices(service?: ServiceInstances) { this.services = service; this.onServicesUpdated(); } diff --git a/server/services/index.ts b/server/services/index.ts index 23f3d45d..e0ce7752 100644 --- a/server/services/index.ts +++ b/server/services/index.ts @@ -1,6 +1,5 @@ import type {UserInDatabase} from '@shared/schema/Auth'; -import BaseService from './BaseService'; import ClientGatewayService from './interfaces/clientGatewayService'; import FeedService from './feedService'; import HistoryService from './historyService'; @@ -13,171 +12,65 @@ import QBittorrentClientGatewayService from './qBittorrent/clientGatewayService' import RTorrentClientGatewayService from './rTorrent/clientGatewayService'; import TransmissionClientGatewayService from './Transmission/clientGatewayService'; -type ClientGatewayServiceImpl = typeof ClientGatewayService & { - new (...args: ConstructorParameters): - | QBittorrentClientGatewayService - | RTorrentClientGatewayService - | TransmissionClientGatewayService; -}; +export interface ServiceInstances { + clientGatewayService: ClientGatewayService; + feedService: FeedService; + historyService: HistoryService; + notificationService: NotificationService; + settingService: SettingService; + taxonomyService: TaxonomyService; + torrentService: TorrentService; +} -type Service = - | ClientGatewayServiceImpl - | typeof FeedService - | typeof HistoryService - | typeof NotificationService - | typeof SettingService - | typeof TaxonomyService - | typeof TorrentService; +const serviceInstances: Record = {}; -const serviceInstances: { - clientGatewayServices: Record; - feedServices: Record; - historyServices: Record; - notificationServices: Record; - settingServices: Record; - taxonomyServices: Record; - torrentServices: Record; -} = { - clientGatewayServices: {}, - feedServices: {}, - historyServices: {}, - notificationServices: {}, - settingServices: {}, - taxonomyServices: {}, - torrentServices: {}, -}; - -type ServiceMap = keyof typeof serviceInstances; - -const getService = (servicesMap: ServiceMap, Service: S, user: UserInDatabase): InstanceType => { - // if a service instance for user exists, return it - const serviceInstance = serviceInstances[servicesMap][user._id]; - if (serviceInstance != null) { - return serviceInstance as InstanceType; - } - - // otherwise, create a new service instance and return it - const newInstance = new Service(user) as InstanceType; - serviceInstances[servicesMap][user._id] = newInstance; - return newInstance; -}; - -const getClientGatewayService = (user: UserInDatabase): ClientGatewayService | undefined => { +const newClientGatewayService = (user: UserInDatabase): ClientGatewayService => { switch (user.client.client) { case 'qBittorrent': - return getService('clientGatewayServices', QBittorrentClientGatewayService, user); + return new QBittorrentClientGatewayService(user); case 'rTorrent': - return getService('clientGatewayServices', RTorrentClientGatewayService, user); + return new RTorrentClientGatewayService(user); case 'Transmission': - return getService('clientGatewayServices', TransmissionClientGatewayService, user); - default: - return undefined; + return new TransmissionClientGatewayService(user); } }; -const getFeedService = (user: UserInDatabase): FeedService => { - return getService('feedServices', FeedService, user); +export const getAllServices = ({_id}: UserInDatabase) => { + return serviceInstances[_id]; }; -const getHistoryService = (user: UserInDatabase): HistoryService => { - return getService('historyServices', HistoryService, user); -}; - -const getNotificationService = (user: UserInDatabase): NotificationService => { - return getService('notificationServices', NotificationService, user); -}; - -const getSettingService = (user: UserInDatabase): SettingService => { - return getService('settingServices', SettingService, user); -}; - -const getTaxonomyService = (user: UserInDatabase): TaxonomyService => { - return getService('taxonomyServices', TaxonomyService, user); -}; - -const getTorrentService = (user: UserInDatabase): TorrentService => { - return getService('torrentServices', TorrentService, user); -}; - -const getAllServices = (user: UserInDatabase) => - ({ - get clientGatewayService(): ClientGatewayService { - return getClientGatewayService(user) as ClientGatewayService; - }, - - get feedService() { - return getFeedService(user); - }, - - get historyService() { - return getHistoryService(user); - }, - - get notificationService() { - return getNotificationService(user); - }, - - get settingService() { - return getSettingService(user); - }, - - get taxonomyService() { - return getTaxonomyService(user); - }, - - get torrentService() { - return getTorrentService(user); - }, - } as const); - -const createUserServices = (user: UserInDatabase): boolean => { - return !Object.values(getAllServices(user)).some((service) => { - if (service == null) { - return true; - } - return false; +export const destroyUserServices = (userId: UserInDatabase['_id']) => { + const userServiceInstances = serviceInstances[userId]; + delete serviceInstances[userId]; + Object.keys(userServiceInstances).forEach((key) => { + const serviceName = key as keyof ServiceInstances; + userServiceInstances[serviceName].destroy(); }); }; -const destroyUserServices = (userId: UserInDatabase['_id']) => { - Object.keys(serviceInstances).forEach((key) => { - const serviceMap = key as keyof typeof serviceInstances; - const userService = serviceInstances[serviceMap][userId]; - if (userService != null) { - delete serviceInstances[serviceMap][userId]; - userService.destroy(); - } - }); -}; +export const bootstrapServicesForUser = (user: UserInDatabase) => { + const {_id} = user; -const linkUserServices = (user: UserInDatabase) => { - Object.keys(serviceInstances).forEach((key) => { - const serviceMap = key as ServiceMap; - const service = serviceInstances[serviceMap][user._id]; - if (service != null) { - service.updateServices(getAllServices(user)); - } - }); -}; - -const bootstrapServicesForUser = (user: UserInDatabase) => { - if (createUserServices(user) === false) { - console.error(`Failed to initialize services for user ${user.username}`); - return; + if (serviceInstances[_id] != null) { + destroyUserServices(_id); } - linkUserServices(user); -}; -export type UserServices = ReturnType; + const userServiceInstances = { + clientGatewayService: newClientGatewayService(user), + feedService: new FeedService(user), + historyService: new HistoryService(user), + notificationService: new NotificationService(user), + settingService: new SettingService(user), + taxonomyService: new TaxonomyService(user), + torrentService: new TorrentService(user), + }; -export default { - bootstrapServicesForUser, - destroyUserServices, - getAllServices, - getClientGatewayService, - getHistoryService, - getNotificationService, - getSettingService, - getTaxonomyService, - getTorrentService, + Object.keys(userServiceInstances).forEach((key) => { + const serviceName = key as keyof ServiceInstances; + if (userServiceInstances[serviceName] != null) { + userServiceInstances[serviceName].updateServices(userServiceInstances); + } + }); + + serviceInstances[_id] = userServiceInstances; }; diff --git a/server/services/interfaces/clientGatewayService.ts b/server/services/interfaces/clientGatewayService.ts index 0dbeafe0..c192faa0 100644 --- a/server/services/interfaces/clientGatewayService.ts +++ b/server/services/interfaces/clientGatewayService.ts @@ -23,6 +23,7 @@ import type {TorrentListSummary, TorrentProperties} from '@shared/types/Torrent' import type {TorrentPeer} from '@shared/types/TorrentPeer'; import type {TorrentTracker} from '@shared/types/TorrentTracker'; import type {TransferSummary} from '@shared/types/TransferData'; +import type {UserInDatabase} from '@shared/schema/Auth'; import BaseService from '../BaseService'; import config from '../../../config'; @@ -36,7 +37,7 @@ interface ClientGatewayServiceEvents { abstract class ClientGatewayService extends BaseService { errorCount = 0; - retryTimer: NodeJS.Timeout | null = null; + retryTimer?: NodeJS.Timeout | null = null; /** * Adds torrents by file @@ -213,6 +214,14 @@ abstract class ClientGatewayService extends BaseService; + constructor(user: UserInDatabase) { + super(user); + + this.testGateway() + .then(this.processClientRequestSuccess, this.processClientRequestError) + .catch(() => undefined); + } + destroyTimer() { if (this.retryTimer != null) { clearTimeout(this.retryTimer); @@ -222,11 +231,12 @@ abstract class ClientGatewayService extends BaseService { this.errorCount += 1; this.destroyTimer();