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() {