Upgrading dependencies #66
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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, | ||
| }) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,38 +1,29 @@ | ||
| import type { RedisClient } from './redisClient' | ||
|
|
||
| import logger from '../../logger' | ||
|
|
||
| export default class TokenStore { | ||
| private readonly prefix = 'systemToken:' | ||
|
|
||
| constructor(private readonly client: RedisClient) { | ||
| client.on('error', error => { | ||
| logger.error(error, `Redis error`) | ||
| }) | ||
| } | ||
|
|
||
| private async ensureConnected() { | ||
| if (!this.client.isOpen) { | ||
| await this.client.connect() | ||
| } | ||
| } | ||
|
|
||
| public async setToken(key: string, token: string, durationSeconds: number): Promise<void> { | ||
| await this.ensureConnected() | ||
| await this.client.set(`${this.prefix}${key}`, token, { EX: durationSeconds }) | ||
| } | ||
|
|
||
| public async getToken(key: string): Promise<string> { | ||
| await this.ensureConnected() | ||
| return this.client.get(`${this.prefix}${key}`) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,20 @@ | ||
| import session from 'express-session' | ||
| import connectRedis, { Client } from 'connect-redis' | ||
| import addRequestId from 'express-request-id' | ||
| import express, { Router } from 'express' | ||
| import { createRedisClient } from '../data/redisClient' | ||
| import config from '../config' | ||
|
|
||
| const RedisStore = connectRedis(session) | ||
|
|
||
| export default function setUpWebSession(): Router { | ||
| const client = createRedisClient(true) | ||
| client.connect() | ||
|
|
||
| const router = express.Router() | ||
| router.use( | ||
| session({ | ||
| store: new RedisStore({ client: client as unknown as Client }), | ||
|
There was a problem hiding this comment. bit gnarly - waiting on connect-redis to update to new version of redis client: |
||
| cookie: { secure: config.https, sameSite: 'lax', maxAge: config.session.expiryMinutes * 60 * 1000 }, | ||
| secret: config.session.secret, | ||
| resave: false, // redis implements touch so shouldn't need this | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reduce logging on errors related to this issue
Also need to add new load path for
.as govuk frontend now explicitly reference govuk node modules as a dependency related to this: ministryofjustice/moj-frontend#147There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the second one? Or do we only import bits of govuk? ...and a separate issue, what point are we ditching IE8 support? I don't believe it's in our supported browsers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need all 3 - or we have to start referring to things with full paths.
The new load path is used only to bodge around the change in the moj-frontend and how they refer to assets from the gov uk frontend library.
I think we still have users in probation who use IE8, we did the last time I checked..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation @andrewrlee 👍