Repository navigation
Abilities API: Add a core/content-query ability - #12195
jorgefilipecosta wants to merge 63 commits into
Conversation
|
Hi there! 👋 Thank you for your contribution to WordPress! 💖 It looks like this is your first pull request to No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making. More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook. Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook. If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook. The Developer Hub also documents the various coding standards that are followed:
Thank you, |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
87c0275 to
3cf4957
Compare
103a8d6 to
8123116
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Thanks @jorgefilipecosta! I went through this against the companion plugin PR WordPress/ai#739. The Core class is a truthful representation of what is proposed there. After accounting for the expected adaptations (no namespace, no On sequencing: the AI plugin PR should go first. Since #739 is still open, we are matching a proposal, not merged code, so this PR should wait on that merge to avoid the two drifting. One small thing to add: the Trac ticket reference is missing. There should already be one for this ability, like the sibling PRs have (users links #64657 and settings links #64605). Could you add it to the description and the test |
peterwilsoncc
left a comment
There was a problem hiding this comment.
I've added a few notes inline. LMK if this is no longer the canonical URL as there are a few linked to the ticket.
| /** This filter is documented in wp-includes/post-template.php. */ | ||
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Applying the core content filter to mirror REST rendering. | ||
| $content = apply_filters( 'the_content', $post->post_content ); |
There was a problem hiding this comment.
I think the phpcs ignore will need to be on the same line so the docs parser recognises the docblock reference.
| /** This filter is documented in wp-includes/post-template.php. */ | |
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Applying the core content filter to mirror REST rendering. | |
| $content = apply_filters( 'the_content', $post->post_content ); | |
| /** This filter is documented in wp-includes/post-template.php. */ | |
| $content = apply_filters( 'the_content', $post->post_content ); // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Applying the core content filter to mirror REST rendering. |
| $response = $this->server->dispatch( $this->run_request( array( 'post_type' => 'post' ) ) ); | ||
| $data = $response->get_data(); | ||
|
|
||
| $this->assertSame( 200, $response->get_status() ); |
There was a problem hiding this comment.
This and other tests containing multiple assertions will need a unique $message param for each assertion to assist with debugging at a later date.
🔢 This applies to multiple tests but I won't repeat myself.
| ) | ||
| ); | ||
| } finally { | ||
| array_pop( $wp_current_filter ); |
There was a problem hiding this comment.
$wp_current_filter is restored as part of the default teardown so you can remove the finally array_pop pattern in Core.
🔢 This applies to other tests so I won't repeat myself.
See:
wordpress-develop/tests/phpunit/includes/abstract-testcase.php
Lines 416 to 432 in 74bb933
|
|
||
| $this->assertContains( $post_id, $ids, 'The custom post type should be queryable through the content ability.' ); | ||
| } finally { | ||
| unregister_post_type( 'wpai_content_cpt' ); |
There was a problem hiding this comment.
Post types set up by the test suite are automatically removed
🔢 Applies to other tests so I won't repeat myself
see
wordpress-develop/tests/phpunit/includes/abstract-testcase.php
Lines 336 to 350 in 74bb933
|
|
||
| $post_type_object = $exposed[ $post_type ]; | ||
| if ( $requires_edit ) { | ||
| return current_user_can( $this->post_type_cap( $post_type_object, 'edit_posts' ) ); // phpcs:ignore WordPress.WP.Capabilities.Undetermined -- Capability is resolved from the post type's capability object. |
There was a problem hiding this comment.
I'm unclear of the benefit $this->post_type_cap provides. If the post type doesn't exist or isn't exposed the permission check will fail above.
| // `orderby` is left unset, which orders by `post_date` descending, matching the | ||
| // default of the REST posts controller. |
There was a problem hiding this comment.
Multi line comment format -- https://developer.wordpress.org/coding-standards/inline-documentation-standards/php/#5-2-multi-line-comments
🔢 Applies in a few places so I won't repeat.
|
In addition to addressing the feedback from @peterwilsoncc, we need to sync the latest fixes applied in the AI plugin. Most importantly: |
core/content-query ability
Adds a read-only `core/content` ability that retrieves one or more posts of a post type exposed to abilities via a new `show_in_abilities` post type argument (enabled for `post` and `page` by default). Fetch a single post by ID or by slug, or query multiple posts filtered by post type, status, author, or parent, selecting a support-aware set of fields per post. Permissions follow the REST posts model: a coarse status/capability gate plus an authoritative per-post read_post check, with password-protected content withheld from users who cannot edit the post and a uniform not-found response to avoid leaking the existence of posts.
Mirrors the refinements from the core/settings review that also apply to core/content:
- Memoize the exposed post types so the input schema and the permission/execute
callbacks derive from a single walk of the registered post types.
- Default the input schema to an empty object so the type:object default serializes as {}.
- Harden input/value handling (type guards, a capability resolver, and a non-negative
integer helper) against loosely-typed request data.
Convert WP_Content_Abilities from a static class to a final, instance-based one, matching WP_Settings_Abilities and the canonical abilities pattern: register() is now invoked via ( new WP_Content_Abilities() )->register() from wp_register_core_abilities(). The externally-invoked entry points (register, check_permission, execute_get_content, return_raw_title_format) stay public; register_get_content() and the shared helpers become private; CATEGORY and the per-page bounds become private consts; FIELDS becomes a private instance property; and the cached exposed post types become instance state. Behaviour is unchanged. The per-page assertions in the test read the now-private constants by value.
… modes. Replace the flat anyOf(id|post_type) input schema with a oneOf of two modes, each with additionalProperties:false: - Get a single post by id (optionally guarded by post_type), plus fields. - Query a set of posts by post_type plus slug/status/author/parent/page/per_page, plus fields. Invalid combinations (e.g. per_page alongside id) now fail validation instead of being silently ignored. Update wpRegisterCoreContentAbility accordingly and add coverage for the id-mode rejecting query-only params and accepting a post_type guard.
The `list<string>` type of `$fields` is wider than the other types, so the `@param` tags of format_post() and build_post_fields() were misaligned. This matches WordPress/wordpress-develop#12195.
WordPress/wordpress-develop#12195 dropped the `open_world` annotation from core/content-query, because WP_Ability only documents the `readonly`, `destructive`, and `idempotent` annotations. The plugin keeps it for MCP clients, which assume open-world when the hint is absent, so the comment now marks it as a plugin difference.
Requesting `link` for a hierarchical post type loaded the parent of each returned page with its own query, because page permalinks walk the ancestors through `get_page_uri()`. Inherited read permissions read the parent too. The REST posts controller primes both the parent and the author caches, but query mode, whose comment says it mirrors that controller, only primed the authors. Query mode now calls `update_post_parent_caches()` as well, which runs no query when no returned post has a parent. A new test requests `link` for three child pages and expects three queries against the posts table. Before, each parent took its own query, for five in total.
…xcerpts `format_post()` unlocks a password-protected post for a user who can edit it. The excerpt test set an explicit `post_excerpt`, which `wp_trim_excerpt()` returns as is, so it passed even without the bypass. The post now has no excerpt, so it is generated from the content, which `get_the_content()` replaces with the password form unless the post is unlocked.
- Use the `set_up()` and `tear_down()` fixtures instead of PHPUnit's `setUp()` and `tearDown()`. - Drop teardown steps the test case already takes care of: its `set_up()` registers the built-in post types again, and its `tear_down()` logs the user out. - Drop the `wpai` prefix, which came from the AI plugin, from the post types, statuses, slugs, and nicenames the tests create. - Drop the leading backslashes from class names, since the tests are not namespaced. - Align the `@param` tags of `replace_cached_post_date_columns()`. - Use the `restapi` group, like the other REST API tests.
- Drop the leading backslashes from class names, since the class is not namespaced. - Make the field lists and the loop globals class constants, like `CATEGORY` and `MAX_PER_PAGE`, since they never change. - Label the ability "Query Content", so that it starts with a verb, like "Get Site Information".
- Resolve ID and slug requests with one `get_requested_post()` helper, which replaces `get_content_by_id()`. A request with an `id` or a `slug` now always takes the single-post path, so one that does not resolve fails closed, instead of falling through to query mode, when the callbacks run on input that skipped schema validation. - Parse `page` and `per_page` with `parse_filter_int()`, like `id` and `parent`, and drop `input_int()`. Values that skipped schema validation and are not positive integers now fall back to the defaults. - Read GMT dates with `get_post_datetime()` and convert the result back to UTC, which replaces `is_usable_date()` and the `$field` normalization. - Pick the post a slug resolves to in one loop. - Look up author slugs with `get_user_by()`. Core keeps nicenames unique, so the check for users sharing one, and its test case, are gone. - Copy the loop globals with `array_intersect_key()`. - Pass the query posts to `update_post_author_caches()` directly, like the REST posts controller does. - Drop the self-parent check from the inherited read permission, since the cycle guard already rejects a post that is its own parent.
The curated post types map documented its values as `true` or an array "reserved for enabling specific operations in the future". No such operations exist, so any non-empty array exposes the post type in full: core/content-query checks the flag with empty(). Giving arrays a per-operation meaning later would change the behavior of any plugin passing one today. The map now documents its values as booleans, as `show_in_rest` is for post types, matching WordPress/wordpress-develop#12195.
Requesting `link` for a hierarchical post type loaded the parent of each returned page with its own query, because page permalinks walk the ancestors through get_page_uri(). Inherited read permissions read the parent too. The REST posts controller primes both the parent and the author caches, but query mode, whose comment says it mirrors that controller, only primed the authors. Query mode now calls update_post_parent_caches() as well, which runs no query when no returned post has a parent, as in WordPress/wordpress-develop#12195. Core passes it `$query->posts`; the WordPress stubs type that as post objects or IDs, so the plugin passes the post objects it already filtered for update_post_author_caches(), and a Plugin: comment marks the difference. A new test requests `link` for three child pages and expects three queries against the posts table.
format_post() unlocks a password-protected post for a user who can edit it. The excerpt test set an explicit `post_excerpt`, which wp_trim_excerpt() returns as is, so it passed even without the bypass. The post now has no excerpt, so it is generated from the content, which get_the_content() replaces with the password form unless the post is unlocked. This matches WordPress/wordpress-develop#12195.
The `array<string, mixed>` type of `$columns` is wider than `int`, so the tags were misaligned. This matches WordPress/wordpress-develop#12195. The rest of that commit moves core's tests to core's conventions, which the plugin's namespaced tests do not share: they use PHPUnit's setUp() and tearDown() like every other plugin test, keep the `wpai` prefix and the leading backslashes, and their tearDown() removes the flag the polyfill sets on `post` and `page`.
The edit fields, the cache priming fields, the default fields, and the loop globals never change, so they are now class constants, like CATEGORY and MAX_PER_PAGE, as in WordPress/wordpress-develop#12195. Like the plugin's other array constants, they carry a phpcs:ignore for Slevomat's multi-constant sniff, which reads the array items as separate constants. Core's commit also drops the leading backslashes from class names, which the plugin's namespaced class needs.
Ports the simplifications of WordPress/wordpress-develop#12195: - Resolve ID and slug requests with one get_requested_post() helper. A request with an `id` or a `slug` now always takes the single-post path, so one that does not resolve fails closed, instead of falling through to query mode, when the callbacks run on input that skipped schema validation. - Parse `page` and `per_page` with parse_filter_int(), like `id` and `parent`, and drop input_int(). Values that skipped schema validation and are not positive integers now fall back to the defaults. - Read GMT dates with get_post_datetime() and convert the result back to UTC, which replaces is_usable_date() and the `$field` normalization. - Pick the post a slug resolves to in one loop. - Look up author slugs with get_user_by(). Core keeps nicenames unique, so the check for users sharing one, and its test case, are gone. The write abilities' `author_slug` uses the same lookup. - Copy the loop globals with array_intersect_key(). - Drop the self-parent check from the inherited read permission, since the cycle guard already rejects a post that is its own parent. The write abilities keep get_content_by_id(), now a wrapper that only resolves requests with an `id`: their `slug` is the slug to write, not one to look the post up by. The update's date check, the other user of is_usable_date(), now compares the stored GMT date unless it is the zero date, as the REST posts controller does. Where the plugin's tools need more than core's code, the plugin differs, with Plugin: comments: the query posts are filtered to post objects before they prime the caches and in the slug loop, for PHPStan, and the slug loop ends with an early exit, as the plugin's coding standards ask. A @phpstan-param tag types the date formatters' `$field` as 'date' or 'modified', which the removed normalization guaranteed.
The label now starts with a verb, like "Get Site Information", as in WordPress/wordpress-develop#12195. The deprecated core/read-content alias takes its label from it, so it becomes "Query Content (deprecated)".
get_title() returns an empty string when a `the_title` filter returns a non-string, instead of failing its string return type and turning the query into an `ability_callback_exception`. The guard came from this plugin, but only WordPress/wordpress-develop#12195 tested it. This adds that test.
format_post() now sets the post up as the global post while it builds all of the post's fields, as the REST posts controller does, and restores the previous context in the same finally block that removes the password filter. Title filters now see the requested post too, and requesting both rendered fields no longer sets the context up twice. Every post now goes through setup_postdata(), which reads the author, so the author caches are primed for every page, as the REST posts controller does. Before, rendered fields ran one user query per author.
Schema validation accepts a single integer for `include`, because wp_parse_list() handles integers, but normalize_include() only took arrays and strings, so the query failed with `content_invalid_filter`.
- Build the `content_invalid_filter` errors with one helper. - Keep the `author_slug` parameter name out of the translatable strings. - Drop a check from the total page count that `ceil()` already covers, since it returns 0 for an empty result.
Notes the `content` category in wp_register_core_ability_categories() and the `core/content-query` ability in wp_register_core_abilities().
A test turns off `show_in_abilities` on `post` and `page`. When it is the last test of its class to run, the next class's set_up_before_class() runs before the next set_up() resets the post types, so a class that registers the core abilities there, such as Tests_REST_API_WpRestAbilitiesContentController, registers no content ability, and all of its tests fail. An earlier cleanup dropped this reset, so a comment now explains why it stays.
- Assert that the post type objects exist. get_post_type_object() returns null, not false, so assertNotFalse() always passed. - Merge the post fixtures that only differed in their text, and drop three tests that other tests already cover. - Use REST_TESTS_IMPOSSIBLY_HIGH_NUMBER for posts that do not exist.
The content query ability has its own ticket, #66268. The tests used #64606, the earlier ticket for post management abilities.
set_up_post_context() saved the loop globals with array_intersect_key( $GLOBALS, ... ), which keeps a global as a reference when a calling function binds it with `global`, as load_template() and WP_Block::render() do. Setting up the requested post then changed the saved copy as well, so the restore left the requested post in `$post` and `$id` while the other loop globals went back to the surrounding post. In a classic theme, the rest of the template, including the comment form, then used the wrong post.
format_post() sets each post up as the global post, and query mode primes the authors that setup_postdata() reads. Test that title and permalink filters see the requested post when no post was set up before, as in a REST request, and that rendered fields read primed authors instead of querying once per author.
Ports the latest changes of WordPress/wordpress-develop#12195: - format_post() now sets the post up as the global post while it builds all of the post's fields, as the REST posts controller does, and restores the previous context in the same finally block that removes the password filter. Title and permalink filters now see the requested post too, and requesting both rendered fields no longer sets the context up twice. - Every post now goes through setup_postdata(), which reads the author, so the author caches are primed for every page. Before, rendered fields ran one user query per author. - set_up_post_context() saves the loop globals by value. array_intersect_key() kept a global as a reference when a calling function binds it with `global`, as load_template() and WP_Block::render() do, so the restore left the requested post in `$post` and `$id`. - The include filter accepts a single ID, as schema validation does. Before, the query failed with `content_invalid_filter`. - One helper builds the `content_invalid_filter` errors, the `author_slug` parameter name stays out of the translatable strings, and the total page count drops a check that ceil() already covers. The tests drop three that other tests already cover, merge the post fixtures that only differed in their text, assert that the post type objects exist, since get_post_type_object() returns null rather than false, and use REST_TESTS_IMPOSSIBLY_HIGH_NUMBER for posts that do not exist. New tests cover a single include ID, title and permalink filters, globals bound by the caller, and author priming. The plugin wraps a single include ID in an array before passing it to wp_parse_id_list(), with a Plugin: comment: that function only supports an integer since WordPress 7.2, and the WordPress stubs type its input as an array or a string.
…t query The integer schema also accepts whole floats such as 2.0 and strings such as "2.0" or "+2", and callers other than the REST run controller, such as the MCP adapter, pass them on unconverted. The content query read `page` and `per_page` with parse_filter_int(), which rejects them, so such a request silently returned page 1 at the default page size: a client paging with 2.0, 3.0, and so on never got past page 1 or reached the error for a page past the last one. Read both with absint() again, as the REST posts controller does.
restore_post_context() unset the loop globals that were not set before, even after setup_postdata() had set up the previous global post again. A global post that was assigned but never set up, such as the main post before the loop starts, then had no `$pages` after `the_post` had fired, so get_the_content() without a post, as the Post Content block and the_content() outside the loop call it, threw a TypeError. Such a post now keeps the loop globals that setup_postdata() gives it.
peterwilsoncc
left a comment
There was a problem hiding this comment.
Added a few more notes inline. Sorry, this is big so it's easy (for me anyway) to miss things on a first pass.
| * @return bool True if edit-context fields were explicitly requested. | ||
| */ | ||
| private function has_explicit_edit_fields( array $input ): bool { | ||
| return array() !== array_intersect( self::EDIT_FIELDS, $this->parse_list_input( $input, 'fields' ) ); |
| return new WP_Error( | ||
| 'content_invalid_page_number', | ||
| __( 'The page number requested is larger than the number of pages available.' ), | ||
| array( 'status' => 400 ) |
There was a problem hiding this comment.
File not found seems more appropriate here. The request isn't malformed per se.
This isn't a hill I will die on.
| array( 'status' => 400 ) | |
| array( 'status' => 404 ) |
| private function get_query_total( WP_Query $query, array $query_args, int $page ): int { | ||
| $total = (int) $query->found_posts; | ||
|
|
||
| if ( $total > 0 || $page <= 1 ) { |
There was a problem hiding this comment.
A $total of zero is valid and should not trigger a second DB query.
| 'type' => 'integer', | ||
| 'description' => __( 'The post ID.' ), | ||
| ), | ||
| 'post_type' => array( |
There was a problem hiding this comment.
This is the only property with the prefix post_.
Maybe... let me know what the reasoning is if it's a choice rather than an oversight.
| 'post_type' => array( | |
| 'type' => array( |
| $this->assertSame( 'shadowed-slug', get_post( $draft_id )->post_name, 'Precondition: the draft should share the published slug.' ); | ||
| $this->register_ability(); | ||
|
|
||
| foreach ( array( 'subscriber', 'editor' ) as $role ) { |
There was a problem hiding this comment.
A data provider would be better here. If it's failing for both subscribers and editors it would be best to know that in a single test run.
| ) | ||
| ); | ||
| } finally { | ||
| remove_action( 'pre_get_posts', $spy ); |
There was a problem hiding this comment.
Not required. You can remove the try...finally
| ); | ||
|
|
||
| $this->assertNotEmpty( $result['posts'], 'Posts with an empty field projection should still be returned.' ); | ||
| $this->assertSame( count( $result['posts'] ), $result['total'], 'The reported total should match the returned posts when projections are empty.' ); |
| * | ||
| * @since 7.2.0 | ||
| */ | ||
| public static function set_up_before_class(): void { |
There was a problem hiding this comment.
Any reason you can't use wpSetupBeforeClass and wpTeardownAfterClass for these? I'm not sure if there's a coding standard but it's a heavily used convention.
Part of WordPress/ai#40. Ports the
core/content-queryability from the AI plugin, added in WordPress/ai#739, renamed fromcore/read-contentin WordPress/ai#1002, and updated with the read changes from WordPress/ai#1025.core/content-queryability, in a newcontentcategory, through the internalWP_Content_Abilitiesclass.show_in_abilitiespost type argument that controls which post types the ability can read. It defaults tofalse;postandpageset it totrue.id, or bypost_typeandslug, or a page of posts of one type filtered bystatus,author_slug,parent, orinclude, withfieldsto choose what each post returns.publishneed edit access, orread_private_postsforprivate.Testing Instructions
Verify unit tests are passing:
Trac ticket: https://core.trac.wordpress.org/ticket/66268
Use of AI Tools
AI assistance: Yes. Claude Code was used to sync this PR with the plugin and apply the review feedback; I reviewed the changes.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.