Merge pull request #1705 from ShyamGadde/add/mark-url-metrics-stale-on-tag-visitor-change

Mark existing URL Metrics as stale when a new tag visitor is registered

Author: westonruter

plugins/embed-optimizer/tests/test-optimization-detective.php

@@ -18,6 +18,12 @@ public function set_up(): void {
 		if ( ! defined( 'OPTIMIZATION_DETECTIVE_VERSION' ) ) {
 			$this->markTestSkipped( 'Optimization Detective is not active.' );
 		}
+
+		// Normalize the data for computing the current URL Metrics ETag to work around the issue where there is no
+		// global variable storing the OD_Tag_Visitor_Registry instance along with any registered tag visitors, so
+		// during set up we do not know what the ETag will look like. The current ETag is only established when
+		// the output begins to be processed by od_optimize_template_output_buffer().
+		add_filter( 'od_current_url_metrics_etag_data', '__return_empty_array' );
 	}
 
 	/**

plugins/image-prioritizer/tests/test-helper.php

@@ -10,6 +10,19 @@
 class Test_Image_Prioritizer_Helper extends WP_UnitTestCase {
 	use Optimization_Detective_Test_Helpers;
 
+	/**
+	 * Runs the routine before each test is executed.
+	 */
+	public function set_up(): void {
+		parent::set_up();
+
+		// Normalize the data for computing the current URL Metrics ETag to work around the issue where there is no
+		// global variable storing the OD_Tag_Visitor_Registry instance along with any registered tag visitors, so
+		// during set up we do not know what the ETag will look like. The current ETag is only established when
+		// the output begins to be processed by od_optimize_template_output_buffer().
+		add_filter( 'od_current_url_metrics_etag_data', '__return_empty_array' );
+	}
+
 	/**
 	 * @return array<string, array<string, mixed>>
 	 */

plugins/optimization-detective/class-od-url-metric-group-collection.php

@@ -35,6 +35,14 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega
 	 */
 	private $groups;
 
+	/**
+	 * The current ETag.
+	 *
+	 * @since n.e.x.t
+	 * @var non-empty-string
+	 */
+	private $current_etag;
+
 	/**
 	 * Breakpoints in max widths.
 	 *
@@ -93,12 +101,27 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega
 	 *
 	 * @throws InvalidArgumentException When an invalid argument is supplied.
 	 *
-	 * @param OD_URL_Metric[] $url_metrics   URL Metrics.
-	 * @param int[]           $breakpoints   Breakpoints in max widths.
-	 * @param int             $sample_size   Sample size for the maximum number of viewports in a group between breakpoints.
-	 * @param int             $freshness_ttl Freshness age (TTL) for a given URL Metric.
+	 * @param OD_URL_Metric[]  $url_metrics   URL Metrics.
+	 * @param non-empty-string $current_etag  The current ETag.
+	 * @param int[]            $breakpoints   Breakpoints in max widths.
+	 * @param int              $sample_size   Sample size for the maximum number of viewports in a group between breakpoints.
+	 * @param int              $freshness_ttl Freshness age (TTL) for a given URL Metric.
 	 */
