Use a single WordPress.org API request to get information for all plugins #1562

narenin · 2024-09-25T12:23:06Z

Summary

In this PR we have implemented a single WordPress.org API request to get plugin information because currently, the Performance features screen makes individual plugin_information queries to the WordPress.org API, which is rather inefficient, especially the more plugins we have.

https://api.wordpress.org/plugins/info/1.2/?action=query_plugins&request[author]=wordpressdotorg&request[tag]=performance&request[per_page]=100

Fixes #1542

Relevant technical choices

github-actions · 2024-09-25T12:23:24Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: narenin <narenin@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: mukeshpanchal27 <mukesh27@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

plugins/performance-lab/includes/admin/plugins.php

Co-authored-by: Weston Ruter <westonruter@google.com>

westonruter · 2024-09-25T17:43:29Z

plugins/performance-lab/includes/admin/plugins.php

-		'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' );


Why not use plugins_api() here?

@westonruter Thanks for reviewing this PR. I have not used plugins_api() because this does not support passing multiple specific slugs (which would be ideal) . Please let me know if I am missing anything.

plugins_api() is a wrapper for any API calls to https://api.wordpress.org/plugins/, so you can use it here too. It's only the API itself that doesn't support passing multiple specific slugs, but we're not doing that here anyway.

You can call the plugins_api() function with the query_plugins action and provide the relevant request arguments.

This seems to work fine:

$response = plugins_api( 'query_plugins', array( 'author' => 'wordpressdotorg', 'tag' => 'performance', 'per_page' => 100, ) );

I can see that $response->plugins is populated as expected.

@felixarntz @westonruter Thanks for the feedback, I have implemented the same.

westonruter · 2024-09-25T17:45:03Z

plugins/performance-lab/includes/admin/plugins.php

