[wp-trac] [WordPress Trac] #60299: Default assignment of `use_ssl` user meta value causes unnecessary DB write.

[WordPress Trac]

#60299: Default assignment of `use_ssl` user meta value causes unnecessary DB
write.
-------------------------------------------------+-------------------------
 Reporter:  prettyboymp                          |       Owner:
                                                 |  adamsilverstein
     Type:  defect (bug)                         |      Status:  assigned
 Priority:  normal                               |   Milestone:  6.6
Component:  Users                                |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests has-        |     Focuses:
  testing-info needs-testing                     |  performance
-------------------------------------------------+-------------------------

Comment (by rayhatron):

 `wp_insert_user()` receives the userdata from `wp_update_user()`
 [https://github.com/WordPress/WordPress/blob/bf41fcb7ecaa2b74de865401e6ce3ee5be8635dd
 /wp-includes/user.php#L2632 here]. `wp_update_user()` receives the
 userdata from `edit_user()`
 [https://github.com/WordPress/WordPress/blob/e67e9caef43512751aae60f37d91cf589dce78b0
 /wp-admin/includes/user.php#L233 here]. In `edit_user()`, we see `use_ssl`
 being set to either 0 or 1 as integers
 [https://github.com/WordPress/WordPress/blob/e67e9caef43512751aae60f37d91cf589dce78b0
 /wp-admin/includes/user.php#L143 here].

 Coming back to `wp_insert_user`, we see
 [https://github.com/WordPress/WordPress/blob/bf41fcb7ecaa2b74de865401e6ce3ee5be8635dd
 /wp-includes/user.php#L2357 here] `$meta['use_ssl']` being set to either
 the integer 0 if `$userdata['use_ssl']` is empty otherwise it sets it to
 the boolean equivalent of what `$userdata['use_ssl']` would have.

 In the case where we have the userdata being passed as explained earlier,
 it means `$meta['use_ssl']` will either be `true` or `false`. So when
 `update_user_meta()` is called and it calls `update_metadata()`, we then
 see the old value already stored in DB (which is a string,
 [https://codex.wordpress.org/Database_Description#Table:_wp_usermeta see
 schema]) being compared to the boolean in `$meta['use_ssl']`
 [https://github.com/WordPress/WordPress/blob/bf41fcb7ecaa2b74de865401e6ce3ee5be8635dd
 /wp-includes/meta.php#L244 here].

 And those two values won't match the strict comparison since they're
 different values and data types. This then means `$wpdb->update()` is
 called
 [https://github.com/WordPress/WordPress/blob/bf41fcb7ecaa2b74de865401e6ce3ee5be8635dd
 /wp-includes/meta.php#L308 here]. This is the unnecessary db write that
 was mentioned in the description.

 Now the patch did 2 things:

 1. Set `$meta['use_ssl']` to the string '0' instead of the integer 0 if
 `$userdata['use_ssl']` is empty.
 2. Force set `$meta['use_ssl']` to the string '1' instead of attempting to
 retrieve it from the passed `$userdata['use_ssl']`.

 We have a filter `insert_user_meta` and in its documentation `$use_ssl` is
 specified to either be an int or bool. This means changing
 `$meta['use_ssl']` to be a string will potentially cause existing code in
 the community that hooks on that filter to break because the datatype has
 changed from what was originally specified.

 So a different approach that we can take is at the point of comparing, if
 the meta key is `use_ssl` then we make sure the old value retrieved from
 the database is changed to the same datatype as the new value which will
 either be an int or a boolean.

 We'll also need to make the unit tests verify that we indeed have 1 less
 db query.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/60299#comment:17>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform