From 091b2256726d5853ef438ae113aeea2f275808f1 Mon Sep 17 00:00:00 2001 From: Erik Michelson Date: Wed, 30 Sep 2020 23:37:57 +0200 Subject: [PATCH] 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 | 22 +++--- src/api/revisions/types.d.ts | 11 +++ src/api/users/index.ts | 10 ++- src/components/common/cache/cache.test.ts | 77 +++++++++++++++++++ src/components/common/cache/cache.ts | 45 +++++++++++ .../revisions/revision-modal-list-entry.tsx | 2 +- .../document-bar/revisions/revision-modal.tsx | 10 +-- .../editor/document-bar/revisions/utils.ts | 10 +-- 8 files changed, 156 insertions(+), 31 deletions(-) create mode 100644 src/api/revisions/types.d.ts create mode 100644 src/components/common/cache/cache.test.ts create mode 100644 src/components/common/cache/cache.ts diff --git a/src/api/revisions/index.ts b/src/api/revisions/index.ts index 746e85f17..12899aabd 100644 --- a/src/api/revisions/index.ts +++ b/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(3600) export const getRevision = async (noteId: string, timestamp: number): Promise => { + 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 + const revisionData = await response.json() as Revision + revisionCache.put(cacheKey, revisionData) + return revisionData } export const getAllRevisions = async (noteId: string): Promise => { diff --git a/src/api/revisions/types.d.ts b/src/api/revisions/types.d.ts new file mode 100644 index 000000000..e0dcee682 --- /dev/null +++ b/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[] +} diff --git a/src/api/users/index.ts b/src/api/users/index.ts index 98e640345..e7851f0ff 100644 --- a/src/api/users/index.ts +++ b/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(600) + export const getUserById = async (userid: string): Promise => { + 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 } diff --git a/src/components/common/cache/cache.test.ts b/src/components/common/cache/cache.test.ts new file mode 100644 index 000000000..d41408a2a --- /dev/null +++ b/src/components/common/cache/cache.test.ts @@ -0,0 +1,77 @@ +import { Cache } from './cache' + +describe('Test caching functionality', () => { + let testCache: Cache + + beforeEach(() => { + testCache = new Cache(1000) + }) + + it('initialize with right lifetime, no entry limit', () => { + const lifetime = 1000 + const lifetimedCache = new Cache(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(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(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(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) + }) +}) diff --git a/src/components/common/cache/cache.ts b/src/components/common/cache/cache.ts new file mode 100644 index 000000000..85bd70a3c --- /dev/null +++ b/src/components/common/cache/cache.ts @@ -0,0 +1,45 @@ +export interface CacheEntry { + entryCreated: number + data: T +} + +export class Cache { + private store = new Map>() + + 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 + }) + } +} diff --git a/src/components/editor/document-bar/revisions/revision-modal-list-entry.tsx b/src/components/editor/document-bar/revisions/revision-modal-list-entry.tsx index 75eec4b34..ddc540704 100644 --- a/src/components/editor/document-bar/revisions/revision-modal-list-entry.tsx +++ b/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' diff --git a/src/components/editor/document-bar/revisions/revision-modal.tsx b/src/components/editor/document-bar/revisions/revision-modal.tsx index 63f98a94b..dc8b42e9d 100644 --- a/src/components/editor/document-bar/revisions/revision-modal.tsx +++ b/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 = ( const [selectedRevision, setSelectedRevision] = useState(null) const [error, setError] = useState(false) const revisionAuthorListMap = useRef(new Map()) - const revisionCacheMap = useRef(new Map()) const darkModeEnabled = useSelector((state: ApplicationState) => state.darkMode.darkMode) const { id } = useParams<{ id: string }>() @@ -42,14 +42,8 @@ export const RevisionModal: React.FC = ( 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]) diff --git a/src/components/editor/document-bar/revisions/utils.ts b/src/components/editor/document-bar/revisions/utils.ts index fc1440b1e..49e9b734f 100644 --- a/src/components/editor/document-bar/revisions/utils.ts +++ b/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() - 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)) })