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

Fix #4906: cache the roles array in the user info object. #4907

Merged
merged 1 commit into from
Nov 10, 2021

Conversation

zyhfish
Copy link
Contributor

@zyhfish zyhfish commented Nov 9, 2021

Fix #4906.

Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

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

Makes sense to me

@bdukes
Copy link
Contributor

bdukes commented Nov 9, 2021

How does this cache get cleared if the roles were to change?

@abner-diaz
Copy link

Hi -- I work for the client that request this patch as a result of our internal load testing. I think the wording cache is not as accurate it just being a private member in the class.

The UserInfo class is CacheLevel.notCacheable, so it's lifecycle seems limited to a request. Also, the code pattern used in the Roles get accessor was taken from the Membership get accessor. So any issues with cache refresh or stale data would also occur there (since that is also likely to change). That is unlikely because the UserInfo class does not seem targeted for caching.

@zyhfish
Copy link
Contributor Author

zyhfish commented Nov 10, 2021

How does this cache get cleared if the roles were to change?

Hi @bdukes , the Roles info is calculated from Social.Roles property that is saved in a private field as well, so it should be good enough to store in a private field for it.

@valadas
Copy link
Contributor

valadas commented Nov 10, 2021

Sounds good to me... @bdukes you are fine with merging this ?

@valadas valadas added this to the 9.10.3 milestone Nov 10, 2021
@bdukes bdukes merged commit 0703796 into dnnsoftware:develop Nov 10, 2021
@zyhfish zyhfish deleted the bug/issue-4906 branch November 10, 2021 13:07
@valadas valadas modified the milestones: 9.10.3, 9.11.0 Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UserInfo.Roles object need to be cached to optimize the performance
5 participants