-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Site Health test to verify that static assets are served with far-future expires #1727
base: trunk
Are you sure you want to change the base?
Changes from 13 commits
dc9e364
4cb07aa
ec9a552
dd8e70d
0078cfc
3e5d958
35e7977
3db5886
655d4e5
20099bf
54f3621
0a776c5
3c98337
7afd746
3fcd641
54c73ec
6a2da47
48166ac
17d380e
200b518
1b4cef4
ed28196
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,299 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<?php | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Helper function to detect if static assets have far-future expiration headers. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @package performance-lab | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( ! defined( 'ABSPATH' ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
exit; // Exit if accessed directly. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Callback for the far-future caching test. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function perflab_ffh_assets_test(): array { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$result = array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'label' => __( 'Your site serves static assets with far-future expiration headers', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'status' => 'good', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'badge' => array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'label' => __( 'Performance', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'color' => 'blue', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'description' => sprintf( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'<p>%s</p>', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
esc_html__( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'Serving static assets with far-future expiration headers improves performance by allowing browsers to cache files for a long time, reducing repeated requests.', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'performance-lab' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'actions' => '', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'test' => 'is_far_future_headers_enabled', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// List of assets to check. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$assets = array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
includes_url( 'js/wp-embed.min.js' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
includes_url( 'css/buttons.min.css' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
includes_url( 'fonts/dashicons.woff2' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
includes_url( 'images/media/video.png' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
westonruter marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Filters the list of assets to check for far-future headers. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param string[] $assets List of asset URLs to check. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$assets = apply_filters( 'perflab_ffh_assets_to_check', $assets ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
westonruter marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Check if far-future headers are enabled for all assets. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$results = perflab_ffh_check_assets( $assets ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( 'good' !== $results['final_status'] ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$result['status'] = $results['final_status']; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$result['label'] = __( 'Your site does not serve static assets with recommended far-future expiration headers', 'performance-lab' ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above, maybe this could be rephrased to be at a higher level. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( count( $results['details'] ) > 0 ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$result['actions'] = sprintf( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'<p>%s</p>%s<p>%s</p>', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
esc_html__( 'The following file types do not have the recommended far-future headers. Consider adding or adjusting Cache-Control or Expires headers for these asset types.', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
perflab_ffh_get_extensions_table( $results['details'] ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
esc_html__( 'Note: "Conditionally cached" means that the browser can re-validate the resource using ETag or Last-Modified headers. This results in fewer full downloads but still requires the browser to make requests, unlike far-future expiration headers that allow the browser to fully rely on its local cache for a longer duration.', 'performance-lab' ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$result['actions'] = sprintf( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'<p>%s</p>', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
esc_html__( 'Far-future Cache-Control or Expires headers can be added or adjusted with a small configuration change by your hosting provider.', 'performance-lab' ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this explanation not present if there are |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two different checks which are getting performed one with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the new 3db5886 commit a table is used to display reason for different failure cases. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return $result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Checks if far-future expiration headers are enabled for a list of assets. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param string[] $assets List of asset URLs to check. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @return array{final_status: string, details: array{filename: string, reason: string}[]} Final status and details. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function perflab_ffh_check_assets( array $assets ): array { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$final_status = 'good'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$fail_details = array(); // Array of arrays with 'filename' and 'reason'. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
foreach ( $assets as $asset ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$response = wp_remote_get( $asset, array( 'sslverify' => false ) ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Extract filename from the URL. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$path_info = pathinfo( (string) wp_parse_url( $asset, PHP_URL_PATH ) ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$filename = isset( $path_info['basename'] ) ? $path_info['basename'] : basename( $asset ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( is_wp_error( $response ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Can't determine headers if request failed, consider it a fail. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$final_status = 'recommended'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$fail_details[] = array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'filename' => $filename, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => __( 'Could not retrieve headers', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$headers = wp_remote_retrieve_headers( $response ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( ! is_object( $headers ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This error is encountered when using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's surprising that |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// No valid headers retrieved. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$final_status = 'recommended'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$fail_details[] = array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'filename' => $filename, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => __( 'No valid headers retrieved', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$check = perflab_ffh_check_headers( $headers ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( isset( $check['passed'] ) && $check['passed'] ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// This asset passed far-future headers test, no action needed. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// If not passed, decide whether to try conditional request. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( false === $check ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Only if no far-future headers at all, we try conditional request. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$conditional_pass = perflab_ffh_try_conditional_request( $asset, $headers ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( ! $conditional_pass ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$final_status = 'recommended'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$fail_details[] = array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'filename' => $filename, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => __( 'No far-future headers and no conditional caching', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$final_status = 'recommended'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$fail_details[] = array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'filename' => $filename, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => __( 'No far-future headers but conditionally cached', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Factoring
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// If there's a max-age or expires but below threshold, we skip conditional. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$final_status = 'recommended'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$fail_details[] = array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'filename' => $filename, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => $check['reason'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'final_status' => $final_status, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'details' => $fail_details, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently even if one asset fails the header check then this test will be show in the site health. Would it be better to show a table with mime type which will specifically tell for which mime type needs to add the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A table makes sense to me. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Checks if far-future expiration headers are enabled. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param WpOrg\Requests\Utility\CaseInsensitiveDictionary $headers Response headers. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @return array{passed: bool, reason: string}|false Detailed result. If passed=false, reason explains why it failed and false if no headers found. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function perflab_ffh_check_headers( WpOrg\Requests\Utility\CaseInsensitiveDictionary $headers ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would raise a PHPStan error; it seems we also need to provide the type of array. Below is what I used to solve it: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, good point. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Filters the threshold for far-future headers. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param int $threshold Threshold in seconds. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$threshold = apply_filters( 'perflab_far_future_headers_threshold', YEAR_IN_SECONDS ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$cache_control = isset( $headers['cache-control'] ) ? $headers['cache-control'] : ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$expires = isset( $headers['expires'] ) ? $headers['expires'] : ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this works even when
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Check Cache-Control header for max-age. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$max_age = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( '' !== $cache_control ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// There can be multiple cache-control headers, we only care about max-age. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$controls = is_array( $cache_control ) ? $cache_control : array( $cache_control ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
foreach ( $controls as $control ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems this could be simplified a bit:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( (bool) preg_match( '/max-age\s*=\s*(\d+)/', $control, $matches ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because PhpStorm complains about the unnecessary cast, but then without it PHPStan complains about "Only booleans are allowed in an if condition, int|false given."
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$max_age = (int) $matches[1]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// If max-age meets or exceeds the threshold, we consider it good. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( $max_age >= $threshold ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'passed' => true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => '', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// If max-age is too low or not present, check Expires. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( is_string( $expires ) && '' !== $expires ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$expires_time = strtotime( $expires ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( (bool) $expires_time && ( $expires_time - time() ) >= $threshold ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Good - Expires far in the future. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'passed' => true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => '', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Expires header exists but not far enough in the future. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( $max_age > 0 && $max_age < $threshold ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition doesn't seem to be necessary:
Suggested change
Because above we're already doing: if ( $max_age >= $threshold ) {
return array(
'passed' => true,
'reason' => '',
);
} So we already know that |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'passed' => false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => __( 'max-age below threshold', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'passed' => false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => __( 'expires below threshold', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// No max-age or expires found at all or max-age < threshold and no expires. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( 0 === $max_age ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// max-age was present but below threshold and no expires. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'passed' => false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'reason' => __( 'max-age below threshold', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would seem helpful to indicate the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes sense I have added the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think converting the expires to seconds makes sense. I don't think we need to worry about converting to anything other than seconds, however. Just pass the number of seconds through |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Attempt a conditional request with ETag/Last-Modified. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param string $url The asset URL. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param WpOrg\Requests\Utility\CaseInsensitiveDictionary $headers The initial response headers. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @return bool True if a 304 response was received. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function perflab_ffh_try_conditional_request( string $url, WpOrg\Requests\Utility\CaseInsensitiveDictionary $headers ): bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
westonruter marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$etag = isset( $headers['etag'] ) ? $headers['etag'] : ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$last_modified = isset( $headers['last-modified'] ) ? $headers['last-modified'] : ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$conditional_headers = array(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( '' !== $etag ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$conditional_headers['If-None-Match'] = $etag; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( '' !== $last_modified ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$conditional_headers['If-Modified-Since'] = $last_modified; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$response = wp_remote_get( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
array( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'sslverify' => false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'headers' => $conditional_headers, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( is_wp_error( $response ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$status_code = wp_remote_retrieve_response_code( $response ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ( 304 === $status_code ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Generate a table listing files that need far-future headers, including reasons. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param array<array{filename: string, reason: string}> $fail_details Array of arrays with 'filename' and 'reason'. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @return string HTML formatted table. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
function perflab_ffh_get_extensions_table( array $fail_details ): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$html_table = sprintf( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'<table class="widefat striped"><thead><tr><th scope="col">%s</th><th scope="col">%s</th></tr></thead><tbody>', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
esc_html__( 'File', 'performance-lab' ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
esc_html__( 'Status', 'performance-lab' ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
foreach ( $fail_details as $detail ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$html_table .= sprintf( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'<tr><td>%s</td><td>%s</td></tr>', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
esc_html( $detail['filename'] ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
esc_html( $detail['reason'] ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$html_table .= '</tbody></table>'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return $html_table; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
<?php | ||
/** | ||
* Hook callbacks used for far-future headers. | ||
* | ||
* @package performance-lab | ||
* @since n.e.x.t | ||
*/ | ||
|
||
if ( ! defined( 'ABSPATH' ) ) { | ||
exit; // Exit if accessed directly. | ||
} | ||
|
||
/** | ||
* Adds tests to site health. | ||
* | ||
* @since n.e.x.t | ||
* | ||
* @param array{direct: array<string, array{label: string, test: string}>} $tests Site Health Tests. | ||
* @return array{direct: array<string, array{label: string, test: string}>} Amended tests. | ||
*/ | ||
function perflab_ffh_add_test( array $tests ): array { | ||
$tests['direct']['far_future_headers'] = array( | ||
'label' => __( 'Far-Future Caching Headers', 'performance-lab' ), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above, IMO this is too technical for the heading. How about "Effective Caching Headers"? |
||
'test' => 'perflab_ffh_assets_test', | ||
); | ||
return $tests; | ||
} | ||
add_filter( 'site_status_tests', 'perflab_ffh_add_test' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "far-future expiration" a term used elsewhere? If yes, that's fine, but otherwise it sounds a bit strange to me. To a non-technical user for instance it does not convey at all whether that's a good thing or a bad thing.
Maybe this could be changed to something like "Your site serves static assets with an effective caching strategy"? We could still mention the more technical explanation in the description.