fix(server): use randomized cron for version check scheduling (#27626)

Also removes unnecessary rate limit

server/src/enum.ts

@@ -848,6 +848,7 @@ export enum AssetVisibility {
 export enum CronJob {
   LibraryScan = 'LibraryScan',
   NightlyJobs = 'NightlyJobs',
+  VersionCheck = 'VersionCheck',
 }
 
 export enum ApiTag {

server/src/services/api.service.ts

@@ -1,14 +1,11 @@
 import { Injectable, NotAcceptableException } from '@nestjs/common';
-import { Interval } from '@nestjs/schedule';
 import { NextFunction, Request, Response } from 'express';
 import { readFileSync } from 'node:fs';
 import sanitizeHtml from 'sanitize-html';
-import { ONE_HOUR } from 'src/constants';
 import { ConfigRepository } from 'src/repositories/config.repository';
 import { LoggingRepository } from 'src/repositories/logging.repository';
 import { AuthService } from 'src/services/auth.service';
 import { SharedLinkService } from 'src/services/shared-link.service';
-import { VersionService } from 'src/services/version.service';
 import { OpenGraphTags } from 'src/utils/misc';
 
 export const render = (index: string, meta: OpenGraphTags) => {
@@ -40,18 +37,12 @@ export class ApiService {
   constructor(
     private authService: AuthService,
     private sharedLinkService: SharedLinkService,
-    private versionService: VersionService,
     private configRepository: ConfigRepository,
     private logger: LoggingRepository,
   ) {
     this.logger.setContext(ApiService.name);
   }
 
-  @Interval(ONE_HOUR.as('milliseconds'))
-  async onVersionCheck() {
-    await this.versionService.handleQueueVersionCheck();
-  }
-
   ssr(excludePaths: string[]) {
     const { resourcePaths } = this.configRepository.getEnv();
 

server/src/services/version.service.spec.ts

@@ -2,7 +2,7 @@ import { DateTime } from 'luxon';
 import { SemVer } from 'semver';
 import { defaults } from 'src/config';
 import { serverVersion } from 'src/constants';
-import { JobName, JobStatus, SystemMetadataKey } from 'src/enum';
+import { CronJob, JobName, JobStatus, SystemMetadataKey } from 'src/enum';
 import { VersionService } from 'src/services/version.service';
 import { factory } from 'test/small.factory';
 import { newTestService, ServiceMocks } from 'test/utils';
@@ -18,6 +18,8 @@ describe(VersionService.name, () => {
 
   beforeEach(() => {
     ({ sut, mocks } = newTestService(VersionService));
+    mocks.cron.create.mockResolvedValue();
+    mocks.cron.update.mockResolvedValue();
   });
 
   it('should work', () => {
@@ -44,6 +46,20 @@ describe(VersionService.name, () => {
       await expect(sut.onBootstrap()).resolves.toBeUndefined();
       expect(mocks.versionHistory.create).not.toHaveBeenCalled();
     });
+
+    it('should create a version check cron job', async () => {
+      mocks.versionHistory.getLatest.mockResolvedValue({
+        id: 'version-1',
+        createdAt: new Date(),
+        version: serverVersion.toString(),
+      });
+      await sut.onBootstrap();
+      expect(mocks.cron.create).toHaveBeenCalledWith(
+        expect.objectContaining({
+          name: CronJob.VersionCheck,
+        }),
+      );
+    });
   });
 
   describe('getVersion', () => {
@@ -72,25 +88,13 @@ describe(VersionService.name, () => {
   });
 
   describe('handVersionCheck', () => {
-    it('should not run if the last check was < 60 minutes ago', async () => {
-      mocks.systemMetadata.get.mockResolvedValue({
-        checkedAt: DateTime.utc().minus({ minutes: 5 }).toISO(),
-        releaseVersion: '1.0.0',
-      });
-      await expect(sut.handleVersionCheck()).resolves.toEqual(JobStatus.Skipped);
-    });
-
     it('should not run if version check is disabled', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ newVersionCheck: { enabled: false } });
       await expect(sut.handleVersionCheck()).resolves.toEqual(JobStatus.Skipped);
     });
 
-    it('should run if it has been > 60 minutes', async () => {
+    it('should run and notify if a new version is available', async () => {
       mocks.serverInfo.getLatestRelease.mockResolvedValue(mockVersionResponse('v100.0.0'));
-      mocks.systemMetadata.get.mockResolvedValue({
-        checkedAt: DateTime.utc().minus({ minutes: 65 }).toISO(),
-        releaseVersion: '1.0.0',
-      });
       await expect(sut.handleVersionCheck()).resolves.toEqual(JobStatus.Success);
       expect(mocks.systemMetadata.set).toHaveBeenCalled();
       expect(mocks.logger.log).toHaveBeenCalled();

server/src/services/version.service.ts

@@ -4,10 +4,11 @@ import semver, { SemVer } from 'semver';
 import { serverVersion } from 'src/constants';
 import { OnEvent, OnJob } from 'src/decorators';
 import { ReleaseNotification, ServerVersionResponseDto } from 'src/dtos/server.dto';
-import { DatabaseLock, JobName, JobStatus, QueueName, SystemMetadataKey } from 'src/enum';
+import { CronJob, DatabaseLock, JobName, JobStatus, QueueName, SystemMetadataKey } from 'src/enum';
 import { ArgOf } from 'src/repositories/event.repository';
 import { BaseService } from 'src/services/base.service';
 import { VersionCheckMetadata } from 'src/types';
+import { handlePromiseError } from 'src/utils/misc';
 
 const asNotification = ({ checkedAt, releaseVersion }: VersionCheckMetadata): ReleaseNotification => {
   return {
@@ -24,6 +25,15 @@ export class VersionService extends BaseService {
   async onBootstrap(): Promise<void> {
     await this.handleVersionCheck();
 
+    const randomMinute = Math.floor(Math.random() * 60);
+    const expression = `${randomMinute} * * * *`;
+    this.logger.debug(`Scheduling version check for cron ${expression}`);
+    this.cronRepository.create({
+      name: CronJob.VersionCheck,
+      expression,
+      onTick: () => handlePromiseError(this.handleQueueVersionCheck(), this.logger),
+    });
+
     await this.databaseRepository.withLock(DatabaseLock.VersionHistory, async () => {
       const previous = await this.versionRepository.getLatest();
       const current = serverVersion.toString();
@@ -76,16 +86,6 @@ export class VersionService extends BaseService {
         return JobStatus.Skipped;
       }
 
-      const versionCheck = await this.systemMetadataRepository.get(SystemMetadataKey.VersionCheckState);
-      if (versionCheck?.checkedAt) {
-        const lastUpdate = DateTime.fromISO(versionCheck.checkedAt);
-        const elapsedTime = DateTime.now().diff(lastUpdate).as('minutes');
-        // check once per hour (max)
-        if (elapsedTime < 60) {
-          return JobStatus.Skipped;
-        }
-      }
-
       const { version: releaseVersion, published_at: publishedAt } = await this.serverInfoRepository.getLatestRelease();
       const metadata: VersionCheckMetadata = { checkedAt: DateTime.utc().toISO(), releaseVersion };
 

server/test/medium.factory.ts

@@ -26,6 +26,7 @@ import { AssetEditRepository } from 'src/repositories/asset-edit.repository';
 import { AssetJobRepository } from 'src/repositories/asset-job.repository';
 import { AssetRepository } from 'src/repositories/asset.repository';
 import { ConfigRepository } from 'src/repositories/config.repository';
+import { CronRepository } from 'src/repositories/cron.repository';
 import { CryptoRepository } from 'src/repositories/crypto.repository';
 import { DatabaseRepository } from 'src/repositories/database.repository';
 import { EmailRepository } from 'src/repositories/email.repository';
@@ -500,6 +501,10 @@ const newMockRepository = <T>(key: ClassConstructor<T>) => {
       });
     }
 
+    case CronRepository: {
+      return automock(CronRepository, { args: [undefined, { setContext: () => {} }], strict: false });
+    }
+
     case EmailRepository: {
       return automock(EmailRepository, { args: [{ setContext: () => {} }] });
     }

server/test/medium/specs/services/version.service.spec.ts

@@ -1,6 +1,7 @@
 import { Kysely } from 'kysely';
 import { serverVersion } from 'src/constants';
 import { JobName } from 'src/enum';
+import { CronRepository } from 'src/repositories/cron.repository';
 import { DatabaseRepository } from 'src/repositories/database.repository';
 import { JobRepository } from 'src/repositories/job.repository';
 import { LoggingRepository } from 'src/repositories/logging.repository';
@@ -16,7 +17,7 @@ const setup = (db?: Kysely<DB>) => {
   return newMediumService(VersionService, {
     database: db || defaultDatabase,
     real: [DatabaseRepository, VersionHistoryRepository],
-    mock: [LoggingRepository, JobRepository],
+    mock: [LoggingRepository, JobRepository, CronRepository],
   });
 };