server: simplify service manager, fixes dangling ref

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<typeof services['getAllServices']>;
+      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);
   }

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<unknown, unknown, unknown, {historySnapshot:
     return;
   }
 
-  const serviceInstances = services.getAllServices(user);
+  const serviceInstances = getAllServices(user);
   const serverEvent = new ServerEvent(res);
   const fetchTorrentList = serviceInstances.torrentService.fetchTorrentList();
 

server/models/Users.ts

@@ -6,7 +6,7 @@ import path from 'path';
 
 import {AccessLevel} from '../../shared/schema/constants/Auth';
 import config from '../../config';
-import services from '../services';
+import {bootstrapServicesForUser} from '../services';
 
 import type {ClientConnectionSettings} from '../../shared/schema/ClientConnectionSettings';
 import type {Credentials, UserInDatabase} from '../../shared/schema/Auth';
@@ -50,7 +50,7 @@ class Users {
 
   async bootstrapServicesForAllUsers(): Promise<void> {
     return this.listUsers()
-      .then((users) => Promise.all(users.map((user) => services.bootstrapServicesForUser(user))))
+      .then((users) => Promise.all(users.map((user) => bootstrapServicesForUser(user))))
       .then(() => undefined);
   }
 

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<unknown, unknown, AuthRegistrationOptions, {cookie: string}>(
     // 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<Response> => {
     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({});
         });
       })

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<E = unknown> extends (EventEmitter as {
   new <T>(): TypedEmitter<T>;
 })<E> {
   user: UserInDatabase;
-  services?: UserServices;
+  services?: ServiceInstances;
 
   constructor(user: UserInDatabase) {
     super();
@@ -28,7 +28,7 @@ class BaseService<E = unknown> extends (EventEmitter as {
     this.user = user;
   }
 
-  updateServices(service?: UserServices) {
+  updateServices(service?: ServiceInstances) {
     this.services = service;
     this.onServicesUpdated();
   }

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<typeof BaseService>):
-    | 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<string, ServiceInstances> = {};
 
-const serviceInstances: {
-  clientGatewayServices: Record<string, ClientGatewayService>;
-  feedServices: Record<string, FeedService>;
-  historyServices: Record<string, HistoryService>;
-  notificationServices: Record<string, NotificationService>;
-  settingServices: Record<string, SettingService>;
-  taxonomyServices: Record<string, TaxonomyService>;
-  torrentServices: Record<string, TorrentService>;
-} = {
-  clientGatewayServices: {},
-  feedServices: {},
-  historyServices: {},
-  notificationServices: {},
-  settingServices: {},
-  taxonomyServices: {},
-  torrentServices: {},
-};
-
-type ServiceMap = keyof typeof serviceInstances;
-
-const getService = <S extends Service>(servicesMap: ServiceMap, Service: S, user: UserInDatabase): InstanceType<S> => {
-  // if a service instance for user exists, return it
-  const serviceInstance = serviceInstances[servicesMap][user._id];
-  if (serviceInstance != null) {
-    return serviceInstance as InstanceType<S>;
-  }
-
-  // otherwise, create a new service instance and return it
-  const newInstance = new Service(user) as InstanceType<S>;
-  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<typeof getAllServices>;
+  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;
 };

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<ClientGatewayServiceEvents> {
   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<ClientGatewayServiceEven
 
   abstract testGateway(): Promise<void>;
 
+  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<ClientGatewayServiceEven
 
   destroy() {
     this.destroyTimer();
+    this.retryTimer = undefined;
     super.destroy();
   }
 
   startTimer() {
-    if (this.retryTimer == null) {
+    if (this.retryTimer === null) {
       this.retryTimer = setTimeout(() => {
         this.errorCount += 1;
         this.destroyTimer();