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", diff --git a/src/server/common/helpers/logging/log-codes.js b/src/server/common/helpers/logging/log-codes.js index 32211d6f1..0e939b4d6 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 ${messageOptions.method} API call to ${messageOptions.endpoint} for identity=${messageOptions.identity || 'unknown'} - stateSummary=${JSON.stringify(messageOptions.stateSummary || 'N/A')}` + }, 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/start-server.test.js b/src/server/common/helpers/start-server.test.js index fe2d7b3ed..902f2e225 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 = vi.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..8f2e56edf 100644 --- a/src/server/common/helpers/state/fetch-saved-state-helper.js +++ b/src/server/common/helpers/state/fetch-saved-state-helper.js @@ -1,20 +1,28 @@ 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 { log, LogCodes } from '../logging/log.js' const GRANTS_UI_BACKEND_ENDPOINT = config.get('session.cache.apiEndpoint') -export async function fetchSavedStateFromApi(request) { +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) let json = null + const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) try { - const url = new URL('/state/', GRANTS_UI_BACKEND_ENDPOINT) + log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { + method: 'GET', + endpoint: url.href, + identity: key + }) + url.searchParams.set('userId', userId) url.searchParams.set('businessId', organisationId) url.searchParams.set('grantId', grantId) @@ -26,6 +34,12 @@ export async function fetchSavedStateFromApi(request) { 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' + }) return null } throw new Error(`Failed to fetch saved state: ${response.status}`) @@ -34,11 +48,21 @@ 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 }) + log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + method: 'GET', + endpoint: url.href, + identity: key, + error: `Unexpected or empty state format: ${json}` + }) return null } } catch (err) { - request.logger.error(['fetch-saved-state'], 'Failed to fetch saved state from API', err) + log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + method: 'GET', + endpoint: url.href, + 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 8ebb0f508..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 @@ -1,52 +1,77 @@ import { vi } from 'vitest' -import { mockRequestWithIdentity } from './mock-request-with-identity.test-helper.js' 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() + +const mockLogger = { + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn() } -global.fetch = vi.fn() +vi.mock('../logging/logger.js', () => ({ + createLogger: () => mockLogger +})) + +// Mock parseSessionKey +const mockParseSessionKey = vi.fn() +vi.mock('./get-cache-key-helper.js', () => ({ + parseSessionKey: mockParseSessionKey +})) let fetchSavedStateFromApi +let log +let LogCodes -const mockRequest = mockRequestWithIdentity({ params: { slug: TEST_USER_IDS.GRANT_ID } }) -const mockRequestWithLogger = { - ...mockRequest, - logger: mockRequestLogger() -} +describe('fetchSavedStateFromApi', () => { + const key = `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}` -const successfulResponse = { - ok: true, - json: () => MOCK_STATE_DATA.DEFAULT -} + 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) + } + }) -const createFailedResponse = (status, statusText = 'Error') => ({ - ok: false, - status, - statusText, - json: () => { - throw new Error(ERROR_MESSAGES.NO_CONTENT) - } -}) + beforeEach(() => { + mockParseSessionKey.mockReturnValue({ + userId: TEST_USER_IDS.DEFAULT, + organisationId: TEST_USER_IDS.ORGANISATION_ID, + grantId: TEST_USER_IDS.GRANT_ID + }) + }) -describe('fetchSavedStateFromApi', () => { describe('With backend configured correctly', () => { beforeEach(async () => { vi.clearAllMocks() vi.resetModules() vi.doMock('~/src/config/config.js', createMockConfig) + 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() } + } + } + })) 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(() => { @@ -54,18 +79,26 @@ 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(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, + expect.objectContaining({ + method: 'GET', + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}` + }) + ) }) 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,35 +118,53 @@ 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(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, + expect.objectContaining({ + method: 'GET', + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}`, + stateSummary: 'No state found in backend' + }) + ) }) 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(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_ERROR, + expect.objectContaining({ + method: 'GET', + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}`, + error: 'Failed to fetch saved state: 500' + }) + ) }) - 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(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_ERROR, + expect.objectContaining({ + method: 'GET', + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}`, + error: 'Unexpected or empty state format: 123' + }) ) }) @@ -121,13 +172,17 @@ describe('fetchSavedStateFromApi', () => { 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(log).toHaveBeenCalledWith( + LogCodes.SYSTEM.EXTERNAL_API_ERROR, + expect.objectContaining({ + method: 'GET', + endpoint: expect.stringContaining('/state/'), + identity: `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.ORGANISATION_ID}:${TEST_USER_IDS.GRANT_ID}`, + error: 'Network error' + }) ) }) }) @@ -146,7 +201,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/get-cache-key-helper.test.js b/src/server/common/helpers/state/get-cache-key-helper.test.js index aef380a63..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 @@ -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', + organisationId: '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') + }) +}) diff --git a/src/server/common/helpers/state/persist-state-helper.js b/src/server/common/helpers/state/persist-state-helper.js index 51a5f13f4..23a3e2fd5 100644 --- a/src/server/common/helpers/state/persist-state-helper.js +++ b/src/server/common/helpers/state/persist-state-helper.js @@ -1,19 +1,29 @@ 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 { log, LogCodes } from '../logging/log.js' const GRANTS_UI_BACKEND_ENDPOINT = config.get('session.cache.apiEndpoint') -export async function persistStateToApi(state, request) { +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) + + log(LogCodes.SYSTEM.EXTERNAL_API_CALL_DEBUG, { + method: 'POST', + endpoint: url.href, + identity: key, + stateSummary: { + hasReference: Boolean(state?.$$__referenceNumber), + keyCount: Object.keys(state || {}).length + } + }) try { const response = await fetch(url.href, { @@ -29,11 +39,20 @@ export async function persistStateToApi(state, request) { }) if (!response.ok) { - request.logger.error(`Failed to persist state to API: ${response.status} - ${response.statusText}`) + log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + method: 'POST', + endpoint: url.href, + identity: key, + error: `${response.status} - ${response.statusText}` + }) } } catch (err) { - request.logger.error(`Failed to persist state to API: ${err.message}`) - + log(LogCodes.SYSTEM.EXTERNAL_API_ERROR, { + method: 'POST', + endpoint: url.href, + identity: key, + 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 670806226..ed9abe2d8 100644 --- a/src/server/common/helpers/state/persist-state-helper.test.js +++ b/src/server/common/helpers/state/persist-state-helper.test.js @@ -1,58 +1,59 @@ import { vi } from 'vitest' -import { mockRequestWithIdentity } from './mock-request-with-identity.test-helper.js' 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' +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 })) 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 createMockRequest = () => mockRequestWithIdentity({ params: { slug: TEST_USER_IDS.GRANT_ID } }) - - const createMockRequestWithLogger = () => { - const request = createMockRequest() - request.logger = mockRequestLogger() - return request - } - - const createTestState = () => MOCK_STATE_DATA.WITH_STEP + const key = `${TEST_USER_IDS.DEFAULT}:${TEST_USER_IDS.BUSINESS_ID}:${TEST_USER_IDS.GRANT_ID}` + const testState = 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.resetModules() vi.doMock('~/src/config/config.js', createMockConfig) const helper = await import('~/src/server/common/helpers/state/persist-state-helper.js') persistStateToApi = helper.persistStateToApi - setupMockCacheKey() + const logModule = await import('../logging/log.js') + log = logModule.log + LogCodes = logModule.LogCodes vi.clearAllMocks() }) @@ -77,16 +78,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 +101,16 @@ 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(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 + } + }) ) }) @@ -112,14 +118,16 @@ 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( - `${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}` + }) ) }) @@ -127,24 +135,27 @@ 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(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 + }) + ) }) }) 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 +164,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/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..5b131fc4f --- /dev/null +++ b/src/server/common/services/state-persistence/state-persistence.service.js @@ -0,0 +1,99 @@ +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) + const state = await fetchSavedStateFromApi(key) + return state ?? {} + } + + /** + * 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, organisationId, grantId } = getCacheKey(request) + return `${userId}:${organisationId}:${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..11d1c2c9a --- /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', 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', 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', organisationId: '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', 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', organisationId: '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', 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', organisationId: '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 c3fcdb57d..63b2b6141 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -41,13 +41,11 @@ 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 SectionEndController from './section-end/section-end.controller.js' +import { StatePersistenceService } from './common/services/state-persistence/state-persistence.service.js' const SESSION_CACHE_NAME = 'session.cache.name' @@ -119,20 +117,8 @@ const registerFormsPlugin = async (server, prefix = '') => { plugin, options: { ...(prefix && { routes: { prefix } }), - cacheName: config.get(SESSION_CACHE_NAME), + cache: new StatePersistenceService({ server }), baseUrl: config.get('baseUrl'), - saveAndReturn: { - keyGenerator: (request) => { - const { userId, organisationId, grantId } = getCacheKey(request) - return `${userId}:${organisationId}:${grantId}` - }, - sessionHydrator: async (request) => { - return fetchSavedStateFromApi(request) - }, - sessionPersister: async (state, request) => { - return persistStateToApi(state, request) - } - }, onRequest: formsAuthCallback, services: { formsService: await formsService(),