Merge pull request #1940 from ShyamGadde/add/option-to-disable-freshness-ttl

Allow disabling timestamp-based freshness checks by using negative TTL values

Author: westonruter

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<string, float>,
 	 *             common_lcp_element: ?OD_Element,

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,

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.'

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)

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<string, mixed> $data Data.
 	 */

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() );
 	}
 
 	/**

plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php

@@ -22,79 +22,79 @@ 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(),
 				'sample_size'   => 3,
 				'freshness_ttl' => HOUR_IN_SECONDS,
 				'exception'     => '',
 			),
-			'negative_breakpoint_bad'    => array(
+			'negative_breakpoint_bad'   => array(
 				'url_metrics'   => array(),
 				'current_etag'  => $current_etag,
 				'breakpoints'   => array( -1 ),
 				'sample_size'   => 3,
 				'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 ),
 				'sample_size'   => 3,
 				'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' ),
 				'sample_size'   => 3,
 				'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 ),
 				'sample_size'   => -3,
 				'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 ),
 				'sample_size'   => 3,
 				'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 ),
 				'sample_size'   => 3,
 				'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 ),
 				'sample_size'   => 3,
 				'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() );
+		}
 	}
 
 	/**