perf: cache raw stfc rolls call #892
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ACLay
reviewed
Dec 16, 2024
| await client.getRolesForUser(process.env.EXTERNAL_AUTH_TOKEN, userNumber) | ||
| )?.return; | ||
| const stfcRoles: stfcRole[] | null = await ( | ||
| this.userDataSource as StfcUserDataSource |
There was a problem hiding this comment.
Is the cast to StfcUserDataSource necessary? It looks like the variable is already typed that way.
| @@ -128,6 +128,11 @@ export class StfcUserDataSource implements UserDataSource { | |||
| StfcUserDataSource.rolesCacheSecondsToLive | |||
| ).enableStatsLogging('uowsRolesCache'); | |||
|
|
|||
| private uowsRawRolesCache = new Cache<Promise<stfcRole[] | null>>( | |||
There was a problem hiding this comment.
I see where you're coming from with the raw names, but I wonder if it might be more intuitive if this new cache was called uowsRolesCache or stfcRolesCache and the existing one was renamed to uopRolesCache?
ACLay
approved these changes
Dec 16, 2024
jekabs-karklins
approved these changes
Dec 19, 2024
There was a problem hiding this comment.
This looks ok, but one thing I want to ask if maybe this is making more sense to be cached at UOWSSoapClient level rather than datasource.
Of course it is up to you, I'm just thinking that putting it in there seems more logical as if there is another component relying on UOWSSoapClient it would benefit from the cache.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes UserOfficeProject/issue-tracker#1290
Description
This PR improves the performance of our application by caching raw STFC roles call.
Motivation and Context
This change is required to optimize the application and reduce unnecessary calls to the STFC service. By caching the raw STFC roles call, we are able to significantly reduce the load time and enhance the user experience.
Changes
getRolesForUserin theStfcUserAuthorizationclass has been updated to fetch roles fromStfcUserDataSourceinstead of directly from the client.uowsRawRolesCachehas been introduced in theStfcUserDataSourceclass to store raw STFC roles.getRolesForUserin theStfcUserDataSourceclass has been updated to use the newly introduced cache. If the roles are not in the cache, it fetches them from the client, stores them in the cache, and then returns them.getUserRolesin theStfcUserDataSourceclass has been updated to use thegetRolesForUsermethod instead of directly calling the client. This allows it to benefit from the caching mechanism.How Has This Been Tested?
Fixes Jira Issue
https://jira.esss.lu.se/browse/
Depends On
Tests included/Docs Updated?