-
Notifications
You must be signed in to change notification settings - Fork 98
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
Use a single WordPress.org API request to get information for all plugins #1562
Changes from 5 commits
6c3796a
6179a0c
d6424a6
23bd591
075cf49
0e669f2
9298221
fcfdc43
b4b4dfd
04ec261
c9aceb2
72bb1bd
d17c835
5dc56de
3490ae8
bf170bc
c9e287e
798de3e
b52f0e0
a082741
843b12d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,58 +19,66 @@ | |
* @return array{name: string, slug: string, short_description: string, requires: string|false, requires_php: string|false, requires_plugins: string[], download_link: string, version: string}|WP_Error Array of plugin data or WP_Error if failed. | ||
*/ | ||
function perflab_query_plugin_info( string $plugin_slug ) { | ||
$plugin = get_transient( 'perflab_plugin_info_' . $plugin_slug ); | ||
|
||
if ( is_array( $plugin ) ) { | ||
/** | ||
* Validated (mostly) plugin data. | ||
* | ||
* @var array{name: string, slug: string, short_description: string, requires: string|false, requires_php: string|false, requires_plugins: string[], download_link: string, version: string} $plugin | ||
*/ | ||
return $plugin; | ||
$transient_key = 'perflab_plugins_info'; | ||
$plugins = get_transient( $transient_key ); | ||
|
||
if ( is_array( $plugins ) && isset( $plugins[ $plugin_slug ] ) ) { | ||
return $plugins[ $plugin_slug ]; | ||
} | ||
|
||
$fields = array( | ||
'name', | ||
'slug', | ||
'short_description', | ||
'requires', | ||
'requires_php', | ||
'requires_plugins', | ||
'download_link', | ||
'version', // Needed by install_plugin_install_status(). | ||
); | ||
$request = wp_remote_get( 'https://api.wordpress.org/plugins/info/1.2/?action=query_plugins&request[author]=wordpressdotorg&request[tag]=performance&request[per_page]=100' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @westonruter Thanks for reviewing this PR. I have not used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You can call the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to work fine: $response = plugins_api(
'query_plugins',
array(
'author' => 'wordpressdotorg',
'tag' => 'performance',
'per_page' => 100,
)
); I can see that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @felixarntz @westonruter Thanks for the feedback, I have implemented the same. |
||
|
||
$plugin = plugins_api( | ||
'plugin_information', | ||
array( | ||
'slug' => $plugin_slug, | ||
'fields' => array_fill_keys( $fields, true ), | ||
) | ||
); | ||
|
||
if ( is_wp_error( $plugin ) ) { | ||
return $plugin; | ||
if ( is_wp_error( $request ) ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A better variable name would be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @westonruter I have corrected the same please check. |
||
return new WP_Error( 'api_error', __( 'Failed to retrieve plugins data from WordPress.org API.', 'default' ) ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally the error being returned would be passed through. This string isn't actually part of core, right? So There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have corrected the same, please check. |
||
} | ||
|
||
if ( is_object( $plugin ) ) { | ||
$plugin = (array) $plugin; | ||
$body = wp_remote_retrieve_body( $request ); | ||
$data = json_decode( $body, true ); | ||
|
||
if ( ! isset( $data['plugins'] ) || ! is_array( $data['plugins'] ) ) { | ||
return new WP_Error( 'no_plugins', __( 'No plugins found in the API response.', 'default' ) ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, is this string actually found in core? If not, then There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @westonruter This has been fixed. |
||
} | ||
|
||
// Only store what we need. | ||
$plugin = wp_array_slice_assoc( $plugin, $fields ); | ||
$plugins = array(); | ||
foreach ( $data['plugins'] as $plugin_data ) { | ||
$plugin_info = wp_array_slice_assoc( | ||
$plugin_data, | ||
array( | ||
'name', | ||
'slug', | ||
'short_description', | ||
'requires', | ||
'requires_php', | ||
'requires_plugins', | ||
'download_link', | ||
'version', | ||
) | ||
); | ||
|
||
// Ensure the 'requires_plugins' is always an array. | ||
if ( ! isset( $plugin_info['requires_plugins'] ) || ! is_array( $plugin_info['requires_plugins'] ) ) { | ||
$plugin_info['requires_plugins'] = array(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this code needed? Right below the if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
||
// Make sure all fields default to false in case another plugin is modifying the response from WordPress.org via the plugins_api filter. | ||
$plugin = array_merge( array_fill_keys( $fields, false ), $plugin ); | ||
// Ensure 'requires' and 'requires_php' are either strings or false. | ||
$plugin_info['requires'] = isset( $plugin_info['requires'] ) ? $plugin_info['requires'] : false; | ||
$plugin_info['requires_php'] = isset( $plugin_info['requires_php'] ) ? $plugin_info['requires_php'] : false; | ||
narenin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
set_transient( 'perflab_plugin_info_' . $plugin_slug, $plugin, HOUR_IN_SECONDS ); | ||
$plugins[ $plugin_data['slug'] ] = $plugin_info; | ||
} | ||
|
||
set_transient( $transient_key, $plugins, HOUR_IN_SECONDS ); | ||
|
||
if ( ! isset( $plugins[ $plugin_slug ] ) ) { | ||
return new WP_Error( 'plugin_not_found', __( 'Plugin not found.', 'default' ) ); | ||
} | ||
|
||
/** | ||
* Validated (mostly) plugin data. | ||
* | ||
* @var array{name: string, slug: string, short_description: string, requires: string|false, requires_php: string|false, requires_plugins: string[], download_link: string, version: string} $plugin | ||
* @var array<string, array{name: string, slug: string, short_description: string, requires: string|false, requires_php: string|false, requires_plugins: string[], download_link: string, version: string}> $plugins | ||
*/ | ||
return $plugin; | ||
return $plugins[ $plugin_slug ]; | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second check here is problematic. We can't check that in the same clause, otherwise a missing plugin will lead to endless re-issuing of the API request and bypassing the cache.
If the cache data is an array, we know it's a cache hit. So then we need to check on that data if the
$plugin_slug
is not set, and if not return the same error as below.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@felixarntz I have implemented this, could you please take a look.