Fix ZacTokenService Issue #42
Conversation
| tokenServices.setUseHttps(this.useHttps); | ||
| tokenServices.setTrustedIssuers(trustedIssuers); | ||
| try { | ||
| tokenServices = this.beanFactory.getBean(FastTokenServices.class); |
There was a problem hiding this comment.
why get this bean from the context, if this class is going to override all properties anyway? I suggest use new always.
Also, tokenService is private to this class, and never expected to be configured externally.
There was a problem hiding this comment.
Got it. Since fastTokenservices is referenced/created in a local method scope in the ZacTokenService, would need to bring in powermock to write unit tests. I suspect the Creator might have been used to overcome testing challenges.
| public class FastTokenServicesCreator { | ||
|
|
||
| public FastTokenServices newInstance() { | ||
| return new FastTokenServices(); | ||
| FastTokenServices fastTokenServices = new FastTokenServices(); |
There was a problem hiding this comment.
Instead of having to catch Exception here, consider changing FastTokenService to use a regular map in the initializer for tokenKeys (line 87). It can then override to a expiring map, if afterPropertiesSet is called. This file will not require any change other than marking as deprecated.
There was a problem hiding this comment.
I like that less change and more control. Practicality trumps idealism.
Signed-off-by: Henry <henry.zhao1@ge.com>
ge-guardians
force-pushed
the
DE60291-FastTokenServices-TimedRefresh
branch
from
958e361
to
6fb2d55
Compare
Signed-off-by: Henry <henry.zhao1@ge.com>
…ded some javadoc.
No description provided.