Skip to content

[msal-node] Cache-1: Msal node cache entities #1444

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

Merged
merged 109 commits into from
May 20, 2020
Merged

Conversation

sameerag
Copy link
Member

@sameerag sameerag commented Apr 1, 2020

This is the first PR for "Cache" supporting UnifiedSchema for msal-node.This PR adds cache entries for the below entities:

  • Account
  • AccessToken
  • IdToken
  • RefreshToken
  • AppMetadata

Tests added. AccountEntity is not 100% covered, and will be added in a follow-up PR.

This was referenced Apr 1, 2020
Copy link
Contributor

@sangonzal sangonzal left a comment

Choose a reason for hiding this comment

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

Looks good. A couple of small comments. Also, I think it would be clearer if we named some of the classes under the entities directory (like AccessTokenCache, AppMetadataCache, etc) AccessTokenCacheEntitiy, AppMetadataEntity, etc, as it denotes that it's a singular entry vs the whole cache. We can discuss this as a group though before making the change.

@sameerag
Copy link
Member Author

sameerag commented Apr 3, 2020

Thanks @sangonzal, regarding the naming, I agree with you, I wasn't a fan of the name as it was too long but may be we should add it to be accurate.

sameerag and others added 24 commits May 7, 2020 17:14
[msal-node] Cache-4: ResponseHandler added to `msal-common`
[msal-node] Cache-3: Add NodeStorage, token generation and separate SPAresponse
[msal-node] Cache-2: Msal node cache serialization and helpers
Remove file read and write from node lib
[msal-node]Cache-6: Read and write to disk, merge cache
@coveralls
Copy link

Coverage Status

Coverage decreased (-2.7%) to 78.531% when pulling b6803ba on msal-node-cache-entities into ad1a86c on dev.

@sameerag
Copy link
Member Author

@jasonnutter Since I got a verbal ACK last time on this PR, I am merging this. If there are any comments, future PRs can attest to them. Since we have too many changes to be pushed to dev going forward, I did not want to wait. Thanks.

@sameerag sameerag closed this May 20, 2020
@sameerag sameerag reopened this May 20, 2020
@sameerag sameerag merged commit 78334c2 into dev May 20, 2020
@sameerag sameerag deleted the msal-node-cache-entities branch May 20, 2020 08:06
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.

6 participants