Clarify parameters for callbacks for `register_rest_field()`

[flixos90]

There is an issue going on with register_rest_field() in combination with the WP_REST_Meta_Fields classes: The method WP_REST_Meta_Fields::register_field() uses callbacks that accept an object ID only. However, the REST controller methods that handle the additional fields always pass an object to these callbacks.

Working with metadata via the REST API is still possible now, since the WP_REST_Meta_Fields::register_field() method is never used in Core, instead metadata is handled manually by each controller. So it is now a minor issue, but I think we should figure out what kind of parameters the register_rest_field() callbacks can accept (I could not find anything clear in the documentation) and then actually use WP_REST_Meta_Fields::register_field(), as I don't think there's a point in handling meta manually if we have this method already available in a more abstracted way.

Furthermore, this inconsistency gets more critical once developers start writing their own endpoints for custom object types that use their own metadata table. One might assume that the WP_REST_Meta_Fields base class can be leveraged to handle meta automatically, but because of the inconsistencies now, doing this would likely cause fatal errors and confusion.

[flixos90]

To give a precise example: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/rest-api/endpoints/class-wp-rest-controller.php#L432 passes an object to the update_callback passed to register_rest_field(), but https://core.trac.wordpress.org/browser/trunk/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php#L124 which is passed to register_rest_field() as an update_callback accepts an object ID only. So which of the two is right? Should register_rest_field() callbacks accept an object or an ID?

[kadamwhite]

My sense is that they should be allowed to accept objects, for consistency with what we're doing now; but other parts of WP generally pass IDs. @rmccue Can you speak to why this class method is not being leveraged by the native endpoints?

[jnylen0]

Replying to flixos90:

So which of the two is right? Should register_rest_field() callbacks accept an object or an ID?

They should accept an object. See the first code example at ​https://developer.wordpress.org/rest-api/extending-the-rest-api/modifying-responses/.

It sounds like the unused WP_REST_Meta_Fields::register_field() method was never tested, and it should be updated to accept an object. Also, we need to improve the register_rest_field documentation to indicate the types/signatures of the values accepted by its $args argument.

[TimothyBlynJacobs]

Going to take a look at this and how it relates to #44432.

[TimothyBlynJacobs]

I think it makes sense to deprecate WP_REST_Meta_Fields::register_field() because we can't really go from an object to an id value cleanly.

We could update the update_callback to do something like this...

register_rest_field(
        $this->get_rest_field_type(),
        'meta',
        array(
                'get_callback'    => function( $prepared, $field_name, $request ) {
                        return $this->get_value( $prepared['id'], $request );
                },
                'update_callback' => function( $data, $object ) {
                        if ( $object instanceof WP_Post ) {
                                $id = $object->ID;
                        } elseif ( $object instanceof WP_User ) {
                                $id = $object->ID;
                        } elseif ( $object instanceof WP_Comment ) {
                                $id = $object->comment_ID;
                        } elseif ( $object instanceof WP_Term ) {
                                $id = $object->term_id;
                        } else {
                                // now what?
                        }

                        return $this->update_value( $data, $id );
                },
                'schema'          => $this->get_field_schema(),
        )
);

But that only works for built in objects which wouldn't use register_rest_field anyways. And doesn't do anything for non-core meta types that might want to actually leverage the register_rest_field.

There is also no guarantee for the get_callback that the id property exists in the response data for the custom endpoint.

So from my perspective, the current API methods are the most flexible way to interact with the meta fields helper and trying to make register_field work will end up being less clean than the current API.

[TimothyBlynJacobs]

In 49295:

REST API: Deprecate WP_REST_Meta_Fields::register_field().

This method never worked properly and cannot be fixed due to incompatible method signatures.

Props flixos90, kadamwhite, jnylen0, TimothyBlynJacobs.
Fixes #39959.