Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added MemoryStore shutdown method #322

Merged
merged 7 commits into from Sep 4, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 23 additions & 10 deletions source/memory-store.ts
Expand Up @@ -41,28 +41,32 @@ export default class MemoryStore implements Store {
*/
resetTime!: Date

/**
* Reference to the active timer.
*/
interval?: NodeJS.Timer

/**
* Method that initializes the store.
*
* @param options {Options} - The options used to setup the middleware.
*/
init(options: Options): void {
// Get the duration of a window from the options
// Get the duration of a window from the options.
this.windowMs = options.windowMs
// Then calculate the reset time using that
// Then calculate the reset time using that.
this.resetTime = calculateNextResetTime(this.windowMs)

// Initialise the hit counter map
// Initialise the hit counter map.
this.hits = {}

// Reset hit counts for ALL clients every `windowMs` - this will also
// re-calculate the `resetTime`
const interval = setInterval(async () => {
this.interval = setInterval(async () => {
await this.resetAll()
}, this.windowMs)
if (interval.unref) {
interval.unref()
}
// Cleaning up the interval will be taken care of by the `shutdown` method.
if (this.interval.unref) this.interval.unref()
}

/**
Expand Down Expand Up @@ -93,9 +97,8 @@ export default class MemoryStore implements Store {
*/
async decrement(key: string): Promise<void> {
const current = this.hits[key]
if (current) {
this.hits[key] = current - 1
}

if (current) this.hits[key] = current - 1
}

/**
Expand All @@ -118,4 +121,14 @@ export default class MemoryStore implements Store {
this.hits = {}
this.resetTime = calculateNextResetTime(this.windowMs)
}

/**
* Method to stop the timer (if currently running) and prevent any memory
* leaks.
*
* @public
*/
shutdown(): void {
clearInterval(this.interval)
}
}
5 changes: 5 additions & 0 deletions source/types.ts
Expand Up @@ -157,6 +157,11 @@ export interface Store {
* Method to reset everyone's hit counter.
*/
resetAll?: () => Promise<void> | void

/**
* Method to shutdown the store, stop timers, and release all resources.
*/
shutdown?: () => Promise<void> | void
}

/**
Expand Down
16 changes: 11 additions & 5 deletions test/external/imports/default-import/ts-cjs/source/app.ts
Expand Up @@ -8,7 +8,7 @@ import rateLimit, {
IncrementResponse,
} from 'express-rate-limit'

class TestStore implements Store {
export class TestStore implements Store {
hits: Record<string, number> = {}

async increment(key: string): Promise<IncrementResponse> {
Expand All @@ -28,19 +28,25 @@ class TestStore implements Store {
async resetKey(key: string): Promise<void> {
delete this.hits[key]
}

async shutdown(): Promise<void> {
console.log('Shutdown successful')
}
}

const app = createServer()
export const app = createServer()

export const store = Math.floor(Math.random() * 2)
? new TestStore()
: new MemoryStore()

app.use(
rateLimit({
max: 2,
legacyHeaders: false,
standardHeaders: true,
store: Math.floor(Math.random() * 2) ? new TestStore() : new MemoryStore(),
store,
}),
)

app.get('/', (request, response) => response.send('Hello!'))

export default app
@@ -1,7 +1,7 @@
// /source/index.ts
// Run the server

import app from './app'
import { app } from './app'

app.listen(8080, () =>
console.log('Make a GET request to http://localhost:8080!'),
Expand Down
@@ -1,12 +1,19 @@
// /test/server-test.ts
// Tests the server's rate limiting middleware

import { jest } from '@jest/globals'
import { agent as request } from 'supertest'

import app from '../source/app.js'
import { app, store, TestStore } from '../source/app.js'

test('rate limiting middleware', async () => {
jest.spyOn(global.console, 'log').mockImplementation(() => {})

await request(app).get('/').expect(200)
await request(app).get('/').expect(200)
await request(app).get('/').expect(429)

await store.shutdown()
if (store instanceof TestStore)
expect(console.log).toHaveBeenCalledWith('Shutdown successful')
Comment on lines +17 to +18
Copy link
Member

@nfriedly nfriedly Sep 1, 2022

Choose a reason for hiding this comment

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

All of these console.log tests don't really seem necessary to me, but I know it was @gamemaker1's request and I guess it's not really hurting anything either. 🤷

})
16 changes: 11 additions & 5 deletions test/external/imports/default-import/ts-esm/source/app.ts
Expand Up @@ -8,7 +8,7 @@ import rateLimit, {
IncrementResponse,
} from 'express-rate-limit'

class TestStore implements Store {
export class TestStore implements Store {
hits: Record<string, number> = {}

async increment(key: string): Promise<IncrementResponse> {
Expand All @@ -28,19 +28,25 @@ class TestStore implements Store {
async resetKey(key: string): Promise<void> {
delete this.hits[key]
}

async shutdown(): Promise<void> {
console.log('Shutdown successful')
}
}

const app = createServer()
export const app = createServer()

export const store = Math.floor(Math.random() * 2)
? new TestStore()
: new MemoryStore()

app.use(
rateLimit({
max: 2,
legacyHeaders: false,
standardHeaders: true,
store: Math.floor(Math.random() * 2) ? new TestStore() : new MemoryStore(),
store,
}),
)

app.get('/', (request, response) => response.send('Hello!'))

export default app
@@ -1,7 +1,7 @@
// /source/index.ts
// Run the server

import app from './app.js'
import { app } from './app.js'

app.listen(8080, () =>
console.log('Make a GET request to http://localhost:8080!'),
Expand Down
@@ -1,12 +1,19 @@
// /test/server-test.ts
// Tests the server's rate limiting middleware

import { jest } from '@jest/globals'
import { agent as request } from 'supertest'

import app from '../source/app.js'
import { app, store, TestStore } from '../source/app.js'

test('rate limiting middleware', async () => {
jest.spyOn(global.console, 'log').mockImplementation(() => {})

await request(app).get('/').expect(200)
await request(app).get('/').expect(200)
await request(app).get('/').expect(429)

await store.shutdown()
if (store instanceof TestStore)
expect(console.log).toHaveBeenCalledWith('Shutdown successful')
})
16 changes: 11 additions & 5 deletions test/external/imports/named-import/ts-cjs/source/app.ts
Expand Up @@ -9,7 +9,7 @@ import {
IncrementResponse,
} from 'express-rate-limit'

class TestStore implements Store {
export class TestStore implements Store {
hits: Record<string, number> = {}

async increment(key: string): Promise<IncrementResponse> {
Expand All @@ -29,19 +29,25 @@ class TestStore implements Store {
async resetKey(key: string): Promise<void> {
delete this.hits[key]
}

async shutdown(): Promise<void> {
console.log('Shutdown successful')
}
}

const app = createServer()
export const app = createServer()

export const store = Math.floor(Math.random() * 2)
? new TestStore()
: new MemoryStore()

app.use(
rateLimit({
max: 2,
legacyHeaders: false,
standardHeaders: true,
store: Math.floor(Math.random() * 2) ? new TestStore() : new MemoryStore(),
store,
}),
)

app.get('/', (request, response) => response.send('Hello!'))

export default app
@@ -1,12 +1,19 @@
// /test/server-test.ts
// Tests the server's rate limiting middleware

import { jest } from '@jest/globals'
import { agent as request } from 'supertest'

import app from '../source/app.js'
import { app, store, TestStore } from '../source/app.js'

test('rate limiting middleware', async () => {
jest.spyOn(global.console, 'log').mockImplementation(() => {})

await request(app).get('/').expect(200)
await request(app).get('/').expect(200)
await request(app).get('/').expect(429)

await store.shutdown()
if (store instanceof TestStore)
expect(console.log).toHaveBeenCalledWith('Shutdown successful')
})
16 changes: 11 additions & 5 deletions test/external/imports/named-import/ts-esm/source/app.ts
Expand Up @@ -9,7 +9,7 @@ import {
IncrementResponse,
} from 'express-rate-limit'

class TestStore implements Store {
export class TestStore implements Store {
hits: Record<string, number> = {}

async increment(key: string): Promise<IncrementResponse> {
Expand All @@ -29,19 +29,25 @@ class TestStore implements Store {
async resetKey(key: string): Promise<void> {
delete this.hits[key]
}

async shutdown(): Promise<void> {
console.log('Shutdown successful')
}
}

const app = createServer()
export const app = createServer()

export const store = Math.floor(Math.random() * 2)
? new TestStore()
: new MemoryStore()

app.use(
rateLimit({
max: 2,
legacyHeaders: false,
standardHeaders: true,
store: Math.floor(Math.random() * 2) ? new TestStore() : new MemoryStore(),
store,
}),
)

app.get('/', (request, response) => response.send('Hello!'))

export default app
@@ -1,12 +1,19 @@
// /test/server-test.ts
// Tests the server's rate limiting middleware

import { jest } from '@jest/globals'
import { agent as request } from 'supertest'

import app from '../source/app.js'
import { app, store, TestStore } from '../source/app.js'

test('rate limiting middleware', async () => {
jest.spyOn(global.console, 'log').mockImplementation(() => {})

await request(app).get('/').expect(200)
await request(app).get('/').expect(200)
await request(app).get('/').expect(429)

await store.shutdown()
if (store instanceof TestStore)
expect(console.log).toHaveBeenCalledWith('Shutdown successful')
})
9 changes: 9 additions & 0 deletions test/library/memory-store-test.ts
Expand Up @@ -9,6 +9,7 @@ import MemoryStore from '../../source/memory-store.js'
describe('memory store test', () => {
beforeEach(() => {
jest.useFakeTimers()
jest.spyOn(global, 'clearInterval')
})
afterEach(() => {
jest.useRealTimers()
Expand Down Expand Up @@ -76,6 +77,14 @@ describe('memory store test', () => {
expect(totalHitsTwo).toEqual(1)
})

it('clears the timer when `shutdown` is called', async () => {
const store = new MemoryStore()
store.init({ windowMs: -1 } as Options)
expect(store.interval).toBeDefined()
store.shutdown()
expect(clearInterval).toHaveBeenCalledWith(store.interval)
})

describe('reset time', () => {
beforeEach(() => {
jest.useFakeTimers()
Expand Down