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

Cache user API call for 5 minutes #907

Merged
merged 6 commits into from
Oct 16, 2024
Merged

Cache user API call for 5 minutes #907

merged 6 commits into from
Oct 16, 2024

Conversation

remyperona
Copy link
Contributor

Description

Cache the user API call for 5 minutes to avoid the request on each page load.

Type of change

  • Bug fix (non-breaking change which fixes an issue).

Detailed scenario

  • With current develop, using query monitor, you can see there is a request to https://app.imagify.io/api/users/me/ on every page load
  • Using this PR branch, the request happens once, and should not happen again for 5 minutes

Technical description

Documentation

Following changes made in #899, #900, #902, the User class is used on more pages than before, and this class makes an API call to get the user information, with a timeout of 10 seconds.

Caching the result of the API call allows to prevent the request on every page.

It was not possible to use the existing imagify_get_cached_user() function, because this function returns a cache that doesn't match the result from the API call, but a result from the cache of the User class itself.

Mandatory Checklist

Code validation

  • I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.

Code style

  • I wrote a self-explanatory code about what it does.
  • I protected entry points against unexpected inputs.
  • I did not introduce unnecessary complexity.

@remyperona remyperona requested a review from a team October 11, 2024 19:39
@remyperona remyperona self-assigned this Oct 11, 2024
@remyperona remyperona added this to the 2.2.3 milestone Oct 11, 2024
@Mai-Saad Mai-Saad self-requested a review October 14, 2024 10:10
@wordpressfan
Copy link
Contributor

@Tabrisrp
Do u know what is the difference between this PR and the code found here:

/**
* Cache Imagify user data for 5 minutes.
* Runs every methods to store the results. Also stores formatted data like the quota and the next update date.
*
* @since 1.7
*
* @return object|bool An object on success. False otherwise.
*/
function imagify_cache_user() {
if ( ! Imagify_Requirements::is_api_key_valid() ) {
return false;
}
$user = new User();
$data = (object) get_object_vars( $user );
$methods = get_class_methods( $user );
foreach ( $methods as $method ) {
if ( '__construct' !== $method ) {
$data->$method = $user->$method();
}
}
$data->quota_formatted = imagify_size_format( $user->quota * pow( 1024, 2 ) );
$data->next_date_update_formatted = date_i18n( get_option( 'date_format' ), strtotime( $user->next_date_update ) );
if ( imagify_is_active_for_network() ) {
set_site_transient( 'imagify_user', $data, 5 * MINUTE_IN_SECONDS );
} else {
set_transient( 'imagify_user', $data, 5 * MINUTE_IN_SECONDS );
}
return $data;
}

Now we inroduce a transient called imagify_user_cache and there is another one called imagify_user

if it's OK to introduce this transient, should we update the data being saved to have those attributes?

$data->quota_formatted = imagify_size_format( $user->quota * pow( 1024, 2 ) );
$data->next_date_update_formatted = date_i18n( get_option( 'date_format' ), strtotime( $user->next_date_update ) );

@remyperona
Copy link
Contributor Author

I explained it in the technical description, they're not exactly the same data. One is caching the API call itself, the other is caching the User class.

Ideally we would implement things differently, but it would require too much work right now.

@Mai-Saad
Copy link

On PR => refresh admin won't send API call to imagify till this transient is expired imagify_check_api_version
On trunk => API call is there with every refresh

@Mai-Saad Mai-Saad added this pull request to the merge queue Oct 16, 2024
Merged via the queue into develop with commit 25c8bfa Oct 16, 2024
6 checks passed
@Mai-Saad Mai-Saad deleted the fix/cache-user-api branch October 16, 2024 08:00
@wordpressfan wordpressfan mentioned this pull request Oct 31, 2024
Miraeld added a commit that referenced this pull request Nov 8, 2024
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.

4 participants