Skip to content

Commit

Permalink
Upgrade: Migrate from whatwg-fetch to axios for all requests (#939)
Browse files Browse the repository at this point in the history
  • Loading branch information
jstoffan authored Feb 22, 2019
1 parent f0507bb commit fb76b13
Show file tree
Hide file tree
Showing 31 changed files with 459 additions and 497 deletions.
7 changes: 2 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@
"eslint-plugin-jsx-a11y": "^6.0.2",
"eslint-plugin-react": "^7.5.1",
"extract-text-webpack-plugin": "3.0.2",
"fetch-mock": "^5.13.1",
"fetch-mock-forwarder": "^1.0.0",
"file-loader": "^1.1.5",
"fscreen": "^1.0.2",
"husky": "^0.14.3",
Expand Down Expand Up @@ -88,7 +86,7 @@
"react-virtualized": "^9.13.0",
"sass-loader": "^6.0.6",
"selenium-webdriver": "^3.6.0",
"sinon": "^7.2.2",
"sinon": "^7.2.3",
"sinon-chai": "3.3.0",
"string-replace-loader": "^1.3.0",
"style-loader": "^0.19.0",
Expand All @@ -100,8 +98,7 @@
"webdriverio": "^4.12.0",
"webpack": "^3.10.0",
"webpack-bundle-analyzer": "^2.9.1",
"webpack-dev-server": "^2.11.3",
"whatwg-fetch": "^2.0.3"
"webpack-dev-server": "^2.11.3"
},
"engines": {
"node": ">=8.9.4",
Expand Down
6 changes: 5 additions & 1 deletion src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@
/* global Box */
var preview = new Box.Preview();

var previewOptions = options || {};
var previewOptions = options || {
enableThumbnailsSidebar: true,
showAnnotations: true,
showDownload: true,
};
previewOptions.container = '.preview-container';

preview.show(fileid, token, previewOptions);
Expand Down
4 changes: 3 additions & 1 deletion src/lib/DownloadReachability.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import api from './api';
import { openUrlInsideIframe, isLocalStorageAvailable } from './util';

const DEFAULT_DOWNLOAD_HOST_PREFIX = 'https://dl.';
Expand Down Expand Up @@ -147,7 +148,8 @@ class DownloadReachability {
* @return {void}
*/
static setDownloadReachability(downloadUrl) {
return fetch(downloadUrl, { method: 'HEAD' })
return api
.head(downloadUrl)
.then(() => {
return Promise.resolve(false);
})
Expand Down
21 changes: 12 additions & 9 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import EventEmitter from 'events';
import cloneDeep from 'lodash/cloneDeep';
import throttle from 'lodash/throttle';
/* eslint-enable import/first */
import api from './api';
import Browser from './Browser';
import Logger from './Logger';
import loaderList from './loaders';
Expand All @@ -15,9 +16,7 @@ import getTokens from './tokens';
import Timer from './Timer';
import DownloadReachability from './DownloadReachability';
import {
get,
getProp,
post,
decodeKeydown,
getHeaders,
findScriptLocation,
Expand Down Expand Up @@ -544,7 +543,7 @@ class Preview extends EventEmitter {
// Otherwise, get the content download URL of the original file and download
} else {
const getDownloadUrl = appendQueryParams(getDownloadURL(this.file.id, apiHost), queryParams);
get(getDownloadUrl, this.getRequestHeaders()).then((data) => {
api.get(getDownloadUrl, { headers: this.getRequestHeaders() }).then((data) => {
const downloadUrl = appendQueryParams(data.download_url, queryParams);
DownloadReachability.downloadWithReachabilityCheck(downloadUrl);
});
Expand Down Expand Up @@ -1009,7 +1008,8 @@ class Preview extends EventEmitter {
Timer.start(tag);

const fileInfoUrl = appendQueryParams(getURL(this.file.id, fileVersionId, apiHost), params);
get(fileInfoUrl, this.getRequestHeaders())
api
.get(fileInfoUrl, { headers: this.getRequestHeaders() })
.then(this.handleFileInfoResponse)
.catch(this.handleFetchError);
}
Expand Down Expand Up @@ -1349,15 +1349,17 @@ class Preview extends EventEmitter {
this.logRetryCount = this.logRetryCount || 0;

const { apiHost, token, sharedLink, sharedLinkPassword } = options;
const headers = getHeaders({}, token, sharedLink, sharedLinkPassword);

post(`${apiHost}/2.0/events`, headers, {
const data = {
event_type: 'preview',
source: {
type: 'file',
id: fileId
}
})
};
const headers = getHeaders({}, token, sharedLink, sharedLinkPassword);

api
.post(`${apiHost}/2.0/events`, data, { headers })
.then(() => {
// Reset retry count after successfully logging
this.logRetryCount = 0;
Expand Down Expand Up @@ -1637,7 +1639,8 @@ class Preview extends EventEmitter {
const fileInfoUrl = appendQueryParams(getURL(fileId, fileVersionId, apiHost), params);

// Prefetch and cache file information and content
get(fileInfoUrl, this.getRequestHeaders(token))
api
.get(fileInfoUrl, { headers: this.getRequestHeaders(token) })
.then((file) => {
// Cache file info
cacheFile(this.cache, file);
Expand Down
5 changes: 3 additions & 2 deletions src/lib/RepStatus.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import EventEmitter from 'events';
import { get, appendAuthParams } from './util';
import api from './api';
import { appendAuthParams } from './util';
import { STATUS_SUCCESS, STATUS_VIEWABLE, STATUS_PENDING, STATUS_NONE } from './constants';
import PreviewError from './PreviewError';
import Timer from './Timer';
Expand Down Expand Up @@ -84,7 +85,7 @@ class RepStatus extends EventEmitter {
const tag = Timer.createTag(this.fileId, LOAD_METRIC.convertTime);
Timer.start(tag);

return get(this.infoUrl).then((info) => {
return api.get(this.infoUrl).then((info) => {
clearTimeout(this.statusTimeout);

if (info.metadata) {
Expand Down
10 changes: 3 additions & 7 deletions src/lib/__tests__/DownloadReachability-test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
/* eslint-disable no-unused-expressions */
import 'whatwg-fetch';
import fetchMock from 'fetch-mock';
import DownloadReachability from '../DownloadReachability';
import * as util from '../util';
import api from '../api';
import DownloadReachability from '../DownloadReachability';

const sandbox = sinon.sandbox.create();

Expand Down Expand Up @@ -130,12 +129,9 @@ describe('lib/DownloadReachability', () => {
});

describe('setDownloadReachability()', () => {
afterEach(() => {
fetchMock.restore();
});
it('should catch an errored response', () => {
const setDownloadHostFallbackStub = sandbox.stub(DownloadReachability, 'setDownloadHostFallback');
fetchMock.head('https://dl3.boxcloud.com', { throws: new Error() });
sandbox.stub(api, 'head').rejects(new Error());

return DownloadReachability.setDownloadReachability('https://dl3.boxcloud.com').catch(() => {
expect(setDownloadHostFallbackStub).to.be.called;
Expand Down
21 changes: 10 additions & 11 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable no-unused-expressions */
import fetchMock from 'fetch-mock';
import api from '../api';
import Preview from '../Preview';
import loaders from '../loaders';
import Logger from '../Logger';
Expand Down Expand Up @@ -40,7 +40,6 @@ describe('lib/Preview', () => {
afterEach(() => {
sandbox.verifyAndRestore();
fixture.cleanup();
fetchMock.restore();
preview.destroy();
preview = null;
stubs = null;
Expand Down Expand Up @@ -818,7 +817,7 @@ describe('lib/Preview', () => {

sandbox.stub(file, 'getDownloadURL');
sandbox.stub(preview, 'getRequestHeaders');
sandbox.stub(util, 'get');
sandbox.stub(api, 'get');
});

it('should show error notification and not download file if file cannot be downloaded', () => {
Expand Down Expand Up @@ -873,7 +872,7 @@ describe('lib/Preview', () => {
const promise = Promise.resolve({
download_url: url
});
util.get.returns(promise);
api.get.returns(promise);

preview.download();

Expand All @@ -893,7 +892,7 @@ describe('lib/Preview', () => {
download_url: url
});

util.get.returns(promise);
api.get.returns(promise);
preview.download();
expect(preview.emit).to.be.calledWith('preview_metric');
});
Expand Down Expand Up @@ -1365,7 +1364,7 @@ describe('lib/Preview', () => {
describe('loadFromServer()', () => {
beforeEach(() => {
stubs.promise = Promise.resolve('file');
stubs.get = sandbox.stub(util, 'get').returns(stubs.promise);
stubs.get = sandbox.stub(api, 'get').returns(stubs.promise);
stubs.handleFileInfoResponse = sandbox.stub(preview, 'handleFileInfoResponse');
stubs.handleFetchError = sandbox.stub(preview, 'handleFetchError');
stubs.getURL = sandbox.stub(file, 'getURL').returns('/get_url');
Expand Down Expand Up @@ -1998,14 +1997,14 @@ describe('lib/Preview', () => {
});

it('should get the headers for the post request', () => {
sandbox.stub(util, 'post').returns(stubs.promiseResolve);
sandbox.stub(api, 'post').returns(stubs.promiseResolve);

preview.logPreviewEvent(0, {});
expect(stubs.getHeaders).to.be.called;
});

it('should reset the log retry count on a successful post', () => {
sandbox.stub(util, 'post').returns(stubs.promiseResolve);
sandbox.stub(api, 'post').returns(stubs.promiseResolve);
preview.logRetryCount = 3;

preview.logPreviewEvent(0, {});
Expand All @@ -2016,7 +2015,7 @@ describe('lib/Preview', () => {

it('should reset the log retry count if the post fails and retry limit has been reached', () => {
const promiseReject = Promise.reject({}); // eslint-disable-line prefer-promise-reject-errors
sandbox.stub(util, 'post').returns(promiseReject);
sandbox.stub(api, 'post').returns(promiseReject);
preview.logRetryCount = 3;
preview.logRetryTimeout = true;

Expand All @@ -2030,7 +2029,7 @@ describe('lib/Preview', () => {
it('should set a timeout to try to log the preview event again if post fails and the limit has not been met', () => {
const promiseReject = Promise.reject({}); // eslint-disable-line prefer-promise-reject-errors
sandbox
.stub(util, 'post')
.stub(api, 'post')
.onCall(0)
.returns(promiseReject);
preview.logRetryCount = 3;
Expand Down Expand Up @@ -2452,7 +2451,7 @@ describe('lib/Preview', () => {
id: 0
});

stubs.get = sandbox.stub(util, 'get').returns(stubs.getPromiseResolve);
stubs.get = sandbox.stub(api, 'get').returns(stubs.getPromiseResolve);
stubs.getURL = sandbox.stub(file, 'getURL');
stubs.getRequestHeaders = sandbox.stub(preview, 'getRequestHeaders');
stubs.set = sandbox.stub(preview.cache, 'set');
Expand Down
9 changes: 5 additions & 4 deletions src/lib/__tests__/RepStatus-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable no-unused-expressions */
import RepStatus from '../RepStatus';
import * as util from '../util';
import api from '../api';
import RepStatus from '../RepStatus';
import { LOAD_METRIC } from '../events';
import Timer from '../Timer';
import { STATUS_SUCCESS } from '../constants';
Expand Down Expand Up @@ -106,7 +107,7 @@ describe('lib/RepStatus', () => {

it('should fetch latest status', () => {
sandbox
.mock(util)
.mock(api)
.expects('get')
.returns(
Promise.resolve({
Expand All @@ -124,7 +125,7 @@ describe('lib/RepStatus', () => {

it('should update provided metadata', () => {
sandbox
.mock(util)
.mock(api)
.expects('get')
.returns(
Promise.resolve({
Expand All @@ -146,7 +147,7 @@ describe('lib/RepStatus', () => {

it('should return a resolved promise if there is no info url', () => {
sandbox
.mock(util)
.mock(api)
.expects('get')
.never();
repStatus.infoUrl = '';
Expand Down
Loading

0 comments on commit fb76b13

Please sign in to comment.