diff --git a/RFC-Caching-Layer.md b/RFC-Caching-Layer.md new file mode 100644 index 000000000..c4d53f53e --- /dev/null +++ b/RFC-Caching-Layer.md @@ -0,0 +1,212 @@ + + +# 1. Introduction + +Key supported use cases & goals: + +- **Zero Caching:** The library must function perfectly even with caching completely disabled +- **Async, Remote Caching:** Support for external, asynchronous data stores like Redis +- **In-Memory Caching:** The traditional `Collection`-based synchronous cache (default) +- **Complete Flexibility**: Absolute user control over what is cached and for how long, without any internal assumptions or limitations + +I ask readers to address the dedicated "Problems" sections in each part of the proposal, as well as any other general +concerns. + +Also of note, I'm aware that we're going to be using some sort of dependency injection container. For the sake of simplicity, +this proposal just takes in the `Client` everywhere. + +# 2. Design Principles + +1. **Async-First:** All cache operations must return an `Awaitable` +2. **Fetch-First:** Users should use `manager.fetch(id)` to get data. The manager will check the cache first, and if it misses + (or if caching is disabled), it will hit the API +3. **Map-like:** The cache interface should remain map-like, as it currently is in mainlib + +# 3. The Cache Interface + +At this time, I think those are the primary methods that need implemented. They are most certainly enough for most use +cases. Some considerations around being able to iterate on the data should be made. + +## Suggested Interface + +```ts +import type { Awaitable } from '@discordjs/util'; +import type { Structure } from '@discordjs/structures'; +import type { Snowflake } from 'discord-api-types/globals'; + +export type RawAPIType = T extends Structure ? Raw : never; + +export type StructureCreator< + Value extends Structure<{ id: Snowflake }>, + Raw extends RawAPIType = RawAPIType, +> = (data: Partial & { id: Snowflake }) => Value; + +export interface Cache, Raw extends RawAPIType = RawAPIType> { + /** + * The function used to construct instances of the structure this cache holds. + */ + readonly construct: StructureCreator; + + /** + * Retrieves an item from the cache. + */ + get(key: Snowflake): Awaitable; + + /** + * Sets an item in the cache. + */ + set(key: Snowflake, value: Value): Awaitable; + + /** + * Adds or updates data in the cache, returning the instantiated structure. + * If the item exists, it patches it with the new data unless `overwrite` is true. + * If it does not exist, it constructs a new instance and stores it. + */ + add(data: Partial & { id: Snowflake }, overwrite?: boolean): Awaitable; + + /** + * Checks if an item exists in the cache. + */ + has(key: Snowflake): Awaitable; + + /** + * Deletes an item from the cache. + */ + delete(key: Snowflake): Awaitable; + + /** + * Clears all items from the cache. + */ + clear(): Awaitable; + + /** + * Gets the number of items in the cache. + */ + getSize(): Awaitable; +} +``` + +[^1] + +## Problems + +- Consider use cases that require iterating over cached items. `keys()`, `values()`, and `entries()` could be added, + but they raise some type questions. They probably have to return `AsyncIterable | Iterable`? +- It's possible there's an oversight with `add` being so highly-responsible (i.e. functioning as more of an upsert). + +# 4. The Manager Pattern + +## Suggested implementation + +```ts +import type { Client } from '../client/Client.js'; +import type { Cache, RawAPIType } from '../cache/Cache.js'; +import type { Structure } from '@discordjs/structures'; +import type { Snowflake } from 'discord-api-types/globals'; + +export abstract class BaseManager< + Value extends Structure<{ id: Snowflake }>, + Raw extends RawAPIType = RawAPIType, +> { + public constructor( + public readonly client: Client, + public readonly cache: Cache, + ) {} + + // Child classes provide methods specific to the endpoints they manage. +} +``` + +## Problems + +- Considering `_add` is now notably absent, we can potentially justify just having an interface rather than a base class. +- Something to consider is that perhaps `StructureCreator` should be on the Manager rather than the Cache. + I handled it this way to avoid the `Cache` needing a back-reference to the Manager, but we do end up with cases + where Manager code has to `this.cache.construct`. Perhaps an option is for both the `Manager` and `Cache` to have it. + +## Specific manager: UserManager + +The `UserManager` implements the `fetch` method, which is the primary way users will interact with data. + +```ts +import { BaseManager } from './BaseManager.js'; +import { User } from '@discordjs/structures'; +import type { APIUser } from 'discord-api-types/v10'; +import type { Snowflake } from 'discord-api-types/globals'; + +export interface FetchOptions { + /** + * Whether to skip the cache check and force an API request. + */ + force?: boolean; + /** + * Whether to cache the fetched result. + */ + cache?: boolean; +} + +export class UserManager extends BaseManager { + /** + * Fetches a user by ID. + * Checks the cache first unless `force` is true. + */ + public async fetch(id: Snowflake, options: FetchOptions = {}): Promise { + const { force = false, cache = true } = options; + + if (!force) { + const cached = await this.cache.get(id); + if (cached) { + return cached; + } + } + + // Cache miss or forced fetch. Hit the API. + // Assuming `this.client.api` is an instance of `@discordjs/core` API + const rawUser = await this.client.api.users.get(id); + + if (!cache) { + return this.cache.construct(rawUser); + } + + // Pass the raw data to the cache to handle caching and instantiation. + // We pass overwrite=true because we know this is a fresh, complete object from the API. + return await this.cache.add(rawUser, true); + } + + // Other methods relevant to /users endpoints +} +``` + +## Note on `_add` + +In the old implementation, `BaseManager` had an `_add` method responsible for instantiating structures and +patching existing ones. In this new design, that responsibility has been entirely shifted to the `Cache` interface. + +# 5. Usage examples + +```ts +// Userland code. Notably what you would do today +const user = await client.users.fetch('123456789012345678'); +console.log(user.username); + +// A difference that will very quickly & obviously pop up is around structures we normally guaranteed were cached: +const channel = await client.channels.fetch(process.env.SOME_CHANNEL_ID); +console.log(channel.name); + +// Gateway Event Example (Internal Library Code) +client.ws.on('USER_UPDATE', async (partialData) => { + // partialData might only contain { id: '...', username: 'new_name' } + // A great aspect about this is that we have no cloning logic here! Patching logic under the hood does not mutate. + // A drawback is that `add` will invoke `.get` internally as well. Might want to consider some method of optimization here. + const existingUser = await client.users.cache.get(partialData.id); + // Note: If the user is not in the cache, this will construct and cache a partial User structure. + const updatedUser = await client.users.cache.add(partialData); + if (existingUser) { + client.emit('userUpdate', existingUser, updatedUser); + } +}); +``` + +# Footnotes + +[^1] Derived from some existing work/PoC done in #10983 diff --git a/packages/brokers/package.json b/packages/brokers/package.json index fb583b145..2be25d68e 100644 --- a/packages/brokers/package.json +++ b/packages/brokers/package.json @@ -67,6 +67,8 @@ "homepage": "https://discord.js.org", "funding": "https://github.com/discordjs/discord.js?sponsor", "dependencies": { + "@discordjs/core": "workspace:^", + "@discordjs/ws": "workspace:^", "@msgpack/msgpack": "^3.1.3", "@vladfrangu/async_event_emitter": "^2.4.7", "ioredis": "^5.9.3" diff --git a/packages/brokers/src/brokers/redis/RedisGateway.ts b/packages/brokers/src/brokers/redis/RedisGateway.ts new file mode 100644 index 000000000..4e4498615 --- /dev/null +++ b/packages/brokers/src/brokers/redis/RedisGateway.ts @@ -0,0 +1,138 @@ +import type { Gateway, GatewayDispatchPayload, GatewaySendPayload, GatewayDispatchEvents } from '@discordjs/core'; +import type { ManagerShardEventsMap, WebSocketShardEvents } from '@discordjs/ws'; +import { AsyncEventEmitter } from '@vladfrangu/async_event_emitter'; +import type { PubSubRedisBroker } from './PubSubRedis.js'; + +export type DiscordEvents = { + [K in GatewayDispatchEvents]: GatewayDispatchPayload & { + t: K; + }; +}; + +interface BrokerProps { + payload: Payload; + shardId: number; +} + +interface Events extends DiscordEvents { + // eslint-disable-next-line @typescript-eslint/no-use-before-define + [RedisGateway.GatewaySendEvent]: GatewaySendPayload; +} + +export type RedisBrokerDiscordEvents = { + [K in keyof Events]: BrokerProps; +}; + +/** + * RedisGateway is an implementation for core's Gateway interface built on top of our Redis brokers. + * + * Some important notes: + * - Instances for this class are for your consumers/services that need the gateway. Naturally, the events passed into + * `init` are the only ones the core client will be able to emit + * - You can also opt to use the class as-is without `@discordjs/core`, if you so desire. Events are properly typed + * - You need to implement your own gateway service. Refer to the example below for how that would look like. This class + * offers some static methods and properties that help in this errand. It is extremely important that you `publish` + * events as the receiving service expects, and also that you handle GatewaySend events. + * - One drawback to using this directly with `@discordjs/core` is that you lose granular control over when to `ack` + * events. This implementation `ack`s as soon as the event is emitted to listeners. In practice, this means that if your + * service crashes while handling an event, it's pretty arbitrary wether that event gets re-processed on restart or not. + * (Mostly dependant on if your handler is async or not, and also if the `ack` call has time to go through). + * + * @example + * ```ts + * // gateway-service/index.ts + * import { RedisGateway, PubSubRedisBroker, kUseRandomGroupName } from '@discordjs/brokers'; + * import Redis from 'ioredis'; + * + * // the `name` here probably should be env-determined if you need to scale this. see the main README for more information. + * // also, we use a random group because we do NOT want work-balancing on gateway_send events. + * const broker = new PubSubRedisBroker(new Redis(), { group: kUseRandomGroupName, name: 'send-consumer-1' }); + * const gateway = new WebSocketManager(gatewayOptionsHere); // see @discordjs/ws for examples. + * + * // emit events over the broker + * gateway.on(WebSocketShardEvents.Dispatch, (...data) => void broker.publish(...RedisGateway.toPublishArgs(data))); + * + * // listen to payloads we should send to Discord + * broker.on(RedisGateway.GatewaySendEvent, async ({ data: { payload, shardId }, ack }) => { + * await gateway.send(shardId, payload); + * await ack(); + * }); + * await broker.subscribe([RedisGateway.GatewaySendEvent]); + * await gateway.connect(); + * ``` + * + * ```ts + * // other-service/index.ts + * import { RedisGateway, PubSubRedisBroker, kUseRandomGroupName } from '@discordjs/brokers'; + * import Redis from 'ioredis'; + * + * // the name here should absolutely be env-determined, see the main README for more information. + * const broker = new PubSubRedisBroker(new Redis(), { group: 'my-service-name', name: 'service-name-instance-1' }); + * // unfortunately, we have to know the shard count. ideally this should be an env var + * const gateway = new RedisGateway(broker, Number.parseInt(process.env.SHARD_COUNT, 10)); + * + * const rest = new REST({ version: '10' }).setToken(process.env.DISCORD_TOKEN); + * const client = new Client({ rest, gateway }); + * + * // set up your client as you normally would with core + * + * // subscribe to the events that you want + * await gateway.init([GatewayDispatchEvents.GuildCreate, GatewayDispatchEvents.MessageCreate]); + * ``` + */ +export class RedisGateway + extends AsyncEventEmitter<{ dispatch: ManagerShardEventsMap[WebSocketShardEvents.Dispatch] }> + implements Gateway +{ + /** + * Event used over the broker used to tell shards to send a payload to Discord. + */ + public static readonly GatewaySendEvent = 'gateway_send' as const; + + /** + * Converts a dispatch event from `@discordjs/ws` to arguments for a `broker.publish` call. + */ + public static toPublishArgs( + data: ManagerShardEventsMap[WebSocketShardEvents.Dispatch], + ): [GatewayDispatchEvents, BrokerProps] { + const [payload, shardId] = data; + return [payload.t, { shardId, payload }]; + } + + public constructor( + private readonly broker: PubSubRedisBroker, + private readonly shardCount: number, + ) { + super(); + } + + public getShardCount(): number { + return this.shardCount; + } + + public async send(shardId: number, payload: GatewaySendPayload): Promise { + await this.broker.publish(RedisGateway.GatewaySendEvent, { payload, shardId }); + } + + public async init(events: GatewayDispatchEvents[]) { + for (const event of events) { + // async_event_emitter nukes our types on this one. + this.broker.on( + event, + ({ + ack, + data: { payload, shardId }, + }: { + // eslint-disable-next-line @typescript-eslint/method-signature-style + ack: () => Promise; + data: BrokerProps; + }) => { + this.emit('dispatch', payload, shardId); + void ack(); + }, + ); + } + + await this.broker.subscribe(events); + } +} diff --git a/packages/brokers/src/index.ts b/packages/brokers/src/index.ts index 4e69a08d8..65412b67d 100644 --- a/packages/brokers/src/index.ts +++ b/packages/brokers/src/index.ts @@ -1,5 +1,6 @@ export * from './brokers/redis/BaseRedis.js'; export * from './brokers/redis/PubSubRedis.js'; +export * from './brokers/redis/RedisGateway.js'; export * from './brokers/redis/RPCRedis.js'; export * from './brokers/Broker.js'; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 35209178b..8d388b5e0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -683,6 +683,12 @@ importers: packages/brokers: dependencies: + '@discordjs/core': + specifier: workspace:^ + version: link:../core + '@discordjs/ws': + specifier: workspace:^ + version: link:../ws '@msgpack/msgpack': specifier: ^3.1.3 version: 3.1.3 @@ -5737,6 +5743,7 @@ packages: '@smithy/middleware-endpoint@4.4.16': resolution: {integrity: sha512-L5GICFCSsNhbJ5JSKeWFGFy16Q2OhoBizb3X2DrxaJwXSEujVvjG9Jt386dpQn2t7jINglQl0b4K/Su69BdbMA==} engines: {node: '>=18.0.0'} + deprecated: Please upgrade to @smithy/middleware-endpoint@4.1.15 or higher to fix a bug preventing the resolution of ENV and config file custom endpoints https://github.com/smithy-lang/smithy-typescript/issues/1645 '@smithy/middleware-retry@4.4.33': resolution: {integrity: sha512-jLqZOdJhtIL4lnA9hXnAG6GgnJlo1sD3FqsTxm9wSfjviqgWesY/TMBVnT84yr4O0Vfe0jWoXlfFbzsBVph3WA==}