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

[11.x] Auth Query optimization for api calls #1433

Closed
wants to merge 4 commits into from

Conversation

AgonAziri
Copy link

While logging database queries I saw that there were 5 queries such as:

2 queries to oauth_clients table
3 queries to oauth_access_tokens table (2 of which were made by league/oauth2-server package)
In order to reduce number of queries some changes in code where made:

revoked function in ClientRepository now will accept client as parameter instead of ID (this removes 1 query to oauth_clients table). Client object that is passed to this function is returned from findActive function in ClientRepository. To fit those changes I have removed ->with(1) from clients mock in TokenGuardTests ( revoked ).
getPsrRequestViaBearerToken function in TokenGuard that returns some result from league/oauth2-server now will be passed as parameter to hasValidProvider and from there to client function in TokenGuard (this process removes 1 query from 3 total queries made to oauth_access_tokens table)
Changes that were made in this pull request will remove 2 queries out of 5, bringing down number of total queries on each request from 5 to 3.

1. Changed revoked function in ClientRepository, now it will accept client instance instead of id (-1 query in db)
2. Changed Token guard to fit logic I mentioned before
3. PSR now will be passed from function instead of making 2 queries now we will be making only 1 query
4. Removed ->with(1) from revoked from clients mock in Token Guard Tests
@driesvints driesvints changed the title Auth Query optimization for api calls [11.x] Auth Query optimization for api calls Mar 30, 2021
@taylorotwell
Copy link
Member

No plans to change this atm.

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.

2 participants