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.

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

Account for plugin dependencies when storing relevant plugin info #1613

Merged
merged 4 commits into from
Oct 21, 2024

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented Oct 21, 2024

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.

@westonruter westonruter added the [Type] Bug An existing feature is broken label Oct 21, 2024
@westonruter westonruter added this to the performance-lab 3.5.1 milestone Oct 21, 2024
Copy link

github-actions bot commented Oct 21, 2024

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.

@@ -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';
Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed in #1617

@westonruter

This comment was marked as outdated.

Co-authored-by: swissspidy <[email protected]>
@westonruter
Copy link
Member Author

Updated build for testing: performance-lab.zip

Copy link
Member

@adamsilverstein adamsilverstein left a comment

Choose a reason for hiding this comment

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

Looks good, fixes the issue in my testing

@westonruter westonruter merged commit 70ca5b5 into release/3.5.1 Oct 21, 2024
17 of 18 checks passed
@westonruter westonruter deleted the fix/storing-standalone-plugin-info branch October 21, 2024 18:10
@westonruter
Copy link
Member Author

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

Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

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?

$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.
Copy link
Member

Choose a reason for hiding this comment

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

Is there an issue already for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes: #1616

@westonruter
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants