Skip to content

Commit 855d210

Browse files
committed
CCM-13304: Use named query
1 parent 11823b3 commit 855d210

File tree

11 files changed

+135
-90
lines changed

11 files changed

+135
-90
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
resource "aws_athena_named_query" "daily_report" {
2+
name = "daily_report"
3+
workgroup = aws_athena_workgroup.reporting.id
4+
database = aws_glue_catalog_database.reporting.name
5+
query = file("${path.module}/scripts/sql/reports/daily_report.sql")
6+
}

infrastructure/terraform/components/dl/module_lambda_report_generator.tf

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ module "report_generator" {
3535
log_subscription_role_arn = local.acct.log_subscription_role_arn
3636

3737
lambda_env_vars = {
38-
"ATHENA_WORKGROUP" = aws_athena_workgroup.reporting.name
39-
"ATHENA_DATABASE" = aws_glue_catalog_database.reporting.name
38+
"ATHENA_NAMED_QUERY_ID" = aws_athena_named_query.daily_report.name
4039
"EVENT_PUBLISHER_EVENT_BUS_ARN" = aws_cloudwatch_event_bus.main.arn
4140
"EVENT_PUBLISHER_DLQ_URL" = module.sqs_event_publisher_errors.sqs_queue_url
4241
"MAX_POLL_LIMIT" = var.athena_query_max_polling_attempts
@@ -153,17 +152,4 @@ data "aws_iam_policy_document" "report_generator_lambda" {
153152
module.sqs_event_publisher_errors.sqs_queue_arn,
154153
]
155154
}
156-
157-
statement {
158-
sid = "SQSDLQPermissions"
159-
effect = "Allow"
160-
161-
actions = [
162-
"sqs:SendMessage",
163-
]
164-
165-
resources = [
166-
module.sqs_report_generator.sqs_dlq_arn,
167-
]
168-
}
169155
}

lambdas/report-generator/src/queries/report.sql renamed to infrastructure/terraform/components/dl/script/sql/reports/daily_report.sql

File renamed without changes.

