From 2af6cb8159d9d8d3145003856832d30b684000d8 Mon Sep 17 00:00:00 2001 From: Hoan-Vu Tran-Ho Date: Wed, 12 Aug 2026 18:02:07 -0400 Subject: [PATCH 1/8] KMS-598: Support Older Versions of the GCMD Keywords in KMS --- cdk/app/lib/helper/KmsLambdaFunctions.ts | 18 ++ .../__tests__/handler.test.js | 136 +++++++++++ .../getHistoricalConceptVersions/handler.js | 84 +++++++ .../__tests__/handler.test.js | 230 ++++++++++++++++++ .../getHistoricalConceptsInScheme/handler.js | 154 ++++++++++++ 5 files changed, 622 insertions(+) create mode 100644 serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js create mode 100644 serverless/src/getHistoricalConceptVersions/handler.js create mode 100644 serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js create mode 100644 serverless/src/getHistoricalConceptsInScheme/handler.js diff --git a/cdk/app/lib/helper/KmsLambdaFunctions.ts b/cdk/app/lib/helper/KmsLambdaFunctions.ts index 8a3463e1..5cf68c54 100644 --- a/cdk/app/lib/helper/KmsLambdaFunctions.ts +++ b/cdk/app/lib/helper/KmsLambdaFunctions.ts @@ -234,6 +234,15 @@ export class LambdaFunctions { 'GET' ) + this.createApiLambda( + scope, + 'getHistoricalConceptsInScheme/handler.js', + 'get-historical-concepts-in-scheme', + 'getHistoricalConceptsInScheme', + '/concepts/historical/concept_scheme/{conceptScheme}', + 'GET' + ) + this.createApiLambda( scope, 'getConcepts/handler.js', @@ -279,6 +288,15 @@ export class LambdaFunctions { 'GET' ) + this.createApiLambda( + scope, + 'getHistoricalConceptVersions/handler.js', + 'get-historical-concept-versions', + 'getHistoricalConceptVersions', + '/concept_versions/historical', + 'GET' + ) + this.createApiLambda( scope, 'getFullPath/handler.js', diff --git a/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js b/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js new file mode 100644 index 00000000..d54d76cc --- /dev/null +++ b/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js @@ -0,0 +1,136 @@ +import { + beforeEach, + describe, + expect, + vi +} from 'vitest' + +import { getApplicationConfig } from '@/shared/getConfig' +import { logAnalyticsData } from '@/shared/logAnalyticsData' + +import { getHistoricalConceptVersions } from '../handler' + +// `getS3Client()` is called once at module-load time in handler.js, so the +// mocked client needs to exist before the handler module is imported. Using +// vi.hoisted + a mock factory ensures `mockSend` is available when the +// `@/shared/awsClients` mock is set up, and lets us control its behavior +// per test via mockSend.mockResolvedValueOnce(...). +const { mockSend } = vi.hoisted(() => ({ + mockSend: vi.fn() +})) + +vi.mock('@/shared/awsClients', () => ({ + getS3Client: () => ({ send: mockSend }) +})) + +vi.mock('@/shared/getConfig') +vi.mock('@/shared/logAnalyticsData') + +describe('getHistoricalConceptVersions', () => { + beforeEach(() => { + vi.resetAllMocks() + vi.spyOn(console, 'error').mockImplementation(() => {}) + + // Mock getApplicationConfig + getApplicationConfig.mockReturnValue({ + defaultResponseHeaders: { 'X-Test': 'test-header' } + }) + }) + + describe('when successful', () => { + test('should return 200 with the list of version directories', async () => { + mockSend.mockResolvedValueOnce({ + CommonPrefixes: [ + { Prefix: 'A/' }, + { Prefix: 'B/' } + ] + }) + + const event = {} + const context = {} + const response = await getHistoricalConceptVersions(event, context) + + expect(response.statusCode).toBe(200) + expect(response.headers['X-Test']).toBe('test-header') + expect(JSON.parse(response.body)).toEqual({ historicalVersions: ['A', 'B'] }) + }) + + test('should exclude the draft directory', async () => { + mockSend.mockResolvedValueOnce({ + CommonPrefixes: [ + { Prefix: 'draft/' }, + { Prefix: 'A/' } + ] + }) + + const response = await getHistoricalConceptVersions({}, {}) + + expect(JSON.parse(response.body)).toEqual({ historicalVersions: ['A'] }) + }) + + test('should return an empty array when there are no common prefixes', async () => { + mockSend.mockResolvedValueOnce({}) + + const response = await getHistoricalConceptVersions({}, {}) + + expect(response.statusCode).toBe(200) + expect(JSON.parse(response.body)).toEqual({ historicalVersions: [] }) + }) + + test('should paginate through multiple pages of results', async () => { + mockSend + .mockResolvedValueOnce({ + CommonPrefixes: [{ Prefix: 'A/' }], + NextContinuationToken: 'page-2-token' + }) + .mockResolvedValueOnce({ + CommonPrefixes: [{ Prefix: 'B/' }] + }) + + const response = await getHistoricalConceptVersions({}, {}) + + expect(mockSend).toHaveBeenCalledTimes(2) + expect(JSON.parse(response.body)).toEqual({ historicalVersions: ['A', 'B'] }) + }) + + test('should call logAnalyticsData with the event and context', async () => { + mockSend.mockResolvedValueOnce({ CommonPrefixes: [] }) + + const event = { some: 'event' } + const context = { some: 'context' } + await getHistoricalConceptVersions(event, context) + + expect(logAnalyticsData).toHaveBeenCalledWith({ + event, + context + }) + }) + }) + + describe('when unsuccessful', () => { + test('should return 500 when the S3 request fails', async () => { + mockSend.mockRejectedValueOnce(new Error('The specified bucket does not exist')) + + const response = await getHistoricalConceptVersions({}, {}) + + expect(response.statusCode).toBe(500) + expect(response.headers['X-Test']).toBe('test-header') + expect(JSON.parse(response.body)).toEqual({ + message: 'Failed to fetch version directories' + }) + }) + + test('should log the error to the console', async () => { + const error = new Error('The specified bucket does not exist') + mockSend.mockRejectedValueOnce(error) + + await getHistoricalConceptVersions({}, {}) + + // eslint-disable-next-line no-console + expect(console.error).toHaveBeenCalledWith( + 'Failed to list S3 version directories:', + error.message + ) + }) + }) +}) diff --git a/serverless/src/getHistoricalConceptVersions/handler.js b/serverless/src/getHistoricalConceptVersions/handler.js new file mode 100644 index 00000000..ae778705 --- /dev/null +++ b/serverless/src/getHistoricalConceptVersions/handler.js @@ -0,0 +1,84 @@ +import { ListObjectsV2Command } from '@aws-sdk/client-s3' + +import { getS3Client } from '@/shared/awsClients' +import { getApplicationConfig } from '@/shared/getConfig' +import { logAnalyticsData } from '@/shared/logAnalyticsData' + +/** + * S3 bucket name to list version directories from + * @type {string} + */ +const bucketName = process.env.S3_BUCKET_NAME || 'kms-rdf-backup-sit' + +/** + * S3 Client from shared configuration + * @type {S3Client} + */ +const s3Client = getS3Client() + +/** + * Lists top-level "directory" names in the S3 bucket by using a delimiter + * so S3 groups keys into CommonPrefixes (its equivalent of folders), + * excluding the "draft/" prefix. + * + * @returns {Promise>} Array of version/directory names + */ +const listVersionDirectories = async () => { + const historicalVersions = [] + let continuationToken + + /* eslint-disable no-await-in-loop */ + do { + const command = new ListObjectsV2Command({ + Bucket: bucketName, + Delimiter: '/', + ContinuationToken: continuationToken + }) + + const response = await s3Client.send(command) + + if (response.CommonPrefixes) { + const prefixes = response.CommonPrefixes + .map((p) => p.Prefix) + .filter(Boolean) + .map((prefix) => prefix.replace(/\/$/, '')) // Strip trailing slash + .filter((name) => name !== 'draft') // Exclude the draft "directory" + + historicalVersions.push(...prefixes) + } + + continuationToken = response.NextContinuationToken + } while (continuationToken) + /* eslint-enable no-await-in-loop */ + + return historicalVersions +} + +export const getHistoricalConceptVersions = async (event, context) => { + const { defaultResponseHeaders } = getApplicationConfig() + + logAnalyticsData({ + event, + context + }) + + try { + const historicalVersions = await listVersionDirectories() + + return { + statusCode: 200, + headers: defaultResponseHeaders, + body: JSON.stringify({ historicalVersions }) + } + } catch (error) { + console.error('Failed to list S3 version directories:', error.message) + + return { + statusCode: 500, + headers: defaultResponseHeaders, + body: JSON.stringify({ message: 'Failed to fetch version directories' }) + } + } +} + +export default getHistoricalConceptVersions diff --git a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js new file mode 100644 index 00000000..79ff6ee6 --- /dev/null +++ b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js @@ -0,0 +1,230 @@ +import { + beforeEach, + describe, + expect, + vi +} from 'vitest' + +import { getApplicationConfig } from '@/shared/getConfig' +import { logAnalyticsData } from '@/shared/logAnalyticsData' + +import { getHistoricalConceptsInScheme } from '../handler' + +// GetS3Client() is called once at module-load time in handler.js, so the +// mocked client needs to exist before the handler module is imported. Using +// vi.hoisted + a mock factory ensures `mockSend` is available when the +// `@/shared/awsClients` mock is set up, and lets us control its behavior +// per test via mockSend.mockResolvedValueOnce(...). +const { mockSend } = vi.hoisted(() => ({ + mockSend: vi.fn() +})) + +vi.mock('@/shared/awsClients', () => ({ + getS3Client: () => ({ send: mockSend }) +})) + +vi.mock('@/shared/getConfig') +vi.mock('@/shared/logAnalyticsData') + +describe('getHistoricalConceptsInScheme', () => { + beforeEach(() => { + vi.resetAllMocks() + vi.spyOn(console, 'error').mockImplementation(() => {}) + + // Mock getApplicationConfig + getApplicationConfig.mockReturnValue({ + defaultResponseHeaders: { 'X-Test': 'test-header' } + }) + }) + + describe('when validation errors occur', () => { + test('should return 400 when conceptScheme path parameter is missing', async () => { + const event = { + pathParameters: {}, + queryStringParameters: { version: 'A' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(400) + expect(response.headers['X-Test']).toBe('test-header') + expect(JSON.parse(response.body)).toEqual({ error: 'scheme is required' }) + expect(mockSend).not.toHaveBeenCalled() + }) + + test('should return 400 when version query parameter is missing', async () => { + const event = { + pathParameters: { conceptScheme: 'ScienceKeywords' }, + queryStringParameters: {} + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(400) + expect(JSON.parse(response.body)).toEqual({ error: 'version is required' }) + expect(mockSend).not.toHaveBeenCalled() + }) + + test('should return 400 when pathParameters and queryStringParameters are absent entirely', async () => { + const response = await getHistoricalConceptsInScheme({}, {}) + + expect(response.statusCode).toBe(400) + expect(JSON.parse(response.body)).toEqual({ error: 'scheme is required' }) + }) + }) + + describe('when successful', () => { + test('should return 200 with the CSV content, matching the scheme case-insensitively', async () => { + // First call: ListObjectsV2 under the version prefix + mockSend.mockResolvedValueOnce({ + Contents: [ + { Key: 'A/ScienceKeywords.csv' }, + { Key: 'A/instruments.csv' } + ] + }) + + // Second call: GetObject for the matched key + mockSend.mockResolvedValueOnce({ + Body: { transformToString: () => Promise.resolve('id,label\n1,Foo\n2,Bar') } + }) + + const event = { + pathParameters: { conceptScheme: 'sciencekeywords' }, + queryStringParameters: { version: 'A' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(200) + expect(response.headers['Content-Type']).toBe('text/csv; charset=utf-8') + expect(response.headers['Content-Disposition']).toBe('attachment; filename="sciencekeywords.csv"') + expect(response.headers['X-Test']).toBe('test-header') + expect(response.body).toBe('id,label\n1,Foo\n2,Bar') + + expect(mockSend).toHaveBeenCalledTimes(2) + }) + + test('should paginate through multiple pages when listing keys', async () => { + mockSend + .mockResolvedValueOnce({ + Contents: [{ Key: 'A/other.csv' }], + NextContinuationToken: 'page-2-token' + }) + .mockResolvedValueOnce({ + Contents: [{ Key: 'A/instruments.csv' }] + }) + .mockResolvedValueOnce({ + Body: { transformToString: () => Promise.resolve('id,label\n1,Sensor') } + }) + + const event = { + pathParameters: { conceptScheme: 'instruments' }, + queryStringParameters: { version: 'A' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(200) + expect(response.body).toBe('id,label\n1,Sensor') + expect(mockSend).toHaveBeenCalledTimes(3) + }) + + test('should call logAnalyticsData with the event and context', async () => { + mockSend.mockResolvedValueOnce({ + Contents: [{ Key: 'A/instruments.csv' }] + }) + + mockSend.mockResolvedValueOnce({ + Body: { transformToString: () => Promise.resolve('id,label') } + }) + + const event = { + pathParameters: { conceptScheme: 'instruments' }, + queryStringParameters: { version: 'A' } + } + const context = { some: 'context' } + await getHistoricalConceptsInScheme(event, context) + + expect(logAnalyticsData).toHaveBeenCalledWith({ + event, + context + }) + }) + }) + + describe('when unsuccessful', () => { + test('should return 404 when no CSV matches the requested scheme', async () => { + mockSend.mockResolvedValueOnce({ + Contents: [{ Key: 'A/instruments.csv' }] + }) + + const event = { + pathParameters: { conceptScheme: 'doesnotexist' }, + queryStringParameters: { version: 'A' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(404) + expect(JSON.parse(response.body)).toEqual({ + error: 'No concept scheme "doesnotexist" found for version "A"' + }) + + // Only the list call should happen, never a GetObject + expect(mockSend).toHaveBeenCalledTimes(1) + }) + + test('should return 404 when the version prefix has no objects at all', async () => { + mockSend.mockResolvedValueOnce({}) + + const event = { + pathParameters: { conceptScheme: 'instruments' }, + queryStringParameters: { version: 'unknown-version' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(404) + }) + + test('should return 500 when listing objects fails', async () => { + mockSend.mockRejectedValueOnce(new Error('The specified bucket does not exist')) + + const event = { + pathParameters: { conceptScheme: 'instruments' }, + queryStringParameters: { version: 'A' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(500) + expect(JSON.parse(response.body)).toEqual({ error: 'Failed to fetch concept scheme CSV' }) + }) + + test('should return 500 when downloading the matched object fails', async () => { + mockSend.mockResolvedValueOnce({ + Contents: [{ Key: 'A/instruments.csv' }] + }) + + mockSend.mockRejectedValueOnce(new Error('Access denied')) + + const event = { + pathParameters: { conceptScheme: 'instruments' }, + queryStringParameters: { version: 'A' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(500) + expect(JSON.parse(response.body)).toEqual({ error: 'Failed to fetch concept scheme CSV' }) + }) + + test('should log the error to the console', async () => { + const error = new Error('Access denied') + mockSend.mockRejectedValueOnce(error) + + const event = { + pathParameters: { conceptScheme: 'instruments' }, + queryStringParameters: { version: 'A' } + } + await getHistoricalConceptsInScheme(event, {}) + + // eslint-disable-next-line no-console + expect(console.error).toHaveBeenCalledWith( + 'Failed to download CSV for scheme="instruments", version="A": Access denied' + ) + }) + }) +}) diff --git a/serverless/src/getHistoricalConceptsInScheme/handler.js b/serverless/src/getHistoricalConceptsInScheme/handler.js new file mode 100644 index 00000000..8936afa0 --- /dev/null +++ b/serverless/src/getHistoricalConceptsInScheme/handler.js @@ -0,0 +1,154 @@ +import { GetObjectCommand, ListObjectsV2Command } from '@aws-sdk/client-s3' + +import { getS3Client } from '@/shared/awsClients' +import { getApplicationConfig } from '@/shared/getConfig' +import { logAnalyticsData } from '@/shared/logAnalyticsData' + +/** + * S3 bucket name to download the concept scheme CSV from + * @type {string} + */ +const bucketName = process.env.S3_BUCKET_NAME || 'kms-rdf-backup-sit' + +/** + * S3 Client from shared configuration + * @type {S3Client} + */ +const s3Client = getS3Client() + +/** + * Lists all object keys directly under a version's S3 prefix. + * + * @param {string} versionPrefix - The "{version}/" prefix to list under. + * @returns {Promise>} Array of full S3 object keys. + */ +const listKeysUnderPrefix = async (versionPrefix) => { + const keys = [] + let continuationToken + + /* eslint-disable no-await-in-loop */ + do { + const command = new ListObjectsV2Command({ + Bucket: bucketName, + Prefix: versionPrefix, + ContinuationToken: continuationToken + }) + + const response = await s3Client.send(command) + + if (response.Contents) { + keys.push(...response.Contents.map((obj) => obj.Key).filter(Boolean)) + } + + continuationToken = response.NextContinuationToken + } while (continuationToken) + /* eslint-enable no-await-in-loop */ + + return keys +} + +/** + * Finds the actual S3 key for a scheme's CSV file under a version prefix, + * matching case-insensitively since scheme file names in S3 may be + * lowercase, CamelCase, or any other casing. + * + * @param {Array} keys - Object keys under the version prefix. + * @param {string} versionPrefix - The "{version}/" prefix the keys live under. + * @param {string} scheme - The lowercased scheme name to match against. + * @returns {string|undefined} The matching S3 key, if found. + */ +const findSchemeKey = (keys, versionPrefix, scheme) => keys.find((key) => { + const fileName = key.slice(versionPrefix.length) + + return fileName.toLowerCase() === `${scheme}.csv` +}) + +export const getHistoricalConceptsInScheme = async (event, context) => { + const { defaultResponseHeaders } = getApplicationConfig() + + logAnalyticsData({ + event, + context + }) + + const { pathParameters, queryStringParameters } = event + const { conceptScheme } = pathParameters || {} + const scheme = conceptScheme?.toLowerCase() + const { version } = queryStringParameters || {} + + if (!scheme) { + return { + statusCode: 400, + headers: { + ...defaultResponseHeaders, + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ error: 'scheme is required' }) + } + } + + if (!version) { + return { + statusCode: 400, + headers: { + ...defaultResponseHeaders, + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ error: 'version is required' }) + } + } + + const versionPrefix = `${version}/` + + try { + const keys = await listKeysUnderPrefix(versionPrefix) + const matchedKey = findSchemeKey(keys, versionPrefix, scheme) + + if (!matchedKey) { + return { + statusCode: 404, + headers: { + ...defaultResponseHeaders, + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ error: `No concept scheme "${scheme}" found for version "${version}"` }) + } + } + + const command = new GetObjectCommand({ + Bucket: bucketName, + Key: matchedKey + }) + + const response = await s3Client.send(command) + + if (!response.Body) { + throw new Error('No data returned from S3') + } + + const csvContent = await response.Body.transformToString() + + return { + statusCode: 200, + headers: { + ...defaultResponseHeaders, + 'Content-Type': 'text/csv; charset=utf-8', + 'Content-Disposition': `attachment; filename="${scheme}.csv"` + }, + body: csvContent + } + } catch (error) { + console.error(`Failed to download CSV for scheme="${scheme}", version="${version}": ${error.message}`) + + return { + statusCode: 500, + headers: { + ...defaultResponseHeaders, + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ error: 'Failed to fetch concept scheme CSV' }) + } + } +} + +export default getHistoricalConceptsInScheme From e92e38079a1938f9d42fdd178cb8b15fd17b68d1 Mon Sep 17 00:00:00 2001 From: Hoan-Vu Tran-Ho Date: Wed, 12 Aug 2026 19:39:09 -0400 Subject: [PATCH 2/8] KMS-598: Add input validation --- .../__tests__/handler.test.js | 24 ++++++++++++++ .../getHistoricalConceptsInScheme/handler.js | 32 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js index 79ff6ee6..3e42832e 100644 --- a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js +++ b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js @@ -69,6 +69,30 @@ describe('getHistoricalConceptsInScheme', () => { expect(response.statusCode).toBe(400) expect(JSON.parse(response.body)).toEqual({ error: 'scheme is required' }) }) + + test('should return 400 when conceptScheme contains invalid characters', async () => { + const event = { + pathParameters: { conceptScheme: 'instruments"; DROP TABLE--' }, + queryStringParameters: { version: 'A' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(400) + expect(JSON.parse(response.body)).toEqual({ error: 'scheme contains invalid characters' }) + expect(mockSend).not.toHaveBeenCalled() + }) + + test('should return 400 when version contains invalid characters', async () => { + const event = { + pathParameters: { conceptScheme: 'instruments' }, + queryStringParameters: { version: 'A\r\nX-Injected: true' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(400) + expect(JSON.parse(response.body)).toEqual({ error: 'version contains invalid characters' }) + expect(mockSend).not.toHaveBeenCalled() + }) }) describe('when successful', () => { diff --git a/serverless/src/getHistoricalConceptsInScheme/handler.js b/serverless/src/getHistoricalConceptsInScheme/handler.js index 8936afa0..822d6aec 100644 --- a/serverless/src/getHistoricalConceptsInScheme/handler.js +++ b/serverless/src/getHistoricalConceptsInScheme/handler.js @@ -10,6 +10,16 @@ import { logAnalyticsData } from '@/shared/logAnalyticsData' */ const bucketName = process.env.S3_BUCKET_NAME || 'kms-rdf-backup-sit' +/** + * Allow-list pattern for the `conceptScheme` and `version` inputs. Both + * values are reflected into the S3 Prefix, the Content-Disposition header, + * and error messages, so they're restricted to characters seen in real + * version/scheme names (letters, digits, dot, hyphen, underscore) to rule + * out header injection or unexpectedly broad S3 prefix listings. + * @type {RegExp} + */ +const SAFE_IDENTIFIER_PATTERN = /^[A-Za-z0-9._-]+$/ + /** * S3 Client from shared configuration * @type {S3Client} @@ -98,6 +108,28 @@ export const getHistoricalConceptsInScheme = async (event, context) => { } } + if (!SAFE_IDENTIFIER_PATTERN.test(scheme)) { + return { + statusCode: 400, + headers: { + ...defaultResponseHeaders, + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ error: 'scheme contains invalid characters' }) + } + } + + if (!SAFE_IDENTIFIER_PATTERN.test(version)) { + return { + statusCode: 400, + headers: { + ...defaultResponseHeaders, + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ error: 'version contains invalid characters' }) + } + } + const versionPrefix = `${version}/` try { From ee63e367b4b5c20684a58b26e597f78d66c0061a Mon Sep 17 00:00:00 2001 From: Hoan-Vu Tran-Ho Date: Fri, 14 Aug 2026 14:08:59 -0400 Subject: [PATCH 3/8] KMS-598: Update error messages --- .../getHistoricalConceptsInScheme/__tests__/handler.test.js | 4 ++-- serverless/src/getHistoricalConceptsInScheme/handler.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js index 3e42832e..0f258724 100644 --- a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js +++ b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js @@ -186,7 +186,7 @@ describe('getHistoricalConceptsInScheme', () => { expect(response.statusCode).toBe(404) expect(JSON.parse(response.body)).toEqual({ - error: 'No concept scheme "doesnotexist" found for version "A"' + error: 'No concept scheme doesnotexist found for version A' }) // Only the list call should happen, never a GetObject @@ -247,7 +247,7 @@ describe('getHistoricalConceptsInScheme', () => { // eslint-disable-next-line no-console expect(console.error).toHaveBeenCalledWith( - 'Failed to download CSV for scheme="instruments", version="A": Access denied' + 'Failed to download CSV for scheme=instruments, version=A: Access denied' ) }) }) diff --git a/serverless/src/getHistoricalConceptsInScheme/handler.js b/serverless/src/getHistoricalConceptsInScheme/handler.js index 822d6aec..9794f717 100644 --- a/serverless/src/getHistoricalConceptsInScheme/handler.js +++ b/serverless/src/getHistoricalConceptsInScheme/handler.js @@ -143,7 +143,7 @@ export const getHistoricalConceptsInScheme = async (event, context) => { ...defaultResponseHeaders, 'Content-Type': 'application/json' }, - body: JSON.stringify({ error: `No concept scheme "${scheme}" found for version "${version}"` }) + body: JSON.stringify({ error: `No concept scheme ${scheme} found for version ${version}` }) } } @@ -170,7 +170,7 @@ export const getHistoricalConceptsInScheme = async (event, context) => { body: csvContent } } catch (error) { - console.error(`Failed to download CSV for scheme="${scheme}", version="${version}": ${error.message}`) + console.error(`Failed to download CSV for scheme=${scheme}, version=${version}: ${error.message}`) return { statusCode: 500, From cb9a6aa3bb2a1c92ab08fa453abcc8b66aaf80ab Mon Sep 17 00:00:00 2001 From: Hoan-Vu Tran-Ho Date: Fri, 14 Aug 2026 14:11:52 -0400 Subject: [PATCH 4/8] KMS-598: Updated error messages --- serverless/src/getHistoricalConceptsInScheme/handler.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/serverless/src/getHistoricalConceptsInScheme/handler.js b/serverless/src/getHistoricalConceptsInScheme/handler.js index 9794f717..6bde98cb 100644 --- a/serverless/src/getHistoricalConceptsInScheme/handler.js +++ b/serverless/src/getHistoricalConceptsInScheme/handler.js @@ -143,7 +143,7 @@ export const getHistoricalConceptsInScheme = async (event, context) => { ...defaultResponseHeaders, 'Content-Type': 'application/json' }, - body: JSON.stringify({ error: `No concept scheme ${scheme} found for version ${version}` }) + body: JSON.stringify({ error: `No concept scheme ${conceptScheme} found for version ${version}` }) } } @@ -165,12 +165,12 @@ export const getHistoricalConceptsInScheme = async (event, context) => { headers: { ...defaultResponseHeaders, 'Content-Type': 'text/csv; charset=utf-8', - 'Content-Disposition': `attachment; filename="${scheme}.csv"` + 'Content-Disposition': `attachment; filename="${conceptScheme}.csv"` }, body: csvContent } } catch (error) { - console.error(`Failed to download CSV for scheme=${scheme}, version=${version}: ${error.message}`) + console.error(`Failed to download CSV for scheme=${conceptScheme}, version=${version}: ${error.message}`) return { statusCode: 500, From abb9e044e5bef1a9a82e1299fa31cb378bd3a920 Mon Sep 17 00:00:00 2001 From: Hoan-Vu Tran-Ho Date: Fri, 14 Aug 2026 20:14:17 -0400 Subject: [PATCH 5/8] KMS-598: Update S3 bucket name, getCapabilities and condition for listing versions --- .../getCapabilities/__tests__/handler.test.js | 2 + serverless/src/getCapabilities/handler.js | 16 ++ .../__tests__/handler.test.js | 147 +++++++++++++++--- .../getHistoricalConceptVersions/handler.js | 59 ++++++- .../__tests__/handler.test.js | 27 +++- .../getHistoricalConceptsInScheme/handler.js | 6 +- 6 files changed, 229 insertions(+), 28 deletions(-) diff --git a/serverless/src/getCapabilities/__tests__/handler.test.js b/serverless/src/getCapabilities/__tests__/handler.test.js index 3f11ddac..25a2650a 100644 --- a/serverless/src/getCapabilities/__tests__/handler.test.js +++ b/serverless/src/getCapabilities/__tests__/handler.test.js @@ -56,6 +56,8 @@ describe('getCapabilities', () => { expect(result.body).toContain(' { params: 'scheme=', action: 'GET' } + }, + { + ':@': { + name: 'get_historical_concept_versions', + href: '/concept_versions/historical', + params: 'None', + action: 'GET' + } + }, + { + ':@': { + name: 'get_historical_concepts_in_scheme', + href: '/concepts/historical/concept_scheme/{conceptScheme}', + params: 'version=', + action: 'GET' + } } ] diff --git a/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js b/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js index d54d76cc..012dad4d 100644 --- a/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js +++ b/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js @@ -15,9 +15,15 @@ import { getHistoricalConceptVersions } from '../handler' // vi.hoisted + a mock factory ensures `mockSend` is available when the // `@/shared/awsClients` mock is set up, and lets us control its behavior // per test via mockSend.mockResolvedValueOnce(...). -const { mockSend } = vi.hoisted(() => ({ - mockSend: vi.fn() -})) +// +// handler.js also now reads `RDF_BUCKET_NAME` at module-load time and throws +// if it's missing, so it must be set here too, before the static import of +// `../handler` below runs (vi.hoisted is lifted above all imports). +const { mockSend } = vi.hoisted(() => { + process.env.RDF_BUCKET_NAME = 'test-bucket' + + return { mockSend: vi.fn() } +}) vi.mock('@/shared/awsClients', () => ({ getS3Client: () => ({ send: mockSend }) @@ -38,13 +44,23 @@ describe('getHistoricalConceptVersions', () => { }) describe('when successful', () => { - test('should return 200 with the list of version directories', async () => { - mockSend.mockResolvedValueOnce({ - CommonPrefixes: [ - { Prefix: 'A/' }, - { Prefix: 'B/' } - ] - }) + test('should return 200 with the list of version directories that contain a CSV', async () => { + mockSend + // Top-level listing: two candidate version prefixes + .mockResolvedValueOnce({ + CommonPrefixes: [ + { Prefix: 'A/' }, + { Prefix: 'B/' } + ] + }) + // Per-version CSV check for "A" + .mockResolvedValueOnce({ + Contents: [{ Key: 'A/ScienceKeywords.csv' }] + }) + // Per-version CSV check for "B" + .mockResolvedValueOnce({ + Contents: [{ Key: 'B/ScienceKeywords.csv' }] + }) const event = {} const context = {} @@ -56,18 +72,56 @@ describe('getHistoricalConceptVersions', () => { }) test('should exclude the draft directory', async () => { - mockSend.mockResolvedValueOnce({ - CommonPrefixes: [ - { Prefix: 'draft/' }, - { Prefix: 'A/' } - ] - }) + mockSend + .mockResolvedValueOnce({ + CommonPrefixes: [ + { Prefix: 'draft/' }, + { Prefix: 'A/' } + ] + }) + .mockResolvedValueOnce({ + Contents: [{ Key: 'A/ScienceKeywords.csv' }] + }) const response = await getHistoricalConceptVersions({}, {}) expect(JSON.parse(response.body)).toEqual({ historicalVersions: ['A'] }) }) + test('should exclude versions that have no CSV files, e.g. an rdf.xml-only or incomplete export', async () => { + mockSend + .mockResolvedValueOnce({ + CommonPrefixes: [ + { Prefix: 'A/' }, + { Prefix: 'B/' } + ] + }) + // "A" only has an rdf.xml export, no CSV + .mockResolvedValueOnce({ + Contents: [{ Key: 'A/rdf.xml' }] + }) + // "B" has a CSV + .mockResolvedValueOnce({ + Contents: [{ Key: 'B/ScienceKeywords.csv' }] + }) + + const response = await getHistoricalConceptVersions({}, {}) + + expect(JSON.parse(response.body)).toEqual({ historicalVersions: ['B'] }) + }) + + test('should exclude a version whose prefix has no objects at all', async () => { + mockSend + .mockResolvedValueOnce({ + CommonPrefixes: [{ Prefix: 'A/' }] + }) + .mockResolvedValueOnce({}) + + const response = await getHistoricalConceptVersions({}, {}) + + expect(JSON.parse(response.body)).toEqual({ historicalVersions: [] }) + }) + test('should return an empty array when there are no common prefixes', async () => { mockSend.mockResolvedValueOnce({}) @@ -75,9 +129,11 @@ describe('getHistoricalConceptVersions', () => { expect(response.statusCode).toBe(200) expect(JSON.parse(response.body)).toEqual({ historicalVersions: [] }) + // No candidate versions, so no per-version CSV checks should happen + expect(mockSend).toHaveBeenCalledTimes(1) }) - test('should paginate through multiple pages of results', async () => { + test('should paginate through multiple pages of top-level results', async () => { mockSend .mockResolvedValueOnce({ CommonPrefixes: [{ Prefix: 'A/' }], @@ -86,13 +142,38 @@ describe('getHistoricalConceptVersions', () => { .mockResolvedValueOnce({ CommonPrefixes: [{ Prefix: 'B/' }] }) + .mockResolvedValueOnce({ + Contents: [{ Key: 'A/ScienceKeywords.csv' }] + }) + .mockResolvedValueOnce({ + Contents: [{ Key: 'B/ScienceKeywords.csv' }] + }) const response = await getHistoricalConceptVersions({}, {}) - expect(mockSend).toHaveBeenCalledTimes(2) + expect(mockSend).toHaveBeenCalledTimes(4) expect(JSON.parse(response.body)).toEqual({ historicalVersions: ['A', 'B'] }) }) + test('should paginate through multiple pages when checking a single version for CSVs', async () => { + mockSend + .mockResolvedValueOnce({ + CommonPrefixes: [{ Prefix: 'A/' }] + }) + .mockResolvedValueOnce({ + Contents: [{ Key: 'A/rdf.xml' }], + NextContinuationToken: 'version-page-2-token' + }) + .mockResolvedValueOnce({ + Contents: [{ Key: 'A/ScienceKeywords.csv' }] + }) + + const response = await getHistoricalConceptVersions({}, {}) + + expect(mockSend).toHaveBeenCalledTimes(3) + expect(JSON.parse(response.body)).toEqual({ historicalVersions: ['A'] }) + }) + test('should call logAnalyticsData with the event and context', async () => { mockSend.mockResolvedValueOnce({ CommonPrefixes: [] }) @@ -132,5 +213,35 @@ describe('getHistoricalConceptVersions', () => { error.message ) }) + + test('should return 500 when checking a version for CSVs fails', async () => { + mockSend + .mockResolvedValueOnce({ + CommonPrefixes: [{ Prefix: 'A/' }] + }) + .mockRejectedValueOnce(new Error('Access denied')) + + const response = await getHistoricalConceptVersions({}, {}) + + expect(response.statusCode).toBe(500) + expect(JSON.parse(response.body)).toEqual({ + message: 'Failed to fetch version directories' + }) + }) + }) + + describe('when RDF_BUCKET_NAME is not set', () => { + test('should throw a clear error at module load instead of silently falling back to a default bucket', async () => { + const originalValue = process.env.RDF_BUCKET_NAME + delete process.env.RDF_BUCKET_NAME + + vi.resetModules() + + await expect(import('../handler')).rejects.toThrow( + 'Missing required environment variable: RDF_BUCKET_NAME' + ) + + process.env.RDF_BUCKET_NAME = originalValue + }) }) }) diff --git a/serverless/src/getHistoricalConceptVersions/handler.js b/serverless/src/getHistoricalConceptVersions/handler.js index ae778705..2bd82be3 100644 --- a/serverless/src/getHistoricalConceptVersions/handler.js +++ b/serverless/src/getHistoricalConceptVersions/handler.js @@ -5,10 +5,14 @@ import { getApplicationConfig } from '@/shared/getConfig' import { logAnalyticsData } from '@/shared/logAnalyticsData' /** - * S3 bucket name to list version directories from + * S3 bucket name to list version directories from. * @type {string} */ -const bucketName = process.env.S3_BUCKET_NAME || 'kms-rdf-backup-sit' +const bucketName = process.env.RDF_BUCKET_NAME + +if (!bucketName) { + throw new Error('Missing required environment variable: RDF_BUCKET_NAME') +} /** * S3 Client from shared configuration @@ -16,15 +20,49 @@ const bucketName = process.env.S3_BUCKET_NAME || 'kms-rdf-backup-sit' */ const s3Client = getS3Client() +/** + * Checks whether a given version "directory" contains at least one + * downloadable CSV file. A version may exist as a top-level prefix while + * only containing an rdf.xml export (or an incomplete export with no CSVs + * at all), so this is used to filter those out. + * + * @param {string} version - Version/directory name, e.g. "1.9.1" + * @returns {Promise} Whether the version contains at least one .csv key + */ +const versionHasCsv = async (version) => { + let continuationToken + + /* eslint-disable no-await-in-loop */ + do { + const command = new ListObjectsV2Command({ + Bucket: bucketName, + Prefix: `${version}/`, + ContinuationToken: continuationToken + }) + + const response = await s3Client.send(command) + + if (response.Contents?.some((object) => object.Key.endsWith('.csv'))) { + return true + } + + continuationToken = response.NextContinuationToken + } while (continuationToken) + /* eslint-enable no-await-in-loop */ + + return false +} + /** * Lists top-level "directory" names in the S3 bucket by using a delimiter * so S3 groups keys into CommonPrefixes (its equivalent of folders), - * excluding the "draft/" prefix. + * excluding the "draft/" prefix, and excluding any version that has no + * downloadable CSV files (e.g. rdf.xml-only or incomplete exports). * * @returns {Promise>} Array of version/directory names */ const listVersionDirectories = async () => { - const historicalVersions = [] + const candidateVersions = [] let continuationToken /* eslint-disable no-await-in-loop */ @@ -44,14 +82,23 @@ const listVersionDirectories = async () => { .map((prefix) => prefix.replace(/\/$/, '')) // Strip trailing slash .filter((name) => name !== 'draft') // Exclude the draft "directory" - historicalVersions.push(...prefixes) + candidateVersions.push(...prefixes) } continuationToken = response.NextContinuationToken } while (continuationToken) /* eslint-enable no-await-in-loop */ - return historicalVersions + const versionCsvChecks = await Promise.all( + candidateVersions.map(async (version) => ({ + version, + hasCsv: await versionHasCsv(version) + })) + ) + + return versionCsvChecks + .filter((result) => result.hasCsv) + .map((result) => result.version) } export const getHistoricalConceptVersions = async (event, context) => { diff --git a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js index 0f258724..baa20f0d 100644 --- a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js +++ b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js @@ -15,9 +15,15 @@ import { getHistoricalConceptsInScheme } from '../handler' // vi.hoisted + a mock factory ensures `mockSend` is available when the // `@/shared/awsClients` mock is set up, and lets us control its behavior // per test via mockSend.mockResolvedValueOnce(...). -const { mockSend } = vi.hoisted(() => ({ - mockSend: vi.fn() -})) +// +// handler.js also reads `RDF_BUCKET_NAME` at module-load time and throws if +// it's missing, so it must be set here too, before the static import of +// `../handler` below runs (vi.hoisted is lifted above all imports). +const { mockSend } = vi.hoisted(() => { + process.env.RDF_BUCKET_NAME = 'test-bucket' + + return { mockSend: vi.fn() } +}) vi.mock('@/shared/awsClients', () => ({ getS3Client: () => ({ send: mockSend }) @@ -251,4 +257,19 @@ describe('getHistoricalConceptsInScheme', () => { ) }) }) + + describe('when RDF_BUCKET_NAME is not set', () => { + test('should throw a clear error at module load instead of silently falling back to a default bucket', async () => { + const originalValue = process.env.RDF_BUCKET_NAME + delete process.env.RDF_BUCKET_NAME + + vi.resetModules() + + await expect(import('../handler')).rejects.toThrow( + 'Missing required environment variable: RDF_BUCKET_NAME' + ) + + process.env.RDF_BUCKET_NAME = originalValue + }) + }) }) diff --git a/serverless/src/getHistoricalConceptsInScheme/handler.js b/serverless/src/getHistoricalConceptsInScheme/handler.js index 6bde98cb..db697233 100644 --- a/serverless/src/getHistoricalConceptsInScheme/handler.js +++ b/serverless/src/getHistoricalConceptsInScheme/handler.js @@ -8,7 +8,11 @@ import { logAnalyticsData } from '@/shared/logAnalyticsData' * S3 bucket name to download the concept scheme CSV from * @type {string} */ -const bucketName = process.env.S3_BUCKET_NAME || 'kms-rdf-backup-sit' +const bucketName = process.env.RDF_BUCKET_NAME + +if (!bucketName) { + throw new Error('Missing required environment variable: RDF_BUCKET_NAME') +} /** * Allow-list pattern for the `conceptScheme` and `version` inputs. Both From 0ab1dbe1eae1bee5170a336a9711e16844237d8b Mon Sep 17 00:00:00 2001 From: Hoan-Vu Tran-Ho Date: Fri, 14 Aug 2026 20:30:07 -0400 Subject: [PATCH 6/8] KMS-598: Use pagination --- .../__tests__/handler.test.js | 17 ++- .../getHistoricalConceptVersions/handler.js | 101 ++++++++++-------- .../__tests__/handler.test.js | 9 -- 3 files changed, 62 insertions(+), 65 deletions(-) diff --git a/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js b/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js index 012dad4d..c8344c23 100644 --- a/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js +++ b/serverless/src/getHistoricalConceptVersions/__tests__/handler.test.js @@ -1,3 +1,4 @@ +import { S3Client } from '@aws-sdk/client-s3' import { beforeEach, describe, @@ -10,15 +11,6 @@ import { logAnalyticsData } from '@/shared/logAnalyticsData' import { getHistoricalConceptVersions } from '../handler' -// `getS3Client()` is called once at module-load time in handler.js, so the -// mocked client needs to exist before the handler module is imported. Using -// vi.hoisted + a mock factory ensures `mockSend` is available when the -// `@/shared/awsClients` mock is set up, and lets us control its behavior -// per test via mockSend.mockResolvedValueOnce(...). -// -// handler.js also now reads `RDF_BUCKET_NAME` at module-load time and throws -// if it's missing, so it must be set here too, before the static import of -// `../handler` below runs (vi.hoisted is lifted above all imports). const { mockSend } = vi.hoisted(() => { process.env.RDF_BUCKET_NAME = 'test-bucket' @@ -26,7 +18,12 @@ const { mockSend } = vi.hoisted(() => { }) vi.mock('@/shared/awsClients', () => ({ - getS3Client: () => ({ send: mockSend }) + getS3Client: () => { + const client = new S3Client({ region: 'us-east-1' }) + client.send = mockSend + + return client + } })) vi.mock('@/shared/getConfig') diff --git a/serverless/src/getHistoricalConceptVersions/handler.js b/serverless/src/getHistoricalConceptVersions/handler.js index 2bd82be3..14fd9c17 100644 --- a/serverless/src/getHistoricalConceptVersions/handler.js +++ b/serverless/src/getHistoricalConceptVersions/handler.js @@ -1,4 +1,4 @@ -import { ListObjectsV2Command } from '@aws-sdk/client-s3' +import { paginateListObjectsV2 } from '@aws-sdk/client-s3' import { getS3Client } from '@/shared/awsClients' import { getApplicationConfig } from '@/shared/getConfig' @@ -20,6 +20,43 @@ if (!bucketName) { */ const s3Client = getS3Client() +/** + * Lists candidate version "directory" names in the S3 bucket by using a + * delimiter so S3 groups keys into CommonPrefixes (its equivalent of + * folders), excluding the "draft/" prefix. + * + * @returns {Promise>} Array of candidate version/directory names + */ +const listCandidateVersions = async () => { + const candidateVersions = [] + + const paginator = paginateListObjectsV2( + { + client: s3Client, + pageSize: 1000 + }, + { + Bucket: bucketName, + Delimiter: '/' + } + ) + + // eslint-disable-next-line no-restricted-syntax + for await (const page of paginator) { + if (page.CommonPrefixes) { + const prefixes = page.CommonPrefixes + .map((p) => p.Prefix) + .filter(Boolean) + .map((prefix) => prefix.replace(/\/$/, '')) // Strip trailing slash + .filter((name) => name !== 'draft') // Exclude the draft "directory" + + candidateVersions.push(...prefixes) + } + } + + return candidateVersions +} + /** * Checks whether a given version "directory" contains at least one * downloadable CSV file. A version may exist as a top-level prefix while @@ -30,64 +67,36 @@ const s3Client = getS3Client() * @returns {Promise} Whether the version contains at least one .csv key */ const versionHasCsv = async (version) => { - let continuationToken - - /* eslint-disable no-await-in-loop */ - do { - const command = new ListObjectsV2Command({ + const paginator = paginateListObjectsV2( + { + client: s3Client, + pageSize: 1000 + }, + { Bucket: bucketName, - Prefix: `${version}/`, - ContinuationToken: continuationToken - }) - - const response = await s3Client.send(command) + Prefix: `${version}/` + } + ) - if (response.Contents?.some((object) => object.Key.endsWith('.csv'))) { + // eslint-disable-next-line no-restricted-syntax + for await (const page of paginator) { + if (page.Contents?.some((object) => object.Key.endsWith('.csv'))) { return true } - - continuationToken = response.NextContinuationToken - } while (continuationToken) - /* eslint-enable no-await-in-loop */ + } return false } /** - * Lists top-level "directory" names in the S3 bucket by using a delimiter - * so S3 groups keys into CommonPrefixes (its equivalent of folders), - * excluding the "draft/" prefix, and excluding any version that has no - * downloadable CSV files (e.g. rdf.xml-only or incomplete exports). + * Lists top-level "directory" names in the S3 bucket, excluding the + * "draft/" prefix, and excluding any version that has no downloadable CSV + * files (e.g. rdf.xml-only or incomplete exports). * * @returns {Promise>} Array of version/directory names */ const listVersionDirectories = async () => { - const candidateVersions = [] - let continuationToken - - /* eslint-disable no-await-in-loop */ - do { - const command = new ListObjectsV2Command({ - Bucket: bucketName, - Delimiter: '/', - ContinuationToken: continuationToken - }) - - const response = await s3Client.send(command) - - if (response.CommonPrefixes) { - const prefixes = response.CommonPrefixes - .map((p) => p.Prefix) - .filter(Boolean) - .map((prefix) => prefix.replace(/\/$/, '')) // Strip trailing slash - .filter((name) => name !== 'draft') // Exclude the draft "directory" - - candidateVersions.push(...prefixes) - } - - continuationToken = response.NextContinuationToken - } while (continuationToken) - /* eslint-enable no-await-in-loop */ + const candidateVersions = await listCandidateVersions() const versionCsvChecks = await Promise.all( candidateVersions.map(async (version) => ({ diff --git a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js index baa20f0d..6bd1d859 100644 --- a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js +++ b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js @@ -10,15 +10,6 @@ import { logAnalyticsData } from '@/shared/logAnalyticsData' import { getHistoricalConceptsInScheme } from '../handler' -// GetS3Client() is called once at module-load time in handler.js, so the -// mocked client needs to exist before the handler module is imported. Using -// vi.hoisted + a mock factory ensures `mockSend` is available when the -// `@/shared/awsClients` mock is set up, and lets us control its behavior -// per test via mockSend.mockResolvedValueOnce(...). -// -// handler.js also reads `RDF_BUCKET_NAME` at module-load time and throws if -// it's missing, so it must be set here too, before the static import of -// `../handler` below runs (vi.hoisted is lifted above all imports). const { mockSend } = vi.hoisted(() => { process.env.RDF_BUCKET_NAME = 'test-bucket' From 25348dc1ed01702c8d201ba1afb76822b253e7b9 Mon Sep 17 00:00:00 2001 From: Hoan-Vu Tran-Ho Date: Sat, 15 Aug 2026 01:25:03 -0400 Subject: [PATCH 7/8] KMS-598: Add test --- .../__tests__/handler.test.js | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js index 6bd1d859..75902594 100644 --- a/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js +++ b/serverless/src/getHistoricalConceptsInScheme/__tests__/handler.test.js @@ -232,6 +232,29 @@ describe('getHistoricalConceptsInScheme', () => { expect(JSON.parse(response.body)).toEqual({ error: 'Failed to fetch concept scheme CSV' }) }) + test('should return 500 when the matched object has no Body', async () => { + mockSend.mockResolvedValueOnce({ + Contents: [{ Key: 'A/instruments.csv' }] + }) + + // GetObject resolves, but with no Body on the response + mockSend.mockResolvedValueOnce({}) + + const event = { + pathParameters: { conceptScheme: 'instruments' }, + queryStringParameters: { version: 'A' } + } + const response = await getHistoricalConceptsInScheme(event, {}) + + expect(response.statusCode).toBe(500) + expect(JSON.parse(response.body)).toEqual({ error: 'Failed to fetch concept scheme CSV' }) + + // eslint-disable-next-line no-console + expect(console.error).toHaveBeenCalledWith( + 'Failed to download CSV for scheme=instruments, version=A: No data returned from S3' + ) + }) + test('should log the error to the console', async () => { const error = new Error('Access denied') mockSend.mockRejectedValueOnce(error) From a7d1d7e25296ab9d1f424fbf512e3f8125a7598d Mon Sep 17 00:00:00 2001 From: Hoan-Vu Tran-Ho Date: Fri, 21 Aug 2026 11:43:06 -0400 Subject: [PATCH 8/8] KMS-598: Use RDF_BUCKET_NAME from environment --- cdk/app/lib/KmsStack.ts | 3 ++- cdk/app/lib/helper/IamSetup.ts | 16 +++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/cdk/app/lib/KmsStack.ts b/cdk/app/lib/KmsStack.ts index 3aa1cb24..0d9173d0 100644 --- a/cdk/app/lib/KmsStack.ts +++ b/cdk/app/lib/KmsStack.ts @@ -118,7 +118,8 @@ export class KmsStack extends cdk.Stack { this.stage, this.account, this.region, - this.stackName + this.stackName, + environment.RDF_BUCKET_NAME ) this.lambdaRole = iamSetup.lambdaRole diff --git a/cdk/app/lib/helper/IamSetup.ts b/cdk/app/lib/helper/IamSetup.ts index 0ae5f1a2..bde4d771 100644 --- a/cdk/app/lib/helper/IamSetup.ts +++ b/cdk/app/lib/helper/IamSetup.ts @@ -22,7 +22,8 @@ export class IamSetup { stage: string, account: string, region: string, - stackName: string + stackName: string, + rdfBucketName: string ) { this.lambdaRole = new iam.Role(scope, `${prefix}-KmsServerlessAppRole`, { assumedBy: new iam.CompositePrincipal( @@ -41,7 +42,7 @@ export class IamSetup { ) }) - this.addPolicies(prefix, stage, account, region, stackName) + this.addPolicies(prefix, stage, account, region, stackName, rdfBucketName) } /** @@ -80,9 +81,10 @@ export class IamSetup { stage: string, account: string, region: string, - stackName: string + stackName: string, + rdfBucketName: string ) { - this.addS3AccessPolicy(prefix, stage) + this.addS3AccessPolicy(prefix, stage, rdfBucketName) this.addKMSLambdaBasePolicy() this.addLambdaInvocationPolicy(prefix, region, account, stage, stackName) this.addServiceDiscoveryPolicy(prefix, stage) @@ -94,7 +96,7 @@ export class IamSetup { * @param {string} prefix - The prefix to use for naming resources. * @param {string} stage - The deployment stage. */ - private addS3AccessPolicy(prefix: string, stage: string) { + private addS3AccessPolicy(prefix: string, stage: string, rdfBucketName: string) { this.lambdaRole.addToPolicy(new iam.PolicyStatement({ actions: [ 's3:CreateBucket', @@ -107,8 +109,8 @@ export class IamSetup { 's3:HeadBucket' ], resources: [ - `arn:aws:s3:::kms-rdf-backup-${stage}`, - `arn:aws:s3:::kms-rdf-backup-${stage}/*` + `arn:aws:s3:::${rdfBucketName}`, + `arn:aws:s3:::${rdfBucketName}/*` ] })) }