From d5df07afcc857df85dc4b696858156eb6fbb211e Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Tue, 2 Sep 2025 13:14:01 +0100 Subject: [PATCH 01/16] Persist user answers on Continue --- src/config/config.js | 8 ++ .../session-cache/backend-catbox-client.js | 100 +++++++++++++++++ .../backend-catbox-client.test.js | 97 +++++++++++++++++ .../common/helpers/start-server.test.js | 4 +- .../helpers/state/fetch-saved-state-helper.js | 17 ++- .../state/fetch-saved-state-helper.test.js | 101 +++++++++--------- .../helpers/state/get-cache-key-helper.js | 28 +++++ .../helpers/state/persist-state-helper.js | 16 +-- .../state/persist-state-helper.test.js | 71 +++++------- src/server/index.js | 14 ++- 10 files changed, 347 insertions(+), 109 deletions(-) create mode 100644 src/server/common/helpers/session-cache/backend-catbox-client.js create mode 100644 src/server/common/helpers/session-cache/backend-catbox-client.test.js diff --git a/src/config/config.js b/src/config/config.js index b025c201b..659289016 100644 --- a/src/config/config.js +++ b/src/config/config.js @@ -243,6 +243,14 @@ export const config = convict({ sensitive: true } }, + backend: { + name: { + doc: 'Persistent backend cache used for save-and-continue and long-term storage', + format: String, + default: 'backendSession', + env: 'BACKEND_CACHE_NAME' + } + }, cookie: { ttl: { doc: 'Session cookie ttl', diff --git a/src/server/common/helpers/session-cache/backend-catbox-client.js b/src/server/common/helpers/session-cache/backend-catbox-client.js new file mode 100644 index 000000000..4109c2dcd --- /dev/null +++ b/src/server/common/helpers/session-cache/backend-catbox-client.js @@ -0,0 +1,100 @@ +import { createLogger } from '../logging/logger.js' +import { fetchSavedStateFromApi } from '../state/fetch-saved-state-helper.js' +import { persistStateToApi } from '../state/persist-state-helper.js' + +const ONE_YEAR_MS = 365 * 24 * 60 * 60 * 1000 + +const logger = createLogger() + +// Custom Catbox client that uses backend API for storage +export class BackendCatboxClient { + /** + * Validate a Catbox cache segment name. + * Must be a non-empty string with letters, numbers, underscores, or dashes. + * + * @param {string} segment + * @throws {Error} If segment is invalid + */ + validateSegmentName(segment) { + if (!segment || typeof segment !== 'string' || !/^[a-zA-Z0-9_-]+$/.test(segment)) { + throw new Error('Invalid segment name: ' + segment) + } + return null + } + + /** + * Override the default get method to add logging + * @param {string} key + * @returns {Promise|null>} + */ + async get(key) { + logger.debug(`Cache GET - Key: ${JSON.stringify(key)}`) + const state = await fetchSavedStateFromApi(key) + logger.debug('Fetched State from API:', state) + + return { + item: state ?? null, + stored: Date.now(), // Current timestamp so we never consider it stale + ttl: ONE_YEAR_MS // Set a long TTL since backend manages actual expiry + } + } + + /** + * Override the default set method to add logging + * @param {string} key + * @param {T} value + * @param {number} [ttl] + * @returns {Promise} + */ + async set(key, value, ttl) { + logger.debug(`Cache SET - Value: ${JSON.stringify(value)}, Key: ${JSON.stringify(key)}, TTL: ${ttl}`) + return persistStateToApi(value, key) + } + + /** + * Override the default drop method to add logging + * @param {string} key + * @returns {Promise} + */ + async drop(key) { + // optional: no-op + logger.debug(`Cache DROP - Key: ${JSON.stringify(key)}`) + } + + /** + * Start the cache client. + * + * Called by Hapi/Catbox during server startup. + * Can be a no-op if no initialization is required. + * + * @returns {Promise} + */ + async start() { + logger.debug('BackendCatboxClient start() called') + } + + /** + * Stop the cache client. + * + * Called by Hapi/Catbox during server shutdown. + * Can be a no-op if no cleanup is required. + * + * @returns {Promise} + */ + async stop() { + logger.debug('BackendCatboxClient stop() called') + } + + /** + * Checks whether the backend cache client is ready to handle operations. + * + * Catbox requires this method to determine if the client can be used. + * Since this provider does not maintain a persistent connection, + * it simply returns `true` to indicate readiness. + * + * @returns {boolean} Always returns `true` to satisfy Catbox interface. + */ + isReady() { + return true + } +} diff --git a/src/server/common/helpers/session-cache/backend-catbox-client.test.js b/src/server/common/helpers/session-cache/backend-catbox-client.test.js new file mode 100644 index 000000000..2a8bbce4e --- /dev/null +++ b/src/server/common/helpers/session-cache/backend-catbox-client.test.js @@ -0,0 +1,97 @@ +import { BackendCatboxClient } from '~/src/server/common/helpers/session-cache/backend-catbox-client.js' +import * as fetchModule from '../state/fetch-saved-state-helper.js' +import * as persistModule from '../state/persist-state-helper.js' + +jest.mock('../state/fetch-saved-state-helper.js') +jest.mock('../state/persist-state-helper.js') + +describe('BackendCatboxClient', () => { + let client + + beforeEach(() => { + client = new BackendCatboxClient() + jest.clearAllMocks() + jest.spyOn(console, 'log').mockImplementation(() => {}) + }) + + afterAll(() => { + jest.restoreAllMocks() + }) + + describe('validateSegmentName', () => { + it('throws on invalid segment', () => { + const invalidSegments = [null, '', 123, 'invalid!segment', {}, []] + invalidSegments.forEach((segment) => { + expect(() => client.validateSegmentName(segment)).toThrow() + }) + }) + + it('does not throw on valid segment', () => { + const validSegments = ['abc', 'ABC_123', 'valid-segment'] + validSegments.forEach((segment) => { + expect(client.validateSegmentName(segment)).toBeNull() + }) + }) + }) + + describe('get', () => { + it('calls fetchSavedStateFromApi and returns proper object', async () => { + const key = { userId: 'u1' } + const state = { foo: 'bar' } + fetchModule.fetchSavedStateFromApi.mockResolvedValue(state) + + const result = await client.get(key) + + expect(fetchModule.fetchSavedStateFromApi).toHaveBeenCalledWith(key) + expect(result).toHaveProperty('item', state) + expect(result).toHaveProperty('stored') + expect(result).toHaveProperty('ttl', 365 * 24 * 60 * 60 * 1000) + }) + + it('returns item=null when fetchSavedStateFromApi returns null', async () => { + const key = { userId: 'u1' } + fetchModule.fetchSavedStateFromApi.mockResolvedValue(null) + + const result = await client.get(key) + + expect(result.item).toBeNull() + }) + }) + + describe('set', () => { + it('calls persistStateToApi with value and key', async () => { + const key = { userId: 'u1' } + const value = { foo: 'bar' } + persistModule.persistStateToApi.mockResolvedValue() + + await client.set(key, value, 1234) + + expect(persistModule.persistStateToApi).toHaveBeenCalledWith(value, key) + }) + }) + + describe('drop', () => { + it('logs drop call and does not throw', async () => { + const key = { userId: 'u1' } + await expect(client.drop(key)).resolves.toBeUndefined() + }) + }) + + describe('start', () => { + it('logs start call and does not throw', async () => { + await expect(client.start()).resolves.toBeUndefined() + }) + }) + + describe('stop', () => { + it('logs stop call and does not throw', async () => { + await expect(client.stop()).resolves.toBeUndefined() + }) + }) + + describe('isReady', () => { + it('always returns true', () => { + expect(client.isReady()).toBe(true) + }) + }) +}) diff --git a/src/server/common/helpers/start-server.test.js b/src/server/common/helpers/start-server.test.js index fe2d7b3ed..83902be71 100644 --- a/src/server/common/helpers/start-server.test.js +++ b/src/server/common/helpers/start-server.test.js @@ -2,6 +2,7 @@ import { vi } from 'vitest' import Wreck from '@hapi/wreck' const mockLoggerInfo = vi.fn() const mockLoggerError = vi.fn() +const mockLoggerDebug = jest.fn() const mockHapiLoggerInfo = vi.fn() const mockHapiLoggerError = vi.fn() @@ -21,7 +22,8 @@ vi.mock('~/src/server/common/helpers/logging/logger.js', async () => { const { mockLoggerFactoryWithCustomMethods } = await import('~/src/__mocks__') return mockLoggerFactoryWithCustomMethods({ info: (...args) => mockLoggerInfo(...args), - error: (...args) => mockLoggerError(...args) + error: (...args) => mockLoggerError(...args), + debug: (...args) => mockLoggerDebug(...args) }) }) diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.js b/src/server/common/helpers/state/fetch-saved-state-helper.js index 317ff6972..ac333529a 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.js @@ -1,19 +1,25 @@ import { statusCodes } from '~/src/server/common/constants/status-codes.js' import 'dotenv/config' import { config } from '~/src/config/config.js' -import { getCacheKey } from './get-cache-key-helper.js' +import { parseSessionKey } from './get-cache-key-helper.js' import { createApiHeaders } from './backend-auth-helper.js' +import { createLogger } from '../logging/logger.js' const GRANTS_UI_BACKEND_ENDPOINT = config.get('session.cache.apiEndpoint') -export async function fetchSavedStateFromApi(request) { +const logger = createLogger() + +export async function fetchSavedStateFromApi(key) { if (!GRANTS_UI_BACKEND_ENDPOINT?.length) { return null } - const { userId, organisationId, grantId } = getCacheKey(request) + + const { userId, organisationId, grantId } = parseSessionKey(key.id) let json = null try { + logger.debug(`Fetching saved state from backend for identity: ${key}`) + const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) url.searchParams.set('userId', userId) url.searchParams.set('businessId', organisationId) @@ -26,6 +32,7 @@ export async function fetchSavedStateFromApi(request) { if (!response.ok) { if (response.status === statusCodes.notFound) { + logger.debug(`No state found in backend for identity: ${key}`) return null } throw new Error(`Failed to fetch saved state: ${response.status}`) @@ -34,11 +41,11 @@ export async function fetchSavedStateFromApi(request) { json = await response.json() if (!json || typeof json !== 'object') { - request.logger.warn(['fetch-saved-state'], 'Unexpected or empty state format', { json }) + logger.warn(`fetch-saved-state: Unexpected or empty state format: ${json}`) return null } } catch (err) { - request.logger.error(['fetch-saved-state'], 'Failed to fetch saved state from API', err) + logger.error(`fetch-saved-state: Failed to fetch saved state from API: ${err}`) return null } diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.test.js b/src/server/common/helpers/state/fetch-saved-state-helper.test.js index 8ebb0f508..681fbc4f6 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.test.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.test.js @@ -3,43 +3,51 @@ import { mockRequestWithIdentity } from './mock-request-with-identity.test-helpe import { MOCK_STATE_DATA, HTTP_STATUS, - TEST_USER_IDS, ERROR_MESSAGES, - LOG_MESSAGES, createMockConfig, createMockConfigWithoutEndpoint } from './test-helpers/auth-test-helpers.js' import { mockRequestLogger } from '~/src/__mocks__/logger-mocks.js' -const LOG_TAGS = { - FETCH_SAVED_STATE: 'fetch-saved-state' -} - global.fetch = vi.fn() -let fetchSavedStateFromApi - -const mockRequest = mockRequestWithIdentity({ params: { slug: TEST_USER_IDS.GRANT_ID } }) -const mockRequestWithLogger = { - ...mockRequest, - logger: mockRequestLogger() +const mockLogger = { + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn() } -const successfulResponse = { - ok: true, - json: () => MOCK_STATE_DATA.DEFAULT -} +vi.mock('../logging/logger.js', () => ({ + createLogger: () => mockLogger +})) -const createFailedResponse = (status, statusText = 'Error') => ({ - ok: false, - status, - statusText, - json: () => { - throw new Error(ERROR_MESSAGES.NO_CONTENT) - } -}) +// Mock parseSessionKey +vi.mock('./get-cache-key-helper.js', () => ({ + parseSessionKey: vi.fn(() => ({ + userId: 'user-1', + businessId: 'business-1', + grantId: 'grant-1' + })) +})) + +let fetchSavedStateFromApi describe('fetchSavedStateFromApi', () => { + const key = { id: 'user-1:business-1:grant-1' } + + const createSuccessfulResponse = (data = MOCK_STATE_DATA.DEFAULT) => ({ + ok: true, + json: () => data + }) + const createFailedResponse = (status, statusText = 'Error') => ({ + ok: false, + status, + statusText, + json: () => { + throw new Error(ERROR_MESSAGES.NO_CONTENT) + } + }) + describe('With backend configured correctly', () => { beforeEach(async () => { vi.clearAllMocks() @@ -54,18 +62,19 @@ describe('fetchSavedStateFromApi', () => { }) it('returns state when response is valid', async () => { - fetch.mockResolvedValue(successfulResponse) + fetch.mockResolvedValue(createSuccessfulResponse()) - const result = await fetchSavedStateFromApi(mockRequest) + const result = await fetchSavedStateFromApi(key) expect(result).toHaveProperty('state') expect(fetch).toHaveBeenCalledTimes(1) + expect(mockLogger.debug).toHaveBeenCalled() }) it('includes authorization header in fetch request', async () => { - fetch.mockResolvedValue(successfulResponse) + fetch.mockResolvedValue(createSuccessfulResponse()) - await fetchSavedStateFromApi(mockRequest) + await fetchSavedStateFromApi(key) expect(fetch).toHaveBeenCalledTimes(1) expect(fetch).toHaveBeenCalledWith( @@ -85,50 +94,40 @@ describe('fetchSavedStateFromApi', () => { }) it('returns null on 404', async () => { - fetch.mockResolvedValue(createFailedResponse(HTTP_STATUS.NOT_FOUND, ERROR_MESSAGES.NOT_FOUND)) + fetch.mockResolvedValue(createFailedResponse(HTTP_STATUS.NOT_FOUND)) - const result = await fetchSavedStateFromApi(mockRequest) + const result = await fetchSavedStateFromApi(key) expect(result).toBeNull() - expect(fetch).toHaveBeenCalledTimes(1) + expect(mockLogger.debug).toHaveBeenCalledWith(expect.stringContaining('No state found')) }) it('returns null on non-200 (not 404)', async () => { - fetch.mockResolvedValue( - createFailedResponse(HTTP_STATUS.INTERNAL_SERVER_ERROR, ERROR_MESSAGES.INTERNAL_SERVER_ERROR) - ) + fetch.mockResolvedValue(createFailedResponse(HTTP_STATUS.INTERNAL_SERVER_ERROR)) - const result = await fetchSavedStateFromApi(mockRequest) + const result = await fetchSavedStateFromApi(key) expect(result).toBeNull() - expect(fetch).toHaveBeenCalledTimes(1) + expect(mockLogger.error).toHaveBeenCalled() }) - it('returns null when response JSON is invalid or missing state', async () => { - fetch.mockResolvedValue({ ok: true, json: () => 123 }) + it('returns null when response JSON is invalid', async () => { + fetch.mockResolvedValue(createSuccessfulResponse(123)) - const result = await fetchSavedStateFromApi(mockRequestWithLogger) + const result = await fetchSavedStateFromApi(key) expect(result).toBeNull() - expect(mockRequestWithLogger.logger.warn).toHaveBeenCalledWith( - [LOG_TAGS.FETCH_SAVED_STATE], - LOG_MESSAGES.UNEXPECTED_STATE_FORMAT, - expect.any(Object) - ) + expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('Unexpected or empty state format')) }) it('returns null and logs error on fetch failure', async () => { const networkError = new Error(ERROR_MESSAGES.NETWORK_ERROR) fetch.mockRejectedValue(networkError) - const result = await fetchSavedStateFromApi(mockRequestWithLogger) + const result = await fetchSavedStateFromApi(key) expect(result).toBeNull() - expect(mockRequestWithLogger.logger.error).toHaveBeenCalledWith( - [LOG_TAGS.FETCH_SAVED_STATE], - LOG_MESSAGES.FETCH_FAILED, - networkError - ) + expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('fetch-saved-state')) }) }) @@ -146,7 +145,7 @@ describe('fetchSavedStateFromApi', () => { }) it('returns null when GRANTS_UI_BACKEND_ENDPOINT is not configured', async () => { - const result = await fetchSavedStateFromApi(mockRequest) + const result = await fetchSavedStateFromApi(key) expect(result).toBeNull() expect(fetch).not.toHaveBeenCalled() diff --git a/src/server/common/helpers/state/get-cache-key-helper.js b/src/server/common/helpers/state/get-cache-key-helper.js index a85aef6a9..c8316a462 100644 --- a/src/server/common/helpers/state/get-cache-key-helper.js +++ b/src/server/common/helpers/state/get-cache-key-helper.js @@ -14,6 +14,13 @@ const outputLog = (request, message) => { }) } +/** + * Generates a cache key from a Hapi request by extracting user, business, and grant identifiers. + * + * @param {import('@hapi/hapi').Request} request - The Hapi request object containing authentication credentials and route parameters. + * @returns {{ userId: string, businessId: string, grantId: string }} An object containing identifiers to be used as a cache key. + * @throws {Error} If authentication credentials, user ID, business relationship, or grant ID are missing or malformed. + */ export const getCacheKey = (request) => { const credentials = request.auth?.credentials @@ -41,3 +48,24 @@ export const getCacheKey = (request) => { } return { userId, organisationId, grantId } } + +/** + * Parses a session key into its components. + * + * @param {string} sessionKey - Colon-separated key (userId:businessId:grantId) + * @returns {{ userId: string, organisationId: string, grantId: string }} Parsed values + * @throws {Error} If sessionKey is invalid or missing parts + */ +export function parseSessionKey(sessionKey) { + if (!sessionKey || typeof sessionKey !== 'string') { + throw new Error('Invalid session key: must be a non-empty string') + } + + const [userId, organisationId, grantId] = sessionKey.split(':') + + if (!userId || !organisationId || !grantId) { + throw new Error(`Invalid session key format: ${sessionKey}`) + } + + return { userId, organisationId, grantId } +} diff --git a/src/server/common/helpers/state/persist-state-helper.js b/src/server/common/helpers/state/persist-state-helper.js index 51a5f13f4..c2a4e97e7 100644 --- a/src/server/common/helpers/state/persist-state-helper.js +++ b/src/server/common/helpers/state/persist-state-helper.js @@ -1,19 +1,23 @@ import 'dotenv/config' import { config } from '~/src/config/config.js' -import { getCacheKey } from './get-cache-key-helper.js' +import { parseSessionKey } from './get-cache-key-helper.js' import { createApiHeaders } from './backend-auth-helper.js' +import { createLogger } from '../logging/logger.js' const GRANTS_UI_BACKEND_ENDPOINT = config.get('session.cache.apiEndpoint') -export async function persistStateToApi(state, request) { +const logger = createLogger() + +export async function persistStateToApi(state, key) { if (!GRANTS_UI_BACKEND_ENDPOINT?.length) { return } - const { userId, organisationId, grantId } = getCacheKey(request) const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) - request.logger.info(`Persisting state to backend for identity: ${userId}:${organisationId}:${grantId}`) + const { userId, organisationId, grantId } = parseSessionKey(key.id) + + logger.debug(`Persisting state to backend for identity: ${key.userId}:${key.organisationId}:${key.grantId}`) try { const response = await fetch(url.href, { @@ -29,10 +33,10 @@ export async function persistStateToApi(state, request) { }) if (!response.ok) { - request.logger.error(`Failed to persist state to API: ${response.status} - ${response.statusText}`) + logger.error(`Failed to persist state to API: ${response.status} - ${response.statusText}`) } } catch (err) { - request.logger.error(`Failed to persist state to API: ${err.message}`) + logger.error(`Failed to persist state to API: ${err.message}`) // TODO: See TGC-781 // throw err diff --git a/src/server/common/helpers/state/persist-state-helper.test.js b/src/server/common/helpers/state/persist-state-helper.test.js index 670806226..945f8da60 100644 --- a/src/server/common/helpers/state/persist-state-helper.test.js +++ b/src/server/common/helpers/state/persist-state-helper.test.js @@ -1,5 +1,4 @@ import { vi } from 'vitest' -import { mockRequestWithIdentity } from './mock-request-with-identity.test-helper.js' import { MOCK_STATE_DATA, HTTP_STATUS, @@ -10,13 +9,20 @@ import { createMockConfigWithoutEndpoint } from './test-helpers/auth-test-helpers.js' import { mockRequestLogger } from '~/src/__mocks__/logger-mocks.js' +import { mockRequestWithIdentity } from './mock-request-with-identity.test-helper.js' const GRANT_VERSION = 1 -const mockGetCacheKey = vi.fn() - -vi.mock('~/src/server/common/helpers/state/get-cache-key-helper.js', () => ({ - getCacheKey: mockGetCacheKey +const mockParseSessionKey = vi.fn() +vi.mock('./get-cache-key-helper.js', () => ({ + parseSessionKey: mockParseSessionKey +})) +const mockLogger = { + debug: vi.fn(), + error: vi.fn() +} +vi.mock('../logging/logger.js', () => ({ + createLogger: () => mockLogger })) global.fetch = vi.fn() @@ -24,35 +30,23 @@ global.fetch = vi.fn() let persistStateToApi describe('persistStateToApi', () => { - const createMockRequest = () => mockRequestWithIdentity({ params: { slug: TEST_USER_IDS.GRANT_ID } }) - - const createMockRequestWithLogger = () => { - const request = createMockRequest() - request.logger = mockRequestLogger() - return request - } + const key = { id: 'user-1:business-1:grant-1', userId: 'user-1', businessId: 'business-1', grantId: 'grant-1' } + const testState = MOCK_STATE_DATA.WITH_STEP - const createTestState = () => MOCK_STATE_DATA.WITH_STEP - - const setupMockCacheKey = () => { - mockGetCacheKey.mockReturnValue({ + beforeEach(() => { + mockParseSessionKey.mockReturnValue({ userId: TEST_USER_IDS.DEFAULT, organisationId: TEST_USER_IDS.ORGANISATION_ID, grantId: TEST_USER_IDS.GRANT_ID }) - } - - beforeEach(() => { - setupMockCacheKey() }) describe('With backend configured correctly', () => { beforeEach(async () => { - vi.resetAllMocks() + vi.clearAllMocks() vi.doMock('~/src/config/config.js', createMockConfig) const helper = await import('~/src/server/common/helpers/state/persist-state-helper.js') persistStateToApi = helper.persistStateToApi - setupMockCacheKey() vi.clearAllMocks() }) @@ -77,16 +71,13 @@ describe('persistStateToApi', () => { it('persists state successfully when response is ok', async () => { fetch.mockResolvedValue(createSuccessfulFetchResponse()) - const request = createMockRequest() - const testState = createTestState() - - await persistStateToApi(testState, request) + await persistStateToApi(testState, key) const expectedBody = JSON.stringify({ userId: TEST_USER_IDS.DEFAULT, businessId: TEST_USER_IDS.ORGANISATION_ID, grantId: TEST_USER_IDS.GRANT_ID, - grantVersion: GRANT_VERSION, // TODO: Update when support for same grant versioning is implemented + grantVersion: GRANT_VERSION, state: testState }) @@ -103,8 +94,8 @@ describe('persistStateToApi', () => { }) ) - expect(request.logger.info).toHaveBeenCalledWith( - `Persisting state to backend for identity: ${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}` + expect(mockLogger.debug).toHaveBeenCalledWith( + `Persisting state to backend for identity: ${key.userId}:${key.businessId}:${key.grantId}` ) }) @@ -112,13 +103,10 @@ describe('persistStateToApi', () => { const failedResponse = createFailedFetchResponse() fetch.mockResolvedValue(failedResponse) - const request = createMockRequestWithLogger() - const testState = createTestState() - - await persistStateToApi(testState, request) + await persistStateToApi(testState, key) expect(fetch).toHaveBeenCalledTimes(1) - expect(request.logger.error).toHaveBeenCalledWith( + expect(mockLogger.error).toHaveBeenCalledWith( `${LOG_MESSAGES.PERSIST_FAILED}: ${failedResponse.status} - ${failedResponse.statusText}` ) }) @@ -127,24 +115,20 @@ describe('persistStateToApi', () => { const networkError = new Error(ERROR_MESSAGES.NETWORK_ERROR) fetch.mockRejectedValue(networkError) - const request = createMockRequestWithLogger() - const testState = createTestState() - - await persistStateToApi(testState, request) + await persistStateToApi(testState, key) expect(fetch).toHaveBeenCalledTimes(1) - expect(request.logger.error).toHaveBeenCalledWith(`${LOG_MESSAGES.PERSIST_FAILED}: ${networkError.message}`) + expect(mockLogger.error).toHaveBeenCalledWith(`${LOG_MESSAGES.PERSIST_FAILED}: ${networkError.message}`) }) }) describe('With backend not configured', () => { beforeEach(async () => { - vi.resetAllMocks() + vi.clearAllMocks() vi.resetModules() vi.doMock('~/src/config/config.js', createMockConfigWithoutEndpoint) const helper = await import('~/src/server/common/helpers/state/persist-state-helper.js') persistStateToApi = helper.persistStateToApi - setupMockCacheKey() vi.clearAllMocks() }) @@ -153,8 +137,9 @@ describe('persistStateToApi', () => { }) it('should return early when backend endpoint is not configured', async () => { - const request = createMockRequestWithLogger() - const testState = createTestState() + const request = mockRequestWithIdentity({ params: { slug: TEST_USER_IDS.GRANT_ID } }) + request.logger = mockRequestLogger() + const testState = () => MOCK_STATE_DATA.WITH_STEP await persistStateToApi(testState, request) diff --git a/src/server/index.js b/src/server/index.js index c3fcdb57d..631d98948 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -47,9 +47,11 @@ import { persistStateToApi } from './common/helpers/state/persist-state-helper.j import { contentSecurityPolicy } from '~/src/server/common/helpers/csp.js' import RemoveActionPageController from './land-grants/controllers/remove-action-page.controller.js' import { router } from './router.js' +import { formsAuthCallback } from '~/src/server/auth/forms-engine-plugin-auth-helpers.js' import SectionEndController from './section-end/section-end.controller.js' const SESSION_CACHE_NAME = 'session.cache.name' +const SESSION_BACKEND_NAME = 'session.backend.name' const getViewPaths = () => { const serverDir = path.resolve(path.dirname(fileURLToPath(import.meta.url))) @@ -106,6 +108,10 @@ const createHapiServer = () => { { name: config.get(SESSION_CACHE_NAME), engine: getCacheEngine(/** @type {Engine} */ (config.get('session.cache.engine'))) + }, + { + name: config.get(SESSION_BACKEND_NAME), + engine: new BackendCatboxClient() } ], state: { @@ -119,18 +125,20 @@ const registerFormsPlugin = async (server, prefix = '') => { plugin, options: { ...(prefix && { routes: { prefix } }), - cacheName: config.get(SESSION_CACHE_NAME), + cacheName: config.get(SESSION_BACKEND_NAME), baseUrl: config.get('baseUrl'), saveAndReturn: { keyGenerator: (request) => { const { userId, organisationId, grantId } = getCacheKey(request) return `${userId}:${organisationId}:${grantId}` }, + // dummy: returns null, does nothing sessionHydrator: async (request) => { - return fetchSavedStateFromApi(request) + return null }, + // dummy: returns null, does nothing sessionPersister: async (state, request) => { - return persistStateToApi(state, request) + return null } }, onRequest: formsAuthCallback, From 1201749c58a7edceb8b210e66db3494cdb6b74fc Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Tue, 2 Sep 2025 14:00:26 +0100 Subject: [PATCH 02/16] Remove the ttl and stored property --- .../common/helpers/session-cache/backend-catbox-client.js | 6 +----- .../helpers/session-cache/backend-catbox-client.test.js | 2 -- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/server/common/helpers/session-cache/backend-catbox-client.js b/src/server/common/helpers/session-cache/backend-catbox-client.js index 4109c2dcd..b8b5126a0 100644 --- a/src/server/common/helpers/session-cache/backend-catbox-client.js +++ b/src/server/common/helpers/session-cache/backend-catbox-client.js @@ -2,8 +2,6 @@ import { createLogger } from '../logging/logger.js' import { fetchSavedStateFromApi } from '../state/fetch-saved-state-helper.js' import { persistStateToApi } from '../state/persist-state-helper.js' -const ONE_YEAR_MS = 365 * 24 * 60 * 60 * 1000 - const logger = createLogger() // Custom Catbox client that uses backend API for storage @@ -33,9 +31,7 @@ export class BackendCatboxClient { logger.debug('Fetched State from API:', state) return { - item: state ?? null, - stored: Date.now(), // Current timestamp so we never consider it stale - ttl: ONE_YEAR_MS // Set a long TTL since backend manages actual expiry + item: state ?? null } } diff --git a/src/server/common/helpers/session-cache/backend-catbox-client.test.js b/src/server/common/helpers/session-cache/backend-catbox-client.test.js index 2a8bbce4e..57c7ac144 100644 --- a/src/server/common/helpers/session-cache/backend-catbox-client.test.js +++ b/src/server/common/helpers/session-cache/backend-catbox-client.test.js @@ -44,8 +44,6 @@ describe('BackendCatboxClient', () => { expect(fetchModule.fetchSavedStateFromApi).toHaveBeenCalledWith(key) expect(result).toHaveProperty('item', state) - expect(result).toHaveProperty('stored') - expect(result).toHaveProperty('ttl', 365 * 24 * 60 * 60 * 1000) }) it('returns item=null when fetchSavedStateFromApi returns null', async () => { From f05abe1deae8d4298e5d807b58d9136f899065e7 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Tue, 2 Sep 2025 15:20:41 +0100 Subject: [PATCH 03/16] Add test coverage --- .../state/get-cache-key-helper.test.js | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/server/common/helpers/state/get-cache-key-helper.test.js b/src/server/common/helpers/state/get-cache-key-helper.test.js index aef380a63..3ae463b1e 100644 --- a/src/server/common/helpers/state/get-cache-key-helper.test.js +++ b/src/server/common/helpers/state/get-cache-key-helper.test.js @@ -1,4 +1,4 @@ -import { getCacheKey } from './get-cache-key-helper.js' +import { getCacheKey, parseSessionKey } from './get-cache-key-helper.js' describe('getCacheKey', () => { it('returns userId, organisationId, and grantId when all are present', () => { @@ -92,3 +92,30 @@ describe('getCacheKey', () => { expect(() => getCacheKey(request)).toThrow('Missing grantId') }) }) + +describe('parseSessionKey', () => { + it('parses a valid session key into its components', () => { + const key = 'user123:business456:grant789' + const result = parseSessionKey(key) + + expect(result).toEqual({ + userId: 'user123', + businessId: 'business456', + grantId: 'grant789' + }) + }) + + it('throws error for empty string', () => { + expect(() => parseSessionKey('')).toThrow('Invalid session key') + }) + + it('throws error for non-string input', () => { + expect(() => parseSessionKey(null)).toThrow('Invalid session key') + expect(() => parseSessionKey(123)).toThrow('Invalid session key') + }) + + it('throws error for missing parts', () => { + expect(() => parseSessionKey('user123:business456')).toThrow('Invalid session key format') + expect(() => parseSessionKey('user123')).toThrow('Invalid session key format') + }) +}) From 7de13a51ede23f325b3c5967e6a9d7d8317aece8 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Tue, 2 Sep 2025 15:22:56 +0100 Subject: [PATCH 04/16] Change params to unused --- src/server/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/index.js b/src/server/index.js index 631d98948..3643855a5 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -133,11 +133,11 @@ const registerFormsPlugin = async (server, prefix = '') => { return `${userId}:${organisationId}:${grantId}` }, // dummy: returns null, does nothing - sessionHydrator: async (request) => { + sessionHydrator: async (_request) => { return null }, // dummy: returns null, does nothing - sessionPersister: async (state, request) => { + sessionPersister: async (_state, _request) => { return null } }, From 81fa645b0926616ba00f640ab390eea81030a461 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Wed, 3 Sep 2025 14:54:57 +0100 Subject: [PATCH 05/16] Make logging consistent --- .../helpers/session-cache/backend-catbox-client.js | 14 ++++++++------ .../helpers/state/fetch-saved-state-helper.js | 4 ++-- .../common/helpers/state/persist-state-helper.js | 2 +- .../helpers/state/persist-state-helper.test.js | 8 +++++--- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/server/common/helpers/session-cache/backend-catbox-client.js b/src/server/common/helpers/session-cache/backend-catbox-client.js index b8b5126a0..320efb069 100644 --- a/src/server/common/helpers/session-cache/backend-catbox-client.js +++ b/src/server/common/helpers/session-cache/backend-catbox-client.js @@ -26,9 +26,9 @@ export class BackendCatboxClient { * @returns {Promise|null>} */ async get(key) { - logger.debug(`Cache GET - Key: ${JSON.stringify(key)}`) + logger.debug(`backend-catbox-client: Cache GET - Key: ${JSON.stringify(key)}`) const state = await fetchSavedStateFromApi(key) - logger.debug('Fetched State from API:', state) + logger.debug(`backend-catbox-client: Fetched State from API: ${JSON.stringify(state)}`) return { item: state ?? null @@ -43,7 +43,9 @@ export class BackendCatboxClient { * @returns {Promise} */ async set(key, value, ttl) { - logger.debug(`Cache SET - Value: ${JSON.stringify(value)}, Key: ${JSON.stringify(key)}, TTL: ${ttl}`) + logger.debug( + `backend-catbox-client: Cache SET - Value: ${JSON.stringify(value)}, Key: ${JSON.stringify(key)}, TTL: ${ttl}` + ) return persistStateToApi(value, key) } @@ -54,7 +56,7 @@ export class BackendCatboxClient { */ async drop(key) { // optional: no-op - logger.debug(`Cache DROP - Key: ${JSON.stringify(key)}`) + logger.debug(`backend-catbox-client: Cache DROP - Key: ${JSON.stringify(key)}`) } /** @@ -66,7 +68,7 @@ export class BackendCatboxClient { * @returns {Promise} */ async start() { - logger.debug('BackendCatboxClient start() called') + logger.debug('backend-catbox-client: BackendCatboxClient start() called') } /** @@ -78,7 +80,7 @@ export class BackendCatboxClient { * @returns {Promise} */ async stop() { - logger.debug('BackendCatboxClient stop() called') + logger.debug('backend-catbox-client: BackendCatboxClient stop() called') } /** diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.js b/src/server/common/helpers/state/fetch-saved-state-helper.js index ac333529a..7f1b301a4 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.js @@ -18,7 +18,7 @@ export async function fetchSavedStateFromApi(key) { let json = null try { - logger.debug(`Fetching saved state from backend for identity: ${key}`) + logger.debug(`fetch-saved-state: Fetching saved state from backend for identity: ${JSON.stringify(key)}`) const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) url.searchParams.set('userId', userId) @@ -32,7 +32,7 @@ export async function fetchSavedStateFromApi(key) { if (!response.ok) { if (response.status === statusCodes.notFound) { - logger.debug(`No state found in backend for identity: ${key}`) + logger.debug(`fetch-saved-state: No state found in backend for identity: ${JSON.stringify(key)}`) return null } throw new Error(`Failed to fetch saved state: ${response.status}`) diff --git a/src/server/common/helpers/state/persist-state-helper.js b/src/server/common/helpers/state/persist-state-helper.js index c2a4e97e7..3522dc205 100644 --- a/src/server/common/helpers/state/persist-state-helper.js +++ b/src/server/common/helpers/state/persist-state-helper.js @@ -36,7 +36,7 @@ export async function persistStateToApi(state, key) { logger.error(`Failed to persist state to API: ${response.status} - ${response.statusText}`) } } catch (err) { - logger.error(`Failed to persist state to API: ${err.message}`) + logger.error(`persist-state: Failed to persist state to API: ${err.message}`) // TODO: See TGC-781 // throw err diff --git a/src/server/common/helpers/state/persist-state-helper.test.js b/src/server/common/helpers/state/persist-state-helper.test.js index 945f8da60..2e4678807 100644 --- a/src/server/common/helpers/state/persist-state-helper.test.js +++ b/src/server/common/helpers/state/persist-state-helper.test.js @@ -30,7 +30,7 @@ global.fetch = vi.fn() let persistStateToApi describe('persistStateToApi', () => { - const key = { id: 'user-1:business-1:grant-1', userId: 'user-1', businessId: 'business-1', grantId: 'grant-1' } + const key = { id: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` } const testState = MOCK_STATE_DATA.WITH_STEP beforeEach(() => { @@ -95,7 +95,7 @@ describe('persistStateToApi', () => { ) expect(mockLogger.debug).toHaveBeenCalledWith( - `Persisting state to backend for identity: ${key.userId}:${key.businessId}:${key.grantId}` + `persist-state: Persisting state to backend for identity: ${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` ) }) @@ -118,7 +118,9 @@ describe('persistStateToApi', () => { await persistStateToApi(testState, key) expect(fetch).toHaveBeenCalledTimes(1) - expect(mockLogger.error).toHaveBeenCalledWith(`${LOG_MESSAGES.PERSIST_FAILED}: ${networkError.message}`) + expect(mockLogger.error).toHaveBeenCalledWith( + `persist-state: ${LOG_MESSAGES.PERSIST_FAILED}: ${networkError.message}` + ) }) }) From 9173b0e0604208a80470c0601bfbc0fc9e3bba0f Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Wed, 3 Sep 2025 17:43:41 +0100 Subject: [PATCH 06/16] Adjust persist and fetch state logging with the LogCodes logging strategy --- .../common/helpers/logging/log-codes.js | 8 +- .../common/helpers/logging/log-codes.test.js | 3 +- .../helpers/state/fetch-saved-state-helper.js | 29 +++++-- .../state/fetch-saved-state-helper.test.js | 77 ++++++++++++++++--- .../helpers/state/persist-state-helper.js | 26 +++++-- .../state/persist-state-helper.test.js | 54 +++++++++---- src/server/status-service.js | 29 +++++++ 7 files changed, 183 insertions(+), 43 deletions(-) create mode 100644 src/server/status-service.js diff --git a/src/server/common/helpers/logging/log-codes.js b/src/server/common/helpers/logging/log-codes.js index 32211d6f1..6497f681b 100644 --- a/src/server/common/helpers/logging/log-codes.js +++ b/src/server/common/helpers/logging/log-codes.js @@ -227,9 +227,15 @@ export const LogCodes = { messageFunc: (messageOptions) => `External API call to ${messageOptions.endpoint} for user=${messageOptions.userId || 'unknown'}` }, + EXTERNAL_API_CALL_DEBUG: { + level: 'debug', + messageFunc: (messageOptions) => + `External API call to ${messageOptions.endpoint} for identity=${messageOptions.identity || 'unknown'} - stateSummary=${JSON.stringify(messageOptions.stateSummary || {})}` + }, EXTERNAL_API_ERROR: { level: 'error', - messageFunc: (messageOptions) => `External API error for ${messageOptions.endpoint}: ${messageOptions.error}` + messageFunc: (messageOptions) => + `External API error for ${messageOptions.endpoint} for identity: ${messageOptions.identity || 'unknown'} - error: ${messageOptions.error}` } } } diff --git a/src/server/common/helpers/logging/log-codes.test.js b/src/server/common/helpers/logging/log-codes.test.js index bca6d4827..fa66af22c 100644 --- a/src/server/common/helpers/logging/log-codes.test.js +++ b/src/server/common/helpers/logging/log-codes.test.js @@ -401,9 +401,10 @@ describe('LogCodes', () => { expect( logCode.messageFunc({ endpoint: '/api/grants', + identity: 'test', error: 'Connection failed' }) - ).toBe('External API error for /api/grants: Connection failed') + ).toBe('External API error for /api/grants for identity: test - error: Connection failed') }) }) diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.js b/src/server/common/helpers/state/fetch-saved-state-helper.js index 7f1b301a4..f398b7133 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.js @@ -3,12 +3,10 @@ import 'dotenv/config' import { config } from '~/src/config/config.js' import { parseSessionKey } from './get-cache-key-helper.js' import { createApiHeaders } from './backend-auth-helper.js' -import { createLogger } from '../logging/logger.js' +import { log, LogCodes } from '../logging/log.js' const GRANTS_UI_BACKEND_ENDPOINT = config.get('session.cache.apiEndpoint') -const logger = createLogger() - export async function fetchSavedStateFromApi(key) { if (!GRANTS_UI_BACKEND_ENDPOINT?.length) { return null @@ -17,10 +15,13 @@ export async function fetchSavedStateFromApi(key) { const { userId, organisationId, grantId } = parseSessionKey(key.id) let json = null + const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) try { - logger.debug(`fetch-saved-state: Fetching saved state from backend for identity: ${JSON.stringify(key)}`) + log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { + endpoint: url.href, + identity: key.id + }) - const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) url.searchParams.set('userId', userId) url.searchParams.set('businessId', organisationId) url.searchParams.set('grantId', grantId) @@ -32,7 +33,11 @@ export async function fetchSavedStateFromApi(key) { if (!response.ok) { if (response.status === statusCodes.notFound) { - logger.debug(`fetch-saved-state: No state found in backend for identity: ${JSON.stringify(key)}`) + log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { + endpoint: url.href, + identity: key.id, + stateSummary: 'No state found in backend' + }) return null } throw new Error(`Failed to fetch saved state: ${response.status}`) @@ -41,11 +46,19 @@ export async function fetchSavedStateFromApi(key) { json = await response.json() if (!json || typeof json !== 'object') { - logger.warn(`fetch-saved-state: Unexpected or empty state format: ${json}`) + log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + endpoint: url.href, + identity: key.id, + error: `Unexpected or empty state format: ${json}` + }) return null } } catch (err) { - logger.error(`fetch-saved-state: Failed to fetch saved state from API: ${err}`) + log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + endpoint: url.href, + identity: key.id, + error: err.message + }) return null } diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.test.js b/src/server/common/helpers/state/fetch-saved-state-helper.test.js index 681fbc4f6..c67cb95f2 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.test.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.test.js @@ -3,6 +3,7 @@ import { mockRequestWithIdentity } from './mock-request-with-identity.test-helpe import { MOCK_STATE_DATA, HTTP_STATUS, + TEST_USER_IDS, ERROR_MESSAGES, createMockConfig, createMockConfigWithoutEndpoint @@ -22,18 +23,17 @@ vi.mock('../logging/logger.js', () => ({ })) // Mock parseSessionKey -vi.mock('./get-cache-key-helper.js', () => ({ - parseSessionKey: vi.fn(() => ({ - userId: 'user-1', - businessId: 'business-1', - grantId: 'grant-1' - })) +const mockParseSessionKey = jest.fn() +jest.mock('./get-cache-key-helper.js', () => ({ + parseSessionKey: mockParseSessionKey })) let fetchSavedStateFromApi +let log +let LogCodes describe('fetchSavedStateFromApi', () => { - const key = { id: 'user-1:business-1:grant-1' } + const key = { id: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` } const createSuccessfulResponse = (data = MOCK_STATE_DATA.DEFAULT) => ({ ok: true, @@ -48,13 +48,32 @@ describe('fetchSavedStateFromApi', () => { } }) + beforeEach(() => { + mockParseSessionKey.mockReturnValue({ + userId: TEST_USER_IDS.DEFAULT, + businessId: TEST_USER_IDS.BUSINESS_ID, + grantId: TEST_USER_IDS.GRANT_ID + }) + }) + describe('With backend configured correctly', () => { beforeEach(async () => { vi.clearAllMocks() vi.resetModules() vi.doMock('~/src/config/config.js', createMockConfig) + vi.doMock('../logging/log.js', () => ({ + log: jest.fn(), + LogCodes: { + SYSTEM: { + EXTERNAL_API_CALL_DEBUG: { level: 'debug', messageFunc: jest.fn() }, + EXTERNAL_API_ERROR: { level: 'error', messageFunc: jest.fn() } + } + } + })) const helper = await import('~/src/server/common/helpers/state/fetch-saved-state-helper.js?t=' + Date.now()) fetchSavedStateFromApi = helper.fetchSavedStateFromApi + log = (await import('../logging/log.js')).log + LogCodes = (await import('../logging/log.js')).LogCodes }) afterEach(() => { @@ -68,7 +87,13 @@ describe('fetchSavedStateFromApi', () => { expect(result).toHaveProperty('state') expect(fetch).toHaveBeenCalledTimes(1) - expect(mockLogger.debug).toHaveBeenCalled() + expect(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, + expect.objectContaining({ + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` + }) + ) }) it('includes authorization header in fetch request', async () => { @@ -99,7 +124,14 @@ describe('fetchSavedStateFromApi', () => { const result = await fetchSavedStateFromApi(key) expect(result).toBeNull() - expect(mockLogger.debug).toHaveBeenCalledWith(expect.stringContaining('No state found')) + expect(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, + expect.objectContaining({ + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + stateSummary: 'No state found in backend' + }) + ) }) it('returns null on non-200 (not 404)', async () => { @@ -108,7 +140,14 @@ describe('fetchSavedStateFromApi', () => { const result = await fetchSavedStateFromApi(key) expect(result).toBeNull() - expect(mockLogger.error).toHaveBeenCalled() + expect(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_ERROR, + expect.objectContaining({ + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + error: 'Failed to fetch saved state: 500' + }) + ) }) it('returns null when response JSON is invalid', async () => { @@ -117,7 +156,14 @@ describe('fetchSavedStateFromApi', () => { const result = await fetchSavedStateFromApi(key) expect(result).toBeNull() - expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('Unexpected or empty state format')) + expect(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_ERROR, + expect.objectContaining({ + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + error: 'Unexpected or empty state format: 123' + }) + ) }) it('returns null and logs error on fetch failure', async () => { @@ -127,7 +173,14 @@ describe('fetchSavedStateFromApi', () => { const result = await fetchSavedStateFromApi(key) expect(result).toBeNull() - expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('fetch-saved-state')) + expect(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_ERROR, + expect.objectContaining({ + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + error: 'Network error' + }) + ) }) }) diff --git a/src/server/common/helpers/state/persist-state-helper.js b/src/server/common/helpers/state/persist-state-helper.js index 3522dc205..0177f1877 100644 --- a/src/server/common/helpers/state/persist-state-helper.js +++ b/src/server/common/helpers/state/persist-state-helper.js @@ -2,12 +2,10 @@ import 'dotenv/config' import { config } from '~/src/config/config.js' import { parseSessionKey } from './get-cache-key-helper.js' import { createApiHeaders } from './backend-auth-helper.js' -import { createLogger } from '../logging/logger.js' +import { log, LogCodes } from '../logging/log.js' const GRANTS_UI_BACKEND_ENDPOINT = config.get('session.cache.apiEndpoint') -const logger = createLogger() - export async function persistStateToApi(state, key) { if (!GRANTS_UI_BACKEND_ENDPOINT?.length) { return @@ -17,7 +15,14 @@ export async function persistStateToApi(state, key) { const { userId, organisationId, grantId } = parseSessionKey(key.id) - logger.debug(`Persisting state to backend for identity: ${key.userId}:${key.organisationId}:${key.grantId}`) + log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { + endpoint: url.href, + identity: key.id, + stateSummary: { + hasReference: Boolean(state?.$$__referenceNumber), + keyCount: Object.keys(state || {}).length + } + }) try { const response = await fetch(url.href, { @@ -33,11 +38,18 @@ export async function persistStateToApi(state, key) { }) if (!response.ok) { - logger.error(`Failed to persist state to API: ${response.status} - ${response.statusText}`) + log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + endpoint: url.href, + identity: key.id, + error: `${response.status} - ${response.statusText}` + }) } } catch (err) { - logger.error(`persist-state: Failed to persist state to API: ${err.message}`) - + log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + endpoint: url.href, + identity: key.id, + error: err.message + }) // TODO: See TGC-781 // throw err } diff --git a/src/server/common/helpers/state/persist-state-helper.test.js b/src/server/common/helpers/state/persist-state-helper.test.js index 2e4678807..bad0d6358 100644 --- a/src/server/common/helpers/state/persist-state-helper.test.js +++ b/src/server/common/helpers/state/persist-state-helper.test.js @@ -17,17 +17,22 @@ const mockParseSessionKey = vi.fn() vi.mock('./get-cache-key-helper.js', () => ({ parseSessionKey: mockParseSessionKey })) -const mockLogger = { - debug: vi.fn(), - error: vi.fn() -} -vi.mock('../logging/logger.js', () => ({ - createLogger: () => mockLogger -})) global.fetch = vi.fn() +vi.doMock('../logging/log.js', () => ({ + log: vi.fn(), + LogCodes: { + SYSTEM: { + EXTERNAL_API_CALL_DEBUG: { level: 'debug', messageFunc: vi.fn() }, + EXTERNAL_API_ERROR: { level: 'error', messageFunc: vi.fn() } + } + } +})) + let persistStateToApi +let log +let LogCodes describe('persistStateToApi', () => { const key = { id: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` } @@ -43,10 +48,13 @@ describe('persistStateToApi', () => { describe('With backend configured correctly', () => { beforeEach(async () => { - vi.clearAllMocks() + vi.resetModules() vi.doMock('~/src/config/config.js', createMockConfig) const helper = await import('~/src/server/common/helpers/state/persist-state-helper.js') persistStateToApi = helper.persistStateToApi + const logModule = await import('../logging/log.js') + log = logModule.log + LogCodes = logModule.LogCodes vi.clearAllMocks() }) @@ -94,8 +102,16 @@ describe('persistStateToApi', () => { }) ) - expect(mockLogger.debug).toHaveBeenCalledWith( - `persist-state: Persisting state to backend for identity: ${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` + expect(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, + expect.objectContaining({ + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + stateSummary: { + hasReference: Boolean(testState?.$$__referenceNumber), + keyCount: Object.keys(testState).length + } + }) ) }) @@ -106,8 +122,13 @@ describe('persistStateToApi', () => { await persistStateToApi(testState, key) expect(fetch).toHaveBeenCalledTimes(1) - expect(mockLogger.error).toHaveBeenCalledWith( - `${LOG_MESSAGES.PERSIST_FAILED}: ${failedResponse.status} - ${failedResponse.statusText}` + expect(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_ERROR, + expect.objectContaining({ + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + error: `${failedResponse.status} - ${failedResponse.statusText}` + }) ) }) @@ -118,8 +139,13 @@ describe('persistStateToApi', () => { await persistStateToApi(testState, key) expect(fetch).toHaveBeenCalledTimes(1) - expect(mockLogger.error).toHaveBeenCalledWith( - `persist-state: ${LOG_MESSAGES.PERSIST_FAILED}: ${networkError.message}` + expect(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_ERROR, + expect.objectContaining({ + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + error: networkError.message + }) ) }) }) diff --git a/src/server/status-service.js b/src/server/status-service.js new file mode 100644 index 000000000..6a6c9c433 --- /dev/null +++ b/src/server/status-service.js @@ -0,0 +1,29 @@ +// config of fake applications by ID for local testing +const fakeStatuses = { + 'app-123': { + crn: '1234567890', + sbi: '987654321', + status: 'SUBMITTED', + applicationReference: 'APP-12345' + }, + 'app-456': { + crn: '1234567890', + sbi: '987654321', + status: 'REJECTED', + applicationReference: 'APP-45678' + } +} + +export async function getStatusFromGAS(applicationId) { + // simulate async network delay + await new Promise((resolve) => setTimeout(resolve, 50)) + + return ( + fakeStatuses[applicationId] || { + crn: 'unknown', + sbi: 'unknown', + status: 'UNKNOWN', + applicationReference: applicationId + } + ) +} From ee6c3be96adc55a33f3d976322442d4b4dd6b8c5 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Tue, 9 Sep 2025 12:28:13 +0100 Subject: [PATCH 07/16] Fix tests --- .../session-cache/backend-catbox-client.test.js | 10 +++++----- src/server/common/helpers/start-server.test.js | 2 +- .../helpers/state/fetch-saved-state-helper.test.js | 12 +++++------- .../helpers/state/persist-state-helper.test.js | 1 - 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/server/common/helpers/session-cache/backend-catbox-client.test.js b/src/server/common/helpers/session-cache/backend-catbox-client.test.js index 57c7ac144..2cf6ad4d6 100644 --- a/src/server/common/helpers/session-cache/backend-catbox-client.test.js +++ b/src/server/common/helpers/session-cache/backend-catbox-client.test.js @@ -2,20 +2,20 @@ import { BackendCatboxClient } from '~/src/server/common/helpers/session-cache/b import * as fetchModule from '../state/fetch-saved-state-helper.js' import * as persistModule from '../state/persist-state-helper.js' -jest.mock('../state/fetch-saved-state-helper.js') -jest.mock('../state/persist-state-helper.js') +vi.mock('../state/fetch-saved-state-helper.js') +vi.mock('../state/persist-state-helper.js') describe('BackendCatboxClient', () => { let client beforeEach(() => { client = new BackendCatboxClient() - jest.clearAllMocks() - jest.spyOn(console, 'log').mockImplementation(() => {}) + vi.clearAllMocks() + vi.spyOn(console, 'log').mockImplementation(() => {}) }) afterAll(() => { - jest.restoreAllMocks() + vi.restoreAllMocks() }) describe('validateSegmentName', () => { diff --git a/src/server/common/helpers/start-server.test.js b/src/server/common/helpers/start-server.test.js index 83902be71..902f2e225 100644 --- a/src/server/common/helpers/start-server.test.js +++ b/src/server/common/helpers/start-server.test.js @@ -2,7 +2,7 @@ import { vi } from 'vitest' import Wreck from '@hapi/wreck' const mockLoggerInfo = vi.fn() const mockLoggerError = vi.fn() -const mockLoggerDebug = jest.fn() +const mockLoggerDebug = vi.fn() const mockHapiLoggerInfo = vi.fn() const mockHapiLoggerError = vi.fn() diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.test.js b/src/server/common/helpers/state/fetch-saved-state-helper.test.js index c67cb95f2..2d782f566 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.test.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.test.js @@ -1,5 +1,4 @@ import { vi } from 'vitest' -import { mockRequestWithIdentity } from './mock-request-with-identity.test-helper.js' import { MOCK_STATE_DATA, HTTP_STATUS, @@ -8,7 +7,6 @@ import { createMockConfig, createMockConfigWithoutEndpoint } from './test-helpers/auth-test-helpers.js' -import { mockRequestLogger } from '~/src/__mocks__/logger-mocks.js' global.fetch = vi.fn() @@ -23,8 +21,8 @@ vi.mock('../logging/logger.js', () => ({ })) // Mock parseSessionKey -const mockParseSessionKey = jest.fn() -jest.mock('./get-cache-key-helper.js', () => ({ +const mockParseSessionKey = vi.fn() +vi.mock('./get-cache-key-helper.js', () => ({ parseSessionKey: mockParseSessionKey })) @@ -62,11 +60,11 @@ describe('fetchSavedStateFromApi', () => { vi.resetModules() vi.doMock('~/src/config/config.js', createMockConfig) vi.doMock('../logging/log.js', () => ({ - log: jest.fn(), + log: vi.fn(), LogCodes: { SYSTEM: { - EXTERNAL_API_CALL_DEBUG: { level: 'debug', messageFunc: jest.fn() }, - EXTERNAL_API_ERROR: { level: 'error', messageFunc: jest.fn() } + EXTERNAL_API_CALL_DEBUG: { level: 'debug', messageFunc: vi.fn() }, + EXTERNAL_API_ERROR: { level: 'error', messageFunc: vi.fn() } } } })) diff --git a/src/server/common/helpers/state/persist-state-helper.test.js b/src/server/common/helpers/state/persist-state-helper.test.js index bad0d6358..93576242f 100644 --- a/src/server/common/helpers/state/persist-state-helper.test.js +++ b/src/server/common/helpers/state/persist-state-helper.test.js @@ -4,7 +4,6 @@ import { HTTP_STATUS, TEST_USER_IDS, ERROR_MESSAGES, - LOG_MESSAGES, createMockConfig, createMockConfigWithoutEndpoint } from './test-helpers/auth-test-helpers.js' From cb9bc865762de7ffbb5a77c4034e7d08e84eb286 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Tue, 9 Sep 2025 12:35:40 +0100 Subject: [PATCH 08/16] Update to latest beta 3.0.0 --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index fe482faa7..bf5dd1cd2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "license": "OGL-UK-3.0", "dependencies": { "@babel/runtime": "^7.28.3", - "@defra/forms-engine-plugin": "2.1.10", + "@defra/forms-engine-plugin": "^3.0.0", "@defra/hapi-tracing": "^1.28.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/bell": "^13.1.0", @@ -1931,9 +1931,9 @@ "license": "MIT" }, "node_modules/@defra/forms-engine-plugin": { - "version": "2.1.10", - "resolved": "https://registry.npmjs.org/@defra/forms-engine-plugin/-/forms-engine-plugin-2.1.10.tgz", - "integrity": "sha512-Qmlf/UNlhv/KhANf4XamRbSllUWIH6wyTVS/4xg6kiezXQYaU3+KljzXJlmXR72cYenP/Qjk3F/HEPqks2N6og==", + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@defra/forms-engine-plugin/-/forms-engine-plugin-3.0.0.tgz", + "integrity": "sha512-LQGCiGfOcZFtpLZNW0FXiVbYb1nc5gQJS26IGTcrLE0oma6iEkVYmw3upGmZ/IMU5YZtu/3tJGusQK4/ASK4Ig==", "hasInstallScript": true, "license": "SEE LICENSE IN LICENSE", "dependencies": { diff --git a/package.json b/package.json index abe94a51d..12f7db3a3 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ }, "dependencies": { "@babel/runtime": "^7.28.3", - "@defra/forms-engine-plugin": "2.1.10", + "@defra/forms-engine-plugin": "^3.0.0", "@defra/hapi-tracing": "^1.28.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/bell": "^13.1.0", From 3e8f2b312d460fc6bcec1425a8521369b6284613 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Wed, 10 Sep 2025 14:57:11 +0100 Subject: [PATCH 09/16] Migrate from backend-catbox-client catbox provider to state persistence service that extends DXT's CacheService --- .../session-cache/backend-catbox-client.js | 98 ------------------- .../backend-catbox-client.test.js | 95 ------------------ .../helpers/state/fetch-saved-state-helper.js | 10 +- .../state/fetch-saved-state-helper.test.js | 2 +- .../helpers/state/persist-state-helper.js | 8 +- .../state/persist-state-helper.test.js | 2 +- .../state-persistence.service.js | 98 +++++++++++++++++++ .../state-persistence.service.test.js | 80 +++++++++++++++ src/server/index.js | 23 +---- 9 files changed, 192 insertions(+), 224 deletions(-) delete mode 100644 src/server/common/helpers/session-cache/backend-catbox-client.js delete mode 100644 src/server/common/helpers/session-cache/backend-catbox-client.test.js create mode 100644 src/server/common/services/state-persistence/state-persistence.service.js create mode 100644 src/server/common/services/state-persistence/state-persistence.service.test.js diff --git a/src/server/common/helpers/session-cache/backend-catbox-client.js b/src/server/common/helpers/session-cache/backend-catbox-client.js deleted file mode 100644 index 320efb069..000000000 --- a/src/server/common/helpers/session-cache/backend-catbox-client.js +++ /dev/null @@ -1,98 +0,0 @@ -import { createLogger } from '../logging/logger.js' -import { fetchSavedStateFromApi } from '../state/fetch-saved-state-helper.js' -import { persistStateToApi } from '../state/persist-state-helper.js' - -const logger = createLogger() - -// Custom Catbox client that uses backend API for storage -export class BackendCatboxClient { - /** - * Validate a Catbox cache segment name. - * Must be a non-empty string with letters, numbers, underscores, or dashes. - * - * @param {string} segment - * @throws {Error} If segment is invalid - */ - validateSegmentName(segment) { - if (!segment || typeof segment !== 'string' || !/^[a-zA-Z0-9_-]+$/.test(segment)) { - throw new Error('Invalid segment name: ' + segment) - } - return null - } - - /** - * Override the default get method to add logging - * @param {string} key - * @returns {Promise|null>} - */ - async get(key) { - logger.debug(`backend-catbox-client: Cache GET - Key: ${JSON.stringify(key)}`) - const state = await fetchSavedStateFromApi(key) - logger.debug(`backend-catbox-client: Fetched State from API: ${JSON.stringify(state)}`) - - return { - item: state ?? null - } - } - - /** - * Override the default set method to add logging - * @param {string} key - * @param {T} value - * @param {number} [ttl] - * @returns {Promise} - */ - async set(key, value, ttl) { - logger.debug( - `backend-catbox-client: Cache SET - Value: ${JSON.stringify(value)}, Key: ${JSON.stringify(key)}, TTL: ${ttl}` - ) - return persistStateToApi(value, key) - } - - /** - * Override the default drop method to add logging - * @param {string} key - * @returns {Promise} - */ - async drop(key) { - // optional: no-op - logger.debug(`backend-catbox-client: Cache DROP - Key: ${JSON.stringify(key)}`) - } - - /** - * Start the cache client. - * - * Called by Hapi/Catbox during server startup. - * Can be a no-op if no initialization is required. - * - * @returns {Promise} - */ - async start() { - logger.debug('backend-catbox-client: BackendCatboxClient start() called') - } - - /** - * Stop the cache client. - * - * Called by Hapi/Catbox during server shutdown. - * Can be a no-op if no cleanup is required. - * - * @returns {Promise} - */ - async stop() { - logger.debug('backend-catbox-client: BackendCatboxClient stop() called') - } - - /** - * Checks whether the backend cache client is ready to handle operations. - * - * Catbox requires this method to determine if the client can be used. - * Since this provider does not maintain a persistent connection, - * it simply returns `true` to indicate readiness. - * - * @returns {boolean} Always returns `true` to satisfy Catbox interface. - */ - isReady() { - return true - } -} diff --git a/src/server/common/helpers/session-cache/backend-catbox-client.test.js b/src/server/common/helpers/session-cache/backend-catbox-client.test.js deleted file mode 100644 index 2cf6ad4d6..000000000 --- a/src/server/common/helpers/session-cache/backend-catbox-client.test.js +++ /dev/null @@ -1,95 +0,0 @@ -import { BackendCatboxClient } from '~/src/server/common/helpers/session-cache/backend-catbox-client.js' -import * as fetchModule from '../state/fetch-saved-state-helper.js' -import * as persistModule from '../state/persist-state-helper.js' - -vi.mock('../state/fetch-saved-state-helper.js') -vi.mock('../state/persist-state-helper.js') - -describe('BackendCatboxClient', () => { - let client - - beforeEach(() => { - client = new BackendCatboxClient() - vi.clearAllMocks() - vi.spyOn(console, 'log').mockImplementation(() => {}) - }) - - afterAll(() => { - vi.restoreAllMocks() - }) - - describe('validateSegmentName', () => { - it('throws on invalid segment', () => { - const invalidSegments = [null, '', 123, 'invalid!segment', {}, []] - invalidSegments.forEach((segment) => { - expect(() => client.validateSegmentName(segment)).toThrow() - }) - }) - - it('does not throw on valid segment', () => { - const validSegments = ['abc', 'ABC_123', 'valid-segment'] - validSegments.forEach((segment) => { - expect(client.validateSegmentName(segment)).toBeNull() - }) - }) - }) - - describe('get', () => { - it('calls fetchSavedStateFromApi and returns proper object', async () => { - const key = { userId: 'u1' } - const state = { foo: 'bar' } - fetchModule.fetchSavedStateFromApi.mockResolvedValue(state) - - const result = await client.get(key) - - expect(fetchModule.fetchSavedStateFromApi).toHaveBeenCalledWith(key) - expect(result).toHaveProperty('item', state) - }) - - it('returns item=null when fetchSavedStateFromApi returns null', async () => { - const key = { userId: 'u1' } - fetchModule.fetchSavedStateFromApi.mockResolvedValue(null) - - const result = await client.get(key) - - expect(result.item).toBeNull() - }) - }) - - describe('set', () => { - it('calls persistStateToApi with value and key', async () => { - const key = { userId: 'u1' } - const value = { foo: 'bar' } - persistModule.persistStateToApi.mockResolvedValue() - - await client.set(key, value, 1234) - - expect(persistModule.persistStateToApi).toHaveBeenCalledWith(value, key) - }) - }) - - describe('drop', () => { - it('logs drop call and does not throw', async () => { - const key = { userId: 'u1' } - await expect(client.drop(key)).resolves.toBeUndefined() - }) - }) - - describe('start', () => { - it('logs start call and does not throw', async () => { - await expect(client.start()).resolves.toBeUndefined() - }) - }) - - describe('stop', () => { - it('logs stop call and does not throw', async () => { - await expect(client.stop()).resolves.toBeUndefined() - }) - }) - - describe('isReady', () => { - it('always returns true', () => { - expect(client.isReady()).toBe(true) - }) - }) -}) diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.js b/src/server/common/helpers/state/fetch-saved-state-helper.js index f398b7133..28f1b20f0 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.js @@ -12,14 +12,14 @@ export async function fetchSavedStateFromApi(key) { return null } - const { userId, organisationId, grantId } = parseSessionKey(key.id) + const { userId, organisationId, grantId } = parseSessionKey(key) let json = null const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) try { log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { endpoint: url.href, - identity: key.id + identity: key }) url.searchParams.set('userId', userId) @@ -35,7 +35,7 @@ export async function fetchSavedStateFromApi(key) { if (response.status === statusCodes.notFound) { log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { endpoint: url.href, - identity: key.id, + identity: key, stateSummary: 'No state found in backend' }) return null @@ -48,7 +48,7 @@ export async function fetchSavedStateFromApi(key) { if (!json || typeof json !== 'object') { log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { endpoint: url.href, - identity: key.id, + identity: key, error: `Unexpected or empty state format: ${json}` }) return null @@ -56,7 +56,7 @@ export async function fetchSavedStateFromApi(key) { } catch (err) { log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { endpoint: url.href, - identity: key.id, + identity: key, error: err.message }) return null diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.test.js b/src/server/common/helpers/state/fetch-saved-state-helper.test.js index 2d782f566..4685b1d5d 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.test.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.test.js @@ -31,7 +31,7 @@ let log let LogCodes describe('fetchSavedStateFromApi', () => { - const key = { id: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` } + const key = `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` const createSuccessfulResponse = (data = MOCK_STATE_DATA.DEFAULT) => ({ ok: true, diff --git a/src/server/common/helpers/state/persist-state-helper.js b/src/server/common/helpers/state/persist-state-helper.js index 0177f1877..948b26108 100644 --- a/src/server/common/helpers/state/persist-state-helper.js +++ b/src/server/common/helpers/state/persist-state-helper.js @@ -13,11 +13,11 @@ export async function persistStateToApi(state, key) { const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) - const { userId, organisationId, grantId } = parseSessionKey(key.id) + const { userId, organisationId, grantId } = parseSessionKey(key) log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { endpoint: url.href, - identity: key.id, + identity: key, stateSummary: { hasReference: Boolean(state?.$$__referenceNumber), keyCount: Object.keys(state || {}).length @@ -40,14 +40,14 @@ export async function persistStateToApi(state, key) { if (!response.ok) { log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { endpoint: url.href, - identity: key.id, + identity: key, error: `${response.status} - ${response.statusText}` }) } } catch (err) { log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { endpoint: url.href, - identity: key.id, + identity: key, error: err.message }) // TODO: See TGC-781 diff --git a/src/server/common/helpers/state/persist-state-helper.test.js b/src/server/common/helpers/state/persist-state-helper.test.js index 93576242f..ed9abe2d8 100644 --- a/src/server/common/helpers/state/persist-state-helper.test.js +++ b/src/server/common/helpers/state/persist-state-helper.test.js @@ -34,7 +34,7 @@ let log let LogCodes describe('persistStateToApi', () => { - const key = { id: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` } + const key = `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` const testState = MOCK_STATE_DATA.WITH_STEP beforeEach(() => { diff --git a/src/server/common/services/state-persistence/state-persistence.service.js b/src/server/common/services/state-persistence/state-persistence.service.js new file mode 100644 index 000000000..e0d5450a2 --- /dev/null +++ b/src/server/common/services/state-persistence/state-persistence.service.js @@ -0,0 +1,98 @@ +import { getCacheKey } from '~/src/server/common/helpers/state/get-cache-key-helper.js' +import { fetchSavedStateFromApi } from '../../helpers/state/fetch-saved-state-helper.js' +import { persistStateToApi } from '../../helpers/state/persist-state-helper.js' +import { ADDITIONAL_IDENTIFIER, CacheService } from '@defra/forms-engine-plugin/cache-service.js' + +/** + * Service responsible for persisting form/session state to the backend API. + * Can be used in place of a traditional Hapi cache in Forms Engine Plugin. + */ +export class StatePersistenceService extends CacheService { + /** + * @param {import('@hapi/hapi').Server} options.server - Hapi server (used for logging) + */ + constructor({ server }) { + super({ server }) + this.logger = server.logger + } + + /** + * Get form state from backend persistence. + * @param {import('../plugins/engine/types.js').AnyRequest} request + * @returns {Promise} resolved state or empty object + */ + async getState(request) { + const key = this.Key(request) + return fetchSavedStateFromApi(key) ?? {} + } + + /** + * Persist form state to backend. + * @param {import('../plugins/engine/types.js').AnyFormRequest} request + * @param {object} state + * @returns {Promise} the persisted state + */ + async setState(request, state) { + const key = this.Key(request) + await persistStateToApi(state, key) + return state + } + + /** + * Retrieves the confirmation state for a given request. + * This typically tracks whether the user has confirmed submission or similar actions. + * The state is persisted via the backend API rather than in-memory cache. + * + * @param {import('@hapi/hapi').Request} request + * @returns {Promise<{ confirmed?: true }>} Confirmation state + */ + async getConfirmationState(request) { + const key = this.ConfirmationKey(request) + const state = await fetchSavedStateFromApi(key) + return state ?? {} + } + + /** + * Persists the confirmation state for a given request. + * This allows tracking of actions such as submission confirmation across sessions. + * The state is saved to the backend API rather than in-memory cache. + * + * @param {import('@hapi/hapi').Request} request + * @param {{ confirmed?: true }} confirmationState + */ + async setConfirmationState(request, confirmationState) { + const key = this.ConfirmationKey(request) + await persistStateToApi(confirmationState, key) + return confirmationState + } + + /** + * Clear state in backend (no-op if you don’t want to clear on submission). + * @param {import('../plugins/engine/types.js').AnyFormRequest} request + */ + async clearState(request) { + // NO-OP because you want to keep state even after submission + const key = this.Key(request) + this.logger?.info(`clearState called for ${key || 'unknown session'}, but no action taken.`) + } + + /** + * Generate a unique key for this request. + * @param {import('../plugins/engine/types.js').AnyRequest} request + * @returns string + */ + Key(request) { + const { userId, businessId, grantId } = getCacheKey(request) + return `${userId}:${businessId}:${grantId}` + } + + /** + * Generate a unique confirmation key for this request. + * @param {import('../plugins/engine/types.js').AnyRequest} request + * @returns string + */ + ConfirmationKey(request) { + const key = this.Key(request) + return `${key}${ADDITIONAL_IDENTIFIER.Confirmation}` + } +} diff --git a/src/server/common/services/state-persistence/state-persistence.service.test.js b/src/server/common/services/state-persistence/state-persistence.service.test.js new file mode 100644 index 000000000..c8d4bac09 --- /dev/null +++ b/src/server/common/services/state-persistence/state-persistence.service.test.js @@ -0,0 +1,80 @@ +import { vi } from 'vitest' +import { StatePersistenceService } from './state-persistence.service.js' +import * as fetchModule from '../../helpers/state/fetch-saved-state-helper.js' +import * as persistModule from '../../helpers/state/persist-state-helper.js' +import { getCacheKey } from '~/src/server/common/helpers/state/get-cache-key-helper.js' + +vi.mock('../../helpers/state/fetch-saved-state-helper.js', () => ({ + fetchSavedStateFromApi: vi.fn() +})) +vi.mock('../../helpers/state/persist-state-helper.js', () => ({ + persistStateToApi: vi.fn() +})) +vi.mock('~/src/server/common/helpers/state/get-cache-key-helper.js', () => ({ + getCacheKey: vi.fn() +})) + +describe('StatePersistenceService', () => { + let service + const fakeLogger = vi.fn() + const server = { log: vi.fn(), logger: { info: fakeLogger }, cache: vi.fn(() => ({ get: vi.fn(), set: vi.fn() })) } + + const fakeRequest = { + params: { slug: 'grant-a', state: '123' }, + auth: { credentials: { crn: 'user-1', relationships: ['rel:biz-1'] } } + } + + beforeEach(() => { + service = new StatePersistenceService({ server }) + vi.clearAllMocks() + }) + + test('Key generates the correct key', () => { + getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + const key = service.Key(fakeRequest) + expect(key).toBe('user-1:biz-1:grant-a') + }) + + test('ConfirmationKey appends the confirmation identifier', () => { + getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + const key = service.ConfirmationKey(fakeRequest) + expect(key).toBe(`user-1:biz-1:grant-a:confirmation`) // assuming ADDITIONAL_IDENTIFIER.Confirmation === 'confirmation' + }) + + test('getState calls fetchSavedStateFromApi and returns result', async () => { + getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + fetchModule.fetchSavedStateFromApi.mockResolvedValue({ foo: 'bar' }) + + const result = await service.getState(fakeRequest) + expect(result).toEqual({ foo: 'bar' }) + expect(fetchModule.fetchSavedStateFromApi).toHaveBeenCalledWith('user-1:biz-1:grant-a') + }) + + test('setState calls persistStateToApi and returns state', async () => { + getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + const state = { foo: 'bar' } + await service.setState(fakeRequest, state) + expect(persistModule.persistStateToApi).toHaveBeenCalledWith(state, 'user-1:biz-1:grant-a') + }) + + test('getConfirmationState calls fetchSavedStateFromApi with confirmation key', async () => { + getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + fetchModule.fetchSavedStateFromApi.mockResolvedValue({ confirmed: true }) + const result = await service.getConfirmationState(fakeRequest) + expect(result).toEqual({ confirmed: true }) + expect(fetchModule.fetchSavedStateFromApi).toHaveBeenCalledWith('user-1:biz-1:grant-a:confirmation') + }) + + test('setConfirmationState calls persistStateToApi with confirmation key', async () => { + getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + const state = { confirmed: true } + await service.setConfirmationState(fakeRequest, state) + expect(persistModule.persistStateToApi).toHaveBeenCalledWith(state, 'user-1:biz-1:grant-a:confirmation') + }) + + test('clearState logs info and does nothing', async () => { + getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + await service.clearState(fakeRequest) + expect(fakeLogger).toHaveBeenCalledWith(expect.stringContaining('clearState called for user-1:biz-1:grant-a')) + }) +}) diff --git a/src/server/index.js b/src/server/index.js index 3643855a5..44c0d4ffc 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -49,9 +49,10 @@ import RemoveActionPageController from './land-grants/controllers/remove-action- import { router } from './router.js' import { formsAuthCallback } from '~/src/server/auth/forms-engine-plugin-auth-helpers.js' import SectionEndController from './section-end/section-end.controller.js' +import CheckResponsesPageController from '~/src/server/check-responses/check-responses.controller.js' +import { StatePersistenceService } from './common/services/state-persistence/state-persistence.service.js' const SESSION_CACHE_NAME = 'session.cache.name' -const SESSION_BACKEND_NAME = 'session.backend.name' const getViewPaths = () => { const serverDir = path.resolve(path.dirname(fileURLToPath(import.meta.url))) @@ -108,10 +109,6 @@ const createHapiServer = () => { { name: config.get(SESSION_CACHE_NAME), engine: getCacheEngine(/** @type {Engine} */ (config.get('session.cache.engine'))) - }, - { - name: config.get(SESSION_BACKEND_NAME), - engine: new BackendCatboxClient() } ], state: { @@ -125,22 +122,8 @@ const registerFormsPlugin = async (server, prefix = '') => { plugin, options: { ...(prefix && { routes: { prefix } }), - cacheName: config.get(SESSION_BACKEND_NAME), + cache: new StatePersistenceService({ server }), baseUrl: config.get('baseUrl'), - saveAndReturn: { - keyGenerator: (request) => { - const { userId, organisationId, grantId } = getCacheKey(request) - return `${userId}:${organisationId}:${grantId}` - }, - // dummy: returns null, does nothing - sessionHydrator: async (_request) => { - return null - }, - // dummy: returns null, does nothing - sessionPersister: async (_state, _request) => { - return null - } - }, onRequest: formsAuthCallback, services: { formsService: await formsService(), From b2a7f31f3e918e82a58ff7a15f13b97cd883cd75 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Wed, 10 Sep 2025 15:17:51 +0100 Subject: [PATCH 10/16] Remove mistakenly check in code --- src/server/status-service.js | 29 ----------------------------- 1 file changed, 29 deletions(-) delete mode 100644 src/server/status-service.js diff --git a/src/server/status-service.js b/src/server/status-service.js deleted file mode 100644 index 6a6c9c433..000000000 --- a/src/server/status-service.js +++ /dev/null @@ -1,29 +0,0 @@ -// config of fake applications by ID for local testing -const fakeStatuses = { - 'app-123': { - crn: '1234567890', - sbi: '987654321', - status: 'SUBMITTED', - applicationReference: 'APP-12345' - }, - 'app-456': { - crn: '1234567890', - sbi: '987654321', - status: 'REJECTED', - applicationReference: 'APP-45678' - } -} - -export async function getStatusFromGAS(applicationId) { - // simulate async network delay - await new Promise((resolve) => setTimeout(resolve, 50)) - - return ( - fakeStatuses[applicationId] || { - crn: 'unknown', - sbi: 'unknown', - status: 'UNKNOWN', - applicationReference: applicationId - } - ) -} From 816a63a7b47bcc79814c90a3d9e0f45adaf7ea51 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Thu, 11 Sep 2025 10:42:24 +0100 Subject: [PATCH 11/16] Better logging plus await for fetching saved state --- src/server/common/helpers/logging/log-codes.js | 2 +- src/server/common/helpers/state/fetch-saved-state-helper.js | 2 ++ src/server/common/helpers/state/persist-state-helper.js | 3 +++ .../services/state-persistence/state-persistence.service.js | 3 ++- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/server/common/helpers/logging/log-codes.js b/src/server/common/helpers/logging/log-codes.js index 6497f681b..0e939b4d6 100644 --- a/src/server/common/helpers/logging/log-codes.js +++ b/src/server/common/helpers/logging/log-codes.js @@ -230,7 +230,7 @@ export const LogCodes = { EXTERNAL_API_CALL_DEBUG: { level: 'debug', messageFunc: (messageOptions) => - `External API call to ${messageOptions.endpoint} for identity=${messageOptions.identity || 'unknown'} - stateSummary=${JSON.stringify(messageOptions.stateSummary || {})}` + `External ${messageOptions.method} API call to ${messageOptions.endpoint} for identity=${messageOptions.identity || 'unknown'} - stateSummary=${JSON.stringify(messageOptions.stateSummary || 'N/A')}` }, EXTERNAL_API_ERROR: { level: 'error', diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.js b/src/server/common/helpers/state/fetch-saved-state-helper.js index 28f1b20f0..85c54c745 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.js @@ -18,6 +18,7 @@ export async function fetchSavedStateFromApi(key) { const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) try { log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { + method: 'GET', endpoint: url.href, identity: key }) @@ -47,6 +48,7 @@ export async function fetchSavedStateFromApi(key) { if (!json || typeof json !== 'object') { log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + method: 'GET', endpoint: url.href, identity: key, error: `Unexpected or empty state format: ${json}` diff --git a/src/server/common/helpers/state/persist-state-helper.js b/src/server/common/helpers/state/persist-state-helper.js index 948b26108..23a3e2fd5 100644 --- a/src/server/common/helpers/state/persist-state-helper.js +++ b/src/server/common/helpers/state/persist-state-helper.js @@ -16,6 +16,7 @@ export async function persistStateToApi(state, key) { const { userId, organisationId, grantId } = parseSessionKey(key) log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { + method: 'POST', endpoint: url.href, identity: key, stateSummary: { @@ -39,6 +40,7 @@ export async function persistStateToApi(state, key) { if (!response.ok) { log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + method: 'POST', endpoint: url.href, identity: key, error: `${response.status} - ${response.statusText}` @@ -46,6 +48,7 @@ export async function persistStateToApi(state, key) { } } catch (err) { log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + method: 'POST', endpoint: url.href, identity: key, error: err.message diff --git a/src/server/common/services/state-persistence/state-persistence.service.js b/src/server/common/services/state-persistence/state-persistence.service.js index e0d5450a2..3e753f2e6 100644 --- a/src/server/common/services/state-persistence/state-persistence.service.js +++ b/src/server/common/services/state-persistence/state-persistence.service.js @@ -23,7 +23,8 @@ export class StatePersistenceService extends CacheService { */ async getState(request) { const key = this.Key(request) - return fetchSavedStateFromApi(key) ?? {} + const state = await fetchSavedStateFromApi(key) + return state ?? {} } /** From 42d890ed934cc5c9bdf7e4b9d23f47e05910524b Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Thu, 11 Sep 2025 15:04:09 +0100 Subject: [PATCH 12/16] Add method to debug logging --- src/server/common/helpers/state/fetch-saved-state-helper.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.js b/src/server/common/helpers/state/fetch-saved-state-helper.js index 85c54c745..db05aaaec 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.js @@ -35,6 +35,7 @@ export async function fetchSavedStateFromApi(key) { if (!response.ok) { if (response.status === statusCodes.notFound) { log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { + method: 'GET', endpoint: url.href, identity: key, stateSummary: 'No state found in backend' From db9b0636f6e9ebebf53f1cbb1ca51e656ab04a2b Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Thu, 11 Sep 2025 15:26:34 +0100 Subject: [PATCH 13/16] Fix tests --- .../helpers/state/fetch-saved-state-helper.js | 1 + .../state/fetch-saved-state-helper.test.js | 19 ++++++++++++------- .../state/get-cache-key-helper.test.js | 2 +- src/server/index.js | 5 ----- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.js b/src/server/common/helpers/state/fetch-saved-state-helper.js index db05aaaec..8f2e56edf 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.js @@ -58,6 +58,7 @@ export async function fetchSavedStateFromApi(key) { } } catch (err) { log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + method: 'GET', endpoint: url.href, identity: key, error: err.message diff --git a/src/server/common/helpers/state/fetch-saved-state-helper.test.js b/src/server/common/helpers/state/fetch-saved-state-helper.test.js index 4685b1d5d..4f76f65a6 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.test.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.test.js @@ -31,7 +31,7 @@ let log let LogCodes describe('fetchSavedStateFromApi', () => { - const key = `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` + const key = `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}` const createSuccessfulResponse = (data = MOCK_STATE_DATA.DEFAULT) => ({ ok: true, @@ -49,7 +49,7 @@ describe('fetchSavedStateFromApi', () => { beforeEach(() => { mockParseSessionKey.mockReturnValue({ userId: TEST_USER_IDS.DEFAULT, - businessId: TEST_USER_IDS.BUSINESS_ID, + organisationId: TEST_USER_IDS.ORGANISATION_ID, grantId: TEST_USER_IDS.GRANT_ID }) }) @@ -88,8 +88,9 @@ describe('fetchSavedStateFromApi', () => { expect(log).toHaveBeenCalledWith( LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, expect.objectContaining({ + method: 'GET', endpoint: expect.stringContaining('/state/'), - identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}` }) ) }) @@ -125,8 +126,9 @@ describe('fetchSavedStateFromApi', () => { expect(log).toHaveBeenCalledWith( LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, expect.objectContaining({ + method: 'GET', endpoint: expect.stringContaining('/state/'), - identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}`, stateSummary: 'No state found in backend' }) ) @@ -141,8 +143,9 @@ describe('fetchSavedStateFromApi', () => { expect(log).toHaveBeenCalledWith( LogCodes.SYSTEM.EXTERNAL_API_ERROR, expect.objectContaining({ + method: 'GET', endpoint: expect.stringContaining('/state/'), - identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}`, error: 'Failed to fetch saved state: 500' }) ) @@ -157,8 +160,9 @@ describe('fetchSavedStateFromApi', () => { expect(log).toHaveBeenCalledWith( LogCodes.SYSTEM.EXTERNAL_API_ERROR, expect.objectContaining({ + method: 'GET', endpoint: expect.stringContaining('/state/'), - identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}`, error: 'Unexpected or empty state format: 123' }) ) @@ -174,8 +178,9 @@ describe('fetchSavedStateFromApi', () => { expect(log).toHaveBeenCalledWith( LogCodes.SYSTEM.EXTERNAL_API_ERROR, expect.objectContaining({ + method: 'GET', endpoint: expect.stringContaining('/state/'), - identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}`, + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}`, error: 'Network error' }) ) diff --git a/src/server/common/helpers/state/get-cache-key-helper.test.js b/src/server/common/helpers/state/get-cache-key-helper.test.js index 3ae463b1e..2b1464e3a 100644 --- a/src/server/common/helpers/state/get-cache-key-helper.test.js +++ b/src/server/common/helpers/state/get-cache-key-helper.test.js @@ -100,7 +100,7 @@ describe('parseSessionKey', () => { expect(result).toEqual({ userId: 'user123', - businessId: 'business456', + organisationId: 'business456', grantId: 'grant789' }) }) diff --git a/src/server/index.js b/src/server/index.js index 44c0d4ffc..63b2b6141 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -41,15 +41,10 @@ import { PotentialFundingController } from '~/src/server/non-land-grants/pigs-mi import { tasklistBackButton } from '~/src/server/plugins/tasklist-back-button.js' import { sbiStore } from '~/src/server/sbi/state.js' import { formatCurrency } from '../config/nunjucks/filters/format-currency.js' -import { fetchSavedStateFromApi } from './common/helpers/state/fetch-saved-state-helper.js' -import { getCacheKey } from './common/helpers/state/get-cache-key-helper.js' -import { persistStateToApi } from './common/helpers/state/persist-state-helper.js' import { contentSecurityPolicy } from '~/src/server/common/helpers/csp.js' import RemoveActionPageController from './land-grants/controllers/remove-action-page.controller.js' import { router } from './router.js' -import { formsAuthCallback } from '~/src/server/auth/forms-engine-plugin-auth-helpers.js' import SectionEndController from './section-end/section-end.controller.js' -import CheckResponsesPageController from '~/src/server/check-responses/check-responses.controller.js' import { StatePersistenceService } from './common/services/state-persistence/state-persistence.service.js' const SESSION_CACHE_NAME = 'session.cache.name' From 1760a5843c2ed39fee0b3600086b9fc5c92e48b0 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Fri, 12 Sep 2025 12:49:17 +0100 Subject: [PATCH 14/16] Backend cache segment name is not needed as we are using Grants UI Backend API --- src/config/config.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/config/config.js b/src/config/config.js index 659289016..b025c201b 100644 --- a/src/config/config.js +++ b/src/config/config.js @@ -243,14 +243,6 @@ export const config = convict({ sensitive: true } }, - backend: { - name: { - doc: 'Persistent backend cache used for save-and-continue and long-term storage', - format: String, - default: 'backendSession', - env: 'BACKEND_CACHE_NAME' - } - }, cookie: { ttl: { doc: 'Session cookie ttl', From 89e6df02922ba946f0c67ffca040a4cbeb8dda91 Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Fri, 12 Sep 2025 13:11:15 +0100 Subject: [PATCH 15/16] Update missed organisationId attribute from getCacheKey --- .../state-persistence/state-persistence.service.js | 4 ++-- .../state-persistence.service.test.js | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/server/common/services/state-persistence/state-persistence.service.js b/src/server/common/services/state-persistence/state-persistence.service.js index 3e753f2e6..e5ee68639 100644 --- a/src/server/common/services/state-persistence/state-persistence.service.js +++ b/src/server/common/services/state-persistence/state-persistence.service.js @@ -83,8 +83,8 @@ export class StatePersistenceService extends CacheService { * @returns string */ Key(request) { - const { userId, businessId, grantId } = getCacheKey(request) - return `${userId}:${businessId}:${grantId}` + const { userId, organisationId, grantId } = getCacheKey(request) + return `${userId}:${organisationId}:${grantId}` } /** diff --git a/src/server/common/services/state-persistence/state-persistence.service.test.js b/src/server/common/services/state-persistence/state-persistence.service.test.js index c8d4bac09..3f32205bf 100644 --- a/src/server/common/services/state-persistence/state-persistence.service.test.js +++ b/src/server/common/services/state-persistence/state-persistence.service.test.js @@ -30,19 +30,19 @@ describe('StatePersistenceService', () => { }) test('Key generates the correct key', () => { - getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + getCacheKey.mockReturnValue({ userId: 'user-1', organisationId: 'biz-1', grantId: 'grant-a' }) const key = service.Key(fakeRequest) expect(key).toBe('user-1:biz-1:grant-a') }) test('ConfirmationKey appends the confirmation identifier', () => { - getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + getCacheKey.mockReturnValue({ userId: 'user-1', organisationId: 'biz-1', grantId: 'grant-a' }) const key = service.ConfirmationKey(fakeRequest) expect(key).toBe(`user-1:biz-1:grant-a:confirmation`) // assuming ADDITIONAL_IDENTIFIER.Confirmation === 'confirmation' }) test('getState calls fetchSavedStateFromApi and returns result', async () => { - getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + getCacheKey.mockReturnValue({ userId: 'user-1', organisationId: 'biz-1', grantId: 'grant-a' }) fetchModule.fetchSavedStateFromApi.mockResolvedValue({ foo: 'bar' }) const result = await service.getState(fakeRequest) @@ -51,14 +51,14 @@ describe('StatePersistenceService', () => { }) test('setState calls persistStateToApi and returns state', async () => { - getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + getCacheKey.mockReturnValue({ userId: 'user-1', organisationId: 'biz-1', grantId: 'grant-a' }) const state = { foo: 'bar' } await service.setState(fakeRequest, state) expect(persistModule.persistStateToApi).toHaveBeenCalledWith(state, 'user-1:biz-1:grant-a') }) test('getConfirmationState calls fetchSavedStateFromApi with confirmation key', async () => { - getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + getCacheKey.mockReturnValue({ userId: 'user-1', organisationId: 'biz-1', grantId: 'grant-a' }) fetchModule.fetchSavedStateFromApi.mockResolvedValue({ confirmed: true }) const result = await service.getConfirmationState(fakeRequest) expect(result).toEqual({ confirmed: true }) @@ -66,14 +66,14 @@ describe('StatePersistenceService', () => { }) test('setConfirmationState calls persistStateToApi with confirmation key', async () => { - getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + getCacheKey.mockReturnValue({ userId: 'user-1', organisationId: 'biz-1', grantId: 'grant-a' }) const state = { confirmed: true } await service.setConfirmationState(fakeRequest, state) expect(persistModule.persistStateToApi).toHaveBeenCalledWith(state, 'user-1:biz-1:grant-a:confirmation') }) test('clearState logs info and does nothing', async () => { - getCacheKey.mockReturnValue({ userId: 'user-1', businessId: 'biz-1', grantId: 'grant-a' }) + getCacheKey.mockReturnValue({ userId: 'user-1', organisationId: 'biz-1', grantId: 'grant-a' }) await service.clearState(fakeRequest) expect(fakeLogger).toHaveBeenCalledWith(expect.stringContaining('clearState called for user-1:biz-1:grant-a')) }) From 3d224c07c44bdb3cbd32ebcbe8c3ca0ea0064fbe Mon Sep 17 00:00:00 2001 From: Andrew Folga Date: Fri, 12 Sep 2025 13:15:45 +0100 Subject: [PATCH 16/16] Rename to satisfy Sonar --- .../state-persistence.service.js | 16 ++++++++-------- .../state-persistence.service.test.js | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/server/common/services/state-persistence/state-persistence.service.js b/src/server/common/services/state-persistence/state-persistence.service.js index e5ee68639..5b131fc4f 100644 --- a/src/server/common/services/state-persistence/state-persistence.service.js +++ b/src/server/common/services/state-persistence/state-persistence.service.js @@ -22,7 +22,7 @@ export class StatePersistenceService extends CacheService { * @returns {Promise} resolved state or empty object */ async getState(request) { - const key = this.Key(request) + const key = this._Key(request) const state = await fetchSavedStateFromApi(key) return state ?? {} } @@ -34,7 +34,7 @@ export class StatePersistenceService extends CacheService { * @returns {Promise} the persisted state */ async setState(request, state) { - const key = this.Key(request) + const key = this._Key(request) await persistStateToApi(state, key) return state } @@ -48,7 +48,7 @@ export class StatePersistenceService extends CacheService { * @returns {Promise<{ confirmed?: true }>} Confirmation state */ async getConfirmationState(request) { - const key = this.ConfirmationKey(request) + const key = this._ConfirmationKey(request) const state = await fetchSavedStateFromApi(key) return state ?? {} } @@ -62,7 +62,7 @@ export class StatePersistenceService extends CacheService { * @param {{ confirmed?: true }} confirmationState */ async setConfirmationState(request, confirmationState) { - const key = this.ConfirmationKey(request) + const key = this._ConfirmationKey(request) await persistStateToApi(confirmationState, key) return confirmationState } @@ -73,7 +73,7 @@ export class StatePersistenceService extends CacheService { */ async clearState(request) { // NO-OP because you want to keep state even after submission - const key = this.Key(request) + const key = this._Key(request) this.logger?.info(`clearState called for ${key || 'unknown session'}, but no action taken.`) } @@ -82,7 +82,7 @@ export class StatePersistenceService extends CacheService { * @param {import('../plugins/engine/types.js').AnyRequest} request * @returns string */ - Key(request) { + _Key(request) { const { userId, organisationId, grantId } = getCacheKey(request) return `${userId}:${organisationId}:${grantId}` } @@ -92,8 +92,8 @@ export class StatePersistenceService extends CacheService { * @param {import('../plugins/engine/types.js').AnyRequest} request * @returns string */ - ConfirmationKey(request) { - const key = this.Key(request) + _ConfirmationKey(request) { + const key = this._Key(request) return `${key}${ADDITIONAL_IDENTIFIER.Confirmation}` } } diff --git a/src/server/common/services/state-persistence/state-persistence.service.test.js b/src/server/common/services/state-persistence/state-persistence.service.test.js index 3f32205bf..11d1c2c9a 100644 --- a/src/server/common/services/state-persistence/state-persistence.service.test.js +++ b/src/server/common/services/state-persistence/state-persistence.service.test.js @@ -31,13 +31,13 @@ describe('StatePersistenceService', () => { test('Key generates the correct key', () => { getCacheKey.mockReturnValue({ userId: 'user-1', organisationId: 'biz-1', grantId: 'grant-a' }) - const key = service.Key(fakeRequest) + const key = service._Key(fakeRequest) expect(key).toBe('user-1:biz-1:grant-a') }) test('ConfirmationKey appends the confirmation identifier', () => { getCacheKey.mockReturnValue({ userId: 'user-1', organisationId: 'biz-1', grantId: 'grant-a' }) - const key = service.ConfirmationKey(fakeRequest) + const key = service._ConfirmationKey(fakeRequest) expect(key).toBe(`user-1:biz-1:grant-a:confirmation`) // assuming ADDITIONAL_IDENTIFIER.Confirmation === 'confirmation' })