+	if ( is_wp_error( $request ) ) {
+		return new WP_Error( 'api_error', __( 'Failed to retrieve plugins data from WordPress.org API.', 'default' ) );


Ideally the error being returned would be passed through.

This string isn't actually part of core, right? So default can't be used.

I have corrected the same, please check.

westonruter · 2024-09-25T17:45:32Z

plugins/performance-lab/includes/admin/plugins.php

-
-	if ( is_wp_error( $plugin ) ) {
-		return $plugin;
+	if ( is_wp_error( $request ) ) {


A better variable name would be $response, right? But I think plugins_api() should be used instead in any case.

@westonruter I have corrected the same please check.

westonruter · 2024-09-25T17:46:35Z

plugins/performance-lab/includes/admin/plugins.php

+	$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' ) );


Again, is this string actually found in core? If not, then default cannot be used.

@westonruter This has been fixed.

plugins/performance-lab/includes/admin/plugins.php

Co-authored-by: Felix Arntz <flixos90@gmail.com>

felixarntz · 2024-09-25T18:23:22Z

plugins/performance-lab/includes/admin/plugins.php

-		'download_link',
-		'version', // Needed by install_plugin_install_status().
-	);
+	if ( is_array( $plugins ) && isset( $plugins[ $plugin_slug ] ) ) {


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.

@felixarntz I have implemented this, could you please take a look.

narenin · 2024-09-25T19:26:02Z

@westonruter @felixarntz I have implemented the feedbacks, could you please take a look and share your valuable feedback on this.

westonruter

Minor point in comment styles

plugins/performance-lab/includes/admin/plugins.php

westonruter · 2024-09-25T20:03:40Z

plugins/performance-lab/includes/admin/plugins.php

 	}

-	if ( is_object( $plugin ) ) {
-		$plugin = (array) $plugin;
+	/* Check if the response contains plugins. */


Suggested change

/* Check if the response contains plugins. */

// Check if the response contains plugins.

@westonruter Thanks for the feedback, I have implemented this one.

felixarntz

@narenin This looks almost good to go. A few last notes.

plugins/performance-lab/includes/admin/plugins.php

Co-authored-by: Weston Ruter <westonruter@google.com>

plugins/performance-lab/includes/admin/plugins.php

westonruter · 2024-09-26T19:04:57Z

plugins/performance-lab/includes/admin/plugins.php

+		// 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();
+		}


Why is this code needed? Right below the if $plugin_info['requires_plugins'] is not set, then it is set to false, which will then never occur. Apparently this if statement could be removed?

westonruter · 2024-09-26T19:08:17Z

plugins/performance-lab/includes/admin/plugins.php

+	$plugins = array();
+	foreach ( $response->plugins as $plugin_data ) {
+		$plugin_info = wp_array_slice_assoc( $plugin_data, $fields );
+
+		// 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();
+		}

-	// 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;

-	set_transient( 'perflab_plugin_info_' . $plugin_slug, $plugin, HOUR_IN_SECONDS );
+		$plugins[ $plugin_data['slug'] ] = $plugin_info;
+	}


This would be the equivalent of the existing code:

$plugins = array(); foreach ( $response->plugins as $plugin_data ) { $plugins[ $plugin_data['slug'] ] = wp_array_slice_assoc( $plugin_data, $fields ); }

I'm curious why the additional requires and requires_php checks are now present?

Co-authored-by: Weston Ruter <westonruter@google.com>

narenin · 2024-09-27T01:50:36Z

@westonruter thanks for the suggestions, I have implemented them, please take a look.

westonruter

LGTM!

felixarntz

@narenin Thank you for the PR, this looks great!

narenin · 2024-09-27T18:29:57Z

Thanks @westonruter @felixarntz for your continuous feedback and support.

Implemented single WordPress.org API request to get plugin information

6c3796a

narenin requested review from mukeshpanchal27 and felixarntz as code owners September 25, 2024 12:23

narenin added 3 commits September 25, 2024 17:55

Implemented single WordPress.org API request to get plugin information

6179a0c

Implemented single WordPress.org API request to get plugin information

d6424a6

Implemented single WordPress.org API request to get plugin information

23bd591

westonruter added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Performance Lab Issue relates to work in the Performance Lab Plugin only labels Sep 25, 2024

westonruter added this to the performance-lab n.e.x.t milestone Sep 25, 2024

westonruter reviewed Sep 25, 2024

View reviewed changes

plugins/performance-lab/includes/admin/plugins.php Outdated Show resolved Hide resolved

Update plugins/performance-lab/includes/admin/plugins.php

075cf49

Co-authored-by: Weston Ruter <westonruter@google.com>

westonruter requested changes Sep 25, 2024

View reviewed changes

Implemented the suggested changes.

0e669f2

westonruter reviewed Sep 25, 2024

View reviewed changes

plugins/performance-lab/includes/admin/plugins.php Outdated Show resolved Hide resolved

Implemented translation with Interpolation.

9298221

Co-authored-by: Felix Arntz <flixos90@gmail.com>

felixarntz reviewed Sep 25, 2024

View reviewed changes

narenin added 8 commits September 25, 2024 23:54

Replace $request with $response

fcfdc43

Implemented the suggested changes.

b4b4dfd

Fixed PHPCS Warning

04ec261

Implemented plugin_api()

c9aceb2

Fixed PHPCS Issues

72bb1bd

Update plugins.php

d17c835

Update plugins.php

5dc56de

Update plugins.php

3490ae8

westonruter reviewed Sep 25, 2024

View reviewed changes

felixarntz reviewed Sep 25, 2024

View reviewed changes

plugins/performance-lab/includes/admin/plugins.php Show resolved Hide resolved

narenin and others added 2 commits September 26, 2024 07:22

Update plugins/performance-lab/includes/admin/plugins.php

bf170bc

Co-authored-by: Weston Ruter <westonruter@google.com>

Implemented the suggestions

c9e287e

Implemented the suggestions

798de3e

westonruter reviewed Sep 26, 2024

View reviewed changes

narenin and others added 3 commits September 27, 2024 07:14

Update plugins/performance-lab/includes/admin/plugins.php

b52f0e0

Co-authored-by: Weston Ruter <westonruter@google.com>

Update plugins/performance-lab/includes/admin/plugins.php

a082741

Co-authored-by: Weston Ruter <westonruter@google.com>

Implemented suggestions

843b12d

westonruter approved these changes Sep 27, 2024

View reviewed changes

felixarntz approved these changes Sep 27, 2024

View reviewed changes

felixarntz merged commit ddf297d into WordPress:trunk Sep 27, 2024
18 of 20 checks passed

felixarntz changed the title ~~Implemented single WordPress.org API request to get plugin information~~ Use a single WordPress.org API request to get information for all plugins Sep 27, 2024

mukeshpanchal27 mentioned this pull request Oct 1, 2024

Only store info for relevant standalone plugins in the transient cache #1573

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a single WordPress.org API request to get information for all plugins #1562

Use a single WordPress.org API request to get information for all plugins #1562

narenin commented Sep 25, 2024

github-actions bot commented Sep 25, 2024 •

edited

Loading

westonruter Sep 25, 2024

narenin Sep 25, 2024

felixarntz Sep 25, 2024

westonruter Sep 25, 2024 •

edited

Loading

narenin Sep 25, 2024 •

edited

Loading

westonruter Sep 25, 2024

narenin Sep 25, 2024

westonruter Sep 25, 2024

narenin Sep 25, 2024

westonruter Sep 25, 2024

narenin Sep 25, 2024

felixarntz Sep 25, 2024 •

edited

Loading

narenin Sep 25, 2024

narenin commented Sep 25, 2024

westonruter left a comment

westonruter Sep 25, 2024

narenin Sep 26, 2024

felixarntz left a comment

westonruter Sep 26, 2024

narenin Sep 27, 2024

westonruter Sep 26, 2024

narenin Sep 27, 2024

narenin commented Sep 27, 2024

westonruter left a comment

felixarntz left a comment

narenin commented Sep 27, 2024 •

edited

Loading

		if ( is_wp_error( $request ) ) {
		return new WP_Error( 'api_error', __( 'Failed to retrieve plugins data from WordPress.org API.', 'default' ) );

	/* Check if the response contains plugins. */
	// Check if the response contains plugins.

Use a single WordPress.org API request to get information for all plugins #1562

Use a single WordPress.org API request to get information for all plugins #1562

Conversation

narenin commented Sep 25, 2024

Summary

Relevant technical choices

github-actions bot commented Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

narenin Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixarntz Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narenin commented Sep 25, 2024

westonruter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixarntz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narenin commented Sep 27, 2024

westonruter left a comment

Choose a reason for hiding this comment

felixarntz left a comment

Choose a reason for hiding this comment

narenin commented Sep 27, 2024 • edited Loading

github-actions bot commented Sep 25, 2024 •

edited

Loading

westonruter Sep 25, 2024 •

edited

Loading

narenin Sep 25, 2024 •

edited

Loading

felixarntz Sep 25, 2024 •

edited

Loading

narenin commented Sep 27, 2024 •

edited

Loading