Add caching of user-data (#568)

* Add caching of user-data for 600 seconds

* Make cache-entry interface commonly usable

* Extract revision types

* Remove revision-cache rule

* Use seconds as cache-time interval (Date.now uses milliseconds)

* Fix import error

* Extract cache logic into common cache-class

* Add cache class that was forgotten to commit in last commit

* Start adding unit tests

* Fix bug detected during unit-testing

* Add unit tests for cache

* Made entry-limit test more explicit

* Renamed files to lower-case starting letter

src/api/revisions/index.ts

@@ -1,23 +1,21 @@
+import { Cache } from '../../components/common/cache/cache'
 import { defaultFetchConfig, expectResponseCode, getApiUrl } from '../utils'
+import { Revision, RevisionListEntry } from './types'
 
-export interface Revision {
-  content: string
-  timestamp: number
-  authors: string[]
-}
-
-export interface RevisionListEntry {
-  timestamp: number
-  length: number
-  authors: string[]
-}
+const revisionCache = new Cache<string, Revision>(3600)
 
 export const getRevision = async (noteId: string, timestamp: number): Promise<Revision> => {
+  const cacheKey = `${noteId}:${timestamp}`
+  if (revisionCache.has(cacheKey)) {
+    return revisionCache.get(cacheKey)
+  }
   const response = await fetch(getApiUrl() + `/notes/${noteId}/revisions/${timestamp}`, {
     ...defaultFetchConfig
   })
   expectResponseCode(response)
-  return await response.json() as Promise<Revision>
+  const revisionData = await response.json() as Revision
+  revisionCache.put(cacheKey, revisionData)
+  return revisionData
 }
 
 export const getAllRevisions = async (noteId: string): Promise<RevisionListEntry[]> => {

src/api/revisions/types.d.ts

@@ -0,0 +1,11 @@
+export interface Revision {
+  content: string
+  timestamp: number
+  authors: string[]
+}
+
+export interface RevisionListEntry {
+  timestamp: number
+  length: number
+  authors: string[]
+}

src/api/users/index.ts

@@ -1,10 +1,18 @@
+import { Cache } from '../../components/common/cache/cache'
 import { defaultFetchConfig, expectResponseCode, getApiUrl } from '../utils'
 import { UserResponse } from './types'
 
+const cache = new Cache<string, UserResponse>(600)
+
 export const getUserById = async (userid: string): Promise<UserResponse> => {
+  if (cache.has(userid)) {
+    return cache.get(userid)
+  }
   const response = await fetch(`${getApiUrl()}/users/${userid}`, {
     ...defaultFetchConfig
   })
   expectResponseCode(response)
-  return (await response.json()) as UserResponse
+  const userData = (await response.json()) as UserResponse
+  cache.put(userid, userData)
+  return userData
 }

src/components/common/cache/cache.test.ts

@@ -0,0 +1,77 @@
+import { Cache } from './cache'
+
+describe('Test caching functionality', () => {
+  let testCache: Cache<string, number>
+
+  beforeEach(() => {
+    testCache = new Cache<string, number>(1000)
+  })
+
+  it('initialize with right lifetime, no entry limit', () => {
+    const lifetime = 1000
+    const lifetimedCache = new Cache<string, string>(lifetime)
+    expect(lifetimedCache.entryLifetime).toEqual(lifetime)
+    expect(lifetimedCache.maxEntries).toEqual(0)
+  })
+
+  it('initialize with right lifetime, given entry limit', () => {
+    const lifetime = 1000
+    const maxEntries = 10
+    const limitedCache = new Cache<string, string>(lifetime, maxEntries)
+    expect(limitedCache.entryLifetime).toEqual(lifetime)
+    expect(limitedCache.maxEntries).toEqual(maxEntries)
+  })
+
+  it('entry exists after inserting', () => {
+    testCache.put('test', 123)
+    expect(testCache.has('test')).toBe(true)
+  })
+
+  it('entry does not exist prior inserting', () => {
+    expect(testCache.has('test')).toBe(false)
+  })
+
+  it('entry does expire', () => {
+    const shortLivingCache = new Cache<string, number>(2)
+    shortLivingCache.put('test', 123)
+    expect(shortLivingCache.has('test')).toBe(true)
+    setTimeout(() => {
+      expect(shortLivingCache.has('test')).toBe(false)
+    }, 2000)
+  })
+
+  it('entry value does not change', () => {
+    const testValue = Date.now()
+    testCache.put('test', testValue)
+    expect(testCache.get('test')).toEqual(testValue)
+  })
+
+  it('error is thrown on non-existent entry', () => {
+    const accessNonExistentEntry = () => {
+      testCache.get('test')
+    }
+    expect(accessNonExistentEntry).toThrow(Error)
+  })
+
+  it('newer item replaces older item', () => {
+    testCache.put('test', 123)
+    testCache.put('test', 456)
+    expect(testCache.get('test')).toEqual(456)
+  })
+
+  it('entry limit is respected', () => {
+    const limitedCache = new Cache<string, number>(1000, 2)
+    limitedCache.put('first', 1)
+    expect(limitedCache.has('first')).toBe(true)
+    expect(limitedCache.has('second')).toBe(false)
+    expect(limitedCache.has('third')).toBe(false)
+    limitedCache.put('second', 2)
+    expect(limitedCache.has('first')).toBe(true)
+    expect(limitedCache.has('second')).toBe(true)
+    expect(limitedCache.has('third')).toBe(false)
+    limitedCache.put('third', 3)
+    expect(limitedCache.has('first')).toBe(false)
+    expect(limitedCache.has('second')).toBe(true)
+    expect(limitedCache.has('third')).toBe(true)
+  })
+})

src/components/common/cache/cache.ts

@@ -0,0 +1,45 @@
+export interface CacheEntry<T> {
+  entryCreated: number
+  data: T
+}
+
+export class Cache<K, V> {
+  private store = new Map<K, CacheEntry<V>>()
+
+  readonly entryLifetime: number
+  readonly maxEntries: number
+
+  constructor (lifetime: number, maxEntries = 0) {
+    if (lifetime < 0) {
+      throw new Error('Cache entry lifetime can not be less than 0 seconds.')
+    }
+    this.entryLifetime = lifetime
+    this.maxEntries = maxEntries
+  }
+
+  has (key: K): boolean {
+    if (!this.store.has(key)) {
+      return false
+    }
+    const entry = this.store.get(key)
+    return (!!entry && entry.entryCreated >= (Date.now() - this.entryLifetime * 1000))
+  }
+
+  get (key: K): V {
+    const entry = this.store.get(key)
+    if (!entry) {
+      throw new Error('This cache entry does not exist. Check with ".has()" before using ".get()".')
+    }
+    return entry.data
+  }
+
+  put (key: K, value: V): void {
+    if (this.maxEntries > 0 && this.store.size === this.maxEntries) {
+      this.store.delete(this.store.keys().next().value)
+    }
+    this.store.set(key, {
+      entryCreated: Date.now(),
+      data: value
+    })
+  }
+}

src/components/editor/document-bar/revisions/revision-modal-list-entry.tsx

@@ -2,7 +2,7 @@ import moment from 'moment'
 import React from 'react'
 import { ListGroup } from 'react-bootstrap'
 import { Trans } from 'react-i18next'
-import { RevisionListEntry } from '../../../../api/revisions'
+import { RevisionListEntry } from '../../../../api/revisions/types'
 import { UserResponse } from '../../../../api/users/types'
 import { ForkAwesomeIcon } from '../../../common/fork-awesome/fork-awesome-icon'
 import { UserAvatar } from '../../../common/user-avatar/user-avatar'

src/components/editor/document-bar/revisions/revision-modal.tsx

@@ -4,7 +4,8 @@ import ReactDiffViewer, { DiffMethod } from 'react-diff-viewer'
 import { Trans, useTranslation } from 'react-i18next'
 import { useSelector } from 'react-redux'
 import { useParams } from 'react-router'
-import { getAllRevisions, getRevision, Revision, RevisionListEntry } from '../../../../api/revisions'
+import { getAllRevisions, getRevision } from '../../../../api/revisions'
+import { Revision, RevisionListEntry } from '../../../../api/revisions/types'
 import { UserResponse } from '../../../../api/users/types'
 import { ApplicationState } from '../../../../redux'
 import { CommonModal, CommonModalProps } from '../../../common/modals/common-modal'
@@ -21,7 +22,6 @@ export const RevisionModal: React.FC<CommonModalProps & RevisionButtonProps> = (
   const [selectedRevision, setSelectedRevision] = useState<Revision | null>(null)
   const [error, setError] = useState(false)
   const revisionAuthorListMap = useRef(new Map<number, UserResponse[]>())
-  const revisionCacheMap = useRef(new Map<number, Revision>())
   const darkModeEnabled = useSelector((state: ApplicationState) => state.darkMode.darkMode)
   const { id } = useParams<{ id: string }>()
 
@@ -42,14 +42,8 @@ export const RevisionModal: React.FC<CommonModalProps & RevisionButtonProps> = (
     if (selectedRevisionTimestamp === null) {
       return
     }
-    const cacheEntry = revisionCacheMap.current.get(selectedRevisionTimestamp)
-    if (cacheEntry) {
-      setSelectedRevision(cacheEntry)
-      return
-    }
     getRevision(id, selectedRevisionTimestamp).then(fetchedRevision => {
       setSelectedRevision(fetchedRevision)
-      revisionCacheMap.current.set(selectedRevisionTimestamp, fetchedRevision)
     }).catch(() => setError(true))
   }, [selectedRevisionTimestamp, id])
 

src/components/editor/document-bar/revisions/utils.ts

@@ -1,9 +1,7 @@
-import { Revision } from '../../../../api/revisions'
+import { Revision } from '../../../../api/revisions/types'
 import { getUserById } from '../../../../api/users'
 import { UserResponse } from '../../../../api/users/types'
 
-const userResponseCache = new Map<string, UserResponse>()
-
 export const downloadRevision = (noteId: string, revision: Revision | null): void => {
   if (!revision) {
     return
@@ -23,15 +21,9 @@ export const getUserDataForRevision = (authors: string[]): UserResponse[] => {
     if (index > 9) {
       return
     }
-    const cacheEntry = userResponseCache.get(author)
-    if (cacheEntry) {
-      users.push(cacheEntry)
-      return
-    }
     getUserById(author)
       .then(userData => {
         users.push(userData)
-        userResponseCache.set(author, userData)
       })
       .catch((error) => console.error(error))
   })