Add locking around refresh()

Author: Eric Koleda

src/Service.gs

@@ -504,38 +504,40 @@ Service_.prototype.refresh = function() {
     'Token URL': this.tokenUrl_
   });
 
-  var token = this.getToken();
-  if (!token.refresh_token) {
-    throw new Error('Offline access is required.');
-  }
-  var headers = {
-    'Accept': this.tokenFormat_
-  };
-  if (this.tokenHeaders_) {
-    headers = extend_(headers, this.tokenHeaders_);
-  }
-  var tokenPayload = {
-      refresh_token: token.refresh_token,
-      client_id: this.clientId_,
-      client_secret: this.clientSecret_,
-      grant_type: 'refresh_token'
-  };
-  if (this.tokenPayloadHandler_) {
-    tokenPayload = this.tokenPayloadHandler_(tokenPayload);
-  }
-  // Use the refresh URL if specified, otherwise fallback to the token URL.
-  var url = this.refreshUrl_ || this.tokenUrl_;
-  var response = UrlFetchApp.fetch(url, {
-    method: 'post',
-    headers: headers,
-    payload: tokenPayload,
-    muteHttpExceptions: true
+  this.lockable_(function() {
+    var token = this.getToken();
+    if (!token.refresh_token) {
+      throw new Error('Offline access is required.');
+    }
+    var headers = {
+      'Accept': this.tokenFormat_
+    };
+    if (this.tokenHeaders_) {
+      headers = extend_(headers, this.tokenHeaders_);
+    }
+    var tokenPayload = {
+        refresh_token: token.refresh_token,
+        client_id: this.clientId_,
+        client_secret: this.clientSecret_,
+        grant_type: 'refresh_token'
+    };
+    if (this.tokenPayloadHandler_) {
+      tokenPayload = this.tokenPayloadHandler_(tokenPayload);
+    }
+    // Use the refresh URL if specified, otherwise fallback to the token URL.
+    var url = this.refreshUrl_ || this.tokenUrl_;
+    var response = UrlFetchApp.fetch(url, {
+      method: 'post',
+      headers: headers,
+      payload: tokenPayload,
+      muteHttpExceptions: true
+    });
+    var newToken = this.getTokenFromResponse_(response);
+    if (!newToken.refresh_token) {
+      newToken.refresh_token = token.refresh_token;
+    }
+    this.saveToken_(newToken);
   });
-  var newToken = this.getTokenFromResponse_(response);
-  if (!newToken.refresh_token) {
-    newToken.refresh_token = token.refresh_token;
-  }
-  this.saveToken_(newToken);
 };
 
 /**

test/mocks/urlfetchapp.js

@@ -1,17 +1,18 @@
 var Future = require('fibers/future');
 
 var MockUrlFetchApp = function() {
-  this.delay = 0;
+  this.delayFunction = () => 0;
   this.resultFunction = () => '';
 };
 
 MockUrlFetchApp.prototype.fetch = function(url, optOptions) {
-  var delay = this.delay;
+  var delay = this.delayFunction();
+  var result = this.resultFunction();
   if (delay) {
     sleep(delay).wait();
   }
   return {
-    getContentText: this.resultFunction,
+    getContentText: () => result,
     getResponseCode: () => 200
   };
 };

test/test.js

@@ -147,7 +147,7 @@ describe('Service', function() {
       var properties = new MockProperties();
       properties.setProperty('oauth2.test', JSON.stringify(token));
 
-      mocks.UrlFetchApp.delay = 100;
+      mocks.UrlFetchApp.delayFunction = () => 100;
       mocks.UrlFetchApp.resultFunction = () =>
         JSON.stringify({
           access_token: Math.random().toString(36)
@@ -181,8 +181,69 @@ describe('Service', function() {
       });
     });
   });
+
+  describe('#refresh()', function() {
+    /*
+      A race condition can occur when two executions attempt to refresh the
+      token at the same time. Some OAuth implementations only allow one
+      valid access token at a time, so we need to ensure that the last access
+      token granted is the one that is persisted. To replicate this, we have the
+      first exeuction wait longer for it's response to return through the
+      "network" and have the second execution get it's response back sooner.
+    */
+    it('should use the lock to prevent race conditions', function(done) {
+      var token = {
+        granted_time: 100,
+        expires_in: 100,
+        refresh_token: 'bar'
+      };
+      var properties = new MockProperties();
+      properties.setProperty('oauth2.test', JSON.stringify(token));
+
+      var count = 0;
+      mocks.UrlFetchApp.resultFunction = function() {
+        return JSON.stringify({
+          access_token: 'token' + count++
+        });
+      };
+      var delayGenerator = function*() {
+        yield 100;
+        yield 10;
+      }();
+      mocks.UrlFetchApp.delayFunction = function() {
+        return delayGenerator.next().value;
+      };
+
+      var refreshToken = function() {
+        var service = OAuth2.createService('test')
+            .setClientId('abc')
+            .setClientSecret('def')
+            .setTokenUrl('http://www.example.com')
+            .setPropertyStore(properties)
+            .setLock(new MockLock());
+        service.refresh();
+      }.future();
+
+      Future.task(function() {
+        var first = refreshToken();
+        var second = refreshToken();
+        Future.wait(first, second);
+        return [first.get(), second.get()];
+      }).resolve(function(err) {
+        if (err) {
+          done(err);
+        }
+        var storedToken = JSON.parse(properties.getProperty('oauth2.test'));
+        assert.equal(storedToken.access_token, 'token1');
+        done();
+      });
+    });
+  });
+
 });
 
+
+
 describe('Utilities', function() {
   describe('#extend_()', function() {
     var extend_ = OAuth2.extend_;