Merge pull request #6233 from ampproject/fix/schemaorg-metadata-filtering-during-ob-callback

westonruter · web-flow · commit 833ba2bc234e · 2021-05-14T08:19:07.000-07:00
Call `amp_get_schemaorg_metadata()` before output buffer processing
diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -19,6 +19,8 @@
 use AmpProject\Optimizer;
 use AmpProject\RequestDestination;
 use AmpProject\Tag;
+use AmpProject\AmpWP\Optimizer\Transformer\AmpSchemaOrgMetadata;
+use AmpProject\AmpWP\Optimizer\Transformer\AmpSchemaOrgMetadataConfiguration;
 
 /**
  * Class AMP_Theme_Support
@@ -124,6 +126,13 @@ class AMP_Theme_Support {
 	 */
 	protected static $is_output_buffering = false;
 
+	/**
+	 * Schema.org metadata
+	 *
+	 * @var array
+	 */
+	protected static $metadata;
+
 	/**
 	 * Initialize.
 	 *
@@ -1622,6 +1631,9 @@ public static function start_output_buffering() {
 			newrelic_disable_autorum();
 		}
 
+		// Obtain Schema.org metadata so that it will be available.
+		self::$metadata = (array) amp_get_schemaorg_metadata();
+
 		ob_start( [ __CLASS__, 'finish_output_buffering' ] );
 		self::$is_output_buffering = true;
 	}
@@ -2090,6 +2102,18 @@ static function () use ( $args ) {
 			defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX // phpcs:ignore PHPCompatibility.Constants.NewConstants.php_int_minFound
 		);
 
+		// Supply the Schema.org metadata, previously obtained just before output buffering began, to the AmpSchemaOrgMetadataConfiguration.
+		add_filter(
+			'amp_optimizer_config',
+			function ( $config ) {
+				if ( is_array( self::$metadata ) ) {
+					$config[ AmpSchemaOrgMetadata::class ][ AmpSchemaOrgMetadataConfiguration::METADATA ] = self::$metadata;
+				}
+				return $config;
+			},
+			defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX // phpcs:ignore PHPCompatibility.Constants.NewConstants.php_int_minFound
+		);
+
 		return Services::get( 'injector' )->make( OptimizerService::class );
 	}
 
diff --git a/src/Optimizer/Transformer/AmpSchemaOrgMetadataConfiguration.php b/src/Optimizer/Transformer/AmpSchemaOrgMetadataConfiguration.php
@@ -46,7 +46,7 @@ protected function getAllowedKeys() {
 		}
 
 		return [
-			self::METADATA => (array) amp_get_schemaorg_metadata(),
+			self::METADATA => [],
 		];
 	}
 
diff --git a/tests/php/test-class-amp-schema-org-metadata.php b/tests/php/test-class-amp-schema-org-metadata.php
@@ -55,10 +55,21 @@ public function get_schema_script_data() {
 	 * @param int    $expected Expected count of valid JSON+LD schema.
 	 */
 	public function test_transform( $json, $expected ) {
-		$html        = '<html><head><script type="application/ld+json">%s</script></head><body>Test</body></html>';
-		$dom         = Document::fromHtml( sprintf( $html, $json ), Options::DEFAULTS );
-		$transformer = new AmpSchemaOrgMetadata( new AmpSchemaOrgMetadataConfiguration() );
-		$errors      = new ErrorCollection();
+		$html          = '<html><head><script type="application/ld+json">%s</script></head><body>Test</body></html>';
+		$dom           = Document::fromHtml( sprintf( $html, $json ), Options::DEFAULTS );
+		$configuration = new AmpSchemaOrgMetadataConfiguration(
+			[
+				AmpSchemaOrgMetadataConfiguration::METADATA => [
+					'@context'  => 'http://schema.org',
+					'publisher' => [
+						'@type' => 'Organization',
+						'name'  => 'Acme',
+					],
+				],
+			]
+		);
+		$transformer   = new AmpSchemaOrgMetadata( $configuration );
+		$errors        = new ErrorCollection();
 		$transformer->transform( $dom, $errors );
 		$this->assertEquals( $expected, substr_count( $dom->saveHTML(), 'schema.org' ) );
 	}
@@ -69,15 +80,11 @@ public function test_transform( $json, $expected ) {
 	 * @covers ::transform
 	 */
 	public function test_empty_metadata_configuration() {
-		add_filter( 'amp_schemaorg_metadata', '__return_empty_array' );
-
 		$dom         = new Document();
 		$transformer = new AmpSchemaOrgMetadata( new AmpSchemaOrgMetadataConfiguration() );
 		$transformer->transform( $dom, new ErrorCollection() );
 
 		$xpath_query = '//script[ @type = "application/ld+json" ]';
 		$this->assertEquals( 0, $dom->xpath->query( $xpath_query )->length );
-
-		remove_filter( 'amp_schemaorg_metadata', '__return_empty_array' );
 	}
 }
diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php
@@ -1982,7 +1982,7 @@ public function test_prepare_response_throwing_exception() {
 		);
 
 		add_filter(
-			'amp_schemaorg_metadata',
+			'amp_enable_optimizer',
 			static function () {
 				throw new RuntimeException( 'FAILURE', 42 );
 			}

Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ protected function getAllowedKeys() {`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`return [`
`49`		`- self::METADATA => (array) amp_get_schemaorg_metadata(),`
	`49`	`+ self::METADATA => [],`
`50`	`50`	`];`
`51`	`51`	`}`
`52`	`52`
Original file line number	Diff line number	Diff line change
`@@ -1982,7 +1982,7 @@ public function test_prepare_response_throwing_exception() {`
`1982`	`1982`	`);`
`1983`	`1983`
`1984`	`1984`	`add_filter(`
`1985`		`- 'amp_schemaorg_metadata',`
	`1985`	`+ 'amp_enable_optimizer',`
`1986`	`1986`	`static function () {`
`1987`	`1987`	`throw new RuntimeException( 'FAILURE', 42 );`
`1988`	`1988`	`}`