Expose refreshon parameter in authentication result #829
Conversation
| @@ -155,7 +155,10 @@ private AuthenticationResult createAuthenticationResultFromOauthHttpResponse( | |||
| refreshOn(response.getRefreshIn() > 0 ? currTimestampSec + response.getRefreshIn() : 0). | |||
There was a problem hiding this comment.
Is this already exposed in AuthenticationResult? If it is why do we need it in the metadata as well? If this makes more sense in metadata, maybe remove it from here?
There was a problem hiding this comment.
refreshOn is a field in AuthenticationResult but not the IAuthenticationResult interface, and the return type of our acquireToken methods is IAuthenticationResult. AuthenticationResult itself is a package-private, so other projects can only see the fields in IAuthenticationResult
For various historical design reasons IAuthenticationResult has most of the fields listed out individually but that's not very expandable: adding a new field to a public interface is considered a breaking change unless we also add a (potentially meaningless) default value, but that's considered a bad practice.
That's the reason the AuthenticationResultMetadata class was introduced: adding new fields to that is not a breaking change, and it can be a bucket for more niche things like refreshOn that most users will not need to worry about.
…tion-library-for-java into avdunn/expose-refreshon
Exposes the refresh on parameter in the AuthenticationResultMetadata field of an AuthenticationResult, as per #822
Also replaces the (package-private) AuthenticationResultMetadata constructors with lombok's builder annotation to better handle setting the new refreshOn field and any others that may get added.