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, Avoid checking resources multi times #1452

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

leanhthang
Copy link

When use devise_token_auth_group the method 'set_user_by_token' has been call multi times per mapping name. so that, resource has found not reused. That will Increase performance.

Copy link
Collaborator

@MaicolBen MaicolBen left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests

@@ -24,6 +24,9 @@ def set_request_start

# user auth
def set_user_by_token(mapping = nil)
# Avoid checking resources multi times
return @resource if resource_is_checked?(mapping)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we just use defined?(@resource) instead of this elaborate check?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed

Copy link
Author

Choose a reason for hiding this comment

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

If you want instead by defined?(@resource) that is similar return @resource if @resource
And, I'm try set return @resource if defined?(@resource), It works fine when logged in.
But, when not yet logged in it still check resources many times.
Screenshot from 2021-01-28 10-49-23

@leanhthang leanhthang force-pushed the fix_checking_resource_muti_times branch 3 times, most recently from bd7242f to 0968c7a Compare January 28, 2021 03:43
@leanhthang leanhthang force-pushed the fix_checking_resource_muti_times branch from 0968c7a to c176baf Compare February 17, 2021 10:51
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.

None yet

3 participants