Skip to content
New issue

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

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

Already on GitHub? # to your account

Simplification of the token cache serialization providers #67

Closed
wants to merge 3 commits into from

Conversation

jmprieur
Copy link
Collaborator

Simplification of the token cache serialization providers, which don't need a reference to the options (the ClientId, which was extracted from these options, can be provided by the TokenCacheArguments, or in the call ClearAsync

…t need a reference to the options (the ClientId, which was extracted from these options, can be provided by the TokenCacheArguments, or in the call ClearAsync
@jmprieur jmprieur requested review from jennyf19 and pmaytak March 30, 2020 02:44
…t need a reference to the options (the ClientId, which was extracted from these options, can be provided by the TokenCacheArguments, or in the call ClearAsync
Copy link
Contributor

@pmaytak pmaytak left a comment

Choose a reason for hiding this comment

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

Not sure / don't remember the exact implementation of the cache. Is there one instance of *CacheProvider class per app / client ID? If so, could we store the client ID as an instance variable, instead of passing it in each method call?

@@ -22,6 +22,6 @@ public interface IMsalTokenCacheProvider
/// Clear the cache
/// </summary>
/// <returns></returns>
Task ClearAsync();
Task ClearAsync(string clientId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also update the comments or will there be a task to go over all of the comments in the project?

@jmprieur jmprieur marked this pull request as draft June 17, 2020 14:27
@jmprieur
Copy link
Collaborator Author

@jennyf19 @pmaytak
This would not be needed if we solve #235

Proposing to close this

@jmprieur jmprieur closed this Jun 24, 2020
@jmprieur jmprieur deleted the jmprieur/removeOptionsFromCacheSerializers branch June 28, 2020 15:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants