Matthias Mullie has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/328371 )
Change subject: Use mw.Api instead of XMLHttpRequest
......................................................................
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.
Change-Id: I9c0ef7780dd152d973afac5ff5ea631465d8a15a
---
M resources/handlers/mw.ApiUploadFormDataHandler.js
M resources/handlers/mw.FirefoggHandler.js
M resources/mw.UploadWizard.js
M resources/transports/mw.FormDataTransport.js
M tests/qunit/transports/mw.FormDataTransport.test.js
5 files changed, 139 insertions(+), 178 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard
refs/changes/71/328371/1
diff --git a/resources/handlers/mw.ApiUploadFormDataHandler.js
b/resources/handlers/mw.ApiUploadFormDataHandler.js
index 3d88797..f0622ed 100644
--- a/resources/handlers/mw.ApiUploadFormDataHandler.js
+++ b/resources/handlers/mw.ApiUploadFormDataHandler.js
@@ -23,7 +23,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 );
@@ -60,7 +60,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;
}
@@ -72,7 +72,7 @@
uw.eventFlowLogger.logApiError( 'file', result );
}
handler.upload.setTransported(
result );
- }, function ( result ) {
+ }, function ( code, info, result ) {
if ( !result || result.error ||
( result.upload && result.upload.warnings ) ) {
uw.eventFlowLogger.logApiError( 'file', result );
}
diff --git a/resources/handlers/mw.FirefoggHandler.js
b/resources/handlers/mw.FirefoggHandler.js
index dca4123..7244ad2 100644
--- a/resources/handlers/mw.FirefoggHandler.js
+++ b/resources/handlers/mw.FirefoggHandler.js
@@ -96,7 +96,7 @@
upload.file = file;
transport.uploadHandler = new
mw.ApiUploadFormDataHandler( upload, handler.api );
return transport.uploadHandler.start();
- }, function ( result ) {
+ }, function ( code, info, result ) {
mw.log(
'FirefoggTransport::getTransport> Transport done ' + JSON.stringify( result ) );
upload.setTransported( result );
} );
diff --git a/resources/mw.UploadWizard.js b/resources/mw.UploadWizard.js
index 98d4651..11201eb 100644
--- a/resources/mw.UploadWizard.js
+++ b/resources/mw.UploadWizard.js
@@ -6,7 +6,7 @@
mw.UploadWizard = function ( config ) {
var maxSimPref;
- this.api = new mw.Api( { ajax: { timeout: 0 } } );
+ this.api = this.getApi( { ajax: { timeout: 0 } } );
// making a sort of global for now, should be done by passing
in config or fragments of config
// when needed elsewhere
@@ -83,6 +83,37 @@
this.steps.thanks.setNextStep( this.steps.file );
$( '#mwe-upwiz-steps' ).arrowSteps();
+ },
+
+ /**
+ * @param {Object} options
+ */
+ getApi: function ( options ) {
+ var api = new mw.Api( options );
+
+ api.ajax = function ( parameters, ajaxOptions ) {
+ return mw.Api.prototype.ajax.apply( this, [
parameters, ajaxOptions ] ).then(
+ null, // done handler - doesn't need
overriding
+ function ( code, result ) { // fail
handler
+ if ( code === 'http' && result
) {
+ if ( result.xhr &&
result.xhr.status === 0 ) {
+ code =
'offline';
+ }
+
+ result = {
+ error: {
+ code:
code,
+ info:
result.textStatus
+ }
+ };
+ }
+
+ return $.Deferred().reject(
code, result, result );
+ }
+ );
+ };
+
+ return api;
}
};
diff --git a/resources/transports/mw.FormDataTransport.js
b/resources/transports/mw.FormDataTransport.js
index a877b53..fb286d9 100644
--- a/resources/transports/mw.FormDataTransport.js
+++ b/resources/transports/mw.FormDataTransport.js
@@ -5,23 +5,22 @@
* @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.api = api;
- this.postUrl = postUrl;
// 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,71 +33,77 @@
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.Promise}
*/
- 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 it!
+ */
+ 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 = OO.cloneObject( 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 );
-
- 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;
};
/**
@@ -109,8 +114,7 @@
* @return {jQuery.Promise}
*/
mw.FormDataTransport.prototype.upload = function ( file, tempFileName )
{
- var formData, deferred, ext,
- transport = this;
+ var params, ext;
this.tempname = tempFileName;
// remove unicode characters, tempname is only used during
upload
@@ -126,24 +130,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 );
}
};
@@ -202,18 +191,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 ) {
@@ -224,34 +211,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;
}
@@ -365,7 +339,7 @@
*/
mw.FormDataTransport.prototype.checkStatus = function () {
var transport = this,
- params = {};
+ params = OO.cloneObject( this.formData );
if ( this.aborted ) {
return $.Deferred().reject();
@@ -407,33 +381,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 = {
- error: {
- code: evt.target.status !== 0 ?
'parsererror' : 'offline',
- info: evt.target.responseText
- }
- };
- }
-
- return response;
};
}( mediaWiki, jQuery, OO ) );
diff --git a/tests/qunit/transports/mw.FormDataTransport.test.js
b/tests/qunit/transports/mw.FormDataTransport.test.js
index c3b74c2..617ccd8 100644
--- a/tests/qunit/transports/mw.FormDataTransport.test.js
+++ b/tests/qunit/transports/mw.FormDataTransport.test.js
@@ -18,10 +18,11 @@
( function ( mw, $ ) {
QUnit.module( 'mw.FormDataTransport', QUnit.newMwEnvironment() );
- function createTransport( chunkSize ) {
+ function createTransport( chunkSize, api ) {
var config;
chunkSize = chunkSize || 0;
+ api = api || {};
config = {
useRetryTimeout: false,
@@ -29,7 +30,7 @@
maxPhpUploadSize: chunkSize
};
- return new mw.FormDataTransport( '/w/api.php', {}, config );
+ return new mw.FormDataTransport( api, {}, config );
}
QUnit.test( 'Constructor sanity test', 1, function ( assert ) {
@@ -39,51 +40,48 @@
} );
QUnit.test( 'abort', 3, function ( assert ) {
- var abortStub = this.sandbox.stub(),
- transport = createTransport();
+ var transport = createTransport( 0, { abort:
this.sandbox.stub() } );
- transport.xhr = { abort: abortStub };
-
- assert.ok( abortStub.notCalled );
+ assert.ok( transport.api.abort.notCalled );
transport.abort();
- assert.ok( abortStub.called );
+ assert.ok( transport.api.abort.called );
assert.ok( transport.aborted );
} );
- QUnit.test( 'createXHR', 1, function ( assert ) {
+ QUnit.test( 'createParams', 3, function ( assert ) {
var transport = createTransport( 10 ),
- xhr = transport.createXHR();
+ params = transport.createParams( 'foobar.jpg', 0 );
- assert.ok( xhr );
+ assert.ok( params );
- // TODO there may not be a good way to test events on the XHR,
- // but if such a way crops up later, test 'progress' and
'abort' here.
+ assert.strictEqual( params.filename, 'foobar.jpg' );
+ assert.strictEqual( params.offset, 0 );
} );
- QUnit.test( 'createFormData', 1, function ( assert ) {
- var transport = createTransport( 10 ),
- fd = transport.createFormData( 'foobar.jpg', 0 );
+ QUnit.test( 'post', 2, function ( assert ) {
+ var stub = this.sandbox.stub(),
+ // post() works on a promise and binds .then, so we have to make
+ // sure it actually is a promise, but also that it calls our
stub
+ transport = createTransport( 10, { post: function () {
+ stub();
+ return $.Deferred().resolve();
+ } } );
- assert.ok( fd );
+ this.sandbox.useFakeXMLHttpRequest();
+ this.sandbox.useFakeServer();
- // TODO ARGH APPARENTLY there is no way to access the
properties of a
- // FormData object, so until we can figure THAT out, this is
incomplete.
- } );
+ assert.ok( stub.notCalled );
- QUnit.test( 'sendData', 2, function ( assert ) {
- var transport = createTransport( 10 ),
- fakexhr = { open: this.sandbox.stub(), send:
this.sandbox.stub() };
+ transport.post( {} );
- transport.sendData( fakexhr, {} );
- assert.ok( fakexhr.open.called );
- assert.ok( fakexhr.send.called );
+ assert.ok( stub.called );
} );
QUnit.test( 'upload', 4, function ( assert ) {
var request,
- transport = createTransport( 10 ),
+ transport = createTransport( 10, new mw.Api() ),
fakeFile = {
name: 'test file for fdt.jpg',
size: 5
@@ -97,13 +95,15 @@
assert.strictEqual( this.sandbox.server.requests.length, 1 );
request = this.sandbox.server.requests[ 0 ];
assert.strictEqual( request.method, 'POST' );
- assert.strictEqual( request.url, '/w/api.php' );
+ assert.strictEqual( request.url, mw.util.wikiScript( 'api' ) );
assert.ok( request.async );
+
+ transport.abort();
} );
QUnit.test( 'uploadChunk', 4, function ( assert ) {
var request,
- transport = createTransport( 10 ),
+ transport = createTransport( 10, new mw.Api() ),
fakeFile = {
name: 'test file for fdt.jpg',
size: 20,
@@ -124,12 +124,14 @@
assert.strictEqual( this.sandbox.server.requests.length, 1 );
request = this.sandbox.server.requests[ 0 ];
assert.strictEqual( request.method, 'POST' );
- assert.strictEqual( request.url, '/w/api.php' );
+ assert.strictEqual( request.url, mw.util.wikiScript( 'api' ) );
assert.ok( request.async );
+
+ transport.abort();
} );
QUnit.test( 'checkStatus', 8, function ( assert ) {
- var transport = createTransport( 10 ),
+ var transport = createTransport( 10, new mw.Api() ),
usstub = this.sandbox.stub(),
tstub = this.sandbox.stub(),
poststub = this.sandbox.stub( transport.api, 'post' ),
@@ -181,25 +183,6 @@
postd.reject( 500, 'testing', { error: 'testing' } );
assert.ok( tstub.calledWith( { error: 'testing' } ) );
assert.ok( !usstub.called );
- } );
-
- QUnit.test( 'parseResponse', 2, function ( assert ) {
- var transport = createTransport( 10 ),
- response = {
- target: {
- responseText: '{"testing": "testing"}'
- }
- };
-
- assert.ok( transport.parseResponse( response ), { testing:
'testing' } );
-
- response = { target: { code: 'test', responseText: 'a test
error' } };
- assert.ok( transport.parseResponse( response ), {
- error: {
- code: 'test',
- info: 'a test error'
- }
- } );
} );
}( mediaWiki, jQuery ) );
--
To view, visit https://gerrit.wikimedia.org/r/328371
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9c0ef7780dd152d973afac5ff5ea631465d8a15a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/UploadWizard
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
