Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add ETag to post-processor response with support for 304 status #1481

Merged
merged 3 commits into from
Oct 3, 2018
Merged

Add ETag to post-processor response with support for 304 status #1481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36a591b
Add ETags to post-processor response with support for 304 status
westonruter Oct 1, 2018
19f6093
Send ETag header even in 304 Not Modified responses
westonruter Oct 3, 2018
73d50f7
Add test and send Server-Timing header for amp_processor_cache
westonruter Oct 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 47 additions & 10 deletions includes/class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1515,6 +1515,7 @@ public static function filter_customize_partial_render( $partial ) {
*/
public static function prepare_response( $response, $args = array() ) {
global $content_width;
$prepare_response_start = microtime( true );

if ( isset( $args['validation_error_callback'] ) ) {
_doing_it_wrong( __METHOD__, 'Do not supply validation_error_callback arg.', '1.0' );
Expand Down Expand Up @@ -1558,18 +1559,38 @@ public static function prepare_response( $response, $args = array() ) {
$args['enable_response_caching'] = ! $disable_response_caching;
}

/*
* Set response cache hash, the data values dictates whether a new hash key should be generated or not.
* This is also used as the ETag.
*/
$response_cache_key = md5( wp_json_encode( array(
$args,
$response,
self::$sanitizer_classes,
self::$embed_handlers,
AMP__VERSION,
) ) );

/*
* Per rfc7232:
* "The server generating a 304 response MUST generate any of the
* following header fields that would have been sent in a 200 (OK)
* response to the same request: Cache-Control, Content-Location, Date,
* ETag, Expires, and Vary." The only one of these headers which would
* not have been set yet during the WordPress template generation is
* the ETag. The AMP plugin sends a Vary header at amp_init.
*/
AMP_HTTP::send_header( 'ETag', $response_cache_key );

// Handle responses that are cached by the browser.
if ( isset( $_SERVER['HTTP_IF_NONE_MATCH'] ) && $_SERVER['HTTP_IF_NONE_MATCH'] === $response_cache_key ) {
status_header( 304 );
return '';
westonruter marked this conversation as resolved.
Show resolved Hide resolved
}

// Return cache if enabled and found.
$cache_response = null;
if ( true === $args['enable_response_caching'] ) {
// Set response cache hash, the data values dictates whether a new hash key should be generated or not.
$response_cache_key = md5( wp_json_encode( array(
$args,
$response,
self::$sanitizer_classes,
self::$embed_handlers,
AMP__VERSION,
) ) );

$response_cache = wp_cache_get( $response_cache_key, self::RESPONSE_CACHE_GROUP );

// Make sure that all of the validation errors should be sanitized in the same way; if not, then the cached body should be discarded.
Expand All @@ -1590,6 +1611,18 @@ public static function prepare_response( $response, $args = array() ) {
// Short-circuit response with cached body.
if ( isset( $response_cache['body'] ) ) {

// Re-send the headers that were sent before when the response was first cached.
if ( isset( $response_cache['headers'] ) ) {
foreach ( $response_cache['headers'] as $header ) {
if ( in_array( $header, AMP_HTTP::$headers_sent, true ) ) {
continue; // Skip sending headers that were already sent prior to post-processing.
}
AMP_HTTP::send_header( $header['name'], $header['value'], wp_array_slice_assoc( $header, array( 'replace', 'status_code' ) ) );
}
}

AMP_HTTP::send_server_timing( 'amp_processor_cache_hit', -$prepare_response_start );

// Redirect to non-AMP version.
if ( ! amp_is_canonical() && $blocking_error_count > 0 ) {
if ( AMP_Validation_Manager::has_cap() ) {
Expand All @@ -1611,7 +1644,11 @@ public static function prepare_response( $response, $args = array() ) {

return wp_cache_set(
$response_cache_key,
compact( 'body', 'validation_results' ),
array(
'headers' => AMP_HTTP::$headers_sent,
'body' => $body,
'validation_results' => $validation_results,
),
AMP_Theme_Support::RESPONSE_CACHE_GROUP,
MONTH_IN_SECONDS
);
Expand Down
85 changes: 62 additions & 23 deletions tests/test-class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,21 @@ public function test_exceeded_cache_miss_threshold() {
$this->assertTrue( AMP_Theme_Support::exceeded_cache_miss_threshold() );
}

/**
* Get the ETag header value from the list of sent headers.
*
* @param array $headers Headers sent.
* @return string|null The ETag if found.
*/
protected function get_etag_header_value( $headers ) {
foreach ( $headers as $header_sent ) {
if ( 'ETag' === $header_sent['name'] ) {
return $header_sent['value'];
}
}
return null;
}

/**
* Test prepare_response.
*
Expand All @@ -1160,9 +1175,23 @@ public function test_exceeded_cache_miss_threshold() {
* @covers \amp_render_scripts()
*/
public function test_prepare_response() {
$prepare_response_args = array(
'enable_response_caching' => false,
);

// phpcs:disable WordPress.WP.EnqueuedResources.NonEnqueuedScript, WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
$original_html = $this->get_original_html();
$sanitized_html = AMP_Theme_Support::prepare_response( $original_html );
$original_html = $this->get_original_html();

$call_prepare_response = function() use ( $original_html, &$prepare_response_args ) {
AMP_HTTP::$headers_sent = array();
AMP_Validation_Manager::$validation_results = array();
return AMP_Theme_Support::prepare_response( $original_html, $prepare_response_args );
};

$sanitized_html = $call_prepare_response();

$etag = $this->get_etag_header_value( AMP_HTTP::$headers_sent );
$this->assertNotEmpty( $etag );

$this->assertNotContains( 'handle=', $sanitized_html );
$this->assertEquals( 2, substr_count( $sanitized_html, '<!-- wp_print_scripts -->' ) );
Expand Down Expand Up @@ -1247,49 +1276,57 @@ public function test_prepare_response() {
// Make sure trailing content after </html> gets moved.
$this->assertRegExp( '#<!--comment-after-html-->\s*<div id="after-html"></div>\s*<!--comment-end-html-->\s*</body>\s*</html>\s*$#s', $sanitized_html );

$prepare_response_args = array(
'enable_response_caching' => true,
);
// Test that ETag allows response to short-circuit via If-None-Match request header.
$_SERVER['HTTP_IF_NONE_MATCH'] = $etag;
$this->assertEmpty( '', $call_prepare_response() );
$_SERVER['HTTP_IF_NONE_MATCH'] = $etag . '-v2';
$this->assertNotEmpty( $call_prepare_response() );
$this->assertNotEmpty( $this->get_etag_header_value( AMP_HTTP::$headers_sent ) );
unset( $_SERVER['HTTP_IF_NONE_MATCH'] );

$call_prepare_response = function() use ( $original_html, &$prepare_response_args ) {
AMP_HTTP::$headers_sent = array();
AMP_Validation_Manager::$validation_results = array();
return AMP_Theme_Support::prepare_response( $original_html, $prepare_response_args );
};
$prepare_response_args['enable_response_caching'] = true;

// Test that first response isn't cached.
$first_response = $call_prepare_response();
$this->assertGreaterThan( 0, $this->get_server_timing_header_count() );
$first_response = $call_prepare_response();
$initial_server_timing_headers = $this->get_server_timing_headers();
$this->assertGreaterThan( 0, count( $initial_server_timing_headers ) );
$this->assertContains( '<html amp="">', $first_response ); // Note: AMP because sanitized validation errors.
$this->reset_post_processor_cache_effectiveness();

// Test that response cache is return upon second call.
$this->assertEquals( $first_response, $call_prepare_response() );
$this->assertSame( 0, $this->get_server_timing_header_count() );
$server_timing_headers = $this->get_server_timing_headers();
$this->assertSame( count( $server_timing_headers ), count( $this->get_server_timing_headers() ) );
$this->reset_post_processor_cache_effectiveness();

// Test new cache upon argument change.
$prepare_response_args['test_reset_by_arg'] = true;
$call_prepare_response();
$this->assertGreaterThan( 0, $this->get_server_timing_header_count() );
$server_timing_headers = $this->get_server_timing_headers();
$this->assertSame( count( $server_timing_headers ), count( $this->get_server_timing_headers() ) );
$this->reset_post_processor_cache_effectiveness();

// Test response is cached.
$call_prepare_response();
$this->assertSame( 0, $this->get_server_timing_header_count() );
$server_timing_headers = $this->get_server_timing_headers();
$this->assertSame( count( $server_timing_headers ), count( $this->get_server_timing_headers() ) );
$this->reset_post_processor_cache_effectiveness();

// Test that response is no longer cached due to a change whether validation errors are sanitized.
remove_filter( 'amp_validation_error_sanitized', '__return_true' );
add_filter( 'amp_validation_error_sanitized', '__return_false' );
$prepared_html = $call_prepare_response();
$this->assertGreaterThan( 0, $this->get_server_timing_header_count() );
$prepared_html = $call_prepare_response();
$server_timing_headers = $this->get_server_timing_headers();
$this->assertSame( count( $server_timing_headers ), count( $this->get_server_timing_headers() ) );
$this->assertContains( '<html>', $prepared_html ); // Note: no AMP because unsanitized validation error.
$this->reset_post_processor_cache_effectiveness();

// And test response is cached.
$call_prepare_response();
$this->assertSame( 0, $this->get_server_timing_header_count() );
$server_timing_headers = $this->get_server_timing_headers();
$last_server_timing_header = array_pop( $server_timing_headers );
$this->assertStringStartsWith( 'amp_processor_cache_hit;', $last_server_timing_header['value'] );
$this->assertSame( count( $server_timing_headers ), count( $initial_server_timing_headers ) );

// phpcs:enable WordPress.WP.EnqueuedResources.NonEnqueuedScript, WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
}
Expand Down Expand Up @@ -1344,7 +1381,7 @@ public function test_post_processor_cache_effectiveness() {
$this->assertTrue( AMP_Options_Manager::get_option( 'enable_response_caching' ) );
}

$this->assertGreaterThan( 0, $this->get_server_timing_header_count() );
$this->assertGreaterThan( 0, count( $this->get_server_timing_headers() ) );
}

// Reset.
Expand Down Expand Up @@ -1439,15 +1476,17 @@ private function get_original_html() {
}

/**
* Returns the "Server-Timing" header count.
* Returns the "Server-Timing" headers.
*
* @return array Server-Timing headers.
*/
private function get_server_timing_header_count() {
return count( array_filter(
private function get_server_timing_headers() {
return array_filter(
AMP_HTTP::$headers_sent,
function( $header ) {
return 'Server-Timing' === $header['name'];
}
) );
);
}

/**
Expand Down