From cf2f13193f6589322fae9a6edc636404433be36d Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 23 Jul 2025 09:38:24 -0400 Subject: [PATCH] chore: Fix Redis mock not used consistently in tests (#9716) --- Makefile | 2 +- server/index.ts | 6 +-- server/queues/index.ts | 75 ++++++++++++++++++++++---------- server/queues/queue.ts | 2 +- server/test/setup.ts | 10 ----- server/utils/RateLimiter.ts | 21 ++++++--- server/utils/VerificationCode.ts | 8 ++-- 7 files changed, 77 insertions(+), 47 deletions(-) diff --git a/Makefile b/Makefile index 8d773aaa08..06eb423d49 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ build: docker compose build --pull outline test: - docker compose up -d redis postgres + docker compose up -d postgres NODE_ENV=test yarn sequelize db:drop NODE_ENV=test yarn sequelize db:create NODE_ENV=test yarn sequelize db:migrate diff --git a/server/index.ts b/server/index.ts index 2325677790..0693c7d82f 100644 --- a/server/index.ts +++ b/server/index.ts @@ -24,8 +24,8 @@ import { checkUpdates } from "./utils/updates"; import onerror from "./onerror"; import ShutdownHelper, { ShutdownOrder } from "./utils/ShutdownHelper"; import { checkConnection, sequelize } from "./storage/database"; -import RedisAdapter from "./storage/redis"; -import Metrics from "./logging/Metrics"; +import Redis from "@server/storage/redis"; +import Metrics from "@server/logging/Metrics"; import { PluginManager } from "./utils/PluginManager"; // The number of processes to run, defaults to the number of CPU's available @@ -149,7 +149,7 @@ async function start(_id: number, disconnect: () => void) { } try { - await RedisAdapter.defaultClient.ping(); + await Redis.defaultClient.ping(); } catch (err) { Logger.error("Redis ping failed", err); ctx.status = 500; diff --git a/server/queues/index.ts b/server/queues/index.ts index 3506d4575f..9b4317878c 100644 --- a/server/queues/index.ts +++ b/server/queues/index.ts @@ -1,29 +1,60 @@ import { createQueue } from "@server/queues/queue"; -export const globalEventQueue = createQueue("globalEvents", { - attempts: 5, - backoff: { - type: "exponential", - delay: 1000, +let _globalEventQueue: ReturnType | undefined; +let _processorEventQueue: ReturnType | undefined; +let _websocketQueue: ReturnType | undefined; +let _taskQueue: ReturnType | undefined; + +export const globalEventQueue = new Proxy( + {} as ReturnType, + { + get(_target, prop) { + _globalEventQueue ||= createQueue("globalEvents", { + attempts: 5, + backoff: { + type: "exponential", + delay: 1000, + }, + }); + return _globalEventQueue[prop as keyof typeof _globalEventQueue]; + }, + } +); + +export const processorEventQueue = new Proxy( + {} as ReturnType, + { + get(_target, prop) { + _processorEventQueue ||= createQueue("processorEvents", { + attempts: 5, + backoff: { + type: "exponential", + delay: 10 * 1000, + }, + }); + return _processorEventQueue[prop as keyof typeof _processorEventQueue]; + }, + } +); + +export const websocketQueue = new Proxy({} as ReturnType, { + get(_target, prop) { + _websocketQueue ||= createQueue("websockets", { + timeout: 10 * 1000, + }); + return _websocketQueue[prop as keyof typeof _websocketQueue]; }, }); -export const processorEventQueue = createQueue("processorEvents", { - attempts: 5, - backoff: { - type: "exponential", - delay: 10 * 1000, - }, -}); - -export const websocketQueue = createQueue("websockets", { - timeout: 10 * 1000, -}); - -export const taskQueue = createQueue("tasks", { - attempts: 5, - backoff: { - type: "exponential", - delay: 10 * 1000, +export const taskQueue = new Proxy({} as ReturnType, { + get(_target, prop) { + _taskQueue ||= createQueue("tasks", { + attempts: 5, + backoff: { + type: "exponential", + delay: 10 * 1000, + }, + }); + return _taskQueue[prop as keyof typeof _taskQueue]; }, }); diff --git a/server/queues/queue.ts b/server/queues/queue.ts index a32305482b..3ceb9073fe 100644 --- a/server/queues/queue.ts +++ b/server/queues/queue.ts @@ -4,7 +4,7 @@ import snakeCase from "lodash/snakeCase"; import { Second } from "@shared/utils/time"; import env from "@server/env"; import Metrics from "@server/logging/Metrics"; -import Redis from "@server/storage/redis"; +import Redis from "../storage/redis"; import ShutdownHelper, { ShutdownOrder } from "@server/utils/ShutdownHelper"; export function createQueue( diff --git a/server/test/setup.ts b/server/test/setup.ts index 8e4e38aad0..6a95ac0045 100644 --- a/server/test/setup.ts +++ b/server/test/setup.ts @@ -1,20 +1,14 @@ import "reflect-metadata"; import sharedEnv from "@shared/env"; import env from "@server/env"; -import Redis from "@server/storage/redis"; require("@server/storage/database"); -jest.mock("bull"); - // Enable mocks for Redis-related modules jest.mock("@server/storage/redis"); jest.mock("@server/utils/MutexLock"); jest.mock("@server/utils/CacheHelper"); -// This is needed for the relative manual mock to be picked up -jest.mock("../queues"); - // We never want to make real S3 requests in test environment jest.mock("@aws-sdk/client-s3", () => ({ S3Client: jest.fn(() => ({ @@ -39,10 +33,6 @@ jest.mock("@aws-sdk/s3-request-presigner", () => ({ getSignedUrl: jest.fn(), })); -afterAll(() => { - Redis.defaultClient.disconnect(); -}); - beforeEach(() => { env.URL = sharedEnv.URL = "https://app.outline.dev"; }); diff --git a/server/utils/RateLimiter.ts b/server/utils/RateLimiter.ts index 311c4c0427..cb9125594f 100644 --- a/server/utils/RateLimiter.ts +++ b/server/utils/RateLimiter.ts @@ -20,13 +20,20 @@ export default class RateLimiter { duration: env.RATE_LIMITER_DURATION_WINDOW, }); - static readonly defaultRateLimiter = new RateLimiterRedis({ - storeClient: Redis.defaultClient, - points: env.RATE_LIMITER_REQUESTS, - duration: env.RATE_LIMITER_DURATION_WINDOW, - keyPrefix: this.RATE_LIMITER_REDIS_KEY_PREFIX, - insuranceLimiter: this.insuranceRateLimiter, - }); + private static _defaultRateLimiter: RateLimiterRedis | undefined; + + static get defaultRateLimiter(): RateLimiterRedis { + if (!this._defaultRateLimiter) { + this._defaultRateLimiter = new RateLimiterRedis({ + storeClient: Redis.defaultClient, + points: env.RATE_LIMITER_REQUESTS, + duration: env.RATE_LIMITER_DURATION_WINDOW, + keyPrefix: this.RATE_LIMITER_REDIS_KEY_PREFIX, + insuranceLimiter: this.insuranceRateLimiter, + }); + } + return this._defaultRateLimiter; + } static getRateLimiter(path: string): RateLimiterRedis { return this.rateLimiterMap.get(path) || this.defaultRateLimiter; diff --git a/server/utils/VerificationCode.ts b/server/utils/VerificationCode.ts index 20efc11767..2ef0971f6a 100644 --- a/server/utils/VerificationCode.ts +++ b/server/utils/VerificationCode.ts @@ -1,6 +1,6 @@ import { randomInt } from "crypto"; import { Minute } from "@shared/utils/time"; -import RedisAdapter from "@server/storage/redis"; +import Redis from "@server/storage/redis"; /** * This class manages verification codes for email authentication. @@ -8,9 +8,11 @@ import RedisAdapter from "@server/storage/redis"; */ export class VerificationCode { /** - * Redis client instance + * Redis client instance (lazy initialized) */ - private static redis = RedisAdapter.defaultClient; + private static get redis() { + return Redis.defaultClient; + } /** * TTL for verification codes in milliseconds (10 minutes)