Bartosz Dziewoński has uploaded a new change for review. https://gerrit.wikimedia.org/r/280339
Change subject: uw.controller.Step: Refactor simultaneous transitions ...................................................................... uw.controller.Step: Refactor simultaneous transitions Problems: * We assumed that Upload and Details steps are sufficiently alike to use the same code for transitioning from them, but they aren't. * The transitionAll() function, while otherwise adequate for Details, would blow up if uploads were removed while the queue was running. * The transitionAll() function doesn't allow adding uploads while the queue is already running, which we tried to do in the Upload step. This resulted in the queue starting for every upload, effectively ignoring the maximum simultaneous upload limit. Solutions: * Introduce uw.ConcurrentQueue, which takes care of executing async functions, waiting for them to finish, then executing further ones, while also allowing actions to be added to or removed from the queue at any time and emitting events when things happen. * uw.controller.Upload: Don't use transitionAll(), implement separate logic using uw.ConcurrentQueue. Update mw.UploadWizard to match. * uw.controller.Details: Rewrite transitionAll() using uw.ConcurrentQueue. * Remove the broken common implementation in uw.controller.Step. Bug: T92809 Change-Id: I32fbd941f5a5dca50c030c3a93889a18537555ef (cherry picked from commit a8a5966bf6aa76cb137b4b7cda4f3aa2635be991) --- M UploadWizardHooks.php M resources/controller/uw.controller.Details.js M resources/controller/uw.controller.Step.js M resources/controller/uw.controller.Upload.js M resources/mw.UploadWizard.js A resources/uw.ConcurrentQueue.js M tests/qunit/controller/uw.controller.Details.test.js M tests/qunit/controller/uw.controller.Step.test.js M tests/qunit/controller/uw.controller.Upload.test.js A tests/qunit/uw.ConcurrentQueue.test.js 10 files changed, 755 insertions(+), 132 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard refs/changes/39/280339/1 diff --git a/UploadWizardHooks.php b/UploadWizardHooks.php index 35f7863..bae1a85 100644 --- a/UploadWizardHooks.php +++ b/UploadWizardHooks.php @@ -500,6 +500,7 @@ 'uw.controller.Step' => array( 'scripts' => array( + 'resources/uw.ConcurrentQueue.js', 'resources/controller/uw.controller.Step.js', ), @@ -911,6 +912,7 @@ 'tests/qunit/transports/mw.FormDataTransport.test.js', 'tests/qunit/transports/mw.IframeTransport.test.js', 'tests/qunit/uw.EventFlowLogger.test.js', + 'tests/qunit/uw.ConcurrentQueue.test.js', 'tests/qunit/mw.UploadWizard.test.js', 'tests/qunit/mw.UploadWizardUpload.test.js', 'tests/qunit/mw.UploadWizardLicenseInput.test.js', diff --git a/resources/controller/uw.controller.Details.js b/resources/controller/uw.controller.Details.js index 3a9aed6..2d174fd 100644 --- a/resources/controller/uw.controller.Details.js +++ b/resources/controller/uw.controller.Details.js @@ -36,6 +36,11 @@ this.stepName = 'details'; this.finishState = 'complete'; + + this.queue = new uw.ConcurrentQueue( { + count: this.config.maxSimultaneousConnections, + action: this.transitionOne.bind( this ) + } ); }; OO.inheritClass( uw.controller.Details, uw.controller.Step ); @@ -192,11 +197,41 @@ ); }; + /** + * Perform this step's changes on one upload. + * + * @return {jQuery.Promise} + */ uw.controller.Details.prototype.transitionOne = function ( upload ) { return upload.details.submit(); }; /** + * Perform this step's changes on all uploads. + * + * @return {jQuery.Promise} + */ + uw.controller.Details.prototype.transitionAll = function () { + var + deferred = $.Deferred(), + details = this; + + $.each( this.uploads, function ( i, upload ) { + if ( upload === undefined ) { + return; + } + if ( details.canTransition( upload ) ) { + details.queue.addItem( upload ); + } + } ); + + this.queue.on( 'complete', deferred.resolve ); + this.queue.startExecuting(); + + return deferred.promise(); + }; + + /** * Submit details to the API. * * @return {jQuery.Promise} diff --git a/resources/controller/uw.controller.Step.js b/resources/controller/uw.controller.Step.js index 0b64e04..66e7ae4 100644 --- a/resources/controller/uw.controller.Step.js +++ b/resources/controller/uw.controller.Step.js @@ -36,11 +36,6 @@ */ this.config = config; - /** - * @property {number} uploadsTransitioning The number of uploads currently in this step and in transition. - */ - this.uploadsTransitioning = 0; - this.ui = ui; this.ui.on( 'next-step', function () { @@ -160,65 +155,12 @@ }; /** - * Perform this step's changes on all uploads. Replaces makeTransitioner - * in the UploadWizard class. - * - * @return {jQuery.Promise} - */ - uw.controller.Step.prototype.transitionAll = function () { - var i, - step = this, - transpromises = [], - uploadsQueued = []; - - function startNextUpload() { - var ix, upload; - - // Run through uploads looking for one we can transition. In most - // cases this will be the next upload. - while ( uploadsQueued.length > 0 ) { - ix = uploadsQueued.shift(); - upload = step.uploads[ ix ]; - - if ( step.canTransition( upload ) ) { - return step.transitionOne( upload ).then( startNextUpload ); - } - } - - return $.Deferred().resolve(); - } - - $.each( this.uploads, function ( i, upload ) { - if ( upload === undefined ) { - return; - } - - uploadsQueued.push( i ); - } ); - - for ( i = 0; i < this.config.maxSimultaneousConnections; i++ ) { - transpromises.push( startNextUpload() ); - } - - return $.when.apply( $, transpromises ); - }; - - /** * Check if upload is able to be put through this step's changes. * * @return {boolean} */ uw.controller.Step.prototype.canTransition = function () { - return this.uploadsTransitioning < this.config.maxSimultaneousConnections; - }; - - /** - * Perform this step's changes on one upload. - * - * @return {jQuery.Promise} - */ - uw.controller.Step.prototype.transitionOne = function () { - return $.Deferred().reject( 'Using default transitioner is not supported' ); + return true; }; /** diff --git a/resources/controller/uw.controller.Upload.js b/resources/controller/uw.controller.Upload.js index 366761b..6a3fd30 100644 --- a/resources/controller/uw.controller.Upload.js +++ b/resources/controller/uw.controller.Upload.js @@ -38,6 +38,12 @@ this.stepName = 'file'; this.finishState = 'stashed'; + + this.queue = new uw.ConcurrentQueue( { + count: this.config.maxSimultaneousConnections, + action: this.transitionOne.bind( this ) + } ); + this.queue.on( 'complete', this.showNext.bind( this ) ); }; OO.inheritClass( uw.controller.Upload, uw.controller.Step ); @@ -128,6 +134,11 @@ ); }; + /** + * Perform this step's changes on one upload. + * + * @return {jQuery.Promise} + */ uw.controller.Upload.prototype.transitionOne = function ( upload ) { var promise = upload.start(); this.maybeStartProgressBar(); @@ -135,46 +146,43 @@ }; /** - * Kick off the upload processes. - * Does some precalculations, changes the interface to be less mutable, moves the uploads to a queue, - * and kicks off a thread which will take from the queue. + * Queue an upload object to be uploaded. * - * @return {jQuery.Promise} + * @param {mw.UploadWizardUpload} upload */ - uw.controller.Upload.prototype.startUploads = function () { - var step = this; + uw.controller.Upload.prototype.queueUpload = function ( upload ) { + if ( this.canTransition( upload ) ) { + this.queue.addItem( upload ); + } + }; - this.ui.hideEndButtons(); - - // remove ability to change files - // ideally also hide the "button"... but then we require styleable file input CSS trickery - // although, we COULD do this just for files already in progress... - - // it might be interesting to just make this creational -- attach it to the dom element representing - // the progress bar and elapsed time - - return this.transitionAll().done( function () { - step.showNext(); - } ); + /** + * Kick off the upload processes. + */ + uw.controller.Upload.prototype.startQueuedUploads = function () { + this.queue.startExecuting(); }; uw.controller.Upload.prototype.retry = function () { + var controller = this; uw.eventFlowLogger.logEvent( 'retry-uploads-button-clicked' ); - // reset any uploads in error state back to be shiny & new $.each( this.uploads, function ( i, upload ) { if ( upload === undefined ) { return; } if ( upload.state === 'error' ) { + // reset any uploads in error state back to be shiny & new upload.state = 'new'; upload.ui.clearIndicator(); upload.ui.clearStatus(); + // and queue them + controller.queueUpload( upload ); } } ); - this.startUploads(); + this.startQueuedUploads(); }; }( mediaWiki, mediaWiki.uploadWizard, jQuery, OO ) ); diff --git a/resources/mw.UploadWizard.js b/resources/mw.UploadWizard.js index 01016d7..246a9fd 100644 --- a/resources/mw.UploadWizard.js +++ b/resources/mw.UploadWizard.js @@ -391,7 +391,8 @@ this.uploads.push( upload ); this.steps.file.updateFileCounts( this.uploads ); // Start uploads now, no reason to wait--leave the remove button alone - this.steps.file.startUploads(); + this.steps.file.queueUpload( upload ); + this.steps.file.startQueuedUploads(); }, /** @@ -417,6 +418,10 @@ this.uploads.splice( index, 1 ); } + // TODO We should only be doing this for whichever step is currently active + this.steps.file.queue.removeItem( upload ); + this.steps.details.queue.removeItem( upload ); + this.steps.file.updateFileCounts( this.uploads ); if ( this.uploads && this.uploads.length !== 0 ) { diff --git a/resources/uw.ConcurrentQueue.js b/resources/uw.ConcurrentQueue.js new file mode 100644 index 0000000..0495e4a --- /dev/null +++ b/resources/uw.ConcurrentQueue.js @@ -0,0 +1,176 @@ +( function ( mw, uw, $, OO ) { + + /** + * A queue that will execute the asynchronous function `action` for each item in the queue in + * order, taking care not to allow more than `count` instances to be executing at the same time. + * + * Items can be added or removed (#addItem, #removeItem) while the queue is already being + * executed. + * + * @mixins OO.EventEmitter + * @param {Object} options + * @param {Function} options.action Action to execute for each item, must return a Promise + * @param {number} options.count Number of functions to execute concurrently + */ + uw.ConcurrentQueue = function UWConcurrentQueue( options ) { + OO.EventEmitter.call( this ); + + this.count = options.count; + this.action = options.action; + + this.queued = []; + this.running = []; + this.done = []; + + this.completed = false; + this.executing = false; + }; + OO.initClass( uw.ConcurrentQueue ); + OO.mixinClass( uw.ConcurrentQueue, OO.EventEmitter ); + + /** + * A 'progress' event is emitted when one of the functions' promises is resolved or rejected. + * + * @event progress + */ + + /** + * A 'complete' event is emitted when all of the functions' promises have been resolved or rejected. + * + * @event complete + */ + + /** + * A 'change' event is emitted when an item is added to or removed from the queue. + * + * @event change + */ + + /** + * Add an item to the queue. + * + * @param {Object} item + * @return {boolean} true + */ + uw.ConcurrentQueue.prototype.addItem = function ( item ) { + this.queued.push( item ); + this.emit( 'change' ); + if ( this.executing ) { + this.executeNext(); + } + return true; + }; + + /** + * Remove an item from the queue. + * + * While it's possible to remove an item that is being executed, it doesn't stop the execution. + * + * @param {Object} item + * @return {boolean} Whether the item was removed + */ + uw.ConcurrentQueue.prototype.removeItem = function ( item ) { + var index, found; + + found = false; + + index = this.queued.indexOf( item ); + if ( index !== -1 ) { + this.queued.splice( index, 1 ); + found = true; + } + + index = this.done.indexOf( item ); + if ( index !== -1 ) { + this.done.splice( index, 1 ); + found = true; + } + + index = this.running.indexOf( item ); + if ( index !== -1 ) { + this.running.splice( index, 1 ); + this.executeNext(); + found = true; + } + + if ( found ) { + this.emit( 'change' ); + this.checkIfComplete(); + } + + return found; + }; + + /** + * @private + */ + uw.ConcurrentQueue.prototype.promiseComplete = function ( item ) { + var index; + index = this.running.indexOf( item ); + // Check if this item was removed while it was being executed + if ( index !== -1 ) { + this.running.splice( index, 1 ); + this.done.push( item ); + this.emit( 'progress' ); + } + + this.checkIfComplete(); + + this.executeNext(); + }; + + /** + * @private + */ + uw.ConcurrentQueue.prototype.executeNext = function () { + var item, promise, execute, callback; + if ( this.running.length === this.count || !this.executing ) { + return; + } + item = this.queued.shift(); + if ( !item ) { + return; + } + this.running.push( item ); + callback = this.promiseComplete.bind( this, item ); + execute = this.action.bind( null, item ); + // We don't want to accidentally recurse if the promise completes immediately + setTimeout( function () { + promise = execute(); + promise.always( callback ); + } ); + }; + + /** + * Start executing the queue. If the queue is already executing, do nothing. + * + * When the queue finishes executing, a 'complete' event will be emitted. + */ + uw.ConcurrentQueue.prototype.startExecuting = function () { + var i; + if ( this.executing ) { + return; + } + this.completed = false; + this.executing = true; + for ( i = 0; i < this.count; i++ ) { + this.executeNext(); + } + // In case the queue was empty + setTimeout( this.checkIfComplete.bind( this ) ); + }; + + /** + * @private + */ + uw.ConcurrentQueue.prototype.checkIfComplete = function () { + if ( this.running.length === 0 ) { + if ( !this.completed ) { + this.completed = true; + this.executing = false; + this.emit( 'complete' ); + } + } + }; + +} )( mediaWiki, mediaWiki.uploadWizard, jQuery, OO ); diff --git a/tests/qunit/controller/uw.controller.Details.test.js b/tests/qunit/controller/uw.controller.Details.test.js index 4ae46d8..1d75451 100644 --- a/tests/qunit/controller/uw.controller.Details.test.js +++ b/tests/qunit/controller/uw.controller.Details.test.js @@ -15,7 +15,7 @@ * along with DetailsWizard. If not, see <http://www.gnu.org/licenses/>. */ -( function ( uw ) { +( function ( uw, $ ) { QUnit.module( 'mw.uw.controller.Details', QUnit.newMwEnvironment() ); function createTestUpload( sandbox, customDeedChooser, aborted ) { @@ -45,14 +45,18 @@ } QUnit.test( 'Constructor sanity test', 3, function ( assert ) { - var step = new uw.controller.Details(); + var step = new uw.controller.Details( { + maxSimultaneousConnections: 1 + } ); assert.ok( step ); assert.ok( step instanceof uw.controller.Step ); assert.ok( step.ui ); } ); QUnit.test( 'moveTo', 16, function ( assert ) { - var step = new uw.controller.Details(), + var step = new uw.controller.Details( { + maxSimultaneousConnections: 1 + } ), testUpload = createTestUpload( this.sandbox ), stepUiStub = this.sandbox.stub( step.ui, 'moveTo' ); @@ -88,7 +92,7 @@ assert.ok( stepUiStub.called ); } ); - QUnit.test( 'canTransition', 4, function ( assert ) { + QUnit.test( 'canTransition', 3, function ( assert ) { var upload = {}, step = new uw.controller.Details( { maxSimultaneousConnections: 1 @@ -97,10 +101,56 @@ assert.strictEqual( step.canTransition( upload ), false ); upload.state = 'details'; assert.strictEqual( step.canTransition( upload ), true ); - step.uploadsTransitioning = 1; - assert.strictEqual( step.canTransition( upload ), false ); - step.uploadsTransitioning = 0; upload.state = 'complete'; assert.strictEqual( step.canTransition( upload ), false ); } ); -}( mediaWiki.uploadWizard ) ); + + QUnit.asyncTest( 'transitionAll', 4, function ( assert ) { + var tostub, promise, + donestub = this.sandbox.stub(), + ds = [ $.Deferred(), $.Deferred(), $.Deferred() ], + ps = [ ds[ 0 ].promise(), ds[ 1 ].promise(), ds[ 2 ].promise() ], + calls = [], + step; + + tostub = this.sandbox.stub( uw.controller.Details.prototype, 'transitionOne' ); + tostub.onFirstCall().returns( ps[ 0 ] ); + tostub.onSecondCall().returns( ps[ 1 ] ); + tostub.onThirdCall().returns( ps[ 2 ] ); + + this.sandbox.stub( uw.controller.Details.prototype, 'canTransition' ).returns( true ); + + step = new uw.controller.Details( { + maxSimultaneousConnections: 3 + } ); + + step.uploads = [ + { id: 15 }, + undefined, + { id: 21 }, + { id: 'aoeu' } + ]; + + promise = step.transitionAll().done( donestub ); + setTimeout( function () { + calls = [ tostub.getCall( 0 ), tostub.getCall( 1 ), tostub.getCall( 2 ) ]; + + assert.strictEqual( calls[ 0 ].args[ 0 ].id, 15 ); + assert.strictEqual( calls[ 1 ].args[ 0 ].id, 21 ); + + ds[ 0 ].resolve(); + ds[ 1 ].resolve(); + setTimeout( function () { + assert.strictEqual( donestub.called, false ); + + ds[ 2 ].resolve(); + setTimeout( function () { + assert.ok( donestub.called ); + + QUnit.start(); + } ); + } ); + } ); + } ); + +}( mediaWiki.uploadWizard, jQuery ) ); diff --git a/tests/qunit/controller/uw.controller.Step.test.js b/tests/qunit/controller/uw.controller.Step.test.js index 081490d..2d1cdea 100644 --- a/tests/qunit/controller/uw.controller.Step.test.js +++ b/tests/qunit/controller/uw.controller.Step.test.js @@ -24,41 +24,4 @@ assert.ok( step.ui ); } ); - QUnit.test( 'transitionAll', 4, function ( assert ) { - var tostub, promise, - donestub = this.sandbox.stub(), - ds = [ $.Deferred(), $.Deferred(), $.Deferred() ], - ps = [ ds[ 0 ].promise(), ds[ 1 ].promise(), ds[ 2 ].promise() ], - calls = [], - step = new uw.controller.Step( { on: $.noop }, { - maxSimultaneousConnections: 3 - } ); - - step.uploads = [ - { id: 15 }, - undefined, - { id: 21 }, - { id: 'aoeu' } - ]; - - tostub = this.sandbox.stub( step, 'transitionOne' ); - tostub.onFirstCall().returns( ps[ 0 ] ); - tostub.onSecondCall().returns( ps[ 1 ] ); - tostub.onThirdCall().returns( ps[ 2 ] ); - - this.sandbox.stub( step, 'canTransition' ).returns( true ); - - promise = step.transitionAll().done( donestub ); - calls = [ tostub.getCall( 0 ), tostub.getCall( 1 ), tostub.getCall( 2 ) ]; - - assert.strictEqual( calls[ 0 ].args[ 0 ].id, 15 ); - assert.strictEqual( calls[ 1 ].args[ 0 ].id, 21 ); - - ds[ 0 ].resolve(); - ds[ 1 ].resolve(); - assert.strictEqual( donestub.called, false ); - - ds[ 2 ].resolve(); - assert.ok( donestub.called ); - } ); }( mediaWiki.uploadWizard, jQuery ) ); diff --git a/tests/qunit/controller/uw.controller.Upload.test.js b/tests/qunit/controller/uw.controller.Upload.test.js index 2ae9f95..3dfd02a 100644 --- a/tests/qunit/controller/uw.controller.Upload.test.js +++ b/tests/qunit/controller/uw.controller.Upload.test.js @@ -19,14 +19,20 @@ QUnit.module( 'mw.uw.controller.Upload', QUnit.newMwEnvironment() ); QUnit.test( 'Constructor sanity test', 3, function ( assert ) { - var step = new uw.controller.Upload( { maxUploads: 10 } ); + var step = new uw.controller.Upload( { + maxUploads: 10, + maxSimultaneousConnections: 3 + } ); assert.ok( step ); assert.ok( step instanceof uw.controller.Step ); assert.ok( step.ui ); } ); QUnit.test( 'updateFileCounts', 3, function ( assert ) { - var step = new uw.controller.Upload( { maxUploads: 5 } ), + var step = new uw.controller.Upload( { + maxUploads: 5, + maxSimultaneousConnections: 3 + } ), ufcStub = this.sandbox.stub( step.ui, 'updateFileCounts' ); step.updateFileCounts( [ 1, 2 ] ); @@ -41,7 +47,7 @@ assert.ok( ufcStub.calledWith( true, false ) ); } ); - QUnit.test( 'canTransition', 4, function ( assert ) { + QUnit.test( 'canTransition', 3, function ( assert ) { var upload = {}, step = new uw.controller.Upload( { maxSimultaneousConnections: 1 @@ -50,9 +56,6 @@ assert.strictEqual( step.canTransition( upload ), false ); upload.state = 'new'; assert.strictEqual( step.canTransition( upload ), true ); - step.uploadsTransitioning = 1; - assert.strictEqual( step.canTransition( upload ), false ); - step.uploadsTransitioning = 0; upload.state = 'stashed'; assert.strictEqual( step.canTransition( upload ), false ); } ); @@ -61,7 +64,9 @@ var upload = { start: this.sandbox.stub() }, - step = new uw.controller.Upload( {} ); + step = new uw.controller.Upload( { + maxSimultaneousConnections: 1 + } ); this.sandbox.stub( step, 'maybeStartProgressBar' ); assert.strictEqual( upload.start.called, false ); diff --git a/tests/qunit/uw.ConcurrentQueue.test.js b/tests/qunit/uw.ConcurrentQueue.test.js new file mode 100644 index 0000000..c5b137b --- /dev/null +++ b/tests/qunit/uw.ConcurrentQueue.test.js @@ -0,0 +1,437 @@ +/* + * This file is part of the MediaWiki extension UploadWizard. + * + * UploadWizard is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * UploadWizard is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with UploadWizard. If not, see <http://www.gnu.org/licenses/>. + */ + +( function ( uw, $ ) { + QUnit.module( 'uw.ConcurrentQueue', QUnit.newMwEnvironment() ); + + // This returns a function that returns a Promise. At first it resolves after 20 ms. Then, for + // each call of the returned function, the Promise it returns will take 10 ms longer to resolve. + // This ensures that actions in ConcurrentQueue don't all finish at the same instant, which would + // break tests (they make stronger assumptions about the order than ConcurrentQueue guarantees). + function incrementallyDelayedPromise() { + var delay = 20; + return function () { + var deferred = $.Deferred(); + setTimeout( function () { + deferred.resolve(); + }, delay ); + delay += 10; + return deferred.promise(); + }; + } + + // Asserts that the given stub functions were called in the given order. + // SinonJS's assert.callOrder doesn't allow to check individual calls. + function assertCalledInOrder() { + var calls, i, currSpyCall, nextSpyCall; + // Map stubs to specific calls + calls = Array.prototype.map.call( arguments, function ( spy ) { + if ( !spy.assertCallsInOrderLastCall ) { + spy.assertCallsInOrderLastCall = 0; + } + return spy.getCall( spy.assertCallsInOrderLastCall++ ); + } ); + // Assert stuff + for ( i = 0; i < calls.length - 1; i++ ) { + currSpyCall = calls[ i ]; + nextSpyCall = calls[ i + 1 ]; + if ( currSpyCall ) { + QUnit.assert.ok( + currSpyCall.callId < ( nextSpyCall ? nextSpyCall.callId : -1 ), + 'Call ' + ( i + 1 ) + ' (callId ' + currSpyCall.callId + ') is in the right order' + ); + } else { + QUnit.assert.ok( false, 'Call ' + ( i + 1 ) + ' (never called) is in the right order' ); + } + } + QUnit.assert.ok( + nextSpyCall, + 'Call ' + calls.length + ' is in the right order' + ); + } + + QUnit.test( 'Basic behavior', function ( assert ) { + var done, action, queue; + done = assert.async(); + action = sinon.spy( incrementallyDelayedPromise() ); + queue = new uw.ConcurrentQueue( { + count: 3, + action: action + } ); + + queue.on( 'progress', function () { + QUnit.assert.ok( queue.running.length <= 3, 'No more than 3 items are executing' ); + } ); + + queue.on( 'complete', function () { + // All items executed + sinon.assert.callCount( action, 5 ); + // All items executed in the expected order + sinon.assert.calledWith( action.getCall( 0 ), 'a' ); + sinon.assert.calledWith( action.getCall( 1 ), 'b' ); + sinon.assert.calledWith( action.getCall( 2 ), 'c' ); + sinon.assert.calledWith( action.getCall( 3 ), 'd' ); + sinon.assert.calledWith( action.getCall( 4 ), 'e' ); + + done(); + } ); + + [ 'a', 'b', 'c', 'd', 'e' ].forEach( function ( v ) { + queue.addItem( v ); + } ); + + queue.startExecuting(); + } ); + + QUnit.test( 'Event emitting', function ( assert ) { + var done, changeHandler, progressHandler, completeHandler, queue; + done = assert.async(); + changeHandler = sinon.stub(); + progressHandler = sinon.stub(); + completeHandler = sinon.stub(); + queue = new uw.ConcurrentQueue( { + count: 3, + action: incrementallyDelayedPromise() + } ); + + queue.connect( null, { + change: changeHandler, + progress: progressHandler, + complete: completeHandler + } ); + + queue.on( 'complete', function () { + sinon.assert.callCount( changeHandler, 3 ); + sinon.assert.callCount( progressHandler, 3 ); + sinon.assert.callCount( completeHandler, 1 ); + + assertCalledInOrder( + changeHandler, // Added 'a' + changeHandler, // Added 'b' + changeHandler, // Added 'c' + progressHandler, // Finished 'a', 'b' or 'c' + progressHandler, // Finished 'a', 'b' or 'c' + progressHandler, // Finished 'a', 'b' or 'c' + completeHandler + ); + + done(); + } ); + + queue.addItem( 'a' ); + queue.addItem( 'b' ); + queue.addItem( 'c' ); + queue.startExecuting(); + } ); + + QUnit.test( 'Restarting a completed queue', function ( assert ) { + var done, queue; + done = assert.async(); + queue = new uw.ConcurrentQueue( { + count: 3, + action: incrementallyDelayedPromise() + } ); + + queue.addItem( 'a' ); + queue.addItem( 'b' ); + queue.addItem( 'c' ); + queue.startExecuting(); + QUnit.assert.equal( queue.completed, false ); + + queue.once( 'complete', function () { + QUnit.assert.equal( queue.completed, true ); + queue.addItem( 'd' ); + queue.addItem( 'e' ); + queue.startExecuting(); + QUnit.assert.equal( queue.completed, false ); + + queue.once( 'complete', function () { + QUnit.assert.equal( queue.completed, true ); + done(); + } ); + } ); + } ); + + QUnit.test( 'Empty queue completes', function ( assert ) { + var done, queue; + done = assert.async(); + queue = new uw.ConcurrentQueue( { + count: 3, + action: incrementallyDelayedPromise() + } ); + + queue.startExecuting(); + QUnit.assert.equal( queue.completed, false ); + + queue.on( 'complete', function () { + QUnit.assert.equal( queue.completed, true ); + + done(); + } ); + } ); + + QUnit.test( 'Adding new items while queue running', function ( assert ) { + var done, changeHandler, progressHandler, completeHandler, queue; + done = assert.async(); + changeHandler = sinon.stub(); + progressHandler = sinon.stub(); + completeHandler = sinon.stub(); + queue = new uw.ConcurrentQueue( { + count: 2, + action: incrementallyDelayedPromise() + } ); + + queue.connect( null, { + change: changeHandler, + progress: progressHandler, + complete: completeHandler + } ); + + queue.on( 'complete', function () { + setTimeout( function () { + sinon.assert.callCount( changeHandler, 6 ); + sinon.assert.callCount( progressHandler, 5 ); + sinon.assert.callCount( completeHandler, 1 ); + + assertCalledInOrder( + changeHandler, // Added 'a' + changeHandler, // Added 'b' + changeHandler, // Added 'c' + progressHandler, // Finished 'a' or 'b' + changeHandler, // Added 'd' + changeHandler, // Added 'e' + progressHandler, // Finished 'a', 'b' or 'c' + progressHandler, // Finished 'a', 'b', 'c' or 'd' + progressHandler, // Finished 'a', 'b', 'c', 'd' or 'e' + progressHandler, // Finished 'a', 'b', 'c', 'd' or 'e' + completeHandler, + changeHandler // Added 'f', but it's not going to be executed + ); + + done(); + } ); + } ); + + queue.addItem( 'a' ); + queue.addItem( 'b' ); + queue.addItem( 'c' ); + queue.once( 'progress', function () { + setTimeout( function () { + queue.addItem( 'd' ); + queue.addItem( 'e' ); + } ); + } ); + queue.on( 'progress', function () { + if ( queue.done.length === 5 ) { + setTimeout( function () { + queue.addItem( 'f' ); + } ); + } + } ); + queue.startExecuting(); + } ); + + QUnit.test( 'Deleting items while queue running', function ( assert ) { + var done, changeHandler, progressHandler, completeHandler, queue; + done = assert.async(); + changeHandler = sinon.stub(); + progressHandler = sinon.stub(); + completeHandler = sinon.stub(); + queue = new uw.ConcurrentQueue( { + count: 2, + action: incrementallyDelayedPromise() + } ); + + queue.connect( null, { + change: changeHandler, + progress: progressHandler, + complete: completeHandler + } ); + + queue.on( 'complete', function () { + setTimeout( function () { + sinon.assert.callCount( changeHandler, 8 ); + sinon.assert.callCount( progressHandler, 4 ); + sinon.assert.callCount( completeHandler, 1 ); + + assertCalledInOrder( + changeHandler, // Added 'a' + changeHandler, // Added 'b' + changeHandler, // Added 'c' + changeHandler, // Added 'd' + changeHandler, // Added 'e' + changeHandler, // Added 'f' + progressHandler, // Finished 'a' or 'b' + changeHandler, // Removed first of the queued (not executing), which is 'd' + progressHandler, // Finished 'a', 'b' or 'c' + changeHandler, // Removed the last one queued (not executing), which is 'f' + progressHandler, // Finished 'a', 'b', 'c' or 'e' + progressHandler, // Finished 'a', 'b', 'c' or 'e' + completeHandler + ); + + done(); + } ); + } ); + + queue.addItem( 'a' ); + queue.addItem( 'b' ); + queue.addItem( 'c' ); + queue.addItem( 'd' ); + queue.addItem( 'e' ); + queue.addItem( 'f' ); + queue.once( 'progress', function () { + setTimeout( function () { + queue.removeItem( queue.queued[ 0 ] ); + } ); + + queue.once( 'progress', function () { + setTimeout( function () { + queue.removeItem( queue.queued[ 0 ] ); + } ); + } ); + } ); + queue.startExecuting(); + } ); + + QUnit.test( 'Deleting currently running item', function ( assert ) { + var done, action, changeHandler, progressHandler, completeHandler, queue; + done = assert.async(); + action = sinon.spy( incrementallyDelayedPromise() ); + changeHandler = sinon.stub(); + progressHandler = sinon.stub(); + completeHandler = sinon.stub(); + queue = new uw.ConcurrentQueue( { + count: 2, + action: action + } ); + + queue.connect( null, { + change: changeHandler, + progress: progressHandler, + complete: completeHandler + } ); + + queue.on( 'complete', function () { + setTimeout( function () { + // Every item in the queue was executed... + sinon.assert.callCount( action, 4 ); + + sinon.assert.callCount( changeHandler, 5 ); + // ...but the one we removed wasn't registered as finished + sinon.assert.callCount( progressHandler, 3 ); + sinon.assert.callCount( completeHandler, 1 ); + + assertCalledInOrder( + changeHandler, // Added 'a' + changeHandler, // Added 'b' + changeHandler, // Added 'c' + changeHandler, // Added 'd' + action, // Started 'a' + action, // Started 'b' + progressHandler, // Finished 'a' or 'b' + changeHandler, // Removed first of the executing, which is 'a' or 'b' + action, // Started 'c' + action, // Started 'd' - note how two threads are running still + progressHandler, // Finished 'c' or 'd' + progressHandler, // Finished 'c' or 'd' + completeHandler + ); + + done(); + } ); + } ); + + queue.addItem( 'a' ); + queue.addItem( 'b' ); + queue.addItem( 'c' ); + queue.addItem( 'd' ); + queue.once( 'progress', function () { + setTimeout( function () { + queue.removeItem( queue.running[ 0 ] ); + } ); + } ); + queue.startExecuting(); + } ); + + QUnit.test( 'Adding a new item when almost done', function ( assert ) { + var done, action, changeHandler, progressHandler, completeHandler, queue, onProgress; + done = assert.async(); + action = sinon.spy( incrementallyDelayedPromise() ); + changeHandler = sinon.stub(); + progressHandler = sinon.stub(); + completeHandler = sinon.stub(); + queue = new uw.ConcurrentQueue( { + count: 2, + action: action + } ); + + queue.connect( null, { + change: changeHandler, + progress: progressHandler, + complete: completeHandler + } ); + + queue.on( 'complete', function () { + setTimeout( function () { + sinon.assert.callCount( action, 5 ); + sinon.assert.callCount( changeHandler, 5 ); + sinon.assert.callCount( progressHandler, 5 ); + sinon.assert.callCount( completeHandler, 1 ); + + assertCalledInOrder( + changeHandler, // Added 'a' + changeHandler, // Added 'b' + changeHandler, // Added 'c' + changeHandler, // Added 'd' + action, // Started 'a' + action, // Started 'b' + progressHandler, // Finished 'a' or 'b' + action, // Started 'c' + progressHandler, // Finished 'a', 'b' or 'c' + action, // Started 'd' + progressHandler, // Finished 'a', 'b', 'c' or 'd' + changeHandler, // Added 'e' + action, // Started 'e' -- this starts a new thread + progressHandler, // Finished 'a', 'b', 'c', 'd' or 'e' + progressHandler, // Finished 'a', 'b', 'c', 'd' or 'e' + completeHandler + ); + + done(); + } ); + } ); + + queue.addItem( 'a' ); + queue.addItem( 'b' ); + queue.addItem( 'c' ); + queue.addItem( 'd' ); + onProgress = function () { + // queue.running.length is always 1 here, because one action just finished. + // Let the queue potentially start the next one and check then. + setTimeout( function () { + if ( queue.running.length === 1 ) { + queue.addItem( 'e' ); + queue.off( 'progress', onProgress ); + } + } ); + }; + queue.on( 'progress', onProgress ); + queue.startExecuting(); + } ); + +}( mediaWiki.uploadWizard, jQuery ) ); -- To view, visit https://gerrit.wikimedia.org/r/280339 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I32fbd941f5a5dca50c030c3a93889a18537555ef Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/UploadWizard Gerrit-Branch: wmf/1.27.0-wmf.19 Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits