diff --git a/src/schema-aggregator/application/schema_map/schema-map-xml-provider.php b/src/schema-aggregator/application/schema_map/schema-map-xml-provider.php new file mode 100644 index 00000000000..bf9c1574375 --- /dev/null +++ b/src/schema-aggregator/application/schema_map/schema-map-xml-provider.php @@ -0,0 +1,73 @@ +aggregate_site_schema_map_command_handler = $aggregate_site_schema_map_command_handler; + $this->xml_cache_manager = $xml_cache_manager; + $this->aggregator_config = $aggregator_config; + } + + /** + * Returns the schema map XML, building and caching it when it is not cached yet. + * + * @return string The schema map XML. + */ + public function get_xml(): string { + $cached_xml = $this->xml_cache_manager->get(); + if ( $cached_xml !== null ) { + return $cached_xml; + } + + $command = new Aggregate_Site_Schema_Map_Command( $this->aggregator_config->get_allowed_post_types() ); + $xml = $this->aggregate_site_schema_map_command_handler->handle( $command ); + + $this->xml_cache_manager->set( $xml ); + + return $xml; + } +} diff --git a/src/schema-aggregator/user-interface/schemamap-xml-rewrite-integration.php b/src/schema-aggregator/user-interface/schemamap-xml-rewrite-integration.php new file mode 100644 index 00000000000..c0a5974d2e7 --- /dev/null +++ b/src/schema-aggregator/user-interface/schemamap-xml-rewrite-integration.php @@ -0,0 +1,195 @@ + The conditionals that must be met to load this. + */ + public static function get_conditionals() { + return [ Schema_Aggregator_Conditional::class, Front_End_Conditional::class ]; + } + + /** + * Schemamap_Xml_Rewrite_Integration constructor. + * + * @param Schema_Map_Xml_Provider $schema_map_xml_provider The schema map XML provider. + * @param Redirect_Helper $redirect_helper The redirect helper. + */ + public function __construct( Schema_Map_Xml_Provider $schema_map_xml_provider, Redirect_Helper $redirect_helper ) { + $this->schema_map_xml_provider = $schema_map_xml_provider; + $this->redirect_helper = $redirect_helper; + } + + /** + * Registers the hooks for the integration. + * + * The schema map is rendered on `pre_get_posts` rather than `template_redirect`, the same way + * `WPSEO_Sitemaps::redirect()` renders sitemaps. That skips the main posts query, which this + * response never uses, and leaves fewer third-party callbacks able to echo ahead of the XML — + * anything printed before the document would stop it parsing as a sitemap. + * + * @return void + */ + public function register_hooks() { + /* + * Integrations register their hooks on `init` priority 10, which is after + * `yoast_add_dynamic_rewrite_rules` has already fired on `init` priority 1. The rule is + * therefore added directly, the same way `WPSEO_Sitemaps::register_sitemap()` does for + * sitemaps registered during `init`. Rewrite rules are not read until `WP::parse_request()`, + * so this is still in time. + */ + $this->add_rewrite_rules( Yoast_Dynamic_Rewrites::instance() ); + + \add_filter( 'query_vars', [ $this, 'add_query_vars' ] ); + \add_action( 'pre_get_posts', [ $this, 'maybe_render_schema_map' ], 1 ); + } + + /** + * Adds the rewrite rule for the schema map. + * + * @param Yoast_Dynamic_Rewrites $dynamic_rewrites Dynamic rewrites handler instance. + * + * @return void + */ + public function add_rewrite_rules( Yoast_Dynamic_Rewrites $dynamic_rewrites ) { + $dynamic_rewrites->add_rule( 'schemamap\.xml$', 'index.php?' . self::QUERY_VAR . '=1', 'top' ); + } + + /** + * Adds the query variable for the schema map. + * + * @param array $query_vars List of query variables to filter. + * + * @return array Filtered query variables. + */ + public function add_query_vars( $query_vars ) { + $query_vars[] = self::QUERY_VAR; + + return $query_vars; + } + + /** + * Outputs the schema map when the main query is a request for it. + * + * @param WP_Query $query The query that is about to run. + * + * @return void + */ + public function maybe_render_schema_map( WP_Query $query ) { + if ( ! $query->is_main_query() || ! $query->get( self::QUERY_VAR ) ) { + return; + } + + /* + * Without control over the headers the response cannot be typed as XML, and echoing the + * document into a half-written HTML body would produce neither a page nor a schema map. + */ + if ( $this->headers_already_sent() ) { + return; + } + + /* + * Build the document before committing any headers. Generating it hits the database and can + * throw, and a failure after the 200 and the cache headers were sent would let a proxy store + * an HTML error page at this path for the full max-age. + */ + $xml = $this->schema_map_xml_provider->get_xml(); + + $this->send_headers(); + + // DOMDocument::createTextNode() already escaped every value; escaping again would corrupt the XML. + echo $xml; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Raw XML. + + $this->finish_request(); + } + + /** + * Sends the response headers for the schema map. + * + * These replace what `WP::send_headers()` already sent: the HTML content type, and the `Expires` + * and `Cache-Control` pair that `wp_get_nocache_headers()` adds for logged-in users. + * + * @return void + */ + protected function send_headers() { + \status_header( 200 ); + // Prevent the search engines from indexing the schema map. + $this->redirect_helper->set_header( 'X-Robots-Tag: noindex, follow' ); + $this->redirect_helper->set_header( 'Content-Type: application/xml; charset=UTF-8' ); + $this->redirect_helper->set_header( 'Cache-Control: public, max-age=300' ); + // The response is public, so the no-cache Expires date from wp_get_nocache_headers() must go. + $this->redirect_helper->remove_header( 'Expires' ); + } + + /** + * Tells whether the response headers have already been sent. + * + * Wrapped in an overridable method so the unit tests can drive both branches of + * maybe_render_schema_map(); the test process cannot control what headers_sent() reports. + * Protected rather than private because Mockery cannot stub private methods. + * + * @codeCoverageIgnore It only wraps a PHP function. + * + * @return bool Whether the headers have already been sent. + */ + protected function headers_already_sent(): bool { + return \headers_sent(); + } + + /** + * Ends the request after the schema map has been output. + * + * The exit() is wrapped in an overridable method so the unit tests can stub it out; executing it + * would terminate the test runner. Protected rather than private because Mockery cannot stub + * private methods. + * + * @codeCoverageIgnore It only wraps a language construct that terminates the request. + * + * @return void + */ + protected function finish_request() { + exit(); + } +} diff --git a/src/schema-aggregator/user-interface/site-schema-aggregator-xml-route.php b/src/schema-aggregator/user-interface/site-schema-aggregator-xml-route.php index a6e30740a22..92e89ef103b 100644 --- a/src/schema-aggregator/user-interface/site-schema-aggregator-xml-route.php +++ b/src/schema-aggregator/user-interface/site-schema-aggregator-xml-route.php @@ -2,15 +2,10 @@ // phpcs:disable Yoast.NamingConventions.NamespaceName.TooLong -- Needed in the folder structure. namespace Yoast\WP\SEO\Schema_Aggregator\User_Interface; -use WP_Error; use WP_REST_Response; use Yoast\WP\SEO\Main; use Yoast\WP\SEO\Routes\Route_Interface; -use Yoast\WP\SEO\Schema_Aggregator\Application\Aggregate_Site_Schema_Map_Command; -use Yoast\WP\SEO\Schema_Aggregator\Application\Aggregate_Site_Schema_Map_Command_Handler; -use Yoast\WP\SEO\Schema_Aggregator\Application\Cache\Xml_Manager; -use Yoast\WP\SEO\Schema_Aggregator\Infrastructure\Aggregator_Config; -use Yoast\WP\SEO\Schema_Aggregator\Infrastructure\Config; +use Yoast\WP\SEO\Schema_Aggregator\Application\Schema_Map\Schema_Map_Xml_Provider; use Yoast\WP\SEO\Schema_Aggregator\Infrastructure\Schema_Aggregator_Conditional; /** @@ -34,25 +29,11 @@ class Site_Schema_Aggregator_Xml_Route implements Route_Interface { public const GET_SCHEMA_ROUTE = self::ROUTE_PREFIX . '/get-xml'; /** - * The command handler instance. + * The schema map XML provider. * - * @var Aggregate_Site_Schema_Map_Command_Handler + * @var Schema_Map_Xml_Provider */ - private $aggregate_site_schema_map_command_handler; - - /** - * The XML cache manager instance. - * - * @var Xml_Manager - */ - private $xml_cache_manager; - - /** - * The aggregator configuration instance. - * - * @var Aggregator_Config - */ - private $aggregator_config; + private $schema_map_xml_provider; /** * Returns the conditional for this route. @@ -66,20 +47,10 @@ public static function get_conditionals() { /** * Site_Schema_Aggregator_Route constructor. * - * @param Aggregate_Site_Schema_Map_Command_Handler $aggregate_site_schema_map_command_handler The command handler. - * @param Xml_Manager $xml_cache_manager The XML cache - * manager. - * @param Aggregator_Config $aggregator_config The aggregator - * configuration. + * @param Schema_Map_Xml_Provider $schema_map_xml_provider The schema map XML provider. */ - public function __construct( - Aggregate_Site_Schema_Map_Command_Handler $aggregate_site_schema_map_command_handler, - Xml_Manager $xml_cache_manager, - Aggregator_Config $aggregator_config - ) { - $this->aggregate_site_schema_map_command_handler = $aggregate_site_schema_map_command_handler; - $this->xml_cache_manager = $xml_cache_manager; - $this->aggregator_config = $aggregator_config; + public function __construct( Schema_Map_Xml_Provider $schema_map_xml_provider ) { + $this->schema_map_xml_provider = $schema_map_xml_provider; } /** @@ -109,25 +80,12 @@ public function get_permission_callback(): bool { } /** - * Returns a XML representation of the possible post types that can be used for schema. + * Returns a XML representation of the post types that are used for schema. * - * @return WP_REST_Response|WP_Error The success or failure response. + * @return WP_REST_Response The response. */ public function render_schema_xml() { - $cached_xml = $this->xml_cache_manager->get(); - if ( $cached_xml !== null ) { - $xml = $cached_xml; - } - else { - - $post_types = $this->aggregator_config->get_allowed_post_types(); - - $command = new Aggregate_Site_Schema_Map_Command( $post_types ); - $xml = $this->aggregate_site_schema_map_command_handler->handle( $command ); - - $this->xml_cache_manager->set( $xml ); - } - $response = new WP_REST_Response( $xml, 200 ); + $response = new WP_REST_Response( $this->schema_map_xml_provider->get_xml(), 200 ); $response->header( 'Content-Type', 'application/xml; charset=UTF-8' ); $response->header( 'Cache-Control', 'public, max-age=300' ); diff --git a/tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Abstract_Schema_Map_Xml_Provider_Test.php b/tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Abstract_Schema_Map_Xml_Provider_Test.php new file mode 100644 index 00000000000..1bf46d32094 --- /dev/null +++ b/tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Abstract_Schema_Map_Xml_Provider_Test.php @@ -0,0 +1,68 @@ +command_handler = Mockery::mock( Aggregate_Site_Schema_Map_Command_Handler::class ); + $this->xml_cache_manager = Mockery::mock( Xml_Manager::class ); + $this->aggregator_config = Mockery::mock( Aggregator_Config::class ); + + $this->instance = new Schema_Map_Xml_Provider( + $this->command_handler, + $this->xml_cache_manager, + $this->aggregator_config, + ); + } +} diff --git a/tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Constructor_Test.php b/tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Constructor_Test.php new file mode 100644 index 00000000000..cc39a7b6de3 --- /dev/null +++ b/tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Constructor_Test.php @@ -0,0 +1,38 @@ +assertInstanceOf( + Aggregate_Site_Schema_Map_Command_Handler::class, + $this->getPropertyValue( $this->instance, 'aggregate_site_schema_map_command_handler' ), + ); + $this->assertInstanceOf( + Xml_Manager::class, + $this->getPropertyValue( $this->instance, 'xml_cache_manager' ), + ); + $this->assertInstanceOf( + Aggregator_Config::class, + $this->getPropertyValue( $this->instance, 'aggregator_config' ), + ); + } +} diff --git a/tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Get_Xml_Test.php b/tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Get_Xml_Test.php new file mode 100644 index 00000000000..8b920f1ac5c --- /dev/null +++ b/tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Get_Xml_Test.php @@ -0,0 +1,61 @@ +'; + + $this->xml_cache_manager->expects( 'get' )->once()->andReturn( $cached_xml ); + $this->aggregator_config->expects( 'get_allowed_post_types' )->never(); + $this->command_handler->expects( 'handle' )->never(); + $this->xml_cache_manager->expects( 'set' )->never(); + + $this->assertSame( $cached_xml, $this->instance->get_xml() ); + } + + /** + * Tests that the XML is built and cached when there is no cached XML. + * + * @return void + */ + public function test_get_xml_builds_and_caches_on_a_cache_miss() { + $built_xml = ''; + + $this->xml_cache_manager->expects( 'get' )->once()->andReturn( null ); + $this->aggregator_config->expects( 'get_allowed_post_types' )->once()->andReturn( [ 'post', 'page' ] ); + + $this->command_handler->expects( 'handle' ) + ->once() + ->with( + Mockery::on( + static function ( $command ) { + return $command instanceof Aggregate_Site_Schema_Map_Command + && $command->get_post_types() === [ 'post', 'page' ]; + }, + ), + ) + ->andReturn( $built_xml ); + + $this->xml_cache_manager->expects( 'set' )->once()->with( $built_xml )->andReturn( true ); + + $this->assertSame( $built_xml, $this->instance->get_xml() ); + } +} diff --git a/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Abstract_Schemamap_Xml_Rewrite_Integration_Test.php b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Abstract_Schemamap_Xml_Rewrite_Integration_Test.php new file mode 100644 index 00000000000..329616231d9 --- /dev/null +++ b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Abstract_Schemamap_Xml_Rewrite_Integration_Test.php @@ -0,0 +1,59 @@ +schema_map_xml_provider = Mockery::mock( Schema_Map_Xml_Provider::class ); + $this->redirect_helper = Mockery::mock( Redirect_Helper::class ); + + $this->instance = new Schemamap_Xml_Rewrite_Integration( + $this->schema_map_xml_provider, + $this->redirect_helper, + ); + } +} diff --git a/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Add_Query_Vars_Test.php b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Add_Query_Vars_Test.php new file mode 100644 index 00000000000..eadb352cc96 --- /dev/null +++ b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Add_Query_Vars_Test.php @@ -0,0 +1,38 @@ +assertSame( [ 'yoast_schemamap' ], $this->instance->add_query_vars( [] ) ); + } + + /** + * Tests that the existing query variables are preserved. + * + * @return void + */ + public function test_add_query_vars_preserves_existing_query_vars() { + $this->assertSame( + [ 'sitemap', 'sitemap_n', 'yoast_schemamap' ], + $this->instance->add_query_vars( [ 'sitemap', 'sitemap_n' ] ), + ); + } +} diff --git a/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Add_Rewrite_Rules_Test.php b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Add_Rewrite_Rules_Test.php new file mode 100644 index 00000000000..1ce1b3661da --- /dev/null +++ b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Add_Rewrite_Rules_Test.php @@ -0,0 +1,35 @@ +expects( 'add_rule' ) + ->once() + ->with( 'schemamap\.xml$', 'index.php?yoast_schemamap=1', 'top' ); + + $this->instance->add_rewrite_rules( $dynamic_rewrites ); + } +} diff --git a/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Get_Conditionals_Test.php b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Get_Conditionals_Test.php new file mode 100644 index 00000000000..b9f5e6b54de --- /dev/null +++ b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Get_Conditionals_Test.php @@ -0,0 +1,34 @@ +assertSame( $expected, $this->instance::get_conditionals() ); + } +} diff --git a/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Maybe_Render_Schema_Map_Test.php b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Maybe_Render_Schema_Map_Test.php new file mode 100644 index 00000000000..cfa3cbdbeed --- /dev/null +++ b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Maybe_Render_Schema_Map_Test.php @@ -0,0 +1,155 @@ +allows( 'is_main_query' )->andReturn( $is_main_query ); + $query->allows( 'get' )->with( 'yoast_schemamap' )->andReturn( $query_var ); + + return $query; + } + + /** + * Builds a partial mock with the methods that cannot run under test stubbed out. + * + * @param bool $headers_sent What the headers_already_sent check should report. + * + * @return Mockery\MockInterface|Schemamap_Xml_Rewrite_Integration The partial mock. + */ + private function mock_instance( $headers_sent ) { + $instance = Mockery::mock( + Schemamap_Xml_Rewrite_Integration::class, + [ $this->schema_map_xml_provider, $this->redirect_helper ], + )->makePartial()->shouldAllowMockingProtectedMethods(); + + $instance->allows( 'headers_already_sent' )->andReturn( $headers_sent ); + + return $instance; + } + + /** + * Tests that nothing is rendered for a secondary query. + * + * @return void + */ + public function test_maybe_render_schema_map_ignores_secondary_queries() { + $this->schema_map_xml_provider->expects( 'get_xml' )->never(); + + $this->expectOutputString( '' ); + + $this->instance->maybe_render_schema_map( $this->mock_query( false, '1' ) ); + } + + /** + * Tests that nothing is rendered when the request is not for the schema map. + * + * @return void + */ + public function test_maybe_render_schema_map_does_nothing_for_other_requests() { + $this->schema_map_xml_provider->expects( 'get_xml' )->never(); + + $this->expectOutputString( '' ); + + $this->instance->maybe_render_schema_map( $this->mock_query( true, '' ) ); + } + + /** + * Tests that nothing is rendered when the response can no longer be typed as XML. + * + * @return void + */ + public function test_maybe_render_schema_map_bails_when_headers_are_sent() { + $instance = $this->mock_instance( true ); + $instance->expects( 'send_headers' )->never(); + $instance->expects( 'finish_request' )->never(); + + $this->schema_map_xml_provider->expects( 'get_xml' )->never(); + + $this->expectOutputString( '' ); + + $instance->maybe_render_schema_map( $this->mock_query( true, '1' ) ); + } + + /** + * Tests that the schema map XML is echoed for a schema map request. + * + * @return void + */ + public function test_maybe_render_schema_map_outputs_the_xml() { + $xml = ''; + + $this->schema_map_xml_provider->expects( 'get_xml' )->once()->andReturn( $xml ); + + $instance = $this->mock_instance( false ); + $instance->expects( 'send_headers' )->once(); + $instance->expects( 'finish_request' )->once(); + + $this->expectOutputString( $xml ); + + $instance->maybe_render_schema_map( $this->mock_query( true, '1' ) ); + } + + /** + * Tests that the XML is built before any header is committed. + * + * A build failure after the 200 and the cache headers went out would let a proxy store an error + * page at this path for the full max-age, so the ordering is part of the contract. + * + * @return void + */ + public function test_maybe_render_schema_map_builds_the_xml_before_sending_headers() { + $calls = []; + + $this->schema_map_xml_provider->expects( 'get_xml' ) + ->once() + ->andReturnUsing( + static function () use ( &$calls ) { + $calls[] = 'get_xml'; + + return ''; + }, + ); + + $instance = $this->mock_instance( false ); + $instance->expects( 'send_headers' ) + ->once() + ->andReturnUsing( + static function () use ( &$calls ) { + $calls[] = 'send_headers'; + }, + ); + $instance->expects( 'finish_request' )->once(); + + $this->expectOutputString( '' ); + + $instance->maybe_render_schema_map( $this->mock_query( true, '1' ) ); + + $this->assertSame( [ 'get_xml', 'send_headers' ], $calls ); + } +} diff --git a/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Register_Hooks_Test.php b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Register_Hooks_Test.php new file mode 100644 index 00000000000..d6314d64dbe --- /dev/null +++ b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Register_Hooks_Test.php @@ -0,0 +1,89 @@ +reset_dynamic_rewrites(); + + $wp_rewrite = Mockery::mock( 'WP_Rewrite' ); + $wp_rewrite->index = 'index.php'; + $wp_rewrite->expects( 'add_rule' )->once(); + + $GLOBALS['wp_rewrite'] = $wp_rewrite; + } + + /** + * Resets the Yoast_Dynamic_Rewrites singleton so it does not leak its WP_Rewrite mock into other tests. + * + * @return void + */ + protected function tear_down() { + $this->reset_dynamic_rewrites(); + + unset( $GLOBALS['wp_rewrite'] ); + + parent::tear_down(); + } + + /** + * Clears the Yoast_Dynamic_Rewrites static instance. + * + * @return void + */ + private function reset_dynamic_rewrites() { + $instance = new ReflectionProperty( Yoast_Dynamic_Rewrites::class, 'instance' ); + $instance->setAccessible( true ); + $instance->setValue( null, null ); + } + + /** + * Tests the registration of the hooks. + * + * @return void + */ + public function test_register_hooks() { + $this->instance->register_hooks(); + + $this->assertNotFalse( + Monkey\Filters\has( 'query_vars', [ $this->instance, 'add_query_vars' ] ), + ); + + /* + * The priority is asserted, not just the registration: rendering has to happen before the + * main query runs and before other callbacks get a chance to echo ahead of the XML. + */ + $this->assertSame( + 1, + Monkey\Actions\has( 'pre_get_posts', [ $this->instance, 'maybe_render_schema_map' ] ), + ); + } +} diff --git a/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Send_Headers_Test.php b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Send_Headers_Test.php new file mode 100644 index 00000000000..0bb743b4e92 --- /dev/null +++ b/tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Send_Headers_Test.php @@ -0,0 +1,43 @@ +once()->with( 200 ); + + $this->redirect_helper->expects( 'set_header' )->once()->with( 'X-Robots-Tag: noindex, follow' ); + $this->redirect_helper->expects( 'set_header' )->once()->with( 'Content-Type: application/xml; charset=UTF-8' ); + $this->redirect_helper->expects( 'set_header' )->once()->with( 'Cache-Control: public, max-age=300' ); + + /* + * WP::send_headers() pairs its no-cache Cache-Control with an Expires date in the past for + * logged-in users. Replacing Cache-Control alone would leave the two contradicting. + */ + $this->redirect_helper->expects( 'remove_header' )->once()->with( 'Expires' ); + + $send_headers = new ReflectionMethod( $this->instance, 'send_headers' ); + $send_headers->setAccessible( true ); + $send_headers->invoke( $this->instance ); + } +} diff --git a/tests/Unit/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_XML_Route/Abstract_Site_Schema_Aggregator_Xml_Route_Test.php b/tests/Unit/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_XML_Route/Abstract_Site_Schema_Aggregator_Xml_Route_Test.php index 8766b576bab..e57ac3f0e3f 100644 --- a/tests/Unit/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_XML_Route/Abstract_Site_Schema_Aggregator_Xml_Route_Test.php +++ b/tests/Unit/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_XML_Route/Abstract_Site_Schema_Aggregator_Xml_Route_Test.php @@ -4,9 +4,7 @@ namespace Yoast\WP\SEO\Tests\Unit\Schema_Aggregator\User_Interface\Site_Schema_Aggregator_XML_Route; use Mockery; -use Yoast\WP\SEO\Schema_Aggregator\Application\Aggregate_Site_Schema_Map_Command_Handler; -use Yoast\WP\SEO\Schema_Aggregator\Application\Cache\Xml_Manager; -use Yoast\WP\SEO\Schema_Aggregator\Infrastructure\Aggregator_Config; +use Yoast\WP\SEO\Schema_Aggregator\Application\Schema_Map\Schema_Map_Xml_Provider; use Yoast\WP\SEO\Schema_Aggregator\User_Interface\Site_Schema_Aggregator_Xml_Route; use Yoast\WP\SEO\Tests\Unit\TestCase; @@ -27,25 +25,11 @@ abstract class Abstract_Site_Schema_Aggregator_Xml_Route_Test extends TestCase { protected $instance; /** - * Holds the command handler mock. + * Holds the schema map XML provider mock. * - * @var Mockery\MockInterface|Aggregate_Site_Schema_Map_Command_Handler + * @var Mockery\MockInterface|Schema_Map_Xml_Provider */ - protected $command_handler; - - /** - * Holds the XML cache manager mock. - * - * @var Mockery\MockInterface|Xml_Manager - */ - protected $xml_cache_manager; - - /** - * Holds the aggregator config mock. - * - * @var Mockery\MockInterface|Aggregator_Config - */ - protected $aggregator_config; + protected $schema_map_xml_provider; /** * Sets up the test fixtures. @@ -55,14 +39,8 @@ abstract class Abstract_Site_Schema_Aggregator_Xml_Route_Test extends TestCase { protected function set_up() { parent::set_up(); - $this->command_handler = Mockery::mock( Aggregate_Site_Schema_Map_Command_Handler::class ); - $this->xml_cache_manager = Mockery::mock( Xml_Manager::class ); - $this->aggregator_config = Mockery::mock( Aggregator_Config::class ); + $this->schema_map_xml_provider = Mockery::mock( Schema_Map_Xml_Provider::class ); - $this->instance = new Site_Schema_Aggregator_Xml_Route( - $this->command_handler, - $this->xml_cache_manager, - $this->aggregator_config, - ); + $this->instance = new Site_Schema_Aggregator_Xml_Route( $this->schema_map_xml_provider ); } } diff --git a/tests/WP/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration_Test.php b/tests/WP/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration_Test.php new file mode 100644 index 00000000000..b58f802c6b0 --- /dev/null +++ b/tests/WP/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration_Test.php @@ -0,0 +1,126 @@ +helpers->options->set( 'enable_schema_aggregation_endpoint', true ); + + /* + * The provider is only ever a constructor dependency, so the DI compiler inlines it and it + * cannot be fetched from the container. These tests cover routing rather than the document, + * which Site_Schema_Aggregator_Xml_Route_Test already exercises against the real provider. + */ + $this->instance = new Schemamap_Xml_Rewrite_Integration( + Mockery::mock( Schema_Map_Xml_Provider::class ), + \YoastSEO()->helpers->redirect, + ); + + $this->set_permalink_structure( '/%postname%/' ); + $this->instance->register_hooks(); + + /* + * go_to() runs the main query, which would reach the render callback and exit the PHPUnit + * process. The rewrite rule and the query var are what these tests assert, so the callback + * is unhooked; rendering is covered by the unit tests. + */ + \remove_action( 'pre_get_posts', [ $this->instance, 'maybe_render_schema_map' ], 1 ); + } + + /** + * Restores the default permalink structure. + * + * @return void + */ + public function tear_down() { + $this->set_permalink_structure( '' ); + + parent::tear_down(); + } + + /** + * Tests that the rewrite rule is applied to the rewrite rules option. + * + * @return void + */ + public function test_rewrite_rule_is_registered() { + global $wp_rewrite; + + $this->assertArrayHasKey( 'schemamap\.xml$', $wp_rewrite->wp_rewrite_rules() ); + $this->assertSame( + 'index.php?yoast_schemamap=1', + $wp_rewrite->wp_rewrite_rules()['schemamap\.xml$'], + ); + } + + /** + * Tests that a request for the schema map resolves to the query variable. + * + * @return void + */ + public function test_request_sets_the_query_var() { + $this->go_to( \home_url( '/schemamap.xml' ) ); + + $this->assertFalse( \is_404() ); + $this->assertSame( '1', \get_query_var( 'yoast_schemamap' ) ); + } + + /** + * Tests that a trailing slash still resolves to the query variable. + * + * @return void + */ + public function test_trailing_slash_request_sets_the_query_var() { + $this->go_to( \home_url( '/schemamap.xml/' ) ); + + $this->assertFalse( \is_404() ); + $this->assertSame( '1', \get_query_var( 'yoast_schemamap' ) ); + } + + /** + * Tests that the schema map is not routed when pretty permalinks are off. + * + * This documents the known limitation: rewrite rules do not run under plain permalinks, so + * discovery falls back to the REST endpoint there. + * + * @return void + */ + public function test_plain_permalinks_do_not_route() { + $this->set_permalink_structure( '' ); + + $this->go_to( \home_url( '/schemamap.xml' ) ); + + $this->assertSame( '', \get_query_var( 'yoast_schemamap' ) ); + } +} diff --git a/tests/WP/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_Xml_Route_Test.php b/tests/WP/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_Xml_Route_Test.php index ee0209ef479..4c2d423bd78 100644 --- a/tests/WP/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_Xml_Route_Test.php +++ b/tests/WP/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_Xml_Route_Test.php @@ -2,14 +2,8 @@ // @phpcs:disable Yoast.NamingConventions.NamespaceName.TooLong -- This namespace should reflect the namespace of the original class. namespace Yoast\WP\SEO\Tests\WP\Schema_Aggregator\User_Interface; -use Mockery; use WP_REST_Request; use WP_REST_Response; -use Yoast\WP\SEO\Schema_Aggregator\Application\Aggregate_Site_Schema_Map_Command_Handler; -use Yoast\WP\SEO\Schema_Aggregator\Application\Cache\Xml_Manager; -use Yoast\WP\SEO\Schema_Aggregator\Infrastructure\Aggregator_Config; -use Yoast\WP\SEO\Schema_Aggregator\Infrastructure\Config; -use Yoast\WP\SEO\Schema_Aggregator\User_Interface\Site_Schema_Aggregator_Xml_Route; use Yoast\WP\SEO\Tests\WP\TestCase; /** @@ -18,69 +12,29 @@ * @group schema-aggregator * * @covers Yoast\WP\SEO\Schema_Aggregator\User_Interface\Site_Schema_Aggregator_Xml_Route::render_schema_xml - * @covers Yoast\WP\SEO\Schema_Aggregator\User_Interface\Site_Schema_Aggregator_Xml_Route::__construct + * @covers Yoast\WP\SEO\Schema_Aggregator\Application\Schema_Map\Schema_Map_Xml_Provider::get_xml */ final class Site_Schema_Aggregator_Xml_Route_Test extends TestCase { - /** - * Holds the instance. - * - * @var Site_Schema_Aggregator_Xml_Route - */ - private $instance; - - /** - * Holds the aggregate site schema map command handler mock. - * - * @var Aggregate_Site_Schema_Map_Command_Handler - */ - private $aggregate_site_schema_map_command_handler; - - /** - * Holds the XML cache manager mock. - * - * @var Xml_Manager - */ - private $xml_cache_manager; - - /** - * Holds the aggregator config mock. - * - * @var Aggregator_Config - */ - private $aggregator_config; - /** * Set up the test. * * @return void */ public function set_up() { - $this->aggregate_site_schema_map_command_handler = Mockery::mock( Aggregate_Site_Schema_Map_Command_Handler::class ); - $this->xml_cache_manager = Mockery::mock( Xml_Manager::class ); - $this->aggregator_config = Mockery::mock( Aggregator_Config::class ); - $this->instance = new Site_Schema_Aggregator_Xml_Route( $this->aggregate_site_schema_map_command_handler, $this->xml_cache_manager, $this->aggregator_config ); + parent::set_up(); + \YoastSEO()->helpers->options->set( 'enable_schema_aggregation_endpoint', true ); \do_action( 'rest_api_init' ); } /** - * Tests the xml map without cache. + * Tests that the route renders the schema map as XML. * * @return void */ - public function test_render_schema_xml_no_cache() { - - $this->xml_cache_manager->expects( 'get' ) - ->once() - ->andReturn( null ); - $this->aggregator_config->expects( 'get_allowed_post_types' ) - ->once() - ->andReturn( [ 'page' ] ); - $this->xml_cache_manager->expects( 'set' ) - ->once(); - + public function test_render_schema_xml() { $request = new WP_REST_Request( 'GET', '/yoast/v1/schema-aggregator/get-xml' ); $response = \rest_get_server()->dispatch( $request ); @@ -94,29 +48,15 @@ public function test_render_schema_xml_no_cache() { } /** - * Tests the xml map with cache. + * Tests that rendering stores the XML in the transient the front-end route also reads. * * @return void */ - public function test_render_schema_xml_with_cache() { - $this->xml_cache_manager->expects( 'get' ) - ->once() - ->andReturn( '' ); - $this->aggregator_config->expects( 'get_allowed_post_types' ) - ->never(); - $this->xml_cache_manager->expects( 'set' ) - ->never(); - - $request = new WP_REST_Request( 'GET', '/yoast/v1/schema-aggregator/get-xml' ); - $response = \rest_get_server()->dispatch( $request ); + public function test_render_schema_xml_populates_the_cache() { + \delete_transient( 'yoast_schema_aggregator_xml_sitemap_v1' ); - $this->assertInstanceOf( WP_REST_Response::class, $response ); - - $response_data = $response->get_data(); + $response = \rest_get_server()->dispatch( new WP_REST_Request( 'GET', '/yoast/v1/schema-aggregator/get-xml' ) ); - $this->assertSame( 200, $response->status ); - $this->assertStringContainsString( 'assertSame( 'application/xml; charset=UTF-8', $response->get_headers()['Content-Type'] ); - $this->assertSame( 'public, max-age=300', $response->get_headers()['Cache-Control'] ); + $this->assertSame( $response->get_data(), \get_transient( 'yoast_schema_aggregator_xml_sitemap_v1' ) ); } }