-	public function __construct( array $url_metrics, array $breakpoints, int $sample_size, int $freshness_ttl ) {
+	public function __construct( array $url_metrics, string $current_etag, array $breakpoints, int $sample_size, int $freshness_ttl ) {
+		// Set current ETag.
+		if ( 1 !== preg_match( '/^[a-f0-9]{32}\z/', $current_etag ) ) {
+			throw new InvalidArgumentException(
+				esc_html(
+					sprintf(
+						/* translators: %s is the invalid ETag */
+						__( 'The current ETag must be a valid MD5 hash, but provided: %s', 'optimization-detective' ),
+						$current_etag
+					)
+				)
+			);
+		}
+		$this->current_etag = $current_etag;
+
 		// Set breakpoints.
 		sort( $breakpoints );
 		$breakpoints = array_values( array_unique( $breakpoints, SORT_NUMERIC ) );
@@ -160,6 +183,17 @@ public function __construct( array $url_metrics, array $breakpoints, int $sample
 		}
 	}
 
+	/**
+	 * Gets the current ETag.
+	 *
+	 * @since n.e.x.t
+	 *
+	 * @return non-empty-string Current ETag.
+	 */
+	public function get_current_etag(): string {
+		return $this->current_etag;
+	}
+
 	/**
 	 * Gets the first URL Metric group.
 	 *
@@ -613,6 +647,7 @@ public function count(): int {
 	 * @since 0.3.1
 	 *
 	 * @return array{
+	 *             current_etag: non-empty-string,
 	 *             breakpoints: positive-int[],
 	 *             freshness_ttl: 0|positive-int,
 	 *             sample_size: positive-int,
@@ -631,6 +666,7 @@ public function count(): int {
 	 */
 	public function jsonSerialize(): array {
 		return array(
+			'current_etag'                        => $this->current_etag,
 			'breakpoints'                         => $this->breakpoints,
 			'freshness_ttl'                       => $this->freshness_ttl,
 			'sample_size'                         => $this->sample_size,

plugins/optimization-detective/class-od-url-metric-group.php

@@ -63,7 +63,7 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer
 	/**
 	 * Collection that this instance belongs to.
 	 *
-	 * @var OD_URL_Metric_Group_Collection|null
+	 * @var OD_URL_Metric_Group_Collection
 	 */
 	private $collection;
 
@@ -82,16 +82,19 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer
 	/**
 	 * Constructor.
 	 *
+	 * This class should never be directly constructed. It should only be constructed by the {@see OD_URL_Metric_Group_Collection::create_groups()}.
+	 *
+	 * @access private
 	 * @throws InvalidArgumentException If arguments are invalid.
 	 *
-	 * @param OD_URL_Metric[]                     $url_metrics            URL Metrics to add to the group.
-	 * @param int                                 $minimum_viewport_width Minimum possible viewport width for the group. Must be zero or greater.
-	 * @param int                                 $maximum_viewport_width Maximum possible viewport width for the group. Must be greater than zero and the minimum viewport width.
-	 * @param int                                 $sample_size            Sample size for the maximum number of viewports in a group between breakpoints.
-	 * @param int                                 $freshness_ttl          Freshness age (TTL) for a given URL Metric.
-	 * @param OD_URL_Metric_Group_Collection|null $collection             Collection that this instance belongs to. Optional.
+	 * @param OD_URL_Metric[]                $url_metrics            URL Metrics to add to the group.
+	 * @param int                            $minimum_viewport_width Minimum possible viewport width for the group. Must be zero or greater.
+	 * @param int                            $maximum_viewport_width Maximum possible viewport width for the group. Must be greater than zero and the minimum viewport width.
+	 * @param int                            $sample_size            Sample size for the maximum number of viewports in a group between breakpoints.
+	 * @param int                            $freshness_ttl          Freshness age (TTL) for a given URL Metric.
+	 * @param OD_URL_Metric_Group_Collection $collection             Collection that this instance belongs to.
 	 */
-	public function __construct( array $url_metrics, int $minimum_viewport_width, int $maximum_viewport_width, int $sample_size, int $freshness_ttl, ?OD_URL_Metric_Group_Collection $collection = null ) {
+	public function __construct( array $url_metrics, int $minimum_viewport_width, int $maximum_viewport_width, int $sample_size, int $freshness_ttl, OD_URL_Metric_Group_Collection $collection ) {
 		if ( $minimum_viewport_width < 0 ) {
 			throw new InvalidArgumentException(
 				esc_html__( 'The minimum viewport width must be at least zero.', 'optimization-detective' )
@@ -135,12 +138,8 @@ public function __construct( array $url_metrics, int $minimum_viewport_width, in
 			);
 		}
 		$this->freshness_ttl = $freshness_ttl;
-
-		if ( ! is_null( $collection ) ) {
-			$this->collection = $collection;
-		}
-
-		$this->url_metrics = $url_metrics;
+		$this->collection    = $collection;
+		$this->url_metrics   = $url_metrics;
 	}
 
 	/**
@@ -191,9 +190,7 @@ public function add_url_metric( OD_URL_Metric $url_metric ): void {
 		}
 
 		$this->result_cache = array();
-		if ( ! is_null( $this->collection ) ) {
-			$this->collection->clear_cache();
-		}
+		$this->collection->clear_cache();
 
 		$url_metric->set_group( $this );
 		$this->url_metrics[] = $url_metric;
@@ -220,6 +217,8 @@ static function ( OD_URL_Metric $a, OD_URL_Metric $b ): int {
 	 * A group is complete if it has the full sample size of URL Metrics
 	 * and all of these URL Metrics are fresh.
 	 *
+	 * @since n.e.x.t If the current environment's generated ETag does not match the URL Metric's ETag, the URL Metric is considered stale.
+	 *
 	 * @return bool Whether complete.
 	 */
 	public function is_complete(): bool {
@@ -233,9 +232,20 @@ public function is_complete(): bool {
 			}
 			$current_time = microtime( true );
 			foreach ( $this->url_metrics as $url_metric ) {
+				// The URL Metric is too old to be fresh.
 				if ( $current_time > $url_metric->get_timestamp() + $this->freshness_ttl ) {
 					return false;
 				}
+
+				// The ETag is not populated yet, so this is stale. Eventually this will be required.
+				if ( $url_metric->get_etag() === null ) {
+					return false;
+				}
+
+				// The ETag of the URL Metric does not match the current ETag for the collection, so it is stale.
+				if ( ! hash_equals( $url_metric->get_etag(), $this->collection->get_current_etag() ) ) {
+					return false;
+				}
 			}
 
 			return true;

plugins/optimization-detective/class-od-url-metric.php

@@ -38,6 +38,7 @@
  *                            }
  * @phpstan-type Data         array{
  *                                uuid: non-empty-string,
+ *                                etag?: non-empty-string,
  *                                url: non-empty-string,
  *                                timestamp: float,
  *                                viewport: ViewportRect,
@@ -155,6 +156,7 @@ public function set_group( OD_URL_Metric_Group $group ): void {
 	 * Gets JSON schema for URL Metric.
 	 *
 	 * @since 0.1.0
+	 * @since n.e.x.t Added the 'etag' property to the schema.
 	 *
 	 * @todo Cache the return value?
 	 *
@@ -208,6 +210,15 @@ public static function get_json_schema(): array {
 					'required'    => true,
 					'readonly'    => true, // Omit from REST API.
 				),
+				'etag'      => array(
+					'description' => __( 'The ETag for the URL Metric.', 'optimization-detective' ),
+					'type'        => 'string',
+					'pattern'     => '^[0-9a-f]{32}\z',
+					'minLength'   => 32,
+					'maxLength'   => 32,
+					'required'    => false, // To be made required in a future release.
+					'readonly'    => true, // Omit from REST API.
+				),
 				'url'       => array(
 					'description' => __( 'The URL for which the metric was obtained.', 'optimization-detective' ),
 					'type'        => 'string',
@@ -309,7 +320,7 @@ public static function get_json_schema(): array {
 			$schema['properties']['elements']['items']['properties'] = self::extend_schema_with_optional_properties(
 				$schema['properties']['elements']['items']['properties'],
 				$additional_properties,
-				'od_url_metric_schema_root_additional_properties'
+				'od_url_metric_schema_element_item_additional_properties'
 			);
 		}
 
@@ -417,6 +428,18 @@ public function get_uuid(): string {
 		return $this->data['uuid'];
 	}
 
+	/**
+	 * Gets ETag.
+	 *
+	 * @since n.e.x.t
+	 *
+	 * @return non-empty-string|null ETag.
+	 */
+	public function get_etag(): ?string {
+		// Since the ETag is optional for now, return null for old URL Metrics that do not have one.
+		return $this->data['etag'] ?? null;
+	}
+
 	/**
 	 * Gets URL.
 	 *

plugins/optimization-detective/detect.js

@@ -237,6 +237,7 @@ function extendElementData( xpath, properties ) {
  * @param {number}                 args.maxViewportAspectRatio     Maximum aspect ratio allowed for the viewport.
  * @param {boolean}                args.isDebug                    Whether to show debug messages.
  * @param {string}                 args.restApiEndpoint            URL for where to send the detection data.
+ * @param {string}                 args.currentETag                Current ETag.
  * @param {string}                 args.currentUrl                 Current URL.
  * @param {string}                 args.urlMetricSlug              Slug for URL Metric.
  * @param {number|null}            args.cachePurgePostId           Cache purge post ID.
@@ -252,6 +253,7 @@ export default async function detect( {
 	isDebug,
 	extensionModuleUrls,
 	restApiEndpoint,
+	currentETag,
 	currentUrl,
 	urlMetricSlug,
 	cachePurgePostId,
@@ -539,6 +541,7 @@ export default async function detect( {
 
 	const url = new URL( restApiEndpoint );
 	url.searchParams.set( 'slug', urlMetricSlug );
+	url.searchParams.set( 'current_etag', currentETag );
 	if ( typeof cachePurgePostId === 'number' ) {
 		url.searchParams.set(
 			'cache_purge_post_id',

plugins/optimization-detective/detection.php

@@ -85,16 +85,20 @@ function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $
 	$cache_purge_post_id = od_get_cache_purge_post_id();
 
 	$current_url = od_get_current_url();
+
+	$current_etag = $group_collection->get_current_etag();
+
 	$detect_args = array(
 		'minViewportAspectRatio' => od_get_minimum_viewport_aspect_ratio(),
 		'maxViewportAspectRatio' => od_get_maximum_viewport_aspect_ratio(),
 		'isDebug'                => WP_DEBUG,
 		'extensionModuleUrls'    => $extension_module_urls,
 		'restApiEndpoint'        => rest_url( OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ),
+		'currentETag'            => $current_etag,
 		'currentUrl'             => $current_url,
 		'urlMetricSlug'          => $slug,
 		'cachePurgePostId'       => od_get_cache_purge_post_id(),
-		'urlMetricHMAC'          => od_get_url_metrics_storage_hmac( $slug, $current_url, $cache_purge_post_id ),
+		'urlMetricHMAC'          => od_get_url_metrics_storage_hmac( $slug, $current_etag, $current_url, $cache_purge_post_id ),
 		'urlMetricGroupStatuses' => array_map(
 			static function ( OD_URL_Metric_Group $group ): array {
 				return array(

plugins/optimization-detective/optimization.php

@@ -195,16 +195,6 @@ function od_optimize_template_output_buffer( string $buffer ): string {
 	$slug = od_get_url_metrics_slug( od_get_normalized_query_vars() );
 	$post = OD_URL_Metrics_Post_Type::get_post( $slug );
 
-	$group_collection = new OD_URL_Metric_Group_Collection(
-		$post instanceof WP_Post ? OD_URL_Metrics_Post_Type::get_url_metrics_from_post( $post ) : array(),
-		od_get_breakpoint_max_widths(),
-		od_get_url_metrics_breakpoint_sample_size(),
-		od_get_url_metric_freshness_ttl()
-	);
-
-	// Whether we need to add the data-od-xpath attribute to elements and whether the detection script should be injected.
-	$needs_detection = ! $group_collection->is_every_group_complete();
-
 	$tag_visitor_registry = new OD_Tag_Visitor_Registry();
 
 	/**
@@ -216,10 +206,22 @@ function od_optimize_template_output_buffer( string $buffer ): string {
 	 */
 	do_action( 'od_register_tag_visitors', $tag_visitor_registry );
 
+	$current_etag         = od_get_current_url_metrics_etag( $tag_visitor_registry );
+	$group_collection     = new OD_URL_Metric_Group_Collection(
+		$post instanceof WP_Post ? OD_URL_Metrics_Post_Type::get_url_metrics_from_post( $post ) : array(),
+		$current_etag,
+		od_get_breakpoint_max_widths(),
+		od_get_url_metrics_breakpoint_sample_size(),
+		od_get_url_metric_freshness_ttl()
+	);
 	$link_collection      = new OD_Link_Collection();
 	$tag_visitor_context  = new OD_Tag_Visitor_Context( $processor, $group_collection, $link_collection );
 	$current_tag_bookmark = 'optimization_detective_current_tag';
 	$visitors             = iterator_to_array( $tag_visitor_registry );
+
+	// Whether we need to add the data-od-xpath attribute to elements and whether the detection script should be injected.
+	$needs_detection = ! $group_collection->is_every_group_complete();
+
 	do {
 		$tracked_in_url_metrics = false;
 		$processor->set_bookmark( $current_tag_bookmark ); // TODO: Should we break if this returns false?

plugins/optimization-detective/readme.txt

@@ -238,6 +238,14 @@ add_filter(
 
 See also [example usage](https://github.com/WordPress/performance/blob/6bb8405c5c446e3b66c2bfa3ae03ba61b188bca2/plugins/embed-optimizer/hooks.php#L128-L144) in Embed Optimizer. Note in particular the structure of the plugin’s [detect.js](https://github.com/WordPress/performance/blob/trunk/plugins/embed-optimizer/detect.js) script module, how it exports `initialize` and `finalize` functions which Optimization Detective then calls when the page loads and when the page unloads, at which time the URL Metric is constructed and sent to the server for storage. Refer also to the [TypeScript type definitions](https://github.com/WordPress/performance/blob/trunk/plugins/optimization-detective/types.ts).
 
+**Filter:** `od_current_url_metrics_etag_data` (default: array with `tag_visitors` key)
+
+Filters the data that goes into computing the current ETag for URL Metrics.
+
+The ETag is a unique identifier that changes whenever the underlying data used to generate it changes. By default, the ETag calculation includes the names of registered tag visitors. This ensures that when a new Optimization Detective-dependent plugin is activated (like Image Prioritizer or Embed Optimizer), any existing URL Metrics are immediately considered stale. This happens because the newly registered tag visitors alter the ETag calculation, making it different from the stored ones.
+
+When the ETag for URL Metrics in a complete viewport group no longer matches the current environment's ETag, new URL Metrics will then begin to be collected until there are no more stored URL Metrics with the old ETag. These new URL Metrics will include data relevant to the newly activated plugins and their tag visitors.
+
 **Action:** `od_url_metric_stored` (argument: `OD_URL_Metric_Store_Request_Context`)
 
 Fires whenever a URL Metric was successfully stored.

plugins/optimization-detective/storage/class-od-url-metrics-post-type.php

@@ -217,8 +217,18 @@ public static function store_url_metric( string $slug, OD_URL_Metric $new_url_me
 			$url_metrics            = array();
 		}
 
+		$etag = $new_url_metric->get_etag();
+		if ( null === $etag ) {
+			// This case actually will never occur in practice because the store_url_metric function is only called
+			// in the REST API endpoint where the ETag parameter is required. It is here exclusively for the sake of
+			// PHPStan's static analysis. This entire condition can be removed in a future release when the 'etag'
+			// property becomes required.
+			return new WP_Error( 'missing_etag' );
+		}
+
 		$group_collection = new OD_URL_Metric_Group_Collection(
 			$url_metrics,
+			$etag,
 			od_get_breakpoint_max_widths(),
 			od_get_url_metrics_breakpoint_sample_size(),
 			od_get_url_metric_freshness_ttl()