From ddf897db0518e86bceb43847580127848b355a53 Mon Sep 17 00:00:00 2001 From: Timothy Jacobs Date: Wed, 10 Jun 2020 02:20:18 +0000 Subject: [PATCH] REST API: Fix updating "multiple" meta keys with non-string values. Previously, the REST API would end up deleting each row of metadata and recreating it unnecessarily. This was caused by a type mismatch where the metadata API would always return a string value, and the REST API operated on a typed value. The REST API now applies the same sanitization and type casting for "multiple" meta keys and "single" meta keys. Fixes #49339. Props renathoc. git-svn-id: https://develop.svn.wordpress.org/trunk@47943 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, 163 insertions(+), 15 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 1de13aac19..f709afa85a 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 @@ -269,13 +269,21 @@ abstract class WP_REST_Meta_Fields { ); } - $current = get_metadata( $meta_type, $object_id, $meta_key, false ); + $current_values = get_metadata( $meta_type, $object_id, $meta_key, false ); + $subtype = get_object_subtype( $meta_type, $object_id ); - $to_remove = $current; + $to_remove = $current_values; $to_add = $values; foreach ( $to_add as $add_key => $value ) { - $remove_keys = array_keys( $to_remove, $value, true ); + $remove_keys = array_keys( + array_filter( + $current_values, + function ( $stored_value ) use ( $meta_key, $subtype, $value ) { + return $this->is_meta_value_same_as_stored_value( $meta_key, $subtype, $stored_value, $value ); + } + ) + ); if ( empty( $remove_keys ) ) { continue; @@ -359,19 +367,9 @@ abstract class WP_REST_Meta_Fields { // Do the exact same check for a duplicate value as in update_metadata() to avoid update_metadata() returning false. $old_value = get_metadata( $meta_type, $object_id, $meta_key ); $subtype = get_object_subtype( $meta_type, $object_id ); - $args = $this->get_registered_fields()[ $meta_key ]; - if ( 1 === count( $old_value ) ) { - $sanitized = sanitize_meta( $meta_key, $value, $meta_type, $subtype ); - - if ( in_array( $args['type'], array( 'string', 'number', 'integer', 'boolean' ), true ) ) { - // The return value of get_metadata will always be a string for scalar types. - $sanitized = (string) $sanitized; - } - - if ( $sanitized === $old_value[0] ) { - return true; - } + if ( 1 === count( $old_value ) && $this->is_meta_value_same_as_stored_value( $meta_key, $subtype, $old_value[0], $value ) ) { + return true; } if ( ! update_metadata( $meta_type, $object_id, wp_slash( $meta_key ), wp_slash_strings_only( $value ) ) ) { @@ -389,6 +387,29 @@ abstract class WP_REST_Meta_Fields { return true; } + /** + * Checks if the user provided value is equivalent to a stored value for the given meta key. + * + * @since 5.5.0 + * + * @param string $meta_key The meta key being checked. + * @param string $subtype The object subtype. + * @param mixed $stored_value The currently stored value retrieved from get_metadata(). + * @param mixed $user_value The value provided by the user. + * @return bool + */ + protected function is_meta_value_same_as_stored_value( $meta_key, $subtype, $stored_value, $user_value ) { + $args = $this->get_registered_fields()[ $meta_key ]; + $sanitized = sanitize_meta( $meta_key, $user_value, $this->get_meta_type(), $subtype ); + + if ( in_array( $args['type'], array( 'string', 'number', 'integer', 'boolean' ), true ) ) { + // The return value of get_metadata will always be a string for scalar types. + $sanitized = (string) $sanitized; + } + + return $sanitized === $stored_value; + } + /** * Retrieves all the registered meta fields. * 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 126871131f..9271d57bca 100644 --- a/tests/phpunit/tests/rest-api/rest-post-meta-fields.php +++ b/tests/phpunit/tests/rest-api/rest-post-meta-fields.php @@ -2655,6 +2655,133 @@ class WP_Test_REST_Post_Meta_Fields extends WP_Test_REST_TestCase { $this->assertEquals( '0', get_post_meta( self::$post_id, 'boolean', true ) ); } + /** + * @ticket 49339 + */ + public function test_update_multi_meta_value_handles_integer_types() { + $this->grant_write_permission(); + + register_post_meta( + 'post', + 'multi_integer', + array( + 'type' => 'integer', + 'show_in_rest' => true, + ) + ); + + $mid1 = add_post_meta( self::$post_id, 'multi_integer', 1 ); + $mid2 = add_post_meta( self::$post_id, 'multi_integer', 2 ); + + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/posts/%d', self::$post_id ) ); + $request->set_body_params( + array( + 'meta' => array( + 'multi_integer' => array( 2, 3 ), + ), + ) + ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertEquals( 200, $response->get_status() ); + $this->assertEquals( array( 2, 3 ), $response->get_data()['meta']['multi_integer'] ); + + $this->assertFalse( get_metadata_by_mid( 'post', $mid1 ) ); + $this->assertNotFalse( get_metadata_by_mid( 'post', $mid2 ) ); + } + + /** + * @ticket 49339 + */ + public function test_update_multi_meta_value_handles_boolean_types() { + $this->grant_write_permission(); + + register_post_meta( + 'post', + 'multi_boolean', + array( + 'type' => 'boolean', + 'sanitize_callback' => 'absint', + 'show_in_rest' => true, + ) + ); + + $mid1 = add_post_meta( self::$post_id, 'multi_boolean', 1 ); + $mid2 = add_post_meta( self::$post_id, 'multi_boolean', 0 ); + + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/posts/%d', self::$post_id ) ); + $request->set_body_params( + array( + 'meta' => array( + 'multi_boolean' => array( 0 ), + ), + ) + ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertEquals( 200, $response->get_status() ); + $this->assertEquals( array( 0 ), $response->get_data()['meta']['multi_boolean'] ); + + $this->assertFalse( get_metadata_by_mid( 'post', $mid1 ) ); + $this->assertNotFalse( get_metadata_by_mid( 'post', $mid2 ) ); + } + + /** + * @ticket 49339 + */ + public function test_update_multi_meta_value_handles_object_types() { + $this->grant_write_permission(); + + register_post_meta( + 'post', + 'multi_object', + array( + 'type' => 'object', + 'show_in_rest' => array( + 'schema' => array( + 'type' => 'object', + 'properties' => array( + 'a' => array( + 'type' => 'string', + ), + ), + ), + ), + ) + ); + + $mid1 = add_post_meta( self::$post_id, 'multi_object', array( 'a' => 'ant' ) ); + $mid2 = add_post_meta( self::$post_id, 'multi_object', array( 'a' => 'anaconda' ) ); + + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/posts/%d', self::$post_id ) ); + $request->set_body_params( + array( + 'meta' => array( + 'multi_object' => array( + array( 'a' => 'anaconda' ), + array( 'a' => 'alpaca' ), + ), + ), + ) + ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertEquals( 200, $response->get_status() ); + $this->assertEquals( + array( + array( 'a' => 'anaconda' ), + array( 'a' => 'alpaca' ), + ), + $response->get_data()['meta']['multi_object'] + ); + + $this->assertFalse( get_metadata_by_mid( 'post', $mid1 ) ); + $this->assertNotFalse( get_metadata_by_mid( 'post', $mid2 ) ); + } + /** * Internal function used to disable an insert query which * will trigger a wpdb error for testing purposes.