Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement forward-compatible REST API search controller #7894

Merged
merged 11 commits into from
Jul 24, 2018
Merged
96 changes: 96 additions & 0 deletions lib/class-wp-rest-object-search-handler.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
<?php
/**
* REST API: WP_REST_Object_Search_Handler class
*
* @package gutenberg
* @since 3.3.0
*/

/**
* Core base class representing a search handler for an object type in the REST API.
*
* @since 3.3.0
*/
abstract class WP_REST_Object_Search_Handler {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match the naming scheme of other abstract classes in the REST API WP_REST_Meta_Fields, WP_REST_Controller, the class name should just be WP_REST_Search_Handler, the implementing classes would follow the naming scheme you uses for WP_REST_Post_Search_Handler.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me!


/**
* Field containing the IDs in the search result.
*/
const RESULT_IDS = 'ids';

/**
* Field containing the total count in the search result.
*/
const RESULT_TOTAL = 'total';

/**
* Object type managed by this search handler.
*
* @since 3.3.0
* @var string
*/
protected $type = '';

/**
* Object subtypes managed by this search handler.
*
* @since 3.3.0
* @var array
*/
protected $subtypes = array();

/**
* Gets the object type managed by this search handler.
*
* @since 3.3.0
*
* @return string Object type identifier.
*/
public function get_type() {
return $this->type;
}

/**
* Gets the object subtypes managed by this search handler.
*
* @since 3.3.0
*
* @return array Array of object subtype identifiers.
*/
public function get_subtypes() {
return $this->subtypes;
}

/**
* Searches the object type content for a given search request.
*
* @since 3.3.0
*
* @param WP_REST_Request $request Full REST request.
* @return array Associative array containing an `WP_REST_Object_Search_Handler::RESULT_IDS` containing
* an array of found IDs and `WP_REST_Object_Search_Handler::RESULT_TOTAL` containing the
* total count for the matching search results.
*/
abstract public function search_items( WP_REST_Request $request );

/**
* Prepares the search result for a given ID.
*
* @since 3.3.0
*
* @param int $id Item ID.
* @param array $fields Fields to include for the item.
* @return array Associative array containing all fields for the item.
*/
abstract public function prepare_item( $id, array $fields );

/**
* Prepares links for the search result of a given ID.
*
* @since 3.3.0
*
* @param int $id Item ID.
* @return array Links for the given item.
*/
abstract public function prepare_item_links( $id );
}
191 changes: 191 additions & 0 deletions lib/class-wp-rest-post-search-handler.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
<?php
/**
* REST API: WP_REST_Post_Search_Handler class
*
* @package gutenberg
* @since 3.3.0
*/

