Skip to content
Merged
Changes from 11 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
120 changes: 82 additions & 38 deletions plugins/performance-lab/includes/admin/plugins.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ function perflab_query_plugin_info( string $plugin_slug ) {
$transient_key = 'perflab_plugins_info';
$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 ] ) ) {
if ( is_array( $plugins ) && isset( $plugins[ $plugin_slug ] ) ) {
if ( isset( $plugins[ $plugin_slug ]['error'] ) ) {
// Plugin was requested before and not found.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Plugin was requested before and not found.
// Plugin was requested before but an error occurred for it.

return new WP_Error(
'plugin_not_found',
__( 'Plugin not found in cached API response.', 'performance-lab' )
$plugins[ $plugin_slug ]['error']['code'],
$plugins[ $plugin_slug ]['error']['message']
);
}
return $plugins[ $plugin_slug ]; // Return cached plugin info if found.
Expand All @@ -54,58 +54,102 @@ function perflab_query_plugin_info( string $plugin_slug ) {
)
);

$has_errors = false;
$plugins = array();

if ( is_wp_error( $response ) ) {
return new WP_Error(
'api_error',
sprintf(
/* translators: %s: API error message */
__( 'Failed to retrieve plugins data from WordPress.org API: %s', 'performance-lab' ),
$response->get_error_message()
)
$plugins[ $plugin_slug ] = array(
'error' => array(
'code' => 'api_error',
'message' => sprintf(
/* translators: %s: API error message */
__( 'Failed to retrieve plugins data from WordPress.org API: %s', 'performance-lab' ),
$response->get_error_message()
),
),
);

$has_errors = true;
}

// Check if the response contains plugins.
if ( ! ( is_object( $response ) && property_exists( $response, 'plugins' ) ) ) {
return new WP_Error( 'no_plugins', __( 'No plugins found in the API response.', 'performance-lab' ) );
if ( ! $has_errors && ! ( is_object( $response ) && property_exists( $response, 'plugins' ) ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

I think it would make more sense to have this as an elseif. Easier to understand, and then there's no need to check for ! $has_errors.

$plugins[ $plugin_slug ] = array(
'error' => array(
'code' => 'no_plugins',
'message' => __( 'No plugins found in the API response.', 'performance-lab' ),
),
);

$has_errors = true;
}

$plugins = array();
$plugin_queue = perflab_get_standalone_plugins();
// Cache error for all standalone plugins.
if ( $has_errors ) {
foreach ( perflab_get_standalone_plugins() as $standalone_plugin ) {
$plugins[ $standalone_plugin ] = $plugins[ $plugin_slug ];
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This works as is, but is hard to follow in terms of code path, because it relies on $has_errors, which is intended for any errors but here it's used to check for whether there are general errors that affect all plugins. It only works correctly because of the position in the code, which is a bit fragile.

I think this would be more intuitive and easier to understand for people looking at the code in the future if we moved this foreach loop into both of the individual conditions where general errors are found (i.e. the conditions in line 57 and 76).

This also avoids an extra if check here that technically is unnecessary.


// Index the plugins from the API response by their slug for efficient lookup.
$all_performance_plugins = array_column( $response->plugins, null, 'slug' );
if ( ! $has_errors && is_object( $response ) && property_exists( $response, 'plugins' ) ) {
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 reiterating an almost similar condition as above, can we move this to an else or elseif please?

$plugin_queue = perflab_get_standalone_plugins();

// Start processing the plugins using a queue-based approach.
while ( count( $plugin_queue ) > 0 ) { // phpcs:ignore Squiz.PHP.DisallowSizeFunctionsInLoops.Found
$current_plugin_slug = array_shift( $plugin_queue );
// Index the plugins from the API response by their slug for efficient lookup.
$all_performance_plugins = array_column( $response->plugins, null, 'slug' );

if ( isset( $plugins[ $current_plugin_slug ] ) ) {
continue;
}
// Start processing the plugins using a queue-based approach.
while ( count( $plugin_queue ) > 0 ) { // phpcs:ignore Squiz.PHP.DisallowSizeFunctionsInLoops.Found
$current_plugin_slug = array_shift( $plugin_queue );

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' )
);
if ( isset( $plugins[ $current_plugin_slug ] ) ) {
continue;
}

if ( ! isset( $all_performance_plugins[ $current_plugin_slug ] ) ) {
// Cache the fact that the plugin was not found.
$plugins[ $current_plugin_slug ] = array(
'error' => array(
'code' => 'plugin_not_found',
'message' => __( 'Plugin not found in API response.', 'performance-lab' ),
),
);

$has_errors = true;
continue;
}

$plugin_data = $all_performance_plugins[ $current_plugin_slug ];
$plugins[ $current_plugin_slug ] = wp_array_slice_assoc( $plugin_data, $fields );

// Enqueue the required plugins slug by adding it to the queue.
if ( isset( $plugin_data['requires_plugins'] ) && is_array( $plugin_data['requires_plugins'] ) ) {
$plugin_queue = array_merge( $plugin_queue, $plugin_data['requires_plugins'] );
}
Copy link
Member

Choose a reason for hiding this comment

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

Trivial point, but I think an else here makes sense:

Suggested change
continue;
}
$plugin_data = $all_performance_plugins[ $current_plugin_slug ];
$plugins[ $current_plugin_slug ] = wp_array_slice_assoc( $plugin_data, $fields );
// Enqueue the required plugins slug by adding it to the queue.
if ( isset( $plugin_data['requires_plugins'] ) && is_array( $plugin_data['requires_plugins'] ) ) {
$plugin_queue = array_merge( $plugin_queue, $plugin_data['requires_plugins'] );
}
} else {
$plugin_data = $all_performance_plugins[ $current_plugin_slug ];
$plugins[ $current_plugin_slug ] = wp_array_slice_assoc( $plugin_data, $fields );
// Enqueue the required plugins slug by adding it to the queue.
if ( isset( $plugin_data['requires_plugins'] ) && is_array( $plugin_data['requires_plugins'] ) ) {
$plugin_queue = array_merge( $plugin_queue, $plugin_data['requires_plugins'] );
}
}

}

$plugin_data = $all_performance_plugins[ $current_plugin_slug ];
$plugins[ $current_plugin_slug ] = wp_array_slice_assoc( $plugin_data, $fields );
if ( ! isset( $plugins[ $plugin_slug ] ) ) {
// Cache the fact that the plugin was not found.
$plugins[ $plugin_slug ] = array(
'error' => array(
'code' => 'plugin_not_found',
'message' => __( 'The requested plugin is not part of Performance Lab plugins.', 'performance-lab' ),
),
);

// Enqueue the required plugins slug by adding it to the queue.
if ( isset( $plugin_data['requires_plugins'] ) && is_array( $plugin_data['requires_plugins'] ) ) {
$plugin_queue = array_merge( $plugin_queue, $plugin_data['requires_plugins'] );
$has_errors = true;
}
}

set_transient( $transient_key, $plugins, HOUR_IN_SECONDS );
if ( $has_errors ) {
set_transient( $transient_key, $plugins, MINUTE_IN_SECONDS );
} else {
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.

To reduce code duplication:

Suggested change
if ( $has_errors ) {
set_transient( $transient_key, $plugins, MINUTE_IN_SECONDS );
} else {
set_transient( $transient_key, $plugins, HOUR_IN_SECONDS );
}
set_transient( $transient_key, $plugins, $has_errors ? MINUTE_IN_SECONDS : HOUR_IN_SECONDS );


if ( ! isset( $plugins[ $plugin_slug ] ) ) {
if ( isset( $plugins[ $plugin_slug ]['error'] ) ) {
return new WP_Error(
'plugin_not_found',
__( 'Plugin not found in API response.', 'performance-lab' )
$plugins[ $plugin_slug ]['error']['code'],
$plugins[ $plugin_slug ]['error']['message']
);
}

Expand Down
Loading