Skip to content
Merged
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
5 changes: 3 additions & 2 deletions source/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,11 @@ export class RedisStore implements Store {

if ('sendCommand' in options && !('sendCommandCluster' in options)) {
// Normal case: wrap the sendCommand function to convert from cluster to regular
const sendCommandFn = options.sendCommand.bind(this)
this.sendCommand = async ({ command }: SendCommandClusterDetails) =>
options.sendCommand(...command)
sendCommandFn(...command)
} else if (!('sendCommand' in options) && 'sendCommandCluster' in options) {
this.sendCommand = options.sendCommandCluster
this.sendCommand = options.sendCommandCluster.bind(this)
} else {
throw new Error(
'rate-limit-redis: Error: options must include either sendCommand or sendCommandCluster (but not both)',
Expand Down
62 changes: 62 additions & 0 deletions test/store-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import MockRedisClient from 'ioredis-mock'
import DefaultExportRedisStore, {
RedisStore,
type RedisReply,
type SendCommandClusterDetails,
} from '../source/index.js'

// The mock redis client to use.
Expand Down Expand Up @@ -345,4 +346,65 @@ describe('redis store test', () => {
// With NEW script we expect a fresh window: hits=1 and ttl reset
expect(result.totalHits).toEqual(1)
})

it('should bind sendCommand to this', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add a similar test for sendCommandCluster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @gamemaker1
test/store-test.ts is missing a mock function -and some tests- for sendCommandCluster. Have you prepared such a method? It seems that sendCommandCluster option should be tested too. Maybe in a different PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gamemaker1 I have pushed a commit for testing sendCommandCluster binding. It's a simple test using the already implemented mock sendCommand. ioredis-mock provides a Cluster mock class -for future use- but it does not follow the specification of a redis cluster command
https://github.com/stipsan/ioredis-mock/?tab=readme-ov-file#clusterexperimental

// A custom sendCommand that verifies `this` is bound to the RedisStore instance
const customSendCommand = async function (
this: CustomRedisStore,
...args: string[]
) {
if (!(this instanceof CustomRedisStore)) {
throw new TypeError('this is not bound to RedisStore instance')
}

// Throw an error on DECR to test disableDecrement provided by the store
if (args[0] === 'DECR' && this.disableDecrement) {
throw new Error('Decrement not supported in this test')
}

return sendCommand(...args)
}

class CustomRedisStore extends RedisStore {
constructor() {
super({
sendCommand: customSendCommand,
})
this.init({ windowMs: 60 } as Options)
}

public get disableDecrement() {
return true
}
}
const store = new CustomRedisStore()
const key = 'test-store'
const { totalHits } = await store.increment(key)
await expect(store.decrement(key)).rejects.toThrow(
'Decrement not supported in this test',
)
expect(totalHits).toEqual(1)
})

it('should bind sendCommandCluster to this', async () => {
// A custom sendCommand that verifies `this` is bound to the RedisStore instance
const customSendCommandCluster = async function (
this: RedisStore,
commandDetails: SendCommandClusterDetails,
) {
if (!(this instanceof RedisStore)) {
throw new TypeError('this is not bound to RedisStore instance')
}

return sendCommand(...commandDetails.command)
}

const store = new RedisStore({
sendCommandCluster: customSendCommandCluster,
})
store.init({ windowMs: 60 } as Options)
const key = 'test-store'
const { totalHits } = await store.increment(key)
expect(totalHits).toEqual(1)
})
})