Get original url from database and do not create new short URL if it exist. Use constructor parameter injection for url shortener service (#21)

Author: hanchiang

src/controllers/index.ts

@@ -1,4 +1,5 @@
-import { Request, Response } from 'express';
+import { KeyGenerationServiceImpl } from './../services/impl/keyGeneration';
+import { Response } from 'express';
 import {
   ValidatedRequest,
   // Extend from this to define a valid schema type/interface
@@ -13,6 +14,7 @@ import { UrlShortenerServiceImpl } from '../services/impl/urlShortener';
 
 import { UrlDaoImpl } from '../db/postgres/dao/urlDao';
 import { Redis } from '../db/redis';
+import { throwError, ErrorCode } from '../utils/error';
 import logger from '../utils/logger';
 
 export const healthCheck = async (
@@ -48,7 +50,7 @@ export const shortenUrl = async (
   res: Response
 ) => {
   const { url, alias }: ShortenUrlRequest = req.body;
-  const shortenUrlService = new UrlShortenerServiceImpl();
+  const shortenUrlService = UrlShortenerServiceImpl.defaultImpl();
   const shortenedUrl = await shortenUrlService.shortenUrl(url, alias);
   res.json(shortenedUrl);
 };
@@ -58,7 +60,14 @@ export const redirectUrl = async (
   res: Response
 ) => {
   const { urlKey } = req.params;
-  const shortenUrlService = new UrlShortenerServiceImpl();
+  const shortenUrlService = UrlShortenerServiceImpl.defaultImpl();
   const originalUrl = await shortenUrlService.getOriginalUrl(urlKey);
+
+  if (!originalUrl) {
+    throwError({
+      status: ErrorCode.NOT_FOUND,
+      message: `Short URL ${urlKey} does not exist`,
+    });
+  }
   res.redirect(307, originalUrl);
 };

src/db/postgres/dao/urlDao.ts

@@ -6,6 +6,7 @@ export interface UrlDao {
   insert(id: string, originalUrl: string): Promise<void>;
   findById(id: string): Promise<Url>;
   findByIds(ids: string[]): Promise<Url[]>;
+  findByOriginalUrl(originalUrl: string): Promise<Url[]>;
 }
 
 export class UrlDaoImpl implements UrlDao {
@@ -26,4 +27,10 @@ export class UrlDaoImpl implements UrlDao {
   public async findByIds(ids: string[]): Promise<Url[]> {
     return Url.query().findByIds(ids);
   }
+
+  public async findByOriginalUrl(originalUrl: string): Promise<Url[]> {
+    return Url.query().where({
+      originalUrl,
+    });
+  }
 }

src/services/impl/urlShortener.ts

@@ -1,4 +1,4 @@
-import { UrlShortenerService } from '../urlShortener';
+import { UrlShortenerService } from './../urlShortener';
 import { KeyGenerationServiceImpl } from './keyGeneration';
 import { KeyGeneration } from '../keyGeneration';
 import config from '../../config';
@@ -13,10 +13,22 @@ export class UrlShortenerServiceImpl implements UrlShortenerService {
   private urlDao: UrlDao;
   private redis: MemoryStore;
 
-  constructor() {
-    this.redis = Redis.getInstance();
-    this.keyGenerationService = new KeyGenerationServiceImpl();
-    this.urlDao = new UrlDaoImpl();
+  constructor(
+    urlDao: UrlDao,
+    keyGenerationService: KeyGeneration,
+    redis: MemoryStore
+  ) {
+    this.redis = redis;
+    this.keyGenerationService = keyGenerationService;
+    this.urlDao = urlDao;
+  }
+
+  public static defaultImpl(): UrlShortenerService {
+    return new UrlShortenerServiceImpl(
+      new UrlDaoImpl(),
+      new KeyGenerationServiceImpl(),
+      Redis.getInstance()
+    );
   }
 
   public async shortenUrl(
@@ -28,6 +40,11 @@ export class UrlShortenerServiceImpl implements UrlShortenerService {
       await this.ensureAliasDoesNotExist(alias);
       key = urlSafe(alias);
     } else {
+      // If key exists, return it, don't store additional records
+      const urlKey = await this.getUrlKeyFromDb(originalUrl);
+      if (urlKey) {
+        return this.constructShortenedUrl(urlKey);
+      }
       key = await this.getAvailableKey();
     }
 
@@ -39,11 +56,28 @@ export class UrlShortenerServiceImpl implements UrlShortenerService {
     return this.constructShortenedUrl(key);
   }
 
+  public async getUrlKeyFromDb(originalUrl: string): Promise<string> {
+    const urls = await this.urlDao.findByOriginalUrl(originalUrl);
+    if (!urls) {
+      logger.info(
+        `Original url ${originalUrl} is not found in database. Returning undefined`
+      );
+      return undefined;
+    }
+    if (urls && urls.length) {
+      const urlKey = urls[0].id;
+      logger.info(
+        `Original url ${originalUrl} is found in database, returning url key ${urlKey}`
+      );
+      return urlKey;
+    }
+  }
+
   public async getOriginalUrl(urlKey: string): Promise<string> {
     const cachedData = await this.redis.getString(urlKey);
     // TODO: Test. Log cache hit or miss
     if (cachedData) {
-      logger.info(`Found short URL ${urlKey} from cache`);
+      logger.info(`Found url key ${urlKey} from cache. Returning original url`);
       return cachedData;
     }
 
@@ -52,11 +86,11 @@ export class UrlShortenerServiceImpl implements UrlShortenerService {
     );
     const urlInDb = await this.urlDao.findById(urlKey);
     if (!urlInDb) {
-      throwError({
-        status: ErrorCode.NOT_FOUND,
-        message: `/${urlKey} does not exist`,
-      });
+      return undefined;
     }
+    logger.info(
+      `Setting url key ${urlKey} and original url ${urlInDb.originalUrl} in cache`
+    );
     await this.redis.setString(urlKey, urlInDb.originalUrl);
     return urlInDb.originalUrl;
   }

test/integration/redirectUrl.ts

@@ -30,7 +30,9 @@ describe('Redirect Url integration test', () => {
     const urlKey = 'unknown';
     const res = await sendRequest(server, 'get', `/${urlKey}`);
     expect(res.status).to.equal(ErrorCode.NOT_FOUND);
-    expect(res.body.error.message).to.equal(`/${urlKey} does not exist`);
+    expect(res.body.error.message).to.equal(
+      `Short URL ${urlKey} does not exist`
+    );
   });
 
   it('should redirect if url key exist', async () => {

test/unit/services/urlShortener.ts

@@ -1,7 +1,7 @@
+import { UrlShortenerServiceImpl } from './../../../src/services/impl/urlShortener';
 import { expect } from 'chai';
 import Sinon from 'sinon';
 
-import { UrlShortenerServiceImpl } from '../../../src/services/impl/urlShortener';
 import { UrlDaoImpl } from '../../../src/db/postgres/dao/urlDao';
 import { Redis } from '../../../src/db/redis';
 
@@ -18,11 +18,31 @@ describe('UrlShortenerService unit tests', () => {
       UrlShortenerServiceImpl.prototype,
       'ensureAliasDoesNotExist' as any
     ).resolves();
+    Sinon.stub(UrlShortenerServiceImpl.prototype, 'getUrlKeyFromDb').resolves(
+      undefined
+    );
     Sinon.stub(UrlDaoImpl.prototype, 'insert').resolves();
     Sinon.stub(Redis, 'getInstance' as any).resolves();
 
-    const urlShortenerService = new UrlShortenerServiceImpl();
+    const urlShortenerService = UrlShortenerServiceImpl.defaultImpl();
     const shortenedUrl = await urlShortenerService.shortenUrl(url, alias);
     expect(shortenedUrl).to.eq(`http://localhost:3000/${alias}`);
   });
+
+  it('Should get shortened URL from DB and do not insert into DB', async () => {
+    const url = 'www.google.com';
+    Sinon.stub(
+      UrlShortenerServiceImpl.prototype,
+      'ensureAliasDoesNotExist' as any
+    ).resolves();
+    Sinon.stub(UrlShortenerServiceImpl.prototype, 'getUrlKeyFromDb').resolves(
+      'urlKey'
+    );
+    Sinon.stub(UrlDaoImpl.prototype, 'insert').resolves();
+    Sinon.stub(Redis, 'getInstance' as any).resolves();
+
+    const urlShortenerService = UrlShortenerServiceImpl.defaultImpl();
+    const shortenedUrl = await urlShortenerService.shortenUrl(url);
+    expect(shortenedUrl).to.eq(`http://localhost:3000/urlKey`);
+  });
 });