From ca51111a40d483b8609b6825c0d236a5ae50e4c0 Mon Sep 17 00:00:00 2001 From: ckohen Date: Tue, 13 Jul 2021 04:16:08 -0700 Subject: [PATCH 01/23] feat(Util): add SweptCollection for auto sweeping of caches --- package-lock.json | 17 +++-- package.json | 2 +- src/client/Client.js | 2 +- src/index.js | 1 + src/util/Collection.js | 2 +- src/util/Options.js | 55 ++++++++++++++- src/util/SweptCollection.js | 129 ++++++++++++++++++++++++++++++++++++ typings/index.d.ts | 41 +++++++++++- 8 files changed, 236 insertions(+), 13 deletions(-) create mode 100644 src/util/SweptCollection.js diff --git a/package-lock.json b/package-lock.json index b0bc83102f60..ccb1f7fd34b8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "license": "Apache-2.0", "dependencies": { "@discordjs/builders": "^0.2.0", - "@discordjs/collection": "^0.1.6", + "@discordjs/collection": "^0.2.0", "@discordjs/form-data": "^3.0.1", "@sapphire/async-queue": "^1.1.4", "@types/ws": "^7.4.5", @@ -1008,9 +1008,12 @@ "integrity": "sha512-N82ooyxVNm6h1riLCoyS9e3fuJ3AMG2zIZs2Gd1ATcSFjSA23Q0fzjjZeh0jbJvWVDZ0cJT8yaNNaaXHzueNjg==" }, "node_modules/@discordjs/collection": { - "version": "0.1.6", - "resolved": "https://registry.npmjs.org/@discordjs/collection/-/collection-0.1.6.tgz", - "integrity": "sha512-utRNxnd9kSS2qhyivo9lMlt5qgAUasH2gb7BEOn6p0efFh24gjGomHzWKMAPn2hEReOPQZCJaRKoURwRotKucQ==" + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@discordjs/collection/-/collection-0.2.0.tgz", + "integrity": "sha512-ZSiyfQsQmJq5EDgTocUg6n7IOft64MH/53RC8q3+Z5Ltipgc6eH1lLyDMJz2fcY/xq5zrILd9LyqFgEdragDNA==", + "engines": { + "node": ">=14.0.0" + } }, "node_modules/@discordjs/docgen": { "version": "0.10.0", @@ -12231,9 +12234,9 @@ } }, "@discordjs/collection": { - "version": "0.1.6", - "resolved": "https://registry.npmjs.org/@discordjs/collection/-/collection-0.1.6.tgz", - "integrity": "sha512-utRNxnd9kSS2qhyivo9lMlt5qgAUasH2gb7BEOn6p0efFh24gjGomHzWKMAPn2hEReOPQZCJaRKoURwRotKucQ==" + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@discordjs/collection/-/collection-0.2.0.tgz", + "integrity": "sha512-ZSiyfQsQmJq5EDgTocUg6n7IOft64MH/53RC8q3+Z5Ltipgc6eH1lLyDMJz2fcY/xq5zrILd9LyqFgEdragDNA==" }, "@discordjs/docgen": { "version": "0.10.0", diff --git a/package.json b/package.json index d07277fc4b70..49dead745c24 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "homepage": "https://github.com/discordjs/discord.js#readme", "dependencies": { "@discordjs/builders": "^0.2.0", - "@discordjs/collection": "^0.1.6", + "@discordjs/collection": "^0.2.0", "@discordjs/form-data": "^3.0.1", "@sapphire/async-queue": "^1.1.4", "@types/ws": "^7.4.5", diff --git a/src/client/Client.js b/src/client/Client.js index dd00cb3f3f4c..32c04f9203a1 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -38,7 +38,7 @@ class Client extends BaseClient { super(Object.assign({ _tokenType: 'Bot' }, options)); const data = require('worker_threads').workerData ?? process.env; - const defaults = Options.createDefault(); + const defaults = Options.createDefault(this); if (this.options.shards === defaults.shards) { if ('SHARDS' in data) { diff --git a/src/index.js b/src/index.js index 477d454393bd..d23b2e9838c1 100644 --- a/src/index.js +++ b/src/index.js @@ -27,6 +27,7 @@ module.exports = { Permissions: require('./util/Permissions'), RateLimitError: require('./rest/RateLimitError'), SnowflakeUtil: require('./util/SnowflakeUtil'), + SweptCollection: require('./util/SweptCollection'), SystemChannelFlags: require('./util/SystemChannelFlags'), ThreadMemberFlags: require('./util/ThreadMemberFlags'), UserFlags: require('./util/UserFlags'), diff --git a/src/util/Collection.js b/src/util/Collection.js index 259244f15e19..7fbdaf9fc01a 100644 --- a/src/util/Collection.js +++ b/src/util/Collection.js @@ -1,6 +1,6 @@ 'use strict'; -const BaseCollection = require('@discordjs/collection'); +const BaseCollection = require('@discordjs/collection').Collection; const Util = require('./Util'); class Collection extends BaseCollection { diff --git a/src/util/Options.js b/src/util/Options.js index cb366cb09be8..720787643386 100644 --- a/src/util/Options.js +++ b/src/util/Options.js @@ -94,12 +94,16 @@ class Options extends null { /** * The default client options. + * @param {Client} [client] The client creating these options * @returns {ClientOptions} */ - static createDefault() { + static createDefault(client) { return { shardCount: 1, - makeCache: this.cacheWithLimits({ MessageManager: 200 }), + makeCache: this.cacheWithLimitsOrSweep({ + MessageManager: 200, + ThreadManager: { sweepArchivedOnly: true, client }, + }), messageCacheLifetime: 0, messageSweepInterval: 0, invalidRequestWarningInterval: 0, @@ -151,6 +155,53 @@ class Options extends null { }; } + /** + * Create a cache factory using predefined sweep settings. + * @param {Record} [settings={}] Settings for structures. + * @returns {CacheFactory} + */ + static cacheWithSweep(settings = {}) { + const Collection = require('./Collection'); + const SweptCollection = require('./SweptCollection'); + + return manager => { + const setting = settings[manager.name]; + if ( + setting === null || setting === undefined || typeof setting !== 'object' || setting.sweepInterval + ? setting.sweepInterval <= 0 || setting.sweepInterval === Infinity + : true + ) { + return new Collection(); + } + return new SweptCollection(setting); + }; + } + + /** + * Create a cache factory using predefined sweep settings or limits. + * @param {Record} [settings={}] Settings or Limits for structures. + * @returns {CacheFactory} + */ + static cacheWithLimitsOrSweep(settings = {}) { + const Collection = require('./Collection'); + const LimitedCollection = require('./LimitedCollection'); + const SweptCollection = require('./SweptCollection'); + + return manager => { + const setting = settings[manager.name]; + if (typeof setting === 'number' && setting !== Infinity) return LimitedCollection(setting); + if ( + setting === null || + setting === undefined || + typeof setting !== 'object' || + (setting.sweepInterval ? setting.sweepInterval <= 0 || setting.sweepInterval === Infinity : true) + ) { + return new Collection(); + } + return new SweptCollection(setting); + }; + } + /** * Create a cache factory that always caches everything. * @returns {CacheFactory} diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js new file mode 100644 index 000000000000..41b5a73ccf23 --- /dev/null +++ b/src/util/SweptCollection.js @@ -0,0 +1,129 @@ +'use strict'; + +const Collection = require('./Collection.js'); +const { TypeError } = require('../errors/DJSError.js'); + +/** + * Options for defining the behavior of a Swept Collection + * @typedef {Object} SweptCollectionOptions + * @property {Iterable} [iterable=null] Optional entries passed to the Map constructor. + * @property {Client} [client=null] A client that instantiates this and has a setInterval function + * @property {number} [maxSize=-1] The maximum size of the Collection + * @property {Function|null} [maxSizePredicate=null] A function used to determine which item to remove + * when reaching the specified max size. The function takes an entry as a paremeter and returns a boolean. + * This can allow the size to grow larger than max size when none of the entries pass the predicate. + * @property {Function|null} [sweepFunction=null] A custom function passed directly to `sweep()` every `sweepInterval`. + * See {@link [Collection#sweep](https://discord.js.org/#/docs/collection/master/class/Collection?scrollTo=sweep)} + * for the definition of this function. This overrides the build in function generated using the other properties. + * @property {number} [sweepLifetime=14400] How long an entry should stay in the collection + * before it is considered sweepable + * @property {string} [sweepLifetimeProperty='createdTimestamp'] The property of the entry to check when + * determining the entries lifetime. + * @property {boolean} [sweepArchivedOnly=false] Whether to only sweep entries with a property `archived: true`. + * E.g. for threads. + * @property {number} [sweepInterval=3600] How frequently, in seconds, to sweep the collection. + */ + +/** + * A Collection which holds a max amount of entries and sweeps periodically. + * The first key is deleted if the Collection has reached max size. + * @extends {Collection} + * @param {SweptCollectionOptions|Iterable} [options=null] Options for constructing the swept collection. + */ +class SweptCollection extends Collection { + constructor(options = null) { + if (typeof options !== 'object') throw new TypeError('INVALID_TYPE', 'options', 'object or iterable', true); + const { + maxSize = -1, + maxSizePredicate = null, + sweepFunction = null, + sweepLifetime = 14400, + sweepLifetimeProperty = 'createdTimestamp', + sweepInterval = 3600, + sweepArchivedOnly = false, + client = null, + iterable = null, + } = options; + + if (typeof maxSize !== 'number') throw new TypeError('INVALID_TYPE', 'maxSize', 'number'); + if (maxSizePredicate !== null && typeof maxSizePredicate !== 'function') { + throw new TypeError('INVALID_TYPE', 'maxSizePreidcate', 'function'); + } + if (sweepFunction !== null && typeof sweepFunction !== 'function') { + throw new TypeError('INVALID_TYPE', 'sweepFunction', 'function'); + } + if (typeof sweepFunction !== 'function' && typeof sweepLifetime !== 'number') { + throw new TypeError('INVALID_TYPE', 'sweepLifetime', 'number'); + } + if (typeof sweepFunction !== 'function' && typeof sweepLifetimeProperty !== 'string') { + throw new TypeError('INVALID_TYPE', 'sweepLifetimeProperty', 'number'); + } + if (typeof sweepInterval !== 'number') throw new TypeError('INVALID_TYPE', 'sweepInterval', 'number'); + if (typeof sweepFunction !== 'function' && typeof sweepArchivedOnly !== 'boolean') { + throw new TypeError('INVALID_TYPE', 'sweepArchivedOnly', 'boolean'); + } + if (typeof client !== 'object') throw new TypeError('INVALID_TYPE', 'client', 'Client'); + + const definiteIterable = options[Symbol.iterator] ? options : iterable; + super(definiteIterable); + + /** + * The client used to setInterval on, uses global setInterval if not provided. + * @type {?Client} + */ + this.client = client; + + /** + * The max size of the Collection. + * @type {number} + */ + this.maxSize = maxSize; + + /** + * A predicate for determining which item to remove when the Collection reaches max size. + * Using this can allow the size to grow larger than max size if none of the entries match the predicate. + * @type {?Function} + */ + this.maxSizePredicate = maxSizePredicate; + + const intervalFunction = sweepFunction + ? () => this.sweep(sweepFunction) + : () => { + if (sweepLifetime <= 0) return; + const lifetimeMs = sweepLifetime * 1000; + const now = Date.now(); + this.sweep(entry => { + if (!entry[sweepLifetimeProperty]) return false; + if (sweepArchivedOnly && !entry.archived) return false; + return now - entry[sweepLifetimeProperty] > lifetimeMs; + }); + }; + + this.interval = + sweepInterval > 0 + ? typeof this.client?.setInterval === 'function' + ? this.client.setInterval(intervalFunction.bind(this), sweepInterval * 1000) + : setInterval(intervalFunction.bind(this), sweepInterval * 1000) + : null; + } + + set(key, value) { + if (this.maxSize === 0) return this; + if (this.maxSize === -1 || this.maxSize === Infinity) super.set(key, value); + if (this.size >= this.maxSize && !this.has(key)) { + if (this.maxSizePredicate) { + for (const [k, v] of this) { + if (this.maxSizePredicate(v)) { + this.delete(k); + break; + } + } + } else { + this.delete(this.firstKey()); + } + } + return super.set(key, value); + } +} + +module.exports = SweptCollection; diff --git a/typings/index.d.ts b/typings/index.d.ts index 281fa3a2e91c..442de4d41166 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -357,8 +357,20 @@ export class ClientUser extends User { export class Options extends null { private constructor(); - public static createDefaultOptions(): ClientOptions; + public static createDefaultOptions(client?: Client): ClientOptions; public static cacheWithLimits(limits?: CacheWithLimitOptions): CacheFactory; + public static cacheWithLimitsOrSweep( + settings?: Record< + string, + SweptCollectionOptions | SweptCollectionFunctionOptions | number + >, + ): CacheFactory; + public static cacheWithSweep( + settings?: Record< + string, + SweptCollectionOptions | SweptCollectionFunctionOptions + >, + ): CacheFactory; public static cacheEverything(): CacheFactory; } @@ -1613,6 +1625,15 @@ export class StoreChannel extends GuildChannel { public type: 'GUILD_STORE'; } +export class SweptCollection extends Collection { + public constructor(options: SweptCollectionOptions | SweptCollectionFunctionOptions); + public constructor(iterable?: Iterable); + public client: Client | null; + public maxSize: number; + public maxSizePredicate: ((value: V) => boolean) | null; + public interval: number | null; +} + export class SystemChannelFlags extends BitField { public static FLAGS: Record; public static resolve(bit?: BitFieldResolvable): number; @@ -4278,6 +4299,24 @@ export interface StageInstanceEditOptions { privacyLevel?: PrivacyLevel | number; } +export interface SweptCollectionBaseOptions { + client?: Client; + iterable?: Iterable; + maxSize?: number; + maxSizePredicate?: (value: V) => boolean; + sweepInterval?: number; +} + +export interface SweptCollectionFunctionOptions extends SweptCollectionBaseOptions { + sweepFunction: (value: V, key: K, collection: this) => boolean; +} + +export interface SweptCollectionOptions extends SweptCollectionBaseOptions { + sweepArchivedOnly?: boolean; + sweepLifetime?: number; + sweepLifetimeProperty?: string; +} + export type TextBasedChannelTypes = | 'DM' | 'GUILD_TEXT' From 693839de30ed6b72d7cb9833edff1589c29868cc Mon Sep 17 00:00:00 2001 From: ckohen Date: Tue, 13 Jul 2021 04:22:31 -0700 Subject: [PATCH 02/23] refactor: remove methods that no longer exist --- src/managers/GuildEmojiRoleManager.js | 4 ++-- src/managers/GuildMemberRoleManager.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/managers/GuildEmojiRoleManager.js b/src/managers/GuildEmojiRoleManager.js index 784ad4faf09e..c05da621b82a 100644 --- a/src/managers/GuildEmojiRoleManager.js +++ b/src/managers/GuildEmojiRoleManager.js @@ -72,7 +72,7 @@ class GuildEmojiRoleManager extends DataManager { resolvedRoleIds.push(roleId); } - const newRoles = this.cache.keyArray().filter(id => !resolvedRoleIds.includes(id)); + const newRoles = [...this.cache.keys()].filter(id => !resolvedRoleIds.includes(id)); return this.set(newRoles); } @@ -97,7 +97,7 @@ class GuildEmojiRoleManager extends DataManager { clone() { const clone = new this.constructor(this.emoji); - clone._patch(this.cache.keyArray().slice()); + clone._patch([...this.cache.keys()].slice()); return clone; } diff --git a/src/managers/GuildMemberRoleManager.js b/src/managers/GuildMemberRoleManager.js index f060c7e5449b..ed5f5601770e 100644 --- a/src/managers/GuildMemberRoleManager.js +++ b/src/managers/GuildMemberRoleManager.js @@ -172,7 +172,7 @@ class GuildMemberRoleManager extends DataManager { clone() { const clone = new this.constructor(this.member); - clone.member._roles = [...this.cache.keyArray()]; + clone.member._roles = [...this.cache.keys()]; return clone; } } From 72c0ba4a451606e9584cb7fe2d5bc1cdb463612c Mon Sep 17 00:00:00 2001 From: ckohen Date: Tue, 13 Jul 2021 11:33:14 -0700 Subject: [PATCH 03/23] refactor: from pr review Co-Authored-By: DTrombett <73136330+DTrombett@users.noreply.github.com> --- src/util/Options.js | 7 +++---- src/util/SweptCollection.js | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/util/Options.js b/src/util/Options.js index 720787643386..cb01b176d91b 100644 --- a/src/util/Options.js +++ b/src/util/Options.js @@ -167,9 +167,9 @@ class Options extends null { return manager => { const setting = settings[manager.name]; if ( - setting === null || setting === undefined || typeof setting !== 'object' || setting.sweepInterval - ? setting.sweepInterval <= 0 || setting.sweepInterval === Infinity - : true + setting === null || + typeof setting !== 'object' || + (setting.sweepInterval ? setting.sweepInterval <= 0 || setting.sweepInterval === Infinity : true) ) { return new Collection(); } @@ -192,7 +192,6 @@ class Options extends null { if (typeof setting === 'number' && setting !== Infinity) return LimitedCollection(setting); if ( setting === null || - setting === undefined || typeof setting !== 'object' || (setting.sweepInterval ? setting.sweepInterval <= 0 || setting.sweepInterval === Infinity : true) ) { diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js index 41b5a73ccf23..57a1e1fb6545 100644 --- a/src/util/SweptCollection.js +++ b/src/util/SweptCollection.js @@ -102,8 +102,8 @@ class SweptCollection extends Collection { this.interval = sweepInterval > 0 ? typeof this.client?.setInterval === 'function' - ? this.client.setInterval(intervalFunction.bind(this), sweepInterval * 1000) - : setInterval(intervalFunction.bind(this), sweepInterval * 1000) + ? this.client.setInterval(intervalFunction, sweepInterval * 1000) + : setInterval(intervalFunction, sweepInterval * 1000) : null; } From f393e5ff347cf81e0dfffb66c107f8932349f009 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 14 Jul 2021 03:13:31 -0700 Subject: [PATCH 04/23] refactor: general cleaniness and better interface Co-Authored-By: 1Computer1 <22125769+1Computer1@users.noreply.github.com> --- src/client/Client.js | 2 +- src/util/Options.js | 10 ++- src/util/SweptCollection.js | 151 +++++++++++++++++++----------------- typings/index.d.ts | 48 +++++------- 4 files changed, 106 insertions(+), 105 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index 32c04f9203a1..dd00cb3f3f4c 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -38,7 +38,7 @@ class Client extends BaseClient { super(Object.assign({ _tokenType: 'Bot' }, options)); const data = require('worker_threads').workerData ?? process.env; - const defaults = Options.createDefault(this); + const defaults = Options.createDefault(); if (this.options.shards === defaults.shards) { if ('SHARDS' in data) { diff --git a/src/util/Options.js b/src/util/Options.js index cb01b176d91b..1628a617bb09 100644 --- a/src/util/Options.js +++ b/src/util/Options.js @@ -94,15 +94,19 @@ class Options extends null { /** * The default client options. - * @param {Client} [client] The client creating these options * @returns {ClientOptions} */ - static createDefault(client) { + static createDefault() { return { shardCount: 1, makeCache: this.cacheWithLimitsOrSweep({ MessageManager: 200, - ThreadManager: { sweepArchivedOnly: true, client }, + ThreadManager: { + sweepInterval: require('./SweptCollection').filterByLiftetme({ + getComparisonTimestamp: e => e.archiveTimestamp, + excludeFromSweep: e => !e.archived, + }), + }, }), messageCacheLifetime: 0, messageSweepInterval: 0, diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js index 57a1e1fb6545..71cb178911af 100644 --- a/src/util/SweptCollection.js +++ b/src/util/SweptCollection.js @@ -6,72 +6,42 @@ const { TypeError } = require('../errors/DJSError.js'); /** * Options for defining the behavior of a Swept Collection * @typedef {Object} SweptCollectionOptions - * @property {Iterable} [iterable=null] Optional entries passed to the Map constructor. - * @property {Client} [client=null] A client that instantiates this and has a setInterval function * @property {number} [maxSize=-1] The maximum size of the Collection - * @property {Function|null} [maxSizePredicate=null] A function used to determine which item to remove - * when reaching the specified max size. The function takes an entry as a paremeter and returns a boolean. - * This can allow the size to grow larger than max size when none of the entries pass the predicate. - * @property {Function|null} [sweepFunction=null] A custom function passed directly to `sweep()` every `sweepInterval`. + * @property {Function|null} [keepAtMaxSize=null] A function used to exclude some entries from being removed + * when reaching the specified max size. + * The function takes an entry as a paremeter and returns a boolean, `true` to keep. + * When the function returns `true` for every entry, + * the first entry will be removed to maintain size + * @property {Function|null} [sweepFilter=null] A custom function taking no parameters, run every every `sweepInterval`. + * The return value of this function is a function passed to `sweep()`, * See {@link [Collection#sweep](https://discord.js.org/#/docs/collection/master/class/Collection?scrollTo=sweep)} - * for the definition of this function. This overrides the build in function generated using the other properties. - * @property {number} [sweepLifetime=14400] How long an entry should stay in the collection - * before it is considered sweepable - * @property {string} [sweepLifetimeProperty='createdTimestamp'] The property of the entry to check when - * determining the entries lifetime. - * @property {boolean} [sweepArchivedOnly=false] Whether to only sweep entries with a property `archived: true`. - * E.g. for threads. + * for the definition of this function. * @property {number} [sweepInterval=3600] How frequently, in seconds, to sweep the collection. */ /** * A Collection which holds a max amount of entries and sweeps periodically. - * The first key is deleted if the Collection has reached max size. * @extends {Collection} - * @param {SweptCollectionOptions|Iterable} [options=null] Options for constructing the swept collection. + * @param {SweptCollectionOptions} [options={}] Options for constructing the swept collection. + * @param {Iterable} [iterable=null] Optional entries passed to the Map constructor. */ class SweptCollection extends Collection { - constructor(options = null) { - if (typeof options !== 'object') throw new TypeError('INVALID_TYPE', 'options', 'object or iterable', true); - const { - maxSize = -1, - maxSizePredicate = null, - sweepFunction = null, - sweepLifetime = 14400, - sweepLifetimeProperty = 'createdTimestamp', - sweepInterval = 3600, - sweepArchivedOnly = false, - client = null, - iterable = null, - } = options; + constructor(options = {}, iterable) { + if (typeof options !== 'object' || options === null) { + throw new TypeError('INVALID_TYPE', 'options', 'object or iterable', true); + } + const { maxSize = -1, keepAtMaxSize = null, sweepFilter = null, sweepInterval = 3600 } = options; if (typeof maxSize !== 'number') throw new TypeError('INVALID_TYPE', 'maxSize', 'number'); - if (maxSizePredicate !== null && typeof maxSizePredicate !== 'function') { + if (keepAtMaxSize !== null && typeof keepAtMaxSize !== 'function') { throw new TypeError('INVALID_TYPE', 'maxSizePreidcate', 'function'); } - if (sweepFunction !== null && typeof sweepFunction !== 'function') { + if (sweepFilter !== null && typeof sweepFilter !== 'function') { throw new TypeError('INVALID_TYPE', 'sweepFunction', 'function'); } - if (typeof sweepFunction !== 'function' && typeof sweepLifetime !== 'number') { - throw new TypeError('INVALID_TYPE', 'sweepLifetime', 'number'); - } - if (typeof sweepFunction !== 'function' && typeof sweepLifetimeProperty !== 'string') { - throw new TypeError('INVALID_TYPE', 'sweepLifetimeProperty', 'number'); - } if (typeof sweepInterval !== 'number') throw new TypeError('INVALID_TYPE', 'sweepInterval', 'number'); - if (typeof sweepFunction !== 'function' && typeof sweepArchivedOnly !== 'boolean') { - throw new TypeError('INVALID_TYPE', 'sweepArchivedOnly', 'boolean'); - } - if (typeof client !== 'object') throw new TypeError('INVALID_TYPE', 'client', 'Client'); - - const definiteIterable = options[Symbol.iterator] ? options : iterable; - super(definiteIterable); - /** - * The client used to setInterval on, uses global setInterval if not provided. - * @type {?Client} - */ - this.client = client; + super(iterable); /** * The max size of the Collection. @@ -80,50 +50,89 @@ class SweptCollection extends Collection { this.maxSize = maxSize; /** - * A predicate for determining which item to remove when the Collection reaches max size. - * Using this can allow the size to grow larger than max size if none of the entries match the predicate. + * A function called to exclude items from being removed when the Collection reaches max size. + * When the function returns `true` for every entry, the first entry is removed to maintain size * @type {?Function} */ - this.maxSizePredicate = maxSizePredicate; + this.keepAtMaxSize = keepAtMaxSize; - const intervalFunction = sweepFunction - ? () => this.sweep(sweepFunction) - : () => { - if (sweepLifetime <= 0) return; - const lifetimeMs = sweepLifetime * 1000; - const now = Date.now(); - this.sweep(entry => { - if (!entry[sweepLifetimeProperty]) return false; - if (sweepArchivedOnly && !entry.archived) return false; - return now - entry[sweepLifetimeProperty] > lifetimeMs; - }); - }; + /** + * A function called every sweep interval that returns a function passed to `sweep` + * @type {?Function} + */ + this.sweepFilter = sweepFilter; + /** + * The id of the interval being used to sweep. + * @type {?Number} + */ this.interval = - sweepInterval > 0 - ? typeof this.client?.setInterval === 'function' - ? this.client.setInterval(intervalFunction, sweepInterval * 1000) - : setInterval(intervalFunction, sweepInterval * 1000) - : null; + sweepInterval > 0 && sweepFilter ? setInterval(() => this.sweep(this.sweepFilter()), sweepInterval * 1000) : null; } set(key, value) { if (this.maxSize === 0) return this; if (this.maxSize === -1 || this.maxSize === Infinity) super.set(key, value); if (this.size >= this.maxSize && !this.has(key)) { - if (this.maxSizePredicate) { + let deleted = false; + if (this.keepAtMaxSize) { for (const [k, v] of this) { - if (this.maxSizePredicate(v)) { + if (!this.keepAtMaxSize(v)) { this.delete(k); + deleted = true; break; } } - } else { + } + if (!deleted) { this.delete(this.firstKey()); } } return super.set(key, value); } + + /** + * Options for generating a filter function based on lifetime + * @typedef {Object} LifetimeFilterOptions + * @property {number} [lifetime=14400] How long an entry should stay in the collection + * before it is considered sweepable + * @property {Function} [getComparisonTimestamp=`e => e.createdTimestamp`] A function that takes an entry + * and returns a timestamp to compare against in order to determine the lifetime of the entry. + * @property {Function} [excludeFromSweep=null] A function that takes an entry and returns a boolean, + * `true` when the entry should not be checked for sweepability. + */ + + /** + * Create a sweepFilter function that uses a lifetime to determine sweepability. + * @param {LifetimeFilterOptions} [options={}] The options used to generate the filter function + * @returns {Function} + */ + static filterByLiftetme({ + lifetime = 14400, + getComparisonTimestamp = e => e?.createdTimestamp, + excludeFromSweep = null, + } = {}) { + if (typeof lifetime !== 'number') throw new TypeError('INVALID_TYPE', 'lifetime', 'number'); + if (typeof getComparisonTimestamp !== 'function') { + throw new TypeError('INVALID_TYPE', 'getComparisonTimestamp', 'function'); + } + if (excludeFromSweep !== null && typeof excludeFromSweep !== 'function') { + throw new TypeError('INVALID_TYPE', 'excludeFromSweep', 'function'); + } + return () => { + if (lifetime <= 0) return () => false; + const lifetimeMs = lifetime * 1000; + const now = Date.now(); + return entry => { + if (excludeFromSweep?.(entry)) { + return false; + } + const comparisonTimestamp = getComparisonTimestamp(entry); + if (!comparisonTimestamp || typeof comparisonTimestamp !== 'number') return false; + return now - comparisonTimestamp > lifetimeMs; + }; + }; + } } module.exports = SweptCollection; diff --git a/typings/index.d.ts b/typings/index.d.ts index 442de4d41166..92bbc322484d 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -357,20 +357,12 @@ export class ClientUser extends User { export class Options extends null { private constructor(); - public static createDefaultOptions(client?: Client): ClientOptions; + public static createDefaultOptions(): ClientOptions; public static cacheWithLimits(limits?: CacheWithLimitOptions): CacheFactory; public static cacheWithLimitsOrSweep( - settings?: Record< - string, - SweptCollectionOptions | SweptCollectionFunctionOptions | number - >, - ): CacheFactory; - public static cacheWithSweep( - settings?: Record< - string, - SweptCollectionOptions | SweptCollectionFunctionOptions - >, + settings?: Record | number>, ): CacheFactory; + public static cacheWithSweep(settings?: Record>): CacheFactory; public static cacheEverything(): CacheFactory; } @@ -1626,12 +1618,13 @@ export class StoreChannel extends GuildChannel { } export class SweptCollection extends Collection { - public constructor(options: SweptCollectionOptions | SweptCollectionFunctionOptions); - public constructor(iterable?: Iterable); - public client: Client | null; - public maxSize: number; - public maxSizePredicate: ((value: V) => boolean) | null; + public constructor(options?: SweptCollectionOptions, iterable?: Iterable); + public keepAtMaxSize: ((value: V) => boolean) | null; public interval: number | null; + public maxSize: number; + public sweepFilter: (() => (value: V) => boolean) | null; + + public static filterByLifetime(options?: LifetimeFilterOptions): () => (value: any) => boolean; } export class SystemChannelFlags extends BitField { @@ -3779,6 +3772,12 @@ export type InviteScope = | 'gdm.join' | 'webhook.incoming'; +export interface LifetimeFilterOptions { + excludeFromSweep?: (value: any) => boolean; + getComparisonTimestamp?: (value: any) => number; + lifetime?: number; +} + export interface MakeErrorOptions { name: string; message: string; @@ -4299,24 +4298,13 @@ export interface StageInstanceEditOptions { privacyLevel?: PrivacyLevel | number; } -export interface SweptCollectionBaseOptions { - client?: Client; - iterable?: Iterable; +export interface SweptCollectionOptions { + keepAtMaxSize?: (value: V) => boolean; maxSize?: number; - maxSizePredicate?: (value: V) => boolean; + sweepFilter?: () => (value: V) => boolean; sweepInterval?: number; } -export interface SweptCollectionFunctionOptions extends SweptCollectionBaseOptions { - sweepFunction: (value: V, key: K, collection: this) => boolean; -} - -export interface SweptCollectionOptions extends SweptCollectionBaseOptions { - sweepArchivedOnly?: boolean; - sweepLifetime?: number; - sweepLifetimeProperty?: string; -} - export type TextBasedChannelTypes = | 'DM' | 'GUILD_TEXT' From 79d2a65faadf15081d22d67139e7dc6771a71653 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 14 Jul 2021 03:34:08 -0700 Subject: [PATCH 05/23] fix: typos and better default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Antonio Román --- src/util/Options.js | 2 +- src/util/SweptCollection.js | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/util/Options.js b/src/util/Options.js index 1628a617bb09..dd68149c828c 100644 --- a/src/util/Options.js +++ b/src/util/Options.js @@ -102,7 +102,7 @@ class Options extends null { makeCache: this.cacheWithLimitsOrSweep({ MessageManager: 200, ThreadManager: { - sweepInterval: require('./SweptCollection').filterByLiftetme({ + sweepInterval: require('./SweptCollection').filterByLifetime({ getComparisonTimestamp: e => e.archiveTimestamp, excludeFromSweep: e => !e.archived, }), diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js index 71cb178911af..12fbc0738707 100644 --- a/src/util/SweptCollection.js +++ b/src/util/SweptCollection.js @@ -67,7 +67,9 @@ class SweptCollection extends Collection { * @type {?Number} */ this.interval = - sweepInterval > 0 && sweepFilter ? setInterval(() => this.sweep(this.sweepFilter()), sweepInterval * 1000) : null; + sweepInterval > 0 && sweepFilter + ? setInterval(() => this.sweep(this.sweepFilter()), sweepInterval * 1000).unref() + : null; } set(key, value) { @@ -98,7 +100,7 @@ class SweptCollection extends Collection { * before it is considered sweepable * @property {Function} [getComparisonTimestamp=`e => e.createdTimestamp`] A function that takes an entry * and returns a timestamp to compare against in order to determine the lifetime of the entry. - * @property {Function} [excludeFromSweep=null] A function that takes an entry and returns a boolean, + * @property {Function} [excludeFromSweep=`() => false)`] A function that takes an entry and returns a boolean, * `true` when the entry should not be checked for sweepability. */ @@ -107,16 +109,16 @@ class SweptCollection extends Collection { * @param {LifetimeFilterOptions} [options={}] The options used to generate the filter function * @returns {Function} */ - static filterByLiftetme({ + static filterByLifetime({ lifetime = 14400, getComparisonTimestamp = e => e?.createdTimestamp, - excludeFromSweep = null, + excludeFromSweep = () => false, } = {}) { if (typeof lifetime !== 'number') throw new TypeError('INVALID_TYPE', 'lifetime', 'number'); if (typeof getComparisonTimestamp !== 'function') { throw new TypeError('INVALID_TYPE', 'getComparisonTimestamp', 'function'); } - if (excludeFromSweep !== null && typeof excludeFromSweep !== 'function') { + if (typeof excludeFromSweep !== 'function') { throw new TypeError('INVALID_TYPE', 'excludeFromSweep', 'function'); } return () => { @@ -124,7 +126,7 @@ class SweptCollection extends Collection { const lifetimeMs = lifetime * 1000; const now = Date.now(); return entry => { - if (excludeFromSweep?.(entry)) { + if (excludeFromSweep(entry)) { return false; } const comparisonTimestamp = getComparisonTimestamp(entry); From 49d7f4e801bd01315ffa4bdd02962de6eaff4209 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 14 Jul 2021 10:36:33 -0700 Subject: [PATCH 06/23] refactor: single function for caching with a different collection Co-Authored-By: 1Computer1 <22125769+1Computer1@users.noreply.github.com> --- src/util/Options.js | 59 ++++++++++--------------------------- src/util/SweptCollection.js | 4 +++ typings/index.d.ts | 10 ++----- 3 files changed, 22 insertions(+), 51 deletions(-) diff --git a/src/util/Options.js b/src/util/Options.js index dd68149c828c..2f189adab3a5 100644 --- a/src/util/Options.js +++ b/src/util/Options.js @@ -99,7 +99,7 @@ class Options extends null { static createDefault() { return { shardCount: 1, - makeCache: this.cacheWithLimitsOrSweep({ + makeCache: this.cacheSome({ MessageManager: 200, ThreadManager: { sweepInterval: require('./SweptCollection').filterByLifetime({ @@ -142,51 +142,22 @@ class Options extends null { } /** - * Create a cache factory using predefined limits. - * @param {Record} [limits={}] Limits for structures. + * Create a cache factory using predefined settings to sweep or limit. + * @param {Record} [settings={}] Settings passed to the relevant constructor. + * If no setting is provided for a manager, it uses Collection. * @returns {CacheFactory} + * @example + * Options.cacheSome({ + * MessageManager: 200, + * ThreadManager: { + * sweepInterval: SweptCollection.filterByLifetime({ + * getComparisonTimestamp: e => e.archiveTimestamp, + * excludeFromSweep: e => !e.archived, + * }), + * }, + * }); */ - static cacheWithLimits(limits = {}) { - const Collection = require('./Collection'); - const LimitedCollection = require('./LimitedCollection'); - - return manager => { - const limit = limits[manager.name]; - if (limit === null || limit === undefined || limit === Infinity) { - return new Collection(); - } - return new LimitedCollection(limit); - }; - } - - /** - * Create a cache factory using predefined sweep settings. - * @param {Record} [settings={}] Settings for structures. - * @returns {CacheFactory} - */ - static cacheWithSweep(settings = {}) { - const Collection = require('./Collection'); - const SweptCollection = require('./SweptCollection'); - - return manager => { - const setting = settings[manager.name]; - if ( - setting === null || - typeof setting !== 'object' || - (setting.sweepInterval ? setting.sweepInterval <= 0 || setting.sweepInterval === Infinity : true) - ) { - return new Collection(); - } - return new SweptCollection(setting); - }; - } - - /** - * Create a cache factory using predefined sweep settings or limits. - * @param {Record} [settings={}] Settings or Limits for structures. - * @returns {CacheFactory} - */ - static cacheWithLimitsOrSweep(settings = {}) { + static cacheSome(settings = {}) { const Collection = require('./Collection'); const LimitedCollection = require('./LimitedCollection'); const SweptCollection = require('./SweptCollection'); diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js index 12fbc0738707..1e173b78e600 100644 --- a/src/util/SweptCollection.js +++ b/src/util/SweptCollection.js @@ -135,6 +135,10 @@ class SweptCollection extends Collection { }; }; } + + static get [Symbol.species]() { + return Collection; + } } module.exports = SweptCollection; diff --git a/typings/index.d.ts b/typings/index.d.ts index 92bbc322484d..7b4231ae7b0f 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -358,11 +358,7 @@ export class ClientUser extends User { export class Options extends null { private constructor(); public static createDefaultOptions(): ClientOptions; - public static cacheWithLimits(limits?: CacheWithLimitOptions): CacheFactory; - public static cacheWithLimitsOrSweep( - settings?: Record | number>, - ): CacheFactory; - public static cacheWithSweep(settings?: Record>): CacheFactory; + public static cacheSome(settings?: CacheSomeOptions): CacheFactory; public static cacheEverything(): CacheFactory; } @@ -2884,8 +2880,8 @@ export interface CacheFactoryArgs { VoiceStateManager: [manager: typeof VoiceStateManager, holds: typeof VoiceState]; } -export type CacheWithLimitOptions = { - [K in CachedManagerTypes]?: number; +export type CacheSomeOptions = { + [K in CachedManagerTypes]?: SweptCollectionOptions | number; }; export interface ChannelCreationOverwrites { From 6116192cfdf82c384b6878fb76f18358009c3691 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 14 Jul 2021 10:40:40 -0700 Subject: [PATCH 07/23] types: fix type checks --- typings/index.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/typings/index.ts b/typings/index.ts index 6cdc2d4c478c..8cb5fb08d4df 100644 --- a/typings/index.ts +++ b/typings/index.ts @@ -54,10 +54,16 @@ import { const client: Client = new Client({ intents: Intents.FLAGS.GUILDS, - makeCache: Options.cacheWithLimits({ + makeCache: Options.cacheSome({ MessageManager: 200, // @ts-expect-error Message: 100, + ThreadManager: { + sweepInterval: require('./SweptCollection').filterByLifetime({ + getComparisonTimestamp: (e: any) => e.archiveTimestamp, + excludeFromSweep: (e: any) => !e.archived, + }), + }, }), }); From 3fd1589adfea54f1093b1580f451fe1ffd6c10a1 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 14 Jul 2021 13:11:31 -0700 Subject: [PATCH 08/23] refactor: remove LimitedCollection likes --- src/errors/Messages.js | 2 ++ src/util/SweptCollection.js | 68 ++++++++++--------------------------- typings/index.d.ts | 8 ++--- 3 files changed, 21 insertions(+), 57 deletions(-) diff --git a/src/errors/Messages.js b/src/errors/Messages.js index 91ad16c263ab..e745d83204c3 100644 --- a/src/errors/Messages.js +++ b/src/errors/Messages.js @@ -143,6 +143,8 @@ const Messages = { INVITE_MISSING_SCOPES: 'At least one valid scope must be provided for the invite', NOT_IMPLEMENTED: (what, name) => `Method ${what} not implemented on ${name}.`, + + SWEEP_FILTER_RETURN: 'The return value of the sweepFilter function was not false or a Function', }; for (const [name, message] of Object.entries(Messages)) register(name, message); diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js index 1e173b78e600..3fffc5e8ea6d 100644 --- a/src/util/SweptCollection.js +++ b/src/util/SweptCollection.js @@ -4,18 +4,17 @@ const Collection = require('./Collection.js'); const { TypeError } = require('../errors/DJSError.js'); /** - * Options for defining the behavior of a Swept Collection - * @typedef {Object} SweptCollectionOptions - * @property {number} [maxSize=-1] The maximum size of the Collection - * @property {Function|null} [keepAtMaxSize=null] A function used to exclude some entries from being removed - * when reaching the specified max size. - * The function takes an entry as a paremeter and returns a boolean, `true` to keep. - * When the function returns `true` for every entry, - * the first entry will be removed to maintain size - * @property {Function|null} [sweepFilter=null] A custom function taking no parameters, run every every `sweepInterval`. - * The return value of this function is a function passed to `sweep()`, + * @typedef {Function} SweepFilter + * @param {SweptCollection} collection The collection being swept + * @returns {Function|false} Return `false` to skip sweeping, otherwise a function passed to `sweep()`, * See {@link [Collection#sweep](https://discord.js.org/#/docs/collection/master/class/Collection?scrollTo=sweep)} * for the definition of this function. + */ + +/** + * Options for defining the behavior of a Swept Collection + * @typedef {Object} SweptCollectionOptions + * @property {Function|null} [sweepFilter=null] A function run every `sweepInterval` to determine how to sweep * @property {number} [sweepInterval=3600] How frequently, in seconds, to sweep the collection. */ @@ -30,12 +29,8 @@ class SweptCollection extends Collection { if (typeof options !== 'object' || options === null) { throw new TypeError('INVALID_TYPE', 'options', 'object or iterable', true); } - const { maxSize = -1, keepAtMaxSize = null, sweepFilter = null, sweepInterval = 3600 } = options; + const { sweepFilter = null, sweepInterval = 3600 } = options; - if (typeof maxSize !== 'number') throw new TypeError('INVALID_TYPE', 'maxSize', 'number'); - if (keepAtMaxSize !== null && typeof keepAtMaxSize !== 'function') { - throw new TypeError('INVALID_TYPE', 'maxSizePreidcate', 'function'); - } if (sweepFilter !== null && typeof sweepFilter !== 'function') { throw new TypeError('INVALID_TYPE', 'sweepFunction', 'function'); } @@ -43,19 +38,6 @@ class SweptCollection extends Collection { super(iterable); - /** - * The max size of the Collection. - * @type {number} - */ - this.maxSize = maxSize; - - /** - * A function called to exclude items from being removed when the Collection reaches max size. - * When the function returns `true` for every entry, the first entry is removed to maintain size - * @type {?Function} - */ - this.keepAtMaxSize = keepAtMaxSize; - /** * A function called every sweep interval that returns a function passed to `sweep` * @type {?Function} @@ -68,31 +50,15 @@ class SweptCollection extends Collection { */ this.interval = sweepInterval > 0 && sweepFilter - ? setInterval(() => this.sweep(this.sweepFilter()), sweepInterval * 1000).unref() + ? setInterval(() => { + const sweepFn = this.sweepFilter(this); + if (sweepFn === false) return; + if (typeof sweepFn !== 'function') throw new TypeError('SWEEP_FILTER_RETURN'); + this.sweep(sweepFn); + }, sweepInterval * 1000).unref() : null; } - set(key, value) { - if (this.maxSize === 0) return this; - if (this.maxSize === -1 || this.maxSize === Infinity) super.set(key, value); - if (this.size >= this.maxSize && !this.has(key)) { - let deleted = false; - if (this.keepAtMaxSize) { - for (const [k, v] of this) { - if (!this.keepAtMaxSize(v)) { - this.delete(k); - deleted = true; - break; - } - } - } - if (!deleted) { - this.delete(this.firstKey()); - } - } - return super.set(key, value); - } - /** * Options for generating a filter function based on lifetime * @typedef {Object} LifetimeFilterOptions @@ -107,7 +73,7 @@ class SweptCollection extends Collection { /** * Create a sweepFilter function that uses a lifetime to determine sweepability. * @param {LifetimeFilterOptions} [options={}] The options used to generate the filter function - * @returns {Function} + * @returns {SweepFilter} */ static filterByLifetime({ lifetime = 14400, diff --git a/typings/index.d.ts b/typings/index.d.ts index 7b4231ae7b0f..59ae36798602 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -1615,10 +1615,8 @@ export class StoreChannel extends GuildChannel { export class SweptCollection extends Collection { public constructor(options?: SweptCollectionOptions, iterable?: Iterable); - public keepAtMaxSize: ((value: V) => boolean) | null; public interval: number | null; - public maxSize: number; - public sweepFilter: (() => (value: V) => boolean) | null; + public sweepFilter: ((collection: this) => ((value: V) => boolean) | false) | null; public static filterByLifetime(options?: LifetimeFilterOptions): () => (value: any) => boolean; } @@ -4295,9 +4293,7 @@ export interface StageInstanceEditOptions { } export interface SweptCollectionOptions { - keepAtMaxSize?: (value: V) => boolean; - maxSize?: number; - sweepFilter?: () => (value: V) => boolean; + sweepFilter?: (collection: SweptCollection) => ((value: V) => boolean) | false; sweepInterval?: number; } From a4c684cc308a3ba6731f835d5206cfd11d473fd8 Mon Sep 17 00:00:00 2001 From: ckohen Date: Sun, 18 Jul 2021 03:26:33 -0700 Subject: [PATCH 09/23] refactor: pr comments and deprecation Co-Authored-By: 1Computer1 <22125769+1Computer1@users.noreply.github.com> Co-Authored-By: NotSugden <28943913+NotSugden@users.noreply.github.com> --- src/client/Client.js | 4 ++++ src/util/Options.js | 37 ++++++++++++++++++++++++++----------- src/util/SweptCollection.js | 26 +++++++++++++------------- typings/index.d.ts | 18 ++++++++++++------ 4 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index dd00cb3f3f4c..471f8d08a706 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -161,6 +161,10 @@ class Client extends BaseClient { this.readyAt = null; if (this.options.messageSweepInterval > 0) { + process.emitWarning( + 'The message sweeping client options are deprecated, use the makeCache option with a SweptCollection instead.', + 'DeprecationWarning', + ); this.sweepMessageInterval = setInterval( this.sweepMessages.bind(this), this.options.messageSweepInterval * 1000, diff --git a/src/util/Options.js b/src/util/Options.js index 2f189adab3a5..304cec9c510e 100644 --- a/src/util/Options.js +++ b/src/util/Options.js @@ -35,10 +35,11 @@ * (e.g. recommended shard count, shard count of the ShardingManager) * @property {CacheFactory} [makeCache] Function to create a cache. * You can use your own function, or the {@link Options} class to customize the Collection used for the cache. - * @property {number} [messageCacheLifetime=0] How long a message should stay in the cache until it is considered - * sweepable (in seconds, 0 for forever) - * @property {number} [messageSweepInterval=0] How frequently to remove messages from the cache that are older than - * the message cache lifetime (in seconds, 0 for never) + * @property {number} [messageCacheLifetime=0] DEPRECATED: Use `makeCache` with a `SweptCollection` instead. + * How long a message should stay in the cache until it is considered sweepable (in seconds, 0 for forever) + * @property {number} [messageSweepInterval=0] DEPRECATED: Use `makeCache` with a `SweptCollection` instead. + * How frequently to remove messages from the cache that are older than the message cache lifetime + * (in seconds, 0 for never) * @property {MessageMentionOptions} [allowedMentions] Default value for {@link MessageOptions#allowedMentions} * @property {number} [invalidRequestWarningInterval=0] The number of invalid REST requests (those that return * 401, 403, or 429) in a 10 minute window between emitted warnings (0 for no warnings). That is, if set to 500, @@ -102,7 +103,7 @@ class Options extends null { makeCache: this.cacheSome({ MessageManager: 200, ThreadManager: { - sweepInterval: require('./SweptCollection').filterByLifetime({ + sweepFilter: require('./SweptCollection').filterByLifetime({ getComparisonTimestamp: e => e.archiveTimestamp, excludeFromSweep: e => !e.archived, }), @@ -143,19 +144,33 @@ class Options extends null { /** * Create a cache factory using predefined settings to sweep or limit. - * @param {Record} [settings={}] Settings passed to the relevant constructor. + * @param {Object} [settings={}] Settings passed to the relevant constructor. * If no setting is provided for a manager, it uses Collection. + * If SweptCollectionOptions are provided for a manager, it uses those settings to form a SweptCollection + * If a number is provided for a manager, it uses that number as the max size for a LimitedCollection * @returns {CacheFactory} * @example + * // Store up to 200 messages per channel and discard archived threads if they were archived more than 4 hours ago. * Options.cacheSome({ * MessageManager: 200, * ThreadManager: { - * sweepInterval: SweptCollection.filterByLifetime({ + * sweepFilter: SweptCollection.filterByLifetime({ * getComparisonTimestamp: e => e.archiveTimestamp, * excludeFromSweep: e => !e.archived, * }), * }, * }); + * @example + * // Sweep messages every 5 minutes, removing messages that have not been edited or created in the last 30 minutes + * Options.cacheSome({ + * MessageManager: { + * sweepInterval: 300, + * sweepFilter: SweptCollection.filterByLifetime({ + * lifetime: 1800, + * getComparisonTimestamp: e => e.editedTimestamp ?? e.createdTimestamp, + * }) + * } + * }); */ static cacheSome(settings = {}) { const Collection = require('./Collection'); @@ -164,11 +179,11 @@ class Options extends null { return manager => { const setting = settings[manager.name]; - if (typeof setting === 'number' && setting !== Infinity) return LimitedCollection(setting); + if (typeof setting === 'number' && setting !== Infinity) return new LimitedCollection(setting); if ( - setting === null || - typeof setting !== 'object' || - (setting.sweepInterval ? setting.sweepInterval <= 0 || setting.sweepInterval === Infinity : true) + setting?.sweepInterval === (undefined || null) || + setting.sweepInterval <= 0 || + setting.sweepInterval === Infinity ) { return new Collection(); } diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js index 3fffc5e8ea6d..57abd0d34a47 100644 --- a/src/util/SweptCollection.js +++ b/src/util/SweptCollection.js @@ -6,7 +6,7 @@ const { TypeError } = require('../errors/DJSError.js'); /** * @typedef {Function} SweepFilter * @param {SweptCollection} collection The collection being swept - * @returns {Function|false} Return `false` to skip sweeping, otherwise a function passed to `sweep()`, + * @returns {Function|null} Return `null` to skip sweeping, otherwise a function passed to `sweep()`, * See {@link [Collection#sweep](https://discord.js.org/#/docs/collection/master/class/Collection?scrollTo=sweep)} * for the definition of this function. */ @@ -14,7 +14,7 @@ const { TypeError } = require('../errors/DJSError.js'); /** * Options for defining the behavior of a Swept Collection * @typedef {Object} SweptCollectionOptions - * @property {Function|null} [sweepFilter=null] A function run every `sweepInterval` to determine how to sweep + * @property {?SweepFitler} [sweepFilter=null] A function run every `sweepInterval` to determine how to sweep * @property {number} [sweepInterval=3600] How frequently, in seconds, to sweep the collection. */ @@ -40,19 +40,19 @@ class SweptCollection extends Collection { /** * A function called every sweep interval that returns a function passed to `sweep` - * @type {?Function} + * @type {?SweepFilter} */ this.sweepFilter = sweepFilter; /** * The id of the interval being used to sweep. - * @type {?Number} + * @type {?Timeout} */ this.interval = sweepInterval > 0 && sweepFilter ? setInterval(() => { const sweepFn = this.sweepFilter(this); - if (sweepFn === false) return; + if (sweepFn === null) return; if (typeof sweepFn !== 'function') throw new TypeError('SWEEP_FILTER_RETURN'); this.sweep(sweepFn); }, sweepInterval * 1000).unref() @@ -64,10 +64,10 @@ class SweptCollection extends Collection { * @typedef {Object} LifetimeFilterOptions * @property {number} [lifetime=14400] How long an entry should stay in the collection * before it is considered sweepable - * @property {Function} [getComparisonTimestamp=`e => e.createdTimestamp`] A function that takes an entry - * and returns a timestamp to compare against in order to determine the lifetime of the entry. - * @property {Function} [excludeFromSweep=`() => false)`] A function that takes an entry and returns a boolean, - * `true` when the entry should not be checked for sweepability. + * @property {Function} [getComparisonTimestamp=`e => e.createdTimestamp`] A function that takes an entry, key, + * and the collection and returns a timestamp to compare against in order to determine the lifetime of the entry. + * @property {Function} [excludeFromSweep=`() => false`] A function that takes an entry, key, and the collection + * and returns a boolean, `true` when the entry should not be checked for sweepability. */ /** @@ -88,14 +88,14 @@ class SweptCollection extends Collection { throw new TypeError('INVALID_TYPE', 'excludeFromSweep', 'function'); } return () => { - if (lifetime <= 0) return () => false; + if (lifetime <= 0) return null; const lifetimeMs = lifetime * 1000; const now = Date.now(); - return entry => { - if (excludeFromSweep(entry)) { + return (entry, key, coll) => { + if (excludeFromSweep(entry, key, coll)) { return false; } - const comparisonTimestamp = getComparisonTimestamp(entry); + const comparisonTimestamp = getComparisonTimestamp(entry, key, coll); if (!comparisonTimestamp || typeof comparisonTimestamp !== 'number') return false; return now - comparisonTimestamp > lifetimeMs; }; diff --git a/typings/index.d.ts b/typings/index.d.ts index 59ae36798602..f7b1c9fcbccc 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -1616,9 +1616,11 @@ export class StoreChannel extends GuildChannel { export class SweptCollection extends Collection { public constructor(options?: SweptCollectionOptions, iterable?: Iterable); public interval: number | null; - public sweepFilter: ((collection: this) => ((value: V) => boolean) | false) | null; + public sweepFilter: ((collection: this) => ((value: V, key: K, collection: this) => boolean) | null) | null; - public static filterByLifetime(options?: LifetimeFilterOptions): () => (value: any) => boolean; + public static filterByLifetime( + options?: LifetimeFilterOptions, + ): () => (value: V, collection: SweptCollection) => boolean; } export class SystemChannelFlags extends BitField { @@ -3011,7 +3013,9 @@ export interface ClientOptions { shards?: number | number[] | 'auto'; shardCount?: number; makeCache?: CacheFactory; + /** @deprecated Use `makeCache` with a `SweptCollection` for `MessageManager` instead. */ messageCacheLifetime?: number; + /** @deprecated Use `makeCache` with a `SweptCollection` for `MessageManager` instead. */ messageSweepInterval?: number; allowedMentions?: MessageMentionOptions; invalidRequestWarningInterval?: number; @@ -3766,9 +3770,9 @@ export type InviteScope = | 'gdm.join' | 'webhook.incoming'; -export interface LifetimeFilterOptions { - excludeFromSweep?: (value: any) => boolean; - getComparisonTimestamp?: (value: any) => number; +export interface LifetimeFilterOptions { + excludeFromSweep?: (value: V, key: K, collection: SweptCollection) => boolean; + getComparisonTimestamp?: (value: V, key: K, collection: SweptCollection) => number; lifetime?: number; } @@ -4293,7 +4297,9 @@ export interface StageInstanceEditOptions { } export interface SweptCollectionOptions { - sweepFilter?: (collection: SweptCollection) => ((value: V) => boolean) | false; + sweepFilter?: ( + collection: SweptCollection, + ) => ((value: V, key: K, collection: SweptCollection) => boolean) | false; sweepInterval?: number; } From f8a66b51c68e7fb020d71f510e78520b13678eeb Mon Sep 17 00:00:00 2001 From: ckohen Date: Thu, 22 Jul 2021 01:25:22 -0700 Subject: [PATCH 10/23] types: cleaner typings Co-Authored-By: 1Computer1 <22125769+1Computer1@users.noreply.github.com> --- typings/index.d.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/typings/index.d.ts b/typings/index.d.ts index f7b1c9fcbccc..2ceee3965c9e 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -1616,11 +1616,9 @@ export class StoreChannel extends GuildChannel { export class SweptCollection extends Collection { public constructor(options?: SweptCollectionOptions, iterable?: Iterable); public interval: number | null; - public sweepFilter: ((collection: this) => ((value: V, key: K, collection: this) => boolean) | null) | null; + public sweepFilter: SweptCollectionSweepFilter | null; - public static filterByLifetime( - options?: LifetimeFilterOptions, - ): () => (value: V, collection: SweptCollection) => boolean; + public static filterByLifetime(options?: LifetimeFilterOptions): SweptCollectionSweepFilter; } export class SystemChannelFlags extends BitField { @@ -4296,10 +4294,12 @@ export interface StageInstanceEditOptions { privacyLevel?: PrivacyLevel | number; } +export type SweptCollectionSweepFilter = ( + collection: SweptCollection, +) => ((value: V, key: K, collection: SweptCollection) => boolean) | null; + export interface SweptCollectionOptions { - sweepFilter?: ( - collection: SweptCollection, - ) => ((value: V, key: K, collection: SweptCollection) => boolean) | false; + sweepFilter?: SweptCollectionSweepFilter; sweepInterval?: number; } From c07aca5bebcfe751398ce74a75353cd918657e16 Mon Sep 17 00:00:00 2001 From: ckohen Date: Thu, 22 Jul 2021 01:25:52 -0700 Subject: [PATCH 11/23] fix: test for sweepInterval properly Co-Authored-By: 1Computer1 <22125769+1Computer1@users.noreply.github.com> --- src/util/Options.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/util/Options.js b/src/util/Options.js index 304cec9c510e..9406f8e592a2 100644 --- a/src/util/Options.js +++ b/src/util/Options.js @@ -180,11 +180,8 @@ class Options extends null { return manager => { const setting = settings[manager.name]; if (typeof setting === 'number' && setting !== Infinity) return new LimitedCollection(setting); - if ( - setting?.sweepInterval === (undefined || null) || - setting.sweepInterval <= 0 || - setting.sweepInterval === Infinity - ) { + /* eslint-disable-next-line eqeqeq */ + if (setting?.sweepInterval == null || setting.sweepInterval <= 0 || setting.sweepInterval === Infinity) { return new Collection(); } return new SweptCollection(setting); From b5a718ef9987b771442d0134e09c14a6820ef1e0 Mon Sep 17 00:00:00 2001 From: ckohen Date: Tue, 27 Jul 2021 16:55:41 -0700 Subject: [PATCH 12/23] refactor: use slice over spread Co-Authored-By: Shino --- src/managers/GuildEmojiRoleManager.js | 7 +++++-- src/managers/GuildMemberRoleManager.js | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/managers/GuildEmojiRoleManager.js b/src/managers/GuildEmojiRoleManager.js index c05da621b82a..1dcb4bf56ee0 100644 --- a/src/managers/GuildEmojiRoleManager.js +++ b/src/managers/GuildEmojiRoleManager.js @@ -72,7 +72,10 @@ class GuildEmojiRoleManager extends DataManager { resolvedRoleIds.push(roleId); } - const newRoles = [...this.cache.keys()].filter(id => !resolvedRoleIds.includes(id)); + const newRoles = this.cache + .keys() + .slice() + .filter(id => !resolvedRoleIds.includes(id)); return this.set(newRoles); } @@ -97,7 +100,7 @@ class GuildEmojiRoleManager extends DataManager { clone() { const clone = new this.constructor(this.emoji); - clone._patch([...this.cache.keys()].slice()); + clone._patch(this.cache.keys().slice()); return clone; } diff --git a/src/managers/GuildMemberRoleManager.js b/src/managers/GuildMemberRoleManager.js index ed5f5601770e..9df0e6ba5119 100644 --- a/src/managers/GuildMemberRoleManager.js +++ b/src/managers/GuildMemberRoleManager.js @@ -172,7 +172,7 @@ class GuildMemberRoleManager extends DataManager { clone() { const clone = new this.constructor(this.member); - clone.member._roles = [...this.cache.keys()]; + clone.member._roles = this.cache.keys().slice(); return clone; } } From d93f6cdb0afe9eb9f5574460cd0fff356e242f95 Mon Sep 17 00:00:00 2001 From: ckohen Date: Tue, 27 Jul 2021 17:31:41 -0700 Subject: [PATCH 13/23] feat(SweptCollection): clear intervals on client destroy Co-Authored-By: SpaceEEC <24881032+SpaceEEC@users.noreply.github.com> --- src/client/Client.js | 11 +++++++++++ src/managers/CachedManager.js | 2 ++ typings/index.d.ts | 3 ++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/client/Client.js b/src/client/Client.js index 471f8d08a706..81df083a7efe 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -72,6 +72,13 @@ class Client extends BaseClient { this._validateOptions(); + /** + * The intervals used by managers with a {@link SweptCollection} to sweep. + * These are automatically cleared when the client is destroyed. + * @type {Set} + */ + this.sweepIntervals = new Set(); + /** * The WebSocket manager of the client * @type {WebSocketManager} @@ -251,6 +258,10 @@ class Client extends BaseClient { */ destroy() { super.destroy(); + + for (const interval of this.sweepIntervals) clearInterval(interval); + this.sweepIntervals.clear(); + if (this.sweepMessageInterval) clearInterval(this.sweepMessageInterval); this.ws.destroy(); diff --git a/src/managers/CachedManager.js b/src/managers/CachedManager.js index 1a3856853613..b3d316d580e0 100644 --- a/src/managers/CachedManager.js +++ b/src/managers/CachedManager.js @@ -13,6 +13,8 @@ class CachedManager extends DataManager { Object.defineProperty(this, '_cache', { value: this.client.options.makeCache(this.constructor, this.holds) }); + if (this._cache.interval) client.sweepIntervals.add(this._cache.interval); + if (iterable) { for (const item of iterable) { this._add(item); diff --git a/typings/index.d.ts b/typings/index.d.ts index 2ceee3965c9e..d17b253856de 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -284,6 +284,7 @@ export class Client extends BaseClient { public readyAt: If; public readonly readyTimestamp: If; public shard: ShardClientUtil | null; + public sweepIntervals: Set; public token: If; public uptime: If; public user: If; @@ -1615,7 +1616,7 @@ export class StoreChannel extends GuildChannel { export class SweptCollection extends Collection { public constructor(options?: SweptCollectionOptions, iterable?: Iterable); - public interval: number | null; + public interval: NodeJS.Timeout | null; public sweepFilter: SweptCollectionSweepFilter | null; public static filterByLifetime(options?: LifetimeFilterOptions): SweptCollectionSweepFilter; From 3341d0499efbff8f7a8d4261e8fc85c141e33d04 Mon Sep 17 00:00:00 2001 From: ckohen Date: Tue, 27 Jul 2021 19:44:11 -0700 Subject: [PATCH 14/23] feat(Managers): add garbage collection to managers BREAKING: updates node version requirement and bumps ecma version to 2021 Co-Authored-By: SpaceEEC <24881032+SpaceEEC@users.noreply.github.com> --- .eslintrc.json | 4 ++-- package-lock.json | 2 +- package.json | 2 +- src/client/Client.js | 22 +++++++++++++++++++ .../ApplicationCommandPermissionsManager.js | 2 +- src/managers/CachedManager.js | 1 + src/util/Options.js | 8 +++++-- 7 files changed, 34 insertions(+), 7 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 201525f6f255..3d85319b927d 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -2,10 +2,10 @@ "extends": ["eslint:recommended", "plugin:prettier/recommended"], "plugins": ["import"], "parserOptions": { - "ecmaVersion": 2020 + "ecmaVersion": 2021 }, "env": { - "es2020": true, + "es2021": true, "node": true }, "rules": { diff --git a/package-lock.json b/package-lock.json index ccb1f7fd34b8..e874abf8623c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -40,7 +40,7 @@ "typescript": "^4.3.4" }, "engines": { - "node": ">=14.0.0", + "node": ">=14.6.0", "npm": ">=7.0.0" } }, diff --git a/package.json b/package.json index 49dead745c24..7a682eca0389 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,7 @@ "typescript": "^4.3.4" }, "engines": { - "node": ">=14.0.0", + "node": ">=14.6.0", "npm": ">=7.0.0" } } diff --git a/src/client/Client.js b/src/client/Client.js index 81df083a7efe..b5bda4de9606 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -79,6 +79,13 @@ class Client extends BaseClient { */ this.sweepIntervals = new Set(); + /** + * The finalizers used within managers to clear. + * @type {FinalizationRegistry} + * @private + */ + this._managerFinalizers = new FinalizationRegistry(this._finalizeManager.bind(this)); + /** * The WebSocket manager of the client * @type {WebSocketManager} @@ -361,6 +368,21 @@ class Client extends BaseClient { const data = await this.api('sticker-packs').get(); return new Collection(data.sticker_packs.map(p => [p.id, new StickerPack(this, p)])); } + /** + * A last ditch cleanup function for garbage collection on managers. + * @param {Collection} options.cache The cache of the manager being GCed + * @param {string} options.type The name of the manager being GCed + */ + _finalizeManager({ cache, type }) { + if (cache.interval) { + clearInterval(cache.interval); + this.sweepIntervals.delete(cache.interval); + this.emit( + Events.DEBUG, + `${type} has no more references and contained a SweptCollection. The interval on the cache has been destroyed`, + ); + } + } /** * Sweeps all text-based channels' messages and removes the ones older than the max message lifetime. diff --git a/src/managers/ApplicationCommandPermissionsManager.js b/src/managers/ApplicationCommandPermissionsManager.js index 71648f250306..d5bd19c2629b 100644 --- a/src/managers/ApplicationCommandPermissionsManager.js +++ b/src/managers/ApplicationCommandPermissionsManager.js @@ -17,7 +17,7 @@ class ApplicationCommandPermissionsManager extends BaseManager { * The manager or command that this manager belongs to * @type {ApplicationCommandManager|ApplicationCommand} */ - this.manager = manager; + this.manager = new WeakRef(manager); /** * The guild that this manager acts on diff --git a/src/managers/CachedManager.js b/src/managers/CachedManager.js index b3d316d580e0..caafe9323fc5 100644 --- a/src/managers/CachedManager.js +++ b/src/managers/CachedManager.js @@ -14,6 +14,7 @@ class CachedManager extends DataManager { Object.defineProperty(this, '_cache', { value: this.client.options.makeCache(this.constructor, this.holds) }); if (this._cache.interval) client.sweepIntervals.add(this._cache.interval); + client._managerFinalizers.register(this, { cache: this._cache, type: this.constructor.name }); if (iterable) { for (const item of iterable) { diff --git a/src/util/Options.js b/src/util/Options.js index 9406f8e592a2..a3a75025ad5b 100644 --- a/src/util/Options.js +++ b/src/util/Options.js @@ -180,8 +180,12 @@ class Options extends null { return manager => { const setting = settings[manager.name]; if (typeof setting === 'number' && setting !== Infinity) return new LimitedCollection(setting); - /* eslint-disable-next-line eqeqeq */ - if (setting?.sweepInterval == null || setting.sweepInterval <= 0 || setting.sweepInterval === Infinity) { + if ( + /* eslint-disable-next-line eqeqeq */ + (setting?.sweepInterval == null && setting?.sweepFilter == null) || + setting.sweepInterval <= 0 || + setting.sweepInterval === Infinity + ) { return new Collection(); } return new SweptCollection(setting); From 1cd1957fd9346acfa1066b500b918e36741eaec7 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 28 Jul 2021 15:21:48 -0700 Subject: [PATCH 15/23] refactor(Collection): better import Co-Authored-By: Noel --- src/util/Collection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/Collection.js b/src/util/Collection.js index 7fbdaf9fc01a..79428a9ad368 100644 --- a/src/util/Collection.js +++ b/src/util/Collection.js @@ -1,6 +1,6 @@ 'use strict'; -const BaseCollection = require('@discordjs/collection').Collection; +const { Collection: BaseCollection } = require('@discordjs/collection'); const Util = require('./Util'); class Collection extends BaseCollection { From 20d38de0d89a623b2dd243b35c0c4eb588587fb2 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 28 Jul 2021 17:07:52 -0700 Subject: [PATCH 16/23] refactor: use symbols and functions instead of hardcoding Co-Authored-By: 1Computer1 <22125769+1Computer1@users.noreply.github.com> --- src/client/Client.js | 25 +++++++++++-------- .../ApplicationCommandPermissionsManager.js | 3 ++- src/managers/CachedManager.js | 8 ++++-- src/util/SweptCollection.js | 4 +++ typings/index.d.ts | 4 +-- 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index b5bda4de9606..2e2eb82a9faa 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -73,11 +73,11 @@ class Client extends BaseClient { this._validateOptions(); /** - * The intervals used by managers with a {@link SweptCollection} to sweep. - * These are automatically cleared when the client is destroyed. - * @type {Set} + * A map of ids to function called when a cache is garbage collected or the Client is destroyed + * @type {Map} + * @private */ - this.sweepIntervals = new Set(); + this._cacheCleanups = new Map(); /** * The finalizers used within managers to clear. @@ -266,8 +266,8 @@ class Client extends BaseClient { destroy() { super.destroy(); - for (const interval of this.sweepIntervals) clearInterval(interval); - this.sweepIntervals.clear(); + for (const fn of this._cacheCleanups.values()) fn(); + this._cacheCleanups.clear(); if (this.sweepMessageInterval) clearInterval(this.sweepMessageInterval); @@ -370,16 +370,19 @@ class Client extends BaseClient { } /** * A last ditch cleanup function for garbage collection on managers. + * @param {Snowflake} options.cleanupId The id used to locate the cleanup in the cacheCleanups map * @param {Collection} options.cache The cache of the manager being GCed * @param {string} options.type The name of the manager being GCed */ - _finalizeManager({ cache, type }) { - if (cache.interval) { - clearInterval(cache.interval); - this.sweepIntervals.delete(cache.interval); + _finalizeManager({ cleanupId, cache, type }) { + const cleanupFn = cache[Symbol.for('djsCacheCleanup')]; + if (cleanupFn) { + cleanupFn.bind(cache)(); + this._cacheCleanups.delete(cleanupId); this.emit( Events.DEBUG, - `${type} has no more references and contained a SweptCollection. The interval on the cache has been destroyed`, + `${type} has no more references and contained a cache with a cleanup function. ` + + `Cleanup called on ${cache.constructor.name}.`, ); } } diff --git a/src/managers/ApplicationCommandPermissionsManager.js b/src/managers/ApplicationCommandPermissionsManager.js index d5bd19c2629b..e165421d551b 100644 --- a/src/managers/ApplicationCommandPermissionsManager.js +++ b/src/managers/ApplicationCommandPermissionsManager.js @@ -15,7 +15,8 @@ class ApplicationCommandPermissionsManager extends BaseManager { /** * The manager or command that this manager belongs to - * @type {ApplicationCommandManager|ApplicationCommand} + * @type {WeakRef} + * @private */ this.manager = new WeakRef(manager); diff --git a/src/managers/CachedManager.js b/src/managers/CachedManager.js index caafe9323fc5..1819998e665a 100644 --- a/src/managers/CachedManager.js +++ b/src/managers/CachedManager.js @@ -1,6 +1,7 @@ 'use strict'; const DataManager = require('./DataManager'); +const SnowflakeUtil = require('../util/SnowflakeUtil'); /** * Manages the API methods of a data model with a mutable cache of instances. @@ -13,8 +14,11 @@ class CachedManager extends DataManager { Object.defineProperty(this, '_cache', { value: this.client.options.makeCache(this.constructor, this.holds) }); - if (this._cache.interval) client.sweepIntervals.add(this._cache.interval); - client._managerFinalizers.register(this, { cache: this._cache, type: this.constructor.name }); + const cleanupId = SnowflakeUtil.generate(); + if (this._cache[Symbol.for('djsCacheCleanup')]) { + client._cacheCleanups.set(cleanupId, this._cache[Symbol.for('djsCacheCleanup')]); + } + client._managerFinalizers.register(this, { cleanupId, cache: this._cache, type: this.constructor.name }); if (iterable) { for (const item of iterable) { diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js index 57abd0d34a47..3c84f74e1495 100644 --- a/src/util/SweptCollection.js +++ b/src/util/SweptCollection.js @@ -102,6 +102,10 @@ class SweptCollection extends Collection { }; } + [Symbol.for('djsCacheCleanup')]() { + clearInterval(this.interval); + } + static get [Symbol.species]() { return Collection; } diff --git a/typings/index.d.ts b/typings/index.d.ts index d17b253856de..73d74b572ea9 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -284,7 +284,6 @@ export class Client extends BaseClient { public readyAt: If; public readonly readyTimestamp: If; public shard: ShardClientUtil | null; - public sweepIntervals: Set; public token: If; public uptime: If; public user: If; @@ -2212,11 +2211,12 @@ export class ApplicationCommandPermissionsManager< CommandIdType, > extends BaseManager { public constructor(manager: ApplicationCommandManager | GuildApplicationCommandManager | ApplicationCommand); + private manager: WeakRef; + public client: Client; public commandId: CommandIdType; public guild: GuildType; public guildId: Snowflake | null; - public manager: ApplicationCommandManager | GuildApplicationCommandManager | ApplicationCommand; public add( options: FetchSingleOptions & { permissions: ApplicationCommandPermissionData[] }, ): Promise; From ccc79c6fd45f5396cd792ebffbc2f1d08e9ff6e1 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 28 Jul 2021 18:08:17 -0700 Subject: [PATCH 17/23] refactor: better interface and non-global symbol Co-Authored-By: 1Computer1 <22125769+1Computer1@users.noreply.github.com> --- src/client/Client.js | 33 ++++++++++++++++----------------- src/managers/CachedManager.js | 16 +++++++++++----- src/util/Constants.js | 2 ++ src/util/SweptCollection.js | 3 ++- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index 2e2eb82a9faa..015c6a6654be 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -73,11 +73,11 @@ class Client extends BaseClient { this._validateOptions(); /** - * A map of ids to function called when a cache is garbage collected or the Client is destroyed - * @type {Map} + * Functions called when a cache is garbage collected or the Client is destroyed + * @type {Set} * @private */ - this._cacheCleanups = new Map(); + this._cacheCleanups = new Set(); /** * The finalizers used within managers to clear. @@ -266,7 +266,7 @@ class Client extends BaseClient { destroy() { super.destroy(); - for (const fn of this._cacheCleanups.values()) fn(); + for (const fn of this._cacheCleanups) fn(); this._cacheCleanups.clear(); if (this.sweepMessageInterval) clearInterval(this.sweepMessageInterval); @@ -370,20 +370,19 @@ class Client extends BaseClient { } /** * A last ditch cleanup function for garbage collection on managers. - * @param {Snowflake} options.cleanupId The id used to locate the cleanup in the cacheCleanups map - * @param {Collection} options.cache The cache of the manager being GCed - * @param {string} options.type The name of the manager being GCed + * @param {Function} options.cleanup The function called to GC + * @param {string} [options.message] The message to send after a successful GC + * @param {string} [options.managerName] The name of the manager being GCed */ - _finalizeManager({ cleanupId, cache, type }) { - const cleanupFn = cache[Symbol.for('djsCacheCleanup')]; - if (cleanupFn) { - cleanupFn.bind(cache)(); - this._cacheCleanups.delete(cleanupId); - this.emit( - Events.DEBUG, - `${type} has no more references and contained a cache with a cleanup function. ` + - `Cleanup called on ${cache.constructor.name}.`, - ); + _finalizeManager({ cleanup, message, managerName }) { + try { + cleanup(); + this._cacheCleanups.delete(cleanup); + if (message) { + this.emit(Events.DEBUG, message); + } + } catch { + this.emit(Events.DEBUG, `Garbage collection failed on ${managerName ?? 'an unkonwn manager'}.`); } } diff --git a/src/managers/CachedManager.js b/src/managers/CachedManager.js index 1819998e665a..83470578190e 100644 --- a/src/managers/CachedManager.js +++ b/src/managers/CachedManager.js @@ -1,7 +1,7 @@ 'use strict'; const DataManager = require('./DataManager'); -const SnowflakeUtil = require('../util/SnowflakeUtil'); +const { _cacheCleanupSymbol } = require('../util/Constants'); /** * Manages the API methods of a data model with a mutable cache of instances. @@ -14,11 +14,17 @@ class CachedManager extends DataManager { Object.defineProperty(this, '_cache', { value: this.client.options.makeCache(this.constructor, this.holds) }); - const cleanupId = SnowflakeUtil.generate(); - if (this._cache[Symbol.for('djsCacheCleanup')]) { - client._cacheCleanups.set(cleanupId, this._cache[Symbol.for('djsCacheCleanup')]); + let cleanup = this._cache[_cacheCleanupSymbol]; + const cacheType = this._cache.constructor.name; + if (cleanup) { + cleanup = cleanup.bind(this._cache); + client._cacheCleanups.add(cleanup); + client._managerFinalizers.register(this, { + cleanup, + message: `Garbage Collection completed on ${this.constructor.name}, which held a ${cacheType}.`, + managerName: this.constructor.name, + }); } - client._managerFinalizers.register(this, { cleanupId, cache: this._cache, type: this.constructor.name }); if (iterable) { for (const item of iterable) { diff --git a/src/util/Constants.js b/src/util/Constants.js index c4ef7039345f..faa5ddfc83e0 100644 --- a/src/util/Constants.js +++ b/src/util/Constants.js @@ -971,6 +971,8 @@ exports.PrivacyLevels = createEnum([null, 'PUBLIC', 'GUILD_ONLY']); */ exports.PremiumTiers = createEnum(['NONE', 'TIER_1', 'TIER_2', 'TIER_3']); +exports._cacheCleanupSymbol = Symbol('djsCacheCleanup'); + function keyMirror(arr) { let tmp = Object.create(null); for (const value of arr) tmp[value] = value; diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js index 3c84f74e1495..138a181591cb 100644 --- a/src/util/SweptCollection.js +++ b/src/util/SweptCollection.js @@ -1,6 +1,7 @@ 'use strict'; const Collection = require('./Collection.js'); +const { _cacheCleanupSymbol } = require('./Constants.js'); const { TypeError } = require('../errors/DJSError.js'); /** @@ -102,7 +103,7 @@ class SweptCollection extends Collection { }; } - [Symbol.for('djsCacheCleanup')]() { + [_cacheCleanupSymbol]() { clearInterval(this.interval); } From 6061ce6b254363712522599b6633a0fc417d0fcd Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 28 Jul 2021 18:46:13 -0700 Subject: [PATCH 18/23] refactor: rename to be lib wide Co-Authored-By: 1Computer1 <22125769+1Computer1@users.noreply.github.com> --- src/client/Client.js | 20 ++++++++++---------- src/managers/CachedManager.js | 10 +++++----- src/util/Constants.js | 2 +- src/util/SweptCollection.js | 4 ++-- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index 015c6a6654be..89a62c792c01 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -77,14 +77,14 @@ class Client extends BaseClient { * @type {Set} * @private */ - this._cacheCleanups = new Set(); + this._cleanups = new Set(); /** - * The finalizers used within managers to clear. + * The finalizers used to cleanup items. * @type {FinalizationRegistry} * @private */ - this._managerFinalizers = new FinalizationRegistry(this._finalizeManager.bind(this)); + this._finalizers = new FinalizationRegistry(this._finalize.bind(this)); /** * The WebSocket manager of the client @@ -266,8 +266,8 @@ class Client extends BaseClient { destroy() { super.destroy(); - for (const fn of this._cacheCleanups) fn(); - this._cacheCleanups.clear(); + for (const fn of this._cleanups) fn(); + this._cleanups.clear(); if (this.sweepMessageInterval) clearInterval(this.sweepMessageInterval); @@ -369,20 +369,20 @@ class Client extends BaseClient { return new Collection(data.sticker_packs.map(p => [p.id, new StickerPack(this, p)])); } /** - * A last ditch cleanup function for garbage collection on managers. + * A last ditch cleanup function for garbage collection. * @param {Function} options.cleanup The function called to GC * @param {string} [options.message] The message to send after a successful GC - * @param {string} [options.managerName] The name of the manager being GCed + * @param {string} [options.name] The name of the item being GCed */ - _finalizeManager({ cleanup, message, managerName }) { + _finalize({ cleanup, message, name }) { try { cleanup(); - this._cacheCleanups.delete(cleanup); + this._cleanups.delete(cleanup); if (message) { this.emit(Events.DEBUG, message); } } catch { - this.emit(Events.DEBUG, `Garbage collection failed on ${managerName ?? 'an unkonwn manager'}.`); + this.emit(Events.DEBUG, `Garbage collection failed on ${name ?? 'an unkonwn item'}.`); } } diff --git a/src/managers/CachedManager.js b/src/managers/CachedManager.js index 83470578190e..42939fafaaa6 100644 --- a/src/managers/CachedManager.js +++ b/src/managers/CachedManager.js @@ -1,7 +1,7 @@ 'use strict'; const DataManager = require('./DataManager'); -const { _cacheCleanupSymbol } = require('../util/Constants'); +const { _cleanupSymbol } = require('../util/Constants'); /** * Manages the API methods of a data model with a mutable cache of instances. @@ -14,15 +14,15 @@ class CachedManager extends DataManager { Object.defineProperty(this, '_cache', { value: this.client.options.makeCache(this.constructor, this.holds) }); - let cleanup = this._cache[_cacheCleanupSymbol]; + let cleanup = this._cache[_cleanupSymbol]; const cacheType = this._cache.constructor.name; if (cleanup) { cleanup = cleanup.bind(this._cache); - client._cacheCleanups.add(cleanup); - client._managerFinalizers.register(this, { + client._cleanups.add(cleanup); + client._finalizers.register(this, { cleanup, message: `Garbage Collection completed on ${this.constructor.name}, which held a ${cacheType}.`, - managerName: this.constructor.name, + name: this.constructor.name, }); } diff --git a/src/util/Constants.js b/src/util/Constants.js index faa5ddfc83e0..c245cc97da1d 100644 --- a/src/util/Constants.js +++ b/src/util/Constants.js @@ -971,7 +971,7 @@ exports.PrivacyLevels = createEnum([null, 'PUBLIC', 'GUILD_ONLY']); */ exports.PremiumTiers = createEnum(['NONE', 'TIER_1', 'TIER_2', 'TIER_3']); -exports._cacheCleanupSymbol = Symbol('djsCacheCleanup'); +exports._cleanupSymbol = Symbol('djsCleanup'); function keyMirror(arr) { let tmp = Object.create(null); diff --git a/src/util/SweptCollection.js b/src/util/SweptCollection.js index 138a181591cb..793a17df7e23 100644 --- a/src/util/SweptCollection.js +++ b/src/util/SweptCollection.js @@ -1,7 +1,7 @@ 'use strict'; const Collection = require('./Collection.js'); -const { _cacheCleanupSymbol } = require('./Constants.js'); +const { _cleanupSymbol } = require('./Constants.js'); const { TypeError } = require('../errors/DJSError.js'); /** @@ -103,7 +103,7 @@ class SweptCollection extends Collection { }; } - [_cacheCleanupSymbol]() { + [_cleanupSymbol]() { clearInterval(this.interval); } From 1f9c5d6b5c08550f352e237fe08bf932c8c72642 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 28 Jul 2021 18:47:03 -0700 Subject: [PATCH 19/23] refactor: remove weakref --- src/managers/ApplicationCommandPermissionsManager.js | 4 ++-- typings/index.d.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/managers/ApplicationCommandPermissionsManager.js b/src/managers/ApplicationCommandPermissionsManager.js index e165421d551b..1f3c6c83c548 100644 --- a/src/managers/ApplicationCommandPermissionsManager.js +++ b/src/managers/ApplicationCommandPermissionsManager.js @@ -15,10 +15,10 @@ class ApplicationCommandPermissionsManager extends BaseManager { /** * The manager or command that this manager belongs to - * @type {WeakRef} + * @type {ApplicationCommandManager|ApplicationCommand>} * @private */ - this.manager = new WeakRef(manager); + this.manager = manager; /** * The guild that this manager acts on diff --git a/typings/index.d.ts b/typings/index.d.ts index 73d74b572ea9..f7d985f53a30 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -2211,7 +2211,7 @@ export class ApplicationCommandPermissionsManager< CommandIdType, > extends BaseManager { public constructor(manager: ApplicationCommandManager | GuildApplicationCommandManager | ApplicationCommand); - private manager: WeakRef; + private manager: ApplicationCommandManager | GuildApplicationCommandManager | ApplicationCommand; public client: Client; public commandId: CommandIdType; From 76770f3c123f56ff66555deb08a8789ae6b91412 Mon Sep 17 00:00:00 2001 From: ckohen Date: Wed, 28 Jul 2021 18:49:56 -0700 Subject: [PATCH 20/23] fix: typo Co-Authored-By: 1Computer1 <22125769+1Computer1@users.noreply.github.com> --- src/client/Client.js | 2 +- src/managers/ApplicationCommandPermissionsManager.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index 89a62c792c01..3cd87612341a 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -382,7 +382,7 @@ class Client extends BaseClient { this.emit(Events.DEBUG, message); } } catch { - this.emit(Events.DEBUG, `Garbage collection failed on ${name ?? 'an unkonwn item'}.`); + this.emit(Events.DEBUG, `Garbage collection failed on ${name ?? 'an unknown item'}.`); } } diff --git a/src/managers/ApplicationCommandPermissionsManager.js b/src/managers/ApplicationCommandPermissionsManager.js index 1f3c6c83c548..bcf09c4e0e44 100644 --- a/src/managers/ApplicationCommandPermissionsManager.js +++ b/src/managers/ApplicationCommandPermissionsManager.js @@ -15,7 +15,7 @@ class ApplicationCommandPermissionsManager extends BaseManager { /** * The manager or command that this manager belongs to - * @type {ApplicationCommandManager|ApplicationCommand>} + * @type {ApplicationCommandManager|ApplicationCommand} * @private */ this.manager = manager; From 123a508cf93c316728299ad9a8a258ef847af59b Mon Sep 17 00:00:00 2001 From: ckohen Date: Fri, 30 Jul 2021 02:47:13 -0700 Subject: [PATCH 21/23] revert: revert spread to slice (iterator not array) --- src/managers/GuildEmojiRoleManager.js | 7 ++----- src/managers/GuildMemberRoleManager.js | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/managers/GuildEmojiRoleManager.js b/src/managers/GuildEmojiRoleManager.js index 1dcb4bf56ee0..73ec4bc9a2b7 100644 --- a/src/managers/GuildEmojiRoleManager.js +++ b/src/managers/GuildEmojiRoleManager.js @@ -72,10 +72,7 @@ class GuildEmojiRoleManager extends DataManager { resolvedRoleIds.push(roleId); } - const newRoles = this.cache - .keys() - .slice() - .filter(id => !resolvedRoleIds.includes(id)); + const newRoles = [...this.cache.keys()].filter(id => !resolvedRoleIds.includes(id)); return this.set(newRoles); } @@ -100,7 +97,7 @@ class GuildEmojiRoleManager extends DataManager { clone() { const clone = new this.constructor(this.emoji); - clone._patch(this.cache.keys().slice()); + clone._patch([...this.cache.keys()]); return clone; } diff --git a/src/managers/GuildMemberRoleManager.js b/src/managers/GuildMemberRoleManager.js index 9df0e6ba5119..ed5f5601770e 100644 --- a/src/managers/GuildMemberRoleManager.js +++ b/src/managers/GuildMemberRoleManager.js @@ -172,7 +172,7 @@ class GuildMemberRoleManager extends DataManager { clone() { const clone = new this.constructor(this.member); - clone.member._roles = this.cache.keys().slice(); + clone.member._roles = [...this.cache.keys()]; return clone; } } From f0463d9fc68c21bc2108437774fb429b9c35b726 Mon Sep 17 00:00:00 2001 From: ckohen Date: Fri, 30 Jul 2021 14:49:40 -0700 Subject: [PATCH 22/23] fix: better success message after gc Co-Authored-By: Vlad Frangu --- src/managers/CachedManager.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/managers/CachedManager.js b/src/managers/CachedManager.js index 42939fafaaa6..eb16a0cc20b5 100644 --- a/src/managers/CachedManager.js +++ b/src/managers/CachedManager.js @@ -15,13 +15,14 @@ class CachedManager extends DataManager { Object.defineProperty(this, '_cache', { value: this.client.options.makeCache(this.constructor, this.holds) }); let cleanup = this._cache[_cleanupSymbol]; - const cacheType = this._cache.constructor.name; if (cleanup) { cleanup = cleanup.bind(this._cache); client._cleanups.add(cleanup); client._finalizers.register(this, { cleanup, - message: `Garbage Collection completed on ${this.constructor.name}, which held a ${cacheType}.`, + message: + `Garbage Collection completed on ${this.constructor.name}, ` + + `which had a ${this._cache.constructor.name} of ${this.holds.name}.`, name: this.constructor.name, }); } From dfb0f3fafa630ca11a62f9e4d9e87cd467a8df4a Mon Sep 17 00:00:00 2001 From: ckohen Date: Fri, 30 Jul 2021 14:51:26 -0700 Subject: [PATCH 23/23] revert: cacheWithLimits to cacheSome --- src/util/Options.js | 8 ++++---- typings/index.d.ts | 4 ++-- typings/index.ts | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/Options.js b/src/util/Options.js index a3a75025ad5b..c86ef6696954 100644 --- a/src/util/Options.js +++ b/src/util/Options.js @@ -100,7 +100,7 @@ class Options extends null { static createDefault() { return { shardCount: 1, - makeCache: this.cacheSome({ + makeCache: this.cacheWithLimits({ MessageManager: 200, ThreadManager: { sweepFilter: require('./SweptCollection').filterByLifetime({ @@ -151,7 +151,7 @@ class Options extends null { * @returns {CacheFactory} * @example * // Store up to 200 messages per channel and discard archived threads if they were archived more than 4 hours ago. - * Options.cacheSome({ + * Options.cacheWithLimits({ * MessageManager: 200, * ThreadManager: { * sweepFilter: SweptCollection.filterByLifetime({ @@ -162,7 +162,7 @@ class Options extends null { * }); * @example * // Sweep messages every 5 minutes, removing messages that have not been edited or created in the last 30 minutes - * Options.cacheSome({ + * Options.cacheWithLimits({ * MessageManager: { * sweepInterval: 300, * sweepFilter: SweptCollection.filterByLifetime({ @@ -172,7 +172,7 @@ class Options extends null { * } * }); */ - static cacheSome(settings = {}) { + static cacheWithLimits(settings = {}) { const Collection = require('./Collection'); const LimitedCollection = require('./LimitedCollection'); const SweptCollection = require('./SweptCollection'); diff --git a/typings/index.d.ts b/typings/index.d.ts index f7d985f53a30..1cf584e706c0 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -358,7 +358,7 @@ export class ClientUser extends User { export class Options extends null { private constructor(); public static createDefaultOptions(): ClientOptions; - public static cacheSome(settings?: CacheSomeOptions): CacheFactory; + public static cacheWithLimits(settings?: CacheWithLimitsOptions): CacheFactory; public static cacheEverything(): CacheFactory; } @@ -2879,7 +2879,7 @@ export interface CacheFactoryArgs { VoiceStateManager: [manager: typeof VoiceStateManager, holds: typeof VoiceState]; } -export type CacheSomeOptions = { +export type CacheWithLimitsOptions = { [K in CachedManagerTypes]?: SweptCollectionOptions | number; }; diff --git a/typings/index.ts b/typings/index.ts index 8cb5fb08d4df..b1b5812d3fc6 100644 --- a/typings/index.ts +++ b/typings/index.ts @@ -54,7 +54,7 @@ import { const client: Client = new Client({ intents: Intents.FLAGS.GUILDS, - makeCache: Options.cacheSome({ + makeCache: Options.cacheWithLimits({ MessageManager: 200, // @ts-expect-error Message: 100,