refactor: merge collections with keeping entries at max (#6242)

src/client/Client.js

@@ -176,7 +176,7 @@ class Client extends BaseClient {
 
     if (this.options.messageSweepInterval > 0) {
       process.emitWarning(
-        'The message sweeping client options are deprecated, use the makeCache option with a SweptCollection instead.',
+        'The message sweeping client options are deprecated, use the makeCache option with LimitedCollection instead.',
         'DeprecationWarning',
       );
       this.sweepMessageInterval = setInterval(

src/index.js

@@ -27,7 +27,6 @@ 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'),

src/managers/CachedManager.js

@@ -14,14 +14,14 @@ class CachedManager extends DataManager {
 
     Object.defineProperty(this, '_cache', { value: this.client.options.makeCache(this.constructor, this.holds) });
 
-    let cleanup = this._cache[_cleanupSymbol];
+    let cleanup = this._cache[_cleanupSymbol]?.();
     if (cleanup) {
       cleanup = cleanup.bind(this._cache);
       client._cleanups.add(cleanup);
       client._finalizers.register(this, {
         cleanup,
         message:
-          `Garbage Collection completed on ${this.constructor.name}, ` +
+          `Garbage collection completed on ${this.constructor.name}, ` +
           `which had a ${this._cache.constructor.name} of ${this.holds.name}.`,
         name: this.constructor.name,
       });

src/util/LimitedCollection.js

@@ -1,30 +1,151 @@
 'use strict';
 
 const { Collection } = require('@discordjs/collection');
+const { _cleanupSymbol } = require('./Constants.js');
+const { TypeError } = require('../errors/DJSError.js');
 
 /**
- * A Collection which holds a max amount of entries. The first key is deleted if the Collection has
+ * @typedef {Function} SweepFilter
- * reached max size.
+ * @param {LimitedCollection} collection The collection being swept
+ * @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.
+ */
+
+/**
+ * Options for defining the behavior of a LimitedCollection
+ * @typedef {Object} LimitedCollectionOptions
+ * @property {?number} [maxSize=0] The maximum size of the Collection
+ * @property {?Function} [keepOverLimit=null] A function, which is passed the value and key of an entry, ran to decide
+ * to keep an entry past the maximum size
+ * @property {?SweepFilter} [sweepFilter=null] A function ran every `sweepInterval` to determine how to sweep
+ * @property {?number} [sweepInterval=0] How frequently, in seconds, to sweep the collection.
+ */
+
+/**
+ * A Collection which holds a max amount of entries and sweeps periodically.
  * @extends {Collection}
- * @param {number} [maxSize=0] The maximum size of the Collection
+ * @param {LimitedCollectionOptions} [options={}] Options for constructing the Collection.
  * @param {Iterable} [iterable=null] Optional entries passed to the Map constructor.
  */
 class LimitedCollection extends Collection {
-  constructor(maxSize = 0, iterable = null) {
+  constructor(options = {}, iterable) {
+    if (typeof options !== 'object' || options === null) {
+      throw new TypeError('INVALID_TYPE', 'options', 'object', true);
+    }
+    const { maxSize = Infinity, keepOverLimit = null, sweepInterval = 0, sweepFilter = null } = options;
+
+    if (typeof maxSize !== 'number') {
+      throw new TypeError('INVALID_TYPE', 'maxSize', 'number');
+    }
+    if (keepOverLimit !== null && typeof keepOverLimit !== 'function') {
+      throw new TypeError('INVALID_TYPE', 'keepOverLimit', 'function');
+    }
+    if (typeof sweepInterval !== 'number') {
+      throw new TypeError('INVALID_TYPE', 'sweepInterval', 'number');
+    }
+    if (sweepFilter !== null && typeof sweepFilter !== 'function') {
+      throw new TypeError('INVALID_TYPE', 'sweepFilter', 'function');
+    }
+
     super(iterable);
+
     /**
      * The max size of the Collection.
      * @type {number}
      */
     this.maxSize = maxSize;
+
+    /**
+     * A function called to check if an entry should be kept when the Collection is at max size.
+     * @type {?Function}
+     */
+    this.keepOverLimit = keepOverLimit;
+
+    /**
+     * A function called every sweep interval that returns a function passed to `sweep`.
+     * @type {?SweepFilter}
+     */
+    this.sweepFilter = sweepFilter;
+
+    /**
+     * The id of the interval being used to sweep.
+     * @type {?Timeout}
+     */
+    this.interval =
+      sweepInterval > 0 && sweepInterval !== Infinity && sweepFilter
+        ? setInterval(() => {
+            const sweepFn = this.sweepFilter(this);
+            if (sweepFn === null) 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.size >= this.maxSize && !this.has(key)) this.delete(this.firstKey());
+    if (this.size >= this.maxSize && !this.has(key)) {
+      for (const [k, v] of this.entries()) {
+        const keep = this.keepOverLimit?.(v, k, this) ?? false;
+        if (!keep) {
+          this.delete(k);
+          break;
+        }
+      }
+    }
     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, 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.
+   */
+
+  /**
+   * Create a sweepFilter function that uses a lifetime to determine sweepability.
+   * @param {LifetimeFilterOptions} [options={}] The options used to generate the filter function
+   * @returns {SweepFilter}
+   */
+  static filterByLifetime({
+    lifetime = 14400,
+    getComparisonTimestamp = e => e?.createdTimestamp,
+    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 (typeof excludeFromSweep !== 'function') {
+      throw new TypeError('INVALID_TYPE', 'excludeFromSweep', 'function');
+    }
+    return () => {
+      if (lifetime <= 0) return null;
+      const lifetimeMs = lifetime * 1000;
+      const now = Date.now();
+      return (entry, key, coll) => {
+        if (excludeFromSweep(entry, key, coll)) {
+          return false;
+        }
+        const comparisonTimestamp = getComparisonTimestamp(entry, key, coll);
+        if (!comparisonTimestamp || typeof comparisonTimestamp !== 'number') return false;
+        return now - comparisonTimestamp > lifetimeMs;
+      };
+    };
+  }
+
+  [_cleanupSymbol]() {
+    return this.interval ? () => clearInterval(this.interval) : null;
+  }
+
   static get [Symbol.species]() {
     return Collection;
   }

src/util/Options.js

@@ -35,9 +35,9 @@
  * (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] DEPRECATED: Use `makeCache` with a `SweptCollection` instead.
+ * @property {number} [messageCacheLifetime=0] DEPRECATED: Use `makeCache` with a `LimitedCollection` 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.
+ * @property {number} [messageSweepInterval=0] DEPRECATED: Use `makeCache` with a `LimitedCollection` 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}
@@ -103,7 +103,8 @@ class Options extends null {
       makeCache: this.cacheWithLimits({
         MessageManager: 200,
         ThreadManager: {
-          sweepFilter: require('./SweptCollection').filterByLifetime({
+          sweepInterval: 3600,
+          sweepFilter: require('./LimitedCollection').filterByLifetime({
             getComparisonTimestamp: e => e.archiveTimestamp,
             excludeFromSweep: e => !e.archived,
           }),
@@ -144,17 +145,18 @@ class Options extends null {
 
   /**
    * Create a cache factory using predefined settings to sweep or limit.
-   * @param {Object<string, SweptCollectionOptions|number>} [settings={}] Settings passed to the relevant constructor.
+   * @param {Object<string, LimitedCollectionOptions|number>} [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.
-   * If a number is provided for a manager, it uses that number as the max size for a LimitedCollection
+   * If LimitedCollectionOptions are provided for a manager, it uses those settings to form 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.cacheWithLimits({
    *    MessageManager: 200,
    *    ThreadManager: {
-   *      sweepFilter: SweptCollection.filterByLifetime({
+   *      sweepInterval: 3600,
+   *      sweepFilter: LimitedCollection.filterByLifetime({
    *        getComparisonTimestamp: e => e.archiveTimestamp,
    *        excludeFromSweep: e => !e.archived,
    *      }),
@@ -165,7 +167,7 @@ class Options extends null {
    * Options.cacheWithLimits({
    *   MessageManager: {
    *     sweepInterval: 300,
-   *     sweepFilter: SweptCollection.filterByLifetime({
+   *     sweepFilter: LimitedCollection.filterByLifetime({
    *       lifetime: 1800,
    *       getComparisonTimestamp: e => e.editedTimestamp ?? e.createdTimestamp,
    *     })
@@ -175,20 +177,31 @@ class Options extends null {
   static cacheWithLimits(settings = {}) {
     const { Collection } = require('@discordjs/collection');
     const LimitedCollection = require('./LimitedCollection');
-    const SweptCollection = require('./SweptCollection');
 
     return manager => {
       const setting = settings[manager.name];
-      if (typeof setting === 'number' && setting !== Infinity) return new LimitedCollection(setting);
+      /* eslint-disable-next-line eqeqeq */
-      if (
+      if (setting == null) {
-        /* eslint-disable-next-line eqeqeq */
-        (setting?.sweepInterval == null && setting?.sweepFilter == null) ||
-        setting.sweepInterval <= 0 ||
-        setting.sweepInterval === Infinity
-      ) {
         return new Collection();
       }
-      return new SweptCollection(setting);
+      if (typeof setting === 'number') {
+        if (setting === Infinity) {
+          return new Collection();
+        }
+        return new LimitedCollection({ maxSize: setting });
+      }
+      /* eslint-disable eqeqeq */
+      const noSweeping =
+        setting.sweepFilter == null ||
+        setting.sweepInterval == null ||
+        setting.sweepInterval <= 0 ||
+        setting.sweepInterval === Infinity;
+      const noLimit = setting.maxSize == null || setting.maxSize === Infinity;
+      /* eslint-enable eqeqeq */
+      if (noSweeping && noLimit) {
+        return new Collection();
+      }
+      return new LimitedCollection(setting);
     };
   }
 

src/util/SweptCollection.js

@@ -1,115 +0,0 @@
-'use strict';
-
-const { Collection } = require('@discordjs/collection');
-const { _cleanupSymbol } = require('./Constants.js');
-const { TypeError } = require('../errors/DJSError.js');
-
-/**
- * @typedef {Function} SweepFilter
- * @param {SweptCollection} collection The collection being swept
- * @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.
- */
-
-/**
- * Options for defining the behavior of a Swept Collection
- * @typedef {Object} SweptCollectionOptions
- * @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.
- */
-
-/**
- * A Collection which holds a max amount of entries and sweeps periodically.
- * @extends {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 = {}, iterable) {
-    if (typeof options !== 'object' || options === null) {
-      throw new TypeError('INVALID_TYPE', 'options', 'object or iterable', true);
-    }
-    const { sweepFilter = null, sweepInterval = 3600 } = options;
-
-    if (sweepFilter !== null && typeof sweepFilter !== 'function') {
-      throw new TypeError('INVALID_TYPE', 'sweepFunction', 'function');
-    }
-    if (typeof sweepInterval !== 'number') throw new TypeError('INVALID_TYPE', 'sweepInterval', 'number');
-
-    super(iterable);
-
-    /**
-     * A function called every sweep interval that returns a function passed to `sweep`
-     * @type {?SweepFilter}
-     */
-    this.sweepFilter = sweepFilter;
-
-    /**
-     * The id of the interval being used to sweep.
-     * @type {?Timeout}
-     */
-    this.interval =
-      sweepInterval > 0 && sweepFilter
-        ? setInterval(() => {
-            const sweepFn = this.sweepFilter(this);
-            if (sweepFn === null) return;
-            if (typeof sweepFn !== 'function') throw new TypeError('SWEEP_FILTER_RETURN');
-            this.sweep(sweepFn);
-          }, sweepInterval * 1000).unref()
-        : null;
-  }
-
-  /**
-   * 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, 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.
-   */
-
-  /**
-   * Create a sweepFilter function that uses a lifetime to determine sweepability.
-   * @param {LifetimeFilterOptions} [options={}] The options used to generate the filter function
-   * @returns {SweepFilter}
-   */
-  static filterByLifetime({
-    lifetime = 14400,
-    getComparisonTimestamp = e => e?.createdTimestamp,
-    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 (typeof excludeFromSweep !== 'function') {
-      throw new TypeError('INVALID_TYPE', 'excludeFromSweep', 'function');
-    }
-    return () => {
-      if (lifetime <= 0) return null;
-      const lifetimeMs = lifetime * 1000;
-      const now = Date.now();
-      return (entry, key, coll) => {
-        if (excludeFromSweep(entry, key, coll)) {
-          return false;
-        }
-        const comparisonTimestamp = getComparisonTimestamp(entry, key, coll);
-        if (!comparisonTimestamp || typeof comparisonTimestamp !== 'number') return false;
-        return now - comparisonTimestamp > lifetimeMs;
-      };
-    };
-  }
-
-  [_cleanupSymbol]() {
-    clearInterval(this.interval);
-  }
-
-  static get [Symbol.species]() {
-    return Collection;
-  }
-}
-
-module.exports = SweptCollection;

test/tester2000.js

@@ -12,6 +12,14 @@ const client = new Client({
   makeCache: Options.cacheWithLimits({
     MessageManager: 10,
     PresenceManager: 10,
+    UserManager: {
+      maxSize: 1,
+      keepOverLimit: v => v.id === client.user.id,
+    },
+    GuildMemberManager: {
+      maxSize: 1,
+      keepOverLimit: v => v.id === client.user.id,
+    },
   }),
 });
 

typings/index.d.ts

@@ -965,8 +965,13 @@ export class InviteGuild extends AnonymousGuild {
 }
 
 export class LimitedCollection<K, V> extends Collection<K, V> {
-  public constructor(maxSize?: number, iterable?: Iterable<readonly [K, V]>);
+  public constructor(options?: LimitedCollectionOptions<K, V>, iterable?: Iterable<readonly [K, V]>);
   public maxSize: number;
+  public keepOverLimit: ((value: V, key: K, collection: this) => boolean) | null;
+  public interval: NodeJS.Timeout | null;
+  public sweepFilter: SweepFilter<K, V> | null;
+
+  public static filterByLifetime<K, V>(options?: LifetimeFilterOptions<K, V>): SweepFilter<K, V>;
 }
 
 export class Message extends Base {
@@ -1611,14 +1616,6 @@ export class StoreChannel extends GuildChannel {
   public type: 'GUILD_STORE';
 }
 
-export class SweptCollection<K, V> extends Collection<K, V> {
-  public constructor(options?: SweptCollectionOptions<K, V>, iterable?: Iterable<readonly [K, V]>);
-  public interval: NodeJS.Timeout | null;
-  public sweepFilter: SweptCollectionSweepFilter<K, V> | null;
-
-  public static filterByLifetime<K, V>(options?: LifetimeFilterOptions<K, V>): SweptCollectionSweepFilter<K, V>;
-}
-
 export class SystemChannelFlags extends BitField<SystemChannelFlagsString> {
   public static FLAGS: Record<SystemChannelFlagsString, number>;
   public static resolve(bit?: BitFieldResolvable<SystemChannelFlagsString, number>): number;
@@ -2859,13 +2856,7 @@ export type BitFieldResolvable<T extends string, N extends number | bigint> =
 
 export type BufferResolvable = Buffer | string;
 
-export type CachedManagerTypes = keyof CacheFactoryArgs;
+export interface Caches {
-
-export type CacheFactory = <T extends CachedManagerTypes>(
-  ...args: CacheFactoryArgs[T]
-) => Collection<Snowflake, CacheFactoryArgs[T][1]>;
-
-export interface CacheFactoryArgs {
   ApplicationCommandManager: [manager: typeof ApplicationCommandManager, holds: typeof ApplicationCommand];
   BaseGuildEmojiManager: [manager: typeof BaseGuildEmojiManager, holds: typeof GuildEmoji];
   ChannelManager: [manager: typeof ChannelManager, holds: typeof Channel];
@@ -2886,8 +2877,21 @@ export interface CacheFactoryArgs {
   VoiceStateManager: [manager: typeof VoiceStateManager, holds: typeof VoiceState];
 }
 
+export type CacheConstructors = {
+  [K in keyof Caches]: Caches[K][0] & { name: K };
+};
+
+// This doesn't actually work the way it looks 😢.
+// Narrowing the type of `manager.name` doesn't propagate type information to `holds` and the return type.
+export type CacheFactory = (
+  manager: CacheConstructors[keyof Caches],
+  holds: Caches[typeof manager['name']][1],
+) => typeof manager['prototype'] extends DataManager<infer K, infer V, any> ? Collection<K, V> : never;
+
 export type CacheWithLimitsOptions = {
-  [K in CachedManagerTypes]?: SweptCollectionOptions<unknown, unknown> | number;
+  [K in keyof Caches]?: Caches[K][0]['prototype'] extends DataManager<infer K, infer V, any>
+    ? LimitedCollectionOptions<K, V> | number
+    : never;
 };
 
 export interface ChannelCreationOverwrites {
@@ -3019,9 +3023,9 @@ export interface ClientOptions {
   shards?: number | number[] | 'auto';
   shardCount?: number;
   makeCache?: CacheFactory;
-  /** @deprecated Use `makeCache` with a `SweptCollection` for `MessageManager` instead. */
+  /** @deprecated Use `makeCache` with a `LimitedCollection` for `MessageManager` instead. */
   messageCacheLifetime?: number;
-  /** @deprecated Use `makeCache` with a `SweptCollection` for `MessageManager` instead. */
+  /** @deprecated Use `makeCache` with a `LimitedCollection` for `MessageManager` instead. */
   messageSweepInterval?: number;
   allowedMentions?: MessageMentionOptions;
   invalidRequestWarningInterval?: number;
@@ -3773,8 +3777,8 @@ export type InviteScope =
   | 'webhook.incoming';
 
 export interface LifetimeFilterOptions<K, V> {
-  excludeFromSweep?: (value: V, key: K, collection: SweptCollection<K, V>) => boolean;
+  excludeFromSweep?: (value: V, key: K, collection: LimitedCollection<K, V>) => boolean;
-  getComparisonTimestamp?: (value: V, key: K, collection: SweptCollection<K, V>) => number;
+  getComparisonTimestamp?: (value: V, key: K, collection: LimitedCollection<K, V>) => number;
   lifetime?: number;
 }
 
@@ -4313,12 +4317,14 @@ export interface StageInstanceEditOptions {
   privacyLevel?: PrivacyLevel | number;
 }
 
-export type SweptCollectionSweepFilter<K, V> = (
+export type SweepFilter<K, V> = (
-  collection: SweptCollection<K, V>,
+  collection: LimitedCollection<K, V>,
-) => ((value: V, key: K, collection: SweptCollection<K, V>) => boolean) | null;
+) => ((value: V, key: K, collection: LimitedCollection<K, V>) => boolean) | null;
 
-export interface SweptCollectionOptions<K, V> {
+export interface LimitedCollectionOptions<K, V> {
-  sweepFilter?: SweptCollectionSweepFilter<K, V>;
+  maxSize?: number;
+  keepOverLimit?: (value: V, key: K, collection: LimitedCollection<K, V>) => boolean;
+  sweepFilter?: SweepFilter<K, V>;
   sweepInterval?: number;
 }
 

typings/index.ts

@@ -3,6 +3,8 @@ import {
   ApplicationCommandData,
   ApplicationCommandManager,
   ApplicationCommandResolvable,
+  CacheFactory,
+  Caches,
   CategoryChannel,
   Client,
   ClientApplication,
@@ -22,12 +24,14 @@ import {
   GuildResolvable,
   Intents,
   Interaction,
+  LimitedCollection,
   Message,
   MessageActionRow,
   MessageAttachment,
   MessageButton,
   MessageCollector,
   MessageEmbed,
+  MessageManager,
   MessageReaction,
   NewsChannel,
   Options,
@@ -59,9 +63,12 @@ const client: Client = new Client({
     // @ts-expect-error
     Message: 100,
     ThreadManager: {
-      sweepInterval: require('./SweptCollection').filterByLifetime({
+      maxSize: 1000,
-        getComparisonTimestamp: (e: any) => e.archiveTimestamp,
+      keepOverLimit: (x: ThreadChannel) => x.id === '123',
-        excludeFromSweep: (e: any) => !e.archived,
+      sweepInterval: 5000,
+      sweepFilter: LimitedCollection.filterByLifetime({
+        getComparisonTimestamp: (x: ThreadChannel) => x.archiveTimestamp ?? 0,
+        excludeFromSweep: (x: ThreadChannel) => !x.archived,
       }),
     },
   }),