From 20bafcb61bee2a9a10a500908850c9a7d5e3c8c5 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 22 Jul 2021 11:15:17 +0200 Subject: [PATCH] Move apicache in peertube Allow us to upgrade to node 16 --- package.json | 1 - server/controllers/activitypub/client.ts | 4 +- server/controllers/api/server/stats.ts | 4 +- server/controllers/bots.ts | 4 +- server/controllers/feeds.ts | 24 +- server/controllers/static.ts | 12 +- server/middlewares/cache.ts | 28 -- server/middlewares/cache/cache.ts | 32 +++ server/middlewares/cache/index.ts | 1 + server/middlewares/cache/shared/api-cache.ts | 269 +++++++++++++++++++ server/middlewares/cache/shared/index.ts | 1 + server/middlewares/index.ts | 1 + server/tests/feeds/feeds.ts | 40 ++- server/typings/express/index.d.ts | 11 + yarn.lock | 5 - 15 files changed, 372 insertions(+), 65 deletions(-) delete mode 100644 server/middlewares/cache.ts create mode 100644 server/middlewares/cache/cache.ts create mode 100644 server/middlewares/cache/index.ts create mode 100644 server/middlewares/cache/shared/api-cache.ts create mode 100644 server/middlewares/cache/shared/index.ts diff --git a/package.json b/package.json index 7611ac9ab..bd18d4c64 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,6 @@ }, "dependencies": { "@uploadx/core": "^4.4.0", - "apicache": "1.6.2", "async": "^3.0.1", "async-lru": "^1.1.1", "bcrypt": "5.0.1", diff --git a/server/controllers/activitypub/client.ts b/server/controllers/activitypub/client.ts index d7de1b9bd..bef4bc068 100644 --- a/server/controllers/activitypub/client.ts +++ b/server/controllers/activitypub/client.ts @@ -24,7 +24,7 @@ import { videosCustomGetValidator, videosShareValidator } from '../../middlewares' -import { cacheRoute } from '../../middlewares/cache' +import { cacheRoute } from '../../middlewares/cache/cache' import { getAccountVideoRateValidatorFactory, videoCommentGetValidator } from '../../middlewares/validators' import { videoFileRedundancyGetValidator, videoPlaylistRedundancyGetValidator } from '../../middlewares/validators/redundancy' import { videoPlaylistElementAPGetValidator, videoPlaylistsGetValidator } from '../../middlewares/validators/videos/video-playlists' @@ -77,7 +77,7 @@ activityPubClientRouter.get('/accounts?/:name/dislikes/:videoId', activityPubClientRouter.get( [ '/videos/watch/:id', '/w/:id' ], executeIfActivityPub, - asyncMiddleware(cacheRoute()(ROUTE_CACHE_LIFETIME.ACTIVITY_PUB.VIDEOS)), + cacheRoute(ROUTE_CACHE_LIFETIME.ACTIVITY_PUB.VIDEOS), asyncMiddleware(videosCustomGetValidator('all')), asyncMiddleware(videoController) ) diff --git a/server/controllers/api/server/stats.ts b/server/controllers/api/server/stats.ts index 3aea12450..397702548 100644 --- a/server/controllers/api/server/stats.ts +++ b/server/controllers/api/server/stats.ts @@ -2,12 +2,12 @@ import * as express from 'express' import { StatsManager } from '@server/lib/stat-manager' import { ROUTE_CACHE_LIFETIME } from '../../../initializers/constants' import { asyncMiddleware } from '../../../middlewares' -import { cacheRoute } from '../../../middlewares/cache' +import { cacheRoute } from '../../../middlewares/cache/cache' const statsRouter = express.Router() statsRouter.get('/stats', - asyncMiddleware(cacheRoute()(ROUTE_CACHE_LIFETIME.STATS)), + cacheRoute(ROUTE_CACHE_LIFETIME.STATS), asyncMiddleware(getStats) ) diff --git a/server/controllers/bots.ts b/server/controllers/bots.ts index 9e92063d4..93aa0cf30 100644 --- a/server/controllers/bots.ts +++ b/server/controllers/bots.ts @@ -5,7 +5,7 @@ import { SitemapStream, streamToPromise } from 'sitemap' import { VideoModel } from '../models/video/video' import { VideoChannelModel } from '../models/video/video-channel' import { AccountModel } from '../models/account/account' -import { cacheRoute } from '../middlewares/cache' +import { cacheRoute } from '../middlewares/cache/cache' import { buildNSFWFilter } from '../helpers/express-utils' import { truncate } from 'lodash' @@ -14,7 +14,7 @@ const botsRouter = express.Router() // Special route that add OpenGraph and oEmbed tags // Do not use a template engine for a so little thing botsRouter.use('/sitemap.xml', - asyncMiddleware(cacheRoute()(ROUTE_CACHE_LIFETIME.SITEMAP)), + cacheRoute(ROUTE_CACHE_LIFETIME.SITEMAP), asyncMiddleware(getSitemap) ) diff --git a/server/controllers/feeds.ts b/server/controllers/feeds.ts index 435b12193..cdc6bfb8b 100644 --- a/server/controllers/feeds.ts +++ b/server/controllers/feeds.ts @@ -16,20 +16,20 @@ import { videosSortValidator, videoSubscriptionFeedsValidator } from '../middlewares' -import { cacheRoute } from '../middlewares/cache' +import { cacheRouteFactory } from '../middlewares/cache/cache' import { VideoModel } from '../models/video/video' import { VideoCommentModel } from '../models/video/video-comment' const feedsRouter = express.Router() +const cacheRoute = cacheRouteFactory({ + headerBlacklist: [ 'Content-Type' ] +}) + feedsRouter.get('/feeds/video-comments.:format', feedsFormatValidator, setFeedFormatContentType, - asyncMiddleware(cacheRoute({ - headerBlacklist: [ - 'Content-Type' - ] - })(ROUTE_CACHE_LIFETIME.FEEDS)), + cacheRoute(ROUTE_CACHE_LIFETIME.FEEDS), asyncMiddleware(videoFeedsValidator), asyncMiddleware(videoCommentsFeedsValidator), asyncMiddleware(generateVideoCommentsFeed) @@ -40,11 +40,7 @@ feedsRouter.get('/feeds/videos.:format', setDefaultVideosSort, feedsFormatValidator, setFeedFormatContentType, - asyncMiddleware(cacheRoute({ - headerBlacklist: [ - 'Content-Type' - ] - })(ROUTE_CACHE_LIFETIME.FEEDS)), + cacheRoute(ROUTE_CACHE_LIFETIME.FEEDS), commonVideosFiltersValidator, asyncMiddleware(videoFeedsValidator), asyncMiddleware(generateVideoFeed) @@ -55,11 +51,7 @@ feedsRouter.get('/feeds/subscriptions.:format', setDefaultVideosSort, feedsFormatValidator, setFeedFormatContentType, - asyncMiddleware(cacheRoute({ - headerBlacklist: [ - 'Content-Type' - ] - })(ROUTE_CACHE_LIFETIME.FEEDS)), + cacheRoute(ROUTE_CACHE_LIFETIME.FEEDS), commonVideosFiltersValidator, asyncMiddleware(videoSubscriptionFeedsValidator), asyncMiddleware(generateVideoFeedForSubscriptions) diff --git a/server/controllers/static.ts b/server/controllers/static.ts index 5900eaff3..912d7e36c 100644 --- a/server/controllers/static.ts +++ b/server/controllers/static.ts @@ -19,7 +19,7 @@ import { } from '../initializers/constants' import { getThemeOrDefault } from '../lib/plugins/theme-utils' import { asyncMiddleware } from '../middlewares' -import { cacheRoute } from '../middlewares/cache' +import { cacheRoute } from '../middlewares/cache/cache' import { UserModel } from '../models/user/user' import { VideoModel } from '../models/video/video' import { VideoCommentModel } from '../models/video/video-comment' @@ -66,7 +66,7 @@ staticRouter.use( // robots.txt service staticRouter.get('/robots.txt', - asyncMiddleware(cacheRoute()(ROUTE_CACHE_LIFETIME.ROBOTS)), + cacheRoute(ROUTE_CACHE_LIFETIME.ROBOTS), (_, res: express.Response) => { res.type('text/plain') return res.send(CONFIG.INSTANCE.ROBOTS) @@ -86,7 +86,7 @@ staticRouter.get('/security.txt', ) staticRouter.get('/.well-known/security.txt', - asyncMiddleware(cacheRoute()(ROUTE_CACHE_LIFETIME.SECURITYTXT)), + cacheRoute(ROUTE_CACHE_LIFETIME.SECURITYTXT), (_, res: express.Response) => { res.type('text/plain') return res.send(CONFIG.INSTANCE.SECURITYTXT + CONFIG.INSTANCE.SECURITYTXT_CONTACT) @@ -95,7 +95,7 @@ staticRouter.get('/.well-known/security.txt', // nodeinfo service staticRouter.use('/.well-known/nodeinfo', - asyncMiddleware(cacheRoute()(ROUTE_CACHE_LIFETIME.NODEINFO)), + cacheRoute(ROUTE_CACHE_LIFETIME.NODEINFO), (_, res: express.Response) => { return res.json({ links: [ @@ -108,13 +108,13 @@ staticRouter.use('/.well-known/nodeinfo', } ) staticRouter.use('/nodeinfo/:version.json', - asyncMiddleware(cacheRoute()(ROUTE_CACHE_LIFETIME.NODEINFO)), + cacheRoute(ROUTE_CACHE_LIFETIME.NODEINFO), asyncMiddleware(generateNodeinfo) ) // dnt-policy.txt service (see https://www.eff.org/dnt-policy) staticRouter.use('/.well-known/dnt-policy.txt', - asyncMiddleware(cacheRoute()(ROUTE_CACHE_LIFETIME.DNT_POLICY)), + cacheRoute(ROUTE_CACHE_LIFETIME.DNT_POLICY), (_, res: express.Response) => { res.type('text/plain') diff --git a/server/middlewares/cache.ts b/server/middlewares/cache.ts deleted file mode 100644 index e508b22a6..000000000 --- a/server/middlewares/cache.ts +++ /dev/null @@ -1,28 +0,0 @@ -import * as apicache from 'apicache' -import { HttpStatusCode } from '../../shared/models/http/http-error-codes' -import { Redis } from '../lib/redis' - -// Ensure Redis is initialized -Redis.Instance.init() - -const defaultOptions = { - redisClient: Redis.Instance.getClient(), - appendKey: () => Redis.Instance.getPrefix(), - statusCodes: { - exclude: [ - HttpStatusCode.FORBIDDEN_403, - HttpStatusCode.NOT_FOUND_404 - ] - } -} - -const cacheRoute = (extraOptions = {}) => apicache.options({ - ...defaultOptions, - ...extraOptions -}).middleware - -// --------------------------------------------------------------------------- - -export { - cacheRoute -} diff --git a/server/middlewares/cache/cache.ts b/server/middlewares/cache/cache.ts new file mode 100644 index 000000000..48162a0ae --- /dev/null +++ b/server/middlewares/cache/cache.ts @@ -0,0 +1,32 @@ +import { HttpStatusCode } from '../../../shared/models/http/http-error-codes' +import { Redis } from '../../lib/redis' +import { ApiCache, APICacheOptions } from './shared' + +// Ensure Redis is initialized +Redis.Instance.init() + +const defaultOptions: APICacheOptions = { + excludeStatus: [ + HttpStatusCode.FORBIDDEN_403, + HttpStatusCode.NOT_FOUND_404 + ] +} + +function cacheRoute (duration: string) { + const instance = new ApiCache(defaultOptions) + + return instance.buildMiddleware(duration) +} + +function cacheRouteFactory (options: APICacheOptions) { + const instance = new ApiCache({ ...defaultOptions, ...options }) + + return instance.buildMiddleware.bind(instance) +} + +// --------------------------------------------------------------------------- + +export { + cacheRoute, + cacheRouteFactory +} diff --git a/server/middlewares/cache/index.ts b/server/middlewares/cache/index.ts new file mode 100644 index 000000000..79b512828 --- /dev/null +++ b/server/middlewares/cache/index.ts @@ -0,0 +1 @@ +export * from './cache' diff --git a/server/middlewares/cache/shared/api-cache.ts b/server/middlewares/cache/shared/api-cache.ts new file mode 100644 index 000000000..f9f7b1b67 --- /dev/null +++ b/server/middlewares/cache/shared/api-cache.ts @@ -0,0 +1,269 @@ +// Thanks: https://github.com/kwhitley/apicache +// We duplicated the library because it is unmaintened and prevent us to upgrade to recent NodeJS versions + +import * as express from 'express' +import { OutgoingHttpHeaders } from 'http' +import { isTestInstance, parseDurationToMs } from '@server/helpers/core-utils' +import { logger } from '@server/helpers/logger' +import { Redis } from '@server/lib/redis' +import { HttpStatusCode } from '@shared/models' + +export interface APICacheOptions { + headerBlacklist?: string[] + excludeStatus?: HttpStatusCode[] +} + +interface CacheObject { + status: number + headers: OutgoingHttpHeaders + data: any + encoding: BufferEncoding + timestamp: number +} + +export class ApiCache { + + private readonly options: APICacheOptions + private readonly timers: { [ id: string ]: NodeJS.Timeout } = {} + + private index: { all: string[] } = { all: [] } + + constructor (options: APICacheOptions) { + this.options = { + headerBlacklist: [], + excludeStatus: [], + + ...options + } + } + + buildMiddleware (strDuration: string) { + const duration = parseDurationToMs(strDuration) + + return (req: express.Request, res: express.Response, next: express.NextFunction) => { + const key = Redis.Instance.getPrefix() + 'api-cache-' + req.originalUrl + const redis = Redis.Instance.getClient() + + if (!redis.connected) return this.makeResponseCacheable(res, next, key, duration) + + try { + redis.hgetall(key, (err, obj) => { + if (!err && obj && obj.response) { + return this.sendCachedResponse(req, res, JSON.parse(obj.response), duration) + } + + return this.makeResponseCacheable(res, next, key, duration) + }) + } catch (err) { + return this.makeResponseCacheable(res, next, key, duration) + } + } + } + + private shouldCacheResponse (response: express.Response) { + if (!response) return false + if (this.options.excludeStatus.includes(response.statusCode)) return false + + return true + } + + private addIndexEntries (key: string) { + this.index.all.unshift(key) + } + + private filterBlacklistedHeaders (headers: OutgoingHttpHeaders) { + return Object.keys(headers) + .filter(key => !this.options.headerBlacklist.includes(key)) + .reduce((acc, header) => { + acc[header] = headers[header] + + return acc + }, {}) + } + + private createCacheObject (status: number, headers: OutgoingHttpHeaders, data: any, encoding: BufferEncoding) { + return { + status, + headers: this.filterBlacklistedHeaders(headers), + data, + encoding, + + // Seconds since epoch, used to properly decrement max-age headers in cached responses. + timestamp: new Date().getTime() / 1000 + } as CacheObject + } + + private cacheResponse (key: string, value: object, duration: number) { + const redis = Redis.Instance.getClient() + + if (redis.connected) { + try { + redis.hset(key, 'response', JSON.stringify(value)) + redis.hset(key, 'duration', duration + '') + redis.expire(key, duration / 1000) + } catch (err) { + logger.error('Cannot set cache in redis.', { err }) + } + } + + // add automatic cache clearing from duration, includes max limit on setTimeout + this.timers[key] = setTimeout(() => this.clear(key), Math.min(duration, 2147483647)) + } + + private accumulateContent (res: express.Response, content: any) { + if (!content) return + + if (typeof content === 'string') { + res.locals.apicache.content = (res.locals.apicache.content || '') + content + return + } + + if (Buffer.isBuffer(content)) { + let oldContent = res.locals.apicache.content + + if (typeof oldContent === 'string') { + oldContent = Buffer.from(oldContent) + } + + if (!oldContent) { + oldContent = Buffer.alloc(0) + } + + res.locals.apicache.content = Buffer.concat( + [ oldContent, content ], + oldContent.length + content.length + ) + + return + } + + res.locals.apicache.content = content + } + + private makeResponseCacheable (res: express.Response, next: express.NextFunction, key: string, duration: number) { + const self = this + + res.locals.apicache = { + write: res.write, + writeHead: res.writeHead, + end: res.end, + cacheable: true, + content: undefined, + headers: {} + } + + // Patch express + res.writeHead = function () { + if (self.shouldCacheResponse(res)) { + res.setHeader('cache-control', 'max-age=' + (duration / 1000).toFixed(0)) + } else { + res.setHeader('cache-control', 'no-cache, no-store, must-revalidate') + } + + res.locals.apicache.headers = Object.assign({}, res.getHeaders()) + return res.locals.apicache.writeHead.apply(this, arguments as any) + } + + res.write = function (chunk: any) { + self.accumulateContent(res, chunk) + return res.locals.apicache.write.apply(this, arguments as any) + } + + res.end = function (content: any, encoding: BufferEncoding) { + if (self.shouldCacheResponse(res)) { + self.accumulateContent(res, content) + + if (res.locals.apicache.cacheable && res.locals.apicache.content) { + self.addIndexEntries(key) + + const headers = res.locals.apicache.headers || res.getHeaders() + const cacheObject = self.createCacheObject( + res.statusCode, + headers, + res.locals.apicache.content, + encoding + ) + self.cacheResponse(key, cacheObject, duration) + } + } + + res.locals.apicache.end.apply(this, arguments as any) + } as any + + next() + } + + private sendCachedResponse (request: express.Request, response: express.Response, cacheObject: CacheObject, duration: number) { + const headers = response.getHeaders() + + if (isTestInstance()) { + Object.assign(headers, { + 'x-api-cache-cached': 'true' + }) + } + + Object.assign(headers, this.filterBlacklistedHeaders(cacheObject.headers || {}), { + // Set properly decremented max-age header + // This ensures that max-age is in sync with the cache expiration + 'cache-control': + 'max-age=' + + Math.max( + 0, + (duration / 1000 - (new Date().getTime() / 1000 - cacheObject.timestamp)) + ).toFixed(0) + }) + + // unstringify buffers + let data = cacheObject.data + if (data && data.type === 'Buffer') { + data = typeof data.data === 'number' + ? Buffer.alloc(data.data) + : Buffer.from(data.data) + } + + // Test Etag against If-None-Match for 304 + const cachedEtag = cacheObject.headers.etag + const requestEtag = request.headers['if-none-match'] + + if (requestEtag && cachedEtag === requestEtag) { + response.writeHead(304, headers) + return response.end() + } + + response.writeHead(cacheObject.status || 200, headers) + + return response.end(data, cacheObject.encoding) + } + + private clear (target: string) { + const redis = Redis.Instance.getClient() + + if (target) { + clearTimeout(this.timers[target]) + delete this.timers[target] + + try { + redis.del(target) + } catch (err) { + logger.error('Cannot delete %s in redis cache.', target, { err }) + } + + this.index.all = this.index.all.filter(key => key !== target) + } else { + for (const key of this.index.all) { + clearTimeout(this.timers[key]) + delete this.timers[key] + + try { + redis.del(key) + } catch (err) { + logger.error('Cannot delete %s in redis cache.', key, { err }) + } + } + + this.index.all = [] + } + + return this.index + } +} diff --git a/server/middlewares/cache/shared/index.ts b/server/middlewares/cache/shared/index.ts new file mode 100644 index 000000000..c707eaf7a --- /dev/null +++ b/server/middlewares/cache/shared/index.ts @@ -0,0 +1 @@ +export * from './api-cache' diff --git a/server/middlewares/index.ts b/server/middlewares/index.ts index 413653dac..a0035f623 100644 --- a/server/middlewares/index.ts +++ b/server/middlewares/index.ts @@ -1,4 +1,5 @@ export * from './validators' +export * from './cache' export * from './activitypub' export * from './async' export * from './auth' diff --git a/server/tests/feeds/feeds.ts b/server/tests/feeds/feeds.ts index 5667207c0..55b434846 100644 --- a/server/tests/feeds/feeds.ts +++ b/server/tests/feeds/feeds.ts @@ -8,6 +8,7 @@ import { createMultipleServers, createSingleServer, doubleFollow, + makeGetRequest, PeerTubeServer, setAccessTokensToServers, waitJobs @@ -52,9 +53,7 @@ describe('Test syndication feeds', () => { } { - const attr = { username: 'john', password: 'password' } - await servers[0].users.create({ username: attr.username, password: attr.password }) - userAccessToken = await servers[0].login.getAccessToken(attr) + userAccessToken = await servers[0].users.generateUserAndToken('john') const user = await servers[0].users.getMyInfo({ token: userAccessToken }) userAccountId = user.account.id @@ -108,6 +107,41 @@ describe('Test syndication feeds', () => { expect(JSON.parse(jsonText)).to.be.jsonSchema({ type: 'object' }) } }) + + it('Should serve the endpoint with a classic request', async function () { + await makeGetRequest({ + url: servers[0].url, + path: '/feeds/videos.xml', + accept: 'application/xml', + expectedStatus: HttpStatusCode.OK_200 + }) + }) + + it('Should serve the endpoint as a cached request', async function () { + const res = await makeGetRequest({ + url: servers[0].url, + path: '/feeds/videos.xml', + accept: 'application/xml', + expectedStatus: HttpStatusCode.OK_200 + }) + + expect(res.headers['x-api-cache-cached']).to.equal('true') + }) + + it('Should not serve the endpoint as a cached request', async function () { + const res = await makeGetRequest({ + url: servers[0].url, + path: '/feeds/videos.xml?v=186', + accept: 'application/xml', + expectedStatus: HttpStatusCode.OK_200 + }) + + expect(res.headers['x-api-cache-cached']).to.not.exist + }) + + it('Should refuse to serve the endpoint without accept header', async function () { + await makeGetRequest({ url: servers[0].url, path: '/feeds/videos.xml', expectedStatus: HttpStatusCode.NOT_ACCEPTABLE_406 }) + }) }) describe('Videos feed', function () { diff --git a/server/typings/express/index.d.ts b/server/typings/express/index.d.ts index 5469f3b83..1a99b598a 100644 --- a/server/typings/express/index.d.ts +++ b/server/typings/express/index.d.ts @@ -1,4 +1,5 @@ +import { OutgoingHttpHeaders } from 'http' import { RegisterServerAuthExternalOptions } from '@server/types' import { MAbuseMessage, @@ -40,6 +41,7 @@ import { MVideoShareActor, MVideoThumbnail } from '../../types/models' +import { Writable } from 'stream' declare module 'express' { export interface Request { @@ -98,6 +100,15 @@ declare module 'express' { }) => void locals: { + apicache: { + content: string | Buffer + write: Writable['write'] + writeHead: Response['writeHead'] + end: Response['end'] + cacheable: boolean + headers: OutgoingHttpHeaders + } + docUrl?: string videoAPI?: MVideoFormattableDetails diff --git a/yarn.lock b/yarn.lock index f68741038..f0cf2d449 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1243,11 +1243,6 @@ anymatch@~3.1.1, anymatch@~3.1.2: normalize-path "^3.0.0" picomatch "^2.0.4" -apicache@1.6.2: - version "1.6.2" - resolved "https://registry.yarnpkg.com/apicache/-/apicache-1.6.2.tgz#a0a3d51024fa2814c4ace7e9e7053ebcb0920ee6" - integrity sha512-3z5e+1E2qwZoqzFVgdx5l9nGhSG0kHv3v2G170vnJSz5uj4mCLVZfRw0o37aWwV8pTPXSkB8OBZz3TIur4H26g== - append-field@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/append-field/-/append-field-1.0.0.tgz#1e3440e915f0b1203d23748e78edd7b9b5b43e56" -- 2.41.0