Skip to content

Commit

Permalink
fix(mediaviewer): address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Mingze Xiao committed Oct 24, 2019
1 parent 66ee34b commit fa8b298
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 62 deletions.
33 changes: 27 additions & 6 deletions src/lib/Preview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const DEFAULT_DISABLED_VIEWERS = ['Office']; // viewers disabled by default
const PREFETCH_COUNT = 4; // number of files to prefetch
const MOUSEMOVE_THROTTLE_MS = 1500; // for showing or hiding the navigation icons
const RETRY_COUNT = 3; // number of times to retry network request for a file
const RETRY_TOKEN_COUNT = 3; // number of times to retry refreshing access token
const KEYDOWN_EXCEPTIONS = ['INPUT', 'SELECT', 'TEXTAREA']; // Ignore keydown events on these elements
const LOG_RETRY_TIMEOUT_MS = 500; // retry interval for logging preview event
const LOG_RETRY_COUNT = 3; // number of times to retry logging preview event
Expand Down Expand Up @@ -109,9 +110,6 @@ class Preview extends EventEmitter {
/** @property {Object} - Map of disabled viewer names */
disabledViewers = {};

/** @property {string|Function} - Access token */
token = '';

/** @property {Object} - Current viewer instance */
viewer;

Expand All @@ -127,6 +125,9 @@ class Preview extends EventEmitter {
/** @property {number} - Number of times a particular preview has been retried */
retryCount = 0;

/** @property {number} - Number of times refreshing token has been retried */
retryTokenCount = 0;

/** @property {number} - Number of times a particular logging call cas been retried */
logRetryCount = 0;

Expand Down Expand Up @@ -974,9 +975,6 @@ class Preview extends EventEmitter {
// Add the response interceptor to the preview instance
this.options.responseInterceptor = options.responseInterceptor;

// Add the token generator to refresh the token if necessary
this.options.tokenGenerator = options.token;

// Disable or enable viewers based on viewer options
Object.keys(this.options.viewers).forEach(viewerName => {
const isDisabled = this.options.viewers[viewerName].disabled;
Expand Down Expand Up @@ -1005,6 +1003,7 @@ class Preview extends EventEmitter {
location: this.location,
cache: this.cache,
ui: this.ui,
refreshToken: this.refreshToken,
});
}

Expand Down Expand Up @@ -1885,6 +1884,28 @@ class Preview extends EventEmitter {
const fileId = typeof fileIdOrFile === 'string' ? fileIdOrFile : fileIdOrFile.id;
return getProp(this.previewOptions, `fileOptions.${fileId}.${optionName}`);
}

/**
* Refresh the access token
*
* @private
* @return {Promise<string|Error>}
*/
refreshToken = () => {
if (typeof this.previewOptions.token !== 'function') {
return Promise.reject(new Error('Token expired and cannot refresh token.'));
}
if (this.retryTokenCount >= RETRY_TOKEN_COUNT) {
return Promise.reject(new Error('Reach refreshing token limit.'));
}
this.retryTokenCount += 1;
return getTokens(this.file.id, this.previewOptions.token).then(tokenOrTokenMap => {
if (!tokenOrTokenMap || typeof tokenOrTokenMap === 'string') {
return tokenOrTokenMap;
}
return tokenOrTokenMap[this.file.id];
});
};
}

global.Box = global.Box || {};
Expand Down
31 changes: 31 additions & 0 deletions src/lib/__tests__/Preview-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const tokens = require('../tokens');
const PREFETCH_COUNT = 4; // number of files to prefetch
const MOUSEMOVE_THROTTLE = 1500; // for showing or hiding the navigation icons
const KEYDOWN_EXCEPTIONS = ['INPUT', 'SELECT', 'TEXTAREA']; // Ignore keydown events on these elements
const RETRY_TOKEN_COUNT = 3; // number of times to retry refreshing access token

const sandbox = sinon.sandbox.create();

Expand Down Expand Up @@ -2837,5 +2838,35 @@ describe('lib/Preview', () => {
expect(preview.getFileOption('123', 'fileVersionId')).to.equal(undefined);
});
});

