Jdlrobson has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/130522

Change subject: Deal better with expired edit tokens
......................................................................

Deal better with expired edit tokens

When it hits a badtoken error postWithToken tries to refresh
the cached edit token. Let's take advantage of this by using the
core method postWithToken

In conjunction with this rename our getToken method to getTokenWithEndpoint
Rename all existing instances.

Bug: 64416
Change-Id: I4fa9964e0e256b92b57364c354fd5a4f8ebb3a98
Dependency: I78ba47a614512f6218e23d03e7c688e2c9efbe45
---
M javascripts/common/api.js
M javascripts/modules/editor/EditorApi.js
M javascripts/modules/mf-watchstar.js
M javascripts/modules/notifications/NotificationsOverlay.js
M javascripts/modules/talk/TalkSectionAddOverlay.js
M javascripts/modules/talk/TalkSectionOverlay.js
M javascripts/modules/uploads/PhotoApi.js
M javascripts/specials/notifications.js
M tests/javascripts/common/test_api.js
M tests/javascripts/modules/editor/test_EditorApi.js
M tests/javascripts/modules/uploads/test_PhotoApi.js
11 files changed, 50 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend 
refs/changes/22/130522/1

diff --git a/javascripts/common/api.js b/javascripts/common/api.js
index 0fd1070..222bb26 100644
--- a/javascripts/common/api.js
+++ b/javascripts/common/api.js
@@ -93,7 +93,8 @@
 
                /**
                 * Retrieves a token for a given endpoint
-                * FIXME: consolidate with mw.Api
+                * FIXME: consolidate with mw.Api.getToken
+                * use postWithToken / getToken where possible
                 *
                 * @method
                 * @param {String} tokenType Name of the type of token needed 
e.g. edit, upload - defaults to edit
@@ -104,13 +105,13 @@
                 * @return {jQuery.Deferred} Object returned by $.ajax(), 
callback will be passed
                 *   the token string, false if the user is anon or undefined 
where not available or a warning is set
                 */
