Template / Template parts revision / autosave REST API are broken

[spacedmonkey]

The REST API endpoints for autosave and revisions the template and template part are broken. The URL structure for template and template part are different from other post controllers.

Standard post controller.

'/' . $this->rest_base . '/(?P<id>[\d]+)',

Template / template parts.

'/%s/(?P<id>%s%s)',
                                $this->rest_base,
                                // Matches theme's directory: `/themes/<subdirectory>/<theme>/` or `/themes/<theme>/`.
                                // Excludes invalid directory name characters: `/:<>*?"|`.
                                '([^\/:<>\*\?"\|]+(?:\/[^\/:<>\*\?"\|]+)?)',
                                // Matches the template name.
                                '[\/\w-]+'

Revision / autosave endpoints expects

'/' . $this->parent_base . '/(?P<parent>[\d]+)/' . $this->rest_base,

Autosave endpoint expects.

When doing a request like this.

wp/v2/template-parts/1480/revisions

Results in

code
: 
"rest_template_not_found"
data
: 
{status: 404}
message
: 
"No templates exist with that id."

As this post type needs a different url structure, a custom revision / autosave will need to be registered.

Trac ticket: https://core.trac.wordpress.org/ticket/56922

[spacedmonkey]

I have made a start on a solution that implements custom autosave / revision endpoints.

[revgeorge]

I think that the path wp/v2/template-parts/1480/revisions is pretty easy to fix, by adding (?<!/revisions) to the end of the route regex in WP_REST_Templates_Controller. This is a negative-lookbehind regex that says "the URL can't end in /revisions".

The harder part is avoiding links to individual revisions like wp/v2/template-parts/1480/revisions/4321. Negative-lookbehinds must be fixed length, we can't do something like (?<!/revisions/\d+) because \d+ could be any size and the regex engine doesn't want to backtrack an arbitrary number of steps.

One thought I had was for the REST API to be able to use a callable as a route in addition to a regex. This would allow for more complex logic than a regex allows. However, it may make it more difficult to use register_rest_route() to override an existing route and make it easy to accidentally add big performance problems.

Another option, that probably won't work due to backwards compatibility, would be to use "The Greatest Regex Trick Ever" (​https://www.rexegg.com/regex-best-trick.html), which is to force regexes to include at least one capture group (something in parenthesis) and to ignore routes that simply match. This would allow us to have a regex like

\d+/revisions$|\d+/revisions/d+$|/templates/(?P<id>([^\/:<>\*\?"\|]+(?:\/[^\/:<>\*\?"\|]+)?)'[\/\w-]+',)

which would match on \d+/revisions$ or \d+/revisions/d+$ but, since they don't have a capture group, the updated router would treat them as non-matches. Existing routes without capture groups (e.g. /templates) would have to be updated though.

[spacedmonkey]

@TimothyBlynJacobs Love your thoughts on this.

What would the autosaves / revisions controllers look like when they need to be custom for templates/template parts.

[andraganescu]

Hello! Is this still something that has a chance to land on 6.2? :) I know it's really late.

[hellofromTonya]

Moving this into 6.4 milestone for visibility and to hopefully wrangle some folks to possibility get it moving forward in time to ship in 6.4.

[antonvlasenko]

Just FYI: I am working on a PR for this issue and am also verifying the solution proposed in ​https://github.com/WordPress/wordpress-develop/pull/3533.

[spacedmonkey]

@antonvlasenko The current PR is just part of the problem. Basically we need the ablity to register custom autosave / revision endpoints for post types. That way we can fix the regex for just those post types. It is not a complex fix, but it might be a lot of code. I just haven't had time to work on it.

### What and why?

Fixes https://core.trac.wordpress.org/ticket/56922.

This PR addresses route conflicts between WP_REST_Templates_Controller and both WP_REST_Revisions_Controller and WP_REST_Autosaves_Controller.

When making REST requests to the following endpoints:

• wp/v2/templates/<parent_post_id>/revisions
• wp/v2/templates/<parent_post_id>/autosaves
• wp/v2/template-parts/<parent_post_id>/revisions
• wp/v2/template-parts/<parent_post_id>/autosaves

the WP_REST_Templates_Controller::get_item() method is incorrectly matched as the callback method to process these requests. This misidentification arises from the regular expression that defines the problematic route in the WP_REST_Templates_Controller class, specifically:
/(?P<id>([^\/:<>\*\?"\|]+(?:\/[^\/:<>\*\?"\|]+)?)[\/\w%-]+).

