Account for plugin dependencies when storing relevant plugin info #1613

westonruter · 2024-10-21T17:46:26Z

This is a follow-up hotfix to #1573 which broke the ability to install the Image Prioritizer plugin from the Performance features screen since the fetched plugin info did not include plugin dependencies, specifically Optimization Detective.

This also bumps the plugin version to 3.5.1.

github-actions · 2024-10-21T17:46:41Z

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: westonruter <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: joemcgill <[email protected]>
Co-authored-by: adamsilverstein <[email protected]>

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

westonruter · 2024-10-21T17:48:56Z

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

 */
 function perflab_query_plugin_info( string $plugin_slug ) {
-	$transient_key = 'perflab_plugins_info';
+	$transient_key = 'perflab_plugins_info-v2';


The transient key needs to be bumped since sites may already have downloaded bad plugin data with the old key.

Seems like we should include a hash of the plugin version or something so that this type of bump would be unnecessary

Oh, additionally, we should probably update the logic below so that if a slug is not found in the cache, it tries to request it from the API instead of returning a WP_Error right away.

We could, but this is analogous to the $wp_db_version in core which doesn't necessarily need to change with each plugin release.

Oh, additionally, we should probably update the logic below so that if a slug is not found in the cache, it tries to request it from the API instead of returning a WP_Error right away.

Yeah, that's a good idea too.

Filed in #1617

Co-authored-by: swissspidy <[email protected]>

westonruter · 2024-10-21T18:01:54Z

Updated build for testing: performance-lab.zip

Post 3.5.0 release merge

… fix/storing-standalone-plugin-info

adamsilverstein

Looks good, fixes the issue in my testing

westonruter · 2024-10-21T18:16:04Z

Pending release diffs from 3.5.0:

`auto-sizes`

Warning

Stable tag is unchanged at 1.3.0, so no plugin release will occur.

svn status:

M       readme.txt

svn diff

Index: readme.txt
===================================================================
--- readme.txt	(revision 3173122)
+++ readme.txt	(working copy)
@@ -1,7 +1,7 @@
 === Enhanced Responsive Images ===
 
 Contributors: wordpressdotorg
-Tested up to: 6.6
+Tested up to: 6.7
 Stable tag:   1.3.0
 License:      GPLv2 or later
 License URI:  https://www.gnu.org/licenses/gpl-2.0.html

`dominant-color-images`

Warning

Stable tag is unchanged at 1.1.2, so no plugin release will occur.

svn status:

M       readme.txt

svn diff

Index: readme.txt
===================================================================
--- readme.txt	(revision 3173122)
+++ readme.txt	(working copy)
@@ -1,7 +1,7 @@
 === Image Placeholders ===
 
 Contributors: wordpressdotorg
-Tested up to: 6.6
+Tested up to: 6.7
 Stable tag:   1.1.2
 License:      GPLv2 or later
 License URI:  https://www.gnu.org/licenses/gpl-2.0.html

`embed-optimizer`

Note

No changes.

`image-prioritizer`

Note

No changes.

`optimization-detective`

Note

No changes.

`performance-lab`

Important

Stable tag change: 3.5.0 → 3.5.1

svn status:

M       includes/admin/plugins.php
M       load.php
M       readme.txt

svn diff

Index: includes/admin/plugins.php
===================================================================
--- includes/admin/plugins.php	(revision 3173122)
+++ includes/admin/plugins.php	(working copy)
@@ -19,7 +19,7 @@
  * @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 ) {
-	$transient_key = 'perflab_plugins_info';
+	$transient_key = 'perflab_plugins_info-v2';
 	$plugins       = get_transient( $transient_key );
 
 	if ( is_array( $plugins ) ) {
@@ -69,7 +69,10 @@
 	}
 
 	$plugins            = array();
-	$standalone_plugins = array_flip( perflab_get_standalone_plugins() );
+	$standalone_plugins = array_merge(
+		array_flip( perflab_get_standalone_plugins() ),
+		array( 'optimization-detective' => array() ) // TODO: Programmatically discover the plugin dependencies and add them here.
+	);
 	foreach ( $response->plugins as $plugin_data ) {
 		if ( ! isset( $standalone_plugins[ $plugin_data['slug'] ] ) ) {
 			continue;
Index: load.php
===================================================================
--- load.php	(revision 3173122)
+++ load.php	(working copy)
@@ -5,7 +5,7 @@
  * Description: Performance plugin from the WordPress Performance Team, which is a collection of standalone performance features.
  * Requires at least: 6.5
  * Requires PHP: 7.2
- * Version: 3.5.0
+ * Version: 3.5.1
  * Author: WordPress Performance Team
  * Author URI: https://make.wordpress.org/performance/
  * License: GPLv2 or later
@@ -19,7 +19,7 @@
 	exit; // Exit if accessed directly.
 }
 
-define( 'PERFLAB_VERSION', '3.5.0' );
+define( 'PERFLAB_VERSION', '3.5.1' );
 define( 'PERFLAB_MAIN_FILE', __FILE__ );
 define( 'PERFLAB_PLUGIN_DIR_PATH', plugin_dir_path( PERFLAB_MAIN_FILE ) );
 define( 'PERFLAB_SCREEN', 'performance-lab' );
Index: readme.txt
===================================================================
--- readme.txt	(revision 3173122)
+++ readme.txt	(working copy)
@@ -2,7 +2,7 @@
 
 Contributors: wordpressdotorg
 Tested up to: 6.7
-Stable tag:   3.5.0
+Stable tag:   3.5.1
 License:      GPLv2 or later
 License URI:  https://www.gnu.org/licenses/gpl-2.0.html
 Tags:         performance, site health, measurement, optimization, diagnostics
@@ -71,6 +71,12 @@
 
 == Changelog ==
 
+= 3.5.1 =
+
+**Bug Fixes**
+
+* Account for plugin dependencies when storing relevant plugin info. ([1613](https://github.com/WordPress/performance/pull/1613))
+
 = 3.5.0 =
 
 **Enhancements**

`speculation-rules`

Warning

Stable tag is unchanged at 1.3.1, so no plugin release will occur.

svn status:

M       class-plsr-url-pattern-prefixer.php
M       helper.php
M       hooks.php
M       load.php
M       readme.txt
M       settings.php

svn diff

Index: class-plsr-url-pattern-prefixer.php
===================================================================
--- class-plsr-url-pattern-prefixer.php	(revision 3173122)
+++ class-plsr-url-pattern-prefixer.php	(working copy)
@@ -35,7 +35,7 @@
 	 *                                        by the {@see PLSR_URL_Pattern_Prefixer::get_default_contexts()} method.
 	 */
 	public function __construct( array $contexts = array() ) {
-		if ( $contexts ) {
+		if ( count( $contexts ) > 0 ) {
 			$this->contexts = array_map(
 				static function ( string $str ): string {
 					return self::escape_pattern_string( trailingslashit( $str ) );
Index: helper.php
===================================================================
--- helper.php	(revision 3173122)
+++ helper.php	(working copy)
@@ -19,22 +19,11 @@
  *
  * @since 1.0.0
  *
- * @return array<string, array<int, array<string, mixed>>> Associative array of speculation rules by type.
+ * @return non-empty-array<string, array<int, array<string, mixed>>> Associative array of speculation rules by type.
  */
 function plsr_get_speculation_rules(): array {
-	$option = get_option( 'plsr_speculation_rules' );
-
-	/*
-	 * This logic is only relevant for edge-cases where the setting may not be registered,
-	 * a.k.a. defensive coding.
-	 */
-	if ( ! $option || ! is_array( $option ) ) {
-		$option = plsr_get_setting_default();
-	} else {
-		$option = array_merge( plsr_get_setting_default(), $option );
-	}
-
-	$mode      = (string) $option['mode'];
+	$option    = plsr_get_stored_setting_value();
+	$mode      = $option['mode'];
 	$eagerness = $option['eagerness'];
 
 	$prefixer = new PLSR_URL_Pattern_Prefixer();
Index: hooks.php
===================================================================
--- hooks.php	(revision 3173122)
+++ hooks.php	(working copy)
@@ -19,30 +19,10 @@
  * @since 1.0.0
  */
 function plsr_print_speculation_rules(): void {
-	$rules = plsr_get_speculation_rules();
-	if ( empty( $rules ) ) {
-		return;
-	}
-
-	// This workaround is needed for WP 6.4. See <https://core.trac.wordpress.org/ticket/60320>.
-	$needs_html5_workaround = (
-		! current_theme_supports( 'html5', 'script' ) &&
-		version_compare( (string) strtok( (string) get_bloginfo( 'version' ), '-' ), '6.4', '>=' ) &&
-		version_compare( (string) strtok( (string) get_bloginfo( 'version' ), '-' ), '6.5', '<' )
-	);
-	if ( $needs_html5_workaround ) {
-		$backup_wp_theme_features = $GLOBALS['_wp_theme_features'];
-		add_theme_support( 'html5', array( 'script' ) );
-	}
-
 	wp_print_inline_script_tag(
-		(string) wp_json_encode( $rules ),
+		(string) wp_json_encode( plsr_get_speculation_rules() ),
 		array( 'type' => 'speculationrules' )
 	);
-
-	if ( $needs_html5_workaround ) {
-		$GLOBALS['_wp_theme_features'] = $backup_wp_theme_features; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
-	}
 }
 add_action( 'wp_footer', 'plsr_print_speculation_rules' );
 
Index: load.php
===================================================================
--- load.php	(revision 3173122)
+++ load.php	(working copy)
@@ -3,7 +3,7 @@
  * Plugin Name: Speculative Loading
  * Plugin URI: https://github.com/WordPress/performance/tree/trunk/plugins/speculation-rules
  * Description: Enables browsers to speculatively prerender or prefetch pages when hovering over links.
- * Requires at least: 6.4
+ * Requires at least: 6.5
  * Requires PHP: 7.2
  * Version: 1.3.1
  * Author: WordPress Performance Team
Index: readme.txt
===================================================================
--- readme.txt	(revision 3173122)
+++ readme.txt	(working copy)
@@ -1,13 +1,11 @@
 === Speculative Loading ===
 
-Contributors:      wordpressdotorg
-Requires at least: 6.4
-Tested up to:      6.5
-Requires PHP:      7.2
-Stable tag:        1.3.1
-License:           GPLv2 or later
-License URI:       https://www.gnu.org/licenses/gpl-2.0.html
-Tags:              performance, javascript, speculation rules, prerender, prefetch
+Contributors: wordpressdotorg
+Tested up to: 6.7
+Stable tag:   1.3.1
+License:      GPLv2 or later
+License URI:  https://www.gnu.org/licenses/gpl-2.0.html
+Tags:         performance, javascript, speculation rules, prerender, prefetch
 
 Enables browsers to speculatively prerender or prefetch pages when hovering over links.
 
Index: settings.php
===================================================================
--- settings.php	(revision 3173122)
+++ settings.php	(working copy)
@@ -16,7 +16,7 @@
  *
  * @since 1.0.0
  *
- * @return array<string, string> Associative array of `$mode => $label` pairs.
+ * @return array{ prefetch: string, prerender: string } Associative array of `$mode => $label` pairs.
  */
 function plsr_get_mode_labels(): array {
 	return array(
@@ -30,7 +30,7 @@
  *
  * @since 1.0.0
  *
- * @return array<string, string> Associative array of `$eagerness => $label` pairs.
+ * @return array{ conservative: string, moderate: string, eager: string } Associative array of `$eagerness => $label` pairs.
  */
 function plsr_get_eagerness_labels(): array {
 	return array(
@@ -45,7 +45,7 @@
  *
  * @since 1.0.0
  *
- * @return array<string, string> {
+ * @return array{ mode: 'prerender', eagerness: 'moderate' } {
  *     Default setting value.
  *
  *     @type string $mode      Mode.
@@ -60,12 +60,29 @@
 }
 
 /**
+ * Returns the stored setting value for Speculative Loading configuration.
+ *
+ * @since n.e.x.t
+ *
+ * @return array{ mode: 'prefetch'|'prerender', eagerness: 'conservative'|'moderate'|'eager' } {
+ *     Stored setting value.
+ *
+ *     @type string $mode      Mode.
+ *     @type string $eagerness Eagerness.
+ * }
+ */
+function plsr_get_stored_setting_value(): array {
+	return plsr_sanitize_setting( get_option( 'plsr_speculation_rules' ) );
+}
+
+/**
  * Sanitizes the setting for Speculative Loading configuration.
  *
  * @since 1.0.0
+ * @todo  Consider whether the JSON schema for the setting could be reused here.
  *
  * @param mixed $input Setting to sanitize.
- * @return array<string, string> {
+ * @return array{ mode: 'prefetch'|'prerender', eagerness: 'conservative'|'moderate'|'eager' } {
  *     Sanitized setting.
  *
  *     @type string $mode      Mode.
@@ -79,17 +96,14 @@
 		return $default_value;
 	}
 
-	$mode_labels      = plsr_get_mode_labels();
-	$eagerness_labels = plsr_get_eagerness_labels();
-
 	// Ensure only valid keys are present.
-	$value = array_intersect_key( $input, $default_value );
+	$value = array_intersect_key( array_merge( $default_value, $input ), $default_value );
 
-	// Set any missing or invalid values to their defaults.
-	if ( ! isset( $value['mode'] ) || ! isset( $mode_labels[ $value['mode'] ] ) ) {
+	// Constrain values to what is allowed.
+	if ( ! in_array( $value['mode'], array_keys( plsr_get_mode_labels() ), true ) ) {
 		$value['mode'] = $default_value['mode'];
 	}
-	if ( ! isset( $value['eagerness'] ) || ! isset( $eagerness_labels[ $value['eagerness'] ] ) ) {
+	if ( ! in_array( $value['eagerness'], array_keys( plsr_get_eagerness_labels() ), true ) ) {
 		$value['eagerness'] = $default_value['eagerness'];
 	}
 
@@ -113,7 +127,8 @@
 			'default'           => plsr_get_setting_default(),
 			'show_in_rest'      => array(
 				'schema' => array(
-					'properties' => array(
+					'type'                 => 'object',
+					'properties'           => array(
 						'mode'      => array(
 							'description' => __( 'Whether to prefetch or prerender URLs.', 'speculation-rules' ),
 							'type'        => 'string',
@@ -125,6 +140,7 @@
 							'enum'        => array_keys( plsr_get_eagerness_labels() ),
 						),
 					),
+					'additionalProperties' => false,
 				),
 			),
 		)
@@ -188,7 +204,7 @@
  * @since 1.0.0
  * @access private
  *
- * @param array<string, string> $args {
+ * @param array{ field: 'mode'|'eagerness', title: non-empty-string, description: non-empty-string } $args {
  *     Associative array of arguments.
  *
  *     @type string $field       The slug of the sub setting controlled by the field.
@@ -197,28 +213,24 @@
  * }
  */
 function plsr_render_settings_field( array $args ): void {
-	if ( empty( $args['field'] ) || empty( $args['title'] ) ) { // Invalid.
-		return;
-	}
+	$option = plsr_get_stored_setting_value();
 
-	$option = get_option( 'plsr_speculation_rules' );
-	if ( ! isset( $option[ $args['field'] ] ) ) { // Invalid.
-		return;
+	switch ( $args['field'] ) {
+		case 'mode':
+			$choices = plsr_get_mode_labels();
+			break;
+		case 'eagerness':
+			$choices = plsr_get_eagerness_labels();
+			break;
+		default:
+			return; // Invalid (and this case should never occur).
 	}
 
-	$value    = $option[ $args['field'] ];
-	$callback = "plsr_get_{$args['field']}_labels";
-	if ( ! is_callable( $callback ) ) {
-		return;
-	}
-	$choices = call_user_func( $callback );
-
+	$value = $option[ $args['field'] ];
 	?>
 	<fieldset>
 		<legend class="screen-reader-text"><?php echo esc_html( $args['title'] ); ?></legend>
-		<?php
-		foreach ( $choices as $slug => $label ) {
-			?>
+		<?php foreach ( $choices as $slug => $label ) : ?>
 			<p>
 				<label>
 					<input
@@ -230,17 +242,11 @@
 					<?php echo esc_html( $label ); ?>
 				</label>
 			</p>
-			<?php
-		}
+		<?php endforeach; ?>
 
-		if ( ! empty( $args['description'] ) ) {
-			?>
-			<p class="description" style="max-width: 800px;">
-				<?php echo esc_html( $args['description'] ); ?>
-			</p>
-			<?php
-		}
-		?>
+		<p class="description" style="max-width: 800px;">
+			<?php echo esc_html( $args['description'] ); ?>
+		</p>
 	</fieldset>
 	<?php
 }

`web-worker-offloading`

Note

No changes.

`webp-uploads`

Warning

Stable tag is unchanged at 2.2.0, so no plugin release will occur.

svn status:

M       readme.txt

svn diff

Index: readme.txt
===================================================================
--- readme.txt	(revision 3173122)
+++ readme.txt	(working copy)
@@ -1,7 +1,7 @@
 === Modern Image Formats ===
 
 Contributors: wordpressdotorg
-Tested up to: 6.6
+Tested up to: 6.7
 Stable tag:   2.2.0
 License:      GPLv2 or later
 License URI:  https://www.gnu.org/licenses/gpl-2.0.html

felixarntz

Thanks @westonruter and everyone involved in fixing this so promptly in a hotfix!

While I understand how this resolves the problem, I wonder why optimization-detective is not part of the perflab_get_standalone_plugin_data() list in the first place. Did we not want to include it only because it's an infrastructure plugin?

felixarntz · 2024-10-21T20:12:53Z

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

-	$standalone_plugins = array_flip( perflab_get_standalone_plugins() );
+	$standalone_plugins = array_merge(
+		array_flip( perflab_get_standalone_plugins() ),
+		array( 'optimization-detective' => array() ) // TODO: Programmatically discover the plugin dependencies and add them here.


Is there an issue already for this?

westonruter · 2024-10-21T20:51:38Z

While I understand how this resolves the problem, I wonder why optimization-detective is not part of the perflab_get_standalone_plugin_data() list in the first place. Did we not want to include it only because it's an infrastructure plugin?

@felixarntz I recall it was to avoid having Optimization Detective as a separate feature listed on the Performance screen since it doesn't provide features on its own.

westonruter added the [Type] Bug An existing feature is broken label Oct 21, 2024

westonruter requested review from felixarntz and mukeshpanchal27 as code owners October 21, 2024 17:46

westonruter added this to the performance-lab 3.5.1 milestone Oct 21, 2024

Account for plugin dependencies when storing relevant plugin info

f385a43

westonruter force-pushed the fix/storing-standalone-plugin-info branch from bd14a92 to f385a43 Compare October 21, 2024 17:47

westonruter requested review from adamsilverstein, joemcgill and swissspidy October 21, 2024 17:47

westonruter commented Oct 21, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

Fix array structure

7e2768e

Co-authored-by: swissspidy <[email protected]>

swissspidy approved these changes Oct 21, 2024

View reviewed changes

westonruter added 2 commits October 21, 2024 11:04

Merge pull request #1611 from WordPress/release/3.5.0

cb1d472

Post 3.5.0 release merge

Merge branch 'trunk' of https://github.com/WordPress/performance into…

cf0d3f2

… fix/storing-standalone-plugin-info

adamsilverstein approved these changes Oct 21, 2024

View reviewed changes

westonruter mentioned this pull request Oct 21, 2024

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

Merged

westonruter merged commit 70ca5b5 into release/3.5.1 Oct 21, 2024
17 of 18 checks passed

westonruter deleted the fix/storing-standalone-plugin-info branch October 21, 2024 18:10

This was referenced Oct 21, 2024

Automatically discover plugin dependencies when obtaining Performance feature plugins from WordPress.org #1616

Closed

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

Closed

felixarntz reviewed Oct 21, 2024

View reviewed changes

westonruter mentioned this pull request Nov 13, 2024

Fix uses of 'Plugin not found' string #1651

Merged

Account for plugin dependencies when storing relevant plugin info #1613

Account for plugin dependencies when storing relevant plugin info #1613

Uh oh!

Conversation

westonruter commented Oct 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

westonruter commented Oct 21, 2024

Uh oh!

adamsilverstein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

westonruter commented Oct 21, 2024

auto-sizes

dominant-color-images

embed-optimizer

image-prioritizer

optimization-detective

performance-lab

speculation-rules

web-worker-offloading

webp-uploads

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter commented Oct 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

westonruter commented Oct 21, 2024 •

edited

Loading

github-actions bot commented Oct 21, 2024 •

edited

Loading

`auto-sizes`

`dominant-color-images`

`embed-optimizer`

`image-prioritizer`

`optimization-detective`

`performance-lab`

`speculation-rules`

`web-worker-offloading`

`webp-uploads`