mediawiki.api: Refactor getToken and postWithToken methods
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 5 Feb 2014 20:54:20 +0000 (12:54 -0800)
committerKrinkle <krinklemail@gmail.com>
Wed, 2 Apr 2014 23:22:35 +0000 (23:22 +0000)
* 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

resources/Resources.php
resources/mediawiki.api/mediawiki.api.js
tests/qunit/suites/resources/mediawiki.api/mediawiki.api.test.js

index 5a255e6..6e0d717 100644 (file)
@@ -738,6 +738,7 @@ return array(
                'dependencies' => array(
                        'mediawiki.api',
                        'mediawiki.Title',
+                       'user.tokens',
                ),
        ),
        'mediawiki.api.login' => array(
index 04919d3..f300672 100644 (file)
                                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.
                 * @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.
                 */
                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 } );
                }
        };
 
index 3be3642..e5066e0 100644 (file)
                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 ) );