Skip to content

Commit

Permalink
New: Support well-formed file object in show() (#328)
Browse files Browse the repository at this point in the history
- Partially reverts changes from #24.
- Adds 'id' to FILE_FIELDS since we use that to validate the file object passed in
  • Loading branch information
tonyjin authored Aug 22, 2017
1 parent 2889b0d commit 540a011
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 27 deletions.
26 changes: 18 additions & 8 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,21 +148,21 @@ class Preview extends EventEmitter {
/**
* Primary function for showing a preview of a file.
*
* @param {string} fileId - Box File ID
* @param {string|Object} fileIdOrFile - Box File ID or well-formed Box File object
* @param {string|Function} token - Access token string or generator function
* @param {Object} [options] - Optional preview options
* @return {void}
*/
show(fileId, token, options = {}) {
show(fileIdOrFile, token, options = {}) {
// Save a reference to the options to be used later
if (typeof token === 'string' || typeof token === 'function') {
this.previewOptions = Object.assign({}, options, { token });
} else {
throw new Error('Missing access token!');
}

// load the preview
this.load(fileId);
// Load the preview
this.load(fileIdOrFile);
}

/**
Expand Down Expand Up @@ -473,10 +473,10 @@ class Preview extends EventEmitter {
* Initial method for loading a preview.
*
* @private
* @param {string} fileId - Box File ID
* @param {string|Object} fileIdOrFile - Box File ID or well-formed Box File object
* @return {void}
*/
load(fileId) {
load(fileIdOrFile) {
// Clean up any existing previews before loading
this.destroy();

Expand All @@ -492,8 +492,18 @@ class Preview extends EventEmitter {
// Save reference to the currently shown file, if any
const currentFileId = this.file ? this.file.id : undefined;

// Use cached file data if available, otherwise create empty file object
this.file = this.cache.get(fileId) || { id: fileId };
// Check if file ID or well-formed file object was passed in
if (typeof fileIdOrFile === 'string') {
// Use cached file data if available, otherwise create empty file object
this.file = this.cache.get(fileIdOrFile) || { id: fileIdOrFile };
} else if (checkFileValid(fileIdOrFile)) {
// Use well-formed file object if available
this.file = fileIdOrFile;
} 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.'
);
}

// Retry up to RETRY_COUNT if we are reloading same file
if (this.file.id === currentFileId) {
Expand Down
76 changes: 68 additions & 8 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe('lib/Preview', () => {
});

it('should set the preview options with string token', () => {
preview.show('file', 'token', { viewer: 'viewer' });
preview.show('123', 'token', { viewer: 'viewer' });
expect(preview.previewOptions).to.deep.equal({
token: 'token',
viewer: 'viewer'
Expand All @@ -112,23 +112,42 @@ describe('lib/Preview', () => {

it('should set the preview options with function token', () => {
const foo = () => {};
preview.show('file', foo, { viewer: 'viewer' });
preview.show('123', foo, { viewer: 'viewer' });
expect(preview.previewOptions).to.deep.equal({
token: foo,
viewer: 'viewer'
});
});

it('should load the given file', () => {
preview.show('file', 'token');
expect(stubs.load).to.be.calledWith('file');
it('should load file associated with the passed in file ID', () => {
preview.show('123', 'token');
expect(stubs.load).to.be.calledWith('123');
});

it('should load file matching the passed in file object', () => {
const file = {
id: '123',
permissions: {},
shared_link: null,
sha1: 'sha1',
file_version: {},
name: 'blah',
size: 123,
extension: 'pdf',
representations: {},
watermark_info: {},
authenticated_download_url: 'url'
}

preview.show(file, 'foken');
expect(stubs.load).to.be.calledWith(file);
});

it('should throw an error if there is no auth token', () => {
const spy = sandbox.spy(preview, 'show');

try {
preview.show('file', {});
preview.show('123', {});
} catch (e) {
expect(spy.threw());
expect(e.message).to.equal('Missing access token!');
Expand Down Expand Up @@ -592,7 +611,7 @@ describe('lib/Preview', () => {
describe('updateToken()', () => {
it('should update token in options with the passed in string or function', () => {
const newToken = 'daredevil';
preview.updateToken(newToken);
preview.updateToken(newToken, false);
expect(preview.previewOptions.token).to.equal(newToken);
});

Expand All @@ -604,7 +623,7 @@ describe('lib/Preview', () => {
});

it('should not reload preview if reloadPreview is false', () => {
preview.file = {};
preview.file = { id: '123' };
sandbox.stub(preview, 'load');
preview.updateToken('nick-fury', false);
expect(preview.load).to.not.be.calledWith(preview.file);
Expand Down Expand Up @@ -656,6 +675,22 @@ describe('lib/Preview', () => {
expect(preview.retryCount).to.equal(0);
});

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.');
}
});

it('should get the tokens and either handle the response or error', () => {
preview.previewOptions.token = 'token';

Expand All @@ -665,6 +700,31 @@ describe('lib/Preview', () => {
expect(stubs.loadPreviewWithTokens).to.be.called;
});
});

it('should accept a well-formed file object', () => {
const token = 'token';
const file = {
id: '123',
permissions: {},
shared_link: null,
sha1: 'sha1',
file_version: {},
name: 'blah',
size: 123,
extension: 'pdf',
representations: {},
watermark_info: {},
authenticated_download_url: 'url'
}
preview.previewOptions.token = token;

preview.load(file);

return stubs.promise.then(() => {
expect(stubs.getTokens).to.be.calledWith(file.id, token);
expect(stubs.loadPreviewWithTokens).to.be.called;
});
});
});

describe('loadPreviewWithTokens()', () => {
Expand Down
14 changes: 8 additions & 6 deletions src/lib/__tests__/file-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('lib/file', () => {
it('should return the correct api url', () => {
assert.equal(
getURL('id', 'api'),
'api/2.0/files/id?fields=permissions,shared_link,sha1,file_version,name,size,extension,representations,watermark_info,authenticated_download_url'
'api/2.0/files/id?fields=id,permissions,shared_link,sha1,file_version,name,size,extension,representations,watermark_info,authenticated_download_url'
);
});
});
Expand Down Expand Up @@ -89,18 +89,20 @@ describe('lib/file', () => {
});

describe('checkFileValid()', () => {
it('should return false if file is null', () => {
const file = null;
it('should return false if file is null or undefined or not an object', () => {
let file = null;
assert.notOk(checkFileValid(file));

file = undefined;
assert.notOk(checkFileValid(file));
});

it('should return false if file is null', () => {
const file = undefined;
file = 'string';
assert.notOk(checkFileValid(file));
});

it('should return true if file has all the appropratie properties', () => {
const file = {
id: '123',
permissions: {},
shared_link: 'blah',
sha1: 'blah',
Expand Down
11 changes: 6 additions & 5 deletions src/lib/file.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { ORIGINAL_REP_NAME } from './constants';

// List of Box Content API fields that the Preview SDK requires for every file. Updating this list is most likely
// List of Box Content API fields that the Preview library requires for every file. Updating this list is most likely
// a breaking change and should be done with care. Clients that leverage functionality dependent on this format
// (e.g. Box.Preview.updateFileCache()) will need to be updated if this list is modified.
const FILE_FIELDS = [
'id',
'permissions',
'shared_link',
'sha1',
Expand Down Expand Up @@ -84,14 +85,14 @@ export function checkFeature(viewer, primary, secondary) {
}

/**
* Checks whether file metadata is valid by checking whether each property
* Checks whether Box File object is valid by checking whether each property
* in FIELDS on the specified file object is defined.
*
* @param {Object} file - Box file metadata to check
* @return {boolean} Whether or not file metadata structure is valid
* @param {Object} file - Box File object to check
* @return {boolean} Whether or not file object structure is valid
*/
export function checkFileValid(file) {
if (!file) {
if (!file || typeof file !== 'object') {
return false;
}

Expand Down

0 comments on commit 540a011

Please sign in to comment.