Matthias Mullie has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/326937 )
Change subject: [WIP] Use mw.Api instead of XMLHttpRequest ...................................................................... [WIP] Use mw.Api instead of XMLHttpRequest This removes some very specific code (e.g. FormData object) that is not commonly used in MW, and mw.Api supports already. It makes it easier to work with, the response is now just a promise like everywhere else. We no longer need to subscribe to these events and have our own handler to parse the response. We can now also just reuse this.api object, which already takes care of some common UW things, like making sure error response is always consistent. Change-Id: I1850436fadbda4d2c06565f4fea4e8056de4e550 --- M resources/handlers/mw.ApiUploadFormDataHandler.js M resources/transports/mw.FormDataTransport.js M tests/qunit/transports/mw.FormDataTransport.test.js 3 files changed, 73 insertions(+), 132 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard refs/changes/37/326937/1 diff --git a/resources/handlers/mw.ApiUploadFormDataHandler.js b/resources/handlers/mw.ApiUploadFormDataHandler.js index a11b71a..f73dbbd 100644 --- a/resources/handlers/mw.ApiUploadFormDataHandler.js +++ b/resources/handlers/mw.ApiUploadFormDataHandler.js @@ -22,7 +22,7 @@ } ); this.transport = new mw.FormDataTransport( - this.api.defaults.ajax.url, + this.api, this.formData ).on( 'update-stage', function ( stage ) { upload.ui.setStatus( 'mwe-upwiz-' + stage ); @@ -59,7 +59,7 @@ return handler.transport.upload( handler.upload.file, handler.upload.title.getMainText() ) .progress( function ( fraction ) { if ( handler.upload.state === 'aborted' ) { - handler.transport.xhr.abort(); + handler.transport.api.abort(); return; } diff --git a/resources/transports/mw.FormDataTransport.js b/resources/transports/mw.FormDataTransport.js index d532bec..801dcea 100644 --- a/resources/transports/mw.FormDataTransport.js +++ b/resources/transports/mw.FormDataTransport.js @@ -5,23 +5,21 @@ * @constructor * @class mw.FormDataTransport * @mixins OO.EventEmitter - * @param {string} postUrl URL to post to. + * @param {mw.Api} api * @param {Object} formData Additional form fields required for upload api call * @param {Object} [config] * @param {Object} [config.chunkSize] * @param {Object} [config.maxPhpUploadSize] * @param {Object} [config.useRetryTimeout] */ - mw.FormDataTransport = function ( postUrl, formData, config ) { + mw.FormDataTransport = function ( api, formData, config ) { this.config = config || mw.UploadWizard.config; OO.EventEmitter.call( this ); this.formData = formData; this.aborted = false; - this.api = new mw.Api(); - - this.postUrl = postUrl; + this.api = api; // Set chunk size to configured chunk size or max php size, // whichever is smaller. this.chunkSize = Math.min( this.config.chunkSize, this.config.maxPhpUploadSize ); @@ -34,76 +32,78 @@ mw.FormDataTransport.prototype.abort = function () { this.aborted = true; - - if ( this.xhr ) { - this.xhr.abort(); - } + this.api.abort(); }; /** - * Creates an XHR and sets some generic event handlers on it. + * Submits an upload to the API. * - * @param {jQuery.Deferred} deferred Object to send events to. - * @return {XMLHttpRequest} + * @param {Object} params Request params + * @return {jQuery.Deferred} */ - mw.FormDataTransport.prototype.createXHR = function ( deferred ) { - var xhr = new XMLHttpRequest(); + mw.FormDataTransport.prototype.post = function ( params ) { + var deferred = $.Deferred(), + request; - xhr.upload.addEventListener( 'progress', function ( evt ) { - var fraction; - if ( evt.lengthComputable ) { - fraction = parseFloat( evt.loaded / evt.total ); - } else { - fraction = null; + request = this.api.post( params, { + /* + * $.ajax is not quite equiped to handle File uploads with params. + * The most convenient way would be to submit it with a FormData + * object, but mw.Api will already do that for us: it'll transform + * params if it encounters a multipart/form-data POST request, and + * submit it accordingly! + * + * @see https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/Using_XMLHttpRequest#Submitting_forms_and_uploading_files + */ + contentType: 'multipart/form-data', + /* + * $.ajax also has no progress event that will allow us to + * figure out how much of the upload has already gone out, so + * let's add that! + */ + xhr: function () { + var xhr = $.ajaxSettings.xhr() ; + xhr.upload.addEventListener( 'progress', function ( evt ) { + var fraction = null; + if ( evt.lengthComputable ) { + fraction = parseFloat( evt.loaded / evt.total ); + } + deferred.notify( fraction ); + }, false ); + return xhr ; } - deferred.notify( fraction ); - }, false ); + } ); - return xhr; + // just pass on success & failures + request.then( deferred.resolve, deferred.reject ); + + return deferred.promise(); }; /** - * Creates a FormData object suitable for upload. + * Creates the upload API params. * * @param {string} filename * @param {number} [offset] For chunked uploads - * @return {FormData} + * @return {Object} */ - mw.FormDataTransport.prototype.createFormData = function ( filename, offset ) { - var formData = new FormData(); + mw.FormDataTransport.prototype.createParams = function ( filename, offset ) { + var params = this.formData; - $.each( this.formData, function ( key, value ) { - formData.append( key, value ); + $.extend( params, { + filename: filename, + + // ignorewarnings is turned on, since warnings are presented in a + // later step and this transport doesn't know how to deal with them. + // Also, it's important to allow people to upload files with (for + // example) blacklisted names, and then rename them later in the + // wizard. + ignorewarnings: true, + + offset: offset || 0 } ); - formData.append( 'filename', filename ); - - // ignorewarnings is turned on, since warnings are presented in a - // later step and this transport doesn't know how to deal with them. - // Also, it's important to allow people to upload files with (for - // example) blacklisted names, and then rename them later in the - // wizard. - formData.append( 'ignorewarnings', true ); - - formData.append( 'offset', offset || 0 ); - - // request parsed errors, in user language - formData.append( 'errorformat', 'html' ); - formData.append( 'uselang', mw.config.get( 'wgUserLanguage' ) ); - formData.append( 'formatversion', 2 ); - - return formData; - }; - - /** - * Sends data in a FormData object through an XHR. - * - * @param {XMLHttpRequest} xhr - * @param {FormData} formData - */ - mw.FormDataTransport.prototype.sendData = function ( xhr, formData ) { - xhr.open( 'POST', this.postUrl, true ); - xhr.send( formData ); + return params; }; /** @@ -114,8 +114,7 @@ * @return {jQuery.Promise} */ mw.FormDataTransport.prototype.upload = function ( file, tempFileName ) { - var formData, deferred, ext, - transport = this; + var params, ext; // use timestamp + filename to avoid conflicts on server this.tempname = ( new Date() ).getTime().toString() + tempFileName; @@ -132,24 +131,9 @@ if ( file.size > this.chunkSize ) { return this.chunkedUpload( file ); } else { - deferred = $.Deferred(); - this.xhr = this.createXHR( deferred ); - this.xhr.addEventListener( 'load', function ( evt ) { - deferred.resolve( transport.parseResponse( evt ) ); - }, false ); - this.xhr.addEventListener( 'error', function ( evt ) { - deferred.reject( transport.parseResponse( evt ) ); - }, false ); - this.xhr.addEventListener( 'abort', function ( evt ) { - deferred.reject( transport.parseResponse( evt ) ); - }, false ); - - formData = this.createFormData( this.tempname ); - formData.append( 'file', file ); - - this.sendData( this.xhr, formData ); - - return deferred.promise(); + params = this.createParams( this.tempname ); + params.file = file; + return this.post( params ); } }; @@ -208,18 +192,16 @@ * @return {jQuery.Promise} */ mw.FormDataTransport.prototype.uploadChunk = function ( file, offset ) { - var formData, - deferred = $.Deferred(), + var params = this.createParams( this.tempname, offset ), transport = this, bytesAvailable = file.size, chunk; if ( this.aborted ) { - if ( this.xhr ) { - this.xhr.abort(); - } - return deferred.reject(); + this.api.abort(); + return $.Deferred().reject(); } + // Slice API was changed and has vendor prefix for now // new version now require start/end and not start/length if ( file.mozSlice ) { @@ -230,34 +212,21 @@ chunk = file.slice( offset, offset + this.chunkSize, file.type ); } - this.xhr = this.createXHR( deferred ); - this.xhr.addEventListener( 'load', deferred.resolve, false ); - this.xhr.addEventListener( 'error', deferred.reject, false ); - this.xhr.addEventListener( 'abort', deferred.reject, false ); - - formData = this.createFormData( this.tempname, offset ); - // only enable async if file is larger 10Mb if ( bytesAvailable > 10 * 1024 * 1024 ) { - formData.append( 'async', true ); + params.async = true; } // If offset is 0, we're uploading the file from scratch. filekey may be set if we're retrying // the first chunk. The API errors out if a filekey is given with zero offset (as it's // nonsensical). TODO Why do we need to retry in this case, if we managed to upload something? if ( this.filekey && offset !== 0 ) { - formData.append( 'filekey', this.filekey ); + params.filekey = this.filekey; } - formData.append( 'filesize', bytesAvailable ); - formData.append( 'chunk', chunk ); + params.filesize = bytesAvailable; + params.chunk = chunk; - this.sendData( this.xhr, formData ); - - return deferred.promise().then( function ( evt ) { - return transport.parseResponse( evt ); - }, function ( evt ) { - return transport.parseResponse( evt ); - } ).then( function ( response ) { + return this.post( params ).then( function ( response ) { if ( response.upload && response.upload.filekey ) { transport.filekey = response.upload.filekey; } @@ -413,34 +382,6 @@ }, function ( code, info, response ) { return $.Deferred().reject( response ); } ); - }; - - /** - * Parse response from the server. - * - * @param {Event} evt - * @return {Object} - */ - mw.FormDataTransport.prototype.parseResponse = function ( evt ) { - var response; - - try { - response = JSON.parse( evt.target.responseText ); - } catch ( e ) { - if ( window.console ) { - // Let's check what caused this, too. - window.console.error( 'parsererror', evt ); - } - - response = { - errors: [ { - code: 'parsererror', - html: mw.message( evt.target.status !== 0 ? 'api-error-parsererror' : 'api-error-offline' ).text() - } ] - }; - } - - return response; }; }( mediaWiki, jQuery, OO ) ); diff --git a/tests/qunit/transports/mw.FormDataTransport.test.js b/tests/qunit/transports/mw.FormDataTransport.test.js index 436a00c..32fe513 100644 --- a/tests/qunit/transports/mw.FormDataTransport.test.js +++ b/tests/qunit/transports/mw.FormDataTransport.test.js @@ -29,7 +29,7 @@ maxPhpUploadSize: chunkSize }; - return new mw.FormDataTransport( '/w/api.php', {}, config ); + return new mw.FormDataTransport( new mw.Api(), {}, config ); } QUnit.test( 'Constructor sanity test', 1, function ( assert ) { -- To view, visit https://gerrit.wikimedia.org/r/326937 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1850436fadbda4d2c06565f4fea4e8056de4e550 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/UploadWizard Gerrit-Branch: master Gerrit-Owner: Matthias Mullie <mmul...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits