From: Timo Tijhof Date: Wed, 5 Feb 2014 20:54:20 +0000 (-0800) Subject: mediawiki.api: Refactor getToken and postWithToken methods X-Git-Tag: 1.31.0-rc.0~16384^2 X-Git-Url: http://git.cyclocoop.org///%22%40url%40//%22?a=commitdiff_plain;h=9641767d06dac34144a723561cb7004a9978bade;p=lhc%2Fweb%2Fwiklou.git mediawiki.api: Refactor getToken and postWithToken methods * getToken now keeps promise objects around so that it can: 1) Re-use already retrieved tokens instead of always making a new request (this was previously done locally inside one of the callers, namely #postWithToken). 2) By storing promises instead of the tokens, we can re-use it asynchronously. While it is unlikely that one module calls getToken twice in a short period amount of time, it is quite likely that two separate modules (both using mediawiki.api) request similar tokens from the API at the same time (e.g. at page load). This cache has to be additionally keyed by API endpoint since mw.Api is a generic class that can be (and in practice, is) instantied for foreign wiki as well. * Initialise the cache with pre-resolved promises for values from user.tokens from the local wiki (embedded in the page output, no point in requesting these again from the API). * postWithToken no longer has its own token cache, it now unconditionally uses getToken. * postWithToken already took care of clearing the token cache when the server responds with a 'badtoken' error. However, it then changed the request to delete the 'token' query and try again without any token. That seems rather silly. Change this to instead have getToken fetch a fresh one and re-submit with that. * Add unit tests. Bug: 34733 Change-Id: I5c25ae5ea4bf3336899bc60fd94ec3b0948050e1 --- diff --git a/resources/Resources.php b/resources/Resources.php index 5a255e6698..6e0d717f85 100644 --- a/resources/Resources.php +++ b/resources/Resources.php @@ -738,6 +738,7 @@ return array( 'dependencies' => array( 'mediawiki.api', 'mediawiki.Title', + 'user.tokens', ), ), 'mediawiki.api.login' => array( diff --git a/resources/mediawiki.api/mediawiki.api.js b/resources/mediawiki.api/mediawiki.api.js index 04919d3f48..f300672554 100644 --- a/resources/mediawiki.api/mediawiki.api.js +++ b/resources/mediawiki.api/mediawiki.api.js @@ -21,7 +21,19 @@ dataType: 'json' } }, - tokenCache = {}; + // Keyed by ajax url and symbolic name for the individual request + deferreds = {}; + + // Pre-populate with fake ajax deferreds to save http requests for tokens + // we already have on the page via the user.tokens module (bug 34733). + deferreds[ defaultOptions.ajax.url ] = {}; + $.each( mw.user.tokens.get(), function ( key, value ) { + // This requires #getToken to use the same key as user.tokens. + // Format: token-type + "Token" (eg. editToken, patrolToken, watchToken). + deferreds[ defaultOptions.ajax.url ][ key ] = $.Deferred() + .resolve( value ) + .promise( { abort: function () {} } ); + } ); /** * Constructor to create an object to interact with the API of a particular MediaWiki server. @@ -202,33 +214,38 @@ * @since 1.22 */ postWithToken: function ( tokenType, params ) { - var api = this, hasOwn = tokenCache.hasOwnProperty; - if ( hasOwn.call( tokenCache, tokenType ) && tokenCache[tokenType] !== undefined ) { - params.token = tokenCache[tokenType]; + var api = this; + + return api.getToken( tokenType ).then( function ( token ) { + params.token = token; return api.post( params ).then( + // If no error, return to caller as-is null, + // Error handler function ( code ) { if ( code === 'badtoken' ) { - // force a new token, clear any old one - tokenCache[tokenType] = params.token = undefined; - return api.post( params ); + // Clear from cache + deferreds[ this.defaults.ajax.url ][ tokenType + 'Token' ] = + params.token = undefined; + + // Try again, once + return api.getToken( tokenType ).then( function ( token ) { + params.token = token; + return api.post( params ); + } ); } - // Pass the promise forward, so the caller gets error codes + + // Different error, pass on to let caller handle the error code return this; } ); - } else { - return api.getToken( tokenType ).then( function ( token ) { - tokenCache[tokenType] = params.token = token; - return api.post( params ); - } ); - } + } ); }, /** - * Api helper to grab any token. + * Get a token for a certain action from the API. * - * @param {string} type Token type. + * @param {string} type Token type * @return {jQuery.Promise} * @return {Function} return.done * @return {string} return.done.token Received token. @@ -236,24 +253,35 @@ */ getToken: function ( type ) { var apiPromise, + deferredGroup = deferreds[ this.defaults.ajax.url ], + d = deferredGroup && deferredGroup[ type + 'Token' ]; + + if ( !d ) { d = $.Deferred(); - apiPromise = this.get( { - action: 'tokens', - type: type - } ) - .done( function ( data ) { - // If token type is not available for this user, - // key '...token' is missing or can contain Boolean false - if ( data.tokens && data.tokens[type + 'token'] ) { - d.resolve( data.tokens[type + 'token'] ); - } else { - d.reject( 'token-missing', data ); - } - } ) - .fail( d.reject ); + apiPromise = this.get( { action: 'tokens', type: type } ) + .done( function ( data ) { + // If token type is not available for this user, + // key '...token' is missing or can contain Boolean false + if ( data.tokens && data.tokens[type + 'token'] ) { + d.resolve( data.tokens[type + 'token'] ); + } else { + d.reject( 'token-missing', data ); + } + } ) + .fail( d.reject ); + + // Attach abort handler + d.abort = apiPromise.abort; + + // Store deferred now so that we can use this again even if it isn't ready yet + if ( !deferredGroup ) { + deferredGroup = deferreds[ this.defaults.ajax.url ] = {}; + } + deferredGroup[ type + 'Token' ] = d; + } - return d.promise( { abort: apiPromise.abort } ); + return d.promise( { abort: d.abort } ); } }; diff --git a/tests/qunit/suites/resources/mediawiki.api/mediawiki.api.test.js b/tests/qunit/suites/resources/mediawiki.api/mediawiki.api.test.js index 3be3642d13..e5066e0daf 100644 --- a/tests/qunit/suites/resources/mediawiki.api/mediawiki.api.test.js +++ b/tests/qunit/suites/resources/mediawiki.api/mediawiki.api.test.js @@ -80,4 +80,47 @@ this.server.respond(); } ); + QUnit.test( 'getToken( cached )', function ( assert ) { + QUnit.expect( 2 ); + + var api = new mw.Api(); + + // Get editToken for local wiki, this should not make + // a request as it should be retrieved from user.tokens. + api.getToken( 'edit' ) + .done( function ( token ) { + assert.ok( token.length, 'Got a token' ); + } ) + .fail( function ( err ) { + assert.equal( '', err, 'API error' ); + } ); + + assert.equal( this.server.requests.length, 0, 'Requests made' ); + } ); + + QUnit.test( 'getToken( uncached )', function ( assert ) { + QUnit.expect( 2 ); + + var api = new mw.Api(); + + // Get a token of a type that isn't prepopulated by user.tokens. + // Could use "block" or "delete" here, but those could in theory + // be added to user.tokens, use a fake one instead. + api.getToken( 'testaction' ) + .done( function ( token ) { + assert.ok( token.length, 'Got a token' ); + } ) + .fail( function ( err ) { + assert.equal( '', err, 'API error' ); + } ); + + assert.equal( this.server.requests.length, 1, 'Requests made' ); + + this.server.respond( function ( request ) { + request.respond( 200, { 'Content-Type': 'application/json' }, + '{ "tokens": { "testactiontoken": "0123abc" } }' + ); + } ); + } ); + }( mediaWiki ) );