-               getToken: function( tokenType, endpoint, caToken ) {
+               getTokenWithEndpoint: function( tokenType, endpoint, caToken ) {
                        var token, data, d = $.Deferred(), isCacheable,
                                // token types available from mw.user.tokens
                                easyTokens = [ 'edit', 'watch', 'patrol' ];
 
                        tokenType = tokenType || 'edit';
-                       isCacheable = tokenType !== 'centralauth';
+                       isCacheable = false; //tokenType !== 'centralauth';
 
                        if ( !this.tokenCache[ endpoint ] ) {
                                this.tokenCache[ endpoint ] = {};
diff --git a/javascripts/modules/editor/EditorApi.js 
b/javascripts/modules/editor/EditorApi.js
index b2886ff..aa7cb4c 100644
--- a/javascripts/modules/editor/EditorApi.js
+++ b/javascripts/modules/editor/EditorApi.js
@@ -102,14 +102,13 @@
                                throw new Error( 'No changes to save' );
                        }
 
-                       function saveContent( token ) {
+                       function saveContent() {
                                var apiOptions = {
                                        action: 'edit',
                                        title: self.title,
                                        summary: options.summary,
                                        captchaid: options.captchaId,
                                        captchaword: options.captchaWord,
-                                       token: token,
                                        basetimestamp: self.timestamp,
                                        starttimestamp: self.timestamp
                                };
@@ -124,7 +123,7 @@
                                        apiOptions.section = self.sectionId;
                                }
 
-                               self.post( apiOptions ).done( function( data ) {
+                               self.postWithToken( 'edit', apiOptions ).done( 
function( data ) {
                                        var code, warning;
 
                                        if ( data && data.edit && 
data.edit.result === 'Success' ) {
@@ -169,8 +168,7 @@
                                } ).fail( $.proxy( result, 'reject', { type: 
'error', details: 'http' } ) );
                        }
 
-                       this.getToken().done( saveContent ).fail( $.proxy( 
result, 'reject' ) );
-
+                       saveContent();
                        return result;
                },
 
diff --git a/javascripts/modules/mf-watchstar.js 
b/javascripts/modules/mf-watchstar.js
index c4d3db9..3db30da 100644
--- a/javascripts/modules/mf-watchstar.js
+++ b/javascripts/modules/mf-watchstar.js
@@ -95,7 +95,7 @@
                }
 
                function toggleWatchStatus( unwatch ) {
-                       api.getToken( 'watch' ).done( function( token ) {
+                       api.getTokenWithEndpoint( 'watch' ).done( function( 
token ) {
                                toggleWatch( title, token, unwatch,
                                        function( data ) {
                                                success( data, token );
diff --git a/javascripts/modules/notifications/NotificationsOverlay.js 
b/javascripts/modules/notifications/NotificationsOverlay.js
index 5f03eeb..946bd58 100644
--- a/javascripts/modules/notifications/NotificationsOverlay.js
+++ b/javascripts/modules/notifications/NotificationsOverlay.js
@@ -85,7 +85,7 @@
                                }
                        },
                        markAllAsRead: function() {
-                               api.getToken( 'edit' ).done( function( token ) {
+                               api.getTokenWithEndpoint( 'edit' ).done( 
function( token ) {
                                        api.post( {
                                                action : 'echomarkread',
                                                all : true,
diff --git a/javascripts/modules/talk/TalkSectionAddOverlay.js 
b/javascripts/modules/talk/TalkSectionAddOverlay.js
index 5976dd4..0ec7d8f 100644
--- a/javascripts/modules/talk/TalkSectionAddOverlay.js
+++ b/javascripts/modules/talk/TalkSectionAddOverlay.js
@@ -60,7 +60,7 @@
                                this.confirm.prop( 'disabled', true );
                                this.$( '.content' ).empty().addClass( 
'loading' );
                                this.$( '.buttonBar' ).hide();
-                               api.getToken().done( function( token ) {
+                               api.getTokenWithEndpoint().done( function( 
token ) {
                                        api.post( {
                                                action: 'edit',
                                                section: 'new',
diff --git a/javascripts/modules/talk/TalkSectionOverlay.js 
b/javascripts/modules/talk/TalkSectionOverlay.js
index a7ebbe6..758f962 100644
--- a/javascripts/modules/talk/TalkSectionOverlay.js
+++ b/javascripts/modules/talk/TalkSectionOverlay.js
@@ -47,7 +47,7 @@
                                                self.$( '.loading' ).show();
                                                // sign and add newline to front
                                                val = '\n\n' + val + ' ~~~~';
-                                               api.getToken().done( function( 
token ) {
+                                               
api.getTokenWithEndpoint().done( function( token ) {
                                                        api.post( {
                                                                action: 'edit',
                                                                title: 
options.title,
diff --git a/javascripts/modules/uploads/PhotoApi.js 
b/javascripts/modules/uploads/PhotoApi.js
index a4b2113..2b10d8c 100644
--- a/javascripts/modules/uploads/PhotoApi.js
+++ b/javascripts/modules/uploads/PhotoApi.js
@@ -237,7 +237,7 @@
                        }
 
                        function getToken() {
-                               return self.getToken.apply( self, arguments )
+                               return self.getTokenWithEndpoint.apply( self, 
arguments )
                                        .fail( $.proxy( result, 'reject', { 
stage: 'upload', type: 'error', details: 'token' } ) );
                        }
 
diff --git a/javascripts/specials/notifications.js 
b/javascripts/specials/notifications.js
index e3e8915..7b7c18f 100644
--- a/javascripts/specials/notifications.js
+++ b/javascripts/specials/notifications.js
@@ -86,7 +86,7 @@
         * Mark notifications as read.
         */
        function markAsRead( unread ) {
-               api.getToken( 'edit' ).done( function( token ) {
+               api.getTokenWithEndpoint( 'edit' ).done( function( token ) {
                        api.post( {
                                action : 'echomarkread',
                                list : unread.join( '|' ),
diff --git a/tests/javascripts/common/test_api.js 
b/tests/javascripts/common/test_api.js
index 8d99ee4..fa95b6c 100644
--- a/tests/javascripts/common/test_api.js
+++ b/tests/javascripts/common/test_api.js
@@ -107,41 +107,41 @@
        }
 } );
 
-QUnit.test( '#getToken - successful edit token', 1, function() {
-       this.api.getToken( 'edit' ).done( function( token ) {
+QUnit.test( '#getTokenWithEndpoint - successful edit token', 1, function() {
+       this.api.getTokenWithEndpoint( 'edit' ).done( function( token ) {
                strictEqual( token, '123', 'Got token' );
        } );
 } );
 
-QUnit.test( '#getToken - load from cache', 2, function() {
-       this.api.getToken( 'edit' );
-       this.api.getToken( 'edit' ).done( function( token ) { // this comes via 
cache
+QUnit.test( '#getTokenWithEndpoint - load from cache', 2, function() {
+       this.api.getTokenWithEndpoint( 'edit' );
+       this.api.getTokenWithEndpoint( 'edit' ).done( function( token ) { // 
this comes via cache
                strictEqual( token, '123', 'Test for bad token name' );
        } );
 
        strictEqual( stub.getCall( 1 ), null, 'Ajax stub was only called once' 
);
 } );
 
-QUnit.test( '#getToken - cors edit token', 1, function() {
-       this.api.getToken( 'watch', 'http://commons.wikimedia.org/w/api.php' 
).done( function( token ) {
+QUnit.test( '#getTokenWithEndpoint - cors edit token', 1, function() {
+       this.api.getTokenWithEndpoint( 'watch', 
'http://commons.wikimedia.org/w/api.php' ).done( function( token ) {
                strictEqual( token, 'zyx', 'Correctly passed via cors' );
        } );
 } );
 
-QUnit.test( '#getToken - default to edit', 1, function() {
-       this.api.getToken().done( function( token ) {
+QUnit.test( '#getTokenWithEndpoint - default to edit', 1, function() {
+       this.api.getTokenWithEndpoint().done( function( token ) {
                strictEqual( token, '123', 'We get an edit token by default 
(most common)' );
        } );
 } );
 
-QUnit.test( '#getToken - get anon token', 1, function() {
-       this.api.getToken( 'upload' ).fail( function( msg ) {
+QUnit.test( '#getTokenWithEndpoint - get anon token', 1, function() {
+       this.api.getTokenWithEndpoint( 'upload' ).fail( function( msg ) {
                strictEqual( msg, 'Anonymous token.', 'No token given - user 
must be anon' );
        } );
 } );
 
 QUnit.test( '#getToken - bad type of token', 1, function() {
-       this.api.getToken( 'rainbows' ).fail( function( msg ) {
+       this.api.getTokenWithEndpoint( 'rainbows' ).fail( function( msg ) {
                strictEqual( msg, 'Bad token name.', 'Test for bad token name' 
);
        } );
 } );
diff --git a/tests/javascripts/modules/editor/test_EditorApi.js 
b/tests/javascripts/modules/editor/test_EditorApi.js
index 9408c13..5f143bf 100644
--- a/tests/javascripts/modules/editor/test_EditorApi.js
+++ b/tests/javascripts/modules/editor/test_EditorApi.js
@@ -373,4 +373,26 @@
                assert.ok( doneSpy.calledWith( '<h1>Heading 1</h1><h2>Heading 
2</h2><p>test content</p>' ) );
        } );
 
+       QUnit.module( 'MobileFrontend modules/editor/EditorApi expired tokens', 
{
+               setup: function() {
+                       this.editorApi = new EditorApi( { title: 
'MediaWiki:Test.css' } );
+
+                       this.sandbox.stub( this.editorApi, 'post' ).
+                               onFirstCall().returns( $.Deferred().reject( 
"badtoken" ) ).
+                               onSecondCall().returns( $.Deferred().resolve( 
{"edit":{"result":"Success"}} ) );
+
+                       this.sandbox.stub( this.editorApi, 'getToken' ).
+                               onFirstCall().returns( $.Deferred().resolve( 
'cachedbadtoken' ) ).
+                               onSecondCall().returns( $.Deferred().resolve( 
'goodtoken' ) );
+               }
+       } );
+
+       QUnit.test( '#save when token has expired', 2, function( assert ) {
+               this.editorApi.getContent();
+               this.editorApi.setContent( 'section 1' );
+               this.editorApi.save();
+               assert.ok( this.editorApi.getToken.calledTwice, true, 'check 
the spy was called twice' );
+               assert.ok( this.editorApi.post.calledTwice, true, 'check the 
spy was called twice' );
+       } );
+
 }( mw.mobileFrontend, jQuery ) );
diff --git a/tests/javascripts/modules/uploads/test_PhotoApi.js 
b/tests/javascripts/modules/uploads/test_PhotoApi.js
index 2fb55a2..cf580c1 100644
--- a/tests/javascripts/modules/uploads/test_PhotoApi.js
+++ b/tests/javascripts/modules/uploads/test_PhotoApi.js
@@ -11,9 +11,11 @@
 
                        editorApi = new EditorApi( {} );
                        photoApi = new PhotoApi( { editorApi: editorApi } );
+                       // Saves to edits will use getToken
                        this.sandbox.stub( editorApi, 'getToken' ).returns( 
$.Deferred().resolve( 'foo' ) );
+                       this.sandbox.stub( editorApi, 'getTokenWithEndpoint' 
).returns( $.Deferred().resolve( 'foo' ) );
                        this.sandbox.stub( editorApi, 'post' ).returns( 
$.Deferred().resolve( { edit: { result: 'Success' } } ) );
-                       this.sandbox.stub( photoApi, 'getToken' ).returns( 
$.Deferred().resolve( 'foo' ) );
+                       this.sandbox.stub( photoApi, 'getTokenWithEndpoint' 
).returns( $.Deferred().resolve( 'foo' ) );
                        this.sandbox.stub( photoApi, 'post' ).returns( 
$.Deferred().resolve( resp ) );
                }
        } );

-- 
To view, visit https://gerrit.wikimedia.org/r/130522
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4fa9964e0e256b92b57364c354fd5a4f8ebb3a98
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to