From 03161482b44bd67f6eafb3e3d51107811b638d4b Mon Sep 17 00:00:00 2001 From: Mohamed Bassem Date: Sun, 9 Nov 2025 17:10:54 +0000 Subject: refactor: Extract ratelimiter into separate plugin (#2112) * refactor(trpc): extract rate limiter into dedicated plugin Move the rate limiting middleware from the trpc package to the centralized plugins package. This improves code organization by consolidating all plugins in a single location. Changes: - Created packages/plugins/trpc-ratelimit/ plugin - Moved rate limiter from packages/trpc/rateLimit.ts to packages/plugins/trpc-ratelimit/src/index.ts - Added trpc-ratelimit export to plugins package.json - Added @trpc/server dependency to plugins package - Updated trpc package to import from @karakeep/plugins/trpc-ratelimit - Added @karakeep/plugins dependency to trpc package - Removed packages/trpc/plugins/ directory * refactor(plugins): decouple rate limiter from tRPC Refactor the rate limiting plugin to be framework-agnostic, allowing it to be used outside of tRPC contexts. The plugin now has a generic core with a tRPC-specific adapter. Changes: - Renamed trpc-ratelimit to ratelimit plugin - Created generic RateLimiter class with framework-agnostic API - Added checkRateLimit() method that returns allow/deny results - Created separate tRPC adapter (src/trpc.ts) that uses the generic core - Exported both generic (RateLimiter, globalRateLimiter) and tRPC-specific (createRateLimitMiddleware) APIs - Updated trpc package to import from @karakeep/plugins/ratelimit - Updated plugins package.json exports Benefits: - Rate limiter can now be used in any context (HTTP handlers, WebSocket, etc.) - Cleaner separation of concerns - Easy to create adapters for other frameworks - Generic API allows for custom error handling * refactor(plugins): integrate rate limiter with plugin registry Refactor the rate limiting plugin to use the centralized plugin system with PluginManager, making it consistent with other plugins like queue and search providers. Changes: - Added RateLimit plugin type to PluginType enum - Created RateLimitClient interface in packages/shared/ratelimiting.ts - Created RateLimitProvider class implementing PluginProvider - Updated plugin to auto-register with PluginManager on import - Updated tRPC adapter to use getRateLimitClient() from PluginManager - Added ratelimit plugin to loadAllPlugins() in shared-server - Updated shared/plugins.ts with RateLimit type mapping Benefits: - Consistent plugin architecture across the codebase - Rate limiter can be swapped with alternative implementations - Centralized plugin management and logging - Better separation of concerns - Framework-agnostic core with tRPC adapter pattern * refactor(trpc): move rate limit middleware to trpc package Move the tRPC-specific rate limiting middleware from the plugins package to the trpc package, making the plugins package framework-agnostic. Changes: - Moved packages/plugins/ratelimit/src/trpc.ts to packages/trpc/lib/rateLimit.ts - Updated packages/trpc/index.ts to import from local lib/rateLimit - Removed tRPC export from packages/plugins/ratelimit/index.ts - Removed @trpc/server dependency from packages/plugins/package.json Benefits: - plugins package is now framework-agnostic - tRPC-specific code lives in the trpc package where it belongs - Cleaner separation of concerns - Rate limiter plugin can be used in any context without tRPC * refactor(plugins): rename to ratelimit-memory and add tests Rename the rate limiting plugin from "ratelimit" to "ratelimit-memory" to better indicate it's an in-memory implementation. This naming leaves room for future implementations like ratelimit-redis. Also added comprehensive test coverage. Changes: - Renamed packages/plugins/ratelimit to ratelimit-memory - Updated package.json export from ./ratelimit to ./ratelimit-memory - Updated shared-server to import @karakeep/plugins/ratelimit-memory - Added comprehensive unit tests (index.test.ts): - Rate limit enforcement tests - Window expiration tests - Identifier and path isolation tests - Reset functionality tests - Cleanup mechanism tests - Added provider integration tests (provider.test.ts): - PluginProvider interface compliance - Client singleton behavior - End-to-end rate limiting functionality Benefits: - More descriptive plugin name indicating the storage mechanism - Better test coverage ensuring reliability - Easier to add alternative implementations (Redis, etc.) * change the api to only take the key * move the serverConfig check to the trpc * fix lockfile * get rid of the timer --------- Co-authored-by: Claude --- .../plugins/ratelimit-memory/src/index.test.ts | 253 +++++++++++++++++++++ 1 file changed, 253 insertions(+) create mode 100644 packages/plugins/ratelimit-memory/src/index.test.ts (limited to 'packages/plugins/ratelimit-memory/src/index.test.ts') diff --git a/packages/plugins/ratelimit-memory/src/index.test.ts b/packages/plugins/ratelimit-memory/src/index.test.ts new file mode 100644 index 00000000..5bbed769 --- /dev/null +++ b/packages/plugins/ratelimit-memory/src/index.test.ts @@ -0,0 +1,253 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { RateLimiter } from "./index"; + +describe("RateLimiter", () => { + let rateLimiter: RateLimiter; + + beforeEach(() => { + rateLimiter = new RateLimiter(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + rateLimiter.clear(); + }); + + describe("checkRateLimit", () => { + it("should allow requests within rate limit", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 3, + }; + + const result1 = rateLimiter.checkRateLimit(config, "user1"); + const result2 = rateLimiter.checkRateLimit(config, "user1"); + const result3 = rateLimiter.checkRateLimit(config, "user1"); + + expect(result1.allowed).toBe(true); + expect(result2.allowed).toBe(true); + expect(result3.allowed).toBe(true); + }); + + it("should block requests exceeding rate limit", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 2, + }; + + const result1 = rateLimiter.checkRateLimit(config, "user1"); + const result2 = rateLimiter.checkRateLimit(config, "user1"); + const result3 = rateLimiter.checkRateLimit(config, "user1"); + + expect(result1.allowed).toBe(true); + expect(result2.allowed).toBe(true); + expect(result3.allowed).toBe(false); + expect(result3.resetInSeconds).toBeDefined(); + expect(result3.resetInSeconds).toBeGreaterThan(0); + }); + + it("should reset after window expires", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 2, + }; + + // First two requests allowed + const result1 = rateLimiter.checkRateLimit(config, "user1"); + const result2 = rateLimiter.checkRateLimit(config, "user1"); + expect(result1.allowed).toBe(true); + expect(result2.allowed).toBe(true); + + // Third request blocked + const result3 = rateLimiter.checkRateLimit(config, "user1"); + expect(result3.allowed).toBe(false); + + // Advance time past the window + vi.advanceTimersByTime(61000); + + // Should allow request after window reset + const result4 = rateLimiter.checkRateLimit(config, "user1"); + expect(result4.allowed).toBe(true); + }); + + it("should isolate rate limits by identifier", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 1, + }; + + const result1 = rateLimiter.checkRateLimit(config, "user1"); + const result2 = rateLimiter.checkRateLimit(config, "user2"); + + expect(result1.allowed).toBe(true); + expect(result2.allowed).toBe(true); + }); + + it("should isolate rate limits by key", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 1, + }; + + const result1 = rateLimiter.checkRateLimit(config, "user1:/api/v1"); + const result2 = rateLimiter.checkRateLimit(config, "user1:/api/v2"); + + expect(result1.allowed).toBe(true); + expect(result2.allowed).toBe(true); + }); + + it("should isolate rate limits by config name", () => { + const config1 = { + name: "api", + windowMs: 60000, + maxRequests: 1, + }; + const config2 = { + name: "auth", + windowMs: 60000, + maxRequests: 1, + }; + + const result1 = rateLimiter.checkRateLimit(config1, "user1"); + const result2 = rateLimiter.checkRateLimit(config2, "user1"); + + expect(result1.allowed).toBe(true); + expect(result2.allowed).toBe(true); + }); + + it("should calculate correct resetInSeconds", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 1, + }; + + // First request allowed + rateLimiter.checkRateLimit(config, "user1"); + + // Advance time by 30 seconds + vi.advanceTimersByTime(30000); + + // Second request blocked + const result = rateLimiter.checkRateLimit(config, "user1"); + expect(result.allowed).toBe(false); + // Should have ~30 seconds remaining + expect(result.resetInSeconds).toBeGreaterThan(29); + expect(result.resetInSeconds).toBeLessThanOrEqual(30); + }); + }); + + describe("reset", () => { + it("should reset rate limit for specific identifier", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 1, + }; + + // Use up the limit + rateLimiter.checkRateLimit(config, "user1"); + const result1 = rateLimiter.checkRateLimit(config, "user1"); + expect(result1.allowed).toBe(false); + + // Reset the limit + rateLimiter.reset(config, "user1"); + + // Should allow request again + const result2 = rateLimiter.checkRateLimit(config, "user1"); + expect(result2.allowed).toBe(true); + }); + + it("should reset rate limit for specific key", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 1, + }; + + // Use up the limit for key1 + rateLimiter.checkRateLimit(config, "user1:/path1"); + const result1 = rateLimiter.checkRateLimit(config, "user1:/path1"); + expect(result1.allowed).toBe(false); + + // Reset only key1 + rateLimiter.reset(config, "user1:/path1"); + + // key1 should be allowed + const result2 = rateLimiter.checkRateLimit(config, "user1:/path1"); + expect(result2.allowed).toBe(true); + }); + + it("should not affect other identifiers", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 1, + }; + + // Use up limits for both users + rateLimiter.checkRateLimit(config, "user1"); + rateLimiter.checkRateLimit(config, "user2"); + + // Reset only user1 + rateLimiter.reset(config, "user1"); + + const result1 = rateLimiter.checkRateLimit(config, "user1"); + const result2 = rateLimiter.checkRateLimit(config, "user2"); + + expect(result1.allowed).toBe(true); + expect(result2.allowed).toBe(false); + }); + }); + + describe("clear", () => { + it("should clear all rate limits", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 1, + }; + + // Use up limits for multiple users + rateLimiter.checkRateLimit(config, "user1"); + rateLimiter.checkRateLimit(config, "user2"); + + // Clear all limits + rateLimiter.clear(); + + // All should be allowed + const result1 = rateLimiter.checkRateLimit(config, "user1"); + const result2 = rateLimiter.checkRateLimit(config, "user2"); + + expect(result1.allowed).toBe(true); + expect(result2.allowed).toBe(true); + }); + }); + + describe("cleanup", () => { + it("should cleanup expired entries", () => { + const config = { + name: "test", + windowMs: 60000, + maxRequests: 1, + }; + + // Create an entry + rateLimiter.checkRateLimit(config, "user1"); + + // Advance time past window + cleanup interval + vi.advanceTimersByTime(61000 + 60000); + + // Entry should be cleaned up and new request allowed + const result = rateLimiter.checkRateLimit(config, "user1"); + expect(result.allowed).toBe(true); + }); + }); +}); -- cgit v1.2.3-70-g09d2