Skip to content

feat: add authenticator invalidate (SHOP-231)#134

Closed
vsolovei-smartling wants to merge 1 commit intomasterfrom
SHOP-231-invalidate
Closed

feat: add authenticator invalidate (SHOP-231)#134
vsolovei-smartling wants to merge 1 commit intomasterfrom
SHOP-231-invalidate

Conversation

@vsolovei-smartling
Copy link
Contributor

Having invalidation will allow to retry invalid token

Comment on lines +119 to +124
public synchronized void invalidate()
{
this.authentication = null;
this.expiresAt = -1;
this.refreshExpiresAt = -1;
}

Choose a reason for hiding this comment

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

It's not called by anyone, isn't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

that filter is only for logging. your http client from sdk already gets 401 response isn't? why can't you handle this 401 in sdk directly?

Comment on lines -123 to +130
this.refreshExpiresAt = authentication.getRefreshExpiresIn() * 100 + System.currentTimeMillis();
this.refreshExpiresAt = authentication.getRefreshExpiresIn() * 1000 + System.currentTimeMillis();

Choose a reason for hiding this comment

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

Seems like the real fix is this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is THE fix, it will reduce full re-authentication count, the only downside of having 100 vs 1000 is the frequency of full re-authentications vs refreshes.

@vsolovei-smartling
Copy link
Contributor Author

Will go with handling in the sdk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants