From 1dbdcca912b5f80ec85bab3a07caa5ce53ef04de Mon Sep 17 00:00:00 2001 From: Priyajeet Hora Date: Fri, 1 Sep 2017 02:00:38 -0700 Subject: [PATCH] Update: Allow passed in collections to be of file objects Also allows tokens to be null or undefined. Useful when the passed in file or collection has well formed file objects. Also changes the token getter to pass in typed ids for files and handle cases when typed ids are returned from the token service to make it consistant with other UI Elements. Since all the UI elements can either be requesting folder or file ids, the token service needs to know what type of id we are sending it, as such either file_ or folder_ is prefixed to all the ids. Likewise the token service will return typed ids after fetching tokens. --- src/lib/Preview.js | 109 ++++++++++++++++++------------ src/lib/__tests__/Preview-test.js | 95 +++++++++++++++++++------- src/lib/__tests__/tokens-test.js | 57 +++++++++++++--- src/lib/tokens.js | 99 +++++++++++++++++---------- 4 files changed, 244 insertions(+), 116 deletions(-) diff --git a/src/lib/Preview.js b/src/lib/Preview.js index 8ba71a544..e8bf1ba9d 100644 --- a/src/lib/Preview.js +++ b/src/lib/Preview.js @@ -155,13 +155,20 @@ class Preview extends EventEmitter { * @return {void} */ show(fileIdOrFile, token, options = {}) { - // Save a reference to the options to be used later - if (typeof token === 'string' || typeof token === 'function') { + // Save a reference to the options to be re-used later. + // Token should either be a function or a string. + // Token can also be null or undefined for offline use case. + // But it cannot be a random object. + if (token === null || typeof token !== 'object') { this.previewOptions = Object.assign({}, options, { token }); } else { - throw new Error('Missing access token!'); + throw new Error('Bad access token!'); } + // Update the optional file navigation collection and caches + // if proper valid file objects were passed in. + this.updateCollection(options.collection); + // Load the preview this.load(fileIdOrFile); } @@ -187,16 +194,41 @@ class Preview extends EventEmitter { } /** - * Updates files to navigate between. + * Updates files to navigate between. Collection can be of files + * or file ids or a mix. We normalize here to file ids for easier + * indexing and cache only the well-formed file objects if provided. * * @public - * @param {string[]} [collection] - Updated collection of file IDs + * @param {string[]} [collection] - Updated collection of file or file IDs * @return {void} */ - updateCollection(collection = []) { - this.collection = Array.isArray(collection) ? collection : []; - // Also update the original collection that was saved from the initial show - this.previewOptions.collection = this.collection; + updateCollection(collection) { + const fileOrIds = Array.isArray(collection) ? collection : []; + const files = []; + const fileIds = []; + + fileOrIds.forEach((fileOrId) => { + if (fileOrId && typeof fileOrId === 'string') { + // String id found in the collection + fileIds.push(fileOrId); + } else if (fileOrId && typeof fileOrId === 'object' && typeof fileOrId.id === 'string') { + // Possible well-formed file object found in the collection + fileIds.push(fileOrId.id); + files.push(fileOrId); + } else { + throw new Error('Bad collection provided!'); + } + }); + + // Update the cache with possibly well-formed file objects. + this.updateFileCache(files); + + // Collection always uses string ids for easier indexing. + this.collection = fileIds; + + // Since update collection is a public method, it can be + // called anytime to update navigation. If we are showing + // a preview already show or hide the navigation arrows. if (this.file) { this.ui.showNavigation(this.file.id, this.collection); } @@ -254,7 +286,7 @@ class Preview extends EventEmitter { } /** - * Returns the current file being previewed. + * Returns the current collection of files that preview is aware of. * * @public * @return {Object|null} Current collection @@ -466,21 +498,23 @@ class Preview extends EventEmitter { * @return {void} */ prefetchViewers(viewerNames = []) { - this.getViewers().filter((viewer) => viewerNames.indexOf(viewer.NAME) !== -1).forEach((viewer) => { - const viewerInstance = new viewer.CONSTRUCTOR( - this.createViewerOptions({ - viewer - }) - ); - - if (typeof viewerInstance.prefetch === 'function') { - viewerInstance.prefetch({ - assets: true, - preload: false, - content: false - }); - } - }); + this.getViewers() + .filter((viewer) => viewerNames.indexOf(viewer.NAME) !== -1) + .forEach((viewer) => { + const viewerInstance = new viewer.CONSTRUCTOR( + this.createViewerOptions({ + viewer + }) + ); + + if (typeof viewerInstance.prefetch === 'function') { + viewerInstance.prefetch({ + assets: true, + preload: false, + content: false + }); + } + }); } //-------------------------------------------------------------------------- @@ -517,6 +551,9 @@ class Preview extends EventEmitter { } else if (checkFileValid(fileIdOrFile)) { // Use well-formed file object if available this.file = fileIdOrFile; + } else if (!!fileIdOrFile && typeof fileIdOrFile.id === 'string') { + // File is not a well-formed file object but has an id + this.file = { id: fileIdOrFile.id }; } else { throw new Error( 'File is not a well-formed Box File object. See FILE_FIELDS in file.js for a list of required fields.' @@ -530,14 +567,10 @@ class Preview extends EventEmitter { this.retryCount = 0; } - if (typeof fileIdOrFile === 'object') { - this.loadPreviewWithTokens({}); - } else { - // Fetch access tokens before proceeding - getTokens(this.file.id, this.previewOptions.token) - .then(this.loadPreviewWithTokens) - .catch(this.triggerFetchError); - } + // Fetch access tokens before proceeding + getTokens(this.file.id, this.previewOptions.token) + .then(this.loadPreviewWithTokens) + .catch(this.triggerFetchError); } /** @@ -573,13 +606,6 @@ class Preview extends EventEmitter { // Update navigation this.ui.showNavigation(this.file.id, this.collection); - // If preview collection is empty, create a collection of one with the - // current file ID. Otherwise, assume the current file ID is already in - // the collection. - if (this.collection.length === 0) { - this.collection = [this.file.id]; - } - if (checkFileValid(this.file)) { // Save file in cache. This also adds the 'ORIGINAL' representation. cacheFile(this.cache, this.file); @@ -640,9 +666,6 @@ class Preview extends EventEmitter { // Custom Box3D application definition this.options.box3dApplication = options.box3dApplication; - // Save the files to iterate through - this.collection = options.collection || []; - // Save the reference to any additional custom options for viewers this.options.viewers = options.viewers || {}; diff --git a/src/lib/__tests__/Preview-test.js b/src/lib/__tests__/Preview-test.js index abb6d58c4..0973fe3e1 100644 --- a/src/lib/__tests__/Preview-test.js +++ b/src/lib/__tests__/Preview-test.js @@ -100,6 +100,7 @@ describe('lib/Preview', () => { describe('show()', () => { beforeEach(() => { stubs.load = sandbox.stub(preview, 'load'); + stubs.updateCollection = sandbox.stub(preview, 'updateCollection'); }); it('should set the preview options with string token', () => { @@ -110,6 +111,7 @@ describe('lib/Preview', () => { }); }); + it('should set the preview options with function token', () => { const foo = () => {}; preview.show('123', foo, { viewer: 'viewer' }); @@ -119,11 +121,35 @@ describe('lib/Preview', () => { }); }); + it('should set the preview options with null token', () => { + preview.show('123', null); + expect(preview.previewOptions).to.deep.equal({ + token: null + }); + }); + + it('should set the preview options with no token', () => { + preview.show('123'); + expect(preview.previewOptions).to.deep.equal({ + token: undefined + }); + }); + + it('should call update collection with optional collection', () => { + preview.show('123', 'token', { collection: 'collection' }); + expect(stubs.updateCollection).to.be.calledWith('collection'); + }); + it('should load file associated with the passed in file ID', () => { preview.show('123', 'token'); expect(stubs.load).to.be.calledWith('123'); }); + it('should call update collection with passed in collection', () => { + preview.show('123', 'token', { collection: 'collection' }); + expect(stubs.updateCollection).to.be.calledWith('collection'); + }); + it('should load file matching the passed in file object', () => { const file = { id: '123', @@ -143,14 +169,14 @@ describe('lib/Preview', () => { expect(stubs.load).to.be.calledWith(file); }); - it('should throw an error if there is no auth token', () => { + it('should throw an error if auth token is a random object', () => { const spy = sandbox.spy(preview, 'show'); try { preview.show('123', {}); } catch (e) { expect(spy.threw()); - expect(e.message).to.equal('Missing access token!'); + expect(e.message).to.equal('Bad access token!'); } }); }); @@ -189,20 +215,45 @@ describe('lib/Preview', () => { describe('updateCollection()', () => { beforeEach(() => { stubs.showNavigation = sandbox.stub(preview.ui, 'showNavigation'); + stubs.updateFileCache = sandbox.stub(preview, 'updateFileCache'); }); - it('should set the preview and preview options collection to an array', () => { - let array = [1, 2, 3, 4]; + it('should set the preview collection to an array of file ids', () => { + let array = ['1', '2', '3', '4']; preview.updateCollection(array); + expect(stubs.updateFileCache).to.be.calledWith([]); expect(preview.collection).to.deep.equal(array); - expect(preview.previewOptions.collection).to.deep.equal(array); + }); + + it('should set the preview collection to an array of file ids when files passed in', () => { + let files = ['1', { id: '2' }, '3', { id: '4' }, { id: '5' }]; + + preview.updateCollection(files); + expect(stubs.updateFileCache).to.be.calledWith([{ id: '2' }, { id: '4' }, { id: '5' }]); + expect(preview.collection).to.deep.equal(['1', '2', '3', '4', '5']); + }); + + it('should throw when bad array of files passed in', () => { + let files = ['1', { }, '3']; - array = '1,2,3,4'; + expect(preview.updateCollection.bind(preview, files)).to.throw(Error, /Bad collection/); + expect(stubs.updateFileCache).to.not.be.called; + }); + + it('should throw when bad array of file ids passed in', () => { + let files = ['', '3']; + + expect(preview.updateCollection.bind(preview, files)).to.throw(Error, /Bad collection/); + expect(stubs.updateFileCache).to.not.be.called; + }); + + it('should reset the preview collection to an empty array', () => { + let array = '1,2,3,4'; preview.updateCollection(array); + expect(stubs.updateFileCache).to.be.calledWith([]); expect(preview.collection).to.deep.equal([]); - expect(preview.previewOptions.collection).to.deep.equal([]); }); it('should show navigation if the file exists', () => { @@ -675,12 +726,6 @@ describe('lib/Preview', () => { expect(preview.retryTimeout).to.equal(undefined); }); - it('should load preview when a well-formed file object is passed', () => { - preview.load(stubs.file); - expect(stubs.loadPreviewWithTokens).to.be.calledWith({}); - expect(stubs.getTokens).to.not.be.called; - }); - it('should set the retry count', () => { preview.retryCount = 0; preview.file.id = '0'; @@ -697,17 +742,21 @@ describe('lib/Preview', () => { it('should throw an error if incompatible file object is passed in', () => { const spy = sandbox.spy(preview, 'load'); const file = { - id: '123', not: 'the', right: 'fields' } - try { - preview.load(file); - } catch (e) { - expect(spy.threw()); - expect(e.message).to.equal('File is not a well-formed Box File object. See FILE_FIELDS in file.js for a list of required fields.'); - } + expect(preview.load.bind(preview, file)).to.throw(Error, 'File is not a well-formed Box File object. See FILE_FIELDS in file.js for a list of required fields.'); + }); + + it('should get the tokens when file id is available', () => { + preview.previewOptions.token = 'token'; + + preview.load({ id: '123' }); + return stubs.promise.then(() => { + expect(stubs.getTokens).to.be.calledWith('123', 'token'); + expect(stubs.loadPreviewWithTokens).to.be.called; + }); }); it('should get the tokens and either handle the response or error', () => { @@ -885,12 +934,6 @@ describe('lib/Preview', () => { expect(preview.options.skipServerUpdate).to.be.true; }); - it('should save the files to iterate through and any options for custom viewers', () => { - preview.parseOptions(preview.previewOptions, stubs.tokens); - expect(preview.collection).to.equal(stubs.collection); - expect(preview.options.viewers instanceof Object).to.be.true; - }); - it('should add user created loaders before standard loaders', () => { const expectedLoaders = stubs.loaders.concat(loaders); diff --git a/src/lib/__tests__/tokens-test.js b/src/lib/__tests__/tokens-test.js index 8e7e3402a..f841b0e68 100644 --- a/src/lib/__tests__/tokens-test.js +++ b/src/lib/__tests__/tokens-test.js @@ -5,19 +5,45 @@ describe('lib/tokens', () => { return Promise.resolve('token'); } - function mapTokenFunction(ids) { - return Promise.resolve({ - [ids[0]]: 'token1', - [ids[1]]: 'token2' - }); + function mapIdTokenFunction(ids) { + if (ids.length > 1) { + return Promise.resolve({ + [ids[0].replace('file_', '')]: 'token1', + [ids[1].replace('file_', '')]: 'token2' + }); + } else { + return Promise.resolve({ + [ids[0].replace('file_', '')]: 'token1' + }); + } + } + + function mapTypedIdTokenFunction(ids) { + if (ids.length > 1) { + return Promise.resolve({ + [ids[0]]: 'token1', + [ids[1]]: 'token2' + }); + } else { + return Promise.resolve({ + [ids[0]]: 'token1' + }); + } } describe('getTokens', () => { it('should throw an error when no id provided', () => { return getTokens(null, 'token').should.be.rejectedWith(Error); }); - it('should throw an error when no token provided', () => { - return getTokens('123').should.be.rejectedWith(Error); + it('should use undefined token when no token provided', () => { + return getTokens('123').then((data) => { + assert.equal(undefined, data['123']); + }); + }); + it('should use null token when null token provided', () => { + return getTokens('123', null).then((data) => { + assert.equal(null, data['123']); + }); }); it('should create id token map with string token and string id', () => { return getTokens('123', 'token').then((data) => { @@ -42,18 +68,29 @@ describe('lib/tokens', () => { }); }); it('should create id token map with function token that returns map and string id', () => { - return getTokens('123', mapTokenFunction).then((data) => { + return getTokens('123', mapIdTokenFunction).then((data) => { assert.equal('token1', data['123']); }); }); it('should create id token map with function token that returns map and array of string ids', () => { - return getTokens(['123', '456'], mapTokenFunction).then((data) => { + return getTokens(['123', '456'], mapIdTokenFunction).then((data) => { + assert.equal('token1', data['123']); + assert.equal('token2', data['456']); + }); + }); + it('should create id token map with function token that returns map and string typed id', () => { + return getTokens('123', mapTypedIdTokenFunction).then((data) => { + assert.equal('token1', data['123']); + }); + }); + it('should create id token map with function token that returns map and array of string typed ids', () => { + return getTokens(['123', '456'], mapTypedIdTokenFunction).then((data) => { assert.equal('token1', data['123']); assert.equal('token2', data['456']); }); }); it('should throw an error when not all tokens could be fetched', () => { - return getTokens(['123', '456', '789'], mapTokenFunction).should.be.rejectedWith(Error); + return getTokens(['123', '456', '789'], mapIdTokenFunction).should.be.rejectedWith(Error); }); }); }); diff --git a/src/lib/tokens.js b/src/lib/tokens.js index 99e6acd83..de7699e31 100644 --- a/src/lib/tokens.js +++ b/src/lib/tokens.js @@ -1,19 +1,46 @@ // Create an error to throw if needed -const error = new Error('Missing Auth Token!'); +const error = new Error('Bad Auth Token!'); + +/** + * Helper function to return typed ids. + * Token service uses typed ids but preview + * deals with simple file ids. + * + * @private + * @param {string} id - Box file ID + * @return {Object} ID to token map + */ +function getTypedId(id) { + return `file_${id}`; +} /** * Helper function to create token map used below. - * Maps the same token to multiple files. + * Maps one or more tokens to multiple files. * * @private * @param {Array} ids - Box file IDs - * @param {string} token - Token to use for map + * @param {string} [tokenOrTokens] - Single token or map * @return {Object} ID to token map */ -function createIdTokenMap(ids, token) { +function createIdTokenMap(ids, tokenOrTokens) { const tokenMap = {}; ids.forEach((id) => { - tokenMap[id] = token; // all files use the same token + const typedId = getTypedId(id); + if (!tokenOrTokens || typeof tokenOrTokens === 'string') { + // All files use the same string or null or undefined token + tokenMap[id] = tokenOrTokens; + } else if (typeof tokenOrTokens === 'object' && !!tokenOrTokens[typedId]) { + // Map typedIds and tokens to ids and tokens + tokenMap[id] = tokenOrTokens[typedId]; + } else if (typeof tokenOrTokens === 'object' && !!tokenOrTokens[id]) { + // This use case is only there for backwards compatibility. + // Remove once old token service is sending back typed ids. + tokenMap[id] = tokenOrTokens[id]; + } else { + // We are missing requested tokens or bad token was provided + throw error; + } }); return tokenMap; } @@ -23,7 +50,8 @@ function createIdTokenMap(ids, token) { * The token can either be a simple string or a function that returns * a promise which resolves to a key value map where key is the file * id and value is the token. The function accepts either a simple id - * or an array of file ids + * or an array of file ids. Tokens can also be null or undefined for + * use cases where token is not needed to make XHRs. * * @public * @param {string|Array} id - box file id(s) @@ -31,45 +59,42 @@ function createIdTokenMap(ids, token) { * @return {Promise} Promise that resolves with ID to token map */ export default function getTokens(id, token) { - // Access token should be available - if (!token || !id) { + // Throw an error when no id but allow null or undefined tokens + if (!id) { return Promise.reject(error); } - let ids = [id]; + let ids; - // If instead id(s) were passed in, we fetch those - // This will be the use case for prefetch and viewers - // Normalize to an array so that we always deal with ids + // Tokens for single or mltiple ids can be requested. + // Normalize to an array so that we always deal with ids. if (Array.isArray(id)) { ids = id; + } else { + ids = [id]; + } + + if (!token || typeof token === 'string') { + // Token is a simple string or null or undefined + return Promise.resolve(createIdTokenMap(ids, token)); } + // Token is a service function that returns a promise + // that resolves to string tokens. Token service requires + // typed ids to be passed in to distinguish between + // possible item types. Preview only deals with files + // so all ids should be prefixed with file_. return new Promise((resolve, reject) => { - if (typeof token === 'function') { - // Token may be a function that returns a promise - token(ids).then((tokens) => { - // Resolved tokens can either be a map of { id: token } - // or it can just be a single string token that applies - // to all the files irrespective of the id. - if (typeof tokens === 'string') { - // String token which is the same for all files - resolve(createIdTokenMap(ids, tokens)); - } else { - // Iterate over all the requested file ids - // and make sure we got them back otherwise - // throw and error about missing tokens - if (!ids.every((fileId) => !!tokens[fileId])) { - reject(error); - } - resolve(tokens); - } - }); - } else { - // Token may just be a string, create a map - // from id to token to normalize. In this case - // the value is going to be the same for all files - resolve(createIdTokenMap(ids, token)); - } + const typedIds = ids.map((fileId) => getTypedId(fileId)); + token(typedIds).then((tokens) => { + // Resolved tokens can either be a map of { typedId: token } + // or it can just be a single string token that applies + // to all the files irrespective of their id. + try { + resolve(createIdTokenMap(ids, tokens)); + } catch (err) { + reject(err); + } + }); }); }