Skip to content

feat: start writing RedirectionDestination in redis cache #2407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dcshzj merged 1 commit into from
Aug 20, 2025
Merged

feat: start writing RedirectionDestination in redis cache #2407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 18 additions & 28 deletions src/server/modules/redirect/__tests__/RedirectController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
CrawlerCheckService,
RedirectService,
} from '../services'
// import { RedirectDestination } from '../../../repositories/types'
import { RedirectDestination } from '../../../repositories/types'

const redisMockClient = redisMock.createClient()
const sequelizeMock = new SequelizeMock()
Expand Down Expand Up @@ -520,50 +520,40 @@ describe('redirect API tests', () => {
test('url does exists in cache but not db', async () => {
const req = createRequestWithShortUrl('Aaa')
const res = httpMocks.createResponse()
// FIXME: Update to use the new RedirectDestination type
// const redirectDestination: RedirectDestination = {
// longUrl: 'aa',
// isFile: false,
// safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
// }

// redisMockClient.set('aaa', JSON.stringify(redirectDestination))
redisMockClient.set('aaa', 'aa')
const redirectDestination: RedirectDestination = {
longUrl: 'aa',
isFile: false,
safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
}

redisMockClient.set('aaa', JSON.stringify(redirectDestination))
mockDbEmpty()

await container
.get<RedirectController>(DependencyIds.redirectController)
.redirect(req, res)

// expect(res.statusCode).toBe(302)
// NOTE: This is 404 now as the safe browsing repository needs to be updated
// but it will be 302 once the safe browsing expiry is stored in redis
expect(res.statusCode).toBe(404)
// expect(res._getRedirectUrl()).toBe('aa')
expect(res.statusCode).toBe(302)
expect(res._getRedirectUrl()).toBe('aa')
})

test('url in cache and db is down', async () => {
const req = createRequestWithShortUrl('Aaa')
const res = httpMocks.createResponse()
// FIXME: Update to use the new RedirectDestination type
// const redirectDestination: RedirectDestination = {
// longUrl: 'aa',
// isFile: false,
// safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
// }
const redirectDestination: RedirectDestination = {
longUrl: 'aa',
isFile: false,
safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
}

mockDbDown()
// redisMockClient.set('aaa', JSON.stringify(redirectDestination))
redisMockClient.set('aaa', 'aa')
redisMockClient.set('aaa', JSON.stringify(redirectDestination))

await container
.get<RedirectController>(DependencyIds.redirectController)
.redirect(req, res)
// expect(res.statusCode).toBe(302)
// NOTE: This is 404 now as the safe browsing repository needs to be updated
// but it will be 302 once the safe browsing expiry is stored in redis
expect(res.statusCode).toBe(404)
// expect(res._getRedirectUrl()).toBe('aa')
expect(res.statusCode).toBe(302)
expect(res._getRedirectUrl()).toBe('aa')
})

test('retrieval of gtag for transition page', async () => {
Expand Down
48 changes: 17 additions & 31 deletions src/server/repositories/UrlRepository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,8 @@ export class UrlRepository implements UrlRepositoryInterface {
} catch {
// Cache failed, look in database
const redirectDestination = await this.getLongUrlFromDatabase(shortUrl)
// FIXME: Cache the entire RedirectDestination object once all app
// clients are updated to use the new RedirectDestination type.
this.cacheShortUrl(shortUrl, redirectDestination.longUrl).catch(
(error) => logger.error(`Unable to cache short URL: ${error}`),
this.cacheShortUrl(shortUrl, redirectDestination).catch((error) =>
logger.error(`Unable to cache short URL: ${error}`),
)
return redirectDestination
}
Expand Down Expand Up @@ -562,35 +560,23 @@ export class UrlRepository implements UrlRepositoryInterface {
* @param {string} shortUrl Short url.
* @param {string} longUrl Long url.
*/
private cacheShortUrl: (shortUrl: string, longUrl: string) => Promise<void> =
(shortUrl, longUrl) => {
return new Promise((resolve, reject) => {
redirectClient.set(shortUrl, longUrl, 'EX', redirectExpiry, (err) => {
private cacheShortUrl: (
shortUrl: string,
redirectDestination: RedirectDestination,
) => Promise<void> = (shortUrl, redirectDestination) => {
return new Promise((resolve, reject) => {
redirectClient.set(
shortUrl,
JSON.stringify(redirectDestination),
'EX',
redirectExpiry,
(err) => {
if (err) reject(err)
else resolve()
})
})
}

// FIXME: This is the future implementation of the cacheShortUrl method to be
// used once all app clients are updated to use the new RedirectDestination
// private cacheShortUrl: (
// shortUrl: string,
// redirectDestination: RedirectDestination,
// ) => Promise<void> = (shortUrl, redirectDestination) => {
// return new Promise((resolve, reject) => {
// redirectClient.set(
// shortUrl,
// JSON.stringify(redirectDestination),
// 'EX',
// redirectExpiry,
// (err) => {
// if (err) reject(err)
// else resolve()
// },
// )
// })
// }
},
)
})
}

/**
* Generates the ranking algorithm to be used in the ORDER BY clause in the
Expand Down
Loading