diff --git a/server/models/Collection.ts b/server/models/Collection.ts index e0522f838b..f194c48e2f 100644 --- a/server/models/Collection.ts +++ b/server/models/Collection.ts @@ -61,6 +61,7 @@ import { CollectionValidation } from "@shared/validations"; import { ValidationError } from "@server/errors"; import type { APIContext } from "@server/types"; import { CacheHelper } from "@server/utils/CacheHelper"; +import { RedisPrefixHelper } from "@server/utils/RedisPrefixHelper"; import removeIndexCollision from "@server/utils/removeIndexCollision"; import { generateUrlId } from "@server/utils/url"; import { ValidateIndex } from "@server/validation"; @@ -347,7 +348,7 @@ class Collection extends ParanoidModel< } if (model.changed("documentStructure")) { await CacheHelper.clearData( - CacheHelper.getCollectionDocumentsKey(model.id) + RedisPrefixHelper.getCollectionDocumentsKey(model.id) ); } } @@ -360,7 +361,7 @@ class Collection extends ParanoidModel< if (model.changed("documentStructure")) { const setData = () => CacheHelper.setData( - CacheHelper.getCollectionDocumentsKey(model.id), + RedisPrefixHelper.getCollectionDocumentsKey(model.id), model.documentStructure, 60 ); diff --git a/server/queues/processors/IntegrationCreatedProcessor.ts b/server/queues/processors/IntegrationCreatedProcessor.ts index 4d8f8cf01c..41a56a8bd8 100644 --- a/server/queues/processors/IntegrationCreatedProcessor.ts +++ b/server/queues/processors/IntegrationCreatedProcessor.ts @@ -3,6 +3,7 @@ import { Integration } from "@server/models"; import BaseProcessor from "@server/queues/processors/BaseProcessor"; import type { IntegrationEvent, Event } from "@server/types"; import { CacheHelper } from "@server/utils/CacheHelper"; +import { RedisPrefixHelper } from "@server/utils/RedisPrefixHelper"; import CacheIssueSourcesTask from "../tasks/CacheIssueSourcesTask"; export default class IntegrationCreatedProcessor extends BaseProcessor { @@ -25,6 +26,6 @@ export default class IntegrationCreatedProcessor extends BaseProcessor { }); // Clear the cache of unfurled data for the team as it may be stale now. - await CacheHelper.clearData(CacheHelper.getUnfurlKey(integration.teamId)); + await CacheHelper.clearData(RedisPrefixHelper.getUnfurlKey(integration.teamId)); } } diff --git a/server/queues/processors/IntegrationDeletedProcessor.ts b/server/queues/processors/IntegrationDeletedProcessor.ts index a33ffa99b1..6147316a93 100644 --- a/server/queues/processors/IntegrationDeletedProcessor.ts +++ b/server/queues/processors/IntegrationDeletedProcessor.ts @@ -3,6 +3,7 @@ import { Integration } from "@server/models"; import BaseProcessor from "@server/queues/processors/BaseProcessor"; import type { IntegrationEvent, Event } from "@server/types"; import { CacheHelper } from "@server/utils/CacheHelper"; +import { RedisPrefixHelper } from "@server/utils/RedisPrefixHelper"; import { Hook, PluginManager } from "@server/utils/PluginManager"; export default class IntegrationDeletedProcessor extends BaseProcessor { @@ -26,7 +27,7 @@ export default class IntegrationDeletedProcessor extends BaseProcessor { // Clear the cache of unfurled data for the team as it may be stale now. if (integration.type === IntegrationType.Embed) { - await CacheHelper.clearData(CacheHelper.getUnfurlKey(integration.teamId)); + await CacheHelper.clearData(RedisPrefixHelper.getUnfurlKey(integration.teamId)); } await integration.destroy({ force: true }); diff --git a/server/routes/api/collections/collections.ts b/server/routes/api/collections/collections.ts index 36f33485f3..88dc31efd8 100644 --- a/server/routes/api/collections/collections.ts +++ b/server/routes/api/collections/collections.ts @@ -39,6 +39,7 @@ import { } from "@server/presenters"; import type { APIContext } from "@server/types"; import { CacheHelper } from "@server/utils/CacheHelper"; +import { RedisPrefixHelper } from "@server/utils/RedisPrefixHelper"; import { RateLimiterStrategy } from "@server/utils/RateLimiter"; import { collectionIndexing } from "@server/utils/indexing"; import pagination from "../middlewares/pagination"; @@ -143,7 +144,7 @@ router.post( authorize(user, "readDocument", collection); const documentStructure = await CacheHelper.getDataOrSet( - CacheHelper.getCollectionDocumentsKey(collection.id), + RedisPrefixHelper.getCollectionDocumentsKey(collection.id), async () => ( await Collection.findByPk(collection.id, { diff --git a/server/routes/api/urls/urls.ts b/server/routes/api/urls/urls.ts index 0becb896c0..72b3368eb5 100644 --- a/server/routes/api/urls/urls.ts +++ b/server/routes/api/urls/urls.ts @@ -15,6 +15,7 @@ import { authorize, can } from "@server/policies"; import presentUnfurl from "@server/presenters/unfurl"; import type { APIContext, Unfurl } from "@server/types"; import { CacheHelper, type CacheResult } from "@server/utils/CacheHelper"; +import { RedisPrefixHelper } from "@server/utils/RedisPrefixHelper"; import { Hook, PluginManager } from "@server/utils/PluginManager"; import { RateLimiterStrategy } from "@server/utils/RateLimiter"; import { @@ -134,7 +135,7 @@ router.post( // External resources // Use getDataOrSet which handles distributed locking to prevent thundering herd // when multiple clients request the same URL simultaneously - const cacheKey = CacheHelper.getUnfurlKey(actor.teamId, url); + const cacheKey = RedisPrefixHelper.getUnfurlKey(actor.teamId, url); const defaultCacheExpiry = 3600; const unfurlResult = await CacheHelper.getDataOrSet< @@ -186,7 +187,7 @@ router.post( const { url } = ctx.input.body; const result = await CacheHelper.getDataOrSet( - CacheHelper.getEmbedCheckKey(url), + RedisPrefixHelper.getEmbedCheckKey(url), () => checkEmbeddability(url), Day.seconds ); diff --git a/server/utils/CacheHelper.ts b/server/utils/CacheHelper.ts index 1aa28f65ac..5a6215e353 100644 --- a/server/utils/CacheHelper.ts +++ b/server/utils/CacheHelper.ts @@ -141,31 +141,4 @@ export class CacheHelper { }) ); } - - // keys - - /** - * Gets key against which unfurl response for the given url is stored - * - * @param teamId The team ID to generate a key for - * @param url The url to generate a key for - */ - public static getUnfurlKey(teamId: string, url = "") { - return `unfurl:${teamId}:${url}`; - } - - public static getCollectionDocumentsKey(collectionId: string) { - return `cd:${collectionId}`; - } - - /** - * Gets key for caching embed check results. This is a global cache key - * (not team-specific) since embed headers are the same for all users. - * - * @param url The URL to generate a cache key for. - * @returns the cache key string. - */ - public static getEmbedCheckKey(url: string) { - return `embed:${url}`; - } } diff --git a/server/utils/RedisPrefixHelper.test.ts b/server/utils/RedisPrefixHelper.test.ts new file mode 100644 index 0000000000..4b3288dbd1 --- /dev/null +++ b/server/utils/RedisPrefixHelper.test.ts @@ -0,0 +1,61 @@ +import { RedisPrefixHelper } from "./RedisPrefixHelper"; + +describe("RedisPrefixHelper", () => { + describe("getUnfurlKey", () => { + it("should generate key with teamId and url", () => { + const teamId = "team-123"; + const url = "https://example.com"; + const result = RedisPrefixHelper.getUnfurlKey(teamId, url); + expect(result).toBe("unfurl:team-123:https://example.com"); + }); + + it("should generate key with teamId and empty url", () => { + const teamId = "team-456"; + const result = RedisPrefixHelper.getUnfurlKey(teamId); + expect(result).toBe("unfurl:team-456:"); + }); + + it("should handle special characters in url", () => { + const teamId = "team-789"; + const url = "https://example.com/path?query=value&other=123"; + const result = RedisPrefixHelper.getUnfurlKey(teamId, url); + expect(result).toBe( + "unfurl:team-789:https://example.com/path?query=value&other=123" + ); + }); + }); + + describe("getCollectionDocumentsKey", () => { + it("should generate key with collectionId", () => { + const collectionId = "col-abc123"; + const result = RedisPrefixHelper.getCollectionDocumentsKey(collectionId); + expect(result).toBe("cd:col-abc123"); + }); + + it("should handle uuid format", () => { + const collectionId = "550e8400-e29b-41d4-a716-446655440000"; + const result = RedisPrefixHelper.getCollectionDocumentsKey(collectionId); + expect(result).toBe("cd:550e8400-e29b-41d4-a716-446655440000"); + }); + }); + + describe("getEmbedCheckKey", () => { + it("should generate key with url", () => { + const url = "https://example.com/embed"; + const result = RedisPrefixHelper.getEmbedCheckKey(url); + expect(result).toBe("embed:https://example.com/embed"); + }); + + it("should handle urls with query parameters", () => { + const url = "https://example.com/video?v=abc123"; + const result = RedisPrefixHelper.getEmbedCheckKey(url); + expect(result).toBe("embed:https://example.com/video?v=abc123"); + }); + + it("should handle urls with fragments", () => { + const url = "https://example.com/page#section"; + const result = RedisPrefixHelper.getEmbedCheckKey(url); + expect(result).toBe("embed:https://example.com/page#section"); + }); + }); +}); diff --git a/server/utils/RedisPrefixHelper.ts b/server/utils/RedisPrefixHelper.ts new file mode 100644 index 0000000000..d53e568324 --- /dev/null +++ b/server/utils/RedisPrefixHelper.ts @@ -0,0 +1,35 @@ +/** + * Helper class for Redis cache key generation. + */ +export class RedisPrefixHelper { + /** + * Gets key against which unfurl response for the given url is stored. + * + * @param teamId The team ID to generate a key for. + * @param url The url to generate a key for. + */ + public static getUnfurlKey(teamId: string, url = "") { + return `unfurl:${teamId}:${url}`; + } + + /** + * Gets key for caching collection documents structure. + * + * @param collectionId The collection ID to generate a key for. + * @returns the cache key string. + */ + public static getCollectionDocumentsKey(collectionId: string) { + return `cd:${collectionId}`; + } + + /** + * Gets key for caching embed check results. This is a global cache key + * (not team-specific) since embed headers are the same for all users. + * + * @param url The URL to generate a cache key for. + * @returns the cache key string. + */ + public static getEmbedCheckKey(url: string) { + return `embed:${url}`; + } +} diff --git a/server/utils/__mocks__/CacheHelper.ts b/server/utils/__mocks__/CacheHelper.ts index b421a91310..c7aa210f6b 100644 --- a/server/utils/__mocks__/CacheHelper.ts +++ b/server/utils/__mocks__/CacheHelper.ts @@ -44,20 +44,4 @@ export class CacheHelper { public static async clearData(_prefix: string) { return; } - - /** - * These are real methods that don't require mocking as they don't - * interact with Redis directly - */ - public static getUnfurlKey(teamId: string, url = "") { - return `unfurl:${teamId}:${url}`; - } - - public static getCollectionDocumentsKey(collectionId: string) { - return `cd:${collectionId}`; - } - - public static getEmbedCheckKey(url: string) { - return `embed:${url}`; - } }