Merge pull request #1794 from ampproject/fix/is-endpoint-timing-check

Add _doing_it_wrong() when calling is_amp_endpoint() before queried object is available
ampproject · Jan 4, 2019 · 4c7c3c8 · 4c7c3c8
2 parents c96e3cd + 744202d
commit 4c7c3c8
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 6 deletions.
diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php
@@ -283,8 +283,15 @@ function is_amp_endpoint() {
 		return true;
 	}
 
-	$availability = AMP_Theme_Support::get_template_availability();
-	return amp_is_canonical() ? $availability['supported'] : ( $has_amp_query_var && $availability['supported'] );
+	if ( ! did_action( 'wp' ) ) {
+		_doing_it_wrong( __FUNCTION__, sprintf( esc_html__( "is_amp_endpoint() was called before the 'wp' action which means it will not have access to the queried object to determine if it is an AMP response, thus neither the amp_skip_post filter nor the AMP enabled publish metabox toggle will not be considered.", 'amp' ) ), '1.0.2' );
+		$supported = true;
+	} else {
+		$availability = AMP_Theme_Support::get_template_availability();
+		$supported    = $availability['supported'];
+	}
+
+	return amp_is_canonical() ? $supported : ( $has_amp_query_var && $supported );
 }
 
 /**

diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -586,10 +586,12 @@ public static function get_template_availability( $query = null ) {
 			 * @var WP_Post $queried_object
 			 */
 			$queried_object = $query->get_queried_object();
-			$support_errors = AMP_Post_Type_Support::get_support_errors( $queried_object );
-			if ( ! empty( $support_errors ) ) {
-				$matching_template['errors']    = array_merge( $matching_template['errors'], $support_errors );
-				$matching_template['supported'] = false;
+			if ( $queried_object instanceof WP_Post ) {
+				$support_errors = AMP_Post_Type_Support::get_support_errors( $queried_object );
+				if ( ! empty( $support_errors ) ) {
+					$matching_template['errors']    = array_merge( $matching_template['errors'], $support_errors );
+					$matching_template['supported'] = false;
+				}
 			}
 		}
 

diff --git a/tests/test-amp-style-sanitizer.php b/tests/test-amp-style-sanitizer.php
@@ -1388,6 +1388,7 @@ public function test_css_import() {
 	 * @covers \AMP_DOM_Utils::get_content_from_dom_node()
 	 */
 	public function test_unicode_stylesheet() {
+		wp();
 		add_theme_support( AMP_Theme_Support::SLUG );
 		AMP_Theme_Support::init();
 		AMP_Theme_Support::finish_init();

diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php
@@ -383,6 +383,7 @@ public function test_add_amp_template_filters() {
 	 * @covers AMP_Theme_Support::prepare_response()
 	 */
 	public function test_validate_non_amp_theme() {
+		wp();
 		add_filter( 'amp_validation_error_sanitized', '__return_true' );
 		add_theme_support( AMP_Theme_Support::SLUG );
 		AMP_Theme_Support::init();
@@ -1052,6 +1053,7 @@ public function test_dequeue_customize_preview_scripts() {
 	 * @covers AMP_Theme_Support::finish_output_buffering()
 	 */
 	public function test_start_output_buffering() {
+		wp();
 		if ( ! function_exists( 'newrelic_disable_autorum ' ) ) {
 			/**
 			 * Define newrelic_disable_autorum to allow passing line.
@@ -1079,6 +1081,7 @@ function newrelic_disable_autorum() {
 	 * @covers AMP_Theme_Support::is_output_buffering()
 	 */
 	public function test_finish_output_buffering() {
+		wp();
 		add_filter( 'amp_validation_error_sanitized', '__return_true' );
 		add_theme_support( AMP_Theme_Support::SLUG );
 		AMP_Theme_Support::init();
@@ -1127,6 +1130,7 @@ public function test_finish_output_buffering() {
 	 * @covers AMP_Theme_Support::filter_customize_partial_render()
 	 */
 	public function test_filter_customize_partial_render() {
+		wp();
 		add_filter( 'amp_validation_error_sanitized', '__return_true' );
 		add_theme_support( AMP_Theme_Support::SLUG );
 		AMP_Theme_Support::init();
@@ -1175,6 +1179,7 @@ protected function get_etag_header_value( $headers ) {
 	 * @covers \amp_render_scripts()
 	 */
 	public function test_prepare_response() {
+		wp();
 		$prepare_response_args = array(
 			'enable_response_caching' => false,
 		);
@@ -1337,6 +1342,7 @@ public function test_prepare_response() {
 	 * @covers AMP_Theme_Support::prepare_response()
 	 */
 	public function test_prepare_response_native_mode_non_amp() {
+		wp();
 		$original_html = $this->get_original_html();
 		add_filter( 'amp_validation_error_sanitized', '__return_false' ); // For testing purpose only. This should not normally be done.
 
@@ -1351,6 +1357,7 @@ public function test_prepare_response_native_mode_non_amp() {
 	 * Test post-processor cache effectiveness in AMP_Theme_Support::prepare_response().
 	 */
 	public function test_post_processor_cache_effectiveness() {
+		wp();
 		$original_html = $this->get_original_html();
 		$args          = array( 'enable_response_caching' => true );
 		wp_using_ext_object_cache( true ); // turn on external object cache flag.
@@ -1508,6 +1515,7 @@ private function reset_post_processor_cache_effectiveness() {
 	 * @covers AMP_Theme_Support::prepare_response()
 	 */
 	public function test_prepare_response_bad_html() {
+		wp();
 		add_filter( 'amp_validation_error_sanitized', '__return_true' );
 		add_theme_support( AMP_Theme_Support::SLUG );
 		AMP_Theme_Support::init();
@@ -1533,6 +1541,7 @@ public function test_prepare_response_bad_html() {
 	 * @covers AMP_Theme_Support::prepare_response()
 	 */
 	public function test_prepare_response_to_add_html5_doctype_and_amp_attribute() {
+		wp();
 		add_filter( 'amp_validation_error_sanitized', '__return_true' );
 		add_theme_support( AMP_Theme_Support::SLUG );
 		AMP_Theme_Support::init();

diff --git a/tests/test-class-amp-widget-archives.php b/tests/test-class-amp-widget-archives.php
@@ -62,6 +62,7 @@ public function test_construct() {
 	 * @see AMP_Widget_Archives::widget().
 	 */
 	public function test_widget() {
+		wp();
 		$this->assertTrue( is_amp_endpoint() );
 		$arguments = array(
 			'before_widget' => '<div>',

diff --git a/tests/test-class-amp-widget-categories.php b/tests/test-class-amp-widget-categories.php
@@ -62,6 +62,7 @@ public function test_construct() {
 	 * @covers AMP_Widget_Categories::widget()
 	 */
 	public function test_widget() {
+		wp();
 		$this->assertTrue( is_amp_endpoint() );
 		$arguments = array(
 			'before_widget' => '<div>',

diff --git a/tests/test-class-amp-widget-text.php b/tests/test-class-amp-widget-text.php
@@ -53,6 +53,7 @@ public function tearDown() {
 	 * @covers AMP_Widget_Text::inject_video_max_width_style()
 	 */
 	public function test_inject_video_max_width_style() {
+		wp();
 		$this->assertTrue( is_amp_endpoint() );
 		$video            = '<video src="http://example.com" height="100" width="200"></video>';
 		$video_only_width = '<video src="http://example.com/this-video" width="500">';