From 091a4a2ee12b8b7d77e8ee1a9c257c94267172e2 Mon Sep 17 00:00:00 2001 From: ThierryA Date: Thu, 17 May 2018 15:25:13 -0700 Subject: [PATCH 1/8] Issue 959: cache post processor response --- includes/class-amp-theme-support.php | 37 ++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 97d76280738..8bffadb0fc5 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -19,6 +19,13 @@ class AMP_Theme_Support { */ const SCRIPTS_PLACEHOLDER = ''; + /** + * Response cache group name. + * + * @var string + */ + const RESPONSE_CACHE_GROUP = 'amp-reponse'; + /** * Sanitizer classes. * @@ -1028,6 +1035,13 @@ public static function prepare_response( $response, $args = array() ) { } $is_validation_debug_mode = isset( $_REQUEST[ AMP_Validation_Utils::DEBUG_QUERY_VAR ] ); // WPCS: csrf ok. + $is_cache_enabled = ( + ! defined( 'WP_DEBUG' ) + || + true !== WP_DEBUG + || + $is_validation_debug_mode + ); $args = array_merge( array( @@ -1040,6 +1054,24 @@ public static function prepare_response( $response, $args = array() ) { $args ); + // Return cache if enabled and found. + if ( $is_cache_enabled ) { + // Set response cache hash. + $response_cache_key = md5( maybe_serialize( array( + 'sanitizer_classes' => self::$sanitizer_classes, + 'embed_handlers' => self::$embed_handlers, + 'args' => $args, + 'response' => $response, + 'plugin_version' => AMP__VERSION, + ) ) ); + + $response_cache = wp_cache_get( $response_cache_key, self::RESPONSE_CACHE_GROUP ); + + if ( ! empty( $response_cache ) ) { + return $response_cache; + } + } + $dom_parse_start = microtime( true ); /* @@ -1140,6 +1172,11 @@ public static function prepare_response( $response, $args = array() ) { AMP_Response_Headers::send_server_timing( 'amp_dom_serialize', -$dom_serialize_start, 'AMP DOM Serialize' ); + // Cache the response if enabled. + if ( $is_cache_enabled ) { + wp_cache_set( $response_cache_key, $response, self::RESPONSE_CACHE_GROUP, MONTH_IN_SECONDS ); + } + return $response; } From 82b6b88148f1747f35f6f3c932974782ea72d761 Mon Sep 17 00:00:00 2001 From: ThierryA Date: Tue, 22 May 2018 12:52:25 +0200 Subject: [PATCH 2/8] Issue 959: code improvements + closure serialization fix --- includes/class-amp-theme-support.php | 70 +++++++++++++++++++--------- 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 8bffadb0fc5..a1f44771252 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -26,6 +26,13 @@ class AMP_Theme_Support { */ const RESPONSE_CACHE_GROUP = 'amp-reponse'; + /** + * Response cache hash key. + * + * @var string + */ + public static $response_cache_key; + /** * Sanitizer classes. * @@ -977,8 +984,6 @@ public static function is_output_buffering() { * @return string Processed Response. */ public static function finish_output_buffering( $response ) { - AMP_Response_Headers::send_server_timing( 'amp_output_buffer', -self::$init_start_time, 'AMP Output Buffer' ); - self::$is_output_buffering = false; return self::prepare_response( $response ); } @@ -1035,13 +1040,6 @@ public static function prepare_response( $response, $args = array() ) { } $is_validation_debug_mode = isset( $_REQUEST[ AMP_Validation_Utils::DEBUG_QUERY_VAR ] ); // WPCS: csrf ok. - $is_cache_enabled = ( - ! defined( 'WP_DEBUG' ) - || - true !== WP_DEBUG - || - $is_validation_debug_mode - ); $args = array_merge( array( @@ -1050,28 +1048,29 @@ public static function prepare_response( $response, $args = array() ) { 'allow_dirty_styles' => self::is_customize_preview_iframe(), // Dirty styles only needed when editing (e.g. for edit shortcodes). 'allow_dirty_scripts' => is_customize_preview(), // Scripts are always needed to inject changeset UUID. 'disable_invalid_removal' => $is_validation_debug_mode, + 'enable_response_caching' => ! defined( 'WP_DEBUG' ) || true !== WP_DEBUG || $is_validation_debug_mode, ), $args ); // Return cache if enabled and found. - if ( $is_cache_enabled ) { - // Set response cache hash. - $response_cache_key = md5( maybe_serialize( array( - 'sanitizer_classes' => self::$sanitizer_classes, - 'embed_handlers' => self::$embed_handlers, - 'args' => $args, - 'response' => $response, - 'plugin_version' => AMP__VERSION, - ) ) ); - - $response_cache = wp_cache_get( $response_cache_key, self::RESPONSE_CACHE_GROUP ); + if ( true === $args['enable_response_caching'] ) { + self::set_response_cache_key( array( + $args, + $response, + self::$sanitizer_classes, + self::$embed_handlers, + AMP__VERSION, + ) ); + $response_cache = wp_cache_get( self::$response_cache_key, self::RESPONSE_CACHE_GROUP ); if ( ! empty( $response_cache ) ) { return $response_cache; } } + AMP_Response_Headers::send_server_timing( 'amp_output_buffer', -self::$init_start_time, 'AMP Output Buffer' ); + $dom_parse_start = microtime( true ); /* @@ -1172,14 +1171,39 @@ public static function prepare_response( $response, $args = array() ) { AMP_Response_Headers::send_server_timing( 'amp_dom_serialize', -$dom_serialize_start, 'AMP DOM Serialize' ); - // Cache the response if enabled. - if ( $is_cache_enabled ) { - wp_cache_set( $response_cache_key, $response, self::RESPONSE_CACHE_GROUP, MONTH_IN_SECONDS ); + // Cache response if enabled. + if ( true === $args['enable_response_caching'] ) { + wp_cache_set( self::$response_cache_key, $response, self::RESPONSE_CACHE_GROUP, MONTH_IN_SECONDS ); } return $response; } + /** + * Set response cache hash, the data values dictates whether a new hash key should be generated or not. + * + * @since 1.0 + * + * @param array $data Data used to build hash key. A new hash key will be generated upon data value changes + * which will eventually trigger a new cached reponse. + */ + protected static function set_response_cache_key( $data ) { + // Loop through the data and make sure all functions and closure are called to prevent serializing issues. + $maybe_call_user_function = function ( $data ) use ( &$maybe_call_user_function ) { + if ( is_array( $data ) ) { + foreach ( $data as $index => $item ) { + $data[ $index ] = $maybe_call_user_function( $item ); + } + } elseif ( is_callable( $data ) ) { + return call_user_func( $data ); + } + + return $data; + }; + + self::$response_cache_key = md5( maybe_serialize( $maybe_call_user_function( $data ) ) ); + } + /** * Adds 'data-amp-layout' to the allowed attributes for wp_kses(). * From 65cfa08e66583e361254a50320abdcb1f051eb75 Mon Sep 17 00:00:00 2001 From: ThierryA Date: Tue, 22 May 2018 12:53:06 +0200 Subject: [PATCH 3/8] Issue 959: add tests for post processor response caching --- tests/test-class-amp-theme-support.php | 40 ++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 1260242420d..19303db25c0 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -960,6 +960,7 @@ public function test_filter_customize_partial_render() { * @global WP_Widget_Factory $wp_widget_factory * @global WP_Scripts $wp_scripts * @covers AMP_Theme_Support::prepare_response() + * @covers AMP_Theme_Support::set_response_cache_key() */ public function test_prepare_response() { global $wp_widget_factory, $wp_scripts, $wp_styles; @@ -1050,6 +1051,45 @@ public function test_prepare_response() { $this->assertInstanceOf( 'DOMElement', $removed_nodes['script'] ); $this->assertInstanceOf( 'DOMAttr', $removed_nodes['onclick'] ); $this->assertInstanceOf( 'DOMAttr', $removed_nodes['handle'] ); + + // Test that response cache is set correctly. + $this->assertEmpty( wp_cache_get( + AMP_Theme_Support::$response_cache_key, + AMP_Theme_Support::RESPONSE_CACHE_GROUP + ) ); + $response = AMP_Theme_Support::prepare_response( $original_html, array( + 'enable_response_caching' => true, + 'test_closure_argument' => function() {}, + ) ); + $this->assertEquals( + $response, + wp_cache_get( + AMP_Theme_Support::$response_cache_key, + AMP_Theme_Support::RESPONSE_CACHE_GROUP + ) + ); + $this->assertContains( 'amp_output_buffer', maybe_serialize( AMP_Response_Headers::$headers_sent ) ); + + // Test that response cache is return upon second call. + AMP_Response_Headers::$headers_sent = array(); + $cache_key = AMP_Theme_Support::$response_cache_key; + $cached_response = AMP_Theme_Support::prepare_response( $original_html, array( + 'enable_response_caching' => true, + 'test_closure_argument' => function() {}, + ) ); + $this->assertEquals( wp_cache_get( $cache_key, AMP_Theme_Support::RESPONSE_CACHE_GROUP ), $cached_response ); + $this->assertEquals( $cache_key, AMP_Theme_Support::$response_cache_key ); + $this->assertNotContains( 'amp_output_buffer', maybe_serialize( AMP_Response_Headers::$headers_sent ) ); + + // Test cache reset. + AMP_Response_Headers::$headers_sent = array(); + AMP_Theme_Support::prepare_response( $original_html, array( + 'enable_response_caching' => true, + 'test_reset_by_arg' => true, + 'test_closure_argument' => function() {}, + ) ); + $this->assertNotEquals( $cache_key, AMP_Theme_Support::$response_cache_key ); + $this->assertContains( 'amp_output_buffer', maybe_serialize( AMP_Response_Headers::$headers_sent ) ); } /** From b3ad9a34d88d165032573debb957543f6e88842b Mon Sep 17 00:00:00 2001 From: ThierryA Date: Tue, 22 May 2018 13:13:24 +0200 Subject: [PATCH 4/8] Issue 959: code formatting not picked up locally --- tests/test-class-amp-theme-support.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 19303db25c0..e30830a35b6 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -1072,8 +1072,8 @@ public function test_prepare_response() { // Test that response cache is return upon second call. AMP_Response_Headers::$headers_sent = array(); - $cache_key = AMP_Theme_Support::$response_cache_key; - $cached_response = AMP_Theme_Support::prepare_response( $original_html, array( + $cache_key = AMP_Theme_Support::$response_cache_key; + $cached_response = AMP_Theme_Support::prepare_response( $original_html, array( 'enable_response_caching' => true, 'test_closure_argument' => function() {}, ) ); From e80ac1600e1764c3c6705e5a29ad338b9dc9c6c1 Mon Sep 17 00:00:00 2001 From: ThierryA Date: Tue, 22 May 2018 13:56:36 +0200 Subject: [PATCH 5/8] Issue 959: add AMP-Response-Cache header + query var to flush cache --- includes/class-amp-theme-support.php | 20 ++++++-- tests/test-class-amp-theme-support.php | 63 ++++++++++++++++++++------ 2 files changed, 66 insertions(+), 17 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index a1f44771252..3ae3bdf9527 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -26,6 +26,13 @@ class AMP_Theme_Support { */ const RESPONSE_CACHE_GROUP = 'amp-reponse'; + /** + * Query var that triggers response cache removal for the given page. + * + * @var string + */ + const FLUSH_RESPONSE_CACHE_VAR = 'amp_flush_response_cache'; + /** * Response cache hash key. * @@ -1062,13 +1069,20 @@ public static function prepare_response( $response, $args = array() ) { self::$embed_handlers, AMP__VERSION, ) ); - $response_cache = wp_cache_get( self::$response_cache_key, self::RESPONSE_CACHE_GROUP ); - if ( ! empty( $response_cache ) ) { - return $response_cache; + if ( isset( $_REQUEST[ self::FLUSH_RESPONSE_CACHE_VAR ] ) ) { // WPCS: csrf ok. + wp_cache_delete( self::$response_cache_key, self::RESPONSE_CACHE_GROUP ); + } else { + $response_cache = wp_cache_get( self::$response_cache_key, self::RESPONSE_CACHE_GROUP ); + + if ( ! empty( $response_cache ) ) { + AMP_Response_Headers::send_header( 'AMP-Response-Cache', true ); + return $response_cache; + } } } + AMP_Response_Headers::send_header( 'AMP-Response-Cache', false ); AMP_Response_Headers::send_server_timing( 'amp_output_buffer', -self::$init_start_time, 'AMP Output Buffer' ); $dom_parse_start = microtime( true ); diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index e30830a35b6..1b95a491aad 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -1053,35 +1053,49 @@ public function test_prepare_response() { $this->assertInstanceOf( 'DOMAttr', $removed_nodes['handle'] ); // Test that response cache is set correctly. + $call_response = function() use ( $original_html ) { + return AMP_Theme_Support::prepare_response( $original_html, array( + 'enable_response_caching' => true, + 'test_closure_argument' => function() {}, + ) ); + }; $this->assertEmpty( wp_cache_get( AMP_Theme_Support::$response_cache_key, AMP_Theme_Support::RESPONSE_CACHE_GROUP ) ); - $response = AMP_Theme_Support::prepare_response( $original_html, array( - 'enable_response_caching' => true, - 'test_closure_argument' => function() {}, - ) ); $this->assertEquals( - $response, + $call_response(), wp_cache_get( AMP_Theme_Support::$response_cache_key, AMP_Theme_Support::RESPONSE_CACHE_GROUP ) ); - $this->assertContains( 'amp_output_buffer', maybe_serialize( AMP_Response_Headers::$headers_sent ) ); + $this->assertContains( + array( + 'name' => 'AMP-Response-Cache', + 'value' => false, + 'replace' => true, + 'status_code' => null, + ), + AMP_Response_Headers::$headers_sent + ); // Test that response cache is return upon second call. AMP_Response_Headers::$headers_sent = array(); $cache_key = AMP_Theme_Support::$response_cache_key; - $cached_response = AMP_Theme_Support::prepare_response( $original_html, array( - 'enable_response_caching' => true, - 'test_closure_argument' => function() {}, - ) ); - $this->assertEquals( wp_cache_get( $cache_key, AMP_Theme_Support::RESPONSE_CACHE_GROUP ), $cached_response ); + $this->assertEquals( wp_cache_get( $cache_key, AMP_Theme_Support::RESPONSE_CACHE_GROUP ), $call_response() ); $this->assertEquals( $cache_key, AMP_Theme_Support::$response_cache_key ); - $this->assertNotContains( 'amp_output_buffer', maybe_serialize( AMP_Response_Headers::$headers_sent ) ); + $this->assertContains( + array( + 'name' => 'AMP-Response-Cache', + 'value' => true, + 'replace' => true, + 'status_code' => null, + ), + AMP_Response_Headers::$headers_sent + ); - // Test cache reset. + // Test new cache upon argument change. AMP_Response_Headers::$headers_sent = array(); AMP_Theme_Support::prepare_response( $original_html, array( 'enable_response_caching' => true, @@ -1089,7 +1103,28 @@ public function test_prepare_response() { 'test_closure_argument' => function() {}, ) ); $this->assertNotEquals( $cache_key, AMP_Theme_Support::$response_cache_key ); - $this->assertContains( 'amp_output_buffer', maybe_serialize( AMP_Response_Headers::$headers_sent ) ); + $this->assertContains( + array( + 'name' => 'AMP-Response-Cache', + 'value' => false, + 'replace' => true, + 'status_code' => null, + ), + AMP_Response_Headers::$headers_sent + ); + + // Test cache flush. + $_REQUEST[ AMP_Theme_Support::FLUSH_RESPONSE_CACHE_VAR ] = true; + $call_response(); + $this->assertContains( + array( + 'name' => 'AMP-Response-Cache', + 'value' => false, + 'replace' => true, + 'status_code' => null, + ), + AMP_Response_Headers::$headers_sent + ); } /** From 9bcc4acfb28d87ba04b5f23c3793cce83b5deaa3 Mon Sep 17 00:00:00 2001 From: ThierryA Date: Wed, 23 May 2018 23:41:26 +0200 Subject: [PATCH 6/8] Issue 959: code improvements (props Weston Ruter) --- includes/class-amp-theme-support.php | 64 ++++++-------------------- tests/test-class-amp-theme-support.php | 36 ++------------- 2 files changed, 17 insertions(+), 83 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 3ae3bdf9527..8a53dae5fc0 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -26,20 +26,6 @@ class AMP_Theme_Support { */ const RESPONSE_CACHE_GROUP = 'amp-reponse'; - /** - * Query var that triggers response cache removal for the given page. - * - * @var string - */ - const FLUSH_RESPONSE_CACHE_VAR = 'amp_flush_response_cache'; - - /** - * Response cache hash key. - * - * @var string - */ - public static $response_cache_key; - /** * Sanitizer classes. * @@ -1055,30 +1041,31 @@ public static function prepare_response( $response, $args = array() ) { 'allow_dirty_styles' => self::is_customize_preview_iframe(), // Dirty styles only needed when editing (e.g. for edit shortcodes). 'allow_dirty_scripts' => is_customize_preview(), // Scripts are always needed to inject changeset UUID. 'disable_invalid_removal' => $is_validation_debug_mode, - 'enable_response_caching' => ! defined( 'WP_DEBUG' ) || true !== WP_DEBUG || $is_validation_debug_mode, + 'enable_response_caching' => ( + ( ! defined( 'WP_DEBUG' ) || true !== WP_DEBUG ) + && + ! AMP_Validation_Utils::should_validate_response() + ), ), $args ); // Return cache if enabled and found. if ( true === $args['enable_response_caching'] ) { - self::set_response_cache_key( array( + // 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, - ) ); + ) ) ); - if ( isset( $_REQUEST[ self::FLUSH_RESPONSE_CACHE_VAR ] ) ) { // WPCS: csrf ok. - wp_cache_delete( self::$response_cache_key, self::RESPONSE_CACHE_GROUP ); - } else { - $response_cache = wp_cache_get( self::$response_cache_key, self::RESPONSE_CACHE_GROUP ); + $response_cache = wp_cache_get( $response_cache_key, self::RESPONSE_CACHE_GROUP ); - if ( ! empty( $response_cache ) ) { - AMP_Response_Headers::send_header( 'AMP-Response-Cache', true ); - return $response_cache; - } + if ( ! empty( $response_cache ) ) { + AMP_Response_Headers::send_header( 'AMP-Response-Cache', true ); + return $response_cache; } } @@ -1187,37 +1174,12 @@ public static function prepare_response( $response, $args = array() ) { // Cache response if enabled. if ( true === $args['enable_response_caching'] ) { - wp_cache_set( self::$response_cache_key, $response, self::RESPONSE_CACHE_GROUP, MONTH_IN_SECONDS ); + wp_cache_set( $response_cache_key, $response, self::RESPONSE_CACHE_GROUP, MONTH_IN_SECONDS ); } return $response; } - /** - * Set response cache hash, the data values dictates whether a new hash key should be generated or not. - * - * @since 1.0 - * - * @param array $data Data used to build hash key. A new hash key will be generated upon data value changes - * which will eventually trigger a new cached reponse. - */ - protected static function set_response_cache_key( $data ) { - // Loop through the data and make sure all functions and closure are called to prevent serializing issues. - $maybe_call_user_function = function ( $data ) use ( &$maybe_call_user_function ) { - if ( is_array( $data ) ) { - foreach ( $data as $index => $item ) { - $data[ $index ] = $maybe_call_user_function( $item ); - } - } elseif ( is_callable( $data ) ) { - return call_user_func( $data ); - } - - return $data; - }; - - self::$response_cache_key = md5( maybe_serialize( $maybe_call_user_function( $data ) ) ); - } - /** * Adds 'data-amp-layout' to the allowed attributes for wp_kses(). * diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 1b95a491aad..4cb7eac17d2 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -960,7 +960,6 @@ public function test_filter_customize_partial_render() { * @global WP_Widget_Factory $wp_widget_factory * @global WP_Scripts $wp_scripts * @covers AMP_Theme_Support::prepare_response() - * @covers AMP_Theme_Support::set_response_cache_key() */ public function test_prepare_response() { global $wp_widget_factory, $wp_scripts, $wp_styles; @@ -1052,24 +1051,14 @@ public function test_prepare_response() { $this->assertInstanceOf( 'DOMAttr', $removed_nodes['onclick'] ); $this->assertInstanceOf( 'DOMAttr', $removed_nodes['handle'] ); - // Test that response cache is set correctly. $call_response = function() use ( $original_html ) { return AMP_Theme_Support::prepare_response( $original_html, array( 'enable_response_caching' => true, - 'test_closure_argument' => function() {}, ) ); }; - $this->assertEmpty( wp_cache_get( - AMP_Theme_Support::$response_cache_key, - AMP_Theme_Support::RESPONSE_CACHE_GROUP - ) ); - $this->assertEquals( - $call_response(), - wp_cache_get( - AMP_Theme_Support::$response_cache_key, - AMP_Theme_Support::RESPONSE_CACHE_GROUP - ) - ); + + // Test that first response isn't cached. + $first_response = $call_response(); $this->assertContains( array( 'name' => 'AMP-Response-Cache', @@ -1082,9 +1071,7 @@ public function test_prepare_response() { // Test that response cache is return upon second call. AMP_Response_Headers::$headers_sent = array(); - $cache_key = AMP_Theme_Support::$response_cache_key; - $this->assertEquals( wp_cache_get( $cache_key, AMP_Theme_Support::RESPONSE_CACHE_GROUP ), $call_response() ); - $this->assertEquals( $cache_key, AMP_Theme_Support::$response_cache_key ); + $this->assertEquals( $first_response, $call_response() ); $this->assertContains( array( 'name' => 'AMP-Response-Cache', @@ -1100,22 +1087,7 @@ public function test_prepare_response() { AMP_Theme_Support::prepare_response( $original_html, array( 'enable_response_caching' => true, 'test_reset_by_arg' => true, - 'test_closure_argument' => function() {}, ) ); - $this->assertNotEquals( $cache_key, AMP_Theme_Support::$response_cache_key ); - $this->assertContains( - array( - 'name' => 'AMP-Response-Cache', - 'value' => false, - 'replace' => true, - 'status_code' => null, - ), - AMP_Response_Headers::$headers_sent - ); - - // Test cache flush. - $_REQUEST[ AMP_Theme_Support::FLUSH_RESPONSE_CACHE_VAR ] = true; - $call_response(); $this->assertContains( array( 'name' => 'AMP-Response-Cache', From 0b34b1bb277d8bd90dd036e1d3e3a3539786cf62 Mon Sep 17 00:00:00 2001 From: ThierryA Date: Thu, 24 May 2018 00:21:35 +0200 Subject: [PATCH 7/8] Travis re-trigger (Failure even after Travis Restart job) From 682808a9d413c78c6b420306505a26f78c1cb94a Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 23 May 2018 15:52:19 -0700 Subject: [PATCH 8/8] Remove non-standard AMP-Response-Cache header --- includes/class-amp-theme-support.php | 2 -- tests/test-class-amp-theme-support.php | 45 +++++++++++--------------- 2 files changed, 18 insertions(+), 29 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 8a53dae5fc0..6989d7246d2 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1064,12 +1064,10 @@ public static function prepare_response( $response, $args = array() ) { $response_cache = wp_cache_get( $response_cache_key, self::RESPONSE_CACHE_GROUP ); if ( ! empty( $response_cache ) ) { - AMP_Response_Headers::send_header( 'AMP-Response-Cache', true ); return $response_cache; } } - AMP_Response_Headers::send_header( 'AMP-Response-Cache', false ); AMP_Response_Headers::send_server_timing( 'amp_output_buffer', -self::$init_start_time, 'AMP Output Buffer' ); $dom_parse_start = microtime( true ); diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 921420772a8..04278d0f8ce 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -1059,28 +1059,22 @@ public function test_prepare_response() { // Test that first response isn't cached. $first_response = $call_response(); - $this->assertContains( - array( - 'name' => 'AMP-Response-Cache', - 'value' => false, - 'replace' => true, - 'status_code' => null, - ), - AMP_Response_Headers::$headers_sent - ); + $this->assertGreaterThan( 0, count( array_filter( + AMP_Response_Headers::$headers_sent, + function( $header ) { + return 'Server-Timing' === $header['name']; + } + ) ) ); // Test that response cache is return upon second call. AMP_Response_Headers::$headers_sent = array(); $this->assertEquals( $first_response, $call_response() ); - $this->assertContains( - array( - 'name' => 'AMP-Response-Cache', - 'value' => true, - 'replace' => true, - 'status_code' => null, - ), - AMP_Response_Headers::$headers_sent - ); + $this->assertSame( 0, count( array_filter( + AMP_Response_Headers::$headers_sent, + function( $header ) { + return 'Server-Timing' === $header['name']; + } + ) ) ); // Test new cache upon argument change. AMP_Response_Headers::$headers_sent = array(); @@ -1088,15 +1082,12 @@ public function test_prepare_response() { 'enable_response_caching' => true, 'test_reset_by_arg' => true, ) ); - $this->assertContains( - array( - 'name' => 'AMP-Response-Cache', - 'value' => false, - 'replace' => true, - 'status_code' => null, - ), - AMP_Response_Headers::$headers_sent - ); + $this->assertGreaterThan( 0, count( array_filter( + AMP_Response_Headers::$headers_sent, + function( $header ) { + return 'Server-Timing' === $header['name']; + } + ) ) ); } /**