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 9ffb7ee39d..202dba7faf 100644 --- a/plugins/optimization-detective/class-od-url-metric-group-collection.php +++ b/plugins/optimization-detective/class-od-url-metric-group-collection.php @@ -73,7 +73,7 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega * A freshness age of zero means a URL Metric will always be considered stale. * * @since 0.1.0 - * @var int<0, max> + * @var int<-1, max> */ private $freshness_ttl; @@ -104,7 +104,7 @@ final class OD_URL_Metric_Group_Collection implements Countable, IteratorAggrega * * @phpstan-param positive-int[] $breakpoints * @phpstan-param int<1, max> $sample_size - * @phpstan-param int<0, max> $freshness_ttl + * @phpstan-param int<-1, max> $freshness_ttl * * @param OD_URL_Metric[] $url_metrics URL Metrics. * @param non-empty-string $current_etag The current ETag. @@ -168,18 +168,7 @@ public function __construct( array $url_metrics, string $current_etag, array $br $this->sample_size = $sample_size; // Set freshness TTL. - if ( $freshness_ttl < 0 ) { - throw new InvalidArgumentException( - esc_html( - sprintf( - /* translators: %d is the invalid sample size */ - __( 'Freshness TTL must be at least zero, but provided: %d', 'optimization-detective' ), - $freshness_ttl - ) - ) - ); - } - $this->freshness_ttl = $freshness_ttl; + $this->freshness_ttl = max( -1, $freshness_ttl ); // Create groups and the URL Metrics to them. $this->groups = $this->create_groups(); @@ -226,7 +215,7 @@ public function get_sample_size(): int { * * @since 1.0.0 * - * @return int<0, max> Freshness age (TTL) for a given URL Metric. + * @return int<-1, max> Freshness age (TTL) for a given URL Metric. */ public function get_freshness_ttl(): int { return $this->freshness_ttl; @@ -702,7 +691,7 @@ public function count(): int { * @return array{ * current_etag: non-empty-string, * breakpoints: positive-int[], - * freshness_ttl: 0|positive-int, + * freshness_ttl: int<-1, max>, * sample_size: positive-int, * all_element_max_intersection_ratios: array, * common_lcp_element: ?OD_Element, diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 7d756005be..e1225f7657 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -62,7 +62,7 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer * * @since 0.1.0 * - * @var int<0, max> + * @var int<-1, max> */ private $freshness_ttl; @@ -102,7 +102,7 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer * @phpstan-param int<0, max> $minimum_viewport_width * @phpstan-param int<1, max>|null $maximum_viewport_width * @phpstan-param int<1, max> $sample_size - * @phpstan-param int<0, max> $freshness_ttl + * @phpstan-param int<-1, max> $freshness_ttl * * @param OD_URL_Metric[] $url_metrics URL Metrics to add to the group. * @param int $minimum_viewport_width Minimum possible viewport width (exclusive) for the group. Must be zero or greater. @@ -145,18 +145,7 @@ public function __construct( array $url_metrics, int $minimum_viewport_width, ?i } $this->sample_size = $sample_size; - if ( $freshness_ttl < 0 ) { - throw new InvalidArgumentException( - esc_html( - sprintf( - /* translators: %d is the invalid sample size */ - __( 'Freshness TTL must be at least zero, but provided: %d', 'optimization-detective' ), - $freshness_ttl - ) - ) - ); - } - $this->freshness_ttl = $freshness_ttl; + $this->freshness_ttl = max( -1, $freshness_ttl ); $this->collection = $collection; $this->url_metrics = $url_metrics; } @@ -203,7 +192,7 @@ public function get_sample_size(): int { * @since 0.9.0 * * @todo Eliminate in favor of readonly public property. - * @return int<0, max> Freshness age. + * @return int<-1, max> Freshness age. */ public function get_freshness_ttl(): int { return $this->freshness_ttl; @@ -285,6 +274,7 @@ static function ( OD_URL_Metric $a, OD_URL_Metric $b ): int { * * @since 0.1.0 * @since 0.9.0 If the current environment's generated ETag does not match the URL Metric's ETag, the URL Metric is considered stale. + * @since n.e.x.t Negative freshness TTL values now disable timestamp-based freshness checks. * * @return bool Whether complete. */ @@ -299,8 +289,8 @@ 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 ) { + // The URL Metric is too old to be fresh (skip if freshness TTL is negative). + if ( $this->freshness_ttl >= 0 && $current_time > $url_metric->get_timestamp() + $this->freshness_ttl ) { return false; } @@ -514,7 +504,7 @@ public function clear_cache(): void { * @since 0.3.1 * * @return array{ - * freshness_ttl: 0|positive-int, + * freshness_ttl: int<-1, max>, * sample_size: positive-int, * minimum_viewport_width: int<0, max>, * maximum_viewport_width: int<1, max>|null, diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index c786db340b..4ec463ace1 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -434,7 +434,8 @@ export default async function detect( { ); if ( ! isNaN( previousVisitTime ) && - ( getCurrentTime() - previousVisitTime ) / 1000 < freshnessTTL + ( freshnessTTL < 0 || + ( getCurrentTime() - previousVisitTime ) / 1000 < freshnessTTL ) ) { log( 'The current client session already submitted a fresh URL Metric for this URL so a new one will not be collected now.' diff --git a/plugins/optimization-detective/docs/hooks.md b/plugins/optimization-detective/docs/hooks.md index 8cf55a0898..50d35b9195 100644 --- a/plugins/optimization-detective/docs/hooks.md +++ b/plugins/optimization-detective/docs/hooks.md @@ -238,22 +238,28 @@ add_filter( 'od_metrics_storage_lock_ttl', function ( int $ttl ): int { ### Filter: `od_url_metric_freshness_ttl` (default: 1 week in seconds) -Filters the freshness age (TTL) for a given URL Metric. The freshness TTL must be at least zero, in which it considers URL Metrics to always be stale. In practice, the value should be at least an hour. If your site content does not change frequently, you may want to increase the TTL even longer, say to a month: +Filters age (TTL) for which a URL Metric can be considered fresh. + +The freshness TTL (time to live) value can be one of the following values: + +* A positive integer (e.g. `3600`, `HOUR_IN_SECONDS`) allows a URL Metric to be fresh for a given period of time into the future. +* A negative integer (`-1`) disables timestamp-based freshness checks, making URL Metrics stay fresh indefinitely unless the current ETag changes. +* A value of zero (`0`) considers URL Metrics to always be stale, which is useful during development. _Never do this on a production site since this can cause a database write for every visitor!_ + +The default value is `WEEK_IN_SECONDS` since changes to the post/page (or the site overall) will cause a change to the current ETag used for URL Metrics. This causes the relevant existing URL Metrics with the previous ETag to be considered stale, allowing new URL Metrics to be collected before the freshness TTL has expired. See the `od_current_url_metrics_etag_data` filter to customize the ETag data. + +For sites where content doesn't change frequently, you can disable the timestamp-based staleness check as follows: ```php add_filter( 'od_url_metric_freshness_ttl', static function (): int { - return MONTH_IN_SECONDS; + return -1; } ); ``` -Note that even if you have large freshness TTL a URL Metric can still become stale sooner; if the page state changes then this results in a change to the ETag associated with a URL Metric. This will allow new URL Metrics to be collected before the freshness TTL has transpired. See the `od_current_url_metrics_etag_data` filter to customize the ETag data. - -During development, this can be useful to set to zero so that you don't have to wait for new URL Metrics to be requested when engineering a new optimization: +As noted above, during development you can set the freshness TTL to zero so that you don't have to wait for new URL Metrics to be requested when developing a new optimization: ```php -add_filter( 'od_url_metric_freshness_ttl', static function (): int { - return 0; -} ); +add_filter( 'od_url_metric_freshness_ttl', '__return_zero' ); ``` ### Filter: `od_minimum_viewport_aspect_ratio` (default: 0.4) diff --git a/plugins/optimization-detective/storage/data.php b/plugins/optimization-detective/storage/data.php index a9848d5267..471c452514 100644 --- a/plugins/optimization-detective/storage/data.php +++ b/plugins/optimization-detective/storage/data.php @@ -20,37 +20,20 @@ * @since 0.1.0 * @access private * - * @return int<0, max> Expiration TTL in seconds. + * @return int<-1, max> Expiration TTL in seconds. */ function od_get_url_metric_freshness_ttl(): int { /** - * Filters the freshness age (TTL) for a given URL Metric. - * - * The freshness TTL must be at least zero, in which it considers URL Metrics to always be stale. - * In practice, the value should be at least an hour. + * Filters age (TTL) for which a URL Metric can be considered fresh. * * @since 0.1.0 + * @since n.e.x.t Negative values disable timestamp-based freshness checks. + * @link https://github.com/WordPress/performance/blob/trunk/plugins/optimization-detective/docs/hooks.md#:~:text=Filter%3A%20od_url_metric_freshness_ttl * * @param int $ttl Expiration TTL in seconds. Defaults to 1 week. */ - $freshness_ttl = (int) apply_filters( 'od_url_metric_freshness_ttl', WEEK_IN_SECONDS ); - - if ( $freshness_ttl < 0 ) { - _doing_it_wrong( - esc_html( "Filter: 'od_url_metric_freshness_ttl'" ), - esc_html( - sprintf( - /* translators: %s is the TTL freshness */ - __( 'Freshness TTL must be at least zero, but saw "%s".', 'optimization-detective' ), - $freshness_ttl - ) - ), - '' - ); - $freshness_ttl = 0; - } - - return $freshness_ttl; + $ttl = (int) apply_filters( 'od_url_metric_freshness_ttl', WEEK_IN_SECONDS ); + return max( -1, $ttl ); } /** @@ -249,6 +232,7 @@ static function ( $post ): ?array { * Filters the data that goes into computing the current ETag for URL Metrics. * * @since 0.9.0 + * @link https://github.com/WordPress/performance/blob/trunk/plugins/optimization-detective/docs/hooks.md#:~:text=Filter%3A%20od_current_url_metrics_etag_data * * @param array $data Data. */ diff --git a/plugins/optimization-detective/tests/storage/test-data.php b/plugins/optimization-detective/tests/storage/test-data.php index df72483063..180ae7ea97 100644 --- a/plugins/optimization-detective/tests/storage/test-data.php +++ b/plugins/optimization-detective/tests/storage/test-data.php @@ -46,20 +46,19 @@ static function (): int { } /** - * Test bad od_get_url_metric_freshness_ttl(). + * Test negative od_get_url_metric_freshness_ttl(). * * @covers ::od_get_url_metric_freshness_ttl */ - public function test_bad_od_get_url_metric_freshness_ttl(): void { - $this->setExpectedIncorrectUsage( 'Filter: 'od_url_metric_freshness_ttl'' ); + public function test_negative_od_get_url_metric_freshness_ttl(): void { add_filter( 'od_url_metric_freshness_ttl', static function (): int { - return -1; + return -12345; } ); - $this->assertSame( 0, od_get_url_metric_freshness_ttl() ); + $this->assertSame( -1, od_get_url_metric_freshness_ttl() ); } /** 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 44a0ecd228..1a85baea3f 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,7 +22,7 @@ public function data_provider_test_construction(): array { $current_etag = md5( '' ); return array( - 'no_breakpoints_ok' => array( + 'no_breakpoints_ok' => array( 'url_metrics' => array(), 'current_etag' => $current_etag, 'breakpoints' => array(), @@ -30,7 +30,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => '', ), - 'negative_breakpoint_bad' => array( + 'negative_breakpoint_bad' => array( 'url_metrics' => array(), 'current_etag' => $current_etag, 'breakpoints' => array( -1 ), @@ -38,7 +38,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'zero_breakpoint_bad' => array( + 'zero_breakpoint_bad' => array( 'url_metrics' => array(), 'current_etag' => $current_etag, 'breakpoints' => array( 0 ), @@ -46,7 +46,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'string_breakpoint_bad' => array( + 'string_breakpoint_bad' => array( 'url_metrics' => array(), 'current_etag' => $current_etag, 'breakpoints' => array( 'narrow' ), @@ -54,7 +54,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'negative_sample_size_bad' => array( + 'negative_sample_size_bad' => array( 'url_metrics' => array(), 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), @@ -62,15 +62,15 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'negative_freshness_tll_bad' => array( + 'negative_freshness_ttl_ok' => array( 'url_metrics' => array(), 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), 'sample_size' => 3, 'freshness_ttl' => -HOUR_IN_SECONDS, - 'exception' => InvalidArgumentException::class, + 'exception' => '', ), - 'invalid_current_etag_bad' => array( + 'invalid_current_etag_bad' => array( 'url_metrics' => array(), 'current_etag' => 'invalid_etag', 'breakpoints' => array( 400 ), @@ -78,7 +78,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'invalid_current_etag_bad2' => array( + '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 ), @@ -86,7 +86,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'invalid_url_metrics_bad' => array( + 'invalid_url_metrics_bad' => array( 'url_metrics' => array( 'bad' ), 'current_etag' => $current_etag, 'breakpoints' => array( 400 ), @@ -94,7 +94,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => TypeError::class, ), - 'all_arguments_good' => array( + 'all_arguments_good' => array( 'url_metrics' => array( $this->get_sample_url_metric( array( 'viewport_width' => 200 ) ), $this->get_sample_url_metric( array( 'viewport_width' => 400 ) ), @@ -135,7 +135,11 @@ public function test_construction( array $url_metrics, string $current_etag, arr $this->assertSame( $current_etag, $group_collection->get_current_etag() ); $this->assertSame( $sample_size, $group_collection->get_sample_size() ); $this->assertSame( $breakpoints, $group_collection->get_breakpoints() ); - $this->assertSame( $freshness_ttl, $group_collection->get_freshness_ttl() ); + if ( $freshness_ttl < 0 ) { + $this->assertSame( -1, $group_collection->get_freshness_ttl() ); + } else { + $this->assertSame( $freshness_ttl, $group_collection->get_freshness_ttl() ); + } } /** 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 209bb2b26d..0b4e2b0db9 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 @@ -20,7 +20,7 @@ class Test_OD_URL_Metric_Group extends WP_UnitTestCase { */ public function data_provider_test_construction(): array { return array( - 'bad_minimum_viewport_width' => array( + 'bad_minimum_viewport_width' => array( 'url_metrics' => array(), 'minimum_viewport_width' => -1, 'maximum_viewport_width' => 100, @@ -28,7 +28,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'bad_maximum_viewport_width' => array( + 'bad_maximum_viewport_width' => array( 'url_metrics' => array(), 'minimum_viewport_width' => 0, 'maximum_viewport_width' => -1, @@ -36,7 +36,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'bad_min_max_viewport_width' => array( + 'bad_min_max_viewport_width' => array( 'url_metrics' => array(), 'minimum_viewport_width' => 200, 'maximum_viewport_width' => 100, @@ -44,7 +44,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'bad_sample_size_viewport_width' => array( + 'bad_sample_size_viewport_width' => array( 'url_metrics' => array(), 'minimum_viewport_width' => 0, 'maximum_viewport_width' => 100, @@ -52,15 +52,15 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => InvalidArgumentException::class, ), - 'bad_freshness_ttl_viewport_width' => array( + 'negative_freshness_ttl_ok' => array( 'url_metrics' => array(), 'minimum_viewport_width' => 0, 'maximum_viewport_width' => 100, 'sample_size' => 3, - 'freshness_ttl' => -HOUR_IN_SECONDS, - 'exception' => InvalidArgumentException::class, + 'freshness_ttl' => -1, + 'exception' => '', ), - 'good_empty_url_metrics' => array( + 'good_empty_url_metrics' => array( 'url_metrics' => array(), 'minimum_viewport_width' => 0, 'maximum_viewport_width' => 100, @@ -68,7 +68,7 @@ public function data_provider_test_construction(): array { 'freshness_ttl' => HOUR_IN_SECONDS, 'exception' => '', ), - 'good_one_url_metric' => array( + 'good_one_url_metric' => array( 'url_metrics' => array( new OD_URL_Metric( array( @@ -260,23 +260,41 @@ public function test_add_url_metric( int $viewport_width, string $exception ): v 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( + 'old_url_metric' => array( 'url_metric' => $this->get_sample_url_metric( array( 'timestamp' => microtime( true ) - ( HOUR_IN_SECONDS + 1 ), 'etag' => md5( '' ), ) ), + 'freshness_ttl' => HOUR_IN_SECONDS, 'expected_is_group_complete' => false, ), - 'etag_mismatch' => array( + 'etag_mismatch' => array( 'url_metric' => $this->get_sample_url_metric( array( 'etag' => md5( 'different_etag' ) ) ), + 'freshness_ttl' => HOUR_IN_SECONDS, 'expected_is_group_complete' => false, ), - 'etag_match' => array( + 'etag_match' => array( 'url_metric' => $this->get_sample_url_metric( array( 'etag' => md5( '' ) ) ), + 'freshness_ttl' => HOUR_IN_SECONDS, + 'expected_is_group_complete' => true, + ), + 'negative_ttl_with_old_url_metric' => array( + 'url_metric' => $this->get_sample_url_metric( + array( + 'timestamp' => microtime( true ) - ( WEEK_IN_SECONDS * 4 ), + 'etag' => md5( '' ), + ) + ), + 'freshness_ttl' => -1, 'expected_is_group_complete' => true, ), + 'negative_ttl_with_etag_mismatch' => array( + 'url_metric' => $this->get_sample_url_metric( array( 'etag' => md5( 'different_etag' ) ) ), + 'freshness_ttl' => -1, + 'expected_is_group_complete' => false, + ), ); } @@ -284,16 +302,25 @@ public function data_provider_test_is_complete(): array { * Test is_complete(). * * @covers ::is_complete + * @covers ::get_freshness_ttl + * @covers OD_URL_Metric_Group_Collection::get_freshness_ttl * * @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 ); + public function test_is_complete( OD_URL_Metric $url_metric, int $freshness_ttl, bool $expected_is_group_complete ): void { + $collection = new OD_URL_Metric_Group_Collection( array(), md5( '' ), array( 768 ), 1, $freshness_ttl ); $group = $collection->get_first_group(); $group->add_url_metric( $url_metric ); $this->assertSame( $expected_is_group_complete, $group->is_complete() ); + if ( $freshness_ttl < 0 ) { + $this->assertSame( -1, $collection->get_freshness_ttl() ); + $this->assertSame( -1, $group->get_freshness_ttl() ); + } else { + $this->assertSame( $freshness_ttl, $collection->get_freshness_ttl() ); + $this->assertSame( $freshness_ttl, $group->get_freshness_ttl() ); + } } /**