Serve the aggregated schema map at /schemamap.xml - #23510
Conversation
The XML REST route was the only place performing the read-through-cache sequence for the schema map. A second consumer is about to need it, so the sequence moves into Schema_Map_Xml_Provider rather than being duplicated. This also gives the sequence its own unit tests; it previously had none, since the route only exercised it through the WP integration test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The schema visualiser used to discover a site's schema map through the Schemamap directive in robots.txt. That directive has been removed, and the visualiser now probes the well-known root /schemamap.xml first, falling back to the wp-json endpoint. Only the fallback was served until now. The route mirrors WP_Sitemaps: a rewrite rule pointing at a query var, and a template_redirect handler that echoes the XML and exits. Yoast_Dynamic_Rewrites is used instead of add_rewrite_rule() so the rule follows the feature toggle without needing a rewrite flush. Integrations register on init priority 10, after yoast_add_dynamic_rewrite_rules has fired, so the rule is added directly the way WPSEO_Sitemaps::register_sitemap() does. Under plain permalinks the rewrite does not run, so /schemamap.xml 404s there, the same as /sitemap_index.xml. Discovery falls back to the wp-json endpoint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses code review on the /schemamap.xml route. Build the XML before committing any header. get_xml() queries the database and Schema_Map_Xml_Renderer can throw, so a failure after the 200 and the cache headers went out would let a proxy store an HTML error page at the well-known path for the full max-age. The visualiser rejects anything that is not a sitemap document, so that window mattered. Render on pre_get_posts priority 1 with an is_main_query() guard instead of template_redirect priority 0, matching WPSEO_Sitemaps::redirect(). This skips the main posts query, which the response never uses, and leaves fewer third-party callbacks able to echo ahead of the document. Clear Expires alongside Cache-Control. wp_get_nocache_headers() sets both for logged-in users, so overriding only Cache-Control left the two contradicting. Headers now go through Redirect_Helper, which exists to make them testable; header() and headers_sent() are PHP internals that Patchwork cannot stub. Drop the redirect_canonical filter. Core registers redirect_canonical() on template_redirect priority 10 and applies the filter nowhere else, so exiting during pre_get_posts makes the callback unreachable. Its two tests asserted a path no request can take. Also adds real coverage for send_headers() and the build-before-headers ordering, asserts the hook priority rather than just its registration, and replaces a WP cache test that passed whether or not caching existed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Coverage Report for CI Build 2Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-0.5%) to 54.947%Details
Uncovered Changes
Coverage Regressions419 previously-covered lines in 8 files lost coverage.
Coverage Stats💛 - Coveralls |
headers_already_sent() and finish_request() are protected one-line wrappers, which read as unexplained indirection. Both exist so Maybe_Render_Schema_Map_Test can stub them on a Mockery partial mock: exit() would terminate the test runner, and the test process cannot control what headers_sent() reports. Record that on the methods, along with why they are protected rather than private. The @codeCoverageIgnore reason on headers_already_sent() was carrying the whole explanation, so it is trimmed back to just the coverage justification. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR makes the aggregated schema map discoverable at the well-known root URL /schemamap.xml by introducing a rewrite-based front-end integration, and refactors the existing REST route to reuse a shared XML provider (so both endpoints serve identical content and share caching).
Changes:
- Add a front-end rewrite integration that serves the schema map at
/schemamap.xml(feature-flagged via the existing schema aggregation endpoint toggle). - Introduce a
Schema_Map_Xml_Providerto centralize XML generation + transient caching, and update the REST route to use it. - Add/adjust WP + unit tests to cover routing behavior, header behavior, and provider caching behavior.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/WP/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_Xml_Route_Test.php | Updates WP integration test coverage for the REST XML route and shared transient usage. |
| tests/WP/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration_Test.php | Adds WP integration tests asserting rewrite rule registration and query-var routing for /schemamap.xml. |
| tests/Unit/Schema_Aggregator/User_Interface/Site_Schema_Aggregator_XML_Route/Abstract_Site_Schema_Aggregator_Xml_Route_Test.php | Refactors unit test base to mock the new XML provider dependency. |
| tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Abstract_Schemamap_Xml_Rewrite_Integration_Test.php | Adds a unit-test base class for the new rewrite integration. |
| tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Add_Query_Vars_Test.php | Unit tests for adding the yoast_schemamap query var. |
| tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Add_Rewrite_Rules_Test.php | Unit tests for the dynamic rewrite rule registration. |
| tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Get_Conditionals_Test.php | Unit tests for conditionals (front-end + schema-aggregator conditional). |
| tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Maybe_Render_Schema_Map_Test.php | Unit tests for conditional rendering logic, ordering, and output behavior. |
| tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Register_Hooks_Test.php | Unit tests for hook registration and priority expectations. |
| tests/Unit/Schema_Aggregator/User_Interface/Schemamap_Xml_Rewrite_Integration/Send_Headers_Test.php | Unit tests asserting XML response headers are set and Expires is removed. |
| tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Abstract_Schema_Map_Xml_Provider_Test.php | Adds provider unit-test base with mocks for command handler/cache/config. |
| tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Constructor_Test.php | Unit tests for provider constructor wiring. |
| tests/Unit/Schema_Aggregator/Application/Schema_Map/Schema_Map_Xml_Provider/Get_Xml_Test.php | Unit tests for cached vs cache-miss behavior of provider XML generation. |
| src/schema-aggregator/user-interface/site-schema-aggregator-xml-route.php | Refactors REST XML route to use the shared Schema_Map_Xml_Provider. |
| src/schema-aggregator/user-interface/schemamap-xml-rewrite-integration.php | Adds the new front-end integration serving /schemamap.xml via rewrite/query var and early render hook. |
| src/schema-aggregator/application/schema_map/schema-map-xml-provider.php | Adds a shared provider responsible for generating and caching the schema map XML. |
| /** | ||
| * Returns a XML representation of the possible post types that can be used for schema. | ||
| * | ||
| * @return WP_REST_Response|WP_Error The success or failure response. | ||
| * @return WP_REST_Response The response. | ||
| */ |
There was a problem hiding this comment.
I think thats legit as well
| 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 ); | ||
| \YoastSEO()->helpers->options->set( 'enable_schema_aggregation_endpoint', true ); | ||
|
|
||
| \do_action( 'rest_api_init' ); | ||
| } |
leonidasmi
left a comment
There was a problem hiding this comment.
CR + Acceptance is ✅
There are two minor suggestions from Copilot that I think are legit @thijsoo. Feel free to merge with or without them, no need for me to re-test
Context
Schemamapdirective was removed fromrobots.txtin Yoast/reserved-tasks#1153, because it caused "Syntax not understood" errors in Google Search Console and Bing Webmaster Tools, and "robots.txt is invalid" warnings in PageSpeed Insights./schemamap.xml, a<link rel="schemamap">tag, and the/wp-json/yoast/v1/schema-aggregator/get-xmlendpoint.Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
WP_Sitemaps: a rewrite rule pointing at a query variable, and a handler that echoes the XML and exits. The content type isapplication/xml; charset=UTF-8, which is what both core's sitemap renderer and our existing REST route already send.Yoast_Dynamic_Rewritesis used instead ofadd_rewrite_rule(), so the rule appears and disappears with the feature toggle without ever needing a rewrite flush. Nothing is written to therewrite_rulesoption./schemamap.xmlreturns a 404 — exactly as/sitemap_index.xmldoes today. Discovery falls back to the wp-json endpoint there. The same applies to sub-directory installs, where the visualiser probes the domain root rather than the WordPress root; WordPress core has the same constraint forrobots.txt.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Preconditions: a site with Yoast SEO from this branch, logged in as an administrator, with a handful of published posts and pages.
The site should not be a multisite/subsite.
https://<your-site>/schemamap.xml.<?xmland containing a<urlset>with one or more<url>entries. It should not be your theme's 404 page.https://<your-site>/wp-json/yoast/v1/schema-aggregator/get-xmland compare.https://<your-site>/schemamap.xml/(with a trailing slash).https://<your-site>/schemamap.xml.<lastmod>date has changed.https://<your-site>/schemamap.xml.https://<your-site>/schemamap.xml./sitemap_index.xmlbehaves. Confirmhttps://<your-site>?rest_route=/yoast/v1/schema-aggregator/get-xmlstill returns the XML.https://<your-site>/schemamap.xmland inspect the response headers (browser devtools → Network tab → select the request).Content-Type: application/xml; charset=UTF-8,Cache-Control: public, max-age=300,X-Robots-Tag: noindex, follow, and noExpiresheader dated 1984.https://<your-site>/sitemap_index.xmlstill loads, andhttps://<your-site>/robots.txtloads and contains noSchemamap:line.Relevant test scenarios
<?xmldeclaration — anything printed ahead of it stops the document being recognised as a schema map./<site>/schemamap.xmlwith its own content, and toggling the setting on one site should not affect another. Note that on a subdirectory site the visualiser's root probe will not reach it — that is a known and accepted limitation, documented above.Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
/wp-json/yoast/v1/schema-aggregator/get-xmlwas refactored to use the new shared provider. Its output, status and headers should be unchanged.yoast_schema_aggregator_xml_sitemap_v1, so cache invalidation on saving content affects both.Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.[yoast-doc-extension], added test instructions for Yoast SEO for Google Docs and attached theGoogle Docs Add-onlabel to this PR.Documentation
Quality assurance
grunt build:imagesand committed the results, if my PR introduces or edits images or SVGs.Innovation
innovationlabel.Fixes Yoast/reserved-tasks#1161