Skip to content

Commit

Permalink
Update: Add BoundedCache for Thumbnails Sidebar (box#917)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conrad Chan authored and Conrad Chan committed Feb 19, 2019
1 parent 240ad38 commit 3321ff2
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 13 deletions.
62 changes: 62 additions & 0 deletions src/lib/BoundedCache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import Cache from './Cache';

class BoundedCache extends Cache {
/** @property {Array} - Maintains the list of cache keys in order in which they were added to the cache */
cacheQueue;

/** @property {number} - The maximum number of entries in the cache */
maxEntries;

/**
* [constructor]
*
* @param {number} [maxEntries] - Override the maximum number of cache entries
*/
constructor(maxEntries) {
super();

this.maxEntries = maxEntries || 500;
this.cache = {};
this.cacheQueue = [];
}

/**
* Destroys the bounded cache
*
* @return {void}
*/
destroy() {
this.cache = null;
this.cacheQueue = null;
}

/**
* Caches a simple object in memory. If the number of cache entries
* then exceeds the maxEntries value, then the earliest key in cacheQueue
* will be removed from the cache.
*
* @param {string} key - The cache key
* @param {*} value - The cache value
* @return {void}
*/
set(key, value) {
// If this key is not already in the cache, then add it
// to the cacheQueue. This avoids adding the same key to
// the cacheQueue multiple times if the cache entry gets updated
if (!this.inCache(key)) {
this.cacheQueue.push(key);
}

super.set(key, value);

// If the cacheQueue exceeds the maxEntries then remove the first
// key from the front of the cacheQueue and unset that entry
// from the cache
if (this.cacheQueue.length > this.maxEntries) {
const deleteKey = this.cacheQueue.shift();
this.unset(deleteKey);
}
}
}

export default BoundedCache;
23 changes: 18 additions & 5 deletions src/lib/ThumbnailsSidebar.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import isFinite from 'lodash/isFinite';
import VirtualScroller from './VirtualScroller';
import { CLASS_HIDDEN } from './constants';
import BoundedCache from './BoundedCache';

const CLASS_BOX_PREVIEW_THUMBNAIL = 'bp-thumbnail';
const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE = 'bp-thumbnail-image';
Expand Down Expand Up @@ -43,7 +44,7 @@ class ThumbnailsSidebar {
this.anchorEl = element;
this.currentThumbnails = [];
this.pdfViewer = pdfViewer;
this.thumbnailImageCache = {};
this.thumbnailImageCache = new BoundedCache();

this.createImageEl = this.createImageEl.bind(this);
this.createPlaceholderThumbnail = this.createPlaceholderThumbnail.bind(this);
Expand Down Expand Up @@ -94,7 +95,11 @@ class ThumbnailsSidebar {
this.virtualScroller = null;
}

this.thumbnailImageCache = null;
if (this.thumbnailImageCache) {
this.thumbnailImageCache.destroy();
this.thumbnailImageCache = null;
}

this.pdfViewer = null;
this.currentThumbnails = [];
this.currentPage = null;
Expand Down Expand Up @@ -201,6 +206,14 @@ class ThumbnailsSidebar {
thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IS_SELECTED);
}

// If image is already in cache, then use it instead of waiting for
// the second render image pass
const cachedImage = this.thumbnailImageCache.get(itemIndex);
if (cachedImage && !cachedImage.inProgress) {
thumbnailEl.appendChild(cachedImage.image);
thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED);
}

return thumbnailEl;
}

