Skip to content

Commit 09d5d9d

Browse files
author
Eric Koleda
committed
Reduce unnecessary usage of LockService.
1 parent 4112c3d commit 09d5d9d

File tree

4 files changed

+59
-23
lines changed

4 files changed

+59
-23
lines changed

src/Service.js

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -376,26 +376,33 @@ Service_.prototype.handleCallback = function(callbackRequest) {
376376
* otherwise.
377377
*/
378378
Service_.prototype.hasAccess = function() {
379-
return this.lockable_(function() {
380-
var token = this.getToken();
381-
if (!token || this.isExpired_(token)) {
382-
try {
383-
if (token && this.canRefresh_(token)) {
384-
this.refresh();
385-
} else if (this.privateKey_) {
386-
this.exchangeJwt_();
387-
} else if (this.grantType_) {
388-
this.exchangeGrant_();
389-
} else {
379+
var token = this.getToken();
380+
if (!token || this.isExpired_(token)) {
381+
return this.lockable_(function() {
382+
// Get the token again, bypassing the local memory cache.
383+
token = this.getToken(true);
384+
// Check to see if the token is still missing or expired, as another
385+
// execution may have refreshed it while we were waiting for the lock.
386+
if (!token || this.isExpired_(token)) {
387+
try {
388+
if (token && this.canRefresh_(token)) {
389+
this.refresh();
390+
} else if (this.privateKey_) {
391+
this.exchangeJwt_();
392+
} else if (this.grantType_) {
393+
this.exchangeGrant_();
394+
} else {
395+
return false;
396+
}
397+
} catch (e) {
398+
this.lastError_ = e;
390399
return false;
391400
}
392-
} catch (e) {
393-
this.lastError_ = e;
394-
return false;
395401
}
396-
}
397-
return true;
398-
});
402+
return true;
403+
});
404+
}
405+
return true;
399406
};
400407

401408
/**
@@ -579,10 +586,12 @@ Service_.prototype.saveToken_ = function(token) {
579586

580587
/**
581588
* Gets the token from the service's property store or cache.
589+
* @param {boolean} optSkipMemory Whether to bypass the local memory cache when
590+
* fetching the token (the default is false).
582591
* @return {Object} The token, or null if no token was found.
583592
*/
584-
Service_.prototype.getToken = function() {
585-
return this.getStorage().getValue(null);
593+
Service_.prototype.getToken = function(optSkipMemory) {
594+
return this.getStorage().getValue(null, optSkipMemory);
586595
};
587596

588597
/**

src/Storage.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,16 @@ Storage_.CACHE_EXPIRATION_TIME_SECONDS = 21600; // 6 hours.
4343
/**
4444
* Gets a stored value.
4545
* @param {string} key The key.
46+
* @param {boolean} optSkipMemory Whether to bypass the local memory cache when
47+
* fetching the value (the default is false).
4648
* @return {*} The stored value.
4749
*/
48-
Storage_.prototype.getValue = function(key) {
49-
// Check memory.
50-
if (this.memory_[key]) {
51-
return this.memory_[key];
50+
Storage_.prototype.getValue = function(key, optSkipMemory) {
51+
if (!optSkipMemory) {
52+
// Check memory.
53+
if (this.memory_[key]) {
54+
return this.memory_[key];
55+
}
5256
}
5357

5458
var prefixedKey = this.getPrefixedKey_(key);

test/mocks/lock.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ var waitingFibers = [];
1212
var MockLock = function() {
1313
this.hasLock_ = false;
1414
this.id = Math.random();
15+
this.counter = 0;
1516
};
1617

1718
MockLock.prototype.waitLock = function(timeoutInMillis) {
@@ -20,6 +21,7 @@ MockLock.prototype.waitLock = function(timeoutInMillis) {
2021
if (!locked || this.hasLock_) {
2122
locked = true;
2223
this.hasLock_ = true;
24+
this.counter++;
2325
return;
2426
} else {
2527
waitingFibers.push(Fiber.current);

test/test.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,27 @@ describe('Service', function() {
176176
done();
177177
});
178178
});
179+
180+
it('should not acquire a lock when the token is not expired', function() {
181+
var token = {
182+
granted_time: (new Date()).getTime(),
183+
expires_in: 1000,
184+
access_token: 'foo',
185+
refresh_token: 'bar'
186+
};
187+
var lock = new MockLock();
188+
var properties = new MockProperties({
189+
'oauth2.test': JSON.stringify(token)
190+
});
191+
var service = OAuth2.createService('test')
192+
.setClientId('abc')
193+
.setClientSecret('def')
194+
.setTokenUrl('http://www.example.com')
195+
.setPropertyStore(properties)
196+
.setLock(lock);
197+
service.hasAccess();
198+
assert.equal(lock.counter, 0);
199+
});
179200
});
180201

181202
describe('#refresh()', function() {

0 commit comments

Comments
 (0)