Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept a custom clock instance in both Credentials Managers [SDK-1973] #358

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

lbalmaceda
Copy link
Contributor

Changes

This change adds support for passing a custom implementation of the Clock used for verifying the expiration of the credentials stored in the any of the two credentials manager implementations.

References

Testing

  • This change adds unit test coverage

  • This change adds integration test coverage

  • This change has been tested on the latest version of the platform/language or why not

Checklist

@lbalmaceda lbalmaceda added CH: Added small Small review labels Oct 9, 2020
@lbalmaceda lbalmaceda added this to the v1-Next milestone Oct 9, 2020
@lbalmaceda lbalmaceda requested a review from a team October 9, 2020 22:02
* <p>
* This class is thread-safe.
*/
final class ClockImpl implements Clock {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to expose this implementation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is using the Impl suffix on something that implements an interface a common thing in the Java world?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some people use it, others prefer the IClock (interface) and Clock (implementation) variation. In this case, this implementation is internal so the pre/suffix doesn't really matter. For reference, in java-jwt we are doing the same https://github.com/auth0/java-jwt/blob/master/lib/src/main/java/com/auth0/jwt/ClockImpl.java.

* @see com.auth0.android.authentication.storage.SecureCredentialsManager
* @see com.auth0.android.authentication.storage.CredentialsManager
*/
public interface Clock {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what the developers would implement

when(storage.retrieveLong("com.auth0.cache_expires_at")).thenReturn(expirationTime);
when(storage.retrieveString("com.auth0.refresh_token")).thenReturn(null);
when(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken");
assertThat(manager.hasValidCredentials(), is(false));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these tests ensure first that the credentials were deemed expired (due to the date) and when the clock instance is swapped, they get fixed

@lbalmaceda lbalmaceda merged commit e29a089 into master Oct 13, 2020
@lbalmaceda lbalmaceda modified the milestones: v1-Next, 1.28.0 Oct 13, 2020
@lbalmaceda lbalmaceda deleted the f-clock branch January 20, 2021 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Added small Small review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get time from Clock [SDK-1973]
2 participants