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.

Sign up for GitHub

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

Disregard transient cache in perflab_query_plugin_info() when a plugin is absent #1694

Merged
merged 20 commits into from
Dec 10, 2024
Merged

Disregard transient cache in perflab_query_plugin_info() when a plugin is absent #1694

Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d55cf6b
Update 'perflab_query_plugin_info' function to disregard transient ca…
b1ink0 Nov 20, 2024
d629ec2
Merge branch 'WordPress:trunk' into update/disregard-transient-cache
b1ink0 Nov 22, 2024
54532b5
Add logic for preventing re-request if dependencies not found
b1ink0 Nov 22, 2024
c923c97
Update the error message
b1ink0 Nov 22, 2024
89b4c7f
Refactor to store error instead of false
b1ink0 Nov 25, 2024
2a344c9
Adjust transient expiration time based on plugin availability
b1ink0 Nov 25, 2024
46b1216
Merge branch 'WordPress:trunk' into update/disregard-transient-cache
b1ink0 Nov 26, 2024
ff1b440
Add missing errors to cache, Shorten cache duration for any error
b1ink0 Nov 26, 2024
1d3ef59
Cache error for standalone plugins and update error message for missi…
b1ink0 Nov 28, 2024
80573b6
Merge branch 'trunk' into update/disregard-transient-cache
b1ink0 Dec 3, 2024
49692cb
Change transient cache duration if any error is encountered
b1ink0 Dec 3, 2024
c376ca2
Refactor to move foreach loop into individual conditions
b1ink0 Dec 4, 2024
74cc52e
Refactor to move logic to elseif and else condition
b1ink0 Dec 4, 2024
aeb9fe5
Clarify error message comment
b1ink0 Dec 4, 2024
74ab1e2
Merge branch 'trunk' into update/disregard-transient-cache
b1ink0 Dec 4, 2024
c0fa985
Refactor conditional logic to improve readability
b1ink0 Dec 5, 2024
3868965
Simplify transient cache duration logic
b1ink0 Dec 5, 2024
7709e41
Merge branch 'trunk' into update/disregard-transient-cache
b1ink0 Dec 5, 2024
a090c1b
Merge branch 'trunk' into update/disregard-transient-cache
b1ink0 Dec 9, 2024
571027b
Merge branch 'trunk' into update/disregard-transient-cache
mukeshpanchal27 Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions plugins/performance-lab/includes/admin/plugins.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ function perflab_query_plugin_info( string $plugin_slug ) {
$plugins = get_transient( $transient_key );

if ( is_array( $plugins ) ) {
// If the specific plugin_slug is not in the cache, return an error.
if ( ! isset( $plugins[ $plugin_slug ] ) ) {
return new WP_Error(
'plugin_not_found',
__( 'Plugin not found in cached API response.', 'performance-lab' )
);
if ( isset( $plugins[ $plugin_slug ] ) ) {
if ( false === $plugins[ $plugin_slug ] ) {
// Plugin was requested before and not found.
return new WP_Error(
'plugin_not_found',
__( 'Plugin not found in cached API response.', 'performance-lab' )
);
}
Copy link
Member

Choose a reason for hiding this comment

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

If $plugins[ $plugin_slug ] is either an array or a WP_Error, then this can just return that.

Suggested change
if ( false === $plugins[ $plugin_slug ] ) {
// Plugin was requested before and not found.
return new WP_Error(
'plugin_not_found',
__( 'Plugin not found in cached API response.', 'performance-lab' )
);
}

return $plugins[ $plugin_slug ]; // Return cached plugin info if found.
}
return $plugins[ $plugin_slug ]; // Return cached plugin info if found.
}

$fields = array(
Expand Down Expand Up @@ -86,10 +88,9 @@ function perflab_query_plugin_info( string $plugin_slug ) {
}

if ( ! isset( $all_performance_plugins[ $current_plugin_slug ] ) ) {
return new WP_Error(
'plugin_not_found',
__( 'Plugin not found in WordPress.org API response.', 'performance-lab' )
);
// Cache the fact that the plugin was not found.
$plugins[ $current_plugin_slug ] = false;
Copy link
Member

Choose a reason for hiding this comment

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

Instead of storing false here, what about storing the WP_Error instance? As it stands right now, there are two conditions resulting in false being logged, which conflates these two error scenarios which we tried to get rid of in #1651 to assist with debugging.

Copy link
Member

Choose a reason for hiding this comment

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

Overall this makes sense to me. We probably shouldn't store the WP_Error instance though, but just its data, since storing PHP class instances is a bit unpredictable with what would happen in different object cache implementations.

continue;
}

$plugin_data = $all_performance_plugins[ $current_plugin_slug ];
Expand All @@ -101,9 +102,14 @@ function perflab_query_plugin_info( string $plugin_slug ) {
}
}

if ( ! isset( $plugins[ $plugin_slug ] ) ) {
// Cache the fact that the plugin was not found.
$plugins[ $plugin_slug ] = false;
Copy link
Member

Choose a reason for hiding this comment

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

See above about storing a WP_Error instance instead.

}

set_transient( $transient_key, $plugins, HOUR_IN_SECONDS );
Copy link
Member

Choose a reason for hiding this comment

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

Here I think it would be good to check if there was an error condition, and if so, set the expiration to 1 minute instead of HOUR_IN_SECONDS.


if ( ! isset( $plugins[ $plugin_slug ] ) ) {
if ( false === $plugins[ $plugin_slug ] ) {
return new WP_Error(
'plugin_not_found',
__( 'Plugin not found in API response.', 'performance-lab' )
Expand Down
Loading