From c07d5956627463505160608fcdbcc3e885e209ec Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Wed, 10 Aug 2022 13:24:16 -0400 Subject: [PATCH] Update to lru-cache@^7 (#10815) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(deps): bump lru-cache from 6.0.0 to 7.13.1 in /install Bumps [lru-cache](https://github.com/isaacs/node-lru-cache) from 6.0.0 to 7.13.1. - [Release notes](https://github.com/isaacs/node-lru-cache/releases) - [Changelog](https://github.com/isaacs/node-lru-cache/blob/main/CHANGELOG.md) - [Commits](https://github.com/isaacs/node-lru-cache/compare/v6.0.0...v7.13.1) --- updated-dependencies: - dependency-name: lru-cache dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] * fix(lru-cache): remove unneeded `length` params for cache creation, as `maxSize` was not used in those init calls, also renamed some methods to match new method names in lru-cache [breaking] Added deprecation notices for old params * fix: replace three direct calls to lru-cache with call to cacheCreate, moved cache creation call in uploads to run on first init as config is not populated at lib init * test: move configs init above cache reset calls in databasemock * move some more code above cache clear * refactor: remove unused * test: lru * test: more debug * test: on more test * use await helpers.uploadFile * fix: tests remove logs * fix: acp cache page * fix: add in one more guard again cache instantiation with `length` prop but no `maxSize` prop * fix(deps): bump markdown Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Barış Soner Uşaklı --- install/package.json | 4 +- src/analytics.js | 7 ++- src/cache.js | 2 +- src/cacheCreate.js | 74 +++++++++++++++++++++++++----- src/controllers/admin/cache.js | 5 +- src/database/cache.js | 3 +- src/groups/cache.js | 2 +- src/middleware/index.js | 6 +-- src/middleware/uploads.js | 9 ++-- src/posts/cache.js | 6 +-- src/user/blocks.js | 3 +- src/views/admin/advanced/cache.tpl | 8 ++-- test/middleware.js | 7 ++- test/mocks/databasemock.js | 13 +++--- test/uploads.js | 5 +- 15 files changed, 105 insertions(+), 49 deletions(-) diff --git a/install/package.json b/install/package.json index 61db96f7a9..855ba36cde 100644 --- a/install/package.json +++ b/install/package.json @@ -77,7 +77,7 @@ "less": "4.1.3", "lodash": "4.17.21", "logrotate-stream": "0.2.8", - "lru-cache": "6.0.0", + "lru-cache": "7.13.1", "material-design-lite": "1.3.0", "mime": "3.0.0", "mkdirp": "1.0.4", @@ -92,7 +92,7 @@ "nodebb-plugin-dbsearch": "5.1.5", "nodebb-plugin-emoji": "4.0.4", "nodebb-plugin-emoji-android": "3.0.0", - "nodebb-plugin-markdown": "10.0.0", + "nodebb-plugin-markdown": "10.1.0", "nodebb-plugin-mentions": "3.0.11", "nodebb-plugin-spam-be-gone": "1.0.0", "nodebb-rewards-essentials": "0.2.1", diff --git a/src/analytics.js b/src/analytics.js index 5d71d161a8..f9e14b4caf 100644 --- a/src/analytics.js +++ b/src/analytics.js @@ -4,7 +4,6 @@ const cronJob = require('cron').CronJob; const winston = require('winston'); const nconf = require('nconf'); const crypto = require('crypto'); -const LRU = require('lru-cache'); const util = require('util'); const _ = require('lodash'); @@ -15,6 +14,7 @@ const utils = require('./utils'); const plugins = require('./plugins'); const meta = require('./meta'); const pubsub = require('./pubsub'); +const cacheCreate = require('./cacheCreate'); const Analytics = module.exports; @@ -37,10 +37,9 @@ let ipCache; const runJobs = nconf.get('runJobs'); Analytics.init = async function () { - ipCache = new LRU({ + ipCache = cacheCreate({ max: parseInt(meta.config['analytics:maxCache'], 10) || 500, - length: function () { return 1; }, - maxAge: 0, + ttl: 0, }); new cronJob('*/10 * * * * *', (async () => { diff --git a/src/cache.js b/src/cache.js index 7bb27961ce..6dcc440536 100644 --- a/src/cache.js +++ b/src/cache.js @@ -5,5 +5,5 @@ const cacheCreate = require('./cacheCreate'); module.exports = cacheCreate({ name: 'local', max: 40000, - maxAge: 0, + ttl: 0, }); diff --git a/src/cacheCreate.js b/src/cacheCreate.js index e3d7342165..deaef9bba4 100644 --- a/src/cacheCreate.js +++ b/src/cacheCreate.js @@ -4,30 +4,72 @@ module.exports = function (opts) { const LRU = require('lru-cache'); const pubsub = require('./pubsub'); - const cache = new LRU(opts); + // lru-cache@7 deprecations + const winston = require('winston'); + const chalk = require('chalk'); + // sometimes we kept passing in `length` with no corresponding `maxSize`. + // This is now enforced in v7; drop superfluous property + if (opts.hasOwnProperty('length') && !opts.hasOwnProperty('maxSize')) { + winston.warn(`[cache/init(${opts.name})] ${chalk.white.bgRed.bold('DEPRECATION')} ${chalk.yellow('length')} was passed in without a corresponding ${chalk.yellow('maxSize')}. Both are now required as of lru-cache@7.0.0.`); + delete opts.length; + } + + const deprecations = new Map([ + ['stale', 'allowStale'], + ['maxAge', 'ttl'], + ['length', 'sizeCalculation'], + ]); + deprecations.forEach((newProp, oldProp) => { + if (opts.hasOwnProperty(oldProp) && !opts.hasOwnProperty(newProp)) { + winston.warn(`[cache/init (${opts.name})] ${chalk.white.bgRed.bold('DEPRECATION')} The option ${chalk.yellow(oldProp)} has been deprecated as of lru-cache@7.0.0. Please change this to ${chalk.yellow(newProp)} instead.`); + opts[newProp] = opts[oldProp]; + delete opts[oldProp]; + } + }); + + const lruCache = new LRU(opts); + + const cache = {}; cache.name = opts.name; cache.hits = 0; cache.misses = 0; cache.enabled = opts.hasOwnProperty('enabled') ? opts.enabled : true; + const cacheSet = lruCache.set; - const cacheSet = cache.set; - const cacheGet = cache.get; - const cacheDel = cache.del; - const cacheReset = cache.reset; + // backwards compatibility + const propertyMap = new Map([ + ['length', 'calculatedSize'], + ['max', 'max'], + ['maxSize', 'maxSize'], + ['itemCount', 'size'], + ]); + propertyMap.forEach((lruProp, cacheProp) => { + Object.defineProperty(cache, cacheProp, { + get: function () { + return lruCache[lruProp]; + }, + configurable: true, + enumerable: true, + }); + }); - cache.set = function (key, value, maxAge) { + cache.set = function (key, value, ttl) { if (!cache.enabled) { return; } - cacheSet.apply(cache, [key, value, maxAge]); + const opts = {}; + if (ttl) { + opts.ttl = ttl; + } + cacheSet.apply(lruCache, [key, value, opts]); }; cache.get = function (key) { if (!cache.enabled) { return undefined; } - const data = cacheGet.apply(cache, [key]); + const data = lruCache.get(key); if (data === undefined) { cache.misses += 1; } else { @@ -41,16 +83,18 @@ module.exports = function (opts) { keys = [keys]; } pubsub.publish(`${cache.name}:cache:del`, keys); - keys.forEach(key => cacheDel.apply(cache, [key])); + keys.forEach(key => lruCache.delete(key)); }; + cache.delete = cache.del; cache.reset = function () { pubsub.publish(`${cache.name}:cache:reset`); localReset(); }; + cache.clear = cache.reset; function localReset() { - cacheReset.apply(cache); + lruCache.clear(); cache.hits = 0; cache.misses = 0; } @@ -61,7 +105,7 @@ module.exports = function (opts) { pubsub.on(`${cache.name}:cache:del`, (keys) => { if (Array.isArray(keys)) { - keys.forEach(key => cacheDel.apply(cache, [key])); + keys.forEach(key => lruCache.delete(key)); } }); @@ -87,5 +131,13 @@ module.exports = function (opts) { return unCachedKeys; }; + cache.dump = function () { + return lruCache.dump(); + }; + + cache.peek = function (key) { + return lruCache.peek(key); + }; + return cache; }; diff --git a/src/controllers/admin/cache.js b/src/controllers/admin/cache.js index 430cc28b8a..67c9864ae5 100644 --- a/src/controllers/admin/cache.js +++ b/src/controllers/admin/cache.js @@ -14,8 +14,11 @@ cacheController.get = function (req, res) { return { length: cache.length, max: cache.max, + maxSize: cache.maxSize, itemCount: cache.itemCount, - percentFull: ((cache.length / cache.max) * 100).toFixed(2), + percentFull: cache.name === 'post' ? + ((cache.length / cache.maxSize) * 100).toFixed(2) : + ((cache.itemCount / cache.max) * 100).toFixed(2), hits: utils.addCommas(String(cache.hits)), misses: utils.addCommas(String(cache.misses)), hitRatio: ((cache.hits / (cache.hits + cache.misses) || 0)).toFixed(4), diff --git a/src/database/cache.js b/src/database/cache.js index 069b181a80..999c860363 100644 --- a/src/database/cache.js +++ b/src/database/cache.js @@ -5,7 +5,6 @@ module.exports.create = function (name) { return cacheCreate({ name: `${name}-object`, max: 40000, - length: function () { return 1; }, - maxAge: 0, + ttl: 0, }); }; diff --git a/src/groups/cache.js b/src/groups/cache.js index 0acea646d6..b83e3e5928 100644 --- a/src/groups/cache.js +++ b/src/groups/cache.js @@ -6,7 +6,7 @@ module.exports = function (Groups) { Groups.cache = cacheCreate({ name: 'group', max: 40000, - maxAge: 0, + ttl: 0, }); Groups.clearCache = function (uid, groupNames) { diff --git a/src/middleware/index.js b/src/middleware/index.js index a31cc4430d..6930021aee 100644 --- a/src/middleware/index.js +++ b/src/middleware/index.js @@ -6,7 +6,6 @@ const csrf = require('csurf'); const validator = require('validator'); const nconf = require('nconf'); const toobusy = require('toobusy-js'); -const LRU = require('lru-cache'); const util = require('util'); const plugins = require('../plugins'); @@ -15,6 +14,7 @@ const user = require('../user'); const groups = require('../groups'); const analytics = require('../analytics'); const privileges = require('../privileges'); +const cacheCreate = require('../cacheCreate'); const helpers = require('./helpers'); const controllers = { @@ -22,8 +22,8 @@ const controllers = { helpers: require('../controllers/helpers'), }; -const delayCache = new LRU({ - maxAge: 1000 * 60, +const delayCache = cacheCreate({ + ttl: 1000 * 60, }); const middleware = module.exports; diff --git a/src/middleware/uploads.js b/src/middleware/uploads.js index 941b9c27bc..beb36c03e0 100644 --- a/src/middleware/uploads.js +++ b/src/middleware/uploads.js @@ -1,16 +1,16 @@ 'use strict'; -const LRU = require('lru-cache'); +const cacheCreate = require('../cacheCreate'); const meta = require('../meta'); const helpers = require('./helpers'); const user = require('../user'); -const cache = new LRU({ - maxAge: meta.config.uploadRateLimitCooldown * 1000, +const cache = cacheCreate({ + ttl: meta.config.uploadRateLimitCooldown * 1000, }); exports.clearCache = function () { - cache.reset(); + cache.clear(); }; exports.ratelimit = helpers.try(async (req, res, next) => { @@ -23,7 +23,6 @@ exports.ratelimit = helpers.try(async (req, res, next) => { if (count > meta.config.uploadRateLimitThreshold) { return next(new Error(['[[error:upload-ratelimit-reached]]'])); } - cache.set(`${req.ip}:uploaded_file_count`, count); next(); }); diff --git a/src/posts/cache.js b/src/posts/cache.js index 061c46d88d..246074697c 100644 --- a/src/posts/cache.js +++ b/src/posts/cache.js @@ -5,8 +5,8 @@ const meta = require('../meta'); module.exports = cacheCreate({ name: 'post', - max: meta.config.postCacheSize, - length: function (n) { return n.length; }, - maxAge: 0, + maxSize: meta.config.postCacheSize, + sizeCalculation: function (n) { return n.length; }, + ttl: 0, enabled: global.env === 'production', }); diff --git a/src/user/blocks.js b/src/user/blocks.js index 6655965f2a..f71589911c 100644 --- a/src/user/blocks.js +++ b/src/user/blocks.js @@ -9,8 +9,7 @@ module.exports = function (User) { _cache: cacheCreate({ name: 'user:blocks', max: 100, - length: function () { return 1; }, - maxAge: 0, + ttl: 0, }), }; diff --git a/src/views/admin/advanced/cache.tpl b/src/views/admin/advanced/cache.tpl index 8de6734da0..3ac75a0d38 100644 --- a/src/views/admin/advanced/cache.tpl +++ b/src/views/admin/advanced/cache.tpl @@ -12,7 +12,7 @@ - {postCache.length} / {postCache.max}
+ {postCache.length} / {postCache.maxSize}
@@ -46,7 +46,7 @@
- {objectCache.length} / {objectCache.max}
+ {objectCache.itemCount} / {objectCache.max}
[[admin/advanced/cache:percent-full, {objectCache.percentFull}]] @@ -72,7 +72,7 @@
- {groupCache.length} / {groupCache.max}
+ {groupCache.itemCount} / {groupCache.max}
@@ -98,7 +98,7 @@
- {localCache.length} / {localCache.max}
+ {localCache.itemCount} / {localCache.max}
diff --git a/test/middleware.js b/test/middleware.js index cdb2797360..47567717d1 100644 --- a/test/middleware.js +++ b/test/middleware.js @@ -4,7 +4,7 @@ const assert = require('assert'); const nconf = require('nconf'); const request = require('request-promise-native'); const db = require('./mocks/databasemock'); -const middleware = require('../src/middleware'); + const user = require('../src/user'); const groups = require('../src/groups'); const utils = require('../src/utils'); @@ -21,6 +21,7 @@ describe('Middlewares', () => { }); it('should expose res.locals.isAdmin = false', (done) => { + const middleware = require('../src/middleware'); const resMock = { locals: {} }; middleware.exposeAdmin({}, resMock, () => { assert.strictEqual(resMock.locals.isAdmin, false); @@ -29,6 +30,7 @@ describe('Middlewares', () => { }); it('should expose res.locals.isAdmin = true', (done) => { + const middleware = require('../src/middleware'); const reqMock = { user: { uid: adminUid } }; const resMock = { locals: {} }; middleware.exposeAdmin(reqMock, resMock, () => { @@ -38,6 +40,7 @@ describe('Middlewares', () => { }); it('should expose privileges in res.locals.privileges and isSelf=true', (done) => { + const middleware = require('../src/middleware'); const reqMock = { user: { uid: adminUid }, params: { uid: adminUid } }; const resMock = { locals: {} }; middleware.exposePrivileges(reqMock, resMock, () => { @@ -51,6 +54,7 @@ describe('Middlewares', () => { }); it('should expose privileges in res.locals.privileges and isSelf=false', (done) => { + const middleware = require('../src/middleware'); const reqMock = { user: { uid: 0 }, params: { uid: adminUid } }; const resMock = { locals: {} }; middleware.exposePrivileges(reqMock, resMock, () => { @@ -64,6 +68,7 @@ describe('Middlewares', () => { }); it('should expose privilege set', (done) => { + const middleware = require('../src/middleware'); const reqMock = { user: { uid: adminUid } }; const resMock = { locals: {} }; middleware.exposePrivilegeSet(reqMock, resMock, () => { diff --git a/test/mocks/databasemock.js b/test/mocks/databasemock.js index d9fd9ac647..d22e4c01b0 100644 --- a/test/mocks/databasemock.js +++ b/test/mocks/databasemock.js @@ -184,20 +184,21 @@ async function setupMockDefaults() { const meta = require('../../src/meta'); await db.emptydb(); - require('../../src/groups').cache.reset(); - require('../../src/posts/cache').reset(); - require('../../src/cache').reset(); - require('../../src/middleware/uploads').clearCache(); - winston.info('test_database flushed'); await setupDefaultConfigs(meta); - await giveDefaultGlobalPrivileges(); + await meta.configs.init(); meta.config.postDelay = 0; meta.config.initialPostDelay = 0; meta.config.newbiePostDelay = 0; meta.config.autoDetectLang = 0; + require('../../src/groups').cache.reset(); + require('../../src/posts/cache').reset(); + require('../../src/cache').reset(); + require('../../src/middleware/uploads').clearCache(); + // privileges must be given after cache reset + await giveDefaultGlobalPrivileges(); await enableDefaultPlugins(); await meta.themes.set({ diff --git a/test/uploads.js b/test/uploads.js index 2d57ea4535..8e851d153d 100644 --- a/test/uploads.js +++ b/test/uploads.js @@ -22,7 +22,6 @@ const helpers = require('./helpers'); const file = require('../src/file'); const image = require('../src/image'); -const uploadFile = util.promisify(helpers.uploadFile); const emptyUploadsFolder = async () => { const files = await fs.readdir(`${nconf.get('upload_path')}/files`); await Promise.all(files.map(async (filename) => { @@ -517,7 +516,7 @@ describe('Upload Controllers', () => { describe('.getOrphans()', () => { before(async () => { const { jar, csrf_token } = await helpers.loginUser('regular', 'zugzug'); - await uploadFile(`${nconf.get('url')}/api/post/upload`, path.join(__dirname, '../test/files/test.png'), {}, jar, csrf_token); + await helpers.uploadFile(`${nconf.get('url')}/api/post/upload`, path.join(__dirname, '../test/files/test.png'), {}, jar, csrf_token); }); it('should return files with no post associated with them', async () => { @@ -538,7 +537,7 @@ describe('Upload Controllers', () => { before(async () => { const { jar, csrf_token } = await helpers.loginUser('regular', 'zugzug'); - await uploadFile(`${nconf.get('url')}/api/post/upload`, path.join(__dirname, '../test/files/test.png'), {}, jar, csrf_token); + await helpers.uploadFile(`${nconf.get('url')}/api/post/upload`, path.join(__dirname, '../test/files/test.png'), {}, jar, csrf_token); // modify all files in uploads folder to be 30 days old const files = await fs.readdir(`${nconf.get('upload_path')}/files`);