Skip to content

Commit

Permalink
Chore: Adding download reachability checks to Downloads, Document and…
Browse files Browse the repository at this point in the history
… Image Viewers (#669)
  • Loading branch information
Jeremy Press authored Mar 2, 2018
1 parent 3f136a7 commit 6468d63
Show file tree
Hide file tree
Showing 20 changed files with 640 additions and 70 deletions.
2 changes: 2 additions & 0 deletions src/i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ notification_button_default_text=Okay
notification_annotation_point_mode=Click anywhere to add a comment to the document
# Notification message shown when user enters drawing annotation mode
notification_annotation_draw_mode=Press down and drag the pointer to draw on the document
# Notification message shown when the user has a degraded preview experience due to blocked download hosts
notification_degraded_preview=It looks like your connection to {1} is being blocked. We think we can make file previews faster for you. To do that, please ask your network admin to configure firewall settings so that {1} is reachable.

# Link Text
link_contact_us=Contact Us
Expand Down
41 changes: 32 additions & 9 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ import {
replacePlaceholders,
stripAuthFromString
} from './util';
import {
isDownloadHostBlocked,
setDownloadReachability,
isCustomDownloadHost,
replaceDownloadHostWithDefault
} from './downloadReachability';
import {
getURL,
getDownloadURL,
Expand Down Expand Up @@ -163,6 +169,7 @@ class Preview extends EventEmitter {
this.handleFileInfoResponse = this.handleFileInfoResponse.bind(this);
this.handleFetchError = this.handleFetchError.bind(this);
this.handleViewerEvents = this.handleViewerEvents.bind(this);
this.handleViewerMetrics = this.handleViewerMetrics.bind(this);
this.triggerError = this.triggerError.bind(this);
this.throttledMousemoveHandler = this.getGlobalMousemoveHandler().bind(this);
this.navigateLeft = this.navigateLeft.bind(this);
Expand Down Expand Up @@ -478,13 +485,29 @@ class Preview extends EventEmitter {
download() {
const { apiHost, queryParams } = this.options;

if (checkPermission(this.file, PERMISSION_DOWNLOAD)) {
// Append optional query params
const downloadUrl = appendQueryParams(getDownloadURL(this.file.id, apiHost), queryParams);
get(downloadUrl, this.getRequestHeaders()).then((data) => {
openUrlInsideIframe(data.download_url);
});
if (!checkPermission(this.file, PERMISSION_DOWNLOAD)) {
return;
}

// Append optional query params
const downloadUrl = appendQueryParams(getDownloadURL(this.file.id, apiHost), queryParams);
get(downloadUrl, this.getRequestHeaders()).then((data) => {
const defaultDownloadUrl = replaceDownloadHostWithDefault(data.download_url);
if (isDownloadHostBlocked() || !isCustomDownloadHost(data.download_url)) {
// If we know the host is blocked, or we are already using the default,
// use the default.
openUrlInsideIframe(defaultDownloadUrl);
} else {
// Try the custom host, then check reachability
openUrlInsideIframe(data.download_url);
setDownloadReachability(data.download_url).then((isBlocked) => {
if (isBlocked) {
// If download is unreachable, try again with default
openUrlInsideIframe(defaultDownloadUrl);
}
});
}
});
}

/**
Expand Down Expand Up @@ -747,6 +770,9 @@ class Preview extends EventEmitter {
this.throttledMousemoveHandler
);

// Set up the notification
this.ui.setupNotification();

// Update navigation
this.ui.showNavigation(this.file.id, this.collection);

Expand Down Expand Up @@ -1200,9 +1226,6 @@ class Preview extends EventEmitter {
// Hide the loading indicator
this.ui.hideLoadingIndicator();

// Set up the notification
this.ui.setupNotification();

// Prefetch next few files
this.prefetchNextFiles();
}
Expand Down
41 changes: 34 additions & 7 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Browser from '../Browser';
import PreviewError from '../PreviewError';
import * as file from '../file';
import * as util from '../util';
import * as dr from '../downloadReachability';
import { API_HOST, CLASS_NAVIGATION_VISIBILITY } from '../constants';
import { VIEWER_EVENT, ERROR_CODE, LOAD_METRIC, PREVIEW_METRIC } from '../events';
import Timer from '../Timer';
Expand Down Expand Up @@ -741,15 +742,21 @@ describe('lib/Preview', () => {
beforeEach(() => {
stubs.promise = Promise.resolve({
data: {
download_url: 'dl.box'
download_url: 'dl.boxcloud.com'
}
});

stubs.reachabilityPromise = Promise.resolve(true);

stubs.checkPermission = sandbox.stub(file, 'checkPermission');
stubs.get = sandbox.stub(util, 'get').returns(stubs.promise);
stubs.get = sandbox.stub(dr, 'setDownloadReachability').returns(stubs.reachabilityPromise);
stubs.openUrlInsideIframe = sandbox.stub(util, 'openUrlInsideIframe');
stubs.getRequestHeaders = sandbox.stub(preview, 'getRequestHeaders');
stubs.getDownloadURL = sandbox.stub(file, 'getDownloadURL');
stubs.isDownloadHostBlocked = sandbox.stub(dr, 'isDownloadHostBlocked');
stubs.isCustomDownloadHost = sandbox.stub(dr, 'isCustomDownloadHost');
stubs.replaceDownloadHostWithDefault = sandbox.stub(dr, 'replaceDownloadHostWithDefault').returns('default');
});

it('should not do anything if there is no download permission', () => {
Expand All @@ -759,12 +766,36 @@ describe('lib/Preview', () => {
expect(stubs.openUrlInsideIframe).to.not.be.called;
});

it('get the file and then open in an iframe', () => {
it('open the default download URL in an iframe if the custom host is blocked or if we were given the default', () => {
stubs.checkPermission.returns(true);
stubs.isDownloadHostBlocked.returns(true);
stubs.isCustomDownloadHost.returns(true);

preview.download();
return stubs.promise.then((data) => {
expect(stubs.openUrlInsideIframe).to.be.calledWith('default');
});

stubs.isDownloadHostBlocked.returns(false);
stubs.isCustomDownloadHost.returns(false);

preview.download();
return stubs.promise.then((data) => {
expect(stubs.openUrlInsideIframe).to.be.calledWith('default');
});
});


it('should check download reachability and fallback if we do not know the status of our custom host', () => {
stubs.checkPermission.returns(true);
stubs.isCustomDownloadHost.returns(true);

preview.download();
return stubs.promise.then((data) => {
expect(stubs.openUrlInsideIframe).to.be.calledWith(data.download_url);
return stubs.reachabilityPromise.then(() => {
expect(stubs.openUrlInsideIframe).to.be.calledWith('default');
});
});
});
});
Expand Down Expand Up @@ -994,6 +1025,7 @@ describe('lib/Preview', () => {
previewUIMock.expects('showLoadingIndicator');
previewUIMock.expects('startProgressBar');
previewUIMock.expects('showNavigation');
previewUIMock.expects('setupNotification');

preview.setupUI();
});
Expand Down Expand Up @@ -1810,11 +1842,6 @@ describe('lib/Preview', () => {
expect(stubs.hideLoadingIndicator).to.be.called;
});

it('should set up the notification', () => {
preview.finishLoading();
expect(stubs.setupNotification).to.be.called;
});

it('should prefetch next files', () => {
preview.finishLoading();
expect(stubs.prefetchNextFiles).to.be.called;
Expand Down
130 changes: 130 additions & 0 deletions src/lib/__tests__/downloadReachability-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/* eslint-disable no-unused-expressions */
import 'whatwg-fetch';
import fetchMock from 'fetch-mock';
import * as dr from '../downloadReachability';

const sandbox = sinon.sandbox.create();

const DEFAULT_DOWNLOAD_HOST_PREFIX = 'https://dl.';
const DOWNLOAD_NOTIFICATION_SHOWN_KEY = 'download_host_notification_shown';
const DOWNLOAD_HOST_FALLBACK_KEY = 'download_host_fallback';

describe('lib/downloadReachability', () => {
beforeEach(() => {
sessionStorage.clear();
localStorage.clear();

});

afterEach(() => {
sessionStorage.clear();
localStorage.clear();
sandbox.verifyAndRestore();

});

describe('isCustomDownloadHost()', () => {
it('should be true if the url does not start with the default host prefix but is a dl host', () => {
let url = 'https://dl3.boxcloud.com/foo';
let result = dr.isCustomDownloadHost(url)
expect(result).to.be.true;

url = 'https://dl.boxcloud.com/foo';
expect(dr.isCustomDownloadHost(url)).to.be.false;

url = 'https://www.google.com';
expect(dr.isCustomDownloadHost(url)).to.be.false;


url = 'https://kld3lk.boxcloud.com';
expect(dr.isCustomDownloadHost(url)).to.be.true;

url = 'https://dl3.user.inside-box.net';
expect(dr.isCustomDownloadHost(url)).to.be.true;


url = 'https://dl.user.inside-box.net';
expect(dr.isCustomDownloadHost(url)).to.be.false;
});
});

describe('replaceDownloadHostWithDefault()', () => {
it('should add the given host to the array of shown hosts', () => {
const blockedHost = 'https://dl3.boxcloud.com';

const result = dr.setDownloadHostNotificationShown(blockedHost);

const shownHostsArr = JSON.parse(localStorage.getItem(DOWNLOAD_NOTIFICATION_SHOWN_KEY)) || [];
expect(shownHostsArr).to.contain('https://dl3.boxcloud.com');
});
});

describe('setDownloadHostFallback()', () => {
it('should set the download host fallback key to be true', () => {
expect(sessionStorage.getItem(DOWNLOAD_HOST_FALLBACK_KEY)).to.not.equal('true')

dr.setDownloadHostFallback();

expect(sessionStorage.getItem(DOWNLOAD_HOST_FALLBACK_KEY)).to.equal('true')

});
});

describe('isDownloadHostBlocked()', () => {
it('should set the download host fallback key to be true', () => {
expect(dr.isDownloadHostBlocked()).to.be.false;

dr.setDownloadHostFallback();

expect(dr.isDownloadHostBlocked()).to.be.true;
});
});

describe('setDownloadHostNotificationShown()', () => {
it('should set the download host fallback key to be true', () => {
expect(dr.isDownloadHostBlocked()).to.be.false;

dr.setDownloadHostFallback();

expect(dr.isDownloadHostBlocked()).to.be.true;
});
});

describe('downloadNotificationToShow()', () => {
beforeEach(() => {
sessionStorage.setItem('download_host_fallback', 'false');
});

it('should return true if we do not have an entry for the given host and our session indicates we are falling back to the default host', () => {
let result = dr.downloadNotificationToShow('https://foo.com');
expect(result).to.be.undefined;;

sessionStorage.setItem('download_host_fallback', 'true');
result = dr.downloadNotificationToShow('https://dl5.boxcloud.com');
expect(result).to.equal('dl5.boxcloud.com');

const shownHostsArr = ['dl5.boxcloud.com'];
localStorage.setItem('download_host_notification_shown', JSON.stringify(shownHostsArr));
result = dr.downloadNotificationToShow('https://dl5.boxcloud.com');
expect(result).to.be.undefined;

});
});

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

return dr.setDownloadReachability('https://dl3.boxcloud.com').catch(() => {
expect(setDownloadHostFallbackStub).to.be.called;
})



});
});
});
27 changes: 26 additions & 1 deletion src/lib/__tests__/util-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ describe('lib/util', () => {
});

describe('get()', () => {
const url = 'foo?bar=bum';
let url;
beforeEach(() => {
url = 'foo?bar=bum';
});

afterEach(() => {
fetchMock.restore();
Expand Down Expand Up @@ -96,6 +99,23 @@ describe('lib/util', () => {
expect(typeof response === 'object').to.be.true;
});
});

it('set the download host fallback and try again if we\'re fetching from a non default host', () => {
url = 'dl7.boxcloud.com'
fetchMock.get(url, {
status: 500
});

return util.get(url, 'any')
.then(() => {
expect(response.status).to.equal(200);
})
.catch(() => {
fetchMock.get(url, {
status: 200
});
})
});
});

describe('post()', () => {
Expand Down Expand Up @@ -327,6 +347,11 @@ describe('lib/util', () => {
it('should return correct content url when no asset_path', () => {
expect(util.createContentUrl('foo', 'bar')).to.equal('foo');
});

it('should replace the download host with the default if we are falling back', () => {
sessionStorage.setItem('download_host_fallback', 'true');
expect(util.createContentUrl('https://dl6.boxcloud.com', 'bar')).to.equal('https://dl.boxcloud.com');
});
});

describe('createAssetUrlCreator()', () => {
Expand Down
Loading

0 comments on commit 6468d63

Please sign in to comment.