From 3cc1a43b8d5a929e9d0c8f8642cd2c46ef4e4e6e Mon Sep 17 00:00:00 2001 From: Jacob Trimble Date: Tue, 19 Jan 2016 15:05:32 -0800 Subject: [PATCH] Added some networking utility functions. This adds some utility functions to NetworkingEngine for common actions. This also cleans up the related unit tests. Change-Id: I1105b77b6dac3637d566c1a4e2f77004ad705e8b --- lib/dash/dash_parser.js | 20 ++---- lib/media/streaming_engine.js | 16 ++--- lib/net/networking_engine.js | 40 ++++++++++++ lib/player.js | 19 ++---- test/data_uri_plugin_unit.js | 6 +- test/http_plugin_unit.js | 17 +++--- test/networking_engine_unit.js | 107 +++++++++++++-------------------- test/streaming_engine_unit.js | 26 +++----- 8 files changed, 116 insertions(+), 135 deletions(-) diff --git a/lib/dash/dash_parser.js b/lib/dash/dash_parser.js index 849a81b03..bbf8c0c1b 100644 --- a/lib/dash/dash_parser.js +++ b/lib/dash/dash_parser.js @@ -304,14 +304,8 @@ shaka.dash.DashParser.prototype.stop = function() { */ shaka.dash.DashParser.prototype.requestManifest_ = function() { var requestType = shaka.net.NetworkingEngine.RequestType.MANIFEST; - var request = { - uris: [this.manifestUri_], - method: 'GET', - body: null, - headers: {}, - allowCrossSiteCredentials: false, - retryParameters: this.retryParameters_ - }; + var request = shaka.net.NetworkingEngine.makeRequest( + [this.manifestUri_], this.retryParameters_); return this.networkingEngine_.request(requestType, request) .then(function(response) { // This may throw; but it will result in a failed promise. @@ -889,14 +883,8 @@ shaka.dash.DashParser.RequestInitSegmentCallback; shaka.dash.DashParser.prototype.requestInitSegment_ = function( uris, startByte, endByte) { var requestType = shaka.net.NetworkingEngine.RequestType.SEGMENT; - var request = { - uris: uris, - method: 'GET', - body: null, - headers: {}, - allowCrossSiteCredentials: false, - retryParameters: this.retryParameters_ - }; + var request = + shaka.net.NetworkingEngine.makeRequest(uris, this.retryParameters_); if (startByte != null) { var end = (endByte != null ? endByte : ''); request.headers['Range'] = 'bytes=' + startByte + '-' + end; diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js index 581a9d03a..1b04b620d 100644 --- a/lib/media/streaming_engine.js +++ b/lib/media/streaming_engine.js @@ -895,23 +895,15 @@ shaka.media.StreamingEngine.prototype.findPeriodContainingStream_ = function( */ shaka.media.StreamingEngine.prototype.fetch_ = function(reference) { var requestType = shaka.net.NetworkingEngine.RequestType.SEGMENT; + var request = shaka.net.NetworkingEngine.makeRequest( + reference.uris, this.config_.retryParameters); // Set Range header if needed. - var headers = {}; if ((reference.startByte != 0) || (reference.endByte != null)) { - headers['Range'] = 'bytes=' + reference.startByte + '-' + - (reference.endByte || ''); + request.headers['Range'] = + 'bytes=' + reference.startByte + '-' + (reference.endByte || ''); } - // TODO: Refactor this to use forthcoming NetworkingEngine factory function. - var request = { - uris: reference.uris, - method: 'GET', - body: null, - headers: headers, - allowCrossSiteCredentials: false, - retryParameters: this.config_.retryParameters - }; shaka.log.v1('Fetching:', reference); var p = this.netEngine_.request(requestType, request); diff --git a/lib/net/networking_engine.js b/lib/net/networking_engine.js index 8f6c9749e..cabbc8ff5 100644 --- a/lib/net/networking_engine.js +++ b/lib/net/networking_engine.js @@ -180,6 +180,46 @@ shaka.net.NetworkingEngine.prototype.unregisterResponseFilter = }; +/** + * Gets a copy of the default retry parameters. + * + * @return {shakaExtern.RetryParameters} + */ +shaka.net.NetworkingEngine.defaultRetryParameters = function() { + // Use a function rather than a constant member so the calling code can + // modify the values without affecting other call results. + return { + maxAttempts: 1, + baseDelay: 1000, + backoffFactor: 2, + fuzzFactor: 0.5, + timeout: 0 + }; +}; + + +/** + * Makes a simple network request for the given URIs. + * + * @param {!Array.} uris + * @param {shakaExtern.RetryParameters=} opt_retryParams + * @param {string=} opt_method + * @return {shakaExtern.Request} + */ +shaka.net.NetworkingEngine.makeRequest = function( + uris, opt_retryParams, opt_method) { + return { + uris: uris, + method: opt_method || 'GET', + body: null, + headers: {}, + allowCrossSiteCredentials: false, + retryParameters: opt_retryParams || + shaka.net.NetworkingEngine.defaultRetryParameters() + }; +}; + + /** @override */ shaka.net.NetworkingEngine.prototype.destroy = function() { this.destroyed_ = true; diff --git a/lib/player.js b/lib/player.js index 0a85aae00..fe13b0fef 100644 --- a/lib/player.js +++ b/lib/player.js @@ -46,13 +46,8 @@ shaka.Player = function() { this.manifest_ = null; /** @private {shakaExtern.RetryParameters} */ - this.defaultRetryParameters_ = { - maxAttempts: 1, - baseDelay: 1000, - backoffFactor: 2, - fuzzFactor: 0.5, - timeout: 0 - }; + this.defaultRetryParameters_ = + shaka.net.NetworkingEngine.defaultRetryParameters(); }; @@ -146,14 +141,8 @@ shaka.Player.prototype.load = function(manifestUri, opt_startTime, if (!factory) { // Try to choose a manifest parser by MIME type. - var headRequest = { - uris: [manifestUri], - method: 'HEAD', - body: null, - headers: {}, - allowCrossSiteCredentials: false, - retryParameters: this.defaultRetryParameters_ - }; + var headRequest = shaka.net.NetworkingEngine.makeRequest( + [manifestUri], this.defaultRetryParameters_, 'HEAD'); var type = shaka.net.NetworkingEngine.RequestType.MANIFEST; p = this.networkingEngine_.request(type, headRequest).then( diff --git a/test/data_uri_plugin_unit.js b/test/data_uri_plugin_unit.js index 5fe2df33a..d4db1ead9 100644 --- a/test/data_uri_plugin_unit.js +++ b/test/data_uri_plugin_unit.js @@ -61,7 +61,8 @@ describe('DataUriPlugin', function() { }); function testSucceeds(uri, text, done) { - shaka.net.DataUriPlugin(uri, {}) + var request = shaka.net.NetworkingEngine.makeRequest([uri]); + shaka.net.DataUriPlugin(uri, request) .then(function(response) { expect(response).toBeTruthy(); expect(response.uri).toBe(uri); @@ -75,8 +76,9 @@ describe('DataUriPlugin', function() { } function testFails(uri, done) { + var request = shaka.net.NetworkingEngine.makeRequest([uri]); shaka.log.setLevel(shaka.log.Level.NONE); - shaka.net.DataUriPlugin(uri, {}) + shaka.net.DataUriPlugin(uri, request) .then(fail) .catch(function() { expect(true).toBe(true); }) .then(function() { diff --git a/test/http_plugin_unit.js b/test/http_plugin_unit.js index 23719f7a7..cfd8d3f73 100644 --- a/test/http_plugin_unit.js +++ b/test/http_plugin_unit.js @@ -50,12 +50,11 @@ describe('HttpPlugin', function() { }); it('sets the correct fields', function(done) { - var request = { - uris: ['https://foo.bar/'], - allowCrossSiteCredentials: true, - method: 'POST', - headers: {'FOO': 'BAR'} - }; + var request = shaka.net.NetworkingEngine.makeRequest(['https://foo.bar/']); + request.allowCrossSiteCredentials = true; + request.method = 'POST'; + request.headers['BAZ'] = '123'; + shaka.net.HttpPlugin(request.uris[0], request) .then(function() { var actual = jasmine.Ajax.requests.mostRecent(); @@ -63,7 +62,7 @@ describe('HttpPlugin', function() { expect(actual.url).toBe(request.uris[0]); expect(actual.method).toBe(request.method); expect(actual.withCredentials).toBe(true); - expect(actual.requestHeaders['FOO']).toBe('BAR'); + expect(actual.requestHeaders['BAZ']).toBe('123'); }) .catch(fail) .then(done); @@ -94,7 +93,7 @@ describe('HttpPlugin', function() { }); function testSucceeds(uri, done, opt_overrideUri) { - var request = {uris: [uri]}; + var request = shaka.net.NetworkingEngine.makeRequest([uri]); shaka.net.HttpPlugin(uri, request) .catch(fail) .then(function(response) { @@ -111,7 +110,7 @@ describe('HttpPlugin', function() { } function testFails(uri, done) { - var request = {uris: [uri]}; + var request = shaka.net.NetworkingEngine.makeRequest([uri]); shaka.net.HttpPlugin(uri, request) .then(fail) .catch(function() { diff --git a/test/networking_engine_unit.js b/test/networking_engine_unit.js index 1eb74e56d..e036b2e58 100644 --- a/test/networking_engine_unit.js +++ b/test/networking_engine_unit.js @@ -48,10 +48,8 @@ describe('NetworkingEngine', function() { describe('retry', function() { it('will retry', function(done) { - var request = { - uris: ['reject://foo'], - retryParameters: {maxAttempts: 2, baseDelay: 0} - }; + var request = + createRequest('reject://foo', {maxAttempts: 2, baseDelay: 0}); rejectScheme.and.callFake(function() { if (rejectScheme.calls.count() == 1) return Promise.reject(); @@ -69,10 +67,8 @@ describe('NetworkingEngine', function() { }); it('will retry twice', function(done) { - var request = { - uris: ['reject://foo'], - retryParameters: {maxAttempts: 3, baseDelay: 0} - }; + var request = + createRequest('reject://foo', {maxAttempts: 3, baseDelay: 0}); rejectScheme.and.callFake(function() { if (rejectScheme.calls.count() < 3) return Promise.reject(); @@ -90,10 +86,8 @@ describe('NetworkingEngine', function() { }); it('will fail overall', function(done) { - var request = { - uris: ['reject://foo'], - retryParameters: {maxAttempts: 3, baseDelay: 0} - }; + var request = + createRequest('reject://foo', {maxAttempts: 3, baseDelay: 0}); networkingEngine.request(requestType, request) .then(fail) .catch(function() { expect(rejectScheme.calls.count()).toBe(3); }) @@ -123,15 +117,12 @@ describe('NetworkingEngine', function() { }); it('uses baseDelay', function(done) { - var request = { - uris: ['reject://foo'], - retryParameters: { - maxAttempts: 2, - baseDelay: baseDelay, - fuzzFactor: 0, - backoffFactor: 2 - } - }; + var request = createRequest('reject://foo', { + maxAttempts: 2, + baseDelay: baseDelay, + fuzzFactor: 0, + backoffFactor: 2 + }); networkingEngine.request(requestType, request) .then(fail) .catch(function() { @@ -143,15 +134,12 @@ describe('NetworkingEngine', function() { }); it('uses backoffFactor', function(done) { - var request = { - uris: ['reject://foo'], - retryParameters: { - maxAttempts: 3, - baseDelay: baseDelay, - fuzzFactor: 0, - backoffFactor: 2 - } - }; + var request = createRequest('reject://foo', { + maxAttempts: 3, + baseDelay: baseDelay, + fuzzFactor: 0, + backoffFactor: 2 + }); networkingEngine.request(requestType, request) .then(fail) .catch(function() { @@ -165,15 +153,12 @@ describe('NetworkingEngine', function() { }); it('uses fuzzFactor', function(done) { - var request = { - uris: ['reject://foo'], - retryParameters: { - maxAttempts: 2, - baseDelay: baseDelay, - fuzzFactor: 1, - backoffFactor: 1 - } - }; + var request = createRequest('reject://foo', { + maxAttempts: 2, + baseDelay: baseDelay, + fuzzFactor: 1, + backoffFactor: 1 + }); networkingEngine.request(requestType, request) .then(fail) .catch(function() { @@ -189,10 +174,8 @@ describe('NetworkingEngine', function() { }); it('uses multiple URIs', function(done) { - var request = { - uris: ['reject://foo', 'resolve://foo'], - retryParameters: {maxAttempts: 3} - }; + var request = createRequest('', {maxAttempts: 3}); + request.uris = ['reject://foo', 'resolve://foo']; networkingEngine.request(requestType, request) .catch(fail) .then(function() { @@ -241,10 +224,12 @@ describe('NetworkingEngine', function() { }); it('passes correct arguments to plugin', function(done) { - var request = {uris: ['resolve://foo'], method: 'POST'}; + var request = createRequest('resolve://foo'); + request.method = 'POST'; + resolveScheme.and.callFake(function(uri, request) { expect(uri).toBe(request.uris[0]); - expect(request).toBe(request); + expect(request).toEqual(request); return Promise.resolve(); }); networkingEngine.request(requestType, request).catch(fail).then(done); @@ -280,7 +265,7 @@ describe('NetworkingEngine', function() { }); it('is given correct arguments', function(done) { - var request = {uris: ['resolve://foo']}; + var request = createRequest('resolve://foo'); networkingEngine.request(requestType, request) .catch(fail) .then(function() { @@ -319,10 +304,8 @@ describe('NetworkingEngine', function() { }); it('if throws will stop requests', function(done) { - var request = { - uris: ['resolve://foo'], - retryParameters: {maxAttempts: 3, baseRetryDelay: 0} - }; + var request = + createRequest('resolve://foo', {maxAttempts: 3, baseRetryDelay: 0}); filter.and.throwError(new Error()); networkingEngine.request(requestType, request) .then(fail) @@ -369,7 +352,7 @@ describe('NetworkingEngine', function() { }); it('is given correct arguments', function(done) { - var request = {uris: ['resolve://foo']}; + var request = createRequest('resolve://foo'); networkingEngine.request(requestType, request) .catch(fail) .then(function() { @@ -419,10 +402,8 @@ describe('NetworkingEngine', function() { }); it('if throws will retry', function(done) { - var request = { - uris: ['resolve://foo'], - retryParameters: {maxAttempts: 2, baseRetryDelay: 0} - }; + var request = + createRequest('resolve://foo', {maxAttempts: 2, baseRetryDelay: 0}); filter.and.callFake(function() { if (filter.calls.count() == 1) throw new Error(); }); @@ -438,7 +419,7 @@ describe('NetworkingEngine', function() { describe('destroy', function() { it('waits for all operations to complete', function(done) { - var request = {uris: ['resolve://foo']}; + var request = createRequest('resolve://foo'); var p = new shaka.util.PublicPromise(); resolveScheme.and.returnValue(p); @@ -467,7 +448,7 @@ describe('NetworkingEngine', function() { }); it('resolves even when a request fails', function(done) { - var request = {uris: ['reject://foo']}; + var request = createRequest('reject://foo'); var p = new shaka.util.PublicPromise(); rejectScheme.and.returnValue(p); @@ -496,7 +477,7 @@ describe('NetworkingEngine', function() { }); it('prevents new requests', function(done) { - var request = {uris: ['resolve://foo']}; + var request = createRequest('resolve://foo'); var p = new shaka.util.PublicPromise(); resolveScheme.and.returnValue(p); @@ -532,10 +513,8 @@ describe('NetworkingEngine', function() { }); it('does not allow further retries', function(done) { - var request = { - uris: ['reject://foo'], - retryParameters: {maxAttempts: 3, baseDelay: 0} - }; + var request = + createRequest('reject://foo', {maxAttempts: 3, baseDelay: 0}); var p1 = new shaka.util.PublicPromise(); var p2 = new shaka.util.PublicPromise(); @@ -572,7 +551,7 @@ describe('NetworkingEngine', function() { }); }); - function createRequest(uri) { - return { uris: [uri] }; + function createRequest(uri, opt_retryParameters) { + return shaka.net.NetworkingEngine.makeRequest([uri], opt_retryParameters); } }); diff --git a/test/streaming_engine_unit.js b/test/streaming_engine_unit.js index 05e6af124..17e6d76b9 100644 --- a/test/streaming_engine_unit.js +++ b/test/streaming_engine_unit.js @@ -276,7 +276,7 @@ describe('StreamingEngine', function() { var config = { rebufferingGoal: 2, bufferingGoal: 5, - retryParameters: {} + retryParameters: shaka.net.NetworkingEngine.defaultRetryParameters() }; streamingEngine = new shaka.media.StreamingEngine( config, playhead, mediaSourceEngine, netEngine, manifest, @@ -794,8 +794,7 @@ describe('StreamingEngine', function() { jasmine.objectContaining({ uris: [period + '_audio_init'], method: 'GET', - headers: {}, - retryParameters: {} + headers: {} })); expect(netEngine.request).toHaveBeenCalledWith( @@ -803,8 +802,7 @@ describe('StreamingEngine', function() { jasmine.objectContaining({ uris: [period + '_video_init'], method: 'GET', - headers: {}, - retryParameters: {} + headers: {} })); expect(netEngine.request).toHaveBeenCalledWith( @@ -812,8 +810,7 @@ describe('StreamingEngine', function() { jasmine.objectContaining({ uris: [period + '_audio_1'], method: 'GET', - headers: {'Range': 'bytes=0-' + segmentSizes.audio}, - retryParameters: {} + headers: {'Range': 'bytes=0-' + segmentSizes.audio} })); expect(netEngine.request).toHaveBeenCalledWith( @@ -821,8 +818,7 @@ describe('StreamingEngine', function() { jasmine.objectContaining({ uris: [period + '_video_1'], method: 'GET', - headers: {'Range': 'bytes=0-' + segmentSizes.video}, - retryParameters: {} + headers: {'Range': 'bytes=0-' + segmentSizes.video} })); expect(netEngine.request).toHaveBeenCalledWith( @@ -830,8 +826,7 @@ describe('StreamingEngine', function() { jasmine.objectContaining({ uris: [period + '_text_1'], method: 'GET', - headers: {'Range': 'bytes=0-' + segmentSizes.text}, - retryParameters: {} + headers: {'Range': 'bytes=0-' + segmentSizes.text} })); expect(netEngine.request).toHaveBeenCalledWith( @@ -839,8 +834,7 @@ describe('StreamingEngine', function() { jasmine.objectContaining({ uris: [period + '_audio_2'], method: 'GET', - headers: {'Range': 'bytes=0-' + segmentSizes.audio}, - retryParameters: {} + headers: {'Range': 'bytes=0-' + segmentSizes.audio} })); expect(netEngine.request).toHaveBeenCalledWith( @@ -848,8 +842,7 @@ describe('StreamingEngine', function() { jasmine.objectContaining({ uris: [period + '_video_2'], method: 'GET', - headers: {'Range': 'bytes=0-' + segmentSizes.video}, - retryParameters: {} + headers: {'Range': 'bytes=0-' + segmentSizes.video} })); expect(netEngine.request).toHaveBeenCalledWith( @@ -857,8 +850,7 @@ describe('StreamingEngine', function() { jasmine.objectContaining({ uris: [period + '_text_2'], method: 'GET', - headers: {'Range': 'bytes=0-' + segmentSizes.text}, - retryParameters: {} + headers: {'Range': 'bytes=0-' + segmentSizes.text} })); netEngine.request.calls.reset();