Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,39 @@ jobs:
- name: Run library tests
run: |
npm ci
npm run test:lib
npm run test:unit
test-integration:
name: Test (Integration)
needs: [test-library]
runs-on: ubuntu-latest
services:
redis:
image: redis:7
ports:
- 6379:6379
redis-cluster:
image: grokzen/redis-cluster:7.0.0
env:
IP: 0.0.0.0
ports:
- 7000-7005:7000-7005
steps:
- name: Checkout the repository
uses: actions/checkout@v3
- name: Use Node lts/*
uses: actions/setup-node@v3
with:
node-version: lts/*
- name: Run integration tests
run: |
npm ci
npm run test:integration
env:
REDIS_PORT: 6379
REDIS_CLUSTER_PORT: 7000
publish:
name: Publish
needs: [lint, test-library]
needs: [lint, test-library, test-integration]
if: startsWith(github.ref, 'refs/tags/v')
runs-on: ubuntu-latest
permissions:
Expand Down
13 changes: 13 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
version: '3'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the Compose file required if we are already mentioning services in the CI file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually don’t need a compose file, but it is required (or recommended) for quickly deploying a Redis cluster on local.

services:
redis:
image: redis:7
ports:
- '6385:6379'

redis-cluster:
image: grokzen/redis-cluster:7.0.0
environment:
- IP=0.0.0.0
ports:
- '7010-7015:7000-7005'
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@
"format:code": "xo --fix",
"format:rest": "prettier --write .",
"format": "run-s format:*",
"test:unit": "jest test/store-test.ts",
"test:integration": "jest test/redis-integration-test.ts",
"test:lib": "jest",
"test": "run-s lint test:*",
"test": "run-s lint test:lib",
Comment on lines +49 to +52
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be simplified by dropping test:lib:

Suggested change
"test:unit": "jest test/store-test.ts",
"test:integration": "jest test/redis-integration-test.ts",
"test:lib": "jest",
"test": "run-s lint test:*",
"test": "run-s lint test:lib",
"test:unit": "jest test/store-test.ts",
"test:integration": "jest test/redis-integration-test.ts",
"test": "run-s lint test:*",

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, alternatively, if we want npm test to work without a local redis server, make it this:

Suggested change
"test:unit": "jest test/store-test.ts",
"test:integration": "jest test/redis-integration-test.ts",
"test:lib": "jest",
"test": "run-s lint test:*",
"test": "run-s lint test:lib",
"test:unit": "jest test/store-test.ts",
"test:integration": "jest test/redis-integration-test.ts",
"test": "run-s lint test:unit",

"pre-commit": "lint-staged",
"prepare": "run-s compile && husky install config/husky"
},
Expand Down
37 changes: 29 additions & 8 deletions source/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export class RedisStore implements Store {
*/
async retryableIncrement(_key: string): Promise<RedisReply> {
const key = this.prefixKey(_key)
const evalCommand = async () =>
const evalShaCommand = async () =>
this.sendCommand({
key,
isReadOnly: false,
Expand All @@ -169,12 +169,28 @@ export class RedisStore implements Store {
],
})

const evalCommand = async () =>
this.sendCommand({
key,
isReadOnly: false,
command: [
'EVAL',
scripts.increment,
'1',
key,
this.resetExpiryOnChange ? '1' : '0',
this.windowMs.toString(),
],
})

try {
const result = await evalCommand()
const result = await evalShaCommand()
return result
} catch {
// TODO: distinguish different error types
this.incrementScriptSha = this.loadIncrementScript(key)
// In Redis Cluster, scripts loaded on one node may not be available
// on the node where the key is located. Fall back to EVAL.
// We don't need to check for NOSCRIPT explicitly because if EVALSHA fails
// for ANY reason, EVAL is a safe fallback that guarantees execution.
return evalCommand()
}
}
Expand Down Expand Up @@ -209,17 +225,22 @@ export class RedisStore implements Store {
async get(_key: string): Promise<ClientRateLimitInfo | undefined> {
const key = this.prefixKey(_key)
let results
const evalCommand = async () =>
const evalShaCommand = async () =>
this.sendCommand({
key,
isReadOnly: true,
command: ['EVALSHA', await this.getScriptSha, '1', key],
})
const evalCommand = async () =>
this.sendCommand({
key,
isReadOnly: true,
command: ['EVAL', scripts.get, '1', key],
})
try {
results = await evalCommand()
results = await evalShaCommand()
} catch {
// TODO: distinguish different error types
this.getScriptSha = this.loadGetScript(key)
// Fallback to EVAL if EVALSHA fails
results = await evalCommand()
}

Expand Down
190 changes: 190 additions & 0 deletions test/redis-integration-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
import process from 'node:process'
import { describe, it, expect, beforeAll, afterAll, jest } from '@jest/globals'
import { Redis, Cluster } from 'ioredis'
import { type Options as RateLimitOptions } from 'express-rate-limit'
import { RedisStore } from '../source/index.js'
import type {
SendCommandClusterDetails,
RedisReply,
Options as RedisOptions,
} from '../source/types.js'

jest.setTimeout(30_000)

describe('Redis Integration Tests', () => {
describe('Single Redis Instance', () => {
let client: Redis
let store: RedisStore

beforeAll(async () => {
const host = process.env.REDIS_HOST ?? 'localhost'
const port = Number(process.env.REDIS_PORT ?? 6385)

client = new Redis({
host,
port,
lazyConnect: true,
})
await client.connect()
})

afterAll(async () => {
await client?.quit()
})

it('should work with sendCommand', async () => {
store = new RedisStore({
async sendCommand(...args: string[]) {
const result = await client.call(args[0], ...args.slice(1))
return result as RedisReply
},
} as RedisOptions)
store.init({ windowMs: 1000 } as RateLimitOptions)

const key = 'test-single'
await store.resetKey(key)

const result1 = await store.increment(key)
expect(result1.totalHits).toBe(1)

const result2 = await store.increment(key)
expect(result2.totalHits).toBe(2)

await store.resetKey(key)
const result3 = await store.increment(key)
expect(result3.totalHits).toBe(1)
})

it('should work with decrement', async () => {
const key = 'test-single-decr'
await store.resetKey(key)

await store.increment(key) // 1
await store.increment(key) // 2
await store.decrement(key) // 1

const result = await store.increment(key) // 2
expect(result.totalHits).toBe(2)
})

it('should work with get', async () => {
const key = 'test-single-get'
await store.resetKey(key)

await store.increment(key)
const info = await store.get(key)
expect(info).toBeDefined()
expect(info?.totalHits).toBe(1)
expect(info?.resetTime).toBeInstanceOf(Date)
})

it('should handle TTL correctly', async () => {
const key = 'test-single-ttl'
// Initialize with short window
const shortStore = new RedisStore({
async sendCommand(...args: string[]) {
const result = await client.call(args[0], ...args.slice(1))
return result as RedisReply
},
} as RedisOptions)
shortStore.init({ windowMs: 1000 } as RateLimitOptions)

await shortStore.resetKey(key)
await shortStore.increment(key)

// Wait for expiration (1.1s)
await new Promise<void>((resolve) => {
setTimeout(resolve, 1100)
})

const result = await shortStore.increment(key)
expect(result.totalHits).toBe(1)
})
})

describe('Redis Cluster', () => {
let client: Cluster
let store: RedisStore

beforeAll(async () => {
const host = process.env.REDIS_CLUSTER_HOST ?? 'localhost'
const port = Number(process.env.REDIS_CLUSTER_PORT ?? 7010)

client = new Cluster([{ host, port }], {
redisOptions: { connectTimeout: 2000 },
clusterRetryStrategy: () => null,
lazyConnect: true,
})
await client.connect()
})

afterAll(async () => {
await client?.quit()
})

it('should work with sendCommandCluster', async () => {
store = new RedisStore({
sendCommandCluster: async (details: SendCommandClusterDetails) =>
client.call(details.command[0], ...details.command.slice(1)),
} as RedisOptions)
store.init({ windowMs: 1000 } as RateLimitOptions)

const key = 'test-cluster'
await store.resetKey(key)

const result1 = await store.increment(key)
expect(result1.totalHits).toBe(1)

const result2 = await store.increment(key)
expect(result2.totalHits).toBe(2)

await store.resetKey(key)
const result3 = await store.increment(key)
expect(result3.totalHits).toBe(1)
})

it('should work with decrement', async () => {
const key = 'test-cluster-decr'
await store.resetKey(key)

await store.increment(key) // 1
await store.increment(key) // 2
await store.decrement(key) // 1

const result = await store.increment(key) // 2
expect(result.totalHits).toBe(2)
})

it('should work with get', async () => {
const key = 'test-cluster-get'
await store.resetKey(key)

await store.increment(key)
const info = await store.get(key)
expect(info).toBeDefined()
expect(info?.totalHits).toBe(1)
expect(info?.resetTime).toBeInstanceOf(Date)
})

it('should handle TTL correctly', async () => {
const key = 'test-cluster-ttl'
// Initialize with short window
const shortStore = new RedisStore({
sendCommandCluster: async (details: SendCommandClusterDetails) =>
client.call(details.command[0], ...details.command.slice(1)),
} as RedisOptions)
shortStore.init({ windowMs: 1000 } as RateLimitOptions)

await shortStore.resetKey(key)
await shortStore.increment(key)

// Wait for expiration (1.1s)
await new Promise<void>((resolve) => {
setTimeout(resolve, 1100)
})

const result = await shortStore.increment(key)
expect(result.totalHits).toBe(1)
})
})
})