Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: Support additional query params in requests #405

Merged
merged 1 commit into from
Sep 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

/* 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