In actuality, the aforementioned requests should be handled by the WP_REST_Revisions_Controller::get_items() and WP_REST_Autosaves_Controller::get_items() callbacks, depending on whether it's a revisions or autosaves request.

### How

This PR modifies the order in which the routes for endpoints in WP_REST_Templates_Controller, WP_REST_Revisions_Controller, and WP_REST_Autosaves_Controller are registered.

Routes in WP_REST_Revisions_Controller and WP_REST_Autosaves_Controller are now registered prior to those in WP_REST_Templates_Controller.

This ensures the correct callback is matched, preventing WP_REST_Templates_Controller::get_item() from being mistakenly matched for the endpoints listed above.

However, this change is limited to the wp_template and wp_template_parts post types to mitigate the risk of introducing new bugs.

[antonvlasenko]

Thank you for the information, @spacedmonkey.

I suggest considering the PR I've just submitted - ​https://github.com/WordPress/wordpress-develop/pull/5194 - as an alternative solution.
I believe it addresses the issue, assuming my understanding of it is accurate.

What and why?

This PR addresses route conflicts between WP_REST_Templates_Controller and both WP_REST_Revisions_Controller and WP_REST_Autosaves_Controller.

When making REST requests to the following endpoints:

• wp/v2/templates/<parent_post_id>/revisions
• wp/v2/templates/<parent_post_id>/autosaves
• wp/v2/template-parts/<parent_post_id>/revisions
• wp/v2/template-parts/<parent_post_id>/autosaves

the WP_REST_Templates_Controller::get_item() method is incorrectly matched as the callback method to handle these requests.
This misidentification arises from the regular expression that defines the problematic route in the WP_REST_Templates_Controller class, specifically:
/(?P<id>([^\/:<>\*\?"\|]+(?:\/[^\/:<>\*\?"\|]+)?)[\/\w%-]+).

In actuality, the aforementioned requests should be handled by the WP_REST_Revisions_Controller::get_items() and WP_REST_Autosaves_Controller::get_items() callbacks, depending on whether it's a revisions or autosaves request.

How

This PR modifies the order in which the routes for endpoints in WP_REST_Templates_Controller, WP_REST_Revisions_Controller, and WP_REST_Autosaves_Controller are registered.

Routes in WP_REST_Revisions_Controller and WP_REST_Autosaves_Controller are now registered prior to those in WP_REST_Templates_Controller.

This ensures the correct callback is matched, preventing WP_REST_Templates_Controller::get_item() from being mistakenly matched for the endpoints listed above.

However, this change is limited to the wp_template and wp_template_parts post types to mitigate the risk of introducing new bugs.

Why register late? Why not register all post type controller like this?

[antonvlasenko]

@spacedmonkey

Why register late?

The root cause of the issue is that the WP_REST_Templates_Controller::get_item() method is incorrectly matched as the callback method to handle revisions and autosave requests for the wp_template and wp_template_part post types.
Do we agree on this?

If so, the solution would be to register the routes defined in both the WP_REST_Revisions_Controller and WP_REST_Autosaves_Controller controllers first.
Then, routes defined in WP_REST_Templates_Controller should be registered afterward.
This ensures that WP_REST_Server::match_request_to_handler() matches correct routes first.
By doing this, when making a request like wp/v2/template-parts/1480/revisions, the correct request handler is called.

I hope this explanation makes sense.
I provided a more detailed explanation in my previous comment.

Why not register all post type controllers like this?

The ability to arbitrarily specify custom post type controllers for post types can be useful.
However, it's unclear to me how this would address the route conflict mentioned earlier.
My PR should resolve the issue, and it seems no additional PRs are necessary.

I will be looking forward to hearing your feedback.

[antonvlasenko]

Test Report

This report confirms that the specified PR addresses the issue.

PR tested: ​https://github.com/WordPress/wordpress-develop/pull/5194

Environment

• WordPress: 6.4-alpha-56267-src
• PHP: 7.3.33
• Server: Apache/2.4.57 (Unix) PHP/7.3.33
• Database: mysqli (Server: 5.7.43)
• Browser: Safari 16.6 (macOS)
• Theme: Twenty Twenty-Three 1.2
• MU-Plugins: None activated
• Plugins:
  • JSON Basic Authentication 0.1

Steps to Reproduce