Expand Down Expand Up @@ -233,7 +246,7 @@ class ThumbnailsSidebar {
* @return {Promise} - promise reolves with the image HTMLElement or null if generation is in progress
*/
createThumbnailImage(itemIndex) {
const cacheEntry = this.thumbnailImageCache[itemIndex];
const cacheEntry = this.thumbnailImageCache.get(itemIndex);

// If this thumbnail has already been cached, use it
if (cacheEntry && cacheEntry.image) {
Expand All @@ -246,13 +259,13 @@ class ThumbnailsSidebar {
}

// Update the cache entry to be in progress
this.thumbnailImageCache[itemIndex] = { ...cacheEntry, inProgress: true };
this.thumbnailImageCache.set(itemIndex, { ...cacheEntry, inProgress: true });

return this.getThumbnailDataURL(itemIndex + 1)
.then(this.createImageEl)
.then((imageEl) => {
// Cache this image element for future use
this.thumbnailImageCache[itemIndex] = { inProgress: false, image: imageEl };
this.thumbnailImageCache.set(itemIndex, { inProgress: false, image: imageEl });

return imageEl;
});
Expand Down
59 changes: 59 additions & 0 deletions src/lib/__tests__/ThumbnailsImageCache-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/* eslint-disable no-unused-expressions */
import BoundedCache from '../BoundedCache';

const sandbox = sinon.sandbox.create();

describe('BoundedCache', () => {
let cache;

beforeEach(() => {
cache = new BoundedCache(2);
});

afterEach(() => {
sandbox.verifyAndRestore();

cache = null;
});

describe('constructor()', () => {
it('should initialize properties', () => {
cache = new BoundedCache();

expect(cache.maxEntries).to.be.equal(500);
expect(cache.cache).to.be.empty;
expect(cache.cacheQueue.length).to.be.equal(0);
});

it('should handle maxEntries', () => {
expect(cache.maxEntries).to.be.equal(2);
});
});

describe('set()', () => {
it('should add the entry to the cache', () => {
cache.set('foo', 'bar');

expect(cache.inCache('foo')).to.be.true;
expect(cache.cacheQueue).to.be.eql(['foo']);
});

it('should not update the cacheQueue if key already exists', () => {
cache.set('foo', 'bar');
cache.set('foo', 'bar2');

expect(cache.inCache('foo')).to.be.true;
expect(cache.get('foo')).to.be.equal('bar2');
expect(cache.cacheQueue).to.be.eql(['foo']);
});

it('should remove the earliest added entry when entries exceed maxEntries', () => {
cache.set('foo', 'bar');
cache.set('hello', 'world');
cache.set('goodnight', 'moon');

expect(cache.inCache('foo')).to.be.false;
expect(cache.cacheQueue).to.be.eql(['hello', 'goodnight']);
});
});
});
23 changes: 15 additions & 8 deletions src/lib/__tests__/ThumbnailsSidebar-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ describe('ThumbnailsSidebar', () => {
sandbox.verifyAndRestore();

if (thumbnailsSidebar && typeof thumbnailsSidebar.destroy === 'function') {
thumbnailsSidebar.thumbnailImageCache = null;
thumbnailsSidebar.destroy();
}

Expand All @@ -67,7 +68,7 @@ describe('ThumbnailsSidebar', () => {
it('should initialize properties', () => {
expect(thumbnailsSidebar.anchorEl.id).to.be.equal('test-thumbnails-sidebar');
expect(thumbnailsSidebar.pdfViewer).to.be.equal(pdfViewer);
expect(thumbnailsSidebar.thumbnailImageCache).to.be.empty;
expect(thumbnailsSidebar.thumbnailImageCache.cache).to.be.empty;
expect(thumbnailsSidebar.scale).to.be.undefined;
expect(thumbnailsSidebar.pageRatio).to.be.undefined;
});
Expand Down Expand Up @@ -226,11 +227,13 @@ describe('ThumbnailsSidebar', () => {
.stub(thumbnailsSidebar, 'getThumbnailDataURL')
.returns(Promise.resolve());
stubs.createImageEl = sandbox.stub(thumbnailsSidebar, 'createImageEl');
stubs.getCacheEntry = sandbox.stub(thumbnailsSidebar.thumbnailImageCache, 'get');
stubs.setCacheEntry = sandbox.stub(thumbnailsSidebar.thumbnailImageCache, 'set');
});

it('should resolve immediately if the image is in cache', () => {
const cachedImage = {};
thumbnailsSidebar.thumbnailImageCache = { 1: { image: cachedImage } };
stubs.getCacheEntry.withArgs(1).returns({ image: cachedImage });

return thumbnailsSidebar.createThumbnailImage(1).then(() => {
expect(stubs.createImageEl).not.to.be.called;
Expand All @@ -239,19 +242,17 @@ describe('ThumbnailsSidebar', () => {

it('should create an image element if not in cache', () => {
const cachedImage = {};
thumbnailsSidebar.thumbnailImageCache = { 1: { image: cachedImage } };
stubs.createImageEl.returns(cachedImage);

return thumbnailsSidebar.createThumbnailImage(0).then((imageEl) => {
expect(stubs.createImageEl).to.be.called;
expect(thumbnailsSidebar.thumbnailImageCache[0].image).to.be.eql(imageEl);
expect(thumbnailsSidebar.thumbnailImageCache[0].inProgress).to.be.false;
expect(stubs.setCacheEntry).to.be.calledWith(0, { inProgress: false, image: imageEl });
});
});

it('should resolve with null if cache entry inProgress is true', () => {
const cachedImage = {};
thumbnailsSidebar.thumbnailImageCache = { 0: { inProgress: true } };
stubs.getCacheEntry.withArgs(0).returns({ inProgress: true });
stubs.createImageEl.returns(cachedImage);

return thumbnailsSidebar.createThumbnailImage(0).then((imageEl) => {
Expand All @@ -262,9 +263,15 @@ describe('ThumbnailsSidebar', () => {
});

describe('getThumbnailDataURL()', () => {
beforeEach(() => {
stubs.getCacheEntry = sandbox.stub(thumbnailsSidebar.thumbnailImageCache, 'get');
stubs.setCacheEntry = sandbox.stub(thumbnailsSidebar.thumbnailImageCache, 'set');
thumbnailsSidebar.thumbnailImageCache = { get: stubs.getCacheEntry, set: stubs.setCacheEntry };
});

it('should scale canvas the same as the first page if page ratio is the same', () => {
const cachedImage = {};
thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage };
stubs.getCacheEntry.withArgs(1).returns(cachedImage);
thumbnailsSidebar.pageRatio = 1;

// Current page has same ratio
Expand All @@ -281,7 +288,7 @@ describe('ThumbnailsSidebar', () => {

it('should handle non-uniform page ratios', () => {
const cachedImage = {};
thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage };
stubs.getCacheEntry.withArgs(1).returns(cachedImage);
thumbnailsSidebar.pageRatio = 1;

// Current page has ratio of 0.5 instead of 1
Expand Down

0 comments on commit 3321ff2

Please sign in to comment.