Skip to content

Commit 9455244

Browse files
committed
feat: start writing RedirectionDestination in redis cache
1 parent 8742777 commit 9455244

File tree

2 files changed

+35
-59
lines changed

2 files changed

+35
-59
lines changed

src/server/modules/redirect/__tests__/RedirectController.test.ts

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
CrawlerCheckService,
2222
RedirectService,
2323
} from '../services'
24-
// import { RedirectDestination } from '../../../repositories/types'
24+
import { RedirectDestination } from '../../../repositories/types'
2525

2626
const redisMockClient = redisMock.createClient()
2727
const sequelizeMock = new SequelizeMock()
@@ -520,50 +520,40 @@ describe('redirect API tests', () => {
520520
test('url does exists in cache but not db', async () => {
521521
const req = createRequestWithShortUrl('Aaa')
522522
const res = httpMocks.createResponse()
523-
// FIXME: Update to use the new RedirectDestination type
524-
// const redirectDestination: RedirectDestination = {
525-
// longUrl: 'aa',
526-
// isFile: false,
527-
// safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
528-
// }
529-
530-
// redisMockClient.set('aaa', JSON.stringify(redirectDestination))
531-
redisMockClient.set('aaa', 'aa')
523+
const redirectDestination: RedirectDestination = {
524+
longUrl: 'aa',
525+
isFile: false,
526+
safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
527+
}
528+
529+
redisMockClient.set('aaa', JSON.stringify(redirectDestination))
532530
mockDbEmpty()
533531

534532
await container
535533
.get<RedirectController>(DependencyIds.redirectController)
536534
.redirect(req, res)
537535

538-
// expect(res.statusCode).toBe(302)
539-
// NOTE: This is 404 now as the safe browsing repository needs to be updated
540-
// but it will be 302 once the safe browsing expiry is stored in redis
541-
expect(res.statusCode).toBe(404)
542-
// expect(res._getRedirectUrl()).toBe('aa')
536+
expect(res.statusCode).toBe(302)
537+
expect(res._getRedirectUrl()).toBe('aa')
543538
})
544539

545540
test('url in cache and db is down', async () => {
546541
const req = createRequestWithShortUrl('Aaa')
547542
const res = httpMocks.createResponse()
548-
// FIXME: Update to use the new RedirectDestination type
549-
// const redirectDestination: RedirectDestination = {
550-
// longUrl: 'aa',
551-
// isFile: false,
552-
// safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
553-
// }
543+
const redirectDestination: RedirectDestination = {
544+
longUrl: 'aa',
545+
isFile: false,
546+
safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
547+
}
554548

555549
mockDbDown()
556-
// redisMockClient.set('aaa', JSON.stringify(redirectDestination))
557-
redisMockClient.set('aaa', 'aa')
550+
redisMockClient.set('aaa', JSON.stringify(redirectDestination))
558551

559552
await container
560553
.get<RedirectController>(DependencyIds.redirectController)
561554
.redirect(req, res)
562-
// expect(res.statusCode).toBe(302)
563-
// NOTE: This is 404 now as the safe browsing repository needs to be updated
564-
// but it will be 302 once the safe browsing expiry is stored in redis
565-
expect(res.statusCode).toBe(404)
566-
// expect(res._getRedirectUrl()).toBe('aa')
555+
expect(res.statusCode).toBe(302)
556+
expect(res._getRedirectUrl()).toBe('aa')
567557
})
568558

569559
test('retrieval of gtag for transition page', async () => {

src/server/repositories/UrlRepository.ts

Lines changed: 17 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,8 @@ export class UrlRepository implements UrlRepositoryInterface {
208208
} catch {
209209
// Cache failed, look in database
210210
const redirectDestination = await this.getLongUrlFromDatabase(shortUrl)
211-
// FIXME: Cache the entire RedirectDestination object once all app
212-
// clients are updated to use the new RedirectDestination type.
213-
this.cacheShortUrl(shortUrl, redirectDestination.longUrl).catch(
214-
(error) => logger.error(`Unable to cache short URL: ${error}`),
211+
this.cacheShortUrl(shortUrl, redirectDestination).catch((error) =>
212+
logger.error(`Unable to cache short URL: ${error}`),
215213
)
216214
return redirectDestination
217215
}
@@ -562,35 +560,23 @@ export class UrlRepository implements UrlRepositoryInterface {
562560
* @param {string} shortUrl Short url.
563561
* @param {string} longUrl Long url.
564562
*/
565-
private cacheShortUrl: (shortUrl: string, longUrl: string) => Promise<void> =
566-
(shortUrl, longUrl) => {
567-
return new Promise((resolve, reject) => {
568-
redirectClient.set(shortUrl, longUrl, 'EX', redirectExpiry, (err) => {
563+
private cacheShortUrl: (
564+
shortUrl: string,
565+
redirectDestination: RedirectDestination,
566+
) => Promise<void> = (shortUrl, redirectDestination) => {
567+
return new Promise((resolve, reject) => {
568+
redirectClient.set(
569+
shortUrl,
570+
JSON.stringify(redirectDestination),
571+
'EX',
572+
redirectExpiry,
573+
(err) => {
569574
if (err) reject(err)
570575
else resolve()
571-
})
572-
})
573-
}
574-
575-
// FIXME: This is the future implementation of the cacheShortUrl method to be
576-
// used once all app clients are updated to use the new RedirectDestination
577-
// private cacheShortUrl: (
578-
// shortUrl: string,
579-
// redirectDestination: RedirectDestination,
580-
// ) => Promise<void> = (shortUrl, redirectDestination) => {
581-
// return new Promise((resolve, reject) => {
582-
// redirectClient.set(
583-
// shortUrl,
584-
// JSON.stringify(redirectDestination),
585-
// 'EX',
586-
// redirectExpiry,
587-
// (err) => {
588-
// if (err) reject(err)
589-
// else resolve()
590-
// },
591-
// )
592-
// })
593-
// }
576+
},
577+
)
578+
})
579+
}
594580

595581
/**
596582
* Generates the ranking algorithm to be used in the ORDER BY clause in the

0 commit comments

Comments
 (0)