1. Navigate to Appearance -> Editor.
2. Create a new template part (also known as "pattern").
3. Save the template part multiple times, making slight modifications to the template before each save. For instance, add new blocks or alter the text.
4. Identify the ID of your template part in the wp_posts table. You can use the following SQL query to find the ID of the template part you just created:

   SELECT 
       ID
   FROM
       wp_posts
   WHERE
       post_type = 'wp_template_part'
   ORDER BY post_date DESC
   LIMIT 1;
   

5. Record the ID value.
6. Install and activate the ​https://github.com/WP-API/Basic-Auth plugin to simplify the authentication of REST requests.
7. Make a REST request to the template parts endpoint. Replace admin:password with your admin credentials and <template_part_id> with the ID value from step 5.

   curl http://path.to.your.wordpress.server/wp-json/wp/v2/template-parts/<template_part_id>/revisions -u "admin:password"
   

8. 🐞 The following response will be received:

   {"code":"rest_template_not_found","message":"No templates exist with that id.","data":{"status":404}}
   

Actual Results

When testing the patch to ensure it functions as intended:

• ✅ A REST request should return a list of the revisions for your template part. For example:

  [{"author":1,"date":"2023-09-12T12:14:30","date_gmt":"2023-09-12T12:14:30","id":10,"modified":"2023-09-12T12:14:30","modified_gmt":"2023-09-12T12:14:30","parent":7,"slug":"7-revision-v1","guid":{"rendered":"http:\/\/wordpress.test\/?p=10","raw":"http:\/\/wordpress.test\/?p=10"},"title":{"raw":"Template Part","rendered":"Template Part"},"content":{"raw":"<!-- wp:paragraph -->\n<p>A simple p. #3<\/p>\n<!-- \/wp:paragraph -->","rendered":"\n<p>A simple p. #3<\/p>\n"},"_links":{"parent":[{"href":"http:\/\/wordpress.test\/wp-json\/wp\/v2\/template-parts\/7"}]}},{"author":1,"date":"2023-09-12T12:14:21","date_gmt":"2023-09-12T12:14:21","id":9,"modified":"2023-09-12T12:14:21","modified_gmt":"2023-09-12T12:14:21","parent":7,"slug":"7-revision-v1","guid":{"rendered":"http:\/\/wordpress.test\/?p=9","raw":"http:\/\/wordpress.test\/?p=9"},"title":{"raw":"Template Part","rendered":"Template Part"},"content":{"raw":"<!-- wp:paragraph -->\n<p>A simple p. #2<\/p>\n<!-- \/wp:paragraph -->","rendered":"\n<p>A simple p. #2<\/p>\n"},"_links":{"parent":[{"href":"http:\/\/wordpress.test\/wp-json\/wp\/v2\/template-parts\/7"}]}},{"author":1,"date":"2023-09-12T12:14:08","date_gmt":"2023-09-12T12:14:08","id":8,"modified":"2023-09-12T12:14:08","modified_gmt":"2023-09-12T12:14:08","parent":7,"slug":"7-revision-v1","guid":{"rendered":"http:\/\/wordpress.test\/?p=8","raw":"http:\/\/wordpress.test\/?p=8"},"title":{"raw":"Template Part","rendered":"Template Part"},"content":{"raw":"<!-- wp:paragraph -->\n<p>A simple p.<\/p>\n<!-- \/wp:paragraph -->","rendered":"\n<p>A simple p.<\/p>\n"},"_links":{"parent":[{"href":"http:\/\/wordpress.test\/wp-json\/wp\/v2\/template-parts\/7"}]}}]
  

In conclusion, the proposed patch successfully addresses the issue. You can also test the templates endpoint for further verification.

[oglekler]

I am adding needs-testing because here we have only one test report, and it is from the patch developer :)

Why register late? Why not register all post type controller like this?

I've replied in the ticket: https://core.trac.wordpress.org/ticket/56922#comment:13
Thank you.

[spacedmonkey]

@antonvlasenko Your PR doesn't fix the problem. Let me explain.

The url pattern for template / template part is like this.

`
wp/v2/template-parts/twentytwentythreefooter
`

They do not use id based lookups. Meaning the urls are broken. This is why this needed a custom controller.

[antonvlasenko]

Exactly! I wasn't sure if my solution is correct, so I really appreciate your input, @spacedmonkey.
I will update my PR accordingly.

[spacedmonkey]

@antonvlasenko I have updated my PR with my thinking - See ​https://github.com/WordPress/wordpress-develop/pull/3533

Basically have a custom autosave / revisions controller with register different pattern urls. The PR is far from complete, but you want to use it as a base, that could work on getting a solution in core.

Closed in favor of ​https://github.com/WordPress/wordpress-develop/pull/3533.