Skip to content

Commit

Permalink
HAAR-335: upgrade dependencies (#27)
Browse files Browse the repository at this point in the history
* HAAR-335: ⬆️ upgrade dependencies

* HAAR-335: Resolving type conflicts

Co-authored-by: Jon Brighton <[email protected]>
  • Loading branch information
simon-mitchell and brightonsbox authored Apr 20, 2022
1 parent 569f570 commit 4193a88
Show file tree
Hide file tree
Showing 12 changed files with 14,996 additions and 864 deletions.
15,531 changes: 14,755 additions & 776 deletions package-lock.json

Large diffs are not rendered by default.

67 changes: 35 additions & 32 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
"clean": "rm -rf dist build node_modules stylesheets"
},
"engines": {
"node": "^14",
"npm": "^6.4.1"
"node": "^16",
"npm": "^8"
},
"jest": {
"preset": "ts-jest",
Expand Down Expand Up @@ -84,32 +84,35 @@
]
},
"dependencies": {
"@ministryofjustice/frontend": "^1.0.1",
"agentkeepalive": "^4.2.0",
"applicationinsights": "^2.2.1",
"body-parser": "^1.19.1",
"@ministryofjustice/frontend": "^1.3.1",
"agentkeepalive": "^4.2.1",
"applicationinsights": "^2.3.1",
"body-parser": "^1.20.0",
"bunyan": "^1.8.15",
"bunyan-format": "^0.2.1",
"compression": "^1.7.4",
"connect-flash": "^0.1.1",
"connect-redis": "^6.0.0",
"connect-redis": "^6.1.3",
"cookie-session": "^2.0.0",
"csurf": "^1.11.0",
"dotenv": "^16.0.0",
"express": "^4.17.2",
"eslint-plugin-no-only-tests": "^2.6.0",
"express": "^4.17.3",
"express-request-id": "^1.4.1",
"express-session": "^1.17.2",
"govuk-frontend": "^3.14.0",
"govuk-frontend": "^4.0.1",
"helmet": "^5.0.2",
"http-errors": "^2.0.0",
"jquery": "^3.6.0",
"jwt-decode": "^3.1.2",
"nocache": "^3.0.1",
"nocache": "^3.0.3",
"nunjucks": "^3.2.3",
"passport": "^0.5.2",
"passport-oauth2": "^1.6.1",
"redis": "^3.1.2",
"superagent": "^7.1.1"
"prom-client": "^14.0.1",
"redis": "^4.0.6",
"superagent": "^7.1.1",
"url-value-parser": "^2.1.0"
},
"devDependencies": {
"@types/bunyan": "^1.8.8",
Expand All @@ -122,40 +125,40 @@
"@types/express-request-id": "^1.4.3",
"@types/express-session": "^1.17.4",
"@types/http-errors": "^1.8.2",
"@types/jest": "^27.4.0",
"@types/jest": "^27.4.1",
"@types/jsonwebtoken": "^8.5.8",
"@types/node": "^17.0.14",
"@types/node": "^17.0.23",
"@types/nunjucks": "^3.2.1",
"@types/passport": "^1.0.7",
"@types/passport-oauth2": "^1.4.11",
"@types/redis": "^2.8.32",
"@types/redis": "^4.0.10",
"@types/superagent": "^4.1.15",
"@types/supertest": "^2.0.11",
"@typescript-eslint/eslint-plugin": "^5.10.2",
"@typescript-eslint/parser": "^5.10.2",
"concurrently": "^7.0.0",
"cypress": "^9.4.1",
"@types/supertest": "^2.0.12",
"@typescript-eslint/eslint-plugin": "^5.19.0",
"@typescript-eslint/parser": "^5.19.0",
"concurrently": "^7.1.0",
"cypress": "^9.5.4",
"cypress-multi-reporters": "^1.5.0",
"eslint": "^8.8.0",
"eslint": "^8.13.0",
"eslint-config-airbnb-base": "^15.0.0",
"eslint-config-prettier": "^8.3.0",
"eslint-import-resolver-typescript": "^2.5.0",
"eslint-config-prettier": "^8.5.0",
"eslint-import-resolver-typescript": "^2.7.1",
"eslint-plugin-cypress": "^2.12.1",
"eslint-plugin-import": "2.25.4",
"eslint-plugin-import": "2.26.0",
"eslint-plugin-prettier": "^4.0.0",
"husky": "^7.0.4",
"jest": "^27.4.7",
"jest": "^27.5.1",
"jest-html-reporter": "^3.4.2",
"jest-junit": "^13.0.0",
"jest-junit": "^13.1.0",
"jsonwebtoken": "^8.5.1",
"lint-staged": "^12.3.3",
"lint-staged": "^12.3.7",
"mocha-junit-reporter": "^2.0.2",
"nock": "^13.2.2",
"nock": "^13.2.4",
"nodemon": "^2.0.15",
"prettier": "^2.5.1",
"sass": "^1.49.7",
"prettier": "^2.6.2",
"sass": "^1.50.0",
"supertest": "^6.2.2",
"ts-jest": "^27.1.3",
"typescript": "^4.5.5"
"ts-jest": "^27.1.4",
"typescript": "^4.6.3"
}
}
2 changes: 1 addition & 1 deletion server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default {
https: production,
staticResourceCacheDuration: 20,
redis: {
host: process.env.REDIS_HOST,
host: get('REDIS_HOST', 'localhost', requiredInProduction),
port: parseInt(process.env.REDIS_PORT, 10) || 6379,
password: process.env.REDIS_AUTH_TOKEN,
tls_enabled: get('REDIS_TLS_ENABLED', 'false'),
Expand Down
18 changes: 18 additions & 0 deletions server/data/redisClient.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { createClient } from 'redis'

import config from '../config'

export type RedisClient = ReturnType<typeof createClient>

const url =
config.redis.tls_enabled === 'true'
? `rediss://${config.redis.host}:${config.redis.port}`
: `redis://${config.redis.host}:${config.redis.port}`

export const createRedisClient = (legacyMode = false): RedisClient => {
return createClient({
url,
password: config.redis.password,
legacyMode,
})
}
12 changes: 8 additions & 4 deletions server/data/restClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import logger from '../../logger'
import sanitiseError from '../sanitisedError'
import { ApiConfig } from '../config'
import type { UnsanitisedError } from '../sanitisedError'
import { restClientMetricsMiddleware } from './restClientMetricsMiddleware'

interface GetRequest {
path?: string
Expand All @@ -19,7 +20,7 @@ interface PostRequest {
path?: string
headers?: Record<string, string>
responseType?: string
data?: Record<string, unknown> | string
data?: Record<string, unknown>
raw?: boolean
}

Expand All @@ -44,12 +45,13 @@ export default class RestClient {
return this.config.timeout
}

async get({ path = null, query = '', headers = {}, responseType = '', raw = false }: GetRequest): Promise<unknown> {
async get<T>({ path = null, query = '', headers = {}, responseType = '', raw = false }: GetRequest): Promise<T> {
logger.info(`Get using user credentials: calling ${this.name}: ${path} ${query}`)
try {
const result = await superagent
.get(`${this.apiUrl()}${path}`)
.agent(this.agent)
.use(restClientMetricsMiddleware)
.retry(2, (err, res) => {
if (err) logger.info(`Retry handler found API error with ${err.code} ${err.message}`)
return undefined // retry handler only for logging retries, not to influence retry logic
Expand All @@ -75,12 +77,13 @@ export default class RestClient {
data = {},
raw = false,
}: PostRequest = {}): Promise<T> {
logger.info(`Post using user credentials: calling ${this.name}: ${this.apiUrl()}${path}`)
logger.info(`Post using user credentials: calling ${this.name}: ${path}`)
try {
const result = await superagent
.post(`${this.apiUrl()}${path}`)
.send(data)
.agent(this.agent)
.use(restClientMetricsMiddleware)
.retry(2, (err, res) => {
if (err) logger.info(`Retry handler found API error with ${err.code} ${err.message}`)
return undefined // retry handler only for logging retries, not to influence retry logic
Expand All @@ -98,13 +101,14 @@ export default class RestClient {
}
}

async stream({ path = null, headers = {} }: StreamRequest = {}): Promise<unknown> {
async stream({ path = null, headers = {} }: StreamRequest = {}): Promise<Readable> {
logger.info(`Get using user credentials: calling ${this.name}: ${path}`)
return new Promise((resolve, reject) => {
superagent
.get(`${this.apiUrl()}${path}`)
.agent(this.agent)
.auth(this.token, { type: 'bearer' })
.use(restClientMetricsMiddleware)
.retry(2, (err, res) => {
if (err) logger.info(`Retry handler found API error with ${err.code} ${err.message}`)
return undefined // retry handler only for logging retries, not to influence retry logic
Expand Down
72 changes: 72 additions & 0 deletions server/data/restClientMetricsMiddleware.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import superagent from 'superagent'
import {
restClientMetricsMiddleware,
normalizePath,
requestHistogram,
timeoutCounter,
} from './restClientMetricsMiddleware'

describe('restClientMetricsMiddleware', () => {
afterEach(() => {
jest.clearAllMocks()
})

describe('normalizePath', () => {
it('removes the query params from the URL path', () => {
const result = normalizePath('https://httpbin.org/?foo=bar')
expect(result).toBe('/')
})

it('normalises recall ids', () => {
const result = normalizePath(
'https://manage-recalls-dev.hmpps.service.justice.gov.uk/recalls/15e4cccf-cc7b-4946-aa22-a82086735ec2/view-recall'
)
expect(result).toBe('/recalls/#val/view-recall')
})

it('normalises nomis ids', () => {
const result = normalizePath('https://manage-recalls-dev.hmpps.service.justice.gov.uk/prisoner/A7826DY')
expect(result).toBe('/prisoner/#val')
})
})

describe('request timers', () => {
it('times the whole request', async () => {
const requestHistogramLabelsSpy = jest.spyOn(requestHistogram, 'labels').mockReturnValue(requestHistogram)
const requestHistogramStartSpy = jest.spyOn(requestHistogram, 'observe')

let code: number

await superagent
.get('https://httpbin.org/')
.use(restClientMetricsMiddleware)
.set('accept', 'json')
.then(res => {
code = res.statusCode
})

expect(code).toBe(200)
expect(requestHistogramLabelsSpy).toHaveBeenCalledTimes(1)
expect(requestHistogramLabelsSpy).toHaveBeenCalledWith('httpbin.org', 'GET', '/', '200')
expect(requestHistogramStartSpy).toHaveBeenCalledTimes(1)
})
})

describe('timeout errors', () => {
// FIXME: For some reason this test just doesn't work, but the code really does count the timeouts...
it.skip('increment the timeoutCounter', async () => {
const timeoutCounterLabelsSpy = jest.spyOn(timeoutCounter, 'labels').mockReturnValue(timeoutCounter)
const timeoutCounterIncSpy = jest.spyOn(timeoutCounter, 'inc')

await superagent
.get('https://httpbin.org/delay/5') // implements a delay of 5 seconds
.use(restClientMetricsMiddleware)
.set('accept', 'json')
.timeout(100) // Wait 100ms for the server to start sending
.end()

expect(timeoutCounterLabelsSpy).toHaveBeenCalledWith('httpbin.org', 'GET', '/delay/5')
expect(timeoutCounterIncSpy).toHaveBeenCalledTimes(1)
})
})
})
49 changes: 49 additions & 0 deletions server/data/restClientMetricsMiddleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { Socket } from 'net'
import promClient from 'prom-client'
import { Response, SuperAgentRequest } from 'superagent'
import UrlValueParser from 'url-value-parser'

const requestHistogram = new promClient.Histogram({
name: 'http_client_requests_seconds',
help: 'Timings and counts of http client requests',
buckets: [0.5, 0.75, 0.95, 0.99, 1],
labelNames: ['clientName', 'method', 'uri', 'status'],
})

const timeoutCounter = new promClient.Counter({
name: 'http_client_requests_timeout_total',
help: 'Count of http client request timeouts',
labelNames: ['clientName', 'method', 'uri'],
})

function restClientMetricsMiddleware(agent: SuperAgentRequest) {
agent.on('request', ({ req }) => {
const { hostname } = new URL(agent.url)
const normalizedPath = normalizePath(agent.url)
const startTime = Date.now()

req.on('socket', (socket: Socket) => {
socket.on('timeout', () => {
timeoutCounter.labels(hostname, req.method, normalizedPath).inc()
})
})

req.on('response', (res: Response, err: Error) => {
res.on('end', () => {
const responseTime = Date.now() - startTime
requestHistogram.labels(hostname, req.method, normalizedPath, String(res.statusCode)).observe(responseTime)
})
})
})

return agent
}

function normalizePath(url: string) {
const { pathname } = new URL(url)
const urlPathReplacement = '#val'
const urlValueParser = new UrlValueParser({ extraMasks: [/^[A-Z|0-9]+/] })
return urlValueParser.replacePathValues(pathname, urlPathReplacement)
}

export { restClientMetricsMiddleware, normalizePath, requestHistogram, timeoutCounter }
42 changes: 32 additions & 10 deletions server/data/tokenStore.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { RedisClient } from 'redis'
import { RedisClient } from './redisClient'
import TokenStore from './tokenStore'

const redisClient = {
on: jest.fn(),
get: jest.fn(),
set: jest.fn(),
}
on: jest.fn(),
connect: jest.fn(),
isOpen: true,
} as unknown as jest.Mocked<RedisClient>

describe('tokenStore', () => {
let tokenStore: TokenStore
Expand All @@ -18,17 +20,37 @@ describe('tokenStore', () => {
jest.resetAllMocks()
})

it('Can retrieve token', async () => {
redisClient.get.mockImplementation((key, callback) => callback(null, 'token-1'))
describe('get token', () => {
it('Can retrieve token', async () => {
redisClient.get.mockResolvedValue('token-1')

await expect(tokenStore.getToken('user-1')).resolves.toBe('token-1')

expect(redisClient.get).toHaveBeenCalledWith('systemToken:user-1')
})

await expect(tokenStore.getToken('user-1')).resolves.toBe('token-1')
it('Connects when no connection calling getToken', async () => {
;(redisClient as unknown as Record<string, boolean>).isOpen = false

expect(redisClient.get).toHaveBeenCalledWith('user-1', expect.any(Function))
await tokenStore.getToken('user-1')

expect(redisClient.connect).toHaveBeenCalledWith()
})
})

it('Can set token', async () => {
await tokenStore.setToken('user-1', 'token-1', 10)
describe('set token', () => {
it('Can set token', async () => {
await tokenStore.setToken('user-1', 'token-1', 10)

expect(redisClient.set).toHaveBeenCalledWith('systemToken:user-1', 'token-1', { EX: 10 })
})

it('Connects when no connection calling set token', async () => {
;(redisClient as unknown as Record<string, boolean>).isOpen = false

await tokenStore.setToken('user-1', 'token-1', 10)

expect(redisClient.set).toHaveBeenCalledWith('user-1', 'token-1', 'EX', 10, expect.any(Function))
expect(redisClient.connect).toHaveBeenCalledWith()
})
})
})
Loading

0 comments on commit 4193a88

Please sign in to comment.