Skip to content

Commit c7873b5

Browse files
danieljchuserdanieljchuser
committed
change result to object
1 parent 1161c42 commit c7873b5

File tree

2 files changed

+9
-206
lines changed

2 files changed

+9
-206
lines changed

‎src/controllers/paid/pta2.js‎

Lines changed: 8 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,15 @@ import {
1515
notFound,
1616
forbidden,
1717
badRequest,
18-
found,
1918
} from '@adobe/spacecat-shared-http-utils';
2019
import {
2120
AWSAthenaClient, getPTASummaryQuery, PTASummaryResponseDto,
2221
} from '@adobe/spacecat-shared-athena-client';
2322
import crypto from 'crypto';
2423
import AccessControlUtil from '../../support/access-control-util.js';
25-
import {
26-
getS3CachedResult,
27-
addResultJsonToCache,
28-
fileExists,
29-
getSignedUrlWithRetries,
30-
} from './caching-helper.js';
31-
32-
function getCacheKey(siteId, query, cacheLocation) {
33-
const outPrefix = crypto.createHash('md5').update(query).digest('hex');
34-
const cacheKey = `${cacheLocation}/${siteId}/${outPrefix}.json`;
35-
return { cacheKey, outPrefix };
24+
25+
function getOutPrefix(query) {
26+
return crypto.createHash('md5').update(query).digest('hex');
3627
}
3728

3829
function validateTemporalParams({ year, week, month }) {
@@ -80,10 +71,8 @@ function validateTemporalParams({ year, week, month }) {
8071
}
8172
}
8273

83-
const isTrue = (value) => value === true || value === 'true';
84-
8574
function PTA2Controller(context, log, env) {
86-
const { dataAccess, s3 } = context;
75+
const { dataAccess } = context;
8776
const { Site } = dataAccess;
8877

8978
const {
@@ -94,26 +83,9 @@ function PTA2Controller(context, log, env) {
9483

9584
// constants
9685
const ATHENA_TEMP_FOLDER = `s3://${bucketName}/rum-metrics-compact/temp/out`;
97-
const CACHE_LOCATION = `s3://${bucketName}/rum-metrics-compact/cache`;
98-
99-
async function tryGetCacheResult(siteId, query, noCache) {
100-
const { cacheKey, outPrefix } = getCacheKey(siteId, query, CACHE_LOCATION);
101-
if (isTrue(noCache)) {
102-
return { cachedResultUrl: null, cacheKey, outPrefix };
103-
}
104-
const maxAttempts = 1;
105-
if (await fileExists(s3, cacheKey, log, maxAttempts)) {
106-
const ignoreNotFound = true;
107-
const cachedUrl = await getS3CachedResult(s3, cacheKey, log, ignoreNotFound);
108-
return { cachedResultUrl: cachedUrl, cacheKey, outPrefix };
109-
}
110-
log.info(`Cached result for file: ${cacheKey} does not exist`);
111-
return { cachedResultUrl: null, cacheKey, outPrefix };
112-
}
11386

11487
async function getPTAWeeklySummary() {
11588
/* c8 ignore next 1 */
116-
const requestId = context.invocation?.requestId;
11789
const siteId = context.params?.siteId;
11890
const site = await Site.findById(siteId);
11991
if (!site) {
@@ -127,7 +99,7 @@ function PTA2Controller(context, log, env) {
12799

128100
// validate input params
129101
const {
130-
year, week, month, noCache,
102+
year, week, month,
131103
} = context.data;
132104

133105
const temporal = validateTemporalParams({ year, week, month });
@@ -138,7 +110,6 @@ function PTA2Controller(context, log, env) {
138110
const { yearInt, weekInt, monthInt } = temporal.values;
139111

140112
const tableName = `${rumMetricsDatabase}.${rumMetricsCompactTable}`;
141-
142113
const description = `fetch PTA2 Weekly Summary data db: ${rumMetricsDatabase}| siteKey: ${siteId} | year: ${year} | month: ${month} | week: ${week} } `;
143114

144115
// build query
@@ -150,17 +121,7 @@ function PTA2Controller(context, log, env) {
150121
tableName,
151122
});
152123

153-
// first try to get from cache
154-
const { cachedResultUrl, cacheKey, outPrefix } = await tryGetCacheResult(
155-
siteId,
156-
query,
157-
noCache,
158-
);
159-
160-
if (cachedResultUrl) {
161-
log.info(`Successfully fetched presigned URL for cached result file: ${cacheKey}. Request ID: ${requestId}`);
162-
return found(cachedResultUrl);
163-
}
124+
const outPrefix = getOutPrefix(query);
164125

165126
// if not cached, query Athena
166127
const resultLocation = `${ATHENA_TEMP_FOLDER}/${outPrefix}`;
@@ -169,27 +130,9 @@ function PTA2Controller(context, log, env) {
169130
const results = await athenaClient.query(query, rumMetricsDatabase, description);
170131
const response = results.map((row) => PTASummaryResponseDto.toJSON(row));
171132

172-
// add to cache
173-
let isCached = false;
174-
if (response && response.length > 0) {
175-
isCached = await addResultJsonToCache(s3, cacheKey, response, log);
176-
log.info(`Athena result JSON to S3 cache (${cacheKey}) successful: ${isCached}`);
177-
}
178-
179-
if (isCached) {
180-
// even though file is saved 503 are possible in short time window,
181-
// verifying file is reachable before returning
182-
const verifiedSignedUrl = await getSignedUrlWithRetries(s3, cacheKey, log, 5);
183-
if (verifiedSignedUrl != null) {
184-
log.debug(`Successfully verified file existence, returning signedUrl from key: ${isCached}. Request ID: ${requestId}`);
185-
return found(
186-
verifiedSignedUrl,
187-
);
188-
}
189-
}
133+
const summary = response?.length ? response[0] : null;
190134

191-
log.warn(`Failed to return cache key ${cacheKey}. Returning response directly. Request ID: ${requestId}`);
192-
return ok(response, {
135+
return ok(summary, {
193136
'content-encoding': 'gzip',
194137
});
195138
}

‎test/controllers/paid/pta2.test.js‎

Lines changed: 1 addition & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -174,22 +174,6 @@ describe('PTA2Controller', () => {
174174
expect(body.message).to.equal('Month must be a valid number');
175175
});
176176

177-
it('returns 302 with cached result if cache exists', async () => {
178-
mockS3.send.callsFake((cmd) => {
179-
if (cmd.constructor && cmd.constructor.name === 'HeadObjectCommand') {
180-
// Simulate cache exists
181-
return Promise.resolve({});
182-
}
183-
return Promise.resolve({});
184-
});
185-
186-
const controller = PTA2Controller(mockContext, mockLog, mockEnv);
187-
const res = await controller.getPTAWeeklySummary();
188-
expect(res.status).to.equal(302);
189-
expect(res.headers.get('location')).to.equal(TEST_PRESIGNED_URL);
190-
expect(mockAthenaQuery).not.to.have.been.called;
191-
});
192-
193177
it('queries Athena and returns 200 with fresh data on cache miss', async () => {
194178
const mockAthenaResults = [
195179
{
@@ -205,7 +189,6 @@ describe('PTA2Controller', () => {
205189
const res = await controller.getPTAWeeklySummary();
206190
expect(res.status).to.equal(200);
207191
expect(mockAthenaQuery).to.have.been.calledOnce;
208-
expect(lastPutObject).to.exist;
209192
});
210193

211194
it('bypasses cache when noCache is true', async () => {
@@ -252,71 +235,7 @@ describe('PTA2Controller', () => {
252235
const gzippedBuffer = Buffer.from(await res.arrayBuffer());
253236
const decompressed = await gunzipAsync(gzippedBuffer);
254237
const body = JSON.parse(decompressed.toString());
255-
expect(body).to.deep.equal([]);
256-
});
257-
258-
it('returns 302 when cache is successfully verified after being created', async () => {
259-
let cacheExists = false;
260-
mockS3.send.callsFake((cmd) => {
261-
if (cmd.constructor && cmd.constructor.name === 'HeadObjectCommand') {
262-
if (cacheExists) {
263-
return Promise.resolve({});
264-
}
265-
const err = new Error('not found');
266-
err.name = 'NotFound';
267-
return Promise.reject(err);
268-
}
269-
if (cmd.constructor && cmd.constructor.name === 'PutObjectCommand') {
270-
lastPutObject = cmd;
271-
cacheExists = true;
272-
return Promise.resolve({});
273-
}
274-
return Promise.resolve({});
275-
});
276-
277-
const mockAthenaResults = [
278-
{
279-
year: 2024,
280-
week: 23,
281-
pageviews: 1000,
282-
},
283-
];
284-
mockAthenaQuery.resolves(mockAthenaResults);
285-
286-
const controller = PTA2Controller(mockContext, mockLog, mockEnv);
287-
const res = await controller.getPTAWeeklySummary();
288-
expect(res.status).to.equal(302);
289-
expect(res.headers.get('location')).to.equal(TEST_PRESIGNED_URL);
290-
expect(mockLog.debug).to.have.been.calledWithMatch('Successfully verified file existence');
291-
});
292-
293-
it('returns response directly if cache write fails', async () => {
294-
mockS3.send.callsFake((cmd) => {
295-
if (cmd.constructor && cmd.constructor.name === 'PutObjectCommand') {
296-
throw new Error('S3 put failed');
297-
}
298-
if (cmd.constructor && cmd.constructor.name === 'HeadObjectCommand' && cmd.input.Key.includes(`${SITE_ID}/`)) {
299-
const err = new Error('not found');
300-
err.name = 'NotFound';
301-
return Promise.reject(err);
302-
}
303-
return Promise.resolve({});
304-
});
305-
306-
const mockAthenaResults = [
307-
{
308-
year: 2024,
309-
week: 23,
310-
pageviews: 1000,
311-
},
312-
];
313-
mockAthenaQuery.resolves(mockAthenaResults);
314-
315-
const controller = PTA2Controller(mockContext, mockLog, mockEnv);
316-
const res = await controller.getPTAWeeklySummary();
317-
expect(res.status).to.equal(200);
318-
const contentEncoding = res.headers.get('content-encoding');
319-
expect(contentEncoding).to.equal('gzip');
238+
expect(body).to.equal(null);
320239
});
321240

322241
it('uses month parameter when week is not provided', async () => {
@@ -399,48 +318,6 @@ describe('PTA2Controller', () => {
399318
expect(mockAthenaQuery).to.have.been.calledOnce;
400319
});
401320

402-
it('handles cache verification failure by returning response directly', async () => {
403-
let putCalled = false;
404-
mockS3.send.callsFake((cmd) => {
405-
if (cmd.constructor && cmd.constructor.name === 'PutObjectCommand') {
406-
putCalled = true;
407-
lastPutObject = cmd;
408-
return Promise.resolve({});
409-
}
410-
if (cmd.constructor && cmd.constructor.name === 'HeadObjectCommand') {
411-
if (putCalled) {
412-
// Simulate verification failure
413-
const err = new Error('not found');
414-
err.name = 'NotFound';
415-
return Promise.reject(err);
416-
}
417-
const err = new Error('not found');
418-
err.name = 'NotFound';
419-
return Promise.reject(err);
420-
}
421-
return Promise.resolve({});
422-
});
423-
424-
// Also stub getSignedUrl to return null
425-
mockContext.s3.getSignedUrl.resolves(null);
426-
427-
const mockAthenaResults = [
428-
{
429-
year: 2024,
430-
week: 23,
431-
pageviews: 1000,
432-
},
433-
];
434-
mockAthenaQuery.resolves(mockAthenaResults);
435-
436-
const controller = PTA2Controller(mockContext, mockLog, mockEnv);
437-
const res = await controller.getPTAWeeklySummary();
438-
expect(res.status).to.equal(200);
439-
expect(mockLog.warn).to.have.been.calledWithMatch('Failed to return cache key');
440-
const contentEncoding = res.headers.get('content-encoding');
441-
expect(contentEncoding).to.equal('gzip');
442-
});
443-
444321
it('uses correct database and table names from env', async () => {
445322
const mockAthenaResults = [
446323
{
@@ -464,22 +341,5 @@ describe('PTA2Controller', () => {
464341
expect(description).to.include('test-db');
465342
expect(description).to.include(SITE_ID);
466343
});
467-
468-
it('logs appropriate messages throughout the flow', async () => {
469-
const mockAthenaResults = [
470-
{
471-
year: 2024,
472-
week: 23,
473-
pageviews: 1000,
474-
},
475-
];
476-
mockAthenaQuery.resolves(mockAthenaResults);
477-
478-
const controller = PTA2Controller(mockContext, mockLog, mockEnv);
479-
await controller.getPTAWeeklySummary();
480-
481-
expect(mockLog.info).to.have.been.calledWithMatch('Cached result for file:');
482-
expect(mockLog.info).to.have.been.calledWithMatch('Athena result JSON to S3 cache');
483-
});
484344
});
485345
});

0 commit comments

Comments
 (0)