Skip to content

Commit

Permalink
Update: Support additional query params in requests (#405)
Browse files Browse the repository at this point in the history
- Replace manual URL manipulation with jsuri library
- Add support for new `queryParams` Preview option. When used, each key/value pair will be appended to all API requests as query params
- Refactor some existing query & auth param logic
  • Loading branch information
tonyjin authored Sep 21, 2017
1 parent 1986f9a commit 08da16b
Show file tree
Hide file tree
Showing 14 changed files with 144 additions and 67 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"file-loader": "^0.10.1",
"husky": "^0.13.4",
"i18n-webpack-plugin": "^1.0.0",
"jsuri": "^1.3.1",
"karma": "^1.5.0",
"karma-chai": "^0.1.0",
"karma-chai-as-promised": "^0.1.2",
Expand Down
33 changes: 28 additions & 5 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,15 @@ import Cache from './Cache';
import PreviewErrorViewer from './viewers/error/PreviewErrorViewer';
import PreviewUI from './PreviewUI';
import getTokens from './tokens';
import { get, post, decodeKeydown, openUrlInsideIframe, getHeaders, findScriptLocation } from './util';
import {
get,
post,
decodeKeydown,
openUrlInsideIframe,
getHeaders,
findScriptLocation,
appendQueryParams
} from './util';
import { getURL, getDownloadURL, checkPermission, checkFeature, checkFileValid, cacheFile, uncacheFile } from './file';
import {
API_HOST,
Expand Down Expand Up @@ -395,8 +403,12 @@ class Preview extends EventEmitter {
* @return {void}
*/
download() {
const { apiHost, queryParams } = this.options;

if (checkPermission(this.file, PERMISSION_DOWNLOAD)) {
get(getDownloadURL(this.file.id, this.options.apiHost), this.getRequestHeaders()).then((data) => {
// Append optional query params
const downloadUrl = appendQueryParams(getDownloadURL(this.file.id, apiHost), queryParams);
get(downloadUrl, this.getRequestHeaders()).then((data) => {
openUrlInsideIframe(data.download_url);
});
}
Expand Down Expand Up @@ -673,6 +685,9 @@ class Preview extends EventEmitter {
// Skip load from server and any server updates
this.options.skipServerUpdate = !!options.skipServerUpdate;

// Optional additional query params to append to requests
this.options.queryParams = options.queryParams || {};

// Prefix any user created loaders before our default ones
this.loaders = (options.loaders || []).concat(loaderList);

Expand Down Expand Up @@ -728,7 +743,10 @@ class Preview extends EventEmitter {
* @return {void}
*/
loadFromServer() {
get(getURL(this.file.id, this.options.apiHost), this.getRequestHeaders())
const { apiHost, queryParams } = this.options;

const fileInfoUrl = appendQueryParams(getURL(this.file.id, apiHost), queryParams);
get(fileInfoUrl, this.getRequestHeaders())
.then(this.handleLoadResponse)
.catch(this.triggerFetchError);
}
Expand Down Expand Up @@ -1120,8 +1138,10 @@ class Preview extends EventEmitter {
* @return {void}
*/
prefetchNextFiles() {
const { apiHost, queryParams, skipServerUpdate } = this.options;

// Don't bother prefetching when there aren't more files or we need to skip server update
if (this.collection.length < 2 || this.options.skipServerUpdate) {
if (this.collection.length < 2 || skipServerUpdate) {
return;
}

Expand All @@ -1145,8 +1165,11 @@ class Preview extends EventEmitter {
filesToPrefetch.forEach((id) => {
const token = tokenMap[id];

// Append optional query params
const fileInfoUrl = appendQueryParams(getURL(id, apiHost), queryParams);

// Prefetch and cache file information and content
get(getURL(id, this.options.apiHost), this.getRequestHeaders(token))
get(fileInfoUrl, this.getRequestHeaders(token))
.then((file) => {
// Cache file info
cacheFile(this.cache, file);
Expand Down
40 changes: 38 additions & 2 deletions src/lib/__tests__/util-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,42 @@ describe('lib/util', () => {
});
});

describe('appendQueryParams()', () => {
it('should return original url when queryParams is null', () => {
const url = 'foo';
expect(util.appendQueryParams(url, null)).to.equal(url);
});

it('should return original url when queryParams is empty object', () => {
const url = 'foo';
expect(util.appendQueryParams(url, {})).to.equal(url);
});

it('should append query params to url', () => {
const url = 'foo';
expect(util.appendQueryParams(url, {
foo: 'bar',
baz: 'boo'
})).to.equal(`${url}/?foo=bar&baz=boo`);
});

it('should correctly append new query params to url', () => {
const url = 'foo?test=hah';
expect(util.appendQueryParams(url, {
foo: 'bar',
baz: 'boo'
})).to.equal(`foo/?test=hah&foo=bar&baz=boo`);
});

it('should replace values for existing keys', () => {
const url = 'test.com/?foo=hah'
expect(util.appendQueryParams(url, {
foo: 'bar',
baz: 'boo'
})).to.equal('test.com/?foo=bar&baz=boo');
});
});

describe('appendAuthParams()', () => {
it('should return url when no token or shared link is provided', () => {
const url = 'foo';
Expand All @@ -246,7 +282,7 @@ describe('lib/util', () => {
const token = 'sometoken';
const sharedLink = 'someSharedLink';
expect(util.appendAuthParams(url, token, sharedLink)).to.equal(
`${url}?access_token=${token}&shared_link=${sharedLink}&box_client_name=${__NAME__}&box_client_version=${__VERSION__}`
`${url}/?access_token=${token}&shared_link=${sharedLink}&box_client_name=${__NAME__}&box_client_version=${__VERSION__}`
);
});

Expand All @@ -256,7 +292,7 @@ describe('lib/util', () => {
const sharedLink = 'someSharedLink';
const sharedLinkPassword = 'somePass';
expect(util.appendAuthParams(url, token, sharedLink, sharedLinkPassword)).to.equal(
`foobar?access_token=${token}&shared_link=${sharedLink}&shared_link_password=${sharedLinkPassword}&box_client_name=${__NAME__}&box_client_version=${__VERSION__}`
`${url}/?access_token=${token}&shared_link=${sharedLink}&shared_link_password=${sharedLinkPassword}&box_client_name=${__NAME__}&box_client_version=${__VERSION__}`
);
});
});
Expand Down
4 changes: 3 additions & 1 deletion src/lib/file.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { appendQueryParams } from './util';
import { ORIGINAL_REP_NAME } from './constants';

// List of Box Content API fields that the Preview library requires for every file. Updating this list is most likely
Expand Down Expand Up @@ -122,9 +123,10 @@ function addOriginalRepresentation(file) {
}

// Add an original representation if it doesn't already exist
const template = appendQueryParams(file.authenticated_download_url, { preview: 'true' });
file.representations.entries.push({
content: {
url_template: `${file.authenticated_download_url}?preview=true`
url_template: template
},
representation: ORIGINAL_REP_NAME,
status: {
Expand Down
75 changes: 37 additions & 38 deletions src/lib/util.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import Uri from 'jsuri';
import 'whatwg-fetch';

const HEADER_CLIENT_NAME = 'X-Box-Client-Name';
const HEADER_CLIENT_VERSION = 'X-Box-Client-Version';
const PARAM_CLIENT_NAME = 'box_client_name';
const PARAM_CLIENT_VERSION = 'box_client_version';
const CLIENT_NAME_KEY = 'box_client_name';
const CLIENT_VERSION_KEY = 'box_client_version';
/* eslint-disable no-undef */
const CLIENT_NAME = __NAME__;
const CLIENT_VERSION = __VERSION__;
Expand Down Expand Up @@ -336,6 +337,33 @@ export function getHeaders(headers = {}, token = '', sharedLink = '', password =
return headers;
}

/**
* Helper function for appending query params to a url
*
* @param {string} url - Original url
* @param {Object} queryParams - Query params object with key value pairs
* @return {string} Url with query params appended
*/
export function appendQueryParams(url, queryParams) {
if (!queryParams) {
return url;
}

const uri = new Uri(url);
Object.keys(queryParams).forEach((key) => {
const value = queryParams[key];
if (value) {
if (uri.hasQueryParam(key)) {
uri.replaceQueryParam(key, value);
} else {
uri.addQueryParam(key, value);
}
}
});

return uri.toString();
}

/**
* Appends auth params to a url
*
Expand All @@ -351,37 +379,13 @@ export function appendAuthParams(url, token = '', sharedLink = '', password = ''
return url;
}

let delim = '?';

if (url.indexOf('?') > 0) {
delim = '&';
}

let params = '';
if (token) {
params = `access_token=${token}`;
}

if (sharedLink) {
if (params) {
params = `${params}&`;
}
params = `${params}shared_link=${encodeURI(sharedLink)}`;
if (password) {
params = `${params}&shared_link_password=${encodeURI(password)}`;
}
}

// Following params are for API analytics
if (CLIENT_NAME) {
params = `${params}&${PARAM_CLIENT_NAME}=${encodeURI(CLIENT_NAME)}`;
}

if (CLIENT_VERSION) {
params = `${params}&${PARAM_CLIENT_VERSION}=${encodeURI(CLIENT_VERSION)}`;
}

return `${url}${delim}${params}`;
return appendQueryParams(url, {
access_token: token,
shared_link: sharedLink,
shared_link_password: password,
[CLIENT_NAME_KEY]: CLIENT_NAME,
[CLIENT_VERSION_KEY]: CLIENT_VERSION
});
}

/**
Expand All @@ -393,11 +397,6 @@ export function appendAuthParams(url, token = '', sharedLink = '', password = ''
* @return {string} Content url
*/
export function createContentUrl(template, asset) {
// @NOTE(tjin): Remove the next 3 lines after reps API is stabilized after 4/6/17
/* eslint-disable no-param-reassign */
template = template.replace('{asset_path}', asset || '');
/* eslint-enable no-param-reassign */

return template.replace('{+asset_path}', asset || '');
}

Expand Down
11 changes: 9 additions & 2 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import debounce from 'lodash.debounce';
import fullscreen from '../Fullscreen';
import RepStatus from '../RepStatus';
import {
appendQueryParams,
appendAuthParams,
getHeaders,
createContentUrl,
Expand Down Expand Up @@ -297,7 +298,9 @@ class BaseViewer extends EventEmitter {
* @return {string} content url
*/
createContentUrl(template, asset) {
return createContentUrl(template, asset);
// Append optional query params
const { queryParams } = this.options;
return appendQueryParams(createContentUrl(template, asset), queryParams);
}

/**
Expand All @@ -311,7 +314,11 @@ class BaseViewer extends EventEmitter {
* @return {string} content url
*/
createContentUrlWithAuthParams(template, asset) {
return this.appendAuthParams(this.createContentUrl(template, asset));
const urlWithAuthParams = this.appendAuthParams(createContentUrl(template, asset));

// Append optional query params
const { queryParams } = this.options;
return appendQueryParams(urlWithAuthParams, queryParams);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ describe('lib/viewers/BaseViewer', () => {

describe('createContentUrl()', () => {
it('should return content url with no asset path', () => {
const url = 'url{asset_path}';
const url = 'url{+asset_path}';
sandbox.spy(util, 'createContentUrl');

const result = base.createContentUrl(url, '');
Expand All @@ -227,7 +227,7 @@ describe('lib/viewers/BaseViewer', () => {
});

it('should return content url with asset path from args', () => {
const url = 'url{asset_path}';
const url = 'url{+asset_path}';

base = new BaseViewer({
viewer: { ASSET: 'foo' },
Expand All @@ -246,11 +246,11 @@ describe('lib/viewers/BaseViewer', () => {

describe('createContentUrlWithAuthParams()', () => {
it('should return content url with no asset path', () => {
sandbox.stub(base, 'createContentUrl').returns('foo');
sandbox.stub(util, 'createContentUrl').returns('foo');
sandbox.stub(base, 'appendAuthParams').returns('bar');
const result = base.createContentUrlWithAuthParams('boo', 'hoo');
expect(result).to.equal('bar');
expect(base.createContentUrl).to.be.calledWith('boo', 'hoo');
expect(util.createContentUrl).to.be.calledWith('boo', 'hoo');
expect(base.appendAuthParams).to.be.calledWith('foo');
});
});
Expand Down
15 changes: 9 additions & 6 deletions src/lib/viewers/box3d/video360/Video360Viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,15 @@ class Video360Viewer extends DashViewer {
this.renderer.on(EVENT_SHOW_VR_BUTTON, this.handleShowVrButton);
this.renderer.staticBaseURI = this.options.location.staticBaseURI;
this.options.sceneEntities = sceneEntities;
this.renderer.initBox3d(this.options).then(this.create360Environment).then(() => {
// calling super.loadeddataHandler() will ready video playback
super.loadeddataHandler();
this.createControls();
this.renderer.initVr();
});
this.renderer
.initBox3d(this.options)
.then(this.create360Environment)
.then(() => {
// calling super.loadeddataHandler() will ready video playback
super.loadeddataHandler();
this.createControls();
this.renderer.initVr();
});
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/lib/viewers/image/ImageViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class ImageViewer extends ImageBaseViewer {
* @return {void}
*/
rotateLeft() {
this.currentRotationAngle = (this.currentRotationAngle - 90) % 3600 % 360;
this.currentRotationAngle = ((this.currentRotationAngle - 90) % 3600) % 360;
this.imageEl.setAttribute('data-rotation-angle', this.currentRotationAngle);
this.imageEl.style.transform = `rotate(${this.currentRotationAngle}deg)`;
this.emit('rotate');
Expand Down Expand Up @@ -238,7 +238,7 @@ class ImageViewer extends ImageBaseViewer {
this.scale = width
? width / this.imageEl.getAttribute('originalWidth')
: height / this.imageEl.getAttribute('originalHeight');
this.rotationAngle = this.currentRotationAngle % 3600 % 360;
this.rotationAngle = (this.currentRotationAngle % 3600) % 360;
this.emit('scale', {
scale: this.scale,
rotationAngle: this.rotationAngle
Expand Down Expand Up @@ -397,7 +397,7 @@ class ImageViewer extends ImageBaseViewer {
this.adjustImageZoomPadding();

this.scale = this.imageEl.clientWidth / this.imageEl.getAttribute('originalWidth');
this.rotationAngle = this.currentRotationAngle % 3600 % 360;
this.rotationAngle = (this.currentRotationAngle % 3600) % 360;
this.emit('scale', {
scale: this.scale,
rotationAngle: this.rotationAngle
Expand Down
2 changes: 1 addition & 1 deletion src/lib/viewers/image/__tests__/MultiImageViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ describe('lib/viewers/image/MultiImageViewer', () => {
}
}
};
const result2 = multiImage.constructImageUrls('file/100/content/{asset_path}');
const result2 = multiImage.constructImageUrls('file/100/content/{+asset_path}');
expect(result2[0]).to.equal(firstURL);
});

Expand Down
Loading

0 comments on commit 08da16b

Please sign in to comment.