From f7295d890286e6be0992961f52dcde3c99ad022d Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Tue, 26 Nov 2024 16:45:57 +0530 Subject: [PATCH 01/48] Store the ETag along with the URL metric --- plugins/optimization-detective/detect.js | 3 +++ plugins/optimization-detective/detection.php | 6 ++++++ plugins/optimization-detective/helper.php | 18 ++++++++++++++++++ plugins/optimization-detective/hooks.php | 1 + .../optimization-detective/optimization.php | 13 +++++++++++++ plugins/optimization-detective/types.ts | 1 + 6 files changed, 42 insertions(+) diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index e5e6cc8ab3..4e9e6c23f7 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 ETag for URL Metric. * @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, @@ -440,6 +442,7 @@ export default async function detect( { } urlMetric = { + eTag: currentETag, url: currentUrl, viewport: { width: win.innerWidth, diff --git a/plugins/optimization-detective/detection.php b/plugins/optimization-detective/detection.php index 32e248ae26..76241d1e94 100644 --- a/plugins/optimization-detective/detection.php +++ b/plugins/optimization-detective/detection.php @@ -66,10 +66,14 @@ function od_get_cache_purge_post_id(): ?int { * @since 0.1.0 * @access private * + * @global string $od_etag ETag for the current environment. + * * @param string $slug URL Metrics slug. * @param OD_URL_Metric_Group_Collection $group_collection URL Metric group collection. */ function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $group_collection ): string { + global $od_etag; + $web_vitals_lib_data = require __DIR__ . '/build/web-vitals.asset.php'; $web_vitals_lib_src = add_query_arg( 'ver', $web_vitals_lib_data['version'], plugin_dir_url( __FILE__ ) . 'build/web-vitals.js' ); @@ -85,12 +89,14 @@ 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(); + $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' => $od_etag, 'currentUrl' => $current_url, 'urlMetricSlug' => $slug, 'cachePurgePostId' => od_get_cache_purge_post_id(), diff --git a/plugins/optimization-detective/helper.php b/plugins/optimization-detective/helper.php index b9dc348f94..620b84d1eb 100644 --- a/plugins/optimization-detective/helper.php +++ b/plugins/optimization-detective/helper.php @@ -100,3 +100,21 @@ function od_get_asset_path( string $src_path, ?string $min_path = null ): string return $min_path; } + +/** + * Adds optional URL Metric schema root properties. + * + * @since n.e.x.t + * + * @param array $additional_properties Additional properties. + * @return array Additional properties. + */ +function od_add_optional_url_metric_schema_root_properties( array $additional_properties ): array { + // The ETag is being kept optional for now so as to avoid invalidating all current URL Metrics. + $additional_properties['eTag'] = array( + 'description' => __( 'The ETag for the URL Metric.', 'optimization-detective' ), + 'type' => 'string', + ); + + return $additional_properties; +} diff --git a/plugins/optimization-detective/hooks.php b/plugins/optimization-detective/hooks.php index c0f94d148c..3bbfc4507d 100644 --- a/plugins/optimization-detective/hooks.php +++ b/plugins/optimization-detective/hooks.php @@ -15,3 +15,4 @@ OD_URL_Metrics_Post_Type::add_hooks(); add_action( 'wp', 'od_maybe_add_template_output_buffer_filter' ); add_action( 'wp_head', 'od_render_generator_meta_tag' ); +add_filter( 'od_url_metric_schema_root_additional_properties', 'od_add_optional_url_metric_schema_root_properties' ); diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index 11f4df10a1..52bbccc3fd 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -217,6 +217,19 @@ function od_optimize_template_output_buffer( string $buffer ): string { $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 ); + + /** + * ETag generated for the current environment. + * + * @since n.e.x.t + * + * @global string $od_etag + */ + global $od_etag; + + // Generate and store an ETag consisting of all the tag visitors' IDs in the current environment. + $od_etag = implode( ',', array_keys( $visitors ) ); + 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/types.ts b/plugins/optimization-detective/types.ts index fc4e375b60..cde37003aa 100644 --- a/plugins/optimization-detective/types.ts +++ b/plugins/optimization-detective/types.ts @@ -13,6 +13,7 @@ export interface ElementData { export type ExtendedElementData = ExcludeProps< ElementData >; export interface URLMetric { + eTag: string; url: string; viewport: { width: number; From 57b79e1a9d8fc53955004b80997a941de3b1781e Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Tue, 26 Nov 2024 16:47:40 +0530 Subject: [PATCH 02/48] Factor in ETag of the URL metric for determining if it is stale --- .../class-od-url-metric-group.php | 16 ++++++++++++++-- plugins/optimization-detective/optimization.php | 6 +++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 963f5b8840..562229f0ae 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -220,20 +220,32 @@ 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. + * + * @global string $od_etag ETag for the current environment. + * * @return bool Whether complete. */ public function is_complete(): bool { + global $od_etag; + if ( array_key_exists( __FUNCTION__, $this->result_cache ) ) { return $this->result_cache[ __FUNCTION__ ]; } - $result = ( function () { + $result = ( function () use ( $od_etag ) { if ( count( $this->url_metrics ) < $this->sample_size ) { return false; } $current_time = microtime( true ); foreach ( $this->url_metrics as $url_metric ) { - if ( $current_time > $url_metric->get_timestamp() + $this->freshness_ttl ) { + if ( + $current_time > $url_metric->get_timestamp() + $this->freshness_ttl + || + // If the generated ETag does not match the URL metric's ETag, consider the URL metric as stale. + // NOTE: Since the ETag is optional for now, existing ones without it are not considered stale. + ( $url_metric->get( 'eTag' ) !== null && $url_metric->get( 'eTag' ) !== $od_etag ) + ) { return false; } } diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index 52bbccc3fd..f6eada8825 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -199,9 +199,6 @@ function od_optimize_template_output_buffer( string $buffer ): string { 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(); /** @@ -230,6 +227,9 @@ function od_optimize_template_output_buffer( string $buffer ): string { // Generate and store an ETag consisting of all the tag visitors' IDs in the current environment. $od_etag = implode( ',', array_keys( $visitors ) ); + // 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? From 7b48f267c7c99ef019f544ae1a1d59a742a1adeb Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Tue, 26 Nov 2024 16:48:33 +0530 Subject: [PATCH 03/48] Update test case to allow ETag to be optional in JSON Schema --- .../tests/test-class-od-url-metric.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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..ce91a87440 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metric.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metric.php @@ -728,7 +728,11 @@ protected function check_schema_subset( array $schema, string $path, bool $exten $this->assertTrue( $schema['additionalProperties'], "Path: $path" ); } foreach ( $schema['properties'] as $key => $property_schema ) { - $this->check_schema_subset( $property_schema, "$path/$key", $extended ); + if ( 'eTag' === $key ) { + $this->check_schema_subset( $property_schema, "$path/$key", true ); + } else { + $this->check_schema_subset( $property_schema, "$path/$key", $extended ); + } } } elseif ( 'array' === $schema['type'] ) { $this->assertArrayHasKey( 'items', $schema, $path ); From 9999eea2a8411634768d94895048ef18d1100eff Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Tue, 26 Nov 2024 16:53:08 +0530 Subject: [PATCH 04/48] Fix filter name typo in URL Metric schema extension --- plugins/optimization-detective/class-od-url-metric.php | 2 +- .../optimization-detective/tests/test-class-od-url-metric.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index d2f772d213..e39be77258 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -309,7 +309,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' ); } 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 ce91a87440..a9911da918 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metric.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metric.php @@ -626,7 +626,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( From 3b01ea291bb3da7c4ac80edd510ecd4bfc06715a Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Tue, 26 Nov 2024 18:34:47 +0530 Subject: [PATCH 05/48] Factor in ETag when computing HMAC --- plugins/optimization-detective/detection.php | 2 +- plugins/optimization-detective/storage/data.php | 11 ++++++----- plugins/optimization-detective/storage/rest-api.php | 2 +- .../tests/storage/test-rest-api.php | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/plugins/optimization-detective/detection.php b/plugins/optimization-detective/detection.php index 76241d1e94..c142bcedd5 100644 --- a/plugins/optimization-detective/detection.php +++ b/plugins/optimization-detective/detection.php @@ -100,7 +100,7 @@ function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $ '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, $od_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/storage/data.php b/plugins/optimization-detective/storage/data.php index a53569301b..78a00deb80 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -150,15 +150,15 @@ function od_get_url_metrics_slug( array $query_vars ): string { * * @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 $etag 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 $etag, string $url, ?int $cache_purge_post_id = null ): string { + $action = "store_url_metric:$slug:$etag:$url:$cache_purge_post_id"; return wp_hash( $action, 'nonce' ); } @@ -173,12 +173,13 @@ function od_get_url_metrics_storage_hmac( string $slug, string $url, ?int $cache * * @param string $hmac HMAC. * @param string $slug Slug (hash of normalized query vars). + * @param string $etag 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 $etag, string $url, ?int $cache_purge_post_id = null ): bool { + return hash_equals( od_get_url_metrics_storage_hmac( $slug, $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..233a38df01 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -58,7 +58,7 @@ function od_register_endpoint(): void { 'required' => true, 'pattern' => '^[0-9a-f]+$', '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['eTag'], $request['url'], $request['cache_purge_post_id'] ?? null ) ) { return new WP_Error( 'invalid_hmac', __( 'URL Metrics HMAC verification failure.', 'optimization-detective' ) ); } return true; diff --git a/plugins/optimization-detective/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index 1d112b4337..407b9e41c1 100644 --- a/plugins/optimization-detective/tests/storage/test-rest-api.php +++ b/plugins/optimization-detective/tests/storage/test-rest-api.php @@ -156,7 +156,7 @@ 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' ) ), '', home_url( '/' ) ), ), 'invalid_hmac_with_queried_object' => array( 'hmac' => od_get_url_metrics_storage_hmac( od_get_url_metrics_slug( array() ), home_url( '/' ), 1 ), @@ -672,7 +672,7 @@ private function get_valid_params( array $extras = array() ): array { $data = array_merge( array( 'slug' => $slug, - 'hmac' => od_get_url_metrics_storage_hmac( $slug, $data['url'] ), + 'hmac' => od_get_url_metrics_storage_hmac( $slug, '', $data['url'] ), ), $data ); From b84f62aeeb0488c0895996159ecf2f9642df7c0c Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Wed, 27 Nov 2024 05:22:50 +0530 Subject: [PATCH 06/48] Make ETag a core property instead of adding it using a filter --- .../class-od-url-metric-group.php | 2 +- .../class-od-url-metric.php | 18 ++++++++++++++++++ plugins/optimization-detective/helper.php | 18 ------------------ plugins/optimization-detective/hooks.php | 1 - .../tests/test-class-od-url-metric.php | 9 +++------ 5 files changed, 22 insertions(+), 26 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 562229f0ae..6d765a45be 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -244,7 +244,7 @@ public function is_complete(): bool { || // If the generated ETag does not match the URL metric's ETag, consider the URL metric as stale. // NOTE: Since the ETag is optional for now, existing ones without it are not considered stale. - ( $url_metric->get( 'eTag' ) !== null && $url_metric->get( 'eTag' ) !== $od_etag ) + ( $url_metric->get_etag() !== null && $url_metric->get_etag() !== $od_etag ) ) { return false; } diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index e39be77258..0a4e41b41a 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?: 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,11 @@ 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', + 'required' => false, + ), 'url' => array( 'description' => __( 'The URL for which the metric was obtained.', 'optimization-detective' ), 'type' => 'string', @@ -417,6 +424,17 @@ public function get_uuid(): string { return $this->data['uuid']; } + /** + * Gets ETag. + * + * @since n.e.x.t + * + * @return string|null The ETag, or null if not set. + */ + public function get_etag(): ?string { + return $this->data['eTag'] ?? null; + } + /** * Gets URL. * diff --git a/plugins/optimization-detective/helper.php b/plugins/optimization-detective/helper.php index 620b84d1eb..b9dc348f94 100644 --- a/plugins/optimization-detective/helper.php +++ b/plugins/optimization-detective/helper.php @@ -100,21 +100,3 @@ function od_get_asset_path( string $src_path, ?string $min_path = null ): string return $min_path; } - -/** - * Adds optional URL Metric schema root properties. - * - * @since n.e.x.t - * - * @param array $additional_properties Additional properties. - * @return array Additional properties. - */ -function od_add_optional_url_metric_schema_root_properties( array $additional_properties ): array { - // The ETag is being kept optional for now so as to avoid invalidating all current URL Metrics. - $additional_properties['eTag'] = array( - 'description' => __( 'The ETag for the URL Metric.', 'optimization-detective' ), - 'type' => 'string', - ); - - return $additional_properties; -} diff --git a/plugins/optimization-detective/hooks.php b/plugins/optimization-detective/hooks.php index 3bbfc4507d..c0f94d148c 100644 --- a/plugins/optimization-detective/hooks.php +++ b/plugins/optimization-detective/hooks.php @@ -15,4 +15,3 @@ OD_URL_Metrics_Post_Type::add_hooks(); add_action( 'wp', 'od_maybe_add_template_output_buffer_filter' ); add_action( 'wp_head', 'od_render_generator_meta_tag' ); -add_filter( 'od_url_metric_schema_root_additional_properties', 'od_add_optional_url_metric_schema_root_properties' ); 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 a9911da918..4dee9a1346 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metric.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metric.php @@ -715,7 +715,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 ); @@ -728,11 +729,7 @@ protected function check_schema_subset( array $schema, string $path, bool $exten $this->assertTrue( $schema['additionalProperties'], "Path: $path" ); } foreach ( $schema['properties'] as $key => $property_schema ) { - if ( 'eTag' === $key ) { - $this->check_schema_subset( $property_schema, "$path/$key", true ); - } else { - $this->check_schema_subset( $property_schema, "$path/$key", $extended ); - } + $this->check_schema_subset( $property_schema, "$path/$key", $extended ); } } elseif ( 'array' === $schema['type'] ) { $this->assertArrayHasKey( 'items', $schema, $path ); From ae708cf9101a99cb8b0f0c822d1368c32c49e6cb Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Wed, 27 Nov 2024 07:35:50 +0530 Subject: [PATCH 07/48] Return empty string instead of null when an existing URL metric doesn't have ETag set --- .../optimization-detective/class-od-url-metric-group.php | 3 +-- plugins/optimization-detective/class-od-url-metric.php | 7 ++++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 6d765a45be..56fb9c8b7c 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -243,8 +243,7 @@ public function is_complete(): bool { $current_time > $url_metric->get_timestamp() + $this->freshness_ttl || // If the generated ETag does not match the URL metric's ETag, consider the URL metric as stale. - // NOTE: Since the ETag is optional for now, existing ones without it are not considered stale. - ( $url_metric->get_etag() !== null && $url_metric->get_etag() !== $od_etag ) + ( $url_metric->get_etag() !== $od_etag ) ) { return false; } diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index 0a4e41b41a..a19843eea1 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -429,10 +429,11 @@ public function get_uuid(): string { * * @since n.e.x.t * - * @return string|null The ETag, or null if not set. + * @return string ETag. */ - public function get_etag(): ?string { - return $this->data['eTag'] ?? null; + public function get_etag(): string { + // Since the ETag is optional for now, return an empty string for old URL metrics that do not have one. + return $this->data['eTag'] ?? ''; } /** From e1151348e5d85b9419b57adff285da78155a5e61 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Wed, 27 Nov 2024 09:17:03 +0530 Subject: [PATCH 08/48] Send ETag as REST API arg instead of URL metric property --- plugins/optimization-detective/class-od-url-metric.php | 1 + plugins/optimization-detective/detect.js | 2 +- plugins/optimization-detective/storage/rest-api.php | 6 ++++++ plugins/optimization-detective/types.ts | 1 - 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index a19843eea1..5093e1c605 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -214,6 +214,7 @@ public static function get_json_schema(): array { 'description' => __( 'The ETag for the URL Metric.', 'optimization-detective' ), 'type' => 'string', 'required' => false, + 'readonly' => true, // Omit from REST API. ), 'url' => array( 'description' => __( 'The URL for which the metric was obtained.', 'optimization-detective' ), diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index 4e9e6c23f7..8a77b867a6 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -442,7 +442,6 @@ export default async function detect( { } urlMetric = { - eTag: currentETag, url: currentUrl, viewport: { width: win.innerWidth, @@ -542,6 +541,7 @@ export default async function detect( { const url = new URL( restApiEndpoint ); url.searchParams.set( 'slug', urlMetricSlug ); + url.searchParams.set( 'eTag', currentETag ); if ( typeof cachePurgePostId === 'number' ) { url.searchParams.set( 'cache_purge_post_id', diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index 233a38df01..92e96694aa 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -46,6 +46,11 @@ function od_register_endpoint(): void { 'required' => true, 'pattern' => '^[0-9a-f]{32}$', ), + 'eTag' => array( + 'type' => 'string', + 'description' => __( 'ETag for the current environment.', 'optimization-detective' ), + 'required' => true, + ), 'cache_purge_post_id' => array( 'type' => 'integer', 'description' => __( 'Cache purge post ID.', 'optimization-detective' ), @@ -182,6 +187,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( 'eTag' ), ) ) ); diff --git a/plugins/optimization-detective/types.ts b/plugins/optimization-detective/types.ts index cde37003aa..fc4e375b60 100644 --- a/plugins/optimization-detective/types.ts +++ b/plugins/optimization-detective/types.ts @@ -13,7 +13,6 @@ export interface ElementData { export type ExtendedElementData = ExcludeProps< ElementData >; export interface URLMetric { - eTag: string; url: string; viewport: { width: number; From 1cd62c00e3c38366d297998b58a017d219b58047 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Wed, 27 Nov 2024 09:47:33 +0530 Subject: [PATCH 09/48] Fix Test_OD_Storage_REST_API --- .../optimization-detective/tests/storage/test-rest-api.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index 407b9e41c1..cace578bc7 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['eTag'], $params['url'], $params['cache_purge_post_id'] ); return $params; }, ), @@ -672,7 +672,7 @@ private function get_valid_params( array $extras = array() ): array { $data = array_merge( array( 'slug' => $slug, - 'hmac' => od_get_url_metrics_storage_hmac( $slug, '', $data['url'] ), + 'hmac' => od_get_url_metrics_storage_hmac( $slug, $data['eTag'], $data['url'] ), ), $data ); From cb64e1dba786f72ebe844746d3ebcee24bdb5442 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Wed, 27 Nov 2024 18:09:11 +0530 Subject: [PATCH 10/48] Store the current ETag as a property of `OD_URL_Metric_Group_Collection` instead of a global variable --- .../class-od-url-metric-group-collection.php | 26 +++++++++++- .../class-od-url-metric-group.php | 10 ++--- plugins/optimization-detective/detection.php | 8 +--- .../optimization-detective/optimization.php | 30 +++++--------- .../class-od-url-metrics-post-type.php | 1 + .../storage/rest-api.php | 1 + .../tests/storage/test-rest-api.php | 1 + .../tests/test-class-od-element.php | 2 +- ...-class-od-url-metrics-group-collection.php | 40 ++++++++++++++----- .../tests/test-class-od-url-metrics-group.php | 2 +- .../tests/test-detection.php | 2 +- 11 files changed, 75 insertions(+), 48 deletions(-) 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..2c1ced1e77 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 string + */ + private $current_etag; + /** * Breakpoints in max widths. * @@ -94,11 +102,14 @@ 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 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 ) { + $this->current_etag = $current_etag; + // Set breakpoints. sort( $breakpoints ); $breakpoints = array_values( array_unique( $breakpoints, SORT_NUMERIC ) ); @@ -160,6 +171,17 @@ public function __construct( array $url_metrics, array $breakpoints, int $sample } } + /** + * Gets the current ETag. + * + * @since n.e.x.t + * + * @return string Current ETag. + */ + public function get_current_etag(): string { + return $this->current_etag; + } + /** * Gets the first URL Metric group. * @@ -613,6 +635,7 @@ public function count(): int { * @since 0.3.1 * * @return array{ + * current_etag: string, * breakpoints: positive-int[], * freshness_ttl: 0|positive-int, * sample_size: positive-int, @@ -631,6 +654,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 56fb9c8b7c..2528964763 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -222,18 +222,14 @@ static function ( OD_URL_Metric $a, OD_URL_Metric $b ): int { * * @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. * - * @global string $od_etag ETag for the current environment. - * * @return bool Whether complete. */ public function is_complete(): bool { - global $od_etag; - if ( array_key_exists( __FUNCTION__, $this->result_cache ) ) { return $this->result_cache[ __FUNCTION__ ]; } - $result = ( function () use ( $od_etag ) { + $result = ( function () { if ( count( $this->url_metrics ) < $this->sample_size ) { return false; } @@ -242,8 +238,8 @@ public function is_complete(): bool { if ( $current_time > $url_metric->get_timestamp() + $this->freshness_ttl || - // If the generated ETag does not match the URL metric's ETag, consider the URL metric as stale. - ( $url_metric->get_etag() !== $od_etag ) + // If the URL Metric's ETag doesn't match the current ETag, consider the URL metric as stale. + ( null !== $this->collection && $url_metric->get_etag() !== $this->collection->get_current_etag() ) ) { return false; } diff --git a/plugins/optimization-detective/detection.php b/plugins/optimization-detective/detection.php index c142bcedd5..9532351908 100644 --- a/plugins/optimization-detective/detection.php +++ b/plugins/optimization-detective/detection.php @@ -66,14 +66,10 @@ function od_get_cache_purge_post_id(): ?int { * @since 0.1.0 * @access private * - * @global string $od_etag ETag for the current environment. - * * @param string $slug URL Metrics slug. * @param OD_URL_Metric_Group_Collection $group_collection URL Metric group collection. */ function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $group_collection ): string { - global $od_etag; - $web_vitals_lib_data = require __DIR__ . '/build/web-vitals.asset.php'; $web_vitals_lib_src = add_query_arg( 'ver', $web_vitals_lib_data['version'], plugin_dir_url( __FILE__ ) . 'build/web-vitals.js' ); @@ -96,11 +92,11 @@ function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $ 'isDebug' => WP_DEBUG, 'extensionModuleUrls' => $extension_module_urls, 'restApiEndpoint' => rest_url( OD_REST_API_NAMESPACE . OD_URL_METRICS_ROUTE ), - 'currentETag' => $od_etag, + 'currentETag' => $group_collection->get_current_etag(), 'currentUrl' => $current_url, 'urlMetricSlug' => $slug, 'cachePurgePostId' => od_get_cache_purge_post_id(), - 'urlMetricHMAC' => od_get_url_metrics_storage_hmac( $slug, $od_etag, $current_url, $cache_purge_post_id ), + 'urlMetricHMAC' => od_get_url_metrics_storage_hmac( $slug, $group_collection->get_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 aa8904561c..cff7948fed 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -195,13 +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() - ); - $tag_visitor_registry = new OD_Tag_Visitor_Registry(); /** @@ -213,22 +206,19 @@ function od_optimize_template_output_buffer( string $buffer ): string { */ do_action( 'od_register_tag_visitors', $tag_visitor_registry ); + $visitors = iterator_to_array( $tag_visitor_registry ); + $current_etag = implode( ',', array_keys( $visitors ) ); + + $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 ); - - /** - * ETag generated for the current environment. - * - * @since n.e.x.t - * - * @global string $od_etag - */ - global $od_etag; - - // Generate and store an ETag consisting of all the tag visitors' IDs in the current environment. - $od_etag = implode( ',', array_keys( $visitors ) ); // 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(); 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..dfb6045615 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 @@ -219,6 +219,7 @@ public static function store_url_metric( string $slug, OD_URL_Metric $new_url_me $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, + $new_url_metric->get_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/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index 92e96694aa..51d004ea07 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -146,6 +146,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( '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/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index cace578bc7..06b9dca0c0 100644 --- a/plugins/optimization-detective/tests/storage/test-rest-api.php +++ b/plugins/optimization-detective/tests/storage/test-rest-api.php @@ -525,6 +525,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['eTag'], od_get_breakpoint_max_widths(), od_get_url_metrics_breakpoint_sample_size(), HOUR_IN_SECONDS diff --git a/plugins/optimization-detective/tests/test-class-od-element.php b/plugins/optimization-detective/tests/test-class-od-element.php index 43a11e378a..b6ba62d154 100644 --- a/plugins/optimization-detective/tests/test-class-od-element.php +++ b/plugins/optimization-detective/tests/test-class-od-element.php @@ -71,7 +71,7 @@ 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 ); + $collection = new OD_URL_Metric_Group_Collection( array( $url_metric ), '', 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-metrics-group-collection.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php index 8553841aba..75330b98ca 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 @@ -22,6 +22,7 @@ public function data_provider_test_construction(): array { return array( 'no_breakpoints_ok' => array( 'url_metrics' => array(), + 'current_etag' => '', 'breakpoints' => array(), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -29,6 +30,7 @@ public function data_provider_test_construction(): array { ), 'negative_breakpoint_bad' => array( 'url_metrics' => array(), + 'current_etag' => '', 'breakpoints' => array( -1 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -36,6 +38,7 @@ public function data_provider_test_construction(): array { ), 'zero_breakpoint_bad' => array( 'url_metrics' => array(), + 'current_etag' => '', 'breakpoints' => array( 0 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -43,6 +46,7 @@ public function data_provider_test_construction(): array { ), 'max_breakpoint_bad' => array( 'url_metrics' => array(), + 'current_etag' => '', 'breakpoints' => array( PHP_INT_MAX ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -50,6 +54,7 @@ public function data_provider_test_construction(): array { ), 'string_breakpoint_bad' => array( 'url_metrics' => array(), + 'current_etag' => '', 'breakpoints' => array( 'narrow' ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -57,6 +62,7 @@ public function data_provider_test_construction(): array { ), 'negative_sample_size_bad' => array( 'url_metrics' => array(), + 'current_etag' => '', 'breakpoints' => array( 400 ), 'sample_size' => -3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -64,6 +70,7 @@ public function data_provider_test_construction(): array { ), 'negative_freshness_tll_bad' => array( 'url_metrics' => array(), + 'current_etag' => '', 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => -HOUR_IN_SECONDS, @@ -71,6 +78,7 @@ public function data_provider_test_construction(): array { ), 'invalid_url_metrics_bad' => array( 'url_metrics' => array( 'bad' ), + 'current_etag' => '', 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -81,6 +89,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' => '', 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -94,15 +103,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 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 +194,7 @@ 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 ); + $group_collection = new OD_URL_Metric_Group_Collection( array(), '', $breakpoints, $sample_size, HOUR_IN_SECONDS ); // Over-populate the sample size for the breakpoints by a dozen. foreach ( $viewport_widths as $viewport_width => $count ) { @@ -212,7 +224,7 @@ public function test_add_url_metric( int $sample_size, array $breakpoints, array public function test_adding_pushes_out_old_metrics(): void { $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(), '', $breakpoints, $sample_size, HOUR_IN_SECONDS ); // Populate the groups with stale URL Metrics. $viewport_widths = array( 300, 500, 700 ); @@ -333,7 +345,7 @@ function ( $viewport_width ) { $viewport_widths ); - $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $breakpoints, 3, HOUR_IN_SECONDS ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, '', $breakpoints, 3, HOUR_IN_SECONDS ); $this->assertCount( count( $breakpoints ) + 1, @@ -497,7 +509,7 @@ public function data_provider_test_get_group_for_viewport_width(): array { * @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 ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, '', $breakpoints, $sample_size, $freshness_ttl ); $this->assertSame( $expected_return, array_map( @@ -531,6 +543,7 @@ public function test_is_every_group_populated(): void { $sample_size = 3; $group_collection = new OD_URL_Metric_Group_Collection( array(), + '', $breakpoints, $sample_size, HOUR_IN_SECONDS @@ -594,6 +607,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 ), ), + '', $breakpoints, $sample_size, HOUR_IN_SECONDS @@ -625,6 +639,7 @@ public function test_get_common_lcp_element(): void { $sample_size = 3; $group_collection = new OD_URL_Metric_Group_Collection( array(), + '', $breakpoints, $sample_size, HOUR_IN_SECONDS @@ -737,7 +752,7 @@ public function data_provider_element_max_intersection_ratios(): array { public function test_get_all_element_max_intersection_ratios( array $url_metrics, array $expected ): void { $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, '', $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 ); @@ -914,7 +929,7 @@ public function data_provider_get_all_elements_positioned_in_any_initial_viewpor public function test_get_all_elements_positioned_in_any_initial_viewport( array $url_metrics, array $expected ): void { $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, '', $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 +953,7 @@ public function test_get_flattened_url_metrics(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, + '', array( 500, 700 ), 3, HOUR_IN_SECONDS @@ -965,6 +981,7 @@ public function test_json_serialize(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, + '', array( 500, 700 ), 3, HOUR_IN_SECONDS @@ -973,6 +990,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..e96da265d8 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 @@ -325,7 +325,7 @@ 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 ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, '', $breakpoints, 10, HOUR_IN_SECONDS ); $lcp_element_xpaths_by_minimum_viewport_widths = array(); foreach ( $group_collection as $group ) { diff --git a/plugins/optimization-detective/tests/test-detection.php b/plugins/optimization-detective/tests/test-detection.php index 6756cd80b0..e25c186ddc 100644 --- a/plugins/optimization-detective/tests/test-detection.php +++ b/plugins/optimization-detective/tests/test-detection.php @@ -130,7 +130,7 @@ public function test_od_get_detection_script_returns_script( Closure $set_up, ar $slug = od_get_url_metrics_slug( array( 'p' => '1' ) ); $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(), '', $breakpoints, 3, HOUR_IN_SECONDS ); $script = od_get_detection_script( $slug, $group_collection ); From a65fbad28c881084c03e451946c9748a5d55d371 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Wed, 27 Nov 2024 18:09:41 +0530 Subject: [PATCH 11/48] Fix test_od_optimize_template_output_buffer --- .../tests/test-cases/complete-url-metrics.php | 3 ++- .../tests/test-cases/many-images.php | 2 +- plugins/optimization-detective/tests/test-cases/video.php | 1 + tests/class-optimization-detective-test-helpers.php | 7 ++++++- 4 files changed, 10 insertions(+), 3 deletions(-) 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..83e8541bf7 100644 --- a/plugins/optimization-detective/tests/test-cases/complete-url-metrics.php +++ b/plugins/optimization-detective/tests/test-cases/complete-url-metrics.php @@ -9,7 +9,8 @@ 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', 'isLCP' => true, ), - ) + ), + 'img,video' ); }, 'buffer' => ' diff --git a/plugins/optimization-detective/tests/test-cases/many-images.php b/plugins/optimization-detective/tests/test-cases/many-images.php index a5526001cf..0d2a59cbca 100644 --- a/plugins/optimization-detective/tests/test-cases/many-images.php +++ b/plugins/optimization-detective/tests/test-cases/many-images.php @@ -8,7 +8,7 @@ 'isLCP' => false, ); } - $test_case->populate_url_metrics( $elements, false ); + $test_case->populate_url_metrics( $elements, 'img,video', false ); }, 'buffer' => ' diff --git a/plugins/optimization-detective/tests/test-cases/video.php b/plugins/optimization-detective/tests/test-cases/video.php index 038cd05d55..8090e029ea 100644 --- a/plugins/optimization-detective/tests/test-cases/video.php +++ b/plugins/optimization-detective/tests/test-cases/video.php @@ -8,6 +8,7 @@ 'isLCP' => true, ), ), + 'img,video', false ); }, diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index 95006c4165..c669264bbf 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -22,10 +22,11 @@ trait Optimization_Detective_Test_Helpers { * * @phpstan-param ElementDataSubset[] $elements * @param array[] $elements Element data. + * @param string $etag ETag to set for the URL metrics. * @param bool $complete Whether to fully populate the groups. * @throws Exception But it won't. */ - public function populate_url_metrics( array $elements, bool $complete = true ): void { + public function populate_url_metrics( array $elements, string $etag = '', bool $complete = true ): void { $slug = od_get_url_metrics_slug( od_get_normalized_query_vars() ); $sample_size = $complete ? od_get_url_metrics_breakpoint_sample_size() : 1; foreach ( array_merge( od_get_breakpoint_max_widths(), array( 1000 ) ) as $viewport_width ) { @@ -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,7 @@ public function get_sample_dom_rect(): array { * Gets a sample URL metric. * * @phpstan-param array{ + * eTag?: string, * url?: string, * viewport_width?: int, * viewport_height?: int, @@ -77,6 +80,7 @@ public function get_sample_dom_rect(): array { public function get_sample_url_metric( array $params ): OD_URL_Metric { $params = array_merge( array( + 'eTag' => '', 'url' => home_url( '/' ), 'viewport_width' => 480, 'elements' => array(), @@ -90,6 +94,7 @@ public function get_sample_url_metric( array $params ): OD_URL_Metric { return new OD_URL_Metric( array( + 'eTag' => $params['eTag'], 'url' => home_url( '/' ), 'viewport' => array( 'width' => $params['viewport_width'], From 5287f79e309ae4ba65139e7d8c9232fb0311ea28 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Wed, 27 Nov 2024 18:18:29 +0530 Subject: [PATCH 12/48] Fix test cases for embed-optimizer --- ...ngle-twitter-embed-inside-viewport-without-resized-data.php | 3 ++- .../tests/test-cases/single-twitter-embed-inside-viewport.php | 3 ++- .../single-twitter-embed-outside-viewport-on-mobile.php | 1 + .../tests/test-cases/single-twitter-embed-outside-viewport.php | 3 ++- .../tests/test-cases/single-youtube-embed-inside-viewport.php | 3 ++- .../single-youtube-embed-outside-viewport-on-mobile.php | 1 + .../tests/test-cases/single-youtube-embed-outside-viewport.php | 3 ++- 7 files changed, 12 insertions(+), 5 deletions(-) diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php index 37886051b9..a477e65586 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php @@ -9,7 +9,8 @@ 'intersectionRatio' => 1, // Intentionally omitting resizedBoundingClientRect here to test behavior when data isn't supplied. ), - ) + ), + 'embeds' ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php index 04b94117d3..bcf9d4d76d 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php @@ -9,7 +9,8 @@ 'intersectionRatio' => 1, 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), - ) + ), + 'embeds' ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php index 3c70222e49..37e9e87993 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php @@ -22,6 +22,7 @@ od_get_url_metrics_slug( od_get_normalized_query_vars() ), $test_case->get_sample_url_metric( array( + 'eTag' => 'embeds', 'viewport_width' => $viewport_width, 'elements' => $elements, ) diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php index ccdf5fbcfc..36e2ea087d 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php @@ -9,7 +9,8 @@ 'intersectionRatio' => 0, 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), - ) + ), + 'embeds' ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php index 5605d8c124..a6bb8384e0 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php @@ -9,7 +9,8 @@ 'intersectionRatio' => 1, 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), - ) + ), + 'embeds' ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php index da1c4b8b47..3371bf135a 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php @@ -22,6 +22,7 @@ od_get_url_metrics_slug( od_get_normalized_query_vars() ), $test_case->get_sample_url_metric( array( + 'eTag' => 'embeds', 'viewport_width' => $viewport_width, 'elements' => $elements, ) diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php index daef5292c6..7f7de84cb4 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php @@ -9,7 +9,8 @@ 'intersectionRatio' => 0, 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), - ) + ), + 'embeds' ); }, 'buffer' => ' From 86023ebe279f4c957e1bcd8d502cd3f7ec52f208 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Wed, 27 Nov 2024 18:19:20 +0530 Subject: [PATCH 13/48] Fix test-cases for image-prioritizer --- ...n-lcp-background-image-with-fully-populated-sample-data.php | 3 ++- ...image-outside-viewport-with-fully-populated-sample-data.php | 1 + .../test-cases/common-lcp-image-with-stale-sample-data.php | 3 ++- ...dy-on-common-lcp-image-with-fully-populated-sample-data.php | 3 ++- .../images-located-above-or-along-initial-viewport.php | 1 + .../test-cases/no-lcp-image-with-populated-url-metrics.php | 3 ++- .../tests/test-cases/responsive-background-images.php | 1 + plugins/image-prioritizer/tests/test-helper.php | 2 +- 8 files changed, 12 insertions(+), 5 deletions(-) diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php index b18b42520f..8849805f32 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php @@ -7,7 +7,8 @@ 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::DIV]', 'isLCP' => true, ), - ) + ), + 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php index b19f83b4d6..5e704606fe 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php @@ -15,6 +15,7 @@ $slug, $test_case->get_sample_url_metric( array( + 'eTag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', 'viewport_width' => $viewport_width, 'elements' => array( array( diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php index 5df495f7b5..b542c00a0d 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php @@ -7,7 +7,8 @@ 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', // Note: This is intentionally not reflecting the IMG in the HTML below. 'isLCP' => true, ), - ) + ), + 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php index b4c504b096..c69565cc47 100644 --- a/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php @@ -7,7 +7,8 @@ 'isLCP' => true, 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', ), - ) + ), + 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php b/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php index 2cfc863452..4b694666d9 100644 --- a/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php +++ b/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php @@ -37,6 +37,7 @@ $slug, $test_case->get_sample_url_metric( array( + 'eTag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', 'viewport_width' => $viewport_width, 'elements' => array( array( diff --git a/plugins/image-prioritizer/tests/test-cases/no-lcp-image-with-populated-url-metrics.php b/plugins/image-prioritizer/tests/test-cases/no-lcp-image-with-populated-url-metrics.php index eb9fe4eb6b..bd60079243 100644 --- a/plugins/image-prioritizer/tests/test-cases/no-lcp-image-with-populated-url-metrics.php +++ b/plugins/image-prioritizer/tests/test-cases/no-lcp-image-with-populated-url-metrics.php @@ -7,7 +7,8 @@ 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::H1]', 'isLCP' => true, ), - ) + ), + 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php b/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php index 1f9d515776..55339c5f39 100644 --- a/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php +++ b/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php @@ -25,6 +25,7 @@ static function () use ( $mobile_breakpoint, $tablet_breakpoint ): array { $slug, $test_case->get_sample_url_metric( array( + 'eTag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', 'viewport_width' => $viewport_width, 'element' => array( 'xpath' => sprintf( '/*[1][self::HTML]/*[2][self::BODY]/*[%d][self::DIV]', $div_index + 1 ), diff --git a/plugins/image-prioritizer/tests/test-helper.php b/plugins/image-prioritizer/tests/test-helper.php index ac67ac867e..f71d9edef1 100644 --- a/plugins/image-prioritizer/tests/test-helper.php +++ b/plugins/image-prioritizer/tests/test-helper.php @@ -205,7 +205,7 @@ public function data_provider_test_auto_sizes(): array { * @phpstan-param array{ xpath: string, isLCP: bool, intersectionRatio: int } $element_metrics */ public function test_auto_sizes( array $element_metrics, string $buffer, string $expected ): void { - $this->populate_url_metrics( array( $element_metrics ) ); + $this->populate_url_metrics( array( $element_metrics ), 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ); $html_start_doc = '...'; $html_end_doc = ''; From 2eb6d98fd99f531811516ecad7a71f33fa6c089d Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Thu, 28 Nov 2024 12:45:01 +0530 Subject: [PATCH 14/48] Add comment to clarify future intent for ETag property Co-authored-by: Weston Ruter --- plugins/optimization-detective/class-od-url-metric.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index 5093e1c605..cbd974bea1 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -213,7 +213,7 @@ public static function get_json_schema(): array { 'eTag' => array( 'description' => __( 'The ETag for the URL Metric.', 'optimization-detective' ), 'type' => 'string', - 'required' => false, + 'required' => false, // To be made required in a future release. 'readonly' => true, // Omit from REST API. ), 'url' => array( From 2c7aee5c3183d16c574ca2254b9826933f0d45bb Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Thu, 28 Nov 2024 13:01:17 +0530 Subject: [PATCH 15/48] Update `get_etag` method to return `null` when ETag is absent Co-authored-by: Weston Ruter --- plugins/optimization-detective/class-od-url-metric.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index cbd974bea1..c565b1595d 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -430,11 +430,11 @@ public function get_uuid(): string { * * @since n.e.x.t * - * @return string ETag. + * @return string|null ETag. */ - public function get_etag(): string { - // Since the ETag is optional for now, return an empty string for old URL metrics that do not have one. - return $this->data['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; } /** From 821b48dcd6782a4ccf6499dd4be433c23205eeb6 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Thu, 28 Nov 2024 20:38:11 +0530 Subject: [PATCH 16/48] Handle PHPStan error for possible null ETag in store_url_metric() method --- .../storage/class-od-url-metrics-post-type.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 dfb6045615..3032ee3519 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 @@ -219,7 +219,12 @@ public static function store_url_metric( string $slug, OD_URL_Metric $new_url_me $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, - $new_url_metric->get_etag(), + // The null coalescing operator (??) is used here to handle cases where the get_etag() method returns null. + // This can occur because the ETag is currently optional and may not exist for some URL Metrics. + // However, in the context of this store_url_metric() method, which is called by the REST API callback, + // the ETag is a required field and should never be null. This usage satisfies PHPStan's requirements + // until a future release where get_etag() will always return a string. + $new_url_metric->get_etag() ?? '', od_get_breakpoint_max_widths(), od_get_url_metrics_breakpoint_sample_size(), od_get_url_metric_freshness_ttl() From 52c486f9da424735bbbb3034fc0c2c64e7fc2896 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Thu, 28 Nov 2024 21:25:27 +0530 Subject: [PATCH 17/48] Improve variable naming --- plugins/optimization-detective/detection.php | 6 ++++-- plugins/optimization-detective/storage/data.php | 12 ++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/plugins/optimization-detective/detection.php b/plugins/optimization-detective/detection.php index 9532351908..2fa2a6dee7 100644 --- a/plugins/optimization-detective/detection.php +++ b/plugins/optimization-detective/detection.php @@ -86,17 +86,19 @@ function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $ $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' => $group_collection->get_current_etag(), + 'currentETag' => $current_etag, 'currentUrl' => $current_url, 'urlMetricSlug' => $slug, 'cachePurgePostId' => od_get_cache_purge_post_id(), - 'urlMetricHMAC' => od_get_url_metrics_storage_hmac( $slug, $group_collection->get_current_etag(), $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/storage/data.php b/plugins/optimization-detective/storage/data.php index 78a00deb80..6e97439afc 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -152,13 +152,13 @@ function od_get_url_metrics_slug( array $query_vars ): string { * @see od_get_url_metrics_slug() * * @param string $slug Slug (hash of normalized query vars). - * @param string $etag ETag. + * @param string $current_tag 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 $etag, string $url, ?int $cache_purge_post_id = null ): string { - $action = "store_url_metric:$slug:$etag:$url:$cache_purge_post_id"; +function od_get_url_metrics_storage_hmac( string $slug, string $current_tag, string $url, ?int $cache_purge_post_id = null ): string { + $action = "store_url_metric:$slug:$current_tag:$url:$cache_purge_post_id"; return wp_hash( $action, 'nonce' ); } @@ -173,13 +173,13 @@ function od_get_url_metrics_storage_hmac( string $slug, string $etag, string $ur * * @param string $hmac HMAC. * @param string $slug Slug (hash of normalized query vars). - * @param string $etag ETag. + * @param 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 $etag, string $url, ?int $cache_purge_post_id = null ): bool { - return hash_equals( od_get_url_metrics_storage_hmac( $slug, $etag, $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 ); } /** From 63758ef0fb3bcf5942a2f354fe4ef5b57e0dc31c Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Thu, 28 Nov 2024 21:27:30 +0530 Subject: [PATCH 18/48] Add missing since tags --- plugins/optimization-detective/storage/data.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/optimization-detective/storage/data.php b/plugins/optimization-detective/storage/data.php index 6e97439afc..5094743df0 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -146,6 +146,7 @@ function od_get_url_metrics_slug( array $query_vars ): string { * 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() @@ -166,6 +167,7 @@ function od_get_url_metrics_storage_hmac( string $slug, string $current_tag, str * 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() From 08f8f8a06e37e8484daa6b66d7904aab6c133a35 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Thu, 28 Nov 2024 21:30:47 +0530 Subject: [PATCH 19/48] Improve naming for URL Metric schema properties --- plugins/optimization-detective/class-od-url-metric.php | 8 ++++---- plugins/optimization-detective/storage/rest-api.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index c565b1595d..a9ba604de2 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -38,7 +38,7 @@ * } * @phpstan-type Data array{ * uuid: non-empty-string, - * eTag?: string, + * etag?: string, * url: non-empty-string, * timestamp: float, * viewport: ViewportRect, @@ -156,7 +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. + * @since n.e.x.t Added the 'etag' property to the schema. * * @todo Cache the return value? * @@ -210,7 +210,7 @@ public static function get_json_schema(): array { 'required' => true, 'readonly' => true, // Omit from REST API. ), - 'eTag' => array( + 'etag' => array( 'description' => __( 'The ETag for the URL Metric.', 'optimization-detective' ), 'type' => 'string', 'required' => false, // To be made required in a future release. @@ -434,7 +434,7 @@ public function get_uuid(): string { */ 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; + return $this->data['etag'] ?? null; } /** diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index 51d004ea07..9b3b962cae 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -188,7 +188,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( 'eTag' ), + 'etag' => $request->get_param( 'eTag' ), ) ) ); From 371e998af1344eca5b6d75c768826700906b40ae Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Thu, 28 Nov 2024 21:31:10 +0530 Subject: [PATCH 20/48] Improve naming for REST API endpoint args --- plugins/optimization-detective/detect.js | 4 ++-- plugins/optimization-detective/storage/rest-api.php | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index 8a77b867a6..385f81b0d5 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -237,7 +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 ETag for URL Metric. + * @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. @@ -541,7 +541,7 @@ export default async function detect( { const url = new URL( restApiEndpoint ); url.searchParams.set( 'slug', urlMetricSlug ); - url.searchParams.set( 'eTag', currentETag ); + url.searchParams.set( 'current_etag', currentETag ); if ( typeof cachePurgePostId === 'number' ) { url.searchParams.set( 'cache_purge_post_id', diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index 9b3b962cae..b705bf53dd 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -46,7 +46,7 @@ function od_register_endpoint(): void { 'required' => true, 'pattern' => '^[0-9a-f]{32}$', ), - 'eTag' => array( + 'current_etag' => array( 'type' => 'string', 'description' => __( 'ETag for the current environment.', 'optimization-detective' ), 'required' => true, @@ -63,7 +63,7 @@ function od_register_endpoint(): void { 'required' => true, 'pattern' => '^[0-9a-f]+$', 'validate_callback' => static function ( string $hmac, WP_REST_Request $request ) { - if ( ! od_verify_url_metrics_storage_hmac( $hmac, $request['slug'], $request['eTag'], $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; @@ -146,7 +146,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( 'eTag' ), + $request->get_param( 'current_etag' ), od_get_breakpoint_max_widths(), od_get_url_metrics_breakpoint_sample_size(), od_get_url_metric_freshness_ttl() @@ -188,7 +188,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( 'eTag' ), + 'etag' => $request->get_param( 'current_etag' ), ) ) ); From ba2ea9347f7f2c035e4cb067690efff84e814f32 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Thu, 28 Nov 2024 22:05:01 +0530 Subject: [PATCH 21/48] Update test cases to use the new property and arg names --- ...twitter-embed-outside-viewport-on-mobile.php | 2 +- ...youtube-embed-outside-viewport-on-mobile.php | 2 +- ...iewport-with-fully-populated-sample-data.php | 2 +- ...-located-above-or-along-initial-viewport.php | 2 +- .../test-cases/responsive-background-images.php | 2 +- .../tests/storage/test-rest-api.php | 17 +++++++++-------- .../tests/test-class-od-url-metric.php | 4 ++-- ...lass-optimization-detective-test-helpers.php | 8 ++++---- 8 files changed, 20 insertions(+), 19 deletions(-) diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php index 37e9e87993..abe89a61c8 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php @@ -22,7 +22,7 @@ od_get_url_metrics_slug( od_get_normalized_query_vars() ), $test_case->get_sample_url_metric( array( - 'eTag' => 'embeds', + 'etag' => 'embeds', 'viewport_width' => $viewport_width, 'elements' => $elements, ) diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php index 3371bf135a..686be6ad15 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php @@ -22,7 +22,7 @@ od_get_url_metrics_slug( od_get_normalized_query_vars() ), $test_case->get_sample_url_metric( array( - 'eTag' => 'embeds', + 'etag' => 'embeds', 'viewport_width' => $viewport_width, 'elements' => $elements, ) diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php index 5e704606fe..e1b84cb5d5 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php @@ -15,7 +15,7 @@ $slug, $test_case->get_sample_url_metric( array( - 'eTag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', + 'etag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', 'viewport_width' => $viewport_width, 'elements' => array( array( diff --git a/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php b/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php index 4b694666d9..a581ecb890 100644 --- a/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php +++ b/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php @@ -37,7 +37,7 @@ $slug, $test_case->get_sample_url_metric( array( - 'eTag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', + 'etag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', 'viewport_width' => $viewport_width, 'elements' => array( array( diff --git a/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php b/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php index 55339c5f39..47f21a8753 100644 --- a/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php +++ b/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php @@ -25,7 +25,7 @@ static function () use ( $mobile_breakpoint, $tablet_breakpoint ): array { $slug, $test_case->get_sample_url_metric( array( - 'eTag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', + 'etag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', 'viewport_width' => $viewport_width, 'element' => array( 'xpath' => sprintf( '/*[1][self::HTML]/*[2][self::BODY]/*[%d][self::DIV]', $div_index + 1 ), diff --git a/plugins/optimization-detective/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index 06b9dca0c0..fac5bc5056 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['eTag'], $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 ) ) @@ -525,7 +525,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['eTag'], + $wider_viewport_params['current_etag'], od_get_breakpoint_max_widths(), od_get_url_metrics_breakpoint_sample_size(), HOUR_IN_SECONDS @@ -669,14 +669,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['eTag'], $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 ); } @@ -721,9 +722,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-class-od-url-metric.php b/plugins/optimization-detective/tests/test-class-od-url-metric.php index 4dee9a1346..87e28891b6 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metric.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metric.php @@ -715,8 +715,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 ); - // Skipping the check for 'root/eTag' as it is currently optional. - if ( ! $extended && 'root/eTag' !== $path ) { + // 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/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index c669264bbf..17c5465813 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -35,7 +35,7 @@ public function populate_url_metrics( array $elements, string $etag = '', bool $ $slug, $this->get_sample_url_metric( array( - 'eTag' => $etag, + 'etag' => $etag, 'viewport_width' => $viewport_width, 'elements' => $elements, ) @@ -67,7 +67,7 @@ public function get_sample_dom_rect(): array { * Gets a sample URL metric. * * @phpstan-param array{ - * eTag?: string, + * etag?: string, * url?: string, * viewport_width?: int, * viewport_height?: int, @@ -80,7 +80,7 @@ public function get_sample_dom_rect(): array { public function get_sample_url_metric( array $params ): OD_URL_Metric { $params = array_merge( array( - 'eTag' => '', + 'etag' => '', 'url' => home_url( '/' ), 'viewport_width' => 480, 'elements' => array(), @@ -94,7 +94,7 @@ public function get_sample_url_metric( array $params ): OD_URL_Metric { return new OD_URL_Metric( array( - 'eTag' => $params['eTag'], + 'etag' => $params['etag'], 'url' => home_url( '/' ), 'viewport' => array( 'width' => $params['viewport_width'], From 8fe38f7d6bc48c7c9e7b45c7efe50eb78aa4f156 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Fri, 29 Nov 2024 01:36:58 +0530 Subject: [PATCH 22/48] Introduce new function to compute the current ETag --- .../class-od-url-metric-group.php | 2 +- .../optimization-detective/optimization.php | 5 ++--- .../class-od-url-metrics-post-type.php | 2 +- .../optimization-detective/storage/data.php | 20 +++++++++++++++++++ 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 2528964763..56cea0dc32 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -239,7 +239,7 @@ public function is_complete(): bool { $current_time > $url_metric->get_timestamp() + $this->freshness_ttl || // If the URL Metric's ETag doesn't match the current ETag, consider the URL metric as stale. - ( null !== $this->collection && $url_metric->get_etag() !== $this->collection->get_current_etag() ) + ( null !== $this->collection && ! hash_equals( $url_metric->get_etag() ?? md5( '' ), $this->collection->get_current_etag() ) ) ) { return false; } diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index cff7948fed..0fd23d86aa 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -206,9 +206,7 @@ function od_optimize_template_output_buffer( string $buffer ): string { */ do_action( 'od_register_tag_visitors', $tag_visitor_registry ); - $visitors = iterator_to_array( $tag_visitor_registry ); - $current_etag = implode( ',', array_keys( $visitors ) ); - + $current_etag = od_compute_current_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, @@ -219,6 +217,7 @@ function od_optimize_template_output_buffer( string $buffer ): string { $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(); 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 3032ee3519..2e66fe4623 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 @@ -224,7 +224,7 @@ public static function store_url_metric( string $slug, OD_URL_Metric $new_url_me // However, in the context of this store_url_metric() method, which is called by the REST API callback, // the ETag is a required field and should never be null. This usage satisfies PHPStan's requirements // until a future release where get_etag() will always return a string. - $new_url_metric->get_etag() ?? '', + $new_url_metric->get_etag() ?? md5( '' ), 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 5094743df0..6d75542777 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -140,6 +140,26 @@ function od_get_url_metrics_slug( array $query_vars ): string { return md5( (string) wp_json_encode( $query_vars ) ); } +/** + * Computes 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 string Current ETag. + */ +function od_compute_current_etag( OD_Tag_Visitor_Registry $tag_visitor_registry ): string { + $data = array( + 'tag_visitors' => array_keys( iterator_to_array( $tag_visitor_registry ) ), + ); + return md5( serialize( $data ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize +} + /** * Computes HMAC for storing URL Metrics for a specific slug. * From e6078a9976d433654c915b31af5b4f75f3181c9c Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Fri, 29 Nov 2024 01:45:01 +0530 Subject: [PATCH 23/48] Add pattern validation for ETag in schema --- plugins/optimization-detective/class-od-url-metric.php | 3 +++ plugins/optimization-detective/storage/rest-api.php | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index a9ba604de2..ad621a740c 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -213,6 +213,9 @@ public static function get_json_schema(): array { 'etag' => array( 'description' => __( 'The ETag for the URL Metric.', 'optimization-detective' ), 'type' => 'string', + 'pattern' => '^[0-9a-f]{32}$', + 'minLength' => 32, + 'maxLength' => 32, 'required' => false, // To be made required in a future release. 'readonly' => true, // Omit from REST API. ), diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index b705bf53dd..ba8bf02259 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -45,11 +45,16 @@ function od_register_endpoint(): void { 'description' => __( 'An MD5 hash of the query args.', 'optimization-detective' ), 'required' => true, 'pattern' => '^[0-9a-f]{32}$', + 'minLength' => 32, + 'maxLength' => 32, ), 'current_etag' => array( 'type' => 'string', 'description' => __( 'ETag for the current environment.', 'optimization-detective' ), 'required' => true, + 'pattern' => '^[0-9a-f]{32}$', + 'minLength' => 32, + 'maxLength' => 32, ), 'cache_purge_post_id' => array( 'type' => 'integer', From 309f9bd470f40cddbc8e02ceac562a82fe17905a Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Fri, 29 Nov 2024 02:22:38 +0530 Subject: [PATCH 24/48] Update test cases to use the new ETag format --- .../tests/storage/test-rest-api.php | 5 +- .../tests/test-cases/complete-url-metrics.php | 6 ++- .../tests/test-cases/many-images.php | 7 ++- .../tests/test-cases/video.php | 6 ++- .../tests/test-class-od-element.php | 3 +- ...-class-od-url-metrics-group-collection.php | 51 +++++++++++-------- .../tests/test-class-od-url-metrics-group.php | 3 +- .../tests/test-detection.php | 5 +- ...ss-optimization-detective-test-helpers.php | 11 ++-- 9 files changed, 63 insertions(+), 34 deletions(-) diff --git a/plugins/optimization-detective/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index fac5bc5056..a7cb38184a 100644 --- a/plugins/optimization-detective/tests/storage/test-rest-api.php +++ b/plugins/optimization-detective/tests/storage/test-rest-api.php @@ -138,6 +138,7 @@ function ( OD_URL_Metric_Store_Request_Context $context ) use ( &$stored_context */ public function data_provider_invalid_params(): array { $valid_element = $this->get_valid_params()['elements'][0]; + $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); return array_map( function ( $params ) { @@ -156,10 +157,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' ) ), $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() ), $current_etag, home_url( '/' ), 1 ), ), 'invalid_viewport_type' => array( 'viewport' => '640x480', 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 83e8541bf7..b3b0e8bab4 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,10 @@ '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 + $tag_visitor_registry = new OD_Tag_Visitor_Registry(); + $tag_visitor_registry->register( 'img', static function (): void {} ); + $tag_visitor_registry->register( 'video', static function (): void {} ); + $test_case->populate_url_metrics( array( array( @@ -10,7 +14,7 @@ 'isLCP' => true, ), ), - 'img,video' + od_compute_current_etag( $tag_visitor_registry ) ); }, 'buffer' => ' diff --git a/plugins/optimization-detective/tests/test-cases/many-images.php b/plugins/optimization-detective/tests/test-cases/many-images.php index 0d2a59cbca..1b42acd506 100644 --- a/plugins/optimization-detective/tests/test-cases/many-images.php +++ b/plugins/optimization-detective/tests/test-cases/many-images.php @@ -8,7 +8,12 @@ 'isLCP' => false, ); } - $test_case->populate_url_metrics( $elements, 'img,video', false ); + + $tag_visitor_registry = new OD_Tag_Visitor_Registry(); + $tag_visitor_registry->register( 'img', static function (): void {} ); + $tag_visitor_registry->register( 'video', static function (): void {} ); + + $test_case->populate_url_metrics( $elements, od_compute_current_etag( $tag_visitor_registry ), false ); }, 'buffer' => ' diff --git a/plugins/optimization-detective/tests/test-cases/video.php b/plugins/optimization-detective/tests/test-cases/video.php index 8090e029ea..68c497eed2 100644 --- a/plugins/optimization-detective/tests/test-cases/video.php +++ b/plugins/optimization-detective/tests/test-cases/video.php @@ -1,6 +1,10 @@ static function ( Test_OD_Optimization $test_case ): void { + $tag_visitor_registry = new OD_Tag_Visitor_Registry(); + $tag_visitor_registry->register( 'img', static function (): void {} ); + $tag_visitor_registry->register( 'video', static function (): void {} ); + $test_case->populate_url_metrics( array( array( @@ -8,7 +12,7 @@ 'isLCP' => true, ), ), - 'img,video', + od_compute_current_etag( $tag_visitor_registry ), false ); }, diff --git a/plugins/optimization-detective/tests/test-class-od-element.php b/plugins/optimization-detective/tests/test-class-od-element.php index b6ba62d154..c13fd11edc 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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $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-metrics-group-collection.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php index 75330b98ca..87914b795d 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,10 +19,12 @@ class Test_OD_URL_Metric_Group_Collection extends WP_UnitTestCase { * @return array Data. */ public function data_provider_test_construction(): array { + $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + return array( 'no_breakpoints_ok' => array( 'url_metrics' => array(), - 'current_etag' => '', + 'current_etag' => $current_etag, 'breakpoints' => array(), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -30,7 +32,7 @@ public function data_provider_test_construction(): array { ), 'negative_breakpoint_bad' => array( 'url_metrics' => array(), - 'current_etag' => '', + 'current_etag' => $current_etag, 'breakpoints' => array( -1 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -38,7 +40,7 @@ public function data_provider_test_construction(): array { ), 'zero_breakpoint_bad' => array( 'url_metrics' => array(), - 'current_etag' => '', + 'current_etag' => $current_etag, 'breakpoints' => array( 0 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -46,7 +48,7 @@ public function data_provider_test_construction(): array { ), 'max_breakpoint_bad' => array( 'url_metrics' => array(), - 'current_etag' => '', + 'current_etag' => $current_etag, 'breakpoints' => array( PHP_INT_MAX ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -54,7 +56,7 @@ public function data_provider_test_construction(): array { ), 'string_breakpoint_bad' => array( 'url_metrics' => array(), - 'current_etag' => '', + 'current_etag' => $current_etag, 'breakpoints' => array( 'narrow' ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -62,7 +64,7 @@ public function data_provider_test_construction(): array { ), 'negative_sample_size_bad' => array( 'url_metrics' => array(), - 'current_etag' => '', + 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), 'sample_size' => -3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -70,7 +72,7 @@ public function data_provider_test_construction(): array { ), 'negative_freshness_tll_bad' => array( 'url_metrics' => array(), - 'current_etag' => '', + 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => -HOUR_IN_SECONDS, @@ -78,7 +80,7 @@ public function data_provider_test_construction(): array { ), 'invalid_url_metrics_bad' => array( 'url_metrics' => array( 'bad' ), - 'current_etag' => '', + 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -89,7 +91,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' => $current_etag, 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => HOUR_IN_SECONDS, @@ -194,7 +196,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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $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 ) { @@ -222,9 +225,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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); $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 ); @@ -345,7 +349,8 @@ function ( $viewport_width ) { $viewport_widths ); - $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, '', $breakpoints, 3, HOUR_IN_SECONDS ); + $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, 3, HOUR_IN_SECONDS ); $this->assertCount( count( $breakpoints ) + 1, @@ -509,7 +514,8 @@ public function data_provider_test_get_group_for_viewport_width(): array { * @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 ); + $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, $freshness_ttl ); $this->assertSame( $expected_return, array_map( @@ -541,9 +547,10 @@ 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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( array(), - '', + $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS @@ -595,6 +602,7 @@ public function test_get_groups_by_lcp_element(): void { $breakpoints = array( 480, 800 ); $sample_size = 3; + $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( array( // Group 1: 0-480 viewport widths. @@ -607,7 +615,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 @@ -637,9 +645,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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( array(), - '', + $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS @@ -750,9 +759,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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); $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 ); @@ -927,9 +937,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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); $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 ); @@ -953,7 +964,7 @@ public function test_get_flattened_url_metrics(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, - '', + od_compute_current_etag( new OD_Tag_Visitor_Registry() ), array( 500, 700 ), 3, HOUR_IN_SECONDS @@ -981,7 +992,7 @@ public function test_json_serialize(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, - '', + od_compute_current_etag( new OD_Tag_Visitor_Registry() ), array( 500, 700 ), 3, HOUR_IN_SECONDS 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 e96da265d8..d6dca74d37 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 @@ -325,7 +325,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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $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 ) { diff --git a/plugins/optimization-detective/tests/test-detection.php b/plugins/optimization-detective/tests/test-detection.php index e25c186ddc..618315c2d5 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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); $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 17c5465813..7f5fe4c36c 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -21,13 +21,14 @@ trait Optimization_Detective_Test_Helpers { * Populates complete URL metrics for the provided element data. * * @phpstan-param ElementDataSubset[] $elements - * @param array[] $elements Element data. - * @param string $etag ETag to set for the URL metrics. - * @param bool $complete Whether to fully populate the groups. + * @param array[] $elements Element data. + * @param string|null $etag ETag to set for the URL metrics. + * @param bool $complete Whether to fully populate the groups. * @throws Exception But it won't. */ - public function populate_url_metrics( array $elements, string $etag = '', bool $complete = true ): void { + public function populate_url_metrics( array $elements, ?string $etag, bool $complete = true ): void { $slug = od_get_url_metrics_slug( od_get_normalized_query_vars() ); + $etag = $etag ?? od_compute_current_etag( new OD_Tag_Visitor_Registry() ); $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++ ) { @@ -80,7 +81,7 @@ public function get_sample_dom_rect(): array { public function get_sample_url_metric( array $params ): OD_URL_Metric { $params = array_merge( array( - 'etag' => '', + 'etag' => od_compute_current_etag( new OD_Tag_Visitor_Registry() ), 'url' => home_url( '/' ), 'viewport_width' => 480, 'elements' => array(), From d1458d8878126fd600126e19b4095701d3d174bc Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Sun, 1 Dec 2024 06:58:41 +0530 Subject: [PATCH 25/48] Refactor staleness check logic for URL Metrics Co-authored-by: Weston Ruter --- .../class-od-url-metric-group.php | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 56cea0dc32..c3cdb7ebe6 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -235,11 +235,21 @@ 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 ( - $current_time > $url_metric->get_timestamp() + $this->freshness_ttl - || - // If the URL Metric's ETag doesn't match the current ETag, consider the URL metric as stale. - ( null !== $this->collection && ! hash_equals( $url_metric->get_etag() ?? md5( '' ), $this->collection->get_current_etag() ) ) + $this->collection instanceof OD_URL_Metric_Group_Collection + && + ! hash_equals( $url_metric->get_etag(), $this->collection->get_current_etag() ) ) { return false; } From c4eb6138c55be58c0da935fb3199c6cf5b0b700f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 17:34:20 -0800 Subject: [PATCH 26/48] Use non-empty-string as more precise type for ETag --- .../class-od-url-metric-group-collection.php | 16 ++++++++-------- .../class-od-url-metric.php | 4 ++-- plugins/optimization-detective/storage/data.php | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) 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 2c1ced1e77..ce528ec77c 100644 --- a/plugins/optimization-detective/class-od-url-metric-group-collection.php +++ b/plugins/optimization-detective/class-od-url-metric-group-collection.php @@ -36,10 +36,10 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega private $groups; /** - * The current Etag. + * The current ETag. * * @since n.e.x.t - * @var string + * @var non-empty-string */ private $current_etag; @@ -101,11 +101,11 @@ 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 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. + * @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, string $current_etag, array $breakpoints, int $sample_size, int $freshness_ttl ) { $this->current_etag = $current_etag; @@ -176,7 +176,7 @@ public function __construct( array $url_metrics, string $current_etag, array $br * * @since n.e.x.t * - * @return string Current ETag. + * @return non-empty-string Current ETag. */ public function get_current_etag(): string { return $this->current_etag; diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index ad621a740c..68a5555f4a 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -38,7 +38,7 @@ * } * @phpstan-type Data array{ * uuid: non-empty-string, - * etag?: string, + * etag?: non-empty-string, * url: non-empty-string, * timestamp: float, * viewport: ViewportRect, @@ -433,7 +433,7 @@ public function get_uuid(): string { * * @since n.e.x.t * - * @return string|null ETag. + * @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. diff --git a/plugins/optimization-detective/storage/data.php b/plugins/optimization-detective/storage/data.php index 6d75542777..2c75f1c7d2 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -151,7 +151,7 @@ function od_get_url_metrics_slug( array $query_vars ): string { * @access private * * @param OD_Tag_Visitor_Registry $tag_visitor_registry Tag visitor registry. - * @return string Current ETag. + * @return non-empty-string Current ETag. */ function od_compute_current_etag( OD_Tag_Visitor_Registry $tag_visitor_registry ): string { $data = array( From 695eecb049a9ed7090ace9fc2dfdea26c4896950 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 17:52:53 -0800 Subject: [PATCH 27/48] Refine ETag parameter in more places --- .../class-od-url-metric-group-collection.php | 2 +- .../optimization-detective/storage/data.php | 22 +++++++++---------- ...-class-od-url-metrics-group-collection.php | 12 +++++----- 3 files changed, 18 insertions(+), 18 deletions(-) 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 ce528ec77c..08457f0283 100644 --- a/plugins/optimization-detective/class-od-url-metric-group-collection.php +++ b/plugins/optimization-detective/class-od-url-metric-group-collection.php @@ -635,7 +635,7 @@ public function count(): int { * @since 0.3.1 * * @return array{ - * current_etag: string, + * current_etag: non-empty-string, * breakpoints: positive-int[], * freshness_ttl: 0|positive-int, * sample_size: positive-int, diff --git a/plugins/optimization-detective/storage/data.php b/plugins/optimization-detective/storage/data.php index 2c75f1c7d2..0233c5001e 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -172,14 +172,14 @@ function od_compute_current_etag( OD_Tag_Visitor_Registry $tag_visitor_registry * @see od_verify_url_metrics_storage_hmac() * @see od_get_url_metrics_slug() * - * @param string $slug Slug (hash of normalized query vars). - * @param string $current_tag Current ETag. - * @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 $current_tag, string $url, ?int $cache_purge_post_id = null ): string { - $action = "store_url_metric:$slug:$current_tag:$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' ); } @@ -193,11 +193,11 @@ function od_get_url_metrics_storage_hmac( string $slug, string $current_tag, str * @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 $current_etag Current ETag. - * @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 $current_etag, string $url, ?int $cache_purge_post_id = null ): bool { 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 87914b795d..a706b2a833 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 @@ -105,12 +105,12 @@ public function data_provider_test_construction(): array { * * @dataProvider data_provider_test_construction * - * @param OD_URL_Metric[] $url_metrics URL Metrics. - * @param 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. + * @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, string $current_etag, array $breakpoints, int $sample_size, int $freshness_ttl, string $exception ): void { if ( '' !== $exception ) { From 5a36beba408e1e29796a7c2f2a5532c475645260 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 18:10:20 -0800 Subject: [PATCH 28/48] Refactor static analysis accommodation code for when ETag is null --- .../storage/class-od-url-metrics-post-type.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) 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 2e66fe4623..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,14 +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, - // The null coalescing operator (??) is used here to handle cases where the get_etag() method returns null. - // This can occur because the ETag is currently optional and may not exist for some URL Metrics. - // However, in the context of this store_url_metric() method, which is called by the REST API callback, - // the ETag is a required field and should never be null. This usage satisfies PHPStan's requirements - // until a future release where get_etag() will always return a string. - $new_url_metric->get_etag() ?? md5( '' ), + $etag, od_get_breakpoint_max_widths(), od_get_url_metrics_breakpoint_sample_size(), od_get_url_metric_freshness_ttl() From d855ae881cd108221aba8cfc7ee51635ac50b735 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 18:11:56 -0800 Subject: [PATCH 29/48] Fix test_add_url_metric --- .../tests/test-class-od-url-metrics-group.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 d6dca74d37..c26f1efe72 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 @@ -199,13 +199,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 ), From 5e395810d0cd791937ba6731473db6fd22e72707 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 18:41:29 -0800 Subject: [PATCH 30/48] Eliminate collection being an optional arg when constructing an OD_URL_Metrics_Group --- .../class-od-url-metric-group.php | 37 ++++++------- .../tests/test-class-od-url-metric.php | 7 ++- .../tests/test-class-od-url-metrics-group.php | 54 ++++++++++++------- 3 files changed, 55 insertions(+), 43 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index c3cdb7ebe6..5ea0b942b1 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. Optional. */ - 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; @@ -246,11 +243,7 @@ public function is_complete(): bool { } // The ETag of the URL Metric does not match the current ETag for the collection, so it is stale. - if ( - $this->collection instanceof OD_URL_Metric_Group_Collection - && - ! hash_equals( $url_metric->get_etag(), $this->collection->get_current_etag() ) - ) { + if ( ! hash_equals( $url_metric->get_etag(), $this->collection->get_current_etag() ) ) { return false; } } 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 87e28891b6..0aa41824c9 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metric.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metric.php @@ -220,7 +220,12 @@ 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 ); + $groups = iterator_to_array( $collection ); + $this->assertCount( 1, $groups ); + $this->assertInstanceOf( OD_URL_Metric_Group::class, $groups[0] ); + $group = $groups[0]; $url_metric->set_group( $group ); $this->assertSame( $group, $url_metric->get_group() ); 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 c26f1efe72..1dcc393f4e 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" ); } @@ -353,19 +368,18 @@ 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 = iterator_to_array( $collection )[0]; + $this->assertInstanceOf( OD_URL_Metric_Group::class, $group ); + $json = wp_json_encode( $group ); $parsed_json = json_decode( $json, true ); $expected_keys = array( From a430c5602155753be1a239512a24da468397c0c6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 18:46:54 -0800 Subject: [PATCH 31/48] Supply null as etag param to address static analysis complaint --- ...image-outside-viewport-with-fully-populated-sample-data.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-and-lazy-loaded-background-image-outside-viewport-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-and-lazy-loaded-background-image-outside-viewport-with-fully-populated-sample-data.php index 6e2f2f9b7b..bc193ea443 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-and-lazy-loaded-background-image-outside-viewport-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-and-lazy-loaded-background-image-outside-viewport-with-fully-populated-sample-data.php @@ -28,7 +28,8 @@ 'intersectionRect' => $outside_viewport_rect, 'boundingClientRect' => $outside_viewport_rect, ), - ) + ), + null ); }, 'buffer' => ' From bcc34c934a78eaab4872f3bdb7239a966dff1a3b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 19:01:35 -0800 Subject: [PATCH 32/48] Temporarily work around errors related to non-hashes being supplied for ETag --- .../embed-optimizer/tests/test-cases/nested-figure-embed.php | 2 +- ...le-spotify-embed-outside-viewport-with-subsequent-script.php | 2 +- ...ingle-twitter-embed-inside-viewport-without-resized-data.php | 2 +- .../tests/test-cases/single-twitter-embed-inside-viewport.php | 2 +- .../single-twitter-embed-outside-viewport-on-mobile.php | 2 +- .../tests/test-cases/single-twitter-embed-outside-viewport.php | 2 +- .../test-cases/single-wordpress-tv-embed-inside-viewport.php | 2 +- .../test-cases/single-wordpress-tv-embed-outside-viewport.php | 2 +- .../tests/test-cases/single-youtube-embed-inside-viewport.php | 2 +- .../single-youtube-embed-outside-viewport-on-mobile.php | 2 +- .../tests/test-cases/single-youtube-embed-outside-viewport.php | 2 +- plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php | 2 +- ...on-lcp-background-image-with-fully-populated-sample-data.php | 2 +- ...-image-outside-viewport-with-fully-populated-sample-data.php | 2 +- .../test-cases/common-lcp-image-with-stale-sample-data.php | 2 +- ...ady-on-common-lcp-image-with-fully-populated-sample-data.php | 2 +- .../images-located-above-or-along-initial-viewport.php | 2 +- ...ground-image-outside-viewport-with-populated-url-metrics.php | 2 +- .../tests/test-cases/responsive-background-images.php | 2 +- plugins/image-prioritizer/tests/test-helper.php | 2 +- tests/class-optimization-detective-test-helpers.php | 2 +- 21 files changed, 21 insertions(+), 21 deletions(-) diff --git a/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php b/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php index 60c56b04ac..af194af46d 100644 --- a/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php +++ b/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php @@ -26,7 +26,7 @@ 'intersectionRatio' => 0, ), ), - false + null ); // This tests how the Embed Optimizer plugin plays along with other tag visitors. diff --git a/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php b/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php index 484b25cb2e..c384a05de9 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php +++ b/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - false + null ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php index a477e65586..e8dd3bcfa9 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php @@ -10,7 +10,7 @@ // Intentionally omitting resizedBoundingClientRect here to test behavior when data isn't supplied. ), ), - 'embeds' + md5( 'embeds' ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php index bcf9d4d76d..fe9b4e04b1 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - 'embeds' + md5( 'embeds' ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php index abe89a61c8..e9c11ee495 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php @@ -22,7 +22,7 @@ od_get_url_metrics_slug( od_get_normalized_query_vars() ), $test_case->get_sample_url_metric( array( - 'etag' => 'embeds', + 'etag' => md5( 'embeds' ), 'viewport_width' => $viewport_width, 'elements' => $elements, ) diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php index 36e2ea087d..ef63722773 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - 'embeds' + md5( 'embeds' ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php index 85a0e4e8e9..e8109f8155 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - false + null ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php index 7d3cd53df8..a9e56d9a71 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - false + null ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php index a6bb8384e0..5a96738bb0 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - 'embeds' + md5( 'embeds' ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php index 686be6ad15..7904d893b9 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php @@ -22,7 +22,7 @@ od_get_url_metrics_slug( od_get_normalized_query_vars() ), $test_case->get_sample_url_metric( array( - 'etag' => 'embeds', + 'etag' => md5( 'embeds' ), 'viewport_width' => $viewport_width, 'elements' => $elements, ) diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php index 7f7de84cb4..bfd546ce7c 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - 'embeds' + md5( 'embeds' ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php b/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php index bd56e9c71d..ba36ecc4af 100644 --- a/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php +++ b/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php @@ -11,7 +11,7 @@ 'intersectionRatio' => 0.0, ), ), - false + null ); // Check what happens when there are too many bookmarks. diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php index d7933e8eb9..c87e736fe9 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php @@ -8,7 +8,7 @@ 'isLCP' => true, ), ), - 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' + md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php index e1b84cb5d5..98eb86d890 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php @@ -15,7 +15,7 @@ $slug, $test_case->get_sample_url_metric( array( - 'etag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', + 'etag' => md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ), 'viewport_width' => $viewport_width, 'elements' => array( array( diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php index b542c00a0d..c90d5eed11 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php @@ -8,7 +8,7 @@ 'isLCP' => true, ), ), - 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' + md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php index c69565cc47..a59f1545d0 100644 --- a/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php @@ -8,7 +8,7 @@ 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', ), ), - 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' + md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php b/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php index b6885ded35..4e12bde38f 100644 --- a/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php +++ b/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php @@ -31,7 +31,7 @@ $slug, $test_case->get_sample_url_metric( array( - 'etag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', + 'etag' => md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ), 'viewport_width' => $viewport_width, 'elements' => array( array( diff --git a/plugins/image-prioritizer/tests/test-cases/no-lcp-image-or-background-image-outside-viewport-with-populated-url-metrics.php b/plugins/image-prioritizer/tests/test-cases/no-lcp-image-or-background-image-outside-viewport-with-populated-url-metrics.php index bd60079243..507cefeca9 100644 --- a/plugins/image-prioritizer/tests/test-cases/no-lcp-image-or-background-image-outside-viewport-with-populated-url-metrics.php +++ b/plugins/image-prioritizer/tests/test-cases/no-lcp-image-or-background-image-outside-viewport-with-populated-url-metrics.php @@ -8,7 +8,7 @@ 'isLCP' => true, ), ), - 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' + md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php b/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php index 47f21a8753..3cc8e7747b 100644 --- a/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php +++ b/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php @@ -25,7 +25,7 @@ static function () use ( $mobile_breakpoint, $tablet_breakpoint ): array { $slug, $test_case->get_sample_url_metric( array( - 'etag' => 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video', + 'etag' => md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ), 'viewport_width' => $viewport_width, 'element' => array( 'xpath' => sprintf( '/*[1][self::HTML]/*[2][self::BODY]/*[%d][self::DIV]', $div_index + 1 ), diff --git a/plugins/image-prioritizer/tests/test-helper.php b/plugins/image-prioritizer/tests/test-helper.php index 94614b68a3..c84b7202a2 100644 --- a/plugins/image-prioritizer/tests/test-helper.php +++ b/plugins/image-prioritizer/tests/test-helper.php @@ -207,7 +207,7 @@ public function data_provider_test_auto_sizes(): array { * @phpstan-param array{ xpath: string, isLCP: bool, intersectionRatio: int } $element_metrics */ public function test_auto_sizes( array $element_metrics, string $buffer, string $expected ): void { - $this->populate_url_metrics( array( $element_metrics ), 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ); + $this->populate_url_metrics( array( $element_metrics ), md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) ); $html_start_doc = '...'; $html_end_doc = ''; diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index 7f5fe4c36c..a0704a58ab 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -68,7 +68,7 @@ public function get_sample_dom_rect(): array { * Gets a sample URL metric. * * @phpstan-param array{ - * etag?: string, + * etag?: non-empty-string, * url?: string, * viewport_width?: int, * viewport_height?: int, From 4f0ac4c63528c7f7a88b7cd7fdeaaecbb89c29f5 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 20:28:30 -0800 Subject: [PATCH 33/48] Rename od_compute_current_etag() to od_get_current_etag() for parity with od_get_url_metrics_slug() --- .../optimization-detective/optimization.php | 2 +- .../optimization-detective/storage/data.php | 4 ++-- .../tests/storage/test-rest-api.php | 2 +- .../tests/test-cases/complete-url-metrics.php | 2 +- .../tests/test-cases/many-images.php | 2 +- .../tests/test-cases/video.php | 2 +- .../tests/test-class-od-element.php | 2 +- ...-class-od-url-metrics-group-collection.php | 24 +++++++++---------- .../tests/test-class-od-url-metrics-group.php | 2 +- .../tests/test-detection.php | 2 +- ...ss-optimization-detective-test-helpers.php | 4 ++-- 11 files changed, 24 insertions(+), 24 deletions(-) diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index 0fd23d86aa..bea2bbe08a 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -206,7 +206,7 @@ function od_optimize_template_output_buffer( string $buffer ): string { */ do_action( 'od_register_tag_visitors', $tag_visitor_registry ); - $current_etag = od_compute_current_etag( $tag_visitor_registry ); + $current_etag = od_get_current_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, diff --git a/plugins/optimization-detective/storage/data.php b/plugins/optimization-detective/storage/data.php index 0233c5001e..d9c561efb3 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -141,7 +141,7 @@ function od_get_url_metrics_slug( array $query_vars ): string { } /** - * Computes the current ETag for URL Metrics. + * 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 @@ -153,7 +153,7 @@ function od_get_url_metrics_slug( array $query_vars ): string { * @param OD_Tag_Visitor_Registry $tag_visitor_registry Tag visitor registry. * @return non-empty-string Current ETag. */ -function od_compute_current_etag( OD_Tag_Visitor_Registry $tag_visitor_registry ): string { +function od_get_current_etag( OD_Tag_Visitor_Registry $tag_visitor_registry ): string { $data = array( 'tag_visitors' => array_keys( iterator_to_array( $tag_visitor_registry ) ), ); diff --git a/plugins/optimization-detective/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index a7cb38184a..34bf661eee 100644 --- a/plugins/optimization-detective/tests/storage/test-rest-api.php +++ b/plugins/optimization-detective/tests/storage/test-rest-api.php @@ -138,7 +138,7 @@ function ( OD_URL_Metric_Store_Request_Context $context ) use ( &$stored_context */ public function data_provider_invalid_params(): array { $valid_element = $this->get_valid_params()['elements'][0]; - $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); return array_map( function ( $params ) { 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 b3b0e8bab4..bdadc0e134 100644 --- a/plugins/optimization-detective/tests/test-cases/complete-url-metrics.php +++ b/plugins/optimization-detective/tests/test-cases/complete-url-metrics.php @@ -14,7 +14,7 @@ 'isLCP' => true, ), ), - od_compute_current_etag( $tag_visitor_registry ) + od_get_current_etag( $tag_visitor_registry ) ); }, 'buffer' => ' diff --git a/plugins/optimization-detective/tests/test-cases/many-images.php b/plugins/optimization-detective/tests/test-cases/many-images.php index 1b42acd506..d9676a3da8 100644 --- a/plugins/optimization-detective/tests/test-cases/many-images.php +++ b/plugins/optimization-detective/tests/test-cases/many-images.php @@ -13,7 +13,7 @@ $tag_visitor_registry->register( 'img', static function (): void {} ); $tag_visitor_registry->register( 'video', static function (): void {} ); - $test_case->populate_url_metrics( $elements, od_compute_current_etag( $tag_visitor_registry ), false ); + $test_case->populate_url_metrics( $elements, od_get_current_etag( $tag_visitor_registry ), false ); }, 'buffer' => ' diff --git a/plugins/optimization-detective/tests/test-cases/video.php b/plugins/optimization-detective/tests/test-cases/video.php index 68c497eed2..85c308ed0a 100644 --- a/plugins/optimization-detective/tests/test-cases/video.php +++ b/plugins/optimization-detective/tests/test-cases/video.php @@ -12,7 +12,7 @@ 'isLCP' => true, ), ), - od_compute_current_etag( $tag_visitor_registry ), + od_get_current_etag( $tag_visitor_registry ), false ); }, diff --git a/plugins/optimization-detective/tests/test-class-od-element.php b/plugins/optimization-detective/tests/test-class-od-element.php index c13fd11edc..7cf7b733b7 100644 --- a/plugins/optimization-detective/tests/test-class-od-element.php +++ b/plugins/optimization-detective/tests/test-class-od-element.php @@ -71,7 +71,7 @@ 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() ); - $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $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-metrics-group-collection.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php index a706b2a833..22152163c0 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,7 +19,7 @@ class Test_OD_URL_Metric_Group_Collection extends WP_UnitTestCase { * @return array Data. */ public function data_provider_test_construction(): array { - $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); return array( 'no_breakpoints_ok' => array( @@ -196,7 +196,7 @@ 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 { - $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $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. @@ -225,7 +225,7 @@ 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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $sample_size = 3; $breakpoints = array( 400, 600 ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS ); @@ -349,7 +349,7 @@ function ( $viewport_width ) { $viewport_widths ); - $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, 3, HOUR_IN_SECONDS ); $this->assertCount( @@ -514,7 +514,7 @@ public function data_provider_test_get_group_for_viewport_width(): array { * @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 { - $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, $freshness_ttl ); $this->assertSame( $expected_return, @@ -547,7 +547,7 @@ 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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, @@ -602,7 +602,7 @@ public function test_get_groups_by_lcp_element(): void { $breakpoints = array( 480, 800 ); $sample_size = 3; - $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( array( // Group 1: 0-480 viewport widths. @@ -645,7 +645,7 @@ 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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, @@ -759,7 +759,7 @@ 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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $breakpoints = array( 480, 600, 782 ); $sample_size = 3; $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, 0 ); @@ -937,7 +937,7 @@ 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 = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $breakpoints = array( 480, 600, 782 ); $sample_size = 3; $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, 0 ); @@ -964,7 +964,7 @@ public function test_get_flattened_url_metrics(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, - od_compute_current_etag( new OD_Tag_Visitor_Registry() ), + od_get_current_etag( new OD_Tag_Visitor_Registry() ), array( 500, 700 ), 3, HOUR_IN_SECONDS @@ -992,7 +992,7 @@ public function test_json_serialize(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, - od_compute_current_etag( new OD_Tag_Visitor_Registry() ), + od_get_current_etag( new OD_Tag_Visitor_Registry() ), array( 500, 700 ), 3, HOUR_IN_SECONDS 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 1dcc393f4e..7fff6f27f1 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 @@ -346,7 +346,7 @@ 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 { - $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $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(); diff --git a/plugins/optimization-detective/tests/test-detection.php b/plugins/optimization-detective/tests/test-detection.php index 618315c2d5..71a126db6e 100644 --- a/plugins/optimization-detective/tests/test-detection.php +++ b/plugins/optimization-detective/tests/test-detection.php @@ -128,7 +128,7 @@ 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' ) ); - $current_etag = od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); $breakpoints = array( 480, 600, 782 ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, $breakpoints, 3, HOUR_IN_SECONDS ); diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index a0704a58ab..b66b3cbe4d 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -28,7 +28,7 @@ trait Optimization_Detective_Test_Helpers { */ public function populate_url_metrics( array $elements, ?string $etag, bool $complete = true ): void { $slug = od_get_url_metrics_slug( od_get_normalized_query_vars() ); - $etag = $etag ?? od_compute_current_etag( new OD_Tag_Visitor_Registry() ); + $etag = $etag ?? od_get_current_etag( new OD_Tag_Visitor_Registry() ); $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++ ) { @@ -81,7 +81,7 @@ public function get_sample_dom_rect(): array { public function get_sample_url_metric( array $params ): OD_URL_Metric { $params = array_merge( array( - 'etag' => od_compute_current_etag( new OD_Tag_Visitor_Registry() ), + 'etag' => od_get_current_etag( new OD_Tag_Visitor_Registry() ), 'url' => home_url( '/' ), 'viewport_width' => 480, 'elements' => array(), From ae3eab0b393d32da8929c87fd25ec8a63df29ef0 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 20:34:59 -0800 Subject: [PATCH 34/48] Use JSON-encoding instead of PHP serialization Co-authored-by: Shyamsundar Gadde --- plugins/optimization-detective/storage/data.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/optimization-detective/storage/data.php b/plugins/optimization-detective/storage/data.php index d9c561efb3..17e2f51f24 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -157,7 +157,7 @@ function od_get_current_etag( OD_Tag_Visitor_Registry $tag_visitor_registry ): s $data = array( 'tag_visitors' => array_keys( iterator_to_array( $tag_visitor_registry ) ), ); - return md5( serialize( $data ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + return md5( (string) wp_json_encode( $data ) ); } /** From a33eb28aa3d0e7c88088f217128fcdee50aee0bf Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 21:34:32 -0800 Subject: [PATCH 35/48] Introduce od_current_url_metrics_etag_data filter for extensibility and to improve testability --- .../tests/test-cases/nested-figure-embed.php | 2 +- ...utside-viewport-with-subsequent-script.php | 2 +- ...d-inside-viewport-without-resized-data.php | 3 +-- .../single-twitter-embed-inside-viewport.php | 3 +-- ...itter-embed-outside-viewport-on-mobile.php | 1 - .../single-twitter-embed-outside-viewport.php | 3 +-- ...gle-wordpress-tv-embed-inside-viewport.php | 2 +- ...le-wordpress-tv-embed-outside-viewport.php | 2 +- .../single-youtube-embed-inside-viewport.php | 3 +-- ...utube-embed-outside-viewport-on-mobile.php | 1 - .../single-youtube-embed-outside-viewport.php | 3 +-- .../tests/test-cases/too-many-bookmarks.php | 2 +- .../tests/test-optimization-detective.php | 6 +++++ ...wport-with-fully-populated-sample-data.php | 3 +-- ...image-with-fully-populated-sample-data.php | 3 +-- ...wport-with-fully-populated-sample-data.php | 1 - ...ommon-lcp-image-with-stale-sample-data.php | 3 +-- ...image-with-fully-populated-sample-data.php | 3 +-- ...ocated-above-or-along-initial-viewport.php | 1 - ...de-viewport-with-populated-url-metrics.php | 3 +-- .../responsive-background-images.php | 1 - .../image-prioritizer/tests/test-helper.php | 15 +++++++++++- .../optimization-detective/optimization.php | 2 +- .../optimization-detective/storage/data.php | 12 +++++++++- .../tests/storage/test-rest-api.php | 2 +- .../tests/test-cases/complete-url-metrics.php | 11 +++++---- .../tests/test-cases/many-images.php | 2 +- .../tests/test-cases/video.php | 5 ---- .../tests/test-class-od-element.php | 2 +- ...-class-od-url-metrics-group-collection.php | 24 +++++++++---------- .../tests/test-class-od-url-metrics-group.php | 2 +- .../tests/test-detection.php | 2 +- ...ss-optimization-detective-test-helpers.php | 11 ++++----- 33 files changed, 75 insertions(+), 66 deletions(-) diff --git a/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php b/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php index af194af46d..60c56b04ac 100644 --- a/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php +++ b/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php @@ -26,7 +26,7 @@ 'intersectionRatio' => 0, ), ), - null + false ); // This tests how the Embed Optimizer plugin plays along with other tag visitors. diff --git a/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php b/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php index c384a05de9..484b25cb2e 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php +++ b/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - null + false ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php index e8dd3bcfa9..37886051b9 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php @@ -9,8 +9,7 @@ 'intersectionRatio' => 1, // Intentionally omitting resizedBoundingClientRect here to test behavior when data isn't supplied. ), - ), - md5( 'embeds' ) + ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php index fe9b4e04b1..04b94117d3 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php @@ -9,8 +9,7 @@ 'intersectionRatio' => 1, 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), - ), - md5( 'embeds' ) + ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php index e9c11ee495..3c70222e49 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport-on-mobile.php @@ -22,7 +22,6 @@ od_get_url_metrics_slug( od_get_normalized_query_vars() ), $test_case->get_sample_url_metric( array( - 'etag' => md5( 'embeds' ), 'viewport_width' => $viewport_width, 'elements' => $elements, ) diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php index ef63722773..ccdf5fbcfc 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php @@ -9,8 +9,7 @@ 'intersectionRatio' => 0, 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), - ), - md5( 'embeds' ) + ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php index e8109f8155..85a0e4e8e9 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - null + false ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php index a9e56d9a71..7d3cd53df8 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php @@ -10,7 +10,7 @@ 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), ), - null + false ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php index 5a96738bb0..5605d8c124 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php @@ -9,8 +9,7 @@ 'intersectionRatio' => 1, 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), - ), - md5( 'embeds' ) + ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php index 7904d893b9..da1c4b8b47 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport-on-mobile.php @@ -22,7 +22,6 @@ od_get_url_metrics_slug( od_get_normalized_query_vars() ), $test_case->get_sample_url_metric( array( - 'etag' => md5( 'embeds' ), 'viewport_width' => $viewport_width, 'elements' => $elements, ) diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php index bfd546ce7c..daef5292c6 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php @@ -9,8 +9,7 @@ 'intersectionRatio' => 0, 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), ), - ), - md5( 'embeds' ) + ) ); }, 'buffer' => ' diff --git a/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php b/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php index ba36ecc4af..bd56e9c71d 100644 --- a/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php +++ b/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php @@ -11,7 +11,7 @@ 'intersectionRatio' => 0.0, ), ), - null + false ); // Check what happens when there are too many bookmarks. 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-cases/common-lcp-background-image-and-lazy-loaded-background-image-outside-viewport-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-and-lazy-loaded-background-image-outside-viewport-with-fully-populated-sample-data.php index bc193ea443..6e2f2f9b7b 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-and-lazy-loaded-background-image-outside-viewport-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-and-lazy-loaded-background-image-outside-viewport-with-fully-populated-sample-data.php @@ -28,8 +28,7 @@ 'intersectionRect' => $outside_viewport_rect, 'boundingClientRect' => $outside_viewport_rect, ), - ), - null + ) ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php index c87e736fe9..5ecd8e7de7 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-background-image-with-fully-populated-sample-data.php @@ -7,8 +7,7 @@ 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::DIV]', 'isLCP' => true, ), - ), - md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) + ) ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php index 98eb86d890..b19f83b4d6 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-and-lazy-loaded-image-outside-viewport-with-fully-populated-sample-data.php @@ -15,7 +15,6 @@ $slug, $test_case->get_sample_url_metric( array( - 'etag' => md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ), 'viewport_width' => $viewport_width, 'elements' => array( array( diff --git a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php index c90d5eed11..5df495f7b5 100644 --- a/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/common-lcp-image-with-stale-sample-data.php @@ -7,8 +7,7 @@ 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', // Note: This is intentionally not reflecting the IMG in the HTML below. 'isLCP' => true, ), - ), - md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) + ) ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php b/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php index a59f1545d0..b4c504b096 100644 --- a/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php +++ b/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-already-on-common-lcp-image-with-fully-populated-sample-data.php @@ -7,8 +7,7 @@ 'isLCP' => true, 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', ), - ), - md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) + ) ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php b/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php index 4e12bde38f..be5ceae574 100644 --- a/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php +++ b/plugins/image-prioritizer/tests/test-cases/images-located-above-or-along-initial-viewport.php @@ -31,7 +31,6 @@ $slug, $test_case->get_sample_url_metric( array( - 'etag' => md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ), 'viewport_width' => $viewport_width, 'elements' => array( array( diff --git a/plugins/image-prioritizer/tests/test-cases/no-lcp-image-or-background-image-outside-viewport-with-populated-url-metrics.php b/plugins/image-prioritizer/tests/test-cases/no-lcp-image-or-background-image-outside-viewport-with-populated-url-metrics.php index 507cefeca9..eb9fe4eb6b 100644 --- a/plugins/image-prioritizer/tests/test-cases/no-lcp-image-or-background-image-outside-viewport-with-populated-url-metrics.php +++ b/plugins/image-prioritizer/tests/test-cases/no-lcp-image-or-background-image-outside-viewport-with-populated-url-metrics.php @@ -7,8 +7,7 @@ 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::H1]', 'isLCP' => true, ), - ), - md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) + ) ); }, 'buffer' => ' diff --git a/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php b/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php index 3cc8e7747b..1f9d515776 100644 --- a/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php +++ b/plugins/image-prioritizer/tests/test-cases/responsive-background-images.php @@ -25,7 +25,6 @@ static function () use ( $mobile_breakpoint, $tablet_breakpoint ): array { $slug, $test_case->get_sample_url_metric( array( - 'etag' => md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ), 'viewport_width' => $viewport_width, 'element' => array( 'xpath' => sprintf( '/*[1][self::HTML]/*[2][self::BODY]/*[%d][self::DIV]', $div_index + 1 ), diff --git a/plugins/image-prioritizer/tests/test-helper.php b/plugins/image-prioritizer/tests/test-helper.php index c84b7202a2..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> */ @@ -207,7 +220,7 @@ public function data_provider_test_auto_sizes(): array { * @phpstan-param array{ xpath: string, isLCP: bool, intersectionRatio: int } $element_metrics */ public function test_auto_sizes( array $element_metrics, string $buffer, string $expected ): void { - $this->populate_url_metrics( array( $element_metrics ), md5( 'image-prioritizer/img,image-prioritizer/background-image,image-prioritizer/video' ) ); + $this->populate_url_metrics( array( $element_metrics ) ); $html_start_doc = '...'; $html_end_doc = ''; diff --git a/plugins/optimization-detective/optimization.php b/plugins/optimization-detective/optimization.php index bea2bbe08a..8e5e524744 100644 --- a/plugins/optimization-detective/optimization.php +++ b/plugins/optimization-detective/optimization.php @@ -206,7 +206,7 @@ function od_optimize_template_output_buffer( string $buffer ): string { */ do_action( 'od_register_tag_visitors', $tag_visitor_registry ); - $current_etag = od_get_current_etag( $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, diff --git a/plugins/optimization-detective/storage/data.php b/plugins/optimization-detective/storage/data.php index 17e2f51f24..a773ff6f1f 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -153,10 +153,20 @@ function od_get_url_metrics_slug( array $query_vars ): string { * @param OD_Tag_Visitor_Registry $tag_visitor_registry Tag visitor registry. * @return non-empty-string Current ETag. */ -function od_get_current_etag( OD_Tag_Visitor_Registry $tag_visitor_registry ): string { +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 ) ); } diff --git a/plugins/optimization-detective/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index 34bf661eee..b6e8f3314f 100644 --- a/plugins/optimization-detective/tests/storage/test-rest-api.php +++ b/plugins/optimization-detective/tests/storage/test-rest-api.php @@ -138,7 +138,7 @@ function ( OD_URL_Metric_Store_Request_Context $context ) use ( &$stored_context */ public function data_provider_invalid_params(): array { $valid_element = $this->get_valid_params()['elements'][0]; - $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); return array_map( function ( $params ) { 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 bdadc0e134..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,9 +3,11 @@ '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 - $tag_visitor_registry = new OD_Tag_Visitor_Registry(); - $tag_visitor_registry->register( 'img', static function (): void {} ); - $tag_visitor_registry->register( 'video', static function (): void {} ); + // 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( @@ -13,8 +15,7 @@ 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', 'isLCP' => true, ), - ), - od_get_current_etag( $tag_visitor_registry ) + ) ); }, 'buffer' => ' diff --git a/plugins/optimization-detective/tests/test-cases/many-images.php b/plugins/optimization-detective/tests/test-cases/many-images.php index d9676a3da8..9a09c837ab 100644 --- a/plugins/optimization-detective/tests/test-cases/many-images.php +++ b/plugins/optimization-detective/tests/test-cases/many-images.php @@ -13,7 +13,7 @@ $tag_visitor_registry->register( 'img', static function (): void {} ); $tag_visitor_registry->register( 'video', static function (): void {} ); - $test_case->populate_url_metrics( $elements, od_get_current_etag( $tag_visitor_registry ), false ); + $test_case->populate_url_metrics( $elements, false ); }, 'buffer' => ' diff --git a/plugins/optimization-detective/tests/test-cases/video.php b/plugins/optimization-detective/tests/test-cases/video.php index 85c308ed0a..038cd05d55 100644 --- a/plugins/optimization-detective/tests/test-cases/video.php +++ b/plugins/optimization-detective/tests/test-cases/video.php @@ -1,10 +1,6 @@ static function ( Test_OD_Optimization $test_case ): void { - $tag_visitor_registry = new OD_Tag_Visitor_Registry(); - $tag_visitor_registry->register( 'img', static function (): void {} ); - $tag_visitor_registry->register( 'video', static function (): void {} ); - $test_case->populate_url_metrics( array( array( @@ -12,7 +8,6 @@ 'isLCP' => true, ), ), - od_get_current_etag( $tag_visitor_registry ), false ); }, diff --git a/plugins/optimization-detective/tests/test-class-od-element.php b/plugins/optimization-detective/tests/test-class-od-element.php index 7cf7b733b7..e0bbaaa6e3 100644 --- a/plugins/optimization-detective/tests/test-class-od-element.php +++ b/plugins/optimization-detective/tests/test-class-od-element.php @@ -71,7 +71,7 @@ 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() ); - $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $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-metrics-group-collection.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php index 22152163c0..308a36bf01 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,7 +19,7 @@ class Test_OD_URL_Metric_Group_Collection extends WP_UnitTestCase { * @return array Data. */ public function data_provider_test_construction(): array { - $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); return array( 'no_breakpoints_ok' => array( @@ -196,7 +196,7 @@ 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 { - $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $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. @@ -225,7 +225,7 @@ 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 = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $sample_size = 3; $breakpoints = array( 400, 600 ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS ); @@ -349,7 +349,7 @@ function ( $viewport_width ) { $viewport_widths ); - $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, 3, HOUR_IN_SECONDS ); $this->assertCount( @@ -514,7 +514,7 @@ public function data_provider_test_get_group_for_viewport_width(): array { * @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 { - $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, $freshness_ttl ); $this->assertSame( $expected_return, @@ -547,7 +547,7 @@ 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 = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, @@ -602,7 +602,7 @@ public function test_get_groups_by_lcp_element(): void { $breakpoints = array( 480, 800 ); $sample_size = 3; - $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( array( // Group 1: 0-480 viewport widths. @@ -645,7 +645,7 @@ 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 = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, @@ -759,7 +759,7 @@ 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 = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $breakpoints = array( 480, 600, 782 ); $sample_size = 3; $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, 0 ); @@ -937,7 +937,7 @@ 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 = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $breakpoints = array( 480, 600, 782 ); $sample_size = 3; $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, 0 ); @@ -964,7 +964,7 @@ public function test_get_flattened_url_metrics(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, - od_get_current_etag( new OD_Tag_Visitor_Registry() ), + od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ), array( 500, 700 ), 3, HOUR_IN_SECONDS @@ -992,7 +992,7 @@ public function test_json_serialize(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, - od_get_current_etag( new OD_Tag_Visitor_Registry() ), + od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ), array( 500, 700 ), 3, HOUR_IN_SECONDS 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 7fff6f27f1..5c3e2a0b04 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 @@ -346,7 +346,7 @@ 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 { - $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $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(); diff --git a/plugins/optimization-detective/tests/test-detection.php b/plugins/optimization-detective/tests/test-detection.php index 71a126db6e..466ac8211e 100644 --- a/plugins/optimization-detective/tests/test-detection.php +++ b/plugins/optimization-detective/tests/test-detection.php @@ -128,7 +128,7 @@ 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' ) ); - $current_etag = od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $breakpoints = array( 480, 600, 782 ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, $breakpoints, 3, HOUR_IN_SECONDS ); diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index b66b3cbe4d..a9317bdc2d 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -21,14 +21,13 @@ trait Optimization_Detective_Test_Helpers { * Populates complete URL metrics for the provided element data. * * @phpstan-param ElementDataSubset[] $elements - * @param array[] $elements Element data. - * @param string|null $etag ETag to set for the URL metrics. - * @param bool $complete Whether to fully populate the groups. + * @param array[] $elements Element data. + * @param bool $complete Whether to fully populate the groups. * @throws Exception But it won't. */ - public function populate_url_metrics( array $elements, ?string $etag, bool $complete = true ): void { + public function populate_url_metrics( array $elements, bool $complete = true ): void { $slug = od_get_url_metrics_slug( od_get_normalized_query_vars() ); - $etag = $etag ?? od_get_current_etag( new OD_Tag_Visitor_Registry() ); + $etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); $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++ ) { @@ -81,7 +80,7 @@ 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_etag( new OD_Tag_Visitor_Registry() ), + 'etag' => od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ), 'url' => home_url( '/' ), 'viewport_width' => 480, 'elements' => array(), From d37d6712e461a329eb2da3699362a9018fab932d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 30 Nov 2024 21:52:25 -0800 Subject: [PATCH 36/48] Use md5() directly when current ETag is not needed --- .../tests/storage/test-rest-api.php | 2 +- .../tests/test-cases/many-images.php | 4 ---- .../tests/test-class-od-element.php | 2 +- ...t-class-od-url-metrics-group-collection.php | 18 +++++++++--------- .../tests/test-class-od-url-metrics-group.php | 5 ++--- .../tests/test-detection.php | 2 +- 6 files changed, 14 insertions(+), 19 deletions(-) diff --git a/plugins/optimization-detective/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index b6e8f3314f..e72e507e6b 100644 --- a/plugins/optimization-detective/tests/storage/test-rest-api.php +++ b/plugins/optimization-detective/tests/storage/test-rest-api.php @@ -138,7 +138,7 @@ function ( OD_URL_Metric_Store_Request_Context $context ) use ( &$stored_context */ public function data_provider_invalid_params(): array { $valid_element = $this->get_valid_params()['elements'][0]; - $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); return array_map( function ( $params ) { diff --git a/plugins/optimization-detective/tests/test-cases/many-images.php b/plugins/optimization-detective/tests/test-cases/many-images.php index 9a09c837ab..4e769484b9 100644 --- a/plugins/optimization-detective/tests/test-cases/many-images.php +++ b/plugins/optimization-detective/tests/test-cases/many-images.php @@ -9,10 +9,6 @@ ); } - $tag_visitor_registry = new OD_Tag_Visitor_Registry(); - $tag_visitor_registry->register( 'img', static function (): void {} ); - $tag_visitor_registry->register( 'video', static function (): void {} ); - $test_case->populate_url_metrics( $elements, false ); }, 'buffer' => ' diff --git a/plugins/optimization-detective/tests/test-class-od-element.php b/plugins/optimization-detective/tests/test-class-od-element.php index e0bbaaa6e3..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,7 @@ 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() ); - $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $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-metrics-group-collection.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php index 308a36bf01..0f5288ace6 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 @@ -196,7 +196,7 @@ 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 { - $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $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. @@ -225,7 +225,7 @@ 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 = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); $sample_size = 3; $breakpoints = array( 400, 600 ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS ); @@ -349,7 +349,7 @@ function ( $viewport_width ) { $viewport_widths ); - $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, 3, HOUR_IN_SECONDS ); $this->assertCount( @@ -602,7 +602,7 @@ public function test_get_groups_by_lcp_element(): void { $breakpoints = array( 480, 800 ); $sample_size = 3; - $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); $group_collection = new OD_URL_Metric_Group_Collection( array( // Group 1: 0-480 viewport widths. @@ -645,7 +645,7 @@ 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 = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, @@ -759,7 +759,7 @@ 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 = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); $breakpoints = array( 480, 600, 782 ); $sample_size = 3; $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, 0 ); @@ -937,7 +937,7 @@ 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 = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); $breakpoints = array( 480, 600, 782 ); $sample_size = 3; $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, $breakpoints, $sample_size, 0 ); @@ -964,7 +964,7 @@ public function test_get_flattened_url_metrics(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, - od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ), + md5( '' ), array( 500, 700 ), 3, HOUR_IN_SECONDS @@ -992,7 +992,7 @@ public function test_json_serialize(): void { $group_collection = new OD_URL_Metric_Group_Collection( $url_metrics, - od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ), + md5( '' ), array( 500, 700 ), 3, HOUR_IN_SECONDS 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 5c3e2a0b04..773bb0376a 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 @@ -346,7 +346,7 @@ 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 { - $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $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(); @@ -377,8 +377,7 @@ function ( $viewport_width ) { ); $collection = new OD_URL_Metric_Group_Collection( $url_metrics, $current_etag, array( 1000 ), 1, HOUR_IN_SECONDS ); - $group = iterator_to_array( $collection )[0]; - $this->assertInstanceOf( OD_URL_Metric_Group::class, $group ); + $group = $collection->get_first_group(); $json = wp_json_encode( $group ); $parsed_json = json_decode( $json, true ); diff --git a/plugins/optimization-detective/tests/test-detection.php b/plugins/optimization-detective/tests/test-detection.php index 466ac8211e..a159591115 100644 --- a/plugins/optimization-detective/tests/test-detection.php +++ b/plugins/optimization-detective/tests/test-detection.php @@ -128,7 +128,7 @@ 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' ) ); - $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); $breakpoints = array( 480, 600, 782 ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, $breakpoints, 3, HOUR_IN_SECONDS ); From 92dc35451dff8cc696d1feca032da69903af41bd Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Sun, 1 Dec 2024 22:02:26 +0530 Subject: [PATCH 37/48] Remove extra empty line Co-authored-by: Weston Ruter --- plugins/optimization-detective/tests/test-cases/many-images.php | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/optimization-detective/tests/test-cases/many-images.php b/plugins/optimization-detective/tests/test-cases/many-images.php index 4e769484b9..a5526001cf 100644 --- a/plugins/optimization-detective/tests/test-cases/many-images.php +++ b/plugins/optimization-detective/tests/test-cases/many-images.php @@ -8,7 +8,6 @@ 'isLCP' => false, ); } - $test_case->populate_url_metrics( $elements, false ); }, 'buffer' => ' From 85540e6b258e8fa09c5363058c4ae15f4c754d9f Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Sun, 1 Dec 2024 08:24:30 +0530 Subject: [PATCH 38/48] Add validation for current_etag paramter in OD_URL_Metric_Group_Collection constructor Signed-off-by: Shyamsundar Gadde --- .../class-od-url-metric-group-collection.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) 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 08457f0283..7a6f7acb04 100644 --- a/plugins/optimization-detective/class-od-url-metric-group-collection.php +++ b/plugins/optimization-detective/class-od-url-metric-group-collection.php @@ -108,6 +108,18 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega * @param int $freshness_ttl Freshness age (TTL) for a given URL Metric. */ 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}$/', $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. From 731fe91d2645835c9d8e914d289039bd3a049f6a Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Sun, 1 Dec 2024 09:56:47 +0530 Subject: [PATCH 39/48] Use passed URL in test helper instead of default home_url() Signed-off-by: Shyamsundar Gadde --- tests/class-optimization-detective-test-helpers.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index a9317bdc2d..c8eb061484 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -95,7 +95,7 @@ public function get_sample_url_metric( array $params ): OD_URL_Metric { return new OD_URL_Metric( array( 'etag' => $params['etag'], - 'url' => home_url( '/' ), + 'url' => $params['url'], 'viewport' => array( 'width' => $params['viewport_width'], 'height' => $params['viewport_height'] ?? ceil( $params['viewport_width'] / 2 ), From c3b7344c415cc69d2ab1384d3ef40e6f229a62fa Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Sun, 1 Dec 2024 19:01:46 +0530 Subject: [PATCH 40/48] Update test case for group collection constructor Signed-off-by: Shyamsundar Gadde --- .../tests/test-class-od-url-metrics-group-collection.php | 8 ++++++++ 1 file changed, 8 insertions(+) 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 0f5288ace6..984535ca09 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 @@ -78,6 +78,14 @@ public function data_provider_test_construction(): array { '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_url_metrics_bad' => array( 'url_metrics' => array( 'bad' ), 'current_etag' => $current_etag, From 9fcfb7fa028e9a86112563e498e53bc0cc81afb8 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Sun, 1 Dec 2024 21:57:40 +0530 Subject: [PATCH 41/48] Add tests for OD_URL_Metric_Group::is_complete Signed-off-by: Shyamsundar Gadde --- .../tests/test-class-od-url-metrics-group.php | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) 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 773bb0376a..62924cc343 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 @@ -242,6 +242,60 @@ 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 ) ) ), + '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. * From b225fba5a9e37f6a2231db34f400d0c8c23db6dd Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Sun, 1 Dec 2024 21:58:52 +0530 Subject: [PATCH 42/48] Add tests for OD_URL_Metric::get_etag Signed-off-by: Shyamsundar Gadde --- .../tests/test-class-od-url-metric.php | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) 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 0aa41824c9..0752e8b0a8 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,53 @@ 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_etag' => 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}$.', + ), + '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 +138,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 +149,7 @@ static function ( $value ) { 'bad_viewport' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => array( 'height' => 'tall', @@ -113,6 +163,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 +177,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 +191,7 @@ static function ( $value ) { 'missing_timestamp' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'url' => home_url( '/' ), 'viewport' => $viewport, 'elements' => array(), @@ -148,6 +201,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 +211,7 @@ static function ( $value ) { 'missing_url' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), + 'etag' => md5( '' ), 'viewport' => $viewport, 'timestamp' => microtime( true ), 'elements' => array(), @@ -166,6 +221,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 +236,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 +259,7 @@ static function ( $value ) { * @covers ::get_viewport_width * @covers ::get_timestamp * @covers ::get_elements + * @covers ::get_etag * @covers ::jsonSerialize * @covers ::get * @covers ::get_json_schema @@ -257,6 +315,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' ) ); From 7b01e50fe53d8a8eb633fdd1784fb7965aa90e2f Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Sun, 1 Dec 2024 21:59:19 +0530 Subject: [PATCH 43/48] Add missing `@covers ::get_url` annotation in test case Signed-off-by: Shyamsundar Gadde --- .../optimization-detective/tests/test-class-od-url-metric.php | 1 + 1 file changed, 1 insertion(+) 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 0752e8b0a8..15d3bbc818 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metric.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metric.php @@ -259,6 +259,7 @@ static function ( $value ) { * @covers ::get_viewport_width * @covers ::get_timestamp * @covers ::get_elements + * @covers ::get_url * @covers ::get_etag * @covers ::jsonSerialize * @covers ::get From 44565632f097e41cc1356cf076218d3151a60aea Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 1 Dec 2024 13:53:53 -0800 Subject: [PATCH 44/48] Harden ETag regex to disregard newlines --- .../class-od-url-metric-group-collection.php | 2 +- .../class-od-url-metric-group.php | 2 +- .../class-od-url-metric.php | 2 +- .../storage/rest-api.php | 6 +++--- .../tests/storage/test-data.php | 4 ++-- .../tests/storage/test-rest-api.php | 18 +++++++++++------ .../tests/test-class-od-url-metric.php | 20 +++++++++++++------ ...-class-od-url-metrics-group-collection.php | 8 ++++++++ 8 files changed, 42 insertions(+), 20 deletions(-) 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 7a6f7acb04..9b50846eb6 100644 --- a/plugins/optimization-detective/class-od-url-metric-group-collection.php +++ b/plugins/optimization-detective/class-od-url-metric-group-collection.php @@ -109,7 +109,7 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega */ 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}$/', $current_etag ) ) { + if ( 1 !== preg_match( '/^[a-f0-9]{32}\z/', $current_etag ) ) { throw new InvalidArgumentException( esc_html( sprintf( diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 5ea0b942b1..9f026f9ff0 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -92,7 +92,7 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer * @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. Optional. + * @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 ) { if ( $minimum_viewport_width < 0 ) { diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index 68a5555f4a..d3b0d984f7 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -213,7 +213,7 @@ public static function get_json_schema(): array { 'etag' => array( 'description' => __( 'The ETag for the URL Metric.', 'optimization-detective' ), 'type' => 'string', - 'pattern' => '^[0-9a-f]{32}$', + 'pattern' => '^[0-9a-f]{32}\z', 'minLength' => 32, 'maxLength' => 32, 'required' => false, // To be made required in a future release. diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index ba8bf02259..4de890f960 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -44,7 +44,7 @@ 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, ), @@ -52,7 +52,7 @@ function od_register_endpoint(): void { 'type' => 'string', 'description' => __( 'ETag for the current environment.', 'optimization-detective' ), 'required' => true, - 'pattern' => '^[0-9a-f]{32}$', + 'pattern' => '^[0-9a-f]{32}\z', 'minLength' => 32, 'maxLength' => 32, ), @@ -66,7 +66,7 @@ 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['current_etag'], $request['url'], $request['cache_purge_post_id'] ?? null ) ) { return new WP_Error( 'invalid_hmac', __( 'URL Metrics HMAC verification failure.', 'optimization-detective' ) ); diff --git a/plugins/optimization-detective/tests/storage/test-data.php b/plugins/optimization-detective/tests/storage/test-data.php index 2e6e3c17c2..f7ea1636d8 100644 --- a/plugins/optimization-detective/tests/storage/test-data.php +++ b/plugins/optimization-detective/tests/storage/test-data.php @@ -282,7 +282,7 @@ 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 ); } } @@ -328,7 +328,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 e72e507e6b..ad514aa487 100644 --- a/plugins/optimization-detective/tests/storage/test-rest-api.php +++ b/plugins/optimization-detective/tests/storage/test-rest-api.php @@ -137,19 +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]; - $current_etag = md5( '' ); + $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' => '', ), @@ -157,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' ) ), $current_etag, 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() ), $current_etag, 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', 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 15d3bbc818..51b23c243e 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metric.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metric.php @@ -104,7 +104,7 @@ static function ( $value ) { ), 'error' => 'OD_URL_Metric[etag] must be at most 32 characters long.', ), - 'bad_etag' => array( + 'bad_etag1' => array( 'data' => array( 'uuid' => wp_generate_uuid4(), 'etag' => 'd41d8cd98f00b204e9800998ecf8427$', @@ -113,7 +113,18 @@ static function ( $value ) { 'timestamp' => microtime( true ), 'elements' => array(), ), - 'error' => 'OD_URL_Metric[etag] does not match pattern ^[0-9a-f]{32}$.', + '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( @@ -281,10 +292,7 @@ public function test_constructor( array $data, string $error = '' ): void { $this->assertNull( $url_metric->get_group() ); $current_etag = md5( '' ); $collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, array(), 1, DAY_IN_SECONDS ); - $groups = iterator_to_array( $collection ); - $this->assertCount( 1, $groups ); - $this->assertInstanceOf( OD_URL_Metric_Group::class, $groups[0] ); - $group = $groups[0]; + $group = $collection->get_first_group(); $url_metric->set_group( $group ); $this->assertSame( $group, $url_metric->get_group() ); 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 984535ca09..a71d5fc75e 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 @@ -86,6 +86,14 @@ public function data_provider_test_construction(): array { '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, From 034fe46c5510885a31e33218781f28e13780949d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 1 Dec 2024 17:09:48 -0800 Subject: [PATCH 45/48] Add test for od_get_current_url_metrics_etag() --- .../tests/storage/test-data.php | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/plugins/optimization-detective/tests/storage/test-data.php b/plugins/optimization-detective/tests/storage/test-data.php index f7ea1636d8..c90df29ef2 100644 --- a/plugins/optimization-detective/tests/storage/test-data.php +++ b/plugins/optimization-detective/tests/storage/test-data.php @@ -286,6 +286,58 @@ public function test_od_get_url_metrics_slug(): void { } } + /** + * 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. * From 02e9759c7eb5ab57fc904a2cf5ed627ca0c94cc0 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 1 Dec 2024 17:32:31 -0800 Subject: [PATCH 46/48] Add docs for od_current_url_metrics_etag_data filter --- plugins/optimization-detective/readme.txt | 8 ++++++++ 1 file changed, 8 insertions(+) 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. From 54183afeddd11689d873f9f8b3d41690c45b724d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 1 Dec 2024 17:54:38 -0800 Subject: [PATCH 47/48] Eliminate constructing etag via OD_Tag_Visitor_Registry() unless side effects are needed --- ...-class-od-url-metrics-group-collection.php | 90 ++++++++++++++++--- ...ss-optimization-detective-test-helpers.php | 4 +- 2 files changed, 81 insertions(+), 13 deletions(-) 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 a71d5fc75e..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,7 +19,7 @@ class Test_OD_URL_Metric_Group_Collection extends WP_UnitTestCase { * @return array Data. */ public function data_provider_test_construction(): array { - $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); return array( 'no_breakpoints_ok' => array( @@ -402,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 ) ) ) @@ -421,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 ) ) ) @@ -429,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, @@ -508,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, + ), + ) + ), ); } @@ -523,14 +563,14 @@ 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 { - $current_etag = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + 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, @@ -563,7 +603,7 @@ 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 = od_get_current_url_metrics_etag( new OD_Tag_Visitor_Registry() ); + $current_etag = md5( '' ); $group_collection = new OD_URL_Metric_Group_Collection( array(), $current_etag, @@ -573,20 +613,48 @@ public function test_is_every_group_populated(): void { ); $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() ); diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index c8eb061484..448fbfd8dd 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -27,7 +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() ); + $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++ ) { @@ -80,7 +80,7 @@ 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() ), + '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(), From 6960a2871e478380390b96c634ab373851421bee Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 1 Dec 2024 18:12:21 -0800 Subject: [PATCH 48/48] Fix checked logic in old_url_metric test case --- .../tests/test-class-od-url-metrics-group.php | 7 ++++++- tests/class-optimization-detective-test-helpers.php | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) 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 62924cc343..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 @@ -251,7 +251,12 @@ 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 ) ) ), + '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. diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index 448fbfd8dd..c7924480a0 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -67,6 +67,7 @@ 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, @@ -84,6 +85,7 @@ public function get_sample_url_metric( array $params ): OD_URL_Metric { 'url' => home_url( '/' ), 'viewport_width' => 480, 'elements' => array(), + 'timestamp' => microtime( true ), ), $params ); @@ -100,7 +102,7 @@ public function get_sample_url_metric( array $params ): OD_URL_Metric { '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(