feat: start writing RedirectionDestination in redis cache

Author: dcshzj

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

@@ -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()
@@ -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 () => {

src/server/repositories/UrlRepository.ts

@@ -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
       }
@@ -535,22 +533,11 @@ export class UrlRepository implements UrlRepositoryInterface {
             const redirectDestination = JSON.parse(cacheLongUrl)
             resolve(redirectDestination)
           } catch (_) {
-            logger.info(
-              `Cache lookup returned a string instead of an object:\tshortUrl=${shortUrl}`,
+            reject(
+              new NotFoundError(
+                `longUrl not found in cache:\tshortUrl=${shortUrl}`,
+              ),
             )
-            resolve({
-              longUrl: cacheLongUrl,
-              isFile: false,
-              safeBrowsingExpiry: null,
-            })
-
-            // FIXME: Throw NotFoundError once all app clients are updated to
-            // use the new RedirectDestination type.
-            // reject(
-            //   new NotFoundError(
-            //     `longUrl not found in cache:\tshortUrl=${shortUrl}`,
-            //   ),
-            // )
           }
         }
       }),
@@ -562,35 +549,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