From 30feb9c6c6c2c5ef5f101b38629a712ca8705a2f Mon Sep 17 00:00:00 2001 From: Christoph Tupi Date: Mon, 17 Aug 2026 18:28:17 +0200 Subject: [PATCH 1/3] perf(cache): gate ineligible requests before cache lookup --- server/src/middlewares/cache.ts | 48 +++++++---- server/src/middlewares/graphql.ts | 41 +++++---- test/middlewares/cache.test.ts | 137 ++++++++++++++++++++++++++---- test/middlewares/graphql.test.ts | 105 ++++++++++++++++++++++- 4 files changed, 278 insertions(+), 53 deletions(-) diff --git a/server/src/middlewares/cache.ts b/server/src/middlewares/cache.ts index 524fa06..1616654 100644 --- a/server/src/middlewares/cache.ts +++ b/server/src/middlewares/cache.ts @@ -7,26 +7,18 @@ import { decodeBufferToText, decompressBuffer, streamToBuffer } from '../utils/b import { getCacheHeaderConfig, getHeadersToStore } from '../utils/header'; const middleware = async (ctx: Context, next: any) => { - const cacheService = strapi.plugin('strapi-cache').services.service as CacheService; + const { url, method } = ctx.request; + + if (method !== 'GET') { + await next(); + return; + } + const cacheableEntities = strapi.plugin('strapi-cache').config('cacheableEntities') as - | string[] - | undefined; + string[] | undefined; const cacheableRoutes = strapi.plugin('strapi-cache').config('cacheableRoutes') as string[]; const excludeRoutes = strapi.plugin('strapi-cache').config('excludeRoutes') as string[]; - const keyGenerator = strapi.plugin('strapi-cache').config('keyGenerator') as - | CacheKeyGenerator - | undefined; - const { cacheHeaders, cacheHeadersDenyList, cacheHeadersAllowList, cacheAuthorizedRequests } = - getCacheHeaderConfig(); - const cacheStore = cacheService.getCacheInstance(); - const { url } = ctx.request; - const key = generateCacheKey(ctx, keyGenerator); - const cacheEntry = await cacheStore.get(key); - const cacheControlHeader = ctx.request.headers['cache-control']; - const noCache = cacheControlHeader && cacheControlHeader.includes('no-cache'); const restApiPrefix = strapi.config.get('api.rest.prefix', '/api'); - const entityKey = generateEntityKey(url, restApiPrefix); - const routeIsExcluded = excludeRoutes.some((route) => url.startsWith(route)); if (routeIsExcluded) { @@ -35,6 +27,7 @@ const middleware = async (ctx: Context, next: any) => { return; } + const entityKey = generateEntityKey(url, restApiPrefix); const entityIsCacheable = cacheableEntities?.length ? cacheableEntities.includes(entityKey) : undefined; @@ -43,14 +36,35 @@ const middleware = async (ctx: Context, next: any) => { (cacheableRoutes.length === 0 && url.startsWith(restApiPrefix)); const isCacheable = entityIsCacheable ?? routeIsCacheable; + if (!isCacheable) { + await next(); + return; + } + + const { cacheHeaders, cacheHeadersDenyList, cacheHeadersAllowList, cacheAuthorizedRequests } = + getCacheHeaderConfig(); + const cacheControlHeader = ctx.request.headers['cache-control']; + const noCache = cacheControlHeader && cacheControlHeader.includes('no-cache'); const authorizationHeader = ctx.request.headers['authorization']; if (authorizationHeader && !cacheAuthorizedRequests) { - loggy.info(`Authorized request bypassing cache: ${key}`); + loggy.info(`Authorized request bypassing cache: ${url}`); await next(); return; } + if (noCache) { + await next(); + return; + } + + const cacheService = strapi.plugin('strapi-cache').services.service as CacheService; + const keyGenerator = strapi.plugin('strapi-cache').config('keyGenerator') as + CacheKeyGenerator | undefined; + const cacheStore = cacheService.getCacheInstance(); + const key = generateCacheKey(ctx, keyGenerator); + const cacheEntry = await cacheStore.get(key); + const middlewaresConfig = strapi.config.get('middlewares') as any[]; const corsMiddleware = middlewaresConfig.find((mw: any) => mw.name === 'strapi::cors'); diff --git a/server/src/middlewares/graphql.ts b/server/src/middlewares/graphql.ts index a9db3ce..bf3827f 100644 --- a/server/src/middlewares/graphql.ts +++ b/server/src/middlewares/graphql.ts @@ -14,15 +14,32 @@ const middleware = async (ctx: any, next: any) => { return; } - const cacheService = strapi.plugin('strapi-cache').services.service as CacheService; - const keyGenerator = strapi.plugin('strapi-cache').config('keyGenerator') as - | CacheKeyGenerator - | undefined; + const isGet = method === 'GET'; + if (!isGet && method !== 'POST') { + await next(); + return; + } + const { cacheHeaders, cacheHeadersDenyList, cacheHeadersAllowList, cacheAuthorizedRequests } = getCacheHeaderConfig(); - const cacheStore = cacheService.getCacheInstance(); + const authorizationHeader = ctx.request.headers['authorization']; - const isGet = method === 'GET'; + if (authorizationHeader && !cacheAuthorizedRequests) { + loggy.info('Authorized request bypassing GraphQL cache'); + await next(); + return; + } + + const cacheControlHeader = ctx.request.headers['cache-control']; + const noCache = cacheControlHeader && cacheControlHeader.includes('no-cache'); + + if (noCache) { + await next(); + return; + } + + const keyGenerator = strapi.plugin('strapi-cache').config('keyGenerator') as + CacheKeyGenerator | undefined; let body: string; if (isGet) { @@ -72,18 +89,10 @@ const middleware = async (ctx: any, next: any) => { await next(); return; } + const cacheService = strapi.plugin('strapi-cache').services.service as CacheService; + const cacheStore = cacheService.getCacheInstance(); const cacheEntry = await cacheStore.get(key); - const cacheControlHeader = ctx.request.headers['cache-control']; - const noCache = cacheControlHeader && cacheControlHeader.includes('no-cache'); - const authorizationHeader = ctx.request.headers['authorization']; - - if (authorizationHeader && !cacheAuthorizedRequests) { - loggy.info(`Authorized request bypassing cache: ${key}`); - await next(); - return; - } - const middlewaresConfig = strapi.config.get('middlewares') as any[]; const corsMiddleware = middlewaresConfig.find((mw: any) => mw.name === 'strapi::cors'); diff --git a/test/middlewares/cache.test.ts b/test/middlewares/cache.test.ts index 4801910..d995c07 100644 --- a/test/middlewares/cache.test.ts +++ b/test/middlewares/cache.test.ts @@ -7,17 +7,21 @@ describe('cache middleware', () => { get: vi.fn(), set: vi.fn(), }; + const getCacheInstance = vi.fn(() => mockCacheStore); const keyGenerator = vi.fn((ctx: Context) => `custom:${ctx.request.method}:${ctx.request.url}`); + let cacheableRoutes: string[] = []; + let excludeRoutes: string[] = []; + let cacheAuthorizedRequests = false; const pluginConfig = vi.fn((key: string) => { switch (key) { case 'cacheableEntities': return undefined; case 'cacheableRoutes': - return []; + return cacheableRoutes; case 'excludeRoutes': - return []; + return excludeRoutes; case 'keyGenerator': return keyGenerator; case 'cacheHeaders': @@ -27,7 +31,7 @@ describe('cache middleware', () => { case 'cacheHeadersAllowList': return []; case 'cacheAuthorizedRequests': - return false; + return cacheAuthorizedRequests; default: return undefined; } @@ -37,7 +41,7 @@ describe('cache middleware', () => { plugin: vi.fn().mockReturnValue({ services: { service: { - getCacheInstance: () => mockCacheStore, + getCacheInstance, }, }, config: pluginConfig, @@ -59,6 +63,9 @@ describe('cache middleware', () => { beforeEach(() => { vi.clearAllMocks(); + cacheableRoutes = []; + excludeRoutes = []; + cacheAuthorizedRequests = false; mockCacheStore.get.mockResolvedValue({ body: { cached: true }, headers: {}, @@ -69,21 +76,30 @@ describe('cache middleware', () => { vi.clearAllMocks(); }); - it('uses configured keyGenerator for cache lookup', async () => { - const ctx = { - request: { - url: '/api/articles?populate=*', - method: 'GET', - headers: {}, - }, - method: 'GET', - response: { - headers: {}, - }, + const createContext = ({ + url = '/api/articles', + method = 'GET', + headers = {}, + status = 200, + body, + }: { + url?: string; + method?: string; + headers?: Record; + status?: number; + body?: unknown; + } = {}) => + ({ + request: { url, method, headers }, + method, + response: { headers: {} }, set: vi.fn(), - status: 200, - body: undefined, - } as unknown as Context; + status, + body, + }) as unknown as Context; + + it('uses configured keyGenerator for cache lookup', async () => { + const ctx = createContext({ url: '/api/articles?populate=*' }); const next = vi.fn(); @@ -93,4 +109,89 @@ describe('cache middleware', () => { expect(mockCacheStore.get).toHaveBeenCalledWith('custom:GET:/api/articles?populate=*'); expect(next).not.toHaveBeenCalled(); }); + + it('bypasses the cache before lookup for non-GET requests', async () => { + const ctx = createContext({ method: 'POST' }); + const next = vi.fn(); + + await cacheMiddleware(ctx, next); + + expect(getCacheInstance).not.toHaveBeenCalled(); + expect(mockCacheStore.get).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('bypasses excluded routes before lookup', async () => { + excludeRoutes = ['/api/private']; + const ctx = createContext({ url: '/api/private/profile' }); + const next = vi.fn(); + + await cacheMiddleware(ctx, next); + + expect(getCacheInstance).not.toHaveBeenCalled(); + expect(mockCacheStore.get).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('bypasses non-cacheable routes before lookup', async () => { + cacheableRoutes = ['/api/products']; + const ctx = createContext({ url: '/api/articles' }); + const next = vi.fn(); + + await cacheMiddleware(ctx, next); + + expect(getCacheInstance).not.toHaveBeenCalled(); + expect(mockCacheStore.get).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('bypasses authorized requests before lookup', async () => { + const ctx = createContext({ headers: { authorization: 'Bearer token' } }); + const next = vi.fn(); + + await cacheMiddleware(ctx, next); + + expect(getCacheInstance).not.toHaveBeenCalled(); + expect(mockCacheStore.get).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('still reads authorized requests when configured to cache them', async () => { + cacheAuthorizedRequests = true; + const ctx = createContext({ headers: { authorization: 'Bearer token' } }); + const next = vi.fn(); + + await cacheMiddleware(ctx, next); + + expect(mockCacheStore.get).toHaveBeenCalledWith('custom:GET:/api/articles'); + expect(next).not.toHaveBeenCalled(); + }); + + it('bypasses no-cache requests before lookup', async () => { + const ctx = createContext({ headers: { 'cache-control': 'no-cache' } }); + const next = vi.fn(); + + await cacheMiddleware(ctx, next); + + expect(getCacheInstance).not.toHaveBeenCalled(); + expect(mockCacheStore.get).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('stores a successful cacheable response after a miss', async () => { + mockCacheStore.get.mockResolvedValueOnce(null); + const ctx = createContext(); + const next = vi.fn(async () => { + ctx.status = 200; + ctx.body = { data: { articles: [] } }; + }); + + await cacheMiddleware(ctx, next); + + expect(next).toHaveBeenCalledOnce(); + expect(mockCacheStore.set).toHaveBeenCalledWith('custom:GET:/api/articles', { + body: { data: { articles: [] } }, + headers: null, + }); + }); }); diff --git a/test/middlewares/graphql.test.ts b/test/middlewares/graphql.test.ts index e53e207..557ec9d 100644 --- a/test/middlewares/graphql.test.ts +++ b/test/middlewares/graphql.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { Context } from 'koa'; + +const { rawBodyMock } = vi.hoisted(() => ({ rawBodyMock: vi.fn() })); + +vi.mock('raw-body', () => ({ default: rawBodyMock })); + import graphqlMiddleware from '../../server/src/middlewares/graphql'; describe('graphql middleware', () => { @@ -7,8 +12,10 @@ describe('graphql middleware', () => { get: vi.fn(), set: vi.fn(), }; + const getCacheInstance = vi.fn(() => mockCacheStore); const keyGenerator = vi.fn((ctx: Context) => `custom:${ctx.request.method}:${ctx.request.url}`); + let cacheAuthorizedRequests = false; const pluginConfig = vi.fn((key: string) => { switch (key) { case 'keyGenerator': @@ -20,7 +27,7 @@ describe('graphql middleware', () => { case 'cacheHeadersAllowList': return []; case 'cacheAuthorizedRequests': - return false; + return cacheAuthorizedRequests; default: return undefined; } @@ -32,7 +39,7 @@ describe('graphql middleware', () => { return { services: { service: { - getCacheInstance: () => mockCacheStore, + getCacheInstance, }, }, config: pluginConfig, @@ -59,6 +66,7 @@ describe('graphql middleware', () => { beforeEach(() => { vi.clearAllMocks(); + cacheAuthorizedRequests = false; mockCacheStore.get.mockResolvedValue({ body: { cached: true }, headers: {}, @@ -69,6 +77,99 @@ describe('graphql middleware', () => { vi.clearAllMocks(); }); + it('bypasses unsupported methods before reading the body or cache', async () => { + const ctx = { + request: { + url: '/graphql', + method: 'PUT', + headers: {}, + }, + method: 'PUT', + req: {}, + } as unknown as Context; + const next = vi.fn(); + + await graphqlMiddleware(ctx, next); + + expect(rawBodyMock).not.toHaveBeenCalled(); + expect(getCacheInstance).not.toHaveBeenCalled(); + expect(mockCacheStore.get).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('bypasses authorized requests before reading the body or cache', async () => { + const ctx = { + request: { + url: '/graphql', + method: 'POST', + headers: { authorization: 'Bearer token' }, + }, + method: 'POST', + req: {}, + } as unknown as Context; + const next = vi.fn(); + + await graphqlMiddleware(ctx, next); + + expect(rawBodyMock).not.toHaveBeenCalled(); + expect(getCacheInstance).not.toHaveBeenCalled(); + expect(mockCacheStore.get).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('still reads authorized POST requests when configured to cache them', async () => { + cacheAuthorizedRequests = true; + const originalReq = { + headers: {}, + method: 'POST', + url: '/graphql', + httpVersion: '1.1', + socket: {}, + connection: {}, + }; + rawBodyMock.mockResolvedValue(Buffer.from('{ articles { id } }')); + const ctx = { + request: { + url: '/graphql', + method: 'POST', + headers: { authorization: 'Bearer token' }, + }, + method: 'POST', + req: originalReq, + response: { headers: {} }, + set: vi.fn(), + status: 200, + body: undefined, + } as unknown as Context; + const next = vi.fn(); + + await graphqlMiddleware(ctx, next); + + expect(rawBodyMock).toHaveBeenCalledWith(originalReq); + expect(mockCacheStore.get).toHaveBeenCalledOnce(); + expect(next).not.toHaveBeenCalled(); + }); + + it('bypasses no-cache requests before reading the body or cache', async () => { + const ctx = { + request: { + url: '/graphql', + method: 'POST', + headers: { 'cache-control': 'no-cache' }, + }, + method: 'POST', + req: {}, + } as unknown as Context; + const next = vi.fn(); + + await graphqlMiddleware(ctx, next); + + expect(rawBodyMock).not.toHaveBeenCalled(); + expect(getCacheInstance).not.toHaveBeenCalled(); + expect(mockCacheStore.get).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledOnce(); + }); + it('uses configured keyGenerator for GraphQL cache lookup', async () => { const ctx = { request: { From 3883c19d1897966ffe917e39c3fe4c2c0b1133e3 Mon Sep 17 00:00:00 2001 From: Christoph Tupi Date: Mon, 17 Aug 2026 18:39:55 +0200 Subject: [PATCH 2/3] chore: sonarcube code duplication fix --- server/src/middlewares/cache.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/server/src/middlewares/cache.ts b/server/src/middlewares/cache.ts index 1616654..e6c850f 100644 --- a/server/src/middlewares/cache.ts +++ b/server/src/middlewares/cache.ts @@ -10,8 +10,7 @@ const middleware = async (ctx: Context, next: any) => { const { url, method } = ctx.request; if (method !== 'GET') { - await next(); - return; + return next(); } const cacheableEntities = strapi.plugin('strapi-cache').config('cacheableEntities') as @@ -23,8 +22,7 @@ const middleware = async (ctx: Context, next: any) => { if (routeIsExcluded) { loggy.info(`Route excluded from cache: ${url}`); - await next(); - return; + return next(); } const entityKey = generateEntityKey(url, restApiPrefix); @@ -37,8 +35,7 @@ const middleware = async (ctx: Context, next: any) => { const isCacheable = entityIsCacheable ?? routeIsCacheable; if (!isCacheable) { - await next(); - return; + return next(); } const { cacheHeaders, cacheHeadersDenyList, cacheHeadersAllowList, cacheAuthorizedRequests } = @@ -49,13 +46,11 @@ const middleware = async (ctx: Context, next: any) => { if (authorizationHeader && !cacheAuthorizedRequests) { loggy.info(`Authorized request bypassing cache: ${url}`); - await next(); - return; + return next(); } if (noCache) { - await next(); - return; + return next(); } const cacheService = strapi.plugin('strapi-cache').services.service as CacheService; From 7f536335fad93f0cde990223b18d4392dc5b7758 Mon Sep 17 00:00:00 2001 From: Christoph Tupi Date: Mon, 17 Aug 2026 18:45:36 +0200 Subject: [PATCH 3/3] chore: update contribution --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 68dc05f..04c4a9b 100644 --- a/README.md +++ b/README.md @@ -161,4 +161,4 @@ If you encounter any issues, please feel free to open an issue on the [GitHub re ## 🛠️ Contributing -Contributions are welcome! If you have suggestions or improvements, please open an issue or submit a pull request. +Contributions are welcome! If you have suggestions or improvements, please open an issue or submit a pull request to the `dev` branch.