fix: ImpersonatedCredentials does not use Calendar for expiration#1908
fix: ImpersonatedCredentials does not use Calendar for expiration#1908
Conversation
| @Test | ||
| void refreshAccessToken_GMT_dateParsedCorrectly() throws IOException, IllegalStateException { | ||
| Calendar c = Calendar.getInstance(); | ||
| c.add(Calendar.SECOND, VALID_LIFETIME); | ||
|
|
||
| mockTransportFactory.getTransport().setTargetPrincipal(IMPERSONATED_CLIENT_EMAIL); | ||
| mockTransportFactory.getTransport().setAccessToken(ACCESS_TOKEN); | ||
| mockTransportFactory.getTransport().setExpireTime(getFormattedTime(c.getTime())); | ||
| mockTransportFactory.getTransport().addStatusCodeAndMessage(HttpStatusCodes.STATUS_CODE_OK, ""); |
There was a problem hiding this comment.
Removed old tests that reference the obsolete api (setting custom calendar)
There was a problem hiding this comment.
qq: Are there still use cases where calendar may be used without serializing and deserializing the credentials?
|
diegomarquezp
left a comment
diegomarquezp
left a comment
There was a problem hiding this comment.
LGTM overall, added a couple of questions.
| expirationInstant = | ||
| Instant.from( | ||
| DateTimeFormatter.ISO_INSTANT | ||
| .withZone(calendar.getTimeZone().toZoneId()) |
There was a problem hiding this comment.
IIUC the only effect of the calendar parameter is to change the formatter's timezone. Does that affect the effective expiration instant in the end, or is it just format?
| @Test | ||
| void refreshAccessToken_GMT_dateParsedCorrectly() throws IOException, IllegalStateException { | ||
| Calendar c = Calendar.getInstance(); | ||
| c.add(Calendar.SECOND, VALID_LIFETIME); | ||
|
|
||
| mockTransportFactory.getTransport().setTargetPrincipal(IMPERSONATED_CLIENT_EMAIL); | ||
| mockTransportFactory.getTransport().setAccessToken(ACCESS_TOKEN); | ||
| mockTransportFactory.getTransport().setExpireTime(getFormattedTime(c.getTime())); | ||
| mockTransportFactory.getTransport().addStatusCodeAndMessage(HttpStatusCodes.STATUS_CODE_OK, ""); |
There was a problem hiding this comment.
qq: Are there still use cases where calendar may be used without serializing and deserializing the credentials?
2 participants
See b/423905355 for more information.
Previously seen issues where calendar (marked transient) results in NPE when serializing a credential