Add Site Health test for full page caching (advanced cache)

[westonruter]

Feature Description

I previously reported this in trac (#54423), but I'm copying it here for visibility:

A best practice for performance is to enable full page caching for a WordPress site. Doing so reduces the time to first byte (TTFB) and thus directly impacts Core Web Vitals. This is especially important on shared hosts which have limited compute power, while also protecting a site from going down due to traffic spikes.

As with XML Sitemaps, it may make sense for WordPress core to include a basic full page caching solution out of the box which can be extended by the many plugins that provide page caching today. In the meantime, there should at least be a Site Health test with a low-severity recommended status to point users to where to go to enable page caching. Since hosts often have built-in caching solutions in place, the Site Health test should be easily filterable so they can provide a link to their specific dashboard or documentation to enable caching which may not be a plugin at all but rather a reverse proxy like Varnish.

In the context of the AMP plugin for WordPress, we've already developed such a test which could be adapted for core. See ampproject/amp-wp#6456. It's an async test which issues three loopback requests to the homepage, and the test will pass if responses include either of the following headers:

1. Cache-Control with non-zero max-age.
2. Expires with a future time.
3. Age with a positive number (as is set via a proxy like Varnish).

Since the above was written and, after releasing the page caching site health test in the AMP plugin v2.2, it came to light that the detection was not robust enough. A major improvement followed (ampproject/amp-wp#6849) which accounts for many more scenarios (ampproject/amp-wp#6849 (comment)). These were uncovered with help from @piotrbak in a WP Rocket issue (wp-media/wp-rocket#4638 (comment)).

The page caching code in question: https://github.com/ampproject/amp-wp/blob/f7c7230a27fb23932606f9847bfcf04c83b8ff06/src/Admin/SiteHealth.php#L333-L661

Scenarios

Via ampproject/amp-wp#6849 (comment):

Failure to Do Loopback Request

With this plugin active:

add_action('template_redirect', function () {
	status_header( 500 );
	exit;
});

No Page Caching with Slow Response Time

With this plugin active:

add_action( 'wp_footer', function () {
	sleep(1);
} );

No Page Caching with Good Response Time

No special plugins active.

Page Caching Header(s) with Bad Response Time

Test plugin:

add_filter(
	'wp_headers',
	function ( $headers ) {
		$headers['etag'] = '"foo"';
		$headers['last-modified'] = gmdate('r');
		return $headers;
	}
);

add_action( 'wp_footer', function () {
	sleep(1);
} );

Note that since page caching response headers are present, the lack of a page caching plugin is not mentioned (since it's likely an external page cache is being used):

Page Caching Header with Good Response Time

Plugin:

add_filter(
	'wp_headers',
	function ( $headers ) {
		$headers['etag'] = '"foo"';
		$headers['last-modified'] = gmdate('r');
		return $headers;
	}
);

Note that since page caching response headers are present, the lack of a page caching plugin is not mentioned (since it's likely an external page cache is being used):

Page Caching Plugin with Good Response Time

With WP Super Cache active and with the following plugin active:

add_action( 'wp_footer', function () {
	sleep(1);
} );

Note that since the median time is used, the 1-second response time is disregarded:

[manuelRod]

Hello, @westonruter I would love to take a look, you can assign it to me!

[eclarke1]

Just added this to the Site Health project and it's currently in the backlog. I've added the 'Needs Dev' label and assigned to @manuelRod

[manuelRod]

@westonruter you can take a look at my draft: PR
Are you missing something?

As I was checking AMP code, I figured out a couple of things:

• Notice :
  [24-Mar-2022 16:40:35 UTC] PHP Notice: Undefined variable: header_value in /Users/xxx/Desktop/development/xwp/app/public/wp-content/plugins/amp-wp-develop/src/Admin/SiteHealth.php on line 542
  get_page_cache_headers() is provoking that notice as $header_value is not passed as argument.

• Unit test Error:
  Callback for cache-control is expecting a string as $header_value, not an array, that is failing the test when using this index from no-cache-arrays from get_page_cache_data dataprovider.
  I've implemented a workaround on my PR

[westonruter]

@manuelRod I think you may have been seeing ampproject/amp-wp#6937 which is addressed by ampproject/amp-wp#6938.

The $header_value should be getting passed via array_filter() here: https://github.com/ampproject/amp-wp/blob/4bd0387b0a68c06806f139d15befe9de21717b79/src/Admin/SiteHealth.php#L638

[manuelRod]

@westonruter indeed, that was it. I've implemented your approach and fixed the tests, it's ready to be reviewed.

[manuelRod]

@eclarke1 can you add the Needs Review tag on this? Thanks!

[eclarke1]

@manuelRod absolutely 👍 done and also moved to the 'In Review' column. Thanks so much

[manuelRod]

@westonruter Thanks for the feedback, I've addressed it. We need another reviewer, maybe @felixarntz wants to take an eye?

[tommydv]

Hello westonruter.

I am the owner of a wordpress website.

I use amp plugin and wp rocket plugin.

I then removed wp rocket and replaced it with wp super cache.

Checking my system got this message.

Can you show me how to fix this error?

Thank you.

[mxbclang]

@tommydv It sounds like your issue is not related to the Performance Lab plugin, which is what this repo and issue are for. Can you please post to the AMP plugin support forum for assistance? Thanks!

[manuelRod]

@felixarntz why do we move this to 1.2.0? The PR is ready, it could get in this week.
I saw the comment in the PR.

[felixarntz]

@manuelRod @westonruter I've added a [Module] Audit Full Page Cache label for the eventual module here.