WP_REST_Meta_Fields::update_meta_value handles update_metadata return value incorrectly

[wordpressscyt]

In the WP_REST_Meta_Fields::update_meta_value method the update_metadata function is used and handles every "false" return value from this function as an error.

See ​https://github.com/WordPress/wordpress-develop/blob/0b800e21c311e302824e4a025946a6fdb4f6c794/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php#L392

update_metadata returns false if the value is not updated. That also happens if the old and new value are the same. Which is obviously not an error.

[TimothyBlynJacobs]

Hi @wordpressscyt,

Welcome to trac! Yes, you are correct that update_metadata can return false in those instances which is why we first check whether the stored value is equal to the proposed new value before calling the function in the previous lines of the WP_REST_Meta_Fields::update_meta_value method.

[shanemac10]

Replying to TimothyBlynJacobs:

Hi @wordpressscyt,

Welcome to trac! Yes, you are correct that update_metadata can return false in those instances which is why we first check whether the stored value is equal to the proposed new value before calling the function in the previous lines of the WP_REST_Meta_Fields::update_meta_value method.

So I've been chasing a bug that has lead me to also find this issue as reported above, and upon closer look, I believe there is a possible issue here, and at minimum an opportunity to DRY up the code a little more.

Even though update_meta_value says // Do the exact same check for a duplicate value as in update_metadata() to avoid update_metadata() returning false. in it, it does not appear to be truly "exact". Where update_meta_value uses get_metadata, update_metadata uses get_metadata_raw. update_metadata as calls wp_unslash on $meta_value beforehand. Admittedly, I am having trouble holding it all in my head and walking through the these processes side by side, and it may be fine, but I cannot tell. Whether or not there is an issue here, it would be prudent for these functions to call upon the same DRY function, that would call and check the meta values. This would truly be "the exact same check".

Note: the meta value that is being checked and/or updated in my bug hunting instance appears to only be an integer 0 or 1. It's a value stored by a plugin, and when a post is updated it is not saved because of error returning from update_post_meta being called. I'm curious if the get_metadata_raw and get_metadata have a discrepancy in this case. If this helps in any way.