From 096f23db6546c405c2dc11ea91bcffac829db2fc Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Mon, 21 Nov 2016 05:40:46 +0000 Subject: [PATCH] REST API: Correctly map meta keys to field names. This accidentally assumed $name was the same as $meta_key, which ruined the whole point of $name. Props tharsheblows, joehoyle. Fixes #38786. git-svn-id: https://develop.svn.wordpress.org/trunk@39328 602fd350-edb4-49c9-b593-d223f7449a82 --- .../fields/class-wp-rest-meta-fields.php | 51 +++---- .../tests/rest-api/rest-post-meta-fields.php | 127 ++++++++++++++++++ 2 files changed, 155 insertions(+), 23 deletions(-) diff --git a/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php b/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php index 372b233a2b..e19a8129ea 100644 --- a/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php +++ b/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php @@ -65,8 +65,9 @@ abstract class WP_REST_Meta_Fields { $fields = $this->get_registered_fields(); $response = array(); - foreach ( $fields as $name => $args ) { - $all_values = get_metadata( $this->get_meta_type(), $object_id, $name, false ); + foreach ( $fields as $meta_key => $args ) { + $name = $args['name']; + $all_values = get_metadata( $this->get_meta_type(), $object_id, $meta_key, false ); if ( $args['single'] ) { if ( empty( $all_values ) ) { $value = $args['schema']['default']; @@ -122,7 +123,8 @@ abstract class WP_REST_Meta_Fields { */ public function update_value( $request, $object_id ) { $fields = $this->get_registered_fields(); - foreach ( $fields as $name => $args ) { + foreach ( $fields as $meta_key => $args ) { + $name = $args['name']; if ( ! array_key_exists( $name, $request ) ) { continue; } @@ -132,7 +134,7 @@ abstract class WP_REST_Meta_Fields { * from the database and then relying on the default value. */ if ( is_null( $request[ $name ] ) ) { - $result = $this->delete_meta_value( $object_id, $name ); + $result = $this->delete_meta_value( $object_id, $meta_key, $name ); if ( is_wp_error( $result ) ) { return $result; } @@ -148,9 +150,9 @@ abstract class WP_REST_Meta_Fields { $value = rest_sanitize_value_from_schema( $request[ $name ], $args['schema'] ); if ( $args['single'] ) { - $result = $this->update_meta_value( $object_id, $name, $value ); + $result = $this->update_meta_value( $object_id, $meta_key, $name, $value ); } else { - $result = $this->update_multi_meta_value( $object_id, $name, $value ); + $result = $this->update_multi_meta_value( $object_id, $meta_key, $name, $value ); } if ( is_wp_error( $result ) ) { @@ -168,12 +170,13 @@ abstract class WP_REST_Meta_Fields { * @access protected * * @param int $object_id Object ID the field belongs to. - * @param string $name Key for the field. + * @param string $meta_key Key for the field. + * @param string $name Name for the field that is exposed in the REST API. * @return bool|WP_Error True if meta field is deleted, WP_Error otherwise. */ - protected function delete_meta_value( $object_id, $name ) { + protected function delete_meta_value( $object_id, $meta_key, $name ) { $meta_type = $this->get_meta_type(); - if ( ! current_user_can( "delete_{$meta_type}_meta", $object_id, $name ) ) { + if ( ! current_user_can( "delete_{$meta_type}_meta", $object_id, $meta_key ) ) { return new WP_Error( 'rest_cannot_delete', /* translators: %s: custom field key */ @@ -182,7 +185,7 @@ abstract class WP_REST_Meta_Fields { ); } - if ( ! delete_metadata( $meta_type, $object_id, wp_slash( $name ) ) ) { + if ( ! delete_metadata( $meta_type, $object_id, wp_slash( $meta_key ) ) ) { return new WP_Error( 'rest_meta_database_error', __( 'Could not delete meta value from database.' ), @@ -202,13 +205,14 @@ abstract class WP_REST_Meta_Fields { * @access protected * * @param int $object_id Object ID to update. - * @param string $name Key for the custom field. + * @param string $meta_key Key for the custom field. + * @param string $name Name for the field that is exposed in the REST API. * @param array $values List of values to update to. * @return bool|WP_Error True if meta fields are updated, WP_Error otherwise. */ - protected function update_multi_meta_value( $object_id, $name, $values ) { + protected function update_multi_meta_value( $object_id, $meta_key, $name, $values ) { $meta_type = $this->get_meta_type(); - if ( ! current_user_can( "edit_{$meta_type}_meta", $object_id, $name ) ) { + if ( ! current_user_can( "edit_{$meta_type}_meta", $object_id, $meta_key ) ) { return new WP_Error( 'rest_cannot_update', /* translators: %s: custom field key */ @@ -217,7 +221,7 @@ abstract class WP_REST_Meta_Fields { ); } - $current = get_metadata( $meta_type, $object_id, $name, false ); + $current = get_metadata( $meta_type, $object_id, $meta_key, false ); $to_remove = $current; $to_add = $values; @@ -244,7 +248,7 @@ abstract class WP_REST_Meta_Fields { $to_remove = array_unique( $to_remove ); foreach ( $to_remove as $value ) { - if ( ! delete_metadata( $meta_type, $object_id, wp_slash( $name ), wp_slash( $value ) ) ) { + if ( ! delete_metadata( $meta_type, $object_id, wp_slash( $meta_key ), wp_slash( $value ) ) ) { return new WP_Error( 'rest_meta_database_error', __( 'Could not update meta value in database.' ), @@ -254,7 +258,7 @@ abstract class WP_REST_Meta_Fields { } foreach ( $to_add as $value ) { - if ( ! add_metadata( $meta_type, $object_id, wp_slash( $name ), wp_slash( $value ) ) ) { + if ( ! add_metadata( $meta_type, $object_id, wp_slash( $meta_key ), wp_slash( $value ) ) ) { return new WP_Error( 'rest_meta_database_error', __( 'Could not update meta value in database.' ), @@ -273,13 +277,14 @@ abstract class WP_REST_Meta_Fields { * @access protected * * @param int $object_id Object ID to update. - * @param string $name Key for the custom field. + * @param string $meta_key Key for the custom field. + * @param string $name Name for the field that is exposed in the REST API. * @param mixed $value Updated value. * @return bool|WP_Error True if the meta field was updated, WP_Error otherwise. */ - protected function update_meta_value( $object_id, $name, $value ) { + protected function update_meta_value( $object_id, $meta_key, $name, $value ) { $meta_type = $this->get_meta_type(); - if ( ! current_user_can( "edit_{$meta_type}_meta", $object_id, $name ) ) { + if ( ! current_user_can( "edit_{$meta_type}_meta", $object_id, $meta_key ) ) { return new WP_Error( 'rest_cannot_update', /* translators: %s: custom field key */ @@ -288,7 +293,7 @@ abstract class WP_REST_Meta_Fields { ); } - $meta_key = wp_slash( $name ); + $meta_key = wp_slash( $meta_key ); $meta_value = wp_slash( $value ); // Do the exact same check for a duplicate value as in update_metadata() to avoid update_metadata() returning false. @@ -364,7 +369,7 @@ abstract class WP_REST_Meta_Fields { $rest_args['schema']['type'] = 'array'; } - $registered[ $rest_args['name'] ] = $rest_args; + $registered[ $name ] = $rest_args; } return $registered; @@ -388,8 +393,8 @@ abstract class WP_REST_Meta_Fields { 'properties' => array(), ); - foreach ( $fields as $key => $args ) { - $schema['properties'][ $key ] = $args['schema']; + foreach ( $fields as $args ) { + $schema['properties'][ $args['name'] ] = $args['schema']; } return $schema; diff --git a/tests/phpunit/tests/rest-api/rest-post-meta-fields.php b/tests/phpunit/tests/rest-api/rest-post-meta-fields.php index 98a95e9783..6684cad95e 100644 --- a/tests/phpunit/tests/rest-api/rest-post-meta-fields.php +++ b/tests/phpunit/tests/rest-api/rest-post-meta-fields.php @@ -79,6 +79,22 @@ class WP_Test_REST_Post_Meta_Fields extends WP_Test_REST_TestCase { 'show_in_rest' => true, )); + register_meta( 'post', 'test_custom_name', array( + 'single' => true, + 'type' => 'string', + 'show_in_rest' => array( + 'name' => 'new_name', + ), + )); + + register_meta( 'post', 'test_custom_name_multi', array( + 'single' => false, + 'type' => 'string', + 'show_in_rest' => array( + 'name' => 'new_name_multi', + ), + )); + /** @var WP_REST_Server $wp_rest_server */ global $wp_rest_server; $this->server = $wp_rest_server = new Spy_REST_Server; @@ -227,6 +243,22 @@ class WP_Test_REST_Post_Meta_Fields extends WP_Test_REST_TestCase { $this->assertSame( true, $meta['test_bool'] ); } + public function test_get_value_custom_name() { + add_post_meta( self::$post_id, 'test_custom_name', 'janet' ); + + $request = new WP_REST_Request( 'GET', sprintf( '/wp/v2/posts/%d', self::$post_id ) ); + $response = $this->server->dispatch( $request ); + + $this->assertEquals( 200, $response->get_status() ); + + $data = $response->get_data(); + $this->assertArrayHasKey( 'meta', $data ); + + $meta = (array) $data['meta']; + $this->assertArrayHasKey( 'new_name', $meta ); + $this->assertEquals( 'janet', $meta['new_name'] ); + } + /** * @depends test_get_value */ @@ -681,6 +713,79 @@ class WP_Test_REST_Post_Meta_Fields extends WP_Test_REST_TestCase { $this->assertContains( 8, $meta ); } + /** + * @depends test_get_value_custom_name + */ + public function test_set_value_custom_name() { + // Ensure no data exists currently. + $values = get_post_meta( self::$post_id, 'test_custom_name', false ); + $this->assertEmpty( $values ); + + $this->grant_write_permission(); + + $data = array( + 'meta' => array( + 'new_name' => 'janet', + ), + ); + $request = new WP_REST_Request( 'POST', sprintf( '/wp/v2/posts/%d', self::$post_id ) ); + $request->set_body_params( $data ); + + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + + $meta = get_post_meta( self::$post_id, 'test_custom_name', false ); + $this->assertNotEmpty( $meta ); + $this->assertCount( 1, $meta ); + $this->assertEquals( 'janet', $meta[0] ); + + $data = $response->get_data(); + $meta = (array) $data['meta']; + $this->assertArrayHasKey( 'new_name', $meta ); + $this->assertEquals( 'janet', $meta['new_name'] ); + } + + public function test_set_value_custom_name_multiple() { + // Ensure no data exists currently. + $values = get_post_meta( self::$post_id, 'test_custom_name_multi', false ); + $this->assertEmpty( $values ); + + $this->grant_write_permission(); + + $data = array( + 'meta' => array( + 'new_name_multi' => array( 'janet' ), + ), + ); + $request = new WP_REST_Request( 'POST', sprintf( '/wp/v2/posts/%d', self::$post_id ) ); + $request->set_body_params( $data ); + + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + + $meta = get_post_meta( self::$post_id, 'test_custom_name_multi', false ); + $this->assertNotEmpty( $meta ); + $this->assertCount( 1, $meta ); + $this->assertEquals( 'janet', $meta[0] ); + + // Add another value. + $data = array( + 'meta' => array( + 'new_name_multi' => array( 'janet', 'graeme' ), + ), + ); + $request->set_body_params( $data ); + + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + + $meta = get_post_meta( self::$post_id, 'test_custom_name_multi', false ); + $this->assertNotEmpty( $meta ); + $this->assertCount( 2, $meta ); + $this->assertContains( 'janet', $meta ); + $this->assertContains( 'graeme', $meta ); + } + public function test_remove_multi_value_db_error() { add_post_meta( self::$post_id, 'test_multi', 'val1' ); $values = get_post_meta( self::$post_id, 'test_multi', false ); @@ -791,6 +896,28 @@ class WP_Test_REST_Post_Meta_Fields extends WP_Test_REST_TestCase { $this->assertErrorResponse( 'rest_meta_database_error', $response, 500 ); } + public function test_delete_value_custom_name() { + add_post_meta( self::$post_id, 'test_custom_name', 'janet' ); + $current = get_post_meta( self::$post_id, 'test_custom_name', true ); + $this->assertEquals( 'janet', $current ); + + $this->grant_write_permission(); + + $data = array( + 'meta' => array( + 'new_name' => null, + ), + ); + $request = new WP_REST_Request( 'POST', sprintf( '/wp/v2/posts/%d', self::$post_id ) ); + $request->set_body_params( $data ); + + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + + $meta = get_post_meta( self::$post_id, 'test_custom_name', false ); + $this->assertEmpty( $meta ); + } + public function test_get_schema() { $request = new WP_REST_Request( 'OPTIONS', sprintf( '/wp/v2/posts/%d', self::$post_id ) ); $response = $this->server->dispatch( $request );