describe('refreshToken()', () => {
it('should return a new token if the previewOptions.token is a function', done => {
preview.file = {
id: 'file_123',
};
preview.previewOptions.token = id => Promise.resolve({ [id]: 'new_token' });
preview.refreshToken().then(token => {
expect(token).to.equal('new_token');
done();
});
});

it('should reject if tried to refresh token more than 3 times', done => {
preview.previewOptions.token = id => Promise.resolve({ [id]: 'new_token' });
preview.retryTokenCount = RETRY_TOKEN_COUNT + 1;
preview.refreshToken().catch(error => {
expect(error.message).to.equal('Reach refreshing token limit.');
done();
});
});

it('should reject if previewOptions.token is not a function', done => {
preview.previewOptions.token = 'token';
preview.refreshToken().catch(error => {
expect(error.message).to.equal('Token expired and cannot refresh token.');
done();
});
});
});
});
/* eslint-enable no-unused-expressions */
13 changes: 9 additions & 4 deletions src/lib/viewers/media/DashViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,15 @@ class DashViewer extends VideoBaseViewer {
normalizedShakaError.code === shaka.util.Error.Code.BAD_HTTP_STATUS &&
normalizedShakaError.data[1] === 401 // token expired
) {
this.refreshToken().then(newToken => {
this.options.token = newToken;
this.player.retryStreaming();
});
this.options
.refreshToken()
.then(newToken => {
this.options.token = newToken;
this.player.retryStreaming();
})
.catch(tokenError => {
this.triggerError(tokenError);
});
return;
}

Expand Down
43 changes: 15 additions & 28 deletions src/lib/viewers/media/MediaBaseViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import Browser from '../../Browser';
import MediaControls from './MediaControls';
import PreviewError from '../../PreviewError';
import Timer from '../../Timer';
import getTokens from '../../tokens';
import { CLASS_ELEM_KEYBOARD_FOCUS, CLASS_HIDDEN, CLASS_IS_BUFFERING, CLASS_IS_VISIBLE } from '../../constants';
import { ERROR_CODE, MEDIA_METRIC, MEDIA_METRIC_EVENTS, VIEWER_EVENT } from '../../events';
import { getProp } from '../../util';
Expand Down Expand Up @@ -269,21 +268,6 @@ class MediaBaseViewer extends BaseViewer {
this.wrapperEl.classList.add(CLASS_IS_VISIBLE);
}

/**
* Refresh the access token
*
* @private
* @return {Promise<Object>}
*/
refreshToken = () => {
return getTokens(this.options.file.id, this.options.tokenGenerator).then(tokenOrTokenMap => {
if (!tokenOrTokenMap || typeof tokenOrTokenMap === 'string') {
return tokenOrTokenMap;
}
return tokenOrTokenMap[this.options.file.id];
});
};

/**
* Handles media element loading errors.
*
Expand All @@ -301,18 +285,21 @@ class MediaBaseViewer extends BaseViewer {
const errorDetails = errorCode ? { error_code: errorCode, error_message: errorMessage } : {};

// refresh the token if token expired
if (
errorCode === MediaError.MEDIA_ERR_NETWORK &&
errorMessage.includes(MEDIA_TOKEN_EXPIRE_ERROR) &&
typeof this.options.tokenGenerator === 'function'
) {
this.refreshToken().then(newToken => {
const { currentTime } = this.mediaEl;
this.currentTime = currentTime;
this.options.token = newToken;
this.mediaUrl = this.createContentUrlWithAuthParams(this.options.representation.content.url_template);
this.mediaEl.src = this.mediaUrl;
});
if (errorCode === MediaError.MEDIA_ERR_NETWORK && errorMessage.includes(MEDIA_TOKEN_EXPIRE_ERROR)) {
this.options
.refreshToken()
.then(newToken => {
const { currentTime } = this.mediaEl;
this.currentTime = currentTime;
this.options.token = newToken;
this.mediaUrl = this.createContentUrlWithAuthParams(
this.options.representation.content.url_template,
);
this.mediaEl.src = this.mediaUrl;
})
.catch(error => {
this.triggerError(error);
});
return;
}

Expand Down
24 changes: 0 additions & 24 deletions src/lib/viewers/media/__tests__/MediaBaseViewer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,30 +185,6 @@ describe('lib/viewers/media/MediaBaseViewer', () => {
});
});

describe('refreshToken()', () => {
it('should return the same token if the tokenGenerator is a string', done => {
media.options.file = {
id: 'file_123',
};
media.options.tokenGenerator = 'new_token';
media.refreshToken().then(token => {
expect(token).to.equal('new_token');
done();
});
});

it('should return a new token if the tokenGenerator is a function', done => {
media.options.file = {
id: 'file_123',
};
media.options.tokenGenerator = id => Promise.resolve({ [id]: 'new_token' });
media.refreshToken().then(token => {
expect(token).to.equal('new_token');
done();
});
});
});

describe('errorHandler()', () => {
before(() => {
window.MediaError = {
Expand Down

0 comments on commit fa8b298

Please sign in to comment.