feat: implement cache for long url and description

src/server/controllers/RedirectController.ts

@@ -133,7 +133,7 @@ export class RedirectController implements RedirectControllerInterface {
     return longUrl.replace(/["]/g, encodeURIComponent)
   }
 
-  private static renderMetaTagDescription(description: string | null) {
+  private static renderMetaTagDescription(description: string) {
     if (!description) {
       return ''
     }

src/server/repositories/UrlRepository.ts

@@ -3,7 +3,7 @@
 import { inject, injectable } from 'inversify'
 import { QueryTypes } from 'sequelize'
 import { Url, UrlType } from '../models/url'
-import { NotFoundError } from '../util/error'
+import { InvalidFormatError, NotFoundError } from '../util/error'
 import { redirectClient } from '../redis'
 import {
   logger,
@@ -121,36 +121,26 @@ export class UrlRepository implements UrlRepositoryInterface {
   public getLongUrl: (shortUrl: string) => Promise<string> = async (
     shortUrl,
   ) => {
+    const { longUrl } = await this.getLongUrlAndDescription(shortUrl)
+    return longUrl
+  }
+
+  public getLongUrlAndDescription: (
+    shortUrl: string,
+  ) => Promise<{ longUrl: string; description: string }> = async (shortUrl) => {
     try {
       // Cache lookup
-      return await this.getLongUrlFromCache(shortUrl)
+      return await this.getLongUrlAndDescriptionFromCache(shortUrl)
     } catch {
       // Cache failed, look in database
-      const longUrl = await this.getLongUrlFromDatabase(shortUrl)
-      this.cacheShortUrl(shortUrl, longUrl).catch((error) =>
-        logger.error(`Unable to cache short URL: ${error}`),
+      const longUrlAndDescription = await this.getLongUrlAndDescriptionFromDatabase(
+        shortUrl,
       )
-      return longUrl
-    }
-  }
-
-  /**
-   * Retrieves the description of the short url from the database.
-   * @param  {string} shortUrl Short url.
-   * @returns The description of the short url.
-   */
-  public getDescription: (shortUrl: string) => Promise<string> = async (
-    shortUrl,
-  ) => {
-    const url = await Url.findOne({
-      where: { shortUrl, state: StorableUrlState.Active },
-    })
-    if (!url) {
-      throw new NotFoundError(
-        `shortUrl not found in database:\tshortUrl=${shortUrl}`,
+      this.cacheShortUrl(shortUrl, longUrlAndDescription).catch((error) =>
+        logger.error(`Unable to cache short URL: ${error}`),
       )
+      return longUrlAndDescription
     }
-    return url.description
   }
 
   public plainTextSearch: (
@@ -202,14 +192,14 @@ export class UrlRepository implements UrlRepositoryInterface {
   }
 
   /**
-   * Retrieves the long url which the short url redirects to
+   * Retrieves the long url which the short url redirects to, and description
    * from the database.
    * @param  {string} shortUrl Short url.
-   * @returns The long url that the short url redirects to.
+   * @returns The long url that the short url redirects to, and description.
    */
-  private getLongUrlFromDatabase: (
+  private getLongUrlAndDescriptionFromDatabase: (
     shortUrl: string,
-  ) => Promise<string> = async (shortUrl) => {
+  ) => Promise<{ longUrl: string; description: string }> = async (shortUrl) => {
     const url = await Url.findOne({
       where: { shortUrl, state: StorableUrlState.Active },
     })
@@ -218,51 +208,73 @@ export class UrlRepository implements UrlRepositoryInterface {
         `shortUrl not found in database:\tshortUrl=${shortUrl}`,
       )
     }
-    return url.longUrl
+
+    return {
+      longUrl: url.longUrl,
+      description: url.description,
+    }
   }
 
   /**
-   * Retrieves the long url which the short url redirects to
+   * Retrieves the long url which the short url redirects to, and description
    * from the cache.
    * @param  {string} shortUrl Short url.
-   * @returns The long url that the short url redirects to.
+   * @returns The long url that the short url redirects to, and description.
    */
-  private getLongUrlFromCache: (shortUrl: string) => Promise<string> = (
-    shortUrl,
-  ) => {
+  private getLongUrlAndDescriptionFromCache: (
+    shortUrl: string,
+  ) => Promise<{ longUrl: string; description: string }> = (shortUrl) => {
     return new Promise((resolve, reject) =>
-      redirectClient.get(shortUrl, (cacheError, cacheLongUrl) => {
+      redirectClient.get(shortUrl, (cacheError, cacheLongUrlAndDescription) => {
         if (cacheError) {
           logger.error(`Cache lookup failed unexpectedly:\t${cacheError}`)
           reject(cacheError)
         } else {
-          if (!cacheLongUrl) {
+          if (!cacheLongUrlAndDescription) {
             reject(
               new NotFoundError(
-                `longUrl not found in cache:\tshortUrl=${shortUrl}`,
+                `longUrl and description not found in cache:\tshortUrl=${shortUrl}`,
+              ),
+            )
+          }
+
+          try {
+            const { longUrl, description } = JSON.parse(
+              cacheLongUrlAndDescription,
+            )
+            resolve({ longUrl, description })
+          } catch (ex) {
+            reject(
+              new InvalidFormatError(
+                `Failed to parse json for cached longUrl and description:\tshortUrl=${shortUrl}`,
               ),
             )
           }
-          resolve(cacheLongUrl)
         }
       }),
     )
   }
 
   /**
-   * Caches the input short url to long url mapping in redis cache.
+   * Caches the input short url to long url and description mapping in redis cache.
    * @param  {string} shortUrl Short url.
-   * @param  {string} longUrl Long url.
+   * @param  {longUrl:string;description:string;} longUrlAndDescription Object of long url and description.
    */
   private cacheShortUrl: (
     shortUrl: string,
-    longUrl: string,
-  ) => Promise<void> = (shortUrl, longUrl) => {
+    longUrlAndDescription: { longUrl: string; description: string },
+  ) => Promise<void> = (shortUrl, longUrlAndDescription) => {
     return new Promise((resolve, reject) => {
-      redirectClient.set(shortUrl, longUrl, 'EX', redirectExpiry, (err) => {
-        if (err) reject(err)
-        else resolve()
-      })
+      redirectClient.set(
+        shortUrl,
+        JSON.stringify(longUrlAndDescription),
+        'EX',
+        redirectExpiry,
+        (err) => {
+          if (err) reject(err)
+          else resolve()
+        },
+      )
     })
   }
 

src/server/repositories/interfaces/UrlRepositoryInterface.ts

@@ -32,22 +32,16 @@ export interface UrlRepositoryInterface {
   ): Promise<StorableUrl>
 
   /**
-   * Looks up the longUrl given a shortUrl from the cache, falling back
+   * Looks up the longUrl and description given a shortUrl from the cache, falling back
    * to the database. The cache is re-populated if the database lookup is
    * performed successfully.
    * @param {string} shortUrl The shortUrl.
-   * @returns Promise that resolves to the longUrl.
+   * @returns Promise that resolves to the longUrl and description.
    * @throws {NotFoundError}
    */
-  getLongUrl: (shortUrl: string) => Promise<string>
-
-  /**
-   * Looks up the description given a shortUrl from the database.
-   * @param {string} shortUrl The shortUrl.
-   * @returns Promise that resolves to the description.
-   * @throws {NotFoundError}
-   */
-  getDescription: (shortUrl: string) => Promise<string>
+  getLongUrlAndDescription: (
+    shortUrl: string,
+  ) => Promise<{ longUrl: string; description: string }>
 
   /**
    * Performs plain text search on Urls based on their shortUrl and

src/server/services/RedirectService.ts

@@ -52,10 +52,11 @@ export class RedirectService implements RedirectServiceInterface {
 
     const shortUrl = rawShortUrl.toLowerCase()
 
-    // Find longUrl to redirect to
-    const longUrl = await this.urlRepository.getLongUrl(shortUrl)
-    // Find description of the shortUrl
-    const description = await this.urlRepository.getDescription(shortUrl)
+    // Find longUrl to redirect to, and description of the shortUrl
+    const {
+      longUrl,
+      description,
+    } = await this.urlRepository.getLongUrlAndDescription(shortUrl)
 
     // Update clicks and click statistics in database.
     this.linkStatisticsService.updateLinkStatistics(shortUrl, userAgent)

src/server/services/types.ts

@@ -9,7 +9,7 @@ export enum RedirectType {
 export type RedirectResult = {
   visitedUrls: string[]
   longUrl: string
-  description: string | null
+  description: string
   redirectType: RedirectType
 }
 

src/server/util/error.ts

@@ -8,6 +8,14 @@ export class NotFoundError extends Error {
   }
 }
 
+export class InvalidFormatError extends Error {
+  constructor(message: string) {
+    super(message)
+    this.name = 'InvalidFormatError'
+    Object.setPrototypeOf(this, InvalidFormatError.prototype)
+  }
+}
+
 export class InvalidOtpError extends Error {
   public retries: number
 

test/server/api/util.ts

@@ -137,6 +137,7 @@ export const urlModelMock = sequelizeMock.define(
     longUrl: 'aa',
     state: ACTIVE,
     clicks: 8,
+    description: 'bb',
   },
   {
     instanceMethods: {

test/server/repositories/UrlRepository.test.ts

@@ -323,6 +323,37 @@ describe('UrlRepository', () => {
     })
   })
 
+  describe('getLongUrlAndDescription', () => {
+    it('should return from db when cache is empty', async () => {
+      await expect(repository.getLongUrlAndDescription('a')).resolves.toEqual({
+        longUrl: 'aa',
+        description: 'bb',
+      })
+    })
+
+    it('should return from cache when cache is filled', async () => {
+      const json = JSON.stringify({ longUrl: 'aaa', description: 'bbb' })
+      redisMockClient.set('a', json)
+      await expect(repository.getLongUrlAndDescription('a')).resolves.toEqual(
+        json,
+      )
+    })
+
+    it('should return from db when cache is down', async () => {
+      cacheGetSpy.mockImplementationOnce((_, callback) => {
+        if (!callback) {
+          return false
+        }
+        callback(new Error('Cache down'), 'Error')
+        return false
+      })
+      await expect(repository.getLongUrlAndDescription('a')).resolves.toEqual({
+        longUrl: 'aa',
+        description: 'bb',
+      })
+    })
+  })
+
   describe('plainTextSearch', () => {
     beforeEach(() => {
       mockQuery.mockClear()