lambdas/report-generator/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
{
22
"dependencies": {
33
"@aws-sdk/client-athena": "^3.984.0",
4-
"@aws-sdk/client-sqs": "^3.984.0",
54
"digital-letters-events": "^0.0.1",
65
"utils": "^0.0.1"
76
},
@@ -15,7 +14,7 @@
1514
"name": "nhs-notify-digital-letters-report-generator",
1615
"private": true,
1716
"scripts": {
18-
"lambda-build": "rm -rf dist && npx esbuild --bundle --minify --sourcemap --target=es2020 --platform=node --loader:.node=file --entry-names=[name] --outdir=dist src/index.ts && cp -r src/queries dist/",
17+
"lambda-build": "rm -rf dist && npx esbuild --bundle --minify --sourcemap --target=es2020 --platform=node --loader:.node=file --entry-names=[name] --outdir=dist src/index.ts",
1918
"lint": "eslint .",
2019
"lint:fix": "eslint . --fix",
2120
"test:unit": "jest",

lambdas/report-generator/src/__tests__/app/report-generator.test.ts

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ describe('ReportGenerator', () => {
1010
let mockReportService: jest.Mocked<IReportService>;
1111
let reportGenerator: ReportGenerator;
1212
const reportName = 'completed_communications';
13+
const athenaNamedQueryId = 'test-athena-named-query-id';
1314

1415
beforeEach(() => {
1516
mockLogger = {
@@ -27,6 +28,7 @@ describe('ReportGenerator', () => {
2728
mockLogger,
2829
mockReportService,
2930
reportName,
31+
athenaNamedQueryId,
3032
);
3133

3234
jest.clearAllMocks();
@@ -64,15 +66,16 @@ describe('ReportGenerator', () => {
6466

6567
const result = await reportGenerator.generate(mockEvent);
6668

67-
expect(fs.readFileSync).toHaveBeenCalledWith(
68-
'/var/task/queries/report.sql',
69-
'utf8',
70-
);
71-
expect(mockLogger.info).toHaveBeenCalledWith(
72-
'Generating report for sender sender-123 and date 2025-01-15',
73-
);
69+
expect(mockLogger.info).toHaveBeenCalledWith({
70+
description: 'Generating report',
71+
senderId: 'sender-123',
72+
reportDate: '2025-01-15',
73+
athenaNamedQueryId,
74+
reportFilePath:
75+
'event-reports/sender-123/completed_communications/completed_communications_2025-01-15.csv',
76+
});
7477
expect(mockReportService.generateReport).toHaveBeenCalledWith(
75-
mockQuery,
78+
athenaNamedQueryId,
7679
["'2025-01-15'", "'sender-123'"],
7780
'event-reports/sender-123/completed_communications/completed_communications_2025-01-15.csv',
7881
);
@@ -98,24 +101,5 @@ describe('ReportGenerator', () => {
98101
outcome: 'failed',
99102
});
100103
});
101-
102-
it('should return failed outcome when file read throws error', async () => {
103-
const error = new Error('File not found');
104-
(fs.readFileSync as jest.Mock).mockImplementation(() => {
105-
throw error;
106-
});
107-
108-
const result = await reportGenerator.generate(mockEvent);
109-
110-
expect(mockLogger.error).toHaveBeenCalledWith({
111-
err: error,
112-
description: 'Error generating report',
113-
senderId: 'sender-123',
114-
reportDate: '2025-01-15',
115-
});
116-
expect(result).toEqual({
117-
outcome: 'failed',
118-
});
119-
});
120104
});
121105
});

lambdas/report-generator/src/app/report-generator.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { IReportService, Logger } from 'utils';
22
import { GenerateReport } from 'digital-letters-events';
3-
import fs from 'node:fs';
43

54
export type ReportGeneratorOutcome = 'generated' | 'failed';
65

@@ -18,21 +17,25 @@ export class ReportGenerator {
1817
private readonly logger: Logger,
1918
private readonly reportService: IReportService,
2019
private readonly reportName: string,
20+
private readonly athenaNamedQueryId: string,
2121
) {}
2222

2323
async generate(event: GenerateReport): Promise<ReportGeneratorResult> {
2424
const { reportDate, senderId } = event.data;
2525

2626
try {
27-
const query = fs.readFileSync('/var/task/queries/report.sql', 'utf8');
2827
const reportFilePath = `event-reports/${senderId}/${this.reportName}/${this.reportName}_${reportDate}.csv`;
2928

30-
this.logger.info(
31-
`Generating report for sender ${senderId} and date ${reportDate}`,
32-
);
29+
this.logger.info({
30+
description: 'Generating report',
31+
senderId,
32+
reportDate,
33+
athenaNamedQueryId: this.athenaNamedQueryId,
34+
reportFilePath,
35+
});
3336

3437
const location = await this.reportService.generateReport(
35-
query,
38+
this.athenaNamedQueryId,
3639
[`'${reportDate}'`, `'${senderId}'`],
3740
reportFilePath,
3841
);

lambdas/report-generator/src/container.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ import { AthenaClient } from '@aws-sdk/client-athena';
1616

1717
export const createContainer = () => {
1818
const {
19-
athenaDatabase,
20-
athenaWorkgroup,
19+
athenaNamedQueryId,
2120
eventPublisherDlqUrl,
2221
eventPublisherEventBusArn,
2322
maxPollLimit,
@@ -32,8 +31,6 @@ export const createContainer = () => {
3231

3332
const dataRepositoryDependencies: AthenaDataRepositoryDependencies = {
3433
athenaClient,
35-
athenaDatabase,
36-
athenaWorkgroup,
3734
};
3835

3936
const dataRepository = new AthenaDataRepository(dataRepositoryDependencies);
@@ -56,6 +53,7 @@ export const createContainer = () => {
5653
logger,
5754
reportService,
5855
reportName,
56+
athenaNamedQueryId,
5957
);
6058

6159
const eventPublisher = new EventPublisher({

lambdas/report-generator/src/infra/config.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { defaultConfigReader } from 'utils';
22

33
export type ReportGeneratorConfig = {
4-
athenaWorkgroup: string;
5-
athenaDatabase: string;
4+
athenaNamedQueryId: string;
65
eventPublisherEventBusArn: string;
76
eventPublisherDlqUrl: string;
87
maxPollLimit: number;
@@ -13,8 +12,7 @@ export type ReportGeneratorConfig = {
1312

1413
export function loadConfig(): ReportGeneratorConfig {
1514
return {
16-
athenaWorkgroup: defaultConfigReader.getValue('ATHENA_WORKGROUP'),
17-
athenaDatabase: defaultConfigReader.getValue('ATHENA_DATABASE'),
15+
athenaNamedQueryId: defaultConfigReader.getValue('ATHENA_NAMED_QUERY_ID'),
1816
eventPublisherEventBusArn: defaultConfigReader.getValue(
1917
'EVENT_PUBLISHER_EVENT_BUS_ARN',
2018
),

utils/utils/src/__tests__/reporting/data-repository.test.ts

Lines changed: 68 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { AthenaClient } from '@aws-sdk/client-athena';
1+
import {
2+
AthenaClient,
3+
GetNamedQueryCommand,
4+
StartQueryExecutionCommand,
5+
} from '@aws-sdk/client-athena';
26
import { mockClient } from 'aws-sdk-client-mock';
37
import {
48
AthenaDataRepository,
@@ -11,8 +15,6 @@ describe('AthenaDataRepository', () => {
1115
let repository: AthenaDataRepository;
1216
const config: AthenaDataRepositoryDependencies = {
1317
athenaClient: new AthenaClient({}),
14-
athenaWorkgroup: 'test-workgroup',
15-
athenaDatabase: 'test-database',
1618
};
1719

1820
beforeEach(() => {
@@ -23,11 +25,19 @@ describe('AthenaDataRepository', () => {
2325
describe('startQuery', () => {
2426
it('should start query execution and return query execution ID', async () => {
2527
const mockQueryExecutionId = 'query-123';
26-
athenaClientMock.onAnyCommand().resolves({
28+
athenaClientMock.on(GetNamedQueryCommand).resolves({
29+
NamedQuery: {
30+
Name: 'Test Named Query',
31+
Database: 'test-database',
32+
WorkGroup: 'test-workgroup',
33+
QueryString: 'SELECT * FROM table',
34+
},
35+
});
36+
athenaClientMock.on(StartQueryExecutionCommand).resolves({
2737
QueryExecutionId: mockQueryExecutionId,
2838
});
2939

30-
const result = await repository.startQuery('SELECT * FROM table', [
40+
const result = await repository.startQuery('testNamedQueryId', [
3141
'param1',
3242
'param2',
3343
]);
@@ -38,32 +48,74 @@ describe('AthenaDataRepository', () => {
3848
it('should send correct parameters to Athena client', async () => {
3949
const query = 'SELECT * FROM table WHERE id = ?';
4050
const executionParameters = ['123'];
51+
const namedQueryId = 'testNamedQueryId';
52+
const testDatabase = 'test-database';
53+
const testWorkGroup = 'test-workgroup';
4154

42-
athenaClientMock.onAnyCommand().resolves({
43-
QueryExecutionId: 'query-456',
55+
const mockQueryExecutionId = 'query-123';
56+
athenaClientMock.on(GetNamedQueryCommand).resolves({
57+
NamedQuery: {
58+
Name: 'Test Named Query',
59+
Database: testDatabase,
60+
WorkGroup: testWorkGroup,
61+
QueryString: query,
62+
},
63+
});
64+
athenaClientMock.on(StartQueryExecutionCommand).resolves({
65+
QueryExecutionId: mockQueryExecutionId,
4466
});
4567

46-
await repository.startQuery(query, executionParameters);
68+
await repository.startQuery(namedQueryId, executionParameters);
4769

4870
const calls = athenaClientMock.commandCalls(
49-
Object.getPrototypeOf(athenaClientMock.calls()[0].args[0]).constructor,
71+
Object.getPrototypeOf(athenaClientMock.calls()[1].args[0]).constructor,
5072
);
5173
expect(calls[0].args[0].input).toEqual({
5274
QueryString: query,
53-
WorkGroup: 'test-workgroup',
54-
QueryExecutionContext: { Database: 'test-database' },
75+
WorkGroup: testWorkGroup,
76+
QueryExecutionContext: { Database: testDatabase },
5577
ExecutionParameters: executionParameters,
5678
});
5779
});
5880

59-
it('should handle empty execution parameters', async () => {
60-
athenaClientMock.onAnyCommand().resolves({
61-
QueryExecutionId: 'query-789',
81+
it('should throw an error if named query is not found', async () => {
82+
athenaClientMock.on(GetNamedQueryCommand).resolves({});
83+
84+
await expect(
85+
repository.startQuery('nonExistentNamedQuery', ['param']),
86+
).rejects.toThrow(
87+
'Named query nonExistentNamedQuery not found or has no SQL.',
88+
);
89+
});
90+
91+
it('should throw an error if named query does not specify a database', async () => {
92+
athenaClientMock.on(GetNamedQueryCommand).resolves({
93+
NamedQuery: {
94+
WorkGroup: 'test-workgroup',
95+
QueryString: 'SELECT 1',
96+
} as any,
6297
});
6398

64-
const result = await repository.startQuery('SELECT 1', []);
99+
await expect(
100+
repository.startQuery('namedQueryWithoutDatabase', ['param']),
101+
).rejects.toThrow(
102+
'Named query namedQueryWithoutDatabase does not specify a database.',
103+
);
104+
});
105+
106+
it('should throw an error if named query does not specify a workgroup', async () => {
107+
athenaClientMock.on(GetNamedQueryCommand).resolves({
108+
NamedQuery: {
109+
Database: 'test-database',
110+
QueryString: 'SELECT 1',
111+
} as any,
112+
});
65113

66-
expect(result).toBe('query-789');
114+
await expect(
115+
repository.startQuery('namedQueryWithoutWorkgroup', ['param']),
116+
).rejects.toThrow(
117+
'Named query namedQueryWithoutWorkgroup does not specify a workgroup.',
118+
);
67119
});
68120

69121
it('should propagate Athena client errors', async () => {

0 commit comments

Comments
 (0)