diff --git a/plugins/embed-optimizer/tests/test-optimization-detective.php b/plugins/embed-optimizer/tests/test-optimization-detective.php index 0355864f66..9994859e1d 100644 --- a/plugins/embed-optimizer/tests/test-optimization-detective.php +++ b/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' ); } /** diff --git a/plugins/image-prioritizer/tests/test-helper.php b/plugins/image-prioritizer/tests/test-helper.php index bd646ab98a..b6054fbb17 100644 --- a/plugins/image-prioritizer/tests/test-helper.php +++ b/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> */ diff --git a/plugins/optimization-detective/class-od-url-metric-group-collection.php b/plugins/optimization-detective/class-od-url-metric-group-collection.php index 0832360cb9..9b50846eb6 100644 --- a/plugins/optimization-detective/class-od-url-metric-group-collection.php +++ b/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, diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 963f5b8840..9f026f9ff0 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/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; diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index d2f772d213..d3b0d984f7 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/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. * diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index e5e6cc8ab3..385f81b0d5 100644 --- a/plugins/optimization-detective/detect.js +++ b/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', diff --git a/plugins/optimization-detective/detection.php b/plugins/optimization-detective/detection.php index 32e248ae26..2fa2a6dee7 100644 --- a/plugins/optimization-detective/detection.php +++ b/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( diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index 9fd32e1ee0..8e5e524744 100644 --- a/plugins/optimization-detective/optimization.php +++ b/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? diff --git a/plugins/optimization-detective/readme.txt b/plugins/optimization-detective/readme.txt index 0d5e0cd246..dbe6afd220 100644 --- a/plugins/optimization-detective/readme.txt +++ b/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. diff --git a/plugins/optimization-detective/storage/class-od-url-metrics-post-type.php b/plugins/optimization-detective/storage/class-od-url-metrics-post-type.php index e55028a328..814abaac94 100644 --- a/plugins/optimization-detective/storage/class-od-url-metrics-post-type.php +++ b/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() diff --git a/plugins/optimization-detective/storage/data.php b/plugins/optimization-detective/storage/data.php index a53569301b..a773ff6f1f 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -140,25 +140,56 @@ function od_get_url_metrics_slug( array $query_vars ): string { return md5( (string) wp_json_encode( $query_vars ) ); } +/** + * Gets the current ETag for URL Metrics. + * + * The ETag is a hash based on the IDs of the registered tag visitors + * in the current environment. It is used for marking the URL Metrics as stale + * when its value changes. + * + * @since n.e.x.t + * @access private + * + * @param OD_Tag_Visitor_Registry $tag_visitor_registry Tag visitor registry. + * @return non-empty-string Current ETag. + */ +function od_get_current_url_metrics_etag( OD_Tag_Visitor_Registry $tag_visitor_registry ): string { + $data = array( + 'tag_visitors' => array_keys( iterator_to_array( $tag_visitor_registry ) ), + ); + + /** + * Filters the data that goes into computing the current ETag for URL Metrics. + * + * @since n.e.x.t + * + * @param array $data Data. + */ + $data = (array) apply_filters( 'od_current_url_metrics_etag_data', $data ); + + return md5( (string) wp_json_encode( $data ) ); +} + /** * Computes HMAC for storing URL Metrics for a specific slug. * * This is used in the REST API to authenticate the storage of new URL Metrics from a given URL. * * @since 0.8.0 + * @since n.e.x.t Introduced the `$current_etag` parameter. * @access private * * @see od_verify_url_metrics_storage_hmac() * @see od_get_url_metrics_slug() - * @todo This should also include an ETag as a parameter. See . * - * @param string $slug Slug (hash of normalized query vars). - * @param string $url URL. - * @param int|null $cache_purge_post_id Cache purge post ID. + * @param string $slug Slug (hash of normalized query vars). + * @param non-empty-string $current_etag Current ETag. + * @param string $url URL. + * @param int|null $cache_purge_post_id Cache purge post ID. * @return string HMAC. */ -function od_get_url_metrics_storage_hmac( string $slug, string $url, ?int $cache_purge_post_id = null ): string { - $action = "store_url_metric:$slug:$url:$cache_purge_post_id"; +function od_get_url_metrics_storage_hmac( string $slug, string $current_etag, string $url, ?int $cache_purge_post_id = null ): string { + $action = "store_url_metric:$slug:$current_etag:$url:$cache_purge_post_id"; return wp_hash( $action, 'nonce' ); } @@ -166,19 +197,21 @@ function od_get_url_metrics_storage_hmac( string $slug, string $url, ?int $cache * Verifies HMAC for storing URL Metrics for a specific slug. * * @since 0.8.0 + * @since n.e.x.t Introduced the `$current_etag` parameter. * @access private * * @see od_get_url_metrics_storage_hmac() * @see od_get_url_metrics_slug() * - * @param string $hmac HMAC. - * @param string $slug Slug (hash of normalized query vars). - * @param String $url URL. - * @param int|null $cache_purge_post_id Cache purge post ID. + * @param string $hmac HMAC. + * @param string $slug Slug (hash of normalized query vars). + * @param non-empty-string $current_etag Current ETag. + * @param string $url URL. + * @param int|null $cache_purge_post_id Cache purge post ID. * @return bool Whether the HMAC is valid. */ -function od_verify_url_metrics_storage_hmac( string $hmac, string $slug, string $url, ?int $cache_purge_post_id = null ): bool { - return hash_equals( od_get_url_metrics_storage_hmac( $slug, $url, $cache_purge_post_id ), $hmac ); +function od_verify_url_metrics_storage_hmac( string $hmac, string $slug, string $current_etag, string $url, ?int $cache_purge_post_id = null ): bool { + return hash_equals( od_get_url_metrics_storage_hmac( $slug, $current_etag, $url, $cache_purge_post_id ), $hmac ); } /** diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index fe622be468..4de890f960 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -44,7 +44,17 @@ function od_register_endpoint(): void { 'type' => 'string', 'description' => __( 'An MD5 hash of the query args.', 'optimization-detective' ), 'required' => true, - 'pattern' => '^[0-9a-f]{32}$', + 'pattern' => '^[0-9a-f]{32}\z', + 'minLength' => 32, + 'maxLength' => 32, + ), + 'current_etag' => array( + 'type' => 'string', + 'description' => __( 'ETag for the current environment.', 'optimization-detective' ), + 'required' => true, + 'pattern' => '^[0-9a-f]{32}\z', + 'minLength' => 32, + 'maxLength' => 32, ), 'cache_purge_post_id' => array( 'type' => 'integer', @@ -56,9 +66,9 @@ function od_register_endpoint(): void { 'type' => 'string', 'description' => __( 'HMAC originally computed by server required to authorize the request.', 'optimization-detective' ), 'required' => true, - 'pattern' => '^[0-9a-f]+$', + 'pattern' => '^[0-9a-f]+\z', 'validate_callback' => static function ( string $hmac, WP_REST_Request $request ) { - if ( ! od_verify_url_metrics_storage_hmac( $hmac, $request['slug'], $request['url'], $request['cache_purge_post_id'] ?? null ) ) { + if ( ! od_verify_url_metrics_storage_hmac( $hmac, $request['slug'], $request['current_etag'], $request['url'], $request['cache_purge_post_id'] ?? null ) ) { return new WP_Error( 'invalid_hmac', __( 'URL Metrics HMAC verification failure.', 'optimization-detective' ) ); } return true; @@ -141,6 +151,7 @@ function od_handle_rest_request( WP_REST_Request $request ) { $url_metric_group_collection = new OD_URL_Metric_Group_Collection( $post instanceof WP_Post ? OD_URL_Metrics_Post_Type::get_url_metrics_from_post( $post ) : array(), + $request->get_param( 'current_etag' ), od_get_breakpoint_max_widths(), od_get_url_metrics_breakpoint_sample_size(), od_get_url_metric_freshness_ttl() @@ -182,6 +193,7 @@ function od_handle_rest_request( WP_REST_Request $request ) { // Now supply the readonly args which were omitted from the REST API params due to being `readonly`. 'timestamp' => microtime( true ), 'uuid' => wp_generate_uuid4(), + 'etag' => $request->get_param( 'current_etag' ), ) ) ); diff --git a/plugins/optimization-detective/tests/storage/test-data.php b/plugins/optimization-detective/tests/storage/test-data.php index 2e6e3c17c2..c90df29ef2 100644 --- a/plugins/optimization-detective/tests/storage/test-data.php +++ b/plugins/optimization-detective/tests/storage/test-data.php @@ -282,10 +282,62 @@ public function test_od_get_url_metrics_slug(): void { $second = od_get_url_metrics_slug( array( 'p' => 1 ) ); $this->assertNotEquals( $second, $first ); foreach ( array( $first, $second ) as $slug ) { - $this->assertMatchesRegularExpression( '/^[0-9a-f]{32}$/', $slug ); + $this->assertMatchesRegularExpression( '/^[0-9a-f]{32}\z/', $slug ); } } + /** + * Test od_get_current_url_metrics_etag(). + * + * @covers ::od_get_current_url_metrics_etag + */ + public function test_od_get_current_url_metrics_etag(): void { + remove_all_filters( 'od_current_url_metrics_etag_data' ); + $registry = new OD_Tag_Visitor_Registry(); + + $captured_etag_data = array(); + add_filter( + 'od_current_url_metrics_etag_data', + static function ( array $data ) use ( &$captured_etag_data ) { + $captured_etag_data[] = $data; + return $data; + }, + PHP_INT_MAX + ); + $etag1 = od_get_current_url_metrics_etag( $registry ); + $this->assertMatchesRegularExpression( '/^[a-z0-9]{32}\z/', $etag1 ); + $etag2 = od_get_current_url_metrics_etag( $registry ); + $this->assertSame( $etag1, $etag2 ); + $this->assertCount( 2, $captured_etag_data ); + $this->assertSame( array( 'tag_visitors' => array() ), $captured_etag_data[0] ); + $this->assertSame( $captured_etag_data[ count( $captured_etag_data ) - 2 ], $captured_etag_data[ count( $captured_etag_data ) - 1 ] ); + + $registry->register( 'foo', static function (): void {} ); + $registry->register( 'bar', static function (): void {} ); + $registry->register( 'baz', static function (): void {} ); + $etag3 = od_get_current_url_metrics_etag( $registry ); + $this->assertNotEquals( $etag2, $etag3 ); + $this->assertNotEquals( $captured_etag_data[ count( $captured_etag_data ) - 2 ], $captured_etag_data[ count( $captured_etag_data ) - 1 ] ); + $this->assertSame( array( 'tag_visitors' => array( 'foo', 'bar', 'baz' ) ), $captured_etag_data[ count( $captured_etag_data ) - 1 ] ); + add_filter( + 'od_current_url_metrics_etag_data', + static function ( $data ): array { + $data['last_modified'] = '2024-03-02T01:00:00'; + return $data; + } + ); + $etag4 = od_get_current_url_metrics_etag( $registry ); + $this->assertNotEquals( $etag3, $etag4 ); + $this->assertNotEquals( $captured_etag_data[ count( $captured_etag_data ) - 2 ], $captured_etag_data[ count( $captured_etag_data ) - 1 ] ); + $this->assertSame( + array( + 'tag_visitors' => array( 'foo', 'bar', 'baz' ), + 'last_modified' => '2024-03-02T01:00:00', + ), + $captured_etag_data[ count( $captured_etag_data ) - 1 ] + ); + } + /** * Data provider. * @@ -328,7 +380,7 @@ public function test_od_get_url_metrics_storage_hmac_and_od_verify_url_metrics_s list( $url, $slug, $cache_purge_post_id ) = $set_up(); $this->go_to( $url ); $hmac = od_get_url_metrics_storage_hmac( $slug, $url, $cache_purge_post_id ); - $this->assertMatchesRegularExpression( '/^[0-9a-f]+$/', $hmac ); + $this->assertMatchesRegularExpression( '/^[0-9a-f]+\z/', $hmac ); $this->assertTrue( od_verify_url_metrics_storage_hmac( $hmac, $slug, $url, $cache_purge_post_id ) ); } diff --git a/plugins/optimization-detective/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index 1d112b4337..ad514aa487 100644 --- a/plugins/optimization-detective/tests/storage/test-rest-api.php +++ b/plugins/optimization-detective/tests/storage/test-rest-api.php @@ -57,7 +57,7 @@ static function ( array $properties ): array { $params['cache_purge_post_id'] = self::factory()->post->create(); $params['url'] = get_permalink( $params['cache_purge_post_id'] ); $params['slug'] = od_get_url_metrics_slug( array( 'p' => $params['cache_purge_post_id'] ) ); - $params['hmac'] = od_get_url_metrics_storage_hmac( $params['slug'], $params['url'], $params['cache_purge_post_id'] ); + $params['hmac'] = od_get_url_metrics_storage_hmac( $params['slug'], $params['current_etag'], $params['url'], $params['cache_purge_post_id'] ); return $params; }, ), @@ -111,7 +111,7 @@ function ( OD_URL_Metric_Store_Request_Context $context ) use ( &$stored_context $this->assertSame( $valid_params['viewport']['width'], $url_metrics[0]->get_viewport_width() ); $expected_data = $valid_params; - unset( $expected_data['hmac'], $expected_data['slug'], $expected_data['cache_purge_post_id'] ); + unset( $expected_data['hmac'], $expected_data['slug'], $expected_data['current_etag'], $expected_data['cache_purge_post_id'] ); $this->assertSame( $expected_data, wp_array_slice_assoc( $url_metrics[0]->jsonSerialize(), array_keys( $expected_data ) ) @@ -137,18 +137,25 @@ function ( OD_URL_Metric_Store_Request_Context $context ) use ( &$stored_context * @return array Test data. */ public function data_provider_invalid_params(): array { - $valid_element = $this->get_valid_params()['elements'][0]; + $valid_params = $this->get_valid_params(); + $valid_element = $valid_params['elements'][0]; return array_map( - function ( $params ) { + static function ( $params ) use ( $valid_params ) { return array( - 'params' => array_merge( $this->get_valid_params(), $params ), + 'params' => array_merge( $valid_params, $params ), ); }, array( 'bad_url' => array( 'url' => 'bad://url', ), + 'bad_current_etag1' => array( + 'current_etag' => 'foo', + ), + 'bad_current_etag2' => array( + 'current_etag' => $valid_params['current_etag'] . "\n", + ), 'bad_slug' => array( 'slug' => '', ), @@ -156,10 +163,10 @@ function ( $params ) { 'hmac' => 'not even a hash', ), 'invalid_hmac' => array( - 'hmac' => od_get_url_metrics_storage_hmac( od_get_url_metrics_slug( array( 'different' => 'query vars' ) ), home_url( '/' ) ), + 'hmac' => od_get_url_metrics_storage_hmac( od_get_url_metrics_slug( array( 'different' => 'query vars' ) ), $valid_params['current_etag'], home_url( '/' ) ), ), 'invalid_hmac_with_queried_object' => array( - 'hmac' => od_get_url_metrics_storage_hmac( od_get_url_metrics_slug( array() ), home_url( '/' ), 1 ), + 'hmac' => od_get_url_metrics_storage_hmac( od_get_url_metrics_slug( array() ), $valid_params['current_etag'], home_url( '/' ), 1 ), ), 'invalid_viewport_type' => array( 'viewport' => '640x480', @@ -525,6 +532,7 @@ static function () use ( $breakpoint_width ): array { // Sanity check that the groups were constructed as expected. $group_collection = new OD_URL_Metric_Group_Collection( OD_URL_Metrics_Post_Type::get_url_metrics_from_post( OD_URL_Metrics_Post_Type::get_post( od_get_url_metrics_slug( array() ) ) ), + $wider_viewport_params['current_etag'], od_get_breakpoint_max_widths(), od_get_url_metrics_breakpoint_sample_size(), HOUR_IN_SECONDS @@ -668,14 +676,15 @@ private function get_valid_params( array $extras = array() ): array { ), ) )->jsonSerialize(); - unset( $data['timestamp'], $data['uuid'] ); // Since these are readonly. $data = array_merge( array( - 'slug' => $slug, - 'hmac' => od_get_url_metrics_storage_hmac( $slug, $data['url'] ), + 'slug' => $slug, + 'hmac' => od_get_url_metrics_storage_hmac( $slug, $data['etag'], $data['url'] ), + 'current_etag' => $data['etag'], ), $data ); + unset( $data['timestamp'], $data['uuid'], $data['etag'] ); // Since these are readonly. if ( count( $extras ) > 0 ) { $data = $this->recursive_merge( $data, $extras ); } @@ -720,9 +729,9 @@ private function create_request( array $params ): WP_REST_Request { */ $request = new WP_REST_Request( 'POST', self::ROUTE ); $request->set_header( 'Content-Type', 'application/json' ); - $request->set_query_params( wp_array_slice_assoc( $params, array( 'hmac', 'slug', 'cache_purge_post_id' ) ) ); + $request->set_query_params( wp_array_slice_assoc( $params, array( 'hmac', 'current_etag', 'slug', 'cache_purge_post_id' ) ) ); $request->set_header( 'Origin', home_url() ); - unset( $params['hmac'], $params['slug'], $params['cache_purge_post_id'] ); + unset( $params['hmac'], $params['slug'], $params['current_etag'], $params['cache_purge_post_id'] ); $request->set_body( wp_json_encode( $params ) ); return $request; } diff --git a/plugins/optimization-detective/tests/test-cases/complete-url-metrics.php b/plugins/optimization-detective/tests/test-cases/complete-url-metrics.php index 237f183081..95dc5ec95d 100644 --- a/plugins/optimization-detective/tests/test-cases/complete-url-metrics.php +++ b/plugins/optimization-detective/tests/test-cases/complete-url-metrics.php @@ -3,6 +3,12 @@ 'set_up' => static function ( Test_OD_Optimization $test_case ): void { ini_set( 'default_mimetype', 'text/html; charset=utf-8' ); // phpcs:ignore WordPress.PHP.IniSet.Risky + // 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' ); + $test_case->populate_url_metrics( array( array( diff --git a/plugins/optimization-detective/tests/test-class-od-element.php b/plugins/optimization-detective/tests/test-class-od-element.php index 43a11e378a..52a4cc43ff 100644 --- a/plugins/optimization-detective/tests/test-class-od-element.php +++ b/plugins/optimization-detective/tests/test-class-od-element.php @@ -71,7 +71,8 @@ static function ( array $schema ): array { $this->assertInstanceOf( OD_Element::class, $element ); $this->assertSame( $url_metric, $element->get_url_metric() ); $this->assertNull( $element->get_url_metric_group() ); - $collection = new OD_URL_Metric_Group_Collection( array( $url_metric ), array(), 1, DAY_IN_SECONDS ); + $current_etag = md5( '' ); + $collection = new OD_URL_Metric_Group_Collection( array( $url_metric ), $current_etag, array(), 1, DAY_IN_SECONDS ); $collection->add_url_metric( $url_metric ); $this->assertSame( iterator_to_array( $collection )[0], $element->get_url_metric_group() ); diff --git a/plugins/optimization-detective/tests/test-class-od-url-metric.php b/plugins/optimization-detective/tests/test-class-od-url-metric.php index 71ba5799a6..51b23c243e 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metric.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metric.php @@ -31,6 +31,7 @@ public function data_provider_to_test_constructor(): array { return array( 'valid_minimal' => array( 'data' => array( + // Note: The 'etag' field is currently optional, so this data is still valid without it. 'url' => home_url( '/' ), 'viewport' => $viewport, 'timestamp' => microtime( true ), @@ -40,6 +41,7 @@ public function data_provider_to_test_constructor(): array { 'valid_with_element' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => $viewport, 'timestamp' => microtime( true ), @@ -51,6 +53,7 @@ public function data_provider_to_test_constructor(): array { // This tests that sanitization converts values into their expected PHP types. 'valid_but_props_are_strings' => array( 'data' => array( + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => array_map( 'strval', $viewport ), 'timestamp' => (string) microtime( true ), @@ -71,6 +74,7 @@ static function ( $value ) { 'bad_uuid' => array( 'data' => array( 'uuid' => 'foo', + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => $viewport, 'timestamp' => microtime( true ), @@ -78,9 +82,64 @@ static function ( $value ) { ), 'error' => 'OD_URL_Metric[uuid] is not a valid UUID.', ), + 'etag_too_short' => array( + 'data' => array( + 'uuid' => wp_generate_uuid4(), + 'etag' => 'foo', + 'url' => home_url( '/' ), + 'viewport' => $viewport, + 'timestamp' => microtime( true ), + 'elements' => array(), + ), + 'error' => 'OD_URL_Metric[etag] must be at least 32 characters long.', + ), + 'etag_too_long' => array( + 'data' => array( + 'uuid' => wp_generate_uuid4(), + 'etag' => 'd41d8cd98f00b204e9800998ecf8427e1', + 'url' => home_url( '/' ), + 'viewport' => $viewport, + 'timestamp' => microtime( true ), + 'elements' => array(), + ), + 'error' => 'OD_URL_Metric[etag] must be at most 32 characters long.', + ), + 'bad_etag1' => array( + 'data' => array( + 'uuid' => wp_generate_uuid4(), + 'etag' => 'd41d8cd98f00b204e9800998ecf8427$', + 'url' => home_url( '/' ), + 'viewport' => $viewport, + 'timestamp' => microtime( true ), + 'elements' => array(), + ), + 'error' => 'OD_URL_Metric[etag] does not match pattern ^[0-9a-f]{32}\z.', + ), + 'bad_etag2' => array( + 'data' => array( + 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ) . "\n", + 'url' => home_url( '/' ), + 'viewport' => $viewport, + 'timestamp' => microtime( true ), + 'elements' => array(), + ), + 'error' => 'OD_URL_Metric[etag] must be at most 32 characters long.', + ), + 'missing_etag' => array( + 'data' => array( + 'uuid' => wp_generate_uuid4(), + 'url' => home_url( '/' ), + 'viewport' => $viewport, + 'timestamp' => microtime( true ), + 'elements' => array(), + ), + // Note: Add error message 'etag is a required property of OD_URL_Metric.' when 'etag' becomes mandatory. + ), 'missing_viewport' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'timestamp' => microtime( true ), 'elements' => array(), @@ -90,6 +149,7 @@ static function ( $value ) { 'missing_viewport_width' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => array( 'height' => 640 ), 'timestamp' => microtime( true ), @@ -100,6 +160,7 @@ static function ( $value ) { 'bad_viewport' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => array( 'height' => 'tall', @@ -113,6 +174,7 @@ static function ( $value ) { 'viewport_aspect_ratio_too_small' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => array( 'width' => 1000, @@ -126,6 +188,7 @@ static function ( $value ) { 'viewport_aspect_ratio_too_large' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => array( 'width' => 10000, @@ -139,6 +202,7 @@ static function ( $value ) { 'missing_timestamp' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => $viewport, 'elements' => array(), @@ -148,6 +212,7 @@ static function ( $value ) { 'missing_elements' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => $viewport, 'timestamp' => microtime( true ), @@ -157,6 +222,7 @@ static function ( $value ) { 'missing_url' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'viewport' => $viewport, 'timestamp' => microtime( true ), 'elements' => array(), @@ -166,6 +232,7 @@ static function ( $value ) { 'bad_elements' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => $viewport, 'timestamp' => microtime( true ), @@ -180,6 +247,7 @@ static function ( $value ) { 'bad_intersection_width' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => $viewport, 'timestamp' => microtime( true ), @@ -202,6 +270,8 @@ static function ( $value ) { * @covers ::get_viewport_width * @covers ::get_timestamp * @covers ::get_elements + * @covers ::get_url + * @covers ::get_etag * @covers ::jsonSerialize * @covers ::get * @covers ::get_json_schema @@ -220,7 +290,9 @@ public function test_constructor( array $data, string $error = '' ): void { } $url_metric = new OD_URL_Metric( $data ); $this->assertNull( $url_metric->get_group() ); - $group = new OD_URL_Metric_Group( array( $url_metric ), 0, PHP_INT_MAX, 1, DAY_IN_SECONDS ); + $current_etag = md5( '' ); + $collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, array(), 1, DAY_IN_SECONDS ); + $group = $collection->get_first_group(); $url_metric->set_group( $group ); $this->assertSame( $group, $url_metric->get_group() ); @@ -252,6 +324,15 @@ static function ( OD_Element $element ) { $this->assertSame( $data['url'], $url_metric->get_url() ); $this->assertSame( $data['url'], $url_metric->get( 'url' ) ); + // Note: When the 'etag' field becomes required, the else statement can be removed. + if ( array_key_exists( 'etag', $data ) ) { + $this->assertSame( $data['etag'], $url_metric->get_etag() ); + $this->assertSame( $data['etag'], $url_metric->get( 'etag' ) ); + $this->assertTrue( 1 === preg_match( '/^[a-f0-9]{32}$/', $url_metric->get_etag() ) ); + } else { + $this->assertNull( $url_metric->get_etag() ); + } + $this->assertTrue( wp_is_uuid( $url_metric->get_uuid() ) ); $this->assertSame( $url_metric->get_uuid(), $url_metric->get( 'uuid' ) ); @@ -626,7 +707,7 @@ static function ( $additional_properties ) { 'assert' => function ( array $original_schema, $extended_schema ): void { $this->assertSame( $original_schema, $extended_schema ); }, - 'expected_incorrect_usage' => 'Filter: 'od_url_metric_schema_root_additional_properties'', + 'expected_incorrect_usage' => 'Filter: 'od_url_metric_schema_element_item_additional_properties'', ), 'adding_root_string' => array( @@ -715,7 +796,8 @@ public function test_get_json_schema_extensibility( Closure $set_up, Closure $as */ protected function check_schema_subset( array $schema, string $path, bool $extended = false ): void { $this->assertArrayHasKey( 'required', $schema, $path ); - if ( ! $extended ) { + // Skipping the check for 'root/etag' as it is currently optional. + if ( ! $extended && 'root/etag' !== $path ) { $this->assertTrue( $schema['required'], $path ); } $this->assertArrayHasKey( 'type', $schema, $path ); diff --git a/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php index 8553841aba..066f34f8b2 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php @@ -19,9 +19,12 @@ class Test_OD_URL_Metric_Group_Collection extends WP_UnitTestCase { * @return array Data. */ public function data_provider_test_construction(): array { + $current_etag = md5( '' ); + return array( 'no_breakpoints_ok' => array( 'url_metrics' => array(), + 'current_etag' => $current_etag, 'breakpoints' => array(), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -29,6 +32,7 @@ public function data_provider_test_construction(): array { ), 'negative_breakpoint_bad' => array( 'url_metrics' => array(), + 'current_etag' => $current_etag, 'breakpoints' => array( -1 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -36,6 +40,7 @@ public function data_provider_test_construction(): array { ), 'zero_breakpoint_bad' => array( 'url_metrics' => array(), + 'current_etag' => $current_etag, 'breakpoints' => array( 0 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -43,6 +48,7 @@ public function data_provider_test_construction(): array { ), 'max_breakpoint_bad' => array( 'url_metrics' => array(), + 'current_etag' => $current_etag, 'breakpoints' => array( PHP_INT_MAX ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -50,6 +56,7 @@ public function data_provider_test_construction(): array { ), 'string_breakpoint_bad' => array( 'url_metrics' => array(), + 'current_etag' => $current_etag, 'breakpoints' => array( 'narrow' ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -57,6 +64,7 @@ public function data_provider_test_construction(): array { ), 'negative_sample_size_bad' => array( 'url_metrics' => array(), + 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), 'sample_size' => -3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -64,13 +72,31 @@ public function data_provider_test_construction(): array { ), 'negative_freshness_tll_bad' => array( 'url_metrics' => array(), + 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => -HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), + 'invalid_current_etag_bad' => array( + 'url_metrics' => array(), + 'current_etag' => 'invalid_etag', + 'breakpoints' => array( 400 ), + 'sample_size' => 3, + 'freshness_ttl' => HOUR_IN_SECONDS, + 'exception' => InvalidArgumentException::class, + ), + 'invalid_current_etag_bad2' => array( + 'url_metrics' => array(), + 'current_etag' => md5( '' ) . PHP_EOL, // Note that /^[a-f0-9]{32}$/ would erroneously validate this. So the \z is required instead in /^[a-f0-9]{32}\z/. + 'breakpoints' => array( 400 ), + 'sample_size' => 3, + 'freshness_ttl' => HOUR_IN_SECONDS, + 'exception' => InvalidArgumentException::class, + ), 'invalid_url_metrics_bad' => array( 'url_metrics' => array( 'bad' ), + 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -81,6 +107,7 @@ public function data_provider_test_construction(): array { $this->get_sample_url_metric( array( 'viewport_width' => 200 ) ), $this->get_sample_url_metric( array( 'viewport_width' => 400 ) ), ), + 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -94,15 +121,18 @@ public function data_provider_test_construction(): array { * * @dataProvider data_provider_test_construction * - * @param OD_URL_Metric[] $url_metrics URL Metrics. - * @param int[] $breakpoints Breakpoints. - * @param int $sample_size Sample size. + * @param OD_URL_Metric[] $url_metrics URL Metrics. + * @param non-empty-string $current_etag Current ETag. + * @param int[] $breakpoints Breakpoints. + * @param int $sample_size Sample size. + * @param int $freshness_ttl Freshness TTL. + * @param string $exception Expected exception. */ - public function test_construction( array $url_metrics, array $breakpoints, int $sample_size, int $freshness_ttl, string $exception ): void { + public function test_construction( array $url_metrics, string $current_etag, array $breakpoints, int $sample_size, int $freshness_ttl, string $exception ): void { if ( '' !== $exception ) { $this->expectException( $exception ); } - $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $breakpoints, $sample_size, $freshness_ttl ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, $freshness_ttl ); $this->assertCount( count( $breakpoints ) + 1, $group_collection ); } @@ -182,7 +212,8 @@ public function data_provider_sample_size_and_breakpoints(): array { * @dataProvider data_provider_sample_size_and_breakpoints */ public function test_add_url_metric( int $sample_size, array $breakpoints, array $viewport_widths, array $expected_counts ): void { - $group_collection = new OD_URL_Metric_Group_Collection( array(), $breakpoints, $sample_size, HOUR_IN_SECONDS ); + $current_etag = md5( '' ); + $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS ); // Over-populate the sample size for the breakpoints by a dozen. foreach ( $viewport_widths as $viewport_width => $count ) { @@ -210,9 +241,10 @@ public function test_add_url_metric( int $sample_size, array $breakpoints, array * @covers ::add_url_metric */ public function test_adding_pushes_out_old_metrics(): void { + $current_etag = md5( '' ); $sample_size = 3; $breakpoints = array( 400, 600 ); - $group_collection = new OD_URL_Metric_Group_Collection( array(), $breakpoints, $sample_size, HOUR_IN_SECONDS ); + $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS ); // Populate the groups with stale URL Metrics. $viewport_widths = array( 300, 500, 700 ); @@ -333,7 +365,8 @@ function ( $viewport_width ) { $viewport_widths ); - $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $breakpoints, 3, HOUR_IN_SECONDS ); + $current_etag = md5( '' ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, 3, HOUR_IN_SECONDS ); $this->assertCount( count( $breakpoints ) + 1, @@ -369,16 +402,22 @@ static function ( OD_URL_Metric $url_metric ): int { */ public function data_provider_test_get_group_for_viewport_width(): array { $current_time = microtime( true ); + $current_etag = md5( '' ); $none_needed_data = array( - 'url_metrics' => ( function () use ( $current_time ): array { + 'url_metrics' => ( function () use ( $current_time, $current_etag ): array { return array_merge( array_fill( 0, 3, new OD_URL_Metric( array_merge( - $this->get_sample_url_metric( array( 'viewport_width' => 400 ) )->jsonSerialize(), + $this->get_sample_url_metric( + array( + 'viewport_width' => 400, + 'etag' => $current_etag, + ) + )->jsonSerialize(), array( 'timestamp' => $current_time ) ) ) @@ -388,7 +427,12 @@ public function data_provider_test_get_group_for_viewport_width(): array { 3, new OD_URL_Metric( array_merge( - $this->get_sample_url_metric( array( 'viewport_width' => 600 ) )->jsonSerialize(), + $this->get_sample_url_metric( + array( + 'viewport_width' => 600, + 'etag' => $current_etag, + ) + )->jsonSerialize(), array( 'timestamp' => $current_time ) ) ) @@ -396,6 +440,7 @@ public function data_provider_test_get_group_for_viewport_width(): array { ); } )(), 'current_time' => $current_time, + 'current_etag' => $current_etag, 'breakpoints' => array( 480 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -475,6 +520,34 @@ public function data_provider_test_get_group_for_viewport_width(): array { ), ) ), + + 'url-metric-stale-etag' => array_merge( + ( static function ( $data ): array { + $url_metrics_data = $data['url_metrics'][ count( $data['url_metrics'] ) - 1 ]->jsonSerialize(); + $url_metrics_data['etag'] = md5( 'something new!' ); + $data['url_metrics'][ count( $data['url_metrics'] ) - 1 ] = new OD_URL_Metric( $url_metrics_data ); + return $data; + } )( $none_needed_data ), + array( + 'expected_return' => array( + array( + 'minimumViewportWidth' => 0, + 'complete' => true, + ), + array( + 'minimumViewportWidth' => 481, + 'complete' => false, + ), + ), + 'expected_is_group_complete' => array( + 200 => true, + 400 => true, + 480 => true, + 481 => false, + 500 => false, + ), + ) + ), ); } @@ -490,14 +563,15 @@ public function data_provider_test_get_group_for_viewport_width(): array { * * @param OD_URL_Metric[] $url_metrics URL Metrics. * @param float $current_time Current time. + * @param non-empty-string $current_etag Current ETag. * @param int[] $breakpoints Breakpoints. * @param int $sample_size Sample size. * @param int $freshness_ttl Freshness TTL. * @param array $expected_return Expected return. * @param array $expected_is_group_complete Expected is group complete. */ - public function test_get_group_for_viewport_width( array $url_metrics, float $current_time, array $breakpoints, int $sample_size, int $freshness_ttl, array $expected_return, array $expected_is_group_complete ): void { - $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $breakpoints, $sample_size, $freshness_ttl ); + public function test_get_group_for_viewport_width( array $url_metrics, float $current_time, string $current_etag, array $breakpoints, int $sample_size, int $freshness_ttl, array $expected_return, array $expected_is_group_complete ): void { + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, $freshness_ttl ); $this->assertSame( $expected_return, array_map( @@ -529,28 +603,58 @@ static function ( OD_URL_Metric_Group $group ): array { public function test_is_every_group_populated(): void { $breakpoints = array( 480, 800 ); $sample_size = 3; + $current_etag = md5( '' ); $group_collection = new OD_URL_Metric_Group_Collection( array(), + $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS ); $this->assertFalse( $group_collection->is_every_group_populated() ); $this->assertFalse( $group_collection->is_every_group_complete() ); - $group_collection->add_url_metric( $this->get_sample_url_metric( array( 'viewport_width' => 200 ) ) ); + $group_collection->add_url_metric( + $this->get_sample_url_metric( + array( + 'viewport_width' => 200, + 'etag' => $current_etag, + ) + ) + ); $this->assertFalse( $group_collection->is_every_group_populated() ); $this->assertFalse( $group_collection->is_every_group_complete() ); - $group_collection->add_url_metric( $this->get_sample_url_metric( array( 'viewport_width' => 500 ) ) ); + $group_collection->add_url_metric( + $this->get_sample_url_metric( + array( + 'viewport_width' => 500, + 'etag' => $current_etag, + ) + ) + ); $this->assertFalse( $group_collection->is_every_group_populated() ); $this->assertFalse( $group_collection->is_every_group_complete() ); - $group_collection->add_url_metric( $this->get_sample_url_metric( array( 'viewport_width' => 900 ) ) ); + $group_collection->add_url_metric( + $this->get_sample_url_metric( + array( + 'viewport_width' => 900, + 'etag' => $current_etag, + ) + ) + ); $this->assertTrue( $group_collection->is_every_group_populated() ); $this->assertFalse( $group_collection->is_every_group_complete() ); // Now finish completing all the groups. foreach ( array_merge( $breakpoints, array( 1000 ) ) as $viewport_width ) { for ( $i = 0; $i < $sample_size; $i++ ) { - $group_collection->add_url_metric( $this->get_sample_url_metric( array( 'viewport_width' => $viewport_width ) ) ); + $group_collection->add_url_metric( + $this->get_sample_url_metric( + array( + 'viewport_width' => $viewport_width, + 'etag' => $current_etag, + ) + ) + ); } } $this->assertTrue( $group_collection->is_every_group_complete() ); @@ -582,6 +686,7 @@ public function test_get_groups_by_lcp_element(): void { $breakpoints = array( 480, 800 ); $sample_size = 3; + $current_etag = md5( '' ); $group_collection = new OD_URL_Metric_Group_Collection( array( // Group 1: 0-480 viewport widths. @@ -594,6 +699,7 @@ public function test_get_groups_by_lcp_element(): void { $get_url_metric_with_one_lcp_element( 820, $first_child_image_xpath ), $get_url_metric_with_one_lcp_element( 900, $first_child_image_xpath ), ), + $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS @@ -623,8 +729,10 @@ public function test_get_groups_by_lcp_element(): void { public function test_get_common_lcp_element(): void { $breakpoints = array( 480, 800 ); $sample_size = 3; + $current_etag = md5( '' ); $group_collection = new OD_URL_Metric_Group_Collection( array(), + $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS @@ -735,9 +843,10 @@ public function data_provider_element_max_intersection_ratios(): array { * @param array $expected Expected. */ public function test_get_all_element_max_intersection_ratios( array $url_metrics, array $expected ): void { + $current_etag = md5( '' ); $breakpoints = array( 480, 600, 782 ); $sample_size = 3; - $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $breakpoints, $sample_size, 0 ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, 0 ); $actual = $group_collection->get_all_element_max_intersection_ratios(); $this->assertSame( $actual, $group_collection->get_all_element_max_intersection_ratios(), 'Cached result is identical.' ); $this->assertSame( $expected, $actual ); @@ -912,9 +1021,10 @@ public function data_provider_get_all_elements_positioned_in_any_initial_viewpor * @param array $expected Expected. */ public function test_get_all_elements_positioned_in_any_initial_viewport( array $url_metrics, array $expected ): void { + $current_etag = md5( '' ); $breakpoints = array( 480, 600, 782 ); $sample_size = 3; - $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $breakpoints, $sample_size, 0 ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, 0 ); $actual = $group_collection->get_all_elements_positioned_in_any_initial_viewport(); $this->assertSame( $actual, $group_collection->get_all_elements_positioned_in_any_initial_viewport(), 'Cached result is identical.' ); $this->assertSame( $expected, $actual ); @@ -938,6 +1048,7 @@ public function test_get_flattened_url_metrics(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, + md5( '' ), array( 500, 700 ), 3, HOUR_IN_SECONDS @@ -965,6 +1076,7 @@ public function test_json_serialize(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, + md5( '' ), array( 500, 700 ), 3, HOUR_IN_SECONDS @@ -973,6 +1085,7 @@ public function test_json_serialize(): void { $json = wp_json_encode( $group_collection ); $parsed_json = json_decode( $json, true ); $expected_keys = array( + 'current_etag', 'breakpoints', 'freshness_ttl', 'sample_size', diff --git a/plugins/optimization-detective/tests/test-class-od-url-metrics-group.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group.php index ef59513515..2b1538c7a5 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metrics-group.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metrics-group.php @@ -111,7 +111,12 @@ public function test_construction( array $url_metrics, int $minimum_viewport_wid if ( '' !== $exception ) { $this->expectException( $exception ); } - $group = new OD_URL_Metric_Group( $url_metrics, $minimum_viewport_width, $maximum_viewport_width, $sample_size, $freshness_ttl ); + + // This is not example usage for how a group should be constructed. Normally, a group is only ever constructed + // by OD_URL_Metric_Group_Collection when the collection is constructed. The OD_URL_Metric_Group is being + // constructed here just for the sake of testing. + $dummy_collection = new OD_URL_Metric_Group_Collection( array(), md5( '' ), array(), 1, DAY_IN_SECONDS ); + $group = new OD_URL_Metric_Group( $url_metrics, $minimum_viewport_width, $maximum_viewport_width, $sample_size, $freshness_ttl, $dummy_collection ); $this->assertCount( count( $url_metrics ), $group ); $this->assertSame( $minimum_viewport_width, $group->get_minimum_viewport_width() ); @@ -128,8 +133,8 @@ public function test_construction( array $url_metrics, int $minimum_viewport_wid public function data_provider_test_is_viewport_width_in_range(): array { return array( '0-10' => array( - 'minimum_viewport_width' => 0, - 'maximum_viewport_width' => 10, + 'breakpoints' => array( 10 ), + 'group_index' => 0, 'viewport_widths_expected' => array( 0 => true, 1 => true, @@ -139,8 +144,8 @@ public function data_provider_test_is_viewport_width_in_range(): array { ), ), '100-200' => array( - 'minimum_viewport_width' => 100, - 'maximum_viewport_width' => 200, + 'breakpoints' => array( 99, 200 ), + 'group_index' => 1, 'viewport_widths_expected' => array( 0 => false, 99 => false, @@ -160,12 +165,22 @@ public function data_provider_test_is_viewport_width_in_range(): array { * * @dataProvider data_provider_test_is_viewport_width_in_range * - * @param int $minimum_viewport_width Minimum viewport width. - * @param int $maximum_viewport_width Maximum viewport width. + * @param int[] $breakpoints Breakpoints. + * @param int $group_index Group index. * @param array $viewport_widths_expected Viewport widths expected. */ - public function test_is_viewport_width_in_range( int $minimum_viewport_width, int $maximum_viewport_width, array $viewport_widths_expected ): void { - $group = new OD_URL_Metric_Group( array(), $minimum_viewport_width, $maximum_viewport_width, 3, HOUR_IN_SECONDS ); + public function test_is_viewport_width_in_range( array $breakpoints, int $group_index, array $viewport_widths_expected ): void { + $collection = new OD_URL_Metric_Group_Collection( + array(), + md5( '' ), + $breakpoints, + 3, + HOUR_IN_SECONDS + ); + $groups = iterator_to_array( $collection ); + $this->assertArrayHasKey( $group_index, $groups ); + $group = $groups[ $group_index ]; + $this->assertInstanceOf( OD_URL_Metric_Group::class, $group ); foreach ( $viewport_widths_expected as $viewport_width => $expected ) { $this->assertSame( $expected, $group->is_viewport_width_in_range( $viewport_width ), "Failed for viewport width of $viewport_width" ); } @@ -199,13 +214,19 @@ public function test_add_url_metric( int $viewport_width, string $exception ): v if ( '' !== $exception ) { $this->expectException( $exception ); } - $group = new OD_URL_Metric_Group( array(), 480, 799, 1, HOUR_IN_SECONDS ); + + $etag = md5( '' ); + $collection = new OD_URL_Metric_Group_Collection( array(), $etag, array( 480, 800 ), 1, HOUR_IN_SECONDS ); + $groups = iterator_to_array( $collection ); + $this->assertCount( 3, $groups ); + $group = $groups[1]; $this->assertFalse( $group->is_complete() ); $group->add_url_metric( new OD_URL_Metric( array( 'url' => home_url( '/' ), + 'etag' => $etag, 'viewport' => array( 'width' => $viewport_width, 'height' => ceil( $viewport_width / 2 ), @@ -221,6 +242,65 @@ public function test_add_url_metric( int $viewport_width, string $exception ): v $this->assertTrue( $group->is_complete() ); } + /** + * Data provider. + * + * @return array Data. + */ + public function data_provider_test_is_complete(): array { + // Note: Test cases for empty URL Metrics and for exact sample size are already covered in the test_add_url_metric() method. + return array( + 'old_url_metric' => array( + 'url_metric' => $this->get_sample_url_metric( + array( + 'timestamp' => microtime( true ) - ( HOUR_IN_SECONDS + 1 ), + 'etag' => md5( '' ), + ) + ), + 'expected_is_group_complete' => false, + ), + // Note: The following test case will not be required once the ETag is mandatory in a future release. + 'etag_missing' => array( + 'url_metric' => new OD_URL_Metric( + array( + 'url' => home_url( '/' ), + 'viewport' => array( + 'width' => 400, + 'height' => 700, + ), + 'timestamp' => microtime( true ), + 'elements' => array(), + ) + ), + 'expected_is_group_complete' => false, + ), + 'etag_mismatch' => array( + 'url_metric' => $this->get_sample_url_metric( array( 'etag' => md5( 'different_etag' ) ) ), + 'expected_is_group_complete' => false, + ), + 'etag_match' => array( + 'url_metric' => $this->get_sample_url_metric( array( 'etag' => md5( '' ) ) ), + 'expected_is_group_complete' => true, + ), + ); + } + + /** + * Test is_complete(). + * + * @covers ::is_complete + * + * @dataProvider data_provider_test_is_complete + */ + public function test_is_complete( OD_URL_Metric $url_metric, bool $expected_is_group_complete ): void { + $collection = new OD_URL_Metric_Group_Collection( array(), md5( '' ), array( 768 ), 1, HOUR_IN_SECONDS ); + $group = $collection->get_first_group(); + + $group->add_url_metric( $url_metric ); + + $this->assertSame( $expected_is_group_complete, $group->is_complete() ); + } + /** * Data provider. * @@ -325,7 +405,8 @@ public function data_provider_test_get_lcp_element(): array { * @param array $expected_lcp_element_xpaths Expected XPaths. */ public function test_get_lcp_element( array $breakpoints, array $url_metrics, array $expected_lcp_element_xpaths ): void { - $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $breakpoints, 10, HOUR_IN_SECONDS ); + $current_etag = md5( '' ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, 10, HOUR_IN_SECONDS ); $lcp_element_xpaths_by_minimum_viewport_widths = array(); foreach ( $group_collection as $group ) { @@ -346,19 +427,17 @@ public function test_get_lcp_element( array $breakpoints, array $url_metrics, ar * @covers ::jsonSerialize */ public function test_json_serialize(): void { - $group = new OD_URL_Metric_Group( - array_map( - function ( $viewport_width ) { - return $this->get_sample_url_metric( array( 'viewport_width' => $viewport_width ) ); - }, - array( 400, 600, 800 ) - ), - 0, - 1000, - 1, - HOUR_IN_SECONDS + $current_etag = md5( '' ); + $url_metrics = array_map( + function ( $viewport_width ) { + return $this->get_sample_url_metric( array( 'viewport_width' => $viewport_width ) ); + }, + array( 400, 600, 800 ) ); + $collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, array( 1000 ), 1, HOUR_IN_SECONDS ); + $group = $collection->get_first_group(); + $json = wp_json_encode( $group ); $parsed_json = json_decode( $json, true ); $expected_keys = array( diff --git a/plugins/optimization-detective/tests/test-detection.php b/plugins/optimization-detective/tests/test-detection.php index 6756cd80b0..a159591115 100644 --- a/plugins/optimization-detective/tests/test-detection.php +++ b/plugins/optimization-detective/tests/test-detection.php @@ -127,10 +127,11 @@ static function ( array $urls ): array { */ public function test_od_get_detection_script_returns_script( Closure $set_up, array $expected_exports ): void { $set_up(); - $slug = od_get_url_metrics_slug( array( 'p' => '1' ) ); + $slug = od_get_url_metrics_slug( array( 'p' => '1' ) ); + $current_etag = md5( '' ); $breakpoints = array( 480, 600, 782 ); - $group_collection = new OD_URL_Metric_Group_Collection( array(), $breakpoints, 3, HOUR_IN_SECONDS ); + $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, $breakpoints, 3, HOUR_IN_SECONDS ); $script = od_get_detection_script( $slug, $group_collection ); diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index 95006c4165..c7924480a0 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -27,6 +27,7 @@ trait Optimization_Detective_Test_Helpers { */ public function populate_url_metrics( array $elements, bool $complete = true ): void { $slug = od_get_url_metrics_slug( od_get_normalized_query_vars() ); + $etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); // Note: Tests rely on the od_current_url_metrics_etag_data filter to set the desired value. $sample_size = $complete ? od_get_url_metrics_breakpoint_sample_size() : 1; foreach ( array_merge( od_get_breakpoint_max_widths(), array( 1000 ) ) as $viewport_width ) { for ( $i = 0; $i < $sample_size; $i++ ) { @@ -34,6 +35,7 @@ public function populate_url_metrics( array $elements, bool $complete = true ): $slug, $this->get_sample_url_metric( array( + 'etag' => $etag, 'viewport_width' => $viewport_width, 'elements' => $elements, ) @@ -65,6 +67,8 @@ public function get_sample_dom_rect(): array { * Gets a sample URL metric. * * @phpstan-param array{ + * timestamp?: float, + * etag?: non-empty-string, * url?: string, * viewport_width?: int, * viewport_height?: int, @@ -77,9 +81,11 @@ public function get_sample_dom_rect(): array { public function get_sample_url_metric( array $params ): OD_URL_Metric { $params = array_merge( array( + 'etag' => od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ), // Note: Tests rely on the od_current_url_metrics_etag_data filter to set the desired value. 'url' => home_url( '/' ), 'viewport_width' => 480, 'elements' => array(), + 'timestamp' => microtime( true ), ), $params ); @@ -90,12 +96,13 @@ public function get_sample_url_metric( array $params ): OD_URL_Metric { return new OD_URL_Metric( array( - 'url' => home_url( '/' ), + 'etag' => $params['etag'], + 'url' => $params['url'], 'viewport' => array( 'width' => $params['viewport_width'], 'height' => $params['viewport_height'] ?? ceil( $params['viewport_width'] / 2 ), ), - 'timestamp' => microtime( true ), + 'timestamp' => $params['timestamp'], 'elements' => array_map( function ( array $element ): array { return array_merge(