/**
* Core class representing a search handler for posts in the REST API.
*
* @since 3.3.0
*/
class WP_REST_Post_Search_Handler extends WP_REST_Object_Search_Handler {

/**
* Constructor.
*
* @since 3.3.0
*/
public function __construct() {
$this->type = 'post';
$this->subtypes = array_values( get_post_types( array(
'public' => true,
'show_in_rest' => true,
), 'names' ) );
}

/**
* Searches the object type content for a given search request.
*
* @since 3.3.0
*
* @param WP_REST_Request $request Full REST request.
* @return array Associative array containing an `WP_REST_Object_Search_Handler::RESULT_IDS` containing
* an array of found IDs and `WP_REST_Object_Search_Handler::RESULT_TOTAL` containing the
* total count for the matching search results.
*/
public function search_items( WP_REST_Request $request ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core doesn't typically type cast.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but I don't see why this cannot change. To be fair, there are a few areas in core where type hints are present, and them being available (where possible in PHP 5.2) ensures that the parameter is valid.

I'm not strongly opposed to changing it, but only if there's other arguments/opinions against it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a handful of uses in Core, I have no problem with adding this one.

(Run ack --php 'ack --php '(?:^|\s)function [^(]+\([^)]*?(\S+[^(,] \$[^ ),]+)'' on Core for examples.)


// Get the post types to search for the current request.
$post_types = $request[ WP_REST_Search_Controller::PROP_SUBTYPE ];
if ( in_array( WP_REST_Search_Controller::TYPE_ANY, $post_types, true ) ) {
$post_types = $this->subtypes;
}

// Get the public post statuses as only those should be searched.
$post_statuses = array_values( get_post_stati( array(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For parity with core, we should simply use post_status=>publish. We can discuss custom statuses at greater depth with #3144

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not fully convinced why as the REST API handles post statuses like that in core already, but I'm okay simplifying it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not fully convinced why as the REST API handles post statuses like that in core already, but I'm okay simplifying it.

Oh. If that's the case, I'm fine with it as-is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I decided to follow your suggestion and only use publish for now. The only problem that brings is that the controller won't be able to search attachments at this point because they typically use inherit. Is it acceptable not having that in the first iteration? Pinging @pento for an opinion on this too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because searching for attachments will always return an empty array for now (which is unexpected), I explicitly removed support for type=>post and subtype=attachment. Once we add support, we can allow that again.

'public' => true,
'internal' => false,
), 'names' ) );

$query_args = array(
'post_type' => $post_types,
'post_status' => $post_statuses,
'paged' => (int) $request['page'],
'posts_per_page' => (int) $request['per_page'],
'orderby' => 'ID',
'order' => 'DESC',
'ignore_sticky_posts' => true,
'fields' => 'ids',
);

// If a search term is given, add it and order by relevance.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we default to ID and then switch to relevance?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When search is not given, it is impossible to sort by relevance, so we need to have some default.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When search is not given, it is impossible to sort by relevance, so we need to have some default.

Makes sense. It'd be great to have this clarified in the comment.

if ( ! empty( $request['search'] ) ) {
$query_args['s'] = $request['search'];
$query_args['orderby'] = 'relevance';
$query_args['order'] = 'DESC';
}

$query = new WP_Query();
$found_ids = $query->query( $query_args );
$total = $query->found_posts;

return array(
WP_REST_Object_Search_Handler::RESULT_IDS => $found_ids,
WP_REST_Object_Search_Handler::RESULT_TOTAL => $total,
);
}

/**
* Prepares the search result for a given ID.
*
* @since 3.3.0
*
* @param int $id Item ID.
* @param array $fields Fields to include for the item.
* @return array Associative array containing all fields for the item.
*/
public function prepare_item( $id, array $fields ) {
$post = get_post( $id );

$data = array();

if ( in_array( WP_REST_Search_Controller::PROP_ID, $fields, true ) ) {
$data[ WP_REST_Search_Controller::PROP_ID ] = (int) $post->ID;
}

if ( in_array( WP_REST_Search_Controller::PROP_TITLE, $fields, true ) ) {
if ( post_type_supports( $post->post_type, 'title' ) ) {
add_filter( 'protected_title_format', array( $this, 'protected_title_format' ) );
$data[ WP_REST_Search_Controller::PROP_TITLE ] = get_the_title( $post->ID );
remove_filter( 'protected_title_format', array( $this, 'protected_title_format' ) );
} else {
$data[ WP_REST_Search_Controller::PROP_TITLE ] = '';
}
}

if ( in_array( WP_REST_Search_Controller::PROP_URL, $fields, true ) ) {
$data[ WP_REST_Search_Controller::PROP_URL ] = get_permalink( $post->ID );
}

if ( in_array( WP_REST_Search_Controller::PROP_TYPE, $fields, true ) ) {
$data[ WP_REST_Search_Controller::PROP_TYPE ] = $this->type;
}

if ( in_array( WP_REST_Search_Controller::PROP_SUBTYPE, $fields, true ) ) {
$data[ WP_REST_Search_Controller::PROP_SUBTYPE ] = $post->post_type;
}

return $data;
}

/**
* Prepares links for the search result of a given ID.
*
* @since 3.3.0
*
* @param int $id Item ID.
* @return array Links for the given item.
*/
public function prepare_item_links( $id ) {
$post = get_post( $id );

$links = array();

$item_route = $this->detect_rest_item_route( $post );
if ( ! empty( $item_route ) ) {
$links['self'] = array(
'href' => rest_url( $item_route ),
'embeddable' => true,
);
}

$links['about'] = array(
'href' => rest_url( 'wp/v2/types/' . $post->post_type ),
);

return $links;
}

/**
* Overwrites the default protected title format.
*
* By default, WordPress will show password protected posts with a title of
* "Protected: %s". As the REST API communicates the protected status of a post
* in a machine readable format, we remove the "Protected: " prefix.
*
* @since 3.3.0
*
* @return string Protected title format.
*/
public function protected_title_format() {
return '%s';
}

/**
* Attempts to detect the route to access a single item.
*
* @since 3.3.0
*
* @param WP_Post $post Post object.
* @return string REST route relative to the REST base URI, or empty string if unknown.
*/
protected function detect_rest_item_route( $post ) {
$post_type = get_post_type_object( $post->post_type );
if ( ! $post_type ) {
return '';
}

// It's currently impossible to detect the REST URL from a custom controller.
if ( ! empty( $post_type->rest_controller_class ) && 'WP_REST_Posts_Controller' !== $post_type->rest_controller_class ) {
return '';
}

$namespace = 'wp/v2';
$rest_base = ! empty( $post_type->rest_base ) ? $post_type->rest_base : $post_type->name;

return sprintf( '%s/%s/%d', $namespace, $rest_base, $post->ID );
}
}
Loading