refactor: re-org StatisticsController and its dependencies (#185)

src/server/api/statistics.ts

@@ -1,88 +1,17 @@
 import Express from 'express'
-import { statClient } from '../redis'
-import { logger, statisticsExpiry } from '../config'
 import { DependencyIds } from '../constants'
 import { container } from '../util/inversify'
-import { UrlRepositoryInterface } from '../repositories/interfaces/UrlRepositoryInterface'
-import { UserRepositoryInterface } from '../repositories/interfaces/UserRepositoryInterface'
+import { StatisticsControllerInterface } from '../controllers/interfaces/StatisticsControllerInterface'
 
 const router = Express.Router()
-const urlRepository = container.get<UrlRepositoryInterface>(
-  DependencyIds.urlRepository,
-)
-const userRepository = container.get<UserRepositoryInterface>(
-  DependencyIds.userRepository,
+
+const statisticsController = container.get<StatisticsControllerInterface>(
+  DependencyIds.statisticsController,
 )
 
 /**
  * Endpoint to retrieve total user, link, and click counts.
  */
-router.get('/', async (_: Express.Request, res: Express.Response) => {
-  // Check if cache contains value
-  statClient.mget(
-    ['userCount', 'linkCount', 'clickCount'],
-    async (cacheError, results: Array<string | null>) => {
-      if (cacheError) {
-        // log and fallback to database
-        logger.error(
-          `Access to statistics cache failed unexpectedly:\t${cacheError}`,
-        )
-      }
-
-      // Since the expiry in Redis of the values are the same,
-      // all 3 should be present (or absent) from Redis together
-      // If the data is not in Redis, results will be [null, null, null]
-      if (!cacheError && !results.includes(null)) {
-        // Turn each value into an integer
-        const [userCount, linkCount, clickCount] = results.map((x) => Number(x))
-
-        res.json({ userCount, linkCount, clickCount })
-        return
-      }
-
-      // If the values are not found in the cache, we read from the DB
-      const [userCount, linkCount, clickCountUntrusted] = await Promise.all([
-        userRepository.getNumUsers(),
-        urlRepository.getNumUrls(),
-        urlRepository.getTotalLinkClicks(),
-      ])
-
-      // Cater to the edge case where clickCount is NaN because there are no links
-      const clickCount = Number.isNaN(clickCountUntrusted)
-        ? 0
-        : clickCountUntrusted
-
-      res.json({ userCount, linkCount, clickCount })
-
-      // Store values into Redis
-      const callback = (err: Error | null) => {
-        if (err) {
-          logger.error(`Cache write failed:\t${err}`)
-        }
-      }
-      statClient.set(
-        'userCount',
-        `${userCount}`,
-        'EX',
-        statisticsExpiry,
-        callback,
-      )
-      statClient.set(
-        'linkCount',
-        `${linkCount}`,
-        'EX',
-        statisticsExpiry,
-        callback,
-      )
-      statClient.set(
-        'clickCount',
-        `${clickCount}`,
-        'EX',
-        statisticsExpiry,
-        callback,
-      )
-    },
-  )
-})
+router.get('/', statisticsController.getGlobalStatistics)
 
 export = router

src/server/constants.ts

@@ -16,6 +16,9 @@ export const DependencyIds = {
   redirectService: Symbol.for('redirectService'),
   crawlerCheckService: Symbol.for('crawlerCheckService'),
   redirectController: Symbol.for('redirectController'),
+  statisticsRepository: Symbol.for('statisticsRepository'),
+  statisticsService: Symbol.for('repositoryService'),
+  statisticsController: Symbol.for('statisticsController'),
 }
 
 export default DependencyIds

src/server/controllers/StatisticsController.ts

@@ -0,0 +1,26 @@
+import Express from 'express'
+import { inject, injectable } from 'inversify'
+import { StatisticsControllerInterface } from './interfaces/StatisticsControllerInterface'
+import { StatisticsServiceInterface } from '../services/interfaces/StatisticsServiceInterface'
+import { DependencyIds } from '../constants'
+
+@injectable()
+export class StatisticsController implements StatisticsControllerInterface {
+  private statisticsService: StatisticsServiceInterface
+
+  public constructor(
+    @inject(DependencyIds.statisticsService)
+    statisticsService: StatisticsServiceInterface,
+  ) {
+    this.statisticsService = statisticsService
+  }
+
+  public getGlobalStatistics: (
+    req: Express.Request,
+    res: Express.Response,
+  ) => Promise<void> = async (_, res) => {
+    res.json(await this.statisticsService.getGlobalStatistics())
+  }
+}
+
+export default StatisticsController

src/server/controllers/interfaces/StatisticsControllerInterface.ts

@@ -0,0 +1,8 @@
+import Express from 'express'
+
+export interface StatisticsControllerInterface {
+  getGlobalStatistics(
+    req: Express.Request,
+    res: Express.Response,
+  ): Promise<void>
+}

src/server/inversify.config.ts

@@ -18,6 +18,9 @@ import { OtpMapper } from './mappers/OtpMapper'
 import { RedirectService } from './services/RedirectService'
 import { RedirectController } from './controllers/RedirectController'
 import { CrawlerCheckService } from './services/CrawlerCheckService'
+import { StatisticsRepository } from './repositories/StatisticsRepository'
+import { StatisticsService } from './services/StatisticsService'
+import { StatisticsController } from './controllers/StatisticsController'
 
 function bindIfUnbound<T>(
   dependencyId: symbol,
@@ -41,6 +44,9 @@ export default () => {
   bindIfUnbound(DependencyIds.redirectController, RedirectController)
   bindIfUnbound(DependencyIds.redirectService, RedirectService)
   bindIfUnbound(DependencyIds.crawlerCheckService, CrawlerCheckService)
+  bindIfUnbound(DependencyIds.statisticsController, StatisticsController)
+  bindIfUnbound(DependencyIds.statisticsRepository, StatisticsRepository)
+  bindIfUnbound(DependencyIds.statisticsService, StatisticsService)
 
   container.bind(DependencyIds.s3Bucket).toConstantValue(s3Bucket)
 

src/server/repositories/StatisticsRepository.ts

@@ -0,0 +1,69 @@
+/* eslint-disable class-methods-use-this */
+
+import { injectable } from 'inversify'
+import { User } from '../models/user'
+import { Url } from '../models/url'
+import { statClient } from '../redis'
+import { logger, statisticsExpiry } from '../config'
+import { StatisticsRepositoryInterface } from './interfaces/StatisticsRepositoryInterface'
+import { GlobalStatistics } from './types'
+
+const USER_COUNT_KEY = 'userCount'
+const CLICK_COUNT_KEY = 'clickCount'
+const LINK_COUNT_KEY = 'linkCount'
+
+@injectable()
+export class StatisticsRepository implements StatisticsRepositoryInterface {
+  public getGlobalStatistics: () => Promise<GlobalStatistics> = async () => {
+    const counts = await this.tryGetFromCache([
+      USER_COUNT_KEY,
+      CLICK_COUNT_KEY,
+      LINK_COUNT_KEY,
+    ])
+
+    let [userCount, clickCount, linkCount] = counts.map((count) =>
+      count != null ? Number(count) : null,
+    )
+
+    if (userCount == null) {
+      userCount = await User.count()
+      this.trySetCache(USER_COUNT_KEY, userCount.toString())
+    }
+
+    if (clickCount == null) {
+      clickCount = await Url.sum('clicks')
+      this.trySetCache(CLICK_COUNT_KEY, clickCount.toString())
+    }
+
+    if (linkCount == null) {
+      linkCount = await Url.count()
+      this.trySetCache(LINK_COUNT_KEY, linkCount.toString())
+    }
+
+    return { linkCount, clickCount, userCount }
+  }
+
+  private tryGetFromCache(keys: string[]): Promise<(string | null)[]> {
+    return new Promise((resolve) =>
+      statClient.mget(keys, (cacheError, result) => {
+        if (cacheError) {
+          logger.error(
+            `Access to statistics cache failed unexpectedly:\t${cacheError}`,
+          )
+          resolve(keys.map(() => null))
+        }
+        resolve(result)
+      }),
+    )
+  }
+
+  private trySetCache(key: string, value: string): void {
+    statClient.set(key, value, 'EX', statisticsExpiry, (err: Error | null) => {
+      if (err) {
+        logger.error(`Cache write failed:\t${err}`)
+      }
+    })
+  }
+}
+
+export default StatisticsRepository

src/server/repositories/UrlRepository.ts

@@ -30,14 +30,6 @@ export class UrlRepository implements UrlRepositoryInterface {
     this.urlMapper = urlMapper
   }
 
-  getTotalLinkClicks: () => Promise<number> = () => {
-    return Url.sum('clicks')
-  }
-
-  getNumUrls: () => Promise<number> = () => {
-    return Url.count()
-  }
-
   public findByShortUrl: (
     shortUrl: string,
   ) => Promise<StorableUrl | null> = async (shortUrl) => {

src/server/repositories/UserRepository.ts

@@ -109,10 +109,6 @@ export class UserRepository implements UserRepositoryInterface {
 
     return { urls, count }
   }
-
-  public getNumUsers: () => Promise<number> = () => {
-    return User.count()
-  }
 }
 
 export default UserRepository

src/server/repositories/interfaces/StatisticsRepositoryInterface.ts

@@ -0,0 +1,10 @@
+import { GlobalStatistics } from '../types'
+
+export interface StatisticsRepositoryInterface {
+  /**
+   * Retrieves the global statistics from the store.
+   * These include total click, link and user count.
+   * @returns Promise that resolves to an object that encapsulates statistics.
+   */
+  getGlobalStatistics(): Promise<GlobalStatistics>
+}

src/server/repositories/interfaces/UrlRepositoryInterface.ts

@@ -46,18 +46,4 @@ export interface UrlRepositoryInterface {
    * @returns Promise that resolves to be empty.
    */
   incrementClick: (shortUrl: string) => Promise<void>
-
-  /**
-   * Retrieves the sum of link clicks across all links.
-   * @param {string} shortUrl
-   * @returns Promise that resolves to the total number of clicks.
-   */
-  getTotalLinkClicks: () => Promise<number>
-
-  /**
-   * Retrieves the number of URLs in the data store.
-   * @param {string} shortUrl
-   * @returns Promise that resolves to the total number of urls.
-   */
-  getNumUrls: () => Promise<number>
 }

src/server/repositories/interfaces/UserRepositoryInterface.ts

@@ -51,10 +51,4 @@ export interface UserRepositoryInterface {
    * @returns Promise that resolves to an object containing the urls and total count.
    */
   findUrlsForUser(conditions: UserUrlsQueryConditions): Promise<UrlsPaginated>
-
-  /**
-   * Fetches the total number of users in the data store.
-   * @returns Promise which resolves to the number.
-   */
-  getNumUsers(): Promise<number>
 }

src/server/repositories/types.ts

@@ -49,3 +49,9 @@ export type StorableOtp = {
   hashedOtp: string
   retries: number
 }
+
+export type GlobalStatistics = {
+  userCount: number
+  clickCount: number
+  linkCount: number
+}

src/server/services/StatisticsService.ts

@@ -0,0 +1,23 @@
+import { inject, injectable } from 'inversify'
+import { StatisticsServiceInterface } from './interfaces/StatisticsServiceInterface'
+import { DependencyIds } from '../constants'
+import { StatisticsRepositoryInterface } from '../repositories/interfaces/StatisticsRepositoryInterface'
+import { GlobalStatistics } from '../repositories/types'
+
+@injectable()
+export class StatisticsService implements StatisticsServiceInterface {
+  private statisticsRepository: StatisticsRepositoryInterface
+
+  public constructor(
+    @inject(DependencyIds.statisticsRepository)
+    statisticsRepository: StatisticsRepositoryInterface,
+  ) {
+    this.statisticsRepository = statisticsRepository
+  }
+
+  getGlobalStatistics: () => Promise<GlobalStatistics> = async () => {
+    return this.statisticsRepository.getGlobalStatistics()
+  }
+}
+
+export default StatisticsService

src/server/services/interfaces/StatisticsServiceInterface.ts

@@ -0,0 +1,5 @@
+import { GlobalStatistics } from '../../repositories/types'
+
+export interface StatisticsServiceInterface {
+  getGlobalStatistics(): Promise<GlobalStatistics>
+}

test/server/mocks/repositories/StatisticsRepository.ts

@@ -0,0 +1,14 @@
+/* eslint-disable class-methods-use-this */
+
+import { injectable } from 'inversify'
+import { StatisticsRepositoryInterface } from '../../../../src/server/repositories/interfaces/StatisticsRepositoryInterface'
+import { GlobalStatistics } from '../../../../src/server/repositories/types'
+
+@injectable()
+export class MockStatisticsRepository implements StatisticsRepositoryInterface {
+  getGlobalStatistics(): Promise<GlobalStatistics> {
+    return Promise.resolve({ userCount: 1, clickCount: 2, linkCount: 3 })
+  }
+}
+
+export default MockStatisticsRepository

test/server/services/StatisticsService.test.ts

@@ -0,0 +1,18 @@
+import { StatisticsService } from '../../../src/server/services/StatisticsService'
+import { MockStatisticsRepository } from '../mocks/repositories/StatisticsRepository'
+
+/**
+ * Unit tests for StatisticService.
+ */
+describe('StatisticService tests', () => {
+  describe('getGlobalStatistics tests', () => {
+    it('Should return statistics from repository', async () => {
+      const service = new StatisticsService(new MockStatisticsRepository())
+      await expect(service.getGlobalStatistics()).resolves.toStrictEqual({
+        userCount: 1,
+        clickCount: 2,
+        linkCount: 3,
+      })
+    })
+  })
+})