Simplify hasAccess logic further

Author: Eric Koleda

src/Service.js

@@ -377,32 +377,37 @@ Service_.prototype.handleCallback = function(callbackRequest) {
  */
 Service_.prototype.hasAccess = function() {
   var token = this.getToken();
-  if (!token || this.isExpired_(token)) {
-    return this.lockable_(function() {
-      // Get the token again, bypassing the local memory cache.
-      token = this.getToken(true);
-      // Check to see if the token is still missing or expired, as another
-      // execution may have refreshed it while we were waiting for the lock.
-      if (!token || this.isExpired_(token)) {
-        try {
-          if (token && this.canRefresh_(token)) {
-            this.refresh();
-          } else if (this.privateKey_) {
-            this.exchangeJwt_();
-          } else if (this.grantType_) {
-            this.exchangeGrant_();
-          } else {
-            return false;
-          }
-        } catch (e) {
-          this.lastError_ = e;
-          return false;
-        }
+  if (token && !this.isExpired_(token)) return true; // Token still has access.
+  var canGetToken = (token && this.canRefresh_(token)) ||
+      this.privateKey_ || this.grantType_;
+  if (!canGetToken) return false;
+
+  return this.lockable_(function() {
+    // Get the token again, bypassing the local memory cache.
+    token = this.getToken(true);
+    // Check to see if the token is no longer missing or expired, as another
+    // execution may have refreshed it while we were waiting for the lock.
+    if (token && !this.isExpired_(token)) return true; // Token now has access.
+    try {
+      if (token && this.canRefresh_(token)) {
+        this.refresh();
+        return true;
+      } else if (this.privateKey_) {
+        this.exchangeJwt_();
+        return true;
+      } else if (this.grantType_) {
+        this.exchangeGrant_();
+        return true;
+      } else {
+        // This should never happen, since canGetToken should have been false
+        // earlier.
+        return false;
       }
-      return true;
-    });
-  }
-  return true;
+    } catch (e) {
+      this.lastError_ = e;
+      return false;
+    }
+  });
 };
 
 /**
@@ -586,12 +591,13 @@ Service_.prototype.saveToken_ = function(token) {
 
 /**
  * Gets the token from the service's property store or cache.
- * @param {boolean} optSkipMemory Whether to bypass the local memory cache when
- *     fetching the token (the default is false).
+ * @param {boolean?} optSkipMemoryCheck If true, bypass the local memory cache
+ *     when fetching the token.
  * @return {Object} The token, or null if no token was found.
  */
-Service_.prototype.getToken = function(optSkipMemory) {
-  return this.getStorage().getValue(null, optSkipMemory);
+Service_.prototype.getToken = function(optSkipMemoryCheck) {
+  // Gets the stored value under the null key, which is reserved for the token.
+  return this.getStorage().getValue(null, optSkipMemoryCheck);
 };
 
 /**

src/Storage.js

@@ -43,13 +43,13 @@ Storage_.CACHE_EXPIRATION_TIME_SECONDS = 21600; // 6 hours.
 /**
  * Gets a stored value.
  * @param {string} key The key.
- * @param {boolean} optSkipMemory Whether to bypass the local memory cache when
- *     fetching the value (the default is false).
+ * @param {boolean?} optSkipMemoryCheck Whether to bypass the local memory cache
+ *     when fetching the value (the default is false).
  * @return {*} The stored value.
  */
-Storage_.prototype.getValue = function(key, optSkipMemory) {
-  if (!optSkipMemory) {
-    // Check memory.
+Storage_.prototype.getValue = function(key, optSkipMemoryCheck) {
+  if (!optSkipMemoryCheck) {
+    // Check in-memory cache.
     if (this.memory_[key]) {
       return this.memory_[key];
     }

test/test.js

@@ -214,6 +214,26 @@ describe('Service', function() {
       service.hasAccess();
       assert.equal(lock.counter, 0);
     });
+
+    it('should not acquire a lock when there is no refresh token', function() {
+      var token = {
+        granted_time: 100,
+        expires_in: 100,
+        access_token: 'foo',
+      };
+      var lock = new MockLock();
+      var properties = new MockProperties({
+        'oauth2.test': JSON.stringify(token)
+      });
+      var service = OAuth2.createService('test')
+          .setClientId('abc')
+          .setClientSecret('def')
+          .setTokenUrl('http://www.example.com')
+          .setPropertyStore(properties)
+          .setLock(lock);
+      service.hasAccess();
+      assert.equal(lock.counter, 0);
+    });
   });
 
   describe('#refresh()', function() {